On Mon, Dec 13, 2021 at 8:08 PM Ard Biesheuvel <a...@kernel.org> wrote: > > On Mon, 13 Dec 2021 at 13:41, chandni cherukuri <ch.chan...@gmail.com> wrote: > > > > Hi Ard, > > > > Thanks for the response. > > > > I had a look at the Synquacer platform where there is a single custom > > PCIe library implemented. > > So, is it better to implement a single custom PCIe library instead of > > two custom PCIe libraries? > > > > Yes, for two reasons: > - there is prior art for this solution, and solving the same problem > in two different ways in the same codebase is bad form; > - those two libraries are layered one on top of the other, which means > that your fix requires changes to two different layers, making the > interface between those two layers obviously unfit to represent the > hardware in question. > > Thanks for the response. Will implement a single custom PCIe library and post a new patchset.
Thanks Chandni > > If that is the case then will implement a single custom PCIe library > > for Morello. > > > > Thanks > > Chandni > > On Fri, Dec 10, 2021 at 4:12 PM Ard Biesheuvel <a...@kernel.org> wrote: > > > > > > On Thu, 9 Dec 2021 at 13:30, chandni cherukuri <ch.chan...@gmail.com> > > > wrote: > > > > > > > > Hi Ard, Leif, Sami, Pierre, > > > > > > > > For Morello platform, we created two custom libraries based on the > > > > below two native libraries: > > > > - > > > > https://github.com/tianocore/edk2/tree/master/MdePkg/Library/BasePciSegmentLibPci > > > > - > > > > https://github.com/tianocore/edk2/tree/master/MdePkg/Library/BasePciExpressLib > > > > > > > > because of the following reasons. > > > > 1. In Morello platform, the ECAM region of the two root ports > > > > are placed in non-contiguous memory as per the memory map architecture > > > > of the Morello platform. The native PCI Express Library expects both > > > > the ECAM regions for all the root ports to be contiguous. > > > > > > Do you mean root ports or host bridges? If root ports don't share the > > > MMIO resource windows provided by the respective host bridges, they > > > are really different host bridges and should be modeled as such. > > > > > > Note that the Cadence and Synopsys IP usually does not implement a > > > root port at all, and gets away with this because it only implements a > > > single link. > > > > > > > 2. IORT and kernel currently require each root port to be in a > > > > separate segment. You can refer to the code for more details - > > > > > > > > > > Please take a look at the SynQuaecer platform for inspiration. That > > > platform is very similar in this respect. > > > > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/acpi/arm64/iort.c#n307. > > > > The native PCI Segment Library supports only a single segment. > > > > > > > > Because of these reasons, the current patch series adapts the native > > > > libraries as follows: > > > > > > > > The custom PCI Segment Library passes the complete address which is > > > > consumed by the custom PCI Express library where based on the Segment > > > > number, the base address of the PCI Express is returned. > > > > > > > > The rationale behind maintaining two separate libraries is that when > > > > the software evolves and support for multiple segments is adapted in > > > > the native PCI Segment Library the custom library could be removed. > > > > Also, we might have other protocols which might try to use the PCI > > > > Express library directly. However, there are some other platforms > > > > where all the platform specific changes have gone in a single custom > > > > PCI library > > > > > > > > Could you please provide some inputs as to which of two approaches > > > > would be better to follow as there is one more platform to follow > > > > based on the decision taken? > > > > > > > > Thanks > > > > Chandni > > > > > > > > On Wed, Dec 8, 2021 at 8:25 AM Khasim Mohammed > > > > <khasim.moham...@arm.com> wrote: > > > > > > > > > > Hi Sami, Chandni, > > > > > > > > > > There was a suggestion from Pierre on a similar patch for N1SDP to > > > > > remove PCIExpressLib.c and move to workarounds to PCISegmentLib.c, > > > > > > > > > > https://edk2.groups.io/g/devel/message/84165?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3Arecentpostdate%2Fsticky%2C%2Ckhasim%2C20%2C2%2C0%2C87257273 > > > > > > > > > > I think we should discuss this implementation for both N1SDP and > > > > > Morello as they have similar implementation. > > > > > > > > > > Regards, > > > > > Khasim > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#84747): https://edk2.groups.io/g/devel/message/84747 Mute This Topic: https://groups.io/mt/87497024/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
