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

[Pratyush Yadav]

Hi Bin,

On 29/07/21 07:58PM, 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.cn at gmail.com>
> ---
> 
>  drivers/mtd/spi/spi-nor-core.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index 2883d092fc..d6a0ed39c5 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -2597,6 +2597,8 @@ static int spi_nor_init_params(struct spi_nor *nor,
>  			       const struct flash_info *info,
>  			       struct spi_nor_flash_parameter *params)
>  {
> +	struct spi_slave *spi = nor->spi;
> +
>  	/* Set legacy flash parameters as default. */
>  	memset(params, 0, sizeof(*params));
>  
> @@ -2604,18 +2606,25 @@ 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 (!ofnode_read_bool(dev_ofnode(spi->dev), "m25p,fast-read"))

Do you need to check if dev_ofnode() is valid? Can spi-nor be 
instantiated from a non-DT platform (like platdata maybe)? I am not very 
familiar with DT/platdata stuff, other than the basics.

> +			params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
> +	}
> +
>  	/* (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);
> -	}

The patch looks good to me other than the point above.

>  
>  	if (info->flags & SPI_NOR_DUAL_READ) {
>  		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;
> -- 
> 2.25.1
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.