linux-kernel - Re: [PATCH v4 5/6] iio: imu: adis16550: add adis16550 support

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: <604cf5b9c5f2efef4618b9e99bd8d8bf24f999a1.camel@gmail.com>
Date: Mon, 13 Jan 2025 09:48:18 +0000
From: Nuno Sá <noname.nuno@...il.com>
To: Jonathan Cameron <jic23@...nel.org>, Robert Budai
 <robert.budai@...log.com>
Cc: Nuno Sa <nuno.sa@...log.com>, Ramona Gradinariu	
 <ramona.gradinariu@...log.com>, Antoniu Miclaus
 <antoniu.miclaus@...log.com>,  Lars-Peter Clausen	 <lars@...afoo.de>,
 Michael Hennerich <Michael.Hennerich@...log.com>, Rob Herring
 <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley	
 <conor+dt@...nel.org>, Jonathan Corbet <corbet@....net>, Alex Lanzano	
 <lanzano.alex@...il.com>, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, 	linux-kernel@...r.kernel.org,
 linux-doc@...r.kernel.org
Subject: Re: [PATCH v4 5/6] iio: imu: adis16550: add adis16550 support

On Sun, 2025-01-12 at 16:11 +0000, Jonathan Cameron wrote:
> On Fri, 10 Jan 2025 09:42:53 +0200
> Robert Budai <robert.budai@...log.com> wrote:
> 
> > The ADIS16550 is a complete inertial system that includes a triaxis
> > gyroscope and a triaxis accelerometer. Each inertial sensor in
> > the ADIS16550 combines industry leading MEMS only technology
> > with signal conditioning that optimizes dynamic performance. The
> > factory calibration characterizes each sensor for sensitivity, bias,
> > and alignment. As a result, each sensor has its own dynamic com-
> > pensation formulas that provide accurate sensor measurements
> > 
> > Co-developed-by: Ramona Gradinariu <ramona.gradinariu@...log.com>
> > Signed-off-by: Ramona Gradinariu <ramona.gradinariu@...log.com>
> > Co-developed-by: Antoniu Miclaus <antoniu.miclaus@...log.com>
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@...log.com>
> > Signed-off-by: Nuno Sá <nuno.sa@...log.com>
> > Signed-off-by: Robert Budai <robert.budai@...log.com>
> > ---
> > 
> > 4:
> > - reorganized channels to match the order in the datasheet
> > - removed extra checks and goto statements
> > - for buffer memory allocation used only kfree, since adis library already
> > takes care of freeing the buffer
> 
> That last bit makes for a mess wrt to who owns the buffer and lifetime
> management. Suggestions inline.
> 
> Jonathan
> 
> > diff --git a/drivers/iio/imu/adis16550.c b/drivers/iio/imu/adis16550.c
> > new file mode 100644
> > index 000000000000..49c3ff9ef1e2
> > --- /dev/null
> > +++ b/drivers/iio/imu/adis16550.c
> > @@ -0,0 +1,1202 @@
> ...
> 
> 
> > +static int adis16550_set_accl_filter_freq(struct adis16550 *st, int
> > freq_hz)
> > +{
> > +	bool en = false;
> > +
> > +	if (freq_hz)
> > +		en = true;
> > +
> > +	return __adis_update_bits(&st->adis, ADIS16550_REG_CONFIG,
> > +				  ADIS16550_ACCL_FIR_EN_MASK,
> > +				 
> > (u32)FIELD_PREP(ADIS16550_ACCL_FIR_EN_MASK, en));
> 
> Why is the cast needed? Only bit 3 is set.

Typically this is needed for the cases where we want to write in 2 byte
registers and we want to make sure sizeof(val) (on the macro evaluation) gives
us the proper size. But yes, for this case as we want 4 bytes, it should not be
needed. Hmm but I think we might get 'unsigned long' from FIELD_PREP() since
mask is also of that type?

- Nuno Sá