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

[Tejun Heo <htejun@gmail.com>]

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.
> 
> How about refactoring the PCI-IDE specific logic from libata-core to a 
> seperate source file, say, ata_pciide.c?
> 
> Currently we have many PCI-IDE specific driving logic in libata-core.c.
> Maybe we can seperate the different driving logic required by different
> hardware interface into different source files?
> This could make libata-core.c to be more abstract and generic from the underlying
> hardware: libata-core only knows about the ata_port_operations interface.
> (ata_port_operations should be generic enough to cover all the hardware types.)
> LLDDs can either select default implementation for its hardware interface type
> (such as ata_bmdma_setup() from ata_pciide.c) or override harddware specfic functions.

I agree.  If for nothing else, the size of libata-core.c is getting too 
large.  Also, we currently have multiple levels of methods in 
ata_port_operations, some operations are only used by other operations, 
which is a bit confusing I think.

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.

> Ex. hardware interface types:
>   - PCI IDE (bmdma + PRD tables)
>     (covers legacy taskfile registers interface) => ata_pciide.c
>   - ADMA     => pdc_adma.c
>   - AHCI     => ahci.c
>   - Initio   => sata_initio.c (ata_qc_issue_pio and ata_qc_pio_intr implementation here)

ata_qc_issue_pio was bad naming probably.  As I wrote before, I was 
asking for a PIO helper which inic_qc_issue() can call to drive PIO 
logic and the same for ata_qc_pio_intr().

>   - SAS/SATA => sata_sas.c (ata_sas_port_start/stop implementation here)
>   - etc.     

Thanks.

-- 
tejun