2024-01-27 16:04:36

by Petre Rodan

[permalink] [raw]
Subject: [PATCH v2 0/4] iio: pressure: hsc030pa: cleanup and triggered buffer

This set of patches covers the following:

- small cleanup
- mandatory 2ms delay between readings
- support for triggered buffer readings

The support for devices that have "sleep mode" factory option that was
present in v1 of this patchset was removed, for a few reasons:

- a Honeywell employee told me that this chip variant is extremely
unlikely to be found in the wild, which also makes testing the
driver functionality impossible
- I found no reliable way of generating i2c/spi bus traffic with no
payload (toggle CS for SPI case, send i2c packet containing only
the address byte) that are required for the wakeup sequence.


Petre Rodan (4):
dt-bindings: iio: pressure: honeywell,hsc030pa.yaml add spi props
iio: pressure: hsc030pa cleanup
iio: pressure: hsc030pa add mandatory delay
iio: pressure: hsc030pa add triggered buffer

.../iio/pressure/honeywell,hsc030pa.yaml | 3 ++
drivers/iio/pressure/Kconfig | 2 +
drivers/iio/pressure/hsc030pa.c | 49 ++++++++++++++++++-
drivers/iio/pressure/hsc030pa.h | 7 +++
drivers/iio/pressure/hsc030pa_i2c.c | 9 +++-
drivers/iio/pressure/hsc030pa_spi.c | 7 ++-
6 files changed, 73 insertions(+), 4 deletions(-)

--
2.43.0



2024-01-27 16:04:44

by Petre Rodan

[permalink] [raw]
Subject: [PATCH v2 1/4] dt-bindings: iio: pressure: honeywell,hsc030pa.yaml add spi props

Add spi-peripheral-props.yaml requirement needed by the
spi-max-frequency property.

Signed-off-by: Petre Rodan <[email protected]>
---
v2 -> v1 change the commit message based on Krzysztof's request
.../devicetree/bindings/iio/pressure/honeywell,hsc030pa.yaml | 3 +++
1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/pressure/honeywell,hsc030pa.yaml b/Documentation/devicetree/bindings/iio/pressure/honeywell,hsc030pa.yaml
index 65a24ed67b3c..89977b9f01cf 100644
--- a/Documentation/devicetree/bindings/iio/pressure/honeywell,hsc030pa.yaml
+++ b/Documentation/devicetree/bindings/iio/pressure/honeywell,hsc030pa.yaml
@@ -99,6 +99,9 @@ required:
- honeywell,transfer-function
- honeywell,pressure-triplet

+allOf:
+ - $ref: /schemas/spi/spi-peripheral-props.yaml
+
additionalProperties: false

dependentSchemas:
--
2.43.0


2024-01-27 16:04:47

by Petre Rodan

[permalink] [raw]
Subject: [PATCH v2 3/4] iio: pressure: hsc030pa add mandatory delay

Add a mandatory 2ms delay between consecutive chip reads.

I found a new Technical Note pdf that specifies that the measurement cycle
in these chips takes around 1.26ms. By adding this 2ms delay we make sure
that we never get stale measurements.

For more details, please see "Figure 1" in the pdf below:

https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/common/documents/sps-siot-sleep-mode-technical-note-008286-1-en-ciid-155793.pdf

Signed-off-by: Petre Rodan <[email protected]>
---
v1 -> v2 no change
drivers/iio/pressure/hsc030pa.h | 1 +
drivers/iio/pressure/hsc030pa_i2c.c | 3 +++
drivers/iio/pressure/hsc030pa_spi.c | 2 ++
3 files changed, 6 insertions(+)

diff --git a/drivers/iio/pressure/hsc030pa.h b/drivers/iio/pressure/hsc030pa.h
index f1079a70799f..56dc8e88194b 100644
--- a/drivers/iio/pressure/hsc030pa.h
+++ b/drivers/iio/pressure/hsc030pa.h
@@ -13,6 +13,7 @@
#include <linux/iio/iio.h>

#define HSC_REG_MEASUREMENT_RD_SIZE 4
+#define HSC_RESP_TIME_MS 2

struct device;

diff --git a/drivers/iio/pressure/hsc030pa_i2c.c b/drivers/iio/pressure/hsc030pa_i2c.c
index b5810bafef40..b3fd230e71da 100644
--- a/drivers/iio/pressure/hsc030pa_i2c.c
+++ b/drivers/iio/pressure/hsc030pa_i2c.c
@@ -8,6 +8,7 @@
* Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/common/documents/sps-siot-sleep-mode-technical-note-008286-1-en-ciid-155793.pdf
*/

+#include <linux/delay.h>
#include <linux/device.h>
#include <linux/errno.h>
#include <linux/i2c.h>
@@ -25,6 +26,8 @@ static int hsc_i2c_recv(struct hsc_data *data)
struct i2c_msg msg;
int ret;

+ msleep_interruptible(HSC_RESP_TIME_MS);
+
msg.addr = client->addr;
msg.flags = client->flags | I2C_M_RD;
msg.len = HSC_REG_MEASUREMENT_RD_SIZE;
diff --git a/drivers/iio/pressure/hsc030pa_spi.c b/drivers/iio/pressure/hsc030pa_spi.c
index 8d3441f1dcf9..818fa6303454 100644
--- a/drivers/iio/pressure/hsc030pa_spi.c
+++ b/drivers/iio/pressure/hsc030pa_spi.c
@@ -8,6 +8,7 @@
* Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/common/documents/sps-siot-sleep-mode-technical-note-008286-1-en-ciid-155793.pdf
*/

+#include <linux/delay.h>
#include <linux/device.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
@@ -28,6 +29,7 @@ static int hsc_spi_recv(struct hsc_data *data)
.len = HSC_REG_MEASUREMENT_RD_SIZE,
};

+ msleep_interruptible(HSC_RESP_TIME_MS);
return spi_sync_transfer(spi, &xfer, 1);
}

--
2.43.0


2024-01-27 16:04:48

by Petre Rodan

[permalink] [raw]
Subject: [PATCH v2 2/4] iio: pressure: hsc030pa cleanup

Use signed type to variable holding the result given by div_s64().

Add includes based on prior reviews from Andy.

Provide bus-specific technical datasheet in the _i2c.c _spi.c headers
instead of the generic one.

Signed-off-by: Petre Rodan <[email protected]>
---
v1 -> v2 no change
drivers/iio/pressure/hsc030pa.c | 2 +-
drivers/iio/pressure/hsc030pa.h | 2 ++
drivers/iio/pressure/hsc030pa_i2c.c | 6 ++++--
drivers/iio/pressure/hsc030pa_spi.c | 5 ++++-
4 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/pressure/hsc030pa.c b/drivers/iio/pressure/hsc030pa.c
index d6a51f0c335f..7e3f74d53b47 100644
--- a/drivers/iio/pressure/hsc030pa.c
+++ b/drivers/iio/pressure/hsc030pa.c
@@ -406,7 +406,7 @@ int hsc_common_probe(struct device *dev, hsc_recv_fn recv)
struct hsc_data *hsc;
struct iio_dev *indio_dev;
const char *triplet;
- u64 tmp;
+ s64 tmp;
int ret;

indio_dev = devm_iio_device_alloc(dev, sizeof(*hsc));
diff --git a/drivers/iio/pressure/hsc030pa.h b/drivers/iio/pressure/hsc030pa.h
index d20420dba4f6..f1079a70799f 100644
--- a/drivers/iio/pressure/hsc030pa.h
+++ b/drivers/iio/pressure/hsc030pa.h
@@ -10,6 +10,8 @@

#include <linux/types.h>

+#include <linux/iio/iio.h>
+
#define HSC_REG_MEASUREMENT_RD_SIZE 4

struct device;
diff --git a/drivers/iio/pressure/hsc030pa_i2c.c b/drivers/iio/pressure/hsc030pa_i2c.c
index e2b524b36417..b5810bafef40 100644
--- a/drivers/iio/pressure/hsc030pa_i2c.c
+++ b/drivers/iio/pressure/hsc030pa_i2c.c
@@ -4,14 +4,16 @@
*
* Copyright (c) 2023 Petre Rodan <[email protected]>
*
- * Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/board-mount-pressure-sensors/trustability-hsc-series/documents/sps-siot-trustability-hsc-series-high-accuracy-board-mount-pressure-sensors-50099148-a-en-ciid-151133.pdf [hsc]
- * Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/board-mount-pressure-sensors/common/documents/sps-siot-i2c-comms-digital-output-pressure-sensors-tn-008201-3-en-ciid-45841.pdf [i2c related]
+ * Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/board-mount-pressure-sensors/common/documents/sps-siot-i2c-comms-digital-output-pressure-sensors-tn-008201-3-en-ciid-45841.pdf
+ * Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/common/documents/sps-siot-sleep-mode-technical-note-008286-1-en-ciid-155793.pdf
*/

+#include <linux/device.h>
#include <linux/errno.h>
#include <linux/i2c.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
+#include <linux/types.h>

#include <linux/iio/iio.h>

diff --git a/drivers/iio/pressure/hsc030pa_spi.c b/drivers/iio/pressure/hsc030pa_spi.c
index a719bade8326..8d3441f1dcf9 100644
--- a/drivers/iio/pressure/hsc030pa_spi.c
+++ b/drivers/iio/pressure/hsc030pa_spi.c
@@ -4,13 +4,16 @@
*
* Copyright (c) 2023 Petre Rodan <[email protected]>
*
- * Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/board-mount-pressure-sensors/trustability-hsc-series/documents/sps-siot-trustability-hsc-series-high-accuracy-board-mount-pressure-sensors-50099148-a-en-ciid-151133.pdf
+ * Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/common/documents/sps-siot-spi-comms-digital-ouptu-pressure-sensors-tn-008202-3-en-ciid-45843.pdf
+ * Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/common/documents/sps-siot-sleep-mode-technical-note-008286-1-en-ciid-155793.pdf
*/

+#include <linux/device.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/spi/spi.h>
#include <linux/stddef.h>
+#include <linux/types.h>

#include <linux/iio/iio.h>

--
2.43.0


2024-01-27 16:06:49

by Petre Rodan

[permalink] [raw]
Subject: [PATCH v2 4/4] iio: pressure: hsc030pa add triggered buffer

Add triggered buffer feature.

Signed-off-by: Petre Rodan <[email protected]>
---
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(+)

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);
};

--
2.43.0


2024-01-27 16:58:06

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] iio: pressure: hsc030pa cleanup

On Sat, 27 Jan 2024 18:03:56 +0200
Petre Rodan <[email protected]> wrote:

> Use signed type to variable holding the result given by div_s64().
>
> Add includes based on prior reviews from Andy.
>
> Provide bus-specific technical datasheet in the _i2c.c _spi.c headers
> instead of the generic one.

Given this lists 3 things, should really be 3 patches...
They are small though so meh, if nothing else comes up to require
a v3 I might take it in the interests of expediency.

>
> Signed-off-by: Petre Rodan <[email protected]>
> ---
> v1 -> v2 no change
> drivers/iio/pressure/hsc030pa.c | 2 +-
> drivers/iio/pressure/hsc030pa.h | 2 ++
> drivers/iio/pressure/hsc030pa_i2c.c | 6 ++++--
> drivers/iio/pressure/hsc030pa_spi.c | 5 ++++-
> 4 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/pressure/hsc030pa.c b/drivers/iio/pressure/hsc030pa.c
> index d6a51f0c335f..7e3f74d53b47 100644
> --- a/drivers/iio/pressure/hsc030pa.c
> +++ b/drivers/iio/pressure/hsc030pa.c
> @@ -406,7 +406,7 @@ int hsc_common_probe(struct device *dev, hsc_recv_fn recv)
> struct hsc_data *hsc;
> struct iio_dev *indio_dev;
> const char *triplet;
> - u64 tmp;
> + s64 tmp;
> int ret;
>
> indio_dev = devm_iio_device_alloc(dev, sizeof(*hsc));
> diff --git a/drivers/iio/pressure/hsc030pa.h b/drivers/iio/pressure/hsc030pa.h
> index d20420dba4f6..f1079a70799f 100644
> --- a/drivers/iio/pressure/hsc030pa.h
> +++ b/drivers/iio/pressure/hsc030pa.h
> @@ -10,6 +10,8 @@
>
> #include <linux/types.h>
>
> +#include <linux/iio/iio.h>
> +
> #define HSC_REG_MEASUREMENT_RD_SIZE 4
>
> struct device;
> diff --git a/drivers/iio/pressure/hsc030pa_i2c.c b/drivers/iio/pressure/hsc030pa_i2c.c
> index e2b524b36417..b5810bafef40 100644
> --- a/drivers/iio/pressure/hsc030pa_i2c.c
> +++ b/drivers/iio/pressure/hsc030pa_i2c.c
> @@ -4,14 +4,16 @@
> *
> * Copyright (c) 2023 Petre Rodan <[email protected]>
> *
> - * Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/board-mount-pressure-sensors/trustability-hsc-series/documents/sps-siot-trustability-hsc-series-high-accuracy-board-mount-pressure-sensors-50099148-a-en-ciid-151133.pdf [hsc]
> - * Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/board-mount-pressure-sensors/common/documents/sps-siot-i2c-comms-digital-output-pressure-sensors-tn-008201-3-en-ciid-45841.pdf [i2c related]
> + * Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/board-mount-pressure-sensors/common/documents/sps-siot-i2c-comms-digital-output-pressure-sensors-tn-008201-3-en-ciid-45841.pdf
> + * Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/common/documents/sps-siot-sleep-mode-technical-note-008286-1-en-ciid-155793.pdf
> */
>
> +#include <linux/device.h>
> #include <linux/errno.h>
> #include <linux/i2c.h>
> #include <linux/mod_devicetable.h>
> #include <linux/module.h>
> +#include <linux/types.h>
>
> #include <linux/iio/iio.h>
>
> diff --git a/drivers/iio/pressure/hsc030pa_spi.c b/drivers/iio/pressure/hsc030pa_spi.c
> index a719bade8326..8d3441f1dcf9 100644
> --- a/drivers/iio/pressure/hsc030pa_spi.c
> +++ b/drivers/iio/pressure/hsc030pa_spi.c
> @@ -4,13 +4,16 @@
> *
> * Copyright (c) 2023 Petre Rodan <[email protected]>
> *
> - * Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/board-mount-pressure-sensors/trustability-hsc-series/documents/sps-siot-trustability-hsc-series-high-accuracy-board-mount-pressure-sensors-50099148-a-en-ciid-151133.pdf
> + * Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/common/documents/sps-siot-spi-comms-digital-ouptu-pressure-sensors-tn-008202-3-en-ciid-45843.pdf
> + * Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/common/documents/sps-siot-sleep-mode-technical-note-008286-1-en-ciid-155793.pdf
> */
>
> +#include <linux/device.h>
> #include <linux/mod_devicetable.h>
> #include <linux/module.h>
> #include <linux/spi/spi.h>
> #include <linux/stddef.h>
> +#include <linux/types.h>
>
> #include <linux/iio/iio.h>
>


2024-01-27 17:00:09

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] iio: pressure: hsc030pa add triggered buffer

On Sat, 27 Jan 2024 18:03:58 +0200
Petre Rodan <[email protected]> wrote:

> Add triggered buffer feature.
>
> Signed-off-by: Petre Rodan <[email protected]>

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


2024-01-29 08:23:53

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] dt-bindings: iio: pressure: honeywell,hsc030pa.yaml add spi props

On 27/01/2024 17:03, Petre Rodan wrote:
> Add spi-peripheral-props.yaml requirement needed by the
> spi-max-frequency property.
>
> Signed-off-by: Petre Rodan <[email protected]>
> ---

Reviewed-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof


2024-02-04 13:44:35

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] iio: pressure: hsc030pa add triggered buffer

On Sat, 27 Jan 2024 16:59:48 +0000
Jonathan Cameron <[email protected]> wrote:

> On Sat, 27 Jan 2024 18:03:58 +0200
> Petre Rodan <[email protected]> wrote:
>
> > Add triggered buffer feature.
> >
> > Signed-off-by: Petre Rodan <[email protected]>
>
> 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
>
>


2024-02-04 16:04:47

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] iio: pressure: hsc030pa add triggered buffer

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;
> +}

--
With Best Regards,
Andy Shevchenko



2024-02-04 16:11:15

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] iio: pressure: hsc030pa add triggered buffer

On Sun, 4 Feb 2024 13:44:15 +0000
Jonathan Cameron <[email protected]> wrote:

> On Sat, 27 Jan 2024 16:59:48 +0000
> Jonathan Cameron <[email protected]> wrote:
>
> > On Sat, 27 Jan 2024 18:03:58 +0200
> > Petre Rodan <[email protected]> wrote:
> >
> > > Add triggered buffer feature.
> > >
> > > Signed-off-by: Petre Rodan <[email protected]>
> >
> > 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
> >
> >
>
>


2024-02-05 16:48:32

by Petre Rodan

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] iio: pressure: hsc030pa add triggered buffer


hello Andy,

On Sun, Feb 04, 2024 at 06:03:28PM +0200, [email protected] 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


Attachments:
(No filename) (575.00 B)
signature.asc (849.00 B)
Download all attachments

2024-02-10 15:10:43

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] iio: pressure: hsc030pa add triggered buffer

On Mon, 5 Feb 2024 18:39:02 +0200
Petre Rodan <[email protected]> wrote:

> hello Andy,
>
> On Sun, Feb 04, 2024 at 06:03:28PM +0200, [email protected] 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


2024-02-10 15:13:06

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] iio: pressure: hsc030pa add triggered buffer

On Sat, Feb 10, 2024 at 5:09 PM Jonathan Cameron <[email protected]> wrote:
> On Mon, 5 Feb 2024 18:39:02 +0200
> Petre Rodan <[email protected]> wrote:
> > On Sun, Feb 04, 2024 at 06:03:28PM +0200, [email protected] 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.


--
With Best Regards,
Andy Shevchenko