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 <[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
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 <[email protected]>
---
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
Hi Kamal,
Kamal Dasu <[email protected]> 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 <[email protected]>
> ---
> 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
Hi Kamal,
Kamal Dasu <[email protected]> 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 <[email protected]>
> ---
With this addressed:
Reviewed-by: Miquel Raynal <[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
Thanks,
Miquèl
Will make the changes and send a V2 patch.
On Mon, May 20, 2019 at 8:44 AM Miquel Raynal <[email protected]> wrote:
>
> Hi Kamal,
>
> Kamal Dasu <[email protected]> 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 <[email protected]>
> > ---
> > 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
Hi Kamal,
Kamal Dasu <[email protected]> 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 <[email protected]> wrote:
> >
> > Hi Kamal,
> >
> > Kamal Dasu <[email protected]> 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 <[email protected]>
> > > ---
> > > 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
On Mon, May 20, 2019 at 1:34 PM Miquel Raynal <[email protected]> wrote:
>
> Hi Kamal,
>
> Kamal Dasu <[email protected]> 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 <[email protected]> wrote:
> > >
> > > Hi Kamal,
> > >
> > > Kamal Dasu <[email protected]> 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 <[email protected]>
> > > > ---
> > > > 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
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 <[email protected]>
> ---
> Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
Reviewed-by: Rob Herring <[email protected]>