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

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

On Thu, Apr 23, 2020 at 11:38:12 +0000, Pankaj Bansal (OSS) wrote:
> > > +
> > > +  @return The value read.
> > > +**/
> > > +UINT32
> > > +EFIAPI
> > > +DcfgRead32 (
> > > +  IN  UINTN     Address
> > > +  )
> > > +{
> > > +  MMIO_OPERATIONS_32  *DcfgOps;
> > > +
> > > +  DcfgOps = GetMmioOperations32 (FeaturePcdGet (PcdDcfgBigEndian));
> > > +
> > > +  return DcfgOps->Read32 (Address);
> > > +}
> > 
> > The intended usage model for IoAccessLib is to retrieve the function
> > pointer struct once and then always refer to it. Since this is a
> > library, we could have a CONSTRUCTOR function (specified in the .inf)
> 
> I had thought of this, but decided against it because of this reason:
>     The order of Library constructor call for a module cannot be guaranteed.
>     https://edk2.groups.io/g/devel/message/57254  is an good example of this.
>     BaseDebugLibSerialPortConstructor would need to depend on ChassisLibConstructor,
>     to retrieve the UART clock frequency. If the constructor calls are not guaranteed, BaseDebugLibSerialPortConstructor
>     would fail and it would cause ASSERT due to ASSERT_RETURN_ERROR (Status)

If this is a problem (and I recall Ard pointing out some shortcomings
in the dependency handling in the past), we can solve this with an
explicit initialisation call in the SoC or platform init code.

/
    Leif

> 
> > and do something like:
> > 
> > STATIC MMIO_OPERATIONS mDcfgOps;
> > 
> > /**
> >   Read Dcfg register
> > 
> >   @param  Address The MMIO register to read.
> > 
> >   @return The value read.
> > **/
> > UINT32
> > EFIAPI
> > DcfgRead32 (
> >   IN  UINTN     Address
> >   )
> > {
> >   return mDcfgOps->Read32 (Address);
> > }
> > 
> > /**
> >   Write Dcfg register
> > 
> >   @param  Address The MMIO register to write.
> >   @param  Value   The value to write to the MMIO register.
> > 
> >   @return Value.
> > 
> > **/
> > UINT32
> > EFIAPI
> > DcfgWrite32 (
> >   IN      UINTN                     Address,
> >   IN      UINT32                    Value
> >   )
> > {
> >   return mDcfgOps->Write32 (Address, Value);
> > }
> > 
> > ...
> > 
> > /**
> >   The constructor function initializes the IoAccessLib
> >   function pointer structure.
> > 
> >   @retval RETURN_SUCCESS   The constructor always returns EFI_SUCCESS.
> > 
> > **/
> > EFI_STATUS
> > EFIAPI
> > ChassisLibConstructor (
> >   VOID
> >   )
> > {
> >   mDcfgOps = GetMmioOperations (FeaturePcdGet (PcdDcfgBigEndian));
> > 
> >   return EFI_SUCCESS;
> > }
> > 
> > /
> >     Leif
> > 
>