[U-Boot] ppc440epx(sequoia) USB ehci fix.

[Stefan Roese]

Hi Chris,

On Tuesday 05 January 2010 23:04:35 Chris Zhang wrote:
> This patch makes sequoia board USB ehci working.

Thanks, that's good news.
 
> The problem seems to be when doing port reset (see ehci-hcd.c), the reset
> is not terminated. EHCI spec says "A host controller must terminate the
> reset and stabilize the state of the port within 2 milliseconds".
> 
> This is only tested on Sequoia board (with USB mass storage devices).

It would be better, if you could send a "git-style" patch instead. Best 
generated using "git format-patch", with a proper commit message and a Signed-
off by line.

Please find some more comments below.
 
> cheers,
> Chris Zhang
> 
> diff --git a/drivers/usb/host/ehci-ppc4xx.c
> b/drivers/usb/host/ehci-ppc4xx.c
> new file mode 100644
> index 0000000..9a23509
> --- /dev/null
> +++ b/drivers/usb/host/ehci-ppc4xx.c
> @@ -0,0 +1,29 @@
> +/* Code derived from ehci-fsl.c */

Please use a proper comment header with Copyright note instead.

> +#include <common.h>
> +#include <usb.h>
> +
> +#include "ehci.h"
> +#include "ehci-core.h"
> +
> +/*
> + * Create the appropriate control structures to manage
> + * a new EHCI host controller.
> + */
> +int ehci_hcd_init(void)
> +{
> +       hccr = (struct ehci_hccr *)(0xe0000300);

Please don't use a magic number here. Better define this in the board config 
header and use the macro here. This way it's better portable to other 4xx 
platforms too.

> +       hcor = (struct ehci_hcor *)((uint32_t) hccr +
> +               HC_LENGTH(ehci_readl(&hccr->cr_capbase)));
> +       usb_dev_init();
> +       return 0;
> +}
> +
> +/*
> + * Destroy the appropriate control structures corresponding
> + * the the EHCI host controller.
> + */
> +int ehci_hcd_stop(void)
> +{
> +       return 0;
> +}
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index 940d4a8..255679a 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -36,6 +36,7 @@ COBJS-$(CONFIG_USB_SL811HS) += sl811-hcd.o
>  # echi
>  COBJS-$(CONFIG_USB_EHCI) += ehci-hcd.o
>  COBJS-$(CONFIG_USB_EHCI_FSL) += ehci-fsl.o
> +COBJS-$(CONFIG_USB_EHCI_PPC4XX) += ehci-ppc4xx.o
>  COBJS-$(CONFIG_USB_EHCI_IXP4XX) += ehci-ixp.o
>  COBJS-$(CONFIG_USB_EHCI_KIRKWOOD) += ehci-kirkwood.o
>  COBJS-$(CONFIG_USB_EHCI_PCI) += ehci-pci.o
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index ba85991..fabd68a 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -708,6 +708,9 @@ ehci_submit_root(struct usb_device *dev, unsigned long
> pipe,
>  void *buffer,
>                                  * root
>                                  */
>                                 wait_ms(50);
> +                               /* clear port reset */
> +                               ehci_writel(status_reg, reg &
> ~EHCI_PS_PR);
> +                               wait_ms(2);

Your mailer has line-wrapped this patch. Please take care next time that this 
doesn't happen. Best use "git send-email".

And also it would be better, if you would send this change/fix to "ehci-hcd.c" 
in a separate patch, with it's own commit text and Signed-off-by.

>                                 portreset |= 1 << le16_to_cpu(req->index);
>                         }
>                         break;
> diff --git a/include/configs/sequoia.h b/include/configs/sequoia.h
> index 5788d58..e31e3f3 100644
> --- a/include/configs/sequoia.h
> +++ b/include/configs/sequoia.h
> @@ -282,8 +282,8 @@
> 
>  /* USB */
>  #ifdef CONFIG_440EPX
> +#if 0  /* Enable this for OHCI */
>  #define CONFIG_USB_OHCI_NEW
> -#define CONFIG_USB_STORAGE
>  #define CONFIG_SYS_OHCI_BE_CONTROLLER
> 
>  #undef CONFIG_SYS_USB_OHCI_BOARD_INIT
> @@ -291,7 +291,16 @@
>  #define CONFIG_SYS_USB_OHCI_REGS_BASE  CONFIG_SYS_USB_HOST
>  #define CONFIG_SYS_USB_OHCI_SLOT_NAME  "ppc440"
>  #define CONFIG_SYS_USB_OHCI_MAX_ROOT_PORTS 15
> +#else  /* EHCI */
> +
> +#define CONFIG_USB_EHCI
> +#define CONFIG_USB_EHCI_PPC4XX
> +#define CONFIG_EHCI_HCD_INIT_AFTER_RESET
> +#define CONFIG_EHCI_MMIO_BIG_ENDIAN
> +#define CONFIG_EHCI_DESC_BIG_ENDIAN
> +#endif
> 
> +#define CONFIG_USB_STORAGE
>  /* Comment this out to enable USB 1.1 device */
>  #define USB_2_0_DEVICE
> 
> @@ -500,4 +509,10 @@
>  #define CONFIG_CMD_BMP
>  #endif
> 
> +/*
> + * Enable this only when you want EHCI.
> + */
> +#if 1
> +#endif
> +

Please remove this.

Thanks.

Cheers,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de