public inbox for linux-riscv@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Change the sg2042 timer layout to fit aclint format
@ 2023-11-14  0:44 Inochi Amaoto
  2023-11-14  0:45 ` [PATCH v2 1/2] dt-bindings: timer: thead,c900-aclint-mtimer: separate mtime and mtimecmp regs Inochi Amaoto
  2023-11-14  0:45 ` [PATCH v2 2/2] riscv: dts: sophgo: separate sg2042 mtime and mtimecmp to fit aclint format Inochi Amaoto
  0 siblings, 2 replies; 12+ messages in thread
From: Inochi Amaoto @ 2023-11-14  0:44 UTC (permalink / raw)
  To: Daniel Lezcano, Thomas Gleixner, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Chao Wei, Chen Wang, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Inochi Amaoto, Xiaoguang Xing, Guo Ren
  Cc: Jisheng Zhang, linux-kernel, devicetree, linux-riscv

As the sg2042 uses different address for timer and mswi of its clint
device, it should follow the aclint format. For the previous patchs,
it only use only one address for both mtime and mtimer, this is can
not be parsed by OpenSBI. To resolve this, separate these two registers
in the dtb.

Link: https://lists.infradead.org/pipermail/opensbi/2023-October/005693.html
Link: https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc

Changed from v1:
1. change the commit to address the reason for ABI change.
2. remove unnecessary link in the commit.

Inochi Amaoto (2):
  dt-bindings: timer: thead,c900-aclint-mtimer: separate mtime and
    mtimecmp regs
  riscv: dts: sophgo: separate sg2042 mtime and mtimecmp to fit aclint
    format

 .../timer/thead,c900-aclint-mtimer.yaml       |  5 +-
 arch/riscv/boot/dts/sophgo/sg2042.dtsi        | 80 +++++++++++--------
 2 files changed, 51 insertions(+), 34 deletions(-)

--
2.42.1


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

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

* [PATCH v2 1/2] dt-bindings: timer: thead,c900-aclint-mtimer: separate mtime and mtimecmp regs
  2023-11-14  0:44 [PATCH v2 0/2] Change the sg2042 timer layout to fit aclint format Inochi Amaoto
@ 2023-11-14  0:45 ` Inochi Amaoto
  2023-11-14  1:09   ` Chen Wang
  2023-11-14  0:45 ` [PATCH v2 2/2] riscv: dts: sophgo: separate sg2042 mtime and mtimecmp to fit aclint format Inochi Amaoto
  1 sibling, 1 reply; 12+ messages in thread
From: Inochi Amaoto @ 2023-11-14  0:45 UTC (permalink / raw)
  To: Daniel Lezcano, Thomas Gleixner, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Inochi Amaoto, Chen Wang
  Cc: Guo Ren, Jisheng Zhang, linux-kernel, devicetree, linux-riscv

The timer registers of aclint don't follow the clint layout and can
be mapped on any different offset. As sg2042 uses separated timer
and mswi for its clint, it should follow the aclint spec and have
separated registers.

The previous patch introduces a new type of T-HEAD aclint timer which
has clint timer layout. Although the timer has the clint layout, it
should follow the aclint spec and uses the separated mtime and mtimecmp
regs. So a ABI change is needed to make the timer fit the aclint spec.

To make T-HEAD aclint timer more closer to the aclint spec, use two regs
to represent the mtime and mtimecmp.

Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
Fixes: 4734449f7311 ("dt-bindings: timer: Add Sophgo sg2042 CLINT timer")
Link: https://lists.infradead.org/pipermail/opensbi/2023-October/005693.html
Link: https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc
---
 .../devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml  | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml b/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
index fbd235650e52..c3080962d902 100644
--- a/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
+++ b/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
@@ -17,7 +17,7 @@ properties:
       - const: thead,c900-aclint-mtimer

   reg:
-    maxItems: 1
+    maxItems: 2

   interrupts-extended:
     minItems: 1
@@ -38,6 +38,7 @@ examples:
                             <&cpu2intc 7>,
                             <&cpu3intc 7>,
                             <&cpu4intc 7>;
-      reg = <0xac000000 0x00010000>;
+      reg = <0xac000000 0x00000000>,
+            <0xac000000 0x0000c000>;
     };
 ...
--
2.42.1


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

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

* [PATCH v2 2/2] riscv: dts: sophgo: separate sg2042 mtime and mtimecmp to fit aclint format
  2023-11-14  0:44 [PATCH v2 0/2] Change the sg2042 timer layout to fit aclint format Inochi Amaoto
  2023-11-14  0:45 ` [PATCH v2 1/2] dt-bindings: timer: thead,c900-aclint-mtimer: separate mtime and mtimecmp regs Inochi Amaoto
@ 2023-11-14  0:45 ` Inochi Amaoto
  2023-11-14  1:06   ` Chen Wang
  1 sibling, 1 reply; 12+ messages in thread
From: Inochi Amaoto @ 2023-11-14  0:45 UTC (permalink / raw)
  To: Chao Wei, Chen Wang, Conor Dooley, Rob Herring,
	Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Inochi Amaoto, Xiaoguang Xing
  Cc: Guo Ren, Jisheng Zhang, linux-riscv, devicetree, linux-kernel

Change the timer layout in the dtb to fit the format that needed by
the SBI.

Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
Fixes: 967a94a92aaa ("riscv: dts: add initial Sophgo SG2042 SoC device tree")
---
 arch/riscv/boot/dts/sophgo/sg2042.dtsi | 80 +++++++++++++++-----------
 1 file changed, 48 insertions(+), 32 deletions(-)

diff --git a/arch/riscv/boot/dts/sophgo/sg2042.dtsi b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
index 93256540d078..0b5d93b5c783 100644
--- a/arch/riscv/boot/dts/sophgo/sg2042.dtsi
+++ b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
@@ -93,144 +93,160 @@ clint_mswi: interrupt-controller@7094000000 {
 					      <&cpu63_intc 3>;
 		};

-		clint_mtimer0: timer@70ac000000 {
+		clint_mtimer0: timer@70ac004000 {
 			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
-			reg = <0x00000070 0xac000000 0x00000000 0x00007ff8>;
+			reg = <0x00000070 0xac004000 0x00000000 0x00000000>,
+			      <0x00000070 0xac004000 0x00000000 0x0000c000>;
 			interrupts-extended = <&cpu0_intc 7>,
 					      <&cpu1_intc 7>,
 					      <&cpu2_intc 7>,
 					      <&cpu3_intc 7>;
 		};

-		clint_mtimer1: timer@70ac010000 {
+		clint_mtimer1: timer@70ac014000 {
 			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
-			reg = <0x00000070 0xac010000 0x00000000 0x00007ff8>;
+			reg = <0x00000070 0xac014000 0x00000000 0x00000000>,
+			      <0x00000070 0xac014000 0x00000000 0x0000c000>;
 			interrupts-extended = <&cpu4_intc 7>,
 					      <&cpu5_intc 7>,
 					      <&cpu6_intc 7>,
 					      <&cpu7_intc 7>;
 		};

-		clint_mtimer2: timer@70ac020000 {
+		clint_mtimer2: timer@70ac024000 {
 			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
-			reg = <0x00000070 0xac020000 0x00000000 0x00007ff8>;
+			reg = <0x00000070 0xac024000 0x00000000 0x00000000>,
+			      <0x00000070 0xac024000 0x00000000 0x0000c000>;
 			interrupts-extended = <&cpu8_intc 7>,
 					      <&cpu9_intc 7>,
 					      <&cpu10_intc 7>,
 					      <&cpu11_intc 7>;
 		};

-		clint_mtimer3: timer@70ac030000 {
+		clint_mtimer3: timer@70ac034000 {
 			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
-			reg = <0x00000070 0xac030000 0x00000000 0x00007ff8>;
+			reg = <0x00000070 0xac034000 0x00000000 0x00000000>,
+			      <0x00000070 0xac034000 0x00000000 0x0000c000>;
 			interrupts-extended = <&cpu12_intc 7>,
 					      <&cpu13_intc 7>,
 					      <&cpu14_intc 7>,
 					      <&cpu15_intc 7>;
 		};

-		clint_mtimer4: timer@70ac040000 {
+		clint_mtimer4: timer@70ac044000 {
 			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
-			reg = <0x00000070 0xac040000 0x00000000 0x00007ff8>;
+			reg = <0x00000070 0xac044000 0x00000000 0x00000000>,
+			      <0x00000070 0xac044000 0x00000000 0x0000c000>;
 			interrupts-extended = <&cpu16_intc 7>,
 					      <&cpu17_intc 7>,
 					      <&cpu18_intc 7>,
 					      <&cpu19_intc 7>;
 		};

-		clint_mtimer5: timer@70ac050000 {
+		clint_mtimer5: timer@70ac054000 {
 			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
-			reg = <0x00000070 0xac050000 0x00000000 0x00007ff8>;
+			reg = <0x00000070 0xac054000 0x00000000 0x00000000>,
+			      <0x00000070 0xac054000 0x00000000 0x0000c000>;
 			interrupts-extended = <&cpu20_intc 7>,
 					      <&cpu21_intc 7>,
 					      <&cpu22_intc 7>,
 					      <&cpu23_intc 7>;
 		};

-		clint_mtimer6: timer@70ac060000 {
+		clint_mtimer6: timer@70ac064000 {
 			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
-			reg = <0x00000070 0xac060000 0x00000000 0x00007ff8>;
+			reg = <0x00000070 0xac064000 0x00000000 0x00000000>,
+			      <0x00000070 0xac064000 0x00000000 0x0000c000>;
 			interrupts-extended = <&cpu24_intc 7>,
 					      <&cpu25_intc 7>,
 					      <&cpu26_intc 7>,
 					      <&cpu27_intc 7>;
 		};

-		clint_mtimer7: timer@70ac070000 {
+		clint_mtimer7: timer@70ac074000 {
 			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
-			reg = <0x00000070 0xac070000 0x00000000 0x00007ff8>;
+			reg = <0x00000070 0xac074000 0x00000000 0x00000000>,
+			      <0x00000070 0xac074000 0x00000000 0x0000c000>;
 			interrupts-extended = <&cpu28_intc 7>,
 					      <&cpu29_intc 7>,
 					      <&cpu30_intc 7>,
 					      <&cpu31_intc 7>;
 		};

-		clint_mtimer8: timer@70ac080000 {
+		clint_mtimer8: timer@70ac084000 {
 			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
-			reg = <0x00000070 0xac080000 0x00000000 0x00007ff8>;
+			reg = <0x00000070 0xac084000 0x00000000 0x00000000>,
+			      <0x00000070 0xac084000 0x00000000 0x0000c000>;
 			interrupts-extended = <&cpu32_intc 7>,
 					      <&cpu33_intc 7>,
 					      <&cpu34_intc 7>,
 					      <&cpu35_intc 7>;
 		};

-		clint_mtimer9: timer@70ac090000 {
+		clint_mtimer9: timer@70ac094000 {
 			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
-			reg = <0x00000070 0xac090000 0x00000000 0x00007ff8>;
+			reg = <0x00000070 0xac094000 0x00000000 0x00000000>,
+			      <0x00000070 0xac094000 0x00000000 0x0000c000>;
 			interrupts-extended = <&cpu36_intc 7>,
 					      <&cpu37_intc 7>,
 					      <&cpu38_intc 7>,
 					      <&cpu39_intc 7>;
 		};

-		clint_mtimer10: timer@70ac0a0000 {
+		clint_mtimer10: timer@70ac0a4000 {
 			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
-			reg = <0x00000070 0xac0a0000 0x00000000 0x00007ff8>;
+			reg = <0x00000070 0xac0a4000 0x00000000 0x00000000>,
+			      <0x00000070 0xac0a4000 0x00000000 0x0000c000>;
 			interrupts-extended = <&cpu40_intc 7>,
 					      <&cpu41_intc 7>,
 					      <&cpu42_intc 7>,
 					      <&cpu43_intc 7>;
 		};

-		clint_mtimer11: timer@70ac0b0000 {
+		clint_mtimer11: timer@70ac0b4000 {
 			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
-			reg = <0x00000070 0xac0b0000 0x00000000 0x00007ff8>;
+			reg = <0x00000070 0xac0b4000 0x00000000 0x00000000>,
+			      <0x00000070 0xac0b4000 0x00000000 0x0000c000>;
 			interrupts-extended = <&cpu44_intc 7>,
 					      <&cpu45_intc 7>,
 					      <&cpu46_intc 7>,
 					      <&cpu47_intc 7>;
 		};

-		clint_mtimer12: timer@70ac0c0000 {
+		clint_mtimer12: timer@70ac0c4000 {
 			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
-			reg = <0x00000070 0xac0c0000 0x00000000 0x00007ff8>;
+			reg = <0x00000070 0xac0c4000 0x00000000 0x00000000>,
+			      <0x00000070 0xac0c4000 0x00000000 0x0000c000>;
 			interrupts-extended = <&cpu48_intc 7>,
 					      <&cpu49_intc 7>,
 					      <&cpu50_intc 7>,
 					      <&cpu51_intc 7>;
 		};

-		clint_mtimer13: timer@70ac0d0000 {
+		clint_mtimer13: timer@70ac0d4000 {
 			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
-			reg = <0x00000070 0xac0d0000 0x00000000 0x00007ff8>;
+			reg = <0x00000070 0xac0d4000 0x00000000 0x00000000>,
+			      <0x00000070 0xac0d4000 0x00000000 0x0000c000>;
 			interrupts-extended = <&cpu52_intc 7>,
 					      <&cpu53_intc 7>,
 					      <&cpu54_intc 7>,
 					      <&cpu55_intc 7>;
 		};

-		clint_mtimer14: timer@70ac0e0000 {
+		clint_mtimer14: timer@70ac0e4000 {
 			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
-			reg = <0x00000070 0xac0e0000 0x00000000 0x00007ff8>;
+			reg = <0x00000070 0xac0e4000 0x00000000 0x00000000>,
+			      <0x00000070 0xac0e4000 0x00000000 0x0000c000>;
 			interrupts-extended = <&cpu56_intc 7>,
 					      <&cpu57_intc 7>,
 					      <&cpu58_intc 7>,
 					      <&cpu59_intc 7>;
 		};

-		clint_mtimer15: timer@70ac0f0000 {
+		clint_mtimer15: timer@70ac0f4000 {
 			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
-			reg = <0x00000070 0xac0f0000 0x00000000 0x00007ff8>;
+			reg = <0x00000070 0xac0f4000 0x00000000 0x00000000>,
+			      <0x00000070 0xac0f4000 0x00000000 0x0000c000>;
 			interrupts-extended = <&cpu60_intc 7>,
 					      <&cpu61_intc 7>,
 					      <&cpu62_intc 7>,
--
2.42.1


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

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

* Re: [PATCH v2 2/2] riscv: dts: sophgo: separate sg2042 mtime and mtimecmp to fit aclint format
  2023-11-14  0:45 ` [PATCH v2 2/2] riscv: dts: sophgo: separate sg2042 mtime and mtimecmp to fit aclint format Inochi Amaoto
@ 2023-11-14  1:06   ` Chen Wang
  2023-11-14  1:47     ` Inochi Amaoto
  0 siblings, 1 reply; 12+ messages in thread
From: Chen Wang @ 2023-11-14  1:06 UTC (permalink / raw)
  To: Inochi Amaoto, Chao Wei, Conor Dooley, Rob Herring,
	Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Xiaoguang Xing
  Cc: Guo Ren, Jisheng Zhang, linux-riscv, devicetree, linux-kernel


On 2023/11/14 8:45, Inochi Amaoto wrote:
> Change the timer layout in the dtb to fit the format that needed by
> the SBI.
>
> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
> Fixes: 967a94a92aaa ("riscv: dts: add initial Sophgo SG2042 SoC device tree")
> ---
>   arch/riscv/boot/dts/sophgo/sg2042.dtsi | 80 +++++++++++++++-----------
>   1 file changed, 48 insertions(+), 32 deletions(-)
>
> diff --git a/arch/riscv/boot/dts/sophgo/sg2042.dtsi b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> index 93256540d078..0b5d93b5c783 100644
> --- a/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> +++ b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> @@ -93,144 +93,160 @@ clint_mswi: interrupt-controller@7094000000 {
>   					      <&cpu63_intc 3>;
>   		};
>
> -		clint_mtimer0: timer@70ac000000 {
> +		clint_mtimer0: timer@70ac004000 {

The address of timer register is changed,  and I guess it is another 
change not directly related to the topic of this patch.

Can you please add some comments in the commit message?

>   			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
> -			reg = <0x00000070 0xac000000 0x00000000 0x00007ff8>;
> +			reg = <0x00000070 0xac004000 0x00000000 0x00000000>,
Why the length of first item is zero? Can you please add some 
clarification in commit message?
> +			      <0x00000070 0xac004000 0x00000000 0x0000c000>;
>   			interrupts-extended = <&cpu0_intc 7>,
>   					      <&cpu1_intc 7>,
>   					      <&cpu2_intc 7>,
>   					      <&cpu3_intc 7>;
>   		};
>
> -		clint_mtimer1: timer@70ac010000 {
> +		clint_mtimer1: timer@70ac014000 {
>   			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
> -			reg = <0x00000070 0xac010000 0x00000000 0x00007ff8>;
> +			reg = <0x00000070 0xac014000 0x00000000 0x00000000>,
> +			      <0x00000070 0xac014000 0x00000000 0x0000c000>;
>   			interrupts-extended = <&cpu4_intc 7>,
>   					      <&cpu5_intc 7>,
>   					      <&cpu6_intc 7>,
>   					      <&cpu7_intc 7>;
>   		};
>
> -		clint_mtimer2: timer@70ac020000 {
> +		clint_mtimer2: timer@70ac024000 {
>   			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
> -			reg = <0x00000070 0xac020000 0x00000000 0x00007ff8>;
> +			reg = <0x00000070 0xac024000 0x00000000 0x00000000>,
> +			      <0x00000070 0xac024000 0x00000000 0x0000c000>;
>   			interrupts-extended = <&cpu8_intc 7>,
>   					      <&cpu9_intc 7>,
>   					      <&cpu10_intc 7>,
>   					      <&cpu11_intc 7>;
>   		};
>
> -		clint_mtimer3: timer@70ac030000 {
> +		clint_mtimer3: timer@70ac034000 {
>   			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
> -			reg = <0x00000070 0xac030000 0x00000000 0x00007ff8>;
> +			reg = <0x00000070 0xac034000 0x00000000 0x00000000>,
> +			      <0x00000070 0xac034000 0x00000000 0x0000c000>;
>   			interrupts-extended = <&cpu12_intc 7>,
>   					      <&cpu13_intc 7>,
>   					      <&cpu14_intc 7>,
>   					      <&cpu15_intc 7>;
>   		};
>
> -		clint_mtimer4: timer@70ac040000 {
> +		clint_mtimer4: timer@70ac044000 {
>   			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
> -			reg = <0x00000070 0xac040000 0x00000000 0x00007ff8>;
> +			reg = <0x00000070 0xac044000 0x00000000 0x00000000>,
> +			      <0x00000070 0xac044000 0x00000000 0x0000c000>;
>   			interrupts-extended = <&cpu16_intc 7>,
>   					      <&cpu17_intc 7>,
>   					      <&cpu18_intc 7>,
>   					      <&cpu19_intc 7>;
>   		};
>
> -		clint_mtimer5: timer@70ac050000 {
> +		clint_mtimer5: timer@70ac054000 {
>   			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
> -			reg = <0x00000070 0xac050000 0x00000000 0x00007ff8>;
> +			reg = <0x00000070 0xac054000 0x00000000 0x00000000>,
> +			      <0x00000070 0xac054000 0x00000000 0x0000c000>;
>   			interrupts-extended = <&cpu20_intc 7>,
>   					      <&cpu21_intc 7>,
>   					      <&cpu22_intc 7>,
>   					      <&cpu23_intc 7>;
>   		};
>
> -		clint_mtimer6: timer@70ac060000 {
> +		clint_mtimer6: timer@70ac064000 {
>   			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
> -			reg = <0x00000070 0xac060000 0x00000000 0x00007ff8>;
> +			reg = <0x00000070 0xac064000 0x00000000 0x00000000>,
> +			      <0x00000070 0xac064000 0x00000000 0x0000c000>;
>   			interrupts-extended = <&cpu24_intc 7>,
>   					      <&cpu25_intc 7>,
>   					      <&cpu26_intc 7>,
>   					      <&cpu27_intc 7>;
>   		};
>
> -		clint_mtimer7: timer@70ac070000 {
> +		clint_mtimer7: timer@70ac074000 {
>   			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
> -			reg = <0x00000070 0xac070000 0x00000000 0x00007ff8>;
> +			reg = <0x00000070 0xac074000 0x00000000 0x00000000>,
> +			      <0x00000070 0xac074000 0x00000000 0x0000c000>;
>   			interrupts-extended = <&cpu28_intc 7>,
>   					      <&cpu29_intc 7>,
>   					      <&cpu30_intc 7>,
>   					      <&cpu31_intc 7>;
>   		};
>
> -		clint_mtimer8: timer@70ac080000 {
> +		clint_mtimer8: timer@70ac084000 {
>   			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
> -			reg = <0x00000070 0xac080000 0x00000000 0x00007ff8>;
> +			reg = <0x00000070 0xac084000 0x00000000 0x00000000>,
> +			      <0x00000070 0xac084000 0x00000000 0x0000c000>;
>   			interrupts-extended = <&cpu32_intc 7>,
>   					      <&cpu33_intc 7>,
>   					      <&cpu34_intc 7>,
>   					      <&cpu35_intc 7>;
>   		};
>
> -		clint_mtimer9: timer@70ac090000 {
> +		clint_mtimer9: timer@70ac094000 {
>   			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
> -			reg = <0x00000070 0xac090000 0x00000000 0x00007ff8>;
> +			reg = <0x00000070 0xac094000 0x00000000 0x00000000>,
> +			      <0x00000070 0xac094000 0x00000000 0x0000c000>;
>   			interrupts-extended = <&cpu36_intc 7>,
>   					      <&cpu37_intc 7>,
>   					      <&cpu38_intc 7>,
>   					      <&cpu39_intc 7>;
>   		};
>
> -		clint_mtimer10: timer@70ac0a0000 {
> +		clint_mtimer10: timer@70ac0a4000 {
>   			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
> -			reg = <0x00000070 0xac0a0000 0x00000000 0x00007ff8>;
> +			reg = <0x00000070 0xac0a4000 0x00000000 0x00000000>,
> +			      <0x00000070 0xac0a4000 0x00000000 0x0000c000>;
>   			interrupts-extended = <&cpu40_intc 7>,
>   					      <&cpu41_intc 7>,
>   					      <&cpu42_intc 7>,
>   					      <&cpu43_intc 7>;
>   		};
>
> -		clint_mtimer11: timer@70ac0b0000 {
> +		clint_mtimer11: timer@70ac0b4000 {
>   			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
> -			reg = <0x00000070 0xac0b0000 0x00000000 0x00007ff8>;
> +			reg = <0x00000070 0xac0b4000 0x00000000 0x00000000>,
> +			      <0x00000070 0xac0b4000 0x00000000 0x0000c000>;
>   			interrupts-extended = <&cpu44_intc 7>,
>   					      <&cpu45_intc 7>,
>   					      <&cpu46_intc 7>,
>   					      <&cpu47_intc 7>;
>   		};
>
> -		clint_mtimer12: timer@70ac0c0000 {
> +		clint_mtimer12: timer@70ac0c4000 {
>   			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
> -			reg = <0x00000070 0xac0c0000 0x00000000 0x00007ff8>;
> +			reg = <0x00000070 0xac0c4000 0x00000000 0x00000000>,
> +			      <0x00000070 0xac0c4000 0x00000000 0x0000c000>;
>   			interrupts-extended = <&cpu48_intc 7>,
>   					      <&cpu49_intc 7>,
>   					      <&cpu50_intc 7>,
>   					      <&cpu51_intc 7>;
>   		};
>
> -		clint_mtimer13: timer@70ac0d0000 {
> +		clint_mtimer13: timer@70ac0d4000 {
>   			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
> -			reg = <0x00000070 0xac0d0000 0x00000000 0x00007ff8>;
> +			reg = <0x00000070 0xac0d4000 0x00000000 0x00000000>,
> +			      <0x00000070 0xac0d4000 0x00000000 0x0000c000>;
>   			interrupts-extended = <&cpu52_intc 7>,
>   					      <&cpu53_intc 7>,
>   					      <&cpu54_intc 7>,
>   					      <&cpu55_intc 7>;
>   		};
>
> -		clint_mtimer14: timer@70ac0e0000 {
> +		clint_mtimer14: timer@70ac0e4000 {
>   			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
> -			reg = <0x00000070 0xac0e0000 0x00000000 0x00007ff8>;
> +			reg = <0x00000070 0xac0e4000 0x00000000 0x00000000>,
> +			      <0x00000070 0xac0e4000 0x00000000 0x0000c000>;
>   			interrupts-extended = <&cpu56_intc 7>,
>   					      <&cpu57_intc 7>,
>   					      <&cpu58_intc 7>,
>   					      <&cpu59_intc 7>;
>   		};
>
> -		clint_mtimer15: timer@70ac0f0000 {
> +		clint_mtimer15: timer@70ac0f4000 {
>   			compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
> -			reg = <0x00000070 0xac0f0000 0x00000000 0x00007ff8>;
> +			reg = <0x00000070 0xac0f4000 0x00000000 0x00000000>,
> +			      <0x00000070 0xac0f4000 0x00000000 0x0000c000>;
>   			interrupts-extended = <&cpu60_intc 7>,
>   					      <&cpu61_intc 7>,
>   					      <&cpu62_intc 7>,
> --
> 2.42.1
>

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

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

* Re: [PATCH v2 1/2] dt-bindings: timer: thead,c900-aclint-mtimer: separate mtime and mtimecmp regs
  2023-11-14  0:45 ` [PATCH v2 1/2] dt-bindings: timer: thead,c900-aclint-mtimer: separate mtime and mtimecmp regs Inochi Amaoto
@ 2023-11-14  1:09   ` Chen Wang
  2023-11-14  1:45     ` Inochi Amaoto
  0 siblings, 1 reply; 12+ messages in thread
From: Chen Wang @ 2023-11-14  1:09 UTC (permalink / raw)
  To: Inochi Amaoto, Daniel Lezcano, Thomas Gleixner, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
	Albert Ou
  Cc: Guo Ren, Jisheng Zhang, linux-kernel, devicetree, linux-riscv


On 2023/11/14 8:45, Inochi Amaoto wrote:
> The timer registers of aclint don't follow the clint layout and can
> be mapped on any different offset. As sg2042 uses separated timer
> and mswi for its clint, it should follow the aclint spec and have
> separated registers.
>
> The previous patch introduces a new type of T-HEAD aclint timer which
> has clint timer layout. Although the timer has the clint layout, it
> should follow the aclint spec and uses the separated mtime and mtimecmp
> regs. So a ABI change is needed to make the timer fit the aclint spec.
>
> To make T-HEAD aclint timer more closer to the aclint spec, use two regs
> to represent the mtime and mtimecmp.
>
> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
> Fixes: 4734449f7311 ("dt-bindings: timer: Add Sophgo sg2042 CLINT timer")
> Link: https://lists.infradead.org/pipermail/opensbi/2023-October/005693.html
> Link: https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc
> ---
>   .../devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml  | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml b/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
> index fbd235650e52..c3080962d902 100644
> --- a/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
> +++ b/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
> @@ -17,7 +17,7 @@ properties:
>         - const: thead,c900-aclint-mtimer
>
>     reg:
> -    maxItems: 1
> +    maxItems: 2

The first one is for mtime and the second one is for mtimecmp, right? 
Recommend to add some comment in binding file to make it clear.

Chen

>
>     interrupts-extended:
>       minItems: 1
> @@ -38,6 +38,7 @@ examples:
>                               <&cpu2intc 7>,
>                               <&cpu3intc 7>,
>                               <&cpu4intc 7>;
> -      reg = <0xac000000 0x00010000>;
> +      reg = <0xac000000 0x00000000>,
> +            <0xac000000 0x0000c000>;
>       };
>   ...
> --
> 2.42.1
>

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

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

* Re: [PATCH v2 1/2] dt-bindings: timer: thead,c900-aclint-mtimer: separate mtime and mtimecmp regs
  2023-11-14  1:09   ` Chen Wang
@ 2023-11-14  1:45     ` Inochi Amaoto
  2023-11-14 15:21       ` Conor Dooley
  0 siblings, 1 reply; 12+ messages in thread
From: Inochi Amaoto @ 2023-11-14  1:45 UTC (permalink / raw)
  To: Chen Wang, Daniel Lezcano, Thomas Gleixner, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
	Albert Ou
  Cc: Inochi Amaoto, Guo Ren, Jisheng Zhang, linux-kernel, devicetree,
	linux-riscv

>On 2023/11/14 8:45, Inochi Amaoto wrote:
>> The timer registers of aclint don't follow the clint layout and can
>> be mapped on any different offset. As sg2042 uses separated timer
>> and mswi for its clint, it should follow the aclint spec and have
>> separated registers.
>>
>> The previous patch introduces a new type of T-HEAD aclint timer which
>> has clint timer layout. Although the timer has the clint layout, it
>> should follow the aclint spec and uses the separated mtime and mtimecmp
>> regs. So a ABI change is needed to make the timer fit the aclint spec.
>>
>> To make T-HEAD aclint timer more closer to the aclint spec, use two regs
>> to represent the mtime and mtimecmp.
>>
>> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
>> Fixes: 4734449f7311 ("dt-bindings: timer: Add Sophgo sg2042 CLINT timer")
>> Link: https://lists.infradead.org/pipermail/opensbi/2023-October/005693.html
>> Link: https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc
>> ---
>>   .../devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml  | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml b/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
>> index fbd235650e52..c3080962d902 100644
>> --- a/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
>> +++ b/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
>> @@ -17,7 +17,7 @@ properties:
>>         - const: thead,c900-aclint-mtimer
>>
>>     reg:
>> -    maxItems: 1
>> +    maxItems: 2
>
>The first one is for mtime and the second one is for mtimecmp, right?

Yes, that is right.

>Recommend to add some comment in binding file to make it clear.
>

Thanks for your advice.

>Chen
>
>>
>>     interrupts-extended:
>>       minItems: 1
>> @@ -38,6 +38,7 @@ examples:
>>                               <&cpu2intc 7>,
>>                               <&cpu3intc 7>,
>>                               <&cpu4intc 7>;
>> -      reg = <0xac000000 0x00010000>;
>> +      reg = <0xac000000 0x00000000>,
>> +            <0xac000000 0x0000c000>;
>>       };
>>   ...
>> --
>> 2.42.1
>>
>

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

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

* Re: [PATCH v2 2/2] riscv: dts: sophgo: separate sg2042 mtime and mtimecmp to fit aclint format
  2023-11-14  1:06   ` Chen Wang
@ 2023-11-14  1:47     ` Inochi Amaoto
  2023-11-14  2:10       ` Inochi Amaoto
  2023-11-14 15:24       ` Conor Dooley
  0 siblings, 2 replies; 12+ messages in thread
From: Inochi Amaoto @ 2023-11-14  1:47 UTC (permalink / raw)
  To: Chen Wang, Chao Wei, Conor Dooley, Rob Herring,
	Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Xiaoguang Xing
  Cc: Inochi Amaoto, Guo Ren, Jisheng Zhang, linux-riscv, devicetree,
	linux-kernel

>On 2023/11/14 8:45, Inochi Amaoto wrote:
>> Change the timer layout in the dtb to fit the format that needed by
>> the SBI.
>>
>> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
>> Fixes: 967a94a92aaa ("riscv: dts: add initial Sophgo SG2042 SoC device tree")
>> ---
>>   arch/riscv/boot/dts/sophgo/sg2042.dtsi | 80 +++++++++++++++-----------
>>   1 file changed, 48 insertions(+), 32 deletions(-)
>>
>> diff --git a/arch/riscv/boot/dts/sophgo/sg2042.dtsi b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
>> index 93256540d078..0b5d93b5c783 100644
>> --- a/arch/riscv/boot/dts/sophgo/sg2042.dtsi
>> +++ b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
>> @@ -93,144 +93,160 @@ clint_mswi: interrupt-controller@7094000000 {
>>                             <&cpu63_intc 3>;
>>           };
>>
>> -        clint_mtimer0: timer@70ac000000 {
>> +        clint_mtimer0: timer@70ac004000 {
>
>The address of timer register is changed,  and I guess it is another change not directly related to the topic of this patch.
>
>Can you please add some comments in the commit message?
>

As it needs to follow aclint format, the timer offset is applied to
identify the actual timer. So there is a change.

>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>> -            reg = <0x00000070 0xac000000 0x00000000 0x00007ff8>;
>> +            reg = <0x00000070 0xac004000 0x00000000 0x00000000>,
>Why the length of first item is zero? Can you please add some clarification in commit message?

I uses length zero to address that the mtimer is not supported, so the
SBI can know there is no mtimer in the timer.

>> +                  <0x00000070 0xac004000 0x00000000 0x0000c000>;
>>               interrupts-extended = <&cpu0_intc 7>,
>>                             <&cpu1_intc 7>,
>>                             <&cpu2_intc 7>,
>>                             <&cpu3_intc 7>;
>>           };
>>
>> -        clint_mtimer1: timer@70ac010000 {
>> +        clint_mtimer1: timer@70ac014000 {
>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>> -            reg = <0x00000070 0xac010000 0x00000000 0x00007ff8>;
>> +            reg = <0x00000070 0xac014000 0x00000000 0x00000000>,
>> +                  <0x00000070 0xac014000 0x00000000 0x0000c000>;
>>               interrupts-extended = <&cpu4_intc 7>,
>>                             <&cpu5_intc 7>,
>>                             <&cpu6_intc 7>,
>>                             <&cpu7_intc 7>;
>>           };
>>
>> -        clint_mtimer2: timer@70ac020000 {
>> +        clint_mtimer2: timer@70ac024000 {
>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>> -            reg = <0x00000070 0xac020000 0x00000000 0x00007ff8>;
>> +            reg = <0x00000070 0xac024000 0x00000000 0x00000000>,
>> +                  <0x00000070 0xac024000 0x00000000 0x0000c000>;
>>               interrupts-extended = <&cpu8_intc 7>,
>>                             <&cpu9_intc 7>,
>>                             <&cpu10_intc 7>,
>>                             <&cpu11_intc 7>;
>>           };
>>
>> -        clint_mtimer3: timer@70ac030000 {
>> +        clint_mtimer3: timer@70ac034000 {
>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>> -            reg = <0x00000070 0xac030000 0x00000000 0x00007ff8>;
>> +            reg = <0x00000070 0xac034000 0x00000000 0x00000000>,
>> +                  <0x00000070 0xac034000 0x00000000 0x0000c000>;
>>               interrupts-extended = <&cpu12_intc 7>,
>>                             <&cpu13_intc 7>,
>>                             <&cpu14_intc 7>,
>>                             <&cpu15_intc 7>;
>>           };
>>
>> -        clint_mtimer4: timer@70ac040000 {
>> +        clint_mtimer4: timer@70ac044000 {
>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>> -            reg = <0x00000070 0xac040000 0x00000000 0x00007ff8>;
>> +            reg = <0x00000070 0xac044000 0x00000000 0x00000000>,
>> +                  <0x00000070 0xac044000 0x00000000 0x0000c000>;
>>               interrupts-extended = <&cpu16_intc 7>,
>>                             <&cpu17_intc 7>,
>>                             <&cpu18_intc 7>,
>>                             <&cpu19_intc 7>;
>>           };
>>
>> -        clint_mtimer5: timer@70ac050000 {
>> +        clint_mtimer5: timer@70ac054000 {
>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>> -            reg = <0x00000070 0xac050000 0x00000000 0x00007ff8>;
>> +            reg = <0x00000070 0xac054000 0x00000000 0x00000000>,
>> +                  <0x00000070 0xac054000 0x00000000 0x0000c000>;
>>               interrupts-extended = <&cpu20_intc 7>,
>>                             <&cpu21_intc 7>,
>>                             <&cpu22_intc 7>,
>>                             <&cpu23_intc 7>;
>>           };
>>
>> -        clint_mtimer6: timer@70ac060000 {
>> +        clint_mtimer6: timer@70ac064000 {
>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>> -            reg = <0x00000070 0xac060000 0x00000000 0x00007ff8>;
>> +            reg = <0x00000070 0xac064000 0x00000000 0x00000000>,
>> +                  <0x00000070 0xac064000 0x00000000 0x0000c000>;
>>               interrupts-extended = <&cpu24_intc 7>,
>>                             <&cpu25_intc 7>,
>>                             <&cpu26_intc 7>,
>>                             <&cpu27_intc 7>;
>>           };
>>
>> -        clint_mtimer7: timer@70ac070000 {
>> +        clint_mtimer7: timer@70ac074000 {
>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>> -            reg = <0x00000070 0xac070000 0x00000000 0x00007ff8>;
>> +            reg = <0x00000070 0xac074000 0x00000000 0x00000000>,
>> +                  <0x00000070 0xac074000 0x00000000 0x0000c000>;
>>               interrupts-extended = <&cpu28_intc 7>,
>>                             <&cpu29_intc 7>,
>>                             <&cpu30_intc 7>,
>>                             <&cpu31_intc 7>;
>>           };
>>
>> -        clint_mtimer8: timer@70ac080000 {
>> +        clint_mtimer8: timer@70ac084000 {
>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>> -            reg = <0x00000070 0xac080000 0x00000000 0x00007ff8>;
>> +            reg = <0x00000070 0xac084000 0x00000000 0x00000000>,
>> +                  <0x00000070 0xac084000 0x00000000 0x0000c000>;
>>               interrupts-extended = <&cpu32_intc 7>,
>>                             <&cpu33_intc 7>,
>>                             <&cpu34_intc 7>,
>>                             <&cpu35_intc 7>;
>>           };
>>
>> -        clint_mtimer9: timer@70ac090000 {
>> +        clint_mtimer9: timer@70ac094000 {
>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>> -            reg = <0x00000070 0xac090000 0x00000000 0x00007ff8>;
>> +            reg = <0x00000070 0xac094000 0x00000000 0x00000000>,
>> +                  <0x00000070 0xac094000 0x00000000 0x0000c000>;
>>               interrupts-extended = <&cpu36_intc 7>,
>>                             <&cpu37_intc 7>,
>>                             <&cpu38_intc 7>,
>>                             <&cpu39_intc 7>;
>>           };
>>
>> -        clint_mtimer10: timer@70ac0a0000 {
>> +        clint_mtimer10: timer@70ac0a4000 {
>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>> -            reg = <0x00000070 0xac0a0000 0x00000000 0x00007ff8>;
>> +            reg = <0x00000070 0xac0a4000 0x00000000 0x00000000>,
>> +                  <0x00000070 0xac0a4000 0x00000000 0x0000c000>;
>>               interrupts-extended = <&cpu40_intc 7>,
>>                             <&cpu41_intc 7>,
>>                             <&cpu42_intc 7>,
>>                             <&cpu43_intc 7>;
>>           };
>>
>> -        clint_mtimer11: timer@70ac0b0000 {
>> +        clint_mtimer11: timer@70ac0b4000 {
>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>> -            reg = <0x00000070 0xac0b0000 0x00000000 0x00007ff8>;
>> +            reg = <0x00000070 0xac0b4000 0x00000000 0x00000000>,
>> +                  <0x00000070 0xac0b4000 0x00000000 0x0000c000>;
>>               interrupts-extended = <&cpu44_intc 7>,
>>                             <&cpu45_intc 7>,
>>                             <&cpu46_intc 7>,
>>                             <&cpu47_intc 7>;
>>           };
>>
>> -        clint_mtimer12: timer@70ac0c0000 {
>> +        clint_mtimer12: timer@70ac0c4000 {
>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>> -            reg = <0x00000070 0xac0c0000 0x00000000 0x00007ff8>;
>> +            reg = <0x00000070 0xac0c4000 0x00000000 0x00000000>,
>> +                  <0x00000070 0xac0c4000 0x00000000 0x0000c000>;
>>               interrupts-extended = <&cpu48_intc 7>,
>>                             <&cpu49_intc 7>,
>>                             <&cpu50_intc 7>,
>>                             <&cpu51_intc 7>;
>>           };
>>
>> -        clint_mtimer13: timer@70ac0d0000 {
>> +        clint_mtimer13: timer@70ac0d4000 {
>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>> -            reg = <0x00000070 0xac0d0000 0x00000000 0x00007ff8>;
>> +            reg = <0x00000070 0xac0d4000 0x00000000 0x00000000>,
>> +                  <0x00000070 0xac0d4000 0x00000000 0x0000c000>;
>>               interrupts-extended = <&cpu52_intc 7>,
>>                             <&cpu53_intc 7>,
>>                             <&cpu54_intc 7>,
>>                             <&cpu55_intc 7>;
>>           };
>>
>> -        clint_mtimer14: timer@70ac0e0000 {
>> +        clint_mtimer14: timer@70ac0e4000 {
>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>> -            reg = <0x00000070 0xac0e0000 0x00000000 0x00007ff8>;
>> +            reg = <0x00000070 0xac0e4000 0x00000000 0x00000000>,
>> +                  <0x00000070 0xac0e4000 0x00000000 0x0000c000>;
>>               interrupts-extended = <&cpu56_intc 7>,
>>                             <&cpu57_intc 7>,
>>                             <&cpu58_intc 7>,
>>                             <&cpu59_intc 7>;
>>           };
>>
>> -        clint_mtimer15: timer@70ac0f0000 {
>> +        clint_mtimer15: timer@70ac0f4000 {
>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>> -            reg = <0x00000070 0xac0f0000 0x00000000 0x00007ff8>;
>> +            reg = <0x00000070 0xac0f4000 0x00000000 0x00000000>,
>> +                  <0x00000070 0xac0f4000 0x00000000 0x0000c000>;
>>               interrupts-extended = <&cpu60_intc 7>,
>>                             <&cpu61_intc 7>,
>>                             <&cpu62_intc 7>,
>> --
>> 2.42.1
>>
>

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

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

* Re: [PATCH v2 2/2] riscv: dts: sophgo: separate sg2042 mtime and mtimecmp to fit aclint format
  2023-11-14  1:47     ` Inochi Amaoto
@ 2023-11-14  2:10       ` Inochi Amaoto
  2023-11-14 15:24       ` Conor Dooley
  1 sibling, 0 replies; 12+ messages in thread
From: Inochi Amaoto @ 2023-11-14  2:10 UTC (permalink / raw)
  To: Chen Wang, Chao Wei, Conor Dooley, Rob Herring,
	Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Xiaoguang Xing
  Cc: Inochi Amaoto, Guo Ren, Jisheng Zhang, linux-riscv, devicetree,
	linux-kernel

>
>>On 2023/11/14 8:45, Inochi Amaoto wrote:
>>> Change the timer layout in the dtb to fit the format that needed by
>>> the SBI.
>>>
>>> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
>>> Fixes: 967a94a92aaa ("riscv: dts: add initial Sophgo SG2042 SoC device tree")
>>> ---
>>>   arch/riscv/boot/dts/sophgo/sg2042.dtsi | 80 +++++++++++++++-----------
>>>   1 file changed, 48 insertions(+), 32 deletions(-)
>>>
>>> diff --git a/arch/riscv/boot/dts/sophgo/sg2042.dtsi b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
>>> index 93256540d078..0b5d93b5c783 100644
>>> --- a/arch/riscv/boot/dts/sophgo/sg2042.dtsi
>>> +++ b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
>>> @@ -93,144 +93,160 @@ clint_mswi: interrupt-controller@7094000000 {
>>>                             <&cpu63_intc 3>;
>>>           };
>>>
>>> -        clint_mtimer0: timer@70ac000000 {
>>> +        clint_mtimer0: timer@70ac004000 {
>>
>>The address of timer register is changed,  and I guess it is another change not directly related to the topic of this patch.
>>
>>Can you please add some comments in the commit message?
>>
>
>As it needs to follow aclint format, the timer offset is applied to
>identify the actual timer. So there is a change.
>
>>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>>> -            reg = <0x00000070 0xac000000 0x00000000 0x00007ff8>;
>>> +            reg = <0x00000070 0xac004000 0x00000000 0x00000000>,
>>Why the length of first item is zero? Can you please add some clarification in commit message?
>
>I uses length zero to address that the mtimer is not supported, so the
>SBI can know there is no mtimer in the timer.
>

There is a misspell: mtimer -> mtime.

>>> +                  <0x00000070 0xac004000 0x00000000 0x0000c000>;
>>>               interrupts-extended = <&cpu0_intc 7>,
>>>                             <&cpu1_intc 7>,
>>>                             <&cpu2_intc 7>,
>>>                             <&cpu3_intc 7>;
>>>           };
>>>
>>> -        clint_mtimer1: timer@70ac010000 {
>>> +        clint_mtimer1: timer@70ac014000 {
>>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>>> -            reg = <0x00000070 0xac010000 0x00000000 0x00007ff8>;
>>> +            reg = <0x00000070 0xac014000 0x00000000 0x00000000>,
>>> +                  <0x00000070 0xac014000 0x00000000 0x0000c000>;
>>>               interrupts-extended = <&cpu4_intc 7>,
>>>                             <&cpu5_intc 7>,
>>>                             <&cpu6_intc 7>,
>>>                             <&cpu7_intc 7>;
>>>           };
>>>
>>> -        clint_mtimer2: timer@70ac020000 {
>>> +        clint_mtimer2: timer@70ac024000 {
>>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>>> -            reg = <0x00000070 0xac020000 0x00000000 0x00007ff8>;
>>> +            reg = <0x00000070 0xac024000 0x00000000 0x00000000>,
>>> +                  <0x00000070 0xac024000 0x00000000 0x0000c000>;
>>>               interrupts-extended = <&cpu8_intc 7>,
>>>                             <&cpu9_intc 7>,
>>>                             <&cpu10_intc 7>,
>>>                             <&cpu11_intc 7>;
>>>           };
>>>
>>> -        clint_mtimer3: timer@70ac030000 {
>>> +        clint_mtimer3: timer@70ac034000 {
>>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>>> -            reg = <0x00000070 0xac030000 0x00000000 0x00007ff8>;
>>> +            reg = <0x00000070 0xac034000 0x00000000 0x00000000>,
>>> +                  <0x00000070 0xac034000 0x00000000 0x0000c000>;
>>>               interrupts-extended = <&cpu12_intc 7>,
>>>                             <&cpu13_intc 7>,
>>>                             <&cpu14_intc 7>,
>>>                             <&cpu15_intc 7>;
>>>           };
>>>
>>> -        clint_mtimer4: timer@70ac040000 {
>>> +        clint_mtimer4: timer@70ac044000 {
>>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>>> -            reg = <0x00000070 0xac040000 0x00000000 0x00007ff8>;
>>> +            reg = <0x00000070 0xac044000 0x00000000 0x00000000>,
>>> +                  <0x00000070 0xac044000 0x00000000 0x0000c000>;
>>>               interrupts-extended = <&cpu16_intc 7>,
>>>                             <&cpu17_intc 7>,
>>>                             <&cpu18_intc 7>,
>>>                             <&cpu19_intc 7>;
>>>           };
>>>
>>> -        clint_mtimer5: timer@70ac050000 {
>>> +        clint_mtimer5: timer@70ac054000 {
>>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>>> -            reg = <0x00000070 0xac050000 0x00000000 0x00007ff8>;
>>> +            reg = <0x00000070 0xac054000 0x00000000 0x00000000>,
>>> +                  <0x00000070 0xac054000 0x00000000 0x0000c000>;
>>>               interrupts-extended = <&cpu20_intc 7>,
>>>                             <&cpu21_intc 7>,
>>>                             <&cpu22_intc 7>,
>>>                             <&cpu23_intc 7>;
>>>           };
>>>
>>> -        clint_mtimer6: timer@70ac060000 {
>>> +        clint_mtimer6: timer@70ac064000 {
>>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>>> -            reg = <0x00000070 0xac060000 0x00000000 0x00007ff8>;
>>> +            reg = <0x00000070 0xac064000 0x00000000 0x00000000>,
>>> +                  <0x00000070 0xac064000 0x00000000 0x0000c000>;
>>>               interrupts-extended = <&cpu24_intc 7>,
>>>                             <&cpu25_intc 7>,
>>>                             <&cpu26_intc 7>,
>>>                             <&cpu27_intc 7>;
>>>           };
>>>
>>> -        clint_mtimer7: timer@70ac070000 {
>>> +        clint_mtimer7: timer@70ac074000 {
>>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>>> -            reg = <0x00000070 0xac070000 0x00000000 0x00007ff8>;
>>> +            reg = <0x00000070 0xac074000 0x00000000 0x00000000>,
>>> +                  <0x00000070 0xac074000 0x00000000 0x0000c000>;
>>>               interrupts-extended = <&cpu28_intc 7>,
>>>                             <&cpu29_intc 7>,
>>>                             <&cpu30_intc 7>,
>>>                             <&cpu31_intc 7>;
>>>           };
>>>
>>> -        clint_mtimer8: timer@70ac080000 {
>>> +        clint_mtimer8: timer@70ac084000 {
>>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>>> -            reg = <0x00000070 0xac080000 0x00000000 0x00007ff8>;
>>> +            reg = <0x00000070 0xac084000 0x00000000 0x00000000>,
>>> +                  <0x00000070 0xac084000 0x00000000 0x0000c000>;
>>>               interrupts-extended = <&cpu32_intc 7>,
>>>                             <&cpu33_intc 7>,
>>>                             <&cpu34_intc 7>,
>>>                             <&cpu35_intc 7>;
>>>           };
>>>
>>> -        clint_mtimer9: timer@70ac090000 {
>>> +        clint_mtimer9: timer@70ac094000 {
>>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>>> -            reg = <0x00000070 0xac090000 0x00000000 0x00007ff8>;
>>> +            reg = <0x00000070 0xac094000 0x00000000 0x00000000>,
>>> +                  <0x00000070 0xac094000 0x00000000 0x0000c000>;
>>>               interrupts-extended = <&cpu36_intc 7>,
>>>                             <&cpu37_intc 7>,
>>>                             <&cpu38_intc 7>,
>>>                             <&cpu39_intc 7>;
>>>           };
>>>
>>> -        clint_mtimer10: timer@70ac0a0000 {
>>> +        clint_mtimer10: timer@70ac0a4000 {
>>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>>> -            reg = <0x00000070 0xac0a0000 0x00000000 0x00007ff8>;
>>> +            reg = <0x00000070 0xac0a4000 0x00000000 0x00000000>,
>>> +                  <0x00000070 0xac0a4000 0x00000000 0x0000c000>;
>>>               interrupts-extended = <&cpu40_intc 7>,
>>>                             <&cpu41_intc 7>,
>>>                             <&cpu42_intc 7>,
>>>                             <&cpu43_intc 7>;
>>>           };
>>>
>>> -        clint_mtimer11: timer@70ac0b0000 {
>>> +        clint_mtimer11: timer@70ac0b4000 {
>>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>>> -            reg = <0x00000070 0xac0b0000 0x00000000 0x00007ff8>;
>>> +            reg = <0x00000070 0xac0b4000 0x00000000 0x00000000>,
>>> +                  <0x00000070 0xac0b4000 0x00000000 0x0000c000>;
>>>               interrupts-extended = <&cpu44_intc 7>,
>>>                             <&cpu45_intc 7>,
>>>                             <&cpu46_intc 7>,
>>>                             <&cpu47_intc 7>;
>>>           };
>>>
>>> -        clint_mtimer12: timer@70ac0c0000 {
>>> +        clint_mtimer12: timer@70ac0c4000 {
>>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>>> -            reg = <0x00000070 0xac0c0000 0x00000000 0x00007ff8>;
>>> +            reg = <0x00000070 0xac0c4000 0x00000000 0x00000000>,
>>> +                  <0x00000070 0xac0c4000 0x00000000 0x0000c000>;
>>>               interrupts-extended = <&cpu48_intc 7>,
>>>                             <&cpu49_intc 7>,
>>>                             <&cpu50_intc 7>,
>>>                             <&cpu51_intc 7>;
>>>           };
>>>
>>> -        clint_mtimer13: timer@70ac0d0000 {
>>> +        clint_mtimer13: timer@70ac0d4000 {
>>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>>> -            reg = <0x00000070 0xac0d0000 0x00000000 0x00007ff8>;
>>> +            reg = <0x00000070 0xac0d4000 0x00000000 0x00000000>,
>>> +                  <0x00000070 0xac0d4000 0x00000000 0x0000c000>;
>>>               interrupts-extended = <&cpu52_intc 7>,
>>>                             <&cpu53_intc 7>,
>>>                             <&cpu54_intc 7>,
>>>                             <&cpu55_intc 7>;
>>>           };
>>>
>>> -        clint_mtimer14: timer@70ac0e0000 {
>>> +        clint_mtimer14: timer@70ac0e4000 {
>>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>>> -            reg = <0x00000070 0xac0e0000 0x00000000 0x00007ff8>;
>>> +            reg = <0x00000070 0xac0e4000 0x00000000 0x00000000>,
>>> +                  <0x00000070 0xac0e4000 0x00000000 0x0000c000>;
>>>               interrupts-extended = <&cpu56_intc 7>,
>>>                             <&cpu57_intc 7>,
>>>                             <&cpu58_intc 7>,
>>>                             <&cpu59_intc 7>;
>>>           };
>>>
>>> -        clint_mtimer15: timer@70ac0f0000 {
>>> +        clint_mtimer15: timer@70ac0f4000 {
>>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>>> -            reg = <0x00000070 0xac0f0000 0x00000000 0x00007ff8>;
>>> +            reg = <0x00000070 0xac0f4000 0x00000000 0x00000000>,
>>> +                  <0x00000070 0xac0f4000 0x00000000 0x0000c000>;
>>>               interrupts-extended = <&cpu60_intc 7>,
>>>                             <&cpu61_intc 7>,
>>>                             <&cpu62_intc 7>,
>>> --
>>> 2.42.1
>>>
>>
>

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

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

* Re: [PATCH v2 1/2] dt-bindings: timer: thead,c900-aclint-mtimer: separate mtime and mtimecmp regs
  2023-11-14  1:45     ` Inochi Amaoto
@ 2023-11-14 15:21       ` Conor Dooley
  2023-11-14 22:45         ` Inochi Amaoto
  0 siblings, 1 reply; 12+ messages in thread
From: Conor Dooley @ 2023-11-14 15:21 UTC (permalink / raw)
  To: Inochi Amaoto
  Cc: Chen Wang, Daniel Lezcano, Thomas Gleixner, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Guo Ren, Jisheng Zhang, linux-kernel, devicetree,
	linux-riscv


[-- Attachment #1.1: Type: text/plain, Size: 2104 bytes --]

On Tue, Nov 14, 2023 at 09:45:33AM +0800, Inochi Amaoto wrote:
> >On 2023/11/14 8:45, Inochi Amaoto wrote:
> >> The timer registers of aclint don't follow the clint layout and can
> >> be mapped on any different offset. As sg2042 uses separated timer
> >> and mswi for its clint, it should follow the aclint spec and have
> >> separated registers.
> >>
> >> The previous patch introduces a new type of T-HEAD aclint timer which
> >> has clint timer layout. Although the timer has the clint layout, it
> >> should follow the aclint spec and uses the separated mtime and mtimecmp
> >> regs. So a ABI change is needed to make the timer fit the aclint spec.
> >>
> >> To make T-HEAD aclint timer more closer to the aclint spec, use two regs
> >> to represent the mtime and mtimecmp.
> >>
> >> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
> >> Fixes: 4734449f7311 ("dt-bindings: timer: Add Sophgo sg2042 CLINT timer")
> >> Link: https://lists.infradead.org/pipermail/opensbi/2023-October/005693.html
> >> Link: https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc
> >> ---
> >>   .../devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml  | 5 +++--
> >>   1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml b/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
> >> index fbd235650e52..c3080962d902 100644
> >> --- a/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
> >> +++ b/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
> >> @@ -17,7 +17,7 @@ properties:
> >>         - const: thead,c900-aclint-mtimer
> >>
> >>     reg:
> >> -    maxItems: 1
> >> +    maxItems: 2
> >
> >The first one is for mtime and the second one is for mtimecmp, right?
> 
> Yes, that is right.
> 
> >Recommend to add some comment in binding file to make it clear.
> >
> 
> Thanks for your advice.

Sorry for not noticing that on v1 - you should indeed describe these in
the binding, by using the items property.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

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

* Re: [PATCH v2 2/2] riscv: dts: sophgo: separate sg2042 mtime and mtimecmp to fit aclint format
  2023-11-14  1:47     ` Inochi Amaoto
  2023-11-14  2:10       ` Inochi Amaoto
@ 2023-11-14 15:24       ` Conor Dooley
  2023-11-14 23:13         ` Inochi Amaoto
  1 sibling, 1 reply; 12+ messages in thread
From: Conor Dooley @ 2023-11-14 15:24 UTC (permalink / raw)
  To: Inochi Amaoto
  Cc: Chen Wang, Chao Wei, Rob Herring, Krzysztof Kozlowski,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Xiaoguang Xing, Guo Ren,
	Jisheng Zhang, linux-riscv, devicetree, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 2087 bytes --]

On Tue, Nov 14, 2023 at 09:47:19AM +0800, Inochi Amaoto wrote:
> >On 2023/11/14 8:45, Inochi Amaoto wrote:
> >> Change the timer layout in the dtb to fit the format that needed by
> >> the SBI.
> >>
> >> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
> >> Fixes: 967a94a92aaa ("riscv: dts: add initial Sophgo SG2042 SoC device tree")
> >> ---
> >>   arch/riscv/boot/dts/sophgo/sg2042.dtsi | 80 +++++++++++++++-----------
> >>   1 file changed, 48 insertions(+), 32 deletions(-)
> >>
> >> diff --git a/arch/riscv/boot/dts/sophgo/sg2042.dtsi b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> >> index 93256540d078..0b5d93b5c783 100644
> >> --- a/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> >> +++ b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> >> @@ -93,144 +93,160 @@ clint_mswi: interrupt-controller@7094000000 {
> >>                             <&cpu63_intc 3>;
> >>           };
> >>
> >> -        clint_mtimer0: timer@70ac000000 {
> >> +        clint_mtimer0: timer@70ac004000 {
> >
> >The address of timer register is changed,  and I guess it is another change not directly related to the topic of this patch.
> >
> >Can you please add some comments in the commit message?
> >
> 
> As it needs to follow aclint format, the timer offset is applied to
> identify the actual timer. So there is a change.
> 
> >>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
> >> -            reg = <0x00000070 0xac000000 0x00000000 0x00007ff8>;
> >> +            reg = <0x00000070 0xac004000 0x00000000 0x00000000>,
> >Why the length of first item is zero? Can you please add some clarification in commit message?
> 
> I uses length zero to address that the mtimer is not supported, so the
> SBI can know there is no mtimer in the timer.

No, that's unacceptably hacky. If there is only one of the two registers
present, then you need to provide only one of them, not spoof the
presence of two. I suppose that means you need to add reg-names to the
binding & get your registers by name in the SBI implementation, not by
index.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

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

* Re: [PATCH v2 1/2] dt-bindings: timer: thead,c900-aclint-mtimer: separate mtime and mtimecmp regs
  2023-11-14 15:21       ` Conor Dooley
@ 2023-11-14 22:45         ` Inochi Amaoto
  0 siblings, 0 replies; 12+ messages in thread
From: Inochi Amaoto @ 2023-11-14 22:45 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Inochi Amaoto, Chen Wang, Daniel Lezcano, Thomas Gleixner,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Guo Ren, Jisheng Zhang, linux-kernel,
	devicetree, linux-riscv

>On Tue, Nov 14, 2023 at 09:45:33AM +0800, Inochi Amaoto wrote:
>>> On 2023/11/14 8:45, Inochi Amaoto wrote:
>>>> The timer registers of aclint don't follow the clint layout and can
>>>> be mapped on any different offset. As sg2042 uses separated timer
>>>> and mswi for its clint, it should follow the aclint spec and have
>>>> separated registers.
>>>>
>>>> The previous patch introduces a new type of T-HEAD aclint timer which
>>>> has clint timer layout. Although the timer has the clint layout, it
>>>> should follow the aclint spec and uses the separated mtime and mtimecmp
>>>> regs. So a ABI change is needed to make the timer fit the aclint spec.
>>>>
>>>> To make T-HEAD aclint timer more closer to the aclint spec, use two regs
>>>> to represent the mtime and mtimecmp.
>>>>
>>>> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
>>>> Fixes: 4734449f7311 ("dt-bindings: timer: Add Sophgo sg2042 CLINT timer")
>>>> Link: https://lists.infradead.org/pipermail/opensbi/2023-October/005693.html
>>>> Link: https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc
>>>> ---
>>>>   .../devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml  | 5 +++--
>>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml b/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
>>>> index fbd235650e52..c3080962d902 100644
>>>> --- a/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
>>>> +++ b/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
>>>> @@ -17,7 +17,7 @@ properties:
>>>>         - const: thead,c900-aclint-mtimer
>>>>
>>>>     reg:
>>>> -    maxItems: 1
>>>> +    maxItems: 2
>>>
>>> The first one is for mtime and the second one is for mtimecmp, right?
>>
>> Yes, that is right.
>>
>>> Recommend to add some comment in binding file to make it clear.
>>>
>>
>> Thanks for your advice.
>
>Sorry for not noticing that on v1 -

Sorry for this, I have seen the v1 and improve the comment of the v2. I
will give a feedback next time. Anyway, thank you for your advice in v1.

>you should indeed describe these in the binding, by using the items property.
>

Thanks, I will have a try.

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

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

* Re: [PATCH v2 2/2] riscv: dts: sophgo: separate sg2042 mtime and mtimecmp to fit aclint format
  2023-11-14 15:24       ` Conor Dooley
@ 2023-11-14 23:13         ` Inochi Amaoto
  0 siblings, 0 replies; 12+ messages in thread
From: Inochi Amaoto @ 2023-11-14 23:13 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Inochi Amaoto, Chen Wang, Chao Wei, Rob Herring,
	Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Xiaoguang Xing, Guo Ren, Jisheng Zhang, linux-riscv, devicetree,
	linux-kernel

>On Tue, Nov 14, 2023 at 09:47:19AM +0800, Inochi Amaoto wrote:
>>> On 2023/11/14 8:45, Inochi Amaoto wrote:
>>>> Change the timer layout in the dtb to fit the format that needed by
>>>> the SBI.
>>>>
>>>> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
>>>> Fixes: 967a94a92aaa ("riscv: dts: add initial Sophgo SG2042 SoC device tree")
>>>> ---
>>>>   arch/riscv/boot/dts/sophgo/sg2042.dtsi | 80 +++++++++++++++-----------
>>>>   1 file changed, 48 insertions(+), 32 deletions(-)
>>>>
>>>> diff --git a/arch/riscv/boot/dts/sophgo/sg2042.dtsi b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
>>>> index 93256540d078..0b5d93b5c783 100644
>>>> --- a/arch/riscv/boot/dts/sophgo/sg2042.dtsi
>>>> +++ b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
>>>> @@ -93,144 +93,160 @@ clint_mswi: interrupt-controller@7094000000 {
>>>>                             <&cpu63_intc 3>;
>>>>           };
>>>>
>>>> -        clint_mtimer0: timer@70ac000000 {
>>>> +        clint_mtimer0: timer@70ac004000 {
>>>
>>> The address of timer register is changed,  and I guess it is another change not directly related to the topic of this patch.
>>>
>>> Can you please add some comments in the commit message?
>>>
>>
>> As it needs to follow aclint format, the timer offset is applied to
>> identify the actual timer. So there is a change.
>>
>>>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>>>> -            reg = <0x00000070 0xac000000 0x00000000 0x00007ff8>;
>>>> +            reg = <0x00000070 0xac004000 0x00000000 0x00000000>,
>>> Why the length of first item is zero? Can you please add some clarification in commit message?
>>
>> I uses length zero to address that the mtimer is not supported, so the
>> SBI can know there is no mtimer in the timer.
>
>No, that's unacceptably hacky. If there is only one of the two registers
>present, then you need to provide only one of them, not spoof the
>presence of two. I suppose that means you need to add reg-names to the
>binding & get your registers by name in the SBI implementation, not by
>index.
>

Yes, this is more clear. And it can fully utilize the change from DT
binding. I will figure out how to improve this with reg-names.

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

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

end of thread, other threads:[~2023-11-14 23:13 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-14  0:44 [PATCH v2 0/2] Change the sg2042 timer layout to fit aclint format Inochi Amaoto
2023-11-14  0:45 ` [PATCH v2 1/2] dt-bindings: timer: thead,c900-aclint-mtimer: separate mtime and mtimecmp regs Inochi Amaoto
2023-11-14  1:09   ` Chen Wang
2023-11-14  1:45     ` Inochi Amaoto
2023-11-14 15:21       ` Conor Dooley
2023-11-14 22:45         ` Inochi Amaoto
2023-11-14  0:45 ` [PATCH v2 2/2] riscv: dts: sophgo: separate sg2042 mtime and mtimecmp to fit aclint format Inochi Amaoto
2023-11-14  1:06   ` Chen Wang
2023-11-14  1:47     ` Inochi Amaoto
2023-11-14  2:10       ` Inochi Amaoto
2023-11-14 15:24       ` Conor Dooley
2023-11-14 23:13         ` Inochi Amaoto

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox