Re: [PATCH 2/2] ARM: dts: meson: Adding hwrev syscon node

Arnd Bergmann
2016-02-18T12:43:01Z

On Thursday 18 February 2016 13:33:04 Romain Perier wrote:
> 2016-02-17 21:36 GMT+01:00 Carlo Caione <carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org>:
> > On Wed, Feb 17, 2016 at 6:28 PM, Romain Perier <romain.perier-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> These are the CBUS registers used to retrieve the revision and the
> >> identifier of the SoC on Meson8.
> >>
> >> Signed-off-by: Romain Perier <romain.perier-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> ---
> >> arch/arm/boot/dts/meson8b.dtsi | 5 +++++
> >> 1 file changed, 5 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
> >> index 0477a81..71009dc 100644
> >> --- a/arch/arm/boot/dts/meson8b.dtsi
> >> +++ b/arch/arm/boot/dts/meson8b.dtsi
> >> @@ -99,6 +99,11 @@
> >> };
> >> };
> >>
> >> + hwrev@c1107d4c {
> >> + compatible = "amlogic,meson8b-hwrev", "syscon";
> >> + reg = <0xc1107d4c 0x460>;
> >
> > Interesting. Where did you get 0x460?
>
> Carlo, Arnd.
>
> Well, what I did is the following :
> - CBUS_PHY_BASE is 0xc1100000 (CBUS is a larger block of registers,
> like slcr on zynq)
> - the serial is at CBUS_PHY_BASE + 0x7d4c
> - the revision is at CBUS_PHY_BASE + 0x81a8
>
> So I decided to create a device_node for hw revision at 0xc1107d4c, in
> this case the lenght is 0x460...
> Am I wrong ?

Yes, you should describe the device that is actually there, which would be
something like

cbus@c1100000 {
compatible = "amlogic,meson8b-cbus", "syscon";
reg = <0xc1100000 0x10000>; /* no idea what the length is, fix it */
};

Don't just make things up because you happen to access them in a particular
way, but try to stay as close as you can to describing the actual hardware.

Please also check that the register ranges don't overlap with something
that might already be there, in case someone else made the same mistake.

Arnd