[PATCH 0/7] iio: buffer: document calling context when pushing to buffers

[PATCH 0/7] iio: buffer: document calling context when pushing to buffers

[David Lechner]

It came up in a recent discussion [1] that we need to be careful about
the calling context for various iio_push_to_buffer*() functions. Here is
a series that adds some documentation in a number of places to make this
a bit more visible.

[1]: https://lore.kernel.org/linux-iio/CAHp75Vc8u2N2AHWtnPRmRXWKN3u8Qi=yvx5afbFh4NLNb8-y9A@mail.gmail.com/

---
David Lechner (7):
      iio: buffer: document iio_push_to_buffers_with_ts_unaligned() may sleep
      iio: buffer: iio_push_to_buffers_with_ts_unaligned() might_sleep()
      iio: buffer: document iio_push_to_buffers_with_ts()
      iio: buffer: deprecated iio_push_to_buffers_with_timestamp()
      iio: buffer: document iio_push_to_buffers() calling context
      iio: buffer: document store_to() callback may be called in any context
      iio: buffer: document that buffer callback must be context safe

 drivers/iio/buffer/industrialio-buffer-cb.c |  1 +
 drivers/iio/industrialio-buffer.c           |  8 ++++++++
 include/linux/iio/buffer.h                  | 22 +++++++++++++++++-----
 include/linux/iio/buffer_impl.h             |  3 ++-
 include/linux/iio/consumer.h                |  3 ++-
 5 files changed, 30 insertions(+), 7 deletions(-)
---
base-commit: b8902d55155cec7bd743dc1129e0b32e70b1751f
change-id: 20250912-iio-doc-push-to-buffers-context-df3ca98fe0df

Best regards,
-- 
David Lechner <dlechner@baylibre.com>

[PATCH 1/7] iio: buffer: document iio_push_to_buffers_with_ts_unaligned() may sleep

[David Lechner]

Add Context: documentation comment that
iio_push_to_buffers_with_ts_unaligned() may sleep because it calls
devm_krealloc().

Also document Return: value while here.

Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/industrialio-buffer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index a80f7cc25a27109cfe22ce447c4117fc94bc8310..7da43a1f2f75f32dc93b9a5fe903378a79e82fe3 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -2401,6 +2401,9 @@ EXPORT_SYMBOL_GPL(iio_push_to_buffers);
  * not require space for the timestamp, or 8 byte alignment of data.
  * It does however require an allocation on first call and additional
  * copies on all calls, so should be avoided if possible.
+ *
+ * Context: May sleep.
+ * Return: 0 on success, negative error code on failure.
  */
 int iio_push_to_buffers_with_ts_unaligned(struct iio_dev *indio_dev,
 					  const void *data,

-- 
2.43.0

Re: [PATCH 1/7] iio: buffer: document iio_push_to_buffers_with_ts_unaligned() may sleep

[Andy Shevchenko]

On Fri, Sep 12, 2025 at 11:05:52AM -0500, David Lechner wrote:
> Add Context: documentation comment that
> iio_push_to_buffers_with_ts_unaligned() may sleep because it calls
> devm_krealloc().
> 
> Also document Return: value while here.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

-- 
With Best Regards,
Andy Shevchenko

[PATCH 2/7] iio: buffer: iio_push_to_buffers_with_ts_unaligned() might_sleep()

[David Lechner]

Call might_sleep() in iio_push_to_buffers_with_ts_unaligned() since it
can allocate memory, which may sleep.

Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/industrialio-buffer.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 7da43a1f2f75f32dc93b9a5fe903378a79e82fe3..5599fa37b698dda6ff126496f62939331c12f524 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -2412,6 +2412,8 @@ int iio_push_to_buffers_with_ts_unaligned(struct iio_dev *indio_dev,
 {
 	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
 
+	might_sleep();
+
 	/*
 	 * Conservative estimate - we can always safely copy the minimum
 	 * of either the data provided or the length of the destination buffer.

-- 
2.43.0

Re: [PATCH 2/7] iio: buffer: iio_push_to_buffers_with_ts_unaligned() might_sleep()

[Andy Shevchenko]

On Fri, Sep 12, 2025 at 11:05:53AM -0500, David Lechner wrote:
> Call might_sleep() in iio_push_to_buffers_with_ts_unaligned() since it
> can allocate memory, which may sleep.

It can or does it always do?
If the first one is correct, better to use might_sleep_if().

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/7] iio: buffer: iio_push_to_buffers_with_ts_unaligned() might_sleep()

[David Lechner]

On 9/12/25 1:10 PM, Andy Shevchenko wrote:
> On Fri, Sep 12, 2025 at 11:05:53AM -0500, David Lechner wrote:
>> Call might_sleep() in iio_push_to_buffers_with_ts_unaligned() since it
>> can allocate memory, which may sleep.
> 
> It can or does it always do?
> If the first one is correct, better to use might_sleep_if().
> 

Just below this in the function is:

	if (iio_dev_opaque->bounce_buffer_size !=  indio_dev->scan_bytes) {
		void *bb;

		bb = devm_krealloc(&indio_dev->dev,
				   iio_dev_opaque->bounce_buffer,
				   indio_dev->scan_bytes, GFP_KERNEL);
		if (!bb)
			return -ENOMEM;
		iio_dev_opaque->bounce_buffer = bb;
		iio_dev_opaque->bounce_buffer_size = indio_dev->scan_bytes;
	}


Would it make sense to move the might_sleep() inside of this
if statement rather than repeat the condition in might_sleep_if()?

devm_krealloc() is the only part of this function that might sleep.

Re: [PATCH 2/7] iio: buffer: iio_push_to_buffers_with_ts_unaligned() might_sleep()

[Jonathan Cameron]

On Fri, 12 Sep 2025 13:40:37 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 9/12/25 1:10 PM, Andy Shevchenko wrote:
> > On Fri, Sep 12, 2025 at 11:05:53AM -0500, David Lechner wrote:  
> >> Call might_sleep() in iio_push_to_buffers_with_ts_unaligned() since it
> >> can allocate memory, which may sleep.  
> > 
> > It can or does it always do?
> > If the first one is correct, better to use might_sleep_if().
> >   
> 
> Just below this in the function is:
> 
> 	if (iio_dev_opaque->bounce_buffer_size !=  indio_dev->scan_bytes) {
> 		void *bb;
> 
> 		bb = devm_krealloc(&indio_dev->dev,
> 				   iio_dev_opaque->bounce_buffer,
> 				   indio_dev->scan_bytes, GFP_KERNEL);
> 		if (!bb)
> 			return -ENOMEM;
> 		iio_dev_opaque->bounce_buffer = bb;
> 		iio_dev_opaque->bounce_buffer_size = indio_dev->scan_bytes;
> 	}
> 
> 
> Would it make sense to move the might_sleep() inside of this
> if statement rather than repeat the condition in might_sleep_if()?
> 
> devm_krealloc() is the only part of this function that might sleep.
> 
Whilst true that the sleep is only at this point, we always go into
this path the first time (assuming I remember correctly how this works).

So I'd argue a might_sleep() where you have it is appropriate
as we will already have spat out the debug info if we get to the
second case that doesn't sleep.

If this ever matters to a driver, we could add a new function
to allocate the bounce buffer earlier. 

Jonathan

[PATCH 3/7] iio: buffer: document iio_push_to_buffers_with_ts()

[David Lechner]

Document the iio_push_to_buffers_with_ts() function.

This is copied and slightly cleaned up from
iio_push_to_buffers_with_timestamp().

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 include/linux/iio/buffer.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
index 5c84ec4a981001dfba3b369f811368bab70a3117..47f23bc0470eaf308fc3a739faaf9b56cac0ef67 100644
--- a/include/linux/iio/buffer.h
+++ b/include/linux/iio/buffer.h
@@ -45,6 +45,22 @@ static inline int iio_push_to_buffers_with_timestamp(struct iio_dev *indio_dev,
 	return iio_push_to_buffers(indio_dev, data);
 }
 
+/**
+ * iio_push_to_buffers_with_ts() - push data and timestamp to buffers
+ * @indio_dev:		iio_dev structure for device.
+ * @data:		Pointer to sample data buffer.
+ * @data_total_len:	The size of @data in bytes.
+ * @timestamp:		Timestamp for the sample data.
+ *
+ * Pushes data to the IIO device's buffers. If timestamps are enabled for the
+ * device the function will store the supplied timestamp as the last element in
+ * the sample data buffer before pushing it to the device buffers. The sample
+ * data buffer needs to be large enough to hold the additional timestamp
+ * (usually the buffer should be indio->scan_bytes bytes large).
+ *
+ * Context: Any context.
+ * Return: 0 on success, a negative error code otherwise.
+ */
 static inline int iio_push_to_buffers_with_ts(struct iio_dev *indio_dev,
 					      void *data, size_t data_total_len,
 					      s64 timestamp)

-- 
2.43.0

Re: [PATCH 3/7] iio: buffer: document iio_push_to_buffers_with_ts()

[Jonathan Cameron]

On Fri, 12 Sep 2025 11:05:54 -0500
David Lechner <dlechner@baylibre.com> wrote:

> Document the iio_push_to_buffers_with_ts() function.
> 
> This is copied and slightly cleaned up from
> iio_push_to_buffers_with_timestamp().
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
>  include/linux/iio/buffer.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
> index 5c84ec4a981001dfba3b369f811368bab70a3117..47f23bc0470eaf308fc3a739faaf9b56cac0ef67 100644
> --- a/include/linux/iio/buffer.h
> +++ b/include/linux/iio/buffer.h
> @@ -45,6 +45,22 @@ static inline int iio_push_to_buffers_with_timestamp(struct iio_dev *indio_dev,
>  	return iio_push_to_buffers(indio_dev, data);
>  }
>  
> +/**
> + * iio_push_to_buffers_with_ts() - push data and timestamp to buffers
> + * @indio_dev:		iio_dev structure for device.
> + * @data:		Pointer to sample data buffer.
> + * @data_total_len:	The size of @data in bytes.
> + * @timestamp:		Timestamp for the sample data.
> + *
> + * Pushes data to the IIO device's buffers. If timestamps are enabled for the
> + * device the function will store the supplied timestamp as the last element in
> + * the sample data buffer before pushing it to the device buffers. The sample
> + * data buffer needs to be large enough to hold the additional timestamp
> + * (usually the buffer should be indio->scan_bytes bytes large).
usually the buffer should be at least indio->scan_bytes bytes large)

It is really common for it to be bigger and don't want people thinking the
need to dynamically resize it!

> + *
> + * Context: Any context.
> + * Return: 0 on success, a negative error code otherwise.
> + */
>  static inline int iio_push_to_buffers_with_ts(struct iio_dev *indio_dev,
>  					      void *data, size_t data_total_len,
>  					      s64 timestamp)
> 

[PATCH 4/7] iio: buffer: deprecated iio_push_to_buffers_with_timestamp()

[David Lechner]

Replace the documentation of iio_push_to_buffers_with_timestamp() with
a deprecation notice pointing to the preferred alternative.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 include/linux/iio/buffer.h | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
index 47f23bc0470eaf308fc3a739faaf9b56cac0ef67..f07f6bae0cb25b0f51bd671206177508d1064b79 100644
--- a/include/linux/iio/buffer.h
+++ b/include/linux/iio/buffer.h
@@ -26,11 +26,7 @@ int iio_pop_from_buffer(struct iio_buffer *buffer, void *data);
  * @data:		sample data
  * @timestamp:		timestamp for the sample data
  *
- * Pushes data to the IIO device's buffers. If timestamps are enabled for the
- * device the function will store the supplied timestamp as the last element in
- * the sample data buffer before pushing it to the device buffers. The sample
- * data buffer needs to be large enough to hold the additional timestamp
- * (usually the buffer should be indio->scan_bytes bytes large).
+ * This function is deprecated. Use iio_push_to_buffers_with_ts() instead.
  *
  * Returns 0 on success, a negative error code otherwise.
  */

-- 
2.43.0

Re: [PATCH 4/7] iio: buffer: deprecated iio_push_to_buffers_with_timestamp()

[Andy Shevchenko]

On Fri, Sep 12, 2025 at 11:05:55AM -0500, David Lechner wrote:
> Replace the documentation of iio_push_to_buffers_with_timestamp() with
> a deprecation notice pointing to the preferred alternative.

...

> + * This function is deprecated. Use iio_push_to_buffers_with_ts() instead.

There is informal practice to mark function deprecated (in the kernel
documentation), it's something like using capitalised DEPRECATED. Grep for
better examples.

-- 
With Best Regards,
Andy Shevchenko

[PATCH 5/7] iio: buffer: document iio_push_to_buffers() calling context

[David Lechner]

Document that iio_push_to_buffers() can be called from any context.

Also document the Return: value while here.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/industrialio-buffer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 5599fa37b698dda6ff126496f62939331c12f524..f1448ae1b843fc577599fc1b9cf6d859bba226f1 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -2372,6 +2372,9 @@ static int iio_push_to_buffer(struct iio_buffer *buffer, const void *data)
  * iio_push_to_buffers() - push to a registered buffer.
  * @indio_dev:		iio_dev structure for device.
  * @data:		Full scan.
+ *
+ * Context: Any context.
+ * Return: 0 on success, negative error code on failure.
  */
 int iio_push_to_buffers(struct iio_dev *indio_dev, const void *data)
 {

-- 
2.43.0

[PATCH 6/7] iio: buffer: document store_to() callback may be called in any context

[David Lechner]

Document that the struct iio_buffer_access_funcs.store_to() callback
must be safe to call from any context since it is called from
iio_push_to_buffer() which may be called from any context.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 include/linux/iio/buffer_impl.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/iio/buffer_impl.h b/include/linux/iio/buffer_impl.h
index e72552e026f3a3c15fca04d26aaa91dbd27e17bc..0daff9ff20ce49de67fa0f2ac6191882de2f4a67 100644
--- a/include/linux/iio/buffer_impl.h
+++ b/include/linux/iio/buffer_impl.h
@@ -24,7 +24,8 @@ struct sg_table;
 
 /**
  * struct iio_buffer_access_funcs - access functions for buffers.
- * @store_to:		actually store stuff to the buffer
+ * @store_to:		actually store stuff to the buffer - must be safe to
+ *			call from any context (e.g. must not sleep).
  * @read:		try to get a specified number of bytes (must exist)
  * @data_available:	indicates how much data is available for reading from
  *			the buffer.

-- 
2.43.0

[PATCH 7/7] iio: buffer: document that buffer callback must be context safe

[David Lechner]

Document that the callback registered with iio_channel_get_all_cb()
must be safe to call from any context since it is called from by
iio_push_to_buffer() which can be called in any context.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/buffer/industrialio-buffer-cb.c | 1 +
 include/linux/iio/consumer.h                | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/buffer/industrialio-buffer-cb.c b/drivers/iio/buffer/industrialio-buffer-cb.c
index 3e27385069edb5b717a4c56e72dff2c0a238fa07..f4ebff9684936365ec179489d9041d8bdbe58738 100644
--- a/drivers/iio/buffer/industrialio-buffer-cb.c
+++ b/drivers/iio/buffer/industrialio-buffer-cb.c
@@ -13,6 +13,7 @@
 
 struct iio_cb_buffer {
 	struct iio_buffer buffer;
+	/* Must be safe to call from any context (e.g. must not sleep). */
 	int (*cb)(const void *data, void *private);
 	void *private;
 	struct iio_channel *channels;
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index a38b277c2c02cb85a766d645a046bdfa534db51a..5039558267e4c17b2e09246036d0df352e242d06 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -131,7 +131,8 @@ struct iio_cb_buffer;
 /**
  * iio_channel_get_all_cb() - register callback for triggered capture
  * @dev:		Pointer to client device.
- * @cb:			Callback function.
+ * @cb:			Callback function. Must be safe to call from any context
+ *			(e.g. must not sleep).
  * @private:		Private data passed to callback.
  *
  * NB right now we have no ability to mux data from multiple devices.

-- 
2.43.0

Re: [PATCH 0/7] iio: buffer: document calling context when pushing to buffers

[Jonathan Cameron]

On Fri, 12 Sep 2025 11:05:51 -0500
David Lechner <dlechner@baylibre.com> wrote:

> It came up in a recent discussion [1] that we need to be careful about
> the calling context for various iio_push_to_buffer*() functions. Here is
> a series that adds some documentation in a number of places to make this
> a bit more visible.
> 
> [1]: https://lore.kernel.org/linux-iio/CAHp75Vc8u2N2AHWtnPRmRXWKN3u8Qi=yvx5afbFh4NLNb8-y9A@mail.gmail.com/
> 
> ---
> David Lechner (7):
>       iio: buffer: document iio_push_to_buffers_with_ts_unaligned() may sleep
>       iio: buffer: iio_push_to_buffers_with_ts_unaligned() might_sleep()
>       iio: buffer: document iio_push_to_buffers_with_ts()
>       iio: buffer: deprecated iio_push_to_buffers_with_timestamp()
>       iio: buffer: document iio_push_to_buffers() calling context
>       iio: buffer: document store_to() callback may be called in any context
>       iio: buffer: document that buffer callback must be context safe
> 
>  drivers/iio/buffer/industrialio-buffer-cb.c |  1 +
>  drivers/iio/industrialio-buffer.c           |  8 ++++++++
>  include/linux/iio/buffer.h                  | 22 +++++++++++++++++-----
>  include/linux/iio/buffer_impl.h             |  3 ++-
>  include/linux/iio/consumer.h                |  3 ++-
>  5 files changed, 30 insertions(+), 7 deletions(-)
> ---
> base-commit: b8902d55155cec7bd743dc1129e0b32e70b1751f
> change-id: 20250912-iio-doc-push-to-buffers-context-df3ca98fe0df
> 
> Best regards,

Whilst this is very nearly good to go (assuming the might_sleep() discussion
is done), I'm going to be lazy and ask for a v2 to resolve that DEPRECATED
comment style thing and the tiny change I asked for.

Thanks for doing this and I'm a bit embarrassed that I didn't document
the with_ts() function when introducing it :( 

Jonathan