On Mon, Dec 19, 2011 at 10:35:22AM -0500, Alan Stern wrote:
> On Mon, 19 Dec 2011, Felipe Balbi wrote:
>
> > that's useful information to expose to userland.
> >
> > NYET-Signed-off-by: Felipe Balbi <balbi@xxxxxx>
>
> > diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> > index 7d64f89..6aa9fc2 100644
> > --- a/include/linux/usb/gadget.h
> > +++ b/include/linux/usb/gadget.h
> > @@ -478,6 +478,53 @@ struct usb_gadget_ops {
> > int (*stop)(struct usb_gadget_driver *);
> > };
> >
> > +enum usb_device_status {
> > + USB_STATUS_IDLE,
>
> IDLE is not the best name. It suggests the gadget isn't doing
> anything, whereas in fact it's supposed to mean the gadget isn't
> getting any bus power. How about USB_STATUS_NOTATTACHED instead?
>
Yes, USB_STATUS_NOTATTACHED seems better. But how this usb_device_status
live with enum usb_device_status at ch9.h? How about re-use the one
at ch9.h, and at some status if necessaries?
> > + USB_STATUS_CONNECTED,
> > + USB_STATUS_RESETTING,
> > +
> > + USB_STATUS_DEFAULT,
> > + USB_STATUS_ADDRESSED,
> > + USB_STATUS_UNCONFIGURED,
> > + USB_STATUS_CONFIGURED,
>
> As pointed out earlier, there is no difference between ADDRESSED and
> UNCONFIGURED.
>
> Furthermore, UDC drivers don't know whether the gadget is configured or
> not. This means gadget drivers would have to make the call to set the
> status to or from CONFIGURED, which is awkward.
at udc driver, is it ok we use below to set CONFIGURED status.
if (udc->driver->setup(&udc->gadget, setup_packet_buff) >= 0)
if (setup->bRequest == USB_REQ_SET_CONFIGURATION)
usb_gadget_set_status(&udc->gadget, USB_STATUS_CONFIGURED);
>
> > +
> > + USB_STATUS_SUSPENDING,
> > + USB_STATUS_SUSPENDED,
> > + USB_STATUS_RESUMING,
> > + USB_STATUS_RESUMED,
>
> As pointed out earlier, there is no RESUMED state. The gadget has to
> go back to the state it had before resuming -- which could be either
> DEFAULT, ADDRESSED, or CONFIGURED. Somebody needs to remember this;
> the UDC core is probably the best place. But it means adding a second
> usb_device_status field to the gadget structure.
>
> > +};
>
> How about USB_STATUS_HOST for use by OTG gadgets? Or does the entire
> gadget structure get deallocated when the controller goes into host
> mode?
>
> > +
> > +static inline const char *
> > +usb_device_status_string(enum usb_device_status status)
> > +{
> > + switch (status) {
> > + case USB_STATUS_IDLE:
> > + return "idle";
> > + case USB_STATUS_CONNECTED:
> > + return "connected";
> > + case USB_STATUS_RESETTING:
> > + return "resetting";
> > + case USB_STATUS_DEFAULT:
> > + return "default";
> > + case USB_STATUS_ADDRESSED:
> > + return "addressed";
> > + case USB_STATUS_UNCONFIGURED:
> > + return "unconfigured";
> > + case USB_STATUS_CONFIGURED:
> > + return "configured";
> > + case USB_STATUS_SUSPENDING:
> > + return "suspending";
> > + case USB_STATUS_SUSPENDED:
> > + return "suspended";
> > + case USB_STATUS_RESUMING:
> > + return "resuming";
> > + case USB_STATUS_RESUMED:
> > + return "resumed";
> > + default:
> > + return "UNKNOWN";
> > + }
> > +}
>
> This should not be an inline routine. It needs to go into udc-core.c
> (which is where it gets used, after all).
>
> Alan Stern
>
>
> --
> 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
>
--
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
