[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250508200933.5b1ad6af@jic23-huawei>
Date: Thu, 8 May 2025 20:09:33 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: David Lechner <dlechner@...libre.com>
Cc: Nuno Sá <nuno.sa@...log.com>, Andy Shevchenko
<andy@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>, Michael Hennerich
<Michael.Hennerich@...log.com>, Eugen Hristev <eugen.hristev@...aro.org>,
Nicolas Ferre <nicolas.ferre@...rochip.com>, Alexandre Belloni
<alexandre.belloni@...tlin.com>, Claudiu Beznea <claudiu.beznea@...on.dev>,
linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, Trevor Gamblin
<tgamblin@...libre.com>
Subject: Re: [PATCH v6 0/7] iio: introduce IIO_DECLARE_BUFFER_WITH_TS
On Wed, 07 May 2025 15:42:39 -0500
David Lechner <dlechner@...libre.com> wrote:
> Creating a buffer of the proper size and correct alignment for use with
> iio_push_to_buffers_with_ts() is commonly used and not easy to get
> right (as seen by a number of recent fixes on the mailing list).
>
> In general, we prefer to use this pattern for creating such buffers:
>
> struct {
> u16 data[2];
> aligned_s64 timestamp;
> } buffer;
>
> However, there are many cases where a driver may have a large number of
> channels that can be optionally enabled or disabled in a scan or the
> driver might support a range of chips that have different numbers of
> channels or different storage sizes for the data. In these cases, the
> timestamp may not always be at the same place relative to the data. To
> handle these, we allocate a buffer large enough for the largest possible
> case and don't care exactly where the timestamp ends up in the buffer.
>
> For these cases, we propose to introduce new macros to make it easier
> it easier for both the authors to get it right and for readers of the
> code to not have to do all of the math to verify that it is correct.
>
> I have just included a few examples of drivers that can make use of this
> new macro, but there are dozens more.
>
Seems better. Applied and pushed out as testing to see if any other
compiler versions don't like it!
J
> ---
> Changes in v6:
> - Rework IIO_DMA_MINALIGN to work around clang compiler issue.
> - Link to v5: https://lore.kernel.org/r/20250505-iio-introduce-iio_declare_buffer_with_ts-v5-0-814b72b1cae3@baylibre.com
>
> Changes in v5:
> - Add new patch to set minimum alignment to 8 for IIO_DMA_MINALIGN.
> - Adjust IIO_DECLARE_DMA_BUFFER_WITH_TS() macro for above change.
> - Drop one ad4695 patch that was already applied.
> - Link to v4: https://lore.kernel.org/r/20250428-iio-introduce-iio_declare_buffer_with_ts-v4-0-6f7f6126f1cb@baylibre.com
>
> Changes in v4:
> - Dropped static_assert()s from the first patch.
> - Handle case when IIO_DMA_MINALIGN < sizeof(timestamp).
> - Added one more patch for ad4695 to rename a confusing macro.
> - Link to v3: https://lore.kernel.org/r/20250425-iio-introduce-iio_declare_buffer_with_ts-v3-0-f12df1bff248@baylibre.com
>
> Changes in v3:
> - Fixed a few mistakes, style issues and incorporate other feedback (see
> individual commit message changelogs for details).
> - Link to v2: https://lore.kernel.org/r/20250422-iio-introduce-iio_declare_buffer_with_ts-v2-0-3fd36475c706@baylibre.com
>
> Changes in v2:
> - Add 2nd macro for case where we need DMA alignment.
> - Add new patch for ad4695 to convert buffer from u8 to u16 before
> making use of the new macro.
> - Drop the bmp280 patch since it was determined to have a better
> alternative not using these macros.
> - Add a few more examples to show the non-DMA case, both in a struct and
> stack allocated.
> - Link to v1: https://lore.kernel.org/r/20250418-iio-introduce-iio_declare_buffer_with_ts-v1-0-ee0c62a33a0f@baylibre.com
>
> ---
> David Lechner (7):
> iio: make IIO_DMA_MINALIGN minimum of 8 bytes
> iio: introduce IIO_DECLARE_BUFFER_WITH_TS macros
> iio: adc: ad4695: use IIO_DECLARE_DMA_BUFFER_WITH_TS
> iio: adc: ad4695: rename AD4695_MAX_VIN_CHANNELS
> iio: adc: ad7380: use IIO_DECLARE_DMA_BUFFER_WITH_TS
> iio: accel: sca3300: use IIO_DECLARE_BUFFER_WITH_TS
> iio: adc: at91-sama5d2: use IIO_DECLARE_BUFFER_WITH_TS
>
> drivers/iio/accel/sca3300.c | 18 ++---------------
> drivers/iio/adc/ad4695.c | 11 +++++-----
> drivers/iio/adc/ad7380.c | 3 +--
> drivers/iio/adc/at91-sama5d2_adc.c | 13 ++----------
> include/linux/iio/iio.h | 41 +++++++++++++++++++++++++++++++++++++-
> 5 files changed, 50 insertions(+), 36 deletions(-)
> ---
> base-commit: 7e9a82ab5b861d3c33c99a22c1245a5b262ee502
> change-id: 20250418-iio-introduce-iio_declare_buffer_with_ts-2f8773f7dad6
>
> Best regards,
Powered by blists - more mailing lists