[PATCH 4/4] arm64: Add initial DTS for APM X-Gene Storm SOC and APM Mustang board

[Mark Rutland]

On Mon, Apr 22, 2013 at 01:23:51PM +0100, Vinayak Kale wrote:
> On Mon, Apr 22, 2013 at 4:44 PM, Mark Rutland <mark.rutland at arm.com> wrote:
> > Hi,
> >
> > I have a couple of minor comments on the dts:
> >
> >> diff --git a/arch/arm64/boot/dts/storm.dtsi b/arch/arm64/boot/dts/storm.dtsi
> >> new file mode 100644
> >> index 0000000..8cab5d1
> >> --- /dev/null
> >> +++ b/arch/arm64/boot/dts/storm.dtsi
> >> @@ -0,0 +1,116 @@
> >> +/*
> >> + * dts file for AppliedMicro (APM) X-Gene Storm SOC
> >> + *
> >> + * Copyright (C) 2013, Applied Micro Circuits Corporation
> >> + *
> >> + * This program is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU General Public License as
> >> + * published by the Free Software Foundation; either version 2 of
> >> + * the License, or (at your option) any later version.
> >> + */
> >> +
> >> +/ {
> >> +     compatible = "apm,xgene,storm", "apm,xgene";
> >> +     interrupt-parent = <&gic>;
> >> +     #address-cells = <2>;
> >> +     #size-cells = <2>;
> >> +
> >> +     cpus {
> >> +             #address-cells = <1>;
> >
> > The reg is 64-bit, so address-cells should be 2.
> >
> Please see my comment below regarding 'reg' property & cpu node naming.
> >> +             #size-cells = <0>;
> >> +
> >> +             cpu at 0 {
> >> +                     device_type = "cpu";
> >> +                     compatible = "arm,armv8";
> >
> > It might be good to also have a compatible string for the X-Gene's CPU
> > implementation, e.g.
> >
> > compatible = "apm,xgene-cpu", "arm,armv8";
> >
> Okay, will add a compatible string for APM-Xgene.
> >> +                     reg = <0x0 0x0000>;
> >> +                     enable-method = "spin-table";
> >> +                     cpu-release-addr = <0x1 0x0000fff8>;
> >> +             };
> >> +             cpu at 1 {
> >> +                     device_type = "cpu";
> >> +                     compatible = "arm,armv8";
> >> +                     reg = <0x0 0x0001>;
> >> +                     enable-method = "spin-table";
> >> +                     cpu-release-addr = <0x1 0x0000fff8>;
> >> +             };
> >> +             cpu at 2 {
> >> +                     device_type = "cpu";
> >> +                     compatible = "arm,armv8";
> >> +                     reg = <0x0 0x0100>;
> >> +                     enable-method = "spin-table";
> >> +                     cpu-release-addr = <0x1 0x0000fff8>;
> >> +             };
> >
> > This should be cpu at 101 to match the reg property (and the following nodes
> > should be fixed up similarly).
> >
> I would like to keep the cpu numbering as is (0 to 7). It helps the
> boot-loader to iterate over cpu nodes based on logical numbers to do
> fix-ups. Also here 'reg' property is not holding an address instead it
> contains value to be compared with mpidr_el1.

We use the address bits in a cpu node's reg property to hold the mpidr_el1
value. Regardless of the semantics of an address vs CPU id, the address bits in
the reg property take 2 cells, and hence #address-cells should be 2, as ePAPR
states:

"The #address-cells property defines the number of <u32> cells used to encode
the address field in a child node’s reg property."

Regarding the unit-address, ePAPR states:

"The unit-address must match the first address specified in the reg property of
the node."

We've tried to stick to this so far.

Thanks,
Mark.