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

Yixun Lan posted 2 patches 7 months, 4 weeks ago
There is a newer version of this series
[PATCH 2/2] riscv: dts: spacemit: Acquire clocks for UART
Posted by Yixun Lan 7 months, 4 weeks ago
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
Posted by Alex Elder 7 months, 3 weeks ago
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";
>