| Message ID | 1584604252-13172-2-git-send-email-hayashi.kunihiko@socionext.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add devicetree features and fixes for UniPhier SoCs | expand |
On Thu, Mar 19, 2020 at 4:51 PM Kunihiko Hayashi <hayashi.kunihiko@socionext.com> wrote: > > The address of the extension register region in example is incorrect, > however, this extension register region is optional On which SoC is it optional? In your previous DT submission, every reg was, like this: reg = <0x5fc10000 0x1000>, <0x5fc20000 0x800>; and you meant reg = <0x5fc10000 0x1000>, <0x5fc12000 0x800>; ? > and isn't currently > referred from the driver, so the description of the region should be > suppressed until referred by the driver. This sounds like you plan to get it back as you extend the driver. I checked the datasheet. This controller has more registers, so you split the reg into small chunks, the final form will look scary: reg = <0x5fc10000 0x1000>, <0x5fc12000 0x800>, <0x5fc14000 0x100>, <0x5fc15000 0x100>; My question is why you did not use a single reg tuple that covers the whole registers. Is any other hardware reg interleaved in between? > > 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 | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/dma/socionext,uniphier-xdmac.yaml b/Documentation/devicetree/bindings/dma/socionext,uniphier-xdmac.yaml > index 86cfb59..f4c3f49 100644 > --- a/Documentation/devicetree/bindings/dma/socionext,uniphier-xdmac.yaml > +++ b/Documentation/devicetree/bindings/dma/socionext,uniphier-xdmac.yaml > @@ -24,7 +24,6 @@ properties: > reg: > items: > - description: XDMAC base register region (offset and length) > - - description: XDMAC extension 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 0x1000>; > interrupts = <0 188 4>; > #dma-cells = <2>; > dma-channels = <16>; > -- > 2.7.4 >
On Thu, 19 Mar 2020 23:18:29 +0900 <masahiroy@kernel.org> wrote: > On Thu, Mar 19, 2020 at 4:51 PM Kunihiko Hayashi > <hayashi.kunihiko@socionext.com> wrote: > > > > The address of the extension register region in example is incorrect, > > however, this extension register region is optional > > > On which SoC is it optional? > > In your previous DT submission, every reg was, > like this: > > reg = <0x5fc10000 0x1000>, <0x5fc20000 0x800>; > > > > and you meant > > reg = <0x5fc10000 0x1000>, <0x5fc12000 0x800>; > > ? Yes. 'Optional' might not be appropriate because all SoCs has the region. > > and isn't currently > > referred from the driver, so the description of the region should be > > suppressed until referred by the driver. > > This sounds like you plan to get it back > as you extend the driver. Right, however, it isn't desiable that the description of the region is changed by extending the driver. > I checked the datasheet. This controller has more registers, > so you split the reg into small chunks, the final form will look scary: > > reg = <0x5fc10000 0x1000>, <0x5fc12000 0x800>, > <0x5fc14000 0x100>, <0x5fc15000 0x100>; > > > My question is why you did not use a single reg tuple > that covers the whole registers. > > Is any other hardware reg interleaved in between? No, there is no other hardware reg between each region. Surely it seems pointless to divide all tuples individually. I'll rewrite it to cover entire xdmac reg region. Thank you, --- Best Regards, Kunihiko Hayashi
diff --git a/Documentation/devicetree/bindings/dma/socionext,uniphier-xdmac.yaml b/Documentation/devicetree/bindings/dma/socionext,uniphier-xdmac.yaml index 86cfb59..f4c3f49 100644 --- a/Documentation/devicetree/bindings/dma/socionext,uniphier-xdmac.yaml +++ b/Documentation/devicetree/bindings/dma/socionext,uniphier-xdmac.yaml @@ -24,7 +24,6 @@ properties: reg: items: - description: XDMAC base register region (offset and length) - - description: XDMAC extension 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 0x1000>; interrupts = <0 188 4>; #dma-cells = <2>; dma-channels = <16>;
The address of the extension register region in example is incorrect, however, this extension register region is optional and isn't currently referred from the driver, so the description of the region should be suppressed until referred by the driver. 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 | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)