Messages in this thread | | | From | Zheyu Ma <> | Date | Tue, 31 May 2022 11:50:18 +0800 | Subject | Re: [PATCH v3] char: xillybus: Check endpoint type at probe time |
| |
On Mon, May 30, 2022 at 11:13 PM Eli Billauer <eli.billauer@gmail.com> wrote: > > On 29/05/22 09:58, Zheyu Ma wrote: > > static int xillyusb_setup_base_eps(struct xillyusb_dev *xdev) > > { > > + int ret; > > + struct usb_endpoint_descriptor *in, *out; > > + > > + ret = usb_find_common_endpoints(xdev->intf->cur_altsetting,&in,&out, NULL, NULL); > > + if (ret) > > + return ret; > > + > > + if (in->bEndpointAddress != (IN_EP_NUM | USB_DIR_IN) || > > + out->bEndpointAddress != (MSG_EP_NUM | USB_DIR_OUT)) > > + return -EINVAL; > > + > > > As far as I understand, this finds the first BULK endpoints in both > directions, and verifies that their addresses are MSG_EP_NUM and > IN_EP_NUM. Because both of these happen to equal 1, I suppose this > indeed checks the right thing. But am I right that this won't work if > either MSG_EP_NUM or IN_EP_NUM have a value that isn't 1? Not that I > think that will ever happen, but still.
Indeed, the correctness of this code comes from the fact that both MSG_EP_NUM and IN_EP_NUM are the first bulk endpoint, without such an assumption this check does not hold. I did this just for convenience, we can also use xillyusb_check_endpoint() to check them.
> > +static int xillyusb_check_endpoint(struct xillyusb_dev *xdev, u8 addr) > > +{ > > + int i; > > + struct usb_host_interface *if_desc = xdev->intf->altsetting; > > + > > + for (i = 0; i< if_desc->desc.bNumEndpoints; i++) { > > + struct usb_endpoint_descriptor *ep =&if_desc->endpoint[i].desc; > > + > > + if (ep->bEndpointAddress == addr&& usb_endpoint_is_bulk_out(ep)) > > + return 0; > > + } > > + > > + return -EINVAL; > > +} > > + > > > > > Given that you've added this function, why isn't it used in > xillyusb_setup_base_eps()?
It is feasible to use this function for checking in xillyusb_setup_base_eps(), perhaps with some modifications. Anyway, I will do so in the next version of the patch.
Thanks, Zheyu Ma
| |