[v2] mtd: rawnand: Ensure the nand chip supports cached reads

  Both the JEDEC and ONFI specification say that read cache sequential
support is an optional command. This means that we not only need to
check whether the individual controller supports the command, we also
need to check the parameter pages for both ONFI and JEDEC NAND flashes
before enabling sequential cache reads.

This fixes support for NAND flashes which don't support enabling cache
reads, i.e. Samsung K9F4G08U0F or Toshiba TC58NVG0S3HTA00.

Sequential cache reads are now only available for ONFI and JEDEC
devices, if individual vendors implement this, it needs to be enabled
per vendor.

Tested on i.MX6Q with a Samsung NAND flash chip that doesn't support
sequential reads.

Fixes: 003fe4b9545b ("mtd: rawnand: Support for sequential cache reads")
Cc: stable@vger.kernel.org
Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
---
v2:
- change title as suggested by Miquel
- adjust controller sentence from implement to support
- fix missing true assignement for flashes
- add CC stable instead of empty line
- add documentation comment for new supports_read_cache bool inside
  nand parameter struct

 drivers/mtd/nand/raw/nand_base.c  | 3 +++
 drivers/mtd/nand/raw/nand_jedec.c | 3 +++
 drivers/mtd/nand/raw/nand_onfi.c  | 3 +++
 include/linux/mtd/jedec.h         | 3 +++
 include/linux/mtd/onfi.h          | 1 +
 include/linux/mtd/rawnand.h       | 2 ++
 6 files changed, 15 insertions(+)


base-commit: 42dc814987c1feb6410904e58cfd4c36c4146150
  

Hi Rouven,

On Fri, 2023-09-22 at 16:17 +0200, Rouven Czerwinski wrote:
> Both the JEDEC and ONFI specification say that read cache sequential
> support is an optional command. This means that we not only need to
> check whether the individual controller supports the command, we also
> need to check the parameter pages for both ONFI and JEDEC NAND
> flashes
> before enabling sequential cache reads.
> 
> This fixes support for NAND flashes which don't support enabling
> cache
> reads, i.e. Samsung K9F4G08U0F or Toshiba TC58NVG0S3HTA00.
> 
> Sequential cache reads are now only available for ONFI and JEDEC
> devices, if individual vendors implement this, it needs to be enabled
> per vendor.
> 
> Tested on i.MX6Q with a Samsung NAND flash chip that doesn't support
> sequential reads.
> 
> Fixes: 003fe4b9545b ("mtd: rawnand: Support for sequential cache
> reads")
> Cc: stable@vger.kernel.org
> Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>

Thanks for this. It works as expected for my Toshiba chip, obviously
because it doesn't use ONFI or JEDEC.

Unfortunately, my Micron chip does use ONFI, and it sets the cached-
read-supported bit. It then fails when reading afterwords:

kernel: ONFI_OPT_CMD_READ_CACHE # debug added by me
kernel: nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xdc
kernel: nand: Micron MT29F4G08ABAFAWP
kernel: nand: 512 MiB, SLC, erase size: 256 KiB, page size: 4096, OOB
size: 256
kernel: nand: continued read supported # debug added by me
kernel: Bad block table found at page 131008, version 0x01
kernel: Bad block table found at page 130944, version 0x01
kernel: 2 fixed-partitions partitions found on MTD device gpmi-nand
kernel: Creating 2 MTD partitions on "gpmi-nand":
kernel: 0x000000000000-0x000000800000 : "boot"
kernel: 0x000000800000-0x000020000000 : "ubi"
kernel: gpmi-nand 1806000.nand-controller: driver registered.

...

kernel: ubi0: default fastmap pool size: 100
kernel: ubi0: default fastmap WL pool size: 50
kernel: ubi0: attaching mtd1
kernel: ubi0: scanning is finished
kernel: ubi0: attached mtd1 (name "ubi", size 504 MiB)
kernel: ubi0: PEB size: 262144 bytes (256 KiB), LEB size: 253952 bytes
kernel: ubi0: min./max. I/O unit sizes: 4096/4096, sub-page size 4096
kernel: ubi0: VID header offset: 4096 (aligned 4096), data offset: 8192
kernel: ubi0: good PEBs: 2012, bad PEBs: 4, corrupted PEBs: 0
kernel: ubi0: user volume: 9, internal volumes: 1, max. volumes count:
128
kernel: ubi0: max/mean erase counter: 4/2, WL threshold: 4096, image
sequence number: 1431497221
kernel: ubi0: available PEBs: 12, total reserved PEBs: 2000, PEBs
reserved for bad PEB handling: 36
kernel: block ubiblock0_4: created from ubi0:4(rootfs.a)
kernel: ubi0: background thread "ubi_bgt0d" started, PID 36
kernel: block ubiblock0_6: created from ubi0:6(appfs.a)
kernel: block ubiblock0_7: created from ubi0:7(appfs.b)

...

kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
kernel: SQUASHFS error: Unable to read directory block [4b6f15e:125]
kernel: SQUASHFS error: Unable to read directory block [4b6d15c:1dae]
kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
(d-sysctl)[55]: systemd-sysctl.service: Failed to set up credentials:
Protocol error
kernel: SQUASHFS error: Unable to read directory block [4b73162:14f0]
kernel: SQUASHFS error: Unable to read directory block [4b6f15e:838]
systemd[1]: Starting Create Static Device Nodes in /dev...
kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
kernel: SQUASHFS error: Unable to read directory block [4b6f15e:838]
kernel: SQUASHFS error: Unable to read directory block [4b6d15c:1dae]
kernel: SQUASHFS error: Unable to read directory block [4b6f15e:125]

I've briefly tried adding some error info the the squashfs error
messages, but it looks like it's getting bad data. I.e. one failure a
sanity check of `dir_count`:

if (dir_count > SQUASHFS_DIR_COUNT)
	goto data_error;

It fails with `dir_count` being 1952803684 ...

So is this a case of wrong/bad timings?

Miquel:
I can tell from the code, that the READCACHESEQ operations are followed
by NAND_OP_WAIT_RDY(tR_max, tRR_min). From the Micron datasheet[0], it
should be NAND_OP_WAIT_RDY(tRCBSY_max, tRR_min), where tRCBSY is
defined to be between 3 and 25 µs.

Not sure if this is related though.

[0] (login required)
https://www.micron.com/products/nand-flash/slc-nand/part-catalog/mt29f4g08abadawp

// Martin

> ---
> v2:
> - change title as suggested by Miquel
> - adjust controller sentence from implement to support
> - fix missing true assignement for flashes
> - add CC stable instead of empty line
> - add documentation comment for new supports_read_cache bool inside
>   nand parameter struct
> 
>  drivers/mtd/nand/raw/nand_base.c  | 3 +++
>  drivers/mtd/nand/raw/nand_jedec.c | 3 +++
>  drivers/mtd/nand/raw/nand_onfi.c  | 3 +++
>  include/linux/mtd/jedec.h         | 3 +++
>  include/linux/mtd/onfi.h          | 1 +
>  include/linux/mtd/rawnand.h       | 2 ++
>  6 files changed, 15 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c
> b/drivers/mtd/nand/raw/nand_base.c
> index d4b55155aeae..1fcac403cee6 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -5110,6 +5110,9 @@ static void
> rawnand_check_cont_read_support(struct nand_chip *chip)
>  {
>         struct mtd_info *mtd = nand_to_mtd(chip);
>  
> +       if (!chip->parameters.supports_read_cache)
> +               return;
> +
>         if (chip->read_retries)
>                 return;
>  
> diff --git a/drivers/mtd/nand/raw/nand_jedec.c
> b/drivers/mtd/nand/raw/nand_jedec.c
> index 836757717660..b3cc8f360529 100644
> --- a/drivers/mtd/nand/raw/nand_jedec.c
> +++ b/drivers/mtd/nand/raw/nand_jedec.c
> @@ -94,6 +94,9 @@ int nand_jedec_detect(struct nand_chip *chip)
>                 goto free_jedec_param_page;
>         }
>  
> +       if (p->opt_cmd[0] & JEDEC_OPT_CMD_READ_CACHE)
> +               chip->parameters.supports_read_cache = true;
> +
>         memorg->pagesize = le32_to_cpu(p->byte_per_page);
>         mtd->writesize = memorg->pagesize;
>  
> diff --git a/drivers/mtd/nand/raw/nand_onfi.c
> b/drivers/mtd/nand/raw/nand_onfi.c
> index f15ef90aec8c..861975e44b55 100644
> --- a/drivers/mtd/nand/raw/nand_onfi.c
> +++ b/drivers/mtd/nand/raw/nand_onfi.c
> @@ -303,6 +303,9 @@ int nand_onfi_detect(struct nand_chip *chip)
>                            ONFI_FEATURE_ADDR_TIMING_MODE, 1);
>         }
>  
> +       if (le16_to_cpu(p->opt_cmd) & ONFI_OPT_CMD_READ_CACHE)
> +               chip->parameters.supports_read_cache = true;
> +
>         onfi = kzalloc(sizeof(*onfi), GFP_KERNEL);
>         if (!onfi) {
>                 ret = -ENOMEM;
> diff --git a/include/linux/mtd/jedec.h b/include/linux/mtd/jedec.h
> index 0b6b59f7cfbd..56047a4e54c9 100644
> --- a/include/linux/mtd/jedec.h
> +++ b/include/linux/mtd/jedec.h
> @@ -21,6 +21,9 @@ struct jedec_ecc_info {
>  /* JEDEC features */
>  #define JEDEC_FEATURE_16_BIT_BUS       (1 << 0)
>  
> +/* JEDEC Optional Commands */
> +#define JEDEC_OPT_CMD_READ_CACHE       BIT(1)
> +
>  struct nand_jedec_params {
>         /* rev info and features block */
>         /* 'J' 'E' 'S' 'D'  */
> diff --git a/include/linux/mtd/onfi.h b/include/linux/mtd/onfi.h
> index a7376f9beddf..55ab2e4d62f9 100644
> --- a/include/linux/mtd/onfi.h
> +++ b/include/linux/mtd/onfi.h
> @@ -55,6 +55,7 @@
>  #define ONFI_SUBFEATURE_PARAM_LEN      4
>  
>  /* ONFI optional commands SET/GET FEATURES supported? */
> +#define ONFI_OPT_CMD_READ_CACHE                BIT(1)
>  #define ONFI_OPT_CMD_SET_GET_FEATURES  BIT(2)
>  
>  struct nand_onfi_params {
> diff --git a/include/linux/mtd/rawnand.h
> b/include/linux/mtd/rawnand.h
> index 90a141ba2a5a..c29ace15a053 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -225,6 +225,7 @@ struct gpio_desc;
>   * struct nand_parameters - NAND generic parameters from the
> parameter page
>   * @model: Model name
>   * @supports_set_get_features: The NAND chip supports
> setting/getting features
> + * @supports_read_cache: The NAND chip supports read cache
> operations
>   * @set_feature_list: Bitmap of features that can be set
>   * @get_feature_list: Bitmap of features that can be get
>   * @onfi: ONFI specific parameters
> @@ -233,6 +234,7 @@ struct nand_parameters {
>         /* Generic parameters */
>         const char *model;
>         bool supports_set_get_features;
> +       bool supports_read_cache;
>         DECLARE_BITMAP(set_feature_list, ONFI_FEATURE_NUMBER);
>         DECLARE_BITMAP(get_feature_list, ONFI_FEATURE_NUMBER);
>  
> 
> base-commit: 42dc814987c1feb6410904e58cfd4c36c4146150
  

Hi Martin,

+ Bean and Domenico

martin@geanix.com wrote on Mon, 25 Sep 2023 13:01:06 +0200:

> Hi Rouven,
> 
> On Fri, 2023-09-22 at 16:17 +0200, Rouven Czerwinski wrote:
> > Both the JEDEC and ONFI specification say that read cache sequential
> > support is an optional command. This means that we not only need to
> > check whether the individual controller supports the command, we also
> > need to check the parameter pages for both ONFI and JEDEC NAND
> > flashes
> > before enabling sequential cache reads.
> > 
> > This fixes support for NAND flashes which don't support enabling
> > cache
> > reads, i.e. Samsung K9F4G08U0F or Toshiba TC58NVG0S3HTA00.
> > 
> > Sequential cache reads are now only available for ONFI and JEDEC
> > devices, if individual vendors implement this, it needs to be enabled
> > per vendor.
> > 
> > Tested on i.MX6Q with a Samsung NAND flash chip that doesn't support
> > sequential reads.
> > 
> > Fixes: 003fe4b9545b ("mtd: rawnand: Support for sequential cache
> > reads")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>  
> 
> Thanks for this. It works as expected for my Toshiba chip, obviously
> because it doesn't use ONFI or JEDEC.
> 
> Unfortunately, my Micron chip does use ONFI, and it sets the cached-
> read-supported bit. It then fails when reading afterwords:
> 
> kernel: ONFI_OPT_CMD_READ_CACHE # debug added by me
> kernel: nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xdc
> kernel: nand: Micron MT29F4G08ABAFAWP
> kernel: nand: 512 MiB, SLC, erase size: 256 KiB, page size: 4096, OOB
> size: 256
> kernel: nand: continued read supported # debug added by me
> kernel: Bad block table found at page 131008, version 0x01
> kernel: Bad block table found at page 130944, version 0x01
> kernel: 2 fixed-partitions partitions found on MTD device gpmi-nand
> kernel: Creating 2 MTD partitions on "gpmi-nand":
> kernel: 0x000000000000-0x000000800000 : "boot"
> kernel: 0x000000800000-0x000020000000 : "ubi"
> kernel: gpmi-nand 1806000.nand-controller: driver registered.
> 
> ...
> 
> kernel: ubi0: default fastmap pool size: 100
> kernel: ubi0: default fastmap WL pool size: 50
> kernel: ubi0: attaching mtd1
> kernel: ubi0: scanning is finished
> kernel: ubi0: attached mtd1 (name "ubi", size 504 MiB)
> kernel: ubi0: PEB size: 262144 bytes (256 KiB), LEB size: 253952 bytes
> kernel: ubi0: min./max. I/O unit sizes: 4096/4096, sub-page size 4096
> kernel: ubi0: VID header offset: 4096 (aligned 4096), data offset: 8192
> kernel: ubi0: good PEBs: 2012, bad PEBs: 4, corrupted PEBs: 0
> kernel: ubi0: user volume: 9, internal volumes: 1, max. volumes count:
> 128
> kernel: ubi0: max/mean erase counter: 4/2, WL threshold: 4096, image
> sequence number: 1431497221
> kernel: ubi0: available PEBs: 12, total reserved PEBs: 2000, PEBs
> reserved for bad PEB handling: 36
> kernel: block ubiblock0_4: created from ubi0:4(rootfs.a)
> kernel: ubi0: background thread "ubi_bgt0d" started, PID 36
> kernel: block ubiblock0_6: created from ubi0:6(appfs.a)
> kernel: block ubiblock0_7: created from ubi0:7(appfs.b)
> 
> ...
> 
> kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
> kernel: SQUASHFS error: Unable to read directory block [4b6f15e:125]
> kernel: SQUASHFS error: Unable to read directory block [4b6d15c:1dae]
> kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
> (d-sysctl)[55]: systemd-sysctl.service: Failed to set up credentials:
> Protocol error
> kernel: SQUASHFS error: Unable to read directory block [4b73162:14f0]
> kernel: SQUASHFS error: Unable to read directory block [4b6f15e:838]
> systemd[1]: Starting Create Static Device Nodes in /dev...
> kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
> kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
> kernel: SQUASHFS error: Unable to read directory block [4b6f15e:838]
> kernel: SQUASHFS error: Unable to read directory block [4b6d15c:1dae]
> kernel: SQUASHFS error: Unable to read directory block [4b6f15e:125]
> 
> I've briefly tried adding some error info the the squashfs error
> messages, but it looks like it's getting bad data. I.e. one failure a
> sanity check of `dir_count`:
> 
> if (dir_count > SQUASHFS_DIR_COUNT)
> 	goto data_error;
> 
> It fails with `dir_count` being 1952803684 ...
> 
> So is this a case of wrong/bad timings?
> 
> Miquel:
> I can tell from the code, that the READCACHESEQ operations are followed
> by NAND_OP_WAIT_RDY(tR_max, tRR_min). From the Micron datasheet[0], it
> should be NAND_OP_WAIT_RDY(tRCBSY_max, tRR_min), where tRCBSY is
> defined to be between 3 and 25 µs.
> 
> Not sure if this is related though.

I believe tR_max is bigger (200us), but please check that this is
actually the case, you can also try enlarging this delay. But if it was
a timeout issue we would probably see errors from the controller driver?

Bean, Domenico, is there any AN or specific document which I missed
asking not to use this feature on specific chips? Are they supposed to
work?

> 
> [0] (login required)
> https://www.micron.com/products/nand-flash/slc-nand/part-catalog/mt29f4g08abadawp
> 
> // Martin
> 
> > ---
> > v2:
> > - change title as suggested by Miquel
> > - adjust controller sentence from implement to support
> > - fix missing true assignement for flashes
> > - add CC stable instead of empty line
> > - add documentation comment for new supports_read_cache bool inside
> >   nand parameter struct
> > 
> >  drivers/mtd/nand/raw/nand_base.c  | 3 +++
> >  drivers/mtd/nand/raw/nand_jedec.c | 3 +++
> >  drivers/mtd/nand/raw/nand_onfi.c  | 3 +++
> >  include/linux/mtd/jedec.h         | 3 +++
> >  include/linux/mtd/onfi.h          | 1 +
> >  include/linux/mtd/rawnand.h       | 2 ++
> >  6 files changed, 15 insertions(+)
> > 
> > diff --git a/drivers/mtd/nand/raw/nand_base.c
> > b/drivers/mtd/nand/raw/nand_base.c
> > index d4b55155aeae..1fcac403cee6 100644
> > --- a/drivers/mtd/nand/raw/nand_base.c
> > +++ b/drivers/mtd/nand/raw/nand_base.c
> > @@ -5110,6 +5110,9 @@ static void
> > rawnand_check_cont_read_support(struct nand_chip *chip)
> >  {
> >         struct mtd_info *mtd = nand_to_mtd(chip);
> >  
> > +       if (!chip->parameters.supports_read_cache)
> > +               return;
> > +
> >         if (chip->read_retries)
> >                 return;
> >  
> > diff --git a/drivers/mtd/nand/raw/nand_jedec.c
> > b/drivers/mtd/nand/raw/nand_jedec.c
> > index 836757717660..b3cc8f360529 100644
> > --- a/drivers/mtd/nand/raw/nand_jedec.c
> > +++ b/drivers/mtd/nand/raw/nand_jedec.c
> > @@ -94,6 +94,9 @@ int nand_jedec_detect(struct nand_chip *chip)
> >                 goto free_jedec_param_page;
> >         }
> >  
> > +       if (p->opt_cmd[0] & JEDEC_OPT_CMD_READ_CACHE)
> > +               chip->parameters.supports_read_cache = true;
> > +
> >         memorg->pagesize = le32_to_cpu(p->byte_per_page);
> >         mtd->writesize = memorg->pagesize;
> >  
> > diff --git a/drivers/mtd/nand/raw/nand_onfi.c
> > b/drivers/mtd/nand/raw/nand_onfi.c
> > index f15ef90aec8c..861975e44b55 100644
> > --- a/drivers/mtd/nand/raw/nand_onfi.c
> > +++ b/drivers/mtd/nand/raw/nand_onfi.c
> > @@ -303,6 +303,9 @@ int nand_onfi_detect(struct nand_chip *chip)
> >                            ONFI_FEATURE_ADDR_TIMING_MODE, 1);
> >         }
> >  
> > +       if (le16_to_cpu(p->opt_cmd) & ONFI_OPT_CMD_READ_CACHE)
> > +               chip->parameters.supports_read_cache = true;
> > +
> >         onfi = kzalloc(sizeof(*onfi), GFP_KERNEL);
> >         if (!onfi) {
> >                 ret = -ENOMEM;
> > diff --git a/include/linux/mtd/jedec.h b/include/linux/mtd/jedec.h
> > index 0b6b59f7cfbd..56047a4e54c9 100644
> > --- a/include/linux/mtd/jedec.h
> > +++ b/include/linux/mtd/jedec.h
> > @@ -21,6 +21,9 @@ struct jedec_ecc_info {
> >  /* JEDEC features */
> >  #define JEDEC_FEATURE_16_BIT_BUS       (1 << 0)
> >  
> > +/* JEDEC Optional Commands */
> > +#define JEDEC_OPT_CMD_READ_CACHE       BIT(1)
> > +
> >  struct nand_jedec_params {
> >         /* rev info and features block */
> >         /* 'J' 'E' 'S' 'D'  */
> > diff --git a/include/linux/mtd/onfi.h b/include/linux/mtd/onfi.h
> > index a7376f9beddf..55ab2e4d62f9 100644
> > --- a/include/linux/mtd/onfi.h
> > +++ b/include/linux/mtd/onfi.h
> > @@ -55,6 +55,7 @@
> >  #define ONFI_SUBFEATURE_PARAM_LEN      4
> >  
> >  /* ONFI optional commands SET/GET FEATURES supported? */
> > +#define ONFI_OPT_CMD_READ_CACHE                BIT(1)
> >  #define ONFI_OPT_CMD_SET_GET_FEATURES  BIT(2)
> >  
> >  struct nand_onfi_params {
> > diff --git a/include/linux/mtd/rawnand.h
> > b/include/linux/mtd/rawnand.h
> > index 90a141ba2a5a..c29ace15a053 100644
> > --- a/include/linux/mtd/rawnand.h
> > +++ b/include/linux/mtd/rawnand.h
> > @@ -225,6 +225,7 @@ struct gpio_desc;
> >   * struct nand_parameters - NAND generic parameters from the
> > parameter page
> >   * @model: Model name
> >   * @supports_set_get_features: The NAND chip supports
> > setting/getting features
> > + * @supports_read_cache: The NAND chip supports read cache
> > operations
> >   * @set_feature_list: Bitmap of features that can be set
> >   * @get_feature_list: Bitmap of features that can be get
> >   * @onfi: ONFI specific parameters
> > @@ -233,6 +234,7 @@ struct nand_parameters {
> >         /* Generic parameters */
> >         const char *model;
> >         bool supports_set_get_features;
> > +       bool supports_read_cache;
> >         DECLARE_BITMAP(set_feature_list, ONFI_FEATURE_NUMBER);
> >         DECLARE_BITMAP(get_feature_list, ONFI_FEATURE_NUMBER);
> >  
> > 
> > base-commit: 42dc814987c1feb6410904e58cfd4c36c4146150  
> 


Thanks,
Miquèl
  

Hi Martin,

+ Bean and Domenico, there is a question for you below.

martin@geanix.com wrote on Mon, 25 Sep 2023 13:01:06 +0200:

> Hi Rouven,
> 
> On Fri, 2023-09-22 at 16:17 +0200, Rouven Czerwinski wrote:
> > Both the JEDEC and ONFI specification say that read cache sequential
> > support is an optional command. This means that we not only need to
> > check whether the individual controller supports the command, we also
> > need to check the parameter pages for both ONFI and JEDEC NAND
> > flashes
> > before enabling sequential cache reads.
> > 
> > This fixes support for NAND flashes which don't support enabling
> > cache
> > reads, i.e. Samsung K9F4G08U0F or Toshiba TC58NVG0S3HTA00.
> > 
> > Sequential cache reads are now only available for ONFI and JEDEC
> > devices, if individual vendors implement this, it needs to be enabled
> > per vendor.
> > 
> > Tested on i.MX6Q with a Samsung NAND flash chip that doesn't support
> > sequential reads.
> > 
> > Fixes: 003fe4b9545b ("mtd: rawnand: Support for sequential cache
> > reads")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>  
> 
> Thanks for this. It works as expected for my Toshiba chip, obviously
> because it doesn't use ONFI or JEDEC.
> 
> Unfortunately, my Micron chip does use ONFI, and it sets the cached-
> read-supported bit. It then fails when reading afterwords:
> 
> kernel: ONFI_OPT_CMD_READ_CACHE # debug added by me
> kernel: nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xdc
> kernel: nand: Micron MT29F4G08ABAFAWP
> kernel: nand: 512 MiB, SLC, erase size: 256 KiB, page size: 4096, OOB
> size: 256
> kernel: nand: continued read supported # debug added by me
> kernel: Bad block table found at page 131008, version 0x01
> kernel: Bad block table found at page 130944, version 0x01
> kernel: 2 fixed-partitions partitions found on MTD device gpmi-nand
> kernel: Creating 2 MTD partitions on "gpmi-nand":
> kernel: 0x000000000000-0x000000800000 : "boot"
> kernel: 0x000000800000-0x000020000000 : "ubi"
> kernel: gpmi-nand 1806000.nand-controller: driver registered.
> 
> ...
> 
> kernel: ubi0: default fastmap pool size: 100
> kernel: ubi0: default fastmap WL pool size: 50
> kernel: ubi0: attaching mtd1
> kernel: ubi0: scanning is finished
> kernel: ubi0: attached mtd1 (name "ubi", size 504 MiB)
> kernel: ubi0: PEB size: 262144 bytes (256 KiB), LEB size: 253952 bytes
> kernel: ubi0: min./max. I/O unit sizes: 4096/4096, sub-page size 4096
> kernel: ubi0: VID header offset: 4096 (aligned 4096), data offset: 8192
> kernel: ubi0: good PEBs: 2012, bad PEBs: 4, corrupted PEBs: 0
> kernel: ubi0: user volume: 9, internal volumes: 1, max. volumes count:
> 128
> kernel: ubi0: max/mean erase counter: 4/2, WL threshold: 4096, image
> sequence number: 1431497221
> kernel: ubi0: available PEBs: 12, total reserved PEBs: 2000, PEBs
> reserved for bad PEB handling: 36
> kernel: block ubiblock0_4: created from ubi0:4(rootfs.a)
> kernel: ubi0: background thread "ubi_bgt0d" started, PID 36
> kernel: block ubiblock0_6: created from ubi0:6(appfs.a)
> kernel: block ubiblock0_7: created from ubi0:7(appfs.b)
> 
> ...
> 
> kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
> kernel: SQUASHFS error: Unable to read directory block [4b6f15e:125]
> kernel: SQUASHFS error: Unable to read directory block [4b6d15c:1dae]
> kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
> (d-sysctl)[55]: systemd-sysctl.service: Failed to set up credentials:
> Protocol error
> kernel: SQUASHFS error: Unable to read directory block [4b73162:14f0]
> kernel: SQUASHFS error: Unable to read directory block [4b6f15e:838]
> systemd[1]: Starting Create Static Device Nodes in /dev...
> kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
> kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
> kernel: SQUASHFS error: Unable to read directory block [4b6f15e:838]
> kernel: SQUASHFS error: Unable to read directory block [4b6d15c:1dae]
> kernel: SQUASHFS error: Unable to read directory block [4b6f15e:125]
> 
> I've briefly tried adding some error info the the squashfs error
> messages, but it looks like it's getting bad data. I.e. one failure a
> sanity check of `dir_count`:
> 
> if (dir_count > SQUASHFS_DIR_COUNT)
> 	goto data_error;
> 
> It fails with `dir_count` being 1952803684 ...
> 
> So is this a case of wrong/bad timings?
> 
> Miquel:
> I can tell from the code, that the READCACHESEQ operations are followed
> by NAND_OP_WAIT_RDY(tR_max, tRR_min). From the Micron datasheet[0], it
> should be NAND_OP_WAIT_RDY(tRCBSY_max, tRR_min), where tRCBSY is
> defined to be between 3 and 25 µs.

I found a place in the ONFI spec states taht tRCBSY_max should be
between 3 and tR_max, so indeed we should be fine on that regard.

However, I asked myself whether we could have issues when crossing
boundaries. Block boundaries should be fine, however your device does
not support crossing plane boundaries, as bit 4 ("read cache
supported") of byte 114 ("Multi-plane operation attributes") in the
memory organization block of the parameter page is not set (the value
of the byte should be 0x0E if I get it right.

Anyway, our main issue here does not seem related to the boundaries. It
does not seem to be explicitly marked anywhere else but on the front
page:
	Advanced command set
	– Program page cache mode (4)
	– Read page cache mode (4)
	– Two-plane commands (4)

	(4) These commands supported only with ECC disabled.

Read page cache mode without ECC makes the feature pretty useless IMHO.

Bean, Domenico, how do we know which devices allow ECC correction
during sequential page reads and which don't? Is there a (vendor?) bit
somewhere in the parameter page for that? Do we have any way to know
besides a list of devices allowing that? If so, can you provide one
with a few IDs? 

Thanks,
Miquèl
  

Micron Confidential

Hello Miquel,

The MPN mt29f4g08abadawp is a single LUN device and it is supposed to support cache read with the limitation reported into the datasheet.

Can you clarify about the IDs list you need?

Thanks.
Regards,
Domenico P.




Micron Confidential
-----Original Message-----
From: Miquel Raynal <miquel.raynal@bootlin.com>
Sent: Tuesday, September 26, 2023 1:27 PM
To: Martin Hundebøll <martin@geanix.com>
Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>; Måns Rullgård <mans@mansr.com>; Alexander Shiyan <eagle.alexander923@gmail.com>; Richard Weinberger <richard@nod.at>; Vignesh Raghavendra <vigneshr@ti.com>; JaimeLiao <jaimeliao.tw@gmail.com>; kernel@pengutronix.de; stable@vger.kernel.org; linux-mtd@lists.infradead.org; linux-kernel@vger.kernel.org; Sean Nyekjær <sean@geanix.com>; Domenico Punzo <dpunzo@micron.com>; Bean Huo <beanhuo@micron.com>
Subject: [EXT] Re: [PATCH v2] mtd: rawnand: Ensure the nand chip supports cached reads

CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless you recognize the sender and were expecting this message.


Hi Martin,

+ Bean and Domenico, there is a question for you below.

martin@geanix.com wrote on Mon, 25 Sep 2023 13:01:06 +0200:

> Hi Rouven,
>
> On Fri, 2023-09-22 at 16:17 +0200, Rouven Czerwinski wrote:
> > Both the JEDEC and ONFI specification say that read cache sequential
> > support is an optional command. This means that we not only need to
> > check whether the individual controller supports the command, we
> > also need to check the parameter pages for both ONFI and JEDEC NAND
> > flashes before enabling sequential cache reads.
> >
> > This fixes support for NAND flashes which don't support enabling
> > cache reads, i.e. Samsung K9F4G08U0F or Toshiba TC58NVG0S3HTA00.
> >
> > Sequential cache reads are now only available for ONFI and JEDEC
> > devices, if individual vendors implement this, it needs to be
> > enabled per vendor.
> >
> > Tested on i.MX6Q with a Samsung NAND flash chip that doesn't support
> > sequential reads.
> >
> > Fixes: 003fe4b9545b ("mtd: rawnand: Support for sequential cache
> > reads")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
>
> Thanks for this. It works as expected for my Toshiba chip, obviously
> because it doesn't use ONFI or JEDEC.
>
> Unfortunately, my Micron chip does use ONFI, and it sets the cached-
> read-supported bit. It then fails when reading afterwords:
>
> kernel: ONFI_OPT_CMD_READ_CACHE # debug added by me
> kernel: nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xdc
> kernel: nand: Micron MT29F4G08ABAFAWP
> kernel: nand: 512 MiB, SLC, erase size: 256 KiB, page size: 4096, OOB
> size: 256
> kernel: nand: continued read supported # debug added by me
> kernel: Bad block table found at page 131008, version 0x01
> kernel: Bad block table found at page 130944, version 0x01
> kernel: 2 fixed-partitions partitions found on MTD device gpmi-nand
> kernel: Creating 2 MTD partitions on "gpmi-nand":
> kernel: 0x000000000000-0x000000800000 : "boot"
> kernel: 0x000000800000-0x000020000000 : "ubi"
> kernel: gpmi-nand 1806000.nand-controller: driver registered.
>
> ...
>
> kernel: ubi0: default fastmap pool size: 100
> kernel: ubi0: default fastmap WL pool size: 50
> kernel: ubi0: attaching mtd1
> kernel: ubi0: scanning is finished
> kernel: ubi0: attached mtd1 (name "ubi", size 504 MiB)
> kernel: ubi0: PEB size: 262144 bytes (256 KiB), LEB size: 253952 bytes
> kernel: ubi0: min./max. I/O unit sizes: 4096/4096, sub-page size 4096
> kernel: ubi0: VID header offset: 4096 (aligned 4096), data offset:
> 8192
> kernel: ubi0: good PEBs: 2012, bad PEBs: 4, corrupted PEBs: 0
> kernel: ubi0: user volume: 9, internal volumes: 1, max. volumes count:
> 128
> kernel: ubi0: max/mean erase counter: 4/2, WL threshold: 4096, image
> sequence number: 1431497221
> kernel: ubi0: available PEBs: 12, total reserved PEBs: 2000, PEBs
> reserved for bad PEB handling: 36
> kernel: block ubiblock0_4: created from ubi0:4(rootfs.a)
> kernel: ubi0: background thread "ubi_bgt0d" started, PID 36
> kernel: block ubiblock0_6: created from ubi0:6(appfs.a)
> kernel: block ubiblock0_7: created from ubi0:7(appfs.b)
>
> ...
>
> kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
> kernel: SQUASHFS error: Unable to read directory block [4b6f15e:125]
> kernel: SQUASHFS error: Unable to read directory block [4b6d15c:1dae]
> kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
> (d-sysctl)[55]: systemd-sysctl.service: Failed to set up credentials:
> Protocol error
> kernel: SQUASHFS error: Unable to read directory block [4b73162:14f0]
> kernel: SQUASHFS error: Unable to read directory block [4b6f15e:838]
> systemd[1]: Starting Create Static Device Nodes in /dev...
> kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
> kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
> kernel: SQUASHFS error: Unable to read directory block [4b6f15e:838]
> kernel: SQUASHFS error: Unable to read directory block [4b6d15c:1dae]
> kernel: SQUASHFS error: Unable to read directory block [4b6f15e:125]
>
> I've briefly tried adding some error info the the squashfs error
> messages, but it looks like it's getting bad data. I.e. one failure a
> sanity check of `dir_count`:
>
> if (dir_count > SQUASHFS_DIR_COUNT)
>       goto data_error;
>
> It fails with `dir_count` being 1952803684 ...
>
> So is this a case of wrong/bad timings?
>
> Miquel:
> I can tell from the code, that the READCACHESEQ operations are
> followed by NAND_OP_WAIT_RDY(tR_max, tRR_min). From the Micron
> datasheet[0], it should be NAND_OP_WAIT_RDY(tRCBSY_max, tRR_min),
> where tRCBSY is defined to be between 3 and 25 µs.

I found a place in the ONFI spec states taht tRCBSY_max should be between 3 and tR_max, so indeed we should be fine on that regard.

However, I asked myself whether we could have issues when crossing boundaries. Block boundaries should be fine, however your device does not support crossing plane boundaries, as bit 4 ("read cache
supported") of byte 114 ("Multi-plane operation attributes") in the memory organization block of the parameter page is not set (the value of the byte should be 0x0E if I get it right.

Anyway, our main issue here does not seem related to the boundaries. It does not seem to be explicitly marked anywhere else but on the front
page:
        Advanced command set
        – Program page cache mode (4)
        – Read page cache mode (4)
        – Two-plane commands (4)

        (4) These commands supported only with ECC disabled.

Read page cache mode without ECC makes the feature pretty useless IMHO.

Bean, Domenico, how do we know which devices allow ECC correction during sequential page reads and which don't? Is there a (vendor?) bit somewhere in the parameter page for that? Do we have any way to know besides a list of devices allowing that? If so, can you provide one with a few IDs?

Thanks,
Miquèl
  

Hi Domenico,

dpunzo@micron.com wrote on Wed, 27 Sep 2023 06:28:04 +0000:

> Micron Confidential
> 
> Hello Miquel,
> 
> The MPN mt29f4g08abadawp is a single LUN device and it is supposed to support cache read with the limitation reported into the datasheet.

Yes, but this limitation is not covered by the ONFI specification.

> Can you clarify about the IDs list you need?

In the NAND core, we need to know which devices do/do not support
sequential cache reads *with* ECC enabled.

We can look at the parameter page and know whether sequential cache
reads are supported or not, but there is no way (as far as I know) to
detect whether a chip supports sequential cache reads with ECC enabled
other than looking at the datasheet.

Can you help by either telling how we can dynamically discover whether
ECC can be enabled during cache operations on Micron NAND chips or, if
that is not possible, provide a list of devices which support it?

Thanks,
Miquèl

> 
> Thanks.
> Regards,
> Domenico P.
> 
> 
> 
> 
> Micron Confidential
> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Sent: Tuesday, September 26, 2023 1:27 PM
> To: Martin Hundebøll <martin@geanix.com>
> Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>; Måns Rullgård <mans@mansr.com>; Alexander Shiyan <eagle.alexander923@gmail.com>; Richard Weinberger <richard@nod.at>; Vignesh Raghavendra <vigneshr@ti.com>; JaimeLiao <jaimeliao.tw@gmail.com>; kernel@pengutronix.de; stable@vger.kernel.org; linux-mtd@lists.infradead.org; linux-kernel@vger.kernel.org; Sean Nyekjær <sean@geanix.com>; Domenico Punzo <dpunzo@micron.com>; Bean Huo <beanhuo@micron.com>
> Subject: [EXT] Re: [PATCH v2] mtd: rawnand: Ensure the nand chip supports cached reads
> 
> CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless you recognize the sender and were expecting this message.
> 
> 
> Hi Martin,
> 
> + Bean and Domenico, there is a question for you below.
> 
> martin@geanix.com wrote on Mon, 25 Sep 2023 13:01:06 +0200:
> 
> > Hi Rouven,
> >
> > On Fri, 2023-09-22 at 16:17 +0200, Rouven Czerwinski wrote:  
> > > Both the JEDEC and ONFI specification say that read cache sequential
> > > support is an optional command. This means that we not only need to
> > > check whether the individual controller supports the command, we
> > > also need to check the parameter pages for both ONFI and JEDEC NAND
> > > flashes before enabling sequential cache reads.
> > >
> > > This fixes support for NAND flashes which don't support enabling
> > > cache reads, i.e. Samsung K9F4G08U0F or Toshiba TC58NVG0S3HTA00.
> > >
> > > Sequential cache reads are now only available for ONFI and JEDEC
> > > devices, if individual vendors implement this, it needs to be
> > > enabled per vendor.
> > >
> > > Tested on i.MX6Q with a Samsung NAND flash chip that doesn't support
> > > sequential reads.
> > >
> > > Fixes: 003fe4b9545b ("mtd: rawnand: Support for sequential cache
> > > reads")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>  
> >
> > Thanks for this. It works as expected for my Toshiba chip, obviously
> > because it doesn't use ONFI or JEDEC.
> >
> > Unfortunately, my Micron chip does use ONFI, and it sets the cached-
> > read-supported bit. It then fails when reading afterwords:
> >
> > kernel: ONFI_OPT_CMD_READ_CACHE # debug added by me
> > kernel: nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xdc
> > kernel: nand: Micron MT29F4G08ABAFAWP
> > kernel: nand: 512 MiB, SLC, erase size: 256 KiB, page size: 4096, OOB
> > size: 256
> > kernel: nand: continued read supported # debug added by me
> > kernel: Bad block table found at page 131008, version 0x01
> > kernel: Bad block table found at page 130944, version 0x01
> > kernel: 2 fixed-partitions partitions found on MTD device gpmi-nand
> > kernel: Creating 2 MTD partitions on "gpmi-nand":
> > kernel: 0x000000000000-0x000000800000 : "boot"
> > kernel: 0x000000800000-0x000020000000 : "ubi"
> > kernel: gpmi-nand 1806000.nand-controller: driver registered.
> >
> > ...
> >
> > kernel: ubi0: default fastmap pool size: 100
> > kernel: ubi0: default fastmap WL pool size: 50
> > kernel: ubi0: attaching mtd1
> > kernel: ubi0: scanning is finished
> > kernel: ubi0: attached mtd1 (name "ubi", size 504 MiB)
> > kernel: ubi0: PEB size: 262144 bytes (256 KiB), LEB size: 253952 bytes
> > kernel: ubi0: min./max. I/O unit sizes: 4096/4096, sub-page size 4096
> > kernel: ubi0: VID header offset: 4096 (aligned 4096), data offset:
> > 8192
> > kernel: ubi0: good PEBs: 2012, bad PEBs: 4, corrupted PEBs: 0
> > kernel: ubi0: user volume: 9, internal volumes: 1, max. volumes count:
> > 128
> > kernel: ubi0: max/mean erase counter: 4/2, WL threshold: 4096, image
> > sequence number: 1431497221
> > kernel: ubi0: available PEBs: 12, total reserved PEBs: 2000, PEBs
> > reserved for bad PEB handling: 36
> > kernel: block ubiblock0_4: created from ubi0:4(rootfs.a)
> > kernel: ubi0: background thread "ubi_bgt0d" started, PID 36
> > kernel: block ubiblock0_6: created from ubi0:6(appfs.a)
> > kernel: block ubiblock0_7: created from ubi0:7(appfs.b)
> >
> > ...
> >
> > kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
> > kernel: SQUASHFS error: Unable to read directory block [4b6f15e:125]
> > kernel: SQUASHFS error: Unable to read directory block [4b6d15c:1dae]
> > kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
> > (d-sysctl)[55]: systemd-sysctl.service: Failed to set up credentials:
> > Protocol error
> > kernel: SQUASHFS error: Unable to read directory block [4b73162:14f0]
> > kernel: SQUASHFS error: Unable to read directory block [4b6f15e:838]
> > systemd[1]: Starting Create Static Device Nodes in /dev...
> > kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
> > kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
> > kernel: SQUASHFS error: Unable to read directory block [4b6f15e:838]
> > kernel: SQUASHFS error: Unable to read directory block [4b6d15c:1dae]
> > kernel: SQUASHFS error: Unable to read directory block [4b6f15e:125]
> >
> > I've briefly tried adding some error info the the squashfs error
> > messages, but it looks like it's getting bad data. I.e. one failure a
> > sanity check of `dir_count`:
> >
> > if (dir_count > SQUASHFS_DIR_COUNT)
> >       goto data_error;
> >
> > It fails with `dir_count` being 1952803684 ...
> >
> > So is this a case of wrong/bad timings?
> >
> > Miquel:
> > I can tell from the code, that the READCACHESEQ operations are
> > followed by NAND_OP_WAIT_RDY(tR_max, tRR_min). From the Micron
> > datasheet[0], it should be NAND_OP_WAIT_RDY(tRCBSY_max, tRR_min),
> > where tRCBSY is defined to be between 3 and 25 µs.  
> 
> I found a place in the ONFI spec states taht tRCBSY_max should be between 3 and tR_max, so indeed we should be fine on that regard.
> 
> However, I asked myself whether we could have issues when crossing boundaries. Block boundaries should be fine, however your device does not support crossing plane boundaries, as bit 4 ("read cache
> supported") of byte 114 ("Multi-plane operation attributes") in the memory organization block of the parameter page is not set (the value of the byte should be 0x0E if I get it right.
> 
> Anyway, our main issue here does not seem related to the boundaries. It does not seem to be explicitly marked anywhere else but on the front
> page:
>         Advanced command set
>         – Program page cache mode (4)
>         – Read page cache mode (4)
>         – Two-plane commands (4)
> 
>         (4) These commands supported only with ECC disabled.
> 
> Read page cache mode without ECC makes the feature pretty useless IMHO.
> 
> Bean, Domenico, how do we know which devices allow ECC correction during sequential page reads and which don't? Is there a (vendor?) bit somewhere in the parameter page for that? Do we have any way to know besides a list of devices allowing that? If so, can you provide one with a few IDs?
> 
> Thanks,
> Miquèl
  

Hi Martin,

miquel.raynal@bootlin.com wrote on Tue, 26 Sep 2023 13:27:25 +0200:

> Hi Martin,
> 
> + Bean and Domenico, there is a question for you below.
> 
> martin@geanix.com wrote on Mon, 25 Sep 2023 13:01:06 +0200:
> 
> > Hi Rouven,
> > 
> > On Fri, 2023-09-22 at 16:17 +0200, Rouven Czerwinski wrote:  
> > > Both the JEDEC and ONFI specification say that read cache sequential
> > > support is an optional command. This means that we not only need to
> > > check whether the individual controller supports the command, we also
> > > need to check the parameter pages for both ONFI and JEDEC NAND
> > > flashes
> > > before enabling sequential cache reads.
> > > 
> > > This fixes support for NAND flashes which don't support enabling
> > > cache
> > > reads, i.e. Samsung K9F4G08U0F or Toshiba TC58NVG0S3HTA00.
> > > 
> > > Sequential cache reads are now only available for ONFI and JEDEC
> > > devices, if individual vendors implement this, it needs to be enabled
> > > per vendor.
> > > 
> > > Tested on i.MX6Q with a Samsung NAND flash chip that doesn't support
> > > sequential reads.
> > > 
> > > Fixes: 003fe4b9545b ("mtd: rawnand: Support for sequential cache
> > > reads")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>    
> > 
> > Thanks for this. It works as expected for my Toshiba chip, obviously
> > because it doesn't use ONFI or JEDEC.
> > 
> > Unfortunately, my Micron chip does use ONFI, and it sets the cached-
> > read-supported bit. It then fails when reading afterwords:

I might have over reacted regarding my findings in Micron's datasheet,
I need to know if you use the on-die ECC engine or if you use the one
on the controller. In the former case the failure is expected. In the
latter case, it's not.

Thanks,
Miquèl

> > kernel: ONFI_OPT_CMD_READ_CACHE # debug added by me
> > kernel: nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xdc
> > kernel: nand: Micron MT29F4G08ABAFAWP
> > kernel: nand: 512 MiB, SLC, erase size: 256 KiB, page size: 4096, OOB
> > size: 256
> > kernel: nand: continued read supported # debug added by me
> > kernel: Bad block table found at page 131008, version 0x01
> > kernel: Bad block table found at page 130944, version 0x01
> > kernel: 2 fixed-partitions partitions found on MTD device gpmi-nand
> > kernel: Creating 2 MTD partitions on "gpmi-nand":
> > kernel: 0x000000000000-0x000000800000 : "boot"
> > kernel: 0x000000800000-0x000020000000 : "ubi"
> > kernel: gpmi-nand 1806000.nand-controller: driver registered.
> > 
> > ...
> > 
> > kernel: ubi0: default fastmap pool size: 100
> > kernel: ubi0: default fastmap WL pool size: 50
> > kernel: ubi0: attaching mtd1
> > kernel: ubi0: scanning is finished
> > kernel: ubi0: attached mtd1 (name "ubi", size 504 MiB)
> > kernel: ubi0: PEB size: 262144 bytes (256 KiB), LEB size: 253952 bytes
> > kernel: ubi0: min./max. I/O unit sizes: 4096/4096, sub-page size 4096
> > kernel: ubi0: VID header offset: 4096 (aligned 4096), data offset: 8192
> > kernel: ubi0: good PEBs: 2012, bad PEBs: 4, corrupted PEBs: 0
> > kernel: ubi0: user volume: 9, internal volumes: 1, max. volumes count:
> > 128
> > kernel: ubi0: max/mean erase counter: 4/2, WL threshold: 4096, image
> > sequence number: 1431497221
> > kernel: ubi0: available PEBs: 12, total reserved PEBs: 2000, PEBs
> > reserved for bad PEB handling: 36
> > kernel: block ubiblock0_4: created from ubi0:4(rootfs.a)
> > kernel: ubi0: background thread "ubi_bgt0d" started, PID 36
> > kernel: block ubiblock0_6: created from ubi0:6(appfs.a)
> > kernel: block ubiblock0_7: created from ubi0:7(appfs.b)
> > 
> > ...
> > 
> > kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
> > kernel: SQUASHFS error: Unable to read directory block [4b6f15e:125]
> > kernel: SQUASHFS error: Unable to read directory block [4b6d15c:1dae]
> > kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
> > (d-sysctl)[55]: systemd-sysctl.service: Failed to set up credentials:
> > Protocol error
> > kernel: SQUASHFS error: Unable to read directory block [4b73162:14f0]
> > kernel: SQUASHFS error: Unable to read directory block [4b6f15e:838]
> > systemd[1]: Starting Create Static Device Nodes in /dev...
> > kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
> > kernel: SQUASHFS error: Unable to read directory block [4b6d15c:ed1]
> > kernel: SQUASHFS error: Unable to read directory block [4b6f15e:838]
> > kernel: SQUASHFS error: Unable to read directory block [4b6d15c:1dae]
> > kernel: SQUASHFS error: Unable to read directory block [4b6f15e:125]
> > 
> > I've briefly tried adding some error info the the squashfs error
> > messages, but it looks like it's getting bad data. I.e. one failure a
> > sanity check of `dir_count`:
> > 
> > if (dir_count > SQUASHFS_DIR_COUNT)
> > 	goto data_error;
> > 
> > It fails with `dir_count` being 1952803684 ...
> > 
> > So is this a case of wrong/bad timings?
> > 
> > Miquel:
> > I can tell from the code, that the READCACHESEQ operations are followed
> > by NAND_OP_WAIT_RDY(tR_max, tRR_min). From the Micron datasheet[0], it
> > should be NAND_OP_WAIT_RDY(tRCBSY_max, tRR_min), where tRCBSY is
> > defined to be between 3 and 25 µs.  
> 
> I found a place in the ONFI spec states taht tRCBSY_max should be
> between 3 and tR_max, so indeed we should be fine on that regard.
> 
> However, I asked myself whether we could have issues when crossing
> boundaries. Block boundaries should be fine, however your device does
> not support crossing plane boundaries, as bit 4 ("read cache
> supported") of byte 114 ("Multi-plane operation attributes") in the
> memory organization block of the parameter page is not set (the value
> of the byte should be 0x0E if I get it right.
> 
> Anyway, our main issue here does not seem related to the boundaries. It
> does not seem to be explicitly marked anywhere else but on the front
> page:
> 	Advanced command set
> 	– Program page cache mode (4)
> 	– Read page cache mode (4)
> 	– Two-plane commands (4)
> 
> 	(4) These commands supported only with ECC disabled.
> 
> Read page cache mode without ECC makes the feature pretty useless IMHO.
> 
> Bean, Domenico, how do we know which devices allow ECC correction
> during sequential page reads and which don't? Is there a (vendor?) bit
> somewhere in the parameter page for that? Do we have any way to know
> besides a list of devices allowing that? If so, can you provide one
> with a few IDs? 
> 
> Thanks,
> Miquèl
  

Hi Miquel,

On Wed, 2023-09-27 at 17:05 +0200, Miquel Raynal wrote:
> Hi Martin,
> 
> miquel.raynal@bootlin.com wrote on Tue, 26 Sep 2023 13:27:25 +0200:
> 
> > Hi Martin,
> > 
> > + Bean and Domenico, there is a question for you below.
> > 
> > martin@geanix.com wrote on Mon, 25 Sep 2023 13:01:06 +0200:
> > 
> > > Hi Rouven,
> > > 
> > > On Fri, 2023-09-22 at 16:17 +0200, Rouven Czerwinski wrote:  
> > > > Both the JEDEC and ONFI specification say that read cache
> > > > sequential
> > > > support is an optional command. This means that we not only
> > > > need to
> > > > check whether the individual controller supports the command,
> > > > we also
> > > > need to check the parameter pages for both ONFI and JEDEC NAND
> > > > flashes
> > > > before enabling sequential cache reads.
> > > > 
> > > > This fixes support for NAND flashes which don't support
> > > > enabling
> > > > cache
> > > > reads, i.e. Samsung K9F4G08U0F or Toshiba TC58NVG0S3HTA00.
> > > > 
> > > > Sequential cache reads are now only available for ONFI and
> > > > JEDEC
> > > > devices, if individual vendors implement this, it needs to be
> > > > enabled
> > > > per vendor.
> > > > 
> > > > Tested on i.MX6Q with a Samsung NAND flash chip that doesn't
> > > > support
> > > > sequential reads.
> > > > 
> > > > Fixes: 003fe4b9545b ("mtd: rawnand: Support for sequential
> > > > cache
> > > > reads")
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Rouven Czerwinski
> > > > <r.czerwinski@pengutronix.de>    
> > > 
> > > Thanks for this. It works as expected for my Toshiba chip,
> > > obviously
> > > because it doesn't use ONFI or JEDEC.
> > > 
> > > Unfortunately, my Micron chip does use ONFI, and it sets the
> > > cached-
> > > read-supported bit. It then fails when reading afterwords:
> 
> I might have over reacted regarding my findings in Micron's
> datasheet,
> I need to know if you use the on-die ECC engine or if you use the one
> on the controller. In the former case the failure is expected. In the
> latter case, it's not.

I use the default, which seems to be the controller engine?

// Martin

> Thanks,
> Miquèl
> 
> > > kernel: ONFI_OPT_CMD_READ_CACHE # debug added by me
> > > kernel: nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xdc
> > > kernel: nand: Micron MT29F4G08ABAFAWP
> > > kernel: nand: 512 MiB, SLC, erase size: 256 KiB, page size: 4096,
> > > OOB
> > > size: 256
> > > kernel: nand: continued read supported # debug added by me
> > > kernel: Bad block table found at page 131008, version 0x01
> > > kernel: Bad block table found at page 130944, version 0x01
> > > kernel: 2 fixed-partitions partitions found on MTD device gpmi-
> > > nand
> > > kernel: Creating 2 MTD partitions on "gpmi-nand":
> > > kernel: 0x000000000000-0x000000800000 : "boot"
> > > kernel: 0x000000800000-0x000020000000 : "ubi"
> > > kernel: gpmi-nand 1806000.nand-controller: driver registered.
> > > 
> > > ...
> > > 
> > > kernel: ubi0: default fastmap pool size: 100
> > > kernel: ubi0: default fastmap WL pool size: 50
> > > kernel: ubi0: attaching mtd1
> > > kernel: ubi0: scanning is finished
> > > kernel: ubi0: attached mtd1 (name "ubi", size 504 MiB)
> > > kernel: ubi0: PEB size: 262144 bytes (256 KiB), LEB size: 253952
> > > bytes
> > > kernel: ubi0: min./max. I/O unit sizes: 4096/4096, sub-page size
> > > 4096
> > > kernel: ubi0: VID header offset: 4096 (aligned 4096), data
> > > offset: 8192
> > > kernel: ubi0: good PEBs: 2012, bad PEBs: 4, corrupted PEBs: 0
> > > kernel: ubi0: user volume: 9, internal volumes: 1, max. volumes
> > > count:
> > > 128
> > > kernel: ubi0: max/mean erase counter: 4/2, WL threshold: 4096,
> > > image
> > > sequence number: 1431497221
> > > kernel: ubi0: available PEBs: 12, total reserved PEBs: 2000, PEBs
> > > reserved for bad PEB handling: 36
> > > kernel: block ubiblock0_4: created from ubi0:4(rootfs.a)
> > > kernel: ubi0: background thread "ubi_bgt0d" started, PID 36
> > > kernel: block ubiblock0_6: created from ubi0:6(appfs.a)
> > > kernel: block ubiblock0_7: created from ubi0:7(appfs.b)
> > > 
> > > ...
> > > 
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6d15c:ed1]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6f15e:125]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6d15c:1dae]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6d15c:ed1]
> > > (d-sysctl)[55]: systemd-sysctl.service: Failed to set up
> > > credentials:
> > > Protocol error
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b73162:14f0]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6f15e:838]
> > > systemd[1]: Starting Create Static Device Nodes in /dev...
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6d15c:ed1]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6d15c:ed1]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6f15e:838]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6d15c:1dae]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6f15e:125]
> > > 
> > > I've briefly tried adding some error info the the squashfs error
> > > messages, but it looks like it's getting bad data. I.e. one
> > > failure a
> > > sanity check of `dir_count`:
> > > 
> > > if (dir_count > SQUASHFS_DIR_COUNT)
> > >         goto data_error;
> > > 
> > > It fails with `dir_count` being 1952803684 ...
> > > 
> > > So is this a case of wrong/bad timings?
> > > 
> > > Miquel:
> > > I can tell from the code, that the READCACHESEQ operations are
> > > followed
> > > by NAND_OP_WAIT_RDY(tR_max, tRR_min). From the Micron
> > > datasheet[0], it
> > > should be NAND_OP_WAIT_RDY(tRCBSY_max, tRR_min), where tRCBSY is
> > > defined to be between 3 and 25 µs.  
> > 
> > I found a place in the ONFI spec states taht tRCBSY_max should be
> > between 3 and tR_max, so indeed we should be fine on that regard.
> > 
> > However, I asked myself whether we could have issues when crossing
> > boundaries. Block boundaries should be fine, however your device
> > does
> > not support crossing plane boundaries, as bit 4 ("read cache
> > supported") of byte 114 ("Multi-plane operation attributes") in the
> > memory organization block of the parameter page is not set (the
> > value
> > of the byte should be 0x0E if I get it right.
> > 
> > Anyway, our main issue here does not seem related to the
> > boundaries. It
> > does not seem to be explicitly marked anywhere else but on the
> > front
> > page:
> >         Advanced command set
> >         – Program page cache mode (4)
> >         – Read page cache mode (4)
> >         – Two-plane commands (4)
> > 
> >         (4) These commands supported only with ECC disabled.
> > 
> > Read page cache mode without ECC makes the feature pretty useless
> > IMHO.
> > 
> > Bean, Domenico, how do we know which devices allow ECC correction
> > during sequential page reads and which don't? Is there a (vendor?)
> > bit
> > somewhere in the parameter page for that? Do we have any way to
> > know
> > besides a list of devices allowing that? If so, can you provide one
> > with a few IDs? 
> > 
> > Thanks,
> > Miquèl
  

Hi Martin,

martin@geanix.com wrote on Thu, 28 Sep 2023 09:19:56 +0200:

> Hi Miquel,
> 
> On Wed, 2023-09-27 at 17:05 +0200, Miquel Raynal wrote:
> > Hi Martin,
> > 
> > miquel.raynal@bootlin.com wrote on Tue, 26 Sep 2023 13:27:25 +0200:
> >   
> > > Hi Martin,
> > > 
> > > + Bean and Domenico, there is a question for you below.
> > > 
> > > martin@geanix.com wrote on Mon, 25 Sep 2023 13:01:06 +0200:
> > >   
> > > > Hi Rouven,
> > > > 
> > > > On Fri, 2023-09-22 at 16:17 +0200, Rouven Czerwinski wrote:    
> > > > > Both the JEDEC and ONFI specification say that read cache
> > > > > sequential
> > > > > support is an optional command. This means that we not only
> > > > > need to
> > > > > check whether the individual controller supports the command,
> > > > > we also
> > > > > need to check the parameter pages for both ONFI and JEDEC NAND
> > > > > flashes
> > > > > before enabling sequential cache reads.
> > > > > 
> > > > > This fixes support for NAND flashes which don't support
> > > > > enabling
> > > > > cache
> > > > > reads, i.e. Samsung K9F4G08U0F or Toshiba TC58NVG0S3HTA00.
> > > > > 
> > > > > Sequential cache reads are now only available for ONFI and
> > > > > JEDEC
> > > > > devices, if individual vendors implement this, it needs to be
> > > > > enabled
> > > > > per vendor.
> > > > > 
> > > > > Tested on i.MX6Q with a Samsung NAND flash chip that doesn't
> > > > > support
> > > > > sequential reads.
> > > > > 
> > > > > Fixes: 003fe4b9545b ("mtd: rawnand: Support for sequential
> > > > > cache
> > > > > reads")
> > > > > Cc: stable@vger.kernel.org
> > > > > Signed-off-by: Rouven Czerwinski
> > > > > <r.czerwinski@pengutronix.de>      
> > > > 
> > > > Thanks for this. It works as expected for my Toshiba chip,
> > > > obviously
> > > > because it doesn't use ONFI or JEDEC.
> > > > 
> > > > Unfortunately, my Micron chip does use ONFI, and it sets the
> > > > cached-
> > > > read-supported bit. It then fails when reading afterwords:  
> > 
> > I might have over reacted regarding my findings in Micron's
> > datasheet,
> > I need to know if you use the on-die ECC engine or if you use the one
> > on the controller. In the former case the failure is expected. In the
> > latter case, it's not.  
> 
> I use the default, which seems to be the controller engine?

Yeah, you're using the gpmi NAND controller right? If that's the case,
it seems that only ECC correction is supported.

Thanks,
Miquèl
  

Micron Confidential

Hello Miquel,

Here is a short list of devices having cache read with ECC enabled.

MT29F2G08ABAGAH4, MT29F2G08ABBGAH4, MT29F2G16ABBGAH4
MT29F1G08ABAFAH4, MT29F1G08ABBFAH4, MT29F1G16ABBFAH4

Thanks.
Regards,
Domenico P.




Micron Confidential
-----Original Message-----
From: Martin Hundebøll <martin@geanix.com>
Sent: Thursday, September 28, 2023 9:20 AM
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>; Måns Rullgård <mans@mansr.com>; Alexander Shiyan <eagle.alexander923@gmail.com>; Richard Weinberger <richard@nod.at>; Vignesh Raghavendra <vigneshr@ti.com>; JaimeLiao <jaimeliao.tw@gmail.com>; kernel@pengutronix.de; stable@vger.kernel.org; linux-mtd@lists.infradead.org; linux-kernel@vger.kernel.org; Sean Nyekjær <sean@geanix.com>; Domenico Punzo <dpunzo@micron.com>; Bean Huo <beanhuo@micron.com>
Subject: [EXT] Re: [PATCH v2] mtd: rawnand: Ensure the nand chip supports cached reads

CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless you recognize the sender and were expecting this message.


Hi Miquel,

On Wed, 2023-09-27 at 17:05 +0200, Miquel Raynal wrote:
> Hi Martin,
>
> miquel.raynal@bootlin.com wrote on Tue, 26 Sep 2023 13:27:25 +0200:
>
> > Hi Martin,
> >
> > + Bean and Domenico, there is a question for you below.
> >
> > martin@geanix.com wrote on Mon, 25 Sep 2023 13:01:06 +0200:
> >
> > > Hi Rouven,
> > >
> > > On Fri, 2023-09-22 at 16:17 +0200, Rouven Czerwinski wrote:
> > > > Both the JEDEC and ONFI specification say that read cache
> > > > sequential support is an optional command. This means that we
> > > > not only need to check whether the individual controller
> > > > supports the command, we also need to check the parameter pages
> > > > for both ONFI and JEDEC NAND flashes before enabling sequential
> > > > cache reads.
> > > >
> > > > This fixes support for NAND flashes which don't support enabling
> > > > cache reads, i.e. Samsung K9F4G08U0F or Toshiba TC58NVG0S3HTA00.
> > > >
> > > > Sequential cache reads are now only available for ONFI and JEDEC
> > > > devices, if individual vendors implement this, it needs to be
> > > > enabled per vendor.
> > > >
> > > > Tested on i.MX6Q with a Samsung NAND flash chip that doesn't
> > > > support sequential reads.
> > > >
> > > > Fixes: 003fe4b9545b ("mtd: rawnand: Support for sequential cache
> > > > reads")
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > >
> > > Thanks for this. It works as expected for my Toshiba chip,
> > > obviously because it doesn't use ONFI or JEDEC.
> > >
> > > Unfortunately, my Micron chip does use ONFI, and it sets the
> > > cached-
> > > read-supported bit. It then fails when reading afterwords:
>
> I might have over reacted regarding my findings in Micron's datasheet,
> I need to know if you use the on-die ECC engine or if you use the one
> on the controller. In the former case the failure is expected. In the
> latter case, it's not.

I use the default, which seems to be the controller engine?

// Martin

> Thanks,
> Miquèl
>
> > > kernel: ONFI_OPT_CMD_READ_CACHE # debug added by me
> > > kernel: nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xdc
> > > kernel: nand: Micron MT29F4G08ABAFAWP
> > > kernel: nand: 512 MiB, SLC, erase size: 256 KiB, page size: 4096,
> > > OOB
> > > size: 256
> > > kernel: nand: continued read supported # debug added by me
> > > kernel: Bad block table found at page 131008, version 0x01
> > > kernel: Bad block table found at page 130944, version 0x01
> > > kernel: 2 fixed-partitions partitions found on MTD device gpmi-
> > > nand
> > > kernel: Creating 2 MTD partitions on "gpmi-nand":
> > > kernel: 0x000000000000-0x000000800000 : "boot"
> > > kernel: 0x000000800000-0x000020000000 : "ubi"
> > > kernel: gpmi-nand 1806000.nand-controller: driver registered.
> > >
> > > ...
> > >
> > > kernel: ubi0: default fastmap pool size: 100
> > > kernel: ubi0: default fastmap WL pool size: 50
> > > kernel: ubi0: attaching mtd1
> > > kernel: ubi0: scanning is finished
> > > kernel: ubi0: attached mtd1 (name "ubi", size 504 MiB)
> > > kernel: ubi0: PEB size: 262144 bytes (256 KiB), LEB size: 253952
> > > bytes
> > > kernel: ubi0: min./max. I/O unit sizes: 4096/4096, sub-page size
> > > 4096
> > > kernel: ubi0: VID header offset: 4096 (aligned 4096), data
> > > offset: 8192
> > > kernel: ubi0: good PEBs: 2012, bad PEBs: 4, corrupted PEBs: 0
> > > kernel: ubi0: user volume: 9, internal volumes: 1, max. volumes
> > > count:
> > > 128
> > > kernel: ubi0: max/mean erase counter: 4/2, WL threshold: 4096,
> > > image sequence number: 1431497221
> > > kernel: ubi0: available PEBs: 12, total reserved PEBs: 2000, PEBs
> > > reserved for bad PEB handling: 36
> > > kernel: block ubiblock0_4: created from ubi0:4(rootfs.a)
> > > kernel: ubi0: background thread "ubi_bgt0d" started, PID 36
> > > kernel: block ubiblock0_6: created from ubi0:6(appfs.a)
> > > kernel: block ubiblock0_7: created from ubi0:7(appfs.b)
> > >
> > > ...
> > >
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6d15c:ed1]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6f15e:125]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6d15c:1dae]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6d15c:ed1]
> > > (d-sysctl)[55]: systemd-sysctl.service: Failed to set up
> > > credentials:
> > > Protocol error
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b73162:14f0]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6f15e:838]
> > > systemd[1]: Starting Create Static Device Nodes in /dev...
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6d15c:ed1]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6d15c:ed1]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6f15e:838]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6d15c:1dae]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6f15e:125]
> > >
> > > I've briefly tried adding some error info the the squashfs error
> > > messages, but it looks like it's getting bad data. I.e. one
> > > failure a sanity check of `dir_count`:
> > >
> > > if (dir_count > SQUASHFS_DIR_COUNT)
> > >         goto data_error;
> > >
> > > It fails with `dir_count` being 1952803684 ...
> > >
> > > So is this a case of wrong/bad timings?
> > >
> > > Miquel:
> > > I can tell from the code, that the READCACHESEQ operations are
> > > followed by NAND_OP_WAIT_RDY(tR_max, tRR_min). From the Micron
> > > datasheet[0], it should be NAND_OP_WAIT_RDY(tRCBSY_max, tRR_min),
> > > where tRCBSY is defined to be between 3 and 25 µs.
> >
> > I found a place in the ONFI spec states taht tRCBSY_max should be
> > between 3 and tR_max, so indeed we should be fine on that regard.
> >
> > However, I asked myself whether we could have issues when crossing
> > boundaries. Block boundaries should be fine, however your device
> > does not support crossing plane boundaries, as bit 4 ("read cache
> > supported") of byte 114 ("Multi-plane operation attributes") in the
> > memory organization block of the parameter page is not set (the
> > value of the byte should be 0x0E if I get it right.
> >
> > Anyway, our main issue here does not seem related to the boundaries.
> > It does not seem to be explicitly marked anywhere else but on the
> > front
> > page:
> >         Advanced command set
> >         – Program page cache mode (4)
> >         – Read page cache mode (4)
> >         – Two-plane commands (4)
> >
> >         (4) These commands supported only with ECC disabled.
> >
> > Read page cache mode without ECC makes the feature pretty useless
> > IMHO.
> >
> > Bean, Domenico, how do we know which devices allow ECC correction
> > during sequential page reads and which don't? Is there a (vendor?)
> > bit somewhere in the parameter page for that? Do we have any way to
> > know besides a list of devices allowing that? If so, can you provide
> > one with a few IDs?
> >
> > Thanks,
> > Miquèl
  

Hi Domenico,

dpunzo@micron.com wrote on Tue, 3 Oct 2023 11:29:33 +0000:

> Micron Confidential
> 
> Hello Miquel,
> 
> Here is a short list of devices having cache read with ECC enabled.
> 
> MT29F2G08ABAGAH4, MT29F2G08ABBGAH4, MT29F2G16ABBGAH4
> MT29F1G08ABAFAH4, MT29F1G08ABBFAH4, MT29F1G16ABBFAH4

Great! Thanks a lot.

It appears that even without internal ECC enabled we get failures on
Micron parts, so I believe there is something else that we need to
investigate. But thanks for the list, I will propose a patch preventing
the use of on-die ECC together with sequential cache reads on all parts
but the ones in this list (can be extended later).

Thanks,
Miquèl
  

On Fri, 2023-09-22 at 14:17:16 UTC, Rouven Czerwinski wrote:
> Both the JEDEC and ONFI specification say that read cache sequential
> support is an optional command. This means that we not only need to
> check whether the individual controller supports the command, we also
> need to check the parameter pages for both ONFI and JEDEC NAND flashes
> before enabling sequential cache reads.
> 
> This fixes support for NAND flashes which don't support enabling cache
> reads, i.e. Samsung K9F4G08U0F or Toshiba TC58NVG0S3HTA00.
> 
> Sequential cache reads are now only available for ONFI and JEDEC
> devices, if individual vendors implement this, it needs to be enabled
> per vendor.
> 
> Tested on i.MX6Q with a Samsung NAND flash chip that doesn't support
> sequential reads.
> 
> Fixes: 003fe4b9545b ("mtd: rawnand: Support for sequential cache reads")
> Cc: stable@vger.kernel.org
> Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/fixes, thanks.

Miquel