Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> writes:
> On 08/25/2012 12:11 AM, Michal Nazarewicz wrote:
>> Sebastian Andrzej Siewior<bigeasy@xxxxxxxxxxxxx> writes:
>>> This patch pushes the iManufacturer module argument from composite into
>>> each gadget. Once the user uses the module paramter, the string is
>>> overwritten with the final value.
>>
>> Why to remove the generation of the string from those gadgets and let
>> composite handle this?
>
> This could be done, yes. There are a few gadgets (2 or 3 or so) which
> have a custom entry. Webcam is one of those. I don't think this entry
> is *very* important so I could remove it and let composite handle it.
I've actually meant leaving the code more or less as it is.
But with the addition of the strings to gadget's strings_dev, this (I
feel) could be done in a generic way I feel:
if (iManufacturer)
strings_dev[STRING_MANUFACTURER_IDX].s = iSerialNumber;
if (!strings_dev[STRING_MANUFACTURER_IDX].s[0])
strings_dev[STRING_MANUFACTURER_IDX].s = \
usb_composite_default_manufacturer;
device_desc.iManufacturer =
strings_dev[STRING_MANUFACTURER_IDX].id;
This way, composite gadgets would just assign their desired value to the
string in strings_dev
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.
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.
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +----<email/xmpp: mpn@xxxxxxxxxx>--------------ooO--(_)--Ooo--Attachment:
pgpUI_VT2UA_w.pgp
Description: PGP signature
