On Tue, Oct 20, 2015 at 3:16 PM, Yinghai Lu <yinghai@xxxxxxxxxx> wrote:
> On Tue, Oct 20, 2015 at 2:57 PM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
>> On Thu, Oct 08, 2015 at 02:38:22PM -0700, Yinghai Lu wrote:
>>> We register regions for legacy and iommu and all have open code.
>>>
>>> Unify them to pci_register_region() and call it accordingly.
>>>
>>> + if (!mem_res->flags)
>>> return;
>>>
>>> - p->name = "Video RAM area";
>>> - p->start = mem_res->start + 0xa0000UL;
>>> - p->end = p->start + 0x1ffffUL;
>>> - p->flags = IORESOURCE_BUSY;
>>> - request_resource(mem_res, p);
>>> + mem_rstart = mem_res->start - offset;
>>> + mem_rend = mem_res->end - offset;
>>
>> This is essentially doing pcibios_bus_to_resource(), except in an
>> unnecessarily arch-dependent way. Can you rework it to use
>> pcibios_bus_to_resource()? I think you'll have to do
>> pci_register_legacy_regions() in pci_scan_one_pbm() instead of in
>> pci_determine_mem_io_space(), so that you have a struct pci_bus to
>> use, but your next patch does that anyway.
>>
>>>
>>> - p = kzalloc(sizeof(*p), GFP_KERNEL);
>>> - if (!p)
>>> + /* contain checking */
>>> + if (!(mem_rstart <= rstart && mem_rend >= rend))
>>> return;
>
> Difference is here.
>
> pcibios_bus_to_resource() would return resource even region is not contained.
>
>>> + res->name = name;
>>> + res->flags = IORESOURCE_MEM | IORESOURCE_BUSY;
>>> + res->start = rstart + offset;
>>> + res->end = rend + offset;
>>> + conflict = request_resource_conflict(mem_res, res);
>>> + if (conflict) {
>>> + printk(KERN_DEBUG "PCI: %s can't claim %s %pR: address conflict with %s %pR\n",
>>> + pbm->name, res->name, res, conflict->name, conflict);
>>> + kfree(res);
>>> + }
>
> also we will need to get parent resource to use request_resource_conflict.
so do you agree following change to pcibios_bus_to_resource() ?
then we can use __pcibios_bus_resource().
diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
index 5f4a2e0..6c73327 100644
--- a/drivers/pci/host-bridge.c
+++ b/drivers/pci/host-bridge.c
@@ -70,12 +70,15 @@ static bool region_contains(struct pci_bus_region *region1,
return region1->start <= region2->start && region1->end >= region2->end;
}
-void pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
- struct pci_bus_region *region)
+int __pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
+ struct pci_bus_region *region,
+ struct resource **res_ret,
+ resource_size *off_ret)
{
struct pci_host_bridge *bridge = pci_find_host_bridge(bus);
struct resource_entry *window;
resource_size_t offset = 0;
+ int found = 0;
resource_list_for_each_entry(window, &bridge->windows) {
struct pci_bus_region bus_region;
@@ -88,11 +91,23 @@ void pcibios_bus_to_resource(struct pci_bus *bus,
struct resource *res,
if (region_contains(&bus_region, region)) {
offset = window->offset;
+ if (off_ret)
+ *off_ret = offset;
+ if (res_ret)
+ *res_ret = window->res;
+ found = 1;
break;
}
}
res->start = region->start + offset;
res->end = region->end + offset;
+
+ return found;
+}
+void pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
+ struct pci_bus_region *region)
+{
+ __pcibios_bus_to_resource(bus, res, region, NULL, NULL);
}
EXPORT_SYMBOL(pcibios_bus_to_resource);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index b54fbf1..bec3eed 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -781,6 +781,10 @@ void pci_fixup_cardbus(struct pci_bus *);
void pcibios_resource_to_bus(struct pci_bus *bus, struct
pci_bus_region *region,
struct resource *res);
+int __pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
+ struct pci_bus_region *region,
+ struct resource **res_ret,
+ resource_size *off_ret);
void pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
struct pci_bus_region *region);
void pcibios_scan_specific_bus(int busn);
--
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
