[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@beagleboard.org>

From: u-boot-bounces@lists.denx.de
[mailto:u-boot-bounces@lists.denx.de] On Behalf Of Jason Kridner
Sent: Friday, November 05, 2010 11:23 AM
To: u-boot@lists.denx.de; beagleboard@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@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@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?
     Can this be simplified by one function that takes an argument?
     Should reduce code and ifdefs - if at all - get confined to one function.

~sanjeev

From: u-boot-bounces@lists.denx.de
[mailto:u-boot-bounces@lists.denx.de] On Behalf Of Jason Kridner
Sent: Friday, November 05, 2010 11:23 AM
To: u-boot@lists.denx.de; beagleboard@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@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@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?

Dear Jason Kridner,