On Monday, September 12, 2011 07:20:40 PM Jana Rapava wrote:
> /board/efikamx/efikamx-usb.c: cleanup
> 
> Signed-off-by: Jana Rapava <ferma...@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, &regs->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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to