>> We need, some platforms pass NULL pointer as host bridge parent. > > Yijing, > > May I suggest a different approach here? Rather than having to pass an opaque > pointer that gets converted by the host bridge driver back to the private > structure, what about promoting a new style of usage, that is similar to the > way device drivers work? Lets try to promote the embedding of the generic > pci_host_bridge structure in the host bridge specific structure! Then we can > access the private data doing container_of(). > > Something like this: > > struct pci_controller { > struct pci_host_bridge bridge; > /* private host bridge data here */ > ..... > }; > > #define PCI_CONTROLLER(bus) ({ > struct pci_host_bridge *hb = to_pci_host_bridge(bus->bridge); \ > container_of(hb, struct pci_controller, bridge); }) > > > Then we can retrieve the host bridge structure from everywhere we have a device. Hi Liviu, it looks good to me, because this change will involve lots platforms, I would think more about it. Thanks for your suggestion very much! :) Thanks! Yijing. > > Best regards, > Liviu > >> >>> >>>> + host = kzalloc(sizeof(*host), GFP_KERNEL); >>>> + if (!host) >>>> + return NULL; >>> >>> devm_kzalloc maybe? >> >> I don't know much detail about devm_kzalloc(), but we have no pci host driver >> here, and I found no devm_kzalloc() uses in core PCI code before. >> >>> >>>> + if (!resources) { >>>> + /* Use default IO/MEM/BUS resources*/ >>>> + pci_add_resource(&host->windows, &ioport_resource); >>>> + pci_add_resource(&host->windows, &iomem_resource); >>>> + pci_add_resource(&host->windows, &busn_resource); >>>> + } else { >>>> + list_for_each_entry_safe(window, n, resources, list) >>>> + list_move_tail(&window->list, &host->windows); >>>> + } >>> >>> I think we should assume that the correct resources are passed. You >>> could add a wrapper around this function to convert old platforms >>> though. >> >> OK, I will move these code out of pci_create_host_bridge, and add a wrapper >> to setup the default resources. >> >>> >>>> +EXPORT_SYMBOL(pci_create_host_bridge); >>> >>> EXPORT_SYMBOL_GPL() maybe? >> >> OK, will update it. >> >>> >>>> diff --git a/include/linux/pci.h b/include/linux/pci.h >>>> index 8b11b38..daa7f40 100644 >>>> --- a/include/linux/pci.h >>>> +++ b/include/linux/pci.h >>>> @@ -402,7 +402,12 @@ struct pci_host_bridge_window { >>>> struct pci_host_bridge { >>>> struct device dev; >>>> struct pci_bus *bus; /* root bus */ >>>> + struct list_head list; >>>> struct list_head windows; /* pci_host_bridge_windows */ >>>> + int busnum; >>> >>> The busnum should already be implied through the bus resource. >> >> Yes, I will consider remove it and introduce a helper function to get the root bus number, thanks! >> >> Thanks! >> Yijing. >> >>> >>> Arnd >>> >>> . >>> >> >> >> -- >> 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 >> > -- 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