On Sunday 01 November 2009 06:33:34 Wolfgang Grandegger wrote:
> + if (op == 's') {
> + else if (op == 'i') {
> + else if (op == 'r') {
> + } else if (op == 'x') {
> + } else {
your if style here is inconsistent, but ignoring that, shouldnt this really be
a switch() ? although, by only checking the first char, you allow people to
encode typos into their commands and not realize it until some point in the
future where things get stricter. i.e. people can do `can ilovecandy ...`
> + unsigned int dev_num = 0, ibaud = 0;
> + struct can_dev *dev;
> +
> + if (argc > 2)
> + dev_num = simple_strtoul (argv[2], NULL, 10);
> + if (argc > 3) {
> + ibaud = simple_strtoul (argv[3], NULL, 10);
> + if (ibaud > 2)
> + ibaud = 2;
> + }
> + dev = can_init (dev_num, ibaud);
> + if (!dev)
> + return 1;
> + can_dev = dev;
if i told CAN to init an unknown device, i would expect to get an error and
the command state to remain in said error state until i selected a proper CAN
device. otherwise, a script that didnt check the can init status would
incorrectly operate on the previously selected can device.
how do other commands work ? am i complaining about common convention here ?
> + printf ("Usage:\n%s\n", cmdtp->usage);
cmd_usage() ?
> + can, 3, 1, do_can,
> + "can - CAN bus commands\n",
> + "can status [level]\n"
> + "can init [dev] [baud-index]\n"
> + "can xmit [id] [d0] [d1] ... [d7]\n"
> + "can recv, abort with CTRL-C\n"
does the help really display correctly here ? i think the "can status" line
will have too many "can"'s ?
-mike
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
