Re: [PATCH 1/2] libata: Add three more columns to the ata_timing table.

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

David Daney wrote:

>>> diff --git a/include/linux/libata.h b/include/linux/libata.h
>>> index 59b0f1c..7c44e45 100644
>>> --- a/include/linux/libata.h
>>> +++ b/include/linux/libata.h

>> [...]

>>> @@ -863,6 +868,9 @@ struct ata_timing {
>>>      unsigned short cyc8b;        /* t0 for 8-bit I/O */
>>>      unsigned short active;        /* t2 or tD */
>>>      unsigned short recover;        /* t2i or tK */
>>> +    unsigned short write_hold;    /* t4 */
>>> +    unsigned short read_hold;    /* t6 */

>>   -DIOR hold time is 5 ns for all PIO and MWDMA modes -- there's no 
>> sense in storing it here.

    Oops, that doesn't follow indeed...

> By storing it here, ata_timing_compute() will compute the bus clock 
> counts for me.  Makes perfect sense to me.

    Then let's just put *every* timing specified by the ATA standard there. 
That would be consistent at least. :-)

>>> +    unsigned short read_holdz;    /* t6z  or tj */

>>   T6z and Tj are not the same timing. Well, you specify it as 0 for  
>> DMA modes anyway...

> I had intended to overload this column for DMA, but somehow screwed it 
> up.  For my next patch I plan to populate this column for DMA modes.

    There seems to be no timing "parallel" for T6z in the DMA transfers, so 
you'll need another field then. Remember, the PIO and DMA timings are also 
"mergeable" -- by selecting the maximum field value of 2 'struct ata_timing' 
instances, so there should be no arbitrary field overloading.

> David Daney

WBR, Sergei