add hotplug support
PATCH 9/9 add hotplug support
Hellooooo, all.
Finally, the last one. This is the first take of add-hotplug-support patchset. This patchset includes 13 patches.
#01-04 implement hotplug framework
#05-07 implement warm plug (SCSI scan/delete callbacks)
#08-10 hook and activate hotplug
#11-13 add LLDD hotplug supports (PHY status change notifications)
Hotplugging is implemented as an integral part of EH. A new EH helper ata_eh_hotplug() is defined which should be called after all regular EH is done. LLDDs have to do very little to support hotplug.
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.
One of the design goals was making hotplug support and the driver resillient. Hardware might get stuck but the driver should NEVER oops or malfunction no matter what the user does. As far as libata is conerned, all are allowed - yanking and plugging at any time, requesting warm plug or unplug under heavy IO or during EH. So, if you have some time, torture libata, be pervert and break it. If you succeed, please let me know. I'll try to reproduce your perverseness and fix the breakage.
Some hardware gotchas I've found...
* ICH7 in piix mode has troubles seeing new SATA devices if the port was not occupied on boot.
* sil3124 locks up really hard if it's requested to scan the bus (which triggers immediate ATA_EH_FREEZE) while commands to the port are in progress. Port reset cannot resume the port. Power cycling works. Maybe host reset needed?
This patchset is against
upstream (c2a6585296009379e0f4eff39cdcb108b457ebf2)
+ [1] misc-reset-updates patchset (repost)
+ [2] implement-and-use-ata_wait_register patchset (repost)
+ [3] misc-ata_bus_probe-updates patchset
+ [4] fixes-errata-workaround-and-reset-updates patchset, take 2
+ [5] implement-scsi_eh_schedule patchset
+ [6] fix-scsi_kill_request-busy-count-handling patch
+ [7] new-EH-framework patchset, take 2
+ [8] new-EH-implementation patchset, take 2
+ [9] add-NCQ-support patchset, take 2
+ [10] prep-for-hotplug-support patchset
Thanks.
--
tejun
[1] http://article.gmane.org/gmane.linux.ide/9495
[2] http://article.gmane.org/gmane.linux.ide/9499
[3] http://article.gmane.org/gmane.linux.ide/9506
[4] http://article.gmane.org/gmane.linux.ide/9516
[5] http://article.gmane.org/gmane.linux.ide/9290
[6] http://article.gmane.org/gmane.linux.ide/9487
[7] http://article.gmane.org/gmane.linux.ide/9524
[8] http://article.gmane.org/gmane.linux.ide/9540
[9] http://article.gmane.org/gmane.linux.ide/9555
[10] http://article.gmane.org/gmane.linux.ide/9579
PATCH 1/15 libata-hp: implement ata_eh_detach_dev()
This function is to be called from EH to disable a device and mark it for detach.
---
drivers/scsi/libata-core.c | 1 + drivers/scsi/libata-eh.c | 20 ++++++++++++++++++++ include/linux/libata.h | 1 + 3 files changed, 22 insertions(+), 0 deletions(-)
7e411eeec0a5d02daa41a48debc6a73ad970921d
drivers/scsi/libata-core.c +1 -0
@@ -5483,6 +5483,7 @@ EXPORT_SYMBOL_GPL(ata_eng_timeout);EXPORT_SYMBOL_GPL (ata_eh_schedule_port);EXPORT_SYMBOL_GPL (ata_eh_qc_complete);EXPORT_SYMBOL_GPL (ata_eh_qc_retry); +EXPORT_SYMBOL_GPL (ata_eh_detach_dev);EXPORT_SYMBOL_GPL (ata_eh_determine_qc);EXPORT_SYMBOL_GPL (ata_eh_autopsy);EXPORT_SYMBOL_GPL (ata_eh_report);
drivers/scsi/libata-eh.c +20 -0
@@ -560,6 +560,26 @@ static unsigned int atapi_eh_request_sen }/** +* ata_eh_detach_dev - EH helper to disable device and schedule detach +* @ap: port on which device to be detached resides +* @dev: device to be detached +* +* Disable @dev and schedule it to be detached. +* +* LOCKING: +* None. +*/ +void ata_eh_detach_dev (struct ata_port *ap,struct ata_device *dev) +{ +unsigned long flags; + +ata_dev_disable (ap, dev); +spin_lock_irqsave (&ap->host_set->lock, flags); + dev->flags |=ATA_DFLAG_DETACH_ATA |ATA_DFLAG_DETACH_SCSI ; +spin_unlock_irqrestore (&ap->host_set->lock, flags); +} + +/** * ata_read_log_page - read a specific log page * @ap: port on which device we wish to probe resides * @dev: target device
include/linux/libata.h +1 -0
@@ -719,6 +719,7 @@ extern void ata_eng_timeout(struct ata_pextern void ata_eh_schedule_port (struct ata_port *ap,unsigned int flags);extern void ata_eh_qc_complete (struct ata_queued_cmd *qc);extern void ata_eh_qc_retry (struct ata_queued_cmd *qc); +extern void ata_eh_detach_dev (struct ata_port *ap,struct ata_device *dev);extern struct ata_queued_cmd *ata_eh_determine_qc (struct ata_port *ap,int use_log_10h,struct ata_taskfile *tf);
Signature · 1 line
1.2.4
PATCH 2/15 libata-hp: implement ata_eh_hotplug()
In reply to Tejun HeoThis is ATA part of hotplug support. This function is to be called from ->error_handler after all regular EH stuff is complete. ata_eh_hotplug() may automatically mark some devices for detach and detaches all marked devices. Then, if requested and makes sense, ata_bus_probe() is invoked to probe newly attached devices.
ata_eh_hotplug() makes sure that a detached ATA device is not accessed from its corresponding SCSI device by offlining the SCSI device before reusing the ATA device. The rest of SCSI hotplug will be implemented in the next patch.
---
drivers/scsi/libata-core.c | 1 drivers/scsi/libata-eh.c | 103 ++++++++++++++++++++++++++++++++++++++++++++ drivers/scsi/libata-scsi.c | 18 ++++++++ drivers/scsi/libata.h | 1 include/linux/libata.h | 1 5 files changed, 124 insertions(+), 0 deletions(-)
adf5feec346d7dea425d98a9e473a8704b2751d6
drivers/scsi/libata-core.c +1 -0
@@ -5489,3 +5489,4 @@ EXPORT_SYMBOL_GPL(ata_eh_autopsy);EXPORT_SYMBOL_GPL (ata_eh_report);EXPORT_SYMBOL_GPL (ata_eh_revive);EXPORT_SYMBOL_GPL (ata_eh_finish_qcs); +EXPORT_SYMBOL_GPL (ata_eh_hotplug);
drivers/scsi/libata-eh.c +103 -0
112 lines not on this page · raw message
drivers/scsi/libata-scsi.c +18 -0
@@ -2760,3 +2760,21 @@ void ata_scsi_scan_host(struct ata_port } } +/** +* ata_scsi_offline_dev - offline attached SCSI device +* @ap: ATA port @dev is on +* @dev: ATA device to offline attached SCSI device for +* +* This function is called from ata_eh_hotplug() and responsible +* for taking the SCSI device attached to @dev offline. This +* function is called with host_set lock which protects dev->sdev +* against clearing. +* +* LOCKING: +* spin_lock_irqsave(host_set lock) +*/ +void ata_scsi_offline_dev (struct ata_port *ap,struct ata_device *dev) +{ +if (dev->sdev) +scsi_device_set_state (dev->sdev,SDEV_OFFLINE ); +}
drivers/scsi/libata.h +1 -0
@@ -76,6 +76,7 @@ extern int ata_cmd_ioctl(struct scsi_devextern struct scsi_transport_template ata_scsi_transport_template;extern void ata_scsi_scan_host (struct ata_port *ap); +extern void ata_scsi_offline_dev (struct ata_port *ap,struct ata_device *dev);extern unsigned int ata_scsiop_inq_std (struct ata_scsi_args *args,u8 *rbuf,unsigned int buflen);
include/linux/libata.h +1 -0
@@ -734,6 +734,7 @@ extern int ata_eh_revive(struct ata_port ata_postreset_fn_t postreset);extern void ata_eh_finish_qcs (struct ata_port *ap,struct ata_queued_cmd *qc,struct ata_taskfile *tf); +extern void ata_eh_hotplug (struct ata_port *ap); static inline int
Signature · 1 line
1.2.4
PATCH 3/15 libata-hp: implement ata_eh_scsi_hotplug()
In reply to Tejun HeoThis is SCSI part of hotplug support. Devices marked for SCSI unplug are scsi_remove'd, and, if SCSI plug is requested, all enabled but unattached ATA devices get scanned and attached. This patch only implements SCSI hotplug support. The next patch will connect ATA hotplug events to SCSI hotplug.
---
drivers/scsi/libata-eh.c | 59 ++++++++++++++++++++++++++++++++++++++++++++ drivers/scsi/libata-scsi.c | 41 +++++++++++++++++++++++++++++++ drivers/scsi/libata.h | 2 + 3 files changed, 102 insertions(+), 0 deletions(-)
14867aadbb30dc446856a73ff519456a38e1758d
drivers/scsi/libata-eh.c +59 -0
@@ -1472,3 +1472,62 @@ void ata_eh_hotplug(struct ata_port *ap)* current EH run is complete. */ } + +/** +* ata_eh_scsi_hotplug - SCSI part of hot [un]plugging +* @data: Pointer to ATA port to perform SCSI hot [un]plugging on +* +* Perform SCSI part of hot [un]plugging. It's executed from a +* separate workqueue after EH completes. This is necessary +* because SCSI hot plugging requires working EH and hot +* unplugging is synchronized with hot plugging with a mutex. +*/ +void ata_eh_scsi_hotplug (void *data) +{ +struct ata_port *ap = data; +unsigned long timeout; +int i; + +ata_clr_hotplug_flags (ap,ATA_HOTPLUG_SCSI_UNPLUG ); + +/* always unplug detached devices */ +for (i =0 ; i <ATA_MAX_DEVICES ; i++) { +struct ata_device *dev = &ap->device[i]; + +if (dev->flags &ATA_DFLAG_DETACH_SCSI ) { +unsigned long flags; + +spin_lock_irqsave (&ap->host_set->lock, flags); + dev->flags &= ~ATA_DFLAG_DETACH_SCSI ; +spin_unlock_irqrestore (&ap->host_set->lock, flags); + +ata_scsi_remove_dev (ap, dev); + } + } + +/* SCSI hotplug is requested. EH might still be running and +* we wanna scan the bus after EH is complete; otherwise, SCSI +* scan fails silently. scsi_block_when_processing_errors() +* cannot be used because we might not have a sdev to wait on. +* Poll for !scsi_host_in_recovery() for at most 5 secs. +* +* The worst that can happen here is bus scan failure, well, +* the user will have to replug the device. No biggie. +*/ + timeout = jiffies +5 *HZ ; +do { +if (!scsi_host_in_recovery (ap->host)) +break ; +msleep (100 ); + }while (time_before (jiffies, timeout)); + +if (scsi_host_in_recovery (ap->host)) +printk (KERN_WARNING "ata%u: EH still in progress after " +"5 secs, SCSI scan might fail\n" , ap->id); + +/* scan if plug operation is requested */ +if (ap->hotplug_flags &ATA_HOTPLUG_SCSI_PLUG ) { +ata_clr_hotplug_flags (ap,ATA_HOTPLUG_SCSI_PLUG ); +ata_scsi_scan_host (ap); + } +}
drivers/scsi/libata-scsi.c +41 -0
44 lines not on this page · raw message
drivers/scsi/libata.h +2 -0
@@ -77,6 +77,7 @@ extern struct scsi_transport_template atextern void ata_scsi_scan_host (struct ata_port *ap);extern void ata_scsi_offline_dev (struct ata_port *ap,struct ata_device *dev); +extern void ata_scsi_remove_dev (struct ata_port *ap,struct ata_device *dev);extern unsigned int ata_scsiop_inq_std (struct ata_scsi_args *args,u8 *rbuf,unsigned int buflen);
@@ -110,5 +111,6 @@ extern void ata_scsi_rbuf_fill(struct atextern void ata_ering_init (struct ata_ering *ering,int size);extern enum scsi_eh_timer_return ata_scsi_timed_out (struct scsi_cmnd *cmd);extern void ata_eh_schedule_qc (struct ata_queued_cmd *qc); +extern void ata_eh_scsi_hotplug (void *data);#endif /* __LIBATA_H__ */
Signature · 1 line
1.2.4
PATCH 4/15 libata-hp: connect ATA hotplug events to SCSI hotplug
In reply to Tejun HeoSchedule SCSI plug/unplug operations from EH with appropriate hotplug flags and run scsi hotplug from ata_hotplug_wq on EH completion. On host initialization, SCSI hotplug starts disabled until ATA_HOTPLUG_RUNNING flag is set on boot probing completion. This is to avoid running multiple SCSI plug/unplug operations simultaneously.
---
drivers/scsi/libata-core.c | 3 +++ drivers/scsi/libata-eh.c | 12 ++++++++++++ include/linux/libata.h | 1 + 3 files changed, 16 insertions(+), 0 deletions(-)
dce4c2009a3abc3fa370b639feda83e09c96dc9c
drivers/scsi/libata-core.c +3 -0
@@ -4896,6 +4896,7 @@ static void ata_host_init(struct ata_por ap->last_ctl =0xFF ;INIT_WORK (&ap->port_task,NULL ,NULL ); +INIT_WORK (&ap->hotplug_task, ata_eh_scsi_hotplug, ap);INIT_LIST_HEAD (&ap->eh_done_q);for (i =0 ; i <ATA_MAX_DEVICES ; i++)
@@ -5088,6 +5089,8 @@ int ata_device_add(const struct ata_probstruct ata_port *ap = host_set->ports[i];ata_scsi_scan_host (ap); +/* boot probe done, allow hotplugging */ +ata_set_hotplug_flags (ap,ATA_HOTPLUG_RUNNING ); }dev_set_drvdata (dev, host_set);
drivers/scsi/libata-eh.c +12 -0
@@ -240,6 +240,12 @@ int ata_scsi_error(struct Scsi_Host *hos/* bookkeeping for hotplug */ ata_clr_hotplug_flags (ap,ATA_HOTPLUG_PROBE |ATA_HOTPLUG_DID_PROBE ); +/* schedule SCSI hotplug if requested */ +if (ap->hotplug_flags &ATA_HOTPLUG_RUNNING && + ap->hotplug_flags & (ATA_HOTPLUG_SCSI_PLUG | +ATA_HOTPLUG_SCSI_UNPLUG )) +queue_work (ata_hotplug_wq, &ap->hotplug_task); +DPRINTK ("EXIT\n" );return 0 ; }
@@ -1434,6 +1440,9 @@ void ata_eh_hotplug(struct ata_port *ap) dev->flags &= ~ATA_DFLAG_DETACH_ATA ;ata_scsi_offline_dev (ap, dev);spin_unlock_irqrestore (&ap->host_set->lock, flags); + +/* schedule SCSI unplug */ +ata_set_hotplug_flags (ap,ATA_HOTPLUG_SCSI_UNPLUG ); }/* probe requested? */
@@ -1530,4 +1539,7 @@ void ata_eh_scsi_hotplug(void *data)ata_clr_hotplug_flags (ap,ATA_HOTPLUG_SCSI_PLUG );ata_scsi_scan_host (ap); } + +/* schedule SCSI plug */ +ata_set_hotplug_flags (ap,ATA_HOTPLUG_SCSI_PLUG ); }
include/linux/libata.h +1 -0
@@ -482,6 +482,7 @@ struct ata_port {struct list_head eh_done_q;unsigned long hotplug_flags; +struct work_struct hotplug_task;void *private_data; };
Signature · 1 line
1.2.4
PATCH 5/15 libata-hp: implement ata_scsi_slave_destroy()
In reply to Tejun HeoThis function is called during sdev removal by SCSI midlayer. For user-initiated removals, this is the only indication we get from SCSI midlayer that the sdev is going away. In such cases, schedule ATA detach and invoke EH. For libata-initiated removal, nothing needs to be done.
---
drivers/scsi/libata-core.c | 1 + drivers/scsi/libata-eh.c | 15 ++++++++++++--- drivers/scsi/libata-scsi.c | 33 +++++++++++++++++++++++++++++++++ include/linux/libata.h | 1 + 4 files changed, 47 insertions(+), 3 deletions(-)
50661abaf2563e89f2277c7e39c817123459779f
drivers/scsi/libata-core.c +1 -0
@@ -5452,6 +5452,7 @@ EXPORT_SYMBOL_GPL(ata_port_queue_task);EXPORT_SYMBOL_GPL (ata_scsi_ioctl);EXPORT_SYMBOL_GPL (ata_scsi_queuecmd);EXPORT_SYMBOL_GPL (ata_scsi_slave_config); +EXPORT_SYMBOL_GPL (ata_scsi_slave_destroy);EXPORT_SYMBOL_GPL (ata_scsi_change_queue_depth);EXPORT_SYMBOL_GPL (ata_scsi_release);EXPORT_SYMBOL_GPL (ata_host_intr);
drivers/scsi/libata-eh.c +12 -3
@@ -175,7 +175,7 @@ int ata_scsi_error(struct Scsi_Host *hos {struct ata_port *ap = (struct ata_port *)&host->hostdata[0 ];spinlock_t *hs_lock = &ap->host_set->lock; -int repeat_cnt =ATA_EH_MAX_REPEAT ; +int i, repeat_cnt =ATA_EH_MAX_REPEAT ;unsigned long flags;DPRINTK ("ENTER\n" );
@@ -185,10 +185,19 @@ int ata_scsi_error(struct Scsi_Host *hosspin_unlock_wait (hs_lock);ata_port_flush_task (ap); +/* Scheduled to be detached but not disabled status happens +* only for devices user requested to be detached via +* slave_destroy. Make sure they are disabled. +*/ +for (i =0 ; i <ATA_MAX_DEVICES ; i++) { +struct ata_device *dev = &ap->device[i]; + +if (dev->flags &ATA_DFLAG_DETACH_ATA ) +ata_dev_disable (ap, dev); + } +/* invoke error handler */ if (ap->ops->error_handler) { -int i; -/* for new EH, all the qc's are ours now */ for (i =0 ; i <ATA_MAX_QUEUE ; i++) {struct ata_queued_cmd *qc;
drivers/scsi/libata-scsi.c +33 -0
@@ -723,6 +723,39 @@ int ata_scsi_slave_config(struct scsi_de }/** +* ata_scsi_slave_destroy - SCSI device is about to be destroyed +* @sdev: SCSI device to be destroyed +* +* @sdev is about to be destroyed for hot/warm unplugging. If +* this unplugging was initiated by libata as indicated by NULL +* dev->sdev, this function doesn't have to do anything. +* Otherwise, SCSI layer initiated warm-unplug is in progress. +* Clear dev->sdev, schedule the device for ATA detach and invoke +* EH. +* +* LOCKING: +* Defined by SCSI layer. We don't really care. +*/ +void ata_scsi_slave_destroy (struct scsi_device *sdev) +{ +struct ata_port *ap = (struct ata_port *)&sdev->host->hostdata[0 ]; +unsigned long flags; +struct ata_device *dev; + +if (!ap->ops->error_handler) +return ; + +spin_lock_irqsave (&ap->host_set->lock, flags); + dev =__ata_scsi_find_dev (ap, sdev); +if (dev && dev->sdev) { + dev->sdev =NULL ; + dev->flags |=ATA_DFLAG_DETACH_ATA ; +ata_eh_schedule_port (ap,ATA_EH_FREEZE ); + } +spin_unlock_irqrestore (&ap->host_set->lock, flags); +} + +/** * ata_scsi_change_queue_depth - SCSI callback for queue depth config * @sdev: SCSI device to configure queue depth for * @queue_depth: new queue depth
include/linux/libata.h +1 -0
@@ -663,6 +663,7 @@ extern int ata_std_bios_param(struct scsstruct block_device *bdev, sector_t capacity, int geom[]);extern int ata_scsi_slave_config (struct scsi_device *sdev); +extern void ata_scsi_slave_destroy (struct scsi_device *sdev);extern int ata_scsi_change_queue_depth (struct scsi_device *sdev,int queue_depth); extern struct ata_device *ata_dev_pair(struct ata_port *ap,
Signature · 1 line
1.2.4
Re: [PATCH 05/15] libata-hp: implement ata_scsi_slave_destroy()
Quoted 6 lines · Tejun Heo
This function is called during sdev removal by SCSI midlayer. For user-initiated removals, this is the only indication we get from SCSI midlayer that the sdev is going away. In such cases, schedule ATA detach and invoke EH. For libata-initiated removal, nothing needs to be done.
User initiated warm unplugging has a race condition. If user initiated warm unplug kicks int after ata_scsi_remove_dev() fetched dev->sdev but before it actually removes the device, the sdev will go away while ata_scsi_remove_dev() is still trying to remove it. This can be solved by doing scsi_device_get() in ata_scsi_remove_dev() after fetching dev->sdev.
I'll post the fixed version in the next round. The #upstream change breaks all EH/NCQ/hotplug patchsets anyway. My repo currently contains the following changes from the posted version.
* above mentioned sdev removal race fix
* scsi_eh_schedule_* are updated to reflect ->eh_strategy_handler relocation
* duplicate .can_queue initialization removed from sil24-NCQ patch removed (why doesn't compiler complian about this?)
* updated to use ata_shost_to_port()
I think above changes shouldn't hinder reviewing process too much. Though, if you want me to post the updated version, just let me know.
Thanks.
Signature · 1 line
tejun
Re: [PATCH 05/15] libata-hp: implement ata_scsi_slave_destroy()
Quoted 26 lines · Tejun Heo
Quoted 6 lines · Tejun Heo
This function is called during sdev removal by SCSI midlayer. For user-initiated removals, this is the only indication we get from SCSI midlayer that the sdev is going away. In such cases, schedule ATA detach and invoke EH. For libata-initiated removal, nothing needs to be done.
User initiated warm unplugging has a race condition. If user initiated warm unplug kicks int after
ata_scsi_remove_dev()fetcheddev->sdevbut before it actually removes the device, the sdev will go away whileata_scsi_remove_dev()is still trying to remove it. This can be solved by doingscsi_device_get()inata_scsi_remove_dev()after fetchingdev->sdev.I'll post the fixed version in the next round. The #upstream change breaks all EH/NCQ/hotplug patchsets anyway. My repo currently contains the following changes from the posted version.
* above mentioned sdev removal race fix
* scsi_eh_schedule_* are updated to reflect ->eh_strategy_handler relocation
* duplicate .can_queue initialization removed from sil24-NCQ patch removed (why doesn't compiler complian about this?)
* updated to useata_shost_to_port()
Quoted 2 lines
I think above changes shouldn't hinder reviewing process too much. Though, if you want me to post the updated version, just let me know.
Actually, you have so many patches now, it would make the process go faster to post them in cascading git branches somewhere.
tejun.git#p5 == libata-dev.git#upstream + new EH framework
tejun.git#p6 == #p5 + new EH implementation
tejun.git#p7 == #p6 + add new NCQ support
etc.
That way, if I approve through patchset #7, I can just pull tejun.git#p7, and get all patchsets through patchset 7. If some patches need to be revised, a script should be able to reconstitute these sets of branches.
git is recommended for high volume submittors :)
Jeff
Re: [PATCH 05/15] libata-hp: implement ata_scsi_slave_destroy()
Quoted 30 lines · Jeff Garzik
Quoted 26 lines · Tejun Heo
Quoted 6 lines · Tejun Heo
This function is called during sdev removal by SCSI midlayer. For user-initiated removals, this is the only indication we get from SCSI midlayer that the sdev is going away. In such cases, schedule ATA detach and invoke EH. For libata-initiated removal, nothing needs to be done.
User initiated warm unplugging has a race condition. If user initiated warm unplug kicks int after
ata_scsi_remove_dev()fetcheddev->sdevbut before it actually removes the device, the sdev will go away whileata_scsi_remove_dev()is still trying to remove it. This can be solved by doingscsi_device_get()inata_scsi_remove_dev()after fetchingdev->sdev.I'll post the fixed version in the next round. The #upstream change breaks all EH/NCQ/hotplug patchsets anyway. My repo currently contains the following changes from the posted version.
* above mentioned sdev removal race fix
* scsi_eh_schedule_* are updated to reflect ->eh_strategy_handler relocation
* duplicate .can_queue initialization removed from sil24-NCQ patch removed (why doesn't compiler complian about this?)
* updated to useata_shost_to_port()Quoted 2 lines
I think above changes shouldn't hinder reviewing process too much. Though, if you want me to post the updated version, just let me know.
Quoted 9 lines
Actually, you have so many patches now, it would make the process go faster to post them in cascading git branches somewhere.
tejun.git#p5 == libata-dev.git#upstream + new EH framework tejun.git#p6 == #p5 + new EH implementation tejun.git#p7 == #p6 + add new NCQ supportetc.
That way, if I approve through patchset #7, I can just pull tejun.git#p7, and get all patchsets through patchset 7. If some patches need to be revised, a script should be able to reconstitute these sets of branches.
Yeap, will do.
Thanks for reviewing all these. :)
Signature · 1 line
tejun
PATCH 6/15 libata-hp: use ata_scsi_slave_destroy() in low level drivers
In reply to Tejun HeoUse ata_scsi_slave_destroy() in all libata low level drivers.
---
drivers/scsi/ahci.c | 1 + drivers/scsi/ata_piix.c | 1 + drivers/scsi/pdc_adma.c | 1 + drivers/scsi/sata_mv.c | 1 + drivers/scsi/sata_nv.c | 1 + drivers/scsi/sata_promise.c | 1 + drivers/scsi/sata_qstor.c | 1 + drivers/scsi/sata_sil.c | 1 + drivers/scsi/sata_sil24.c | 1 + drivers/scsi/sata_sis.c | 1 + drivers/scsi/sata_svw.c | 1 + drivers/scsi/sata_sx4.c | 1 + drivers/scsi/sata_uli.c | 1 + drivers/scsi/sata_via.c | 1 + drivers/scsi/sata_vsc.c | 1 + 15 files changed, 15 insertions(+), 0 deletions(-)
ad8e69e7f7a668b6d60583cb4bfeb1f690331ca7
drivers/scsi/ahci.c +1 -0
@@ -226,6 +226,7 @@ static struct scsi_host_template ahci_sh .proc_name = DRV_NAME, .dma_boundary = AHCI_DMA_BOUNDARY, .slave_configure = ata_scsi_slave_config, + .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, };
drivers/scsi/ata_piix.c +1 -0
@@ -219,6 +219,7 @@ static struct scsi_host_template piix_sh .proc_name = DRV_NAME, .dma_boundary = ATA_DMA_BOUNDARY, .slave_configure = ata_scsi_slave_config, + .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, .resume = ata_scsi_device_resume, .suspend = ata_scsi_device_suspend,
drivers/scsi/pdc_adma.c +1 -0
@@ -153,6 +153,7 @@ static struct scsi_host_template adma_at .proc_name = DRV_NAME, .dma_boundary = ADMA_DMA_BOUNDARY, .slave_configure = ata_scsi_slave_config, + .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, };
drivers/scsi/sata_mv.c +1 -0
@@ -388,6 +388,7 @@ static struct scsi_host_template mv_sht .proc_name = DRV_NAME, .dma_boundary = MV_DMA_BOUNDARY, .slave_configure = ata_scsi_slave_config, + .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, };
drivers/scsi/sata_nv.c +1 -0
@@ -211,6 +211,7 @@ static struct scsi_host_template nv_sht .proc_name = DRV_NAME, .dma_boundary = ATA_DMA_BOUNDARY, .slave_configure = ata_scsi_slave_config, + .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, };
drivers/scsi/sata_promise.c +1 -0
@@ -121,6 +121,7 @@ static struct scsi_host_template pdc_ata .proc_name = DRV_NAME, .dma_boundary = ATA_DMA_BOUNDARY, .slave_configure = ata_scsi_slave_config, + .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, };
drivers/scsi/sata_qstor.c +1 -0
@@ -143,6 +143,7 @@ static struct scsi_host_template qs_ata_ .proc_name = DRV_NAME, .dma_boundary = QS_DMA_BOUNDARY, .slave_configure = ata_scsi_slave_config, + .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, };
drivers/scsi/sata_sil.c +1 -0
@@ -159,6 +159,7 @@ static struct scsi_host_template sil_sht .proc_name = DRV_NAME, .dma_boundary = ATA_DMA_BOUNDARY, .slave_configure = ata_scsi_slave_config, + .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, };
drivers/scsi/sata_sil24.c +1 -0
@@ -371,6 +371,7 @@ static struct scsi_host_template sil24_s .proc_name = DRV_NAME, .dma_boundary = ATA_DMA_BOUNDARY, .slave_configure = ata_scsi_slave_config, + .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, };
drivers/scsi/sata_sis.c +1 -0
@@ -97,6 +97,7 @@ static struct scsi_host_template sis_sht .proc_name = DRV_NAME, .dma_boundary = ATA_DMA_BOUNDARY, .slave_configure = ata_scsi_slave_config, + .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, };
drivers/scsi/sata_svw.c +1 -0
@@ -300,6 +300,7 @@ static struct scsi_host_template k2_sata .proc_name = DRV_NAME, .dma_boundary = ATA_DMA_BOUNDARY, .slave_configure = ata_scsi_slave_config, + .slave_destroy = ata_scsi_slave_destroy,#ifdef CONFIG_PPC_OF .proc_info = k2_sata_proc_info,#endif
drivers/scsi/sata_sx4.c +1 -0
@@ -192,6 +192,7 @@ static struct scsi_host_template pdc_sat .proc_name = DRV_NAME, .dma_boundary = ATA_DMA_BOUNDARY, .slave_configure = ata_scsi_slave_config, + .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, };
drivers/scsi/sata_uli.c +1 -0
@@ -91,6 +91,7 @@ static struct scsi_host_template uli_sht .proc_name = DRV_NAME, .dma_boundary = ATA_DMA_BOUNDARY, .slave_configure = ata_scsi_slave_config, + .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, };
drivers/scsi/sata_via.c +1 -0
@@ -104,6 +104,7 @@ static struct scsi_host_template svia_sh .proc_name = DRV_NAME, .dma_boundary = ATA_DMA_BOUNDARY, .slave_configure = ata_scsi_slave_config, + .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, };
drivers/scsi/sata_vsc.c +1 -0
@@ -273,6 +273,7 @@ static struct scsi_host_template vsc_sat .proc_name = DRV_NAME, .dma_boundary = ATA_DMA_BOUNDARY, .slave_configure = ata_scsi_slave_config, + .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, };
Signature · 1 line
1.2.4
PATCH 7/15 libata-hp: implement transportt->user_scan
In reply to Tejun HeoImplement ata_scsi_user_scan() which is used for ata_scsi_transport_template->user_scan. This function schedules ATA hotplug probe on user request.
---
drivers/scsi/libata-scsi.c | 37 +++++++++++++++++++++++++++++++++++++ 1 files changed, 37 insertions(+), 0 deletions(-)
18519ac3e6800187945e6d81bd1574c091eb0eaf
drivers/scsi/libata-scsi.c +37 -0
@@ -55,6 +55,8 @@ typedef unsigned int (*ata_xlat_func_t)(static struct ata_device *__ata_scsi_find_dev (struct ata_port *ap,const struct scsi_device *scsidev); +static int ata_scsi_user_scan (struct Scsi_Host *shost,unsigned int channel, +unsigned int id,unsigned int lun);#define RW_RECOVERY_MPAGE 0x1#define RW_RECOVERY_MPAGE_LEN 12
@@ -101,6 +103,7 @@ static const u8 def_control_mpage[CONTRO*/ struct scsi_transport_template ata_scsi_transport_template = { .eh_timed_out = ata_scsi_timed_out, + .user_scan = ata_scsi_user_scan, };
@@ -2852,3 +2855,37 @@ void ata_scsi_remove_dev(struct ata_portscsi_remove_device (sdev); } + +/** +* ata_scsi_user_scan - indication for user-initiated bus scan +* @shost: SCSI host to scan +* @channel: Channel to scan +* @id: ID to scan +* @lun: LUN to scan +* +* This function is called when user explicitly requests bus +* scan. Set probe pending flag and invoke EH. libata scans the +* whole bus regardless of channel, id and lun arguments. +* +* LOCKING: +* SCSI layer (we don't care) +* +* RETURNS: +* Zero. +*/ +static int ata_scsi_user_scan (struct Scsi_Host *shost,unsigned int channel, +unsigned int id,unsigned int lun) +{ +struct ata_port *ap = (struct ata_port *)&shost->hostdata[0 ]; +unsigned long flags; + +if (!ap->ops->error_handler) +return -EOPNOTSUPP ; + +spin_lock_irqsave (&ap->host_set->lock, flags); +ata_set_hotplug_flags (ap,ATA_HOTPLUG_PROBE ); +ata_eh_schedule_port (ap,ATA_EH_FREEZE ); +spin_unlock_irqrestore (&ap->host_set->lock, flags); + +return 0 ; +}
Signature · 1 line
1.2.4
PATCH 8/15 libata-hp: add hotplug hooks into regular EH
In reply to Tejun HeoAdd hotplug handling into ata_eh_analyze_serror() and ata_eh_revive() such that they schedule probe or detach if it seems appropriate.
---
drivers/scsi/libata-eh.c | 12 ++++++++++-- 1 files changed, 10 insertions(+), 2 deletions(-)
a335b73795061654e9dee96c2fc5f161e92780c7
drivers/scsi/libata-eh.c +10 -2
@@ -855,7 +855,8 @@ static unsigned int ata_eh_analyze_tf(st* @p_err_mask: Resulting err_mask * * Analyze SError if available and further determine cause of -* failure. +* failure. This function also schedules probing if PHY status +* has changed. * * LOCKING: * None.
@@ -885,6 +886,11 @@ static unsigned int ata_eh_analyze_serro *p_err_mask |=AC_ERR_SYSTEM ; action |=ATA_PORT_SOFTRESET ; } +if (serror & (SERR_PHYRDY_CHG |SERR_DEV_XCHG )) { + *p_err_mask |=AC_ERR_ATA_BUS ; + action |=ATA_PORT_SOFTRESET ; +ata_schedule_probe (ap); + }return action; }
@@ -1282,8 +1288,10 @@ int ata_eh_revive(struct ata_port *ap, u fail:switch (rc) { -case -EINVAL :case -ENODEV : +ata_schedule_probe (ap); +case -EINVAL : +ata_eh_detach_dev (ap, dev); tries[dev->devno] =0 ;break ;case -EIO :
Signature · 1 line
1.2.4
PATCH 9/15 libata-hp: activate hotplug by adding a call to ata_eh_hotplug() from EH
In reply to Tejun HeoNow all the required pieces for hotplug support are in place. Update all ->error_handler's to call ata_eh_hotplug() after regular EH is finished. This enables hotplug support for all drivers which use new EH. With this patch, all new EH drivers can do warm unplug and plug.
---
drivers/scsi/ahci.c | 1 + drivers/scsi/libata-bmdma.c | 1 + drivers/scsi/sata_sil24.c | 1 + 3 files changed, 3 insertions(+), 0 deletions(-)
5a9a286e07ca0d216be1b8f81ab6c6e7048ae963
drivers/scsi/ahci.c +1 -0
@@ -864,6 +864,7 @@ static void ahci_error_handler(struct atata_eh_revive (ap, action, ahci_softreset, ahci_hardreset, ahci_postreset);ata_eh_finish_qcs (ap, qc, &tf); +ata_eh_hotplug (ap); }static void ahci_post_internal_cmd (struct ata_queued_cmd *qc)
drivers/scsi/libata-bmdma.c +1 -0
@@ -729,6 +729,7 @@ void ata_bmdma_drive_eh(struct ata_portata_eh_report (ap, qc, &tf, serror, action,NULL );ata_eh_revive (ap, action, softreset, hardreset, postreset);ata_eh_finish_qcs (ap, qc, &tf); +ata_eh_hotplug (ap); }/**
drivers/scsi/sata_sil24.c +1 -0
@@ -852,6 +852,7 @@ static void sil24_error_handler(struct aata_eh_revive (ap, action, sil24_softreset, sil24_hardreset, sil24_postreset);ata_eh_finish_qcs (ap, qc, &tf); +ata_eh_hotplug (ap); }static void sil24_post_internal_cmd (struct ata_queued_cmd *qc)
Signature · 1 line
1.2.4
Re: [PATCH 09/15] libata-hp: activate hotplug by adding a call to ata_eh_hotplug() from EH
Quoted 44 lines · Tejun Heo
Now all the required pieces for hotplug support are in place. Update all ->error_handler's to call
ata_eh_hotplug()after regular EH is finished. This enables hotplug support for all drivers which use new EH. With this patch, all new EH drivers can do warm unplug and plug.Signed-off-by Tejun Heo <[email protected]>---
drivers/scsi/ahci.c | 1 + drivers/scsi/libata-bmdma.c | 1 + drivers/scsi/sata_sil24.c | 1 + 3 files changed, 3 insertions(+), 0 deletions(-)
5a9a286e07ca0d216be1b8f81ab6c6e7048ae963drivers/scsi/ahci.c +1
-0@@ -864,6 +864,7 @@ static void ahci_error_handler(struct atata_eh_revive (ap, action, ahci_softreset, ahci_hardreset, ahci_postreset);ata_eh_finish_qcs (ap, qc, &tf); +ata_eh_hotplug (ap); }static void ahci_post_internal_cmd (struct ata_queued_cmd *qc)drivers/scsi/libata-bmdma.c +1
-0@@ -729,6 +729,7 @@ void ata_bmdma_drive_eh(struct ata_portata_eh_report (ap, qc, &tf, serror, action,NULL );ata_eh_revive (ap, action, softreset, hardreset, postreset);ata_eh_finish_qcs (ap, qc, &tf); +ata_eh_hotplug (ap); }/** drivers/scsi/sata_sil24.c +0
-0
drivers/scsi/sata_sil24.c static void sil24_error_handler(struct a · line 857
Quoted 6 lines
drivers/scsi/sata_sil24.c +1
-0@@ -852,6 +852,7 @@ static void sil24_error_handler(struct aata_eh_revive (ap, action, sil24_softreset, sil24_hardreset, sil24_postreset);ata_eh_finish_qcs (ap, qc, &tf); +ata_eh_hotplug (ap); }
Does it make sense to invoke ata_eh_hotplug() in ata_scsi_error() instead of invoking ata_eh_hotplug() in each LLDD in order to make code more general?
Thanks,
Forrest
Re: [PATCH 09/15] libata-hp: activate hotplug by adding a call to ata_eh_hotplug() from EH
Quoted 39 lines · forrest
Quoted 38 lines · Tejun Heo
Now all the required pieces for hotplug support are in place. Update all ->error_handler's to call
ata_eh_hotplug()after regular EH is finished. This enables hotplug support for all drivers which use new EH. With this patch, all new EH drivers can do warm unplug and plug.Signed-off-by Tejun Heo <[email protected]>---
drivers/scsi/ahci.c | 1 + drivers/scsi/libata-bmdma.c | 1 + drivers/scsi/sata_sil24.c | 1 + 3 files changed, 3 insertions(+), 0 deletions(-)
5a9a286e07ca0d216be1b8f81ab6c6e7048ae963drivers/scsi/ahci.c +1
-0@@ -864,6 +864,7 @@ static void ahci_error_handler(struct atata_eh_revive (ap, action, ahci_softreset, ahci_hardreset, ahci_postreset);ata_eh_finish_qcs (ap, qc, &tf); +ata_eh_hotplug (ap); }static void ahci_post_internal_cmd (struct ata_queued_cmd *qc)drivers/scsi/libata-bmdma.c +1
-0@@ -729,6 +729,7 @@ void ata_bmdma_drive_eh(struct ata_portata_eh_report (ap, qc, &tf, serror, action,NULL );ata_eh_revive (ap, action, softreset, hardreset, postreset);ata_eh_finish_qcs (ap, qc, &tf); +ata_eh_hotplug (ap); }
Quoted 17 lines
drivers/scsi/sata_sil24.c static void sil24_error_handler(struct a · line 857
Quoted 12 lines
drivers/scsi/libata-bmdma.c +0
-0/** drivers/scsi/sata_sil24.c +1
-0@@ -852,6 +852,7 @@ static void sil24_error_handler(struct aata_eh_revive (ap, action, sil24_softreset, sil24_hardreset, sil24_postreset);ata_eh_finish_qcs (ap, qc, &tf); +ata_eh_hotplug (ap); }Does it make sense to invoke
ata_eh_hotplug()inata_scsi_error()instead of invokingata_eh_hotplug()in each LLDD in order to make code more general?
No, not really. To do that we'll need another port operation ->hotplug(), which, I think, has no real benefits.
Signature · 1 line
tejun
Re: [PATCH 09/15] libata-hp: activate hotplug by adding a call to ata_eh_hotplug() from EH
Quoted 58 lines · Tejun Heo
Quoted 39 lines · forrest
Quoted 38 lines · Tejun Heo
Now all the required pieces for hotplug support are in place. Update all ->error_handler's to call
ata_eh_hotplug()after regular EH is finished. This enables hotplug support for all drivers which use new EH. With this patch, all new EH drivers can do warm unplug and plug.Signed-off-by Tejun Heo <[email protected]>---
drivers/scsi/ahci.c | 1 + drivers/scsi/libata-bmdma.c | 1 + drivers/scsi/sata_sil24.c | 1 + 3 files changed, 3 insertions(+), 0 deletions(-)
5a9a286e07ca0d216be1b8f81ab6c6e7048ae963drivers/scsi/ahci.c +1
-0@@ -864,6 +864,7 @@ static void ahci_error_handler(struct atata_eh_revive (ap, action, ahci_softreset, ahci_hardreset, ahci_postreset);ata_eh_finish_qcs (ap, qc, &tf); +ata_eh_hotplug (ap); }static void ahci_post_internal_cmd (struct ata_queued_cmd *qc)drivers/scsi/libata-bmdma.c +1
-0@@ -729,6 +729,7 @@ void ata_bmdma_drive_eh(struct ata_portata_eh_report (ap, qc, &tf, serror, action,NULL );ata_eh_revive (ap, action, softreset, hardreset, postreset);ata_eh_finish_qcs (ap, qc, &tf); +ata_eh_hotplug (ap); }Quoted 17 lines
drivers/scsi/sata_sil24.c static void sil24_error_handler(struct a · line 857
Quoted 12 lines
drivers/scsi/libata-bmdma.c +0
-0/** drivers/scsi/sata_sil24.c +1
-0@@ -852,6 +852,7 @@ static void sil24_error_handler(struct aata_eh_revive (ap, action, sil24_softreset, sil24_hardreset, sil24_postreset);ata_eh_finish_qcs (ap, qc, &tf); +ata_eh_hotplug (ap); }Does it make sense to invoke
ata_eh_hotplug()inata_scsi_error()instead of invokingata_eh_hotplug()in each LLDD in order to make code more general?
Quoted 4 lines
No, not really. To do that we'll need another port operation ->hotplug(), which, I think, has no real benefits.
From my understanding, in
ata_scsi_errorinvoking
ata_eh_hotplug(ap); after ap->ops->error_handler(ap); will do. No need to add ->hotplug() for port operation. This way we avoid to invoke ata_eh_hotplug() in each LLDD.
Thanks,
Forrest
Re: [PATCH 09/15] libata-hp: activate hotplug by adding a call to ata_eh_hotplug() from EH
Quoted 57 lines · forrest
Quoted 56 lines · Tejun Heo
Quoted 55 lines · forrest
Quoted 44 lines · Tejun Heo
Now all the required pieces for hotplug support are in place. Update all ->error_handler's to call
ata_eh_hotplug()after regular EH is finished. This enables hotplug support for all drivers which use new EH. With this patch, all new EH drivers can do warm unplug and plug.Signed-off-by Tejun Heo <[email protected]>---
drivers/scsi/ahci.c | 1 + drivers/scsi/libata-bmdma.c | 1 + drivers/scsi/sata_sil24.c | 1 + 3 files changed, 3 insertions(+), 0 deletions(-)
5a9a286e07ca0d216be1b8f81ab6c6e7048ae963drivers/scsi/ahci.c +1
-0@@ -864,6 +864,7 @@ static void ahci_error_handler(struct atata_eh_revive (ap, action, ahci_softreset, ahci_hardreset, ahci_postreset);ata_eh_finish_qcs (ap, qc, &tf); +ata_eh_hotplug (ap); }static void ahci_post_internal_cmd (struct ata_queued_cmd *qc)drivers/scsi/libata-bmdma.c +1
-0@@ -729,6 +729,7 @@ void ata_bmdma_drive_eh(struct ata_portata_eh_report (ap, qc, &tf, serror, action,NULL );ata_eh_revive (ap, action, softreset, hardreset, postreset);ata_eh_finish_qcs (ap, qc, &tf); +ata_eh_hotplug (ap); }/** drivers/scsi/sata_sil24.c +0
-0drivers/scsi/sata_sil24.c static void sil24_error_handler(struct a · line 857
Quoted 6 lines
drivers/scsi/sata_sil24.c +1
-0@@ -852,6 +852,7 @@ static void sil24_error_handler(struct aata_eh_revive (ap, action, sil24_softreset, sil24_hardreset, sil24_postreset);ata_eh_finish_qcs (ap, qc, &tf); +ata_eh_hotplug (ap); }Does it make sense to invoke
ata_eh_hotplug()inata_scsi_error()instead of invokingata_eh_hotplug()in each LLDD in order to make code more general?
Quoted 8 lines
Quoted 4 lines
No, not really. To do that we'll need another port operation ->hotplug(), which, I think, has no real benefits.
From my understanding, in
ata_scsi_errorinvoking
ata_eh_hotplug(ap); afterap->ops->error_handler(ap); will do. No need to add ->hotplug() for port operation. This way we avoid to invokeata_eh_hotplug()in each LLDD.
Hmmmm... maybe you're right and we don't need to allow LLDDs to override hotplug operation. After all, LLDDs cannot override ata_bus_probe(). But, then again, it's just nice and consistent to allow overriding of the operation. And, ata_eh_hotplug() fits nicely with other EH operations, IMHO. And calling one more operation isn't really a big deal.
Maybe, we need to come up with the function which packages standard calls to eh helpers for drivers like sata_sil24 and ahci, but I think the current level of code duplication is well within acceptable range.
Do you have any other reason than duplicated calls to ata_eh_hotplug()?
Signature · 1 line
tejun
PATCH 10/15 libata-hp: skip EH reset if no device to recover and hotplug pending
In reply to Tejun HeoNow that hotplug support is online, there is no reason to reset frozen empty port with pending hotplug probe (it will do the same thing). Skip EH reset in such cases.
---
drivers/scsi/libata-eh.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-)
0d1308467ab2bc82faee607440c00b9a14b0af71
drivers/scsi/libata-eh.c +6 -2
@@ -1208,8 +1208,12 @@ int ata_eh_revive(struct ata_port *ap, u } action &= ~ATA_PORT_REVALIDATE ; -/* Skip reset if possible. */ -if (!nr_enabled && !(ap->flags &ATA_FLAG_FROZEN )) +/* Skip reset if possible. Note that ata_eh_hotplug() might +* determine not to perform reset and EH can finish with a +* frozen port. EH framework will take care of such cases. +*/ +if (!nr_enabled && (!(ap->flags &ATA_FLAG_FROZEN ) || + ap->hotplug_flags &ATA_HOTPLUG_PROBE )) goto out;/* give devices some time to breath */
Signature · 1 line
1.2.4
PATCH 11/15 sata_sil: add new constants in preparation for new interrupt handler
In reply to Tejun Heosata_sil is about to get brand new interrupt handler. Add relevant constants. While at it, add a new line for readability.
---
drivers/scsi/sata_sil.c | 24 ++++++++++++++++++++---- 1 files changed, 20 insertions(+), 4 deletions(-)
7e4271599752e0c0ba1742e98c26094502638a0e
drivers/scsi/sata_sil.c +20 -4
@@ -54,6 +54,7 @@ enum {*/ SIL_FLAG_RERR_ON_DMA_ACT = (1 <<29 ),SIL_FLAG_MOD15WRITE = (1 <<30 ), +SIL_DFL_HOST_FLAGS =ATA_FLAG_SATA |ATA_FLAG_NO_LEGACY | ATA_FLAG_MMIO,
@@ -84,6 +85,20 @@ enum {/* BMDMA/BMDMA2 */ SIL_INTR_STEERING = (1 <<1 ), +SIL_DMA_ENABLE = (1 <<0 ),/* DMA run switch */ +SIL_DMA_RDWR = (1 <<3 ),/* DMA Rd-Wr */ +SIL_DMA_SATA_IRQ = (1 <<4 ),/* OR of all SATA IRQs */ +SIL_DMA_ACTIVE = (1 <<16 ),/* DMA running */ +SIL_DMA_ERROR = (1 <<17 ),/* PCI bus error */ +SIL_DMA_COMPLETE = (1 <<18 ),/* cmd complete / IRQ pending */ +SIL_DMA_N_SATA_IRQ = (1 <<6 ),/* SATA_IRQ for the next channel */ +SIL_DMA_N_ACTIVE = (1 <<24 ),/* ACTIVE for the next channel */ +SIL_DMA_N_ERROR = (1 <<25 ),/* ERROR for the next channel */ +SIL_DMA_N_COMPLETE = (1 <<26 ),/* COMPLETE for the next channel */ + +/* SIEN */ +SIL_SIEN_N = (1 <<16 ),/* triggered by SError.N */ +/* * Others */
@@ -227,6 +242,7 @@ static const struct {unsigned long tf;/* ATA taskfile register block */ unsigned long ctl;/* ATA control/altstatus register block */ unsigned long bmdma;/* DMA register block */ +unsigned long bmdma2;/* DMA register block #2 */ unsigned long fifo_cfg;/* FIFO Valid Byte Count and Control */ unsigned long scr;/* SATA control register block */ unsigned long sien;/* SATA Interrupt Enable register */
@@ -234,10 +250,10 @@ static const struct {unsigned long sfis_cfg;/* SATA FIS reception config register */ } sil_port[] = { /* port 0 ... */ - { 0x80, 0x8A, 0x00, 0x40, 0x100, 0x148, 0xb4, 0x14c }, - { 0xC0, 0xCA, 0x08, 0x44, 0x180, 0x1c8, 0xf4, 0x1cc }, - { 0x280, 0x28A, 0x200, 0x240, 0x300, 0x348, 0x2b4, 0x34c }, - { 0x2C0, 0x2CA, 0x208, 0x244, 0x380, 0x3c8, 0x2f4, 0x3cc }, + { 0x80, 0x8A, 0x00, 0x10, 0x40, 0x100, 0x148, 0xb4, 0x14c }, + { 0xC0, 0xCA, 0x08, 0x18, 0x44, 0x180, 0x1c8, 0xf4, 0x1cc }, + { 0x280, 0x28A, 0x200, 0x210, 0x240, 0x300, 0x348, 0x2b4, 0x34c }, + { 0x2C0, 0x2CA, 0x208, 0x218, 0x244, 0x380, 0x3c8, 0x2f4, 0x3cc }, /* ... port 3 */ };
Signature · 1 line
1.2.4
PATCH 12/15 sata_sil: new interrupt handler
In reply to Tejun HeoThe DMA complete bit of these controllers reflects ATA IRQ status while no DMA command is in progress. So, we can tell whether the controller is raising an interrupt or not in deterministic manner. This patch gives sata_sil its own interrupt handler which behaves much better than the original one in terms of error detection and handling. This change is also necessary for later hotplug support.
Further improvements are possible, in both 2 and 4 ports versions, we can get all status with only one readl and using custom bmdma operations can further cut down register accesses.
---
drivers/scsi/sata_sil.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 81 insertions(+), 1 deletions(-)
cae31a3e780bd2cef46e299e6bf681af88d7f51d
drivers/scsi/sata_sil.c +81 -1
@@ -112,6 +112,8 @@ static u32 sil_scr_read (struct ata_portstatic void sil_scr_write (struct ata_port *ap,unsigned int sc_reg,u32 val);static int sil_probe_reset (struct ata_port *ap,unsigned int *classes);static void sil_post_set_mode (struct ata_port *ap); +static irqreturn_t sil_interrupt (int irq,void *dev_instance, +struct pt_regs *regs);static void sil_freeze (struct ata_port *ap);static void sil_error_handler (struct ata_port *ap);
@@ -197,7 +199,7 @@ static const struct ata_port_operations .freeze = sil_freeze, .error_handler = sil_error_handler, .post_internal_cmd = ata_bmdma_post_internal_cmd, - .irq_handler = ata_interrupt, + .irq_handler = sil_interrupt, .irq_clear = ata_bmdma_irq_clear, .scr_read = sil_scr_read, .scr_write = sil_scr_write,
@@ -357,6 +359,84 @@ static int sil_probe_reset(struct ata_po sil_postreset, classes); } +static void sil_host_intr (struct ata_port *ap,u32 bmdma2) +{ +struct ata_queued_cmd *qc =ata_qc_from_tag (ap, ap->active_tag); +u8 status; + +if (unlikely (!qc || qc->tf.ctl &ATA_NIEN )) + goto freeze; + +switch (qc->tf.protocol) { +case ATA_PROT_DMA : +case ATA_PROT_ATAPI_DMA : +case ATA_PROT_ATAPI : +if (bmdma2 &SIL_DMA_ERROR ) { + qc->err_mask |=AC_ERR_HOST_BUS ; + goto freeze; + } + +/* clear DMA-Start bit */ + ap->ops->bmdma_stop (qc); + +/* fall through */ +case ATA_PROT_ATAPI_NODATA : +case ATA_PROT_NODATA : +/* check main status, clearing INTRQ */ + status =ata_chk_status (ap); +if (unlikely (status &ATA_BUSY )) { + qc->err_mask |=AC_ERR_HSM ; + goto freeze; + } + +/* ack bmdma irq events */ + ap->ops->irq_clear (ap); + +/* complete taskfile transaction */ + qc->err_mask |=ac_err_mask (status); +ata_qc_complete (qc); +break ; + +default : + goto freeze; + } +return ; + + freeze: +ata_eh_schedule_port (ap,ATA_EH_FREEZE ); +} + +static irqreturn_t sil_interrupt (int irq,void *dev_instance, +struct pt_regs *regs) +{ +struct ata_host_set *host_set = dev_instance; +void __iomem *mmio_base = host_set->mmio_base; +int handled =0 ; +unsigned long flags; +int i; + +/* TODO: make _irqsave conditional on x86 PCI IDE legacy mode */ +spin_lock_irqsave (&host_set->lock, flags); + +for (i =0 ; i < host_set->n_ports; i++) { +struct ata_port *ap = host_set->ports[i]; +u32 bmdma2 =readl (mmio_base + sil_port[ap->port_no].bmdma2); + +if (unlikely (!ap || ap->flags &ATA_FLAG_DISABLED )) +continue ; + +if (!(bmdma2 &SIL_DMA_COMPLETE )) +continue ; + +sil_host_intr (ap, bmdma2); + handled =1 ; + } + +spin_unlock_irqrestore (&host_set->lock, flags); + +return IRQ_RETVAL (handled); +} +static void sil_freeze (struct ata_port *ap) {void __iomem *mmio_base = ap->host_set->mmio_base;
Signature · 1 line
1.2.4
PATCH 13/15 sata_sil: add hotplug support
In reply to Tejun HeoAdd hotplug support by enabling SATA IRQ for SError.N and scheduling EH if the interrupt occurs. Standard EH will do the right thing and schedule probe after analyzing SError.
---
drivers/scsi/sata_sil.c | 15 ++++++++------- 1 files changed, 8 insertions(+), 7 deletions(-)
2f423e8392edda125a3cec0269e4cd7ceb7afa46
drivers/scsi/sata_sil.c +8 -7
@@ -346,6 +346,9 @@ static void sil_postreset(struct ata_porata_std_postreset (ap, classes); +/* turn on SATA IRQ */ +writel (SIL_SIEN_N , mmio_base + sil_port[ap->port_no].sien); +/* everything is back to normal, turn on IRQ */ tmp =readl (mmio_base +SIL_SYSCFG ); tmp &= ~(SIL_MASK_IDE0_INT << ap->port_no);
@@ -364,7 +367,7 @@ static void sil_host_intr(struct ata_porstruct ata_queued_cmd *qc =ata_qc_from_tag (ap, ap->active_tag);u8 status; -if (unlikely (!qc || qc->tf.ctl &ATA_NIEN )) +if (unlikely (bmdma2 &SIL_DMA_SATA_IRQ || !qc || qc->tf.ctl &ATA_NIEN )) goto freeze;switch (qc->tf.protocol) {
@@ -425,7 +428,7 @@ static irqreturn_t sil_interrupt(int irqif (unlikely (!ap || ap->flags &ATA_FLAG_DISABLED ))continue ; -if (!(bmdma2 &SIL_DMA_COMPLETE )) +if (!(bmdma2 & (SIL_DMA_COMPLETE |SIL_DMA_SATA_IRQ )))continue ;sil_host_intr (ap, bmdma2);
@@ -442,6 +445,9 @@ static void sil_freeze(struct ata_port *void __iomem *mmio_base = ap->host_set->mmio_base;u32 tmp; +/* global IRQ mask doesn't block SATA IRQ, turn off explicitly */ +writel (0 , mmio_base + sil_port[ap->port_no].sien); +/* plug IRQ */ tmp =readl (mmio_base +SIL_SYSCFG ); tmp |=SIL_MASK_IDE0_INT << ap->port_no;
@@ -622,11 +628,6 @@ static int sil_init_one (struct pci_dev mmio_base + sil_port[2].bmdma); } -/* mask all SATA phy-related interrupts */ -/* TODO: unmask bit 6 (SError N bit) for hotplug */ -for (i =0 ; i < probe_ent->n_ports; i++) -writel (0 , mmio_base + sil_port[i].sien); -pci_set_master (pdev);/* FIXME: check ata_device_add return value */
Signature · 1 line
1.2.4
PATCH 14/15 ahci: add hotplug support
In reply to Tejun HeoAdd hotplug support by enabling PORT_IRQ_PHYRDY. Standard EH will do the right thing and schedule probe after analyzing SError.
---
drivers/scsi/ahci.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
ded75cede39c99d38376691e952327cff47bb4f6
drivers/scsi/ahci.c +5 -2
@@ -135,6 +135,7 @@ enum {PORT_IRQ_FREEZE =PORT_IRQ_HBUS_ERR |PORT_IRQ_IF_ERR |PORT_IRQ_CONNECT | +PORT_IRQ_PHYRDY |PORT_IRQ_UNK_FIS ,PORT_IRQ_ERROR =PORT_IRQ_FREEZE | PORT_IRQ_TF_ERR |
@@ -801,10 +802,12 @@ static unsigned int ahci_eh_autopsy(stru desc_sz -= rc; } -if (irq_stat &PORT_IRQ_CONNECT ) { +if (irq_stat & (PORT_IRQ_CONNECT |PORT_IRQ_PHYRDY )) { err_mask |=AC_ERR_ATA_BUS ; action |=ATA_PORT_SOFTRESET ; - rc =scnprintf (desc, desc_sz,", connection status changed" ); + rc =scnprintf (desc, desc_sz,", %s" , + irq_stat &PORT_IRQ_CONNECT ? +"connection status changed" :"PHY RDY changed" ); desc += rc; desc_sz -= rc; }
Signature · 1 line
1.2.4
PATCH 15/15 sata_sil24: add hotplug support
In reply to Tejun HeoAdd hotplug support by enabling PORT_IRQ_PHYRDY_CHG and scheduling probe when either of PORT_IRQ_PHYRDY_CHG or PORT_IRQ_DEV_XCHG happens. Explicit ata_schedule_probe() is necessary because sil24's SError doesn't always reflect DEV_XCHG or PHYRDY_CHG conditions properly.
---
drivers/scsi/sata_sil24.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-)
dc756727b3be5cf5bff4a327bc13604235419fc6
drivers/scsi/sata_sil24.c +9 -3
@@ -156,7 +156,8 @@ enum {PORT_IRQ_HANDSHAKE = (1 <<10 ),/* handshake error threshold */ PORT_IRQ_SDB_NOTIFY = (1 <<11 ),/* SDB notify received */ -PORT_IRQ_FREEZE =PORT_IRQ_DEV_XCHG |PORT_IRQ_UNK_FIS , +PORT_IRQ_FREEZE =PORT_IRQ_PHYRDY_CHG |PORT_IRQ_DEV_XCHG | +PORT_IRQ_UNK_FIS ,DEF_PORT_IRQ =PORT_IRQ_FREEZE | PORT_IRQ_COMPLETE | PORT_IRQ_ERROR,
@@ -770,15 +771,20 @@ static unsigned int sil24_eh_autopsy(str desc += rc; desc_sz -= rc; -if (irq_stat &PORT_IRQ_DEV_XCHG ) { +if (irq_stat & (PORT_IRQ_PHYRDY_CHG |PORT_IRQ_DEV_XCHG )) { err_mask |=AC_ERR_ATA_BUS ;/* sil24 doesn't recover very well from phy * disconnection with a softreset. Force hardreset. */ action |=ATA_PORT_HARDRESET ; - rc =scnprintf (desc, desc_sz,", device exchanged" ); + rc =scnprintf (desc, desc_sz,", %s" , + irq_stat &PORT_IRQ_PHYRDY_CHG ? +"PHY RDY changed" :"device exchanged" ); desc += rc; desc_sz -= rc; + +/* request probing */ +ata_schedule_probe (ap); }if (irq_stat &PORT_IRQ_UNK_FIS ) {
Signature · 1 line
1.2.4
Quoted 4 lines · Tejun Heo
Hellooooo, all.
Finally, the last one. This is the first take of add-hotplug-support patchset. This patchset includes 13 patches.
And, of course, 15 patches.
Quoted 3 lines
#01-04 implement hotplug framework
#05-07 implement warm plug (SCSI scan/delete callbacks)
#08-10 hook and activate hotplug
#11-12 prep sata_sil for hotplug support
#13-15 add LLDD hotplug supports (PHY status change notifications)
Signature · 1 line
tejun
Quoted 7 lines · Tejun Heo
Warm unplug/plug comes at almost no cost for LLDDs implementing new EH
- adding a call toata_eh_hotplug()at the end of ->error_handler is enough (even this is unnecessary if it's using bmdmaerror_handler). e.g.ata_piixisn't changed by hotplug patchsets at all but still supports warm plugging (even PATA warm plugging works although it's electronically unsafe).
Quoted 6 lines
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_silandsata_sil24are 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?
Thanks,
Forrest
Quoted 16 lines · forrest
Quoted 7 lines · Tejun Heo
Warm unplug/plug comes at almost no cost for LLDDs implementing new EH
- adding a call toata_eh_hotplug()at the end of ->error_handler is enough (even this is unnecessary if it's using bmdmaerror_handler). e.g.ata_piixisn't changed by hotplug patchsets at all but still supports warm plugging (even PATA warm plugging works although it's electronically unsafe).Quoted 6 lines
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_silandsata_sil24are converted this way and hotplugging works perfectly for those drivers.
Quoted 5 lines
In order to make
ata_eh_hotplug()called, LLDD have to schedule EH in its interrupt handler(i.e. invokescsi_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).
Signature · 1 line
tejun
Quoted 16 lines · Tejun Heo
Quoted 15 lines · forrest
Quoted 13 lines · Tejun Heo
Warm unplug/plug comes at almost no cost for LLDDs implementing new EH
- adding a call toata_eh_hotplug()at the end of ->error_handler is enough (even this is unnecessary if it's using bmdmaerror_handler). e.g.ata_piixisn'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_silandsata_sil24are converted this way and hotplugging works perfectly for those drivers.
Quoted 8 lines
Quoted 5 lines
In order to make
ata_eh_hotplug()called, LLDD have to schedule EH in its interrupt handler(i.e. invokescsi_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().
Jeff
Quoted 22 lines · Jeff Garzik
Quoted 21 lines · Tejun Heo
Quoted 20 lines · forrest
Quoted 7 lines · Tejun Heo
Warm unplug/plug comes at almost no cost for LLDDs implementing new EH
- adding a call toata_eh_hotplug()at the end of ->error_handler is enough (even this is unnecessary if it's using bmdmaerror_handler). e.g.ata_piixisn't changed by hotplug patchsets at all but still supports warm plugging (even PATA warm plugging works although it's electronically unsafe).Quoted 6 lines
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_silandsata_sil24are 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. invokescsi_eh_schedule_host()directly or indirectly), but I didn't find such code in your patch set. Did I miss anything?
Quoted 6 lines
Quoted 2 lines
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.
Signature · 1 line
tejun
Quoted 5 lines · Tejun Heo
Hellooooo, all.
Finally, the last one. This is the first take of add-hotplug-support patchset. This patchset includes 13 patches.
Quoted 8 lines
#01-04 implement hotplug framework
#05-07 implement warm plug (SCSI scan/delete callbacks)
#08-10 hook and activate hotplug
#11-13 add LLDD hotplug supports (PHY status change notifications)Hotplugging is implemented as an integral part of EH. A new EH helper
ata_eh_hotplug()is defined which should be called after all regular EH is done. LLDDs have to do very little to support hotplug.
I'm going to leave the substantive review of this patchset for the next time it gets resent.
Overall,
* my quick review didn't turn up any obvious problems, but that was just looking at each patch, not looking at the end result (big picture) at all.
* hotplug design I want to see is (also mentioned in another email):
1a) user requests bus scan
or
1b) hotplug interrupt
2) wait a bit, swallowing any 1a- or 1b-type events that appear during the wait (debounce).
3) rescan bus, revalidate device(s)
I'm careful to use "revalidate", because that covers all cases:
- existing device goes away
- new device appears
- existing device "blipped", but its still there, so we can keep talking to it.
Quoted 11 lines · Jeff Garzik
Quoted 10 lines · Tejun Heo
Hellooooo, all.
Finally, the last one. This is the first take of add-hotplug-support patchset. This patchset includes 13 patches.
#01-04 implement hotplug framework #05-07 implement warm plug (SCSI scan/delete callbacks) #08-10 hook and activate hotplug #11-13 add LLDD hotplug supports (PHY status change notifications)
Quoted 5 lines
Quoted 3 lines
Hotplugging is implemented as an integral part of EH. A new EH helper
ata_eh_hotplug()is defined which should be called after all regular EH is done. LLDDs have to do very little to support hotplug.I'm going to leave the substantive review of this patchset for the next time it gets resent.
Yeap, please don't spend too much time on it. Hotplug has been changed quite a bit in my current repo.
Quoted 4 lines
Overall,
* my quick review didn't turn up any obvious problems, but that was just looking at each patch, not looking at the end result (big picture) at all.
Quoted 8 lines
* hotplug design I want to see is (also mentioned in another email):
1a) user requests bus scan
or
1b) hotplug interrupt
2) wait a bit, swallowing any 1a- or 1b-type events that appear during the wait (debounce).
3) rescan bus, revalidate device(s)
In my working repo, hardware debouncing is done by invoking the following function in ->prereset() with the port frozen. I'm not very sure whether debouncing user request is necessary though.
/**
* sata_debounce - debounce SATA phy status
* @link: ATA link to debounce SATA phy status for
* @interval_msec: polling interval in millisecs
* @duration_msec: debounce duration in millisecs
* @timeout_msec: timeout in millisecs
*
* Make sure SStatus of @link reaches stable state, determined by
* holding the same value where DET is not 1 for @duration_msec
* polled every @interval_msec, before @timeout_msec. Note the
* timeout constraints the beginning of the stable state.
*
* LOCKING:
* Kernel thread context (may sleep)
*
* RETURNS:
* 0 on success, -errno on failure.
*/
int sata_debounce(struct ata_link *link, unsigned long interval_msec,
unsigned long duration_msec, unsigned long timeout_msec)
{
unsigned long duration = duration_msec * HZ / 1000;
unsigned long timeout = jiffies + timeout_msec * HZ / 1000;
unsigned long last_jiffies;
u32 last, cur;
int rc;
if ((rc = ata_scr_read(link, SCR_STATUS, &cur)))
return rc;
cur &= 0xf;
last = cur;
last_jiffies = jiffies;
while (1) {
msleep(interval_msec);
if ((rc = ata_scr_read(link, SCR_STATUS, &cur)))
return rc;
cur &= 0xf;
/* DET stable? */
if (cur == last && cur != 1) {
if (time_after(jiffies, last_jiffies + duration))
return 0;
continue;
}
/* unstable, start over */
last = cur;
last_jiffies = jiffies;
/* check timeout */
if (time_after(jiffies, timeout))
return -EBUSY;
}
}Quoted 7 lines
I'm careful to use "revalidate", because that covers all cases:
- existing device goes away
- new device appears
- existing device "blipped", but its still there, so we can keep talking to it.
Yeap, all bases covered.
I'm currently finishing up PM support. It took a lot longer than I though but it's shaping up pretty good. Everything is handled nicely, hotplug, EH, qc deferring (e.g. not issuing ATAPI command if commands are outstanding to more than three devices for sil24...) are all handled in generic and unified way. Adding PM support necessitated quite a bit of changes to EH and hotplug. Currently, major changes in my repo are...
- boot scan, hotplug, EH all rolled up into single EH revive operation.
- simpler EH/irq synchronization. EH now works on its own copy of EH info created on entry to EH.
- much tighter event handling (almost no EH/hotplug event/info loss except for pathological cases)
- fine-grained user scan request (user can request scan of specific device)
- ata_link abstraction for PM
- PM support with the same level of EH/NCQ/hotplug support as host ports (sil24 and working on AHCI)
Above list is what comes to my mind ATM. I probably have forgotten a lot. I'll make a full list when I post the next round of patches.
Jeff, until when are you available? I think I can post the next round in a few days (I'm pretty sure this time :). I'm thinking of setting up a git repo and merge irq-pio there too in the order you requested. If schedule isn't too tight, it would be nice to push this thing to some branch in libata-dev.
Thanks.
Signature · 1 line
tejun
Quoted 1 line · Tejun Heo
In my working repo, hardware debouncing is done by invoking the following function in ->prereset() with the port frozen. I'm not very sure whether debouncing user request is necessary though.
Agreed, I'm not sure either.
Quoted 48 lines
/**
* sata_debounce - debounce SATA phy status * @link: ATA link to debounce SATA phy status for * @interval_msec: polling interval in millisecs * @duration_msec: debounce duration in millisecs * @timeout_msec: timeout in millisecs * * Make sure SStatus of @link reaches stable state, determined by * holding the same value where DET is not 1 for @duration_msec * polled every @interval_msec, before @timeout_msec. Note the * timeout constraints the beginning of the stable state.*
* LOCKING:
* Kernel thread context (may sleep)
*
* RETURNS:
* 0 on success, -errno on failure.
*/
int sata_debounce(structata_link*link, unsigned longinterval_msec,
unsigned longduration_msec, unsigned longtimeout_msec){ unsigned long duration = duration_msec * HZ / 1000; unsigned long timeout = jiffies + timeout_msec * HZ / 1000; unsigned long last_jiffies; u32 last, cur; int rc; if ((rc = ata_scr_read(link, SCR_STATUS, &cur))) return rc; cur &= 0xf; last = cur; last_jiffies = jiffies; while (1) { msleep(interval_msec); if ((rc = ata_scr_read(link, SCR_STATUS, &cur))) return rc; cur &= 0xf; /* DET stable? */ if (cur == last && cur != 1) { if (time_after(jiffies, last_jiffies + duration)) return 0; continue; } /* unstable, start over */
Quoted 8 lines
last = cur;
last_jiffies = jiffies;
/* check timeout */
if (time_after(jiffies, timeout))
return -EBUSY;
}
}hmmm, I would think something more along the lines of
get HP irq
ack HP irq ata_I_got_hotplug_event()
if test_and_clear_bit(got_hotplug)
start 1-second timer
timer fires...
clear got_hotplug
handle hotplug, revalidate port
There's not much point in polling, the reason for the debounce period is to throw away spurious hotplug/unplug/hotplug events the hardware throws while it is figuring shit out.
Should just need a pause, following by port recovery/revalidate.
Quoted 12 lines
Quoted 7 lines
I'm careful to use "revalidate", because that covers all cases:
- existing device goes away
- new device appears
- existing device "blipped", but its still there, so we can keep talking to it.Yeap, all bases covered.
I'm currently finishing up PM support. It took a lot longer than I though but it's shaping up pretty good. Everything is handled nicely, hotplug, EH, qc deferring (e.g. not issuing ATAPI command if commands are outstanding to more than three devices for sil24...) are all handled in generic and unified way. Adding PM support necessitated quite a bit of changes to EH and hotplug. Currently, major changes in my repo are...
Quoted 7 lines
- boot scan, hotplug, EH all rolled up into single EH revive operation.
- simpler EH/irq synchronization. EH now works on its own copy of EH info created on entry to EH.
- much tighter event handling (almost no EH/hotplug event/info loss except for pathological cases)
- fine-grained user scan request (user can request scan of specific device)
-ata_linkabstraction for PM
that's nice
Quoted 2 lines
- PM support with the same level of EH/NCQ/hotplug support as host ports (sil24 and working on AHCI)
what kind of PM are you testing on?
Quoted 3 lines
Above list is what comes to my mind ATM. I probably have forgotten a lot. I'll make a full list when I post the next round of patches.
Jeff, until when are you available? I think I can post the next round in a few days (I'm pretty sure this time :). I'm thinking of setting up a git repo and merge irq-pio there too in the order you requested. If schedule isn't too tight, it would be nice to push this thing to some branch in libata-dev.
I leave May 3rd. So sometime between now and then. The goal should be to get #irq-pio and whatever other work you want into #upstream before I leave, so that people have a nice long period for testing in -mm. irq-pio will definitely want some testing, as will your work. Its a lot to throw at people all at once.
Jeff
Quoted 8 lines · Jeff Garzik
Quoted 1 line · Tejun Heo
In my working repo, hardware debouncing is done by invoking the following function in ->prereset() with the port frozen. I'm not very sure whether debouncing user request is necessary though.
Agreed, I'm not sure either.
Quoted 2 lines
/**
*sata_debounce- debounce SATA phy status
[--snip--]
Quoted 11 lines
Quoted 2 lines
} }hmmm, I would think something more along the lines of
get HP irq
ack HP irq
ata_I_got_hotplug_event()
iftest_and_clear_bit(got_hotplug)
start 1-second timer
Quoted 8 lines
timer fires...
cleargot_hotplug
handle hotplug, revalidate portThere's not much point in polling, the reason for the debounce period is to throw away spurious hotplug/unplug/hotplug events the hardware throws while it is figuring shit out.
Should just need a pause, following by port recovery/revalidate.
The thing is that on detection of hotplug event, all the active qcs should be aborted and EH should be entered anyway. The current working tree is much nicer to hardware compared to the previous posting. It freezes the port only when really necessary.
I think I need to explain more about the whole freezing/polling thing.
I once participated in a static discharge / power fluctation test for SATA. Although the serial link is convenient, pretty and definitely the way to go, it gets easily interfered by electromagnetic fluctations. Also, because the communication is packet based necessitating relatively complex state machine inside the link layer, controllers implementing such interfaces seem to exhibit various convoluted behaviors when put under stress.
I could see several different types of screaming interrupt lockups in the period of a few hours. And we all know screaming interrupt is a scary thing. This is the reason behind the whole 'freezing' stuff.
What I tried to achieve is to isolate the controller if things start to go south so that the machine as a whole isn't affected. Fortunately, most such conditions are recoverable by some form of resetting and we can determine if the controller is acting sanely by watching how the reset goes. If it seems okay, we turn it back on. If it fails to come back after several retries, we leave the controller frozen such that the rest of the machine can function.
phy status change is a dangerous event. I'm pretty sure most of electromagnetic interferences would trigger the event too. Also, it's not like we can do anything other than resetting and recovering the device after such an event. So, in new EH, phy status change is a freezing event.
As soon as such an event is detected, libata assumes the controller is lost and freezes it. EH kicks in immediately and performs reset and, by doing so, it makes sure that the controller isn't trying to eat the machine alive. Only after EH is sure that the controller is acting sanely, it thaws the port and revalidates the attached device.
So, in the above control flow, it's natural and even necessary to perform debouncing by polling. So, the implementation. It also has the advantage of being generic. As most controllers generate phy related interrupts from SCR updates, they can use generic stat_debounce() instead of writing its own irq debouncing routine.
One more thing to note is that such debouncing is needed before and during several stages of resets anyway. The current upstream code does this from sata_phy_resume() by waiting until DET assumes some other value than 1. Unfortunately, this doesn't work for some controllers (sil24) as DET dances together with other bits. So, we need better debouncing routine anyway, and as now all resets are probing resets, we need to debounce prior to every reset. And, debouncing in ->prereset() can satisfy all the requirements.
Oh.. and this has been on my mind for some time now. It would be nice if we can set up a project to certify controllers and drivers which pass certain set of standard static discharge / power fluctation tests. With new EH, we have the framework but I'm pretty sure a lot of drivers would need some special case code to cope with such tests. Equipments to perform such tests are not cheap, but it would be very helpful to a lot of people, especially server crowd and people trying to use Linux on consumer products. Those static discharges and power fluctations are facts of life. They do occur. And, ATM, we're not dealing with it very well. I hope we can persuade some companies to sponsor such a project.
Quoted 22 lines
Quoted 13 lines
Quoted 8 lines
I'm careful to use "revalidate", because that covers all cases:
- existing device goes away
- new device appears
- existing device "blipped", but its still there, so we can keep talking to it.Yeap, all bases covered.
I'm currently finishing up PM support. It took a lot longer than I though but it's shaping up pretty good. Everything is handled nicely, hotplug, EH, qc deferring (e.g. not issuing ATAPI command if commands are outstanding to more than three devices for sil24...) are all handled in generic and unified way. Adding PM support necessitated quite a bit of changes to EH and hotplug. Currently, major changes in my repo are...
Quoted 8 lines
- boot scan, hotplug, EH all rolled up into single EH revive operation.
- simpler EH/irq synchronization. EH now works on its own copy of EH info created on entry to EH.
- much tighter event handling (almost no EH/hotplug event/info loss except for pathological cases)
- fine-grained user scan request (user can request scan of specific device)
-ata_linkabstraction for PM
Quoted 8 lines
that's nice
Quoted 2 lines
- PM support with the same level of EH/NCQ/hotplug support as host ports (sil24 and working on AHCI)
what kind of PM are you testing on?
sil4726. Silicon Image was kind enough to send a sample board to me. :)
Quoted 7 lines
Quoted 4 lines
Above list is what comes to my mind ATM. I probably have forgotten a lot. I'll make a full list when I post the next round of patches.
Jeff, until when are you available? I think I can post the next round in a few days (I'm pretty sure this time :). I'm thinking of setting up a git repo and merge irq-pio there too in the order you requested. If schedule isn't too tight, it would be nice to push this thing to some branch in libata-dev.
I leave May 3rd. So sometime between now and then. The goal should be to get #irq-pio and whatever other work you want into #upstream before I leave, so that people have a nice long period for testing in -mm. irq-pio will definitely want some testing, as will your work. Its a lot to throw at people all at once.
I think/hope I can pull something off in that time frame. And, yeah, it's a LOT to throw at people and definitely needs a lot of testing.
Thanks.
Signature · 1 line
tejun