| Message ID | 1360589756-26355-8-git-send-email-otavio@ossystems.com.br |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
Dear Otavio Salvador, > The i.MX23 just one USB port so disable the second controller probe > when building for i.MX23. > > Signed-off-by: Otavio Salvador <otavio@ossystems.com.br> > --- > Changes in v2: > - Avoid wrong clock setting in MX23 > > drivers/usb/host/ehci-mxs.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/usb/host/ehci-mxs.c b/drivers/usb/host/ehci-mxs.c > index 5062af5..b7bf856 100644 > --- a/drivers/usb/host/ehci-mxs.c > +++ b/drivers/usb/host/ehci-mxs.c > @@ -50,10 +50,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,7 +69,9 @@ 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) { > @@ -95,8 +99,12 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr, > struct ehci_hcor **hcor) writel(CLKCTRL_PLL1CTRL0_EN_USB_CLKS | > CLKCTRL_PLL1CTRL0_POWER, > &clkctrl_regs->hw_clkctrl_pll1ctrl0_set); > > +#if defined(CONFIG_MX23) > + writel(HW_DIGCTL_CTRL_USB0_CLKGATE, &digctl_ctrl->reg_clr); What is that large space before '&digctl_regs->...' ? > +#elif defined(CONFIG_MX28) > writel(HW_DIGCTL_CTRL_USB0_CLKGATE | HW_DIGCTL_CTRL_USB1_CLKGATE, > &digctl_ctrl->reg_clr); > +#endif I'm sure these can be wrapped in much more elegant way, yes? > > /* Start USB PHY */ > writel(0, &ehci_mxs.phy_regs->hw_usbphy_pwd); > @@ -153,8 +161,12 @@ int ehci_hcd_stop(int index) > &clkctrl_regs->hw_clkctrl_pll1ctrl0_clr); > > /* Gate off the USB clock */ > +#if defined(CONFIG_MX23) > + writel(HW_DIGCTL_CTRL_USB0_CLKGATE, &digctl_ctrl->reg_set); > +#elif defined(CONFIG_MX28) > writel(HW_DIGCTL_CTRL_USB0_CLKGATE | HW_DIGCTL_CTRL_USB1_CLKGATE, > &digctl_ctrl->reg_set); > +#endif > > return 0; > } Best regards, Marek Vasut
On Wed, Feb 13, 2013 at 7:15 PM, Marek Vasut <marex@denx.de> wrote: > Dear Otavio Salvador, > >> The i.MX23 just one USB port so disable the second controller probe >> when building for i.MX23. >> >> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br> >> --- >> Changes in v2: >> - Avoid wrong clock setting in MX23 >> >> drivers/usb/host/ehci-mxs.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/drivers/usb/host/ehci-mxs.c b/drivers/usb/host/ehci-mxs.c >> index 5062af5..b7bf856 100644 >> --- a/drivers/usb/host/ehci-mxs.c >> +++ b/drivers/usb/host/ehci-mxs.c >> @@ -50,10 +50,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,7 +69,9 @@ 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) { >> @@ -95,8 +99,12 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr, >> struct ehci_hcor **hcor) writel(CLKCTRL_PLL1CTRL0_EN_USB_CLKS | >> CLKCTRL_PLL1CTRL0_POWER, >> &clkctrl_regs->hw_clkctrl_pll1ctrl0_set); >> >> +#if defined(CONFIG_MX23) >> + writel(HW_DIGCTL_CTRL_USB0_CLKGATE, &digctl_ctrl->reg_clr); > > What is that large space before '&digctl_regs->...' ? My fault. I can fix it. >> +#elif defined(CONFIG_MX28) >> writel(HW_DIGCTL_CTRL_USB0_CLKGATE | HW_DIGCTL_CTRL_USB1_CLKGATE, >> &digctl_ctrl->reg_clr); >> +#endif > > I'm sure these can be wrapped in much more elegant way, yes? I wanted to keep the changes at minimum; so seems the right route. What you'd like me to do? Use a temporary variable? >> /* Start USB PHY */ >> writel(0, &ehci_mxs.phy_regs->hw_usbphy_pwd); >> @@ -153,8 +161,12 @@ int ehci_hcd_stop(int index) >> &clkctrl_regs->hw_clkctrl_pll1ctrl0_clr); >> >> /* Gate off the USB clock */ >> +#if defined(CONFIG_MX23) >> + writel(HW_DIGCTL_CTRL_USB0_CLKGATE, &digctl_ctrl->reg_set); >> +#elif defined(CONFIG_MX28) >> writel(HW_DIGCTL_CTRL_USB0_CLKGATE | HW_DIGCTL_CTRL_USB1_CLKGATE, >> &digctl_ctrl->reg_set); >> +#endif >> >> return 0; >> } > > Best regards, > Marek Vasut
Dear Otavio Salvador, > On Wed, Feb 13, 2013 at 7:15 PM, Marek Vasut <marex@denx.de> wrote: > > Dear Otavio Salvador, > > > >> The i.MX23 just one USB port so disable the second controller probe > >> when building for i.MX23. > >> > >> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br> > >> --- > >> Changes in v2: > >> - Avoid wrong clock setting in MX23 > >> > >> drivers/usb/host/ehci-mxs.c | 12 ++++++++++++ > >> 1 file changed, 12 insertions(+) > >> > >> diff --git a/drivers/usb/host/ehci-mxs.c b/drivers/usb/host/ehci-mxs.c > >> index 5062af5..b7bf856 100644 > >> --- a/drivers/usb/host/ehci-mxs.c > >> +++ b/drivers/usb/host/ehci-mxs.c > >> @@ -50,10 +50,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,7 +69,9 @@ 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) { > >> @@ -95,8 +99,12 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr, > >> struct ehci_hcor **hcor) writel(CLKCTRL_PLL1CTRL0_EN_USB_CLKS | > >> CLKCTRL_PLL1CTRL0_POWER, > >> > >> &clkctrl_regs->hw_clkctrl_pll1ctrl0_set); > >> > >> +#if defined(CONFIG_MX23) > >> + writel(HW_DIGCTL_CTRL_USB0_CLKGATE, &digctl_ctrl->reg_clr); > > > > What is that large space before '&digctl_regs->...' ? > > My fault. I can fix it. > > >> +#elif defined(CONFIG_MX28) > >> > >> writel(HW_DIGCTL_CTRL_USB0_CLKGATE | HW_DIGCTL_CTRL_USB1_CLKGATE, > >> > >> &digctl_ctrl->reg_clr); > >> > >> +#endif > > > > I'm sure these can be wrapped in much more elegant way, yes? > > I wanted to keep the changes at minimum; so seems the right route. > What you'd like me to do? Use a temporary variable? Either that or wrap it all in struct ehci_mxs and do these ungatings etc on a per-port basis.
On Wed, Feb 13, 2013 at 7:36 PM, Marek Vasut <marex@denx.de> wrote: > Dear Otavio Salvador, > >> On Wed, Feb 13, 2013 at 7:15 PM, Marek Vasut <marex@denx.de> wrote: >> > Dear Otavio Salvador, >> > >> >> The i.MX23 just one USB port so disable the second controller probe >> >> when building for i.MX23. >> >> >> >> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br> >> >> --- >> >> Changes in v2: >> >> - Avoid wrong clock setting in MX23 >> >> >> >> drivers/usb/host/ehci-mxs.c | 12 ++++++++++++ >> >> 1 file changed, 12 insertions(+) >> >> >> >> diff --git a/drivers/usb/host/ehci-mxs.c b/drivers/usb/host/ehci-mxs.c >> >> index 5062af5..b7bf856 100644 >> >> --- a/drivers/usb/host/ehci-mxs.c >> >> +++ b/drivers/usb/host/ehci-mxs.c >> >> @@ -50,10 +50,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,7 +69,9 @@ 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) { >> >> @@ -95,8 +99,12 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr, >> >> struct ehci_hcor **hcor) writel(CLKCTRL_PLL1CTRL0_EN_USB_CLKS | >> >> CLKCTRL_PLL1CTRL0_POWER, >> >> >> >> &clkctrl_regs->hw_clkctrl_pll1ctrl0_set); >> >> >> >> +#if defined(CONFIG_MX23) >> >> + writel(HW_DIGCTL_CTRL_USB0_CLKGATE, &digctl_ctrl->reg_clr); >> > >> > What is that large space before '&digctl_regs->...' ? >> >> My fault. I can fix it. >> >> >> +#elif defined(CONFIG_MX28) >> >> >> >> writel(HW_DIGCTL_CTRL_USB0_CLKGATE | HW_DIGCTL_CTRL_USB1_CLKGATE, >> >> >> >> &digctl_ctrl->reg_clr); >> >> >> >> +#endif >> > >> > I'm sure these can be wrapped in much more elegant way, yes? >> >> I wanted to keep the changes at minimum; so seems the right route. >> What you'd like me to do? Use a temporary variable? > > Either that or wrap it all in struct ehci_mxs and do these ungatings etc on a > per-port basis. I've prepared the v3 of the patch; I will give it a runtime test tomorrow and send.
diff --git a/drivers/usb/host/ehci-mxs.c b/drivers/usb/host/ehci-mxs.c index 5062af5..b7bf856 100644 --- a/drivers/usb/host/ehci-mxs.c +++ b/drivers/usb/host/ehci-mxs.c @@ -50,10 +50,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,7 +69,9 @@ 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) { @@ -95,8 +99,12 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor) writel(CLKCTRL_PLL1CTRL0_EN_USB_CLKS | CLKCTRL_PLL1CTRL0_POWER, &clkctrl_regs->hw_clkctrl_pll1ctrl0_set); +#if defined(CONFIG_MX23) + writel(HW_DIGCTL_CTRL_USB0_CLKGATE, &digctl_ctrl->reg_clr); +#elif defined(CONFIG_MX28) writel(HW_DIGCTL_CTRL_USB0_CLKGATE | HW_DIGCTL_CTRL_USB1_CLKGATE, &digctl_ctrl->reg_clr); +#endif /* Start USB PHY */ writel(0, &ehci_mxs.phy_regs->hw_usbphy_pwd); @@ -153,8 +161,12 @@ int ehci_hcd_stop(int index) &clkctrl_regs->hw_clkctrl_pll1ctrl0_clr); /* Gate off the USB clock */ +#if defined(CONFIG_MX23) + writel(HW_DIGCTL_CTRL_USB0_CLKGATE, &digctl_ctrl->reg_set); +#elif defined(CONFIG_MX28) writel(HW_DIGCTL_CTRL_USB0_CLKGATE | HW_DIGCTL_CTRL_USB1_CLKGATE, &digctl_ctrl->reg_set); +#endif return 0; }
The i.MX23 just one USB port so disable the second controller probe when building for i.MX23. Signed-off-by: Otavio Salvador <otavio@ossystems.com.br> --- Changes in v2: - Avoid wrong clock setting in MX23 drivers/usb/host/ehci-mxs.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)