On Thu, Jun 11, 2009 at 10:50:21PM +0800, Alek Du wrote:
> Changes from v1:
>
> Add a has_otg to ehci_hcd to do otg work only when has_otg is set.
> Thank Paulius to point that out.
>
>
>
> From f72772e1ea26be04232695737c1d0f1bfd0b355d Mon Sep 17 00:00:00 2001
> From: Alek Du <alek.du@xxxxxxxxx>
> Date: Wed, 10 Jun 2009 15:20:21 +0800
> Subject: [PATCH] EHCI: Support Intel Moorestown MPH and OTG host
>
> 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>
> ---
> drivers/usb/Kconfig | 4 +-
> drivers/usb/host/ehci-hcd.c | 22 ++++++
> drivers/usb/host/ehci-hub.c | 49 ++++++++++++
> drivers/usb/host/ehci-pci.c | 176 +++++++++++++++++++++++++++++++++++++++++++
> drivers/usb/host/ehci.h | 1 +
> 5 files changed, 250 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index c6c816b..df54917 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -101,6 +101,8 @@ source "drivers/usb/mon/Kconfig"
>
> source "drivers/usb/wusbcore/Kconfig"
>
> +source "drivers/usb/otg/Kconfig"
> +
> source "drivers/usb/host/Kconfig"
>
> source "drivers/usb/musb/Kconfig"
> @@ -151,6 +153,4 @@ source "drivers/usb/atm/Kconfig"
>
> source "drivers/usb/gadget/Kconfig"
>
> -source "drivers/usb/otg/Kconfig"
Why reorder things here? Are you sure this is going to not break
anything else that currently is working?
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index a546ce6..31122e0 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -44,6 +44,11 @@
> #include <asm/system.h>
> #include <asm/unaligned.h>
>
> +#ifdef CONFIG_USB_LANGWELL_OTG
> +#include <linux/usb/otg.h>
> +#include <linux/usb/langwell_otg.h>
> +#endif
Don't use ifdefs in .c code, just always include the header files
please.
> /*-------------------------------------------------------------------------*/
>
> /*
> @@ -1046,6 +1051,9 @@ MODULE_LICENSE ("GPL");
> #ifdef CONFIG_PCI
> #include "ehci-pci.c"
> #define PCI_DRIVER ehci_pci_driver
> +#ifdef CONFIG_USB_LANGWELL_OTG
> +#define LNW_OTG_HOST_DRIVER ehci_otg_driver
> +#endif
Why is this ifdef and define needed?
> #endif
>
> #ifdef CONFIG_USB_EHCI_FSL
> @@ -1133,8 +1141,19 @@ static int __init ehci_hcd_init(void)
> if (retval < 0)
> goto clean3;
> #endif
> +
> +#ifdef LNW_OTG_HOST_DRIVER
> + retval = langwell_register_host(&LNW_OTG_HOST_DRIVER);
> + if (retval < 0)
> + goto clean4;
> +#endif
No #ifdef in .c code please, you should handle this by making
langwell_register_host() a noop if you don't have the config option
enabled.
And what if you do have it enabled, what will happen if you don't have
the hardware?
> return retval;
>
> +#ifdef LNW_OTG_HOST_DRIVER
> +clean4:
> + langwell_unregister_host(&LNW_OTG_HOST_DRIVER);
> +#endif
Same as above. And for the rest of this patch. Please fix it up.
thanks,
greg k-h
--
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
