Dear Mike,
On Tuesday 31 July 2012 02:36:58 Lukasz Majewski wrote:
--- /dev/null +++ b/drivers/usb/gadget/f_dfu.c
+static struct usb_interface_descriptor dfu_intf_runtime = {
can this be made const ?
Unfortunately those structs cannot be const, since some of their fields are filled during running DFU code(when switching between "normal" and "dfu" mode).
+static struct usb_descriptor_header *dfu_runtime_descs[] = {
- (struct usb_descriptor_header *) &dfu_intf_runtime,
can you change the descs array to be const ? static const struct usb_descriptor_header * const dfu_runtime_descs[] = {
The same as above. Moreover the DFU implementation passes the information about available alt settings as descriptors filled at runtime.
then i think you can drop the cast there ...
+static struct usb_qualifier_descriptor dev_qualifier = {
Struct above can only be defined as const.
+static struct usb_gadget_strings stringtab_dfu_generic = { +static struct usb_gadget_strings *dfu_generic_strings[] = { +static struct usb_gadget_strings stringtab_dfu = { +static struct usb_gadget_strings *dfu_strings[] = {
can these be made const ?
+static void handle_getstate(struct usb_request *req) +{
- struct f_dfu *f_dfu = req->context;
- ((u8 *)req->buf)[0] = f_dfu->dfu_state & 0xff;
pretty sure you don't need that "& 0xff"
OK.
+static int state_app_idle(struct f_dfu *f_dfu,
const struct usb_ctrlrequest *ctrl,struct usb_gadget *gadget,struct usb_request *req)+{
- int value = 0;
might be good to push this down into the 1 case statement (USB_REQ_DFU_GETSTATE) that uses it rather than init it up top
I haven't understood this comment.
Do you suggest to combine all those functions (like state_app_* or state_dfu_*) in one big switch - case clause? The state machine was initially implemented in this way, but for sake of clearness Marek Vasut asked to split it down do separate functions as defined at dfu_state[]. I also think that this approach is better than one bit switch - case.
same goes for all the other funcs below that follow this style -mike