[U-Boot] [PATCH] BeagleBoard: Added LED driver
Jason Kridner
jkridner at beagleboard.org
Fri Nov 5 18:32:59 CET 2010
On Fri, Nov 5, 2010 at 8:02 AM, Premi, Sanjeev <premi at ti.com> wrote:
>> -----Original Message-----
>> From: u-boot-bounces at lists.denx.de
>> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Jason Kridner
>> Sent: Friday, November 05, 2010 11:23 AM
>> To: u-boot at lists.denx.de; beagleboard at 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 <jkridner at 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 <jkridner at 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 at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>>
More information about the U-Boot
mailing list