All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] USB: move usb debugfs directory creation to the usb common core
@ 2019-06-05  9:28 ` Greg Kroah-Hartman
  0 siblings, 0 replies; 16+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-05  9:28 UTC (permalink / raw)
  To: Felipe Balbi, Chunfeng Yun
  Cc: Matthias Brugger, linux-usb, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek

The USB gadget subsystem wants to use the USB debugfs root directory, so
move it to the common "core" USB code so that it is properly initialized
and removed as needed.

In order to properly do this, we need to load the common code before the
usb core code, when everything is linked into the kernel, so reorder the
link order of the code.

Also as the usb common code has the possibility of the led trigger logic
to be merged into it, handle the build option properly by only having
one module init/exit function and have the common code initialize the
led trigger if needed.

Reported-by: From: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---

Chunfeng, can you test this version to verify it works for you when
building the code into the kernel?

v2: handle led common code link error reported by kbuild
    handle subsys_initcall issue pointed out by Chunfeng

 drivers/usb/Makefile        |  3 +--
 drivers/usb/common/common.c | 21 +++++++++++++++++++++
 drivers/usb/common/common.h | 14 ++++++++++++++
 drivers/usb/common/led.c    |  9 +++------
 drivers/usb/core/usb.c      | 10 ++++------
 5 files changed, 43 insertions(+), 14 deletions(-)
 create mode 100644 drivers/usb/common/common.h

diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index 7d1b8c82b208..ecc2de1ffaae 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -5,6 +5,7 @@
 
 # Object files in subdirectories
 
+obj-$(CONFIG_USB_COMMON)	+= common/
 obj-$(CONFIG_USB)		+= core/
 obj-$(CONFIG_USB_SUPPORT)	+= phy/
 
@@ -60,8 +61,6 @@ obj-$(CONFIG_USB_CHIPIDEA)	+= chipidea/
 obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
 obj-$(CONFIG_USB_GADGET)	+= gadget/
 
-obj-$(CONFIG_USB_COMMON)	+= common/
-
 obj-$(CONFIG_USBIP_CORE)	+= usbip/
 
 obj-$(CONFIG_TYPEC)		+= typec/
diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index 18f5dcf58b0d..84a4423aaddf 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -15,6 +15,8 @@
 #include <linux/usb/of.h>
 #include <linux/usb/otg.h>
 #include <linux/of_platform.h>
+#include <linux/debugfs.h>
+#include "common.h"
 
 static const char *const ep_type_names[] = {
 	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
@@ -291,4 +293,23 @@ struct device *usb_of_get_companion_dev(struct device *dev)
 EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
 #endif
 
+struct dentry *usb_debug_root;
+EXPORT_SYMBOL_GPL(usb_debug_root);
+
+static int usb_common_init(void)
+{
+	usb_debug_root = debugfs_create_dir("usb", NULL);
+	ledtrig_usb_init();
+	return 0;
+}
+
+static void usb_common_exit(void)
+{
+	ledtrig_usb_exit();
+	debugfs_remove_recursive(usb_debug_root);
+}
+
+subsys_initcall(usb_common_init);
+module_exit(usb_common_exit);
+
 MODULE_LICENSE("GPL");
diff --git a/drivers/usb/common/common.h b/drivers/usb/common/common.h
new file mode 100644
index 000000000000..424a91316a4b
--- /dev/null
+++ b/drivers/usb/common/common.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __LINUX_USB_COMMON_H
+#define __LINUX_USB_COMMON_H
+
+#if defined(CONFIG_USB_LED_TRIG)
+void ledtrig_usb_init(void);
+void ledtrig_usb_exit(void);
+#else
+static inline void ledtrig_usb_init(void) { }
+static inline void ledtrig_usb_exit(void) { }
+#endif
+
+#endif	/* __LINUX_USB_COMMON_H */
diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
index 7bd81166b77d..0865dd44a80a 100644
--- a/drivers/usb/common/led.c
+++ b/drivers/usb/common/led.c
@@ -10,6 +10,7 @@
 #include <linux/init.h>
 #include <linux/leds.h>
 #include <linux/usb.h>
+#include "common.h"
 
 #define BLINK_DELAY 30
 
@@ -36,18 +37,14 @@ void usb_led_activity(enum usb_led_event ev)
 EXPORT_SYMBOL_GPL(usb_led_activity);
 
 
-static int __init ledtrig_usb_init(void)
+void __init ledtrig_usb_init(void)
 {
 	led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
 	led_trigger_register_simple("usb-host", &ledtrig_usb_host);
-	return 0;
 }
 
-static void __exit ledtrig_usb_exit(void)
+void __exit ledtrig_usb_exit(void)
 {
 	led_trigger_unregister_simple(ledtrig_usb_gadget);
 	led_trigger_unregister_simple(ledtrig_usb_host);
 }
-
-module_init(ledtrig_usb_init);
-module_exit(ledtrig_usb_exit);
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 7fcb9f782931..5a0df527a8ca 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
 	.notifier_call = usb_bus_notify,
 };
 
-struct dentry *usb_debug_root;
-EXPORT_SYMBOL_GPL(usb_debug_root);
+static struct dentry *usb_devices_root;
 
 static void usb_debugfs_init(void)
 {
-	usb_debug_root = debugfs_create_dir("usb", NULL);
-	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
-			    &usbfs_devices_fops);
+	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
+					       NULL, &usbfs_devices_fops);
 }
 
 static void usb_debugfs_cleanup(void)
 {
-	debugfs_remove_recursive(usb_debug_root);
+	debugfs_remove(usb_devices_root);
 }
 
 /*
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2] USB: move usb debugfs directory creation to the usb common core
@ 2019-06-05  9:28 ` Greg Kroah-Hartman
  0 siblings, 0 replies; 16+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-05  9:28 UTC (permalink / raw)
  To: Felipe Balbi, Chunfeng Yun
  Cc: devicetree, linux-usb, linux-kernel, linux-mediatek,
	Matthias Brugger, linux-arm-kernel

The USB gadget subsystem wants to use the USB debugfs root directory, so
move it to the common "core" USB code so that it is properly initialized
and removed as needed.

In order to properly do this, we need to load the common code before the
usb core code, when everything is linked into the kernel, so reorder the
link order of the code.

Also as the usb common code has the possibility of the led trigger logic
to be merged into it, handle the build option properly by only having
one module init/exit function and have the common code initialize the
led trigger if needed.

Reported-by: From: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---

Chunfeng, can you test this version to verify it works for you when
building the code into the kernel?

v2: handle led common code link error reported by kbuild
    handle subsys_initcall issue pointed out by Chunfeng

 drivers/usb/Makefile        |  3 +--
 drivers/usb/common/common.c | 21 +++++++++++++++++++++
 drivers/usb/common/common.h | 14 ++++++++++++++
 drivers/usb/common/led.c    |  9 +++------
 drivers/usb/core/usb.c      | 10 ++++------
 5 files changed, 43 insertions(+), 14 deletions(-)
 create mode 100644 drivers/usb/common/common.h

diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index 7d1b8c82b208..ecc2de1ffaae 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -5,6 +5,7 @@
 
 # Object files in subdirectories
 
+obj-$(CONFIG_USB_COMMON)	+= common/
 obj-$(CONFIG_USB)		+= core/
 obj-$(CONFIG_USB_SUPPORT)	+= phy/
 
@@ -60,8 +61,6 @@ obj-$(CONFIG_USB_CHIPIDEA)	+= chipidea/
 obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
 obj-$(CONFIG_USB_GADGET)	+= gadget/
 
-obj-$(CONFIG_USB_COMMON)	+= common/
-
 obj-$(CONFIG_USBIP_CORE)	+= usbip/
 
 obj-$(CONFIG_TYPEC)		+= typec/
diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index 18f5dcf58b0d..84a4423aaddf 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -15,6 +15,8 @@
 #include <linux/usb/of.h>
 #include <linux/usb/otg.h>
 #include <linux/of_platform.h>
+#include <linux/debugfs.h>
+#include "common.h"
 
 static const char *const ep_type_names[] = {
 	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
@@ -291,4 +293,23 @@ struct device *usb_of_get_companion_dev(struct device *dev)
 EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
 #endif
 
+struct dentry *usb_debug_root;
+EXPORT_SYMBOL_GPL(usb_debug_root);
+
+static int usb_common_init(void)
+{
+	usb_debug_root = debugfs_create_dir("usb", NULL);
+	ledtrig_usb_init();
+	return 0;
+}
+
+static void usb_common_exit(void)
+{
+	ledtrig_usb_exit();
+	debugfs_remove_recursive(usb_debug_root);
+}
+
+subsys_initcall(usb_common_init);
+module_exit(usb_common_exit);
+
 MODULE_LICENSE("GPL");
diff --git a/drivers/usb/common/common.h b/drivers/usb/common/common.h
new file mode 100644
index 000000000000..424a91316a4b
--- /dev/null
+++ b/drivers/usb/common/common.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __LINUX_USB_COMMON_H
+#define __LINUX_USB_COMMON_H
+
+#if defined(CONFIG_USB_LED_TRIG)
+void ledtrig_usb_init(void);
+void ledtrig_usb_exit(void);
+#else
+static inline void ledtrig_usb_init(void) { }
+static inline void ledtrig_usb_exit(void) { }
+#endif
+
+#endif	/* __LINUX_USB_COMMON_H */
diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
index 7bd81166b77d..0865dd44a80a 100644
--- a/drivers/usb/common/led.c
+++ b/drivers/usb/common/led.c
@@ -10,6 +10,7 @@
 #include <linux/init.h>
 #include <linux/leds.h>
 #include <linux/usb.h>
+#include "common.h"
 
 #define BLINK_DELAY 30
 
@@ -36,18 +37,14 @@ void usb_led_activity(enum usb_led_event ev)
 EXPORT_SYMBOL_GPL(usb_led_activity);
 
 
-static int __init ledtrig_usb_init(void)
+void __init ledtrig_usb_init(void)
 {
 	led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
 	led_trigger_register_simple("usb-host", &ledtrig_usb_host);
-	return 0;
 }
 
-static void __exit ledtrig_usb_exit(void)
+void __exit ledtrig_usb_exit(void)
 {
 	led_trigger_unregister_simple(ledtrig_usb_gadget);
 	led_trigger_unregister_simple(ledtrig_usb_host);
 }
-
-module_init(ledtrig_usb_init);
-module_exit(ledtrig_usb_exit);
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 7fcb9f782931..5a0df527a8ca 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
 	.notifier_call = usb_bus_notify,
 };
 
-struct dentry *usb_debug_root;
-EXPORT_SYMBOL_GPL(usb_debug_root);
+static struct dentry *usb_devices_root;
 
 static void usb_debugfs_init(void)
 {
-	usb_debug_root = debugfs_create_dir("usb", NULL);
-	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
-			    &usbfs_devices_fops);
+	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
+					       NULL, &usbfs_devices_fops);
 }
 
 static void usb_debugfs_cleanup(void)
 {
-	debugfs_remove_recursive(usb_debug_root);
+	debugfs_remove(usb_devices_root);
 }
 
 /*
-- 
2.21.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] USB: move usb debugfs directory creation to the usb common core
  2019-06-05  9:28 ` Greg Kroah-Hartman
  (?)
@ 2019-06-05 11:01   ` Chunfeng Yun
  -1 siblings, 0 replies; 16+ messages in thread
From: Chunfeng Yun @ 2019-06-05 11:01 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Felipe Balbi, Matthias Brugger, linux-usb, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek

Hi Greg,
On Wed, 2019-06-05 at 11:28 +0200, Greg Kroah-Hartman wrote:
> The USB gadget subsystem wants to use the USB debugfs root directory, so
> move it to the common "core" USB code so that it is properly initialized
> and removed as needed.
> 
> In order to properly do this, we need to load the common code before the
> usb core code, when everything is linked into the kernel, so reorder the
> link order of the code.
> 
> Also as the usb common code has the possibility of the led trigger logic
> to be merged into it, handle the build option properly by only having
> one module init/exit function and have the common code initialize the
> led trigger if needed.
> 
> Reported-by: From: Chunfeng Yun <chunfeng.yun@mediatek.com>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> 
> Chunfeng, can you test this version to verify it works for you when
> building the code into the kernel?
> 
> v2: handle led common code link error reported by kbuild
>     handle subsys_initcall issue pointed out by Chunfeng
> 
>  drivers/usb/Makefile        |  3 +--
>  drivers/usb/common/common.c | 21 +++++++++++++++++++++
>  drivers/usb/common/common.h | 14 ++++++++++++++
>  drivers/usb/common/led.c    |  9 +++------
>  drivers/usb/core/usb.c      | 10 ++++------
>  5 files changed, 43 insertions(+), 14 deletions(-)
>  create mode 100644 drivers/usb/common/common.h
> 
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index 7d1b8c82b208..ecc2de1ffaae 100644
> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -5,6 +5,7 @@
>  
>  # Object files in subdirectories
>  
> +obj-$(CONFIG_USB_COMMON)	+= common/
>  obj-$(CONFIG_USB)		+= core/
>  obj-$(CONFIG_USB_SUPPORT)	+= phy/
>  
> @@ -60,8 +61,6 @@ obj-$(CONFIG_USB_CHIPIDEA)	+= chipidea/
>  obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
>  obj-$(CONFIG_USB_GADGET)	+= gadget/
>  
> -obj-$(CONFIG_USB_COMMON)	+= common/
> -
>  obj-$(CONFIG_USBIP_CORE)	+= usbip/
>  
>  obj-$(CONFIG_TYPEC)		+= typec/
> diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> index 18f5dcf58b0d..84a4423aaddf 100644
> --- a/drivers/usb/common/common.c
> +++ b/drivers/usb/common/common.c
> @@ -15,6 +15,8 @@
>  #include <linux/usb/of.h>
>  #include <linux/usb/otg.h>
>  #include <linux/of_platform.h>
> +#include <linux/debugfs.h>
> +#include "common.h"
>  
>  static const char *const ep_type_names[] = {
>  	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> @@ -291,4 +293,23 @@ struct device *usb_of_get_companion_dev(struct device *dev)
>  EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
>  #endif
>  
> +struct dentry *usb_debug_root;
> +EXPORT_SYMBOL_GPL(usb_debug_root);
> +
> +static int usb_common_init(void)
> +{
> +	usb_debug_root = debugfs_create_dir("usb", NULL);
> +	ledtrig_usb_init();
> +	return 0;
> +}
> +
> +static void usb_common_exit(void)
> +{
> +	ledtrig_usb_exit();
> +	debugfs_remove_recursive(usb_debug_root);
> +}
> +
When enable CONFIG_LED_TRIGGER, there is a warning

 MODPOST vmlinux.o
WARNING: vmlinux.o(.text+0x68e300): Section mismatch in reference from
the function usb_common_init() to the
function .init.text:ledtrig_usb_init()
The function usb_common_init() references
the function __init ledtrig_usb_init().
This is often because usb_common_init lacks a __init
annotation or the annotation of ledtrig_usb_init is wrong.

WARNING: vmlinux.o(.text+0x68e318): Section mismatch in reference from
the function usb_common_exit() to the
function .exit.text:ledtrig_usb_exit()
The function usb_common_exit() references a function in an exit section.
Often the function ledtrig_usb_exit() has valid usage outside the exit
section
and the fix is to remove the __exit annotation of ledtrig_usb_exit.

seems need add __init and __exit for usb_common_init/exit

> +subsys_initcall(usb_common_init);
> +module_exit(usb_common_exit);
> +
>  MODULE_LICENSE("GPL");
> diff --git a/drivers/usb/common/common.h b/drivers/usb/common/common.h
> new file mode 100644
> index 000000000000..424a91316a4b
> --- /dev/null
> +++ b/drivers/usb/common/common.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __LINUX_USB_COMMON_H
> +#define __LINUX_USB_COMMON_H
> +
> +#if defined(CONFIG_USB_LED_TRIG)
> +void ledtrig_usb_init(void);
> +void ledtrig_usb_exit(void);
> +#else
> +static inline void ledtrig_usb_init(void) { }
> +static inline void ledtrig_usb_exit(void) { }
> +#endif
> +
> +#endif	/* __LINUX_USB_COMMON_H */
> diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
> index 7bd81166b77d..0865dd44a80a 100644
> --- a/drivers/usb/common/led.c
> +++ b/drivers/usb/common/led.c
> @@ -10,6 +10,7 @@
>  #include <linux/init.h>
>  #include <linux/leds.h>
>  #include <linux/usb.h>
> +#include "common.h"
>  
>  #define BLINK_DELAY 30
>  
> @@ -36,18 +37,14 @@ void usb_led_activity(enum usb_led_event ev)
>  EXPORT_SYMBOL_GPL(usb_led_activity);
>  
> 
> -static int __init ledtrig_usb_init(void)
> +void __init ledtrig_usb_init(void)
>  {
>  	led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
>  	led_trigger_register_simple("usb-host", &ledtrig_usb_host);
> -	return 0;
>  }
>  
> -static void __exit ledtrig_usb_exit(void)
> +void __exit ledtrig_usb_exit(void)
>  {
>  	led_trigger_unregister_simple(ledtrig_usb_gadget);
>  	led_trigger_unregister_simple(ledtrig_usb_host);
>  }
> -
> -module_init(ledtrig_usb_init);
> -module_exit(ledtrig_usb_exit);
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 7fcb9f782931..5a0df527a8ca 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
>  	.notifier_call = usb_bus_notify,
>  };
>  
> -struct dentry *usb_debug_root;
> -EXPORT_SYMBOL_GPL(usb_debug_root);
> +static struct dentry *usb_devices_root;
>  
>  static void usb_debugfs_init(void)
>  {
> -	usb_debug_root = debugfs_create_dir("usb", NULL);
> -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> -			    &usbfs_devices_fops);
> +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
> +					       NULL, &usbfs_devices_fops);
>  }
>  
>  static void usb_debugfs_cleanup(void)
>  {
> -	debugfs_remove_recursive(usb_debug_root);
> +	debugfs_remove(usb_devices_root);
>  }
>  
>  /*



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] USB: move usb debugfs directory creation to the usb common core
@ 2019-06-05 11:01   ` Chunfeng Yun
  0 siblings, 0 replies; 16+ messages in thread
From: Chunfeng Yun @ 2019-06-05 11:01 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: devicetree, Felipe Balbi, linux-usb, linux-kernel,
	linux-mediatek, Matthias Brugger, linux-arm-kernel

Hi Greg,
On Wed, 2019-06-05 at 11:28 +0200, Greg Kroah-Hartman wrote:
> The USB gadget subsystem wants to use the USB debugfs root directory, so
> move it to the common "core" USB code so that it is properly initialized
> and removed as needed.
> 
> In order to properly do this, we need to load the common code before the
> usb core code, when everything is linked into the kernel, so reorder the
> link order of the code.
> 
> Also as the usb common code has the possibility of the led trigger logic
> to be merged into it, handle the build option properly by only having
> one module init/exit function and have the common code initialize the
> led trigger if needed.
> 
> Reported-by: From: Chunfeng Yun <chunfeng.yun@mediatek.com>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> 
> Chunfeng, can you test this version to verify it works for you when
> building the code into the kernel?
> 
> v2: handle led common code link error reported by kbuild
>     handle subsys_initcall issue pointed out by Chunfeng
> 
>  drivers/usb/Makefile        |  3 +--
>  drivers/usb/common/common.c | 21 +++++++++++++++++++++
>  drivers/usb/common/common.h | 14 ++++++++++++++
>  drivers/usb/common/led.c    |  9 +++------
>  drivers/usb/core/usb.c      | 10 ++++------
>  5 files changed, 43 insertions(+), 14 deletions(-)
>  create mode 100644 drivers/usb/common/common.h
> 
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index 7d1b8c82b208..ecc2de1ffaae 100644
> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -5,6 +5,7 @@
>  
>  # Object files in subdirectories
>  
> +obj-$(CONFIG_USB_COMMON)	+= common/
>  obj-$(CONFIG_USB)		+= core/
>  obj-$(CONFIG_USB_SUPPORT)	+= phy/
>  
> @@ -60,8 +61,6 @@ obj-$(CONFIG_USB_CHIPIDEA)	+= chipidea/
>  obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
>  obj-$(CONFIG_USB_GADGET)	+= gadget/
>  
> -obj-$(CONFIG_USB_COMMON)	+= common/
> -
>  obj-$(CONFIG_USBIP_CORE)	+= usbip/
>  
>  obj-$(CONFIG_TYPEC)		+= typec/
> diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> index 18f5dcf58b0d..84a4423aaddf 100644
> --- a/drivers/usb/common/common.c
> +++ b/drivers/usb/common/common.c
> @@ -15,6 +15,8 @@
>  #include <linux/usb/of.h>
>  #include <linux/usb/otg.h>
>  #include <linux/of_platform.h>
> +#include <linux/debugfs.h>
> +#include "common.h"
>  
>  static const char *const ep_type_names[] = {
>  	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> @@ -291,4 +293,23 @@ struct device *usb_of_get_companion_dev(struct device *dev)
>  EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
>  #endif
>  
> +struct dentry *usb_debug_root;
> +EXPORT_SYMBOL_GPL(usb_debug_root);
> +
> +static int usb_common_init(void)
> +{
> +	usb_debug_root = debugfs_create_dir("usb", NULL);
> +	ledtrig_usb_init();
> +	return 0;
> +}
> +
> +static void usb_common_exit(void)
> +{
> +	ledtrig_usb_exit();
> +	debugfs_remove_recursive(usb_debug_root);
> +}
> +
When enable CONFIG_LED_TRIGGER, there is a warning

 MODPOST vmlinux.o
WARNING: vmlinux.o(.text+0x68e300): Section mismatch in reference from
the function usb_common_init() to the
function .init.text:ledtrig_usb_init()
The function usb_common_init() references
the function __init ledtrig_usb_init().
This is often because usb_common_init lacks a __init
annotation or the annotation of ledtrig_usb_init is wrong.

WARNING: vmlinux.o(.text+0x68e318): Section mismatch in reference from
the function usb_common_exit() to the
function .exit.text:ledtrig_usb_exit()
The function usb_common_exit() references a function in an exit section.
Often the function ledtrig_usb_exit() has valid usage outside the exit
section
and the fix is to remove the __exit annotation of ledtrig_usb_exit.

seems need add __init and __exit for usb_common_init/exit

> +subsys_initcall(usb_common_init);
> +module_exit(usb_common_exit);
> +
>  MODULE_LICENSE("GPL");
> diff --git a/drivers/usb/common/common.h b/drivers/usb/common/common.h
> new file mode 100644
> index 000000000000..424a91316a4b
> --- /dev/null
> +++ b/drivers/usb/common/common.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __LINUX_USB_COMMON_H
> +#define __LINUX_USB_COMMON_H
> +
> +#if defined(CONFIG_USB_LED_TRIG)
> +void ledtrig_usb_init(void);
> +void ledtrig_usb_exit(void);
> +#else
> +static inline void ledtrig_usb_init(void) { }
> +static inline void ledtrig_usb_exit(void) { }
> +#endif
> +
> +#endif	/* __LINUX_USB_COMMON_H */
> diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
> index 7bd81166b77d..0865dd44a80a 100644
> --- a/drivers/usb/common/led.c
> +++ b/drivers/usb/common/led.c
> @@ -10,6 +10,7 @@
>  #include <linux/init.h>
>  #include <linux/leds.h>
>  #include <linux/usb.h>
> +#include "common.h"
>  
>  #define BLINK_DELAY 30
>  
> @@ -36,18 +37,14 @@ void usb_led_activity(enum usb_led_event ev)
>  EXPORT_SYMBOL_GPL(usb_led_activity);
>  
> 
> -static int __init ledtrig_usb_init(void)
> +void __init ledtrig_usb_init(void)
>  {
>  	led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
>  	led_trigger_register_simple("usb-host", &ledtrig_usb_host);
> -	return 0;
>  }
>  
> -static void __exit ledtrig_usb_exit(void)
> +void __exit ledtrig_usb_exit(void)
>  {
>  	led_trigger_unregister_simple(ledtrig_usb_gadget);
>  	led_trigger_unregister_simple(ledtrig_usb_host);
>  }
> -
> -module_init(ledtrig_usb_init);
> -module_exit(ledtrig_usb_exit);
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 7fcb9f782931..5a0df527a8ca 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
>  	.notifier_call = usb_bus_notify,
>  };
>  
> -struct dentry *usb_debug_root;
> -EXPORT_SYMBOL_GPL(usb_debug_root);
> +static struct dentry *usb_devices_root;
>  
>  static void usb_debugfs_init(void)
>  {
> -	usb_debug_root = debugfs_create_dir("usb", NULL);
> -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> -			    &usbfs_devices_fops);
> +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
> +					       NULL, &usbfs_devices_fops);
>  }
>  
>  static void usb_debugfs_cleanup(void)
>  {
> -	debugfs_remove_recursive(usb_debug_root);
> +	debugfs_remove(usb_devices_root);
>  }
>  
>  /*

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] USB: move usb debugfs directory creation to the usb common core
@ 2019-06-05 11:01   ` Chunfeng Yun
  0 siblings, 0 replies; 16+ messages in thread
From: Chunfeng Yun @ 2019-06-05 11:01 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: devicetree, Felipe Balbi, linux-usb, linux-kernel,
	linux-mediatek, Matthias Brugger, linux-arm-kernel

Hi Greg,
On Wed, 2019-06-05 at 11:28 +0200, Greg Kroah-Hartman wrote:
> The USB gadget subsystem wants to use the USB debugfs root directory, so
> move it to the common "core" USB code so that it is properly initialized
> and removed as needed.
> 
> In order to properly do this, we need to load the common code before the
> usb core code, when everything is linked into the kernel, so reorder the
> link order of the code.
> 
> Also as the usb common code has the possibility of the led trigger logic
> to be merged into it, handle the build option properly by only having
> one module init/exit function and have the common code initialize the
> led trigger if needed.
> 
> Reported-by: From: Chunfeng Yun <chunfeng.yun@mediatek.com>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> 
> Chunfeng, can you test this version to verify it works for you when
> building the code into the kernel?
> 
> v2: handle led common code link error reported by kbuild
>     handle subsys_initcall issue pointed out by Chunfeng
> 
>  drivers/usb/Makefile        |  3 +--
>  drivers/usb/common/common.c | 21 +++++++++++++++++++++
>  drivers/usb/common/common.h | 14 ++++++++++++++
>  drivers/usb/common/led.c    |  9 +++------
>  drivers/usb/core/usb.c      | 10 ++++------
>  5 files changed, 43 insertions(+), 14 deletions(-)
>  create mode 100644 drivers/usb/common/common.h
> 
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index 7d1b8c82b208..ecc2de1ffaae 100644
> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -5,6 +5,7 @@
>  
>  # Object files in subdirectories
>  
> +obj-$(CONFIG_USB_COMMON)	+= common/
>  obj-$(CONFIG_USB)		+= core/
>  obj-$(CONFIG_USB_SUPPORT)	+= phy/
>  
> @@ -60,8 +61,6 @@ obj-$(CONFIG_USB_CHIPIDEA)	+= chipidea/
>  obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
>  obj-$(CONFIG_USB_GADGET)	+= gadget/
>  
> -obj-$(CONFIG_USB_COMMON)	+= common/
> -
>  obj-$(CONFIG_USBIP_CORE)	+= usbip/
>  
>  obj-$(CONFIG_TYPEC)		+= typec/
> diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> index 18f5dcf58b0d..84a4423aaddf 100644
> --- a/drivers/usb/common/common.c
> +++ b/drivers/usb/common/common.c
> @@ -15,6 +15,8 @@
>  #include <linux/usb/of.h>
>  #include <linux/usb/otg.h>
>  #include <linux/of_platform.h>
> +#include <linux/debugfs.h>
> +#include "common.h"
>  
>  static const char *const ep_type_names[] = {
>  	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> @@ -291,4 +293,23 @@ struct device *usb_of_get_companion_dev(struct device *dev)
>  EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
>  #endif
>  
> +struct dentry *usb_debug_root;
> +EXPORT_SYMBOL_GPL(usb_debug_root);
> +
> +static int usb_common_init(void)
> +{
> +	usb_debug_root = debugfs_create_dir("usb", NULL);
> +	ledtrig_usb_init();
> +	return 0;
> +}
> +
> +static void usb_common_exit(void)
> +{
> +	ledtrig_usb_exit();
> +	debugfs_remove_recursive(usb_debug_root);
> +}
> +
When enable CONFIG_LED_TRIGGER, there is a warning

 MODPOST vmlinux.o
WARNING: vmlinux.o(.text+0x68e300): Section mismatch in reference from
the function usb_common_init() to the
function .init.text:ledtrig_usb_init()
The function usb_common_init() references
the function __init ledtrig_usb_init().
This is often because usb_common_init lacks a __init
annotation or the annotation of ledtrig_usb_init is wrong.

WARNING: vmlinux.o(.text+0x68e318): Section mismatch in reference from
the function usb_common_exit() to the
function .exit.text:ledtrig_usb_exit()
The function usb_common_exit() references a function in an exit section.
Often the function ledtrig_usb_exit() has valid usage outside the exit
section
and the fix is to remove the __exit annotation of ledtrig_usb_exit.

seems need add __init and __exit for usb_common_init/exit

> +subsys_initcall(usb_common_init);
> +module_exit(usb_common_exit);
> +
>  MODULE_LICENSE("GPL");
> diff --git a/drivers/usb/common/common.h b/drivers/usb/common/common.h
> new file mode 100644
> index 000000000000..424a91316a4b
> --- /dev/null
> +++ b/drivers/usb/common/common.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __LINUX_USB_COMMON_H
> +#define __LINUX_USB_COMMON_H
> +
> +#if defined(CONFIG_USB_LED_TRIG)
> +void ledtrig_usb_init(void);
> +void ledtrig_usb_exit(void);
> +#else
> +static inline void ledtrig_usb_init(void) { }
> +static inline void ledtrig_usb_exit(void) { }
> +#endif
> +
> +#endif	/* __LINUX_USB_COMMON_H */
> diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
> index 7bd81166b77d..0865dd44a80a 100644
> --- a/drivers/usb/common/led.c
> +++ b/drivers/usb/common/led.c
> @@ -10,6 +10,7 @@
>  #include <linux/init.h>
>  #include <linux/leds.h>
>  #include <linux/usb.h>
> +#include "common.h"
>  
>  #define BLINK_DELAY 30
>  
> @@ -36,18 +37,14 @@ void usb_led_activity(enum usb_led_event ev)
>  EXPORT_SYMBOL_GPL(usb_led_activity);
>  
> 
> -static int __init ledtrig_usb_init(void)
> +void __init ledtrig_usb_init(void)
>  {
>  	led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
>  	led_trigger_register_simple("usb-host", &ledtrig_usb_host);
> -	return 0;
>  }
>  
> -static void __exit ledtrig_usb_exit(void)
> +void __exit ledtrig_usb_exit(void)
>  {
>  	led_trigger_unregister_simple(ledtrig_usb_gadget);
>  	led_trigger_unregister_simple(ledtrig_usb_host);
>  }
> -
> -module_init(ledtrig_usb_init);
> -module_exit(ledtrig_usb_exit);
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 7fcb9f782931..5a0df527a8ca 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
>  	.notifier_call = usb_bus_notify,
>  };
>  
> -struct dentry *usb_debug_root;
> -EXPORT_SYMBOL_GPL(usb_debug_root);
> +static struct dentry *usb_devices_root;
>  
>  static void usb_debugfs_init(void)
>  {
> -	usb_debug_root = debugfs_create_dir("usb", NULL);
> -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> -			    &usbfs_devices_fops);
> +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
> +					       NULL, &usbfs_devices_fops);
>  }
>  
>  static void usb_debugfs_cleanup(void)
>  {
> -	debugfs_remove_recursive(usb_debug_root);
> +	debugfs_remove(usb_devices_root);
>  }
>  
>  /*



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] USB: move usb debugfs directory creation to the usb common core
  2019-06-05  9:28 ` Greg Kroah-Hartman
  (?)
  (?)
@ 2019-06-05 11:44 ` Marc Gonzalez
  2019-06-05 12:39   ` Greg Kroah-Hartman
  -1 siblings, 1 reply; 16+ messages in thread
From: Marc Gonzalez @ 2019-06-05 11:44 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-usb, Felipe Balbi, Chunfeng Yun

On 05/06/2019 11:28, Greg Kroah-Hartman wrote:

> The USB gadget subsystem wants to use the USB debugfs root directory, so
> move it to the common "core" USB code so that it is properly initialized
> and removed as needed.
> 
> In order to properly do this, we need to load the common code before the
> usb core code, when everything is linked into the kernel, so reorder the
> link order of the code.
> 
> Also as the usb common code has the possibility of the led trigger logic
> to be merged into it, handle the build option properly by only having
> one module init/exit function and have the common code initialize the
> led trigger if needed.
> 
> Reported-by: From: Chunfeng Yun <chunfeng.yun@mediatek.com>

You have a spurious "From: " in there ;-)

Regards.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] USB: move usb debugfs directory creation to the usb common core
  2019-06-05 11:44 ` Marc Gonzalez
@ 2019-06-05 12:39   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 16+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-05 12:39 UTC (permalink / raw)
  To: Marc Gonzalez; +Cc: linux-usb, Felipe Balbi, Chunfeng Yun

On Wed, Jun 05, 2019 at 01:44:26PM +0200, Marc Gonzalez wrote:
> On 05/06/2019 11:28, Greg Kroah-Hartman wrote:
> 
> > The USB gadget subsystem wants to use the USB debugfs root directory, so
> > move it to the common "core" USB code so that it is properly initialized
> > and removed as needed.
> > 
> > In order to properly do this, we need to load the common code before the
> > usb core code, when everything is linked into the kernel, so reorder the
> > link order of the code.
> > 
> > Also as the usb common code has the possibility of the led trigger logic
> > to be merged into it, handle the build option properly by only having
> > one module init/exit function and have the common code initialize the
> > led trigger if needed.
> > 
> > Reported-by: From: Chunfeng Yun <chunfeng.yun@mediatek.com>
> 
> You have a spurious "From: " in there ;-)

Ugh, cut/paste error, thanks, will fix!

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] USB: move usb debugfs directory creation to the usb common core
  2019-06-05 11:01   ` Chunfeng Yun
@ 2019-06-05 12:40     ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 16+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-05 12:40 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Felipe Balbi, Matthias Brugger, linux-usb, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek

On Wed, Jun 05, 2019 at 07:01:55PM +0800, Chunfeng Yun wrote:
> Hi Greg,
> On Wed, 2019-06-05 at 11:28 +0200, Greg Kroah-Hartman wrote:
> > The USB gadget subsystem wants to use the USB debugfs root directory, so
> > move it to the common "core" USB code so that it is properly initialized
> > and removed as needed.
> > 
> > In order to properly do this, we need to load the common code before the
> > usb core code, when everything is linked into the kernel, so reorder the
> > link order of the code.
> > 
> > Also as the usb common code has the possibility of the led trigger logic
> > to be merged into it, handle the build option properly by only having
> > one module init/exit function and have the common code initialize the
> > led trigger if needed.
> > 
> > Reported-by: From: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > 
> > Chunfeng, can you test this version to verify it works for you when
> > building the code into the kernel?
> > 
> > v2: handle led common code link error reported by kbuild
> >     handle subsys_initcall issue pointed out by Chunfeng
> > 
> >  drivers/usb/Makefile        |  3 +--
> >  drivers/usb/common/common.c | 21 +++++++++++++++++++++
> >  drivers/usb/common/common.h | 14 ++++++++++++++
> >  drivers/usb/common/led.c    |  9 +++------
> >  drivers/usb/core/usb.c      | 10 ++++------
> >  5 files changed, 43 insertions(+), 14 deletions(-)
> >  create mode 100644 drivers/usb/common/common.h
> > 
> > diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> > index 7d1b8c82b208..ecc2de1ffaae 100644
> > --- a/drivers/usb/Makefile
> > +++ b/drivers/usb/Makefile
> > @@ -5,6 +5,7 @@
> >  
> >  # Object files in subdirectories
> >  
> > +obj-$(CONFIG_USB_COMMON)	+= common/
> >  obj-$(CONFIG_USB)		+= core/
> >  obj-$(CONFIG_USB_SUPPORT)	+= phy/
> >  
> > @@ -60,8 +61,6 @@ obj-$(CONFIG_USB_CHIPIDEA)	+= chipidea/
> >  obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
> >  obj-$(CONFIG_USB_GADGET)	+= gadget/
> >  
> > -obj-$(CONFIG_USB_COMMON)	+= common/
> > -
> >  obj-$(CONFIG_USBIP_CORE)	+= usbip/
> >  
> >  obj-$(CONFIG_TYPEC)		+= typec/
> > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > index 18f5dcf58b0d..84a4423aaddf 100644
> > --- a/drivers/usb/common/common.c
> > +++ b/drivers/usb/common/common.c
> > @@ -15,6 +15,8 @@
> >  #include <linux/usb/of.h>
> >  #include <linux/usb/otg.h>
> >  #include <linux/of_platform.h>
> > +#include <linux/debugfs.h>
> > +#include "common.h"
> >  
> >  static const char *const ep_type_names[] = {
> >  	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> > @@ -291,4 +293,23 @@ struct device *usb_of_get_companion_dev(struct device *dev)
> >  EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
> >  #endif
> >  
> > +struct dentry *usb_debug_root;
> > +EXPORT_SYMBOL_GPL(usb_debug_root);
> > +
> > +static int usb_common_init(void)
> > +{
> > +	usb_debug_root = debugfs_create_dir("usb", NULL);
> > +	ledtrig_usb_init();
> > +	return 0;
> > +}
> > +
> > +static void usb_common_exit(void)
> > +{
> > +	ledtrig_usb_exit();
> > +	debugfs_remove_recursive(usb_debug_root);
> > +}
> > +
> When enable CONFIG_LED_TRIGGER, there is a warning
> 
>  MODPOST vmlinux.o
> WARNING: vmlinux.o(.text+0x68e300): Section mismatch in reference from
> the function usb_common_init() to the
> function .init.text:ledtrig_usb_init()
> The function usb_common_init() references
> the function __init ledtrig_usb_init().
> This is often because usb_common_init lacks a __init
> annotation or the annotation of ledtrig_usb_init is wrong.
> 
> WARNING: vmlinux.o(.text+0x68e318): Section mismatch in reference from
> the function usb_common_exit() to the
> function .exit.text:ledtrig_usb_exit()
> The function usb_common_exit() references a function in an exit section.
> Often the function ledtrig_usb_exit() has valid usage outside the exit
> section
> and the fix is to remove the __exit annotation of ledtrig_usb_exit.
> 
> seems need add __init and __exit for usb_common_init/exit

Yes, you are right, I'll go add those markings to those functions, good
catch.

greg k-h

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] USB: move usb debugfs directory creation to the usb common core
@ 2019-06-05 12:40     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 16+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-05 12:40 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: devicetree, Felipe Balbi, linux-usb, linux-kernel,
	linux-mediatek, Matthias Brugger, linux-arm-kernel

On Wed, Jun 05, 2019 at 07:01:55PM +0800, Chunfeng Yun wrote:
> Hi Greg,
> On Wed, 2019-06-05 at 11:28 +0200, Greg Kroah-Hartman wrote:
> > The USB gadget subsystem wants to use the USB debugfs root directory, so
> > move it to the common "core" USB code so that it is properly initialized
> > and removed as needed.
> > 
> > In order to properly do this, we need to load the common code before the
> > usb core code, when everything is linked into the kernel, so reorder the
> > link order of the code.
> > 
> > Also as the usb common code has the possibility of the led trigger logic
> > to be merged into it, handle the build option properly by only having
> > one module init/exit function and have the common code initialize the
> > led trigger if needed.
> > 
> > Reported-by: From: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > 
> > Chunfeng, can you test this version to verify it works for you when
> > building the code into the kernel?
> > 
> > v2: handle led common code link error reported by kbuild
> >     handle subsys_initcall issue pointed out by Chunfeng
> > 
> >  drivers/usb/Makefile        |  3 +--
> >  drivers/usb/common/common.c | 21 +++++++++++++++++++++
> >  drivers/usb/common/common.h | 14 ++++++++++++++
> >  drivers/usb/common/led.c    |  9 +++------
> >  drivers/usb/core/usb.c      | 10 ++++------
> >  5 files changed, 43 insertions(+), 14 deletions(-)
> >  create mode 100644 drivers/usb/common/common.h
> > 
> > diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> > index 7d1b8c82b208..ecc2de1ffaae 100644
> > --- a/drivers/usb/Makefile
> > +++ b/drivers/usb/Makefile
> > @@ -5,6 +5,7 @@
> >  
> >  # Object files in subdirectories
> >  
> > +obj-$(CONFIG_USB_COMMON)	+= common/
> >  obj-$(CONFIG_USB)		+= core/
> >  obj-$(CONFIG_USB_SUPPORT)	+= phy/
> >  
> > @@ -60,8 +61,6 @@ obj-$(CONFIG_USB_CHIPIDEA)	+= chipidea/
> >  obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
> >  obj-$(CONFIG_USB_GADGET)	+= gadget/
> >  
> > -obj-$(CONFIG_USB_COMMON)	+= common/
> > -
> >  obj-$(CONFIG_USBIP_CORE)	+= usbip/
> >  
> >  obj-$(CONFIG_TYPEC)		+= typec/
> > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > index 18f5dcf58b0d..84a4423aaddf 100644
> > --- a/drivers/usb/common/common.c
> > +++ b/drivers/usb/common/common.c
> > @@ -15,6 +15,8 @@
> >  #include <linux/usb/of.h>
> >  #include <linux/usb/otg.h>
> >  #include <linux/of_platform.h>
> > +#include <linux/debugfs.h>
> > +#include "common.h"
> >  
> >  static const char *const ep_type_names[] = {
> >  	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> > @@ -291,4 +293,23 @@ struct device *usb_of_get_companion_dev(struct device *dev)
> >  EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
> >  #endif
> >  
> > +struct dentry *usb_debug_root;
> > +EXPORT_SYMBOL_GPL(usb_debug_root);
> > +
> > +static int usb_common_init(void)
> > +{
> > +	usb_debug_root = debugfs_create_dir("usb", NULL);
> > +	ledtrig_usb_init();
> > +	return 0;
> > +}
> > +
> > +static void usb_common_exit(void)
> > +{
> > +	ledtrig_usb_exit();
> > +	debugfs_remove_recursive(usb_debug_root);
> > +}
> > +
> When enable CONFIG_LED_TRIGGER, there is a warning
> 
>  MODPOST vmlinux.o
> WARNING: vmlinux.o(.text+0x68e300): Section mismatch in reference from
> the function usb_common_init() to the
> function .init.text:ledtrig_usb_init()
> The function usb_common_init() references
> the function __init ledtrig_usb_init().
> This is often because usb_common_init lacks a __init
> annotation or the annotation of ledtrig_usb_init is wrong.
> 
> WARNING: vmlinux.o(.text+0x68e318): Section mismatch in reference from
> the function usb_common_exit() to the
> function .exit.text:ledtrig_usb_exit()
> The function usb_common_exit() references a function in an exit section.
> Often the function ledtrig_usb_exit() has valid usage outside the exit
> section
> and the fix is to remove the __exit annotation of ledtrig_usb_exit.
> 
> seems need add __init and __exit for usb_common_init/exit

Yes, you are right, I'll go add those markings to those functions, good
catch.

greg k-h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3] USB: move usb debugfs directory creation to the usb common core
  2019-06-05  9:28 ` Greg Kroah-Hartman
@ 2019-06-05 12:44   ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 16+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-05 12:44 UTC (permalink / raw)
  To: Felipe Balbi, Chunfeng Yun
  Cc: Matthias Brugger, linux-usb, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek

The USB gadget subsystem wants to use the USB debugfs root directory, so
move it to the common "core" USB code so that it is properly initialized
and removed as needed.

In order to properly do this, we need to load the common code before the
usb core code, when everything is linked into the kernel, so reorder the
link order of the code.

Also as the usb common code has the possibility of the led trigger logic
to be merged into it, handle the build option properly by only having
one module init/exit function and have the common code initialize the
led trigger if needed.

Reported-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Chunfeng, can you try testing this again?

v3: Fix __init and __exit error when building into the tree as reported
    by Chunfeng
    Fix Reported-by: line as reported
v2: handle led common code link error reported by kbuild
    handle subsys_initcall issue pointed out by Chunfeng

 drivers/usb/Makefile        |  3 +--
 drivers/usb/common/common.c | 21 +++++++++++++++++++++
 drivers/usb/common/common.h | 14 ++++++++++++++
 drivers/usb/common/led.c    |  9 +++------
 drivers/usb/core/usb.c      | 10 ++++------
 5 files changed, 43 insertions(+), 14 deletions(-)
 create mode 100644 drivers/usb/common/common.h

diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index 7d1b8c82b208..ecc2de1ffaae 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -5,6 +5,7 @@
 
 # Object files in subdirectories
 
+obj-$(CONFIG_USB_COMMON)	+= common/
 obj-$(CONFIG_USB)		+= core/
 obj-$(CONFIG_USB_SUPPORT)	+= phy/
 
@@ -60,8 +61,6 @@ obj-$(CONFIG_USB_CHIPIDEA)	+= chipidea/
 obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
 obj-$(CONFIG_USB_GADGET)	+= gadget/
 
-obj-$(CONFIG_USB_COMMON)	+= common/
-
 obj-$(CONFIG_USBIP_CORE)	+= usbip/
 
 obj-$(CONFIG_TYPEC)		+= typec/
diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index 18f5dcf58b0d..1433260d99b4 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -15,6 +15,8 @@
 #include <linux/usb/of.h>
 #include <linux/usb/otg.h>
 #include <linux/of_platform.h>
+#include <linux/debugfs.h>
+#include "common.h"
 
 static const char *const ep_type_names[] = {
 	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
@@ -291,4 +293,23 @@ struct device *usb_of_get_companion_dev(struct device *dev)
 EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
 #endif
 
+struct dentry *usb_debug_root;
+EXPORT_SYMBOL_GPL(usb_debug_root);
+
+static int __init usb_common_init(void)
+{
+	usb_debug_root = debugfs_create_dir("usb", NULL);
+	ledtrig_usb_init();
+	return 0;
+}
+
+static void __exit usb_common_exit(void)
+{
+	ledtrig_usb_exit();
+	debugfs_remove_recursive(usb_debug_root);
+}
+
+subsys_initcall(usb_common_init);
+module_exit(usb_common_exit);
+
 MODULE_LICENSE("GPL");
diff --git a/drivers/usb/common/common.h b/drivers/usb/common/common.h
new file mode 100644
index 000000000000..424a91316a4b
--- /dev/null
+++ b/drivers/usb/common/common.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __LINUX_USB_COMMON_H
+#define __LINUX_USB_COMMON_H
+
+#if defined(CONFIG_USB_LED_TRIG)
+void ledtrig_usb_init(void);
+void ledtrig_usb_exit(void);
+#else
+static inline void ledtrig_usb_init(void) { }
+static inline void ledtrig_usb_exit(void) { }
+#endif
+
+#endif	/* __LINUX_USB_COMMON_H */
diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
index 7bd81166b77d..0865dd44a80a 100644
--- a/drivers/usb/common/led.c
+++ b/drivers/usb/common/led.c
@@ -10,6 +10,7 @@
 #include <linux/init.h>
 #include <linux/leds.h>
 #include <linux/usb.h>
+#include "common.h"
 
 #define BLINK_DELAY 30
 
@@ -36,18 +37,14 @@ void usb_led_activity(enum usb_led_event ev)
 EXPORT_SYMBOL_GPL(usb_led_activity);
 
 
-static int __init ledtrig_usb_init(void)
+void __init ledtrig_usb_init(void)
 {
 	led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
 	led_trigger_register_simple("usb-host", &ledtrig_usb_host);
-	return 0;
 }
 
-static void __exit ledtrig_usb_exit(void)
+void __exit ledtrig_usb_exit(void)
 {
 	led_trigger_unregister_simple(ledtrig_usb_gadget);
 	led_trigger_unregister_simple(ledtrig_usb_host);
 }
-
-module_init(ledtrig_usb_init);
-module_exit(ledtrig_usb_exit);
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 7fcb9f782931..5a0df527a8ca 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
 	.notifier_call = usb_bus_notify,
 };
 
-struct dentry *usb_debug_root;
-EXPORT_SYMBOL_GPL(usb_debug_root);
+static struct dentry *usb_devices_root;
 
 static void usb_debugfs_init(void)
 {
-	usb_debug_root = debugfs_create_dir("usb", NULL);
-	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
-			    &usbfs_devices_fops);
+	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
+					       NULL, &usbfs_devices_fops);
 }
 
 static void usb_debugfs_cleanup(void)
 {
-	debugfs_remove_recursive(usb_debug_root);
+	debugfs_remove(usb_devices_root);
 }
 
 /*
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3] USB: move usb debugfs directory creation to the usb common core
@ 2019-06-05 12:44   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 16+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-05 12:44 UTC (permalink / raw)
  To: Felipe Balbi, Chunfeng Yun
  Cc: devicetree, linux-usb, linux-kernel, linux-mediatek,
	Matthias Brugger, linux-arm-kernel

The USB gadget subsystem wants to use the USB debugfs root directory, so
move it to the common "core" USB code so that it is properly initialized
and removed as needed.

In order to properly do this, we need to load the common code before the
usb core code, when everything is linked into the kernel, so reorder the
link order of the code.

Also as the usb common code has the possibility of the led trigger logic
to be merged into it, handle the build option properly by only having
one module init/exit function and have the common code initialize the
led trigger if needed.

Reported-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Chunfeng, can you try testing this again?

v3: Fix __init and __exit error when building into the tree as reported
    by Chunfeng
    Fix Reported-by: line as reported
v2: handle led common code link error reported by kbuild
    handle subsys_initcall issue pointed out by Chunfeng

 drivers/usb/Makefile        |  3 +--
 drivers/usb/common/common.c | 21 +++++++++++++++++++++
 drivers/usb/common/common.h | 14 ++++++++++++++
 drivers/usb/common/led.c    |  9 +++------
 drivers/usb/core/usb.c      | 10 ++++------
 5 files changed, 43 insertions(+), 14 deletions(-)
 create mode 100644 drivers/usb/common/common.h

diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index 7d1b8c82b208..ecc2de1ffaae 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -5,6 +5,7 @@
 
 # Object files in subdirectories
 
+obj-$(CONFIG_USB_COMMON)	+= common/
 obj-$(CONFIG_USB)		+= core/
 obj-$(CONFIG_USB_SUPPORT)	+= phy/
 
@@ -60,8 +61,6 @@ obj-$(CONFIG_USB_CHIPIDEA)	+= chipidea/
 obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
 obj-$(CONFIG_USB_GADGET)	+= gadget/
 
-obj-$(CONFIG_USB_COMMON)	+= common/
-
 obj-$(CONFIG_USBIP_CORE)	+= usbip/
 
 obj-$(CONFIG_TYPEC)		+= typec/
diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index 18f5dcf58b0d..1433260d99b4 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -15,6 +15,8 @@
 #include <linux/usb/of.h>
 #include <linux/usb/otg.h>
 #include <linux/of_platform.h>
+#include <linux/debugfs.h>
+#include "common.h"
 
 static const char *const ep_type_names[] = {
 	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
@@ -291,4 +293,23 @@ struct device *usb_of_get_companion_dev(struct device *dev)
 EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
 #endif
 
+struct dentry *usb_debug_root;
+EXPORT_SYMBOL_GPL(usb_debug_root);
+
+static int __init usb_common_init(void)
+{
+	usb_debug_root = debugfs_create_dir("usb", NULL);
+	ledtrig_usb_init();
+	return 0;
+}
+
+static void __exit usb_common_exit(void)
+{
+	ledtrig_usb_exit();
+	debugfs_remove_recursive(usb_debug_root);
+}
+
+subsys_initcall(usb_common_init);
+module_exit(usb_common_exit);
+
 MODULE_LICENSE("GPL");
diff --git a/drivers/usb/common/common.h b/drivers/usb/common/common.h
new file mode 100644
index 000000000000..424a91316a4b
--- /dev/null
+++ b/drivers/usb/common/common.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __LINUX_USB_COMMON_H
+#define __LINUX_USB_COMMON_H
+
+#if defined(CONFIG_USB_LED_TRIG)
+void ledtrig_usb_init(void);
+void ledtrig_usb_exit(void);
+#else
+static inline void ledtrig_usb_init(void) { }
+static inline void ledtrig_usb_exit(void) { }
+#endif
+
+#endif	/* __LINUX_USB_COMMON_H */
diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
index 7bd81166b77d..0865dd44a80a 100644
--- a/drivers/usb/common/led.c
+++ b/drivers/usb/common/led.c
@@ -10,6 +10,7 @@
 #include <linux/init.h>
 #include <linux/leds.h>
 #include <linux/usb.h>
+#include "common.h"
 
 #define BLINK_DELAY 30
 
@@ -36,18 +37,14 @@ void usb_led_activity(enum usb_led_event ev)
 EXPORT_SYMBOL_GPL(usb_led_activity);
 
 
-static int __init ledtrig_usb_init(void)
+void __init ledtrig_usb_init(void)
 {
 	led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
 	led_trigger_register_simple("usb-host", &ledtrig_usb_host);
-	return 0;
 }
 
-static void __exit ledtrig_usb_exit(void)
+void __exit ledtrig_usb_exit(void)
 {
 	led_trigger_unregister_simple(ledtrig_usb_gadget);
 	led_trigger_unregister_simple(ledtrig_usb_host);
 }
-
-module_init(ledtrig_usb_init);
-module_exit(ledtrig_usb_exit);
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 7fcb9f782931..5a0df527a8ca 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
 	.notifier_call = usb_bus_notify,
 };
 
-struct dentry *usb_debug_root;
-EXPORT_SYMBOL_GPL(usb_debug_root);
+static struct dentry *usb_devices_root;
 
 static void usb_debugfs_init(void)
 {
-	usb_debug_root = debugfs_create_dir("usb", NULL);
-	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
-			    &usbfs_devices_fops);
+	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
+					       NULL, &usbfs_devices_fops);
 }
 
 static void usb_debugfs_cleanup(void)
 {
-	debugfs_remove_recursive(usb_debug_root);
+	debugfs_remove(usb_devices_root);
 }
 
 /*
-- 
2.21.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] USB: move usb debugfs directory creation to the usb common core
  2019-06-05 12:44   ` Greg Kroah-Hartman
  (?)
@ 2019-06-06  2:32     ` Chunfeng Yun
  -1 siblings, 0 replies; 16+ messages in thread
From: Chunfeng Yun @ 2019-06-06  2:32 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Felipe Balbi, Matthias Brugger, linux-usb, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek

On Wed, 2019-06-05 at 14:44 +0200, Greg Kroah-Hartman wrote:
> The USB gadget subsystem wants to use the USB debugfs root directory, so
> move it to the common "core" USB code so that it is properly initialized
> and removed as needed.
> 
> In order to properly do this, we need to load the common code before the
> usb core code, when everything is linked into the kernel, so reorder the
> link order of the code.
> 
> Also as the usb common code has the possibility of the led trigger logic
> to be merged into it, handle the build option properly by only having
> one module init/exit function and have the common code initialize the
> led trigger if needed.
> 
> Reported-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> Chunfeng, can you try testing this again?

Tested-by: Chunfeng Yun <chunfeng.yun@mediatek.com>

Thank you, Greg, Felipe

> 
> v3: Fix __init and __exit error when building into the tree as reported
>     by Chunfeng
>     Fix Reported-by: line as reported
> v2: handle led common code link error reported by kbuild
>     handle subsys_initcall issue pointed out by Chunfeng
> 
>  drivers/usb/Makefile        |  3 +--
>  drivers/usb/common/common.c | 21 +++++++++++++++++++++
>  drivers/usb/common/common.h | 14 ++++++++++++++
>  drivers/usb/common/led.c    |  9 +++------
>  drivers/usb/core/usb.c      | 10 ++++------
>  5 files changed, 43 insertions(+), 14 deletions(-)
>  create mode 100644 drivers/usb/common/common.h
> 
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index 7d1b8c82b208..ecc2de1ffaae 100644
> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -5,6 +5,7 @@
>  
>  # Object files in subdirectories
>  
> +obj-$(CONFIG_USB_COMMON)	+= common/
>  obj-$(CONFIG_USB)		+= core/
>  obj-$(CONFIG_USB_SUPPORT)	+= phy/
>  
> @@ -60,8 +61,6 @@ obj-$(CONFIG_USB_CHIPIDEA)	+= chipidea/
>  obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
>  obj-$(CONFIG_USB_GADGET)	+= gadget/
>  
> -obj-$(CONFIG_USB_COMMON)	+= common/
> -
>  obj-$(CONFIG_USBIP_CORE)	+= usbip/
>  
>  obj-$(CONFIG_TYPEC)		+= typec/
> diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> index 18f5dcf58b0d..1433260d99b4 100644
> --- a/drivers/usb/common/common.c
> +++ b/drivers/usb/common/common.c
> @@ -15,6 +15,8 @@
>  #include <linux/usb/of.h>
>  #include <linux/usb/otg.h>
>  #include <linux/of_platform.h>
> +#include <linux/debugfs.h>
> +#include "common.h"
>  
>  static const char *const ep_type_names[] = {
>  	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> @@ -291,4 +293,23 @@ struct device *usb_of_get_companion_dev(struct device *dev)
>  EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
>  #endif
>  
> +struct dentry *usb_debug_root;
> +EXPORT_SYMBOL_GPL(usb_debug_root);
> +
> +static int __init usb_common_init(void)
> +{
> +	usb_debug_root = debugfs_create_dir("usb", NULL);
> +	ledtrig_usb_init();
> +	return 0;
> +}
> +
> +static void __exit usb_common_exit(void)
> +{
> +	ledtrig_usb_exit();
> +	debugfs_remove_recursive(usb_debug_root);
> +}
> +
> +subsys_initcall(usb_common_init);
> +module_exit(usb_common_exit);
> +
>  MODULE_LICENSE("GPL");
> diff --git a/drivers/usb/common/common.h b/drivers/usb/common/common.h
> new file mode 100644
> index 000000000000..424a91316a4b
> --- /dev/null
> +++ b/drivers/usb/common/common.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __LINUX_USB_COMMON_H
> +#define __LINUX_USB_COMMON_H
> +
> +#if defined(CONFIG_USB_LED_TRIG)
> +void ledtrig_usb_init(void);
> +void ledtrig_usb_exit(void);
> +#else
> +static inline void ledtrig_usb_init(void) { }
> +static inline void ledtrig_usb_exit(void) { }
> +#endif
> +
> +#endif	/* __LINUX_USB_COMMON_H */
> diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
> index 7bd81166b77d..0865dd44a80a 100644
> --- a/drivers/usb/common/led.c
> +++ b/drivers/usb/common/led.c
> @@ -10,6 +10,7 @@
>  #include <linux/init.h>
>  #include <linux/leds.h>
>  #include <linux/usb.h>
> +#include "common.h"
>  
>  #define BLINK_DELAY 30
>  
> @@ -36,18 +37,14 @@ void usb_led_activity(enum usb_led_event ev)
>  EXPORT_SYMBOL_GPL(usb_led_activity);
>  
> 
> -static int __init ledtrig_usb_init(void)
> +void __init ledtrig_usb_init(void)
>  {
>  	led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
>  	led_trigger_register_simple("usb-host", &ledtrig_usb_host);
> -	return 0;
>  }
>  
> -static void __exit ledtrig_usb_exit(void)
> +void __exit ledtrig_usb_exit(void)
>  {
>  	led_trigger_unregister_simple(ledtrig_usb_gadget);
>  	led_trigger_unregister_simple(ledtrig_usb_host);
>  }
> -
> -module_init(ledtrig_usb_init);
> -module_exit(ledtrig_usb_exit);
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 7fcb9f782931..5a0df527a8ca 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
>  	.notifier_call = usb_bus_notify,
>  };
>  
> -struct dentry *usb_debug_root;
> -EXPORT_SYMBOL_GPL(usb_debug_root);
> +static struct dentry *usb_devices_root;
>  
>  static void usb_debugfs_init(void)
>  {
> -	usb_debug_root = debugfs_create_dir("usb", NULL);
> -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> -			    &usbfs_devices_fops);
> +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
> +					       NULL, &usbfs_devices_fops);
>  }
>  
>  static void usb_debugfs_cleanup(void)
>  {
> -	debugfs_remove_recursive(usb_debug_root);
> +	debugfs_remove(usb_devices_root);
>  }
>  
>  /*



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] USB: move usb debugfs directory creation to the usb common core
@ 2019-06-06  2:32     ` Chunfeng Yun
  0 siblings, 0 replies; 16+ messages in thread
From: Chunfeng Yun @ 2019-06-06  2:32 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Felipe Balbi, Matthias Brugger, linux-usb, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek

On Wed, 2019-06-05 at 14:44 +0200, Greg Kroah-Hartman wrote:
> The USB gadget subsystem wants to use the USB debugfs root directory, so
> move it to the common "core" USB code so that it is properly initialized
> and removed as needed.
> 
> In order to properly do this, we need to load the common code before the
> usb core code, when everything is linked into the kernel, so reorder the
> link order of the code.
> 
> Also as the usb common code has the possibility of the led trigger logic
> to be merged into it, handle the build option properly by only having
> one module init/exit function and have the common code initialize the
> led trigger if needed.
> 
> Reported-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> Chunfeng, can you try testing this again?

Tested-by: Chunfeng Yun <chunfeng.yun@mediatek.com>

Thank you, Greg, Felipe

> 
> v3: Fix __init and __exit error when building into the tree as reported
>     by Chunfeng
>     Fix Reported-by: line as reported
> v2: handle led common code link error reported by kbuild
>     handle subsys_initcall issue pointed out by Chunfeng
> 
>  drivers/usb/Makefile        |  3 +--
>  drivers/usb/common/common.c | 21 +++++++++++++++++++++
>  drivers/usb/common/common.h | 14 ++++++++++++++
>  drivers/usb/common/led.c    |  9 +++------
>  drivers/usb/core/usb.c      | 10 ++++------
>  5 files changed, 43 insertions(+), 14 deletions(-)
>  create mode 100644 drivers/usb/common/common.h
> 
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index 7d1b8c82b208..ecc2de1ffaae 100644
> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -5,6 +5,7 @@
>  
>  # Object files in subdirectories
>  
> +obj-$(CONFIG_USB_COMMON)	+= common/
>  obj-$(CONFIG_USB)		+= core/
>  obj-$(CONFIG_USB_SUPPORT)	+= phy/
>  
> @@ -60,8 +61,6 @@ obj-$(CONFIG_USB_CHIPIDEA)	+= chipidea/
>  obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
>  obj-$(CONFIG_USB_GADGET)	+= gadget/
>  
> -obj-$(CONFIG_USB_COMMON)	+= common/
> -
>  obj-$(CONFIG_USBIP_CORE)	+= usbip/
>  
>  obj-$(CONFIG_TYPEC)		+= typec/
> diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> index 18f5dcf58b0d..1433260d99b4 100644
> --- a/drivers/usb/common/common.c
> +++ b/drivers/usb/common/common.c
> @@ -15,6 +15,8 @@
>  #include <linux/usb/of.h>
>  #include <linux/usb/otg.h>
>  #include <linux/of_platform.h>
> +#include <linux/debugfs.h>
> +#include "common.h"
>  
>  static const char *const ep_type_names[] = {
>  	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> @@ -291,4 +293,23 @@ struct device *usb_of_get_companion_dev(struct device *dev)
>  EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
>  #endif
>  
> +struct dentry *usb_debug_root;
> +EXPORT_SYMBOL_GPL(usb_debug_root);
> +
> +static int __init usb_common_init(void)
> +{
> +	usb_debug_root = debugfs_create_dir("usb", NULL);
> +	ledtrig_usb_init();
> +	return 0;
> +}
> +
> +static void __exit usb_common_exit(void)
> +{
> +	ledtrig_usb_exit();
> +	debugfs_remove_recursive(usb_debug_root);
> +}
> +
> +subsys_initcall(usb_common_init);
> +module_exit(usb_common_exit);
> +
>  MODULE_LICENSE("GPL");
> diff --git a/drivers/usb/common/common.h b/drivers/usb/common/common.h
> new file mode 100644
> index 000000000000..424a91316a4b
> --- /dev/null
> +++ b/drivers/usb/common/common.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __LINUX_USB_COMMON_H
> +#define __LINUX_USB_COMMON_H
> +
> +#if defined(CONFIG_USB_LED_TRIG)
> +void ledtrig_usb_init(void);
> +void ledtrig_usb_exit(void);
> +#else
> +static inline void ledtrig_usb_init(void) { }
> +static inline void ledtrig_usb_exit(void) { }
> +#endif
> +
> +#endif	/* __LINUX_USB_COMMON_H */
> diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
> index 7bd81166b77d..0865dd44a80a 100644
> --- a/drivers/usb/common/led.c
> +++ b/drivers/usb/common/led.c
> @@ -10,6 +10,7 @@
>  #include <linux/init.h>
>  #include <linux/leds.h>
>  #include <linux/usb.h>
> +#include "common.h"
>  
>  #define BLINK_DELAY 30
>  
> @@ -36,18 +37,14 @@ void usb_led_activity(enum usb_led_event ev)
>  EXPORT_SYMBOL_GPL(usb_led_activity);
>  
> 
> -static int __init ledtrig_usb_init(void)
> +void __init ledtrig_usb_init(void)
>  {
>  	led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
>  	led_trigger_register_simple("usb-host", &ledtrig_usb_host);
> -	return 0;
>  }
>  
> -static void __exit ledtrig_usb_exit(void)
> +void __exit ledtrig_usb_exit(void)
>  {
>  	led_trigger_unregister_simple(ledtrig_usb_gadget);
>  	led_trigger_unregister_simple(ledtrig_usb_host);
>  }
> -
> -module_init(ledtrig_usb_init);
> -module_exit(ledtrig_usb_exit);
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 7fcb9f782931..5a0df527a8ca 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
>  	.notifier_call = usb_bus_notify,
>  };
>  
> -struct dentry *usb_debug_root;
> -EXPORT_SYMBOL_GPL(usb_debug_root);
> +static struct dentry *usb_devices_root;
>  
>  static void usb_debugfs_init(void)
>  {
> -	usb_debug_root = debugfs_create_dir("usb", NULL);
> -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> -			    &usbfs_devices_fops);
> +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
> +					       NULL, &usbfs_devices_fops);
>  }
>  
>  static void usb_debugfs_cleanup(void)
>  {
> -	debugfs_remove_recursive(usb_debug_root);
> +	debugfs_remove(usb_devices_root);
>  }
>  
>  /*

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] USB: move usb debugfs directory creation to the usb common core
@ 2019-06-06  2:32     ` Chunfeng Yun
  0 siblings, 0 replies; 16+ messages in thread
From: Chunfeng Yun @ 2019-06-06  2:32 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: devicetree, Felipe Balbi, linux-usb, linux-kernel,
	linux-mediatek, Matthias Brugger, linux-arm-kernel

On Wed, 2019-06-05 at 14:44 +0200, Greg Kroah-Hartman wrote:
> The USB gadget subsystem wants to use the USB debugfs root directory, so
> move it to the common "core" USB code so that it is properly initialized
> and removed as needed.
> 
> In order to properly do this, we need to load the common code before the
> usb core code, when everything is linked into the kernel, so reorder the
> link order of the code.
> 
> Also as the usb common code has the possibility of the led trigger logic
> to be merged into it, handle the build option properly by only having
> one module init/exit function and have the common code initialize the
> led trigger if needed.
> 
> Reported-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> Chunfeng, can you try testing this again?

Tested-by: Chunfeng Yun <chunfeng.yun@mediatek.com>

Thank you, Greg, Felipe

> 
> v3: Fix __init and __exit error when building into the tree as reported
>     by Chunfeng
>     Fix Reported-by: line as reported
> v2: handle led common code link error reported by kbuild
>     handle subsys_initcall issue pointed out by Chunfeng
> 
>  drivers/usb/Makefile        |  3 +--
>  drivers/usb/common/common.c | 21 +++++++++++++++++++++
>  drivers/usb/common/common.h | 14 ++++++++++++++
>  drivers/usb/common/led.c    |  9 +++------
>  drivers/usb/core/usb.c      | 10 ++++------
>  5 files changed, 43 insertions(+), 14 deletions(-)
>  create mode 100644 drivers/usb/common/common.h
> 
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index 7d1b8c82b208..ecc2de1ffaae 100644
> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -5,6 +5,7 @@
>  
>  # Object files in subdirectories
>  
> +obj-$(CONFIG_USB_COMMON)	+= common/
>  obj-$(CONFIG_USB)		+= core/
>  obj-$(CONFIG_USB_SUPPORT)	+= phy/
>  
> @@ -60,8 +61,6 @@ obj-$(CONFIG_USB_CHIPIDEA)	+= chipidea/
>  obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
>  obj-$(CONFIG_USB_GADGET)	+= gadget/
>  
> -obj-$(CONFIG_USB_COMMON)	+= common/
> -
>  obj-$(CONFIG_USBIP_CORE)	+= usbip/
>  
>  obj-$(CONFIG_TYPEC)		+= typec/
> diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> index 18f5dcf58b0d..1433260d99b4 100644
> --- a/drivers/usb/common/common.c
> +++ b/drivers/usb/common/common.c
> @@ -15,6 +15,8 @@
>  #include <linux/usb/of.h>
>  #include <linux/usb/otg.h>
>  #include <linux/of_platform.h>
> +#include <linux/debugfs.h>
> +#include "common.h"
>  
>  static const char *const ep_type_names[] = {
>  	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> @@ -291,4 +293,23 @@ struct device *usb_of_get_companion_dev(struct device *dev)
>  EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
>  #endif
>  
> +struct dentry *usb_debug_root;
> +EXPORT_SYMBOL_GPL(usb_debug_root);
> +
> +static int __init usb_common_init(void)
> +{
> +	usb_debug_root = debugfs_create_dir("usb", NULL);
> +	ledtrig_usb_init();
> +	return 0;
> +}
> +
> +static void __exit usb_common_exit(void)
> +{
> +	ledtrig_usb_exit();
> +	debugfs_remove_recursive(usb_debug_root);
> +}
> +
> +subsys_initcall(usb_common_init);
> +module_exit(usb_common_exit);
> +
>  MODULE_LICENSE("GPL");
> diff --git a/drivers/usb/common/common.h b/drivers/usb/common/common.h
> new file mode 100644
> index 000000000000..424a91316a4b
> --- /dev/null
> +++ b/drivers/usb/common/common.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __LINUX_USB_COMMON_H
> +#define __LINUX_USB_COMMON_H
> +
> +#if defined(CONFIG_USB_LED_TRIG)
> +void ledtrig_usb_init(void);
> +void ledtrig_usb_exit(void);
> +#else
> +static inline void ledtrig_usb_init(void) { }
> +static inline void ledtrig_usb_exit(void) { }
> +#endif
> +
> +#endif	/* __LINUX_USB_COMMON_H */
> diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
> index 7bd81166b77d..0865dd44a80a 100644
> --- a/drivers/usb/common/led.c
> +++ b/drivers/usb/common/led.c
> @@ -10,6 +10,7 @@
>  #include <linux/init.h>
>  #include <linux/leds.h>
>  #include <linux/usb.h>
> +#include "common.h"
>  
>  #define BLINK_DELAY 30
>  
> @@ -36,18 +37,14 @@ void usb_led_activity(enum usb_led_event ev)
>  EXPORT_SYMBOL_GPL(usb_led_activity);
>  
> 
> -static int __init ledtrig_usb_init(void)
> +void __init ledtrig_usb_init(void)
>  {
>  	led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
>  	led_trigger_register_simple("usb-host", &ledtrig_usb_host);
> -	return 0;
>  }
>  
> -static void __exit ledtrig_usb_exit(void)
> +void __exit ledtrig_usb_exit(void)
>  {
>  	led_trigger_unregister_simple(ledtrig_usb_gadget);
>  	led_trigger_unregister_simple(ledtrig_usb_host);
>  }
> -
> -module_init(ledtrig_usb_init);
> -module_exit(ledtrig_usb_exit);
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 7fcb9f782931..5a0df527a8ca 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -1185,19 +1185,17 @@ static struct notifier_block usb_bus_nb = {
>  	.notifier_call = usb_bus_notify,
>  };
>  
> -struct dentry *usb_debug_root;
> -EXPORT_SYMBOL_GPL(usb_debug_root);
> +static struct dentry *usb_devices_root;
>  
>  static void usb_debugfs_init(void)
>  {
> -	usb_debug_root = debugfs_create_dir("usb", NULL);
> -	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
> -			    &usbfs_devices_fops);
> +	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
> +					       NULL, &usbfs_devices_fops);
>  }
>  
>  static void usb_debugfs_cleanup(void)
>  {
> -	debugfs_remove_recursive(usb_debug_root);
> +	debugfs_remove(usb_devices_root);
>  }
>  
>  /*



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] USB: move usb debugfs directory creation to the usb common core
  2019-06-06  2:32     ` Chunfeng Yun
@ 2019-06-06  6:58       ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 16+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-06  6:58 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Felipe Balbi, Matthias Brugger, linux-usb, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek

On Thu, Jun 06, 2019 at 10:32:48AM +0800, Chunfeng Yun wrote:
> On Wed, 2019-06-05 at 14:44 +0200, Greg Kroah-Hartman wrote:
> > The USB gadget subsystem wants to use the USB debugfs root directory, so
> > move it to the common "core" USB code so that it is properly initialized
> > and removed as needed.
> > 
> > In order to properly do this, we need to load the common code before the
> > usb core code, when everything is linked into the kernel, so reorder the
> > link order of the code.
> > 
> > Also as the usb common code has the possibility of the led trigger logic
> > to be merged into it, handle the build option properly by only having
> > one module init/exit function and have the common code initialize the
> > led trigger if needed.
> > 
> > Reported-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > Chunfeng, can you try testing this again?
> 
> Tested-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> 
> Thank you, Greg, Felipe

Thanks for the review, and all of the different iterations for this.
For something so "simple" it sure took us all a number of tries :)

I'll go queue it up now so that your future work can rely on it.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] USB: move usb debugfs directory creation to the usb common core
@ 2019-06-06  6:58       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 16+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-06  6:58 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: devicetree, Felipe Balbi, linux-usb, linux-kernel,
	linux-mediatek, Matthias Brugger, linux-arm-kernel

On Thu, Jun 06, 2019 at 10:32:48AM +0800, Chunfeng Yun wrote:
> On Wed, 2019-06-05 at 14:44 +0200, Greg Kroah-Hartman wrote:
> > The USB gadget subsystem wants to use the USB debugfs root directory, so
> > move it to the common "core" USB code so that it is properly initialized
> > and removed as needed.
> > 
> > In order to properly do this, we need to load the common code before the
> > usb core code, when everything is linked into the kernel, so reorder the
> > link order of the code.
> > 
> > Also as the usb common code has the possibility of the led trigger logic
> > to be merged into it, handle the build option properly by only having
> > one module init/exit function and have the common code initialize the
> > led trigger if needed.
> > 
> > Reported-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > Chunfeng, can you try testing this again?
> 
> Tested-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> 
> Thank you, Greg, Felipe

Thanks for the review, and all of the different iterations for this.
For something so "simple" it sure took us all a number of tries :)

I'll go queue it up now so that your future work can rely on it.

thanks,

greg k-h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-06-06  6:58 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-05  9:28 [PATCH v2] USB: move usb debugfs directory creation to the usb common core Greg Kroah-Hartman
2019-06-05  9:28 ` Greg Kroah-Hartman
2019-06-05 11:01 ` Chunfeng Yun
2019-06-05 11:01   ` Chunfeng Yun
2019-06-05 11:01   ` Chunfeng Yun
2019-06-05 12:40   ` Greg Kroah-Hartman
2019-06-05 12:40     ` Greg Kroah-Hartman
2019-06-05 11:44 ` Marc Gonzalez
2019-06-05 12:39   ` Greg Kroah-Hartman
2019-06-05 12:44 ` [PATCH v3] " Greg Kroah-Hartman
2019-06-05 12:44   ` Greg Kroah-Hartman
2019-06-06  2:32   ` Chunfeng Yun
2019-06-06  2:32     ` Chunfeng Yun
2019-06-06  2:32     ` Chunfeng Yun
2019-06-06  6:58     ` Greg Kroah-Hartman
2019-06-06  6:58       ` Greg Kroah-Hartman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.