linux-kernel - Re: [PATCH v3 1/7] iio: core: Add and export __iio_dev_mode

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260116201847.3560a2e2@jic23-huawei>
Date: Fri, 16 Jan 2026 20:18:47 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Kurt Borja <kuurtb@...il.com>
Cc: Andy Shevchenko <andriy.shevchenko@...el.com>, Lars-Peter Clausen
 <lars@...afoo.de>, Michael Hennerich <Michael.Hennerich@...log.com>, Benson
 Leung <bleung@...omium.org>, Antoniu Miclaus <antoniu.miclaus@...log.com>,
 Gwendal Grignou <gwendal@...omium.org>, Shrikant Raskar
 <raskar.shree97@...il.com>, Per-Daniel Olsson <perdaniel.olsson@...s.com>,
 David Lechner <dlechner@...libre.com>, Nuno Sá
 <nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>, Guenter Roeck
 <groeck@...omium.org>, Jonathan Cameron <Jonathan.Cameron@...wei.com>,
 linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
 chrome-platform@...ts.linux.dev
Subject: Re: [PATCH v3 1/7] iio: core: Add and export __iio_dev_mode_lock()

On Tue, 06 Jan 2026 03:06:56 -0500
Kurt Borja <kuurtb@...il.com> wrote:

> Add unconditional wrappers around the internal IIO mode lock.
> 
> As mentioned in the documentation, this is not meant to be used by
> drivers, instead this will aid in the eventual addition of cleanup
> classes around conditional locks.
> 
> Signed-off-by: Kurt Borja <kuurtb@...il.com>
Hi Kurt,

I'm being a bit conservative in looking to apply this so apologies
if it seems like I'm ignoring you! I wanted to give plenty of time
for others to take a look.

A few comments, but if we go with this version I'll tweak the
punctuation if I remember whilst applying.

Jonathan


> ---
>  drivers/iio/industrialio-core.c | 30 ++++++++++++++++++++++++++++++
>  include/linux/iio/iio.h         |  3 +++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index f69deefcfb6f..34867a860a84 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -2171,6 +2171,36 @@ int __devm_iio_device_register(struct device *dev, struct iio_dev *indio_dev,
>  }
>  EXPORT_SYMBOL_GPL(__devm_iio_device_register);
>  
> +/**
> + * __iio_dev_mode_lock - Locks the current IIO device mode
> + * @indio_dev: the iio_dev associated with the device
> + *
> + * If the device is either in direct or buffer mode, it's guaranteed to stay
> + * that way until __iio_dev_mode_unlock() is called.
> + *
> + * This function is not meant to be used directly by drivers to protect internal
> + * state, a driver should have it's own mechanisms for that matter.

Nitpick, shouldn't be a comma.  Either

 * This function is not meant to be used directly by drivers to protect internal
 * state. A driver should have it's own mechanisms for that matter.

or if you like fancy uses of the semi colon.

 * This function is not meant to be used directly by drivers to protect internal
 * state; a driver should have it's own mechanisms for that matter.

> + *
> + * There are very few cases where a driver actually needs to lock the current
> + * mode unconditionally. It's recommended to use iio_device_claim_direct() or
> + * iio_device_claim_buffer_mode() pairs or related helpers instead.
> + */
> +void __iio_dev_mode_lock(struct iio_dev *indio_dev)
> +{
> +	mutex_lock(&to_iio_dev_opaque(indio_dev)->mlock);
> +}
> +EXPORT_SYMBOL_GPL(__iio_dev_mode_lock);
> +
> +/**
> + * __iio_dev_mode_unlock - Unlocks the current IIO device mode
> + * @indio_dev: the iio_dev associated with the device
> + */
> +void __iio_dev_mode_unlock(struct iio_dev *indio_dev)
> +{
> +	mutex_unlock(&to_iio_dev_opaque(indio_dev)->mlock);
> +}
> +EXPORT_SYMBOL_GPL(__iio_dev_mode_unlock);
> +
>  /**
>   * __iio_device_claim_direct - Keep device in direct mode
>   * @indio_dev:	the iio_dev associated with the device
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index 872ebdf0dd77..aecda887d833 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -661,6 +661,9 @@ void iio_device_unregister(struct iio_dev *indio_dev);
>  int __devm_iio_device_register(struct device *dev, struct iio_dev *indio_dev,
>  			       struct module *this_mod);
>  int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp);
> +
> +void __iio_dev_mode_lock(struct iio_dev *indio_dev) __acquires(indio_dev);
> +void __iio_dev_mode_unlock(struct iio_dev *indio_dev) __releases(indio_dev);
This is an interesting notation choice as there are several locks embedded
in iio_devs but I think it is the only one we want to expose so fair enough
if we don't see any false warnings from this!

Jonathan

>  bool __iio_device_claim_direct(struct iio_dev *indio_dev);
>  void __iio_device_release_direct(struct iio_dev *indio_dev);
>  
>