On Mon, 22 Aug 2011, Sarah Sharp wrote: > On Mon, Aug 15, 2011 at 04:51:27PM +0200, Sebastian Andrzej Siewior wrote: > > usb_hcd_pci_pm_ops provides run-time functionality for the USB HCDs. > > It expects that driver's data is a struct usb_hcd. This does not work > > if we create the HCD device is a platform device and its parent is the > > PCI device. We create the pci_dev from device and usb_hcd from parent's > > driver data. > > This patch is preparation to let the xhci core work under a platform > > device. > > This patch looks a bit messy, with all the nearly-duplicate function > names like check_root_hub_suspended and _check_root_hub_suspended. > > I think the code might be more readable if check_root_hub_suspended was > renamed to check_common_root_hub_suspended and you added two new > functions: the check_plat_pci_root_hub_suspended function you already > have, and another function called check_pci_root_hub_suspended. > You could do similar things with all the functions in this patch. > > Alan, what do you think? There's no doubt that the patch is messy. This is because it tries to cater to two different approaches simultaneously: the old (use the PCI device directly) and the new (interpose a fake platform device below the PCI device). It would be better to settle on a single approach. Personally, I think it's more than a little odd to have the fake platform device in the middle like that. It also runs the risk of messing up existing userspace programs that do power management on the controller device; they might easily contain references to paths like /sys/bus/usb/devices/usb3/../power/. Certainly I have done things like that in the past. Is there a better way to accomplish the desired effect? Not just for this one patch but for the entire series? For example, maybe the core parts of xhci-hcd could be made into a standalone module, containing routines that would be called by separate driver modules (one for xhci-pci and others for various platform variants). There would be a disadvantage, because now two modules would have to be loaded to manage an xHCI controller, but overall it would be cleaner. And of course, the same approach could be applied to the other joint PCI/platform HCDs. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
