Re: [linux-usb-devel] Re: PATCH: usb-storage: fix typo

[Matthew Dharm <mdharm-scsi@one-eyed-alien.net>]

[-- 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 --]