[PATCH] libertas_sdio lockup fix

[Dan Williams]

On Thu, 2010-04-01 at 09:31 +0200, Jes Sorensen wrote:
> On 04/01/10 09:06, Dan Williams wrote:
> > We're trying to kill lbs_prepare_and_send_command() completely though
> > and move most commands to separate handlers.  Any chance you could port
> > the patch to do that instead?  Basically create a function modeled after
> > cmd.c::lbs_get_tx_power()
> 
> Hi Dan,
> 
> Thanks for the feedback. The problem I had was that I know zilch about
> the libertas chips, so I was guessing my way through based on the old
> patch that I found.
> 
> I'll try and setup a build environment and see if I can reproduce it as
> the last test I did was a full rpm build. My test box is a 1.1GHz with
> 16GB SSD so it's not really up for kernel builds.

It should be pretty easy to just grab the kernel SRPM, copy out the
drivers/net/wireless/libertas directory, then inside that directory you
can:

make -C /lib/modules/`uname -r`/build SUBDIRS=`pwd` modules

and then you can insmod the .ko files from that directory.  No need to
do a full kernel build.

> > int lbs_get_log (struct lbs_private *priv, u32 *retries, u32 *code, u32 *misc)
> > {
> > 	struct cmd_ds_802_11_get_log cmd;
> > 	int ret;
> >
> > 	lbs_deb_enter(LBS_DEB_CMD);
> >
> > 	memset(&cmd, 0, sizeof(cmd));
> > 	cmd.hdr.size = cpu_to_le16(sizeof(cmd));
> >
> > 	ret = lbs_cmd_with_response(priv, CMD_802_11_GET_LOG,&cmd);
> > 	if (ret == 0) {
> > 		*retries = get_unaligned_le32(cmd.retry);
> > 		*code = get_unaligned_le32(cmd.code);
> > 		*misc = get_unaligned_le32(cmd.misc);
> > 	}
> >
> > 	lbs_deb_leave(LBS_DEB_CMD);
> > 	return ret;
> > }
> >
> > Then at the bottom of the function call lbs_get_log() and use the
> > returned u32 values to populate priv->wstats.  But really, it looks like
> > the only functional changes here are:
> 
> This part looks easy enough.
> 
> > 1) the cmd size passed to the 8686 is larger with this patch, since you
> > haven't removed the header from struct cmd_ds_802_11_get_log; it could
> > be that we got the size slightly wrong or something when converting it
> > to a "direct" command?
> >     - pad the structure in host.h by another u32 at the bottom and see if
> > this helps
> 
> The size in my patch was pure guessware to be honest. The original patch
> used sizeof(struct cmd_ds_802_11_get_log) + S_DS_GEN and I didn't find
> S_DS_GEN anywhere.
> 
> > 2) calling GET_LOG after RSSI.  Not sure why that would make a
> > difference, but it could
> >     - do the "direct" command thing above but move the lbs_get_log() call
> > after the RSSI call
> 
> I think I can figure this out.
> 
> > 3) not calling GET_LOG through the blocking command path during the WEXT
> > stats requests
> >     - call GET_LOG async and just ignore updating the wstats values to
> > see if the blocking nature of "direct" commands is indeed the issue
> > since the wext stats stuff gets called in different places with
> > different locking semantics
> 
> I am not sure if I follow you here. Is there an easy async path for this
> or do I need to come up with something?

__lbs_cmd_async is the magic function for firing off an async command
and ignoring the result.

> > Any chance you could try these three options and see if they also fix
> > the problem?  I'd like to figure out why this is happing instead of just
> > apply what looks like a partial revert first.
> 
> I am happy to help as much as I can. Main problem is that the libertas
> codebase is pretty unknown to me.

Let me know if you have any more questions!

Dan