| Message ID | 20240904021943.2076343-3-ruanjinjie@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | spi: geni-qcom: Undo runtime PM changes at driver exit time | expand |
Hi, On Tue, Sep 3, 2024 at 7:11 PM Jinjie Ruan <ruanjinjie@huawei.com> wrote: > > It's important to undo pm_runtime_use_autosuspend() with > pm_runtime_dont_use_autosuspend() at driver exit time unless driver > initially enabled pm_runtime with devm_pm_runtime_enable() > (which handles it for you). > > Hence, switch to devm_pm_runtime_enable() to fix it. > > Fixes: cfdab2cd85ec ("spi: spi-geni-qcom: Set an autosuspend delay of 250 ms") > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> > --- > drivers/spi/spi-geni-qcom.c | 13 +++++-------- > 1 file changed, 5 insertions(+), 8 deletions(-) Given that this is a "Fix" it should be the first patch in the series. Your ${SUBJECT} should also not contain "-next". Not only is that not a convention for the SPI tree (that I'm aware of) it also is probably wrong since fixes should usually get queued to an earlier tree... As per my response to patch #1 in your series, the pm_runtime fix also needs to be first in your series for correctness. > @@ -1153,10 +1153,9 @@ static int spi_geni_probe(struct platform_device *pdev) > goto spi_geni_release_dma; > > return 0; > + Unrelated whitespace change? -Doug
diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c index 77eb874e4f54..e5bece7be892 100644 --- a/drivers/spi/spi-geni-qcom.c +++ b/drivers/spi/spi-geni-qcom.c @@ -1110,25 +1110,25 @@ static int spi_geni_probe(struct platform_device *pdev) spin_lock_init(&mas->lock); pm_runtime_use_autosuspend(&pdev->dev); pm_runtime_set_autosuspend_delay(&pdev->dev, 250); - pm_runtime_enable(dev); + devm_pm_runtime_enable(dev); if (device_property_read_bool(&pdev->dev, "spi-slave")) spi->target = true; ret = geni_icc_get(&mas->se, NULL); if (ret) - goto spi_geni_probe_runtime_disable; + return ret; /* Set the bus quota to a reasonable value for register access */ mas->se.icc_paths[GENI_TO_CORE].avg_bw = Bps_to_icc(CORE_2X_50_MHZ); mas->se.icc_paths[CPU_TO_GENI].avg_bw = GENI_DEFAULT_BW; ret = geni_icc_set_bw(&mas->se); if (ret) - goto spi_geni_probe_runtime_disable; + return ret; ret = spi_geni_init(mas); if (ret) - goto spi_geni_probe_runtime_disable; + return ret; /* * check the mode supported and set_cs for fifo mode only @@ -1153,10 +1153,9 @@ static int spi_geni_probe(struct platform_device *pdev) goto spi_geni_release_dma; return 0; + spi_geni_release_dma: spi_geni_release_dma_chan(mas); -spi_geni_probe_runtime_disable: - pm_runtime_disable(dev); return ret; } @@ -1169,8 +1168,6 @@ static void spi_geni_remove(struct platform_device *pdev) spi_unregister_controller(spi); spi_geni_release_dma_chan(mas); - - pm_runtime_disable(&pdev->dev); } static int __maybe_unused spi_geni_runtime_suspend(struct device *dev)
It's important to undo pm_runtime_use_autosuspend() with pm_runtime_dont_use_autosuspend() at driver exit time unless driver initially enabled pm_runtime with devm_pm_runtime_enable() (which handles it for you). Hence, switch to devm_pm_runtime_enable() to fix it. Fixes: cfdab2cd85ec ("spi: spi-geni-qcom: Set an autosuspend delay of 250 ms") Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> --- drivers/spi/spi-geni-qcom.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)