Skip to contentSkip to conversation

usb: dwc3: Set default mode for dwc_usb31

DISCUSSION Felipe Balbi 1 participant 1 message Jul 27, 2018 USB

lore
Share & tools
lore

Felipe Balbi

author
#0 lore raw

Hi,

Quoted 41 lines · Andy Shevchenko
Quoted 30 lines · Thinh Nguyen
Quoted 29 lines · Thinh Nguyen
Quoted 28 lines · Andy Shevchenko
Quoted 14 lines · Thinh Nguyen

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(+)
drivers/usb/dwc3/core.c +0 -0
index 21e4931d0cc0..64ba664d467c 100644

drivers/usb/dwc3/core.c static int dwc3_get_dr_mode(struct dwc3 *dwc) · line 87

Quoted 10 lines
drivers/usb/dwc3/core.c +7 -0
@@ -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

Quoted 7 lines
Quoted 3 lines
Quoted 3 lines

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?

Quoted 7 lines

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

Quoted 4 lines

mode = dwc->dr_mode;
hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);

switch (hw_mode) {

^^^^^^^
Switching on hw_mode, not mode.

Quoted 13 lines

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;
Quoted 8 lines
	}
	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.

Quoted 6 lines
	/*
	 * 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.

Quoted 2 lines
		mode = USB_DR_MODE_PERIPHERAL;
}