2011-04-08 09:15:19

by Nishant Sarmukadam

Subject: [PATCH] mwl8k: interrupt handling changes

We do not need to enable all the interrupts in mwl8k_probe_hw.
We need to enable only MWL8K_A2H_INT_OPC_DONE interrupt for sending
commands to the firmware. Keep the other interrupts masked in
mwl8k_probe_hw. Also, in mwl8k_start, where we expect other interrupts,
enable only those interrupts we are interested in.

Signed-off-by: Nishant Sarmukadam <[email protected]>
---
drivers/net/wireless/mwl8k.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/mwl8k.c b/drivers/net/wireless/mwl8k.c
index 8913180..07e8134 100644
--- a/drivers/net/wireless/mwl8k.c
+++ b/drivers/net/wireless/mwl8k.c
@@ -4285,6 +4285,8 @@ static int mwl8k_start(struct ieee80211_hw *hw)

/* Enable interrupts */
iowrite32(MWL8K_A2H_EVENTS, priv->regs + MWL8K_HIU_A2H_INTERRUPT_MASK);
+ iowrite32(MWL8K_A2H_EVENTS,
+ priv->regs + MWL8K_HIU_A2H_INTERRUPT_STATUS_MASK);

rc = mwl8k_fw_lock(hw);
if (!rc) {
@@ -5283,7 +5285,8 @@ static int mwl8k_probe_hw(struct ieee80211_hw *hw)
iowrite32(MWL8K_A2H_INT_TX_DONE|MWL8K_A2H_INT_RX_READY|
MWL8K_A2H_INT_BA_WATCHDOG,
priv->regs + MWL8K_HIU_A2H_INTERRUPT_CLEAR_SEL);
- iowrite32(0xffffffff, priv->regs + MWL8K_HIU_A2H_INTERRUPT_STATUS_MASK);
+ iowrite32(MWL8K_A2H_INT_OPC_DONE,
+ priv->regs + MWL8K_HIU_A2H_INTERRUPT_STATUS_MASK);

rc = request_irq(priv->pdev->irq, mwl8k_interrupt,
IRQF_SHARED, MWL8K_NAME, hw);
--
1.6.0.3

2011-04-08 18:17:15

by Lennert Buytenhek

Subject: Re: [PATCH] mwl8k: interrupt handling changes

On Fri, Apr 08, 2011 at 02:38:27PM +0530, Nishant Sarmukadam wrote:

> We do not need to enable all the interrupts in mwl8k_probe_hw.
> We need to enable only MWL8K_A2H_INT_OPC_DONE interrupt for sending
> commands to the firmware. Keep the other interrupts masked in
> mwl8k_probe_hw.

This part of the description matches the patch. But..


> Also, in mwl8k_start, where we expect other interrupts,
> enable only those interrupts we are interested in.

..this comment does not match the code.


> /* Enable interrupts */
> iowrite32(MWL8K_A2H_EVENTS, priv->regs + MWL8K_HIU_A2H_INTERRUPT_MASK);
> + iowrite32(MWL8K_A2H_EVENTS,
> + priv->regs + MWL8K_HIU_A2H_INTERRUPT_STATUS_MASK);

This writes to the status register. Why does it do that?

2011-04-11 06:23:01

by Nishant Sarmukadam

Subject: Re: [PATCH] mwl8k: interrupt handling changes

On Fri, 2011-04-08 at 11:19 -0700, Lennert Buytenhek wrote:
> On Fri, Apr 08, 2011 at 02:38:27PM +0530, Nishant Sarmukadam wrote:
>
> > We do not need to enable all the interrupts in mwl8k_probe_hw.
> > We need to enable only MWL8K_A2H_INT_OPC_DONE interrupt for sending
> > commands to the firmware. Keep the other interrupts masked in
> > mwl8k_probe_hw.
>
> This part of the description matches the patch. But..
>
>
> > Also, in mwl8k_start, where we expect other interrupts,
> > enable only those interrupts we are interested in.
>
> ..this comment does not match the code.
>
>
> > /* Enable interrupts */
> > iowrite32(MWL8K_A2H_EVENTS, priv->regs + MWL8K_HIU_A2H_INTERRUPT_MASK);
> > + iowrite32(MWL8K_A2H_EVENTS,
> > + priv->regs + MWL8K_HIU_A2H_INTERRUPT_STATUS_MASK);
>
> This writes to the status register. Why does it do that?

Here we are writing to the 'status mask' register and not the 'status'
register. Interrupts will not be enabled until we write 1's in the
status mask register. Earlier, we were writing 0xffffffff in this
register, whereas enabling only interrupts specified in MWL8K_A2H_EVENTS
should be sufficient since driver is interested in only those
interrupts. Hence the description.