Re: [rfc patch] libertas: fix if_spi_prog_helper_firmware()

[Paul Fox <pgf@laptop.org>]

dan wrote:
 > On Tue, 2010-08-24 at 14:07 +0200, Dan Carpenter wrote:
 > > The indenting is not correct here.  I don't have this hardware and I'm
 > > just guessing as to what was intended.  I think that if there is an
 > > error we should return an error code, but if there isn't an error we
 > > should return success directly without releasing the firmware.
 > > 
 > > Signed-off-by: Dan Carpenter <error27@gmail.com>
 > 
 > I've significantly changed firmware loading in wireless-testing which
 > should hit the next merge window.  It won't have this problem, and it
 > does correctly release the firmware later on.  It does preserve the
 > existing behavior of releasing the firmware after load, instead of
 > keeping it around for resume.  If there are fixes I think they should be
 > against wireless-testing actually since that's what's "next".

as part of the firmware and suspend/resume topic:  on the XO 1.5
laptop, running 2.6.31, we see an apparent leak of the firmware on
every suspend/resume where the card is powered down.  (our driver
keeps the wlan module powered acros s/r if there are wakeup events
configured, otherwise it powers down -- i can't remember if that change
has been upstreamed or not.) our trac ticket is here: 
http://dev.laptop.org/ticket/9928

i got as far as deciding that the leak wasn't in the libertas driver
itself before we decided we had more important fish to fry, so our
investigation is/was incomplete.  from what i found, it seemed like
perhaps the leak would go away if the driver cached a copy of the
firmware, as other drivers do.  but that still leaves the suspicion
that there's another copy hanging around in the download path still.

paul

 > 
 > Dan
 > 
 > > diff --git a/drivers/net/wireless/libertas/if_spi.c 
 > b/drivers/net/wireless/libertas/if_spi.c
 > > index fe3f080..123a541 100644
 > > --- a/drivers/net/wireless/libertas/if_spi.c
 > > +++ b/drivers/net/wireless/libertas/if_spi.c
 > > @@ -471,9 +471,12 @@ static int if_spi_prog_helper_firmware(struct 
 > if_spi_card *card)
 > >  		goto release_firmware;
 > >  	err = spu_write_u16(card, IF_SPI_CARD_INT_CAUSE_REG,
 > >  				IF_SPI_CIC_CMD_DOWNLOAD_OVER);
 > > +	if (err)
 > >  		goto release_firmware;
 > >  
 > > -	lbs_deb_spi("waiting for helper to boot...\n");
 > > +	lbs_deb_spi("helper firmware loaded...\n");
 > > +
 > > +	return 0;
 > >  
 > >  release_firmware:
 > >  	release_firmware(firmware);
 > > --
 > > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
 > > the body of a message to majordomo@vger.kernel.org
 > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
 > 
 > 
 > 
 > _______________________________________________
 > libertas-dev mailing list
 > libertas-dev@lists.infradead.org
 > http://lists.infradead.org/mailman/listinfo/libertas-dev

=---------------------
 paul fox, pgf@laptop.org