| Message ID | 20240223115545.354541-2-miquel.raynal@bootlin.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | mtd: rawnand: More continuous read fixes | expand |
Hi Miquel, On 2/23/24 12:55, Miquel Raynal wrote: > We need to avoid the first page if we don't read it entirely. > We need to avoid the last page if we don't read it entirely. > While rather simple, this logic has been failed in the previous > fix. This time I wrote about 30 unit tests locally to check each > possible condition, hopefully I covered them all. > > Reported-by: Christophe Kerello <christophe.kerello@foss.st.com> > Closes: https://lore.kernel.org/linux-mtd/20240221175327.42f7076d@xps-13/T/#m399bacb10db8f58f6b1f0149a1df867ec086bb0a > Suggested-by: Christophe Kerello <christophe.kerello@foss.st.com> > Fixes: 828f6df1bcba ("mtd: rawnand: Clarify conditions to enable continuous reads") > Cc: stable@vger.kernel.org > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> Tested-by: Christophe Kerello <christophe.kerello@foss.st.com> Regards, Christophe Kerello. > --- > drivers/mtd/nand/raw/nand_base.c | 38 ++++++++++++++++++-------------- > 1 file changed, 22 insertions(+), 16 deletions(-) > > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c > index 3b3ce2926f5d..bcfd99a1699f 100644 > --- a/drivers/mtd/nand/raw/nand_base.c > +++ b/drivers/mtd/nand/raw/nand_base.c > @@ -3466,30 +3466,36 @@ static void rawnand_enable_cont_reads(struct nand_chip *chip, unsigned int page, > u32 readlen, int col) > { > struct mtd_info *mtd = nand_to_mtd(chip); > - unsigned int end_page, end_col; > + unsigned int first_page, last_page; > > chip->cont_read.ongoing = false; > > if (!chip->controller->supported_op.cont_read) > return; > > - end_page = DIV_ROUND_UP(col + readlen, mtd->writesize); > - end_col = (col + readlen) % mtd->writesize; > + /* > + * Don't bother making any calculations if the length is too small. > + * Side effect: avoids possible integer underflows below. > + */ > + if (readlen < (2 * mtd->writesize)) > + return; > > + /* Derive the page where continuous read should start (the first full page read) */ > + first_page = page; > if (col) > - page++; > - > - if (end_col && end_page) > - end_page--; > - > - if (page + 1 > end_page) > - return; > - > - chip->cont_read.first_page = page; > - chip->cont_read.last_page = end_page; > - chip->cont_read.ongoing = true; > - > - rawnand_cap_cont_reads(chip); > + first_page++; > + > + /* Derive the page where continuous read should stop (the last full page read) */ > + last_page = page + ((col + readlen) / mtd->writesize) - 1; > + > + /* Configure and enable continuous read when suitable */ > + if (first_page < last_page) { > + chip->cont_read.first_page = first_page; > + chip->cont_read.last_page = last_page; > + chip->cont_read.ongoing = true; > + /* May reset the ongoing flag */ > + rawnand_cap_cont_reads(chip); > + } > } > > static void rawnand_cont_read_skip_first_page(struct nand_chip *chip, unsigned int page)
On Fri, 2024-02-23 at 11:55:43 UTC, Miquel Raynal wrote: > We need to avoid the first page if we don't read it entirely. > We need to avoid the last page if we don't read it entirely. > While rather simple, this logic has been failed in the previous > fix. This time I wrote about 30 unit tests locally to check each > possible condition, hopefully I covered them all. > > Reported-by: Christophe Kerello <christophe.kerello@foss.st.com> > Closes: https://lore.kernel.org/linux-mtd/20240221175327.42f7076d@xps-13/T/#m399bacb10db8f58f6b1f0149a1df867ec086bb0a > Suggested-by: Christophe Kerello <christophe.kerello@foss.st.com> > Fixes: 828f6df1bcba ("mtd: rawnand: Clarify conditions to enable continuous reads") > Cc: stable@vger.kernel.org > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > Tested-by: Christophe Kerello <christophe.kerello@foss.st.com> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next. Miquel
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index 3b3ce2926f5d..bcfd99a1699f 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -3466,30 +3466,36 @@ static void rawnand_enable_cont_reads(struct nand_chip *chip, unsigned int page, u32 readlen, int col) { struct mtd_info *mtd = nand_to_mtd(chip); - unsigned int end_page, end_col; + unsigned int first_page, last_page; chip->cont_read.ongoing = false; if (!chip->controller->supported_op.cont_read) return; - end_page = DIV_ROUND_UP(col + readlen, mtd->writesize); - end_col = (col + readlen) % mtd->writesize; + /* + * Don't bother making any calculations if the length is too small. + * Side effect: avoids possible integer underflows below. + */ + if (readlen < (2 * mtd->writesize)) + return; + /* Derive the page where continuous read should start (the first full page read) */ + first_page = page; if (col) - page++; - - if (end_col && end_page) - end_page--; - - if (page + 1 > end_page) - return; - - chip->cont_read.first_page = page; - chip->cont_read.last_page = end_page; - chip->cont_read.ongoing = true; - - rawnand_cap_cont_reads(chip); + first_page++; + + /* Derive the page where continuous read should stop (the last full page read) */ + last_page = page + ((col + readlen) / mtd->writesize) - 1; + + /* Configure and enable continuous read when suitable */ + if (first_page < last_page) { + chip->cont_read.first_page = first_page; + chip->cont_read.last_page = last_page; + chip->cont_read.ongoing = true; + /* May reset the ongoing flag */ + rawnand_cap_cont_reads(chip); + } } static void rawnand_cont_read_skip_first_page(struct nand_chip *chip, unsigned int page)
We need to avoid the first page if we don't read it entirely. We need to avoid the last page if we don't read it entirely. While rather simple, this logic has been failed in the previous fix. This time I wrote about 30 unit tests locally to check each possible condition, hopefully I covered them all. Reported-by: Christophe Kerello <christophe.kerello@foss.st.com> Closes: https://lore.kernel.org/linux-mtd/20240221175327.42f7076d@xps-13/T/#m399bacb10db8f58f6b1f0149a1df867ec086bb0a Suggested-by: Christophe Kerello <christophe.kerello@foss.st.com> Fixes: 828f6df1bcba ("mtd: rawnand: Clarify conditions to enable continuous reads") Cc: stable@vger.kernel.org Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/mtd/nand/raw/nand_base.c | 38 ++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 16 deletions(-)