All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Javier Carrasco <javier.carrasco.cruz@gmail.com>
Cc: John Horan <knasher@gmail.com>,
	Henrik Rydberg <rydberg@bitmath.org>,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	syzbot+348331f63b034f89b622@syzkaller.appspotmail.com
Subject: Re: [PATCH v2] Input: bcm5974 - check endpoint type before starting traffic
Date: Fri, 13 Oct 2023 20:03:40 -0700	[thread overview]
Message-ID: <ZSoFDLv8_CG1SGN6@google.com> (raw)
In-Reply-To: <20231007-topic-bcm5974_bulk-v2-1-021131c83efb@gmail.com>

Hi Javier,

On Thu, Oct 12, 2023 at 06:51:49PM +0200, Javier Carrasco wrote:
>  
> +static bool bcm5974_ep_is_int_in(struct usb_host_interface *iface, int addr)
> +{
> +	struct usb_endpoint_descriptor *endpoint;
> +	int i;
> +
> +	for (i = 0; i < iface->desc.bNumEndpoints; i++) {
> +		endpoint = &iface->endpoint[i].desc;
> +		if (endpoint->bEndpointAddress == addr) {
> +			if (usb_endpoint_is_int_in(endpoint))
> +				return true;
> +		}
> +	}
> +	return false;
> +}

This essentially reimplements usb_find_endpoint() in a sense, so can we
instead do:

	ep = usb_find_endpoint(iface, addr);
	if (!ep || !usb_endpoint_is_int_in(ep)) {
		dev_err(...);
		return ...;
	}


Also it looks like the handling of button endpoint is interleaved with
the trackpad endpoint, I wonder if it would not be better if we have a
separate "if (cfg->tp_type == TYPE1)" where we would do the check,
allocate URB, and did all the rest of set up for button transfers.

Thanks.

-- 
Dmitry

  reply	other threads:[~2023-10-14  3:03 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-12 16:51 [PATCH v2] Input: bcm5974 - check endpoint type before starting traffic Javier Carrasco
2023-10-14  3:03 ` Dmitry Torokhov [this message]
2023-10-14  7:57   ` Javier Carrasco

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=ZSoFDLv8_CG1SGN6@google.com \
    --to=dmitry.torokhov@gmail.com \
    --cc=javier.carrasco.cruz@gmail.com \
    --cc=knasher@gmail.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rydberg@bitmath.org \
    --cc=syzbot+348331f63b034f89b622@syzkaller.appspotmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.