| Message ID | 20240211205211.2890931-3-megi@xff.cz |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for AF8133J magnetometer | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dt-meta-schema | fail | build log |
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.
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 > + > + 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. > + > +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 = <®_dldo1>; > + dvdd-supply = <®_dldo1>; > + reset-gpios = <&pio 1 1 GPIO_ACTIVE_LOW>; > + }; > + };
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 = <®_dldo1>; > > + dvdd-supply = <®_dldo1>; > > + reset-gpios = <&pio 1 1 GPIO_ACTIVE_LOW>; > > + }; > > + }; >
> > > + > > > + 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. 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
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 > >
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. > > 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. Ok. Good to know. :) regards, o. > Jonathan > >
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 + + reg: + maxItems: 1 + + reset-gpios: + description: | + an pin needed for AF8133J to set the reset state. This should be usually + active low. + + avdd-supply: + description: | + an optional regulator that needs to be on to provide AVDD power (Working + power, usually 3.3V) to the sensor. + + 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 + +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 = <®_dldo1>; + dvdd-supply = <®_dldo1>; + reset-gpios = <&pio 1 1 GPIO_ACTIVE_LOW>; + }; + };