Re: libata & scsi rescan.

[Andy Warner <andyw@pobox.com>]

Jeff Garzik wrote:
> Andy Warner wrote:
> > 		sdev = scsi_add_device(ap->host, 0, 0, 0) ;
> 
> > 		sdev = scsi_device_lookup(ap->host, 0, 0, 0);
> 
> These args are wrong.  Consider master/slave configurations.

Like I said: "... (ignore hardcoded values and sloppy error
checking for now)"

> Also, remember my short message about a debounce timer.  It's not as 
> simple as this... you need to make sure the driver doesn't "scream" 
> add_device/remove_device while the SATA bus settles.

I didn't show that logic, but I think the code in the interrupt
handler deals with that:

if ((err & SCR_DIAG_N) && (ap->hotplug_task_state == HOTPLUG_ST_IDLE)) {
	/*
	 * Schedule hotplug task to deal with it.
	 */
	ap->hotplug_task_state = HOTPLUG_ST_PENDING;
	queue_delayed_work(ata_wq, &ap->hotplug_task, 20);
}

[again, ignore locking issues for now]

What this effectively does is start a one-shot on the first
interrupt due to bit 16, and ignore any subsequent transitions.
>= 20 jiffies later (TODO: figure out the optimal value) the other
hotplug_task runs, by which time the bus will have settled. It reads
the value and does what it must. I think this will accomplish
what is needed.

I too want to cache dev in *ap, but that's another bunch of issues
(like either unrolling scsi_scan_host() as called from ata_device_add(),
figuring out ap->dev after scsi_scan_host() has run, or just
letting the hotswap logic add the drives after initting the
host.) I'll get to that after I fixup the scsi_remove_device()
problems.

I think I also need logic to ensure that I don't call scsi_add_device()
twice [if (!(ap->dev)) { ...} springs to mind].

Thanks for confirming that the "sync the disk that just went
away"/scsi_remove_device() problem is structural, not just me
calling the wrong function. I'll look into implementing flags
as you suggest.
-- 
andyw@pobox.com

Andy Warner		Voice: (612) 801-8549	Fax: (208) 575-5634