RE: [PATCH 3/5] mwifiex: do not free firmware dump memory in shutdown_drv

[Amitkumar Karwar <akarwar@marvell.com>]

Hi Dmitry,

> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: Tuesday, October 25, 2016 5:47 AM
> To: Amitkumar Karwar
> Cc: linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam;
> briannorris@google.com; Xinming Hu
> Subject: Re: [PATCH 3/5] mwifiex: do not free firmware dump memory in
> shutdown_drv
> 
> On Mon, Oct 24, 2016 at 07:51:30PM +0530, Amitkumar Karwar wrote:
> > From: Xinming Hu <huxm@marvell.com>
> >
> > mwifiex_upload_device_dump() already takes care of freeing firmware
> > dump memory. Doing the same thing in mwifiex_shutdown_drv() is
> redundant.
> >
> > Signed-off-by: Xinming Hu <huxm@marvell.com>
> > Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> > ---
> >  drivers/net/wireless/marvell/mwifiex/init.c | 19 -------------------
> >  1 file changed, 19 deletions(-)
> >
> > diff --git a/drivers/net/wireless/marvell/mwifiex/init.c
> > b/drivers/net/wireless/marvell/mwifiex/init.c
> > index 8e5e424..365efb8 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/init.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/init.c
> > @@ -407,8 +407,6 @@ static void mwifiex_free_lock_list(struct
> > mwifiex_adapter *adapter)  static void  mwifiex_adapter_cleanup(struct
> > mwifiex_adapter *adapter)  {
> > -	int idx;
> > -
> >  	if (!adapter) {
> >  		pr_err("%s: adapter is NULL\n", __func__);
> >  		return;
> > @@ -426,23 +424,6 @@ mwifiex_adapter_cleanup(struct mwifiex_adapter
> *adapter)
> >  	mwifiex_dbg(adapter, INFO, "info: free cmd buffer\n");
> >  	mwifiex_free_cmd_buffer(adapter);
> >
> > -	for (idx = 0; idx < adapter->num_mem_types; idx++) {
> > -		struct memory_type_mapping *entry =
> > -				&adapter->mem_type_mapping_tbl[idx];
> > -
> > -		if (entry->mem_ptr) {
> > -			vfree(entry->mem_ptr);
> > -			entry->mem_ptr = NULL;
> > -		}
> > -		entry->mem_size = 0;
> > -	}
> > -
> > -	if (adapter->drv_info_dump) {
> > -		vfree(adapter->drv_info_dump);
> > -		adapter->drv_info_dump = NULL;
> > -		adapter->drv_info_size = 0;
> > -	}
> 
> Why do you even keep the pointer to dump memory in the adapter
> structure? You allocate it in mwifiex_drv_info_dump() and immediately
> use it in mwifiex_upload_device_dump(). Why not simply pass the pointer
> between the functions?
> 

Thanks. This makes sense. I will pass the pointer and get rid of adapter variable in updated version.

Regards,
Amitkumar