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


Groups > linux.kernel > #1592753 > unrolled thread

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

Started byJonathan Cameron <jic23@kernel.org>
First post2017-03-05 13:30 +0100
Last post2017-03-05 13:40 +0100
Articles 2 — 1 participant

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

  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

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

FromJonathan Cameron <jic23@kernel.org>
Date2017-03-05 13:30 +0100
SubjectRe: [PATCH v3 6/6] iio: adc: stm32: add support for EXTI trigger
Message-ID<thDsu-bD-5@gated-at.bofh.it>
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] | [next] | [standalone]


#1592754

FromJonathan Cameron <jic23@kernel.org>
Date2017-03-05 13:40 +0100
Message-ID<thDCa-gN-13@gated-at.bofh.it>
In reply to#1592753
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