Hi Frans,

It was hinted (iirc by Detlev) that I'd have a stab at subcmd handling (at least for i2c).

Yep.

As example bootm was suggested.

This was only one example, there are more usages of this. Good old grep will be your friend.

I've peeked at the bootm code and the code in command.c. It seems bootm has some internal state engine, that is somewhat less applicable for other commands.

This doesn't really matter. If you want an easier example, check board/inka4x0/inkadiag.c and do_inkadiag in particular.

My proposal for i2c:

have an array static cmd_tbl_t cmd_i2c_sub[] = { }; whiich is populated/initialized by U_BOOT_CMD_MKENT macro invocations for the various subcommands.

The function do_i2c which now handles the subcmd parsing changes into:

int do_i2c(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) { cmd_tbl_t *c;

    /* Strip off leading 'i2c' command argument */
    argc--;
    argv++;

    c = find_cmd_tbl(argv[0], &cmd_i2c_sub[0], ARRAY_SIZE(cmd_i2c_sub));

if (c) {
   return  c->cmd(cmdtp, flag, argc, argv);
}
else

{ cmd_usage(cmdtp); return 0;

}

}

Yep, looks roughly comparable to the example I pointed out above.

(and sorry for the messy layout)

A few questions before I actually implement and test this:

any comments on this proposal ?

What should be the return value in case the usage is printed? do_i2c returns 0 in that case however do_bootm_subcommand returns 1 in that case.

As the command failed, 1 should be returned. I would consider the 0 return of do_i2c to be a (minor) bug.

We might also consider putting the help texts in U_BOOT_CMD_MKENT. Then U_BOOT_CMD should probably be aware of the child commands and should use the help of the child commands to print its help.

Hm? This is one of the reasons for using this infrastructure. Somehow I feel I do not understand your question.

The first entry in U_BOOT_CMD_MKENT is the name. This is stringified by using a # in the macro. Personally I would like to change it and have string quotes in the macro invocation so that it is immediately visible when reading the code that it is a string, not e.g. a variable. E.g. we would get U_BOOT_CMD_MKENT("start", ... instead of U_BOOT_CMD_MKENT(start, ... what we have now where for the casual reader not up to date on the definition of the macro it might look as if a var is passed. (btw passing a string could also allow for passing an empty string, which could be used to add non-command assoicated help texts).

As I have no plans to implement this more than once, I would appreciate feedback before starting to actually code it.

Do you feel that thi slight difference is worth the effort? I guess it is not clear to me what you want to change. You could try to show us only a protoitype of your proposed change and then we can comment more sensibly.

Thanks Detlev