On Wed, 3 Apr 2013, Felipe Balbi wrote:
> Hi,
>
> On Sun, Mar 17, 2013 at 05:59:00PM +0100, Sebastian Andrzej Siewior wrote:
> > >diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
> > >index 40b1d88..8a1eeb2 100644
> > >--- a/drivers/usb/gadget/udc-core.c
> > >+++ b/drivers/usb/gadget/udc-core.c
> > >@@ -197,6 +207,8 @@ int usb_add_gadget_udc(struct device *parent, struct
> > >usb_gadget *gadget)
> > > if (ret)
> > > goto err4;
> > >
> > >+ usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED);
> > >+
> > > mutex_unlock(&udc_lock);
> > >
> > > return 0;
> >
> > I'm not sure if it is a good idea to start USB_STATE_NOTATTACHED. It
> > will work fine with updated drivers but the others it will report wrong
> > status and people might not know that the driver isn't yet supporting
> > this. On the other hand they might get suspicious once they plug a
> > device which is fully working and the kernel still reports "not
> > attached" :)
> > Do you think it makes sense to make this missing feature more obvious to
> > the user by doing something like this?
> >
> > diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
> > index 7999cc6..4a38dff 100644
> > --- a/drivers/usb/gadget/udc-core.c
> > +++ b/drivers/usb/gadget/udc-core.c
> > @@ -225,7 +225,7 @@ int usb_add_gadget_udc_release(struct device *parent,
> > struct usb_gadget *gadget,
> > if (ret)
> > goto err4;
> >
> > - usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED);
> > + usb_gadget_set_state(gadget, USB_STATE_NOT_TRACKED);
> >
> > mutex_unlock(&udc_lock);
> >
> > diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c
> > index 0db0a91..78f60e5 100644
> > --- a/drivers/usb/usb-common.c
> > +++ b/drivers/usb/usb-common.c
> > @@ -70,6 +70,7 @@ const char *usb_state_string(enum usb_device_state state)
> > [USB_STATE_ADDRESS] = "addresssed",
> > [USB_STATE_CONFIGURED] = "configured",
> > [USB_STATE_SUSPENDED] = "suspended",
> > + [USB_STATE_NOT_TRACKED] = "unknown / not tracked",
> > };
> >
> > if (state < 0 || state >= ARRAY_SIZE(names))
> > diff --git a/include/uapi/linux/usb/ch9.h b/include/uapi/linux/usb/ch9.h
> > index f738e25..f6d949b 100644
> > --- a/include/uapi/linux/usb/ch9.h
> > +++ b/include/uapi/linux/usb/ch9.h
> > @@ -931,14 +931,15 @@ enum usb_device_state {
> > USB_STATE_ADDRESS,
> > USB_STATE_CONFIGURED, /* most functions */
> >
> > - USB_STATE_SUSPENDED
> > -
> > + USB_STATE_SUSPENDED,
> > /* NOTE: there are actually four different SUSPENDED
> > * states, returning to POWERED, DEFAULT, ADDRESS, or
> > * CONFIGURED respectively when SOF tokens flow again.
> > * At this level there's no difference between L1 and L2
> > * suspend states. (L2 being original USB 1.1 suspend.)
> > */
> > +
> > + USB_STATE_NOT_TRACKED,
> > };
>
> looks alright, although I'd call it USB_STATE_UNKNOWN, instead of
> NOT_TRACKED.
>
> Greg and Alan need to ack it and I guess we can push this on v3.11
Adding USB_STATE_UNKNOWN is okay with me. With that change,
Acked-by: Alan Stern <[email protected]>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
