On Fri, Jul 27, 2018 at 1:14 PM, Felipe Balbi <ba...@kernel.org> wrote:
>>>>>> +
>>>>>> + /*
>>>>>> + * dwc_usb31 does not support OTG mode. If the controller
>>>>>> + * supports DRD but the dr_mode is not specified or set
>>>>>> to OTG,
>>>>>> + * then set the mode to peripheral.
>>>>>> + */
>>>>>> + if (mode == USB_DR_MODE_OTG && dwc3_is_usb31(dwc))
>>>>> shouldn't be simple
>>>>>
>>>>> else if (dwc3_is_usb31(...))
>>>>>
>>>>> ?
>>>
>>> Actually, no. We want to set the mode to peripheral _only_ when dr_mode
>>> was not specified or set to OTG. Just checking for dwc3_is_usb31(...) is
>>> not enough.
>>
>> How come?
>>
>> If I read the code correctly...
>>
>> When you go to default case in this switch it's possible if and only
>> if you have mode _exactly_ OTG. You can't have mode unknown here
>> either.
>> The check is redundant and absence of else adds additional burden on
>> the all the rest cases.
>
> Look a little closer
>
>> mode = dwc->dr_mode;
>> hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
>>
>> switch (hw_mode) {
> ^^^^^^^
> Switching on hw_mode, not mode.
Ah, indeed. That's what I have missed.
Thanks for detailed explanation!
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
