Re: [PATCH linux 1/3] ARM: dts: aspeed: Update palmetto device tree

[Cyril Bur <cyrilbur@gmail.com>]

On Tue, 2016-12-13 at 11:44 +1100, Joel Stanley wrote:
> Hi Cyril,
> 
> On Fri, Dec 9, 2016 at 4:43 PM, Cyril Bur <cyrilbur@gmail.com> wrote:
> > Palmettos have 512mb of ram. There is always framebuffer memory at
> > the
> > top of ram.
> > 
> > This patch also reserves BMC ram for host to BMC communication.
> > 
> > Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
> > ---
> >  arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts | 25
> > ++++++++++++++++++++++++-
> >  arch/arm/boot/dts/aspeed-g4.dtsi              |  6 ++++++
> >  2 files changed, 30 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
> > b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
> > index acaceda2..e810ae7 100644
> > --- a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
> > +++ b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
> > @@ -17,7 +17,30 @@
> >         };
> > 
> >         memory {
> > -               reg = <0x40000000 0x10000000>;
> > +               reg = <0x40000000 0x20000000>;
> > +       };
> 
> Can you send this in a separate patch? It should go into the tree
> now.
> 

Yep, should I send just for palmetto? 

Do you have a list of other platforms? I figure all the ast2400 based
aspeed-bmc-opp-* should get this change, should I include them all in
one patch or all individually?

> > +
> > +       reserved-memory {
> > +               #address-cells = <1>;
> > +               #size-cells= <1>;
> > +               ranges;
> > +
> > +               flash_mem: region@52000000 {
> > +                       compatible = "aspeed,lpc-ctrl";
> > +                       no-map;
> > +                       reg = <0x54000000 0x04000000>; /* 64m */
> > +               };
> > +
> > +               fb_mem: framebuffer@5f000000 {
> > +                       no-map;
> > +                       reg = <0x5f000000 0x01000000>; /* 16m */
> > +               };
> 
> This should be a separate patch too, which we can take straight away.
> 

I'll resend.

> > +       };
> > +
> > +       flash_buffer {
> 
> It's convention to put the reg value after @ in the node name:
> 
>   flash_buffer@1e789000
> 
> > +               compatible = "aspeed,lpc-ctrl";
> 
> Upstream demands we put the SoC in the compatible string
> 
>  aspeed,ast2400-lpc-ctrl.
> 
> > +               memory-region = <&flash_mem>;
> > +               reg = <0x1e789000 8>;
> 
> Make the second cell be 0x8 for consistency.
> 
> >         };
> > --- a/arch/arm/boot/dts/aspeed-g4.dtsi
> > +++ b/arch/arm/boot/dts/aspeed-g4.dtsi
> > @@ -870,6 +870,12 @@
> >                                 interrupts = <8>;
> >                         };
> > 
> > +                       mbox: mbox@1e789200 {
> > +                               compatible = "aspeed,mbox-host";
> 
> aspeed,ast2400-mbox-host
> 
> Not sure on the name here.
> 

Yeah I'm not sold on the name either...


Thanks for review.

Cyril

> > +                               reg = <0x1e789200 0x5c>;
> > +                               interrupts = <46>;
> > +                       };
> > +
> >                         wdt1: wdt@1e785000 {
> >                                 compatible = "aspeed,ast2400-wdt";
> >                                 reg = <0x1e785000 0x1c>;
> > --
> > 2.10.2
> > 
> > _______________________________________________
> > openbmc mailing list
> > openbmc@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/openbmc