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

[Jonathan Cameron]

> 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.
> 
> Signed-off-by: David Lechner (TI) <dlechner@xxxxxxxxxxxx>

Hi David,

A few trivial things inline.

Thanks,

Jonathan
>
> diff --git a/drivers/iio/adc/ti-ads112c14.c b/drivers/iio/adc/ti-ads112c14.c
> index 878764deffc5..58c815f5faeb 100644
> --- a/drivers/iio/adc/ti-ads112c14.c
> +++ b/drivers/iio/adc/ti-ads112c14.c
> @@ -76,6 +76,11 @@
>  #define   ADS112C14_DEVICE_CFG_PWDN			BIT(7)
>  #define   ADS112C14_DEVICE_CFG_STBY_MODE		BIT(6)
>  #define   ADS112C14_DEVICE_CFG_BOCS			GENMASK(5, 4)
> +#define     ADS112C14_DEVICE_CFG_BOCS_DISABLED		  0
> +#define     ADS112C14_DEVICE_CFG_BOCS_200_nA		  1
> +#define     ADS112C14_DEVICE_CFG_BOCS_1_uA		  2
> +#define     ADS112C14_DEVICE_CFG_BOCS_10_uA		  3

...


>  static int ads112c14_populate_idac_mag(u32 current_nA, u8 *idac_mag)
>  {
>  	u32 current_uA = current_nA / (NANO / MICRO);
> @@ -1285,6 +1365,34 @@ static int ads112c14_parse_channels(struct iio_dev *indio_dev,
>  		measurement->global_chop = fwnode_property_read_bool(child,
>  								     "input-chopping");
>
> +		if (fwnode_property_present(child, "burn-out-current-nanoamp")) {

Andy already covered this.  I kind of fall on the side of having the
text inline for grepping when there are only 1 or two instances, but
when we reach 3 it seems like the time has come to use a local
variable.

> +			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");
> +
> +			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,
> +						     "invalid burn-out-current-nanoamp value\n");
> +			}
> +
> +			if (measurement->burnout)

I would use an explicit match against
!= ADS112C14_DEVICE_CFG_BOCS_DISABLED
here rather than non 0.

-- 
Jonathan Cameron <jonathan.cameron@xxxxxxxxxxxxxxxx>