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

["Nuno Sá" <noname.nuno@gmail.com>]

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@analog.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@analog.com>
> > Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
> > Co-developed-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> > Signed-off-by: Robert Budai <robert.budai@analog.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á