Re: [PATCH v9 07/19] mtd: spi-nor: sfdp: prepare BFPT parsing for JESD216 rev D

[Tudor.Ambarus]

On Monday, May 25, 2020 12:15:32 PM EEST Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> JESD216 rev D makes BFPT 20 DWORDs. Update the BFPT size define to
> reflect that.
> 
> The check for rev A or later compared the BFPT header length with the
> maximum BFPT length, BFPT_DWORD_MAX. Since BFPT_DWORD_MAX was 16, and so
> was the BFPT length for both rev A and B, this check worked fine. But
> now, since BFPT_DWORD_MAX is 20, it means this check will also stop BFPT
> parsing for rev A or B, since their length is 16.
> 
> So, instead check for BFPT_DWORD_MAX_JESD216 to stop BFPT parsing for
> the first JESD216 version, and check for BFPT_DWORD_MAX_JESD216B for the
> next two versions.
> 
> Signed-off-by: Pratyush Yadav <p.yadav@xxxxxx>
> ---
>  drivers/mtd/spi-nor/sfdp.c | 7 ++++++-
>  drivers/mtd/spi-nor/sfdp.h | 5 +++--
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index 5cecc4ba2141..96960f2f3d7a 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -549,7 +549,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>                                   SNOR_ERASE_TYPE_MASK;
> 
>         /* Stop here if not JESD216 rev A or later. */
> -       if (bfpt_header->length < BFPT_DWORD_MAX)
> +       if (bfpt_header->length == BFPT_DWORD_MAX_JESD216)
>                 return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt,
>                                                 params);
> 
> @@ -605,6 +605,11 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>                 return -EINVAL;
>         }
> 
> +       /* Stop here if JESD216 rev B. */

Applies for both rev A and rev B. Used the following comment instead:
/* Stop here if not JESD216 rev C or later. */

> +       if (bfpt_header->length == BFPT_DWORD_MAX_JESD216B)
> +               return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt,
> +                                               params);
> +
>         return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt, params);
>  }
> 
> diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
> index e0a8ded04890..f8198af43a63 100644
> --- a/drivers/mtd/spi-nor/sfdp.h
> +++ b/drivers/mtd/spi-nor/sfdp.h
> @@ -10,11 +10,11 @@
>  /* Basic Flash Parameter Table */
> 
>  /*
> - * JESD216 rev B defines a Basic Flash Parameter Table of 16 DWORDs.
> + * JESD216 rev D defines a Basic Flash Parameter Table of 20 DWORDs.
>   * They are indexed from 1 but C arrays are indexed from 0.
>   */
>  #define BFPT_DWORD(i)          ((i) - 1)
> -#define BFPT_DWORD_MAX         16
> +#define BFPT_DWORD_MAX         20
> 
>  struct sfdp_bfpt {
>         u32     dwords[BFPT_DWORD_MAX];
> @@ -22,6 +22,7 @@ struct sfdp_bfpt {
> 
>  /* The first version of JESD216 defined only 9 DWORDs. */
>  #define BFPT_DWORD_MAX_JESD216                 9
> +#define BFPT_DWORD_MAX_JESD216B                        16
> 
>  /* 1st DWORD. */
>  #define BFPT_DWORD1_FAST_READ_1_1_2            BIT(16)
> --
> 2.26.2

Applied, thanks.