Re: [RFC PATCH 1/5] mtd: nand: ecc-qcom: Add support for ECC Engine Driver

From: Md Sadre Alam
Date: Mon Nov 20 2023 - 01:41:10 EST




On 11/3/2023 7:16 PM, Miquel Raynal wrote:
Hello,

Based on below feedback [1] and NAK on the device node patch
got idea of having separate device node for ECC is not acceptable.
Could you please help to clarify that.

If I may try to compare with the Macronix situation, the ECC engine
was an independent hardware block, with its own mapping and its own
registers, so it was described as an independent node in the DT. The
type of ECC controller (pipelined or external) is described by the
nand-ecc-engine property which either points at the parent node
(pipelined) or an external node (external). The SPI host would itself
point at the external ECC engine node with its own nand-ecc-engine
property (see mtd/mxicy,nand-ecc-engine.yaml in the bindings).

Sorry for late reply.

Since QPIC controller ECC engine is not a separate HW block. To control ECC
functionality there is only one register 4-bytes long.As you suggested above,
ECC controller described by the property nand-ecc-engine.I have checked
mtd/mxicy,nand-ecc-engine.yaml file and got to know I can use like
nand-ecc-engine = <&qpic_nand>; in dts.Now additional ECC node not needed
in DTS. Will clean up everything in next patch.


Since ECC block is inlined with QPIC controller so is the below
device node acceptable ?

bch: qpic_ecc {
compatible = "qcom,ipq9574-ecc";
status = "ok";
};

If it does not has its own mapping and if you access the ECC engine
through the host registers then the controller should be part of the
host node, but I am not sure it really needs to be described
explicitly, most of them are not for historical reasons. Conceptually
there is a problem with subnodes of each of these controllers having
a signification already: SPI devices or NAND chips.

New device node for spi nand looks like as below.

&qpic_nand {
status = "okay";
pinctrl-0 = <&qspi_nand_pins>;
pinctrl-names = "default";
spi_nand: spi_nand@0 {
compatible = "spi-nand";
reg = <0>;
#address-cells = <1>;
#size-cells = <1>;
nand-ecc-engine = <&qpic_nand>;
nand-ecc-strength = <4>;
nand-ecc-step-size = <512>;
spi-max-frequency = <8000000>;
};
};

With the above device node I have tested the spi nand device enumeration
its working fine. Will cleanup everything and post in next patch.



Thanks,
Miquèl