Re: [PATCH V4 01/10] dt-bindings: iio: imu: icm42600: Add icm42607 binding

Chris Morgan
2026-05-04T17:17:54Z

On Mon, May 04, 2026 at 05:51:28PM +0100, Jonathan Cameron wrote:
> On Sun, 3 May 2026 15:51:30 -0500
> Chris Morgan <macromorgan@hotmail.com> wrote:
>
> > On Sun, May 03, 2026 at 02:18:18PM +0200, Krzysztof Kozlowski wrote:
> > > On Fri, May 01, 2026 at 05:11:40PM -0500, Chris Morgan wrote:
> > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > >
> > > > Add devicetree binding for the Invensense ICM42607 and Invensense
> > > > ICM42607P inertial measurement unit. This unit is a combined
> > > > accelerometer, gyroscope, and thermometer available via I2C or SPI.
> > > >
> > > > This device is functionally very similar to the icm42600 series with a
> > > > very different register layout. Additionally, add mount-matrix
> > > > attribute to schema.
> > >
> > > Why adding it? Is it something new? Is it applicable to other variants?
> > > If not, why it is allowed for them?
> >
> > It's not new, technically this is a bug/oversight from the very first
> > iteration of the invensense icm42600 driver. The driver requests a
> > mount matrix using iio_read_mount_matrix and then returns an error from
>
> Unless it's broken (always possible) iio_read_mount_matrix() is supposed
> to return an identity matrix if there isn't any info in the binding.
>
> /* Matrix was not declared at all: fallback to identity. */
> return iio_setup_mount_idmatrix(dev, matrix);
>
> So not required by the linux driver at least. Probably not something
> we should require in general.
>
> Separate patch to introduce it to the binding as optional and say something
> about what it is for to justify it's inclusion. That patch goes before
> this one.

Okay, that makes more sense. I saw the return condition but didn't realize
that an empty matrix didn't result in an error.

So for the next version I'll add an additionall commit with this as a new
attribute, noting that it has always been supported with this driver but
never specified in the binding documentation.

Thank you,
Chris

>
> > the probe function if it can't read one [1]. So it's very much required
> > (and for the next version I'll upgrade it to a required element). I'm
> > adding it because a cursory grep suggests that my use case (for the
> > Anbernic RG-DS and in the near future the Anbernic RG Vita Pro) are the
> > first devices to use this driver in mainline in a device tree based
> > system.
> >
> > Thank you,
> > Chris
> >
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c?h=v7.1-rc1#n746
> >
> > >
> > > Do not say WHAT you did, say why you did it.
> > >
> > > Best regards,
> > > Krzysztof
> > >
>