On Mon, 13 Jul 2009, Alek Du wrote:
> From 53ab3cbd332e22eee90a3f73de1bc5d865fd83d1 Mon Sep 17 00:00:00 2001
> From: Alek Du <alek.du@xxxxxxxxx>
> Date: Wed, 10 Jun 2009 14:38:15 +0800
> Subject: [PATCH] EHCI: Add Intel Moorestown EHCI controller HOSTPCx extensions and support phy low power mode
>
> The Intel Moorestown EHCI controller supports non-standard HOSTPCx register
> extension. This register controls the LPM behaviour and controls the behaviour
> of each USB port.
I just noticed this:
> index eab8499..0d77974 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -249,6 +249,12 @@ static int ehci_reset (struct ehci_hcd *ehci)
> retval = handshake (ehci, &ehci->regs->command,
> CMD_RESET, 0, 250 * 1000);
>
> + if (ehci->has_hostpc) {
> + ehci_writel(ehci, USBMODE_EX_HC | USBMODE_EX_VBPS,
> + (u32 __iomem *)(((u8 *)ehci->regs) + USBMODE_EX));
> + ehci_writel(ehci, TXFIFO_DEFAULT,
> + (u32 __iomem *)(((u8 *)ehci->regs) + TXFILLTUNING));
> + }
It seems odd to cast between u8 * and u32 * this way. Wouldn't it be
better if you could simply write:
ehci_writel(ehci, USBMODE_EX_HC | USBMODE_EX_VBPS,
ehci->regs + USBMODE_EX);
All that's needed is a simple change to the definitions of the register
offsets:
> --- a/include/linux/usb/ehci_def.h
> +++ b/include/linux/usb/ehci_def.h
> @@ -132,6 +132,19 @@ struct ehci_regs {
> #define USBMODE_CM_HC (3<<0) /* host controller mode */
> #define USBMODE_CM_IDLE (0<<0) /* idle state */
>
> +/* Moorestown has some non-standard registers, partially due to the fact that
> + * its EHCI controller has both TT and LPM support. HOSTPCx are extentions to
> + * PORTSCx
> + */
> +#define HOSTPC0 0x84 /* HOSTPC extension */
...
> +#define USBMODE_EX 0xc8 /* USB Device mode extension */
...
> +#define TXFILLTUNING 0x24 /* TX FIFO Tuning register */
Change these to (0x84/4), (0xc8/4), and (0x24/4) respectively.
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
