[PATCH 0/2] riscv: dts: spacemit: Add clocks to pinctrl and UART

[PATCH 0/2] riscv: dts: spacemit: Add clocks to pinctrl and UART

[Yixun Lan]

Populate clock property for pinctrl and UART controller.

The pinctrl's clock dt-binding patch is still waiting to be merged[1].

The UART's dt-binding and driver code has already been accepted[2],
so we now are only sending the DT part patch.

These two patches are abased on SpacemiT SoC tree's for-next branch[3]

Link: https://lore.kernel.org/r/20250416-02-k1-pinctrl-clk-v2-0-2b5fcbd4183c@gentoo.org [1]
Link: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/log/?h=tty-next [2]
Link: https://github.com/spacemit-com/linux/tree/for-next [3]
Signed-off-by: Yixun Lan <dlan@gentoo.org>
---
Yixun Lan (2):
      riscv: dts: spacemit: Acquire clocks for pinctrl
      riscv: dts: spacemit: Acquire clocks for UART

 arch/riscv/boot/dts/spacemit/k1.dtsi | 39 +++++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 9 deletions(-)
---
base-commit: 279d51ad9f6dc0c667f6f141a669b2c921277d1a
change-id: 20250419-05-dts-clock-026bfca75e5b

Best regards,
-- 
Yixun Lan

[PATCH 1/2] riscv: dts: spacemit: Acquire clocks for pinctrl

[Yixun Lan]

Pinctrl of K1 SoC need two clocks, so explicitly acquire them.

Signed-off-by: Yixun Lan <dlan@gentoo.org>
---
 arch/riscv/boot/dts/spacemit/k1.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
index 584f0dbc60f5b0d078c7127cc4021ad6022cb182..153fd1160182b42fe1a2f7f042c9c1da90f63b0c 100644
--- a/arch/riscv/boot/dts/spacemit/k1.dtsi
+++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
@@ -450,6 +450,9 @@ uart9: serial@d4017800 {
 		pinctrl: pinctrl@d401e000 {
 			compatible = "spacemit,k1-pinctrl";
 			reg = <0x0 0xd401e000 0x0 0x400>;
+			clocks = <&syscon_apbc CLK_AIB>,
+				 <&syscon_apbc CLK_AIB_BUS>;
+			clock-names = "func", "bus";
 		};
 
 		syscon_mpmu: system-controller@d4050000 {

-- 
2.49.0

Re: [PATCH 1/2] riscv: dts: spacemit: Acquire clocks for pinctrl

[Alex Elder]

On 4/18/25 10:32 PM, Yixun Lan wrote:
> Pinctrl of K1 SoC need two clocks, so explicitly acquire them.
> 
> Signed-off-by: Yixun Lan <dlan@gentoo.org>
> ---
>   arch/riscv/boot/dts/spacemit/k1.dtsi | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> index 584f0dbc60f5b0d078c7127cc4021ad6022cb182..153fd1160182b42fe1a2f7f042c9c1da90f63b0c 100644
> --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> @@ -450,6 +450,9 @@ uart9: serial@d4017800 {
>   		pinctrl: pinctrl@d401e000 {
>   			compatible = "spacemit,k1-pinctrl";
>   			reg = <0x0 0xd401e000 0x0 0x400>;
> +			clocks = <&syscon_apbc CLK_AIB>,
> +				 <&syscon_apbc CLK_AIB_BUS>;
> +			clock-names = "func", "bus";
>   		};
>   
>   		syscon_mpmu: system-controller@d4050000 {
> 

This looks good.

Reviewed-by: Alex Elder <elder@riscstar.com>

[PATCH 2/2] riscv: dts: spacemit: Acquire clocks for UART

[Yixun Lan]

The K1 SoC features two clocks for UART controller,
Acquire them explicitly in the driver.

Signed-off-by: Yixun Lan <dlan@gentoo.org>
---
 arch/riscv/boot/dts/spacemit/k1.dtsi | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
index 153fd1160182b42fe1a2f7f042c9c1da90f63b0c..415e1c3e1c78db987cbb65759adc26e98aaa24d3 100644
--- a/arch/riscv/boot/dts/spacemit/k1.dtsi
+++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
@@ -360,8 +360,10 @@ syscon_apbc: system-control@d4015000 {
 		uart0: serial@d4017000 {
 			compatible = "spacemit,k1-uart", "intel,xscale-uart";
 			reg = <0x0 0xd4017000 0x0 0x100>;
+			clocks = <&syscon_apbc CLK_UART0>,
+				 <&syscon_apbc CLK_UART0_BUS>;
+			clock-names = "core", "bus";
 			interrupts = <42>;
-			clock-frequency = <14857000>;
 			reg-shift = <2>;
 			reg-io-width = <4>;
 			status = "disabled";
@@ -370,8 +372,10 @@ uart0: serial@d4017000 {
 		uart2: serial@d4017100 {
 			compatible = "spacemit,k1-uart", "intel,xscale-uart";
 			reg = <0x0 0xd4017100 0x0 0x100>;
+			clocks = <&syscon_apbc CLK_UART2>,
+				 <&syscon_apbc CLK_UART2_BUS>;
+			clock-names = "core", "bus";
 			interrupts = <44>;
-			clock-frequency = <14857000>;
 			reg-shift = <2>;
 			reg-io-width = <4>;
 			status = "disabled";
@@ -380,8 +384,10 @@ uart2: serial@d4017100 {
 		uart3: serial@d4017200 {
 			compatible = "spacemit,k1-uart", "intel,xscale-uart";
 			reg = <0x0 0xd4017200 0x0 0x100>;
+			clocks = <&syscon_apbc CLK_UART3>,
+				 <&syscon_apbc CLK_UART3_BUS>;
+			clock-names = "core", "bus";
 			interrupts = <45>;
-			clock-frequency = <14857000>;
 			reg-shift = <2>;
 			reg-io-width = <4>;
 			status = "disabled";
@@ -390,8 +396,10 @@ uart3: serial@d4017200 {
 		uart4: serial@d4017300 {
 			compatible = "spacemit,k1-uart", "intel,xscale-uart";
 			reg = <0x0 0xd4017300 0x0 0x100>;
+			clocks = <&syscon_apbc CLK_UART4>,
+				 <&syscon_apbc CLK_UART4_BUS>;
+			clock-names = "core", "bus";
 			interrupts = <46>;
-			clock-frequency = <14857000>;
 			reg-shift = <2>;
 			reg-io-width = <4>;
 			status = "disabled";
@@ -400,8 +408,10 @@ uart4: serial@d4017300 {
 		uart5: serial@d4017400 {
 			compatible = "spacemit,k1-uart", "intel,xscale-uart";
 			reg = <0x0 0xd4017400 0x0 0x100>;
+			clocks = <&syscon_apbc CLK_UART5>,
+				 <&syscon_apbc CLK_UART5_BUS>;
+			clock-names = "core", "bus";
 			interrupts = <47>;
-			clock-frequency = <14857000>;
 			reg-shift = <2>;
 			reg-io-width = <4>;
 			status = "disabled";
@@ -410,8 +420,10 @@ uart5: serial@d4017400 {
 		uart6: serial@d4017500 {
 			compatible = "spacemit,k1-uart", "intel,xscale-uart";
 			reg = <0x0 0xd4017500 0x0 0x100>;
+			clocks = <&syscon_apbc CLK_UART6>,
+				 <&syscon_apbc CLK_UART6_BUS>;
+			clock-names = "core", "bus";
 			interrupts = <48>;
-			clock-frequency = <14857000>;
 			reg-shift = <2>;
 			reg-io-width = <4>;
 			status = "disabled";
@@ -420,8 +432,10 @@ uart6: serial@d4017500 {
 		uart7: serial@d4017600 {
 			compatible = "spacemit,k1-uart", "intel,xscale-uart";
 			reg = <0x0 0xd4017600 0x0 0x100>;
+			clocks = <&syscon_apbc CLK_UART7>,
+				 <&syscon_apbc CLK_UART7_BUS>;
+			clock-names = "core", "bus";
 			interrupts = <49>;
-			clock-frequency = <14857000>;
 			reg-shift = <2>;
 			reg-io-width = <4>;
 			status = "disabled";
@@ -431,7 +445,9 @@ uart8: serial@d4017700 {
 			compatible = "spacemit,k1-uart", "intel,xscale-uart";
 			reg = <0x0 0xd4017700 0x0 0x100>;
 			interrupts = <50>;
-			clock-frequency = <14857000>;
+			clocks = <&syscon_apbc CLK_UART8>,
+				 <&syscon_apbc CLK_UART8_BUS>;
+			clock-names = "core", "bus";
 			reg-shift = <2>;
 			reg-io-width = <4>;
 			status = "disabled";
@@ -441,7 +457,9 @@ uart9: serial@d4017800 {
 			compatible = "spacemit,k1-uart", "intel,xscale-uart";
 			reg = <0x0 0xd4017800 0x0 0x100>;
 			interrupts = <51>;
-			clock-frequency = <14857000>;
+			clocks = <&syscon_apbc CLK_UART9>,
+				 <&syscon_apbc CLK_UART9_BUS>;
+			clock-names = "core", "bus";
 			reg-shift = <2>;
 			reg-io-width = <4>;
 			status = "disabled";

-- 
2.49.0

Re: [PATCH 2/2] riscv: dts: spacemit: Acquire clocks for UART

[Alex Elder]

On 4/18/25 10:32 PM, Yixun Lan wrote:
> The K1 SoC features two clocks for UART controller,
> Acquire them explicitly in the driver.
> 
> Signed-off-by: Yixun Lan <dlan@gentoo.org>

I had an almost identical patch queued up to do this.

I think I'd mention explicitly in this description that you
are removing the clock-frequency property from all these nodes
(it is required to do this, otherwise the clock properties are
ignored by of_platform_serial_setup() in "8250_of.c").

Two more requests below.  Otherwise this looks good.

If you address all three of my comments, feel free to add:

Reviewed-by: Alex Elder <elder@riscstar.com>

> ---
>   arch/riscv/boot/dts/spacemit/k1.dtsi | 36 +++++++++++++++++++++++++++---------
>   1 file changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> index 153fd1160182b42fe1a2f7f042c9c1da90f63b0c..415e1c3e1c78db987cbb65759adc26e98aaa24d3 100644
> --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> @@ -360,8 +360,10 @@ syscon_apbc: system-control@d4015000 {
>   		uart0: serial@d4017000 {
>   			compatible = "spacemit,k1-uart", "intel,xscale-uart";
>   			reg = <0x0 0xd4017000 0x0 0x100>;
> +			clocks = <&syscon_apbc CLK_UART0>,
> +				 <&syscon_apbc CLK_UART0_BUS>;
> +			clock-names = "core", "bus";
>   			interrupts = <42>;
> -			clock-frequency = <14857000>;
>   			reg-shift = <2>;
>   			reg-io-width = <4>;
>   			status = "disabled";
> @@ -370,8 +372,10 @@ uart0: serial@d4017000 {
>   		uart2: serial@d4017100 {
>   			compatible = "spacemit,k1-uart", "intel,xscale-uart";
>   			reg = <0x0 0xd4017100 0x0 0x100>;
> +			clocks = <&syscon_apbc CLK_UART2>,
> +				 <&syscon_apbc CLK_UART2_BUS>;
> +			clock-names = "core", "bus";
>   			interrupts = <44>;
> -			clock-frequency = <14857000>;
>   			reg-shift = <2>;
>   			reg-io-width = <4>;
>   			status = "disabled";
> @@ -380,8 +384,10 @@ uart2: serial@d4017100 {
>   		uart3: serial@d4017200 {
>   			compatible = "spacemit,k1-uart", "intel,xscale-uart";
>   			reg = <0x0 0xd4017200 0x0 0x100>;
> +			clocks = <&syscon_apbc CLK_UART3>,
> +				 <&syscon_apbc CLK_UART3_BUS>;
> +			clock-names = "core", "bus";
>   			interrupts = <45>;
> -			clock-frequency = <14857000>;
>   			reg-shift = <2>;
>   			reg-io-width = <4>;
>   			status = "disabled";
> @@ -390,8 +396,10 @@ uart3: serial@d4017200 {
>   		uart4: serial@d4017300 {
>   			compatible = "spacemit,k1-uart", "intel,xscale-uart";
>   			reg = <0x0 0xd4017300 0x0 0x100>;
> +			clocks = <&syscon_apbc CLK_UART4>,
> +				 <&syscon_apbc CLK_UART4_BUS>;
> +			clock-names = "core", "bus";
>   			interrupts = <46>;
> -			clock-frequency = <14857000>;
>   			reg-shift = <2>;
>   			reg-io-width = <4>;
>   			status = "disabled";
> @@ -400,8 +408,10 @@ uart4: serial@d4017300 {
>   		uart5: serial@d4017400 {
>   			compatible = "spacemit,k1-uart", "intel,xscale-uart";
>   			reg = <0x0 0xd4017400 0x0 0x100>;
> +			clocks = <&syscon_apbc CLK_UART5>,
> +				 <&syscon_apbc CLK_UART5_BUS>;
> +			clock-names = "core", "bus";
>   			interrupts = <47>;
> -			clock-frequency = <14857000>;
>   			reg-shift = <2>;
>   			reg-io-width = <4>;
>   			status = "disabled";
> @@ -410,8 +420,10 @@ uart5: serial@d4017400 {
>   		uart6: serial@d4017500 {
>   			compatible = "spacemit,k1-uart", "intel,xscale-uart";
>   			reg = <0x0 0xd4017500 0x0 0x100>;
> +			clocks = <&syscon_apbc CLK_UART6>,
> +				 <&syscon_apbc CLK_UART6_BUS>;
> +			clock-names = "core", "bus";
>   			interrupts = <48>;
> -			clock-frequency = <14857000>;
>   			reg-shift = <2>;
>   			reg-io-width = <4>;
>   			status = "disabled";
> @@ -420,8 +432,10 @@ uart6: serial@d4017500 {
>   		uart7: serial@d4017600 {
>   			compatible = "spacemit,k1-uart", "intel,xscale-uart";
>   			reg = <0x0 0xd4017600 0x0 0x100>;
> +			clocks = <&syscon_apbc CLK_UART7>,
> +				 <&syscon_apbc CLK_UART7_BUS>;
> +			clock-names = "core", "bus";
>   			interrupts = <49>;
> -			clock-frequency = <14857000>;
>   			reg-shift = <2>;
>   			reg-io-width = <4>;
>   			status = "disabled";
> @@ -431,7 +445,9 @@ uart8: serial@d4017700 {
>   			compatible = "spacemit,k1-uart", "intel,xscale-uart";
>   			reg = <0x0 0xd4017700 0x0 0x100>;
>   			interrupts = <50>;
> -			clock-frequency = <14857000>;
> +			clocks = <&syscon_apbc CLK_UART8>,
> +				 <&syscon_apbc CLK_UART8_BUS>;
> +			clock-names = "core", "bus";

Please insert the clocks and clock-names properties *above* the
interrupts property, as you did for all of the above.

>   			reg-shift = <2>;
>   			reg-io-width = <4>;
>   			status = "disabled";
> @@ -441,7 +457,9 @@ uart9: serial@d4017800 {
>   			compatible = "spacemit,k1-uart", "intel,xscale-uart";
>   			reg = <0x0 0xd4017800 0x0 0x100>;
>   			interrupts = <51>;
> -			clock-frequency = <14857000>;
> +			clocks = <&syscon_apbc CLK_UART9>,
> +				 <&syscon_apbc CLK_UART9_BUS>;
> +			clock-names = "core", "bus";

Same comment here.

					-Alex

>   			reg-shift = <2>;
>   			reg-io-width = <4>;
>   			status = "disabled";
> 

Re: [PATCH 0/2] riscv: dts: spacemit: Add clocks to pinctrl and UART

[Haylen Chu]

On Sat, Apr 19, 2025 at 11:32:29AM +0800, Yixun Lan wrote:
> Populate clock property for pinctrl and UART controller.
> 
> The pinctrl's clock dt-binding patch is still waiting to be merged[1].
> 
> The UART's dt-binding and driver code has already been accepted[2],
> so we now are only sending the DT part patch.
> 
> These two patches are abased on SpacemiT SoC tree's for-next branch[3]
> 
> Link: https://lore.kernel.org/r/20250416-02-k1-pinctrl-clk-v2-0-2b5fcbd4183c@gentoo.org [1]
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/log/?h=tty-next [2]
> Link: https://github.com/spacemit-com/linux/tree/for-next [3]
> Signed-off-by: Yixun Lan <dlan@gentoo.org>

Generally this looks good to me, but I realized that splitting the
commit that introduces clock controllers and the one that correctly
fills clock properties for various peripherals may cause bisectable
issues, i.e. the UART won't function with only the clock controller
introduced and no clk_ignore_unused specified on commandline.

If this isn't really a problem, for the whole series,

Reviewed-by: Haylen Chu <heylenay@4d2.org>

> ---
> Yixun Lan (2):
>       riscv: dts: spacemit: Acquire clocks for pinctrl
>       riscv: dts: spacemit: Acquire clocks for UART
> 
>  arch/riscv/boot/dts/spacemit/k1.dtsi | 39 +++++++++++++++++++++++++++---------
>  1 file changed, 30 insertions(+), 9 deletions(-)
> ---
> base-commit: 279d51ad9f6dc0c667f6f141a669b2c921277d1a
> change-id: 20250419-05-dts-clock-026bfca75e5b
> 
> Best regards,
> -- 
> Yixun Lan
> 

Re: [PATCH 0/2] riscv: dts: spacemit: Add clocks to pinctrl and UART

[Yixun Lan]

On 09:42 Sat 19 Apr     , Haylen Chu wrote:
> On Sat, Apr 19, 2025 at 11:32:29AM +0800, Yixun Lan wrote:
> > Populate clock property for pinctrl and UART controller.
> > 
> > The pinctrl's clock dt-binding patch is still waiting to be merged[1].
> > 
> > The UART's dt-binding and driver code has already been accepted[2],
> > so we now are only sending the DT part patch.
> > 
> > These two patches are abased on SpacemiT SoC tree's for-next branch[3]
> > 
> > Link: https://lore.kernel.org/r/20250416-02-k1-pinctrl-clk-v2-0-2b5fcbd4183c@gentoo.org [1]
> > Link: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/log/?h=tty-next [2]
> > Link: https://github.com/spacemit-com/linux/tree/for-next [3]
> > Signed-off-by: Yixun Lan <dlan@gentoo.org>
> 
> Generally this looks good to me, but I realized that splitting the
> commit that introduces clock controllers and the one that correctly
> fills clock properties for various peripherals may cause bisectable
> issues, i.e. the UART won't function with only the clock controller
> introduced and no clk_ignore_unused specified on commandline.
> 
I don't think you should worry about this, my plan is to apply
these two patches to for-next branch of spacemiT SoC tree, which
exactly on top of clock patches, besides pinctrl[1], uart[2] patches
will go via different subsystem, and this series depend on them
in order to work properly at run time phase, so regarding this, it's
 kind of broken already.. but if take a high picture that they all
will be merged into for-next/master branch, then it's fine


> If this isn't really a problem, for the whole series,
> 
> Reviewed-by: Haylen Chu <heylenay@4d2.org>
> 
> > ---
> > Yixun Lan (2):
> >       riscv: dts: spacemit: Acquire clocks for pinctrl
> >       riscv: dts: spacemit: Acquire clocks for UART
> > 
> >  arch/riscv/boot/dts/spacemit/k1.dtsi | 39 +++++++++++++++++++++++++++---------
> >  1 file changed, 30 insertions(+), 9 deletions(-)
> > ---
> > base-commit: 279d51ad9f6dc0c667f6f141a669b2c921277d1a
> > change-id: 20250419-05-dts-clock-026bfca75e5b
> > 
> > Best regards,
> > -- 
> > Yixun Lan
> > 
> 

-- 
Yixun Lan (dlan)