| Message ID | 20210729115827.603282-1-bmeng.cn@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | [1/2] mtd: spi-nor: Respect flash's hwcaps in spi_nor_adjust_hwcaps() | expand |
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 >
Hi Pratyush, On Thu, Jul 29, 2021 at 8:08 PM Pratyush Yadav <p.yadav@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@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 for the review! Will update comments in v2. Regards, Bin
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(). */ - *hwcaps = SNOR_HWCAPS_ALL; + *hwcaps = SNOR_HWCAPS_ALL & params->hwcaps.mask; /* X-X-X modes are not supported yet, mask them all. */ *hwcaps &= ~SNOR_HWCAPS_X_X_X;
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(-)