Message ID | 1436507759-4546-3-git-send-email-johnny.kim@atmel.com (mailing list archive) |
---|---|
State | Not Applicable |
Delegated to: | Kalle Valo |
Headers | show |
Hi Johnny, On Fri, Jul 10, 2015 at 3:55 PM, Johnny Kim <johnny.kim@atmel.com> wrote: > Last argument of wilc_wlan_cfg_get function is actually structure's address. > This should be changed to be compatible with 64bit machine. > Because wilc_wlan_cfg_get function is mapped by function pointer later, > wilc_wlan_oup_t.wlan_cfg_get should be changed together. > > tstrWILC_WFIDrv structure is defined after wilc_wlan_oup_t.wlan_cfg_get > is defined. So, this patch changes the argument to void type pointer. Why not add a patch moving the structure definition before wilc_wlan_oup_t.wlan_cfg_get is defined? > Signed-off-by: Johnny Kim <johnny.kim@atmel.com> > --- > drivers/staging/wilc1000/coreconfigurator.c | 2 +- > drivers/staging/wilc1000/wilc_wlan.c | 2 +- > drivers/staging/wilc1000/wilc_wlan_if.h | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c > index b069614..141d7b4 100644 > --- a/drivers/staging/wilc1000/coreconfigurator.c > +++ b/drivers/staging/wilc1000/coreconfigurator.c > @@ -2094,7 +2094,7 @@ s32 SendConfigPkt(u8 u8Mode, tstrWID *pstrWIDs, > (counter == u32WIDsCount - 1)); > if (!gpstrWlanOps->wlan_cfg_get(!counter, > pstrWIDs[counter].u16WIDid, > - (counter == u32WIDsCount - 1), drvHandler)) { > + (counter == u32WIDsCount - 1), (void *)drvHandler)) { If I recall correctly, you shouldn't need void * casts. Thanks,
On 2015? 07? 10? 15:25, Julian Calaby wrote: > Hi Johnny, > > On Fri, Jul 10, 2015 at 3:55 PM, Johnny Kim <johnny.kim@atmel.com> wrote: >> Last argument of wilc_wlan_cfg_get function is actually structure's address. >> This should be changed to be compatible with 64bit machine. >> Because wilc_wlan_cfg_get function is mapped by function pointer later, >> wilc_wlan_oup_t.wlan_cfg_get should be changed together. >> >> tstrWILC_WFIDrv structure is defined after wilc_wlan_oup_t.wlan_cfg_get >> is defined. So, this patch changes the argument to void type pointer. > Why not add a patch moving the structure definition before > wilc_wlan_oup_t.wlan_cfg_get is defined? Current patch focus on accessing 64bit address rightly. The define order you and I mentioned should be repaired with another subject because of complexity among files. >> Signed-off-by: Johnny Kim <johnny.kim@atmel.com> >> --- >> drivers/staging/wilc1000/coreconfigurator.c | 2 +- >> drivers/staging/wilc1000/wilc_wlan.c | 2 +- >> drivers/staging/wilc1000/wilc_wlan_if.h | 2 +- >> 3 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c >> index b069614..141d7b4 100644 >> --- a/drivers/staging/wilc1000/coreconfigurator.c >> +++ b/drivers/staging/wilc1000/coreconfigurator.c >> @@ -2094,7 +2094,7 @@ s32 SendConfigPkt(u8 u8Mode, tstrWID *pstrWIDs, >> (counter == u32WIDsCount - 1)); >> if (!gpstrWlanOps->wlan_cfg_get(!counter, >> pstrWIDs[counter].u16WIDid, >> - (counter == u32WIDsCount - 1), drvHandler)) { >> + (counter == u32WIDsCount - 1), (void *)drvHandler)) { > If I recall correctly, you shouldn't need void * casts. > > Thanks, > Thanks. Johnny. -- 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
Hi Johnny, On Fri, Jul 10, 2015 at 6:11 PM, Johnny Kim <johnny.kim@atmel.com> wrote: > > On 2015? 07? 10? 15:25, Julian Calaby wrote: >> >> Hi Johnny, >> >> On Fri, Jul 10, 2015 at 3:55 PM, Johnny Kim <johnny.kim@atmel.com> wrote: >>> >>> Last argument of wilc_wlan_cfg_get function is actually structure's >>> address. >>> This should be changed to be compatible with 64bit machine. >>> Because wilc_wlan_cfg_get function is mapped by function pointer later, >>> wilc_wlan_oup_t.wlan_cfg_get should be changed together. >>> >>> tstrWILC_WFIDrv structure is defined after wilc_wlan_oup_t.wlan_cfg_get >>> is defined. So, this patch changes the argument to void type pointer. >> >> Why not add a patch moving the structure definition before >> wilc_wlan_oup_t.wlan_cfg_get is defined? > > Current patch focus on accessing 64bit address rightly. > The define order you and I mentioned should be repaired with another subject > because of complexity among files. I'm not saying it should be part of this patch, I'm saying that it should be a patch in this series. Some of the changes you're making look like you're fixing one bug only to replace it with another different one. But back to the whole issue of order: tstrWILC_WFIDrv is defined in host_interface.h wlan_cfg_get is defined in wilc_wlan_if.h A patch ensuring that host_interface.h is included before wilc_wlan_if.h in all files should be pretty easy to produce, assuming that host_interface.h doesn't use anything in wilc_wlan_if.h. If you include this patch, you can avoid patches later to change the void pointers to typed pointers. Thanks,
On Fri, Jul 10, 2015 at 02:55:56PM +0900, Johnny Kim wrote: > Last argument of wilc_wlan_cfg_get function is actually structure's address. > This should be changed to be compatible with 64bit machine. > Because wilc_wlan_cfg_get function is mapped by function pointer later, > wilc_wlan_oup_t.wlan_cfg_get should be changed together. > > tstrWILC_WFIDrv structure is defined after wilc_wlan_oup_t.wlan_cfg_get > is defined. So, this patch changes the argument to void type pointer. No, you should never use a void pointer, that indicates the code is designed wrong. Please reorder the structures in a .h file if needed, there is nothing preventing you from doing the right thing here. Please fix up this whole series and resend, I can't take it as-is, sorry. greg k-h -- 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/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c index b069614..141d7b4 100644 --- a/drivers/staging/wilc1000/coreconfigurator.c +++ b/drivers/staging/wilc1000/coreconfigurator.c @@ -2094,7 +2094,7 @@ s32 SendConfigPkt(u8 u8Mode, tstrWID *pstrWIDs, (counter == u32WIDsCount - 1)); if (!gpstrWlanOps->wlan_cfg_get(!counter, pstrWIDs[counter].u16WIDid, - (counter == u32WIDsCount - 1), drvHandler)) { + (counter == u32WIDsCount - 1), (void *)drvHandler)) { ret = -1; printk("[Sendconfigpkt]Get Timed out\n"); break; diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c index d32e45e..c0a8063 100644 --- a/drivers/staging/wilc1000/wilc_wlan.c +++ b/drivers/staging/wilc1000/wilc_wlan.c @@ -1938,7 +1938,7 @@ static int wilc_wlan_cfg_set(int start, uint32_t wid, uint8_t *buffer, uint32_t return ret_size; } -static int wilc_wlan_cfg_get(int start, uint32_t wid, int commit, uint32_t drvHandler) +static int wilc_wlan_cfg_get(int start, uint32_t wid, int commit, void* drvHandler) { wilc_wlan_dev_t *p = (wilc_wlan_dev_t *)&g_wlan; uint32_t offset; diff --git a/drivers/staging/wilc1000/wilc_wlan_if.h b/drivers/staging/wilc1000/wilc_wlan_if.h index 8735a6a..8c293ab 100644 --- a/drivers/staging/wilc1000/wilc_wlan_if.h +++ b/drivers/staging/wilc1000/wilc_wlan_if.h @@ -194,7 +194,7 @@ typedef struct { void (*wlan_handle_rx_isr)(void); void (*wlan_cleanup)(void); int (*wlan_cfg_set)(int, uint32_t, uint8_t *, uint32_t, int, uint32_t); - int (*wlan_cfg_get)(int, uint32_t, int, uint32_t); + int (*wlan_cfg_get)(int, uint32_t, int, void *); int (*wlan_cfg_get_value)(uint32_t, uint8_t *, uint32_t); /*Bug3959: transmitting mgmt frames received from host*/ #if defined(WILC_AP_EXTERNAL_MLME) || defined(WILC_P2P)
Last argument of wilc_wlan_cfg_get function is actually structure's address. This should be changed to be compatible with 64bit machine. Because wilc_wlan_cfg_get function is mapped by function pointer later, wilc_wlan_oup_t.wlan_cfg_get should be changed together. tstrWILC_WFIDrv structure is defined after wilc_wlan_oup_t.wlan_cfg_get is defined. So, this patch changes the argument to void type pointer. Signed-off-by: Johnny Kim <johnny.kim@atmel.com> --- drivers/staging/wilc1000/coreconfigurator.c | 2 +- drivers/staging/wilc1000/wilc_wlan.c | 2 +- drivers/staging/wilc1000/wilc_wlan_if.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)