Dear Otavio Salvador,
The i.MX23 just one USB port so we shouldn't mess up with PLL1CTRL and USB1 port when building for i.MX23.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br
Changes in v4:
- Rework soc_ehci_hcd_{enable,disable}_clock to mxs_ehci_hcd_clock (Fabio /
Marek)
Changes in v3:
- Improve commit log
- Move code to enable/disable clock to soc_ehci_hcd_{enable,disable}_clock
- Proper use mx23 clock registers
Changes in v2:
- Avoid wrong clock setting in MX23
drivers/usb/host/ehci-mxs.c | 58 +++++++++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/host/ehci-mxs.c b/drivers/usb/host/ehci-mxs.c index 5062af5..45333ce 100644 --- a/drivers/usb/host/ehci-mxs.c +++ b/drivers/usb/host/ehci-mxs.c @@ -23,7 +23,11 @@ #include <asm/io.h> #include <asm/arch/regs-common.h> #include <asm/arch/regs-base.h> +#if defined(CONFIG_MX23) +#include <asm/arch/regs-clkctrl-mx23.h> +#elif defined(CONFIG_MX28) #include <asm/arch/regs-clkctrl-mx28.h> +#endif
This should be handled automatically in imx-regs.h no ? I believe all this regs- xxx.h crap should just be part of imx-regs.h and none of that should be here at all.
#include <asm/arch/regs-usb.h> #include <asm/arch/regs-usbphy.h>
@@ -50,10 +54,12 @@ int mxs_ehci_get_port(struct ehci_mxs *mxs_usb, int port) usb_base = MXS_USBCTRL0_BASE; phy_base = MXS_USBPHY0_BASE; break; +#ifdef CONFIG_MX28 case 1: usb_base = MXS_USBCTRL1_BASE; phy_base = MXS_USBPHY1_BASE; break; +#endif default: printf("CONFIG_EHCI_MXS_PORT (port = %d)\n", port); return -1; @@ -67,17 +73,40 @@ int mxs_ehci_get_port(struct ehci_mxs *mxs_usb, int port) /* This DIGCTL register ungates clock to USB */ #define HW_DIGCTL_CTRL 0x8001c000 #define HW_DIGCTL_CTRL_USB0_CLKGATE (1 << 2) +#ifdef CONFIG_MX28 #define HW_DIGCTL_CTRL_USB1_CLKGATE (1 << 16) +#endif
-int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor) +static void mxs_ehci_hcd_clock(bool enable)
I wonder if this shall not go to arch/arm.../mxs/clock.c ?
I'd say, pass also "int index" argument and call it twice for mx28 and once for mx23. That'd align well with the multi-controller stuff.
{
- int ret;
- uint32_t usb_base, cap_base; struct mxs_register_32 *digctl_ctrl = (struct mxs_register_32 *)HW_DIGCTL_CTRL; struct mxs_clkctrl_regs *clkctrl_regs = (struct mxs_clkctrl_regs *)MXS_CLKCTRL_BASE;
- uint32_t reg = HW_DIGCTL_CTRL_USB0_CLKGATE;
- writel(CLKCTRL_PLL0CTRL0_EN_USB_CLKS | CLKCTRL_PLL0CTRL0_POWER,
(enable ? &clkctrl_regs->hw_clkctrl_pll0ctrl0_set : \&clkctrl_regs->hw_clkctrl_pll0ctrl0_clr));+#ifdef CONFIG_MX28
- /* i.MX28 has two USB controllers */
- reg |= HW_DIGCTL_CTRL_USB1_CLKGATE;
- writel(CLKCTRL_PLL1CTRL0_EN_USB_CLKS | CLKCTRL_PLL1CTRL0_POWER,
(enable ? &clkctrl_regs->hw_clkctrl_pll1ctrl0_set : \&clkctrl_regs->hw_clkctrl_pll1ctrl0_clr));+#endif
- /* Gate/gateoff the USB clock */
- writel(reg, (enable ? &digctl_ctrl->reg_clr : \
&digctl_ctrl->reg_set));
Uh ... uh ... uh ...
No, kill the ternary operators. Good old if (enable) .... else .... please.
Something like (imprecise code warning):
if (enable) reg_offset = offsetof(mxs_register32, set); else reg_offset = offsetof(mxs_register32, clk);
do_all_the_stuff here, all writel(val, mxs_register32 + offset); like this example.
+}
+int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor) +{
int ret;
uint32_t usb_base, cap_base;
ret = mxs_ehci_get_port(&ehci_mxs, CONFIG_EHCI_MXS_PORT); if (ret)
@@ -90,13 +119,7 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor) &ehci_mxs.phy_regs->hw_usbphy_ctrl_clr);
/* Enable USB clock */
- writel(CLKCTRL_PLL0CTRL0_EN_USB_CLKS | CLKCTRL_PLL0CTRL0_POWER,
&clkctrl_regs->hw_clkctrl_pll0ctrl0_set);- writel(CLKCTRL_PLL1CTRL0_EN_USB_CLKS | CLKCTRL_PLL1CTRL0_POWER,
&clkctrl_regs->hw_clkctrl_pll1ctrl0_set);- writel(HW_DIGCTL_CTRL_USB0_CLKGATE | HW_DIGCTL_CTRL_USB1_CLKGATE,
&digctl_ctrl->reg_clr);
mxs_ehci_hcd_clock(true);
/* Start USB PHY */ writel(0, &ehci_mxs.phy_regs->hw_usbphy_pwd);
@@ -118,10 +141,6 @@ int ehci_hcd_stop(int index) { int ret; uint32_t usb_base, cap_base, tmp;
- struct mxs_register_32 *digctl_ctrl =
(struct mxs_register_32 *)HW_DIGCTL_CTRL;- struct mxs_clkctrl_regs *clkctrl_regs =
(struct mxs_clkctrl_regs *)MXS_CLKCTRL_BASE;@@ -147,14 +166,7 @@ int ehci_hcd_stop(int index) writel(tmp, &ehci_mxs.phy_regs->hw_usbphy_pwd);
/* Disable USB clock */
- writel(CLKCTRL_PLL0CTRL0_EN_USB_CLKS,
&clkctrl_regs->hw_clkctrl_pll0ctrl0_clr);- writel(CLKCTRL_PLL1CTRL0_EN_USB_CLKS,
&clkctrl_regs->hw_clkctrl_pll1ctrl0_clr);- /* Gate off the USB clock */
- writel(HW_DIGCTL_CTRL_USB0_CLKGATE | HW_DIGCTL_CTRL_USB1_CLKGATE,
&digctl_ctrl->reg_set);
mxs_ehci_hcd_clock(false);
return 0;
}
You might want to separate out the USB patches, they might need some work.
Best regards, Marek Vasut