diff mbox series

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

Message ID 1584955970-8162-2-git-send-email-hayashi.kunihiko@socionext.com
State Changes Requested, archived
Headers show
Series Add devicetree features and fixes for UniPhier SoCs | expand

Checks

Context Check Description
robh/checkpatch warning "total: 0 errors, 1 warnings, 17 lines checked"

Commit Message

Kunihiko Hayashi March 23, 2020, 9:32 a.m. UTC
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.

Fixes: b9fb56b6ba8a ("dt-bindings: dmaengine: Add UniPhier external DMA controller bindings")
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 Documentation/devicetree/bindings/dma/socionext,uniphier-xdmac.yaml | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Masahiro Yamada March 30, 2020, 10:20 a.m. UTC | #1
On Mon, Mar 23, 2020 at 6:33 PM Kunihiko Hayashi
<hayashi.kunihiko@socionext.com> 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@socionext.com>
> ---
>  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
>
Kunihiko Hayashi March 31, 2020, 10:28 a.m. UTC | #2
On Mon, 30 Mar 2020 19:20:35 +0900 <yamada.masahiro@socionext.com> wrote:

> On Mon, Mar 23, 2020 at 6:33 PM Kunihiko Hayashi
> <hayashi.kunihiko@socionext.com> 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 see. I forgot that this also applies to commit logs.

>
> 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.

Thanks for your suggestion.

> 
> > Fixes: b9fb56b6ba8a ("dt-bindings: dmaengine: Add UniPhier external DMA controller bindings")
> > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> > ---
> >  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.

Okay, I'll notice it.

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

Hmm, You're right.
I missed the information somewhere.

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

I checked it. This looks good to me and thanks for your help.
I'll fix the remaining patches next.

Thank you,

---
Best Regards,
Kunihiko Hayashi
diff mbox series

Patch

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)
 
   interrupts:
     maxItems: 1
@@ -54,7 +53,7 @@  examples:
   - |
     xdmac: dma-controller@5fc10000 {
         compatible = "socionext,uniphier-xdmac";
-        reg = <0x5fc10000 0x1000>, <0x5fc20000 0x800>;
+        reg = <0x5fc10000 0x5100>;
         interrupts = <0 188 4>;
         #dma-cells = <2>;
         dma-channels = <16>;