| 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
| ||
|
Message-ID: <8e598681-f0a2-4ced-8f68-17384ef95de4@linux.dev>
Date: Tue, 15 Jul 2025 11:43:14 -0400
From: Sean Anderson <sean.anderson@...ux.dev>
To: Nuno Sá <noname.nuno@...il.com>,
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 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@...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
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
Powered by blists - more mailing lists