From: Jonathan Cameron <jic23@kernel.org>
To: Fabrice Gasnier <fabrice.gasnier@st.com>
Cc: <linux-arm-kernel@lists.infradead.org>,
<linux-kernel@vger.kernel.org>, <mcoquelin.stm32@gmail.com>,
<alexandre.torgue@st.com>, <linux-iio@vger.kernel.org>,
<lars@metafoo.de>, <knaack.h@gmx.de>, <pmeerw@pmeerw.net>,
<linux-stm32@st-md-mailman.stormreply.com>
Subject: Re: [PATCH 1/3] iio: adc: stm32-adc: move self-calibration to prepare routine
Date: Sun, 25 Nov 2018 13:03:39 +0000 [thread overview]
Message-ID: <20181125130339.61430399@archlinux> (raw)
In-Reply-To: <1542708752-26163-2-git-send-email-fabrice.gasnier@st.com>
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,
next prev parent reply other threads:[~2018-11-25 13:03 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181125130339.61430399@archlinux \
--to=jic23@kernel.org \
--cc=alexandre.torgue@st.com \
--cc=fabrice.gasnier@st.com \
--cc=knaack.h@gmx.de \
--cc=lars@metafoo.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=mcoquelin.stm32@gmail.com \
--cc=pmeerw@pmeerw.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).