Hi, On Thu, Jan 24, 2013 at 02:58:19PM -0500, Alan Stern wrote: > On Thu, 24 Jan 2013, Felipe Balbi wrote: > > > that's useful information to expose to userland. > > > > NYET-Signed-off-by: Felipe Balbi <[email protected]> > > --- > > drivers/usb/gadget/udc-core.c | 52 > > +++++++++++++++++++++++++++++++++++++++++++ > > include/linux/usb/gadget.h | 11 +++++++++ > > 2 files changed, 63 insertions(+) > > > > diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c > > index b6c9110..15e484f 100644 > > --- a/drivers/usb/gadget/udc-core.c > > +++ b/drivers/usb/gadget/udc-core.c > > @@ -27,6 +27,8 @@ > > #include <linux/usb/ch9.h> > > #include <linux/usb/gadget.h> > > > > +#include <uapi/linux/usb/ch9.h> > > Not needed; linux/usb/ch9.h already #includes it.
wasn't it so that we shouldn't depend on indirect inclusion ?
> > @@ -50,6 +52,34 @@ static DEFINE_MUTEX(udc_lock);
> >
> > /*
> > ------------------------------------------------------------------------- */
> >
> > +static const char *usb_device_state_string(enum usb_device_state state)
> > +{
> > + switch(state) {
> > + case USB_STATE_NOTATTACHED:
> > + return "not attached";
> > + case USB_STATE_ATTACHED:
> > + return "attached";
> > + case USB_STATE_POWERED:
> > + return "powered";
> > + case USB_STATE_RECONNECTING:
> > + return "reconnecting";
> > + case USB_STATE_UNAUTHENTICATED:
> > + return "unauthenticated";
> > + case USB_STATE_DEFAULT:
> > + return "default";
> > + case USB_STATE_ADDRESS:
> > + return "addresssed";
> > + case USB_STATE_CONFIGURED:
> > + return "configured";
> > + case USB_STATE_SUSPENDED:
> > + return "suspended";
> > + default:
> > + return "UNKNOWN";
> > + }
> > +}
>
> In theory, this function really belongs in usb-common.c. It's
> potentially useful on both the gadget and the host side. For example,
> you could add a "state" attribute to core/sysfs.c.
fair enough, will split to a separate patch.
> > @@ -101,6 +131,16 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
> >
> > /*
> > ------------------------------------------------------------------------- */
> >
> > +void usb_gadget_set_state(struct usb_gadget *gadget,
> > + enum usb_device_state state)
> > +{
> > + gadget->state = state;
> > + sysfs_notify(&gadget->dev.kobj, NULL, "status");
> > +}
> > +EXPORT_SYMBOL_GPL(usb_gadget_set_state);
> > +
> > +/*
> > ------------------------------------------------------------------------- */
> > +
> > /**
> > * usb_gadget_udc_start - tells usb device controller to start up
> > * @gadget: The gadget we want to get started
> > @@ -407,6 +447,17 @@ static ssize_t usb_udc_softconn_store(struct device
> > *dev,
> > }
> > static DEVICE_ATTR(soft_connect, S_IWUSR, NULL, usb_udc_softconn_store);
> >
> > +static ssize_t usb_gadget_state_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct usb_udc *udc = container_of(dev, struct usb_udc, dev);
> > + struct usb_gadget *gadget = udc->gadget;
> > +
> > + return snprintf(buf, PAGE_SIZE, "%s\n",
> > + usb_device_state_string(gadget->state));
>
> How about just sprintf? If the state string overruns a page, something
> is badly wrong anyway.
can do.
> > --- a/include/linux/usb/gadget.h
> > +++ b/include/linux/usb/gadget.h
> > @@ -24,6 +24,8 @@
> > #include <linux/types.h>
> > #include <linux/usb/ch9.h>
> >
> > +#include <uapi/linux/usb/ch9.h>
>
> Again, not needed.
no indirect inclusion.
--
balbi
signature.asc
Description: Digital signature
