* [PATCH] iio: adc: ad7192: Avoid disabling a clock that was never enabled.
@ 2021-05-09 11:41 Jonathan Cameron
2021-05-10 11:30 ` Alexandru Ardelean
0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Cameron @ 2021-05-09 11:41 UTC (permalink / raw)
To: linux-iio; +Cc: Jonathan Cameron, Alexandru Tachici, Alexandru Ardelean
From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Found by inspection.
If the internal clock source is being used, the driver doesn't
call clk_prepare_enable() and as such we should not call
clk_disable_unprepare()
Use the same condition to protect the disable path as is used
on the enable one. Note this will all get simplified when
the driver moves over to a full devm_ flow, but that would make
backporting the fix harder.
Fix obviously predates move out of staging, but backporting will
become more complex (and is unlikely to happen), hence that patch
is given in the fixes tag.
Fixes: b581f748cce0 ("staging: iio: adc: ad7192: move out of staging")
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Alexandru Tachici <alexandru.tachici@analog.com>
Cc: Alexandru Ardelean <ardeleanalex@gmail.com>
---
drivers/iio/adc/ad7192.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index 2ed580521d81..d3be67aa0522 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -1014,7 +1014,9 @@ static int ad7192_probe(struct spi_device *spi)
return 0;
error_disable_clk:
- clk_disable_unprepare(st->mclk);
+ if (st->clock_sel == AD7192_CLK_EXT_MCLK1_2 ||
+ st->clock_sel == AD7192_CLK_EXT_MCLK2)
+ clk_disable_unprepare(st->mclk);
error_remove_trigger:
ad_sd_cleanup_buffer_and_trigger(indio_dev);
error_disable_dvdd:
@@ -1031,7 +1033,9 @@ static int ad7192_remove(struct spi_device *spi)
struct ad7192_state *st = iio_priv(indio_dev);
iio_device_unregister(indio_dev);
- clk_disable_unprepare(st->mclk);
+ if (st->clock_sel == AD7192_CLK_EXT_MCLK1_2 ||
+ st->clock_sel == AD7192_CLK_EXT_MCLK2)
+ clk_disable_unprepare(st->mclk);
ad_sd_cleanup_buffer_and_trigger(indio_dev);
regulator_disable(st->dvdd);
--
2.31.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] iio: adc: ad7192: Avoid disabling a clock that was never enabled.
2021-05-09 11:41 [PATCH] iio: adc: ad7192: Avoid disabling a clock that was never enabled Jonathan Cameron
@ 2021-05-10 11:30 ` Alexandru Ardelean
2021-05-10 11:56 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Alexandru Ardelean @ 2021-05-10 11:30 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, Jonathan Cameron, Alexandru Tachici
On Sun, May 9, 2021 at 2:42 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Found by inspection.
>
> If the internal clock source is being used, the driver doesn't
> call clk_prepare_enable() and as such we should not call
> clk_disable_unprepare()
>
> Use the same condition to protect the disable path as is used
> on the enable one. Note this will all get simplified when
> the driver moves over to a full devm_ flow, but that would make
> backporting the fix harder.
>
> Fix obviously predates move out of staging, but backporting will
> become more complex (and is unlikely to happen), hence that patch
> is given in the fixes tag.
>
This also looks like a conversion to devm_ would help.
But later.
Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com>
> Fixes: b581f748cce0 ("staging: iio: adc: ad7192: move out of staging")
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Alexandru Tachici <alexandru.tachici@analog.com>
> Cc: Alexandru Ardelean <ardeleanalex@gmail.com>
> ---
> drivers/iio/adc/ad7192.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
> index 2ed580521d81..d3be67aa0522 100644
> --- a/drivers/iio/adc/ad7192.c
> +++ b/drivers/iio/adc/ad7192.c
> @@ -1014,7 +1014,9 @@ static int ad7192_probe(struct spi_device *spi)
> return 0;
>
> error_disable_clk:
> - clk_disable_unprepare(st->mclk);
> + if (st->clock_sel == AD7192_CLK_EXT_MCLK1_2 ||
> + st->clock_sel == AD7192_CLK_EXT_MCLK2)
> + clk_disable_unprepare(st->mclk);
> error_remove_trigger:
> ad_sd_cleanup_buffer_and_trigger(indio_dev);
> error_disable_dvdd:
> @@ -1031,7 +1033,9 @@ static int ad7192_remove(struct spi_device *spi)
> struct ad7192_state *st = iio_priv(indio_dev);
>
> iio_device_unregister(indio_dev);
> - clk_disable_unprepare(st->mclk);
> + if (st->clock_sel == AD7192_CLK_EXT_MCLK1_2 ||
> + st->clock_sel == AD7192_CLK_EXT_MCLK2)
> + clk_disable_unprepare(st->mclk);
> ad_sd_cleanup_buffer_and_trigger(indio_dev);
>
> regulator_disable(st->dvdd);
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] iio: adc: ad7192: Avoid disabling a clock that was never enabled.
2021-05-10 11:30 ` Alexandru Ardelean
@ 2021-05-10 11:56 ` Jonathan Cameron
0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2021-05-10 11:56 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: Jonathan Cameron, linux-iio, Alexandru Tachici
On Mon, 10 May 2021 14:30:48 +0300
Alexandru Ardelean <ardeleanalex@gmail.com> wrote:
> On Sun, May 9, 2021 at 2:42 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > Found by inspection.
> >
> > If the internal clock source is being used, the driver doesn't
> > call clk_prepare_enable() and as such we should not call
> > clk_disable_unprepare()
> >
> > Use the same condition to protect the disable path as is used
> > on the enable one. Note this will all get simplified when
> > the driver moves over to a full devm_ flow, but that would make
> > backporting the fix harder.
> >
> > Fix obviously predates move out of staging, but backporting will
> > become more complex (and is unlikely to happen), hence that patch
> > is given in the fixes tag.
> >
>
> This also looks like a conversion to devm_ would help.
> But later.
Absolutely. I decided to do a pre parse of likely targets for
devmification to see if we had fixed that needed to go first and
that's when I noticed this one.
That way we don't end up with the mess I caused in the previous
set where I have to remember to finally pick up the non fix
part after the fixes are in.
Jonathan
>
> Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com>
>
> > Fixes: b581f748cce0 ("staging: iio: adc: ad7192: move out of staging")
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Cc: Alexandru Tachici <alexandru.tachici@analog.com>
> > Cc: Alexandru Ardelean <ardeleanalex@gmail.com>
> > ---
> > drivers/iio/adc/ad7192.c | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
> > index 2ed580521d81..d3be67aa0522 100644
> > --- a/drivers/iio/adc/ad7192.c
> > +++ b/drivers/iio/adc/ad7192.c
> > @@ -1014,7 +1014,9 @@ static int ad7192_probe(struct spi_device *spi)
> > return 0;
> >
> > error_disable_clk:
> > - clk_disable_unprepare(st->mclk);
> > + if (st->clock_sel == AD7192_CLK_EXT_MCLK1_2 ||
> > + st->clock_sel == AD7192_CLK_EXT_MCLK2)
> > + clk_disable_unprepare(st->mclk);
> > error_remove_trigger:
> > ad_sd_cleanup_buffer_and_trigger(indio_dev);
> > error_disable_dvdd:
> > @@ -1031,7 +1033,9 @@ static int ad7192_remove(struct spi_device *spi)
> > struct ad7192_state *st = iio_priv(indio_dev);
> >
> > iio_device_unregister(indio_dev);
> > - clk_disable_unprepare(st->mclk);
> > + if (st->clock_sel == AD7192_CLK_EXT_MCLK1_2 ||
> > + st->clock_sel == AD7192_CLK_EXT_MCLK2)
> > + clk_disable_unprepare(st->mclk);
> > ad_sd_cleanup_buffer_and_trigger(indio_dev);
> >
> > regulator_disable(st->dvdd);
> > --
> > 2.31.1
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-05-10 13:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-09 11:41 [PATCH] iio: adc: ad7192: Avoid disabling a clock that was never enabled Jonathan Cameron
2021-05-10 11:30 ` Alexandru Ardelean
2021-05-10 11:56 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).