Re: [PATCH v14 05/15] mtd: spi-nor: sfdp: parse xSPI Profile 1.0 table

[<Tudor.Ambarus@microchip.com>]

On 9/30/20 9:57 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> This table is indication that the flash is xSPI compliant and hence
> supports octal DTR mode. Extract information like the fast read opcode,
> dummy cycles, the number of dummy cycles needed for a Read Status
> Register command, and the number of address bytes needed for a Read
> Status Register command.
> 
> We don't know what speed the controller is running at. Find the fast
> read dummy cycles for the fastest frequency the flash can run at to be
> sure we are never short of dummy cycles. If nothing is available,
> default to 20. Flashes that use a different value should update it in
> their fixup hooks.
> 
> Since we want to set read settings, expose spi_nor_set_read_settings()
> in core.h.
> 
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> ---
>  drivers/mtd/spi-nor/core.c |  2 +-
>  drivers/mtd/spi-nor/core.h | 10 +++++
>  drivers/mtd/spi-nor/sfdp.c | 91 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 102 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index beb17639dc92..97e82a4821f6 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2332,7 +2332,7 @@ static int spi_nor_check(struct spi_nor *nor)
>         return 0;
>  }
> 
> -static void
> +void
>  spi_nor_set_read_settings(struct spi_nor_read_command *read,
>                           u8 num_mode_clocks,
>                           u8 num_wait_states,
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 5d95b4183a33..9a33c8d07335 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -192,6 +192,9 @@ struct spi_nor_locking_ops {
>   *
>   * @size:              the flash memory density in bytes.
>   * @page_size:         the page size of the SPI NOR flash memory.
> + * @rdsr_dummy:                dummy cycles needed for Read Status Register command.
> + * @rdsr_addr_nbytes:  dummy address bytes needed for Read Status Register
> + *                     command.
>   * @hwcaps:            describes the read and page program hardware
>   *                     capabilities.
>   * @reads:             read capabilities ordered by priority: the higher index
> @@ -214,6 +217,8 @@ struct spi_nor_locking_ops {
>  struct spi_nor_flash_parameter {
>         u64                             size;
>         u32                             page_size;
> +       u8                              rdsr_dummy;
> +       u8                              rdsr_addr_nbytes;
> 
>         struct spi_nor_hwcaps           hwcaps;
>         struct spi_nor_read_command     reads[SNOR_CMD_READ_MAX];
> @@ -425,6 +430,11 @@ ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
> 
>  int spi_nor_hwcaps_read2cmd(u32 hwcaps);
>  u8 spi_nor_convert_3to4_read(u8 opcode);
> +void spi_nor_set_read_settings(struct spi_nor_read_command *read,
> +                              u8 num_mode_clocks,
> +                              u8 num_wait_states,
> +                              u8 opcode,
> +                              enum spi_nor_protocol proto);
>  void spi_nor_set_pp_settings(struct spi_nor_pp_command *pp, u8 opcode,
>                              enum spi_nor_protocol proto);
> 
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index c77655968f80..ebc1188f7533 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -4,6 +4,7 @@
>   * Copyright (C) 2014, Freescale Semiconductor, Inc.
>   */
> 
> +#include <linux/bitfield.h>
>  #include <linux/slab.h>
>  #include <linux/sort.h>
>  #include <linux/mtd/spi-nor.h>
> @@ -19,6 +20,7 @@
>  #define SFDP_BFPT_ID           0xff00  /* Basic Flash Parameter Table */
>  #define SFDP_SECTOR_MAP_ID     0xff81  /* Sector Map Table */
>  #define SFDP_4BAIT_ID          0xff84  /* 4-byte Address Instruction Table */
> +#define SFDP_PROFILE1_ID       0xff05  /* xSPI Profile 1.0 table. */
> 
>  #define SFDP_SIGNATURE         0x50444653U
> 
> @@ -1108,6 +1110,91 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
>         return ret;
>  }
> 
> +#define PROFILE1_DWORD1_RDSR_ADDR_BYTES                BIT(29)
> +#define PROFILE1_DWORD1_RDSR_DUMMY             BIT(28)
> +#define PROFILE1_DWORD1_RD_FAST_CMD            GENMASK(15, 8)
> +#define PROFILE1_DWORD4_DUMMY_200MHZ           GENMASK(11, 7)
> +#define PROFILE1_DWORD5_DUMMY_166MHZ           GENMASK(31, 27)
> +#define PROFILE1_DWORD5_DUMMY_133MHZ           GENMASK(21, 17)
> +#define PROFILE1_DWORD5_DUMMY_100MHZ           GENMASK(11, 7)
> +
> +/**
> + * spi_nor_parse_profile1() - parse the xSPI Profile 1.0 table
> + * @nor:               pointer to a 'struct spi_nor'
> + * @profile1_header:   pointer to the 'struct sfdp_parameter_header' describing
> + *                     the Profile 1.0 Table length and version.
> + * @params:            pointer to the 'struct spi_nor_flash_parameter' to be.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spi_nor_parse_profile1(struct spi_nor *nor,
> +                                 const struct sfdp_parameter_header *profile1_header,
> +                                 struct spi_nor_flash_parameter *params)
> +{
> +       u32 *dwords, addr;
> +       size_t len;
> +       int ret;
> +       u8 dummy = 0, opcode;

explicit init to 0 not needed. With this addressed, one can add:

Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>

> +
> +       len = profile1_header->length * sizeof(*dwords);
> +       dwords = kmalloc(len, GFP_KERNEL);
> +       if (!dwords)
> +               return -ENOMEM;
> +
> +       addr = SFDP_PARAM_HEADER_PTP(profile1_header);
> +       ret = spi_nor_read_sfdp(nor, addr, len, dwords);
> +       if (ret)
> +               goto out;
> +
> +       le32_to_cpu_array(dwords, profile1_header->length);
> +
> +       /* Get 8D-8D-8D fast read opcode and dummy cycles. */
> +       opcode = FIELD_GET(PROFILE1_DWORD1_RD_FAST_CMD, dwords[0]);
> +
> +        /* Set the Read Status Register dummy cycles and dummy address bytes. */
> +       if (dwords[0] & PROFILE1_DWORD1_RDSR_DUMMY)
> +               params->rdsr_dummy = 8;
> +       else
> +               params->rdsr_dummy = 4;
> +
> +       if (dwords[0] & PROFILE1_DWORD1_RDSR_ADDR_BYTES)
> +               params->rdsr_addr_nbytes = 4;
> +       else
> +               params->rdsr_addr_nbytes = 0;
> +
> +       /*
> +        * We don't know what speed the controller is running at. Find the
> +        * dummy cycles for the fastest frequency the flash can run at to be
> +        * sure we are never short of dummy cycles. A value of 0 means the
> +        * frequency is not supported.
> +        *
> +        * Default to PROFILE1_DUMMY_DEFAULT if we don't find anything, and let
> +        * flashes set the correct value if needed in their fixup hooks.
> +        */
> +       dummy = FIELD_GET(PROFILE1_DWORD4_DUMMY_200MHZ, dwords[3]);
> +       if (!dummy)
> +               dummy = FIELD_GET(PROFILE1_DWORD5_DUMMY_166MHZ, dwords[4]);
> +       if (!dummy)
> +               dummy = FIELD_GET(PROFILE1_DWORD5_DUMMY_133MHZ, dwords[4]);
> +       if (!dummy)
> +               dummy = FIELD_GET(PROFILE1_DWORD5_DUMMY_100MHZ, dwords[4]);
> +       if (!dummy)
> +               dev_dbg(nor->dev,
> +                       "Can't find dummy cycles from Profile 1.0 table\n");
> +
> +       /* Round up to an even value to avoid tripping controllers up. */
> +       dummy = round_up(dummy, 2);
> +
> +       /* Update the fast read settings. */
> +       spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_8_8_8_DTR],
> +                                 0, dummy, opcode,
> +                                 SNOR_PROTO_8_8_8_DTR);
> +
> +out:
> +       kfree(dwords);
> +       return ret;
> +}
> +
>  /**
>   * spi_nor_parse_sfdp() - parse the Serial Flash Discoverable Parameters.
>   * @nor:               pointer to a 'struct spi_nor'
> @@ -1209,6 +1296,10 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
>                         err = spi_nor_parse_4bait(nor, param_header, params);
>                         break;
> 
> +               case SFDP_PROFILE1_ID:
> +                       err = spi_nor_parse_profile1(nor, param_header, params);
> +                       break;
> +
>                 default:
>                         break;
>                 }
> --
> 2.28.0
>