diff mbox series

[v2] arm64: dts: rockchip: add rk3399 UART DMAs

Message ID 20190321162244.10080-1-katsuhiro@katsuster.net (mailing list archive)
State New, archived
Headers show
Series [v2] arm64: dts: rockchip: add rk3399 UART DMAs | expand

Commit Message

Katsuhiro Suzuki March 21, 2019, 4:22 p.m. UTC
Add UART dma channels as specified by the rk3399 TRM.

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

Signed-off-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>

---

Changes from v1:
  - Add dma property for UART4
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Heiko Stübner March 25, 2019, 12:34 p.m. UTC | #1
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 <katsuhiro@katsuster.net>

applied for 5.2

Thanks
Heiko
Robin Murphy March 26, 2019, 11:48 a.m. UTC | #2
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 <katsuhiro@katsuster.net>
> 
> 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.
Katsuhiro Suzuki March 26, 2019, 1:49 p.m. UTC | #3
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 <katsuhiro@katsuster.net>
>>
>> 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.
>
Heiko Stübner March 27, 2019, noon UTC | #4
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 <katsuhiro@katsuster.net>
> >>
> >> 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.
> > 
> 
>
Robin Murphy March 27, 2019, 12:10 p.m. UTC | #5
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 <katsuhiro@katsuster.net>
>>>
>>> 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.
>>
>
Robin Murphy March 29, 2019, 11:24 p.m. UTC | #6
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.
Heiko Stübner April 11, 2019, 12:46 p.m. UTC | #7
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
Robin Murphy April 11, 2019, 1:48 p.m. UTC | #8
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.
Robin Murphy April 11, 2019, 7:17 p.m. UTC | #9
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.
diff mbox series

Patch

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";