2009-05-20 00:20:13

by Andrey Yurovsky

[permalink] [raw]
Subject: libertas: fix GSPI card event handling

The GPSI interface driver does not re-enable the Card Event Interrupt, which
causes problems after a card event (for example: link-loss) comes in. This
can lead, for example, to the card failing to re-associate. This patch
ensures that we re-enable the Card Event Interrupt when we handle card events.

Signed-off-by: Andrey Yurovsky <[email protected]>
diff --git a/drivers/net/wireless/libertas/if_spi.c b/drivers/net/wireless/libertas/if_spi.c
index dccd01f..5fa55fe 100644
--- a/drivers/net/wireless/libertas/if_spi.c
+++ b/drivers/net/wireless/libertas/if_spi.c
@@ -814,6 +814,13 @@ static void if_spi_e2h(struct if_spi_card *card)
if (err)
goto out;

+ /* re-enable the card event interrupt */
+ spu_write_u16(card, IF_SPI_HOST_INT_STATUS_REG,
+ ~IF_SPI_HICU_CARD_EVENT);
+
+ /* generate a card interrupt */
+ spu_write_u16(card, IF_SPI_CARD_INT_CAUSE_REG, IF_SPI_CIC_HOST_EVENT);
+
spin_lock_irqsave(&priv->driver_lock, flags);
lbs_queue_event(priv, cause & 0xff);
spin_unlock_irqrestore(&priv->driver_lock, flags);




2009-05-20 22:18:17

by Dan Williams

[permalink] [raw]
Subject: Re: libertas: fix GSPI card event handling

On Tue, 2009-05-19 at 17:20 -0700, [email protected] wrote:
> The GPSI interface driver does not re-enable the Card Event Interrupt, which
> causes problems after a card event (for example: link-loss) comes in. This
> can lead, for example, to the card failing to re-associate. This patch
> ensures that we re-enable the Card Event Interrupt when we handle card events.
>
> Signed-off-by: Andrey Yurovsky <[email protected]>

Where does that bit ever get disabled? Is that done by the firmware
after it sends the interrupt? I don't think I quite understand what's
going on here...

Dan

> diff --git a/drivers/net/wireless/libertas/if_spi.c b/drivers/net/wireless/libertas/if_spi.c
> index dccd01f..5fa55fe 100644
> --- a/drivers/net/wireless/libertas/if_spi.c
> +++ b/drivers/net/wireless/libertas/if_spi.c
> @@ -814,6 +814,13 @@ static void if_spi_e2h(struct if_spi_card *card)
> if (err)
> goto out;
>
> + /* re-enable the card event interrupt */
> + spu_write_u16(card, IF_SPI_HOST_INT_STATUS_REG,
> + ~IF_SPI_HICU_CARD_EVENT);
> +
> + /* generate a card interrupt */
> + spu_write_u16(card, IF_SPI_CARD_INT_CAUSE_REG, IF_SPI_CIC_HOST_EVENT);
> +
> spin_lock_irqsave(&priv->driver_lock, flags);
> lbs_queue_event(priv, cause & 0xff);
> spin_unlock_irqrestore(&priv->driver_lock, flags);
>
>


2009-05-20 22:30:03

by Dan Williams

[permalink] [raw]
Subject: Re: libertas: fix GSPI card event handling

On Wed, 2009-05-20 at 15:28 -0700, Andrey Yurovsky wrote:
> On Wed, May 20, 2009 at 3:18 PM, Dan Williams <[email protected]> wrote:
> > On Tue, 2009-05-19 at 17:20 -0700, [email protected] wrote:
> >> The GPSI interface driver does not re-enable the Card Event Interrupt, which
> >> causes problems after a card event (for example: link-loss) comes in. This
> >> can lead, for example, to the card failing to re-associate. This patch
> >> ensures that we re-enable the Card Event Interrupt when we handle card events.
> >>
> >> Signed-off-by: Andrey Yurovsky <[email protected]>
> >
> > Where does that bit ever get disabled? Is that done by the firmware
> > after it sends the interrupt? I don't think I quite understand what's
> > going on here...
>
> That's correct. The firmware and host communicate by setting or
> clearing bits and forcing an interrupt. The event mechanism is,
> basically:
> 1) firmware sets that bit and triggers an IRQ
> 2) host handles the IRQ (noticing that it's an Event and reading the
> Event Cause)
> 3) host clears the bit
> 4) host triggers an interrupt (on the card) so that the card notices
> the change (ie: that the bit was cleared)

Ah, step 4 was what I wasn't yet thinking of. Sounds fine.

Acked-by: Dan Williams <[email protected]>

> Steps 3 an 4 were missing in if_spi.c, which caused problems if, for
> example, you associate to an AP and then cut the AP's power. In this
> case the card will never re-associate subsequently because the Card
> Event bit never gets cleared.
>
> >> diff --git a/drivers/net/wireless/libertas/if_spi.c b/drivers/net/wireless/libertas/if_spi.c
> >> index dccd01f..5fa55fe 100644
> >> --- a/drivers/net/wireless/libertas/if_spi.c
> >> +++ b/drivers/net/wireless/libertas/if_spi.c
> >> @@ -814,6 +814,13 @@ static void if_spi_e2h(struct if_spi_card *card)
> >> if (err)
> >> goto out;
> >>
> >> + /* re-enable the card event interrupt */
> >> + spu_write_u16(card, IF_SPI_HOST_INT_STATUS_REG,
> >> + ~IF_SPI_HICU_CARD_EVENT);
> >> +
> >> + /* generate a card interrupt */
> >> + spu_write_u16(card, IF_SPI_CARD_INT_CAUSE_REG, IF_SPI_CIC_HOST_EVENT);
> >> +
> >> spin_lock_irqsave(&priv->driver_lock, flags);
> >> lbs_queue_event(priv, cause & 0xff);
> >> spin_unlock_irqrestore(&priv->driver_lock, flags);
> >>
> >>
> >
> >
>
>
>


2009-05-20 22:28:14

by Andrey Yurovsky

[permalink] [raw]
Subject: Re: libertas: fix GSPI card event handling

On Wed, May 20, 2009 at 3:18 PM, Dan Williams <[email protected]> wrote:
> On Tue, 2009-05-19 at 17:20 -0700, [email protected] wrote:
>> The GPSI interface driver does not re-enable the Card Event Interrupt, which
>> causes problems after a card event (for example: link-loss) comes in. ?This
>> can lead, for example, to the card failing to re-associate. ?This patch
>> ensures that we re-enable the Card Event Interrupt when we handle card events.
>>
>> Signed-off-by: Andrey Yurovsky <[email protected]>
>
> Where does that bit ever get disabled? ?Is that done by the firmware
> after it sends the interrupt? ?I don't think I quite understand what's
> going on here...

That's correct. The firmware and host communicate by setting or
clearing bits and forcing an interrupt. The event mechanism is,
basically:
1) firmware sets that bit and triggers an IRQ
2) host handles the IRQ (noticing that it's an Event and reading the
Event Cause)
3) host clears the bit
4) host triggers an interrupt (on the card) so that the card notices
the change (ie: that the bit was cleared)

Steps 3 an 4 were missing in if_spi.c, which caused problems if, for
example, you associate to an AP and then cut the AP's power. In this
case the card will never re-associate subsequently because the Card
Event bit never gets cleared.

>> diff --git a/drivers/net/wireless/libertas/if_spi.c b/drivers/net/wireless/libertas/if_spi.c
>> index dccd01f..5fa55fe 100644
>> --- a/drivers/net/wireless/libertas/if_spi.c
>> +++ b/drivers/net/wireless/libertas/if_spi.c
>> @@ -814,6 +814,13 @@ static void if_spi_e2h(struct if_spi_card *card)
>> ? ? ? if (err)
>> ? ? ? ? ? ? ? goto out;
>>
>> + ? ? /* re-enable the card event interrupt */
>> + ? ? spu_write_u16(card, IF_SPI_HOST_INT_STATUS_REG,
>> + ? ? ? ? ? ? ? ? ? ? ~IF_SPI_HICU_CARD_EVENT);
>> +
>> + ? ? /* generate a card interrupt */
>> + ? ? spu_write_u16(card, IF_SPI_CARD_INT_CAUSE_REG, IF_SPI_CIC_HOST_EVENT);
>> +
>> ? ? ? spin_lock_irqsave(&priv->driver_lock, flags);
>> ? ? ? lbs_queue_event(priv, cause & 0xff);
>> ? ? ? spin_unlock_irqrestore(&priv->driver_lock, flags);
>>
>>
>
>



--
Andrey Yurovsky
cozybit Inc.