linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Chen <peter.chen@kernel.org>
To: Jack Pham <jackp@codeaurora.org>
Cc: Felipe Balbi <balbi@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Roger Quadros <rogerq@kernel.org>,
	linux-usb@vger.kernel.org, Wesley Cheng <wcheng@codeaurora.org>
Subject: Re: [PATCH] usb: dwc3: debugfs: Add and remove endpoint dirs dynamically
Date: Tue, 1 Jun 2021 15:07:44 +0800	[thread overview]
Message-ID: <20210601070744.GA9087@nchen> (raw)
In-Reply-To: <20210529192932.22912-1-jackp@codeaurora.org>

On 21-05-29 12:29:32, Jack Pham wrote:
> The DWC3 DebugFS directory and files are currently created once
> during probe.  This includes creation of subdirectories for each
> of the gadget's endpoints.  This works fine for peripheral-only
> controllers, as dwc3_core_init_mode() calls dwc3_gadget_init()
> just prior to calling dwc3_debugfs_init().
> 
> However, for dual-role controllers, dwc3_core_init_mode() will
> instead call dwc3_drd_init() which is problematic in a few ways.
> First, the initial state must be determined, then dwc3_set_mode()
> will have to schedule drd_work and by then dwc3_debugfs_init()
> could have already been invoked.  Even if the initial mode is
> peripheral, dwc3_gadget_init() happens after the DebugFS files
> are created, and worse so if the initial state is host and the
> controller switches to peripheral much later.  And secondly,
> even if the gadget endpoints' debug entries were successfully
> created, if the controller exits peripheral mode, its dwc3_eps
> are freed so the debug files would now hold stale references.
> 
> So it is best if the DebugFS endpoint entries are created and
> removed dynamically at the same time the underlying dwc3_eps are.
> Do this by calling dwc3_debugfs_create_endpoint_dir() as each
> endpoint is created, and conversely remove the DebugFS entry when
> the endpoint is freed.
> 
> Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly")
> Signed-off-by: Jack Pham <jackp@codeaurora.org>
> ---
>  drivers/usb/dwc3/debug.h   |  3 +++
>  drivers/usb/dwc3/debugfs.c | 21 ++-------------------
>  drivers/usb/dwc3/gadget.c  |  3 +++
>  3 files changed, 8 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h
> index d0ac89c5b317..d223c54115f4 100644
> --- a/drivers/usb/dwc3/debug.h
> +++ b/drivers/usb/dwc3/debug.h
> @@ -413,9 +413,12 @@ static inline const char *dwc3_gadget_generic_cmd_status_string(int status)
>  
>  
>  #ifdef CONFIG_DEBUG_FS
> +extern void dwc3_debugfs_create_endpoint_dir(struct dwc3_ep *dep);
>  extern void dwc3_debugfs_init(struct dwc3 *d);
>  extern void dwc3_debugfs_exit(struct dwc3 *d);
>  #else
> +static inline void dwc3_debugfs_create_endpoint_dir(struct dwc3_ep *dep)
> +{  }
>  static inline void dwc3_debugfs_init(struct dwc3 *d)
>  {  }
>  static inline void dwc3_debugfs_exit(struct dwc3 *d)
> diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
> index 7146ee2ac057..5dbbe53269d3 100644
> --- a/drivers/usb/dwc3/debugfs.c
> +++ b/drivers/usb/dwc3/debugfs.c
> @@ -886,30 +886,14 @@ static void dwc3_debugfs_create_endpoint_files(struct dwc3_ep *dep,
>  	}
>  }
>  
> -static void dwc3_debugfs_create_endpoint_dir(struct dwc3_ep *dep,
> -		struct dentry *parent)
> +void dwc3_debugfs_create_endpoint_dir(struct dwc3_ep *dep)
>  {
>  	struct dentry		*dir;
>  
> -	dir = debugfs_create_dir(dep->name, parent);
> +	dir = debugfs_create_dir(dep->name, dep->dwc->root);
>  	dwc3_debugfs_create_endpoint_files(dep, dir);
>  }
>  
> -static void dwc3_debugfs_create_endpoint_dirs(struct dwc3 *dwc,
> -		struct dentry *parent)
> -{
> -	int			i;
> -
> -	for (i = 0; i < dwc->num_eps; i++) {
> -		struct dwc3_ep	*dep = dwc->eps[i];
> -
> -		if (!dep)
> -			continue;
> -
> -		dwc3_debugfs_create_endpoint_dir(dep, parent);
> -	}
> -}
> -
>  void dwc3_debugfs_init(struct dwc3 *dwc)
>  {
>  	struct dentry		*root;
> @@ -940,7 +924,6 @@ void dwc3_debugfs_init(struct dwc3 *dwc)
>  				&dwc3_testmode_fops);
>  		debugfs_create_file("link_state", 0644, root, dwc,
>  				    &dwc3_link_state_fops);
> -		dwc3_debugfs_create_endpoint_dirs(dwc, root);
>  	}
>  }
>  
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 65d9b7227752..dbba31d415d7 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2754,6 +2754,8 @@ static int dwc3_gadget_init_endpoint(struct dwc3 *dwc, u8 epnum)
>  	INIT_LIST_HEAD(&dep->started_list);
>  	INIT_LIST_HEAD(&dep->cancelled_list);
>  
> +	dwc3_debugfs_create_endpoint_dir(dep);
> +
>  	return 0;
>  }
>  
> @@ -2797,6 +2799,7 @@ static void dwc3_gadget_free_endpoints(struct dwc3 *dwc)
>  			list_del(&dep->endpoint.ep_list);
>  		}
>  
> +		debugfs_remove_recursive(debugfs_lookup(dep->name, dwc->root));

There is one more debugfs_remove_recursive at dwc3_debugfs_exit, need to delete?

>  		kfree(dep);
>  	}
>  }
> -- 
> 2.24.0
> 

-- 

Thanks,
Peter Chen


  reply	other threads:[~2021-06-01  7:07 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-29 19:29 [PATCH] usb: dwc3: debugfs: Add and remove endpoint dirs dynamically Jack Pham
2021-06-01  7:07 ` Peter Chen [this message]
2021-06-01 16:09   ` Jack Pham
2021-06-02  0:55 ` Peter Chen

Reply instructions:

You may reply publicly 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=20210601070744.GA9087@nchen \
    --to=peter.chen@kernel.org \
    --cc=balbi@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jackp@codeaurora.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=rogerq@kernel.org \
    --cc=wcheng@codeaurora.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).