[U-Boot] [PATCH] Added preliminary support to EHCI USB FSL controller for platform MPC5121

[Wolfgang Denk]

Dear "Rendine Francesco",

In message <19AFD08D18580F478430CE6278E20225211784 at MSRV-BE20.valueteam.com> you wrote:
>
> I would like to submit you a patch to add a preliminary support to EHCI
> USB Freescale controller integrated on MPC5121.

I apologize for the long delay.

> There is still much job to do..for now, this patch add support for USB0
> on UTMI phy. 
> I've personally tested this feature with mass storage and it does work.
> 
> Could be a base for future development..I'm available to possible
> improvement of source code with your suggestions.
> 
> Summary:
> - introduced ehci_hcd_init and ehci_hcd_stop functions for MPC5121
> integrated EHCI USB controller
> - introduced enable of EHCI USB controller in include/mpc5121ads.h
> - introduced many defines and struct ehci fields to be able to provvide
> OTG feature

Can you please 1) rebase this patch against current code, 2) re-submit
it with a commit message that can be used without further editing, and
3) clean up the issues marked below?  Thanks in advance.

> Signed-off-by: Francesco Rendine <francesco.rendine at valueteam.com> 
> 
> --- a/drivers/usb/host/ehci-fsl.c
> +++ b/drivers/usb/host/ehci-fsl.c
...
> @@ -38,34 +42,93 @@
>   */
>  int ehci_hcd_init(void)
>  {
> -	struct usb_ehci *ehci;
> +        struct usb_ehci *ehci;

Please always use TABs for indentation. (Fix globally!)

> -	/* Set to Host mode */
> -	setbits_le32((void *)ehci->usbmode, CM_HOST);
> +        /* Set to Host mode */
> +        setbits_le32((void *)ehci->usbmode, CM_HOST);

Especially don't mess with code you're not even changing at all.

...
> +int ehci_hcd_init(void)
> +{
...
> +	/* Confgure interface for UTMI_WIDE */
> +        temp = in_be32(&ehci->isiphyctrl);
> +        out_be32(&ehci->isiphyctrl, temp | PHYCTRL_PHYE | PHYCTRL_PXE);

Use setbits_be16() instead.

> +        temp = in_be32(&ehci->usbgenctrl);
> +        out_be32(&ehci->usbgenctrl, temp | GC_PPP | GC_PFP );

Ditto.

> +        /* Workaround: disable ULPI phy */
> +        temp = 0x60000000 | 0x0C << 16 | 0x20 ;
> +        temp = cpu_to_hc32(temp);
> +        out_be32(&ehci->ulpi_viewpoint, temp);
> +
> +	/* Configure interrupt */
> +	temp = ehci_readl(&hcor->or_usbintr);
> +	temp |= (INTR_UE | INTR_UEE | INTR_PCE | INTR_SEE | INTR_AAE);
> +	ehci_writel(&hcor->or_usbintr, temp);

Ditto.

> +	/* Init phy USB0 to UTMI+ */
> +	temp = ehci_readl(&hcor->or_portsc[0]);
> +	temp &= ~(PORT_PTS_MSK | PORT_PTS_PTW);
> +	temp |= (PORT_PTS_UTMI | PORT_PTS_PTW);
> +	ehci_writel(&hcor->or_portsc[0], temp);

Use clrsetbits_le32() instead. (ehci_writel is LE, isn't it?)

> @@ -75,5 +138,16 @@ int ehci_hcd_init(void)
>   */
>  int ehci_hcd_stop(void)
>  {
> +	u32 temp;
> +
> +	/* Disable phy USB0 */
> +	temp = ehci_readl(&hcor->or_portsc[0]);
> +	temp &= ~(PORT_PTS_MSK | PORT_PTS_PTW | PORT_PP | PORT_PR);
> +	ehci_writel(&hcor->or_portsc[0], temp);

Use clrbits_le32() instead.

> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -71,6 +71,11 @@ struct ehci_hcor {
>  #define	STD_ASS		(1 << 15)
>  #define STS_HALT	(1 << 12)
>  	uint32_t or_usbintr;
> +#define INTR_UE         (1 << 0)                /* USB interrupt enable

Add blank line before that.

> */
> +#define INTR_UEE        (1 << 1)                /* USB error interrupt
> enable */
> +#define INTR_PCE        (1 << 2)                /* Port change detect
> enable */
> +#define INTR_SEE        (1 << 4)                /* system error enable
> */

Fix your mailer, it line-wrapped the patch.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Harrisberger's Fourth Law of the Lab:
	Experience is directly proportional to the
	amount of equipment ruined.