Linux-ide Archive mirror
 help / color / mirror / Atom feed
* [PATCH] ata: SATL compliance for Inquiry Product Revision
@ 2014-04-29 19:25 Keith Busch
  2014-05-01 15:15 ` Tejun Heo
  0 siblings, 1 reply; 3+ messages in thread
From: Keith Busch @ 2014-04-29 19:25 UTC (permalink / raw
  To: linux-ide; +Cc: Keith Busch

The SCSI/ATA Translation standard says to use data words 25 and 26 unless
they are spaces. These words are generally more useful anway for devices
that are not padding the firmware field with spaces.

Signed-off-by: Keith Busch <keith.busch@intel.com>
Reviewed-by: Matthew Wilcox <matthew.r.wilcox@intel.com>
---
 drivers/ata/libata-scsi.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index ef8567d..8034a38 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1993,7 +1993,10 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
 	memcpy(rbuf, hdr, sizeof(hdr));
 	memcpy(&rbuf[8], "ATA     ", 8);
 	ata_id_string(args->id, &rbuf[16], ATA_ID_PROD, 16);
-	ata_id_string(args->id, &rbuf[32], ATA_ID_FW_REV, 4);
+	if (args->id[25] == 0x2020 && args->id[26] == 0x2020)
+		ata_id_string(args->id, &rbuf[32], ATA_ID_FW_REV, 4);
+	else
+		ata_id_string(args->id, &rbuf[32], ATA_ID_FW_REV + 2, 4);
 
 	if (rbuf[32] == 0 || rbuf[32] == ' ')
 		memcpy(&rbuf[32], "n/a ", 4);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] ata: SATL compliance for Inquiry Product Revision
  2014-04-29 19:25 [PATCH] ata: SATL compliance for Inquiry Product Revision Keith Busch
@ 2014-05-01 15:15 ` Tejun Heo
  2014-05-01 16:44   ` Keith Busch
  0 siblings, 1 reply; 3+ messages in thread
From: Tejun Heo @ 2014-05-01 15:15 UTC (permalink / raw
  To: Keith Busch; +Cc: linux-ide

On Tue, Apr 29, 2014 at 01:25:24PM -0600, Keith Busch wrote:
> The SCSI/ATA Translation standard says to use data words 25 and 26 unless
> they are spaces. These words are generally more useful anway for devices
> that are not padding the firmware field with spaces.
> 
> Signed-off-by: Keith Busch <keith.busch@intel.com>
> Reviewed-by: Matthew Wilcox <matthew.r.wilcox@intel.com>
> ---
>  drivers/ata/libata-scsi.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index ef8567d..8034a38 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -1993,7 +1993,10 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
>  	memcpy(rbuf, hdr, sizeof(hdr));
>  	memcpy(&rbuf[8], "ATA     ", 8);
>  	ata_id_string(args->id, &rbuf[16], ATA_ID_PROD, 16);
> -	ata_id_string(args->id, &rbuf[32], ATA_ID_FW_REV, 4);
> +	if (args->id[25] == 0x2020 && args->id[26] == 0x2020)

Maybe strncmp(" ") is more readable?  And can you please add some
comment explaining what's going on?

> +		ata_id_string(args->id, &rbuf[32], ATA_ID_FW_REV, 4);
> +	else
> +		ata_id_string(args->id, &rbuf[32], ATA_ID_FW_REV + 2, 4);

So, this would change the behavior for devices which were reporting
stuff in ATA_ID_FW_REV.  Prolly no biggiee.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ata: SATL compliance for Inquiry Product Revision
  2014-05-01 15:15 ` Tejun Heo
@ 2014-05-01 16:44   ` Keith Busch
  0 siblings, 0 replies; 3+ messages in thread
From: Keith Busch @ 2014-05-01 16:44 UTC (permalink / raw
  To: Tejun Heo; +Cc: Keith Busch, linux-ide

On Thu, 1 May 2014, Tejun Heo wrote:
> On Tue, Apr 29, 2014 at 01:25:24PM -0600, Keith Busch wrote:
>> @@ -1993,7 +1993,10 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
>>  	memcpy(rbuf, hdr, sizeof(hdr));
>>  	memcpy(&rbuf[8], "ATA     ", 8);
>>  	ata_id_string(args->id, &rbuf[16], ATA_ID_PROD, 16);
>> -	ata_id_string(args->id, &rbuf[32], ATA_ID_FW_REV, 4);
>> +	if (args->id[25] == 0x2020 && args->id[26] == 0x2020)
>
> Maybe strncmp(" ") is more readable?  And can you please add some
> comment explaining what's going on?

Sounds good. Will do a v2 with that.

>> +		ata_id_string(args->id, &rbuf[32], ATA_ID_FW_REV, 4);
>> +	else
>> +		ata_id_string(args->id, &rbuf[32], ATA_ID_FW_REV + 2, 4);
>
> So, this would change the behavior for devices which were reporting
> stuff in ATA_ID_FW_REV.  Prolly no biggiee.

Yep, though many devices do pad with spaces so it won't change for
those. I did do a sanity check on both kinds to make sure it looked right.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2014-05-01 16:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-29 19:25 [PATCH] ata: SATL compliance for Inquiry Product Revision Keith Busch
2014-05-01 15:15 ` Tejun Heo
2014-05-01 16:44   ` Keith Busch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).