diff mbox series

[v7,2/6] dt-bindings: thermal: Add binding document for LVTS thermal controllers

Message ID 20220524152552.246193-3-abailon@baylibre.com (mailing list archive)
State New, archived
Headers show
Series Add LVTS architecture thermal | expand

Commit Message

Alexandre Bailon May 24, 2022, 3:25 p.m. UTC 
This patch adds binding document for mt8192 and mt8195 thermal
controllers.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
---
 .../thermal/mediatek,mt8192-lvts.yaml         | 81 +++++++++++++++++++
 1 file changed, 81 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/mediatek,mt8192-lvts.yaml

Comments

Rob Herring (Arm) May 25, 2022, 12:44 p.m. UTC | #1 
On Tue, 24 May 2022 17:25:49 +0200, Alexandre Bailon wrote:
> This patch adds binding document for mt8192 and mt8195 thermal
> controllers.
> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
>  .../thermal/mediatek,mt8192-lvts.yaml         | 81 +++++++++++++++++++
>  1 file changed, 81 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/mediatek,mt8192-lvts.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/linux-dt-review/Documentation/devicetree/bindings/thermal/mediatek,mt8192-lvts.yaml: allOf:1:$ref: '/nvmem/nvmem-consumer.yaml#' does not match '^(/schemas/|\\.\\./|#(/|$)|[a-zA-Z0-9]+)'
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/thermal/mediatek,mt8192-lvts.yaml: ignoring, error in schema: allOf: 1: $ref
Error: Documentation/devicetree/bindings/thermal/mediatek,mt8192-lvts.example.dts:34.36-37 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:364: Documentation/devicetree/bindings/thermal/mediatek,mt8192-lvts.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1401: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

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

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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.
Rob Herring (Arm) May 26, 2022, 2:05 a.m. UTC | #2 
On Tue, May 24, 2022 at 05:25:49PM +0200, Alexandre Bailon wrote:
> This patch adds binding document for mt8192 and mt8195 thermal
> controllers.
> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
>  .../thermal/mediatek,mt8192-lvts.yaml         | 81 +++++++++++++++++++
>  1 file changed, 81 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/mediatek,mt8192-lvts.yaml
> 
> diff --git a/Documentation/devicetree/bindings/thermal/mediatek,mt8192-lvts.yaml b/Documentation/devicetree/bindings/thermal/mediatek,mt8192-lvts.yaml
> new file mode 100644
> index 000000000000..914c877d1f2f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/mediatek,mt8192-lvts.yaml
> @@ -0,0 +1,81 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/thermal/mediatek,mt8192-lvts.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek SoC LVTS thermal controller
> +
> +maintainers:
> +  - Yu-Chia Chang <ethan.chang@mediatek.com>
> +  - Ben Tseng <ben.tseng@mediatek.com>
> +
> +allOf:
> +  - $ref: thermal-sensor.yaml#
> +  - $ref: /nvmem/nvmem-consumer.yaml#

Besides having an error, there is no need to reference this. It is 
applied to all nodes and you still have to list the properties here to 
define how many entries and what they are.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - mediatek,mt8192-lvts
> +      - mediatek,mt8195-lvts
> +
> +  reg:
> +    minItems: 2
> +    maxItems: 4
> +
> +  interrupts:
> +    maxItems: 2
> +
> +  clocks:
> +    maxItems: 1
> +
> +  "#thermal-sensor-cells":
> +    const: 1
> +
> +  nvmem-cells:
> +    maxItems: 2
> +    description: Calibration data for thermal sensors

Need to define what each entry is.

> +
> +  nvmem-cell-names:
> +    items:
> +      - const: e_data1
> +      - const: e_data2

Wow, those are useful names.

> +
> +  resets:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array

Already has a type. Need to define how many entries and what each one is 
(if more than 1).

> +
> +
> +required:
> +  - '#thermal-sensor-cells'
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - nvmem-cells
> +  - nvmem-cell-names
> +  - resets
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/thermal/thermal.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/mt8195-clk.h>
> +    #include <dt-bindings/reset/mt8195-resets.h>
> +
> +    lvts: lvts@1100b000 {

Drop unused label.

thermal-sensor@...

> +        compatible = "mediatek,mt8195-lvts";
> +        #thermal-sensor-cells = <1>;
> +        reg = <0 0x1100b000 0 0x1000>,
> +              <0 0x11278000 0 0x1000>;
> +        interrupts = <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH 0>,
> +                     <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH 0>;
> +        clocks = <&infracfg_ao CLK_INFRA_AO_THERM>;
> +        resets = <&infracfg_ao MT8195_INFRA_RST0_THERM_CTRL_SWRST>,
> +                 <&infracfg_ao MT8195_INFRA_RST4_THERM_CTRL_MCU_SWRST>;
> +        nvmem-cells = <&lvts_e_data1 &lvts_e_data2>;
> +        nvmem-cell-names = "e_data1","e_data2";
> +    };
> +...
> -- 
> 2.35.1
> 
>
AngeloGioacchino Del Regno May 26, 2022, 12:58 p.m. UTC | #3 
Il 24/05/22 17:25, Alexandre Bailon ha scritto:
> This patch adds binding document for mt8192 and mt8195 thermal
> controllers.
> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
>   .../thermal/mediatek,mt8192-lvts.yaml         | 81 +++++++++++++++++++
>   1 file changed, 81 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/thermal/mediatek,mt8192-lvts.yaml
> 
> diff --git a/Documentation/devicetree/bindings/thermal/mediatek,mt8192-lvts.yaml b/Documentation/devicetree/bindings/thermal/mediatek,mt8192-lvts.yaml
> new file mode 100644
> index 000000000000..914c877d1f2f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/mediatek,mt8192-lvts.yaml
> @@ -0,0 +1,81 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/thermal/mediatek,mt8192-lvts.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek SoC LVTS thermal controller
> +
> +maintainers:
> +  - Yu-Chia Chang <ethan.chang@mediatek.com>
> +  - Ben Tseng <ben.tseng@mediatek.com>
> +
> +allOf:
> +  - $ref: thermal-sensor.yaml#
> +  - $ref: /nvmem/nvmem-consumer.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - mediatek,mt8192-lvts
> +      - mediatek,mt8195-lvts
> +
> +  reg:
> +    minItems: 2
> +    maxItems: 4
> +
> +  interrupts:
> +    maxItems: 2
> +
> +  clocks:
> +    maxItems: 1
> +
> +  "#thermal-sensor-cells":
> +    const: 1
> +
> +  nvmem-cells:
> +    maxItems: 2
> +    description: Calibration data for thermal sensors
> +
> +  nvmem-cell-names:
> +    items:
> +      - const: e_data1
> +      - const: e_data2

 From what I understand, each nvmem cell has calibration data for each thermal
sensor, so let's use some more descriptive names...
some ideas: "lvts1-calib" "lvts1-cal" "calib1".

Also, I think that the best option is to register one driver instance for each
hardware instance, meaning that you should have only one reg, only one nvmem-cell,
only one reset, only one interrupt. This makes it possible to have as many
LVTS instances as possible without any code adjustments in the future.

An example for MT8195:

thermal-sensor@1100b000 {
	compatible = "mediatek,mt8195-lvts";
	reg = <0 0x1100b000 0 0x1000>;
	interrupts = <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH 0>;
	#thermal-sensor-cells = <1>;
	clocks = <&infracfg_ao CLK_INFRA_AO_THERM>;
	nvmem-cells = <&lvts_e_data1>;
	nvmem-cell-names = "calibration";

	/* is this a reset for lvts1? */
	resets = <&infracfg_ao MT8195_INFRA_RST0_THERM_CTRL_SWRST>;
};

thermal-sensor@11278000 {
	compatible = "mediatek,mt8195-lvts";
	reg = <0 0x11278000 0 0x1000>;
	interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH 0>;
	#thermal-sensor-cells = <1>;
	clocks = <&infracfg_ao CLK_INFRA_AO_THERM>;
	nvmem-cells = <&lvts_e_data2>;
	nvmem-cell-names = "calibration";

	/* is this a reset for lvts2? */
	resets = <&infracfg_ao MT8195_INFRA_RST4_THERM_CTRL_MCU_SWRST>;
};

In the future, a new MediaTek SoC may simply define more nodes..... :-)

> +
> +  resets:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +
> +
> +required:
> +  - '#thermal-sensor-cells'
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - nvmem-cells
> +  - nvmem-cell-names
> +  - resets
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/thermal/thermal.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/mt8195-clk.h>
> +    #include <dt-bindings/reset/mt8195-resets.h>
> +
> +    lvts: lvts@1100b000 {

Please use generic names: this should be thermal-sensor@1100b000

> +        compatible = "mediatek,mt8195-lvts";
> +        #thermal-sensor-cells = <1>;
> +        reg = <0 0x1100b000 0 0x1000>,
> +              <0 0x11278000 0 0x1000>;
> +        interrupts = <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH 0>,
> +                     <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH 0>;
> +        clocks = <&infracfg_ao CLK_INFRA_AO_THERM>;
> +        resets = <&infracfg_ao MT8195_INFRA_RST0_THERM_CTRL_SWRST>,
> +                 <&infracfg_ao MT8195_INFRA_RST4_THERM_CTRL_MCU_SWRST>;
> +        nvmem-cells = <&lvts_e_data1 &lvts_e_data2>;
> +        nvmem-cell-names = "e_data1","e_data2";
> +    };
> +...
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/thermal/mediatek,mt8192-lvts.yaml b/Documentation/devicetree/bindings/thermal/mediatek,mt8192-lvts.yaml
new file mode 100644
index 000000000000..914c877d1f2f
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/mediatek,mt8192-lvts.yaml
@@ -0,0 +1,81 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/mediatek,mt8192-lvts.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek SoC LVTS thermal controller
+
+maintainers:
+  - Yu-Chia Chang <ethan.chang@mediatek.com>
+  - Ben Tseng <ben.tseng@mediatek.com>
+
+allOf:
+  - $ref: thermal-sensor.yaml#
+  - $ref: /nvmem/nvmem-consumer.yaml#
+
+properties:
+  compatible:
+    enum:
+      - mediatek,mt8192-lvts
+      - mediatek,mt8195-lvts
+
+  reg:
+    minItems: 2
+    maxItems: 4
+
+  interrupts:
+    maxItems: 2
+
+  clocks:
+    maxItems: 1
+
+  "#thermal-sensor-cells":
+    const: 1
+
+  nvmem-cells:
+    maxItems: 2
+    description: Calibration data for thermal sensors
+
+  nvmem-cell-names:
+    items:
+      - const: e_data1
+      - const: e_data2
+
+  resets:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+
+
+required:
+  - '#thermal-sensor-cells'
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - nvmem-cells
+  - nvmem-cell-names
+  - resets
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/thermal/thermal.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/mt8195-clk.h>
+    #include <dt-bindings/reset/mt8195-resets.h>
+
+    lvts: lvts@1100b000 {
+        compatible = "mediatek,mt8195-lvts";
+        #thermal-sensor-cells = <1>;
+        reg = <0 0x1100b000 0 0x1000>,
+              <0 0x11278000 0 0x1000>;
+        interrupts = <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH 0>,
+                     <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH 0>;
+        clocks = <&infracfg_ao CLK_INFRA_AO_THERM>;
+        resets = <&infracfg_ao MT8195_INFRA_RST0_THERM_CTRL_SWRST>,
+                 <&infracfg_ao MT8195_INFRA_RST4_THERM_CTRL_MCU_SWRST>;
+        nvmem-cells = <&lvts_e_data1 &lvts_e_data2>;
+        nvmem-cell-names = "e_data1","e_data2";
+    };
+...