| Message ID | 1445578124-31486-5-git-send-email-glen.lee@atmel.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Kalle Valo |
| Headers | show |
On Fri, Oct 23, 2015 at 02:28:21PM +0900, Glen Lee wrote: > This patch add new argument wilc to wilc_handle_isr and pass wilc to > the function. It's not important enough to redo the patch but why are we sometimes using "wl" and sometimes "wilc"? regards, dan carpenter -- 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
On 2015? 10? 23? 15:57, Dan Carpenter wrote: > On Fri, Oct 23, 2015 at 02:28:21PM +0900, Glen Lee wrote: >> This patch add new argument wilc to wilc_handle_isr and pass wilc to >> the function. > It's not important enough to redo the patch but why are we sometimes > using "wl" and sometimes "wilc"? We have wl_xxx_xxx as naming conventions for the function prefix to represent that they are owned in the wireless link controller because the driver will support other line-ups as well as wilc1000, so we will remove wilc1000 prefix and are changing them. In addition, the function parameter names will be wilc for the variable of struct wilc. The "wl" is local variable naming as well. Do you point out "struct wilc *wilc" in the structure wilc_sdio? Thanks for your review always. Tony. > regards, > dan carpenter > -- 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
On Fri, Oct 23, 2015 at 04:36:07PM +0900, Tony Cho wrote: > In addition, the function parameter names will be wilc > for the variable of struct wilc. > > The "wl" is local variable naming as well. So if it is a parameter it is wilc but if it is a local variable then it is wl? That seems like an arbitrary meaningless distinction. It just makes searching harder and complicates things for no reason. regards, dan carpenter -- 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
On 2015? 10? 23? 16:52, Dan Carpenter wrote: > On Fri, Oct 23, 2015 at 04:36:07PM +0900, Tony Cho wrote: >> In addition, the function parameter names will be wilc >> for the variable of struct wilc. >> >> The "wl" is local variable naming as well. > So if it is a parameter it is wilc but if it is a local variable then it > is wl? That seems like an arbitrary meaningless distinction. It just > makes searching harder and complicates things for no reason. I will be on the business trip for the time being so let me ask Glen to reconsider your concerns to apply your opinion to coming patches. I always appreciate your guide and reviews because learning valuable things from you. Thanks, Tony. > > regards, > dan carpenter > -- 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
diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c index 75cbacf..3f5ae55 100644 --- a/drivers/staging/wilc1000/linux_wlan.c +++ b/drivers/staging/wilc1000/linux_wlan.c @@ -260,7 +260,7 @@ irqreturn_t isr_bh_routine(int irq, void *userdata) } PRINT_D(INT_DBG, "Interrupt received BH\n"); - wilc_handle_isr(); + wilc_handle_isr(wl); return IRQ_HANDLED; } diff --git a/drivers/staging/wilc1000/linux_wlan_sdio.c b/drivers/staging/wilc1000/linux_wlan_sdio.c index 1f8d874..4aff953 100644 --- a/drivers/staging/wilc1000/linux_wlan_sdio.c +++ b/drivers/staging/wilc1000/linux_wlan_sdio.c @@ -27,7 +27,6 @@ struct wilc_sdio { }; struct sdio_func *local_sdio_func; -extern void wilc_handle_isr(void); static unsigned int sdio_default_speed; @@ -42,9 +41,13 @@ static const struct sdio_device_id wilc_sdio_ids[] = { static void wilc_sdio_interrupt(struct sdio_func *func) { + struct wilc_sdio *wl_sdio; + + wl_sdio = sdio_get_drvdata(func); + #ifndef WILC_SDIO_IRQ_GPIO sdio_release_host(func); - wilc_handle_isr(); + wilc_handle_isr(wl_sdio->wilc); sdio_claim_host(func); #endif } diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c index 67b0c52..be6f631 100644 --- a/drivers/staging/wilc1000/wilc_wlan.c +++ b/drivers/staging/wilc1000/wilc_wlan.c @@ -1353,7 +1353,7 @@ _end_: wilc_wlan_handle_rxq(); } -void wilc_handle_isr(void) +void wilc_handle_isr(void *wilc) { u32 int_status; diff --git a/drivers/staging/wilc1000/wilc_wlan.h b/drivers/staging/wilc1000/wilc_wlan.h index bd89689..a07375b 100644 --- a/drivers/staging/wilc1000/wilc_wlan.h +++ b/drivers/staging/wilc1000/wilc_wlan.h @@ -301,7 +301,7 @@ int wilc_wlan_stop(void); int wilc_wlan_txq_add_net_pkt(void *priv, u8 *buffer, u32 buffer_size, wilc_tx_complete_func_t func); int wilc_wlan_handle_txq(u32 *pu32TxqCount); -void wilc_handle_isr(void); +void wilc_handle_isr(void *wilc); void wilc_wlan_cleanup(void); int wilc_wlan_cfg_set(int start, u32 wid, u8 *buffer, u32 buffer_size, int commit, u32 drvHandler);
This patch add new argument wilc to wilc_handle_isr and pass wilc to the function. It is void type for now because wilc_wlan.c was implemented platform independently at the beginning (linux_wlan.c is implementation of LINUX part) so the struct wilc header file cannot be included at this moment, but this driver is dedicated to LINUX so wilc_wlan.c and linux_wlan.c will be merged. After that, this void type will be changed with struct wilc as well as other functions which are using void type in wilc_wlan.h. Signed-off-by: Glen Lee <glen.lee@atmel.com> --- drivers/staging/wilc1000/linux_wlan.c | 2 +- drivers/staging/wilc1000/linux_wlan_sdio.c | 7 +++++-- drivers/staging/wilc1000/wilc_wlan.c | 2 +- drivers/staging/wilc1000/wilc_wlan.h | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-)