Re: [PATCH v3 1/3] libata: Populate host-to-device FIS "auxiliary" field

[Marc C <marc.ceeeee@gmail.com>]

Hello,
> The auxiliary field is part of ata taskfile for all intents and purposes.
> FIS is the new command structure anyway and struct ata_taskfile proper
> should be able to describe the command with ata_queuedcmd providing
> the surrounding context.  The argument that ata_taskfile shouldn't
> contain anything which wasn't in PATA taskfile is bogus as it already
> contains ATA_TFLAG_*.
>     That's what make 'struct ata_taskfile' bogus. I'm going to remove 
> 'protocol', 'flags', and 'ctl' fields from there (at least to save a space in 
> 'struct ata_queued_cmd' because they're not used in 'result_tf').
>
>> So, please put the aux field into ata_taskfile.  That's where it
>> belongs.
>     Can't agree to that. I was going to make 'struct ata_taskfile' reflect the 
> historical notion and remove from it all not belonging to that notion. Alas, 
> libata has a bad history of mistreating the historical terms...

While I understand that some non-spec items don't belong in the taskfile struct (and should/will be cleaned up), I have to agree with Tejun regarding how the auxiliary field should be put in 'struct ata_taskfile'. If anything, the struct should represent ALL items that an ATA command will use as inputs. As of now, it's the stuff that's currently in the taskfile + this pesky new 'auxiliary' entry. (FYI, ATA-8 ACS refers to these items as"fields.")

>From what I can tell, there are a couple of places where a rigid definition of a taskfile truly matters, and that's:

1) SCSI to ATA passthrough
2) PATA

But in any event, sticking 'auxiliary' in doesn't matter from a compatibility standpoint, since only NCQ commands use the new field, and the above can't transport NCQ (yet in (1) case).

Tejun put it succinctly by stating:

> I mean, FIS is the new TF.  We can rename ata_taskfile to
> ata_fis and map things the other way but that'd just be extra churn.

Ideally this would be the case, since the SATA FIS is a super-set of any/all fields defined in all ATA commands.

Regards,
Marc