diff mbox series

[v1] wireless/at76c50x-usb.c : Use devm_kzalloc replaces kmalloc

Message ID 20230113133503.58336-1-sensor1010@163.com (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series [v1] wireless/at76c50x-usb.c : Use devm_kzalloc replaces kmalloc | expand

Commit Message

李哲 Jan. 13, 2023, 1:35 p.m. UTC 
use devm_kzalloc replaces kamlloc

Signed-off-by: Lizhe <sensor1010@163.com>
---
 drivers/net/wireless/atmel/at76c50x-usb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Eric Dumazet Jan. 13, 2023, 1:44 p.m. UTC | #1 
On Fri, Jan 13, 2023 at 2:35 PM Lizhe <sensor1010@163.com> wrote:
>
> use devm_kzalloc replaces kamlloc
>
> Signed-off-by: Lizhe <sensor1010@163.com>
> ---
>  drivers/net/wireless/atmel/at76c50x-usb.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/atmel/at76c50x-usb.c b/drivers/net/wireless/atmel/at76c50x-usb.c
> index 009bca34ece3..ebd8ef525557 100644
> --- a/drivers/net/wireless/atmel/at76c50x-usb.c
> +++ b/drivers/net/wireless/atmel/at76c50x-usb.c
> @@ -2444,7 +2444,7 @@ static int at76_probe(struct usb_interface *interface,
>
>         udev = usb_get_dev(interface_to_usbdev(interface));
>
> -       fwv = kmalloc(sizeof(*fwv), GFP_KERNEL);
> +       fwv = devm_kzalloc(sizeof(*fwv), GFP_KERNEL);

Have you compiled this patch ?

>         if (!fwv) {
>                 ret = -ENOMEM;
>                 goto exit;
> @@ -2535,7 +2535,6 @@ static int at76_probe(struct usb_interface *interface,
>                 at76_delete_device(priv);
>
>  exit:
> -       kfree(fwv);
>         if (ret < 0)
>                 usb_put_dev(udev);
>         return ret;
> --
> 2.17.1
>
kernel test robot Jan. 13, 2023, 9:15 p.m. UTC | #2 
Hi Lizhe,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on wireless-next/main]
[also build test WARNING on wireless/main net-next/master net/master linus/master v6.2-rc3 next-20230113]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Lizhe/wireless-at76c50x-usb-c-Use-devm_kzalloc-replaces-kmalloc/20230113-214552
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
patch link:    https://lore.kernel.org/r/20230113133503.58336-1-sensor1010%40163.com
patch subject: [PATCH v1] wireless/at76c50x-usb.c : Use devm_kzalloc replaces kmalloc
config: m68k-allyesconfig
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/8158106c5c3501cdef60ac5af6c604ea07cb7561
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Lizhe/wireless-at76c50x-usb-c-Use-devm_kzalloc-replaces-kmalloc/20230113-214552
        git checkout 8158106c5c3501cdef60ac5af6c604ea07cb7561
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/net/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/net/wireless/atmel/at76c50x-usb.c: In function 'at76_probe':
>> drivers/net/wireless/atmel/at76c50x-usb.c:2447:28: warning: passing argument 1 of 'devm_kzalloc' makes pointer from integer without a cast [-Wint-conversion]
    2447 |         fwv = devm_kzalloc(sizeof(*fwv), GFP_KERNEL);
         |                            ^~~~~~~~~~~~
         |                            |
         |                            unsigned int
   In file included from include/linux/usb.h:19,
                    from drivers/net/wireless/atmel/at76c50x-usb.c:31:
   include/linux/device.h:207:49: note: expected 'struct device *' but argument is of type 'unsigned int'
     207 | static inline void *devm_kzalloc(struct device *dev, size_t size, gfp_t gfp)
         |                                  ~~~~~~~~~~~~~~~^~~
   drivers/net/wireless/atmel/at76c50x-usb.c:2447:15: error: too few arguments to function 'devm_kzalloc'
    2447 |         fwv = devm_kzalloc(sizeof(*fwv), GFP_KERNEL);
         |               ^~~~~~~~~~~~
   include/linux/device.h:207:21: note: declared here
     207 | static inline void *devm_kzalloc(struct device *dev, size_t size, gfp_t gfp)
         |                     ^~~~~~~~~~~~


vim +/devm_kzalloc +2447 drivers/net/wireless/atmel/at76c50x-usb.c

  2432	
  2433	static int at76_probe(struct usb_interface *interface,
  2434			      const struct usb_device_id *id)
  2435	{
  2436		int ret;
  2437		struct at76_priv *priv;
  2438		struct fwentry *fwe;
  2439		struct usb_device *udev;
  2440		int op_mode;
  2441		int need_ext_fw = 0;
  2442		struct mib_fw_version *fwv = NULL;
  2443		int board_type = (int)id->driver_info;
  2444	
  2445		udev = usb_get_dev(interface_to_usbdev(interface));
  2446	
> 2447		fwv = devm_kzalloc(sizeof(*fwv), GFP_KERNEL);
  2448		if (!fwv) {
  2449			ret = -ENOMEM;
  2450			goto exit;
  2451		}
  2452	
  2453		/* Load firmware into kernel memory */
  2454		fwe = at76_load_firmware(udev, board_type);
  2455		if (!fwe) {
  2456			ret = -ENOENT;
  2457			goto exit;
  2458		}
  2459	
  2460		op_mode = at76_get_op_mode(udev);
  2461	
  2462		at76_dbg(DBG_DEVSTART, "opmode %d", op_mode);
  2463	
  2464		/* we get OPMODE_NONE with 2.4.23, SMC2662W-AR ???
  2465		   we get 204 with 2.4.23, Fiberline FL-WL240u (505A+RFMD2958) ??? */
  2466	
  2467		if (op_mode == OPMODE_HW_CONFIG_MODE) {
  2468			dev_err(&interface->dev,
  2469				"cannot handle a device in HW_CONFIG_MODE\n");
  2470			ret = -EBUSY;
  2471			goto exit;
  2472		}
  2473	
  2474		if (op_mode != OPMODE_NORMAL_NIC_WITH_FLASH
  2475		    && op_mode != OPMODE_NORMAL_NIC_WITHOUT_FLASH) {
  2476			/* download internal firmware part */
  2477			dev_printk(KERN_DEBUG, &interface->dev,
  2478				   "downloading internal firmware\n");
  2479			ret = at76_load_internal_fw(udev, fwe);
  2480			if (ret < 0) {
  2481				dev_err(&interface->dev,
  2482					"error %d downloading internal firmware\n",
  2483					ret);
  2484			}
  2485			goto exit;
  2486		}
  2487	
  2488		/* Internal firmware already inside the device.  Get firmware
  2489		 * version to test if external firmware is loaded.
  2490		 * This works only for newer firmware, e.g. the Intersil 0.90.x
  2491		 * says "control timeout on ep0in" and subsequent
  2492		 * at76_get_op_mode() fail too :-( */
  2493	
  2494		/* if version >= 0.100.x.y or device with built-in flash we can
  2495		 * query the device for the fw version */
  2496		if ((fwe->fw_version.major > 0 || fwe->fw_version.minor >= 100)
  2497		    || (op_mode == OPMODE_NORMAL_NIC_WITH_FLASH)) {
  2498			ret = at76_get_mib(udev, MIB_FW_VERSION, fwv, sizeof(*fwv));
  2499			if (ret < 0 || (fwv->major | fwv->minor) == 0)
  2500				need_ext_fw = 1;
  2501		} else
  2502			/* No way to check firmware version, reload to be sure */
  2503			need_ext_fw = 1;
  2504	
  2505		if (need_ext_fw) {
  2506			dev_printk(KERN_DEBUG, &interface->dev,
  2507				   "downloading external firmware\n");
  2508	
  2509			ret = at76_load_external_fw(udev, fwe);
  2510			if (ret < 0)
  2511				goto exit;
  2512	
  2513			/* Re-check firmware version */
  2514			ret = at76_get_mib(udev, MIB_FW_VERSION, fwv, sizeof(*fwv));
  2515			if (ret < 0) {
  2516				dev_err(&interface->dev,
  2517					"error %d getting firmware version\n", ret);
  2518				goto exit;
  2519			}
  2520		}
  2521	
  2522		priv = at76_alloc_new_device(udev);
  2523		if (!priv) {
  2524			ret = -ENOMEM;
  2525			goto exit;
  2526		}
  2527	
  2528		usb_set_intfdata(interface, priv);
  2529	
  2530		memcpy(&priv->fw_version, fwv, sizeof(struct mib_fw_version));
  2531		priv->board_type = board_type;
  2532	
  2533		ret = at76_init_new_device(priv, interface);
  2534		if (ret < 0)
  2535			at76_delete_device(priv);
  2536	
  2537	exit:
  2538		if (ret < 0)
  2539			usb_put_dev(udev);
  2540		return ret;
  2541	}
  2542
kernel test robot Jan. 14, 2023, 7:39 p.m. UTC | #3 
Hi Lizhe,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on wireless-next/main]
[also build test ERROR on wireless/main net-next/master net/master linus/master v6.2-rc3 next-20230113]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Lizhe/wireless-at76c50x-usb-c-Use-devm_kzalloc-replaces-kmalloc/20230113-214552
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
patch link:    https://lore.kernel.org/r/20230113133503.58336-1-sensor1010%40163.com
patch subject: [PATCH v1] wireless/at76c50x-usb.c : Use devm_kzalloc replaces kmalloc
config: sh-allmodconfig
compiler: sh4-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/8158106c5c3501cdef60ac5af6c604ea07cb7561
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Lizhe/wireless-at76c50x-usb-c-Use-devm_kzalloc-replaces-kmalloc/20230113-214552
        git checkout 8158106c5c3501cdef60ac5af6c604ea07cb7561
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sh olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sh SHELL=/bin/bash drivers/net/wireless/atmel/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/net/wireless/atmel/at76c50x-usb.c: In function 'at76_probe':
   drivers/net/wireless/atmel/at76c50x-usb.c:2447:28: warning: passing argument 1 of 'devm_kzalloc' makes pointer from integer without a cast [-Wint-conversion]
    2447 |         fwv = devm_kzalloc(sizeof(*fwv), GFP_KERNEL);
         |                            ^~~~~~~~~~~~
         |                            |
         |                            unsigned int
   In file included from include/linux/usb.h:19,
                    from drivers/net/wireless/atmel/at76c50x-usb.c:31:
   include/linux/device.h:207:49: note: expected 'struct device *' but argument is of type 'unsigned int'
     207 | static inline void *devm_kzalloc(struct device *dev, size_t size, gfp_t gfp)
         |                                  ~~~~~~~~~~~~~~~^~~
>> drivers/net/wireless/atmel/at76c50x-usb.c:2447:15: error: too few arguments to function 'devm_kzalloc'
    2447 |         fwv = devm_kzalloc(sizeof(*fwv), GFP_KERNEL);
         |               ^~~~~~~~~~~~
   include/linux/device.h:207:21: note: declared here
     207 | static inline void *devm_kzalloc(struct device *dev, size_t size, gfp_t gfp)
         |                     ^~~~~~~~~~~~


vim +/devm_kzalloc +2447 drivers/net/wireless/atmel/at76c50x-usb.c

  2432	
  2433	static int at76_probe(struct usb_interface *interface,
  2434			      const struct usb_device_id *id)
  2435	{
  2436		int ret;
  2437		struct at76_priv *priv;
  2438		struct fwentry *fwe;
  2439		struct usb_device *udev;
  2440		int op_mode;
  2441		int need_ext_fw = 0;
  2442		struct mib_fw_version *fwv = NULL;
  2443		int board_type = (int)id->driver_info;
  2444	
  2445		udev = usb_get_dev(interface_to_usbdev(interface));
  2446	
> 2447		fwv = devm_kzalloc(sizeof(*fwv), GFP_KERNEL);
  2448		if (!fwv) {
  2449			ret = -ENOMEM;
  2450			goto exit;
  2451		}
  2452	
  2453		/* Load firmware into kernel memory */
  2454		fwe = at76_load_firmware(udev, board_type);
  2455		if (!fwe) {
  2456			ret = -ENOENT;
  2457			goto exit;
  2458		}
  2459	
  2460		op_mode = at76_get_op_mode(udev);
  2461	
  2462		at76_dbg(DBG_DEVSTART, "opmode %d", op_mode);
  2463	
  2464		/* we get OPMODE_NONE with 2.4.23, SMC2662W-AR ???
  2465		   we get 204 with 2.4.23, Fiberline FL-WL240u (505A+RFMD2958) ??? */
  2466	
  2467		if (op_mode == OPMODE_HW_CONFIG_MODE) {
  2468			dev_err(&interface->dev,
  2469				"cannot handle a device in HW_CONFIG_MODE\n");
  2470			ret = -EBUSY;
  2471			goto exit;
  2472		}
  2473	
  2474		if (op_mode != OPMODE_NORMAL_NIC_WITH_FLASH
  2475		    && op_mode != OPMODE_NORMAL_NIC_WITHOUT_FLASH) {
  2476			/* download internal firmware part */
  2477			dev_printk(KERN_DEBUG, &interface->dev,
  2478				   "downloading internal firmware\n");
  2479			ret = at76_load_internal_fw(udev, fwe);
  2480			if (ret < 0) {
  2481				dev_err(&interface->dev,
  2482					"error %d downloading internal firmware\n",
  2483					ret);
  2484			}
  2485			goto exit;
  2486		}
  2487	
  2488		/* Internal firmware already inside the device.  Get firmware
  2489		 * version to test if external firmware is loaded.
  2490		 * This works only for newer firmware, e.g. the Intersil 0.90.x
  2491		 * says "control timeout on ep0in" and subsequent
  2492		 * at76_get_op_mode() fail too :-( */
  2493	
  2494		/* if version >= 0.100.x.y or device with built-in flash we can
  2495		 * query the device for the fw version */
  2496		if ((fwe->fw_version.major > 0 || fwe->fw_version.minor >= 100)
  2497		    || (op_mode == OPMODE_NORMAL_NIC_WITH_FLASH)) {
  2498			ret = at76_get_mib(udev, MIB_FW_VERSION, fwv, sizeof(*fwv));
  2499			if (ret < 0 || (fwv->major | fwv->minor) == 0)
  2500				need_ext_fw = 1;
  2501		} else
  2502			/* No way to check firmware version, reload to be sure */
  2503			need_ext_fw = 1;
  2504	
  2505		if (need_ext_fw) {
  2506			dev_printk(KERN_DEBUG, &interface->dev,
  2507				   "downloading external firmware\n");
  2508	
  2509			ret = at76_load_external_fw(udev, fwe);
  2510			if (ret < 0)
  2511				goto exit;
  2512	
  2513			/* Re-check firmware version */
  2514			ret = at76_get_mib(udev, MIB_FW_VERSION, fwv, sizeof(*fwv));
  2515			if (ret < 0) {
  2516				dev_err(&interface->dev,
  2517					"error %d getting firmware version\n", ret);
  2518				goto exit;
  2519			}
  2520		}
  2521	
  2522		priv = at76_alloc_new_device(udev);
  2523		if (!priv) {
  2524			ret = -ENOMEM;
  2525			goto exit;
  2526		}
  2527	
  2528		usb_set_intfdata(interface, priv);
  2529	
  2530		memcpy(&priv->fw_version, fwv, sizeof(struct mib_fw_version));
  2531		priv->board_type = board_type;
  2532	
  2533		ret = at76_init_new_device(priv, interface);
  2534		if (ret < 0)
  2535			at76_delete_device(priv);
  2536	
  2537	exit:
  2538		if (ret < 0)
  2539			usb_put_dev(udev);
  2540		return ret;
  2541	}
  2542
Kalle Valo Jan. 16, 2023, 6:32 a.m. UTC | #4 
Eric Dumazet <edumazet@google.com> writes:

> On Fri, Jan 13, 2023 at 2:35 PM Lizhe <sensor1010@163.com> wrote:
>>
>> use devm_kzalloc replaces kamlloc
>>
>> Signed-off-by: Lizhe <sensor1010@163.com>
>> ---
>>  drivers/net/wireless/atmel/at76c50x-usb.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/atmel/at76c50x-usb.c b/drivers/net/wireless/atmel/at76c50x-usb.c
>> index 009bca34ece3..ebd8ef525557 100644
>> --- a/drivers/net/wireless/atmel/at76c50x-usb.c
>> +++ b/drivers/net/wireless/atmel/at76c50x-usb.c
>> @@ -2444,7 +2444,7 @@ static int at76_probe(struct usb_interface *interface,
>>
>>         udev = usb_get_dev(interface_to_usbdev(interface));
>>
>> -       fwv = kmalloc(sizeof(*fwv), GFP_KERNEL);
>> +       fwv = devm_kzalloc(sizeof(*fwv), GFP_KERNEL);
>
> Have you compiled this patch ?

Clearly not:

https://lore.kernel.org/linux-wireless/202301140533.jMlST9Ur-lkp@intel.com/

Lizhe, do not EVER submit untested patches. In some simple patches doing
just a compilation test might suffice, but please mention the patch is
only compile tested clearly in the commit log. But not even doing a
compilation test is a big no.
diff mbox series

Patch

diff --git a/drivers/net/wireless/atmel/at76c50x-usb.c b/drivers/net/wireless/atmel/at76c50x-usb.c
index 009bca34ece3..ebd8ef525557 100644
--- a/drivers/net/wireless/atmel/at76c50x-usb.c
+++ b/drivers/net/wireless/atmel/at76c50x-usb.c
@@ -2444,7 +2444,7 @@  static int at76_probe(struct usb_interface *interface,
 
 	udev = usb_get_dev(interface_to_usbdev(interface));
 
-	fwv = kmalloc(sizeof(*fwv), GFP_KERNEL);
+	fwv = devm_kzalloc(sizeof(*fwv), GFP_KERNEL);
 	if (!fwv) {
 		ret = -ENOMEM;
 		goto exit;
@@ -2535,7 +2535,6 @@  static int at76_probe(struct usb_interface *interface,
 		at76_delete_device(priv);
 
 exit:
-	kfree(fwv);
 	if (ret < 0)
 		usb_put_dev(udev);
 	return ret;