[v2,4/4] iio: pressure: hsc030pa add triggered buffer
Commit Message
Add triggered buffer feature.
Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1 -> v2 add Kconfig select for IIO_*BUFFER
a few changes based on Jonathan's review
drivers/iio/pressure/Kconfig | 2 ++
drivers/iio/pressure/hsc030pa.c | 47 +++++++++++++++++++++++++++++++++
drivers/iio/pressure/hsc030pa.h | 4 +++
3 files changed, 53 insertions(+)
Comments
On Sat, 27 Jan 2024 18:03:58 +0200
Petre Rodan <petre.rodan@subdimension.ro> wrote:
> Add triggered buffer feature.
>
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
Rest of series looks fine to me, but I'll leave it on list for
a few days at least for others to comment.
Thanks,
Jonathan
On Sat, 27 Jan 2024 16:59:48 +0000
Jonathan Cameron <jic23@kernel.org> wrote:
> On Sat, 27 Jan 2024 18:03:58 +0200
> Petre Rodan <petre.rodan@subdimension.ro> wrote:
>
> > Add triggered buffer feature.
> >
> > Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
>
> Rest of series looks fine to me, but I'll leave it on list for
> a few days at least for others to comment.
Applied to the togreg branch of iio.git and initially pushed out
as testing for 0-day to take a look-see at it!
Thanks,
Jonathan
>
> Thanks,
>
> Jonathan
>
>
Sat, Jan 27, 2024 at 06:03:58PM +0200, Petre Rodan kirjoitti:
> Add triggered buffer feature.
..
> +static irqreturn_t hsc_trigger_handler(int irq, void *private)
> +{
> + struct iio_poll_func *pf = private;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct hsc_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + ret = hsc_get_measurement(data);
> + if (ret)
> + goto error;
> + memcpy(&data->scan.chan[0], &data->buffer, 2);
You probably wanted here &data->buffer[0] as currently you use pointer to
the poiner.
> + memcpy(&data->scan.chan[1], &data->buffer[2], 2);
Hmm... We don't have fixed-size memcpy() :-(
> + iio_push_to_buffers_with_timestamp(indio_dev, &data->scan,
> + iio_get_time_ns(indio_dev));
> +
> +error:
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> +}
On Sun, 4 Feb 2024 13:44:15 +0000
Jonathan Cameron <jic23@kernel.org> wrote:
> On Sat, 27 Jan 2024 16:59:48 +0000
> Jonathan Cameron <jic23@kernel.org> wrote:
>
> > On Sat, 27 Jan 2024 18:03:58 +0200
> > Petre Rodan <petre.rodan@subdimension.ro> wrote:
> >
> > > Add triggered buffer feature.
> > >
> > > Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> >
> > Rest of series looks fine to me, but I'll leave it on list for
> > a few days at least for others to comment.
>
> Applied to the togreg branch of iio.git and initially pushed out
> as testing for 0-day to take a look-see at it!
Dropped as comments came in from Andy.
>
> Thanks,
>
> Jonathan
>
> >
> > Thanks,
> >
> > Jonathan
> >
> >
>
>
hello Andy,
On Sun, Feb 04, 2024 at 06:03:28PM +0200, andy.shevchenko@gmail.com wrote:
[..]
> > + memcpy(&data->scan.chan[1], &data->buffer[2], 2);
>
> Hmm... We don't have fixed-size memcpy() :-(
__be16 *ptr;
ptr = (__be16 *) data->buffer;
data->scan.chan[0] = *ptr;
data->scan.chan[1] = *++ptr;
is this an acceptable replacement? I do not understand that your concern was, my
intent was to copy exactly 2 bytes over.
> > + iio_push_to_buffers_with_timestamp(indio_dev, &data->scan,
> > + iio_get_time_ns(indio_dev));
thanks,
peter
On Mon, 5 Feb 2024 18:39:02 +0200
Petre Rodan <petre.rodan@subdimension.ro> wrote:
> hello Andy,
>
> On Sun, Feb 04, 2024 at 06:03:28PM +0200, andy.shevchenko@gmail.com wrote:
> [..]
> > > + memcpy(&data->scan.chan[1], &data->buffer[2], 2);
> >
> > Hmm... We don't have fixed-size memcpy() :-(
>
> __be16 *ptr;
>
> ptr = (__be16 *) data->buffer;
> data->scan.chan[0] = *ptr;
> data->scan.chan[1] = *++ptr;
>
> is this an acceptable replacement? I do not understand that your concern was, my
> intent was to copy exactly 2 bytes over.
Andy?
I'm not sure what you meant here either.
There is an existing oddity that the read_raw deals with this as a be32 and
masking out the right sections for each channel rather than perhaps more logical
be16 pair here.
Jonathan
>
> > > + iio_push_to_buffers_with_timestamp(indio_dev, &data->scan,
> > > + iio_get_time_ns(indio_dev));
>
> thanks,
> peter
On Sat, Feb 10, 2024 at 5:09 PM Jonathan Cameron <jic23@kernel.org> wrote:
> On Mon, 5 Feb 2024 18:39:02 +0200
> Petre Rodan <petre.rodan@subdimension.ro> wrote:
> > On Sun, Feb 04, 2024 at 06:03:28PM +0200, andy.shevchenko@gmail.com wrote:
> > > > + memcpy(&data->scan.chan[1], &data->buffer[2], 2);
> > >
> > > Hmm... We don't have fixed-size memcpy() :-(
> >
> > __be16 *ptr;
> >
> > ptr = (__be16 *) data->buffer;
> > data->scan.chan[0] = *ptr;
> > data->scan.chan[1] = *++ptr;
> >
> > is this an acceptable replacement? I do not understand that your concern was, my
> > intent was to copy exactly 2 bytes over.
>
> Andy?
>
> I'm not sure what you meant here either.
It was just a rhetorical remark, no AR implied. I.o.w. the current code is okay.
> There is an existing oddity that the read_raw deals with this as a be32 and
> masking out the right sections for each channel rather than perhaps more logical
> be16 pair here.
@@ -114,6 +114,8 @@ config HSC030PA
depends on (I2C || SPI_MASTER)
select HSC030PA_I2C if I2C
select HSC030PA_SPI if SPI_MASTER
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
help
Say Y here to build support for the Honeywell TruStability
HSC and SSC pressure and temperature sensor series.
@@ -22,8 +22,11 @@
#include <linux/types.h>
#include <linux/units.h>
+#include <linux/iio/buffer.h>
#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
#include <asm/unaligned.h>
@@ -297,6 +300,29 @@ static int hsc_get_measurement(struct hsc_data *data)
return 0;
}
+static irqreturn_t hsc_trigger_handler(int irq, void *private)
+{
+ struct iio_poll_func *pf = private;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct hsc_data *data = iio_priv(indio_dev);
+ int ret;
+
+ ret = hsc_get_measurement(data);
+ if (ret)
+ goto error;
+
+ memcpy(&data->scan.chan[0], &data->buffer, 2);
+ memcpy(&data->scan.chan[1], &data->buffer[2], 2);
+
+ iio_push_to_buffers_with_timestamp(indio_dev, &data->scan,
+ iio_get_time_ns(indio_dev));
+
+error:
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
/*
* IIO ABI expects
* value = (conv + offset) * scale
@@ -382,13 +408,29 @@ static const struct iio_chan_spec hsc_channels[] = {
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
BIT(IIO_CHAN_INFO_SCALE) |
BIT(IIO_CHAN_INFO_OFFSET),
+ .scan_index = 0,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 14,
+ .storagebits = 16,
+ .endianness = IIO_BE,
+ },
},
{
.type = IIO_TEMP,
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
BIT(IIO_CHAN_INFO_SCALE) |
BIT(IIO_CHAN_INFO_OFFSET),
+ .scan_index = 1,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 11,
+ .storagebits = 16,
+ .shift = 5,
+ .endianness = IIO_BE,
+ },
},
+ IIO_CHAN_SOFT_TIMESTAMP(2),
};
static const struct iio_info hsc_info = {
@@ -485,6 +527,11 @@ int hsc_common_probe(struct device *dev, hsc_recv_fn recv)
indio_dev->channels = hsc->chip->channels;
indio_dev->num_channels = hsc->chip->num_channels;
+ ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
+ hsc_trigger_handler, NULL);
+ if (ret)
+ return ret;
+
return devm_iio_device_register(dev, indio_dev);
}
EXPORT_SYMBOL_NS(hsc_common_probe, IIO_HONEYWELL_HSC030PA);
@@ -56,6 +56,10 @@ struct hsc_data {
s32 p_scale_dec;
s64 p_offset;
s32 p_offset_dec;
+ struct {
+ __be16 chan[2];
+ s64 timestamp __aligned(8);
+ } scan;
u8 buffer[HSC_REG_MEASUREMENT_RD_SIZE] __aligned(IIO_DMA_MINALIGN);
};