Re: irq-pio branch updated with Tejun's patches

[Tejun <htejun@gmail.com>]

Jeff Garzik wrote:
> Tejun Heo wrote:
> 
>> Hello, Albert.
>>
>> Albert Lee wrote:
>>
>>>>
>>>> This is sort of OT, but would it be possible to separate PIO
>>>> issuing/interrupt handling from ata_qc_issue_prot and ata_host_intr?
>>>>
>>>
>>> Isn't overriding the ->qc_issue() and ->irq_handle() in LLDD good 
>>> enough?
>>
>>
>>
>> The driver needs its own ->qc_issue() and ->irq_handler() but it also 
>> needs PIO support.  AFAICS, the current PIO implementation is rather 
>> strongly tied into ata_qc_issue_prot() and ata_host_intr() making it 
>> difficult to use PIO support in drivers which use private ->qc_issue() 
>> and ->irq_handler().  So, what I was asking was to separate out PIO 
>> handling from ata_qc_issue_prot() and ata_host_intr() such that other 
>> issue and intr routines can use PIO.
> 
> 
> What needs this separation?  initio?
> 

Yeah, it seems.  Its broken driver currently emulates BMDMA status to 
work with standard issue/intr functions, but it seems fragile.  In 
addition, for NCQ commands, the controller operates in different mode 
and the code will have to be distorted more than now.  So, I think it's 
better to use separate issue/intr handler and calling PIO helpers when 
needed.  Well, if that driver ever works, that is.  I'll contact Initio 
support again.  Hmmm.. I might even call them.  Heh heh. :-)

> 
>> Separating out traditional driving logic from libata-core.c would 
>> require another level of indirection such that traditional low level 
>> drivers still can mix & match different legacy operations.  I thought 
>> about it and wasn't really sure whether the added abstraction was 
>> worth the clean up.  We currently don't have too many controllers 
>> which are legacy-free, it seems.
> 
> IMO the solution is time.  Time and evolution will determine what's 
> best.  When a particular section of code get particularly large (PCI IDE 
> driver helper code), it certainly makes sense to split it out.  "Do what 
> you must, and no more" is the Linux maxim :)

Being a stupid over-designer, I'm sometimes a bit unconformtable with 
the approach but I guess I'll finally adapt.  I'll talk about this more 
in the thread regarding probe_reset component operatons.

Thanks.

-- 
tejun