On Thu, 3 Oct 2019 19:33:58 +0200
Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:
> Maxim's max1027/29/31 series returns the measured voltages with a
> resolution of 10 bits. There is a very similar series, max1227/29/31
> which works identically but uses a resolution of 12 bits. Prepare the
> support for these chips by turning the 'depth' into a macro parameter
> instead of hardcoding it everywhere. Also reorganize just a bit the
> macros at the top to avoid repeating tens of lines when adding support
> for a new chip.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
Minor comments inline.
Jonathan
> ---
> drivers/iio/adc/max1027.c | 78 ++++++++++++++++++---------------------
> 1 file changed, 36 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/iio/adc/max1027.c b/drivers/iio/adc/max1027.c
> index f9b473ee6711..5d5d223dd42a 100644
> --- a/drivers/iio/adc/max1027.c
> +++ b/drivers/iio/adc/max1027.c
> @@ -83,7 +83,7 @@ static const struct of_device_id max1027_adc_dt_ids[] = {
> MODULE_DEVICE_TABLE(of, max1027_adc_dt_ids);
> #endif
>
> -#define MAX1027_V_CHAN(index) \
> +#define MAX1027_V_CHAN(index, depth) \
> { \
> .type = IIO_VOLTAGE, \
> .indexed = 1, \
> @@ -93,7 +93,7 @@ MODULE_DEVICE_TABLE(of, max1027_adc_dt_ids);
> .scan_index = index + 1, \
> .scan_type = { \
> .sign = 'u', \
> - .realbits = 10, \
> + .realbits = depth, \
> .storagebits = 16, \
> .shift = 2, \
> .endianness = IIO_BE, \
> @@ -115,52 +115,42 @@ MODULE_DEVICE_TABLE(of, max1027_adc_dt_ids);
> }, \
> }
>
> +#define MAX1X27_CHANNELS(depth) \
> + MAX1027_T_CHAN, \
> + MAX1027_V_CHAN(0, depth), \
> + MAX1027_V_CHAN(1, depth), \
> + MAX1027_V_CHAN(2, depth), \
> + MAX1027_V_CHAN(3, depth), \
> + MAX1027_V_CHAN(4, depth), \
> + MAX1027_V_CHAN(5, depth), \
> + MAX1027_V_CHAN(6, depth), \
> + MAX1027_V_CHAN(7, depth)
> +
> +#define MAX1X29_CHANNELS(depth) \
> + MAX1027_V_CHAN(8, depth), \
> + MAX1027_V_CHAN(9, depth), \
> + MAX1027_V_CHAN(10, depth), \
> + MAX1027_V_CHAN(11, depth)
> +
Modify this a touch so the macro for MAX1X29_CHANNELS includes
MAX1X27_CHANNELS. That way each macro's name matches what it
does rather than the 'additional channels' for that device.
> +#define MAX1X31_CHANNELS(depth) \
> + MAX1027_V_CHAN(12, depth), \
> + MAX1027_V_CHAN(13, depth), \
> + MAX1027_V_CHAN(14, depth), \
> + MAX1027_V_CHAN(15, depth)
> +
> static const struct iio_chan_spec max1027_channels[] = {
> - MAX1027_T_CHAN,
> - MAX1027_V_CHAN(0),
> - MAX1027_V_CHAN(1),
> - MAX1027_V_CHAN(2),
> - MAX1027_V_CHAN(3),
> - MAX1027_V_CHAN(4),
> - MAX1027_V_CHAN(5),
> - MAX1027_V_CHAN(6),
> - MAX1027_V_CHAN(7)
> + MAX1X27_CHANNELS(10)
> };
>
> static const struct iio_chan_spec max1029_channels[] = {
> - MAX1027_T_CHAN,
> - MAX1027_V_CHAN(0),
> - MAX1027_V_CHAN(1),
> - MAX1027_V_CHAN(2),
> - MAX1027_V_CHAN(3),
> - MAX1027_V_CHAN(4),
> - MAX1027_V_CHAN(5),
> - MAX1027_V_CHAN(6),
> - MAX1027_V_CHAN(7),
> - MAX1027_V_CHAN(8),
> - MAX1027_V_CHAN(9),
> - MAX1027_V_CHAN(10),
> - MAX1027_V_CHAN(11)
> + MAX1X27_CHANNELS(10),
> + MAX1X29_CHANNELS(10)
> };
>
> static const struct iio_chan_spec max1031_channels[] = {
> - MAX1027_T_CHAN,
> - MAX1027_V_CHAN(0),
> - MAX1027_V_CHAN(1),
> - MAX1027_V_CHAN(2),
> - MAX1027_V_CHAN(3),
> - MAX1027_V_CHAN(4),
> - MAX1027_V_CHAN(5),
> - MAX1027_V_CHAN(6),
> - MAX1027_V_CHAN(7),
> - MAX1027_V_CHAN(8),
> - MAX1027_V_CHAN(9),
> - MAX1027_V_CHAN(10),
> - MAX1027_V_CHAN(11),
> - MAX1027_V_CHAN(12),
> - MAX1027_V_CHAN(13),
> - MAX1027_V_CHAN(14),
> - MAX1027_V_CHAN(15)
> + MAX1X27_CHANNELS(10),
> + MAX1X29_CHANNELS(10),
> + MAX1X31_CHANNELS(10)
> };
>
> static const unsigned long max1027_available_scan_masks[] = {
> @@ -181,6 +171,7 @@ static const unsigned long max1031_available_scan_masks[] = {
> struct max1027_chip_info {
> const struct iio_chan_spec *channels;
> unsigned int num_channels;
> + unsigned int depth;
Could we use the channel real_bits field instead of replicating the info?
> const unsigned long *available_scan_masks;
> };
>
> @@ -188,16 +179,19 @@ static const struct max1027_chip_info max1027_chip_info_tbl[] = {
> [max1027] = {
> .channels = max1027_channels,
> .num_channels = ARRAY_SIZE(max1027_channels),
> + .depth = 10,
> .available_scan_masks = max1027_available_scan_masks,
> },
> [max1029] = {
> .channels = max1029_channels,
> .num_channels = ARRAY_SIZE(max1029_channels),
> + .depth = 10,
> .available_scan_masks = max1029_available_scan_masks,
> },
> [max1031] = {
> .channels = max1031_channels,
> .num_channels = ARRAY_SIZE(max1031_channels),
> + .depth = 10,
> .available_scan_masks = max1031_available_scan_masks,
> },
> };
> @@ -284,7 +278,7 @@ static int max1027_read_raw(struct iio_dev *indio_dev,
> break;
> case IIO_VOLTAGE:
> *val = 2500;
> - *val2 = 10;
> + *val2 = st->info->depth;
> ret = IIO_VAL_FRACTIONAL_LOG2;
> break;
> default:
