Skip to contentSkip to conversation

libata: Populate host-to-device FIS "auxiliary" field

PATCH Marc C 1 participant 1 message Aug 10, 2013 1 message not in the archive libata

lore
Share & tools
lore

Marc C

author
#1 lore raw
In reply to a message not in the archive

Hello,

Quoted 10 lines

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').

Quoted 5 lines
Quoted 2 lines

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.")

Quoted 1 line

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:

Quoted 2 lines

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