| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2d086fee-47d5-4be2-8b86-bf1cac6af803@riscstar.com>
Date: Mon, 21 Apr 2025 12:40:17 -0500
From: Alex Elder <elder@...cstar.com>
To: Yixun Lan <dlan@...too.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
Alexandre Ghiti <alex@...ti.fr>
Cc: Haylen Chu <heylenay@....org>, devicetree@...r.kernel.org,
linux-riscv@...ts.infradead.org, spacemit@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] riscv: dts: spacemit: Acquire clocks for UART
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@...too.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@...cstar.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@...15000 {
> uart0: serial@...17000 {
> 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@...17000 {
> uart2: serial@...17100 {
> 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@...17100 {
> uart3: serial@...17200 {
> 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@...17200 {
> uart4: serial@...17300 {
> 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@...17300 {
> uart5: serial@...17400 {
> 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@...17400 {
> uart6: serial@...17500 {
> 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@...17500 {
> uart7: serial@...17600 {
> 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@...17700 {
> 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@...17800 {
> 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";
>
Powered by blists - more mailing lists