| Message ID | 20210819132416.175644-1-yangyingliang@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [-next] iio: adc: add missing clk_disable_unprepare() in rzg2l_adc_pm_runtime_resume() | expand |
On Thu, Aug 19, 2021 at 4:19 PM Yang Yingliang <yangyingliang@huawei.com> 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?!
Hi, On 2021/8/20 1:20, Andy Shevchenko wrote: > On Thu, Aug 19, 2021 at 4:19 PM Yang Yingliang <yangyingliang@huawei.com> 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. ^ ^^ >
> From: Andy Shevchenko <andy.shevchenko@gmail.com> > Sent: Thursday, August 19, 2021 7:21 PM > To: Yang Yingliang <yangyingliang@huawei.com> > Cc: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org; > prabhakar.mahadev-lad.rj@bp.renesas.com; jic23@kernel.org > 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 > <yangyingliang@huawei.com> 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á
On Fri, Aug 20, 2021 at 4:52 AM Yang Yingliang <yangyingliang@huawei.com> wrote: > On 2021/8/20 1:20, Andy Shevchenko wrote: > > On Thu, Aug 19, 2021 at 4:19 PM Yang Yingliang <yangyingliang@huawei.com> 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?
Hi Yang, Thank you for the patch. > -----Original Message----- > From: Yang Yingliang <yangyingliang@huawei.com> > Sent: 19 August 2021 14:24 > To: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org > Cc: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; jic23@kernel.org > 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 <hulkci@huawei.com> > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > --- > 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 <prabhakar.mahadev-lad.rj@bp.renesas.com> 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
On Fri, 20 Aug 2021 12:17:46 +0300 Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Fri, Aug 20, 2021 at 4:52 AM Yang Yingliang <yangyingliang@huawei.com> wrote: > > On 2021/8/20 1:20, Andy Shevchenko wrote: > > > On Thu, Aug 19, 2021 at 4:19 PM Yang Yingliang <yangyingliang@huawei.com> 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. >
On Fri, 20 Aug 2021 14:04:15 +0000 Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > Hi Yang, > > Thank you for the patch. > > > -----Original Message----- > > From: Yang Yingliang <yangyingliang@huawei.com> > > Sent: 19 August 2021 14:24 > > To: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org > > Cc: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; jic23@kernel.org > > 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 <hulkci@huawei.com> > > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > > --- > > 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 <prabhakar.mahadev-lad.rj@bp.renesas.com> 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 >
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);
Add clk_disable_unprepare() on error path in rzg2l_adc_pm_runtime_resume(). Reported-by: Hulk Robot <hulkci@huawei.com> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- drivers/iio/adc/rzg2l_adc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)