* [U-Boot] [PATCH V3] usb:ci_udc: Remove ULPI setting for i.MX OTG controller @ 2015-12-31 7:24 Peng Fan 2015-12-31 9:07 ` Marek Vasut 0 siblings, 1 reply; 5+ messages in thread From: Peng Fan @ 2015-12-31 7:24 UTC (permalink / raw) To: u-boot From: "Ye.Li" <B37916@freescale.com> All the i.MX6, i.MX23 and i.MX28 OTG controllers only support UTMI interface. Set to ULPI is not correct, even the controller will reject this setting in PORTSC register. Signed-off-by: Ye.Li <B37916@freescale.com> Signed-off-by: Peng Fan <peng.fan@nxp.com> --- Hi Marek, In V2: https://patchwork.ozlabs.org/patch/541663/ The reason still use DBG, but not change to debug, is that the driver use DBG for debugging and error handling usage. So In this patch, I still keep DBG. If you think, it better to switch to use function debug, I can write another patch to convert all the DBGs to debug for ci_udc driver. Regards, Peng. Changes since v2: - Correct code indent. Changes since v1: - Add error handling for udc probe drivers/usb/gadget/ci_udc.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index 1ba5054..d36bcf6 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -1018,18 +1018,10 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver) return ret; ret = ci_udc_probe(); -#if defined(CONFIG_USB_EHCI_MX6) || defined(CONFIG_USB_EHCI_MXS) - /* - * FIXME: usb_lowlevel_init()->ehci_hcd_init() should be doing all - * HW-specific initialization, e.g. ULPI-vs-UTMI PHY selection - */ - if (!ret) { - struct ci_udc *udc = (struct ci_udc *)controller.ctrl->hcor; - - /* select ULPI phy */ - writel(PTS(PTS_ENABLE) | PFSC, &udc->portsc); + if (ret) { + DBG("udc probe failed, returned %d\n", ret); + return ret; } -#endif ret = driver->bind(&controller.gadget); if (ret) { -- 2.6.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH V3] usb:ci_udc: Remove ULPI setting for i.MX OTG controller 2015-12-31 7:24 [U-Boot] [PATCH V3] usb:ci_udc: Remove ULPI setting for i.MX OTG controller Peng Fan @ 2015-12-31 9:07 ` Marek Vasut 2015-12-31 9:34 ` Li Ye-B37916 0 siblings, 1 reply; 5+ messages in thread From: Marek Vasut @ 2015-12-31 9:07 UTC (permalink / raw) To: u-boot On Thursday, December 31, 2015 at 08:24:45 AM, Peng Fan wrote: > From: "Ye.Li" <B37916@freescale.com> > > All the i.MX6, i.MX23 and i.MX28 OTG controllers only support UTMI > interface. Set to ULPI is not correct, even the controller will > reject this setting in PORTSC register. > > Signed-off-by: Ye.Li <B37916@freescale.com> > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > > Hi Marek, > In V2: https://patchwork.ozlabs.org/patch/541663/ > The reason still use DBG, but not change to debug, is that the driver > use DBG for debugging and error handling usage. > So In this patch, I still keep DBG. If you think, it better to switch > to use function debug, I can write another patch to convert all the > DBGs to debug for ci_udc driver. > > Regards, > Peng. > > Changes since v2: > - Correct code indent. > Changes since v1: > - Add error handling for udc probe > > drivers/usb/gadget/ci_udc.c | 14 +++----------- > 1 file changed, 3 insertions(+), 11 deletions(-) > > diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c > index 1ba5054..d36bcf6 100644 > --- a/drivers/usb/gadget/ci_udc.c > +++ b/drivers/usb/gadget/ci_udc.c > @@ -1018,18 +1018,10 @@ int usb_gadget_register_driver(struct > usb_gadget_driver *driver) return ret; > > ret = ci_udc_probe(); > -#if defined(CONFIG_USB_EHCI_MX6) || defined(CONFIG_USB_EHCI_MXS) > - /* > - * FIXME: usb_lowlevel_init()->ehci_hcd_init() should be doing all > - * HW-specific initialization, e.g. ULPI-vs-UTMI PHY selection > - */ > - if (!ret) { > - struct ci_udc *udc = (struct ci_udc *)controller.ctrl->hcor; > - > - /* select ULPI phy */ > - writel(PTS(PTS_ENABLE) | PFSC, &udc->portsc); So if ci_udc_probe() doesn't fail, this code will select PTS_ENABLE and PFSC in the portsc register, right ? If you remove this code, who will do this configuration ? I suspect this breaks all i.MXes, no ? > + if (ret) { > + DBG("udc probe failed, returned %d\n", ret); > + return ret; > } > -#endif > > ret = driver->bind(&controller.gadget); > if (ret) { Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH V3] usb:ci_udc: Remove ULPI setting for i.MX OTG controller 2015-12-31 9:07 ` Marek Vasut @ 2015-12-31 9:34 ` Li Ye-B37916 2016-01-03 20:51 ` Marek Vasut 2016-01-05 11:40 ` Marek Vasut 0 siblings, 2 replies; 5+ messages in thread From: Li Ye-B37916 @ 2015-12-31 9:34 UTC (permalink / raw) To: u-boot On 12/31/2015 5:07 PM, Marek Vasut wrote: > On Thursday, December 31, 2015 at 08:24:45 AM, Peng Fan wrote: >> From: "Ye.Li" <B37916@freescale.com> >> >> All the i.MX6, i.MX23 and i.MX28 OTG controllers only support UTMI >> interface. Set to ULPI is not correct, even the controller will >> reject this setting in PORTSC register. >> >> Signed-off-by: Ye.Li <B37916@freescale.com> >> Signed-off-by: Peng Fan <peng.fan@nxp.com> >> --- >> >> Hi Marek, >> In V2: https://patchwork.ozlabs.org/patch/541663/ >> The reason still use DBG, but not change to debug, is that the driver >> use DBG for debugging and error handling usage. >> So In this patch, I still keep DBG. If you think, it better to switch >> to use function debug, I can write another patch to convert all the >> DBGs to debug for ci_udc driver. >> >> Regards, >> Peng. >> >> Changes since v2: >> - Correct code indent. >> Changes since v1: >> - Add error handling for udc probe >> >> drivers/usb/gadget/ci_udc.c | 14 +++----------- >> 1 file changed, 3 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c >> index 1ba5054..d36bcf6 100644 >> --- a/drivers/usb/gadget/ci_udc.c >> +++ b/drivers/usb/gadget/ci_udc.c >> @@ -1018,18 +1018,10 @@ int usb_gadget_register_driver(struct >> usb_gadget_driver *driver) return ret; >> >> ret = ci_udc_probe(); >> -#if defined(CONFIG_USB_EHCI_MX6) || defined(CONFIG_USB_EHCI_MXS) >> - /* >> - * FIXME: usb_lowlevel_init()->ehci_hcd_init() should be doing all >> - * HW-specific initialization, e.g. ULPI-vs-UTMI PHY selection >> - */ >> - if (!ret) { >> - struct ci_udc *udc = (struct ci_udc *)controller.ctrl->hcor; >> - >> - /* select ULPI phy */ >> - writel(PTS(PTS_ENABLE) | PFSC, &udc->portsc); > So if ci_udc_probe() doesn't fail, this code will select PTS_ENABLE and PFSC > in the portsc register, right ? If you remove this code, who will do this > configuration ? I suspect this breaks all i.MXes, no ? Selecting PTS_ENABLE and PFSC are NOT correct for i.MXes. This code wants to select ULPI PHY, but only UTMI PHY is supported. So we have to remove this code and leave the default UTMI setting. Actually when setting to ULPI, other platforms except the i.MX6UL refuse the settings and keep default value. So there is no problem for them. But on 6UL TO1.0, there is a IC bug that the PTW bit of PORTSC1 register which is documented as RO can change. This cause the interface setting problem with USB PHY. And finally we traced to this incorrect setting. >> + if (ret) { >> + DBG("udc probe failed, returned %d\n", ret); >> + return ret; >> } >> -#endif >> >> ret = driver->bind(&controller.gadget); >> if (ret) { > Best regards, > Marek Vasut Best regards, Ye Li ^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH V3] usb:ci_udc: Remove ULPI setting for i.MX OTG controller 2015-12-31 9:34 ` Li Ye-B37916 @ 2016-01-03 20:51 ` Marek Vasut 2016-01-05 11:40 ` Marek Vasut 1 sibling, 0 replies; 5+ messages in thread From: Marek Vasut @ 2016-01-03 20:51 UTC (permalink / raw) To: u-boot On Thursday, December 31, 2015 at 10:34:06 AM, Li Ye-B37916 wrote: > On 12/31/2015 5:07 PM, Marek Vasut wrote: > > On Thursday, December 31, 2015 at 08:24:45 AM, Peng Fan wrote: > >> From: "Ye.Li" <B37916@freescale.com> > >> > >> All the i.MX6, i.MX23 and i.MX28 OTG controllers only support UTMI > >> interface. Set to ULPI is not correct, even the controller will > >> reject this setting in PORTSC register. > >> > >> Signed-off-by: Ye.Li <B37916@freescale.com> > >> Signed-off-by: Peng Fan <peng.fan@nxp.com> > >> --- > >> > >> Hi Marek, > >> > >> In V2: https://patchwork.ozlabs.org/patch/541663/ > >> The reason still use DBG, but not change to debug, is that the driver > >> use DBG for debugging and error handling usage. > >> So In this patch, I still keep DBG. If you think, it better to switch > >> to use function debug, I can write another patch to convert all the > >> DBGs to debug for ci_udc driver. > >> > >> Regards, > >> Peng. > >> > >> Changes since v2: > >> - Correct code indent. > >> > >> Changes since v1: > >> - Add error handling for udc probe > >> > >> drivers/usb/gadget/ci_udc.c | 14 +++----------- > >> 1 file changed, 3 insertions(+), 11 deletions(-) > >> > >> diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c > >> index 1ba5054..d36bcf6 100644 > >> --- a/drivers/usb/gadget/ci_udc.c > >> +++ b/drivers/usb/gadget/ci_udc.c > >> @@ -1018,18 +1018,10 @@ int usb_gadget_register_driver(struct > >> usb_gadget_driver *driver) return ret; > >> > >> ret = ci_udc_probe(); > >> > >> -#if defined(CONFIG_USB_EHCI_MX6) || defined(CONFIG_USB_EHCI_MXS) > >> - /* > >> - * FIXME: usb_lowlevel_init()->ehci_hcd_init() should be doing all > >> - * HW-specific initialization, e.g. ULPI-vs-UTMI PHY selection > >> - */ > >> - if (!ret) { > >> - struct ci_udc *udc = (struct ci_udc *)controller.ctrl->hcor; > >> - > >> - /* select ULPI phy */ > >> - writel(PTS(PTS_ENABLE) | PFSC, &udc->portsc); > > > > So if ci_udc_probe() doesn't fail, this code will select PTS_ENABLE and > > PFSC in the portsc register, right ? If you remove this code, who will > > do this configuration ? I suspect this breaks all i.MXes, no ? > > Selecting PTS_ENABLE and PFSC are NOT correct for i.MXes. This code wants > to select ULPI PHY, but only UTMI PHY is supported. So we have to remove > this code and leave the default UTMI setting. Yeah, the code does look nonsensical. I am wondering how this ever worked. CCing Eric, Fabio, Tim. > Actually when setting to ULPI, other platforms except the i.MX6UL refuse > the settings and keep default value. So there is no problem for them. But > on 6UL TO1.0, there is a IC bug that the PTW bit of PORTSC1 register which > is documented as RO can change. This cause the interface setting problem > with USB PHY. And finally we traced to this incorrect setting. I see. I'd like to pick this, but I'd also like to get some Tested-bys on this first. > >> + if (ret) { > >> + DBG("udc probe failed, returned %d\n", ret); > >> + return ret; > >> > >> } > >> > >> -#endif > >> > >> ret = driver->bind(&controller.gadget); > >> if (ret) { > > > > Best regards, > > Marek Vasut > > Best regards, > Ye Li ^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH V3] usb:ci_udc: Remove ULPI setting for i.MX OTG controller 2015-12-31 9:34 ` Li Ye-B37916 2016-01-03 20:51 ` Marek Vasut @ 2016-01-05 11:40 ` Marek Vasut 1 sibling, 0 replies; 5+ messages in thread From: Marek Vasut @ 2016-01-05 11:40 UTC (permalink / raw) To: u-boot On Thursday, December 31, 2015 at 10:34:06 AM, Li Ye-B37916 wrote: > On 12/31/2015 5:07 PM, Marek Vasut wrote: > > On Thursday, December 31, 2015 at 08:24:45 AM, Peng Fan wrote: > >> From: "Ye.Li" <B37916@freescale.com> > >> > >> All the i.MX6, i.MX23 and i.MX28 OTG controllers only support UTMI > >> interface. Set to ULPI is not correct, even the controller will > >> reject this setting in PORTSC register. > >> > >> Signed-off-by: Ye.Li <B37916@freescale.com> > >> Signed-off-by: Peng Fan <peng.fan@nxp.com> > >> --- > >> > >> Hi Marek, > >> > >> In V2: https://patchwork.ozlabs.org/patch/541663/ > >> The reason still use DBG, but not change to debug, is that the driver > >> use DBG for debugging and error handling usage. > >> So In this patch, I still keep DBG. If you think, it better to switch > >> to use function debug, I can write another patch to convert all the > >> DBGs to debug for ci_udc driver. > >> > >> Regards, > >> Peng. > >> > >> Changes since v2: > >> - Correct code indent. > >> > >> Changes since v1: > >> - Add error handling for udc probe > >> > >> drivers/usb/gadget/ci_udc.c | 14 +++----------- > >> 1 file changed, 3 insertions(+), 11 deletions(-) > >> > >> diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c > >> index 1ba5054..d36bcf6 100644 > >> --- a/drivers/usb/gadget/ci_udc.c > >> +++ b/drivers/usb/gadget/ci_udc.c > >> @@ -1018,18 +1018,10 @@ int usb_gadget_register_driver(struct > >> usb_gadget_driver *driver) return ret; > >> > >> ret = ci_udc_probe(); > >> > >> -#if defined(CONFIG_USB_EHCI_MX6) || defined(CONFIG_USB_EHCI_MXS) > >> - /* > >> - * FIXME: usb_lowlevel_init()->ehci_hcd_init() should be doing all > >> - * HW-specific initialization, e.g. ULPI-vs-UTMI PHY selection > >> - */ > >> - if (!ret) { > >> - struct ci_udc *udc = (struct ci_udc *)controller.ctrl->hcor; > >> - > >> - /* select ULPI phy */ > >> - writel(PTS(PTS_ENABLE) | PFSC, &udc->portsc); > > > > So if ci_udc_probe() doesn't fail, this code will select PTS_ENABLE and > > PFSC in the portsc register, right ? If you remove this code, who will > > do this configuration ? I suspect this breaks all i.MXes, no ? > > Selecting PTS_ENABLE and PFSC are NOT correct for i.MXes. This code wants > to select ULPI PHY, but only UTMI PHY is supported. So we have to remove > this code and leave the default UTMI setting. > > Actually when setting to ULPI, other platforms except the i.MX6UL refuse > the settings and keep default value. So there is no problem for them. But > on 6UL TO1.0, there is a IC bug that the PTW bit of PORTSC1 register which > is documented as RO can change. This cause the interface setting problem > with USB PHY. And finally we traced to this incorrect setting. Tested on i.MX28 M28EVK board and applied, thanks. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-01-05 11:40 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-12-31 7:24 [U-Boot] [PATCH V3] usb:ci_udc: Remove ULPI setting for i.MX OTG controller Peng Fan 2015-12-31 9:07 ` Marek Vasut 2015-12-31 9:34 ` Li Ye-B37916 2016-01-03 20:51 ` Marek Vasut 2016-01-05 11:40 ` Marek Vasut
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.