SDIO firmware download error
Dan Williams
dcbw at redhat.com
Tue Apr 6 01:12:06 EDT 2010
On Mon, 2010-04-05 at 19:06 +0530, Alagu Sankar wrote:
> On Thu, Apr 1, 2010 at 12:41 PM, Dan Williams <dcbw at redhat.com> wrote:
> > On Wed, 2010-03-31 at 11:11 +0100, Jonathan Cameron wrote:
> >> On 03/30/10 21:29, Dan Williams wrote:
> >> > On Thu, 2010-03-11 at 13:49 +0530, Alagu Sankar wrote:
> >> >> Trying my luck with a different subject as the previous one was rejected..
> >> >>
> >> >> I am in the process of adding SDIO Support for Davinci and using the
> >> >> Libertas SDIO driver as my standard test setup. I am using the Linux
> >> >> 2.6.33-rc6 Linux kernel and the associated libertas driver. I have
> >> >> access to 8385, 8686 and 8688 cards from different vendors, with
> >> >> diferent firmware versions. I would like to get some inputs from the
> >> >> list for the following issue that I am facing.
> >> >>
> >> >> When I use the libertas driver as is, the helper download is always
> >> >> successful, but the primary firmware loading fails after the first
> >> >> transfer. I get a req_size of 17 after the initial 16 byte transfer,
> >> >> indicating that the error bit is set. For a typical working setup,
> >> >> there is a 16 byte transfer, followed by a 12 byte transfer and so on.
> >> >> If I introduce a delay of 2 milli-seconds between each transfer of
> >> >> the helper firmware, then there is no problem in downloading the
> >> >> primary firmware. Even enabling the debug message
> >> >> "lbs_deb_sdio("sending %d bytes chunk\n", chunk_size);" alone would
> >> >> result in successful firmware download.
> >> >
> >> > If we can reproduce this issue on some other platform (to determine that
> >> > the host controller is *not* the culprit) I'm not necessarily opposed to
> >> > adding a small delay during helper firmware download in the driver. But
> >> > beyond adding some debugging prints to the MMC layer to conclusively
> >> > determine that the HC is responding correctly or incorrectly to the
> >> > transfers, I'm not really sure. Are you aware of any errata for your
> >> > SDIO controller that might affect this?
> >> A similar problem exists on a pxa271. I'm carrying the following patch
> >> for it... Note this is against a fairly old kernel, 2.6.32-rc6
> >>
> >> Symptoms were exactly the same though. There was a previous thread on this
> >> list about it:
> >>
> >> sdio 8686 firmware load problem
> >>
> >> back on the 11th of August last year.
> >
> > Any chance if you see the issue you could make the mdelay() just a few
> > lines higher inside the while (1) loop into mdelay(2) ? If that doesn't
> > work, move the mdelay(1) in the while(1) loop up somewhere above the "if
> > ((status & IF_SDIO_IO_RDY) &&" part.
> >
> > Dan
> >
>
> Tried increasing the mdelay(1) upto mdelay(5) and it did not help. But
> moving the mdelay(1) above the "if ((status & IF_SDIO_IO_RDY) &&" part
> and increasing it to mdelay(2) resolved the issues on Davinci
> platforms.
Can you test the following patch for me? It works on both a Marvell
8686 developer board and the Wi2Wi 8686 board with my Ricoh laptop SD
controller (from which I'm writing this mail).
The fact that the delay worked for you there means that either the
firmware or your SDHC needs (a) a short delay before banging the STATUS
register, and/or (b) a short delay after banging that status register
before trying to write a packet to the card. Or something like that.
Let me know if the patch works and if so I'll forward it to John.
Dan
---
diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
index 7a73f62..61a46df 100644
--- a/drivers/net/wireless/libertas/if_sdio.c
+++ b/drivers/net/wireless/libertas/if_sdio.c
@@ -312,12 +312,33 @@ out:
return ret;
}
+static int if_sdio_wait_status(struct if_sdio_card *card, const u8 condition)
+{
+ u8 status;
+ unsigned long timeout;
+ int ret = 0;
+
+ timeout = jiffies + HZ;
+ while (1) {
+ status = sdio_readb(card->func, IF_SDIO_STATUS, &ret);
+
+ /* Short delay between reads which some platforms and firmware
+ * versions (like Davinci) seem to need.
+ */
+ mdelay(2);
+
+ if (ret || (status & condition))
+ break;
+ if (time_after(jiffies, timeout))
+ return -ETIMEDOUT;
+ }
+ return ret;
+}
+
static int if_sdio_card_to_host(struct if_sdio_card *card)
{
int ret;
- u8 status;
u16 size, type, chunk;
- unsigned long timeout;
lbs_deb_enter(LBS_DEB_SDIO);
@@ -332,19 +353,9 @@ static int if_sdio_card_to_host(struct if_sdio_card *card)
goto out;
}
- timeout = jiffies + HZ;
- while (1) {
- status = sdio_readb(card->func, IF_SDIO_STATUS, &ret);
- if (ret)
- goto out;
- if (status & IF_SDIO_IO_RDY)
- break;
- if (time_after(jiffies, timeout)) {
- ret = -ETIMEDOUT;
- goto out;
- }
- mdelay(1);
- }
+ ret = if_sdio_wait_status(card, IF_SDIO_IO_RDY);
+ if (ret)
+ goto out;
/*
* The transfer must be in one transaction or the firmware
@@ -411,8 +422,6 @@ static void if_sdio_host_to_card_worker(struct work_struct *work)
{
struct if_sdio_card *card;
struct if_sdio_packet *packet;
- unsigned long timeout;
- u8 status;
int ret;
unsigned long flags;
@@ -432,25 +441,15 @@ static void if_sdio_host_to_card_worker(struct work_struct *work)
sdio_claim_host(card->func);
- timeout = jiffies + HZ;
- while (1) {
- status = sdio_readb(card->func, IF_SDIO_STATUS, &ret);
- if (ret)
- goto release;
- if (status & IF_SDIO_IO_RDY)
- break;
- if (time_after(jiffies, timeout)) {
- ret = -ETIMEDOUT;
- goto release;
- }
- mdelay(1);
+ ret = if_sdio_wait_status(card, IF_SDIO_IO_RDY);
+ if (ret == 0) {
+ ret = sdio_writesb(card->func, card->ioport,
+ packet->buffer, packet->nb);
}
- ret = sdio_writesb(card->func, card->ioport,
- packet->buffer, packet->nb);
if (ret)
- goto release;
-release:
+ lbs_pr_err("error %d sending packet to firmware\n", ret);
+
sdio_release_host(card->func);
kfree(packet);
@@ -463,10 +462,11 @@ release:
/* Firmware */
/********************************************************************/
+#define FW_DL_READY_STATUS (IF_SDIO_IO_RDY | IF_SDIO_DL_RDY)
+
static int if_sdio_prog_helper(struct if_sdio_card *card)
{
int ret;
- u8 status;
const struct firmware *fw;
unsigned long timeout;
u8 *chunk_buffer;
@@ -498,20 +498,9 @@ static int if_sdio_prog_helper(struct if_sdio_card *card)
size = fw->size;
while (size) {
- timeout = jiffies + HZ;
- while (1) {
- status = sdio_readb(card->func, IF_SDIO_STATUS, &ret);
- if (ret)
- goto release;
- if ((status & IF_SDIO_IO_RDY) &&
- (status & IF_SDIO_DL_RDY))
- break;
- if (time_after(jiffies, timeout)) {
- ret = -ETIMEDOUT;
- goto release;
- }
- mdelay(1);
- }
+ ret = if_sdio_wait_status(card, FW_DL_READY_STATUS);
+ if (ret)
+ goto release;
chunk_size = min(size, (size_t)60);
@@ -581,7 +570,6 @@ out:
static int if_sdio_prog_real(struct if_sdio_card *card)
{
int ret;
- u8 status;
const struct firmware *fw;
unsigned long timeout;
u8 *chunk_buffer;
@@ -613,20 +601,9 @@ static int if_sdio_prog_real(struct if_sdio_card *card)
size = fw->size;
while (size) {
- timeout = jiffies + HZ;
- while (1) {
- status = sdio_readb(card->func, IF_SDIO_STATUS, &ret);
- if (ret)
- goto release;
- if ((status & IF_SDIO_IO_RDY) &&
- (status & IF_SDIO_DL_RDY))
- break;
- if (time_after(jiffies, timeout)) {
- ret = -ETIMEDOUT;
- goto release;
- }
- mdelay(1);
- }
+ ret = if_sdio_wait_status(card, FW_DL_READY_STATUS);
+ if (ret)
+ goto release;
req_size = sdio_readb(card->func, IF_SDIO_RD_BASE, &ret);
if (ret)
More information about the libertas-dev
mailing list