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