* [RFC PATCH] mtd: spi-nor-core: Handle SPI_RX_SLOW in spi_nor_adjust_hwcaps()
@ 2021-07-28 15:56 Bin Meng
2021-07-28 17:42 ` Pratyush Yadav
0 siblings, 1 reply; 3+ messages in thread
From: Bin Meng @ 2021-07-28 15:56 UTC (permalink / raw
To: Jagan Teki, Vignesh Raghavendra, Pratyush Yadav, u-boot
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.cn@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);
+
rdidx = spi_nor_hwcaps_read2cmd(BIT(cap));
if (rdidx >= 0 &&
spi_nor_check_readop(nor, ¶ms->reads[rdidx]))
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC PATCH] mtd: spi-nor-core: Handle SPI_RX_SLOW in spi_nor_adjust_hwcaps()
2021-07-28 15:56 [RFC PATCH] mtd: spi-nor-core: Handle SPI_RX_SLOW in spi_nor_adjust_hwcaps() Bin Meng
@ 2021-07-28 17:42 ` Pratyush Yadav
2021-07-29 9:17 ` Bin Meng
0 siblings, 1 reply; 3+ messages in thread
From: Pratyush Yadav @ 2021-07-28 17:42 UTC (permalink / raw
To: Bin Meng; +Cc: Jagan Teki, Vignesh Raghavendra, u-boot
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.cn@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?
> rdidx = spi_nor_hwcaps_read2cmd(BIT(cap));
> if (rdidx >= 0 &&
> spi_nor_check_readop(nor, ¶ms->reads[rdidx]))
> --
> 2.25.1
>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC PATCH] mtd: spi-nor-core: Handle SPI_RX_SLOW in spi_nor_adjust_hwcaps()
2021-07-28 17:42 ` Pratyush Yadav
@ 2021-07-29 9:17 ` Bin Meng
0 siblings, 0 replies; 3+ messages in thread
From: Bin Meng @ 2021-07-29 9:17 UTC (permalink / raw
To: Pratyush Yadav; +Cc: Jagan Teki, Vignesh Raghavendra, U-Boot Mailing List
On Thu, Jul 29, 2021 at 1:42 AM Pratyush Yadav <p.yadav@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.cn@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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-07-29 9:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-07-28 15:56 [RFC PATCH] mtd: spi-nor-core: Handle SPI_RX_SLOW in spi_nor_adjust_hwcaps() Bin Meng
2021-07-28 17:42 ` Pratyush Yadav
2021-07-29 9:17 ` Bin Meng
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.