On Wed, 2009-05-20 at 16:50 -0700, Bing Zhao wrote: > 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? Yeah, that's better. Also saves some processing time because the scratch reg doesn't have to be set each time a frame comes in. Want to resubmit with the Signed-off-by and description just for fun? :) Dan > 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 > > > _______________________________________________ > libertas-dev mailing list > libertas-dev@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/libertas-dev -- 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
