Re: [PATCH v10 3/3] dt-bindings: dma: uart: rename binding

From: Matthias Brugger
Date: Thu Feb 14 2019 - 05:13:19 EST




On 18/01/2019 04:10, Long Cheng wrote:
> The filename matches mtk-uart-apdma.c.
> So using "mtk-uart-apdma.txt" should be better.
>
> Signed-off-by: Long Cheng <long.cheng@xxxxxxxxxxxx>
> ---
> .../devicetree/bindings/dma/8250_mtk_dma.txt | 33 --------------------
> .../devicetree/bindings/dma/mtk-uart-apdma.txt | 33 ++++++++++++++++++++
> 2 files changed, 33 insertions(+), 33 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/dma/8250_mtk_dma.txt
> create mode 100644 Documentation/devicetree/bindings/dma/mtk-uart-apdma.txt
>
> diff --git a/Documentation/devicetree/bindings/dma/8250_mtk_dma.txt b/Documentation/devicetree/bindings/dma/8250_mtk_dma.txt
> deleted file mode 100644
> index 3fe0961..0000000
> --- a/Documentation/devicetree/bindings/dma/8250_mtk_dma.txt
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -* Mediatek UART APDMA Controller
> -
> -Required properties:
> -- compatible should contain:
> - * "mediatek,mt2712-uart-dma" for MT2712 compatible APDMA
> - * "mediatek,mt6577-uart-dma" for MT6577 and all of the above
> -
> -- reg: The base address of the APDMA register bank.> -
> -- interrupts: A single interrupt specifier.

One interrupt per dma-request, or 8 if no dma-requests property is present.

> -
> -- clocks : Must contain an entry for each entry in clock-names.
> - See ../clocks/clock-bindings.txt for details.
> -- clock-names: The APDMA clock for register accesses> -
> -Examples:
> -
> - apdma: dma-controller@11000380 {
> - compatible = "mediatek,mt2712-uart-dma";
> - reg = <0 0x11000380 0 0x400>;
> - interrupts = <GIC_SPI 63 IRQ_TYPE_LEVEL_LOW>,
> - <GIC_SPI 64 IRQ_TYPE_LEVEL_LOW>,
> - <GIC_SPI 65 IRQ_TYPE_LEVEL_LOW>,
> - <GIC_SPI 66 IRQ_TYPE_LEVEL_LOW>,
> - <GIC_SPI 67 IRQ_TYPE_LEVEL_LOW>,
> - <GIC_SPI 68 IRQ_TYPE_LEVEL_LOW>,
> - <GIC_SPI 69 IRQ_TYPE_LEVEL_LOW>,
> - <GIC_SPI 70 IRQ_TYPE_LEVEL_LOW>;
> - clocks = <&pericfg CLK_PERI_AP_DMA>;
> - clock-names = "apdma";
> - #dma-cells = <1>;
> - };
> -
> diff --git a/Documentation/devicetree/bindings/dma/mtk-uart-apdma.txt b/Documentation/devicetree/bindings/dma/mtk-uart-apdma.txt
> new file mode 100644
> index 0000000..3fe0961
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/mtk-uart-apdma.txt
> @@ -0,0 +1,33 @@
> +* Mediatek UART APDMA Controller
> +
> +Required properties:
> +- compatible should contain:
> + * "mediatek,mt2712-uart-dma" for MT2712 compatible APDMA
> + * "mediatek,mt6577-uart-dma" for MT6577 and all of the above
> +
> +- reg: The base address of the APDMA register bank.
> +

There is one address needed for every dma-request or 8 if dma-requests property
is not present. As already mentioned in the driver review dma-requests property
is missing.

> +- interrupts: A single interrupt specifier.
> +
> +- clocks : Must contain an entry for each entry in clock-names.
> + See ../clocks/clock-bindings.txt for details.
> +- clock-names: The APDMA clock for register accesses
> +

Missing option property dma-33bits (should be mtk,dma-33bits?).

> +Examples:
> +
> + apdma: dma-controller@11000380 {
> + compatible = "mediatek,mt2712-uart-dma";
> + reg = <0 0x11000380 0 0x400>;

We are missing the io mem areas here, right?

Regards,
Matthias

> + interrupts = <GIC_SPI 63 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 64 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 65 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 66 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 67 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 68 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 69 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 70 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&pericfg CLK_PERI_AP_DMA>;
> + clock-names = "apdma";
> + #dma-cells = <1>;
> + };
> +
>