On 02/02/2018 04:58 PM, Alexey Brodkin wrote:
According to the databook "ULPI_UTMI_Sel" bit (#4) in GUSBCFG register selects between ULPI and UTMI+ interfaces such that: 0 - stands for UTMI+ interface 1 - stands for ULPI interface
Currently implemented logic in the driver is incorrect because CONFIG_DWC2_PHY_TYPE is not a "bool" but instead this is an "enum" which starts from 0 in case of full-speed and is either 1 for UTMI or 2 for ULPI.
In dwc2.h we have: ---------------------->8---------------------- #define DWC2_PHY_TYPE_FS 0 #define DWC2_PHY_TYPE_UTMI 1 #define DWC2_PHY_TYPE_ULPI 2 #define CONFIG_DWC2_PHY_TYPE DWC2_PHY_TYPE_UTMI ---------------------->8----------------------
And in dwc2.c we do "|= CONFIG_DWC2_PHY_TYPE << 4" effectively setting "ULPI_UTMI_Sel" bit for UTMI+ and what's even worse for ULMI we keep "ULPI_UTMI_Sel" zeroed while setting the next "FSIntf" bit (#5) unexpectedly selecting Full speed interface!
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com Cc: Marek Vasut marex@denx.de
drivers/usb/host/dwc2.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index 784fcbdbd94f..29138a2579ab 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -366,7 +366,9 @@ static void dwc_otg_core_init(struct dwc2_priv *priv) * immediately after setting phyif. */ usbcfg &= ~(DWC2_GUSBCFG_ULPI_UTMI_SEL | DWC2_GUSBCFG_PHYIF);
- usbcfg |= CONFIG_DWC2_PHY_TYPE << DWC2_GUSBCFG_ULPI_UTMI_SEL_OFFSET;
What happens if PHY type is set to FS or UTMI , won't that change the behavior of this code ?
+#if (CONFIG_DWC2_PHY_TYPE == DWC2_PHY_TYPE_ULPI)
- usbcfg |= DWC2_GUSBCFG_ULPI_UTMI_SEL;
+#endif
if (usbcfg & DWC2_GUSBCFG_ULPI_UTMI_SEL) { /* ULPI interface */
This condition should be tweaked, I think. You set the ULPI_UTMI_SEL bit above and check for it here again, that's a bit odd.
#ifdef CONFIG_DWC2_PHY_ULPI_DDR