| Message ID | 20231215123208.516590-5-miquel.raynal@bootlin.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | mtd: rawnand: Fix sequential reads | expand |
On Fri, 2023-12-15 at 12:32:08 UTC, Miquel Raynal wrote: > The current logic is probably fine but is a bit convoluted. Plus, we > don't want partial pages to be part of the sequential operation just in > case the core would optimize the page read with a subpage read (which > would break the sequence). This may happen on the first and last page > only, so if the start offset or the end offset is not aligned with a > page boundary, better avoid them to prevent any risk. > > Cc: stable@vger.kernel.org > Fixes: 003fe4b9545b ("mtd: rawnand: Support for sequential cache reads") > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next. Miquel
Hi Miquel,
I am testing last nand/next branch with the MP1 board, and i get an
issue since this patch was applied.
When I read the SLC NAND using nandump tool (reading page 0 and page 1),
the OOB is not displayed at expected. For page 1, oob is displayed when
for page 0 the first data of the page are displayed.
The nanddump command used is: nanddump -c -o -l 0x2000 /dev/mtd9
Page 0:
OOB Data: 7f 45 4c 46 01 01 01 00 00 00 00 00 00 00 00 00
|.ELF............|
OOB Data: 03 00 28 00 01 00 00 00 a4 03 00 00 34 00 00 00
|..(.........4...|
OOB Data: 7c 11 00 00 00 04 00 05 34 00 20 00 06 00 28 00
||.......4. ...(.|
OOB Data: 1b 00 1a 00 01 00 00 00 00 00 00 00 00 00 00 00
|................|
OOB Data: 00 00 00 00 10 05 00 00 10 05 00 00 05 00 00 00
|................|
OOB Data: 00 00 01 00 01 00 00 00 e8 0e 00 00 e8 0e 01 00
|................|
OOB Data: e8 0e 01 00 44 01 00 00 48 01 00 00 06 00 00 00
|....D...H.......|
OOB Data: 00 00 01 00 02 00 00 00 f0 0e 00 00 f0 0e 01 00
|................|
OOB Data: f0 0e 01 00 10 01 00 00 10 01 00 00 06 00 00 00
|................|
OOB Data: 04 00 00 00 04 00 00 00 f4 00 00 00 f4 00 00 00
|................|
OOB Data: f4 00 00 00 44 00 00 00 44 00 00 00 04 00 00 00
|....D...D.......|
OOB Data: 04 00 00 00 51 e5 74 64 00 00 00 00 00 00 00 00
|....Q.td........|
OOB Data: 00 00 00 00 00 00 00 00 00 00 00 00 06 00 00 00
|................|
OOB Data: 10 00 00 00 52 e5 74 64 e8 0e 00 00 e8 0e 01 00
|....R.td........|
Page 1:
OOB Data: ff ff 94 25 8c 3c c7 44 e7 c0 b7 b0 92 5e 50 fb
|...%.<.D.....^P.|
OOB Data: 80 ca a3 de e2 73 b4 4e 58 39 fe b4 85 76 65 31
|.....s.NX9...ve1|
OOB Data: 48 86 91 f3 58 0b 59 df 2c 08 75 8b 6f 48 36 a6
|H...X.Y.,.u.oH6.|
OOB Data: bc 16 61 58 db 52 08 75 8b 6f 48 36 a6 bc 16 61
|..aX.R.u.oH6...a|
OOB Data: 58 db 52 08 75 8b 6f 48 36 a6 bc 16 61 58 db 52
|X.R.u.oH6...aX.R|
OOB Data: 08 75 8b 6f 48 36 a6 bc 16 61 58 db 52 08 75 8b
|.u.oH6...aX.R.u.|
OOB Data: 6f 48 36 a6 bc 16 61 58 db 52 ff ff ff ff ff ff
|oH6...aX.R......|
OOB Data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
|................|
OOB Data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
|................|
OOB Data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
|................|
OOB Data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
|................|
OOB Data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
|................|
OOB Data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
|................|
OOB Data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
|................|
I have checked what is happening in rawnand_enable_cont_reads function,
and for page 0, con_read.ongoing = true when for page 1 con_read.ongoing
= false
page 0:
[ 51.785623] rawnand_enable_cont_reads: page=0, col=0, readlen=4096,
mtd->writesize=4096
[ 51.793751] rawnand_enable_cont_reads: end_page=1, end_col=0
[ 51.799356] rawnand_enable_cont_reads: con_read.ongoing=1
page 1:
[ 53.493337] rawnand_enable_cont_reads: page=1, col=0, readlen=4096,
mtd->writesize=4096
[ 53.501413] rawnand_enable_cont_reads: end_page=1, end_col=0
[ 53.507013] rawnand_enable_cont_reads: con_read.ongoing=0
I do not expect con_read.ongoing set to true when we read one page.
I have also dumped what happened when we read the bad block table and it
is also strange for me in particular the value end_page.
[ 1.581940] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xd3
[ 1.581966] nand: Micron MT29F8G08ABACAH4
[ 1.581974] nand: 1024 MiB, SLC, erase size: 256 KiB, page size:
4096, OOB size: 224
[ 1.582379] rawnand_enable_cont_reads: page=262080, col=0, readlen=5,
mtd->writesize=4096
[ 1.582411] rawnand_enable_cont_reads: end_page=0, end_col=5
[ 1.582419] rawnand_enable_cont_reads: con_read.ongoing=0
[ 1.585817] Bad block table found at page 262080, version 0x01
[ 1.585943] rawnand_enable_cont_reads: page=262080, col=0, readlen=5,
mtd->writesize=4096
[ 1.585960] rawnand_enable_cont_reads: end_page=0, end_col=5
[ 1.585968] rawnand_enable_cont_reads: con_read.ongoing=0
[ 1.586677] rawnand_enable_cont_reads: page=262016, col=0, readlen=5,
mtd->writesize=4096
[ 1.586700] rawnand_enable_cont_reads: end_page=0, end_col=5
[ 1.586708] rawnand_enable_cont_reads: con_read.ongoing=0
[ 1.587139] Bad block table found at page 262016, version 0x01
[ 1.587168] rawnand_enable_cont_reads: page=262081, col=5,
readlen=1019, mtd->writesize=4096
[ 1.587181] rawnand_enable_cont_reads: end_page=0, end_col=1024
[ 1.587189] rawnand_enable_cont_reads: con_read.ongoing=0
[ 1.587672] rawnand_enable_cont_reads: page=262081, col=1024,
readlen=5, mtd->writesize=4096
[ 1.587692] rawnand_enable_cont_reads: end_page=0, end_col=1029
[ 1.587700] rawnand_enable_cont_reads: con_read.ongoing=0
I currently do not understand the logic implemented but there is
something suspect around end_page variable.
end_page = DIV_ROUND_UP(col + readlen, mtd->writesize);
=> So, if i have well understood, end_page is the number of pages we are
going to read.
if (page + 1 > end_page) {
=> We are comparing the page that we are starting to read with the
number of pages to read and not the last page to read
chip->cont_read.first_page = page;
chip->cont_read.last_page = end_page;
=> first_page is the first page to read and last page is the number of
pages to read.
Before this patch,
chip->cont_read.last_page = page + ((readlen >> chip->page_shift) &
chip->pagemask);
=> last page was the last page to read.
Regards,
Christophe Kerello.
On 12/22/23 12:37, Miquel Raynal wrote:
> On Fri, 2023-12-15 at 12:32:08 UTC, Miquel Raynal wrote:
>> The current logic is probably fine but is a bit convoluted. Plus, we
>> don't want partial pages to be part of the sequential operation just in
>> case the core would optimize the page read with a subpage read (which
>> would break the sequence). This may happen on the first and last page
>> only, so if the start offset or the end offset is not aligned with a
>> page boundary, better avoid them to prevent any risk.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 003fe4b9545b ("mtd: rawnand: Support for sequential cache reads")
>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
>
> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next.
>
> Miquel
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
Hi Christophe, christophe.kerello@foss.st.com wrote on Fri, 9 Feb 2024 14:35:44 +0100: > Hi Miquel, > > I am testing last nand/next branch with the MP1 board, and i get an issue since this patch was applied. > > When I read the SLC NAND using nandump tool (reading page 0 and page 1), the OOB is not displayed at expected. For page 1, oob is displayed when for page 0 the first data of the page are displayed. > > The nanddump command used is: nanddump -c -o -l 0x2000 /dev/mtd9 > > Page 0: > OOB Data: 7f 45 4c 46 01 01 01 00 00 00 00 00 00 00 00 00 |.ELF............| > OOB Data: 03 00 28 00 01 00 00 00 a4 03 00 00 34 00 00 00 |..(.........4...| > OOB Data: 7c 11 00 00 00 04 00 05 34 00 20 00 06 00 28 00 ||.......4. ...(.| > OOB Data: 1b 00 1a 00 01 00 00 00 00 00 00 00 00 00 00 00 |................| > OOB Data: 00 00 00 00 10 05 00 00 10 05 00 00 05 00 00 00 |................| > OOB Data: 00 00 01 00 01 00 00 00 e8 0e 00 00 e8 0e 01 00 |................| > OOB Data: e8 0e 01 00 44 01 00 00 48 01 00 00 06 00 00 00 |....D...H.......| > OOB Data: 00 00 01 00 02 00 00 00 f0 0e 00 00 f0 0e 01 00 |................| > OOB Data: f0 0e 01 00 10 01 00 00 10 01 00 00 06 00 00 00 |................| > OOB Data: 04 00 00 00 04 00 00 00 f4 00 00 00 f4 00 00 00 |................| > OOB Data: f4 00 00 00 44 00 00 00 44 00 00 00 04 00 00 00 |....D...D.......| > OOB Data: 04 00 00 00 51 e5 74 64 00 00 00 00 00 00 00 00 |....Q.td........| > OOB Data: 00 00 00 00 00 00 00 00 00 00 00 00 06 00 00 00 |................| > OOB Data: 10 00 00 00 52 e5 74 64 e8 0e 00 00 e8 0e 01 00 |....R.td........| > > Page 1: > OOB Data: ff ff 94 25 8c 3c c7 44 e7 c0 b7 b0 92 5e 50 fb |...%.<.D.....^P.| > OOB Data: 80 ca a3 de e2 73 b4 4e 58 39 fe b4 85 76 65 31 |.....s.NX9...ve1| > OOB Data: 48 86 91 f3 58 0b 59 df 2c 08 75 8b 6f 48 36 a6 |H...X.Y.,.u.oH6.| > OOB Data: bc 16 61 58 db 52 08 75 8b 6f 48 36 a6 bc 16 61 |..aX.R.u.oH6...a| > OOB Data: 58 db 52 08 75 8b 6f 48 36 a6 bc 16 61 58 db 52 |X.R.u.oH6...aX.R| > OOB Data: 08 75 8b 6f 48 36 a6 bc 16 61 58 db 52 08 75 8b |.u.oH6...aX.R.u.| > OOB Data: 6f 48 36 a6 bc 16 61 58 db 52 ff ff ff ff ff ff |oH6...aX.R......| > OOB Data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| > OOB Data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| > OOB Data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| > OOB Data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| > OOB Data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| > OOB Data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| > OOB Data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| > > I have checked what is happening in rawnand_enable_cont_reads function, > and for page 0, con_read.ongoing = true when for page 1 con_read.ongoing = false > > page 0: > [ 51.785623] rawnand_enable_cont_reads: page=0, col=0, readlen=4096, mtd->writesize=4096 > [ 51.793751] rawnand_enable_cont_reads: end_page=1, end_col=0 > [ 51.799356] rawnand_enable_cont_reads: con_read.ongoing=1 > > page 1: > [ 53.493337] rawnand_enable_cont_reads: page=1, col=0, readlen=4096, mtd->writesize=4096 > [ 53.501413] rawnand_enable_cont_reads: end_page=1, end_col=0 > [ 53.507013] rawnand_enable_cont_reads: con_read.ongoing=0 > > I do not expect con_read.ongoing set to true when we read one page. > > I have also dumped what happened when we read the bad block table and it is also strange for me in particular the value end_page. > > [ 1.581940] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xd3 > [ 1.581966] nand: Micron MT29F8G08ABACAH4 > [ 1.581974] nand: 1024 MiB, SLC, erase size: 256 KiB, page size: 4096, OOB size: 224 > [ 1.582379] rawnand_enable_cont_reads: page=262080, col=0, readlen=5, mtd->writesize=4096 > [ 1.582411] rawnand_enable_cont_reads: end_page=0, end_col=5 > [ 1.582419] rawnand_enable_cont_reads: con_read.ongoing=0 > [ 1.585817] Bad block table found at page 262080, version 0x01 > [ 1.585943] rawnand_enable_cont_reads: page=262080, col=0, readlen=5, mtd->writesize=4096 > [ 1.585960] rawnand_enable_cont_reads: end_page=0, end_col=5 > [ 1.585968] rawnand_enable_cont_reads: con_read.ongoing=0 > [ 1.586677] rawnand_enable_cont_reads: page=262016, col=0, readlen=5, mtd->writesize=4096 > [ 1.586700] rawnand_enable_cont_reads: end_page=0, end_col=5 > [ 1.586708] rawnand_enable_cont_reads: con_read.ongoing=0 > [ 1.587139] Bad block table found at page 262016, version 0x01 > [ 1.587168] rawnand_enable_cont_reads: page=262081, col=5, readlen=1019, mtd->writesize=4096 > [ 1.587181] rawnand_enable_cont_reads: end_page=0, end_col=1024 > [ 1.587189] rawnand_enable_cont_reads: con_read.ongoing=0 > [ 1.587672] rawnand_enable_cont_reads: page=262081, col=1024, readlen=5, mtd->writesize=4096 > [ 1.587692] rawnand_enable_cont_reads: end_page=0, end_col=1029 > [ 1.587700] rawnand_enable_cont_reads: con_read.ongoing=0 Interesting, I played with those corner cases in earlier tests but for this series I was focused on playing with filesystems and the fact that sometimes continuous read was very sporadically breaking, so I played with much more complex patterns but I don't remember checking these two basic cases again... Sorry for the breakage, I will have a look and keep you updated. I believe the continuous read feature is fine per se, but the problem here is that there is a mismatch between the actual operation and the continuous read configuration on "top" of it, which should in these cases not be enabled at all. I am away this week, I will look into this when I'm back. Thanks for the useful report, Miquèl
Hi Christophe, christophe.kerello@foss.st.com wrote on Fri, 9 Feb 2024 14:35:44 +0100: > Hi Miquel, > > I am testing last nand/next branch with the MP1 board, and i get an issue since this patch was applied. > > When I read the SLC NAND using nandump tool (reading page 0 and page 1), the OOB is not displayed at expected. For page 1, oob is displayed when for page 0 the first data of the page are displayed. > > The nanddump command used is: nanddump -c -o -l 0x2000 /dev/mtd9 I believe the issue is not in the indexes but related to the OOB. I currently test on a device on which I would prefer not to smash the content, so this is just compile tested and not run time verified, but could you tell me if this solves the issue: --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -3577,7 +3577,8 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from, oob = ops->oobbuf; oob_required = oob ? 1 : 0; - rawnand_enable_cont_reads(chip, page, readlen, col); + if (!oob_required) + rawnand_enable_cont_reads(chip, page, readlen, col); while (1) { struct mtd_ecc_stats ecc_stats = mtd->ecc_stats; If that does not work, I'll destroy the content of the flash and properly reproduce. Thanks, Miquèl
Hi Miquel, On 2/21/24 12:20, Miquel Raynal wrote: > Hi Christophe, > > christophe.kerello@foss.st.com wrote on Fri, 9 Feb 2024 14:35:44 +0100: > >> Hi Miquel, >> >> I am testing last nand/next branch with the MP1 board, and i get an issue since this patch was applied. >> >> When I read the SLC NAND using nandump tool (reading page 0 and page 1), the OOB is not displayed at expected. For page 1, oob is displayed when for page 0 the first data of the page are displayed. >> >> The nanddump command used is: nanddump -c -o -l 0x2000 /dev/mtd9 > > I believe the issue is not in the indexes but related to the OOB. I > currently test on a device on which I would prefer not to smash the > content, so this is just compile tested and not run time verified, but > could you tell me if this solves the issue: > > --- a/drivers/mtd/nand/raw/nand_base.c > +++ b/drivers/mtd/nand/raw/nand_base.c > @@ -3577,7 +3577,8 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from, > oob = ops->oobbuf; > oob_required = oob ? 1 : 0; > > - rawnand_enable_cont_reads(chip, page, readlen, col); > + if (!oob_required) > + rawnand_enable_cont_reads(chip, page, readlen, col); I am still able to reproduce the problem with the patch applied. In fact, when nanddump reads the OOB, nand_do_read_ops is not called, but nand_read_oob_op is called, and as cont_read.ongoing=1, we are not dumping the oob but the first data of the page. page 0: [ 57.642144] rawnand_enable_cont_reads: page=0, col=0, readlen=4096, mtd->writesize=4096 [ 57.650210] rawnand_enable_cont_reads: end_page=1 [ 57.654858] nand_do_read_ops: cont_read.ongoing=1 [ 59.352562] nand_read_oob_op page 1: [ 59.355966] rawnand_enable_cont_reads: page=1, col=0, readlen=4096, mtd->writesize=4096 [ 59.364045] rawnand_enable_cont_reads: end_page=1 [ 59.368757] nand_do_read_ops: cont_read.ongoing=0 [ 61.390098] nand_read_oob_op I have not currently bandwidth to work on this topic and I need to understand how continuous read is working, but I have made a patch and I do not have issues with it when I am using nanddump or mtd_debug tools. I have not tested it on a file system, so it is just a proposal. --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -3466,22 +3466,18 @@ 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 end_page; chip->cont_read.ongoing = false; - if (!chip->controller->supported_op.cont_read) + if (!chip->controller->supported_op.cont_read || col + readlen <= mtd->writesize) return; - end_page = DIV_ROUND_UP(col + readlen, mtd->writesize); - end_col = (col + readlen) % mtd->writesize; + end_page = page + DIV_ROUND_UP(col + readlen, mtd->writesize) - 1; if (col) page++; - if (end_col && end_page) - end_page--; - if (page + 1 > end_page) return; Tell me if this patch is breaking the continuous read feature or if it can be pushed on the mailing list. Regards, Christophe Kerello. > > while (1) { > struct mtd_ecc_stats ecc_stats = mtd->ecc_stats; > > > If that does not work, I'll destroy the content of the flash and > properly reproduce. > > Thanks, > Miquèl
Hi Christophe, christophe.kerello@foss.st.com wrote on Wed, 21 Feb 2024 17:29:45 +0100: > Hi Miquel, > > On 2/21/24 12:20, Miquel Raynal wrote: > > Hi Christophe, > > > > christophe.kerello@foss.st.com wrote on Fri, 9 Feb 2024 14:35:44 +0100: > > > >> Hi Miquel, > >> > >> I am testing last nand/next branch with the MP1 board, and i get an issue since this patch was applied. > >> > >> When I read the SLC NAND using nandump tool (reading page 0 and page 1), the OOB is not displayed at expected. For page 1, oob is displayed when for page 0 the first data of the page are displayed. > >> > >> The nanddump command used is: nanddump -c -o -l 0x2000 /dev/mtd9 > > > > I believe the issue is not in the indexes but related to the OOB. I > > currently test on a device on which I would prefer not to smash the > > content, so this is just compile tested and not run time verified, but > > could you tell me if this solves the issue: > > > > --- a/drivers/mtd/nand/raw/nand_base.c > > +++ b/drivers/mtd/nand/raw/nand_base.c > > @@ -3577,7 +3577,8 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from, > > oob = ops->oobbuf; > > oob_required = oob ? 1 : 0; > > > - rawnand_enable_cont_reads(chip, page, readlen, col); > > + if (!oob_required) > > + rawnand_enable_cont_reads(chip, page, readlen, col); > > I am still able to reproduce the problem with the patch applied. > In fact, when nanddump reads the OOB, nand_do_read_ops is not called, but nand_read_oob_op is called, and as cont_read.ongoing=1, we are not dumping the oob but the first data of the page. > > page 0: > [ 57.642144] rawnand_enable_cont_reads: page=0, col=0, readlen=4096, mtd->writesize=4096 > [ 57.650210] rawnand_enable_cont_reads: end_page=1 > [ 57.654858] nand_do_read_ops: cont_read.ongoing=1 > [ 59.352562] nand_read_oob_op > page 1: > [ 59.355966] rawnand_enable_cont_reads: page=1, col=0, readlen=4096, mtd->writesize=4096 > [ 59.364045] rawnand_enable_cont_reads: end_page=1 > [ 59.368757] nand_do_read_ops: cont_read.ongoing=0 > [ 61.390098] nand_read_oob_op > > I have not currently bandwidth to work on this topic and I need to understand how continuous read is working, but I have made a patch and I do not have issues with it when I am using nanddump or mtd_debug tools. Actually since my previous answer I managed to reproduce the issue. I was unable to do it because I was testing at the beginning of the second partition, instead of the beginning of the device. I also observe the same behavior. > I have not tested it on a file system, so it is just a proposal. > > --- a/drivers/mtd/nand/raw/nand_base.c > +++ b/drivers/mtd/nand/raw/nand_base.c > @@ -3466,22 +3466,18 @@ 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 end_page; > > chip->cont_read.ongoing = false; > > - if (!chip->controller->supported_op.cont_read) > + if (!chip->controller->supported_op.cont_read || col + readlen <= mtd->writesize) > return; > > - end_page = DIV_ROUND_UP(col + readlen, mtd->writesize); > + end_page = page + DIV_ROUND_UP(col + readlen, mtd->writesize) - 1; I had a similar change on my side so I believe this is needed. > - end_col = (col + readlen) % mtd->writesize; We shall ensure we only enable continuous reads on full pages, to avoid conflicts with the core trying to optimize things out. So I believe this change won't fly, but I get the idea, there is definitely something to fix there. > > if (col) > page++; > > - if (end_col && end_page) > - end_page--; > - > if (page + 1 > end_page) > return; > > Tell me if this patch is breaking the continuous read feature or if it can be pushed on the mailing list. I'll have deeper look into this tomorrow and get back to you. Thanks a lot for the proposal though, I will work on it. > > Regards, > Christophe Kerello. > > > > while (1) { > > struct mtd_ecc_stats ecc_stats = mtd->ecc_stats; > > > > > > If that does not work, I'll destroy the content of the flash and > > properly reproduce. > > > > Thanks, > > Miquèl Thanks, Miquèl
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index 139fdf3e58c0..bbdcfbe643f3 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -3460,21 +3460,29 @@ 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; + + chip->cont_read.ongoing = false; if (!chip->controller->supported_op.cont_read) return; - if ((col && col + readlen < (3 * mtd->writesize)) || - (!col && readlen < (2 * mtd->writesize))) { - chip->cont_read.ongoing = false; - return; - } + end_page = DIV_ROUND_UP(col + readlen, mtd->writesize); + end_col = (col + readlen) % mtd->writesize; - chip->cont_read.ongoing = true; - chip->cont_read.first_page = page; if (col) - chip->cont_read.first_page++; - chip->cont_read.last_page = page + ((readlen >> chip->page_shift) & chip->pagemask); + 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); }
The current logic is probably fine but is a bit convoluted. Plus, we don't want partial pages to be part of the sequential operation just in case the core would optimize the page read with a subpage read (which would break the sequence). This may happen on the first and last page only, so if the start offset or the end offset is not aligned with a page boundary, better avoid them to prevent any risk. Cc: stable@vger.kernel.org Fixes: 003fe4b9545b ("mtd: rawnand: Support for sequential cache reads") Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/mtd/nand/raw/nand_base.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-)