[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <4FE9C172.5060306@gmail.com>
Date: Tue, 26 Jun 2012 09:04:34 -0500
From: Rob Herring <robherring2@...il.com>
To: Mitch Bradley <wmb@...mworks.com>
CC: Alexandre Pereira da Silva <aletes.xgr@...il.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
devicetree-discuss@...ts.ozlabs.org, linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org, Felipe Balbi <balbi@...com>
Subject: Re: [RFC 2/2] usb: gadget: composite: parse dt overrides
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@...il.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@...ts.ozlabs.org
>> https://lists.ozlabs.org/listinfo/devicetree-discuss
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists