Re: [PATCH 2/7] iio: inkern: Add API for reading/writing events

[Sean Anderson <sean.anderson@linux.dev>]

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