From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f47.google.com (mail-wr1-f47.google.com [209.85.221.47]) by mx.groups.io with SMTP id smtpd.web09.1954.1633463997510772373 for ; Tue, 05 Oct 2021 12:59:58 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20210112.gappssmtp.com header.s=20210112 header.b=2HsqDl6K; spf=pass (domain: nuviainc.com, ip: 209.85.221.47, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f47.google.com with SMTP id r18so1292394wrg.6 for ; Tue, 05 Oct 2021 12:59:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=HNB6WvUdf7gbXyrBM3vhSgMsRA/7X5EBRD+4mVIakYo=; b=2HsqDl6Kpg9XiMvG62M/b3AWLAUYl9umYwMXKr7KtW6oNqm1ZWBSLu13FvsgPGL5JW sA1OsXCxX71oMO57sjXrj8RrT3SXjGVW+ok41e2/ag0IUCCEwJMjzDXZz2kb9jvtR763 Zi5AAkCVIW8yc0y+P3OIqaAxbGe0fZyEjTzSFSf8KwfdnNTt8KuBdSzvZfxUYsyngZ7L PMMikTvyeBX+EdY4y9gkQsJVdw3bZX+qwvQfZIcqOI5TnxqiUAM6wofJ4cr2w75sKGtB vH0khjU284+hukWXn+G4HKLdpR1n4dVHWAyzHgINk76ic6a45MS0KhTEr5NSIK2hwZrm 8d8w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=HNB6WvUdf7gbXyrBM3vhSgMsRA/7X5EBRD+4mVIakYo=; b=FidKr1XqFNUc05K65g0Bq5L7GJ5SSHTf7rQhcUkxvjkyNw3sFTb4z5+7JJwCF9Erzx /NqY0JjKgxBWLTeQb+DOirXkKDCMDSlWg93JpQ8yP76j+ZSRCsKTUNKEPnUQ9brqbDz/ jtPVPTjFgkn3nfcFQIVMdSkMsHQtD3eXWWl0sM3aprIQm7s70WlBbYM/Ytp15ny/L4cO yspjbZxQRGQ/CXWmpCPB9kxDvP8HM0Y+qnWhoqG4OnY50Oz4lD4QGizcwqLhq0aVNa/O JzxL3niBOUEA/dSrh6iITPvKTiGvo+r8ANpLSnDuoS5PjnAcbh5I9Rp/3mJqtMuXGpTd S6uQ== X-Gm-Message-State: AOAM533sqZo+ERCuu7EAtGWTjE1A7X0gB03LFEmTctZlHvGQvrpGBkDB ncLPsoPdwixgnaJOS+OP4JOPrA== X-Google-Smtp-Source: ABdhPJytdpVOXrhqd155ICWT3JZjcKa9qsGCgYiwcp9hnHxc1l+ZXXEywpOMvEfcaG4rmjxPTBfiKg== X-Received: by 2002:adf:f481:: with SMTP id l1mr23678368wro.411.1633463996041; Tue, 05 Oct 2021 12:59:56 -0700 (PDT) Return-Path: Received: from leviathan (cpc92314-cmbg19-2-0-cust559.5-4.cable.virginm.net. [82.11.186.48]) by smtp.gmail.com with ESMTPSA id b16sm6354912wrw.46.2021.10.05.12.59.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 Oct 2021 12:59:55 -0700 (PDT) Date: Tue, 5 Oct 2021 20:59:53 +0100 From: "Leif Lindholm" To: Nhi Pham Cc: devel@edk2.groups.io, patches@amperecomputing.com, vunguyen@os.amperecomputing.com, Thang Nguyen , Chuong Tran , Phong Vo , Michael D Kinney , Ard Biesheuvel , Nate DeSimone Subject: Re: [PATCH v3 12/28] AmpereAltraPkg: Add Ac01PcieLib library instance Message-ID: <20211005195953.eqmym4dkkkpqatik@leviathan> References: <20210915155527.8176-1-nhi@os.amperecomputing.com> <20210915155527.8176-13-nhi@os.amperecomputing.com> <20210923134931.gzxueebb4axflhdi@leviathan> MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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