Hi, On Thu, Mar 14, 2013 at 05:24:39PM +0800, Peter Chen wrote: > > > @@ -278,7 +278,10 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *dri > > > driver->unbind(gadget); > > > goto err1; > > > } > > > - usb_gadget_connect(gadget); > > > + if (!gadget->ops->vbus_session || > > > + (gadget->ops->vbus_session > > > + && gadget->vbus_active)) > > > + usb_gadget_connect(gadget); > > > > > > kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE); > > > return 0; > > > @@ -384,7 +387,10 @@ static ssize_t usb_udc_softconn_store(struct device *dev, > > > > > > if (sysfs_streq(buf, "connect")) { > > > usb_gadget_udc_start(gadget, udc->driver); > > > - usb_gadget_connect(gadget); > > > + if (!gadget->ops->vbus_session || > > > + (gadget->ops->vbus_session > > > + && gadget->vbus_active)) > > > + usb_gadget_connect(gadget); > > > > this patch is incomplete. What happens if this test fails ? Who will > > connect pullup then ? > > gadget->ops->vbus_session will handle it when the vbus interrupt comes for your driver, what about all the others ? Also, we shouldn't allow this ping-pong between who handles pullup and who handles vbus_session. It should all be managed by udc-core with UDC drivers just providing enough hooks. If we allow the UDC driver to connect pullups when VBUS IRQ comes, we could fall into all sorts of traps: a) we could connect cable with no gadget driver loaded b) there will be code duplication among all UDC drivers to call usb_gadge_connect() from vbus_session c) we might screw up the usb_function_activate()/deactivate() counter Need to be very careful with all these details, there are many, many users to udc-core with different requirements. So unless we can come up with a way which wouldn't cause code duplication or regressions, I don't think we can solve the real problem. I guess the best solution to all problems is to start deferring pullup to when gadget driver says it's ok to connect them. It's far more likely that we will already have connection to a host and VBUS will be alive. Also, I'm not sure what does this all mean for SRP. Should we connect pullup before or after SRP ? -- balbi
Attachment:
signature.asc
Description: Digital signature