[U-Boot] [PATCH v2 3/4] usb: dwc3: add support for 16 bit UTMI+ interface

[Marek Vasut]

On 08/25/2016 03:17 AM, Kever Yang wrote:
> Hi Marek,
> 
> On 08/24/2016 07:38 PM, Marek Vasut wrote:
>> On 08/24/2016 05:46 AM, Kever Yang wrote:
>>> The dwc3 controller is using 8 bit UTMI+ interface for USB2.0 PHY,
>>> add one variable in dwc3/dwc3_device struct to support 16 bit
>>> UTMI+ interface on some SoCs like Rockchip rk3399.
>>>
>>> Signed-off-by: Kever Yang <kever.yang at rock-chips.com>
>>> ---
>>>
>>> Changes in v2:
>>> - use a variable to identify utmi+ bus width instead of CONFIG MACRO
>>>
>>>   drivers/usb/dwc3/core.c |  6 ++++++
>>>   drivers/usb/dwc3/core.h | 12 ++++++++++++
>>>   include/dwc3-uboot.h    |  1 +
>>>   3 files changed, 19 insertions(+)
>>>
>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>> index 85cc96a..0613508 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>> @@ -388,6 +388,11 @@ static void dwc3_phy_setup(struct dwc3 *dwc)
>>>       if (dwc->dis_u2_susphy_quirk)
>>>           reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>>>   +    if (dwc->usb2_phyif_utmi_width == 16) {
>>> +        reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK;
>>> +        reg |= DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT;
>>> +        reg |= DWC3_GUSB2PHYCFG_PHYIF_16BIT;
>>> +    }
>> Didn't we agree to pull this info from OF ?
> 
> Yes, the dwc->usb2_phyif_utmi_width is from OF,  but I make the DT parse
> in board file instead of in the dwc3 driver,  see my patch:
> [PATCH v2 2/4] board: evb-rk3399: add api to support dwc3 gadget

So this is basically a passing of platform data ?

> I think implement in this way won't affect other soc also using dwc3
> controller,
> the DT parse would cost some time which may not necessary for other soc.

The time needed to run the DT parsing is completely insignificant.
This sounds like a premature optimization.

>>>       dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>>>         mdelay(100);
>>> @@ -676,6 +681,7 @@ int dwc3_uboot_init(struct dwc3_device *dwc3_dev)
>>>         dwc->lpm_nyet_threshold = lpm_nyet_threshold;
>>>       dwc->tx_de_emphasis = tx_de_emphasis;
>>> +    dwc->usb2_phyif_utmi_width = dwc3_dev->usb2_phyif_utmi_width;
>>>         dwc->hird_threshold = hird_threshold
>>>           | (dwc->is_utmi_l1_suspend << 4);
>>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>>> index 72d2fcd..d5bdf97 100644
>>> --- a/drivers/usb/dwc3/core.h
>>> +++ b/drivers/usb/dwc3/core.h
>>> @@ -74,6 +74,7 @@
>>>   #define DWC3_GCTL        0xc110
>>>   #define DWC3_GEVTEN        0xc114
>>>   #define DWC3_GSTS        0xc118
>>> +#define DWC3_GUCTL1        0xc11c
>>>   #define DWC3_GSNPSID        0xc120
>>>   #define DWC3_GGPIO        0xc124
>>>   #define DWC3_GUID        0xc128
>>> @@ -162,7 +163,17 @@
>>>     /* Global USB2 PHY Configuration Register */
>>>   #define DWC3_GUSB2PHYCFG_PHYSOFTRST    (1 << 31)
>>> +#define DWC3_GUSB2PHYCFG_ENBLSLPM   (1 << 8)
>>>   #define DWC3_GUSB2PHYCFG_SUSPHY        (1 << 6)
>>> +#define DWC3_GUSB2PHYCFG_PHYIF_8BIT    (0 << 3)
>>> +#define DWC3_GUSB2PHYCFG_PHYIF_16BIT    (1 << 3)
>>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT    (10)
>>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_MASK    (0xf << \
>>> +        DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT)
>>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT (0x5 << \
>>> +        DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT)
>>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_8BIT (0x9 << \
>>> +        DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT)
>>>     /* Global USB3 PIPE Control Register */
>>>   #define DWC3_GUSB3PIPECTL_PHYSOFTRST    (1 << 31)
>>> @@ -813,6 +824,7 @@ struct dwc3 {
>>>         unsigned        tx_de_emphasis_quirk:1;
>>>       unsigned        tx_de_emphasis:2;
>>> +    unsigned        usb2_phyif_utmi_width;
>>>       int            index;
>>>       struct list_head        list;
>>>   };
>>> diff --git a/include/dwc3-uboot.h b/include/dwc3-uboot.h
>>> index 7af2ad1..cc9ffe8 100644
>>> --- a/include/dwc3-uboot.h
>>> +++ b/include/dwc3-uboot.h
>>> @@ -33,6 +33,7 @@ struct dwc3_device {
>>>       unsigned dis_u2_susphy_quirk;
>>>       unsigned tx_de_emphasis_quirk;
>>>       unsigned tx_de_emphasis;
>>> +    unsigned usb2_phyif_utmi_width;
>> The utmi width is either 8 or 16, so you can continue the bitfield
>> instead of wasting 32 bits.
> 
> I think you mean the usb2_phyif_utmi_width in struct dwc3, but not
> struct dwc3_device, right?

Yes, sorry.

> Thanks,
> -Kever
>>
>>>       int index;
>>>   };
>>>  
>>
> 
> 


-- 
Best regards,
Marek Vasut