[U-Boot] [PATCH v2 07/10] usb: dwc2: fix macro error and change default config for rk3328

Fix macro error of dwc2 driver, add macro definition to config force mode and HNP/SRP capability.
Signed-off-by: Meng Dongyang daniel.meng@rock-chips.com ---
Changes in v2: - Splited from patch [07/08] of v1
drivers/usb/host/dwc2.c | 9 ++++++++- drivers/usb/host/dwc2.h | 10 ++++++---- 2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index 0e5df15..51619d6 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -370,7 +370,7 @@ static void dwc_otg_core_init(struct dwc2_priv *priv) usbcfg &= ~DWC2_GUSBCFG_DDRSEL; #endif } else { /* UTMI+ interface */ -#if (CONFIG_DWC2_UTMI_PHY_WIDTH == 16) +#if (CONFIG_DWC2_UTMI_WIDTH == 16) usbcfg |= DWC2_GUSBCFG_PHYIF; #endif } @@ -394,6 +394,11 @@ static void dwc_otg_core_init(struct dwc2_priv *priv) usbcfg |= DWC2_GUSBCFG_ULPI_CLK_SUS_M; } #endif + +#ifdef CONFIG_DWC2_FORCE_HOST_MODE + usbcfg |= DWC2_GUSBCFG_FORCEHOSTMODE; +#endif + writel(usbcfg, ®s->gusbcfg);
/* Program the GAHBCFG Register. */ @@ -422,8 +427,10 @@ static void dwc_otg_core_init(struct dwc2_priv *priv)
writel(ahbcfg, ®s->gahbcfg);
+#ifdef CONFIG_DWC2_HNP_SRP_CAP /* Program the GUSBCFG register for HNP/SRP. */ setbits_le32(®s->gusbcfg, DWC2_GUSBCFG_HNPCAP | DWC2_GUSBCFG_SRPCAP); +#endif
#ifdef CONFIG_DWC2_IC_USB_CAP setbits_le32(®s->gusbcfg, DWC2_GUSBCFG_IC_USB_CAP); diff --git a/drivers/usb/host/dwc2.h b/drivers/usb/host/dwc2.h index 4482dc6..3c62e9b 100644 --- a/drivers/usb/host/dwc2.h +++ b/drivers/usb/host/dwc2.h @@ -771,11 +771,11 @@ struct dwc2_core_regs { #define CONFIG_DWC2_MAX_TRANSFER_SIZE 65535 #define CONFIG_DWC2_MAX_PACKET_COUNT 511
-#define DWC2_PHY_TYPE_FS 0 -#define DWC2_PHY_TYPE_UTMI 1 -#define DWC2_PHY_TYPE_ULPI 2 +#define DWC2_PHY_TYPE_UTMI 0 +#define DWC2_PHY_TYPE_ULPI 1 +#define DWC2_PHY_TYPE_FS 2 #define CONFIG_DWC2_PHY_TYPE DWC2_PHY_TYPE_UTMI /* PHY type */ -#define CONFIG_DWC2_UTMI_WIDTH 8 /* UTMI bus width (8/16) */ +#define CONFIG_DWC2_UTMI_WIDTH 16 /* UTMI bus width (8/16) */
#undef CONFIG_DWC2_PHY_ULPI_DDR /* ULPI PHY uses DDR mode */ #define CONFIG_DWC2_PHY_ULPI_EXT_VBUS /* ULPI PHY controls VBUS */ @@ -785,5 +785,7 @@ struct dwc2_core_regs { #undef CONFIG_DWC2_THR_CTL /* Threshold control */ #define CONFIG_DWC2_TX_THR_LENGTH 64 #undef CONFIG_DWC2_IC_USB_CAP /* IC Cap */ +#define CONFIG_DWC2_FORCE_HOST_MODE /* Force host mode */ +#undef CONFIG_DWC2_HNP_SRP_CAP /* HNP/SRP Cap */
#endif /* __DWC2_H__ */

On 1 June 2017 at 05:25, Meng Dongyang daniel.meng@rock-chips.com wrote:
Fix macro error of dwc2 driver, add macro definition to config force mode and HNP/SRP capability.
Signed-off-by: Meng Dongyang daniel.meng@rock-chips.com
Changes in v2:
- Splited from patch [07/08] of v1
drivers/usb/host/dwc2.c | 9 ++++++++- drivers/usb/host/dwc2.h | 10 ++++++---- 2 files changed, 14 insertions(+), 5 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On 06/01/2017 01:25 PM, Meng Dongyang wrote:
Fix macro error of dwc2 driver, add macro definition to config force mode and HNP/SRP capability.
Can you please elaborate some more on the purpose of this patch ? It seems wrong, since the driver works on systems using DWC2 as is and this patch changes constants used in the driver.
Signed-off-by: Meng Dongyang daniel.meng@rock-chips.com
Changes in v2:
- Splited from patch [07/08] of v1
drivers/usb/host/dwc2.c | 9 ++++++++- drivers/usb/host/dwc2.h | 10 ++++++---- 2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index 0e5df15..51619d6 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -370,7 +370,7 @@ static void dwc_otg_core_init(struct dwc2_priv *priv) usbcfg &= ~DWC2_GUSBCFG_DDRSEL; #endif } else { /* UTMI+ interface */ -#if (CONFIG_DWC2_UTMI_PHY_WIDTH == 16) +#if (CONFIG_DWC2_UTMI_WIDTH == 16) usbcfg |= DWC2_GUSBCFG_PHYIF; #endif } @@ -394,6 +394,11 @@ static void dwc_otg_core_init(struct dwc2_priv *priv) usbcfg |= DWC2_GUSBCFG_ULPI_CLK_SUS_M; } #endif
+#ifdef CONFIG_DWC2_FORCE_HOST_MODE
- usbcfg |= DWC2_GUSBCFG_FORCEHOSTMODE;
+#endif
writel(usbcfg, ®s->gusbcfg);
/* Program the GAHBCFG Register. */
@@ -422,8 +427,10 @@ static void dwc_otg_core_init(struct dwc2_priv *priv)
writel(ahbcfg, ®s->gahbcfg);
+#ifdef CONFIG_DWC2_HNP_SRP_CAP /* Program the GUSBCFG register for HNP/SRP. */ setbits_le32(®s->gusbcfg, DWC2_GUSBCFG_HNPCAP | DWC2_GUSBCFG_SRPCAP); +#endif
#ifdef CONFIG_DWC2_IC_USB_CAP setbits_le32(®s->gusbcfg, DWC2_GUSBCFG_IC_USB_CAP); diff --git a/drivers/usb/host/dwc2.h b/drivers/usb/host/dwc2.h index 4482dc6..3c62e9b 100644 --- a/drivers/usb/host/dwc2.h +++ b/drivers/usb/host/dwc2.h @@ -771,11 +771,11 @@ struct dwc2_core_regs { #define CONFIG_DWC2_MAX_TRANSFER_SIZE 65535 #define CONFIG_DWC2_MAX_PACKET_COUNT 511
-#define DWC2_PHY_TYPE_FS 0 -#define DWC2_PHY_TYPE_UTMI 1 -#define DWC2_PHY_TYPE_ULPI 2 +#define DWC2_PHY_TYPE_UTMI 0 +#define DWC2_PHY_TYPE_ULPI 1 +#define DWC2_PHY_TYPE_FS 2 #define CONFIG_DWC2_PHY_TYPE DWC2_PHY_TYPE_UTMI /* PHY type */ -#define CONFIG_DWC2_UTMI_WIDTH 8 /* UTMI bus width (8/16) */ +#define CONFIG_DWC2_UTMI_WIDTH 16 /* UTMI bus width (8/16) */
So basically you now force 16bit UTMI bus width instead of 8 ? This breaks every single platform in U-Boot using the DWC2.
#undef CONFIG_DWC2_PHY_ULPI_DDR /* ULPI PHY uses DDR mode */ #define CONFIG_DWC2_PHY_ULPI_EXT_VBUS /* ULPI PHY controls VBUS */ @@ -785,5 +785,7 @@ struct dwc2_core_regs { #undef CONFIG_DWC2_THR_CTL /* Threshold control */ #define CONFIG_DWC2_TX_THR_LENGTH 64 #undef CONFIG_DWC2_IC_USB_CAP /* IC Cap */ +#define CONFIG_DWC2_FORCE_HOST_MODE /* Force host mode */ +#undef CONFIG_DWC2_HNP_SRP_CAP /* HNP/SRP Cap */
#endif /* __DWC2_H__ */

On 2017/6/3 18:23, Marek Vasut wrote:
On 06/01/2017 01:25 PM, Meng Dongyang wrote:
Fix macro error of dwc2 driver, add macro definition to config force mode and HNP/SRP capability.
Can you please elaborate some more on the purpose of this patch ? It seems wrong, since the driver works on systems using DWC2 as is and this patch changes constants used in the driver.
OK, I will think about making it works as before. But I found that some config need to be change on some platform.
Signed-off-by: Meng Dongyang daniel.meng@rock-chips.com
Changes in v2:
Splited from patch [07/08] of v1
drivers/usb/host/dwc2.c | 9 ++++++++- drivers/usb/host/dwc2.h | 10 ++++++---- 2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index 0e5df15..51619d6 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -370,7 +370,7 @@ static void dwc_otg_core_init(struct dwc2_priv *priv) usbcfg &= ~DWC2_GUSBCFG_DDRSEL; #endif } else { /* UTMI+ interface */ -#if (CONFIG_DWC2_UTMI_PHY_WIDTH == 16) +#if (CONFIG_DWC2_UTMI_WIDTH == 16)
The macro definition of UTMI width is CONFIG_DWC2_UTMI_WIDTH, yet here is CONFIG_DWC2_UTMI_PHY_WIDTH. So maybe it's better to change them to the same one.
usbcfg |= DWC2_GUSBCFG_PHYIF;
#endif } @@ -394,6 +394,11 @@ static void dwc_otg_core_init(struct dwc2_priv *priv) usbcfg |= DWC2_GUSBCFG_ULPI_CLK_SUS_M; } #endif
+#ifdef CONFIG_DWC2_FORCE_HOST_MODE
- usbcfg |= DWC2_GUSBCFG_FORCEHOSTMODE;
+#endif
I found that on the platform not support HNP/SRP capability, the dr_mode will still be otg after init and the host mode can't work. So may be a config of force_mode is necessary.
writel(usbcfg, ®s->gusbcfg);
/* Program the GAHBCFG Register. */ @@ -422,8 +427,10 @@ static void dwc_otg_core_init(struct dwc2_priv *priv)
writel(ahbcfg, ®s->gahbcfg);
+#ifdef CONFIG_DWC2_HNP_SRP_CAP /* Program the GUSBCFG register for HNP/SRP. */ setbits_le32(®s->gusbcfg, DWC2_GUSBCFG_HNPCAP | DWC2_GUSBCFG_SRPCAP); +#endif
#ifdef CONFIG_DWC2_IC_USB_CAP setbits_le32(®s->gusbcfg, DWC2_GUSBCFG_IC_USB_CAP); diff --git a/drivers/usb/host/dwc2.h b/drivers/usb/host/dwc2.h index 4482dc6..3c62e9b 100644 --- a/drivers/usb/host/dwc2.h +++ b/drivers/usb/host/dwc2.h @@ -771,11 +771,11 @@ struct dwc2_core_regs { #define CONFIG_DWC2_MAX_TRANSFER_SIZE 65535 #define CONFIG_DWC2_MAX_PACKET_COUNT 511
-#define DWC2_PHY_TYPE_FS 0 -#define DWC2_PHY_TYPE_UTMI 1 -#define DWC2_PHY_TYPE_ULPI 2 +#define DWC2_PHY_TYPE_UTMI 0 +#define DWC2_PHY_TYPE_ULPI 1 +#define DWC2_PHY_TYPE_FS 2
According to table 5-10 of "DesignWare Cores USB 2.0 Hi-Speed On-The-Go(OTG)" 3.10a databook,
ULPI or UTMI+ Select (ULPI_UTMI_Sel) The application uses this bit to select either a UTMI+ interface or ULPI Interface. ■ 1'b0: UTMI+ Interface ■ 1'b1: ULPI Interface
But the macor definition in current code would make a opposite config on ULPI_UTMI_Sel bit, (in the function "dwc_otg_core_init") 1: UTMI+ Interface 0: ULPI Interface
It seems wrong.
#define CONFIG_DWC2_PHY_TYPE DWC2_PHY_TYPE_UTMI /* PHY type */ -#define CONFIG_DWC2_UTMI_WIDTH 8 /* UTMI bus width (8/16) */ +#define CONFIG_DWC2_UTMI_WIDTH 16 /* UTMI bus width (8/16) */
So basically you now force 16bit UTMI bus width instead of 8 ?
Three changes basically 1. force 16bit UTMI bus width instead of 8 2. config force mode on the platform without HNP/SRP capability. 3. change macro definition about PHY type (ULPI_UTMI_Sel).
This breaks every single platform in U-Boot using the DWC2.
#undef CONFIG_DWC2_PHY_ULPI_DDR /* ULPI PHY uses DDR mode */ #define CONFIG_DWC2_PHY_ULPI_EXT_VBUS /* ULPI PHY controls VBUS */ @@ -785,5 +785,7 @@ struct dwc2_core_regs { #undef CONFIG_DWC2_THR_CTL /* Threshold control */ #define CONFIG_DWC2_TX_THR_LENGTH 64 #undef CONFIG_DWC2_IC_USB_CAP /* IC Cap */ +#define CONFIG_DWC2_FORCE_HOST_MODE /* Force host mode */ +#undef CONFIG_DWC2_HNP_SRP_CAP /* HNP/SRP Cap */
#endif /* __DWC2_H__ */

On 06/05/2017 06:09 AM, rock-chips(daniel.meng) wrote:
On 2017/6/3 18:23, Marek Vasut wrote:
On 06/01/2017 01:25 PM, Meng Dongyang wrote:
Fix macro error of dwc2 driver, add macro definition to config force mode and HNP/SRP capability.
Can you please elaborate some more on the purpose of this patch ? It seems wrong, since the driver works on systems using DWC2 as is and this patch changes constants used in the driver.
OK, I will think about making it works as before. But I found that some config need to be change on some platform.
If you have platform specific tweaks, they need to be incorporated such that they don't break other platforms !
Signed-off-by: Meng Dongyang daniel.meng@rock-chips.com
Changes in v2:
- Splited from patch [07/08] of v1
drivers/usb/host/dwc2.c | 9 ++++++++- drivers/usb/host/dwc2.h | 10 ++++++---- 2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index 0e5df15..51619d6 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -370,7 +370,7 @@ static void dwc_otg_core_init(struct dwc2_priv *priv) usbcfg &= ~DWC2_GUSBCFG_DDRSEL; #endif } else { /* UTMI+ interface */ -#if (CONFIG_DWC2_UTMI_PHY_WIDTH == 16) +#if (CONFIG_DWC2_UTMI_WIDTH == 16)
The macro definition of UTMI width is CONFIG_DWC2_UTMI_WIDTH, yet here is CONFIG_DWC2_UTMI_PHY_WIDTH. So maybe it's better to change them to the same one.
usbcfg |= DWC2_GUSBCFG_PHYIF;
#endif } @@ -394,6 +394,11 @@ static void dwc_otg_core_init(struct dwc2_priv *priv) usbcfg |= DWC2_GUSBCFG_ULPI_CLK_SUS_M; } #endif
+#ifdef CONFIG_DWC2_FORCE_HOST_MODE
- usbcfg |= DWC2_GUSBCFG_FORCEHOSTMODE;
+#endif
I found that on the platform not support HNP/SRP capability, the dr_mode will still be otg after init and the host mode can't work. So may be a config of force_mode is necessary.
Can we use DT props instead of ad-hoc config options ?
writel(usbcfg, ®s->gusbcfg);
/* Program the GAHBCFG Register. */ @@ -422,8 +427,10 @@ static void dwc_otg_core_init(struct dwc2_priv *priv)
writel(ahbcfg, ®s->gahbcfg);
+#ifdef CONFIG_DWC2_HNP_SRP_CAP /* Program the GUSBCFG register for HNP/SRP. */ setbits_le32(®s->gusbcfg, DWC2_GUSBCFG_HNPCAP | DWC2_GUSBCFG_SRPCAP); +#endif
#ifdef CONFIG_DWC2_IC_USB_CAP setbits_le32(®s->gusbcfg, DWC2_GUSBCFG_IC_USB_CAP); diff --git a/drivers/usb/host/dwc2.h b/drivers/usb/host/dwc2.h index 4482dc6..3c62e9b 100644 --- a/drivers/usb/host/dwc2.h +++ b/drivers/usb/host/dwc2.h @@ -771,11 +771,11 @@ struct dwc2_core_regs { #define CONFIG_DWC2_MAX_TRANSFER_SIZE 65535 #define CONFIG_DWC2_MAX_PACKET_COUNT 511
-#define DWC2_PHY_TYPE_FS 0 -#define DWC2_PHY_TYPE_UTMI 1 -#define DWC2_PHY_TYPE_ULPI 2 +#define DWC2_PHY_TYPE_UTMI 0 +#define DWC2_PHY_TYPE_ULPI 1 +#define DWC2_PHY_TYPE_FS 2
According to table 5-10 of "DesignWare Cores USB 2.0 Hi-Speed On-The-Go(OTG)" 3.10a databook,
ULPI or UTMI+ Select (ULPI_UTMI_Sel) The application uses this bit to select either a UTMI+ interface or ULPI Interface. ■ 1'b0: UTMI+ Interface ■ 1'b1: ULPI Interface
But the macor definition in current code would make a opposite config on ULPI_UTMI_Sel bit, (in the function "dwc_otg_core_init") 1: UTMI+ Interface 0: ULPI Interface
It seems wrong.
Please check at least socfpga, exynos and possibly other DWC2 users and ev. fix them up. Changing what seems wrong but works might cause quite some breakage.
#define CONFIG_DWC2_PHY_TYPE DWC2_PHY_TYPE_UTMI /* PHY type */ -#define CONFIG_DWC2_UTMI_WIDTH 8 /* UTMI bus width (8/16) */ +#define CONFIG_DWC2_UTMI_WIDTH 16 /* UTMI bus width (8/16) */
So basically you now force 16bit UTMI bus width instead of 8 ?
Three changes basically
Three patches then.
- force 16bit UTMI bus width instead of 8
- config force mode on the platform without HNP/SRP capability.
- change macro definition about PHY type (ULPI_UTMI_Sel).
This breaks every single platform in U-Boot using the DWC2.
#undef CONFIG_DWC2_PHY_ULPI_DDR /* ULPI PHY uses DDR mode */ #define CONFIG_DWC2_PHY_ULPI_EXT_VBUS /* ULPI PHY controls VBUS */ @@ -785,5 +785,7 @@ struct dwc2_core_regs { #undef CONFIG_DWC2_THR_CTL /* Threshold control */ #define CONFIG_DWC2_TX_THR_LENGTH 64 #undef CONFIG_DWC2_IC_USB_CAP /* IC Cap */ +#define CONFIG_DWC2_FORCE_HOST_MODE /* Force host mode */ +#undef CONFIG_DWC2_HNP_SRP_CAP /* HNP/SRP Cap */
#endif /* __DWC2_H__ */
participants (4)
-
Marek Vasut
-
Meng Dongyang
-
rock-chips(daniel.meng)
-
Simon Glass