* [PATCH 2/6] arm64: dts: ti: k3-am64: Remove UART baud rate selection
From: Andrew Davis @ 2024-03-26 18:54 UTC (permalink / raw)
To: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-kernel, devicetree, linux-kernel, Andrew Davis
In-Reply-To: <20240326185441.29656-1-afd@ti.com>
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 <afd@ti.com>
---
arch/arm64/boot/dts/ti/k3-am642-evm.dts | 1 -
arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts | 2 --
arch/arm64/boot/dts/ti/k3-am642-sk.dts | 1 -
3 files changed, 4 deletions(-)
diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm.dts b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
index 53fe1d065ddbb..e20e4ffd0f1fa 100644
--- a/arch/arm64/boot/dts/ti/k3-am642-evm.dts
+++ b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
@@ -473,7 +473,6 @@ &main_uart0 {
status = "okay";
pinctrl-names = "default";
pinctrl-0 = <&main_uart0_pins_default>;
- current-speed = <115200>;
};
/* main_uart1 is reserved for firmware usage */
diff --git a/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts b/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts
index 8237b8c815b84..dc774f9c75417 100644
--- a/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts
@@ -275,7 +275,6 @@ &main_uart0 {
status = "okay";
pinctrl-names = "default";
pinctrl-0 = <&main_uart0_pins_default>;
- current-speed = <115200>;
};
&main_uart1 {
@@ -283,7 +282,6 @@ &main_uart1 {
pinctrl-names = "default";
pinctrl-0 = <&main_uart1_pins_default>;
uart-has-rtscts;
- current-speed = <115200>;
};
&sdhci1 {
diff --git a/arch/arm64/boot/dts/ti/k3-am642-sk.dts b/arch/arm64/boot/dts/ti/k3-am642-sk.dts
index 67cd41bf806ea..5b028b3a3192f 100644
--- a/arch/arm64/boot/dts/ti/k3-am642-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am642-sk.dts
@@ -381,7 +381,6 @@ &main_uart0 {
status = "okay";
pinctrl-names = "default";
pinctrl-0 = <&main_uart0_pins_default>;
- current-speed = <115200>;
};
&main_uart1 {
--
2.39.2
^ permalink raw reply related
* [PATCH 5/6] arm64: dts: ti: k3-j721s2: Remove UART baud rate selection
From: Andrew Davis @ 2024-03-26 18:54 UTC (permalink / raw)
To: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-kernel, devicetree, linux-kernel, Andrew Davis
In-Reply-To: <20240326185441.29656-1-afd@ti.com>
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 <afd@ti.com>
---
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
^ permalink raw reply related
* [PATCH 4/6] arm64: dts: ti: k3-j721e: Remove UART baud rate selection
From: Andrew Davis @ 2024-03-26 18:54 UTC (permalink / raw)
To: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-kernel, devicetree, linux-kernel, Andrew Davis
In-Reply-To: <20240326185441.29656-1-afd@ti.com>
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 <afd@ti.com>
---
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
^ permalink raw reply related
* [PATCH 6/6] arm64: dts: ti: k3-j784s4: Remove UART baud rate selection
From: Andrew Davis @ 2024-03-26 18:54 UTC (permalink / raw)
To: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-kernel, devicetree, linux-kernel, Andrew Davis
In-Reply-To: <20240326185441.29656-1-afd@ti.com>
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 <afd@ti.com>
---
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
^ permalink raw reply related
* [PATCH 1/6] arm64: dts: ti: k3-am65: Remove UART baud rate selection
From: Andrew Davis @ 2024-03-26 18:54 UTC (permalink / raw)
To: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-kernel, devicetree, linux-kernel, Andrew Davis
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 <afd@ti.com>
---
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
^ permalink raw reply related
* Re: [PATCH v1 1/8] dt-bindings: clock: add Loongson-2K expand clock index
From: Conor Dooley @ 2024-03-26 18:54 UTC (permalink / raw)
To: Binbin Zhou
Cc: Binbin Zhou, Huacai Chen, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Yinbo Zhu,
Huacai Chen, loongson-kernel, linux-clk, devicetree, Xuerui Wang,
loongarch
In-Reply-To: <0cfd237c2c7c1e4f89b6e5f7af0a64d306de1c8f.1710926402.git.zhoubinbin@loongson.cn>
[-- Attachment #1: Type: text/plain, Size: 268 bytes --]
On Tue, Mar 26, 2024 at 05:01:00PM +0800, Binbin Zhou wrote:
> -#define LOONGSON2_CLK_END 18
> +#define LOONGSON2_CLK_END 35
Please just delete this. If you can change it, it is not a binding.
Just define it in the driver if it is needed there.
Thanks,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v2 4/4] dt-bindings: rtc: nxp,lpc1788-rtc: convert to dtschema
From: Rob Herring @ 2024-03-26 18:47 UTC (permalink / raw)
To: Javier Carrasco
Cc: Conor Dooley, Baruch Siach, Krzysztof Kozlowski, linux-arm-kernel,
devicetree, linux-kernel, linux-rtc, Alexandre Belloni
In-Reply-To: <20240325-rtc-yaml-v2-4-ff9f68f43dbc@gmail.com>
On Mon, 25 Mar 2024 22:10:11 +0100, Javier Carrasco wrote:
> Convert existing binding to dtschema to support validation.
>
> This is a direct conversion with no additions.
>
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
> .../devicetree/bindings/rtc/nxp,lpc1788-rtc.txt | 21 --------
> .../devicetree/bindings/rtc/nxp,lpc1788-rtc.yaml | 60 ++++++++++++++++++++++
> 2 files changed, 60 insertions(+), 21 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 3/4] dt-bindings: rtc: digicolor-rtc: move to trivial-rtc
From: Rob Herring @ 2024-03-26 18:47 UTC (permalink / raw)
To: Javier Carrasco
Cc: linux-arm-kernel, Conor Dooley, devicetree, Baruch Siach,
linux-kernel, Krzysztof Kozlowski, Alexandre Belloni, linux-rtc
In-Reply-To: <20240325-rtc-yaml-v2-3-ff9f68f43dbc@gmail.com>
On Mon, 25 Mar 2024 22:10:10 +0100, Javier Carrasco wrote:
> Convert existing binding to dtschema to support validation.
>
> This device meets the requirements to be moved to trivial-rtc
> (compatible, reg and a single interrupt).
>
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
> Documentation/devicetree/bindings/rtc/digicolor-rtc.txt | 17 -----------------
> Documentation/devicetree/bindings/rtc/trivial-rtc.yaml | 2 ++
> 2 files changed, 2 insertions(+), 17 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 2/4] dt-bindings: rtc: alphascale,asm9260-rtc: convert to dtschema
From: Rob Herring @ 2024-03-26 18:47 UTC (permalink / raw)
To: Javier Carrasco
Cc: linux-rtc, devicetree, Baruch Siach, Conor Dooley,
Krzysztof Kozlowski, linux-kernel, linux-arm-kernel,
Alexandre Belloni
In-Reply-To: <20240325-rtc-yaml-v2-2-ff9f68f43dbc@gmail.com>
On Mon, 25 Mar 2024 22:10:09 +0100, Javier Carrasco wrote:
> Convert existing binding to dtschema to support validation.
>
> This is a direct conversion with no additions.
>
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
> .../bindings/rtc/alphascale,asm9260-rtc.txt | 19 --------
> .../bindings/rtc/alphascale,asm9260-rtc.yaml | 52 ++++++++++++++++++++++
> 2 files changed, 52 insertions(+), 19 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: rtc: armada-380-rtc: convert to dtschema
From: Rob Herring @ 2024-03-26 18:47 UTC (permalink / raw)
To: Javier Carrasco
Cc: linux-arm-kernel, linux-kernel, Alexandre Belloni,
Krzysztof Kozlowski, devicetree, linux-rtc, Conor Dooley,
Baruch Siach
In-Reply-To: <20240325-rtc-yaml-v2-1-ff9f68f43dbc@gmail.com>
On Mon, 25 Mar 2024 22:10:08 +0100, Javier Carrasco wrote:
> Convert existing binding to dtschema to support validation.
>
> This is a direct conversion with no additions.
>
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
> .../devicetree/bindings/rtc/armada-380-rtc.txt | 24 ----------
> .../bindings/rtc/marvell,armada-380-rtc.yaml | 51 ++++++++++++++++++++++
> 2 files changed, 51 insertions(+), 24 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 04/25] clk: meson: a1: add the audio clock controller driver
From: Jan Dakinevich @ 2024-03-26 18:44 UTC (permalink / raw)
To: Jerome Brunet
Cc: Neil Armstrong, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Kevin Hilman,
Martin Blumenstingl, Liam Girdwood, Mark Brown, Linus Walleij,
Jaroslav Kysela, Takashi Iwai, linux-amlogic, linux-clk,
devicetree, linux-kernel, linux-arm-kernel, alsa-devel,
linux-sound, linux-gpio, kernel
In-Reply-To: <1jy1a580g0.fsf@starbuckisacylon.baylibre.com>
On 3/26/24 18:26, Jerome Brunet wrote:
>
> On Sat 23 Mar 2024 at 21:02, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
>
>> Jerome, I have reworked my driver reusing axg-audio code as most as I
>> could and now I have one more question. Lets see on this definition from
>> axg-audio:
>>
>> #define AUD_MST_MUX(_name, _reg, _flag) \
>> AUD_MUX(_name##_sel, _reg, 0x7, 24, _flag, \
>> mst_mux_parent_data, 0)
>>
>> #define AUD_MST_MCLK_MUX(_name, _reg) \
>> AUD_MST_MUX(_name, _reg, CLK_MUX_ROUND_CLOSEST)
>>
>> CLK_SET_RATE_PARENT is not set here. But why? It means, that topmost pll
>> clock will not be reconfigured at runtime to satisfy the rate that was
>> requested from axg-tdm.
>>
>
> Yes, that is by design. It is another area where mainline audio differs
> greatly from AML vendor code. The PLLs are expected be to fixed and the
> audio master clock will reparent to the most adequate PLL source
> depending on the use case.
>
> This is how we manage to satisfy all audio interfaces with a very
> limited number of PLLs
>
> On AXG/G12 there is at most 6 concurrent interfaces (3 FRDDR/TODDR) - 8
> on sm1 - and we can satisfy on that with 3 PLLs. That would not be
> possible if interfaces were having their way with the PLLs, reseting it
> everytime a stream is started.
> > The PLL rate should be carefully chosen so it can be derived easily. On
> AXG/G12/SM1 that is:
> * one PLL per rate family, to maximize clock precision
> * x24 x32: to handle different sample sizes
> * x2 until we reach the PLL limits to allow higher rates such as 384kHz
> or even higher
>
Thank you. Now it has become much clearer.
> If you have less PLLs on A1, you'll have to make compromises, like a less
> precise clock to support multiple family with one PLL.
> This is why the PLLs are set for each platform in DT because that choice
> may depend on the platform use case.
>
Unfortunately, on A1 we have only one PLL.
Yes, for us it would be better to have hifi_pll with predefined rate.
For instance it will allow to avoid that ugly workaround in PDM (sysrate
property, etc).
But what whould be preferred for upstream? I can imagine a scenario
where samples with different rate should be played, PDM attached to
fclk_divN and there are no conflicts with TDM. In this case
reconfiguration of hifi_pll on demand could better satisfy somebody's
requirements.
>>
>> On 3/19/24 11:30, Jerome Brunet wrote:
>>>
>>> On Tue 19 Mar 2024 at 04:47, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
>>>
>>>> Let's start from the end:
>>>>
>>>>> No - Looks to me you just have two clock controllers you are trying
>>>> force into one.
>>>>
>>>>> Again, this shows 2 devices. The one related to your 'map0' should
>>>> request AUD2_CLKID_AUDIOTOP as input and enable it right away.
>>>>
>>>> Most of fishy workarounds that you commented is caused the fact the mmio
>>>> of this clock controller is divided into two parts. Compare it with
>>>> axg-audio driver, things that was part of contigous memory region (like
>>>> pdm) here are moved to second region. Is this enough to make a guess
>>>> that these are two devices?
>>>
>>> I see obsolutely no reason to think it is a single device nor to add all the quirks
>>> you have the way you did. So yes, in that case, 2 zones, 2 devices.
>>>
>>>>
>>>> Concerning AUD2_CLKID_AUDIOTOP clock, as it turned out, it must be
>>>> enabled before enabling of clocks from second region too. That is
>>>> AUD2_CLKID_AUDIOTOP clock feeds both parts of this clock controller.
>>>>
>>>
>>> Yes. I understood the first time around and already commented on that.
>>>
>>>>
>>>> On 3/15/24 12:20, Jerome Brunet wrote:
>>>>>
>>>>> On Fri 15 Mar 2024 at 02:21, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
>>>>>
>>>>>> This controller provides clocks and reset functionality for audio
>>>>>> peripherals on Amlogic A1 SoC family.
>>>>>>
>>>>>> The driver is almost identical to 'axg-audio', however it would be better
>>>>>> to keep it separate due to following reasons:
>>>>>>
>>>>>> - significant amount of bits has another definition. I will bring there
>>>>>> a mess of new defines with A1_ suffixes.
>>>>>>
>>>>>> - registers of this controller are located in two separate regions. It
>>>>>> will give a lot of complications for 'axg-audio' to support this.
>>>>>>
>>>>>> Signed-off-by: Jan Dakinevich <jan.dakinevich@salutedevices.com>
>>>>>> ---
>>>>>> drivers/clk/meson/Kconfig | 13 +
>>>>>> drivers/clk/meson/Makefile | 1 +
>>>>>> drivers/clk/meson/a1-audio.c | 556 +++++++++++++++++++++++++++++++++++
>>>>>> drivers/clk/meson/a1-audio.h | 58 ++++
>>>>>> 4 files changed, 628 insertions(+)
>>>>>> create mode 100644 drivers/clk/meson/a1-audio.c
>>>>>> create mode 100644 drivers/clk/meson/a1-audio.h
>>>>>>
>>>>>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>>>>>> index d6a2fa5f7e88..80c4a18c83d2 100644
>>>>>> --- a/drivers/clk/meson/Kconfig
>>>>>> +++ b/drivers/clk/meson/Kconfig
>>>>>> @@ -133,6 +133,19 @@ config COMMON_CLK_A1_PERIPHERALS
>>>>>> device, A1 SoC Family. Say Y if you want A1 Peripherals clock
>>>>>> controller to work.
>>>>>>
>>>>>> +config COMMON_CLK_A1_AUDIO
>>>>>> + tristate "Amlogic A1 SoC Audio clock controller support"
>>>>>> + depends on ARM64
>>>>>> + select COMMON_CLK_MESON_REGMAP
>>>>>> + select COMMON_CLK_MESON_CLKC_UTILS
>>>>>> + select COMMON_CLK_MESON_PHASE
>>>>>> + select COMMON_CLK_MESON_SCLK_DIV
>>>>>> + select COMMON_CLK_MESON_AUDIO_RSTC
>>>>>> + help
>>>>>> + Support for the Audio clock controller on Amlogic A113L based
>>>>>> + device, A1 SoC Family. Say Y if you want A1 Audio clock controller
>>>>>> + to work.
>>>>>> +
>>>>>> config COMMON_CLK_G12A
>>>>>> tristate "G12 and SM1 SoC clock controllers support"
>>>>>> depends on ARM64
>>>>>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>>>>>> index 88d94921a4dc..4968fc7ad555 100644
>>>>>> --- a/drivers/clk/meson/Makefile
>>>>>> +++ b/drivers/clk/meson/Makefile
>>>>>> @@ -20,6 +20,7 @@ obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
>>>>>> obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
>>>>>> obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
>>>>>> obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o
>>>>>> +obj-$(CONFIG_COMMON_CLK_A1_AUDIO) += a1-audio.o
>>>>>> obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
>>>>>> obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
>>>>>> obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o meson8-ddr.o
>>>>>> diff --git a/drivers/clk/meson/a1-audio.c b/drivers/clk/meson/a1-audio.c
>>>>>> new file mode 100644
>>>>>> index 000000000000..6039116c93ba
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/clk/meson/a1-audio.c
>>>>>> @@ -0,0 +1,556 @@
>>>>>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>>>>>> +/*
>>>>>> + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
>>>>>> + *
>>>>>> + * Author: Jan Dakinevich <jan.dakinevich@salutedevices.com>
>>>>>> + */
>>>>>> +
>>>>>> +#include <linux/clk.h>
>>>>>> +#include <linux/clk-provider.h>
>>>>>> +#include <linux/init.h>
>>>>>> +#include <linux/of_device.h>
>>>>>> +#include <linux/module.h>
>>>>>> +#include <linux/platform_device.h>
>>>>>> +#include <linux/regmap.h>
>>>>>> +#include <linux/reset.h>
>>>>>> +#include <linux/reset-controller.h>
>>>>>> +#include <linux/slab.h>
>>>>>> +
>>>>>> +#include "meson-clkc-utils.h"
>>>>>> +#include "meson-audio-rstc.h"
>>>>>> +#include "clk-regmap.h"
>>>>>> +#include "clk-phase.h"
>>>>>> +#include "sclk-div.h"
>>>>>> +#include "a1-audio.h"
>>>>>> +
>>>>>> +#define AUDIO_PDATA(_name) \
>>>>>> + ((const struct clk_parent_data[]) { { .hw = &(_name).hw } })
>>>>>
>>>>> Not a fan - yet another level of macro.
>>>>>
>>>>>> +
>>>>>> +#define AUDIO_MUX(_name, _reg, _mask, _shift, _pdata) \
>>>>>> +static struct clk_regmap _name = { \
>>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>>> + .data = &(struct clk_regmap_mux_data){ \
>>>>>> + .offset = AUDIO_REG_OFFSET(_reg), \
>>>>>> + .mask = (_mask), \
>>>>>> + .shift = (_shift), \
>>>>>> + }, \
>>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>>> + .name = #_name, \
>>>>>> + .ops = &clk_regmap_mux_ops, \
>>>>>> + .parent_data = (_pdata), \
>>>>>> + .num_parents = ARRAY_SIZE(_pdata), \
>>>>>> + .flags = CLK_SET_RATE_PARENT, \
>>>>>> + }, \
>>>>>> +}
>>>>>> +
>>>>>> +#define AUDIO_DIV(_name, _reg, _shift, _width, _pdata) \
>>>>>> +static struct clk_regmap _name = { \
>>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>>> + .data = &(struct clk_regmap_div_data){ \
>>>>>> + .offset = AUDIO_REG_OFFSET(_reg), \
>>>>>> + .shift = (_shift), \
>>>>>> + .width = (_width), \
>>>>>> + }, \
>>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>>> + .name = #_name, \
>>>>>> + .ops = &clk_regmap_divider_ops, \
>>>>>> + .parent_data = (_pdata), \
>>>>>> + .num_parents = 1, \
>>>>>> + .flags = CLK_SET_RATE_PARENT, \
>>>>>> + }, \
>>>>>> +}
>>>>>> +
>>>>>> +#define AUDIO_GATE(_name, _reg, _bit, _pdata) \
>>>>>> +static struct clk_regmap _name = { \
>>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>>> + .data = &(struct clk_regmap_gate_data){ \
>>>>>> + .offset = AUDIO_REG_OFFSET(_reg), \
>>>>>> + .bit_idx = (_bit), \
>>>>>> + }, \
>>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>>> + .name = #_name, \
>>>>>> + .ops = &clk_regmap_gate_ops, \
>>>>>> + .parent_data = (_pdata), \
>>>>>> + .num_parents = 1, \
>>>>>> + .flags = CLK_SET_RATE_PARENT, \
>>>>>> + }, \
>>>>>> +}
>>>>>> +
>>>>>> +#define AUDIO_SCLK_DIV(_name, _reg, _div_shift, _div_width, \
>>>>>> + _hi_shift, _hi_width, _pdata, _set_rate_parent) \
>>>>>> +static struct clk_regmap _name = { \
>>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>>> + .data = &(struct meson_sclk_div_data) { \
>>>>>> + .div = { \
>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>> + .shift = (_div_shift), \
>>>>>> + .width = (_div_width), \
>>>>>> + }, \
>>>>>> + .hi = { \
>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>> + .shift = (_hi_shift), \
>>>>>> + .width = (_hi_width), \
>>>>>> + }, \
>>>>>> + }, \
>>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>>> + .name = #_name, \
>>>>>> + .ops = &meson_sclk_div_ops, \
>>>>>> + .parent_data = (_pdata), \
>>>>>> + .num_parents = 1, \
>>>>>> + .flags = (_set_rate_parent) ? CLK_SET_RATE_PARENT : 0, \
>>>>>
>>>>> Does not help readeability. Just pass the flag as axg-audio does.
>>>>>
>>>>>> + }, \
>>>>>> +}
>>>>>> +
>>>>>> +#define AUDIO_TRIPHASE(_name, _reg, _width, _shift0, _shift1, _shift2, \
>>>>>> + _pdata) \
>>>>>> +static struct clk_regmap _name = { \
>>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>>> + .data = &(struct meson_clk_triphase_data) { \
>>>>>> + .ph0 = { \
>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>> + .shift = (_shift0), \
>>>>>> + .width = (_width), \
>>>>>> + }, \
>>>>>> + .ph1 = { \
>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>> + .shift = (_shift1), \
>>>>>> + .width = (_width), \
>>>>>> + }, \
>>>>>> + .ph2 = { \
>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>> + .shift = (_shift2), \
>>>>>> + .width = (_width), \
>>>>>> + }, \
>>>>>> + }, \
>>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>>> + .name = #_name, \
>>>>>> + .ops = &meson_clk_triphase_ops, \
>>>>>> + .parent_data = (_pdata), \
>>>>>> + .num_parents = 1, \
>>>>>> + .flags = CLK_SET_RATE_PARENT | CLK_DUTY_CYCLE_PARENT, \
>>>>>> + }, \
>>>>>> +}
>>>>>> +
>>>>>> +#define AUDIO_SCLK_WS(_name, _reg, _width, _shift_ph, _shift_ws, \
>>>>>> + _pdata) \
>>>>>> +static struct clk_regmap _name = { \
>>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>>> + .data = &(struct meson_sclk_ws_inv_data) { \
>>>>>> + .ph = { \
>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>> + .shift = (_shift_ph), \
>>>>>> + .width = (_width), \
>>>>>> + }, \
>>>>>> + .ws = { \
>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>> + .shift = (_shift_ws), \
>>>>>> + .width = (_width), \
>>>>>> + }, \
>>>>>> + }, \
>>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>>> + .name = #_name, \
>>>>>> + .ops = &meson_sclk_ws_inv_ops, \
>>>>>> + .parent_data = (_pdata), \
>>>>>> + .num_parents = 1, \
>>>>>> + .flags = CLK_SET_RATE_PARENT | CLK_DUTY_CYCLE_PARENT, \
>>>>>> + }, \
>>>>>> +}
>>>>>
>>>>> All the above does essentially the same things as the macro of
>>>>> axg-audio, to some minor differences. Yet it is another set to maintain.
>>>>>
>>>>
>>>> Except one thing... Here I keep memory identifier to which this clock
>>>> belongs:
>>>>
>>>> .map = AUDIO_REG_MAP(_reg),
>>>>
>>>> It is workaround, but ->map the only common field in clk_regmap that
>>>> could be used for this purpose.
>>>>
>>>>
>>>>> I'd much prefer if you put the axg-audio macro in a header a re-used
>>>>> those. There would a single set to maintain. You may then specialize the
>>>>> included in the driver C file, to avoid redundant parameters
>>>>>
>>>>> Rework axg-audio to use clk_parent_data if you must, but not in the same
>>>>> series please.
>>>>>
>>>>>> +
>>>>>> +static const struct clk_parent_data a1_pclk_pdata[] = {
>>>>>> + { .fw_name = "pclk", },
>>>>>> +};
>>>>>> +
>>>>>> +AUDIO_GATE(audio_ddr_arb, AUDIO_CLK_GATE_EN0, 0, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_tdmin_a, AUDIO_CLK_GATE_EN0, 1, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_tdmin_b, AUDIO_CLK_GATE_EN0, 2, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_tdmin_lb, AUDIO_CLK_GATE_EN0, 3, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_loopback, AUDIO_CLK_GATE_EN0, 4, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_tdmout_a, AUDIO_CLK_GATE_EN0, 5, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_tdmout_b, AUDIO_CLK_GATE_EN0, 6, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_frddr_a, AUDIO_CLK_GATE_EN0, 7, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_frddr_b, AUDIO_CLK_GATE_EN0, 8, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_toddr_a, AUDIO_CLK_GATE_EN0, 9, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_toddr_b, AUDIO_CLK_GATE_EN0, 10, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_spdifin, AUDIO_CLK_GATE_EN0, 11, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_resample, AUDIO_CLK_GATE_EN0, 12, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_eqdrc, AUDIO_CLK_GATE_EN0, 13, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_audiolocker, AUDIO_CLK_GATE_EN0, 14, a1_pclk_pdata);
>>>>> This is what I mean by redundant parameter ^
>>>>>
>>>>
>>>> Yep. I could define something like AUDIO_PCLK_GATE().
>>>>
>>>>>> +
>>>>>> +AUDIO_GATE(audio2_ddr_arb, AUDIO2_CLK_GATE_EN0, 0, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio2_pdm, AUDIO2_CLK_GATE_EN0, 1, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio2_tdmin_vad, AUDIO2_CLK_GATE_EN0, 2, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio2_toddr_vad, AUDIO2_CLK_GATE_EN0, 3, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio2_vad, AUDIO2_CLK_GATE_EN0, 4, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio2_audiotop, AUDIO2_CLK_GATE_EN0, 7, a1_pclk_pdata);
>>>>>> +
>>>>>> +static const struct clk_parent_data a1_mst_pdata[] = {
>>>>>> + { .fw_name = "dds_in" },
>>>>>> + { .fw_name = "fclk_div2" },
>>>>>> + { .fw_name = "fclk_div3" },
>>>>>> + { .fw_name = "hifi_pll" },
>>>>>> + { .fw_name = "xtal" },
>>>>>> +};
>>>>>> +
>>>>>> +#define AUDIO_MST_MCLK(_name, _reg) \
>>>>>> + AUDIO_MUX(_name##_mux, (_reg), 0x7, 24, a1_mst_pdata); \
>>>>>> + AUDIO_DIV(_name##_div, (_reg), 0, 16, \
>>>>>> + AUDIO_PDATA(_name##_mux)); \
>>>>>> + AUDIO_GATE(_name, (_reg), 31, AUDIO_PDATA(_name##_div))
>>>>>> +
>>>>>> +AUDIO_MST_MCLK(audio_mst_a_mclk, AUDIO_MCLK_A_CTRL);
>>>>>> +AUDIO_MST_MCLK(audio_mst_b_mclk, AUDIO_MCLK_B_CTRL);
>>>>>> +AUDIO_MST_MCLK(audio_mst_c_mclk, AUDIO_MCLK_C_CTRL);
>>>>>> +AUDIO_MST_MCLK(audio_mst_d_mclk, AUDIO_MCLK_D_CTRL);
>>>>>> +AUDIO_MST_MCLK(audio_spdifin_clk, AUDIO_CLK_SPDIFIN_CTRL);
>>>>>> +AUDIO_MST_MCLK(audio_eqdrc_clk, AUDIO_CLK_EQDRC_CTRL);
>>>>>> +
>>>>>> +AUDIO_MUX(audio_resample_clk_mux, AUDIO_CLK_RESAMPLE_CTRL, 0xf, 24,
>>>>>> + a1_mst_pdata);
>>>>>> +AUDIO_DIV(audio_resample_clk_div, AUDIO_CLK_RESAMPLE_CTRL, 0, 8,
>>>>>> + AUDIO_PDATA(audio_resample_clk_mux));
>>>>>> +AUDIO_GATE(audio_resample_clk, AUDIO_CLK_RESAMPLE_CTRL, 31,
>>>>>> + AUDIO_PDATA(audio_resample_clk_div));
>>>>>> +
>>>>>> +AUDIO_MUX(audio_locker_in_clk_mux, AUDIO_CLK_LOCKER_CTRL, 0xf, 8,
>>>>>> + a1_mst_pdata);
>>>>>> +AUDIO_DIV(audio_locker_in_clk_div, AUDIO_CLK_LOCKER_CTRL, 0, 8,
>>>>>> + AUDIO_PDATA(audio_locker_in_clk_mux));
>>>>>> +AUDIO_GATE(audio_locker_in_clk, AUDIO_CLK_LOCKER_CTRL, 15,
>>>>>> + AUDIO_PDATA(audio_locker_in_clk_div));
>>>>>> +
>>>>>> +AUDIO_MUX(audio_locker_out_clk_mux, AUDIO_CLK_LOCKER_CTRL, 0xf, 24,
>>>>>> + a1_mst_pdata);
>>>>>> +AUDIO_DIV(audio_locker_out_clk_div, AUDIO_CLK_LOCKER_CTRL, 16, 8,
>>>>>> + AUDIO_PDATA(audio_locker_out_clk_mux));
>>>>>> +AUDIO_GATE(audio_locker_out_clk, AUDIO_CLK_LOCKER_CTRL, 31,
>>>>>> + AUDIO_PDATA(audio_locker_out_clk_div));
>>>>>> +
>>>>>> +AUDIO_MST_MCLK(audio2_vad_mclk, AUDIO2_MCLK_VAD_CTRL);
>>>>>> +AUDIO_MST_MCLK(audio2_vad_clk, AUDIO2_CLK_VAD_CTRL);
>>>>>> +AUDIO_MST_MCLK(audio2_pdm_dclk, AUDIO2_CLK_PDMIN_CTRL0);
>>>>>> +AUDIO_MST_MCLK(audio2_pdm_sysclk, AUDIO2_CLK_PDMIN_CTRL1);
>>>>>> +
>>>>>> +#define AUDIO_MST_SCLK(_name, _reg0, _reg1, _pdata) \
>>>>>> + AUDIO_GATE(_name##_pre_en, (_reg0), 31, (_pdata)); \
>>>>>> + AUDIO_SCLK_DIV(_name##_div, (_reg0), 20, 10, 0, 0, \
>>>>>> + AUDIO_PDATA(_name##_pre_en), true); \
>>>>>> + AUDIO_GATE(_name##_post_en, (_reg0), 30, \
>>>>>> + AUDIO_PDATA(_name##_div)); \
>>>>>> + AUDIO_TRIPHASE(_name, (_reg1), 1, 0, 2, 4, \
>>>>>> + AUDIO_PDATA(_name##_post_en))
>>>>>> +
>>>>>
>>>>> Again, I'm not a fan of this many levels of macro. I can live with it
>>>>> but certainly don't want the burden of reviewing and maintaining for
>>>>> clock driver. AXG / G12 and A1 are obviously closely related, so make it common.
>>>>>
>>>>>> +#define AUDIO_MST_LRCLK(_name, _reg0, _reg1, _pdata) \
>>>>>> + AUDIO_SCLK_DIV(_name##_div, (_reg0), 0, 10, 10, 10, \
>>>>>> + (_pdata), false); \
>>>>>> + AUDIO_TRIPHASE(_name, (_reg1), 1, 1, 3, 5, \
>>>>>> + AUDIO_PDATA(_name##_div))
>>>>>> +
>>>>>> +AUDIO_MST_SCLK(audio_mst_a_sclk, AUDIO_MST_A_SCLK_CTRL0, AUDIO_MST_A_SCLK_CTRL1,
>>>>>> + AUDIO_PDATA(audio_mst_a_mclk));
>>>>>> +AUDIO_MST_SCLK(audio_mst_b_sclk, AUDIO_MST_B_SCLK_CTRL0, AUDIO_MST_B_SCLK_CTRL1,
>>>>>> + AUDIO_PDATA(audio_mst_b_mclk));
>>>>>> +AUDIO_MST_SCLK(audio_mst_c_sclk, AUDIO_MST_C_SCLK_CTRL0, AUDIO_MST_C_SCLK_CTRL1,
>>>>>> + AUDIO_PDATA(audio_mst_c_mclk));
>>>>>> +AUDIO_MST_SCLK(audio_mst_d_sclk, AUDIO_MST_D_SCLK_CTRL0, AUDIO_MST_D_SCLK_CTRL1,
>>>>>> + AUDIO_PDATA(audio_mst_d_mclk));
>>>>>> +
>>>>>> +AUDIO_MST_LRCLK(audio_mst_a_lrclk, AUDIO_MST_A_SCLK_CTRL0, AUDIO_MST_A_SCLK_CTRL1,
>>>>>> + AUDIO_PDATA(audio_mst_a_sclk_post_en));
>>>>>> +AUDIO_MST_LRCLK(audio_mst_b_lrclk, AUDIO_MST_B_SCLK_CTRL0, AUDIO_MST_B_SCLK_CTRL1,
>>>>>> + AUDIO_PDATA(audio_mst_b_sclk_post_en));
>>>>>> +AUDIO_MST_LRCLK(audio_mst_c_lrclk, AUDIO_MST_C_SCLK_CTRL0, AUDIO_MST_C_SCLK_CTRL1,
>>>>>> + AUDIO_PDATA(audio_mst_c_sclk_post_en));
>>>>>> +AUDIO_MST_LRCLK(audio_mst_d_lrclk, AUDIO_MST_D_SCLK_CTRL0, AUDIO_MST_D_SCLK_CTRL1,
>>>>>> + AUDIO_PDATA(audio_mst_d_sclk_post_en));
>>>>>> +
>>>>>> +static const struct clk_parent_data a1_mst_sclk_pdata[] = {
>>>>>> + { .hw = &audio_mst_a_sclk.hw },
>>>>>> + { .hw = &audio_mst_b_sclk.hw },
>>>>>> + { .hw = &audio_mst_c_sclk.hw },
>>>>>> + { .hw = &audio_mst_d_sclk.hw },
>>>>>> + { .fw_name = "slv_sclk0" },
>>>>>> + { .fw_name = "slv_sclk1" },
>>>>>> + { .fw_name = "slv_sclk2" },
>>>>>> + { .fw_name = "slv_sclk3" },
>>>>>> + { .fw_name = "slv_sclk4" },
>>>>>> + { .fw_name = "slv_sclk5" },
>>>>>> + { .fw_name = "slv_sclk6" },
>>>>>> + { .fw_name = "slv_sclk7" },
>>>>>> + { .fw_name = "slv_sclk8" },
>>>>>> + { .fw_name = "slv_sclk9" },
>>>>>> +};
>>>>>> +
>>>>>> +static const struct clk_parent_data a1_mst_lrclk_pdata[] = {
>>>>>> + { .hw = &audio_mst_a_lrclk.hw },
>>>>>> + { .hw = &audio_mst_b_lrclk.hw },
>>>>>> + { .hw = &audio_mst_c_lrclk.hw },
>>>>>> + { .hw = &audio_mst_d_lrclk.hw },
>>>>>> + { .fw_name = "slv_lrclk0" },
>>>>>> + { .fw_name = "slv_lrclk1" },
>>>>>> + { .fw_name = "slv_lrclk2" },
>>>>>> + { .fw_name = "slv_lrclk3" },
>>>>>> + { .fw_name = "slv_lrclk4" },
>>>>>> + { .fw_name = "slv_lrclk5" },
>>>>>> + { .fw_name = "slv_lrclk6" },
>>>>>> + { .fw_name = "slv_lrclk7" },
>>>>>> + { .fw_name = "slv_lrclk8" },
>>>>>> + { .fw_name = "slv_lrclk9" },
>>>>>> +};
>>>>>> +
>>>>>> +#define AUDIO_TDM_SCLK(_name, _reg) \
>>>>>> + AUDIO_MUX(_name##_mux, (_reg), 0xf, 24, a1_mst_sclk_pdata); \
>>>>>> + AUDIO_GATE(_name##_pre_en, (_reg), 31, \
>>>>>> + AUDIO_PDATA(_name##_mux)); \
>>>>>> + AUDIO_GATE(_name##_post_en, (_reg), 30, \
>>>>>> + AUDIO_PDATA(_name##_pre_en)); \
>>>>>> + AUDIO_SCLK_WS(_name, (_reg), 1, 29, 28, \
>>>>>> + AUDIO_PDATA(_name##_post_en))
>>>>>> +
>>>>>> +#define AUDIO_TDM_LRCLK(_name, _reg) \
>>>>>> + AUDIO_MUX(_name, (_reg), 0xf, 20, a1_mst_lrclk_pdata)
>>>>>> +
>>>>>> +AUDIO_TDM_SCLK(audio_tdmin_a_sclk, AUDIO_CLK_TDMIN_A_CTRL);
>>>>>> +AUDIO_TDM_SCLK(audio_tdmin_b_sclk, AUDIO_CLK_TDMIN_B_CTRL);
>>>>>> +AUDIO_TDM_SCLK(audio_tdmin_lb_sclk, AUDIO_CLK_TDMIN_LB_CTRL);
>>>>>> +AUDIO_TDM_SCLK(audio_tdmout_a_sclk, AUDIO_CLK_TDMOUT_A_CTRL);
>>>>>> +AUDIO_TDM_SCLK(audio_tdmout_b_sclk, AUDIO_CLK_TDMOUT_B_CTRL);
>>>>>> +
>>>>>> +AUDIO_TDM_LRCLK(audio_tdmin_a_lrclk, AUDIO_CLK_TDMIN_A_CTRL);
>>>>>> +AUDIO_TDM_LRCLK(audio_tdmin_b_lrclk, AUDIO_CLK_TDMIN_B_CTRL);
>>>>>> +AUDIO_TDM_LRCLK(audio_tdmin_lb_lrclk, AUDIO_CLK_TDMIN_LB_CTRL);
>>>>>> +AUDIO_TDM_LRCLK(audio_tdmout_a_lrclk, AUDIO_CLK_TDMOUT_A_CTRL);
>>>>>> +AUDIO_TDM_LRCLK(audio_tdmout_b_lrclk, AUDIO_CLK_TDMOUT_B_CTRL);
>>>>>> +
>>>>>> +static struct clk_hw *a1_audio_hw_clks[] = {
>>>>>> + [AUD_CLKID_DDR_ARB] = &audio_ddr_arb.hw,
>>>>>> + [AUD_CLKID_TDMIN_A] = &audio_tdmin_a.hw,
>>>>>> + [AUD_CLKID_TDMIN_B] = &audio_tdmin_b.hw,
>>>>>> + [AUD_CLKID_TDMIN_LB] = &audio_tdmin_lb.hw,
>>>>>> + [AUD_CLKID_LOOPBACK] = &audio_loopback.hw,
>>>>>> + [AUD_CLKID_TDMOUT_A] = &audio_tdmout_a.hw,
>>>>>> + [AUD_CLKID_TDMOUT_B] = &audio_tdmout_b.hw,
>>>>>> + [AUD_CLKID_FRDDR_A] = &audio_frddr_a.hw,
>>>>>> + [AUD_CLKID_FRDDR_B] = &audio_frddr_b.hw,
>>>>>> + [AUD_CLKID_TODDR_A] = &audio_toddr_a.hw,
>>>>>> + [AUD_CLKID_TODDR_B] = &audio_toddr_b.hw,
>>>>>> + [AUD_CLKID_SPDIFIN] = &audio_spdifin.hw,
>>>>>> + [AUD_CLKID_RESAMPLE] = &audio_resample.hw,
>>>>>> + [AUD_CLKID_EQDRC] = &audio_eqdrc.hw,
>>>>>> + [AUD_CLKID_LOCKER] = &audio_audiolocker.hw,
>>>>>> + [AUD_CLKID_MST_A_MCLK_SEL] = &audio_mst_a_mclk_mux.hw,
>>>>>> + [AUD_CLKID_MST_A_MCLK_DIV] = &audio_mst_a_mclk_div.hw,
>>>>>> + [AUD_CLKID_MST_A_MCLK] = &audio_mst_a_mclk.hw,
>>>>>> + [AUD_CLKID_MST_B_MCLK_SEL] = &audio_mst_b_mclk_mux.hw,
>>>>>> + [AUD_CLKID_MST_B_MCLK_DIV] = &audio_mst_b_mclk_div.hw,
>>>>>> + [AUD_CLKID_MST_B_MCLK] = &audio_mst_b_mclk.hw,
>>>>>> + [AUD_CLKID_MST_C_MCLK_SEL] = &audio_mst_c_mclk_mux.hw,
>>>>>> + [AUD_CLKID_MST_C_MCLK_DIV] = &audio_mst_c_mclk_div.hw,
>>>>>> + [AUD_CLKID_MST_C_MCLK] = &audio_mst_c_mclk.hw,
>>>>>> + [AUD_CLKID_MST_D_MCLK_SEL] = &audio_mst_d_mclk_mux.hw,
>>>>>> + [AUD_CLKID_MST_D_MCLK_DIV] = &audio_mst_d_mclk_div.hw,
>>>>>> + [AUD_CLKID_MST_D_MCLK] = &audio_mst_d_mclk.hw,
>>>>>> + [AUD_CLKID_RESAMPLE_CLK_SEL] = &audio_resample_clk_mux.hw,
>>>>>> + [AUD_CLKID_RESAMPLE_CLK_DIV] = &audio_resample_clk_div.hw,
>>>>>> + [AUD_CLKID_RESAMPLE_CLK] = &audio_resample_clk.hw,
>>>>>> + [AUD_CLKID_LOCKER_IN_CLK_SEL] = &audio_locker_in_clk_mux.hw,
>>>>>> + [AUD_CLKID_LOCKER_IN_CLK_DIV] = &audio_locker_in_clk_div.hw,
>>>>>> + [AUD_CLKID_LOCKER_IN_CLK] = &audio_locker_in_clk.hw,
>>>>>> + [AUD_CLKID_LOCKER_OUT_CLK_SEL] = &audio_locker_out_clk_mux.hw,
>>>>>> + [AUD_CLKID_LOCKER_OUT_CLK_DIV] = &audio_locker_out_clk_div.hw,
>>>>>> + [AUD_CLKID_LOCKER_OUT_CLK] = &audio_locker_out_clk.hw,
>>>>>> + [AUD_CLKID_SPDIFIN_CLK_SEL] = &audio_spdifin_clk_mux.hw,
>>>>>> + [AUD_CLKID_SPDIFIN_CLK_DIV] = &audio_spdifin_clk_div.hw,
>>>>>> + [AUD_CLKID_SPDIFIN_CLK] = &audio_spdifin_clk.hw,
>>>>>> + [AUD_CLKID_EQDRC_CLK_SEL] = &audio_eqdrc_clk_mux.hw,
>>>>>> + [AUD_CLKID_EQDRC_CLK_DIV] = &audio_eqdrc_clk_div.hw,
>>>>>> + [AUD_CLKID_EQDRC_CLK] = &audio_eqdrc_clk.hw,
>>>>>> + [AUD_CLKID_MST_A_SCLK_PRE_EN] = &audio_mst_a_sclk_pre_en.hw,
>>>>>> + [AUD_CLKID_MST_A_SCLK_DIV] = &audio_mst_a_sclk_div.hw,
>>>>>> + [AUD_CLKID_MST_A_SCLK_POST_EN] = &audio_mst_a_sclk_post_en.hw,
>>>>>> + [AUD_CLKID_MST_A_SCLK] = &audio_mst_a_sclk.hw,
>>>>>> + [AUD_CLKID_MST_B_SCLK_PRE_EN] = &audio_mst_b_sclk_pre_en.hw,
>>>>>> + [AUD_CLKID_MST_B_SCLK_DIV] = &audio_mst_b_sclk_div.hw,
>>>>>> + [AUD_CLKID_MST_B_SCLK_POST_EN] = &audio_mst_b_sclk_post_en.hw,
>>>>>> + [AUD_CLKID_MST_B_SCLK] = &audio_mst_b_sclk.hw,
>>>>>> + [AUD_CLKID_MST_C_SCLK_PRE_EN] = &audio_mst_c_sclk_pre_en.hw,
>>>>>> + [AUD_CLKID_MST_C_SCLK_DIV] = &audio_mst_c_sclk_div.hw,
>>>>>> + [AUD_CLKID_MST_C_SCLK_POST_EN] = &audio_mst_c_sclk_post_en.hw,
>>>>>> + [AUD_CLKID_MST_C_SCLK] = &audio_mst_c_sclk.hw,
>>>>>> + [AUD_CLKID_MST_D_SCLK_PRE_EN] = &audio_mst_d_sclk_pre_en.hw,
>>>>>> + [AUD_CLKID_MST_D_SCLK_DIV] = &audio_mst_d_sclk_div.hw,
>>>>>> + [AUD_CLKID_MST_D_SCLK_POST_EN] = &audio_mst_d_sclk_post_en.hw,
>>>>>> + [AUD_CLKID_MST_D_SCLK] = &audio_mst_d_sclk.hw,
>>>>>> + [AUD_CLKID_MST_A_LRCLK_DIV] = &audio_mst_a_lrclk_div.hw,
>>>>>> + [AUD_CLKID_MST_A_LRCLK] = &audio_mst_a_lrclk.hw,
>>>>>> + [AUD_CLKID_MST_B_LRCLK_DIV] = &audio_mst_b_lrclk_div.hw,
>>>>>> + [AUD_CLKID_MST_B_LRCLK] = &audio_mst_b_lrclk.hw,
>>>>>> + [AUD_CLKID_MST_C_LRCLK_DIV] = &audio_mst_c_lrclk_div.hw,
>>>>>> + [AUD_CLKID_MST_C_LRCLK] = &audio_mst_c_lrclk.hw,
>>>>>> + [AUD_CLKID_MST_D_LRCLK_DIV] = &audio_mst_d_lrclk_div.hw,
>>>>>> + [AUD_CLKID_MST_D_LRCLK] = &audio_mst_d_lrclk.hw,
>>>>>> + [AUD_CLKID_TDMIN_A_SCLK_SEL] = &audio_tdmin_a_sclk_mux.hw,
>>>>>> + [AUD_CLKID_TDMIN_A_SCLK_PRE_EN] = &audio_tdmin_a_sclk_pre_en.hw,
>>>>>> + [AUD_CLKID_TDMIN_A_SCLK_POST_EN] = &audio_tdmin_a_sclk_post_en.hw,
>>>>>> + [AUD_CLKID_TDMIN_A_SCLK] = &audio_tdmin_a_sclk.hw,
>>>>>> + [AUD_CLKID_TDMIN_A_LRCLK] = &audio_tdmin_a_lrclk.hw,
>>>>>> + [AUD_CLKID_TDMIN_B_SCLK_SEL] = &audio_tdmin_b_sclk_mux.hw,
>>>>>> + [AUD_CLKID_TDMIN_B_SCLK_PRE_EN] = &audio_tdmin_b_sclk_pre_en.hw,
>>>>>> + [AUD_CLKID_TDMIN_B_SCLK_POST_EN] = &audio_tdmin_b_sclk_post_en.hw,
>>>>>> + [AUD_CLKID_TDMIN_B_SCLK] = &audio_tdmin_b_sclk.hw,
>>>>>> + [AUD_CLKID_TDMIN_B_LRCLK] = &audio_tdmin_b_lrclk.hw,
>>>>>> + [AUD_CLKID_TDMIN_LB_SCLK_SEL] = &audio_tdmin_lb_sclk_mux.hw,
>>>>>> + [AUD_CLKID_TDMIN_LB_SCLK_PRE_EN] = &audio_tdmin_lb_sclk_pre_en.hw,
>>>>>> + [AUD_CLKID_TDMIN_LB_SCLK_POST_EN] = &audio_tdmin_lb_sclk_post_en.hw,
>>>>>> + [AUD_CLKID_TDMIN_LB_SCLK] = &audio_tdmin_lb_sclk.hw,
>>>>>> + [AUD_CLKID_TDMIN_LB_LRCLK] = &audio_tdmin_lb_lrclk.hw,
>>>>>> + [AUD_CLKID_TDMOUT_A_SCLK_SEL] = &audio_tdmout_a_sclk_mux.hw,
>>>>>> + [AUD_CLKID_TDMOUT_A_SCLK_PRE_EN] = &audio_tdmout_a_sclk_pre_en.hw,
>>>>>> + [AUD_CLKID_TDMOUT_A_SCLK_POST_EN] = &audio_tdmout_a_sclk_post_en.hw,
>>>>>> + [AUD_CLKID_TDMOUT_A_SCLK] = &audio_tdmout_a_sclk.hw,
>>>>>> + [AUD_CLKID_TDMOUT_A_LRCLK] = &audio_tdmout_a_lrclk.hw,
>>>>>> + [AUD_CLKID_TDMOUT_B_SCLK_SEL] = &audio_tdmout_b_sclk_mux.hw,
>>>>>> + [AUD_CLKID_TDMOUT_B_SCLK_PRE_EN] = &audio_tdmout_b_sclk_pre_en.hw,
>>>>>> + [AUD_CLKID_TDMOUT_B_SCLK_POST_EN] = &audio_tdmout_b_sclk_post_en.hw,
>>>>>> + [AUD_CLKID_TDMOUT_B_SCLK] = &audio_tdmout_b_sclk.hw,
>>>>>> + [AUD_CLKID_TDMOUT_B_LRCLK] = &audio_tdmout_b_lrclk.hw,
>>>>>> +
>>>>>> + [AUD2_CLKID_DDR_ARB] = &audio2_ddr_arb.hw,
>>>>>> + [AUD2_CLKID_PDM] = &audio2_pdm.hw,
>>>>>> + [AUD2_CLKID_TDMIN_VAD] = &audio2_tdmin_vad.hw,
>>>>>> + [AUD2_CLKID_TODDR_VAD] = &audio2_toddr_vad.hw,
>>>>>> + [AUD2_CLKID_VAD] = &audio2_vad.hw,
>>>>>> + [AUD2_CLKID_AUDIOTOP] = &audio2_audiotop.hw,
>>>>>> + [AUD2_CLKID_VAD_MCLK_SEL] = &audio2_vad_mclk_mux.hw,
>>>>>> + [AUD2_CLKID_VAD_MCLK_DIV] = &audio2_vad_mclk_div.hw,
>>>>>> + [AUD2_CLKID_VAD_MCLK] = &audio2_vad_mclk.hw,
>>>>>> + [AUD2_CLKID_VAD_CLK_SEL] = &audio2_vad_clk_mux.hw,
>>>>>> + [AUD2_CLKID_VAD_CLK_DIV] = &audio2_vad_clk_div.hw,
>>>>>> + [AUD2_CLKID_VAD_CLK] = &audio2_vad_clk.hw,
>>>>>> + [AUD2_CLKID_PDM_DCLK_SEL] = &audio2_pdm_dclk_mux.hw,
>>>>>> + [AUD2_CLKID_PDM_DCLK_DIV] = &audio2_pdm_dclk_div.hw,
>>>>>> + [AUD2_CLKID_PDM_DCLK] = &audio2_pdm_dclk.hw,
>>>>>> + [AUD2_CLKID_PDM_SYSCLK_SEL] = &audio2_pdm_sysclk_mux.hw,
>>>>>> + [AUD2_CLKID_PDM_SYSCLK_DIV] = &audio2_pdm_sysclk_div.hw,
>>>>>> + [AUD2_CLKID_PDM_SYSCLK] = &audio2_pdm_sysclk.hw,
>>>>>> +};
>>>>>> +
>>>>>> +static struct meson_clk_hw_data a1_audio_clks = {
>>>>>> + .hws = a1_audio_hw_clks,
>>>>>> + .num = ARRAY_SIZE(a1_audio_hw_clks),
>>>>>> +};
>>>>>> +
>>>>>> +static struct regmap *a1_audio_map(struct platform_device *pdev,
>>>>>> + unsigned int index)
>>>>>> +{
>>>>>> + char name[32];
>>>>>> + const struct regmap_config cfg = {
>>>>>> + .reg_bits = 32,
>>>>>> + .val_bits = 32,
>>>>>> + .reg_stride = 4,
>>>>>> + .name = name,
>>>>>
>>>>> Not necessary
>>>>>
>>>>
>>>> This implementation uses two regmaps, and this field allow to avoid
>>>> errors like this:
>>>>
>>>> [ 0.145530] debugfs: Directory 'fe050000.audio-clock-controller' with
>>>> parent 'regmap' already present!
>>>>
>>>>>> + };
>>>>>> + void __iomem *base;
>>>>>> +
>>>>>> + base = devm_platform_ioremap_resource(pdev, index);
>>>>>> + if (IS_ERR(base))
>>>>>> + return base;
>>>>>> +
>>>>>> + scnprintf(name, sizeof(name), "%d", index);
>>>>>> + return devm_regmap_init_mmio(&pdev->dev, base, &cfg);
>>>>>> +}
>>>>>
>>>>> That is overengineered. Please keep it simple. Declare the regmap_config
>>>>> as static const global, and do it like axg-audio please.
>>>>>
>>>>
>>>> This only reason why it is not "static const" because I need to set
>>>> unique name for each regmap.
>>>>
>>>>>> +
>>>>>> +static int a1_register_clk(struct platform_device *pdev,
>>>>>> + struct regmap *map0, struct regmap *map1,
>>>>>> + struct clk_hw *hw)
>>>>>> +{
>>>>>> + struct clk_regmap *clk = container_of(hw, struct clk_regmap, hw);
>>>>>> +
>>>>>> + if (!hw)
>>>>>> + return 0;
>>>>>> +
>>>>>> + switch ((unsigned long)clk->map) {
>>>>>> + case AUDIO_RANGE_0:
>>>>>> + clk->map = map0;
>>>>>> + break;
>>>>>> + case AUDIO_RANGE_1:
>>>>>> + clk->map = map1;
>>>>>> + break;
>>>>>
>>>>> ... fishy
>>>>>
>>>>>> + default:
>>>>>> + WARN_ON(1);
>>>>>> + return -EINVAL;
>>>>>> + }
>>>>>> +
>>>>>> + return devm_clk_hw_register(&pdev->dev, hw);
>>>>>> +}
>>>>>> +
>>>>>> +static int a1_audio_clkc_probe(struct platform_device *pdev)
>>>>>> +{
>>>>>> + struct regmap *map0, *map1;
>>>>>> + struct clk *clk;
>>>>>> + unsigned int i;
>>>>>> + int ret;
>>>>>> +
>>>>>> + clk = devm_clk_get_enabled(&pdev->dev, "pclk");
>>>>>> + if (WARN_ON(IS_ERR(clk)))
>>>>>> + return PTR_ERR(clk);
>>>>>> +
>>>>>> + map0 = a1_audio_map(pdev, 0);
>>>>>> + if (IS_ERR(map0))
>>>>>> + return PTR_ERR(map0);
>>>>>> +
>>>>>> + map1 = a1_audio_map(pdev, 1);
>>>>>> + if (IS_ERR(map1))
>>>>>> + return PTR_ERR(map1);
>>>>>
>>>>> No - Looks to me you just have two clock controllers you are trying
>>>>> force into one.
>>>>>
>>>>
>>>> See the begining.
>>>>
>>>>>> +
>>>>>> + /*
>>>>>> + * Register and enable AUD2_CLKID_AUDIOTOP clock first. Unless
>>>>>> + * it is enabled any read/write to 'map0' hangs the CPU.
>>>>>> + */
>>>>>> +
>>>>>> + ret = a1_register_clk(pdev, map0, map1,
>>>>>> + a1_audio_clks.hws[AUD2_CLKID_AUDIOTOP]);
>>>>>> + if (ret)
>>>>>> + return ret;
>>>>>> +
>>>>>> + ret = clk_prepare_enable(a1_audio_clks.hws[AUD2_CLKID_AUDIOTOP]->clk);
>>>>>> + if (ret)
>>>>>> + return ret;
>>>>>
>>>>> Again, this shows 2 devices. The one related to your 'map0' should
>>>>> request AUD2_CLKID_AUDIOTOP as input and enable it right away.
>>>>>
>>>>
>>>> See the begining.
>>>>
>>>>>> +
>>>>>> + for (i = 0; i < a1_audio_clks.num; i++) {
>>>>>> + if (i == AUD2_CLKID_AUDIOTOP)
>>>>>> + continue;
>>>>>> +
>>>>>> + ret = a1_register_clk(pdev, map0, map1, a1_audio_clks.hws[i]);
>>>>>> + if (ret)
>>>>>> + return ret;
>>>>>> + }
>>>>>> +
>>>>>> + ret = devm_of_clk_add_hw_provider(&pdev->dev, meson_clk_hw_get,
>>>>>> + &a1_audio_clks);
>>>>>> + if (ret)
>>>>>> + return ret;
>>>>>> +
>>>>>> + BUILD_BUG_ON((unsigned long)AUDIO_REG_MAP(AUDIO_SW_RESET0) !=
>>>>>> + AUDIO_RANGE_0);
>>>>>
>>>>> Why is that necessary ?
>>>>>
>>>>
>>>> A little paranoia. Here AUDIO_SW_RESET0 is handled as map0's register,
>>>> and I want to assert it.
>>>>
>>>>>> + return meson_audio_rstc_register(&pdev->dev, map0,
>>>>>> + AUDIO_REG_OFFSET(AUDIO_SW_RESET0), 32);
>>>>>> +}
>>>>>> +
>>>>>> +static const struct of_device_id a1_audio_clkc_match_table[] = {
>>>>>> + { .compatible = "amlogic,a1-audio-clkc", },
>>>>>> + {}
>>>>>> +};
>>>>>> +MODULE_DEVICE_TABLE(of, a1_audio_clkc_match_table);
>>>>>> +
>>>>>> +static struct platform_driver a1_audio_clkc_driver = {
>>>>>> + .probe = a1_audio_clkc_probe,
>>>>>> + .driver = {
>>>>>> + .name = "a1-audio-clkc",
>>>>>> + .of_match_table = a1_audio_clkc_match_table,
>>>>>> + },
>>>>>> +};
>>>>>> +module_platform_driver(a1_audio_clkc_driver);
>>>>>> +
>>>>>> +MODULE_DESCRIPTION("Amlogic A1 Audio Clock driver");
>>>>>> +MODULE_AUTHOR("Jan Dakinevich <jan.dakinevich@salutedevices.com>");
>>>>>> +MODULE_LICENSE("GPL");
>>>>>> diff --git a/drivers/clk/meson/a1-audio.h b/drivers/clk/meson/a1-audio.h
>>>>>> new file mode 100644
>>>>>> index 000000000000..f994e87276cd
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/clk/meson/a1-audio.h
>>>>>> @@ -0,0 +1,58 @@
>>>>>> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
>>>>>> +/*
>>>>>> + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
>>>>>> + *
>>>>>> + * Author: Jan Dakinevich <jan.dakinevich@salutedevices.com>
>>>>>> + */
>>>>>> +
>>>>>> +#ifndef __A1_AUDIO_H
>>>>>> +#define __A1_AUDIO_H
>>>>>> +
>>>>>> +#define AUDIO_RANGE_0 0xa
>>>>>> +#define AUDIO_RANGE_1 0xb
>>>>>> +#define AUDIO_RANGE_SHIFT 16
>>>>>> +
>>>>>> +#define AUDIO_REG(_range, _offset) \
>>>>>> + (((_range) << AUDIO_RANGE_SHIFT) + (_offset))
>>>>>> +
>>>>>> +#define AUDIO_REG_OFFSET(_reg) \
>>>>>> + ((_reg) & ((1 << AUDIO_RANGE_SHIFT) - 1))
>>>>>> +
>>>>>> +#define AUDIO_REG_MAP(_reg) \
>>>>>> + ((void *)((_reg) >> AUDIO_RANGE_SHIFT))
>>>>>
>>>>> That is seriouly overengineered.
>>>>> The following are offset. Just write what they are.
>>>>>
>>>>
>>>> This is all in order to keep range's identifier together with offset and
>>>> then use it to store the identifier in clk_regmaps.
>>>>
>>>>> There is not reason to put that into a header. It is only going to be
>>>>> used by a single driver.
>>>>>>> +
>>>>>> +#define AUDIO_CLK_GATE_EN0 AUDIO_REG(AUDIO_RANGE_0, 0x000)
>>>>>> +#define AUDIO_MCLK_A_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x008)
>>>>>> +#define AUDIO_MCLK_B_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x00c)
>>>>>> +#define AUDIO_MCLK_C_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x010)
>>>>>> +#define AUDIO_MCLK_D_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x014)
>>>>>> +#define AUDIO_MCLK_E_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x018)
>>>>>> +#define AUDIO_MCLK_F_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x01c)
>>>>>> +#define AUDIO_SW_RESET0 AUDIO_REG(AUDIO_RANGE_0, 0x028)
>>>>>> +#define AUDIO_MST_A_SCLK_CTRL0 AUDIO_REG(AUDIO_RANGE_0, 0x040)
>>>>>> +#define AUDIO_MST_A_SCLK_CTRL1 AUDIO_REG(AUDIO_RANGE_0, 0x044)
>>>>>> +#define AUDIO_MST_B_SCLK_CTRL0 AUDIO_REG(AUDIO_RANGE_0, 0x048)
>>>>>> +#define AUDIO_MST_B_SCLK_CTRL1 AUDIO_REG(AUDIO_RANGE_0, 0x04c)
>>>>>> +#define AUDIO_MST_C_SCLK_CTRL0 AUDIO_REG(AUDIO_RANGE_0, 0x050)
>>>>>> +#define AUDIO_MST_C_SCLK_CTRL1 AUDIO_REG(AUDIO_RANGE_0, 0x054)
>>>>>> +#define AUDIO_MST_D_SCLK_CTRL0 AUDIO_REG(AUDIO_RANGE_0, 0x058)
>>>>>> +#define AUDIO_MST_D_SCLK_CTRL1 AUDIO_REG(AUDIO_RANGE_0, 0x05c)
>>>>>> +#define AUDIO_CLK_TDMIN_A_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x080)
>>>>>> +#define AUDIO_CLK_TDMIN_B_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x084)
>>>>>> +#define AUDIO_CLK_TDMIN_LB_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x08c)
>>>>>> +#define AUDIO_CLK_TDMOUT_A_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x090)
>>>>>> +#define AUDIO_CLK_TDMOUT_B_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x094)
>>>>>> +#define AUDIO_CLK_SPDIFIN_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x09c)
>>>>>> +#define AUDIO_CLK_RESAMPLE_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x0a4)
>>>>>> +#define AUDIO_CLK_LOCKER_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x0a8)
>>>>>> +#define AUDIO_CLK_EQDRC_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x0c0)
>>>>>> +
>>>>>> +#define AUDIO2_CLK_GATE_EN0 AUDIO_REG(AUDIO_RANGE_1, 0x00c)
>>>>>> +#define AUDIO2_MCLK_VAD_CTRL AUDIO_REG(AUDIO_RANGE_1, 0x040)
>>>>>> +#define AUDIO2_CLK_VAD_CTRL AUDIO_REG(AUDIO_RANGE_1, 0x044)
>>>>>> +#define AUDIO2_CLK_PDMIN_CTRL0 AUDIO_REG(AUDIO_RANGE_1, 0x058)
>>>>>> +#define AUDIO2_CLK_PDMIN_CTRL1 AUDIO_REG(AUDIO_RANGE_1, 0x05c)
>>>>>> +
>>>>>> +#include <dt-bindings/clock/amlogic,a1-audio-clkc.h>
>>>>>> +
>>>>>> +#endif /* __A1_AUDIO_H */
>>>>>
>>>>>
>>>
>>>
>
>
--
Best regards
Jan Dakinevich
^ permalink raw reply
* Re: [PATCH v3 net-next 2/2] net: phy: air_en8811h: Add the Airoha EN8811H PHY driver
From: Andrew Lunn @ 2024-03-26 18:44 UTC (permalink / raw)
To: Eric Woudstra
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiner Kallweit,
Russell King, Matthias Brugger, AngeloGioacchino Del Regno,
Frank Wunderlich, Daniel Golle, Lucien Jheng, Zhi-Jun You, netdev,
devicetree
In-Reply-To: <20240326162305.303598-3-ericwouds@gmail.com>
On Tue, Mar 26, 2024 at 05:23:05PM +0100, Eric Woudstra wrote:
> Add the driver for the Airoha EN8811H 2.5 Gigabit PHY. The phy supports
> 100/1000/2500 Mbps with auto negotiation only.
>
> The driver uses two firmware files, for which updated versions are added to
> linux-firmware already.
>
> Note: At phy-address + 8 there is another device on the mdio bus, that
> belongs to the EN881H. While the original driver writes to it, Airoha
> has confirmed this is not needed. Therefore, communication with this
> device is not included in this driver.
>
> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties
From: Dave Stevenson @ 2024-03-26 18:41 UTC (permalink / raw)
To: Stefan Wahren
Cc: Krzysztof Kozlowski, Laurent Pinchart, devicetree,
linux-rpi-kernel, linux-arm-kernel, linux-gpio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, Ray Jui, Scott Branden,
Linus Walleij, Bartosz Golaszewski
In-Reply-To: <45242028-edf7-49fc-80bf-be9eb242b4cd@gmx.net>
On Tue, 26 Mar 2024 at 11:47, Stefan Wahren <wahrenst@gmx.net> wrote:
>
> [add Dave since he's working on DMA for Raspberry Pi 4 and maybe have a
> opinion about this]
No real opinion from me, but I am far from a DT expert, and AFAIK this
bit isn't impacted by the stuff I'm looking at.
Laurent is correct that it's missing from the binding doc when it
looks to be needed. Adding it would therefore be the correct thing to
do.
Dave
> [drop Emma Anholt old address since she is not involved anymore]
>
> Am 26.03.24 um 08:06 schrieb Krzysztof Kozlowski:
> > On 26/03/2024 01:49, Laurent Pinchart wrote:
> >> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
> >> and, as a result, also needs to specify #address-cells and #size-cells.
> >> Those properties have been added to thebcm2835-rpi.dtsi in commits
> >> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
> >> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
> >> limitations"), but the DT bindings haven't been updated, resulting in
> >> validation errors:
> >>
> >> arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: firmware: '#address-cells', '#size-cells', 'dma-ranges', 'gpio' do not match any of the regexes: 'pinctrl-[0-9]+'
> >> from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#
> >>
> >> Fix this by adding the properties to the bindings.
> >>
> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Children do not perform any IO on their own, because everything is
> > handled by parent. It is really odd to see dma-ranges without ranges.
> > Referenced commits might be also wrong.
> >
> > Best regards,
> > Krzysztof
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* Re: [PATCH v3 net-next 1/2] dt-bindings: net: airoha,en8811h: Add en8811h
From: Andrew Lunn @ 2024-03-26 18:37 UTC (permalink / raw)
To: Eric Woudstra
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiner Kallweit,
Russell King, Matthias Brugger, AngeloGioacchino Del Regno,
Frank Wunderlich, Daniel Golle, Lucien Jheng, Zhi-Jun You, netdev,
devicetree
In-Reply-To: <20240326162305.303598-2-ericwouds@gmail.com>
On Tue, Mar 26, 2024 at 05:23:04PM +0100, Eric Woudstra wrote:
> Add the Airoha EN8811H 2.5 Gigabit PHY.
>
> The en8811h phy can be set with serdes polarity reversed on rx and/or tx.
>
> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: dma: snps,dw-axi-dmac: Add JH8100 support
From: Conor Dooley @ 2024-03-26 18:32 UTC (permalink / raw)
To: Tan Chun Hau
Cc: Eugeniy Paltsev, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Ley Foon Tan, Jee Heng Sia, dmaengine, devicetree,
linux-kernel
In-Reply-To: <20240326095457.201572-2-chunhau.tan@starfivetech.com>
[-- Attachment #1: Type: text/plain, Size: 1112 bytes --]
On Tue, Mar 26, 2024 at 02:54:56AM -0700, Tan Chun Hau wrote:
> Add support for StarFive JH8100 SoC in Sysnopsys Designware AXI DMA
> controller.
Your commit message should explain what makes this incompatible with
existing devices. That inforatiion does appear to be in the driver
patch, but should also be here. Otherwise,
Acked-by: Conor Dooley <conor.dooley@microchip.com>
>
> Signed-off-by: Tan Chun Hau <chunhau.tan@starfivetech.com>
> ---
> Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
> index 363cf8bd150d..525f5f3932f5 100644
> --- a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
> +++ b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
> @@ -21,6 +21,7 @@ properties:
> - snps,axi-dma-1.01a
> - intel,kmb-axi-dma
> - starfive,jh7110-axi-dma
> + - starfive,jh8100-axi-dma
>
> reg:
> minItems: 1
> --
> 2.25.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH] arm64: dts: qcom: qcs6490-rb3gen2: Enable UFS
From: Bjorn Andersson @ 2024-03-26 18:02 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, Bjorn Andersson
The rb3gen2 has UFS memory, adjust the necessary supply voltage and add
the controller and phy nodes to enable this.
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index 63ebe0774f1d..0177d93ecd1d 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -221,8 +221,8 @@ vreg_l6b_1p2: ldo6 {
vreg_l7b_2p952: ldo7 {
regulator-name = "vreg_l7b_2p952";
- regulator-min-microvolt = <2400000>;
- regulator-max-microvolt = <3544000>;
+ regulator-min-microvolt = <2952000>;
+ regulator-max-microvolt = <2952000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
@@ -508,6 +508,25 @@ &usb_1_qmpphy {
status = "okay";
};
+&ufs_mem_hc {
+ reset-gpios = <&tlmm 175 GPIO_ACTIVE_LOW>;
+ vcc-supply = <&vreg_l7b_2p952>;
+ vcc-max-microamp = <800000>;
+ vccq-supply = <&vreg_l9b_1p2>;
+ vccq-max-microamp = <900000>;
+ vccq2-supply = <&vreg_l9b_1p2>;
+ vccq2-max-microamp = <900000>;
+
+ status = "okay";
+};
+
+&ufs_mem_phy {
+ vdda-phy-supply = <&vreg_l10c_0p88>;
+ vdda-pll-supply = <&vreg_l6b_1p2>;
+
+ status = "okay";
+};
+
&wifi {
memory-region = <&wlan_fw_mem>;
};
---
base-commit: 084c8e315db34b59d38d06e684b1a0dd07d30287
change-id: 20240326-rb3gen2-ufs-7ddb07157556
Best regards,
--
Bjorn Andersson <quic_bjorande@quicinc.com>
^ permalink raw reply related
* Re: [PATCH v6 1/2] dt-bindings: arm: aspeed: add ASUS X4TF board
From: Conor Dooley @ 2024-03-26 18:24 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Kelly Hung, robh+dt, krzysztof.kozlowski+dt, conor+dt, joel,
andrew, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
openbmc, kelly_hung, Allenyy_Hsu, Rob Herring, Zev Weiss,
Conor Dooley
In-Reply-To: <32cd6f33-b4e9-4b7a-bcea-b1f2e421d67e@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 946 bytes --]
On Tue, Mar 26, 2024 at 12:36:50PM +0100, Krzysztof Kozlowski wrote:
> On 26/03/2024 11:35, Kelly Hung wrote:
> > Document the new compatibles used on ASUS X4TF.
> >
> > Signed-off-by: Kelly Hung <Kelly_Hung@asus.com>
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > Reviewed-by: Rob Herring <robh@kernel.org>
>
> Where did it happen? Where did you receive this tag? Please provide link
> to lore.
Robh's bot sent two reports, but I do not see a tag:
https://lore.kernel.org/all/?q=c%3AKelly_Hung%40asus.com+f%3Arobh
> > Reviewed-by: Zev Weiss <zweiss@equinix.com>
>
> Where did it happen? Where did you receive this tag? Please provide link
> to lore.
Zev seems to have left a comment on this, but not provided a tag:
https://lore.kernel.org/all/20240223220115.GB9299@packtop/
Kelly, it is important that you do not add tags unless someone gives
them to you explicitly.
Thanks,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 1/5] dt-bindings: iio: dac: ti,dac5571: Add DAC081C081 support
From: Jonathan Cameron @ 2024-03-26 18:21 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Laurent Pinchart, devicetree, imx, linux-arm-kernel,
Trevor Zaharichuk, Greg Lytle, Lars-Peter Clausen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sean Nyekjaer, linux-iio
In-Reply-To: <20240326171316.000018f5@Huawei.com>
On Tue, 26 Mar 2024 17:13:16 +0000
Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
> On Tue, 26 Mar 2024 18:50:43 +0200
> Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
>
> > On Tue, Mar 26, 2024 at 03:29:27PM +0000, Jonathan Cameron wrote:
> > > On Mon, 25 Mar 2024 22:56:41 +0200 Laurent Pinchart wrote:
> > > > On Mon, Mar 25, 2024 at 08:48:57PM +0000, Jonathan Cameron wrote:
> > > > > On Mon, 25 Mar 2024 22:32:41 +0200 Laurent Pinchart wrote:
> > > > >
> > > > > > The DAC081C081 is a TI DAC whose software interface is compatible with
> > > > > > the DAC5571. It is the 8-bit version of the DAC121C081, already
> > > > > > supported by the DAC5571 bindings. Extends the bindings to support this
> > > > > > chip.
> > > > > >
> > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > >
> > > > > Hi Laurent,
> > > > >
> > > > > Given it's a part number where no one is going to guess it is compatible
> > > > > with the DAC5571 and that we don't have a history of fallback compatibles
> > > > > I'm fine with this change, but just wanted to ask is a fallback compatible
> > > > > useful to you to run with older kernels?
> > > > >
> > > > > I should have noticed when Peter added the dac121c081. If we add a fallback
> > > > > should do that one as well.
> > > >
> > > > I've indeed noticed that there should have been a fallback for
> > > > dac121c081, but didn't stop to ponder why that wasn't the case, and just
> > > > went along with the flow :-) I agree a fallback could be useful, which
> > > > would then allow dropping patch 2/5 from this series (*). I can do so if
> > > > you prefer.
> > > >
> > > > * This is not entirely true. While the DAC1081C081 is largely compatible
> > > > with the DAC5573, they have different values for one of the power-down
> > > > resistors (2.5kΩ instead of 1kΩ if I recall correctly). To be completely
> > > > accurate, the driver should report that. We could still use the fallback
> > > > compatible, reporting the wrong power-down resistor value.
> > >
> > > Hmm - Would anyone really care about that value being wrong?
> >
> > I don't have enough expertise with IIO to be sure, but my guess is that
> > nobody would.
> >
> > > I think perhaps that's just about significant enough that maybe a fallback
> > > compatible doesn't make sense here.
> >
> > Then let's keep it simple and just merge this patch as-is ? :-)
> Makes sense. I'm on wrong computer at the moment but will pick it up at
> weekend if not before.
Sooner it is. Applied 1 and 2 to the togreg branch of iio.git and pushed
out as testing on the extremely unlikely chance that 0-day finds a build
issue...
I grabbed them today because I have a memory like a gold fish and the thread
is deep enough that I might skip over later on basis 'it must have outstanding
questions' :)
Jonathan
>
> Jonathan
>
>
> >
> > > > > > ---
> > > > > > Documentation/devicetree/bindings/iio/dac/ti,dac5571.yaml | 1 +
> > > > > > 1 file changed, 1 insertion(+)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/iio/dac/ti,dac5571.yaml b/Documentation/devicetree/bindings/iio/dac/ti,dac5571.yaml
> > > > > > index 79da0323c327..e59db861e2eb 100644
> > > > > > --- a/Documentation/devicetree/bindings/iio/dac/ti,dac5571.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/iio/dac/ti,dac5571.yaml
> > > > > > @@ -21,6 +21,7 @@ properties:
> > > > > > - ti,dac5573
> > > > > > - ti,dac6573
> > > > > > - ti,dac7573
> > > > > > + - ti,dac081c081
> > > > > > - ti,dac121c081
> > > > > >
> > > > > > reg:
> >
>
^ permalink raw reply
* Re: [PATCH] dt-bindings: crypto: ti,omap-sham: Convert to dtschema
From: Conor Dooley @ 2024-03-26 18:20 UTC (permalink / raw)
To: Animesh Agarwal
Cc: Herbert Xu, David S. Miller, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-crypto, devicetree, linux-kernel
In-Reply-To: <20240326120107.13442-1-animeshagarwal28@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3660 bytes --]
On Tue, Mar 26, 2024 at 05:31:00PM +0530, Animesh Agarwal wrote:
> Convert the OMAP SoC SHA crypto Module bindings to DT Schema.
>
> Signed-off-by: Animesh Agarwal <animeshagarwal28@gmail.com>
> ---
> .../devicetree/bindings/crypto/omap-sham.txt | 28 ----------
> .../bindings/crypto/ti,omap-sham.yaml | 56 +++++++++++++++++++
> 2 files changed, 56 insertions(+), 28 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/crypto/omap-sham.txt
> create mode 100644 Documentation/devicetree/bindings/crypto/ti,omap-sham.yaml
>
> diff --git a/Documentation/devicetree/bindings/crypto/omap-sham.txt b/Documentation/devicetree/bindings/crypto/omap-sham.txt
> deleted file mode 100644
> index ad9115569611..000000000000
> --- a/Documentation/devicetree/bindings/crypto/omap-sham.txt
> +++ /dev/null
> @@ -1,28 +0,0 @@
> -OMAP SoC SHA crypto Module
> -
> -Required properties:
> -
> -- compatible : Should contain entries for this and backward compatible
> - SHAM versions:
> - - "ti,omap2-sham" for OMAP2 & OMAP3.
> - - "ti,omap4-sham" for OMAP4 and AM33XX.
> - - "ti,omap5-sham" for OMAP5, DRA7 and AM43XX.
> -- ti,hwmods: Name of the hwmod associated with the SHAM module
> -- reg : Offset and length of the register set for the module
> -- interrupts : the interrupt-specifier for the SHAM module.
> -
> -Optional properties:
> -- dmas: DMA specifiers for the rx dma. See the DMA client binding,
> - Documentation/devicetree/bindings/dma/dma.txt
> -- dma-names: DMA request name. Should be "rx" if a dma is present.
> -
> -Example:
> - /* AM335x */
> - sham: sham@53100000 {
> - compatible = "ti,omap4-sham";
> - ti,hwmods = "sham";
> - reg = <0x53100000 0x200>;
> - interrupts = <109>;
> - dmas = <&edma 36>;
> - dma-names = "rx";
> - };
> diff --git a/Documentation/devicetree/bindings/crypto/ti,omap-sham.yaml b/Documentation/devicetree/bindings/crypto/ti,omap-sham.yaml
> new file mode 100644
> index 000000000000..7a2529cc4cae
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/ti,omap-sham.yaml
> @@ -0,0 +1,56 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/crypto/ti,omap-sham.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: OMAP SoC SHA crypto Module
> +
> +maintainers:
> + - Animesh Agarwal <animeshagarwal28@gmail.com>
> +
> +properties:
> + compatible:
> + enum:
> + - ti,omap2-sham
> + - ti,omap4-sham
> + - ti,omap5-sham
> +
> + ti,hwmods:
> + description: Name of the hwmod associated with the SHAM module
> + $ref: /schemas/types.yaml#/definitions/string
> + enum: [sham]
Is there really only one value possible here?
Also, the convention is to put vendor properties like this after more
common properties like reg, interrupts etc.
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + dmas:
> + maxItems: 1
> +
> + dma-names:
> + const: rx
> +
> +dependencies:
> + dmas: [dma-names]
Is this needed? Unless I'm sorely mistaken dt-schema enforces this itself
(and same for any $foo-names).
Thanks,
Conor.
> +
> +additionalProperties: false
> +
> +required:
> + - compatible
> + - ti,hwmods
> + - reg
> + - interrupts
> +
> +examples:
> + - |
> + sham@53100000 {
> + compatible = "ti,omap4-sham";
> + ti,hwmods = "sham";
> + reg = <0x53100000 0x200>;
> + interrupts = <109>;
> + dmas = <&edma 36>;
> + dma-names = "rx";
> + };
> --
> 2.44.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH] dt-bindings: ata: ahci-da850: Convert to dtschema
From: Conor Dooley @ 2024-03-26 18:14 UTC (permalink / raw)
To: Animesh Agarwal
Cc: Damien Le Moal, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-ide, devicetree, linux-kernel
In-Reply-To: <20240326121735.11994-1-animeshagarwal28@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 586 bytes --]
On Tue, Mar 26, 2024 at 05:47:28PM +0530, Animesh Agarwal wrote:
> - - compatible: must be "ti,da850-ahci"
> - - reg: physical base addresses and sizes of the two register regions
> - used by the controller: the register map as defined by the
> - AHCI 1.1 standard and the Power Down Control Register (PWRDN)
> - for enabling/disabling the SATA clock receiver
> + reg:
> + minItems: 2
> + maxItems: 2
Could you make this an items list with a pair of text descriptions
please? The original text binding's text for each can be reused.
Thanks,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v6 17/23] dt-bindings: crypto: meson: remove clk and second interrupt line for GXL
From: Conor Dooley @ 2024-03-26 18:09 UTC (permalink / raw)
To: Alexey Romanov
Cc: neil.armstrong, clabbe, herbert, davem, robh+dt,
krzysztof.kozlowski+dt, conor+dt, khilman, jbrunet,
martin.blumenstingl, vadim.fedorenko, linux-crypto, linux-amlogic,
linux-kernel, devicetree, linux-arm-kernel, kernel
In-Reply-To: <20240326153219.2915080-18-avromanov@salutedevices.com>
[-- Attachment #1: Type: text/plain, Size: 1864 bytes --]
On Tue, Mar 26, 2024 at 06:32:13PM +0300, Alexey Romanov wrote:
> GXL crypto IP uses DMA engine, which doesn't require clk input
> and second interrupt line.
>
> Fixes: c4a0457eb858 ("ARM64: dts: amlogic: adds crypto hardware node")
How does this fix a commit that only modifies a dts? The commit cited
here should be a dt-bindings commit.
However, your commit message says "require" but you do more than remove
these as required, you remove them entirely. What am I missing?
Thanks,
Conor.
> Signed-off-by: Alexey Romanov <avromanov@salutedevices.com>
> ---
> .../bindings/crypto/amlogic,gxl-crypto.yaml | 13 +------------
> 1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> index 948e11ebe4ee..d3af7b4d5f39 100644
> --- a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> +++ b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> @@ -20,20 +20,11 @@ properties:
> interrupts:
> items:
> - description: Interrupt for flow 0
> - - description: Interrupt for flow 1
> -
> - clocks:
> - maxItems: 1
> -
> - clock-names:
> - const: blkmv
>
> required:
> - compatible
> - reg
> - interrupts
> - - clocks
> - - clock-names
>
> additionalProperties: false
>
> @@ -46,7 +37,5 @@ examples:
> crypto: crypto-engine@c883e000 {
> compatible = "amlogic,gxl-crypto";
> reg = <0xc883e000 0x36>;
> - interrupts = <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>, <GIC_SPI 189 IRQ_TYPE_EDGE_RISING>;
> - clocks = <&clkc CLKID_BLKMV>;
> - clock-names = "blkmv";
> + interrupts = <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>;
> };
> --
> 2.34.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v6 19/23] dt-bindings: crypto: meson: support new SoC's
From: Conor Dooley @ 2024-03-26 18:04 UTC (permalink / raw)
To: Alexey Romanov
Cc: neil.armstrong, clabbe, herbert, davem, robh+dt,
krzysztof.kozlowski+dt, conor+dt, khilman, jbrunet,
martin.blumenstingl, vadim.fedorenko, linux-crypto, linux-amlogic,
linux-kernel, devicetree, linux-arm-kernel, kernel
In-Reply-To: <20240326153219.2915080-20-avromanov@salutedevices.com>
[-- Attachment #1: Type: text/plain, Size: 1543 bytes --]
On Tue, Mar 26, 2024 at 06:32:15PM +0300, Alexey Romanov wrote:
> Now crypto module available at G12A/G12B/S4/A1/SM1/AXG.
>
> 1. Add new compatibles:
> - amlogic,g12a-crypto
> - amlogic,axg-crypto
> - amlogic,a1-crypto
> - amlogic,s4-crypto (uses a1-crypto as fallback)
>
> 2. Add power-domains in schema.
>
> Signed-off-by: Alexey Romanov <avromanov@salutedevices.com>
> ---
> .../bindings/crypto/amlogic,gxl-crypto.yaml | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> index d3af7b4d5f39..c92edde314aa 100644
> --- a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> +++ b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> @@ -11,8 +11,16 @@ maintainers:
>
> properties:
> compatible:
> - items:
> - - const: amlogic,gxl-crypto
> + oneOf:
> + - items:
> + - enum:
> + - amlogic,s4-crypto
> + - const: amlogic,a1-crypto
> + - enum:
> + - amlogic,gxl-crypto
> + - amlogic,axg-crypto
> + - amlogic,g12a-crypto
> + - amlogic,a1-crypto
>
> reg:
> maxItems: 1
> @@ -21,6 +29,9 @@ properties:
> items:
> - description: Interrupt for flow 0
>
> + power-domains:
> + maxItems: 1
Is power-domains valid for the devices that existed prior to your patch?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties
From: Laurent Pinchart @ 2024-03-26 18:00 UTC (permalink / raw)
To: Stefan Wahren
Cc: Krzysztof Kozlowski, devicetree, linux-rpi-kernel,
linux-arm-kernel, linux-gpio, Dave Stevenson, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, Ray Jui, Scott Branden,
Linus Walleij, Bartosz Golaszewski
In-Reply-To: <e6334490-501c-4967-a48b-168868acfd39@gmx.net>
On Tue, Mar 26, 2024 at 06:40:52PM +0100, Stefan Wahren wrote:
> Am 26.03.24 um 18:18 schrieb Laurent Pinchart:
> > On Tue, Mar 26, 2024 at 12:47:34PM +0100, Stefan Wahren wrote:
> >> [add Dave since he's working on DMA for Raspberry Pi 4 and maybe have a
> >> opinion about this]
> >>
> >> [drop Emma Anholt old address since she is not involved anymore]
> >>
> >> Am 26.03.24 um 08:06 schrieb Krzysztof Kozlowski:
> >>> On 26/03/2024 01:49, Laurent Pinchart wrote:
> >>>> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
> >>>> and, as a result, also needs to specify #address-cells and #size-cells.
> >>>> Those properties have been added to thebcm2835-rpi.dtsi in commits
> >>>> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
> >>>> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
> >>>> limitations"), but the DT bindings haven't been updated, resulting in
> >>>> validation errors:
> >>>>
> >>>> arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: firmware: '#address-cells', '#size-cells', 'dma-ranges', 'gpio' do not match any of the regexes: 'pinctrl-[0-9]+'
> >>>> from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#
> >>>>
> >>>> Fix this by adding the properties to the bindings.
> >>>>
> >>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>
> >>> Children do not perform any IO on their own, because everything is
> >>> handled by parent. It is really odd to see dma-ranges without ranges.
> >>> Referenced commits might be also wrong.
> >
> > Comunication with the firmware goes through a mailbox interface, which
> > uses DMA transfers. See for instance
> >
> > rpi_firmware_transaction(struct rpi_firmware *fw, u32 chan, u32 data)
> > {
> > u32 message = MBOX_MSG(chan, data);
> > int ret;
> >
> > WARN_ON(data & 0xf);
> >
> > mutex_lock(&transaction_lock);
> > reinit_completion(&fw->c);
> > ret = mbox_send_message(fw->chan, &message);
> > if (ret >= 0) {
> > if (wait_for_completion_timeout(&fw->c, HZ)) {
> > ret = 0;
> > } else {
> > ret = -ETIMEDOUT;
> > WARN_ONCE(1, "Firmware transaction timeout");
> > }
> > } else {
> > dev_err(fw->cl.dev, "mbox_send_message returned %d\n", ret);
> > }
> > mutex_unlock(&transaction_lock);
> >
> > return ret;
> > }
> >
> > int rpi_firmware_property_list(struct rpi_firmware *fw,
> > void *data, size_t tag_size)
> > {
> > size_t size = tag_size + 12;
> > u32 *buf;
> > dma_addr_t bus_addr;
> > int ret;
> >
> > /* Packets are processed a dword at a time. */
> > if (size & 3)
> > return -EINVAL;
> >
> > buf = dma_alloc_coherent(fw->cl.dev, PAGE_ALIGN(size), &bus_addr,
> > GFP_ATOMIC);
> > if (!buf)
> > return -ENOMEM;
> >
> > /* The firmware will error out without parsing in this case. */
> > WARN_ON(size >= 1024 * 1024);
> >
> > buf[0] = size;
> > buf[1] = RPI_FIRMWARE_STATUS_REQUEST;
> > memcpy(&buf[2], data, tag_size);
> > buf[size / 4 - 1] = RPI_FIRMWARE_PROPERTY_END;
> > wmb();
> >
> > ret = rpi_firmware_transaction(fw, MBOX_CHAN_PROPERTY, bus_addr);
> >
> > rmb();
> > memcpy(data, &buf[2], tag_size);
> > if (ret == 0 && buf[1] != RPI_FIRMWARE_STATUS_SUCCESS) {
> > /*
> > * The tag name here might not be the one causing the
> > * error, if there were multiple tags in the request.
> > * But single-tag is the most common, so go with it.
> > */
> > dev_err(fw->cl.dev, "Request 0x%08x returned status 0x%08x\n",
> > buf[2], buf[1]);
> > ret = -EINVAL;
> > }
> >
> > dma_free_coherent(fw->cl.dev, PAGE_ALIGN(size), buf, bus_addr);
> >
> > return ret;
> > }
> >
> > fw->cl.dev is the device for the firmware child node. That may be where
> > the problem comes from, shouldn't we use the mailbox device for DMA
> > mapping ?
>
> From devicetree perspective this is the mailbox DT part [1] and this
> the matching dt-binding [2].
>
> [1] - https://elixir.bootlin.com/linux/v6.9-rc1/source/arch/arm/boot/dts/broadcom/bcm283x.dtsi#L100
> [2] - https://elixir.bootlin.com/linux/v6.9-rc1/source/Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.yaml
That's the device performing DMA, so I think it should be used for DMA
mapping.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: net: wireless: ath10k: add qcom,no-msa-ready-indicator prop
From: Dmitry Baryshkov @ 2024-03-26 17:51 UTC (permalink / raw)
To: Marc Gonzalez
Cc: Kalle Valo, Jeff Johnson, ath10k, wireless, DT,
Pierre-Hugues Husson, Arnaud Vrac, Jami Kettunen, Jeffrey Hugo,
Bjorn Andersson, Konrad Dybcio
In-Reply-To: <9293793d-00e8-42ce-87b2-05abc3b49387@freebox.fr>
On Tue, 26 Mar 2024 at 19:45, Marc Gonzalez <mgonzalez@freebox.fr> wrote:
>
> [ It has been pointed out to me that the previous message was unclear. ]
> [ Below is my 2nd attempt at a clearer message. ]
>
> Problem: firmware-5.bin has not been parsed yet when we have to handle
> the ATH10K_QMI_EVENT_SERVER_ARRIVE case, so we can't rely on feature bits
> to work around the lack of MSA_READY indicator.
Then, I'd say, we have to resort to the DT property, unless Kalle or
Jeff have other proposals.
>
>
> On 26/03/2024 16:04, Marc Gonzalez wrote:
>
> > QMI stuff happens much EARLIER than firmware-5.bin handling.
> > (ar->running_fw is still NULL)
> >
> >
> > [ 14.547563] ath10k_snoc 18800000.wifi: qmi wifi fw qmi service connected
> > [ 14.555054] ath10k_snoc 18800000.wifi: qmi indication register request completed
> > [ 14.561406] ath10k_snoc 18800000.wifi: qmi host capability request completed
> > [ 14.568365] ath10k_snoc 18800000.wifi: qmi msa mem region 0 addr 0x0x0000000094400000 size 0x4000 flag 0x00000001
> > [ 14.573775] ath10k_snoc 18800000.wifi: qmi msa mem region 1 addr 0x0x0000000094404000 size 0xfc000 flag 0x00000000
> > [ 14.583896] ath10k_snoc 18800000.wifi: qmi msa mem info request completed
> > [ 14.630179] ath10k_snoc 18800000.wifi: qmi msa mem ready request completed
> > [ 14.681647] ath10k_snoc 18800000.wifi: qmi chip_id 0x30214 chip_family 0x4001 board_id 0xff soc_id 0x40010002
> > [ 14.681726] ath10k_snoc 18800000.wifi: qmi fw_version 0x100204b2 fw_build_timestamp 2019-09-04 03:01 fw_build_id QC_IMAGE_VERSION_STRING=WLAN.HL.1.0-01202-QCAHLSWMTPLZ-1.221523.2
> > *** ATH10K_QMI_EVENT_SERVER_ARRIVE is handled at this point
> > [ 14.690657] ath10k_snoc 18800000.wifi: DT bdf variant name not set.
> > [ 14.706423] ath10k_snoc 18800000.wifi: boot using board name 'bus=snoc,qmi-board-id=ff,qmi-chip-id=30214'
> > [ 14.712587] ath10k_snoc 18800000.wifi: boot using board name 'bus=snoc,qmi-board-id=ff,qmi-chip-id=30214'
> > [ 14.722309] ath10k_snoc 18800000.wifi: boot using board name 'bus=snoc,qmi-board-id=ff'
> > [ 14.745634] ath10k_snoc 18800000.wifi: boot fw request 'ath10k/WCN3990/hw1.0/board-2.bin': 0
> > ... snip boards dump
> > [ 15.560607] ath10k_snoc 18800000.wifi: boot found match for name 'bus=snoc,qmi-board-id=ff,qmi-chip-id=30214'
> > [ 15.570590] ath10k_snoc 18800000.wifi: boot found board data for 'bus=snoc,qmi-board-id=ff,qmi-chip-id=30214'
> > [ 15.580393] ath10k_snoc 18800000.wifi: using board api 2
> > [ 15.591465] ath10k_snoc 18800000.wifi: qmi bdf download request completed
> > [ 15.595786] ath10k_snoc 18800000.wifi: qmi cal report request completed
> > [ 18.667441] ath10k_snoc 18800000.wifi: wifi fw ready event received
> > [ 18.667519] ath10k_snoc 18800000.wifi: ath10k_snoc_hif_power_up:WCN3990 driver state = 0
> > [ 18.672579] ath10k_snoc 18800000.wifi: soc power on
> > [ 18.684550] ath10k_snoc 18800000.wifi: qmi mode 0 config 00000000313ae0ca
> > [ 18.692591] ath10k_snoc 18800000.wifi: qmi config request completed
> > [ 18.748262] ath10k_snoc 18800000.wifi: qmi wlan mode req completed: 0
> > [ 18.748370] ath10k_snoc 18800000.wifi: boot init ce src ring id 0 entries 16 base_addr 00000000b9feff9c
> > [ 18.753792] ath10k_snoc 18800000.wifi: boot ce dest ring id 1 entries 512 base_addr 000000005f850e88
> > [ 18.762971] ath10k_snoc 18800000.wifi: boot ce dest ring id 2 entries 64 base_addr 000000002467084e
> > [ 18.772375] ath10k_snoc 18800000.wifi: boot init ce src ring id 3 entries 32 base_addr 00000000ded78c3f
> > [ 18.781122] ath10k_snoc 18800000.wifi: boot init ce src ring id 4 entries 2048 base_addr 00000000c9e8883a
> > [ 18.790482] ath10k_snoc 18800000.wifi: boot ce dest ring id 5 entries 512 base_addr 00000000309e9375
> > [ 18.800220] ath10k_snoc 18800000.wifi: boot init ce src ring id 7 entries 2 base_addr 00000000e5e3fb73
> > [ 18.809395] ath10k_snoc 18800000.wifi: boot ce dest ring id 7 entries 2 base_addr 00000000c08890b2
> > [ 18.818507] ath10k_snoc 18800000.wifi: boot ce dest ring id 8 entries 128 base_addr 000000006af0777e
> > [ 18.827466] ath10k_snoc 18800000.wifi: boot ce dest ring id 9 entries 512 base_addr 00000000f2c0ce43
> > [ 18.836770] ath10k_snoc 18800000.wifi: boot ce dest ring id 10 entries 512 base_addr 00000000269a2564
> > [ 18.845863] ath10k_snoc 18800000.wifi: boot ce dest ring id 11 entries 512 base_addr 00000000f4a8c90e
> > [ 18.855002] ath10k_snoc 18800000.wifi: Hardware name wcn3990 hw1.0 version 0x8
> > [ 18.865605] ath10k_snoc 18800000.wifi: boot fw request 'ath10k/pre-cal-snoc-18800000.wifi.bin': -2
> > [ 18.871747] ath10k_snoc 18800000.wifi: boot fw request 'ath10k/cal-snoc-18800000.wifi.bin': -2
> > [ 18.880307] ath10k_snoc 18800000.wifi: trying fw api 6
> > [ 18.889520] ath10k_snoc 18800000.wifi: boot fw request 'ath10k/WCN3990/hw1.0/firmware-6.bin': -2
> > [ 18.894020] ath10k_snoc 18800000.wifi: trying fw api 5
> > *** firmware-5.bin is handled at this point
> > [ 18.904176] ath10k_snoc 18800000.wifi: boot fw request 'ath10k/WCN3990/hw1.0/firmware-5.bin': 0
> > [ 18.907802] ath10k_snoc 18800000.wifi: found fw timestamp 1539237028
> > [ 18.916460] ath10k_snoc 18800000.wifi: found firmware features ie (3 B)
> > [ 18.923027] ath10k_snoc 18800000.wifi: Enabling feature bit: 6
> > [ 18.929373] ath10k_snoc 18800000.wifi: Enabling feature bit: 18
> > [ 18.935279] ath10k_snoc 18800000.wifi: Enabling feature bit: 19
> > [ 18.941085] ath10k_snoc 18800000.wifi: features
> > [ 18.946975] ath10k_snoc 18800000.wifi: 00000000: 40 00 0c 00 00 00 00 00 @.......
> > [ 18.951534] ath10k_snoc 18800000.wifi: found fw ie wmi op version 4
> > [ 18.961235] ath10k_snoc 18800000.wifi: found fw ie htt op version 3
> > [ 18.967390] ath10k_snoc 18800000.wifi: using fw api 5
> >
> >
> >
> > I don't know how to solve this problem.
> > (If we just skip waiting for MSA_READY, there is no problem)
> >
> > Kalle, Jeff, do you see a way out of this conundrum?
>
> Regards.
>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v2] arm64: dts: renesas: white-hawk: ethernet: Describe avb1 and avb2
From: Niklas Söderlund @ 2024-03-26 17:50 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-renesas-soc,
devicetree
In-Reply-To: <CAMuHMdXBhpbH2=21e26BeuknpW08eoX_yG4UQg-Ep5TijY3Vfw@mail.gmail.com>
Hi Geert,
Thanks for your report.
On 2024-03-26 16:54:49 +0100, Geert Uytterhoeven wrote:
> Hi Niklas,
>
> On Sat, Mar 9, 2024 at 4:56 PM Niklas Söderlund
> <niklas.soderlund+renesas@ragnatech.se> wrote:
> > Describe the two Marvel 88Q2110/QFN40 PHYs available on the R-Car V4H
> > White Hawk RAVB/Ethernet(1000Base-T1) sub-board. The two PHYs are wired
> > up on the board by default, there is no need to move any resistors which
> > are needed to access other PHYs available on this sub-board.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>
> When accidentally booting a kernel without CONFIG_MARVELL_88Q2XXX_PHY=y,
> I am greeted with the following warning splat (same for the second PHY):
I can reproduce this, but I'm not really sure how to deal with it and
it's unrelated to Renesas AVB Ethernet driver.
What happens when when running without CONFIG_MARVELL_88Q2XXX_PHY=y is
that the core fallback to use the generic Generic Clause 45 PHY driver.
And the generic driver don't work correctly for the PHY on V4H (88Q2110).
The splat happens when the generic driver tries to enable auto
negotiation. This is known to be broken for 88Q2110 and the
CONFIG_MARVELL_88Q2XXX_PHY=y driver disables this. From
marvell-88q2xxx.c
/* The PHY signalizes it supports autonegotiation. Unfortunately, so
* far it was not possible to get a link even when following the init
* sequence provided by Marvell. Disable it for now until a proper
* workaround is found or a new PHY revision is released.
*/
if (phydev->drv->phy_id == MARVELL_PHY_ID_88Q2110)
linkmode_clear_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
phydev->supported);
If I hack the generic PHY to also not try auto negotiation the splat go
away and the PHY binds using the generic driver. I did not test using it
further.
I can't try to submit such a hack as that would break all other PHY
where auto negotiation works. So I'm not sure we can do anything about
this other than make sure we have the correct PHY drivers around, or use
PHYs that works with the Clause 45 specification.
>
> -mv88q2110 e6810000.ethernet-ffffffff:00: attached PHY driver
> (mii_bus:phy_addr=e6810000.ethernet-ffffffff:00, irq=POLL)
> +Generic Clause 45 PHY e6810000.ethernet-ffffffff:00: attached PHY
> driver (mii_bus:phy_addr=e6810000.ethernet-ffffffff:00, irq=POLL)
> +rcar-du feb00000.display: adding to PM domain always-on
> -mv88q2110 e6820000.ethernet-ffffffff:00: attached PHY driver
> (mii_bus:phy_addr=e6820000.ethernet-ffffffff:00, irq=POLL)
> +rcar-du feb00000.display: removing from PM domain always-on
> +------------[ cut here ]------------
> +_phy_start_aneg+0x0/0xa8: returned: -22
> +WARNING: CPU: 2 PID: 55 at drivers/net/phy/phy.c:1262
> _phy_state_machine+0x120/0x198
> +Modules linked in:
> +CPU: 2 PID: 55 Comm: kworker/2:1 Not tainted
> 6.9.0-rc1-white-hawk-02587-g577b6a49a6d4 #235
> +Hardware name: Renesas White Hawk CPU and Breakout boards based on
> r8a779g0 (DT)
> +Workqueue: events_power_efficient phy_state_machine
> +pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> +pc : _phy_state_machine+0x120/0x198
> +lr : _phy_state_machine+0x120/0x198
> +sp : ffffffc082dd3d10
> +x29: ffffffc082dd3d10 x28: ffffff8440089c05 x27: ffffffc081090000
> +x26: ffffffc080e03008 x25: 0000000000000000 x24: ffffffc0815603d0
> +x23: ffffffc080e03008 x22: ffffff86bef98100 x21: 0000000000000004
> +x20: 0000000000000001 x19: ffffff84435b3000 x18: 0000000000000000
> +x17: 0000000000000000 x16: 0000000000000000 x15: 0720072007320732
> +x14: 072d0720073a0764 x13: 0720072007320732 x12: 072d0720073a0764
> +x11: 000000000000033a x10: ffffffc0810b9ac8 x9 : ffffffc081379ca8
> +x8 : ffffffc082dd3a18 x7 : ffffffc082dd3a20 x6 : 00000000ffff7fff
> +x5 : c0000000ffff7fff x4 : 0000000000000000 x3 : 0000000000000001
> +x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffffff8440a98000
> +Call trace:
> + _phy_state_machine+0x120/0x198
> + phy_state_machine+0x2c/0x5c
> + process_scheduled_works+0x314/0x4d4
> + worker_thread+0x1b8/0x20c
> + kthread+0xd8/0xe8
> + ret_from_fork+0x10/0x20
> +irq event stamp: 16
> +hardirqs last enabled at (15): [<ffffffc080913144>]
> _raw_spin_unlock_irq+0x2c/0x40
> +hardirqs last disabled at (16): [<ffffffc08090d434>] __schedule+0x1cc/0x870
> +softirqs last enabled at (0): [<ffffffc0800800f8>] copy_process+0x698/0x1924
> +softirqs last disabled at (0): [<0000000000000000>] 0x0
> +---[ end trace 0000000000000000 ]---
>
> Is that expected behavior?
> Thanks!
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
--
Kind Regards,
Niklas Söderlund
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox