Drop wrapper functions and clean up the code

PATCH In review Shivani Bhardwaj 3 participants 11 messages Oct 28, 2015 – Oct 29, 2015 Staging

Share & tools

Shivani Bhardwaj

author

2015-10-28 11:47 UTC #0 lore raw

v2 cover Drop wrapper functions and clean up the code

This patchset removes the wrapper functions which are not required and replaces their calls with appropriate functions. Also, some definitions are swapped between files in accordance with the functions that needed them. Unnecessary blank lines are also removed.

After applying this patch, code becomes cleaner.

Shivani Bhardwaj (3):
Staging: wilc1000: linux_wlan_sdio: Drop unneeded wrapper functions
Staging: wilc1000: linux_wlan_sdio: Remove unnecessary blank lines
Staging: wilc1000: linux_wlan: Replace function calls

 drivers/staging/wilc1000/linux_wlan.c      | 15 +++++++++--
 drivers/staging/wilc1000/linux_wlan_sdio.c | 41 +-----------------------------
 2 files changed, 14 insertions(+), 42 deletions(-)

Signature · 1 line

2.1.0

Shivani Bhardwaj

author

2015-10-28 11:48 UTC #1 lore raw

PATCH v2 1/3 Staging: wilc1000: linux_wlan_sdio: Drop unneeded wrapper functions

Remove the functions linux_sdio_get_speed(), linux_sdio_set_max_speed() and linux_sdio_set_default_speed(). Replace their calls with appropriate functions. Also, definitions associated with these functions should be removed.

Signed-off-by Shivani Bhardwaj <[email protected]>

---
Changes in v2:
-Break the patch 1/2 of v1 into two for clarity

 drivers/staging/wilc1000/linux_wlan_sdio.c | 24 +-----------------------
 1 file changed, 1 insertion(+), 23 deletions(-)

drivers/staging/wilc1000/linux_wlan_sdio.c +1 ~~-23~~

index 1f8d874..802ede2 100644
@@ -10,16 +10,6 @@
 
 #define SDIO_MODALIAS "wilc1000_sdio"
 
-#if defined(CUSTOMER_PLATFORM)
-/* TODO : User have to stable bus clock as user's environment. */
- #ifdef MAX_BUS_SPEED
- #define MAX_SPEED MAX_BUS_SPEED
- #else
- #define MAX_SPEED 50000000
- #endif
-#else
- #define MAX_SPEED (6 * 1000000) /* Max 50M */
-#endif
 
 struct wilc_sdio {
 	struct sdio_func *func;

@@ -206,10 +196,6 @@ static int linux_sdio_set_speed(int speed)
 	return 1;
 }
 
-static int linux_sdio_get_speed(void)
-{
-	return local_sdio_func->card->host->ios.clock;
-}
 
 int linux_sdio_init(void *pv)
 {

@@ -219,7 +205,7 @@ int linux_sdio_init(void *pv)
 	 **/
 
 
-	sdio_default_speed = linux_sdio_get_speed();
+	sdio_default_speed = local_sdio_func->card->host->ios.clock;
 	return 1;
 }

@@ -234,15 +220,7 @@ void linux_sdio_deinit(void *pv)
 	sdio_unregister_driver(&wilc_bus);
 }
 
-int linux_sdio_set_max_speed(void)
-{
-	return linux_sdio_set_speed(MAX_SPEED);
-}
 
-int linux_sdio_set_default_speed(void)
-{
-	return linux_sdio_set_speed(sdio_default_speed);
-}

Signature · 1 line

2.1.0

Sudip Mukherjee

2015-10-28 13:05 UTC #2 lore raw

Re: [Outreachy kernel] [PATCH v2 1/3] Staging: wilc1000: linux_wlan_sdio: Drop unneeded wrapper functions

Quoted 7 lines · Shivani Bhardwaj

Remove the functions linux_sdio_get_speed(), linux_sdio_set_max_speed() and linux_sdio_set_default_speed(). Replace their calls with appropriate functions. Also, definitions associated with these functions should be removed.
Signed-off-by Shivani Bhardwaj <[email protected]>
---

You have not build tested it.
WARNING: "linux_sdio_set_default_speed" [drivers/staging/wilc1000/wilc1000.ko] undefined!
WARNING: "linux_sdio_set_max_speed" [drivers/staging/wilc1000/wilc1000.ko] undefined!

We have:
wilc1000/linux_wlan.c: nwi->io_func.u.sdio.sdio_set_default_speed = linux_sdio_set_default_speed;

and also:
nwi->io_func.u.sdio.sdio_set_max_speed = linux_sdio_set_max_speed;

regards
sudip

Shivani Bhardwaj

author

2015-10-28 11:48 UTC #3 lore raw

PATCH v2 2/3 Staging: wilc1000: linux_wlan_sdio: Remove unnecessary blank lines

In reply to Shivani Bhardwaj

Remove all the blank lines which were not required to improve the readability of code.

Signed-off-by Shivani Bhardwaj <[email protected]>

---
Changes in v2:
-Break the patch 1/2 of v1 into two for clarity

 drivers/staging/wilc1000/linux_wlan_sdio.c | 17 -----------------
 1 file changed, 17 deletions(-)

drivers/staging/wilc1000/linux_wlan_sdio.c +0 ~~-17~~

index 802ede2..f89744d 100644
@@ -1,16 +1,12 @@
 #include "wilc_wfi_netdevice.h"
-
 #include <linux/mmc/sdio_func.h>
 #include <linux/mmc/card.h>
 #include <linux/mmc/sdio_ids.h>
 #include <linux/mmc/sdio.h>
 #include <linux/mmc/host.h>
 
-
-
 #define SDIO_MODALIAS "wilc1000_sdio"
 
-
 struct wilc_sdio {
 	struct sdio_func *func;
 	struct wilc *wilc;

@@ -29,7 +25,6 @@ static const struct sdio_device_id wilc_sdio_ids[] = {
 	{ },
 };
 
-
 static void wilc_sdio_interrupt(struct sdio_func *func)
 {
 #ifndef WILC_SDIO_IRQ_GPIO

@@ -39,7 +34,6 @@ static void wilc_sdio_interrupt(struct sdio_func *func)
 #endif
 }
 
-
 int linux_sdio_cmd52(sdio_cmd52_t *cmd)
 {
 	struct sdio_func *func = g_linux_wlan->wilc_sdio_func;

@@ -71,7 +65,6 @@ int linux_sdio_cmd52(sdio_cmd52_t *cmd)
 	return 1;
 }
 
-
 int linux_sdio_cmd53(sdio_cmd53_t *cmd)
 {
 	struct sdio_func *func = g_linux_wlan->wilc_sdio_func;

@@ -199,12 +192,9 @@ static int linux_sdio_set_speed(int speed)
 
 int linux_sdio_init(void *pv)
 {
-
 	/**
 	 *      TODO :
 	 **/
-
-
 	sdio_default_speed = local_sdio_func->card->host->ios.clock;
 	return 1;
 }

@@ -215,12 +205,5 @@ void linux_sdio_deinit(void *pv)
 	/**
 	 *      TODO :
 	 **/
-
-
 	sdio_unregister_driver(&wilc_bus);
 }
-
-
-
-
-

Signature · 1 line

2.1.0

Shivani Bhardwaj

author

2015-10-28 11:49 UTC #4 lore raw

PATCH v2 3/3 Staging: wilc1000: linux_wlan: Replace function calls

In reply to Shivani Bhardwaj

Replace calls to wrapper functions with the actual functions called by wrapper functions. Definitions needed to make those calls should also be added.

Signed-off-by Shivani Bhardwaj <[email protected]>

---
Changes in v2:
-Break the patch 1/2 of v1 into two for clarity

 drivers/staging/wilc1000/linux_wlan.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

drivers/staging/wilc1000/linux_wlan.c +13 -2

index 9f899da..fbc3584 100644
@@ -45,6 +45,17 @@
  #define _linux_wlan_device_removal()		{}
 #endif
 
+#if defined(CUSTOMER_PLATFORM)
+/* TODO : User have to stable bus clock as user's environment. */
+#ifdef MAX_BUS_SPEED
+#define MAX_SPEED MAX_BUS_SPEED
+#else
+#define MAX_SPEED 50000000
+#endif
+#else
+#define MAX_SPEED (6 * 1000000) /* Max 50M */
+#endif
+
 extern bool g_obtainingIP;
 extern u16 Set_machw_change_vir_if(bool bValue);
 extern void resolve_disconnect_aberration(void *drvHandler);

@@ -976,8 +987,8 @@ void linux_to_wlan(wilc_wlan_inp_t *nwi, struct wilc *nic)
 	nwi->io_func.io_deinit = linux_sdio_deinit;
 	nwi->io_func.u.sdio.sdio_cmd52 = linux_sdio_cmd52;
 	nwi->io_func.u.sdio.sdio_cmd53 = linux_sdio_cmd53;
-	nwi->io_func.u.sdio.sdio_set_max_speed = linux_sdio_set_max_speed;
-	nwi->io_func.u.sdio.sdio_set_default_speed = linux_sdio_set_default_speed;
+	nwi->io_func.u.sdio.sdio_set_max_speed = linux_sdio_set_speed(MAX_SPEED);
+	nwi->io_func.u.sdio.sdio_set_default_speed = linux_sdio_set_speed(sdio_default_speed);
 #else
 	nwi->io_func.io_type = HIF_SPI;
 	nwi->io_func.io_init = linux_spi_init;

Signature · 1 line

2.1.0

Sudip Mukherjee

2015-10-28 13:10 UTC #5 lore raw

Re: [Outreachy kernel] [PATCH v2 3/3] Staging: wilc1000: linux_wlan: Replace function calls

Quoted 5 lines · Shivani Bhardwaj

Replace calls to wrapper functions with the actual functions called by wrapper functions. Definitions needed to make those calls should also be added.
Signed-off-by Shivani Bhardwaj <[email protected]>
---

Not build tested. :(

error: implicit declaration of function ‘linux_sdio_set_speed’ [-Werror=implicit-function-declaration]
nwi->io_func.u.sdio.sdio_set_max_speed = linux_sdio_set_speed(MAX_SPEED);

drivers/staging/wilc1000/linux_wlan.c:990:41: warning: assignment makes pointer from integer without a cast [enabled by default]
nwi->io_func.u.sdio.sdio_set_max_speed = linux_sdio_set_speed(MAX_SPEED);

drivers/staging/wilc1000/linux_wlan.c:991:68: error: ‘sdio_default_speed’ undeclared (first use in this function)

regards
sudip

Shivani Bhardwaj

author

2015-10-28 16:09 UTC #6 lore raw

Re: [Outreachy kernel] [PATCH v2 3/3] Staging: wilc1000: linux_wlan: Replace function calls

Quoted 12 lines · Sudip Mukherjee

Quoted 5 lines · Shivani Bhardwaj
Replace calls to wrapper functions with the actual functions called by wrapper functions. Definitions needed to make those calls should also be added.
Signed-off-by Shivani Bhardwaj <[email protected]>
---
Not build tested. :(
error: implicit declaration of function ‘linux_sdio_set_speed’ [-Werror=implicit-function-declaration]
nwi->io_func.u.sdio.sdio_set_max_speed = linux_sdio_set_speed(MAX_SPEED);

Quoted 7 lines

drivers/staging/wilc1000/linux_wlan.c:990:41: warning: assignment makes pointer from integer without a cast [enabled by default]
nwi->io_func.u.sdio.sdio_set_max_speed = linux_sdio_set_speed(MAX_SPEED);
drivers/staging/wilc1000/linux_wlan.c:991:68: error: ‘sdio_default_speed’ undeclared (first use in this function)
regards
sudip

I'm really very sorry I didn't do make allmodconfig, make didn't show me any errors at that time. In order to fix these mistakes, I'm thinking of changing some part of code. Please check the next version. Thank you

Shivani Bhardwaj

author

2015-10-28 16:58 UTC #7 lore raw

Re: [Outreachy kernel] [PATCH v2 3/3] Staging: wilc1000: linux_wlan: Replace function calls

Quoted 5 lines · Shivani Bhardwaj

Quoted 3 lines · Sudip Mukherjee
Quoted 2 lines · Shivani Bhardwaj
Replace calls to wrapper functions with the actual functions called by wrapper functions. Definitions needed to make those calls should also

be added.

Quoted 7 lines

Quoted 7 lines
Quoted 3 lines
Signed-off-by Shivani Bhardwaj <[email protected]>
---
Not build tested. :(
error: implicit declaration of function ‘linux_sdio_set_speed’

[-Werror=implicit-function-declaration]

Quoted 1 line

Quoted 1 line
nwi->io_func.u.sdio.sdio_set_max_speed =

linux_sdio_set_speed(MAX_SPEED);

Quoted 2 lines

Quoted 2 lines
drivers/staging/wilc1000/linux_wlan.c:990:41: warning: assignment makes

pointer from integer without a cast [enabled by default]

Quoted 1 line

Quoted 1 line
nwi->io_func.u.sdio.sdio_set_max_speed =

linux_sdio_set_speed(MAX_SPEED);

Quoted 2 lines

Quoted 2 lines
drivers/staging/wilc1000/linux_wlan.c:991:68: error:

‘sdio_default_speed’ undeclared (first use in this function)

Quoted 8 lines

Quoted 3 lines
regards
sudip
I'm really very sorry I didn't do make allmodconfig, make didn't show me any errors at that time. In order to fix these mistakes, I'm thinking of changing some part of code. Please check the next version. Thank you

I need a bit of help here. While replacing the calls of wrapper function by actual function, I get the following:
warning: assignment makes pointer from integer without a cast

Why is it like that? Wrapper function returns function which in turn returns an integer so ultimately the value that is to be returned is int.

I think I'm missing something trivial. I don't know what.

Wrapper functions:
linux_sdio_set_max_speed
<http://lxr.free-electrons.com/source/drivers/staging/wilc1000/linux_wlan_sdio.c#L195>
linux_sdio_set_default_speed
<http://lxr.free-electrons.com/source/drivers/staging/wilc1000/linux_wlan_sdio.c#L195>

Replaced with:
linux_sdio_set_speed
<http://lxr.free-electrons.com/source/drivers/staging/wilc1000/linux_wlan_sdio.c#L195>

Called at:
WILC_SDIO
<http://lxr.free-electrons.com/source/drivers/staging/wilc1000/linux_wlan.c>

Sudip Mukherjee

2015-10-29 05:41 UTC #8 lore raw

Re: [Outreachy kernel] [PATCH v2 3/3] Staging: wilc1000: linux_wlan: Replace function calls

Quoted 35 lines · Shivani Bhardwaj

Quoted 5 lines · Shivani Bhardwaj
Quoted 3 lines · Sudip Mukherjee
Quoted 2 lines · Shivani Bhardwaj
Replace calls to wrapper functions with the actual functions called by wrapper functions. Definitions needed to make those calls should also
be added.
Quoted 7 lines
Quoted 7 lines
Quoted 3 lines
Signed-off-by Shivani Bhardwaj <[email protected]>
---
Not build tested. :(
error: implicit declaration of function ‘linux_sdio_set_speed’
[-Werror=implicit-function-declaration]
Quoted 1 line
Quoted 1 line
nwi->io_func.u.sdio.sdio_set_max_speed =
linux_sdio_set_speed(MAX_SPEED);
Quoted 2 lines
Quoted 2 lines
drivers/staging/wilc1000/linux_wlan.c:990:41: warning: assignment makes
pointer from integer without a cast [enabled by default]
Quoted 1 line
Quoted 1 line
nwi->io_func.u.sdio.sdio_set_max_speed =
linux_sdio_set_speed(MAX_SPEED);
Quoted 2 lines
Quoted 2 lines
drivers/staging/wilc1000/linux_wlan.c:991:68: error:
‘sdio_default_speed’ undeclared (first use in this function)
Quoted 8 lines
Quoted 3 lines
regards
sudip
I'm really very sorry I didn't do make allmodconfig, make didn't show me any errors at that time. In order to fix these mistakes, I'm thinking of changing some part of code. Please check the next version. Thank you

Quoted 8 lines

I need a bit of help here. While replacing the calls of wrapper function by actual function, I get the following:
warning: assignment makes pointer from integer without a cast
Why is it like that? Wrapper function returns function which in turn returns an integer so ultimately the value that is to be returned is int.
I think I'm missing something trivial. I don't know what.

When it is doing nwi->io_func.u.sdio.sdio_set_max_speed = linux_sdio_set_max_speed; it is saving the address of the function linux_sdio_set_max_speed in nwi->io_func.u.sdio.sdio_set_max_speed.
But when you are doing:
nwi->io_func.u.sdio.sdio_set_max_speed = linux_sdio_set_speed(MAX_SPEED); you are trying to save the speed value in a place where the function pointer is supposed to stay.

If you want to remove this wrapper, then you can do:
nwi->io_func.u.sdio.sdio_set_max_speed = linux_sdio_set_speed;

and then in all places where nwi->io_func.u.sdio.sdio_set_max_speed is used you need to modify that to use MAX_SPEED as argument.

But I guess that will make the code less readable.

regards
sudip

Julia Lawall

2015-10-28 13:48 UTC #9 lore raw

Re: [Outreachy kernel] [PATCH v2 3/3] Staging: wilc1000: linux_wlan: Replace function calls

In reply to Shivani Bhardwaj

Quoted 2 lines · Shivani Bhardwaj

Replace calls to wrapper functions with the actual functions called by wrapper functions. Definitions needed to make those calls should also be added.

The second sentence of the commit message is not clear. When you say "should be" it gives the impression that you expect someone else to do it. So it would be better to say "are added".

julia

Quoted 52 lines

Signed-off-by Shivani Bhardwaj <[email protected]>
---
Changes in v2:
-Break the patch 1/2 of v1 into two for clarity
 drivers/staging/wilc1000/linux_wlan.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)
drivers/staging/wilc1000/linux_wlan.c +13 -3
index 9f899da..fbc3584 100644
@@ -45,6 +45,17 @@
  #define _linux_wlan_device_removal()		{}
 #endif
 
+#if defined(CUSTOMER_PLATFORM)
+/* TODO : User have to stable bus clock as user's environment. */
+#ifdef MAX_BUS_SPEED
+#define MAX_SPEED MAX_BUS_SPEED
+#else
+#define MAX_SPEED 50000000
+#endif
+#else
+#define MAX_SPEED (6 * 1000000) /* Max 50M */
+#endif
+
 extern bool g_obtainingIP;
 extern u16 Set_machw_change_vir_if(bool bValue);
 extern void resolve_disconnect_aberration(void *drvHandler);
@@ -976,8 +987,8 @@ void linux_to_wlan(wilc_wlan_inp_t *nwi, struct wilc *nic)
 	nwi->io_func.io_deinit = linux_sdio_deinit;
 	nwi->io_func.u.sdio.sdio_cmd52 = linux_sdio_cmd52;
 	nwi->io_func.u.sdio.sdio_cmd53 = linux_sdio_cmd53;
-	nwi->io_func.u.sdio.sdio_set_max_speed = linux_sdio_set_max_speed;
-	nwi->io_func.u.sdio.sdio_set_default_speed = linux_sdio_set_default_speed;
+	nwi->io_func.u.sdio.sdio_set_max_speed = linux_sdio_set_speed(MAX_SPEED);
+	nwi->io_func.u.sdio.sdio_set_default_speed = linux_sdio_set_speed(sdio_default_speed);
 #else
 	nwi->io_func.io_type = HIF_SPI;
 	nwi->io_func.io_init = linux_spi_init;
--
2.1.0
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/f2a06d6455d1f322d873ec728f622162fd00d959.1446032441.git.shivanib134%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

Shivani Bhardwaj

author

2015-10-28 16:07 UTC #10 lore raw

Re: [Outreachy kernel] [PATCH v2 3/3] Staging: wilc1000: linux_wlan: Replace function calls

Quoted 11 lines · Julia Lawall

Quoted 2 lines · Shivani Bhardwaj
Replace calls to wrapper functions with the actual functions called by wrapper functions. Definitions needed to make those calls should also be added.
The second sentence of the commit message is not clear. When you say "should be" it gives the impression that you expect someone else to do it. So it would be better to say "are added".
julia

I'll correct that.
Thank you

Quoted 52 lines

Quoted 52 lines
Signed-off-by Shivani Bhardwaj <[email protected]>
---
Changes in v2:
-Break the patch 1/2 of v1 into two for clarity
 drivers/staging/wilc1000/linux_wlan.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)
drivers/staging/wilc1000/linux_wlan.c +13 -3
index 9f899da..fbc3584 100644
@@ -45,6 +45,17 @@
  #define _linux_wlan_device_removal()                {}
 #endif
 
+#if defined(CUSTOMER_PLATFORM)
+/* TODO : User have to stable bus clock as user's environment. */
+#ifdef MAX_BUS_SPEED
+#define MAX_SPEED MAX_BUS_SPEED
+#else
+#define MAX_SPEED 50000000
+#endif
+#else
+#define MAX_SPEED (6 * 1000000) /* Max 50M */
+#endif
+
 extern bool g_obtainingIP;
 extern u16 Set_machw_change_vir_if(bool bValue);
 extern void resolve_disconnect_aberration(void *drvHandler);
@@ -976,8 +987,8 @@ void linux_to_wlan(wilc_wlan_inp_t *nwi, struct wilc *nic)
      nwi->io_func.io_deinit = linux_sdio_deinit;
      nwi->io_func.u.sdio.sdio_cmd52 = linux_sdio_cmd52;
      nwi->io_func.u.sdio.sdio_cmd53 = linux_sdio_cmd53;
-     nwi->io_func.u.sdio.sdio_set_max_speed = linux_sdio_set_max_speed;
-     nwi->io_func.u.sdio.sdio_set_default_speed = linux_sdio_set_default_speed;
+     nwi->io_func.u.sdio.sdio_set_max_speed = linux_sdio_set_speed(MAX_SPEED);
+     nwi->io_func.u.sdio.sdio_set_default_speed = linux_sdio_set_speed(sdio_default_speed);
 #else
      nwi->io_func.io_type = HIF_SPI;
      nwi->io_func.io_init = linux_spi_init;
--
2.1.0
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/f2a06d6455d1f322d873ec728f622162fd00d959.1446032441.git.shivanib134%40gmail.com.
For more options, visit https://groups.google.com/d/optout.