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

[Marek Vasut]

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 at 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, &regs->gusbcfg);
>>>  
>>>  	/* Program the GAHBCFG Register. */
>>> @@ -422,8 +427,10 @@ static void dwc_otg_core_init(struct dwc2_priv *priv)
>>>  
>>>  	writel(ahbcfg, &regs->gahbcfg);
>>>  
>>> +#ifdef CONFIG_DWC2_HNP_SRP_CAP
>>>  	/* Program the GUSBCFG register for HNP/SRP. */
>>>  	setbits_le32(&regs->gusbcfg, DWC2_GUSBCFG_HNPCAP | DWC2_GUSBCFG_SRPCAP);
>>> +#endif
>>>  
>>>  #ifdef CONFIG_DWC2_IC_USB_CAP
>>>  	setbits_le32(&regs->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