Re: [PATCH v2 1/5] dt-bindings: mtd: add Marvell NAND controller documentation

[Miquel RAYNAL <miquel.raynal@free-electrons.com>]

Hi Rob,

On Wed, 20 Dec 2017 15:05:11 -0600
Rob Herring <robh@kernel.org> wrote:

> On Tue, Dec 19, 2017 at 02:29:38PM +0100, Miquel Raynal wrote:
> > Document the legacy and the new bindings for Marvell NAND
> > controller.
> > 
> > The pxa3xx_nand.c driver does only support legacy bindings, which
> > are incomplete and inaccurate. A rework of this controller (called
> > marvell_nand.c) does support both.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
> > ---
> >  .../devicetree/bindings/mtd/marvell-nand.txt       | 123
> > +++++++++++++++++++++ 1 file changed, 123 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/mtd/marvell-nand.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/marvell-nand.txt
> > b/Documentation/devicetree/bindings/mtd/marvell-nand.txt new file
> > mode 100644 index 000000000000..aa6a1ed045b2
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/marvell-nand.txt
> > @@ -0,0 +1,123 @@
> > +Marvell NAND Flash Controller (NFC)
> > +
> > +Required properties:
> > +- compatible: can be one of the following:
> > +    * "marvell,armada-8k-nand-controller"
> > +    * "marvell,armada370-nand-controller"
> > +    * "marvell,pxa3xx-nand-controller"
> > +    * "marvell,armada-8k-nand" (deprecated)
> > +    * "marvell,armada370-nand" (deprecated)
> > +    * "marvell,pxa3xx-nand" (deprecated)
> > +  Compatibles marked deprecated support only the old bindings
> > described
> > +  at the bottom.
> > +- reg: NAND flash controller memory area.
> > +- #address-cells: shall be set to 1. Encode the NAND CS.
> > +- #size-cells: shall be set to 0.
> > +- interrupts: shall define the NAND controller interrupt.
> > +- clocks: shall reference the NAND controller clock.
> > +- marvell,system-controller: Set to retrieve the syscon node that
> > handles
> > +  NAND controller related registers (only required with the
> > +  "marvell,armada-8k-nand[-controller]" compatibles).
> > +
> > +Optional properties:
> > +- label: see partition.txt. New platforms shall omit this property.
> > +- dmas: shall reference DMA channel associated to the NAND
> > controller.
> > +  This property is only used with
> > "marvell,pxa3xx-nand[-controller]"
> > +  compatible strings.
> > +- dma-names: shall be "rxtx".
> > +  This property is only used with
> > "marvell,pxa3xx-nand[-controller]"
> > +  compatible strings.
> > +
> > +Optional children nodes:
> > +Children nodes represent the available NAND chips.
> > +
> > +Required properties:
> > +- reg: shall contain the native Chip Select ids (0-3)
> > +- marvell,rb: shall contain the native Ready/Busy ids (0-1)  
> 
> We already have at least 2 other <vendor>,rb properties. Let's not
> add a 3rd and make a common one instead.

I switched to "nand-rb", added this property in nand.txt (in another
commit) and updated all the DT accordingly.

> 
> > +
> > +Optional properties:
> > +- marvell,nand-keep-config: orders the driver not to take the
> > timings
> > +  from the core and leaving them completely untouched. Bootloader
> > +  timings will then be used.
> > +- label: MTD name.
> > +- nand-on-flash-bbt: see nand.txt.
> > +- nand-ecc-mode: see nand.txt. Will use hardware ECC if not
> > specified. +- nand-ecc-algo: see nand.txt. This property may be
> > added when using
> > +  hardware ECC for clarification but will be ignored by the driver
> > +  because ECC mode is chosen depending on the page size and the
> > strength
> > +  required by the NAND chip. This value may be overwritten with
> > +  nand-ecc-strength property.  
> 
> If not used, then drop it.

Well, nand-ecc-algo will be useful when not using the ECC
engine, so it cannot be removed. I tried to explain better the
situation.

> 
> > +- nand-ecc-strength: see nand.txt.
> > +- nand-ecc-step-size: see nand.txt. This has no effect and will be
> > +  ignored by the driver when using hardware ECC because Marvell's
> > NAND
> > +  flash controller does use fixed strength (1-bit for Hamming,
> > 16-bit
> > +  for BCH), so the step size will shrink or grow in order to fit
> > the
> > +  required strength. Step sizes are not completely random for all
> > and
> > +  follow certain patterns described in AN-379, "Marvell SoC NFC
> > ECC".  
> 
> Same here.

The comment about being ignored was not accurate, I simply removed
that part.

Thanks,
Miquèl