diff mbox series

[6/9] usb: gadget: eliminate anonymous module_init & module_exit

Message ID 20220316192010.19001-7-rdunlap@infradead.org (mailing list archive)
State Not Applicable
Delegated to: Herbert Xu
Headers show
Series treewide: eliminate anonymous module_init & module_exit | expand

Commit Message

Randy Dunlap March 16, 2022, 7:20 p.m. UTC
Eliminate anonymous module_init() and module_exit(), which can lead to
confusion or ambiguity when reading System.map, crashes/oops/bugs,
or an initcall_debug log.

Give each of these init and exit functions unique driver-specific
names to eliminate the anonymous names.

Example 1: (System.map)
 ffffffff832fc78c t init
 ffffffff832fc79e t init
 ffffffff832fc8f8 t init

Example 2: (initcall_debug log)
 calling  init+0x0/0x12 @ 1
 initcall init+0x0/0x12 returned 0 after 15 usecs
 calling  init+0x0/0x60 @ 1
 initcall init+0x0/0x60 returned 0 after 2 usecs
 calling  init+0x0/0x9a @ 1
 initcall init+0x0/0x9a returned 0 after 74 usecs

Fixes: bd25a14edb75 ("usb: gadget: legacy/serial: allow dynamic removal")
Fixes: 7bb5ea54be47 ("usb gadget serial: use composite gadget framework")
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: linux-usb@vger.kernel.org
---
 drivers/usb/gadget/legacy/inode.c  |    8 ++++----
 drivers/usb/gadget/legacy/serial.c |   10 +++++-----
 drivers/usb/gadget/udc/dummy_hcd.c |    8 ++++----
 3 files changed, 13 insertions(+), 13 deletions(-)

Comments

Ira Weiny March 17, 2022, 3:29 a.m. UTC | #1
On Wed, Mar 16, 2022 at 12:20:07PM -0700, Randy Dunlap wrote:
> Eliminate anonymous module_init() and module_exit(), which can lead to
> confusion or ambiguity when reading System.map, crashes/oops/bugs,
> or an initcall_debug log.
> 
> Give each of these init and exit functions unique driver-specific
> names to eliminate the anonymous names.
> 
> Example 1: (System.map)
>  ffffffff832fc78c t init
>  ffffffff832fc79e t init
>  ffffffff832fc8f8 t init
> 
> Example 2: (initcall_debug log)
>  calling  init+0x0/0x12 @ 1
>  initcall init+0x0/0x12 returned 0 after 15 usecs
>  calling  init+0x0/0x60 @ 1
>  initcall init+0x0/0x60 returned 0 after 2 usecs
>  calling  init+0x0/0x9a @ 1
>  initcall init+0x0/0x9a returned 0 after 74 usecs
> 
> Fixes: bd25a14edb75 ("usb: gadget: legacy/serial: allow dynamic removal")
> Fixes: 7bb5ea54be47 ("usb gadget serial: use composite gadget framework")
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")

I continue to be confused about the latest rules for the Fixes tag but this one
in particular seems completely useless.  This is the 'beginning of time' commit
by Linus AFAICT.  So do any of these Fixes tags need to be in this series?

Regardless:

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Cc: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: linux-usb@vger.kernel.org
> ---
>  drivers/usb/gadget/legacy/inode.c  |    8 ++++----
>  drivers/usb/gadget/legacy/serial.c |   10 +++++-----
>  drivers/usb/gadget/udc/dummy_hcd.c |    8 ++++----
>  3 files changed, 13 insertions(+), 13 deletions(-)
> 
> --- lnx-517-rc8.orig/drivers/usb/gadget/legacy/serial.c
> +++ lnx-517-rc8/drivers/usb/gadget/legacy/serial.c
> @@ -273,7 +273,7 @@ static struct usb_composite_driver gseri
>  static int switch_gserial_enable(bool do_enable)
>  {
>  	if (!serial_config_driver.label)
> -		/* init() was not called, yet */
> +		/* gserial_init() was not called, yet */
>  		return 0;
>  
>  	if (do_enable)
> @@ -283,7 +283,7 @@ static int switch_gserial_enable(bool do
>  	return 0;
>  }
>  
> -static int __init init(void)
> +static int __init gserial_init(void)
>  {
>  	/* We *could* export two configs; that'd be much cleaner...
>  	 * but neither of these product IDs was defined that way.
> @@ -314,11 +314,11 @@ static int __init init(void)
>  
>  	return usb_composite_probe(&gserial_driver);
>  }
> -module_init(init);
> +module_init(gserial_init);
>  
> -static void __exit cleanup(void)
> +static void __exit gserial_cleanup(void)
>  {
>  	if (enable)
>  		usb_composite_unregister(&gserial_driver);
>  }
> -module_exit(cleanup);
> +module_exit(gserial_cleanup);
> --- lnx-517-rc8.orig/drivers/usb/gadget/udc/dummy_hcd.c
> +++ lnx-517-rc8/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -2765,7 +2765,7 @@ static struct platform_driver dummy_hcd_
>  static struct platform_device *the_udc_pdev[MAX_NUM_UDC];
>  static struct platform_device *the_hcd_pdev[MAX_NUM_UDC];
>  
> -static int __init init(void)
> +static int __init dummy_hcd_init(void)
>  {
>  	int	retval = -ENOMEM;
>  	int	i;
> @@ -2887,9 +2887,9 @@ err_alloc_udc:
>  		platform_device_put(the_hcd_pdev[i]);
>  	return retval;
>  }
> -module_init(init);
> +module_init(dummy_hcd_init);
>  
> -static void __exit cleanup(void)
> +static void __exit dummy_hcd_cleanup(void)
>  {
>  	int i;
>  
> @@ -2905,4 +2905,4 @@ static void __exit cleanup(void)
>  	platform_driver_unregister(&dummy_udc_driver);
>  	platform_driver_unregister(&dummy_hcd_driver);
>  }
> -module_exit(cleanup);
> +module_exit(dummy_hcd_cleanup);
> --- lnx-517-rc8.orig/drivers/usb/gadget/legacy/inode.c
> +++ lnx-517-rc8/drivers/usb/gadget/legacy/inode.c
> @@ -2101,7 +2101,7 @@ MODULE_ALIAS_FS("gadgetfs");
>  
>  /*----------------------------------------------------------------------*/
>  
> -static int __init init (void)
> +static int __init gadgetfs_init (void)
>  {
>  	int status;
>  
> @@ -2111,12 +2111,12 @@ static int __init init (void)
>  			shortname, driver_desc);
>  	return status;
>  }
> -module_init (init);
> +module_init (gadgetfs_init);
>  
> -static void __exit cleanup (void)
> +static void __exit gadgetfs_cleanup (void)
>  {
>  	pr_debug ("unregister %s\n", shortname);
>  	unregister_filesystem (&gadgetfs_type);
>  }
> -module_exit (cleanup);
> +module_exit (gadgetfs_cleanup);
>
Randy Dunlap March 17, 2022, 4:59 a.m. UTC | #2
On 3/16/22 20:29, Ira Weiny wrote:
> On Wed, Mar 16, 2022 at 12:20:07PM -0700, Randy Dunlap wrote:
>> Eliminate anonymous module_init() and module_exit(), which can lead to
>> confusion or ambiguity when reading System.map, crashes/oops/bugs,
>> or an initcall_debug log.
>>
>> Give each of these init and exit functions unique driver-specific
>> names to eliminate the anonymous names.
>>
>> Example 1: (System.map)
>>  ffffffff832fc78c t init
>>  ffffffff832fc79e t init
>>  ffffffff832fc8f8 t init
>>
>> Example 2: (initcall_debug log)
>>  calling  init+0x0/0x12 @ 1
>>  initcall init+0x0/0x12 returned 0 after 15 usecs
>>  calling  init+0x0/0x60 @ 1
>>  initcall init+0x0/0x60 returned 0 after 2 usecs
>>  calling  init+0x0/0x9a @ 1
>>  initcall init+0x0/0x9a returned 0 after 74 usecs
>>
>> Fixes: bd25a14edb75 ("usb: gadget: legacy/serial: allow dynamic removal")
>> Fixes: 7bb5ea54be47 ("usb gadget serial: use composite gadget framework")
>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> 
> I continue to be confused about the latest rules for the Fixes tag but this one
> in particular seems completely useless.  This is the 'beginning of time' commit
> by Linus AFAICT.  So do any of these Fixes tags need to be in this series?

I guess it mostly depends on whether they get applied to stable trees, but
it's entirely fine with me if they don't.

{I also corrected Felipe's email address here.}

> Regardless:
> 
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>

Thanks.

> 
>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
>> Cc: Michał Mirosław <mirq-linux@rere.qmqm.pl>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>> Cc: linux-usb@vger.kernel.org
>> ---
>>  drivers/usb/gadget/legacy/inode.c  |    8 ++++----
>>  drivers/usb/gadget/legacy/serial.c |   10 +++++-----
>>  drivers/usb/gadget/udc/dummy_hcd.c |    8 ++++----
>>  3 files changed, 13 insertions(+), 13 deletions(-)
>>
>> --- lnx-517-rc8.orig/drivers/usb/gadget/legacy/serial.c
>> +++ lnx-517-rc8/drivers/usb/gadget/legacy/serial.c
>> @@ -273,7 +273,7 @@ static struct usb_composite_driver gseri
>>  static int switch_gserial_enable(bool do_enable)
>>  {
>>  	if (!serial_config_driver.label)
>> -		/* init() was not called, yet */
>> +		/* gserial_init() was not called, yet */
>>  		return 0;
>>  
>>  	if (do_enable)
>> @@ -283,7 +283,7 @@ static int switch_gserial_enable(bool do
>>  	return 0;
>>  }
>>  
>> -static int __init init(void)
>> +static int __init gserial_init(void)
>>  {
>>  	/* We *could* export two configs; that'd be much cleaner...
>>  	 * but neither of these product IDs was defined that way.
>> @@ -314,11 +314,11 @@ static int __init init(void)
>>  
>>  	return usb_composite_probe(&gserial_driver);
>>  }
>> -module_init(init);
>> +module_init(gserial_init);
>>  
>> -static void __exit cleanup(void)
>> +static void __exit gserial_cleanup(void)
>>  {
>>  	if (enable)
>>  		usb_composite_unregister(&gserial_driver);
>>  }
>> -module_exit(cleanup);
>> +module_exit(gserial_cleanup);
>> --- lnx-517-rc8.orig/drivers/usb/gadget/udc/dummy_hcd.c
>> +++ lnx-517-rc8/drivers/usb/gadget/udc/dummy_hcd.c
>> @@ -2765,7 +2765,7 @@ static struct platform_driver dummy_hcd_
>>  static struct platform_device *the_udc_pdev[MAX_NUM_UDC];
>>  static struct platform_device *the_hcd_pdev[MAX_NUM_UDC];
>>  
>> -static int __init init(void)
>> +static int __init dummy_hcd_init(void)
>>  {
>>  	int	retval = -ENOMEM;
>>  	int	i;
>> @@ -2887,9 +2887,9 @@ err_alloc_udc:
>>  		platform_device_put(the_hcd_pdev[i]);
>>  	return retval;
>>  }
>> -module_init(init);
>> +module_init(dummy_hcd_init);
>>  
>> -static void __exit cleanup(void)
>> +static void __exit dummy_hcd_cleanup(void)
>>  {
>>  	int i;
>>  
>> @@ -2905,4 +2905,4 @@ static void __exit cleanup(void)
>>  	platform_driver_unregister(&dummy_udc_driver);
>>  	platform_driver_unregister(&dummy_hcd_driver);
>>  }
>> -module_exit(cleanup);
>> +module_exit(dummy_hcd_cleanup);
>> --- lnx-517-rc8.orig/drivers/usb/gadget/legacy/inode.c
>> +++ lnx-517-rc8/drivers/usb/gadget/legacy/inode.c
>> @@ -2101,7 +2101,7 @@ MODULE_ALIAS_FS("gadgetfs");
>>  
>>  /*----------------------------------------------------------------------*/
>>  
>> -static int __init init (void)
>> +static int __init gadgetfs_init (void)
>>  {
>>  	int status;
>>  
>> @@ -2111,12 +2111,12 @@ static int __init init (void)
>>  			shortname, driver_desc);
>>  	return status;
>>  }
>> -module_init (init);
>> +module_init (gadgetfs_init);
>>  
>> -static void __exit cleanup (void)
>> +static void __exit gadgetfs_cleanup (void)
>>  {
>>  	pr_debug ("unregister %s\n", shortname);
>>  	unregister_filesystem (&gadgetfs_type);
>>  }
>> -module_exit (cleanup);
>> +module_exit (gadgetfs_cleanup);
>>
diff mbox series

Patch

--- lnx-517-rc8.orig/drivers/usb/gadget/legacy/serial.c
+++ lnx-517-rc8/drivers/usb/gadget/legacy/serial.c
@@ -273,7 +273,7 @@  static struct usb_composite_driver gseri
 static int switch_gserial_enable(bool do_enable)
 {
 	if (!serial_config_driver.label)
-		/* init() was not called, yet */
+		/* gserial_init() was not called, yet */
 		return 0;
 
 	if (do_enable)
@@ -283,7 +283,7 @@  static int switch_gserial_enable(bool do
 	return 0;
 }
 
-static int __init init(void)
+static int __init gserial_init(void)
 {
 	/* We *could* export two configs; that'd be much cleaner...
 	 * but neither of these product IDs was defined that way.
@@ -314,11 +314,11 @@  static int __init init(void)
 
 	return usb_composite_probe(&gserial_driver);
 }
-module_init(init);
+module_init(gserial_init);
 
-static void __exit cleanup(void)
+static void __exit gserial_cleanup(void)
 {
 	if (enable)
 		usb_composite_unregister(&gserial_driver);
 }
-module_exit(cleanup);
+module_exit(gserial_cleanup);
--- lnx-517-rc8.orig/drivers/usb/gadget/udc/dummy_hcd.c
+++ lnx-517-rc8/drivers/usb/gadget/udc/dummy_hcd.c
@@ -2765,7 +2765,7 @@  static struct platform_driver dummy_hcd_
 static struct platform_device *the_udc_pdev[MAX_NUM_UDC];
 static struct platform_device *the_hcd_pdev[MAX_NUM_UDC];
 
-static int __init init(void)
+static int __init dummy_hcd_init(void)
 {
 	int	retval = -ENOMEM;
 	int	i;
@@ -2887,9 +2887,9 @@  err_alloc_udc:
 		platform_device_put(the_hcd_pdev[i]);
 	return retval;
 }
-module_init(init);
+module_init(dummy_hcd_init);
 
-static void __exit cleanup(void)
+static void __exit dummy_hcd_cleanup(void)
 {
 	int i;
 
@@ -2905,4 +2905,4 @@  static void __exit cleanup(void)
 	platform_driver_unregister(&dummy_udc_driver);
 	platform_driver_unregister(&dummy_hcd_driver);
 }
-module_exit(cleanup);
+module_exit(dummy_hcd_cleanup);
--- lnx-517-rc8.orig/drivers/usb/gadget/legacy/inode.c
+++ lnx-517-rc8/drivers/usb/gadget/legacy/inode.c
@@ -2101,7 +2101,7 @@  MODULE_ALIAS_FS("gadgetfs");
 
 /*----------------------------------------------------------------------*/
 
-static int __init init (void)
+static int __init gadgetfs_init (void)
 {
 	int status;
 
@@ -2111,12 +2111,12 @@  static int __init init (void)
 			shortname, driver_desc);
 	return status;
 }
-module_init (init);
+module_init (gadgetfs_init);
 
-static void __exit cleanup (void)
+static void __exit gadgetfs_cleanup (void)
 {
 	pr_debug ("unregister %s\n", shortname);
 	unregister_filesystem (&gadgetfs_type);
 }
-module_exit (cleanup);
+module_exit (gadgetfs_cleanup);