2019-05-20 19:07:30

by Kamal Dasu

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

nand-ecc-strength and nand-ecc-step-size can be made optional as
brcmnand driver can support using raw NAND layer detected values.

Signed-off-by: Kamal Dasu <[email protected]>
---
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



2019-05-20 19:08:05

by Kamal Dasu

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

This change supports nand-ecc-step-size and nand-ecc-strength 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, raw NAND layer does detect ECC information by
reading ONFI extended parameter page for parts using ONFI >= 2.1.
In case of non-ONFI NAND parts there could be a nand_id table entry with
ECC information. If there is 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 <[email protected]>
---
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index ce0b8ff..a4d2057 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -2144,6 +2144,17 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
return -EINVAL;
}

+ if (chip->ecc.mode != NAND_ECC_NONE &&
+ (!chip->ecc.size || !chip->ecc.strength)) {
+ if (chip->base.eccreq.step_size && chip->base.eccreq.strength) {
+ /* use detected ECC parameters */
+ chip->ecc.size = chip->base.eccreq.step_size;
+ chip->ecc.strength = chip->base.eccreq.strength;
+ pr_info("Using ECC step-size %d, 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


2019-05-20 19:13:30

by Florian Fainelli

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

On 5/20/19 12:05 PM, Kamal Dasu wrote:
> This change supports nand-ecc-step-size and nand-ecc-strength 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, raw NAND layer does detect ECC information by
> reading ONFI extended parameter page for parts using ONFI >= 2.1.
> In case of non-ONFI NAND parts there could be a nand_id table entry with
> ECC information. If there is 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 <[email protected]>
> ---
> drivers/mtd/nand/raw/brcmnand/brcmnand.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index ce0b8ff..a4d2057 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -2144,6 +2144,17 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
> return -EINVAL;
> }
>
> + if (chip->ecc.mode != NAND_ECC_NONE &&
> + (!chip->ecc.size || !chip->ecc.strength)) {
> + if (chip->base.eccreq.step_size && chip->base.eccreq.strength) {
> + /* use detected ECC parameters */
> + chip->ecc.size = chip->base.eccreq.step_size;
> + chip->ecc.strength = chip->base.eccreq.strength;
> + pr_info("Using ECC step-size %d, strength %d\n",
> + chip->ecc.size, chip->ecc.strength);

Nit: should not we use dev_info(&host->pdev->dev) for printing the
message in case we have multiple NAND controllers on chip, that way we
can still differentiate them from the prints?
--
Florian

2019-05-21 08:55:19

by Miquel Raynal

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

Hi Kamal,

Kamal Dasu <[email protected]> wrote on Mon, 20 May 2019 15:05:11
-0400:

> nand-ecc-strength and nand-ecc-step-size can be made optional as
> brcmnand driver can support using raw NAND layer detected values.
>
> Signed-off-by: Kamal Dasu <[email protected]>
> ---
> 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


Reviewed-by: Miquel Raynal <[email protected]>


Thanks,
Miquèl

2019-05-21 08:55:25

by Miquel Raynal

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

Hi Florian,

Florian Fainelli <[email protected]> wrote on Mon, 20 May 2019
12:11:42 -0700:

> On 5/20/19 12:05 PM, Kamal Dasu wrote:
> > This change supports nand-ecc-step-size and nand-ecc-strength 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, raw NAND layer does detect ECC information by
> > reading ONFI extended parameter page for parts using ONFI >= 2.1.
> > In case of non-ONFI NAND parts there could be a nand_id table entry with
> > ECC information. If there is 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 <[email protected]>
> > ---
> > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > index ce0b8ff..a4d2057 100644
> > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > @@ -2144,6 +2144,17 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
> > return -EINVAL;
> > }
> >
> > + if (chip->ecc.mode != NAND_ECC_NONE &&
> > + (!chip->ecc.size || !chip->ecc.strength)) {
> > + if (chip->base.eccreq.step_size && chip->base.eccreq.strength) {
> > + /* use detected ECC parameters */
> > + chip->ecc.size = chip->base.eccreq.step_size;
> > + chip->ecc.strength = chip->base.eccreq.strength;
> > + pr_info("Using ECC step-size %d, strength %d\n",
> > + chip->ecc.size, chip->ecc.strength);
>
> Nit: should not we use dev_info(&host->pdev->dev) for printing the
> message in case we have multiple NAND controllers on chip, that way we
> can still differentiate them from the prints?

Yes, that would fit what the rest of the driver does. After that I
think the patchset will be ready.

Thanks,
Miquèl

2019-05-21 08:55:57

by Miquel Raynal

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

Hi Florian,

Florian Fainelli <[email protected]> wrote on Mon, 20 May 2019
12:11:42 -0700:

> On 5/20/19 12:05 PM, Kamal Dasu wrote:
> > This change supports nand-ecc-step-size and nand-ecc-strength 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, raw NAND layer does detect ECC information by
> > reading ONFI extended parameter page for parts using ONFI >= 2.1.
> > In case of non-ONFI NAND parts there could be a nand_id table entry with
> > ECC information. If there is 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 <[email protected]>
> > ---
> > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > index ce0b8ff..a4d2057 100644
> > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > @@ -2144,6 +2144,17 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
> > return -EINVAL;
> > }
> >
> > + if (chip->ecc.mode != NAND_ECC_NONE &&
> > + (!chip->ecc.size || !chip->ecc.strength)) {
> > + if (chip->base.eccreq.step_size && chip->base.eccreq.strength) {
> > + /* use detected ECC parameters */
> > + chip->ecc.size = chip->base.eccreq.step_size;
> > + chip->ecc.strength = chip->base.eccreq.strength;
> > + pr_info("Using ECC step-size %d, strength %d\n",
> > + chip->ecc.size, chip->ecc.strength);
>
> Nit: should not we use dev_info(&host->pdev->dev) for printing the
> message in case we have multiple NAND controllers on chip, that way we
> can still differentiate them from the prints?

With the above changed

Reviewed-by: Miquel Raynal <[email protected]>

Thanks,
Miquèl

2019-05-21 09:32:28

by Frieder Schrempf

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

Hi Kamal,

On 20.05.19 21:05, Kamal Dasu wrote:
> nand-ecc-strength and nand-ecc-step-size can be made optional as
> brcmnand driver can support using raw NAND layer detected values.
>
> Signed-off-by: Kamal Dasu <[email protected]>
> ---
> 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

I think you also need to change all references to nand.txt. This file
was recently moved to nand-controller.yaml.

Regards,
Frieder

2019-05-21 09:34:58

by Miquel Raynal

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


Schrempf Frieder <[email protected]> wrote on Tue, 21 May
2019 09:31:04 +0000:

> Hi Kamal,
>
> On 20.05.19 21:05, Kamal Dasu wrote:
> > nand-ecc-strength and nand-ecc-step-size can be made optional as
> > brcmnand driver can support using raw NAND layer detected values.
> >
> > Signed-off-by: Kamal Dasu <[email protected]>
> > ---
> > 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
>
> I think you also need to change all references to nand.txt. This file
> was recently moved to nand-controller.yaml.
>

Oops, completely forgot about that *again*. Thanks for pointing it
Frieder!

Miquèl