On Wed, 4 Aug 2010, 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 in another
> patch. This patch depends on the OTG transceive and OTG client patch from Hang
> Yuan that should be accepted already.
>
> Signed-off-by: Jacob Pan <jacob.jun.pan@xxxxxxxxx>
> Signed-off-by: Alek Du <alek.du@xxxxxxxxx>
> Signed-off-by: Alan Cox <alan@xxxxxxxxxxxxxxx>
> diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
> index cd88220..1227650 100644
> --- a/drivers/usb/core/usb.h
> +++ b/drivers/usb/core/usb.h
> @@ -147,4 +147,3 @@ extern void usb_notify_add_device(struct usb_device *udev);
> extern void usb_notify_remove_device(struct usb_device *udev);
> extern void usb_notify_add_bus(struct usb_bus *ubus);
> extern void usb_notify_remove_bus(struct usb_bus *ubus);
> -
Hmm. This isn't a particularly productive change. (Not that there's
anything actually _wrong_ with it...)
> diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
> index 796ea0c..90773c4 100644
> --- a/drivers/usb/host/ehci-hub.c
> +++ b/drivers/usb/host/ehci-hub.c
> @@ -178,6 +178,8 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
> int port;
> int mask;
> int changed;
> + u32 __iomem *hostpc_reg = NULL;
> + int rc = 0;
>
> ehci_dbg(ehci, "suspend root hub\n");
>
> @@ -297,13 +299,18 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
> ehci_readl(ehci, &ehci->regs->intr_enable);
>
> ehci->next_statechange = jiffies + msecs_to_jiffies(10);
> +
> +#ifdef CONFIG_USB_OTG
> + if (ehci->has_otg && ehci->otg_suspend)
> + rc = ehci->otg_suspend(hcd);
> +#endif
> spin_unlock_irq (&ehci->lock);
>
> /* ehci_work() may have re-enabled the watchdog timer, which we do not
> * want, and so we must delete any pending watchdog timer events.
> */
> del_timer_sync(&ehci->watchdog);
> - return 0;
> + return rc;
> }
Don't you get a compiler warning about hostpc_reg not being used?
> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> index e5b9ece..74fb60b 100644
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -143,6 +143,13 @@ struct ehci_hcd { /* one per controller */
> unsigned has_hostpc:1;
> unsigned has_lpm:1; /* support link power management */
> unsigned has_ppcd:1; /* support per-port change bits */
> +
> +#ifdef CONFIG_USB_OTG
> + unsigned has_otg:1; /* if it is otg host*/
> + /* otg host has additional bus_suspend and bus_resume */
> + int (*otg_suspend)(struct usb_hcd *hcd);
> + int (*otg_resume)(struct usb_hcd *hcd);
> +#endif
> u8 sbrn; /* packed release number */
Here and elsewhere, can we get rid of the "#ifdef CONFIG_USB_OTG" tests
all over the place? The additional overhead for the non-OTG case isn't
terribly big.
And when that's done, maybe the new function pointers can be moved
somewhere else in the structure (near some other pointers instead of
among the bitfields).
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
