On Wed, Feb 27, 2013 at 11:06:05PM +0200, Alexander Shishkin wrote: > On Feb 27, 2013 8:33 AM, "Peter Chen" <peter.chen@xxxxxxxxxxxxx> wrote: > > > > - During the connect/disconnect host, we need to pullup > > and pulldown dp > > - Make sure the dp is not pullup until the vbus is on when > > flag CI13XXX_PULLUP_ON_VBUS is set > > - Using hw_device_state when set run/stop bit > > > > Signed-off-by: Peter Chen <peter.chen@xxxxxxxxxxxxx> > > --- > > drivers/usb/chipidea/udc.c | 10 ++++++++-- > > 1 files changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c > > index e82dae4..597ae64 100644 > > --- a/drivers/usb/chipidea/udc.c > > +++ b/drivers/usb/chipidea/udc.c > > @@ -91,8 +91,10 @@ static int hw_device_state(struct ci13xxx *ci, u32 > dma) > > /* interrupt, error, port change, reset, sleep/suspend > */ > > hw_write(ci, OP_USBINTR, ~0, > > > USBi_UI|USBi_UEI|USBi_PCI|USBi_URI|USBi_SLI); > > + hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS); > > } else { > > hw_write(ci, OP_USBINTR, ~0, 0); > > + hw_write(ci, OP_USBCMD, USBCMD_RS, 0); > > } > > return 0; > > } > > @@ -1424,10 +1426,14 @@ static int ci13xxx_pullup(struct usb_gadget > *_gadget, int is_on) > > { > > struct ci13xxx *ci = container_of(_gadget, struct ci13xxx, > gadget); > > > > + if ((ci->platdata->flags & CI13XXX_PULLUP_ON_VBUS) && > > + !ci->vbus_active) > > + return -ENOTSUPP; > > This doesn't look right, if !ci->vbus_active *and* is_on, then we can't do > what we're asked to do, so it makes sense to return error (which, by the > looks of it, will be ignored by gadgets anyway, but that's another thing), > and I don't think ENOTSUPP is the right choice here. > > In case !ci->vbus_active and !is_on, it should be safe to return 0. At later patch, the flag CI13XXX_PULLUP_ON_VBUS is retired. The code likes below: static int ci13xxx_pullup(struct usb_gadget *_gadget, int is_on) { struct ci13xxx *ci = container_of(_gadget, struct ci13xxx, gadget); if (!ci->vbus_active) return -ENOTSUPP; if (is_on) hw_device_state(ci, ci->ep0out->qh.dma); else hw_device_state(ci, 0); return 0; } So you suggested I change like below: static int ci13xxx_pullup(struct usb_gadget *_gadget, int is_on) { struct ci13xxx *ci = container_of(_gadget, struct ci13xxx, gadget); if (!ci->vbus_active) return -EPERM; if (is_on) hw_device_state(ci, ci->ep0out->qh.dma); else hw_device_state(ci, 0); return 0; } -- Best Regards, Peter Chen -- 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