[2/4] dt-bindings: iio: magnetometer: Add DT binding for Voltafield AF8133J

  From: Icenowy Zheng <icenowy@aosc.io>

Voltafield AF8133J is a simple magnetometer sensor produced by Voltafield
Technology Corp, with dual power supplies (one for core and one for I/O)
and active-low reset pin.

The sensor has configurable range 1.2 - 2.2 mT and a software controlled
standby mode.

Add a device tree binding for it.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
Signed-off-by: Ondřej Jirman <megi@xff.cz>
---
 .../iio/magnetometer/voltafield,af8133j.yaml  | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/voltafield,af8133j.yaml
  

On Sun, 11 Feb 2024 21:51:58 +0100, Ondřej Jirman wrote:
> From: Icenowy Zheng <icenowy@aosc.io>
> 
> Voltafield AF8133J is a simple magnetometer sensor produced by Voltafield
> Technology Corp, with dual power supplies (one for core and one for I/O)
> and active-low reset pin.
> 
> The sensor has configurable range 1.2 - 2.2 mT and a software controlled
> standby mode.
> 
> Add a device tree binding for it.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> Signed-off-by: Ondřej Jirman <megi@xff.cz>
> ---
>  .../iio/magnetometer/voltafield,af8133j.yaml  | 58 +++++++++++++++++++
>  1 file changed, 58 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/voltafield,af8133j.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/magnetometer/voltafield,af8133j.yaml: properties:compatible: ['voltafield,af8133j'] is not of type 'object', 'boolean'
	from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/magnetometer/voltafield,af8133j.yaml: properties:compatible: ['voltafield,af8133j'] is not of type 'object', 'boolean'
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/magnetometer/voltafield,af8133j.yaml: ignoring, error in schema: properties: compatible
Documentation/devicetree/bindings/iio/magnetometer/voltafield,af8133j.example.dtb: /example-0/i2c/magnetometer@1c: failed to match any schema with compatible: ['voltafield,af8133j']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240211205211.2890931-3-megi@xff.cz

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
  

Hi Ondřej,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on robh/for-next linus/master v6.8-rc4 next-20240212]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ond-ej-Jirman/dt-bindings-vendor-prefix-Add-prefix-for-Voltafield/20240212-045510
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20240211205211.2890931-3-megi%40xff.cz
patch subject: [PATCH 2/4] dt-bindings: iio: magnetometer: Add DT binding for Voltafield AF8133J
:::::: branch date: 11 hours ago
:::::: commit date: 11 hours ago
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240212/202402121531.EoXy0HWe-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/r/202402121531.EoXy0HWe-lkp@intel.com/

dtcheck warnings: (new ones prefixed by >>)
>> Documentation/devicetree/bindings/iio/magnetometer/voltafield,af8133j.yaml: properties:compatible: ['voltafield,af8133j'] is not of type 'object', 'boolean'
   	from schema $id: http://json-schema.org/draft-07/schema#
>> Documentation/devicetree/bindings/iio/magnetometer/voltafield,af8133j.yaml: properties:compatible: ['voltafield,af8133j'] is not of type 'object', 'boolean'
   	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
--
>> Documentation/devicetree/bindings/iio/magnetometer/voltafield,af8133j.yaml: ignoring, error in schema: properties: compatible
  

Hi Jonathan,

thanks for the feedback.

On Mon, Feb 12, 2024 at 11:47:38AM +0000, Jonathan Cameron wrote:
> On Sun, 11 Feb 2024 21:51:58 +0100
> Ondřej Jirman <megi@xff.cz> wrote:
> 
> > From: Icenowy Zheng <icenowy@aosc.io>
> 
> Title doesn't need to mention binding (it's implicit from the dt-bindings bit
> dt-bindings: iio: magnetometer: Add Voltafield AF8133J
> 
> > 
> > Voltafield AF8133J is a simple magnetometer sensor produced by Voltafield
> > Technology Corp, with dual power supplies (one for core and one for I/O)
> > and active-low reset pin.
> > 
> > The sensor has configurable range 1.2 - 2.2 mT and a software controlled
> > standby mode.
> > 
> > Add a device tree binding for it.
> > 
> > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > Signed-off-by: Ondřej Jirman <megi@xff.cz>
> 
> Hi Ondřej
> 
> A few quick comments.
> 
> 
> > ---
> >  .../iio/magnetometer/voltafield,af8133j.yaml  | 58 +++++++++++++++++++
> >  1 file changed, 58 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/voltafield,af8133j.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/magnetometer/voltafield,af8133j.yaml b/Documentation/devicetree/bindings/iio/magnetometer/voltafield,af8133j.yaml
> > new file mode 100644
> > index 000000000000..ab56c0f798ad
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/magnetometer/voltafield,af8133j.yaml
> > @@ -0,0 +1,58 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/magnetometer/voltafield,af8133j.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Voltafield AF8133J magnetometer sensor
> > +
> > +maintainers:
> > +  - Ondřej Jirman <megi@xff.cz>
> > +
> > +properties:
> > +  compatible:
> > +    - voltafield,af8133j
> Test your bindings (as described in the bot message).
>     const: voltafield,af8133j

I did, but didn't notice that DT_SCHEMA_FILES= didn't work as expected when
provided with full path to the bindings file. :)

Just using DT_SCHEMA_FILES=voltafield,af8133j.yaml works better and catches this
issue.

> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  reset-gpios:
> > +    description: |
> No need for the | as the formatting doesn't need to be preserved.
> 
> > +      an pin needed for AF8133J to set the reset state. This should be usually
> 
> A pin
> 
> > +      active low.
> > +
> > +  avdd-supply:
> > +    description: |
> > +      an optional regulator that needs to be on to provide AVDD power (Working
> 
> An optional (if it were optional, A regulator if not)
> 
> > +      power, usually 3.3V) to the sensor.
> Seems unlikely the power supply is optional (though specifying it in DT might be -
> however see below).
> 
> > +
> > +  dvdd-supply:
> > +    description: |
> > +      an optional regulator that needs to be on to provide DVDD power (Digital
> > +      IO power, 1.8V~AVDD) to the sensor.
> > +
> > +  mount-matrix:
> > +    description: an optional 3x3 mounting rotation matrix.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> 
> Any power supply that is required for operation should be listed here (even though
> we can rely on the stub supplies if it isn't in the DT).
> I used to think this wasn't necessary, so lots of bindings upstream don't yet
> have it.

By stub supply you mean dummy supply created when the *-supply property is not
specified in DT? Or something else?

Because DTC_CHK prints a warning if I make the proerty required in bindings and
not specify it in DT

arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.2b.dtb: magnetometer@1c: 'avdd-supply' is a required property
	from schema $id: http://devicetree.org/schemas/iio/magnetometer/voltafield,af8133j.yaml#


kind regards,
	o.

> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        magnetometer@1c {
> > +            compatible = "voltafield,af8133j";
> > +            reg = <0x1c>;
> > +            avdd-supply = <&reg_dldo1>;
> > +            dvdd-supply = <&reg_dldo1>;
> > +            reset-gpios = <&pio 1 1 GPIO_ACTIVE_LOW>;
> > +        };
> > +    };
>
  

On Wed, Feb 14, 2024 at 04:31:16PM +0000, Jonathan Cameron wrote:
> 
> > > > +
> > > > +  dvdd-supply:
> > > > +    description: |
> > > > +      an optional regulator that needs to be on to provide DVDD power (Digital
> > > > +      IO power, 1.8V~AVDD) to the sensor.
> > > > +
> > > > +  mount-matrix:
> > > > +    description: an optional 3x3 mounting rotation matrix.
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg  
> > > 
> > > Any power supply that is required for operation should be listed here (even though
> > > we can rely on the stub supplies if it isn't in the DT).
> > > I used to think this wasn't necessary, so lots of bindings upstream don't yet
> > > have it.  
> > 
> > By stub supply you mean dummy supply created when the *-supply property is not
> > specified in DT? Or something else?
> 
> Ah yes. I got the term wrong.
> > 
> > Because DTC_CHK prints a warning if I make the proerty required in bindings and
> > not specify it in DT
> > 
> > arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.2b.dtb: magnetometer@1c: 'avdd-supply' is a required property
> > 	from schema $id: http://devicetree.org/schemas/iio/magnetometer/voltafield,af8133j.yaml#
> 
> Provide one, or don't worry about that warning. 

For the sake of the platform maintainer, please choose option 1.

Thanks,
Conor.

> Various discussions have taken place on this over time and end
> result is bindings should require them to differentiate from power
> supplies that are actually optional.
> 
> Jonathan
> 
>