* [PATCH 0/3] iio: stm32-adc: add PM support
@ 2018-11-20 10:12 Fabrice Gasnier
2018-11-20 10:12 ` [PATCH 1/3] iio: adc: stm32-adc: move self-calibration to prepare routine Fabrice Gasnier
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Fabrice Gasnier @ 2018-11-20 10:12 UTC (permalink / raw)
To: jic23
Cc: linux-arm-kernel, linux-kernel, mcoquelin.stm32,
alexandre.torgue, fabrice.gasnier, linux-iio, lars, knaack.h,
pmeerw, linux-stm32
This patch series adds support for power management to stm32-adc iio driver.
It slighlty reworks regulator and clock handling, to move them in dedicated
routines. Then these routines can be called from probe/remove and PM callbacks.
It also takes care of running ADC when going to low power mode.
Fabrice Gasnier (3):
iio: adc: stm32-adc: move self-calibration to prepare routine
iio: adc: stm32-adc: add power management support
iio: adc: stm32-adc: switch off running adc when going to low power
drivers/iio/adc/stm32-adc-core.c | 182 +++++++++++++++--------
drivers/iio/adc/stm32-adc.c | 303 ++++++++++++++++++++++++++++-----------
2 files changed, 343 insertions(+), 142 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/3] iio: adc: stm32-adc: move self-calibration to prepare routine 2018-11-20 10:12 [PATCH 0/3] iio: stm32-adc: add PM support Fabrice Gasnier @ 2018-11-20 10:12 ` Fabrice Gasnier 2018-11-25 13:03 ` Jonathan Cameron 2018-11-20 10:12 ` [PATCH 2/3] iio: adc: stm32-adc: add power management support Fabrice Gasnier 2018-11-20 10:12 ` [PATCH 3/3] iio: adc: stm32-adc: switch off running adc when going to low power Fabrice Gasnier 2 siblings, 1 reply; 9+ messages in thread From: Fabrice Gasnier @ 2018-11-20 10:12 UTC (permalink / raw) To: jic23 Cc: linux-arm-kernel, linux-kernel, mcoquelin.stm32, alexandre.torgue, fabrice.gasnier, linux-iio, lars, knaack.h, pmeerw, linux-stm32 Move self-calibration routine to prepare routine. - This is precursor patch to ease power management handling. - This also allow to factorize few error cases (error handling). Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> --- drivers/iio/adc/stm32-adc.c | 59 ++++++++++++++++++--------------------------- 1 file changed, 24 insertions(+), 35 deletions(-) diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c index 3784118..dca8733 100644 --- a/drivers/iio/adc/stm32-adc.c +++ b/drivers/iio/adc/stm32-adc.c @@ -199,11 +199,13 @@ struct stm32_adc_trig_info { * @calfact_s: Calibration offset for single ended channels * @calfact_d: Calibration offset in differential * @lincalfact: Linearity calibration factor + * @calibrated: Indicates calibration status */ struct stm32_adc_calib { u32 calfact_s; u32 calfact_d; u32 lincalfact[STM32H7_LINCALFACT_NUM]; + bool calibrated; }; /** @@ -251,7 +253,6 @@ struct stm32_adc_regspec { * @trigs: external trigger sources * @clk_required: clock is required * @has_vregready: vregready status flag presence - * @selfcalib: optional routine for self-calibration * @prepare: optional prepare routine (power-up, enable) * @start_conv: routine to start conversions * @stop_conv: routine to stop conversions @@ -264,7 +265,6 @@ struct stm32_adc_cfg { struct stm32_adc_trig_info *trigs; bool clk_required; bool has_vregready; - int (*selfcalib)(struct stm32_adc *); int (*prepare)(struct stm32_adc *); void (*start_conv)(struct stm32_adc *, bool dma); void (*stop_conv)(struct stm32_adc *); @@ -777,6 +777,7 @@ static void stm32h7_adc_disable(struct stm32_adc *adc) /** * stm32h7_adc_read_selfcalib() - read calibration shadow regs, save result * @adc: stm32 adc instance + * Note: Must be called once ADC is enabled, so LINCALRDYW[1..6] are writable */ static int stm32h7_adc_read_selfcalib(struct stm32_adc *adc) { @@ -784,11 +785,6 @@ static int stm32h7_adc_read_selfcalib(struct stm32_adc *adc) int i, ret; u32 lincalrdyw_mask, val; - /* Enable adc so LINCALRDYW1..6 bits are writable */ - ret = stm32h7_adc_enable(adc); - if (ret) - return ret; - /* Read linearity calibration */ lincalrdyw_mask = STM32H7_LINCALRDYW6; for (i = STM32H7_LINCALFACT_NUM - 1; i >= 0; i--) { @@ -801,7 +797,7 @@ static int stm32h7_adc_read_selfcalib(struct stm32_adc *adc) 100, STM32_ADC_TIMEOUT_US); if (ret) { dev_err(&indio_dev->dev, "Failed to read calfact\n"); - goto disable; + return ret; } val = stm32_adc_readl(adc, STM32H7_ADC_CALFACT2); @@ -817,11 +813,9 @@ static int stm32h7_adc_read_selfcalib(struct stm32_adc *adc) adc->cal.calfact_s >>= STM32H7_CALFACT_S_SHIFT; adc->cal.calfact_d = (val & STM32H7_CALFACT_D_MASK); adc->cal.calfact_d >>= STM32H7_CALFACT_D_SHIFT; + adc->cal.calibrated = true; -disable: - stm32h7_adc_disable(adc); - - return ret; + return 0; } /** @@ -898,9 +892,9 @@ static int stm32h7_adc_restore_selfcalib(struct stm32_adc *adc) #define STM32H7_ADC_CALIB_TIMEOUT_US 100000 /** - * stm32h7_adc_selfcalib() - Procedure to calibrate ADC (from power down) + * stm32h7_adc_selfcalib() - Procedure to calibrate ADC * @adc: stm32 adc instance - * Exit from power down, calibrate ADC, then return to power down. + * Note: Must be called once ADC is out of power down. */ static int stm32h7_adc_selfcalib(struct stm32_adc *adc) { @@ -908,9 +902,8 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc) int ret; u32 val; - ret = stm32h7_adc_exit_pwr_down(adc); - if (ret) - return ret; + if (adc->cal.calibrated) + return adc->cal.calibrated; /* * Select calibration mode: @@ -927,7 +920,7 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc) STM32H7_ADC_CALIB_TIMEOUT_US); if (ret) { dev_err(&indio_dev->dev, "calibration failed\n"); - goto pwr_dwn; + goto out; } /* @@ -944,18 +937,13 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc) STM32H7_ADC_CALIB_TIMEOUT_US); if (ret) { dev_err(&indio_dev->dev, "calibration failed\n"); - goto pwr_dwn; + goto out; } +out: stm32_adc_clr_bits(adc, STM32H7_ADC_CR, STM32H7_ADCALDIF | STM32H7_ADCALLIN); - /* Read calibration result for future reference */ - ret = stm32h7_adc_read_selfcalib(adc); - -pwr_dwn: - stm32h7_adc_enter_pwr_down(adc); - return ret; } @@ -972,19 +960,28 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc) */ static int stm32h7_adc_prepare(struct stm32_adc *adc) { - int ret; + int calib, ret; ret = stm32h7_adc_exit_pwr_down(adc); if (ret) return ret; + ret = stm32h7_adc_selfcalib(adc); + if (ret < 0) + goto pwr_dwn; + calib = ret; + stm32_adc_writel(adc, STM32H7_ADC_DIFSEL, adc->difsel); ret = stm32h7_adc_enable(adc); if (ret) goto pwr_dwn; - ret = stm32h7_adc_restore_selfcalib(adc); + /* Either restore or read calibration result for future reference */ + if (calib) + ret = stm32h7_adc_restore_selfcalib(adc); + else + ret = stm32h7_adc_read_selfcalib(adc); if (ret) goto disable; @@ -1880,12 +1877,6 @@ static int stm32_adc_probe(struct platform_device *pdev) goto err_clk_disable; stm32_adc_set_res(adc); - if (adc->cfg->selfcalib) { - ret = adc->cfg->selfcalib(adc); - if (ret) - goto err_clk_disable; - } - ret = stm32_adc_chan_of_init(indio_dev); if (ret < 0) goto err_clk_disable; @@ -1961,7 +1952,6 @@ static int stm32_adc_remove(struct platform_device *pdev) .regs = &stm32h7_adc_regspec, .adc_info = &stm32h7_adc_info, .trigs = stm32h7_adc_trigs, - .selfcalib = stm32h7_adc_selfcalib, .start_conv = stm32h7_adc_start_conv, .stop_conv = stm32h7_adc_stop_conv, .prepare = stm32h7_adc_prepare, @@ -1974,7 +1964,6 @@ static int stm32_adc_remove(struct platform_device *pdev) .adc_info = &stm32h7_adc_info, .trigs = stm32h7_adc_trigs, .has_vregready = true, - .selfcalib = stm32h7_adc_selfcalib, .start_conv = stm32h7_adc_start_conv, .stop_conv = stm32h7_adc_stop_conv, .prepare = stm32h7_adc_prepare, -- 1.9.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] iio: adc: stm32-adc: move self-calibration to prepare routine 2018-11-20 10:12 ` [PATCH 1/3] iio: adc: stm32-adc: move self-calibration to prepare routine Fabrice Gasnier @ 2018-11-25 13:03 ` Jonathan Cameron 2018-11-25 13:14 ` Jonathan Cameron 0 siblings, 1 reply; 9+ messages in thread From: Jonathan Cameron @ 2018-11-25 13:03 UTC (permalink / raw) To: Fabrice Gasnier Cc: linux-arm-kernel, linux-kernel, mcoquelin.stm32, alexandre.torgue, linux-iio, lars, knaack.h, pmeerw, linux-stm32 On Tue, 20 Nov 2018 11:12:30 +0100 Fabrice Gasnier <fabrice.gasnier@st.com> wrote: > Move self-calibration routine to prepare routine. > - This is precursor patch to ease power management handling. > - This also allow to factorize few error cases (error handling). > > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> one trivial point inline. Otherwise seems a sensible bit of refactoring. Thanks, Jonathan > --- > drivers/iio/adc/stm32-adc.c | 59 ++++++++++++++++++--------------------------- > 1 file changed, 24 insertions(+), 35 deletions(-) > > diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c > index 3784118..dca8733 100644 > --- a/drivers/iio/adc/stm32-adc.c > +++ b/drivers/iio/adc/stm32-adc.c > @@ -199,11 +199,13 @@ struct stm32_adc_trig_info { > * @calfact_s: Calibration offset for single ended channels > * @calfact_d: Calibration offset in differential > * @lincalfact: Linearity calibration factor > + * @calibrated: Indicates calibration status > */ > struct stm32_adc_calib { > u32 calfact_s; > u32 calfact_d; > u32 lincalfact[STM32H7_LINCALFACT_NUM]; > + bool calibrated; > }; > > /** > @@ -251,7 +253,6 @@ struct stm32_adc_regspec { > * @trigs: external trigger sources > * @clk_required: clock is required > * @has_vregready: vregready status flag presence > - * @selfcalib: optional routine for self-calibration > * @prepare: optional prepare routine (power-up, enable) > * @start_conv: routine to start conversions > * @stop_conv: routine to stop conversions > @@ -264,7 +265,6 @@ struct stm32_adc_cfg { > struct stm32_adc_trig_info *trigs; > bool clk_required; > bool has_vregready; > - int (*selfcalib)(struct stm32_adc *); > int (*prepare)(struct stm32_adc *); > void (*start_conv)(struct stm32_adc *, bool dma); > void (*stop_conv)(struct stm32_adc *); > @@ -777,6 +777,7 @@ static void stm32h7_adc_disable(struct stm32_adc *adc) > /** > * stm32h7_adc_read_selfcalib() - read calibration shadow regs, save result > * @adc: stm32 adc instance > + * Note: Must be called once ADC is enabled, so LINCALRDYW[1..6] are writable > */ > static int stm32h7_adc_read_selfcalib(struct stm32_adc *adc) > { > @@ -784,11 +785,6 @@ static int stm32h7_adc_read_selfcalib(struct stm32_adc *adc) > int i, ret; > u32 lincalrdyw_mask, val; > > - /* Enable adc so LINCALRDYW1..6 bits are writable */ > - ret = stm32h7_adc_enable(adc); > - if (ret) > - return ret; > - > /* Read linearity calibration */ > lincalrdyw_mask = STM32H7_LINCALRDYW6; > for (i = STM32H7_LINCALFACT_NUM - 1; i >= 0; i--) { > @@ -801,7 +797,7 @@ static int stm32h7_adc_read_selfcalib(struct stm32_adc *adc) > 100, STM32_ADC_TIMEOUT_US); > if (ret) { > dev_err(&indio_dev->dev, "Failed to read calfact\n"); > - goto disable; > + return ret; > } > > val = stm32_adc_readl(adc, STM32H7_ADC_CALFACT2); > @@ -817,11 +813,9 @@ static int stm32h7_adc_read_selfcalib(struct stm32_adc *adc) > adc->cal.calfact_s >>= STM32H7_CALFACT_S_SHIFT; > adc->cal.calfact_d = (val & STM32H7_CALFACT_D_MASK); > adc->cal.calfact_d >>= STM32H7_CALFACT_D_SHIFT; > + adc->cal.calibrated = true; > > -disable: > - stm32h7_adc_disable(adc); > - > - return ret; > + return 0; > } > > /** > @@ -898,9 +892,9 @@ static int stm32h7_adc_restore_selfcalib(struct stm32_adc *adc) > #define STM32H7_ADC_CALIB_TIMEOUT_US 100000 > > /** > - * stm32h7_adc_selfcalib() - Procedure to calibrate ADC (from power down) > + * stm32h7_adc_selfcalib() - Procedure to calibrate ADC > * @adc: stm32 adc instance > - * Exit from power down, calibrate ADC, then return to power down. > + * Note: Must be called once ADC is out of power down. > */ > static int stm32h7_adc_selfcalib(struct stm32_adc *adc) > { > @@ -908,9 +902,8 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc) > int ret; > u32 val; > > - ret = stm32h7_adc_exit_pwr_down(adc); > - if (ret) > - return ret; > + if (adc->cal.calibrated) > + return adc->cal.calibrated; return true seems more logical given this is a boolean. > > /* > * Select calibration mode: > @@ -927,7 +920,7 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc) > STM32H7_ADC_CALIB_TIMEOUT_US); > if (ret) { > dev_err(&indio_dev->dev, "calibration failed\n"); > - goto pwr_dwn; > + goto out; > } > > /* > @@ -944,18 +937,13 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc) > STM32H7_ADC_CALIB_TIMEOUT_US); > if (ret) { > dev_err(&indio_dev->dev, "calibration failed\n"); > - goto pwr_dwn; > + goto out; > } > > +out: > stm32_adc_clr_bits(adc, STM32H7_ADC_CR, > STM32H7_ADCALDIF | STM32H7_ADCALLIN); > > - /* Read calibration result for future reference */ > - ret = stm32h7_adc_read_selfcalib(adc); > - > -pwr_dwn: > - stm32h7_adc_enter_pwr_down(adc); > - > return ret; > } > > @@ -972,19 +960,28 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc) > */ > static int stm32h7_adc_prepare(struct stm32_adc *adc) > { > - int ret; > + int calib, ret; > > ret = stm32h7_adc_exit_pwr_down(adc); > if (ret) > return ret; > > + ret = stm32h7_adc_selfcalib(adc); > + if (ret < 0) > + goto pwr_dwn; > + calib = ret; > + > stm32_adc_writel(adc, STM32H7_ADC_DIFSEL, adc->difsel); > > ret = stm32h7_adc_enable(adc); > if (ret) > goto pwr_dwn; > > - ret = stm32h7_adc_restore_selfcalib(adc); > + /* Either restore or read calibration result for future reference */ > + if (calib) > + ret = stm32h7_adc_restore_selfcalib(adc); > + else > + ret = stm32h7_adc_read_selfcalib(adc); > if (ret) > goto disable; > > @@ -1880,12 +1877,6 @@ static int stm32_adc_probe(struct platform_device *pdev) > goto err_clk_disable; > stm32_adc_set_res(adc); > > - if (adc->cfg->selfcalib) { > - ret = adc->cfg->selfcalib(adc); > - if (ret) > - goto err_clk_disable; > - } > - > ret = stm32_adc_chan_of_init(indio_dev); > if (ret < 0) > goto err_clk_disable; > @@ -1961,7 +1952,6 @@ static int stm32_adc_remove(struct platform_device *pdev) > .regs = &stm32h7_adc_regspec, > .adc_info = &stm32h7_adc_info, > .trigs = stm32h7_adc_trigs, > - .selfcalib = stm32h7_adc_selfcalib, > .start_conv = stm32h7_adc_start_conv, > .stop_conv = stm32h7_adc_stop_conv, > .prepare = stm32h7_adc_prepare, > @@ -1974,7 +1964,6 @@ static int stm32_adc_remove(struct platform_device *pdev) > .adc_info = &stm32h7_adc_info, > .trigs = stm32h7_adc_trigs, > .has_vregready = true, > - .selfcalib = stm32h7_adc_selfcalib, > .start_conv = stm32h7_adc_start_conv, > .stop_conv = stm32h7_adc_stop_conv, > .prepare = stm32h7_adc_prepare, ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] iio: adc: stm32-adc: move self-calibration to prepare routine 2018-11-25 13:03 ` Jonathan Cameron @ 2018-11-25 13:14 ` Jonathan Cameron 2018-11-26 10:41 ` Fabrice Gasnier 0 siblings, 1 reply; 9+ messages in thread From: Jonathan Cameron @ 2018-11-25 13:14 UTC (permalink / raw) To: Fabrice Gasnier Cc: linux-arm-kernel, linux-kernel, mcoquelin.stm32, alexandre.torgue, linux-iio, lars, knaack.h, pmeerw, linux-stm32 On Sun, 25 Nov 2018 13:03:39 +0000 Jonathan Cameron <jic23@kernel.org> wrote: > On Tue, 20 Nov 2018 11:12:30 +0100 > Fabrice Gasnier <fabrice.gasnier@st.com> wrote: > > > Move self-calibration routine to prepare routine. > > - This is precursor patch to ease power management handling. > > - This also allow to factorize few error cases (error handling). > > > > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> > one trivial point inline. Otherwise seems a sensible bit of refactoring. Given this was the only 'issue' I found in the whole set I've just applied it with that changed. Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > > Thanks, > > Jonathan > > > --- > > drivers/iio/adc/stm32-adc.c | 59 ++++++++++++++++++--------------------------- > > 1 file changed, 24 insertions(+), 35 deletions(-) > > > > diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c > > index 3784118..dca8733 100644 > > --- a/drivers/iio/adc/stm32-adc.c > > +++ b/drivers/iio/adc/stm32-adc.c > > @@ -199,11 +199,13 @@ struct stm32_adc_trig_info { > > * @calfact_s: Calibration offset for single ended channels > > * @calfact_d: Calibration offset in differential > > * @lincalfact: Linearity calibration factor > > + * @calibrated: Indicates calibration status > > */ > > struct stm32_adc_calib { > > u32 calfact_s; > > u32 calfact_d; > > u32 lincalfact[STM32H7_LINCALFACT_NUM]; > > + bool calibrated; > > }; > > > > /** > > @@ -251,7 +253,6 @@ struct stm32_adc_regspec { > > * @trigs: external trigger sources > > * @clk_required: clock is required > > * @has_vregready: vregready status flag presence > > - * @selfcalib: optional routine for self-calibration > > * @prepare: optional prepare routine (power-up, enable) > > * @start_conv: routine to start conversions > > * @stop_conv: routine to stop conversions > > @@ -264,7 +265,6 @@ struct stm32_adc_cfg { > > struct stm32_adc_trig_info *trigs; > > bool clk_required; > > bool has_vregready; > > - int (*selfcalib)(struct stm32_adc *); > > int (*prepare)(struct stm32_adc *); > > void (*start_conv)(struct stm32_adc *, bool dma); > > void (*stop_conv)(struct stm32_adc *); > > @@ -777,6 +777,7 @@ static void stm32h7_adc_disable(struct stm32_adc *adc) > > /** > > * stm32h7_adc_read_selfcalib() - read calibration shadow regs, save result > > * @adc: stm32 adc instance > > + * Note: Must be called once ADC is enabled, so LINCALRDYW[1..6] are writable > > */ > > static int stm32h7_adc_read_selfcalib(struct stm32_adc *adc) > > { > > @@ -784,11 +785,6 @@ static int stm32h7_adc_read_selfcalib(struct stm32_adc *adc) > > int i, ret; > > u32 lincalrdyw_mask, val; > > > > - /* Enable adc so LINCALRDYW1..6 bits are writable */ > > - ret = stm32h7_adc_enable(adc); > > - if (ret) > > - return ret; > > - > > /* Read linearity calibration */ > > lincalrdyw_mask = STM32H7_LINCALRDYW6; > > for (i = STM32H7_LINCALFACT_NUM - 1; i >= 0; i--) { > > @@ -801,7 +797,7 @@ static int stm32h7_adc_read_selfcalib(struct stm32_adc *adc) > > 100, STM32_ADC_TIMEOUT_US); > > if (ret) { > > dev_err(&indio_dev->dev, "Failed to read calfact\n"); > > - goto disable; > > + return ret; > > } > > > > val = stm32_adc_readl(adc, STM32H7_ADC_CALFACT2); > > @@ -817,11 +813,9 @@ static int stm32h7_adc_read_selfcalib(struct stm32_adc *adc) > > adc->cal.calfact_s >>= STM32H7_CALFACT_S_SHIFT; > > adc->cal.calfact_d = (val & STM32H7_CALFACT_D_MASK); > > adc->cal.calfact_d >>= STM32H7_CALFACT_D_SHIFT; > > + adc->cal.calibrated = true; > > > > -disable: > > - stm32h7_adc_disable(adc); > > - > > - return ret; > > + return 0; > > } > > > > /** > > @@ -898,9 +892,9 @@ static int stm32h7_adc_restore_selfcalib(struct stm32_adc *adc) > > #define STM32H7_ADC_CALIB_TIMEOUT_US 100000 > > > > /** > > - * stm32h7_adc_selfcalib() - Procedure to calibrate ADC (from power down) > > + * stm32h7_adc_selfcalib() - Procedure to calibrate ADC > > * @adc: stm32 adc instance > > - * Exit from power down, calibrate ADC, then return to power down. > > + * Note: Must be called once ADC is out of power down. > > */ > > static int stm32h7_adc_selfcalib(struct stm32_adc *adc) > > { > > @@ -908,9 +902,8 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc) > > int ret; > > u32 val; > > > > - ret = stm32h7_adc_exit_pwr_down(adc); > > - if (ret) > > - return ret; > > + if (adc->cal.calibrated) > > + return adc->cal.calibrated; > return true seems more logical given this is a boolean. > > > > > /* > > * Select calibration mode: > > @@ -927,7 +920,7 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc) > > STM32H7_ADC_CALIB_TIMEOUT_US); > > if (ret) { > > dev_err(&indio_dev->dev, "calibration failed\n"); > > - goto pwr_dwn; > > + goto out; > > } > > > > /* > > @@ -944,18 +937,13 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc) > > STM32H7_ADC_CALIB_TIMEOUT_US); > > if (ret) { > > dev_err(&indio_dev->dev, "calibration failed\n"); > > - goto pwr_dwn; > > + goto out; > > } > > > > +out: > > stm32_adc_clr_bits(adc, STM32H7_ADC_CR, > > STM32H7_ADCALDIF | STM32H7_ADCALLIN); > > > > - /* Read calibration result for future reference */ > > - ret = stm32h7_adc_read_selfcalib(adc); > > - > > -pwr_dwn: > > - stm32h7_adc_enter_pwr_down(adc); > > - > > return ret; > > } > > > > @@ -972,19 +960,28 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc) > > */ > > static int stm32h7_adc_prepare(struct stm32_adc *adc) > > { > > - int ret; > > + int calib, ret; > > > > ret = stm32h7_adc_exit_pwr_down(adc); > > if (ret) > > return ret; > > > > + ret = stm32h7_adc_selfcalib(adc); > > + if (ret < 0) > > + goto pwr_dwn; > > + calib = ret; > > + > > stm32_adc_writel(adc, STM32H7_ADC_DIFSEL, adc->difsel); > > > > ret = stm32h7_adc_enable(adc); > > if (ret) > > goto pwr_dwn; > > > > - ret = stm32h7_adc_restore_selfcalib(adc); > > + /* Either restore or read calibration result for future reference */ > > + if (calib) > > + ret = stm32h7_adc_restore_selfcalib(adc); > > + else > > + ret = stm32h7_adc_read_selfcalib(adc); > > if (ret) > > goto disable; > > > > @@ -1880,12 +1877,6 @@ static int stm32_adc_probe(struct platform_device *pdev) > > goto err_clk_disable; > > stm32_adc_set_res(adc); > > > > - if (adc->cfg->selfcalib) { > > - ret = adc->cfg->selfcalib(adc); > > - if (ret) > > - goto err_clk_disable; > > - } > > - > > ret = stm32_adc_chan_of_init(indio_dev); > > if (ret < 0) > > goto err_clk_disable; > > @@ -1961,7 +1952,6 @@ static int stm32_adc_remove(struct platform_device *pdev) > > .regs = &stm32h7_adc_regspec, > > .adc_info = &stm32h7_adc_info, > > .trigs = stm32h7_adc_trigs, > > - .selfcalib = stm32h7_adc_selfcalib, > > .start_conv = stm32h7_adc_start_conv, > > .stop_conv = stm32h7_adc_stop_conv, > > .prepare = stm32h7_adc_prepare, > > @@ -1974,7 +1964,6 @@ static int stm32_adc_remove(struct platform_device *pdev) > > .adc_info = &stm32h7_adc_info, > > .trigs = stm32h7_adc_trigs, > > .has_vregready = true, > > - .selfcalib = stm32h7_adc_selfcalib, > > .start_conv = stm32h7_adc_start_conv, > > .stop_conv = stm32h7_adc_stop_conv, > > .prepare = stm32h7_adc_prepare, > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] iio: adc: stm32-adc: move self-calibration to prepare routine 2018-11-25 13:14 ` Jonathan Cameron @ 2018-11-26 10:41 ` Fabrice Gasnier 0 siblings, 0 replies; 9+ messages in thread From: Fabrice Gasnier @ 2018-11-26 10:41 UTC (permalink / raw) To: Jonathan Cameron Cc: linux-arm-kernel, linux-kernel, mcoquelin.stm32, alexandre.torgue, linux-iio, lars, knaack.h, pmeerw, linux-stm32 On 11/25/18 2:14 PM, Jonathan Cameron wrote: > On Sun, 25 Nov 2018 13:03:39 +0000 > Jonathan Cameron <jic23@kernel.org> wrote: > >> On Tue, 20 Nov 2018 11:12:30 +0100 >> Fabrice Gasnier <fabrice.gasnier@st.com> wrote: >> >>> Move self-calibration routine to prepare routine. >>> - This is precursor patch to ease power management handling. >>> - This also allow to factorize few error cases (error handling). >>> >>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> >> one trivial point inline. Otherwise seems a sensible bit of refactoring. > Given this was the only 'issue' I found in the whole set I've > just applied it with that changed. Hi Jonathan, Many thanks. Best Regards, Fabrice > > Applied to the togreg branch of iio.git and pushed out as testing > for the autobuilders to play with it. > > Thanks, > > Jonathan > >> >> Thanks, >> >> Jonathan >> >>> --- >>> drivers/iio/adc/stm32-adc.c | 59 ++++++++++++++++++--------------------------- >>> 1 file changed, 24 insertions(+), 35 deletions(-) >>> >>> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c >>> index 3784118..dca8733 100644 >>> --- a/drivers/iio/adc/stm32-adc.c >>> +++ b/drivers/iio/adc/stm32-adc.c >>> @@ -199,11 +199,13 @@ struct stm32_adc_trig_info { >>> * @calfact_s: Calibration offset for single ended channels >>> * @calfact_d: Calibration offset in differential >>> * @lincalfact: Linearity calibration factor >>> + * @calibrated: Indicates calibration status >>> */ >>> struct stm32_adc_calib { >>> u32 calfact_s; >>> u32 calfact_d; >>> u32 lincalfact[STM32H7_LINCALFACT_NUM]; >>> + bool calibrated; >>> }; >>> >>> /** >>> @@ -251,7 +253,6 @@ struct stm32_adc_regspec { >>> * @trigs: external trigger sources >>> * @clk_required: clock is required >>> * @has_vregready: vregready status flag presence >>> - * @selfcalib: optional routine for self-calibration >>> * @prepare: optional prepare routine (power-up, enable) >>> * @start_conv: routine to start conversions >>> * @stop_conv: routine to stop conversions >>> @@ -264,7 +265,6 @@ struct stm32_adc_cfg { >>> struct stm32_adc_trig_info *trigs; >>> bool clk_required; >>> bool has_vregready; >>> - int (*selfcalib)(struct stm32_adc *); >>> int (*prepare)(struct stm32_adc *); >>> void (*start_conv)(struct stm32_adc *, bool dma); >>> void (*stop_conv)(struct stm32_adc *); >>> @@ -777,6 +777,7 @@ static void stm32h7_adc_disable(struct stm32_adc *adc) >>> /** >>> * stm32h7_adc_read_selfcalib() - read calibration shadow regs, save result >>> * @adc: stm32 adc instance >>> + * Note: Must be called once ADC is enabled, so LINCALRDYW[1..6] are writable >>> */ >>> static int stm32h7_adc_read_selfcalib(struct stm32_adc *adc) >>> { >>> @@ -784,11 +785,6 @@ static int stm32h7_adc_read_selfcalib(struct stm32_adc *adc) >>> int i, ret; >>> u32 lincalrdyw_mask, val; >>> >>> - /* Enable adc so LINCALRDYW1..6 bits are writable */ >>> - ret = stm32h7_adc_enable(adc); >>> - if (ret) >>> - return ret; >>> - >>> /* Read linearity calibration */ >>> lincalrdyw_mask = STM32H7_LINCALRDYW6; >>> for (i = STM32H7_LINCALFACT_NUM - 1; i >= 0; i--) { >>> @@ -801,7 +797,7 @@ static int stm32h7_adc_read_selfcalib(struct stm32_adc *adc) >>> 100, STM32_ADC_TIMEOUT_US); >>> if (ret) { >>> dev_err(&indio_dev->dev, "Failed to read calfact\n"); >>> - goto disable; >>> + return ret; >>> } >>> >>> val = stm32_adc_readl(adc, STM32H7_ADC_CALFACT2); >>> @@ -817,11 +813,9 @@ static int stm32h7_adc_read_selfcalib(struct stm32_adc *adc) >>> adc->cal.calfact_s >>= STM32H7_CALFACT_S_SHIFT; >>> adc->cal.calfact_d = (val & STM32H7_CALFACT_D_MASK); >>> adc->cal.calfact_d >>= STM32H7_CALFACT_D_SHIFT; >>> + adc->cal.calibrated = true; >>> >>> -disable: >>> - stm32h7_adc_disable(adc); >>> - >>> - return ret; >>> + return 0; >>> } >>> >>> /** >>> @@ -898,9 +892,9 @@ static int stm32h7_adc_restore_selfcalib(struct stm32_adc *adc) >>> #define STM32H7_ADC_CALIB_TIMEOUT_US 100000 >>> >>> /** >>> - * stm32h7_adc_selfcalib() - Procedure to calibrate ADC (from power down) >>> + * stm32h7_adc_selfcalib() - Procedure to calibrate ADC >>> * @adc: stm32 adc instance >>> - * Exit from power down, calibrate ADC, then return to power down. >>> + * Note: Must be called once ADC is out of power down. >>> */ >>> static int stm32h7_adc_selfcalib(struct stm32_adc *adc) >>> { >>> @@ -908,9 +902,8 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc) >>> int ret; >>> u32 val; >>> >>> - ret = stm32h7_adc_exit_pwr_down(adc); >>> - if (ret) >>> - return ret; >>> + if (adc->cal.calibrated) >>> + return adc->cal.calibrated; >> return true seems more logical given this is a boolean. >> >>> >>> /* >>> * Select calibration mode: >>> @@ -927,7 +920,7 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc) >>> STM32H7_ADC_CALIB_TIMEOUT_US); >>> if (ret) { >>> dev_err(&indio_dev->dev, "calibration failed\n"); >>> - goto pwr_dwn; >>> + goto out; >>> } >>> >>> /* >>> @@ -944,18 +937,13 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc) >>> STM32H7_ADC_CALIB_TIMEOUT_US); >>> if (ret) { >>> dev_err(&indio_dev->dev, "calibration failed\n"); >>> - goto pwr_dwn; >>> + goto out; >>> } >>> >>> +out: >>> stm32_adc_clr_bits(adc, STM32H7_ADC_CR, >>> STM32H7_ADCALDIF | STM32H7_ADCALLIN); >>> >>> - /* Read calibration result for future reference */ >>> - ret = stm32h7_adc_read_selfcalib(adc); >>> - >>> -pwr_dwn: >>> - stm32h7_adc_enter_pwr_down(adc); >>> - >>> return ret; >>> } >>> >>> @@ -972,19 +960,28 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc) >>> */ >>> static int stm32h7_adc_prepare(struct stm32_adc *adc) >>> { >>> - int ret; >>> + int calib, ret; >>> >>> ret = stm32h7_adc_exit_pwr_down(adc); >>> if (ret) >>> return ret; >>> >>> + ret = stm32h7_adc_selfcalib(adc); >>> + if (ret < 0) >>> + goto pwr_dwn; >>> + calib = ret; >>> + >>> stm32_adc_writel(adc, STM32H7_ADC_DIFSEL, adc->difsel); >>> >>> ret = stm32h7_adc_enable(adc); >>> if (ret) >>> goto pwr_dwn; >>> >>> - ret = stm32h7_adc_restore_selfcalib(adc); >>> + /* Either restore or read calibration result for future reference */ >>> + if (calib) >>> + ret = stm32h7_adc_restore_selfcalib(adc); >>> + else >>> + ret = stm32h7_adc_read_selfcalib(adc); >>> if (ret) >>> goto disable; >>> >>> @@ -1880,12 +1877,6 @@ static int stm32_adc_probe(struct platform_device *pdev) >>> goto err_clk_disable; >>> stm32_adc_set_res(adc); >>> >>> - if (adc->cfg->selfcalib) { >>> - ret = adc->cfg->selfcalib(adc); >>> - if (ret) >>> - goto err_clk_disable; >>> - } >>> - >>> ret = stm32_adc_chan_of_init(indio_dev); >>> if (ret < 0) >>> goto err_clk_disable; >>> @@ -1961,7 +1952,6 @@ static int stm32_adc_remove(struct platform_device *pdev) >>> .regs = &stm32h7_adc_regspec, >>> .adc_info = &stm32h7_adc_info, >>> .trigs = stm32h7_adc_trigs, >>> - .selfcalib = stm32h7_adc_selfcalib, >>> .start_conv = stm32h7_adc_start_conv, >>> .stop_conv = stm32h7_adc_stop_conv, >>> .prepare = stm32h7_adc_prepare, >>> @@ -1974,7 +1964,6 @@ static int stm32_adc_remove(struct platform_device *pdev) >>> .adc_info = &stm32h7_adc_info, >>> .trigs = stm32h7_adc_trigs, >>> .has_vregready = true, >>> - .selfcalib = stm32h7_adc_selfcalib, >>> .start_conv = stm32h7_adc_start_conv, >>> .stop_conv = stm32h7_adc_stop_conv, >>> .prepare = stm32h7_adc_prepare, >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] iio: adc: stm32-adc: add power management support 2018-11-20 10:12 [PATCH 0/3] iio: stm32-adc: add PM support Fabrice Gasnier 2018-11-20 10:12 ` [PATCH 1/3] iio: adc: stm32-adc: move self-calibration to prepare routine Fabrice Gasnier @ 2018-11-20 10:12 ` Fabrice Gasnier 2018-11-25 13:16 ` Jonathan Cameron 2018-11-20 10:12 ` [PATCH 3/3] iio: adc: stm32-adc: switch off running adc when going to low power Fabrice Gasnier 2 siblings, 1 reply; 9+ messages in thread From: Fabrice Gasnier @ 2018-11-20 10:12 UTC (permalink / raw) To: jic23 Cc: linux-arm-kernel, linux-kernel, mcoquelin.stm32, alexandre.torgue, fabrice.gasnier, linux-iio, lars, knaack.h, pmeerw, linux-stm32 Add support for runtime PM & sleep. Move all regulator and clock management to dedicated HW start/stop routines. Then rely on (runtime) PM OPS to call them. Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> --- drivers/iio/adc/stm32-adc-core.c | 182 +++++++++++++++++++++++++++------------ drivers/iio/adc/stm32-adc.c | 169 ++++++++++++++++++++++++++++-------- 2 files changed, 258 insertions(+), 93 deletions(-) diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c index ca432e7..2327ec1 100644 --- a/drivers/iio/adc/stm32-adc-core.c +++ b/drivers/iio/adc/stm32-adc-core.c @@ -16,6 +16,7 @@ #include <linux/irqdomain.h> #include <linux/module.h> #include <linux/of_device.h> +#include <linux/pm_runtime.h> #include <linux/regulator/consumer.h> #include <linux/slab.h> @@ -48,15 +49,19 @@ #define STM32H7_CKMODE_SHIFT 16 #define STM32H7_CKMODE_MASK GENMASK(17, 16) +#define STM32_ADC_CORE_SLEEP_DELAY_MS 2000 + /** * stm32_adc_common_regs - stm32 common registers, compatible dependent data * @csr: common status register offset + * @ccr: common control register offset * @eoc1: adc1 end of conversion flag in @csr * @eoc2: adc2 end of conversion flag in @csr * @eoc3: adc3 end of conversion flag in @csr */ struct stm32_adc_common_regs { u32 csr; + u32 ccr; u32 eoc1_msk; u32 eoc2_msk; u32 eoc3_msk; @@ -85,6 +90,7 @@ struct stm32_adc_priv_cfg { * @vref: regulator reference * @cfg: compatible configuration data * @common: common data for all ADC instances + * @ccr_bak: backup CCR in low power mode */ struct stm32_adc_priv { int irq[STM32_ADC_MAX_ADCS]; @@ -94,6 +100,7 @@ struct stm32_adc_priv { struct regulator *vref; const struct stm32_adc_priv_cfg *cfg; struct stm32_adc_common common; + u32 ccr_bak; }; static struct stm32_adc_priv *to_stm32_adc_priv(struct stm32_adc_common *com) @@ -265,6 +272,7 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev, /* STM32F4 common registers definitions */ static const struct stm32_adc_common_regs stm32f4_adc_common_regs = { .csr = STM32F4_ADC_CSR, + .ccr = STM32F4_ADC_CCR, .eoc1_msk = STM32F4_EOC1, .eoc2_msk = STM32F4_EOC2, .eoc3_msk = STM32F4_EOC3, @@ -273,6 +281,7 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev, /* STM32H7 common registers definitions */ static const struct stm32_adc_common_regs stm32h7_adc_common_regs = { .csr = STM32H7_ADC_CSR, + .ccr = STM32H7_ADC_CCR, .eoc1_msk = STM32H7_EOC_MST, .eoc2_msk = STM32H7_EOC_SLV, }; @@ -379,6 +388,61 @@ static void stm32_adc_irq_remove(struct platform_device *pdev, } } +static int stm32_adc_core_hw_start(struct device *dev) +{ + struct stm32_adc_common *common = dev_get_drvdata(dev); + struct stm32_adc_priv *priv = to_stm32_adc_priv(common); + int ret; + + ret = regulator_enable(priv->vref); + if (ret < 0) { + dev_err(dev, "vref enable failed\n"); + return ret; + } + + if (priv->bclk) { + ret = clk_prepare_enable(priv->bclk); + if (ret < 0) { + dev_err(dev, "bus clk enable failed\n"); + goto err_regulator_disable; + } + } + + if (priv->aclk) { + ret = clk_prepare_enable(priv->aclk); + if (ret < 0) { + dev_err(dev, "adc clk enable failed\n"); + goto err_bclk_disable; + } + } + + writel_relaxed(priv->ccr_bak, priv->common.base + priv->cfg->regs->ccr); + + return 0; + +err_bclk_disable: + if (priv->bclk) + clk_disable_unprepare(priv->bclk); +err_regulator_disable: + regulator_disable(priv->vref); + + return ret; +} + +static void stm32_adc_core_hw_stop(struct device *dev) +{ + struct stm32_adc_common *common = dev_get_drvdata(dev); + struct stm32_adc_priv *priv = to_stm32_adc_priv(common); + + /* Backup CCR that may be lost (depends on power state to achieve) */ + priv->ccr_bak = readl_relaxed(priv->common.base + priv->cfg->regs->ccr); + if (priv->aclk) + clk_disable_unprepare(priv->aclk); + if (priv->bclk) + clk_disable_unprepare(priv->bclk); + regulator_disable(priv->vref); +} + static int stm32_adc_probe(struct platform_device *pdev) { struct stm32_adc_priv *priv; @@ -393,6 +457,7 @@ static int stm32_adc_probe(struct platform_device *pdev) priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); if (!priv) return -ENOMEM; + platform_set_drvdata(pdev, &priv->common); priv->cfg = (const struct stm32_adc_priv_cfg *) of_match_device(dev->driver->of_match_table, dev)->data; @@ -410,67 +475,51 @@ static int stm32_adc_probe(struct platform_device *pdev) return ret; } - ret = regulator_enable(priv->vref); - if (ret < 0) { - dev_err(&pdev->dev, "vref enable failed\n"); - return ret; - } - - ret = regulator_get_voltage(priv->vref); - if (ret < 0) { - dev_err(&pdev->dev, "vref get voltage failed, %d\n", ret); - goto err_regulator_disable; - } - priv->common.vref_mv = ret / 1000; - dev_dbg(&pdev->dev, "vref+=%dmV\n", priv->common.vref_mv); - priv->aclk = devm_clk_get(&pdev->dev, "adc"); if (IS_ERR(priv->aclk)) { ret = PTR_ERR(priv->aclk); - if (ret == -ENOENT) { - priv->aclk = NULL; - } else { + if (ret != -ENOENT) { dev_err(&pdev->dev, "Can't get 'adc' clock\n"); - goto err_regulator_disable; - } - } - - if (priv->aclk) { - ret = clk_prepare_enable(priv->aclk); - if (ret < 0) { - dev_err(&pdev->dev, "adc clk enable failed\n"); - goto err_regulator_disable; + return ret; } + priv->aclk = NULL; } priv->bclk = devm_clk_get(&pdev->dev, "bus"); if (IS_ERR(priv->bclk)) { ret = PTR_ERR(priv->bclk); - if (ret == -ENOENT) { - priv->bclk = NULL; - } else { + if (ret != -ENOENT) { dev_err(&pdev->dev, "Can't get 'bus' clock\n"); - goto err_aclk_disable; + return ret; } + priv->bclk = NULL; } - if (priv->bclk) { - ret = clk_prepare_enable(priv->bclk); - if (ret < 0) { - dev_err(&pdev->dev, "adc clk enable failed\n"); - goto err_aclk_disable; - } + pm_runtime_get_noresume(dev); + pm_runtime_set_active(dev); + pm_runtime_set_autosuspend_delay(dev, STM32_ADC_CORE_SLEEP_DELAY_MS); + pm_runtime_use_autosuspend(dev); + pm_runtime_enable(dev); + + ret = stm32_adc_core_hw_start(dev); + if (ret) + goto err_pm_stop; + + ret = regulator_get_voltage(priv->vref); + if (ret < 0) { + dev_err(&pdev->dev, "vref get voltage failed, %d\n", ret); + goto err_hw_stop; } + priv->common.vref_mv = ret / 1000; + dev_dbg(&pdev->dev, "vref+=%dmV\n", priv->common.vref_mv); ret = priv->cfg->clk_sel(pdev, priv); if (ret < 0) - goto err_bclk_disable; + goto err_hw_stop; ret = stm32_adc_irq_probe(pdev, priv); if (ret < 0) - goto err_bclk_disable; - - platform_set_drvdata(pdev, &priv->common); + goto err_hw_stop; ret = of_platform_populate(np, NULL, NULL, &pdev->dev); if (ret < 0) { @@ -478,21 +527,19 @@ static int stm32_adc_probe(struct platform_device *pdev) goto err_irq_remove; } + pm_runtime_mark_last_busy(dev); + pm_runtime_put_autosuspend(dev); + return 0; err_irq_remove: stm32_adc_irq_remove(pdev, priv); - -err_bclk_disable: - if (priv->bclk) - clk_disable_unprepare(priv->bclk); - -err_aclk_disable: - if (priv->aclk) - clk_disable_unprepare(priv->aclk); - -err_regulator_disable: - regulator_disable(priv->vref); +err_hw_stop: + stm32_adc_core_hw_stop(dev); +err_pm_stop: + pm_runtime_disable(dev); + pm_runtime_set_suspended(dev); + pm_runtime_put_noidle(dev); return ret; } @@ -502,17 +549,39 @@ static int stm32_adc_remove(struct platform_device *pdev) struct stm32_adc_common *common = platform_get_drvdata(pdev); struct stm32_adc_priv *priv = to_stm32_adc_priv(common); + pm_runtime_get_sync(&pdev->dev); of_platform_depopulate(&pdev->dev); stm32_adc_irq_remove(pdev, priv); - if (priv->bclk) - clk_disable_unprepare(priv->bclk); - if (priv->aclk) - clk_disable_unprepare(priv->aclk); - regulator_disable(priv->vref); + stm32_adc_core_hw_stop(&pdev->dev); + pm_runtime_disable(&pdev->dev); + pm_runtime_set_suspended(&pdev->dev); + pm_runtime_put_noidle(&pdev->dev); return 0; } +#if defined(CONFIG_PM) +static int stm32_adc_core_runtime_suspend(struct device *dev) +{ + stm32_adc_core_hw_stop(dev); + + return 0; +} + +static int stm32_adc_core_runtime_resume(struct device *dev) +{ + return stm32_adc_core_hw_start(dev); +} +#endif + +static const struct dev_pm_ops stm32_adc_core_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, + pm_runtime_force_resume) + SET_RUNTIME_PM_OPS(stm32_adc_core_runtime_suspend, + stm32_adc_core_runtime_resume, + NULL) +}; + static const struct stm32_adc_priv_cfg stm32f4_adc_priv_cfg = { .regs = &stm32f4_adc_common_regs, .clk_sel = stm32f4_adc_clk_sel, @@ -552,6 +621,7 @@ static int stm32_adc_remove(struct platform_device *pdev) .driver = { .name = "stm32-adc-core", .of_match_table = stm32_adc_of_match, + .pm = &stm32_adc_core_pm_ops, }, }; module_platform_driver(stm32_adc_driver); diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c index dca8733..32c9c61 100644 --- a/drivers/iio/adc/stm32-adc.c +++ b/drivers/iio/adc/stm32-adc.c @@ -22,6 +22,7 @@ #include <linux/iopoll.h> #include <linux/module.h> #include <linux/platform_device.h> +#include <linux/pm_runtime.h> #include <linux/of.h> #include <linux/of_device.h> @@ -148,6 +149,7 @@ enum stm32h7_adc_dmngt { #define STM32_ADC_MAX_SMP 7 /* SMPx range is [0..7] */ #define STM32_ADC_TIMEOUT_US 100000 #define STM32_ADC_TIMEOUT (msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000)) +#define STM32_ADC_HW_STOP_DELAY_MS 100 #define STM32_DMA_BUFFER_SIZE PAGE_SIZE @@ -623,6 +625,47 @@ static void stm32_adc_set_res(struct stm32_adc *adc) stm32_adc_writel(adc, res->reg, val); } +static int stm32_adc_hw_stop(struct device *dev) +{ + struct stm32_adc *adc = dev_get_drvdata(dev); + + if (adc->cfg->unprepare) + adc->cfg->unprepare(adc); + + if (adc->clk) + clk_disable_unprepare(adc->clk); + + return 0; +} + +static int stm32_adc_hw_start(struct device *dev) +{ + struct stm32_adc *adc = dev_get_drvdata(dev); + int ret; + + if (adc->clk) { + ret = clk_prepare_enable(adc->clk); + if (ret) + return ret; + } + + stm32_adc_set_res(adc); + + if (adc->cfg->prepare) { + ret = adc->cfg->prepare(adc); + if (ret) + goto err_clk_dis; + } + + return 0; + +err_clk_dis: + if (adc->clk) + clk_disable_unprepare(adc->clk); + + return ret; +} + /** * stm32f4_adc_start_conv() - Start conversions for regular channels. * @adc: stm32 adc instance @@ -1171,6 +1214,7 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev, int *res) { struct stm32_adc *adc = iio_priv(indio_dev); + struct device *dev = indio_dev->dev.parent; const struct stm32_adc_regspec *regs = adc->cfg->regs; long timeout; u32 val; @@ -1180,10 +1224,10 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev, adc->bufi = 0; - if (adc->cfg->prepare) { - ret = adc->cfg->prepare(adc); - if (ret) - return ret; + ret = pm_runtime_get_sync(dev); + if (ret < 0) { + pm_runtime_put_noidle(dev); + return ret; } /* Apply sampling time settings */ @@ -1221,8 +1265,8 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev, stm32_adc_conv_irq_disable(adc); - if (adc->cfg->unprepare) - adc->cfg->unprepare(adc); + pm_runtime_mark_last_busy(dev); + pm_runtime_put_autosuspend(dev); return ret; } @@ -1330,15 +1374,22 @@ static int stm32_adc_update_scan_mode(struct iio_dev *indio_dev, const unsigned long *scan_mask) { struct stm32_adc *adc = iio_priv(indio_dev); + struct device *dev = indio_dev->dev.parent; int ret; + ret = pm_runtime_get_sync(dev); + if (ret < 0) { + pm_runtime_put_noidle(dev); + return ret; + } + adc->num_conv = bitmap_weight(scan_mask, indio_dev->masklength); ret = stm32_adc_conf_scan_seq(indio_dev, scan_mask); - if (ret) - return ret; + pm_runtime_mark_last_busy(dev); + pm_runtime_put_autosuspend(dev); - return 0; + return ret; } static int stm32_adc_of_xlate(struct iio_dev *indio_dev, @@ -1368,12 +1419,23 @@ static int stm32_adc_debugfs_reg_access(struct iio_dev *indio_dev, unsigned *readval) { struct stm32_adc *adc = iio_priv(indio_dev); + struct device *dev = indio_dev->dev.parent; + int ret; + + ret = pm_runtime_get_sync(dev); + if (ret < 0) { + pm_runtime_put_noidle(dev); + return ret; + } if (!readval) stm32_adc_writel(adc, reg, writeval); else *readval = stm32_adc_readl(adc, reg); + pm_runtime_mark_last_busy(dev); + pm_runtime_put_autosuspend(dev); + return 0; } @@ -1459,18 +1521,19 @@ static int stm32_adc_dma_start(struct iio_dev *indio_dev) static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev) { struct stm32_adc *adc = iio_priv(indio_dev); + struct device *dev = indio_dev->dev.parent; int ret; - if (adc->cfg->prepare) { - ret = adc->cfg->prepare(adc); - if (ret) - return ret; + ret = pm_runtime_get_sync(dev); + if (ret < 0) { + pm_runtime_put_noidle(dev); + return ret; } ret = stm32_adc_set_trig(indio_dev, indio_dev->trig); if (ret) { dev_err(&indio_dev->dev, "Can't set trigger\n"); - goto err_unprepare; + goto err_pm_put; } ret = stm32_adc_dma_start(indio_dev); @@ -1498,9 +1561,9 @@ static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev) dmaengine_terminate_all(adc->dma_chan); err_clr_trig: stm32_adc_set_trig(indio_dev, NULL); -err_unprepare: - if (adc->cfg->unprepare) - adc->cfg->unprepare(adc); +err_pm_put: + pm_runtime_mark_last_busy(dev); + pm_runtime_put_autosuspend(dev); return ret; } @@ -1508,6 +1571,7 @@ static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev) static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev) { struct stm32_adc *adc = iio_priv(indio_dev); + struct device *dev = indio_dev->dev.parent; int ret; adc->cfg->stop_conv(adc); @@ -1524,8 +1588,8 @@ static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev) if (stm32_adc_set_trig(indio_dev, NULL)) dev_err(&indio_dev->dev, "Can't clear trigger\n"); - if (adc->cfg->unprepare) - adc->cfg->unprepare(adc); + pm_runtime_mark_last_busy(dev); + pm_runtime_put_autosuspend(dev); return ret; } @@ -1864,26 +1928,17 @@ static int stm32_adc_probe(struct platform_device *pdev) } } - if (adc->clk) { - ret = clk_prepare_enable(adc->clk); - if (ret < 0) { - dev_err(&pdev->dev, "clk enable failed\n"); - return ret; - } - } - ret = stm32_adc_of_get_resolution(indio_dev); if (ret < 0) - goto err_clk_disable; - stm32_adc_set_res(adc); + return ret; ret = stm32_adc_chan_of_init(indio_dev); if (ret < 0) - goto err_clk_disable; + return ret; ret = stm32_adc_dma_request(indio_dev); if (ret < 0) - goto err_clk_disable; + return ret; ret = iio_triggered_buffer_setup(indio_dev, &iio_pollfunc_store_time, @@ -1894,15 +1949,35 @@ static int stm32_adc_probe(struct platform_device *pdev) goto err_dma_disable; } + /* Get stm32-adc-core PM online */ + pm_runtime_get_noresume(dev); + pm_runtime_set_active(dev); + pm_runtime_set_autosuspend_delay(dev, STM32_ADC_HW_STOP_DELAY_MS); + pm_runtime_use_autosuspend(dev); + pm_runtime_enable(dev); + + ret = stm32_adc_hw_start(dev); + if (ret) + goto err_buffer_cleanup; + ret = iio_device_register(indio_dev); if (ret) { dev_err(&pdev->dev, "iio dev register failed\n"); - goto err_buffer_cleanup; + goto err_hw_stop; } + pm_runtime_mark_last_busy(dev); + pm_runtime_put_autosuspend(dev); + return 0; +err_hw_stop: + stm32_adc_hw_stop(dev); + err_buffer_cleanup: + pm_runtime_disable(dev); + pm_runtime_set_suspended(dev); + pm_runtime_put_noidle(dev); iio_triggered_buffer_cleanup(indio_dev); err_dma_disable: @@ -1912,9 +1987,6 @@ static int stm32_adc_probe(struct platform_device *pdev) adc->rx_buf, adc->rx_dma_buf); dma_release_channel(adc->dma_chan); } -err_clk_disable: - if (adc->clk) - clk_disable_unprepare(adc->clk); return ret; } @@ -1924,7 +1996,12 @@ static int stm32_adc_remove(struct platform_device *pdev) struct stm32_adc *adc = platform_get_drvdata(pdev); struct iio_dev *indio_dev = iio_priv_to_dev(adc); + pm_runtime_get_sync(&pdev->dev); iio_device_unregister(indio_dev); + stm32_adc_hw_stop(&pdev->dev); + pm_runtime_disable(&pdev->dev); + pm_runtime_set_suspended(&pdev->dev); + pm_runtime_put_noidle(&pdev->dev); iio_triggered_buffer_cleanup(indio_dev); if (adc->dma_chan) { dma_free_coherent(adc->dma_chan->device->dev, @@ -1932,12 +2009,29 @@ static int stm32_adc_remove(struct platform_device *pdev) adc->rx_buf, adc->rx_dma_buf); dma_release_channel(adc->dma_chan); } - if (adc->clk) - clk_disable_unprepare(adc->clk); return 0; } +#if defined(CONFIG_PM) +static int stm32_adc_runtime_suspend(struct device *dev) +{ + return stm32_adc_hw_stop(dev); +} + +static int stm32_adc_runtime_resume(struct device *dev) +{ + return stm32_adc_hw_start(dev); +} +#endif + +static const struct dev_pm_ops stm32_adc_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, + pm_runtime_force_resume) + SET_RUNTIME_PM_OPS(stm32_adc_runtime_suspend, stm32_adc_runtime_resume, + NULL) +}; + static const struct stm32_adc_cfg stm32f4_adc_cfg = { .regs = &stm32f4_adc_regspec, .adc_info = &stm32f4_adc_info, @@ -1985,6 +2079,7 @@ static int stm32_adc_remove(struct platform_device *pdev) .driver = { .name = "stm32-adc", .of_match_table = stm32_adc_of_match, + .pm = &stm32_adc_pm_ops, }, }; module_platform_driver(stm32_adc_driver); -- 1.9.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] iio: adc: stm32-adc: add power management support 2018-11-20 10:12 ` [PATCH 2/3] iio: adc: stm32-adc: add power management support Fabrice Gasnier @ 2018-11-25 13:16 ` Jonathan Cameron 0 siblings, 0 replies; 9+ messages in thread From: Jonathan Cameron @ 2018-11-25 13:16 UTC (permalink / raw) To: Fabrice Gasnier Cc: linux-arm-kernel, linux-kernel, mcoquelin.stm32, alexandre.torgue, linux-iio, lars, knaack.h, pmeerw, linux-stm32 On Tue, 20 Nov 2018 11:12:31 +0100 Fabrice Gasnier <fabrice.gasnier@st.com> wrote: > Add support for runtime PM & sleep. Move all regulator and clock management > to dedicated HW start/stop routines. Then rely on (runtime) PM OPS to > call them. > > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> Whilst I'll be the first to admit that runtime pm in particular gives me a headache everytime I try to review a patch with it in, this looks good to me. Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > --- > drivers/iio/adc/stm32-adc-core.c | 182 +++++++++++++++++++++++++++------------ > drivers/iio/adc/stm32-adc.c | 169 ++++++++++++++++++++++++++++-------- > 2 files changed, 258 insertions(+), 93 deletions(-) > > diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c > index ca432e7..2327ec1 100644 > --- a/drivers/iio/adc/stm32-adc-core.c > +++ b/drivers/iio/adc/stm32-adc-core.c > @@ -16,6 +16,7 @@ > #include <linux/irqdomain.h> > #include <linux/module.h> > #include <linux/of_device.h> > +#include <linux/pm_runtime.h> > #include <linux/regulator/consumer.h> > #include <linux/slab.h> > > @@ -48,15 +49,19 @@ > #define STM32H7_CKMODE_SHIFT 16 > #define STM32H7_CKMODE_MASK GENMASK(17, 16) > > +#define STM32_ADC_CORE_SLEEP_DELAY_MS 2000 > + > /** > * stm32_adc_common_regs - stm32 common registers, compatible dependent data > * @csr: common status register offset > + * @ccr: common control register offset > * @eoc1: adc1 end of conversion flag in @csr > * @eoc2: adc2 end of conversion flag in @csr > * @eoc3: adc3 end of conversion flag in @csr > */ > struct stm32_adc_common_regs { > u32 csr; > + u32 ccr; > u32 eoc1_msk; > u32 eoc2_msk; > u32 eoc3_msk; > @@ -85,6 +90,7 @@ struct stm32_adc_priv_cfg { > * @vref: regulator reference > * @cfg: compatible configuration data > * @common: common data for all ADC instances > + * @ccr_bak: backup CCR in low power mode > */ > struct stm32_adc_priv { > int irq[STM32_ADC_MAX_ADCS]; > @@ -94,6 +100,7 @@ struct stm32_adc_priv { > struct regulator *vref; > const struct stm32_adc_priv_cfg *cfg; > struct stm32_adc_common common; > + u32 ccr_bak; > }; > > static struct stm32_adc_priv *to_stm32_adc_priv(struct stm32_adc_common *com) > @@ -265,6 +272,7 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev, > /* STM32F4 common registers definitions */ > static const struct stm32_adc_common_regs stm32f4_adc_common_regs = { > .csr = STM32F4_ADC_CSR, > + .ccr = STM32F4_ADC_CCR, > .eoc1_msk = STM32F4_EOC1, > .eoc2_msk = STM32F4_EOC2, > .eoc3_msk = STM32F4_EOC3, > @@ -273,6 +281,7 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev, > /* STM32H7 common registers definitions */ > static const struct stm32_adc_common_regs stm32h7_adc_common_regs = { > .csr = STM32H7_ADC_CSR, > + .ccr = STM32H7_ADC_CCR, > .eoc1_msk = STM32H7_EOC_MST, > .eoc2_msk = STM32H7_EOC_SLV, > }; > @@ -379,6 +388,61 @@ static void stm32_adc_irq_remove(struct platform_device *pdev, > } > } > > +static int stm32_adc_core_hw_start(struct device *dev) > +{ > + struct stm32_adc_common *common = dev_get_drvdata(dev); > + struct stm32_adc_priv *priv = to_stm32_adc_priv(common); > + int ret; > + > + ret = regulator_enable(priv->vref); > + if (ret < 0) { > + dev_err(dev, "vref enable failed\n"); > + return ret; > + } > + > + if (priv->bclk) { > + ret = clk_prepare_enable(priv->bclk); > + if (ret < 0) { > + dev_err(dev, "bus clk enable failed\n"); > + goto err_regulator_disable; > + } > + } > + > + if (priv->aclk) { > + ret = clk_prepare_enable(priv->aclk); > + if (ret < 0) { > + dev_err(dev, "adc clk enable failed\n"); > + goto err_bclk_disable; > + } > + } > + > + writel_relaxed(priv->ccr_bak, priv->common.base + priv->cfg->regs->ccr); > + > + return 0; > + > +err_bclk_disable: > + if (priv->bclk) > + clk_disable_unprepare(priv->bclk); > +err_regulator_disable: > + regulator_disable(priv->vref); > + > + return ret; > +} > + > +static void stm32_adc_core_hw_stop(struct device *dev) > +{ > + struct stm32_adc_common *common = dev_get_drvdata(dev); > + struct stm32_adc_priv *priv = to_stm32_adc_priv(common); > + > + /* Backup CCR that may be lost (depends on power state to achieve) */ > + priv->ccr_bak = readl_relaxed(priv->common.base + priv->cfg->regs->ccr); > + if (priv->aclk) > + clk_disable_unprepare(priv->aclk); > + if (priv->bclk) > + clk_disable_unprepare(priv->bclk); > + regulator_disable(priv->vref); > +} > + > static int stm32_adc_probe(struct platform_device *pdev) > { > struct stm32_adc_priv *priv; > @@ -393,6 +457,7 @@ static int stm32_adc_probe(struct platform_device *pdev) > priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > if (!priv) > return -ENOMEM; > + platform_set_drvdata(pdev, &priv->common); > > priv->cfg = (const struct stm32_adc_priv_cfg *) > of_match_device(dev->driver->of_match_table, dev)->data; > @@ -410,67 +475,51 @@ static int stm32_adc_probe(struct platform_device *pdev) > return ret; > } > > - ret = regulator_enable(priv->vref); > - if (ret < 0) { > - dev_err(&pdev->dev, "vref enable failed\n"); > - return ret; > - } > - > - ret = regulator_get_voltage(priv->vref); > - if (ret < 0) { > - dev_err(&pdev->dev, "vref get voltage failed, %d\n", ret); > - goto err_regulator_disable; > - } > - priv->common.vref_mv = ret / 1000; > - dev_dbg(&pdev->dev, "vref+=%dmV\n", priv->common.vref_mv); > - > priv->aclk = devm_clk_get(&pdev->dev, "adc"); > if (IS_ERR(priv->aclk)) { > ret = PTR_ERR(priv->aclk); > - if (ret == -ENOENT) { > - priv->aclk = NULL; > - } else { > + if (ret != -ENOENT) { > dev_err(&pdev->dev, "Can't get 'adc' clock\n"); > - goto err_regulator_disable; > - } > - } > - > - if (priv->aclk) { > - ret = clk_prepare_enable(priv->aclk); > - if (ret < 0) { > - dev_err(&pdev->dev, "adc clk enable failed\n"); > - goto err_regulator_disable; > + return ret; > } > + priv->aclk = NULL; > } > > priv->bclk = devm_clk_get(&pdev->dev, "bus"); > if (IS_ERR(priv->bclk)) { > ret = PTR_ERR(priv->bclk); > - if (ret == -ENOENT) { > - priv->bclk = NULL; > - } else { > + if (ret != -ENOENT) { > dev_err(&pdev->dev, "Can't get 'bus' clock\n"); > - goto err_aclk_disable; > + return ret; > } > + priv->bclk = NULL; > } > > - if (priv->bclk) { > - ret = clk_prepare_enable(priv->bclk); > - if (ret < 0) { > - dev_err(&pdev->dev, "adc clk enable failed\n"); > - goto err_aclk_disable; > - } > + pm_runtime_get_noresume(dev); > + pm_runtime_set_active(dev); > + pm_runtime_set_autosuspend_delay(dev, STM32_ADC_CORE_SLEEP_DELAY_MS); > + pm_runtime_use_autosuspend(dev); > + pm_runtime_enable(dev); > + > + ret = stm32_adc_core_hw_start(dev); > + if (ret) > + goto err_pm_stop; > + > + ret = regulator_get_voltage(priv->vref); > + if (ret < 0) { > + dev_err(&pdev->dev, "vref get voltage failed, %d\n", ret); > + goto err_hw_stop; > } > + priv->common.vref_mv = ret / 1000; > + dev_dbg(&pdev->dev, "vref+=%dmV\n", priv->common.vref_mv); > > ret = priv->cfg->clk_sel(pdev, priv); > if (ret < 0) > - goto err_bclk_disable; > + goto err_hw_stop; > > ret = stm32_adc_irq_probe(pdev, priv); > if (ret < 0) > - goto err_bclk_disable; > - > - platform_set_drvdata(pdev, &priv->common); > + goto err_hw_stop; > > ret = of_platform_populate(np, NULL, NULL, &pdev->dev); > if (ret < 0) { > @@ -478,21 +527,19 @@ static int stm32_adc_probe(struct platform_device *pdev) > goto err_irq_remove; > } > > + pm_runtime_mark_last_busy(dev); > + pm_runtime_put_autosuspend(dev); > + > return 0; > > err_irq_remove: > stm32_adc_irq_remove(pdev, priv); > - > -err_bclk_disable: > - if (priv->bclk) > - clk_disable_unprepare(priv->bclk); > - > -err_aclk_disable: > - if (priv->aclk) > - clk_disable_unprepare(priv->aclk); > - > -err_regulator_disable: > - regulator_disable(priv->vref); > +err_hw_stop: > + stm32_adc_core_hw_stop(dev); > +err_pm_stop: > + pm_runtime_disable(dev); > + pm_runtime_set_suspended(dev); > + pm_runtime_put_noidle(dev); > > return ret; > } > @@ -502,17 +549,39 @@ static int stm32_adc_remove(struct platform_device *pdev) > struct stm32_adc_common *common = platform_get_drvdata(pdev); > struct stm32_adc_priv *priv = to_stm32_adc_priv(common); > > + pm_runtime_get_sync(&pdev->dev); > of_platform_depopulate(&pdev->dev); > stm32_adc_irq_remove(pdev, priv); > - if (priv->bclk) > - clk_disable_unprepare(priv->bclk); > - if (priv->aclk) > - clk_disable_unprepare(priv->aclk); > - regulator_disable(priv->vref); > + stm32_adc_core_hw_stop(&pdev->dev); > + pm_runtime_disable(&pdev->dev); > + pm_runtime_set_suspended(&pdev->dev); > + pm_runtime_put_noidle(&pdev->dev); > > return 0; > } > > +#if defined(CONFIG_PM) > +static int stm32_adc_core_runtime_suspend(struct device *dev) > +{ > + stm32_adc_core_hw_stop(dev); > + > + return 0; > +} > + > +static int stm32_adc_core_runtime_resume(struct device *dev) > +{ > + return stm32_adc_core_hw_start(dev); > +} > +#endif > + > +static const struct dev_pm_ops stm32_adc_core_pm_ops = { > + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, > + pm_runtime_force_resume) > + SET_RUNTIME_PM_OPS(stm32_adc_core_runtime_suspend, > + stm32_adc_core_runtime_resume, > + NULL) > +}; > + > static const struct stm32_adc_priv_cfg stm32f4_adc_priv_cfg = { > .regs = &stm32f4_adc_common_regs, > .clk_sel = stm32f4_adc_clk_sel, > @@ -552,6 +621,7 @@ static int stm32_adc_remove(struct platform_device *pdev) > .driver = { > .name = "stm32-adc-core", > .of_match_table = stm32_adc_of_match, > + .pm = &stm32_adc_core_pm_ops, > }, > }; > module_platform_driver(stm32_adc_driver); > diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c > index dca8733..32c9c61 100644 > --- a/drivers/iio/adc/stm32-adc.c > +++ b/drivers/iio/adc/stm32-adc.c > @@ -22,6 +22,7 @@ > #include <linux/iopoll.h> > #include <linux/module.h> > #include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > #include <linux/of.h> > #include <linux/of_device.h> > > @@ -148,6 +149,7 @@ enum stm32h7_adc_dmngt { > #define STM32_ADC_MAX_SMP 7 /* SMPx range is [0..7] */ > #define STM32_ADC_TIMEOUT_US 100000 > #define STM32_ADC_TIMEOUT (msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000)) > +#define STM32_ADC_HW_STOP_DELAY_MS 100 > > #define STM32_DMA_BUFFER_SIZE PAGE_SIZE > > @@ -623,6 +625,47 @@ static void stm32_adc_set_res(struct stm32_adc *adc) > stm32_adc_writel(adc, res->reg, val); > } > > +static int stm32_adc_hw_stop(struct device *dev) > +{ > + struct stm32_adc *adc = dev_get_drvdata(dev); > + > + if (adc->cfg->unprepare) > + adc->cfg->unprepare(adc); > + > + if (adc->clk) > + clk_disable_unprepare(adc->clk); > + > + return 0; > +} > + > +static int stm32_adc_hw_start(struct device *dev) > +{ > + struct stm32_adc *adc = dev_get_drvdata(dev); > + int ret; > + > + if (adc->clk) { > + ret = clk_prepare_enable(adc->clk); > + if (ret) > + return ret; > + } > + > + stm32_adc_set_res(adc); > + > + if (adc->cfg->prepare) { > + ret = adc->cfg->prepare(adc); > + if (ret) > + goto err_clk_dis; > + } > + > + return 0; > + > +err_clk_dis: > + if (adc->clk) > + clk_disable_unprepare(adc->clk); > + > + return ret; > +} > + > /** > * stm32f4_adc_start_conv() - Start conversions for regular channels. > * @adc: stm32 adc instance > @@ -1171,6 +1214,7 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev, > int *res) > { > struct stm32_adc *adc = iio_priv(indio_dev); > + struct device *dev = indio_dev->dev.parent; > const struct stm32_adc_regspec *regs = adc->cfg->regs; > long timeout; > u32 val; > @@ -1180,10 +1224,10 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev, > > adc->bufi = 0; > > - if (adc->cfg->prepare) { > - ret = adc->cfg->prepare(adc); > - if (ret) > - return ret; > + ret = pm_runtime_get_sync(dev); > + if (ret < 0) { > + pm_runtime_put_noidle(dev); > + return ret; > } > > /* Apply sampling time settings */ > @@ -1221,8 +1265,8 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev, > > stm32_adc_conv_irq_disable(adc); > > - if (adc->cfg->unprepare) > - adc->cfg->unprepare(adc); > + pm_runtime_mark_last_busy(dev); > + pm_runtime_put_autosuspend(dev); > > return ret; > } > @@ -1330,15 +1374,22 @@ static int stm32_adc_update_scan_mode(struct iio_dev *indio_dev, > const unsigned long *scan_mask) > { > struct stm32_adc *adc = iio_priv(indio_dev); > + struct device *dev = indio_dev->dev.parent; > int ret; > > + ret = pm_runtime_get_sync(dev); > + if (ret < 0) { > + pm_runtime_put_noidle(dev); > + return ret; > + } > + > adc->num_conv = bitmap_weight(scan_mask, indio_dev->masklength); > > ret = stm32_adc_conf_scan_seq(indio_dev, scan_mask); > - if (ret) > - return ret; > + pm_runtime_mark_last_busy(dev); > + pm_runtime_put_autosuspend(dev); > > - return 0; > + return ret; > } > > static int stm32_adc_of_xlate(struct iio_dev *indio_dev, > @@ -1368,12 +1419,23 @@ static int stm32_adc_debugfs_reg_access(struct iio_dev *indio_dev, > unsigned *readval) > { > struct stm32_adc *adc = iio_priv(indio_dev); > + struct device *dev = indio_dev->dev.parent; > + int ret; > + > + ret = pm_runtime_get_sync(dev); > + if (ret < 0) { > + pm_runtime_put_noidle(dev); > + return ret; > + } > > if (!readval) > stm32_adc_writel(adc, reg, writeval); > else > *readval = stm32_adc_readl(adc, reg); > > + pm_runtime_mark_last_busy(dev); > + pm_runtime_put_autosuspend(dev); > + > return 0; > } > > @@ -1459,18 +1521,19 @@ static int stm32_adc_dma_start(struct iio_dev *indio_dev) > static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev) > { > struct stm32_adc *adc = iio_priv(indio_dev); > + struct device *dev = indio_dev->dev.parent; > int ret; > > - if (adc->cfg->prepare) { > - ret = adc->cfg->prepare(adc); > - if (ret) > - return ret; > + ret = pm_runtime_get_sync(dev); > + if (ret < 0) { > + pm_runtime_put_noidle(dev); > + return ret; > } > > ret = stm32_adc_set_trig(indio_dev, indio_dev->trig); > if (ret) { > dev_err(&indio_dev->dev, "Can't set trigger\n"); > - goto err_unprepare; > + goto err_pm_put; > } > > ret = stm32_adc_dma_start(indio_dev); > @@ -1498,9 +1561,9 @@ static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev) > dmaengine_terminate_all(adc->dma_chan); > err_clr_trig: > stm32_adc_set_trig(indio_dev, NULL); > -err_unprepare: > - if (adc->cfg->unprepare) > - adc->cfg->unprepare(adc); > +err_pm_put: > + pm_runtime_mark_last_busy(dev); > + pm_runtime_put_autosuspend(dev); > > return ret; > } > @@ -1508,6 +1571,7 @@ static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev) > static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev) > { > struct stm32_adc *adc = iio_priv(indio_dev); > + struct device *dev = indio_dev->dev.parent; > int ret; > > adc->cfg->stop_conv(adc); > @@ -1524,8 +1588,8 @@ static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev) > if (stm32_adc_set_trig(indio_dev, NULL)) > dev_err(&indio_dev->dev, "Can't clear trigger\n"); > > - if (adc->cfg->unprepare) > - adc->cfg->unprepare(adc); > + pm_runtime_mark_last_busy(dev); > + pm_runtime_put_autosuspend(dev); > > return ret; > } > @@ -1864,26 +1928,17 @@ static int stm32_adc_probe(struct platform_device *pdev) > } > } > > - if (adc->clk) { > - ret = clk_prepare_enable(adc->clk); > - if (ret < 0) { > - dev_err(&pdev->dev, "clk enable failed\n"); > - return ret; > - } > - } > - > ret = stm32_adc_of_get_resolution(indio_dev); > if (ret < 0) > - goto err_clk_disable; > - stm32_adc_set_res(adc); > + return ret; > > ret = stm32_adc_chan_of_init(indio_dev); > if (ret < 0) > - goto err_clk_disable; > + return ret; > > ret = stm32_adc_dma_request(indio_dev); > if (ret < 0) > - goto err_clk_disable; > + return ret; > > ret = iio_triggered_buffer_setup(indio_dev, > &iio_pollfunc_store_time, > @@ -1894,15 +1949,35 @@ static int stm32_adc_probe(struct platform_device *pdev) > goto err_dma_disable; > } > > + /* Get stm32-adc-core PM online */ > + pm_runtime_get_noresume(dev); > + pm_runtime_set_active(dev); > + pm_runtime_set_autosuspend_delay(dev, STM32_ADC_HW_STOP_DELAY_MS); > + pm_runtime_use_autosuspend(dev); > + pm_runtime_enable(dev); > + > + ret = stm32_adc_hw_start(dev); > + if (ret) > + goto err_buffer_cleanup; > + > ret = iio_device_register(indio_dev); > if (ret) { > dev_err(&pdev->dev, "iio dev register failed\n"); > - goto err_buffer_cleanup; > + goto err_hw_stop; > } > > + pm_runtime_mark_last_busy(dev); > + pm_runtime_put_autosuspend(dev); > + > return 0; > > +err_hw_stop: > + stm32_adc_hw_stop(dev); > + > err_buffer_cleanup: > + pm_runtime_disable(dev); > + pm_runtime_set_suspended(dev); > + pm_runtime_put_noidle(dev); > iio_triggered_buffer_cleanup(indio_dev); > > err_dma_disable: > @@ -1912,9 +1987,6 @@ static int stm32_adc_probe(struct platform_device *pdev) > adc->rx_buf, adc->rx_dma_buf); > dma_release_channel(adc->dma_chan); > } > -err_clk_disable: > - if (adc->clk) > - clk_disable_unprepare(adc->clk); > > return ret; > } > @@ -1924,7 +1996,12 @@ static int stm32_adc_remove(struct platform_device *pdev) > struct stm32_adc *adc = platform_get_drvdata(pdev); > struct iio_dev *indio_dev = iio_priv_to_dev(adc); > > + pm_runtime_get_sync(&pdev->dev); > iio_device_unregister(indio_dev); > + stm32_adc_hw_stop(&pdev->dev); > + pm_runtime_disable(&pdev->dev); > + pm_runtime_set_suspended(&pdev->dev); > + pm_runtime_put_noidle(&pdev->dev); > iio_triggered_buffer_cleanup(indio_dev); > if (adc->dma_chan) { > dma_free_coherent(adc->dma_chan->device->dev, > @@ -1932,12 +2009,29 @@ static int stm32_adc_remove(struct platform_device *pdev) > adc->rx_buf, adc->rx_dma_buf); > dma_release_channel(adc->dma_chan); > } > - if (adc->clk) > - clk_disable_unprepare(adc->clk); > > return 0; > } > > +#if defined(CONFIG_PM) > +static int stm32_adc_runtime_suspend(struct device *dev) > +{ > + return stm32_adc_hw_stop(dev); > +} > + > +static int stm32_adc_runtime_resume(struct device *dev) > +{ > + return stm32_adc_hw_start(dev); > +} > +#endif > + > +static const struct dev_pm_ops stm32_adc_pm_ops = { > + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, > + pm_runtime_force_resume) > + SET_RUNTIME_PM_OPS(stm32_adc_runtime_suspend, stm32_adc_runtime_resume, > + NULL) > +}; > + > static const struct stm32_adc_cfg stm32f4_adc_cfg = { > .regs = &stm32f4_adc_regspec, > .adc_info = &stm32f4_adc_info, > @@ -1985,6 +2079,7 @@ static int stm32_adc_remove(struct platform_device *pdev) > .driver = { > .name = "stm32-adc", > .of_match_table = stm32_adc_of_match, > + .pm = &stm32_adc_pm_ops, > }, > }; > module_platform_driver(stm32_adc_driver); ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] iio: adc: stm32-adc: switch off running adc when going to low power 2018-11-20 10:12 [PATCH 0/3] iio: stm32-adc: add PM support Fabrice Gasnier 2018-11-20 10:12 ` [PATCH 1/3] iio: adc: stm32-adc: move self-calibration to prepare routine Fabrice Gasnier 2018-11-20 10:12 ` [PATCH 2/3] iio: adc: stm32-adc: add power management support Fabrice Gasnier @ 2018-11-20 10:12 ` Fabrice Gasnier 2018-11-25 13:17 ` Jonathan Cameron 2 siblings, 1 reply; 9+ messages in thread From: Fabrice Gasnier @ 2018-11-20 10:12 UTC (permalink / raw) To: jic23 Cc: linux-arm-kernel, linux-kernel, mcoquelin.stm32, alexandre.torgue, fabrice.gasnier, linux-iio, lars, knaack.h, pmeerw, linux-stm32 Switch off ADC when going to low power mode, in case it has been left running in buffer mode. Then re-enable it when resuming. Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> --- drivers/iio/adc/stm32-adc.c | 79 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 63 insertions(+), 16 deletions(-) diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c index 32c9c61..2a9891c 100644 --- a/drivers/iio/adc/stm32-adc.c +++ b/drivers/iio/adc/stm32-adc.c @@ -1518,7 +1518,7 @@ static int stm32_adc_dma_start(struct iio_dev *indio_dev) return 0; } -static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev) +static int __stm32_adc_buffer_postenable(struct iio_dev *indio_dev) { struct stm32_adc *adc = iio_priv(indio_dev); struct device *dev = indio_dev->dev.parent; @@ -1542,10 +1542,6 @@ static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev) goto err_clr_trig; } - ret = iio_triggered_buffer_postenable(indio_dev); - if (ret < 0) - goto err_stop_dma; - /* Reset adc buffer index */ adc->bufi = 0; @@ -1556,9 +1552,6 @@ static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev) return 0; -err_stop_dma: - if (adc->dma_chan) - dmaengine_terminate_all(adc->dma_chan); err_clr_trig: stm32_adc_set_trig(indio_dev, NULL); err_pm_put: @@ -1568,20 +1561,30 @@ static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev) return ret; } -static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev) +static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev) +{ + int ret; + + ret = iio_triggered_buffer_postenable(indio_dev); + if (ret < 0) + return ret; + + ret = __stm32_adc_buffer_postenable(indio_dev); + if (ret < 0) + iio_triggered_buffer_predisable(indio_dev); + + return ret; +} + +static void __stm32_adc_buffer_predisable(struct iio_dev *indio_dev) { struct stm32_adc *adc = iio_priv(indio_dev); struct device *dev = indio_dev->dev.parent; - int ret; adc->cfg->stop_conv(adc); if (!adc->dma_chan) stm32_adc_conv_irq_disable(adc); - ret = iio_triggered_buffer_predisable(indio_dev); - if (ret < 0) - dev_err(&indio_dev->dev, "predisable failed\n"); - if (adc->dma_chan) dmaengine_terminate_all(adc->dma_chan); @@ -1590,6 +1593,17 @@ static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev) pm_runtime_mark_last_busy(dev); pm_runtime_put_autosuspend(dev); +} + +static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev) +{ + int ret; + + __stm32_adc_buffer_predisable(indio_dev); + + ret = iio_triggered_buffer_predisable(indio_dev); + if (ret < 0) + dev_err(&indio_dev->dev, "predisable failed\n"); return ret; } @@ -2013,6 +2027,40 @@ static int stm32_adc_remove(struct platform_device *pdev) return 0; } +#if defined(CONFIG_PM_SLEEP) +static int stm32_adc_suspend(struct device *dev) +{ + struct stm32_adc *adc = dev_get_drvdata(dev); + struct iio_dev *indio_dev = iio_priv_to_dev(adc); + + if (iio_buffer_enabled(indio_dev)) + __stm32_adc_buffer_predisable(indio_dev); + + return pm_runtime_force_suspend(dev); +} + +static int stm32_adc_resume(struct device *dev) +{ + struct stm32_adc *adc = dev_get_drvdata(dev); + struct iio_dev *indio_dev = iio_priv_to_dev(adc); + int ret; + + ret = pm_runtime_force_resume(dev); + if (ret < 0) + return ret; + + if (!iio_buffer_enabled(indio_dev)) + return 0; + + ret = stm32_adc_update_scan_mode(indio_dev, + indio_dev->active_scan_mask); + if (ret < 0) + return ret; + + return __stm32_adc_buffer_postenable(indio_dev); +} +#endif + #if defined(CONFIG_PM) static int stm32_adc_runtime_suspend(struct device *dev) { @@ -2026,8 +2074,7 @@ static int stm32_adc_runtime_resume(struct device *dev) #endif static const struct dev_pm_ops stm32_adc_pm_ops = { - SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, - pm_runtime_force_resume) + SET_SYSTEM_SLEEP_PM_OPS(stm32_adc_suspend, stm32_adc_resume) SET_RUNTIME_PM_OPS(stm32_adc_runtime_suspend, stm32_adc_runtime_resume, NULL) }; -- 1.9.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] iio: adc: stm32-adc: switch off running adc when going to low power 2018-11-20 10:12 ` [PATCH 3/3] iio: adc: stm32-adc: switch off running adc when going to low power Fabrice Gasnier @ 2018-11-25 13:17 ` Jonathan Cameron 0 siblings, 0 replies; 9+ messages in thread From: Jonathan Cameron @ 2018-11-25 13:17 UTC (permalink / raw) To: Fabrice Gasnier Cc: linux-arm-kernel, linux-kernel, mcoquelin.stm32, alexandre.torgue, linux-iio, lars, knaack.h, pmeerw, linux-stm32 On Tue, 20 Nov 2018 11:12:32 +0100 Fabrice Gasnier <fabrice.gasnier@st.com> wrote: > Switch off ADC when going to low power mode, in case it has been left > running in buffer mode. Then re-enable it when resuming. > > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> My suspicion is that we have other drivers not correctly handing this case, but as far as I can see you have it well covered here. Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > --- > drivers/iio/adc/stm32-adc.c | 79 ++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 63 insertions(+), 16 deletions(-) > > diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c > index 32c9c61..2a9891c 100644 > --- a/drivers/iio/adc/stm32-adc.c > +++ b/drivers/iio/adc/stm32-adc.c > @@ -1518,7 +1518,7 @@ static int stm32_adc_dma_start(struct iio_dev *indio_dev) > return 0; > } > > -static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev) > +static int __stm32_adc_buffer_postenable(struct iio_dev *indio_dev) > { > struct stm32_adc *adc = iio_priv(indio_dev); > struct device *dev = indio_dev->dev.parent; > @@ -1542,10 +1542,6 @@ static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev) > goto err_clr_trig; > } > > - ret = iio_triggered_buffer_postenable(indio_dev); > - if (ret < 0) > - goto err_stop_dma; > - > /* Reset adc buffer index */ > adc->bufi = 0; > > @@ -1556,9 +1552,6 @@ static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev) > > return 0; > > -err_stop_dma: > - if (adc->dma_chan) > - dmaengine_terminate_all(adc->dma_chan); > err_clr_trig: > stm32_adc_set_trig(indio_dev, NULL); > err_pm_put: > @@ -1568,20 +1561,30 @@ static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev) > return ret; > } > > -static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev) > +static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev) > +{ > + int ret; > + > + ret = iio_triggered_buffer_postenable(indio_dev); > + if (ret < 0) > + return ret; > + > + ret = __stm32_adc_buffer_postenable(indio_dev); > + if (ret < 0) > + iio_triggered_buffer_predisable(indio_dev); > + > + return ret; > +} > + > +static void __stm32_adc_buffer_predisable(struct iio_dev *indio_dev) > { > struct stm32_adc *adc = iio_priv(indio_dev); > struct device *dev = indio_dev->dev.parent; > - int ret; > > adc->cfg->stop_conv(adc); > if (!adc->dma_chan) > stm32_adc_conv_irq_disable(adc); > > - ret = iio_triggered_buffer_predisable(indio_dev); > - if (ret < 0) > - dev_err(&indio_dev->dev, "predisable failed\n"); > - > if (adc->dma_chan) > dmaengine_terminate_all(adc->dma_chan); > > @@ -1590,6 +1593,17 @@ static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev) > > pm_runtime_mark_last_busy(dev); > pm_runtime_put_autosuspend(dev); > +} > + > +static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev) > +{ > + int ret; > + > + __stm32_adc_buffer_predisable(indio_dev); > + > + ret = iio_triggered_buffer_predisable(indio_dev); > + if (ret < 0) > + dev_err(&indio_dev->dev, "predisable failed\n"); > > return ret; > } > @@ -2013,6 +2027,40 @@ static int stm32_adc_remove(struct platform_device *pdev) > return 0; > } > > +#if defined(CONFIG_PM_SLEEP) > +static int stm32_adc_suspend(struct device *dev) > +{ > + struct stm32_adc *adc = dev_get_drvdata(dev); > + struct iio_dev *indio_dev = iio_priv_to_dev(adc); > + > + if (iio_buffer_enabled(indio_dev)) > + __stm32_adc_buffer_predisable(indio_dev); > + > + return pm_runtime_force_suspend(dev); > +} > + > +static int stm32_adc_resume(struct device *dev) > +{ > + struct stm32_adc *adc = dev_get_drvdata(dev); > + struct iio_dev *indio_dev = iio_priv_to_dev(adc); > + int ret; > + > + ret = pm_runtime_force_resume(dev); > + if (ret < 0) > + return ret; > + > + if (!iio_buffer_enabled(indio_dev)) > + return 0; > + > + ret = stm32_adc_update_scan_mode(indio_dev, > + indio_dev->active_scan_mask); > + if (ret < 0) > + return ret; > + > + return __stm32_adc_buffer_postenable(indio_dev); > +} > +#endif > + > #if defined(CONFIG_PM) > static int stm32_adc_runtime_suspend(struct device *dev) > { > @@ -2026,8 +2074,7 @@ static int stm32_adc_runtime_resume(struct device *dev) > #endif > > static const struct dev_pm_ops stm32_adc_pm_ops = { > - SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, > - pm_runtime_force_resume) > + SET_SYSTEM_SLEEP_PM_OPS(stm32_adc_suspend, stm32_adc_resume) > SET_RUNTIME_PM_OPS(stm32_adc_runtime_suspend, stm32_adc_runtime_resume, > NULL) > }; ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-11-26 10:41 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-20 10:12 [PATCH 0/3] iio: stm32-adc: add PM support Fabrice Gasnier 2018-11-20 10:12 ` [PATCH 1/3] iio: adc: stm32-adc: move self-calibration to prepare routine Fabrice Gasnier 2018-11-25 13:03 ` Jonathan Cameron 2018-11-25 13:14 ` Jonathan Cameron 2018-11-26 10:41 ` Fabrice Gasnier 2018-11-20 10:12 ` [PATCH 2/3] iio: adc: stm32-adc: add power management support Fabrice Gasnier 2018-11-25 13:16 ` Jonathan Cameron 2018-11-20 10:12 ` [PATCH 3/3] iio: adc: stm32-adc: switch off running adc when going to low power Fabrice Gasnier 2018-11-25 13:17 ` 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).