Re: [PATCH] scsi/libata: Support variable-length cdb of ata pass-thru(32).

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, 2017-06-17 at 20:00 +0900, Minwoo Im wrote: 
> -	if ((tf->protocol = ata_scsi_map_proto(cdb[1])) == ATA_PROT_UNKNOWN) {
> +	/*
> +	 * if SCSI operation code in cdb[0] is ATA_12 or ATA_16,
> +	 * then cdb[1] will contain protocol of ATA PASS-THROUGH.
> +	 * otherwise, Its operation code shall be ATA_32(7Fh).
> +	 * in this case, cdb[10] will contain protocol of it.
> +	 * we call this command as a variable-length cdb.
> +	 */
> +	if (cdb[0] == ATA_12 || cdb[0] == ATA_16)
> +		tf->protocol = ata_scsi_map_proto(cdb[1]);
> +	else
> +		tf->protocol = ata_scsi_map_proto(cdb[10]);
> +
> +	if (tf->protocol == ATA_PROT_UNKNOWN) {
>  		fp = 1;
>  		goto invalid_fld;
>  	}

Hello Minwoo,

Please consider introducing a variable (e.g. called "cdb_offset") in which the
value 9 is stored for 32-byte CDBs and 0 for 12-byte and 16-byte CDBs. That
will allow to rewrite the above code as follows:

	tf->protocol = ata_scsi_map_proto(cdb[1 + cdb_offset])

I think that will allow to remove most "if (cdb[0] == ATA_12 || cdb[0] == ATA_16)"
statements introduced by this patch.

> +		tf->auxiliary = (cdb[28] << 24) | (cdb[29] << 16)
> +			| (cdb[30] << 8) | cdb[31];

Please use get_unaligned_be32() instead of open-coding it.

> +	const u16 sa = (cdb[8] >> 8) | cdb[9];	/* service action */

Please use get_unaligned_be16() instead of open-coding it.

> @@ -4385,7 +4463,12 @@ int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht)
>  		shost->max_id = 16;
>  		shost->max_lun = 1;
>  		shost->max_channel = 1;
> -		shost->max_cmd_len = 16;
> +		/*
> +		 * SPC-3, SPC-4: Definition of CDB
> +		 * A CDB may have a fixed length of up to 16 bytes or
> +		 * variable length of between 12 and 260 bytes.
> +		 */
> +		shost->max_cmd_len = 260;

Does ATA pass-through really support 260-byte CDBs or is the maximum CDB length
that is supported by ATA_32 perhaps 32 bytes?

Thanks,

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux