Re: [PATCH 1/2] mtd: spi-nor: Respect flash's hwcaps in spi_nor_adjust_hwcaps()

[Pratyush Yadav <p.yadav@ti.com>]

Hi Bin,

On 29/07/21 07:58PM, Bin Meng wrote:
> The smart spi_nor_adjust_hwcaps() does not respect the SPI flash's
> hwcaps, and only looks to the controller on what can be supported.
> 
> The flash's hwcaps needs to be AND'ed before checking.
> 
> Fixes: 71025f013ccb ("mtd: spi-nor-core: Rework hwcaps selection")
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>  drivers/mtd/spi/spi-nor-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index 99e2f16349..2883d092fc 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -2861,7 +2861,7 @@ spi_nor_adjust_hwcaps(struct spi_nor *nor,
>  	 * Enable all caps by default. We will mask them after checking what's
>  	 * really supported using spi_mem_supports_op().
>  	 */

Please update the comment above. Maybe:

  Start by assuming the controller supports every cap. We will mask 
  them...

> -	*hwcaps = SNOR_HWCAPS_ALL;
> +	*hwcaps = SNOR_HWCAPS_ALL & params->hwcaps.mask;

This is practically equivalent to *hwcaps = params->hwcaps.mask but I 
think having SNOR_HWCAPS_ALL here to signify that we are assuming 
controller supports everything is not a bad idea IMO.

With the above comment updated,

Reviewed-by: Pratyush Yadav <p.yadav@ti.com>

Thanks.

>  
>  	/* X-X-X modes are not supported yet, mask them all. */
>  	*hwcaps &= ~SNOR_HWCAPS_X_X_X;
> -- 
> 2.25.1
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.