public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Tian, Feng" <feng.tian@intel.com>
To: Jeremy Linton <jeremy.linton@arm.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "ard.biesheuvel@linaro.org" <ard.biesheuvel@linaro.org>,
	"Steve.Capper@arm.com" <Steve.Capper@arm.com>,
	"ryan.harkin@linaro.org" <ryan.harkin@linaro.org>,
	"leif.lindholm@linaro.org" <leif.lindholm@linaro.org>,
	"linaro-uefi@lists.linaro.org" <linaro-uefi@lists.linaro.org>,
	"Tian, Feng" <feng.tian@intel.com>
Subject: Re: [PATCH v3 2/7] MdePkg IndustryStandard/Scsi.h: Add sense code macro
Date: Fri, 24 Feb 2017 06:42:53 +0000	[thread overview]
Message-ID: <7F1BAD85ADEA444D97065A60D2E97EE5699B5197@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <20170223223355.11383-3-jeremy.linton@arm.com>

Linton,

Could you let me know where the +#define EFI_SCSI_REQUEST_SENSE_ERROR  (0x70) comes from?

According to SCSI SPC5r08 spec, the first byte of sense data is the RESPONSE CODE field. 

And 0x70 means

a) the result of an error, exception condition, or protocol specific failure that is associated with CHECK
CONDITION status; or
b) additional information that is associated with a status other than CHECK CONDITION.

It's possible that 0x70 means success... so this naming may be not quite appropriate.

And in your usage, you forcedly convert Packet->SenseData to UINT8 and access each sense data field by array index. I think it's unnecessary. Packet->SenseData is EFI_SCSI_SENSE_DATA type, which has defined the meaning of each field. With this structure, you don't need introduce EFI_SCSI_SK_VALUE macro again.

+        sense = (UINT8 *)Packet->SenseData;
+        if ((Packet->SenseDataLength > 13) &&
+            (sense[0] & EFI_SCSI_REQUEST_SENSE_ERROR)) {
+          DEBUG ((DEBUG_INFO, "SiI3132ScsiPassRead() Key %X ASC(Q) %02X%02X\n",
+                  EFI_SCSI_SK_VALUE (sense[2]), sense[12], sense[13]));
+        }

Thanks
Feng

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jeremy Linton
Sent: Friday, February 24, 2017 6:34 AM
To: edk2-devel@lists.01.org
Cc: ard.biesheuvel@linaro.org; Steve.Capper@arm.com; ryan.harkin@linaro.org; leif.lindholm@linaro.org; linaro-uefi@lists.linaro.org
Subject: [edk2] [PATCH v3 2/7] MdePkg IndustryStandard/Scsi.h: Add sense code macro

Add some definitions to mask the sense key from sense data, and check the validity of the returned sense data.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 MdePkg/Include/IndustryStandard/Scsi.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MdePkg/Include/IndustryStandard/Scsi.h b/MdePkg/Include/IndustryStandard/Scsi.h
index 0d81314..802479e 100644
--- a/MdePkg/Include/IndustryStandard/Scsi.h
+++ b/MdePkg/Include/IndustryStandard/Scsi.h
@@ -369,6 +369,8 @@ typedef struct {
 //
 // Sense Key
 //
+#define EFI_SCSI_REQUEST_SENSE_ERROR  (0x70)
+#define EFI_SCSI_SK_VALUE(byte)       (byte&0x0F)
 #define EFI_SCSI_SK_NO_SENSE          (0x0)
 #define EFI_SCSI_SK_RECOVERY_ERROR    (0x1)
 #define EFI_SCSI_SK_NOT_READY         (0x2)
--
2.9.3

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


  reply	other threads:[~2017-02-24  6:42 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-23 22:33 [PATCH v3 0/7] ATAPI support on SiI SATA adapter Jeremy Linton
2017-02-23 22:33 ` [PATCH v3 1/7] EmbeddedPkg: SiI3132: Note that ARM is using this Dxe Jeremy Linton
2017-02-23 22:33 ` [PATCH v3 2/7] MdePkg IndustryStandard/Scsi.h: Add sense code macro Jeremy Linton
2017-02-24  6:42   ` Tian, Feng [this message]
2017-02-24 18:23     ` Jeremy Linton
2017-02-23 22:33 ` [PATCH v3 3/7] EmbeddedPkg: SiI3132: Add ScsiProtocol callbacks Jeremy Linton
2017-02-24 17:08   ` Ard Biesheuvel
2017-03-03  1:05     ` Jeremy Linton
2017-02-23 22:33 ` [PATCH v3 4/7] EmbeddedPkg: SiI3132: Add SCSI protocol support to header Jeremy Linton
2017-02-24 17:09   ` Ard Biesheuvel
2017-02-23 22:33 ` [PATCH v3 5/7] EmbeddedPkg: SiI3132: Break out FIS command submission Jeremy Linton
2017-02-23 22:33 ` [PATCH v3 6/7] EmbeddedPkg: SiI3132: Cleanup device node creation Jeremy Linton
2017-02-23 22:33 ` [PATCH v3 7/7] EmbeddedPkg: SiI3132: Enable SCSI pass-through protocol Jeremy Linton
2017-02-23 22:33 ` [PATCH] Platforms/ARM/Juno: Add " Jeremy Linton

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-list 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=7F1BAD85ADEA444D97065A60D2E97EE5699B5197@SHSMSX101.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox