Re: [PATCH v5 1/4] dt-bindings: iio: magnetometer: ak8975: convert format to yaml, add maintainer

[Jonathan Albrieux <jonathan.albrieux@gmail.com>]

On Sun, May 24, 2020 at 03:57:45PM +0100, Jonathan Cameron wrote:
> On Wed, 20 May 2020 18:34:06 +0200
> Jonathan Albrieux <jonathan.albrieux@gmail.com> wrote:
> 
> > Converts documentation from txt format to yaml.
> > 
> > Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
> 
> An additional request inline.  Doesn't effect this patch 'as such'.
> 
> Also the file name thing just below here.
> 
> > ---
> >  .../bindings/iio/magnetometer/ak8975.txt      | 30 --------
> >  .../bindings/iio/magnetometer/ak8975.yaml     | 71 +++++++++++++++++++
> File naming should match the compatible.
> 
> ashahi-kasei,ak8975.yaml
>

Ok, I'll take this as general rule then :-)
 
> >  2 files changed, 71 insertions(+), 30 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt
> >  create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt b/Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt
> > deleted file mode 100644
> > index aa67ceb0d4e0..000000000000
> > --- a/Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt
> > +++ /dev/null
> > @@ -1,30 +0,0 @@
> > -* AsahiKASEI AK8975 magnetometer sensor
> > -
> > -Required properties:
> > -
> > -  - compatible : should be "asahi-kasei,ak8975"
> > -  - reg : the I2C address of the magnetometer
> > -
> > -Optional properties:
> > -
> > -  - gpios : should be device tree identifier of the magnetometer DRDY pin
> > -  - vdd-supply: an optional regulator that needs to be on to provide VDD
> > -  - mount-matrix: an optional 3x3 mounting rotation matrix
> > -
> > -Example:
> > -
> > -ak8975@c {
> > -        compatible = "asahi-kasei,ak8975";
> > -        reg = <0x0c>;
> > -        gpios = <&gpj0 7 0>;
> > -        vdd-supply = <&ldo_3v3_gnss>;
> > -        mount-matrix = "-0.984807753012208",  /* x0 */
> > -                       "0",                   /* y0 */
> > -                       "-0.173648177666930",  /* z0 */
> > -                       "0",                   /* x1 */
> > -                       "-1",                  /* y1 */
> > -                       "0",                   /* z1 */
> > -                       "-0.173648177666930",  /* x2 */
> > -                       "0",                   /* y2 */
> > -                       "0.984807753012208";   /* z2 */
> > -};
> > diff --git a/Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml b/Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml
> > new file mode 100644
> > index 000000000000..8bde423a2ffa
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml
> > @@ -0,0 +1,71 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/magnetometer/ak8975.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: AsahiKASEI AK8975 magnetometer sensor
> > +
> > +maintainers:
> > +  - Jonathan Albrieux <jonathan.albrieux@gmail.com>
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - const: asahi-kasei,ak8975
> > +      - const: asahi-kasei,ak8963
> > +      - const: asahi-kasei,ak09911
> > +      - const: asahi-kasei,ak09912
> > +      - const: ak8975
> > +        deprecated: true
> > +      - const: ak8963
> > +        deprecated: true
> > +      - const: ak09911
> > +        deprecated: true
> > +      - const: ak09912
> > +        deprecated: true
> > +
> > +  reg:
> > +    maxItems: 1
> > +    description: the I2C address of the magnetometer
> > +
> > +  gpios:
> > +    description: should be device tree identifier of the magnetometer DRDY pin
> 
> Nothing to do with your patch obviously but this is horrible...
> + the driver will quite happily take an interrupt instead.
> 
> Do you mind documenting the interrupt here as well?
> 
> Should be a separate patch though.  So a follow up one to
> this one.  The 
> arch/arm/boot/dts/motorola-mapphone-common.dtsi
> does it that way, unlike the samsung s3 which uses the gpio
> binding (I think it's the only one against quite a few
> using interrupts).
> 
> Also switch the example to an interrupts one so we don't
> encourage anyone to go the gpios route.
> 
> Note can be a follow up if this series is otherwise ready to go.
> 

Oh ok, will add a new patch for that in this series if you don't
mind. It may even be better to split this series in two given
the amount of changes to the documentation outnumbered the changes
related to the driver itself, the first one regarding just the
conversion to yaml and clean-up-related changes to documentation
and the second one, depending on the first one, with the
reset-related changes.

Do you have advices on which of the two should be the best strategy?

I'll prepare both the versions today in order to choose the right
one once the path to follow has been decided,

Thank you,

Best regards,
Jonathan Albrieux

> 
> > +
> > +  vdd-supply:
> > +    maxItems: 1
> > +    description: |
> > +      an optional regulator that needs to be on to provide VDD power to
> > +      the sensor.
> > +
> > +  mount-matrix:
> > +    description: an optional 3x3 mounting rotation matrix
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    i2c@78b7000 {
> > +        reg = <0x78b6000 0x600>;
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        magnetometer@c {
> > +            compatible = "asahi-kasei,ak8975";
> > +            reg = <0x0c>;
> > +            gpios = <&gpj0 7 GPIO_ACTIVE_HIGH>;
> > +            vdd-supply = <&ldo_3v3_gnss>;
> > +            mount-matrix = "-0.984807753012208",  /* x0 */
> > +                           "0",                   /* y0 */
> > +                           "-0.173648177666930",  /* z0 */
> > +                           "0",                   /* x1 */
> > +                           "-1",                  /* y1 */
> > +                           "0",                   /* z1 */
> > +                           "-0.173648177666930",  /* x2 */
> > +                           "0",                   /* y2 */
> > +                           "0.984807753012208";   /* z2 */
> > +        };
> > +    };
>