Re: sata_promise.c:pdc_irq_clear() buglet?

[Jeff Garzik <jgarzik@pobox.com>]

Mikael Pettersson wrote:
> Jeff,
> 
> In the 2.6.8 kernel you added sata_promise.c:pdc_irq_clear()
> which looks as follows:
> 
>> static void pdc_irq_clear(struct ata_port *ap)
>> {
>>        struct ata_host *host = ap->host;
>>        void __iomem *mmio = host->iomap[PDC_MMIO_BAR];
>>
>>        readl(mmio + PDC_INT_SEQMASK);
>> }
> 
> I have two issues with this code:
> 1. It reads from the SEQMASK register, which is global for
>    all ports. Hence, calling pdc_irq_clear() on one port
>    potentially affects all ports.
> 2. Promise's drivers (both 1st and 2nd generation chip drivers)
>    instead read the port's ATA Command/Status register when
>    they want to clear any pending IRQs.
> 
> Did you write the code this way on purpose, or did you just
> accidentally use the wrong mechanism?
> 
> If you have no objections, I intend to change this to read
> Command/Status instead.

For sata_promise, at the time it was written, irq_clear was largely a 
sledgehammer for SFF situations that didn't automatically resolve 
themselves through normal operation.  Given the amount of SFF code that 
is _not_ enabled when using sata_promise is in use, ->irq_clear() is 
largely a relic for PIO data xfer situations.

Feel free to replace if you think is appropriate...

	Jeff