All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iio: adc: stm32: fix stm32h7_adc_enable error handling
@ 2018-01-23 16:04 ` Fabrice Gasnier
  0 siblings, 0 replies; 4+ messages in thread
From: Fabrice Gasnier @ 2018-01-23 16:04 UTC (permalink / raw)
  To: jic23
  Cc: linux-arm-kernel, linux-kernel, mcoquelin.stm32,
	alexandre.torgue, fabrice.gasnier, linux-iio, lars, knaack.h,
	pmeerw

Error handling in stm32h7_adc_enable routine doesn't unwind enable
sequence correctly. ADEN can only be cleared by hardware (e.g. by
writing one to ADDIS).
It's also better to clear ADRDY just after it's been set by hardware.

Fixes: 95e339b6e85d ("iio: adc: stm32: add support for STM32H7")

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/iio/adc/stm32-adc.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index ca3b865..8177a92 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -735,8 +735,6 @@ static int stm32h7_adc_enable(struct stm32_adc *adc)
 	int ret;
 	u32 val;
 
-	/* Clear ADRDY by writing one, then enable ADC */
-	stm32_adc_set_bits(adc, STM32H7_ADC_ISR, STM32H7_ADRDY);
 	stm32_adc_set_bits(adc, STM32H7_ADC_CR, STM32H7_ADEN);
 
 	/* Poll for ADRDY to be set (after adc startup time) */
@@ -744,8 +742,11 @@ static int stm32h7_adc_enable(struct stm32_adc *adc)
 					   val & STM32H7_ADRDY,
 					   100, STM32_ADC_TIMEOUT_US);
 	if (ret) {
-		stm32_adc_clr_bits(adc, STM32H7_ADC_CR, STM32H7_ADEN);
+		stm32_adc_set_bits(adc, STM32H7_ADC_CR, STM32H7_ADDIS);
 		dev_err(&indio_dev->dev, "Failed to enable ADC\n");
+	} else {
+		/* Clear ADRDY by writing one */
+		stm32_adc_set_bits(adc, STM32H7_ADC_ISR, STM32H7_ADRDY);
 	}
 
 	return ret;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH] iio: adc: stm32: fix stm32h7_adc_enable error handling
@ 2018-01-23 16:04 ` Fabrice Gasnier
  0 siblings, 0 replies; 4+ messages in thread
From: Fabrice Gasnier @ 2018-01-23 16:04 UTC (permalink / raw)
  To: linux-arm-kernel

Error handling in stm32h7_adc_enable routine doesn't unwind enable
sequence correctly. ADEN can only be cleared by hardware (e.g. by
writing one to ADDIS).
It's also better to clear ADRDY just after it's been set by hardware.

Fixes: 95e339b6e85d ("iio: adc: stm32: add support for STM32H7")

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/iio/adc/stm32-adc.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index ca3b865..8177a92 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -735,8 +735,6 @@ static int stm32h7_adc_enable(struct stm32_adc *adc)
 	int ret;
 	u32 val;
 
-	/* Clear ADRDY by writing one, then enable ADC */
-	stm32_adc_set_bits(adc, STM32H7_ADC_ISR, STM32H7_ADRDY);
 	stm32_adc_set_bits(adc, STM32H7_ADC_CR, STM32H7_ADEN);
 
 	/* Poll for ADRDY to be set (after adc startup time) */
@@ -744,8 +742,11 @@ static int stm32h7_adc_enable(struct stm32_adc *adc)
 					   val & STM32H7_ADRDY,
 					   100, STM32_ADC_TIMEOUT_US);
 	if (ret) {
-		stm32_adc_clr_bits(adc, STM32H7_ADC_CR, STM32H7_ADEN);
+		stm32_adc_set_bits(adc, STM32H7_ADC_CR, STM32H7_ADDIS);
 		dev_err(&indio_dev->dev, "Failed to enable ADC\n");
+	} else {
+		/* Clear ADRDY by writing one */
+		stm32_adc_set_bits(adc, STM32H7_ADC_ISR, STM32H7_ADRDY);
 	}
 
 	return ret;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] iio: adc: stm32: fix stm32h7_adc_enable error handling
  2018-01-23 16:04 ` Fabrice Gasnier
@ 2018-01-28  8:19   ` Jonathan Cameron
  -1 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2018-01-28  8:19 UTC (permalink / raw)
  To: Fabrice Gasnier
  Cc: linux-arm-kernel, linux-kernel, mcoquelin.stm32,
	alexandre.torgue, linux-iio, lars, knaack.h, pmeerw

On Tue, 23 Jan 2018 17:04:56 +0100
Fabrice Gasnier <fabrice.gasnier@st.com> wrote:

> Error handling in stm32h7_adc_enable routine doesn't unwind enable
> sequence correctly. ADEN can only be cleared by hardware (e.g. by
> writing one to ADDIS).
> It's also better to clear ADRDY just after it's been set by hardware.
> 
> Fixes: 95e339b6e85d ("iio: adc: stm32: add support for STM32H7")
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Applied to fixes-togreg-post-rc1 branch of iio.git and marked for
stable.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/stm32-adc.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index ca3b865..8177a92 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -735,8 +735,6 @@ static int stm32h7_adc_enable(struct stm32_adc *adc)
>  	int ret;
>  	u32 val;
>  
> -	/* Clear ADRDY by writing one, then enable ADC */
> -	stm32_adc_set_bits(adc, STM32H7_ADC_ISR, STM32H7_ADRDY);
>  	stm32_adc_set_bits(adc, STM32H7_ADC_CR, STM32H7_ADEN);
>  
>  	/* Poll for ADRDY to be set (after adc startup time) */
> @@ -744,8 +742,11 @@ static int stm32h7_adc_enable(struct stm32_adc *adc)
>  					   val & STM32H7_ADRDY,
>  					   100, STM32_ADC_TIMEOUT_US);
>  	if (ret) {
> -		stm32_adc_clr_bits(adc, STM32H7_ADC_CR, STM32H7_ADEN);
> +		stm32_adc_set_bits(adc, STM32H7_ADC_CR, STM32H7_ADDIS);
>  		dev_err(&indio_dev->dev, "Failed to enable ADC\n");
> +	} else {
> +		/* Clear ADRDY by writing one */
> +		stm32_adc_set_bits(adc, STM32H7_ADC_ISR, STM32H7_ADRDY);
>  	}
>  
>  	return ret;

^ permalink raw reply	[flat|nested] 4+ messages in thread
* [PATCH] iio: adc: stm32: fix stm32h7_adc_enable error handling
@ 2018-01-28  8:19   ` Jonathan Cameron
  0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2018-01-28  8:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 23 Jan 2018 17:04:56 +0100
Fabrice Gasnier <fabrice.gasnier@st.com> wrote:

> Error handling in stm32h7_adc_enable routine doesn't unwind enable
> sequence correctly. ADEN can only be cleared by hardware (e.g. by
> writing one to ADDIS).
> It's also better to clear ADRDY just after it's been set by hardware.
> 
> Fixes: 95e339b6e85d ("iio: adc: stm32: add support for STM32H7")
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Applied to fixes-togreg-post-rc1 branch of iio.git and marked for
stable.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/stm32-adc.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index ca3b865..8177a92 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -735,8 +735,6 @@ static int stm32h7_adc_enable(struct stm32_adc *adc)
>  	int ret;
>  	u32 val;
>  
> -	/* Clear ADRDY by writing one, then enable ADC */
> -	stm32_adc_set_bits(adc, STM32H7_ADC_ISR, STM32H7_ADRDY);
>  	stm32_adc_set_bits(adc, STM32H7_ADC_CR, STM32H7_ADEN);
>  
>  	/* Poll for ADRDY to be set (after adc startup time) */
> @@ -744,8 +742,11 @@ static int stm32h7_adc_enable(struct stm32_adc *adc)
>  					   val & STM32H7_ADRDY,
>  					   100, STM32_ADC_TIMEOUT_US);
>  	if (ret) {
> -		stm32_adc_clr_bits(adc, STM32H7_ADC_CR, STM32H7_ADEN);
> +		stm32_adc_set_bits(adc, STM32H7_ADC_CR, STM32H7_ADDIS);
>  		dev_err(&indio_dev->dev, "Failed to enable ADC\n");
> +	} else {
> +		/* Clear ADRDY by writing one */
> +		stm32_adc_set_bits(adc, STM32H7_ADC_ISR, STM32H7_ADRDY);
>  	}
>  
>  	return ret;

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-01-28  8:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-23 16:04 [PATCH] iio: adc: stm32: fix stm32h7_adc_enable error handling Fabrice Gasnier
2018-01-23 16:04 ` Fabrice Gasnier
2018-01-28  8:19 ` Jonathan Cameron
2018-01-28  8:19   ` Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.