Message-ID: <470f6d93-4c83-e271-7e99-7e23f3f76e71@codeaurora.org>
Date:	Fri, 3 Jun 2016 10:59:35 -0600
From:	Jeffrey Hugo <jhugo@...eaurora.org>
To:	Gabriele Paoloni <gabriele.paoloni@...wei.com>,
	Christopher Covington <cov@...eaurora.org>,
	Tomasz Nowicki <tn@...ihalf.com>,
	"helgaas@...nel.org" <helgaas@...nel.org>,
	"arnd@...db.de" <arnd@...db.de>,
	"will.deacon@....com" <will.deacon@....com>,
	"catalin.marinas@....com" <catalin.marinas@....com>,
	"rafael@...nel.org" <rafael@...nel.org>,
	"hanjun.guo@...aro.org" <hanjun.guo@...aro.org>,
	"Lorenzo.Pieralisi@....com" <Lorenzo.Pieralisi@....com>,
	"okaya@...eaurora.org" <okaya@...eaurora.org>,
	"jchandra@...adcom.com" <jchandra@...adcom.com>
Cc:	"liudongdong (C)" <liudongdong3@...wei.com>,
	"linaro-acpi@...ts.linaro.org" <linaro-acpi@...ts.linaro.org>,
	"jcm@...hat.com" <jcm@...hat.com>,
	"dhdang@....com" <dhdang@....com>,
	"Liviu.Dudau@....com" <Liviu.Dudau@....com>,
	"ddaney@...iumnetworks.com" <ddaney@...iumnetworks.com>,
	"jeremy.linton@....com" <jeremy.linton@....com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	"robert.richter@...iumnetworks.com" 
	<robert.richter@...iumnetworks.com>,
	"msalter@...hat.com" <msalter@...hat.com>,
	"Suravee.Suthikulpanit@....com" <Suravee.Suthikulpanit@....com>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	Wangyijing <wangyijing@...wei.com>,
	"mw@...ihalf.com" <mw@...ihalf.com>,
	"andrea.gallo@...aro.org" <andrea.gallo@...aro.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [RFC PATCH 1/3] pci, acpi: Match PCI config space accessors
 against platfrom specific ECAM quirks.

On 6/3/2016 9:32 AM, Gabriele Paoloni wrote:
> Hi Cov
>
>> -----Original Message-----
>> From: linux-pci-owner@...r.kernel.org [mailto:linux-pci-
>> owner@...r.kernel.org] On Behalf Of Christopher Covington
>> Sent: 03 June 2016 16:15
>> To: Tomasz Nowicki; helgaas@...nel.org; arnd@...db.de;
>> will.deacon@....com; catalin.marinas@....com; rafael@...nel.org;
>> hanjun.guo@...aro.org; Lorenzo.Pieralisi@....com; okaya@...eaurora.org;
>> jchandra@...adcom.com
>> Cc: jcm@...hat.com; linaro-acpi@...ts.linaro.org; linux-
>> pci@...r.kernel.org; dhdang@....com; Liviu.Dudau@....com;
>> ddaney@...iumnetworks.com; jeremy.linton@....com; linux-
>> kernel@...r.kernel.org; linux-acpi@...r.kernel.org;
>> robert.richter@...iumnetworks.com; Suravee.Suthikulpanit@....com;
>> msalter@...hat.com; Wangyijing; mw@...ihalf.com;
>> andrea.gallo@...aro.org; linux-arm-kernel@...ts.infradead.org;
>> liudongdong (C); Gabriele Paoloni
>> Subject: Re: [RFC PATCH 1/3] pci, acpi: Match PCI config space
>> accessors against platfrom specific ECAM quirks.
>>
>> Hi Tomasz,
>>
>> Thanks for your work on this.
>>
>> On 06/02/2016 04:41 AM, 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. Algorithm traverses available quirk list, matches against
>>> <oem_id, oem_rev, domain, bus number> tuple and returns corresponding
>>> PCI config ops. oem_id and oem_rev come from MCFG table standard
>> header.
>>> All quirks can be defined using DECLARE_ACPI_MCFG_FIXUP() macro and
>>> kept self contained. Example:
>>>
>>> /* Custom PCI config ops */
>>> static struct pci_generic_ecam_ops foo_pci_ops = {
>>> 	.bus_shift	= 24,
>>> 	.pci_ops = {
>>> 		.map_bus = pci_ecam_map_bus,
>>> 		.read = foo_ecam_config_read,
>>> 		.write = foo_ecam_config_write,
>>> 	}
>>> };
>>>
>>> DECLARE_ACPI_MCFG_FIXUP(&foo_pci_ops, <oem_id_str>, <oem_rev>,
>> <domain_nr>, <bus_nr>);
>>>
>>> Signed-off-by: Tomasz Nowicki <tn@...ihalf.com>
>>> ---
>>>  drivers/acpi/pci_mcfg.c           | 32
>> ++++++++++++++++++++++++++++++++
>>>  include/asm-generic/vmlinux.lds.h |  7 +++++++
>>>  include/linux/pci-acpi.h          | 19 +++++++++++++++++++
>>>  3 files changed, 58 insertions(+)
>>>
>>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>>> index 1847f74..f3d4570 100644
>>> --- a/drivers/acpi/pci_mcfg.c
>>> +++ b/drivers/acpi/pci_mcfg.c
>>> @@ -22,11 +22,43 @@
>>>  #include <linux/kernel.h>
>>>  #include <linux/pci.h>
>>>  #include <linux/pci-acpi.h>
>>> +#include <linux/pci-ecam.h>
>>>
>>>  /* Root pointer to the mapped MCFG table */
>>>  static struct acpi_table_mcfg *mcfg_table;
>>>  static int mcfg_entries;
>>>
>>> +extern struct pci_cfg_fixup __start_acpi_mcfg_fixups[];
>>> +extern struct pci_cfg_fixup __end_acpi_mcfg_fixups[];
>>> +
>>> +struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root *root)
>>> +{
>>> +	int bus_num = root->secondary.start;
>>> +	int domain = root->segment;
>>> +	struct pci_cfg_fixup *f;
>>> +
>>> +	if (!mcfg_table)
>>> +		return &pci_generic_ecam_ops;
>>> +
>>> +	/*
>>> +	 * Match against platform specific quirks and return
>> corresponding
>>> +	 * CAM ops.
>>> +	 *
>>> +	 * First match against PCI topology <domain:bus> then use OEM ID
>> and
>>> +	 * OEM revision from MCFG table standard header.
>>> +	 */
>>> +	for (f = __start_acpi_mcfg_fixups; f < __end_acpi_mcfg_fixups;
>> f++) {
>>> +		if ((f->domain == domain || f->domain ==
>> PCI_MCFG_DOMAIN_ANY) &&
>>> +		    (f->bus_num == bus_num || f->bus_num ==
>> PCI_MCFG_BUS_ANY) &&
>>> +		    (!strncmp(f->oem_id, mcfg_table->header.oem_id,
>>> +			      ACPI_OEM_ID_SIZE)) &&
>>> +		    (f->oem_revision == mcfg_table->header.oem_revision))
>>
>> Is this more likely to be updated between quirky and fixed platforms
>> than oem_table_id? What do folks think about using oem_table_id instead
>> of, or in addition to, oem_revision?
>
> From my understanding we need to stick to this mechanism as (otherwise)
> there are platforms out in the field that would need a FW update.
>
> So I don't think that using oem_table_id "instead" is possible; about
> "in addition" I think it is doable, but I do not see the advantage much.
> I mean that if a platform gets fixed the oem revision should change too,
> Right?

Cov and I had a discussion about this, so hopefully I can bring a 
slightly different perspective that will make sense.

We forsee a situation where we have platform A that needs a quirk, and 
platform B that does not.  The OEM id is the same for both platforms as 
they are different platforms from the same OEM.  Using the OEM revision 
field does not seem to be appropriate since these are different 
platforms and the revision field appears to be for the purpose of 
tracking differences within a single platform.  Therefore, Cov is 
proposing using the OEM table id as a mechanism to distinguish platform 
A (needs quirk applied) vs platform B (no quirks) from the same OEM.

>
> Thanks
>
> Gab
>
>>
>> In case these details are helpful, here was my simple prototype of an
>> MCFG based approach:
>>
>> https://codeaurora.org/cgit/quic/server/kernel/commit/?h=cov/4.7-rc1-
>> testing&id=c5d8bc49a198fd8f61f82c7d8f169564d6176b07
>> https://codeaurora.org/cgit/quic/server/kernel/commit/?h=cov/4.7-rc1-
>> testing&id=50bfe77ccd1639e6ce8c7c4fcca187d50e0bead4
>>
>> Thanks,
>> Cov
>>
>> --
>> Qualcomm Innovation Center, Inc.
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> a Linux Foundation Collaborative Project
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@...r.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>


-- 
Jeffrey Hugo
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

Powered by blists - more mailing lists