The SPI driver does a couple of card cleanup steps in the wrong order on
module removal. If IEEE PS is enabled, this results in the card being
left in IEEE PS mode and subsequent failures to reload the module. The
problem is that the surpriseremoved flag is set before calling
lbs_remove_card, but that function needs to issue a command to exit IEEE
PS mode (the flag blocks the command path). In addition, lbs_stop_card
should be called first because it clears out any pending commands.
Tested on a GSPI device with V9 firmware by confirming that we can
reload the module with or without IEEE PS enabled.
Also fix a warning from the wrong uint format in a printk.
Signed-off-by: Andrey Yurovsky <[email protected]>
---
drivers/net/wireless/libertas/if_spi.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/libertas/if_spi.c b/drivers/net/wireless/libertas/if_spi.c
index 923ed58..8d8bc0b 100644
--- a/drivers/net/wireless/libertas/if_spi.c
+++ b/drivers/net/wireless/libertas/if_spi.c
@@ -737,7 +737,7 @@ static int if_spi_c2h_data(struct if_spi_card *card)
goto out;
} else if (len > MRVDRV_ETH_RX_PACKET_BUFFER_SIZE) {
lbs_pr_err("%s: error: card has %d bytes of data, but "
- "our maximum skb size is %lu\n",
+ "our maximum skb size is %u\n",
__func__, len, MRVDRV_ETH_RX_PACKET_BUFFER_SIZE);
err = -EINVAL;
goto out;
@@ -1171,12 +1171,13 @@ static int __devexit libertas_spi_remove(struct spi_device *spi)
lbs_deb_spi("libertas_spi_remove\n");
lbs_deb_enter(LBS_DEB_SPI);
- priv->surpriseremoved = 1;
lbs_stop_card(priv);
+ lbs_remove_card(priv); /* will call free_netdev */
+
+ priv->surpriseremoved = 1;
free_irq(spi->irq, card);
if_spi_terminate_spi_thread(card);
- lbs_remove_card(priv); /* will call free_netdev */
if (card->pdata->teardown)
card->pdata->teardown(spi);
free_if_spi_card(card);
--
1.5.6.3
* Andrey Yurovsky | 2009-06-17 19:08:45 [-0700]:
>Also fix a warning from the wrong uint format in a printk.
>
>Signed-off-by: Andrey Yurovsky <[email protected]>
>---
> drivers/net/wireless/libertas/if_spi.c | 7 ++++---
> 1 files changed, 4 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/net/wireless/libertas/if_spi.c b/drivers/net/wireless/libertas/if_spi.c
>index 923ed58..8d8bc0b 100644
>--- a/drivers/net/wireless/libertas/if_spi.c
>+++ b/drivers/net/wireless/libertas/if_spi.c
>@@ -737,7 +737,7 @@ static int if_spi_c2h_data(struct if_spi_card *card)
> goto out;
> } else if (len > MRVDRV_ETH_RX_PACKET_BUFFER_SIZE) {
> lbs_pr_err("%s: error: card has %d bytes of data, but "
>- "our maximum skb size is %lu\n",
>+ "our maximum skb size is %u\n",
ehm, this reverts commit 9b171ffe1 aka "libertas: fix format warning" in
Dave's next tree [0]. Max skb is defined via
|#define MRVDRV_ETH_RX_PACKET_BUFFER_SIZE \
| (ETH_FRAME_LEN + sizeof(struct rxpd) \
| + MRVDRV_SNAP_HEADER_LEN + EXTRA_LEN)
and sizeof() probably says which type it is, so this is probably 32bit
vs 64bit. Wouldn't this be the perfect place for the z modifier?
[0]
http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=commitdiff;h=9b171ffe1b3004587f4a90ef293531a4a262e538
Sebastian
The SPI driver does a couple of card cleanup steps in the wrong order on
module removal. If IEEE PS is enabled, this results in the card being
left in IEEE PS mode and subsequent failures to reload the module. The
problem is that the surpriseremoved flag is set before calling
lbs_remove_card, but that function needs to issue a command to exit IEEE
PS mode (the flag blocks the command path). In addition, lbs_stop_card
should be called first because it clears out any pending commands.
Tested on a GSPI device with V9 firmware by confirming that we can
reload the module with or without IEEE PS enabled.
Also fix a warning from the wrong uint format in a printk.
V2: use z modifier, thanks Sebastian.
Signed-off-by: Andrey Yurovsky <[email protected]>
---
drivers/net/wireless/libertas/if_spi.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/libertas/if_spi.c b/drivers/net/wireless/libertas/if_spi.c
index 923ed58..8d8bc0b 100644
--- a/drivers/net/wireless/libertas/if_spi.c
+++ b/drivers/net/wireless/libertas/if_spi.c
@@ -737,7 +737,7 @@ static int if_spi_c2h_data(struct if_spi_card *card)
goto out;
} else if (len > MRVDRV_ETH_RX_PACKET_BUFFER_SIZE) {
lbs_pr_err("%s: error: card has %d bytes of data, but "
- "our maximum skb size is %lu\n",
+ "our maximum skb size is %zu\n",
__func__, len, MRVDRV_ETH_RX_PACKET_BUFFER_SIZE);
err = -EINVAL;
goto out;
@@ -1171,12 +1171,13 @@ static int __devexit libertas_spi_remove(struct spi_device *spi)
lbs_deb_spi("libertas_spi_remove\n");
lbs_deb_enter(LBS_DEB_SPI);
- priv->surpriseremoved = 1;
lbs_stop_card(priv);
+ lbs_remove_card(priv); /* will call free_netdev */
+
+ priv->surpriseremoved = 1;
free_irq(spi->irq, card);
if_spi_terminate_spi_thread(card);
- lbs_remove_card(priv); /* will call free_netdev */
if (card->pdata->teardown)
card->pdata->teardown(spi);
free_if_spi_card(card);
--
1.5.6.3
On Thu, 2009-06-18 at 09:51 -0700, Andrey Yurovsky wrote:
> The SPI driver does a couple of card cleanup steps in the wrong order on
> module removal. If IEEE PS is enabled, this results in the card being
> left in IEEE PS mode and subsequent failures to reload the module. The
> problem is that the surpriseremoved flag is set before calling
> lbs_remove_card, but that function needs to issue a command to exit IEEE
> PS mode (the flag blocks the command path). In addition, lbs_stop_card
> should be called first because it clears out any pending commands.
>
> Tested on a GSPI device with V9 firmware by confirming that we can
> reload the module with or without IEEE PS enabled.
>
> Also fix a warning from the wrong uint format in a printk.
>
> V2: use z modifier, thanks Sebastian.
>
> Signed-off-by: Andrey Yurovsky <[email protected]>
Acked-by: Dan Williams <[email protected]>
> ---
> drivers/net/wireless/libertas/if_spi.c | 7 ++++---
> 1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/libertas/if_spi.c b/drivers/net/wireless/libertas/if_spi.c
> index 923ed58..8d8bc0b 100644
> --- a/drivers/net/wireless/libertas/if_spi.c
> +++ b/drivers/net/wireless/libertas/if_spi.c
> @@ -737,7 +737,7 @@ static int if_spi_c2h_data(struct if_spi_card *card)
> goto out;
> } else if (len > MRVDRV_ETH_RX_PACKET_BUFFER_SIZE) {
> lbs_pr_err("%s: error: card has %d bytes of data, but "
> - "our maximum skb size is %lu\n",
> + "our maximum skb size is %zu\n",
> __func__, len, MRVDRV_ETH_RX_PACKET_BUFFER_SIZE);
> err = -EINVAL;
> goto out;
> @@ -1171,12 +1171,13 @@ static int __devexit libertas_spi_remove(struct spi_device *spi)
>
> lbs_deb_spi("libertas_spi_remove\n");
> lbs_deb_enter(LBS_DEB_SPI);
> - priv->surpriseremoved = 1;
>
> lbs_stop_card(priv);
> + lbs_remove_card(priv); /* will call free_netdev */
> +
> + priv->surpriseremoved = 1;
> free_irq(spi->irq, card);
> if_spi_terminate_spi_thread(card);
> - lbs_remove_card(priv); /* will call free_netdev */
> if (card->pdata->teardown)
> card->pdata->teardown(spi);
> free_if_spi_card(card);