On 24/01/17 14:37, Fabrice Gasnier wrote:
> On 01/22/2017 01:55 PM, Jonathan Cameron wrote:
>> On 19/01/17 13:34, Fabrice Gasnier wrote:
>>> STM32 ADC has external timer trigger sources. Use stm32 timer triggers
>>> API (e.g. is_stm32_timer_trigger()) with local ADC lookup table to
>>> validate a trigger can be used.
>>> This also provides correct trigger selection value (e.g. extsel).
>>>
>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@xxxxxx>
>> Looks good. Observations inline.
>>> ---
>>> drivers/iio/adc/Kconfig | 2 ++
>>> drivers/iio/adc/stm32-adc.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 62 insertions(+)
>>>
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index 33341f4..9a7b090 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -447,6 +447,8 @@ config STM32_ADC_CORE
>>> depends on OF
>>> depends on REGULATOR
>>> select IIO_BUFFER
>>> + select MFD_STM32_TIMERS
>>> + select IIO_STM32_TIMER_TRIGGER
>>> select IIO_TRIGGERED_BUFFER
>>> help
>>> Select this option to enable the core driver for STMicroelectronics
>>> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
>>> index 8d0b74b..30708bc 100644
>>> --- a/drivers/iio/adc/stm32-adc.c
>>> +++ b/drivers/iio/adc/stm32-adc.c
>>> @@ -23,6 +23,7 @@
>>> #include <linux/delay.h>
>>> #include <linux/iio/iio.h>
>>> #include <linux/iio/buffer.h>
>>> +#include <linux/iio/timer/stm32-timer-trigger.h>
>>> #include <linux/iio/trigger.h>
>>> #include <linux/iio/trigger_consumer.h>
>>> #include <linux/iio/triggered_buffer.h>
>>> @@ -122,6 +123,35 @@ enum stm32_adc_exten {
>>> STM32_EXTEN_HWTRIG_BOTH_EDGES,
>>> };
>>>
>>> +/* extsel - trigger mux selection value */
>>> +enum stm32_adc_extsel {
>>> + STM32_EXT0,
>>> + STM32_EXT1,
>>> + STM32_EXT2,
>>> + STM32_EXT3,
>>> + STM32_EXT4,
>>> + STM32_EXT5,
>>> + STM32_EXT6,
>>> + STM32_EXT7,
>>> + STM32_EXT8,
>>> + STM32_EXT9,
>>> + STM32_EXT10,
>>> + STM32_EXT11,
>>> + STM32_EXT12,
>>> + STM32_EXT13,
>>> + STM32_EXT14,
>>> + STM32_EXT15,
>>> +};
>>> +
>>> +/**
>>> + * struct stm32_adc_trig_info - ADC trigger info
>>> + * @name: name of the trigger, corresponding to its source
>>> + * @extsel: trigger selection
>>> + */
>>> +struct stm32_adc_trig_info {
>>> + const char *name;
>>> + enum stm32_adc_extsel extsel;
>>> +};
>>>
>>> /**
>>> * struct stm32_adc - private data of each ADC IIO instance
>>> @@ -218,6 +248,26 @@ struct stm32_adc_regs {
>>> { STM32F4_ADC_SQR1, STM32F4_SQ16_MASK, STM32F4_SQ16_SHIFT },
>>> };
>>>
>>> +/* STM32F4 external trigger sources for all instances */
>>> +static struct stm32_adc_trig_info stm32f4_adc_timer_trigs[] = {
>>> + { TIM1_CH1, STM32_EXT0 },
>>> + { TIM1_CH2, STM32_EXT1 },
>>> + { TIM1_CH3, STM32_EXT2 },
>>> + { TIM2_CH2, STM32_EXT3 },
>>> + { TIM2_CH3, STM32_EXT4 },
>>> + { TIM2_CH4, STM32_EXT5 },
>>> + { TIM2_TRGO, STM32_EXT6 },
>>> + { TIM3_CH1, STM32_EXT7 },
>>> + { TIM3_TRGO, STM32_EXT8 },
>>> + { TIM4_CH4, STM32_EXT9 },
>>> + { TIM5_CH1, STM32_EXT10 },
>>> + { TIM5_CH2, STM32_EXT11 },
>>> + { TIM5_CH3, STM32_EXT12 },
>>> + { TIM8_CH1, STM32_EXT13 },
>>> + { TIM8_TRGO, STM32_EXT14 },
>>> + {}, /* sentinel */
>>> +};
>>> +
>>> /**
>>> * STM32 ADC registers access routines
>>> * @adc: stm32 adc instance
>>> @@ -362,6 +412,16 @@ static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
>>> static int stm32_adc_get_trig_extsel(struct iio_dev *indio_dev,
>>> struct iio_trigger *trig)
>>> {
>>> + int i;
>> Ah. This makes more sense than patch 1 on it's own did.
>>> +
>>> + /* lookup triggers registered by stm32 timer trigger driver */
>>> + for (i = 0; stm32f4_adc_timer_trigs[i].name; i++) {
>>> + if (is_stm32_timer_trigger(trig) &&
>>> + !strcmp(stm32f4_adc_timer_trigs[i].name, trig->name)) {
>>> + return stm32f4_adc_timer_trigs[i].extsel;
>> Good. The combination of the first check and the name match should make this safe
>> against those triggers that can be assigned arbitrary names.
>
> Do you wish I add a comment about it ?
Nope. I just thought it was nicely done ;)
>
> Best Regards,
> Fabrice
>
>>> + }
>>> + }
>>> +
>>> return -EINVAL;
>>> }
>>>
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
