Re: [PATCH v2 1/2] iio: adc: ti-ads112c14: add burnout current support

[Andy Shevchenko]

On Thu, Aug 27, 2026 at 05:27:02PM -0500, David Lechner (TI) wrote:
> Add a custom attribute via ext_info when a channel has a burnout current
> specified in the devicetree. This adds an in_{voltageY,resistanceY,
> voltageY-voltageX}_burnoutraw sysfs attribute for the channel that
> performs a single conversion (same as _raw attribute) except that it
> enables the burnout current. The chip also has a restriction that input
> chopping cannot be enabled when burnout current is enabled, so we also
> disable input chopping when burnout current is active.

...

>  	ret = regmap_update_bits(data->regmap, ADS112C14_REG_DATA_RATE_CFG,
>  				 ADS112C14_DATA_RATE_CFG_GC_EN,
>  				 FIELD_PREP(ADS112C14_DATA_RATE_CFG_GC_EN,
> -					    measurement->global_chop));
> +					    measurement->global_chop && !en_burnout));

Strictly speaking the argument for FIELD_PREP should be integer and not boolean.

>  	if (ret)
>  		return ret;

...

> +static ssize_t ads112c14_read_burnout_raw(struct iio_dev *indio_dev,
> +					  uintptr_t private,
> +					  struct iio_chan_spec const *chan,
> +					  char *buf)
> +{
> +	struct ads112c14_data *data = iio_priv(indio_dev);
> +	struct ads112c14_measurement *measurement;
> +	int ret, ret2, val;
> +	u8 raw_buf[3];
> +
> +	if (chan->channel >= ADS112C14_SYS_MON_CHANNEL_BASE)
> +		return -EINVAL;
> +
> +	measurement = &data->measurements[chan->scan_index];

> +

Unneeded blank line.

> +	if (!measurement->burnout)
> +		return -EINVAL;
> +
> +	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> +	if (IIO_DEV_ACQUIRE_FAILED(claim))
> +		return -EBUSY;
> +
> +	ret = regmap_update_bits(data->regmap, ADS112C14_REG_DEVICE_CFG,
> +				 ADS112C14_DEVICE_CFG_BOCS,
> +				 FIELD_PREP(ADS112C14_DEVICE_CFG_BOCS,
> +					    measurement->burnout));
> +	if (ret)
> +		return ret;
> +
> +	ret = ads112c14_single_conversion(data, chan, raw_buf, true, false);
> +
> +	/*
> +	 * Important to always turn off burnout current even if the conversion
> +	 * fails so that it does not affect subsequent measurements. This error
> +	 * also takes precedence over the conversion error since the device may
> +	 * be left in a bad state.
> +	 */
> +	ret2 = regmap_update_bits(data->regmap, ADS112C14_REG_DEVICE_CFG,
> +				  ADS112C14_DEVICE_CFG_BOCS,
> +				  FIELD_PREP(ADS112C14_DEVICE_CFG_BOCS,
> +					     ADS112C14_DEVICE_CFG_BOCS_DISABLED));
> +	if (ret2)
> +		return ret2;

What we will get of sharing this error code instead of 'ret' in case of single
conversion failure? I think there is no recovery mechanism involved, right?

> +	if (ret < 0)
> +		return ret;

> +	switch (data->chip_info->resolution_bits) {
> +	case 16:
> +		val = get_unaligned_be16(raw_buf);
> +		break;
> +	case 24:
> +		val = get_unaligned_be24(raw_buf);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (measurement->bipolar)
> +		val = sign_extend32(val, data->chip_info->resolution_bits - 1);
> +
> +	return sysfs_emit(buf, "%d\n", val);
> +}

And this is after all a user space interaction, so whatever error code is
returned, user space will know it. I assume you are telling that the restoring
the "burnout current" setting is important, but how will user space distinguish
that case from the single conversion failure?

...

With

	const char *propname;
	...
		propname = "burn-out-current-nanoamp";

> +		if (fwnode_property_present(child, "burn-out-current-nanoamp")) {
> +			u32 burnout_nA;
> +
> +			ret = fwnode_property_read_u32(child, "burn-out-current-nanoamp",
> +						       &burnout_nA);
> +			if (ret)
> +				return dev_err_probe(dev, ret,
> +						     "failed to read burn-out-current-nanoamp property\n");

This can be shortened as well as the other message below.

> +			switch (burnout_nA) {
> +			case 200:
> +				measurement->burnout = ADS112C14_DEVICE_CFG_BOCS_200_nA;
> +				break;
> +			case 1000:
> +				measurement->burnout = ADS112C14_DEVICE_CFG_BOCS_1_uA;
> +				break;
> +			case 10000:
> +				measurement->burnout = ADS112C14_DEVICE_CFG_BOCS_10_uA;
> +				break;
> +			default:
> +				return dev_err_probe(dev, -EINVAL,

I would use different error code, EINVAL is abused and overloaded a lot in the
kernel, and basically errors like ENODEV and EINVAL are synonyms to "*an* error"
happened. Unfortunately, reading the errno*.h I haven't found anything better.

> +						     "invalid burn-out-current-nanoamp value\n");
> +			}
> +
> +			if (measurement->burnout)
> +				spec->ext_info = ads112c14_ext_info_burnout;
> +		}

-- 
With Best Regards,
Andy Shevchenko