Re: [PATCH 7/8] dt-bindings: iio: imu: Add inv_icm45600 documentation

[Jonathan Cameron]

On Fri, 11 Apr 2025 16:18:39 -0500
David Lechner <dlechner@xxxxxxxxxxxx> wrote:

> On 4/11/25 8:28 AM, Remi Buisson via B4 Relay wrote:
> > From: Remi Buisson <remi.buisson@xxxxxxx>
> > 
> > Document the ICM-456xxx devices devicetree bindings.
> > Describe custom sysfs API for controlling the power modes.
> > 
> > Signed-off-by: Remi Buisson <remi.buisson@xxxxxxx>
> > ---  
> 
> IMHO, it is more logical to have the dt-bindings patch first in the series
> before the code that uses it.
> 
> >  .../ABI/testing/sysfs-bus-iio-inv_icm45600         |  37 ++++++
> >  .../bindings/iio/imu/invensense,icm45600.yaml      | 136 +++++++++++++++++++++
> >  2 files changed, 173 insertions(+)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-inv_icm45600 b/Documentation/ABI/testing/sysfs-bus-iio-inv_icm45600
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..8d2d9b68ad9e35fe0d6c157e984afc327eab92ec
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-bus-iio-inv_icm45600  
> 
> 
> ABI documentation is separate from dt-bindings and needs to go in a
> different patch.
> 
> Also, it looks like /sys/.../iio:deviceX/in_accelY_power_mode is
> already a standard attribute in Documentation/ABI/testing/sysfs-bus-iio
> so we could add to that instead of creating a new file.

Just a quick side note.  Those powermode interfaces are hard for userspace
to reason about, so where possible it is better to set power mode in response
to more explicit demands such as sampling frequency.  I can't remember when
I got persuaded to let that one in. I checked - long ago and we still only
have 2 users :)

> 
> And there is Documentation/ABI/testing/sysfs-bus-iio-inv_icm42600
> that has the same attribute essentially. So it would be good to
> delete this file and consolidate everything in the main file.
> 
> > diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm45600.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm45600.yaml
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..51455f0b5cb90abdd823f154e45891ad364296e6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm45600.yaml
> > @@ -0,0 +1,136 @@  
> 
> ...
> 
> > +allOf:
> > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#  
> 
> Since this can be connected to different buses, I don't think we want
> to always ref this. It gets included implicitly for all child nodes on a
> spi controller node anyway.
>