Re: [edk2-devel] [PATCH edk2-platforms v3 16/24] Silicon/NXP: Add Chassis2 Package

["Leif Lindholm" <leif@nuviainc.com>]

On Tue, Apr 28, 2020 at 19:46:36 +0200, Ard Biesheuvel wrote:
> On 4/24/20 5:51 PM, Leif Lindholm via groups.io wrote:
> > On Fri, Apr 24, 2020 at 02:42:13 +0000, Pankaj Bansal (OSS) wrote:
> > > > > > Why would multiple modules need to initialize the serial port?
> > > > > 
> > > > > That's how the DebugLib has been designed.
> > > > > DebugLib is used by all modules to print info on console.
> > > > > BaseDebugLibSerialPortConstructor calls SerialPortInitialize.
> > > > > So SerialPortInitialize is called by all the modules.
> > > > 
> > > > Sure, but the bit where ChassisLib returns the active clock
> > > > configuration does not need to happen for each initialization.
> > > > That value can be cached.
> > > 
> > > The only mechanism I know for passing a cached value between different modules
> > > is either use PCDs or use HOBs.
> > > We have already explored both in https://edk2.groups.io/g/devel/message/57254
> > > and https://edk2.groups.io/g/devel/message/56530
> > 
> > That was discussing what to do with regards to the generic 16550
> > driver. If we go with Laszlo's suggestion[1] for a separate
> > SerialUartClockLib instead of adding a vendor GUID HOB *into the
> > generic driver*, that does not preclude your using a HOB to cache the
> > value in your platform code for later use in your own
> > SerialUartClockLib implementation.
> > 
> > [1] https://edk2.groups.io/g/devel/message/56767
> > 
> 
> Caching using a HOB is a bit problematic, given that SerialPortInitialize()
> does not honour constructor ordering (it may be called before any of the
> constructors), and so whether we implement Laszlo's suggestion or not, using
> a HOB to cache anything that is required to set the correct baud rate is not
> going to work (given that HobLib may rely on its constructor to be able to
> access the HOB list)
> 
> Unfortunately, that leaves us with little else, given that we cannot use
> global variables either, since BASE libraries may be incorporated into PEIMs
> that run execute-in-place from ROM.
> 
> So the bottom line is that we don't have that many options that are actually
> feasible, and so converging on one of the non-optimal ones is all that we
> can really hope for at this point.

Argh, OK. I hadn't tweaked this - so thanks for pointing it out.

OK, given that:
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
for this patch in its current state.

Just, please use a single call to set up the struct pointer for future
components where that is possible.

/
    Leif