All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] arm64: dts: rockchip: add timer0 clocks on rk3368
@ 2021-04-24 19:19 ` Ezequiel Garcia
  0 siblings, 0 replies; 12+ messages in thread
From: Ezequiel Garcia @ 2021-04-24 19:19 UTC (permalink / raw)
  To: linux-rockchip, devicetree
  Cc: Heiko Stuebner, Kever Yang, Daniel Lezcano, Rob Herring,
	Ezequiel Garcia, kernel

The timer driver requires pclk and sclk clocks
to be present in the device tree node, so add them.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 arch/arm64/boot/dts/rockchip/rk3368.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3368.dtsi b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
index 242f821a90ba..548be81c9d3f 100644
--- a/arch/arm64/boot/dts/rockchip/rk3368.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
@@ -668,6 +668,8 @@ timer0: timer@ff810000 {
 		compatible = "rockchip,rk3368-timer", "rockchip,rk3288-timer";
 		reg = <0x0 0xff810000 0x0 0x20>;
 		interrupts = <GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cru PCLK_TIMER0>, <&cru SCLK_TIMER00>;
+		clock-names = "pclk", "timer";
 	};
 
 	spdif: spdif@ff880000 {
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 1/2] arm64: dts: rockchip: add timer0 clocks on rk3368
@ 2021-04-24 19:19 ` Ezequiel Garcia
  0 siblings, 0 replies; 12+ messages in thread
From: Ezequiel Garcia @ 2021-04-24 19:19 UTC (permalink / raw)
  To: linux-rockchip, devicetree
  Cc: Heiko Stuebner, Kever Yang, Daniel Lezcano, Rob Herring,
	Ezequiel Garcia, kernel

The timer driver requires pclk and sclk clocks
to be present in the device tree node, so add them.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 arch/arm64/boot/dts/rockchip/rk3368.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3368.dtsi b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
index 242f821a90ba..548be81c9d3f 100644
--- a/arch/arm64/boot/dts/rockchip/rk3368.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
@@ -668,6 +668,8 @@ timer0: timer@ff810000 {
 		compatible = "rockchip,rk3368-timer", "rockchip,rk3288-timer";
 		reg = <0x0 0xff810000 0x0 0x20>;
 		interrupts = <GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cru PCLK_TIMER0>, <&cru SCLK_TIMER00>;
+		clock-names = "pclk", "timer";
 	};
 
 	spdif: spdif@ff880000 {
-- 
2.30.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] dt-bindings: timer: convert rockchip,rk-timer.txt to YAML
  2021-04-24 19:19 ` Ezequiel Garcia
@ 2021-04-24 19:19   ` Ezequiel Garcia
  -1 siblings, 0 replies; 12+ messages in thread
From: Ezequiel Garcia @ 2021-04-24 19:19 UTC (permalink / raw)
  To: linux-rockchip, devicetree
  Cc: Heiko Stuebner, Kever Yang, Daniel Lezcano, Rob Herring,
	Ezequiel Garcia, kernel

Convert Rockchip Timer dt-bindings to YAML.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 .../bindings/timer/rockchip,rk-timer.txt      | 27 --------
 .../bindings/timer/rockchip,rk-timer.yaml     | 67 +++++++++++++++++++
 2 files changed, 67 insertions(+), 27 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
 create mode 100644 Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml

diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
deleted file mode 100644
index d65fdce7c7f0..000000000000
--- a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
+++ /dev/null
@@ -1,27 +0,0 @@
-Rockchip rk timer
-
-Required properties:
-- compatible: should be:
-  "rockchip,rv1108-timer", "rockchip,rk3288-timer": for Rockchip RV1108
-  "rockchip,rk3036-timer", "rockchip,rk3288-timer": for Rockchip RK3036
-  "rockchip,rk3066-timer", "rockchip,rk3288-timer": for Rockchip RK3066
-  "rockchip,rk3188-timer", "rockchip,rk3288-timer": for Rockchip RK3188
-  "rockchip,rk3228-timer", "rockchip,rk3288-timer": for Rockchip RK3228
-  "rockchip,rk3229-timer", "rockchip,rk3288-timer": for Rockchip RK3229
-  "rockchip,rk3288-timer": for Rockchip RK3288
-  "rockchip,rk3368-timer", "rockchip,rk3288-timer": for Rockchip RK3368
-  "rockchip,rk3399-timer": for Rockchip RK3399
-- reg: base address of the timer register starting with TIMERS CONTROL register
-- interrupts: should contain the interrupts for Timer0
-- clocks : must contain an entry for each entry in clock-names
-- clock-names : must include the following entries:
-  "timer", "pclk"
-
-Example:
-	timer: timer@ff810000 {
-		compatible = "rockchip,rk3288-timer";
-		reg = <0xff810000 0x20>;
-		interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&xin24m>, <&cru PCLK_TIMER>;
-		clock-names = "timer", "pclk";
-	};
diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
new file mode 100644
index 000000000000..f1bc3ac7abc8
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/timer/rockchip,rk-timer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip Timer Device Tree Bindings
+
+maintainers:
+  - Daniel Lezcano <daniel.lezcano@linaro.org>
+
+properties:
+  compatible:
+    oneOf:
+      - const: rockchip,rk3288-timer
+      - const: rockchip,rk3399-timer
+      - items:
+          - enum:
+            - rockchip,rv1108-timer
+            - rockchip,rk3036-timer
+            - rockchip,rk3066-timer
+            - rockchip,rk3188-timer
+            - rockchip,rk3228-timer
+            - rockchip,rk3229-timer
+            - rockchip,rk3288-timer
+            - rockchip,rk3368-timer
+            - rockchip,px30-timer
+          - const: rockchip,rk3288-timer
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 2
+    maxItems: 2
+
+  clock-names:
+    items:
+      enum:
+        - timer
+        - pclk
+    minItems: 2
+    maxItems: 2
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/rk3288-cru.h>
+
+    timer: timer@ff810000 {
+        compatible = "rockchip,rk3288-timer";
+        reg = <0xff810000 0x20>;
+        interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&xin24m>, <&cru PCLK_TIMER>;
+        clock-names = "timer", "pclk";
+    };
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] dt-bindings: timer: convert rockchip, rk-timer.txt to YAML
@ 2021-04-24 19:19   ` Ezequiel Garcia
  0 siblings, 0 replies; 12+ messages in thread
From: Ezequiel Garcia @ 2021-04-24 19:19 UTC (permalink / raw)
  To: linux-rockchip, devicetree
  Cc: Heiko Stuebner, Kever Yang, Daniel Lezcano, Rob Herring,
	Ezequiel Garcia, kernel

Convert Rockchip Timer dt-bindings to YAML.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 .../bindings/timer/rockchip,rk-timer.txt      | 27 --------
 .../bindings/timer/rockchip,rk-timer.yaml     | 67 +++++++++++++++++++
 2 files changed, 67 insertions(+), 27 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
 create mode 100644 Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml

diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
deleted file mode 100644
index d65fdce7c7f0..000000000000
--- a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
+++ /dev/null
@@ -1,27 +0,0 @@
-Rockchip rk timer
-
-Required properties:
-- compatible: should be:
-  "rockchip,rv1108-timer", "rockchip,rk3288-timer": for Rockchip RV1108
-  "rockchip,rk3036-timer", "rockchip,rk3288-timer": for Rockchip RK3036
-  "rockchip,rk3066-timer", "rockchip,rk3288-timer": for Rockchip RK3066
-  "rockchip,rk3188-timer", "rockchip,rk3288-timer": for Rockchip RK3188
-  "rockchip,rk3228-timer", "rockchip,rk3288-timer": for Rockchip RK3228
-  "rockchip,rk3229-timer", "rockchip,rk3288-timer": for Rockchip RK3229
-  "rockchip,rk3288-timer": for Rockchip RK3288
-  "rockchip,rk3368-timer", "rockchip,rk3288-timer": for Rockchip RK3368
-  "rockchip,rk3399-timer": for Rockchip RK3399
-- reg: base address of the timer register starting with TIMERS CONTROL register
-- interrupts: should contain the interrupts for Timer0
-- clocks : must contain an entry for each entry in clock-names
-- clock-names : must include the following entries:
-  "timer", "pclk"
-
-Example:
-	timer: timer@ff810000 {
-		compatible = "rockchip,rk3288-timer";
-		reg = <0xff810000 0x20>;
-		interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&xin24m>, <&cru PCLK_TIMER>;
-		clock-names = "timer", "pclk";
-	};
diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
new file mode 100644
index 000000000000..f1bc3ac7abc8
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/timer/rockchip,rk-timer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip Timer Device Tree Bindings
+
+maintainers:
+  - Daniel Lezcano <daniel.lezcano@linaro.org>
+
+properties:
+  compatible:
+    oneOf:
+      - const: rockchip,rk3288-timer
+      - const: rockchip,rk3399-timer
+      - items:
+          - enum:
+            - rockchip,rv1108-timer
+            - rockchip,rk3036-timer
+            - rockchip,rk3066-timer
+            - rockchip,rk3188-timer
+            - rockchip,rk3228-timer
+            - rockchip,rk3229-timer
+            - rockchip,rk3288-timer
+            - rockchip,rk3368-timer
+            - rockchip,px30-timer
+          - const: rockchip,rk3288-timer
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 2
+    maxItems: 2
+
+  clock-names:
+    items:
+      enum:
+        - timer
+        - pclk
+    minItems: 2
+    maxItems: 2
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/rk3288-cru.h>
+
+    timer: timer@ff810000 {
+        compatible = "rockchip,rk3288-timer";
+        reg = <0xff810000 0x20>;
+        interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&xin24m>, <&cru PCLK_TIMER>;
+        clock-names = "timer", "pclk";
+    };
-- 
2.30.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] dt-bindings: timer: convert rockchip,rk-timer.txt to YAML
  2021-04-24 19:19   ` [PATCH 2/2] dt-bindings: timer: convert rockchip, rk-timer.txt " Ezequiel Garcia
@ 2021-04-26 21:04     ` Rob Herring
  -1 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2021-04-26 21:04 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: Heiko Stuebner, linux-rockchip, devicetree, Kever Yang,
	Rob Herring, Daniel Lezcano, kernel

On Sat, 24 Apr 2021 16:19:46 -0300, Ezequiel Garcia wrote:
> Convert Rockchip Timer dt-bindings to YAML.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  .../bindings/timer/rockchip,rk-timer.txt      | 27 --------
>  .../bindings/timer/rockchip,rk-timer.yaml     | 67 +++++++++++++++++++
>  2 files changed, 67 insertions(+), 27 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
>  create mode 100644 Documentation/devicetree/bindings/timer/rockchip,rk-timer.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:
./Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml:19:13: [warning] wrong indentation: expected 14 but found 12 (indentation)

dtschema/dtc warnings/errors:

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

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.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] dt-bindings: timer: convert rockchip, rk-timer.txt to YAML
@ 2021-04-26 21:04     ` Rob Herring
  0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2021-04-26 21:04 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: Heiko Stuebner, linux-rockchip, devicetree, Kever Yang,
	Rob Herring, Daniel Lezcano, kernel

On Sat, 24 Apr 2021 16:19:46 -0300, Ezequiel Garcia wrote:
> Convert Rockchip Timer dt-bindings to YAML.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  .../bindings/timer/rockchip,rk-timer.txt      | 27 --------
>  .../bindings/timer/rockchip,rk-timer.yaml     | 67 +++++++++++++++++++
>  2 files changed, 67 insertions(+), 27 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
>  create mode 100644 Documentation/devicetree/bindings/timer/rockchip,rk-timer.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:
./Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml:19:13: [warning] wrong indentation: expected 14 but found 12 (indentation)

dtschema/dtc warnings/errors:

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

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.


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] dt-bindings: timer: convert rockchip,rk-timer.txt to YAML
  2021-04-24 19:19   ` [PATCH 2/2] dt-bindings: timer: convert rockchip, rk-timer.txt " Ezequiel Garcia
@ 2021-04-29 16:25     ` Rob Herring
  -1 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2021-04-29 16:25 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: linux-rockchip, devicetree, Heiko Stuebner, Kever Yang,
	Daniel Lezcano, kernel

On Sat, Apr 24, 2021 at 04:19:46PM -0300, Ezequiel Garcia wrote:
> Convert Rockchip Timer dt-bindings to YAML.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  .../bindings/timer/rockchip,rk-timer.txt      | 27 --------
>  .../bindings/timer/rockchip,rk-timer.yaml     | 67 +++++++++++++++++++
>  2 files changed, 67 insertions(+), 27 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
>  create mode 100644 Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
> 
> diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
> deleted file mode 100644
> index d65fdce7c7f0..000000000000
> --- a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
> +++ /dev/null
> @@ -1,27 +0,0 @@
> -Rockchip rk timer
> -
> -Required properties:
> -- compatible: should be:
> -  "rockchip,rv1108-timer", "rockchip,rk3288-timer": for Rockchip RV1108
> -  "rockchip,rk3036-timer", "rockchip,rk3288-timer": for Rockchip RK3036
> -  "rockchip,rk3066-timer", "rockchip,rk3288-timer": for Rockchip RK3066
> -  "rockchip,rk3188-timer", "rockchip,rk3288-timer": for Rockchip RK3188
> -  "rockchip,rk3228-timer", "rockchip,rk3288-timer": for Rockchip RK3228
> -  "rockchip,rk3229-timer", "rockchip,rk3288-timer": for Rockchip RK3229
> -  "rockchip,rk3288-timer": for Rockchip RK3288
> -  "rockchip,rk3368-timer", "rockchip,rk3288-timer": for Rockchip RK3368
> -  "rockchip,rk3399-timer": for Rockchip RK3399
> -- reg: base address of the timer register starting with TIMERS CONTROL register
> -- interrupts: should contain the interrupts for Timer0
> -- clocks : must contain an entry for each entry in clock-names
> -- clock-names : must include the following entries:
> -  "timer", "pclk"
> -
> -Example:
> -	timer: timer@ff810000 {
> -		compatible = "rockchip,rk3288-timer";
> -		reg = <0xff810000 0x20>;
> -		interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
> -		clocks = <&xin24m>, <&cru PCLK_TIMER>;
> -		clock-names = "timer", "pclk";
> -	};
> diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
> new file mode 100644
> index 000000000000..f1bc3ac7abc8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/timer/rockchip,rk-timer.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip Timer Device Tree Bindings
> +
> +maintainers:
> +  - Daniel Lezcano <daniel.lezcano@linaro.org>

This should be someone that knows the h/w and cares about Rockchip.

> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: rockchip,rk3288-timer
> +      - const: rockchip,rk3399-timer
> +      - items:
> +          - enum:
> +            - rockchip,rv1108-timer
> +            - rockchip,rk3036-timer
> +            - rockchip,rk3066-timer
> +            - rockchip,rk3188-timer
> +            - rockchip,rk3228-timer
> +            - rockchip,rk3229-timer
> +            - rockchip,rk3288-timer
> +            - rockchip,rk3368-timer
> +            - rockchip,px30-timer
> +          - const: rockchip,rk3288-timer
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 2
> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      enum:
> +        - timer
> +        - pclk

We can't define the order here? We should fix dts files if they are 
inconsistent.

> +    minItems: 2
> +    maxItems: 2
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/rk3288-cru.h>
> +
> +    timer: timer@ff810000 {
> +        compatible = "rockchip,rk3288-timer";
> +        reg = <0xff810000 0x20>;
> +        interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&xin24m>, <&cru PCLK_TIMER>;
> +        clock-names = "timer", "pclk";
> +    };
> -- 
> 2.30.0
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] dt-bindings: timer: convert rockchip, rk-timer.txt to YAML
@ 2021-04-29 16:25     ` Rob Herring
  0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2021-04-29 16:25 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: linux-rockchip, devicetree, Heiko Stuebner, Kever Yang,
	Daniel Lezcano, kernel

On Sat, Apr 24, 2021 at 04:19:46PM -0300, Ezequiel Garcia wrote:
> Convert Rockchip Timer dt-bindings to YAML.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  .../bindings/timer/rockchip,rk-timer.txt      | 27 --------
>  .../bindings/timer/rockchip,rk-timer.yaml     | 67 +++++++++++++++++++
>  2 files changed, 67 insertions(+), 27 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
>  create mode 100644 Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
> 
> diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
> deleted file mode 100644
> index d65fdce7c7f0..000000000000
> --- a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
> +++ /dev/null
> @@ -1,27 +0,0 @@
> -Rockchip rk timer
> -
> -Required properties:
> -- compatible: should be:
> -  "rockchip,rv1108-timer", "rockchip,rk3288-timer": for Rockchip RV1108
> -  "rockchip,rk3036-timer", "rockchip,rk3288-timer": for Rockchip RK3036
> -  "rockchip,rk3066-timer", "rockchip,rk3288-timer": for Rockchip RK3066
> -  "rockchip,rk3188-timer", "rockchip,rk3288-timer": for Rockchip RK3188
> -  "rockchip,rk3228-timer", "rockchip,rk3288-timer": for Rockchip RK3228
> -  "rockchip,rk3229-timer", "rockchip,rk3288-timer": for Rockchip RK3229
> -  "rockchip,rk3288-timer": for Rockchip RK3288
> -  "rockchip,rk3368-timer", "rockchip,rk3288-timer": for Rockchip RK3368
> -  "rockchip,rk3399-timer": for Rockchip RK3399
> -- reg: base address of the timer register starting with TIMERS CONTROL register
> -- interrupts: should contain the interrupts for Timer0
> -- clocks : must contain an entry for each entry in clock-names
> -- clock-names : must include the following entries:
> -  "timer", "pclk"
> -
> -Example:
> -	timer: timer@ff810000 {
> -		compatible = "rockchip,rk3288-timer";
> -		reg = <0xff810000 0x20>;
> -		interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
> -		clocks = <&xin24m>, <&cru PCLK_TIMER>;
> -		clock-names = "timer", "pclk";
> -	};
> diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
> new file mode 100644
> index 000000000000..f1bc3ac7abc8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/timer/rockchip,rk-timer.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip Timer Device Tree Bindings
> +
> +maintainers:
> +  - Daniel Lezcano <daniel.lezcano@linaro.org>

This should be someone that knows the h/w and cares about Rockchip.

> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: rockchip,rk3288-timer
> +      - const: rockchip,rk3399-timer
> +      - items:
> +          - enum:
> +            - rockchip,rv1108-timer
> +            - rockchip,rk3036-timer
> +            - rockchip,rk3066-timer
> +            - rockchip,rk3188-timer
> +            - rockchip,rk3228-timer
> +            - rockchip,rk3229-timer
> +            - rockchip,rk3288-timer
> +            - rockchip,rk3368-timer
> +            - rockchip,px30-timer
> +          - const: rockchip,rk3288-timer
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 2
> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      enum:
> +        - timer
> +        - pclk

We can't define the order here? We should fix dts files if they are 
inconsistent.

> +    minItems: 2
> +    maxItems: 2
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/rk3288-cru.h>
> +
> +    timer: timer@ff810000 {
> +        compatible = "rockchip,rk3288-timer";
> +        reg = <0xff810000 0x20>;
> +        interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&xin24m>, <&cru PCLK_TIMER>;
> +        clock-names = "timer", "pclk";
> +    };
> -- 
> 2.30.0
> 

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] dt-bindings: timer: convert rockchip,rk-timer.txt to YAML
  2021-04-29 16:25     ` [PATCH 2/2] dt-bindings: timer: convert rockchip, rk-timer.txt " Rob Herring
@ 2021-04-29 17:03       ` Ezequiel Garcia
  -1 siblings, 0 replies; 12+ messages in thread
From: Ezequiel Garcia @ 2021-04-29 17:03 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-rockchip, devicetree, Heiko Stuebner, Kever Yang,
	Daniel Lezcano, kernel

On Thu, 2021-04-29 at 11:25 -0500, Rob Herring wrote:
> On Sat, Apr 24, 2021 at 04:19:46PM -0300, Ezequiel Garcia wrote:
> > Convert Rockchip Timer dt-bindings to YAML.
> > 
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > ---
> >  .../bindings/timer/rockchip,rk-timer.txt      | 27 --------
> >  .../bindings/timer/rockchip,rk-timer.yaml     | 67 +++++++++++++++++++
> >  2 files changed, 67 insertions(+), 27 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
> >  create mode 100644 Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
> > deleted file mode 100644
> > index d65fdce7c7f0..000000000000
> > --- a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
> > +++ /dev/null
> > @@ -1,27 +0,0 @@
> > -Rockchip rk timer
> > -
> > -Required properties:
> > -- compatible: should be:
> > -  "rockchip,rv1108-timer", "rockchip,rk3288-timer": for Rockchip RV1108
> > -  "rockchip,rk3036-timer", "rockchip,rk3288-timer": for Rockchip RK3036
> > -  "rockchip,rk3066-timer", "rockchip,rk3288-timer": for Rockchip RK3066
> > -  "rockchip,rk3188-timer", "rockchip,rk3288-timer": for Rockchip RK3188
> > -  "rockchip,rk3228-timer", "rockchip,rk3288-timer": for Rockchip RK3228
> > -  "rockchip,rk3229-timer", "rockchip,rk3288-timer": for Rockchip RK3229
> > -  "rockchip,rk3288-timer": for Rockchip RK3288
> > -  "rockchip,rk3368-timer", "rockchip,rk3288-timer": for Rockchip RK3368
> > -  "rockchip,rk3399-timer": for Rockchip RK3399
> > -- reg: base address of the timer register starting with TIMERS CONTROL register
> > -- interrupts: should contain the interrupts for Timer0
> > -- clocks : must contain an entry for each entry in clock-names
> > -- clock-names : must include the following entries:
> > -  "timer", "pclk"
> > -
> > -Example:
> > -       timer: timer@ff810000 {
> > -               compatible = "rockchip,rk3288-timer";
> > -               reg = <0xff810000 0x20>;
> > -               interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
> > -               clocks = <&xin24m>, <&cru PCLK_TIMER>;
> > -               clock-names = "timer", "pclk";
> > -       };
> > diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
> > new file mode 100644
> > index 000000000000..f1bc3ac7abc8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
> > @@ -0,0 +1,67 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/timer/rockchip,rk-timer.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Rockchip Timer Device Tree Bindings
> > +
> > +maintainers:
> > +  - Daniel Lezcano <daniel.lezcano@linaro.org>
> 
> This should be someone that knows the h/w and cares about Rockchip.
> 

Daniel wrote the driver, so I figured he'd care :)
If not, perhaps Heiko (if he agrees)?

> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - const: rockchip,rk3288-timer
> > +      - const: rockchip,rk3399-timer
> > +      - items:
> > +          - enum:
> > +            - rockchip,rv1108-timer
> > +            - rockchip,rk3036-timer
> > +            - rockchip,rk3066-timer
> > +            - rockchip,rk3188-timer
> > +            - rockchip,rk3228-timer
> > +            - rockchip,rk3229-timer
> > +            - rockchip,rk3288-timer
> > +            - rockchip,rk3368-timer
> > +            - rockchip,px30-timer
> > +          - const: rockchip,rk3288-timer
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    minItems: 2
> > +    maxItems: 2
> > +
> > +  clock-names:
> > +    items:
> > +      enum:
> > +        - timer
> > +        - pclk
> 
> We can't define the order here? We should fix dts files if they are 
> inconsistent.
> 

The driver requests the clocks by name, and unfortunately DTS
rely on that. We can change all the DTSI, but wouldn't that
be too much trouble for something that is currently working fine?

Why is the order important?

> > +    minItems: 2
> > +    maxItems: 2
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/clock/rk3288-cru.h>
> > +
> > +    timer: timer@ff810000 {
> > +        compatible = "rockchip,rk3288-timer";
> > +        reg = <0xff810000 0x20>;
> > +        interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
> > +        clocks = <&xin24m>, <&cru PCLK_TIMER>;
> > +        clock-names = "timer", "pclk";
> > +    };
> > -- 
> > 2.30.0
> > 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] dt-bindings: timer: convert rockchip,rk-timer.txt to YAML
@ 2021-04-29 17:03       ` Ezequiel Garcia
  0 siblings, 0 replies; 12+ messages in thread
From: Ezequiel Garcia @ 2021-04-29 17:03 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-rockchip, devicetree, Heiko Stuebner, Kever Yang,
	Daniel Lezcano, kernel

On Thu, 2021-04-29 at 11:25 -0500, Rob Herring wrote:
> On Sat, Apr 24, 2021 at 04:19:46PM -0300, Ezequiel Garcia wrote:
> > Convert Rockchip Timer dt-bindings to YAML.
> > 
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > ---
> >  .../bindings/timer/rockchip,rk-timer.txt      | 27 --------
> >  .../bindings/timer/rockchip,rk-timer.yaml     | 67 +++++++++++++++++++
> >  2 files changed, 67 insertions(+), 27 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
> >  create mode 100644 Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
> > deleted file mode 100644
> > index d65fdce7c7f0..000000000000
> > --- a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
> > +++ /dev/null
> > @@ -1,27 +0,0 @@
> > -Rockchip rk timer
> > -
> > -Required properties:
> > -- compatible: should be:
> > -  "rockchip,rv1108-timer", "rockchip,rk3288-timer": for Rockchip RV1108
> > -  "rockchip,rk3036-timer", "rockchip,rk3288-timer": for Rockchip RK3036
> > -  "rockchip,rk3066-timer", "rockchip,rk3288-timer": for Rockchip RK3066
> > -  "rockchip,rk3188-timer", "rockchip,rk3288-timer": for Rockchip RK3188
> > -  "rockchip,rk3228-timer", "rockchip,rk3288-timer": for Rockchip RK3228
> > -  "rockchip,rk3229-timer", "rockchip,rk3288-timer": for Rockchip RK3229
> > -  "rockchip,rk3288-timer": for Rockchip RK3288
> > -  "rockchip,rk3368-timer", "rockchip,rk3288-timer": for Rockchip RK3368
> > -  "rockchip,rk3399-timer": for Rockchip RK3399
> > -- reg: base address of the timer register starting with TIMERS CONTROL register
> > -- interrupts: should contain the interrupts for Timer0
> > -- clocks : must contain an entry for each entry in clock-names
> > -- clock-names : must include the following entries:
> > -  "timer", "pclk"
> > -
> > -Example:
> > -       timer: timer@ff810000 {
> > -               compatible = "rockchip,rk3288-timer";
> > -               reg = <0xff810000 0x20>;
> > -               interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
> > -               clocks = <&xin24m>, <&cru PCLK_TIMER>;
> > -               clock-names = "timer", "pclk";
> > -       };
> > diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
> > new file mode 100644
> > index 000000000000..f1bc3ac7abc8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
> > @@ -0,0 +1,67 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/timer/rockchip,rk-timer.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Rockchip Timer Device Tree Bindings
> > +
> > +maintainers:
> > +  - Daniel Lezcano <daniel.lezcano@linaro.org>
> 
> This should be someone that knows the h/w and cares about Rockchip.
> 

Daniel wrote the driver, so I figured he'd care :)
If not, perhaps Heiko (if he agrees)?

> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - const: rockchip,rk3288-timer
> > +      - const: rockchip,rk3399-timer
> > +      - items:
> > +          - enum:
> > +            - rockchip,rv1108-timer
> > +            - rockchip,rk3036-timer
> > +            - rockchip,rk3066-timer
> > +            - rockchip,rk3188-timer
> > +            - rockchip,rk3228-timer
> > +            - rockchip,rk3229-timer
> > +            - rockchip,rk3288-timer
> > +            - rockchip,rk3368-timer
> > +            - rockchip,px30-timer
> > +          - const: rockchip,rk3288-timer
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    minItems: 2
> > +    maxItems: 2
> > +
> > +  clock-names:
> > +    items:
> > +      enum:
> > +        - timer
> > +        - pclk
> 
> We can't define the order here? We should fix dts files if they are 
> inconsistent.
> 

The driver requests the clocks by name, and unfortunately DTS
rely on that. We can change all the DTSI, but wouldn't that
be too much trouble for something that is currently working fine?

Why is the order important?

> > +    minItems: 2
> > +    maxItems: 2
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/clock/rk3288-cru.h>
> > +
> > +    timer: timer@ff810000 {
> > +        compatible = "rockchip,rk3288-timer";
> > +        reg = <0xff810000 0x20>;
> > +        interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
> > +        clocks = <&xin24m>, <&cru PCLK_TIMER>;
> > +        clock-names = "timer", "pclk";
> > +    };
> > -- 
> > 2.30.0
> > 



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] dt-bindings: timer: convert rockchip,rk-timer.txt to YAML
  2021-04-29 17:03       ` Ezequiel Garcia
@ 2021-04-29 20:28         ` Rob Herring
  -1 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2021-04-29 20:28 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: open list:ARM/Rockchip SoC...,
	devicetree, Heiko Stuebner, Kever Yang, Daniel Lezcano,
	Collabora Kernel ML

On Thu, Apr 29, 2021 at 12:03 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
>
> On Thu, 2021-04-29 at 11:25 -0500, Rob Herring wrote:
> > On Sat, Apr 24, 2021 at 04:19:46PM -0300, Ezequiel Garcia wrote:
> > > Convert Rockchip Timer dt-bindings to YAML.
> > >
> > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > > ---
> > >  .../bindings/timer/rockchip,rk-timer.txt      | 27 --------
> > >  .../bindings/timer/rockchip,rk-timer.yaml     | 67 +++++++++++++++++++
> > >  2 files changed, 67 insertions(+), 27 deletions(-)
> > >  delete mode 100644 Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
> > >  create mode 100644 Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
> > > deleted file mode 100644
> > > index d65fdce7c7f0..000000000000
> > > --- a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
> > > +++ /dev/null
> > > @@ -1,27 +0,0 @@
> > > -Rockchip rk timer
> > > -
> > > -Required properties:
> > > -- compatible: should be:
> > > -  "rockchip,rv1108-timer", "rockchip,rk3288-timer": for Rockchip RV1108
> > > -  "rockchip,rk3036-timer", "rockchip,rk3288-timer": for Rockchip RK3036
> > > -  "rockchip,rk3066-timer", "rockchip,rk3288-timer": for Rockchip RK3066
> > > -  "rockchip,rk3188-timer", "rockchip,rk3288-timer": for Rockchip RK3188
> > > -  "rockchip,rk3228-timer", "rockchip,rk3288-timer": for Rockchip RK3228
> > > -  "rockchip,rk3229-timer", "rockchip,rk3288-timer": for Rockchip RK3229
> > > -  "rockchip,rk3288-timer": for Rockchip RK3288
> > > -  "rockchip,rk3368-timer", "rockchip,rk3288-timer": for Rockchip RK3368
> > > -  "rockchip,rk3399-timer": for Rockchip RK3399
> > > -- reg: base address of the timer register starting with TIMERS CONTROL register
> > > -- interrupts: should contain the interrupts for Timer0
> > > -- clocks : must contain an entry for each entry in clock-names
> > > -- clock-names : must include the following entries:
> > > -  "timer", "pclk"
> > > -
> > > -Example:
> > > -       timer: timer@ff810000 {
> > > -               compatible = "rockchip,rk3288-timer";
> > > -               reg = <0xff810000 0x20>;
> > > -               interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
> > > -               clocks = <&xin24m>, <&cru PCLK_TIMER>;
> > > -               clock-names = "timer", "pclk";
> > > -       };
> > > diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
> > > new file mode 100644
> > > index 000000000000..f1bc3ac7abc8
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
> > > @@ -0,0 +1,67 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/timer/rockchip,rk-timer.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Rockchip Timer Device Tree Bindings
> > > +
> > > +maintainers:
> > > +  - Daniel Lezcano <daniel.lezcano@linaro.org>
> >
> > This should be someone that knows the h/w and cares about Rockchip.
> >
>
> Daniel wrote the driver, so I figured he'd care :)

Ah, then that's fine I guess. Given he is also the subsystem
maintainer I was confused.

> If not, perhaps Heiko (if he agrees)?
>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > > +      - const: rockchip,rk3288-timer
> > > +      - const: rockchip,rk3399-timer
> > > +      - items:
> > > +          - enum:
> > > +            - rockchip,rv1108-timer
> > > +            - rockchip,rk3036-timer
> > > +            - rockchip,rk3066-timer
> > > +            - rockchip,rk3188-timer
> > > +            - rockchip,rk3228-timer
> > > +            - rockchip,rk3229-timer
> > > +            - rockchip,rk3288-timer
> > > +            - rockchip,rk3368-timer
> > > +            - rockchip,px30-timer
> > > +          - const: rockchip,rk3288-timer
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +  clocks:
> > > +    minItems: 2
> > > +    maxItems: 2
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      enum:
> > > +        - timer
> > > +        - pclk
> >
> > We can't define the order here? We should fix dts files if they are
> > inconsistent.
> >
>
> The driver requests the clocks by name, and unfortunately DTS
> rely on that.

That's good, then the OS doesn't care if you change it. Of course, I
didn't have to look to tell that. If the order varied, the OS would
have to use the names.

> We can change all the DTSI, but wouldn't that
> be too much trouble for something that is currently working fine?

It's not *all*. At least half are correct. Pick the order that's the majority.

> Why is the order important?

Because that's the DT convention. Why is random order important?

I don't really care so much on an individual binding, but overall if
defining the order doesn't cost anything then we should. The more
order is not defined, the more people are going to copy those
examples. Yes, there's a cost on fixing dts files, but if we're
writing schema to pass on existing dts files, what's the point of
schema?

Rob

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] dt-bindings: timer: convert rockchip, rk-timer.txt to YAML
@ 2021-04-29 20:28         ` Rob Herring
  0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2021-04-29 20:28 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: open list:ARM/Rockchip SoC...,
	devicetree, Heiko Stuebner, Kever Yang, Daniel Lezcano,
	Collabora Kernel ML

On Thu, Apr 29, 2021 at 12:03 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
>
> On Thu, 2021-04-29 at 11:25 -0500, Rob Herring wrote:
> > On Sat, Apr 24, 2021 at 04:19:46PM -0300, Ezequiel Garcia wrote:
> > > Convert Rockchip Timer dt-bindings to YAML.
> > >
> > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > > ---
> > >  .../bindings/timer/rockchip,rk-timer.txt      | 27 --------
> > >  .../bindings/timer/rockchip,rk-timer.yaml     | 67 +++++++++++++++++++
> > >  2 files changed, 67 insertions(+), 27 deletions(-)
> > >  delete mode 100644 Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
> > >  create mode 100644 Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
> > > deleted file mode 100644
> > > index d65fdce7c7f0..000000000000
> > > --- a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
> > > +++ /dev/null
> > > @@ -1,27 +0,0 @@
> > > -Rockchip rk timer
> > > -
> > > -Required properties:
> > > -- compatible: should be:
> > > -  "rockchip,rv1108-timer", "rockchip,rk3288-timer": for Rockchip RV1108
> > > -  "rockchip,rk3036-timer", "rockchip,rk3288-timer": for Rockchip RK3036
> > > -  "rockchip,rk3066-timer", "rockchip,rk3288-timer": for Rockchip RK3066
> > > -  "rockchip,rk3188-timer", "rockchip,rk3288-timer": for Rockchip RK3188
> > > -  "rockchip,rk3228-timer", "rockchip,rk3288-timer": for Rockchip RK3228
> > > -  "rockchip,rk3229-timer", "rockchip,rk3288-timer": for Rockchip RK3229
> > > -  "rockchip,rk3288-timer": for Rockchip RK3288
> > > -  "rockchip,rk3368-timer", "rockchip,rk3288-timer": for Rockchip RK3368
> > > -  "rockchip,rk3399-timer": for Rockchip RK3399
> > > -- reg: base address of the timer register starting with TIMERS CONTROL register
> > > -- interrupts: should contain the interrupts for Timer0
> > > -- clocks : must contain an entry for each entry in clock-names
> > > -- clock-names : must include the following entries:
> > > -  "timer", "pclk"
> > > -
> > > -Example:
> > > -       timer: timer@ff810000 {
> > > -               compatible = "rockchip,rk3288-timer";
> > > -               reg = <0xff810000 0x20>;
> > > -               interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
> > > -               clocks = <&xin24m>, <&cru PCLK_TIMER>;
> > > -               clock-names = "timer", "pclk";
> > > -       };
> > > diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
> > > new file mode 100644
> > > index 000000000000..f1bc3ac7abc8
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.yaml
> > > @@ -0,0 +1,67 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/timer/rockchip,rk-timer.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Rockchip Timer Device Tree Bindings
> > > +
> > > +maintainers:
> > > +  - Daniel Lezcano <daniel.lezcano@linaro.org>
> >
> > This should be someone that knows the h/w and cares about Rockchip.
> >
>
> Daniel wrote the driver, so I figured he'd care :)

Ah, then that's fine I guess. Given he is also the subsystem
maintainer I was confused.

> If not, perhaps Heiko (if he agrees)?
>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > > +      - const: rockchip,rk3288-timer
> > > +      - const: rockchip,rk3399-timer
> > > +      - items:
> > > +          - enum:
> > > +            - rockchip,rv1108-timer
> > > +            - rockchip,rk3036-timer
> > > +            - rockchip,rk3066-timer
> > > +            - rockchip,rk3188-timer
> > > +            - rockchip,rk3228-timer
> > > +            - rockchip,rk3229-timer
> > > +            - rockchip,rk3288-timer
> > > +            - rockchip,rk3368-timer
> > > +            - rockchip,px30-timer
> > > +          - const: rockchip,rk3288-timer
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +  clocks:
> > > +    minItems: 2
> > > +    maxItems: 2
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      enum:
> > > +        - timer
> > > +        - pclk
> >
> > We can't define the order here? We should fix dts files if they are
> > inconsistent.
> >
>
> The driver requests the clocks by name, and unfortunately DTS
> rely on that.

That's good, then the OS doesn't care if you change it. Of course, I
didn't have to look to tell that. If the order varied, the OS would
have to use the names.

> We can change all the DTSI, but wouldn't that
> be too much trouble for something that is currently working fine?

It's not *all*. At least half are correct. Pick the order that's the majority.

> Why is the order important?

Because that's the DT convention. Why is random order important?

I don't really care so much on an individual binding, but overall if
defining the order doesn't cost anything then we should. The more
order is not defined, the more people are going to copy those
examples. Yes, there's a cost on fixing dts files, but if we're
writing schema to pass on existing dts files, what's the point of
schema?

Rob

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-04-29 20:28 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-24 19:19 [PATCH 1/2] arm64: dts: rockchip: add timer0 clocks on rk3368 Ezequiel Garcia
2021-04-24 19:19 ` Ezequiel Garcia
2021-04-24 19:19 ` [PATCH 2/2] dt-bindings: timer: convert rockchip,rk-timer.txt to YAML Ezequiel Garcia
2021-04-24 19:19   ` [PATCH 2/2] dt-bindings: timer: convert rockchip, rk-timer.txt " Ezequiel Garcia
2021-04-26 21:04   ` [PATCH 2/2] dt-bindings: timer: convert rockchip,rk-timer.txt " Rob Herring
2021-04-26 21:04     ` [PATCH 2/2] dt-bindings: timer: convert rockchip, rk-timer.txt " Rob Herring
2021-04-29 16:25   ` [PATCH 2/2] dt-bindings: timer: convert rockchip,rk-timer.txt " Rob Herring
2021-04-29 16:25     ` [PATCH 2/2] dt-bindings: timer: convert rockchip, rk-timer.txt " Rob Herring
2021-04-29 17:03     ` [PATCH 2/2] dt-bindings: timer: convert rockchip,rk-timer.txt " Ezequiel Garcia
2021-04-29 17:03       ` Ezequiel Garcia
2021-04-29 20:28       ` Rob Herring
2021-04-29 20:28         ` [PATCH 2/2] dt-bindings: timer: convert rockchip, rk-timer.txt " Rob Herring

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.