Re: [PATCH 3/7] iio: adc: stm32: add trigger polarity extended attribute

Fabrice Gasnier
2017-01-24T14:42:34Z

On 01/22/2017 01:58 PM, Jonathan Cameron wrote:
> On 19/01/17 13:34, Fabrice Gasnier wrote:
>> Define extended attribute so that user may choose rising, falling or both
>> edges for external trigger sources.
>> Default to rising edge in case it isn't set.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Creates a custom attibute. Documentation please!
> /Documentation/ABI/testing/sysfs-bus-iio-stm32 or similar.

Ok, I'll add something like:
Documentation/ABI/testing/sysfs-bus-iio-adc-stm32:

+What: /sys/bus/iio/devices/triggerX/trigger_polarity
+KernelVersion: 4.11
+Contact: fabrice.gasnier@st.com
+Description:
+ The STM32 ADC can be configured to use external trigger sources
+ (e.g. timers, pwm or exti gpio). Then, it can be tuned to start
+ conversions on external trigger by either:
+ - "rising-edge"
+ - "falling-edge"
+ - "both-edges".
+ Reading returns current trigger polarity.
+ Writing value before enabling conversions sets trigger polarity.

Best Regards,
Thanks,
Fabrice
>
> The approach seems reasonable, but easier to discuss when it's
> formally described.
>
> I'd also not care about characters saved and go with
> trigger_polarity rather than the short form (unless this you have
> a good reason for it!)
>
> Jonathan
>> ---
>> drivers/iio/adc/stm32-adc.c | 51 ++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 50 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
>> index 30708bc..9753c39 100644
>> --- a/drivers/iio/adc/stm32-adc.c
>> +++ b/drivers/iio/adc/stm32-adc.c
>> @@ -164,6 +164,7 @@ struct stm32_adc_trig_info {
>> * @lock: spinlock
>> * @bufi: data buffer index
>> * @num_conv: expected number of scan conversions
>> + * @exten: external trigger config (enable/polarity)
>> */
>> struct stm32_adc {
>> struct stm32_adc_common *common;
>> @@ -175,6 +176,7 @@ struct stm32_adc {
>> spinlock_t lock; /* interrupt lock */
>> int bufi;
>> int num_conv;
>> + enum stm32_adc_exten exten;
>> };
>>
>> /**
>> @@ -449,7 +451,9 @@ static int stm32_adc_set_trig(struct iio_dev *indio_dev,
>>
>> /* set trigger source, default to rising edge */
>> extsel = ret;
>> - exten = STM32_EXTEN_HWTRIG_RISING_EDGE;
>> + if (adc->exten == STM32_EXTEN_SWTRIG)
>> + adc->exten = STM32_EXTEN_HWTRIG_RISING_EDGE;
>> + exten = adc->exten;
>> }
>>
>> spin_lock_irqsave(&adc->lock, flags);
>> @@ -463,6 +467,39 @@ static int stm32_adc_set_trig(struct iio_dev *indio_dev,
>> return 0;
>> }
>>
>> +static int stm32_adc_set_trig_pol(struct iio_dev *indio_dev,
>> + const struct iio_chan_spec *chan,
>> + unsigned int type)
>> +{
>> + struct stm32_adc *adc = iio_priv(indio_dev);
>> +
>> + adc->exten = type;
>> +
>> + return 0;
>> +}
>> +
>> +static int stm32_adc_get_trig_pol(struct iio_dev *indio_dev,
>> + const struct iio_chan_spec *chan)
>> +{
>> + struct stm32_adc *adc = iio_priv(indio_dev);
>> +
>> + return adc->exten;
>> +}
>> +
>> +static const char * const stm32_trig_pol_items[] = {
>> + [STM32_EXTEN_SWTRIG] = "swtrig",
>> + [STM32_EXTEN_HWTRIG_RISING_EDGE] = "rising-edge",
>> + [STM32_EXTEN_HWTRIG_FALLING_EDGE] = "falling-edge",
>> + [STM32_EXTEN_HWTRIG_BOTH_EDGES] = "both-edges",
>> +};
>> +
>> +const struct iio_enum stm32_adc_trig_pol = {
>> + .items = stm32_trig_pol_items,
>> + .num_items = ARRAY_SIZE(stm32_trig_pol_items),
>> + .get = stm32_adc_get_trig_pol,
>> + .set = stm32_adc_set_trig_pol,
>> +};
>> +
>> /**
>> * stm32_adc_single_conv() - Performs a single conversion
>> * @indio_dev: IIO device
>> @@ -745,6 +782,17 @@ static irqreturn_t stm32_adc_trigger_handler(int irq, void *p)
>> return IRQ_HANDLED;
>> }
>>
>> +static const struct iio_chan_spec_ext_info stm32_adc_ext_info[] = {
>> + IIO_ENUM("trigger_pol", IIO_SHARED_BY_ALL, &stm32_adc_trig_pol),
>> + {
>> + .name = "trigger_pol_available",
>> + .shared = IIO_SHARED_BY_ALL,
>> + .read = iio_enum_available_read,
>> + .private = (uintptr_t)&stm32_adc_trig_pol,
>> + },
>> + {},
>> +};
>> +
>> static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>> struct iio_chan_spec *chan,
>> const struct stm32_adc_chan_spec *channel,
>> @@ -760,6 +808,7 @@ static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>> chan->scan_type.sign = 'u';
>> chan->scan_type.realbits = 12;
>> chan->scan_type.storagebits = 16;
>> + chan->ext_info = stm32_adc_ext_info;
>> }
>>
>> static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>>
>