| Message ID | 20200602045506.40574-1-navid.emamdoost@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spi: tegra114: missing put on pm_runtime_get_sync failure | expand |
On 02/06/2020 05:55, Navid Emamdoost wrote: > the call to pm_runtime_get_sync increments the counter even > in case of failure leading to incorrect ref count. > Call pm_runtime_put if pm_runtime_get_sync fails. > > Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com> > --- > drivers/spi/spi-tegra114.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/spi/spi-tegra114.c b/drivers/spi/spi-tegra114.c > index 83edabdb41ad..dccd2ac1a975 100644 > --- a/drivers/spi/spi-tegra114.c > +++ b/drivers/spi/spi-tegra114.c > @@ -974,6 +974,7 @@ static int tegra_spi_setup(struct spi_device *spi) > dev_err(tspi->dev, "pm runtime failed, e = %d\n", ret); > if (cdata) > tegra_spi_cleanup(spi); > + pm_runtime_put(tspi->dev); > return ret; > } > > @@ -1398,6 +1399,7 @@ static int tegra_spi_probe(struct platform_device *pdev) > ret = pm_runtime_get_sync(&pdev->dev); > if (ret < 0) { > dev_err(&pdev->dev, "pm runtime get failed, e = %d\n", ret); > + pm_runtime_put(&pdev->dev); > goto exit_pm_disable; > } I am wondering if it is better we use put_sync() here to ensure that this happens before we exit the probe. Jon
On Fri, Jun 5, 2020 at 1:09 AM Jon Hunter <jonathanh@nvidia.com> wrote: > > > On 02/06/2020 05:55, Navid Emamdoost wrote: > > the call to pm_runtime_get_sync increments the counter even > > in case of failure leading to incorrect ref count. > > Call pm_runtime_put if pm_runtime_get_sync fails. > > > > Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com> > > --- > > drivers/spi/spi-tegra114.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/spi/spi-tegra114.c b/drivers/spi/spi-tegra114.c > > index 83edabdb41ad..dccd2ac1a975 100644 > > --- a/drivers/spi/spi-tegra114.c > > +++ b/drivers/spi/spi-tegra114.c > > @@ -974,6 +974,7 @@ static int tegra_spi_setup(struct spi_device *spi) > > dev_err(tspi->dev, "pm runtime failed, e = %d\n", ret); > > if (cdata) > > tegra_spi_cleanup(spi); > > + pm_runtime_put(tspi->dev); > > return ret; > > } > > > > @@ -1398,6 +1399,7 @@ static int tegra_spi_probe(struct platform_device *pdev) > > ret = pm_runtime_get_sync(&pdev->dev); > > if (ret < 0) { > > dev_err(&pdev->dev, "pm runtime get failed, e = %d\n", ret); > > + pm_runtime_put(&pdev->dev); > > goto exit_pm_disable; > > } > > I am wondering if it is better we use put_sync() here to ensure that > this happens before we exit the probe. To be honest I am not sure when to use different flavors of pm_runtime_put (like pm_runtime_put_noidle, pm_runtime_put_autosuspend, pm_runtime_put_sync, pm_runtime_put, pm_runtime_put_sync_suspend). I'd appreciate it if you could give me a pointer on how to decide on this. > > Jon > > -- > nvpublic
diff --git a/drivers/spi/spi-tegra114.c b/drivers/spi/spi-tegra114.c index 83edabdb41ad..dccd2ac1a975 100644 --- a/drivers/spi/spi-tegra114.c +++ b/drivers/spi/spi-tegra114.c @@ -974,6 +974,7 @@ static int tegra_spi_setup(struct spi_device *spi) dev_err(tspi->dev, "pm runtime failed, e = %d\n", ret); if (cdata) tegra_spi_cleanup(spi); + pm_runtime_put(tspi->dev); return ret; } @@ -1398,6 +1399,7 @@ static int tegra_spi_probe(struct platform_device *pdev) ret = pm_runtime_get_sync(&pdev->dev); if (ret < 0) { dev_err(&pdev->dev, "pm runtime get failed, e = %d\n", ret); + pm_runtime_put(&pdev->dev); goto exit_pm_disable; } @@ -1479,6 +1481,7 @@ static int tegra_spi_resume(struct device *dev) ret = pm_runtime_get_sync(dev); if (ret < 0) { dev_err(dev, "pm runtime failed, e = %d\n", ret); + pm_runtime_put(dev); return ret; } tegra_spi_writel(tspi, tspi->command1_reg, SPI_COMMAND1);
the call to pm_runtime_get_sync increments the counter even in case of failure leading to incorrect ref count. Call pm_runtime_put if pm_runtime_get_sync fails. Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com> --- drivers/spi/spi-tegra114.c | 3 +++ 1 file changed, 3 insertions(+)