On Fri, 26 Aug 2011, Alan Cox wrote:
> From: Alek Du <alek.du@xxxxxxxxx>
>
> The Intel Moorestown platform has EHCI MPH and EHCI OTG host. This patch adds
> PCI probe part for them. The HNP part and SRAM part will be added later.
USB people can be expected to know what EHCI, OTG, PCI, HNP, and SRAM
stand for. What's MPH?
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1614,6 +1614,14 @@ static void hub_free_dev(struct usb_device *udev)
> hcd->driver->free_dev(hcd, udev);
> }
>
> +static void otg_notify(struct usb_device *udev, unsigned action)
> +{
> + struct usb_hcd *hcd = bus_to_hcd(udev->bus);
> +
> + if (hcd->otg_notify)
> + hcd->otg_notify(udev, action);
> +}
> +
> /**
> * usb_disconnect - disconnect a device (usbcore-internal)
> * @pdev: pointer to device being disconnected
> @@ -1670,6 +1678,7 @@ void usb_disconnect(struct usb_device **pdev)
> * notifier chain (used by usbfs and possibly others).
> */
> device_del(&udev->dev);
> + otg_notify(udev, USB_DEVICE_REMOVE);
>
> /* Free the device number and delete the parent's children[]
> * (or root_hub) pointer.
> @@ -1900,6 +1909,7 @@ int usb_new_device(struct usb_device *udev)
> dev_err(&udev->dev, "can't device_add, error %d\n", err);
> goto fail;
> }
> + otg_notify(udev, USB_DEVICE_ADD);
>
> (void) usb_create_ep_devs(&udev->dev, &udev->ep0, udev);
> usb_mark_last_busy(udev);
We already have drivers supporting OTG, and obviously they know when
devices are added and removed. Why is a new notification mechanism
needed?
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -1179,6 +1181,10 @@ MODULE_LICENSE ("GPL");
> #ifdef CONFIG_PCI
> #include "ehci-pci.c"
> #define PCI_DRIVER ehci_pci_driver
> +#ifdef CONFIG_USB_LANGWELL_OTG
> +#include "ehci-langwell-pci.c"
> +#define LNW_OTG_HOST_DRIVER ehci_otg_driver
> +#endif
> #endif
Maybe I'm not looking at the correct branch, but there doesn't seem to
be any ehci-langwell-pci.c source file in this directory.
> #ifdef CONFIG_USB_EHCI_FSL
> @@ -1363,8 +1369,18 @@ static int __init ehci_hcd_init(void)
> if (retval < 0)
> goto clean4;
> #endif
> +
> +#ifdef LNW_OTG_HOST_DRIVER
> + retval = langwell_register_host(&LNW_OTG_HOST_DRIVER);
> + if (retval < 0)
> + goto clean5;
> +#endif
> return retval;
I'm a little confused about the point of this. Why is a special
registration for the USB host controller driver needed? Is it somehow
too drastically different from a normal PCI driver?
> --- a/include/linux/usb/hcd.h
> +++ b/include/linux/usb/hcd.h
> @@ -174,6 +174,12 @@ struct usb_hcd {
> * (ohci 32, uhci 1024, ehci 256/512/1024).
> */
>
> + /* some otg HCDs need this to get USB_DEVICE_ADD and USB_DEVICE_REMOVE
> + * from root hub, we do not want to use USB notification chain, since
> + * it would be a over kill to use high level notification.
> + */
> + void (*otg_notify) (struct usb_device *udev, unsigned action);
Why is it overkill to use the high-level notification mechanism? It's
already there and its notifications are going to be sent out
regardless. This is exactly the sort of thing it was meant for.
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
