Add an in-kernel API for reading/writing event properties. Like the
raw-to-processed conversion, with processed-to-raw we only convert the
integer part, introducing some round-off error.
A common case is for other drivers to re-expose IIO events as sysfs
properties with a different API. To help out with this, iio_event_mode
returns the appropriate mode. It can also be used to test for existence
if the consumer doesn't care about read/write capability.
Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---
drivers/iio/inkern.c | 198 +++++++++++++++++++++++++++++++++++
include/linux/iio/consumer.h | 56 ++++++++++
2 files changed, 254 insertions(+)
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index c174ebb7d5e6..d3bbd2444fb5 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -1028,3 +1028,201 @@ ssize_t iio_read_channel_label(struct iio_channel *chan, char *buf)
return do_iio_read_channel_label(chan->indio_dev, chan->channel, buf);
}
EXPORT_SYMBOL_GPL(iio_read_channel_label);
+
+static bool iio_event_exists(struct iio_channel *channel,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info)
+{
+ struct iio_chan_spec const *chan = channel->channel;
+ int i;
+
+ if (!channel->indio_dev->info)
+ return false;
+
+ for (i = 0; i < chan->num_event_specs; i++) {
+ if (chan->event_spec[i].type != type)
+ continue;
+ if (chan->event_spec[i].dir != dir)
+ continue;
+ if (chan->event_spec[i].mask_separate & BIT(info))
+ return true;
+ }
+
+ return false;
+}
+
+umode_t iio_event_mode(struct iio_channel *chan, enum iio_event_type type,
+ enum iio_event_direction dir, enum iio_event_info info)
+{
+ struct iio_dev *indio_dev = chan->indio_dev;
+ struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
+ umode_t mode = 0;
+
+ guard(mutex)(&iio_dev_opaque->info_exist_lock);
+ if (!iio_event_exists(chan, type, dir, info))
+ return 0;
+
+ if (info == IIO_EV_INFO_ENABLE) {
+ if (indio_dev->info->read_event_config)
+ mode |= 0444;
+
+ if (indio_dev->info->write_event_config)
+ mode |= 0200;
+ } else {
+ if (indio_dev->info->read_event_value)
+ mode |= 0444;
+
+ if (indio_dev->info->write_event_value)
+ mode |= 0200;
+ }
+
+ return mode;
+}
+EXPORT_SYMBOL_GPL(iio_event_mode);
+
+int iio_read_event_processed_scale(struct iio_channel *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info, int *val,
+ unsigned int scale)
+{
+ struct iio_dev *indio_dev = chan->indio_dev;
+ struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
+ int ret, raw;
+
+ guard(mutex)(&iio_dev_opaque->info_exist_lock);
+ if (!iio_event_exists(chan, type, dir, info))
+ return -ENODEV;
+
+ if (info == IIO_EV_INFO_ENABLE) {
+ if (!indio_dev->info->read_event_config)
+ return -EINVAL;
+
+ raw = indio_dev->info->read_event_config(indio_dev,
+ chan->channel, type,
+ dir);
+ if (raw < 0)
+ return raw;
+
+ *val = raw;
+ return 0;
+ }
+
+ if (!indio_dev->info->read_event_value)
+ return -EINVAL;
+
+ ret = indio_dev->info->read_event_value(indio_dev, chan->channel, type,
+ dir, info, &raw, NULL);
+ if (ret < 0)
+ return ret;
+
+ return iio_convert_raw_to_processed_unlocked(chan, raw, val, scale);
+}
+EXPORT_SYMBOL_GPL(iio_read_event_processed_scale);
+
+static int iio_convert_processed_to_raw_unlocked(struct iio_channel *chan,
+ int processed, int *raw,
+ unsigned int scale)
+{
+ int scale_type, scale_val, scale_val2;
+ int offset_type, offset_val, offset_val2;
+ s64 r, scale64, raw64;
+
+ scale_type = iio_channel_read(chan, &scale_val, &scale_val2,
+ IIO_CHAN_INFO_SCALE);
+ if (scale_type < 0) {
+ raw64 = processed / scale;
+ } else {
+ switch (scale_type) {
+ case IIO_VAL_INT:
+ scale64 = (s64)scale_val * scale;
+ if (scale64 <= INT_MAX && scale64 >= INT_MIN)
+ raw64 = processed / (int)scale64;
+ else
+ raw64 = 0;
+ break;
+ case IIO_VAL_INT_PLUS_MICRO:
+ scale64 = scale_val * scale * 1000000LL + scale_val2;
+ raw64 = div64_s64_rem(processed, scale64, &r);
+ raw64 = raw64 * 1000000 +
+ div64_s64(r * 1000000, scale64);
+ break;
+ case IIO_VAL_INT_PLUS_NANO:
+ scale64 = scale_val * scale * 1000000000LL + scale_val2;
+ raw64 = div64_s64_rem(processed, scale64, &r);
+ raw64 = raw64 * 1000000000 +
+ div64_s64(r * 1000000000, scale64);
+ break;
+ case IIO_VAL_FRACTIONAL:
+ raw64 = div64_s64((s64)processed * scale_val2,
+ (s64)scale_val * scale);
+ break;
+ case IIO_VAL_FRACTIONAL_LOG2:
+ raw64 = div64_s64((s64)processed << scale_val2,
+ (s64)scale_val * scale);
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+
+ offset_type = iio_channel_read(chan, &offset_val, &offset_val2,
+ IIO_CHAN_INFO_OFFSET);
+ if (offset_type >= 0) {
+ switch (offset_type) {
+ case IIO_VAL_INT:
+ case IIO_VAL_INT_PLUS_MICRO:
+ case IIO_VAL_INT_PLUS_NANO:
+ raw64 -= offset_val;
+ break;
+ case IIO_VAL_FRACTIONAL:
+ raw64 -= offset_val / offset_val2;
+ break;
+ case IIO_VAL_FRACTIONAL_LOG2:
+ raw64 -= offset_val >> offset_val2;
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+
+ *raw = clamp(raw64, (s64)INT_MIN, (s64)INT_MAX);
+ return 0;
+}
+
+int iio_write_event_processed_scale(struct iio_channel *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info, int processed,
+ unsigned int scale)
+{
+ struct iio_dev *indio_dev = chan->indio_dev;
+ struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(chan->indio_dev);
+ int ret, raw;
+
+ guard(mutex)(&iio_dev_opaque->info_exist_lock);
+ if (!iio_event_exists(chan, type, dir, info))
+ return -ENODEV;
+
+ if (info == IIO_EV_INFO_ENABLE) {
+ if (!indio_dev->info->write_event_config)
+ return -EINVAL;
+
+ return indio_dev->info->write_event_config(indio_dev,
+ chan->channel, type,
+ dir, processed);
+ }
+
+ if (!indio_dev->info->write_event_value)
+ return -EINVAL;
+
+ ret = iio_convert_processed_to_raw_unlocked(chan, processed, &raw,
+ scale);
+ if (ret < 0)
+ return ret;
+
+ return indio_dev->info->write_event_value(indio_dev, chan->channel,
+ type, dir, info, raw, 0);
+}
+EXPORT_SYMBOL_GPL(iio_write_event_processed_scale);
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index 6a4479616479..16e7682474f3 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -451,4 +451,60 @@ ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr,
*/
ssize_t iio_read_channel_label(struct iio_channel *chan, char *buf);
+/**
+ * iio_event_mode() - get file mode for an event property
+ * @chan: Channel being queried
+ * @type: Event type (theshold, rate-of-change, etc.)
+ * @dir: Event direction (rising, falling, etc.)
+ * @info: Event property (enable, value, etc.)
+ *
+ * Determine an appropriate mode for sysfs files derived from this event.
+ *
+ * Return:
+ * - `0000` if the event is unsupported or otherwise unavailable
+ * - `0444` if the event is read-only
+ * - `0200` if the event is write-only
+ * - `0644` if the event is read-write
+ */
+umode_t iio_event_mode(struct iio_channel *chan, enum iio_event_type type,
+ enum iio_event_direction dir, enum iio_event_info info);
+
+/**
+ * iio_read_event_processed_scale() - Read an event property
+ * @chan: Channel being queried
+ * @type: Event type (theshold, rate-of-change, etc.)
+ * @dir: Event direction (rising, falling, etc.)
+ * @info: Event property (enable, value, etc.)
+ * @val: Processed property value
+ * @scale: Factor to scale @val by
+ *
+ * Read a processed (scaled and offset) event property of a given channel.
+ *
+ * Return: 0 on success, or negative error on failure
+ */
+int iio_read_event_processed_scale(struct iio_channel *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info, int *val,
+ unsigned int scale);
+
+/**
+ * iio_write_event_processed_scale() - Read an event property
+ * @chan: Channel being queried
+ * @type: Event type (theshold, rate-of-change, etc.)
+ * @dir: Event direction (rising, falling, etc.)
+ * @info: Event property (enable, value, etc.)
+ * @processed: Processed property value
+ * @scale: Factor to scale @processed by
+ *
+ * Write a processed (scaled and offset) event property of a given channel.
+ *
+ * Return: 0 on success, or negative error on failure
+ */
+int iio_write_event_processed_scale(struct iio_channel *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info, int processed,
+ unsigned int scale);
+
#endif
--
2.35.1.1320.gc452695387.dirty
On Mon, 14 Jul 2025 21:20:18 -0400 Sean Anderson <sean.anderson@linux.dev> wrote: > Add an in-kernel API for reading/writing event properties. Like the > raw-to-processed conversion, with processed-to-raw we only convert the > integer part, introducing some round-off error. > > A common case is for other drivers to re-expose IIO events as sysfs > properties with a different API. To help out with this, iio_event_mode > returns the appropriate mode. It can also be used to test for existence > if the consumer doesn't care about read/write capability. > > Signed-off-by: Sean Anderson <sean.anderson@linux.dev> Hi Sean, A few minor comments inline. > --- > > drivers/iio/inkern.c | 198 +++++++++++++++++++++++++++++++++++ > include/linux/iio/consumer.h | 56 ++++++++++ > 2 files changed, 254 insertions(+) > > diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c > index c174ebb7d5e6..d3bbd2444fb5 100644 > --- a/drivers/iio/inkern.c > +++ b/drivers/iio/inkern.c > @@ -1028,3 +1028,201 @@ ssize_t iio_read_channel_label(struct iio_channel *chan, char *buf) > return do_iio_read_channel_label(chan->indio_dev, chan->channel, buf); > } > EXPORT_SYMBOL_GPL(iio_read_channel_label); > +umode_t iio_event_mode(struct iio_channel *chan, enum iio_event_type type, > + enum iio_event_direction dir, enum iio_event_info info) > +{ > + struct iio_dev *indio_dev = chan->indio_dev; > + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); > + umode_t mode = 0; > + > + guard(mutex)(&iio_dev_opaque->info_exist_lock); > + if (!iio_event_exists(chan, type, dir, info)) > + return 0; > + > + if (info == IIO_EV_INFO_ENABLE) { > + if (indio_dev->info->read_event_config) > + mode |= 0444; > + > + if (indio_dev->info->write_event_config) > + mode |= 0200; > + } else { > + if (indio_dev->info->read_event_value) > + mode |= 0444; > + > + if (indio_dev->info->write_event_value) > + mode |= 0200; > + } > + > + return mode; > +} > +EXPORT_SYMBOL_GPL(iio_event_mode); > + > +int iio_read_event_processed_scale(struct iio_channel *chan, > + enum iio_event_type type, > + enum iio_event_direction dir, > + enum iio_event_info info, int *val, Maybe rename info to ev_info or similar to avoid confusion with indio_dev->info > + unsigned int scale) > +{ > + struct iio_dev *indio_dev = chan->indio_dev; > + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); > + int ret, raw; > + > + guard(mutex)(&iio_dev_opaque->info_exist_lock); > + if (!iio_event_exists(chan, type, dir, info)) > + return -ENODEV; > + Perhaps a local variable struct iio_info info; info = indio_dev->info; > + if (info == IIO_EV_INFO_ENABLE) { > + if (!indio_dev->info->read_event_config) > + return -EINVAL; > + > + raw = indio_dev->info->read_event_config(indio_dev, raw = info->read_event_config(indio_dev, chan->channel, type, dir); > + chan->channel, type, > + dir); > + if (raw < 0) > + return raw; > + > + *val = raw; > + return 0; > + } > + > + if (!indio_dev->info->read_event_value) > + return -EINVAL; > + > + ret = indio_dev->info->read_event_value(indio_dev, chan->channel, type, > + dir, info, &raw, NULL); > + if (ret < 0) > + return ret; > + > + return iio_convert_raw_to_processed_unlocked(chan, raw, val, scale); > +} > +EXPORT_SYMBOL_GPL(iio_read_event_processed_scale); > + > +int iio_write_event_processed_scale(struct iio_channel *chan, > + enum iio_event_type type, > + enum iio_event_direction dir, > + enum iio_event_info info, int processed, > + unsigned int scale) > +{ > + struct iio_dev *indio_dev = chan->indio_dev; > + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(chan->indio_dev); > + int ret, raw; > + > + guard(mutex)(&iio_dev_opaque->info_exist_lock); > + if (!iio_event_exists(chan, type, dir, info)) > + return -ENODEV; > + > + if (info == IIO_EV_INFO_ENABLE) { > + if (!indio_dev->info->write_event_config) > + return -EINVAL; > + > + return indio_dev->info->write_event_config(indio_dev, Similar to above, feels like a local variable to shorten these would be good, > + chan->channel, type, > + dir, processed); > + } > + > + if (!indio_dev->info->write_event_value) > + return -EINVAL; > + > + ret = iio_convert_processed_to_raw_unlocked(chan, processed, &raw, > + scale); > + if (ret < 0) > + return ret; > + > + return indio_dev->info->write_event_value(indio_dev, chan->channel, > + type, dir, info, raw, 0); > +} > +EXPORT_SYMBOL_GPL(iio_write_event_processed_scale); > diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h > index 6a4479616479..16e7682474f3 100644 > --- a/include/linux/iio/consumer.h > +++ b/include/linux/iio/consumer.h > @@ -451,4 +451,60 @@ ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr, > */ > ssize_t iio_read_channel_label(struct iio_channel *chan, char *buf); > > +/** > + * iio_event_mode() - get file mode for an event property Can we name this something more specific. Sounds like it might be the mode of the events, not the mode of the file. > + * @chan: Channel being queried > + * @type: Event type (theshold, rate-of-change, etc.) > + * @dir: Event direction (rising, falling, etc.) > + * @info: Event property (enable, value, etc.) > + * > + * Determine an appropriate mode for sysfs files derived from this event. This isn't precise unfortunately as if we have a mix of read only and rw for different events (maybe some thresholds fixed and others not). We should maybe say that it may indicate more control than actually possible. In most cases it'll be right though. > + * > + * Return: > + * - `0000` if the event is unsupported or otherwise unavailable > + * - `0444` if the event is read-only > + * - `0200` if the event is write-only > + * - `0644` if the event is read-write > + */ So here is one of those bits of "don't do it the way we currently do it". Move the docs next to the implementation in the c file rather than the header. We are horribly inconsistent in IIO mostly because of younger me making a mess of it. General thinking today is that we are much less likely to forget to update docs if they are next to the code. > +umode_t iio_event_mode(struct iio_channel *chan, enum iio_event_type type, > + enum iio_event_direction dir, enum iio_event_info info); > + > +/** > + * iio_read_event_processed_scale() - Read an event property > + * @chan: Channel being queried > + * @type: Event type (theshold, rate-of-change, etc.) > + * @dir: Event direction (rising, falling, etc.) > + * @info: Event property (enable, value, etc.) > + * @val: Processed property value > + * @scale: Factor to scale @val by > + * > + * Read a processed (scaled and offset) event property of a given channel. > + * > + * Return: 0 on success, or negative error on failure > + */ > +int iio_read_event_processed_scale(struct iio_channel *chan, > + enum iio_event_type type, > + enum iio_event_direction dir, > + enum iio_event_info info, int *val, > + unsigned int scale); > + > +/** > + * iio_write_event_processed_scale() - Read an event property > + * @chan: Channel being queried > + * @type: Event type (theshold, rate-of-change, etc.) > + * @dir: Event direction (rising, falling, etc.) > + * @info: Event property (enable, value, etc.) > + * @processed: Processed property value > + * @scale: Factor to scale @processed by > + * > + * Write a processed (scaled and offset) event property of a given channel. > + * > + * Return: 0 on success, or negative error on failure > + */ > +int iio_write_event_processed_scale(struct iio_channel *chan, > + enum iio_event_type type, > + enum iio_event_direction dir, > + enum iio_event_info info, int processed, > + unsigned int scale); > + > #endif
On Mon, 2025-07-14 at 21:20 -0400, Sean Anderson wrote: > Add an in-kernel API for reading/writing event properties. Like the > raw-to-processed conversion, with processed-to-raw we only convert the > integer part, introducing some round-off error. > > A common case is for other drivers to re-expose IIO events as sysfs > properties with a different API. To help out with this, iio_event_mode > returns the appropriate mode. It can also be used to test for existence > if the consumer doesn't care about read/write capability. > > Signed-off-by: Sean Anderson <sean.anderson@linux.dev> > --- Just one comment on top of Andy's review > > drivers/iio/inkern.c | 198 +++++++++++++++++++++++++++++++++++ > include/linux/iio/consumer.h | 56 ++++++++++ > 2 files changed, 254 insertions(+) > > diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c > index c174ebb7d5e6..d3bbd2444fb5 100644 > --- a/drivers/iio/inkern.c > +++ b/drivers/iio/inkern.c > @@ -1028,3 +1028,201 @@ ssize_t iio_read_channel_label(struct iio_channel > *chan, char *buf) > return do_iio_read_channel_label(chan->indio_dev, chan->channel, > buf); > } > EXPORT_SYMBOL_GPL(iio_read_channel_label); > + > +static bool iio_event_exists(struct iio_channel *channel, > + enum iio_event_type type, > + enum iio_event_direction dir, > + enum iio_event_info info) > +{ > + struct iio_chan_spec const *chan = channel->channel; > + int i; > + Can we iio_event_exists() -> iio_event_exists_locked()? Or likely the best way would be to annotate it with lockdep (though that would mean some dance to get the opaque object. Anyways, bottom line is it should clear that iio_event_exists() is to be called with the lock held. - Nuno Sá > + if (!channel->indio_dev->info) > + return false; > + > + for (i = 0; i < chan->num_event_specs; i++) { > + if (chan->event_spec[i].type != type) > + continue; > + if (chan->event_spec[i].dir != dir) > + continue; > + if (chan->event_spec[i].mask_separate & BIT(info)) > + return true; > + } > + > + return false; > +} > + > +umode_t iio_event_mode(struct iio_channel *chan, enum iio_event_type type, > + enum iio_event_direction dir, enum iio_event_info > info) > +{ > + struct iio_dev *indio_dev = chan->indio_dev; > + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); > + umode_t mode = 0; > + > + guard(mutex)(&iio_dev_opaque->info_exist_lock); > + if (!iio_event_exists(chan, type, dir, info)) > + return 0; > + > + if (info == IIO_EV_INFO_ENABLE) { > + if (indio_dev->info->read_event_config) > + mode |= 0444; > + > + if (indio_dev->info->write_event_config) > + mode |= 0200; > + } else { > + if (indio_dev->info->read_event_value) > + mode |= 0444; > + > + if (indio_dev->info->write_event_value) > + mode |= 0200; > + } > + > + return mode; > +} > +EXPORT_SYMBOL_GPL(iio_event_mode); > + > +int iio_read_event_processed_scale(struct iio_channel *chan, > + enum iio_event_type type, > + enum iio_event_direction dir, > + enum iio_event_info info, int *val, > + unsigned int scale) > +{ > + struct iio_dev *indio_dev = chan->indio_dev; > + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); > + int ret, raw; > + > + guard(mutex)(&iio_dev_opaque->info_exist_lock); > + if (!iio_event_exists(chan, type, dir, info)) > + return -ENODEV; > + > + if (info == IIO_EV_INFO_ENABLE) { > + if (!indio_dev->info->read_event_config) > + return -EINVAL; > + > + raw = indio_dev->info->read_event_config(indio_dev, > + chan->channel, type, > + dir); > + if (raw < 0) > + return raw; > + > + *val = raw; > + return 0; > + } > + > + if (!indio_dev->info->read_event_value) > + return -EINVAL; > + > + ret = indio_dev->info->read_event_value(indio_dev, chan->channel, > type, > + dir, info, &raw, NULL); > + if (ret < 0) > + return ret; > + > + return iio_convert_raw_to_processed_unlocked(chan, raw, val, scale); > +} > +EXPORT_SYMBOL_GPL(iio_read_event_processed_scale); > + > +static int iio_convert_processed_to_raw_unlocked(struct iio_channel *chan, > + int processed, int *raw, > + unsigned int scale) > +{ > + int scale_type, scale_val, scale_val2; > + int offset_type, offset_val, offset_val2; > + s64 r, scale64, raw64; > + > + scale_type = iio_channel_read(chan, &scale_val, &scale_val2, > + IIO_CHAN_INFO_SCALE); > + if (scale_type < 0) { > + raw64 = processed / scale; > + } else { > + switch (scale_type) { > + case IIO_VAL_INT: > + scale64 = (s64)scale_val * scale; > + if (scale64 <= INT_MAX && scale64 >= INT_MIN) > + raw64 = processed / (int)scale64; > + else > + raw64 = 0; > + break; > + case IIO_VAL_INT_PLUS_MICRO: > + scale64 = scale_val * scale * 1000000LL + scale_val2; > + raw64 = div64_s64_rem(processed, scale64, &r); > + raw64 = raw64 * 1000000 + > + div64_s64(r * 1000000, scale64); > + break; > + case IIO_VAL_INT_PLUS_NANO: > + scale64 = scale_val * scale * 1000000000LL + > scale_val2; > + raw64 = div64_s64_rem(processed, scale64, &r); > + raw64 = raw64 * 1000000000 + > + div64_s64(r * 1000000000, scale64); > + break; > + case IIO_VAL_FRACTIONAL: > + raw64 = div64_s64((s64)processed * scale_val2, > + (s64)scale_val * scale); > + break; > + case IIO_VAL_FRACTIONAL_LOG2: > + raw64 = div64_s64((s64)processed << scale_val2, > + (s64)scale_val * scale); > + break; > + default: > + return -EINVAL; > + } > + } > + > + offset_type = iio_channel_read(chan, &offset_val, &offset_val2, > + IIO_CHAN_INFO_OFFSET); > + if (offset_type >= 0) { > + switch (offset_type) { > + case IIO_VAL_INT: > + case IIO_VAL_INT_PLUS_MICRO: > + case IIO_VAL_INT_PLUS_NANO: > + raw64 -= offset_val; > + break; > + case IIO_VAL_FRACTIONAL: > + raw64 -= offset_val / offset_val2; > + break; > + case IIO_VAL_FRACTIONAL_LOG2: > + raw64 -= offset_val >> offset_val2; > + break; > + default: > + return -EINVAL; > + } > + } > + > + *raw = clamp(raw64, (s64)INT_MIN, (s64)INT_MAX); > + return 0; > +} > + > +int iio_write_event_processed_scale(struct iio_channel *chan, > + enum iio_event_type type, > + enum iio_event_direction dir, > + enum iio_event_info info, int processed, > + unsigned int scale) > +{ > + struct iio_dev *indio_dev = chan->indio_dev; > + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(chan- > >indio_dev); > + int ret, raw; > + > + guard(mutex)(&iio_dev_opaque->info_exist_lock); > + if (!iio_event_exists(chan, type, dir, info)) > + return -ENODEV; > + > + if (info == IIO_EV_INFO_ENABLE) { > + if (!indio_dev->info->write_event_config) > + return -EINVAL; > + > + return indio_dev->info->write_event_config(indio_dev, > + chan->channel, > type, > + dir, processed); > + } > + > + if (!indio_dev->info->write_event_value) > + return -EINVAL; > + > + ret = iio_convert_processed_to_raw_unlocked(chan, processed, &raw, > + scale); > + if (ret < 0) > + return ret; > + > + return indio_dev->info->write_event_value(indio_dev, chan->channel, > + type, dir, info, raw, 0); > +} > +EXPORT_SYMBOL_GPL(iio_write_event_processed_scale); > diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h > index 6a4479616479..16e7682474f3 100644 > --- a/include/linux/iio/consumer.h > +++ b/include/linux/iio/consumer.h > @@ -451,4 +451,60 @@ ssize_t iio_write_channel_ext_info(struct iio_channel > *chan, const char *attr, > */ > ssize_t iio_read_channel_label(struct iio_channel *chan, char *buf); > > +/** > + * iio_event_mode() - get file mode for an event property > + * @chan: Channel being queried > + * @type: Event type (theshold, rate-of-change, etc.) > + * @dir: Event direction (rising, falling, etc.) > + * @info: Event property (enable, value, etc.) > + * > + * Determine an appropriate mode for sysfs files derived from this event. > + * > + * Return: > + * - `0000` if the event is unsupported or otherwise unavailable > + * - `0444` if the event is read-only > + * - `0200` if the event is write-only > + * - `0644` if the event is read-write > + */ > +umode_t iio_event_mode(struct iio_channel *chan, enum iio_event_type type, > + enum iio_event_direction dir, enum iio_event_info > info); > + > +/** > + * iio_read_event_processed_scale() - Read an event property > + * @chan: Channel being queried > + * @type: Event type (theshold, rate-of-change, etc.) > + * @dir: Event direction (rising, falling, etc.) > + * @info: Event property (enable, value, etc.) > + * @val: Processed property value > + * @scale: Factor to scale @val by > + * > + * Read a processed (scaled and offset) event property of a given channel. > + * > + * Return: 0 on success, or negative error on failure > + */ > +int iio_read_event_processed_scale(struct iio_channel *chan, > + enum iio_event_type type, > + enum iio_event_direction dir, > + enum iio_event_info info, int *val, > + unsigned int scale); > + > +/** > + * iio_write_event_processed_scale() - Read an event property > + * @chan: Channel being queried > + * @type: Event type (theshold, rate-of-change, etc.) > + * @dir: Event direction (rising, falling, etc.) > + * @info: Event property (enable, value, etc.) > + * @processed: Processed property value > + * @scale: Factor to scale @processed by > + * > + * Write a processed (scaled and offset) event property of a given channel. > + * > + * Return: 0 on success, or negative error on failure > + */ > +int iio_write_event_processed_scale(struct iio_channel *chan, > + enum iio_event_type type, > + enum iio_event_direction dir, > + enum iio_event_info info, int processed, > + unsigned int scale); > + > #endif
On 7/15/25 06:35, Nuno Sá wrote: > On Mon, 2025-07-14 at 21:20 -0400, Sean Anderson wrote: >> Add an in-kernel API for reading/writing event properties. Like the >> raw-to-processed conversion, with processed-to-raw we only convert the >> integer part, introducing some round-off error. >> >> A common case is for other drivers to re-expose IIO events as sysfs >> properties with a different API. To help out with this, iio_event_mode >> returns the appropriate mode. It can also be used to test for existence >> if the consumer doesn't care about read/write capability. >> >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev> >> --- > > Just one comment on top of Andy's review > >> >> drivers/iio/inkern.c | 198 +++++++++++++++++++++++++++++++++++ >> include/linux/iio/consumer.h | 56 ++++++++++ >> 2 files changed, 254 insertions(+) >> >> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c >> index c174ebb7d5e6..d3bbd2444fb5 100644 >> --- a/drivers/iio/inkern.c >> +++ b/drivers/iio/inkern.c >> @@ -1028,3 +1028,201 @@ ssize_t iio_read_channel_label(struct iio_channel >> *chan, char *buf) >> return do_iio_read_channel_label(chan->indio_dev, chan->channel, >> buf); >> } >> EXPORT_SYMBOL_GPL(iio_read_channel_label); >> + >> +static bool iio_event_exists(struct iio_channel *channel, >> + enum iio_event_type type, >> + enum iio_event_direction dir, >> + enum iio_event_info info) >> +{ >> + struct iio_chan_spec const *chan = channel->channel; >> + int i; >> + > > Can we iio_event_exists() -> iio_event_exists_locked()? Or likely the best way wouldn't _unlocked be the convention for this file? > would be to annotate it with lockdep (though that would mean some dance to get > the opaque object. I will add a lockdep annotation. --Sean > Anyways, bottom line is it should clear that iio_event_exists() is to be called > with the lock held. > > - Nuno Sá > >> + if (!channel->indio_dev->info) >> + return false; >> + >> + for (i = 0; i < chan->num_event_specs; i++) { >> + if (chan->event_spec[i].type != type) >> + continue; >> + if (chan->event_spec[i].dir != dir) >> + continue; >> + if (chan->event_spec[i].mask_separate & BIT(info)) >> + return true; >> + } >> + >> + return false; >> +} >> + >> +umode_t iio_event_mode(struct iio_channel *chan, enum iio_event_type type, >> + enum iio_event_direction dir, enum iio_event_info >> info) >> +{ >> + struct iio_dev *indio_dev = chan->indio_dev; >> + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); >> + umode_t mode = 0; >> + >> + guard(mutex)(&iio_dev_opaque->info_exist_lock); >> + if (!iio_event_exists(chan, type, dir, info)) >> + return 0; >> + >> + if (info == IIO_EV_INFO_ENABLE) { >> + if (indio_dev->info->read_event_config) >> + mode |= 0444; >> + >> + if (indio_dev->info->write_event_config) >> + mode |= 0200; >> + } else { >> + if (indio_dev->info->read_event_value) >> + mode |= 0444; >> + >> + if (indio_dev->info->write_event_value) >> + mode |= 0200; >> + } >> + >> + return mode; >> +} >> +EXPORT_SYMBOL_GPL(iio_event_mode); >> + >> +int iio_read_event_processed_scale(struct iio_channel *chan, >> + enum iio_event_type type, >> + enum iio_event_direction dir, >> + enum iio_event_info info, int *val, >> + unsigned int scale) >> +{ >> + struct iio_dev *indio_dev = chan->indio_dev; >> + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); >> + int ret, raw; >> + >> + guard(mutex)(&iio_dev_opaque->info_exist_lock); >> + if (!iio_event_exists(chan, type, dir, info)) >> + return -ENODEV; >> + >> + if (info == IIO_EV_INFO_ENABLE) { >> + if (!indio_dev->info->read_event_config) >> + return -EINVAL; >> + >> + raw = indio_dev->info->read_event_config(indio_dev, >> + chan->channel, type, >> + dir); >> + if (raw < 0) >> + return raw; >> + >> + *val = raw; >> + return 0; >> + } >> + >> + if (!indio_dev->info->read_event_value) >> + return -EINVAL; >> + >> + ret = indio_dev->info->read_event_value(indio_dev, chan->channel, >> type, >> + dir, info, &raw, NULL); >> + if (ret < 0) >> + return ret; >> + >> + return iio_convert_raw_to_processed_unlocked(chan, raw, val, scale); >> +} >> +EXPORT_SYMBOL_GPL(iio_read_event_processed_scale); >> + >> +static int iio_convert_processed_to_raw_unlocked(struct iio_channel *chan, >> + int processed, int *raw, >> + unsigned int scale) >> +{ >> + int scale_type, scale_val, scale_val2; >> + int offset_type, offset_val, offset_val2; >> + s64 r, scale64, raw64; >> + >> + scale_type = iio_channel_read(chan, &scale_val, &scale_val2, >> + IIO_CHAN_INFO_SCALE); >> + if (scale_type < 0) { >> + raw64 = processed / scale; >> + } else { >> + switch (scale_type) { >> + case IIO_VAL_INT: >> + scale64 = (s64)scale_val * scale; >> + if (scale64 <= INT_MAX && scale64 >= INT_MIN) >> + raw64 = processed / (int)scale64; >> + else >> + raw64 = 0; >> + break; >> + case IIO_VAL_INT_PLUS_MICRO: >> + scale64 = scale_val * scale * 1000000LL + scale_val2; >> + raw64 = div64_s64_rem(processed, scale64, &r); >> + raw64 = raw64 * 1000000 + >> + div64_s64(r * 1000000, scale64); >> + break; >> + case IIO_VAL_INT_PLUS_NANO: >> + scale64 = scale_val * scale * 1000000000LL + >> scale_val2; >> + raw64 = div64_s64_rem(processed, scale64, &r); >> + raw64 = raw64 * 1000000000 + >> + div64_s64(r * 1000000000, scale64); >> + break; >> + case IIO_VAL_FRACTIONAL: >> + raw64 = div64_s64((s64)processed * scale_val2, >> + (s64)scale_val * scale); >> + break; >> + case IIO_VAL_FRACTIONAL_LOG2: >> + raw64 = div64_s64((s64)processed << scale_val2, >> + (s64)scale_val * scale); >> + break; >> + default: >> + return -EINVAL; >> + } >> + } >> + >> + offset_type = iio_channel_read(chan, &offset_val, &offset_val2, >> + IIO_CHAN_INFO_OFFSET); >> + if (offset_type >= 0) { >> + switch (offset_type) { >> + case IIO_VAL_INT: >> + case IIO_VAL_INT_PLUS_MICRO: >> + case IIO_VAL_INT_PLUS_NANO: >> + raw64 -= offset_val; >> + break; >> + case IIO_VAL_FRACTIONAL: >> + raw64 -= offset_val / offset_val2; >> + break; >> + case IIO_VAL_FRACTIONAL_LOG2: >> + raw64 -= offset_val >> offset_val2; >> + break; >> + default: >> + return -EINVAL; >> + } >> + } >> + >> + *raw = clamp(raw64, (s64)INT_MIN, (s64)INT_MAX); >> + return 0; >> +} >> + >> +int iio_write_event_processed_scale(struct iio_channel *chan, >> + enum iio_event_type type, >> + enum iio_event_direction dir, >> + enum iio_event_info info, int processed, >> + unsigned int scale) >> +{ >> + struct iio_dev *indio_dev = chan->indio_dev; >> + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(chan- >> >indio_dev); >> + int ret, raw; >> + >> + guard(mutex)(&iio_dev_opaque->info_exist_lock); >> + if (!iio_event_exists(chan, type, dir, info)) >> + return -ENODEV; >> + >> + if (info == IIO_EV_INFO_ENABLE) { >> + if (!indio_dev->info->write_event_config) >> + return -EINVAL; >> + >> + return indio_dev->info->write_event_config(indio_dev, >> + chan->channel, >> type, >> + dir, processed); >> + } >> + >> + if (!indio_dev->info->write_event_value) >> + return -EINVAL; >> + >> + ret = iio_convert_processed_to_raw_unlocked(chan, processed, &raw, >> + scale); >> + if (ret < 0) >> + return ret; >> + >> + return indio_dev->info->write_event_value(indio_dev, chan->channel, >> + type, dir, info, raw, 0); >> +} >> +EXPORT_SYMBOL_GPL(iio_write_event_processed_scale); >> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h >> index 6a4479616479..16e7682474f3 100644 >> --- a/include/linux/iio/consumer.h >> +++ b/include/linux/iio/consumer.h >> @@ -451,4 +451,60 @@ ssize_t iio_write_channel_ext_info(struct iio_channel >> *chan, const char *attr, >> */ >> ssize_t iio_read_channel_label(struct iio_channel *chan, char *buf); >> >> +/** >> + * iio_event_mode() - get file mode for an event property >> + * @chan: Channel being queried >> + * @type: Event type (theshold, rate-of-change, etc.) >> + * @dir: Event direction (rising, falling, etc.) >> + * @info: Event property (enable, value, etc.) >> + * >> + * Determine an appropriate mode for sysfs files derived from this event. >> + * >> + * Return: >> + * - `0000` if the event is unsupported or otherwise unavailable >> + * - `0444` if the event is read-only >> + * - `0200` if the event is write-only >> + * - `0644` if the event is read-write >> + */ >> +umode_t iio_event_mode(struct iio_channel *chan, enum iio_event_type type, >> + enum iio_event_direction dir, enum iio_event_info >> info); >> + >> +/** >> + * iio_read_event_processed_scale() - Read an event property >> + * @chan: Channel being queried >> + * @type: Event type (theshold, rate-of-change, etc.) >> + * @dir: Event direction (rising, falling, etc.) >> + * @info: Event property (enable, value, etc.) >> + * @val: Processed property value >> + * @scale: Factor to scale @val by >> + * >> + * Read a processed (scaled and offset) event property of a given channel. >> + * >> + * Return: 0 on success, or negative error on failure >> + */ >> +int iio_read_event_processed_scale(struct iio_channel *chan, >> + enum iio_event_type type, >> + enum iio_event_direction dir, >> + enum iio_event_info info, int *val, >> + unsigned int scale); >> + >> +/** >> + * iio_write_event_processed_scale() - Read an event property >> + * @chan: Channel being queried >> + * @type: Event type (theshold, rate-of-change, etc.) >> + * @dir: Event direction (rising, falling, etc.) >> + * @info: Event property (enable, value, etc.) >> + * @processed: Processed property value >> + * @scale: Factor to scale @processed by >> + * >> + * Write a processed (scaled and offset) event property of a given channel. >> + * >> + * Return: 0 on success, or negative error on failure >> + */ >> +int iio_write_event_processed_scale(struct iio_channel *chan, >> + enum iio_event_type type, >> + enum iio_event_direction dir, >> + enum iio_event_info info, int processed, >> + unsigned int scale); >> + >> #endif
On Tue, 2025-07-15 at 11:43 -0400, Sean Anderson wrote: > On 7/15/25 06:35, Nuno Sá wrote: > > On Mon, 2025-07-14 at 21:20 -0400, Sean Anderson wrote: > > > Add an in-kernel API for reading/writing event properties. Like the > > > raw-to-processed conversion, with processed-to-raw we only convert the > > > integer part, introducing some round-off error. > > > > > > A common case is for other drivers to re-expose IIO events as sysfs > > > properties with a different API. To help out with this, iio_event_mode > > > returns the appropriate mode. It can also be used to test for existence > > > if the consumer doesn't care about read/write capability. > > > > > > Signed-off-by: Sean Anderson <sean.anderson@linux.dev> > > > --- > > > > Just one comment on top of Andy's review > > > > > > > > drivers/iio/inkern.c | 198 +++++++++++++++++++++++++++++++++++ > > > include/linux/iio/consumer.h | 56 ++++++++++ > > > 2 files changed, 254 insertions(+) > > > > > > diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c > > > index c174ebb7d5e6..d3bbd2444fb5 100644 > > > --- a/drivers/iio/inkern.c > > > +++ b/drivers/iio/inkern.c > > > @@ -1028,3 +1028,201 @@ ssize_t iio_read_channel_label(struct iio_channel > > > *chan, char *buf) > > > return do_iio_read_channel_label(chan->indio_dev, chan->channel, > > > buf); > > > } > > > EXPORT_SYMBOL_GPL(iio_read_channel_label); > > > + > > > +static bool iio_event_exists(struct iio_channel *channel, > > > + enum iio_event_type type, > > > + enum iio_event_direction dir, > > > + enum iio_event_info info) > > > +{ > > > + struct iio_chan_spec const *chan = channel->channel; > > > + int i; > > > + > > > > Can we iio_event_exists() -> iio_event_exists_locked()? Or likely the best way > > wouldn't _unlocked be the convention for this file? Oh, indeed! > > > would be to annotate it with lockdep (though that would mean some dance to get > > the opaque object. > > I will add a lockdep annotation. > > --Sean > > > Anyways, bottom line is it should clear that iio_event_exists() is to be called > > with the lock held. > > > > - Nuno Sá > > > > > + if (!channel->indio_dev->info) > > > + return false; > > > + > > > + for (i = 0; i < chan->num_event_specs; i++) { > > > + if (chan->event_spec[i].type != type) > > > + continue; > > > + if (chan->event_spec[i].dir != dir) > > > + continue; > > > + if (chan->event_spec[i].mask_separate & BIT(info)) > > > + return true; > > > + } > > > + > > > + return false; > > > +} > > > + > > > +umode_t iio_event_mode(struct iio_channel *chan, enum iio_event_type type, > > > + enum iio_event_direction dir, enum iio_event_info > > > info) > > > +{ > > > + struct iio_dev *indio_dev = chan->indio_dev; > > > + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); > > > + umode_t mode = 0; > > > + > > > + guard(mutex)(&iio_dev_opaque->info_exist_lock); > > > + if (!iio_event_exists(chan, type, dir, info)) > > > + return 0; > > > + > > > + if (info == IIO_EV_INFO_ENABLE) { > > > + if (indio_dev->info->read_event_config) > > > + mode |= 0444; > > > + > > > + if (indio_dev->info->write_event_config) > > > + mode |= 0200; > > > + } else { > > > + if (indio_dev->info->read_event_value) > > > + mode |= 0444; > > > + > > > + if (indio_dev->info->write_event_value) > > > + mode |= 0200; > > > + } > > > + > > > + return mode; > > > +} > > > +EXPORT_SYMBOL_GPL(iio_event_mode); > > > + > > > +int iio_read_event_processed_scale(struct iio_channel *chan, > > > + enum iio_event_type type, > > > + enum iio_event_direction dir, > > > + enum iio_event_info info, int *val, > > > + unsigned int scale) > > > +{ > > > + struct iio_dev *indio_dev = chan->indio_dev; > > > + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); > > > + int ret, raw; > > > + > > > + guard(mutex)(&iio_dev_opaque->info_exist_lock); > > > + if (!iio_event_exists(chan, type, dir, info)) > > > + return -ENODEV; > > > + > > > + if (info == IIO_EV_INFO_ENABLE) { > > > + if (!indio_dev->info->read_event_config) > > > + return -EINVAL; > > > + > > > + raw = indio_dev->info->read_event_config(indio_dev, > > > + chan->channel, type, > > > + dir); > > > + if (raw < 0) > > > + return raw; > > > + > > > + *val = raw; > > > + return 0; > > > + } > > > + > > > + if (!indio_dev->info->read_event_value) > > > + return -EINVAL; > > > + > > > + ret = indio_dev->info->read_event_value(indio_dev, chan->channel, > > > type, > > > + dir, info, &raw, NULL); > > > + if (ret < 0) > > > + return ret; > > > + > > > + return iio_convert_raw_to_processed_unlocked(chan, raw, val, scale); > > > +} > > > +EXPORT_SYMBOL_GPL(iio_read_event_processed_scale); > > > + > > > +static int iio_convert_processed_to_raw_unlocked(struct iio_channel *chan, > > > + int processed, int *raw, > > > + unsigned int scale) > > > +{ > > > + int scale_type, scale_val, scale_val2; > > > + int offset_type, offset_val, offset_val2; > > > + s64 r, scale64, raw64; > > > + > > > + scale_type = iio_channel_read(chan, &scale_val, &scale_val2, > > > + IIO_CHAN_INFO_SCALE); > > > + if (scale_type < 0) { > > > + raw64 = processed / scale; > > > + } else { > > > + switch (scale_type) { > > > + case IIO_VAL_INT: > > > + scale64 = (s64)scale_val * scale; > > > + if (scale64 <= INT_MAX && scale64 >= INT_MIN) > > > + raw64 = processed / (int)scale64; > > > + else > > > + raw64 = 0; > > > + break; > > > + case IIO_VAL_INT_PLUS_MICRO: > > > + scale64 = scale_val * scale * 1000000LL + scale_val2; > > > + raw64 = div64_s64_rem(processed, scale64, &r); > > > + raw64 = raw64 * 1000000 + > > > + div64_s64(r * 1000000, scale64); > > > + break; > > > + case IIO_VAL_INT_PLUS_NANO: > > > + scale64 = scale_val * scale * 1000000000LL + > > > scale_val2; > > > + raw64 = div64_s64_rem(processed, scale64, &r); > > > + raw64 = raw64 * 1000000000 + > > > + div64_s64(r * 1000000000, scale64); > > > + break; > > > + case IIO_VAL_FRACTIONAL: > > > + raw64 = div64_s64((s64)processed * scale_val2, > > > + (s64)scale_val * scale); > > > + break; > > > + case IIO_VAL_FRACTIONAL_LOG2: > > > + raw64 = div64_s64((s64)processed << scale_val2, > > > + (s64)scale_val * scale); > > > + break; > > > + default: > > > + return -EINVAL; > > > + } > > > + } > > > + > > > + offset_type = iio_channel_read(chan, &offset_val, &offset_val2, > > > + IIO_CHAN_INFO_OFFSET); > > > + if (offset_type >= 0) { > > > + switch (offset_type) { > > > + case IIO_VAL_INT: > > > + case IIO_VAL_INT_PLUS_MICRO: > > > + case IIO_VAL_INT_PLUS_NANO: > > > + raw64 -= offset_val; > > > + break; > > > + case IIO_VAL_FRACTIONAL: > > > + raw64 -= offset_val / offset_val2; > > > + break; > > > + case IIO_VAL_FRACTIONAL_LOG2: > > > + raw64 -= offset_val >> offset_val2; > > > + break; > > > + default: > > > + return -EINVAL; > > > + } > > > + } > > > + > > > + *raw = clamp(raw64, (s64)INT_MIN, (s64)INT_MAX); > > > + return 0; > > > +} > > > + > > > +int iio_write_event_processed_scale(struct iio_channel *chan, > > > + enum iio_event_type type, > > > + enum iio_event_direction dir, > > > + enum iio_event_info info, int processed, > > > + unsigned int scale) > > > +{ > > > + struct iio_dev *indio_dev = chan->indio_dev; > > > + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(chan- > > > > indio_dev); > > > + int ret, raw; > > > + > > > + guard(mutex)(&iio_dev_opaque->info_exist_lock); > > > + if (!iio_event_exists(chan, type, dir, info)) > > > + return -ENODEV; > > > + > > > + if (info == IIO_EV_INFO_ENABLE) { > > > + if (!indio_dev->info->write_event_config) > > > + return -EINVAL; > > > + > > > + return indio_dev->info->write_event_config(indio_dev, > > > + chan->channel, > > > type, > > > + dir, processed); > > > + } > > > + > > > + if (!indio_dev->info->write_event_value) > > > + return -EINVAL; > > > + > > > + ret = iio_convert_processed_to_raw_unlocked(chan, processed, &raw, > > > + scale); > > > + if (ret < 0) > > > + return ret; > > > + > > > + return indio_dev->info->write_event_value(indio_dev, chan->channel, > > > + type, dir, info, raw, 0); > > > +} > > > +EXPORT_SYMBOL_GPL(iio_write_event_processed_scale); > > > diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h > > > index 6a4479616479..16e7682474f3 100644 > > > --- a/include/linux/iio/consumer.h > > > +++ b/include/linux/iio/consumer.h > > > @@ -451,4 +451,60 @@ ssize_t iio_write_channel_ext_info(struct iio_channel > > > *chan, const char *attr, > > > */ > > > ssize_t iio_read_channel_label(struct iio_channel *chan, char *buf); > > > > > > +/** > > > + * iio_event_mode() - get file mode for an event property > > > + * @chan: Channel being queried > > > + * @type: Event type (theshold, rate-of-change, etc.) > > > + * @dir: Event direction (rising, falling, etc.) > > > + * @info: Event property (enable, value, etc.) > > > + * > > > + * Determine an appropriate mode for sysfs files derived from this event. > > > + * > > > + * Return: > > > + * - `0000` if the event is unsupported or otherwise unavailable > > > + * - `0444` if the event is read-only > > > + * - `0200` if the event is write-only > > > + * - `0644` if the event is read-write > > > + */ > > > +umode_t iio_event_mode(struct iio_channel *chan, enum iio_event_type type, > > > + enum iio_event_direction dir, enum iio_event_info > > > info); > > > + > > > +/** > > > + * iio_read_event_processed_scale() - Read an event property > > > + * @chan: Channel being queried > > > + * @type: Event type (theshold, rate-of-change, etc.) > > > + * @dir: Event direction (rising, falling, etc.) > > > + * @info: Event property (enable, value, etc.) > > > + * @val: Processed property value > > > + * @scale: Factor to scale @val by > > > + * > > > + * Read a processed (scaled and offset) event property of a given channel. > > > + * > > > + * Return: 0 on success, or negative error on failure > > > + */ > > > +int iio_read_event_processed_scale(struct iio_channel *chan, > > > + enum iio_event_type type, > > > + enum iio_event_direction dir, > > > + enum iio_event_info info, int *val, > > > + unsigned int scale); > > > + > > > +/** > > > + * iio_write_event_processed_scale() - Read an event property > > > + * @chan: Channel being queried > > > + * @type: Event type (theshold, rate-of-change, etc.) > > > + * @dir: Event direction (rising, falling, etc.) > > > + * @info: Event property (enable, value, etc.) > > > + * @processed: Processed property value > > > + * @scale: Factor to scale @processed by > > > + * > > > + * Write a processed (scaled and offset) event property of a given channel. > > > + * > > > + * Return: 0 on success, or negative error on failure > > > + */ > > > +int iio_write_event_processed_scale(struct iio_channel *chan, > > > + enum iio_event_type type, > > > + enum iio_event_direction dir, > > > + enum iio_event_info info, int processed, > > > + unsigned int scale); > > > + > > > #endif >
On Mon, Jul 14, 2025 at 09:20:18PM -0400, Sean Anderson wrote: > Add an in-kernel API for reading/writing event properties. Like the > raw-to-processed conversion, with processed-to-raw we only convert the > integer part, introducing some round-off error. > > A common case is for other drivers to re-expose IIO events as sysfs > properties with a different API. To help out with this, iio_event_mode > returns the appropriate mode. It can also be used to test for existence > if the consumer doesn't care about read/write capability. ... > +EXPORT_SYMBOL_GPL(iio_event_mode); Can we move this to namespace? Otherwise it will be never ending story... Ditto for other new APIs. ... > + if (scale64 <= INT_MAX && scale64 >= INT_MIN) > + raw64 = processed / (int)scale64; Do you need the casting? (I mean if the compiler is dumb enough to not see this) ... > + case IIO_VAL_INT_PLUS_MICRO: > + scale64 = scale_val * scale * 1000000LL + scale_val2; > + raw64 = div64_s64_rem(processed, scale64, &r); > + raw64 = raw64 * 1000000 + > + div64_s64(r * 1000000, scale64); Logically this should be 1000000L, but can we somehow use the constants? Like scale64 = (s64)MICRO * scale_val * scale + scale_val2; raw64 = div64_s64_rem(processed, scale64, &r); raw64 = raw64 * (s32)MICRO + div64_s64(r * (s64)MICRO, scale64); > + break; Ditto for other cases? ... > + *raw = clamp(raw64, (s64)INT_MIN, (s64)INT_MAX); You already have similar approach here... ... > + ret = iio_convert_processed_to_raw_unlocked(chan, processed, &raw, > + scale); > + if (ret < 0) Why ' < 0' ? > + return ret; -- With Best Regards, Andy Shevchenko
On 7/15/25 04:18, Andy Shevchenko wrote: > On Mon, Jul 14, 2025 at 09:20:18PM -0400, Sean Anderson wrote: >> Add an in-kernel API for reading/writing event properties. Like the >> raw-to-processed conversion, with processed-to-raw we only convert the >> integer part, introducing some round-off error. >> >> A common case is for other drivers to re-expose IIO events as sysfs >> properties with a different API. To help out with this, iio_event_mode >> returns the appropriate mode. It can also be used to test for existence >> if the consumer doesn't care about read/write capability. > > ... > >> +EXPORT_SYMBOL_GPL(iio_event_mode); > > Can we move this to namespace? Otherwise it will be never ending story... > Ditto for other new APIs. Never ending story of what? > ... > >> + if (scale64 <= INT_MAX && scale64 >= INT_MIN) >> + raw64 = processed / (int)scale64; > > Do you need the casting? (I mean if the compiler is dumb enough to not see this) AIUI 64-bit division is not available on 32-bit platforms. The cast ensures we get 32-bit division. > ... > >> + case IIO_VAL_INT_PLUS_MICRO: >> + scale64 = scale_val * scale * 1000000LL + scale_val2; >> + raw64 = div64_s64_rem(processed, scale64, &r); >> + raw64 = raw64 * 1000000 + >> + div64_s64(r * 1000000, scale64); > > Logically this should be 1000000L, but can we somehow use the constants? > Like > > scale64 = (s64)MICRO * scale_val * scale + scale_val2; > raw64 = div64_s64_rem(processed, scale64, &r); > raw64 = raw64 * (s32)MICRO + > div64_s64(r * (s64)MICRO, scale64); > This follows iio_convert_raw_to_processed_unlocked but ok... >> + break; > > Ditto for other cases? > > ... > >> + *raw = clamp(raw64, (s64)INT_MIN, (s64)INT_MAX); > > You already have similar approach here... Well, I can spell it 0x7fffffffLL if you'd like... >> + ret = iio_convert_processed_to_raw_unlocked(chan, processed, &raw, >> + scale); >> + if (ret < 0) > > Why ' < 0' ? Originally I returned IIO_VAL_INT but later decided against it. --Sean
On Tue, Jul 15, 2025 at 11:42:05AM -0400, Sean Anderson wrote: > On 7/15/25 04:18, Andy Shevchenko wrote: > > On Mon, Jul 14, 2025 at 09:20:18PM -0400, Sean Anderson wrote: ... > >> +EXPORT_SYMBOL_GPL(iio_event_mode); > > > > Can we move this to namespace? Otherwise it will be never ending story... > > Ditto for other new APIs. > > Never ending story of what? Of converting IIO core to use exported namespaces. ... > >> + if (scale64 <= INT_MAX && scale64 >= INT_MIN) > >> + raw64 = processed / (int)scale64; > > > > Do you need the casting? (I mean if the compiler is dumb enough to not see this) > > AIUI 64-bit division is not available on 32-bit platforms. The cast > ensures we get 32-bit division. I put specifically a remark in the parentheses. So, the Q is if the compiler doesn't recognize that. Can you confirm that 32-bit compilation without cast is broken? ... > >> + *raw = clamp(raw64, (s64)INT_MIN, (s64)INT_MAX); > > > > You already have similar approach here... > > Well, I can spell it 0x7fffffffLL if you'd like... Nope, I like to have named constants instead of magics, but actually are those castings needed for the clamp()? -- With Best Regards, Andy Shevchenko
On 7/16/25 05:28, Andy Shevchenko wrote: > On Tue, Jul 15, 2025 at 11:42:05AM -0400, Sean Anderson wrote: >> On 7/15/25 04:18, Andy Shevchenko wrote: >> > On Mon, Jul 14, 2025 at 09:20:18PM -0400, Sean Anderson wrote: > > ... > >> >> +EXPORT_SYMBOL_GPL(iio_event_mode); >> > >> > Can we move this to namespace? Otherwise it will be never ending story... >> > Ditto for other new APIs. >> >> Never ending story of what? > > Of converting IIO core to use exported namespaces. What's the purpose? >> >> + if (scale64 <= INT_MAX && scale64 >= INT_MIN) >> >> + raw64 = processed / (int)scale64; >> > >> > Do you need the casting? (I mean if the compiler is dumb enough to not see this) >> >> AIUI 64-bit division is not available on 32-bit platforms. The cast >> ensures we get 32-bit division. > > I put specifically a remark in the parentheses. So, the Q is if the compiler > doesn't recognize that. Can you confirm that 32-bit compilation without cast > is broken? inkern.c:(.text.iio_write_event_processed_scale+0x14c): undefined reference to `__aeabi_ldivmod' >> >> + *raw = clamp(raw64, (s64)INT_MIN, (s64)INT_MAX); >> > >> > You already have similar approach here... >> >> Well, I can spell it 0x7fffffffLL if you'd like... > > Nope, I like to have named constants instead of magics, but actually are those > castings needed for the clamp()? Apparently not. The checks in __clamp_once are only for matching signedness. And the ints are promoted to s64s when the comparison is made. --Sean
On Thu, 17 Jul 2025 12:42:13 -0400 Sean Anderson <sean.anderson@linux.dev> wrote: > On 7/16/25 05:28, Andy Shevchenko wrote: > > On Tue, Jul 15, 2025 at 11:42:05AM -0400, Sean Anderson wrote: > >> On 7/15/25 04:18, Andy Shevchenko wrote: > >> > On Mon, Jul 14, 2025 at 09:20:18PM -0400, Sean Anderson wrote: > > > > ... > > > >> >> +EXPORT_SYMBOL_GPL(iio_event_mode); > >> > > >> > Can we move this to namespace? Otherwise it will be never ending story... > >> > Ditto for other new APIs. > >> > >> Never ending story of what? > > > > Of converting IIO core to use exported namespaces. > > What's the purpose? Aim here is in general to reduce the massive exposed ABI by applying some namespaces so that only drivers that opt in to specific functionality can use particular symbols. We've used it extensively for groups of related drivers and to some libraries and the DMA buffers, but so far not pushed it into the IIO core. I'd be fine with these new functions all being under IIO_CONSUMER or similar. Quite a bit of feedback on this set will be of the lines of don't do it the way we did it before as now we know better! > > >> >> + if (scale64 <= INT_MAX && scale64 >= INT_MIN) > >> >> + raw64 = processed / (int)scale64; > >> > > >> > Do you need the casting? (I mean if the compiler is dumb enough to not see this) > >> > >> AIUI 64-bit division is not available on 32-bit platforms. The cast > >> ensures we get 32-bit division. > > > > I put specifically a remark in the parentheses. So, the Q is if the compiler > > doesn't recognize that. Can you confirm that 32-bit compilation without cast > > is broken? > > inkern.c:(.text.iio_write_event_processed_scale+0x14c): undefined reference to `__aeabi_ldivmod' > > >> >> + *raw = clamp(raw64, (s64)INT_MIN, (s64)INT_MAX); > >> > > >> > You already have similar approach here... > >> > >> Well, I can spell it 0x7fffffffLL if you'd like... > > > > Nope, I like to have named constants instead of magics, but actually are those > > castings needed for the clamp()? > > Apparently not. The checks in __clamp_once are only for matching signedness. And > the ints are promoted to s64s when the comparison is made. > > --Sean
© 2016 - 2025 Red Hat, Inc.