linux-kernel - Re: [PATCH RESEND v3 08/17] iio: adc: ad7768-1: convert driver to use regmap

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: <20250216160610.4cbfb73f@jic23-huawei>
Date: Sun, 16 Feb 2025 16:06:10 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Jonathan Santos <Jonathan.Santos@...log.com>
Cc: <linux-iio@...r.kernel.org>, <devicetree@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>, <lars@...afoo.de>,
 <Michael.Hennerich@...log.com>, <marcelo.schmitt@...log.com>,
 <robh@...nel.org>, <krzk+dt@...nel.org>, <conor+dt@...nel.org>,
 <jonath4nns@...il.com>, <marcelo.schmitt1@...il.com>,
 <dlechner@...libre.com>
Subject: Re: [PATCH RESEND v3 08/17] iio: adc: ad7768-1: convert driver to
 use regmap

On Wed, 12 Feb 2025 15:17:16 -0300
Jonathan Santos <Jonathan.Santos@...log.com> wrote:

> Convert the AD7768-1 driver to use the regmap API for register
> access. This change simplifies and standardizes register interactions,
> reducing code duplication and improving maintainability.
> 
> Create two regmap configurations, one for 8-bit register values and
> other for 24-bit register values.
> 
> Since we are using regmap now, define the remaining registers from 0x32
> to 0x34.
> 
> Signed-off-by: Jonathan Santos <Jonathan.Santos@...log.com>
Looks good.  Just one passing suggestion that might reduce chance of
future bugs a tiny bit.  Any bug that uses the wrong regmap should show
up quickly in testing but none the less, maybe naming can help too.

Not an important comment though so don't respin the series just for this.

Jonathan


>  
>  static int ad7768_scan_direct(struct iio_dev *indio_dev)
> @@ -233,9 +269,10 @@ static int ad7768_scan_direct(struct iio_dev *indio_dev)
>  	if (!ret)
>  		return -ETIMEDOUT;
>  
> -	readval = ad7768_spi_reg_read(st, AD7768_REG_ADC_DATA, 3);
> -	if (readval < 0)
> -		return readval;
> +	ret = regmap_read(st->regmap24, AD7768_REG_ADC_DATA, &readval);
I wonder if it is worth reducing the possibility of reading the register
via the wrong regmap by changing the defintion to
AD7768_REG24_ADC_DATA or something along those lines?

> +	if (ret)
> +		return ret;
> +