| Message ID | 8688555079cf30f5848bb020b5ecf0b0132b2c7e.1620207353.git.mchehab+huawei@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix some PM runtime issues at the media subsystem | expand |
On Wed, 5 May 2021 11:42:06 +0200 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > The pm_runtime_get_sync() internally increments the > dev->power.usage_count without decrementing it, even on errors. > Replace it by the new pm_runtime_resume_and_get(), introduced by: > commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter") > in order to properly decrement the usage counter, avoiding > a potential PM usage counter leak. > > While here, ensure that the driver will check if PM runtime > resumed at vpfe_initialize_device(). > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/media/platform/am437x/am437x-vpfe.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c > index 6cdc77dda0e4..1c9cb9e05fdf 100644 > --- a/drivers/media/platform/am437x/am437x-vpfe.c > +++ b/drivers/media/platform/am437x/am437x-vpfe.c > @@ -1021,7 +1021,9 @@ static int vpfe_initialize_device(struct vpfe_device *vpfe) > if (ret) > return ret; > > - pm_runtime_get_sync(vpfe->pdev); > + ret = pm_runtime_resume_and_get(vpfe->pdev); > + if (ret < 0) > + return ret; > > vpfe_config_enable(&vpfe->ccdc, 1); > > @@ -2443,7 +2445,11 @@ static int vpfe_probe(struct platform_device *pdev) > pm_runtime_enable(&pdev->dev); > > /* for now just enable it here instead of waiting for the open */ > - pm_runtime_get_sync(&pdev->dev); > + ret = pm_runtime_resume_and_get(&pdev->dev); > + if (ret < 0) { > + vpfe_err(vpfe, "Unable to resume device.\n"); > + goto probe_out_v4l2_unregister; > + } > > vpfe_ccdc_config_defaults(ccdc); > > @@ -2530,6 +2536,11 @@ static int vpfe_suspend(struct device *dev) > > /* only do full suspend if streaming has started */ > if (vb2_start_streaming_called(&vpfe->buffer_queue)) { > + /* > + * ignore RPM resume errors here, as it is already too late. > + * A check like that should happen earlier, either at > + * open() or just before start streaming. > + */ > pm_runtime_get_sync(dev); > vpfe_config_enable(ccdc, 1); >
Hi Mauro, Thank you for the patch. On Wed, May 5, 2021 at 10:42 AM Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > > The pm_runtime_get_sync() internally increments the > dev->power.usage_count without decrementing it, even on errors. > Replace it by the new pm_runtime_resume_and_get(), introduced by: > commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter") > in order to properly decrement the usage counter, avoiding > a potential PM usage counter leak. > > While here, ensure that the driver will check if PM runtime > resumed at vpfe_initialize_device(). > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > --- > drivers/media/platform/am437x/am437x-vpfe.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > Acked-by: Lad Prabhakar <prabhakar.csengg@gmail.com> Cheers, Prabhakar > diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c > index 6cdc77dda0e4..1c9cb9e05fdf 100644 > --- a/drivers/media/platform/am437x/am437x-vpfe.c > +++ b/drivers/media/platform/am437x/am437x-vpfe.c > @@ -1021,7 +1021,9 @@ static int vpfe_initialize_device(struct vpfe_device *vpfe) > if (ret) > return ret; > > - pm_runtime_get_sync(vpfe->pdev); > + ret = pm_runtime_resume_and_get(vpfe->pdev); > + if (ret < 0) > + return ret; > > vpfe_config_enable(&vpfe->ccdc, 1); > > @@ -2443,7 +2445,11 @@ static int vpfe_probe(struct platform_device *pdev) > pm_runtime_enable(&pdev->dev); > > /* for now just enable it here instead of waiting for the open */ > - pm_runtime_get_sync(&pdev->dev); > + ret = pm_runtime_resume_and_get(&pdev->dev); > + if (ret < 0) { > + vpfe_err(vpfe, "Unable to resume device.\n"); > + goto probe_out_v4l2_unregister; > + } > > vpfe_ccdc_config_defaults(ccdc); > > @@ -2530,6 +2536,11 @@ static int vpfe_suspend(struct device *dev) > > /* only do full suspend if streaming has started */ > if (vb2_start_streaming_called(&vpfe->buffer_queue)) { > + /* > + * ignore RPM resume errors here, as it is already too late. > + * A check like that should happen earlier, either at > + * open() or just before start streaming. > + */ > pm_runtime_get_sync(dev); > vpfe_config_enable(ccdc, 1); > > -- > 2.30.2 >
diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c index 6cdc77dda0e4..1c9cb9e05fdf 100644 --- a/drivers/media/platform/am437x/am437x-vpfe.c +++ b/drivers/media/platform/am437x/am437x-vpfe.c @@ -1021,7 +1021,9 @@ static int vpfe_initialize_device(struct vpfe_device *vpfe) if (ret) return ret; - pm_runtime_get_sync(vpfe->pdev); + ret = pm_runtime_resume_and_get(vpfe->pdev); + if (ret < 0) + return ret; vpfe_config_enable(&vpfe->ccdc, 1); @@ -2443,7 +2445,11 @@ static int vpfe_probe(struct platform_device *pdev) pm_runtime_enable(&pdev->dev); /* for now just enable it here instead of waiting for the open */ - pm_runtime_get_sync(&pdev->dev); + ret = pm_runtime_resume_and_get(&pdev->dev); + if (ret < 0) { + vpfe_err(vpfe, "Unable to resume device.\n"); + goto probe_out_v4l2_unregister; + } vpfe_ccdc_config_defaults(ccdc); @@ -2530,6 +2536,11 @@ static int vpfe_suspend(struct device *dev) /* only do full suspend if streaming has started */ if (vb2_start_streaming_called(&vpfe->buffer_queue)) { + /* + * ignore RPM resume errors here, as it is already too late. + * A check like that should happen earlier, either at + * open() or just before start streaming. + */ pm_runtime_get_sync(dev); vpfe_config_enable(ccdc, 1);
The pm_runtime_get_sync() internally increments the dev->power.usage_count without decrementing it, even on errors. Replace it by the new pm_runtime_resume_and_get(), introduced by: commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter") in order to properly decrement the usage counter, avoiding a potential PM usage counter leak. While here, ensure that the driver will check if PM runtime resumed at vpfe_initialize_device(). Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> --- drivers/media/platform/am437x/am437x-vpfe.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)