Re: [PATCH 016/347] FogBugz #516535: Fix QSPI write issues
Hi,
On 30/08/22 11:50 am, Jit Loon Lim wrote:
> From: Chee Hong Ang
>
> QSPI driver perform chip select on every flash read/write
> access. The driver need to disable/enable the QSPI controller
> while performing chip select. This may cause some data lost
> especially the QSPI controller is configured to run at slower
> speed as it may take longer time to access the flash device.
> This patch prevent the driver from disable/enable the QSPI
> controller too soon and inadvertently halting any ongoing flash
> read/write access by ensuring the QSPI controller is always in
> idle mode after each read/write access.
>
> Signed-off-by: Chee Hong Ang
> ---
> drivers/spi/cadence_qspi_apb.c | 15 ---
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> index 2cdf4c9c9f..5e03495f45 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -858,13 +858,9 @@ cadence_qspi_apb_indirect_read_execute(struct
> cadence_spi_plat *plat,
> writel(CQSPI_REG_INDIRECTRD_DONE,
> plat->regbase + CQSPI_REG_INDIRECTRD);
>
> - /* Check indirect done status */
> - ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTRD,
> - CQSPI_REG_INDIRECTRD_DONE, 0, 10, 0);
> - if (ret) {
> - printf("Indirect read clear completion error (%i)\n", ret);
> - goto failrd;
> - }
Why would you drop looking at CQSPI_REG_INDIRECTRD_DONE status bit? This
gives out a much granular error wrt what operation actually failed.
> + /* Wait til QSPI is idle */
> + if (!cadence_qspi_wait_idle(plat->regbase))
> + return -EIO;
>
> return 0;
>
> @@ -1031,6 +1027,11 @@ cadence_qspi_apb_indirect_write_execute(struct
> cadence_spi_plat *plat,
>
> if (bounce_buf)
> free(bounce_buf);
> +
> + /* Wait til QSPI is idle */
> + if (!cadence_qspi_wait_idle(plat->regbase))
> + return -EIO;
> +
> return 0;
>
> failwr:
Re: [PATCH 016/347] FogBugz #516535: Fix QSPI write issues
On Tue, Aug 30, 2022 at 11:50 AM Jit Loon Lim wrote: > > From: Chee Hong Ang > > QSPI driver perform chip select on every flash read/write > access. The driver need to disable/enable the QSPI controller > while performing chip select. This may cause some data lost > especially the QSPI controller is configured to run at slower > speed as it may take longer time to access the flash device. > This patch prevent the driver from disable/enable the QSPI > controller too soon and inadvertently halting any ongoing flash > read/write access by ensuring the QSPI controller is always in > idle mode after each read/write access. > > Signed-off-by: Chee Hong Ang > -- "FogBugz #516535:" This commit head is invalid for the upstream patch.
RE: [PATCH 016/347] FogBugz #516535: Fix QSPI write issues
Hi Jit Loon,
> -Original Message-
> From: Lim, Jit Loon
> Sent: Tuesday, 2 August, 2022 9:55 PM
> To: u-boot@lists.denx.de
> Cc: Jagan Teki ; Vignesh R ;
> Vasut, Marek ; Simon ;
> Chaplin, Kris ; Chee, Tien Fong
> ; Hea, Kok Kiang ; Lim,
> Elly Siew Chin ; Kho, Sin Hui
> ; Lokanathan, Raaj ;
> Maniyam, Dinesh ; Ng, Boon Khai
> ; Yuslaimi, Alif Zakuan
> ; Chong, Teik Heng
> ; Zamri, Muhammad Hazim Izzat
> ; Lim, Jit Loon
> ; Chee Hong Ang
> Subject: [PATCH 016/347] FogBugz #516535: Fix QSPI write issues
Please drop the "[PATCH 016/347]" and replace "FogBugz #516535" with
appropriate tag.
>
> From: Chee Hong Ang
>
> QSPI driver perform chip select on every flash read/write access. The driver
> need
> to disable/enable the QSPI controller while performing chip select. This may
> cause some data lost especially the QSPI controller is configured to run at
> slower speed as it may take longer time to access the flash device.
> This patch prevent the driver from disable/enable the QSPI controller too soon
> and inadvertently halting any ongoing flash read/write access by ensuring the
> QSPI controller is always in idle mode after each read/write access.
>
> Signed-off-by: Chee Hong Ang
> ---
> drivers/spi/cadence_qspi_apb.c | 15 ---
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> index 2cdf4c9c9f..5e03495f45 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -858,13 +858,9 @@ cadence_qspi_apb_indirect_read_execute(struct
> cadence_spi_plat *plat,
> writel(CQSPI_REG_INDIRECTRD_DONE,
> plat->regbase + CQSPI_REG_INDIRECTRD);
>
> - /* Check indirect done status */
> - ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTRD,
> - CQSPI_REG_INDIRECTRD_DONE, 0, 10, 0);
> - if (ret) {
> - printf("Indirect read clear completion error (%i)\n", ret);
> - goto failrd;
> - }
> + /* Wait til QSPI is idle */
> + if (!cadence_qspi_wait_idle(plat->regbase))
> + return -EIO;
>
> return 0;
>
> @@ -1031,6 +1027,11 @@ cadence_qspi_apb_indirect_write_execute(struct
> cadence_spi_plat *plat,
>
> if (bounce_buf)
> free(bounce_buf);
> +
> + /* Wait til QSPI is idle */
> + if (!cadence_qspi_wait_idle(plat->regbase))
> + return -EIO;
> +
> return 0;
>
> failwr:
> --
> 2.25.1
Regards,
Tien Fong
