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

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

[Andre Przywara]

This is the first attempt to convert the SP804 timer binding to yaml.
This 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).
For some platforms I wonder how this worked before (missing arm,primecell
compatible), maybe they magically spring to life now?

Cheers,
Andre

Andre Przywara (6):
  dt-bindings: timers: sp-804: Convert to json-schema
  ARM: dts: arm: Fix SP804 users
  ARM: dts: broadcom: Fix SP804 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  | 82 +++++++++++++++++++
 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, 133 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 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.
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  | 82 +++++++++++++++++++
 2 files changed, 82 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..77d35bc41176
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/arm,sp804.yaml
@@ -0,0 +1,82 @@
+# 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.
+
+# 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
+
+  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
+
+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 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>
---
 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 3/6] ARM: dts: broadcom: Fix SP804 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.

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 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 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 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 3/6] ARM: dts: broadcom: Fix SP804 node

[Florian Fainelli]

On 8/26/20 11:38 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.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

The commit subject should be:

ARM: dts: NSP: Fix SP804 compatible node

and we should probably have a Fixes tag that is:

Fixes: a0efb0d28b77 ("ARM: dts: NSP: Add SP804 Support to DT")

Could you please re-submit with those things corrected? Thanks

> ---
>  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>;
> 


-- 
Florian

Re: [PATCH 3/6] ARM: dts: broadcom: Fix SP804 node

[André Przywara]

On 26/08/2020 19:42, Florian Fainelli wrote:

Hi,

> On 8/26/20 11:38 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.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> 
> The commit subject should be:
> 
> ARM: dts: NSP: Fix SP804 compatible node
> 
> and we should probably have a Fixes tag that is:
> 
> Fixes: a0efb0d28b77 ("ARM: dts: NSP: Add SP804 Support to DT")
> 
> Could you please re-submit with those things corrected? Thanks

Sure, will include that in a v2.

Out of curiosity, do you have the hardware and can check the impact that
has?
Not sure we actually create the device without the primecell compatible?
Or is the sp804 an exception here, compared to the other AMBA devices
(SP805, PL011)?

Cheers,
Andre

>> ---
>>  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>;
>>
> 
> 

Re: [PATCH 3/6] ARM: dts: broadcom: Fix SP804 node

[Florian Fainelli]

On 8/26/20 11:53 AM, André Przywara wrote:
> On 26/08/2020 19:42, Florian Fainelli wrote:
> 
> Hi,
> 
>> On 8/26/20 11:38 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.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>
>> The commit subject should be:
>>
>> ARM: dts: NSP: Fix SP804 compatible node
>>
>> and we should probably have a Fixes tag that is:
>>
>> Fixes: a0efb0d28b77 ("ARM: dts: NSP: Add SP804 Support to DT")
>>
>> Could you please re-submit with those things corrected? Thanks
> 
> Sure, will include that in a v2.
> 
> Out of curiosity, do you have the hardware and can check the impact that
> has?

I have the hardware and could run some tests if you would like.

> Not sure we actually create the device without the primecell compatible?
> Or is the sp804 an exception here, compared to the other AMBA devices
> (SP805, PL011)?

No idea, I have never used those timers personally, and I doubt that
anybody besides me within broadcom and hobbyists actually care about NSP
these days.
-- 
Florian

Re: [PATCH 3/6] ARM: dts: broadcom: Fix SP804 node

[Florian Fainelli]

On 8/26/20 11:59 AM, Florian Fainelli wrote:
> On 8/26/20 11:53 AM, André Przywara wrote:
>> On 26/08/2020 19:42, Florian Fainelli wrote:
>>
>> Hi,
>>
>>> On 8/26/20 11:38 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.
>>>>
>>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>>
>>> The commit subject should be:
>>>
>>> ARM: dts: NSP: Fix SP804 compatible node
>>>
>>> and we should probably have a Fixes tag that is:
>>>
>>> Fixes: a0efb0d28b77 ("ARM: dts: NSP: Add SP804 Support to DT")
>>>
>>> Could you please re-submit with those things corrected? Thanks
>>
>> Sure, will include that in a v2.
>>
>> Out of curiosity, do you have the hardware and can check the impact that
>> has?
> 
> I have the hardware and could run some tests if you would like.
> 
>> Not sure we actually create the device without the primecell compatible?
>> Or is the sp804 an exception here, compared to the other AMBA devices
>> (SP805, PL011)?
> 
> No idea, I have never used those timers personally, and I doubt that
> anybody besides me within broadcom and hobbyists actually care about NSP
> these days.

Seems to be working fine for me with your patch applied, it probes:

# dmesg | grep sp804
[    0.035363] clocksource: arm,sp804: mask: 0xffffffff max_cycles:
0xffffffff, max_idle_ns: 15290083572 ns

and it is usable:

# cat clocksource0/available_clocksource
arm_global_timer arm,sp804

and appears to work:

# echo "arm,sp804" > clocksource0/current_clocksource
[  105.108547] clocksource: Switched to clocksource arm,sp804

# date; sleep 5; date
Thu Jan  1 00:01:51 UTC 1970
Thu Jan  1 00:01:56 UTC 1970

Feel free to add Tested-by: Florian Fainelli <f.fainelli@gmail.com> in
your v2, thanks André!
-- 
Florian

Re: [PATCH 3/6] ARM: dts: broadcom: Fix SP804 node

[Rob Herring]

On Wed, Aug 26, 2020 at 12:54 PM André Przywara <andre.przywara@arm.com> wrote:
>
> On 26/08/2020 19:42, Florian Fainelli wrote:
>
> Hi,
>
> > On 8/26/20 11:38 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.
> >>
> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> >
> > The commit subject should be:
> >
> > ARM: dts: NSP: Fix SP804 compatible node
> >
> > and we should probably have a Fixes tag that is:
> >
> > Fixes: a0efb0d28b77 ("ARM: dts: NSP: Add SP804 Support to DT")
> >
> > Could you please re-submit with those things corrected? Thanks
>
> Sure, will include that in a v2.
>
> Out of curiosity, do you have the hardware and can check the impact that
> has?
> Not sure we actually create the device without the primecell compatible?

My first thought was no, but since the timer isn't using the driver
model (i.e. amba bus), it doesn't need it.

So I think without it, we'd create a platform device instead, but then
there's some logic to prevent that IIRC.

Rob

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

[Rob Herring]

On Wed, Aug 26, 2020 at 12:38 PM Andre Przywara <andre.przywara@arm.com> 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.
> The requirement for the APB clock is enforced by the primecell binding
> already.

At least add 'clock-names: true' so you can add 'additionalProperties:
false'. Otherwise, looks good to me.

Rob

Re: [PATCH 3/6] ARM: dts: broadcom: Fix SP804 node

[André Przywara]

On 26/08/2020 21:55, Florian Fainelli wrote:
> On 8/26/20 11:59 AM, Florian Fainelli wrote:
>> On 8/26/20 11:53 AM, André Przywara wrote:
>>> On 26/08/2020 19:42, Florian Fainelli wrote:

Hi Florian,

>>>
>>> Hi,
>>>
>>>> On 8/26/20 11:38 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.
>>>>>
>>>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>>>
>>>> The commit subject should be:
>>>>
>>>> ARM: dts: NSP: Fix SP804 compatible node
>>>>
>>>> and we should probably have a Fixes tag that is:
>>>>
>>>> Fixes: a0efb0d28b77 ("ARM: dts: NSP: Add SP804 Support to DT")
>>>>
>>>> Could you please re-submit with those things corrected? Thanks
>>>
>>> Sure, will include that in a v2.
>>>
>>> Out of curiosity, do you have the hardware and can check the impact that
>>> has?
>>
>> I have the hardware and could run some tests if you would like.
>>
>>> Not sure we actually create the device without the primecell compatible?
>>> Or is the sp804 an exception here, compared to the other AMBA devices
>>> (SP805, PL011)?
>>
>> No idea, I have never used those timers personally, and I doubt that
>> anybody besides me within broadcom and hobbyists actually care about NSP
>> these days.
> 
> Seems to be working fine for me with your patch applied, it probes:
> 
> # dmesg | grep sp804
> [    0.035363] clocksource: arm,sp804: mask: 0xffffffff max_cycles:
> 0xffffffff, max_idle_ns: 15290083572 ns
> 
> and it is usable:
> 
> # cat clocksource0/available_clocksource
> arm_global_timer arm,sp804
> 
> and appears to work:
> 
> # echo "arm,sp804" > clocksource0/current_clocksource
> [  105.108547] clocksource: Switched to clocksource arm,sp804
> 
> # date; sleep 5; date
> Thu Jan  1 00:01:51 UTC 1970
> Thu Jan  1 00:01:56 UTC 1970
> 
> Feel free to add Tested-by: Florian Fainelli <f.fainelli@gmail.com> in
> your v2, thanks André!

Wow, thanks a lot for this test!

Sending out a v2 in a minute.

Cheers,
Andre

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

[Linus Walleij]

On Wed, Aug 26, 2020 at 8:38 PM Andre Przywara <andre.przywara@arm.com> wrote:

> 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>

This looks good to me, shall I simply apply this patch to my
Versatile tree (I suppose Sudeep should ack it too) or are
you sending it upstream to the soc tree?

Yours,
Linus Walleij

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

[André Przywara]

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

Hi,

> On Wed, Aug 26, 2020 at 8:38 PM Andre Przywara <andre.przywara@arm.com> wrote:
> 
>> 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>

Thanks!

> 
> This looks good to me, shall I simply apply this patch to my
> Versatile tree (I suppose Sudeep should ack it too) or are
> you sending it upstream to the soc tree?

If you want to take it (and Sudeep is OK with it), I am happy with that.
The DTs should work either way, so there is no dependency or anything.
One patch less to carry around for me ;-)
Just sent a v2 with your ACK, so please pick this one.

Thanks,
Andre