* [RFC 2/2] usb: gadget: composite: parse dt overrides
@ 2012-06-25 20:23 Alexandre Pereira da Silva
2012-06-25 21:30 ` Michal Nazarewicz
2012-06-25 23:49 ` Rob Herring
0 siblings, 2 replies; 6+ messages in thread
From: Alexandre Pereira da Silva @ 2012-06-25 20:23 UTC (permalink / raw)
To: Felipe Balbi, Greg Kroah-Hartman, linux-usb, linux-kernel,
Grant Likely, Rob Herring, devicetree-discuss
Cc: Alexandre Pereira da Silva
Grab the devicetree node properties to override VendorId, ProductId,
bcdDevice, Manucacturer, Product and SerialNumber
Signed-off-by: Alexandre Pereira da Silva <aletes.xgr@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC 2/2] usb: gadget: composite: parse dt overrides
2012-06-25 20:23 [RFC 2/2] usb: gadget: composite: parse dt overrides Alexandre Pereira da Silva
@ 2012-06-25 21:30 ` Michal Nazarewicz
2012-06-25 23:49 ` Rob Herring
1 sibling, 0 replies; 6+ messages in thread
From: Michal Nazarewicz @ 2012-06-25 21:30 UTC (permalink / raw)
To: Felipe Balbi, Greg Kroah-Hartman, linux-usb, linux-kernel,
Grant Likely, Rob Herring, devicetree-discuss,
Alexandre Pereira da Silva
On Mon, 25 Jun 2012 22:23:52 +0200, Alexandre Pereira da Silva <aletes.xgr@gmail.com> wrote:
> Grab the devicetree node properties to override VendorId, ProductId,
> bcdDevice, Manucacturer, Product and SerialNumber
>
> Signed-off-by: Alexandre Pereira da Silva <aletes.xgr@gmail.com>
Acked-by: Michal Nazarewicz <mina86@mina86.com>
> @@ -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: mpn@google.com>--------------ooO--(_)--Ooo--
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC 2/2] usb: gadget: composite: parse dt overrides
2012-06-25 20:23 [RFC 2/2] usb: gadget: composite: parse dt overrides Alexandre Pereira da Silva
2012-06-25 21:30 ` Michal Nazarewicz
@ 2012-06-25 23:49 ` Rob Herring
[not found] ` <4FE8F90B.2010303-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2012-06-26 5:38 ` Mitch Bradley
1 sibling, 2 replies; 6+ messages in thread
From: Rob Herring @ 2012-06-25 23:49 UTC (permalink / raw)
To: Alexandre Pereira da Silva
Cc: Felipe Balbi, Greg Kroah-Hartman, linux-usb, linux-kernel,
Grant Likely, devicetree-discuss
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 <aletes.xgr@gmail.com>
> ---
> 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);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC 2/2] usb: gadget: composite: parse dt overrides
[not found] ` <4FE8F90B.2010303-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2012-06-26 1:12 ` Alexandre Pereira da Silva
0 siblings, 0 replies; 6+ messages in thread
From: Alexandre Pereira da Silva @ 2012-06-26 1:12 UTC (permalink / raw)
To: Rob Herring
Cc: Felipe Balbi, Greg Kroah-Hartman,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Grant Likely,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
On Mon, Jun 25, 2012 at 8:49 PM, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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 <aletes.xgr-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> 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
--
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC 2/2] usb: gadget: composite: parse dt overrides
2012-06-25 23:49 ` Rob Herring
[not found] ` <4FE8F90B.2010303-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2012-06-26 5:38 ` Mitch Bradley
2012-06-26 14:04 ` Rob Herring
1 sibling, 1 reply; 6+ messages in thread
From: Mitch Bradley @ 2012-06-26 5:38 UTC (permalink / raw)
To: Rob Herring
Cc: Alexandre Pereira da Silva, Greg Kroah-Hartman,
devicetree-discuss, linux-usb, linux-kernel, Felipe Balbi
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 <aletes.xgr@gmail.com>
>> ---
>> 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
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC 2/2] usb: gadget: composite: parse dt overrides
2012-06-26 5:38 ` Mitch Bradley
@ 2012-06-26 14:04 ` Rob Herring
0 siblings, 0 replies; 6+ messages in thread
From: Rob Herring @ 2012-06-26 14:04 UTC (permalink / raw)
To: Mitch Bradley
Cc: Alexandre Pereira da Silva, Greg Kroah-Hartman,
devicetree-discuss, linux-usb, linux-kernel, Felipe Balbi
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 <aletes.xgr@gmail.com>
>>> ---
>>> 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
>> devicetree-discuss@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/devicetree-discuss
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-06-26 14:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-25 20:23 [RFC 2/2] usb: gadget: composite: parse dt overrides Alexandre Pereira da Silva
2012-06-25 21:30 ` Michal Nazarewicz
2012-06-25 23:49 ` Rob Herring
[not found] ` <4FE8F90B.2010303-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2012-06-26 1:12 ` Alexandre Pereira da Silva
2012-06-26 5:38 ` Mitch Bradley
2012-06-26 14:04 ` Rob Herring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).