[PATCH v2 0/3] Drop wrapper functions and clean up the code

[PATCH v2 0/3] Drop wrapper functions and clean up the code

[Shivani Bhardwaj]

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(-)

-- 
2.1.0

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

[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 <shivanib134@gmail.com>
---
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(-)

diff --git a/drivers/staging/wilc1000/linux_wlan_sdio.c b/drivers/staging/wilc1000/linux_wlan_sdio.c
index 1f8d874..802ede2 100644
--- a/drivers/staging/wilc1000/linux_wlan_sdio.c
+++ b/drivers/staging/wilc1000/linux_wlan_sdio.c
@@ -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);
-}
 
 
 
-- 
2.1.0

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

[Shivani Bhardwaj]

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

Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
---
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(-)

diff --git a/drivers/staging/wilc1000/linux_wlan_sdio.c b/drivers/staging/wilc1000/linux_wlan_sdio.c
index 802ede2..f89744d 100644
--- a/drivers/staging/wilc1000/linux_wlan_sdio.c
+++ b/drivers/staging/wilc1000/linux_wlan_sdio.c
@@ -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);
 }
-
-
-
-
-
-- 
2.1.0

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

[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 <shivanib134@gmail.com>
---
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(-)

diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
index 9f899da..fbc3584 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -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

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

[Sudip Mukherjee]

On Wed, Oct 28, 2015 at 05:18:21PM +0530, Shivani Bhardwaj wrote:
> 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 <shivanib134@gmail.com>
> ---

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

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

[Sudip Mukherjee]

On Wed, Oct 28, 2015 at 05:19:04PM +0530, Shivani Bhardwaj wrote:
> 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 <shivanib134@gmail.com>
> ---

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

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

[Julia Lawall]

On Wed, 28 Oct 2015, Shivani Bhardwaj wrote:

> 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

>
> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
> ---
> 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(-)
>
> diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
> index 9f899da..fbc3584 100644
> --- a/drivers/staging/wilc1000/linux_wlan.c
> +++ b/drivers/staging/wilc1000/linux_wlan.c
> @@ -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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
>

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

[Shivani Bhardwaj]

On Wed, Oct 28, 2015 at 7:18 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> On Wed, 28 Oct 2015, Shivani Bhardwaj wrote:
>
>> 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

>>
>> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
>> ---
>> 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(-)
>>
>> diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
>> index 9f899da..fbc3584 100644
>> --- a/drivers/staging/wilc1000/linux_wlan.c
>> +++ b/drivers/staging/wilc1000/linux_wlan.c
>> @@ -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 outreachy-kernel+unsubscribe@googlegroups.com.
>> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> 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.
>>

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

[Shivani Bhardwaj]

On Wed, Oct 28, 2015 at 6:40 PM, Sudip Mukherjee
<sudipm.mukherjee@gmail.com> wrote:
> On Wed, Oct 28, 2015 at 05:19:04PM +0530, Shivani Bhardwaj wrote:
>> 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 <shivanib134@gmail.com>
>> ---
>
> 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

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

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

[Shivani Bhardwaj]

[-- Attachment #1: Type: text/plain, Size: 2183 bytes --]

On Wed, Oct 28, 2015 at 9:39 PM, Shivani Bhardwaj <shivanib134@gmail.com>
wrote:
> On Wed, Oct 28, 2015 at 6:40 PM, Sudip Mukherjee
> <sudipm.mukherjee@gmail.com> wrote:
>> On Wed, Oct 28, 2015 at 05:19:04PM +0530, Shivani Bhardwaj wrote:
>>> 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 <shivanib134@gmail.com>
>>> ---
>>
>> 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
>
> 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>

[-- Attachment #2: Type: text/html, Size: 2694 bytes --]

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

[Sudip Mukherjee]

On Wed, Oct 28, 2015 at 10:28:29PM +0530, Shivani Bhardwaj wrote:
> On Wed, Oct 28, 2015 at 9:39 PM, Shivani Bhardwaj <shivanib134@gmail.com>
> wrote:
> > On Wed, Oct 28, 2015 at 6:40 PM, Sudip Mukherjee
> > <sudipm.mukherjee@gmail.com> wrote:
> >> On Wed, Oct 28, 2015 at 05:19:04PM +0530, Shivani Bhardwaj wrote:
> >>> 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 <shivanib134@gmail.com>
> >>> ---
> >>
> >> 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
> >
> > 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.

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