[U-Boot] [PATCH V3] usb:ci_udc: Remove ULPI setting for i.MX OTG controller
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) {
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
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
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
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.
participants (3)
-
Li Ye-B37916 -
Marek Vasut -
Peng Fan