[U-Boot] [PATCH v3 07/10] usb: dwc2: force to host mode if HNP/SRP not support

[Marek Vasut]

On 06/07/2017 05:33 AM, rock-chips(daniel.meng) wrote:
> 
> 
> On 2017/6/6 23:02, Marek Vasut wrote:
>> On 06/06/2017 12:33 PM, Meng Dongyang wrote:
>>> In current code, after running the command of "usb start", the
>>> controller
>>> will keep in otg mode and can't switch to host mode if not support
>>> SNP/SRP capability. So add the property of "hnp-srp-disable" in the DTS
>>> to config the contrller work in force mode of host.
>>>
>>> Signed-off-by: Meng Dongyang <daniel.meng at rock-chips.com>
>>> ---
>>>
>>> Changes in v3:
>>> - revert change of macro definition in dwc2 driver
>>> - support host mode without HNP/SRP capability through DTS
>>>
>>> Changes in v2:
>>> - Splited from patch [07/08] of v1
>>>
>>>   drivers/usb/host/dwc2.c | 13 ++++++++++++-
>>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
>>> index 0e5df15..73a0290 100644
>>> --- a/drivers/usb/host/dwc2.c
>>> +++ b/drivers/usb/host/dwc2.c
>>> @@ -43,6 +43,7 @@ struct dwc2_priv {
>>>       struct dwc2_core_regs *regs;
>>>       int root_hub_devnum;
>>>       bool ext_vbus;
>>> +    bool hnp_srp_disable;
>>>       bool oc_disable;
>>>   };
>>>   @@ -394,6 +395,9 @@ static void dwc_otg_core_init(struct dwc2_priv
>>> *priv)
>>>           usbcfg |= DWC2_GUSBCFG_ULPI_CLK_SUS_M;
>>>       }
>>>   #endif
>>> +    if (priv->hnp_srp_disable)
>>> +        usbcfg |= DWC2_GUSBCFG_FORCEHOSTMODE;
>>> +
>>>       writel(usbcfg, &regs->gusbcfg);
>>>         /* Program the GAHBCFG Register. */
>>> @@ -423,7 +427,9 @@ static void dwc_otg_core_init(struct dwc2_priv
>>> *priv)
>>>       writel(ahbcfg, &regs->gahbcfg);
>>>         /* Program the GUSBCFG register for HNP/SRP. */
>>> -    setbits_le32(&regs->gusbcfg, DWC2_GUSBCFG_HNPCAP |
>>> DWC2_GUSBCFG_SRPCAP);
>>> +    if (!priv->hnp_srp_disable)
>>> +        setbits_le32(&regs->gusbcfg,
>>> +                 DWC2_GUSBCFG_HNPCAP | DWC2_GUSBCFG_SRPCAP);
>> I guess you can prepare the mask and then do setbits...() once ?
>> Looks good otherwise.
> I have attempted to move this config to the last writel of usbcfg and
> checked on our platform.
> Do you think it is better?

There is no last writel() call in this function, so no. There're only
setbits() call and this can be reduced to a single setbits() call at the
end. Note that setbits...() is a RMW call.

> Prepare a mask maybe looks a little strange
> since it only use with
> writel() and the other setbits..() in this function use macro definition
> directly.

Not sure I understand this concern. You need to set conditional bits in
the mask.

ie.
u32 mask = FOO;

if (bar)
 mask |= BAZ;

mask |= QUUX;
setbits...(reg, mask);

>>>   #ifdef CONFIG_DWC2_IC_USB_CAP
>>>       setbits_le32(&regs->gusbcfg, DWC2_GUSBCFG_IC_USB_CAP);
>>> @@ -1244,6 +1250,11 @@ static int dwc2_usb_ofdata_to_platdata(struct
>>> udevice *dev)
>>>       if (prop)
>>>           priv->oc_disable = true;
>>>   +    prop = fdt_getprop(gd->fdt_blob, dev_of_offset(dev),
>>> +               "hnp-srp-disable", NULL);
>>> +    if (prop)
>>> +        priv->hnp_srp_disable = true;
>>> +
>>>       return 0;
>>>   }
>>>  
>>
> 
> 


-- 
Best regards,
Marek Vasut