| Message ID | 20210415071714.19334-1-dinghao.liu@zju.edu.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] spi: spi-zynqmp-gqspi: Fix runtime PM imbalance in zynqmp_qspi_probe | expand |
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);
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; + /* 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);
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(-)