Lukasz Kosewski wrote:
On 9/28/05, Jeff Garzik <jgarzik@xxxxxxxxx> wrote:
+ dev->flags &= ~ATA_DFLAG_LBA48;
+
probably should just clear out all the flags...
I'll look into making this cleaner :) During testing this was the
only one that was necessary, but I'll do some clean-up foo on this.
is ata_chk_err() call in ata_to_sense_error() the only place that needs
to talk to the hardware? If so, maybe we could work around that by
making sure it is passed register values, avoiding the need to poke the h/w.
Yep, it is the only place as far as I can see. As for passing
register values, the reason I decided not to do that is because, say,
a user unplugged his drive because there WAS some kind of error which
set ATA_ERR. This would attempt to perform some kind of error check
which would compound the problem. However, I agree this is dirty :)
Should I just check the register values, & ~ATA_ERR, and then pass that in?
+ spin_lock(&ap->host_set->lock);
+ hotplug_todo = ap->plug; // Make sure we don't modify while reading!
+ spin_unlock(&ap->host_set->lock);
this lock should always be acquired using spin_lock_irqsave()
Oops.
+ ata_scsi_handle_unplug(ap);
+
+ // The following flag is necessary on some Seagate drives.
+ ap->flags |= ATA_FLAG_SATA_RESET;
we can't unconditionally set this for all controllers
For PATA hotplug, which this code will eventually handle, the SATA
SControl register doesn't even exist :)
if (ap & ATA_FLAG_SATA) ap->flags |= ATA_FLAG_SATA_RESET
? :)
+ ap->udma_mask = ap->orig_udma_mask;
dumb question -- what is this for?
for hotplug we'll want to go through the entire probe sequence,
configuring pio/dma masks all over again.
OK I'll admit I may not have looked through the code enough on that
one, but here's a situation I remember about this:
Had a drive which supported UDMA 5 max. Unplugged it, plugged in a
drive which supported UDMA 6, but libata reported UDMA 5 max. Since
the flags aren't reset, and ata_mode_string() works from slowest to
fastest, this will always happen unless we reset the flags to some
default value. So... voila. Do you have a suggestion for making this
cleaner?
Well, overall what needs to happen for newly-plugged-in devices is the a
re-run of the probe sequence. The flags should certainly be reset to a
default value, the same value that the device would see had it been
plugged in when the driver loaded: ata_bus_probe() call sequence, after
initialization from ata_host_init(). This may require saving a few
pieces of data, as you have done, agreed.
Jeff
-
: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
