Re: [PATCH v3 1/6] dt-bindings: dma: uniphier-xdmac: Consolidate register region description

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 23, 2020 at 6:33 PM Kunihiko Hayashi
<hayashi.kunihiko@xxxxxxxxxxxxx> wrote:
>
> The extension register region isn't currently referred from the driver, so
> this consolidates the extension register region description into the base
> register region, and spreads the region size in example.


You should not say this in the commit log.

The DT binding is hardware description.
Whether it is used or not in the driver is not a primary reason.



I recommend you to read this:


Documentation/devicetree/bindings/writing-bindings.txt:

- DON'T refer to Linux or "device driver" in bindings. Bindings should be
  based on what the hardware has, not what an OS and driver currently support.




> Fixes: b9fb56b6ba8a ("dt-bindings: dmaengine: Add UniPhier external DMA controller bindings")
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@xxxxxxxxxxxxx>
> ---
>  Documentation/devicetree/bindings/dma/socionext,uniphier-xdmac.yaml | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/dma/socionext,uniphier-xdmac.yaml b/Documentation/devicetree/bindings/dma/socionext,uniphier-xdmac.yaml
> index 86cfb59..830cd88 100644
> --- a/Documentation/devicetree/bindings/dma/socionext,uniphier-xdmac.yaml
> +++ b/Documentation/devicetree/bindings/dma/socionext,uniphier-xdmac.yaml
> @@ -23,8 +23,7 @@ properties:
>
>    reg:
>      items:
> -      - description: XDMAC base register region (offset and length)
> -      - description: XDMAC extension register region (offset and length)
> +      - description: XDMAC register region (offset and length)

Now that the reg is a single tuple,
the "items" is unneeded.



>    interrupts:
>      maxItems: 1
> @@ -54,7 +53,7 @@ examples:
>    - |
>      xdmac: dma-controller@5fc10000 {
>          compatible = "socionext,uniphier-xdmac";
> -        reg = <0x5fc10000 0x1000>, <0x5fc20000 0x800>;
> +        reg = <0x5fc10000 0x5100>;


Checking the datasheet (LD20), this seems still wrong.

The last register is XDMAID3 : 0x5fc1520c

So, reg = <0x5fc10000 0x5300>;


I attached a patch, which I think more correct.
Please check it, and I will send it to the correct ML.



>          interrupts = <0 188 4>;
>          #dma-cells = <2>;
>          dma-channels = <16>;
> --
> 2.7.4
>

-- 
Best Regards
Masahiro Yamada
From 94642b0fc7976be514acc70a095ee3efeac3d8e3 Mon Sep 17 00:00:00 2001
From: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
Date: Mon, 30 Mar 2020 18:31:18 +0900
Subject: [PATCH] dt-bindings: dma: uniphier-xdmac: switch to single reg region

The reg in the example "<0x5fc10000 0x1000>, <0x5fc20000 0x800>"
is wrong. The register region of this controller is much smaller,
and there is no other hardware register interleaved. There is
no good reason to split it into two regions.

Just use a single, contiguous register region.

While I am here, I made the 'dma-channels' property mandatory because
otherwise there is no way to determine the number of the channels.

Please note the original binding was merged recently. Since there
is no user yet, this change has no actual impact.

Fixes: b9fb56b6ba8a ("dt-bindings: dmaengine: Add UniPhier external DMA controller bindings")
Signed-off-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
---
 .../devicetree/bindings/dma/socionext,uniphier-xdmac.yaml  | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/socionext,uniphier-xdmac.yaml b/Documentation/devicetree/bindings/dma/socionext,uniphier-xdmac.yaml
index 86cfb599256e..371f18773198 100644
--- a/Documentation/devicetree/bindings/dma/socionext,uniphier-xdmac.yaml
+++ b/Documentation/devicetree/bindings/dma/socionext,uniphier-xdmac.yaml
@@ -22,9 +22,7 @@ properties:
     const: socionext,uniphier-xdmac
 
   reg:
-    items:
-      - description: XDMAC base register region (offset and length)
-      - description: XDMAC extension register region (offset and length)
+    maxItems: 1
 
   interrupts:
     maxItems: 1
@@ -49,12 +47,13 @@ required:
   - reg
   - interrupts
   - "#dma-cells"
+  - dma-channels
 
 examples:
   - |
     xdmac: dma-controller@5fc10000 {
         compatible = "socionext,uniphier-xdmac";
-        reg = <0x5fc10000 0x1000>, <0x5fc20000 0x800>;
+        reg = <0x5fc10000 0x5300>;
         interrupts = <0 188 4>;
         #dma-cells = <2>;
         dma-channels = <16>;
-- 
2.17.1


[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux