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

[Bin Meng]

Hi Pratyush,

On Thu, Jul 29, 2021 at 8:08 PM Pratyush Yadav <p.yadav at ti.com> wrote:
>
> 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 at 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 at ti.com>
>

Thanks for the review! Will update comments in v2.

Regards,
Bin