On 2015/11/6 2:19, Lorenzo Pieralisi wrote:
> On Thu, Nov 05, 2015 at 03:21:34PM +0100, Tomasz Nowicki wrote:
>> On 14.10.2015 08:29, Jiang Liu wrote:
>
> [...]
>
>>> +static void acpi_pci_root_validate_resources(struct device *dev,
>>> + struct list_head *resources,
>>> + unsigned long type)
>>> +{
>>> + LIST_HEAD(list);
>>> + struct resource *res1, *res2, *root = NULL;
>>> + struct resource_entry *tmp, *entry, *entry2;
>>> +
>>> + BUG_ON((type & (IORESOURCE_MEM | IORESOURCE_IO)) == 0);
>>> + root = (type & IORESOURCE_MEM) ? &iomem_resource : &ioport_resource;
>>> +
>>> + list_splice_init(resources, &list);
>>> + resource_list_for_each_entry_safe(entry, tmp, &list) {
>>> + bool free = false;
>>> + resource_size_t end;
>>> +
>>> + res1 = entry->res;
>>> + if (!(res1->flags & type))
>>> + goto next;
>>> +
>>> + /* Exclude non-addressable range or non-addressable portion */
>>> + end = min(res1->end, root->end);
>>> + if (end <= res1->start) {
>>> + dev_info(dev, "host bridge window %pR (ignored, not CPU addressable)\n",
>>> + res1);
>>> + free = true;
>>> + goto next;
>>> + } else if (res1->end != end) {
>>> + dev_info(dev, "host bridge window %pR ([%#llx-%#llx] ignored, not CPU addressable)\n",
>>> + res1, (unsigned long long)end + 1,
>>> + (unsigned long long)res1->end);
>>> + res1->end = end;
>>> + }
>>> +
>>> + resource_list_for_each_entry(entry2, resources) {
>>> + res2 = entry2->res;
>>> + if (!(res2->flags & type))
>>> + continue;
>>> +
>>> + /*
>>> + * I don't like throwing away windows because then
>>> + * our resources no longer match the ACPI _CRS, but
>>> + * the kernel resource tree doesn't allow overlaps.
>>> + */
>>> + if (resource_overlaps(res1, res2)) {
>>> + res2->start = min(res1->start, res2->start);
>>> + res2->end = max(res1->end, res2->end);
>>> + dev_info(dev, "host bridge window expanded to %pR; %pR ignored\n",
>>> + res2, res1);
>>> + free = true;
>>> + goto next;
>>> + }
>>> + }
>>> +
>>> +next:
>>> + resource_list_del(entry);
>>> + if (free)
>>> + resource_list_free_entry(entry);
>>> + else
>>> + resource_list_add_tail(entry, resources);
>>> + }
>>> +}
>>> +
>>> +int acpi_pci_probe_root_resources(struct acpi_pci_root_info *info)
>>> +{
>>> + int ret;
>>> + struct list_head *list = &info->resources;
>>> + struct acpi_device *device = info->bridge;
>>> + struct resource_entry *entry, *tmp;
>>> + unsigned long flags;
>>> +
>>> + flags = IORESOURCE_IO | IORESOURCE_MEM | IORESOURCE_MEM_8AND16BIT;
>>> + ret = acpi_dev_get_resources(device, list,
>>> + acpi_dev_filter_resource_type_cb,
>>> + (void *)flags);
>>> + if (ret < 0)
>>> + dev_warn(&device->dev,
>>> + "failed to parse _CRS method, error code %d\n", ret);
>>> + else if (ret == 0)
>>> + dev_dbg(&device->dev,
>>> + "no IO and memory resources present in _CRS\n");
>>> + else {
>>> + resource_list_for_each_entry_safe(entry, tmp, list) {
>>> + if (entry->res->flags & IORESOURCE_DISABLED)
>>> + resource_list_destroy_entry(entry);
>>> + else
>>> + entry->res->name = info->name;
>>> + }
>>> + acpi_pci_root_validate_resources(&device->dev, list,
>>> + IORESOURCE_MEM);
>>> + acpi_pci_root_validate_resources(&device->dev, list,
>>> + IORESOURCE_IO);
>>
>> It is not clear to me why we need these two calls above ^^^. We are
>> using pci_acpi_root_add_resources(info) later. Is it not enough?
>>
>> Also, I cannot use acpi_pci_probe_root_resources() in my ARM64 PCI
>> driver. It is because acpi_dev_get_resources is adding
>> translation_offset to IO ranges start address and then:
>> acpi_pci_root_validate_resources(&device->dev, list,
>> IORESOURCE_IO);
>> rejects that IO regions as it is out of my 0x0-SZ_16M window.
>>
>> Does acpi_pci_probe_root_resources meant to be x86 specific and I
>> should avoid using it?
>
> IIUC, you _have_ to have the proper translation_offset to map the bridge
> window into the IO address space:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-June/348708.html
>
> Then, using the offset, you should do something ia64 does, namely,
> retrieve the CPU address corresponding to IO space (see arch/ia64/pci/pci.c
> - add_io_space()) and map it in the physical address space by using
> pci_remap_iospace(), it is similar to what we have to do with DT.
>
> It is extremely confusing and I am not sure I got it right myself,
> I am still grokking ia64 code to understand what it really does.
>
> So basically, the IO bridge window coming from acpi_dev_get_resource()
> should represent the IO space in 0 - 16M, IIUC.
>
> By using the offset (that was initialized using translation_offset) and
> the resource->start, you can retrieve the cpu address that you need to
> actually map the IO space, since that's what we do on ARM (ie the
> IO resource is an offset into the virtual address space set aside
> for IO).
>
> Confusing, to say the least. Jiang, did I get it right ?
Yes, you are right, but seems I have done something wrong here.
Currently res->start = acpi_des->start + acpi_des->translation_offset,
which seems wrong. I will try to check IA64 side again and find
a fix for this.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
