Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Duc Dang]

On Mon, Jun 13, 2016 at 8:59 AM, Jeffrey Hugo <jh...@codeaurora.org> wrote:
> On 6/13/2016 9:12 AM, ok...@codeaurora.org wrote:
>>
>> On 2016-06-13 10:29, Gabriele Paoloni wrote:
>>>
>>> Hi Sinan
>>>
>>>> -Original Message-
>>>> From: Sinan Kaya [mailto:ok...@codeaurora.org]
>>>> Sent: 13 June 2016 15:03
>>>> To: Gabriele Paoloni; liudongdong (C); helg...@kernel.org;
>>>> a...@arndb.de; will.dea...@arm.com; catalin.mari...@arm.com;
>>>> raf...@kernel.org; hanjun@linaro.org; lorenzo.pieral...@arm.com;
>>>> jchan...@broadcom.com; t...@semihalf.com
>>>> Cc: robert.rich...@caviumnetworks.com; m...@semihalf.com;
>>>> liviu.du...@arm.com; dda...@caviumnetworks.com; Wangyijing;
>>>> suravee.suthikulpa...@amd.com; msal...@redhat.com; linux-
>>>> p...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
>>>> a...@vger.kernel.org; linux-kernel@vger.kernel.org; linaro-
>>>> a...@lists.linaro.org; j...@redhat.com; andrea.ga...@linaro.org;
>>>> dhd...@apm.com; jeremy.lin...@arm.com; c...@codeaurora.org; Chenxin
>>>> (Charles); Linuxarm
>>>> Subject: Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space
>>>> accessors against platfrom specific ECAM quirks
>>>>
>>>> On 6/13/2016 9:54 AM, Gabriele Paoloni wrote:
>>>> > As you can see here Liudongdong has replaced oem_revision with
>>>> > oem_table_id.
>>>> >
>>>> > Now it seems that there are some platforms that have already shipped
>>>> > using a matching based on the oem_revision (right Jon?)
>>>> >
>>>> > However I guess that if in FW they have defined oem_table_id properly
>>>> > they should be able to use this mechanism without needing to a FW
>>>> update.
>>>> >
>>>> > Can these vendors confirm this?
>>>> >
>>>> > Tomasz do you think this can work for Cavium Thunder?
>>>> >
>>>> > Thanks
>>>> >
>>>> > Gab
>>>>
>>>> Why not have all three of them?
>>>>
>>>> The initial approach was OEM id and revision id.
>>>>
>>>> Jeff Hugo indicated that addition (not removing any other fields) of
>>>> table id
>>>> would make more sense.
>>>
>>>
>>> Mmm from last email of Jeff Hugo on "[RFC PATCH 1/3] pci, acpi: Match
>>> PCI config space accessors against platfrom specific ECAM quirks."
>>>
>>> I quote:
>>>
>>>  "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."
>>>
>>> So it looks to me that he pointed out that using the OEM revision field
>>> is wrong...and this is why I have asked if replacing it with the table
>>> id can work for other vendors
>>>
>>> Thanks
>>>
>>> Gab
>>>
>>
>> I had an internal discussion with jeff and cov before posting on the
>> maillist.
>>
>> I think there is missing info in the email.
>>
>> Usage of oem id + table id + revision is ok.
>>
>> Usage of oem id + revision is not ok as one oem can build multiple chips
>> with the same oem id and revision id but different table id. Otherwise,
>> we can run out of revisions very quickly.
>
>
> Agreed.
>
> I'm sorry for the confusion.  My intent was to point out that revision alone
> appeared insufficient to address all the identified problems, but I believe
> there is still a case for using revision. Table id is useful for
> differentiating between platforms/chips.  Revision is useful for
> differentiation between different versions of a single platform/chip
> assuming the silicon is respun or some other fix is applied.  Both solve
> different scenarios, and I'm not aware of a reason why they could not be
> used together to solve all currently identified cases.

Using OEM ID + Table ID + Revision will work for X-Gene platforms as well.

Regards,
Duc Dang.
>
>>
>>>
>>>>
>>>> --
>>>> Sinan Kaya
>>>> Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center,
>>>> Inc.
>>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
>>>> Linux Foundation Collaborative Project
>>
>>
>> ___
>> linux-arm-kernel mailing list
>> linux-arm-ker...@lists.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

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Duc Dang]

On Mon, Jun 13, 2016 at 8:59 AM, Jeffrey Hugo  wrote:
> On 6/13/2016 9:12 AM, ok...@codeaurora.org wrote:
>>
>> On 2016-06-13 10:29, Gabriele Paoloni wrote:
>>>
>>> Hi Sinan
>>>
>>>> -Original Message-
>>>> From: Sinan Kaya [mailto:ok...@codeaurora.org]
>>>> Sent: 13 June 2016 15:03
>>>> To: Gabriele Paoloni; liudongdong (C); helg...@kernel.org;
>>>> a...@arndb.de; will.dea...@arm.com; catalin.mari...@arm.com;
>>>> raf...@kernel.org; hanjun@linaro.org; lorenzo.pieral...@arm.com;
>>>> jchan...@broadcom.com; t...@semihalf.com
>>>> Cc: robert.rich...@caviumnetworks.com; m...@semihalf.com;
>>>> liviu.du...@arm.com; dda...@caviumnetworks.com; Wangyijing;
>>>> suravee.suthikulpa...@amd.com; msal...@redhat.com; linux-
>>>> p...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
>>>> a...@vger.kernel.org; linux-kernel@vger.kernel.org; linaro-
>>>> a...@lists.linaro.org; j...@redhat.com; andrea.ga...@linaro.org;
>>>> dhd...@apm.com; jeremy.lin...@arm.com; c...@codeaurora.org; Chenxin
>>>> (Charles); Linuxarm
>>>> Subject: Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space
>>>> accessors against platfrom specific ECAM quirks
>>>>
>>>> On 6/13/2016 9:54 AM, Gabriele Paoloni wrote:
>>>> > As you can see here Liudongdong has replaced oem_revision with
>>>> > oem_table_id.
>>>> >
>>>> > Now it seems that there are some platforms that have already shipped
>>>> > using a matching based on the oem_revision (right Jon?)
>>>> >
>>>> > However I guess that if in FW they have defined oem_table_id properly
>>>> > they should be able to use this mechanism without needing to a FW
>>>> update.
>>>> >
>>>> > Can these vendors confirm this?
>>>> >
>>>> > Tomasz do you think this can work for Cavium Thunder?
>>>> >
>>>> > Thanks
>>>> >
>>>> > Gab
>>>>
>>>> Why not have all three of them?
>>>>
>>>> The initial approach was OEM id and revision id.
>>>>
>>>> Jeff Hugo indicated that addition (not removing any other fields) of
>>>> table id
>>>> would make more sense.
>>>
>>>
>>> Mmm from last email of Jeff Hugo on "[RFC PATCH 1/3] pci, acpi: Match
>>> PCI config space accessors against platfrom specific ECAM quirks."
>>>
>>> I quote:
>>>
>>>  "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."
>>>
>>> So it looks to me that he pointed out that using the OEM revision field
>>> is wrong...and this is why I have asked if replacing it with the table
>>> id can work for other vendors
>>>
>>> Thanks
>>>
>>> Gab
>>>
>>
>> I had an internal discussion with jeff and cov before posting on the
>> maillist.
>>
>> I think there is missing info in the email.
>>
>> Usage of oem id + table id + revision is ok.
>>
>> Usage of oem id + revision is not ok as one oem can build multiple chips
>> with the same oem id and revision id but different table id. Otherwise,
>> we can run out of revisions very quickly.
>
>
> Agreed.
>
> I'm sorry for the confusion.  My intent was to point out that revision alone
> appeared insufficient to address all the identified problems, but I believe
> there is still a case for using revision. Table id is useful for
> differentiating between platforms/chips.  Revision is useful for
> differentiation between different versions of a single platform/chip
> assuming the silicon is respun or some other fix is applied.  Both solve
> different scenarios, and I'm not aware of a reason why they could not be
> used together to solve all currently identified cases.

Using OEM ID + Table ID + Revision will work for X-Gene platforms as well.

Regards,
Duc Dang.
>
>>
>>>
>>>>
>>>> --
>>>> Sinan Kaya
>>>> Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center,
>>>> Inc.
>>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
>>>> Linux Foundation Collaborative Project
>>
>>
>> ___
>> linux-arm-kernel mailing list
>> linux-arm-ker...@lists.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

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Duc Dang]

On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
 wrote:
> Hi Dongdong,
>
> On 06/13/2016 09:02 AM, Dongdong Liu wrote:
>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>> index d3c3e85..49612b3 100644
>> --- a/drivers/acpi/pci_mcfg.c
>> +++ b/drivers/acpi/pci_mcfg.c
>> @@ -22,6 +22,10 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>> +
>> +/* Root pointer to the mapped MCFG table */
>> +static struct acpi_table_mcfg *mcfg_table;
>>
>>  /* Structure to hold entries from the MCFG table */
>>  struct mcfg_entry {
>> @@ -35,6 +39,38 @@ struct mcfg_entry {
>>  /* List to save mcfg entries */
>>  static LIST_HEAD(pci_mcfg_list);
>>
>> +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 _generic_ecam_ops;
>> +
>> + /*
>> +  * Match against platform specific quirks and return corresponding
>> +  * CAM ops.
>> +  *
>> +  * First match against PCI topology  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)) &&
>> + (!strncmp(f->oem_table_id, mcfg_table->header.oem_table_id,
>> +   ACPI_OEM_TABLE_ID_SIZE)))
>
> This would just be a small convenience, but if the character count used here 
> were
>
> min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)
>
> then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be substrings and
> wouldn't need to be padded out to the full length.
>
>> + return f->ops;
>> + }
>> + /* No quirks, use ECAM */
>> + return _generic_ecam_ops;
>> +}
>
>> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
>> index 7d63a66..088a1da 100644
>> --- a/include/linux/pci-acpi.h
>> +++ b/include/linux/pci-acpi.h
>> @@ -25,6 +25,7 @@ static inline acpi_status 
>> pci_acpi_remove_pm_notifier(struct acpi_device *dev)
>>  extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle);
>>
>>  extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource *bus_res);
>> +extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root *root);
>>
>>  static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
>>  {
>> @@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
>>   int (*prepare_resources)(struct acpi_pci_root_info *info);
>>  };
>>
>> +struct pci_cfg_fixup {
>> + struct pci_ecam_ops *ops;
>> + char *oem_id;
>> + char *oem_table_id;
>> + int domain;
>> + int bus_num;
>> +};
>> +
>> +#define PCI_MCFG_DOMAIN_ANY  -1
>> +#define PCI_MCFG_BUS_ANY -1
>> +
>> +/* Designate a routine to fix up buggy MCFG */
>> +#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom, bus) \
>> + static const struct pci_cfg_fixup   \
>> + __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
>
> I'm not entirely sure that this is the right fix--I'm pretty blindly
> following a GCC documentation suggestion [1]--but removing the first two
> preprocessor concatenation operators "##" solved the following build error
> for me.
>
> include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and ""QCOM"" 
> does not give a valid preprocessing token
>   __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \

I think the problem is gcc is not happy with quoted string when
processing these tokens
(""QCOM"", the extra "" are added by gcc). So should we not concat
string tokens and
use the fixup definition in v1 of this RFC:
/* Designate a routine to fix up buggy MCFG */
#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, rev, dom, bus) \
static const struct pci_cfg_fixup __mcfg_fixup_##system##dom##bus\
 __used __attribute__((__section__(".acpi_fixup_mcfg"), \
aligned((sizeof(void *) =   \
{ ops, oem_id, rev, dom, bus };

Regards,
Duc Dang.


>   ^
> arch/arm64/kernel/pci.c:225:1: note: in expansion of macro 
> ‘DECLARE_ACPI_MCFG_FIXUP’
>  DECLARE_ACPI_MCFG_FIXUP(_32b_ecam_ops, "QCOM", "QDF2432", 
> PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);
>  ^
> arch/arm64/kernel/pci.c:225:44: error: pasting ""QCOM"" and ""QDF2432"" does 
> not give a valid preprocessing token
>  DECLARE_ACPI_MCFG_FIXUP(_32b_ecam_ops, "QCOM", "QDF2432", 
> PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);
>

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Duc Dang]

On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
 wrote:
> Hi Dongdong,
>
> On 06/13/2016 09:02 AM, Dongdong Liu wrote:
>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>> index d3c3e85..49612b3 100644
>> --- a/drivers/acpi/pci_mcfg.c
>> +++ b/drivers/acpi/pci_mcfg.c
>> @@ -22,6 +22,10 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>> +
>> +/* Root pointer to the mapped MCFG table */
>> +static struct acpi_table_mcfg *mcfg_table;
>>
>>  /* Structure to hold entries from the MCFG table */
>>  struct mcfg_entry {
>> @@ -35,6 +39,38 @@ struct mcfg_entry {
>>  /* List to save mcfg entries */
>>  static LIST_HEAD(pci_mcfg_list);
>>
>> +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 _generic_ecam_ops;
>> +
>> + /*
>> +  * Match against platform specific quirks and return corresponding
>> +  * CAM ops.
>> +  *
>> +  * First match against PCI topology  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)) &&
>> + (!strncmp(f->oem_table_id, mcfg_table->header.oem_table_id,
>> +   ACPI_OEM_TABLE_ID_SIZE)))
>
> This would just be a small convenience, but if the character count used here 
> were
>
> min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)
>
> then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be substrings and
> wouldn't need to be padded out to the full length.
>
>> + return f->ops;
>> + }
>> + /* No quirks, use ECAM */
>> + return _generic_ecam_ops;
>> +}
>
>> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
>> index 7d63a66..088a1da 100644
>> --- a/include/linux/pci-acpi.h
>> +++ b/include/linux/pci-acpi.h
>> @@ -25,6 +25,7 @@ static inline acpi_status 
>> pci_acpi_remove_pm_notifier(struct acpi_device *dev)
>>  extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle);
>>
>>  extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource *bus_res);
>> +extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root *root);
>>
>>  static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
>>  {
>> @@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
>>   int (*prepare_resources)(struct acpi_pci_root_info *info);
>>  };
>>
>> +struct pci_cfg_fixup {
>> + struct pci_ecam_ops *ops;
>> + char *oem_id;
>> + char *oem_table_id;
>> + int domain;
>> + int bus_num;
>> +};
>> +
>> +#define PCI_MCFG_DOMAIN_ANY  -1
>> +#define PCI_MCFG_BUS_ANY -1
>> +
>> +/* Designate a routine to fix up buggy MCFG */
>> +#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom, bus) \
>> + static const struct pci_cfg_fixup   \
>> + __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
>
> I'm not entirely sure that this is the right fix--I'm pretty blindly
> following a GCC documentation suggestion [1]--but removing the first two
> preprocessor concatenation operators "##" solved the following build error
> for me.
>
> include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and ""QCOM"" 
> does not give a valid preprocessing token
>   __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \

I think the problem is gcc is not happy with quoted string when
processing these tokens
(""QCOM"", the extra "" are added by gcc). So should we not concat
string tokens and
use the fixup definition in v1 of this RFC:
/* Designate a routine to fix up buggy MCFG */
#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, rev, dom, bus) \
static const struct pci_cfg_fixup __mcfg_fixup_##system##dom##bus\
 __used __attribute__((__section__(".acpi_fixup_mcfg"), \
aligned((sizeof(void *) =   \
{ ops, oem_id, rev, dom, bus };

Regards,
Duc Dang.


>   ^
> arch/arm64/kernel/pci.c:225:1: note: in expansion of macro 
> ‘DECLARE_ACPI_MCFG_FIXUP’
>  DECLARE_ACPI_MCFG_FIXUP(_32b_ecam_ops, "QCOM", "QDF2432", 
> PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);
>  ^
> arch/arm64/kernel/pci.c:225:44: error: pasting ""QCOM"" and ""QDF2432"" does 
> not give a valid preprocessing token
>  DECLARE_ACPI_MCFG_FIXUP(_32b_ecam_ops, "QCOM", "QDF2432", 
> PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);
> ^
> 

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Duc Dang]

On Tue, Jun 14, 2016 at 4:52 AM, Tomasz Nowicki  wrote:
> On 14.06.2016 11:45, Dongdong Liu wrote:
>>
>> Hi Duc
>>
>> 在 2016/6/14 17:00, Duc Dang 写道:
>>>
>>> On Mon, Jun 13, 2016 at 10:51 PM, Dongdong Liu
>>>  wrote:

 Hi Duc

 在 2016/6/14 4:57, Duc Dang 写道:
>
>
> On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
>  wrote:
>>
>>
>> Hi Dongdong,
>>
>> On 06/13/2016 09:02 AM, Dongdong Liu wrote:
>>>
>>>
>>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>>> index d3c3e85..49612b3 100644
>>> --- a/drivers/acpi/pci_mcfg.c
>>> +++ b/drivers/acpi/pci_mcfg.c
>>> @@ -22,6 +22,10 @@
>>>#include 
>>>#include 
>>>#include 
>>> +#include 
>>> +
>>> +/* Root pointer to the mapped MCFG table */
>>> +static struct acpi_table_mcfg *mcfg_table;
>>>
>>>/* Structure to hold entries from the MCFG table */
>>>struct mcfg_entry {
>>> @@ -35,6 +39,38 @@ struct mcfg_entry {
>>>/* List to save mcfg entries */
>>>static LIST_HEAD(pci_mcfg_list);
>>>
>>> +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 _generic_ecam_ops;
>>> +
>>> + /*
>>> +  * Match against platform specific quirks and return
>>> corresponding
>>> +  * CAM ops.
>>> +  *
>>> +  * First match against PCI topology  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)) &&
>>> + (!strncmp(f->oem_table_id,
>>> mcfg_table->header.oem_table_id,
>>> +   ACPI_OEM_TABLE_ID_SIZE)))
>>
>>
>>
>> This would just be a small convenience, but if the character count
>> used
>> here were
>>
>> min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)
>>
>> then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be
>> substrings
>> and
>> wouldn't need to be padded out to the full length.
>>
>>> + return f->ops;
>>> + }
>>> + /* No quirks, use ECAM */
>>> + return _generic_ecam_ops;
>>> +}
>>
>>
>>
>>> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
>>> index 7d63a66..088a1da 100644
>>> --- a/include/linux/pci-acpi.h
>>> +++ b/include/linux/pci-acpi.h
>>> @@ -25,6 +25,7 @@ static inline acpi_status
>>> pci_acpi_remove_pm_notifier(struct acpi_device *dev)
>>>extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle
>>> handle);
>>>
>>>extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource
>>> *bus_res);
>>> +extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root
>>> *root);
>>>
>>>static inline acpi_handle acpi_find_root_bridge_handle(struct
>>> pci_dev
>>> *pdev)
>>>{
>>> @@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
>>> int (*prepare_resources)(struct acpi_pci_root_info *info);
>>>};
>>>
>>> +struct pci_cfg_fixup {
>>> + struct pci_ecam_ops *ops;
>>> + char *oem_id;
>>> + char *oem_table_id;
>>> + int domain;
>>> + int bus_num;
>>> +};
>>> +
>>> +#define PCI_MCFG_DOMAIN_ANY  -1
>>> +#define PCI_MCFG_BUS_ANY -1
>>> +
>>> +/* Designate a routine to fix up buggy MCFG */
>>> +#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom,
>>> bus) \
>>> + static const struct
>>> pci_cfg_fixup   \
>>> +
>>> __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
>>
>>
>>
>> I'm not entirely sure that this is the right fix--I'm pretty blindly
>> following a GCC documentation suggestion [1]--but removing the
>> first two
>> preprocessor concatenation operators "##" solved the following build
>> error
>> for me.
>>
>> include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and
>> ""QCOM"" does not give a valid preprocessing token
>> 

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Duc Dang]

On Tue, Jun 14, 2016 at 4:52 AM, Tomasz Nowicki  wrote:
> On 14.06.2016 11:45, Dongdong Liu wrote:
>>
>> Hi Duc
>>
>> 在 2016/6/14 17:00, Duc Dang 写道:
>>>
>>> On Mon, Jun 13, 2016 at 10:51 PM, Dongdong Liu
>>>  wrote:

 Hi Duc

 在 2016/6/14 4:57, Duc Dang 写道:
>
>
> On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
>  wrote:
>>
>>
>> Hi Dongdong,
>>
>> On 06/13/2016 09:02 AM, Dongdong Liu wrote:
>>>
>>>
>>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>>> index d3c3e85..49612b3 100644
>>> --- a/drivers/acpi/pci_mcfg.c
>>> +++ b/drivers/acpi/pci_mcfg.c
>>> @@ -22,6 +22,10 @@
>>>#include 
>>>#include 
>>>#include 
>>> +#include 
>>> +
>>> +/* Root pointer to the mapped MCFG table */
>>> +static struct acpi_table_mcfg *mcfg_table;
>>>
>>>/* Structure to hold entries from the MCFG table */
>>>struct mcfg_entry {
>>> @@ -35,6 +39,38 @@ struct mcfg_entry {
>>>/* List to save mcfg entries */
>>>static LIST_HEAD(pci_mcfg_list);
>>>
>>> +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 _generic_ecam_ops;
>>> +
>>> + /*
>>> +  * Match against platform specific quirks and return
>>> corresponding
>>> +  * CAM ops.
>>> +  *
>>> +  * First match against PCI topology  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)) &&
>>> + (!strncmp(f->oem_table_id,
>>> mcfg_table->header.oem_table_id,
>>> +   ACPI_OEM_TABLE_ID_SIZE)))
>>
>>
>>
>> This would just be a small convenience, but if the character count
>> used
>> here were
>>
>> min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)
>>
>> then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be
>> substrings
>> and
>> wouldn't need to be padded out to the full length.
>>
>>> + return f->ops;
>>> + }
>>> + /* No quirks, use ECAM */
>>> + return _generic_ecam_ops;
>>> +}
>>
>>
>>
>>> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
>>> index 7d63a66..088a1da 100644
>>> --- a/include/linux/pci-acpi.h
>>> +++ b/include/linux/pci-acpi.h
>>> @@ -25,6 +25,7 @@ static inline acpi_status
>>> pci_acpi_remove_pm_notifier(struct acpi_device *dev)
>>>extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle
>>> handle);
>>>
>>>extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource
>>> *bus_res);
>>> +extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root
>>> *root);
>>>
>>>static inline acpi_handle acpi_find_root_bridge_handle(struct
>>> pci_dev
>>> *pdev)
>>>{
>>> @@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
>>> int (*prepare_resources)(struct acpi_pci_root_info *info);
>>>};
>>>
>>> +struct pci_cfg_fixup {
>>> + struct pci_ecam_ops *ops;
>>> + char *oem_id;
>>> + char *oem_table_id;
>>> + int domain;
>>> + int bus_num;
>>> +};
>>> +
>>> +#define PCI_MCFG_DOMAIN_ANY  -1
>>> +#define PCI_MCFG_BUS_ANY -1
>>> +
>>> +/* Designate a routine to fix up buggy MCFG */
>>> +#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom,
>>> bus) \
>>> + static const struct
>>> pci_cfg_fixup   \
>>> +
>>> __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
>>
>>
>>
>> I'm not entirely sure that this is the right fix--I'm pretty blindly
>> following a GCC documentation suggestion [1]--but removing the
>> first two
>> preprocessor concatenation operators "##" solved the following build
>> error
>> for me.
>>
>> include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and
>> ""QCOM"" does not give a valid preprocessing token
>> __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
>
>
>
> I think the problem is gcc 

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Duc Dang]

On Mon, Jun 13, 2016 at 10:51 PM, Dongdong Liu  wrote:
> Hi Duc
>
> 在 2016/6/14 4:57, Duc Dang 写道:
>>
>> On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
>>  wrote:
>>>
>>> Hi Dongdong,
>>>
>>> On 06/13/2016 09:02 AM, Dongdong Liu wrote:

 diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
 index d3c3e85..49612b3 100644
 --- a/drivers/acpi/pci_mcfg.c
 +++ b/drivers/acpi/pci_mcfg.c
 @@ -22,6 +22,10 @@
   #include 
   #include 
   #include 
 +#include 
 +
 +/* Root pointer to the mapped MCFG table */
 +static struct acpi_table_mcfg *mcfg_table;

   /* Structure to hold entries from the MCFG table */
   struct mcfg_entry {
 @@ -35,6 +39,38 @@ struct mcfg_entry {
   /* List to save mcfg entries */
   static LIST_HEAD(pci_mcfg_list);

 +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 _generic_ecam_ops;
 +
 + /*
 +  * Match against platform specific quirks and return corresponding
 +  * CAM ops.
 +  *
 +  * First match against PCI topology  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)) &&
 + (!strncmp(f->oem_table_id,
 mcfg_table->header.oem_table_id,
 +   ACPI_OEM_TABLE_ID_SIZE)))
>>>
>>>
>>> This would just be a small convenience, but if the character count used
>>> here were
>>>
>>> min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)
>>>
>>> then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be substrings
>>> and
>>> wouldn't need to be padded out to the full length.
>>>
 + return f->ops;
 + }
 + /* No quirks, use ECAM */
 + return _generic_ecam_ops;
 +}
>>>
>>>
 diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
 index 7d63a66..088a1da 100644
 --- a/include/linux/pci-acpi.h
 +++ b/include/linux/pci-acpi.h
 @@ -25,6 +25,7 @@ static inline acpi_status
 pci_acpi_remove_pm_notifier(struct acpi_device *dev)
   extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle);

   extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource
 *bus_res);
 +extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root
 *root);

   static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev
 *pdev)
   {
 @@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
int (*prepare_resources)(struct acpi_pci_root_info *info);
   };

 +struct pci_cfg_fixup {
 + struct pci_ecam_ops *ops;
 + char *oem_id;
 + char *oem_table_id;
 + int domain;
 + int bus_num;
 +};
 +
 +#define PCI_MCFG_DOMAIN_ANY  -1
 +#define PCI_MCFG_BUS_ANY -1
 +
 +/* Designate a routine to fix up buggy MCFG */
 +#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom, bus) \
 + static const struct pci_cfg_fixup   \
 + __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
>>>
>>>
>>> I'm not entirely sure that this is the right fix--I'm pretty blindly
>>> following a GCC documentation suggestion [1]--but removing the first two
>>> preprocessor concatenation operators "##" solved the following build
>>> error
>>> for me.
>>>
>>> include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and
>>> ""QCOM"" does not give a valid preprocessing token
>>>__mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
>>
>>
>> I think the problem is gcc is not happy with quoted string when
>> processing these tokens
>> (""QCOM"", the extra "" are added by gcc). So should we not concat
>> string tokens and
>> use the fixup definition in v1 of this RFC:
>> /* Designate a routine to fix up buggy MCFG */
>> #define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, rev, dom, bus) \
>>  static const struct pci_cfg_fixup
>> __mcfg_fixup_##system##dom##bus\
>>   __used __attribute__((__section__(".acpi_fixup_mcfg"), \
>>  aligned((sizeof(void *) =   \
>>  { ops, oem_id, rev, dom, bus };
>
>
> V1 fixup exist the 

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Duc Dang]

On Mon, Jun 13, 2016 at 10:51 PM, Dongdong Liu  wrote:
> Hi Duc
>
> 在 2016/6/14 4:57, Duc Dang 写道:
>>
>> On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
>>  wrote:
>>>
>>> Hi Dongdong,
>>>
>>> On 06/13/2016 09:02 AM, Dongdong Liu wrote:

 diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
 index d3c3e85..49612b3 100644
 --- a/drivers/acpi/pci_mcfg.c
 +++ b/drivers/acpi/pci_mcfg.c
 @@ -22,6 +22,10 @@
   #include 
   #include 
   #include 
 +#include 
 +
 +/* Root pointer to the mapped MCFG table */
 +static struct acpi_table_mcfg *mcfg_table;

   /* Structure to hold entries from the MCFG table */
   struct mcfg_entry {
 @@ -35,6 +39,38 @@ struct mcfg_entry {
   /* List to save mcfg entries */
   static LIST_HEAD(pci_mcfg_list);

 +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 _generic_ecam_ops;
 +
 + /*
 +  * Match against platform specific quirks and return corresponding
 +  * CAM ops.
 +  *
 +  * First match against PCI topology  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)) &&
 + (!strncmp(f->oem_table_id,
 mcfg_table->header.oem_table_id,
 +   ACPI_OEM_TABLE_ID_SIZE)))
>>>
>>>
>>> This would just be a small convenience, but if the character count used
>>> here were
>>>
>>> min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)
>>>
>>> then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be substrings
>>> and
>>> wouldn't need to be padded out to the full length.
>>>
 + return f->ops;
 + }
 + /* No quirks, use ECAM */
 + return _generic_ecam_ops;
 +}
>>>
>>>
 diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
 index 7d63a66..088a1da 100644
 --- a/include/linux/pci-acpi.h
 +++ b/include/linux/pci-acpi.h
 @@ -25,6 +25,7 @@ static inline acpi_status
 pci_acpi_remove_pm_notifier(struct acpi_device *dev)
   extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle);

   extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource
 *bus_res);
 +extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root
 *root);

   static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev
 *pdev)
   {
 @@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
int (*prepare_resources)(struct acpi_pci_root_info *info);
   };

 +struct pci_cfg_fixup {
 + struct pci_ecam_ops *ops;
 + char *oem_id;
 + char *oem_table_id;
 + int domain;
 + int bus_num;
 +};
 +
 +#define PCI_MCFG_DOMAIN_ANY  -1
 +#define PCI_MCFG_BUS_ANY -1
 +
 +/* Designate a routine to fix up buggy MCFG */
 +#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom, bus) \
 + static const struct pci_cfg_fixup   \
 + __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
>>>
>>>
>>> I'm not entirely sure that this is the right fix--I'm pretty blindly
>>> following a GCC documentation suggestion [1]--but removing the first two
>>> preprocessor concatenation operators "##" solved the following build
>>> error
>>> for me.
>>>
>>> include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and
>>> ""QCOM"" does not give a valid preprocessing token
>>>__mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
>>
>>
>> I think the problem is gcc is not happy with quoted string when
>> processing these tokens
>> (""QCOM"", the extra "" are added by gcc). So should we not concat
>> string tokens and
>> use the fixup definition in v1 of this RFC:
>> /* Designate a routine to fix up buggy MCFG */
>> #define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, rev, dom, bus) \
>>  static const struct pci_cfg_fixup
>> __mcfg_fixup_##system##dom##bus\
>>   __used __attribute__((__section__(".acpi_fixup_mcfg"), \
>>  aligned((sizeof(void *) =   \
>>  { ops, oem_id, rev, dom, bus };
>
>
> V1 fixup exist the redefinition error when compiling mutiple
> 

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Duc Dang]

On Tue, Jun 14, 2016 at 4:52 AM, Tomasz Nowicki  wrote:
> On 14.06.2016 11:45, Dongdong Liu wrote:
>>
>> Hi Duc
>>
>> 在 2016/6/14 17:00, Duc Dang 写道:
>>>
>>> On Mon, Jun 13, 2016 at 10:51 PM, Dongdong Liu
>>>  wrote:

 Hi Duc

 在 2016/6/14 4:57, Duc Dang 写道:
>
>
> On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
>  wrote:
>>
>>
>> Hi Dongdong,
>>
>> On 06/13/2016 09:02 AM, Dongdong Liu wrote:
>>>
>>>
>>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>>> index d3c3e85..49612b3 100644
>>> --- a/drivers/acpi/pci_mcfg.c
>>> +++ b/drivers/acpi/pci_mcfg.c
>>> @@ -22,6 +22,10 @@
>>>#include 
>>>#include 
>>>#include 
>>> +#include 
>>> +
>>> +/* Root pointer to the mapped MCFG table */
>>> +static struct acpi_table_mcfg *mcfg_table;
>>>
>>>/* Structure to hold entries from the MCFG table */
>>>struct mcfg_entry {
>>> @@ -35,6 +39,38 @@ struct mcfg_entry {
>>>/* List to save mcfg entries */
>>>static LIST_HEAD(pci_mcfg_list);
>>>
>>> +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 _generic_ecam_ops;
>>> +
>>> + /*
>>> +  * Match against platform specific quirks and return
>>> corresponding
>>> +  * CAM ops.
>>> +  *
>>> +  * First match against PCI topology  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)) &&
>>> + (!strncmp(f->oem_table_id,
>>> mcfg_table->header.oem_table_id,
>>> +   ACPI_OEM_TABLE_ID_SIZE)))
>>
>>
>>
>> This would just be a small convenience, but if the character count
>> used
>> here were
>>
>> min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)
>>
>> then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be
>> substrings
>> and
>> wouldn't need to be padded out to the full length.
>>
>>> + return f->ops;
>>> + }
>>> + /* No quirks, use ECAM */
>>> + return _generic_ecam_ops;
>>> +}
>>
>>
>>
>>> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
>>> index 7d63a66..088a1da 100644
>>> --- a/include/linux/pci-acpi.h
>>> +++ b/include/linux/pci-acpi.h
>>> @@ -25,6 +25,7 @@ static inline acpi_status
>>> pci_acpi_remove_pm_notifier(struct acpi_device *dev)
>>>extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle
>>> handle);
>>>
>>>extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource
>>> *bus_res);
>>> +extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root
>>> *root);
>>>
>>>static inline acpi_handle acpi_find_root_bridge_handle(struct
>>> pci_dev
>>> *pdev)
>>>{
>>> @@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
>>> int (*prepare_resources)(struct acpi_pci_root_info *info);
>>>};
>>>
>>> +struct pci_cfg_fixup {
>>> + struct pci_ecam_ops *ops;
>>> + char *oem_id;
>>> + char *oem_table_id;
>>> + int domain;
>>> + int bus_num;
>>> +};
>>> +
>>> +#define PCI_MCFG_DOMAIN_ANY  -1
>>> +#define PCI_MCFG_BUS_ANY -1
>>> +
>>> +/* Designate a routine to fix up buggy MCFG */
>>> +#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom,
>>> bus) \
>>> + static const struct
>>> pci_cfg_fixup   \
>>> +
>>> __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
>>
>>
>>
>> I'm not entirely sure that this is the right fix--I'm pretty blindly
>> following a GCC documentation suggestion [1]--but removing the
>> first two
>> preprocessor concatenation operators "##" solved the following build
>> error
>> for me.
>>
>> include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and
>> ""QCOM"" does not give a valid preprocessing token
>> 

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Duc Dang]

On Tue, Jun 14, 2016 at 4:52 AM, Tomasz Nowicki  wrote:
> On 14.06.2016 11:45, Dongdong Liu wrote:
>>
>> Hi Duc
>>
>> 在 2016/6/14 17:00, Duc Dang 写道:
>>>
>>> On Mon, Jun 13, 2016 at 10:51 PM, Dongdong Liu
>>>  wrote:

 Hi Duc

 在 2016/6/14 4:57, Duc Dang 写道:
>
>
> On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
>  wrote:
>>
>>
>> Hi Dongdong,
>>
>> On 06/13/2016 09:02 AM, Dongdong Liu wrote:
>>>
>>>
>>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>>> index d3c3e85..49612b3 100644
>>> --- a/drivers/acpi/pci_mcfg.c
>>> +++ b/drivers/acpi/pci_mcfg.c
>>> @@ -22,6 +22,10 @@
>>>#include 
>>>#include 
>>>#include 
>>> +#include 
>>> +
>>> +/* Root pointer to the mapped MCFG table */
>>> +static struct acpi_table_mcfg *mcfg_table;
>>>
>>>/* Structure to hold entries from the MCFG table */
>>>struct mcfg_entry {
>>> @@ -35,6 +39,38 @@ struct mcfg_entry {
>>>/* List to save mcfg entries */
>>>static LIST_HEAD(pci_mcfg_list);
>>>
>>> +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 _generic_ecam_ops;
>>> +
>>> + /*
>>> +  * Match against platform specific quirks and return
>>> corresponding
>>> +  * CAM ops.
>>> +  *
>>> +  * First match against PCI topology  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)) &&
>>> + (!strncmp(f->oem_table_id,
>>> mcfg_table->header.oem_table_id,
>>> +   ACPI_OEM_TABLE_ID_SIZE)))
>>
>>
>>
>> This would just be a small convenience, but if the character count
>> used
>> here were
>>
>> min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)
>>
>> then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be
>> substrings
>> and
>> wouldn't need to be padded out to the full length.
>>
>>> + return f->ops;
>>> + }
>>> + /* No quirks, use ECAM */
>>> + return _generic_ecam_ops;
>>> +}
>>
>>
>>
>>> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
>>> index 7d63a66..088a1da 100644
>>> --- a/include/linux/pci-acpi.h
>>> +++ b/include/linux/pci-acpi.h
>>> @@ -25,6 +25,7 @@ static inline acpi_status
>>> pci_acpi_remove_pm_notifier(struct acpi_device *dev)
>>>extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle
>>> handle);
>>>
>>>extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource
>>> *bus_res);
>>> +extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root
>>> *root);
>>>
>>>static inline acpi_handle acpi_find_root_bridge_handle(struct
>>> pci_dev
>>> *pdev)
>>>{
>>> @@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
>>> int (*prepare_resources)(struct acpi_pci_root_info *info);
>>>};
>>>
>>> +struct pci_cfg_fixup {
>>> + struct pci_ecam_ops *ops;
>>> + char *oem_id;
>>> + char *oem_table_id;
>>> + int domain;
>>> + int bus_num;
>>> +};
>>> +
>>> +#define PCI_MCFG_DOMAIN_ANY  -1
>>> +#define PCI_MCFG_BUS_ANY -1
>>> +
>>> +/* Designate a routine to fix up buggy MCFG */
>>> +#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom,
>>> bus) \
>>> + static const struct
>>> pci_cfg_fixup   \
>>> +
>>> __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
>>
>>
>>
>> I'm not entirely sure that this is the right fix--I'm pretty blindly
>> following a GCC documentation suggestion [1]--but removing the
>> first two
>> preprocessor concatenation operators "##" solved the following build
>> error
>> for me.
>>
>> include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and
>> ""QCOM"" does not give a valid preprocessing token
>> __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
>
>
>
> I think the problem is gcc 

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Tomasz Nowicki]

On 14.06.2016 11:45, Dongdong Liu wrote:

Hi Duc

在 2016/6/14 17:00, Duc Dang 写道:

On Mon, Jun 13, 2016 at 10:51 PM, Dongdong Liu
 wrote:

Hi Duc

在 2016/6/14 4:57, Duc Dang 写道:


On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
 wrote:


Hi Dongdong,

On 06/13/2016 09:02 AM, Dongdong Liu wrote:


diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
index d3c3e85..49612b3 100644
--- a/drivers/acpi/pci_mcfg.c
+++ b/drivers/acpi/pci_mcfg.c
@@ -22,6 +22,10 @@
   #include 
   #include 
   #include 
+#include 
+
+/* Root pointer to the mapped MCFG table */
+static struct acpi_table_mcfg *mcfg_table;

   /* Structure to hold entries from the MCFG table */
   struct mcfg_entry {
@@ -35,6 +39,38 @@ struct mcfg_entry {
   /* List to save mcfg entries */
   static LIST_HEAD(pci_mcfg_list);

+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 _generic_ecam_ops;
+
+ /*
+  * Match against platform specific quirks and return
corresponding
+  * CAM ops.
+  *
+  * First match against PCI topology  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)) &&
+ (!strncmp(f->oem_table_id,
mcfg_table->header.oem_table_id,
+   ACPI_OEM_TABLE_ID_SIZE)))



This would just be a small convenience, but if the character count
used
here were

min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)

then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be
substrings
and
wouldn't need to be padded out to the full length.


+ return f->ops;
+ }
+ /* No quirks, use ECAM */
+ return _generic_ecam_ops;
+}




diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index 7d63a66..088a1da 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -25,6 +25,7 @@ static inline acpi_status
pci_acpi_remove_pm_notifier(struct acpi_device *dev)
   extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle
handle);

   extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource
*bus_res);
+extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root
*root);

   static inline acpi_handle acpi_find_root_bridge_handle(struct
pci_dev
*pdev)
   {
@@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
int (*prepare_resources)(struct acpi_pci_root_info *info);
   };

+struct pci_cfg_fixup {
+ struct pci_ecam_ops *ops;
+ char *oem_id;
+ char *oem_table_id;
+ int domain;
+ int bus_num;
+};
+
+#define PCI_MCFG_DOMAIN_ANY  -1
+#define PCI_MCFG_BUS_ANY -1
+
+/* Designate a routine to fix up buggy MCFG */
+#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom,
bus) \
+ static const struct
pci_cfg_fixup   \
+
__mcfg_fixup_##oem_id##oem_table_id##dom##bus   \



I'm not entirely sure that this is the right fix--I'm pretty blindly
following a GCC documentation suggestion [1]--but removing the
first two
preprocessor concatenation operators "##" solved the following build
error
for me.

include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and
""QCOM"" does not give a valid preprocessing token
__mcfg_fixup_##oem_id##oem_table_id##dom##bus   \



I think the problem is gcc is not happy with quoted string when
processing these tokens
(""QCOM"", the extra "" are added by gcc). So should we not concat
string tokens and
use the fixup definition in v1 of this RFC:
/* Designate a routine to fix up buggy MCFG */
#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, rev, dom,
bus) \
  static const struct pci_cfg_fixup
__mcfg_fixup_##system##dom##bus\
   __used
__attribute__((__section__(".acpi_fixup_mcfg"), \
  aligned((sizeof(void *)
=   \
  { ops, oem_id, rev, dom, bus };



V1 fixup exist the redefinition error when compiling mutiple
DECLARE_ACPI_MCFG_FIXUP
with the same PCI_MCFG_DOMAIN_ANY and PCI_MCFG_BUS_ANY.

#define EFI_ACPI_HISI_OEM_ID "HISI"
#define EFI_ACPI_HISI_D02_OEM_TABLE_ID "HISI-D02"
#define EFI_ACPI_HISI_D03_OEM_TABLE_ID "HISI-D03"

DECLARE_ACPI_MCFG_FIXUP(_pcie_ecam_ops, EFI_ACPI_HISI_OEM_ID,
EFI_ACPI_HISI_D02_OEM_TABLE_ID, PCI_MCFG_DOMAIN_ANY,
PCI_MCFG_BUS_ANY);

DECLARE_ACPI_MCFG_FIXUP(_pcie_ecam_ops, EFI_ACPI_HISI_OEM_ID,

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Tomasz Nowicki]

On 14.06.2016 11:45, Dongdong Liu wrote:

Hi Duc

在 2016/6/14 17:00, Duc Dang 写道:

On Mon, Jun 13, 2016 at 10:51 PM, Dongdong Liu
 wrote:

Hi Duc

在 2016/6/14 4:57, Duc Dang 写道:


On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
 wrote:


Hi Dongdong,

On 06/13/2016 09:02 AM, Dongdong Liu wrote:


diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
index d3c3e85..49612b3 100644
--- a/drivers/acpi/pci_mcfg.c
+++ b/drivers/acpi/pci_mcfg.c
@@ -22,6 +22,10 @@
   #include 
   #include 
   #include 
+#include 
+
+/* Root pointer to the mapped MCFG table */
+static struct acpi_table_mcfg *mcfg_table;

   /* Structure to hold entries from the MCFG table */
   struct mcfg_entry {
@@ -35,6 +39,38 @@ struct mcfg_entry {
   /* List to save mcfg entries */
   static LIST_HEAD(pci_mcfg_list);

+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 _generic_ecam_ops;
+
+ /*
+  * Match against platform specific quirks and return
corresponding
+  * CAM ops.
+  *
+  * First match against PCI topology  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)) &&
+ (!strncmp(f->oem_table_id,
mcfg_table->header.oem_table_id,
+   ACPI_OEM_TABLE_ID_SIZE)))



This would just be a small convenience, but if the character count
used
here were

min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)

then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be
substrings
and
wouldn't need to be padded out to the full length.


+ return f->ops;
+ }
+ /* No quirks, use ECAM */
+ return _generic_ecam_ops;
+}




diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index 7d63a66..088a1da 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -25,6 +25,7 @@ static inline acpi_status
pci_acpi_remove_pm_notifier(struct acpi_device *dev)
   extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle
handle);

   extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource
*bus_res);
+extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root
*root);

   static inline acpi_handle acpi_find_root_bridge_handle(struct
pci_dev
*pdev)
   {
@@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
int (*prepare_resources)(struct acpi_pci_root_info *info);
   };

+struct pci_cfg_fixup {
+ struct pci_ecam_ops *ops;
+ char *oem_id;
+ char *oem_table_id;
+ int domain;
+ int bus_num;
+};
+
+#define PCI_MCFG_DOMAIN_ANY  -1
+#define PCI_MCFG_BUS_ANY -1
+
+/* Designate a routine to fix up buggy MCFG */
+#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom,
bus) \
+ static const struct
pci_cfg_fixup   \
+
__mcfg_fixup_##oem_id##oem_table_id##dom##bus   \



I'm not entirely sure that this is the right fix--I'm pretty blindly
following a GCC documentation suggestion [1]--but removing the
first two
preprocessor concatenation operators "##" solved the following build
error
for me.

include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and
""QCOM"" does not give a valid preprocessing token
__mcfg_fixup_##oem_id##oem_table_id##dom##bus   \



I think the problem is gcc is not happy with quoted string when
processing these tokens
(""QCOM"", the extra "" are added by gcc). So should we not concat
string tokens and
use the fixup definition in v1 of this RFC:
/* Designate a routine to fix up buggy MCFG */
#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, rev, dom,
bus) \
  static const struct pci_cfg_fixup
__mcfg_fixup_##system##dom##bus\
   __used
__attribute__((__section__(".acpi_fixup_mcfg"), \
  aligned((sizeof(void *)
=   \
  { ops, oem_id, rev, dom, bus };



V1 fixup exist the redefinition error when compiling mutiple
DECLARE_ACPI_MCFG_FIXUP
with the same PCI_MCFG_DOMAIN_ANY and PCI_MCFG_BUS_ANY.

#define EFI_ACPI_HISI_OEM_ID "HISI"
#define EFI_ACPI_HISI_D02_OEM_TABLE_ID "HISI-D02"
#define EFI_ACPI_HISI_D03_OEM_TABLE_ID "HISI-D03"

DECLARE_ACPI_MCFG_FIXUP(_pcie_ecam_ops, EFI_ACPI_HISI_OEM_ID,
EFI_ACPI_HISI_D02_OEM_TABLE_ID, PCI_MCFG_DOMAIN_ANY,
PCI_MCFG_BUS_ANY);

DECLARE_ACPI_MCFG_FIXUP(_pcie_ecam_ops, EFI_ACPI_HISI_OEM_ID,
EFI_ACPI_HISI_D03_OEM_TABLE_ID, PCI_MCFG_DOMAIN_ANY,
PCI_MCFG_BUS_ANY);

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Dongdong Liu]

Hi Duc

在 2016/6/14 17:00, Duc Dang 写道:

On Mon, Jun 13, 2016 at 10:51 PM, Dongdong Liu  wrote:

Hi Duc

在 2016/6/14 4:57, Duc Dang 写道:


On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
 wrote:


Hi Dongdong,

On 06/13/2016 09:02 AM, Dongdong Liu wrote:


diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
index d3c3e85..49612b3 100644
--- a/drivers/acpi/pci_mcfg.c
+++ b/drivers/acpi/pci_mcfg.c
@@ -22,6 +22,10 @@
   #include 
   #include 
   #include 
+#include 
+
+/* Root pointer to the mapped MCFG table */
+static struct acpi_table_mcfg *mcfg_table;

   /* Structure to hold entries from the MCFG table */
   struct mcfg_entry {
@@ -35,6 +39,38 @@ struct mcfg_entry {
   /* List to save mcfg entries */
   static LIST_HEAD(pci_mcfg_list);

+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 _generic_ecam_ops;
+
+ /*
+  * Match against platform specific quirks and return corresponding
+  * CAM ops.
+  *
+  * First match against PCI topology  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)) &&
+ (!strncmp(f->oem_table_id,
mcfg_table->header.oem_table_id,
+   ACPI_OEM_TABLE_ID_SIZE)))



This would just be a small convenience, but if the character count used
here were

min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)

then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be substrings
and
wouldn't need to be padded out to the full length.


+ return f->ops;
+ }
+ /* No quirks, use ECAM */
+ return _generic_ecam_ops;
+}




diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index 7d63a66..088a1da 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -25,6 +25,7 @@ static inline acpi_status
pci_acpi_remove_pm_notifier(struct acpi_device *dev)
   extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle);

   extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource
*bus_res);
+extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root
*root);

   static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev
*pdev)
   {
@@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
int (*prepare_resources)(struct acpi_pci_root_info *info);
   };

+struct pci_cfg_fixup {
+ struct pci_ecam_ops *ops;
+ char *oem_id;
+ char *oem_table_id;
+ int domain;
+ int bus_num;
+};
+
+#define PCI_MCFG_DOMAIN_ANY  -1
+#define PCI_MCFG_BUS_ANY -1
+
+/* Designate a routine to fix up buggy MCFG */
+#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom, bus) \
+ static const struct pci_cfg_fixup   \
+ __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \



I'm not entirely sure that this is the right fix--I'm pretty blindly
following a GCC documentation suggestion [1]--but removing the first two
preprocessor concatenation operators "##" solved the following build
error
for me.

include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and
""QCOM"" does not give a valid preprocessing token
__mcfg_fixup_##oem_id##oem_table_id##dom##bus   \



I think the problem is gcc is not happy with quoted string when
processing these tokens
(""QCOM"", the extra "" are added by gcc). So should we not concat
string tokens and
use the fixup definition in v1 of this RFC:
/* Designate a routine to fix up buggy MCFG */
#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, rev, dom, bus) \
  static const struct pci_cfg_fixup
__mcfg_fixup_##system##dom##bus\
   __used __attribute__((__section__(".acpi_fixup_mcfg"), \
  aligned((sizeof(void *) =   \
  { ops, oem_id, rev, dom, bus };



V1 fixup exist the redefinition error when compiling mutiple
DECLARE_ACPI_MCFG_FIXUP
with the same PCI_MCFG_DOMAIN_ANY and PCI_MCFG_BUS_ANY.

#define EFI_ACPI_HISI_OEM_ID "HISI"
#define EFI_ACPI_HISI_D02_OEM_TABLE_ID "HISI-D02"
#define EFI_ACPI_HISI_D03_OEM_TABLE_ID "HISI-D03"

DECLARE_ACPI_MCFG_FIXUP(_pcie_ecam_ops, EFI_ACPI_HISI_OEM_ID,
EFI_ACPI_HISI_D02_OEM_TABLE_ID, PCI_MCFG_DOMAIN_ANY,
PCI_MCFG_BUS_ANY);

DECLARE_ACPI_MCFG_FIXUP(_pcie_ecam_ops, EFI_ACPI_HISI_OEM_ID,
EFI_ACPI_HISI_D03_OEM_TABLE_ID, PCI_MCFG_DOMAIN_ANY,

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Dongdong Liu]

Hi Duc

在 2016/6/14 17:00, Duc Dang 写道:

On Mon, Jun 13, 2016 at 10:51 PM, Dongdong Liu  wrote:

Hi Duc

在 2016/6/14 4:57, Duc Dang 写道:


On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
 wrote:


Hi Dongdong,

On 06/13/2016 09:02 AM, Dongdong Liu wrote:


diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
index d3c3e85..49612b3 100644
--- a/drivers/acpi/pci_mcfg.c
+++ b/drivers/acpi/pci_mcfg.c
@@ -22,6 +22,10 @@
   #include 
   #include 
   #include 
+#include 
+
+/* Root pointer to the mapped MCFG table */
+static struct acpi_table_mcfg *mcfg_table;

   /* Structure to hold entries from the MCFG table */
   struct mcfg_entry {
@@ -35,6 +39,38 @@ struct mcfg_entry {
   /* List to save mcfg entries */
   static LIST_HEAD(pci_mcfg_list);

+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 _generic_ecam_ops;
+
+ /*
+  * Match against platform specific quirks and return corresponding
+  * CAM ops.
+  *
+  * First match against PCI topology  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)) &&
+ (!strncmp(f->oem_table_id,
mcfg_table->header.oem_table_id,
+   ACPI_OEM_TABLE_ID_SIZE)))



This would just be a small convenience, but if the character count used
here were

min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)

then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be substrings
and
wouldn't need to be padded out to the full length.


+ return f->ops;
+ }
+ /* No quirks, use ECAM */
+ return _generic_ecam_ops;
+}




diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index 7d63a66..088a1da 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -25,6 +25,7 @@ static inline acpi_status
pci_acpi_remove_pm_notifier(struct acpi_device *dev)
   extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle);

   extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource
*bus_res);
+extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root
*root);

   static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev
*pdev)
   {
@@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
int (*prepare_resources)(struct acpi_pci_root_info *info);
   };

+struct pci_cfg_fixup {
+ struct pci_ecam_ops *ops;
+ char *oem_id;
+ char *oem_table_id;
+ int domain;
+ int bus_num;
+};
+
+#define PCI_MCFG_DOMAIN_ANY  -1
+#define PCI_MCFG_BUS_ANY -1
+
+/* Designate a routine to fix up buggy MCFG */
+#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom, bus) \
+ static const struct pci_cfg_fixup   \
+ __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \



I'm not entirely sure that this is the right fix--I'm pretty blindly
following a GCC documentation suggestion [1]--but removing the first two
preprocessor concatenation operators "##" solved the following build
error
for me.

include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and
""QCOM"" does not give a valid preprocessing token
__mcfg_fixup_##oem_id##oem_table_id##dom##bus   \



I think the problem is gcc is not happy with quoted string when
processing these tokens
(""QCOM"", the extra "" are added by gcc). So should we not concat
string tokens and
use the fixup definition in v1 of this RFC:
/* Designate a routine to fix up buggy MCFG */
#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, rev, dom, bus) \
  static const struct pci_cfg_fixup
__mcfg_fixup_##system##dom##bus\
   __used __attribute__((__section__(".acpi_fixup_mcfg"), \
  aligned((sizeof(void *) =   \
  { ops, oem_id, rev, dom, bus };



V1 fixup exist the redefinition error when compiling mutiple
DECLARE_ACPI_MCFG_FIXUP
with the same PCI_MCFG_DOMAIN_ANY and PCI_MCFG_BUS_ANY.

#define EFI_ACPI_HISI_OEM_ID "HISI"
#define EFI_ACPI_HISI_D02_OEM_TABLE_ID "HISI-D02"
#define EFI_ACPI_HISI_D03_OEM_TABLE_ID "HISI-D03"

DECLARE_ACPI_MCFG_FIXUP(_pcie_ecam_ops, EFI_ACPI_HISI_OEM_ID,
EFI_ACPI_HISI_D02_OEM_TABLE_ID, PCI_MCFG_DOMAIN_ANY,
PCI_MCFG_BUS_ANY);

DECLARE_ACPI_MCFG_FIXUP(_pcie_ecam_ops, EFI_ACPI_HISI_OEM_ID,
EFI_ACPI_HISI_D03_OEM_TABLE_ID, PCI_MCFG_DOMAIN_ANY,
PCI_MCFG_BUS_ANY);

In file included from 

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Duc Dang]

On Mon, Jun 13, 2016 at 10:51 PM, Dongdong Liu  wrote:
> Hi Duc
>
> 在 2016/6/14 4:57, Duc Dang 写道:
>>
>> On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
>>  wrote:
>>>
>>> Hi Dongdong,
>>>
>>> On 06/13/2016 09:02 AM, Dongdong Liu wrote:

 diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
 index d3c3e85..49612b3 100644
 --- a/drivers/acpi/pci_mcfg.c
 +++ b/drivers/acpi/pci_mcfg.c
 @@ -22,6 +22,10 @@
   #include 
   #include 
   #include 
 +#include 
 +
 +/* Root pointer to the mapped MCFG table */
 +static struct acpi_table_mcfg *mcfg_table;

   /* Structure to hold entries from the MCFG table */
   struct mcfg_entry {
 @@ -35,6 +39,38 @@ struct mcfg_entry {
   /* List to save mcfg entries */
   static LIST_HEAD(pci_mcfg_list);

 +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 _generic_ecam_ops;
 +
 + /*
 +  * Match against platform specific quirks and return corresponding
 +  * CAM ops.
 +  *
 +  * First match against PCI topology  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)) &&
 + (!strncmp(f->oem_table_id,
 mcfg_table->header.oem_table_id,
 +   ACPI_OEM_TABLE_ID_SIZE)))
>>>
>>>
>>> This would just be a small convenience, but if the character count used
>>> here were
>>>
>>> min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)
>>>
>>> then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be substrings
>>> and
>>> wouldn't need to be padded out to the full length.
>>>
 + return f->ops;
 + }
 + /* No quirks, use ECAM */
 + return _generic_ecam_ops;
 +}
>>>
>>>
 diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
 index 7d63a66..088a1da 100644
 --- a/include/linux/pci-acpi.h
 +++ b/include/linux/pci-acpi.h
 @@ -25,6 +25,7 @@ static inline acpi_status
 pci_acpi_remove_pm_notifier(struct acpi_device *dev)
   extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle);

   extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource
 *bus_res);
 +extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root
 *root);

   static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev
 *pdev)
   {
 @@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
int (*prepare_resources)(struct acpi_pci_root_info *info);
   };

 +struct pci_cfg_fixup {
 + struct pci_ecam_ops *ops;
 + char *oem_id;
 + char *oem_table_id;
 + int domain;
 + int bus_num;
 +};
 +
 +#define PCI_MCFG_DOMAIN_ANY  -1
 +#define PCI_MCFG_BUS_ANY -1
 +
 +/* Designate a routine to fix up buggy MCFG */
 +#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom, bus) \
 + static const struct pci_cfg_fixup   \
 + __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
>>>
>>>
>>> I'm not entirely sure that this is the right fix--I'm pretty blindly
>>> following a GCC documentation suggestion [1]--but removing the first two
>>> preprocessor concatenation operators "##" solved the following build
>>> error
>>> for me.
>>>
>>> include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and
>>> ""QCOM"" does not give a valid preprocessing token
>>>__mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
>>
>>
>> I think the problem is gcc is not happy with quoted string when
>> processing these tokens
>> (""QCOM"", the extra "" are added by gcc). So should we not concat
>> string tokens and
>> use the fixup definition in v1 of this RFC:
>> /* Designate a routine to fix up buggy MCFG */
>> #define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, rev, dom, bus) \
>>  static const struct pci_cfg_fixup
>> __mcfg_fixup_##system##dom##bus\
>>   __used __attribute__((__section__(".acpi_fixup_mcfg"), \
>>  aligned((sizeof(void *) =   \
>>  { ops, oem_id, rev, dom, bus };
>
>
> V1 fixup exist the 

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Duc Dang]

On Mon, Jun 13, 2016 at 10:51 PM, Dongdong Liu  wrote:
> Hi Duc
>
> 在 2016/6/14 4:57, Duc Dang 写道:
>>
>> On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
>>  wrote:
>>>
>>> Hi Dongdong,
>>>
>>> On 06/13/2016 09:02 AM, Dongdong Liu wrote:

 diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
 index d3c3e85..49612b3 100644
 --- a/drivers/acpi/pci_mcfg.c
 +++ b/drivers/acpi/pci_mcfg.c
 @@ -22,6 +22,10 @@
   #include 
   #include 
   #include 
 +#include 
 +
 +/* Root pointer to the mapped MCFG table */
 +static struct acpi_table_mcfg *mcfg_table;

   /* Structure to hold entries from the MCFG table */
   struct mcfg_entry {
 @@ -35,6 +39,38 @@ struct mcfg_entry {
   /* List to save mcfg entries */
   static LIST_HEAD(pci_mcfg_list);

 +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 _generic_ecam_ops;
 +
 + /*
 +  * Match against platform specific quirks and return corresponding
 +  * CAM ops.
 +  *
 +  * First match against PCI topology  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)) &&
 + (!strncmp(f->oem_table_id,
 mcfg_table->header.oem_table_id,
 +   ACPI_OEM_TABLE_ID_SIZE)))
>>>
>>>
>>> This would just be a small convenience, but if the character count used
>>> here were
>>>
>>> min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)
>>>
>>> then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be substrings
>>> and
>>> wouldn't need to be padded out to the full length.
>>>
 + return f->ops;
 + }
 + /* No quirks, use ECAM */
 + return _generic_ecam_ops;
 +}
>>>
>>>
 diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
 index 7d63a66..088a1da 100644
 --- a/include/linux/pci-acpi.h
 +++ b/include/linux/pci-acpi.h
 @@ -25,6 +25,7 @@ static inline acpi_status
 pci_acpi_remove_pm_notifier(struct acpi_device *dev)
   extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle);

   extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource
 *bus_res);
 +extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root
 *root);

   static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev
 *pdev)
   {
 @@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
int (*prepare_resources)(struct acpi_pci_root_info *info);
   };

 +struct pci_cfg_fixup {
 + struct pci_ecam_ops *ops;
 + char *oem_id;
 + char *oem_table_id;
 + int domain;
 + int bus_num;
 +};
 +
 +#define PCI_MCFG_DOMAIN_ANY  -1
 +#define PCI_MCFG_BUS_ANY -1
 +
 +/* Designate a routine to fix up buggy MCFG */
 +#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom, bus) \
 + static const struct pci_cfg_fixup   \
 + __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
>>>
>>>
>>> I'm not entirely sure that this is the right fix--I'm pretty blindly
>>> following a GCC documentation suggestion [1]--but removing the first two
>>> preprocessor concatenation operators "##" solved the following build
>>> error
>>> for me.
>>>
>>> include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and
>>> ""QCOM"" does not give a valid preprocessing token
>>>__mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
>>
>>
>> I think the problem is gcc is not happy with quoted string when
>> processing these tokens
>> (""QCOM"", the extra "" are added by gcc). So should we not concat
>> string tokens and
>> use the fixup definition in v1 of this RFC:
>> /* Designate a routine to fix up buggy MCFG */
>> #define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, rev, dom, bus) \
>>  static const struct pci_cfg_fixup
>> __mcfg_fixup_##system##dom##bus\
>>   __used __attribute__((__section__(".acpi_fixup_mcfg"), \
>>  aligned((sizeof(void *) =   \
>>  { ops, oem_id, rev, dom, bus };
>
>
> V1 fixup exist the redefinition error when compiling mutiple
> 

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Dongdong Liu]

Hi Duc

在 2016/6/14 4:57, Duc Dang 写道:

On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
 wrote:

Hi Dongdong,

On 06/13/2016 09:02 AM, Dongdong Liu wrote:

diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
index d3c3e85..49612b3 100644
--- a/drivers/acpi/pci_mcfg.c
+++ b/drivers/acpi/pci_mcfg.c
@@ -22,6 +22,10 @@
  #include 
  #include 
  #include 
+#include 
+
+/* Root pointer to the mapped MCFG table */
+static struct acpi_table_mcfg *mcfg_table;

  /* Structure to hold entries from the MCFG table */
  struct mcfg_entry {
@@ -35,6 +39,38 @@ struct mcfg_entry {
  /* List to save mcfg entries */
  static LIST_HEAD(pci_mcfg_list);

+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 _generic_ecam_ops;
+
+ /*
+  * Match against platform specific quirks and return corresponding
+  * CAM ops.
+  *
+  * First match against PCI topology  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)) &&
+ (!strncmp(f->oem_table_id, mcfg_table->header.oem_table_id,
+   ACPI_OEM_TABLE_ID_SIZE)))


This would just be a small convenience, but if the character count used here 
were

min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)

then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be substrings and
wouldn't need to be padded out to the full length.


+ return f->ops;
+ }
+ /* No quirks, use ECAM */
+ return _generic_ecam_ops;
+}



diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index 7d63a66..088a1da 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -25,6 +25,7 @@ static inline acpi_status pci_acpi_remove_pm_notifier(struct 
acpi_device *dev)
  extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle);

  extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource *bus_res);
+extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root *root);

  static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
  {
@@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
   int (*prepare_resources)(struct acpi_pci_root_info *info);
  };

+struct pci_cfg_fixup {
+ struct pci_ecam_ops *ops;
+ char *oem_id;
+ char *oem_table_id;
+ int domain;
+ int bus_num;
+};
+
+#define PCI_MCFG_DOMAIN_ANY  -1
+#define PCI_MCFG_BUS_ANY -1
+
+/* Designate a routine to fix up buggy MCFG */
+#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom, bus) \
+ static const struct pci_cfg_fixup   \
+ __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \


I'm not entirely sure that this is the right fix--I'm pretty blindly
following a GCC documentation suggestion [1]--but removing the first two
preprocessor concatenation operators "##" solved the following build error
for me.

include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and ""QCOM"" does 
not give a valid preprocessing token
   __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \


I think the problem is gcc is not happy with quoted string when
processing these tokens
(""QCOM"", the extra "" are added by gcc). So should we not concat
string tokens and
use the fixup definition in v1 of this RFC:
/* Designate a routine to fix up buggy MCFG */
#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, rev, dom, bus) \
 static const struct pci_cfg_fixup __mcfg_fixup_##system##dom##bus\
  __used __attribute__((__section__(".acpi_fixup_mcfg"), \
 aligned((sizeof(void *) =   \
 { ops, oem_id, rev, dom, bus };


V1 fixup exist the redefinition error when compiling mutiple 
DECLARE_ACPI_MCFG_FIXUP
with the same PCI_MCFG_DOMAIN_ANY and PCI_MCFG_BUS_ANY.

#define EFI_ACPI_HISI_OEM_ID "HISI"
#define EFI_ACPI_HISI_D02_OEM_TABLE_ID "HISI-D02"
#define EFI_ACPI_HISI_D03_OEM_TABLE_ID "HISI-D03"

DECLARE_ACPI_MCFG_FIXUP(_pcie_ecam_ops, EFI_ACPI_HISI_OEM_ID,
   EFI_ACPI_HISI_D02_OEM_TABLE_ID, PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);

DECLARE_ACPI_MCFG_FIXUP(_pcie_ecam_ops, EFI_ACPI_HISI_OEM_ID,
   EFI_ACPI_HISI_D03_OEM_TABLE_ID, PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);

In file included from drivers/pci/host/pcie-hisi-acpi.c:15:0:
include/linux/pci-acpi.h:98:43: error: redefinition of 

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Dongdong Liu]

Hi Duc

在 2016/6/14 4:57, Duc Dang 写道:

On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
 wrote:

Hi Dongdong,

On 06/13/2016 09:02 AM, Dongdong Liu wrote:

diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
index d3c3e85..49612b3 100644
--- a/drivers/acpi/pci_mcfg.c
+++ b/drivers/acpi/pci_mcfg.c
@@ -22,6 +22,10 @@
  #include 
  #include 
  #include 
+#include 
+
+/* Root pointer to the mapped MCFG table */
+static struct acpi_table_mcfg *mcfg_table;

  /* Structure to hold entries from the MCFG table */
  struct mcfg_entry {
@@ -35,6 +39,38 @@ struct mcfg_entry {
  /* List to save mcfg entries */
  static LIST_HEAD(pci_mcfg_list);

+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 _generic_ecam_ops;
+
+ /*
+  * Match against platform specific quirks and return corresponding
+  * CAM ops.
+  *
+  * First match against PCI topology  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)) &&
+ (!strncmp(f->oem_table_id, mcfg_table->header.oem_table_id,
+   ACPI_OEM_TABLE_ID_SIZE)))


This would just be a small convenience, but if the character count used here 
were

min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)

then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be substrings and
wouldn't need to be padded out to the full length.


+ return f->ops;
+ }
+ /* No quirks, use ECAM */
+ return _generic_ecam_ops;
+}



diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index 7d63a66..088a1da 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -25,6 +25,7 @@ static inline acpi_status pci_acpi_remove_pm_notifier(struct 
acpi_device *dev)
  extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle);

  extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource *bus_res);
+extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root *root);

  static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
  {
@@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
   int (*prepare_resources)(struct acpi_pci_root_info *info);
  };

+struct pci_cfg_fixup {
+ struct pci_ecam_ops *ops;
+ char *oem_id;
+ char *oem_table_id;
+ int domain;
+ int bus_num;
+};
+
+#define PCI_MCFG_DOMAIN_ANY  -1
+#define PCI_MCFG_BUS_ANY -1
+
+/* Designate a routine to fix up buggy MCFG */
+#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom, bus) \
+ static const struct pci_cfg_fixup   \
+ __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \


I'm not entirely sure that this is the right fix--I'm pretty blindly
following a GCC documentation suggestion [1]--but removing the first two
preprocessor concatenation operators "##" solved the following build error
for me.

include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and ""QCOM"" does 
not give a valid preprocessing token
   __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \


I think the problem is gcc is not happy with quoted string when
processing these tokens
(""QCOM"", the extra "" are added by gcc). So should we not concat
string tokens and
use the fixup definition in v1 of this RFC:
/* Designate a routine to fix up buggy MCFG */
#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, rev, dom, bus) \
 static const struct pci_cfg_fixup __mcfg_fixup_##system##dom##bus\
  __used __attribute__((__section__(".acpi_fixup_mcfg"), \
 aligned((sizeof(void *) =   \
 { ops, oem_id, rev, dom, bus };


V1 fixup exist the redefinition error when compiling mutiple 
DECLARE_ACPI_MCFG_FIXUP
with the same PCI_MCFG_DOMAIN_ANY and PCI_MCFG_BUS_ANY.

#define EFI_ACPI_HISI_OEM_ID "HISI"
#define EFI_ACPI_HISI_D02_OEM_TABLE_ID "HISI-D02"
#define EFI_ACPI_HISI_D03_OEM_TABLE_ID "HISI-D03"

DECLARE_ACPI_MCFG_FIXUP(_pcie_ecam_ops, EFI_ACPI_HISI_OEM_ID,
   EFI_ACPI_HISI_D02_OEM_TABLE_ID, PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);

DECLARE_ACPI_MCFG_FIXUP(_pcie_ecam_ops, EFI_ACPI_HISI_OEM_ID,
   EFI_ACPI_HISI_D03_OEM_TABLE_ID, PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);

In file included from drivers/pci/host/pcie-hisi-acpi.c:15:0:
include/linux/pci-acpi.h:98:43: error: redefinition of 

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Duc Dang]

On Mon, Jun 13, 2016 at 8:59 AM, Jeffrey Hugo <jh...@codeaurora.org> wrote:
> On 6/13/2016 9:12 AM, ok...@codeaurora.org wrote:
>>
>> On 2016-06-13 10:29, Gabriele Paoloni wrote:
>>>
>>> Hi Sinan
>>>
>>>> -Original Message-
>>>> From: Sinan Kaya [mailto:ok...@codeaurora.org]
>>>> Sent: 13 June 2016 15:03
>>>> To: Gabriele Paoloni; liudongdong (C); helg...@kernel.org;
>>>> a...@arndb.de; will.dea...@arm.com; catalin.mari...@arm.com;
>>>> raf...@kernel.org; hanjun@linaro.org; lorenzo.pieral...@arm.com;
>>>> jchan...@broadcom.com; t...@semihalf.com
>>>> Cc: robert.rich...@caviumnetworks.com; m...@semihalf.com;
>>>> liviu.du...@arm.com; dda...@caviumnetworks.com; Wangyijing;
>>>> suravee.suthikulpa...@amd.com; msal...@redhat.com; linux-
>>>> p...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
>>>> a...@vger.kernel.org; linux-kernel@vger.kernel.org; linaro-
>>>> a...@lists.linaro.org; j...@redhat.com; andrea.ga...@linaro.org;
>>>> dhd...@apm.com; jeremy.lin...@arm.com; c...@codeaurora.org; Chenxin
>>>> (Charles); Linuxarm
>>>> Subject: Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space
>>>> accessors against platfrom specific ECAM quirks
>>>>
>>>> On 6/13/2016 9:54 AM, Gabriele Paoloni wrote:
>>>> > As you can see here Liudongdong has replaced oem_revision with
>>>> > oem_table_id.
>>>> >
>>>> > Now it seems that there are some platforms that have already shipped
>>>> > using a matching based on the oem_revision (right Jon?)
>>>> >
>>>> > However I guess that if in FW they have defined oem_table_id properly
>>>> > they should be able to use this mechanism without needing to a FW
>>>> update.
>>>> >
>>>> > Can these vendors confirm this?
>>>> >
>>>> > Tomasz do you think this can work for Cavium Thunder?
>>>> >
>>>> > Thanks
>>>> >
>>>> > Gab
>>>>
>>>> Why not have all three of them?
>>>>
>>>> The initial approach was OEM id and revision id.
>>>>
>>>> Jeff Hugo indicated that addition (not removing any other fields) of
>>>> table id
>>>> would make more sense.
>>>
>>>
>>> Mmm from last email of Jeff Hugo on "[RFC PATCH 1/3] pci, acpi: Match
>>> PCI config space accessors against platfrom specific ECAM quirks."
>>>
>>> I quote:
>>>
>>>  "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."
>>>
>>> So it looks to me that he pointed out that using the OEM revision field
>>> is wrong...and this is why I have asked if replacing it with the table
>>> id can work for other vendors
>>>
>>> Thanks
>>>
>>> Gab
>>>
>>
>> I had an internal discussion with jeff and cov before posting on the
>> maillist.
>>
>> I think there is missing info in the email.
>>
>> Usage of oem id + table id + revision is ok.
>>
>> Usage of oem id + revision is not ok as one oem can build multiple chips
>> with the same oem id and revision id but different table id. Otherwise,
>> we can run out of revisions very quickly.
>
>
> Agreed.
>
> I'm sorry for the confusion.  My intent was to point out that revision alone
> appeared insufficient to address all the identified problems, but I believe
> there is still a case for using revision. Table id is useful for
> differentiating between platforms/chips.  Revision is useful for
> differentiation between different versions of a single platform/chip
> assuming the silicon is respun or some other fix is applied.  Both solve
> different scenarios, and I'm not aware of a reason why they could not be
> used together to solve all currently identified cases.

Using OEM ID + Table ID + Revision will work for X-Gene platforms as well.

Regards,
Duc Dang.
>
>>
>>>
>>>>
>>>> --
>>>> Sinan Kaya
>>>> Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center,
>>>> Inc.
>>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
>>>> Linux Foundation Collaborative Project
>>
>>
>> ___
>> linux-arm-kernel mailing list
>> linux-arm-ker...@lists.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

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Duc Dang]

On Mon, Jun 13, 2016 at 8:59 AM, Jeffrey Hugo  wrote:
> On 6/13/2016 9:12 AM, ok...@codeaurora.org wrote:
>>
>> On 2016-06-13 10:29, Gabriele Paoloni wrote:
>>>
>>> Hi Sinan
>>>
>>>> -Original Message-
>>>> From: Sinan Kaya [mailto:ok...@codeaurora.org]
>>>> Sent: 13 June 2016 15:03
>>>> To: Gabriele Paoloni; liudongdong (C); helg...@kernel.org;
>>>> a...@arndb.de; will.dea...@arm.com; catalin.mari...@arm.com;
>>>> raf...@kernel.org; hanjun@linaro.org; lorenzo.pieral...@arm.com;
>>>> jchan...@broadcom.com; t...@semihalf.com
>>>> Cc: robert.rich...@caviumnetworks.com; m...@semihalf.com;
>>>> liviu.du...@arm.com; dda...@caviumnetworks.com; Wangyijing;
>>>> suravee.suthikulpa...@amd.com; msal...@redhat.com; linux-
>>>> p...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
>>>> a...@vger.kernel.org; linux-kernel@vger.kernel.org; linaro-
>>>> a...@lists.linaro.org; j...@redhat.com; andrea.ga...@linaro.org;
>>>> dhd...@apm.com; jeremy.lin...@arm.com; c...@codeaurora.org; Chenxin
>>>> (Charles); Linuxarm
>>>> Subject: Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space
>>>> accessors against platfrom specific ECAM quirks
>>>>
>>>> On 6/13/2016 9:54 AM, Gabriele Paoloni wrote:
>>>> > As you can see here Liudongdong has replaced oem_revision with
>>>> > oem_table_id.
>>>> >
>>>> > Now it seems that there are some platforms that have already shipped
>>>> > using a matching based on the oem_revision (right Jon?)
>>>> >
>>>> > However I guess that if in FW they have defined oem_table_id properly
>>>> > they should be able to use this mechanism without needing to a FW
>>>> update.
>>>> >
>>>> > Can these vendors confirm this?
>>>> >
>>>> > Tomasz do you think this can work for Cavium Thunder?
>>>> >
>>>> > Thanks
>>>> >
>>>> > Gab
>>>>
>>>> Why not have all three of them?
>>>>
>>>> The initial approach was OEM id and revision id.
>>>>
>>>> Jeff Hugo indicated that addition (not removing any other fields) of
>>>> table id
>>>> would make more sense.
>>>
>>>
>>> Mmm from last email of Jeff Hugo on "[RFC PATCH 1/3] pci, acpi: Match
>>> PCI config space accessors against platfrom specific ECAM quirks."
>>>
>>> I quote:
>>>
>>>  "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."
>>>
>>> So it looks to me that he pointed out that using the OEM revision field
>>> is wrong...and this is why I have asked if replacing it with the table
>>> id can work for other vendors
>>>
>>> Thanks
>>>
>>> Gab
>>>
>>
>> I had an internal discussion with jeff and cov before posting on the
>> maillist.
>>
>> I think there is missing info in the email.
>>
>> Usage of oem id + table id + revision is ok.
>>
>> Usage of oem id + revision is not ok as one oem can build multiple chips
>> with the same oem id and revision id but different table id. Otherwise,
>> we can run out of revisions very quickly.
>
>
> Agreed.
>
> I'm sorry for the confusion.  My intent was to point out that revision alone
> appeared insufficient to address all the identified problems, but I believe
> there is still a case for using revision. Table id is useful for
> differentiating between platforms/chips.  Revision is useful for
> differentiation between different versions of a single platform/chip
> assuming the silicon is respun or some other fix is applied.  Both solve
> different scenarios, and I'm not aware of a reason why they could not be
> used together to solve all currently identified cases.

Using OEM ID + Table ID + Revision will work for X-Gene platforms as well.

Regards,
Duc Dang.
>
>>
>>>
>>>>
>>>> --
>>>> Sinan Kaya
>>>> Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center,
>>>> Inc.
>>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
>>>> Linux Foundation Collaborative Project
>>
>>
>> ___
>> linux-arm-kernel mailing list
>> linux-arm-ker...@lists.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

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Duc Dang]

On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
 wrote:
> Hi Dongdong,
>
> On 06/13/2016 09:02 AM, Dongdong Liu wrote:
>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>> index d3c3e85..49612b3 100644
>> --- a/drivers/acpi/pci_mcfg.c
>> +++ b/drivers/acpi/pci_mcfg.c
>> @@ -22,6 +22,10 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>> +
>> +/* Root pointer to the mapped MCFG table */
>> +static struct acpi_table_mcfg *mcfg_table;
>>
>>  /* Structure to hold entries from the MCFG table */
>>  struct mcfg_entry {
>> @@ -35,6 +39,38 @@ struct mcfg_entry {
>>  /* List to save mcfg entries */
>>  static LIST_HEAD(pci_mcfg_list);
>>
>> +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 _generic_ecam_ops;
>> +
>> + /*
>> +  * Match against platform specific quirks and return corresponding
>> +  * CAM ops.
>> +  *
>> +  * First match against PCI topology  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)) &&
>> + (!strncmp(f->oem_table_id, mcfg_table->header.oem_table_id,
>> +   ACPI_OEM_TABLE_ID_SIZE)))
>
> This would just be a small convenience, but if the character count used here 
> were
>
> min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)
>
> then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be substrings and
> wouldn't need to be padded out to the full length.
>
>> + return f->ops;
>> + }
>> + /* No quirks, use ECAM */
>> + return _generic_ecam_ops;
>> +}
>
>> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
>> index 7d63a66..088a1da 100644
>> --- a/include/linux/pci-acpi.h
>> +++ b/include/linux/pci-acpi.h
>> @@ -25,6 +25,7 @@ static inline acpi_status 
>> pci_acpi_remove_pm_notifier(struct acpi_device *dev)
>>  extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle);
>>
>>  extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource *bus_res);
>> +extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root *root);
>>
>>  static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
>>  {
>> @@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
>>   int (*prepare_resources)(struct acpi_pci_root_info *info);
>>  };
>>
>> +struct pci_cfg_fixup {
>> + struct pci_ecam_ops *ops;
>> + char *oem_id;
>> + char *oem_table_id;
>> + int domain;
>> + int bus_num;
>> +};
>> +
>> +#define PCI_MCFG_DOMAIN_ANY  -1
>> +#define PCI_MCFG_BUS_ANY -1
>> +
>> +/* Designate a routine to fix up buggy MCFG */
>> +#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom, bus) \
>> + static const struct pci_cfg_fixup   \
>> + __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
>
> I'm not entirely sure that this is the right fix--I'm pretty blindly
> following a GCC documentation suggestion [1]--but removing the first two
> preprocessor concatenation operators "##" solved the following build error
> for me.
>
> include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and ""QCOM"" 
> does not give a valid preprocessing token
>   __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \

I think the problem is gcc is not happy with quoted string when
processing these tokens
(""QCOM"", the extra "" are added by gcc). So should we not concat
string tokens and
use the fixup definition in v1 of this RFC:
/* Designate a routine to fix up buggy MCFG */
#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, rev, dom, bus) \
static const struct pci_cfg_fixup __mcfg_fixup_##system##dom##bus\
 __used __attribute__((__section__(".acpi_fixup_mcfg"), \
aligned((sizeof(void *) =   \
{ ops, oem_id, rev, dom, bus };

Regards,
Duc Dang.


>   ^
> arch/arm64/kernel/pci.c:225:1: note: in expansion of macro 
> ‘DECLARE_ACPI_MCFG_FIXUP’
>  DECLARE_ACPI_MCFG_FIXUP(_32b_ecam_ops, "QCOM", "QDF2432", 
> PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);
>  ^
> arch/arm64/kernel/pci.c:225:44: error: pasting ""QCOM"" and ""QDF2432"" does 
> not give a valid preprocessing token
>  DECLARE_ACPI_MCFG_FIXUP(_32b_ecam_ops, "QCOM", "QDF2432", 
> PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);
>

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Duc Dang]

On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
 wrote:
> Hi Dongdong,
>
> On 06/13/2016 09:02 AM, Dongdong Liu wrote:
>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>> index d3c3e85..49612b3 100644
>> --- a/drivers/acpi/pci_mcfg.c
>> +++ b/drivers/acpi/pci_mcfg.c
>> @@ -22,6 +22,10 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>> +
>> +/* Root pointer to the mapped MCFG table */
>> +static struct acpi_table_mcfg *mcfg_table;
>>
>>  /* Structure to hold entries from the MCFG table */
>>  struct mcfg_entry {
>> @@ -35,6 +39,38 @@ struct mcfg_entry {
>>  /* List to save mcfg entries */
>>  static LIST_HEAD(pci_mcfg_list);
>>
>> +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 _generic_ecam_ops;
>> +
>> + /*
>> +  * Match against platform specific quirks and return corresponding
>> +  * CAM ops.
>> +  *
>> +  * First match against PCI topology  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)) &&
>> + (!strncmp(f->oem_table_id, mcfg_table->header.oem_table_id,
>> +   ACPI_OEM_TABLE_ID_SIZE)))
>
> This would just be a small convenience, but if the character count used here 
> were
>
> min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)
>
> then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be substrings and
> wouldn't need to be padded out to the full length.
>
>> + return f->ops;
>> + }
>> + /* No quirks, use ECAM */
>> + return _generic_ecam_ops;
>> +}
>
>> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
>> index 7d63a66..088a1da 100644
>> --- a/include/linux/pci-acpi.h
>> +++ b/include/linux/pci-acpi.h
>> @@ -25,6 +25,7 @@ static inline acpi_status 
>> pci_acpi_remove_pm_notifier(struct acpi_device *dev)
>>  extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle);
>>
>>  extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource *bus_res);
>> +extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root *root);
>>
>>  static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
>>  {
>> @@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
>>   int (*prepare_resources)(struct acpi_pci_root_info *info);
>>  };
>>
>> +struct pci_cfg_fixup {
>> + struct pci_ecam_ops *ops;
>> + char *oem_id;
>> + char *oem_table_id;
>> + int domain;
>> + int bus_num;
>> +};
>> +
>> +#define PCI_MCFG_DOMAIN_ANY  -1
>> +#define PCI_MCFG_BUS_ANY -1
>> +
>> +/* Designate a routine to fix up buggy MCFG */
>> +#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom, bus) \
>> + static const struct pci_cfg_fixup   \
>> + __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
>
> I'm not entirely sure that this is the right fix--I'm pretty blindly
> following a GCC documentation suggestion [1]--but removing the first two
> preprocessor concatenation operators "##" solved the following build error
> for me.
>
> include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and ""QCOM"" 
> does not give a valid preprocessing token
>   __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \

I think the problem is gcc is not happy with quoted string when
processing these tokens
(""QCOM"", the extra "" are added by gcc). So should we not concat
string tokens and
use the fixup definition in v1 of this RFC:
/* Designate a routine to fix up buggy MCFG */
#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, rev, dom, bus) \
static const struct pci_cfg_fixup __mcfg_fixup_##system##dom##bus\
 __used __attribute__((__section__(".acpi_fixup_mcfg"), \
aligned((sizeof(void *) =   \
{ ops, oem_id, rev, dom, bus };

Regards,
Duc Dang.


>   ^
> arch/arm64/kernel/pci.c:225:1: note: in expansion of macro 
> ‘DECLARE_ACPI_MCFG_FIXUP’
>  DECLARE_ACPI_MCFG_FIXUP(_32b_ecam_ops, "QCOM", "QDF2432", 
> PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);
>  ^
> arch/arm64/kernel/pci.c:225:44: error: pasting ""QCOM"" and ""QDF2432"" does 
> not give a valid preprocessing token
>  DECLARE_ACPI_MCFG_FIXUP(_32b_ecam_ops, "QCOM", "QDF2432", 
> PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);
> ^
> 

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Jeffrey Hugo]

On 6/13/2016 9:12 AM, ok...@codeaurora.org wrote:

On 2016-06-13 10:29, Gabriele Paoloni wrote:

Hi Sinan


-Original Message-
From: Sinan Kaya [mailto:ok...@codeaurora.org]
Sent: 13 June 2016 15:03
To: Gabriele Paoloni; liudongdong (C); helg...@kernel.org;
a...@arndb.de; will.dea...@arm.com; catalin.mari...@arm.com;
raf...@kernel.org; hanjun@linaro.org; lorenzo.pieral...@arm.com;
jchan...@broadcom.com; t...@semihalf.com
Cc: robert.rich...@caviumnetworks.com; m...@semihalf.com;
liviu.du...@arm.com; dda...@caviumnetworks.com; Wangyijing;
suravee.suthikulpa...@amd.com; msal...@redhat.com; linux-
p...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
a...@vger.kernel.org; linux-kernel@vger.kernel.org; linaro-
a...@lists.linaro.org; j...@redhat.com; andrea.ga...@linaro.org;
dhd...@apm.com; jeremy.lin...@arm.com; c...@codeaurora.org; Chenxin
(Charles); Linuxarm
Subject: Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space
accessors against platfrom specific ECAM quirks

On 6/13/2016 9:54 AM, Gabriele Paoloni wrote:
> As you can see here Liudongdong has replaced oem_revision with
> oem_table_id.
>
> Now it seems that there are some platforms that have already shipped
> using a matching based on the oem_revision (right Jon?)
>
> However I guess that if in FW they have defined oem_table_id properly
> they should be able to use this mechanism without needing to a FW
update.
>
> Can these vendors confirm this?
>
> Tomasz do you think this can work for Cavium Thunder?
>
> Thanks
>
> Gab

Why not have all three of them?

The initial approach was OEM id and revision id.

Jeff Hugo indicated that addition (not removing any other fields) of
table id
would make more sense.


Mmm from last email of Jeff Hugo on "[RFC PATCH 1/3] pci, acpi: Match
PCI config space accessors against platfrom specific ECAM quirks."

I quote:

 "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."

So it looks to me that he pointed out that using the OEM revision field
is wrong...and this is why I have asked if replacing it with the table
id can work for other vendors

Thanks

Gab



I had an internal discussion with jeff and cov before posting on the
maillist.

I think there is missing info in the email.

Usage of oem id + table id + revision is ok.

Usage of oem id + revision is not ok as one oem can build multiple chips
with the same oem id and revision id but different table id. Otherwise,
we can run out of revisions very quickly.


Agreed.

I'm sorry for the confusion.  My intent was to point out that revision 
alone appeared insufficient to address all the identified problems, but 
I believe there is still a case for using revision. Table id is useful 
for differentiating between platforms/chips.  Revision is useful for 
differentiation between different versions of a single platform/chip 
assuming the silicon is respun or some other fix is applied.  Both solve 
different scenarios, and I'm not aware of a reason why they could not be 
used together to solve all currently identified cases.








--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center,
Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project


___
linux-arm-kernel mailing list
linux-arm-ker...@lists.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

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Jeffrey Hugo]

On 6/13/2016 9:12 AM, ok...@codeaurora.org wrote:

On 2016-06-13 10:29, Gabriele Paoloni wrote:

Hi Sinan


-Original Message-
From: Sinan Kaya [mailto:ok...@codeaurora.org]
Sent: 13 June 2016 15:03
To: Gabriele Paoloni; liudongdong (C); helg...@kernel.org;
a...@arndb.de; will.dea...@arm.com; catalin.mari...@arm.com;
raf...@kernel.org; hanjun@linaro.org; lorenzo.pieral...@arm.com;
jchan...@broadcom.com; t...@semihalf.com
Cc: robert.rich...@caviumnetworks.com; m...@semihalf.com;
liviu.du...@arm.com; dda...@caviumnetworks.com; Wangyijing;
suravee.suthikulpa...@amd.com; msal...@redhat.com; linux-
p...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
a...@vger.kernel.org; linux-kernel@vger.kernel.org; linaro-
a...@lists.linaro.org; j...@redhat.com; andrea.ga...@linaro.org;
dhd...@apm.com; jeremy.lin...@arm.com; c...@codeaurora.org; Chenxin
(Charles); Linuxarm
Subject: Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space
accessors against platfrom specific ECAM quirks

On 6/13/2016 9:54 AM, Gabriele Paoloni wrote:
> As you can see here Liudongdong has replaced oem_revision with
> oem_table_id.
>
> Now it seems that there are some platforms that have already shipped
> using a matching based on the oem_revision (right Jon?)
>
> However I guess that if in FW they have defined oem_table_id properly
> they should be able to use this mechanism without needing to a FW
update.
>
> Can these vendors confirm this?
>
> Tomasz do you think this can work for Cavium Thunder?
>
> Thanks
>
> Gab

Why not have all three of them?

The initial approach was OEM id and revision id.

Jeff Hugo indicated that addition (not removing any other fields) of
table id
would make more sense.


Mmm from last email of Jeff Hugo on "[RFC PATCH 1/3] pci, acpi: Match
PCI config space accessors against platfrom specific ECAM quirks."

I quote:

 "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."

So it looks to me that he pointed out that using the OEM revision field
is wrong...and this is why I have asked if replacing it with the table
id can work for other vendors

Thanks

Gab



I had an internal discussion with jeff and cov before posting on the
maillist.

I think there is missing info in the email.

Usage of oem id + table id + revision is ok.

Usage of oem id + revision is not ok as one oem can build multiple chips
with the same oem id and revision id but different table id. Otherwise,
we can run out of revisions very quickly.


Agreed.

I'm sorry for the confusion.  My intent was to point out that revision 
alone appeared insufficient to address all the identified problems, but 
I believe there is still a case for using revision. Table id is useful 
for differentiating between platforms/chips.  Revision is useful for 
differentiation between different versions of a single platform/chip 
assuming the silicon is respun or some other fix is applied.  Both solve 
different scenarios, and I'm not aware of a reason why they could not be 
used together to solve all currently identified cases.








--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center,
Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project


___
linux-arm-kernel mailing list
linux-arm-ker...@lists.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

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Christopher Covington]

Hi Dongdong,

On 06/13/2016 09:02 AM, Dongdong Liu wrote:
> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> index d3c3e85..49612b3 100644
> --- a/drivers/acpi/pci_mcfg.c
> +++ b/drivers/acpi/pci_mcfg.c
> @@ -22,6 +22,10 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +
> +/* Root pointer to the mapped MCFG table */
> +static struct acpi_table_mcfg *mcfg_table;
>  
>  /* Structure to hold entries from the MCFG table */
>  struct mcfg_entry {
> @@ -35,6 +39,38 @@ struct mcfg_entry {
>  /* List to save mcfg entries */
>  static LIST_HEAD(pci_mcfg_list);
>  
> +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 _generic_ecam_ops;
> +
> + /*
> +  * Match against platform specific quirks and return corresponding
> +  * CAM ops.
> +  *
> +  * First match against PCI topology  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)) &&
> + (!strncmp(f->oem_table_id, mcfg_table->header.oem_table_id,
> +   ACPI_OEM_TABLE_ID_SIZE)))

This would just be a small convenience, but if the character count used here 
were

min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)

then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be substrings and
wouldn't need to be padded out to the full length.

> + return f->ops;
> + }
> + /* No quirks, use ECAM */
> + return _generic_ecam_ops;
> +}

> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index 7d63a66..088a1da 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -25,6 +25,7 @@ static inline acpi_status 
> pci_acpi_remove_pm_notifier(struct acpi_device *dev)
>  extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle);
>  
>  extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource *bus_res);
> +extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root *root);
>  
>  static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
>  {
> @@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
>   int (*prepare_resources)(struct acpi_pci_root_info *info);
>  };
>  
> +struct pci_cfg_fixup {
> + struct pci_ecam_ops *ops;
> + char *oem_id;
> + char *oem_table_id;
> + int domain;
> + int bus_num;
> +};
> +
> +#define PCI_MCFG_DOMAIN_ANY  -1
> +#define PCI_MCFG_BUS_ANY -1
> +
> +/* Designate a routine to fix up buggy MCFG */
> +#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom, bus) \
> + static const struct pci_cfg_fixup   \
> + __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \

I'm not entirely sure that this is the right fix--I'm pretty blindly
following a GCC documentation suggestion [1]--but removing the first two
preprocessor concatenation operators "##" solved the following build error
for me.

include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and ""QCOM"" does 
not give a valid preprocessing token
  __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
  ^
arch/arm64/kernel/pci.c:225:1: note: in expansion of macro 
‘DECLARE_ACPI_MCFG_FIXUP’
 DECLARE_ACPI_MCFG_FIXUP(_32b_ecam_ops, "QCOM", "QDF2432", 
PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);
 ^
arch/arm64/kernel/pci.c:225:44: error: pasting ""QCOM"" and ""QDF2432"" does 
not give a valid preprocessing token
 DECLARE_ACPI_MCFG_FIXUP(_32b_ecam_ops, "QCOM", "QDF2432", 
PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);
^
include/linux/pci-acpi.h:90:17: note: in definition of macro 
‘DECLARE_ACPI_MCFG_FIXUP’
  __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
 ^
arch/arm64/kernel/pci.c:225:52: error: pasting ""QDF2432"" and 
"PCI_MCFG_DOMAIN_ANY" does not give a valid preprocessi
ng token
 DECLARE_ACPI_MCFG_FIXUP(_32b_ecam_ops, "QCOM", "QDF2432", 
PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);
^
include/linux/pci-acpi.h:90:25: note: in definition of macro 
‘DECLARE_ACPI_MCFG_FIXUP’
  __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
 ^
arch/arm64/kernel/pci.c:225:44: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or 
‘__attribute__’ before string constant
 DECLARE_ACPI_MCFG_FIXUP(_32b_ecam_ops, "QCOM", "QDF2432", 
PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);
  

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Christopher Covington]

Hi Dongdong,

On 06/13/2016 09:02 AM, Dongdong Liu wrote:
> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> index d3c3e85..49612b3 100644
> --- a/drivers/acpi/pci_mcfg.c
> +++ b/drivers/acpi/pci_mcfg.c
> @@ -22,6 +22,10 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +
> +/* Root pointer to the mapped MCFG table */
> +static struct acpi_table_mcfg *mcfg_table;
>  
>  /* Structure to hold entries from the MCFG table */
>  struct mcfg_entry {
> @@ -35,6 +39,38 @@ struct mcfg_entry {
>  /* List to save mcfg entries */
>  static LIST_HEAD(pci_mcfg_list);
>  
> +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 _generic_ecam_ops;
> +
> + /*
> +  * Match against platform specific quirks and return corresponding
> +  * CAM ops.
> +  *
> +  * First match against PCI topology  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)) &&
> + (!strncmp(f->oem_table_id, mcfg_table->header.oem_table_id,
> +   ACPI_OEM_TABLE_ID_SIZE)))

This would just be a small convenience, but if the character count used here 
were

min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)

then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be substrings and
wouldn't need to be padded out to the full length.

> + return f->ops;
> + }
> + /* No quirks, use ECAM */
> + return _generic_ecam_ops;
> +}

> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index 7d63a66..088a1da 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -25,6 +25,7 @@ static inline acpi_status 
> pci_acpi_remove_pm_notifier(struct acpi_device *dev)
>  extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle);
>  
>  extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource *bus_res);
> +extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root *root);
>  
>  static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
>  {
> @@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
>   int (*prepare_resources)(struct acpi_pci_root_info *info);
>  };
>  
> +struct pci_cfg_fixup {
> + struct pci_ecam_ops *ops;
> + char *oem_id;
> + char *oem_table_id;
> + int domain;
> + int bus_num;
> +};
> +
> +#define PCI_MCFG_DOMAIN_ANY  -1
> +#define PCI_MCFG_BUS_ANY -1
> +
> +/* Designate a routine to fix up buggy MCFG */
> +#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom, bus) \
> + static const struct pci_cfg_fixup   \
> + __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \

I'm not entirely sure that this is the right fix--I'm pretty blindly
following a GCC documentation suggestion [1]--but removing the first two
preprocessor concatenation operators "##" solved the following build error
for me.

include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and ""QCOM"" does 
not give a valid preprocessing token
  __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
  ^
arch/arm64/kernel/pci.c:225:1: note: in expansion of macro 
‘DECLARE_ACPI_MCFG_FIXUP’
 DECLARE_ACPI_MCFG_FIXUP(_32b_ecam_ops, "QCOM", "QDF2432", 
PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);
 ^
arch/arm64/kernel/pci.c:225:44: error: pasting ""QCOM"" and ""QDF2432"" does 
not give a valid preprocessing token
 DECLARE_ACPI_MCFG_FIXUP(_32b_ecam_ops, "QCOM", "QDF2432", 
PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);
^
include/linux/pci-acpi.h:90:17: note: in definition of macro 
‘DECLARE_ACPI_MCFG_FIXUP’
  __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
 ^
arch/arm64/kernel/pci.c:225:52: error: pasting ""QDF2432"" and 
"PCI_MCFG_DOMAIN_ANY" does not give a valid preprocessi
ng token
 DECLARE_ACPI_MCFG_FIXUP(_32b_ecam_ops, "QCOM", "QDF2432", 
PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);
^
include/linux/pci-acpi.h:90:25: note: in definition of macro 
‘DECLARE_ACPI_MCFG_FIXUP’
  __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \
 ^
arch/arm64/kernel/pci.c:225:44: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or 
‘__attribute__’ before string constant
 DECLARE_ACPI_MCFG_FIXUP(_32b_ecam_ops, "QCOM", "QDF2432", 
PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);
  

RE: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[okaya]

On 2016-06-13 10:29, Gabriele Paoloni wrote:

Hi Sinan


-Original Message-
From: Sinan Kaya [mailto:ok...@codeaurora.org]
Sent: 13 June 2016 15:03
To: Gabriele Paoloni; liudongdong (C); helg...@kernel.org;
a...@arndb.de; will.dea...@arm.com; catalin.mari...@arm.com;
raf...@kernel.org; hanjun@linaro.org; lorenzo.pieral...@arm.com;
jchan...@broadcom.com; t...@semihalf.com
Cc: robert.rich...@caviumnetworks.com; m...@semihalf.com;
liviu.du...@arm.com; dda...@caviumnetworks.com; Wangyijing;
suravee.suthikulpa...@amd.com; msal...@redhat.com; linux-
p...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
a...@vger.kernel.org; linux-kernel@vger.kernel.org; linaro-
a...@lists.linaro.org; j...@redhat.com; andrea.ga...@linaro.org;
dhd...@apm.com; jeremy.lin...@arm.com; c...@codeaurora.org; Chenxin
(Charles); Linuxarm
Subject: Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space
accessors against platfrom specific ECAM quirks

On 6/13/2016 9:54 AM, Gabriele Paoloni wrote:
> As you can see here Liudongdong has replaced oem_revision with
> oem_table_id.
>
> Now it seems that there are some platforms that have already shipped
> using a matching based on the oem_revision (right Jon?)
>
> However I guess that if in FW they have defined oem_table_id properly
> they should be able to use this mechanism without needing to a FW
update.
>
> Can these vendors confirm this?
>
> Tomasz do you think this can work for Cavium Thunder?
>
> Thanks
>
> Gab

Why not have all three of them?

The initial approach was OEM id and revision id.

Jeff Hugo indicated that addition (not removing any other fields) of
table id
would make more sense.


Mmm from last email of Jeff Hugo on "[RFC PATCH 1/3] pci, acpi: Match
PCI config space accessors against platfrom specific ECAM quirks."

I quote:

 "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."

So it looks to me that he pointed out that using the OEM revision field
is wrong...and this is why I have asked if replacing it with the table
id can work for other vendors

Thanks

Gab



I had an internal discussion with jeff and cov before posting on the 
maillist.


I think there is missing info in the email.

Usage of oem id + table id + revision is ok.

Usage of oem id + revision is not ok as one oem can build multiple chips 
with the same oem id and revision id but different table id. Otherwise, 
we can run out of revisions very quickly.






--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center,
Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project

RE: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[okaya]

On 2016-06-13 10:29, Gabriele Paoloni wrote:

Hi Sinan


-Original Message-
From: Sinan Kaya [mailto:ok...@codeaurora.org]
Sent: 13 June 2016 15:03
To: Gabriele Paoloni; liudongdong (C); helg...@kernel.org;
a...@arndb.de; will.dea...@arm.com; catalin.mari...@arm.com;
raf...@kernel.org; hanjun@linaro.org; lorenzo.pieral...@arm.com;
jchan...@broadcom.com; t...@semihalf.com
Cc: robert.rich...@caviumnetworks.com; m...@semihalf.com;
liviu.du...@arm.com; dda...@caviumnetworks.com; Wangyijing;
suravee.suthikulpa...@amd.com; msal...@redhat.com; linux-
p...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
a...@vger.kernel.org; linux-kernel@vger.kernel.org; linaro-
a...@lists.linaro.org; j...@redhat.com; andrea.ga...@linaro.org;
dhd...@apm.com; jeremy.lin...@arm.com; c...@codeaurora.org; Chenxin
(Charles); Linuxarm
Subject: Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space
accessors against platfrom specific ECAM quirks

On 6/13/2016 9:54 AM, Gabriele Paoloni wrote:
> As you can see here Liudongdong has replaced oem_revision with
> oem_table_id.
>
> Now it seems that there are some platforms that have already shipped
> using a matching based on the oem_revision (right Jon?)
>
> However I guess that if in FW they have defined oem_table_id properly
> they should be able to use this mechanism without needing to a FW
update.
>
> Can these vendors confirm this?
>
> Tomasz do you think this can work for Cavium Thunder?
>
> Thanks
>
> Gab

Why not have all three of them?

The initial approach was OEM id and revision id.

Jeff Hugo indicated that addition (not removing any other fields) of
table id
would make more sense.


Mmm from last email of Jeff Hugo on "[RFC PATCH 1/3] pci, acpi: Match
PCI config space accessors against platfrom specific ECAM quirks."

I quote:

 "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."

So it looks to me that he pointed out that using the OEM revision field
is wrong...and this is why I have asked if replacing it with the table
id can work for other vendors

Thanks

Gab



I had an internal discussion with jeff and cov before posting on the 
maillist.


I think there is missing info in the email.

Usage of oem id + table id + revision is ok.

Usage of oem id + revision is not ok as one oem can build multiple chips 
with the same oem id and revision id but different table id. Otherwise, 
we can run out of revisions very quickly.






--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center,
Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project

RE: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Gabriele Paoloni]

Hi Sinan

> -Original Message-
> From: Sinan Kaya [mailto:ok...@codeaurora.org]
> Sent: 13 June 2016 15:03
> To: Gabriele Paoloni; liudongdong (C); helg...@kernel.org;
> a...@arndb.de; will.dea...@arm.com; catalin.mari...@arm.com;
> raf...@kernel.org; hanjun@linaro.org; lorenzo.pieral...@arm.com;
> jchan...@broadcom.com; t...@semihalf.com
> Cc: robert.rich...@caviumnetworks.com; m...@semihalf.com;
> liviu.du...@arm.com; dda...@caviumnetworks.com; Wangyijing;
> suravee.suthikulpa...@amd.com; msal...@redhat.com; linux-
> p...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
> a...@vger.kernel.org; linux-kernel@vger.kernel.org; linaro-
> a...@lists.linaro.org; j...@redhat.com; andrea.ga...@linaro.org;
> dhd...@apm.com; jeremy.lin...@arm.com; c...@codeaurora.org; Chenxin
> (Charles); Linuxarm
> Subject: Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space
> accessors against platfrom specific ECAM quirks
> 
> On 6/13/2016 9:54 AM, Gabriele Paoloni wrote:
> > As you can see here Liudongdong has replaced oem_revision with
> > oem_table_id.
> >
> > Now it seems that there are some platforms that have already shipped
> > using a matching based on the oem_revision (right Jon?)
> >
> > However I guess that if in FW they have defined oem_table_id properly
> > they should be able to use this mechanism without needing to a FW
> update.
> >
> > Can these vendors confirm this?
> >
> > Tomasz do you think this can work for Cavium Thunder?
> >
> > Thanks
> >
> > Gab
> 
> Why not have all three of them?
> 
> The initial approach was OEM id and revision id.
> 
> Jeff Hugo indicated that addition (not removing any other fields) of
> table id
> would make more sense.

Mmm from last email of Jeff Hugo on "[RFC PATCH 1/3] pci, acpi: Match
PCI config space accessors against platfrom specific ECAM quirks."

I quote:

 "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."

So it looks to me that he pointed out that using the OEM revision field
is wrong...and this is why I have asked if replacing it with the table
id can work for other vendors

Thanks

Gab


> 
> --
> Sinan Kaya
> Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center,
> Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
> Linux Foundation Collaborative Project

RE: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Gabriele Paoloni]

Hi Sinan

> -Original Message-
> From: Sinan Kaya [mailto:ok...@codeaurora.org]
> Sent: 13 June 2016 15:03
> To: Gabriele Paoloni; liudongdong (C); helg...@kernel.org;
> a...@arndb.de; will.dea...@arm.com; catalin.mari...@arm.com;
> raf...@kernel.org; hanjun@linaro.org; lorenzo.pieral...@arm.com;
> jchan...@broadcom.com; t...@semihalf.com
> Cc: robert.rich...@caviumnetworks.com; m...@semihalf.com;
> liviu.du...@arm.com; dda...@caviumnetworks.com; Wangyijing;
> suravee.suthikulpa...@amd.com; msal...@redhat.com; linux-
> p...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
> a...@vger.kernel.org; linux-kernel@vger.kernel.org; linaro-
> a...@lists.linaro.org; j...@redhat.com; andrea.ga...@linaro.org;
> dhd...@apm.com; jeremy.lin...@arm.com; c...@codeaurora.org; Chenxin
> (Charles); Linuxarm
> Subject: Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space
> accessors against platfrom specific ECAM quirks
> 
> On 6/13/2016 9:54 AM, Gabriele Paoloni wrote:
> > As you can see here Liudongdong has replaced oem_revision with
> > oem_table_id.
> >
> > Now it seems that there are some platforms that have already shipped
> > using a matching based on the oem_revision (right Jon?)
> >
> > However I guess that if in FW they have defined oem_table_id properly
> > they should be able to use this mechanism without needing to a FW
> update.
> >
> > Can these vendors confirm this?
> >
> > Tomasz do you think this can work for Cavium Thunder?
> >
> > Thanks
> >
> > Gab
> 
> Why not have all three of them?
> 
> The initial approach was OEM id and revision id.
> 
> Jeff Hugo indicated that addition (not removing any other fields) of
> table id
> would make more sense.

Mmm from last email of Jeff Hugo on "[RFC PATCH 1/3] pci, acpi: Match
PCI config space accessors against platfrom specific ECAM quirks."

I quote:

 "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."

So it looks to me that he pointed out that using the OEM revision field
is wrong...and this is why I have asked if replacing it with the table
id can work for other vendors

Thanks

Gab


> 
> --
> Sinan Kaya
> Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center,
> Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
> Linux Foundation Collaborative Project

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Sinan Kaya]

On 6/13/2016 9:54 AM, Gabriele Paoloni wrote:
> As you can see here Liudongdong has replaced oem_revision with
> oem_table_id.
> 
> Now it seems that there are some platforms that have already shipped
> using a matching based on the oem_revision (right Jon?)
> 
> However I guess that if in FW they have defined oem_table_id properly
> they should be able to use this mechanism without needing to a FW update.
> 
> Can these vendors confirm this?
> 
> Tomasz do you think this can work for Cavium Thunder?
> 
> Thanks
> 
> Gab

Why not have all three of them? 

The initial approach was OEM id and revision id. 

Jeff Hugo indicated that addition (not removing any other fields) of table id
would make more sense.

-- 
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Sinan Kaya]

On 6/13/2016 9:54 AM, Gabriele Paoloni wrote:
> As you can see here Liudongdong has replaced oem_revision with
> oem_table_id.
> 
> Now it seems that there are some platforms that have already shipped
> using a matching based on the oem_revision (right Jon?)
> 
> However I guess that if in FW they have defined oem_table_id properly
> they should be able to use this mechanism without needing to a FW update.
> 
> Can these vendors confirm this?
> 
> Tomasz do you think this can work for Cavium Thunder?
> 
> Thanks
> 
> Gab

Why not have all three of them? 

The initial approach was OEM id and revision id. 

Jeff Hugo indicated that addition (not removing any other fields) of table id
would make more sense.

-- 
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project

RE: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Gabriele Paoloni]

Hi Tomasz, Jon

> -Original Message-
> From: liudongdong (C)
> Sent: 13 June 2016 14:02
> To: helg...@kernel.org; a...@arndb.de; will.dea...@arm.com;
> catalin.mari...@arm.com; raf...@kernel.org; hanjun@linaro.org;
> lorenzo.pieral...@arm.com; ok...@codeaurora.org; jchan...@broadcom.com;
> t...@semihalf.com
> Cc: robert.rich...@caviumnetworks.com; m...@semihalf.com;
> liviu.du...@arm.com; dda...@caviumnetworks.com; Wangyijing;
> suravee.suthikulpa...@amd.com; msal...@redhat.com; linux-
> p...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
> a...@vger.kernel.org; linux-kernel@vger.kernel.org; linaro-
> a...@lists.linaro.org; j...@redhat.com; andrea.ga...@linaro.org;
> dhd...@apm.com; jeremy.lin...@arm.com; liudongdong (C);
> c...@codeaurora.org; Gabriele Paoloni; Chenxin (Charles); Linuxarm
> Subject: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors
> against platfrom specific ECAM quirks
> 
> From: Tomasz Nowicki 
> 
> 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
>  tuple and returns
> corresponding
> PCI config ops. oem_id and oem_table_id 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(_pci_ops, , ,
> , );
> 
> Signed-off-by: Tomasz Nowicki 
> Signed-off-by: Dongdong Liu 
> ---
>  drivers/acpi/pci_mcfg.c   | 41
> ---
>  include/asm-generic/vmlinux.lds.h |  7 +++
>  include/linux/pci-acpi.h  | 20 +++
>  3 files changed, 65 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> index d3c3e85..49612b3 100644
> --- a/drivers/acpi/pci_mcfg.c
> +++ b/drivers/acpi/pci_mcfg.c
> @@ -22,6 +22,10 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +
> +/* Root pointer to the mapped MCFG table */
> +static struct acpi_table_mcfg *mcfg_table;
> 
>  /* Structure to hold entries from the MCFG table */
>  struct mcfg_entry {
> @@ -35,6 +39,38 @@ struct mcfg_entry {
>  /* List to save mcfg entries */
>  static LIST_HEAD(pci_mcfg_list);
> 
> +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 _generic_ecam_ops;
> +
> + /*
> +  * Match against platform specific quirks and return
> corresponding
> +  * CAM ops.
> +  *
> +  * First match against PCI topology  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)) &&
> + (!strncmp(f->oem_table_id, mcfg_table-
> >header.oem_table_id,
> +   ACPI_OEM_TABLE_ID_SIZE)))

As you can see here Liudongdong has replaced oem_revision with
oem_table_id.

Now it seems that there are some platforms that have already shipped
using a matching based on the oem_revision (right Jon?)

However I guess that if in FW they have defined oem_table_id properly
they should be able to use this mechanism without needing to a FW update.

Can these vendors confirm this?

Tomasz do you think this can work for Cavium Thunder?

Thanks

Gab


> + return f->ops;
> + }
> + /* No quirks, use ECAM */
> + return _generic_ecam_ops;
> +}
> +
>  phys_addr_t pci_mcfg_lookup(u16 seg, struct resource *bus_res)
>  {
>   struct mcfg_entry *e;
> @@ -54,7 +90,6 @@ phys_addr_t pci_mcfg_lookup(u16 seg, struct resource
> *bus_res)
> 
>  static __init int pci_mcfg_parse(struct acpi_table_header *header)
>  {
> - struct acpi_table_mcfg *mcfg;
>   struct acpi_mcfg_allocation *mptr;
>   struct mcfg_entry *e, *arr;
>   int i, n;
> @@ -64,8 +99,8 @@ static __init int pci_mcfg_parse(struct
> acpi_table_header *header)
> 
>   n = (header->length - sizeof(struct acpi_table_mcfg)) /
> 

RE: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Gabriele Paoloni]

Hi Tomasz, Jon

> -Original Message-
> From: liudongdong (C)
> Sent: 13 June 2016 14:02
> To: helg...@kernel.org; a...@arndb.de; will.dea...@arm.com;
> catalin.mari...@arm.com; raf...@kernel.org; hanjun@linaro.org;
> lorenzo.pieral...@arm.com; ok...@codeaurora.org; jchan...@broadcom.com;
> t...@semihalf.com
> Cc: robert.rich...@caviumnetworks.com; m...@semihalf.com;
> liviu.du...@arm.com; dda...@caviumnetworks.com; Wangyijing;
> suravee.suthikulpa...@amd.com; msal...@redhat.com; linux-
> p...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
> a...@vger.kernel.org; linux-kernel@vger.kernel.org; linaro-
> a...@lists.linaro.org; j...@redhat.com; andrea.ga...@linaro.org;
> dhd...@apm.com; jeremy.lin...@arm.com; liudongdong (C);
> c...@codeaurora.org; Gabriele Paoloni; Chenxin (Charles); Linuxarm
> Subject: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors
> against platfrom specific ECAM quirks
> 
> From: Tomasz Nowicki 
> 
> 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
>  tuple and returns
> corresponding
> PCI config ops. oem_id and oem_table_id 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(_pci_ops, , ,
> , );
> 
> Signed-off-by: Tomasz Nowicki 
> Signed-off-by: Dongdong Liu 
> ---
>  drivers/acpi/pci_mcfg.c   | 41
> ---
>  include/asm-generic/vmlinux.lds.h |  7 +++
>  include/linux/pci-acpi.h  | 20 +++
>  3 files changed, 65 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> index d3c3e85..49612b3 100644
> --- a/drivers/acpi/pci_mcfg.c
> +++ b/drivers/acpi/pci_mcfg.c
> @@ -22,6 +22,10 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +
> +/* Root pointer to the mapped MCFG table */
> +static struct acpi_table_mcfg *mcfg_table;
> 
>  /* Structure to hold entries from the MCFG table */
>  struct mcfg_entry {
> @@ -35,6 +39,38 @@ struct mcfg_entry {
>  /* List to save mcfg entries */
>  static LIST_HEAD(pci_mcfg_list);
> 
> +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 _generic_ecam_ops;
> +
> + /*
> +  * Match against platform specific quirks and return
> corresponding
> +  * CAM ops.
> +  *
> +  * First match against PCI topology  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)) &&
> + (!strncmp(f->oem_table_id, mcfg_table-
> >header.oem_table_id,
> +   ACPI_OEM_TABLE_ID_SIZE)))

As you can see here Liudongdong has replaced oem_revision with
oem_table_id.

Now it seems that there are some platforms that have already shipped
using a matching based on the oem_revision (right Jon?)

However I guess that if in FW they have defined oem_table_id properly
they should be able to use this mechanism without needing to a FW update.

Can these vendors confirm this?

Tomasz do you think this can work for Cavium Thunder?

Thanks

Gab


> + return f->ops;
> + }
> + /* No quirks, use ECAM */
> + return _generic_ecam_ops;
> +}
> +
>  phys_addr_t pci_mcfg_lookup(u16 seg, struct resource *bus_res)
>  {
>   struct mcfg_entry *e;
> @@ -54,7 +90,6 @@ phys_addr_t pci_mcfg_lookup(u16 seg, struct resource
> *bus_res)
> 
>  static __init int pci_mcfg_parse(struct acpi_table_header *header)
>  {
> - struct acpi_table_mcfg *mcfg;
>   struct acpi_mcfg_allocation *mptr;
>   struct mcfg_entry *e, *arr;
>   int i, n;
> @@ -64,8 +99,8 @@ static __init int pci_mcfg_parse(struct
> acpi_table_header *header)
> 
>   n = (header->length - sizeof(struct acpi_table_mcfg)) /
>   sizeof(struct acpi_mcfg_allocation);
> - mcfg = (struct acpi_table_mcfg