Grab the devicetree node properties to override VendorId, ProductId,
bcdDevice, Manucacturer, Product and SerialNumber
Signed-off-by: Alexandre Pereira da Silva <[email protected]>
---
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", ®) == 0)
+ idVendor = reg;
+
+ if (!idProduct &&
+ of_property_read_u32(np, "product_id", ®) == 0)
+ idProduct = reg;
+
+ if (!bcdDevice &&
+ of_property_read_u32(np, "bcd_device", ®) == 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
On Mon, 25 Jun 2012 22:23:52 +0200, Alexandre Pereira da Silva <[email protected]> wrote:
> Grab the devicetree node properties to override VendorId, ProductId,
> bcdDevice, Manucacturer, Product and SerialNumber
>
> Signed-off-by: Alexandre Pereira da Silva <[email protected]>
Acked-by: Michal Nazarewicz <[email protected]>
> @@ -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", ®) == 0)
> + idVendor = reg;
> +
> + if (!idProduct &&
> + of_property_read_u32(np, "product_id", ®) == 0)
> + idProduct = reg;
> +
> + if (!bcdDevice &&
> + of_property_read_u32(np, "bcd_device", ®) == 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: [email protected]>--------------ooO--(_)--Ooo--
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 <[email protected]>
> ---
> 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-'.
>
> 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", ®) == 0)
> + idVendor = reg;
if (!idVendor)
of_property_read_u32(np, "vendor_id", &idVendor);
Rob
> +
> + if (!idProduct &&
> + of_property_read_u32(np, "product_id", ®) == 0)
> + idProduct = reg;
> +
> + if (!bcdDevice &&
> + of_property_read_u32(np, "bcd_device", ®) == 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);
On Mon, Jun 25, 2012 at 8:49 PM, Rob Herring <[email protected]> wrote:
> 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 <[email protected]>
>> ---
>> ?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?
>>
>> 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.
>> + ? ? if (np) {
>> + ? ? ? ? ? ? u32 reg;
>> +
>> + ? ? ? ? ? ? if (!idVendor &&
>> + ? ? ? ? ? ? ? ? ? ? of_property_read_u32(np, "vendor_id", ®) == 0)
>> + ? ? ? ? ? ? ? ? ? ? idVendor = reg;
>
> if (!idVendor)
> ? ? ? ?of_property_read_u32(np, "vendor_id", &idVendor);
>
> Rob
On 6/25/2012 1:49 PM, Rob Herring wrote:
> 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 <[email protected]>
>> ---
>> 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
>
>>
>> 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", ®) == 0)
>> + idVendor = reg;
>
> if (!idVendor)
> of_property_read_u32(np, "vendor_id", &idVendor);
>
> Rob
>
>> +
>> + if (!idProduct &&
>> + of_property_read_u32(np, "product_id", ®) == 0)
>> + idProduct = reg;
>> +
>> + if (!bcdDevice &&
>> + of_property_read_u32(np, "bcd_device", ®) == 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
> [email protected]
> https://lists.ozlabs.org/listinfo/devicetree-discuss
>
On 06/26/2012 12:38 AM, Mitch Bradley wrote:
> On 6/25/2012 1:49 PM, Rob Herring wrote:
>> 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 <[email protected]>
>>> ---
>>> 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
>
>
>
>>
>>>
>>> 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", ®) == 0)
>>> + idVendor = reg;
>>
>> if (!idVendor)
>> of_property_read_u32(np, "vendor_id", &idVendor);
>>
>> Rob
>>
>>> +
>>> + if (!idProduct &&
>>> + of_property_read_u32(np, "product_id", ®) == 0)
>>> + idProduct = reg;
>>> +
>>> + if (!bcdDevice &&
>>> + of_property_read_u32(np, "bcd_device", ®) == 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
>> [email protected]
>> https://lists.ozlabs.org/listinfo/devicetree-discuss
>>
>