Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1311520 > unrolled thread

Re: [PATCH V3 20/21] pci, acpi: Match PCI config space accessors against platfrom specific quirks.

Started byTomasz Nowicki <tn@semihalf.com>
First post2016-01-18 13:50 +0100
Last post2016-01-19 10:00 +0100
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V3 20/21] pci, acpi: Match PCI config space accessors  against platfrom specific quirks. Tomasz Nowicki <tn@semihalf.com> - 2016-01-18 13:50 +0100
    Re: [PATCH V3 20/21] pci, acpi: Match PCI config space accessors  against platfrom specific quirks. "liudongdong (C)" <liudongdong3@huawei.com> - 2016-01-19 03:00 +0100
      Re: [PATCH V3 20/21] pci, acpi: Match PCI config space accessors  against platfrom specific quirks. Tomasz Nowicki <tn@semihalf.com> - 2016-01-19 09:00 +0100
        Re: [PATCH V3 20/21] pci, acpi: Match PCI config space accessors  against platfrom specific quirks. "liudongdong (C)" <liudongdong3@huawei.com> - 2016-01-19 10:00 +0100

#1311520 — Re: [PATCH V3 20/21] pci, acpi: Match PCI config space accessors against platfrom specific quirks.

FromTomasz Nowicki <tn@semihalf.com>
Date2016-01-18 13:50 +0100
SubjectRe: [PATCH V3 20/21] pci, acpi: Match PCI config space accessors against platfrom specific quirks.
Message-ID<qShpT-GJ-5@gated-at.bofh.it>
On 14.01.2016 16:36, Mark Salter wrote:
>> +extern struct pci_mcfg_fixup __start_acpi_mcfg_fixups[];
>> >+extern struct pci_mcfg_fixup __end_acpi_mcfg_fixups[];
>> >+
>> >+static struct pci_ops *pci_mcfg_check_quirks(struct acpi_pci_root *root)
>> >+{
>> >+	struct pci_mcfg_fixup *f;
>> >+	int bus_num = root->secondary.start;
>> >+	int domain = root->segment;
>> >+
>> >+	/*
>> >+	 * First match against PCI topology <domain:bus> then use DMI or
>> >+	 * custom match handler.
>> >+	 */
>> >+	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) &&
>> >+		    (f->system ? dmi_check_system(f->system) : 0 ||
>> >+		     f->match ? f->match(f, root) : 0))
>> >+			return f->ops;
> I think this would be better as:
>
> 		    (f->system ? dmi_check_system(f->system) : 1 &&
> 		     f->match ? f->match(f, root) : 1))
> 			return f->ops;
>
> Otherwise, one has to call dmi_check_system() from f->match() if
> access to root is needed.

Makes a lot of sense to me, I will modify as you suggested.

Tomasz

[toc] | [next] | [standalone]

#1311895

From"liudongdong (C)" <liudongdong3@huawei.com>
Date2016-01-19 03:00 +0100
Message-ID<qStKr-Kn-21@gated-at.bofh.it>
In reply to#1311520
Hi Tomasz, Mark

在 2016/1/18 20:41, Tomasz Nowicki 写道:
> On 14.01.2016 16:36, Mark Salter wrote:
>>> +extern struct pci_mcfg_fixup __start_acpi_mcfg_fixups[];
>>> >+extern struct pci_mcfg_fixup __end_acpi_mcfg_fixups[];
>>> >+
>>> >+static struct pci_ops *pci_mcfg_check_quirks(struct acpi_pci_root *root)
>>> >+{
>>> >+    struct pci_mcfg_fixup *f;
>>> >+    int bus_num = root->secondary.start;
>>> >+    int domain = root->segment;
>>> >+
>>> >+    /*
>>> >+     * First match against PCI topology <domain:bus> then use DMI or
>>> >+     * custom match handler.
>>> >+     */
>>> >+    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) &&
>>> >+            (f->system ? dmi_check_system(f->system) : 0 ||
>>> >+             f->match ? f->match(f, root) : 0))
>>> >+            return f->ops;
>> I think this would be better as:
>>
>>             (f->system ? dmi_check_system(f->system) : 1 &&
>>              f->match ? f->match(f, root) : 1))
>>             return f->ops;
>>
>> Otherwise, one has to call dmi_check_system() from f->match() if
>> access to root is needed.
>

Non-DMI, we need not to call dmi_check_system() from f->match(),
we can use _HID to decide to hook the pci_ops or not.

Device (PCI1)
{
         Name (_HID, "HISI0080") // PCI Express Root Bridge
         Name (_CID, "PNP0A03") // Compatible PCI Root Bridge
	...
}

static const struct acpi_device_id hisi_pcie_ids[] = {
     {"HISI0080", 0},
     {"", 0},
};

static int hisi_pcie_match(struct pci_mcfg_fixup *fixup, struct acpi_pci_root *root)
{
     int ret;
     struct acpi_device *device;

     device = root->device;
     ret = acpi_match_device_ids(device, hisi_pcie_ids);
     if (ret)
         return 0;
     ......
     return 1;
}

static struct pci_ops hisi_ecam_pci_ops = {
     .map_bus = pci_mcfg_dev_base,
     .read = hisi_pci_read,
     .write = hisi_pci_write,
};

DECLARE_ACPI_MCFG_FIXUP(NULL, hisi_pcie_match, &hisi_ecam_pci_ops,
     PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);

Thanks
Dongdong
> Makes a lot of sense to me, I will modify as you suggested.
>
> Tomasz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> .
>

[toc] | [prev] | [next] | [standalone]

#1311985

FromTomasz Nowicki <tn@semihalf.com>
Date2016-01-19 09:00 +0100
Message-ID<qSzmO-4JC-21@gated-at.bofh.it>
In reply to#1311895
On 19.01.2016 02:49, liudongdong (C) wrote:
> Hi Tomasz, Mark
>
> 在 2016/1/18 20:41, Tomasz Nowicki 写道:
>> On 14.01.2016 16:36, Mark Salter wrote:
>>>> +extern struct pci_mcfg_fixup __start_acpi_mcfg_fixups[];
>>>> >+extern struct pci_mcfg_fixup __end_acpi_mcfg_fixups[];
>>>> >+
>>>> >+static struct pci_ops *pci_mcfg_check_quirks(struct acpi_pci_root
>>>> *root)
>>>> >+{
>>>> >+    struct pci_mcfg_fixup *f;
>>>> >+    int bus_num = root->secondary.start;
>>>> >+    int domain = root->segment;
>>>> >+
>>>> >+    /*
>>>> >+     * First match against PCI topology <domain:bus> then use DMI or
>>>> >+     * custom match handler.
>>>> >+     */
>>>> >+    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) &&
>>>> >+            (f->system ? dmi_check_system(f->system) : 0 ||
>>>> >+             f->match ? f->match(f, root) : 0))
>>>> >+            return f->ops;
>>> I think this would be better as:
>>>
>>>             (f->system ? dmi_check_system(f->system) : 1 &&
>>>              f->match ? f->match(f, root) : 1))
>>>             return f->ops;
>>>
>>> Otherwise, one has to call dmi_check_system() from f->match() if
>>> access to root is needed.
>>
>
> Non-DMI, we need not to call dmi_check_system() from f->match(),
> we can use _HID to decide to hook the pci_ops or not.

Sorry, but I dont understand your point. Can you elaborate?

With Mark modification, you can use the following cases to identify 
platform:
1. DMI only
2. f->match() only (_HID can be used there)
3. DMI and f->match()

DMI used to be very convenient way to recognise platform, sometimes it 
is not enough, hence f->match() alternative.

Tomasz

[toc] | [prev] | [next] | [standalone]

#1312019

From"liudongdong (C)" <liudongdong3@huawei.com>
Date2016-01-19 10:00 +0100
Message-ID<qSAiS-5me-29@gated-at.bofh.it>
In reply to#1311985

在 2016/1/19 15:55, Tomasz Nowicki 写道:
> On 19.01.2016 02:49, liudongdong (C) wrote:
>> Hi Tomasz, Mark
>>
>> 在 2016/1/18 20:41, Tomasz Nowicki 写道:
>>> On 14.01.2016 16:36, Mark Salter wrote:
>>>>> +extern struct pci_mcfg_fixup __start_acpi_mcfg_fixups[];
>>>>> >+extern struct pci_mcfg_fixup __end_acpi_mcfg_fixups[];
>>>>> >+
>>>>> >+static struct pci_ops *pci_mcfg_check_quirks(struct acpi_pci_root
>>>>> *root)
>>>>> >+{
>>>>> >+    struct pci_mcfg_fixup *f;
>>>>> >+    int bus_num = root->secondary.start;
>>>>> >+    int domain = root->segment;
>>>>> >+
>>>>> >+    /*
>>>>> >+     * First match against PCI topology <domain:bus> then use DMI or
>>>>> >+     * custom match handler.
>>>>> >+     */
>>>>> >+    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) &&
>>>>> >+            (f->system ? dmi_check_system(f->system) : 0 ||
>>>>> >+             f->match ? f->match(f, root) : 0))
>>>>> >+            return f->ops;
>>>> I think this would be better as:
>>>>
>>>>             (f->system ? dmi_check_system(f->system) : 1 &&
>>>>              f->match ? f->match(f, root) : 1))
>>>>             return f->ops;
>>>>
>>>> Otherwise, one has to call dmi_check_system() from f->match() if
>>>> access to root is needed.
>>>
>>
>> Non-DMI, we need not to call dmi_check_system() from f->match(),
>> we can use _HID to decide to hook the pci_ops or not.
>
> Sorry, but I dont understand your point. Can you elaborate?
>
> With Mark modification, you can use the following cases to identify platform:
> 1. DMI only
> 2. f->match() only (_HID can be used there)
> 3. DMI and f->match()
>
> DMI used to be very convenient way to recognise platform, sometimes it is not enough, hence f->match() alternative.
>

Yes, you are right,  I was wrong.
In my case, I can use the second point.
2. f->match() only (_HID can be used there)

Thanks
Dongdong

> Tomasz
>
>
>
> .
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web