Mike Frysinger wrote:
> 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 ...`
Will fix.
>> + 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.
can_init will already print an error message. But that might be changed.
> how do other commands work ? am i complaining about common convention here ?
>
>> + printf ("Usage:\n%s\n", cmdtp->usage);
>
> cmd_usage() ?
OK.
>> + 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 ?
I think the output was OK. But I will check later next week.
Note that this is a RFC trying to discuss the real requirements of a CAN
interface in U-Boot. I think it would also be nice to have can_xmit()
and can_recv() with a timeout parameter, e.g.:
can_xmit(struct can_dev *dev, int timeout_us);
And maybe also a can_xmit_done() function.
Wolfgang.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
