[External] Re: [PATCH v1 1/1] ARM: dts: aspeed: update Hr855xg2 device tree

[Joel Stanley]

On Thu, 9 Jan 2020 at 08:07, Andrew MS1 Peng <pengms1 at lenovo.com> wrote:
>
> Hi Joel,
>
> Please help to check below my comments. Thanks.
>
> Regards,
> Andrew Peng
>
> > -----邮件原件-----
> > 发件人: Joel Stanley <joel at jms.id.au>
> > 发送时间: 2020年1月6日 14:48
> > 收件人: Andrew MS1 Peng <pengms1 at lenovo.com>
> > 抄送: Rob Herring <robh+dt at kernel.org>; Mark Rutland
> > <mark.rutland at arm.com>; Andrew Jeffery <andrew at aj.id.au>; devicetree
> > <devicetree at vger.kernel.org>; Linux ARM
> > <linux-arm-kernel at lists.infradead.org>; linux-aspeed
> > <linux-aspeed at lists.ozlabs.org>; Linux Kernel Mailing List
> > <linux-kernel at vger.kernel.org>; Benjamin Fair <benjaminfair at google.com>;
> > OpenBMC Maillist <openbmc at lists.ozlabs.org>; Derek Lin23
> > <dlin23 at lenovo.com>; Yonghui YH21 Liu <liuyh21 at lenovo.com>
> > 主题: [External] Re: [PATCH v1 1/1] ARM: dts: aspeed: update Hr855xg2
> > device tree
> >
> > On Thu, 26 Dec 2019 at 08:54, Andrew Peng <pengms1 at lenovo.com> wrote:
> > >
> >
> > When you have a list of things like below, it's sometimes a good hint that you
> > should be sending one patch for each bullet point. This makes it easier to
> > review.
> >
>
> Should I separate below changes to six patches for next patch version?
> For example: [PATCH v2 0/5]  [PATCH v2 1/5] ...etc

It's up to you.

I do not require it.

>
> > > Update i2c aliases.
> > > Change flash_memory mapping address and size.
> > > Add in a gpio-keys section.
> > > Enable vhub, vuart, spi1 and spi2.
> > > Add raa228006, ir38164 and sn1701022 hwmon sensors.
> > > Remove some unuse gpio from gpio section.
> >
> > unused?
> >
>
> It was my mistake, the correct sentence should be "Remove gpio from gpio section since it controlled by user space."
>
> > >
> > > Signed-off-by: Andrew Peng <pengms1 at lenovo.com>
> > > Signed-off-by: Derek Lin <dlin23 at lenovo.com>
> > > Signed-off-by: Yonghui Liu <liuyh21 at lenovo.com>
> >
> > I got two copies of this. I think they are the same.
> >
>
> I apologize to send twice.
>
> > > ---
> > > v1: initial version
> > >
> > >  arch/arm/boot/dts/aspeed-bmc-lenovo-hr855xg2.dts | 557
> > > ++++++++++++++++-------
> > >  1 file changed, 382 insertions(+), 175 deletions(-)
> > >
> > > diff --git a/arch/arm/boot/dts/aspeed-bmc-lenovo-hr855xg2.dts
> > > b/arch/arm/boot/dts/aspeed-bmc-lenovo-hr855xg2.dts
> > > index 8193fad..e1386d4 100644
> > > --- a/arch/arm/boot/dts/aspeed-bmc-lenovo-hr855xg2.dts
> > > +++ b/arch/arm/boot/dts/aspeed-bmc-lenovo-hr855xg2.dts
> > > @@ -15,14 +15,21 @@
> > >         compatible = "lenovo,hr855xg2-bmc", "aspeed,ast2500";
> > >
> >
> > > -               flash_memory: region at 98000000 {
> > > +               flash_memory: region at 9EFF0000 {
> > >                         no-map;
> > > -                       reg = <0x98000000 0x00100000>; /* 1M */
> > > +                       reg = <0x9EFF0000 0x00010000>; /* 64K */
> >
> > Do you really use 64K here, or was this a workaround for the lpc-ctlr driver
> > requiring a memory region?
> >
>
> We reserve 64K for L2A In-Band Firmware Update (phosphor-ipmi-flash).

Okay, thanks for clarifying.

I am happy with rest of your responses. Please send v2 with these things fixed.

Cheers,

Joel