LinuxLists.cc - [PATCH 1/6] arm64: dts: ti: k3-am65: Remove UART baud rate selection

2024-03-26 18:55:07

Subject: [PATCH 1/6] arm64: dts: ti: k3-am65: Remove UART baud rate selection

2024-03-26 18:55:27

Subject: [PATCH 4/6] arm64: dts: ti: k3-j721e: Remove UART baud rate selection

As described in the binding document for the "current-speed" property:

"This should only be present in case a driver has no chance to know the
baud rate of the slave device."

This is not the case for the UART used in K3 devices, the current
baud-rate can be calculated from the registers. Having this property
has the effect of actually skipping the baud-rate setup in some drivers
as it assumes it will already be set to this rate, which may not always
be the case.

It seems this property's purpose was mistaken as selecting the desired
baud-rate, which it does not. It would have been wrong to select that
here anyway as DT is not the place for configuration, especially when
there are already more standard ways to set serial baud-rates.

Signed-off-by: Andrew Davis <[email protected]>
---
arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 10 ----------
arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi | 2 --
2 files changed, 12 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
index c7eafbc862f96..0da785be80ff4 100644
--- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
@@ -1337,7 +1337,6 @@ main_uart0: serial@2800000 {
reg = <0x00 0x02800000 0x00 0x100>;
interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 146 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 146 0>;
clock-names = "fclk";
@@ -1349,7 +1348,6 @@ main_uart1: serial@2810000 {
reg = <0x00 0x02810000 0x00 0x100>;
interrupts = <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 278 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 278 0>;
clock-names = "fclk";
@@ -1361,7 +1359,6 @@ main_uart2: serial@2820000 {
reg = <0x00 0x02820000 0x00 0x100>;
interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 279 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 279 0>;
clock-names = "fclk";
@@ -1373,7 +1370,6 @@ main_uart3: serial@2830000 {
reg = <0x00 0x02830000 0x00 0x100>;
interrupts = <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 280 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 280 0>;
clock-names = "fclk";
@@ -1385,7 +1381,6 @@ main_uart4: serial@2840000 {
reg = <0x00 0x02840000 0x00 0x100>;
interrupts = <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 281 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 281 0>;
clock-names = "fclk";
@@ -1397,7 +1392,6 @@ main_uart5: serial@2850000 {
reg = <0x00 0x02850000 0x00 0x100>;
interrupts = <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 282 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 282 0>;
clock-names = "fclk";
@@ -1409,7 +1403,6 @@ main_uart6: serial@2860000 {
reg = <0x00 0x02860000 0x00 0x100>;
interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 283 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 283 0>;
clock-names = "fclk";
@@ -1421,7 +1414,6 @@ main_uart7: serial@2870000 {
reg = <0x00 0x02870000 0x00 0x100>;
interrupts = <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 284 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 284 0>;
clock-names = "fclk";
@@ -1433,7 +1425,6 @@ main_uart8: serial@2880000 {
reg = <0x00 0x02880000 0x00 0x100>;
interrupts = <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 285 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 285 0>;
clock-names = "fclk";
@@ -1445,7 +1436,6 @@ main_uart9: serial@2890000 {
reg = <0x00 0x02890000 0x00 0x100>;
interrupts = <GIC_SPI 249 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 286 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 286 0>;
clock-names = "fclk";
diff --git a/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi
index 4618b697fbc47..8fc768d84b759 100644
--- a/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi
@@ -243,7 +243,6 @@ wkup_uart0: serial@42300000 {
reg = <0x00 0x42300000 0x00 0x100>;
interrupts = <GIC_SPI 897 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 287 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 287 0>;
clock-names = "fclk";
@@ -255,7 +254,6 @@ mcu_uart0: serial@40a00000 {
reg = <0x00 0x40a00000 0x00 0x100>;
interrupts = <GIC_SPI 846 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <96000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 149 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 149 0>;
clock-names = "fclk";
--
2.39.2

2024-03-26 18:55:31

by Andrew Davis

[permalink] [raw]

Subject: [PATCH 5/6] arm64: dts: ti: k3-j721s2: Remove UART baud rate selection

As described in the binding document for the "current-speed" property:

"This should only be present in case a driver has no chance to know the
baud rate of the slave device."

This is not the case for the UART used in K3 devices, the current
baud-rate can be calculated from the registers. Having this property
has the effect of actually skipping the baud-rate setup in some drivers
as it assumes it will already be set to this rate, which may not always
be the case.

It seems this property's purpose was mistaken as selecting the desired
baud-rate, which it does not. It would have been wrong to select that
here anyway as DT is not the place for configuration, especially when
there are already more standard ways to set serial baud-rates.

Signed-off-by: Andrew Davis <[email protected]>
---
arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi | 10 ----------
arch/arm64/boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi | 2 --
2 files changed, 12 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
index b70c8615e3c15..3cb06a7e4117f 100644
--- a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
@@ -459,7 +459,6 @@ main_uart0: serial@2800000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x02800000 0x00 0x200>;
interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 146 3>;
clock-names = "fclk";
power-domains = <&k3_pds 146 TI_SCI_PD_EXCLUSIVE>;
@@ -470,7 +469,6 @@ main_uart1: serial@2810000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x02810000 0x00 0x200>;
interrupts = <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 350 3>;
clock-names = "fclk";
power-domains = <&k3_pds 350 TI_SCI_PD_EXCLUSIVE>;
@@ -481,7 +479,6 @@ main_uart2: serial@2820000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x02820000 0x00 0x200>;
interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 351 3>;
clock-names = "fclk";
power-domains = <&k3_pds 351 TI_SCI_PD_EXCLUSIVE>;
@@ -492,7 +489,6 @@ main_uart3: serial@2830000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x02830000 0x00 0x200>;
interrupts = <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 352 3>;
clock-names = "fclk";
power-domains = <&k3_pds 352 TI_SCI_PD_EXCLUSIVE>;
@@ -503,7 +499,6 @@ main_uart4: serial@2840000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x02840000 0x00 0x200>;
interrupts = <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 353 3>;
clock-names = "fclk";
power-domains = <&k3_pds 353 TI_SCI_PD_EXCLUSIVE>;
@@ -514,7 +509,6 @@ main_uart5: serial@2850000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x02850000 0x00 0x200>;
interrupts = <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 354 3>;
clock-names = "fclk";
power-domains = <&k3_pds 354 TI_SCI_PD_EXCLUSIVE>;
@@ -525,7 +519,6 @@ main_uart6: serial@2860000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x02860000 0x00 0x200>;
interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 355 3>;
clock-names = "fclk";
power-domains = <&k3_pds 355 TI_SCI_PD_EXCLUSIVE>;
@@ -536,7 +529,6 @@ main_uart7: serial@2870000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x02870000 0x00 0x200>;
interrupts = <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 356 3>;
clock-names = "fclk";
power-domains = <&k3_pds 356 TI_SCI_PD_EXCLUSIVE>;
@@ -547,7 +539,6 @@ main_uart8: serial@2880000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x02880000 0x00 0x200>;
interrupts = <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 357 3>;
clock-names = "fclk";
power-domains = <&k3_pds 357 TI_SCI_PD_EXCLUSIVE>;
@@ -558,7 +549,6 @@ main_uart9: serial@2890000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x02890000 0x00 0x200>;
interrupts = <GIC_SPI 249 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 358 3>;
clock-names = "fclk";
power-domains = <&k3_pds 358 TI_SCI_PD_EXCLUSIVE>;
diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi
index eaf7f709440e6..5ccb04c7c4624 100644
--- a/arch/arm64/boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi
@@ -298,7 +298,6 @@ wkup_uart0: serial@42300000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x42300000 0x00 0x200>;
interrupts = <GIC_SPI 897 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 359 3>;
clock-names = "fclk";
power-domains = <&k3_pds 359 TI_SCI_PD_EXCLUSIVE>;
@@ -309,7 +308,6 @@ mcu_uart0: serial@40a00000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x40a00000 0x00 0x200>;
interrupts = <GIC_SPI 846 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 149 3>;
clock-names = "fclk";
power-domains = <&k3_pds 149 TI_SCI_PD_EXCLUSIVE>;
--
2.39.2

2024-03-26 18:56:06

by Andrew Davis

[permalink] [raw]

Subject: [PATCH 2/6] arm64: dts: ti: k3-am64: Remove UART baud rate selection

2024-03-26 18:56:19

by Andrew Davis

[permalink] [raw]

Subject: [PATCH 3/6] arm64: dts: ti: k3-j7200: Remove UART baud rate selection

As described in the binding document for the "current-speed" property:

"This should only be present in case a driver has no chance to know the
baud rate of the slave device."

This is not the case for the UART used in K3 devices, the current
baud-rate can be calculated from the registers. Having this property
has the effect of actually skipping the baud-rate setup in some drivers
as it assumes it will already be set to this rate, which may not always
be the case.

It seems this property's purpose was mistaken as selecting the desired
baud-rate, which it does not. It would have been wrong to select that
here anyway as DT is not the place for configuration, especially when
there are already more standard ways to set serial baud-rates.

Signed-off-by: Andrew Davis <[email protected]>
---
arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 10 ----------
arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 2 --
2 files changed, 12 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
index 657f9cc9f4ea0..9386bf3ef9f68 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
@@ -440,7 +440,6 @@ main_uart0: serial@2800000 {
reg = <0x00 0x02800000 0x00 0x100>;
interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 146 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 146 2>;
clock-names = "fclk";
@@ -452,7 +451,6 @@ main_uart1: serial@2810000 {
reg = <0x00 0x02810000 0x00 0x100>;
interrupts = <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 278 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 278 2>;
clock-names = "fclk";
@@ -464,7 +462,6 @@ main_uart2: serial@2820000 {
reg = <0x00 0x02820000 0x00 0x100>;
interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 279 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 279 2>;
clock-names = "fclk";
@@ -476,7 +473,6 @@ main_uart3: serial@2830000 {
reg = <0x00 0x02830000 0x00 0x100>;
interrupts = <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 280 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 280 2>;
clock-names = "fclk";
@@ -488,7 +484,6 @@ main_uart4: serial@2840000 {
reg = <0x00 0x02840000 0x00 0x100>;
interrupts = <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 281 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 281 2>;
clock-names = "fclk";
@@ -500,7 +495,6 @@ main_uart5: serial@2850000 {
reg = <0x00 0x02850000 0x00 0x100>;
interrupts = <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 282 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 282 2>;
clock-names = "fclk";
@@ -512,7 +506,6 @@ main_uart6: serial@2860000 {
reg = <0x00 0x02860000 0x00 0x100>;
interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 283 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 283 2>;
clock-names = "fclk";
@@ -524,7 +517,6 @@ main_uart7: serial@2870000 {
reg = <0x00 0x02870000 0x00 0x100>;
interrupts = <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 284 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 284 2>;
clock-names = "fclk";
@@ -536,7 +528,6 @@ main_uart8: serial@2880000 {
reg = <0x00 0x02880000 0x00 0x100>;
interrupts = <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 285 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 285 2>;
clock-names = "fclk";
@@ -548,7 +539,6 @@ main_uart9: serial@2890000 {
reg = <0x00 0x02890000 0x00 0x100>;
interrupts = <GIC_SPI 249 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 286 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 286 2>;
clock-names = "fclk";
diff --git a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
index 7cf21c99956e0..c547bc8fb6082 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
@@ -259,7 +259,6 @@ wkup_uart0: serial@42300000 {
reg = <0x00 0x42300000 0x00 0x100>;
interrupts = <GIC_SPI 897 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 287 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 287 2>;
clock-names = "fclk";
@@ -271,7 +270,6 @@ mcu_uart0: serial@40a00000 {
reg = <0x00 0x40a00000 0x00 0x100>;
interrupts = <GIC_SPI 846 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <96000000>;
- current-speed = <115200>;
power-domains = <&k3_pds 149 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 149 2>;
clock-names = "fclk";
--
2.39.2

2024-03-26 19:04:08

by Andrew Davis

[permalink] [raw]

Subject: [PATCH 6/6] arm64: dts: ti: k3-j784s4: Remove UART baud rate selection

As described in the binding document for the "current-speed" property:

"This should only be present in case a driver has no chance to know the
baud rate of the slave device."

This is not the case for the UART used in K3 devices, the current
baud-rate can be calculated from the registers. Having this property
has the effect of actually skipping the baud-rate setup in some drivers
as it assumes it will already be set to this rate, which may not always
be the case.

It seems this property's purpose was mistaken as selecting the desired
baud-rate, which it does not. It would have been wrong to select that
here anyway as DT is not the place for configuration, especially when
there are already more standard ways to set serial baud-rates.

Signed-off-by: Andrew Davis <[email protected]>
---
arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi | 10 ----------
arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi | 2 --
2 files changed, 12 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
index b67c37460a73d..d42f25cacf23d 100644
--- a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
@@ -404,7 +404,6 @@ main_uart0: serial@2800000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x02800000 0x00 0x200>;
interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 146 0>;
clock-names = "fclk";
power-domains = <&k3_pds 146 TI_SCI_PD_EXCLUSIVE>;
@@ -415,7 +414,6 @@ main_uart1: serial@2810000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x02810000 0x00 0x200>;
interrupts = <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 388 0>;
clock-names = "fclk";
power-domains = <&k3_pds 388 TI_SCI_PD_EXCLUSIVE>;
@@ -426,7 +424,6 @@ main_uart2: serial@2820000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x02820000 0x00 0x200>;
interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 389 0>;
clock-names = "fclk";
power-domains = <&k3_pds 389 TI_SCI_PD_EXCLUSIVE>;
@@ -437,7 +434,6 @@ main_uart3: serial@2830000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x02830000 0x00 0x200>;
interrupts = <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 390 0>;
clock-names = "fclk";
power-domains = <&k3_pds 390 TI_SCI_PD_EXCLUSIVE>;
@@ -448,7 +444,6 @@ main_uart4: serial@2840000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x02840000 0x00 0x200>;
interrupts = <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 391 0>;
clock-names = "fclk";
power-domains = <&k3_pds 391 TI_SCI_PD_EXCLUSIVE>;
@@ -459,7 +454,6 @@ main_uart5: serial@2850000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x02850000 0x00 0x200>;
interrupts = <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 392 0>;
clock-names = "fclk";
power-domains = <&k3_pds 392 TI_SCI_PD_EXCLUSIVE>;
@@ -470,7 +464,6 @@ main_uart6: serial@2860000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x02860000 0x00 0x200>;
interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 393 0>;
clock-names = "fclk";
power-domains = <&k3_pds 393 TI_SCI_PD_EXCLUSIVE>;
@@ -481,7 +474,6 @@ main_uart7: serial@2870000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x02870000 0x00 0x200>;
interrupts = <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 394 0>;
clock-names = "fclk";
power-domains = <&k3_pds 394 TI_SCI_PD_EXCLUSIVE>;
@@ -492,7 +484,6 @@ main_uart8: serial@2880000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x02880000 0x00 0x200>;
interrupts = <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 395 0>;
clock-names = "fclk";
power-domains = <&k3_pds 395 TI_SCI_PD_EXCLUSIVE>;
@@ -503,7 +494,6 @@ main_uart9: serial@2890000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x02890000 0x00 0x200>;
interrupts = <GIC_SPI 249 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 396 0>;
clock-names = "fclk";
power-domains = <&k3_pds 396 TI_SCI_PD_EXCLUSIVE>;
diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
index 77a8d99139ec1..b47338e0f4812 100644
--- a/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
@@ -304,7 +304,6 @@ wkup_uart0: serial@42300000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x42300000 0x00 0x200>;
interrupts = <GIC_SPI 897 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 397 0>;
clock-names = "fclk";
power-domains = <&k3_pds 397 TI_SCI_PD_EXCLUSIVE>;
@@ -315,7 +314,6 @@ mcu_uart0: serial@40a00000 {
compatible = "ti,j721e-uart", "ti,am654-uart";
reg = <0x00 0x40a00000 0x00 0x200>;
interrupts = <GIC_SPI 846 IRQ_TYPE_LEVEL_HIGH>;
- current-speed = <115200>;
clocks = <&k3_clks 149 0>;
clock-names = "fclk";
power-domains = <&k3_pds 149 TI_SCI_PD_EXCLUSIVE>;
--
2.39.2

2024-03-26 20:21:28

by Nishanth Menon

[permalink] [raw]

Subject: Re: [PATCH 1/6] arm64: dts: ti: k3-am65: Remove UART baud rate selection

On 13:54-20240326, Andrew Davis wrote:
> As described in the binding document for the "current-speed" property:
>
> "This should only be present in case a driver has no chance to know the
> baud rate of the slave device."
>
> This is not the case for the UART used in K3 devices, the current
> baud-rate can be calculated from the registers. Having this property

I do not understand the explanation above -> how does one do this?
If you are talking of the 8250 divider registers, someone has to program
those - how do you compute the baud rate from registers that aren't
programmed? Note: I am not commenting on the rationale of removing the
property, just trying to understand the assertion above.

> has the effect of actually skipping the baud-rate setup in some drivers
> as it assumes it will already be set to this rate, which may not always
> be the case.
>
> It seems this property's purpose was mistaken as selecting the desired
> baud-rate, which it does not. It would have been wrong to select that
> here anyway as DT is not the place for configuration, especially when
> there are already more standard ways to set serial baud-rates.
>
> Signed-off-by: Andrew Davis <[email protected]>
> ---
> arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 1 -
> arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi | 1 -
> arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi | 1 -
> 3 files changed, 3 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> index ff857117d7193..670557c89f756 100644
> --- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> @@ -89,7 +89,6 @@ main_uart0: serial@2800000 {
> reg = <0x00 0x02800000 0x00 0x100>;
> interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>;
> clock-frequency = <48000000>;
> - current-speed = <115200>;
> power-domains = <&k3_pds 146 TI_SCI_PD_EXCLUSIVE>;
> status = "disabled";
> };
> diff --git a/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi b/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi
> index 6ff3ccc39fb44..4f808e5089755 100644
> --- a/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi
> @@ -43,7 +43,6 @@ mcu_uart0: serial@40a00000 {
> reg = <0x00 0x40a00000 0x00 0x100>;
> interrupts = <GIC_SPI 565 IRQ_TYPE_LEVEL_HIGH>;
> clock-frequency = <96000000>;
> - current-speed = <115200>;
> power-domains = <&k3_pds 149 TI_SCI_PD_EXCLUSIVE>;
> status = "disabled";
> };
> diff --git a/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
> index 37527890ddeaf..eee072e44a42f 100644
> --- a/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
> @@ -59,7 +59,6 @@ wkup_uart0: serial@42300000 {
> reg = <0x42300000 0x100>;
> interrupts = <GIC_SPI 697 IRQ_TYPE_LEVEL_HIGH>;
> clock-frequency = <48000000>;
> - current-speed = <115200>;
> power-domains = <&k3_pds 150 TI_SCI_PD_EXCLUSIVE>;
> status = "disabled";
> };
> --
> 2.39.2
>

--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D

2024-03-26 20:56:10

by Andrew Davis

[permalink] [raw]

Subject: Re: [PATCH 1/6] arm64: dts: ti: k3-am65: Remove UART baud rate selection

On 3/26/24 3:21 PM, Nishanth Menon wrote:
> On 13:54-20240326, Andrew Davis wrote:
>> As described in the binding document for the "current-speed" property:
>>
>> "This should only be present in case a driver has no chance to know the
>> baud rate of the slave device."
>>
>> This is not the case for the UART used in K3 devices, the current
>> baud-rate can be calculated from the registers. Having this property
>
> I do not understand the explanation above -> how does one do this?
> If you are talking of the 8250 divider registers, someone has to program
> those - how do you compute the baud rate from registers that aren't
> programmed? Note: I am not commenting on the rationale of removing the
> property, just trying to understand the assertion above.
>

This divider register, when "unprogrammed", has a default value. When one
wants to use a TTY/serial device they pick the baud-rate(stty, termios, etc.),
that then programs the register with the value they selected.

As stated below, this property does not program that register, it only
tells the driver what it should have already been set to. Some drivers
need this as they do not have a way to know what the divider is actually
set to.

Setting this in our case is wrong for two reasons:
1) Our driver can and does just check the divider itself.
2) We do not setup most of these UARTs before the kernel starts, so
if our driver *did* care about this property it would not reflect
the actual divider value (which is unprogrammed at this point).

Andrew

>> has the effect of actually skipping the baud-rate setup in some drivers
>> as it assumes it will already be set to this rate, which may not always
>> be the case.
>>
>> It seems this property's purpose was mistaken as selecting the desired
>> baud-rate, which it does not. It would have been wrong to select that
>> here anyway as DT is not the place for configuration, especially when
>> there are already more standard ways to set serial baud-rates.
>>
>> Signed-off-by: Andrew Davis <[email protected]>
>> ---
>> arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 1 -
>> arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi | 1 -
>> arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi | 1 -
>> 3 files changed, 3 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
>> index ff857117d7193..670557c89f756 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
>> @@ -89,7 +89,6 @@ main_uart0: serial@2800000 {
>> reg = <0x00 0x02800000 0x00 0x100>;
>> interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>;
>> clock-frequency = <48000000>;
>> - current-speed = <115200>;
>> power-domains = <&k3_pds 146 TI_SCI_PD_EXCLUSIVE>;
>> status = "disabled";
>> };
>> diff --git a/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi b/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi
>> index 6ff3ccc39fb44..4f808e5089755 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi
>> @@ -43,7 +43,6 @@ mcu_uart0: serial@40a00000 {
>> reg = <0x00 0x40a00000 0x00 0x100>;
>> interrupts = <GIC_SPI 565 IRQ_TYPE_LEVEL_HIGH>;
>> clock-frequency = <96000000>;
>> - current-speed = <115200>;
>> power-domains = <&k3_pds 149 TI_SCI_PD_EXCLUSIVE>;
>> status = "disabled";
>> };
>> diff --git a/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
>> index 37527890ddeaf..eee072e44a42f 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
>> @@ -59,7 +59,6 @@ wkup_uart0: serial@42300000 {
>> reg = <0x42300000 0x100>;
>> interrupts = <GIC_SPI 697 IRQ_TYPE_LEVEL_HIGH>;
>> clock-frequency = <48000000>;
>> - current-speed = <115200>;
>> power-domains = <&k3_pds 150 TI_SCI_PD_EXCLUSIVE>;
>> status = "disabled";
>> };
>> --
>> 2.39.2
>>
>

2024-04-09 19:41:52

by Nishanth Menon

[permalink] [raw]

Subject: Re: [PATCH 1/6] arm64: dts: ti: k3-am65: Remove UART baud rate selection

Hi Andrew Davis,

On Tue, 26 Mar 2024 13:54:36 -0500, Andrew Davis wrote:
> As described in the binding document for the "current-speed" property:
>
> "This should only be present in case a driver has no chance to know the
> baud rate of the slave device."
>
> This is not the case for the UART used in K3 devices, the current
> baud-rate can be calculated from the registers. Having this property
> has the effect of actually skipping the baud-rate setup in some drivers
> as it assumes it will already be set to this rate, which may not always
> be the case.
>
> [...]

I have applied the following to branch ti-k3-dts-next on [1].
Thank you!

[1/6] arm64: dts: ti: k3-am65: Remove UART baud rate selection
commit: 9a1cedcdb98387bfc5d4fb9f1a1ada9121d32757
[2/6] arm64: dts: ti: k3-am64: Remove UART baud rate selection
commit: 78331a044106f7022915058376ac56ec5d048124
[3/6] arm64: dts: ti: k3-j7200: Remove UART baud rate selection
commit: 69dfa876d4b03f7bc043b196c472fca5d339722a
[4/6] arm64: dts: ti: k3-j721e: Remove UART baud rate selection
commit: 14e556b08380b20833c5e2f6a5826d0669e0e0fa
[5/6] arm64: dts: ti: k3-j721s2: Remove UART baud rate selection
commit: cfcfcd47c9f049be4ebcee90eafaf78b03cdf05f
[6/6] arm64: dts: ti: k3-j784s4: Remove UART baud rate selection
commit: bd305a20fd338690fdd8ab8c4a50ab9d14066dfe

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent up the chain during
the next merge window (or sooner if it is a relevant bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/ti/linux.git
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D