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

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

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@redhat.com>

I have one question below. Otherwise:

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  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?

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


thanks,

-- 
heikki