>> struct pci_host_bridge *pci_create_host_bridge(
>> - struct device *parent, u32 db,
>> - struct pci_ops *ops, void *sysdata,
>> - struct list_head *resources)
>> + struct device *parent, u32 db, struct pci_ops *ops,
>> + struct pci_host_info *info)
>> {
>> int error;
>> struct pci_bus *b;
>> struct pci_host_bridge *host, *h;
>> - struct pci_host_bridge_window *window, *n;
>>
>> down_read(&pci_host_bridge_sem);
>> list_for_each_entry(h, &pci_host_bridge_list, list) {
>> @@ -53,7 +51,7 @@ struct pci_host_bridge *pci_create_host_bridge(
>> if (!host)
>> return NULL;
>>
>> - host->sysdata = sysdata;
>> + host->sysdata = info->arg;
>> host->busnum = PCI_BUSNUM(db);
>> host->domain = PCI_DOMAIN(db);
>> host->ops = ops;
>> @@ -63,18 +61,23 @@ struct pci_host_bridge *pci_create_host_bridge(
>>
>> /* this is hack, just for build, will be removed later*/
>
> Why do you need this hack? Just for calling pci_domain_nr() ?
Yes, it's temporary code, we need domain number here for pci host bridge register.
>
>> b = kzalloc(sizeof(*b), GFP_KERNEL);
>> - b->sysdata = sysdata;
>> + b->sysdata = host->sysdata;
>> pci_bus_assign_domain_nr(b, parent);
>> host->domain = pci_domain_nr(b);
>> + kfree(b);
>>
...
>> +static int pci_default_init_res(struct pci_host_bridge *host,
>> + struct pci_host_info *info)
>> +{
>> + struct pci_host_bridge_window *window, *n;
>> +
>> + if (info->res_type != PCI_HOST_RES_DEFAULT)
>> + list_for_each_entry_safe(window, n, info->resources,
>> + list)
>> + list_move_tail(&window->list, &host->windows);
>> + else
>> + info->res_type = PCI_HOST_RES_DEFAULT;
>
> I'm confused about this assignment. Isn't this a nop as the else part
> means info->res_type *is* PCI_HOST_RES_DEFAULT?
No, in this patch, host drivers pass a pci host bridge resources init hook
in pci_host_info *info, and we call this info->init_res() in pci_create_host_bridge().
+struct pci_host_info {
+ u8 res_type;
+ void *arg;
+ struct list_head *resources; /*just for build, will clean up later */
+ int (*init_res)(struct pci_host_bridge *host,
+ struct pci_host_info *info);
+};
+
>
>> +
>> + return 0;
>> +}
...
>> @@ -2038,8 +2057,13 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, u32 db,
>> bool found = false;
>> struct pci_host_bridge *host;
>> int max;
>> + struct pci_host_info info;
>> +
>> + info.arg = sysdata;
>> + info.resources = resources;
>> + info.init_res = pci_default_init_res;
>
> I have mixed feelings about this patch. While it is heading in the right direction
> of moving pci_host_bridge relevant information towards the right user, I don't think
> you picked up the right set to move. The resource list is going to be copied into
> internal pci_host_bridge list anyway, keeping another copy is not helpful *and*
> you have increased the code size.
>
> I think for now we should aim to get the *missing* data into pci_host_bridge: MSI
> controllers and PCI domain/segment. Then we can do more cleanup.
Hi Liviu, I agree with you here, the changes to resources stuff seems not a perfect
solution. In my patch 6, we could pass pci domain nr by u32 PCI_DOMBUS(domain, bus) argument,
and store it in pci_host_bridge. For msi controller, we couldn't save the msi_controller
in pci_host_bridge. Before we assume one pci host bridge only had one msi_controller,
but now something changes, Jiang introduce hierarchy irq domain in x86, and now
one pci host bridge may has more than one msi_controller. So I prefer to add a
function to pci_host_bridge something like
struct msi_controller *pci_get_msi_controller(struct pci_dev *dev)
>
>>
>> - host = pci_create_host_bridge(parent, db, ops, sysdata, resources);
>> + host = pci_create_host_bridge(parent, db, ops, &info);
>> if (!host)
>> return NULL;
>>
>> @@ -2070,6 +2094,47 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, u32 db,
>> }
>> EXPORT_SYMBOL(pci_scan_root_bus);
>>
>> +struct pci_host_bridge *pci_scan_host_bridge(
>> + struct device *parent, u32 db, struct pci_ops *ops,
>> + struct pci_host_info *info)
>> +{
>> + struct pci_host_bridge_window *window;
>> + bool found = false;
>> + struct pci_host_bridge *host;
>> + int max;
>> +
>> + host = pci_create_host_bridge(parent, db, ops, info);
>> + if (!host)
>> + return NULL;
>> +
>> + list_for_each_entry(window, &host->windows, list)
>> + if (window->res->flags & IORESOURCE_BUS) {
>> + found = true;
>> + break;
>> + }
>> +
>> + host->bus = __pci_create_root_bus(host);
>> + if (!host->bus) {
>> + pci_free_host_bridge(host);
>> + return NULL;
>> + }
>> +
>> + if (!found) {
>> + dev_info(&host->bus->dev,
>> + "No busn resource found for root bus, will use [bus %02x-ff]\n",
>> + host->busnum);
>> + pci_bus_insert_busn_res(host->bus, host->busnum, 255);
>> + }
>> +
>> + max = pci_scan_child_bus(host->bus);
>> + if (!found)
>> + pci_bus_update_busn_res_end(host->bus, max);
>> +
>> + return host;
>> +
>> +}
>> +EXPORT_SYMBOL(pci_scan_host_bridge);
>> +
>> /**
>> * pci_rescan_bus_bridge_resize - scan a PCI bus for devices.
>> * @bridge: PCI bridge for the bus to scan
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index daa7f40..a51f5f5 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -412,6 +412,21 @@ struct pci_host_bridge {
>> void *release_data;
>> };
>>
>> +struct pci_host_info {
>> + u8 res_type;
>> + void *arg;
>> + struct list_head *resources; /*just for build, will clean up later */
>> + int (*init_res)(struct pci_host_bridge *host,
>> + struct pci_host_info *info);
>> +};
>> +
>> +static inline void init_pci_host_info(struct pci_host_info *info)
>> +{
>> + memset(info, 0 , sizeof(*info));
>> +}
>
> Where is this used?
Host driver uses it to init pci_host_info.
>
>> +
>> +#define PCI_HOST_RES_DEFAULT 0x2
>> +
>
> Magic number?
Hmmm, I will rework pci host bridge resources stuff.
>
> Best regards,
> Liviu
>
>> #define to_pci_host_bridge(n) container_of(n, struct pci_host_bridge, dev)
>> void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
>> void (*release_fn)(struct pci_host_bridge *),
>> @@ -420,7 +435,7 @@ void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
>> int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge);
>> struct pci_host_bridge *pci_create_host_bridge(
>> struct device *parent, u32 db, struct pci_ops *ops,
>> - void *sys, struct list_head *resources);
>> + struct pci_host_info *info);
>> /*
>> * The first PCI_BRIDGE_RESOURCE_NUM PCI bus resources (those that correspond
>> * to P2P or CardBus bridge windows) go in a table. Additional ones (for
>> @@ -785,6 +800,9 @@ void pci_bus_release_busn_res(struct pci_bus *b);
>> struct pci_bus *pci_scan_root_bus(struct device *parent, u32 bus,
>> struct pci_ops *ops, void *sysdata,
>> struct list_head *resources);
>> +struct pci_host_bridge *pci_scan_host_bridge(struct device *parent,
>> + u32 db, struct pci_ops *ops,
>> + struct pci_host_info *info);
>> struct pci_bus *pci_add_new_bus(struct pci_bus *parent, struct pci_dev *dev,
>> int busnr);
>> void pcie_update_link_speed(struct pci_bus *bus, u16 link_status);
>> --
>> 1.7.1
>>
>> --
>> 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
>>
>
--
Thanks!
Yijing
--
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
