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 -- Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform Group _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot