[PATCH] dt-bindings: thermal: convert hisilicon-thermal.txt to dt-schema

[PATCH] dt-bindings: thermal: convert hisilicon-thermal.txt to dt-schema

[Abdulrasaq Lawani]

Convert the hisilicon SoCs tsensor txt bindings to dt-schema

Signed-off-by: Abdulrasaq Lawani <abdulrasaqolawani@gmail.com>
---
Validated with dtschema and tested against `hi3660-hikey960.dts`

 .../bindings/thermal/hisilicon-thermal.txt    | 32 ---------
 .../bindings/thermal/hisilicon-thermal.yaml   | 71 +++++++++++++++++++
 2 files changed, 71 insertions(+), 32 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/thermal/hisilicon-thermal.txt
 create mode 100644 Documentation/devicetree/bindings/thermal/hisilicon-thermal.yaml

diff --git a/Documentation/devicetree/bindings/thermal/hisilicon-thermal.txt b/Documentation/devicetree/bindings/thermal/hisilicon-thermal.txt
deleted file mode 100644
index 4b19d80e6558..000000000000
--- a/Documentation/devicetree/bindings/thermal/hisilicon-thermal.txt
+++ /dev/null
@@ -1,32 +0,0 @@
-* Temperature Sensor on hisilicon SoCs
-
-** Required properties :
-
-- compatible: "hisilicon,tsensor".
-- reg: physical base address of thermal sensor and length of memory mapped
-  region.
-- interrupt: The interrupt number to the cpu. Defines the interrupt used
-  by /SOCTHERM/tsensor.
-- clock-names: Input clock name, should be 'thermal_clk'.
-- clocks: phandles for clock specified in "clock-names" property.
-- #thermal-sensor-cells: Should be 1. See Documentation/devicetree/bindings/thermal/thermal-sensor.yaml for a description.
-
-Example :
-
-for Hi6220:
-	tsensor: tsensor@0,f7030700 {
-		compatible = "hisilicon,tsensor";
-		reg = <0x0 0xf7030700 0x0 0x1000>;
-		interrupts = <0 7 0x4>;
-		clocks = <&sys_ctrl HI6220_TSENSOR_CLK>;
-		clock-names = "thermal_clk";
-		#thermal-sensor-cells = <1>;
-	}
-
-for Hi3660:
-	tsensor: tsensor@fff30000 {
-		compatible = "hisilicon,hi3660-tsensor";
-		reg = <0x0 0xfff30000 0x0 0x1000>;
-		interrupts = <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>;
-		#thermal-sensor-cells = <1>;
-	};
diff --git a/Documentation/devicetree/bindings/thermal/hisilicon-thermal.yaml b/Documentation/devicetree/bindings/thermal/hisilicon-thermal.yaml
new file mode 100644
index 000000000000..56ded6ebe1b2
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/hisilicon-thermal.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/hisilicon-thermal.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Temperature Sensor on hisilicon SoCs
+
+maintainers:
+  - Abdulrasaq Lawani <abdulrasaqolawani@gmail.com>
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - hisilicon,tsensor
+          - hisilicon,hi3660-tsensor
+
+  reg:
+    description: physical base address of thermal sensor and length of memory mapped region.
+    minItems: 1
+    maxItems: 2
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: thermal_clk
+
+  interrupts:
+    description:
+      The interrupt number to the cpu. Defines the interrupt used
+      by /SOCTHERM/tsensor.
+    maxItems: 1
+
+  # See Documentation/devicetree/bindings/thermal/thermal-sensor.yaml for details
+  '#thermal-sensor-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - '#thermal-sensor-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/hi6220-clock.h>
+
+     // for Hi6220:
+     tsensor: tsensor@0,f7030700 {
+     compatible = "hisilicon,tsensor";
+     reg = <0x0 0xf7030700 0x0 0x1000>;
+     interrupts = <0 7 0x4>;
+     clocks = <&sys_ctrl HI6220_TSENSOR_CLK>;
+     clock-names = "thermal_clk";
+     #thermal-sensor-cells = <1>;
+     };
+
+     // for Hi3660:
+     tsensor1: tsensor@fff30000 {
+     compatible = "hisilicon,hi3660-tsensor";
+     reg = <0x0 0xfff30000 0x0 0x1000>;
+     interrupts = <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>;
+     #thermal-sensor-cells = <1>;
+     };
+
-- 
2.43.0

Re: [PATCH] dt-bindings: thermal: convert hisilicon-thermal.txt to dt-schema

[Krzysztof Kozlowski]

On 14/06/2024 00:42, Abdulrasaq Lawani wrote:
> Convert the hisilicon SoCs tsensor txt bindings to dt-schema
> 
> Signed-off-by: Abdulrasaq Lawani <abdulrasaqolawani@gmail.com>
> ---
> Validated with dtschema and tested against `hi3660-hikey960.dts`
> 


> diff --git a/Documentation/devicetree/bindings/thermal/hisilicon-thermal.yaml b/Documentation/devicetree/bindings/thermal/hisilicon-thermal.yaml
> new file mode 100644
> index 000000000000..56ded6ebe1b2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/hisilicon-thermal.yaml

Filename: hisilicon,tsensor.yaml

> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/thermal/hisilicon-thermal.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Temperature Sensor on hisilicon SoCs
> +
> +maintainers:
> +  - Abdulrasaq Lawani <abdulrasaqolawani@gmail.com>
> +

Missing $ref to thermal-sensor.yaml#

> +properties:
> +  compatible:
> +    items:

No need for items

> +      - enum:
> +          - hisilicon,tsensor
> +          - hisilicon,hi3660-tsensor
> +
> +  reg:
> +    description: physical base address of thermal sensor and length of memory mapped region.

Drop description, pointless.

> +    minItems: 1
> +    maxItems: 2

Instead you need to list items and describe them. But don't repeat
redundant parts like "physical base address". Just say which block is this.

Or... it's just wrong. Why two items?

> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: thermal_clk
> +
> +  interrupts:
> +    description:
> +      The interrupt number to the cpu. Defines the interrupt used
> +      by /SOCTHERM/tsensor.

No need for description, it's redudant.

> +    maxItems: 1
> +
> +  # See Documentation/devicetree/bindings/thermal/thermal-sensor.yaml for details

Drop

> +  '#thermal-sensor-cells':
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - '#thermal-sensor-cells'
> +
> +additionalProperties: false

unevaluatedProperties instead (after adding ref)
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/hi6220-clock.h>
> +
> +     // for Hi6220:
> +     tsensor: tsensor@0,f7030700 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> +     compatible = "hisilicon,tsensor";
> +     reg = <0x0 0xf7030700 0x0 0x1000>;

Oh man...  get this past your mentors first.

1. Messed indentation.
2. 0, unit address looks unnecessary.


> +     interrupts = <0 7 0x4>;

Use proper defines for both common constants. There is a reason you
included arm-gic header, right?

> +     clocks = <&sys_ctrl HI6220_TSENSOR_CLK>;
> +     clock-names = "thermal_clk";
> +     #thermal-sensor-cells = <1>;
> +     };
> +
> +     // for Hi3660:
> +     tsensor1: tsensor@fff30000 {

Drop entire node. One example is enough.


Best regards,
Krzysztof

Re: [PATCH] dt-bindings: thermal: convert hisilicon-thermal.txt to dt-schema

[Javier Carrasco]

On 14/06/2024 10:49, Krzysztof Kozlowski wrote:
> On 14/06/2024 00:42, Abdulrasaq Lawani wrote:
>> Convert the hisilicon SoCs tsensor txt bindings to dt-schema
>>
>> Signed-off-by: Abdulrasaq Lawani <abdulrasaqolawani@gmail.com>
>> ---
>> Validated with dtschema and tested against `hi3660-hikey960.dts`
>>
> 
>> +     compatible = "hisilicon,tsensor";
>> +     reg = <0x0 0xf7030700 0x0 0x1000>;
> 
> Oh man...  get this past your mentors first.
> 
> 1. Messed indentation.
> 2. 0, unit address looks unnecessary.
> 

Sorry Krzysztof, it seems that this conversion went "live" without a
preliminary review by the mentors. We will make sure that the first step
is not omitted for v2.

Best regards,
Javier Carrasco