[U-Boot] QSPI support on i.MX7D Sabre Eval Board

Hi all,
I've built current u-boot (v2019.07-rc4) with QSPI support for our i.MX7D Sabre eval system. I am using mx7dsabresd_qspi_defconfig.
The resulting u-boot fails to read from QSPI, i.e. the data read is not the same than that written before with a previous (v2018) version.
After some investigation I found that the 'is_controller_busy' function comes back with -ETIMEDOUT during read operation, because the retry count of 5 is too low.
To figure out how many retries are needed, I added some debug code as part of a while(1) loop to the is_controller_busy function:
diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c index 1598c4f698..fa5b7a29f2 100644 --- a/drivers/spi/fsl_qspi.c +++ b/drivers/spi/fsl_qspi.c @@ -152,16 +152,20 @@ static inline int is_controller_busy(const struct fsl_qspi_priv *priv) u32 val; const u32 mask = QSPI_SR_BUSY_MASK | QSPI_SR_AHB_ACC_MASK | QSPI_SR_IP_ACC_MASK; - unsigned int retry = 5; + unsigned int retry = 0;
do { val = qspi_read32(priv->flags, &priv->regs->sr);
- if ((~val & mask) == mask) + if ((~val & mask) == mask) { + if (retry > 5) + printf("%s: timeout! retry = %d\n", __func__, retry); return 0; + }
udelay(1); - } while (--retry); + retry++; + } while (1);
return -ETIMEDOUT; }
On my MX7 eval system I found that a retry count between 73 and 147 is necessary to make sure that the controller will be ready before the next operation.
So from my point of view we have 2 options to fix this issue:
- increase retry from 5 to 200 - wait in an endless loop (while (1) ) and return only when the controller is ready (which means that the board will hang if the controller doesn't come back for some other reason.
What do you think?
Best regards, Thomas Schäfer SW Design Engineer
Kontron - An S&T Company Heinrich-Barth-Straße 1-1a | 66115 Saarbrücken | Germany
thomas.schaefer@kontron.com
http://www.kontron.com Website | Blog | Twitter | LinkedIn | YouTube | Facebook
Kontron Europe GmbH Die gesetzlichen Pflichtangaben finden Sie hier. Please find our mandatory legal statements here. Mit dem Öffnen dieses E-Mails stimmen Sie Kontrons Richtlinien zur elektronischen Kommunikation zu. By opening this email you are agreeing to Kontron's Electronic Communications Policy.

Hi Thomas,
On Wed, Jun 19, 2019 at 10:04 AM Thomas Schaefer Thomas.Schaefer@kontron.com wrote:
Hi all,
I've built current u-boot (v2019.07-rc4) with QSPI support for our i.MX7D Sabre eval system. I am using mx7dsabresd_qspi_defconfig.
The resulting u-boot fails to read from QSPI, i.e. the data read is not the same than that written before with a previous (v2018) version.
After some investigation I found that the 'is_controller_busy' function comes back with -ETIMEDOUT during read operation, because the retry count of 5 is too low.
To figure out how many retries are needed, I added some debug code as part of a while(1) loop to the is_controller_busy function:
diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c index 1598c4f698..fa5b7a29f2 100644 --- a/drivers/spi/fsl_qspi.c +++ b/drivers/spi/fsl_qspi.c @@ -152,16 +152,20 @@ static inline int is_controller_busy(const struct fsl_qspi_priv *priv) u32 val; const u32 mask = QSPI_SR_BUSY_MASK | QSPI_SR_AHB_ACC_MASK | QSPI_SR_IP_ACC_MASK;
unsigned int retry = 5;
unsigned int retry = 0; do { val = qspi_read32(priv->flags, &priv->regs->sr);
if ((~val & mask) == mask)
if ((~val & mask) == mask) {
if (retry > 5)
printf("%s: timeout! retry = %d\n", __func__, retry); return 0;
} udelay(1);
} while (--retry);
retry++;
} while (1); return -ETIMEDOUT;
}
On my MX7 eval system I found that a retry count between 73 and 147 is necessary to make sure that the controller will be ready before the next operation.
So from my point of view we have 2 options to fix this issue:
- increase retry from 5 to 200
- wait in an endless loop (while (1) ) and return only when the controller is ready (which means that the board will hang if the controller doesn't come back for some other reason.
What do you think?
I think that readl_poll_timeout() could be used here as it will timeout if the QSPI controller is not ready.
Also, could you check how this is handled in the fsl qspi driver in the kernel?

Hi Fabio,
Hi Thomas,
On Wed, Jun 19, 2019 at 10:04 AM Thomas Schaefer Thomas.Schaefer@kontron.com wrote:
Hi all,
I've built current u-boot (v2019.07-rc4) with QSPI support for our i.MX7D Sabre eval system. I am using mx7dsabresd_qspi_defconfig.
The resulting u-boot fails to read from QSPI, i.e. the data read is not the same than that written before with a previous (v2018) version.
After some investigation I found that the 'is_controller_busy' function comes back with -ETIMEDOUT during read operation, because the retry count >of 5 is too low.
To figure out how many retries are needed, I added some debug code as part of a while(1) loop to the is_controller_busy function:
diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c index 1598c4f698..fa5b7a29f2 100644 --- a/drivers/spi/fsl_qspi.c +++ b/drivers/spi/fsl_qspi.c @@ -152,16 +152,20 @@ static inline int is_controller_busy(const struct fsl_qspi_priv *priv) u32 val; const u32 mask = QSPI_SR_BUSY_MASK | QSPI_SR_AHB_ACC_MASK | QSPI_SR_IP_ACC_MASK;
unsigned int retry = 5;
unsigned int retry = 0; do { val = qspi_read32(priv->flags, &priv->regs->sr);
if ((~val & mask) == mask)
if ((~val & mask) == mask) {
if (retry > 5)
printf("%s: timeout! retry = %d\n",
__func__, retry); return 0;
} udelay(1);
} while (--retry);
retry++;
} while (1); return -ETIMEDOUT;
}
On my MX7 eval system I found that a retry count between 73 and 147 is necessary to make sure that the controller will be ready before the next operation.
So from my point of view we have 2 options to fix this issue:
- increase retry from 5 to 200
- wait in an endless loop (while (1) ) and return only when the controller is ready (which means that the board will hang if the controller doesn't come back for some other reason.
What do you think?
I think that readl_poll_timeout() could be used here as it will timeout if the QSPI controller is not ready.
Also, could you check how this is handled in the fsl qspi driver in the kernel?
There is a similar readl_plll_tout call in 5.1.12 linux kernel driver (spi-fsl-qspi.c) with 1000us timeout:
/* wait for the controller being ready */ fsl_qspi_readl_poll_tout(q, base + QUADSPI_SR, (QUADSPI_SR_IP_ACC_MASK | QUADSPI_SR_AHB_ACC_MASK), 10, 1000);
The fsl additions take concern about endianess before calling the general 'readl_poll_timeout' function.
Thomas

Hi all,
Hi Fabio,
Hi Thomas,
On Wed, Jun 19, 2019 at 10:04 AM Thomas Schaefer Thomas.Schaefer@kontron.com wrote:
Hi all,
I've built current u-boot (v2019.07-rc4) with QSPI support for our i.MX7D Sabre eval system. I am using mx7dsabresd_qspi_defconfig.
The resulting u-boot fails to read from QSPI, i.e. the data read is not the same than that written before with a previous (v2018) version.
After some investigation I found that the 'is_controller_busy' function comes back with -ETIMEDOUT during read operation, because the retry count >of 5 is too low.
To figure out how many retries are needed, I added some debug code as part of a while(1) loop to the is_controller_busy function:
diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c index 1598c4f698..fa5b7a29f2 100644 --- a/drivers/spi/fsl_qspi.c +++ b/drivers/spi/fsl_qspi.c @@ -152,16 +152,20 @@ static inline int is_controller_busy(const struct fsl_qspi_priv *priv) u32 val; const u32 mask = QSPI_SR_BUSY_MASK | QSPI_SR_AHB_ACC_MASK | QSPI_SR_IP_ACC_MASK;
unsigned int retry = 5;
unsigned int retry = 0; do { val = qspi_read32(priv->flags, &priv->regs->sr);
if ((~val & mask) == mask)
if ((~val & mask) == mask) {
if (retry > 5)
printf("%s: timeout! retry = %d\n",
__func__, retry); return 0;
} udelay(1);
} while (--retry);
retry++;
} while (1); return -ETIMEDOUT;
}
On my MX7 eval system I found that a retry count between 73 and 147 is necessary to make sure that the controller will be ready before the next operation.
So from my point of view we have 2 options to fix this issue:
- increase retry from 5 to 200
- wait in an endless loop (while (1) ) and return only when the controller is ready (which means that the board will hang if the controller doesn't come back for some other reason.
What do you think?
I think that readl_poll_timeout() could be used here as it will timeout if the QSPI controller is not ready.
Also, could you check how this is handled in the fsl qspi driver in the kernel?
There is a similar readl_plll_tout call in 5.1.12 linux kernel driver (spi-fsl-qspi.c) with 1000us timeout:
/* wait for the controller being ready */ fsl_qspi_readl_poll_tout(q, base + QUADSPI_SR, (QUADSPI_SR_IP_ACC_MASK | QUADSPI_SR_AHB_ACC_MASK), 10, 1000);
The fsl additions take concern about endianess before calling the general 'readl_poll_timeout' function.
Thomas
Following Fabio's suggestion I have prepared the following patch that fixes the QSPI read support issue of the fsl_qspi driver on my i.MX7D Sabre eval system. Could you please review/check this patch and introduce it into upcoming v2019.07 version?
Thanks, Thomas
From: Thomas Schaefer thomas.schaefer@kontron.com Date: Mon, 24 Jun 2019 17:15:57 +0200 Subject: [PATCH] driver/spi: fsl_qspi: fix is_controller_busy check
Use readl_poll_timeout function to check for busy controller with a timeout of 1000 us instead of 5 fixed test loops.
Signed-off-by: Thomas Schaefer thomas.schaefer@kontron.com --- drivers/spi/fsl_qspi.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c index 1598c4f698..41abe1996f 100644 --- a/drivers/spi/fsl_qspi.c +++ b/drivers/spi/fsl_qspi.c @@ -10,6 +10,7 @@ #include <spi.h> #include <asm/io.h> #include <linux/sizes.h> +#include <linux/iopoll.h> #include <dm.h> #include <errno.h> #include <watchdog.h> @@ -150,20 +151,13 @@ static void qspi_write32(u32 flags, u32 *addr, u32 val) static inline int is_controller_busy(const struct fsl_qspi_priv *priv) { u32 val; - const u32 mask = QSPI_SR_BUSY_MASK | QSPI_SR_AHB_ACC_MASK | - QSPI_SR_IP_ACC_MASK; - unsigned int retry = 5; + u32 mask = QSPI_SR_BUSY_MASK | QSPI_SR_AHB_ACC_MASK | + QSPI_SR_IP_ACC_MASK;
- do { - val = qspi_read32(priv->flags, &priv->regs->sr); + if (priv->flags & QSPI_FLAG_REGMAP_ENDIAN_BIG) + mask = (u32)cpu_to_be32(mask);
- if ((~val & mask) == mask) - return 0; - - udelay(1); - } while (--retry); - - return -ETIMEDOUT; + return readl_poll_timeout(&priv->regs->sr, val, !(val & mask), 1000); }
/* QSPI support swapping the flash read/write data -- 2.11.0

Hi Thomas,
On Mon, Jun 24, 2019 at 12:47 PM Thomas Schaefer Thomas.Schaefer@kontron.com wrote:
Following Fabio's suggestion I have prepared the following patch that fixes the QSPI read support issue of the fsl_qspi driver on my i.MX7D Sabre eval system. Could you please review/check this patch and introduce it into upcoming v2019.07 version?
Thanks, Thomas
From: Thomas Schaefer thomas.schaefer@kontron.com Date: Mon, 24 Jun 2019 17:15:57 +0200 Subject: [PATCH] driver/spi: fsl_qspi: fix is_controller_busy check
Use readl_poll_timeout function to check for busy controller with a timeout of 1000 us instead of 5 fixed test loops.
Signed-off-by: Thomas Schaefer thomas.schaefer@kontron.com
I think the change looks good, but I suggest to improve the commit log by stating that with the current code you observe timeout during QSPI read.
Please submit it as formal patch.
Thanks
participants (2)
-
Fabio Estevam
-
Thomas Schaefer