[PATCH 4/7] mtd: spi-nor: fsl-quadspi: i.MX6SX: fixed the random QSPI access failed issue

Han Xu xhnjupt at gmail.com
Fri Jul 24 08:38:48 PDT 2015


On Tue, Jul 21, 2015 at 2:39 PM,  <Frank.Li at freescale.com> wrote:
> From: Allen Xu <b45815 at freescale.com>
>
> We found there is a low probability(5%) QSPI access timeout issue,
> usually it happened on kernel boot stage, the first time kernel tried to
> access QSPI chip. The READ_ID command was sent but not executed,
> consequently the probe function failed.
>
> Finally we located the issue by these steps.
>
> 1. Since the issue happened randomly and usually it cost half day to
> reproduce, we add more debug code in driver, to create a timeout file if
> the issue occurred.
>
> 2. Prepared an autorun script to keep rebooting the system and check if
> the timeout file existed, if the file existed, stop reboot.
>
> 3. The system will stop rebooting when timeout error occurred, set the
> CCM_CCOSR register and related IOMUX to measure QPSI clock, found there
> is no clock output, while clock output can be measured when QSPI driver
> successfully probed.
>
> 4. Check the code and found QSPI clock rate was changed while not
> disabled clock gate, most of the multiplexers on i.MX6 are glitch ones,
> clock glitch may occurred and propagated into downstream clock dividers
>
> Based on the new clock flag(CLK_SET_RATE_GATE) and new framework, we
> need to change the approach of seting clock rate. In current
> implementation, there are several places in which the clock was touched.
>
> 1. probe function. prepare and enable clock before setting the QSPI
> register, disable and unprepare the clock before exit.
>
> 2. nor_setup & nor_setup_last, since we change clock rate in these two
> functions.
>
> 3. fsl_qspi_prep and fsl_qspi_unprep, clock was enabled only when got
> QSPI access request.
>
> 4. resume function. Clock was required to restroe the setting after
> resume, disable the clock before exit.
>
> Signed-off-by: Allen Xu <b45815 at freescale.com>
> Signed-off-by: Frank Li <Frank.Li at freescale.com>

Acked-by: Han Xu <han.xu at freescale.com>

> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 83 +++++++++++++++++++++++++++------------
>  1 file changed, 58 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index 921fba1..1419f1f 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -653,6 +653,32 @@ static void fsl_qspi_init_abh_read(struct fsl_qspi *q)
>                 q->iobase + QUADSPI_BFGENCR);
>  }
>
> +/* This function was used to prepare and enable QSPI clock */
> +static int fsl_qspi_clk_prep_enable(struct fsl_qspi *q)
> +{
> +       int ret;
> +
> +       ret = clk_prepare_enable(q->clk_en);
> +       if (ret)
> +               return ret;
> +
> +       ret = clk_prepare_enable(q->clk);
> +       if (ret) {
> +               clk_disable_unprepare(q->clk_en);
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +/* This function was used to disable and unprepare QSPI clock */
> +static void fsl_qspi_clk_disable_unprep(struct fsl_qspi *q)
> +{
> +       clk_disable_unprepare(q->clk);
> +       clk_disable_unprepare(q->clk_en);
> +
> +}
> +
>  /* We use this function to do some basic init for spi_nor_scan(). */
>  static int fsl_qspi_nor_setup(struct fsl_qspi *q)
>  {
> @@ -660,11 +686,19 @@ static int fsl_qspi_nor_setup(struct fsl_qspi *q)
>         u32 reg;
>         int ret;
>
> -       /* the default frequency, we will change it in the future.*/
> +       /* disable and unprepare clock first */
> +       fsl_qspi_clk_disable_unprep(q);
> +
> +       /* the default frequency, we will change it in the future. */
>         ret = clk_set_rate(q->clk, 66000000);
>         if (ret)
>                 return ret;
>
> +       /* prepare and enable the clock */
> +       ret = fsl_qspi_clk_prep_enable(q);
> +       if (ret)
> +               return ret;
> +
>         /* Init the LUT table. */
>         fsl_qspi_init_lut(q);
>
> @@ -696,10 +730,18 @@ static int fsl_qspi_nor_setup_last(struct fsl_qspi *q)
>         if (needs_4x_clock(q))
>                 rate *= 4;
>
> +       /* disable and unprepare clock first */
> +       fsl_qspi_clk_disable_unprep(q);
> +
>         ret = clk_set_rate(q->clk, rate);
>         if (ret)
>                 return ret;
>
> +       /* prepare and enable the clock */
> +       ret = fsl_qspi_clk_prep_enable(q);
> +       if (ret)
> +               return ret;
> +
>         /* Init the LUT table again. */
>         fsl_qspi_init_lut(q);
>
> @@ -841,22 +883,16 @@ static int fsl_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
>         int ret;
>
>         mutex_lock(&q->lock);
> -       ret = clk_enable(q->clk_en);
> -       if (ret)
> -               goto err_mutex;
>
> -       ret = clk_enable(q->clk);
> +       ret = fsl_qspi_clk_prep_enable(q);
>         if (ret)
> -               goto err_clk;
> +               goto err_mutex;
>
>         fsl_qspi_set_base_addr(q, nor);
>         return 0;
>
> -err_clk:
> -       clk_disable(q->clk_en);
>  err_mutex:
>         mutex_unlock(&q->lock);
> -
>         return ret;
>  }
>
> @@ -864,8 +900,7 @@ static void fsl_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
>  {
>         struct fsl_qspi *q = nor->priv;
>
> -       clk_disable(q->clk);
> -       clk_disable(q->clk_en);
> +       fsl_qspi_clk_disable_unprep(q);
>         mutex_unlock(&q->lock);
>  }
>
> @@ -909,15 +944,9 @@ static int fsl_qspi_probe(struct platform_device *pdev)
>         if (IS_ERR(q->clk))
>                 return PTR_ERR(q->clk);
>
> -       ret = clk_prepare_enable(q->clk_en);
> +       ret = fsl_qspi_clk_prep_enable(q);
>         if (ret) {
> -               dev_err(dev, "cannot enable the qspi_en clock: %d\n", ret);
> -               return ret;
> -       }
> -
> -       ret = clk_prepare_enable(q->clk);
> -       if (ret) {
> -               dev_err(dev, "cannot enable the qspi clock: %d\n", ret);
> +               dev_err(dev, "can not enable the clock\n");
>                 goto clk_failed;
>         }
>
> @@ -1023,8 +1052,7 @@ static int fsl_qspi_probe(struct platform_device *pdev)
>         if (ret)
>                 goto last_init_failed;
>
> -       clk_disable(q->clk);
> -       clk_disable(q->clk_en);
> +       fsl_qspi_clk_disable_unprep(q);
>         return 0;
>
>  last_init_failed:
> @@ -1037,9 +1065,9 @@ last_init_failed:
>  mutex_failed:
>         mutex_destroy(&q->lock);
>  irq_failed:
> -       clk_disable_unprepare(q->clk);
> +       fsl_qspi_clk_disable_unprep(q);
>  clk_failed:
> -       clk_disable_unprepare(q->clk_en);
> +       dev_err(dev, "Freescale QuadSPI probe failed\n");
>         return ret;
>  }
>
> @@ -1060,8 +1088,6 @@ static int fsl_qspi_remove(struct platform_device *pdev)
>         writel(0x0, q->iobase + QUADSPI_RSER);
>
>         mutex_destroy(&q->lock);
> -       clk_unprepare(q->clk);
> -       clk_unprepare(q->clk_en);
>
>         if (q->ahb_addr)
>                 iounmap(q->ahb_addr);
> @@ -1076,12 +1102,19 @@ static int fsl_qspi_suspend(struct platform_device *pdev, pm_message_t state)
>
>  static int fsl_qspi_resume(struct platform_device *pdev)
>  {
> +       int ret;
>         struct fsl_qspi *q = platform_get_drvdata(pdev);
>
> +       ret = fsl_qspi_clk_prep_enable(q);
> +       if (ret)
> +               return ret;
> +
>         fsl_qspi_nor_setup(q);
>         fsl_qspi_set_map_addr(q);
>         fsl_qspi_nor_setup_last(q);
>
> +       fsl_qspi_clk_disable_unprep(q);
> +
>         return 0;
>  }
>
> --
> 1.9.1
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



More information about the linux-mtd mailing list