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 <[email protected]> >>> --- >>> >>> 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. > 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__ */ >>> >> > -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot

