| Message ID | 20191218042121.1471954-3-anarsoul@gmail.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | add thermal sensor driver for A64, A83T, H3, H5, H6, R40 | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | fail | build log |
Hi, On Tue, Dec 17, 2019 at 08:21:16PM -0800, Vasily Khoruzhick wrote: > From: Yangtao Li <tiny.windzz@gmail.com> > > sun8i-thermal driver supports thermal sensor in wide range of Allwinner > SoCs. Add YAML schema for its bindings. > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > --- > .../thermal/allwinner,sun8i-a83t-ths.yaml | 146 ++++++++++++++++++ > 1 file changed, 146 insertions(+) > create mode 100644 Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml > > diff --git a/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml > new file mode 100644 > index 000000000000..8768c2450633 > --- /dev/null > +++ b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml > @@ -0,0 +1,146 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/thermal/allwinner,sun8i-a83t-ths.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Allwinner SUN8I Thermal Controller Device Tree Bindings > + > +maintainers: > + - Yangtao Li <tiny.windzz@gmail.com> > + > +properties: > + compatible: > + enum: > + - allwinner,sun8i-a83t-ths > + - allwinner,sun8i-h3-ths > + - allwinner,sun8i-r40-ths > + - allwinner,sun50i-a64-ths > + - allwinner,sun50i-h5-ths > + - allwinner,sun50i-h6-ths > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + resets: > + maxItems: 1 > + > + nvmem-cells: > + maxItems: 1 > + description: Calibration data for thermal sensors > + > + nvmem-cell-names: > + const: calibration > + > +allOf: > + - if: > + properties: > + compatible: > + contains: > + const: allwinner,sun50i-h6-ths > + > + then: > + properties: > + clocks: > + minItems: 1 > + maxItems: 1 When minItems and maxItems are equal, you can only set one, the other will be filled automatically. > + > + clock-names: > + minItems: 1 > + maxItems: 1 > + items: > + - const: bus And this can even be just clock-names: const: bus > + > + else: > + properties: > + clocks: > + minItems: 1 > + maxItems: 2 > + > + clock-names: > + minItems: 1 > + maxItems: 2 > + items: > + - const: bus > + - const: mod I'm not sure why you need the minItems set to 1 here though? it's always 2 for the !H6 case, right? if so, then we should even do something like: properties: ... # This is needed because we will need to check both the H6 and !H6 # case, and it must validate. So we make sure we match against the # union of both cases. clocks: minItems: 1 maxItems: 2 items: - description: Bus Clock - description: Module Clock # Same story here clock-names: minItems: 1 maxItems: 2 items: - const: bus - const: mod allOf: - if: properties: compatible: contains: const: allwinner,sun50i-h6-ths # Here we validate in the H6 case we only have one clock then: properties: clocks: maxItems: 1 clock-names: maxItems: 1 # and here that in the other case we have two clocks, the names # being validated by the schema above else: properties: clocks: maxItems: 2 clock-names: maxItems: 2 # And now we can set this since all our properties will have been # expressed in the upper level schema additionalProperties: false > + > + - if: > + properties: > + compatible: > + contains: > + const: allwinner,sun8i-h3-ths > + > + then: > + properties: > + "#thermal-sensor-cells": > + const: 0 > + > + else: > + properties: > + "#thermal-sensor-cells": > + const: 1 Same thing here, you should have an enum accepting both values in the upper schema, the condition here only making further checks. Also, in the case where #thermal-sensor-cells is one, then you need to document what that argument is. Thanks! Maxime
On Wed, Dec 18, 2019 at 2:00 PM Maxime Ripard <mripard@kernel.org> wrote: > > Hi, > > On Tue, Dec 17, 2019 at 08:21:16PM -0800, Vasily Khoruzhick wrote: > > From: Yangtao Li <tiny.windzz@gmail.com> > > > > sun8i-thermal driver supports thermal sensor in wide range of Allwinner > > SoCs. Add YAML schema for its bindings. > > > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > > --- > > .../thermal/allwinner,sun8i-a83t-ths.yaml | 146 ++++++++++++++++++ > > 1 file changed, 146 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml > > > > diff --git a/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml > > new file mode 100644 > > index 000000000000..8768c2450633 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml > > @@ -0,0 +1,146 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/thermal/allwinner,sun8i-a83t-ths.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Allwinner SUN8I Thermal Controller Device Tree Bindings > > + > > +maintainers: > > + - Yangtao Li <tiny.windzz@gmail.com> > > + > > +properties: > > + compatible: > > + enum: > > + - allwinner,sun8i-a83t-ths > > + - allwinner,sun8i-h3-ths > > + - allwinner,sun8i-r40-ths > > + - allwinner,sun50i-a64-ths > > + - allwinner,sun50i-h5-ths > > + - allwinner,sun50i-h6-ths > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > + resets: > > + maxItems: 1 > > + > > + nvmem-cells: > > + maxItems: 1 > > + description: Calibration data for thermal sensors > > + > > + nvmem-cell-names: > > + const: calibration > > + > > +allOf: > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: allwinner,sun50i-h6-ths > > + > > + then: > > + properties: > > + clocks: > > + minItems: 1 > > + maxItems: 1 > > When minItems and maxItems are equal, you can only set one, the other > will be filled automatically. Is it documented anywhere? I have a feeling like I'm shooting in the dark. So far I've read Documentation/devicetree/writing-schema.rst, Documentation/devicetree/bindings/example-schema.yaml and few other schemas for inspiration but yet I don't have solid understanding how it's supposed to be written. Examples are pretty scarce and figuring out why certain construction doesn't work is pretty tricky. > > + > > + clock-names: > > + minItems: 1 > > + maxItems: 1 > > + items: > > + - const: bus > > And this can even be just > > clock-names: > const: bus OK > > + > > + else: > > + properties: > > + clocks: > > + minItems: 1 > > + maxItems: 2 > > + > > + clock-names: > > + minItems: 1 > > + maxItems: 2 > > + items: > > + - const: bus > > + - const: mod > > I'm not sure why you need the minItems set to 1 here though? > > it's always 2 for the !H6 case, right? > > if so, then we should even do something like: > > properties: > ... > > # This is needed because we will need to check both the H6 and !H6 > # case, and it must validate. So we make sure we match against the > # union of both cases. > clocks: > minItems: 1 > maxItems: 2 > items: > - description: Bus Clock > - description: Module Clock > > # Same story here > clock-names: > minItems: 1 > maxItems: 2 > items: > - const: bus > - const: mod > > allOf: > - if: > properties: > compatible: > contains: > const: allwinner,sun50i-h6-ths > > # Here we validate in the H6 case we only have one clock > then: > properties: > clocks: > maxItems: 1 > > clock-names: > maxItems: 1 > > # and here that in the other case we have two clocks, the names > # being validated by the schema above > else: > properties: > clocks: > maxItems: 2 > > clock-names: > maxItems: 2 > > # And now we can set this since all our properties will have been > # expressed in the upper level schema > additionalProperties: false OK > > + > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: allwinner,sun8i-h3-ths > > + > > + then: > > + properties: > > + "#thermal-sensor-cells": > > + const: 0 > > + > > + else: > > + properties: > > + "#thermal-sensor-cells": > > + const: 1 > > Same thing here, you should have an enum accepting both values in the > upper schema, the condition here only making further checks. Also, in > the case where #thermal-sensor-cells is one, then you need to document > what that argument is. OK > > Thanks! > Maxime
On Wed, Dec 18, 2019 at 02:27:00PM -0800, Vasily Khoruzhick wrote: > On Wed, Dec 18, 2019 at 2:00 PM Maxime Ripard <mripard@kernel.org> wrote: > > On Tue, Dec 17, 2019 at 08:21:16PM -0800, Vasily Khoruzhick wrote: > > > From: Yangtao Li <tiny.windzz@gmail.com> > > > > > > sun8i-thermal driver supports thermal sensor in wide range of Allwinner > > > SoCs. Add YAML schema for its bindings. > > > > > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > > > --- > > > .../thermal/allwinner,sun8i-a83t-ths.yaml | 146 ++++++++++++++++++ > > > 1 file changed, 146 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml > > > new file mode 100644 > > > index 000000000000..8768c2450633 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml > > > @@ -0,0 +1,146 @@ > > > +# SPDX-License-Identifier: GPL-2.0 > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/thermal/allwinner,sun8i-a83t-ths.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Allwinner SUN8I Thermal Controller Device Tree Bindings > > > + > > > +maintainers: > > > + - Yangtao Li <tiny.windzz@gmail.com> > > > + > > > +properties: > > > + compatible: > > > + enum: > > > + - allwinner,sun8i-a83t-ths > > > + - allwinner,sun8i-h3-ths > > > + - allwinner,sun8i-r40-ths > > > + - allwinner,sun50i-a64-ths > > > + - allwinner,sun50i-h5-ths > > > + - allwinner,sun50i-h6-ths > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + interrupts: > > > + maxItems: 1 > > > + > > > + resets: > > > + maxItems: 1 > > > + > > > + nvmem-cells: > > > + maxItems: 1 > > > + description: Calibration data for thermal sensors > > > + > > > + nvmem-cell-names: > > > + const: calibration > > > + > > > +allOf: > > > + - if: > > > + properties: > > > + compatible: > > > + contains: > > > + const: allwinner,sun50i-h6-ths > > > + > > > + then: > > > + properties: > > > + clocks: > > > + minItems: 1 > > > + maxItems: 1 > > > > When minItems and maxItems are equal, you can only set one, the other > > will be filled automatically. > > Is it documented anywhere? I have a feeling like I'm shooting in the > dark. So far I've read Documentation/devicetree/writing-schema.rst, > Documentation/devicetree/bindings/example-schema.yaml and few other > schemas for inspiration but yet I don't have solid understanding how > it's supposed to be written. Examples are pretty scarce and figuring > out why certain construction doesn't work is pretty tricky. It's somewhat documented with https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/example-schema.yaml#n80 and https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/example-schema.yaml#n102 But I guess it wasn't clear enough? Feel free to update it to something that would be better Maxime
diff --git a/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml new file mode 100644 index 000000000000..8768c2450633 --- /dev/null +++ b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml @@ -0,0 +1,146 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/thermal/allwinner,sun8i-a83t-ths.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Allwinner SUN8I Thermal Controller Device Tree Bindings + +maintainers: + - Yangtao Li <tiny.windzz@gmail.com> + +properties: + compatible: + enum: + - allwinner,sun8i-a83t-ths + - allwinner,sun8i-h3-ths + - allwinner,sun8i-r40-ths + - allwinner,sun50i-a64-ths + - allwinner,sun50i-h5-ths + - allwinner,sun50i-h6-ths + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + resets: + maxItems: 1 + + nvmem-cells: + maxItems: 1 + description: Calibration data for thermal sensors + + nvmem-cell-names: + const: calibration + +allOf: + - if: + properties: + compatible: + contains: + const: allwinner,sun50i-h6-ths + + then: + properties: + clocks: + minItems: 1 + maxItems: 1 + + clock-names: + minItems: 1 + maxItems: 1 + items: + - const: bus + + else: + properties: + clocks: + minItems: 1 + maxItems: 2 + + clock-names: + minItems: 1 + maxItems: 2 + items: + - const: bus + - const: mod + + - if: + properties: + compatible: + contains: + const: allwinner,sun8i-h3-ths + + then: + properties: + "#thermal-sensor-cells": + const: 0 + + else: + properties: + "#thermal-sensor-cells": + const: 1 + + - if: + properties: + compatible: + contains: + enum: + - const: allwinner,sun8i-h3-ths + - const: allwinner,sun8i-r40-ths + - const: allwinner,sun50i-a64-ths + - const: allwinner,sun50i-h5-ths + - const: allwinner,sun50i-h6-ths + + then: + required: + - clocks + - clock-names + - resets + +required: + - compatible + - reg + - interrupts + - '#thermal-sensor-cells' + +examples: + - | + thermal-sensor@1f04000 { + compatible = "allwinner,sun8i-a83t-ths"; + reg = <0x01f04000 0x100>; + interrupts = <0 31 0>; + nvmem-cells = <&ths_calibration>; + nvmem-cell-names = "calibration"; + #thermal-sensor-cells = <1>; + }; + + - | + thermal-sensor@1c25000 { + compatible = "allwinner,sun8i-h3-ths"; + reg = <0x01c25000 0x400>; + clocks = <&ccu 0>, <&ccu 1>; + clock-names = "bus", "mod"; + resets = <&ccu 2>; + interrupts = <0 31 0>; + nvmem-cells = <&ths_calibration>; + nvmem-cell-names = "calibration"; + #thermal-sensor-cells = <0>; + }; + + - | + thermal-sensor@5070400 { + compatible = "allwinner,sun50i-h6-ths"; + reg = <0x05070400 0x100>; + clocks = <&ccu 0>; + clock-names = "bus"; + resets = <&ccu 2>; + interrupts = <0 15 0>; + nvmem-cells = <&ths_calibration>; + nvmem-cell-names = "calibration"; + #thermal-sensor-cells = <1>; + }; + +...