* [PATCH] USB: move usb debugfs directory creation to the usb common core
@ 2019-06-04 9:32 Greg Kroah-Hartman
2019-06-04 11:59 ` Greg Kroah-Hartman
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-04 9:32 UTC (permalink / raw)
To: Felipe Balbi
Cc: devicetree, linux-usb, linux-kernel, Matthias Brugger,
linux-mediatek, Chunfeng Yun, 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.
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
This should be the "correct" version of this, Chunfeng, can you test
this to verify it works for you?
diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index 18f5dcf58b0d..3b5e4263ffef 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -15,6 +15,7 @@
#include <linux/usb/of.h>
#include <linux/usb/otg.h>
#include <linux/of_platform.h>
+#include <linux/debugfs.h>
static const char *const ep_type_names[] = {
[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
@@ -291,4 +292,21 @@ 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);
+ return 0;
+}
+
+static void usb_common_exit(void)
+{
+ debugfs_remove_recursive(usb_debug_root);
+}
+
+module_init(usb_common_init);
+module_exit(usb_common_exit);
+
MODULE_LICENSE("GPL");
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 7fcb9f782931..f3d6b1ab80cb 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_recursive(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 related [flat|nested] 10+ messages in thread
* Re: [PATCH] USB: move usb debugfs directory creation to the usb common core
2019-06-04 9:32 [PATCH] USB: move usb debugfs directory creation to the usb common core Greg Kroah-Hartman
@ 2019-06-04 11:59 ` Greg Kroah-Hartman
2019-06-05 2:15 ` Chunfeng Yun
2019-06-05 7:50 ` Chunfeng Yun
2019-06-04 12:25 ` Felipe Balbi
2019-06-05 7:20 ` kbuild test robot
2 siblings, 2 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-04 11:59 UTC (permalink / raw)
To: Felipe Balbi
Cc: devicetree, linux-usb, linux-kernel, Matthias Brugger,
linux-mediatek, Chunfeng Yun, linux-arm-kernel
On Tue, Jun 04, 2019 at 11:32:58AM +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.
>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> ---
>
> This should be the "correct" version of this, Chunfeng, can you test
> this to verify it works for you?
>
>
> diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> index 18f5dcf58b0d..3b5e4263ffef 100644
> --- a/drivers/usb/common/common.c
> +++ b/drivers/usb/common/common.c
> @@ -15,6 +15,7 @@
> #include <linux/usb/of.h>
> #include <linux/usb/otg.h>
> #include <linux/of_platform.h>
> +#include <linux/debugfs.h>
>
> static const char *const ep_type_names[] = {
> [USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> @@ -291,4 +292,21 @@ 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);
> + return 0;
> +}
> +
> +static void usb_common_exit(void)
> +{
> + debugfs_remove_recursive(usb_debug_root);
> +}
> +
> +module_init(usb_common_init);
> +module_exit(usb_common_exit);
> +
> MODULE_LICENSE("GPL");
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 7fcb9f782931..f3d6b1ab80cb 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_recursive(usb_devices_root);
That should just be debugfs_remove();
I'll fix it up after someone tests this :)
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] 10+ messages in thread
* Re: [PATCH] USB: move usb debugfs directory creation to the usb common core
2019-06-04 9:32 [PATCH] USB: move usb debugfs directory creation to the usb common core Greg Kroah-Hartman
2019-06-04 11:59 ` Greg Kroah-Hartman
@ 2019-06-04 12:25 ` Felipe Balbi
2019-06-04 12:43 ` Greg Kroah-Hartman
2019-06-05 7:20 ` kbuild test robot
2 siblings, 1 reply; 10+ messages in thread
From: Felipe Balbi @ 2019-06-04 12:25 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, linux-usb, linux-kernel, Matthias Brugger,
linux-mediatek, Chunfeng Yun, linux-arm-kernel
Hi,
Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 7fcb9f782931..f3d6b1ab80cb 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,
don't we have a race now? Can usbcore ever probe before usb common?
--
balbi
_______________________________________________
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] 10+ messages in thread
* Re: [PATCH] USB: move usb debugfs directory creation to the usb common core
2019-06-04 12:25 ` Felipe Balbi
@ 2019-06-04 12:43 ` Greg Kroah-Hartman
2019-06-05 7:28 ` Felipe Balbi
0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-04 12:43 UTC (permalink / raw)
To: Felipe Balbi
Cc: devicetree, linux-usb, linux-kernel, Matthias Brugger,
linux-mediatek, Chunfeng Yun, linux-arm-kernel
On Tue, Jun 04, 2019 at 03:25:14PM +0300, Felipe Balbi wrote:
>
> Hi,
>
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > index 7fcb9f782931..f3d6b1ab80cb 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,
>
> don't we have a race now? Can usbcore ever probe before usb common?
How can that happen if usb_debug_root is in usb common? The module
loader will not let that happen. Or it shouldn't :)
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] 10+ messages in thread
* Re: [PATCH] USB: move usb debugfs directory creation to the usb common core
2019-06-04 11:59 ` Greg Kroah-Hartman
@ 2019-06-05 2:15 ` Chunfeng Yun
2019-06-05 7:50 ` Chunfeng Yun
1 sibling, 0 replies; 10+ messages in thread
From: Chunfeng Yun @ 2019-06-05 2:15 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Felipe Balbi, linux-usb, linux-kernel,
linux-mediatek, Matthias Brugger, linux-arm-kernel
On Tue, 2019-06-04 at 13:59 +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 04, 2019 at 11:32:58AM +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.
> >
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >
> > ---
> >
> > This should be the "correct" version of this, Chunfeng, can you test
> > this to verify it works for you?
I'll test it ASAP, thanks a lot
> >
> >
> > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > index 18f5dcf58b0d..3b5e4263ffef 100644
> > --- a/drivers/usb/common/common.c
> > +++ b/drivers/usb/common/common.c
> > @@ -15,6 +15,7 @@
> > #include <linux/usb/of.h>
> > #include <linux/usb/otg.h>
> > #include <linux/of_platform.h>
> > +#include <linux/debugfs.h>
> >
> > static const char *const ep_type_names[] = {
> > [USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> > @@ -291,4 +292,21 @@ 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);
> > + return 0;
> > +}
> > +
> > +static void usb_common_exit(void)
> > +{
> > + debugfs_remove_recursive(usb_debug_root);
> > +}
> > +
> > +module_init(usb_common_init);
> > +module_exit(usb_common_exit);
> > +
> > MODULE_LICENSE("GPL");
> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > index 7fcb9f782931..f3d6b1ab80cb 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_recursive(usb_devices_root);
>
> That should just be debugfs_remove();
>
> I'll fix it up after someone tests this :)
>
> 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] 10+ messages in thread
* Re: [PATCH] USB: move usb debugfs directory creation to the usb common core
2019-06-04 9:32 [PATCH] USB: move usb debugfs directory creation to the usb common core Greg Kroah-Hartman
2019-06-04 11:59 ` Greg Kroah-Hartman
2019-06-04 12:25 ` Felipe Balbi
@ 2019-06-05 7:20 ` kbuild test robot
2 siblings, 0 replies; 10+ messages in thread
From: kbuild test robot @ 2019-06-05 7:20 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Felipe Balbi, linux-usb, linux-kernel,
Matthias Brugger, linux-mediatek, kbuild-all, Chunfeng Yun,
linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]
Hi Greg,
I love your patch! Yet something to improve:
[auto build test ERROR on usb/usb-testing]
[also build test ERROR on v5.2-rc3 next-20190604]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Greg-Kroah-Hartman/USB-move-usb-debugfs-directory-creation-to-the-usb-common-core/20190605-114326
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=sh
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/usb/common/led.o: In function `ledtrig_usb_exit':
>> led.c:(.exit.text+0x0): multiple definition of `cleanup_module'
drivers/usb/common/common.o:common.c:(.text+0x518): first defined here
drivers/usb/common/led.o: In function `ledtrig_usb_init':
>> led.c:(.init.text+0x0): multiple definition of `init_module'
drivers/usb/common/common.o:common.c:(.text+0x4dc): first defined here
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 52829 bytes --]
[-- Attachment #3: Type: text/plain, Size: 176 bytes --]
_______________________________________________
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] 10+ messages in thread
* Re: [PATCH] USB: move usb debugfs directory creation to the usb common core
2019-06-04 12:43 ` Greg Kroah-Hartman
@ 2019-06-05 7:28 ` Felipe Balbi
2019-06-05 8:37 ` Chunfeng Yun
0 siblings, 1 reply; 10+ messages in thread
From: Felipe Balbi @ 2019-06-05 7:28 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, linux-usb, linux-kernel, Matthias Brugger,
linux-mediatek, Chunfeng Yun, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 1199 bytes --]
Hi,
Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
>> > index 7fcb9f782931..f3d6b1ab80cb 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,
>>
>> don't we have a race now? Can usbcore ever probe before usb common?
>
> How can that happen if usb_debug_root is in usb common? The module
> loader will not let that happen. Or it shouldn't :)
argh, indeed. The very fact that usbcore tries to resolve usb_debug_root
already forces a dependency :-p
--
balbi
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
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] 10+ messages in thread
* Re: [PATCH] USB: move usb debugfs directory creation to the usb common core
2019-06-04 11:59 ` Greg Kroah-Hartman
2019-06-05 2:15 ` Chunfeng Yun
@ 2019-06-05 7:50 ` Chunfeng Yun
2019-06-05 8:52 ` Greg Kroah-Hartman
1 sibling, 1 reply; 10+ messages in thread
From: Chunfeng Yun @ 2019-06-05 7:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Felipe Balbi, linux-usb, linux-kernel,
linux-mediatek, Matthias Brugger, linux-arm-kernel
On Tue, 2019-06-04 at 13:59 +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 04, 2019 at 11:32:58AM +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.
> >
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >
> > ---
> >
> > This should be the "correct" version of this, Chunfeng, can you test
> > this to verify it works for you?
> >
> >
> > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > index 18f5dcf58b0d..3b5e4263ffef 100644
> > --- a/drivers/usb/common/common.c
> > +++ b/drivers/usb/common/common.c
> > @@ -15,6 +15,7 @@
> > #include <linux/usb/of.h>
> > #include <linux/usb/otg.h>
> > #include <linux/of_platform.h>
> > +#include <linux/debugfs.h>
> >
> > static const char *const ep_type_names[] = {
> > [USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> > @@ -291,4 +292,21 @@ 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);
> > + return 0;
> > +}
> > +
> > +static void usb_common_exit(void)
> > +{
> > + debugfs_remove_recursive(usb_debug_root);
> > +}
> > +
> > +module_init(usb_common_init);
I tested this patch.
Here use module_init() indeed have a race as Felipe said before.
usbcore uses subsys_initcall(), and have a higher priority than
module_init(), so when usbcore tries to create "devices" file,
usb_debug_root is not created.
after I replace it by postcore_initcall() (debugfs uses
core_initcall()), test two cases:
1. buildin usbcore/udc-core
"usb" directory is created, and usb/devices file is also created by
usbcore
2. build both usbcore and gadget as ko
usbcore.ko, udc-core.ko and usb-common.ko are created.
2.1
insmod usb-common.ko // "usb" directory is created
insmod usb-core.ko // usb/devices file is created
2.2
rmmod usb-common.ko // failed, usb_common is in use by usb-core
2.3
rmmod usb-core.ko // usb/devices file is destroyed
rmmod usb-common.ko // usb directory is destroyed
2.4
insmod usb-common.ko // "usb" directory is created
insmod udc-core.ko
2.5
rmmod usb-common.ko // failed, usb_common is in use by udc-core
2.6
rmmod udc-core.ko
rmmod usb-common.ko // usb directory is destroyed
they are all in line with expectations
> > +module_exit(usb_common_exit);
> > +
> > MODULE_LICENSE("GPL");
> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > index 7fcb9f782931..f3d6b1ab80cb 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_recursive(usb_devices_root);
>
> That should just be debugfs_remove();
>
> I'll fix it up after someone tests this :)
>
> 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] 10+ messages in thread
* Re: [PATCH] USB: move usb debugfs directory creation to the usb common core
2019-06-05 7:28 ` Felipe Balbi
@ 2019-06-05 8:37 ` Chunfeng Yun
0 siblings, 0 replies; 10+ messages in thread
From: Chunfeng Yun @ 2019-06-05 8:37 UTC (permalink / raw)
To: Felipe Balbi
Cc: devicetree, Greg Kroah-Hartman, linux-usb, linux-kernel,
linux-mediatek, Matthias Brugger, linux-arm-kernel
On Wed, 2019-06-05 at 10:28 +0300, Felipe Balbi wrote:
> Hi,
>
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> >> > index 7fcb9f782931..f3d6b1ab80cb 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,
> >>
> >> don't we have a race now? Can usbcore ever probe before usb common?
> >
> > How can that happen if usb_debug_root is in usb common? The module
> > loader will not let that happen. Or it shouldn't :)
>
> argh, indeed. The very fact that usbcore tries to resolve usb_debug_root
> already forces a dependency :-p
When build as module, usbcore depend on usb-common, but when buildin,
usbcore init before usb-common (use module_init)
>
_______________________________________________
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] 10+ messages in thread
* Re: [PATCH] USB: move usb debugfs directory creation to the usb common core
2019-06-05 7:50 ` Chunfeng Yun
@ 2019-06-05 8:52 ` Greg Kroah-Hartman
0 siblings, 0 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-05 8:52 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 03:50:31PM +0800, Chunfeng Yun wrote:
> On Tue, 2019-06-04 at 13:59 +0200, Greg Kroah-Hartman wrote:
> > On Tue, Jun 04, 2019 at 11:32:58AM +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.
> > >
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > >
> > > ---
> > >
> > > This should be the "correct" version of this, Chunfeng, can you test
> > > this to verify it works for you?
> > >
> > >
> > > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > > index 18f5dcf58b0d..3b5e4263ffef 100644
> > > --- a/drivers/usb/common/common.c
> > > +++ b/drivers/usb/common/common.c
> > > @@ -15,6 +15,7 @@
> > > #include <linux/usb/of.h>
> > > #include <linux/usb/otg.h>
> > > #include <linux/of_platform.h>
> > > +#include <linux/debugfs.h>
> > >
> > > static const char *const ep_type_names[] = {
> > > [USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> > > @@ -291,4 +292,21 @@ 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);
> > > + return 0;
> > > +}
> > > +
> > > +static void usb_common_exit(void)
> > > +{
> > > + debugfs_remove_recursive(usb_debug_root);
> > > +}
> > > +
> > > +module_init(usb_common_init);
> I tested this patch.
>
> Here use module_init() indeed have a race as Felipe said before.
> usbcore uses subsys_initcall(), and have a higher priority than
> module_init(), so when usbcore tries to create "devices" file,
> usb_debug_root is not created.
Ah, let me fix that, it should have the same init level and I'll ensure
it comes first in the linking.
> after I replace it by postcore_initcall() (debugfs uses
> core_initcall()), test two cases:
>
> 1. buildin usbcore/udc-core
>
> "usb" directory is created, and usb/devices file is also created by
> usbcore
>
> 2. build both usbcore and gadget as ko
>
> usbcore.ko, udc-core.ko and usb-common.ko are created.
>
> 2.1
> insmod usb-common.ko // "usb" directory is created
> insmod usb-core.ko // usb/devices file is created
>
> 2.2
> rmmod usb-common.ko // failed, usb_common is in use by usb-core
>
> 2.3
> rmmod usb-core.ko // usb/devices file is destroyed
> rmmod usb-common.ko // usb directory is destroyed
>
> 2.4
> insmod usb-common.ko // "usb" directory is created
> insmod udc-core.ko
>
> 2.5
> rmmod usb-common.ko // failed, usb_common is in use by udc-core
>
> 2.6
> rmmod udc-core.ko
> rmmod usb-common.ko // usb directory is destroyed
>
> they are all in line with expectations
Wonderful!
Let me fix up the init level, and the build issue tha kbuild found, and
post a v2 patch.
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] 10+ messages in thread
end of thread, other threads:[~2019-06-05 8:52 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-04 9:32 [PATCH] USB: move usb debugfs directory creation to the usb common core Greg Kroah-Hartman
2019-06-04 11:59 ` Greg Kroah-Hartman
2019-06-05 2:15 ` Chunfeng Yun
2019-06-05 7:50 ` Chunfeng Yun
2019-06-05 8:52 ` Greg Kroah-Hartman
2019-06-04 12:25 ` Felipe Balbi
2019-06-04 12:43 ` Greg Kroah-Hartman
2019-06-05 7:28 ` Felipe Balbi
2019-06-05 8:37 ` Chunfeng Yun
2019-06-05 7:20 ` kbuild test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).