Re: [PATCH V2 22/23] pci, acpi: Match PCI config space accessors against platfrom specific quirks.

[Mark Salter]

On Fri, 2016-01-08 at 15:36 +0100, Tomasz Nowicki wrote:
> On 08.01.2016 15:16, Mark Salter wrote:
> > On Wed, 2015-12-16 at 16:16 +0100, Tomasz Nowicki wrote:
> > > Some platforms may not be fully compliant with generic set of PCI config
> > > accessors. For these cases we implement the way to overwrite accessors
> > > set before PCI buses enumeration. Algorithm that overwrite accessors
> > > matches against platform ID (DMI), domain and bus number, hopefully
> > > enough for all cases. All quirks can be defined using:
> > > DECLARE_ACPI_MCFG_FIXUP() and keep self contained.
> > > 
> > > example:
> > > 
> > > static const struct dmi_system_id yyy[] = {
> > > ÂÂÂÂÂÂÂÂÂ{
> > > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ.ident = "<Platform ident string>",
> > > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ.callback = <handler>,
> > > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ.matches = {
> > > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂDMI_MATCH(DMI_SYS_VENDOR, "<system vendor>"),
> > > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂDMI_MATCH(DMI_PRODUCT_NAME, "<product name>"),
> > > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂDMI_MATCH(DMI_PRODUCT_VERSION, "product version"),
> > > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ},
> > > ÂÂÂÂÂÂÂÂÂ},
> > > ÂÂÂÂÂÂÂÂÂ{ }
> > > };
> > > 
> > 
> > This seems awkward to me in the case where the quirk is SoC-based and there
> > may be multiple platforms affected. Needing a DECLARE_ACPI_MCFG_FIXUP for
> > each platform using such a SoC (i.e. Mustang and Moonshot) doesn't seem
> > right. In that case, I think it'd be better to check CPUID and possibly
> > some SoC register to cover all platforms affected.
> 
> Right, my next version already has alternative to DMI match handler, soÂ
> there will be two ways to match:
> 1. DMI, like in this patch set
> 2. int (*match)(struct pci_mcfg_fixup *) where you can read CPUID, andÂ
> whatever is necessary.
> 

Great. Thanks.

> > 
> > Also, there doesn't seem to be a way to connect a given quirk check to the
> > MCFG/device requesting the ops. So if there is a platform with multiple PCIE
> > roots and not all of them have quirks, how does one no whether to override the
> > default ecam ops?
> > 
> 
> Then we can identify them using <domain:bus>. I was wondering to passÂ
> acpi device handler to match handler for the case where we need e.g.Â
> extra properties from related DSDT device descriptor. Does it make senseÂ
> to you?
> 

The thing with domain:bus is that it is really a firmware setting. So on one
platform domain 0 may be associated with hwdev X but on another platform it
may be associated with hwdev Y. But yes, an acpi handle would make it much
easier for the match handler to find out which hw dev it is being asked to
match.