| Message ID | 1484832854-6314-4-git-send-email-fabrice.gasnier@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. 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) >
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) >> >
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)
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> --- drivers/iio/adc/stm32-adc.c | 51 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-)