Re: [PATCH] libata-sff: Correct use of check_status()

[Jeff Garzik <jeff@garzik.org>]

Alan Cox wrote:
>>> +	tf->command = ata_chk_status(ap);
>>>  	tf->feature = ioread8(ioaddr->error_addr);
>>>  	tf->nsect = ioread8(ioaddr->nsect_addr);
>>>  	tf->lbal = ioread8(ioaddr->lbal_addr);
>> applied, with a sigh:  it's in SFF, so I saw nothing wrong with 
>> ata_check_status().  I checked -- no SFF driver overrides it according 
>> to my audit, therefore the previous version was faster while still correct.
> 
> I hit it with the NS87415 where ata_check_status() doesn't do the right
> thing. Later on it turned out that there were enough other bugs when
> enabling DMA that it needed its own tf read method anyway.
> 
> Alternative is to go through and clear up chk_atatus v check_status
> (rename one sff_ to be clear), and explicitly document the assumption in
> tf_read. That way it'll be obvious to whoever hits it in future and they
> can supply their own tf_read method.
> 
> Preferences ?

That was my general preference -- use your own ->tf_read()

	Jeff