diff mbox series

spi: imx: enable runtime pm support

Message ID 20200727063354.17031-1-xiaoning.wang@nxp.com (mailing list archive)
State Mainlined
Commit 525c9e5a32bd7951eae3f06d9d077fea51718a6c
Headers show
Series spi: imx: enable runtime pm support | expand

Commit Message

Clark Wang July 27, 2020, 6:33 a.m. UTC
Enable runtime pm support for spi-imx driver.

Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
---
 drivers/spi/spi-imx.c | 121 ++++++++++++++++++++++++++++++------------
 1 file changed, 88 insertions(+), 33 deletions(-)

Comments

Mark Brown July 27, 2020, 1:57 p.m. UTC | #1
On Mon, 27 Jul 2020 14:33:54 +0800, Clark Wang wrote:
> Enable runtime pm support for spi-imx driver.

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: imx: enable runtime pm support
      commit: 525c9e5a32bd7951eae3f06d9d077fea51718a6c

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
Nikita Shubin Nov. 13, 2020, 9:18 a.m. UTC | #2
Hello Clark, 

This patch breaks spi-imx on imx7d.
Toradex Colibri imx7d spi reports with:

    [    4.258468] inv-mpu6000-spi spi2.0: I/O Error in PIO
    [    4.264269] inv-mpu6000-spi spi2.0: SPI transfer failed: -110
    [    4.264305] spi_master spi2: failed to transfer one message from queue

We are using spi-imx with dma.

Reverting your patch fixes this issue.

The baseline commit 951cbbc386ff01b50da4f46387e994e81d9ab431 (tag: v5.9.8, stable/linux-5.9.y)

Could you please give some comments on this issue ?
Fabio Estevam Nov. 13, 2020, 11:26 a.m. UTC | #3
Hi Nikita,

On Fri, Nov 13, 2020 at 6:18 AM Nikita Shubin <nikita.shubin@maquefel.me> wrote:
>
> Hello Clark,
>
> This patch breaks spi-imx on imx7d.
> Toradex Colibri imx7d spi reports with:
>
>     [    4.258468] inv-mpu6000-spi spi2.0: I/O Error in PIO
>     [    4.264269] inv-mpu6000-spi spi2.0: SPI transfer failed: -110
>     [    4.264305] spi_master spi2: failed to transfer one message from queue
>
> We are using spi-imx with dma.
>
> Reverting your patch fixes this issue.
>
> The baseline commit 951cbbc386ff01b50da4f46387e994e81d9ab431 (tag: v5.9.8, stable/linux-5.9.y)
>
> Could you please give some comments on this issue ?

Could you please try this patch from Sascha?
https://lkml.org/lkml/2020/10/12/981
Nikita Shubin Nov. 13, 2020, 12:04 p.m. UTC | #4
Hi Fabio,

Apllied on the top of

   Revert "spi: imx: Fix freeing of DMA channels if spi_bitbang_start() fails"

   This reverts commit 9e68d974b2fd4977e896e166656c36e2f80293d6.

Still experiencing the same issue:

[    4.322131] inv-mpu6000-spi spi2.0: I/O Error in PIO
[    4.329353] inv-mpu6000-spi spi2.0: SPI transfer failed: -110
[    4.391504] spi_master spi2: failed to transfer one message from queue

I have config CONFIG_PM=y.

13.11.2020, 14:26, "Fabio Estevam" <festevam@gmail.com>:
> Hi Nikita,
>
> On Fri, Nov 13, 2020 at 6:18 AM Nikita Shubin <nikita.shubin@maquefel.me> wrote:
>>  Hello Clark,
>>
>>  This patch breaks spi-imx on imx7d.
>>  Toradex Colibri imx7d spi reports with:
>>
>>      [ 4.258468] inv-mpu6000-spi spi2.0: I/O Error in PIO
>>      [ 4.264269] inv-mpu6000-spi spi2.0: SPI transfer failed: -110
>>      [ 4.264305] spi_master spi2: failed to transfer one message from queue
>>
>>  We are using spi-imx with dma.
>>
>>  Reverting your patch fixes this issue.
>>
>>  The baseline commit 951cbbc386ff01b50da4f46387e994e81d9ab431 (tag: v5.9.8, stable/linux-5.9.y)
>>
>>  Could you please give some comments on this issue ?
>
> Could you please try this patch from Sascha?
> https://lkml.org/lkml/2020/10/12/981
Clark Wang Nov. 24, 2020, 8:57 a.m. UTC | #5
Hi Nikita,

Sorry for this issue caused by my patch.
Yes, it because I did not add the !CONFIG_PM support when add the runtime pm patch. This may cause the clks cannot be enabled when do not enable CONFIG_PM.
Sascha has fix this issue with the patch:
43b6bf406cd0 spi: imx: fix runtime pm support for !CONFIG_PM

You may try the latest upstream driver again.

I also fix a warning dump issue with CONFIG_PM enabled. It is under reviewed. So that, this driver can work well in both cases.

Thanks!

Best Regards,
Clark Wang

> -----Original Message-----
> From: Nikita Shubin <nikita.shubin@maquefel.me>
> Sent: Friday, November 13, 2020 17:18
> To: Clark Wang <xiaoning.wang@nxp.com>
> Cc: broonie@kernel.org; festevam@gmail.com; kernel@pengutronix.de;
> linux-arm-kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>;
> linux-kernel@vger.kernel.org; linux-spi@vger.kernel.org;
> s.hauer@pengutronix.de; shawnguo@kernel.org; Nikita Shubin
> <nikita.shubin@maquefel.me>
> Subject: [EXT] [PATCH] spi: imx: enable runtime pm support
> 
> Caution: EXT Email
> 
> Hello Clark,
> 
> This patch breaks spi-imx on imx7d.
> Toradex Colibri imx7d spi reports with:
> 
>     [    4.258468] inv-mpu6000-spi spi2.0: I/O Error in PIO
>     [    4.264269] inv-mpu6000-spi spi2.0: SPI transfer failed: -110
>     [    4.264305] spi_master spi2: failed to transfer one message from queue
> 
> We are using spi-imx with dma.
> 
> Reverting your patch fixes this issue.
> 
> The baseline commit 951cbbc386ff01b50da4f46387e994e81d9ab431 (tag:
> v5.9.8, stable/linux-5.9.y)
> 
> Could you please give some comments on this issue ?
diff mbox series

Patch

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index fdc25f549378..38a5f1304cec 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -13,7 +13,9 @@ 
 #include <linux/irq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/slab.h>
 #include <linux/spi/spi.h>
 #include <linux/spi/spi_bitbang.h>
@@ -30,6 +32,8 @@  static bool use_dma = true;
 module_param(use_dma, bool, 0644);
 MODULE_PARM_DESC(use_dma, "Enable usage of DMA when available (default)");
 
+#define MXC_RPM_TIMEOUT		2000 /* 2000ms */
+
 #define MXC_CSPIRXDATA		0x00
 #define MXC_CSPITXDATA		0x04
 #define MXC_CSPICTRL		0x08
@@ -1530,20 +1534,16 @@  spi_imx_prepare_message(struct spi_master *master, struct spi_message *msg)
 	struct spi_imx_data *spi_imx = spi_master_get_devdata(master);
 	int ret;
 
-	ret = clk_enable(spi_imx->clk_per);
-	if (ret)
-		return ret;
-
-	ret = clk_enable(spi_imx->clk_ipg);
-	if (ret) {
-		clk_disable(spi_imx->clk_per);
+	ret = pm_runtime_get_sync(spi_imx->dev);
+	if (ret < 0) {
+		dev_err(spi_imx->dev, "failed to enable clock\n");
 		return ret;
 	}
 
 	ret = spi_imx->devtype_data->prepare_message(spi_imx, msg);
 	if (ret) {
-		clk_disable(spi_imx->clk_ipg);
-		clk_disable(spi_imx->clk_per);
+		pm_runtime_mark_last_busy(spi_imx->dev);
+		pm_runtime_put_autosuspend(spi_imx->dev);
 	}
 
 	return ret;
@@ -1554,8 +1554,8 @@  spi_imx_unprepare_message(struct spi_master *master, struct spi_message *msg)
 {
 	struct spi_imx_data *spi_imx = spi_master_get_devdata(master);
 
-	clk_disable(spi_imx->clk_ipg);
-	clk_disable(spi_imx->clk_per);
+	pm_runtime_mark_last_busy(spi_imx->dev);
+	pm_runtime_put_autosuspend(spi_imx->dev);
 	return 0;
 }
 
@@ -1674,13 +1674,15 @@  static int spi_imx_probe(struct platform_device *pdev)
 		goto out_master_put;
 	}
 
-	ret = clk_prepare_enable(spi_imx->clk_per);
-	if (ret)
-		goto out_master_put;
+	pm_runtime_enable(spi_imx->dev);
+	pm_runtime_set_autosuspend_delay(spi_imx->dev, MXC_RPM_TIMEOUT);
+	pm_runtime_use_autosuspend(spi_imx->dev);
 
-	ret = clk_prepare_enable(spi_imx->clk_ipg);
-	if (ret)
-		goto out_put_per;
+	ret = pm_runtime_get_sync(spi_imx->dev);
+	if (ret < 0) {
+		dev_err(spi_imx->dev, "failed to enable clock\n");
+		goto out_runtime_pm_put;
+	}
 
 	spi_imx->spi_clk = clk_get_rate(spi_imx->clk_per);
 	/*
@@ -1690,7 +1692,7 @@  static int spi_imx_probe(struct platform_device *pdev)
 	if (spi_imx->devtype_data->has_dmamode) {
 		ret = spi_imx_sdma_init(&pdev->dev, spi_imx, master);
 		if (ret == -EPROBE_DEFER)
-			goto out_clk_put;
+			goto out_runtime_pm_put;
 
 		if (ret < 0)
 			dev_err(&pdev->dev, "dma setup error %d, use pio\n",
@@ -1705,19 +1707,20 @@  static int spi_imx_probe(struct platform_device *pdev)
 	ret = spi_bitbang_start(&spi_imx->bitbang);
 	if (ret) {
 		dev_err(&pdev->dev, "bitbang start failed with %d\n", ret);
-		goto out_clk_put;
+		goto out_runtime_pm_put;
 	}
 
 	dev_info(&pdev->dev, "probed\n");
 
-	clk_disable(spi_imx->clk_ipg);
-	clk_disable(spi_imx->clk_per);
+	pm_runtime_mark_last_busy(spi_imx->dev);
+	pm_runtime_put_autosuspend(spi_imx->dev);
+
 	return ret;
 
-out_clk_put:
-	clk_disable_unprepare(spi_imx->clk_ipg);
-out_put_per:
-	clk_disable_unprepare(spi_imx->clk_per);
+out_runtime_pm_put:
+	pm_runtime_dont_use_autosuspend(spi_imx->dev);
+	pm_runtime_put_sync(spi_imx->dev);
+	pm_runtime_disable(spi_imx->dev);
 out_master_put:
 	spi_master_put(master);
 
@@ -1732,30 +1735,82 @@  static int spi_imx_remove(struct platform_device *pdev)
 
 	spi_bitbang_stop(&spi_imx->bitbang);
 
-	ret = clk_enable(spi_imx->clk_per);
+	ret = pm_runtime_get_sync(spi_imx->dev);
+	if (ret < 0) {
+		dev_err(spi_imx->dev, "failed to enable clock\n");
+		return ret;
+	}
+
+	writel(0, spi_imx->base + MXC_CSPICTRL);
+
+	pm_runtime_dont_use_autosuspend(spi_imx->dev);
+	pm_runtime_put_sync(spi_imx->dev);
+	pm_runtime_disable(spi_imx->dev);
+
+	spi_imx_sdma_exit(spi_imx);
+	spi_master_put(master);
+
+	return 0;
+}
+
+static int __maybe_unused spi_imx_runtime_resume(struct device *dev)
+{
+	struct spi_master *master = dev_get_drvdata(dev);
+	struct spi_imx_data *spi_imx;
+	int ret;
+
+	spi_imx = spi_master_get_devdata(master);
+
+	ret = clk_prepare_enable(spi_imx->clk_per);
 	if (ret)
 		return ret;
 
-	ret = clk_enable(spi_imx->clk_ipg);
+	ret = clk_prepare_enable(spi_imx->clk_ipg);
 	if (ret) {
-		clk_disable(spi_imx->clk_per);
+		clk_disable_unprepare(spi_imx->clk_per);
 		return ret;
 	}
 
-	writel(0, spi_imx->base + MXC_CSPICTRL);
-	clk_disable_unprepare(spi_imx->clk_ipg);
+	return 0;
+}
+
+static int __maybe_unused spi_imx_runtime_suspend(struct device *dev)
+{
+	struct spi_master *master = dev_get_drvdata(dev);
+	struct spi_imx_data *spi_imx;
+
+	spi_imx = spi_master_get_devdata(master);
+
 	clk_disable_unprepare(spi_imx->clk_per);
-	spi_imx_sdma_exit(spi_imx);
-	spi_master_put(master);
+	clk_disable_unprepare(spi_imx->clk_ipg);
+
+	return 0;
+}
 
+static int __maybe_unused spi_imx_suspend(struct device *dev)
+{
+	pinctrl_pm_select_sleep_state(dev);
 	return 0;
 }
 
+static int __maybe_unused spi_imx_resume(struct device *dev)
+{
+	pinctrl_pm_select_default_state(dev);
+	return 0;
+}
+
+static const struct dev_pm_ops imx_spi_pm = {
+	SET_RUNTIME_PM_OPS(spi_imx_runtime_suspend,
+				spi_imx_runtime_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(spi_imx_suspend, spi_imx_resume)
+};
+
 static struct platform_driver spi_imx_driver = {
 	.driver = {
 		   .name = DRIVER_NAME,
 		   .of_match_table = spi_imx_dt_ids,
-		   },
+		   .pm = &imx_spi_pm,
+	},
 	.id_table = spi_imx_devtype,
 	.probe = spi_imx_probe,
 	.remove = spi_imx_remove,