[PATCH v2 09/39] x86: apl: Move p2sb ofdata reading to the correct method

[Simon Glass]

Hi Andy,

On Tue, 10 Mar 2020 at 08:39, Andy Shevchenko
<andriy.shevchenko at linux.intel.com> wrote:
>
> On Sun, Mar 08, 2020 at 09:44:33PM -0600, Simon Glass 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.
>
> >               if (plat->bdf < 0)
> >                       return log_msg_ret("Cannot get p2sb PCI address",
> > -                                        plat->bdf);
> > +                                             plat->bdf);
>
> Not sure I understand this hunk WRT the patch itself.

This is to fix a checkpatch error - it reports problems in nearby lines.

>
> > +     if (spl_phase() == PHASE_TPL)
> >               return p2sb_early_init(dev);
>
> > +     else if (spl_phase() == PHASE_SPL)
>
> Redundant 'else', but I think we already discussed that and you prefer this
> way. However, I think this is waste of compilation time. In any case, matter
> of taste.

Yes. I feel that it shows that there are two options.

Just for fun I wrote a program to try to benchmark the difference and
it does not seem to be detectable.

>
> > +             return p2sb_spl_init(dev);

Regards,
Simon