On Thu, Feb 14, 2013 at 09:36:26PM +0200, Felipe Balbi wrote: > Hi, > > On Thu, Feb 14, 2013 at 07:30:26PM +0100, Sascha Hauer wrote: > > > > > yeah, this is why I said we should ignore dr_mode (or bail out) when > > > > > !OTG. > > > > > > > > Ok, that's what the patch effectively does. We have this in chipidea/core.c: > > > > > > > > | dr_mode = ci->platdata->dr_mode; > > > > | if (dr_mode == USB_DR_MODE_UNKNOWN) > > > > | dr_mode = USB_DR_MODE_OTG; > > > > > > > > default to otg if nothing specified. > > > > > > you missed my point. I wanted something like: > > > > > > dr_mode = ci->platdata->dr_mode; > > > if ((dr_mode == USB_DR_MODE_UNKNOWN) || !IS_ENABLED(CONFIG_USB_CHIPIDEA_OTG) > > > dr_mode = USB_DR_MODE_OTG; > > > > So everytime the chipidea driver cannot do OTG the driver falls back to > > exactly this mode? > > hehe, that was me being stupid I meant something like: > > if (!IS_ENABLED(HOST) && dr_mode == HOST) > > or > > if (!IS_ENABLED(GADGET) && dr_mode == GADGET) > > in those cases, it's best to either force OTG (then driver will work > with whatever it has) or bail out. > > Thinking a bit harder, it's best to bail as you can't be sure udc.c or > host.c have been compiled in. Look again, that's what the current code does. Maybe it's not coded as explicit as you describe above. | /* initialize role(s) before the interrupt is requested */ | if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) { | ret = ci_hdrc_host_init(ci); | if (ret) | dev_info(dev, "doesn't support host\n"); | } if IS_ENABLED(HOST), then ci_hdrc_host_init() will set ci->roles[CI_ROLE_HOST] in case it manages to initialize the host driver. if !IS_ENABLED(HOST), then ci_hdrc_host_init() will be a static inline noop function, so ci->roles[CI_ROLE_HOST] ends up being NULL. | | if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_PERIPHERAL) { | ret = ci_hdrc_gadget_init(ci); | if (ret) | dev_info(dev, "doesn't support gadget\n"); | } Same applies for gadget. | | if (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) { | dev_err(dev, "no supported roles\n"); | ret = -ENODEV; | goto rm_wq; | } At this point (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) means that the intersection of what is requested and what we are able to do is empty. The reasons could be That the requested mode is not compiled into the kernel, or the requested role failed to initialize. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html