Re: [PATCH v1 2/2] iio: adc: add MAX40080 current-sense amplifier driver

[Jonathan Cameron <jic23@kernel.org>]

> > +/*
> > + * The RANGE field (CFG bit 6) selects one of two current-sense full-scale
> > + * ranges (the MAX40080 supports exactly two: +/-50 mV and +/-10 mV). Ordered
> > + * so that the array index equals the RANGE field value: index 0 = 50 mV range
> > + * (gain 25 V/V), index 1 = 10 mV range (gain 125 V/V).
> > + */
> > +static const int max40080_csa_gain[] = {
> > +	MAX40080_CSA_50MV_GAIN, MAX40080_CSA_10MV_GAIN,
> > +};  
> 
> With a given comment and one time use of those definitions, do we need them
> at all?  I leave it to Jonathan, because my memory is weak on his preference
> in the cases like this.
> 

Defines for the indexes would be good so the ordering comment isn't needed.
The define as currently used don't benefit anyone.

static const int max400080_csa_gain[] = {
	[MAX400080_CFG_RANGE_50MV] = 25,
	[MAX400080_CFG_RANGE_10MV] = 125,
};

Related somewhat
> +#define MAX40080_REG_CFG		0x00
> +#define  MAX40080_MODE_MSK		GENMASK(2, 0)
> +#define  MAX40080_PEC_EN_MSK		BIT(5)
> +#define  MAX40080_RANGE_MSK		BIT(6)
> +#define  MAX40080_FILTER_MSK		GENMASK(14, 12)

The fields should be named so that it's is obvious which register they are in
then add field value defines where they are relevant - again naming them
to make register and field part of the name.

#define MAX400080_REG_CFG ...
#define   MAX40080_CFG_MODE_MSK ...
...
#define   MAX40080_CFG_RANGE_MSK
#define     MAX40080_CFG_RANGE_50MV	0
#define     MAX40080_CFG_RANGE_10MV     1

> ...