linux-kernel - Re: [PATCH v2 4/5] iio: adc: ad7380: fix supplies for ad7380-4

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <037d7ebb4d037edb32f9d717e456ab545621ea94.camel@gmail.com>
Date: Mon, 21 Oct 2024 13:22:19 +0200
From: Nuno Sá <noname.nuno@...il.com>
To: Julien Stephan <jstephan@...libre.com>, Lars-Peter Clausen
 <lars@...afoo.de>,  Michael Hennerich <Michael.Hennerich@...log.com>, Nuno
 Sá <nuno.sa@...log.com>, David Lechner	
 <dlechner@...libre.com>, Jonathan Cameron <jic23@...nel.org>, Rob Herring	
 <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley	
 <conor+dt@...nel.org>, Liam Girdwood <lgirdwood@...il.com>, Mark Brown	
 <broonie@...nel.org>, Jonathan Corbet <corbet@....net>
Cc: linux-iio@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Conor Dooley <conor.dooley@...rochip.com>, 
 Jonathan Cameron <Jonathan.Cameron@...wei.com>, linux-doc@...r.kernel.org
Subject: Re: [PATCH v2 4/5] iio: adc: ad7380: fix supplies for ad7380-4

On Mon, 2024-10-21 at 12:00 +0200, Julien Stephan wrote:
> ad7380-4 is the only device in the family that does not have an internal
> reference. It uses "refin" as a required external reference.
> All other devices in the family use "refio"" as an optional external
> reference.
> 
> Fixes: 737413da8704 ("iio: adc: ad7380: add support for ad738x-4 4 channels
> variants")
> Signed-off-by: Julien Stephan <jstephan@...libre.com>
> ---

Hi Julien,

Patch looks good. Sorry if this already came out in the previous version or in
the other patchset you mention but shouldn't this fix come first in the series?

Anyways, for the patch itself:

Reviewed-by: Nuno Sa <nuno.sa@...log.com>

>  drivers/iio/adc/ad7380.c | 36 ++++++++++++++++++++++++++----------
>  1 file changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c
> index
> e257f78d63edd7910fcb936ec5344922f8e70b99..65096717f0dd3ea6a4ff7020bc544d62b84c
> b8fd 100644
> --- a/drivers/iio/adc/ad7380.c
> +++ b/drivers/iio/adc/ad7380.c
> @@ -89,6 +89,7 @@ struct ad7380_chip_info {
>  	bool has_mux;
>  	const char * const *supplies;
>  	unsigned int num_supplies;
> +	bool external_ref_only;
>  	const char * const *vcm_supplies;
>  	unsigned int num_vcm_supplies;
>  	const unsigned long *available_scan_masks;
> @@ -431,6 +432,7 @@ static const struct ad7380_chip_info ad7380_4_chip_info =
> {
>  	.num_simult_channels = 4,
>  	.supplies = ad7380_supplies,
>  	.num_supplies = ARRAY_SIZE(ad7380_supplies),
> +	.external_ref_only = true,
>  	.available_scan_masks = ad7380_4_channel_scan_masks,
>  	.timing_specs = &ad7380_4_timing,
>  };
> @@ -1047,17 +1049,31 @@ static int ad7380_probe(struct spi_device *spi)
>  				     "Failed to enable power supplies\n");
>  	msleep(T_POWERUP_MS);
>  
> -	/*
> -	 * If there is no REFIO supply, then it means that we are using
> -	 * the internal 2.5V reference, otherwise REFIO is reference voltage.
> -	 */
> -	ret = devm_regulator_get_enable_read_voltage(&spi->dev, "refio");
> -	if (ret < 0 && ret != -ENODEV)
> -		return dev_err_probe(&spi->dev, ret,
> -				     "Failed to get refio regulator\n");
> +	if (st->chip_info->external_ref_only) {
> +		ret = devm_regulator_get_enable_read_voltage(&spi->dev,
> +							     "refin");
> +		if (ret < 0)
> +			return dev_err_probe(&spi->dev, ret,
> +					     "Failed to get refin
> regulator\n");
> +
> +		st->vref_mv = ret / 1000;
>  
> -	external_ref_en = ret != -ENODEV;
> -	st->vref_mv = external_ref_en ? ret / 1000 : AD7380_INTERNAL_REF_MV;
> +		/* these chips don't have a register bit for this */
> +		external_ref_en = false;
> +	} else {
> +		/*
> +		 * If there is no REFIO supply, then it means that we are
> using
> +		 * the internal reference, otherwise REFIO is reference
> voltage.
> +		 */
> +		ret = devm_regulator_get_enable_read_voltage(&spi->dev,
> +							     "refio");
> +		if (ret < 0 && ret != -ENODEV)
> +			return dev_err_probe(&spi->dev, ret,
> +					     "Failed to get refio
> regulator\n");
> +
> +		external_ref_en = ret != -ENODEV;
> +		st->vref_mv = external_ref_en ? ret / 1000 :
> AD7380_INTERNAL_REF_MV;
> +	}
>  
>  	if (st->chip_info->num_vcm_supplies > ARRAY_SIZE(st->vcm_mv))
>  		return dev_err_probe(&spi->dev, -EINVAL,
>