[PATCH] libertas: Fix sd8686 firmware reload

[Steve deRosier]

On Wed, Oct 13, 2010 at 9:15 AM, Dan Williams <dcbw at redhat.com> wrote:
> On Sun, 2010-10-10 at 10:53 +0100, Daniel Drake wrote:
>> From: Paul Fox <pgf at laptop.org>
>>
>> The firmware presence check is not fully reliable. Broaden the check
>> to see if the firmware is running.
>
> I'm not against this per-se, but the firmware spec clearly indicates
> that the procedure for checking whether firmware is active is to read
> FN0 and FN1 and look for 0xDC and 0xFE respectively.  This patch
> modifies that for sd8686 to look for *any* value.  Second, Marvell
> reference SD8686 driver "SD-8686-LINUX26-MONAHANS-9.70.3.p30-26409.P51"
> also looks only for 0xFEDC, as does the Motorola A980 SD8385 driver
> version 138.p23.

Dan,

As I was the originator of this check (for libertas_tf_sdio), I
thought I'd weigh in on the discussion.  The check for 0xFEDC doesn't
quite work, at least on the sd8686.  The scratch register is indeed
set to 0xFEDC after the firmware is loaded, but if the firmware and
driver have been working for some time, the register contains the rx
packet length, on the sd8686 specifically, as the comment on
if_sdio_read_scratch() tells us:
/*
 *  For SD8385/SD8686, this function reads firmware status after
 *  the image is downloaded, or reads RX packet length when
 *  interrupt (with IF_SDIO_H_INT_UPLD bit set) is received.
 */

So, we ran into the problem that if we unloaded the module and
reloaded the module, it couldn't get past this point and probe failed,
as it seemed firmware wasn't running, yet it actually was, and trying
to reload the firmware to a running firmware doesn't work well.

So the existing check was flawed and I translated it to the one that
Dan and Paul eventually moved to libertas.  The new check is intended
to see if there's a rx packet length sitting in the register and if
so, assume the firmware is running and try to communicate to it.
There are other outs later in the code if it doesn't work.

I don't know if Marvell will say it's the "right way", but it works
very well in practice.

I have one issue with the patch.  Due to testing that the Gumstix
community is doing with libertas_tf, we've found that at least on that
platform, the high bit is always set. We've subsequently changed the
check to mask out the high bit:
	} else if ((card->model == IF_SDIO_MODEL_8686) && ((scratch & 0x7fff) != 0)) {

- Steve