[RFC/PATCH 0/1] mtd: Add NAND ECC devicetree binding

[RFC/PATCH 0/1] mtd: Add NAND ECC devicetree binding

[Ezequiel Garcia]

This patch is our first proposal to address the need for a suitable ECC
devicetree binding.

NAND controllers have special ECC modes, raising per-driver ECC mode devicetree
binding. See for instance the binding for OMAP:

 - ti,nand-ecc-opt: A string setting the ECC layout to use. One of:
	"sw"		<deprecated> use "ham1" instead
	"hw"		<deprecated> use "ham1" instead
	"hw-romcode"	<deprecated> use "ham1" instead
	"ham1"		1-bit Hamming ecc code
	"bch4"		4-bit BCH ecc code
	"bch8"		8-bit BCH ecc code

Other drivers (such as pxa3xx-nand) have similar requirements, with special
(controller-specific) ECC modes. Instead of adding a possibly different binding
per compatible-string, let's add generic ECC strength and ECC step size.

This properties should describe completely the ECC mode and let drivers choose
the appropriate ECC mode.

Ezequiel Garcia (1):
  mtd: nand: Add a devicetree binding for ECC strength and ECC step size

 Documentation/devicetree/bindings/mtd/nand.txt | 4 ++++
 1 file changed, 4 insertions(+)

-- 
1.8.1.5

[RFC/PATCH 1/1] mtd: nand: Add a devicetree binding for ECC strength and ECC step size

[Ezequiel Garcia]

Some flashes can only be properly accessed when the ECC mode is
specified, and a way to describe such mode is required.

Such ECC mode is completely driver-specific so instead of having one binding
per compatible-string, let's add generic ECC strength and ECC step size.
Driver's can choose the appropriate ECC mode, based on this specification.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 Documentation/devicetree/bindings/mtd/nand.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
index 03855c8..683a310 100644
--- a/Documentation/devicetree/bindings/mtd/nand.txt
+++ b/Documentation/devicetree/bindings/mtd/nand.txt
@@ -3,5 +3,9 @@
 - nand-ecc-mode : String, operation mode of the NAND ecc mode.
   Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
   "soft_bch".
+- nand-ecc-strength : integer ECC required strength.
+- nand-ecc-size : integer step size associated to the ECC strength.
+  The exact meaning of the ECC strength and ECC size parameters is completely
+  driver-specific.
 - nand-bus-width : 8 or 16 bus width if not present 8
 - nand-on-flash-bbt: boolean to enable on flash bbt option if not present false
-- 
1.8.1.5

RE: [RFC/PATCH 0/1] mtd: Add NAND ECC devicetree binding

[Gupta, Pekon]

Hi Ezequiel,

>From: Ezequiel Garcia [mailto:ezequiel.garcia@free-electrons.com]
>
>This patch is our first proposal to address the need for a suitable ECC
>devicetree binding.
>
>NAND controllers have special ECC modes, raising per-driver ECC mode devicetree
>binding. See for instance the binding for OMAP:
>
> - ti,nand-ecc-opt: A string setting the ECC layout to use. One of:
>	"sw"		<deprecated> use "ham1" instead
>	"hw"		<deprecated> use "ham1" instead
>	"hw-romcode"	<deprecated> use "ham1" instead
>	"ham1"		1-bit Hamming ecc code
>	"bch4"		4-bit BCH ecc code
>	"bch8"		8-bit BCH ecc code
>
>Other drivers (such as pxa3xx-nand) have similar requirements, with special
>(controller-specific) ECC modes. Instead of adding a possibly different binding
>per compatible-string, let's add generic ECC strength and ECC step size.
>
>This properties should describe completely the ECC mode and let drivers choose
>the appropriate ECC mode.
>
Yes, this is good approach.
It was found earlier that generic NAND DT bindings are not much use to other
controllers as well, as different h/w engines have different interpretations.
Brian Norris had similar comments giving example of his hardware.
(hope following reference helps).

[1] http://lists.infradead.org/pipermail/linux-mtd/2013-September/048869.html

with regards, pekon

Re: [RFC/PATCH 0/1] mtd: Add NAND ECC devicetree binding

[Ezequiel Garcia]

On Fri, Jan 17, 2014 at 05:58:13PM +0000, Gupta, Pekon wrote:
> Hi Ezequiel,
> 
> >From: Ezequiel Garcia [mailto:ezequiel.garcia@free-electrons.com]
> >
> >This patch is our first proposal to address the need for a suitable ECC
> >devicetree binding.
> >
> >NAND controllers have special ECC modes, raising per-driver ECC mode devicetree
> >binding. See for instance the binding for OMAP:
> >
> > - ti,nand-ecc-opt: A string setting the ECC layout to use. One of:
> >	"sw"		<deprecated> use "ham1" instead
> >	"hw"		<deprecated> use "ham1" instead
> >	"hw-romcode"	<deprecated> use "ham1" instead
> >	"ham1"		1-bit Hamming ecc code
> >	"bch4"		4-bit BCH ecc code
> >	"bch8"		8-bit BCH ecc code
> >
> >Other drivers (such as pxa3xx-nand) have similar requirements, with special
> >(controller-specific) ECC modes. Instead of adding a possibly different binding
> >per compatible-string, let's add generic ECC strength and ECC step size.
> >
> >This properties should describe completely the ECC mode and let drivers choose
> >the appropriate ECC mode.
> >
> Yes, this is good approach.
> It was found earlier that generic NAND DT bindings are not much use to other
> controllers as well, as different h/w engines have different interpretations.
> Brian Norris had similar comments giving example of his hardware.
> (hope following reference helps).
> 
> [1] http://lists.infradead.org/pipermail/linux-mtd/2013-September/048869.html
> 

Yes, Brian suggested this ecc-strength/ecc-size approach on IRC.

Pekon, do you think this binding proposal is good enough to describe OMAP NAND
ECC mode?

I'm not implying we should deprecate the recently added "ti-nand-ecc-opt",
but just want to know it's eventually possible.

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

RE: [RFC/PATCH 0/1] mtd: Add NAND ECC devicetree binding

[Gupta, Pekon]

Hi Ezequiel,

>From: Ezequiel Garcia 
>>On Fri, Jan 17, 2014 at 05:58:13PM +0000, Gupta, Pekon wrote:
[...]

>> Yes, this is good approach.
>> It was found earlier that generic NAND DT bindings are not much use to other
>> controllers as well, as different h/w engines have different interpretations.
>> Brian Norris had similar comments giving example of his hardware.
>> (hope following reference helps).
>>
>> [1] http://lists.infradead.org/pipermail/linux-mtd/2013-September/048869.html
>>
>
>Yes, Brian suggested this ecc-strength/ecc-size approach on IRC.
>
>Pekon, do you think this binding proposal is good enough to describe OMAP NAND
>ECC mode?
>
>I'm not implying we should deprecate the recently added "ti-nand-ecc-opt",
>but just want to know it's eventually possible.
>
Yes, this is good approach for long-term, and it can replace "ti-nand-ecc-opt"
"ti-nand-ecc-opt" is not new DT binding, it just got some new values added
However, you have to convince DT Maintainers to get this in, and then deprecate
other vendor specific bindings. It would be difficult to maintain backward
compatibility to these bindings, if we move to 'nand-ecc-strength'.

At some-point we need to get some concrete guidelines from DT Maintainers on
how long we should support deprecated bindings in our code, And what is the
age of DT binding. I think David Woodhouse should throw more light, as he had
some discussions & ideas on about DT binding life, during a linux conference.


with regards, pekon

Re: [RFC/PATCH 0/1] mtd: Add NAND ECC devicetree binding

[Ezequiel Garcia]

On Mon, Jan 20, 2014 at 06:21:35AM +0000, Gupta, Pekon wrote:
[..]
> >
> >Pekon, do you think this binding proposal is good enough to describe OMAP NAND
> >ECC mode?
> >
> >I'm not implying we should deprecate the recently added "ti-nand-ecc-opt",
> >but just want to know it's eventually possible.
> >
> Yes, this is good approach for long-term, and it can replace "ti-nand-ecc-opt"
> "ti-nand-ecc-opt" is not new DT binding, it just got some new values added
> However, you have to convince DT Maintainers to get this in, and then deprecate
> other vendor specific bindings. It would be difficult to maintain backward
> compatibility to these bindings, if we move to 'nand-ecc-strength'.
> 

Putting the DT maintainers on the To: field to get some feedback.

> At some-point we need to get some concrete guidelines from DT Maintainers on
> how long we should support deprecated bindings in our code, And what is the
> age of DT binding. I think David Woodhouse should throw more light, as he had
> some discussions & ideas on about DT binding life, during a linux conference.
> 

That's a good question :-)
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [RFC/PATCH 0/1] mtd: Add NAND ECC devicetree binding

[Brian Norris]

On Fri, Jan 17, 2014 at 05:33:48PM -0300, Ezequiel Garcia wrote:
> On Fri, Jan 17, 2014 at 05:58:13PM +0000, Gupta, Pekon wrote:
> > Hi Ezequiel,
> > 
> > >From: Ezequiel Garcia [mailto:ezequiel.garcia@free-electrons.com]
> > >
> > >This patch is our first proposal to address the need for a suitable ECC
> > >devicetree binding.
> > >
> > >NAND controllers have special ECC modes, raising per-driver ECC mode devicetree
> > >binding. See for instance the binding for OMAP:
> > >
> > > - ti,nand-ecc-opt: A string setting the ECC layout to use. One of:
> > >	"sw"		<deprecated> use "ham1" instead
> > >	"hw"		<deprecated> use "ham1" instead
> > >	"hw-romcode"	<deprecated> use "ham1" instead
> > >	"ham1"		1-bit Hamming ecc code
> > >	"bch4"		4-bit BCH ecc code
> > >	"bch8"		8-bit BCH ecc code
> > >
> > >Other drivers (such as pxa3xx-nand) have similar requirements, with special
> > >(controller-specific) ECC modes. Instead of adding a possibly different binding
> > >per compatible-string, let's add generic ECC strength and ECC step size.
> > >
> > >This properties should describe completely the ECC mode and let drivers choose
> > >the appropriate ECC mode.
> > >
> > Yes, this is good approach.
> > It was found earlier that generic NAND DT bindings are not much use to other
> > controllers as well, as different h/w engines have different interpretations.
> > Brian Norris had similar comments giving example of his hardware.
> > (hope following reference helps).
> > 
> > [1] http://lists.infradead.org/pipermail/linux-mtd/2013-September/048869.html
> > 
> 
> Yes, Brian suggested this ecc-strength/ecc-size approach on IRC.

And for the record, I also alluded to it in the last paragraph of the
email Pekon linked to:

  So I'm not convinced that extending this nand-ecc-mode property is
  correct at all. But if we do want to, perhaps we'd need to introduce
  additional orthogonal properties to specify strength and step size,
  rather than listing all combinations as separate values for
  nand-ecc-mode.

[IOW, nand-ecc-strength and nand-ecc-step-size]

Brian

Re: [RFC/PATCH 0/1] mtd: Add NAND ECC devicetree binding

[Brian Norris]

On Mon, Jan 20, 2014 at 06:21:35AM +0000, Pekon Gupta wrote:
> >From: Ezequiel Garcia 
> >I'm not implying we should deprecate the recently added "ti-nand-ecc-opt",
> >but just want to know it's eventually possible.
> >
> Yes, this is good approach for long-term, and it can replace "ti-nand-ecc-opt"
> "ti-nand-ecc-opt" is not new DT binding, it just got some new values added
> However, you have to convince DT Maintainers to get this in, and then deprecate
> other vendor specific bindings. It would be difficult to maintain backward
> compatibility to these bindings, if we move to 'nand-ecc-strength'.

I don't see why it would be difficult to support both the more generic
nand-ecc-strength and the OMAP-specific ti-nand-ecc-opt in the same
driver. OMAP NAND would just have to define a helper that translates
properties like "bch8" into a strength and sector size (8 and 512,
respectively). From then on, you can treat them identically.

> At some-point we need to get some concrete guidelines from DT Maintainers on
> how long we should support deprecated bindings in our code, And what is the
> age of DT binding. I think David Woodhouse should throw more light, as he had
> some discussions & ideas on about DT binding life, during a linux conference.

Barring some new direction from the DT folks, I believe DT is an eternal
ABI. We can and should maintain compatibility forever. And in this case,
I don't think it's that hard.

Brian

Re: [RFC/PATCH 1/1] mtd: nand: Add a devicetree binding for ECC strength and ECC step size

[Brian Norris]

Hi Ezequiel,

On Fri, Jan 17, 2014 at 09:13:40AM -0300, Ezequiel Garcia wrote:
> Some flashes can only be properly accessed when the ECC mode is
> specified, and a way to describe such mode is required.
> 
> Such ECC mode is completely driver-specific so instead of having one binding
> per compatible-string, let's add generic ECC strength and ECC step size.
> Driver's can choose the appropriate ECC mode, based on this specification.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  Documentation/devicetree/bindings/mtd/nand.txt | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
> index 03855c8..683a310 100644
> --- a/Documentation/devicetree/bindings/mtd/nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> @@ -3,5 +3,9 @@
>  - nand-ecc-mode : String, operation mode of the NAND ecc mode.
>    Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
>    "soft_bch".
> +- nand-ecc-strength : integer ECC required strength.
> +- nand-ecc-size : integer step size associated to the ECC strength.

Probably should be nand-ecc-step-size, to be clearer.

> +  The exact meaning of the ECC strength and ECC size parameters is completely
> +  driver-specific.

I think we can be much more specific than this. We need to at least
describe how the strength and size are related, and we need to mention
how this represents the ECC scheme to use, rather than the minimum
requirement of the flash.

I'd say something like this. Feel free to improve it, but it covers the
gist of what I think we can say in a generic document:

 - nand-ecc-strength: integer representing the number of bits to correct
   per ECC step
 - nand-ecc-step-size: integer representing the number of data bytes
   that are covered by a single ECC step

   Together, the ECC strength and step size define the correction
   capability, so that we say we will correct "X bit errors per Y
   bytes". The interpretation of these parameters is
   implementation-defined, but they often have ramifications on the
   formation, interpretation, and placement of correction metadata on
   the flash. Not all implementations must support all possible
   combinations. Implementations are encouraged to further define the
   value(s) they support.

>  - nand-bus-width : 8 or 16 bus width if not present 8
>  - nand-on-flash-bbt: boolean to enable on flash bbt option if not present false

Brian

Re: [RFC/PATCH 1/1] mtd: nand: Add a devicetree binding for ECC strength and ECC step sizeç

[Ezequiel Garcia]

On Wed, Feb 12, 2014 at 12:00:14AM -0800, Brian Norris wrote:
> Hi Ezequiel,
> 
> On Fri, Jan 17, 2014 at 09:13:40AM -0300, Ezequiel Garcia wrote:
> > Some flashes can only be properly accessed when the ECC mode is
> > specified, and a way to describe such mode is required.
> > 
> > Such ECC mode is completely driver-specific so instead of having one binding
> > per compatible-string, let's add generic ECC strength and ECC step size.
> > Driver's can choose the appropriate ECC mode, based on this specification.
> > 
> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > ---
> >  Documentation/devicetree/bindings/mtd/nand.txt | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
> > index 03855c8..683a310 100644
> > --- a/Documentation/devicetree/bindings/mtd/nand.txt
> > +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> > @@ -3,5 +3,9 @@
> >  - nand-ecc-mode : String, operation mode of the NAND ecc mode.
> >    Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
> >    "soft_bch".
> > +- nand-ecc-strength : integer ECC required strength.
> > +- nand-ecc-size : integer step size associated to the ECC strength.
> 
> Probably should be nand-ecc-step-size, to be clearer.
> 

No problem.

> > +  The exact meaning of the ECC strength and ECC size parameters is completely
> > +  driver-specific.
> 
> I think we can be much more specific than this. We need to at least
> describe how the strength and size are related, and we need to mention
> how this represents the ECC scheme to use, rather than the minimum
> requirement of the flash.
> 
> I'd say something like this. Feel free to improve it, but it covers the
> gist of what I think we can say in a generic document:
> 
>  - nand-ecc-strength: integer representing the number of bits to correct
>    per ECC step
>  - nand-ecc-step-size: integer representing the number of data bytes
>    that are covered by a single ECC step
> 
>    Together, the ECC strength and step size define the correction
>    capability, so that we say we will correct "X bit errors per Y
>    bytes". The interpretation of these parameters is
>    implementation-defined, but they often have ramifications on the
>    formation, interpretation, and placement of correction metadata on
>    the flash. Not all implementations must support all possible
>    combinations. Implementations are encouraged to further define the
>    value(s) they support.
> 

Much better, thanks!
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com