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

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

[Petre Rodan]

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

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

[Petre Rodan]

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

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
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

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

[Krzysztof Kozlowski]

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 <petre.rodan@subdimension.ro>
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

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

[Petre Rodan]

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 <petre.rodan@subdimension.ro>
---
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 <petre.rodan@subdimension.ro>
  *
- * 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 <petre.rodan@subdimension.ro>
  *
- * 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

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

[Jonathan Cameron]

On Sat, 27 Jan 2024 18:03:56 +0200
Petre Rodan <petre.rodan@subdimension.ro> 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 <petre.rodan@subdimension.ro>
> ---
> 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 <petre.rodan@subdimension.ro>
>   *
> - * 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 <petre.rodan@subdimension.ro>
>   *
> - * 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>
>  

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

[Petre Rodan]

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 <petre.rodan@subdimension.ro>
---
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

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

[Petre Rodan]

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(+)

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

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

[Jonathan Cameron]

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

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

[Jonathan Cameron]

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
> 
> 

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

[Jonathan Cameron]

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
> > 
> >   
> 
> 

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

[andy.shevchenko]

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

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

[Petre Rodan]

[-- Attachment #1: Type: text/plain, Size: 575 bytes --]


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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Jonathan Cameron]

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

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

[Andy Shevchenko]

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.


-- 
With Best Regards,
Andy Shevchenko