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

[David Lechner]

On 8/28/26 2:38 AM, Andy Shevchenko wrote:
> 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.
> 

...

>> +	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?

I think returning ret here is better.

> 
>> +	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?
> 

This is not expected to ever fail, so any error just tells userspace that
hardware is broken. Any error here is not really recoverable without
resetting the hardware.

>> +			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.

There is an error message to disambiguate.
> 
>> +						     "invalid burn-out-current-nanoamp value\n");
>> +			}
>> +
>> +			if (measurement->burnout)
>> +				spec->ext_info = ads112c14_ext_info_burnout;
>> +		}
>