diff mbox series

[RESEND,v5,5/6] arm64: dts: marvell: armada-37xx: add device node for UART clock and use it

Message ID 20210922105433.11744-6-pali@kernel.org (mailing list archive)
State New, archived
Headers show
Series serial: mvebu-uart: Support for higher baudrates | expand

Commit Message

Pali Rohár Sept. 22, 2021, 10:54 a.m. UTC 
This change defines DT node for UART clock "marvell,armada-3700-uart-clock"
and use this UART clock as a base clock for all UART devices.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 arch/arm64/boot/dts/marvell/armada-3720-db.dts    |  4 ++++
 .../boot/dts/marvell/armada-3720-espressobin.dtsi |  4 ++++
 .../boot/dts/marvell/armada-3720-turris-mox.dts   |  4 ++++
 arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts  |  4 ++++
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi      | 15 +++++++++++++--
 5 files changed, 29 insertions(+), 2 deletions(-)

Comments

Gregory CLEMENT Sept. 22, 2021, 3:16 p.m. UTC | #1 
Hello Pali,

> This change defines DT node for UART clock "marvell,armada-3700-uart-clock"
> and use this UART clock as a base clock for all UART devices.

Sorry to not have pointed this earlier but I found something a little
unusual, see below:

>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  arch/arm64/boot/dts/marvell/armada-3720-db.dts    |  4 ++++
>  .../boot/dts/marvell/armada-3720-espressobin.dtsi |  4 ++++
>  .../boot/dts/marvell/armada-3720-turris-mox.dts   |  4 ++++
>  arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts  |  4 ++++
>  arch/arm64/boot/dts/marvell/armada-37xx.dtsi      | 15 +++++++++++++--
>  5 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-db.dts b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
> index 3e5789f37206..accf014a6a1e 100644
> --- a/arch/arm64/boot/dts/marvell/armada-3720-db.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
> @@ -191,6 +191,10 @@
>  	};
>  };
>  
> +&uartclk {
> +	status = "okay";

I found unusual to have to enable the clock at device tree level.
Usually the clock driver is always loaded and then the clock is really
enabled or disabled through the clock framework.

[...]

> diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> index 9acc5d2b5a00..5bc61c9615f5 100644
> --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> @@ -132,10 +132,21 @@
>  				reg = <0x11500 0x40>;
>  			};
>  
> +			uartclk: uartclk@12000 {
> +				compatible = "marvell,armada-3700-uart-clock";
> +				reg = <0x12010 0x4>, <0x12210 0x4>;
> +				clocks = <&tbg 0>, <&tbg 1>, <&tbg 2>,
> +					<&tbg 3>, <&xtalclk>;
> +				clock-names = "TBG-A-P", "TBG-B-P", "TBG-A-S",
> +					"TBG-B-S", "xtal";
> +				#clock-cells = <1>;

I think you could remove the following line and thanks to this there
won't be any change in the dts of the board:
> +				status = "disabled";
> +			};
> +

Gregory


>  			uart0: serial@12000 {
>  				compatible = "marvell,armada-3700-uart";
>  				reg = <0x12000 0x18>;
> -				clocks = <&xtalclk>;
> +				clocks = <&uartclk 0>;
>  				interrupts =
>  				<GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
>  				<GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>,
> @@ -147,7 +158,7 @@
>  			uart1: serial@12200 {
>  				compatible = "marvell,armada-3700-uart-ext";
>  				reg = <0x12200 0x30>;
> -				clocks = <&xtalclk>;
> +				clocks = <&uartclk 1>;
>  				interrupts =
>  				<GIC_SPI 30 IRQ_TYPE_EDGE_RISING>,
>  				<GIC_SPI 31 IRQ_TYPE_EDGE_RISING>;
> -- 
> 2.20.1
>
Pali Rohár Sept. 22, 2021, 4:07 p.m. UTC | #2 
On Wednesday 22 September 2021 17:16:22 Gregory CLEMENT wrote:
> Hello Pali,
> 
> > This change defines DT node for UART clock "marvell,armada-3700-uart-clock"
> > and use this UART clock as a base clock for all UART devices.
> 
> Sorry to not have pointed this earlier but I found something a little
> unusual, see below:
> 
> >
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >  arch/arm64/boot/dts/marvell/armada-3720-db.dts    |  4 ++++
> >  .../boot/dts/marvell/armada-3720-espressobin.dtsi |  4 ++++
> >  .../boot/dts/marvell/armada-3720-turris-mox.dts   |  4 ++++
> >  arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts  |  4 ++++
> >  arch/arm64/boot/dts/marvell/armada-37xx.dtsi      | 15 +++++++++++++--
> >  5 files changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/marvell/armada-3720-db.dts b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
> > index 3e5789f37206..accf014a6a1e 100644
> > --- a/arch/arm64/boot/dts/marvell/armada-3720-db.dts
> > +++ b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
> > @@ -191,6 +191,10 @@
> >  	};
> >  };
> >  
> > +&uartclk {
> > +	status = "okay";
> 
> I found unusual to have to enable the clock at device tree level.
> Usually the clock driver is always loaded and then the clock is really
> enabled or disabled through the clock framework.
> 
> [...]
> 
> > diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > index 9acc5d2b5a00..5bc61c9615f5 100644
> > --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > @@ -132,10 +132,21 @@
> >  				reg = <0x11500 0x40>;
> >  			};
> >  
> > +			uartclk: uartclk@12000 {
> > +				compatible = "marvell,armada-3700-uart-clock";
> > +				reg = <0x12010 0x4>, <0x12210 0x4>;
> > +				clocks = <&tbg 0>, <&tbg 1>, <&tbg 2>,
> > +					<&tbg 3>, <&xtalclk>;
> > +				clock-names = "TBG-A-P", "TBG-B-P", "TBG-A-S",
> > +					"TBG-B-S", "xtal";
> > +				#clock-cells = <1>;
> 
> I think you could remove the following line and thanks to this there
> won't be any change in the dts of the board:
> > +				status = "disabled";

After removing "status" from dtsi and then also from board bts files,
UART is still working fine.

So I will include this change into V6.

Is there anything else?

> > +			};
> > +
> 
> Gregory
> 
> 
> >  			uart0: serial@12000 {
> >  				compatible = "marvell,armada-3700-uart";
> >  				reg = <0x12000 0x18>;
> > -				clocks = <&xtalclk>;
> > +				clocks = <&uartclk 0>;
> >  				interrupts =
> >  				<GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
> >  				<GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>,
> > @@ -147,7 +158,7 @@
> >  			uart1: serial@12200 {
> >  				compatible = "marvell,armada-3700-uart-ext";
> >  				reg = <0x12200 0x30>;
> > -				clocks = <&xtalclk>;
> > +				clocks = <&uartclk 1>;
> >  				interrupts =
> >  				<GIC_SPI 30 IRQ_TYPE_EDGE_RISING>,
> >  				<GIC_SPI 31 IRQ_TYPE_EDGE_RISING>;
> > -- 
> > 2.20.1
> >
> 
> -- 
> Gregory Clement, Bootlin
> Embedded Linux and Kernel engineering
> http://bootlin.com
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-db.dts b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
index 3e5789f37206..accf014a6a1e 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
@@ -191,6 +191,10 @@ 
 	};
 };
 
+&uartclk {
+	status = "okay";
+};
+
 /*
  * Exported on the micro USB connector CON30(V2.0)/CON32(V1.4) through
  * an FTDI (also on CON24(V2.0)/CON26(V1.4)).
diff --git a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi
index 5fc613d24151..d03c7cdfbfb3 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi
@@ -117,6 +117,10 @@ 
 	};
 };
 
+&uartclk {
+	status = "okay";
+};
+
 /* Exported on the micro USB connector J5 through an FTDI */
 &uart0 {
 	pinctrl-names = "default";
diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
index 04da07ae4420..fb85adc8657d 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
@@ -157,6 +157,10 @@ 
 	status = "disabled";
 };
 
+&uartclk {
+	status = "okay";
+};
+
 &uart0 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts b/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts
index 95d46e8d081c..c8217440b8dd 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts
@@ -183,6 +183,10 @@ 
 	phy-names = "usb2-utmi-otg-phy";
 };
 
+&uartclk {
+	status = "okay";
+};
+
 &uart0 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index 9acc5d2b5a00..5bc61c9615f5 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -132,10 +132,21 @@ 
 				reg = <0x11500 0x40>;
 			};
 
+			uartclk: uartclk@12000 {
+				compatible = "marvell,armada-3700-uart-clock";
+				reg = <0x12010 0x4>, <0x12210 0x4>;
+				clocks = <&tbg 0>, <&tbg 1>, <&tbg 2>,
+					<&tbg 3>, <&xtalclk>;
+				clock-names = "TBG-A-P", "TBG-B-P", "TBG-A-S",
+					"TBG-B-S", "xtal";
+				#clock-cells = <1>;
+				status = "disabled";
+			};
+
 			uart0: serial@12000 {
 				compatible = "marvell,armada-3700-uart";
 				reg = <0x12000 0x18>;
-				clocks = <&xtalclk>;
+				clocks = <&uartclk 0>;
 				interrupts =
 				<GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
 				<GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>,
@@ -147,7 +158,7 @@ 
 			uart1: serial@12200 {
 				compatible = "marvell,armada-3700-uart-ext";
 				reg = <0x12200 0x30>;
-				clocks = <&xtalclk>;
+				clocks = <&uartclk 1>;
 				interrupts =
 				<GIC_SPI 30 IRQ_TYPE_EDGE_RISING>,
 				<GIC_SPI 31 IRQ_TYPE_EDGE_RISING>;