All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boaz Harrosh <bharrosh@panasas.com>
To: "James Bottomley" <James.Bottomley@SteelEye.com>,
	"\"Jürgen E. Fischer\"" <fischer@norbit.de>,
	"FUJITA Tomonori" <fujita.tomonori@lab.ntt.co.jp>,
	linux-scsi <linux-scsi@vger.kernel.org>,
	"Randy Dunlap" <rdunlap@xenotime.net>
Subject: [PATCH 3/6] aha152x.c - Preliminary fixes and some comments
Date: Sun, 29 Jul 2007 22:22:04 +0300	[thread overview]
Message-ID: <46ACE8DC.1020907@panasas.com> (raw)
In-Reply-To: <46ACE62F.4070108@panasas.com>


  hunk by hunk:
  - CHECK_CONDITION is what happens to cmnd->status >> 1
    or after status_byte() macro. But here it is used
    directly on status which means 0x1 which is an undefined
    bit in the standard. And is a status that will never
    return from a target.

  - in busfree_run at the DONE_SC phase we have 3 distinct
    operation:
	1-if(DONE_SC->SCp.phase & check_condition)
          The REQUEST_SENSE command return.
          - Restore original command
	  - Than continue to operation 3.
	2-if(DONE_SC->SCp.Status==SAM_STAT_CHECK_CONDITION)
          A regular command returned with a status.
	  - Internally re-Q a REQUEST_SENSE.
	  - Do not do operation 3.
	3-
	  - Complete the command and return it to scsi-ml
     So the 0x2 in both these operations (1,2) means the scsi
     check-condition status, hence SAM_STAT_CHECK_CONDITION

  - Here the code asks about !(DONE_SC->SCp.Status & not_issued)
    but "not_issued" is an enum belonging to the "phase" member
    and not to the Status returned from target. The reason this
    works is because not_issued==1 and Also CHECK_CONDITION==1
    (remember from hunk 1). So actually the code was asking
    !(DONE_SC->SCp.Status & CHECK_CONDITION). Which means
    "Has the status been read from target yet?"
    Staus is read at status_run(). "not_issued" is
    cleared in seldo_run() which is usually earlier than
    status_run().

  So this patch does nothing as far as assembly is concerned
  but it does let the reader understand what is going on.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>

---
 drivers/scsi/aha152x.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/aha152x.c b/drivers/scsi/aha152x.c
index c841f11..dc7d84b 100644
--- a/drivers/scsi/aha152x.c
+++ b/drivers/scsi/aha152x.c
@@ -986,7 +986,7 @@ static int aha152x_internal_queue(Scsi_Cmnd *SCpnt, struct completion *complete,
 	SCpnt->scsi_done	= done;
 	SCpnt->resid 		= SCpnt->request_bufflen;
 	SCpnt->SCp.phase	= not_issued | phase;
-	SCpnt->SCp.Status	= CHECK_CONDITION;
+	SCpnt->SCp.Status	= 0x1; /* Ilegal status by SCSI standard */
 	SCpnt->SCp.Message	= 0;
 	SCpnt->SCp.have_data_in	= 0;
 	SCpnt->SCp.sent_command	= 0;
@@ -1574,12 +1574,12 @@ static void busfree_run(struct Scsi_Host *shpnt)
 			cmd->use_sg          = sc->use_sg;
 			cmd->cmd_len         = sc->cmd_len;
 
-			cmd->SCp.Status = 0x02;
+			cmd->SCp.Status = SAM_STAT_CHECK_CONDITION;
 
 			HOSTDATA(shpnt)->commands--;
 			if (!HOSTDATA(shpnt)->commands)
 				SETPORT(PORTA, 0);	/* turn led off */
-		} else if(DONE_SC->SCp.Status==0x02) {
+		} else if(DONE_SC->SCp.Status==SAM_STAT_CHECK_CONDITION) {
 #if defined(AHA152X_STAT)
 			HOSTDATA(shpnt)->busfree_with_check_condition++;
 #endif
@@ -1587,7 +1587,7 @@ static void busfree_run(struct Scsi_Host *shpnt)
 			DPRINTK(debug_eh, ERR_LEAD "CHECK CONDITION found\n", CMDINFO(DONE_SC));
 #endif
 
-			if(!(DONE_SC->SCp.Status & not_issued)) {
+			if(!(DONE_SC->SCp.phase & not_issued)) {
 				Scsi_Cmnd *ptr = DONE_SC;
 				DONE_SC=NULL;
 #if 0
-- 
1.5.2.2.249.g45fd



  parent reply	other threads:[~2007-07-29 19:22 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-29 19:10 [patch 0/6] aha152x.c - Cleanup, bugfixes, convert to accessors Boaz Harrosh
2007-07-29 19:16 ` [PATCH 1/6] aha152x.c - In debug mode Boaz Harrosh
2007-07-29 19:18 ` [PATCH 2/6] aha152x.c - use bounce buffer Boaz Harrosh
2007-07-29 19:22 ` Boaz Harrosh [this message]
2007-07-29 19:24 ` [PATCH 4/6] aha152x.c - Clean Reset path Boaz Harrosh
2007-07-29 19:27 ` [PATCH 5/6] aha152x.c - Fix check_condition code-path Boaz Harrosh
2007-07-31  0:13   ` Randy Dunlap
2007-07-31  7:59     ` Boaz Harrosh
2007-07-31 17:08       ` Randy Dunlap
2007-07-31 18:40       ` Randy Dunlap
2007-08-01 13:51         ` James Bottomley
2007-08-01 16:34           ` Randy Dunlap
2007-08-02 11:26             ` Boaz Harrosh
2007-08-02 19:09               ` Randy Dunlap
2007-08-02 19:08                 ` James Bottomley
2007-08-02 20:22                 ` Jürgen E. Fischer
2007-08-02 22:47                 ` Randy Dunlap
2007-07-29 19:29 ` [PATCH 6/6] aha152x.c - use data accessors and !use_sg cleanup Boaz Harrosh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=46ACE8DC.1020907@panasas.com \
    --to=bharrosh@panasas.com \
    --cc=James.Bottomley@SteelEye.com \
    --cc=fischer@norbit.de \
    --cc=fujita.tomonori@lab.ntt.co.jp \
    --cc=linux-scsi@vger.kernel.org \
    --cc=rdunlap@xenotime.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.