Re: [PATCH 7/9] usb: gadget: Add Android Composite Gadget driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux