Re: Organized summary of the pacthes

[Tejun Heo <htejun@gmail.com>]

Albert Lee wrote:
> 
> Jeff,
> 
>> Albert Lee wrote:
>>
>>> Summary of the pacthes for your review (as of 6/28/2005):
>>> 1. atapi_pio_bytes() fixes
>>>   => Three patches pending:
>>>      #1-2. __atapi_pio_bytes(): if condition fix
>>>      http://marc.theaimsgroup.com/?l=linux-ide&m=111838913812842&w=2
>>
>>
>>
>> I'll probably apply this
>>
>>
>>>      #1-3. __atapi_pio_bytes(): trailing data handling fix
>>>      http://marc.theaimsgroup.com/?l=linux-ide&m=111838955410270&w=2
>>>
>>>      #1-4. __atapi_pio_bytes(): odd-length data handling fix
>>>      http://marc.theaimsgroup.com/?l=linux-ide&m=111838987630460&w=2
>>>      Patch #1-4 will overrun the odd-length buffer by one byte. :(
>>>      Maybe we can just ignore the last byte of odd-length buffer?
>>
>>
>>
>> These two patches make me REALLY nervous.  This overrun business needs 
>> to be handled in a different way.
>>
>> For DMA, we will want to copy 0-3 odd bytes into a 4-byte buffer, and 
>> then make that 4-byte buffer than final DMA segment.
>>
>> For PIO, we might as well use the same buffer, and copy the last 0-3 
>> (or 0-1?) odd bytes.
>>
>> Never overrun.
>>
> 
> Thanks for the review and tolerance for the newbie blindness.
> I'll revise/resubmit patch 1-3 and 1-4 per your advice.
> I will also study the DMA padding/alignment advised by you and Alan.
> 
>>
>>> 2. atapi_packet_task() assertion failed fix
>>>   => Revised patch submitted:
>>>      http://marc.theaimsgroup.com/?l=linux-ide&m=111830130223323&w=2
>>>   => Need more revision to remove the 'case ATA_PROT_ATAPI' handling 
>>> in it.
>>>     (It conflicts with patch #3-2 below:
>>>      http://marc.theaimsgroup.com/?l=linux-ide&m=111951880730921&w=2)
>>>
>>>
>>> 3. ata_pio_task() race condition fixes.
>>>   => Two patches submitted. Waiting for review:
>>>    #3-1  http://marc.theaimsgroup.com/?l=linux-ide&m=111958527005103&w=2
>>>    #3-2  http://marc.theaimsgroup.com/?l=linux-ide&m=111951880730921&w=2
>>
>>
>>
>> These are probably OK.
>>
>> However, I am pondering scrapping all the polling code, since on SATA, 
>> interrupt-driven mode is much more desirable.
> 
> 
> Is there any draft plan or design for the interrupt driven PIO code
> available? Also as Alan said, PIO polling would be useful for some
> situation. Any plan for libata to support both PIO polling mode and the
> interrupt driven mode?
> 
> If permitted, I would like to take this opportunity to participate in
> the interrupt driven PIO task, under your design guideline.
> 

  Hi, Albert.

  For interrupt-driven PIO, IDE driver should be an example albeit quite 
complex.  Once interrupt-driven PIO is implemented, IMHO, generic 
polling IO can easily be added by calling interrupt handler periodically 
while the driver is expecting notification (any command is in-flight). 
Most low-level SATA driver interrupt handlers are already ready to 
handle spurious interrupts, and maybe we can add a flag to disable 
polling for those unable to swallow spurious interrupts if any such 
device/driver exsits.

-- 
tejun