linux-kernel - Re: [PATCH v6 10/11] iio: adc: ad7768-1: add filter type and oversampling ratio attributes

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: <64b41a31-6224-4ab7-b26d-ea3e60a60d3b@baylibre.com>
Date: Tue, 6 May 2025 14:10:04 -0500
From: David Lechner <dlechner@...libre.com>
To: 20250505170950.1d7941d0@...23-huawei.smtp.subspace.kernel.org,
	Jonathan Cameron <jic23@...nel.org>
Cc: Jonathan Santos <Jonathan.Santos@...log.com>, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-gpio@...r.kernel.org, andy@...nel.org, nuno.sa@...log.com,
 Michael.Hennerich@...log.com, marcelo.schmitt@...log.com, robh@...nel.org,
 krzk+dt@...nel.org, conor+dt@...nel.org, marcelo.schmitt1@...il.com,
 linus.walleij@...aro.org, brgl@...ev.pl, lgirdwood@...il.com,
 broonie@...nel.org, Pop Paul <paul.pop@...log.com>
Subject: Re: [PATCH v6 10/11] iio: adc: ad7768-1: add filter type and
 oversampling ratio attributes

On 5/6/25 2:03 PM, Jonathan Santos wrote:
> On 05/05, Jonathan Cameron wrote:
>> On Sun, 27 Apr 2025 21:14:17 -0300
>> Jonathan Santos <Jonathan.Santos@...log.com> wrote:
>>
> ...
>>  drivers/iio/adc/ad7768-1.c | 363 ++++++++++++++++++++++++++++++-------
>>>  1 file changed, 293 insertions(+), 70 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
>>> index 10791a85d2c5..e2b8f12260a5 100644
>>> --- a/drivers/iio/adc/ad7768-1.c
>>> +++ b/drivers/iio/adc/ad7768-1.c
>>> @@ -20,6 +20,8 @@
>>>  #include <linux/regulator/driver.h>
>>>  #include <linux/sysfs.h>
>>>  #include <linux/spi/spi.h>
>>> +#include <linux/unaligned.h>
>>> +#include <linux/util_macros.h>
>>>  
>>>  #include <linux/iio/buffer.h>
>>>  #include <linux/iio/iio.h>
>>> @@ -77,7 +79,7 @@
>>>  #define AD7768_PWR_PWRMODE(x)		FIELD_PREP(AD7768_PWR_PWRMODE_MSK, x)
>>>  
>>>  /* AD7768_REG_DIGITAL_FILTER */
>>> -#define AD7768_DIG_FIL_FIL_MSK		GENMASK(6, 4)
>>> +#define AD7768_DIG_FIL_FIL_MSK		GENMASK(7, 4)
>>
>> Bug?  If so does this belong in a precursor patch?
>>
> 
> Actually not, this extra bit is to include the 60Hz rejection enable
> for sinc3 filter

Seems odd to me to group those together since they are two separate fields in
the register. It would make more sense to have a separate BIT(7) for the
EN_60HZ_REJ bit.

If we need to manipulate both at the same time in the driver, then we would
use AD7768_DIG_FIL_EN_60HZ_REJ | AD7768_DIG_FIL_FIL_MSK.

> 
>>>  #define AD7768_DIG_FIL_FIL(x)		FIELD_PREP(AD7768_DIG_FIL_FIL_MSK, x)
>>
> ...
>