[PATCH v2 0/6] dt-bindings: Convert SP804 to Json-schema (and fix users)

[PATCH v2 0/6] dt-bindings: Convert SP804 to Json-schema (and fix users)

[Andre Przywara]

This is the second attempt at converting the SP804 timer binding to yaml.
Compared to v1, I forbid additional properties, and included the primecell
binding. Also the clock-names property is now listed, although without
further requirements on the names. Changelog below.

--------------
The yaml conversion is done in the first patch, the remaining five fix
some DT users.

I couldn't test any of those DT files on actual machines, but tried
to make the changes in a way that would be transparent to at least the
Linux driver. The only other SP804 DT user I could find is FreeBSD,
but they seem to use a different binding (no clocks, but a
clock-frequency property).

Cheers,
Andre

Changelog v1 .. v2:
- Add additional-properties: false
- Allow clock-names property
- Include primecell binding
- Fix subject on Broadcom patch
- Add Florian's Tested-by: on Broadcom patch
- Add Linus' Acked-by: on Arm patch

Andre Przywara (6):
  dt-bindings: timers: sp-804: Convert to json-schema
  ARM: dts: arm: Fix SP804 users
  ARM: dts: NSP: Fix SP804 compatible node
  ARM: dts: hisilicon: Fix SP804 users
  ARM: dts: nspire: Fix SP804 users
  arm64: dts: lg: Fix SP804 users

 .../devicetree/bindings/timer/arm,sp804.txt   | 29 ------
 .../devicetree/bindings/timer/arm,sp804.yaml  | 93 +++++++++++++++++++
 arch/arm/boot/dts/arm-realview-pb11mp.dts     | 16 ++--
 arch/arm/boot/dts/bcm-nsp.dtsi                |  2 +-
 arch/arm/boot/dts/hi3620.dtsi                 | 30 ++++--
 arch/arm/boot/dts/hip04.dtsi                  |  4 +-
 arch/arm/boot/dts/mps2.dtsi                   |  6 +-
 arch/arm/boot/dts/nspire.dtsi                 | 12 ++-
 arch/arm/boot/dts/vexpress-v2p-ca9.dts        |  4 +-
 arch/arm64/boot/dts/lg/lg1312.dtsi            |  6 +-
 arch/arm64/boot/dts/lg/lg1313.dtsi            |  6 +-
 11 files changed, 144 insertions(+), 64 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/timer/arm,sp804.txt
 create mode 100644 Documentation/devicetree/bindings/timer/arm,sp804.yaml

-- 
2.17.1

[PATCH v2 1/6] dt-bindings: timers: sp-804: Convert to json-schema

[Andre Przywara]

This converts the DT binding documentation for the ARM SP-804 timer IP
over to json-schema.
Most properties are just carried over, the clocks property requirement
(either one or three clocks) is now formalised and enforced.
As the former binding didn't specify clock-names, and there is no
common name used by the existing DTs, I refrained from adding them in
detail (just allowing the property).
The requirement for the APB clock is enforced by the primecell binding
already.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 .../devicetree/bindings/timer/arm,sp804.txt   | 29 ------
 .../devicetree/bindings/timer/arm,sp804.yaml  | 93 +++++++++++++++++++
 2 files changed, 93 insertions(+), 29 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/timer/arm,sp804.txt
 create mode 100644 Documentation/devicetree/bindings/timer/arm,sp804.yaml

diff --git a/Documentation/devicetree/bindings/timer/arm,sp804.txt b/Documentation/devicetree/bindings/timer/arm,sp804.txt
deleted file mode 100644
index 5cd8eee74af1..000000000000
--- a/Documentation/devicetree/bindings/timer/arm,sp804.txt
+++ /dev/null
@@ -1,29 +0,0 @@
-ARM sp804 Dual Timers
----------------------------------------
-
-Required properties:
-- compatible: Should be "arm,sp804" & "arm,primecell"
-- interrupts: Should contain the list of Dual Timer interrupts. This is the
-	interrupt for timer 1 and timer 2. In the case of a single entry, it is
-	the combined interrupt or if "arm,sp804-has-irq" is present that
-	specifies which timer interrupt is connected.
-- reg: Should contain location and length for dual timer register.
-- clocks: clocks driving the dual timer hardware. This list should be 1 or 3
-	clocks.	With 3 clocks, the order is timer0 clock, timer1 clock,
-	apb_pclk. A single clock can also be specified if the same clock is
-	used for all clock inputs.
-
-Optional properties:
-- arm,sp804-has-irq = <#>: In the case of only 1 timer irq line connected, this
-	specifies if the irq connection is for timer 1 or timer 2. A value of 1
-	or 2 should be used.
-
-Example:
-
-	timer0: timer@fc800000 {
-		compatible = "arm,sp804", "arm,primecell";
-		reg = <0xfc800000 0x1000>;
-		interrupts = <0 0 4>, <0 1 4>;
-		clocks = <&timclk1 &timclk2 &pclk>;
-		clock-names = "timer1", "timer2", "apb_pclk";
-	};
diff --git a/Documentation/devicetree/bindings/timer/arm,sp804.yaml b/Documentation/devicetree/bindings/timer/arm,sp804.yaml
new file mode 100644
index 000000000000..609972379637
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/arm,sp804.yaml
@@ -0,0 +1,93 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/timer/arm,sp804.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ARM sp804 Dual Timers
+
+maintainers:
+  - Haojian Zhuang <haojian.zhuang@linaro.org>
+
+description: |+
+  The Arm SP804 IP implements two independent timers, configurable for
+  16 or 32 bit operation and capable of running in one-shot, periodic, or
+  free-running mode. The input clock is shared, but can be gated and prescaled
+  independently for each timer.
+
+allOf:
+  - $ref: /schemas/arm/primecell.yaml#
+
+# Need a custom select here or 'arm,primecell' will match on lots of nodes
+select:
+  properties:
+    compatible:
+      contains:
+        const: arm,sp804
+  required:
+    - compatible
+
+properties:
+  compatible:
+    items:
+      - const: arm,sp804
+      - const: arm,primecell
+
+  interrupts:
+    description: |
+      If two interrupts are listed, those are the interrupts for timer
+      1 and 2, respectively. If there is only a single interrupt, it is
+      either a combined interrupt or the sole interrupt of one timer, as
+      specified by the "arm,sp804-has-irq" property.
+    minItems: 1
+    maxItems: 2
+
+  reg:
+    description: The physical base address of the SP804 IP.
+    maxItems: 1
+
+  clocks:
+    description: |
+      Clocks driving the dual timer hardware. This list should
+      be 1 or 3 clocks. With 3 clocks, the order is timer0 clock, timer1
+      clock, apb_pclk. A single clock can also be specified if the same
+      clock is used for all clock inputs.
+    oneOf:
+      - items:
+        - description: clock for timer 1
+        - description: clock for timer 2
+        - description: bus clock
+      - items:
+        - description: unified clock for both timers and the bus
+
+  clock-names: true
+    # The original binding did not specify any clock names, and there is no
+    # consistent naming used in the existing DTs. The primecell binding
+    # requires the "apb_pclk" name, so we need this property.
+    # Use "timer0clk", "timer1clk", "apb_pclk" for new DTs.
+
+  arm,sp804-has-irq:
+    description: If only one interrupt line is connected to the interrupt
+      controller, this property specifies which timer is connected to this
+      line.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+    maximum: 2
+
+required:
+  - compatible
+  - interrupts
+  - reg
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    timer0: timer@fc800000 {
+        compatible = "arm,sp804", "arm,primecell";
+        reg = <0xfc800000 0x1000>;
+        interrupts = <0 0 4>, <0 1 4>;
+        clocks = <&timclk1>, <&timclk2>, <&pclk>;
+        clock-names = "timer1", "timer2", "apb_pclk";
+    };
-- 
2.17.1

[PATCH v2 2/6] ARM: dts: arm: Fix SP804 users

[Andre Przywara]

The SP804 DT nodes for Realview, MPS2 and VExpress were not complying
with the binding: it requires either one or three clocks, but does not
allow exactly two clocks.

Simply duplicate the first clock to satisfy the binding requirement.
For MPS2, we triple the clock, and add the clock-names property, as this
is required by the Linux primecell driver.
Try to make the clock-names more consistent on the way.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/boot/dts/arm-realview-pb11mp.dts | 16 ++++++++--------
 arch/arm/boot/dts/mps2.dtsi               |  6 ++++--
 arch/arm/boot/dts/vexpress-v2p-ca9.dts    |  4 ++--
 3 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/arm-realview-pb11mp.dts b/arch/arm/boot/dts/arm-realview-pb11mp.dts
index 9748e0fe800f..564e2eee2c24 100644
--- a/arch/arm/boot/dts/arm-realview-pb11mp.dts
+++ b/arch/arm/boot/dts/arm-realview-pb11mp.dts
@@ -568,8 +568,8 @@
 			clocks = <&sp810_syscon 0>,
 			         <&sp810_syscon 1>,
 				 <&pclk>;
-			clock-names = "timerclk0",
-				    "timerclk1",
+			clock-names = "timer0clk",
+				    "timer1clk",
 				    "apb_pclk";
 		};
 
@@ -582,8 +582,8 @@
 			clocks = <&sp810_syscon 2>,
 			         <&sp810_syscon 3>,
 				 <&pclk>;
-			clock-names = "timerclk2",
-				    "timerclk3",
+			clock-names = "timer0clk",
+				    "timer1clk",
 				    "apb_pclk";
 		};
 
@@ -645,16 +645,16 @@
 		timer45: timer@10018000 {
 			compatible = "arm,sp804", "arm,primecell";
 			reg = <0x10018000 0x1000>;
-			clocks = <&timclk>, <&pclk>;
-			clock-names = "timer", "apb_pclk";
+			clocks = <&timclk>, <&timclk>, <&pclk>;
+			clock-names = "timer0clk", "timer1clk", "apb_pclk";
 			status = "disabled";
 		};
 
 		timer67: timer@10019000 {
 			compatible = "arm,sp804", "arm,primecell";
 			reg = <0x10019000 0x1000>;
-			clocks = <&timclk>, <&pclk>;
-			clock-names = "timer", "apb_pclk";
+			clocks = <&timclk>, <&timclk>, <&pclk>;
+			clock-names = "timer0clk", "timer1clk", "apb_pclk";
 			status = "disabled";
 		};
 
diff --git a/arch/arm/boot/dts/mps2.dtsi b/arch/arm/boot/dts/mps2.dtsi
index 96fb5a5cf4d3..48c34fa282af 100644
--- a/arch/arm/boot/dts/mps2.dtsi
+++ b/arch/arm/boot/dts/mps2.dtsi
@@ -161,9 +161,11 @@
 			};
 
 			timer2: dual-timer@2000 {
-				compatible = "arm,sp804";
+				compatible = "arm,sp804", "arm,primecell";
 				reg = <0x2000 0x1000>;
-				clocks = <&sysclk>;
+				clocks = <&sysclk>, <&sysclk>, <&sysclk>;
+				clock-names = "timer0clk", "timer1clk",
+					       "apb_pclk";
 				interrupts = <10>;
 				status = "disabled";
 			};
diff --git a/arch/arm/boot/dts/vexpress-v2p-ca9.dts b/arch/arm/boot/dts/vexpress-v2p-ca9.dts
index 623246f37448..6cddea25a292 100644
--- a/arch/arm/boot/dts/vexpress-v2p-ca9.dts
+++ b/arch/arm/boot/dts/vexpress-v2p-ca9.dts
@@ -122,8 +122,8 @@
 		reg = <0x100e4000 0x1000>;
 		interrupts = <0 48 4>,
 			     <0 49 4>;
-		clocks = <&oscclk2>, <&oscclk2>;
-		clock-names = "timclk", "apb_pclk";
+		clocks = <&oscclk2>, <&oscclk2>, <&oscclk2>;
+		clock-names = "timer0clk", "timer1clk", "apb_pclk";
 		status = "disabled";
 	};
 
-- 
2.17.1

[PATCH v2 3/6] ARM: dts: NSP: Fix SP804 compatible node

[Andre Przywara]

The DT binding for SP804 requires to have an "arm,primecell" compatible
string.
Add this string so that the Linux primecell bus driver picks the device
up and activates the clock.

Fixes: a0efb0d28b77 ("ARM: dts: NSP: Add SP804 Support to DT")
Tested-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/boot/dts/bcm-nsp.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
index 0346ea621f0f..1333ef8be0a2 100644
--- a/arch/arm/boot/dts/bcm-nsp.dtsi
+++ b/arch/arm/boot/dts/bcm-nsp.dtsi
@@ -368,7 +368,7 @@
 		};
 
 		ccbtimer0: timer@34000 {
-			compatible = "arm,sp804";
+			compatible = "arm,sp804", "arm,primecell";
 			reg = <0x34000 0x1000>;
 			interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
-- 
2.17.1

[PATCH v2 4/6] ARM: dts: hisilicon: Fix SP804 users

[Andre Przywara]

The SP804 binding only specifies one or three clocks, but does not allow
just two clocks.
The HiSi 3620 .dtsi specified two clocks for the two timers, plus gave
one "apb_pclk" clock-name to appease the primecell bus driver.

Extend the clocks by duplicating the first clock to the end of the clock
list, and add two dummy clock-names to make the primecell driver happy.

I don't know what the real APB clock for the IP is, but with the current
DT the first timer clock was used for that, so this change keeps the
current status.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/boot/dts/hi3620.dtsi | 30 ++++++++++++++++++++----------
 arch/arm/boot/dts/hip04.dtsi  |  4 ++--
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/hi3620.dtsi b/arch/arm/boot/dts/hi3620.dtsi
index f0af1bf2b4d8..355175b25fd6 100644
--- a/arch/arm/boot/dts/hi3620.dtsi
+++ b/arch/arm/boot/dts/hi3620.dtsi
@@ -111,8 +111,10 @@
 			reg = <0x800000 0x1000>;
 			/* timer00 & timer01 */
 			interrupts = <0 0 4>, <0 1 4>;
-			clocks = <&clock HI3620_TIMER0_MUX>, <&clock HI3620_TIMER1_MUX>;
-			clock-names = "apb_pclk";
+			clocks = <&clock HI3620_TIMER0_MUX>,
+				 <&clock HI3620_TIMER1_MUX>,
+				 <&clock HI3620_TIMER0_MUX>;
+			clock-names = "timer0clk", "timer1clk", "apb_pclk";
 			status = "disabled";
 		};
 
@@ -121,8 +123,10 @@
 			reg = <0x801000 0x1000>;
 			/* timer10 & timer11 */
 			interrupts = <0 2 4>, <0 3 4>;
-			clocks = <&clock HI3620_TIMER2_MUX>, <&clock HI3620_TIMER3_MUX>;
-			clock-names = "apb_pclk";
+			clocks = <&clock HI3620_TIMER2_MUX>,
+				 <&clock HI3620_TIMER3_MUX>,
+				 <&clock HI3620_TIMER2_MUX>;
+			clock-names = "timer0clk", "timer1clk", "apb_pclk";
 			status = "disabled";
 		};
 
@@ -131,8 +135,10 @@
 			reg = <0xa01000 0x1000>;
 			/* timer20 & timer21 */
 			interrupts = <0 4 4>, <0 5 4>;
-			clocks = <&clock HI3620_TIMER4_MUX>, <&clock HI3620_TIMER5_MUX>;
-			clock-names = "apb_pclk";
+			clocks = <&clock HI3620_TIMER4_MUX>,
+				 <&clock HI3620_TIMER5_MUX>,
+				 <&clock HI3620_TIMER4_MUX>;
+			clock-names = "timer0lck", "timer1clk", "apb_pclk";
 			status = "disabled";
 		};
 
@@ -141,8 +147,10 @@
 			reg = <0xa02000 0x1000>;
 			/* timer30 & timer31 */
 			interrupts = <0 6 4>, <0 7 4>;
-			clocks = <&clock HI3620_TIMER6_MUX>, <&clock HI3620_TIMER7_MUX>;
-			clock-names = "apb_pclk";
+			clocks = <&clock HI3620_TIMER6_MUX>,
+				 <&clock HI3620_TIMER7_MUX>,
+				 <&clock HI3620_TIMER6_MUX>;
+			clock-names = "timer0clk", "timer1clk", "apb_pclk";
 			status = "disabled";
 		};
 
@@ -151,8 +159,10 @@
 			reg = <0xa03000 0x1000>;
 			/* timer40 & timer41 */
 			interrupts = <0 96 4>, <0 97 4>;
-			clocks = <&clock HI3620_TIMER8_MUX>, <&clock HI3620_TIMER9_MUX>;
-			clock-names = "apb_pclk";
+			clocks = <&clock HI3620_TIMER8_MUX>,
+				 <&clock HI3620_TIMER9_MUX>,
+				 <&clock HI3620_TIMER8_MUX>;
+			clock-names = "timer0clk", "timer1clk", "apb_pclk";
 			status = "disabled";
 		};
 
diff --git a/arch/arm/boot/dts/hip04.dtsi b/arch/arm/boot/dts/hip04.dtsi
index 4263a9339c2e..f5871b1d1ec4 100644
--- a/arch/arm/boot/dts/hip04.dtsi
+++ b/arch/arm/boot/dts/hip04.dtsi
@@ -226,8 +226,8 @@
 			compatible = "arm,sp804", "arm,primecell";
 			reg = <0x3000000 0x1000>;
 			interrupts = <0 224 4>;
-			clocks = <&clk_50m>, <&clk_50m>;
-			clock-names = "apb_pclk";
+			clocks = <&clk_50m>, <&clk_50m>, <&clk_50m>;
+			clock-names = "timer0clk", "timer1clk", "apb_pclk";
 		};
 
 		arm-pmu {
-- 
2.17.1

[PATCH v2 5/6] ARM: dts: nspire: Fix SP804 users

[Andre Przywara]

Even though the SP804 binding allows to specify only one clock, the
primecell driver requires a named clock to activate the bus clock.

Specify the one clock three times and provide some clock-names, to
make the DT match the SP804 and primecell binding.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/boot/dts/nspire.dtsi | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/nspire.dtsi b/arch/arm/boot/dts/nspire.dtsi
index d9a0fd7524dc..90e033d9141f 100644
--- a/arch/arm/boot/dts/nspire.dtsi
+++ b/arch/arm/boot/dts/nspire.dtsi
@@ -145,15 +145,19 @@
 
 			timer0: timer@900C0000 {
 				reg = <0x900C0000 0x1000>;
-
-				clocks = <&timer_clk>;
+				clocks = <&timer_clk>, <&timer_clk>,
+					 <&timer_clk>;
+				clock-names = "timer0clk", "timer1clk",
+					      "apb_pclk";
 			};
 
 			timer1: timer@900D0000 {
 				reg = <0x900D0000 0x1000>;
 				interrupts = <19>;
-
-				clocks = <&timer_clk>;
+				clocks = <&timer_clk>, <&timer_clk>,
+					 <&timer_clk>;
+				clock-names = "timer0clk", "timer1clk",
+					      "apb_pclk";
 			};
 
 			watchdog: watchdog@90060000 {
-- 
2.17.1

[PATCH v2 6/6] arm64: dts: lg: Fix SP804 users

[Andre Przywara]

Even though the SP804 binding allows to specify only one clock, the
primecell driver requires a named clock to activate the bus clock.

Specify the one clock three times and provide some clock-names, to
make the DT match the SP804 and primecell binding.
Also add the missing arm,primecell compatible string.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/boot/dts/lg/lg1312.dtsi | 6 +++---
 arch/arm64/boot/dts/lg/lg1313.dtsi | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/lg/lg1312.dtsi b/arch/arm64/boot/dts/lg/lg1312.dtsi
index 64f3b135068d..e2a1564597c8 100644
--- a/arch/arm64/boot/dts/lg/lg1312.dtsi
+++ b/arch/arm64/boot/dts/lg/lg1312.dtsi
@@ -131,11 +131,11 @@
 		ranges;
 
 		timers: timer@fd100000 {
-			compatible = "arm,sp804";
+			compatible = "arm,sp804", "arm,primecell";
 			reg = <0x0 0xfd100000 0x1000>;
 			interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&clk_bus>;
-			clock-names = "apb_pclk";
+			clocks = <&clk_bus>, <&clk_bus>, <&clk_bus>;
+			clock-names = "timer0clk", "timer1clk", "apb_pclk";
 		};
 		wdog: watchdog@fd200000 {
 			compatible = "arm,sp805", "arm,primecell";
diff --git a/arch/arm64/boot/dts/lg/lg1313.dtsi b/arch/arm64/boot/dts/lg/lg1313.dtsi
index ac23592ab011..a54d14d7ae6f 100644
--- a/arch/arm64/boot/dts/lg/lg1313.dtsi
+++ b/arch/arm64/boot/dts/lg/lg1313.dtsi
@@ -131,11 +131,11 @@
 		ranges;
 
 		timers: timer@fd100000 {
-			compatible = "arm,sp804";
+			compatible = "arm,sp804", "arm,primecell";
 			reg = <0x0 0xfd100000 0x1000>;
 			interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&clk_bus>;
-			clock-names = "apb_pclk";
+			clocks = <&clk_bus>, <&clk_bus>, <&clk_bus>;
+			clock-names = "timer0clk", "timer1clk", "apb_pclk";
 		};
 		wdog: watchdog@fd200000 {
 			compatible = "arm,sp805", "arm,primecell";
-- 
2.17.1

Re: [PATCH v2 0/6] dt-bindings: Convert SP804 to Json-schema (and fix users)

[Linus Walleij]

On Fri, Aug 28, 2020 at 4:20 PM Andre Przywara <andre.przywara@arm.com> wrote:

> This is the second attempt at converting the SP804 timer binding to yaml.
> Compared to v1, I forbid additional properties, and included the primecell
> binding. Also the clock-names property is now listed, although without
> further requirements on the names. Changelog below.

The series:
Acked-by: Linus Walleij <linus.walleij@linaro.org>

> I couldn't test any of those DT files on actual machines, but tried
> to make the changes in a way that would be transparent to at least the
> Linux driver. The only other SP804 DT user I could find is FreeBSD,
> but they seem to use a different binding (no clocks, but a
> clock-frequency property).

That's annoying. I suppose FreeBSD just made that up and doesn't
even have a binding document for it?

In an ideal world I suppose we should go and fix FreeBSD but I have
no idea how easy or hard that is.

Yours,
Linus Walleij

Re: [PATCH v2 0/6] dt-bindings: Convert SP804 to Json-schema (and fix users)

[André Przywara]

On 28/08/2020 15:54, Linus Walleij wrote:

Hi,

> On Fri, Aug 28, 2020 at 4:20 PM Andre Przywara <andre.przywara@arm.com> wrote:
> 
>> This is the second attempt at converting the SP804 timer binding to yaml.
>> Compared to v1, I forbid additional properties, and included the primecell
>> binding. Also the clock-names property is now listed, although without
>> further requirements on the names. Changelog below.
> 
> The series:
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
>> I couldn't test any of those DT files on actual machines, but tried
>> to make the changes in a way that would be transparent to at least the
>> Linux driver. The only other SP804 DT user I could find is FreeBSD,
>> but they seem to use a different binding (no clocks, but a
>> clock-frequency property).
> 
> That's annoying. I suppose FreeBSD just made that up and doesn't
> even have a binding document for it?

I couldn't find bindings at all in their git tree. I don't think they
treat this very formally, it seems to be more use-case driven.
Their SP804 driver does not know how to handle clock properties, so most
of the DTs (in sys/gnu/dts, so apparently copied from Linux) would not
work really well, because the driver assumes a hardcoded frequency of
1MHz by default.
There is only one DT (Annapurna Alpine with Cortex-A15) that provides
this clock-frequency property. The Linux DT does not mention the SP804
in there at all, interestingly.

> In an ideal world I suppose we should go and fix FreeBSD but I have
> no idea how easy or hard that is.

It seems to be messy, at least in this case, and I guess unifying DTs
means some work on drivers as well.
But AFAIK most of the more modern platforms copy the DTs (and thus
implicitly the bindings) from Linux, so there is probably much less
deviation for many more relevant boards.

Cheers,
Andre

Re: [PATCH v2 3/6] ARM: dts: NSP: Fix SP804 compatible node

[Florian Fainelli]

On 8/28/20 7:20 AM, Andre Przywara wrote:
> The DT binding for SP804 requires to have an "arm,primecell" compatible
> string.
> Add this string so that the Linux primecell bus driver picks the device
> up and activates the clock.
> 
> Fixes: a0efb0d28b77 ("ARM: dts: NSP: Add SP804 Support to DT")
> Tested-by: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

This looks fine, however there is a ccbtimer1 instance that you missed,
can you resubmit with it included?

With that:

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH v2 0/6] dt-bindings: Convert SP804 to Json-schema (and fix users)

[Emmanuel Vadot]

On Fri, 28 Aug 2020 16:44:28 +0100
André Przywara <andre.przywara@arm.com> wrote:

> On 28/08/2020 15:54, Linus Walleij wrote:
> 
> Hi,
> 
> > On Fri, Aug 28, 2020 at 4:20 PM Andre Przywara <andre.przywara@arm.com> wrote:
> > 
> >> This is the second attempt at converting the SP804 timer binding to yaml.
> >> Compared to v1, I forbid additional properties, and included the primecell
> >> binding. Also the clock-names property is now listed, although without
> >> further requirements on the names. Changelog below.
> > 
> > The series:
> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > 
> >> I couldn't test any of those DT files on actual machines, but tried
> >> to make the changes in a way that would be transparent to at least the
> >> Linux driver. The only other SP804 DT user I could find is FreeBSD,
> >> but they seem to use a different binding (no clocks, but a
> >> clock-frequency property).
> > 
> > That's annoying. I suppose FreeBSD just made that up and doesn't
> > even have a binding document for it?
> 
> I couldn't find bindings at all in their git tree.

 That's because I don't merge the bindings in the main branch.

> I don't think they
> treat this very formally, it seems to be more use-case driven.
> Their SP804 driver does not know how to handle clock properties, so most
> of the DTs (in sys/gnu/dts, so apparently copied from Linux) would not
> work really well, because the driver assumes a hardcoded frequency of
> 1MHz by default.

 In addition to sys/gnu/dts we also have sys/dts/ which are our own DTs
before we used the Linux ones (a long time ago but some platform
weren't converted, they will just die sometime in the futur if nobody
takes care of them I guess).

> There is only one DT (Annapurna Alpine with Cortex-A15) that provides
> this clock-frequency property. The Linux DT does not mention the SP804
> in there at all, interestingly.

 I'm not familiar with this platform at all, it was done under
contract by Semihalf and I'm sure that if something fails and their
client starts to complain they will fix it.

> > In an ideal world I suppose we should go and fix FreeBSD but I have
> > no idea how easy or hard that is.
> 
> It seems to be messy, at least in this case, and I guess unifying DTs
> means some work on drivers as well.

 I wouldn't worry about us on this case, this binding requirements
seems to have be done a long time ago before we had any clock framework
and if our drivers needs to be updated we will do it when we imports
DTS from whatever Linux version this will be merged in.

> But AFAIK most of the more modern platforms copy the DTs (and thus
> implicitly the bindings) from Linux, so there is probably much less
> deviation for many more relevant boards.

 Yes, I (and others) insist on using the DTs from Linux and not doing
any patches to it without sending them to the Linux ML.

> Cheers,
> Andre

 Cheers,

> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


-- 
Emmanuel Vadot <manu@bidouilliste.com>

Re: [PATCH v2 3/6] ARM: dts: NSP: Fix SP804 compatible node

[Florian Fainelli]

On 8/28/2020 10:12 AM, Florian Fainelli wrote:
> On 8/28/20 7:20 AM, Andre Przywara wrote:
>> The DT binding for SP804 requires to have an "arm,primecell" compatible
>> string.
>> Add this string so that the Linux primecell bus driver picks the device
>> up and activates the clock.
>>
>> Fixes: a0efb0d28b77 ("ARM: dts: NSP: Add SP804 Support to DT")
>> Tested-by: Florian Fainelli <f.fainelli@gmail.com>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> 
> This looks fine, however there is a ccbtimer1 instance that you missed,
> can you resubmit with it included?
> 
> With that:
> 
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>

Andre are you going to resubmit a patch with the second instance 
(ccbtimer1) fixed as well, or should I take care of that while applying 
the patch? Either way is fine, just let me know.
-- 
Florian

Re: [PATCH v2 3/6] ARM: dts: NSP: Fix SP804 compatible node

[André Przywara]

On 02/09/2020 00:04, Florian Fainelli wrote:

Hi Florian,

sorry, the mail got swamped in my inbox...

> On 8/28/2020 10:12 AM, Florian Fainelli wrote:
>> On 8/28/20 7:20 AM, Andre Przywara wrote:
>>> The DT binding for SP804 requires to have an "arm,primecell" compatible
>>> string.
>>> Add this string so that the Linux primecell bus driver picks the device
>>> up and activates the clock.
>>>
>>> Fixes: a0efb0d28b77 ("ARM: dts: NSP: Add SP804 Support to DT")
>>> Tested-by: Florian Fainelli <f.fainelli@gmail.com>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>
>> This looks fine, however there is a ccbtimer1 instance that you missed,
>> can you resubmit with it included?
>>
>> With that:
>>
>> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> Andre are you going to resubmit a patch with the second instance
> (ccbtimer1) fixed as well, or should I take care of that while applying
> the patch? Either way is fine, just let me know.

So I was waiting for more comments, but there was nothing so far that
justifies a new version. So would you mind fixing this while applying? I
must have indeed missed this instance while diffing before and after.

Many thanks!
Andre.

Re: [PATCH v2 3/6] ARM: dts: NSP: Fix SP804 compatible node

[Florian Fainelli]

On 9/3/2020 6:04 PM, André Przywara wrote:
> On 02/09/2020 00:04, Florian Fainelli wrote:
> 
> Hi Florian,
> 
> sorry, the mail got swamped in my inbox...
> 
>> On 8/28/2020 10:12 AM, Florian Fainelli wrote:
>>> On 8/28/20 7:20 AM, Andre Przywara wrote:
>>>> The DT binding for SP804 requires to have an "arm,primecell" compatible
>>>> string.
>>>> Add this string so that the Linux primecell bus driver picks the device
>>>> up and activates the clock.
>>>>
>>>> Fixes: a0efb0d28b77 ("ARM: dts: NSP: Add SP804 Support to DT")
>>>> Tested-by: Florian Fainelli <f.fainelli@gmail.com>
>>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>>
>>> This looks fine, however there is a ccbtimer1 instance that you missed,
>>> can you resubmit with it included?
>>>
>>> With that:
>>>
>>> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
>>
>> Andre are you going to resubmit a patch with the second instance
>> (ccbtimer1) fixed as well, or should I take care of that while applying
>> the patch? Either way is fine, just let me know.
> 
> So I was waiting for more comments, but there was nothing so far that
> justifies a new version. So would you mind fixing this while applying? I
> must have indeed missed this instance while diffing before and after.

Applied and fixed up the ccbtimer1 node, thanks.
-- 
Florian

Re: [PATCH v2 0/6] dt-bindings: Convert SP804 to Json-schema (and fix users)

[Sudeep Holla]

On Fri, 28 Aug 2020 15:20:12 +0100, Andre Przywara wrote:
> This is the second attempt at converting the SP804 timer binding to yaml.
> Compared to v1, I forbid additional properties, and included the primecell
> binding. Also the clock-names property is now listed, although without
> further requirements on the names. Changelog below.
> 
> --------------
> The yaml conversion is done in the first patch, the remaining five fix
> some DT users.
> 
> [...]

I have picked one patch for Arm Ltd boards/models.

Applied to sudeep.holla/linux (for-next/juno), thanks!

[1/1] ARM: dts: arm: Fix SP804 users
      https://git.kernel.org/sudeep.holla/c/34a4591871

--

Regards,
Sudeep

Re: [PATCH v2 1/6] dt-bindings: timers: sp-804: Convert to json-schema

[Rob Herring]

On Fri, 28 Aug 2020 15:20:13 +0100, Andre Przywara wrote:
> This converts the DT binding documentation for the ARM SP-804 timer IP
> over to json-schema.
> Most properties are just carried over, the clocks property requirement
> (either one or three clocks) is now formalised and enforced.
> As the former binding didn't specify clock-names, and there is no
> common name used by the existing DTs, I refrained from adding them in
> detail (just allowing the property).
> The requirement for the APB clock is enforced by the primecell binding
> already.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  .../devicetree/bindings/timer/arm,sp804.txt   | 29 ------
>  .../devicetree/bindings/timer/arm,sp804.yaml  | 93 +++++++++++++++++++
>  2 files changed, 93 insertions(+), 29 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/timer/arm,sp804.txt
>  create mode 100644 Documentation/devicetree/bindings/timer/arm,sp804.yaml
> 

Applied, thanks!

I dropped the primecell.yaml ref as it is redundant.

Rob

Re: [PATCH v2 1/6] dt-bindings: timers: sp-804: Convert to json-schema

[André Przywara]

On 08/09/2020 18:28, Rob Herring wrote:
> On Fri, 28 Aug 2020 15:20:13 +0100, Andre Przywara wrote:
>> This converts the DT binding documentation for the ARM SP-804 timer IP
>> over to json-schema.
>> Most properties are just carried over, the clocks property requirement
>> (either one or three clocks) is now formalised and enforced.
>> As the former binding didn't specify clock-names, and there is no
>> common name used by the existing DTs, I refrained from adding them in
>> detail (just allowing the property).
>> The requirement for the APB clock is enforced by the primecell binding
>> already.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>  .../devicetree/bindings/timer/arm,sp804.txt   | 29 ------
>>  .../devicetree/bindings/timer/arm,sp804.yaml  | 93 +++++++++++++++++++
>>  2 files changed, 93 insertions(+), 29 deletions(-)
>>  delete mode 100644 Documentation/devicetree/bindings/timer/arm,sp804.txt
>>  create mode 100644 Documentation/devicetree/bindings/timer/arm,sp804.yaml
>>
> 
> Applied, thanks!
> 
> I dropped the primecell.yaml ref as it is redundant.

Interesting, because I explicitly added it to cover one property that
was only described in primecell.yaml. But I think this one node was
originally missing the actual primecell compatible string.

So I tested it now again and don't see any issues without the explicit
primecell.yaml reference anymore.

Thanks for taking it!

Cheers,
Andre.