On 08/23/2011 04:27 PM, Alan Stern wrote:
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?
Look this device-specific stuff that works nice:
On kexec for instance, the ->shutdown callback is called. The xhci-core has:
static void xhci_plat_shutdown(struct platform_device *pdev)
{
struct usb_hcd *hcd = platform_get_drvdata(pdev);
if (!hcd)
return;
if (test_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags) &&
hcd->driver->shutdown)
hcd->driver->shutdown(hcd);
}
which handles the xhci specific stuff. The PCI wrapper has only
static void usb_xhci_pci_shutdown(struct pci_dev *dev)
{
pci_disable_device(dev);
}
Other platforms could have similar tweaks. The same goes for PM. So on
embedded platforms you could easy add clock support or fiddle with some
SoC specific registers where you disable parts of the core or put it in
low power mode while don't have to touch the PCI code at all via ifdefs.
I agree that this patch is kinda messy. I did not want to create
hcd-plat.c and copy the code with the different prototypes /
assumptions. I think the mess should go away if ehci/ohci gets
converted in a similar fashion.
Alan Stern
Sebastian
--
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
