public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* PATCH: usb-storage: move to SCSI hotplugging
@ 2003-01-19 22:30 Matthew Dharm
  2003-01-19 22:56 ` Oliver Neukum
  2003-01-20  1:16 ` PATCH: usb-storage: fix typo Matthew Dharm
  0 siblings, 2 replies; 9+ messages in thread
From: Matthew Dharm @ 2003-01-19 22:30 UTC (permalink / raw)
  To: Greg KH, USB Developers, USB Storage List; +Cc: Mike Anderson, Linux SCSI list

[-- Attachment #1: Type: text/plain, Size: 34776 bytes --]

The attached patch is my first implementation of SCSI hotplugging.

It's only been tested that it compiles, as I can't get the current
linux-2.5 tree from linuxusb to boot.  It dies _very_ early.  Greg, I'm not
sure if you'll want to apply this.  Linus seemed to want this very much,
and it is 2.5.x... I say go for it, but I can understand if you have
reservations.

I would definately like to see this tested by anyone who can get a kernel
to boot.

This patch is quite large.  Lots of things had to be changed.  Among them:

(o) The proc interface now uses the host number to look up the SCSI host
    structure, and then finds the usb-storage structure from that.
(o) The SCSI interface has been changed.  The code flow is now much
    clearer, as more work is done from the USB probe/detach functions than
    from auxillary functions.
(o) Names have been changed for newer conventions
(o) GUIDs have been removed
(o) The linked-list of devices has been removed, and it's associated
    semaphore
(o) All code dealing with re-attaching a device to it's old association has
    been removed
(o) Some spaces changed to tabs
(o) usb-storage now takes one directory under /proc/scsi instead of
    one per virtual-HBA
(o) All control threads now have the same name.  This could be changed back
    to the old behavior, if enough people want it.

Known problems:
(o) Testing, testing, testing
(o) More dead code needs to be cut
(o) It's a unclear how a LLD is supposed to cut off the flow of
    commands, so that the unregister() call always succeeds.  SCSI folks
    need to work on this.
(o) Probing needs to be broken down into smaller functions, probably.

Matt

# This is a BitKeeper generated patch for the following project:
# Project Name: greg k-h's linux 2.5 USB kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.666   -> 1.667  
#	drivers/usb/storage/usb.h	1.31    -> 1.35   
#	drivers/usb/storage/scsiglue.h	1.4     -> 1.5    
#	drivers/usb/storage/scsiglue.c	1.35    -> 1.39   
#	drivers/usb/storage/usb.c	1.64    -> 1.68   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/01/19	mdharm@zen.san.one-eyed-alien.net	1.667
# First pass implementation of SCSI hotplug/unplug.
# --------------------------------------------
#
diff -Nru a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
--- a/drivers/usb/storage/scsiglue.c	Sun Jan 19 14:10:31 2003
+++ b/drivers/usb/storage/scsiglue.c	Sun Jan 19 14:10:31 2003
@@ -50,23 +50,25 @@
 #include "transport.h"
 
 #include <linux/slab.h>
+#include <linux/module.h>
 
 
 /***********************************************************************
  * Host functions 
  ***********************************************************************/
 
-static const char* host_info(struct Scsi_Host *host)
+static const char* usb_storage_info(struct Scsi_Host *host)
 {
 	return "SCSI emulation for USB Mass Storage devices";
 }
 
+#if 0
 /* detect a virtual adapter (always works)
  * Synchronization: 2.4: with the io_request_lock
  * 			2.5: no locks.
  * fortunately we don't care.
  * */
-static int detect(struct SHT *sht)
+static int usb_storage_detect(struct SHT *sht)
 {
 	struct us_data *us;
 	char local_name[32];
@@ -109,7 +111,7 @@
  * the driver and we're doing each virtual host in turn, not in parallel
  * Synchronization: BKL, no spinlock.
  */
-static int release(struct Scsi_Host *psh)
+static int usb_storage_release(struct Scsi_Host *psh)
 {
 	struct us_data *us = (struct us_data *)psh->hostdata[0];
 
@@ -132,18 +134,11 @@
 	/* we always have a successful release */
 	return 0;
 }
-
-/* run command */
-static int command( Scsi_Cmnd *srb )
-{
-	US_DEBUGP("Bad use of us_command\n");
-
-	return DID_BAD_TARGET << 16;
-}
+#endif
 
 /* queue a command */
 /* This is always called with scsi_lock(srb->host) held */
-static int queuecommand( Scsi_Cmnd *srb , void (*done)(Scsi_Cmnd *))
+static int usb_storage_queuecommand( Scsi_Cmnd *srb , void (*done)(Scsi_Cmnd *))
 {
 	struct us_data *us = (struct us_data *)srb->host->hostdata[0];
 
@@ -168,7 +163,7 @@
 
 /* Command abort */
 /* This is always called with scsi_lock(srb->host) held */
-static int command_abort( Scsi_Cmnd *srb )
+static int usb_storage_command_abort( Scsi_Cmnd *srb )
 {
 	struct us_data *us = (struct us_data *)srb->host->hostdata[0];
 
@@ -187,7 +182,7 @@
 /* This invokes the transport reset mechanism to reset the state of the
  * device */
 /* This is always called with scsi_lock(srb->host) held */
-static int device_reset( Scsi_Cmnd *srb )
+static int usb_storage_device_reset( Scsi_Cmnd *srb )
 {
 	struct us_data *us = (struct us_data *)srb->host->hostdata[0];
 	int result;
@@ -219,7 +214,7 @@
  * disconnect/reconnect for all drivers which have claimed
  * interfaces, including ourself. */
 /* This is always called with scsi_lock(srb->host) held */
-static int bus_reset( Scsi_Cmnd *srb )
+static int usb_storage_bus_reset( Scsi_Cmnd *srb )
 {
 	struct us_data *us = (struct us_data *)srb->host->hostdata[0];
 	int i;
@@ -274,14 +269,6 @@
 	return SUCCESS;
 }
 
-/* FIXME: This doesn't do anything right now */
-static int host_reset( Scsi_Cmnd *srb )
-{
-	printk(KERN_CRIT "usb-storage: host_reset() requested but not implemented\n" );
-	bus_reset(srb);
-	return FAILED;
-}
-
 /***********************************************************************
  * /proc/scsi/ functions
  ***********************************************************************/
@@ -291,29 +278,24 @@
 #define SPRINTF(args...) \
 	do { if (pos < buffer+length) pos += sprintf(pos, ## args); } while (0)
 
-static int proc_info (char *buffer, char **start, off_t offset, int length,
-		int hostno, int inout)
+static int usb_storage_proc_info (char *buffer, char **start, off_t offset,
+		int length, int hostno, int inout)
 {
 	struct us_data *us;
 	char *pos = buffer;
+	struct Scsi_Host *hostptr;
 
 	/* if someone is sending us data, just throw it away */
 	if (inout)
 		return length;
 
-	/* lock the data structures */
-	down(&us_list_semaphore);
-
-	/* find our data from hostno */
-	us = us_list;
-	while (us) {
-		if (us->host_no == hostno)
-			break;
-		us = us->next;
+	/* find our data from the given hostno */
+	hostptr = scsi_host_hn_get(hostno);
+	if (!hostptr) {	 /* if we couldn't find it, we return an error */
+		return -ESRCH;
 	}
-
-	/* release our lock on the data structures */
-	up(&us_list_semaphore);
+	us = (struct us_data*)hostptr->hostdata[0];
+	scsi_host_put(hostptr);
 
 	/* if we couldn't find it, we return an error */
 	if (!us) {
@@ -332,8 +314,7 @@
 	SPRINTF("     Protocol: %s\n", us->protocol_name);
 	SPRINTF("    Transport: %s\n", us->transport_name);
 
-	/* show the GUID of the device */
-	SPRINTF("         GUID: " GUID_FORMAT "\n", GUID_ARGS(us->guid));
+	/* show attached status of the device */
 	SPRINTF("     Attached: %s\n", (us->flags & US_FL_DEV_ATTACHED ?
 			"Yes" : "No"));
 
@@ -351,33 +332,69 @@
 }
 
 /*
- * this defines our 'host'
+ * this defines our host template, with which we'll allocate hosts
  */
 
-Scsi_Host_Template usb_stor_host_template = {
-	.name =			"usb-storage",
-	.proc_info =		proc_info,
-	.info =			host_info,
-
-	.detect =		detect,
-	.release =		release,
-	.command =		command,
-	.queuecommand =		queuecommand,
-
-	.eh_abort_handler =	command_abort,
-	.eh_device_reset_handler =device_reset,
-	.eh_bus_reset_handler =	bus_reset,
-	.eh_host_reset_handler =host_reset,
-
-	.can_queue =		1,
-	.this_id =		-1,
-
-	.sg_tablesize =		SG_ALL,
-	.cmd_per_lun =		1,
-	.present =		0,
-	.unchecked_isa_dma =	FALSE,
-	.use_clustering =	TRUE,
-	.emulated =		TRUE
+struct SHT usb_stor_host_template = {
+	/* basic userland interface stuff */
+	.name =				"usb-storage",
+	.proc_name =			"usb-storage",
+	.proc_info =			usb_storage_proc_info,
+	.proc_dir =			NULL,
+	.info =				usb_storage_info,
+	.ioctl =			NULL,
+
+	/* old-style detect and release */
+	.detect =			NULL,
+	.release =			NULL,
+
+	/* command interface -- queued only */
+	.command =			NULL,
+	.queuecommand =			usb_storage_queuecommand,
+
+	/* error and abort handlers */
+	.eh_abort_handler =		usb_storage_command_abort,
+	.eh_device_reset_handler =	usb_storage_device_reset,
+	.eh_bus_reset_handler =		usb_storage_bus_reset,
+	.eh_host_reset_handler =	NULL,
+	.eh_strategy_handler =		NULL,
+
+	/* queue commands only, only one command per LUN */
+	.can_queue =			1,
+	.cmd_per_lun =			1,
+
+	/* unknown initiator id */
+	.this_id =			-1,
+
+	/* no limit on commands */
+	.max_sectors =			0,
+	
+	/* pre- and post- device scan functions */
+	.slave_alloc =			NULL,
+	.slave_configure =		NULL,
+	.slave_destroy =		NULL,
+
+	/* lots of sg segments can be handled */
+	.sg_tablesize =			SG_ALL,
+
+	/* use 32-bit address space for DMA */
+	.unchecked_isa_dma =		FALSE,
+	.highmem_io =			FALSE,
+
+	/* merge commands... this seems to help performance, but
+	 * periodically someone should test to see which setting is more
+	 * optimal.
+	 */
+	.use_clustering =		TRUE,
+
+	/* emulated HBA */
+	.emulated =			TRUE,
+
+	/* sorry, no BIOS to help us */
+	.bios_param =			NULL,
+
+	/* module management */
+	.module =			THIS_MODULE
 };
 
 /* For a device that is "Not Ready" */
diff -Nru a/drivers/usb/storage/scsiglue.h b/drivers/usb/storage/scsiglue.h
--- a/drivers/usb/storage/scsiglue.h	Sun Jan 19 14:10:31 2003
+++ b/drivers/usb/storage/scsiglue.h	Sun Jan 19 14:10:31 2003
@@ -47,7 +47,7 @@
 
 extern unsigned char usb_stor_sense_notready[18];
 extern unsigned char usb_stor_sense_invalidCDB[18];
-extern Scsi_Host_Template usb_stor_host_template;
+extern struct SHT usb_stor_host_template;
 extern int usb_stor_scsiSense10to6(Scsi_Cmnd*);
 extern int usb_stor_scsiSense6to10(Scsi_Cmnd*);
 
diff -Nru a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
--- a/drivers/usb/storage/usb.c	Sun Jan 19 14:10:31 2003
+++ b/drivers/usb/storage/usb.c	Sun Jan 19 14:10:31 2003
@@ -93,16 +93,6 @@
 MODULE_DESCRIPTION("USB Mass Storage driver for Linux");
 MODULE_LICENSE("GPL");
 
-/*
- * Per device data
- */
-
-static int my_host_number;
-
-/* The list of structures and the protective lock for them */
-struct us_data *us_list;
-struct semaphore us_list_semaphore;
-
 static int storage_probe(struct usb_interface *iface,
 			 const struct usb_device_id *id);
 
@@ -319,7 +309,7 @@
 	spin_unlock_irq(&current->sig->siglock);
 
 	/* set our name for identification purposes */
-	sprintf(current->comm, "usb-storage-%d", us->host_number);
+	sprintf(current->comm, "usb-storage");
 
 	unlock_kernel();
 
@@ -563,12 +553,10 @@
 	char mf[USB_STOR_STRING_LEN];		     /* manufacturer */
 	char prod[USB_STOR_STRING_LEN];		     /* product */
 	char serial[USB_STOR_STRING_LEN];	     /* serial number */
-	GUID(guid);			   /* Global Unique Identifier */
 	unsigned int flags;
 	struct us_unusual_dev *unusual_dev;
 	struct us_data *ss = NULL;
 	int result;
-	int new_device = 0;
 
 	/* these are temporary copies -- we test on these, then put them
 	 * in the us-data structure 
@@ -676,8 +664,7 @@
 	/* At this point, we've decided to try to use the device */
 	usb_get_dev(dev);
 
-	/* clear the GUID and fetch the strings */
-	GUID_CLEAR(guid);
+	/* fetch the strings */
 	if (dev->descriptor.iManufacturer)
 		usb_string(dev, dev->descriptor.iManufacturer, 
 			   mf, sizeof(mf));
@@ -688,348 +675,276 @@
 		usb_string(dev, dev->descriptor.iSerialNumber, 
 			   serial, sizeof(serial));
 
-	/* Create a GUID for this device */
-	if (dev->descriptor.iSerialNumber && serial[0]) {
-		/* If we have a serial number, and it's a non-NULL string */
-		make_guid(guid, dev->descriptor.idVendor, 
-			  dev->descriptor.idProduct, serial);
-	} else {
-		/* We don't have a serial number, so we use 0 */
-		make_guid(guid, dev->descriptor.idVendor, 
-			  dev->descriptor.idProduct, "0");
+	/* New device -- allocate memory and initialize */
+	if ((ss = (struct us_data *)kmalloc(sizeof(struct us_data), 
+					    GFP_KERNEL)) == NULL) {
+		printk(KERN_WARNING USB_STORAGE "Out of memory\n");
+		usb_put_dev(dev);
+		return -ENOMEM;
+	}
+	memset(ss, 0, sizeof(struct us_data));
+
+	/* Initialize the mutexes only when the struct is new */
+	init_completion(&(ss->notify));
+	init_MUTEX_LOCKED(&(ss->dev_semaphore));
+
+	/* copy over the subclass and protocol data */
+	ss->subclass = subclass;
+	ss->protocol = protocol;
+	ss->flags = flags | US_FL_DEV_ATTACHED;
+	ss->unusual_dev = unusual_dev;
+
+	/* copy over the endpoint data */
+	ss->ep_in = ep_in->bEndpointAddress & 
+		USB_ENDPOINT_NUMBER_MASK;
+	ss->ep_out = ep_out->bEndpointAddress & 
+		USB_ENDPOINT_NUMBER_MASK;
+	if (ep_int) {
+		ss->ep_int = ep_int->bEndpointAddress & 
+			USB_ENDPOINT_NUMBER_MASK;
+		ss->ep_bInterval = ep_int->bInterval;
 	}
+	else
+		ss->ep_int = ss->ep_bInterval = 0;
 
-	/*
-	 * Now check if we have seen this GUID before
-	 * We're looking for a device with a matching GUID that isn't
-	 * already on the system
-	 */
-	ss = us_list;
-	while ((ss != NULL) && 
-	           ((ss->flags & US_FL_DEV_ATTACHED) ||
-		    !GUID_EQUAL(guid, ss->guid)))
-		ss = ss->next;
-
-	if (ss != NULL) {
-		/* Existing device -- re-connect */
-		US_DEBUGP("Found existing GUID " GUID_FORMAT "\n",
-			  GUID_ARGS(guid));
-
-		/* lock the device pointers */
-		down(&(ss->dev_semaphore));
-
-		/* establish the connection to the new device upon reconnect */
-		ss->ifnum = ifnum;
-		ss->pusb_dev = dev;
-		ss->flags |= US_FL_DEV_ATTACHED;
-
-		/* copy over the endpoint data */
-		ss->ep_in = ep_in->bEndpointAddress & 
-			USB_ENDPOINT_NUMBER_MASK;
-		ss->ep_out = ep_out->bEndpointAddress & 
-			USB_ENDPOINT_NUMBER_MASK;
-		if (ep_int) {
-			ss->ep_int = ep_int->bEndpointAddress & 
-				USB_ENDPOINT_NUMBER_MASK;
-			ss->ep_bInterval = ep_int->bInterval;
-		}
+	/* establish the connection to the new device */
+	ss->ifnum = ifnum;
+	ss->pusb_dev = dev;
+
+	/* copy over the identifiying strings */
+	strncpy(ss->vendor, mf, USB_STOR_STRING_LEN);
+	strncpy(ss->product, prod, USB_STOR_STRING_LEN);
+	strncpy(ss->serial, serial, USB_STOR_STRING_LEN);
+	if (strlen(ss->vendor) == 0) {
+		if (unusual_dev->vendorName)
+			strncpy(ss->vendor, unusual_dev->vendorName,
+				USB_STOR_STRING_LEN);
 		else
-			ss->ep_int = ss->ep_bInterval = 0;
-
-		/* allocate the URB, the usb_ctrlrequest, and the IRQ URB */
-		if (usb_stor_allocate_urbs(ss))
-			goto BadDevice;
-
-                /* Re-Initialize the device if it needs it */
-		if (unusual_dev && unusual_dev->initFunction)
-			(unusual_dev->initFunction)(ss);
-
-		/* unlock the device pointers */
-		up(&(ss->dev_semaphore));
-
-	} else { 
-		/* New device -- allocate memory and initialize */
-		US_DEBUGP("New GUID " GUID_FORMAT "\n", GUID_ARGS(guid));
-
-		if ((ss = (struct us_data *)kmalloc(sizeof(struct us_data), 
-						    GFP_KERNEL)) == NULL) {
-			printk(KERN_WARNING USB_STORAGE "Out of memory\n");
-			usb_put_dev(dev);
-			return -ENOMEM;
-		}
-		memset(ss, 0, sizeof(struct us_data));
-		new_device = 1;
-
-		/* Initialize the mutexes only when the struct is new */
-		init_completion(&(ss->notify));
-		init_MUTEX_LOCKED(&(ss->dev_semaphore));
-
-		/* copy over the subclass and protocol data */
-		ss->subclass = subclass;
-		ss->protocol = protocol;
-		ss->flags = flags | US_FL_DEV_ATTACHED;
-		ss->unusual_dev = unusual_dev;
-
-		/* copy over the endpoint data */
-		ss->ep_in = ep_in->bEndpointAddress & 
-			USB_ENDPOINT_NUMBER_MASK;
-		ss->ep_out = ep_out->bEndpointAddress & 
-			USB_ENDPOINT_NUMBER_MASK;
-		if (ep_int) {
-			ss->ep_int = ep_int->bEndpointAddress & 
-				USB_ENDPOINT_NUMBER_MASK;
-			ss->ep_bInterval = ep_int->bInterval;
-		}
+			strncpy(ss->vendor, "Unknown",
+				USB_STOR_STRING_LEN);
+	}
+	if (strlen(ss->product) == 0) {
+		if (unusual_dev->productName)
+			strncpy(ss->product, unusual_dev->productName,
+				USB_STOR_STRING_LEN);
 		else
-			ss->ep_int = ss->ep_bInterval = 0;
-
-		/* establish the connection to the new device */
-		ss->ifnum = ifnum;
-		ss->pusb_dev = dev;
-
-		/* copy over the identifiying strings */
-		strncpy(ss->vendor, mf, USB_STOR_STRING_LEN);
-		strncpy(ss->product, prod, USB_STOR_STRING_LEN);
-		strncpy(ss->serial, serial, USB_STOR_STRING_LEN);
-		if (strlen(ss->vendor) == 0) {
-			if (unusual_dev->vendorName)
-				strncpy(ss->vendor, unusual_dev->vendorName,
-					USB_STOR_STRING_LEN);
-			else
-				strncpy(ss->vendor, "Unknown",
-					USB_STOR_STRING_LEN);
-		}
-		if (strlen(ss->product) == 0) {
-			if (unusual_dev->productName)
-				strncpy(ss->product, unusual_dev->productName,
-					USB_STOR_STRING_LEN);
-			else
-				strncpy(ss->product, "Unknown",
-					USB_STOR_STRING_LEN);
-		}
-		if (strlen(ss->serial) == 0)
-			strncpy(ss->serial, "None", USB_STOR_STRING_LEN);
-
-		/* copy the GUID we created before */
-		memcpy(ss->guid, guid, sizeof(guid));
-
-		/* 
-		 * Set the handler pointers based on the protocol
-		 * Again, this data is persistant across reattachments
-		 */
-		switch (ss->protocol) {
-		case US_PR_CB:
-			ss->transport_name = "Control/Bulk";
-			ss->transport = usb_stor_CB_transport;
-			ss->transport_reset = usb_stor_CB_reset;
-			ss->max_lun = 7;
-			break;
-
-		case US_PR_CBI:
-			ss->transport_name = "Control/Bulk/Interrupt";
-			ss->transport = usb_stor_CBI_transport;
-			ss->transport_reset = usb_stor_CB_reset;
-			ss->max_lun = 7;
-			break;
+			strncpy(ss->product, "Unknown",
+				USB_STOR_STRING_LEN);
+	}
+	if (strlen(ss->serial) == 0)
+		strncpy(ss->serial, "None", USB_STOR_STRING_LEN);
 
-		case US_PR_BULK:
-			ss->transport_name = "Bulk";
-			ss->transport = usb_stor_Bulk_transport;
-			ss->transport_reset = usb_stor_Bulk_reset;
-			ss->max_lun = usb_stor_Bulk_max_lun(ss);
-			break;
+	/* 
+	 * Set the handler pointers based on the protocol
+	 * Again, this data is persistant across reattachments
+	 */
+	switch (ss->protocol) {
+	case US_PR_CB:
+		ss->transport_name = "Control/Bulk";
+		ss->transport = usb_stor_CB_transport;
+		ss->transport_reset = usb_stor_CB_reset;
+		ss->max_lun = 7;
+		break;
+
+	case US_PR_CBI:
+		ss->transport_name = "Control/Bulk/Interrupt";
+		ss->transport = usb_stor_CBI_transport;
+		ss->transport_reset = usb_stor_CB_reset;
+		ss->max_lun = 7;
+		break;
+
+	case US_PR_BULK:
+		ss->transport_name = "Bulk";
+		ss->transport = usb_stor_Bulk_transport;
+		ss->transport_reset = usb_stor_Bulk_reset;
+		ss->max_lun = usb_stor_Bulk_max_lun(ss);
+		break;
 
 #ifdef CONFIG_USB_STORAGE_HP8200e
-		case US_PR_SCM_ATAPI:
-			ss->transport_name = "SCM/ATAPI";
-			ss->transport = hp8200e_transport;
-			ss->transport_reset = usb_stor_CB_reset;
-			ss->max_lun = 1;
-			break;
+	case US_PR_SCM_ATAPI:
+		ss->transport_name = "SCM/ATAPI";
+		ss->transport = hp8200e_transport;
+		ss->transport_reset = usb_stor_CB_reset;
+		ss->max_lun = 1;
+		break;
 #endif
 
 #ifdef CONFIG_USB_STORAGE_SDDR09
-		case US_PR_EUSB_SDDR09:
-			ss->transport_name = "EUSB/SDDR09";
-			ss->transport = sddr09_transport;
-			ss->transport_reset = usb_stor_CB_reset;
-			ss->max_lun = 0;
-			break;
+	case US_PR_EUSB_SDDR09:
+		ss->transport_name = "EUSB/SDDR09";
+		ss->transport = sddr09_transport;
+		ss->transport_reset = usb_stor_CB_reset;
+		ss->max_lun = 0;
+		break;
 #endif
 
 #ifdef CONFIG_USB_STORAGE_SDDR55
-		case US_PR_SDDR55:
-			ss->transport_name = "SDDR55";
-			ss->transport = sddr55_transport;
-			ss->transport_reset = sddr55_reset;
-			ss->max_lun = 0;
-			break;
+	case US_PR_SDDR55:
+		ss->transport_name = "SDDR55";
+		ss->transport = sddr55_transport;
+		ss->transport_reset = sddr55_reset;
+		ss->max_lun = 0;
+		break;
 #endif
 
 #ifdef CONFIG_USB_STORAGE_DPCM
-		case US_PR_DPCM_USB:
-			ss->transport_name = "Control/Bulk-EUSB/SDDR09";
-			ss->transport = dpcm_transport;
-			ss->transport_reset = usb_stor_CB_reset;
-			ss->max_lun = 1;
-			break;
+	case US_PR_DPCM_USB:
+		ss->transport_name = "Control/Bulk-EUSB/SDDR09";
+		ss->transport = dpcm_transport;
+		ss->transport_reset = usb_stor_CB_reset;
+		ss->max_lun = 1;
+		break;
 #endif
 
 #ifdef CONFIG_USB_STORAGE_FREECOM
-                case US_PR_FREECOM:
-                        ss->transport_name = "Freecom";
-                        ss->transport = freecom_transport;
-                        ss->transport_reset = usb_stor_freecom_reset;
-                        ss->max_lun = 0;
-                        break;
+	case US_PR_FREECOM:
+		ss->transport_name = "Freecom";
+		ss->transport = freecom_transport;
+		ss->transport_reset = usb_stor_freecom_reset;
+		ss->max_lun = 0;
+		break;
 #endif
 
 #ifdef CONFIG_USB_STORAGE_DATAFAB
-                case US_PR_DATAFAB:
-                        ss->transport_name  = "Datafab Bulk-Only";
-                        ss->transport = datafab_transport;
-                        ss->transport_reset = usb_stor_Bulk_reset;
-                        ss->max_lun = 1;
-                        break;
+	case US_PR_DATAFAB:
+		ss->transport_name  = "Datafab Bulk-Only";
+		ss->transport = datafab_transport;
+		ss->transport_reset = usb_stor_Bulk_reset;
+		ss->max_lun = 1;
+		break;
 #endif
 
 #ifdef CONFIG_USB_STORAGE_JUMPSHOT
-                case US_PR_JUMPSHOT:
-                        ss->transport_name  = "Lexar Jumpshot Control/Bulk";
-                        ss->transport = jumpshot_transport;
-                        ss->transport_reset = usb_stor_Bulk_reset;
-                        ss->max_lun = 1;
-                        break;
+	case US_PR_JUMPSHOT:
+		ss->transport_name  = "Lexar Jumpshot Control/Bulk";
+		ss->transport = jumpshot_transport;
+		ss->transport_reset = usb_stor_Bulk_reset;
+		ss->max_lun = 1;
+		break;
 #endif
 
-		default:
-			/* ss->transport_name = "Unknown"; */
-			goto BadDevice;
-		}
-		US_DEBUGP("Transport: %s\n", ss->transport_name);
-
-		/* fix for single-lun devices */
-		if (ss->flags & US_FL_SINGLE_LUN)
-			ss->max_lun = 0;
-
-		switch (ss->subclass) {
-		case US_SC_RBC:
-			ss->protocol_name = "Reduced Block Commands (RBC)";
-			ss->proto_handler = usb_stor_transparent_scsi_command;
-			break;
-
-		case US_SC_8020:
-			ss->protocol_name = "8020i";
-			ss->proto_handler = usb_stor_ATAPI_command;
-			ss->max_lun = 0;
-			break;
-
-		case US_SC_QIC:
-			ss->protocol_name = "QIC-157";
-			ss->proto_handler = usb_stor_qic157_command;
-			ss->max_lun = 0;
-			break;
-
-		case US_SC_8070:
-			ss->protocol_name = "8070i";
-			ss->proto_handler = usb_stor_ATAPI_command;
-			ss->max_lun = 0;
-			break;
-
-		case US_SC_SCSI:
-			ss->protocol_name = "Transparent SCSI";
-			ss->proto_handler = usb_stor_transparent_scsi_command;
-			break;
-
-		case US_SC_UFI:
-			ss->protocol_name = "Uniform Floppy Interface (UFI)";
-			ss->proto_handler = usb_stor_ufi_command;
-			break;
+	default:
+		/* ss->transport_name = "Unknown"; */
+		goto BadDevice;
+	}
+	US_DEBUGP("Transport: %s\n", ss->transport_name);
+
+	/* fix for single-lun devices */
+	if (ss->flags & US_FL_SINGLE_LUN)
+		ss->max_lun = 0;
+
+	switch (ss->subclass) {
+	case US_SC_RBC:
+		ss->protocol_name = "Reduced Block Commands (RBC)";
+		ss->proto_handler = usb_stor_transparent_scsi_command;
+		break;
+
+	case US_SC_8020:
+		ss->protocol_name = "8020i";
+		ss->proto_handler = usb_stor_ATAPI_command;
+		ss->max_lun = 0;
+		break;
+
+	case US_SC_QIC:
+		ss->protocol_name = "QIC-157";
+		ss->proto_handler = usb_stor_qic157_command;
+		ss->max_lun = 0;
+		break;
+
+	case US_SC_8070:
+		ss->protocol_name = "8070i";
+		ss->proto_handler = usb_stor_ATAPI_command;
+		ss->max_lun = 0;
+		break;
+
+	case US_SC_SCSI:
+		ss->protocol_name = "Transparent SCSI";
+		ss->proto_handler = usb_stor_transparent_scsi_command;
+		break;
+
+	case US_SC_UFI:
+		ss->protocol_name = "Uniform Floppy Interface (UFI)";
+		ss->proto_handler = usb_stor_ufi_command;
+		break;
 
 #ifdef CONFIG_USB_STORAGE_ISD200
-                case US_SC_ISD200:
-                        ss->protocol_name = "ISD200 ATA/ATAPI";
-                        ss->proto_handler = isd200_ata_command;
-                        break;
+	case US_SC_ISD200:
+		ss->protocol_name = "ISD200 ATA/ATAPI";
+		ss->proto_handler = isd200_ata_command;
+		break;
 #endif
 
-		default:
-			/* ss->protocol_name = "Unknown"; */
-			goto BadDevice;
-		}
-		US_DEBUGP("Protocol: %s\n", ss->protocol_name);
-
-		/* allocate the URB, the usb_ctrlrequest, and the IRQ URB */
-		if (usb_stor_allocate_urbs(ss))
-			goto BadDevice;
-
-		/*
-		 * Since this is a new device, we need to generate a scsi 
-		 * host definition, and register with the higher SCSI layers
-		 */
-
-		/* Initialize the host template based on the default one */
-		memcpy(&(ss->htmplt), &usb_stor_host_template, 
-		       sizeof(usb_stor_host_template));
-
-		/* Grab the next host number */
-		ss->host_number = my_host_number++;
-
-		/* We abuse this pointer so we can pass the ss pointer to 
-		 * the host controller thread in us_detect.  But how else are
-		 * we to do it?
-		 */
-		(struct us_data *)ss->htmplt.proc_dir = ss; 
-
-		/* Just before we start our control thread, initialize
-		 * the device if it needs initialization */
-		if (unusual_dev && unusual_dev->initFunction)
-			unusual_dev->initFunction(ss);
-
-		/* start up our control thread */
-		atomic_set(&ss->sm_state, US_STATE_IDLE);
-		ss->pid = kernel_thread(usb_stor_control_thread, ss,
-					CLONE_VM);
-		if (ss->pid < 0) {
-			printk(KERN_WARNING USB_STORAGE 
-			       "Unable to start control thread\n");
-			goto BadDevice;
-		}
-
-		/* wait for the thread to start */
-		wait_for_completion(&(ss->notify));
+	default:
+		/* ss->protocol_name = "Unknown"; */
+		goto BadDevice;
+	}
+	US_DEBUGP("Protocol: %s\n", ss->protocol_name);
+
+	/* allocate the URB, the usb_ctrlrequest, and the IRQ URB */
+	if (usb_stor_allocate_urbs(ss))
+		goto BadDevice;
 
-		/* unlock the device pointers */
-		up(&(ss->dev_semaphore));
+	/*
+	 * Since this is a new device, we need to generate a scsi 
+	 * host definition, and register with the higher SCSI layers
+	 */
 
-		/* now register	 - our detect function will be called */
-		ss->htmplt.module = THIS_MODULE;
-		result = scsi_register_host(&(ss->htmplt));
-		if (result) {
-			printk(KERN_WARNING USB_STORAGE
-				"Unable to register the scsi host\n");
-
-			/* tell the control thread to exit */
-			ss->srb = NULL;
-			up(&ss->sema);
-			wait_for_completion(&ss->notify);
-
-			/* re-lock the device pointers */
-			down(&ss->dev_semaphore);
-			goto BadDevice;
-		}
+	/* Just before we start our control thread, initialize
+	 * the device if it needs initialization */
+	if (unusual_dev && unusual_dev->initFunction)
+		unusual_dev->initFunction(ss);
+
+	/* start up our control thread */
+	atomic_set(&ss->sm_state, US_STATE_IDLE);
+	ss->pid = kernel_thread(usb_stor_control_thread, ss,
+				CLONE_VM);
+	if (ss->pid < 0) {
+		printk(KERN_WARNING USB_STORAGE 
+		       "Unable to start control thread\n");
+		goto BadDevice;
+	}
 
-		/* lock access to the data structures */
-		down(&us_list_semaphore);
+	/* wait for the thread to start */
+	wait_for_completion(&(ss->notify));
 
-		/* put us in the list */
-		ss->next = us_list;
-		us_list = ss;
+	/* unlock the device pointers */
+	up(&(ss->dev_semaphore));
 
-		/* release the data structure lock */
-		up(&us_list_semaphore);
+	/* now register	*/
+	ss->host = scsi_register(&usb_stor_host_template, sizeof(ss));
+	if (ss->host) {
+		printk(KERN_WARNING USB_STORAGE
+			"Unable to register the scsi host\n");
+
+		/* tell the control thread to exit */
+		ss->srb = NULL;
+		up(&ss->sema);
+		wait_for_completion(&ss->notify);
+
+		/* re-lock the device pointers */
+		down(&ss->dev_semaphore);
+		goto BadDevice;
+	}
+
+	/* now add the host */
+	result = scsi_add_host(ss->host, NULL);
+	if (result) {
+		printk(KERN_WARNING USB_STORAGE
+			"Unable to add the scsi host\n");
+
+		/* tell the control thread to exit */
+		ss->srb = NULL;
+		up(&ss->sema);
+		wait_for_completion(&ss->notify);
+
+		/* re-lock the device pointers */
+		down(&ss->dev_semaphore);
+		goto BadDevice;
 	}
 
+	ss->host->hostdata[0] = (unsigned long)ss;
+	scsi_set_device(ss->host, &intf->dev);
+
 	printk(KERN_DEBUG 
 	       "WARNING: USB Mass Storage data integrity not assured\n");
 	printk(KERN_DEBUG 
@@ -1041,12 +956,11 @@
 
 	/* we come here if there are any problems */
 	/* ss->dev_semaphore must be locked */
-	BadDevice:
+BadDevice:
 	US_DEBUGP("storage_probe() failed\n");
 	usb_stor_deallocate_urbs(ss);
 	up(&ss->dev_semaphore);
-	if (new_device)
-		kfree(ss);
+	kfree(ss);
 	return -EIO;
 }
 
@@ -1065,9 +979,52 @@
 		return;
 	}
 
+	/* lock device access -- no need to unlock, as we're going away */
 	down(&(ss->dev_semaphore));
+
+	/* remove the pointer to the data structure we were using */
+	(struct us_data*)ss->host->hostdata[0] = NULL;
+
+	/* begin SCSI host removal sequence */
+	if(scsi_remove_host(ss->host)) {
+		US_DEBUGP("-- SCSI refused to unregister\n");
+		BUG();
+		return;
+	};
+
+	/* finish SCSI host removal sequence */
+	scsi_unregister(ss->host);
+
+	/* Kill the control threads
+	 *
+	 * Enqueue the command, wake up the thread, and wait for 
+	 * notification that it has exited.
+	 */
+	US_DEBUGP("-- sending exit command to thread\n");
+	BUG_ON(atomic_read(&ss->sm_state) != US_STATE_IDLE);
+	ss->srb = NULL;
+	up(&(ss->sema));
+	wait_for_completion(&(ss->notify));
+
+	/* free allocated urbs */
 	usb_stor_deallocate_urbs(ss);
-	up(&(ss->dev_semaphore));
+
+	/* If there's extra data in the us_data structure then
+	 * free that first */
+	if (ss->extra) {
+		/* call the destructor routine, if it exists */
+		if (ss->extra_destructor) {
+			US_DEBUGP("-- calling extra_destructor()\n");
+			ss->extra_destructor(ss->extra);
+		}
+
+		/* destroy the extra data */
+		US_DEBUGP("-- freeing the data structure\n");
+		kfree(ss->extra);
+	}
+
+	/* free the structure itself */
+	kfree (ss);
 }
 
 /***********************************************************************
@@ -1078,11 +1035,6 @@
 {
 	printk(KERN_INFO "Initializing USB Mass Storage driver...\n");
 
-	/* initialize internal global data elements */
-	us_list = NULL;
-	init_MUTEX(&us_list_semaphore);
-	my_host_number = 0;
-
 	/* register the driver, return -1 if error */
 	if (usb_register(&usb_storage_driver) < 0)
 		return -1;
@@ -1094,16 +1046,16 @@
 
 void __exit usb_stor_exit(void)
 {
-	struct us_data *next;
-
 	US_DEBUGP("usb_stor_exit() called\n");
 
 	/* Deregister the driver
-	 * This eliminates races with probes and disconnects 
+	 * This will cause disconnect() to be called for each
+	 * attached unit
 	 */
 	US_DEBUGP("-- calling usb_deregister()\n");
 	usb_deregister(&usb_storage_driver) ;
 
+#if 0
 	/* While there are still virtual hosts, unregister them
 	 * Note that it's important to do this completely before removing
 	 * the structures because of possible races with the /proc
@@ -1111,7 +1063,7 @@
 	 */
 	for (next = us_list; next; next = next->next) {
 		US_DEBUGP("-- calling scsi_unregister_host()\n");
-		scsi_unregister_host(&(next->htmplt));
+		scsi_unregister_host(&usb_stor_host_template);
 	}
 
 	/* While there are still structures, free them.  Note that we are
@@ -1137,11 +1089,12 @@
 		}
 
 		/* free the structure itself */
-                kfree (us_list);
+		kfree (us_list);
 
 		/* advance the list pointer */
 		us_list = next;
 	}
+#endif
 }
 
 module_init(usb_stor_init);
diff -Nru a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h
--- a/drivers/usb/storage/usb.h	Sun Jan 19 14:10:31 2003
+++ b/drivers/usb/storage/usb.h	Sun Jan 19 14:10:31 2003
@@ -52,33 +52,6 @@
 #include "scsi.h"
 #include "hosts.h"
 
-/* 
- * GUID definitions
- */
-
-#define GUID(x) __u32 x[3]
-#define GUID_EQUAL(x, y) (x[0] == y[0] && x[1] == y[1] && x[2] == y[2])
-#define GUID_CLEAR(x) x[0] = x[1] = x[2] = 0;
-#define GUID_NONE(x) (!x[0] && !x[1] && !x[2])
-#define GUID_FORMAT "%08x%08x%08x"
-#define GUID_ARGS(x) x[0], x[1], x[2]
-
-static inline void make_guid( __u32 *pg, __u16 vendor, __u16 product, char *serial)
-{
-	pg[0] = (vendor << 16) | product;
-	pg[1] = pg[2] = 0;
-	while (*serial) {
-		pg[1] <<= 4;
-		pg[1] |= pg[2] >> 28;
-		pg[2] <<= 4;
-		if (*serial >= 'a')
-			*serial -= 'a' - 'A';
-		pg[2] |= (*serial <= '9' && *serial >= '0') ? *serial - '0'
-			: *serial - 'A' + 10;
-		serial++;
-	}
-}
-
 struct us_data;
 
 /*
@@ -124,8 +97,6 @@
 
 /* we allocate one of these for every device that we remember */
 struct us_data {
-	struct us_data		*next;		 /* next device */
-
 	/* The device we're working with
 	 * It's important to note:
 	 *    (o) you must hold dev_semaphore to change pusb_dev
@@ -163,11 +134,7 @@
 	proto_cmnd		proto_handler;	 /* protocol handler	   */
 
 	/* SCSI interfaces */
-	GUID(guid);				 /* unique dev id	*/
 	struct Scsi_Host	*host;		 /* our dummy host data */
-	Scsi_Host_Template	htmplt;		 /* own host template	*/
-	int			host_number;	 /* to find us		*/
-	int			host_no;	 /* allocated by scsi	*/
 	Scsi_Cmnd		*srb;		 /* current srb		*/
 
 	/* thread information */
@@ -191,10 +158,6 @@
 	void			*extra;		 /* Any extra data          */
 	extra_data_destructor	extra_destructor;/* extra data destructor   */
 };
-
-/* The list of structures and the protective lock for them */
-extern struct us_data *us_list;
-extern struct semaphore us_list_semaphore;
 
 /* The structure which defines our driver */
 extern struct usb_driver usb_storage_driver;
-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

S:  Another stupid question?
G:  There's no such thing as a stupid question, only stupid people.
					-- Stef and Greg
User Friendly, 7/15/1998

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PATCH: usb-storage: move to SCSI hotplugging
  2003-01-19 22:30 PATCH: usb-storage: move to SCSI hotplugging Matthew Dharm
@ 2003-01-19 22:56 ` Oliver Neukum
  2003-01-20  1:11   ` Matthew Dharm
  2003-01-20  1:16 ` PATCH: usb-storage: fix typo Matthew Dharm
  1 sibling, 1 reply; 9+ messages in thread
From: Oliver Neukum @ 2003-01-19 22:56 UTC (permalink / raw)
  To: Matthew Dharm, Greg KH, USB Developers, USB Storage List
  Cc: Mike Anderson, Linux SCSI list

Am Sonntag, 19. Januar 2003 23:30 schrieb Matthew Dharm:
> The attached patch is my first implementation of SCSI hotplugging.
>
> It's only been tested that it compiles, as I can't get the current
> linux-2.5 tree from linuxusb to boot.  It dies _very_ early.  Greg, I'm not

Just a black screen?
I suffered from the same problem until today. The key was to compile
without VGA mode selection.

> sure if you'll want to apply this.  Linus seemed to want this very much,
> and it is 2.5.x... I say go for it, but I can understand if you have
> reservations.
>
> I would definately like to see this tested by anyone who can get a kernel
> to boot.

Sorry, no storage device :-(

As for the patch itself, it's quite hard to grasp something of this size.
Is the information previously used to make GUID available to other parts
of the kernel? Otherwise I think you should not cut it out, but only remove
the reattachment part.

	Regards
		Oliver


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PATCH: usb-storage: move to SCSI hotplugging
  2003-01-19 22:56 ` Oliver Neukum
@ 2003-01-20  1:11   ` Matthew Dharm
  0 siblings, 0 replies; 9+ messages in thread
From: Matthew Dharm @ 2003-01-20  1:11 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: Greg KH, USB Developers, USB Storage List, Mike Anderson,
	Linux SCSI list

[-- Attachment #1: Type: text/plain, Size: 1259 bytes --]

On Sun, Jan 19, 2003 at 11:56:01PM +0100, Oliver Neukum wrote:
> Am Sonntag, 19. Januar 2003 23:30 schrieb Matthew Dharm:
> > The attached patch is my first implementation of SCSI hotplugging.
> >
> > It's only been tested that it compiles, as I can't get the current
> > linux-2.5 tree from linuxusb to boot.  It dies _very_ early.  Greg, I'm not
> 
> Just a black screen?
> I suffered from the same problem until today. The key was to compile
> without VGA mode selection.

Nope.  It loads, uncompresses, and stops a few lines after reading the
memory map.

> As for the patch itself, it's quite hard to grasp something of this size.
> Is the information previously used to make GUID available to other parts
> of the kernel? Otherwise I think you should not cut it out, but only remove
> the reattachment part.

GUID is a combination of VID, PID, and serial number.  Anyone can make one.
The only reason to have it was for re-attachment.

Matt

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

We've made a business out of making money from annoying and stupid people.
I think you fall under that category.
					-- Chief
User Friendly, 7/11/1998

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* PATCH: usb-storage: fix typo
  2003-01-19 22:30 PATCH: usb-storage: move to SCSI hotplugging Matthew Dharm
  2003-01-19 22:56 ` Oliver Neukum
@ 2003-01-20  1:16 ` Matthew Dharm
  2003-01-23  4:43   ` Greg KH
  1 sibling, 1 reply; 9+ messages in thread
From: Matthew Dharm @ 2003-01-20  1:16 UTC (permalink / raw)
  To: Greg KH, USB Developers, USB Storage List, Mike Anderson,
	Linux SCSI list

[-- Attachment #1: Type: text/plain, Size: 1407 bytes --]

This patch goes on top of the last one.  It fixes a typo in the test for
scsi_register() failure.

Please apply.

Matt

# This is a BitKeeper generated patch for the following project:
# Project Name: greg k-h's linux 2.5 USB kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.667   -> 1.668  
#	drivers/usb/storage/usb.c	1.68    -> 1.69   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/01/19	mdharm@zen.san.one-eyed-alien.net	1.668
# Fix a typo -- reversed the logic of failure test for scsi_register()
# --------------------------------------------
#
diff -Nru a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
--- a/drivers/usb/storage/usb.c	Sun Jan 19 17:15:18 2003
+++ b/drivers/usb/storage/usb.c	Sun Jan 19 17:15:18 2003
@@ -912,7 +912,7 @@
 
 	/* now register	*/
 	ss->host = scsi_register(&usb_stor_host_template, sizeof(ss));
-	if (ss->host) {
+	if (!ss->host) {
 		printk(KERN_WARNING USB_STORAGE
 			"Unable to register the scsi host\n");
 
-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

I don't have a left mouse button.  I only have one mouse and it's on my right.
					-- Customer
User Friendly, 2/13/1999

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PATCH: usb-storage: fix typo
  2003-01-20  1:16 ` PATCH: usb-storage: fix typo Matthew Dharm
@ 2003-01-23  4:43   ` Greg KH
  2003-01-23 19:40     ` Matthew Dharm
  2003-01-25 18:11     ` [linux-usb-devel] " Matthew Dharm
  0 siblings, 2 replies; 9+ messages in thread
From: Greg KH @ 2003-01-23  4:43 UTC (permalink / raw)
  To: USB Developers, USB Storage List, Mike Anderson, Linux SCSI list

On Sun, Jan 19, 2003 at 05:16:02PM -0800, Matthew Dharm wrote:
> This patch goes on top of the last one.  It fixes a typo in the test for
> scsi_register() failure.

With these two patches applied, when plugging in a usb-storage device I
get the following oops.  So I've backed the changes out :)

Any thoughts of splitting them up into smaller pieces to try to figure
out where the problem would be?

thanks,

greg k-h


drivers/usb/host/uhci-hcd.c: 1c20: wakeup_hc
hub 1-0:0: port 1, status 101, change 1, 12 Mb/s
hub 1-0:0: debounce: port 1: delay 100ms stable 4 status 0x101
hub 1-0:0: new USB device on port 1, assigned address 2
usb 1-1: new device strings: Mfr=73, Product=90, SerialNumber=110
usb 1-1: Product: USB Storage Adapter
usb 1-1: Manufacturer: In-System Design
usb 1-1: SerialNumber: 1300E00000DEEF2E
DEV: registering device: ID = '1-1', name = USB Storage Adapter (In-System Design)
kobject 1-1: registering. parent: usb1, set: devices
bus usb: add device 1-1
bound device '1-1' to driver 'usb'
dev_hotplug
do_hotplug
fill_devpath: path = 'devices/pci0/00:07.2/usb1/1-1'
drivers/usb/core/usb.c: usb_hotplug
do_hotplug: /sbin/hotplug usb HOME=/ PATH=/sbin:/bin:/usr/sbin:/usr/bin ACTION=add DEVPATH=devices/pci0/00:07.2/usb1/1-1
usb 1-1: usb_new_device - registering interface 1-1:0
DEV: registering device: ID = '1-1:0', name = usb-00:07.2-1 interface 0
kobject 1-1:0: registering. parent: 1-1, set: devices
bus usb: add device 1-1:0
usb-storage 1-1:0: usb_device_probe
usb-storage 1-1:0: usb_device_probe - got id
scsi0 : SCSI emulation for USB Mass Storage devices
Unable to handle kernel NULL pointer dereference at virtual address 000000b8
 printing eip:
c88b1023
*pde = 00000000
Oops: 0000
CPU:    0
EIP:    0060:[<c88b1023>]    Not tainted
EFLAGS: 00010097
EIP is at usb_storage_queuecommand+0x13/0x3774eff0 [usb_storage]
eax: c7f6f440   ebx: 00000297   ecx: c569ae00   edx: 00000000
esi: c569ae00   edi: c7f6f440   ebp: 00000000   esp: c55cbbf4
ds: 007b   es: 007b   ss: 0068
Process khubd (pid: 990, threadinfo=c55ca000 task=c6a80120)
Stack: c88995e7 c569ae00 c8899ab0 c569a88c c7f6f440 c569aa00 c55cbc2c c889e640 
       c569ae00 c569ae00 c569ae00 c569a88c c569ac00 00000202 c569a800 c01b0147 
       c569ac00 c569ac00 c569a88c 00000001 00000000 c55cbc98 00001770 00000024 
Call Trace:
 [<c88995e7>] scsi_dispatch_cmd+0xf7/0x37766b10 [scsi_mod]
 [<c8899ab0>] scsi_done+0x0/0x37766550 [scsi_mod]
 [<c889e640>] scsi_request_fn+0x1c0/0x37761b80 [scsi_mod]
 [<c01b0147>] blk_insert_request+0x47/0x50
 [<c889d8f4>] scsi_insert_special_req+0x24/0x37762730 [scsi_mod]
 [<c88996d0>] scsi_wait_req+0x70/0x377669a0 [scsi_mod]
 [<c8899000>] +0x0/0x37767000 [scsi_mod]
 [<c889f36f>] scsi_probe_lun+0x4f/0x37760ce0 [scsi_mod]
 [<c012f0f7>] kmalloc+0x77/0xa0
 [<c889f7c4>] scsi_probe_and_add_lun+0x94/0x377608d0 [scsi_mod]
 [<c889fab8>] scsi_scan_target+0x38/0x37760580 [scsi_mod]
 [<c889fbac>] scsi_scan_host+0x4c/0x377604a0 [scsi_mod]
 [<c889ac09>] __scsi_add_host+0x39/0x37765430 [scsi_mod]
 [<c88a11e0>] +0x340/0x3775f160 [scsi_mod]
 [<c88bbe20>] +0x40/0x37744220 [usb_storage]
 [<c88b3d9e>] storage_probe+0x79e/0x3774ca00 [usb_storage]
 [<c012f045>] kmem_cache_alloc+0x25/0x60
 [<c88bf75c>] us_unusual_dev_list+0x7a8/0x3774104c [usb_storage]
 [<c88bf828>] usb_storage_driver+0x88/0x37740860 [usb_storage]
 [<c88bf7a0>] usb_storage_driver+0x0/0x37740860 [usb_storage]
 [<c88bf7a0>] usb_storage_driver+0x0/0x37740860 [usb_storage]
 [<c886212a>] usb_device_probe+0x10a/0x3779dfe0 [usbcore]
 [<c88bef88>] storage_usb_ids+0x7a8/0x37741820 [usb_storage]
 [<c88bef88>] storage_usb_ids+0x7a8/0x37741820 [usb_storage]
 [<c88bf7b8>] usb_storage_driver+0x18/0x37740860 [usb_storage]
 [<c8878900>] usb_bus_type+0x0/0x37787700 [usbcore]
 [<c0199564>] bus_match+0x34/0x60
 [<c88bf7ec>] usb_storage_driver+0x4c/0x37740860 [usb_storage]
 [<c8878958>] usb_bus_type+0x58/0x37787700 [usbcore]
 [<c01995e0>] device_attach+0x50/0x60
 [<c88bf7b8>] usb_storage_driver+0x18/0x37740860 [usb_storage]
 [<c8878940>] usb_bus_type+0x40/0x37787700 [usbcore]
 [<c0199762>] bus_add_device+0x82/0xc0
 [<c0198b28>] device_add+0xe8/0x130
 [<c886e2d4>] +0xa8/0x37791dd4 [usbcore]
 [<c8863441>] usb_new_device+0x4f1/0x3779d0b0 [usbcore]
 [<c886fa20>] +0x6a0/0x37790c80 [usbcore]
 [<c886e2d4>] +0xa8/0x37791dd4 [usbcore]
 [<c886e415>] +0x1e9/0x37791dd4 [usbcore]
 [<c0115f35>] __wake_up+0x15/0x30
 [<c88653bd>] usb_hub_port_connect_change+0x27d/0x3779aec0 [usbcore]
 [<c8870b00>] +0x1780/0x37790c80 [usbcore]
 [<c886e9cd>] +0x7a1/0x37791dd4 [usbcore]
 [<c886557d>] usb_hub_events+0x12d/0x3779abb0 [usbcore]
 [<c88657e5>] usb_hub_thread+0x45/0x3779a860 [usbcore]
 [<c0115e80>] default_wake_function+0x0/0x40
 [<c8878a0c>] khubd_wait+0x4/0x377875f8 [usbcore]
 [<c8878a0c>] khubd_wait+0x4/0x377875f8 [usbcore]
 [<c88657a0>] usb_hub_thread+0x0/0x3779a860 [usbcore]
 [<c0106f31>] kernel_thread_helper+0x5/0x14

Code: 8b 82 b8 00 00 00 48 74 08 0f 0b 95 00 60 be 8b c8 8b 82 b0 
 <1>Unable to handle kernel NULL pointer dereference at virtual address 000000b0
 printing eip:
c88b107d
*pde = 00000000
Oops: 0000
CPU:    0
EIP:    0060:[<c88b107d>]    Not tainted
EFLAGS: 00010002
EIP is at usb_storage_command_abort+0xd/0x3774ef90 [usb_storage]
eax: 00000000   ebx: 00000202   ecx: 00002003   edx: c569ae00
esi: c7f6f440   edi: c7f6f440   ebp: 00000000   esp: c5c95f6c
ds: 007b   es: 007b   ss: 0068
Process scsi_eh_0 (pid: 1038, threadinfo=c5c94000 task=c54d3240)
Stack: c889cd13 c569ae00 c569ae00 c889cdec c569ae00 c7f6f440 c88a325b c889d4a7 
       c569ae00 c7f6f440 c569ae00 c5c95fc4 c889d5eb c7f6f440 00000000 00000000 
       00000000 c5c95fd0 c5c95fd0 00000000 c55cbd14 c0108ae5 00000000 00000000 
Call Trace:
 [<c889cd13>] scsi_try_to_abort_cmd+0x43/0x37763330 [scsi_mod]
 [<c889cdec>] scsi_eh_abort_cmd+0x1c/0x37763230 [scsi_mod]
 [<c88a325b>] +0x23bb/0x3775f160 [scsi_mod]
 [<c889d4a7>] scsi_unjam_host+0x37/0x37762b90 [scsi_mod]
 [<c889d5eb>] scsi_error_handler+0xbb/0x37762ad0 [scsi_mod]
 [<c0108ae5>] ret_from_fork+0x5/0x14
 [<c889d530>] scsi_error_handler+0x0/0x37762ad0 [scsi_mod]
 [<c0106f31>] kernel_thread_helper+0x5/0x14

Code: 39 90 b0 00 00 00 74 0b b8 03 20 00 00 c3 90 8d 74 26 00 50 
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PATCH: usb-storage: fix typo
  2003-01-23  4:43   ` Greg KH
@ 2003-01-23 19:40     ` Matthew Dharm
  2003-01-24  8:26       ` Greg KH
  2003-01-25 18:11     ` [linux-usb-devel] " Matthew Dharm
  1 sibling, 1 reply; 9+ messages in thread
From: Matthew Dharm @ 2003-01-23 19:40 UTC (permalink / raw)
  To: Greg KH; +Cc: USB Developers, USB Storage List, Mike Anderson, Linux SCSI list

[-- Attachment #1: Type: text/plain, Size: 7751 bytes --]

Well, it's stored in my local tree as a large series of deltas.  But I'd
settle for just being able to boot the kernel with debugging enabled and
try it myself.

The problem is, it doesn't make any sense if split up into smaller pieces.
The change from old hotplug system to new hotplug system really is one
package.  I mean, I could send you the delta where I convert the probe
mechanism to use the new system, but then it's just going to blow up when a
device is detached.

I'll keep working on getting a booting configuration.  Some people have
suggested turning off framebuffer support, but that's already off....

Matt

On Wed, Jan 22, 2003 at 08:43:06PM -0800, Greg KH wrote:
> On Sun, Jan 19, 2003 at 05:16:02PM -0800, Matthew Dharm wrote:
> > This patch goes on top of the last one.  It fixes a typo in the test for
> > scsi_register() failure.
> 
> With these two patches applied, when plugging in a usb-storage device I
> get the following oops.  So I've backed the changes out :)
> 
> Any thoughts of splitting them up into smaller pieces to try to figure
> out where the problem would be?
> 
> thanks,
> 
> greg k-h
> 
> 
> drivers/usb/host/uhci-hcd.c: 1c20: wakeup_hc
> hub 1-0:0: port 1, status 101, change 1, 12 Mb/s
> hub 1-0:0: debounce: port 1: delay 100ms stable 4 status 0x101
> hub 1-0:0: new USB device on port 1, assigned address 2
> usb 1-1: new device strings: Mfr=73, Product=90, SerialNumber=110
> usb 1-1: Product: USB Storage Adapter
> usb 1-1: Manufacturer: In-System Design
> usb 1-1: SerialNumber: 1300E00000DEEF2E
> DEV: registering device: ID = '1-1', name = USB Storage Adapter (In-System Design)
> kobject 1-1: registering. parent: usb1, set: devices
> bus usb: add device 1-1
> bound device '1-1' to driver 'usb'
> dev_hotplug
> do_hotplug
> fill_devpath: path = 'devices/pci0/00:07.2/usb1/1-1'
> drivers/usb/core/usb.c: usb_hotplug
> do_hotplug: /sbin/hotplug usb HOME=/ PATH=/sbin:/bin:/usr/sbin:/usr/bin ACTION=add DEVPATH=devices/pci0/00:07.2/usb1/1-1
> usb 1-1: usb_new_device - registering interface 1-1:0
> DEV: registering device: ID = '1-1:0', name = usb-00:07.2-1 interface 0
> kobject 1-1:0: registering. parent: 1-1, set: devices
> bus usb: add device 1-1:0
> usb-storage 1-1:0: usb_device_probe
> usb-storage 1-1:0: usb_device_probe - got id
> scsi0 : SCSI emulation for USB Mass Storage devices
> Unable to handle kernel NULL pointer dereference at virtual address 000000b8
>  printing eip:
> c88b1023
> *pde = 00000000
> Oops: 0000
> CPU:    0
> EIP:    0060:[<c88b1023>]    Not tainted
> EFLAGS: 00010097
> EIP is at usb_storage_queuecommand+0x13/0x3774eff0 [usb_storage]
> eax: c7f6f440   ebx: 00000297   ecx: c569ae00   edx: 00000000
> esi: c569ae00   edi: c7f6f440   ebp: 00000000   esp: c55cbbf4
> ds: 007b   es: 007b   ss: 0068
> Process khubd (pid: 990, threadinfo=c55ca000 task=c6a80120)
> Stack: c88995e7 c569ae00 c8899ab0 c569a88c c7f6f440 c569aa00 c55cbc2c c889e640 
>        c569ae00 c569ae00 c569ae00 c569a88c c569ac00 00000202 c569a800 c01b0147 
>        c569ac00 c569ac00 c569a88c 00000001 00000000 c55cbc98 00001770 00000024 
> Call Trace:
>  [<c88995e7>] scsi_dispatch_cmd+0xf7/0x37766b10 [scsi_mod]
>  [<c8899ab0>] scsi_done+0x0/0x37766550 [scsi_mod]
>  [<c889e640>] scsi_request_fn+0x1c0/0x37761b80 [scsi_mod]
>  [<c01b0147>] blk_insert_request+0x47/0x50
>  [<c889d8f4>] scsi_insert_special_req+0x24/0x37762730 [scsi_mod]
>  [<c88996d0>] scsi_wait_req+0x70/0x377669a0 [scsi_mod]
>  [<c8899000>] +0x0/0x37767000 [scsi_mod]
>  [<c889f36f>] scsi_probe_lun+0x4f/0x37760ce0 [scsi_mod]
>  [<c012f0f7>] kmalloc+0x77/0xa0
>  [<c889f7c4>] scsi_probe_and_add_lun+0x94/0x377608d0 [scsi_mod]
>  [<c889fab8>] scsi_scan_target+0x38/0x37760580 [scsi_mod]
>  [<c889fbac>] scsi_scan_host+0x4c/0x377604a0 [scsi_mod]
>  [<c889ac09>] __scsi_add_host+0x39/0x37765430 [scsi_mod]
>  [<c88a11e0>] +0x340/0x3775f160 [scsi_mod]
>  [<c88bbe20>] +0x40/0x37744220 [usb_storage]
>  [<c88b3d9e>] storage_probe+0x79e/0x3774ca00 [usb_storage]
>  [<c012f045>] kmem_cache_alloc+0x25/0x60
>  [<c88bf75c>] us_unusual_dev_list+0x7a8/0x3774104c [usb_storage]
>  [<c88bf828>] usb_storage_driver+0x88/0x37740860 [usb_storage]
>  [<c88bf7a0>] usb_storage_driver+0x0/0x37740860 [usb_storage]
>  [<c88bf7a0>] usb_storage_driver+0x0/0x37740860 [usb_storage]
>  [<c886212a>] usb_device_probe+0x10a/0x3779dfe0 [usbcore]
>  [<c88bef88>] storage_usb_ids+0x7a8/0x37741820 [usb_storage]
>  [<c88bef88>] storage_usb_ids+0x7a8/0x37741820 [usb_storage]
>  [<c88bf7b8>] usb_storage_driver+0x18/0x37740860 [usb_storage]
>  [<c8878900>] usb_bus_type+0x0/0x37787700 [usbcore]
>  [<c0199564>] bus_match+0x34/0x60
>  [<c88bf7ec>] usb_storage_driver+0x4c/0x37740860 [usb_storage]
>  [<c8878958>] usb_bus_type+0x58/0x37787700 [usbcore]
>  [<c01995e0>] device_attach+0x50/0x60
>  [<c88bf7b8>] usb_storage_driver+0x18/0x37740860 [usb_storage]
>  [<c8878940>] usb_bus_type+0x40/0x37787700 [usbcore]
>  [<c0199762>] bus_add_device+0x82/0xc0
>  [<c0198b28>] device_add+0xe8/0x130
>  [<c886e2d4>] +0xa8/0x37791dd4 [usbcore]
>  [<c8863441>] usb_new_device+0x4f1/0x3779d0b0 [usbcore]
>  [<c886fa20>] +0x6a0/0x37790c80 [usbcore]
>  [<c886e2d4>] +0xa8/0x37791dd4 [usbcore]
>  [<c886e415>] +0x1e9/0x37791dd4 [usbcore]
>  [<c0115f35>] __wake_up+0x15/0x30
>  [<c88653bd>] usb_hub_port_connect_change+0x27d/0x3779aec0 [usbcore]
>  [<c8870b00>] +0x1780/0x37790c80 [usbcore]
>  [<c886e9cd>] +0x7a1/0x37791dd4 [usbcore]
>  [<c886557d>] usb_hub_events+0x12d/0x3779abb0 [usbcore]
>  [<c88657e5>] usb_hub_thread+0x45/0x3779a860 [usbcore]
>  [<c0115e80>] default_wake_function+0x0/0x40
>  [<c8878a0c>] khubd_wait+0x4/0x377875f8 [usbcore]
>  [<c8878a0c>] khubd_wait+0x4/0x377875f8 [usbcore]
>  [<c88657a0>] usb_hub_thread+0x0/0x3779a860 [usbcore]
>  [<c0106f31>] kernel_thread_helper+0x5/0x14
> 
> Code: 8b 82 b8 00 00 00 48 74 08 0f 0b 95 00 60 be 8b c8 8b 82 b0 
>  <1>Unable to handle kernel NULL pointer dereference at virtual address 000000b0
>  printing eip:
> c88b107d
> *pde = 00000000
> Oops: 0000
> CPU:    0
> EIP:    0060:[<c88b107d>]    Not tainted
> EFLAGS: 00010002
> EIP is at usb_storage_command_abort+0xd/0x3774ef90 [usb_storage]
> eax: 00000000   ebx: 00000202   ecx: 00002003   edx: c569ae00
> esi: c7f6f440   edi: c7f6f440   ebp: 00000000   esp: c5c95f6c
> ds: 007b   es: 007b   ss: 0068
> Process scsi_eh_0 (pid: 1038, threadinfo=c5c94000 task=c54d3240)
> Stack: c889cd13 c569ae00 c569ae00 c889cdec c569ae00 c7f6f440 c88a325b c889d4a7 
>        c569ae00 c7f6f440 c569ae00 c5c95fc4 c889d5eb c7f6f440 00000000 00000000 
>        00000000 c5c95fd0 c5c95fd0 00000000 c55cbd14 c0108ae5 00000000 00000000 
> Call Trace:
>  [<c889cd13>] scsi_try_to_abort_cmd+0x43/0x37763330 [scsi_mod]
>  [<c889cdec>] scsi_eh_abort_cmd+0x1c/0x37763230 [scsi_mod]
>  [<c88a325b>] +0x23bb/0x3775f160 [scsi_mod]
>  [<c889d4a7>] scsi_unjam_host+0x37/0x37762b90 [scsi_mod]
>  [<c889d5eb>] scsi_error_handler+0xbb/0x37762ad0 [scsi_mod]
>  [<c0108ae5>] ret_from_fork+0x5/0x14
>  [<c889d530>] scsi_error_handler+0x0/0x37762ad0 [scsi_mod]
>  [<c0106f31>] kernel_thread_helper+0x5/0x14
> 
> Code: 39 90 b0 00 00 00 74 0b b8 03 20 00 00 c3 90 8d 74 26 00 50 
>  
> -
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

You are needink to look more evil.  You likink very strong coffee?
					-- Pitr to Dust Puppy
User Friendly, 10/16/1998

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PATCH: usb-storage: fix typo
  2003-01-23 19:40     ` Matthew Dharm
@ 2003-01-24  8:26       ` Greg KH
  0 siblings, 0 replies; 9+ messages in thread
From: Greg KH @ 2003-01-24  8:26 UTC (permalink / raw)
  To: USB Developers, USB Storage List, Mike Anderson, Linux SCSI list

On Thu, Jan 23, 2003 at 11:40:40AM -0800, Matthew Dharm wrote:
> Well, it's stored in my local tree as a large series of deltas.  But I'd
> settle for just being able to boot the kernel with debugging enabled and
> try it myself.
> 
> The problem is, it doesn't make any sense if split up into smaller pieces.
> The change from old hotplug system to new hotplug system really is one
> package.  I mean, I could send you the delta where I convert the probe
> mechanism to use the new system, but then it's just going to blow up when a
> device is detached.

But your patch did much more than just change systems, as you noted
(typedef changes, etc.).  Splitting it up into smaller pieces would help
in trying to figure out where the problem in the patch might be.

As it is, I can't do that :(

> I'll keep working on getting a booting configuration.  Some people have
> suggested turning off framebuffer support, but that's already off....

Hm, don't know what to say, I'm typing from 2.5.59 right now...

Good luck,

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [linux-usb-devel] Re: PATCH: usb-storage: fix typo
  2003-01-23  4:43   ` Greg KH
  2003-01-23 19:40     ` Matthew Dharm
@ 2003-01-25 18:11     ` Matthew Dharm
  2003-01-25 20:32       ` Matthew Dharm
  1 sibling, 1 reply; 9+ messages in thread
From: Matthew Dharm @ 2003-01-25 18:11 UTC (permalink / raw)
  To: Greg KH; +Cc: USB Developers, USB Storage List, Mike Anderson, Linux SCSI list

[-- Attachment #1: Type: text/plain, Size: 2397 bytes --]

On Wed, Jan 22, 2003 at 08:43:06PM -0800, Greg KH wrote:
> On Sun, Jan 19, 2003 at 05:16:02PM -0800, Matthew Dharm wrote:
> > This patch goes on top of the last one.  It fixes a typo in the test for
> > scsi_register() failure.
> 
> With these two patches applied, when plugging in a usb-storage device I
> get the following oops.  So I've backed the changes out :)

Try this patch on top of the other two.  It should fix the OOPS on attach.

This fixes a silly error where I fail to initialize a pointer early enough
for the scanning code.  If this isn't a perfect example of why
scsi_register() and scsi_add_host() aren't two separate functions, I don't
know what is.  :)

Oh, and I added a couple of comments, too.

Testing and comments are welcome, as always.

Matt

# This is a BitKeeper generated patch for the following project:
# Project Name: greg k-h's linux 2.5 USB kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.668   -> 1.669  
#	drivers/usb/storage/usb.c	1.69    -> 1.70   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/01/25	mdharm@zen.san.one-eyed-alien.net	1.669
# Fix an OOPS by moving the setting of the hostdata[] pointer to _before_
# the device scan starts.
# --------------------------------------------
#
diff -Nru a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
--- a/drivers/usb/storage/usb.c	Sat Jan 25 10:07:43 2003
+++ b/drivers/usb/storage/usb.c	Sat Jan 25 10:07:43 2003
@@ -926,6 +926,12 @@
 		goto BadDevice;
 	}
 
+	/* set the hostdata to prepare for scanning */
+	ss->host->hostdata[0] = (unsigned long)ss;
+
+	/* associate this host with our interface */
+	scsi_set_device(ss->host, &intf->dev);
+
 	/* now add the host */
 	result = scsi_add_host(ss->host, NULL);
 	if (result) {
@@ -941,9 +947,6 @@
 		down(&ss->dev_semaphore);
 		goto BadDevice;
 	}
-
-	ss->host->hostdata[0] = (unsigned long)ss;
-	scsi_set_device(ss->host, &intf->dev);
 
 	printk(KERN_DEBUG 
 	       "WARNING: USB Mass Storage data integrity not assured\n");
-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

I need a computer?
					-- Customer
User Friendly, 2/19/1998

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [linux-usb-devel] Re: PATCH: usb-storage: fix typo
  2003-01-25 18:11     ` [linux-usb-devel] " Matthew Dharm
@ 2003-01-25 20:32       ` Matthew Dharm
  0 siblings, 0 replies; 9+ messages in thread
From: Matthew Dharm @ 2003-01-25 20:32 UTC (permalink / raw)
  To: Greg KH, USB Developers, USB Storage List, Mike Anderson,
	Linux SCSI list

[-- Attachment #1: Type: text/plain, Size: 3973 bytes --]

I finally got 2.5.59 to run, tho without modules.  I can confirm that the
three patches, taken together, produce a working hotplug system.

So far, it's not 100%.  But I think these patches should go in, because
there is still some debate going on as to how to make hot-unplug work best.
That's the last problem area.

I'm able to attach devices (which then get SCSI /dev/s), use them, then
hot-unplug the device (only tested for devices in quiet state).  Attaching
a device (new or the old one) gets a new set of /dev entries.

In addition, the sysfs entries seem to all be correct.  The lsscsi utility
likes it all enough, too.

It's a bit odd to see one device as /dev/sda one moment, then another as
/dev/sda the next (in an unplug-then-replace scenario), but that's for the
high-level drivers (sd, sg, etc.) to sort out.

You can crash this if you try.  The methods seem to be:
(1) Lots of plugging, as the unplug cleanup code isn't 100% yet
(2) Unplug a device that's still in use.

Both of these issues will be addressed once the unplugging technique gets
worked out (see the copious e-mails on the topic on l-u-d and linux-scsi).

Matt

On Sat, Jan 25, 2003 at 10:11:41AM -0800, Matthew Dharm wrote:
> On Wed, Jan 22, 2003 at 08:43:06PM -0800, Greg KH wrote:
> > On Sun, Jan 19, 2003 at 05:16:02PM -0800, Matthew Dharm wrote:
> > > This patch goes on top of the last one.  It fixes a typo in the test for
> > > scsi_register() failure.
> > 
> > With these two patches applied, when plugging in a usb-storage device I
> > get the following oops.  So I've backed the changes out :)
> 
> Try this patch on top of the other two.  It should fix the OOPS on attach.
> 
> This fixes a silly error where I fail to initialize a pointer early enough
> for the scanning code.  If this isn't a perfect example of why
> scsi_register() and scsi_add_host() aren't two separate functions, I don't
> know what is.  :)
> 
> Oh, and I added a couple of comments, too.
> 
> Testing and comments are welcome, as always.
> 
> Matt
> 
> # This is a BitKeeper generated patch for the following project:
> # Project Name: greg k-h's linux 2.5 USB kernel tree
> # This patch format is intended for GNU patch command version 2.5 or higher.
> # This patch includes the following deltas:
> #	           ChangeSet	1.668   -> 1.669  
> #	drivers/usb/storage/usb.c	1.69    -> 1.70   
> #
> # The following is the BitKeeper ChangeSet Log
> # --------------------------------------------
> # 03/01/25	mdharm@zen.san.one-eyed-alien.net	1.669
> # Fix an OOPS by moving the setting of the hostdata[] pointer to _before_
> # the device scan starts.
> # --------------------------------------------
> #
> diff -Nru a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
> --- a/drivers/usb/storage/usb.c	Sat Jan 25 10:07:43 2003
> +++ b/drivers/usb/storage/usb.c	Sat Jan 25 10:07:43 2003
> @@ -926,6 +926,12 @@
>  		goto BadDevice;
>  	}
>  
> +	/* set the hostdata to prepare for scanning */
> +	ss->host->hostdata[0] = (unsigned long)ss;
> +
> +	/* associate this host with our interface */
> +	scsi_set_device(ss->host, &intf->dev);
> +
>  	/* now add the host */
>  	result = scsi_add_host(ss->host, NULL);
>  	if (result) {
> @@ -941,9 +947,6 @@
>  		down(&ss->dev_semaphore);
>  		goto BadDevice;
>  	}
> -
> -	ss->host->hostdata[0] = (unsigned long)ss;
> -	scsi_set_device(ss->host, &intf->dev);
>  
>  	printk(KERN_DEBUG 
>  	       "WARNING: USB Mass Storage data integrity not assured\n");
> -- 
> Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
> Maintainer, Linux USB Mass Storage Driver
> 
> I need a computer?
> 					-- Customer
> User Friendly, 2/19/1998



-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

It was a new hope.
					-- Dust Puppy
User Friendly, 12/25/1998

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2003-01-25 20:32 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-01-19 22:30 PATCH: usb-storage: move to SCSI hotplugging Matthew Dharm
2003-01-19 22:56 ` Oliver Neukum
2003-01-20  1:11   ` Matthew Dharm
2003-01-20  1:16 ` PATCH: usb-storage: fix typo Matthew Dharm
2003-01-23  4:43   ` Greg KH
2003-01-23 19:40     ` Matthew Dharm
2003-01-24  8:26       ` Greg KH
2003-01-25 18:11     ` [linux-usb-devel] " Matthew Dharm
2003-01-25 20:32       ` Matthew Dharm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox