2010-08-09 15:01:13

by Michal Nazarewicz

[permalink] [raw]
Subject: [PATCHv7 0/8] Various USB patches

Changes compared to v6:
- USB: gadget: storage_common: fixed warning building mass storage function

Removed, no longer needed after implementing nofua in mass storage
(the last pathc):

USB: gadget: g_multi: fixed vendor and product ID
USB: gadget: g_ffs: fixed vendor and product ID

No changes here.

* USB: gadget: composite: Better string override handling

Previous code missed one #include.

USB: gadget: mass_storage: moved strings handling code to composite

Na changes here.

* USB: gadget: functionfs: code cleanup

MODULE_PARAM_DESC() was used instead of MODULE_PARM_DESC().

USB: gadget: g_multi: moved strings handling code to composite

No changes.

+ usb: gadget: storage: remove nofua file when unbinding

New patch. Fixes issue with Andy Shevchenko's patch.

+ usb: gadget: mass_storage: optional SCSI WRITE FUA bit

New patch. Implements nofua from Andy Shevchenko's patch in
the Mass Storage Function.

drivers/usb/gadget/composite.c | 96 +++++++++++++++++++++++------------
drivers/usb/gadget/f_mass_storage.c | 15 +++++-
drivers/usb/gadget/file_storage.c | 1 +
drivers/usb/gadget/g_ffs.c | 85 ++++++++++---------------------
drivers/usb/gadget/mass_storage.c | 72 ++------------------------
drivers/usb/gadget/multi.c | 23 ++-------
include/linux/usb/composite.h | 13 +++++
7 files changed, 131 insertions(+), 174 deletions(-)


2010-08-09 15:01:22

by Michal Nazarewicz

[permalink] [raw]
Subject: [PATCHv7 2/8] USB: gadget: g_ffs: fixed vendor and product ID

This patch fixes the vendor and product ID the gadget uses
by replacing the temporary IDs that were used during
development (which should never get into mainline) with
proper IDs.

Signed-off-by: Michal Nazarewicz <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
drivers/usb/gadget/g_ffs.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c
index a9474f8..3c2f0a4 100644
--- a/drivers/usb/gadget/g_ffs.c
+++ b/drivers/usb/gadget/g_ffs.c
@@ -53,8 +53,8 @@ MODULE_AUTHOR("Michal Nazarewicz");
MODULE_LICENSE("GPL");


-static unsigned short gfs_vendor_id = 0x0525; /* XXX NetChip */
-static unsigned short gfs_product_id = 0xa4ac; /* XXX */
+static unsigned short gfs_vendor_id = 0x1d6b; /* Linux Foundation */
+static unsigned short gfs_product_id = 0x0105; /* FunctionFS Gadget */

static struct usb_device_descriptor gfs_dev_desc = {
.bLength = sizeof gfs_dev_desc,
--
1.7.1

2010-08-09 15:01:29

by Michal Nazarewicz

[permalink] [raw]
Subject: [PATCHv7 6/8] USB: gadget: g_multi: moved strings handling code to composite

This patch removes some of the string registration from the
Multifunction Composite Gadget as composite layer can handle
the iManufacturer and iProduct for us.

This also adds the "needs_serial" so that composite layer will
issue a warning if user space fails to provide the iSerialNumber
module parameter.

Signed-off-by: Michal Nazarewicz <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
drivers/usb/gadget/multi.c | 19 +++----------------
1 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/gadget/multi.c b/drivers/usb/gadget/multi.c
index 36d67a3..ca51661 100644
--- a/drivers/usb/gadget/multi.c
+++ b/drivers/usb/gadget/multi.c
@@ -121,8 +121,6 @@ static const struct usb_descriptor_header *otg_desc[] = {


enum {
- MULTI_STRING_MANUFACTURER_IDX,
- MULTI_STRING_PRODUCT_IDX,
#ifdef CONFIG_USB_G_MULTI_RNDIS
MULTI_STRING_RNDIS_CONFIG_IDX,
#endif
@@ -131,11 +129,7 @@ enum {
#endif
};

-static char manufacturer[50];
-
static struct usb_string strings_dev[] = {
- [MULTI_STRING_MANUFACTURER_IDX].s = manufacturer,
- [MULTI_STRING_PRODUCT_IDX].s = DRIVER_DESC,
#ifdef CONFIG_USB_G_MULTI_RNDIS
[MULTI_STRING_RNDIS_CONFIG_IDX].s = "Multifunction with RNDIS",
#endif
@@ -314,20 +308,11 @@ static int __ref multi_bind(struct usb_composite_dev *cdev)
device_desc.bcdDevice = cpu_to_le16(0x0300 | 0x0099);
}

- /* allocate string descriptor numbers */
- snprintf(manufacturer, sizeof manufacturer, "%s %s with %s",
- init_utsname()->sysname, init_utsname()->release,
- gadget->name);
-
+ /* allocate string IDs */
status = usb_string_ids_tab(cdev, strings_dev);
if (unlikely(status < 0))
goto fail2;

- device_desc.iManufacturer =
- strings_dev[MULTI_STRING_MANUFACTURER_IDX].id;
- device_desc.iProduct =
- strings_dev[MULTI_STRING_PRODUCT_IDX].id;
-
/* register configurations */
status = rndis_config_register(cdev);
if (unlikely(status < 0))
@@ -370,6 +355,8 @@ static struct usb_composite_driver multi_driver = {
.strings = dev_strings,
.bind = multi_bind,
.unbind = __exit_p(multi_unbind),
+ .iProduct = DRIVER_DESC,
+ .needs_serial = 1,
};


--
1.7.1

2010-08-09 15:01:18

by Michal Nazarewicz

[permalink] [raw]
Subject: [PATCHv7 5/8] USB: gadget: functionfs: code cleanup

This patch removes some of the string registration from the
FunctionFS Gadget as composite layer can handle the
iManufacturer and iProduct for us.

It also removes some of the module parameters which were
redundant as well as changes the name of others to better much
the module parameter of the composite layer.

Other then that, it also fixes formatting of multiline comments
to match the coding style.

Signed-off-by: Michal Nazarewicz <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
drivers/usb/gadget/g_ffs.c | 85 ++++++++++++++-----------------------------
1 files changed, 28 insertions(+), 57 deletions(-)

diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c
index 3c2f0a4..52fd3fa 100644
--- a/drivers/usb/gadget/g_ffs.c
+++ b/drivers/usb/gadget/g_ffs.c
@@ -52,9 +52,8 @@ MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_AUTHOR("Michal Nazarewicz");
MODULE_LICENSE("GPL");

-
-static unsigned short gfs_vendor_id = 0x1d6b; /* Linux Foundation */
-static unsigned short gfs_product_id = 0x0105; /* FunctionFS Gadget */
+#define GFS_VENDOR_ID 0x1d6b /* Linux Foundation */
+#define GFS_PRODUCT_ID 0x0105 /* FunctionFS Gadget */

static struct usb_device_descriptor gfs_dev_desc = {
.bLength = sizeof gfs_dev_desc,
@@ -63,29 +62,16 @@ static struct usb_device_descriptor gfs_dev_desc = {
.bcdUSB = cpu_to_le16(0x0200),
.bDeviceClass = USB_CLASS_PER_INTERFACE,

- /* Vendor and product id can be overridden by module parameters. */
- /* .idVendor = cpu_to_le16(gfs_vendor_id), */
- /* .idProduct = cpu_to_le16(gfs_product_id), */
- /* .bcdDevice = f(hardware) */
- /* .iManufacturer = DYNAMIC */
- /* .iProduct = DYNAMIC */
- /* NO SERIAL NUMBER */
- .bNumConfigurations = 1,
+ .idVendor = cpu_to_le16(GFS_VENDOR_ID),
+ .idProduct = cpu_to_le16(GFS_PRODUCT_ID),
};

-#define GFS_MODULE_PARAM_DESC(name, field) \
- MODULE_PARM_DESC(name, "Value of the " #field " field of the device descriptor sent to the host. Takes effect only prior to the user-space driver registering to the FunctionFS.")
-
-module_param_named(usb_class, gfs_dev_desc.bDeviceClass, byte, 0644);
-GFS_MODULE_PARAM_DESC(usb_class, bDeviceClass);
-module_param_named(usb_subclass, gfs_dev_desc.bDeviceSubClass, byte, 0644);
-GFS_MODULE_PARAM_DESC(usb_subclass, bDeviceSubClass);
-module_param_named(usb_protocol, gfs_dev_desc.bDeviceProtocol, byte, 0644);
-GFS_MODULE_PARAM_DESC(usb_protocol, bDeviceProtocol);
-module_param_named(usb_vendor, gfs_vendor_id, ushort, 0644);
-GFS_MODULE_PARAM_DESC(usb_vendor, idVendor);
-module_param_named(usb_product, gfs_product_id, ushort, 0644);
-GFS_MODULE_PARAM_DESC(usb_product, idProduct);
+module_param_named(bDeviceClass, gfs_dev_desc.bDeviceClass, byte, 0644);
+MODULE_PARM_DESC(bDeviceClass, "USB Device class");
+module_param_named(bDeviceSubClass, gfs_dev_desc.bDeviceSubClass, byte, 0644);
+MODULE_PARM_DESC(bDeviceSubClass, "USB Device subclass");
+module_param_named(bDeviceProtocol, gfs_dev_desc.bDeviceProtocol, byte, 0644);
+MODULE_PARM_DESC(bDeviceProtocol, "USB Device protocol");



@@ -95,8 +81,10 @@ static const struct usb_descriptor_header *gfs_otg_desc[] = {
.bLength = sizeof(struct usb_otg_descriptor),
.bDescriptorType = USB_DT_OTG,

- /* REVISIT SRP-only hardware is possible, although
- * it would not be called "OTG" ... */
+ /*
+ * REVISIT SRP-only hardware is possible, although
+ * it would not be called "OTG" ...
+ */
.bmAttributes = USB_OTG_SRP | USB_OTG_HNP,
},

@@ -105,19 +93,7 @@ static const struct usb_descriptor_header *gfs_otg_desc[] = {

/* string IDs are assigned dynamically */

-enum {
- GFS_STRING_MANUFACTURER_IDX,
- GFS_STRING_PRODUCT_IDX,
- GFS_STRING_FIRST_CONFIG_IDX,
-};
-
-static char gfs_manufacturer[50];
-static const char gfs_driver_desc[] = DRIVER_DESC;
-static const char gfs_short_name[] = DRIVER_NAME;
-
static struct usb_string gfs_strings[] = {
- [GFS_STRING_MANUFACTURER_IDX].s = gfs_manufacturer,
- [GFS_STRING_PRODUCT_IDX].s = gfs_driver_desc,
#ifdef CONFIG_USB_FUNCTIONFS_RNDIS
{ .s = "FunctionFS + RNDIS" },
#endif
@@ -168,11 +144,12 @@ static int gfs_unbind(struct usb_composite_dev *cdev);
static int gfs_do_config(struct usb_configuration *c);

static struct usb_composite_driver gfs_driver = {
- .name = gfs_short_name,
+ .name = DRIVER_NAME,
.dev = &gfs_dev_desc,
.strings = gfs_dev_strings,
.bind = gfs_bind,
.unbind = gfs_unbind,
+ .iProduct = DRIVER_DESC,
};


@@ -245,20 +222,10 @@ static int gfs_bind(struct usb_composite_dev *cdev)
if (unlikely(ret < 0))
goto error_quick;

- gfs_dev_desc.idVendor = cpu_to_le16(gfs_vendor_id);
- gfs_dev_desc.idProduct = cpu_to_le16(gfs_product_id);
-
- snprintf(gfs_manufacturer, sizeof gfs_manufacturer, "%s %s with %s",
- init_utsname()->sysname, init_utsname()->release,
- cdev->gadget->name);
-
ret = usb_string_ids_tab(cdev, gfs_strings);
if (unlikely(ret < 0))
goto error;

- gfs_dev_desc.iManufacturer = gfs_strings[GFS_STRING_MANUFACTURER_IDX].id;
- gfs_dev_desc.iProduct = gfs_strings[GFS_STRING_PRODUCT_IDX].id;
-
ret = functionfs_bind(gfs_ffs_data, cdev);
if (unlikely(ret < 0))
goto error;
@@ -266,9 +233,8 @@ static int gfs_bind(struct usb_composite_dev *cdev)
for (i = 0; i < ARRAY_SIZE(gfs_configurations); ++i) {
struct gfs_configuration *c = gfs_configurations + i;

- ret = GFS_STRING_FIRST_CONFIG_IDX + i;
- c->c.label = gfs_strings[ret].s;
- c->c.iConfiguration = gfs_strings[ret].id;
+ c->c.label = gfs_strings[i].s;
+ c->c.iConfiguration = gfs_strings[i].id;
c->c.bind = gfs_do_config;
c->c.bConfigurationValue = 1 + i;
c->c.bmAttributes = USB_CONFIG_ATT_SELFPOWER;
@@ -293,13 +259,14 @@ static int gfs_unbind(struct usb_composite_dev *cdev)
{
ENTER();

- /* We may have been called in an error recovery frem
+ /*
+ * We may have been called in an error recovery from
* composite_bind() after gfs_unbind() failure so we need to
* check if gfs_ffs_data is not NULL since gfs_bind() handles
* all error recovery itself. I'd rather we werent called
* from composite on orror recovery, but what you're gonna
- * do...? */
-
+ * do...?
+ */
if (gfs_ffs_data) {
gether_cleanup();
functionfs_unbind(gfs_ffs_data);
@@ -334,14 +301,16 @@ static int gfs_do_config(struct usb_configuration *c)
if (unlikely(ret < 0))
return ret;

- /* After previous do_configs there may be some invalid
+ /*
+ * After previous do_configs there may be some invalid
* pointers in c->interface array. This happens every time
* a user space function with fewer interfaces than a user
* space function that was run before the new one is run. The
* compasit's set_config() assumes that if there is no more
* then MAX_CONFIG_INTERFACES interfaces in a configuration
* then there is a NULL pointer after the last interface in
- * c->interface array. We need to make sure this is true. */
+ * c->interface array. We need to make sure this is true.
+ */
if (c->next_interface_id < ARRAY_SIZE(c->interface))
c->interface[c->next_interface_id] = NULL;

@@ -350,10 +319,12 @@ static int gfs_do_config(struct usb_configuration *c)


#ifdef CONFIG_USB_FUNCTIONFS_ETH
+
static int eth_bind_config(struct usb_configuration *c, u8 ethaddr[ETH_ALEN])
{
return can_support_ecm(c->cdev->gadget)
? ecm_bind_config(c, ethaddr)
: geth_bind_config(c, ethaddr);
}
+
#endif
--
1.7.1

2010-08-09 15:01:16

by Michal Nazarewicz

[permalink] [raw]
Subject: [PATCHv7 1/8] USB: gadget: g_multi: fixed vendor and product ID

This patch fixes the vendor and product ID the gadget uses
by replacing the temporary IDs that were used during
development (which should never get into mainline) with
proper IDs.

Signed-off-by: Michal Nazarewicz <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
drivers/usb/gadget/multi.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/multi.c b/drivers/usb/gadget/multi.c
index 795d762..36d67a3 100644
--- a/drivers/usb/gadget/multi.c
+++ b/drivers/usb/gadget/multi.c
@@ -74,8 +74,8 @@ MODULE_LICENSE("GPL");

/***************************** Device Descriptor ****************************/

-#define MULTI_VENDOR_NUM 0x0525 /* XXX NetChip */
-#define MULTI_PRODUCT_NUM 0xa4ab /* XXX */
+#define MULTI_VENDOR_NUM 0x1d6b /* Linux Foundation */
+#define MULTI_PRODUCT_NUM 0x0104 /* Multifunction Composite Gadget */


enum {
--
1.7.1

2010-08-09 15:02:22

by Michal Nazarewicz

[permalink] [raw]
Subject: [PATCHv7 3/8] USB: gadget: composite: Better string override handling

The iManufatcurer, iProduct and iSerialNumber composite module
parameters were only used when the gadget driver registers
strings for manufacturer, product and serial number. If the
gadget never bothered to set corresponding fields in USB device
descriptors those module parameters are ignored.

This commit makes the parameters work even if the strings ID
have not been assigned. It also changes the way IDs are
overridden -- what IDs are overridden is now saved in
usb_composite_dev structure -- which makes it unnecessary to
modify the string tables the way previous code did.

The commit also adds a iProduct and iManufatcurer fields to the
usb_composite_device structure. If they are set, appropriate
strings are reserved and added to device descriptor. This makes
it unnecessary for gadget drivers to maintain code for setting
those. If iProduct is not set it defaults to
usb_composite_device::name; if iManufatcurer is not set
a default "<system> <release> with <gadget-name>" is used.

The last thing is that if needs_serial field of
usb_composite_device is set and user failed to provided
iSerialNumber parameter a warning is issued.

Signed-off-by: Michal Nazarewicz <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
drivers/usb/gadget/composite.c | 96 ++++++++++++++++++++++++++-------------
include/linux/usb/composite.h | 13 +++++
2 files changed, 77 insertions(+), 32 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index e483f80..471c759 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -24,6 +24,7 @@
#include <linux/kernel.h>
#include <linux/slab.h>
#include <linux/device.h>
+#include <linux/utsname.h>

#include <linux/usb/composite.h>

@@ -69,6 +70,8 @@ static char *iSerialNumber;
module_param(iSerialNumber, charp, 0);
MODULE_PARM_DESC(iSerialNumber, "SerialNumber string");

+static char composite_manufacturer[50];
+
/*-------------------------------------------------------------------------*/

/**
@@ -599,6 +602,7 @@ static int get_string(struct usb_composite_dev *cdev,
struct usb_configuration *c;
struct usb_function *f;
int len;
+ const char *str;

/* Yes, not only is USB's I18N support probably more than most
* folk will ever care about ... also, it's all supported here.
@@ -638,9 +642,29 @@ static int get_string(struct usb_composite_dev *cdev,
return s->bLength;
}

- /* Otherwise, look up and return a specified string. String IDs
- * are device-scoped, so we look up each string table we're told
- * about. These lookups are infrequent; simpler-is-better here.
+ /* Otherwise, look up and return a specified string. First
+ * check if the string has not been overridden.
+ */
+ if (cdev->manufacturer_override == id)
+ str = iManufacturer ?: composite->iManufacturer ?:
+ composite_manufacturer;
+ else if (cdev->product_override == id)
+ str = iProduct ?: composite->iProduct;
+ else if (cdev->serial_override == id)
+ str = iSerialNumber;
+ else
+ str = NULL;
+ if (str) {
+ struct usb_gadget_strings strings = {
+ .language = language,
+ .strings = &(struct usb_string) { 0xff, str }
+ };
+ return usb_gadget_get_string(&strings, 0xff, buf);
+ }
+
+ /* String IDs are device-scoped, so we look up each string
+ * table we're told about. These lookups are infrequent;
+ * simpler-is-better here.
*/
if (composite->strings) {
len = lookup_string(composite->strings, buf, language, id);
@@ -1025,26 +1049,17 @@ composite_unbind(struct usb_gadget *gadget)
composite = NULL;
}

-static void
-string_override_one(struct usb_gadget_strings *tab, u8 id, const char *s)
+static u8 override_id(struct usb_composite_dev *cdev, u8 *desc)
{
- struct usb_string *str = tab->strings;
-
- for (str = tab->strings; str->s; str++) {
- if (str->id == id) {
- str->s = s;
- return;
- }
+ if (!*desc) {
+ int ret = usb_string_id(cdev);
+ if (unlikely(ret < 0))
+ WARNING(cdev, "failed to override string ID\n");
+ else
+ *desc = ret;
}
-}

-static void
-string_override(struct usb_gadget_strings **tab, u8 id, const char *s)
-{
- while (*tab) {
- string_override_one(*tab, id, s);
- tab++;
- }
+ return *desc;
}

static int composite_bind(struct usb_gadget *gadget)
@@ -1101,19 +1116,34 @@ static int composite_bind(struct usb_gadget *gadget)
cdev->desc = *composite->dev;
cdev->desc.bMaxPacketSize0 = gadget->ep0->maxpacket;

- /* strings can't be assigned before bind() allocates the
- * releavnt identifiers
- */
- if (cdev->desc.iManufacturer && iManufacturer)
- string_override(composite->strings,
- cdev->desc.iManufacturer, iManufacturer);
- if (cdev->desc.iProduct && iProduct)
- string_override(composite->strings,
- cdev->desc.iProduct, iProduct);
- if (cdev->desc.iSerialNumber && iSerialNumber)
- string_override(composite->strings,
- cdev->desc.iSerialNumber, iSerialNumber);
+ /* stirng overrides */
+ if (iManufacturer || !cdev->desc.iManufacturer) {
+ if (!iManufacturer && !composite->iManufacturer &&
+ !*composite_manufacturer)
+ snprintf(composite_manufacturer,
+ sizeof composite_manufacturer,
+ "%s %s with %s",
+ init_utsname()->sysname,
+ init_utsname()->release,
+ gadget->name);
+
+ cdev->manufacturer_override =
+ override_id(cdev, &cdev->desc.iManufacturer);
+ }
+
+ if (iProduct || (!cdev->desc.iProduct && composite->iProduct))
+ cdev->product_override =
+ override_id(cdev, &cdev->desc.iProduct);
+
+ if (iSerialNumber)
+ cdev->serial_override =
+ override_id(cdev, &cdev->desc.iSerialNumber);
+
+ /* has userspace failed to provide a serial number? */
+ if (composite->needs_serial && !cdev->desc.iSerialNumber)
+ WARNING(cdev, "userspace failed to provide iSerialNumber\n");

+ /* finish up */
status = device_create_file(&gadget->dev, &dev_attr_suspended);
if (status)
goto fail;
@@ -1211,6 +1241,8 @@ int usb_composite_register(struct usb_composite_driver *driver)
if (!driver || !driver->dev || !driver->bind || composite)
return -EINVAL;

+ if (!driver->iProduct)
+ driver->iProduct = driver->name;
if (!driver->name)
driver->name = "composite";
composite_driver.function = (char *) driver->name;
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 890bc14..d590904 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -237,10 +237,17 @@ int usb_add_config(struct usb_composite_dev *,
/**
* struct usb_composite_driver - groups configurations into a gadget
* @name: For diagnostics, identifies the driver.
+ * @iProduct: Used as iProduct override if @dev->iProduct is not set.
+ * If NULL value of @name is taken.
+ * @iManufacturer: Used as iManufacturer override if @dev->iManufacturer is
+ * not set. If NULL a default "<system> <release> with <udc>" value
+ * will be used.
* @dev: Template descriptor for the device, including default device
* identifiers.
* @strings: tables of strings, keyed by identifiers assigned during bind()
* and language IDs provided in control requests
+ * @needs_serial: set to 1 if the gadget needs userspace to provide
+ * a serial number. If one is not provided, warning will be printed.
* @bind: (REQUIRED) Used to allocate resources that are shared across the
* whole device, such as string IDs, and add its configurations using
* @usb_add_config(). This may fail by returning a negative errno
@@ -265,8 +272,11 @@ int usb_add_config(struct usb_composite_dev *,
*/
struct usb_composite_driver {
const char *name;
+ const char *iProduct;
+ const char *iManufacturer;
const struct usb_device_descriptor *dev;
struct usb_gadget_strings **strings;
+ unsigned needs_serial:1;

/* REVISIT: bind() functions can be marked __init, which
* makes trouble for section mismatch analysis. See if
@@ -333,6 +343,9 @@ struct usb_composite_dev {
struct list_head configs;
struct usb_composite_driver *driver;
u8 next_string_id;
+ u8 manufacturer_override;
+ u8 product_override;
+ u8 serial_override;

/* the gadget driver won't enable the data pullup
* while the deactivation count is nonzero.
--
1.7.1

2010-08-09 15:02:41

by Michal Nazarewicz

[permalink] [raw]
Subject: [PATCHv7 8/8] usb: gadget: mass_storage: optional SCSI WRITE FUA bit

The nofua parameter (optionally ignore SCSI WRITE FUA) was added
to the File Storage Gadget some time ago. This patch adds the
same functionality to the Mass Storage Function.

Signed-off-by: Michal Nazarewicz <[email protected]>
Cc: Andy Shevchenko <[email protected]>
---
drivers/usb/gadget/f_mass_storage.c | 15 ++++++++++++++-
1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
index 32cce02..ec97232 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -73,6 +73,8 @@
* being removable.
* ->cdrom Flag specifying that LUN shall be reported as
* being a CD-ROM.
+ * ->nofua Flag specifying that FUA flag in SCSI WRITE(10,12)
+ * commands for this LUN shall be ignored.
*
* lun_name_format A printf-like format for names of the LUN
* devices. This determines how the
@@ -127,6 +129,8 @@
* Default true, boolean for removable media.
* cdrom=b[,b...] Default false, boolean for whether to emulate
* a CD-ROM drive.
+ * nofua=b[,b...] Default false, booleans for ignore FUA flag
+ * in SCSI WRITE(10,12) commands
* luns=N Default N = number of filenames, number of
* LUNs to support.
* stall Default determined according to the type of
@@ -409,6 +413,7 @@ struct fsg_config {
char ro;
char removable;
char cdrom;
+ char nofua;
} luns[FSG_MAX_LUNS];

const char *lun_name_format;
@@ -887,7 +892,7 @@ static int do_write(struct fsg_common *common)
curlun->sense_data = SS_INVALID_FIELD_IN_CDB;
return -EINVAL;
}
- if (common->cmnd[1] & 0x08) { /* FUA */
+ if (!curlun->nofua && (common->cmnd[1] & 0x08)) { /* FUA */
spin_lock(&curlun->filp->f_lock);
curlun->filp->f_flags |= O_SYNC;
spin_unlock(&curlun->filp->f_lock);
@@ -2662,6 +2667,7 @@ static int fsg_main_thread(void *common_)

/* Write permission is checked per LUN in store_*() functions. */
static DEVICE_ATTR(ro, 0644, fsg_show_ro, fsg_store_ro);
+static DEVICE_ATTR(nofua, 0644, fsg_show_nofua, fsg_store_nofua);
static DEVICE_ATTR(file, 0644, fsg_show_file, fsg_store_file);


@@ -2768,6 +2774,9 @@ static struct fsg_common *fsg_common_init(struct fsg_common *common,
rc = device_create_file(&curlun->dev, &dev_attr_file);
if (rc)
goto error_luns;
+ rc = device_create_file(&curlun->dev, &dev_attr_nofua);
+ if (rc)
+ goto error_luns;

if (lcfg->filename) {
rc = fsg_lun_open(curlun, lcfg->filename);
@@ -3069,8 +3078,10 @@ struct fsg_module_parameters {
int ro[FSG_MAX_LUNS];
int removable[FSG_MAX_LUNS];
int cdrom[FSG_MAX_LUNS];
+ int nofua[FSG_MAX_LUNS];

unsigned int file_count, ro_count, removable_count, cdrom_count;
+ unsigned int nofua_count;
unsigned int luns; /* nluns */
int stall; /* can_stall */
};
@@ -3096,6 +3107,8 @@ struct fsg_module_parameters {
"true to simulate removable media"); \
_FSG_MODULE_PARAM_ARRAY(prefix, params, cdrom, bool, \
"true to simulate CD-ROM instead of disk"); \
+ _FSG_MODULE_PARAM_ARRAY(prefix, params, nofua, bool, \
+ "true to ignore SCSI WRITE(10,12) FUA bit"); \
_FSG_MODULE_PARAM(prefix, params, luns, uint, \
"number of LUNs"); \
_FSG_MODULE_PARAM(prefix, params, stall, bool, \
--
1.7.1

2010-08-09 15:03:15

by Michal Nazarewicz

[permalink] [raw]
Subject: [PATCHv7 4/8] USB: gadget: mass_storage: moved strings handling code to composite

This patch removes string registration from the Mass Storage
Gadget. With recent changes to the composite framework, all
that we need is handled by the composite layer. This means
composite registers a string ID for manufacturer and product.

This also adds the "needs_serial" so that composite layer will
issue a warning if user space fails to provide the iSerialNumber
module parameter.

Signed-off-by: Michal Nazarewicz <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
drivers/usb/gadget/mass_storage.c | 72 +++---------------------------------
1 files changed, 6 insertions(+), 66 deletions(-)

diff --git a/drivers/usb/gadget/mass_storage.c b/drivers/usb/gadget/mass_storage.c
index 585f255..493d153 100644
--- a/drivers/usb/gadget/mass_storage.c
+++ b/drivers/usb/gadget/mass_storage.c
@@ -75,10 +75,6 @@ static struct usb_device_descriptor msg_device_desc = {
/* Vendor and product id can be overridden by module parameters. */
.idVendor = cpu_to_le16(FSG_VENDOR_ID),
.idProduct = cpu_to_le16(FSG_PRODUCT_ID),
- /* .bcdDevice = f(hardware) */
- /* .iManufacturer = DYNAMIC */
- /* .iProduct = DYNAMIC */
- /* NO SERIAL NUMBER */
.bNumConfigurations = 1,
};

@@ -86,7 +82,8 @@ static struct usb_otg_descriptor otg_descriptor = {
.bLength = sizeof otg_descriptor,
.bDescriptorType = USB_DT_OTG,

- /* REVISIT SRP-only hardware is possible, although
+ /*
+ * REVISIT SRP-only hardware is possible, although
* it would not be called "OTG" ...
*/
.bmAttributes = USB_OTG_SRP | USB_OTG_HNP,
@@ -98,33 +95,6 @@ static const struct usb_descriptor_header *otg_desc[] = {
};


-/* string IDs are assigned dynamically */
-
-#define STRING_MANUFACTURER_IDX 0
-#define STRING_PRODUCT_IDX 1
-#define STRING_CONFIGURATION_IDX 2
-
-static char manufacturer[50];
-
-static struct usb_string strings_dev[] = {
- [STRING_MANUFACTURER_IDX].s = manufacturer,
- [STRING_PRODUCT_IDX].s = DRIVER_DESC,
- [STRING_CONFIGURATION_IDX].s = "Self Powered",
- { } /* end of list */
-};
-
-static struct usb_gadget_strings stringtab_dev = {
- .language = 0x0409, /* en-us */
- .strings = strings_dev,
-};
-
-static struct usb_gadget_strings *dev_strings[] = {
- &stringtab_dev,
- NULL,
-};
-
-
-
/****************************** Configurations ******************************/

static struct fsg_module_parameters mod_data = {
@@ -173,52 +143,22 @@ static struct usb_configuration msg_config_driver = {
.label = "Linux File-Backed Storage",
.bind = msg_do_config,
.bConfigurationValue = 1,
- /* .iConfiguration = DYNAMIC */
.bmAttributes = USB_CONFIG_ATT_SELFPOWER,
};


-
/****************************** Gadget Bind ******************************/

-
static int __ref msg_bind(struct usb_composite_dev *cdev)
{
- struct usb_gadget *gadget = cdev->gadget;
int status;

- /* Allocate string descriptor numbers ... note that string
- * contents can be overridden by the composite_dev glue.
- */
-
- /* device descriptor strings: manufacturer, product */
- snprintf(manufacturer, sizeof manufacturer, "%s %s with %s",
- init_utsname()->sysname, init_utsname()->release,
- gadget->name);
- status = usb_string_id(cdev);
- if (status < 0)
- return status;
- strings_dev[STRING_MANUFACTURER_IDX].id = status;
- msg_device_desc.iManufacturer = status;
-
- status = usb_string_id(cdev);
- if (status < 0)
- return status;
- strings_dev[STRING_PRODUCT_IDX].id = status;
- msg_device_desc.iProduct = status;
-
- status = usb_string_id(cdev);
- if (status < 0)
- return status;
- strings_dev[STRING_CONFIGURATION_IDX].id = status;
- msg_config_driver.iConfiguration = status;
-
- /* register our second configuration */
status = usb_add_config(cdev, &msg_config_driver);
if (status < 0)
return status;

- dev_info(&gadget->dev, DRIVER_DESC ", version: " DRIVER_VERSION "\n");
+ dev_info(&cdev->gadget->dev,
+ DRIVER_DESC ", version: " DRIVER_VERSION "\n");
set_bit(0, &msg_registered);
return 0;
}
@@ -226,12 +166,12 @@ static int __ref msg_bind(struct usb_composite_dev *cdev)

/****************************** Some noise ******************************/

-
static struct usb_composite_driver msg_driver = {
.name = "g_mass_storage",
.dev = &msg_device_desc,
- .strings = dev_strings,
.bind = msg_bind,
+ .iProduct = DRIVER_DESC,
+ .needs_serial = 1,
};

MODULE_DESCRIPTION(DRIVER_DESC);
--
1.7.1

2010-08-09 15:03:34

by Michal Nazarewicz

[permalink] [raw]
Subject: [PATCHv7 7/8] usb: gadget: storage: remove nofua file when unbinding

The dev_attr_nofua file was created during fsg_bind() but
was never removed. Made it a bit more symmetrical and added
code to remove the file in fsg_unbind().

Signed-off-by: Michal Nazarewicz <[email protected]>
Cc: Andy Shevchenko <[email protected]>
---
drivers/usb/gadget/file_storage.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c
index a857b7a..ab77792 100644
--- a/drivers/usb/gadget/file_storage.c
+++ b/drivers/usb/gadget/file_storage.c
@@ -3178,6 +3178,7 @@ static void /* __init_or_exit */ fsg_unbind(struct usb_gadget *gadget)
for (i = 0; i < fsg->nluns; ++i) {
curlun = &fsg->luns[i];
if (curlun->registered) {
+ device_remove_file(&curlun->dev, &dev_attr_nofua);
device_remove_file(&curlun->dev, &dev_attr_ro);
device_remove_file(&curlun->dev, &dev_attr_file);
fsg_lun_close(curlun);
--
1.7.1

2010-08-12 11:32:27

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCHv7 7/8] usb: gadget: storage: remove nofua file when unbinding

On Mon, Aug 9, 2010 at 6:02 PM, Michal Nazarewicz
<[email protected]> wrote:
> The dev_attr_nofua file was created during fsg_bind() but
> was never removed.  Made it a bit more symmetrical and added
> code to remove the file in fsg_unbind().
>
> Signed-off-by: Michal Nazarewicz <[email protected]>
> Cc: Andy Shevchenko <[email protected]>
> ---
>  drivers/usb/gadget/file_storage.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c
> index a857b7a..ab77792 100644
> --- a/drivers/usb/gadget/file_storage.c
> +++ b/drivers/usb/gadget/file_storage.c
> @@ -3178,6 +3178,7 @@ static void /* __init_or_exit */ fsg_unbind(struct usb_gadget *gadget)
>        for (i = 0; i < fsg->nluns; ++i) {
>                curlun = &fsg->luns[i];
>                if (curlun->registered) {
> +                       device_remove_file(&curlun->dev, &dev_attr_nofua);
>                        device_remove_file(&curlun->dev, &dev_attr_ro);
>                        device_remove_file(&curlun->dev, &dev_attr_file);
>                        fsg_lun_close(curlun);
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Acked-by: Andy Shevchenko <[email protected]>

--
With Best Regards,
Andy Shevchenko

2010-08-12 11:33:44

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCHv7 8/8] usb: gadget: mass_storage: optional SCSI WRITE FUA bit

On Mon, Aug 9, 2010 at 6:02 PM, Michal Nazarewicz
<[email protected]> wrote:
> The nofua parameter (optionally ignore SCSI WRITE FUA) was added
> to the File Storage Gadget some time ago.  This patch adds the
> same functionality to the Mass Storage Function.
>
> Signed-off-by: Michal Nazarewicz <[email protected]>
> Cc: Andy Shevchenko <[email protected]>
> +               rc = device_create_file(&curlun->dev, &dev_attr_nofua);
> +               if (rc)
> +                       goto error_luns;
It has the same issue as my original patch - forgot to remove device node.

--
With Best Regards,
Andy Shevchenko