linux-kernel - RE: [PATCH 0/4] Remove LPC register partitioning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <HK0PR06MB33800F282095AA96884B2FC0F2350@HK0PR06MB3380.apcprd06.prod.outlook.com>
Date:   Mon, 28 Sep 2020 07:43:25 +0000
From:   Ryan Chen <ryan_chen@...eedtech.com>
To:     ChiaWei Wang <chiawei_wang@...eedtech.com>,
        Andrew Jeffery <andrew@...id.au>, Joel Stanley <joel@....id.au>
CC:     Rob Herring <robh+dt@...nel.org>, Corey Minyard <minyard@....org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Haiyue Wang <haiyue.wang@...ux.intel.com>,
        Cyril Bur <cyrilbur@...il.com>,
        Robert Lippert <rlippert@...gle.com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        linux-aspeed <linux-aspeed@...ts.ozlabs.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        OpenBMC Maillist <openbmc@...ts.ozlabs.org>
Subject: RE: [PATCH 0/4] Remove LPC register partitioning

Hello Joel & Andrew,
	Those patches are more organize for ASPEED SOC LPC register layout. 
	Does those patches have any feedback?
	
Ryan

> -----Original Message-----
> From: ChiaWei Wang <chiawei_wang@...eedtech.com>
> Sent: Friday, September 11, 2020 4:21 PM
> To: Andrew Jeffery <andrew@...id.au>; Joel Stanley <joel@....id.au>
> Cc: Rob Herring <robh+dt@...nel.org>; Corey Minyard <minyard@....org>;
> Linus Walleij <linus.walleij@...aro.org>; Haiyue Wang
> <haiyue.wang@...ux.intel.com>; Cyril Bur <cyrilbur@...il.com>; Robert
> Lippert <rlippert@...gle.com>; Linux ARM
> <linux-arm-kernel@...ts.infradead.org>; linux-aspeed
> <linux-aspeed@...ts.ozlabs.org>; Linux Kernel Mailing List
> <linux-kernel@...r.kernel.org>; OpenBMC Maillist
> <openbmc@...ts.ozlabs.org>; Ryan Chen <ryan_chen@...eedtech.com>
> Subject: RE: [PATCH 0/4] Remove LPC register partitioning
> 
> Hello,
> 
> Thanks for your prompt feedback.
> 
> > -----Original Message-----
> > From: Andrew Jeffery <andrew@...id.au>
> > Sent: Friday, September 11, 2020 12:46 PM
> > To: Joel Stanley <joel@....id.au>; ChiaWei Wang
> > <chiawei_wang@...eedtech.com>
> > Subject: Re: [PATCH 0/4] Remove LPC register partitioning
> >
> >
> > On Fri, 11 Sep 2020, at 13:33, Joel Stanley wrote:
> > > Hello,
> > >
> > > On Fri, 11 Sep 2020 at 03:46, Chia-Wei, Wang
> > > <chiawei_wang@...eedtech.com> wrote:
> > > >
> > > > The LPC controller has no concept of the BMC and the Host partitions.
> > > > The incorrect partitioning can impose unnecessary range
> > > > restrictions on register access through the syscon regmap interface.
> > > >
> > > > For instance, HICRB contains the I/O port address configuration of
> > > > KCS channel 1/2. However, the KCS#1/#2 drivers cannot access HICRB
> > > > as it is located at the other LPC partition.
> >
> > Thanks for addressing this, I've regretted that choice for a while now.
> >
> > The split was rooted in trying to support pinmux while not being
> > across every detail of the LPC controller, and so I made some poor decisions.
> >
> > > >
> > > > In addition, to be backward compatible, the newly added HW control
> > > > bits could be added at any reserved bits over the LPC addressing space.
> > > >
> > > > Thereby, this patch series aims to remove the LPC partitioning for
> > > > better driver development and maintenance.
> > >
> > > I support this cleanup. The only consideration is to be careful with
> > > breaking the driver/device-tree relationship. We either need to
> > > ensure the drivers remain compatible with  both device trees.
> > >
> > > Another solution is to get agreement from all parties that for the
> > > LPC device the device tree is always the one shipped with the
> > > kernel, so it is okay to make incompatible changes.
> If it is possible, I would prefer this solution to avoid adding additional if-logic
> for the compatibility support in the driver implementation.
> As the patch can be less change made to register offset definitions and leave
> the core logic untouched.
> > >
> > > While we are doing a cleanup, Andrew suggested we remove the
> > > detailed description of LPC out of the device tree. We would have
> > > the one LPC node, and create a LPC driver that creates all of the
> > > sub devices (snoop, FW cycles, kcs, bt, vuart). Andrew, can  you
> > > elaborate on this plan?
> >
> > I dug up the conversation I had with Rob over a year ago about being
> > unhappy with what I'd cooked up.
> >
> > https://lore.kernel.org/linux-arm-kernel/CAL_JsqJ+sFDG8eKbV3gvmqVHx+ot
> > W
> > bki4dY213apzXgfhbXXEw@...l.gmail.com/
> >
> > But I think you covered most of the idea there: We have the LPC driver
> > create the subdevices and that moves the details out of the devicetree.
> > However, I haven't thought about it more than that, and I think there
> > are still problems with that idea. For instance, how we manage
> > configuration of those devices, and how to enable only the devices a
> > given platform actually cares about (i.e. the problems that devicetree solves
> for us).
> Another concern to make centralized LPC driver implementation more
> complicated is the relationship with eSPI driver.
> AST2500 binds the reset control of LPC and eSPI together. If eSPI is used for the
> Host communication, the behavior in current "lpc-ctrl" should be skipped but
> not for KCS, BT, Snoop, etc.
> And this will be much easier to achieve by devicetree if LPC sub devices are
> individually described.
> >
> > It may be that the only way to do that is with platform code, and
> > that's not really a direction we should be going either.
> >