Messages in this thread | | | Subject | Re: [PATCH] [v4] spi: spi-zynqmp-gqspi: Fix runtime PM imbalance in zynqmp_qspi_probe | From | "quanyang.wang" <> | Date | Thu, 15 Apr 2021 16:19:27 +0800 |
| |
On 4/15/21 3:46 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. > > v4: - Add error message on failure of pm_runtime_get_sync(). > --- > drivers/spi/spi-zynqmp-gqspi.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/spi/spi-zynqmp-gqspi.c b/drivers/spi/spi-zynqmp-gqspi.c > index c8fa6ee18ae7..38f3ddd3ea7c 100644 > --- a/drivers/spi/spi-zynqmp-gqspi.c > +++ b/drivers/spi/spi-zynqmp-gqspi.c > @@ -1160,11 +1160,16 @@ 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) { > + dev_err(&pdev->dev, "Failed to pm_runtime_get_sync: %d\n", ret); > + 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 +1192,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 +1200,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);
Test this patch at zcu102 board:
root@xilinx-zynqmp:~# dmesg | grep domain2 [ 0.905407] zynqmp_gpd_attach_dev() ff0f0000.spi attached to domain2 domain [ 0.912390] zynqmp_gpd_power_on() Powered on domain2 domain [ 4.350331] zynqmp_gpd_power_off() Powered off domain2 domain root@xilinx-zynqmp:~# flash_erase /dev/mtd3 0 0 Erasing 4 Kibyte @ 0 -- 0 % complete [ 153.125894] zynqmp_gpd_power_on() Powered on domain2 domain Erasing 4 Kibyte @ 2e8000 -- 49 % complete [ 156.134884] zynqmp_gpd_power_off() Powered off domain2 domain [ 156.142648] zynqmp_gpd_power_on() Powered on domain2 domain Erasing 4 Kibyte @ 5d5000 -- 99 % complete [ 159.148329] zynqmp_gpd_power_off() Powered off domain2 domain [ 159.154579] zynqmp_gpd_power_on() Powered on domain2 domain Erasing 4 Kibyte @ 5df000 -- 100 % complete root@xilinx-zynqmp:~# [ 162.910329] zynqmp_gpd_power_off() Powered off domain2 domain
pm_runtime works now. So
Tested-by: Quanyang Wang <quanyang.wang@windriver.com>
Regards,
Quanyang
| |