On Friday 21 of September 2012 14:47:24 Marek Vasut wrote:
Dear Pavel Herrmann,
[...]
+static int init(struct core_instance *core)
I'd say, rename it to block_core_init() or something, so the syms in u-boot.map are unique.
thic being static, how could it show in u-boot.map?
Argh, not u-boot.map, sorry. But it's much easier for git grep to look up unique syms than this.
ah, OK then
+{
- INIT_LIST_HEAD(&core->succ);
- core->private_data = NULL;
- return 0;
+}
+static int reloc(struct core_instance *core, struct core_instance *old) +{
- struct blockdev_core_entry *entry, *new;
- /* no private_data to copy, yet */
- /* fixup links in old list and prepare new list head */
- /* FIXME */
- /* list_fix_reloc(&old->succ); */
- INIT_LIST_HEAD(&core->succ);
- core->private_data = NULL;
- /* copy list entries to new memory */
- list_for_each_entry(entry, &old->succ, list) {
new = malloc(sizeof(*new));if (!new)return -ENOMEM;INIT_LIST_HEAD(&new->list);new->instance = entry->instance;new->ops = entry->ops;new->name = entry->name;list_add_tail(&new->list, &core->succ);/*no free at this point, old memory should not be freed*/- }
- return 0;
+}
+static int destroy(struct core_instance *core) +{
- struct blockdev_core_entry *entry, *n;
- /* destroy private data */
- free(core->private_data);
- core->private_data = NULL;
- /* destroy successor list */
- list_for_each_entry_safe(entry, n, &core->succ, list) {
list_del(&entry->list);free(entry);- }
- return 0;
+}
+U_BOOT_CORE(CORE_BLOCKDEV,
- init,
- reloc,
- destroy,
- get_count,
- get_child,
- bind,
- unbind,
- replace);
Sep the stuff below away into separate file. Conditionally compile in one or the other.
I distinctly remember you saying to put all this into one file (as opposed to 3 it was before), so why the turn-around now?
Well, I didn't see the code before, so I couldn't make a firm decision, sorry.
No idea what you mean by "one or the other" - you need all this code for it to work.
You need the "driver wrapping API" if DM is enabled? I do not understand this, please elaborate!
What about having a common part for both cases and then compile either the DM part or non-DM part conditionally?
this is all DM-only. driver-wrapping API is wrapper/proxy functions to driver ops (aka it wraps around the ops, doing lookup and activate and whatnot), not a wrapper for old API (which would be in a driver mind you)
+/* Driver wrapping API */ +lbaint_t blockdev_read(struct instance *i, lbaint_t start, lbaint_t blkcnt, + void *buffer) +{
- struct blockdev_core_entry *entry = NULL;
- struct blockdev_ops *device_ops = NULL;
- int error;
- entry = get_entry_by_instance(i);
- if (!entry)
return -ENOENT;- error = driver_activate(i);
- if (error)
return error;- device_ops = entry->ops;
- if (!device_ops || !device_ops->read)
return -EINVAL;- return device_ops->read(i, start, blkcnt, buffer);
+}
Pavel Herrmann
Best regards, Marek Vasut