From: "Leif Lindholm" <leif@nuviainc.com>
To: Nhi Pham <nhi@os.amperecomputing.com>
Cc: devel@edk2.groups.io, patches@amperecomputing.com,
vunguyen@os.amperecomputing.com,
Thang Nguyen <thang@os.amperecomputing.com>,
Chuong Tran <chuong@os.amperecomputing.com>,
Phong Vo <phong@os.amperecomputing.com>,
Michael D Kinney <michael.d.kinney@intel.com>,
Ard Biesheuvel <ardb+tianocore@kernel.org>,
Nate DeSimone <nathaniel.l.desimone@intel.com>
Subject: Re: [PATCH v3 12/28] AmpereAltraPkg: Add Ac01PcieLib library instance
Date: Tue, 5 Oct 2021 20:59:53 +0100 [thread overview]
Message-ID: <20211005195953.eqmym4dkkkpqatik@leviathan> (raw)
In-Reply-To: <bcca1b86-239e-7401-6960-a050867e5615@os.amperecomputing.com>
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
next prev parent reply other threads:[~2021-10-05 19:59 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-15 15:54 [PATCH v3 00/28] Add new Ampere Mt. Jade platform Nhi Pham
2021-09-15 15:55 ` [PATCH v3 01/28] Ampere: Initial support for Ampere Altra processor and " Nhi Pham
2021-09-16 10:40 ` Leif Lindholm
2021-09-16 10:46 ` Leif Lindholm
2021-09-17 6:19 ` Nhi Pham
2021-09-23 13:54 ` Leif Lindholm
2021-09-15 15:55 ` [PATCH v3 02/28] AmpereAltraPkg: Add MmCommunication modules Nhi Pham
2021-09-15 15:55 ` [PATCH v3 03/28] AmperePlatformPkg: Implement FailSafe library Nhi Pham
2021-09-15 15:55 ` [PATCH v3 04/28] AmperePlatformPkg: Add FailSafe and WDT support Nhi Pham
2021-09-16 11:22 ` Leif Lindholm
2021-09-15 15:55 ` [PATCH v3 05/28] AmpereAltraPkg: Add DwI2cLib library Nhi Pham
2021-09-15 15:55 ` [PATCH v3 06/28] AmpereAltraPkg: Add DwGpioLib library Nhi Pham
2021-09-15 15:55 ` [PATCH v3 07/28] JadePkg: Implement RealTimeClockLib for PCF85063 Nhi Pham
2021-09-15 15:55 ` [PATCH v3 08/28] AmpereAltraPkg: Add BootProgress support Nhi Pham
2021-09-15 15:55 ` [PATCH v3 09/28] AmpereAltraPkg: Support UEFI non-volatile variable Nhi Pham
2021-09-23 11:49 ` Leif Lindholm
2021-09-15 15:55 ` [PATCH v3 10/28] AmpereSiliconPkg: Add PlatformManagerUiLib library instance Nhi Pham
2021-09-15 15:55 ` [PATCH v3 11/28] AmpereAltraPkg, JadePkg: Add ACPI support Nhi Pham
2021-09-15 15:55 ` [PATCH v3 12/28] AmpereAltraPkg: Add Ac01PcieLib library instance Nhi Pham
2021-09-23 13:49 ` Leif Lindholm
2021-09-27 14:33 ` Nhi Pham
2021-10-04 12:03 ` Nhi Pham
2021-10-05 19:59 ` Leif Lindholm [this message]
2021-10-06 12:55 ` [edk2-devel] " Nhi Pham
2021-10-07 9:06 ` Leif Lindholm
2021-10-07 9:13 ` Nhi Pham
2021-09-28 10:34 ` Leif Lindholm
2021-10-04 11:36 ` Nhi Pham
2021-09-15 15:55 ` [PATCH v3 13/28] JadePkg: Add BoardPcieLib " Nhi Pham
2021-09-24 12:35 ` Leif Lindholm
2021-09-15 15:55 ` [PATCH v3 14/28] Ampere: PCIe: Add PciHostBridgeLib " Nhi Pham
2021-09-24 13:12 ` Ard Biesheuvel
2021-09-15 15:55 ` [PATCH v3 15/28] Ampere: PCIe: Add PciSegmentLib " Nhi Pham
2021-09-24 13:16 ` Ard Biesheuvel
2021-09-28 10:26 ` Leif Lindholm
2021-09-15 15:55 ` [PATCH v3 16/28] JadePkg: Enable PCIe-related libraries and device drivers Nhi Pham
2021-09-15 15:55 ` [PATCH v3 17/28] JadePkg: Add ASpeed GOP driver Nhi Pham
2021-09-15 15:55 ` [PATCH v3 18/28] Ampere: PCIe: Add PciPlatformDxe driver Nhi Pham
2021-09-24 12:59 ` Leif Lindholm
2021-09-15 15:55 ` [PATCH v3 19/28] AmpereAltraPkg: Add Random Number Generator Support Nhi Pham
2021-09-15 15:55 ` [PATCH v3 20/28] JadePkg: Add SMBIOS tables support Nhi Pham
2021-09-15 15:55 ` [PATCH v3 21/28] AmpereAltraPkg: Add DebugInfoPei module Nhi Pham
2021-09-24 12:43 ` Leif Lindholm
2021-09-15 15:55 ` [PATCH v3 22/28] AmpereAltraPkg: Add platform info screen Nhi Pham
2021-09-15 15:55 ` [PATCH v3 23/28] AmpereAltraPkg: Add configuration screen for memory Nhi Pham
2021-09-24 12:50 ` Leif Lindholm
2021-09-15 15:55 ` [PATCH v3 24/28] AmpereAltraPkg: Add configuration screen for CPU Nhi Pham
2021-09-24 12:51 ` Leif Lindholm
2021-09-15 15:55 ` [PATCH v3 25/28] AmpereAltraPkg: Add configuration screen for ACPI Nhi Pham
2021-09-24 12:53 ` Leif Lindholm
2021-09-15 15:55 ` [PATCH v3 26/28] AmpereAltraPkg: Add configuration screen for RAS Nhi Pham
2021-09-24 12:54 ` Leif Lindholm
2021-09-15 15:55 ` [PATCH v3 27/28] AmpereAltraPkg: Add configuration screen for Watchdog timer Nhi Pham
2021-09-24 12:55 ` Leif Lindholm
2021-09-15 15:55 ` [PATCH v3 28/28] AmpereAltraPkg: Add configuration screen for Pcie Devices Nhi Pham
2021-09-24 12:57 ` Leif Lindholm
2021-09-16 10:09 ` [PATCH v3 00/28] Add new Ampere Mt. Jade platform Leif Lindholm
2021-09-17 6:10 ` Nhi Pham
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211005195953.eqmym4dkkkpqatik@leviathan \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox