On Wed, Nov 19, 2014 at 10:24:52AM +0800, Yijing Wang wrote:
> >> 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! :)
Given that I also look at this area maybe we should join forces and divide the problem?
Best regards,
Liviu
>
>
> 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-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
-------------------
.oooO
( )
\ ( Oooo.
\_) ( )
) /
(_/
One small step
for me ...
--
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
