[PATCH V12 7/9] iio: imu: inv_icm42607: Add Accelerometer for icm42607
Chris Morgan
macromorgan at hotmail.com
Mon Jun 15 09:40:31 PDT 2026
On Mon, Jun 15, 2026 at 06:07:15PM +0300, Andy Shevchenko wrote:
> On Mon, Jun 15, 2026 at 09:51:40AM -0500, Chris Morgan wrote:
> > On Mon, Jun 15, 2026 at 02:21:05PM +0300, Andy Shevchenko wrote:
> > > On Thu, Jun 11, 2026 at 03:26:04PM -0500, Chris Morgan wrote:
>
> ...
>
> > > Please, please, use IWYU! So many headers are missing...
> > > (Same comment to all files in this series.)
> > >
> > > + array_size.h
> > > + bits.h // BIT()
> > > + cleanup.h // guard()()
> > > + device/devres.h // devm_kasprintf()
> > > + err.h // -EINVAL, IS_ERR()
> > >
> > > > +#include <linux/iio/iio.h>
> > > > +#include <linux/mutex.h>
> > > > +#include <linux/pm_runtime.h>
> > > > +#include <linux/regmap.h>
> > >
> > > + types.h // s16, __be16
> > >
> > > Also you need to have
> > >
> > > asm/byteorder.h // be16_to_cpup()
> >
> > How are you running IWYU against the builds? So far I've tried but I
> > can't seem to get it to run properly.
>
> Sorry, I meant "use IWYU principle". I don't run the tool, I just looked into
> the code.
I've done what I can then, I have added these headers where I am using
them. I'm not going to add asm/byteorder.h though because I am dropping
that in the next revision and replacing it with get_unaligned_be16() to
further simplify things. Note that I am adding linux/unaligned.h for
the files where I use that function.
The additional headers will be added in the next revision. If you see
any other obvious ones missing let me know but for now I *think* it's
correct.
>
> ...
>
> > > > + for (i = 5; i < ARRAY_SIZE(inv_icm42607_accel_odr); ++i) {
> > >
> > > Why pre-increment? Same for all other cases.
> >
> > The register starts at 5 and all values below 5 are invalid. Starting
> > this increment at 5 ensures we don't expose invalid values to
> > userspace.
>
> It doesn't explain pre-increment. Post-increment should work as is.
The array this references starts at 5, because those correspond to the
values written to the odr register. That said, I do see a bug because
the odr register is from highest to smallest and this array is
backwards in the accel and gyro code. I'll fix that.
>
> > > > + if (i == odr)
> > > > + break;
> > > > + }
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
Thank you,
Chris
More information about the Linux-rockchip
mailing list