Date: Wed, 23 Nov 2011 09:03:54 +0100
From: Stanislaw Gruszka <sgruszka@redhat.com>
To: David Miller <davem@davemloft.net>
Cc: Larry.Finger@lwfinger.net, linville@tuxdriver.com,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Ivo van Doorn <IvDoorn@gmail.com>,
	Gertjan van Wingerde <gwingerde@gmail.com>,
	Helmut Schaa <helmut.schaa@googlemail.com>, hedayaty@gmail.com
Subject: Re: pull request: wireless 2011-11-22
Message-ID: <20111123080353.GA3752@redhat.com> (sfid-20111123_090204_971651_9DF7663A)
References: <20111122.160511.2302883259439708995.davem@davemloft.net>
 <20111122.161322.454163033398038634.davem@davemloft.net>
 <4ECC1479.8090209@lwfinger.net>
 <20111122.164057.1411098663938026383.davem@davemloft.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
In-Reply-To: <20111122.164057.1411098663938026383.davem@davemloft.net>
Sender: linux-wireless-owner@vger.kernel.org

On Tue, Nov 22, 2011 at 04:40:57PM -0500, David Miller wrote:
> >>> It just means that we won't notice spurious interrupts if the device
> >>> sharing the line with rt2800pci generates one.
> >>>
> >>> This change is wrong.

Ehh, I had a feeling that I was doing something wrong ... 

> >> BTW, look at it this way, if what you say is true John then what's the
> >> point
> >> in returning any specific value at all?
> >>
> >> Everyone can just return IRQ_HANDLED and everything would just work.
> >>
> >> But you know that's not the case, and that it's important that this
> >> value
> >> is returned accurately.
> > 
> > I was trying to find the thread that reported the improvement in
> > performance with this change, but failed. Is it possible that their
> > change just papered over an interrupt storm from some other device
> > that shared that interrupt?
> 
> It doesn't fix a performance problem, it fixes a problem wherein the
> IRQ line is disabled by the generic IRQ code because all handlers
> return IRQ_NONE.

According to Amir, who reports this problem patch fix the performance
(https://bugzilla.redhat.com/show_bug.cgi?id=658451),
from very bad to expected on wifi link. It can be explained, if we
disable interrupt line, driver still operate, but read TX/RX statuses
from hardware when internal driver watchdog timeout. Hence we operate,
but very slow. If we prevent to disable interrupt by IRQ controller
driver, device operate normally.

The main problem here, that we have hardware (or firmware) that
generate interrupt with empty interrupt status register, so we can
not detect if interrupt is really generated by Ralink device.

Perhaps exist good fix for that problem, i.e. we can write to some
register to stop hardware generating spurious interrupts, or exist
other way than reading status register to find out if Ralink device
generated interrupt. But that require detailed hardware knowledge,
and I'm not sure if we ever can get such informations. Also this 
could be a hardware bug, device just generate spurious interrupts
and we can not do anything about it. I looked at driver from Ralink
site, and it do exactly that, it return IRQ_HANDLED if status register
is empty.

Ok, I'm thinking about other fix now ...

Stanislaw