[v2,1/2] dt-bindings: regulator: Add DA9121
diff mbox series

Message ID 20201103100021.19603-2-vincent.whitchurch@axis.com
State Accepted
Commit 1119c59404141200125af31f775d3fbbba52c651
Headers show
Series
  • DA9121 regulator support
Related show

Commit Message

Vincent Whitchurch Nov. 3, 2020, 10 a.m. UTC
Add bindings for the Dialog Semiconductor DA9121 voltage regulator.

Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
---
 .../bindings/regulator/dlg,da9121.yaml        | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/dlg,da9121.yaml

Comments

Rob Herring Nov. 4, 2020, 6:57 p.m. UTC | #1
On Tue, 03 Nov 2020 11:00:20 +0100, Vincent Whitchurch wrote:
> Add bindings for the Dialog Semiconductor DA9121 voltage regulator.
> 
> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
> ---
>  .../bindings/regulator/dlg,da9121.yaml        | 47 +++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/dlg,da9121.yaml
> 


My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml: 'additionalProperties' is a required property
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml: ignoring, error in schema: 
warning: no schema found in file: ./Documentation/devicetree/bindings/regulator/dlg,da9121.yaml


See https://patchwork.ozlabs.org/patch/1392753

The base for the patch is generally the last rc1. Any dependencies
should be noted.

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.
Vincent Whitchurch Nov. 5, 2020, 9:39 a.m. UTC | #2
On Wed, Nov 04, 2020 at 07:57:55PM +0100, Rob Herring wrote:
> On Tue, 03 Nov 2020 11:00:20 +0100, Vincent Whitchurch wrote:
> > Add bindings for the Dialog Semiconductor DA9121 voltage regulator.
> > 
> > Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
> > ---
> >  .../bindings/regulator/dlg,da9121.yaml        | 47 +++++++++++++++++++
> >  1 file changed, 47 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/regulator/dlg,da9121.yaml
> > 
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml: 'additionalProperties' is a required property
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml: ignoring, error in schema: 
> warning: no schema found in file: ./Documentation/devicetree/bindings/regulator/dlg,da9121.yaml
> 
> See https://patchwork.ozlabs.org/patch/1392753

OK, thanks.  I'll fix it by changing the unevaluatedProperties to an
additionalProperties (since the $ref has moved).  

I think I should also move the unevaluatedProperties to the buck1 level
instead of removing it completely, because I see a bunch of other
regulator bindings doing it, but the checks don't complain about that
being missing and I can't see that making any difference for the
validation.

For example, I was hoping that this:

  buck1:
    description:
      Initial data for the Buck1 regulator.
    $ref: "regulator.yaml#"
    type: object
    unevaluatedProperties: false

would complain about something like:

        buck1 {
	  not-a-real-property;
          regulator-min-microvolt = <680000>;
          regulator-max-microvolt = <820000>;
        };

but it doesn't, so I don't quite understand what "unevaluatedProperties:
false" prevents.

> 
> The base for the patch is generally the last rc1. Any dependencies
> should be noted.
> 
> 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

Not sure if it's just something wrong in my setup, but I had dtschema
installed from master a while ago, and the above command didn't upgrade
it for me.  I had to explicitly upgrade it with the full URL again to
get the latest version:

 pip3 install --upgrade git+https://github.com/devicetree-org/dt-schema.git@master
Rob Herring Nov. 20, 2020, 8:32 p.m. UTC | #3
On Thu, Nov 5, 2020 at 3:39 AM Vincent Whitchurch
<vincent.whitchurch@axis.com> wrote:
>
> On Wed, Nov 04, 2020 at 07:57:55PM +0100, Rob Herring wrote:
> > On Tue, 03 Nov 2020 11:00:20 +0100, Vincent Whitchurch wrote:
> > > Add bindings for the Dialog Semiconductor DA9121 voltage regulator.
> > >
> > > Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
> > > ---
> > >  .../bindings/regulator/dlg,da9121.yaml        | 47 +++++++++++++++++++
> > >  1 file changed, 47 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/regulator/dlg,da9121.yaml
> > >
> >
> > My bot found errors running 'make dt_binding_check' on your patch:
> >
> > yamllint warnings/errors:
> >
> > dtschema/dtc warnings/errors:
> > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml: 'additionalProperties' is a required property
> > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml: ignoring, error in schema:
> > warning: no schema found in file: ./Documentation/devicetree/bindings/regulator/dlg,da9121.yaml
> >
> > See https://patchwork.ozlabs.org/patch/1392753
>
> OK, thanks.  I'll fix it by changing the unevaluatedProperties to an
> additionalProperties (since the $ref has moved).
>
> I think I should also move the unevaluatedProperties to the buck1 level
> instead of removing it completely, because I see a bunch of other
> regulator bindings doing it, but the checks don't complain about that
> being missing and I can't see that making any difference for the
> validation.

Because the meta-schema is not recursing down levels. It probably
should, but that's another round of fixing all the current cases. And
the top-level is more well defined as to what the structure is (IOW,
we might not be able to define a rule that works everywhere).

> For example, I was hoping that this:
>
>   buck1:
>     description:
>       Initial data for the Buck1 regulator.
>     $ref: "regulator.yaml#"
>     type: object
>     unevaluatedProperties: false
>
> would complain about something like:
>
>         buck1 {
>           not-a-real-property;
>           regulator-min-microvolt = <680000>;
>           regulator-max-microvolt = <820000>;
>         };
>
> but it doesn't, so I don't quite understand what "unevaluatedProperties:
> false" prevents.

That's because it's currently a nop as the Python jsonschema package
doesn't yet support 2019.09 version of jsonschema.

If you aren't defining properties in addition to what's defined by a
$ref, then use additionalProperties.

Rob

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml b/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml
new file mode 100644
index 000000000000..cde0d82dd201
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml
@@ -0,0 +1,47 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/dlg,da9121.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Dialog Semiconductor DA9121 voltage regulator
+
+maintainers:
+  - Vincent Whitchurch <vincent.whitchurch@axis.com>
+
+properties:
+  compatible:
+    const: dlg,da9121
+
+  reg:
+    maxItems: 1
+
+  buck1:
+    description:
+      Initial data for the Buck1 regulator.
+    $ref: "regulator.yaml#"
+    type: object
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      regulator@68 {
+        compatible = "dlg,da9121";
+        reg = <0x68>;
+
+        buck1 {
+          regulator-min-microvolt = <680000>;
+          regulator-max-microvolt = <820000>;
+        };
+      };
+    };
+
+...