Hi Dan, > -----Original Message----- > From: Dan Williams [mailto:dcbw@xxxxxxxxxx] > Sent: Wednesday, May 20, 2009 3:46 PM > To: Bing Zhao > Cc: libertas-dev@xxxxxxxxxxxxxxxxxxx; linux-wireless@xxxxxxxxxxxxxxx > 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@xxxxxxxxxx> > > Dan > > > Signed-off-by: Bing Zhao <bzhao@xxxxxxxxxxx> > > --- > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
