From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.101.70]) by ml01.01.org (Postfix) with ESMTP id 4AFC88218F for ; Fri, 24 Feb 2017 10:23:33 -0800 (PST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B79F128; Fri, 24 Feb 2017 10:23:32 -0800 (PST) Received: from [192.168.229.136] (u201426.usa.arm.com [10.118.28.62]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D80833F483; Fri, 24 Feb 2017 10:23:31 -0800 (PST) To: "Tian, Feng" , "edk2-devel@lists.01.org" References: <20170223223355.11383-1-jeremy.linton@arm.com> <20170223223355.11383-3-jeremy.linton@arm.com> <7F1BAD85ADEA444D97065A60D2E97EE5699B5197@SHSMSX101.ccr.corp.intel.com> Cc: "ard.biesheuvel@linaro.org" , "Steve.Capper@arm.com" , "ryan.harkin@linaro.org" , "leif.lindholm@linaro.org" , "linaro-uefi@lists.linaro.org" From: Jeremy Linton Message-ID: <8e55d3fc-f59a-dfbf-c0e3-8f605488de97@arm.com> Date: Fri, 24 Feb 2017 12:23:30 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <7F1BAD85ADEA444D97065A60D2E97EE5699B5197@SHSMSX101.ccr.corp.intel.com> Subject: Re: [PATCH v3 2/7] MdePkg IndustryStandard/Scsi.h: Add sense code macro X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 24 Feb 2017 18:23:33 -0000 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Hi, On 02/24/2017 12:42 AM, Tian, Feng wrote: > Linton, > > Could you let me know where the +#define EFI_SCSI_REQUEST_SENSE_ERROR (0x70) comes from? Its being anded with the response code field in an attempt to pick off 0x70..0x72 as a sanity check that the response data looks valid given that its also long enough to be a valid return. So, your right its not well named either. Likely as its just a debug print, the best plan is to drop the whole thing, particularly since the transport code is now stable enough that the sense data is usually valid. > > 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 > --- > 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 >