On Mon, Oct 04, 2021 at 19:03:40 +0700, Nhi Pham wrote:
> Hi Leif,
>
> There are two comments that I would like to clarify with you.
>
> On 23/09/2021 20:49, Leif Lindholm wrote:
> > > +VOID
> > > +Ac01PcieMmioWr (
> > > + UINT64 Addr,
> > > + UINT32 Val
> > > + )
> > > +{
> > > + Ac01PcieCsrOut32Serdes ((VOID *)Addr, (UINT32)Val);
> > > +}
> > > +
> > > +VOID
> > > +Ac01PciePuts (
> > Wait, what. We have *two* sets of output overlays in this patch?
>
> This function is consumed by PCIe PHY library (PHYLib). We are making this
> wrapper function to conform with the function prototype defined by PHYLib.
>
> We will reduce DEBUG_PCIE_PHY by using directly the DEBUG (). Does it look
> good to you?
It's still a little bit awkward, but if that's the only way to get
debug out of PCYLib...
> > > +VOID
> > > +Ac01PcieDelay (
> > > + UINT32 Val
> > > + )
> > > +{
> > > + MicroSecondDelay (Val);
> > No, use MicroSecondDelay directly.
>
> Seems above. This wrapper function is to conform with the function prototype
> consumed by PHYLib. It's hard to change it.
So, I'm assuming this PHYLib is a library shared across multiple
codebases?
Would it be possible for PHYLib to link in ArmArchTimerLib directly
and wrap this there instead? Something about this integration just
feels kind of backwards to me.
Because we now have (sort of) an undocumented dircular dependency:
this driver declares a dependency of PciePhyLib, but that library
needs to be manually initialized by this driver.
/
Leif
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81520): https://edk2.groups.io/g/devel/message/81520
Mute This Topic: https://groups.io/mt/85631150/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
