diff mbox

[RFC,v2,03/14] of: mtd: add documentation for nand-ecc-level property

Message ID 1391006064-28890-4-git-send-email-b.brezillon.dev@gmail.com
State Superseded
Headers show

Commit Message

Boris Brezillon Jan. 29, 2014, 2:34 p.m. UTC
nand-ecc-level property statically defines NAND chip's ECC requirements.

Signed-off-by: Boris BREZILLON <b.brezillon.dev@gmail.com>
---
 Documentation/devicetree/bindings/mtd/nand.txt |    3 +++
 1 file changed, 3 insertions(+)

Comments

Ezequiel Garcia Jan. 29, 2014, 5:53 p.m. UTC | #1
On Wed, Jan 29, 2014 at 03:34:13PM +0100, Boris BREZILLON wrote:
> nand-ecc-level property statically defines NAND chip's ECC requirements.
> 
> Signed-off-by: Boris BREZILLON <b.brezillon.dev@gmail.com>
> ---
>  Documentation/devicetree/bindings/mtd/nand.txt |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
> index 03855c8..0c962296 100644
> --- a/Documentation/devicetree/bindings/mtd/nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> @@ -3,5 +3,8 @@
>  - 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-level : Two cells property defining the ECC level requirements.
> +  The first cell represent the strength and the second cell the ECC block size.
> +  E.g. : nand-ecc-level = <4 512>; /* 4 bits / 512 bytes */
>  - 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

Hm.. when was this proposal agreed? It seems I've missed the
discussion...

FWIW, we've already proposed an equivalent one, but it received no
feedback from the devicetree maintainers:

http://comments.gmane.org/gmane.linux.drivers.devicetree/58764

Maybe we can discuss about it now?

  nand-ecc-strength : integer ECC required strength.
  nand-ecc-size : integer step size associated to the ECC strength.

  vs.

  nand-ecc-level : Two cells property defining the ECC level requirements.
  The first cell represent the strength and the second cell the ECC block size.
  E.g. : nand-ecc-level = <4 512>; /* 4 bits / 512 bytes */

It's really the same proposal but with a different format, right?
IMHO, the former is more human-readable, but other than that I see no
difference.

Brian? DT-guys?
Boris Brezillon Jan. 29, 2014, 6:39 p.m. UTC | #2
Hello Ezequiel

Le 29/01/2014 18:53, Ezequiel Garcia a écrit :
> On Wed, Jan 29, 2014 at 03:34:13PM +0100, Boris BREZILLON wrote:
>> nand-ecc-level property statically defines NAND chip's ECC requirements.
>>
>> Signed-off-by: Boris BREZILLON <b.brezillon.dev@gmail.com>
>> ---
>>   Documentation/devicetree/bindings/mtd/nand.txt |    3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
>> index 03855c8..0c962296 100644
>> --- a/Documentation/devicetree/bindings/mtd/nand.txt
>> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
>> @@ -3,5 +3,8 @@
>>   - 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-level : Two cells property defining the ECC level requirements.
>> +  The first cell represent the strength and the second cell the ECC block size.
>> +  E.g. : nand-ecc-level = <4 512>; /* 4 bits / 512 bytes */
>>   - 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
> Hm.. when was this proposal agreed?
Never, this is a proposal based on my needs, and this was not present in the
1st version of this series :-).
> It seems I've missed the
> discussion...
>
> FWIW, we've already proposed an equivalent one, but it received no
> feedback from the devicetree maintainers:
>
> http://comments.gmane.org/gmane.linux.drivers.devicetree/58764
>
> Maybe we can discuss about it now?
>
>    nand-ecc-strength : integer ECC required strength.
>    nand-ecc-size : integer step size associated to the ECC strength.
>
>    vs.
>
>    nand-ecc-level : Two cells property defining the ECC level requirements.
>    The first cell represent the strength and the second cell the ECC block size.
>    E.g. : nand-ecc-level = <4 512>; /* 4 bits / 512 bytes */
>
> It's really the same proposal but with a different format, right?

Yes it is.

> IMHO, the former is more human-readable, but other than that I see no
> difference.

As I already said to Pekon, I won't complain if my proposal is not chosen,
as long as there is a proper way to define these ECC requirements ;-).

Best Regards,

Boris

>
> Brian? DT-guys?
Grant Likely Feb. 5, 2014, 11:15 a.m. UTC | #3
On Wed, 29 Jan 2014 14:53:32 -0300, Ezequiel Garcia <ezequiel.garcia@free-electrons.com> wrote:
> On Wed, Jan 29, 2014 at 03:34:13PM +0100, Boris BREZILLON wrote:
> > nand-ecc-level property statically defines NAND chip's ECC requirements.
> > 
> > Signed-off-by: Boris BREZILLON <b.brezillon.dev@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/mtd/nand.txt |    3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
> > index 03855c8..0c962296 100644
> > --- a/Documentation/devicetree/bindings/mtd/nand.txt
> > +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> > @@ -3,5 +3,8 @@
> >  - 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-level : Two cells property defining the ECC level requirements.
> > +  The first cell represent the strength and the second cell the ECC block size.
> > +  E.g. : nand-ecc-level = <4 512>; /* 4 bits / 512 bytes */
> >  - 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
> 
> Hm.. when was this proposal agreed? It seems I've missed the
> discussion...
> 
> FWIW, we've already proposed an equivalent one, but it received no
> feedback from the devicetree maintainers:

Sorry, binding review has become a huge undertaking.

> 
> http://comments.gmane.org/gmane.linux.drivers.devicetree/58764
> 
> Maybe we can discuss about it now?
> 
>   nand-ecc-strength : integer ECC required strength.
>   nand-ecc-size : integer step size associated to the ECC strength.

I'm okay with either, but the above binding is indeed more readable.

g.
Boris Brezillon Feb. 5, 2014, 1:34 p.m. UTC | #4
On 05/02/2014 12:15, Grant Likely wrote:
> On Wed, 29 Jan 2014 14:53:32 -0300, Ezequiel Garcia <ezequiel.garcia@free-electrons.com> wrote:
>> On Wed, Jan 29, 2014 at 03:34:13PM +0100, Boris BREZILLON wrote:
>>> nand-ecc-level property statically defines NAND chip's ECC requirements.
>>>
>>> Signed-off-by: Boris BREZILLON <b.brezillon.dev@gmail.com>
>>> ---
>>>   Documentation/devicetree/bindings/mtd/nand.txt |    3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
>>> index 03855c8..0c962296 100644
>>> --- a/Documentation/devicetree/bindings/mtd/nand.txt
>>> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
>>> @@ -3,5 +3,8 @@
>>>   - 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-level : Two cells property defining the ECC level requirements.
>>> +  The first cell represent the strength and the second cell the ECC block size.
>>> +  E.g. : nand-ecc-level = <4 512>; /* 4 bits / 512 bytes */
>>>   - 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
>> Hm.. when was this proposal agreed? It seems I've missed the
>> discussion...
>>
>> FWIW, we've already proposed an equivalent one, but it received no
>> feedback from the devicetree maintainers:
> Sorry, binding review has become a huge undertaking.
>
>> http://comments.gmane.org/gmane.linux.drivers.devicetree/58764
>>
>> Maybe we can discuss about it now?
>>
>>    nand-ecc-strength : integer ECC required strength.
>>    nand-ecc-size : integer step size associated to the ECC strength.
> I'm okay with either, but the above binding is indeed more readable.

That's fine by me, if everybody agrees, let's go for the
nand-ecc-strength/nand-ecc-size couple then.

I'll rebase next version of my series on Ezequiel's patch providing
these OF helpers.

Best Regards,

Boris

>
> g.
Ezequiel Garcia Feb. 5, 2014, 2:19 p.m. UTC | #5
Hi Grant, Boris:

(BTW, dropped Russell, Rob Landley and some unrelated mailing lists from Cc,
and added Thomas, Gregory and Rob Herring).

On Wed, Feb 05, 2014 at 02:34:44PM +0100, Boris BREZILLON wrote:
> On 05/02/2014 12:15, Grant Likely wrote:
> > On Wed, 29 Jan 2014 14:53:32 -0300, Ezequiel Garcia <ezequiel.garcia@free-electrons.com> wrote:
[..]
> >>
> >> Maybe we can discuss about it now?
> >>
> >>    nand-ecc-strength : integer ECC required strength.
> >>    nand-ecc-size : integer step size associated to the ECC strength.
> > I'm okay with either, but the above binding is indeed more readable.
> 
> That's fine by me, if everybody agrees, let's go for the
> nand-ecc-strength/nand-ecc-size couple then.
> 

Great. So, if some DT dictator^C^Cmaintainers can Ack this binding,
I can send a new patchset, with pxa3xx-nand using it...

> I'll rebase next version of my series on Ezequiel's patch providing
> these OF helpers.
> 

... and then you can base on it?

This is the original patchset:

http://permalink.gmane.org/gmane.linux.drivers.devicetree/58764
http://permalink.gmane.org/gmane.linux.drivers.devicetree/58763
Grant Likely Feb. 17, 2014, 4:43 p.m. UTC | #6
On Wed, Feb 5, 2014 at 2:19 PM, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> Hi Grant, Boris:
>
> (BTW, dropped Russell, Rob Landley and some unrelated mailing lists from Cc,
> and added Thomas, Gregory and Rob Herring).
>
> On Wed, Feb 05, 2014 at 02:34:44PM +0100, Boris BREZILLON wrote:
>> On 05/02/2014 12:15, Grant Likely wrote:
>> > On Wed, 29 Jan 2014 14:53:32 -0300, Ezequiel Garcia <ezequiel.garcia@free-electrons.com> wrote:
> [..]
>> >>
>> >> Maybe we can discuss about it now?
>> >>
>> >>    nand-ecc-strength : integer ECC required strength.
>> >>    nand-ecc-size : integer step size associated to the ECC strength.
>> > I'm okay with either, but the above binding is indeed more readable.
>>
>> That's fine by me, if everybody agrees, let's go for the
>> nand-ecc-strength/nand-ecc-size couple then.
>>
>
> Great. So, if some DT dictator^C^Cmaintainers can Ack this binding,
> I can send a new patchset, with pxa3xx-nand using it...
>
>> I'll rebase next version of my series on Ezequiel's patch providing
>> these OF helpers.
>>
>
> ... and then you can base on it?
>
> This is the original patchset:
>
> http://permalink.gmane.org/gmane.linux.drivers.devicetree/58764
> http://permalink.gmane.org/gmane.linux.drivers.devicetree/58763

I've looked at both. Go ahead and add my a-b line:

Acked-by: Grant Likely <grant.likely@linaro.org>
Ezequiel Garcia Feb. 17, 2014, 6:19 p.m. UTC | #7
On Mon, Feb 17, 2014 at 04:43:51PM +0000, Grant Likely wrote:
> On Wed, Feb 5, 2014 at 2:19 PM, Ezequiel Garcia
> <ezequiel.garcia@free-electrons.com> wrote:
> > Hi Grant, Boris:
> >
> > (BTW, dropped Russell, Rob Landley and some unrelated mailing lists from Cc,
> > and added Thomas, Gregory and Rob Herring).
> >
> > On Wed, Feb 05, 2014 at 02:34:44PM +0100, Boris BREZILLON wrote:
> >> On 05/02/2014 12:15, Grant Likely wrote:
> >> > On Wed, 29 Jan 2014 14:53:32 -0300, Ezequiel Garcia <ezequiel.garcia@free-electrons.com> wrote:
> > [..]
> >> >>
> >> >> Maybe we can discuss about it now?
> >> >>
> >> >>    nand-ecc-strength : integer ECC required strength.
> >> >>    nand-ecc-size : integer step size associated to the ECC strength.
> >> > I'm okay with either, but the above binding is indeed more readable.
> >>
> >> That's fine by me, if everybody agrees, let's go for the
> >> nand-ecc-strength/nand-ecc-size couple then.
> >>
> >
> > Great. So, if some DT dictator^C^Cmaintainers can Ack this binding,
> > I can send a new patchset, with pxa3xx-nand using it...
> >
> >> I'll rebase next version of my series on Ezequiel's patch providing
> >> these OF helpers.
> >>
> >
> > ... and then you can base on it?
> >
> > This is the original patchset:
> >
> > http://permalink.gmane.org/gmane.linux.drivers.devicetree/58764
> > http://permalink.gmane.org/gmane.linux.drivers.devicetree/58763
> 
> I've looked at both. Go ahead and add my a-b line:
> 
> Acked-by: Grant Likely <grant.likely@linaro.org>

Cool. Thanks!

I'll push a patchset now.
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
index 03855c8..0c962296 100644
--- a/Documentation/devicetree/bindings/mtd/nand.txt
+++ b/Documentation/devicetree/bindings/mtd/nand.txt
@@ -3,5 +3,8 @@ 
 - 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-level : Two cells property defining the ECC level requirements.
+  The first cell represent the strength and the second cell the ECC block size.
+  E.g. : nand-ecc-level = <4 512>; /* 4 bits / 512 bytes */
 - 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