| Message ID | 1394904668-22978-2-git-send-email-ron@debian.org |
|---|---|
| State | Accepted |
| Commit | 4a4163caccae97a23d97c29032664ee7b7a498d0 |
| Headers | show |
+ Florian (FYI Ron: 3 of your emails were filtered as spam in my mailbox, for some reason. I only pass over my spam mailbox every once in a few weeks; I only noticed this because of your IRC ping!) On Sun, Mar 16, 2014 at 04:01:07AM +1030, Ron wrote: > Commit 7351d3a5dbf42ba3299af71db3296be447bc1516 added an index variable > as part of fixing checkpatch warnings, presumably as a tool to make some > long lines shorter, however it only set that index in the case of there > being no gaps in eccpos for the fragment being read. Which means the > later step of filling ecccode from oob_poi will use the wrong indexing > into eccpos in that case. > > This patch restores the behaviour that existed prior to that change. > > Signed-off-by: Ron <ron@debian.org> > --- > drivers/mtd/nand/nand_base.c | 5 ++--- > 1 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index 9715a7b..be23dff 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -1170,13 +1170,14 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip, > int data_col_addr, i, gaps = 0; > int datafrag_len, eccfrag_len, aligned_len, aligned_pos; > int busw = (chip->options & NAND_BUSWIDTH_16) ? 2 : 1; > - int index = 0; > + int index; > unsigned int max_bitflips = 0; > > /* Column address within the page aligned to ECC size (256bytes) */ > start_step = data_offs / chip->ecc.size; > end_step = (data_offs + readlen - 1) / chip->ecc.size; > num_steps = end_step - start_step + 1; > + index = start_step * chip->ecc.bytes; > > /* Data size aligned to ECC ecc.size */ > datafrag_len = num_steps * chip->ecc.size; > @@ -1213,8 +1214,6 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip, > * Send the command to read the particular ECC bytes take care > * about buswidth alignment in read_buf. > */ > - index = start_step * chip->ecc.bytes; > - > aligned_pos = eccpos[index] & ~(busw - 1); > aligned_len = eccfrag_len; > if (eccpos[index] & (busw - 1)) > -- > 1.7.2.5 >
Hi, 2014-03-31 9:56 GMT-07:00 Brian Norris <computersforpeace@gmail.com>: > + Florian > > (FYI Ron: 3 of your emails were filtered as spam in my mailbox, for some > reason. I only pass over my spam mailbox every once in a few weeks; I > only noticed this because of your IRC ping!) BTW, you will need to sign-off on these patches using your full name. Your patch does look good to me. Thanks! > > On Sun, Mar 16, 2014 at 04:01:07AM +1030, Ron wrote: >> Commit 7351d3a5dbf42ba3299af71db3296be447bc1516 added an index variable >> as part of fixing checkpatch warnings, presumably as a tool to make some >> long lines shorter, however it only set that index in the case of there >> being no gaps in eccpos for the fragment being read. Which means the >> later step of filling ecccode from oob_poi will use the wrong indexing >> into eccpos in that case. >> >> This patch restores the behaviour that existed prior to that change. >> >> Signed-off-by: Ron <ron@debian.org> >> --- >> drivers/mtd/nand/nand_base.c | 5 ++--- >> 1 files changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c >> index 9715a7b..be23dff 100644 >> --- a/drivers/mtd/nand/nand_base.c >> +++ b/drivers/mtd/nand/nand_base.c >> @@ -1170,13 +1170,14 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip, >> int data_col_addr, i, gaps = 0; >> int datafrag_len, eccfrag_len, aligned_len, aligned_pos; >> int busw = (chip->options & NAND_BUSWIDTH_16) ? 2 : 1; >> - int index = 0; >> + int index; >> unsigned int max_bitflips = 0; >> >> /* Column address within the page aligned to ECC size (256bytes) */ >> start_step = data_offs / chip->ecc.size; >> end_step = (data_offs + readlen - 1) / chip->ecc.size; >> num_steps = end_step - start_step + 1; >> + index = start_step * chip->ecc.bytes; >> >> /* Data size aligned to ECC ecc.size */ >> datafrag_len = num_steps * chip->ecc.size; >> @@ -1213,8 +1214,6 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip, >> * Send the command to read the particular ECC bytes take care >> * about buswidth alignment in read_buf. >> */ >> - index = start_step * chip->ecc.bytes; >> - >> aligned_pos = eccpos[index] & ~(busw - 1); >> aligned_len = eccfrag_len; >> if (eccpos[index] & (busw - 1)) >> -- >> 1.7.2.5 >>
On Tue, Apr 01, 2014 at 03:14:24PM -0700, Florian Fainelli wrote: > Hi, > > 2014-03-31 9:56 GMT-07:00 Brian Norris <computersforpeace@gmail.com>: > > + Florian > > > > (FYI Ron: 3 of your emails were filtered as spam in my mailbox, for some > > reason. I only pass over my spam mailbox every once in a few weeks; I > > only noticed this because of your IRC ping!) > > BTW, you will need to sign-off on these patches using your full name. > Your patch does look good to me. Thanks! Which would be: Signed-off-by: Ron Lee <ron@debian.org> Cheers, Ron > > On Sun, Mar 16, 2014 at 04:01:07AM +1030, Ron wrote: > >> Commit 7351d3a5dbf42ba3299af71db3296be447bc1516 added an index variable > >> as part of fixing checkpatch warnings, presumably as a tool to make some > >> long lines shorter, however it only set that index in the case of there > >> being no gaps in eccpos for the fragment being read. Which means the > >> later step of filling ecccode from oob_poi will use the wrong indexing > >> into eccpos in that case. > >> > >> This patch restores the behaviour that existed prior to that change. > >> > >> Signed-off-by: Ron <ron@debian.org> > >> --- > >> drivers/mtd/nand/nand_base.c | 5 ++--- > >> 1 files changed, 2 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > >> index 9715a7b..be23dff 100644 > >> --- a/drivers/mtd/nand/nand_base.c > >> +++ b/drivers/mtd/nand/nand_base.c > >> @@ -1170,13 +1170,14 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip, > >> int data_col_addr, i, gaps = 0; > >> int datafrag_len, eccfrag_len, aligned_len, aligned_pos; > >> int busw = (chip->options & NAND_BUSWIDTH_16) ? 2 : 1; > >> - int index = 0; > >> + int index; > >> unsigned int max_bitflips = 0; > >> > >> /* Column address within the page aligned to ECC size (256bytes) */ > >> start_step = data_offs / chip->ecc.size; > >> end_step = (data_offs + readlen - 1) / chip->ecc.size; > >> num_steps = end_step - start_step + 1; > >> + index = start_step * chip->ecc.bytes; > >> > >> /* Data size aligned to ECC ecc.size */ > >> datafrag_len = num_steps * chip->ecc.size; > >> @@ -1213,8 +1214,6 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip, > >> * Send the command to read the particular ECC bytes take care > >> * about buswidth alignment in read_buf. > >> */ > >> - index = start_step * chip->ecc.bytes; > >> - > >> aligned_pos = eccpos[index] & ~(busw - 1); > >> aligned_len = eccfrag_len; > >> if (eccpos[index] & (busw - 1)) > >> -- > >> 1.7.2.5 > >> > > > > -- > Florian
On Wed, Apr 02, 2014 at 04:28:07PM +1030, Ron wrote:
> Signed-off-by: Ron Lee <ron@debian.org>
Thanks. Pushed the first one to l2-mtd.git (should go out for 3.15, I
think), but I'll leave the other trivial one for later.
Thanks,
Brian
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index 9715a7b..be23dff 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -1170,13 +1170,14 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip, int data_col_addr, i, gaps = 0; int datafrag_len, eccfrag_len, aligned_len, aligned_pos; int busw = (chip->options & NAND_BUSWIDTH_16) ? 2 : 1; - int index = 0; + int index; unsigned int max_bitflips = 0; /* Column address within the page aligned to ECC size (256bytes) */ start_step = data_offs / chip->ecc.size; end_step = (data_offs + readlen - 1) / chip->ecc.size; num_steps = end_step - start_step + 1; + index = start_step * chip->ecc.bytes; /* Data size aligned to ECC ecc.size */ datafrag_len = num_steps * chip->ecc.size; @@ -1213,8 +1214,6 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip, * Send the command to read the particular ECC bytes take care * about buswidth alignment in read_buf. */ - index = start_step * chip->ecc.bytes; - aligned_pos = eccpos[index] & ~(busw - 1); aligned_len = eccfrag_len; if (eccpos[index] & (busw - 1))
Commit 7351d3a5dbf42ba3299af71db3296be447bc1516 added an index variable as part of fixing checkpatch warnings, presumably as a tool to make some long lines shorter, however it only set that index in the case of there being no gaps in eccpos for the fragment being read. Which means the later step of filling ecccode from oob_poi will use the wrong indexing into eccpos in that case. This patch restores the behaviour that existed prior to that change. Signed-off-by: Ron <ron@debian.org> --- drivers/mtd/nand/nand_base.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-)