[PATCH] clean up some more tmscsim scan logic

[PATCH] clean up some more tmscsim scan logic

[Christoph Hellwig]

 - cleanup checks in ->queuecommand, we only get here either for
   inquiry or a found device now
 - DCBmap in the acb is gone, was only used for debug prints after
   the prevous changes
 - kill some more debugging keyed of by ->scan_devices


--- 1.19/drivers/scsi/scsiiom.c	2004-07-05 00:25:48 +02:00
+++ edited/drivers/scsi/scsiiom.c	2004-08-17 19:56:00 +02:00
@@ -1398,26 +1398,7 @@
 	pSRB->SRBFlag &= ~AUTO_REQSENSE;
 	pSRB->AdaptStatus = 0;
 	pSRB->TargetStatus = CHECK_CONDITION << 1;
-#ifdef DC390_REMOVABLEDEBUG
-	switch (pcmd->sense_buffer[2] & 0x0f)
-	{	    
-	 case NOT_READY: printk (KERN_INFO "DC390: ReqSense: NOT_READY (Cmnd = 0x%02x, Dev = %i-%i, Stat = %i, Scan = %i)\n",
-				 pcmd->cmnd[0], pDCB->TargetID, pDCB->TargetLUN,
-				 status, pACB->scan_devices); break;
-	 case UNIT_ATTENTION: printk (KERN_INFO "DC390: ReqSense: UNIT_ATTENTION (Cmnd = 0x%02x, Dev = %i-%i, Stat = %i, Scan = %i)\n",
-				      pcmd->cmnd[0], pDCB->TargetID, pDCB->TargetLUN,
-				      status, pACB->scan_devices); break;
-	 case ILLEGAL_REQUEST: printk (KERN_INFO "DC390: ReqSense: ILLEGAL_REQUEST (Cmnd = 0x%02x, Dev = %i-%i, Stat = %i, Scan = %i)\n",
-				       pcmd->cmnd[0], pDCB->TargetID, pDCB->TargetLUN,
-				       status, pACB->scan_devices); break;
-	 case MEDIUM_ERROR: printk (KERN_INFO "DC390: ReqSense: MEDIUM_ERROR (Cmnd = 0x%02x, Dev = %i-%i, Stat = %i, Scan = %i)\n",
-				    pcmd->cmnd[0], pDCB->TargetID, pDCB->TargetLUN,
-				    status, pACB->scan_devices); break;
-	 case HARDWARE_ERROR: printk (KERN_INFO "DC390: ReqSense: HARDWARE_ERROR (Cmnd = 0x%02x, Dev = %i-%i, Stat = %i, Scan = %i)\n",
-				      pcmd->cmnd[0], pDCB->TargetID, pDCB->TargetLUN,
-				      status, pACB->scan_devices); break;
-	}
-#endif
+
 	//pcmd->result = MK_RES(DRIVER_SENSE,DID_OK,0,status);
 	if (status == (CHECK_CONDITION << 1))
 	{
@@ -1571,23 +1552,6 @@
 	    pDCB->Inquiry7 = ptr->Flags;
 
 ckc_e:
-    if( pACB->scan_devices )
-    {
-	if( pcmd->cmnd[0] == TEST_UNIT_READY ||
-	    pcmd->cmnd[0] == INQUIRY)
-	{
-#ifdef DC390_DEBUG0
-	    printk (KERN_INFO "DC390: %s: result: %08x", 
-		    (pcmd->cmnd[0] == INQUIRY? "INQUIRY": "TEST_UNIT_READY"),
-		    pcmd->result);
-	    if (pcmd->result & (DRIVER_SENSE << 24)) printk (" (sense: %02x %02x %02x %02x)\n",
-				   pcmd->sense_buffer[0], pcmd->sense_buffer[1],
-				   pcmd->sense_buffer[2], pcmd->sense_buffer[3]);
-	    else printk ("\n");
-#endif
-	}
-    }
-
     if( pcmd->cmnd[0] == INQUIRY && 
 	(pcmd->result == (DID_OK << 16) || status_byte(pcmd->result) & CHECK_CONDITION) )
      {
--- 1.45/drivers/scsi/tmscsim.c	2004-07-05 00:35:28 +02:00
+++ edited/drivers/scsi/tmscsim.c	2004-08-17 20:05:52 +02:00
@@ -612,11 +609,7 @@
      {
 	pDCB = pDCB->pNextDCB;
 	if (pDCB == pACB->pLinkDCB)
-	  {
-	     DCBDEBUG(printk (KERN_WARNING "DC390: DCB not found (DCB=%p, DCBmap[%2x]=%2x)\n",
-			      pDCB, id, pACB->DCBmap[id]));
 	     return 0;
-	  }
      }
    DCBDEBUG1( printk (KERN_DEBUG "DCB %p (%02x,%02x) found.\n",	\
 		      pDCB, pDCB->TargetID, pDCB->TargetLUN));
@@ -984,28 +977,6 @@
     struct dc390_srb*   pSRB;
     struct dc390_acb*   pACB = (struct dc390_acb*) cmd->device->host->hostdata;
 
-    DEBUG0(/*  if(pACB->scan_devices) */	\
-	printk(KERN_INFO "DC390: Queue Cmd=%02x,Tgt=%d,LUN=%d (pid=%li), buffer=%p\n",\
-	       cmd->cmnd[0],cmd->device->id,cmd->device->lun,cmd->pid, cmd->buffer));
-
-    /* TODO: Change the policy: Always accept TEST_UNIT_READY or INQUIRY 
-     * commands and alloc a DCB for the device if not yet there. DCB will
-     * be removed in dc390_SRBdone if SEL_TIMEOUT */
-    if (!(pACB->scan_devices) && !(pACB->DCBmap[cmd->device->id] & (1 << cmd->device->lun))) {
-	printk(KERN_INFO "DC390: Ignore target %02x lun %02x\n",
-		cmd->device->id, cmd->device->lun); 
-	goto fail;
-    }
-
-    /* Should it be: BUG_ON(!pDCB); ? */
-
-    if (!pDCB)
-    {  /* should never happen */
-	printk (KERN_ERR "DC390: no DCB found, target %02x lun %02x\n", 
-		cmd->device->id, cmd->device->lun);
-	goto fail;
-    }
-
     pACB->Cmds++;
     cmd->scsi_done = done;
     cmd->result = 0;
@@ -1026,10 +997,6 @@
 
  requeue:
     return 1;
- fail:
-    cmd->result = DID_BAD_TARGET << 16;
-    done(cmd);
-    return 0;
 }
 
 /* We ignore mapping problems, as we expect everybody to respect 
@@ -1394,7 +1361,6 @@
 static void __devinit dc390_initACB (struct Scsi_Host *psh, unsigned long io_port, u8 Irq, u8 index)
 {
     struct dc390_acb*    pACB;
-    u8   i;
 
     psh->can_queue = MAX_CMD_QUEUE;
     psh->cmd_per_lun = MAX_CMD_PER_LUN;
@@ -1441,8 +1407,6 @@
     dc390_linkSRB( pACB );
     pACB->pTmpSRB = &pACB->TmpSRB;
     dc390_initSRB( pACB->pTmpSRB );
-    for(i=0; i<MAX_SCSI_ID; i++)
-	pACB->DCBmap[i] = 0;
     pACB->sel_timeout = SEL_TIMEOUT;
     pACB->glitch_cfg = EATER_25NS;
     pACB->Cmds = pACB->CmdInQ = pACB->CmdOutOfSRB = 0;
@@ -1603,7 +1567,6 @@
 			pDCB->CtrlR4 |= NEGATE_REQACKDATA | NEGATE_REQACK;
 	}
 
-	pACB->DCBmap[id] |= (1 << lun);
 	dc390_updateDCB(pACB, pDCB);
 
 	pACB->scan_devices = 1;
@@ -1627,8 +1590,6 @@
 
 	BUG_ON(pDCB->GoingSRBCnt > 1);
 	
-	pACB->DCBmap[pDCB->TargetID] &= ~(1 << pDCB->TargetLUN);
-   
 	if (pDCB == pACB->pLinkDCB) {
 		if (pACB->pLastDCB == pDCB) {
 			pDCB->pNextDCB = NULL;
:q	
--- 1.8/drivers/scsi/tmscsim.h	2004-06-24 23:33:43 +02:00
+++ edited/drivers/scsi/tmscsim.h	2004-08-17 19:56:00 +02:00
@@ -163,7 +163,6 @@
 struct dc390_srb	*pTmpSRB;
 
 u8		msgin123[4];
-u8		DCBmap[MAX_SCSI_ID];
 u8		Connected;
 u8		pad;
 

Re: [PATCH] clean up some more tmscsim scan logic

[Guennadi Liakhovetski]

On Tue, 17 Aug 2004, Christoph Hellwig wrote:

> - cleanup checks in ->queuecommand, we only get here either for
>   inquiry or a found device now
> - DCBmap in the acb is gone, was only used for debug prints after
>   the prevous changes
> - kill some more debugging keyed of by ->scan_devices

Thanks for the patch!

Christoph, when I have a couple of free minutes, I am trying to look at 
the driver to see what else could one through away there:-), or what to 
improve / break... I've got a couple of points, I am trying to implement, 
but I am not sure if at all I am moving in the right direction, besides, 
maybe you also have them on your todo. So, to avoid effort duplication, 
maybe you could help me with my doubts, or tell me if you've already 
implemented it:

Currently SCSI drivers (tmscsim included) implement multiplecommand 
queues. Which, in part, makes sense, but the generic code changed, and not 
all drivers nave been converted to fully use it promptly. So, as a 
maximum, I saw (maybe you could add a couple:-)):

1) Queue in the block-layer
2) Incoming queue in a scsi LLD (recently removed in tmscsim)
3) Issue queue in LLD (Waiting in tmscsim)
4) Submitted tagged command queue or untagged 1 current command in LLD 
with a timer (Going in tmscsim)
5) tcq in SCSI mid-layer / block.

So, I can't figure out, nor find a sample implementation or a document 
about which queues should really be implemented in LLDs and how they 
should inter-operate with the mid- / block-layer. Say, a document, 
describing the (SCSI-)command flow would be very useful, like

...
n)   scsi.c: LLD->queuecommand()
n+1) LLD:    send out on the bus, insert on tcq / return
      SCSI_MLQUEUE_DEVICE_BUSY
n+2) LLD:    on command-completion, if successful, report (scsi_done()),
      if not - auto request sense, put the request-sense command on
      internal "going" queue
...

Where n+2 is, actually, the second question - I've seen a comment 
somewhere, that LLDs should (not?) auto request-sense, rather than failing 
and letting the mid-layer issue one...

Is there a "perfect" LLD, I could look at for an example? I looked at 
53c700.c, as it was until recently the only one using tcq, but it also has 
a few of those internal queues...

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: [PATCH] clean up some more tmscsim scan logic

[Christoph Hellwig]

On Tue, Aug 17, 2004 at 10:17:14PM +0200, Guennadi Liakhovetski wrote:
> Currently SCSI drivers (tmscsim included) implement multiplecommand 
> queues. Which, in part, makes sense, but the generic code changed, and not 
> all drivers nave been converted to fully use it promptly. So, as a 
> maximum, I saw (maybe you could add a couple:-)):
> 
> 1) Queue in the block-layer
> 2) Incoming queue in a scsi LLD (recently removed in tmscsim)
> 3) Issue queue in LLD (Waiting in tmscsim)
> 4) Submitted tagged command queue or untagged 1 current command in LLD 
> with a timer (Going in tmscsim)
> 5) tcq in SCSI mid-layer / block.
> 
> So, I can't figure out, nor find a sample implementation or a document 
> about which queues should really be implemented in LLDs and how they 
> should inter-operate with the mid- / block-layer. Say, a document, 
> describing the (SCSI-)command flow would be very useful, like

None should be in the LLDD.  I have an experimental patch that doesn't
remove the per-device queue yet but only does the easier part of
returning MLQUEUE_DEVICE_BUSY in ->queuecommand but doesn't do the
requeueing from the isr yet.  I'll send it out ASAP.

> n)   scsi.c: LLD->queuecommand()
> n+1) LLD:    send out on the bus, insert on tcq / return
>      SCSI_MLQUEUE_DEVICE_BUSY
> n+2) LLD:    on command-completion, if successful, report (scsi_done()),
>      if not - auto request sense, put the request-sense command on
>      internal "going" queue
> ...
> 
> Where n+2 is, actually, the second question - I've seen a comment 
> somewhere, that LLDs should (not?) auto request-sense, rather than failing 
> and letting the mid-layer issue one...

No, mid-layer auto request sense is deprecated and isn't that reliable.
See the comment ontop of scsi_eh_get_sense() in scsi_error.c

> Is there a "perfect" LLD, I could look at for an example? I looked at 
> 53c700.c, as it was until recently the only one using tcq, but it also has 
> a few of those internal queues...

that driver queues commands in HBA memory, not in the driver.  It's one
of the NCR scripts family HBAs where the firmware is written specificly
to match the driver and they cooperate closely.  (better ask James on
the details..)

Re: [PATCH] clean up some more tmscsim scan logic

[Guennadi Liakhovetski]

Hi

On Thu, 19 Aug 2004, Christoph Hellwig wrote:

> On Tue, Aug 17, 2004 at 10:17:14PM +0200, Guennadi Liakhovetski wrote:
>> Currently SCSI drivers (tmscsim included) implement multiplecommand
>> queues. Which, in part, makes sense, but the generic code changed, and not
>> all drivers nave been converted to fully use it promptly. So, as a
>> maximum, I saw (maybe you could add a couple:-)):
>>
>> 1) Queue in the block-layer
>> 2) Incoming queue in a scsi LLD (recently removed in tmscsim)
>> 3) Issue queue in LLD (Waiting in tmscsim)
>> 4) Submitted tagged command queue or untagged 1 current command in LLD
>> with a timer (Going in tmscsim)
>> 5) tcq in SCSI mid-layer / block.
>>
>> So, I can't figure out, nor find a sample implementation or a document
>> about which queues should really be implemented in LLDs and how they
>> should inter-operate with the mid- / block-layer. Say, a document,
>> describing the (SCSI-)command flow would be very useful, like
>
> None should be in the LLDD.  I have an experimental patch that doesn't
> remove the per-device queue yet but only does the easier part of
> returning MLQUEUE_DEVICE_BUSY in ->queuecommand but doesn't do the
> requeueing from the isr yet.  I'll send it out ASAP.

Looks good, thanks! Compiles too. I'll test it on the hardware tomorrow, 
should work, still, I'd do a short test. And, yes, please, feel free to 
offload any part of that further cleanup on me - I would just need a 
couple of hints. I'll do it slower and you'll have to fix it afterwards 
and consult me before, but I want to have my piece of fun too!:-)

>> n)   scsi.c: LLD->queuecommand()
>> n+1) LLD:    send out on the bus, insert on tcq / return
>>      SCSI_MLQUEUE_DEVICE_BUSY
>> n+2) LLD:    on command-completion, if successful, report (scsi_done()),
>>      if not - auto request sense, put the request-sense command on
>>      internal "going" queue
>> ...
>>
>> Where n+2 is, actually, the second question - I've seen a comment
>> somewhere, that LLDs should (not?) auto request-sense, rather than failing
>> and letting the mid-layer issue one...
>
> No, mid-layer auto request sense is deprecated and isn't that reliable.
> See the comment ontop of scsi_eh_get_sense() in scsi_error.c

That's actually what I was reading and that's where I didn't understand:

  * Notes:
  *    This has the unfortunate side effect that if a shost adapter does
  *    not automatically request sense information, that we end up shutting
  *    it down before we request it.  All shosts should be doing this
 								^^^^
  *    anyways, so for now all I have to say is tough noogies if you end up
  *    in here.  On second thought, this is probably a good idea.  We
  *    *really* want to give authors an incentive to automatically request
  *    this.

So, I failed to understand if "this" above means "All shosts should be 
automatically requesting sense" or the opposite. Then the "second 
thought"... So, you mean LLDs should really auto-request sense on 
failures. Well, if we detect an error condition when we are CONNECTED to a 
host, it should be possible to directly send a REQUEST SENSE - the bus 
should be ours at this time, right?

>> Is there a "perfect" LLD, I could look at for an example? I looked at
>> 53c700.c, as it was until recently the only one using tcq, but it also has
>> a few of those internal queues...
>
> that driver queues commands in HBA memory, not in the driver.  It's one
> of the NCR scripts family HBAs where the firmware is written specificly
> to match the driver and they cooperate closely.  (better ask James on
> the details..)

Ok, I see (roughly), thanks.

Many thanks
Guennadi
---
Guennadi Liakhovetski

Re: [PATCH] clean up some more tmscsim scan logic

[Luben Tuikov]

The device server should send sense information when the task
finishes with CHECK CONDITION status _with_ the same transaction.
(SAM-3r13, 5.9.6, SPC-3r18, 4.5.1).  This used to be called
"autosense" a couple of years back, but no more as it is now
protocol.

This has been off-loaded to LLDD when the device doesn't support
it, so that SCSI Core is basically SAM.

BTW, the state and task queue would most likely change between
the time SCSI Core gets CHECK CONDITION (with no sense) until
the time it issues REQUEST SENSE, to get the sense for _that_
command which failed.  Thus we want LLDDs to emulate it for
devices which do not support it, since it has better (closer)
control of the device.


Guennadi Liakhovetski wrote:
> Hi
> 
> On Thu, 19 Aug 2004, Christoph Hellwig wrote:
> 
>  > On Tue, Aug 17, 2004 at 10:17:14PM +0200, Guennadi Liakhovetski wrote:
>  >> Currently SCSI drivers (tmscsim included) implement multiplecommand
>  >> queues. Which, in part, makes sense, but the generic code changed, 
> and not
>  >> all drivers nave been converted to fully use it promptly. So, as a
>  >> maximum, I saw (maybe you could add a couple:-)):
>  >>
>  >> 1) Queue in the block-layer
>  >> 2) Incoming queue in a scsi LLD (recently removed in tmscsim)
>  >> 3) Issue queue in LLD (Waiting in tmscsim)
>  >> 4) Submitted tagged command queue or untagged 1 current command in LLD
>  >> with a timer (Going in tmscsim)
>  >> 5) tcq in SCSI mid-layer / block.
>  >>
>  >> So, I can't figure out, nor find a sample implementation or a document
>  >> about which queues should really be implemented in LLDs and how they
>  >> should inter-operate with the mid- / block-layer. Say, a document,
>  >> describing the (SCSI-)command flow would be very useful, like
>  >
>  > None should be in the LLDD.  I have an experimental patch that doesn't
>  > remove the per-device queue yet but only does the easier part of
>  > returning MLQUEUE_DEVICE_BUSY in ->queuecommand but doesn't do the
>  > requeueing from the isr yet.  I'll send it out ASAP.
> 
> Looks good, thanks! Compiles too. I'll test it on the hardware tomorrow,
> should work, still, I'd do a short test. And, yes, please, feel free to
> offload any part of that further cleanup on me - I would just need a
> couple of hints. I'll do it slower and you'll have to fix it afterwards
> and consult me before, but I want to have my piece of fun too!:-)
> 
>  >> n)   scsi.c: LLD->queuecommand()
>  >> n+1) LLD:    send out on the bus, insert on tcq / return
>  >>      SCSI_MLQUEUE_DEVICE_BUSY
>  >> n+2) LLD:    on command-completion, if successful, report 
> (scsi_done()),
>  >>      if not - auto request sense, put the request-sense command on
>  >>      internal "going" queue
>  >> ...
>  >>
>  >> Where n+2 is, actually, the second question - I've seen a comment
>  >> somewhere, that LLDs should (not?) auto request-sense, rather than 
> failing
>  >> and letting the mid-layer issue one...
>  >
>  > No, mid-layer auto request sense is deprecated and isn't that reliable.
>  > See the comment ontop of scsi_eh_get_sense() in scsi_error.c
> 
> That's actually what I was reading and that's where I didn't understand:
> 
>   * Notes:
>   *    This has the unfortunate side effect that if a shost adapter does
>   *    not automatically request sense information, that we end up shutting
>   *    it down before we request it.  All shosts should be doing this
>                                                                 ^^^^
>   *    anyways, so for now all I have to say is tough noogies if you end up
>   *    in here.  On second thought, this is probably a good idea.  We
>   *    *really* want to give authors an incentive to automatically request
>   *    this.
> 
> So, I failed to understand if "this" above means "All shosts should be
> automatically requesting sense" or the opposite. Then the "second
> thought"... So, you mean LLDs should really auto-request sense on
> failures. Well, if we detect an error condition when we are CONNECTED to a
> host, it should be possible to directly send a REQUEST SENSE - the bus
> should be ours at this time, right?
> 
>  >> Is there a "perfect" LLD, I could look at for an example? I looked at
>  >> 53c700.c, as it was until recently the only one using tcq, but it 
> also has
>  >> a few of those internal queues...
>  >
>  > that driver queues commands in HBA memory, not in the driver.  It's one
>  > of the NCR scripts family HBAs where the firmware is written specificly
>  > to match the driver and they cooperate closely.  (better ask James on
>  > the details..)
> 
> Ok, I see (roughly), thanks.
> 
> Many thanks
> Guennadi
> ---
> Guennadi Liakhovetski
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] clean up some more tmscsim scan logic

[Guennadi Liakhovetski]

On Thu, 19 Aug 2004, Luben Tuikov wrote:

> The device server should send sense information when the task
> finishes with CHECK CONDITION status _with_ the same transaction.
> (SAM-3r13, 5.9.6, SPC-3r18, 4.5.1).  This used to be called
> "autosense" a couple of years back, but no more as it is now
> protocol.
>
> This has been off-loaded to LLDD when the device doesn't support
> it, so that SCSI Core is basically SAM.
>
> BTW, the state and task queue would most likely change between
> the time SCSI Core gets CHECK CONDITION (with no sense) until
> the time it issues REQUEST SENSE, to get the sense for _that_
> command which failed.  Thus we want LLDDs to emulate it for
> devices which do not support it, since it has better (closer)
> control of the device.

Thanks for the explanations, Luben! Sounds reasonable, and crystalises my 
suspicions and guesses into a solid medium:-)

Guennadi
---
Guennadi Liakhovetski

Re: [PATCH] clean up some more tmscsim scan logic

[Guennadi Liakhovetski]

On Thu, 19 Aug 2004, Guennadi Liakhovetski wrote:

> On Thu, 19 Aug 2004, Christoph Hellwig wrote:
>
>> None should be in the LLDD.  I have an experimental patch that doesn't
>> remove the per-device queue yet but only does the easier part of
>> returning MLQUEUE_DEVICE_BUSY in ->queuecommand but doesn't do the
>> requeueing from the isr yet.  I'll send it out ASAP.
>
> Looks good, thanks! Compiles too. I'll test it on the hardware tomorrow,

Ok, tested, works. So, James, please, apply. As for the proc-removal 
patch, don't know. The whole /proc/scsi and its subdirs are not going to 
disappear tomorrow, and a few other LLDs have their subdirs there (e.g., 
aic7xxx, sym53c8xx - those I can see here at least) so, don't know if it 
is the right thing to remove the tmscsim subdir. What's the current trend?

On a side note - my suspicion that INQUIRY outside of initial bus-scan was 
not such an exceptional situation turned right:-) E.g., scsiinfo issues 
it. And then my fix for bug 2139 breaks - it only handles those broken 
drives on initial scan. I've tried to improve that fix to generally not 
use tags for INQUIRY - it worked. But, I guess, unless anybody else 
complains, I will not apply / submit this patch - it is better to switch 
to tcq altogether, and the midlayer doesn't tag INQUIRY, right?

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: [PATCH] clean up some more tmscsim scan logic

[Christoph Hellwig]

On Thu, Aug 19, 2004 at 11:16:00PM +0200, Guennadi Liakhovetski wrote:
> That's actually what I was reading and that's where I didn't understand:
> 
>  * Notes:
>  *    This has the unfortunate side effect that if a shost adapter does
>  *    not automatically request sense information, that we end up shutting
>  *    it down before we request it.  All shosts should be doing this
> 								^^^^
>  *    anyways, so for now all I have to say is tough noogies if you end up
>  *    in here.  On second thought, this is probably a good idea.  We
>  *    *really* want to give authors an incentive to automatically request
>  *    this.
> 
> So, I failed to understand if "this" above means "All shosts should be 
> automatically requesting sense" or the opposite.

I'm pretty sure it means the HBA or LLDD should do it.  I'll send James
a patch to fix up the wording.


> Then the "second 
> thought"... So, you mean LLDs should really auto-request sense on 
> failures. Well, if we detect an error condition when we are CONNECTED to a 
> host, it should be possible to directly send a REQUEST SENSE - the bus 
> should be ours at this time, right?

I think so.

Re: [PATCH] clean up some more tmscsim scan logic

[Guennadi Liakhovetski]

On Tue, 17 Aug 2004, Christoph Hellwig wrote:

> - cleanup checks in ->queuecommand, we only get here either for
>   inquiry or a found device now
> - DCBmap in the acb is gone, was only used for debug prints after
>   the prevous changes
> - kill some more debugging keyed of by ->scan_devices

James, please, apply.

Thanks
Guennadi

>
>
> --- 1.19/drivers/scsi/scsiiom.c	2004-07-05 00:25:48 +02:00
> +++ edited/drivers/scsi/scsiiom.c	2004-08-17 19:56:00 +02:00
> @@ -1398,26 +1398,7 @@
> 	pSRB->SRBFlag &= ~AUTO_REQSENSE;
> 	pSRB->AdaptStatus = 0;
> 	pSRB->TargetStatus = CHECK_CONDITION << 1;
> -#ifdef DC390_REMOVABLEDEBUG
> -	switch (pcmd->sense_buffer[2] & 0x0f)
> -	{
> -	 case NOT_READY: printk (KERN_INFO "DC390: ReqSense: NOT_READY (Cmnd = 0x%02x, Dev = %i-%i, Stat = %i, Scan = %i)\n",
> -				 pcmd->cmnd[0], pDCB->TargetID, pDCB->TargetLUN,
> -				 status, pACB->scan_devices); break;
> -	 case UNIT_ATTENTION: printk (KERN_INFO "DC390: ReqSense: UNIT_ATTENTION (Cmnd = 0x%02x, Dev = %i-%i, Stat = %i, Scan = %i)\n",
> -				      pcmd->cmnd[0], pDCB->TargetID, pDCB->TargetLUN,
> -				      status, pACB->scan_devices); break;
> -	 case ILLEGAL_REQUEST: printk (KERN_INFO "DC390: ReqSense: ILLEGAL_REQUEST (Cmnd = 0x%02x, Dev = %i-%i, Stat = %i, Scan = %i)\n",
> -				       pcmd->cmnd[0], pDCB->TargetID, pDCB->TargetLUN,
> -				       status, pACB->scan_devices); break;
> -	 case MEDIUM_ERROR: printk (KERN_INFO "DC390: ReqSense: MEDIUM_ERROR (Cmnd = 0x%02x, Dev = %i-%i, Stat = %i, Scan = %i)\n",
> -				    pcmd->cmnd[0], pDCB->TargetID, pDCB->TargetLUN,
> -				    status, pACB->scan_devices); break;
> -	 case HARDWARE_ERROR: printk (KERN_INFO "DC390: ReqSense: HARDWARE_ERROR (Cmnd = 0x%02x, Dev = %i-%i, Stat = %i, Scan = %i)\n",
> -				      pcmd->cmnd[0], pDCB->TargetID, pDCB->TargetLUN,
> -				      status, pACB->scan_devices); break;
> -	}
> -#endif
> +
> 	//pcmd->result = MK_RES(DRIVER_SENSE,DID_OK,0,status);
> 	if (status == (CHECK_CONDITION << 1))
> 	{
> @@ -1571,23 +1552,6 @@
> 	    pDCB->Inquiry7 = ptr->Flags;
>
> ckc_e:
> -    if( pACB->scan_devices )
> -    {
> -	if( pcmd->cmnd[0] == TEST_UNIT_READY ||
> -	    pcmd->cmnd[0] == INQUIRY)
> -	{
> -#ifdef DC390_DEBUG0
> -	    printk (KERN_INFO "DC390: %s: result: %08x",
> -		    (pcmd->cmnd[0] == INQUIRY? "INQUIRY": "TEST_UNIT_READY"),
> -		    pcmd->result);
> -	    if (pcmd->result & (DRIVER_SENSE << 24)) printk (" (sense: %02x %02x %02x %02x)\n",
> -				   pcmd->sense_buffer[0], pcmd->sense_buffer[1],
> -				   pcmd->sense_buffer[2], pcmd->sense_buffer[3]);
> -	    else printk ("\n");
> -#endif
> -	}
> -    }
> -
>     if( pcmd->cmnd[0] == INQUIRY &&
> 	(pcmd->result == (DID_OK << 16) || status_byte(pcmd->result) & CHECK_CONDITION) )
>      {
> --- 1.45/drivers/scsi/tmscsim.c	2004-07-05 00:35:28 +02:00
> +++ edited/drivers/scsi/tmscsim.c	2004-08-17 20:05:52 +02:00
> @@ -612,11 +609,7 @@
>      {
> 	pDCB = pDCB->pNextDCB;
> 	if (pDCB == pACB->pLinkDCB)
> -	  {
> -	     DCBDEBUG(printk (KERN_WARNING "DC390: DCB not found (DCB=%p, DCBmap[%2x]=%2x)\n",
> -			      pDCB, id, pACB->DCBmap[id]));
> 	     return 0;
> -	  }
>      }
>    DCBDEBUG1( printk (KERN_DEBUG "DCB %p (%02x,%02x) found.\n",	\
> 		      pDCB, pDCB->TargetID, pDCB->TargetLUN));
> @@ -984,28 +977,6 @@
>     struct dc390_srb*   pSRB;
>     struct dc390_acb*   pACB = (struct dc390_acb*) cmd->device->host->hostdata;
>
> -    DEBUG0(/*  if(pACB->scan_devices) */	\
> -	printk(KERN_INFO "DC390: Queue Cmd=%02x,Tgt=%d,LUN=%d (pid=%li), buffer=%p\n",\
> -	       cmd->cmnd[0],cmd->device->id,cmd->device->lun,cmd->pid, cmd->buffer));
> -
> -    /* TODO: Change the policy: Always accept TEST_UNIT_READY or INQUIRY
> -     * commands and alloc a DCB for the device if not yet there. DCB will
> -     * be removed in dc390_SRBdone if SEL_TIMEOUT */
> -    if (!(pACB->scan_devices) && !(pACB->DCBmap[cmd->device->id] & (1 << cmd->device->lun))) {
> -	printk(KERN_INFO "DC390: Ignore target %02x lun %02x\n",
> -		cmd->device->id, cmd->device->lun);
> -	goto fail;
> -    }
> -
> -    /* Should it be: BUG_ON(!pDCB); ? */
> -
> -    if (!pDCB)
> -    {  /* should never happen */
> -	printk (KERN_ERR "DC390: no DCB found, target %02x lun %02x\n",
> -		cmd->device->id, cmd->device->lun);
> -	goto fail;
> -    }
> -
>     pACB->Cmds++;
>     cmd->scsi_done = done;
>     cmd->result = 0;
> @@ -1026,10 +997,6 @@
>
>  requeue:
>     return 1;
> - fail:
> -    cmd->result = DID_BAD_TARGET << 16;
> -    done(cmd);
> -    return 0;
> }
>
> /* We ignore mapping problems, as we expect everybody to respect
> @@ -1394,7 +1361,6 @@
> static void __devinit dc390_initACB (struct Scsi_Host *psh, unsigned long io_port, u8 Irq, u8 index)
> {
>     struct dc390_acb*    pACB;
> -    u8   i;
>
>     psh->can_queue = MAX_CMD_QUEUE;
>     psh->cmd_per_lun = MAX_CMD_PER_LUN;
> @@ -1441,8 +1407,6 @@
>     dc390_linkSRB( pACB );
>     pACB->pTmpSRB = &pACB->TmpSRB;
>     dc390_initSRB( pACB->pTmpSRB );
> -    for(i=0; i<MAX_SCSI_ID; i++)
> -	pACB->DCBmap[i] = 0;
>     pACB->sel_timeout = SEL_TIMEOUT;
>     pACB->glitch_cfg = EATER_25NS;
>     pACB->Cmds = pACB->CmdInQ = pACB->CmdOutOfSRB = 0;
> @@ -1603,7 +1567,6 @@
> 			pDCB->CtrlR4 |= NEGATE_REQACKDATA | NEGATE_REQACK;
> 	}
>
> -	pACB->DCBmap[id] |= (1 << lun);
> 	dc390_updateDCB(pACB, pDCB);
>
> 	pACB->scan_devices = 1;
> @@ -1627,8 +1590,6 @@
>
> 	BUG_ON(pDCB->GoingSRBCnt > 1);
>
> -	pACB->DCBmap[pDCB->TargetID] &= ~(1 << pDCB->TargetLUN);
> -
> 	if (pDCB == pACB->pLinkDCB) {
> 		if (pACB->pLastDCB == pDCB) {
> 			pDCB->pNextDCB = NULL;
> :q
> --- 1.8/drivers/scsi/tmscsim.h	2004-06-24 23:33:43 +02:00
> +++ edited/drivers/scsi/tmscsim.h	2004-08-17 19:56:00 +02:00
> @@ -163,7 +163,6 @@
> struct dc390_srb	*pTmpSRB;
>
> u8		msgin123[4];
> -u8		DCBmap[MAX_SCSI_ID];
> u8		Connected;
> u8		pad;
>
>
>

---
Guennadi Liakhovetski

Re: [PATCH] clean up some more tmscsim scan logic

[Christoph Hellwig]

On Tue, Aug 17, 2004 at 10:17:55PM +0200, Guennadi Liakhovetski wrote:
> On Tue, 17 Aug 2004, Christoph Hellwig wrote:
> 
> >- cleanup checks in ->queuecommand, we only get here either for
> >  inquiry or a found device now
> >- DCBmap in the acb is gone, was only used for debug prints after
> >  the prevous changes
> >- kill some more debugging keyed of by ->scan_devices
> 
> James, please, apply.

Warning, without the proc_info removal patch it won't compile.  The
fix is trivial though, all reference to DCBmap in ->proc_info need
to be removed.