Message ID | 20171218103245.22516-1-boris.brezillon@free-electrons.com |
---|---|
State | Accepted |
Delegated to: | Boris Brezillon |
Headers | show |
Series | mtd: nand: pxa3xx: Fix READOOB implementation | expand |
On 18 December 2017 at 07:32, Boris Brezillon <boris.brezillon@free-electrons.com> wrote: > In the current driver, OOB bytes are accessed in raw mode, and when a > page access is done with NDCR_SPARE_EN set and NDCR_ECC_EN cleared, the > driver must read the whole spare area (64 bytes in case of a 2k page, > 16 bytes for a 512 page). The driver was only reading the free OOB > bytes, which was leaving some unread data in the FIFO and was somehow > leading to a timeout. > > We could patch the driver to read ->spare_size + ->ecc_size instead of > just ->spare_size when READOOB is requested, but we'd better make > in-band and OOB accesses consistent. > Since the driver is always accessing in-band data in non-raw mode (with > the ECC engine enabled), we should also access OOB data in this mode. > That's particularly useful when using the BCH engine because in this > mode the free OOB bytes are also ECC protected. > > Fixes: 43bcfd2bb24a ("mtd: nand: pxa3xx: Add driver-specific ECC BCH support") > Cc: <stable@kernel.vger.org> > Reported-by: Sean Nyekjær <sean.nyekjaer@prevas.dk> > Tested-by: Willy Tarreau <w@1wt.eu> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> > --- > drivers/mtd/nand/pxa3xx_nand.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c > index 021374fe59dc..d1979c7dbe7e 100644 > --- a/drivers/mtd/nand/pxa3xx_nand.c > +++ b/drivers/mtd/nand/pxa3xx_nand.c > @@ -961,6 +961,7 @@ static void prepare_start_command(struct pxa3xx_nand_info *info, int command) > > switch (command) { > case NAND_CMD_READ0: > + case NAND_CMD_READOOB: > case NAND_CMD_PAGEPROG: > info->use_ecc = 1; > break; > -- > 2.11.0 > Acked-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> Thanks a lot Boris!
On 2017-12-18 13:33, Ezequiel Garcia wrote: > On 18 December 2017 at 07:32, Boris Brezillon > <boris.brezillon@free-electrons.com> wrote: >> In the current driver, OOB bytes are accessed in raw mode, and when a >> page access is done with NDCR_SPARE_EN set and NDCR_ECC_EN cleared, the >> driver must read the whole spare area (64 bytes in case of a 2k page, >> 16 bytes for a 512 page). The driver was only reading the free OOB >> bytes, which was leaving some unread data in the FIFO and was somehow >> leading to a timeout. >> >> We could patch the driver to read ->spare_size + ->ecc_size instead of >> just ->spare_size when READOOB is requested, but we'd better make >> in-band and OOB accesses consistent. >> Since the driver is always accessing in-band data in non-raw mode (with >> the ECC engine enabled), we should also access OOB data in this mode. >> That's particularly useful when using the BCH engine because in this >> mode the free OOB bytes are also ECC protected. >> >> Fixes: 43bcfd2bb24a ("mtd: nand: pxa3xx: Add driver-specific ECC BCH support") >> Cc: <stable@kernel.vger.org> >> Reported-by: Sean Nyekjær <sean.nyekjaer@prevas.dk> >> Tested-by: Willy Tarreau <w@1wt.eu> >> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> >> --- >> drivers/mtd/nand/pxa3xx_nand.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c >> index 021374fe59dc..d1979c7dbe7e 100644 >> --- a/drivers/mtd/nand/pxa3xx_nand.c >> +++ b/drivers/mtd/nand/pxa3xx_nand.c >> @@ -961,6 +961,7 @@ static void prepare_start_command(struct pxa3xx_nand_info *info, int command) >> >> switch (command) { >> case NAND_CMD_READ0: >> + case NAND_CMD_READOOB: >> case NAND_CMD_PAGEPROG: >> info->use_ecc = 1; >> break; >> -- >> 2.11.0 >> > Acked-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > > Thanks a lot Boris! Tested-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk> Thanks
Boris Brezillon <boris.brezillon@free-electrons.com> (by way of Boris Brezillon <boris.brezillon@free-electrons.com>) writes: > In the current driver, OOB bytes are accessed in raw mode, and when a > page access is done with NDCR_SPARE_EN set and NDCR_ECC_EN cleared, the > driver must read the whole spare area (64 bytes in case of a 2k page, > 16 bytes for a 512 page). The driver was only reading the free OOB > bytes, which was leaving some unread data in the FIFO and was somehow > leading to a timeout. That's a very good catch. Acked-by: Robert Jarzmik <robert.jarzmik@free.fr> Cheers. -- Robert
diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c index 021374fe59dc..d1979c7dbe7e 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c +++ b/drivers/mtd/nand/pxa3xx_nand.c @@ -961,6 +961,7 @@ static void prepare_start_command(struct pxa3xx_nand_info *info, int command) switch (command) { case NAND_CMD_READ0: + case NAND_CMD_READOOB: case NAND_CMD_PAGEPROG: info->use_ecc = 1; break;