| Message ID | 20210509114118.660422-1-jic23@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iio: adc: ad7192: Avoid disabling a clock that was never enabled. | expand |
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 >
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 > >
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);