[U-Boot] [PATCH 3/4] SDP: fix wrong usb request size and add high speed endpoint descriptor

Lukasz Majewski
2019-07-18T07:32:20Z

Hi Sherry,

> From: Sherry Sun <sherry.sun@nxp.com>
>
> Because the buffer length of sdp usb request is 65,

Is the value 65 mentioned somewhere (in any doc/spec)?

As fair as I remember other protocols - like DFU use 64B. Do we need
here to store extra \0 or \n ?

The 64 bytes are default packet size on EP0.

If I may ask - what user space program do you use? imx_loader (imx_usb)
or uuu ?

> we have to
> allocate 65 bytes not 64 bytes. Otherwise there is potential buffer
> overflow.
>
> So the wMaxPacketSize of fullspeed can't meet the needs. Add HS
> endpoint descriptor for SDP. Then we can use high speed endpoint,
> and the SDP device can send packet with 512 byte size.
>
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> Signed-off-by: Ye Li <ye.li@nxp.com>
> ---
> drivers/usb/gadget/f_sdp.c | 33 ++++++++++++++++++++++++++++++---
> 1 file changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/gadget/f_sdp.c b/drivers/usb/gadget/f_sdp.c
> index b1601575e0..532041063a 100644
> --- a/drivers/usb/gadget/f_sdp.c
> +++ b/drivers/usb/gadget/f_sdp.c
> @@ -157,6 +157,16 @@ static struct usb_endpoint_descriptor in_desc = {
> .bInterval = 1,
> };
>
> +static struct usb_endpoint_descriptor in_hs_desc = {
> + .bLength = USB_DT_ENDPOINT_SIZE,
> + .bDescriptorType =
> USB_DT_ENDPOINT, /*USB_DT_CS_ENDPOINT*/ +
> + .bEndpointAddress = 1 | USB_DIR_IN,
> + .bmAttributes = USB_ENDPOINT_XFER_INT,
> + .wMaxPacketSize = 512,
> + .bInterval = 1,

Which endpoint do we use in SDP? EP0 or EP1...n ?

> +};
> +
> static struct usb_descriptor_header *sdp_runtime_descs[] = {
> (struct usb_descriptor_header *)&sdp_intf_runtime,
> (struct usb_descriptor_header *)&sdp_hid_desc,
> @@ -164,6 +174,13 @@ static struct usb_descriptor_header
> *sdp_runtime_descs[] = { NULL,
> };
>
> +static struct usb_descriptor_header *sdp_runtime_hs_descs[] = {
> + (struct usb_descriptor_header *)&sdp_intf_runtime,
> + (struct usb_descriptor_header *)&sdp_hid_desc,
> + (struct usb_descriptor_header *)&in_hs_desc,
> + NULL,
> +};
> +
> /* This is synchronized with what the SoC implementation reports */
> static struct hid_report sdp_hid_report = {
> .usage_page = {
> @@ -489,6 +506,11 @@ static int sdp_bind(struct usb_configuration *c,
> struct usb_function *f) goto error;
> }
>
> + if (gadget_is_dualspeed(gadget)) {
> + /* Assume endpoint addresses are the same for both
> speeds */
> + in_hs_desc.bEndpointAddress =
> in_desc.bEndpointAddress;
> + }
> +
> sdp->in_ep = ep; /* Store IN EP for enabling @ setup */
>
> cdev->req->context = sdp;
> @@ -526,7 +548,7 @@ static struct usb_request *sdp_start_ep(struct
> usb_ep *ep) {
> struct usb_request *req;
>
> - req = alloc_ep_req(ep, 64);
> + req = alloc_ep_req(ep, 65);
> debug("%s: ep:%p req:%p\n", __func__, ep, req);
>
> if (!req)
> @@ -541,11 +563,15 @@ static int sdp_set_alt(struct usb_function *f,
> unsigned intf, unsigned alt) {
> struct f_sdp *sdp = func_to_sdp(f);
> struct usb_composite_dev *cdev = f->config->cdev;
> + struct usb_gadget *gadget = cdev->gadget;
> int result;
>
> debug("%s: intf: %d alt: %d\n", __func__, intf, alt);
>
> - result = usb_ep_enable(sdp->in_ep, &in_desc);
> + if (gadget_is_dualspeed(gadget) && gadget->speed ==
> USB_SPEED_HIGH)
> + result = usb_ep_enable(sdp->in_ep, &in_hs_desc);
> + else
> + result = usb_ep_enable(sdp->in_ep, &in_desc);
> if (result)
> return result;
> sdp->in_req = sdp_start_ep(sdp->in_ep);
> @@ -591,7 +617,7 @@ static int sdp_bind_config(struct
> usb_configuration *c) memset(sdp_func, 0, sizeof(*sdp_func));
>
> sdp_func->usb_function.name = "sdp";
> - sdp_func->usb_function.hs_descriptors = sdp_runtime_descs;
> + sdp_func->usb_function.hs_descriptors = sdp_runtime_hs_descs;
> sdp_func->usb_function.descriptors = sdp_runtime_descs;
> sdp_func->usb_function.bind = sdp_bind;
> sdp_func->usb_function.unbind = sdp_unbind;
> @@ -724,6 +750,7 @@ static void sdp_handle_in_ep(struct
> spl_image_info *spl_image) /* In SPL, allow jumps to U-Boot images */
> struct spl_image_info spl_image = {};
> spl_parse_image_header(&spl_image, header);
> +
> jump_to_image_no_args(&spl_image);
> #else
> /* In U-Boot, allow jumps to scripts */

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190718/e15ff166/attachment.sig>