staging: wilc1000: WILC1000_DYNAMICALLY_ALLOCATE_MEMROY

PATCH In review Stanislav Kholmanskikh 4 participants 8 messages Oct 8, 2015 – Oct 13, 2015 Staging

lore

Share & tools

lore

Stanislav Kholmanskikh

author

2015-10-08 18:29 UTC #0 lore raw

Hi!

I noticed that drivers/staging/wilc1000/Kconfic defines:

config WILC1000_DYNAMICALLY_ALLOCATE_MEMROY
bool "Dynamically allocate memory in real time" ---help---
This choice supports dynamic allocation of the memory for the receive buffer. The driver will allocate the RX buffer when it is required.

"MEMROY" looks suspicious. Is it a typo (MEMORY?) or by intent?

Thanks!

Greg KH

maintainer

2015-10-08 18:47 UTC #1 lore raw

Quoted 12 lines · Stanislav Kholmanskikh

Hi!
I noticed that drivers/staging/wilc1000/Kconfic defines:
config WILC1000_DYNAMICALLY_ALLOCATE_MEMROY
bool "Dynamically allocate memory in real time" ---help---
This choice supports dynamic allocation of the memory for the receive buffer. The driver will allocate the RX buffer when it is required.

Quoted 1 line

"MEMROY" looks suspicious. Is it a typo (MEMORY?) or by intent?

Probably a typo, but be sure to see if it lines up with what the code expects before changing anything.

Ideally in the end this option should be removed as the driver should only do dynamic allocation, no static allocation should be needed.

thanks,

greg k-h

Stanislav Kholmanskikh

author

2015-10-09 18:19 UTC #2 lore raw

PATCH 1/1 staging: wilc1000: typo in WILC1000_DYNAMICALLY_ALLOCATE_MEMROY

Looking at the code, it seems that "MEMROY" in the config option name is a typo. It should be "MEMORY" instead.

Changed the config option name accordingly.

Signed-off-by Stanislav Kholmanskikh <[email protected]>

---
 drivers/staging/wilc1000/Kconfig  | 2 +-
 drivers/staging/wilc1000/Makefile | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

drivers/staging/wilc1000/Kconfig +1 -1

index 59ccecc..d636f46 100644
@@ -19,7 +19,7 @@ config WILC1000_PREALLOCATE_AT_LOADING_DRIVER
 	  during initial time. The driver will also free the buffer
 	  by calling network device stop.
 
-config WILC1000_DYNAMICALLY_ALLOCATE_MEMROY
+config WILC1000_DYNAMICALLY_ALLOCATE_MEMORY
         bool "Dynamically allocate memory in real time"
         ---help---
 	  This choice supports dynamic allocation of the memory

drivers/staging/wilc1000/Makefile +1 -1

index f495061..026609a 100644
@@ -15,7 +15,7 @@ ccflags-y += -I$(src)/ -D__CHECK_ENDIAN__ -DWILC_ASIC_A0 \
 ccflags-$(CONFIG_WILC1000_PREALLOCATE_AT_LOADING_DRIVER) += -DMEMORY_STATIC \
 								-DWILC_PREALLOC_AT_INSMOD
 
-ccflags-$(CONFIG_WILC1000_DYNAMICALLY_ALLOCATE_MEMROY) += -DWILC_NORMAL_ALLOC
+ccflags-$(CONFIG_WILC1000_DYNAMICALLY_ALLOCATE_MEMORY) += -DWILC_NORMAL_ALLOC
 
 
 wilc1000-objs := wilc_wfi_cfgoperations.o linux_wlan.o linux_mon.o \

Signature · 1 line

1.9.1

Dan Carpenter

2015-10-09 18:24 UTC #3 lore raw

Re: [PATCH] staging: wilc1000: typo in WILC1000_DYNAMICALLY_ALLOCATE_MEMROY

Quoted 6 lines · Stanislav Kholmanskikh

Looking at the code, it seems that "MEMROY" in the config option name is a typo. It should be "MEMORY" instead.
Changed the config option name accordingly.
Signed-off-by Stanislav Kholmanskikh <[email protected]>

Just delete it and delete -DWILC_NORMAL_ALLOC as well.

regards,
dan carpenter

Stanislav Kholmanskikh

author

2015-10-10 12:42 UTC #4 lore raw

PATCH 1/1 staging: wilc1000: remove WILC1000_DYNAMICALLY_ALLOCATE_MEMROY

The config option in the subject may be removed, because, indeed, it only serves as the 'n' value for CONFIG_WILC1000_PREALLOCATE_AT_LOADING_DRIVER

Signed-off-by Stanislav Kholmanskikh <[email protected]>

---
 drivers/staging/wilc1000/Kconfig  | 15 ++-------------
 drivers/staging/wilc1000/Makefile |  3 ---
 2 files changed, 2 insertions(+), 16 deletions(-)

drivers/staging/wilc1000/Kconfig +2 ~~-13~~

index 59ccecc..2701a01 100644
@@ -6,27 +6,16 @@ config WILC1000
 	---help---
 	  This module only support IEEE 802.11n WiFi.
 
-choice
-        prompt "Memory Allocation"
-        depends on WILC1000
-        default WILC1000_PREALLOCATE_AT_LOADING_DRIVER
-
 config WILC1000_PREALLOCATE_AT_LOADING_DRIVER
 	bool "Preallocate memory at loading driver"
+	depends on WILC1000
+	default y
 	---help---
 	  This choice supports static allocation of the memory
 	  for the receive buffer. The driver will allocate the RX buffer
 	  during initial time. The driver will also free the buffer
 	  by calling network device stop.
 
-config WILC1000_DYNAMICALLY_ALLOCATE_MEMROY
-        bool "Dynamically allocate memory in real time"
-        ---help---
-	  This choice supports dynamic allocation of the memory
-	  for the receive buffer. The driver will allocate the RX buffer
-	  when it is required.
-endchoice
-
 choice
 	prompt "Bus Type"
 	depends on WILC1000

drivers/staging/wilc1000/Makefile +0 -3

index f495061..eb8f51d 100644
@@ -15,9 +15,6 @@ ccflags-y += -I$(src)/ -D__CHECK_ENDIAN__ -DWILC_ASIC_A0 \
 ccflags-$(CONFIG_WILC1000_PREALLOCATE_AT_LOADING_DRIVER) += -DMEMORY_STATIC \
 								-DWILC_PREALLOC_AT_INSMOD
 
-ccflags-$(CONFIG_WILC1000_DYNAMICALLY_ALLOCATE_MEMROY) += -DWILC_NORMAL_ALLOC
-
-
 wilc1000-objs := wilc_wfi_cfgoperations.o linux_wlan.o linux_mon.o \
 			wilc_msgqueue.o \
 			coreconfigurator.o host_interface.o \

Signature · 1 line

1.9.1

Greg KH

maintainer

2015-10-13 05:47 UTC #5 lore raw

Re: [PATCH] staging: wilc1000: remove WILC1000_DYNAMICALLY_ALLOCATE_MEMROY

Quoted 17 lines · Stanislav Kholmanskikh

The config option in the subject may be removed, because, indeed, it only serves as the 'n' value for CONFIG_WILC1000_PREALLOCATE_AT_LOADING_DRIVER
Signed-off-by Stanislav Kholmanskikh <[email protected]>
---
 drivers/staging/wilc1000/Kconfig  | 15 ++-------------
 drivers/staging/wilc1000/Makefile |  3 ---
 2 files changed, 2 insertions(+), 16 deletions(-)
drivers/staging/wilc1000/Kconfig +0 -0
index 59ccecc..2701a01 100644
@@ -6,27 +6,16 @@ config WILC1000
 	---help---
 	  This module only support IEEE 802.11n WiFi.

drivers/staging/wilc1000/Kconfig config WILC1000 · line 12

Quoted 10 lines

drivers/staging/wilc1000/Kconfig +2 -5

 
-choice
-        prompt "Memory Allocation"
-        depends on WILC1000
-        default WILC1000_PREALLOCATE_AT_LOADING_DRIVER
-
 config WILC1000_PREALLOCATE_AT_LOADING_DRIVER
 	bool "Preallocate memory at loading driver"
+	depends on WILC1000
+	default y

Hm, why have this now? Both of these options should go away, please work on fixing up the code to not need them and then delete the options, right now you are just deleting config options which doesn't seem wise.

thanks,

greg k-h

Tony Cho

2015-10-12 02:13 UTC #6 lore raw

In reply to Greg KH

Quoted 13 lines · Greg KH

Quoted 12 lines · Stanislav Kholmanskikh
Hi!
I noticed that drivers/staging/wilc1000/Kconfic defines:
config WILC1000_DYNAMICALLY_ALLOCATE_MEMROY
bool "Dynamically allocate memory in real time" ---help---
This choice supports dynamic allocation of the memory for the receive buffer. The driver will allocate the RX buffer when it is required.

Quoted 6 lines

Quoted 1 line
"MEMROY" looks suspicious. Is it a typo (MEMORY?) or by intent?
Probably a typo, but be sure to see if it lines up with what the code expects before changing anything.
Ideally in the end this option should be removed as the driver should only do dynamic allocation, no static allocation should be needed.

I agree with Greg's opinion. The team is now restructuring the driver and they will be removed and changed.

Thanks for your valuable opinion,

Tony.

Quoted 4 lines

thanks,
greg k-h

Tony Cho

2015-10-12 02:11 UTC #7 lore raw

In reply to Stanislav Kholmanskikh

Quoted 12 lines · Stanislav Kholmanskikh

Hi!
I noticed that drivers/staging/wilc1000/Kconfic defines:
config WILC1000_DYNAMICALLY_ALLOCATE_MEMROY
bool "Dynamically allocate memory in real time" ---help---
This choice supports dynamic allocation of the memory for the receive buffer. The driver will allocate the RX buffer when it is required.

Quoted 1 line

"MEMROY" looks suspicious. Is it a typo (MEMORY?) or by intent?

These parts will be wholly changed but I need to get important driver updates. It will be done soon.

Thanks for your review,

Tony.

Quoted 1 line

Thanks!