lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0fad26b4384e4163f4807f6b779361099f505a86.camel@gmail.com>
Date: Tue, 15 Jul 2025 11:35:32 +0100
From: Nuno Sá <noname.nuno@...il.com>
To: Sean Anderson <sean.anderson@...ux.dev>, Jonathan Cameron
 <jic23@...nel.org>,  Jean Delvare <jdelvare@...e.com>, Guenter Roeck
 <linux@...ck-us.net>, linux-iio@...r.kernel.org, 
	linux-hwmon@...r.kernel.org
Cc: Andy Shevchenko <andy@...nel.org>, Nuno Sá
	 <nuno.sa@...log.com>, linux-kernel@...r.kernel.org, David Lechner
	 <dlechner@...libre.com>
Subject: Re: [PATCH 2/7] iio: inkern: Add API for reading/writing events

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@...ux.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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ