[PATCH v4] usb: create usb_debug_root for gadget only

[PATCH v4] usb: create usb_debug_root for gadget only

[Chunfeng Yun]

When CONFIG_USB is not set, and CONFIG_USB_GADGET is set,
there is an issue, e.g.:

drivers/usb/mtu3/mtu3_debugfs.o: in function 'ssusb_debugfs_create_root':
mtu3_debugfs.c:(.text+0xba3): undefined reference to 'usb_debug_root'

usb_debug_root is currently only built when host is supported
(CONFIG_USB is set), for convenience, we also want it created when
gadget only is enabled, this patch try to support it.

Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v4:
  move common API into common.c suggested by Felipe

v3:
  1. still create usb_debug_root for gadget only
  2. abandon mtu3's change
  3. drop acked-by Randy

v2(resend): add acked-by Randy

v1: fix mtu3's build error, replace usb_debug_root by NULL;
---
 drivers/usb/common/common.c   | 26 ++++++++++++++++++++++++++
 drivers/usb/core/usb.c        | 16 ++++------------
 drivers/usb/gadget/udc/core.c |  3 +++
 include/linux/usb.h           |  2 ++
 4 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index 18f5dcf58b0d..c52e9c9f58ec 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",
@@ -139,6 +140,31 @@ enum usb_dr_mode usb_get_dr_mode(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(usb_get_dr_mode);
 
+struct dentry *usb_debug_root;
+EXPORT_SYMBOL_GPL(usb_debug_root);
+
+static atomic_t usb_debug_root_refcnt = ATOMIC_INIT(0);
+
+struct dentry *usb_debugfs_init(void)
+{
+	if (!usb_debug_root)
+		usb_debug_root = debugfs_create_dir("usb", NULL);
+
+	atomic_inc(&usb_debug_root_refcnt);
+
+	return usb_debug_root;
+}
+EXPORT_SYMBOL_GPL(usb_debugfs_init);
+
+void usb_debugfs_cleanup(void)
+{
+	if (atomic_dec_and_test(&usb_debug_root_refcnt)) {
+		debugfs_remove_recursive(usb_debug_root);
+		usb_debug_root = NULL;
+	}
+}
+EXPORT_SYMBOL_GPL(usb_debugfs_cleanup);
+
 #ifdef CONFIG_OF
 /**
  * of_usb_get_dr_mode_by_phy - Get dual role mode for the controller device
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 7fcb9f782931..9d8db7faf75f 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -1185,19 +1185,11 @@ static struct notifier_block usb_bus_nb = {
 	.notifier_call = usb_bus_notify,
 };
 
-struct dentry *usb_debug_root;
-EXPORT_SYMBOL_GPL(usb_debug_root);
-
-static void usb_debugfs_init(void)
+static void usb_core_debugfs_init(void)
 {
-	usb_debug_root = debugfs_create_dir("usb", NULL);
-	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
-			    &usbfs_devices_fops);
-}
+	struct dentry *root = usb_debugfs_init();
 
-static void usb_debugfs_cleanup(void)
-{
-	debugfs_remove_recursive(usb_debug_root);
+	debugfs_create_file("devices", 0444, root, NULL, &usbfs_devices_fops);
 }
 
 /*
@@ -1212,7 +1204,7 @@ static int __init usb_init(void)
 	}
 	usb_init_pool_max();
 
-	usb_debugfs_init();
+	usb_core_debugfs_init();
 
 	usb_acpi_register();
 	retval = bus_register(&usb_bus_type);
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 7cf34beb50df..8ef000b9baa4 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1597,12 +1597,15 @@ static int __init usb_udc_init(void)
 	}
 
 	udc_class->dev_uevent = usb_udc_uevent;
+	usb_debugfs_init();
+
 	return 0;
 }
 subsys_initcall(usb_udc_init);
 
 static void __exit usb_udc_exit(void)
 {
+	usb_debugfs_cleanup();
 	class_destroy(udc_class);
 }
 module_exit(usb_udc_exit);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index ae82d9d1112b..175e4a0b1847 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1995,6 +1995,8 @@ extern void usb_unregister_notify(struct notifier_block *nb);
 
 /* debugfs stuff */
 extern struct dentry *usb_debug_root;
+extern struct dentry *usb_debugfs_init(void);
+extern void usb_debugfs_cleanup(void);
 
 /* LED triggers */
 enum usb_led_event {
-- 
2.21.0

[PATCH v4] usb: create usb_debug_root for gadget only

[Chunfeng Yun]

When CONFIG_USB is not set, and CONFIG_USB_GADGET is set,
there is an issue, e.g.:

drivers/usb/mtu3/mtu3_debugfs.o: in function 'ssusb_debugfs_create_root':
mtu3_debugfs.c:(.text+0xba3): undefined reference to 'usb_debug_root'

usb_debug_root is currently only built when host is supported
(CONFIG_USB is set), for convenience, we also want it created when
gadget only is enabled, this patch try to support it.

Reported-by: Randy Dunlap <rdunlap-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Signed-off-by: Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
v4:
  move common API into common.c suggested by Felipe

v3:
  1. still create usb_debug_root for gadget only
  2. abandon mtu3's change
  3. drop acked-by Randy

v2(resend): add acked-by Randy

v1: fix mtu3's build error, replace usb_debug_root by NULL;
---
 drivers/usb/common/common.c   | 26 ++++++++++++++++++++++++++
 drivers/usb/core/usb.c        | 16 ++++------------
 drivers/usb/gadget/udc/core.c |  3 +++
 include/linux/usb.h           |  2 ++
 4 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index 18f5dcf58b0d..c52e9c9f58ec 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",
@@ -139,6 +140,31 @@ enum usb_dr_mode usb_get_dr_mode(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(usb_get_dr_mode);
 
+struct dentry *usb_debug_root;
+EXPORT_SYMBOL_GPL(usb_debug_root);
+
+static atomic_t usb_debug_root_refcnt = ATOMIC_INIT(0);
+
+struct dentry *usb_debugfs_init(void)
+{
+	if (!usb_debug_root)
+		usb_debug_root = debugfs_create_dir("usb", NULL);
+
+	atomic_inc(&usb_debug_root_refcnt);
+
+	return usb_debug_root;
+}
+EXPORT_SYMBOL_GPL(usb_debugfs_init);
+
+void usb_debugfs_cleanup(void)
+{
+	if (atomic_dec_and_test(&usb_debug_root_refcnt)) {
+		debugfs_remove_recursive(usb_debug_root);
+		usb_debug_root = NULL;
+	}
+}
+EXPORT_SYMBOL_GPL(usb_debugfs_cleanup);
+
 #ifdef CONFIG_OF
 /**
  * of_usb_get_dr_mode_by_phy - Get dual role mode for the controller device
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 7fcb9f782931..9d8db7faf75f 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -1185,19 +1185,11 @@ static struct notifier_block usb_bus_nb = {
 	.notifier_call = usb_bus_notify,
 };
 
-struct dentry *usb_debug_root;
-EXPORT_SYMBOL_GPL(usb_debug_root);
-
-static void usb_debugfs_init(void)
+static void usb_core_debugfs_init(void)
 {
-	usb_debug_root = debugfs_create_dir("usb", NULL);
-	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
-			    &usbfs_devices_fops);
-}
+	struct dentry *root = usb_debugfs_init();
 
-static void usb_debugfs_cleanup(void)
-{
-	debugfs_remove_recursive(usb_debug_root);
+	debugfs_create_file("devices", 0444, root, NULL, &usbfs_devices_fops);
 }
 
 /*
@@ -1212,7 +1204,7 @@ static int __init usb_init(void)
 	}
 	usb_init_pool_max();
 
-	usb_debugfs_init();
+	usb_core_debugfs_init();
 
 	usb_acpi_register();
 	retval = bus_register(&usb_bus_type);
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 7cf34beb50df..8ef000b9baa4 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1597,12 +1597,15 @@ static int __init usb_udc_init(void)
 	}
 
 	udc_class->dev_uevent = usb_udc_uevent;
+	usb_debugfs_init();
+
 	return 0;
 }
 subsys_initcall(usb_udc_init);
 
 static void __exit usb_udc_exit(void)
 {
+	usb_debugfs_cleanup();
 	class_destroy(udc_class);
 }
 module_exit(usb_udc_exit);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index ae82d9d1112b..175e4a0b1847 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1995,6 +1995,8 @@ extern void usb_unregister_notify(struct notifier_block *nb);
 
 /* debugfs stuff */
 extern struct dentry *usb_debug_root;
+extern struct dentry *usb_debugfs_init(void);
+extern void usb_debugfs_cleanup(void);
 
 /* LED triggers */
 enum usb_led_event {
-- 
2.21.0

[PATCH v4] usb: create usb_debug_root for gadget only

[Chunfeng Yun]

When CONFIG_USB is not set, and CONFIG_USB_GADGET is set,
there is an issue, e.g.:

drivers/usb/mtu3/mtu3_debugfs.o: in function 'ssusb_debugfs_create_root':
mtu3_debugfs.c:(.text+0xba3): undefined reference to 'usb_debug_root'

usb_debug_root is currently only built when host is supported
(CONFIG_USB is set), for convenience, we also want it created when
gadget only is enabled, this patch try to support it.

Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v4:
  move common API into common.c suggested by Felipe

v3:
  1. still create usb_debug_root for gadget only
  2. abandon mtu3's change
  3. drop acked-by Randy

v2(resend): add acked-by Randy

v1: fix mtu3's build error, replace usb_debug_root by NULL;
---
 drivers/usb/common/common.c   | 26 ++++++++++++++++++++++++++
 drivers/usb/core/usb.c        | 16 ++++------------
 drivers/usb/gadget/udc/core.c |  3 +++
 include/linux/usb.h           |  2 ++
 4 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index 18f5dcf58b0d..c52e9c9f58ec 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",
@@ -139,6 +140,31 @@ enum usb_dr_mode usb_get_dr_mode(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(usb_get_dr_mode);
 
+struct dentry *usb_debug_root;
+EXPORT_SYMBOL_GPL(usb_debug_root);
+
+static atomic_t usb_debug_root_refcnt = ATOMIC_INIT(0);
+
+struct dentry *usb_debugfs_init(void)
+{
+	if (!usb_debug_root)
+		usb_debug_root = debugfs_create_dir("usb", NULL);
+
+	atomic_inc(&usb_debug_root_refcnt);
+
+	return usb_debug_root;
+}
+EXPORT_SYMBOL_GPL(usb_debugfs_init);
+
+void usb_debugfs_cleanup(void)
+{
+	if (atomic_dec_and_test(&usb_debug_root_refcnt)) {
+		debugfs_remove_recursive(usb_debug_root);
+		usb_debug_root = NULL;
+	}
+}
+EXPORT_SYMBOL_GPL(usb_debugfs_cleanup);
+
 #ifdef CONFIG_OF
 /**
  * of_usb_get_dr_mode_by_phy - Get dual role mode for the controller device
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 7fcb9f782931..9d8db7faf75f 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -1185,19 +1185,11 @@ static struct notifier_block usb_bus_nb = {
 	.notifier_call = usb_bus_notify,
 };
 
-struct dentry *usb_debug_root;
-EXPORT_SYMBOL_GPL(usb_debug_root);
-
-static void usb_debugfs_init(void)
+static void usb_core_debugfs_init(void)
 {
-	usb_debug_root = debugfs_create_dir("usb", NULL);
-	debugfs_create_file("devices", 0444, usb_debug_root, NULL,
-			    &usbfs_devices_fops);
-}
+	struct dentry *root = usb_debugfs_init();
 
-static void usb_debugfs_cleanup(void)
-{
-	debugfs_remove_recursive(usb_debug_root);
+	debugfs_create_file("devices", 0444, root, NULL, &usbfs_devices_fops);
 }
 
 /*
@@ -1212,7 +1204,7 @@ static int __init usb_init(void)
 	}
 	usb_init_pool_max();
 
-	usb_debugfs_init();
+	usb_core_debugfs_init();
 
 	usb_acpi_register();
 	retval = bus_register(&usb_bus_type);
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 7cf34beb50df..8ef000b9baa4 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1597,12 +1597,15 @@ static int __init usb_udc_init(void)
 	}
 
 	udc_class->dev_uevent = usb_udc_uevent;
+	usb_debugfs_init();
+
 	return 0;
 }
 subsys_initcall(usb_udc_init);
 
 static void __exit usb_udc_exit(void)
 {
+	usb_debugfs_cleanup();
 	class_destroy(udc_class);
 }
 module_exit(usb_udc_exit);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index ae82d9d1112b..175e4a0b1847 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1995,6 +1995,8 @@ extern void usb_unregister_notify(struct notifier_block *nb);
 
 /* debugfs stuff */
 extern struct dentry *usb_debug_root;
+extern struct dentry *usb_debugfs_init(void);
+extern void usb_debugfs_cleanup(void);
 
 /* LED triggers */
 enum usb_led_event {
-- 
2.21.0


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

Re: [PATCH v4] usb: create usb_debug_root for gadget only

[Greg Kroah-Hartman]

On Tue, Jun 04, 2019 at 03:34:07PM +0800, Chunfeng Yun wrote:
> When CONFIG_USB is not set, and CONFIG_USB_GADGET is set,
> there is an issue, e.g.:
> 
> drivers/usb/mtu3/mtu3_debugfs.o: in function 'ssusb_debugfs_create_root':
> mtu3_debugfs.c:(.text+0xba3): undefined reference to 'usb_debug_root'
> 
> usb_debug_root is currently only built when host is supported
> (CONFIG_USB is set), for convenience, we also want it created when
> gadget only is enabled, this patch try to support it.
> 
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> v4:
>   move common API into common.c suggested by Felipe
> 
> v3:
>   1. still create usb_debug_root for gadget only
>   2. abandon mtu3's change
>   3. drop acked-by Randy
> 
> v2(resend): add acked-by Randy
> 
> v1: fix mtu3's build error, replace usb_debug_root by NULL;
> ---
>  drivers/usb/common/common.c   | 26 ++++++++++++++++++++++++++
>  drivers/usb/core/usb.c        | 16 ++++------------
>  drivers/usb/gadget/udc/core.c |  3 +++
>  include/linux/usb.h           |  2 ++
>  4 files changed, 35 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> index 18f5dcf58b0d..c52e9c9f58ec 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",
> @@ -139,6 +140,31 @@ enum usb_dr_mode usb_get_dr_mode(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(usb_get_dr_mode);
>  
> +struct dentry *usb_debug_root;
> +EXPORT_SYMBOL_GPL(usb_debug_root);
> +
> +static atomic_t usb_debug_root_refcnt = ATOMIC_INIT(0);

Ick, no.

> +
> +struct dentry *usb_debugfs_init(void)
> +{
> +	if (!usb_debug_root)
> +		usb_debug_root = debugfs_create_dir("usb", NULL);
> +
> +	atomic_inc(&usb_debug_root_refcnt);
> +
> +	return usb_debug_root;
> +}
> +EXPORT_SYMBOL_GPL(usb_debugfs_init);
> +
> +void usb_debugfs_cleanup(void)
> +{
> +	if (atomic_dec_and_test(&usb_debug_root_refcnt)) {
> +		debugfs_remove_recursive(usb_debug_root);
> +		usb_debug_root = NULL;
> +	}
> +}
> +EXPORT_SYMBOL_GPL(usb_debugfs_cleanup);

Only remove the debugfs subdir if the usbcore module is removed.  Create
the debugfs subdir when the usbcore module is loaded.  No need for any
reference counting of any sort at all.  No need to overthink this :)

thanks,

greg k-h

Re: [PATCH v4] usb: create usb_debug_root for gadget only

[Greg Kroah-Hartman]

On Tue, Jun 04, 2019 at 03:34:07PM +0800, Chunfeng Yun wrote:
> When CONFIG_USB is not set, and CONFIG_USB_GADGET is set,
> there is an issue, e.g.:
> 
> drivers/usb/mtu3/mtu3_debugfs.o: in function 'ssusb_debugfs_create_root':
> mtu3_debugfs.c:(.text+0xba3): undefined reference to 'usb_debug_root'
> 
> usb_debug_root is currently only built when host is supported
> (CONFIG_USB is set), for convenience, we also want it created when
> gadget only is enabled, this patch try to support it.
> 
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> v4:
>   move common API into common.c suggested by Felipe
> 
> v3:
>   1. still create usb_debug_root for gadget only
>   2. abandon mtu3's change
>   3. drop acked-by Randy
> 
> v2(resend): add acked-by Randy
> 
> v1: fix mtu3's build error, replace usb_debug_root by NULL;
> ---
>  drivers/usb/common/common.c   | 26 ++++++++++++++++++++++++++
>  drivers/usb/core/usb.c        | 16 ++++------------
>  drivers/usb/gadget/udc/core.c |  3 +++
>  include/linux/usb.h           |  2 ++
>  4 files changed, 35 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> index 18f5dcf58b0d..c52e9c9f58ec 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",
> @@ -139,6 +140,31 @@ enum usb_dr_mode usb_get_dr_mode(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(usb_get_dr_mode);
>  
> +struct dentry *usb_debug_root;
> +EXPORT_SYMBOL_GPL(usb_debug_root);
> +
> +static atomic_t usb_debug_root_refcnt = ATOMIC_INIT(0);

Ick, no.

> +
> +struct dentry *usb_debugfs_init(void)
> +{
> +	if (!usb_debug_root)
> +		usb_debug_root = debugfs_create_dir("usb", NULL);
> +
> +	atomic_inc(&usb_debug_root_refcnt);
> +
> +	return usb_debug_root;
> +}
> +EXPORT_SYMBOL_GPL(usb_debugfs_init);
> +
> +void usb_debugfs_cleanup(void)
> +{
> +	if (atomic_dec_and_test(&usb_debug_root_refcnt)) {
> +		debugfs_remove_recursive(usb_debug_root);
> +		usb_debug_root = NULL;
> +	}
> +}
> +EXPORT_SYMBOL_GPL(usb_debugfs_cleanup);

Only remove the debugfs subdir if the usbcore module is removed.  Create
the debugfs subdir when the usbcore module is loaded.  No need for any
reference counting of any sort at all.  No need to overthink 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

Re: [PATCH v4] usb: create usb_debug_root for gadget only

[Felipe Balbi]

Hi,

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> +struct dentry *usb_debugfs_init(void)
>> +{
>> +	if (!usb_debug_root)
>> +		usb_debug_root = debugfs_create_dir("usb", NULL);
>> +
>> +	atomic_inc(&usb_debug_root_refcnt);
>> +
>> +	return usb_debug_root;
>> +}
>> +EXPORT_SYMBOL_GPL(usb_debugfs_init);
>> +
>> +void usb_debugfs_cleanup(void)
>> +{
>> +	if (atomic_dec_and_test(&usb_debug_root_refcnt)) {
>> +		debugfs_remove_recursive(usb_debug_root);
>> +		usb_debug_root = NULL;
>> +	}
>> +}
>> +EXPORT_SYMBOL_GPL(usb_debugfs_cleanup);
>
> Only remove the debugfs subdir if the usbcore module is removed.  Create
> the debugfs subdir when the usbcore module is loaded.  No need for any
> reference counting of any sort at all.  No need to overthink this :)

There is a slight need to overthink. He wants to use the same directory
for gadget-only builds too :-)

-- 
balbi

Re: [PATCH v4] usb: create usb_debug_root for gadget only

[Felipe Balbi]

Hi,

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> +struct dentry *usb_debugfs_init(void)
>> +{
>> +	if (!usb_debug_root)
>> +		usb_debug_root = debugfs_create_dir("usb", NULL);
>> +
>> +	atomic_inc(&usb_debug_root_refcnt);
>> +
>> +	return usb_debug_root;
>> +}
>> +EXPORT_SYMBOL_GPL(usb_debugfs_init);
>> +
>> +void usb_debugfs_cleanup(void)
>> +{
>> +	if (atomic_dec_and_test(&usb_debug_root_refcnt)) {
>> +		debugfs_remove_recursive(usb_debug_root);
>> +		usb_debug_root = NULL;
>> +	}
>> +}
>> +EXPORT_SYMBOL_GPL(usb_debugfs_cleanup);
>
> Only remove the debugfs subdir if the usbcore module is removed.  Create
> the debugfs subdir when the usbcore module is loaded.  No need for any
> reference counting of any sort at all.  No need to overthink this :)

There is a slight need to overthink. He wants to use the same directory
for gadget-only builds too :-)

-- 
balbi

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

Re: [PATCH v4] usb: create usb_debug_root for gadget only

[Greg Kroah-Hartman]

On Tue, Jun 04, 2019 at 10:47:55AM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >> +struct dentry *usb_debugfs_init(void)
> >> +{
> >> +	if (!usb_debug_root)
> >> +		usb_debug_root = debugfs_create_dir("usb", NULL);
> >> +
> >> +	atomic_inc(&usb_debug_root_refcnt);
> >> +
> >> +	return usb_debug_root;
> >> +}
> >> +EXPORT_SYMBOL_GPL(usb_debugfs_init);
> >> +
> >> +void usb_debugfs_cleanup(void)
> >> +{
> >> +	if (atomic_dec_and_test(&usb_debug_root_refcnt)) {
> >> +		debugfs_remove_recursive(usb_debug_root);
> >> +		usb_debug_root = NULL;
> >> +	}
> >> +}
> >> +EXPORT_SYMBOL_GPL(usb_debugfs_cleanup);
> >
> > Only remove the debugfs subdir if the usbcore module is removed.  Create
> > the debugfs subdir when the usbcore module is loaded.  No need for any
> > reference counting of any sort at all.  No need to overthink this :)
> 
> There is a slight need to overthink. He wants to use the same directory
> for gadget-only builds too :-)

Again, that's fine, this file will be loaded for those builds as well,
right?  Otherwise, how would this code even be present?  :)

thanks,

greg k-h

Re: [PATCH v4] usb: create usb_debug_root for gadget only

[Greg Kroah-Hartman]

On Tue, Jun 04, 2019 at 10:47:55AM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >> +struct dentry *usb_debugfs_init(void)
> >> +{
> >> +	if (!usb_debug_root)
> >> +		usb_debug_root = debugfs_create_dir("usb", NULL);
> >> +
> >> +	atomic_inc(&usb_debug_root_refcnt);
> >> +
> >> +	return usb_debug_root;
> >> +}
> >> +EXPORT_SYMBOL_GPL(usb_debugfs_init);
> >> +
> >> +void usb_debugfs_cleanup(void)
> >> +{
> >> +	if (atomic_dec_and_test(&usb_debug_root_refcnt)) {
> >> +		debugfs_remove_recursive(usb_debug_root);
> >> +		usb_debug_root = NULL;
> >> +	}
> >> +}
> >> +EXPORT_SYMBOL_GPL(usb_debugfs_cleanup);
> >
> > Only remove the debugfs subdir if the usbcore module is removed.  Create
> > the debugfs subdir when the usbcore module is loaded.  No need for any
> > reference counting of any sort at all.  No need to overthink this :)
> 
> There is a slight need to overthink. He wants to use the same directory
> for gadget-only builds too :-)

Again, that's fine, this file will be loaded for those builds as well,
right?  Otherwise, how would this code even be present?  :)

thanks,

greg k-h

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

Re: [PATCH v4] usb: create usb_debug_root for gadget only

[Chunfeng Yun]

On Tue, 2019-06-04 at 09:37 +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 04, 2019 at 03:34:07PM +0800, Chunfeng Yun wrote:
> > When CONFIG_USB is not set, and CONFIG_USB_GADGET is set,
> > there is an issue, e.g.:
> > 
> > drivers/usb/mtu3/mtu3_debugfs.o: in function 'ssusb_debugfs_create_root':
> > mtu3_debugfs.c:(.text+0xba3): undefined reference to 'usb_debug_root'
> > 
> > usb_debug_root is currently only built when host is supported
> > (CONFIG_USB is set), for convenience, we also want it created when
> > gadget only is enabled, this patch try to support it.
> > 
> > Reported-by: Randy Dunlap <rdunlap@infradead.org>
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> > v4:
> >   move common API into common.c suggested by Felipe
> > 
> > v3:
> >   1. still create usb_debug_root for gadget only
> >   2. abandon mtu3's change
> >   3. drop acked-by Randy
> > 
> > v2(resend): add acked-by Randy
> > 
> > v1: fix mtu3's build error, replace usb_debug_root by NULL;
> > ---
> >  drivers/usb/common/common.c   | 26 ++++++++++++++++++++++++++
> >  drivers/usb/core/usb.c        | 16 ++++------------
> >  drivers/usb/gadget/udc/core.c |  3 +++
> >  include/linux/usb.h           |  2 ++
> >  4 files changed, 35 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > index 18f5dcf58b0d..c52e9c9f58ec 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",
> > @@ -139,6 +140,31 @@ enum usb_dr_mode usb_get_dr_mode(struct device *dev)
> >  }
> >  EXPORT_SYMBOL_GPL(usb_get_dr_mode);
> >  
> > +struct dentry *usb_debug_root;
> > +EXPORT_SYMBOL_GPL(usb_debug_root);
> > +
> > +static atomic_t usb_debug_root_refcnt = ATOMIC_INIT(0);
> 
> Ick, no.
> 
> > +
> > +struct dentry *usb_debugfs_init(void)
> > +{
> > +	if (!usb_debug_root)
> > +		usb_debug_root = debugfs_create_dir("usb", NULL);
> > +
> > +	atomic_inc(&usb_debug_root_refcnt);
> > +
> > +	return usb_debug_root;
> > +}
> > +EXPORT_SYMBOL_GPL(usb_debugfs_init);
> > +
> > +void usb_debugfs_cleanup(void)
> > +{
> > +	if (atomic_dec_and_test(&usb_debug_root_refcnt)) {
> > +		debugfs_remove_recursive(usb_debug_root);
> > +		usb_debug_root = NULL;
> > +	}
> > +}
> > +EXPORT_SYMBOL_GPL(usb_debugfs_cleanup);
> 
> Only remove the debugfs subdir if the usbcore module is removed. 
Both usbcore module and gadget module will use this "usb" subdir now.
Gadget module may still use it when remove usbcore module.

>  Create
> the debugfs subdir when the usbcore module is loaded.  No need for any
> reference counting of any sort at all.  No need to overthink this :)
> 
> thanks,
> 
> greg k-h

Re: [PATCH v4] usb: create usb_debug_root for gadget only

[Chunfeng Yun]

On Tue, 2019-06-04 at 09:37 +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 04, 2019 at 03:34:07PM +0800, Chunfeng Yun wrote:
> > When CONFIG_USB is not set, and CONFIG_USB_GADGET is set,
> > there is an issue, e.g.:
> > 
> > drivers/usb/mtu3/mtu3_debugfs.o: in function 'ssusb_debugfs_create_root':
> > mtu3_debugfs.c:(.text+0xba3): undefined reference to 'usb_debug_root'
> > 
> > usb_debug_root is currently only built when host is supported
> > (CONFIG_USB is set), for convenience, we also want it created when
> > gadget only is enabled, this patch try to support it.
> > 
> > Reported-by: Randy Dunlap <rdunlap@infradead.org>
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> > v4:
> >   move common API into common.c suggested by Felipe
> > 
> > v3:
> >   1. still create usb_debug_root for gadget only
> >   2. abandon mtu3's change
> >   3. drop acked-by Randy
> > 
> > v2(resend): add acked-by Randy
> > 
> > v1: fix mtu3's build error, replace usb_debug_root by NULL;
> > ---
> >  drivers/usb/common/common.c   | 26 ++++++++++++++++++++++++++
> >  drivers/usb/core/usb.c        | 16 ++++------------
> >  drivers/usb/gadget/udc/core.c |  3 +++
> >  include/linux/usb.h           |  2 ++
> >  4 files changed, 35 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > index 18f5dcf58b0d..c52e9c9f58ec 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",
> > @@ -139,6 +140,31 @@ enum usb_dr_mode usb_get_dr_mode(struct device *dev)
> >  }
> >  EXPORT_SYMBOL_GPL(usb_get_dr_mode);
> >  
> > +struct dentry *usb_debug_root;
> > +EXPORT_SYMBOL_GPL(usb_debug_root);
> > +
> > +static atomic_t usb_debug_root_refcnt = ATOMIC_INIT(0);
> 
> Ick, no.
> 
> > +
> > +struct dentry *usb_debugfs_init(void)
> > +{
> > +	if (!usb_debug_root)
> > +		usb_debug_root = debugfs_create_dir("usb", NULL);
> > +
> > +	atomic_inc(&usb_debug_root_refcnt);
> > +
> > +	return usb_debug_root;
> > +}
> > +EXPORT_SYMBOL_GPL(usb_debugfs_init);
> > +
> > +void usb_debugfs_cleanup(void)
> > +{
> > +	if (atomic_dec_and_test(&usb_debug_root_refcnt)) {
> > +		debugfs_remove_recursive(usb_debug_root);
> > +		usb_debug_root = NULL;
> > +	}
> > +}
> > +EXPORT_SYMBOL_GPL(usb_debugfs_cleanup);
> 
> Only remove the debugfs subdir if the usbcore module is removed. 
Both usbcore module and gadget module will use this "usb" subdir now.
Gadget module may still use it when remove usbcore module.

>  Create
> the debugfs subdir when the usbcore module is loaded.  No need for any
> reference counting of any sort at all.  No need to overthink this :)
> 
> thanks,
> 
> greg k-h

Re: [PATCH v4] usb: create usb_debug_root for gadget only

[Chunfeng Yun]

On Tue, 2019-06-04 at 09:37 +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 04, 2019 at 03:34:07PM +0800, Chunfeng Yun wrote:
> > When CONFIG_USB is not set, and CONFIG_USB_GADGET is set,
> > there is an issue, e.g.:
> > 
> > drivers/usb/mtu3/mtu3_debugfs.o: in function 'ssusb_debugfs_create_root':
> > mtu3_debugfs.c:(.text+0xba3): undefined reference to 'usb_debug_root'
> > 
> > usb_debug_root is currently only built when host is supported
> > (CONFIG_USB is set), for convenience, we also want it created when
> > gadget only is enabled, this patch try to support it.
> > 
> > Reported-by: Randy Dunlap <rdunlap@infradead.org>
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> > v4:
> >   move common API into common.c suggested by Felipe
> > 
> > v3:
> >   1. still create usb_debug_root for gadget only
> >   2. abandon mtu3's change
> >   3. drop acked-by Randy
> > 
> > v2(resend): add acked-by Randy
> > 
> > v1: fix mtu3's build error, replace usb_debug_root by NULL;
> > ---
> >  drivers/usb/common/common.c   | 26 ++++++++++++++++++++++++++
> >  drivers/usb/core/usb.c        | 16 ++++------------
> >  drivers/usb/gadget/udc/core.c |  3 +++
> >  include/linux/usb.h           |  2 ++
> >  4 files changed, 35 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > index 18f5dcf58b0d..c52e9c9f58ec 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",
> > @@ -139,6 +140,31 @@ enum usb_dr_mode usb_get_dr_mode(struct device *dev)
> >  }
> >  EXPORT_SYMBOL_GPL(usb_get_dr_mode);
> >  
> > +struct dentry *usb_debug_root;
> > +EXPORT_SYMBOL_GPL(usb_debug_root);
> > +
> > +static atomic_t usb_debug_root_refcnt = ATOMIC_INIT(0);
> 
> Ick, no.
> 
> > +
> > +struct dentry *usb_debugfs_init(void)
> > +{
> > +	if (!usb_debug_root)
> > +		usb_debug_root = debugfs_create_dir("usb", NULL);
> > +
> > +	atomic_inc(&usb_debug_root_refcnt);
> > +
> > +	return usb_debug_root;
> > +}
> > +EXPORT_SYMBOL_GPL(usb_debugfs_init);
> > +
> > +void usb_debugfs_cleanup(void)
> > +{
> > +	if (atomic_dec_and_test(&usb_debug_root_refcnt)) {
> > +		debugfs_remove_recursive(usb_debug_root);
> > +		usb_debug_root = NULL;
> > +	}
> > +}
> > +EXPORT_SYMBOL_GPL(usb_debugfs_cleanup);
> 
> Only remove the debugfs subdir if the usbcore module is removed. 
Both usbcore module and gadget module will use this "usb" subdir now.
Gadget module may still use it when remove usbcore module.

>  Create
> the debugfs subdir when the usbcore module is loaded.  No need for any
> reference counting of any sort at all.  No need to overthink 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

Re: [PATCH v4] usb: create usb_debug_root for gadget only

[Chunfeng Yun]

On Tue, 2019-06-04 at 10:24 +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 04, 2019 at 10:47:55AM +0300, Felipe Balbi wrote:
> > 
> > Hi,
> > 
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> > >> +struct dentry *usb_debugfs_init(void)
> > >> +{
> > >> +	if (!usb_debug_root)
> > >> +		usb_debug_root = debugfs_create_dir("usb", NULL);
> > >> +
> > >> +	atomic_inc(&usb_debug_root_refcnt);
> > >> +
> > >> +	return usb_debug_root;
> > >> +}
> > >> +EXPORT_SYMBOL_GPL(usb_debugfs_init);
> > >> +
> > >> +void usb_debugfs_cleanup(void)
> > >> +{
> > >> +	if (atomic_dec_and_test(&usb_debug_root_refcnt)) {
> > >> +		debugfs_remove_recursive(usb_debug_root);
> > >> +		usb_debug_root = NULL;
> > >> +	}
> > >> +}
> > >> +EXPORT_SYMBOL_GPL(usb_debugfs_cleanup);
> > >
> > > Only remove the debugfs subdir if the usbcore module is removed.  Create
> > > the debugfs subdir when the usbcore module is loaded.  No need for any
> > > reference counting of any sort at all.  No need to overthink this :)
> > 
> > There is a slight need to overthink. He wants to use the same directory
> > for gadget-only builds too :-)
> 
> Again, that's fine, this file will be loaded for those builds as well,
> right?  
Yes, either usbcore or gadget will select this file.

> Otherwise, how would this code even be present?  :)
> 
> thanks,
> 
> greg k-h

Re: [PATCH v4] usb: create usb_debug_root for gadget only

[Chunfeng Yun]

On Tue, 2019-06-04 at 10:24 +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 04, 2019 at 10:47:55AM +0300, Felipe Balbi wrote:
> > 
> > Hi,
> > 
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> > >> +struct dentry *usb_debugfs_init(void)
> > >> +{
> > >> +	if (!usb_debug_root)
> > >> +		usb_debug_root = debugfs_create_dir("usb", NULL);
> > >> +
> > >> +	atomic_inc(&usb_debug_root_refcnt);
> > >> +
> > >> +	return usb_debug_root;
> > >> +}
> > >> +EXPORT_SYMBOL_GPL(usb_debugfs_init);
> > >> +
> > >> +void usb_debugfs_cleanup(void)
> > >> +{
> > >> +	if (atomic_dec_and_test(&usb_debug_root_refcnt)) {
> > >> +		debugfs_remove_recursive(usb_debug_root);
> > >> +		usb_debug_root = NULL;
> > >> +	}
> > >> +}
> > >> +EXPORT_SYMBOL_GPL(usb_debugfs_cleanup);
> > >
> > > Only remove the debugfs subdir if the usbcore module is removed.  Create
> > > the debugfs subdir when the usbcore module is loaded.  No need for any
> > > reference counting of any sort at all.  No need to overthink this :)
> > 
> > There is a slight need to overthink. He wants to use the same directory
> > for gadget-only builds too :-)
> 
> Again, that's fine, this file will be loaded for those builds as well,
> right?  
Yes, either usbcore or gadget will select this file.

> Otherwise, how would this code even be present?  :)
> 
> thanks,
> 
> greg k-h

Re: [PATCH v4] usb: create usb_debug_root for gadget only

[Chunfeng Yun]

On Tue, 2019-06-04 at 10:24 +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 04, 2019 at 10:47:55AM +0300, Felipe Balbi wrote:
> > 
> > Hi,
> > 
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> > >> +struct dentry *usb_debugfs_init(void)
> > >> +{
> > >> +	if (!usb_debug_root)
> > >> +		usb_debug_root = debugfs_create_dir("usb", NULL);
> > >> +
> > >> +	atomic_inc(&usb_debug_root_refcnt);
> > >> +
> > >> +	return usb_debug_root;
> > >> +}
> > >> +EXPORT_SYMBOL_GPL(usb_debugfs_init);
> > >> +
> > >> +void usb_debugfs_cleanup(void)
> > >> +{
> > >> +	if (atomic_dec_and_test(&usb_debug_root_refcnt)) {
> > >> +		debugfs_remove_recursive(usb_debug_root);
> > >> +		usb_debug_root = NULL;
> > >> +	}
> > >> +}
> > >> +EXPORT_SYMBOL_GPL(usb_debugfs_cleanup);
> > >
> > > Only remove the debugfs subdir if the usbcore module is removed.  Create
> > > the debugfs subdir when the usbcore module is loaded.  No need for any
> > > reference counting of any sort at all.  No need to overthink this :)
> > 
> > There is a slight need to overthink. He wants to use the same directory
> > for gadget-only builds too :-)
> 
> Again, that's fine, this file will be loaded for those builds as well,
> right?  
Yes, either usbcore or gadget will select this file.

> Otherwise, how would this code even be present?  :)
> 
> thanks,
> 
> greg k-h



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

Re: [PATCH v4] usb: create usb_debug_root for gadget only

[Greg Kroah-Hartman]

On Tue, Jun 04, 2019 at 04:59:14PM +0800, Chunfeng Yun wrote:
> On Tue, 2019-06-04 at 09:37 +0200, Greg Kroah-Hartman wrote:
> > On Tue, Jun 04, 2019 at 03:34:07PM +0800, Chunfeng Yun wrote:
> > > When CONFIG_USB is not set, and CONFIG_USB_GADGET is set,
> > > there is an issue, e.g.:
> > > 
> > > drivers/usb/mtu3/mtu3_debugfs.o: in function 'ssusb_debugfs_create_root':
> > > mtu3_debugfs.c:(.text+0xba3): undefined reference to 'usb_debug_root'
> > > 
> > > usb_debug_root is currently only built when host is supported
> > > (CONFIG_USB is set), for convenience, we also want it created when
> > > gadget only is enabled, this patch try to support it.
> > > 
> > > Reported-by: Randy Dunlap <rdunlap@infradead.org>
> > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > ---
> > > v4:
> > >   move common API into common.c suggested by Felipe
> > > 
> > > v3:
> > >   1. still create usb_debug_root for gadget only
> > >   2. abandon mtu3's change
> > >   3. drop acked-by Randy
> > > 
> > > v2(resend): add acked-by Randy
> > > 
> > > v1: fix mtu3's build error, replace usb_debug_root by NULL;
> > > ---
> > >  drivers/usb/common/common.c   | 26 ++++++++++++++++++++++++++
> > >  drivers/usb/core/usb.c        | 16 ++++------------
> > >  drivers/usb/gadget/udc/core.c |  3 +++
> > >  include/linux/usb.h           |  2 ++
> > >  4 files changed, 35 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > > index 18f5dcf58b0d..c52e9c9f58ec 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",
> > > @@ -139,6 +140,31 @@ enum usb_dr_mode usb_get_dr_mode(struct device *dev)
> > >  }
> > >  EXPORT_SYMBOL_GPL(usb_get_dr_mode);
> > >  
> > > +struct dentry *usb_debug_root;
> > > +EXPORT_SYMBOL_GPL(usb_debug_root);
> > > +
> > > +static atomic_t usb_debug_root_refcnt = ATOMIC_INIT(0);
> > 
> > Ick, no.
> > 
> > > +
> > > +struct dentry *usb_debugfs_init(void)
> > > +{
> > > +	if (!usb_debug_root)
> > > +		usb_debug_root = debugfs_create_dir("usb", NULL);
> > > +
> > > +	atomic_inc(&usb_debug_root_refcnt);
> > > +
> > > +	return usb_debug_root;
> > > +}
> > > +EXPORT_SYMBOL_GPL(usb_debugfs_init);
> > > +
> > > +void usb_debugfs_cleanup(void)
> > > +{
> > > +	if (atomic_dec_and_test(&usb_debug_root_refcnt)) {
> > > +		debugfs_remove_recursive(usb_debug_root);
> > > +		usb_debug_root = NULL;
> > > +	}
> > > +}
> > > +EXPORT_SYMBOL_GPL(usb_debugfs_cleanup);
> > 
> > Only remove the debugfs subdir if the usbcore module is removed. 
> Both usbcore module and gadget module will use this "usb" subdir now.
> Gadget module may still use it when remove usbcore module.

Did you try to remove the usb-common module with the udc gadget module
remaining loaded with this change in place?  I think you will find that
it is impossible given that the udc gadget module now depends on a
symbol in the usb-common code.

So again, just use the module reference counting logic to drive this
functionality, don't create an atomic variable that duplicates the
logic we already have in place today.

thanks,

greg k-h

Re: [PATCH v4] usb: create usb_debug_root for gadget only

[Greg Kroah-Hartman]

On Tue, Jun 04, 2019 at 04:59:14PM +0800, Chunfeng Yun wrote:
> On Tue, 2019-06-04 at 09:37 +0200, Greg Kroah-Hartman wrote:
> > On Tue, Jun 04, 2019 at 03:34:07PM +0800, Chunfeng Yun wrote:
> > > When CONFIG_USB is not set, and CONFIG_USB_GADGET is set,
> > > there is an issue, e.g.:
> > > 
> > > drivers/usb/mtu3/mtu3_debugfs.o: in function 'ssusb_debugfs_create_root':
> > > mtu3_debugfs.c:(.text+0xba3): undefined reference to 'usb_debug_root'
> > > 
> > > usb_debug_root is currently only built when host is supported
> > > (CONFIG_USB is set), for convenience, we also want it created when
> > > gadget only is enabled, this patch try to support it.
> > > 
> > > Reported-by: Randy Dunlap <rdunlap@infradead.org>
> > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > ---
> > > v4:
> > >   move common API into common.c suggested by Felipe
> > > 
> > > v3:
> > >   1. still create usb_debug_root for gadget only
> > >   2. abandon mtu3's change
> > >   3. drop acked-by Randy
> > > 
> > > v2(resend): add acked-by Randy
> > > 
> > > v1: fix mtu3's build error, replace usb_debug_root by NULL;
> > > ---
> > >  drivers/usb/common/common.c   | 26 ++++++++++++++++++++++++++
> > >  drivers/usb/core/usb.c        | 16 ++++------------
> > >  drivers/usb/gadget/udc/core.c |  3 +++
> > >  include/linux/usb.h           |  2 ++
> > >  4 files changed, 35 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > > index 18f5dcf58b0d..c52e9c9f58ec 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",
> > > @@ -139,6 +140,31 @@ enum usb_dr_mode usb_get_dr_mode(struct device *dev)
> > >  }
> > >  EXPORT_SYMBOL_GPL(usb_get_dr_mode);
> > >  
> > > +struct dentry *usb_debug_root;
> > > +EXPORT_SYMBOL_GPL(usb_debug_root);
> > > +
> > > +static atomic_t usb_debug_root_refcnt = ATOMIC_INIT(0);
> > 
> > Ick, no.
> > 
> > > +
> > > +struct dentry *usb_debugfs_init(void)
> > > +{
> > > +	if (!usb_debug_root)
> > > +		usb_debug_root = debugfs_create_dir("usb", NULL);
> > > +
> > > +	atomic_inc(&usb_debug_root_refcnt);
> > > +
> > > +	return usb_debug_root;
> > > +}
> > > +EXPORT_SYMBOL_GPL(usb_debugfs_init);
> > > +
> > > +void usb_debugfs_cleanup(void)
> > > +{
> > > +	if (atomic_dec_and_test(&usb_debug_root_refcnt)) {
> > > +		debugfs_remove_recursive(usb_debug_root);
> > > +		usb_debug_root = NULL;
> > > +	}
> > > +}
> > > +EXPORT_SYMBOL_GPL(usb_debugfs_cleanup);
> > 
> > Only remove the debugfs subdir if the usbcore module is removed. 
> Both usbcore module and gadget module will use this "usb" subdir now.
> Gadget module may still use it when remove usbcore module.

Did you try to remove the usb-common module with the udc gadget module
remaining loaded with this change in place?  I think you will find that
it is impossible given that the udc gadget module now depends on a
symbol in the usb-common code.

So again, just use the module reference counting logic to drive this
functionality, don't create an atomic variable that duplicates the
logic we already have in place today.

thanks,

greg k-h

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

Re: [PATCH v4] usb: create usb_debug_root for gadget only

[Greg Kroah-Hartman]

On Tue, Jun 04, 2019 at 10:24:07AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 04, 2019 at 10:47:55AM +0300, Felipe Balbi wrote:
> > 
> > Hi,
> > 
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> > >> +struct dentry *usb_debugfs_init(void)
> > >> +{
> > >> +	if (!usb_debug_root)
> > >> +		usb_debug_root = debugfs_create_dir("usb", NULL);
> > >> +
> > >> +	atomic_inc(&usb_debug_root_refcnt);
> > >> +
> > >> +	return usb_debug_root;
> > >> +}
> > >> +EXPORT_SYMBOL_GPL(usb_debugfs_init);
> > >> +
> > >> +void usb_debugfs_cleanup(void)
> > >> +{
> > >> +	if (atomic_dec_and_test(&usb_debug_root_refcnt)) {
> > >> +		debugfs_remove_recursive(usb_debug_root);
> > >> +		usb_debug_root = NULL;
> > >> +	}
> > >> +}
> > >> +EXPORT_SYMBOL_GPL(usb_debugfs_cleanup);
> > >
> > > Only remove the debugfs subdir if the usbcore module is removed.  Create
> > > the debugfs subdir when the usbcore module is loaded.  No need for any
> > > reference counting of any sort at all.  No need to overthink this :)
> > 
> > There is a slight need to overthink. He wants to use the same directory
> > for gadget-only builds too :-)
> 
> Again, that's fine, this file will be loaded for those builds as well,
> right?  Otherwise, how would this code even be present?  :)


As it seems to be easier to just write the patch instead of trying to
describe it in email, and the patch is even simpler than the text,
here's what I was thinking of:

Note, it's not fully correct, now that I think of it, but you get the
idea...


From foo@baz Tue 04 Jun 2019 11:25:30 AM CEST
Date: Tue, 04 Jun 2019 11:25:30 +0200
To: Greg KH <gregkh@linuxfoundation.org>
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: [PATCH] USB: move usb debugfs directory creation to the usb core

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>


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..2aa28445277d 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -1185,12 +1185,8 @@ static struct notifier_block usb_bus_nb = {
 	.notifier_call = usb_bus_notify,
 };
 
-struct dentry *usb_debug_root;
-EXPORT_SYMBOL_GPL(usb_debug_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);
 }

Re: [PATCH v4] usb: create usb_debug_root for gadget only

[Greg Kroah-Hartman]

On Tue, Jun 04, 2019 at 10:24:07AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 04, 2019 at 10:47:55AM +0300, Felipe Balbi wrote:
> > 
> > Hi,
> > 
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> > >> +struct dentry *usb_debugfs_init(void)
> > >> +{
> > >> +	if (!usb_debug_root)
> > >> +		usb_debug_root = debugfs_create_dir("usb", NULL);
> > >> +
> > >> +	atomic_inc(&usb_debug_root_refcnt);
> > >> +
> > >> +	return usb_debug_root;
> > >> +}
> > >> +EXPORT_SYMBOL_GPL(usb_debugfs_init);
> > >> +
> > >> +void usb_debugfs_cleanup(void)
> > >> +{
> > >> +	if (atomic_dec_and_test(&usb_debug_root_refcnt)) {
> > >> +		debugfs_remove_recursive(usb_debug_root);
> > >> +		usb_debug_root = NULL;
> > >> +	}
> > >> +}
> > >> +EXPORT_SYMBOL_GPL(usb_debugfs_cleanup);
> > >
> > > Only remove the debugfs subdir if the usbcore module is removed.  Create
> > > the debugfs subdir when the usbcore module is loaded.  No need for any
> > > reference counting of any sort at all.  No need to overthink this :)
> > 
> > There is a slight need to overthink. He wants to use the same directory
> > for gadget-only builds too :-)
> 
> Again, that's fine, this file will be loaded for those builds as well,
> right?  Otherwise, how would this code even be present?  :)


As it seems to be easier to just write the patch instead of trying to
describe it in email, and the patch is even simpler than the text,
here's what I was thinking of:

Note, it's not fully correct, now that I think of it, but you get the
idea...


>From foo@baz Tue 04 Jun 2019 11:25:30 AM CEST
Date: Tue, 04 Jun 2019 11:25:30 +0200
To: Greg KH <gregkh@linuxfoundation.org>
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: [PATCH] USB: move usb debugfs directory creation to the usb core

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>


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..2aa28445277d 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -1185,12 +1185,8 @@ static struct notifier_block usb_bus_nb = {
 	.notifier_call = usb_bus_notify,
 };
 
-struct dentry *usb_debug_root;
-EXPORT_SYMBOL_GPL(usb_debug_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);
 }

Re: [PATCH v4] usb: create usb_debug_root for gadget only

[Greg Kroah-Hartman]

On Tue, Jun 04, 2019 at 10:24:07AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 04, 2019 at 10:47:55AM +0300, Felipe Balbi wrote:
> > 
> > Hi,
> > 
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> > >> +struct dentry *usb_debugfs_init(void)
> > >> +{
> > >> +	if (!usb_debug_root)
> > >> +		usb_debug_root = debugfs_create_dir("usb", NULL);
> > >> +
> > >> +	atomic_inc(&usb_debug_root_refcnt);
> > >> +
> > >> +	return usb_debug_root;
> > >> +}
> > >> +EXPORT_SYMBOL_GPL(usb_debugfs_init);
> > >> +
> > >> +void usb_debugfs_cleanup(void)
> > >> +{
> > >> +	if (atomic_dec_and_test(&usb_debug_root_refcnt)) {
> > >> +		debugfs_remove_recursive(usb_debug_root);
> > >> +		usb_debug_root = NULL;
> > >> +	}
> > >> +}
> > >> +EXPORT_SYMBOL_GPL(usb_debugfs_cleanup);
> > >
> > > Only remove the debugfs subdir if the usbcore module is removed.  Create
> > > the debugfs subdir when the usbcore module is loaded.  No need for any
> > > reference counting of any sort at all.  No need to overthink this :)
> > 
> > There is a slight need to overthink. He wants to use the same directory
> > for gadget-only builds too :-)
> 
> Again, that's fine, this file will be loaded for those builds as well,
> right?  Otherwise, how would this code even be present?  :)


As it seems to be easier to just write the patch instead of trying to
describe it in email, and the patch is even simpler than the text,
here's what I was thinking of:

Note, it's not fully correct, now that I think of it, but you get the
idea...


From foo@baz Tue 04 Jun 2019 11:25:30 AM CEST
Date: Tue, 04 Jun 2019 11:25:30 +0200
To: Greg KH <gregkh@linuxfoundation.org>
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: [PATCH] USB: move usb debugfs directory creation to the usb core

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>


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..2aa28445277d 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -1185,12 +1185,8 @@ static struct notifier_block usb_bus_nb = {
 	.notifier_call = usb_bus_notify,
 };
 
-struct dentry *usb_debug_root;
-EXPORT_SYMBOL_GPL(usb_debug_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);
 }

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