Add driver for Aosong adp810 differential pressure and temperature sensor.
This sensor provides an I2C interface for reading data.
Calculate CRC of the data received using standard crc8 library to verify
data integrity.
Tested on TI am62x sk board with sensor connected at i2c-2.
Signed-off-by: Akhilesh Patil <akhilesh@ee.iitb.ac.in>
---
MAINTAINERS | 7 ++
drivers/iio/pressure/Kconfig | 12 ++
drivers/iio/pressure/Makefile | 8 +-
drivers/iio/pressure/adp810.c | 222 ++++++++++++++++++++++++++++++++++
4 files changed, 245 insertions(+), 4 deletions(-)
create mode 100644 drivers/iio/pressure/adp810.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 545a4776795e..89c56c7dccb4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3745,6 +3745,13 @@ S: Maintained
F: Documentation/devicetree/bindings/iio/chemical/aosong,ags02ma.yaml
F: drivers/iio/chemical/ags02ma.c
+AOSONG ADP810 DIFFERENTIAL PRESSURE SENSOR DRIVER
+M: Akhilesh Patil <akhilesh@ee.iitb.ac.in>
+L: linux-iio@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/iio/pressure/aosong,adp810.yaml
+F: drivers/iio/pressure/adp810.c
+
ASC7621 HARDWARE MONITOR DRIVER
M: George Joseph <george.joseph@fairview5.com>
L: linux-hwmon@vger.kernel.org
diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
index d2cb8c871f6a..2fe9dc90cceb 100644
--- a/drivers/iio/pressure/Kconfig
+++ b/drivers/iio/pressure/Kconfig
@@ -339,4 +339,16 @@ config ZPA2326_SPI
tristate
select REGMAP_SPI
+config ADP810
+ tristate "Aosong adp810 differential pressure and temperature sensor"
+ depends on I2C
+ select CRC8
+ help
+ Say yes here to build adp810 differential pressure and temperature
+ sensor driver. ADP810 can measure pressure range up to 500Pa.
+ It supports an I2C interface for data communication.
+
+ To compile this driver as a module, choose M here: the module will
+ be called adp810
+
endmenu
diff --git a/drivers/iio/pressure/Makefile b/drivers/iio/pressure/Makefile
index 6482288e07ee..a21443e992b9 100644
--- a/drivers/iio/pressure/Makefile
+++ b/drivers/iio/pressure/Makefile
@@ -5,6 +5,7 @@
# When adding new entries keep the list in alphabetical order
obj-$(CONFIG_ABP060MG) += abp060mg.o
+obj-$(CONFIG_ADP810) += adp810.o
obj-$(CONFIG_ROHM_BM1390) += rohm-bm1390.o
obj-$(CONFIG_BMP280) += bmp280.o
bmp280-objs := bmp280-core.o bmp280-regmap.o
@@ -15,6 +16,7 @@ obj-$(CONFIG_DPS310) += dps310.o
obj-$(CONFIG_IIO_CROS_EC_BARO) += cros_ec_baro.o
obj-$(CONFIG_HID_SENSOR_PRESS) += hid-sensor-press.o
obj-$(CONFIG_HP03) += hp03.o
+obj-$(CONFIG_HP206C) += hp206c.o
obj-$(CONFIG_HSC030PA) += hsc030pa.o
obj-$(CONFIG_HSC030PA_I2C) += hsc030pa_i2c.o
obj-$(CONFIG_HSC030PA_SPI) += hsc030pa_spi.o
@@ -34,11 +36,9 @@ obj-$(CONFIG_SDP500) += sdp500.o
obj-$(CONFIG_IIO_ST_PRESS) += st_pressure.o
st_pressure-y := st_pressure_core.o
st_pressure-$(CONFIG_IIO_BUFFER) += st_pressure_buffer.o
+obj-$(CONFIG_IIO_ST_PRESS_I2C) += st_pressure_i2c.o
+obj-$(CONFIG_IIO_ST_PRESS_SPI) += st_pressure_spi.o
obj-$(CONFIG_T5403) += t5403.o
-obj-$(CONFIG_HP206C) += hp206c.o
obj-$(CONFIG_ZPA2326) += zpa2326.o
obj-$(CONFIG_ZPA2326_I2C) += zpa2326_i2c.o
obj-$(CONFIG_ZPA2326_SPI) += zpa2326_spi.o
-
-obj-$(CONFIG_IIO_ST_PRESS_I2C) += st_pressure_i2c.o
-obj-$(CONFIG_IIO_ST_PRESS_SPI) += st_pressure_spi.o
diff --git a/drivers/iio/pressure/adp810.c b/drivers/iio/pressure/adp810.c
new file mode 100644
index 000000000000..5fcb0447c628
--- /dev/null
+++ b/drivers/iio/pressure/adp810.c
@@ -0,0 +1,222 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2025 Akhilesh Patil <akhilesh@ee.iitb.ac.in>
+ *
+ * Driver for adp810 pressure and temperature sensor
+ * Datasheet:
+ * https://aosong.com/userfiles/files/media/Datasheet%20ADP810-Digital.pdf
+ */
+
+#include <linux/cleanup.h>
+#include <linux/crc8.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/dev_printk.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+#include <linux/unaligned.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/types.h>
+
+/*
+ * Time taken in ms by sensor to do measurements after triggering.
+ * As per datasheet 10ms is sufficient but we define 30ms for better margin.
+ */
+#define ADP810_MEASURE_LATENCY_MS 30
+/* Trigger command to send to start measurement by the sensor */
+#define ADP810_TRIGGER_COMMAND 0x2d37
+/*
+ * Refer section 5.4 checksum calculation from datasheet.
+ * This sensor uses CRC polynomial x^8 + x^5 + x^4 + 1 (0x31)
+ */
+#define ADP810_CRC8_POLYNOMIAL 0x31
+
+DECLARE_CRC8_TABLE(crc_table);
+
+struct adp810_read_buf {
+ __be16 dp;
+ u8 dp_crc;
+ __be16 tmp;
+ u8 tmp_crc;
+ __be16 sf;
+ u8 sf_crc;
+} __packed;
+
+struct adp810_data {
+ struct i2c_client *client;
+ /* Use lock to synchronize access to device during read sequence */
+ struct mutex lock;
+};
+
+static int adp810_measure(struct adp810_data *data, struct adp810_read_buf *buf)
+{
+ struct i2c_client *client = data->client;
+ struct device *dev = &client->dev;
+ int ret;
+ u16 trig_cmd = ADP810_TRIGGER_COMMAND;
+
+ /* Send trigger to the sensor for measurement */
+ ret = i2c_master_send(client, (char *)&trig_cmd, sizeof(trig_cmd));
+ if (ret < 0) {
+ dev_err(dev, "Error sending trigger command\n");
+ return ret;
+ }
+ if (ret != sizeof(trig_cmd))
+ return -EIO;
+
+ /*
+ * Wait for the sensor to acquire data. As per datasheet section 5.3.1,
+ * wait for at least 10ms before reading measurements from the sensor.
+ */
+ msleep(ADP810_MEASURE_LATENCY_MS);
+
+ /* Read sensor values */
+ ret = i2c_master_recv(client, (char *)buf, sizeof(*buf));
+ if (ret < 0) {
+ dev_err(dev, "Error reading from sensor\n");
+ return ret;
+ }
+ if (ret != sizeof(*buf))
+ return -EIO;
+
+ /* CRC checks */
+ crc8_populate_msb(crc_table, ADP810_CRC8_POLYNOMIAL);
+ if (buf->dp_crc != crc8(crc_table, (u8 *)&buf->dp, 0x2, CRC8_INIT_VALUE)) {
+ dev_err(dev, "CRC error for pressure\n");
+ return -EIO;
+ }
+
+ if (buf->tmp_crc != crc8(crc_table, (u8 *)&buf->tmp, 0x2, CRC8_INIT_VALUE)) {
+ dev_err(dev, "CRC error for temperature\n");
+ return -EIO;
+ }
+
+ if (buf->sf_crc != crc8(crc_table, (u8 *)&buf->sf, 0x2, CRC8_INIT_VALUE)) {
+ dev_err(dev, "CRC error for scale\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int adp810_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct adp810_data *data = iio_priv(indio_dev);
+ struct device *dev = &data->client->dev;
+ struct adp810_read_buf buf = { };
+ int ret;
+
+ scoped_guard(mutex, &data->lock) {
+ ret = adp810_measure(data, &buf);
+ if (ret) {
+ dev_err(dev, "Failed to read from device\n");
+ return ret;
+ }
+ }
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ switch (chan->type) {
+ case IIO_PRESSURE:
+ *val = get_unaligned_be16(&buf.dp);
+ return IIO_VAL_INT;
+ case IIO_TEMP:
+ *val = get_unaligned_be16(&buf.tmp);
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+ case IIO_CHAN_INFO_SCALE:
+ switch (chan->type) {
+ case IIO_PRESSURE:
+ *val = get_unaligned_be16(&buf.sf);
+ return IIO_VAL_INT;
+ case IIO_TEMP:
+ *val = 200;
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_info adp810_info = {
+ .read_raw = adp810_read_raw,
+};
+
+static const struct iio_chan_spec adp810_channels[] = {
+ {
+ .type = IIO_PRESSURE,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+ },
+ {
+ .type = IIO_TEMP,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+ },
+};
+
+static int adp810_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct iio_dev *indio_dev;
+ struct adp810_data *data;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ data = iio_priv(indio_dev);
+ data->client = client;
+
+ ret = devm_mutex_init(dev, &data->lock);
+ if (ret)
+ return ret;
+
+ indio_dev->name = "adp810";
+ indio_dev->channels = adp810_channels;
+ indio_dev->num_channels = ARRAY_SIZE(adp810_channels);
+ indio_dev->info = &adp810_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ ret = devm_iio_device_register(dev, indio_dev);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to register IIO device\n");
+
+ return 0;
+}
+
+static const struct i2c_device_id adp810_id_table[] = {
+ { "adp810" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, adp810_id_table);
+
+static const struct of_device_id adp810_of_table[] = {
+ { .compatible = "aosong,adp810" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, adp810_of_table);
+
+static struct i2c_driver adp810_driver = {
+ .driver = {
+ .name = "adp810",
+ .of_match_table = adp810_of_table,
+ },
+ .probe = adp810_probe,
+ .id_table = adp810_id_table,
+};
+module_i2c_driver(adp810_driver);
+
+MODULE_AUTHOR("Akhilesh Patil <akhilesh@ee.iitb.ac.in>");
+MODULE_DESCRIPTION("Driver for Aosong ADP810 sensor");
+MODULE_LICENSE("GPL");
--
2.34.1On Tue, 21 Oct 2025 11:20:30 +0530
Akhilesh Patil <akhilesh@ee.iitb.ac.in> wrote:
> Add driver for Aosong adp810 differential pressure and temperature sensor.
> This sensor provides an I2C interface for reading data.
> Calculate CRC of the data received using standard crc8 library to verify
> data integrity.
>
> Tested on TI am62x sk board with sensor connected at i2c-2.
>
> Signed-off-by: Akhilesh Patil <akhilesh@ee.iitb.ac.in>
A couple of trivial bits of follow up from me given you are going to be
doing a v4 for the stuff Andy picked up on. Otherwise I'd just have
tweaked it whilst applying for these two.
> diff --git a/drivers/iio/pressure/adp810.c b/drivers/iio/pressure/adp810.c
> new file mode 100644
> index 000000000000..5fcb0447c628
> --- /dev/null
> +++ b/drivers/iio/pressure/adp810.c
> +/*
> + * Time taken in ms by sensor to do measurements after triggering.
> + * As per datasheet 10ms is sufficient but we define 30ms for better margin.
> + */
> +#define ADP810_MEASURE_LATENCY_MS 30
I'd just put this value in the one place that it is used and combine the two
comments on why it has this particular value.
30ms seems like a bit over the top for handling silicon variation etc.
Any background for the large margin? If you've seen this as necessary
in practice then just state that - it's useful info to have available
to a future reader of the driver.
> +static const struct iio_info adp810_info = {
> + .read_raw = adp810_read_raw,
Trivial but there is no benefit in using a tab before the =
In general aligning this stuff isn't a good plan. It causes
a mass of churn in the long run as some of the callbacks have longer
names and suddenly whole thing needs reindenting.
> +};
On Thu, Oct 23, 2025 at 06:34:17PM +0100, Jonathan Cameron wrote:
> On Tue, 21 Oct 2025 11:20:30 +0530
> Akhilesh Patil <akhilesh@ee.iitb.ac.in> wrote:
>
> > Add driver for Aosong adp810 differential pressure and temperature sensor.
> > This sensor provides an I2C interface for reading data.
> > Calculate CRC of the data received using standard crc8 library to verify
> > data integrity.
> >
> > Tested on TI am62x sk board with sensor connected at i2c-2.
> >
> > Signed-off-by: Akhilesh Patil <akhilesh@ee.iitb.ac.in>
>
> A couple of trivial bits of follow up from me given you are going to be
> doing a v4 for the stuff Andy picked up on. Otherwise I'd just have
> tweaked it whilst applying for these two.
Sure.
>
> > diff --git a/drivers/iio/pressure/adp810.c b/drivers/iio/pressure/adp810.c
> > new file mode 100644
> > index 000000000000..5fcb0447c628
> > --- /dev/null
> > +++ b/drivers/iio/pressure/adp810.c
>
> > +/*
> > + * Time taken in ms by sensor to do measurements after triggering.
> > + * As per datasheet 10ms is sufficient but we define 30ms for better margin.
> > + */
> > +#define ADP810_MEASURE_LATENCY_MS 30
> I'd just put this value in the one place that it is used and combine the two
> comments on why it has this particular value.
Okay. Removed this macro and used value directly along with
improved comment.
>
> 30ms seems like a bit over the top for handling silicon variation etc.
> Any background for the large margin? If you've seen this as necessary
> in practice then just state that - it's useful info to have available
> to a future reader of the driver.
Datasheet recommends value greater than 10ms.
30ms is what I started with for initial testing.
I have checked it working for even 10ms, 20ms etc.
Let me use 20ms here with margin over 10 and mention it
in the comment in code.
>
>
> > +static const struct iio_info adp810_info = {
> > + .read_raw = adp810_read_raw,
> Trivial but there is no benefit in using a tab before the =
>
> In general aligning this stuff isn't a good plan. It causes
> a mass of churn in the long run as some of the callbacks have longer
> names and suddenly whole thing needs reindenting.
okay, got it. Fixed.
Regards,
Akhilesh
On Tue, Oct 21, 2025 at 11:20:30AM +0530, Akhilesh Patil wrote:
> Add driver for Aosong adp810 differential pressure and temperature sensor.
> This sensor provides an I2C interface for reading data.
> Calculate CRC of the data received using standard crc8 library to verify
> data integrity.
Thanks for an update! Looks almost good to me, some comments below.
...
> +M: Akhilesh Patil <akhilesh@ee.iitb.ac.in>
> +L: linux-iio@vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/iio/pressure/aosong,adp810.yaml
Putting it here makes checkpatch unhappy. If someone thinks that is a false
positive of the tool, perhaps one needs to fix that.
> +F: drivers/iio/pressure/adp810.c
...
> # When adding new entries keep the list in alphabetical order
> obj-$(CONFIG_ABP060MG) += abp060mg.o
> +obj-$(CONFIG_ADP810) += adp810.o
> obj-$(CONFIG_ROHM_BM1390) += rohm-bm1390.o
> obj-$(CONFIG_BMP280) += bmp280.o
> bmp280-objs := bmp280-core.o bmp280-regmap.o
> @@ -15,6 +16,7 @@ obj-$(CONFIG_DPS310) += dps310.o
> obj-$(CONFIG_IIO_CROS_EC_BARO) += cros_ec_baro.o
> obj-$(CONFIG_HID_SENSOR_PRESS) += hid-sensor-press.o
> obj-$(CONFIG_HP03) += hp03.o
> +obj-$(CONFIG_HP206C) += hp206c.o
> obj-$(CONFIG_HSC030PA) += hsc030pa.o
> obj-$(CONFIG_HSC030PA_I2C) += hsc030pa_i2c.o
> obj-$(CONFIG_HSC030PA_SPI) += hsc030pa_spi.o
> @@ -34,11 +36,9 @@ obj-$(CONFIG_SDP500) += sdp500.o
> obj-$(CONFIG_IIO_ST_PRESS) += st_pressure.o
> st_pressure-y := st_pressure_core.o
> st_pressure-$(CONFIG_IIO_BUFFER) += st_pressure_buffer.o
> +obj-$(CONFIG_IIO_ST_PRESS_I2C) += st_pressure_i2c.o
> +obj-$(CONFIG_IIO_ST_PRESS_SPI) += st_pressure_spi.o
> obj-$(CONFIG_T5403) += t5403.o
> -obj-$(CONFIG_HP206C) += hp206c.o
> obj-$(CONFIG_ZPA2326) += zpa2326.o
> obj-$(CONFIG_ZPA2326_I2C) += zpa2326_i2c.o
> obj-$(CONFIG_ZPA2326_SPI) += zpa2326_spi.o
> -
> -obj-$(CONFIG_IIO_ST_PRESS_I2C) += st_pressure_i2c.o
> -obj-$(CONFIG_IIO_ST_PRESS_SPI) += st_pressure_spi.o
I would split order fix into a separate change, but if maintainers are okay
with this approach, I would not object.
...
> +#include <linux/cleanup.h>
> +#include <linux/crc8.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/dev_printk.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/mutex.h>
> +#include <linux/unaligned.h>
Something is still missing.
...
> +struct adp810_read_buf {
> + __be16 dp;
> + u8 dp_crc;
> + __be16 tmp;
> + u8 tmp_crc;
> + __be16 sf;
> + u8 sf_crc;
All these types are provided in types.h
> +} __packed;
...
> +static int adp810_measure(struct adp810_data *data, struct adp810_read_buf *buf)
> +{
> + struct i2c_client *client = data->client;
> + struct device *dev = &client->dev;
> + int ret;
> + u16 trig_cmd = ADP810_TRIGGER_COMMAND;
Shouldn't this be __be16 or __le16? Or is that really a full 16-bit command?
I have a gut feeling that this should be u8 x[2] = { ... }; instead.
> + /* Send trigger to the sensor for measurement */
> + ret = i2c_master_send(client, (char *)&trig_cmd, sizeof(trig_cmd));
> + if (ret < 0) {
> + dev_err(dev, "Error sending trigger command\n");
> + return ret;
> + }
> + if (ret != sizeof(trig_cmd))
> + return -EIO;
-EIO is defined down from linux/errno.h.
> + /*
> + * Wait for the sensor to acquire data. As per datasheet section 5.3.1,
> + * wait for at least 10ms before reading measurements from the sensor.
> + */
> + msleep(ADP810_MEASURE_LATENCY_MS);
> +
> + /* Read sensor values */
> + ret = i2c_master_recv(client, (char *)buf, sizeof(*buf));
> + if (ret < 0) {
> + dev_err(dev, "Error reading from sensor\n");
> + return ret;
> + }
> + if (ret != sizeof(*buf))
> + return -EIO;
> +
> + /* CRC checks */
> + crc8_populate_msb(crc_table, ADP810_CRC8_POLYNOMIAL);
> + if (buf->dp_crc != crc8(crc_table, (u8 *)&buf->dp, 0x2, CRC8_INIT_VALUE)) {
> + dev_err(dev, "CRC error for pressure\n");
> + return -EIO;
> + }
> +
> + if (buf->tmp_crc != crc8(crc_table, (u8 *)&buf->tmp, 0x2, CRC8_INIT_VALUE)) {
> + dev_err(dev, "CRC error for temperature\n");
> + return -EIO;
> + }
> +
> + if (buf->sf_crc != crc8(crc_table, (u8 *)&buf->sf, 0x2, CRC8_INIT_VALUE)) {
> + dev_err(dev, "CRC error for scale\n");
> + return -EIO;
> + }
> +
> + return 0;
> +}
...
> + indio_dev->num_channels = ARRAY_SIZE(adp810_channels);
ARRAY_SIZE() is defined in a specific header.
--
With Best Regards,
Andy Shevchenko
On Tue, Oct 21, 2025 at 06:20:52PM +0300, Andy Shevchenko wrote:
> On Tue, Oct 21, 2025 at 11:20:30AM +0530, Akhilesh Patil wrote:
> > Add driver for Aosong adp810 differential pressure and temperature sensor.
> > This sensor provides an I2C interface for reading data.
> > Calculate CRC of the data received using standard crc8 library to verify
> > data integrity.
>
> Thanks for an update! Looks almost good to me, some comments below.
Thanks for the below comments and suggestions.
Addressed them in v4.
Regards,
Akhilesh
>
> ...
>
> > +M: Akhilesh Patil <akhilesh@ee.iitb.ac.in>
> > +L: linux-iio@vger.kernel.org
> > +S: Maintained
> > +F: Documentation/devicetree/bindings/iio/pressure/aosong,adp810.yaml
>
> Putting it here makes checkpatch unhappy. If someone thinks that is a false
> positive of the tool, perhaps one needs to fix that.
This is being followed up in v1 thread.
https://lore.kernel.org/lkml/aPs6raLIcM3QbQXJ@smile.fi.intel.com/
>
> > +F: drivers/iio/pressure/adp810.c
>
> ...
>
> > # When adding new entries keep the list in alphabetical order
> > obj-$(CONFIG_ABP060MG) += abp060mg.o
> > +obj-$(CONFIG_ADP810) += adp810.o
> > obj-$(CONFIG_ROHM_BM1390) += rohm-bm1390.o
> > obj-$(CONFIG_BMP280) += bmp280.o
> > bmp280-objs := bmp280-core.o bmp280-regmap.o
> > @@ -15,6 +16,7 @@ obj-$(CONFIG_DPS310) += dps310.o
> > obj-$(CONFIG_IIO_CROS_EC_BARO) += cros_ec_baro.o
> > obj-$(CONFIG_HID_SENSOR_PRESS) += hid-sensor-press.o
> > obj-$(CONFIG_HP03) += hp03.o
> > +obj-$(CONFIG_HP206C) += hp206c.o
> > obj-$(CONFIG_HSC030PA) += hsc030pa.o
> > obj-$(CONFIG_HSC030PA_I2C) += hsc030pa_i2c.o
> > obj-$(CONFIG_HSC030PA_SPI) += hsc030pa_spi.o
> > @@ -34,11 +36,9 @@ obj-$(CONFIG_SDP500) += sdp500.o
> > obj-$(CONFIG_IIO_ST_PRESS) += st_pressure.o
> > st_pressure-y := st_pressure_core.o
> > st_pressure-$(CONFIG_IIO_BUFFER) += st_pressure_buffer.o
> > +obj-$(CONFIG_IIO_ST_PRESS_I2C) += st_pressure_i2c.o
> > +obj-$(CONFIG_IIO_ST_PRESS_SPI) += st_pressure_spi.o
> > obj-$(CONFIG_T5403) += t5403.o
> > -obj-$(CONFIG_HP206C) += hp206c.o
> > obj-$(CONFIG_ZPA2326) += zpa2326.o
> > obj-$(CONFIG_ZPA2326_I2C) += zpa2326_i2c.o
> > obj-$(CONFIG_ZPA2326_SPI) += zpa2326_spi.o
> > -
> > -obj-$(CONFIG_IIO_ST_PRESS_I2C) += st_pressure_i2c.o
> > -obj-$(CONFIG_IIO_ST_PRESS_SPI) += st_pressure_spi.o
>
> I would split order fix into a separate change, but if maintainers are okay
> with this approach, I would not object.
Okay. Adding only adp810.o here. Will revert all other entries.
This will make this change logically connected to this series.
>
> ...
>
> > +#include <linux/cleanup.h>
> > +#include <linux/crc8.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/dev_printk.h>
> > +#include <linux/i2c.h>
> > +#include <linux/module.h>
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/mutex.h>
> > +#include <linux/unaligned.h>
>
> Something is still missing.
Added more include files as pointed out further below.
>
> ...
>
> > +struct adp810_read_buf {
> > + __be16 dp;
> > + u8 dp_crc;
> > + __be16 tmp;
> > + u8 tmp_crc;
> > + __be16 sf;
> > + u8 sf_crc;
>
> All these types are provided in types.h
ACK. Added linux/types.h
>
> > +} __packed;
>
> ...
>
> > +static int adp810_measure(struct adp810_data *data, struct adp810_read_buf *buf)
> > +{
> > + struct i2c_client *client = data->client;
> > + struct device *dev = &client->dev;
> > + int ret;
> > + u16 trig_cmd = ADP810_TRIGGER_COMMAND;
>
> Shouldn't this be __be16 or __le16? Or is that really a full 16-bit command?
> I have a gut feeling that this should be u8 x[2] = { ... }; instead.
Ohh yes. Nice catch !
This is 16 bit trigger command. We need to send 0x37 then 0x2d.
I defined trig_cmd = 0x2d37 which will work fine on little endian
systems, but will fail on big endian systems I think.
I use ARM v8-A configured in little endian, so did not catch this.
Let me use u8 trig_cmd[2] = {0x37, 0x2d} to make this architecture
agnostic.
>
> > + /* Send trigger to the sensor for measurement */
> > + ret = i2c_master_send(client, (char *)&trig_cmd, sizeof(trig_cmd));
> > + if (ret < 0) {
> > + dev_err(dev, "Error sending trigger command\n");
> > + return ret;
> > + }
> > + if (ret != sizeof(trig_cmd))
> > + return -EIO;
>
> -EIO is defined down from linux/errno.h.
ACK. Added linux/errno.h
>
> > + /*
> > + * Wait for the sensor to acquire data. As per datasheet section 5.3.1,
> > + * wait for at least 10ms before reading measurements from the sensor.
> > + */
> > + msleep(ADP810_MEASURE_LATENCY_MS);
> > +
> > + /* Read sensor values */
> > + ret = i2c_master_recv(client, (char *)buf, sizeof(*buf));
> > + if (ret < 0) {
> > + dev_err(dev, "Error reading from sensor\n");
> > + return ret;
> > + }
> > + if (ret != sizeof(*buf))
> > + return -EIO;
> > +
> > +
> > + if (buf->sf_crc != crc8(crc_table, (u8 *)&buf->sf, 0x2, CRC8_INIT_VALUE)) {
> > + dev_err(dev, "CRC error for scale\n");
> > + return -EIO;
> > + }
> > +
> > + return 0;
> > +}
>
> ...
>
> > + indio_dev->num_channels = ARRAY_SIZE(adp810_channels);
>
> ARRAY_SIZE() is defined in a specific header.
ACK. Added linux/array_size.h
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
> > > # When adding new entries keep the list in alphabetical order > > obj-$(CONFIG_ABP060MG) += abp060mg.o > > +obj-$(CONFIG_ADP810) += adp810.o > > obj-$(CONFIG_ROHM_BM1390) += rohm-bm1390.o > > obj-$(CONFIG_BMP280) += bmp280.o > > bmp280-objs := bmp280-core.o bmp280-regmap.o > > @@ -15,6 +16,7 @@ obj-$(CONFIG_DPS310) += dps310.o > > obj-$(CONFIG_IIO_CROS_EC_BARO) += cros_ec_baro.o > > obj-$(CONFIG_HID_SENSOR_PRESS) += hid-sensor-press.o > > obj-$(CONFIG_HP03) += hp03.o > > +obj-$(CONFIG_HP206C) += hp206c.o > > obj-$(CONFIG_HSC030PA) += hsc030pa.o > > obj-$(CONFIG_HSC030PA_I2C) += hsc030pa_i2c.o > > obj-$(CONFIG_HSC030PA_SPI) += hsc030pa_spi.o > > @@ -34,11 +36,9 @@ obj-$(CONFIG_SDP500) += sdp500.o > > obj-$(CONFIG_IIO_ST_PRESS) += st_pressure.o > > st_pressure-y := st_pressure_core.o > > st_pressure-$(CONFIG_IIO_BUFFER) += st_pressure_buffer.o > > +obj-$(CONFIG_IIO_ST_PRESS_I2C) += st_pressure_i2c.o > > +obj-$(CONFIG_IIO_ST_PRESS_SPI) += st_pressure_spi.o > > obj-$(CONFIG_T5403) += t5403.o > > -obj-$(CONFIG_HP206C) += hp206c.o > > obj-$(CONFIG_ZPA2326) += zpa2326.o > > obj-$(CONFIG_ZPA2326_I2C) += zpa2326_i2c.o > > obj-$(CONFIG_ZPA2326_SPI) += zpa2326_spi.o > > - > > -obj-$(CONFIG_IIO_ST_PRESS_I2C) += st_pressure_i2c.o > > -obj-$(CONFIG_IIO_ST_PRESS_SPI) += st_pressure_spi.o > > I would split order fix into a separate change, but if maintainers are okay > with this approach, I would not object. I was late to replying to v2 on this. Split it out please.
On Thu, Oct 23, 2025 at 06:27:21PM +0100, Jonathan Cameron wrote: > > > > > > # When adding new entries keep the list in alphabetical order > > > obj-$(CONFIG_ABP060MG) += abp060mg.o > > > +obj-$(CONFIG_ADP810) += adp810.o > > > obj-$(CONFIG_ROHM_BM1390) += rohm-bm1390.o > > > obj-$(CONFIG_BMP280) += bmp280.o > > > bmp280-objs := bmp280-core.o bmp280-regmap.o > > > @@ -15,6 +16,7 @@ obj-$(CONFIG_DPS310) += dps310.o > > > obj-$(CONFIG_IIO_CROS_EC_BARO) += cros_ec_baro.o > > > obj-$(CONFIG_HID_SENSOR_PRESS) += hid-sensor-press.o > > > obj-$(CONFIG_HP03) += hp03.o > > > +obj-$(CONFIG_HP206C) += hp206c.o > > > obj-$(CONFIG_HSC030PA) += hsc030pa.o > > > obj-$(CONFIG_HSC030PA_I2C) += hsc030pa_i2c.o > > > obj-$(CONFIG_HSC030PA_SPI) += hsc030pa_spi.o > > > @@ -34,11 +36,9 @@ obj-$(CONFIG_SDP500) += sdp500.o > > > obj-$(CONFIG_IIO_ST_PRESS) += st_pressure.o > > > st_pressure-y := st_pressure_core.o > > > st_pressure-$(CONFIG_IIO_BUFFER) += st_pressure_buffer.o > > > +obj-$(CONFIG_IIO_ST_PRESS_I2C) += st_pressure_i2c.o > > > +obj-$(CONFIG_IIO_ST_PRESS_SPI) += st_pressure_spi.o > > > obj-$(CONFIG_T5403) += t5403.o > > > -obj-$(CONFIG_HP206C) += hp206c.o > > > obj-$(CONFIG_ZPA2326) += zpa2326.o > > > obj-$(CONFIG_ZPA2326_I2C) += zpa2326_i2c.o > > > obj-$(CONFIG_ZPA2326_SPI) += zpa2326_spi.o > > > - > > > -obj-$(CONFIG_IIO_ST_PRESS_I2C) += st_pressure_i2c.o > > > -obj-$(CONFIG_IIO_ST_PRESS_SPI) += st_pressure_spi.o > > > > I would split order fix into a separate change, but if maintainers are okay > > with this approach, I would not object. > > I was late to replying to v2 on this. Split it out please. Sure. I will take reordering of other entries as an independent patch. In this series, let's add adp810.o only at alphabetically correct place. Regards, Akhilesh
© 2016 - 2026 Red Hat, Inc.