[PATCH] dt-bindings: timer: Use non-empty ranges in example

[PATCH] dt-bindings: timer: Use non-empty ranges in example

[Douglas Anderson]

On many arm64 qcom device trees, running `make dtbs_check` yells:

  timer@17c20000: #size-cells:0:0: 1 was expected

It appears that someone was trying to assert the fact that sub-nodes
describing frames would never have a size that's more than 32-bits
big.  That does indeed appear to be true for all cases I could find.

Currently many arm64 qcom device tree files have a #address-cells and
about in commit bede7d2dc8f3 ("arm64: dts: qcom: sdm845: Increase
address and size cells for soc").  That means the only way we can
shrink them down is to use a non-empty ranges.

Since forever it has said in "writing-bindings.txt" to "DO use
non-empty 'ranges' to limit the size of child buses/devices".  I guess
we should start listening to it.

I believe (but am not certain) that this also means that we should use
"ranges" to simplify the "reg" of our sub devices by specifying an
offset.  Let's update the example in the bindings to make this
obvious.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
See:
  https://lore.kernel.org/r/20191212113540.7.Ia9bd3fca24ad34a5faaf1c3e58095c74b38abca1@changeid

...for the patch that sparked this change.

 .../devicetree/bindings/timer/arm,arch_timer_mmio.yaml | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/timer/arm,arch_timer_mmio.yaml b/Documentation/devicetree/bindings/timer/arm,arch_timer_mmio.yaml
index b3f0fe96ff0d..d927b42ddeb8 100644
--- a/Documentation/devicetree/bindings/timer/arm,arch_timer_mmio.yaml
+++ b/Documentation/devicetree/bindings/timer/arm,arch_timer_mmio.yaml
@@ -99,22 +99,22 @@ examples:
       compatible = "arm,armv7-timer-mem";
       #address-cells = <1>;
       #size-cells = <1>;
-      ranges;
+      ranges = <0 0xf0000000 0x1000>;
       reg = <0xf0000000 0x1000>;
       clock-frequency = <50000000>;
 
-      frame@f0001000 {
+      frame@0 {
         frame-number = <0>;
         interrupts = <0 13 0x8>,
                <0 14 0x8>;
-        reg = <0xf0001000 0x1000>,
-              <0xf0002000 0x1000>;
+        reg = <0x0000 0x1000>,
+              <0x1000 0x1000>;
       };
 
       frame@f0003000 {
         frame-number = <1>;
         interrupts = <0 15 0x8>;
-        reg = <0xf0003000 0x1000>;
+        reg = <0x2000 0x1000>;
       };
     };
 
-- 
2.24.1.735.g03f4e72817-goog

Re: [PATCH] dt-bindings: timer: Use non-empty ranges in example

[Doug Anderson]

Hi,

On Mon, Dec 16, 2019 at 10:06 PM Douglas Anderson <dianders@chromium.org> wrote:
>
> On many arm64 qcom device trees, running `make dtbs_check` yells:
>
>   timer@17c20000: #size-cells:0:0: 1 was expected
>
> It appears that someone was trying to assert the fact that sub-nodes
> describing frames would never have a size that's more than 32-bits
> big.  That does indeed appear to be true for all cases I could find.
>
> Currently many arm64 qcom device tree files have a #address-cells and
> about in commit bede7d2dc8f3 ("arm64: dts: qcom: sdm845: Increase
> address and size cells for soc").  That means the only way we can
> shrink them down is to use a non-empty ranges.
>
> Since forever it has said in "writing-bindings.txt" to "DO use
> non-empty 'ranges' to limit the size of child buses/devices".  I guess
> we should start listening to it.
>
> I believe (but am not certain) that this also means that we should use
> "ranges" to simplify the "reg" of our sub devices by specifying an
> offset.  Let's update the example in the bindings to make this
> obvious.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> See:
>   https://lore.kernel.org/r/20191212113540.7.Ia9bd3fca24ad34a5faaf1c3e58095c74b38abca1@changeid
>
> ...for the patch that sparked this change.
>
>  .../devicetree/bindings/timer/arm,arch_timer_mmio.yaml | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/timer/arm,arch_timer_mmio.yaml b/Documentation/devicetree/bindings/timer/arm,arch_timer_mmio.yaml
> index b3f0fe96ff0d..d927b42ddeb8 100644
> --- a/Documentation/devicetree/bindings/timer/arm,arch_timer_mmio.yaml
> +++ b/Documentation/devicetree/bindings/timer/arm,arch_timer_mmio.yaml
> @@ -99,22 +99,22 @@ examples:
>        compatible = "arm,armv7-timer-mem";
>        #address-cells = <1>;
>        #size-cells = <1>;
> -      ranges;
> +      ranges = <0 0xf0000000 0x1000>;

I checked this over a few times and yet I still screwed it up.  :(
This should be:

ranges = <0 0xf0001000 0x1000>;

...which makes the first "frame" below actually start at 0.  I'll wait
before sending out a v2, though, in case this patch is totally wrong
or something.


>        reg = <0xf0000000 0x1000>;
>        clock-frequency = <50000000>;
>
> -      frame@f0001000 {
> +      frame@0 {
>          frame-number = <0>;
>          interrupts = <0 13 0x8>,
>                 <0 14 0x8>;
> -        reg = <0xf0001000 0x1000>,
> -              <0xf0002000 0x1000>;
> +        reg = <0x0000 0x1000>,
> +              <0x1000 0x1000>;
>        };
>
>        frame@f0003000 {
>          frame-number = <1>;
>          interrupts = <0 15 0x8>;
> -        reg = <0xf0003000 0x1000>;
> +        reg = <0x2000 0x1000>;
>        };
>      };

Re: [PATCH] dt-bindings: timer: Use non-empty ranges in example

[Rob Herring]

On Mon, Dec 16, 2019 at 10:16:56PM -0800, Doug Anderson wrote:
> Hi,
> 
> On Mon, Dec 16, 2019 at 10:06 PM Douglas Anderson <dianders@chromium.org> wrote:
> >
> > On many arm64 qcom device trees, running `make dtbs_check` yells:
> >
> >   timer@17c20000: #size-cells:0:0: 1 was expected
> >
> > It appears that someone was trying to assert the fact that sub-nodes
> > describing frames would never have a size that's more than 32-bits
> > big.  That does indeed appear to be true for all cases I could find.
> >
> > Currently many arm64 qcom device tree files have a #address-cells and
> > about in commit bede7d2dc8f3 ("arm64: dts: qcom: sdm845: Increase
> > address and size cells for soc").  That means the only way we can
> > shrink them down is to use a non-empty ranges.
> >
> > Since forever it has said in "writing-bindings.txt" to "DO use
> > non-empty 'ranges' to limit the size of child buses/devices".  I guess
> > we should start listening to it.

I probably need to condition that to account for dma-ranges as you 
wouldn't want to limit the bus in that case.

> >
> > I believe (but am not certain) that this also means that we should use
> > "ranges" to simplify the "reg" of our sub devices by specifying an
> > offset.  Let's update the example in the bindings to make this
> > obvious.
> >
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> > See:
> >   https://lore.kernel.org/r/20191212113540.7.Ia9bd3fca24ad34a5faaf1c3e58095c74b38abca1@changeid
> >
> > ...for the patch that sparked this change.
> >
> >  .../devicetree/bindings/timer/arm,arch_timer_mmio.yaml | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/timer/arm,arch_timer_mmio.yaml b/Documentation/devicetree/bindings/timer/arm,arch_timer_mmio.yaml
> > index b3f0fe96ff0d..d927b42ddeb8 100644
> > --- a/Documentation/devicetree/bindings/timer/arm,arch_timer_mmio.yaml
> > +++ b/Documentation/devicetree/bindings/timer/arm,arch_timer_mmio.yaml
> > @@ -99,22 +99,22 @@ examples:
> >        compatible = "arm,armv7-timer-mem";
> >        #address-cells = <1>;
> >        #size-cells = <1>;
> > -      ranges;
> > +      ranges = <0 0xf0000000 0x1000>;
> 
> I checked this over a few times and yet I still screwed it up.  :(
> This should be:
> 
> ranges = <0 0xf0001000 0x1000>;
> 
> ...which makes the first "frame" below actually start at 0.  I'll wait
> before sending out a v2, though, in case this patch is totally wrong
> or something.

Looks good to me.

Rob