lkml.org 
[lkml]   [2023]   [Oct]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2] Input: bcm5974 - check endpoint type before starting traffic
From
Hi Dmitry,

On 14.10.23 05:03, Dmitry Torokhov wrote:
> 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 ...;
> }
>
Thanks for your feedback. usb_find_endpoint is a static function from
the usb core and in principle is not available here, but your code
snippet seems to reinterpret usb_check_int_endpoints() for a single
address. I would suggest using usb_check_int_endpoints and pass both
addresses at the same time (if both exist, of course).
>
> 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.
Using usb_check_int_endpoints would solve this immediately.
>
> Thanks.
>
Thanks and best regards,
Javier Carrasco

\
 
 \ /
  Last update: 2023-10-14 09:58    [W:0.135 / U:3.584 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site