Greg:
This patch follows the suggestions sent by Todd Fischer and Diego Dompe
for making removable-LUN support part of the normal non-testing version of
the g_file_storage driver. It also moves LUN device registration to the
correct place and eliminates a code path that stalls the bulk-out pipe in
a racy way.
There are also some smaller changes: update some comments, add initial
debugging support for USB suspend/resume, and miscellaneous code cleanups.
Last but not least, the driver has been sufficiently stable for
sufficiently long that it's fair to remove the "(DEVELOPMENT)" warning in
Kconfig.
Please apply.
Alan Stern
Sent-by: Todd Fischer <[EMAIL PROTECTED]>
Signed-off-by: Alan Stern <[EMAIL PROTECTED]>
===== drivers/usb/gadget/Kconfig 1.22 vs edited =====
--- 1.22/drivers/usb/gadget/Kconfig Sat Jul 10 08:40:59 2004
+++ edited/drivers/usb/gadget/Kconfig Wed Jul 28 11:08:24 2004
@@ -275,7 +275,7 @@
dynamically linked module called "gadgetfs".
config USB_FILE_STORAGE
- tristate "File-backed Storage Gadget (DEVELOPMENT)"
+ tristate "File-backed Storage Gadget"
# we don't support the SA1100 because of its limitations
depends on USB_GADGET_SA1100 = n
help
@@ -288,7 +288,7 @@
dynamically linked module called "g_file_storage".
config USB_FILE_STORAGE_TEST
- bool "File-backed Storage Gadget test version"
+ bool "File-backed Storage Gadget testing version"
depends on USB_FILE_STORAGE
default n
help
===== drivers/usb/gadget/file_storage.c 1.14 vs edited =====
--- 1.14/drivers/usb/gadget/file_storage.c Wed Jun 30 12:33:56 2004
+++ edited/drivers/usb/gadget/file_storage.c Wed Jul 28 11:17:49 2004
@@ -46,17 +46,16 @@
*
* Backing storage is provided by a regular file or a block device, specified
* by the "file" module parameter. Access can be limited to read-only by
- * setting the optional "ro" module parameter.
+ * setting the optional "ro" module parameter. The gadget will indicate that
+ * it has removable media if the optional "removable" module parameter is set.
*
* The gadget supports the Control-Bulk (CB), Control-Bulk-Interrupt (CBI),
* and Bulk-Only (also known as Bulk-Bulk-Bulk or BBB) transports, selected
* by the optional "transport" module parameter. It also supports the
* following protocols: RBC (0x01), ATAPI or SFF-8020i (0x02), QIC-157 (0c03),
* UFI (0x04), SFF-8070i (0x05), and transparent SCSI (0x06), selected by
- * the optional "protocol" module parameter. For testing purposes the
- * gadget will indicate that it has removable media if the optional
- * "removable" module parameter is set. In addition, the default Vendor ID,
- * Product ID, and release number can be overridden.
+ * the optional "protocol" module parameter. In addition, the default
+ * Vendor ID, Product ID, and release number can be overridden.
*
* There is support for multiple logical units (LUNs), each of which has
* its own backing file. The number of LUNs can be set using the optional
@@ -79,13 +78,13 @@
* the files or block devices used for
* backing storage
* ro=b[,b...] Default false, booleans for read-only access
+ * removable Default false, boolean for removable media
* luns=N Default N = number of filenames, number of
* LUNs to support
* transport=XXX Default BBB, transport name (CB, CBI, or BBB)
* protocol=YYY Default SCSI, protocol name (RBC, 8020 or
* ATAPI, QIC, UFI, 8070, or SCSI;
* also 1 - 6)
- * removable Default false, boolean for removable media
* vendor=0xVVVV Default 0x0525 (NetChip), USB Vendor ID
* product=0xPPPP Default 0xa4a5 (FSG), USB Product ID
* release=0xRRRR Override the USB release number (bcdDevice)
@@ -97,16 +96,16 @@
* boolean to permit the driver to halt
* bulk endpoints
*
- * If CONFIG_USB_FILE_STORAGE_TEST is not set, only the "file" and "ro"
- * options are available; default values are used for everything else.
+ * If CONFIG_USB_FILE_STORAGE_TEST is not set, only the "file", "ro",
+ * "removable", and "luns" options are available; default values are used
+ * for everything else.
*
* The pathnames of the backing files and the ro settings are available in
* the attribute files "file" and "ro" in the lun<n> subdirectory of the
- * gadget's sysfs directory. If CONFIG_USB_FILE_STORAGE_TEST and the
- * "removable" option are both set, writing to these files will simulate
- * ejecting/loading the medium (writing an empty line means eject) and
- * adjusting a write-enable tab. Changes to the ro setting are not allowed
- * when the medium is loaded.
+ * gadget's sysfs directory. If the "removable" option is set, writing to
+ * these files will simulate ejecting/loading the medium (writing an empty
+ * line means eject) and adjusting a write-enable tab. Changes to the ro
+ * setting are not allowed when the medium is loaded.
*
* This gadget driver is heavily based on "Gadget Zero" by David Brownell.
*/
@@ -178,7 +177,10 @@
* Bulk-only specification requires a stall. In such cases the driver
* will halt the endpoint and set a flag indicating that it should clear
* the halt in software during the next device reset. Hopefully this
- * will permit everything to work correctly.
+ * will permit everything to work correctly. Furthermore, although the
+ * specification allows the bulk-out endpoint to halt when the host sends
+ * too much data, implementing this would cause an unavoidable race.
+ * The driver will always use the "no-stall" approach for OUT transfers.
*
* One subtle point concerns sending status-stage responses for ep0
* requests. Some of these requests, such as device reset, can involve
@@ -246,7 +248,7 @@
#define DRIVER_DESC "File-backed Storage Gadget"
#define DRIVER_NAME "g_file_storage"
-#define DRIVER_VERSION "21 March 2004"
+#define DRIVER_VERSION "28 July 2004"
static const char longname[] = DRIVER_DESC;
static const char shortname[] = DRIVER_NAME;
@@ -371,14 +373,17 @@
module_param_array(ro, bool, mod_data.num_ros, S_IRUGO);
MODULE_PARM_DESC(ro, "true to force read-only");
+module_param_named(luns, mod_data.nluns, uint, S_IRUGO);
+MODULE_PARM_DESC(luns, "number of LUNs");
-/* In the non-TEST version, only the file and ro module parameters
+module_param_named(removable, mod_data.removable, bool, S_IRUGO);
+MODULE_PARM_DESC(removable, "true to simulate removable media");
+
+
+/* In the non-TEST version, only the module parameters listed above
* are available. */
#ifdef CONFIG_USB_FILE_STORAGE_TEST
-module_param_named(luns, mod_data.nluns, uint, S_IRUGO);
-MODULE_PARM_DESC(luns, "number of LUNs");
-
module_param_named(transport, mod_data.transport_parm, charp, S_IRUGO);
MODULE_PARM_DESC(transport, "type of transport (BBB, CBI, or CB)");
@@ -386,9 +391,6 @@
MODULE_PARM_DESC(protocol, "type of protocol (RBC, 8020, QIC, UFI, "
"8070, or SCSI)");
-module_param_named(removable, mod_data.removable, bool, S_IRUGO);
-MODULE_PARM_DESC(removable, "true to simulate removable media");
-
module_param_named(vendor, mod_data.vendor, ushort, S_IRUGO);
MODULE_PARM_DESC(vendor, "USB Vendor ID");
@@ -525,11 +527,6 @@
* parts of the driver that aren't used in the non-TEST version. Even gcc
* can recognize when a test of a constant expression yields a dead code
* path.
- *
- * Also, in the non-TEST version, open_backing_file() is only used during
- * initialization and the sysfs attribute store_xxx routines aren't used
- * at all. We will define NORMALLY_INIT to mark them as __init so they
- * don't occupy kernel code space unnecessarily.
*/
#ifdef CONFIG_USB_FILE_STORAGE_TEST
@@ -537,16 +534,12 @@
#define transport_is_bbb() (mod_data.transport_type == USB_PR_BULK)
#define transport_is_cbi() (mod_data.transport_type == USB_PR_CBI)
#define protocol_is_scsi() (mod_data.protocol_type == USB_SC_SCSI)
-#define backing_file_is_open(curlun) ((curlun)->filp != NULL)
-#define NORMALLY_INIT
#else
#define transport_is_bbb() 1
#define transport_is_cbi() 0
#define protocol_is_scsi() 1
-#define backing_file_is_open(curlun) 1
-#define NORMALLY_INIT __init
#endif /* CONFIG_USB_FILE_STORAGE_TEST */
@@ -567,6 +560,8 @@
struct device dev;
};
+#define backing_file_is_open(curlun) ((curlun)->filp != NULL)
+
static inline struct lun *dev_to_lun(struct device *dev)
{
return container_of(dev, struct lun, dev);
@@ -659,6 +654,7 @@
unsigned long atomic_bitflags;
#define REGISTERED 0
#define CLEAR_BULK_HALTS 1
+#define SUSPENDED 2
struct usb_ep *bulk_in;
struct usb_ep *bulk_out;
@@ -1041,8 +1037,6 @@
function = fs_function;
len = usb_gadget_config_buf(&config_desc, buf, EP0_BUFSIZE, function);
- if (len < 0)
- return len;
((struct usb_config_descriptor *) buf)->bDescriptorType = type;
return len;
}
@@ -1172,9 +1166,10 @@
wakeup_thread(fsg);
}
+
+#ifdef CONFIG_USB_FILE_STORAGE_TEST
static void intr_in_complete(struct usb_ep *ep, struct usb_request *req)
{
-#ifdef CONFIG_USB_FILE_STORAGE_TEST
struct fsg_dev *fsg = (struct fsg_dev *) ep->driver_data;
struct fsg_buffhd *bh = (struct fsg_buffhd *) req->context;
@@ -1190,17 +1185,21 @@
bh->state = BUF_STATE_EMPTY;
spin_unlock(&fsg->lock);
wakeup_thread(fsg);
-#endif /* CONFIG_USB_FILE_STORAGE_TEST */
}
+#else
+static void intr_in_complete(struct usb_ep *ep, struct usb_request *req)
+{}
+#endif /* CONFIG_USB_FILE_STORAGE_TEST */
+
/*-------------------------------------------------------------------------*/
/* Ep0 class-specific handlers. These always run in_irq. */
+#ifdef CONFIG_USB_FILE_STORAGE_TEST
static void received_cbi_adsc(struct fsg_dev *fsg, struct fsg_buffhd *bh)
{
-#ifdef CONFIG_USB_FILE_STORAGE_TEST
struct usb_request *req = fsg->ep0req;
static u8 cbi_reset_cmnd[6] = {
SC_SEND_DIAGNOSTIC, 4, 0xff, 0xff, 0xff, 0xff};
@@ -1238,9 +1237,13 @@
spin_unlock(&fsg->lock);
wakeup_thread(fsg);
-#endif /* CONFIG_USB_FILE_STORAGE_TEST */
}
+#else
+static void received_cbi_adsc(struct fsg_dev *fsg, struct fsg_buffhd *bh)
+{}
+#endif /* CONFIG_USB_FILE_STORAGE_TEST */
+
static int class_setup_req(struct fsg_dev *fsg,
const struct usb_ctrlrequest *ctrl)
@@ -1465,8 +1468,8 @@
/* Respond with data/status or defer until later? */
if (rc >= 0 && rc != DELAYED_STATUS) {
fsg->ep0req->length = rc;
- fsg->ep0req->zero = rc < ctrl->wLength
- && (rc % gadget->ep0->maxpacket) == 0;
+ fsg->ep0req->zero = (rc < ctrl->wLength &&
+ (rc % gadget->ep0->maxpacket) == 0);
fsg->ep0req_name = (ctrl->bRequestType & USB_DIR_IN ?
"ep0-in" : "ep0-out");
rc = ep0_queue(fsg);
@@ -2443,14 +2446,19 @@
rc = -EINTR;
}
- /* We haven't processed all the incoming data. If we are
- * allowed to stall, halt the bulk-out endpoint and cancel
- * any outstanding requests. */
+ /* We haven't processed all the incoming data. Even though
+ * we may be allowed to stall, doing so would cause a race.
+ * The controller may already have ACK'ed all the remaining
+ * bulk-out packets, in which case the host wouldn't see a
+ * STALL. Not realizing the endpoint was halted, it wouldn't
+ * clear the halt -- leading to problems later on. */
+#if 0
else if (mod_data.can_stall) {
fsg_set_halt(fsg, fsg->bulk_out);
raise_exception(fsg, FSG_STATE_ABORT_BULK_OUT);
rc = -EINTR;
}
+#endif
/* We can't stall. Read in the excess data and throw it
* all away. */
@@ -2513,7 +2521,7 @@
} else if (mod_data.transport_type == USB_PR_CB) {
- /* Control-Bulk transport has no status stage! */
+ /* Control-Bulk transport has no status phase! */
return 0;
} else { // USB_PR_CBI
@@ -2603,8 +2611,10 @@
fsg->residue = fsg->usb_amount_left = fsg->data_size;
/* Conflicting data directions is a phase error */
- if (fsg->data_dir != data_dir && fsg->data_size_from_cmnd > 0)
- goto phase_error;
+ if (fsg->data_dir != data_dir && fsg->data_size_from_cmnd > 0) {
+ fsg->phase_error = 1;
+ return -EINVAL;
+ }
/* Verify the length of the command itself */
if (cmnd_size != fsg->cmnd_size) {
@@ -2613,8 +2623,10 @@
* with cbw->Length == 12 (it should be 6). */
if (fsg->cmnd[0] == SC_REQUEST_SENSE && fsg->cmnd_size == 12)
cmnd_size = fsg->cmnd_size;
- else
- goto phase_error;
+ else {
+ fsg->phase_error = 1;
+ return -EINVAL;
+ }
}
/* Check that the LUN values are oonsistent */
@@ -2674,10 +2686,6 @@
}
return 0;
-
-phase_error:
- fsg->phase_error = 1;
- return -EINVAL;
}
@@ -3424,8 +3432,7 @@
/* If the next two routines are called while the gadget is registered,
* the caller must own fsg->filesem for writing. */
-static int NORMALLY_INIT open_backing_file(struct lun *curlun,
- const char *filename)
+static int open_backing_file(struct lun *curlun, const char *filename)
{
int ro;
struct file *filp = NULL;
@@ -3550,8 +3557,7 @@
}
-ssize_t NORMALLY_INIT store_ro(struct device *dev, const char *buf,
- size_t count)
+ssize_t store_ro(struct device *dev, const char *buf, size_t count)
{
ssize_t rc = count;
struct lun *curlun = dev_to_lun(dev);
@@ -3575,8 +3581,7 @@
return rc;
}
-ssize_t NORMALLY_INIT store_file(struct device *dev, const char *buf,
- size_t count)
+ssize_t store_file(struct device *dev, const char *buf, size_t count)
{
struct lun *curlun = dev_to_lun(dev);
struct fsg_dev *fsg = (struct fsg_dev *) dev_get_drvdata(dev);
@@ -3805,9 +3810,8 @@
goto out;
}
- /* Create the LUNs and open their backing files. We can't register
- * the LUN devices until the gadget itself is registered, which
- * doesn't happen until after fsg_bind() returns. */
+ /* Create the LUNs, open their backing files, and register the
+ * LUN devices in sysfs. */
fsg->luns = kmalloc(i * sizeof(struct lun), GFP_KERNEL);
if (!fsg->luns) {
rc = -ENOMEM;
@@ -3825,6 +3829,15 @@
snprintf(curlun->dev.bus_id, BUS_ID_SIZE,
"%s-lun%d", gadget->dev.bus_id, i);
+ if ((rc = device_register(&curlun->dev)) != 0)
+ INFO(fsg, "failed to register LUN%d: %d\n", i, rc);
+ else {
+ curlun->registered = 1;
+ curlun->dev.release = lun_release;
+ device_create_file(&curlun->dev, &dev_attr_ro);
+ device_create_file(&curlun->dev, &dev_attr_file);
+ }
+
if (file[i] && *file[i]) {
if ((rc = open_backing_file(curlun, file[i])) != 0)
goto out;
@@ -3974,6 +3987,25 @@
/*-------------------------------------------------------------------------*/
+static void fsg_suspend(struct usb_gadget *gadget)
+{
+ struct fsg_dev *fsg = get_gadget_data(gadget);
+
+ DBG(fsg, "suspend\n");
+ set_bit(SUSPENDED, &fsg->atomic_bitflags);
+}
+
+static void fsg_resume(struct usb_gadget *gadget)
+{
+ struct fsg_dev *fsg = get_gadget_data(gadget);
+
+ DBG(fsg, "resume\n");
+ clear_bit(SUSPENDED, &fsg->atomic_bitflags);
+}
+
+
+/*-------------------------------------------------------------------------*/
+
static struct usb_gadget_driver fsg_driver = {
#ifdef CONFIG_USB_GADGET_DUALSPEED
.speed = USB_SPEED_HIGH,
@@ -3985,6 +4017,8 @@
.unbind = fsg_unbind,
.disconnect = fsg_disconnect,
.setup = fsg_setup,
+ .suspend = fsg_suspend,
+ .resume = fsg_resume,
.driver = {
.name = (char *) shortname,
@@ -4024,8 +4058,6 @@
{
int rc;
struct fsg_dev *fsg;
- int i;
- struct lun *curlun;
if ((rc = fsg_alloc()) != 0)
return rc;
@@ -4035,19 +4067,6 @@
return rc;
}
set_bit(REGISTERED, &fsg->atomic_bitflags);
-
- /* Register the LUN devices and their attribute files */
- for (i = 0; i < fsg->nluns; ++i) {
- curlun = &fsg->luns[i];
- if ((rc = device_register(&curlun->dev)) != 0)
- INFO(fsg, "failed to register LUN%d: %d\n", i, rc);
- else {
- curlun->registered = 1;
- curlun->dev.release = lun_release;
- device_create_file(&curlun->dev, &dev_attr_ro);
- device_create_file(&curlun->dev, &dev_attr_file);
- }
- }
/* Tell the thread to start working */
complete(&fsg->thread_notifier);
-------------------------------------------------------
This SF.Net email is sponsored by OSTG. Have you noticed the changes on
Linux.com, ITManagersJournal and NewsForge in the past few weeks? Now,
one more big change to announce. We are now OSTG- Open Source Technology
Group. Come see the changes on the new OSTG site. www.ostg.com
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
