2019-03-21 16:25:50

by Katsuhiro Suzuki

[permalink] [raw]
Subject: [PATCH v2] arm64: dts: rockchip: add rk3399 UART DMAs

Add UART dma channels as specified by the rk3399 TRM.

Refer:
RK3399 TRM V1.4: Chapter 12 DMA Controller

Signed-off-by: Katsuhiro Suzuki <[email protected]>

---

Changes from v1:
- Add dma property for UART4
---
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 0301e3e01b38..31e487202676 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -635,6 +635,8 @@
clocks = <&cru SCLK_UART0>, <&cru PCLK_UART0>;
clock-names = "baudclk", "apb_pclk";
interrupts = <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH 0>;
+ dmas = <&dmac_peri 0>, <&dmac_peri 1>;
+ dma-names = "tx", "rx";
reg-shift = <2>;
reg-io-width = <4>;
pinctrl-names = "default";
@@ -648,6 +650,8 @@
clocks = <&cru SCLK_UART1>, <&cru PCLK_UART1>;
clock-names = "baudclk", "apb_pclk";
interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH 0>;
+ dmas = <&dmac_peri 2>, <&dmac_peri 3>;
+ dma-names = "tx", "rx";
reg-shift = <2>;
reg-io-width = <4>;
pinctrl-names = "default";
@@ -661,6 +665,8 @@
clocks = <&cru SCLK_UART2>, <&cru PCLK_UART2>;
clock-names = "baudclk", "apb_pclk";
interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH 0>;
+ dmas = <&dmac_peri 4>, <&dmac_peri 5>;
+ dma-names = "tx", "rx";
reg-shift = <2>;
reg-io-width = <4>;
pinctrl-names = "default";
@@ -674,6 +680,8 @@
clocks = <&cru SCLK_UART3>, <&cru PCLK_UART3>;
clock-names = "baudclk", "apb_pclk";
interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH 0>;
+ dmas = <&dmac_peri 6>, <&dmac_peri 7>;
+ dma-names = "tx", "rx";
reg-shift = <2>;
reg-io-width = <4>;
pinctrl-names = "default";
@@ -1152,6 +1160,8 @@
clocks = <&pmucru SCLK_UART4_PMU>, <&pmucru PCLK_UART4_PMU>;
clock-names = "baudclk", "apb_pclk";
interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH 0>;
+ dmas = <&dmac_peri 8>, <&dmac_peri 9>;
+ dma-names = "tx", "rx";
reg-shift = <2>;
reg-io-width = <4>;
pinctrl-names = "default";
--
2.20.1



2019-03-25 12:35:47

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH v2] arm64: dts: rockchip: add rk3399 UART DMAs

Am Donnerstag, 21. M?rz 2019, 17:22:44 CET schrieb Katsuhiro Suzuki:
> Add UART dma channels as specified by the rk3399 TRM.
>
> Refer:
> RK3399 TRM V1.4: Chapter 12 DMA Controller
>
> Signed-off-by: Katsuhiro Suzuki <[email protected]>

applied for 5.2

Thanks
Heiko



2019-03-26 11:49:11

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH v2] arm64: dts: rockchip: add rk3399 UART DMAs

On 25/03/2019 12:34, Heiko Stuebner wrote:
> Am Donnerstag, 21. März 2019, 17:22:44 CET schrieb Katsuhiro Suzuki:
>> Add UART dma channels as specified by the rk3399 TRM.
>>
>> Refer:
>> RK3399 TRM V1.4: Chapter 12 DMA Controller
>>
>> Signed-off-by: Katsuhiro Suzuki <[email protected]>
>
> applied for 5.2

As a heads-up, I did manage to try my board with this patch applied over
the weekend, and it makes probing the bluetooth adapter fail with
communication errors, so I'm not sure the 8250 and pl330 drivers are
really cooperating well enough :(

Robin.

2019-03-26 13:50:25

by Katsuhiro Suzuki

[permalink] [raw]
Subject: Re: [PATCH v2] arm64: dts: rockchip: add rk3399 UART DMAs

Hello Robin,

Sorry for inconvenience. Since I don't adhere enabling DMA for UARTs,
please revert my patch if you need.

BTW, there are DMA properties in RK3328 device-tree like as this patch.
RK3328 UART DMA could not work correctly too...??

Best Regards,
Katsuhiro Suzuki


On 2019/03/26 20:48, Robin Murphy wrote:
> On 25/03/2019 12:34, Heiko Stuebner wrote:
>> Am Donnerstag, 21. März 2019, 17:22:44 CET schrieb Katsuhiro Suzuki:
>>> Add UART dma channels as specified by the rk3399 TRM.
>>>
>>> Refer:
>>> RK3399 TRM V1.4: Chapter 12 DMA Controller
>>>
>>> Signed-off-by: Katsuhiro Suzuki <[email protected]>
>>
>> applied for 5.2
>
> As a heads-up, I did manage to try my board with this patch applied over
> the weekend, and it makes probing the bluetooth adapter fail with
> communication errors, so I'm not sure the 8250 and pl330 drivers are
> really cooperating well enough :(
>
> Robin.
>


2019-03-27 12:01:32

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH v2] arm64: dts: rockchip: add rk3399 UART DMAs

Hi,

Am Dienstag, 26. M?rz 2019, 14:49:16 CET schrieb Katsuhiro Suzuki:
> Hello Robin,
>
> Sorry for inconvenience. Since I don't adhere enabling DMA for UARTs,
> please revert my patch if you need.

I've dropped the patch from my queue now.

> BTW, there are DMA properties in RK3328 device-tree like as this patch.
> RK3328 UART DMA could not work correctly too...??

I remember Rockcihip dma-controllers having issues with burst-sizes
and flushing (there is a no-flushp option in pl330), so it's
possible that all share the same error up to rk3399 and rk3328

But so far no-one has shouted regarding the rk3328.


Heiko

> On 2019/03/26 20:48, Robin Murphy wrote:
> > On 25/03/2019 12:34, Heiko Stuebner wrote:
> >> Am Donnerstag, 21. M?rz 2019, 17:22:44 CET schrieb Katsuhiro Suzuki:
> >>> Add UART dma channels as specified by the rk3399 TRM.
> >>>
> >>> Refer:
> >>> RK3399 TRM V1.4: Chapter 12 DMA Controller
> >>>
> >>> Signed-off-by: Katsuhiro Suzuki <[email protected]>
> >>
> >> applied for 5.2
> >
> > As a heads-up, I did manage to try my board with this patch applied over
> > the weekend, and it makes probing the bluetooth adapter fail with
> > communication errors, so I'm not sure the 8250 and pl330 drivers are
> > really cooperating well enough :(
> >
> > Robin.
> >
>
>





2019-03-27 12:11:01

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH v2] arm64: dts: rockchip: add rk3399 UART DMAs

On 26/03/2019 13:49, Katsuhiro Suzuki wrote:
> Hello Robin,
>
> Sorry for inconvenience. Since I don't adhere enabling DMA for UARTs,
> please revert my patch if you need.

I found a little time to fire up the board again this morning, so I gave
this another try to double-check - in fact this time I only saw it fail
3 times in 15 reboots. So something's certainly not quite right, but
it's not quite as terrible as the first try implied.

My suspicion at this point is that the DMA implementation might be
losing characters occasionally, and obviously the bluetooth firmware
transfer is going to be a lot more sensitive to that than a text console is.

> BTW, there are DMA properties in RK3328 device-tree like as this patch.
> RK3328 UART DMA could not work correctly too...??

Quite possibly, although my 3328 box doesn't have any UARTS connected or
exposed other than the standard debug console, so I can't easily
investigate there. I do have a 3288 box with similar serial bluetooth to
my 3399 which might be worth digging out for comparison with an
up-to-date kernel.

Robin.

>
> Best Regards,
> Katsuhiro Suzuki
>
>
> On 2019/03/26 20:48, Robin Murphy wrote:
>> On 25/03/2019 12:34, Heiko Stuebner wrote:
>>> Am Donnerstag, 21. März 2019, 17:22:44 CET schrieb Katsuhiro Suzuki:
>>>> Add UART dma channels as specified by the rk3399 TRM.
>>>>
>>>> Refer:
>>>> RK3399 TRM V1.4: Chapter 12 DMA Controller
>>>>
>>>> Signed-off-by: Katsuhiro Suzuki <[email protected]>
>>>
>>> applied for 5.2
>>
>> As a heads-up, I did manage to try my board with this patch applied
>> over the weekend, and it makes probing the bluetooth adapter fail with
>> communication errors, so I'm not sure the 8250 and pl330 drivers are
>> really cooperating well enough :(
>>
>> Robin.
>>
>

2019-03-29 23:25:45

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH v2] arm64: dts: rockchip: add rk3399 UART DMAs

On 2019-03-27 12:00 pm, Heiko Stuebner wrote:
> Hi,
>
> Am Dienstag, 26. März 2019, 14:49:16 CET schrieb Katsuhiro Suzuki:
>> Hello Robin,
>>
>> Sorry for inconvenience. Since I don't adhere enabling DMA for UARTs,
>> please revert my patch if you need.
>
> I've dropped the patch from my queue now.
>
>> BTW, there are DMA properties in RK3328 device-tree like as this patch.
>> RK3328 UART DMA could not work correctly too...??
>
> I remember Rockcihip dma-controllers having issues with burst-sizes
> and flushing (there is a no-flushp option in pl330), so it's
> possible that all share the same error up to rk3399 and rk3328
>
> But so far no-one has shouted regarding the rk3328.

Let me be the first, then, I guess :)

I found an easy way to observe the problem on my 3399, and I've just
fired up my 3328 box with a 5.0 distro kernel to find that it behaves
the same. Basically just dump a large pile of text into 'less' on the
serial console, and scroll through line-by-line - certain lines get
dropped except for a few characters at the end.

I'll see if I can narrow it down a bit, starting with trying
broken-flushp...

Robin.

2019-04-11 12:48:23

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH v2] arm64: dts: rockchip: add rk3399 UART DMAs

Hi Robin,

Am Samstag, 30. M?rz 2019, 00:24:16 CEST schrieb Robin Murphy:
> On 2019-03-27 12:00 pm, Heiko Stuebner wrote:
> > Hi,
> >
> > Am Dienstag, 26. M?rz 2019, 14:49:16 CET schrieb Katsuhiro Suzuki:
> >> Hello Robin,
> >>
> >> Sorry for inconvenience. Since I don't adhere enabling DMA for UARTs,
> >> please revert my patch if you need.
> >
> > I've dropped the patch from my queue now.
> >
> >> BTW, there are DMA properties in RK3328 device-tree like as this patch.
> >> RK3328 UART DMA could not work correctly too...??
> >
> > I remember Rockcihip dma-controllers having issues with burst-sizes
> > and flushing (there is a no-flushp option in pl330), so it's
> > possible that all share the same error up to rk3399 and rk3328
> >
> > But so far no-one has shouted regarding the rk3328.
>
> Let me be the first, then, I guess :)
>
> I found an easy way to observe the problem on my 3399, and I've just
> fired up my 3328 box with a 5.0 distro kernel to find that it behaves
> the same. Basically just dump a large pile of text into 'less' on the
> serial console, and scroll through line-by-line - certain lines get
> dropped except for a few characters at the end.
>
> I'll see if I can narrow it down a bit, starting with trying
> broken-flushp...

did you manage to find time for that test you wanted to do?

Heiko


2019-04-11 13:49:10

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH v2] arm64: dts: rockchip: add rk3399 UART DMAs

On 11/04/2019 13:46, Heiko Stuebner wrote:
> Hi Robin,
>
> Am Samstag, 30. März 2019, 00:24:16 CEST schrieb Robin Murphy:
>> On 2019-03-27 12:00 pm, Heiko Stuebner wrote:
>>> Hi,
>>>
>>> Am Dienstag, 26. März 2019, 14:49:16 CET schrieb Katsuhiro Suzuki:
>>>> Hello Robin,
>>>>
>>>> Sorry for inconvenience. Since I don't adhere enabling DMA for UARTs,
>>>> please revert my patch if you need.
>>>
>>> I've dropped the patch from my queue now.
>>>
>>>> BTW, there are DMA properties in RK3328 device-tree like as this patch.
>>>> RK3328 UART DMA could not work correctly too...??
>>>
>>> I remember Rockcihip dma-controllers having issues with burst-sizes
>>> and flushing (there is a no-flushp option in pl330), so it's
>>> possible that all share the same error up to rk3399 and rk3328
>>>
>>> But so far no-one has shouted regarding the rk3328.
>>
>> Let me be the first, then, I guess :)
>>
>> I found an easy way to observe the problem on my 3399, and I've just
>> fired up my 3328 box with a 5.0 distro kernel to find that it behaves
>> the same. Basically just dump a large pile of text into 'less' on the
>> serial console, and scroll through line-by-line - certain lines get
>> dropped except for a few characters at the end.
>>
>> I'll see if I can narrow it down a bit, starting with trying
>> broken-flushp...
>
> did you manage to find time for that test you wanted to do?

Sadly not - I got somewhat distracted by the ethernet thing, and since
then I've had too much else going on to follow up on any of my 'for fun'
projects :(

If it helps, from a quick look based on what I can remember off-hand,
Rockchip UARTs in general have probably been using DMA since 4.19 with
d8095f94e195 ("dmaengine: add support for reporting pause and resume
separately").

Robin.

2019-04-11 19:19:57

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH v2] arm64: dts: rockchip: add rk3399 UART DMAs

On 2019-04-11 2:48 pm, Robin Murphy wrote:
> On 11/04/2019 13:46, Heiko Stuebner wrote:
>> Hi Robin,
>>
>> Am Samstag, 30. März 2019, 00:24:16 CEST schrieb Robin Murphy:
>>> On 2019-03-27 12:00 pm, Heiko Stuebner wrote:
>>>> Hi,
>>>>
>>>> Am Dienstag, 26. März 2019, 14:49:16 CET schrieb Katsuhiro Suzuki:
>>>>> Hello Robin,
>>>>>
>>>>> Sorry for inconvenience. Since I don't adhere enabling DMA for UARTs,
>>>>> please revert my patch if you need.
>>>>
>>>> I've dropped the patch from my queue now.
>>>>
>>>>> BTW, there are DMA properties in RK3328 device-tree like as this
>>>>> patch.
>>>>> RK3328 UART DMA could not work correctly too...??
>>>>
>>>> I remember Rockcihip dma-controllers having issues with burst-sizes
>>>> and flushing (there is a no-flushp option in pl330), so it's
>>>> possible that all share the same error up to rk3399 and rk3328
>>>>
>>>> But so far no-one has shouted regarding the rk3328.
>>>
>>> Let me be the first, then, I guess :)
>>>
>>> I found an easy way to observe the problem on my 3399, and I've just
>>> fired up my 3328 box with a 5.0 distro kernel to find that it behaves
>>> the same. Basically just dump a large pile of text into 'less' on the
>>> serial console, and scroll through line-by-line - certain lines get
>>> dropped except for a few characters at the end.
>>>
>>> I'll see if I can narrow it down a bit, starting with trying
>>> broken-flushp...
>>
>> did you manage to find time for that test you wanted to do?
>
> Sadly not - I got somewhat distracted by the ethernet thing, and since
> then I've had too much else going on to follow up on any of my 'for fun'
> projects :(

Urgh, and it turns out what I thought were dropped characters seems to
just be my console handling line wrapping weirdly, so now I have no idea
if RK3328 actually has an issue, and coming up with a reliable
reproducer for RK3399 is going to take some thought...

Robin.