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

[Fabrice Gasnier]

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@xxxxxx>
> 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@xxxxxx
+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)