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

["Pankaj Bansal" <pankaj.bansal@nxp.com>]

> -----Original Message-----
> From: Leif Lindholm <leif@nuviainc.com>
> Sent: Thursday, April 23, 2020 5:36 PM
> To: Pankaj Bansal (OSS) <pankaj.bansal@oss.nxp.com>
> Cc: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>; Michael D Kinney
> <michael.d.kinney@intel.com>; devel@edk2.groups.io; Varun Sethi
> <V.Sethi@nxp.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-
> Mahmoud@arm.com>; Jon Nettleton <jon@solid-run.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>
> Subject: Re: [PATCH edk2-platforms v3 16/24] Silicon/NXP: Add Chassis2
> Package
> 
> On Thu, Apr 23, 2020 at 12:02:16 +0000, Pankaj Bansal (OSS) wrote:
> >
> >
> > > -----Original Message-----
> > > From: Leif Lindholm <leif@nuviainc.com>
> > > Sent: Thursday, April 23, 2020 5:27 PM
> > > To: Pankaj Bansal (OSS) <pankaj.bansal@oss.nxp.com>
> > > Cc: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>; Michael D
> Kinney
> > > <michael.d.kinney@intel.com>; devel@edk2.groups.io; Varun Sethi
> > > <V.Sethi@nxp.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-
> > > Mahmoud@arm.com>; Jon Nettleton <jon@solid-run.com>; Ard Biesheuvel
> > > <ard.biesheuvel@linaro.org>
> > > Subject: Re: [PATCH edk2-platforms v3 16/24] Silicon/NXP: Add Chassis2
> > > Package
> > >
> > > 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.
> >
> > You mean put ChassisLibConstructor() call in SerialPortInitialize () ?
> 
> Or before the call to SerialPortInitialize in ChassisInit.

But SerialPortInitialize would be called from each module.
Each module that has SerialPort and ChassiLib linked to it would have a
local copy of mDcfgOps, which needs to be initialized.

> 
> /
>     Leif
> 
> >
> > >
> > > /
> > >     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
> > > > >
> > > >