[U-Boot] [PATCH 1/1] at91: Add command to control up to 3 GPIO LEDs from the console

[Daniel Gorsulowski]

Hi Stefan,

Stefan Roese wrote:
> Hi Daniel,
> 
> On Wednesday 06 May 2009, Daniel Gorsulowski wrote:
>> This patch allows any at91 board, implementing the GPIO LED API,
>> to control the LEDs from the console.
>>
>> led [ 1 | 2 | 3 | all ]  [ on | off ]
> 
> Why limit this to a max of 3 LED's? If this is a generic command (which I like 
> btw) then we should support a user/board defined number of LED's. In your case 
> it's 3, but the infrastructure should support any number.
> 
> More comments below.
> 
> <snip>
> 
...
> 
> I suggest to use something like this here:
> 
> 	led_nr = simple_strtoul(argv[1], NULL, 10);
> 	if (led_nr > CONFIG_LED_MAX) {
> 		printf ("Usage:\n%s\n", cmdtp->usage);
> 		return 1;
> 	}
> 
> 	if (strcmp(argv[2], "off") == 0) {
> 		on = 1;
> 	} else if (strcmp(argv[2], "on") == 0) {
> 		on = 0;
> 	} else {
> 		printf ("Usage:\n%s\n", cmdtp->usage);
> 		return 1;
> 	}
> 
> 	user_led(led_nr, on);
> 
> No ugly #ifdef's in this case. What do you think?
> 
> Best regards,
> Stefan
> 
I agree with you.
Please give me some days, to implement your basic approaches.
I've many other things to do and it's not that easy (for me)
to create a tidy patch.

Best regards,
Daniel