On 10/18/23 20:20, Bart Van Assche wrote:
On 10/16/23 05:15, Hannes Reinecke wrote:
diff --git a/Documentation/scsi/scsi_mid_low_api.rst
b/Documentation/scsi/scsi_mid_low_api.rst
index 022198c51350..85b1384ba4fd 100644
--- a/Documentation/scsi/scsi_mid_low_api.rst
+++ b/Documentation/scsi/scsi_mid_low_api.rst
@@ -777,7 +777,7 @@ Details::
/**
* eh_host_reset_handler - reset host (host bus adapter)
- * @scp: SCSI host that contains this device should be reset
+ * @shp: SCSI host that contains this device should be reset
Please change this into "@shp: SCSI host that should be reset".
Ok.
/* If we can't locate the host to reset, then we failed. */
- if ((hd = shost_priv(SCpnt->device->host)) == NULL){
- printk(KERN_ERR MYNAM ": host reset: "
- "Can't locate host! (sc=%p)\n", SCpnt);
+ if ((hd = shost_priv(sh)) == NULL){
+ printk(KERN_ERR MYNAM ": host reset: Can't locate host!\n");
return FAILED;
}
Please move the assignment out of the if-statement since the
if-statement has to be modified anyway.
I've removed the entire 'if' clause as 'shost_priv()' really cannot be
NULL here.
- printk(MYIOC_s_INFO_FMT "host reset: %s (sc=%p)\n",
- ioc->name, ((retval == 0) ? "SUCCESS" : "FAILED" ), SCpnt);
+ printk(MYIOC_s_INFO_FMT "host reset: %s\n",
+ ioc->name, ((retval == 0) ? "SUCCESS" : "FAILED" ));
Please consider removing the superfluous parentheses from the modified
code.
Ok.
hostdata->eh_complete = NULL;
- /* Revalidate the transport parameters of the failing device */
- if(hostdata->fast)
- spi_schedule_dv_device(SCp->device);
-
- spin_unlock_irq(SCp->device->host->host_lock);
+ /* Revalidate the transport parameters for attached devices */
+ if(hostdata->fast) {
+ struct scsi_device *sdev;
+ __shost_for_each_device(sdev, host)
+ spi_schedule_dv_device(sdev);
+ }
+ spin_unlock_irq(host->host_lock);
return SUCCESS;
Since the above changes behavior of the SPI transport, shouldn't this be
mentioned in the patch description?
Hmm. It doesn't change the behaviour for SPI transport, just for the
53c700 driver. But I'll check what the other SPI drivers are doing;
I would have thought that a host reset would also reset the SPI timings,
and as such all devices would have to be revalidated. If not then
it begs the question why we need to reset only SPI timings for the
device of the first command; there might be several commands failing
referring to different devices.
And if we have to escalate to host reset we would revalidate only the
SPI timings for the first device, with all the other failing devices
still running on their original timing.
Which really makes no sense.
diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index 72ceaf650b0d..9be45b7a2571 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -2852,21 +2852,14 @@ static bool blogic_write_outbox(struct
blogic_adapter *adapter,
/* Error Handling (EH) support */
-static int blogic_hostreset(struct scsi_cmnd *SCpnt)
+static int blogic_hostreset(struct Scsi_Host *shost)
{
- struct blogic_adapter *adapter =
- (struct blogic_adapter *) SCpnt->device->host->hostdata;
-
- unsigned int id = SCpnt->device->id;
- struct blogic_tgt_stats *stats = &adapter->tgt_stats[id];
+ struct blogic_adapter *adapter = shost_priv(shost);
int rc;
- spin_lock_irq(SCpnt->device->host->host_lock);
-
- blogic_inc_count(&stats->adapter_reset_req);
-
+ spin_lock_irq(shost->host_lock);
rc = blogic_resetadapter(adapter, false);
- spin_unlock_irq(SCpnt->device->host->host_lock);
+ spin_unlock_irq(shost->host_lock);
return rc;
}
Why has the blogic_inc_count() call been left out?
Good question. Will be fixing it.
Cheers,
Hannes
