Re: [PATCH 4/7] arm64: dts: mediatek: mt8186: Add and use UART AP_DMA controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `apdma` node provides an incorrect number of cells in its `interrupts` property (3 instead of 4) and likely incorrect polarity, breaking DT parsing and interrupt registration.
- [Medium] Incorrect fallback compatible string `mediatek,mt6577-uart-dma` violates the DT binding schema and silently restricts DMA capabilities to 32 bits.
--

commit 9cc7e542a995507ace5b082f09a644dd3e616c00
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

arm64: dts: mediatek: mt8186: Add and use UART AP_DMA controller

This SoC has a DMA controller (AP_DMA) that provides one channel for each data
direction (transmit and receive) for all of the UART controllers in the SoC.
This commit adds the UART DMA controller and assigns the right channels to each
of the three UART controllers to increase data TX/RX efficiency.

> diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
> index b91f88ffae0e8..7b25c51b1b457 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8186.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
> @@ -1142,6 +1142,22 @@ systimer: timer@10017000 {
>  			clocks = <&clk13m>;
>  		};
>  
> +		apdma: dma-controller@10200d80 {
> +			compatible = "mediatek,mt8186-uart-dma", "mediatek,mt6577-uart-dma";

[Severity: Medium]
Does this fallback compatible string restrict the DMA bitmask?

According to the mediatek,uart-dma.yaml binding, mediatek,mt8186-uart-dma
should fall back to mediatek,mt6835-uart-dma.

Because the mtk-uart-apdma driver uses the matched compatible string to
configure the DMA mask, falling back to mt6577 restricts the controller
to 32 bits instead of the 34 bits provided by mt6835.

Could using mt6577-uart-dma here force hardware with more than 4GB of RAM
to use SWIOTLB bounce buffering for UART DMA transfers?

> +			reg = <0 0x10200d80 0 0x80>, <0 0x10200e00 0 0x80>,
> +			      <0 0x10200e80 0 0x80>, <0 0x10200f00 0 0x80>,
> +			      <0 0x10200f80 0 0x80>, <0 0x10201000 0 0x80>;
> +			interrupts = <GIC_SPI 376 IRQ_TYPE_LEVEL_LOW>,
> +				     <GIC_SPI 377 IRQ_TYPE_LEVEL_LOW>,
> +				     <GIC_SPI 378 IRQ_TYPE_LEVEL_LOW>,
> +				     <GIC_SPI 379 IRQ_TYPE_LEVEL_LOW>,
> +				     <GIC_SPI 380 IRQ_TYPE_LEVEL_LOW>,
> +				     <GIC_SPI 381 IRQ_TYPE_LEVEL_LOW>;

[Severity: High]
Are these interrupts missing the 4th cell?

The root interrupt controller (&gic) requires 4 cells (#interrupt-cells = <4>),
but only 3 cells are provided here for each interrupt.

The missing 4th cell will cause parsing to consume parts of the next
interrupt as the 4th cell of the current one, corrupting the entire
interrupt mapping for this device.

Additionally, GIC SPIs on MT8186 are almost exclusively active-high
(IRQ_TYPE_LEVEL_HIGH). Is IRQ_TYPE_LEVEL_LOW functionally correct for
this controller?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701121929.19374-1-angelogioacchino.delregno@collabora.com?part=4