* [PATCH v9 1/3] iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode
2026-08-04 19:28 [PATCH v9 0/3] iio: adc: ti-ads1100: Add support for TI ADS1110 to ti-ads1100 driver Jakub Szczudlo
@ 2026-08-04 19:28 ` Jakub Szczudlo
2026-08-04 19:44 ` sashiko-bot
2026-08-04 19:28 ` [PATCH v9 2/3] dt-bindings: iio: adc: ti,ads1100: add support for ADS1110 Jakub Szczudlo
2026-08-04 19:28 ` [PATCH v9 3/3] iio: adc: ti-ads1100: Add ti-ads1110 support to ti-ads1100 driver Jakub Szczudlo
2 siblings, 1 reply; 6+ messages in thread
From: Jakub Szczudlo @ 2026-08-04 19:28 UTC (permalink / raw)
To: linux-iio
Cc: jic23, dlechner, nuno.sa, andy, marcelo.schmitt, robh, krzk+dt,
conor+dt, mike.looijmans, devicetree, linux-kernel, jorge.marques,
antoniu.miclaus, mazziesaccount, jishnu.prakash, duje, wens,
sakari.ailus, linusw, Jakub Szczudlo
When device is suspended and it is in single mode then changing
datarate doesn't make it actually wait for new measurement, so to
be sure that read after change is correct, functions that changes
datarate and gain will wait for a new data.
Fixes: 541880542f2b ("iio: adc: Add TI ADS1100 and ADS1000")
Signed-off-by: Jakub Szczudlo <jakubszczudlo40@gmail.com>
---
drivers/iio/adc/ti-ads1100.c | 108 +++++++++++++++++++++++++++++++++--
1 file changed, 104 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
index 9fe8d54cce83..288d209ecf92 100644
--- a/drivers/iio/adc/ti-ads1100.c
+++ b/drivers/iio/adc/ti-ads1100.c
@@ -15,10 +15,12 @@
#include <linux/module.h>
#include <linux/init.h>
#include <linux/i2c.h>
+#include <linux/iopoll.h>
#include <linux/mutex.h>
#include <linux/property.h>
#include <linux/pm_runtime.h>
#include <linux/regulator/consumer.h>
+#include <linux/time.h>
#include <linux/units.h>
#include <linux/iio/iio.h>
@@ -43,6 +45,9 @@
static const int ads1100_data_rate[] = { 128, 32, 16, 8 };
static const int ads1100_data_rate_bits[] = { 12, 14, 15, 16 };
+/* Timeout based on the minimum sample rate of 8 SPS (7500ms) */
+#define ADS1100_MAX_DRDY_TIMEOUT_US (7500 * USEC_PER_MSEC)
+
struct ads1100_data {
struct i2c_client *client;
struct regulator *reg_vdd;
@@ -123,10 +128,87 @@ static int ads1100_get_adc_result(struct ads1100_data *data, int chan, int *val)
return 0;
}
+static int ads1100_conversion_busy(struct ads1100_data *data)
+{
+ u8 buffer[3];
+ int ret;
+
+ ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
+ if (ret < 0) {
+ dev_err(&data->client->dev, "I2C read fail: %d\n", ret);
+ return ret;
+ }
+
+ return FIELD_GET(ADS1100_CFG_ST_BSY, buffer[2]);
+}
+
+static int ads1100_wait_single_conversion(struct ads1100_data *data)
+{
+ int data_rate_index = FIELD_GET(ADS1100_DR_MASK, data->config);
+ int data_rate_Hz = ads1100_data_rate[data_rate_index];
+ unsigned long poll_us = DIV_ROUND_CLOSEST(USEC_PER_SEC, data_rate_Hz) / 4;
+ int busy;
+ int ret;
+
+ ret = readx_poll_timeout(ads1100_conversion_busy, data,
+ busy, busy <= 0,
+ poll_us, ADS1100_MAX_DRDY_TIMEOUT_US);
+ if (busy < 0)
+ return busy;
+
+ return ret;
+}
+
+static int ads1100_start_single_conversion(struct ads1100_data *data)
+{
+ u8 config = data->config | ADS1100_CFG_SC;
+ int ret;
+
+ ret = i2c_master_send(data->client, &config, sizeof(config));
+ if (ret < 0) {
+ dev_err(&data->client->dev, "I2C write fail: %d\n", ret);
+ return ret;
+ }
+ /* Need to wait because of change from continuous to single mode */
+ ret = ads1100_wait_single_conversion(data);
+ if (ret)
+ return ret;
+
+ config |= ADS1100_CFG_ST_BSY;
+
+ ret = i2c_master_send(data->client, &config, sizeof(config));
+ if (ret < 0) {
+ dev_err(&data->client->dev, "I2C write fail: %d\n", ret);
+ return ret;
+ }
+
+ /* No need to cache it, it's status bit */
+ data->config = config & ~ADS1100_CFG_ST_BSY;
+
+ return 0;
+}
+
+static int ads1100_poll_data_ready(struct ads1100_data *data)
+{
+ int ret;
+
+ ret = ads1100_start_single_conversion(data);
+ if (ret)
+ return ret;
+
+ ret = ads1100_wait_single_conversion(data);
+ if (ret)
+ return ret;
+
+ return ads1100_set_config_bits(data, ADS1100_CFG_SC,
+ ADS1100_CONTINUOUS);
+}
+
static int ads1100_set_scale(struct ads1100_data *data, int val, int val2)
{
int microvolts;
int gain;
+ int ret;
/* With Vdd between 2.7 and 5V, the scale is always below 1 */
if (val)
@@ -135,6 +217,11 @@ static int ads1100_set_scale(struct ads1100_data *data, int val, int val2)
if (!val2)
return -EINVAL;
+ PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(&data->client->dev, pm);
+ ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+ if (ret)
+ return ret;
+
microvolts = regulator_get_voltage(data->reg_vdd);
/*
* val2 is in 'micro' units, n = val2 / 1000000
@@ -149,22 +236,35 @@ static int ads1100_set_scale(struct ads1100_data *data, int val, int val2)
ads1100_set_config_bits(data, ADS1100_PGA_MASK, ffs(gain) - 1);
- return 0;
+ return ads1100_poll_data_ready(data);
}
static int ads1100_set_data_rate(struct ads1100_data *data, int chan, int rate)
{
unsigned int i;
unsigned int size;
+ int ret;
size = data->supports_data_rate ? ARRAY_SIZE(ads1100_data_rate) : 1;
for (i = 0; i < size; i++) {
if (ads1100_data_rate[i] == rate)
- return ads1100_set_config_bits(data, ADS1100_DR_MASK,
- FIELD_PREP(ADS1100_DR_MASK, i));
+ break;
}
- return -EINVAL;
+ if (i == size)
+ return -EINVAL;
+
+ PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(&data->client->dev, pm);
+ ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+ if (ret)
+ return ret;
+
+ ret = ads1100_set_config_bits(data, ADS1100_DR_MASK,
+ FIELD_PREP(ADS1100_DR_MASK, i));
+ if (ret)
+ return ret;
+
+ return ads1100_poll_data_ready(data);
}
static int ads1100_get_vdd_millivolts(struct ads1100_data *data)
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v9 1/3] iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode
2026-08-04 19:28 ` [PATCH v9 1/3] iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode Jakub Szczudlo
@ 2026-08-04 19:44 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-08-04 19:44 UTC (permalink / raw)
To: Jakub Szczudlo; +Cc: robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Error paths in `ads1100_start_single_conversion()` leave the device and driver in a permanently desynchronized state on timeout or I2C failure.
- [High] I2C buffers allocated on the stack are passed to DMA-capable I2C APIs.
Pre-existing issues:
- [High] ADC read after runtime resume returns garbage or power-on reset values without waiting for the first conversion to complete.
--
--- Patch [1]: [PATCH v9 1/3] iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode ---
commit e93e149965f30840eff030d72d0901bf664fc6dd
Author: Jakub Szczudlo <jakubszczudlo40@gmail.com>
iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode
When device is suspended and it is in single mode then changing
datarate doesn't make it actually wait for new measurement, so to
be sure that read after change is correct, functions that changes
datarate and gain will wait for a new data.
> diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
> index 9fe8d54cce83c..288d209ecf92d 100644
> --- a/drivers/iio/adc/ti-ads1100.c
> +++ b/drivers/iio/adc/ti-ads1100.c
> @@ -123,10 +128,87 @@ static int ads1100_get_adc_result(struct ads1100_data *data, int chan, int *val)
[Severity: High]
This isn't a bug introduced by this patch, but does ads1100_get_adc_result()
read garbage or power-on reset values if executed immediately after waking up?
In ads1100_get_adc_result():
ret = pm_runtime_resume_and_get(&data->client->dev);
if (ret < 0)
return ret;
ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
Since a single conversion takes up to 125ms (at 8 SPS), calling
i2c_master_recv() immediately after runtime resume reads the ADC before it
completes its initial measurement.
> return 0;
> }
>
> +static int ads1100_conversion_busy(struct ads1100_data *data)
> +{
> + u8 buffer[3];
> + int ret;
> +
> + ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
[Severity: High]
Can this cause memory corruption? Passing the stack-allocated buffer to
i2c_master_recv() is unsafe if the underlying I2C host controller uses DMA,
as it can corrupt adjacent stack variables due to cache line invalidation.
> + if (ret < 0) {
[ ... ]
> +static int ads1100_start_single_conversion(struct ads1100_data *data)
> +{
> + u8 config = data->config | ADS1100_CFG_SC;
> + int ret;
> +
> + ret = i2c_master_send(data->client, &config, sizeof(config));
[Severity: High]
Could passing this stack-allocated config variable to i2c_master_send() also
cause memory corruption if the host controller uses DMA?
> + if (ret < 0) {
> + dev_err(&data->client->dev, "I2C write fail: %d\n", ret);
> + return ret;
> + }
> + /* Need to wait because of change from continuous to single mode */
> + ret = ads1100_wait_single_conversion(data);
> + if (ret)
> + return ret;
> +
> + config |= ADS1100_CFG_ST_BSY;
> +
> + ret = i2c_master_send(data->client, &config, sizeof(config));
> + if (ret < 0) {
> + dev_err(&data->client->dev, "I2C write fail: %d\n", ret);
> + return ret;
> + }
> +
> + /* No need to cache it, it's status bit */
> + data->config = config & ~ADS1100_CFG_ST_BSY;
[Severity: High]
Are we leaving the driver and hardware permanently out of sync on error here?
If ads1100_wait_single_conversion() or the second i2c_master_send() fails,
we return early. However, the physical hardware was transitioned to
single-shot mode while the software cache data->config was not updated to
include ADS1100_CFG_SC.
If userspace later tries to restore continuous mode, won't it silently skip
the I2C write because data->config appears unchanged, leaving the device
stuck in single-shot mode indefinitely?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260804192840.29121-1-jakubszczudlo40@gmail.com?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v9 2/3] dt-bindings: iio: adc: ti,ads1100: add support for ADS1110
2026-08-04 19:28 [PATCH v9 0/3] iio: adc: ti-ads1100: Add support for TI ADS1110 to ti-ads1100 driver Jakub Szczudlo
2026-08-04 19:28 ` [PATCH v9 1/3] iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode Jakub Szczudlo
@ 2026-08-04 19:28 ` Jakub Szczudlo
2026-08-04 19:28 ` [PATCH v9 3/3] iio: adc: ti-ads1100: Add ti-ads1110 support to ti-ads1100 driver Jakub Szczudlo
2 siblings, 0 replies; 6+ messages in thread
From: Jakub Szczudlo @ 2026-08-04 19:28 UTC (permalink / raw)
To: linux-iio
Cc: jic23, dlechner, nuno.sa, andy, marcelo.schmitt, robh, krzk+dt,
conor+dt, mike.looijmans, devicetree, linux-kernel, jorge.marques,
antoniu.miclaus, mazziesaccount, jishnu.prakash, duje, wens,
sakari.ailus, linusw, Jakub Szczudlo, Krzysztof Kozlowski
Register layouts are the same as for ADS1100 but ADS1110 have different
data rates and have internal voltage reference that is always 2.048V.
Also correct order of ads so they will be sorted alphabetically.
Signed-off-by: Jakub Szczudlo <jakubszczudlo40@gmail.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
.../devicetree/bindings/iio/adc/ti,ads1100.yaml | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml
index 970ccab15e1e..28c5e2dd0ad6 100644
--- a/Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml
@@ -4,19 +4,23 @@
$id: http://devicetree.org/schemas/iio/adc/ti,ads1100.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
-title: TI ADS1100/ADS1000 single channel I2C analog to digital converter
+title: TI ADS1100 and similar single channel I2C Analog to Digital Converters
maintainers:
- Mike Looijmans <mike.looijmans@topic.nl>
description: |
- Datasheet at: https://www.ti.com/lit/gpn/ads1100
+ Datasheets:
+ - https://www.ti.com/lit/gpn/ads1000
+ - https://www.ti.com/lit/gpn/ads1100
+ - https://www.ti.com/lit/gpn/ads1110
properties:
compatible:
enum:
- - ti,ads1100
- ti,ads1000
+ - ti,ads1100
+ - ti,ads1110
reg:
maxItems: 1
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v9 3/3] iio: adc: ti-ads1100: Add ti-ads1110 support to ti-ads1100 driver
2026-08-04 19:28 [PATCH v9 0/3] iio: adc: ti-ads1100: Add support for TI ADS1110 to ti-ads1100 driver Jakub Szczudlo
2026-08-04 19:28 ` [PATCH v9 1/3] iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode Jakub Szczudlo
2026-08-04 19:28 ` [PATCH v9 2/3] dt-bindings: iio: adc: ti,ads1100: add support for ADS1110 Jakub Szczudlo
@ 2026-08-04 19:28 ` Jakub Szczudlo
2026-08-04 19:42 ` sashiko-bot
2 siblings, 1 reply; 6+ messages in thread
From: Jakub Szczudlo @ 2026-08-04 19:28 UTC (permalink / raw)
To: linux-iio
Cc: jic23, dlechner, nuno.sa, andy, marcelo.schmitt, robh, krzk+dt,
conor+dt, mike.looijmans, devicetree, linux-kernel, jorge.marques,
antoniu.miclaus, mazziesaccount, jishnu.prakash, duje, wens,
sakari.ailus, linusw, Jakub Szczudlo
Add ADS1110 support that have faster datarate than ADS1100, it also uses
internal voltage reference of 2.048V for measurement.
Signed-off-by: Jakub Szczudlo <jakubszczudlo40@gmail.com>
---
drivers/iio/adc/Kconfig | 9 ++--
drivers/iio/adc/ti-ads1100.c | 85 ++++++++++++++++++++++++++----------
2 files changed, 69 insertions(+), 25 deletions(-)
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 3755a81c1efd..49a9ac3bf43d 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1768,11 +1768,14 @@ config TI_ADS1018
called ti-ads1018.
config TI_ADS1100
- tristate "Texas Instruments ADS1100 and ADS1000 ADC"
+ tristate "Texas Instruments ADS1100 and similar single channel I2C ADC"
depends on I2C
help
- If you say yes here you get support for Texas Instruments ADS1100 and
- ADS1000 ADC chips.
+ If you say yes here you get support for TI single channel I2C Analog
+ Devices.
+ * ADS1000 12-Bit, 128 SPS Analog-to-Digital Converter
+ * ADS1100 16-Bit, 128 SPS Analog-to-Digital Converter
+ * ADS1110 16-Bit, 240 SPS Analog-to-Digital Converter
This driver can also be built as a module. If so, the module will be
called ti-ads1100.
diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
index 288d209ecf92..acbf9b96c545 100644
--- a/drivers/iio/adc/ti-ads1100.c
+++ b/drivers/iio/adc/ti-ads1100.c
@@ -5,7 +5,7 @@
* Copyright (c) 2023, Topic Embedded Products
*
* Datasheet: https://www.ti.com/lit/gpn/ads1100
- * IIO driver for ADS1100 and ADS1000 ADC 16-bit I2C
+ * IIO driver for ADS1100 and similar single channel ADC 16-bit I2C
*/
#include <linux/bitfield.h>
@@ -41,20 +41,44 @@
#define ADS1100_SINGLESHOT ADS1100_CFG_SC
#define ADS1100_SLEEP_DELAY_MS 2000
+#define ADS1110_INTERNAL_REF_mV 2048
static const int ads1100_data_rate[] = { 128, 32, 16, 8 };
+static const int ads1110_data_rate[] = { 240, 60, 30, 15 };
static const int ads1100_data_rate_bits[] = { 12, 14, 15, 16 };
/* Timeout based on the minimum sample rate of 8 SPS (7500ms) */
#define ADS1100_MAX_DRDY_TIMEOUT_US (7500 * USEC_PER_MSEC)
+struct ads1100_config {
+ const char *name;
+ const int *available_data_rate_hz;
+ const int data_rate_count;
+ bool has_internal_vref_only;
+};
+
+static const struct ads1100_config ads1100_config = {
+ .name = "ads1100",
+ .available_data_rate_hz = ads1100_data_rate,
+ .data_rate_count = ARRAY_SIZE(ads1100_data_rate),
+ .has_internal_vref_only = false,
+};
+
+static const struct ads1100_config ads1110_config = {
+ .name = "ads1110",
+ .available_data_rate_hz = ads1110_data_rate,
+ .data_rate_count = ARRAY_SIZE(ads1110_data_rate),
+ .has_internal_vref_only = true,
+};
+
struct ads1100_data {
struct i2c_client *client;
struct regulator *reg_vdd;
struct mutex lock;
int scale_avail[2 * 4]; /* 4 gain settings */
+ const struct ads1100_config *chip_info;
u8 config;
- bool supports_data_rate; /* Only the ADS1100 can select the rate */
+ bool supports_data_rate;
};
static const struct iio_chan_spec ads1100_channel = {
@@ -90,6 +114,20 @@ static int ads1100_set_config_bits(struct ads1100_data *data, u8 mask, u8 value)
return 0;
};
+static int ads1100_get_vref_millivolts(struct ads1100_data *data)
+{
+ int voltage_uV;
+
+ if (data->chip_info->has_internal_vref_only)
+ return ADS1110_INTERNAL_REF_mV;
+
+ voltage_uV = regulator_get_voltage(data->reg_vdd);
+ if (voltage_uV < 0)
+ return voltage_uV;
+
+ return voltage_uV / (MICRO / MILLI);
+}
+
static int ads1100_data_bits(struct ads1100_data *data)
{
return ads1100_data_rate_bits[FIELD_GET(ADS1100_DR_MASK, data->config)];
@@ -145,7 +183,7 @@ static int ads1100_conversion_busy(struct ads1100_data *data)
static int ads1100_wait_single_conversion(struct ads1100_data *data)
{
int data_rate_index = FIELD_GET(ADS1100_DR_MASK, data->config);
- int data_rate_Hz = ads1100_data_rate[data_rate_index];
+ int data_rate_Hz = data->chip_info->available_data_rate_hz[data_rate_index];
unsigned long poll_us = DIV_ROUND_CLOSEST(USEC_PER_SEC, data_rate_Hz) / 4;
int busy;
int ret;
@@ -222,7 +260,7 @@ static int ads1100_set_scale(struct ads1100_data *data, int val, int val2)
if (ret)
return ret;
- microvolts = regulator_get_voltage(data->reg_vdd);
+ microvolts = ads1100_get_vref_millivolts(data) * (MICRO / MILLI);
/*
* val2 is in 'micro' units, n = val2 / 1000000
* result must be millivolts, d = microvolts / 1000
@@ -245,9 +283,9 @@ static int ads1100_set_data_rate(struct ads1100_data *data, int chan, int rate)
unsigned int size;
int ret;
- size = data->supports_data_rate ? ARRAY_SIZE(ads1100_data_rate) : 1;
+ size = data->supports_data_rate ? data->chip_info->data_rate_count : 1;
for (i = 0; i < size; i++) {
- if (ads1100_data_rate[i] == rate)
+ if (data->chip_info->available_data_rate_hz[i] == rate)
break;
}
@@ -267,14 +305,9 @@ static int ads1100_set_data_rate(struct ads1100_data *data, int chan, int rate)
return ads1100_poll_data_ready(data);
}
-static int ads1100_get_vdd_millivolts(struct ads1100_data *data)
-{
- return regulator_get_voltage(data->reg_vdd) / (MICRO / MILLI);
-}
-
static void ads1100_calc_scale_avail(struct ads1100_data *data)
{
- int millivolts = ads1100_get_vdd_millivolts(data);
+ int millivolts = ads1100_get_vref_millivolts(data);
unsigned int i;
for (i = 0; i < ARRAY_SIZE(data->scale_avail) / 2; i++) {
@@ -296,9 +329,9 @@ static int ads1100_read_avail(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_SAMP_FREQ:
*type = IIO_VAL_INT;
- *vals = ads1100_data_rate;
+ *vals = data->chip_info->available_data_rate_hz;
if (data->supports_data_rate)
- *length = ARRAY_SIZE(ads1100_data_rate);
+ *length = data->chip_info->data_rate_count;
else
*length = 1;
return IIO_AVAIL_LIST;
@@ -318,6 +351,7 @@ static int ads1100_read_raw(struct iio_dev *indio_dev,
{
int ret;
struct ads1100_data *data = iio_priv(indio_dev);
+ int data_rate_index;
guard(mutex)(&data->lock);
switch (mask) {
@@ -333,12 +367,12 @@ static int ads1100_read_raw(struct iio_dev *indio_dev,
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
/* full-scale is the supply voltage in millivolts */
- *val = ads1100_get_vdd_millivolts(data);
+ *val = ads1100_get_vref_millivolts(data);
*val2 = 15 + FIELD_GET(ADS1100_PGA_MASK, data->config);
return IIO_VAL_FRACTIONAL_LOG2;
case IIO_CHAN_INFO_SAMP_FREQ:
- *val = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK,
- data->config)];
+ data_rate_index = FIELD_GET(ADS1100_DR_MASK, data->config);
+ *val = data->chip_info->available_data_rate_hz[data_rate_index];
return IIO_VAL_INT;
default:
return -EINVAL;
@@ -418,7 +452,12 @@ static int ads1100_probe(struct i2c_client *client)
data->client = client;
mutex_init(&data->lock);
- indio_dev->name = "ads1100";
+ data->chip_info = i2c_get_match_data(client);
+ if (!data->chip_info)
+ return dev_err_probe(dev, -ENODATA,
+ "Can't get device data from firmware\n");
+
+ indio_dev->name = data->chip_info->name;
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->channels = &ads1100_channel;
indio_dev->num_channels = 1;
@@ -500,16 +539,18 @@ static DEFINE_RUNTIME_DEV_PM_OPS(ads1100_pm_ops,
NULL);
static const struct i2c_device_id ads1100_id[] = {
- { .name = "ads1100" },
- { .name = "ads1000" },
+ { .name = "ads1000", .driver_data = (kernel_ulong_t)&ads1100_config },
+ { .name = "ads1100", .driver_data = (kernel_ulong_t)&ads1100_config },
+ { .name = "ads1110", .driver_data = (kernel_ulong_t)&ads1110_config },
{ }
};
MODULE_DEVICE_TABLE(i2c, ads1100_id);
static const struct of_device_id ads1100_of_match[] = {
- {.compatible = "ti,ads1100" },
- {.compatible = "ti,ads1000" },
+ { .compatible = "ti,ads1000", .data = &ads1100_config },
+ { .compatible = "ti,ads1100", .data = &ads1100_config },
+ { .compatible = "ti,ads1110", .data = &ads1110_config },
{ }
};
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v9 3/3] iio: adc: ti-ads1100: Add ti-ads1110 support to ti-ads1100 driver
2026-08-04 19:28 ` [PATCH v9 3/3] iio: adc: ti-ads1100: Add ti-ads1110 support to ti-ads1100 driver Jakub Szczudlo
@ 2026-08-04 19:42 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-08-04 19:42 UTC (permalink / raw)
To: Jakub Szczudlo; +Cc: robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Missing error check for `ads1100_get_vref_millivolts()` return value propagates negative error codes to userspace.
- [Medium] Missing state restore on error paths in `ads1100_poll_data_ready` leaves the ADC permanently in single-shot mode.
--
--- Patch [3]: [PATCH v9 3/3] iio: adc: ti-ads1100: Add ti-ads1110 support to ti-ads1100 driver ---
commit 1ff426338598760f136ce661b966962981800132
Author: Jakub Szczudlo <jakubszczudlo40@gmail.com>
iio: adc: ti-ads1100: Add ti-ads1110 support to ti-ads1100 driver
Add ADS1110 support that have faster datarate than ADS1100, it also uses
internal voltage reference of 2.048V for measurement.
> diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
> index 288d209ecf92d..acbf9b96c545d 100644
> --- a/drivers/iio/adc/ti-ads1100.c
> +++ b/drivers/iio/adc/ti-ads1100.c
[ ... ]
> @@ -90,6 +114,20 @@ static int ads1100_set_config_bits(struct ads1100_data *data, u8 mask, u8 value)
> return 0;
> };
>
> +static int ads1100_get_vref_millivolts(struct ads1100_data *data)
> +{
> + int voltage_uV;
> +
> + if (data->chip_info->has_internal_vref_only)
> + return ADS1110_INTERNAL_REF_mV;
> +
> + voltage_uV = regulator_get_voltage(data->reg_vdd);
> + if (voltage_uV < 0)
> + return voltage_uV;
> +
> + return voltage_uV / (MICRO / MILLI);
> +}
> +
[Severity: Medium]
Will this return a negative error code (like -EINVAL) to callers that don't
expect it?
If regulator_get_voltage() fails, ads1100_get_vref_millivolts() returns the
error code. However, callers like ads1100_calc_scale_avail() and
ads1100_read_raw() don't check for errors:
drivers/iio/adc/ti-ads1100.c:ads1100_calc_scale_avail() {
int millivolts = ads1100_get_vref_millivolts(data);
...
data->scale_avail[i * 2 + 0] = millivolts;
}
drivers/iio/adc/ti-ads1100.c:ads1100_read_raw() {
case IIO_CHAN_INFO_SCALE:
*val = ads1100_get_vref_millivolts(data);
...
}
Could this regression incorrectly expose a negative integer to userspace as a
valid voltage or scale value?
[ ... ]
> @@ -145,7 +183,7 @@ static int ads1100_conversion_busy(struct ads1100_data *data)
> static int ads1100_wait_single_conversion(struct ads1100_data *data)
> {
> int data_rate_index = FIELD_GET(ADS1100_DR_MASK, data->config);
> - int data_rate_Hz = ads1100_data_rate[data_rate_index];
> + int data_rate_Hz = data->chip_info->available_data_rate_hz[data_rate_index];
> unsigned long poll_us = DIV_ROUND_CLOSEST(USEC_PER_SEC, data_rate_Hz) / 4;
> int busy;
> int ret;
[Severity: Medium]
If ads1100_wait_single_conversion() returns an error here, does it leave the
ADC permanently stuck in single-shot mode?
Looking at the caller ads1100_poll_data_ready():
drivers/iio/adc/ti-ads1100.c:ads1100_poll_data_ready() {
...
ret = ads1100_wait_single_conversion(data);
if (ret)
return ret;
return ads1100_set_config_bits(data, ADS1100_CFG_SC, ADS1100_CONTINUOUS);
}
If ads1100_wait_single_conversion() fails, the function returns immediately,
bypassing the step that restores the hardware to continuous conversion mode.
Could this regression cause subsequent continuous reads to fetch stale data
because the ADC stopped converting?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260804192840.29121-1-jakubszczudlo40@gmail.com?part=3
^ permalink raw reply [flat|nested] 6+ messages in thread