drivers/iio/proximity/sx9500.c | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-)
Use IIO_DECLARE_BUFFER_WITH_TS() to declare a stack allocated buffer
in sx9500_trigger_handler(). Since the scan buffer isn't used outside
of this function, it doesn't need to be in struct sx9500_data.
By always allocating enough space for the maximum number of channels,
we can avoid having to reallocate the buffer each time buffered reads
are enabled.
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
drivers/iio/proximity/sx9500.c | 24 +++---------------------
1 file changed, 3 insertions(+), 21 deletions(-)
diff --git a/drivers/iio/proximity/sx9500.c b/drivers/iio/proximity/sx9500.c
index 05844f17a15f6980ab7d55536e5fecfc5e4fe8c0..373e60b5c92fcd508af6420fff8067794d429a1c 100644
--- a/drivers/iio/proximity/sx9500.c
+++ b/drivers/iio/proximity/sx9500.c
@@ -88,7 +88,6 @@ struct sx9500_data {
bool prox_stat[SX9500_NUM_CHANNELS];
bool event_enabled[SX9500_NUM_CHANNELS];
bool trigger_enabled;
- u16 *buffer;
/* Remember enabled channels and sample rate during suspend. */
unsigned int suspend_ctrl0;
struct completion completion;
@@ -578,22 +577,6 @@ static int sx9500_write_event_config(struct iio_dev *indio_dev,
return ret;
}
-static int sx9500_update_scan_mode(struct iio_dev *indio_dev,
- const unsigned long *scan_mask)
-{
- struct sx9500_data *data = iio_priv(indio_dev);
-
- mutex_lock(&data->mutex);
- kfree(data->buffer);
- data->buffer = kzalloc(indio_dev->scan_bytes, GFP_KERNEL);
- mutex_unlock(&data->mutex);
-
- if (data->buffer == NULL)
- return -ENOMEM;
-
- return 0;
-}
-
static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
"2.500000 3.333333 5 6.666666 8.333333 11.111111 16.666666 33.333333");
@@ -612,7 +595,6 @@ static const struct iio_info sx9500_info = {
.write_raw = &sx9500_write_raw,
.read_event_config = &sx9500_read_event_config,
.write_event_config = &sx9500_write_event_config,
- .update_scan_mode = &sx9500_update_scan_mode,
};
static int sx9500_set_trigger_state(struct iio_trigger *trig,
@@ -645,6 +627,7 @@ static const struct iio_trigger_ops sx9500_trigger_ops = {
static irqreturn_t sx9500_trigger_handler(int irq, void *private)
{
+ IIO_DECLARE_BUFFER_WITH_TS(u16, buffer, SX9500_NUM_CHANNELS);
struct iio_poll_func *pf = private;
struct iio_dev *indio_dev = pf->indio_dev;
struct sx9500_data *data = iio_priv(indio_dev);
@@ -658,10 +641,10 @@ static irqreturn_t sx9500_trigger_handler(int irq, void *private)
if (ret < 0)
goto out;
- data->buffer[i++] = val;
+ buffer[i++] = val;
}
- iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
+ iio_push_to_buffers_with_timestamp(indio_dev, buffer,
iio_get_time_ns(indio_dev));
out:
@@ -984,7 +967,6 @@ static void sx9500_remove(struct i2c_client *client)
iio_triggered_buffer_cleanup(indio_dev);
if (client->irq > 0)
iio_trigger_unregister(data->trig);
- kfree(data->buffer);
}
static int sx9500_suspend(struct device *dev)
---
base-commit: f8f559752d573a051a984adda8d2d1464f92f954
change-id: 20250711-iio-use-more-iio_declare_buffer_with_ts-4-66ddcde563fe
Best regards,
--
David Lechner <dlechner@baylibre.com>On Fri, Jul 11, 2025 at 10:47:57AM -0500, David Lechner wrote: > Use IIO_DECLARE_BUFFER_WITH_TS() to declare a stack allocated buffer > in sx9500_trigger_handler(). Since the scan buffer isn't used outside > of this function, it doesn't need to be in struct sx9500_data. > > By always allocating enough space for the maximum number of channels, > we can avoid having to reallocate the buffer each time buffered reads > are enabled. Ag ood one! Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> -- With Best Regards, Andy Shevchenko
On Fri, 11 Jul 2025 19:42:25 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Fri, Jul 11, 2025 at 10:47:57AM -0500, David Lechner wrote: > > Use IIO_DECLARE_BUFFER_WITH_TS() to declare a stack allocated buffer > > in sx9500_trigger_handler(). Since the scan buffer isn't used outside > > of this function, it doesn't need to be in struct sx9500_data. > > > > By always allocating enough space for the maximum number of channels, > > we can avoid having to reallocate the buffer each time buffered reads > > are enabled. > > Ag ood one! > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Applied.
On Sun, 13 Jul 2025 14:55:33 +0100 Jonathan Cameron <jic23@kernel.org> wrote: > On Fri, 11 Jul 2025 19:42:25 +0300 > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > > On Fri, Jul 11, 2025 at 10:47:57AM -0500, David Lechner wrote: > > > Use IIO_DECLARE_BUFFER_WITH_TS() to declare a stack allocated buffer > > > in sx9500_trigger_handler(). Since the scan buffer isn't used outside > > > of this function, it doesn't need to be in struct sx9500_data. > > > > > > By always allocating enough space for the maximum number of channels, > > > we can avoid having to reallocate the buffer each time buffered reads > > > are enabled. > > > > Ag ood one! > > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > Applied. > Actually on second thoughts - why not a more descriptive structure? There are only a max of 4 channels and so the timestamp is always in the same location. Dropped for now. Jonathan
On 7/13/25 8:58 AM, Jonathan Cameron wrote: > On Sun, 13 Jul 2025 14:55:33 +0100 > Jonathan Cameron <jic23@kernel.org> wrote: > >> On Fri, 11 Jul 2025 19:42:25 +0300 >> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: >> >>> On Fri, Jul 11, 2025 at 10:47:57AM -0500, David Lechner wrote: >>>> Use IIO_DECLARE_BUFFER_WITH_TS() to declare a stack allocated buffer >>>> in sx9500_trigger_handler(). Since the scan buffer isn't used outside >>>> of this function, it doesn't need to be in struct sx9500_data. >>>> >>>> By always allocating enough space for the maximum number of channels, >>>> we can avoid having to reallocate the buffer each time buffered reads >>>> are enabled. >>> >>> Ag ood one! >>> >>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >>> >> >> Applied. >> > > Actually on second thoughts - why not a more descriptive structure? > There are only a max of 4 channels and so the timestamp is always > in the same location. > > Dropped for now. > > Jonathan I didn't do that on this one since a variable number of scan elements are used. But if you prefer structs for anything up to 8 bytes, we can go with that.
On Sun, 13 Jul 2025 11:47:56 -0500 David Lechner <dlechner@baylibre.com> wrote: > On 7/13/25 8:58 AM, Jonathan Cameron wrote: > > On Sun, 13 Jul 2025 14:55:33 +0100 > > Jonathan Cameron <jic23@kernel.org> wrote: > > > >> On Fri, 11 Jul 2025 19:42:25 +0300 > >> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > >> > >>> On Fri, Jul 11, 2025 at 10:47:57AM -0500, David Lechner wrote: > >>>> Use IIO_DECLARE_BUFFER_WITH_TS() to declare a stack allocated buffer > >>>> in sx9500_trigger_handler(). Since the scan buffer isn't used outside > >>>> of this function, it doesn't need to be in struct sx9500_data. > >>>> > >>>> By always allocating enough space for the maximum number of channels, > >>>> we can avoid having to reallocate the buffer each time buffered reads > >>>> are enabled. > >>> > >>> Ag ood one! > >>> > >>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > >>> > >> > >> Applied. > >> > > > > Actually on second thoughts - why not a more descriptive structure? > > There are only a max of 4 channels and so the timestamp is always > > in the same location. > > > > Dropped for now. > > > > Jonathan > > I didn't do that on this one since a variable number of scan > elements are used. But if you prefer structs for anything up > to 8 bytes, we can go with that. I think that makes sense as we don't really care 'what' is in the holes when fewer channels are enabled. Jonathan
© 2016 - 2025 Red Hat, Inc.