Message ID | 20250107-ad7380-add-alert-support-v3-4-bce10afd656b@baylibre.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | iio: adc: ad7380: add alert support | expand |
On 1/7/25 2:48 AM, Julien Stephan wrote: > The alert functionality is an out of range indicator and can be used as > an early indicator of an out of bounds conversion result. > > ALERT_LOW_THRESHOLD and ALERT_HIGH_THRESHOLD registers are common to all > channels. > > When using 1 SDO line (only mode supported by the driver right now), i.e > data outputs only on SDOA, SDOB (or SDOD for 4 channels variants) is > used as an alert pin. The alert pin is updated at the end of the > conversion (set to low if an alert occurs) and is cleared on a falling > edge of CS. > > The ALERT register contains information about the exact alert status: > channel and direction. ALERT register can be accessed using debugfs if > enabled. > > User can set high/low thresholds and enable alert detection using the > regular iio events attributes: > > events/in_thresh_falling_value events/in_thresh_rising_value > events/thresh_either_en > > In most use cases, user will hardwire the alert pin to trigger a shutdown. > > In theory, we could generate userspace IIO events for alerts, but this > is not implemented yet for several reasons [1]. This can be implemented > later if a real use case actually requires it. > > Signed-off-by: Julien Stephan <jstephan@baylibre.com> > > [1] https://lore.kernel.org/all/4be16272-5197-4fa1-918c-c4cdfcaee02e@baylibre.com/ > --- ... > +static int ad7380_read_event_config(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, > + enum iio_event_type type, > + enum iio_event_direction dir) > +{ > + struct ad7380_state *st = iio_priv(indio_dev); > + int alert_en, tmp, ret; > + > + ret = iio_device_claim_direct_mode(indio_dev); > + if (ret) > + return ret; > + > + ret = regmap_read(st->regmap, AD7380_REG_ADDR_CONFIG1, &tmp); > + > + iio_device_release_direct_mode(indio_dev); > + > + if (ret) > + return ret; > + > + alert_en = FIELD_GET(AD7380_CONFIG1_ALERTEN, tmp); nit: return directly and drop alter_en. > + > + return alert_en; > +} > + > +static int ad7380_write_event_config(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, > + enum iio_event_type type, > + enum iio_event_direction dir, > + bool state) > +{ > + struct ad7380_state *st = iio_priv(indio_dev); > + int ret; > + > + ret = iio_device_claim_direct_mode(indio_dev); > + if (ret) > + return ret; > + > + ret = regmap_update_bits(st->regmap, > + AD7380_REG_ADDR_CONFIG1, > + AD7380_CONFIG1_ALERTEN, > + FIELD_PREP(AD7380_CONFIG1_ALERTEN, state)); > + > + iio_device_release_direct_mode(indio_dev); > + > + if (ret) > + return ret; > + > + return 0; return ret; > +} > + > +static int ad7380_read_event_value(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, > + enum iio_event_type type, > + enum iio_event_direction dir, > + enum iio_event_info info, > + int *val, int *val2) > +{ > + struct ad7380_state *st = iio_priv(indio_dev); > + int ret, tmp; > + > + switch (info) { > + case IIO_EV_INFO_VALUE: > + ret = iio_device_claim_direct_mode(indio_dev); > + > + switch (dir) { > + case IIO_EV_DIR_RISING: > + ret = regmap_read(st->regmap, > + AD7380_REG_ADDR_ALERT_HIGH_TH, > + &tmp); > + if (ret) > + return ret; Can't return directly here without releasing direct mode. Suggest to move everything between claim and release to a helper function to simplify things (avoiding goto and break). > + > + *val = FIELD_GET(AD7380_ALERT_HIGH_TH, tmp); > + ret = IIO_VAL_INT; > + break; > + case IIO_EV_DIR_FALLING: > + ret = regmap_read(st->regmap, > + AD7380_REG_ADDR_ALERT_LOW_TH, > + &tmp); > + if (ret) > + return ret; > + > + FIELD_GET(AD7380_ALERT_LOW_TH, tmp); > + ret = IIO_VAL_INT; > + break; > + default: > + ret = -EINVAL; > + break; > + } > + > + iio_device_release_direct_mode(indio_dev); > + return ret; > + default: > + return -EINVAL; > + } > +} > + > +static int ad7380_write_event_value(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, > + enum iio_event_type type, > + enum iio_event_direction dir, > + enum iio_event_info info, > + int val, int val2) > +{ > + struct ad7380_state *st = iio_priv(indio_dev); > + const struct iio_scan_type *scan_type; > + int ret; > + u16 th; > + > + switch (info) { > + case IIO_EV_INFO_VALUE: > + ret = iio_device_claim_direct_mode(indio_dev); > + if (ret) > + return ret; > + > + /* > + * According to the datasheet, > + * AD7380_REG_ADDR_ALERT_HIGH_TH[11:0] are the 12 MSB of the > + * 16-bits internal alert high register. LSB are set to 0xf. > + * AD7380_REG_ADDR_ALERT_LOW_TH[11:0] are the 12 MSB of the > + * 16 bits internal alert low register. LSB are set to 0x0. > + * > + * When alert is enabled the conversion from the adc is compared > + * immediately to the alert high/low thresholds, before any > + * oversampling. This means that the thresholds are the same for > + * normal mode and oversampling mode. > + */ > + > + /* Extract the 12 MSB of val */ > + scan_type = iio_get_current_scan_type(indio_dev, chan); > + if (IS_ERR(scan_type)) > + return PTR_ERR(scan_type); Same with this function, it isn't releasing on error, so we can use a helper here too to solve it. > + > + th = val >> (scan_type->realbits - 12); > + > + switch (dir) { > + case IIO_EV_DIR_RISING: > + ret = regmap_write(st->regmap, > + AD7380_REG_ADDR_ALERT_HIGH_TH, > + th); > + if (ret) > + return ret; > + > + break; > + case IIO_EV_DIR_FALLING: > + ret = regmap_write(st->regmap, > + AD7380_REG_ADDR_ALERT_LOW_TH, > + th); > + if (ret) > + return ret; > + > + break; > + default: > + ret = -EINVAL; > + break; > + } > + > + iio_device_release_direct_mode(indio_dev); > + return ret; > + default: > + return -EINVAL; > + } > +} > +
On Tue, 07 Jan 2025 09:48:28 +0100 Julien Stephan <jstephan@baylibre.com> wrote: > The alert functionality is an out of range indicator and can be used as > an early indicator of an out of bounds conversion result. > > ALERT_LOW_THRESHOLD and ALERT_HIGH_THRESHOLD registers are common to all > channels. > > When using 1 SDO line (only mode supported by the driver right now), i.e > data outputs only on SDOA, SDOB (or SDOD for 4 channels variants) is > used as an alert pin. The alert pin is updated at the end of the > conversion (set to low if an alert occurs) and is cleared on a falling > edge of CS. > > The ALERT register contains information about the exact alert status: > channel and direction. ALERT register can be accessed using debugfs if > enabled. > > User can set high/low thresholds and enable alert detection using the > regular iio events attributes: > > events/in_thresh_falling_value events/in_thresh_rising_value > events/thresh_either_en > > In most use cases, user will hardwire the alert pin to trigger a shutdown. > > In theory, we could generate userspace IIO events for alerts, but this > is not implemented yet for several reasons [1]. This can be implemented > later if a real use case actually requires it. > > Signed-off-by: Julien Stephan <jstephan@baylibre.com> > > [1] https://lore.kernel.org/all/4be16272-5197-4fa1-918c-c4cdfcaee02e@baylibre.com/ Hmm. This does end up odd in that userspace is configuring event related stuff and not getting an event. If we can fix that later I'd like to do so, but we can move forwards with this odd bit of ABI in the meantime. Even then we'd need to support the case where it's not wired to the host but is really controlling an external cut off. I've got nothing else to add to David's review. Jonathan
diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c index a532de4422082df8503454d66fc49f75b52cff68..1fc694c1557cead906ce199c7777bddf9d29d400 100644 --- a/drivers/iio/adc/ad7380.c +++ b/drivers/iio/adc/ad7380.c @@ -34,6 +34,7 @@ #include <linux/util_macros.h> #include <linux/iio/buffer.h> +#include <linux/iio/events.h> #include <linux/iio/iio.h> #include <linux/iio/trigger_consumer.h> #include <linux/iio/triggered_buffer.h> @@ -112,6 +113,24 @@ struct ad7380_chip_info { const struct ad7380_timing_specs *timing_specs; }; +static const struct iio_event_spec ad7380_events[] = { + { + .type = IIO_EV_TYPE_THRESH, + .dir = IIO_EV_DIR_RISING, + .mask_shared_by_dir = BIT(IIO_EV_INFO_VALUE), + }, + { + .type = IIO_EV_TYPE_THRESH, + .dir = IIO_EV_DIR_FALLING, + .mask_shared_by_dir = BIT(IIO_EV_INFO_VALUE), + }, + { + .type = IIO_EV_TYPE_THRESH, + .dir = IIO_EV_DIR_EITHER, + .mask_shared_by_all = BIT(IIO_EV_INFO_ENABLE), + }, +}; + enum { AD7380_SCAN_TYPE_NORMAL, AD7380_SCAN_TYPE_RESOLUTION_BOOST, @@ -214,6 +233,8 @@ static const struct iio_scan_type ad7380_scan_type_16_u[] = { .has_ext_scan_type = 1, \ .ext_scan_type = ad7380_scan_type_##bits##_##sign, \ .num_ext_scan_type = ARRAY_SIZE(ad7380_scan_type_##bits##_##sign), \ + .event_spec = ad7380_events, \ + .num_event_specs = ARRAY_SIZE(ad7380_events), \ } #define AD7380_CHANNEL(index, bits, diff, sign) \ @@ -1157,12 +1178,180 @@ static int ad7380_get_current_scan_type(const struct iio_dev *indio_dev, : AD7380_SCAN_TYPE_NORMAL; } +static int ad7380_read_event_config(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, + enum iio_event_type type, + enum iio_event_direction dir) +{ + struct ad7380_state *st = iio_priv(indio_dev); + int alert_en, tmp, ret; + + ret = iio_device_claim_direct_mode(indio_dev); + if (ret) + return ret; + + ret = regmap_read(st->regmap, AD7380_REG_ADDR_CONFIG1, &tmp); + + iio_device_release_direct_mode(indio_dev); + + if (ret) + return ret; + + alert_en = FIELD_GET(AD7380_CONFIG1_ALERTEN, tmp); + + return alert_en; +} + +static int ad7380_write_event_config(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, + enum iio_event_type type, + enum iio_event_direction dir, + bool state) +{ + struct ad7380_state *st = iio_priv(indio_dev); + int ret; + + ret = iio_device_claim_direct_mode(indio_dev); + if (ret) + return ret; + + ret = regmap_update_bits(st->regmap, + AD7380_REG_ADDR_CONFIG1, + AD7380_CONFIG1_ALERTEN, + FIELD_PREP(AD7380_CONFIG1_ALERTEN, state)); + + iio_device_release_direct_mode(indio_dev); + + if (ret) + return ret; + + return 0; +} + +static int ad7380_read_event_value(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, + enum iio_event_type type, + enum iio_event_direction dir, + enum iio_event_info info, + int *val, int *val2) +{ + struct ad7380_state *st = iio_priv(indio_dev); + int ret, tmp; + + switch (info) { + case IIO_EV_INFO_VALUE: + ret = iio_device_claim_direct_mode(indio_dev); + + switch (dir) { + case IIO_EV_DIR_RISING: + ret = regmap_read(st->regmap, + AD7380_REG_ADDR_ALERT_HIGH_TH, + &tmp); + if (ret) + return ret; + + *val = FIELD_GET(AD7380_ALERT_HIGH_TH, tmp); + ret = IIO_VAL_INT; + break; + case IIO_EV_DIR_FALLING: + ret = regmap_read(st->regmap, + AD7380_REG_ADDR_ALERT_LOW_TH, + &tmp); + if (ret) + return ret; + + FIELD_GET(AD7380_ALERT_LOW_TH, tmp); + ret = IIO_VAL_INT; + break; + default: + ret = -EINVAL; + break; + } + + iio_device_release_direct_mode(indio_dev); + return ret; + default: + return -EINVAL; + } +} + +static int ad7380_write_event_value(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, + enum iio_event_type type, + enum iio_event_direction dir, + enum iio_event_info info, + int val, int val2) +{ + struct ad7380_state *st = iio_priv(indio_dev); + const struct iio_scan_type *scan_type; + int ret; + u16 th; + + switch (info) { + case IIO_EV_INFO_VALUE: + ret = iio_device_claim_direct_mode(indio_dev); + if (ret) + return ret; + + /* + * According to the datasheet, + * AD7380_REG_ADDR_ALERT_HIGH_TH[11:0] are the 12 MSB of the + * 16-bits internal alert high register. LSB are set to 0xf. + * AD7380_REG_ADDR_ALERT_LOW_TH[11:0] are the 12 MSB of the + * 16 bits internal alert low register. LSB are set to 0x0. + * + * When alert is enabled the conversion from the adc is compared + * immediately to the alert high/low thresholds, before any + * oversampling. This means that the thresholds are the same for + * normal mode and oversampling mode. + */ + + /* Extract the 12 MSB of val */ + scan_type = iio_get_current_scan_type(indio_dev, chan); + if (IS_ERR(scan_type)) + return PTR_ERR(scan_type); + + th = val >> (scan_type->realbits - 12); + + switch (dir) { + case IIO_EV_DIR_RISING: + ret = regmap_write(st->regmap, + AD7380_REG_ADDR_ALERT_HIGH_TH, + th); + if (ret) + return ret; + + break; + case IIO_EV_DIR_FALLING: + ret = regmap_write(st->regmap, + AD7380_REG_ADDR_ALERT_LOW_TH, + th); + if (ret) + return ret; + + break; + default: + ret = -EINVAL; + break; + } + + iio_device_release_direct_mode(indio_dev); + return ret; + default: + return -EINVAL; + } +} + static const struct iio_info ad7380_info = { .read_raw = &ad7380_read_raw, .read_avail = &ad7380_read_avail, .write_raw = &ad7380_write_raw, .get_current_scan_type = &ad7380_get_current_scan_type, .debugfs_reg_access = &ad7380_debugfs_reg_access, + .read_event_config = &ad7380_read_event_config, + .write_event_config = &ad7380_write_event_config, + .read_event_value = &ad7380_read_event_value, + .write_event_value = &ad7380_write_event_value, }; static int ad7380_init(struct ad7380_state *st, bool external_ref_en)
The alert functionality is an out of range indicator and can be used as an early indicator of an out of bounds conversion result. ALERT_LOW_THRESHOLD and ALERT_HIGH_THRESHOLD registers are common to all channels. When using 1 SDO line (only mode supported by the driver right now), i.e data outputs only on SDOA, SDOB (or SDOD for 4 channels variants) is used as an alert pin. The alert pin is updated at the end of the conversion (set to low if an alert occurs) and is cleared on a falling edge of CS. The ALERT register contains information about the exact alert status: channel and direction. ALERT register can be accessed using debugfs if enabled. User can set high/low thresholds and enable alert detection using the regular iio events attributes: events/in_thresh_falling_value events/in_thresh_rising_value events/thresh_either_en In most use cases, user will hardwire the alert pin to trigger a shutdown. In theory, we could generate userspace IIO events for alerts, but this is not implemented yet for several reasons [1]. This can be implemented later if a real use case actually requires it. Signed-off-by: Julien Stephan <jstephan@baylibre.com> [1] https://lore.kernel.org/all/4be16272-5197-4fa1-918c-c4cdfcaee02e@baylibre.com/ --- drivers/iio/adc/ad7380.c | 189 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 189 insertions(+)