On Mon, 23 Apr 2012 13:36:23 +0200, Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx> wrote:
+static ssize_t rndis_manufacturer_store(struct device *dev, + struct device_attribute *attr, const char *buf, size_t size) +{ + struct android_usb_function *f = dev_get_drvdata(dev); + struct rndis_function_config *config = f->config; + + if (size >= sizeof(config->manufacturer)) + return -EINVAL; + memcpy(config->manufacturer, buf, size);
Is NUL byte handled here correctly? I feel it should read “size +1” in the memcpy() call or there should be “config->manufacturer[size] = 0;” somewhere.
+ + return size; +}
+static int android_init_functions(struct android_usb_function **functions, + struct usb_composite_dev *cdev) +{ + struct android_dev *dev = _android_dev; + struct android_usb_function *f; + struct device_attribute **attrs; + struct device_attribute *attr; + int err; + int index = 0; + + for (; (f = *functions++); index++) { + f->dev_name = kasprintf(GFP_KERNEL, "f_%s", f->name); + if (!f->dev_name) { + pr_err("%s: Failed to alloc name %s", __func__, + f->name); + err = -ENOMEM; + goto err_alloc; + } + f->dev = device_create(android_class, dev->dev, + MKDEV(0, index), f, f->dev_name); + if (IS_ERR(f->dev)) { + pr_err("%s: Failed to create dev %s", __func__, + f->dev_name); + err = PTR_ERR(f->dev); + f->dev = NULL; + goto err_create; + } + + if (f->init) { + err = f->init(f, cdev); + if (err) { + pr_err("%s: Failed to init %s", __func__, + f->name); + goto err_out; + } + } + + attrs = f->attributes; + if (attrs) { + while ((attr = *attrs++) && !err) + err = device_create_file(f->dev, attr); + }
Have you looked at device_add_attributes()? This would require a few changes in the f->attributes arrays though. I'm leaving it up to you.
+ if (err) { + pr_err("%s: Failed to create function %s attributes", + __func__, f->name); + goto err_uninit; + } + } + return 0; + +err_uninit: + if (f->cleanup) + f->cleanup(f); +err_out: + device_destroy(android_class, f->dev->devt); + f->dev = NULL; +err_create: + kfree(f->dev_name); +err_alloc: + return err; +}
+DESCRIPTOR_ATTR(idVendor, "%04x\n") +DESCRIPTOR_ATTR(idProduct, "%04x\n") +DESCRIPTOR_ATTR(bcdDevice, "%04x\n") +DESCRIPTOR_ATTR(bDeviceClass, "%d\n") +DESCRIPTOR_ATTR(bDeviceSubClass, "%d\n") +DESCRIPTOR_ATTR(bDeviceProtocol, "%d\n") +DESCRIPTOR_STRING_ATTR(iManufacturer, manufacturer_string) +DESCRIPTOR_STRING_ATTR(iProduct, product_string) +DESCRIPTOR_STRING_ATTR(iSerial, serial_string)
As described earlier, I'm not comfortable with this approach. I feel it should be made a part of composite.c. If I'm not mistaken, module_param() can be made writeable by changing the last argument from 0 to 0644. If that's the case, idVendor, idProduct, bcdDevice, iManufacturer, iProduct an iSerial can most likely be handled in composite.c with just a few changes to the code. I feel like the fact that android composite gadget has handling of those is because it was created before handling of those parameters in composite.c and then it was never updated (which is not surprising for out-of-tree code). -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz (o o) ooo +----<email/xmpp: mpn@xxxxxxxxxx>--------------ooO--(_)--Ooo-- -- 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