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

[Andy Shevchenko]

On Tue, Jun 23, 2026 at 12:15:50AM +0200, Jakub Szczudlo wrote:
> Add ADS1110 support that have faster datarate than ADS1100, it also uses
> internal voltage reference of 2.048V for measurement.

...

>  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 TI ADS1100 and similar single
> +	  channel I2C Analog to Digital Converters.

User won't know what similar are really supported. The rule of thumb is to add
the list of supported here as

	  - ADS1000 (...perhaps some very short spec info...)
	  - ADS1100 (...perhaps some very short spec info...)


>  	  This driver can also be built as a module. If so, the module will be
>  	  called ti-ads1100.

...

> +static int ads1100_get_vref_milivolts(struct ads1100_data *data)
> +{
> +	if (data->ads_config->has_internal_vref_only)
> +		return ADS1110_INTERNAL_REF_mV;
> +
> +	return regulator_get_voltage(data->reg_vdd) / MILLI;

For now we used "(MICRO / MILLI)" instead of "MILLI", to show the unit
conversion.

> +}

...

>  	if (ret < 0) {
>  		dev_err(&data->client->dev, "I2C read fail: %d\n", ret);
>  		return ret;

> +	} else if (ret < 2) {

Redundant 'else'.

> +		dev_err(&data->client->dev, "Short I2C read\n");
> +		return -EIO;
>  	}

...

> -	microvolts = regulator_get_voltage(data->reg_vdd);
> +	microvolts = ads1100_get_vref_milivolts(data) * (MICRO / MILLI);

See above, here you correctly used the existing pattern, the above is
inconsistent and needs to be addressed.

...

> +	model = i2c_get_match_data(client);
> +	if (!model)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "Can't get device data from firmware\n");
> +
> +	data->ads_config = (struct ads1100_config *)model;

You can't drop const like this. If you need to apply modification,
use devm_kmemdup(). Otherwise it won't work correctly if you have two different
sensors of the same driver in the system.

-- 
With Best Regards,
Andy Shevchenko