Re: [PATCH 3/6] sata_sil: new interrupt handler

[Tejun Heo <htejun@gmail.com>]

Alan Cox wrote:
> On Gwe, 2006-05-12 at 00:14 +0900, Tejun Heo wrote:
>> The DMA complete bit of these controllers reflects ATA IRQ status
>> while no DMA command is in progress.  So, we can tell whether the
>> controller is raising an interrupt or not in deterministic manner.
>> This patch gives sata_sil its own interrupt handler which behaves much
>> better than the original one in terms of error detection and handling.
>> This change is also necessary for later hotplug support.
>>
> 
> Is it not possible to avoid duplicating so much code. If the
> sil_host_intr and a few other _intr routines end up with their own
> host_intr functions closely based on the core one and intimate state
> machine knowledge then fixes are going to get missed in one or the other
> over time 
> 

Yes, it's a bit worrying.  The current sata_sil irq handler is a little
bit too different to factor codes with the stock handler while a bit too
similar to justify the copying.  I think it's okay to leave it as it is
for the time being.  sata_sil will probably deviate more from the stock
handler as more BMDMA2 stuff gets implemented and controllers which
resemble the traditional BMDMA interface more than sata_sil should be
able to use ata_host_intr() with some wrapping.

-- 
tejun