Finn,
Index: linux/drivers/scsi/atari_NCR5380.c
===================================================================
--- linux.orig/drivers/scsi/atari_NCR5380.c 2014-10-27 16:25:36.000000000
+1100
+++ linux/drivers/scsi/atari_NCR5380.c 2014-10-27 16:25:45.000000000
+1100
@@ -879,10 +879,10 @@ static void NCR5380_exit(struct Scsi_Hos
*
*/
-static int NCR5380_queue_command_lck(struct scsi_cmnd *cmd,
- void (*done)(struct scsi_cmnd *))
+static int NCR5380_queue_command(struct Scsi_Host *instance,
+ struct scsi_cmnd *cmd)
{
- SETUP_HOSTDATA(cmd->device->host);
+ struct NCR5380_hostdata *hostdata = shost_priv(instance);
struct scsi_cmnd *tmp;
unsigned long flags;
Nitpick - why did this change set go into this particular patch? Because you
are converting from NCR5380_queue_command_lck to NCR5380_queue_command?
That's right.
OK, I just wanted to understand why it's being combined with the stram.c
patch (and the directly resulting changes in the SCSI driver).
The change to NCR5380_queue_command() can only now be done, as a result
of the deadlock fixes in this patch, so it follows logically. Makes
perfectly good sense to me now, thanks.
@@ -893,7 +893,7 @@ static int NCR5380_queue_command_lck(str
printk(KERN_NOTICE "scsi%d: WRITE attempted with NO_WRITE debugging flag
set\n",
H_NO(cmd));
cmd->result = (DID_ERROR << 16);
- done(cmd);
+ cmd->scsi_done(cmd);
return 0;
}
#endif /* (NDEBUG & NDEBUG_NO_WRITE) */
@@ -904,8 +904,6 @@ static int NCR5380_queue_command_lck(str
*/
SET_NEXT(cmd, NULL);
- cmd->scsi_done = done;
-
cmd->result = 0;
/*
Ditto for these two.
Again, it follows from the differences in the formal parameters between
NCR5380_queue_command_lck() and NCR5380_queue_command().
Yep, I got that bit. Thanks for the explanation, and apologies for the
noise.
@@ -915,7 +913,6 @@ static int NCR5380_queue_command_lck(str
* sense data is only guaranteed to be valid while the condition exists.
*/
- local_irq_save(flags);
/* ++guenther: now that the issue queue is being set up, we can lock
ST-DMA.
* Otherwise a running NCR5380_main may steal the lock.
* Lock before actually inserting due to fairness reasons explained in
@@ -928,11 +925,13 @@ static int NCR5380_queue_command_lck(str
* because also a timer int can trigger an abort or reset, which would
* alter queues and touch the lock.
*/
- if (!IS_A_TT()) {
- /* perhaps stop command timer here */
- falcon_get_lock();
- /* perhaps restart command timer here */
- }
+ /* perhaps stop command timer here */
+ if (!falcon_get_lock())
+ return SCSI_MLQUEUE_HOST_BUSY;
+ /* perhaps restart command timer here */
+
The comments about stopping and restarting the command timer can be
removed. In 2.4 kernels, the driver would tweak the timers and wait on
the lock unconditionally, Can't ne done anymore, for so many reasons.
Well, you sent an acked-by for this patch, so I'm a bit confused. Do you
want me to re-spin it?
Not at all - this was just something I wanted to raise for future
patches, before it slips my mind, again. Sorry I did not make that clear
at all.
I expect we will have an opportunity to address this when we embark on a
wider cleanup of comments throughout the driver - most of these comments
were for the 0.9 kernel series and might still be somewhat germane up to
2.4. Might be totally misleading now.
Not a priority at all - I know you have bigger fish to fry. I should not
have brought it up in this context.
No problems at all with this patch (or indeed with the rest of the series).
Cheers,
Michael
--
To unsubscribe from this list: 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
