* [PATCH v7 1/3] iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode
2026-07-14 19:55 [PATCH v7 0/3] iio: adc: ti-ads1100: Add support for TI ADS1110 to ti-ads1100 driver Jakub Szczudlo
@ 2026-07-14 19:55 ` Jakub Szczudlo
2026-07-14 20:10 ` sashiko-bot
2026-07-15 6:45 ` Andy Shevchenko
2026-07-14 19:55 ` [PATCH v7 2/3] dt-bindings: iio: adc: ti,ads1100: add support for ADS1110 Jakub Szczudlo
` (2 subsequent siblings)
3 siblings, 2 replies; 14+ messages in thread
From: Jakub Szczudlo @ 2026-07-14 19:55 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 | 71 ++++++++++++++++++++++++++++++++++--
1 file changed, 67 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
index 9fe8d54cce83..f0a30ae139af 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,50 @@ static int ads1100_get_adc_result(struct ads1100_data *data, int chan, int *val)
return 0;
}
+static int ads1100_new_data_is_ready(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_poll_data_ready(struct ads1100_data *data)
+{
+ int data_rate_Hz = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];
+ /* To be sure we wait 5 times more than data rate */
+ unsigned long wait_time_us = DIV_ROUND_CLOSEST(USEC_PER_SEC, 5 * data_rate_Hz);
+ int data_ready;
+ u8 buffer[3];
+ int ret;
+
+ /* To be sure that polled value will have value after config change */
+ 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;
+ }
+
+ ret = readx_poll_timeout(ads1100_new_data_is_ready, data,
+ data_ready, data_ready != 0,
+ wait_time_us, ADS1100_MAX_DRDY_TIMEOUT_US);
+ if (ret)
+ return ret;
+
+ return data_ready < 0 ? data_ready : 0;
+}
+
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 +180,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 +199,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] 14+ messages in thread* Re: [PATCH v7 1/3] iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode
2026-07-14 19:55 ` [PATCH v7 1/3] iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode Jakub Szczudlo
@ 2026-07-14 20:10 ` sashiko-bot
2026-07-15 6:45 ` Andy Shevchenko
1 sibling, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-07-14 20:10 UTC (permalink / raw)
To: Jakub Szczudlo; +Cc: conor+dt, robh, devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Polling logic is broken due to an inverted termination condition, leading to immediate exits when the device is busy.
Pre-existing issues:
- [High] The device driver reads stale or default power-on data immediately after runtime PM resumes it.
--
--- Patch [1]: [PATCH v7 1/3] iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode ---
commit 64de239449801ff35a0ccf32ce8fdee336eb3033
Author: Jakub Szczudlo <jakubszczudlo40@gmail.com>
Subject: iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode
This commit introduces a polling loop to wait for new data to be ready
when the datarate or gain is changed while the device is in single mode.
It adds ads1100_poll_data_ready() and calls it after configuration changes.
> diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
> --- a/drivers/iio/adc/ti-ads1100.c
> +++ b/drivers/iio/adc/ti-ads1100.c
[ ... ]
> @@ -123,10 +128,50 @@ static int ads1100_get_adc_result(struct ads1100_data *data, int chan, int *val)
[ ... ]
> ret = pm_runtime_resume_and_get(&data->client->dev);
> if (ret < 0)
> return ret;
>
> ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
[Severity: High]
This is a pre-existing issue, but does this immediately read a stale or
power-on value (0) instead of a valid measurement after autosuspend?
When pm_runtime_resume_and_get() wakes the device, the ADC is powered on
and placed in continuous mode. However, we immediately execute an I2C
read without waiting for the ADC's first conversion to complete (which can
take up to 125ms at 8 SPS).
>
> pm_runtime_put_autosuspend(&data->client->dev);
>
> if (ret < 0) {
[ ... ]
> +static int ads1100_new_data_is_ready(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_poll_data_ready(struct ads1100_data *data)
> +{
[ ... ]
> + ret = readx_poll_timeout(ads1100_new_data_is_ready, data,
> + data_ready, data_ready != 0,
> + wait_time_us, ADS1100_MAX_DRDY_TIMEOUT_US);
[Severity: High]
Is the termination condition for the polling loop inverted?
ads1100_new_data_is_ready() returns the ST/BSY bit, which is 1 when the ADC
is busy and 0 when it is ready.
Since readx_poll_timeout() exits when the condition evaluates to true,
data_ready != 0 will cause the loop to exit immediately while the device is
still busy, bypassing the wait entirely.
> + if (ret)
> + return ret;
> +
> + return data_ready < 0 ? data_ready : 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714195528.597753-1-jakubszczudlo40@gmail.com?part=1
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v7 1/3] iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode
2026-07-14 19:55 ` [PATCH v7 1/3] iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode Jakub Szczudlo
2026-07-14 20:10 ` sashiko-bot
@ 2026-07-15 6:45 ` Andy Shevchenko
1 sibling, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2026-07-15 6:45 UTC (permalink / raw)
To: Jakub Szczudlo
Cc: linux-iio, 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
On Tue, Jul 14, 2026 at 09:55:26PM +0200, Jakub Szczudlo wrote:
> 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.
...
> +static int ads1100_poll_data_ready(struct ads1100_data *data)
> +{
> + int data_rate_Hz = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];
> + /* To be sure we wait 5 times more than data rate */
> + unsigned long wait_time_us = DIV_ROUND_CLOSEST(USEC_PER_SEC, 5 * data_rate_Hz);
> + int data_ready;
> + u8 buffer[3];
> + int ret;
> +
> + /* To be sure that polled value will have value after config change */
> + 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;
> + }
> +
> + ret = readx_poll_timeout(ads1100_new_data_is_ready, data,
> + data_ready, data_ready != 0,
> + wait_time_us, ADS1100_MAX_DRDY_TIMEOUT_US);
> + if (ret)
> + return ret;
> + return data_ready < 0 ? data_ready : 0;
I would unroll this to if-cond as it will be more readable (to get that
data_ready is negative when it holds an error code).
if (data_ready < 0)
return data_ready;
return 0;
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v7 2/3] dt-bindings: iio: adc: ti,ads1100: add support for ADS1110
2026-07-14 19:55 [PATCH v7 0/3] iio: adc: ti-ads1100: Add support for TI ADS1110 to ti-ads1100 driver Jakub Szczudlo
2026-07-14 19:55 ` [PATCH v7 1/3] iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode Jakub Szczudlo
@ 2026-07-14 19:55 ` Jakub Szczudlo
2026-07-14 20:04 ` sashiko-bot
2026-07-14 19:55 ` [PATCH v7 3/3] iio: adc: ti-ads1100: Add ti-ads1110 support to ti-ads1100 driver Jakub Szczudlo
2026-07-14 20:20 ` [PATCH v7 0/3] iio: adc: ti-ads1100: Add support for TI ADS1110 " David Lechner
3 siblings, 1 reply; 14+ messages in thread
From: Jakub Szczudlo @ 2026-07-14 19:55 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] 14+ messages in thread* Re: [PATCH v7 2/3] dt-bindings: iio: adc: ti,ads1100: add support for ADS1110
2026-07-14 19:55 ` [PATCH v7 2/3] dt-bindings: iio: adc: ti,ads1100: add support for ADS1110 Jakub Szczudlo
@ 2026-07-14 20:04 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-07-14 20:04 UTC (permalink / raw)
To: Jakub Szczudlo; +Cc: devicetree, conor+dt, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Logic error in ads1100_poll_data_ready causes a 7.5 second hang and -ETIMEDOUT on every configuration change
--
--- Patch [2]: [PATCH v7 2/3] dt-bindings: iio: adc: ti,ads1100: add support for ADS1110 ---
commit 020c9fd2512b9b8cdb2cbcd5179eab372c1ab0a5
Author: Jakub Szczudlo <jakubszczudlo40@gmail.com>
dt-bindings: iio: adc: ti,ads1100: add support for ADS1110
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.
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml
> index 970ccab15e1ec..28c5e2dd0ad62 100644
> --- a/Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml
[ ... ]
[Severity: High]
Does this logic in ads1100_poll_data_ready() cause a timeout on every
configuration change?
In drivers/iio/adc/ti-ads1100.c:ads1100_poll_data_ready():
ret = readx_poll_timeout(ads1100_new_data_is_ready, data,
data_ready, data_ready != 0,
wait_time_us, ADS1100_MAX_DRDY_TIMEOUT_US);
if (ret)
return ret;
And in ads1100_new_data_is_ready():
return FIELD_GET(ADS1100_CFG_ST_BSY, buffer[2]);
When the device operates in continuous conversion mode (enforced by
ads1100_runtime_resume before the polling loop), the datasheet specifies
that the ST/BSY bit is always 0.
Since data_ready is always 0, the condition data_ready != 0 evaluates
to false on every iteration. Will the polling loop always block for the
maximum timeout of 7.5 seconds (ADS1100_MAX_DRDY_TIMEOUT_US) and return
-ETIMEDOUT?
This breaks sysfs writes to scale and sampling frequency, severely
impacting driver usability and causing large delays.
Furthermore, even if the device was in single-shot mode, breaking when
data_ready != 0 means it would exit immediately while BUSY (1), instead
of waiting for the conversion to complete (0).
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714195528.597753-1-jakubszczudlo40@gmail.com?part=2
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v7 3/3] iio: adc: ti-ads1100: Add ti-ads1110 support to ti-ads1100 driver
2026-07-14 19:55 [PATCH v7 0/3] iio: adc: ti-ads1100: Add support for TI ADS1110 to ti-ads1100 driver Jakub Szczudlo
2026-07-14 19:55 ` [PATCH v7 1/3] iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode Jakub Szczudlo
2026-07-14 19:55 ` [PATCH v7 2/3] dt-bindings: iio: adc: ti,ads1100: add support for ADS1110 Jakub Szczudlo
@ 2026-07-14 19:55 ` Jakub Szczudlo
2026-07-14 20:07 ` sashiko-bot
2026-07-14 20:18 ` David Lechner
2026-07-14 20:20 ` [PATCH v7 0/3] iio: adc: ti-ads1100: Add support for TI ADS1110 " David Lechner
3 siblings, 2 replies; 14+ messages in thread
From: Jakub Szczudlo @ 2026-07-14 19:55 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 | 87 +++++++++++++++++++++++++++---------
2 files changed, 71 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 f0a30ae139af..5d798bfcc74e 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 *ads_config;
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->ads_config->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)];
@@ -144,7 +182,8 @@ static int ads1100_new_data_is_ready(struct ads1100_data *data)
static int ads1100_poll_data_ready(struct ads1100_data *data)
{
- int data_rate_Hz = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];
+ int data_rate_index = FIELD_GET(ADS1100_DR_MASK, data->config);
+ int data_rate_Hz = data->ads_config->available_data_rate_hz[data_rate_index];
/* To be sure we wait 5 times more than data rate */
unsigned long wait_time_us = DIV_ROUND_CLOSEST(USEC_PER_SEC, 5 * data_rate_Hz);
int data_ready;
@@ -185,7 +224,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
@@ -208,9 +247,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->ads_config->data_rate_count : 1;
for (i = 0; i < size; i++) {
- if (ads1100_data_rate[i] == rate)
+ if (data->ads_config->available_data_rate_hz[i] == rate)
break;
}
@@ -230,14 +269,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++) {
@@ -259,9 +293,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->ads_config->available_data_rate_hz;
if (data->supports_data_rate)
- *length = ARRAY_SIZE(ads1100_data_rate);
+ *length = data->ads_config->data_rate_count;
else
*length = 1;
return IIO_AVAIL_LIST;
@@ -280,6 +314,7 @@ static int ads1100_read_raw(struct iio_dev *indio_dev,
int *val2, long mask)
{
int ret;
+ int data_rate_index;
struct ads1100_data *data = iio_priv(indio_dev);
guard(mutex)(&data->lock);
@@ -296,12 +331,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->ads_config->available_data_rate_hz[data_rate_index];
return IIO_VAL_INT;
default:
return -EINVAL;
@@ -381,7 +416,6 @@ static int ads1100_probe(struct i2c_client *client)
data->client = client;
mutex_init(&data->lock);
- indio_dev->name = "ads1100";
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->channels = &ads1100_channel;
indio_dev->num_channels = 1;
@@ -401,6 +435,13 @@ static int ads1100_probe(struct i2c_client *client)
if (ret)
return ret;
+ data->ads_config = i2c_get_match_data(client);
+ if (!data->ads_config)
+ return dev_err_probe(dev, -EINVAL,
+ "Can't get device data from firmware\n");
+
+ indio_dev->name = data->ads_config->name;
+
ret = ads1100_setup(data);
if (ret)
return dev_err_probe(dev, ret,
@@ -463,16 +504,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] 14+ messages in thread* Re: [PATCH v7 3/3] iio: adc: ti-ads1100: Add ti-ads1110 support to ti-ads1100 driver
2026-07-14 19:55 ` [PATCH v7 3/3] iio: adc: ti-ads1100: Add ti-ads1110 support to ti-ads1100 driver Jakub Szczudlo
@ 2026-07-14 20:07 ` sashiko-bot
2026-07-14 20:18 ` David Lechner
1 sibling, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-07-14 20:07 UTC (permalink / raw)
To: Jakub Szczudlo; +Cc: conor+dt, devicetree, robh
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Inverted polling condition in ads1100_poll_data_ready causes 7.5s hangs and missed data.
- [Medium] Unhandled negative error code from ads1100_get_vref_millivolts() leaks to userspace.
--
--- Patch [3]: [PATCH v7 3/3] iio: adc: ti-ads1100: Add ti-ads1110 support to ti-ads1100 driver ---
commit 0307e42b36df78ace85fb2bcabb35c647c1a78d3
Author: Jakub Szczudlo <jakubszczudlo40@gmail.com>
iio: adc: ti-ads1100: Add ti-ads1110 support to ti-ads1100 driver
This commit adds support for the ADS1110 ADC. It provides faster data rates
than the ADS1100 and uses an internal voltage reference of 2.048V.
> diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
> index f0a30ae139afa..5d798bfcc74e1 100644
> --- a/drivers/iio/adc/ti-ads1100.c
> +++ b/drivers/iio/adc/ti-ads1100.c
[ ... ]
> @@ -144,7 +182,8 @@ static int ads1100_new_data_is_ready(struct ads1100_data *data)
>
> static int ads1100_poll_data_ready(struct ads1100_data *data)
> {
> - int data_rate_Hz = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];
> + int data_rate_index = FIELD_GET(ADS1100_DR_MASK, data->config);
> + int data_rate_Hz = data->ads_config->available_data_rate_hz[data_rate_index];
> /* To be sure we wait 5 times more than data rate */
> unsigned long wait_time_us = DIV_ROUND_CLOSEST(USEC_PER_SEC, 5 * data_rate_Hz);
> int data_ready;
[Severity: High]
Does the inverted break condition in ads1100_poll_data_ready() cause the
polling loop to hang when data is ready?
Further down in this function, the readx_poll_timeout() call evaluates
the data_ready variable:
ret = readx_poll_timeout(ads1100_new_data_is_ready, data,
data_ready, data_ready != 0,
wait_time_us, ADS1100_MAX_DRDY_TIMEOUT_US);
It looks like ads1100_new_data_is_ready() returns 0 when ready, 1 when busy,
and a negative code on I2C error.
Since readx_poll_timeout() stops looping when its condition evaluates to true,
data_ready != 0 would cause it to silently exit on busy or error states, but
hang and loop until the 7.5s timeout when data is actually ready.
[ ... ]
> @@ -230,14 +269,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);
[Severity: Medium]
If regulator_get_voltage() fails inside ads1100_get_vref_millivolts(),
will this permanently cache a bogus negative error code (like -EINVAL) in
the scale_avail array?
> unsigned int i;
>
> for (i = 0; i < ARRAY_SIZE(data->scale_avail) / 2; i++) {
[ ... ]
> @@ -296,12 +331,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);
[Severity: Medium]
Can an unhandled negative error code from ads1100_get_vref_millivolts() leak
to userspace here?
If the VDD regulator driver fails to return its voltage, *val will contain
the negative error code. Because the function returns IIO_VAL_FRACTIONAL_LOG2
instead of the error, this masks the failure from the IIO core.
> *val2 = 15 + FIELD_GET(ADS1100_PGA_MASK, data->config);
> return IIO_VAL_FRACTIONAL_LOG2;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714195528.597753-1-jakubszczudlo40@gmail.com?part=3
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v7 3/3] iio: adc: ti-ads1100: Add ti-ads1110 support to ti-ads1100 driver
2026-07-14 19:55 ` [PATCH v7 3/3] iio: adc: ti-ads1100: Add ti-ads1110 support to ti-ads1100 driver Jakub Szczudlo
2026-07-14 20:07 ` sashiko-bot
@ 2026-07-14 20:18 ` David Lechner
2026-07-15 6:42 ` Andy Shevchenko
1 sibling, 1 reply; 14+ messages in thread
From: David Lechner @ 2026-07-14 20:18 UTC (permalink / raw)
To: Jakub Szczudlo, linux-iio
Cc: jic23, 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
On 7/14/26 2:55 PM, Jakub Szczudlo wrote:
> 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 | 87 +++++++++++++++++++++++++++---------
> 2 files changed, 71 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 f0a30ae139af..5d798bfcc74e 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
Maybe the diff is messing with the tabs, but I hope the
numbers are vertically aligned.
>
> 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 *ads_config;
The name ads_config could be a bit confusing since the is already a
.config field. We usually call these chip_info if you want a different
name.
> 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->ads_config->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)];
> @@ -144,7 +182,8 @@ static int ads1100_new_data_is_ready(struct ads1100_data *data)
>
> static int ads1100_poll_data_ready(struct ads1100_data *data)
> {
> - int data_rate_Hz = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];
> + int data_rate_index = FIELD_GET(ADS1100_DR_MASK, data->config);
> + int data_rate_Hz = data->ads_config->available_data_rate_hz[data_rate_index];
> /* To be sure we wait 5 times more than data rate */
> unsigned long wait_time_us = DIV_ROUND_CLOSEST(USEC_PER_SEC, 5 * data_rate_Hz);
> int data_ready;
> @@ -185,7 +224,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
> @@ -208,9 +247,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->ads_config->data_rate_count : 1;
> for (i = 0; i < size; i++) {
> - if (ads1100_data_rate[i] == rate)
> + if (data->ads_config->available_data_rate_hz[i] == rate)
> break;
> }
>
> @@ -230,14 +269,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++) {
> @@ -259,9 +293,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->ads_config->available_data_rate_hz;
> if (data->supports_data_rate)
> - *length = ARRAY_SIZE(ads1100_data_rate);
> + *length = data->ads_config->data_rate_count;
> else
> *length = 1;
> return IIO_AVAIL_LIST;
> @@ -280,6 +314,7 @@ static int ads1100_read_raw(struct iio_dev *indio_dev,
> int *val2, long mask)
> {
> int ret;
> + int data_rate_index;
> struct ads1100_data *data = iio_priv(indio_dev);
>
> guard(mutex)(&data->lock);
> @@ -296,12 +331,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->ads_config->available_data_rate_hz[data_rate_index];
> return IIO_VAL_INT;
> default:
> return -EINVAL;
> @@ -381,7 +416,6 @@ static int ads1100_probe(struct i2c_client *client)
> data->client = client;
> mutex_init(&data->lock);
>
> - indio_dev->name = "ads1100";
> indio_dev->modes = INDIO_DIRECT_MODE;
> indio_dev->channels = &ads1100_channel;
> indio_dev->num_channels = 1;
> @@ -401,6 +435,13 @@ static int ads1100_probe(struct i2c_client *client)
> if (ret)
> return ret;
>
> + data->ads_config = i2c_get_match_data(client);
> + if (!data->ads_config)
> + return dev_err_probe(dev, -EINVAL,
Andy has been pushing to use ENODEV for these. Current situation
is about equal numbers of EINVAL and ENODEV in existing drivers.
Would be good to pick one and stick with it for new code.
> + "Can't get device data from firmware\n");
> +
> + indio_dev->name = data->ads_config->name;
I would move all of this earlier so we don't have to move
setting the name away from the block of other indio_dev->.
> +
> ret = ads1100_setup(data);
> if (ret)
> return dev_err_probe(dev, ret,
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v7 3/3] iio: adc: ti-ads1100: Add ti-ads1110 support to ti-ads1100 driver
2026-07-14 20:18 ` David Lechner
@ 2026-07-15 6:42 ` Andy Shevchenko
0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2026-07-15 6:42 UTC (permalink / raw)
To: David Lechner
Cc: Jakub Szczudlo, linux-iio, jic23, 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
On Tue, Jul 14, 2026 at 03:18:04PM -0500, David Lechner wrote:
> On 7/14/26 2:55 PM, Jakub Szczudlo wrote:
...
> > #define ADS1100_SLEEP_DELAY_MS 2000
> > +#define ADS1110_INTERNAL_REF_mV 2048
>
> Maybe the diff is messing with the tabs, but I hope the
> numbers are vertically aligned.
They are. Remove the quoting in your MUA and you will see that.
...
> > static int ads1100_read_raw(struct iio_dev *indio_dev,
> > int *val2, long mask)
> > {
> > int ret;
> > + int data_rate_index;
> > struct ads1100_data *data = iio_priv(indio_dev);
Likewise in the rest of the (new) code, try to follow it the reversed xmas tree
order
int ret;
struct ads1100_data *data = iio_priv(indio_dev);
int data_rate_index;
...
> > + data->ads_config = i2c_get_match_data(client);
> > + if (!data->ads_config)
> > + return dev_err_probe(dev, -EINVAL,
>
> Andy has been pushing to use ENODEV for these. Current situation
> is about equal numbers of EINVAL and ENODEV in existing drivers.
> Would be good to pick one and stick with it for new code.
Maybe -ENODATA as you suggested? Sounds perhaps better?
I saw the code where this kind of call and followed by it both use -ENODEV,
and after thinking of your suggestion I now tend towards -ENODATA.
> > + "Can't get device data from firmware\n");
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7 0/3] iio: adc: ti-ads1100: Add support for TI ADS1110 to ti-ads1100 driver
2026-07-14 19:55 [PATCH v7 0/3] iio: adc: ti-ads1100: Add support for TI ADS1110 to ti-ads1100 driver Jakub Szczudlo
` (2 preceding siblings ...)
2026-07-14 19:55 ` [PATCH v7 3/3] iio: adc: ti-ads1100: Add ti-ads1110 support to ti-ads1100 driver Jakub Szczudlo
@ 2026-07-14 20:20 ` David Lechner
2026-07-15 6:46 ` Andy Shevchenko
3 siblings, 1 reply; 14+ messages in thread
From: David Lechner @ 2026-07-14 20:20 UTC (permalink / raw)
To: Jakub Szczudlo, linux-iio
Cc: jic23, 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
On 7/14/26 2:55 PM, Jakub Szczudlo wrote:
> Add support for the TI ADS1110 to the existing ADS1100 ADC IIO driver.
> The ADS1110 is pin-to-pin compatible with the ADS1100 while providing
> higher resolution and an internal voltage reference. This patch series
> extends driver support for ADS1110, updates device tree bindings and
> Kconfig text, and improves the overall hardware description for the
> TI ADS1100 family.
>
> Tested on: Raspberry pi 3b+ with 7.0 stable kernel
Hopefully this applies on the iio/testing branch? A lot could have changed
since 7.0.
>
> ---
I made some suggestions for cosmetic improvements, but good enough for
Reviewed-by: David Lechner <dlechner@baylibre.com>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v7 0/3] iio: adc: ti-ads1100: Add support for TI ADS1110 to ti-ads1100 driver
2026-07-14 20:20 ` [PATCH v7 0/3] iio: adc: ti-ads1100: Add support for TI ADS1110 " David Lechner
@ 2026-07-15 6:46 ` Andy Shevchenko
0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2026-07-15 6:46 UTC (permalink / raw)
To: David Lechner
Cc: Jakub Szczudlo, linux-iio, jic23, 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
On Tue, Jul 14, 2026 at 03:20:18PM -0500, David Lechner wrote:
> On 7/14/26 2:55 PM, Jakub Szczudlo wrote:
> > Add support for the TI ADS1110 to the existing ADS1100 ADC IIO driver.
> > The ADS1110 is pin-to-pin compatible with the ADS1100 while providing
> > higher resolution and an internal voltage reference. This patch series
> > extends driver support for ADS1110, updates device tree bindings and
> > Kconfig text, and improves the overall hardware description for the
> > TI ADS1100 family.
> >
> > Tested on: Raspberry pi 3b+ with 7.0 stable kernel
>
> Hopefully this applies on the iio/testing branch? A lot could have changed
> since 7.0.
> > ---
> I made some suggestions for cosmetic improvements, but good enough for
>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
+1 here, but I think the chip_info naming and actually -ENODATA are good to be
in from day 1. Assuming that is going to be addressed in v8,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v7 1/3] iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode
2026-07-14 19:47 Jakub Szczudlo
@ 2026-07-14 19:47 ` Jakub Szczudlo
2026-07-14 20:03 ` sashiko-bot
0 siblings, 1 reply; 14+ messages in thread
From: Jakub Szczudlo @ 2026-07-14 19:47 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 a new measurement, so to
be sure that read after change is correct functions that changes
datarate and gain will wait for 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 | 71 ++++++++++++++++++++++++++++++++++--
1 file changed, 67 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
index 9fe8d54cce83..f0a30ae139af 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,50 @@ static int ads1100_get_adc_result(struct ads1100_data *data, int chan, int *val)
return 0;
}
+static int ads1100_new_data_is_ready(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_poll_data_ready(struct ads1100_data *data)
+{
+ int data_rate_Hz = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];
+ /* To be sure we wait 5 times more than data rate */
+ unsigned long wait_time_us = DIV_ROUND_CLOSEST(USEC_PER_SEC, 5 * data_rate_Hz);
+ int data_ready;
+ u8 buffer[3];
+ int ret;
+
+ /* To be sure that polled value will have value after config change */
+ 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;
+ }
+
+ ret = readx_poll_timeout(ads1100_new_data_is_ready, data,
+ data_ready, data_ready != 0,
+ wait_time_us, ADS1100_MAX_DRDY_TIMEOUT_US);
+ if (ret)
+ return ret;
+
+ return data_ready < 0 ? data_ready : 0;
+}
+
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 +180,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 +199,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] 14+ messages in thread* Re: [PATCH v7 1/3] iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode
2026-07-14 19:47 ` [PATCH v7 1/3] iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode Jakub Szczudlo
@ 2026-07-14 20:03 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-07-14 20:03 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] The polling logic in `ads1100_poll_data_ready` is fundamentally broken due to an inverted loop condition and impossible hardware polling state in continuous mode.
Pre-existing issues:
- [High] The driver consistently returns stale or invalid data on the first read after waking up from runtime suspend because it does not wait for a new conversion to complete.
- [Medium] Missing return value check for `ads1100_set_config_bits()` in `ads1100_set_scale()` suppresses I2C write errors.
--
--- Patch [1]: [PATCH v7 1/3] iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode ---
commit 110a1aa46c3c440dfe6b3dac8a6c6e80760ff636
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 a new measurement, so to
be sure that read after change is correct functions that changes
datarate and gain will wait for new data.
Fixes: 541880542f2b ("iio: adc: Add TI ADS1100 and ADS1000")
> diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
> index 9fe8d54cce83c..f0a30ae139afa 100644
> --- a/drivers/iio/adc/ti-ads1100.c
> +++ b/drivers/iio/adc/ti-ads1100.c
> @@ -123,10 +128,50 @@ static int ads1100_get_adc_result(struct ads1100_data *data, int chan, int *val)
> return 0;
> }
[Severity: High]
This is a pre-existing issue, but does ads1100_get_adc_result() return stale
or invalid data on the first read after waking up from runtime suspend?
The code executes the I2C read immediately after the runtime PM resume
returns, bypassing the necessary hardware conversion delay:
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));
If the read occurs immediately before the conversion finishes (for example,
125ms delay is needed at 8 SPS), will this return the power-on reset value
of 0 or garbage data instead of a valid measurement?
> +static int ads1100_poll_data_ready(struct ads1100_data *data)
> +{
> + int data_rate_Hz = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];
> + /* To be sure we wait 5 times more than data rate */
> + unsigned long wait_time_us = DIV_ROUND_CLOSEST(USEC_PER_SEC, 5 * data_rate_Hz);
> + int data_ready;
> + u8 buffer[3];
> + int ret;
> +
> + /* To be sure that polled value will have value after config change */
> + 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;
> + }
> +
> + ret = readx_poll_timeout(ads1100_new_data_is_ready, data,
> + data_ready, data_ready != 0,
> + wait_time_us, ADS1100_MAX_DRDY_TIMEOUT_US);
[Severity: High]
Does this readx_poll_timeout() call have an inverted loop condition for
data_ready?
Because the condition is data_ready != 0, it appears the driver will break
early when the device is busy, and spin up to 7.5 seconds when the device
is actually ready.
Since this polling state might be impossible in continuous mode, could this
cause the driver to consistently hit the 7.5-second timeout, completely
breaking data rate and scale configuration when userspace writes to the
in_voltage_scale or in_voltage_sampling_frequency sysfs attributes?
[ ... ]
> @@ -149,22 +199,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);
>
[Severity: Medium]
This is a pre-existing issue, but does ads1100_set_scale() suppress I2C write
errors by missing a return value check for ads1100_set_config_bits()?
If I2C bus errors occur during scale configuration (for example, if the bus
is unstable or the device is disconnected), the driver will continue as if
the configuration succeeded.
Could this lead to incorrect data conversion scales being reported to
userspace?
> - return 0;
> + return ads1100_poll_data_ready(data);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714194717.596193-1-jakubszczudlo40@gmail.com?part=1
^ permalink raw reply [flat|nested] 14+ messages in thread