On Friday, November 05, 2010 01:50:36 Jason Kridner wrote: > +int do_led ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[] )
much of the style of this code is broken. and i cant imagine this code
compiling warning free with current master.
no spaces around the paren, and the argv has been constified.
also, this should be marked static
> + if ((argc != 3)){
space before the brace and useless set of paren here
> + printf("Usage:\n%s\n", cmdtp->usage);
> + return 1;
return cmd_usage(cmdtp);
> + if (strcmp(argv[2], "off") == 0) {
> + state = 0;
> + } else if (strcmp(argv[2], "on") == 0) {
> + state = 1;
i could have sworn we had a helper somewhere to handle "boolean strings" ...
> + printf ("Usage:\n%s\n", cmdtp->usage);
> + return 1;
return cmd_usage(cmdtp);
> +#if defined(STATUS_LED_BIT) && defined(CONFIG_BOARD_SPECIFIC_LED)
> + if (strcmp(argv[1], "0") == 0) {
> + mask = STATUS_LED_BIT;
> + __led_set(mask, state);
> + }
> + else
> +#endif
> +#if defined(STATUS_LED_BIT1) && defined(CONFIG_BOARD_SPECIFIC_LED)
> + if (strcmp(argv[1], "1") == 0) {
> + mask = STATUS_LED_BIT1;
> + __led_set(mask, state);
> + }
> + else
> +#endif
> +#if defined(STATUS_LED_BIT2) && defined(CONFIG_BOARD_SPECIFIC_LED)
> + if (strcmp(argv[1], "2") == 0) {
> + mask = STATUS_LED_BIT2;
> + __led_set(mask, state);
> + }
> + else
> +#endif
> +#if defined(STATUS_LED_BIT3) && defined(CONFIG_BOARD_SPECIFIC_LED)
> + if (strcmp(argv[1], "3") == 0) {
> + mask = STATUS_LED_BIT3;
> + __led_set(mask, state);
> + }
> + else
> +#endif
i dont know why you need the mask variable here at all
also, these #ifdef trees scream for some sort of unification
> + } else {
> + printf ("Usage:\n%s\n", cmdtp->usage);
> + return 1;
return cmd_usage(cmptp);
> +
files should not have trailing new lines
-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
