[PATCH 4/4] libertas: read SD8688 firmware status from new register

[Bing Zhao]

Hi Dan,

> -----Original Message-----
> From: Dan Williams [mailto:dcbw at redhat.com]
> Sent: Wednesday, May 20, 2009 3:46 PM
> To: Bing Zhao
> Cc: libertas-dev at lists.infradead.org; linux-wireless at vger.kernel.org
> Subject: Re: [PATCH 4/4] libertas: read SD8688 firmware status from new register
> 
> On Tue, 2009-05-19 at 19:48 -0700, Bing Zhao wrote:
> > The scratch pad register is used to store firmware status after
> > firmware is downloaded and initialized. After firmware status is
> > verified OK, the same register is used to store RX packet length.
> > Hence the firmware status code is no longer valid afterwards.
> >
> > SD8688 firmware introduces a new register for firmware status
> > which will never be overwritten.
> 
> This could become confusing :)  read_rx_len() already has a switch
> statement, but since read_rx_len() can also call into read_scratch() for
> the 8385 and 8686, that *also* has a switch selector.  I can't think of
> a great way to break it up though without copying a chunk of code around
> or making useless wrapper functions.
> 

Do you think this change makes things better?

diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
index 18132d4..a7e3fc1 100644
--- a/drivers/net/wireless/libertas/if_sdio.c
+++ b/drivers/net/wireless/libertas/if_sdio.c
@@ -100,6 +100,7 @@ struct if_sdio_card {
 
 	int			model;
 	unsigned long		ioport;
+	unsigned int		scratch_reg;
 
 	const char		*helper;
 	const char		*firmware;
@@ -127,25 +128,13 @@ struct if_sdio_card {
  */
 static u16 if_sdio_read_scratch(struct if_sdio_card *card, int *err)
 {
-	int ret, reg;
+	int ret;
 	u16 scratch;
 
-	switch (card->model) {
-	case IF_SDIO_MODEL_8385:
-		reg = IF_SDIO_SCRATCH_OLD;
-		break;
-	case IF_SDIO_MODEL_8686:
-		reg = IF_SDIO_SCRATCH;
-		break;
-	case IF_SDIO_MODEL_8688:
-	default: /* for newer chipsets */
-		reg = IF_SDIO_FW_STATUS;
-		break;
-	}
-
-	scratch = sdio_readb(card->func, reg, &ret);
+	scratch = sdio_readb(card->func, card->scratch_reg, &ret);
 	if (!ret)
-		scratch |= sdio_readb(card->func, reg + 1, &ret) << 8;
+		scratch |= sdio_readb(card->func, card->scratch_reg + 1,
+					&ret) << 8;
 
 	if (err)
 		*err = ret;
@@ -909,6 +898,20 @@ static int if_sdio_probe(struct sdio_func *func,
 
 	card->func = func;
 	card->model = model;
+
+	switch (card->model) {
+	case IF_SDIO_MODEL_8385:
+		card->scratch_reg = IF_SDIO_SCRATCH_OLD;
+		break;
+	case IF_SDIO_MODEL_8686:
+		card->scratch_reg = IF_SDIO_SCRATCH;
+		break;
+	case IF_SDIO_MODEL_8688:
+	default: /* for newer chipsets */
+		card->scratch_reg = IF_SDIO_FW_STATUS;
+		break;
+	}
+
 	spin_lock_init(&card->lock);
 	card->workqueue = create_workqueue("libertas_sdio");
 	INIT_WORK(&card->packet_worker, if_sdio_host_to_card_worker);



Thanks,

Bing


> Acked-by: Dan Williams <dcbw at redhat.com>
> 
> Dan
> 
> > Signed-off-by: Bing Zhao <bzhao at marvell.com>
> > ---
> >  drivers/net/wireless/libertas/if_sdio.c |   20 ++++++++++++++++++--
> >  drivers/net/wireless/libertas/if_sdio.h |    1 +
> >  2 files changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
> > index 478d766..18132d4 100644
> > --- a/drivers/net/wireless/libertas/if_sdio.c
> > +++ b/drivers/net/wireless/libertas/if_sdio.c
> > @@ -119,15 +119,29 @@ struct if_sdio_card {
> >  /* I/O                                                              */
> >  /********************************************************************/
> >
> > +/*
> > + *  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.
> > + *  For SD8688, this function reads firmware status only.
> > + */
> >  static u16 if_sdio_read_scratch(struct if_sdio_card *card, int *err)
> >  {
> >  	int ret, reg;
> >  	u16 scratch;
> >
> > -	if (card->model == IF_SDIO_MODEL_8385)
> > +	switch (card->model) {
> > +	case IF_SDIO_MODEL_8385:
> >  		reg = IF_SDIO_SCRATCH_OLD;
> > -	else
> > +		break;
> > +	case IF_SDIO_MODEL_8686:
> >  		reg = IF_SDIO_SCRATCH;
> > +		break;
> > +	case IF_SDIO_MODEL_8688:
> > +	default: /* for newer chipsets */
> > +		reg = IF_SDIO_FW_STATUS;
> > +		break;
> > +	}
> >
> >  	scratch = sdio_readb(card->func, reg, &ret);
> >  	if (!ret)
> > @@ -706,6 +720,8 @@ static int if_sdio_prog_firmware(struct if_sdio_card *card)
> >  	if (ret)
> >  		goto out;
> >
> > +	lbs_deb_sdio("firmware status = %#x\n", scratch);
> > +
> >  	if (scratch == IF_SDIO_FIRMWARE_OK) {
> >  		lbs_deb_sdio("firmware already loaded\n");
> >  		goto success;
> > diff --git a/drivers/net/wireless/libertas/if_sdio.h b/drivers/net/wireless/libertas/if_sdio.h
> > index d3a4fbe..60c9b2f 100644
> > --- a/drivers/net/wireless/libertas/if_sdio.h
> > +++ b/drivers/net/wireless/libertas/if_sdio.h
> > @@ -42,6 +42,7 @@
> >
> >  #define IF_SDIO_SCRATCH		0x34
> >  #define IF_SDIO_SCRATCH_OLD	0x80fe
> > +#define IF_SDIO_FW_STATUS	0x40
> >  #define   IF_SDIO_FIRMWARE_OK	0xfedc
> >
> >  #define IF_SDIO_RX_LEN		0x42