Hi, Andy Shevchenko <[email protected]> writes: > On Fri, Jul 27, 2018 at 2:02 AM, Thinh Nguyen <[email protected]> > wrote: >> On 7/26/2018 2:59 PM, Thinh Nguyen wrote: >>> On 7/26/2018 2:32 PM, Andy Shevchenko wrote: >>>> On Thu, Jul 26, 2018 at 11:52 PM, Thinh Nguyen >>>> <[email protected]> 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. >>>>> >>>>> Signed-off-by: Thinh Nguyen <[email protected]> >>>>> --- >>>>> drivers/usb/dwc3/core.c | 8 ++++++++ >>>>> 1 file changed, 8 insertions(+) >>>>> >>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>>>> index 21e4931d0cc0..64ba664d467c 100644 >>>>> --- a/drivers/usb/dwc3/core.c >>>>> +++ b/drivers/usb/dwc3/core.c >>>>> @@ -78,6 +78,14 @@ static int dwc3_get_dr_mode(struct dwc3 *dwc) >>>>> mode = USB_DR_MODE_HOST; >>>>> else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET)) >>>>> mode = USB_DR_MODE_PERIPHERAL; >>>>> + >>>>> + /* >>>>> + * 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.
> case DWC3_GHWPARAMS0_MODE_GADGET:
> if (IS_ENABLED(CONFIG_USB_DWC3_HOST)) {
> dev_err(dev,
> "Controller does not support host mode.\n");
> return -EINVAL;
> }
> mode = USB_DR_MODE_PERIPHERAL;
> break;
> case DWC3_GHWPARAMS0_MODE_HOST:
> if (IS_ENABLED(CONFIG_USB_DWC3_GADGET)) {
> dev_err(dev,
> "Controller does not support device mode.\n");
> return -EINVAL;
> }
> mode = USB_DR_MODE_HOST;
> break;
> default:
> if (IS_ENABLED(CONFIG_USB_DWC3_HOST))
> mode = USB_DR_MODE_HOST;
> else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET))
> mode = USB_DR_MODE_PERIPHERAL;
both of these are checking for Kconfig choices.
> /*
> * 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))
^^^^^^^^^^^^^^^^^^^^^^^
making sure device_property "dr_mode" is OTG. Note if I
don't very that mode is OTG, all I know for sure is that HW
is configured for DRD operation and Kconfig enabled support
for both Host and Peripheral roles, but I have not yet
verified e.g. DeviceTree.
> mode = USB_DR_MODE_PERIPHERAL;
> }
--
balbi
signature.asc
Description: PGP signature
