Antwort: Re: [PATCH 009/108] x86: apl: Move p2sb ofdata reading to the correct method

[Wolfgang Wallner]

Hello Bin, Simon,

-----"Bin Meng" <bmeng.cn at gmail.com> schrieb: -----
> +Wolfgang Wallner
> 
> On Mon, Jan 27, 2020 at 1:08 PM Simon Glass <sjg at chromium.org> wrote:
> >
> > With P2SB the initial BAR (base-address register) is set up by TPL and
> > this is used unchanged right through U-Boot.
> >
> > At present the reading of this address is split between the ofdata() and
> > probe() methods. There are a few problems that are unique to the p2sb.
> > One is that its children need to call pcr_read32(), etc. which needs to
> > have the p2sb address correct. Also some of its children are pinctrl
> > devices and pinctrl is used when any device is probed. So p2sb really
> > needs to get its base address set up in ofdata_to_platdata(), before it is
> > probed.
> >
> > Another point is that reading the p2sb BAR will not work if the p2sb is
> > hidden. The FSP-S seems to hide it, presumably to avoid confusing PCI
> > enumeration.
> >
> > Reading ofdata in ofdata_to_platdata() is the correct place anyway, so
> > this is easy to fix.
> >
> > Move the code into one place and use the early-regs property in all cases
> > for simplicity and to avoid needing to probe any PCI devices just to read
> > the BAR.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> >  arch/x86/cpu/apollolake/p2sb.c | 33 +++++++++++----------------------
> >  1 file changed, 11 insertions(+), 22 deletions(-)
> >
> 
> I think this one needs to be rebased.
> 
> I suspect it breaks Wolfgang's use case. Wolfgang, any comments?

Thanks for pointing this out.

As far as I understand, the patch changes the following for my use case
(Coreboot + U-Boot, no TPL and no SPL):

* Before: plat->mmio_base was read from the PCI BAR in apl_p2sb_probe()
* After:  plat->mmio_base is read from the "early-regs" device tree property
          in apl_p2sb_ofdata_to_platdata()

I think this is fine, Coreboot does not change the address in BAR0 and I can
use the "early-regs" property in my device tree.

regards, Wolfgang

Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
Tested-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
          
Tested on an Apollo Lake board with Coreboot + U-Boot.