Re: [PATCH v2 06/14] qla2xxx: Improve T10-DIF/PI handling in driver.

From: "Madhani, Himanshu" <Himanshu.Madhani@cavium.com>
To: Bart Van Assche <Bart.VanAssche@sandisk.com>,
	"hch@infradead.org" <hch@infradead.org>,
	"target-devel@vger.kernel.org" <target-devel@vger.kernel.org>,
	"nab@linux-iscsi.org" <nab@linux-iscsi.org>
Cc: "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	"Malavali, Giridhar" <Giridhar.Malavali@cavium.com>
Subject: Re: [PATCH v2 06/14] qla2xxx: Improve T10-DIF/PI handling in driver.
Date: Wed, 8 Feb 2017 19:11:03 +0000	[thread overview]
Message-ID: <5FC636C4-9E38-43AB-8BFB-03FDBFC70911@cavium.com> (raw)
In-Reply-To: <1486580223.16026.16.camel@sandisk.com>

Hi Bart, 

On 2/8/17, 10:57 AM, "Bart Van Assche" <Bart.VanAssche@sandisk.com> wrote:

>On Fri, 2017-02-03 at 14:40 -0800, Himanshu Madhani wrote:
>> +	/* Response code and sense key */
>> +	((uint32_t *)ctio->u.status1.sense_data)[0] =
>> +	    cpu_to_le32((0x70 << 24) | (sense_key << 8));
>> +	/* Additional sense length */
>> +	((uint32_t *)ctio->u.status1.sense_data)[1] = cpu_to_le32(0x0a);
>> +	/* ASC and ASCQ */
>> +	((uint32_t *)ctio->u.status1.sense_data)[3] =
>> +	    cpu_to_le32((asc << 24) | (ascq << 16));
>
>Please use put_unaligned_le32() instead of open-coding it.
>
>>  struct qla_tgt_cmd {
>> @@ -885,11 +895,25 @@ struct qla_tgt_cmd {
>>  	struct list_head cmd_list;
>>  
>>  	struct atio_from_isp atio;
>> -	/* t10dif */
>> +
>> +	/* T10-DIF */
>> +#define DIF_ERR_NONE 0
>> +#define DIF_ERR_GRD 1
>> +#define DIF_ERR_REF 2
>> +#define DIF_ERR_APP 3
>> +	int8_t dif_err_code;
>>  	struct scatterlist *prot_sg;
>>  	uint32_t prot_sg_cnt;
>> -	uint32_t blk_sz;
>> +	uint32_t blk_sz, num_blks;
>> +	uint8_t scsi_status, sense_key, asc, ascq;
>> +
>>  	struct crc_context *ctx;
>> +	uint32_t	prot_op;
>> +	uint32_t	prot_type;
>> +	uint8_t		*cdb;
>> +	uint64_t	lba;
>> +	uint16_t	a_guard, e_guard, a_app_tag, e_app_tag;
>> +	uint32_t	a_ref_tag, e_ref_tag;
>>  
>>  	uint64_t jiffies_at_alloc;
>>  	uint64_t jiffies_at_free;
>
>There are already equivalents of prot_op, prot_type, cdb and lba in struct
>se_cmd. I think a few weeks ago Christoph had asked you not to duplicate
>se_cmd fields into struct qla_tgt_cmd?

Thanks for the review comments.  Looks like that was a miss in rebasing with latest patch. 

Will fix up patch and resend. 

>
>Bart.
>Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer:
>
>This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system.
>