diff mbox series

[1/4] spi: spi-nxp-fspi: enable runtime pm for fspi

Message ID 1629445443-10300-1-git-send-email-haibo.chen@nxp.com
State Superseded
Headers show
Series [1/4] spi: spi-nxp-fspi: enable runtime pm for fspi | expand

Commit Message

Bough Chen Aug. 20, 2021, 7:44 a.m. UTC 
From: Haibo Chen <haibo.chen@nxp.com>

Enable the runtime PM in fspi driver. Reading the power mode from the
debug monitor, FSPI_0 was on and with the patch it is lp.

Signed-off-by: Han Xu <han.xu@nxp.com>
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/spi/spi-nxp-fspi.c | 81 +++++++++++++++++++++++++++++++-------
 1 file changed, 67 insertions(+), 14 deletions(-)

Comments

Kuldeep Singh Aug. 20, 2021, 8:48 a.m. UTC | #1 
Hi Haibo,

> -----Original Message-----
> From: haibo.chen@nxp.com <haibo.chen@nxp.com>
> Sent: Friday, August 20, 2021 1:14 PM
> To: Ashish Kumar <ashish.kumar@nxp.com>; yogeshgaur.83@gmail.com;
> broonie@kernel.org
> Cc: linux-spi@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Bough Chen
> <haibo.chen@nxp.com>
> Subject: [EXT] [PATCH 2/4] spi: spi-nxp-fspi: change the default lut index
> 
> Caution: EXT Email
> 
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> The fspi dynamic lut use the last lut for all IPS operations, the imx8ulp only
> supports 15 luts, so change the last lut index from
> 31 to 15.
> 
> Signed-off-by: Han Xu <han.xu@nxp.com>
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  drivers/spi/spi-nxp-fspi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c index
> 1eecf20f1dab..a764eb475aed 100644
> --- a/drivers/spi/spi-nxp-fspi.c
> +++ b/drivers/spi/spi-nxp-fspi.c
> @@ -63,9 +63,9 @@
>  /*
>   * 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
> - * read operation, so let's use the last entry (31).
> + * read operation, so let's use the last entry (15).

One nitpick:
Last LUT entry is still 31 for all platforms except imx8ulp which has 15.
QSPI driver uses last lut entry 15 whereas 15 will be middle entry for flexspi(in general).

The word 'last' in current comment does not seem appropriate for all cases.
Maybe you can update comment for ex:
"Index 0 is present at boot with a basic read operation and last lut entry is 31.
Some platforms like imx8ulp supports 16 lut entries and therefore, use lut index entry 15."

Other than that,
Reviewed-by: Kuldeep Singh <kuldeep.singh@nxp.com>

Regards
Kuldeep
Bough Chen Aug. 20, 2021, 9:54 a.m. UTC | #2 
> -----Original Message-----
> From: Kuldeep Singh
> Sent: 2021年8月20日 16:49
> To: Bough Chen <haibo.chen@nxp.com>; Ashish Kumar
> <ashish.kumar@nxp.com>; yogeshgaur.83@gmail.com; broonie@kernel.org
> Cc: linux-spi@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Bough
Chen
> <haibo.chen@nxp.com>
> Subject: RE: [EXT] [PATCH 2/4] spi: spi-nxp-fspi: change the default lut
index
> 
> Hi Haibo,
> 
> > -----Original Message-----
> > From: haibo.chen@nxp.com <haibo.chen@nxp.com>
> > Sent: Friday, August 20, 2021 1:14 PM
> > To: Ashish Kumar <ashish.kumar@nxp.com>; yogeshgaur.83@gmail.com;
> > broonie@kernel.org
> > Cc: linux-spi@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Bough
> > Chen <haibo.chen@nxp.com>
> > Subject: [EXT] [PATCH 2/4] spi: spi-nxp-fspi: change the default lut
> > index
> >
> > Caution: EXT Email
> >
> > From: Haibo Chen <haibo.chen@nxp.com>
> >
> > The fspi dynamic lut use the last lut for all IPS operations, the
> > imx8ulp only supports 15 luts, so change the last lut index from
> > 31 to 15.
> >
> > Signed-off-by: Han Xu <han.xu@nxp.com>
> > Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> > ---
> >  drivers/spi/spi-nxp-fspi.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
> > index 1eecf20f1dab..a764eb475aed 100644
> > --- a/drivers/spi/spi-nxp-fspi.c
> > +++ b/drivers/spi/spi-nxp-fspi.c
> > @@ -63,9 +63,9 @@
> >  /*
> >   * 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
> > - * read operation, so let's use the last entry (31).
> > + * read operation, so let's use the last entry (15).
> 
> One nitpick:
> Last LUT entry is still 31 for all platforms except imx8ulp which has 15.
> QSPI driver uses last lut entry 15 whereas 15 will be middle entry for
flexspi(in
> general).
> 
> The word 'last' in current comment does not seem appropriate for all
cases.
> Maybe you can update comment for ex:
> "Index 0 is present at boot with a basic read operation and last lut entry
is 31.
> Some platforms like imx8ulp supports 16 lut entries and therefore, use lut
index
> entry 15."
> 
> Other than that,
> Reviewed-by: Kuldeep Singh <kuldeep.singh@nxp.com>

Hi Kuldeep,

Thanks for your suggestion. I will do that.

Best Regards
Haibo Chen
> 
> Regards
> Kuldeep
diff mbox series

Patch

diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index a66fa97046ee..1eecf20f1dab 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -48,6 +48,7 @@ 
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/pm_qos.h>
 #include <linux/regmap.h>
 #include <linux/sizes.h>
@@ -57,6 +58,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
@@ -373,6 +376,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)
@@ -864,6 +869,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);
@@ -892,8 +903,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;
 }
 
@@ -1153,12 +1170,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 */
@@ -1192,13 +1214,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);
@@ -1224,20 +1252,50 @@  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 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(pm_runtime_force_suspend, pm_runtime_force_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, },
@@ -1256,11 +1314,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",