| Message ID | 1455300685-27009-2-git-send-email-zajec5@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, Feb 12, 2016 at 1:11 PM, Rafał Miłecki <zajec5@gmail.com> wrote: > > This will allow drivers handle ECC properly. > > Signed-off-by: Rafał Miłecki <zajec5@gmail.com> Reviewed-by: Kamal Dasu <kdasu.kdev@gmail.com> Thanks Rafal > > --- > drivers/mtd/nand/nand_base.c | 6 +++++- > include/linux/mtd/nand.h | 1 + > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index f2c8ff3..ef977f3 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -3979,7 +3979,7 @@ ident_done: > static int nand_dt_init(struct nand_chip *chip) > { > struct device_node *dn = nand_get_flash_node(chip); > - int ecc_mode, ecc_strength, ecc_step; > + int ecc_mode, ecc_algo, ecc_strength, ecc_step; > > if (!dn) > return 0; > @@ -3991,6 +3991,7 @@ static int nand_dt_init(struct nand_chip *chip) > chip->bbt_options |= NAND_BBT_USE_FLASH; > > ecc_mode = of_get_nand_ecc_mode(dn); > + ecc_algo = of_get_nand_ecc_algo(dn); > ecc_strength = of_get_nand_ecc_strength(dn); > ecc_step = of_get_nand_ecc_step_size(dn); > > @@ -4003,6 +4004,9 @@ static int nand_dt_init(struct nand_chip *chip) > if (ecc_mode >= 0) > chip->ecc.mode = ecc_mode; > > + if (ecc_algo >= 0) > + chip->ecc.algo = ecc_algo; > + > if (ecc_strength >= 0) > chip->ecc.strength = ecc_strength; > > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > index 25854d2..8deca1b 100644 > --- a/include/linux/mtd/nand.h > +++ b/include/linux/mtd/nand.h > @@ -513,6 +513,7 @@ struct nand_hw_control { > */ > struct nand_ecc_ctrl { > nand_ecc_modes_t mode; > + enum nand_ecc_algo algo; > int steps; > int size; > int bytes; > -- > 1.8.4.5 >
On Fri, Feb 12, 2016 at 07:11:24PM +0100, Rafał Miłecki wrote: > This will allow drivers handle ECC properly. > > Signed-off-by: Rafał Miłecki <zajec5@gmail.com> Acked-by: Brian Norris <computersforpeace@gmai.com>
On Fri, Feb 12, 2016 at 07:11:24PM +0100, Rafał Miłecki wrote: > This will allow drivers handle ECC properly. > > Signed-off-by: Rafał Miłecki <zajec5@gmail.com> > --- > drivers/mtd/nand/nand_base.c | 6 +++++- > include/linux/mtd/nand.h | 1 + > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index f2c8ff3..ef977f3 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -3979,7 +3979,7 @@ ident_done: > static int nand_dt_init(struct nand_chip *chip) > { > struct device_node *dn = nand_get_flash_node(chip); > - int ecc_mode, ecc_strength, ecc_step; > + int ecc_mode, ecc_algo, ecc_strength, ecc_step; > > if (!dn) > return 0; > @@ -3991,6 +3991,7 @@ static int nand_dt_init(struct nand_chip *chip) > chip->bbt_options |= NAND_BBT_USE_FLASH; > > ecc_mode = of_get_nand_ecc_mode(dn); > + ecc_algo = of_get_nand_ecc_algo(dn); > ecc_strength = of_get_nand_ecc_strength(dn); > ecc_step = of_get_nand_ecc_step_size(dn); > > @@ -4003,6 +4004,9 @@ static int nand_dt_init(struct nand_chip *chip) > if (ecc_mode >= 0) > chip->ecc.mode = ecc_mode; > > + if (ecc_algo >= 0) > + chip->ecc.algo = ecc_algo; > + While this might appear to handle the absence of the nand-ecc-algo property correctly, this isn't safe. This means that any existing DT without the property will get treated as Hamming ECC. Perhaps we need the enum to have a 0 value of 'NAND_ECC_ALGO_UNKNOWN' or something like that? Brian > if (ecc_strength >= 0) > chip->ecc.strength = ecc_strength; > > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > index 25854d2..8deca1b 100644 > --- a/include/linux/mtd/nand.h > +++ b/include/linux/mtd/nand.h > @@ -513,6 +513,7 @@ struct nand_hw_control { > */ > struct nand_ecc_ctrl { > nand_ecc_modes_t mode; > + enum nand_ecc_algo algo; > int steps; > int size; > int bytes; > -- > 1.8.4.5 >
On 1 April 2016 at 18:07, Brian Norris <computersforpeace@gmail.com> wrote: > On Fri, Feb 12, 2016 at 07:11:24PM +0100, Rafał Miłecki wrote: >> This will allow drivers handle ECC properly. >> >> Signed-off-by: Rafał Miłecki <zajec5@gmail.com> >> --- >> drivers/mtd/nand/nand_base.c | 6 +++++- >> include/linux/mtd/nand.h | 1 + >> 2 files changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c >> index f2c8ff3..ef977f3 100644 >> --- a/drivers/mtd/nand/nand_base.c >> +++ b/drivers/mtd/nand/nand_base.c >> @@ -3979,7 +3979,7 @@ ident_done: >> static int nand_dt_init(struct nand_chip *chip) >> { >> struct device_node *dn = nand_get_flash_node(chip); >> - int ecc_mode, ecc_strength, ecc_step; >> + int ecc_mode, ecc_algo, ecc_strength, ecc_step; >> >> if (!dn) >> return 0; >> @@ -3991,6 +3991,7 @@ static int nand_dt_init(struct nand_chip *chip) >> chip->bbt_options |= NAND_BBT_USE_FLASH; >> >> ecc_mode = of_get_nand_ecc_mode(dn); >> + ecc_algo = of_get_nand_ecc_algo(dn); >> ecc_strength = of_get_nand_ecc_strength(dn); >> ecc_step = of_get_nand_ecc_step_size(dn); >> >> @@ -4003,6 +4004,9 @@ static int nand_dt_init(struct nand_chip *chip) >> if (ecc_mode >= 0) >> chip->ecc.mode = ecc_mode; >> >> + if (ecc_algo >= 0) >> + chip->ecc.algo = ecc_algo; >> + > > While this might appear to handle the absence of the nand-ecc-algo > property correctly, this isn't safe. This means that any existing DT > without the property will get treated as Hamming ECC. Perhaps we need > the enum to have a 0 value of 'NAND_ECC_ALGO_UNKNOWN' or something like > that? You're commenting on an old series. If you take a look at: https://patchwork.ozlabs.org/patch/601180/ that also got applied to the: https://github.com/linux-nand/linux/commits/nand/next repo, you'll see we have NAND_ECC_UNKNOWN there.
On Fri, Apr 01, 2016 at 09:32:40PM +0200, Rafał Miłecki wrote: > On 1 April 2016 at 18:07, Brian Norris <computersforpeace@gmail.com> wrote: > > On Fri, Feb 12, 2016 at 07:11:24PM +0100, Rafał Miłecki wrote: > >> This will allow drivers handle ECC properly. > >> > >> Signed-off-by: Rafał Miłecki <zajec5@gmail.com> > >> --- > >> drivers/mtd/nand/nand_base.c | 6 +++++- > >> include/linux/mtd/nand.h | 1 + > >> 2 files changed, 6 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > >> index f2c8ff3..ef977f3 100644 > >> --- a/drivers/mtd/nand/nand_base.c > >> +++ b/drivers/mtd/nand/nand_base.c > >> @@ -3979,7 +3979,7 @@ ident_done: > >> static int nand_dt_init(struct nand_chip *chip) > >> { > >> struct device_node *dn = nand_get_flash_node(chip); > >> - int ecc_mode, ecc_strength, ecc_step; > >> + int ecc_mode, ecc_algo, ecc_strength, ecc_step; > >> > >> if (!dn) > >> return 0; > >> @@ -3991,6 +3991,7 @@ static int nand_dt_init(struct nand_chip *chip) > >> chip->bbt_options |= NAND_BBT_USE_FLASH; > >> > >> ecc_mode = of_get_nand_ecc_mode(dn); > >> + ecc_algo = of_get_nand_ecc_algo(dn); > >> ecc_strength = of_get_nand_ecc_strength(dn); > >> ecc_step = of_get_nand_ecc_step_size(dn); > >> > >> @@ -4003,6 +4004,9 @@ static int nand_dt_init(struct nand_chip *chip) > >> if (ecc_mode >= 0) > >> chip->ecc.mode = ecc_mode; > >> > >> + if (ecc_algo >= 0) > >> + chip->ecc.algo = ecc_algo; > >> + > > > > While this might appear to handle the absence of the nand-ecc-algo > > property correctly, this isn't safe. This means that any existing DT > > without the property will get treated as Hamming ECC. Perhaps we need > > the enum to have a 0 value of 'NAND_ECC_ALGO_UNKNOWN' or something like > > that? > > You're commenting on an old series. If you take a look at: > https://patchwork.ozlabs.org/patch/601180/ > that also got applied to the: > https://github.com/linux-nand/linux/commits/nand/next > repo, you'll see we have NAND_ECC_UNKNOWN there. Ah, I guess I missed that because I was searching for the patch to brcmnand (i.e., fixing the original problem you saw). AFAICT, this v2 series doesn't actually resolve your issue with brcmnand, no? Brian
On 1 April 2016 at 22:07, Brian Norris <computersforpeace@gmail.com> wrote: > On Fri, Apr 01, 2016 at 09:32:40PM +0200, Rafał Miłecki wrote: >> On 1 April 2016 at 18:07, Brian Norris <computersforpeace@gmail.com> wrote: >> > On Fri, Feb 12, 2016 at 07:11:24PM +0100, Rafał Miłecki wrote: >> >> This will allow drivers handle ECC properly. >> >> >> >> Signed-off-by: Rafał Miłecki <zajec5@gmail.com> >> >> --- >> >> drivers/mtd/nand/nand_base.c | 6 +++++- >> >> include/linux/mtd/nand.h | 1 + >> >> 2 files changed, 6 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c >> >> index f2c8ff3..ef977f3 100644 >> >> --- a/drivers/mtd/nand/nand_base.c >> >> +++ b/drivers/mtd/nand/nand_base.c >> >> @@ -3979,7 +3979,7 @@ ident_done: >> >> static int nand_dt_init(struct nand_chip *chip) >> >> { >> >> struct device_node *dn = nand_get_flash_node(chip); >> >> - int ecc_mode, ecc_strength, ecc_step; >> >> + int ecc_mode, ecc_algo, ecc_strength, ecc_step; >> >> >> >> if (!dn) >> >> return 0; >> >> @@ -3991,6 +3991,7 @@ static int nand_dt_init(struct nand_chip *chip) >> >> chip->bbt_options |= NAND_BBT_USE_FLASH; >> >> >> >> ecc_mode = of_get_nand_ecc_mode(dn); >> >> + ecc_algo = of_get_nand_ecc_algo(dn); >> >> ecc_strength = of_get_nand_ecc_strength(dn); >> >> ecc_step = of_get_nand_ecc_step_size(dn); >> >> >> >> @@ -4003,6 +4004,9 @@ static int nand_dt_init(struct nand_chip *chip) >> >> if (ecc_mode >= 0) >> >> chip->ecc.mode = ecc_mode; >> >> >> >> + if (ecc_algo >= 0) >> >> + chip->ecc.algo = ecc_algo; >> >> + >> > >> > While this might appear to handle the absence of the nand-ecc-algo >> > property correctly, this isn't safe. This means that any existing DT >> > without the property will get treated as Hamming ECC. Perhaps we need >> > the enum to have a 0 value of 'NAND_ECC_ALGO_UNKNOWN' or something like >> > that? >> >> You're commenting on an old series. If you take a look at: >> https://patchwork.ozlabs.org/patch/601180/ >> that also got applied to the: >> https://github.com/linux-nand/linux/commits/nand/next >> repo, you'll see we have NAND_ECC_UNKNOWN there. > > Ah, I guess I missed that because I was searching for the patch to > brcmnand (i.e., fixing the original problem you saw). AFAICT, this v2 > series doesn't actually resolve your issue with brcmnand, no? That's correct. I'm planning to clean NAND subsystem first, then patch brcmnand.
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index f2c8ff3..ef977f3 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -3979,7 +3979,7 @@ ident_done: static int nand_dt_init(struct nand_chip *chip) { struct device_node *dn = nand_get_flash_node(chip); - int ecc_mode, ecc_strength, ecc_step; + int ecc_mode, ecc_algo, ecc_strength, ecc_step; if (!dn) return 0; @@ -3991,6 +3991,7 @@ static int nand_dt_init(struct nand_chip *chip) chip->bbt_options |= NAND_BBT_USE_FLASH; ecc_mode = of_get_nand_ecc_mode(dn); + ecc_algo = of_get_nand_ecc_algo(dn); ecc_strength = of_get_nand_ecc_strength(dn); ecc_step = of_get_nand_ecc_step_size(dn); @@ -4003,6 +4004,9 @@ static int nand_dt_init(struct nand_chip *chip) if (ecc_mode >= 0) chip->ecc.mode = ecc_mode; + if (ecc_algo >= 0) + chip->ecc.algo = ecc_algo; + if (ecc_strength >= 0) chip->ecc.strength = ecc_strength; diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index 25854d2..8deca1b 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -513,6 +513,7 @@ struct nand_hw_control { */ struct nand_ecc_ctrl { nand_ecc_modes_t mode; + enum nand_ecc_algo algo; int steps; int size; int bytes;
This will allow drivers handle ECC properly. Signed-off-by: Rafał Miłecki <zajec5@gmail.com> --- drivers/mtd/nand/nand_base.c | 6 +++++- include/linux/mtd/nand.h | 1 + 2 files changed, 6 insertions(+), 1 deletion(-)