Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Guenter Roeck <linux@roeck-us.net>,
	linux-usb@vger.kernel.org
Subject: Re: [PATCH 2/3] usb: typec: fusb: Use usb_debug_root as root for our debugfs entry
Date: Fri, 16 Aug 2019 10:51:09 +0300
Message-ID: <20190816075109.GF24772@kuha.fi.intel.com> (raw)
In-Reply-To: <20190815191815.755606-2-hdegoede@redhat.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

  parent reply index

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-15 19:18 [PATCH 1/3] usb: typec: tcpm: " Hans de Goede
2019-08-15 19:18 ` [PATCH 2/3] usb: typec: fusb: " Hans de Goede
2019-08-15 19:46   ` Guenter Roeck
2019-08-16  7:51   ` Heikki Krogerus [this message]
2019-08-17 18:32     ` Hans de Goede
2019-08-15 19:18 ` [PATCH 3/3] usb: typec: fusb302: Call fusb302_debugfs_init earlier Hans de Goede
2019-08-15 19:47   ` Guenter Roeck
2019-08-16  7:38   ` Heikki Krogerus
2019-08-15 19:46 ` [PATCH 1/3] usb: typec: tcpm: Use usb_debug_root as root for our debugfs entry Guenter Roeck
2019-08-16  7:55 ` Heikki Krogerus

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190816075109.GF24772@kuha.fi.intel.com \
    --to=heikki.krogerus@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hdegoede@redhat.com \
    --cc=linux-usb@vger.kernel.org \
    --cc=linux@roeck-us.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org linux-usb@archiver.kernel.org
	public-inbox-index linux-usb


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/ public-inbox