Re: [PATCH] ide/libata: fix ata_id_is_cfa()

[Sergei Shtylyov <sshtylyov@ru.mvista.com>]

Hello.

Jeff Garzik wrote:

>>>>>>> +    if (id[ATA_ID_MAJOR_VER] == 0xFFFF)
>>>>>>> +        return 0;
>>>>>>> +    return (id[ATA_ID_MAJOR_VER] & (1 << v)) ? 1 : 0;

>>>>>>   Refer to afa_dev_cf_sata() on how it's done in really optimal way.

>>>>> To what ? - there is no ata or afa_dev_cf_sata ?

>>>>    Very funny. Meant to be ata_dev_is_sata(), of course.

>>> We don't have one of those either - do you mean ata_id_is_sata ? If so
>>> then yes that looks like it might be slightly cleaner although its
>>> probably one instruction difference from the .s files.

>>    That extra *if* cost more than instruction I think.

> Either way, this is irrelevant, since this isn't used in any hot path 
> that I am aware of... :)

> Alan just posted a reasonable explanation in the "The logic is this" 
> email, maybe we can reboot the discussion from there?

    Please read all the thread, and you'll see that Alan's CFA patch was 
totally wrong in that part from the very start -- CF devices don't report ATA 
standard support in word 80, that's forbidden (!) by the CF specs since at 
least 2.1.

> Responding to a side point, I don't think its a big deal to combine 
> fixes and improvements into a single patch, if you are dealing with the 
> same few lines of code.

    Not the case here -- the fix is very local, improvements are spread over 
several inlines.

>     Jeff

MBR, Sergei