On 02/05/14 08:13, Marek Vasut wrote:
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.zalega@samsung.com Cc: Lukasz Majewski l.majewski@samsung.com Cc: Marek Vasut marex@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).
Yup
[...]
+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.
Macros or static inlines, it's all the same, there's no point in changing that. The symbols aren't visible outside this compilation unit and function calls are, well, inlined.
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);
- 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
It doesn't matter anyway and can't be supported on Coding Style grounds, don't bug me.
want to use ll_entry_count() and ll_entry_get() with an iterator variable
I don't think using ll_entry_get() in this way is possible with current implementation of linker lists. Moreover, there's plenty of code doing just the same already accepted to U-Boot.
instead to make sure you don't step onto a corrupted field and crash in some weird way.
Linker would have to split sections to make this sort of thing possible. Won't happen.
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);
- 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.
Right, I'll post v3.
[...]
Regards,