| Message ID | 1657012303-6464-1-git-send-email-haibo.chen@nxp.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [01/11] spi: spi-nxp-fspi: enable runtime pm for fspi | expand |
Am 2022-07-05 11:11, schrieb haibo.chen@nxp.com: > From: Haibo Chen <haibo.chen@nxp.com> > > Enable the runtime PM in fspi driver. > Also for system PM, On some board like i.MX8ULP-EVK board, > after system suspend, IOMUX module will lost power, so all > the pinctrl setting will lost when system resume back, need > driver to save/restore the pinctrl setting. On a side note: The mails to Ashish Kumar bounces. He is currently listed as the maintainer for the FlexSPI driver. Will someone from NXP take over? -michael
>-----Original Message----- >From: Michael Walle <michael@walle.cc> >Sent: Tuesday, July 5, 2022 9:01 AM >To: Bough Chen <haibo.chen@nxp.com> >Cc: ashish.kumar@nxp.com; yogeshgaur.83@gmail.com; broonie@kernel.org; >robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; Han Xu ><han.xu@nxp.com>; singh.kuldeep87k@gmail.com; >tudor.ambarus@microchip.com; p.yadav@ti.com; miquel.raynal@bootlin.com; >richard@nod.at; vigneshr@ti.com; shawnguo@kernel.org; >s.hauer@pengutronix.de; kernel@pengutronix.de; linux-spi@vger.kernel.org; >linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; linux- >mtd@lists.infradead.org; festevam@gmail.com; dl-linux-imx <linux- >imx@nxp.com>; linux-arm-kernel@lists.infradead.org; zhengxunli@mxic.com.tw >Subject: Re: [PATCH 01/11] spi: spi-nxp-fspi: enable runtime pm for fspi > >Am 2022-07-05 11:11, schrieb haibo.chen@nxp.com: >> From: Haibo Chen <haibo.chen@nxp.com> >> >> Enable the runtime PM in fspi driver. >> Also for system PM, On some board like i.MX8ULP-EVK board, after >> system suspend, IOMUX module will lost power, so all the pinctrl >> setting will lost when system resume back, need driver to save/restore >> the pinctrl setting. > >On a side note: The mails to Ashish Kumar bounces. He is currently listed as the >maintainer for the FlexSPI driver. Will someone from NXP take over? I will take it over. > >-michael
Am 2022-07-05 11:11, schrieb haibo.chen@nxp.com: > From: Haibo Chen <haibo.chen@nxp.com> > > fspi define four mode for sample clock source selection. > > Here is the list of modes: > mode 0: Dummy Read strobe generated by FlexSPI Controller and loopback > internally > mode 1: Dummy Read strobe generated by FlexSPI Controller and loopback > from DQS pad > mode 2: Reserved > mode 3: Flash provided Read strobe and input from DQS pad > > In default, fspi use mode 0 after reset. > For 8-8-8-DTR mode, need to use mode 3, otherwise 8-8-8-DTR read always > get incorrect data. > > Signed-off-by: Haibo Chen <haibo.chen@nxp.com> > --- > drivers/spi/spi-nxp-fspi.c | 47 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 47 insertions(+) > > diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c > index c32a4f53fa2a..34679dc0e1ad 100644 > --- a/drivers/spi/spi-nxp-fspi.c > +++ b/drivers/spi/spi-nxp-fspi.c > @@ -380,6 +380,7 @@ struct nxp_fspi { > struct pm_qos_request pm_qos_req; > int selected; > #define FSPI_INITILIZED (1 << 0) > +#define FSPI_RXCLKSRC_3 (1 << 1) > int flags; > }; > > @@ -877,6 +878,50 @@ static int nxp_fspi_do_op(struct nxp_fspi *f, > const struct spi_mem_op *op) > return err; > } > > +/* > + * Sample Clock source selection for Flash Reading > + * Four modes defined by fspi: > + * mode 0: Dummy Read strobe generated by FlexSPI Controller > + * and loopback internally > + * mode 1: Dummy Read strobe generated by FlexSPI Controller > + * and loopback from DQS pad > + * mode 2: Reserved > + * mode 3: Flash provided Read strobe and input from DQS pad > + * > + * fspi default use mode 0 after reset > + */ > +static void nxp_fspi_select_rx_sample_clk_source(struct nxp_fspi *f, > + const struct spi_mem_op *op) > +{ > + u32 reg; > + > + /* > + * For 8-8-8-DTR mode, need to use mode 3 (Flash provided Read > + * strobe and input from DQS pad), otherwise read operaton may > + * meet issue. > + * This mode require flash device connect the DQS pad on board. > + * For other modes, still use mode 0, keep align with before. > + * spi_nor_suspend will disable 8-8-8-DTR mode, also need to > + * change the mode back to mode 0. > + */ > + if (!(f->flags & FSPI_RXCLKSRC_3) && > + op->cmd.dtr && op->addr.dtr && > + op->dummy.dtr && op->data.dtr) { > + reg = fspi_readl(f, f->iobase + FSPI_MCR0); > + reg |= FSPI_MCR0_RXCLKSRC(3); > + fspi_writel(f, reg, f->iobase + FSPI_MCR0); > + f->flags |= FSPI_RXCLKSRC_3; > + } else if ((f->flags & FSPI_RXCLKSRC_3) && > + !op->cmd.dtr && !op->addr.dtr && > + !op->dummy.dtr && !op->data.dtr) { > + reg = fspi_readl(f, f->iobase + FSPI_MCR0); > + reg &= ~FSPI_MCR0_RXCLKSRC(3); /* select mode 0 */ > + fspi_writel(f, reg, f->iobase + FSPI_MCR0); > + f->flags &= ~FSPI_RXCLKSRC_3; > + } How is this supposed to work? Are you unconditionally enable flash provided read strobes if DTR is used? What if the flash doesn't provide one or the board haven't DQS connected? -michael
diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c index 2b0301fc971c..b2cd8e06f374 100644 --- a/drivers/spi/spi-nxp-fspi.c +++ b/drivers/spi/spi-nxp-fspi.c @@ -49,6 +49,8 @@ #include <linux/of.h> #include <linux/of_device.h> #include <linux/platform_device.h> +#include <linux/pinctrl/consumer.h> +#include <linux/pm_runtime.h> #include <linux/pm_qos.h> #include <linux/regmap.h> #include <linux/sizes.h> @@ -58,6 +60,8 @@ #include <linux/spi/spi.h> #include <linux/spi/spi-mem.h> +/* runtime pm timeout */ +#define FSPI_RPM_TIMEOUT 50 /* 50ms */ /* * The driver only uses one single LUT entry, that is updated on * each call of exec_op(). Index 0 is preset at boot with a basic @@ -375,6 +379,8 @@ struct nxp_fspi { struct mutex lock; struct pm_qos_request pm_qos_req; int selected; +#define FSPI_INITILIZED (1 << 0) + int flags; }; static inline int needs_ip_only(struct nxp_fspi *f) @@ -866,6 +872,12 @@ static int nxp_fspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op) mutex_lock(&f->lock); + err = pm_runtime_get_sync(f->dev); + if (err < 0) { + dev_err(f->dev, "Failed to enable clock %d\n", __LINE__); + goto err_mutex; + } + /* Wait for controller being ready. */ err = fspi_readl_poll_tout(f, f->iobase + FSPI_STS0, FSPI_STS0_ARB_IDLE, 1, POLL_TOUT, true); @@ -894,8 +906,14 @@ static int nxp_fspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op) /* Invalidate the data in the AHB buffer. */ nxp_fspi_invalid(f); + pm_runtime_mark_last_busy(f->dev); + pm_runtime_put_autosuspend(f->dev); + mutex_unlock(&f->lock); + return err; +err_mutex: + mutex_unlock(&f->lock); return err; } @@ -1141,12 +1159,17 @@ static int nxp_fspi_probe(struct platform_device *pdev) ret = PTR_ERR(f->clk); goto err_put_ctrl; } + } - ret = nxp_fspi_clk_prep_enable(f); - if (ret) { - dev_err(dev, "can not enable the clock\n"); - goto err_put_ctrl; - } + pm_runtime_enable(dev); + pm_runtime_set_autosuspend_delay(dev, FSPI_RPM_TIMEOUT); + pm_runtime_use_autosuspend(dev); + + /* enable clock */ + ret = pm_runtime_get_sync(f->dev); + if (ret < 0) { + dev_err(f->dev, "Failed to enable clock %d\n", __LINE__); + goto err_put_ctrl; } /* Clear potential interrupts */ @@ -1180,13 +1203,19 @@ static int nxp_fspi_probe(struct platform_device *pdev) if (ret) goto err_destroy_mutex; + pm_runtime_mark_last_busy(f->dev); + pm_runtime_put_autosuspend(f->dev); + + /* indicate the controller has been initialized */ + f->flags |= FSPI_INITILIZED; + return 0; err_destroy_mutex: mutex_destroy(&f->lock); err_disable_clk: - nxp_fspi_clk_disable_unprep(f); + pm_runtime_disable(dev); err_put_ctrl: spi_controller_put(ctlr); @@ -1212,20 +1241,79 @@ static int nxp_fspi_remove(struct platform_device *pdev) return 0; } -static int nxp_fspi_suspend(struct device *dev) +#ifdef CONFIG_PM +static int nxp_fspi_initialized(struct nxp_fspi *f) +{ + return f->flags & FSPI_INITILIZED; +} + +static int nxp_fspi_need_reinit(struct nxp_fspi *f) +{ + /* + * we always use the controller in combination mode, so we check + * this register bit to determine if the controller once lost power, + * such as suspend/resume, and need to be re-init. + */ + + return !(readl(f->iobase + FSPI_MCR0) & FSPI_MCR0_OCTCOMB_EN); +} + +static int nxp_fspi_runtime_suspend(struct device *dev) { + struct nxp_fspi *f = dev_get_drvdata(dev); + + nxp_fspi_clk_disable_unprep(f); + return 0; } -static int nxp_fspi_resume(struct device *dev) +static int nxp_fspi_runtime_resume(struct device *dev) { struct nxp_fspi *f = dev_get_drvdata(dev); - nxp_fspi_default_setup(f); + nxp_fspi_clk_prep_enable(f); + + if (nxp_fspi_initialized(f) && nxp_fspi_need_reinit(f)) + nxp_fspi_default_setup(f); return 0; } +static int nxp_fspi_suspend(struct device *dev) +{ + int ret; + + ret = pinctrl_pm_select_sleep_state(dev); + if (ret) { + dev_err(dev, "select flexspi sleep pinctrl failed!\n"); + return ret; + } + + return pm_runtime_force_suspend(dev); +} + +static int nxp_fspi_resume(struct device *dev) +{ + int ret; + + ret = pm_runtime_force_resume(dev); + if (ret) + return ret; + + ret = pinctrl_pm_select_default_state(dev); + if (ret) + dev_err(dev, "select flexspi default pinctrl failed!\n"); + + return ret; +} + + +static const struct dev_pm_ops nxp_fspi_pm_ops = { + SET_RUNTIME_PM_OPS(nxp_fspi_runtime_suspend, nxp_fspi_runtime_resume, NULL) + SET_SYSTEM_SLEEP_PM_OPS(nxp_fspi_suspend, nxp_fspi_resume) +}; +#endif /* CONFIG_PM */ + static const struct of_device_id nxp_fspi_dt_ids[] = { { .compatible = "nxp,lx2160a-fspi", .data = (void *)&lx2160a_data, }, { .compatible = "nxp,imx8mm-fspi", .data = (void *)&imx8mm_data, }, @@ -1244,11 +1332,6 @@ static const struct acpi_device_id nxp_fspi_acpi_ids[] = { MODULE_DEVICE_TABLE(acpi, nxp_fspi_acpi_ids); #endif -static const struct dev_pm_ops nxp_fspi_pm_ops = { - .suspend = nxp_fspi_suspend, - .resume = nxp_fspi_resume, -}; - static struct platform_driver nxp_fspi_driver = { .driver = { .name = "nxp-fspi",