On Mon, Aug 27, 2012 at 04:33:05PM +0200, Michal Nazarewicz wrote:
> PS. It just struck me, is there a reason for
> USB_GADGET_COMPOSITE_OVERWRITE_OPTIONS to be a macro?  It could be just
> an exported function.  I'm thinking something along the lines of:
> 
> 
> struct usb_composite_overwrites {
>       ushort idVendor, idProduct, bcdDevice;
>       const char *serial, *manufacturer, *product;
> };
> 
> #define USB_GADGET_COMPOSITE_OPTIONS()                                        
> \
>       static struct usb_composite_overwrites _usb_composite_overwrites; \
>       module_param_named(idVendor, _usb_composite_overwrites.idVendor, \
>                          ushort, S_IRUGO);                            \
>       /* ... and so on ... */                                         \
>       module_param_named(iProduct, _usb_composite_overwrites.product); \
>       MODULE_PARM_DESC(iProduct, "product name")
> 
> static void usb_composite_overwrite_strings(
>       struct usb_composite_overwrites *overwrites,
>       struct usb_composite_dev *cdev,
>       struct usb_string *strings,
>       unsigned idx)
> {
>       struct usb_device_descriptor *desc = cdev->desc;
> 
>       if (overwrites->serial)
>               strings[idx].s = overwrites->serial;
>       if (strings[idx].s[0])
>               desc.iStringNumber = strings[idx].id;
> 
>       if (overwrites->manufacturer)
>               strings[idx+1].s = overwrites->manufacturer;
>       else if (!strings[idx+1].s[0])
>               strings[idx+1].s =
>                       cdev->gadget->default_manufacturer;
>       desc.iManufacturer = strings[idx+1];
> 
>       if (overwrites->product)
>               strings[idx+2].s = overwrites->product;
>       else if (!strings[idx+2].s[0])
>               strings[idx+2].s = cdev->driver->name;
>       desc.iProduct = strings[idx+2].id;
> }
> 
> void usb_composite_overwrite_options(
>       struct usb_composite_overwrites *overwrites,
>       struct usb_composite_dev *cdev,
>       unsigned idx)
> {
>       struct usb_device_descriptor *desc = cdev->desc;
>       struct usb_gadget_strings **strings;
> 
>       if (overwrites->idVendor)
>               desc->idVendor = cpu_to_le16(overwrites->idVendor);
>       if (overwrites->idProduct)
>               desc->idProduct = cpu_to_le16(overwrites->idProduct);
>       if (overwrites->bcdDevice)
>               desc->bcdDevice = cpu_to_le16(overwrites->bcdDevice);
> 
>       for (strings = cdev->driver->strings; *strings; ++strings)
>               usb_composite_overwrite_strings(overwrites, cdev,
>                                               (*strings)->strings,
>                                               idx);
> }
> EXPORT_SYMBOL_GPL(usb_composite_overwrite_options);
> 
> #define USB_COMPOSITE_OVERWIRET_OPTIONS(cdev, idx) \
>       usb_composite_overwrite_options(_usb_composite_overwrites, cdev, idx)
> 
> 
> Note that in the above, I'm proposing to include the
> default_manufacturer array to usb_gadget structure so that UDC can
> initialise it and than it can be used by anyone who wishes to do so.

After going back and foth I think I can make friends with this.

> 
> 
> PS.  usb_composite_driver has iProduct, iManufacturer and iSerialNumber
> fields but with the amount of refactoring you are doing, I think they
> could be removed in favour of composite gadgets specifying their desired
> defaults in array of usb_strings.

that was the plan more or less. I have the configfs idea in back of the head
and I don't know where to put it. I think once I come along with something to
handle the individual functions things should become clear…

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

Reply via email to