[PATCH 01/28] usb: common: change usb_debug_root as static variable

[PATCH 01/28] usb: common: change usb_debug_root as static variable

[Chunfeng Yun]

Try to avoid using extern global variable, and provide two
functions for the usage cases

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/common/common.c | 16 ++++++++++++++--
 include/linux/usb.h         |  5 ++++-
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index 1433260d99b4..639ee6d243a2 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -293,8 +293,20 @@ 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 struct dentry *usb_debug_root;
+
+struct dentry *usb_debugfs_create_dir(const char *name)
+{
+	return debugfs_create_dir(name, usb_debug_root);
+}
+EXPORT_SYMBOL_GPL(usb_debugfs_create_dir);
+
+struct dentry *usb_debugfs_create_file(const char *name, umode_t mode,
+			void *data, const struct file_operations *fops)
+{
+	return debugfs_create_file(name, mode, usb_debug_root, data, fops);
+}
+EXPORT_SYMBOL_GPL(usb_debugfs_create_file);
 
 static int __init usb_common_init(void)
 {
diff --git a/include/linux/usb.h b/include/linux/usb.h
index e656e7b4b1e4..ad96e0aa0127 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -2001,7 +2001,10 @@ extern void usb_register_notify(struct notifier_block *nb);
 extern void usb_unregister_notify(struct notifier_block *nb);
 
 /* debugfs stuff */
-extern struct dentry *usb_debug_root;
+extern struct dentry *usb_debugfs_create_dir(const char *name);
+extern struct dentry *
+usb_debugfs_create_file(const char *name, umode_t mode, void *data,
+			const struct file_operations *fops);
 
 /* LED triggers */
 enum usb_led_event {
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 02/28] usb: core: use usb_debugfs_create_file() to create debugfs file

[Chunfeng Yun]

Use usb_debugfs_create_file() to create debugfs file under usb
root

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/core/usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index f16c26dc079d..8053af9e4218 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -967,7 +967,7 @@ static struct dentry *usb_devices_root;
 
 static void usb_debugfs_init(void)
 {
-	usb_devices_root = debugfs_create_file("devices", 0444, usb_debug_root,
+	usb_devices_root = usb_debugfs_create_file("devices", 0444,
 					       NULL, &usbfs_devices_fops);
 }
 
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 03/28] usb: host: sl811: use usb_debugfs_create_file() to create file

[Chunfeng Yun]

The usb_debug_root is now a file-scope static variable, use
usb_debugfs_create_file() to create file under usb root

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/sl811-hcd.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/sl811-hcd.c b/drivers/usb/host/sl811-hcd.c
index 72a34a1eb618..8669cd3cd6f9 100644
--- a/drivers/usb/host/sl811-hcd.c
+++ b/drivers/usb/host/sl811-hcd.c
@@ -1496,9 +1496,8 @@ DEFINE_SHOW_ATTRIBUTE(sl811h_debug);
 /* expect just one sl811 per system */
 static void create_debug_file(struct sl811 *sl811)
 {
-	sl811->debug_file = debugfs_create_file("sl811h", S_IRUGO,
-						usb_debug_root, sl811,
-						&sl811h_debug_fops);
+	sl811->debug_file = usb_debugfs_create_file("sl811h", S_IRUGO,
+						sl811, &sl811h_debug_fops);
 }
 
 static void remove_debug_file(struct sl811 *sl811)
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 04/28] usb: host: isp1362: use usb_debugfs_create_file() to create file

[Chunfeng Yun]

The usb_debug_root is now a file-scope static variable, use
usb_debugfs_create_file() to create file under usb root

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/isp1362-hcd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/host/isp1362-hcd.c b/drivers/usb/host/isp1362-hcd.c
index 96f8daa11f25..6eb968dd9974 100644
--- a/drivers/usb/host/isp1362-hcd.c
+++ b/drivers/usb/host/isp1362-hcd.c
@@ -2164,8 +2164,7 @@ DEFINE_SHOW_ATTRIBUTE(isp1362);
 /* expect just one isp1362_hcd per system */
 static void create_debug_file(struct isp1362_hcd *isp1362_hcd)
 {
-	isp1362_hcd->debug_file = debugfs_create_file("isp1362", S_IRUGO,
-						      usb_debug_root,
+	isp1362_hcd->debug_file = usb_debugfs_create_file("isp1362", S_IRUGO,
 						      isp1362_hcd,
 						      &isp1362_fops);
 }
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 05/28] usb: typec: tcpm: use usb_debugfs_create_file() to create debugfs file

[Chunfeng Yun]

Use usb_debugfs_create_file() to create debugfs file under usb
root directory

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 96562744101c..6ee6a1f55bc6 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -578,7 +578,7 @@ static void tcpm_debugfs_init(struct tcpm_port *port)
 
 	mutex_init(&port->logbuffer_lock);
 	snprintf(name, NAME_MAX, "tcpm-%s", dev_name(port->dev));
-	port->dentry = debugfs_create_file(name, S_IFREG | 0444, usb_debug_root,
+	port->dentry = usb_debugfs_create_file(name, S_IFREG | 0444,
 					   port, &tcpm_debug_fops);
 }
 
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 06/28] usb: typec: fusb302: use usb_debugfs_create_file() to create debugfs file

[Chunfeng Yun]

Use usb_debugfs_create_file() to create debugfs file under usb
root directory

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/typec/tcpm/fusb302.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
index ed8655c6af8c..beb1d6afd67d 100644
--- a/drivers/usb/typec/tcpm/fusb302.c
+++ b/drivers/usb/typec/tcpm/fusb302.c
@@ -213,7 +213,7 @@ static void fusb302_debugfs_init(struct fusb302_chip *chip)
 
 	mutex_init(&chip->logbuffer_lock);
 	snprintf(name, NAME_MAX, "fusb302-%s", dev_name(chip->dev));
-	chip->dentry = debugfs_create_file(name, S_IFREG | 0444, usb_debug_root,
+	chip->dentry = usb_debugfs_create_file(name, S_IFREG | 0444,
 					   chip, &fusb302_debug_fops);
 }
 
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 07/28] usb: ehci-hcd: use usb_debugfs_create_dir() to create directory

[Chunfeng Yun]

The usb_debug_root is now a file static variable, use
usb_debugfs_create_dir() to create "ehci" directory under
usb root

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/ehci-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index cf2b7ae93b7e..773ea1f3beec 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1306,7 +1306,7 @@ static int __init ehci_hcd_init(void)
 		 sizeof(struct ehci_itd), sizeof(struct ehci_sitd));
 
 #ifdef CONFIG_DYNAMIC_DEBUG
-	ehci_debug_root = debugfs_create_dir("ehci", usb_debug_root);
+	ehci_debug_root = usb_debugfs_create_dir("ehci");
 #endif
 
 #ifdef PLATFORM_DRIVER
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 08/28] usb: fhci-hcd: use usb_debugfs_create_dir() to create directory

[Chunfeng Yun]

The usb_debug_root is now a file-scope static variable, use
usb_debugfs_create_dir() to create directory under usb root

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/fhci-dbg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/fhci-dbg.c b/drivers/usb/host/fhci-dbg.c
index 100048b3bd17..0baa495c7da9 100644
--- a/drivers/usb/host/fhci-dbg.c
+++ b/drivers/usb/host/fhci-dbg.c
@@ -82,7 +82,7 @@ void fhci_dfs_create(struct fhci_hcd *fhci)
 {
 	struct device *dev = fhci_to_hcd(fhci)->self.controller;
 
-	fhci->dfs_root = debugfs_create_dir(dev_name(dev), usb_debug_root);
+	fhci->dfs_root = usb_debugfs_create_dir(dev_name(dev));
 
 	debugfs_create_file("regs", S_IFREG | S_IRUGO, fhci->dfs_root, fhci,
 			    &fhci_dfs_regs_fops);
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 09/28] usb: ohci-hcd: use usb_debugfs_create_dir() to create directory

[Chunfeng Yun]

The usb_debug_root is now a file-scope static variable, use
usb_debugfs_create_dir() to create directory under usb root

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/ohci-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 4de91653a2c7..6c034e9042b1 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -1279,7 +1279,7 @@ static int __init ohci_hcd_mod_init(void)
 		sizeof (struct ed), sizeof (struct td));
 	set_bit(USB_OHCI_LOADED, &usb_hcds_loaded);
 
-	ohci_debug_root = debugfs_create_dir("ohci", usb_debug_root);
+	ohci_debug_root = usb_debugfs_create_dir("ohci");
 
 #ifdef PS3_SYSTEM_BUS_DRIVER
 	retval = ps3_ohci_driver_register(&PS3_SYSTEM_BUS_DRIVER);
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 10/28] usb: uhci-hcd: use usb_debugfs_create_dir() to create directory

[Chunfeng Yun]

The usb_debug_root is now a file-scope static variable, use
usb_debugfs_create_dir() to create "uhci" directory under
usb root

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/uhci-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
index 03bc59755123..a9707f981072 100644
--- a/drivers/usb/host/uhci-hcd.c
+++ b/drivers/usb/host/uhci-hcd.c
@@ -877,7 +877,7 @@ static int __init uhci_hcd_init(void)
 	errbuf = kmalloc(ERRBUF_LEN, GFP_KERNEL);
 	if (!errbuf)
 		goto errbuf_failed;
-	uhci_debugfs_root = debugfs_create_dir("uhci", usb_debug_root);
+	uhci_debugfs_root = usb_debugfs_create_dir("uhci");
 #endif
 
 	uhci_up_cachep = kmem_cache_create("uhci_urb_priv",
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 11/28] usb: xhci: use usb_debugfs_create_dir() to create directory

[Chunfeng Yun]

The usb_debug_root is now a file-scope static variable, use
usb_debugfs_create_dir() to create "xhci" directory under
usb root

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/xhci-debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index 7ba6afc7ef23..932fdbd2804f 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -601,7 +601,7 @@ void xhci_debugfs_exit(struct xhci_hcd *xhci)
 
 void __init xhci_debugfs_create_root(void)
 {
-	xhci_debugfs_root = debugfs_create_dir("xhci", usb_debug_root);
+	xhci_debugfs_root = usb_debugfs_create_dir("xhci");
 }
 
 void __exit xhci_debugfs_remove_root(void)
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 12/28] usb: host: fotg2: use usb_debugfs_create_dir() to create directory

[Chunfeng Yun]

The usb_debug_root is now a file-scope static variable, use
usb_debugfs_create_dir() to create directory under usb root

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/fotg210-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
index 9e0c98d6bdb0..9c9030044b06 100644
--- a/drivers/usb/host/fotg210-hcd.c
+++ b/drivers/usb/host/fotg210-hcd.c
@@ -5709,7 +5709,7 @@ static int __init fotg210_hcd_init(void)
 			sizeof(struct fotg210_qtd),
 			sizeof(struct fotg210_itd));
 
-	fotg210_debug_root = debugfs_create_dir("fotg210", usb_debug_root);
+	fotg210_debug_root = usb_debugfs_create_dir("fotg210");
 
 	retval = platform_driver_register(&fotg210_hcd_driver);
 	if (retval < 0)
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 13/28] usb: mon: use usb_debugfs_create_dir() to create directory

[Chunfeng Yun]

The usb_debug_root is now a file-scope static variable, use
usb_debugfs_create_dir() to create directory under usb root

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/mon/mon_text.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/mon/mon_text.c b/drivers/usb/mon/mon_text.c
index bc5ecd5ff565..c34f952b70dc 100644
--- a/drivers/usb/mon/mon_text.c
+++ b/drivers/usb/mon/mon_text.c
@@ -763,7 +763,7 @@ static void mon_text_ctor(void *mem)
 
 int __init mon_text_init(void)
 {
-	mon_dir = debugfs_create_dir("usbmon", usb_debug_root);
+	mon_dir = usb_debugfs_create_dir("usbmon");
 	return 0;
 }
 
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 14/28] usb: mtu3: use usb_debugfs_create_dir() to create directory

[Chunfeng Yun]

The usb_debug_root is now a file-scope static variable, use
usb_debugfs_create_dir() to create directory under usb root

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/mtu3/mtu3_debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/mtu3/mtu3_debugfs.c b/drivers/usb/mtu3/mtu3_debugfs.c
index c96e5dab0a48..4195eb6f7a74 100644
--- a/drivers/usb/mtu3/mtu3_debugfs.c
+++ b/drivers/usb/mtu3/mtu3_debugfs.c
@@ -529,7 +529,7 @@ void ssusb_dr_debugfs_init(struct ssusb_mtk *ssusb)
 void ssusb_debugfs_create_root(struct ssusb_mtk *ssusb)
 {
 	ssusb->dbgfs_root =
-		debugfs_create_dir(dev_name(ssusb->dev), usb_debug_root);
+		usb_debugfs_create_dir(dev_name(ssusb->dev));
 }
 
 void ssusb_debugfs_remove_root(struct ssusb_mtk *ssusb)
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 15/28] media: uvcvideo: use usb_debugfs_create_dir() to create directory

[Chunfeng Yun]

The usb_debug_root is now a file-scope static variable, use
usb_debugfs_create_dir() to create directory under usb root

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/media/usb/uvc/uvc_debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/uvc/uvc_debugfs.c b/drivers/media/usb/uvc/uvc_debugfs.c
index d2b109959d82..0a5eb145379d 100644
--- a/drivers/media/usb/uvc/uvc_debugfs.c
+++ b/drivers/media/usb/uvc/uvc_debugfs.c
@@ -110,7 +110,7 @@ void uvc_debugfs_init(void)
 {
 	struct dentry *dir;
 
-	dir = debugfs_create_dir("uvcvideo", usb_debug_root);
+	dir = usb_debugfs_create_dir("uvcvideo");
 	if (IS_ERR_OR_NULL(dir)) {
 		uvc_printk(KERN_INFO, "Unable to create debugfs directory\n");
 		return;
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 16/28] media: siano: use usb_debugfs_create_dir() to create directory

[Chunfeng Yun]

The usb_debug_root is now a file-scope static variable, use
usb_debugfs_create_dir() to create directory under usb root

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/media/common/siano/smsdvb-debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/common/siano/smsdvb-debugfs.c b/drivers/media/common/siano/smsdvb-debugfs.c
index c95d4583498e..28c025e39a00 100644
--- a/drivers/media/common/siano/smsdvb-debugfs.c
+++ b/drivers/media/common/siano/smsdvb-debugfs.c
@@ -514,7 +514,7 @@ void smsdvb_debugfs_register(void)
 	 * node for sdio-based boards, but this may need some logic at sdio
 	 * subsystem.
 	 */
-	d = debugfs_create_dir("smsdvb", usb_debug_root);
+	d = usb_debugfs_create_dir("smsdvb");
 	if (IS_ERR_OR_NULL(d)) {
 		pr_err("Couldn't create sysfs node for smsdvb\n");
 		return;
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 16/28] media: siano: use usb_debugfs_create_dir() to create directory

[Greg Kroah-Hartman]

On Wed, Nov 06, 2019 at 11:15:17AM +0800, Chunfeng Yun wrote:
> The usb_debug_root is now a file-scope static variable, use
> usb_debugfs_create_dir() to create directory under usb root
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  drivers/media/common/siano/smsdvb-debugfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/common/siano/smsdvb-debugfs.c b/drivers/media/common/siano/smsdvb-debugfs.c
> index c95d4583498e..28c025e39a00 100644
> --- a/drivers/media/common/siano/smsdvb-debugfs.c
> +++ b/drivers/media/common/siano/smsdvb-debugfs.c
> @@ -514,7 +514,7 @@ void smsdvb_debugfs_register(void)
>  	 * node for sdio-based boards, but this may need some logic at sdio
>  	 * subsystem.
>  	 */
> -	d = debugfs_create_dir("smsdvb", usb_debug_root);
> +	d = usb_debugfs_create_dir("smsdvb");
>  	if (IS_ERR_OR_NULL(d)) {

This check is not needed.

>  		pr_err("Couldn't create sysfs node for smsdvb\n");

This error is not correct no matter what you did here, so it needs to be
removed too.

thanks,

greg k-h

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 16/28] media: siano: use usb_debugfs_create_dir() to create directory

[Greg Kroah-Hartman]

On Wed, Nov 06, 2019 at 11:15:17AM +0800, Chunfeng Yun wrote:
> The usb_debug_root is now a file-scope static variable, use
> usb_debugfs_create_dir() to create directory under usb root
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  drivers/media/common/siano/smsdvb-debugfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/common/siano/smsdvb-debugfs.c b/drivers/media/common/siano/smsdvb-debugfs.c
> index c95d4583498e..28c025e39a00 100644
> --- a/drivers/media/common/siano/smsdvb-debugfs.c
> +++ b/drivers/media/common/siano/smsdvb-debugfs.c
> @@ -514,7 +514,7 @@ void smsdvb_debugfs_register(void)
>  	 * node for sdio-based boards, but this may need some logic at sdio
>  	 * subsystem.
>  	 */
> -	d = debugfs_create_dir("smsdvb", usb_debug_root);
> +	d = usb_debugfs_create_dir("smsdvb");

It is odd that a media driver is using the usb debugfs directory.
Anyone know why that is?

thanks,

greg k-h

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 17/28] usb: chipidea: debug: create debugfs directory under usb root

[Chunfeng Yun]

Move it's directory from the root of the debugfs filesystem into
the root of usb

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/chipidea/debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/chipidea/debug.c b/drivers/usb/chipidea/debug.c
index fcc91a338875..b777f30cbffe 100644
--- a/drivers/usb/chipidea/debug.c
+++ b/drivers/usb/chipidea/debug.c
@@ -342,7 +342,7 @@ DEFINE_SHOW_ATTRIBUTE(ci_registers);
  */
 void dbg_create_files(struct ci_hdrc *ci)
 {
-	ci->debugfs = debugfs_create_dir(dev_name(ci->dev), NULL);
+	ci->debugfs = usb_debugfs_create_dir(dev_name(ci->dev));
 
 	debugfs_create_file("device", S_IRUGO, ci->debugfs, ci,
 			    &ci_device_fops);
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 18/28] usb: host: imx21: create debugfs directory under usb root

[Chunfeng Yun]

Move it's directory from the root of the debugfs filesystem into
the root of usb

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/imx21-dbg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/imx21-dbg.c b/drivers/usb/host/imx21-dbg.c
index 7fcf1d9dd7f3..42694295fb66 100644
--- a/drivers/usb/host/imx21-dbg.c
+++ b/drivers/usb/host/imx21-dbg.c
@@ -419,7 +419,7 @@ static void create_debug_files(struct imx21 *imx21)
 {
 	struct dentry *root;
 
-	root = debugfs_create_dir(dev_name(imx21->dev), NULL);
+	root = usb_debugfs_create_dir(dev_name(imx21->dev));
 	imx21->debug_root = root;
 
 	debugfs_create_file("status", S_IRUGO, root, imx21, &debug_status_fops);
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 19/28] usb: dwc3: create debugfs subdirectory under usb root directory

[Chunfeng Yun]

Now the USB gadget subsystem can use the USB debugfs root directory,
so move dwc3's directory from the root of the debugfs filesystem into
the root of usb

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/dwc3/debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index 1c792710348f..a1824db7fb2e 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -916,7 +916,7 @@ void dwc3_debugfs_init(struct dwc3 *dwc)
 	dwc->regset->nregs = ARRAY_SIZE(dwc3_regs);
 	dwc->regset->base = dwc->regs - DWC3_GLOBALS_REGS_START;
 
-	root = debugfs_create_dir(dev_name(dwc->dev), NULL);
+	root = usb_debugfs_create_dir(dev_name(dwc->dev));
 	dwc->root = root;
 
 	debugfs_create_regset32("regdump", S_IRUGO, root, dwc->regset);
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 20/28] usb: musb: create debugfs subdirectory under usb root directory

[Chunfeng Yun]

Now the USB gadget subsystem can use the USB debugfs root directory,
so move musb's directory from the root of the debugfs filesystem into
the root of usb

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/musb/musb_debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_debugfs.c b/drivers/usb/musb/musb_debugfs.c
index f42858e2b54c..599576c30991 100644
--- a/drivers/usb/musb/musb_debugfs.c
+++ b/drivers/usb/musb/musb_debugfs.c
@@ -325,7 +325,7 @@ void musb_init_debugfs(struct musb *musb)
 {
 	struct dentry *root;
 
-	root = debugfs_create_dir(dev_name(musb->controller), NULL);
+	root = usb_debugfs_create_dir(dev_name(musb->controller));
 	musb->debugfs_root = root;
 
 	debugfs_create_file("regdump", S_IRUGO, root, musb, &musb_regdump_fops);
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 21/28] usb: musb: dsps: create debugfs subdirectory under usb root directory

[Chunfeng Yun]

Now the USB gadget subsystem can use the USB debugfs root directory,
so move dsps's directory from the root of the debugfs filesystem into
the root of usb

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/musb/musb_dsps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 327d4f7baaf7..987c7c7b9973 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -411,7 +411,7 @@ static int dsps_musb_dbg_init(struct musb *musb, struct dsps_glue *glue)
 	char buf[128];
 
 	sprintf(buf, "%s.dsps", dev_name(musb->controller));
-	root = debugfs_create_dir(buf, NULL);
+	root = usb_debugfs_create_dir(buf);
 	glue->dbgfs_root = root;
 
 	glue->regset.regs = dsps_musb_regs;
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 22/28] usb: dwc2: create debugfs subdirectory under usb root directory

[Chunfeng Yun]

Now the USB gadget subsystem can use the USB debugfs root directory,
so move dwc2's directory from the root of the debugfs filesystem into
the root of usb

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/dwc2/debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/debugfs.c b/drivers/usb/dwc2/debugfs.c
index 7f62f4cdc265..e7709f73d4ff 100644
--- a/drivers/usb/dwc2/debugfs.c
+++ b/drivers/usb/dwc2/debugfs.c
@@ -770,7 +770,7 @@ int dwc2_debugfs_init(struct dwc2_hsotg *hsotg)
 	int			ret;
 	struct dentry		*root;
 
-	root = debugfs_create_dir(dev_name(hsotg->dev), NULL);
+	root = usb_debugfs_create_dir(dev_name(hsotg->dev));
 	hsotg->debug_root = root;
 
 	debugfs_create_file("params", 0444, root, hsotg, &params_fops);
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 23/28] usb: gadget: bcm63xx_udc: create debugfs directory under usb root

[Chunfeng Yun]

Now the USB gadget subsystem can use the USB debugfs root directory,
so move it's directory from the root of the debugfs filesystem into
the root of usb

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/gadget/udc/bcm63xx_udc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/bcm63xx_udc.c b/drivers/usb/gadget/udc/bcm63xx_udc.c
index 97b16463f3ef..ae2c3740540a 100644
--- a/drivers/usb/gadget/udc/bcm63xx_udc.c
+++ b/drivers/usb/gadget/udc/bcm63xx_udc.c
@@ -2248,7 +2248,7 @@ static void bcm63xx_udc_init_debugfs(struct bcm63xx_udc *udc)
 	if (!IS_ENABLED(CONFIG_USB_GADGET_DEBUG_FS))
 		return;
 
-	root = debugfs_create_dir(udc->gadget.name, NULL);
+	root = usb_debugfs_create_dir(udc->gadget.name);
 	udc->debugfs_root = root;
 
 	debugfs_create_file("usbd", 0400, root, udc, &bcm63xx_usbd_dbg_fops);
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 24/28] usb: gadget: atmel: create debugfs directory under usb root

[Chunfeng Yun]

Now the USB gadget subsystem can use the USB debugfs root directory,
so move it's directory from the root of the debugfs filesystem into
the root of usb

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/gadget/udc/atmel_usba_udc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 86ffc8307864..43618833e0f6 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -226,7 +226,7 @@ static void usba_init_debugfs(struct usba_udc *udc)
 	struct dentry *root;
 	struct resource *regs_resource;
 
-	root = debugfs_create_dir(udc->gadget.name, NULL);
+	root = debugfs_create_dir(udc->gadget.name);
 	udc->debugfs_root = root;
 
 	regs_resource = platform_get_resource(udc->pdev, IORESOURCE_MEM,
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 25/28] usb: gadget: udc: renesas_usb3: create debugfs directory under usb root

[Chunfeng Yun]

Now the USB gadget subsystem can use the USB debugfs root directory,
so move it's directory from the root of the debugfs filesystem into
the root of usb

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/gadget/udc/renesas_usb3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
index e098f16c01cb..b1e113923f4b 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -2535,7 +2535,7 @@ static const struct file_operations renesas_usb3_b_device_fops = {
 static void renesas_usb3_debugfs_init(struct renesas_usb3 *usb3,
 				      struct device *dev)
 {
-	usb3->dentry = debugfs_create_dir(dev_name(dev), NULL);
+	usb3->dentry = usb_debugfs_create_dir(dev_name(dev));
 
 	debugfs_create_file("b_device", 0644, usb3->dentry, usb3,
 			    &renesas_usb3_b_device_fops);
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 26/28] usb: gadget: pxa27x: create debugfs directory under usb root

[Chunfeng Yun]

Now the USB gadget subsystem can use the USB debugfs root directory,
so move it's directory from the root of the debugfs filesystem into
the root of usb

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/gadget/udc/pxa27x_udc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c b/drivers/usb/gadget/udc/pxa27x_udc.c
index 014233252299..8368b08cd115 100644
--- a/drivers/usb/gadget/udc/pxa27x_udc.c
+++ b/drivers/usb/gadget/udc/pxa27x_udc.c
@@ -207,7 +207,7 @@ static void pxa_init_debugfs(struct pxa_udc *udc)
 {
 	struct dentry *root;
 
-	root = debugfs_create_dir(udc->gadget.name, NULL);
+	root = usb_debugfs_create_dir(udc->gadget.name);
 	udc->debugfs_root = root;
 
 	debugfs_create_file("udcstate", 0400, root, udc, &state_dbg_fops);
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 27/28] usb: gadget: udc: gr_udc: create debugfs directory under usb root

[Chunfeng Yun]

Now the USB gadget subsystem can use the USB debugfs root directory,
so move it's directory from the root of the debugfs filesystem into
the root of usb

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/gadget/udc/gr_udc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/gr_udc.c b/drivers/usb/gadget/udc/gr_udc.c
index 7a0e9a58c2d8..838174add0f2 100644
--- a/drivers/usb/gadget/udc/gr_udc.c
+++ b/drivers/usb/gadget/udc/gr_udc.c
@@ -208,7 +208,7 @@ static void gr_dfs_create(struct gr_udc *dev)
 {
 	const char *name = "gr_udc_state";
 
-	dev->dfs_root = debugfs_create_dir(dev_name(dev->dev), NULL);
+	dev->dfs_root = usb_debugfs_create_dir(dev_name(dev->dev));
 	debugfs_create_file(name, 0444, dev->dfs_root, dev, &gr_dfs_fops);
 }
 
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 28/28] usb: gadget: udc: s3c2410_udc: create debugfs directory under usb root

[Chunfeng Yun]

Now the USB gadget subsystem can use the USB debugfs root directory,
so move it's directory from the root of the debugfs filesystem into
the root of usb

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/gadget/udc/s3c2410_udc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/s3c2410_udc.c b/drivers/usb/gadget/udc/s3c2410_udc.c
index f82208fbc249..575c63dd0754 100644
--- a/drivers/usb/gadget/udc/s3c2410_udc.c
+++ b/drivers/usb/gadget/udc/s3c2410_udc.c
@@ -1978,7 +1978,7 @@ static int __init udc_init(void)
 
 	dprintk(DEBUG_NORMAL, "%s\n", gadget_name);
 
-	s3c2410_udc_debugfs_root = debugfs_create_dir(gadget_name, NULL);
+	s3c2410_udc_debugfs_root = usb_debugfs_create_dir(gadget_name);
 
 	retval = platform_driver_register(&udc_driver_24x0);
 	if (retval)
-- 
2.23.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 01/28] usb: common: change usb_debug_root as static variable

[Guenter Roeck]

On 11/5/19 7:15 PM, Chunfeng Yun wrote:
> Try to avoid using extern global variable, and provide two
> functions for the usage cases
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>   drivers/usb/common/common.c | 16 ++++++++++++++--
>   include/linux/usb.h         |  5 ++++-
>   2 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> index 1433260d99b4..639ee6d243a2 100644
> --- a/drivers/usb/common/common.c
> +++ b/drivers/usb/common/common.c
> @@ -293,8 +293,20 @@ 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 struct dentry *usb_debug_root;
> +

I don't think it is a good idea to declare this variable static
before all its uses are removed. This patch will leave the system
in a non-compileable state until all patches of the series have
been applied.

I am curious. What is the rationale for this change ?
Did exported variables run out of favor ? Sorry if there was a
patch 0 of the series explaining the rationale and I missed it.

Guenter

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 01/28] usb: common: change usb_debug_root as static variable

[Wolfram Sang]

[-- Attachment #1.1: Type: text/plain, Size: 236 bytes --]


> Did exported variables run out of favor ? Sorry if there was a
> patch 0 of the series explaining the rationale and I missed it.

I neither got it if there was one. If there wasn't, I agree a cover
letter makes a lot of sense here.


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 01/28] usb: common: change usb_debug_root as static variable

[Greg Kroah-Hartman]

On Wed, Nov 06, 2019 at 08:33:10AM +0100, Wolfram Sang wrote:
> 
> > Did exported variables run out of favor ? Sorry if there was a
> > patch 0 of the series explaining the rationale and I missed it.
> 
> I neither got it if there was one. If there wasn't, I agree a cover
> letter makes a lot of sense here.

I don't understand why this whole series is needed either.

What is wrong with the original code?  Also, like I said before, you
broke the build here with the first patch, which is not ok.

greg k-h

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 01/28] usb: common: change usb_debug_root as static variable

[Chunfeng Yun]

On Wed, 2019-11-06 at 09:51 +0100, Greg Kroah-Hartman wrote:
> On Wed, Nov 06, 2019 at 08:33:10AM +0100, Wolfram Sang wrote:
> > 
> > > Did exported variables run out of favor ? Sorry if there was a
> > > patch 0 of the series explaining the rationale and I missed it.
> > 
> > I neither got it if there was one. If there wasn't, I agree a cover
> > letter makes a lot of sense here.
> 
> I don't understand why this whole series is needed either.
> 
> What is wrong with the original code? 
No wrong at all, just think it's good practice to avoid global variable,
more and more controller drivers use it now.
And it's also clear enough for global variable usb_debug_root

>  Also, like I said before, you
> broke the build here with the first patch, which is not ok.
> 
> greg k-h



_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 01/28] usb: common: change usb_debug_root as static variable

[Chunfeng Yun]

On Tue, 2019-11-05 at 20:03 -0800, Guenter Roeck wrote:
> On 11/5/19 7:15 PM, Chunfeng Yun wrote:
> > Try to avoid using extern global variable, and provide two
> > functions for the usage cases
> > 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> >   drivers/usb/common/common.c | 16 ++++++++++++++--
> >   include/linux/usb.h         |  5 ++++-
> >   2 files changed, 18 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > index 1433260d99b4..639ee6d243a2 100644
> > --- a/drivers/usb/common/common.c
> > +++ b/drivers/usb/common/common.c
> > @@ -293,8 +293,20 @@ 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 struct dentry *usb_debug_root;
> > +
> 
> I don't think it is a good idea to declare this variable static
> before all its uses are removed. This patch will leave the system
> in a non-compileable state until all patches of the series have
> been applied.
Yes, you are right, I'll fix it, may be split into two patch

> 
> I am curious. What is the rationale for this change ?
> Did exported variables run out of favor ?
Personally, I think it's good practice to avoid global varibale,
it may not a good reason. 
another one is that want to create all debug directory/file in usb root
for host/device controller drivers.

>  Sorry if there was a
> patch 0 of the series explaining the rationale and I missed it.
Sorry, I do not prepare cover latter, will add it if send out next
version.


> 
> Guenter



_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 01/28] usb: common: change usb_debug_root as static variable

[Guenter Roeck]

On 11/6/19 1:11 AM, Chunfeng Yun wrote:
> On Tue, 2019-11-05 at 20:03 -0800, Guenter Roeck wrote:
>> On 11/5/19 7:15 PM, Chunfeng Yun wrote:
>>> Try to avoid using extern global variable, and provide two
>>> functions for the usage cases
>>>
>>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>>> ---
>>>    drivers/usb/common/common.c | 16 ++++++++++++++--
>>>    include/linux/usb.h         |  5 ++++-
>>>    2 files changed, 18 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
>>> index 1433260d99b4..639ee6d243a2 100644
>>> --- a/drivers/usb/common/common.c
>>> +++ b/drivers/usb/common/common.c
>>> @@ -293,8 +293,20 @@ 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 struct dentry *usb_debug_root;
>>> +
>>
>> I don't think it is a good idea to declare this variable static
>> before all its uses are removed. This patch will leave the system
>> in a non-compileable state until all patches of the series have
>> been applied.
> Yes, you are right, I'll fix it, may be split into two patch
> 
>>
>> I am curious. What is the rationale for this change ?
>> Did exported variables run out of favor ?
> Personally, I think it's good practice to avoid global varibale,
> it may not a good reason.

Personally I don't think it is a good reason at all. When you are done,
someone else may come in and declare the opposite, re-introduce the global
variable, and drop the new functions as unnecessary.

This is ok for new code, but I think we should leave existing code alone
unless it is broken and needs to get fixed. This is not the case here.

Thanks,
Guenter

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 01/28] usb: common: change usb_debug_root as static variable

[Chunfeng Yun]

Hi Greg,

On Wed, 2019-11-06 at 06:00 -0800, Guenter Roeck wrote:
> On 11/6/19 1:11 AM, Chunfeng Yun wrote:
> > On Tue, 2019-11-05 at 20:03 -0800, Guenter Roeck wrote:
> >> On 11/5/19 7:15 PM, Chunfeng Yun wrote:
> >>> Try to avoid using extern global variable, and provide two
> >>> functions for the usage cases
> >>>
> >>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> >>> ---
> >>>    drivers/usb/common/common.c | 16 ++++++++++++++--
> >>>    include/linux/usb.h         |  5 ++++-
> >>>    2 files changed, 18 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> >>> index 1433260d99b4..639ee6d243a2 100644
> >>> --- a/drivers/usb/common/common.c
> >>> +++ b/drivers/usb/common/common.c
> >>> @@ -293,8 +293,20 @@ 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 struct dentry *usb_debug_root;
> >>> +
> >>
> >> I don't think it is a good idea to declare this variable static
> >> before all its uses are removed. This patch will leave the system
> >> in a non-compileable state until all patches of the series have
> >> been applied.
> > Yes, you are right, I'll fix it, may be split into two patch
> > 
> >>
> >> I am curious. What is the rationale for this change ?
> >> Did exported variables run out of favor ?
> > Personally, I think it's good practice to avoid global varibale,
> > it may not a good reason.
> 
> Personally I don't think it is a good reason at all. When you are done,
> someone else may come in and declare the opposite, re-introduce the global
> variable, and drop the new functions as unnecessary.
> 
> This is ok for new code, but I think we should leave existing code alone
> unless it is broken and needs to get fixed. This is not the case here.
What do you think about this, if you also agree with Guenter's opinion,
I'll abandon this patch.

BTW, how about moving udc's debug directory into usb root?

Thanks

> 
> Thanks,
> Guenter



_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 01/28] usb: common: change usb_debug_root as static variable

[Greg Kroah-Hartman]

On Thu, Nov 07, 2019 at 05:06:34PM +0800, Chunfeng Yun wrote:
> Hi Greg,
> 
> On Wed, 2019-11-06 at 06:00 -0800, Guenter Roeck wrote:
> > On 11/6/19 1:11 AM, Chunfeng Yun wrote:
> > > On Tue, 2019-11-05 at 20:03 -0800, Guenter Roeck wrote:
> > >> On 11/5/19 7:15 PM, Chunfeng Yun wrote:
> > >>> Try to avoid using extern global variable, and provide two
> > >>> functions for the usage cases
> > >>>
> > >>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > >>> ---
> > >>>    drivers/usb/common/common.c | 16 ++++++++++++++--
> > >>>    include/linux/usb.h         |  5 ++++-
> > >>>    2 files changed, 18 insertions(+), 3 deletions(-)
> > >>>
> > >>> diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > >>> index 1433260d99b4..639ee6d243a2 100644
> > >>> --- a/drivers/usb/common/common.c
> > >>> +++ b/drivers/usb/common/common.c
> > >>> @@ -293,8 +293,20 @@ 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 struct dentry *usb_debug_root;
> > >>> +
> > >>
> > >> I don't think it is a good idea to declare this variable static
> > >> before all its uses are removed. This patch will leave the system
> > >> in a non-compileable state until all patches of the series have
> > >> been applied.
> > > Yes, you are right, I'll fix it, may be split into two patch
> > > 
> > >>
> > >> I am curious. What is the rationale for this change ?
> > >> Did exported variables run out of favor ?
> > > Personally, I think it's good practice to avoid global varibale,
> > > it may not a good reason.
> > 
> > Personally I don't think it is a good reason at all. When you are done,
> > someone else may come in and declare the opposite, re-introduce the global
> > variable, and drop the new functions as unnecessary.
> > 
> > This is ok for new code, but I think we should leave existing code alone
> > unless it is broken and needs to get fixed. This is not the case here.
> What do you think about this, if you also agree with Guenter's opinion,

I do.

> I'll abandon this patch.

Please do.

> BTW, how about moving udc's debug directory into usb root?

Sure, feel free to send a patch for that.

thanks,

greg k-h

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 01/28] usb: common: change usb_debug_root as static variable

[Greg Kroah-Hartman]

On Wed, Nov 06, 2019 at 11:15:02AM +0800, Chunfeng Yun wrote:
> Try to avoid using extern global variable, and provide two
> functions for the usage cases
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  drivers/usb/common/common.c | 16 ++++++++++++++--
>  include/linux/usb.h         |  5 ++++-
>  2 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> index 1433260d99b4..639ee6d243a2 100644
> --- a/drivers/usb/common/common.c
> +++ b/drivers/usb/common/common.c
> @@ -293,8 +293,20 @@ 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 struct dentry *usb_debug_root;

You just broke the kernel build, which is not acceptable at all :(

greg k-h

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 01/28] usb: common: change usb_debug_root as static variable

[Chunfeng Yun]

Hi Greg,

On Wed, 2019-11-06 at 09:51 +0100, Greg Kroah-Hartman wrote:
> On Wed, Nov 06, 2019 at 11:15:02AM +0800, Chunfeng Yun wrote:
> > Try to avoid using extern global variable, and provide two
> > functions for the usage cases
> > 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> >  drivers/usb/common/common.c | 16 ++++++++++++++--
> >  include/linux/usb.h         |  5 ++++-
> >  2 files changed, 18 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > index 1433260d99b4..639ee6d243a2 100644
> > --- a/drivers/usb/common/common.c
> > +++ b/drivers/usb/common/common.c
> > @@ -293,8 +293,20 @@ 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 struct dentry *usb_debug_root;
> 
> You just broke the kernel build, which is not acceptable at all :(
Vey sorry, I didn't get what you mean before, now get it.


> 
> greg k-h



_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek