Hi, Lukasz
>
> Hi Sherry,
>
> > From: Sherry Sun <sherry....@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 ?
>
You can see case SDP_STATE_TX_STATUS and case SDP_STATE_TX_REGISTER in
static void sdp_handle_in_ep(struct spl_image_info *spl_image)
Here sdp_func->in_req->length = 65;
Data[0] is the report id, and the next 64 bytes are the date to host.
So totally we need 65 bytes.
> 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 ?
>
Yes, I use 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....@nxp.com>
> > Signed-off-by: Ye Li <ye...@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 ?
We use EP0 to transfer datas from host to device in report 1 and report 2.
And use EP1 to transfer datas from device to host in report3 and report 4.
>
> > +};
> > +
> > 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:
> lu...@denx.de
Best regards
Sherry sun
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
