[PATCH] usb: host: Use helper function to get endpoint

[PATCH] usb: host: Use helper function to get endpoint

[Khalid Masum]

Current implementation to convert urb pipe number to struct 
usb_host_endpoint in rquest_single_step_set_feature_urb is a little
messy. 

Use usb_pipe_endpoint helper function to get the endpoint instead.

Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>
---
 drivers/usb/core/hcd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 94b305bbd621..107e29d5d3ae 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2165,8 +2165,7 @@ static struct urb *request_single_step_set_feature_urb(
 		return NULL;
 
 	urb->pipe = usb_rcvctrlpipe(udev, 0);
-	ep = (usb_pipein(urb->pipe) ? udev->ep_in : udev->ep_out)
-				[usb_pipeendpoint(urb->pipe)];
+	ep = usb_pipe_endpoint(udev, urb->pipe);
 	if (!ep) {
 		usb_free_urb(urb);
 		return NULL;
-- 
2.37.1

Re: [PATCH] usb: host: Use helper function to get endpoint

[Alan Stern]

On Wed, Aug 24, 2022 at 07:07:02PM +0600, Khalid Masum wrote:
> Current implementation to convert urb pipe number to struct 
> usb_host_endpoint in rquest_single_step_set_feature_urb is a little
> messy. 
> 
> Use usb_pipe_endpoint helper function to get the endpoint instead.
> 
> Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>
> ---
>  drivers/usb/core/hcd.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 94b305bbd621..107e29d5d3ae 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2165,8 +2165,7 @@ static struct urb *request_single_step_set_feature_urb(
>  		return NULL;
>  
>  	urb->pipe = usb_rcvctrlpipe(udev, 0);
> -	ep = (usb_pipein(urb->pipe) ? udev->ep_in : udev->ep_out)
> -				[usb_pipeendpoint(urb->pipe)];
> +	ep = usb_pipe_endpoint(udev, urb->pipe);
>  	if (!ep) {
>  		usb_free_urb(urb);
>  		return NULL;

Even this is awkward.  It's silly to look up the endpoint in a table 
when you already know that it is endpoint 0.  Just do:

	ep = &udev->ep0;

with no need to check for NULL.

Alan Stern

Re: [PATCH] usb: host: Use helper function to get endpoint

[Khalid Masum]

On Wed, Aug 24, 2022 at 8:40 PM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Wed, Aug 24, 2022 at 07:07:02PM +0600, Khalid Masum wrote:
>
>         ep = &udev->ep0;
>
> with no need to check for NULL.

Thanks, I shall send a v2 with this change.
> Alan Stern

-- Khalid Masum

Re: [PATCH] usb: host: Use helper function to get endpoint

[Khalid Masum]

On Wed, Aug 24, 2022 at 8:40 PM Alan Stern <stern@rowland.harvard.edu> wrote:
>
>
> Even this is awkward.  It's silly to look up the endpoint in a table
> when you already know that it is endpoint 0.  Just do:
>
>         ep = &udev->ep0;
>
> with no need to check for NULL.

After further checking, I realized that  usb_device udev is created by
ehset_single_step_set_feature and depends on usb_hcd and port. So I do
not get why the endpoint is at udev->ep0. Can you help me with this?

-- Khalid Masum

Re: [PATCH] usb: host: Use helper function to get endpoint

[Alan Stern]

On Wed, Aug 24, 2022 at 11:57:25PM +0600, Khalid Masum wrote:
> On Wed, Aug 24, 2022 at 8:40 PM Alan Stern <stern@rowland.harvard.edu> wrote:
> >
> >
> > Even this is awkward.  It's silly to look up the endpoint in a table
> > when you already know that it is endpoint 0.  Just do:
> >
> >         ep = &udev->ep0;
> >
> > with no need to check for NULL.
> 
> After further checking, I realized that  usb_device udev is created by
> ehset_single_step_set_feature and depends on usb_hcd and port. So I do
> not get why the endpoint is at udev->ep0. Can you help me with this?

udev is not a usb_device structure; it is a _pointer_ to a usb_device 
structure.  The pointer is created when ehset_single_step_set_feature() 
calls usb_hub_find_child(), but the structure itself gets created long 
before that, when the USB device is first detected and initialized.

You can see this happening where hub_port_connect() calls 
usb_alloc_dev().  The ep0 field is filled in by hub_port_init() and the 
routines it calls.

Alan Stern

Re: [PATCH] usb: host: Use helper function to get endpoint

[Khalid Masum]

On Thu, Aug 25, 2022 at 1:24 AM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> udev is not a usb_device structure; it is a _pointer_ to a usb_device
> structure.  The pointer is created when ehset_single_step_set_feature()
> calls usb_hub_find_child(), but the structure itself gets created long
> before that, when the USB device is first detected and initialized.
>
> You can see this happening where hub_port_connect() calls
> usb_alloc_dev().  The ep0 field is filled in by hub_port_init() and the
> routines it calls.
>
Thanks for clarifying this. That helps a lot.

-- Khalid Masum