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 |
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
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 ?
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
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
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 --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,
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(-)