On Fri, Nov 5, 2010 at 8:02 AM, Premi, Sanjeev <pr...@ti.com> wrote: >> -----Original Message----- >> From: u-boot-boun...@lists.denx.de >> [mailto:u-boot-boun...@lists.denx.de] On Behalf Of Jason Kridner >> Sent: Friday, November 05, 2010 11:23 AM >> To: u-boot@lists.denx.de; beaglebo...@googlegroups.com >> Subject: [U-Boot] [PATCH] BeagleBoard: Added LED driver >> >> Added LED driver using status_led. USR0 is set to monitor the boot >> status. USR1 is set to be the green LED. >> (cherry picked from commit 048b526fd7cc0c642f27c674b3e235321c880b66) >> >> Signed-off-by: Jason Kridner <jkrid...@beagleboard.org> >> --- >> board/ti/beagle/Makefile | 4 ++- >> board/ti/beagle/beagle.c | 7 ++++ >> board/ti/beagle/led.c | 91 >> ++++++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 101 insertions(+), 1 deletions(-) >> create mode 100644 board/ti/beagle/led.c >> >> diff --git a/board/ti/beagle/Makefile b/board/ti/beagle/Makefile >> index f797112..4cc675c 100644 >> --- a/board/ti/beagle/Makefile >> +++ b/board/ti/beagle/Makefile >> @@ -25,8 +25,10 @@ include $(TOPDIR)/config.mk >> >> LIB = $(obj)lib$(BOARD).a >> >> -COBJS := beagle.o >> +COBJS-y := $(BOARD).o >> +COBJS-$(CONFIG_STATUS_LED) += led.o >> >> +COBJS := $(sort $(COBJS-y)) >> SRCS := $(COBJS:.o=.c) >> OBJS := $(addprefix $(obj),$(COBJS)) >> >> diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c >> index 93c452e..dd7b6b5 100644 >> --- a/board/ti/beagle/beagle.c >> +++ b/board/ti/beagle/beagle.c >> @@ -30,6 +30,9 @@ >> * MA 02111-1307 USA >> */ >> #include <common.h> >> +#ifdef CONFIG_STATUS_LED >> +#include <status_led.h> >> +#endif >> #include <twl4030.h> >> #include <asm/io.h> >> #include <asm/arch/mmc_host_def.h> >> @@ -78,6 +81,10 @@ int board_init(void) >> /* boot param addr */ >> gd->bd->bi_boot_params = (OMAP34XX_SDRC_CS0 + 0x100); >> >> +#if defined(CONFIG_STATUS_LED) && defined(STATUS_LED_BOOT) >> + status_led_set (STATUS_LED_BOOT, STATUS_LED_ON); >> +#endif >> + >> return 0; >> } >> >> diff --git a/board/ti/beagle/led.c b/board/ti/beagle/led.c >> new file mode 100644 >> index 0000000..df26552 >> --- /dev/null >> +++ b/board/ti/beagle/led.c >> @@ -0,0 +1,91 @@ >> +/* >> + * Copyright (c) 2010 Texas Instruments, Inc. >> + * Jason Kridner <jkrid...@beagleboard.org> >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License as >> + * published by the Free Software Foundation; either version 2 of >> + * the License, or (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, write to the Free Software >> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, >> + * MA 02111-1307 USA >> + */ >> +#include <common.h> >> +#include <status_led.h> >> +#include <asm/arch/cpu.h> >> +#include <asm/io.h> >> +#include <asm/arch/sys_proto.h> >> +#include <asm/arch/gpio.h> >> + >> +static unsigned int saved_state[2] = {STATUS_LED_OFF, >> STATUS_LED_OFF}; >> + >> +/* GPIO pins for the LEDs */ >> +#define BEAGLE_LED_USR0 149 >> +#define BEAGLE_LED_USR1 150 >> + >> +#ifdef STATUS_LED_GREEN >> +void green_LED_off (void) >> +{ >> + __led_set (STATUS_LED_GREEN, 0); >> +} >> + >> +void green_LED_on (void) >> +{ >> + __led_set (STATUS_LED_GREEN, 1); >> +} >> +#endif >> + >> +void __led_init (led_id_t mask, int state) >> +{ >> + __led_set (mask, state); >> +} >> + >> +void __led_toggle (led_id_t mask) >> +{ >> +#ifdef STATUS_LED_BIT >> + if (STATUS_LED_BIT & mask) { >> + if (STATUS_LED_ON == saved_state[0]) >> + __led_set(STATUS_LED_BIT, 0); >> + else >> + __led_set(STATUS_LED_BIT, 1); >> + } >> +#endif >> +#ifdef STATUS_LED_BIT1 >> + if (STATUS_LED_BIT1 & mask) { >> + if (STATUS_LED_ON == saved_state[1]) >> + __led_set(STATUS_LED_BIT1, 0); >> + else >> + __led_set(STATUS_LED_BIT1, 1); >> + } >> +#endif >> +} >> + >> +void __led_set (led_id_t mask, int state) >> +{ >> +#ifdef STATUS_LED_BIT >> + if (STATUS_LED_BIT & mask) { >> + if (!omap_request_gpio(BEAGLE_LED_USR0)) { >> + omap_set_gpio_direction(BEAGLE_LED_USR0, 0); >> + omap_set_gpio_dataout(BEAGLE_LED_USR0, state); >> + } >> + saved_state[0] = state; >> + } >> +#endif >> +#ifdef STATUS_LED_BIT1 >> + if (STATUS_LED_BIT1 & mask) { >> + if (!omap_request_gpio(BEAGLE_LED_USR1)) { >> + omap_set_gpio_direction(BEAGLE_LED_USR1, 0); >> + omap_set_gpio_dataout(BEAGLE_LED_USR1, state); >> + } >> + saved_state[1] = state; >> + } >> +#endif >> +} > > [sp] I see too many ifdef blocks in the code above. The also seems to be > repetitive. > > Is user really expected to change the u-boot config for each LED > bit/color?
This is the existing architecture in status_led.h. I want to make sure this code compiles no matter which of the defines have been set. > Can this be simplified by one function that takes an argument? Do you mean set vs. toggle? I think that would be great too, but again following an existing definition. > Should reduce code and ifdefs - if at all - get confined to one function. Do you have any proposal on how to do so? Personally, I'd be inclined to eliminate the colors all together and have 2 values that can be ignored/rejected by the functions if not supported: LED # (an ID) and an integral value (could be brightness or color). Also, I'd be happy with simply having a set value function and having the rest of the system monitor the state (rather than having a toggle method). Thoughts? > > ~sanjeev > >> + >> -- >> 1.5.6.4 >> >> _______________________________________________ >> U-Boot mailing list >> U-Boot@lists.denx.de >> http://lists.denx.de/mailman/listinfo/u-boot >> _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
