Re: [PATCH v2 4/4] usb: dwc3: Workaround for super-speed host on dra7 in dual-role mode

[Roger Quadros]

+Chanwoo

Hi Vivek,

On 23/02/17 10:34, Vivek Gautam wrote:
> 
> 
> On 02/16/2017 06:36 PM, Roger Quadros wrote:
>> dra7 OTG core limits the host controller to USB2.0 (high-speed) mode
>> when we're operating in dual-role.
>>
>> We work around that by bypassing the OTG core and reading the
>> extcon framework directly for ID/VBUS events.
>>
>> Signed-off-by: Roger Quadros <rogerq@xxxxxx>
>> ---
>>   Documentation/devicetree/bindings/usb/dwc3.txt |   2 +
>>   drivers/usb/dwc3/core.c                        | 169 ++++++++++++++++++++++++-
>>   drivers/usb/dwc3/core.h                        |   5 +
>>   3 files changed, 170 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
>> index e3e6983..9955c0d 100644
>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>> @@ -53,6 +53,8 @@ Optional properties:
>>    - snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of GFLADJ
>>       register for post-silicon frame length adjustment when the
>>       fladj_30mhz_sdbnd signal is invalid or incorrect.
>> + - extcon: phandle to the USB connector extcon device. If present, extcon
>> +    device will be used to get USB cable events instead of OTG controller.
>>      - <DEPRECATED> tx-fifo-resize: determines if the FIFO *has* to be reallocated.
>>   diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 619fa7c..b02d911 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
> 
> [snip]
> 
>> @@ -1587,6 +1727,14 @@ static int dwc3_probe(struct platform_device *pdev)
>>         dwc3_get_properties(dwc);
>>   +    if (dev->of_node) {
>> +        if (of_property_read_bool(dev->of_node, "extcon"))
>> +            dwc->edev = extcon_get_edev_by_phandle(dev, 0);
> 
> Don't we want separate edev's for vbus and id ?
> One can have separate pins connected to them and in that case
> we can't get the events out of one pin only.

Is such kind of hardware really there? Ideally one extcon device
represents one connector. So VBUS and ID events of a single USB
port should come on one extcon device.
If it doesn't then you need to fix your platforms extcon driver
so that it does.
Chanwoo can correct me if I'm wrong on this understanding.

Currently, if VBUS and ID come on GPIOs the extcon-usb-gpio driver
takes care of both.

> 
>> +
>> +        if (IS_ERR(dwc->edev))
>> +            return PTR_ERR(dwc->edev);
> 
>  Took me a while to get to this. :)
> 
>                 if (IS_ERR(dwc->edev)) {
>                        ret = PTR_ERR(dwc->edev);
>                        goto err0;
>                 }
> 
> We want to reset the res->start back to its original offset.
> 
> Testing this series currently. Will get back with my results.
> 
> 
Thanks :)

-- 
cheers,
-roger