[PATCH 1/2] dt-bindings: mtd: brcmnand: Make nand-ecc-strength and nand-ecc-step-size optional

[PATCH 1/2] dt-bindings: mtd: brcmnand: Make nand-ecc-strength and nand-ecc-step-size optional

[Kamal Dasu]

nand-ecc-strength and nand-ecc-step-size can be made optional as
brcmanand driver can support using the nand_base driver detected
values.

Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
---
 Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
index bcda1df..29feaba 100644
--- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
+++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
@@ -101,10 +101,10 @@ Required properties:
                               number (e.g., 0, 1, 2, etc.)
 - #address-cells            : see partition.txt
 - #size-cells               : see partition.txt
-- nand-ecc-strength         : see nand.txt
-- nand-ecc-step-size        : must be 512 or 1024. See nand.txt
 
 Optional properties:
+- nand-ecc-strength         : see nand.txt
+- nand-ecc-step-size        : must be 512 or 1024. See nand.txt
 - nand-on-flash-bbt         : boolean, to enable the on-flash BBT for this
                               chip-select. See nand.txt
 - brcm,nand-oob-sector-size : integer, to denote the spare area sector size
-- 
1.9.0.138.g2de3478


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 2/2] mtd: nand: raw: brcmnand: fallback to detected ecc-strength, ecc-step-size

[Kamal Dasu]

This change supports nand-ecc-step-size and nand-ecc-strenght fields in
brcmnand dt node to be  optional.
see: Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt

If both nand-ecc-strength and nand-ecc-step-size are not specified in
device tree node for NAND, nand_base driver does detect onfi ext ecc
info from ONFI extended parameter page for parts using ONFI >= 2.1. In
case of non-onfi NAND there could be a nand_id table entry with the ecc
info. If there is a valid  device tree entry for nand-ecc-strength and
nand-ecc-step-size fields it still shall override the detected values.

Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
---
 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index ce0b8ff..e967b30 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -2144,6 +2144,16 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
 		return -EINVAL;
 	}
 
+	if (!(chip->ecc.size > 0 && chip->ecc.strength > 0) &&
+	    (chip->base.eccreq.strength > 0 &&
+	     chip->base.eccreq.step_size > 0)) {
+		/* use detected ecc parameters */
+		chip->ecc.size = chip->base.eccreq.step_size;
+		chip->ecc.strength = chip->base.eccreq.strength;
+		pr_info("Using detected nand-ecc-step-size %d, nand-ecc-strength %d\n",
+			chip->ecc.size, chip->ecc.strength);
+	}
+
 	switch (chip->ecc.size) {
 	case 512:
 		if (chip->ecc.algo == NAND_ECC_HAMMING)
-- 
1.9.0.138.g2de3478


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: nand: raw: brcmnand: fallback to detected ecc-strength, ecc-step-size

[Miquel Raynal]

Hi Kamal,

Kamal Dasu <kdasu.kdev@gmail.com> wrote on Fri, 17 May 2019 14:29:55
-0400:

> This change supports nand-ecc-step-size and nand-ecc-strenght fields in

                                                       strength

> brcmnand dt node to be  optional.

           DT            ^ extra space

> see: Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
> 
> If both nand-ecc-strength and nand-ecc-step-size are not specified in
> device tree node for NAND, nand_base driver does detect onfi ext ecc

s/nand_base driver/the raw NAND layer/
s/onfi/ONFI/
s/ecc/ECC/

What is "ext"? Please use plain English here.

> info from ONFI extended parameter page for parts using ONFI >= 2.1. In

s/info/information/

> case of non-onfi NAND there could be a nand_id table entry with the ecc

s/ecc/ECC/

> info. If there is a valid  device tree entry for nand-ecc-strength and
> nand-ecc-step-size fields it still shall override the detected values.
> 
> Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> ---
>  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index ce0b8ff..e967b30 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -2144,6 +2144,16 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
>  		return -EINVAL;
>  	}
>  
> +	if (!(chip->ecc.size > 0 && chip->ecc.strength > 0) &&

Is the case where only size OR strength is valid handled?

> +	    (chip->base.eccreq.strength > 0 &&
> +	     chip->base.eccreq.step_size > 0)) {
> +		/* use detected ecc parameters */

                   Use          ECC

> +		chip->ecc.size = chip->base.eccreq.step_size;
> +		chip->ecc.strength = chip->base.eccreq.strength;
> +		pr_info("Using detected nand-ecc-step-size %d, nand-ecc-strength %d\n",
> +			chip->ecc.size, chip->ecc.strength);
> +	}
> +
>  	switch (chip->ecc.size) {
>  	case 512:
>  		if (chip->ecc.algo == NAND_ECC_HAMMING)


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: nand: raw: brcmnand: fallback to detected ecc-strength, ecc-step-size

[Kamal Dasu]

Will make the changes and send a V2 patch.

On Mon, May 20, 2019 at 8:44 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Kamal,
>
> Kamal Dasu <kdasu.kdev@gmail.com> wrote on Fri, 17 May 2019 14:29:55
> -0400:
>
> > This change supports nand-ecc-step-size and nand-ecc-strenght fields in
>
>                                                        strength
>
> > brcmnand dt node to be  optional.
>
>            DT            ^ extra space
>
> > see: Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
> >
> > If both nand-ecc-strength and nand-ecc-step-size are not specified in
> > device tree node for NAND, nand_base driver does detect onfi ext ecc
>
> s/nand_base driver/the raw NAND layer/
> s/onfi/ONFI/
> s/ecc/ECC/
>
> What is "ext"? Please use plain English here.
>
> > info from ONFI extended parameter page for parts using ONFI >= 2.1. In
>
> s/info/information/
>
> > case of non-onfi NAND there could be a nand_id table entry with the ecc
>
> s/ecc/ECC/
>
> > info. If there is a valid  device tree entry for nand-ecc-strength and
> > nand-ecc-step-size fields it still shall override the detected values.
> >
> > Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> > ---
> >  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > index ce0b8ff..e967b30 100644
> > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > @@ -2144,6 +2144,16 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
> >               return -EINVAL;
> >       }
> >
> > +     if (!(chip->ecc.size > 0 && chip->ecc.strength > 0) &&
>
> Is the case where only size OR strength is valid handled?

Both strength and need to be valid, else the driver will behave like
before and will fail the probe.

>
> > +         (chip->base.eccreq.strength > 0 &&
> > +          chip->base.eccreq.step_size > 0)) {
> > +             /* use detected ecc parameters */
>
>                    Use          ECC
>
> > +             chip->ecc.size = chip->base.eccreq.step_size;
> > +             chip->ecc.strength = chip->base.eccreq.strength;
> > +             pr_info("Using detected nand-ecc-step-size %d, nand-ecc-strength %d\n",
> > +                     chip->ecc.size, chip->ecc.strength);
> > +     }
> > +
> >       switch (chip->ecc.size) {
> >       case 512:
> >               if (chip->ecc.algo == NAND_ECC_HAMMING)
>
>
> Thanks,
> Miquèl

Kamal

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: nand: raw: brcmnand: fallback to detected ecc-strength, ecc-step-size

[Miquel Raynal]

Hi Kamal,

Kamal Dasu <kdasu.kdev@gmail.com> wrote on Mon, 20 May 2019 13:31:52
-0400:

> Will make the changes and send a V2 patch.
> 
> On Mon, May 20, 2019 at 8:44 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Kamal,
> >
> > Kamal Dasu <kdasu.kdev@gmail.com> wrote on Fri, 17 May 2019 14:29:55
> > -0400:
> >  
> > > This change supports nand-ecc-step-size and nand-ecc-strenght fields in  
> >
> >                                                        strength
> >  
> > > brcmnand dt node to be  optional.  
> >
> >            DT            ^ extra space
> >  
> > > see: Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
> > >
> > > If both nand-ecc-strength and nand-ecc-step-size are not specified in
> > > device tree node for NAND, nand_base driver does detect onfi ext ecc  
> >
> > s/nand_base driver/the raw NAND layer/
> > s/onfi/ONFI/
> > s/ecc/ECC/
> >
> > What is "ext"? Please use plain English here.
> >  
> > > info from ONFI extended parameter page for parts using ONFI >= 2.1. In  
> >
> > s/info/information/
> >  
> > > case of non-onfi NAND there could be a nand_id table entry with the ecc  
> >
> > s/ecc/ECC/
> >  
> > > info. If there is a valid  device tree entry for nand-ecc-strength and
> > > nand-ecc-step-size fields it still shall override the detected values.
> > >
> > > Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> > > ---
> > >  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > >
> > > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > > index ce0b8ff..e967b30 100644
> > > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > > @@ -2144,6 +2144,16 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
> > >               return -EINVAL;
> > >       }
> > >
> > > +     if (!(chip->ecc.size > 0 && chip->ecc.strength > 0) &&  
> >
> > Is the case where only size OR strength is valid handled?  
> 
> Both strength and need to be valid, else the driver will behave like
> before and will fail the probe.

Yes, but you do not handle the case when either strength OR size is not
valid but the other one is. Is it one purpose?

> 
> >  
> > > +         (chip->base.eccreq.strength > 0 &&
> > > +          chip->base.eccreq.step_size > 0)) {
> > > +             /* use detected ecc parameters */  
> >
> >                    Use          ECC
> >  
> > > +             chip->ecc.size = chip->base.eccreq.step_size;
> > > +             chip->ecc.strength = chip->base.eccreq.strength;
> > > +             pr_info("Using detected nand-ecc-step-size %d, nand-ecc-strength %d\n",
> > > +                     chip->ecc.size, chip->ecc.strength);
> > > +     }
> > > +
> > >       switch (chip->ecc.size) {
> > >       case 512:
> > >               if (chip->ecc.algo == NAND_ECC_HAMMING)  
> >
> >
> > Thanks,
> > Miquèl  
> 
> Kamal




Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: nand: raw: brcmnand: fallback to detected ecc-strength, ecc-step-size

[Kamal Dasu]

On Mon, May 20, 2019 at 1:34 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Kamal,
>
> Kamal Dasu <kdasu.kdev@gmail.com> wrote on Mon, 20 May 2019 13:31:52
> -0400:
>
> > Will make the changes and send a V2 patch.
> >
> > On Mon, May 20, 2019 at 8:44 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > Hi Kamal,
> > >
> > > Kamal Dasu <kdasu.kdev@gmail.com> wrote on Fri, 17 May 2019 14:29:55
> > > -0400:
> > >
> > > > This change supports nand-ecc-step-size and nand-ecc-strenght fields in
> > >
> > >                                                        strength
> > >
> > > > brcmnand dt node to be  optional.
> > >
> > >            DT            ^ extra space
> > >
> > > > see: Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
> > > >
> > > > If both nand-ecc-strength and nand-ecc-step-size are not specified in
> > > > device tree node for NAND, nand_base driver does detect onfi ext ecc
> > >
> > > s/nand_base driver/the raw NAND layer/
> > > s/onfi/ONFI/
> > > s/ecc/ECC/
> > >
> > > What is "ext"? Please use plain English here.
> > >
> > > > info from ONFI extended parameter page for parts using ONFI >= 2.1. In
> > >
> > > s/info/information/
> > >
> > > > case of non-onfi NAND there could be a nand_id table entry with the ecc
> > >
> > > s/ecc/ECC/
> > >
> > > > info. If there is a valid  device tree entry for nand-ecc-strength and
> > > > nand-ecc-step-size fields it still shall override the detected values.
> > > >
> > > > Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> > > > ---
> > > >  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 10 ++++++++++
> > > >  1 file changed, 10 insertions(+)
> > > >
> > > > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > > > index ce0b8ff..e967b30 100644
> > > > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > > > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > > > @@ -2144,6 +2144,16 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
> > > >               return -EINVAL;
> > > >       }
> > > >
> > > > +     if (!(chip->ecc.size > 0 && chip->ecc.strength > 0) &&
> > >
> > > Is the case where only size OR strength is valid handled?
> >
> > Both strength and need to be valid, else the driver will behave like
> > before and will fail the probe.
>
> Yes, but you do not handle the case when either strength OR size is not
> valid but the other one is. Is it one purpose?
>

If I understand you want me to use the following check:

if (ecc->mode != NAND_ECC_NONE && (!ecc->size || !ecc->strength)) {
if (chip->base.eccreq.step_size && chip->base.eccreq.strength) {
     /* use the base values */
}

> >
> > >
> > > > +         (chip->base.eccreq.strength > 0 &&
> > > > +          chip->base.eccreq.step_size > 0)) {
> > > > +             /* use detected ecc parameters */
> > >
> > >                    Use          ECC
> > >
> > > > +             chip->ecc.size = chip->base.eccreq.step_size;
> > > > +             chip->ecc.strength = chip->base.eccreq.strength;
> > > > +             pr_info("Using detected nand-ecc-step-size %d, nand-ecc-strength %d\n",
> > > > +                     chip->ecc.size, chip->ecc.strength);
> > > > +     }
> > > > +
> > > >       switch (chip->ecc.size) {
> > > >       case 512:
> > > >               if (chip->ecc.algo == NAND_ECC_HAMMING)
> > >
> > >
> > > Thanks,
> > > Miquèl
> >
> > Kamal
>
>
>
>
> Thanks,
> Miquèl

Kamal

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/2] dt-bindings: mtd: brcmnand: Make nand-ecc-strength and nand-ecc-step-size optional

[Miquel Raynal]

Hi Kamal,

Kamal Dasu <kdasu.kdev@gmail.com> wrote on Fri, 17 May 2019 14:29:54
-0400:

> nand-ecc-strength and nand-ecc-step-size can be made optional as
> brcmanand driver can support using the nand_base driver detected

      ^ typo                             raw NAND layer

> values.
> 
> Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> ---

With this addressed:

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

>  Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
> index bcda1df..29feaba 100644
> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
> @@ -101,10 +101,10 @@ Required properties:
>                                number (e.g., 0, 1, 2, etc.)
>  - #address-cells            : see partition.txt
>  - #size-cells               : see partition.txt
> -- nand-ecc-strength         : see nand.txt
> -- nand-ecc-step-size        : must be 512 or 1024. See nand.txt
>  
>  Optional properties:
> +- nand-ecc-strength         : see nand.txt
> +- nand-ecc-step-size        : must be 512 or 1024. See nand.txt
>  - nand-on-flash-bbt         : boolean, to enable the on-flash BBT for this
>                                chip-select. See nand.txt
>  - brcm,nand-oob-sector-size : integer, to denote the spare area sector size


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/2] dt-bindings: mtd: brcmnand: Make nand-ecc-strength and nand-ecc-step-size optional

[Rob Herring]

On Fri, 17 May 2019 14:29:54 -0400, Kamal Dasu wrote:
> nand-ecc-strength and nand-ecc-step-size can be made optional as
> brcmanand driver can support using the nand_base driver detected
> values.
> 
> Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> ---
>  Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/