| Message ID | 20240127160405.19696-5-petre.rodan@subdimension.ro (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | iio: pressure: hsc030pa: cleanup and triggered buffer | expand |
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.
diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig index 5da7931dc537..3ad38506028e 100644 --- a/drivers/iio/pressure/Kconfig +++ b/drivers/iio/pressure/Kconfig @@ -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. diff --git a/drivers/iio/pressure/hsc030pa.c b/drivers/iio/pressure/hsc030pa.c index 7e3f74d53b47..f8bddcdf5056 100644 --- a/drivers/iio/pressure/hsc030pa.c +++ b/drivers/iio/pressure/hsc030pa.c @@ -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); diff --git a/drivers/iio/pressure/hsc030pa.h b/drivers/iio/pressure/hsc030pa.h index 56dc8e88194b..9b40f46f575f 100644 --- a/drivers/iio/pressure/hsc030pa.h +++ b/drivers/iio/pressure/hsc030pa.h @@ -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); };
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(+)