Re: [edk2-platforms: PATCH v2 06/14] Marvell/Armada7k8k: Add PciExpressLib implementation

["Marcin Wojtas" <mw@semihalf.com>]

pt., 24 maj 2019 o 15:08 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a):
>
> On Fri, 24 May 2019 at 15:03, Marcin Wojtas <mw@semihalf.com> wrote:
> >
> > pt., 24 maj 2019 o 14:50 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a):
> > >
> > > On Mon, 20 May 2019 at 17:27, Marcin Wojtas <mw@semihalf.com> wrote:
> > > >
> > > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > >
> > > > Implement a special version of PciExpressLib that takes the quirky
> > > > nature of the Synopsys Designware PCIe IP into account. In particular,
> > > > we need to ignore config space accesses to all devices on the first
> > > > bus except device 0, because the broadcast nature of type 0 configuration
> > > > cycles will result in whatever device is in the slot to appear at each
> > > > of the 32 device positions.
> > > >
> > >
> > > I never bothered to implement multisegment support for this SoC, since
> > > MacchiatoBin has only one segment wired up, but since your interest is
> > > in generic support, it might make sense to drop this patch and
> > > implement PciSegmentLib instead (without depending on any of the other
> > > library classes that the generic PciExpressLib depends on)
> > >
> >
> > This was (and still is) my plan, but I've been having some serious
> > time shortages for extra development. In order not to postpone this
> > support any longer I prefer to get merged, what I have and possibly
> > rework on top.
> >
> > About depending on a generic PciExpressLib - do you mean I can filter
> > out devices from bus0 in PciSegmentLib?
> >
>
> Yes, please look at Silicon/Socionext/SynQuacer/Library/SynQuacerPciSegmentLib
>
> If you follow the same approach, you no longer need PciLib or
> PciExpressLib, it is all flattened into PciSegmentLib.

For PciLib I use MdePkg implementation, but if that could be dropped
that's good. I saw your library and it would be perfect for reusing on
Armada, with one difference: PciSegmentLibGetConfigBase.

1. As a first step I could reuse Synquacer library using single base
in PciSegmentLibGetConfigBase (PcdPciExpressBaseAddress) as a stub,
for extending later.

2. Afterwards (i.e. in some next patchset) I'd like to use my board
description infrastructure for obtaining config space base addresses.
How about following solution:
- add PciSegmentLib constructor routine, where I'd create a global
array with config space addesses
- add dispatching for it in PciSegmentLibGetConfigBase?

Looking forward to your feedback. Other than that - do you have any
remarks to the rest of the patches in v2?

Thanks,
Marcin