Re: [PATCH] libata-eh copy result_tf flags from outgoing tf

[Mark Lord <liml@rtr.ca>]

Tejun Heo wrote:
> Mark Lord wrote:
>>> Maybe it's better to set ATA_TFLAG_LBA48 | ATA_TFLAG_ISADDR | 
>>> ATA_TFLAG_DEVICE?  After all, all fields are available after that 
>>> memcpy().
>> ..
>>
>> I just did it the same way as the standard version in libata-core does 
>> it.
>> In this particular case, we know it was NCQ, so we know that the original
>> tf->flags have (ATA_TFLAG_LBA48|ATA_TFLAG_ISADDR|ATA_TFLAG_DEVICE) set.
>>
>> I figured better to copy than just overwrite, because new flags might 
>> come along.
> 
> Those flags are to tell which parts of the TF are valid or should be 
> read in case of TFLAG_LBA48.  So, a TF after tf_read should always have 
> TFLAG_ISADDR and TFLAG_DEVICE.  TFLAG_LBA48 works both ways.  If it was 
> set when tf_read is called the higher part of LBA is read and the 
> resulting tf should have TFLAG_LBA48 set.  But yeah the original code 
> isn't like that.  If you can fix it up together with the NCQ path, it 
> would be great.
..

No, I'm not going to touch the mainline path.  tf_read is used to read
the taskfile, but it doesn't always contain an LBA address, so I'm not so
sure that forcing TFLAG_ISADDR is always correct.

And since I don't know that code as well as you, I don't want to end up
having to help debug something that might break as a result.

This may warrant two patches.. one to fix the NCQ bug (already posted),
and perhaps another from you or Jeff if you want to change the behaviour.

Cheers