Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1589633 > unrolled thread

[PATCH v3 6/6] iio: adc: stm32: add support for EXTI trigger

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v3 6/6] iio: adc: stm32: add support for EXTI trigger Fabrice Gasnier <fabrice.gasnier@st.com> - 2017-02-28 18:20 +0100
    Re: [PATCH v3 6/6] iio: adc: stm32: add support for EXTI trigger Lars-Peter Clausen <lars@metafoo.de> - 2017-03-03 13:50 +0100
      Re: [PATCH v3 6/6] iio: adc: stm32: add support for EXTI trigger Fabrice Gasnier <fabrice.gasnier@st.com> - 2017-03-03 15:40 +0100
        Re: [PATCH v3 6/6] iio: adc: stm32: add support for EXTI trigger Lars-Peter Clausen <lars@metafoo.de> - 2017-03-03 17:00 +0100
          Re: [PATCH v3 6/6] iio: adc: stm32: add support for EXTI trigger Lars-Peter Clausen <lars@metafoo.de> - 2017-03-03 17:00 +0100
    Re: [PATCH v3 6/6] iio: adc: stm32: add support for EXTI trigger Jonathan Cameron <jic23@kernel.org> - 2017-03-05 13:30 +0100
      Re: [PATCH v3 6/6] iio: adc: stm32: add support for EXTI trigger Jonathan Cameron <jic23@kernel.org> - 2017-03-05 13:40 +0100

#1589633 — [PATCH v3 6/6] iio: adc: stm32: add support for EXTI trigger

EXTi (external interrupt) signal can be routed internally as trigger
source for ADC conversions: STM32F4 ADC can use EXTI11.

Retrieve interrupt trigger from DT, so it can be muxed into ADC IP,
via extsel.

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

diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index 9b49a6ad..4d9040d 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -108,6 +108,9 @@ enum stm32_adc_extsel {
 	STM32_EXT15,
 };
 
+/* EXTI 11 trigger selection on STM32F4 */
+#define STM32F4_EXTI11_EXTSEL		STM32_EXT15
+
 /**
  * struct stm32_adc_trig_info - ADC trigger info
  * @name:		name of the trigger, corresponding to its source
@@ -146,6 +149,7 @@ struct stm32_adc_regs {
  * @rx_buf:		dma rx buffer cpu address
  * @rx_dma_buf:		dma rx buffer bus address
  * @rx_buf_sz:		dma rx buffer size
+ * @exti_trig		EXTI trigger
  */
 struct stm32_adc {
 	struct stm32_adc_common	*common;
@@ -162,6 +166,7 @@ struct stm32_adc {
 	u8			*rx_buf;
 	dma_addr_t		rx_dma_buf;
 	unsigned int		rx_buf_sz;
+	struct iio_trigger	*exti_trig;
 };
 
 /**
@@ -395,10 +400,15 @@ static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
  *
  * Returns trigger extsel value, if trig matches, -EINVAL otherwise.
  */
-static int stm32_adc_get_trig_extsel(struct iio_trigger *trig)
+static int stm32_adc_get_trig_extsel(struct iio_dev *indio_dev,
+				     struct iio_trigger *trig)
 {
+	struct stm32_adc *adc = iio_priv(indio_dev);
 	int i;
 
+	if (trig == adc->exti_trig)
+		return STM32F4_EXTI11_EXTSEL;
+
 	/* lookup triggers registered by stm32 timer trigger driver */
 	for (i = 0; stm32f4_adc_trigs[i].name; i++) {
 		/**
@@ -432,7 +442,7 @@ static int stm32_adc_set_trig(struct iio_dev *indio_dev,
 	int ret;
 
 	if (trig) {
-		ret = stm32_adc_get_trig_extsel(trig);
+		ret = stm32_adc_get_trig_extsel(indio_dev, trig);
 		if (ret < 0)
 			return ret;
 
@@ -604,7 +614,7 @@ static irqreturn_t stm32_adc_isr(int irq, void *data)
 static int stm32_adc_validate_trigger(struct iio_dev *indio_dev,
 				      struct iio_trigger *trig)
 {
-	return stm32_adc_get_trig_extsel(trig) < 0 ? -EINVAL : 0;
+	return stm32_adc_get_trig_extsel(indio_dev, trig) < 0 ? -EINVAL : 0;
 }
 
 static int stm32_adc_set_watermark(struct iio_dev *indio_dev, unsigned int val)
@@ -1030,6 +1040,17 @@ static int stm32_adc_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_clk_disable;
 
+	adc->exti_trig = devm_iio_trigger_get_by_name(&pdev->dev, "exti");
+	if (IS_ERR(adc->exti_trig)) {
+		ret = PTR_ERR(adc->exti_trig);
+		if (ret == -EPROBE_DEFER)
+			goto err_dma_disable;
+		dev_dbg(&pdev->dev, "No exti trigger found (%d)\n", ret);
+		adc->exti_trig = NULL;
+	} else {
+		dev_info(&pdev->dev, "Can use %s\n", adc->exti_trig->name);
+	}
+
 	ret = iio_triggered_buffer_setup(indio_dev,
 					 &iio_pollfunc_store_time,
 					 &stm32_adc_trigger_handler,
-- 
1.9.1

[toc] | [next] | [standalone]

#1591919

On 02/28/2017 05:51 PM, Fabrice Gasnier wrote:
> EXTi (external interrupt) signal can be routed internally as trigger
> source for ADC conversions: STM32F4 ADC can use EXTI11.
> 
> Retrieve interrupt trigger from DT, so it can be muxed into ADC IP,
> via extsel.

Hi,

Sorry, I have some trouble understanding the specifics of this. Is EXTI a
hardware input signal into the ADC that automatically triggers a conversion
when asserted? If yes how is it configured which external signal is used
here. Your bindings suggest that any GPIO can be used, but the driver only
differentiates between EXTI on or EXTI off.

Or is this just a software triggered conversion? The GPIO triggers a
software interrupt routine and the interrupt routine than triggers conversion?

[toc] | [prev] | [next] | [standalone]

#1591998

On 03/03/2017 12:45 PM, Lars-Peter Clausen wrote:
> On 02/28/2017 05:51 PM, Fabrice Gasnier wrote:
>> EXTi (external interrupt) signal can be routed internally as trigger
>> source for ADC conversions: STM32F4 ADC can use EXTI11.
>>
>> Retrieve interrupt trigger from DT, so it can be muxed into ADC IP,
>> via extsel.
> 
> Hi,
> 
> Sorry, I have some trouble understanding the specifics of this. Is EXTI a
> hardware input signal into the ADC that automatically triggers a conversion
> when asserted? If yes how is it configured which external signal is used
> here. Your bindings suggest that any GPIO can be used, but the driver only
> differentiates between EXTI on or EXTI off.
Hi Lars,

Yes, STM32 EXTI is external interrupt/event line. In case of STM32 ADC,
EXTI11 signal can be used to start a conversion. In this case, it must
be selected inside ADC IP using extsel bitfield. This EXTI11 line can
mapped from any GPIO bank A,B... line 11 (e.g. PA11 or PB11...) by using
interrupt binding.
This is why I expose this in DT.

> 
> Or is this just a software triggered conversion? The GPIO triggers a
> software interrupt routine and the interrupt routine than triggers conversion?
> 

[toc] | [prev] | [next] | [standalone]

#1592093

On 03/03/2017 02:00 PM, Fabrice Gasnier wrote:
> On 03/03/2017 12:45 PM, Lars-Peter Clausen wrote:
>> On 02/28/2017 05:51 PM, Fabrice Gasnier wrote:
>>> EXTi (external interrupt) signal can be routed internally as trigger
>>> source for ADC conversions: STM32F4 ADC can use EXTI11.
>>>
>>> Retrieve interrupt trigger from DT, so it can be muxed into ADC IP,
>>> via extsel.
>>
>> Hi,
>>
>> Sorry, I have some trouble understanding the specifics of this. Is EXTI a
>> hardware input signal into the ADC that automatically triggers a conversion
>> when asserted? If yes how is it configured which external signal is used
>> here. Your bindings suggest that any GPIO can be used, but the driver only
>> differentiates between EXTI on or EXTI off.
> Hi Lars,
> 
> Yes, STM32 EXTI is external interrupt/event line. In case of STM32 ADC,
> EXTI11 signal can be used to start a conversion. In this case, it must
> be selected inside ADC IP using extsel bitfield. This EXTI11 line can
> mapped from any GPIO bank A,B... line 11 (e.g. PA11 or PB11...) by using
> interrupt binding.
> This is why I expose this in DT.

How is the mapping? That's the part I don't understand. How does requesting
the IRQ for the GPIO as a generic software IRQ establish the hardware
connection between the GPIO block and the ADC?

[toc] | [prev] | [next] | [standalone]

#1592094

On 03/03/2017 04:46 PM, Lars-Peter Clausen wrote:
> On 03/03/2017 02:00 PM, Fabrice Gasnier wrote:
>> On 03/03/2017 12:45 PM, Lars-Peter Clausen wrote:
>>> On 02/28/2017 05:51 PM, Fabrice Gasnier wrote:
>>>> EXTi (external interrupt) signal can be routed internally as trigger
>>>> source for ADC conversions: STM32F4 ADC can use EXTI11.
>>>>
>>>> Retrieve interrupt trigger from DT, so it can be muxed into ADC IP,
>>>> via extsel.
>>>
>>> Hi,
>>>
>>> Sorry, I have some trouble understanding the specifics of this. Is EXTI a
>>> hardware input signal into the ADC that automatically triggers a conversion
>>> when asserted? If yes how is it configured which external signal is used
>>> here. Your bindings suggest that any GPIO can be used, but the driver only
>>> differentiates between EXTI on or EXTI off.
>> Hi Lars,
>>
>> Yes, STM32 EXTI is external interrupt/event line. In case of STM32 ADC,
>> EXTI11 signal can be used to start a conversion. In this case, it must
>> be selected inside ADC IP using extsel bitfield. This EXTI11 line can
>> mapped from any GPIO bank A,B... line 11 (e.g. PA11 or PB11...) by using
>> interrupt binding.
>> This is why I expose this in DT.
> 
> How is the mapping? That's the part I don't understand. How does requesting

How is the mapping done?

> the IRQ for the GPIO as a generic software IRQ establish the hardware
> connection between the GPIO block and the ADC?
> 

[toc] | [prev] | [next] | [standalone]

#1592753

On 28/02/17 16:51, Fabrice Gasnier wrote:
> EXTi (external interrupt) signal can be routed internally as trigger
> source for ADC conversions: STM32F4 ADC can use EXTI11.
> 
> Retrieve interrupt trigger from DT, so it can be muxed into ADC IP,
> via extsel.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Minor question inline.
J
> ---
>  drivers/iio/adc/stm32-adc.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 9b49a6ad..4d9040d 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -108,6 +108,9 @@ enum stm32_adc_extsel {
>  	STM32_EXT15,
>  };
>  
> +/* EXTI 11 trigger selection on STM32F4 */
> +#define STM32F4_EXTI11_EXTSEL		STM32_EXT15
> +
>  /**
>   * struct stm32_adc_trig_info - ADC trigger info
>   * @name:		name of the trigger, corresponding to its source
> @@ -146,6 +149,7 @@ struct stm32_adc_regs {
>   * @rx_buf:		dma rx buffer cpu address
>   * @rx_dma_buf:		dma rx buffer bus address
>   * @rx_buf_sz:		dma rx buffer size
> + * @exti_trig		EXTI trigger
>   */
>  struct stm32_adc {
>  	struct stm32_adc_common	*common;
> @@ -162,6 +166,7 @@ struct stm32_adc {
>  	u8			*rx_buf;
>  	dma_addr_t		rx_dma_buf;
>  	unsigned int		rx_buf_sz;
> +	struct iio_trigger	*exti_trig;
>  };
>  
>  /**
> @@ -395,10 +400,15 @@ static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
>   *
>   * Returns trigger extsel value, if trig matches, -EINVAL otherwise.
>   */
> -static int stm32_adc_get_trig_extsel(struct iio_trigger *trig)
> +static int stm32_adc_get_trig_extsel(struct iio_dev *indio_dev,
> +				     struct iio_trigger *trig)
>  {
> +	struct stm32_adc *adc = iio_priv(indio_dev);
>  	int i;
>  
> +	if (trig == adc->exti_trig)
> +		return STM32F4_EXTI11_EXTSEL;
> +
>  	/* lookup triggers registered by stm32 timer trigger driver */
>  	for (i = 0; stm32f4_adc_trigs[i].name; i++) {
>  		/**
> @@ -432,7 +442,7 @@ static int stm32_adc_set_trig(struct iio_dev *indio_dev,
>  	int ret;
>  
>  	if (trig) {
> -		ret = stm32_adc_get_trig_extsel(trig);
> +		ret = stm32_adc_get_trig_extsel(indio_dev, trig);
>  		if (ret < 0)
>  			return ret;
>  
> @@ -604,7 +614,7 @@ static irqreturn_t stm32_adc_isr(int irq, void *data)
>  static int stm32_adc_validate_trigger(struct iio_dev *indio_dev,
>  				      struct iio_trigger *trig)
>  {
> -	return stm32_adc_get_trig_extsel(trig) < 0 ? -EINVAL : 0;
> +	return stm32_adc_get_trig_extsel(indio_dev, trig) < 0 ? -EINVAL : 0;
>  }
>  
>  static int stm32_adc_set_watermark(struct iio_dev *indio_dev, unsigned int val)
> @@ -1030,6 +1040,17 @@ static int stm32_adc_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		goto err_clk_disable;
>  
> +	adc->exti_trig = devm_iio_trigger_get_by_name(&pdev->dev, "exti");
Can we tighten this in any way? If not I guess we'll have to rely on no
one messing up the devicetree to put the wrong trigger in there with this name.

> +	if (IS_ERR(adc->exti_trig)) {
> +		ret = PTR_ERR(adc->exti_trig);
> +		if (ret == -EPROBE_DEFER)
> +			goto err_dma_disable;
> +		dev_dbg(&pdev->dev, "No exti trigger found (%d)\n", ret);
> +		adc->exti_trig = NULL;
> +	} else {
> +		dev_info(&pdev->dev, "Can use %s\n", adc->exti_trig->name);
> +	}
> +
>  	ret = iio_triggered_buffer_setup(indio_dev,
>  					 &iio_pollfunc_store_time,
>  					 &stm32_adc_trigger_handler,
> 

[toc] | [prev] | [next] | [standalone]

#1592754

On 05/03/17 12:21, Jonathan Cameron wrote:
> On 28/02/17 16:51, Fabrice Gasnier wrote:
>> EXTi (external interrupt) signal can be routed internally as trigger
>> source for ADC conversions: STM32F4 ADC can use EXTI11.
>>
>> Retrieve interrupt trigger from DT, so it can be muxed into ADC IP,
>> via extsel.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Minor question inline.
and another.
> J
>> ---
>>  drivers/iio/adc/stm32-adc.c | 27 ++++++++++++++++++++++++---
>>  1 file changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
>> index 9b49a6ad..4d9040d 100644
>> --- a/drivers/iio/adc/stm32-adc.c
>> +++ b/drivers/iio/adc/stm32-adc.c
>> @@ -108,6 +108,9 @@ enum stm32_adc_extsel {
>>  	STM32_EXT15,
>>  };
>>  
>> +/* EXTI 11 trigger selection on STM32F4 */
>> +#define STM32F4_EXTI11_EXTSEL		STM32_EXT15
>> +
>>  /**
>>   * struct stm32_adc_trig_info - ADC trigger info
>>   * @name:		name of the trigger, corresponding to its source
>> @@ -146,6 +149,7 @@ struct stm32_adc_regs {
>>   * @rx_buf:		dma rx buffer cpu address
>>   * @rx_dma_buf:		dma rx buffer bus address
>>   * @rx_buf_sz:		dma rx buffer size
>> + * @exti_trig		EXTI trigger
>>   */
>>  struct stm32_adc {
>>  	struct stm32_adc_common	*common;
>> @@ -162,6 +166,7 @@ struct stm32_adc {
>>  	u8			*rx_buf;
>>  	dma_addr_t		rx_dma_buf;
>>  	unsigned int		rx_buf_sz;
>> +	struct iio_trigger	*exti_trig;
>>  };
>>  
>>  /**
>> @@ -395,10 +400,15 @@ static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
>>   *
>>   * Returns trigger extsel value, if trig matches, -EINVAL otherwise.
>>   */
>> -static int stm32_adc_get_trig_extsel(struct iio_trigger *trig)
>> +static int stm32_adc_get_trig_extsel(struct iio_dev *indio_dev,
>> +				     struct iio_trigger *trig)
>>  {
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>>  	int i;
>>  
>> +	if (trig == adc->exti_trig)
>> +		return STM32F4_EXTI11_EXTSEL;
>> +
>>  	/* lookup triggers registered by stm32 timer trigger driver */
>>  	for (i = 0; stm32f4_adc_trigs[i].name; i++) {
>>  		/**
>> @@ -432,7 +442,7 @@ static int stm32_adc_set_trig(struct iio_dev *indio_dev,
>>  	int ret;
>>  
>>  	if (trig) {
>> -		ret = stm32_adc_get_trig_extsel(trig);
>> +		ret = stm32_adc_get_trig_extsel(indio_dev, trig);
>>  		if (ret < 0)
>>  			return ret;
>>  
>> @@ -604,7 +614,7 @@ static irqreturn_t stm32_adc_isr(int irq, void *data)
>>  static int stm32_adc_validate_trigger(struct iio_dev *indio_dev,
>>  				      struct iio_trigger *trig)
>>  {
>> -	return stm32_adc_get_trig_extsel(trig) < 0 ? -EINVAL : 0;
>> +	return stm32_adc_get_trig_extsel(indio_dev, trig) < 0 ? -EINVAL : 0;
>>  }
>>  
>>  static int stm32_adc_set_watermark(struct iio_dev *indio_dev, unsigned int val)
>> @@ -1030,6 +1040,17 @@ static int stm32_adc_probe(struct platform_device *pdev)
>>  	if (ret < 0)
>>  		goto err_clk_disable;
>>  
>> +	adc->exti_trig = devm_iio_trigger_get_by_name(&pdev->dev, "exti");
> Can we tighten this in any way? If not I guess we'll have to rely on no
> one messing up the devicetree to put the wrong trigger in there with this name.

Also, given I'm arguing for this association to not be done in devicetree but
rather left to userspace, can we move this get to only occur during validate
trigger rather than here?  To my mind we shouldn't even know the trigger
is registered at during the ADC probe.  Also has the benefit of getting rid
of needing to handle the deferred element.
> 
>> +	if (IS_ERR(adc->exti_trig)) {
>> +		ret = PTR_ERR(adc->exti_trig);
>> +		if (ret == -EPROBE_DEFER)
>> +			goto err_dma_disable;
>> +		dev_dbg(&pdev->dev, "No exti trigger found (%d)\n", ret);
>> +		adc->exti_trig = NULL;
>> +	} else {
>> +		dev_info(&pdev->dev, "Can use %s\n", adc->exti_trig->name);
>> +	}
>> +
>>  	ret = iio_triggered_buffer_setup(indio_dev,
>>  					 &iio_pollfunc_store_time,
>>  					 &stm32_adc_trigger_handler,
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web