On Thu, Jul 29, 2021 at 1:42 AM Pratyush Yadav <p.ya...@ti.com> wrote: > > On 28/07/21 11:56PM, Bin Meng wrote: > > When CONFIG_SPI_FLASH_SMART_HWCAPS is on, SPI_RX_SLOW flag of the > > SPI controller is not honored. This adds the missing logic there. > > > > With this patch, SPI flash read works again with ICH SPI controller > > on Intel Crown Bay board. > > > > Signed-off-by: Bin Meng <bmeng...@gmail.com> > > > > --- > > The quirky stuff of ICH SPI controller is that it only supports normal > > read (opcode 3) but not fast read (opcode 11), so that's why SPI_RX_SLOW > > was introduced before. > > > > At present the ICH SPI driver does not implement the supports_op() hook. > > I can add a check against opcode 11 there, however I see a comment in > > spi_nor_check_op() saying that SPI controller implementation should not > > check the opcode. > > > > /* > > * First test with 4 address bytes. The opcode itself might be a 3B > > * addressing opcode but we don't care, because SPI controller > > * implementation should not check the opcode, but just the sequence. > > */ > > > > drivers/mtd/spi/spi-nor-core.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c > > index 99e2f16349..e9b46c39b5 100644 > > --- a/drivers/mtd/spi/spi-nor-core.c > > +++ b/drivers/mtd/spi/spi-nor-core.c > > @@ -2855,6 +2855,7 @@ spi_nor_adjust_hwcaps(struct spi_nor *nor, > > const struct spi_nor_flash_parameter *params, > > u32 *hwcaps) > > { > > + struct spi_slave *spi = nor->spi; > > unsigned int cap; > > > > /* > > @@ -2879,6 +2880,10 @@ spi_nor_adjust_hwcaps(struct spi_nor *nor, > > if (!(*hwcaps & BIT(cap))) > > continue; > > > > + if ((spi->mode & SPI_RX_SLOW) && > > + (BIT(cap) == SNOR_HWCAPS_READ_FAST)) > > + *hwcaps &= ~BIT(cap); > > + > > NACK. > > We already check for SPI_RX_SLOW in spi_nor_scan(): > > /* Some devices cannot do fast-read, no matter what DT tells us */ > if ((info->flags & SPI_NOR_NO_FR) || (spi->mode & SPI_RX_SLOW)) > params.hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST; > > I think the real bug here is that the smart spi_nor_adjust_hwcaps() does > not respect the flash's hwcaps, and only looks to the controller on what > can be supported. Doing the below should fix this: > > diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c > index 8dd44c0f1e..4323b49651 100644 > --- a/drivers/mtd/spi/spi-nor-core.c > +++ b/drivers/mtd/spi/spi-nor-core.c > @@ -2741,7 +2741,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; > > Can you please test and confirm if it does indeed fix the issue for you?
Yes, this also fixed the issue. In fact, we should update spi-nor to honor the DT property "m25p,fast-read" instead of the SPI_RX_SLOW flag which was set by the ICH SPI driver. I will prepare patches soon. > > > rdidx = spi_nor_hwcaps_read2cmd(BIT(cap)); > > if (rdidx >= 0 && > > spi_nor_check_readop(nor, ¶ms->reads[rdidx])) Regards, Bin