2021-08-19 13:24:11

by Yang Yingliang

[permalink] [raw]
Subject: [PATCH -next] iio: adc: add missing clk_disable_unprepare() in rzg2l_adc_pm_runtime_resume()

Add clk_disable_unprepare() on error path in rzg2l_adc_pm_runtime_resume().

Reported-by: Hulk Robot <[email protected]>
Signed-off-by: Yang Yingliang <[email protected]>
---
drivers/iio/adc/rzg2l_adc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
index 9996d5eef289..c38f43ea624f 100644
--- a/drivers/iio/adc/rzg2l_adc.c
+++ b/drivers/iio/adc/rzg2l_adc.c
@@ -570,8 +570,10 @@ static int __maybe_unused rzg2l_adc_pm_runtime_resume(struct device *dev)
return ret;

ret = clk_prepare_enable(adc->adclk);
- if (ret)
+ if (ret) {
+ clk_disable_unprepare(adc->pclk);
return ret;
+ }

rzg2l_adc_pwr(adc, true);

--
2.25.1


2021-08-19 17:26:59

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH -next] iio: adc: add missing clk_disable_unprepare() in rzg2l_adc_pm_runtime_resume()

On Thu, Aug 19, 2021 at 4:19 PM Yang Yingliang <[email protected]> wrote:
>
> Add clk_disable_unprepare() on error path in rzg2l_adc_pm_runtime_resume().

...

> ret = clk_prepare_enable(adc->adclk);
> - if (ret)
> + if (ret) {
> + clk_disable_unprepare(adc->pclk);
> return ret;
> + }

Huh?!

--
With Best Regards,
Andy Shevchenko

2021-08-20 01:54:20

by Yang Yingliang

[permalink] [raw]
Subject: Re: [PATCH -next] iio: adc: add missing clk_disable_unprepare() in rzg2l_adc_pm_runtime_resume()

Hi,

On 2021/8/20 1:20, Andy Shevchenko wrote:
> On Thu, Aug 19, 2021 at 4:19 PM Yang Yingliang <[email protected]> wrote:
>> Add clk_disable_unprepare() on error path in rzg2l_adc_pm_runtime_resume().
> ...
>
>> ret = clk_prepare_enable(adc->adclk);
>> - if (ret)
>> + if (ret) {
>> + clk_disable_unprepare(adc->pclk);
>> return ret;
>> + }
> Huh?!
The pclk need be disabled, when enable adclk failed.
      ^                                                         ^^
>

2021-08-20 06:53:25

by Nuno Sa

[permalink] [raw]
Subject: RE: [PATCH -next] iio: adc: add missing clk_disable_unprepare() in rzg2l_adc_pm_runtime_resume()

> From: Andy Shevchenko <[email protected]>
> Sent: Thursday, August 19, 2021 7:21 PM
> To: Yang Yingliang <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH -next] iio: adc: add missing
> clk_disable_unprepare() in rzg2l_adc_pm_runtime_resume()
>
> On Thu, Aug 19, 2021 at 4:19 PM Yang Yingliang
> <[email protected]> wrote:
> >
> > Add clk_disable_unprepare() on error path in
> rzg2l_adc_pm_runtime_resume().
>
> ...
>
> > ret = clk_prepare_enable(adc->adclk);
> > - if (ret)
> > + if (ret) {
> > + clk_disable_unprepare(adc->pclk);
> > return ret;
> > + }
>
> Huh?!
>

Had the same reaction when looked at this patch. Look at the
clock names :).

- Nuno Sá

2021-08-20 09:23:05

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH -next] iio: adc: add missing clk_disable_unprepare() in rzg2l_adc_pm_runtime_resume()

On Fri, Aug 20, 2021 at 4:52 AM Yang Yingliang <[email protected]> wrote:
> On 2021/8/20 1:20, Andy Shevchenko wrote:
> > On Thu, Aug 19, 2021 at 4:19 PM Yang Yingliang <[email protected]> wrote:
> >> Add clk_disable_unprepare() on error path in rzg2l_adc_pm_runtime_resume().
> > ...
> >
> >> ret = clk_prepare_enable(adc->adclk);
> >> - if (ret)
> >> + if (ret) {
> >> + clk_disable_unprepare(adc->pclk);
> >> return ret;
> >> + }
> > Huh?!
> The pclk need be disabled, when enable adclk failed.
> ^ ^^

Indeed. I'm wondering if those clocks behave like a bulk or any
combination is possible on a working case?

--
With Best Regards,
Andy Shevchenko

2021-08-20 14:07:02

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: RE: [PATCH -next] iio: adc: add missing clk_disable_unprepare() in rzg2l_adc_pm_runtime_resume()

Hi Yang,

Thank you for the patch.

> -----Original Message-----
> From: Yang Yingliang <[email protected]>
> Sent: 19 August 2021 14:24
> To: [email protected]; [email protected]
> Cc: Prabhakar Mahadev Lad <[email protected]>; [email protected]
> Subject: [PATCH -next] iio: adc: add missing clk_disable_unprepare() in rzg2l_adc_pm_runtime_resume()
>
> Add clk_disable_unprepare() on error path in rzg2l_adc_pm_runtime_resume().
>
> Reported-by: Hulk Robot <[email protected]>
> Signed-off-by: Yang Yingliang <[email protected]>
> ---
> drivers/iio/adc/rzg2l_adc.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
With subject line changed to, iio: adc: rzg2l_adc: add missing clk_disable_unprepare() in rzg2l_adc_pm_runtime_resume()

Reviewed-by: Lad Prabhakar <[email protected]>

Cheers,
Prabhakar

> diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c index
> 9996d5eef289..c38f43ea624f 100644
> --- a/drivers/iio/adc/rzg2l_adc.c
> +++ b/drivers/iio/adc/rzg2l_adc.c
> @@ -570,8 +570,10 @@ static int __maybe_unused rzg2l_adc_pm_runtime_resume(struct device *dev)
> return ret;
>
> ret = clk_prepare_enable(adc->adclk);
> - if (ret)
> + if (ret) {
> + clk_disable_unprepare(adc->pclk);
> return ret;
> + }
>
> rzg2l_adc_pwr(adc, true);
>
> --
> 2.25.1

2021-08-30 10:57:11

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH -next] iio: adc: add missing clk_disable_unprepare() in rzg2l_adc_pm_runtime_resume()

On Fri, 20 Aug 2021 12:17:46 +0300
Andy Shevchenko <[email protected]> wrote:

> On Fri, Aug 20, 2021 at 4:52 AM Yang Yingliang <[email protected]> wrote:
> > On 2021/8/20 1:20, Andy Shevchenko wrote:
> > > On Thu, Aug 19, 2021 at 4:19 PM Yang Yingliang <[email protected]> wrote:
> > >> Add clk_disable_unprepare() on error path in rzg2l_adc_pm_runtime_resume().
> > > ...
> > >
> > >> ret = clk_prepare_enable(adc->adclk);
> > >> - if (ret)
> > >> + if (ret) {
> > >> + clk_disable_unprepare(adc->pclk);
> > >> return ret;
> > >> + }
> > > Huh?!
> > The pclk need be disabled, when enable adclk failed.
> > ^ ^^
>
> Indeed. I'm wondering if those clocks behave like a bulk or any
> combination is possible on a working case?

They are handled independently in other parts of the driver, so bulk
setting is going to be a mess.

>

2021-08-30 10:58:45

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH -next] iio: adc: add missing clk_disable_unprepare() in rzg2l_adc_pm_runtime_resume()

On Fri, 20 Aug 2021 14:04:15 +0000
Prabhakar Mahadev Lad <[email protected]> wrote:

> Hi Yang,
>
> Thank you for the patch.
>
> > -----Original Message-----
> > From: Yang Yingliang <[email protected]>
> > Sent: 19 August 2021 14:24
> > To: [email protected]; [email protected]
> > Cc: Prabhakar Mahadev Lad <[email protected]>; [email protected]
> > Subject: [PATCH -next] iio: adc: add missing clk_disable_unprepare() in rzg2l_adc_pm_runtime_resume()
> >
> > Add clk_disable_unprepare() on error path in rzg2l_adc_pm_runtime_resume().
> >
> > Reported-by: Hulk Robot <[email protected]>
> > Signed-off-by: Yang Yingliang <[email protected]>
> > ---
> > drivers/iio/adc/rzg2l_adc.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> With subject line changed to, iio: adc: rzg2l_adc: add missing clk_disable_unprepare() in rzg2l_adc_pm_runtime_resume()
>
> Reviewed-by: Lad Prabhakar <[email protected]>

Patch title updated as suggested and applied to the fixes-togreg branch of iio.git
which will go upstream sometime after rc1.

Thanks,

Jonathan

>
> Cheers,
> Prabhakar
>
> > diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c index
> > 9996d5eef289..c38f43ea624f 100644
> > --- a/drivers/iio/adc/rzg2l_adc.c
> > +++ b/drivers/iio/adc/rzg2l_adc.c
> > @@ -570,8 +570,10 @@ static int __maybe_unused rzg2l_adc_pm_runtime_resume(struct device *dev)
> > return ret;
> >
> > ret = clk_prepare_enable(adc->adclk);
> > - if (ret)
> > + if (ret) {
> > + clk_disable_unprepare(adc->pclk);
> > return ret;
> > + }
> >
> > rzg2l_adc_pwr(adc, true);
> >
> > --
> > 2.25.1
>