* Michal Nazarewicz | 2011-05-31 18:13:23 [+0200]:
>On Mon, 30 May 2011 19:28:34 +0200, Sebastian Andrzej Siewior wrote:
>
>>this class will be used to abstract away
>>several of the duplicated operations scatered
>
>scattered
fixed
>>among the USB gadget controller drivers.
>>
>>Later, we can add an atomic notifer to tell
>
>notifier
fixed
>>interested drivers about what's happening
>>with the controller. Notifications such as
>>suspend, resume, enumerated, etc will be
>
>etc. (ie. with dot)
fixed
>>+static void usb_gadget_remove_driver(struct usb_udc *udc)
>
>I'd call it __usb_gadget_remove_driver() (ie. with "__"), but
>that's just me.
I think I keep it the way it is.
>
>>+int usb_gadget_register_driver(struct usb_gadget_driver *driver)
>>+{
>>+ struct usb_udc *udc = NULL;
>>+ int ret;
>>+
>>+ if (!driver || !driver->bind || !driver->setup)
>>+ return -EINVAL;
>>+
>>+ mutex_lock(&udc_lock);
>>+ list_for_each_entry(udc, &udc_list, list)
>>+ /* For now we take the first one */
>>+ if (!udc->driver) {
>>+ pr_debug("using UDC '%s'\n", udc->gadget->name);
>
>pr_debug() not needed.
removed.
>>+ goto found;
>>+ }
>>+
>>+ pr_debug("couldn't find an available UDC\n");
>>+ mutex_unlock(&udc_lock);
>>+ return -ENODEV;
>>+
>>+found:
>>+ dev_dbg(&udc->dev, "registering UDC driver [%s]\n",
>>+ driver->function);
>>+
>>+ udc->driver = driver;
>>+ udc->dev.driver = &driver->driver;
>>+
>>+ ret = usb_gadget_start(udc->gadget, driver);
>>+ if (ret) {
>>+ dev_err(&udc->dev, "failed to start %s --> %d\n",
>>+ udc->driver->function, ret);
>
>I'd put the dev_err() after err1 label instead of here, but that's just me.
moved.
>>+static ssize_t usb_udc_srp_store(struct device *dev,
>>+ struct device_attribute *attr, const char *buf, size_t n)
>>+{
>>+ struct usb_udc *udc = dev_get_drvdata(dev);
>>+
>>+ if (sysfs_streq(buf, "1"))
>>+ usb_gadget_wakeup(udc->gadget);
>
>I'd use kzstrtoul() or something but that looks good to.
Okay. I keep it then. Your had the adventage of accepting 01 or 0x1 but
I don't know if we want this.
>>+static ssize_t usb_udc_speed_show(struct device *dev,
>>+ struct device_attribute *attr, char *buf)
>>+{
>>+ struct usb_udc *udc = dev_get_drvdata(dev);
>>+ struct usb_gadget *gadget = udc->gadget;
>>+
>>+ switch (gadget->speed) {
>>+ case USB_SPEED_LOW:
>>+ return snprintf(buf, PAGE_SIZE, "low-speed\n");
>>+ case USB_SPEED_FULL:
>>+ return snprintf(buf, PAGE_SIZE, "full-speed\n");
>>+ case USB_SPEED_HIGH:
>>+ return snprintf(buf, PAGE_SIZE, "high-speed\n");
>>+ case USB_SPEED_WIRELESS:
>>+ return snprintf(buf, PAGE_SIZE, "wireless\n");
>>+ case USB_SPEED_SUPER:
>>+ return snprintf(buf, PAGE_SIZE, "super-speed\n");
>>+ case USB_SPEED_UNKNOWN: /* FALLTHROUGH */
>>+ default:
>>+ return snprintf(buf, PAGE_SIZE, "UNKNOWN\n");
>>+ }
>>+}
>
>Isn't there some function for returning speed of the gadget as
>a string? I remember seeing a patch that added such a function.
>I might be wrong though.
I've been looking and everybody is doing it on its own. So I keep it for
now and make a public USB function later out of it.
>>+static int usb_udc_uevent(struct device *dev, struct
>>kobj_uevent_env *env)
>
>Would it make sense to export it? Just asking.
No, this should remain udc-class-local. I'm trying to get rid of common
code out of drivers.
>>+static void __exit usb_udc_exit(void)
>>+{
>>+ class_destroy(udc_class);
>>+ udc_class = NULL;
>
>That's not needed.
Indeed, gone.
Sebastian
--
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
