Re: [PATCH 4/6] iio: adc: ad7173: refactor ain and vref selection

Jonathan Cameron
2024-04-06T15:04:05Z

On Mon, 01 Apr 2024 18:32:22 +0300
Dumitru Ceclan via B4 Relay <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:

> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>
> Move validation of analog inputs and reference voltage selection to
> separate functions.
>
> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
A few line wrapping comments inline.

> ---
> drivers/iio/adc/ad7173.c | 59 +++++++++++++++++++++++++++++++++---------------
> 1 file changed, 41 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
> index 699bc6970790..bf5a5b384fe2 100644
> --- a/drivers/iio/adc/ad7173.c
> +++ b/drivers/iio/adc/ad7173.c
> @@ -910,6 +910,41 @@ static int ad7173_register_clk_provider(struct iio_dev *indio_dev)
> &st->int_clk_hw);
> }
>
> +static int ad7173_validate_voltage_ain_inputs(struct ad7173_state *st,
> + unsigned int ain[2])
> +{
> + struct device *dev = &st->sd.spi->dev;
> +
> + if (ain[0] >= st->info->num_inputs ||
> + ain[1] >= st->info->num_inputs)

No need to line wrap the above - its under 80 chars on one line with the
new reduced indent due to factoring this out.

> + return dev_err_probe(dev, -EINVAL,
> + "Input pin number out of range for pair (%d %d).\n",
> + ain[0], ain[1]);
> +
> + return 0;
> +}
> +
> +static int ad7173_validate_reference(struct ad7173_state *st, int ref_sel)
> +{
> + struct device *dev = &st->sd.spi->dev;
> + int ret;
> +
> + if (ref_sel == AD7173_SETUP_REF_SEL_INT_REF && !st->info->has_int_ref)
> + return dev_err_probe(dev, -EINVAL,
> + "Internal reference is not available on current model.\n");
> +
> + if (ref_sel == AD7173_SETUP_REF_SEL_EXT_REF2 && !st->info->has_ref2)
> + return dev_err_probe(dev, -EINVAL,
> + "External reference 2 is not available on current model.\n");
> +
> + ret = ad7173_get_ref_voltage_milli(st, ref_sel);
> + if (ret < 0)
> + return dev_err_probe(dev, ret,
> + "Cannot use reference %u\n", ref_sel);

Can pull the string to previous line and then align ref_sel just after (
whilst still remaining under 80 chars and end up a little prettier.

> +
> + return 0;
> +}
> +
> static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
> {
> struct ad7173_channel *chans_st_arr, *chan_st_priv;
> @@ -970,11 +1005,9 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
> if (ret)
> return ret;
>
> - if (ain[0] >= st->info->num_inputs ||
> - ain[1] >= st->info->num_inputs)
> - return dev_err_probe(dev, -EINVAL,
> - "Input pin number out of range for pair (%d %d).\n",
> - ain[0], ain[1]);
> + ret = ad7173_validate_voltage_ain_inputs(st, ain);
> + if (ret)
> + return ret;
>
> ret = fwnode_property_match_property_string(child,
> "adi,reference-select",
> @@ -985,19 +1018,9 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
> else
> ref_sel = ret;
>
> - if (ref_sel == AD7173_SETUP_REF_SEL_INT_REF &&
> - !st->info->has_int_ref)
> - return dev_err_probe(dev, -EINVAL,
> - "Internal reference is not available on current model.\n");
> -
> - if (ref_sel == AD7173_SETUP_REF_SEL_EXT_REF2 && !st->info->has_ref2)
> - return dev_err_probe(dev, -EINVAL,
> - "External reference 2 is not available on current model.\n");
> -
> - ret = ad7173_get_ref_voltage_milli(st, ref_sel);
> - if (ret < 0)
> - return dev_err_probe(dev, ret,
> - "Cannot use reference %u\n", ref_sel);
> + ret = ad7173_validate_reference(st, ref_sel);
> + if (ret)
> + return ret;
>
> if (ref_sel == AD7173_SETUP_REF_SEL_INT_REF)
> st->adc_mode |= AD7173_ADC_MODE_REF_EN;
>