Jeff Garzik wrote: > Tejun Heo wrote: >> zhao, forrest wrote: >>> On Tue, 2006-04-11 at 23:14 +0900, Tejun Heo wrote: >>> >>>> Warm unplug/plug comes at almost no cost for LLDDs implementing new EH >>>> - adding a call to ata_eh_hotplug() at the end of ->error_handler is >>>> enough (even this is unnecessary if it's using bmdma error_handler). >>>> e.g. ata_piix isn't changed by hotplug patchsets at all but still >>>> supports warm plugging (even PATA warm plugging works although it's >>>> electronically unsafe). >>>> >>>> To support full hot unplug/plug, all a LLDD has to do are enabling PHY >>>> status changed interrupt and, when PHY RDY status changes, call >>>> ata_schedule_probe() and schedule EH. ahci, sata_sil and sata_sil24 >>>> are converted this way and hotplugging works perfectly for those >>>> drivers. >>>> >>> In order to make ata_eh_hotplug() called, LLDD have to schedule EH >>> in its interrupt handler(i.e. invoke scsi_eh_schedule_host() directly >>> or indirectly), but I didn't find such code in your patch set. >>> Did I miss anything? >>> >> The last three patches do that. All hotplug interrupts end up invoking >> EH by calling ata_eh_schedule_port(ap, ATA_EH_FREEZE). > > It's certainly nicer and more readable, if the LLDD calls a function > like ata_eh_hotplug(). > Hi, Jeff. Currently, a LLDD has to do the following two to schedule hotplug. * call ata_schedule_probe(ap); * invoke EH by calling ata_eh_schedule_port(ap, ATA_EH_FREEZE); Calling ata_schedule_probe() can be done from interrupt handler or inside EH. It just has to be done before actual hotplug operation begins. ata_schedule_probe() can be changed to include a call to ata_eh_schedule_port() but, as can be seen from three converted drivers, it isn't really necessary. Scheduling hotplug is usually done by the standard ata_eh_autopsy() function in EH after looking at the SError register. So, actually, all an interrupt handler has to do is just to call in to EH if something looks weird. Standard EH will do all the jobs for it including scheduling hotplug operation. Only sata_sil24 calls ata_schedule_probe() explicitly because its SError is sort of flimsy. And even that is not done from the interrupt handler. It's done by sil24_autopsy() function which is the first thing sata_sil24 EH does on entry. So, there really isn't much point in making ata_schedule_port() and ata_eh_schedule_port() done in one step. For most controllers, the latter doesn't need to be called at all and even, when it does, there's a good chance that it will be done from EH making scheduling EH kind of silly, still safe though. Hmmmm... but maybe ata_schedule_hotplug() is a better name than ata_schedule_probe(). Or ata_eh_schedule_hotplug()? ata_eh_hotplug() is already taken by the function which does actual hotplug. The reason why I chose ata_schedule_probe() was the word 'hotplug' is currently used to mean both plugging and unplugging operations, and warm operations too. It means the general facility to attach and detach devices dynamically. So, calling it ata_schedule_hotplug() or something similar is sort of confusing as some hotplug operations happen whether the function is called or not. The terms I've used are... * hotplug : as said above, general facility and operations to dynamically add and remove devices, warm or hot. * attach : the action of adding a new device * detach : the action of removing a device * probe : an operation to discover new devices on a port and attach them Thanks. -- tejun - : send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
