On Tue, Apr 11, 2017 at 10:11:43PM +0530, Aditya Shankar wrote:
> Change the config packet format used in handle_set_wfi_drv_handler()
> to align the host driver with the new format used in the wilc firmware.
> 
> The change updates the format in which the host driver provides the
> firmware with the drv_handler index and also uses two new
> fields viz. "mode" and 'name" in the config packet along with this index
> to directly provide details about the interface and its mode to the
> firmware instead of having multiple if-else statements in the host driver
> to decide which interface to configure.
> 
> Signed-off-by: Aditya Shankar <aditya.shan...@microchip.com>
> Reviewed-by: Arend Van Spriel <arend.vanspr...@broadcom.com>
> ---
> Change in v2: Fix build warning
> Change in v3: Address review comments from v2
> ---
>  drivers/staging/wilc1000/host_interface.c         | 48 
> ++++++++++++++++++-----
>  drivers/staging/wilc1000/host_interface.h         |  9 ++++-
>  drivers/staging/wilc1000/linux_wlan.c             | 37 +++++------------
>  drivers/staging/wilc1000/wilc_wfi_cfgoperations.c |  2 +-
>  drivers/staging/wilc1000/wilc_wfi_netdevice.h     |  1 +
>  drivers/staging/wilc1000/wilc_wlan_if.h           |  2 +-
>  6 files changed, 59 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/host_interface.c 
> b/drivers/staging/wilc1000/host_interface.c
> index c3a8af0..7352488 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -334,14 +334,39 @@ static void handle_set_wfi_drv_handler(struct wilc_vif 
> *vif,
>  {
>       int ret = 0;
>       struct wid wid;
> +     u8 *currbyte, *buffer;
> +     struct host_if_drv *hif_drv = NULL;
> +
> +     if (!vif->hif_drv)
> +             return;
> +
> +     if (!hif_drv_handler)
> +             return;
> +
> +     hif_drv = vif->hif_drv;
> +     buffer = kzalloc(DRV_HANDLER_SIZE, GFP_ATOMIC);
> +
> +     if (!buffer)
> +             return;

Test on the line right after allocation.

And there is no way to return an error from this function?  That seems
bad.

And why do you need GFP_ATOMIC?

And finally, where do you free this buffer?

> @@ -3449,8 +3477,10 @@ void wilc_network_info_received(struct wilc *wilc, u8 
> *buffer, u32 length)
>  
>       id = ((buffer[length - 4]) | (buffer[length - 3] << 8) | (buffer[length 
> - 2] << 16) | (buffer[length - 1] << 24));
>       vif = wilc_get_vif_from_idx(wilc, id);
> +
>       if (!vif)
>               return;
> +
>       hif_drv = vif->hif_drv;
>  
>       if (!hif_drv || hif_drv == terminated_handle)   {

Unneeded whitespace changes.  Don't do that in a patch that does other
things.

thanks,

greg k-h
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to