Re: [PATCH 15/20] usb/gadget: push iManufacturer into gadgets
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
Re: [PATCH 15/20] usb/gadget: push iManufacturer into gadgets
Sebastian Andrzej Siewior writes:
> On 08/25/2012 12:11 AM, Michal Nazarewicz wrote:
>> Sebastian Andrzej Siewior 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 +--ooO--(_)--Ooo--
pgpvONhyh6loy.pgp
Description: PGP signature
Re: [PATCH 15/20] usb/gadget: push iManufacturer into gadgets
On 08/25/2012 12:11 AM, Michal Nazarewicz wrote: Sebastian Andrzej Siewior 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. 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
Re: [PATCH 15/20] usb/gadget: push iManufacturer into gadgets
Sebastian Andrzej Siewior 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? > Signed-off-by: Sebastian Andrzej Siewior -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--ooO--(_)--Ooo-- pgptMReIjfGYM.pgp Description: PGP signature
[PATCH 15/20] usb/gadget: push iManufacturer into gadgets
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.
Signed-off-by: Sebastian Andrzej Siewior
---
drivers/usb/gadget/acm_ms.c |4 +++-
drivers/usb/gadget/audio.c |3 ++-
drivers/usb/gadget/cdc2.c |3 ++-
drivers/usb/gadget/composite.c | 11 +++
drivers/usb/gadget/ether.c |3 ++-
drivers/usb/gadget/g_ffs.c |6 ++
drivers/usb/gadget/gmidi.c |3 ++-
drivers/usb/gadget/hid.c|3 ++-
drivers/usb/gadget/mass_storage.c |7 +++
drivers/usb/gadget/multi.c |7 +++
drivers/usb/gadget/ncm.c|2 ++
drivers/usb/gadget/nokia.c |2 ++
drivers/usb/gadget/printer.c|2 ++
drivers/usb/gadget/serial.c |3 +++
drivers/usb/gadget/tcm_usb_gadget.c |3 +++
drivers/usb/gadget/webcam.c |3 +++
drivers/usb/gadget/zero.c |2 ++
include/linux/usb/composite.h |6 +-
18 files changed, 58 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/gadget/acm_ms.c b/drivers/usb/gadget/acm_ms.c
index 63ef563..51fc568 100644
--- a/drivers/usb/gadget/acm_ms.c
+++ b/drivers/usb/gadget/acm_ms.c
@@ -65,7 +65,6 @@ static struct usb_device_descriptor device_desc = {
.idVendor = cpu_to_le16(ACM_MS_VENDOR_NUM),
.idProduct =cpu_to_le16(ACM_MS_PRODUCT_NUM),
/* .bcdDevice = f(hardware) */
- /* .iManufacturer = DYNAMIC */
/* .iProduct = DYNAMIC */
/* NO SERIAL NUMBER */
/*.bNumConfigurations = DYNAMIC*/
@@ -212,6 +211,9 @@ static int __init acm_ms_bind(struct usb_composite_dev
*cdev)
device_desc.iSerialNumber =
strings_dev[STRING_PRODUCT_SERIAL].id;
}
+ if (iManufacturer)
+strings_dev[STRING_MANUFACTURER_IDX].s = iManufacturer;
+
dev_info(&gadget->dev, "%s, version: " DRIVER_VERSION "\n",
DRIVER_DESC);
fsg_common_put(&fsg_common);
diff --git a/drivers/usb/gadget/audio.c b/drivers/usb/gadget/audio.c
index 336206d..d598e32 100644
--- a/drivers/usb/gadget/audio.c
+++ b/drivers/usb/gadget/audio.c
@@ -100,7 +100,6 @@ static struct usb_device_descriptor device_desc = {
.idVendor = __constant_cpu_to_le16(AUDIO_VENDOR_NUM),
.idProduct =__constant_cpu_to_le16(AUDIO_PRODUCT_NUM),
/* .bcdDevice = f(hardware) */
- /* .iManufacturer = DYNAMIC */
/* .iProduct = DYNAMIC */
/* NO SERIAL NUMBER */
.bNumConfigurations = 1,
@@ -184,6 +183,8 @@ static int __init audio_bind(struct usb_composite_dev *cdev)
device_desc.iSerialNumber =
strings_dev[STRING_PRODUCT_SERIAL].id;
}
+ if (iManufacturer)
+ strings_dev[STRING_MANUFACTURER_IDX].s = iManufacturer;
INFO(cdev, "%s, version: %s\n", DRIVER_DESC, DRIVER_VERSION);
return 0;
diff --git a/drivers/usb/gadget/cdc2.c b/drivers/usb/gadget/cdc2.c
index 692f817..390a7e4 100644
--- a/drivers/usb/gadget/cdc2.c
+++ b/drivers/usb/gadget/cdc2.c
@@ -67,7 +67,6 @@ static struct usb_device_descriptor device_desc = {
.idVendor = cpu_to_le16(CDC_VENDOR_NUM),
.idProduct =cpu_to_le16(CDC_PRODUCT_NUM),
/* .bcdDevice = f(hardware) */
- /* .iManufacturer = DYNAMIC */
/* .iProduct = DYNAMIC */
/* NO SERIAL NUMBER */
.bNumConfigurations = 1,
@@ -211,6 +210,8 @@ static int __init cdc_bind(struct usb_composite_dev *cdev)
device_desc.iSerialNumber =
strings_dev[STRING_PRODUCT_SERIAL].id;
}
+ if (iManufacturer)
+ strings_dev[STRING_MANUFACTURER_IDX].s = iManufacturer;
USB_GADGET_COMPOSITE_OVERWRITE_OPTIONS(device_desc);
dev_info(&gadget->dev, "%s, version: " DRIVER_VERSION "\n",
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 506be03..a89f95d 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -32,9 +32,6 @@
* published in the device descriptor, either numbers or strings or both.
* String parameters are in UTF-8 (superset of ASCII's 7 bit characters).
*/
-static char *iManufacturer;
-module_param(iManufacturer, charp, S_IRUGO);
-MODULE_PARM_DESC(iManufacturer, "USB Manufacturer string");
static char *iProduct;
module_param(iProduct, charp, S_IRUGO);
@@ -916,8 +913,7 @@ static int get_string(struct usb_composite_dev *cdev,
* check if the string has not been overridden.
*/
if (cdev->manufacturer_override == id)
- str = iManufacturer ?: cdriver->iManufacturer ?:
- composite_manufacturer;
+ str = cdriver->iManufacturer ?: composite_manufacturer;
