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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240406160350.751d23ae@jic23-huawei>
Date: Sat, 6 Apr 2024 16:03:50 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Dumitru Ceclan via B4 Relay
 <devnull+dumitru.ceclan.analog.com@...nel.org>
Cc: dumitru.ceclan@...log.com, Lars-Peter Clausen <lars@...afoo.de>, Michael
 Hennerich <Michael.Hennerich@...log.com>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley
 <conor+dt@...nel.org>, David Lechner <dlechner@...libre.com>,
 linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, Dumitru Ceclan <mitrutzceclan@...il.com>
Subject: Re: [PATCH 4/6] iio: adc: ad7173: refactor ain and vref selection

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

> From: Dumitru Ceclan <dumitru.ceclan@...log.com>
> 
> Move validation of analog inputs and reference voltage selection to
> separate functions.
> 
> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@...log.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;
>