Messages in this thread Patch in this message | | | Subject | Re: [PATCH] [v3] spi: spi-zynqmp-gqspi: Fix runtime PM imbalance in zynqmp_qspi_probe | From | "quanyang.wang" <> | Date | Thu, 15 Apr 2021 15:27:51 +0800 |
| |
Hi Dinghao,
On 4/15/21 3:17 PM, Dinghao Liu wrote: > There is a PM usage counter decrement after zynqmp_qspi_init_hw() > without any refcount increment, which leads to refcount leak.Add > a refcount increment to balance the refcount. Also set > auto_runtime_pm to resume suspended spi controller. > > Fixes: 9e3a000362aec ("spi: zynqmp: Add pm runtime support") > Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn> > --- > > Changelog: > > v2: - Add a refcount increment to fix refcout leak instead of the > refcount decrement on error. > Set ctlr->auto_runtime_pm = true. > > v3: - Add fix tag. > Add a return value check against pm_runtime_get_sync(). > Move pm_runtime_{mark_last_busy & put_autosuspend} to the > end of current function. > --- > drivers/spi/spi-zynqmp-gqspi.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/spi/spi-zynqmp-gqspi.c b/drivers/spi/spi-zynqmp-gqspi.c > index c8fa6ee18ae7..781ef3fc76e2 100644 > --- a/drivers/spi/spi-zynqmp-gqspi.c > +++ b/drivers/spi/spi-zynqmp-gqspi.c > @@ -1160,11 +1160,14 @@ static int zynqmp_qspi_probe(struct platform_device *pdev) > pm_runtime_set_autosuspend_delay(&pdev->dev, SPI_AUTOSUSPEND_TIMEOUT); > pm_runtime_set_active(&pdev->dev); > pm_runtime_enable(&pdev->dev); > + > + ret = pm_runtime_get_sync(&pdev->dev); > + if (ret < 0) > + goto clk_dis_all;
Would you please add some error info output here? Just like:
+ if (ret < 0) { + dev_err(&pdev->dev, "failed to pm_runtime_get_sync: %d\n", ret); + goto clk_dis_all; + } Because sometimes it is very helpful to analyse probe failure.
Thanks,
Quanyang
> + > /* QSPI controller initializations */ > zynqmp_qspi_init_hw(xqspi); > > - pm_runtime_mark_last_busy(&pdev->dev); > - pm_runtime_put_autosuspend(&pdev->dev); > xqspi->irq = platform_get_irq(pdev, 0); > if (xqspi->irq <= 0) { > ret = -ENXIO; > @@ -1187,6 +1190,7 @@ static int zynqmp_qspi_probe(struct platform_device *pdev) > ctlr->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | SPI_RX_QUAD | > SPI_TX_DUAL | SPI_TX_QUAD; > ctlr->dev.of_node = np; > + ctlr->auto_runtime_pm = true; > > ret = devm_spi_register_controller(&pdev->dev, ctlr); > if (ret) { > @@ -1194,9 +1198,13 @@ static int zynqmp_qspi_probe(struct platform_device *pdev) > goto clk_dis_all; > } > > + pm_runtime_mark_last_busy(&pdev->dev); > + pm_runtime_put_autosuspend(&pdev->dev); > + > return 0; > > clk_dis_all: > + pm_runtime_put_sync(&pdev->dev); > pm_runtime_set_suspended(&pdev->dev); > pm_runtime_disable(&pdev->dev); > clk_disable_unprepare(xqspi->refclk);
| |