Dear Lukasz Majewski,
Support for f_dfu USB function.
Signed-off-by: Lukasz Majewski l.majewski@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com Cc: Marek Vasut marex@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 :-(