[PATCH 0/3] iio: adc: Extend ti-ads1100 driver

[PATCH 0/3] iio: adc: Extend ti-ads1100 driver

[Jakub Szczudlo]

Extend ADS 1100 driver to support ADS1110, which is a pin-to-pin 
compatible device with higher resolution. This patch also updates the 
device tree bindings and Kconfig description to reflect the new 
supported device.

Signed-off-by: jszczudlo <jakubszczudlo40@gmail.com>
---
jszczudlo (3):
  dt-bindings: iio: adc: Update title and enum
  iio: adc: Update Kconfig description for TI_ADS1100
  iio: adc: Add ti-ads1110 support to ti-ads1100 driver

 .../bindings/iio/adc/ti,ads1100.yaml          |   3 +-
 drivers/iio/adc/Kconfig                       |   6 +-
 drivers/iio/adc/ti-ads1100.c                  | 165 +++++++++++++-----
 3 files changed, 131 insertions(+), 43 deletions(-)

-- 
2.47.3

[PATCH 1/3] dt-bindings: iio: adc: Update title and enum

[Jakub Szczudlo]

From: jszczudlo <jakubszczudlo40@gmail.com>

Add ads1110 into title and enum compatible for ads1100 driver.

Signed-off-by: jszczudlo <jakubszczudlo40@gmail.com>
---
 Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml
index 970ccab15e1e..4c43996b7ec6 100644
--- a/Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml
@@ -4,7 +4,7 @@
 $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/ADS1000/ADS1110 single channel I2C analog to digital converter
 
 maintainers:
   - Mike Looijmans <mike.looijmans@topic.nl>
@@ -17,6 +17,7 @@ properties:
     enum:
       - ti,ads1100
       - ti,ads1000
+      - ti,ads1110
 
   reg:
     maxItems: 1
-- 
2.47.3

[PATCH 2/3] iio: adc: Update Kconfig description for TI_ADS1100

[Jakub Szczudlo]

From: jszczudlo <jakubszczudlo40@gmail.com>

Add mention of ads1110 in Kconfig for TI_ADS1100

Signed-off-by: jszczudlo <jakubszczudlo40@gmail.com>
---
 drivers/iio/adc/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index a9dedbb8eb46..ad2a276ce0f8 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1747,11 +1747,11 @@ config TI_ADS1018
          called ti-ads1018.
 
 config TI_ADS1100
-	tristate "Texas Instruments ADS1100 and ADS1000 ADC"
+	tristate "Texas Instruments ADS1100, ADS1000 and ADS1110 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 Texas Instruments ADS1100,
+	  ADS1000 and ADS1110 ADC chips.
 
 	  This driver can also be built as a module. If so, the module will be
 	  called ti-ads1100.
-- 
2.47.3

[PATCH 3/3] iio: adc: Add ti-ads1110 support to ti-ads1100 driver

[Jakub Szczudlo]

From: jszczudlo <jakubszczudlo40@gmail.com>

add ADS1100 support
make changing gain and datarate wait for new reading
fix unbalanced regulator disable when removing in singleshot mode

Signed-off-by: jszczudlo <jakubszczudlo40@gmail.com>
---
 drivers/iio/adc/ti-ads1100.c | 165 ++++++++++++++++++++++++++---------
 1 file changed, 126 insertions(+), 39 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
index aa8946063c7d..11d6fe1e8abc 100644
--- a/drivers/iio/adc/ti-ads1100.c
+++ b/drivers/iio/adc/ti-ads1100.c
@@ -5,18 +5,15 @@
  * 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, ADS1000 and ADS1110 ADC 16-bit I2C
  */
 
 #include <linux/bitfield.h>
 #include <linux/bits.h>
-#include <linux/cleanup.h>
-#include <linux/delay.h>
-#include <linux/module.h>
-#include <linux/init.h>
 #include <linux/i2c.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
 #include <linux/mutex.h>
-#include <linux/property.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 #include <linux/units.h>
@@ -39,17 +36,39 @@
 #define	ADS1100_SINGLESHOT	ADS1100_CFG_SC
 
 #define ADS1100_SLEEP_DELAY_MS	2000
+#define ADS1110_REFERENCE_VOLTAGE_MICROVOLT 2048000
+
+/* Timeout based on the minimum sample rate of 8 SPS (7500000us) */
+#define ADS11x0_MAX_DRDY_TIMEOUT	7500000
 
 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 };
 
+struct ads11x0_config {
+	const int *data_rate;
+	bool has_reference_voltage;
+};
+
+static const struct ads11x0_config ads1100_config = {
+	.data_rate = ads1100_data_rate,
+	.has_reference_voltage = false,
+};
+
+static const struct ads11x0_config ads1110_config = {
+	.data_rate = ads1110_data_rate,
+	.has_reference_voltage = true,
+};
+
 struct ads1100_data {
 	struct i2c_client *client;
 	struct regulator *reg_vdd;
 	struct mutex lock;
 	int scale_avail[2 * 4]; /* 4 gain settings */
 	u8 config;
-	bool supports_data_rate; /* Only the ADS1100 can select the rate */
+	bool supports_data_rate; /* Only the ADS1100/ADS1110 can select the rate */
+	bool has_reference_voltage; /* The ADS1110 has an internal reference, so fixed scale */
+	const int *data_rate;
 };
 
 static const struct iio_chan_spec ads1100_channel = {
@@ -59,12 +78,6 @@ static const struct iio_chan_spec ads1100_channel = {
 	    BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
 	.info_mask_shared_by_all_available =
 	    BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
-	.scan_type = {
-		      .sign = 's',
-		      .realbits = 16,
-		      .storagebits = 16,
-		      .endianness = IIO_CPU,
-		       },
 	.datasheet_name = "AIN",
 };
 
@@ -85,6 +98,50 @@ static int ads1100_set_config_bits(struct ads1100_data *data, u8 mask, u8 value)
 	return 0;
 };
 
+static int ads11x0_get_voltage_microvolts(struct ads1100_data *data)
+{
+	if (data->has_reference_voltage)
+		return ADS1110_REFERENCE_VOLTAGE_MICROVOLT;
+	else
+		return regulator_get_voltage(data->reg_vdd);
+}
+
+static int ads11x0_get_voltage_milivolts(struct ads1100_data *data)
+{
+	return ads11x0_get_voltage_microvolts(data) / (MICRO / MILLI);
+}
+
+static bool ads11x0_new_data_ready(struct ads1100_data *data)
+{
+	int ret;
+	u8 buffer[3];
+
+	ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
+	if (ret < sizeof(buffer)) {
+		dev_err(&data->client->dev, "I2C read fail: %d\n", ret);
+		return 0;
+	}
+
+	int return_val = FIELD_GET(ADS1100_CFG_ST_BSY, buffer[2]);
+
+	return return_val;
+}
+
+static int ads11x0_poll_data_ready(struct ads1100_data *data)
+{
+	bool data_ready;
+	u8 buffer[3];
+	int datarate = data->data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];
+	unsigned long wait_time = DIV_ROUND_CLOSEST(MICRO, 5 * datarate);
+
+	/* To be sure that polled value will have value after config change */
+	i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
+
+	return read_poll_timeout(ads11x0_new_data_ready, data_ready,
+				 !data_ready, wait_time,
+				 ADS11x0_MAX_DRDY_TIMEOUT, false, data);
+}
+
 static int ads1100_data_bits(struct ads1100_data *data)
 {
 	return ads1100_data_rate_bits[FIELD_GET(ADS1100_DR_MASK, data->config)];
@@ -105,9 +162,10 @@ static int ads1100_get_adc_result(struct ads1100_data *data, int chan, int *val)
 
 	ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
 
+	pm_runtime_mark_last_busy(&data->client->dev);
 	pm_runtime_put_autosuspend(&data->client->dev);
 
-	if (ret < 0) {
+	if (ret < sizeof(buffer)) {
 		dev_err(&data->client->dev, "I2C read fail: %d\n", ret);
 		return ret;
 	}
@@ -127,7 +185,7 @@ 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)
 		return -EINVAL;
@@ -135,7 +193,7 @@ static int ads1100_set_scale(struct ads1100_data *data, int val, int val2)
 	if (!val2)
 		return -EINVAL;
 
-	microvolts = regulator_get_voltage(data->reg_vdd);
+	microvolts = ads11x0_get_voltage_microvolts(data);
 	/*
 	 * val2 is in 'micro' units, n = val2 / 1000000
 	 * result must be millivolts, d = microvolts / 1000
@@ -147,34 +205,49 @@ static int ads1100_set_scale(struct ads1100_data *data, int val, int val2)
 	if (gain < BIT(0) || gain > BIT(3))
 		return -EINVAL;
 
+	ret = pm_runtime_resume_and_get(&data->client->dev);
+	if (ret < 0)
+		return ret;
+
 	ads1100_set_config_bits(data, ADS1100_PGA_MASK, ffs(gain) - 1);
 
-	return 0;
+	ret = ads11x0_poll_data_ready(data);
+
+	pm_runtime_mark_last_busy(&data->client->dev);
+	pm_runtime_put_autosuspend(&data->client->dev);
+
+	return ret;
 }
 
 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));
+		if (data->data_rate[i] == rate) {
+			ret = pm_runtime_resume_and_get(&data->client->dev);
+			if (ret < 0)
+				return ret;
+
+			ads1100_set_config_bits(data, ADS1100_DR_MASK,
+						FIELD_PREP(ADS1100_DR_MASK, i));
+			ret = ads11x0_poll_data_ready(data);
+
+			pm_runtime_mark_last_busy(&data->client->dev);
+			pm_runtime_put_autosuspend(&data->client->dev);
+			return ret;
+		}
 	}
 
 	return -EINVAL;
 }
 
-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 = ads11x0_get_voltage_milivolts(data);
 	unsigned int i;
 
 	for (i = 0; i < ARRAY_SIZE(data->scale_avail) / 2; i++) {
@@ -196,7 +269,7 @@ 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->data_rate;
 		if (data->supports_data_rate)
 			*length = ARRAY_SIZE(ads1100_data_rate);
 		else
@@ -233,12 +306,11 @@ 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 = ads11x0_get_voltage_milivolts(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)];
+		*val = data->data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];
 		return IIO_VAL_INT;
 	default:
 		return -EINVAL;
@@ -280,8 +352,8 @@ static int ads1100_setup(struct ads1100_data *data)
 		return ret;
 
 	ret = i2c_master_recv(data->client, buffer, sizeof(buffer));
-	if (ret < 0)
-		return ret;
+	if (ret < sizeof(buffer))
+		return -1;
 
 	/* Config register returned in third byte, strip away the busy status */
 	data->config = buffer[2] & ~ADS1100_CFG_ST_BSY;
@@ -292,9 +364,13 @@ static int ads1100_setup(struct ads1100_data *data)
 	return 0;
 }
 
-static void ads1100_reg_disable(void *reg)
+static void ads1100_reg_disable(void *data)
 {
-	regulator_disable(reg);
+	struct ads1100_data *ads1100_data = data;
+
+	/* Disable when not already disabled by the driver */
+	if (!(ads1100_data->config & ADS1100_CFG_SC))
+		regulator_disable(ads1100_data->reg_vdd);
 }
 
 static void ads1100_disable_continuous(void *data)
@@ -307,6 +383,7 @@ static int ads1100_probe(struct i2c_client *client)
 	struct iio_dev *indio_dev;
 	struct ads1100_data *data;
 	struct device *dev = &client->dev;
+	const struct ads11x0_config *model;
 	int ret;
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
@@ -334,10 +411,18 @@ static int ads1100_probe(struct i2c_client *client)
 		return dev_err_probe(dev, ret,
 				     "Failed to enable vdd regulator\n");
 
-	ret = devm_add_action_or_reset(dev, ads1100_reg_disable, data->reg_vdd);
+	ret = devm_add_action_or_reset(dev, ads1100_reg_disable, data);
 	if (ret)
 		return ret;
 
+	model = device_get_match_data(dev);
+	if (!model)
+		return dev_err_probe(dev, ret,
+				"Can't set device data\n");
+
+	data->data_rate = model->data_rate;
+	data->has_reference_voltage = model->has_reference_voltage;
+
 	ret = ads1100_setup(data);
 	if (ret)
 		return dev_err_probe(dev, ret,
@@ -400,16 +485,18 @@ static DEFINE_RUNTIME_DEV_PM_OPS(ads1100_pm_ops,
 				 NULL);
 
 static const struct i2c_device_id ads1100_id[] = {
-	{ "ads1100" },
-	{ "ads1000" },
+	{ .name = "ads1100", .driver_data = (kernel_ulong_t)&ads1100_config },
+	{ .name = "ads1000", .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,ads1100", .data = &ads1100_config },
+	{ .compatible = "ti,ads1000", .data = &ads1100_config },
+	{ .compatible = "ti,ads1110", .data = &ads1110_config },
 	{ }
 };
 
-- 
2.47.3

Re: [PATCH 3/3] iio: adc: Add ti-ads1110 support to ti-ads1100 driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] I2C read errors are silently ignored due to implicit type conversion (signed `int` compared to `size_t`), causing the driver to process uninitialized stack data.
- [High] Partial I2C reads cause a positive return value to bypass caller error checks, leaking uninitialized kernel stack memory to userspace.
- [High] The `vdd` regulator is leaked during device unbind due to reversed devres execution order.
- [High] The data polling logic instantly terminates in continuous mode, failing to wait for readings and silently swallowing I2C communication failures.
- [High] Probe returns success (0) instead of an error code when device match data is missing, leading to an uninitialized device state and a subsequent NULL pointer dereference.
- [Medium] I2C write errors are ignored when changing the data rate or scale, silently leaving the software state out of sync with hardware.
--

commit 08fffdde41976a365855c8545a761a652c4906e0
Author: jszczudlo <jakubszczudlo40@gmail.com>

iio: adc: Add ti-ads1110 support to ti-ads1100 driver
    
add ADS1100 support
make changing gain and datarate wait for new reading
fix unbalanced regulator disable when removing in singleshot mode

> diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
> index aa8946063c7d6..11d6fe1e8abc2 100644
> --- a/drivers/iio/adc/ti-ads1100.c
> +++ b/drivers/iio/adc/ti-ads1100.c
[ ... ]
> @@ -85,6 +98,50 @@ static int ads1100_set_config_bits(struct ads1100_data *data, u8 mask, u8 value)
[ ... ]
> +static bool ads11x0_new_data_ready(struct ads1100_data *data)
> +{
> +	int ret;
> +	u8 buffer[3];
> +
> +	ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
> +	if (ret < sizeof(buffer)) {

[Severity: High]
Does this implicit type conversion mask negative error codes?

`sizeof(buffer)` evaluates to a `size_t`. When `ret` is compared to 
`sizeof(buffer)`, it gets promoted to an unsigned value. 

If `i2c_master_recv()` returns a negative error code like `-EIO`, it becomes
a massive positive number, bypassing the error check and allowing the driver
to process uninitialized stack data.

This same signedness comparison issue appears in `ads1100_get_adc_result()` 
and `ads1100_setup()`.

> +		dev_err(&data->client->dev, "I2C read fail: %d\n", ret);
> +		return 0;
> +	}
> +
> +	int return_val = FIELD_GET(ADS1100_CFG_ST_BSY, buffer[2]);
> +
> +	return return_val;

[Severity: High]
Will this polling logic ever actually wait?

According to the datasheet, the ST/BSY bit is always read as 0 in continuous 
conversion mode. If `ads11x0_new_data_ready()` always extracts a 0, 
`data_ready` will be 0.

When `read_poll_timeout()` checks `!data_ready`, it evaluates to true
immediately on the first iteration, causing the loop to exit without waiting.

> +}
[ ... ]
> @@ -105,9 +162,10 @@ static int ads1100_get_adc_result(struct ads1100_data *data, int chan, int *val)
[ ... ]
>  	ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
>  
> +	pm_runtime_mark_last_busy(&data->client->dev);
>  	pm_runtime_put_autosuspend(&data->client->dev);
>  
> -	if (ret < 0) {
> +	if (ret < sizeof(buffer)) {
>  		dev_err(&data->client->dev, "I2C read fail: %d\n", ret);
>  		return ret;

[Severity: High]
If a partial read occurs, could this leak uninitialized stack memory to
userspace?

If `i2c_master_recv()` returns a positive short read length (like 1), this
error path correctly catches it and returns `ret` (which is 1).

However, the caller `ads1100_read_raw()` expects negative error codes:

drivers/iio/adc/ti-ads1100.c:ads1100_read_raw() {
    ...
    ret = ads1100_get_adc_result(data, chan->address, val);
    iio_device_release_direct(indio_dev);
    if (ret < 0)
        return ret;
        
    return IIO_VAL_INT;
}

Because `ret` is positive, `ads1100_read_raw()` will bypass its error check
and return `IIO_VAL_INT` (success) while `*val` remains uninitialized.

>  	}
[ ... ]
> @@ -127,7 +185,7 @@ 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;
> +	ret = ads11x0_poll_data_ready(data);

[Severity: Medium]
Is it intentional to discard the return value of `ads1100_set_config_bits()`
here?

If the I2C write fails, the hardware and software state will be out of sync,
but the driver will proceed to poll and falsely report successful
configuration changes. The same issue appears in `ads1100_set_data_rate()`.

> +
> +	pm_runtime_mark_last_busy(&data->client->dev);
> +	pm_runtime_put_autosuspend(&data->client->dev);
> +
> +	return ret;
>  }
[ ... ]
> @@ -292,9 +364,13 @@ static int ads1100_setup(struct ads1100_data *data)
>  	return 0;
>  }
>  
> -static void ads1100_reg_disable(void *reg)
> +static void ads1100_reg_disable(void *data)
>  {
> -	regulator_disable(reg);
> +	struct ads1100_data *ads1100_data = data;
> +
> +	/* Disable when not already disabled by the driver */
> +	if (!(ads1100_data->config & ADS1100_CFG_SC))
> +		regulator_disable(ads1100_data->reg_vdd);
>  }

[Severity: High]
Could this leak the `vdd` regulator during device unbind due to the devres
execution order?

Devres executes registered actions in reverse order (LIFO). During unbind,
`ads1100_disable_continuous()` executes first and unconditionally sets
`ADS1100_CFG_SC`.

When `ads1100_reg_disable()` runs immediately after, the `ADS1100_CFG_SC` bit
is already set, causing this condition to fail and permanently skipping the
`regulator_disable()` call.

>  
>  static void ads1100_disable_continuous(void *data)
[ ... ]
> @@ -334,10 +411,18 @@ static int ads1100_probe(struct i2c_client *client)
[ ... ]
> +	model = device_get_match_data(dev);
> +	if (!model)
> +		return dev_err_probe(dev, ret,
> +				"Can't set device data\n");

[Severity: High]
Does this return a success code when `device_get_match_data()` fails?

The preceding call to `devm_add_action_or_reset()` succeeds, leaving `ret`
as exactly 0. 

Passing a 0 `ret` to `dev_err_probe()` will cause the probe function to
return 0 (success) instead of a valid error code (like `-ENODEV`). The
device registers successfully with uninitialized device data, leading to a
NULL pointer dereference when userspace tries to read from the device.

> +
> +	data->data_rate = model->data_rate;
> +	data->has_reference_voltage = model->has_reference_voltage;
> +
>  	ret = ads1100_setup(data);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260607183542.368184-1-jakubszczudlo40@gmail.com?part=3