arm64: dts: rockchip: rk356x: Add dma-names to UART device nodes

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

At least one implementation, Linux, requires "dma-names" properties
be used together with "dmas" to describe DMA resources. These are
currently missing, causing DMA to not be used for UARTs.

Add "dma-names" to the UART device nodes.

Fixes: a3adc0b9071d ("arm64: dts: rockchip: add core dtsi for RK3568 SoC")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm64/boot/dts/rockchip/rk356x.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)
  

On Sun, Nov 6, 2022 at 11:15 AM Chen-Yu Tsai <wens@kernel.org> wrote:
>
> From: Chen-Yu Tsai <wens@csie.org>
>
> At least one implementation, Linux, requires "dma-names" properties
> be used together with "dmas" to describe DMA resources. These are
> currently missing, causing DMA to not be used for UARTs.
>
> Add "dma-names" to the UART device nodes.
>
> Fixes: a3adc0b9071d ("arm64: dts: rockchip: add core dtsi for RK3568 SoC")
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Enabling dma globally can cause some interesting issues, have you
tested this fully?

> ---
>  arch/arm64/boot/dts/rockchip/rk356x.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> index 5706c3e24f0a..5cd55487c20e 100644
> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> @@ -447,6 +447,7 @@ uart0: serial@fdd50000 {
>                 clocks = <&pmucru SCLK_UART0>, <&pmucru PCLK_UART0>;
>                 clock-names = "baudclk", "apb_pclk";
>                 dmas = <&dmac0 0>, <&dmac0 1>;
> +               dma-names = "tx", "rx";
>                 pinctrl-0 = <&uart0_xfer>;
>                 pinctrl-names = "default";
>                 reg-io-width = <4>;
> @@ -1326,6 +1327,7 @@ uart1: serial@fe650000 {
>                 clocks = <&cru SCLK_UART1>, <&cru PCLK_UART1>;
>                 clock-names = "baudclk", "apb_pclk";
>                 dmas = <&dmac0 2>, <&dmac0 3>;
> +               dma-names = "tx", "rx";
>                 pinctrl-0 = <&uart1m0_xfer>;
>                 pinctrl-names = "default";
>                 reg-io-width = <4>;
> @@ -1340,6 +1342,7 @@ uart2: serial@fe660000 {
>                 clocks = <&cru SCLK_UART2>, <&cru PCLK_UART2>;
>                 clock-names = "baudclk", "apb_pclk";
>                 dmas = <&dmac0 4>, <&dmac0 5>;
> +               dma-names = "tx", "rx";
>                 pinctrl-0 = <&uart2m0_xfer>;
>                 pinctrl-names = "default";
>                 reg-io-width = <4>;
> @@ -1354,6 +1357,7 @@ uart3: serial@fe670000 {
>                 clocks = <&cru SCLK_UART3>, <&cru PCLK_UART3>;
>                 clock-names = "baudclk", "apb_pclk";
>                 dmas = <&dmac0 6>, <&dmac0 7>;
> +               dma-names = "tx", "rx";
>                 pinctrl-0 = <&uart3m0_xfer>;
>                 pinctrl-names = "default";
>                 reg-io-width = <4>;
> @@ -1368,6 +1372,7 @@ uart4: serial@fe680000 {
>                 clocks = <&cru SCLK_UART4>, <&cru PCLK_UART4>;
>                 clock-names = "baudclk", "apb_pclk";
>                 dmas = <&dmac0 8>, <&dmac0 9>;
> +               dma-names = "tx", "rx";
>                 pinctrl-0 = <&uart4m0_xfer>;
>                 pinctrl-names = "default";
>                 reg-io-width = <4>;
> @@ -1382,6 +1387,7 @@ uart5: serial@fe690000 {
>                 clocks = <&cru SCLK_UART5>, <&cru PCLK_UART5>;
>                 clock-names = "baudclk", "apb_pclk";
>                 dmas = <&dmac0 10>, <&dmac0 11>;
> +               dma-names = "tx", "rx";
>                 pinctrl-0 = <&uart5m0_xfer>;
>                 pinctrl-names = "default";
>                 reg-io-width = <4>;
> @@ -1396,6 +1402,7 @@ uart6: serial@fe6a0000 {
>                 clocks = <&cru SCLK_UART6>, <&cru PCLK_UART6>;
>                 clock-names = "baudclk", "apb_pclk";
>                 dmas = <&dmac0 12>, <&dmac0 13>;
> +               dma-names = "tx", "rx";
>                 pinctrl-0 = <&uart6m0_xfer>;
>                 pinctrl-names = "default";
>                 reg-io-width = <4>;
> @@ -1410,6 +1417,7 @@ uart7: serial@fe6b0000 {
>                 clocks = <&cru SCLK_UART7>, <&cru PCLK_UART7>;
>                 clock-names = "baudclk", "apb_pclk";
>                 dmas = <&dmac0 14>, <&dmac0 15>;
> +               dma-names = "tx", "rx";
>                 pinctrl-0 = <&uart7m0_xfer>;
>                 pinctrl-names = "default";
>                 reg-io-width = <4>;
> @@ -1424,6 +1432,7 @@ uart8: serial@fe6c0000 {
>                 clocks = <&cru SCLK_UART8>, <&cru PCLK_UART8>;
>                 clock-names = "baudclk", "apb_pclk";
>                 dmas = <&dmac0 16>, <&dmac0 17>;
> +               dma-names = "tx", "rx";
>                 pinctrl-0 = <&uart8m0_xfer>;
>                 pinctrl-names = "default";
>                 reg-io-width = <4>;
> @@ -1438,6 +1447,7 @@ uart9: serial@fe6d0000 {
>                 clocks = <&cru SCLK_UART9>, <&cru PCLK_UART9>;
>                 clock-names = "baudclk", "apb_pclk";
>                 dmas = <&dmac0 18>, <&dmac0 19>;
> +               dma-names = "tx", "rx";
>                 pinctrl-0 = <&uart9m0_xfer>;
>                 pinctrl-names = "default";
>                 reg-io-width = <4>;
> --
> 2.34.1
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
  

On Mon, Nov 7, 2022 at 8:52 AM Peter Geis <pgwipeout@gmail.com> wrote:
>
> On Sun, Nov 6, 2022 at 11:15 AM Chen-Yu Tsai <wens@kernel.org> wrote:
> >
> > From: Chen-Yu Tsai <wens@csie.org>
> >
> > At least one implementation, Linux, requires "dma-names" properties
> > be used together with "dmas" to describe DMA resources. These are
> > currently missing, causing DMA to not be used for UARTs.
> >
> > Add "dma-names" to the UART device nodes.
> >
> > Fixes: a3adc0b9071d ("arm64: dts: rockchip: add core dtsi for RK3568 SoC")
> > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>
> Enabling dma globally can cause some interesting issues, have you
> tested this fully?

It seems to work OK with the Bluetooth controller, though I'm not running
extensive transfers over it. Scanning both traditional and LE works, and
does exercise the DMA controller.

If your worried about the DMA controller running out of channels and
impacting other peripherals, the DMA controller for the UARTs is only
shared with other UARTs, SPI, and PWM (which the kernel doesn't support
DMAing to). The UART and SPI drivers can fall back to PIO if DMA isn't
available.

So this isn't like on the RK3399 and prior chips, where it was shared with
audio, and exhausting the DMA channels would cause audio to not probe.

Regards
ChenYu
  

On Sun, Nov 6, 2022 at 8:28 PM Chen-Yu Tsai <wens@kernel.org> wrote:
>
> On Mon, Nov 7, 2022 at 8:52 AM Peter Geis <pgwipeout@gmail.com> wrote:
> >
> > On Sun, Nov 6, 2022 at 11:15 AM Chen-Yu Tsai <wens@kernel.org> wrote:
> > >
> > > From: Chen-Yu Tsai <wens@csie.org>
> > >
> > > At least one implementation, Linux, requires "dma-names" properties
> > > be used together with "dmas" to describe DMA resources. These are
> > > currently missing, causing DMA to not be used for UARTs.
> > >
> > > Add "dma-names" to the UART device nodes.
> > >
> > > Fixes: a3adc0b9071d ("arm64: dts: rockchip: add core dtsi for RK3568 SoC")
> > > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> >
> > Enabling dma globally can cause some interesting issues, have you
> > tested this fully?
>
> It seems to work OK with the Bluetooth controller, though I'm not running
> extensive transfers over it. Scanning both traditional and LE works, and
> does exercise the DMA controller.
>
> If your worried about the DMA controller running out of channels and
> impacting other peripherals, the DMA controller for the UARTs is only
> shared with other UARTs, SPI, and PWM (which the kernel doesn't support
> DMAing to). The UART and SPI drivers can fall back to PIO if DMA isn't
> available.

Nah, enabling it for bluetooth is fine because you have flow control.
My issues have been on channels without flow control. Without DMA it
simply drops messages or the channel hangs until you close and reopen
it. With DMA, when an overflow locks up the channel it is usually
unavailable until the board is rebooted.

It's the main reason I've stopped daisy chaining boards to each other,
when the board powers off the pinmux pulls go crazy just long enough
to lock up the other end. It sometimes happens with reboots and large
data bursts as well.

I'd only enable them on bluetooth channels for the time being because of this.

>
> So this isn't like on the RK3399 and prior chips, where it was shared with
> audio, and exhausting the DMA channels would cause audio to not probe.
>
> Regards
> ChenYu
  

Am Montag, 7. November 2022, 02:50:43 CET schrieb Peter Geis:
> On Sun, Nov 6, 2022 at 8:28 PM Chen-Yu Tsai <wens@kernel.org> wrote:
> >
> > On Mon, Nov 7, 2022 at 8:52 AM Peter Geis <pgwipeout@gmail.com> wrote:
> > >
> > > On Sun, Nov 6, 2022 at 11:15 AM Chen-Yu Tsai <wens@kernel.org> wrote:
> > > >
> > > > From: Chen-Yu Tsai <wens@csie.org>
> > > >
> > > > At least one implementation, Linux, requires "dma-names" properties
> > > > be used together with "dmas" to describe DMA resources. These are
> > > > currently missing, causing DMA to not be used for UARTs.
> > > >
> > > > Add "dma-names" to the UART device nodes.
> > > >
> > > > Fixes: a3adc0b9071d ("arm64: dts: rockchip: add core dtsi for RK3568 SoC")
> > > > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > >
> > > Enabling dma globally can cause some interesting issues, have you
> > > tested this fully?
> >
> > It seems to work OK with the Bluetooth controller, though I'm not running
> > extensive transfers over it. Scanning both traditional and LE works, and
> > does exercise the DMA controller.
> >
> > If your worried about the DMA controller running out of channels and
> > impacting other peripherals, the DMA controller for the UARTs is only
> > shared with other UARTs, SPI, and PWM (which the kernel doesn't support
> > DMAing to). The UART and SPI drivers can fall back to PIO if DMA isn't
> > available.
> 
> Nah, enabling it for bluetooth is fine because you have flow control.
> My issues have been on channels without flow control. Without DMA it
> simply drops messages or the channel hangs until you close and reopen
> it. With DMA, when an overflow locks up the channel it is usually
> unavailable until the board is rebooted.
> 
> It's the main reason I've stopped daisy chaining boards to each other,
> when the board powers off the pinmux pulls go crazy just long enough
> to lock up the other end. It sometimes happens with reboots and large
> data bursts as well.
> 
> I'd only enable them on bluetooth channels for the time being because of this.

I guess I'll agree with Peter here. So enabling uart dmas should likely
happen on a board-level for bluetooth connections.