On Tuesday, February 04, 2014 at 06:02:38 PM, Mateusz Zalega wrote: > Preprocessor definitions and hardcoded implementation selection in > g_dnl core were replaced by a linker list made of (usb_function_name, > bind_callback) pairs. > > Signed-off-by: Mateusz Zalega <m.zal...@samsung.com> > Cc: Lukasz Majewski <l.majew...@samsung.com> > Cc: Marek Vasut <ma...@denx.de>
[...] > + > +/* export dfu_add to g_dnl.o */ > +ll_entry_declare(struct g_dnl_bind_callback, dfu_bind_callback, > + g_dnl_bind_callbacks) = { .usb_function_name = "usb_dnl_dfu", > + .fptr = dfu_add }; from linker-lists.h quote: 104 * ll_entry_declare() - Declare linker-generated array entry [...] 110 * This macro declares a variable that is placed into a linker-generated 111 * array. This is a basic building block for more advanced use of linker- 112 * generated arrays. The user is expected to build their own macro wrapper 113 * around this one. Can you follow this advice and build a macro for declaring these USB devices ? btw would you mind fixing the example for ll_entry_declare() in linker-lists.h ? It has four params in the example for some reason (it's a remnant from rework). [...] > +static inline struct g_dnl_bind_callback * g_dnl_first_bind_callback(void) > +{ > + return ll_entry_start(struct g_dnl_bind_callback, > + g_dnl_bind_callbacks); > +} > + > +static inline struct g_dnl_bind_callback * g_dnl_last_bind_callback(void) > +{ > + return ll_entry_end(struct g_dnl_bind_callback, > + g_dnl_bind_callbacks); > +} > + Are these two new functions called from multiple places at all? If not, just inline these ll_foo() calls and be done with it. FYI you can also make macros for these to avoid having to type all these args all around and duplicating the code. > static int g_dnl_do_config(struct usb_configuration *c) > { > const char *s = c->cdev->driver->name; > - int ret = -1; > > debug("%s: configuration: 0x%p composite dev: 0x%p\n", > - __func__, c, c->cdev); > - > + __func__, c, c->cdev); > printf("GADGET DRIVER: %s\n", s); > - if (!strcmp(s, "usb_dnl_dfu")) > - ret = dfu_add(c); > - else if (!strcmp(s, "usb_dnl_ums")) > - ret = fsg_add(c); > - else if (!strcmp(s, "usb_dnl_thor")) > - ret = thor_add(c); > - > - return ret; > + > + struct g_dnl_bind_callback *callback = g_dnl_first_bind_callback(); > + for (; callback != g_dnl_last_bind_callback(); ++callback) callback++ , this is not C++ where the order might matter. Nonetheless, you do want to use ll_entry_count() and ll_entry_get() with an iterator variable instead to make sure you don't step onto a corrupted field and crash in some weird way. > + if (!strcmp(s, callback->usb_function_name)) > + return callback->fptr(c); > + return -ENODEV; > } > > static int g_dnl_config_register(struct usb_composite_dev *cdev) > @@ -203,12 +210,12 @@ static int g_dnl_bind(struct usb_composite_dev *cdev) > device_desc.bcdDevice = cpu_to_le16(gcnum); > else { > debug("%s: controller '%s' not recognized\n", > - shortname, gadget->name); > + __func__, gadget->name); > device_desc.bcdDevice = __constant_cpu_to_le16(0x9999); > } > > - debug("%s: calling usb_gadget_connect for " > - "controller '%s'\n", shortname, gadget->name); > + debug("%s: calling usb_gadget_connect for controller '%s'\n", > + __func__, gadget->name); Please split all these cleanups into a separate patch. [...] _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot