[RFC 2/2] usb: gadget: composite: parse dt overrides

Subsystems: the rest, usb subsystem

6 messages, 4 authors, 2012-06-26 · open the first message on its own page

[RFC 2/2] usb: gadget: composite: parse dt overrides

From: Alexandre Pereira da Silva <hidden>
Date: 2012-06-25 20:23:51

Grab the devicetree node properties to override VendorId, ProductId,
bcdDevice, Manucacturer, Product and SerialNumber

Signed-off-by: Alexandre Pereira da Silva <redacted>
---
 drivers/usb/gadget/composite.c |   31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 390749b..f3b480e 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/utsname.h>
+#include <linux/of.h>
 
 #include <linux/usb/composite.h>
 #include <asm/unaligned.h>
@@ -1423,6 +1424,7 @@ static int composite_bind(struct usb_gadget *gadget)
 {
 	struct usb_composite_dev	*cdev;
 	int				status = -ENOMEM;
+	struct device_node		*np = gadget->dev.of_node;
 
 	cdev = kzalloc(sizeof *cdev, GFP_KERNEL);
 	if (!cdev)
@@ -1470,6 +1472,35 @@ static int composite_bind(struct usb_gadget *gadget)
 
 	cdev->desc = *composite->dev;
 
+	/* grab overrides from devicetree */
+	if (np) {
+		u32 reg;
+
+		if (!idVendor &&
+			of_property_read_u32(np, "vendor_id", &reg) == 0)
+			idVendor = reg;
+
+		if (!idProduct &&
+			of_property_read_u32(np, "product_id", &reg) == 0)
+			idProduct = reg;
+
+		if (!bcdDevice &&
+			of_property_read_u32(np, "bcd_device", &reg) == 0)
+			bcdDevice = reg;
+
+		if (!iManufacturer)
+			of_property_read_string(np, "manufacturer",
+				&iManufacturer);
+
+		if (!iProduct)
+			of_property_read_string(np, "product",
+				&iProduct);
+
+		if (!iSerialNumber)
+			of_property_read_string(np, "serial_number",
+				&iSerialNumber);
+	}
+
 	/* standardized runtime overrides for device ID data */
 	if (idVendor)
 		cdev->desc.idVendor = cpu_to_le16(idVendor);
-- 
1.7.10

Re: [RFC 2/2] usb: gadget: composite: parse dt overrides

From: Michal Nazarewicz <hidden>
Date: 2012-06-25 21:30:42

On Mon, 25 Jun 2012 22:23:52 +0200, Alexandre Pereira da Silva [off-list ref] wrote:
Grab the devicetree node properties to override VendorId, ProductId,
bcdDevice, Manucacturer, Product and SerialNumber

Signed-off-by: Alexandre Pereira da Silva <redacted>
Acked-by: Michal Nazarewicz <redacted>
quoted hunk
@@ -1470,6 +1472,35 @@ static int composite_bind(struct usb_gadget *gadget)
	cdev->desc = *composite->dev;
+	/* grab overrides from devicetree */
+	if (np) {
+		u32 reg;
+
+		if (!idVendor &&
+			of_property_read_u32(np, "vendor_id", &reg) == 0)
+			idVendor = reg;
+
+		if (!idProduct &&
+			of_property_read_u32(np, "product_id", &reg) == 0)
+			idProduct = reg;
+
+		if (!bcdDevice &&
+			of_property_read_u32(np, "bcd_device", &reg) == 0)
+			bcdDevice = reg;
+
+		if (!iManufacturer)
+			of_property_read_string(np, "manufacturer",
+				&iManufacturer);
+
+		if (!iProduct)
+			of_property_read_string(np, "product",
+				&iProduct);
+
+		if (!iSerialNumber)
+			of_property_read_string(np, "serial_number",
+				&iSerialNumber);
+	}
+
 	/* standardized runtime overrides for device ID data */
 	if (idVendor)
 		cdev->desc.idVendor = cpu_to_le16(idVendor);

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

Re: [RFC 2/2] usb: gadget: composite: parse dt overrides

From: Rob Herring <hidden>
Date: 2012-06-25 23:49:36

On 06/25/2012 03:23 PM, Alexandre Pereira da Silva wrote:
Grab the devicetree node properties to override VendorId, ProductId,
bcdDevice, Manucacturer, Product and SerialNumber

Signed-off-by: Alexandre Pereira da Silva <redacted>
---
 drivers/usb/gadget/composite.c |   31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
Are these bindings documented? I think they should be less generic.
Perhaps prefixed with 'usb-'.
quoted hunk
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 390749b..f3b480e 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/utsname.h>
+#include <linux/of.h>
 
 #include <linux/usb/composite.h>
 #include <asm/unaligned.h>
@@ -1423,6 +1424,7 @@ static int composite_bind(struct usb_gadget *gadget)
 {
 	struct usb_composite_dev	*cdev;
 	int				status = -ENOMEM;
+	struct device_node		*np = gadget->dev.of_node;
 
 	cdev = kzalloc(sizeof *cdev, GFP_KERNEL);
 	if (!cdev)
@@ -1470,6 +1472,35 @@ static int composite_bind(struct usb_gadget *gadget)
 
 	cdev->desc = *composite->dev;
 
+	/* grab overrides from devicetree */
Reading the code, it looks more like the DT entries are defaults rather
than overrides.
+	if (np) {
+		u32 reg;
+
+		if (!idVendor &&
+			of_property_read_u32(np, "vendor_id", &reg) == 0)
+			idVendor = reg;
if (!idVendor)
	of_property_read_u32(np, "vendor_id", &idVendor);

Rob
+
+		if (!idProduct &&
+			of_property_read_u32(np, "product_id", &reg) == 0)
+			idProduct = reg;
+
+		if (!bcdDevice &&
+			of_property_read_u32(np, "bcd_device", &reg) == 0)
+			bcdDevice = reg;
+
+		if (!iManufacturer)
+			of_property_read_string(np, "manufacturer",
+				&iManufacturer);
+
+		if (!iProduct)
+			of_property_read_string(np, "product",
+				&iProduct);
+
+		if (!iSerialNumber)
+			of_property_read_string(np, "serial_number",
+				&iSerialNumber);
+	}
+
 	/* standardized runtime overrides for device ID data */
 	if (idVendor)
 		cdev->desc.idVendor = cpu_to_le16(idVendor);

Re: [RFC 2/2] usb: gadget: composite: parse dt overrides

From: Alexandre Pereira da Silva <hidden>
Date: 2012-06-26 01:13:01

On Mon, Jun 25, 2012 at 8:49 PM, Rob Herring [off-list ref] wrote:
On 06/25/2012 03:23 PM, Alexandre Pereira da Silva wrote:
quoted
Grab the devicetree node properties to override VendorId, ProductId,
bcdDevice, Manucacturer, Product and SerialNumber

Signed-off-by: Alexandre Pereira da Silva <redacted>
---
 drivers/usb/gadget/composite.c |   31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
Are these bindings documented? I think they should be less generic.
Perhaps prefixed with 'usb-'.
Not yet, I will in the final series. This was just to see it had big
issues because the final one needs to change lots of files.
I will add some prefix to them.

About the documentation, should I include it in all of the controllers
bindings or should I add a common file to describe the gadget drivers
binding?
quoted
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 390749b..f3b480e 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/utsname.h>
+#include <linux/of.h>

 #include <linux/usb/composite.h>
 #include <asm/unaligned.h>
@@ -1423,6 +1424,7 @@ static int composite_bind(struct usb_gadget *gadget)
 {
      struct usb_composite_dev        *cdev;
      int                             status = -ENOMEM;
+     struct device_node              *np = gadget->dev.of_node;

      cdev = kzalloc(sizeof *cdev, GFP_KERNEL);
      if (!cdev)
@@ -1470,6 +1472,35 @@ static int composite_bind(struct usb_gadget *gadget)
      cdev->desc = *composite->dev;

+     /* grab overrides from devicetree */
Reading the code, it looks more like the DT entries are defaults rather
than overrides.
Actually, it's mixed. The DT overrides the hard coded defaults. And
module parameters can override both. I think the safest path is to
keep them like this, so a DT would not break existing code.
quoted
+     if (np) {
+             u32 reg;
+
+             if (!idVendor &&
+                     of_property_read_u32(np, "vendor_id", &reg) == 0)
+                     idVendor = reg;
if (!idVendor)
       of_property_read_u32(np, "vendor_id", &idVendor);

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 2/2] usb: gadget: composite: parse dt overrides

From: Mitch Bradley <hidden>
Date: 2012-06-26 05:38:57

On 6/25/2012 1:49 PM, Rob Herring wrote:
On 06/25/2012 03:23 PM, Alexandre Pereira da Silva wrote:
quoted
Grab the devicetree node properties to override VendorId, ProductId,
bcdDevice, Manucacturer, Product and SerialNumber

Signed-off-by: Alexandre Pereira da Silva <redacted>
---
  drivers/usb/gadget/composite.c |   31 +++++++++++++++++++++++++++++++
  1 file changed, 31 insertions(+)
Are these bindings documented? I think they should be less generic.
Perhaps prefixed with 'usb-'.
There is precedent for the following properties in the USB node, namely 
the device tree that has been used on OLPC systems for the past several 
years.  Also, I think that Sun machines from the same timeframe use the 
same property names, based on the fact that Sun is using my USB 2.0 OFW 
driver code.

Here's a listing of the properties of a representative USB child device, 
in this case a USB FLASH drive.

configuration#           00000001
bulk-out-size            00000200
bulk-out-pipe            00000001
bulk-in-size             00000200
bulk-in-pipe             00000001
serial$                  200435137016ae938861
device$                  Cruzer Mini
vendor$                  SanDisk Corporation
compatible               usb781,5150.20
                          usb781,5150
                          usbif781,class8.6.50
                          usbif781,class8.6
                          usbif781,class8
                          usbif,class8.6.50
                          usbif,class8.6
                          usbif,class8
                          usb,device
vendor-id                00000781
device-id                00005150
release                  00000020
name                     scsi
class                    00000008
subclass                 00000006
protocol                 00000050
high-speed
assigned-address         00000002
reg                      00000003 00000000
#size-cells              00000000
#address-cells           00000001

Note that:

a) The separator in "vendor-id" is hyphen, not underscore, in keeping 
with the established property naming convention (except for the 
unfortunate "device_type").

b) There is no "usb," prefix, since the presence of this in a device 
node that is a child of a USB bus implicitly identifies the kind of ID. 
  "vendor-id" and "device-id" in PCI bus child devices is similarly 
un-prefixed.

c) Some of these property names are defined in 
http://www.openfirmware.org/ofwg/bindings/usb/usb-1_0.ps



quoted
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 390749b..f3b480e 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -17,6 +17,7 @@
  #include <linux/module.h>
  #include <linux/device.h>
  #include <linux/utsname.h>
+#include <linux/of.h>

  #include <linux/usb/composite.h>
  #include <asm/unaligned.h>
@@ -1423,6 +1424,7 @@ static int composite_bind(struct usb_gadget *gadget)
  {
  	struct usb_composite_dev	*cdev;
  	int				status = -ENOMEM;
+	struct device_node		*np = gadget->dev.of_node;

  	cdev = kzalloc(sizeof *cdev, GFP_KERNEL);
  	if (!cdev)
@@ -1470,6 +1472,35 @@ static int composite_bind(struct usb_gadget *gadget)

  	cdev->desc = *composite->dev;

+	/* grab overrides from devicetree */
Reading the code, it looks more like the DT entries are defaults rather
than overrides.
quoted
+	if (np) {
+		u32 reg;
+
+		if (!idVendor &&
+			of_property_read_u32(np, "vendor_id", &reg) == 0)
+			idVendor = reg;
if (!idVendor)
	of_property_read_u32(np, "vendor_id", &idVendor);

Rob
quoted
+
+		if (!idProduct &&
+			of_property_read_u32(np, "product_id", &reg) == 0)
+			idProduct = reg;
+
+		if (!bcdDevice &&
+			of_property_read_u32(np, "bcd_device", &reg) == 0)
+			bcdDevice = reg;
+
+		if (!iManufacturer)
+			of_property_read_string(np, "manufacturer",
+				&iManufacturer);
+
+		if (!iProduct)
+			of_property_read_string(np, "product",
+				&iProduct);
+
+		if (!iSerialNumber)
+			of_property_read_string(np, "serial_number",
+				&iSerialNumber);
+	}
+
  	/* standardized runtime overrides for device ID data */
  	if (idVendor)
  		cdev->desc.idVendor = cpu_to_le16(idVendor);
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [RFC 2/2] usb: gadget: composite: parse dt overrides

From: Rob Herring <hidden>
Date: 2012-06-26 14:04:40

On 06/26/2012 12:38 AM, Mitch Bradley wrote:
On 6/25/2012 1:49 PM, Rob Herring wrote:
quoted
On 06/25/2012 03:23 PM, Alexandre Pereira da Silva wrote:
quoted
Grab the devicetree node properties to override VendorId, ProductId,
bcdDevice, Manucacturer, Product and SerialNumber

Signed-off-by: Alexandre Pereira da Silva <redacted>
---
  drivers/usb/gadget/composite.c |   31 +++++++++++++++++++++++++++++++
  1 file changed, 31 insertions(+)
Are these bindings documented? I think they should be less generic.
Perhaps prefixed with 'usb-'.
There is precedent for the following properties in the USB node, namely
the device tree that has been used on OLPC systems for the past several
years.  Also, I think that Sun machines from the same timeframe use the
same property names, based on the fact that Sun is using my USB 2.0 OFW
driver code.
Thanks, I wasn't aware of that.
Here's a listing of the properties of a representative USB child device,
in this case a USB FLASH drive.

configuration#           00000001
bulk-out-size            00000200
bulk-out-pipe            00000001
bulk-in-size             00000200
bulk-in-pipe             00000001
serial$                  200435137016ae938861
device$                  Cruzer Mini
vendor$                  SanDisk Corporation
compatible               usb781,5150.20
                         usb781,5150
                         usbif781,class8.6.50
                         usbif781,class8.6
                         usbif781,class8
                         usbif,class8.6.50
                         usbif,class8.6
                         usbif,class8
                         usb,device
vendor-id                00000781
device-id                00005150
release                  00000020
name                     scsi
class                    00000008
subclass                 00000006
protocol                 00000050
high-speed
assigned-address         00000002
reg                      00000003 00000000
#size-cells              00000000
#address-cells           00000001

Note that:

a) The separator in "vendor-id" is hyphen, not underscore, in keeping
with the established property naming convention (except for the
unfortunate "device_type").

b) There is no "usb," prefix, since the presence of this in a device
node that is a child of a USB bus implicitly identifies the kind of ID.
 "vendor-id" and "device-id" in PCI bus child devices is similarly
un-prefixed.
Okay.
c) Some of these property names are defined in
http://www.openfirmware.org/ofwg/bindings/usb/usb-1_0.ps
We should refer back to this as appropriate, but document what is not
standard. Having an example is useful as well. The original doc was
about describing a device to the host. This case is describing the
device to the device. Perhaps these are the same, but if there are
differences we need to capture that.

Rob

quoted
quoted
diff --git a/drivers/usb/gadget/composite.c
b/drivers/usb/gadget/composite.c
index 390749b..f3b480e 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -17,6 +17,7 @@
  #include <linux/module.h>
  #include <linux/device.h>
  #include <linux/utsname.h>
+#include <linux/of.h>

  #include <linux/usb/composite.h>
  #include <asm/unaligned.h>
@@ -1423,6 +1424,7 @@ static int composite_bind(struct usb_gadget
*gadget)
  {
      struct usb_composite_dev    *cdev;
      int                status = -ENOMEM;
+    struct device_node        *np = gadget->dev.of_node;

      cdev = kzalloc(sizeof *cdev, GFP_KERNEL);
      if (!cdev)
@@ -1470,6 +1472,35 @@ static int composite_bind(struct usb_gadget
*gadget)

      cdev->desc = *composite->dev;

+    /* grab overrides from devicetree */
Reading the code, it looks more like the DT entries are defaults rather
than overrides.
quoted
+    if (np) {
+        u32 reg;
+
+        if (!idVendor &&
+            of_property_read_u32(np, "vendor_id", &reg) == 0)
+            idVendor = reg;
if (!idVendor)
    of_property_read_u32(np, "vendor_id", &idVendor);

Rob
quoted
+
+        if (!idProduct &&
+            of_property_read_u32(np, "product_id", &reg) == 0)
+            idProduct = reg;
+
+        if (!bcdDevice &&
+            of_property_read_u32(np, "bcd_device", &reg) == 0)
+            bcdDevice = reg;
+
+        if (!iManufacturer)
+            of_property_read_string(np, "manufacturer",
+                &iManufacturer);
+
+        if (!iProduct)
+            of_property_read_string(np, "product",
+                &iProduct);
+
+        if (!iSerialNumber)
+            of_property_read_string(np, "serial_number",
+                &iSerialNumber);
+    }
+
      /* standardized runtime overrides for device ID data */
      if (idVendor)
          cdev->desc.idVendor = cpu_to_le16(idVendor);
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help