Re: [PATCH v2 2/2] mtd: spi-nor: Mask out fast read if not requested in DT

[Pratyush Yadav]

On 30/07/21 03:20PM, Bin Meng wrote:
> The DT bindings of "jedec,spi-nor" [1] defines "m25p,fast-read" property
> to indicate that "fast read" opcode can be used to read data from the
> chip instead of the usual "read" opcode.
> 
> If this property is not present in DT, mask out fast read in
> spi_nor_init_params(). This change mirrors the same logic in
> spi_nor_info_init_params() in drivers/mtd/spi-nor/core.c in
> the Linux kernel v5.14-rc3.
> 
> [1] Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml in the kernel 
> tree
> 
> Signed-off-by: Bin Meng <bmeng...@gmail.com>
> 
> ---
> 
> Changes in v2:
> - Guard changes with CONFIG_IS_ENABLED(DM_SPI)

Hehe, I just commented about this on your v1, and I see the v2 in my 
inbox already ;-)

So this will make sure that dev_ofnode() will be valid, right?

> 
>  drivers/mtd/spi/spi-nor-core.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index c8c3bdd890..d5d905fa5a 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -2604,18 +2604,28 @@ static int spi_nor_init_params(struct spi_nor *nor,
>       params->size = info->sector_size * info->n_sectors;
>       params->page_size = info->page_size;
>  
> +     if (!(info->flags & SPI_NOR_NO_FR)) {
> +             /* Default to Fast Read for DT and non-DT platform devices. */
> +             params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
> +
> +             /* Mask out Fast Read if not requested at DT instantiation. */
> +#if CONFIG_IS_ENABLED(DM_SPI)
> +             if (!ofnode_read_bool(dev_ofnode(nor->spi->dev),

Let's avoid the #if here and do

  if (CONFIG_IS_ENABLED(DM_SPI) && !ofnode_read_bool())

> +                                   "m25p,fast-read"))
> +                     params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
> +#endif
> +     }
> +
>       /* (Fast) Read settings. */
>       params->hwcaps.mask |= SNOR_HWCAPS_READ;
>       spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ],
>                                 0, 0, SPINOR_OP_READ,
>                                 SNOR_PROTO_1_1_1);
>  
> -     if (!(info->flags & SPI_NOR_NO_FR)) {
> -             params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
> +     if (params->hwcaps.mask & SNOR_HWCAPS_READ_FAST)
>               spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_FAST],
>                                         0, 8, SPINOR_OP_READ_FAST,
>                                         SNOR_PROTO_1_1_1);
> -     }
>  
>       if (info->flags & SPI_NOR_DUAL_READ) {
>               params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.