RE: [PATCH v6 1/4] ARM: OMAP2+: cleaned-up DT support of various ECC schemes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> > From: Rob Herring [mailto:robherring2@xxxxxxxxx]
> > > From: Pekon Gupta [mailto:pekon@xxxxxx]
> > >
> > > diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> > b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> > > index df338cb..958e5d5 100644
> > > --- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> > > +++ b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> > > @@ -21,11 +21,8 @@ Optional properties:
> > >                                 is wired that way. If not specified, a bus
> > >                                 width of 8 is assumed.
> > >
> > > - - ti,nand-ecc-opt:            A string setting the ECC layout to use. One of:
> > > -
> > > -               "sw"            Software method (default)
> > > -               "hw"            Hardware method
> > > -               "hw-romcode"    gpmc hamming mode method & romcode
> layout
> > > + - ti,nand-ecc-scheme:         A string setting the ECC layout to use. One of:
> > > +               "ham1"          1-bit Hamming ecc code
> >
> > As has been pointed out, this breaks compatibility which should be
> > avoided unless you know the state of use of this binding. I fail to
> > see the advantage of using scheme over opt. You could simply add ham1
> > here and maintain compatibility. Instead of removing sw, hw, etc. you
> > can simply deprecate them or decide that the kernel doesn't support
> > those options.
> >
> Renaming 'ti,nand-ecc-opt' to 'ti,nand-ecc-scheme' was as per earlier
> comments from Olof. So either way is fine with me. Should I revert
> it back to 'ti,nand-ecc-opt' ?
> 
> Also, "sw", "hw_romcode" have been deprecated, they are no longer
> supported in driver. So shouldn't they be removed from the documentation
> ?
> 
> > However, since you are willing to break compatibility, then you should
> > the generic NAND binding and use nand-ecc-mode adding the values you
> > need.
> >
> > Documentation/devicetree/bindings/mtd/nand.txt:
> > * MTD generic binding
> >
> > - nand-ecc-mode : String, operation mode of the NAND ecc mode.
> >   Supported values are: "none", "soft", "hw", "hw_syndrome",
> > "hw_oob_first",
> >   "soft_bch".
> 
> Yes I can use generic 'nand-ecc-mode', But the binding values like
> "soft", "hw", "soft_bch" are too generic to describe the hardware.
> - BCH ECC scheme can itself be of multiple types, bch4/bch8/bch16
>   so "soft_bch" is ambiguous.
> - Similarly "soft" and "hw" do not determine the algorithm used
>    like Hamming or BCH.
> 
> (a) Should I update the generic binding values (if no one else is using) ? like
> 	sw -> ham1_sw
> 	hw -> ham1_hw
> 	soft_bch-> soft_bch4, soft_bch8
> OR
> (b) Should I add newer ones to it (like "ham1", "bch4", "bch8", "bch16") ?
>       keeping the old ones intact. And how should I handle un-supported
>      values, (Is pr_err during kernel probe enough ?).
> 
> [...]
> 
> > > - - elm_id:     Specifies elm device node. This is required to support BCH
> > > -               error correction using ELM module.
> > > + - ti,elm-id:  Specifies pHandle of the ELM devicetree node.
> > > +               ELM is an on-chip hardware engine on TI SoC which is used for
> > > +               locating ECC errors for BCHx algorithms. SoC devices which have
> > > +               ELM hardware engines should specify this device node in .dtsi
> > > +               Using ELM for ECC error correction frees some CPU cycles.
> >
> > While yes, this is better name and documentation, I don't know that
> > breaking compatibility is justified.
> >
> As this is TI specific binding, so manufacturer's name should have been
> included.  But as this was missed while adding this binding, so this should
> be fixed now. (Olof also recommended the same).
> 
> AFAIK, For TI's NAND OMAP driver, currently there are not many
> end-users are using these bindings from mainline DT kernel.
> So this patch series mainly aims to cleanup NAND driver so that migrate
> to latest DT based kernel from board-file one is easy.
> So changes should be acceptable from end-user's point, its only matter
> of which path to take.
> Request you to please help me finalize this before I resend next patch
> series addressing other comments from Brian.
> 

+ Mark Rutland <mark.rutland@xxxxxxx>
+ Pawel Moll <pawel.moll@xxxxxxx>
+ Ian Campbell <ijc+devicetree@xxxxxxxxxxxxxx>
+ Stephen Warren <swarren@xxxxxxxxxxxxx>

CC other DT maintainers, who were missed in this branch of mail-chain.

with regards, pekon
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux