All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: dts: rockchip: nanopi4: decrease Bluetooth UART baud rate
@ 2021-09-20 17:56 ` Chen-Yu Tsai
  0 siblings, 0 replies; 18+ messages in thread
From: Chen-Yu Tsai @ 2021-09-20 17:56 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Chen-Yu Tsai, Robin Murphy, linux-arm-kernel, linux-rockchip,
	devicetree, linux-kernel

From: Chen-Yu Tsai <wens@csie.org>

The RK3399 does not seem to be able to properly generate the required
64 MHz clock for the UART to operate at 4MBd.

Drop the baud rate down to 3MBd, which can be used as the clock
controller is able to produce a 48 MHz clock.

Fixes: 3e2f0bb72be3 ("arm64: dts: rockchip: Add nanopi4 bluetooth")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
index 8c0ff6c96e03..45ff053b119d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
@@ -699,7 +699,7 @@ bluetooth {
 		device-wakeup-gpios = <&gpio2 RK_PD2 GPIO_ACTIVE_HIGH>;
 		host-wakeup-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_HIGH>;
 		shutdown-gpios = <&gpio0 RK_PB1 GPIO_ACTIVE_HIGH>;
-		max-speed = <4000000>;
+		max-speed = <3000000>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&bt_reg_on_h &bt_host_wake_l &bt_wake_l>;
 		vbat-supply = <&vcc3v3_sys>;
-- 
2.30.2


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH] arm64: dts: rockchip: nanopi4: decrease Bluetooth UART baud rate
@ 2021-09-20 17:56 ` Chen-Yu Tsai
  0 siblings, 0 replies; 18+ messages in thread
From: Chen-Yu Tsai @ 2021-09-20 17:56 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Chen-Yu Tsai, Robin Murphy, linux-arm-kernel, linux-rockchip,
	devicetree, linux-kernel

From: Chen-Yu Tsai <wens@csie.org>

The RK3399 does not seem to be able to properly generate the required
64 MHz clock for the UART to operate at 4MBd.

Drop the baud rate down to 3MBd, which can be used as the clock
controller is able to produce a 48 MHz clock.

Fixes: 3e2f0bb72be3 ("arm64: dts: rockchip: Add nanopi4 bluetooth")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
index 8c0ff6c96e03..45ff053b119d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
@@ -699,7 +699,7 @@ bluetooth {
 		device-wakeup-gpios = <&gpio2 RK_PD2 GPIO_ACTIVE_HIGH>;
 		host-wakeup-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_HIGH>;
 		shutdown-gpios = <&gpio0 RK_PB1 GPIO_ACTIVE_HIGH>;
-		max-speed = <4000000>;
+		max-speed = <3000000>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&bt_reg_on_h &bt_host_wake_l &bt_wake_l>;
 		vbat-supply = <&vcc3v3_sys>;
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH] arm64: dts: rockchip: nanopi4: decrease Bluetooth UART baud rate
@ 2021-09-20 17:56 ` Chen-Yu Tsai
  0 siblings, 0 replies; 18+ messages in thread
From: Chen-Yu Tsai @ 2021-09-20 17:56 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Chen-Yu Tsai, Robin Murphy, linux-arm-kernel, linux-rockchip,
	devicetree, linux-kernel

From: Chen-Yu Tsai <wens@csie.org>

The RK3399 does not seem to be able to properly generate the required
64 MHz clock for the UART to operate at 4MBd.

Drop the baud rate down to 3MBd, which can be used as the clock
controller is able to produce a 48 MHz clock.

Fixes: 3e2f0bb72be3 ("arm64: dts: rockchip: Add nanopi4 bluetooth")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
index 8c0ff6c96e03..45ff053b119d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
@@ -699,7 +699,7 @@ bluetooth {
 		device-wakeup-gpios = <&gpio2 RK_PD2 GPIO_ACTIVE_HIGH>;
 		host-wakeup-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_HIGH>;
 		shutdown-gpios = <&gpio0 RK_PB1 GPIO_ACTIVE_HIGH>;
-		max-speed = <4000000>;
+		max-speed = <3000000>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&bt_reg_on_h &bt_host_wake_l &bt_wake_l>;
 		vbat-supply = <&vcc3v3_sys>;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH] arm64: dts: rockchip: rk3399: Hook up DMA for UARTs
  2021-09-20 17:56 ` Chen-Yu Tsai
  (?)
@ 2021-09-20 17:56   ` Chen-Yu Tsai
  -1 siblings, 0 replies; 18+ messages in thread
From: Chen-Yu Tsai @ 2021-09-20 17:56 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Chen-Yu Tsai, Robin Murphy, linux-arm-kernel, linux-rockchip,
	devicetree, linux-kernel

From: Chen-Yu Tsai <wens@csie.org>

The RK3399 has two DMA controllers, one of which is wired up to work
with the SPI controllers and UARTs. The SPI controllers are already
hooked up, but the UARTs aren't.

Add the "dmas" and "dma-names" to the UART device nodes to hook up DMA.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 3871c7fd83b0..87d6e4eb1337 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -608,6 +608,8 @@ uart0: serial@ff180000 {
 		reg = <0x0 0xff180000 0x0 0x100>;
 		clocks = <&cru SCLK_UART0>, <&cru PCLK_UART0>;
 		clock-names = "baudclk", "apb_pclk";
+		dmas = <&dmac_peri 0>, <&dmac_peri 1>;
+		dma-names = "tx", "rx";
 		interrupts = <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH 0>;
 		reg-shift = <2>;
 		reg-io-width = <4>;
@@ -621,6 +623,8 @@ uart1: serial@ff190000 {
 		reg = <0x0 0xff190000 0x0 0x100>;
 		clocks = <&cru SCLK_UART1>, <&cru PCLK_UART1>;
 		clock-names = "baudclk", "apb_pclk";
+		dmas = <&dmac_peri 2>, <&dmac_peri 3>;
+		dma-names = "tx", "rx";
 		interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH 0>;
 		reg-shift = <2>;
 		reg-io-width = <4>;
@@ -634,6 +638,8 @@ uart2: serial@ff1a0000 {
 		reg = <0x0 0xff1a0000 0x0 0x100>;
 		clocks = <&cru SCLK_UART2>, <&cru PCLK_UART2>;
 		clock-names = "baudclk", "apb_pclk";
+		dmas = <&dmac_peri 4>, <&dmac_peri 5>;
+		dma-names = "tx", "rx";
 		interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH 0>;
 		reg-shift = <2>;
 		reg-io-width = <4>;
@@ -647,6 +653,8 @@ uart3: serial@ff1b0000 {
 		reg = <0x0 0xff1b0000 0x0 0x100>;
 		clocks = <&cru SCLK_UART3>, <&cru PCLK_UART3>;
 		clock-names = "baudclk", "apb_pclk";
+		dmas = <&dmac_peri 6>, <&dmac_peri 7>;
+		dma-names = "tx", "rx";
 		interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH 0>;
 		reg-shift = <2>;
 		reg-io-width = <4>;
@@ -1142,6 +1150,8 @@ uart4: serial@ff370000 {
 		reg = <0x0 0xff370000 0x0 0x100>;
 		clocks = <&pmucru SCLK_UART4_PMU>, <&pmucru PCLK_UART4_PMU>;
 		clock-names = "baudclk", "apb_pclk";
+		dmas = <&dmac_peri 8>, <&dmac_peri 9>;
+		dma-names = "tx", "rx";
 		interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH 0>;
 		reg-shift = <2>;
 		reg-io-width = <4>;
-- 
2.30.2


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH] arm64: dts: rockchip: rk3399: Hook up DMA for UARTs
@ 2021-09-20 17:56   ` Chen-Yu Tsai
  0 siblings, 0 replies; 18+ messages in thread
From: Chen-Yu Tsai @ 2021-09-20 17:56 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Chen-Yu Tsai, Robin Murphy, linux-arm-kernel, linux-rockchip,
	devicetree, linux-kernel

From: Chen-Yu Tsai <wens@csie.org>

The RK3399 has two DMA controllers, one of which is wired up to work
with the SPI controllers and UARTs. The SPI controllers are already
hooked up, but the UARTs aren't.

Add the "dmas" and "dma-names" to the UART device nodes to hook up DMA.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 3871c7fd83b0..87d6e4eb1337 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -608,6 +608,8 @@ uart0: serial@ff180000 {
 		reg = <0x0 0xff180000 0x0 0x100>;
 		clocks = <&cru SCLK_UART0>, <&cru PCLK_UART0>;
 		clock-names = "baudclk", "apb_pclk";
+		dmas = <&dmac_peri 0>, <&dmac_peri 1>;
+		dma-names = "tx", "rx";
 		interrupts = <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH 0>;
 		reg-shift = <2>;
 		reg-io-width = <4>;
@@ -621,6 +623,8 @@ uart1: serial@ff190000 {
 		reg = <0x0 0xff190000 0x0 0x100>;
 		clocks = <&cru SCLK_UART1>, <&cru PCLK_UART1>;
 		clock-names = "baudclk", "apb_pclk";
+		dmas = <&dmac_peri 2>, <&dmac_peri 3>;
+		dma-names = "tx", "rx";
 		interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH 0>;
 		reg-shift = <2>;
 		reg-io-width = <4>;
@@ -634,6 +638,8 @@ uart2: serial@ff1a0000 {
 		reg = <0x0 0xff1a0000 0x0 0x100>;
 		clocks = <&cru SCLK_UART2>, <&cru PCLK_UART2>;
 		clock-names = "baudclk", "apb_pclk";
+		dmas = <&dmac_peri 4>, <&dmac_peri 5>;
+		dma-names = "tx", "rx";
 		interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH 0>;
 		reg-shift = <2>;
 		reg-io-width = <4>;
@@ -647,6 +653,8 @@ uart3: serial@ff1b0000 {
 		reg = <0x0 0xff1b0000 0x0 0x100>;
 		clocks = <&cru SCLK_UART3>, <&cru PCLK_UART3>;
 		clock-names = "baudclk", "apb_pclk";
+		dmas = <&dmac_peri 6>, <&dmac_peri 7>;
+		dma-names = "tx", "rx";
 		interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH 0>;
 		reg-shift = <2>;
 		reg-io-width = <4>;
@@ -1142,6 +1150,8 @@ uart4: serial@ff370000 {
 		reg = <0x0 0xff370000 0x0 0x100>;
 		clocks = <&pmucru SCLK_UART4_PMU>, <&pmucru PCLK_UART4_PMU>;
 		clock-names = "baudclk", "apb_pclk";
+		dmas = <&dmac_peri 8>, <&dmac_peri 9>;
+		dma-names = "tx", "rx";
 		interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH 0>;
 		reg-shift = <2>;
 		reg-io-width = <4>;
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH] arm64: dts: rockchip: rk3399: Hook up DMA for UARTs
@ 2021-09-20 17:56   ` Chen-Yu Tsai
  0 siblings, 0 replies; 18+ messages in thread
From: Chen-Yu Tsai @ 2021-09-20 17:56 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Chen-Yu Tsai, Robin Murphy, linux-arm-kernel, linux-rockchip,
	devicetree, linux-kernel

From: Chen-Yu Tsai <wens@csie.org>

The RK3399 has two DMA controllers, one of which is wired up to work
with the SPI controllers and UARTs. The SPI controllers are already
hooked up, but the UARTs aren't.

Add the "dmas" and "dma-names" to the UART device nodes to hook up DMA.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 3871c7fd83b0..87d6e4eb1337 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -608,6 +608,8 @@ uart0: serial@ff180000 {
 		reg = <0x0 0xff180000 0x0 0x100>;
 		clocks = <&cru SCLK_UART0>, <&cru PCLK_UART0>;
 		clock-names = "baudclk", "apb_pclk";
+		dmas = <&dmac_peri 0>, <&dmac_peri 1>;
+		dma-names = "tx", "rx";
 		interrupts = <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH 0>;
 		reg-shift = <2>;
 		reg-io-width = <4>;
@@ -621,6 +623,8 @@ uart1: serial@ff190000 {
 		reg = <0x0 0xff190000 0x0 0x100>;
 		clocks = <&cru SCLK_UART1>, <&cru PCLK_UART1>;
 		clock-names = "baudclk", "apb_pclk";
+		dmas = <&dmac_peri 2>, <&dmac_peri 3>;
+		dma-names = "tx", "rx";
 		interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH 0>;
 		reg-shift = <2>;
 		reg-io-width = <4>;
@@ -634,6 +638,8 @@ uart2: serial@ff1a0000 {
 		reg = <0x0 0xff1a0000 0x0 0x100>;
 		clocks = <&cru SCLK_UART2>, <&cru PCLK_UART2>;
 		clock-names = "baudclk", "apb_pclk";
+		dmas = <&dmac_peri 4>, <&dmac_peri 5>;
+		dma-names = "tx", "rx";
 		interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH 0>;
 		reg-shift = <2>;
 		reg-io-width = <4>;
@@ -647,6 +653,8 @@ uart3: serial@ff1b0000 {
 		reg = <0x0 0xff1b0000 0x0 0x100>;
 		clocks = <&cru SCLK_UART3>, <&cru PCLK_UART3>;
 		clock-names = "baudclk", "apb_pclk";
+		dmas = <&dmac_peri 6>, <&dmac_peri 7>;
+		dma-names = "tx", "rx";
 		interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH 0>;
 		reg-shift = <2>;
 		reg-io-width = <4>;
@@ -1142,6 +1150,8 @@ uart4: serial@ff370000 {
 		reg = <0x0 0xff370000 0x0 0x100>;
 		clocks = <&pmucru SCLK_UART4_PMU>, <&pmucru PCLK_UART4_PMU>;
 		clock-names = "baudclk", "apb_pclk";
+		dmas = <&dmac_peri 8>, <&dmac_peri 9>;
+		dma-names = "tx", "rx";
 		interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH 0>;
 		reg-shift = <2>;
 		reg-io-width = <4>;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: nanopi4: decrease Bluetooth UART baud rate
  2021-09-20 17:56 ` Chen-Yu Tsai
  (?)
@ 2021-10-06 10:49   ` Robin Murphy
  -1 siblings, 0 replies; 18+ messages in thread
From: Robin Murphy @ 2021-10-06 10:49 UTC (permalink / raw)
  To: Chen-Yu Tsai, Heiko Stuebner
  Cc: Chen-Yu Tsai, linux-arm-kernel, linux-rockchip, devicetree, linux-kernel

On 2021-09-20 18:56, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <wens@csie.org>
> 
> The RK3399 does not seem to be able to properly generate the required
> 64 MHz clock for the UART to operate at 4MBd.
> 
> Drop the baud rate down to 3MBd, which can be used as the clock
> controller is able to produce a 48 MHz clock.

Hmm, I've been running mine this way (with DMA) for ages now :/

Looking at clk_summary, clk_uart0_src ends up at 800MHz off CPLL (same 
as several other significant clocks), with clk_uart0 at an exact 64MHz 
as a division of that. I stuck a scope on the UART pins of the module 
and all the edges look nicely lined up to 250ns intervals.

This is with a 5.11.4 kernel, though - I wonder if the recent fractional 
divider changes in the clock driver have changed anything?

Robin.

> Fixes: 3e2f0bb72be3 ("arm64: dts: rockchip: Add nanopi4 bluetooth")
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>   arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> index 8c0ff6c96e03..45ff053b119d 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> @@ -699,7 +699,7 @@ bluetooth {
>   		device-wakeup-gpios = <&gpio2 RK_PD2 GPIO_ACTIVE_HIGH>;
>   		host-wakeup-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_HIGH>;
>   		shutdown-gpios = <&gpio0 RK_PB1 GPIO_ACTIVE_HIGH>;
> -		max-speed = <4000000>;
> +		max-speed = <3000000>;
>   		pinctrl-names = "default";
>   		pinctrl-0 = <&bt_reg_on_h &bt_host_wake_l &bt_wake_l>;
>   		vbat-supply = <&vcc3v3_sys>;
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: nanopi4: decrease Bluetooth UART baud rate
@ 2021-10-06 10:49   ` Robin Murphy
  0 siblings, 0 replies; 18+ messages in thread
From: Robin Murphy @ 2021-10-06 10:49 UTC (permalink / raw)
  To: Chen-Yu Tsai, Heiko Stuebner
  Cc: Chen-Yu Tsai, linux-arm-kernel, linux-rockchip, devicetree, linux-kernel

On 2021-09-20 18:56, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <wens@csie.org>
> 
> The RK3399 does not seem to be able to properly generate the required
> 64 MHz clock for the UART to operate at 4MBd.
> 
> Drop the baud rate down to 3MBd, which can be used as the clock
> controller is able to produce a 48 MHz clock.

Hmm, I've been running mine this way (with DMA) for ages now :/

Looking at clk_summary, clk_uart0_src ends up at 800MHz off CPLL (same 
as several other significant clocks), with clk_uart0 at an exact 64MHz 
as a division of that. I stuck a scope on the UART pins of the module 
and all the edges look nicely lined up to 250ns intervals.

This is with a 5.11.4 kernel, though - I wonder if the recent fractional 
divider changes in the clock driver have changed anything?

Robin.

> Fixes: 3e2f0bb72be3 ("arm64: dts: rockchip: Add nanopi4 bluetooth")
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>   arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> index 8c0ff6c96e03..45ff053b119d 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> @@ -699,7 +699,7 @@ bluetooth {
>   		device-wakeup-gpios = <&gpio2 RK_PD2 GPIO_ACTIVE_HIGH>;
>   		host-wakeup-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_HIGH>;
>   		shutdown-gpios = <&gpio0 RK_PB1 GPIO_ACTIVE_HIGH>;
> -		max-speed = <4000000>;
> +		max-speed = <3000000>;
>   		pinctrl-names = "default";
>   		pinctrl-0 = <&bt_reg_on_h &bt_host_wake_l &bt_wake_l>;
>   		vbat-supply = <&vcc3v3_sys>;
> 

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: nanopi4: decrease Bluetooth UART baud rate
@ 2021-10-06 10:49   ` Robin Murphy
  0 siblings, 0 replies; 18+ messages in thread
From: Robin Murphy @ 2021-10-06 10:49 UTC (permalink / raw)
  To: Chen-Yu Tsai, Heiko Stuebner
  Cc: Chen-Yu Tsai, linux-arm-kernel, linux-rockchip, devicetree, linux-kernel

On 2021-09-20 18:56, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <wens@csie.org>
> 
> The RK3399 does not seem to be able to properly generate the required
> 64 MHz clock for the UART to operate at 4MBd.
> 
> Drop the baud rate down to 3MBd, which can be used as the clock
> controller is able to produce a 48 MHz clock.

Hmm, I've been running mine this way (with DMA) for ages now :/

Looking at clk_summary, clk_uart0_src ends up at 800MHz off CPLL (same 
as several other significant clocks), with clk_uart0 at an exact 64MHz 
as a division of that. I stuck a scope on the UART pins of the module 
and all the edges look nicely lined up to 250ns intervals.

This is with a 5.11.4 kernel, though - I wonder if the recent fractional 
divider changes in the clock driver have changed anything?

Robin.

> Fixes: 3e2f0bb72be3 ("arm64: dts: rockchip: Add nanopi4 bluetooth")
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>   arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> index 8c0ff6c96e03..45ff053b119d 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> @@ -699,7 +699,7 @@ bluetooth {
>   		device-wakeup-gpios = <&gpio2 RK_PD2 GPIO_ACTIVE_HIGH>;
>   		host-wakeup-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_HIGH>;
>   		shutdown-gpios = <&gpio0 RK_PB1 GPIO_ACTIVE_HIGH>;
> -		max-speed = <4000000>;
> +		max-speed = <3000000>;
>   		pinctrl-names = "default";
>   		pinctrl-0 = <&bt_reg_on_h &bt_host_wake_l &bt_wake_l>;
>   		vbat-supply = <&vcc3v3_sys>;
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: nanopi4: decrease Bluetooth UART baud rate
  2021-10-06 10:49   ` Robin Murphy
  (?)
@ 2021-10-07 17:13     ` Chen-Yu Tsai
  -1 siblings, 0 replies; 18+ messages in thread
From: Chen-Yu Tsai @ 2021-10-07 17:13 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Heiko Stuebner, linux-arm-kernel, open list:ARM/Rockchip SoC...,
	devicetree, linux-kernel

On Wed, Oct 6, 2021 at 6:49 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2021-09-20 18:56, Chen-Yu Tsai wrote:
> > From: Chen-Yu Tsai <wens@csie.org>
> >
> > The RK3399 does not seem to be able to properly generate the required
> > 64 MHz clock for the UART to operate at 4MBd.
> >
> > Drop the baud rate down to 3MBd, which can be used as the clock
> > controller is able to produce a 48 MHz clock.
>
> Hmm, I've been running mine this way (with DMA) for ages now :/

I guess you have extra patches on top for DMA? I sent another patch
to hook up DMA.

> Looking at clk_summary, clk_uart0_src ends up at 800MHz off CPLL (same
> as several other significant clocks), with clk_uart0 at an exact 64MHz
> as a division of that. I stuck a scope on the UART pins of the module
> and all the edges look nicely lined up to 250ns intervals.

Could you provide a partial dump of /sys/kernel/debug/clk/clk_summary ?
Just the bits related to uart0 should be enough.

Mine is also running from CPLL, but ends up at 1843200 Hz, which seems
like the clock rate used for 115200 baud:

 xin24m                              24       24        0    24000000
        0     0  50000         Y
    pll_cpll                          1        1        0   800000000
        0     0  50000         Y
       cpll                           7       15        0   800000000
        0     0  50000         Y
          clk_uart0_src               1        1        0   800000000
        0     0  50000         Y
             clk_uart0_div            1        1        0   800000000
        0     0  50000         Y
                clk_uart0_frac        1        1        0     1843200
        0     0  50000         Y
                   clk_uart0          1        1        0     1843200
        0     0  50000         Y

I also observe a couple error messages:

Bluetooth: hci0: BCM: failed to write clock (-56)
Bluetooth: hci0: Failed to set baudrate
Bluetooth: hci0: BCM: chip id 130
Bluetooth: hci0: BCM: features 0x0f
Bluetooth: hci0: BCM4345C5
Bluetooth: hci0: BCM4345C5 (003.006.006) build 0000
Bluetooth: hci0: BCM4345C5 'brcm/BCM4345C5.hcd' Patch
Bluetooth: hci0: BCM: failed to write clock (-56)
Bluetooth: hci0: BCM4345C5 Ampak_CL1.5 UART 37.4 MHz BT 5.0 [Version:
Version: 0033.0080]
Bluetooth: hci0: BCM4345C5 (003.006.006) build 0080

So I think my UART is actually still running at its initial speed.

Another thing is that the Rockchip datasheet says something about the
denominator has to be 20 times larger than the nominator for a stable clock.

> This is with a 5.11.4 kernel, though - I wonder if the recent fractional
> divider changes in the clock driver have changed anything?

I tried reverting the three patches but that didn't make a difference.

Regards
ChenYu


>
> > Fixes: 3e2f0bb72be3 ("arm64: dts: rockchip: Add nanopi4 bluetooth")
> > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > ---
> >   arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> > index 8c0ff6c96e03..45ff053b119d 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> > @@ -699,7 +699,7 @@ bluetooth {
> >               device-wakeup-gpios = <&gpio2 RK_PD2 GPIO_ACTIVE_HIGH>;
> >               host-wakeup-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_HIGH>;
> >               shutdown-gpios = <&gpio0 RK_PB1 GPIO_ACTIVE_HIGH>;
> > -             max-speed = <4000000>;
> > +             max-speed = <3000000>;
> >               pinctrl-names = "default";
> >               pinctrl-0 = <&bt_reg_on_h &bt_host_wake_l &bt_wake_l>;
> >               vbat-supply = <&vcc3v3_sys>;
> >

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: nanopi4: decrease Bluetooth UART baud rate
@ 2021-10-07 17:13     ` Chen-Yu Tsai
  0 siblings, 0 replies; 18+ messages in thread
From: Chen-Yu Tsai @ 2021-10-07 17:13 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Heiko Stuebner, linux-arm-kernel, open list:ARM/Rockchip SoC...,
	devicetree, linux-kernel

On Wed, Oct 6, 2021 at 6:49 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2021-09-20 18:56, Chen-Yu Tsai wrote:
> > From: Chen-Yu Tsai <wens@csie.org>
> >
> > The RK3399 does not seem to be able to properly generate the required
> > 64 MHz clock for the UART to operate at 4MBd.
> >
> > Drop the baud rate down to 3MBd, which can be used as the clock
> > controller is able to produce a 48 MHz clock.
>
> Hmm, I've been running mine this way (with DMA) for ages now :/

I guess you have extra patches on top for DMA? I sent another patch
to hook up DMA.

> Looking at clk_summary, clk_uart0_src ends up at 800MHz off CPLL (same
> as several other significant clocks), with clk_uart0 at an exact 64MHz
> as a division of that. I stuck a scope on the UART pins of the module
> and all the edges look nicely lined up to 250ns intervals.

Could you provide a partial dump of /sys/kernel/debug/clk/clk_summary ?
Just the bits related to uart0 should be enough.

Mine is also running from CPLL, but ends up at 1843200 Hz, which seems
like the clock rate used for 115200 baud:

 xin24m                              24       24        0    24000000
        0     0  50000         Y
    pll_cpll                          1        1        0   800000000
        0     0  50000         Y
       cpll                           7       15        0   800000000
        0     0  50000         Y
          clk_uart0_src               1        1        0   800000000
        0     0  50000         Y
             clk_uart0_div            1        1        0   800000000
        0     0  50000         Y
                clk_uart0_frac        1        1        0     1843200
        0     0  50000         Y
                   clk_uart0          1        1        0     1843200
        0     0  50000         Y

I also observe a couple error messages:

Bluetooth: hci0: BCM: failed to write clock (-56)
Bluetooth: hci0: Failed to set baudrate
Bluetooth: hci0: BCM: chip id 130
Bluetooth: hci0: BCM: features 0x0f
Bluetooth: hci0: BCM4345C5
Bluetooth: hci0: BCM4345C5 (003.006.006) build 0000
Bluetooth: hci0: BCM4345C5 'brcm/BCM4345C5.hcd' Patch
Bluetooth: hci0: BCM: failed to write clock (-56)
Bluetooth: hci0: BCM4345C5 Ampak_CL1.5 UART 37.4 MHz BT 5.0 [Version:
Version: 0033.0080]
Bluetooth: hci0: BCM4345C5 (003.006.006) build 0080

So I think my UART is actually still running at its initial speed.

Another thing is that the Rockchip datasheet says something about the
denominator has to be 20 times larger than the nominator for a stable clock.

> This is with a 5.11.4 kernel, though - I wonder if the recent fractional
> divider changes in the clock driver have changed anything?

I tried reverting the three patches but that didn't make a difference.

Regards
ChenYu


>
> > Fixes: 3e2f0bb72be3 ("arm64: dts: rockchip: Add nanopi4 bluetooth")
> > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > ---
> >   arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> > index 8c0ff6c96e03..45ff053b119d 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> > @@ -699,7 +699,7 @@ bluetooth {
> >               device-wakeup-gpios = <&gpio2 RK_PD2 GPIO_ACTIVE_HIGH>;
> >               host-wakeup-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_HIGH>;
> >               shutdown-gpios = <&gpio0 RK_PB1 GPIO_ACTIVE_HIGH>;
> > -             max-speed = <4000000>;
> > +             max-speed = <3000000>;
> >               pinctrl-names = "default";
> >               pinctrl-0 = <&bt_reg_on_h &bt_host_wake_l &bt_wake_l>;
> >               vbat-supply = <&vcc3v3_sys>;
> >

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: nanopi4: decrease Bluetooth UART baud rate
@ 2021-10-07 17:13     ` Chen-Yu Tsai
  0 siblings, 0 replies; 18+ messages in thread
From: Chen-Yu Tsai @ 2021-10-07 17:13 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Heiko Stuebner, linux-arm-kernel, open list:ARM/Rockchip SoC...,
	devicetree, linux-kernel

On Wed, Oct 6, 2021 at 6:49 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2021-09-20 18:56, Chen-Yu Tsai wrote:
> > From: Chen-Yu Tsai <wens@csie.org>
> >
> > The RK3399 does not seem to be able to properly generate the required
> > 64 MHz clock for the UART to operate at 4MBd.
> >
> > Drop the baud rate down to 3MBd, which can be used as the clock
> > controller is able to produce a 48 MHz clock.
>
> Hmm, I've been running mine this way (with DMA) for ages now :/

I guess you have extra patches on top for DMA? I sent another patch
to hook up DMA.

> Looking at clk_summary, clk_uart0_src ends up at 800MHz off CPLL (same
> as several other significant clocks), with clk_uart0 at an exact 64MHz
> as a division of that. I stuck a scope on the UART pins of the module
> and all the edges look nicely lined up to 250ns intervals.

Could you provide a partial dump of /sys/kernel/debug/clk/clk_summary ?
Just the bits related to uart0 should be enough.

Mine is also running from CPLL, but ends up at 1843200 Hz, which seems
like the clock rate used for 115200 baud:

 xin24m                              24       24        0    24000000
        0     0  50000         Y
    pll_cpll                          1        1        0   800000000
        0     0  50000         Y
       cpll                           7       15        0   800000000
        0     0  50000         Y
          clk_uart0_src               1        1        0   800000000
        0     0  50000         Y
             clk_uart0_div            1        1        0   800000000
        0     0  50000         Y
                clk_uart0_frac        1        1        0     1843200
        0     0  50000         Y
                   clk_uart0          1        1        0     1843200
        0     0  50000         Y

I also observe a couple error messages:

Bluetooth: hci0: BCM: failed to write clock (-56)
Bluetooth: hci0: Failed to set baudrate
Bluetooth: hci0: BCM: chip id 130
Bluetooth: hci0: BCM: features 0x0f
Bluetooth: hci0: BCM4345C5
Bluetooth: hci0: BCM4345C5 (003.006.006) build 0000
Bluetooth: hci0: BCM4345C5 'brcm/BCM4345C5.hcd' Patch
Bluetooth: hci0: BCM: failed to write clock (-56)
Bluetooth: hci0: BCM4345C5 Ampak_CL1.5 UART 37.4 MHz BT 5.0 [Version:
Version: 0033.0080]
Bluetooth: hci0: BCM4345C5 (003.006.006) build 0080

So I think my UART is actually still running at its initial speed.

Another thing is that the Rockchip datasheet says something about the
denominator has to be 20 times larger than the nominator for a stable clock.

> This is with a 5.11.4 kernel, though - I wonder if the recent fractional
> divider changes in the clock driver have changed anything?

I tried reverting the three patches but that didn't make a difference.

Regards
ChenYu


>
> > Fixes: 3e2f0bb72be3 ("arm64: dts: rockchip: Add nanopi4 bluetooth")
> > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > ---
> >   arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> > index 8c0ff6c96e03..45ff053b119d 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> > @@ -699,7 +699,7 @@ bluetooth {
> >               device-wakeup-gpios = <&gpio2 RK_PD2 GPIO_ACTIVE_HIGH>;
> >               host-wakeup-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_HIGH>;
> >               shutdown-gpios = <&gpio0 RK_PB1 GPIO_ACTIVE_HIGH>;
> > -             max-speed = <4000000>;
> > +             max-speed = <3000000>;
> >               pinctrl-names = "default";
> >               pinctrl-0 = <&bt_reg_on_h &bt_host_wake_l &bt_wake_l>;
> >               vbat-supply = <&vcc3v3_sys>;
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: rk3399: Hook up DMA for UARTs
  2021-09-20 17:56   ` Chen-Yu Tsai
  (?)
@ 2021-10-20  8:10     ` Heiko Stuebner
  -1 siblings, 0 replies; 18+ messages in thread
From: Heiko Stuebner @ 2021-10-20  8:10 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Chen-Yu Tsai, Robin Murphy, linux-arm-kernel, linux-rockchip,
	devicetree, linux-kernel

Hi,

Am Montag, 20. September 2021, 19:56:47 CEST schrieb Chen-Yu Tsai:
> From: Chen-Yu Tsai <wens@csie.org>
> 
> The RK3399 has two DMA controllers, one of which is wired up to work
> with the SPI controllers and UARTs. The SPI controllers are already
> hooked up, but the UARTs aren't.
> 
> Add the "dmas" and "dma-names" to the UART device nodes to hook up DMA.

last time this came up, there was the issue of the pl330 driver in the
kernel not being able to handle the case where the number of channels
hooked up was larger than the number of possible channels handled
at the same time (8 for dmac peri according to the TRM).

Did this get solved meanwhile or are we then possibly starving the spi
controllers from dma access when the uarts also get dma channels
and are possibly probed earlier?


Heiko


> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 3871c7fd83b0..87d6e4eb1337 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -608,6 +608,8 @@ uart0: serial@ff180000 {
>  		reg = <0x0 0xff180000 0x0 0x100>;
>  		clocks = <&cru SCLK_UART0>, <&cru PCLK_UART0>;
>  		clock-names = "baudclk", "apb_pclk";
> +		dmas = <&dmac_peri 0>, <&dmac_peri 1>;
> +		dma-names = "tx", "rx";
>  		interrupts = <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH 0>;
>  		reg-shift = <2>;
>  		reg-io-width = <4>;
> @@ -621,6 +623,8 @@ uart1: serial@ff190000 {
>  		reg = <0x0 0xff190000 0x0 0x100>;
>  		clocks = <&cru SCLK_UART1>, <&cru PCLK_UART1>;
>  		clock-names = "baudclk", "apb_pclk";
> +		dmas = <&dmac_peri 2>, <&dmac_peri 3>;
> +		dma-names = "tx", "rx";
>  		interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH 0>;
>  		reg-shift = <2>;
>  		reg-io-width = <4>;
> @@ -634,6 +638,8 @@ uart2: serial@ff1a0000 {
>  		reg = <0x0 0xff1a0000 0x0 0x100>;
>  		clocks = <&cru SCLK_UART2>, <&cru PCLK_UART2>;
>  		clock-names = "baudclk", "apb_pclk";
> +		dmas = <&dmac_peri 4>, <&dmac_peri 5>;
> +		dma-names = "tx", "rx";
>  		interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH 0>;
>  		reg-shift = <2>;
>  		reg-io-width = <4>;
> @@ -647,6 +653,8 @@ uart3: serial@ff1b0000 {
>  		reg = <0x0 0xff1b0000 0x0 0x100>;
>  		clocks = <&cru SCLK_UART3>, <&cru PCLK_UART3>;
>  		clock-names = "baudclk", "apb_pclk";
> +		dmas = <&dmac_peri 6>, <&dmac_peri 7>;
> +		dma-names = "tx", "rx";
>  		interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH 0>;
>  		reg-shift = <2>;
>  		reg-io-width = <4>;
> @@ -1142,6 +1150,8 @@ uart4: serial@ff370000 {
>  		reg = <0x0 0xff370000 0x0 0x100>;
>  		clocks = <&pmucru SCLK_UART4_PMU>, <&pmucru PCLK_UART4_PMU>;
>  		clock-names = "baudclk", "apb_pclk";
> +		dmas = <&dmac_peri 8>, <&dmac_peri 9>;
> +		dma-names = "tx", "rx";
>  		interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH 0>;
>  		reg-shift = <2>;
>  		reg-io-width = <4>;
> 





^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: rk3399: Hook up DMA for UARTs
@ 2021-10-20  8:10     ` Heiko Stuebner
  0 siblings, 0 replies; 18+ messages in thread
From: Heiko Stuebner @ 2021-10-20  8:10 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Chen-Yu Tsai, Robin Murphy, linux-arm-kernel, linux-rockchip,
	devicetree, linux-kernel

Hi,

Am Montag, 20. September 2021, 19:56:47 CEST schrieb Chen-Yu Tsai:
> From: Chen-Yu Tsai <wens@csie.org>
> 
> The RK3399 has two DMA controllers, one of which is wired up to work
> with the SPI controllers and UARTs. The SPI controllers are already
> hooked up, but the UARTs aren't.
> 
> Add the "dmas" and "dma-names" to the UART device nodes to hook up DMA.

last time this came up, there was the issue of the pl330 driver in the
kernel not being able to handle the case where the number of channels
hooked up was larger than the number of possible channels handled
at the same time (8 for dmac peri according to the TRM).

Did this get solved meanwhile or are we then possibly starving the spi
controllers from dma access when the uarts also get dma channels
and are possibly probed earlier?


Heiko


> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 3871c7fd83b0..87d6e4eb1337 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -608,6 +608,8 @@ uart0: serial@ff180000 {
>  		reg = <0x0 0xff180000 0x0 0x100>;
>  		clocks = <&cru SCLK_UART0>, <&cru PCLK_UART0>;
>  		clock-names = "baudclk", "apb_pclk";
> +		dmas = <&dmac_peri 0>, <&dmac_peri 1>;
> +		dma-names = "tx", "rx";
>  		interrupts = <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH 0>;
>  		reg-shift = <2>;
>  		reg-io-width = <4>;
> @@ -621,6 +623,8 @@ uart1: serial@ff190000 {
>  		reg = <0x0 0xff190000 0x0 0x100>;
>  		clocks = <&cru SCLK_UART1>, <&cru PCLK_UART1>;
>  		clock-names = "baudclk", "apb_pclk";
> +		dmas = <&dmac_peri 2>, <&dmac_peri 3>;
> +		dma-names = "tx", "rx";
>  		interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH 0>;
>  		reg-shift = <2>;
>  		reg-io-width = <4>;
> @@ -634,6 +638,8 @@ uart2: serial@ff1a0000 {
>  		reg = <0x0 0xff1a0000 0x0 0x100>;
>  		clocks = <&cru SCLK_UART2>, <&cru PCLK_UART2>;
>  		clock-names = "baudclk", "apb_pclk";
> +		dmas = <&dmac_peri 4>, <&dmac_peri 5>;
> +		dma-names = "tx", "rx";
>  		interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH 0>;
>  		reg-shift = <2>;
>  		reg-io-width = <4>;
> @@ -647,6 +653,8 @@ uart3: serial@ff1b0000 {
>  		reg = <0x0 0xff1b0000 0x0 0x100>;
>  		clocks = <&cru SCLK_UART3>, <&cru PCLK_UART3>;
>  		clock-names = "baudclk", "apb_pclk";
> +		dmas = <&dmac_peri 6>, <&dmac_peri 7>;
> +		dma-names = "tx", "rx";
>  		interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH 0>;
>  		reg-shift = <2>;
>  		reg-io-width = <4>;
> @@ -1142,6 +1150,8 @@ uart4: serial@ff370000 {
>  		reg = <0x0 0xff370000 0x0 0x100>;
>  		clocks = <&pmucru SCLK_UART4_PMU>, <&pmucru PCLK_UART4_PMU>;
>  		clock-names = "baudclk", "apb_pclk";
> +		dmas = <&dmac_peri 8>, <&dmac_peri 9>;
> +		dma-names = "tx", "rx";
>  		interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH 0>;
>  		reg-shift = <2>;
>  		reg-io-width = <4>;
> 





_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: rk3399: Hook up DMA for UARTs
@ 2021-10-20  8:10     ` Heiko Stuebner
  0 siblings, 0 replies; 18+ messages in thread
From: Heiko Stuebner @ 2021-10-20  8:10 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Chen-Yu Tsai, Robin Murphy, linux-arm-kernel, linux-rockchip,
	devicetree, linux-kernel

Hi,

Am Montag, 20. September 2021, 19:56:47 CEST schrieb Chen-Yu Tsai:
> From: Chen-Yu Tsai <wens@csie.org>
> 
> The RK3399 has two DMA controllers, one of which is wired up to work
> with the SPI controllers and UARTs. The SPI controllers are already
> hooked up, but the UARTs aren't.
> 
> Add the "dmas" and "dma-names" to the UART device nodes to hook up DMA.

last time this came up, there was the issue of the pl330 driver in the
kernel not being able to handle the case where the number of channels
hooked up was larger than the number of possible channels handled
at the same time (8 for dmac peri according to the TRM).

Did this get solved meanwhile or are we then possibly starving the spi
controllers from dma access when the uarts also get dma channels
and are possibly probed earlier?


Heiko


> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 3871c7fd83b0..87d6e4eb1337 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -608,6 +608,8 @@ uart0: serial@ff180000 {
>  		reg = <0x0 0xff180000 0x0 0x100>;
>  		clocks = <&cru SCLK_UART0>, <&cru PCLK_UART0>;
>  		clock-names = "baudclk", "apb_pclk";
> +		dmas = <&dmac_peri 0>, <&dmac_peri 1>;
> +		dma-names = "tx", "rx";
>  		interrupts = <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH 0>;
>  		reg-shift = <2>;
>  		reg-io-width = <4>;
> @@ -621,6 +623,8 @@ uart1: serial@ff190000 {
>  		reg = <0x0 0xff190000 0x0 0x100>;
>  		clocks = <&cru SCLK_UART1>, <&cru PCLK_UART1>;
>  		clock-names = "baudclk", "apb_pclk";
> +		dmas = <&dmac_peri 2>, <&dmac_peri 3>;
> +		dma-names = "tx", "rx";
>  		interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH 0>;
>  		reg-shift = <2>;
>  		reg-io-width = <4>;
> @@ -634,6 +638,8 @@ uart2: serial@ff1a0000 {
>  		reg = <0x0 0xff1a0000 0x0 0x100>;
>  		clocks = <&cru SCLK_UART2>, <&cru PCLK_UART2>;
>  		clock-names = "baudclk", "apb_pclk";
> +		dmas = <&dmac_peri 4>, <&dmac_peri 5>;
> +		dma-names = "tx", "rx";
>  		interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH 0>;
>  		reg-shift = <2>;
>  		reg-io-width = <4>;
> @@ -647,6 +653,8 @@ uart3: serial@ff1b0000 {
>  		reg = <0x0 0xff1b0000 0x0 0x100>;
>  		clocks = <&cru SCLK_UART3>, <&cru PCLK_UART3>;
>  		clock-names = "baudclk", "apb_pclk";
> +		dmas = <&dmac_peri 6>, <&dmac_peri 7>;
> +		dma-names = "tx", "rx";
>  		interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH 0>;
>  		reg-shift = <2>;
>  		reg-io-width = <4>;
> @@ -1142,6 +1150,8 @@ uart4: serial@ff370000 {
>  		reg = <0x0 0xff370000 0x0 0x100>;
>  		clocks = <&pmucru SCLK_UART4_PMU>, <&pmucru PCLK_UART4_PMU>;
>  		clock-names = "baudclk", "apb_pclk";
> +		dmas = <&dmac_peri 8>, <&dmac_peri 9>;
> +		dma-names = "tx", "rx";
>  		interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH 0>;
>  		reg-shift = <2>;
>  		reg-io-width = <4>;
> 





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: nanopi4: decrease Bluetooth UART baud rate
  2021-10-07 17:13     ` Chen-Yu Tsai
  (?)
@ 2021-10-20 12:57       ` Robin Murphy
  -1 siblings, 0 replies; 18+ messages in thread
From: Robin Murphy @ 2021-10-20 12:57 UTC (permalink / raw)
  To: wens, Heiko Stuebner
  Cc: linux-arm-kernel, open list:ARM/Rockchip SoC...,
	devicetree, linux-kernel

On 2021-10-07 18:13, Chen-Yu Tsai wrote:
> On Wed, Oct 6, 2021 at 6:49 PM Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> On 2021-09-20 18:56, Chen-Yu Tsai wrote:
>>> From: Chen-Yu Tsai <wens@csie.org>
>>>
>>> The RK3399 does not seem to be able to properly generate the required
>>> 64 MHz clock for the UART to operate at 4MBd.
>>>
>>> Drop the baud rate down to 3MBd, which can be used as the clock
>>> controller is able to produce a 48 MHz clock.
>>
>> Hmm, I've been running mine this way (with DMA) for ages now :/
> 
> I guess you have extra patches on top for DMA? I sent another patch
> to hook up DMA.

Indeed I've been using Katsuhiro's patch from ages ago:

https://lore.kernel.org/linux-rockchip/20190321162244.10080-1-katsuhiro@katsuster.net/

Heiko - as far as I'm aware pl330 still isn't hooked up to virt-dma; I 
think the core change to avoid using DMA for the console UART has merely 
mitigated the issue by leaving just enough channels available for "most" 
use-cases.

>> Looking at clk_summary, clk_uart0_src ends up at 800MHz off CPLL (same
>> as several other significant clocks), with clk_uart0 at an exact 64MHz
>> as a division of that. I stuck a scope on the UART pins of the module
>> and all the edges look nicely lined up to 250ns intervals.
> 
> Could you provide a partial dump of /sys/kernel/debug/clk/clk_summary ?
> Just the bits related to uart0 should be enough.
> 
> Mine is also running from CPLL, but ends up at 1843200 Hz, which seems
> like the clock rate used for 115200 baud:
> 
>   xin24m                              24       24        0    24000000
>          0     0  50000         Y
>      pll_cpll                          1        1        0   800000000
>          0     0  50000         Y
>         cpll                           7       15        0   800000000
>          0     0  50000         Y
>            clk_uart0_src               1        1        0   800000000
>          0     0  50000         Y
>               clk_uart0_div            1        1        0   800000000
>          0     0  50000         Y
>                  clk_uart0_frac        1        1        0     1843200
>          0     0  50000         Y
>                     clk_uart0          1        1        0     1843200
>          0     0  50000         Y
> 
> I also observe a couple error messages:
> 
> Bluetooth: hci0: BCM: failed to write clock (-56)
> Bluetooth: hci0: Failed to set baudrate
> Bluetooth: hci0: BCM: chip id 130
> Bluetooth: hci0: BCM: features 0x0f
> Bluetooth: hci0: BCM4345C5
> Bluetooth: hci0: BCM4345C5 (003.006.006) build 0000
> Bluetooth: hci0: BCM4345C5 'brcm/BCM4345C5.hcd' Patch
> Bluetooth: hci0: BCM: failed to write clock (-56)
> Bluetooth: hci0: BCM4345C5 Ampak_CL1.5 UART 37.4 MHz BT 5.0 [Version:
> Version: 0033.0080]
> Bluetooth: hci0: BCM4345C5 (003.006.006) build 0080
> 
> So I think my UART is actually still running at its initial speed.

FWIW, with a functioning 5.14 kernel back on my board (5.15-rc4 couldn't 
even boot due to eMMC issues), nothing stands out except the lack of 
errors. However, looking again at said errors and digging in to 
bcm_set_baudrate(), it looks like it's actually the module itself being 
unhappy about the special command to update *its* clock, which then 
prevents the host baud rate from being changed from its initial 115200. 
The internet leads me to believe that BCM4345C5 is BCM43455 which is now 
CYW43555 whose datasheet says "UART baud rates up to 4 Mbps", so 
whatever the issue is it looks to be beyond the limit of my knowledge.

Poking around hci_bcm to see if there's any notable difference between 
the modules, I spotted commit e601daed271e mentioning issues with 
AP6256, so I wonder if using the "brcm,bcm4345c5" compatible as an 
override in the M4B DTS makes any difference?

If not, now that things seem to make a bit more sense it might be 
reasonable to tweak the speed for M4B as a workaround to avoid sending 
the offending command (since AP6356S as used on the other variants seems 
unaffected), unless Bluetooth people can shed any more light on why the 
module is unhappy about it.

Cheers,
Robin.

> Another thing is that the Rockchip datasheet says something about the
> denominator has to be 20 times larger than the nominator for a stable clock.
> 
>> This is with a 5.11.4 kernel, though - I wonder if the recent fractional
>> divider changes in the clock driver have changed anything?
> 
> I tried reverting the three patches but that didn't make a difference.
> 
> Regards
> ChenYu
> 
> 
>>
>>> Fixes: 3e2f0bb72be3 ("arm64: dts: rockchip: Add nanopi4 bluetooth")
>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>>> ---
>>>    arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
>>> index 8c0ff6c96e03..45ff053b119d 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
>>> @@ -699,7 +699,7 @@ bluetooth {
>>>                device-wakeup-gpios = <&gpio2 RK_PD2 GPIO_ACTIVE_HIGH>;
>>>                host-wakeup-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_HIGH>;
>>>                shutdown-gpios = <&gpio0 RK_PB1 GPIO_ACTIVE_HIGH>;
>>> -             max-speed = <4000000>;
>>> +             max-speed = <3000000>;
>>>                pinctrl-names = "default";
>>>                pinctrl-0 = <&bt_reg_on_h &bt_host_wake_l &bt_wake_l>;
>>>                vbat-supply = <&vcc3v3_sys>;
>>>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: nanopi4: decrease Bluetooth UART baud rate
@ 2021-10-20 12:57       ` Robin Murphy
  0 siblings, 0 replies; 18+ messages in thread
From: Robin Murphy @ 2021-10-20 12:57 UTC (permalink / raw)
  To: wens, Heiko Stuebner
  Cc: linux-arm-kernel, open list:ARM/Rockchip SoC...,
	devicetree, linux-kernel

On 2021-10-07 18:13, Chen-Yu Tsai wrote:
> On Wed, Oct 6, 2021 at 6:49 PM Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> On 2021-09-20 18:56, Chen-Yu Tsai wrote:
>>> From: Chen-Yu Tsai <wens@csie.org>
>>>
>>> The RK3399 does not seem to be able to properly generate the required
>>> 64 MHz clock for the UART to operate at 4MBd.
>>>
>>> Drop the baud rate down to 3MBd, which can be used as the clock
>>> controller is able to produce a 48 MHz clock.
>>
>> Hmm, I've been running mine this way (with DMA) for ages now :/
> 
> I guess you have extra patches on top for DMA? I sent another patch
> to hook up DMA.

Indeed I've been using Katsuhiro's patch from ages ago:

https://lore.kernel.org/linux-rockchip/20190321162244.10080-1-katsuhiro@katsuster.net/

Heiko - as far as I'm aware pl330 still isn't hooked up to virt-dma; I 
think the core change to avoid using DMA for the console UART has merely 
mitigated the issue by leaving just enough channels available for "most" 
use-cases.

>> Looking at clk_summary, clk_uart0_src ends up at 800MHz off CPLL (same
>> as several other significant clocks), with clk_uart0 at an exact 64MHz
>> as a division of that. I stuck a scope on the UART pins of the module
>> and all the edges look nicely lined up to 250ns intervals.
> 
> Could you provide a partial dump of /sys/kernel/debug/clk/clk_summary ?
> Just the bits related to uart0 should be enough.
> 
> Mine is also running from CPLL, but ends up at 1843200 Hz, which seems
> like the clock rate used for 115200 baud:
> 
>   xin24m                              24       24        0    24000000
>          0     0  50000         Y
>      pll_cpll                          1        1        0   800000000
>          0     0  50000         Y
>         cpll                           7       15        0   800000000
>          0     0  50000         Y
>            clk_uart0_src               1        1        0   800000000
>          0     0  50000         Y
>               clk_uart0_div            1        1        0   800000000
>          0     0  50000         Y
>                  clk_uart0_frac        1        1        0     1843200
>          0     0  50000         Y
>                     clk_uart0          1        1        0     1843200
>          0     0  50000         Y
> 
> I also observe a couple error messages:
> 
> Bluetooth: hci0: BCM: failed to write clock (-56)
> Bluetooth: hci0: Failed to set baudrate
> Bluetooth: hci0: BCM: chip id 130
> Bluetooth: hci0: BCM: features 0x0f
> Bluetooth: hci0: BCM4345C5
> Bluetooth: hci0: BCM4345C5 (003.006.006) build 0000
> Bluetooth: hci0: BCM4345C5 'brcm/BCM4345C5.hcd' Patch
> Bluetooth: hci0: BCM: failed to write clock (-56)
> Bluetooth: hci0: BCM4345C5 Ampak_CL1.5 UART 37.4 MHz BT 5.0 [Version:
> Version: 0033.0080]
> Bluetooth: hci0: BCM4345C5 (003.006.006) build 0080
> 
> So I think my UART is actually still running at its initial speed.

FWIW, with a functioning 5.14 kernel back on my board (5.15-rc4 couldn't 
even boot due to eMMC issues), nothing stands out except the lack of 
errors. However, looking again at said errors and digging in to 
bcm_set_baudrate(), it looks like it's actually the module itself being 
unhappy about the special command to update *its* clock, which then 
prevents the host baud rate from being changed from its initial 115200. 
The internet leads me to believe that BCM4345C5 is BCM43455 which is now 
CYW43555 whose datasheet says "UART baud rates up to 4 Mbps", so 
whatever the issue is it looks to be beyond the limit of my knowledge.

Poking around hci_bcm to see if there's any notable difference between 
the modules, I spotted commit e601daed271e mentioning issues with 
AP6256, so I wonder if using the "brcm,bcm4345c5" compatible as an 
override in the M4B DTS makes any difference?

If not, now that things seem to make a bit more sense it might be 
reasonable to tweak the speed for M4B as a workaround to avoid sending 
the offending command (since AP6356S as used on the other variants seems 
unaffected), unless Bluetooth people can shed any more light on why the 
module is unhappy about it.

Cheers,
Robin.

> Another thing is that the Rockchip datasheet says something about the
> denominator has to be 20 times larger than the nominator for a stable clock.
> 
>> This is with a 5.11.4 kernel, though - I wonder if the recent fractional
>> divider changes in the clock driver have changed anything?
> 
> I tried reverting the three patches but that didn't make a difference.
> 
> Regards
> ChenYu
> 
> 
>>
>>> Fixes: 3e2f0bb72be3 ("arm64: dts: rockchip: Add nanopi4 bluetooth")
>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>>> ---
>>>    arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
>>> index 8c0ff6c96e03..45ff053b119d 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
>>> @@ -699,7 +699,7 @@ bluetooth {
>>>                device-wakeup-gpios = <&gpio2 RK_PD2 GPIO_ACTIVE_HIGH>;
>>>                host-wakeup-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_HIGH>;
>>>                shutdown-gpios = <&gpio0 RK_PB1 GPIO_ACTIVE_HIGH>;
>>> -             max-speed = <4000000>;
>>> +             max-speed = <3000000>;
>>>                pinctrl-names = "default";
>>>                pinctrl-0 = <&bt_reg_on_h &bt_host_wake_l &bt_wake_l>;
>>>                vbat-supply = <&vcc3v3_sys>;
>>>

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: nanopi4: decrease Bluetooth UART baud rate
@ 2021-10-20 12:57       ` Robin Murphy
  0 siblings, 0 replies; 18+ messages in thread
From: Robin Murphy @ 2021-10-20 12:57 UTC (permalink / raw)
  To: wens, Heiko Stuebner
  Cc: linux-arm-kernel, open list:ARM/Rockchip SoC...,
	devicetree, linux-kernel

On 2021-10-07 18:13, Chen-Yu Tsai wrote:
> On Wed, Oct 6, 2021 at 6:49 PM Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> On 2021-09-20 18:56, Chen-Yu Tsai wrote:
>>> From: Chen-Yu Tsai <wens@csie.org>
>>>
>>> The RK3399 does not seem to be able to properly generate the required
>>> 64 MHz clock for the UART to operate at 4MBd.
>>>
>>> Drop the baud rate down to 3MBd, which can be used as the clock
>>> controller is able to produce a 48 MHz clock.
>>
>> Hmm, I've been running mine this way (with DMA) for ages now :/
> 
> I guess you have extra patches on top for DMA? I sent another patch
> to hook up DMA.

Indeed I've been using Katsuhiro's patch from ages ago:

https://lore.kernel.org/linux-rockchip/20190321162244.10080-1-katsuhiro@katsuster.net/

Heiko - as far as I'm aware pl330 still isn't hooked up to virt-dma; I 
think the core change to avoid using DMA for the console UART has merely 
mitigated the issue by leaving just enough channels available for "most" 
use-cases.

>> Looking at clk_summary, clk_uart0_src ends up at 800MHz off CPLL (same
>> as several other significant clocks), with clk_uart0 at an exact 64MHz
>> as a division of that. I stuck a scope on the UART pins of the module
>> and all the edges look nicely lined up to 250ns intervals.
> 
> Could you provide a partial dump of /sys/kernel/debug/clk/clk_summary ?
> Just the bits related to uart0 should be enough.
> 
> Mine is also running from CPLL, but ends up at 1843200 Hz, which seems
> like the clock rate used for 115200 baud:
> 
>   xin24m                              24       24        0    24000000
>          0     0  50000         Y
>      pll_cpll                          1        1        0   800000000
>          0     0  50000         Y
>         cpll                           7       15        0   800000000
>          0     0  50000         Y
>            clk_uart0_src               1        1        0   800000000
>          0     0  50000         Y
>               clk_uart0_div            1        1        0   800000000
>          0     0  50000         Y
>                  clk_uart0_frac        1        1        0     1843200
>          0     0  50000         Y
>                     clk_uart0          1        1        0     1843200
>          0     0  50000         Y
> 
> I also observe a couple error messages:
> 
> Bluetooth: hci0: BCM: failed to write clock (-56)
> Bluetooth: hci0: Failed to set baudrate
> Bluetooth: hci0: BCM: chip id 130
> Bluetooth: hci0: BCM: features 0x0f
> Bluetooth: hci0: BCM4345C5
> Bluetooth: hci0: BCM4345C5 (003.006.006) build 0000
> Bluetooth: hci0: BCM4345C5 'brcm/BCM4345C5.hcd' Patch
> Bluetooth: hci0: BCM: failed to write clock (-56)
> Bluetooth: hci0: BCM4345C5 Ampak_CL1.5 UART 37.4 MHz BT 5.0 [Version:
> Version: 0033.0080]
> Bluetooth: hci0: BCM4345C5 (003.006.006) build 0080
> 
> So I think my UART is actually still running at its initial speed.

FWIW, with a functioning 5.14 kernel back on my board (5.15-rc4 couldn't 
even boot due to eMMC issues), nothing stands out except the lack of 
errors. However, looking again at said errors and digging in to 
bcm_set_baudrate(), it looks like it's actually the module itself being 
unhappy about the special command to update *its* clock, which then 
prevents the host baud rate from being changed from its initial 115200. 
The internet leads me to believe that BCM4345C5 is BCM43455 which is now 
CYW43555 whose datasheet says "UART baud rates up to 4 Mbps", so 
whatever the issue is it looks to be beyond the limit of my knowledge.

Poking around hci_bcm to see if there's any notable difference between 
the modules, I spotted commit e601daed271e mentioning issues with 
AP6256, so I wonder if using the "brcm,bcm4345c5" compatible as an 
override in the M4B DTS makes any difference?

If not, now that things seem to make a bit more sense it might be 
reasonable to tweak the speed for M4B as a workaround to avoid sending 
the offending command (since AP6356S as used on the other variants seems 
unaffected), unless Bluetooth people can shed any more light on why the 
module is unhappy about it.

Cheers,
Robin.

> Another thing is that the Rockchip datasheet says something about the
> denominator has to be 20 times larger than the nominator for a stable clock.
> 
>> This is with a 5.11.4 kernel, though - I wonder if the recent fractional
>> divider changes in the clock driver have changed anything?
> 
> I tried reverting the three patches but that didn't make a difference.
> 
> Regards
> ChenYu
> 
> 
>>
>>> Fixes: 3e2f0bb72be3 ("arm64: dts: rockchip: Add nanopi4 bluetooth")
>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>>> ---
>>>    arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
>>> index 8c0ff6c96e03..45ff053b119d 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
>>> @@ -699,7 +699,7 @@ bluetooth {
>>>                device-wakeup-gpios = <&gpio2 RK_PD2 GPIO_ACTIVE_HIGH>;
>>>                host-wakeup-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_HIGH>;
>>>                shutdown-gpios = <&gpio0 RK_PB1 GPIO_ACTIVE_HIGH>;
>>> -             max-speed = <4000000>;
>>> +             max-speed = <3000000>;
>>>                pinctrl-names = "default";
>>>                pinctrl-0 = <&bt_reg_on_h &bt_host_wake_l &bt_wake_l>;
>>>                vbat-supply = <&vcc3v3_sys>;
>>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-10-20 12:58 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-20 17:56 [PATCH] arm64: dts: rockchip: nanopi4: decrease Bluetooth UART baud rate Chen-Yu Tsai
2021-09-20 17:56 ` Chen-Yu Tsai
2021-09-20 17:56 ` Chen-Yu Tsai
2021-09-20 17:56 ` [PATCH] arm64: dts: rockchip: rk3399: Hook up DMA for UARTs Chen-Yu Tsai
2021-09-20 17:56   ` Chen-Yu Tsai
2021-09-20 17:56   ` Chen-Yu Tsai
2021-10-20  8:10   ` Heiko Stuebner
2021-10-20  8:10     ` Heiko Stuebner
2021-10-20  8:10     ` Heiko Stuebner
2021-10-06 10:49 ` [PATCH] arm64: dts: rockchip: nanopi4: decrease Bluetooth UART baud rate Robin Murphy
2021-10-06 10:49   ` Robin Murphy
2021-10-06 10:49   ` Robin Murphy
2021-10-07 17:13   ` Chen-Yu Tsai
2021-10-07 17:13     ` Chen-Yu Tsai
2021-10-07 17:13     ` Chen-Yu Tsai
2021-10-20 12:57     ` Robin Murphy
2021-10-20 12:57       ` Robin Murphy
2021-10-20 12:57       ` Robin Murphy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.