From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Date: Sat, 5 Sep 2009 01:47:56 +0200
Subject: [U-Boot] [PATCH] AT91: Add support for blue_LED_* and add
 coloured_LED_init to at91/led.c
In-Reply-To: <20090820083619.GB5086@pc-ras4041.res.insa>
References: <1250093434-6605-1-git-send-email-albin.tonnerre@free-electrons.com>
	<20090812211526.GG21725@game.jcrosoft.org>
	<20090812213900.GD4051@pc-ras4041.res.insa>
	<20090817225148.GB4309@game.jcrosoft.org>
	<20090818084958.GD4800@pc-ras4041.res.insa>
	<20090820000045.GN8470@game.jcrosoft.org>
	<20090820083619.GB5086@pc-ras4041.res.insa>
Message-ID: <20090904234756.GY30118@game.jcrosoft.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 10:36 Thu 20 Aug     , Albin Tonnerre wrote:
> On Thu, Aug 20, 2009 at 02:00:45AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote :
> > On 10:49 Tue 18 Aug     , Albin Tonnerre wrote:
> > > On Tue, Aug 18, 2009 at 12:51:48AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote :
> > > > no please take a look on the other LED thread
> > > 
> > > Would you please provide a pointer to this thread ? THe only one remotely
> > > related I can find is
> > > http://lists.denx.de/pipermail/u-boot/2009-May/052160.html, and you did not
> > > participate in this one ...
> > I've as I'm the one who ask Daniel Gorsulowski to base his new code (this patch)
> > against Ulf precedent patch and as other people have done the same comment as I will do
> > no need to repeat it
> 
> Ok, so I'm really confused now. This patch does exactly what you're arguing
> against in the rest of your mail, and you still don't provide a pointer to Ulf's
> patch.
> Would you mind *explaining* to me what your plan is? I just can't get it.
something like this for assembly

.macro set_led \num, \state
	call (c or assembly) set_led_state num state
.endm

.macore set_led_red \state
	call (c or assembly) set_led_state CONFIG_SYS_LED_RED state
.endm

and for c

void set_led(int num, int state)
{
	....
}

void set_reg_led(int state)
{
	set_led(CONFIG_SYS_LED_RED, state);
}

etc... for all colour

maybe an array could help

struct leds [] = {
#ifdef CONFIG_SYS_LED_RED
	{
		.name = "red",
		.num = CONFIG_SYS_LED_RED,
	},
#endif
#ifdef CONFIG_SYS_LED_GREEN
	{
		.name = "green",
		.num = CONFIG_SYS_LED_GREEN,
	},
#endif
};

so we can create a command to manage them

static int do_set_led(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
{
	int i;

	if argc[1] is num && < array_size of leds
	then
		set_led(num, argv[2]);
	else if argc[1] is a colour led
	then
		set_led(get_led_num(argc[1]), argc[2]);
	fi

}

so this will not be arm specific anymore

Best Regards,
J.