ahci: ahci_host_intr() problem(s)?

ahci: ahci_host_intr() problem(s)?

[Bastiaan Jacques]

Hello,


I was looking at ahci_host_intr() when I noticed something strange: it always 
returns 1. Now, that alone is odd because when a function is successful it 
ordinarily returns 0 (zero).

But more importantly, it is called in ahci_interrupt():

			if (!ahci_host_intr(ap, qc))
				if (ata_ratelimit())
					dev_printk(KERN_WARNING, host_set->dev,
					  "unhandled interrupt on port %u\n",
					  i);

So that if-statement is always false; this seems like a logic error to me.

Back to ahci_host_intr itself. it does:

	serr = readl(port_mmio + PORT_SCR_ERR);
	writel(serr, port_mmio + PORT_SCR_ERR);

This looks like first we are reading a value from the port, and then writing 
the same value back without modification. Why?

I would like to fix the first problem described above, but I don't understand 
what this function does.. so perhaps someone who has a greater understanding 
of this code should post a patch. Or explain what it should do :)

---
Bastiaan

Re: ahci: ahci_host_intr() problem(s)?

[Jeff Garzik]

Bastiaan Jacques wrote:
> I was looking at ahci_host_intr() when I noticed something strange: it always 
> returns 1. Now, that alone is odd because when a function is successful it 
> ordinarily returns 0 (zero).
> 
> But more importantly, it is called in ahci_interrupt():
> 
> 			if (!ahci_host_intr(ap, qc))
> 				if (ata_ratelimit())
> 					dev_printk(KERN_WARNING, host_set->dev,
> 					  "unhandled interrupt on port %u\n",
> 					  i);
> 
> So that if-statement is always false; this seems like a logic error to me.

Its dead code at present, yes.


> Back to ahci_host_intr itself. it does:
> 
> 	serr = readl(port_mmio + PORT_SCR_ERR);
> 	writel(serr, port_mmio + PORT_SCR_ERR);
> 
> This looks like first we are reading a value from the port, and then writing 
> the same value back without modification. Why?

Standard write-1s-to-clear register behavior.

	Jeff