Dear Lukasz Majewski, > Support for f_dfu USB function. > > Signed-off-by: Lukasz Majewski <l.majew...@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.p...@samsung.com> > Cc: Marek Vasut <ma...@denx.de> > --- [...] > + > +static const char dfu_name[] = "Device Firmware Upgrade"; > + > +/* static strings, in UTF-8 */ > +/* > + * dfu_genericiguration specific
generi....what? > + */ > +static struct usb_string strings_dfu_generic[] = { > + [0].s = dfu_name, > + { } /* end of list */ > +}; > + > +static struct usb_gadget_strings stringtab_dfu_generic = { > + .language = 0x0409, /* en-us */ > + .strings = strings_dfu_generic, > +}; > + > +static struct usb_gadget_strings *dfu_generic_strings[] = { > + &stringtab_dfu_generic, > + NULL, > +}; > + > +/* > + * usb_function specific > + */ > +static struct usb_gadget_strings stringtab_dfu = { > + .language = 0x0409, /* en-us */ > + /* > + * .strings > + * > + * assigned during initialization, > + * depends on number of flash entities > + * > + */ > +}; > + > +static struct usb_gadget_strings *dfu_strings[] = { > + &stringtab_dfu, > + NULL, > +}; > + > +/*------------------------------------------------------------------------ > -*/ + > +static void dnload_request_complete(struct usb_ep *ep, struct usb_request > *req) +{ > + struct f_dfu *f_dfu = req->context; > + > + dfu_write(dfu_get_entity(f_dfu->altsetting), req->buf, > + req->length, f_dfu->blk_seq_num); > + > + if (req->length == 0) { > + puts("DOWNLOAD ... OK\n"); > + puts("Ctrl+C to exit ...\n"); > + } > +} > + > +static void handle_getstatus(struct usb_request *req) > +{ > + struct dfu_status *dstat = (struct dfu_status *)req->buf; > + struct f_dfu *f_dfu = req->context; > + > + switch (f_dfu->dfu_state) { > + case DFU_STATE_dfuDNLOAD_SYNC: What's this crazy cammel case in here ? :-) > + case DFU_STATE_dfuDNBUSY: > + f_dfu->dfu_state = DFU_STATE_dfuDNLOAD_IDLE; > + break; > + case DFU_STATE_dfuMANIFEST_SYNC: > + break; > + default: > + break; > + } > + > + /* send status response */ > + dstat->bStatus = f_dfu->dfu_status; > + /* FIXME: set dstat->bwPollTimeout */ FIXME ... so fix it ;-) > + dstat->bState = f_dfu->dfu_state; > + dstat->iString = 0; > +} > + > +static void handle_getstate(struct usb_request *req) > +{ > + struct f_dfu *f_dfu = req->context; > + > + ((u8 *)req->buf)[0] = f_dfu->dfu_state & 0xff; Now ... this is ubercrazy ... can't this be made without this typecasting voodoo? > + req->actual = sizeof(u8); > +} > + [...] > +static int handle_dnload(struct usb_gadget *gadget, u16 len) > +{ > + struct usb_composite_dev *cdev = get_gadget_data(gadget); > + struct usb_request *req = cdev->req; > + struct f_dfu *f_dfu = req->context; > + > + if (len == 0) > + f_dfu->dfu_state = DFU_STATE_dfuMANIFEST_SYNC; > + > + req->complete = dnload_request_complete; > + > + return len; > +} One newline too much below. > + > + > +static int > +dfu_handle(struct usb_function *f, const struct usb_ctrlrequest *ctrl) > +{ > + struct usb_gadget *gadget = f->config->cdev->gadget; > + struct usb_request *req = f->config->cdev->req; > + struct f_dfu *f_dfu = f->config->cdev->req->context; > + u16 len = le16_to_cpu(ctrl->wLength); > + u16 w_value = le16_to_cpu(ctrl->wValue); > + int value = 0; > + int standard; > + > + standard = (ctrl->bRequestType & USB_TYPE_MASK) > + == USB_TYPE_STANDARD; > + > + debug("w_value: 0x%x len: 0x%x\n", w_value, len); > + debug("standard: 0x%x ctrl->bRequest: 0x%x f_dfu->dfu_state: 0x%x\n", > + standard, ctrl->bRequest, f_dfu->dfu_state); > + > + if (!standard) { This function doesn't fit on my screen ... that means it's waaaay too long and shall be split into more functions ;-) That's also remove the need for your crazy conditional assignment of standard. > + switch (f_dfu->dfu_state) { > + case DFU_STATE_appIDLE: > + switch (ctrl->bRequest) { > + case USB_REQ_DFU_GETSTATUS: > + handle_getstatus(req); > + value = RET_STAT_LEN; > + break; [...] > + > +/*------------------------------------------------------------------------ > -*/ + > +static int > +dfu_prepare_strings(struct f_dfu *f_dfu, int n) > +{ > + struct dfu_entity *de = NULL; > + int i = 0; > + > + f_dfu->strings = calloc(((n + 1) * sizeof(struct usb_string)), 1); calloc(n, 1) ... that's the same as malloc(), isn't it ? > + if (!f_dfu->strings) > + goto enomem; > + > + for (i = 0; i < n; ++i) { > + de = dfu_get_entity(i); > + f_dfu->strings[i].s = de->name; > + } > + > + f_dfu->strings[i].id = 0; > + f_dfu->strings[i].s = NULL; > + > + return 0; > + > +enomem: > + while (i) > + f_dfu->strings[--i].s = NULL; > + > + kfree(f_dfu->strings); > + > + return -ENOMEM; > +} > + > +static int dfu_prepare_function(struct f_dfu *f_dfu, int n) > +{ > + struct usb_interface_descriptor *d; > + int i = 0; > + > + f_dfu->function = calloc(n * sizeof(struct usb_descriptor_header *), 1); DITTO > + if (!f_dfu->function) > + goto enomem; > + > + for (i = 0; i < n; ++i) { > + d = kzalloc(sizeof(*d), GFP_KERNEL); Why use the kernel alternatives ? just use malloc()/free() ? > + if (!d) > + goto enomem; > + > + d->bLength = sizeof(*d); > + d->bDescriptorType = USB_DT_INTERFACE; > + d->bAlternateSetting = i; > + d->bNumEndpoints = 0; > + d->bInterfaceClass = USB_CLASS_APP_SPEC; > + d->bInterfaceSubClass = 1; > + d->bInterfaceProtocol = 2; > + > + f_dfu->function[i] = (struct usb_descriptor_header *)d; > + } > + f_dfu->function[i] = NULL; > + > + return 0; > + > +enomem: > + while (i) { > + kfree(f_dfu->function[--i]); > + f_dfu->function[i] = NULL; > + } > + kfree(f_dfu->function); > + > + return -ENOMEM; > +} > + > +static int dfu_bind(struct usb_configuration *c, struct usb_function *f) > +{ > + struct usb_composite_dev *cdev = c->cdev; > + struct f_dfu *f_dfu = func_to_dfu(f); > + int alt_num = dfu_get_alt_number(); > + int rv, id, i; > + > + id = usb_interface_id(c, f); > + if (id < 0) > + return id; > + dfu_intf_runtime.bInterfaceNumber = id; > + > + f_dfu->dfu_state = DFU_STATE_appIDLE; > + f_dfu->dfu_status = DFU_STATUS_OK; > + > + rv = dfu_prepare_function(f_dfu, alt_num); > + if (rv) > + goto error; > + > + rv = dfu_prepare_strings(f_dfu, alt_num); > + if (rv) > + goto error; > + for (i = 0; i < alt_num; i++) { > + id = usb_string_id(cdev); > + if (id < 0) > + return id; > + f_dfu->strings[i].id = id; > + ((struct usb_interface_descriptor *)f_dfu->function[i]) > + ->iInterface = id; > + } > + > + stringtab_dfu.strings = f_dfu->strings; > + > + cdev->req->context = f_dfu; > + > + return 0; > +error: > + return rv; So just return the retval ... > +} > + Every time I review patches and find multiple small issues, I feel bad :-( _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot