[U-Boot] [PATCH 1/2 v2] USB cleanup for EfikaMX
Marek Vasut
marek.vasut at gmail.com
Mon Sep 12 21:25:30 CEST 2011
On Monday, September 12, 2011 07:20:40 PM Jana Rapava wrote:
> /board/efikamx/efikamx-usb.c: cleanup
>
> Signed-off-by: Jana Rapava <fermata7 at gmail.com>
> ---
> board/efikamx/efikamx-usb.c | 111
> +++++++++++++++++++++++-------------------- include/usb/ehci-fsl.h |
> 49 +++++++++++++++++++
> 2 files changed, 108 insertions(+), 52 deletions(-)
Dear Jana Rapava,
1) PATCH 2/2 is missing.
2) This doesn't apply on mainline, continue as discussed on IRC -- ie. by
merging the patches.
>
> diff --git a/board/efikamx/efikamx-usb.c b/board/efikamx/efikamx-usb.c
> index 70d0daa..a94a64f 100644
> --- a/board/efikamx/efikamx-usb.c
> +++ b/board/efikamx/efikamx-usb.c
> @@ -161,57 +161,70 @@ void control_regs_setup(void)
> {
[...]
> + tmp = readl(OTG_BASE_ADDR + MX51_USB_CTRL_OFFSET);
> + tmp &= ~(MX51_OTG_WUE_BIT | MX51_OTG_PM_BIT | MX51_H1_ULPI_IE_BIT |
> MX51_H1_WUE_BIT); + tmp |= MX51_H1_PM_BIT;
I assume this is ULPI specific, not MX51 specific.
> + writel(tmp, OTG_BASE_ADDR + MX51_USB_CTRL_OFFSET);
Don't use offset-based access, but struct based access, please fix globally.
struct otg_regs {
uint32_t reg1;
uint32_t reg2;
...
};
then
struct otg_regs *regs = (struct otg_regs *)REG_ADDR;
writel(val, ®s->reg1);
[...]
>
> -#define ULPI_VIEWPORT(base) (base + 0x170)
> +
> +#define ULPI_VIEWPORT(base) (base + 0x170)
struct ehci probably already contains the correct offset of the ulpi viewport.
You just need to cast struct ehci to correct offset of the port.
> +
> +/* ULPI viewport control bits */
> +#define MX51_ULPI_WU_BIT (1 << 31)
> +#define MX51_ULPI_SS_BIT (1 << 27)
> +#define MX51_ULPI_RWRUN (1 << 30)
> +#define MX51_ULPI_RWCTRL (1 << 29)
See above.
> +
> +#define shift_to_ulpiaddr(reg) (reg << 16)
#define ULPI_something_ADDR_OFFSET 16 ?
> +#define ulpi_write_mask(value) (value & 0xff)
> +#define ulpi_read_mask(value) ((value >> 8) & 0xff)
Missing parenthesis around value.
> +
>
> void ulpi_write(u32 base, u32 reg, u32 value)
> {
> - if (!(readl(ULPI_VIEWPORT(base)) & (1 << 27))) {
> - writel(1 << 31, ULPI_VIEWPORT(base));
> - while (readl(ULPI_VIEWPORT(base)) & (1 << 31));
> + if (!(readl(ULPI_VIEWPORT(base)) & MX51_ULPI_SS_BIT)) {
See above.
> + writel(MX51_ULPI_WU_BIT, ULPI_VIEWPORT(base));
> + while (readl(ULPI_VIEWPORT(base)) & MX51_ULPI_WU_BIT);
Semicolon should be on the new line. Please fix globally.
> }
> - writel((1 << 30) | (1 << 29) | (reg << 16) | (value & 0xff),
> ULPI_VIEWPORT(base)); - while (readl(ULPI_VIEWPORT(base)) & (1 << 30));
> + writel(MX51_ULPI_RWRUN | MX51_ULPI_RWCTRL | shift_to_ulpiaddr(reg) |
> ulpi_write_mask(value), ULPI_VIEWPORT(base)); +
> + while (readl(ULPI_VIEWPORT(base)) & MX51_ULPI_RWRUN);
> }
>
> u32 ulpi_read(u32 base, u32 reg)
> {
> u32 tmp;
> - if (!(readl(ULPI_VIEWPORT(base)) & (1 << 27))) {
> - writel(1 << 31, ULPI_VIEWPORT(base));
> - while (readl(ULPI_VIEWPORT(base)) & (1 << 31));
> + if (!(readl(ULPI_VIEWPORT(base)) & MX51_ULPI_SS_BIT)) {
> + writel(MX51_ULPI_WU_BIT, ULPI_VIEWPORT(base));
> + while (readl(ULPI_VIEWPORT(base)) & MX51_ULPI_WU_BIT);
> }
> - writel((1 << 30) | (reg << 16), ULPI_VIEWPORT(base));
> + writel(MX51_ULPI_RWRUN | shift_to_ulpiaddr(reg), ULPI_VIEWPORT(base));
> do {
> tmp = readl(ULPI_VIEWPORT(base));
> - } while (tmp & (1 << 30));
> - return (tmp >> 8) & 0xff;
> + } while (tmp & MX51_ULPI_RWRUN);
> + return ulpi_read_mask(tmp);
> }
>
> #define ULPI_CTL_WRITE_OFF 0x00
> @@ -229,8 +242,9 @@ void ehciX_init(u32 base)
> int reg, i;
>
> /* ULPI INIT */
> - for (reg = 3; reg >=0; reg--)
> + for (reg = MX51_USB_HOST_COUNT-1; reg >= 0; reg--)
COUNT-1 missing spaces.
> tmp |= ulpi_read(base, reg) << (reg * 8);
> + /* split ID into first and second half */
debug() ?
What's "first" and "second" half, do you mean vendor and product ID ?
> printf("Found ULPI TX, ID %04x:%04x\n", tmp >> 16, tmp & 0xffff);
>
> /* ULPI check integrity */
> @@ -238,7 +252,6 @@ void ehciX_init(u32 base)
> ulpi_write(base, ULPI_SCRATCH, 0x55 << i);
> tmp = ulpi_read(base, ULPI_SCRATCH);
>
> - // check
> if (tmp != (0x55 << i)) {
> printf("ULPI integrity check failed\n");
> return;
> @@ -264,8 +277,7 @@ void ehciX_init(u32 base)
>
> void ehci0_init(void)
> {
> - /* Set PORTSC to 16b UTMI mode */
> - writel((1 << 28), OTG_BASE_ADDR + 0x184);
> + writel(MX51_16BIT_UTMI_BIT, OTG_BASE_ADDR + MX51_OTG_PORTSC1_OFFSET);
See above.
> }
>
> void ehci1_init(void)
> @@ -284,24 +296,21 @@ void ehci1_init(void)
> mxc_iomux_set_pad(MX51_PIN_USBH1_STP, USB_PAD_CONFIG);
> udelay(10000);
>
> - tmp = readl(OTG_BASE_ADDR + 0x340);
> - tmp &= ~(0xff << 16);
> - writel(tmp, OTG_BASE_ADDR + 0x340);
> + tmp = readl(OTG_BASE_ADDR + MX51_UH1_USBCMD_OFFSET);
> + tmp &= MX51_ITC_IMMEDIATE_MASK;
> + writel(tmp, OTG_BASE_ADDR + MX51_UH1_USBCMD_OFFSET);
Try clrsetbits_le32() and other calls from asm/io.h
>
> udelay(10000);
>
> - /* Set PORTSC to ULPI mode */
> - writel((2 << 30), OTG_BASE_ADDR + 0x384);
> + writel(MX51_ULPI_MODE_MASK, OTG_BASE_ADDR + MX51_UH1_PORTSC1_OFFSET);
> udelay(10000);
>
> - ehciX_init(OTG_BASE_ADDR + 0x200);
> + ehciX_init(OTG_BASE_ADDR + MX51_UH1_ID_OFFSET);
> }
>
>
> void ehci2_init(void)
> {
> - u32 tmp;
> -
> mxc_request_iomux(MX51_PIN_EIM_A26, IOMUX_CONFIG_ALT1);
> mxc_iomux_set_pad(MX51_PIN_EIM_A26, PAD_CTL_DRV_HIGH |
> PAD_CTL_PKE_ENABLE | PAD_CTL_SRE_FAST);
> @@ -313,11 +322,10 @@ void ehci2_init(void)
> mxc_request_iomux(MX51_PIN_EIM_A26, IOMUX_CONFIG_ALT2);
> mxc_iomux_set_pad(MX51_PIN_EIM_A26, USB_PAD_CONFIG);
>
> - /* Set PORTSC to ULPI mode */
> - writel((2 << 30), OTG_BASE_ADDR + 0x584);
> + writel(MX51_ULPI_MODE_MASK, OTG_BASE_ADDR + MX51_UH2_PORTSC1_OFFSET);
I think struct ehci defines this offset.
> udelay(10000);
>
> - ehciX_init(OTG_BASE_ADDR + 0x400);
> + ehciX_init(OTG_BASE_ADDR + MX51_UH2_ID_OFFSET);
> }
>
> int ehci_hcd_init(void)
> @@ -338,10 +346,9 @@ int ehci_hcd_init(void)
> /* EfikaMX USB has issues ... */
> udelay(10000);
>
> -
> /* Init EHCI core */
> ehci = (struct usb_ehci *)(OTG_BASE_ADDR +
> - (0x200 * CONFIG_MXC_USB_PORT));
> + (MX51_REGISTER_LAYOUT_LENGTH * CONFIG_MXC_USB_PORT));
> hccr = (struct ehci_hccr *)((uint32_t)&ehci->caplength);
> hcor = (struct ehci_hcor *)((uint32_t) hccr +
> HC_LENGTH(ehci_readl(&hccr->cr_capbase)));
> diff --git a/include/usb/ehci-fsl.h b/include/usb/ehci-fsl.h
> index 67600ed..5617275 100644
> --- a/include/usb/ehci-fsl.h
> +++ b/include/usb/ehci-fsl.h
> @@ -169,6 +169,55 @@
> #define CONFIG_SYS_FSL_USB_ADDR CONFIG_SYS_MPC512x_USB_ADDR
> #endif
>
> +#define MX51_USB_HOST_COUNT 4
> +#define MX51_REGISTER_LAYOUT_LENGTH 0x200
> +
> +/* Register offsets for MX51 */
> +#define MX51_UH1_ID_OFFSET 0x200
> +#define MX51_UH2_ID_OFFSET 0x400
> +
> +#define MX51_USB_CTRL_OFFSET 0x800
> +#define MX51_PHY_CTRL0_OFFSET 0x808
> +#define MX51_PHY_CTRL1_OFFSET 0x80c
> +#define MX51_USB_CTRL1_OFFSET 0x810
> +#define MX51_UH2_CTRL_OFFSET 0x814
MX5_, not MX51, I think this is valid for MX53 too.
> +
> +#define MX51_OTG_PORTSC1_OFFSET 0x184
> +#define MX51_UH1_PORTSC1_OFFSET 0x384
> +#define MX51_UH2_PORTSC1_OFFSET 0x584
> +
> +#define MX51_UH1_USBCMD_OFFSET 0x340
Use struct ehci where this is used with proper cast.
> +
> +/* USB_CTRL register bits of interest*/
> +#define MX51_OTG_WUE_BIT (1 << 27)
> +#define MX51_OTG_PM_BIT (1 << 24)
> +#define MX51_H1_ULPI_IE_BIT (1 << 12)
> +#define MX51_H1_WUE_BIT (1 << 11)
> +#define MX51_H1_PM_BIT (1 << 8)
> +
> +/* PHY_CTRL_0 register bits of interest */
> +#define MX51_OTG_OVERCURD_BIT (1 << 8)
> +#define MX51_EHCI_POWERPINSE_BIT (1 << 5)
> +
> +/* PHY_CTRL_1 register bits of interest */
> +#define MX51_SYSCLOCK_24_MHZ_BIT (1 << 0)
> +#define MX51_SYSCLOCK_MASK (0xffffffff << 0)
don't use << 0 here. Is this even needed?
> +
> +/* USB_CTRL_1 register bits of interest */
> +#define MX51_H1_EXTCLKE_BIT (1 << 25)
> +
> +/* USB Host 2 CTRL register bits of interest */
> +#define MX51_H2_ULPI_IE_BIT (1 << 8)
> +#define MX51_H2_WUE_BIT (1 << 7)
> +#define MX51_H2_PM_BIT (1 << 4)
> +
> +/* PORTSCx bits of interest */
> +#define MX51_ULPI_MODE_MASK (2 << 30)
> +#define MX51_16BIT_UTMI_BIT (1 << 28)
> +
> +/* USBCMD bits of interest */
> +#define MX51_ITC_IMMEDIATE_MASK (~(0xff << 16))
> +
> /*
> * USB Registers
> */
Cheers
More information about the U-Boot
mailing list