2010-10-27 14:21:00

by Daniel Drake

[permalink] [raw]
Subject: [PATCH v2] libertas: Fix sd8686 firmware reload

From: Paul Fox <[email protected]>

For the SD8686, we cannot rely on the scratch register to read the firmware
load status, because the same register is used for storing RX packet length.
Broaden the check to account for this.

The module can now be unloaded/reloaded successfully.

Based on the implementation from libertas_tf.

Signed-off-by: Daniel Drake <[email protected]>
---
drivers/net/wireless/libertas/if_sdio.c | 32 ++++++++++++++++++++++++++++--
1 files changed, 29 insertions(+), 3 deletions(-)

v2: rebase on latest pull to overcome a trivial conflict generated by another
recent change

diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
index 296fd00..f41a5ab 100644
--- a/drivers/net/wireless/libertas/if_sdio.c
+++ b/drivers/net/wireless/libertas/if_sdio.c
@@ -684,18 +684,40 @@ static int if_sdio_prog_firmware(struct if_sdio_card *card)

lbs_deb_enter(LBS_DEB_SDIO);

+ /*
+ * Disable interrupts
+ */
+ sdio_claim_host(card->func);
+ sdio_writeb(card->func, 0x00, IF_SDIO_H_INT_MASK, &ret);
+ sdio_release_host(card->func);
+
sdio_claim_host(card->func);
scratch = if_sdio_read_scratch(card, &ret);
sdio_release_host(card->func);

+ lbs_deb_sdio("firmware status = %#x\n", scratch);
+ lbs_deb_sdio("scratch ret = %d\n", ret);
+
if (ret)
goto out;

- lbs_deb_sdio("firmware status = %#x\n", scratch);

+ /*
+ * The manual clearly describes that FEDC is the right code to use
+ * to detect firmware presence, but for SD8686 it is not that simple.
+ * Scratch is also used to store the RX packet length, so we lose
+ * the FEDC value early on. So we use a non-zero check in order
+ * to validate firmware presence.
+ * Additionally, the SD8686 in the Gumstix always has the high scratch
+ * bit set, even when the firmware is not loaded. So we have to
+ * exclude that from the test.
+ */
if (scratch == IF_SDIO_FIRMWARE_OK) {
lbs_deb_sdio("firmware already loaded\n");
goto success;
+ } else if ((card->model == IF_SDIO_MODEL_8686) && (scratch & 0x7fff)) {
+ lbs_deb_sdio("firmware may be running\n");
+ goto success;
}

ret = lbs_get_firmware(&card->func->dev, lbs_helper_name, lbs_fw_name,
@@ -709,10 +731,14 @@ static int if_sdio_prog_firmware(struct if_sdio_card *card)
if (ret)
goto out;

+ lbs_deb_sdio("Helper firmware loaded\n");
+
ret = if_sdio_prog_real(card, mainfw);
if (ret)
goto out;

+ lbs_deb_sdio("Firmware loaded\n");
+
success:
sdio_claim_host(card->func);
sdio_set_block_size(card->func, IF_SDIO_BLOCK_SIZE);
@@ -1042,8 +1068,6 @@ static int if_sdio_probe(struct sdio_func *func,
priv->exit_deep_sleep = if_sdio_exit_deep_sleep;
priv->reset_deep_sleep_wakeup = if_sdio_reset_deep_sleep_wakeup;

- priv->fw_ready = 1;
-
sdio_claim_host(func);

/*
@@ -1064,6 +1088,8 @@ static int if_sdio_probe(struct sdio_func *func,
if (ret)
goto reclaim;

+ priv->fw_ready = 1;
+
/*
* FUNC_INIT is required for SD8688 WLAN/BT multiple functions
*/
--
1.7.2.3



2010-10-28 13:57:57

by Steve deRosier

[permalink] [raw]
Subject: Re: [PATCH v2] libertas: Fix sd8686 firmware reload

On Wed, Oct 27, 2010 at 7:20 AM, Daniel Drake <[email protected]> wrote:
> From: Paul Fox <[email protected]>
>
> For the SD8686, we cannot rely on the scratch register to read the firmware
> load status, because the same register is used for storing RX packet length.
> Broaden the check to account for this.
>
> The module can now be unloaded/reloaded successfully.
>
> Based on the implementation from libertas_tf.
>
> Signed-off-by: Daniel Drake <[email protected]>

Signed-off-by: Steve deRosier <[email protected]>

2010-10-28 05:57:18

by Chris Ball

[permalink] [raw]
Subject: Re: [PATCH v2] libertas: Fix sd8686 firmware reload

Hi,

On Wed, Oct 27, 2010 at 03:54:27PM -0400, John W. Linville wrote:
> CC [M] drivers/net/wireless/libertas/if_sdio.o
> drivers/net/wireless/libertas/if_sdio.c: In function ‘if_sdio_prog_firmware’:
> drivers/net/wireless/libertas/if_sdio.c:718: error: ‘IF_SDIO_MODEL_8686’ undeclared (first use in this function)
> drivers/net/wireless/libertas/if_sdio.c:718: error: (Each undeclared identifier is reported only once
> drivers/net/wireless/libertas/if_sdio.c:718: error: for each function it appears in.)
>
> What tree has this definition?

Odd, it's been upstream in if_sdio.h since 2.6.31; mainline commit e70a5ac5.

- Chris.
--
Chris Ball <[email protected]> <http://printf.net/>
One Laptop Per Child

2010-10-28 14:15:10

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH v2] libertas: Fix sd8686 firmware reload

On Thu, Oct 28, 2010 at 06:57:04AM +0100, Chris Ball wrote:
> Hi,
>
> On Wed, Oct 27, 2010 at 03:54:27PM -0400, John W. Linville wrote:
> > CC [M] drivers/net/wireless/libertas/if_sdio.o
> > drivers/net/wireless/libertas/if_sdio.c: In function ‘if_sdio_prog_firmware’:
> > drivers/net/wireless/libertas/if_sdio.c:718: error: ‘IF_SDIO_MODEL_8686’ undeclared (first use in this function)
> > drivers/net/wireless/libertas/if_sdio.c:718: error: (Each undeclared identifier is reported only once
> > drivers/net/wireless/libertas/if_sdio.c:718: error: for each function it appears in.)
> >
> > What tree has this definition?
>
> Odd, it's been upstream in if_sdio.h since 2.6.31; mainline commit e70a5ac5.

Apparently you guys are missing 3d32a58b87cd251b50842f93b87d5458061c0cfc.

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2010-10-27 20:00:17

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH v2] libertas: Fix sd8686 firmware reload

On Wed, Oct 27, 2010 at 03:20:52PM +0100, Daniel Drake wrote:
> From: Paul Fox <[email protected]>
>
> For the SD8686, we cannot rely on the scratch register to read the firmware
> load status, because the same register is used for storing RX packet length.
> Broaden the check to account for this.
>
> The module can now be unloaded/reloaded successfully.
>
> Based on the implementation from libertas_tf.
>
> Signed-off-by: Daniel Drake <[email protected]>

CC [M] drivers/net/wireless/libertas/if_sdio.o
drivers/net/wireless/libertas/if_sdio.c: In function ‘if_sdio_prog_firmware’:
drivers/net/wireless/libertas/if_sdio.c:718: error: ‘IF_SDIO_MODEL_8686’ undeclared (first use in this function)
drivers/net/wireless/libertas/if_sdio.c:718: error: (Each undeclared identifier is reported only once
drivers/net/wireless/libertas/if_sdio.c:718: error: for each function it appears in.)

What tree has this definition?

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2010-10-28 13:46:34

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH v2] libertas: Fix sd8686 firmware reload

On Wed, 2010-10-27 at 15:20 +0100, Daniel Drake wrote:
> From: Paul Fox <[email protected]>
>
> For the SD8686, we cannot rely on the scratch register to read the firmware
> load status, because the same register is used for storing RX packet length.
> Broaden the check to account for this.
>
> The module can now be unloaded/reloaded successfully.
>
> Based on the implementation from libertas_tf.
>
> Signed-off-by: Daniel Drake <[email protected]>

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

> ---
> drivers/net/wireless/libertas/if_sdio.c | 32 ++++++++++++++++++++++++++++--
> 1 files changed, 29 insertions(+), 3 deletions(-)
>
> v2: rebase on latest pull to overcome a trivial conflict generated by another
> recent change
>
> diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
> index 296fd00..f41a5ab 100644
> --- a/drivers/net/wireless/libertas/if_sdio.c
> +++ b/drivers/net/wireless/libertas/if_sdio.c
> @@ -684,18 +684,40 @@ static int if_sdio_prog_firmware(struct if_sdio_card *card)
>
> lbs_deb_enter(LBS_DEB_SDIO);
>
> + /*
> + * Disable interrupts
> + */
> + sdio_claim_host(card->func);
> + sdio_writeb(card->func, 0x00, IF_SDIO_H_INT_MASK, &ret);
> + sdio_release_host(card->func);
> +
> sdio_claim_host(card->func);
> scratch = if_sdio_read_scratch(card, &ret);
> sdio_release_host(card->func);
>
> + lbs_deb_sdio("firmware status = %#x\n", scratch);
> + lbs_deb_sdio("scratch ret = %d\n", ret);
> +
> if (ret)
> goto out;
>
> - lbs_deb_sdio("firmware status = %#x\n", scratch);
>
> + /*
> + * The manual clearly describes that FEDC is the right code to use
> + * to detect firmware presence, but for SD8686 it is not that simple.
> + * Scratch is also used to store the RX packet length, so we lose
> + * the FEDC value early on. So we use a non-zero check in order
> + * to validate firmware presence.
> + * Additionally, the SD8686 in the Gumstix always has the high scratch
> + * bit set, even when the firmware is not loaded. So we have to
> + * exclude that from the test.
> + */
> if (scratch == IF_SDIO_FIRMWARE_OK) {
> lbs_deb_sdio("firmware already loaded\n");
> goto success;
> + } else if ((card->model == IF_SDIO_MODEL_8686) && (scratch & 0x7fff)) {
> + lbs_deb_sdio("firmware may be running\n");
> + goto success;
> }
>
> ret = lbs_get_firmware(&card->func->dev, lbs_helper_name, lbs_fw_name,
> @@ -709,10 +731,14 @@ static int if_sdio_prog_firmware(struct if_sdio_card *card)
> if (ret)
> goto out;
>
> + lbs_deb_sdio("Helper firmware loaded\n");
> +
> ret = if_sdio_prog_real(card, mainfw);
> if (ret)
> goto out;
>
> + lbs_deb_sdio("Firmware loaded\n");
> +
> success:
> sdio_claim_host(card->func);
> sdio_set_block_size(card->func, IF_SDIO_BLOCK_SIZE);
> @@ -1042,8 +1068,6 @@ static int if_sdio_probe(struct sdio_func *func,
> priv->exit_deep_sleep = if_sdio_exit_deep_sleep;
> priv->reset_deep_sleep_wakeup = if_sdio_reset_deep_sleep_wakeup;
>
> - priv->fw_ready = 1;
> -
> sdio_claim_host(func);
>
> /*
> @@ -1064,6 +1088,8 @@ static int if_sdio_probe(struct sdio_func *func,
> if (ret)
> goto reclaim;
>
> + priv->fw_ready = 1;
> +
> /*
> * FUNC_INIT is required for SD8688 WLAN/BT multiple functions
> */