Re: [PATCH 2/3] usb: typec: fusb: Use usb_debug_root as root for our debugfs entry

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 16-08-19 09:51, Heikki Krogerus wrote:
On Thu, Aug 15, 2019 at 09:18:14PM +0200, Hans de Goede wrote:
Use usb_debug_root as root for our debugfs entry instead of creating our
own subdirectory under the debugfs root.

Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>

I have one question below. Otherwise:

Reviewed-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>

---
  drivers/usb/typec/tcpm/fusb302.c | 9 ++-------
  1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
index 93244d6c4bff..69a2afaf8f68 100644
--- a/drivers/usb/typec/tcpm/fusb302.c
+++ b/drivers/usb/typec/tcpm/fusb302.c
@@ -26,6 +26,7 @@
  #include <linux/spinlock.h>
  #include <linux/string.h>
  #include <linux/types.h>
+#include <linux/usb.h>
  #include <linux/usb/typec.h>
  #include <linux/usb/tcpm.h>
  #include <linux/usb/pd.h>
@@ -206,23 +207,17 @@ static int fusb302_debug_show(struct seq_file *s, void *v)
  }
  DEFINE_SHOW_ATTRIBUTE(fusb302_debug);
-static struct dentry *rootdir;
-
  static void fusb302_debugfs_init(struct fusb302_chip *chip)
  {
  	mutex_init(&chip->logbuffer_lock);
-	if (!rootdir)
-		rootdir = debugfs_create_dir("fusb302", NULL);
-
  	chip->dentry = debugfs_create_file(dev_name(chip->dev),
-					   S_IFREG | 0444, rootdir,
+					   S_IFREG | 0444, usb_debug_root,
  					   chip, &fusb302_debug_fops);

In tcpm.c you named the entries "tcpm-%s", dev_name(...

Shouldn't we do something similar with these as well? I mean,
even though this is just debugfs, shouldn't we give some hint for the
user how to identify these entries?

Well on my test-machinw te dev_name is i2c-fusb302, but I realize the
dev_name is not always going to be so descriptive, so I will send
out a v2 adding a "fusb302-" prefix, like how we are doing for the
tcpm.c debug entry already.

How about if we still continue grouping the entries under the
"fusb302" directory, but just place that under usb_debug_root?

I would rather go with a prefix, the whole purpose of dropping the
subdir is so that we do not have a resource (the dir) shared between
multiple instances of the driver.

Regards,

Hans




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux