mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* LED framework
@ 2010-12-18 15:15 Sascha Hauer
  2010-12-18 15:15 ` [PATCH 1/7] Add generic poll infrastructure Sascha Hauer
                   ` (6 more replies)
  0 siblings, 7 replies; 21+ messages in thread
From: Sascha Hauer @ 2010-12-18 15:15 UTC (permalink / raw)
  To: barebox

Hi All,

The following series adds generic LED support for barebox.
Included is an infrastructure to register gpios as LEDs and
generic triggers for LEDs (Heartbeat, network activity). LEDs and
triggers can be controlled in C code and also on the command line.

The last patch only contains a small usage example, I'm not going
to commit it.

I have implemented this patchset completely remote with printfs in
the gpio functions, so I haven't seen any LED blinking. I hope that
the triggers are not too short for a LED to actually blink.

Sascha

The following changes since commit fad11e85641d27f683e2866ec5700cafd5868179:

  eth: fix 'warning: No MAC address set' when using EEPROM MAC (2010-12-16 08:30:14 +0100)

are available in the git repository at:
  git://git.pengutronix.de/git/barebox.git led

Marc Kleine-Budde (1):
      Add generic poll infrastructure

Sascha Hauer (6):
      basic LED support
      LED: Add gpio LED support
      LED: Add LED trigger support
      LED: Add led command
      LED: Add trigger command
      pcm038: led testing. Not to be committed

 arch/arm/boards/pcm038/pcm038.c |   24 ++++++++-
 commands/Kconfig                |   15 +++++
 commands/Makefile               |    2 +
 commands/led.c                  |   60 ++++++++++++++++++++
 commands/trigger.c              |  106 ++++++++++++++++++++++++++++++++++
 common/Kconfig                  |    3 +
 common/Makefile                 |    1 +
 common/console.c                |    5 ++
 common/poller.c                 |   45 +++++++++++++++
 drivers/Kconfig                 |    1 +
 drivers/Makefile                |    1 +
 drivers/led/Kconfig             |   13 ++++
 drivers/led/Makefile            |    3 +
 drivers/led/core.c              |  119 +++++++++++++++++++++++++++++++++++++++
 drivers/led/led-gpio.c          |   92 ++++++++++++++++++++++++++++++
 include/led.h                   |   69 ++++++++++++++++++++++
 include/net.h                   |    3 +
 include/poller.h                |   31 ++++++++++
 lib/vsprintf.c                  |    4 +
 net/eth.c                       |   20 ++++++-
 net/net.c                       |   21 +++++--
 21 files changed, 631 insertions(+), 7 deletions(-)
 create mode 100644 commands/led.c
 create mode 100644 commands/trigger.c
 create mode 100644 common/poller.c
 create mode 100644 drivers/led/Kconfig
 create mode 100644 drivers/led/Makefile
 create mode 100644 drivers/led/core.c
 create mode 100644 drivers/led/led-gpio.c
 create mode 100644 include/led.h
 create mode 100644 include/poller.h


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/7] Add generic poll infrastructure
  2010-12-18 15:15 LED framework Sascha Hauer
@ 2010-12-18 15:15 ` Sascha Hauer
  2010-12-18 15:28   ` Sascha Hauer
  2010-12-18 15:15 ` [PATCH 2/7] basic LED support Sascha Hauer
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Sascha Hauer @ 2010-12-18 15:15 UTC (permalink / raw)
  To: barebox

From: Marc Kleine-Budde <mkl@pengutronix.de>

Barebox does not have interrupt functionality. Nevertheless it's
sometimes useful to periodically call functions, like for example
a heartbeat LED or watchdog reset. Instead of cluttering the code
with calls to these functions this patch adds a generic polling
infrastructure. Code which might run for longer now can call
poller_call() periodically which in turn will call all registered
pollers.
This patch adds a call to poller_call in two generic pathes. First
of them is getc() which covers waiting for uart input. Second is
ctrlc() which should be called anyway from code which might run
for longer. So instead adding poller_call directly to your code,
consider checking ctrlc instead which also gives additional
convenience to the user.
The poller code is safe against reentrancy which means that it's
safe to call poller_call inside a poller.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/Kconfig   |    3 +++
 common/Makefile  |    1 +
 common/console.c |    5 +++++
 common/poller.c  |   45 +++++++++++++++++++++++++++++++++++++++++++++
 include/poller.h |   31 +++++++++++++++++++++++++++++++
 5 files changed, 85 insertions(+), 0 deletions(-)
 create mode 100644 common/poller.c
 create mode 100644 include/poller.h

diff --git a/common/Kconfig b/common/Kconfig
index 617f640..02bc67e 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -414,6 +414,9 @@ config DEFAULT_ENVIRONMENT_PATH
 	  Relative pathes will be relative to the barebox Toplevel dir, but absolute
 	  pathes are fine aswell.
 
+config POLLER
+	bool "generic polling infrastructure"
+
 endmenu
 
 menu "Debugging                     "
diff --git a/common/Makefile b/common/Makefile
index 753455b..98c9d36 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_OF_FLAT_TREE)	+= ft_build.o
 obj-$(CONFIG_KALLSYMS)		+= kallsyms.o
 obj-$(CONFIG_ENV_HANDLING)	+= environment.o
 obj-$(CONFIG_AUTO_COMPLETE)	+= complete.o
+obj-$(CONFIG_POLLER)		+= poller.o
 
 obj-y += dlmalloc.o
 obj-y += clock.o
diff --git a/common/console.c b/common/console.c
index 82786f2..39ead4b 100644
--- a/common/console.c
+++ b/common/console.c
@@ -34,6 +34,7 @@
 #include <clock.h>
 #include <kfifo.h>
 #include <module.h>
+#include <poller.h>
 #include <linux/list.h>
 
 LIST_HEAD(console_list);
@@ -205,6 +206,8 @@ int getc(void)
 	 */
 	start = get_time_ns();
 	while (1) {
+		poller_call();
+
 		if (tstc()) {
 			kfifo_putc(console_input_buffer, getc_raw());
 
@@ -397,6 +400,8 @@ EXPORT_SYMBOL(vprintf);
 /* test if ctrl-c was pressed */
 int ctrlc (void)
 {
+	poller_call();
+
 	if (tstc() && getc() == 3)
 		return 1;
 	return 0;
diff --git a/common/poller.c b/common/poller.c
new file mode 100644
index 0000000..0583a53
--- /dev/null
+++ b/common/poller.c
@@ -0,0 +1,45 @@
+/*
+ * Copyright (C) 2010 Marc Kleine-Budde <mkl@pengutronix.de>
+ *
+ * This file is released under the GPLv2
+ *
+ */
+
+#include <common.h>
+#include <driver.h>
+#include <malloc.h>
+#include <module.h>
+#include <param.h>
+#include <poller.h>
+
+static LIST_HEAD(poller_list);
+static int poller_active;
+
+int poller_register(struct poller_struct *poller)
+{
+	list_add_tail(&poller->list, &poller_list);
+
+	return 0;
+}
+
+int poller_unregister(struct poller_struct *poller)
+{
+	list_del(&poller->list);
+
+	return 0;
+}
+
+void poller_call(void)
+{
+	struct poller_struct *poller, *tmp;
+
+	if (poller_active)
+		return;
+
+	poller_active = 1;
+
+	list_for_each_entry_safe(poller, tmp, &poller_list, list)
+		poller->func(poller);
+
+	poller_active = 0;
+}
diff --git a/include/poller.h b/include/poller.h
new file mode 100644
index 0000000..dc98155
--- /dev/null
+++ b/include/poller.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) 2010 Marc Kleine-Budde <mkl@pengutronix.de>
+ *
+ * This file is released under the GPLv2
+ *
+ */
+
+#ifndef POLLER_H
+#define POLLER_H
+
+#include <linux/list.h>
+
+struct poller_struct {
+	void (*func)(struct poller_struct *poller);
+
+	struct list_head list;
+};
+
+int poller_register(struct poller_struct *poller);
+int poller_unregister(struct poller_struct *poller);
+
+
+#ifdef CONFIG_POLLER
+void poller_call(void);
+#else
+static inline void poller_call(void)
+{
+}
+#endif	/* CONFIG_POLLER */
+
+#endif	/* !POLLER_H */
-- 
1.7.2.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 2/7] basic LED support
  2010-12-18 15:15 LED framework Sascha Hauer
  2010-12-18 15:15 ` [PATCH 1/7] Add generic poll infrastructure Sascha Hauer
@ 2010-12-18 15:15 ` Sascha Hauer
  2010-12-18 16:38   ` Jean-Christophe PLAGNIOL-VILLARD
                     ` (3 more replies)
  2010-12-18 15:15 ` [PATCH 3/7] LED: Add gpio " Sascha Hauer
                   ` (4 subsequent siblings)
  6 siblings, 4 replies; 21+ messages in thread
From: Sascha Hauer @ 2010-12-18 15:15 UTC (permalink / raw)
  To: barebox

This patch adds core functionality for controlling LEDs.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/Kconfig      |    1 +
 drivers/Makefile     |    1 +
 drivers/led/Kconfig  |    6 +++
 drivers/led/Makefile |    1 +
 drivers/led/core.c   |  119 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/led.h        |   25 ++++++++++
 6 files changed, 153 insertions(+), 0 deletions(-)
 create mode 100644 drivers/led/Kconfig
 create mode 100644 drivers/led/Makefile
 create mode 100644 drivers/led/core.c
 create mode 100644 include/led.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index d94017b..86d8fb5 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -12,5 +12,6 @@ source "drivers/video/Kconfig"
 source "drivers/mci/Kconfig"
 source "drivers/clk/Kconfig"
 source "drivers/mfd/Kconfig"
+source "drivers/led/Kconfig"
 
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 242a564..b1b402f 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_MCI) += mci/
 obj-$(CONFIG_VIDEO) += video/
 obj-y	+= clk/
 obj-y	+= mfd/
+obj-$(CONFIG_LED) += led/
diff --git a/drivers/led/Kconfig b/drivers/led/Kconfig
new file mode 100644
index 0000000..964626c
--- /dev/null
+++ b/drivers/led/Kconfig
@@ -0,0 +1,6 @@
+menuconfig LED
+	bool "LED support"
+
+if LED
+
+endif
diff --git a/drivers/led/Makefile b/drivers/led/Makefile
new file mode 100644
index 0000000..0c1a6b6
--- /dev/null
+++ b/drivers/led/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_LED) += core.o
diff --git a/drivers/led/core.c b/drivers/led/core.c
new file mode 100644
index 0000000..4a0f0a6
--- /dev/null
+++ b/drivers/led/core.c
@@ -0,0 +1,119 @@
+/*
+ * core LED support for barebox
+ *
+ * (C) Copyright 2010 Sascha Hauer, Pengutronix
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 <command.h>
+#include <linux/list.h>
+#include <errno.h>
+#include <asm/gpio.h>
+#include <led.h>
+#include <poller.h>
+#include <clock.h>
+
+/**
+ * @file
+ * @brief LED framework
+ *
+ * This file contains the core LED framework for barebox.
+ *
+ * Each LED can be set to a value where 0 means disabled and values
+ * > 0 mean enabled. LEDs can have different enable values where the
+ * exact meaning depends on the LED, for example a gpio controlled rgb
+ * LED can have enable values from 1 to 7 which correspond to different
+ * colors. value could also mean a brightness.
+ * Each LED is assigned a number. numbers start with 0 and are increased
+ * with each registered LED. The number stays the same during lifecycle,
+ * gaps because of unregistered LEDs are not filled up.
+ */
+
+static LIST_HEAD(leds);
+static int num_leds;
+
+/**
+ * led_by_number - get the number of a LED
+ * @param num number of the LED to return
+ */
+struct led *led_by_number(int num)
+{
+	struct led *led;
+
+	list_for_each_entry(led, &leds, list) {
+		if (led->num == num)
+			return led;
+	}
+
+	return NULL;
+}
+
+/**
+ * led_set - set the value of a LED
+ * @param led	the led
+ * @param value	the value of the LED (0 is disabled)
+ */
+int led_set(struct led *led, unsigned int value)
+{
+	if (value > led->max_value)
+		value = led->max_value;
+
+	led->set(led, value);
+
+	return 0;
+}
+
+/**
+ * led_set_num - set the value of a LED
+ * @param num	the number of the LED
+ * @param value	the value of the LED (0 is disabled)
+ */
+int led_set_num(int num, unsigned int value)
+{
+	struct led *led = led_by_number(num);
+
+	if (!led)
+		return -ENODEV;
+
+	return led_set(led, value);
+}
+
+/**
+ * led_register - Register a LED
+ * @param led	the led
+ */
+int led_register(struct led *led)
+{
+	led->num = num_leds++;
+
+	list_add_tail(&led->list, &leds);
+
+	return 0;
+}
+
+/**
+ * led_unregister - Unegister a LED
+ * @param led	the led
+ */
+void led_unregister(struct led *led)
+{
+	list_del(&led->list);
+}
diff --git a/include/led.h b/include/led.h
new file mode 100644
index 0000000..62d0d08
--- /dev/null
+++ b/include/led.h
@@ -0,0 +1,25 @@
+#ifndef __LED_H
+#define __LED_H
+
+struct led {
+	unsigned long triger;
+	void (*set)(struct led *, unsigned int value);
+	int max_value;
+	int num;
+	struct list_head list;
+};
+
+struct led *led_by_number(int no);
+
+static inline int led_get_number(struct led *led)
+{
+	return led->num;
+}
+
+int led_set_num(int num, unsigned int value);
+int led_set(struct led *led, unsigned int value);
+int led_register(struct led *led);
+void led_unregister(struct led *led);
+void led_unregister(struct led *led);
+
+#endif /* __LED_H */
-- 
1.7.2.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 3/7] LED: Add gpio LED support
  2010-12-18 15:15 LED framework Sascha Hauer
  2010-12-18 15:15 ` [PATCH 1/7] Add generic poll infrastructure Sascha Hauer
  2010-12-18 15:15 ` [PATCH 2/7] basic LED support Sascha Hauer
@ 2010-12-18 15:15 ` Sascha Hauer
  2010-12-18 16:41   ` Jean-Christophe PLAGNIOL-VILLARD
  2010-12-18 15:15 ` [PATCH 4/7] LED: Add LED trigger support Sascha Hauer
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Sascha Hauer @ 2010-12-18 15:15 UTC (permalink / raw)
  To: barebox

This patch adds support for registering gpios as LEDs.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/led/Kconfig    |    4 ++
 drivers/led/Makefile   |    1 +
 drivers/led/led-gpio.c |   92 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/led.h          |   18 +++++++++
 4 files changed, 115 insertions(+), 0 deletions(-)
 create mode 100644 drivers/led/led-gpio.c

diff --git a/drivers/led/Kconfig b/drivers/led/Kconfig
index 964626c..b5e2f97 100644
--- a/drivers/led/Kconfig
+++ b/drivers/led/Kconfig
@@ -3,4 +3,8 @@ menuconfig LED
 
 if LED
 
+config LED_GPIO
+	bool "gpio LED support"
+	depends on GENERIC_GPIO
+
 endif
diff --git a/drivers/led/Makefile b/drivers/led/Makefile
index 0c1a6b6..29b9fd5 100644
--- a/drivers/led/Makefile
+++ b/drivers/led/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_LED) += core.o
+obj-$(CONFIG_LED_GPIO) += led-gpio.o
diff --git a/drivers/led/led-gpio.c b/drivers/led/led-gpio.c
new file mode 100644
index 0000000..d4505d1
--- /dev/null
+++ b/drivers/led/led-gpio.c
@@ -0,0 +1,92 @@
+/*
+ * gpio LED support for barebox
+ *
+ * (C) Copyright 2010 Sascha Hauer, Pengutronix
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 <led.h>
+#include <asm/gpio.h>
+
+static void led_gpio_set(struct led *led, unsigned int value)
+{
+	struct gpio_led *gpio_led = container_of(led, struct gpio_led, led);
+
+	gpio_direction_output(gpio_led->gpio, !!value ^ gpio_led->active_low);
+}
+
+/**
+ * led_gpio_register - register a gpio controlled LED
+ * @param led	The gpio LED
+ *
+ * This function registers a single gpio as a LED. led->gpio
+ * should be initialized to the gpio to control.
+ */
+int led_gpio_register(struct gpio_led *led)
+{
+	led->led.set = led_gpio_set;
+	led->led.max_value = 1;
+
+	return led_register(&led->led);
+}
+
+/**
+ * led_gpio_unregister - remove a gpio controlled LED from the framework
+ * @param led	The gpio LED
+ */
+void led_gpio_unregister(struct gpio_led *led)
+{
+	led_unregister(&led->led);
+}
+
+static void led_gpio_rgb_set(struct led *led, unsigned int value)
+{
+	struct gpio_rgb_led *rgb = container_of(led, struct gpio_rgb_led, led);
+	int al = rgb->active_low;
+
+	gpio_direction_output(rgb->gpio_r, !!(value & 4) ^ al);
+	gpio_direction_output(rgb->gpio_g, !!(value & 2) ^ al);
+	gpio_direction_output(rgb->gpio_b, !!(value & 1) ^ al);
+}
+
+/**
+ * led_gpio_rgb_register - register three gpios as a rgb LED
+ * @param led	The gpio rg LED
+ *
+ * This function registers three gpios as a rgb LED. led->gpio[rgb]
+ * should be initialized to the gpios to control.
+ */
+int led_gpio_rgb_register(struct gpio_rgb_led *led)
+{
+	led->led.set = led_gpio_rgb_set;
+	led->led.max_value = 7;
+
+	return led_register(&led->led);
+}
+
+/**
+ * led_gpio_rgb_unregister - remove a gpio controlled rgb LED from the framework
+ * @param led	The gpio LED
+ */
+void led_gpio_rgb_unregister(struct gpio_led *led)
+{
+	led_unregister(&led->led);
+}
+
diff --git a/include/led.h b/include/led.h
index 62d0d08..98b8c2e 100644
--- a/include/led.h
+++ b/include/led.h
@@ -22,4 +22,22 @@ int led_register(struct led *led);
 void led_unregister(struct led *led);
 void led_unregister(struct led *led);
 
+/* gpio LED support */
+struct gpio_led {
+	int gpio;
+	bool active_low;
+	struct led led;
+};
+
+struct gpio_rgb_led {
+	int gpio_r, gpio_g, gpio_b;
+	bool active_low;
+	struct led led;
+};
+
+int led_gpio_register(struct gpio_led *led);
+void led_gpio_unregister(struct gpio_led *led);
+int led_gpio_rgb_register(struct gpio_rgb_led *led);
+void led_gpio_rgb_unregister(struct gpio_led *led);
+
 #endif /* __LED_H */
-- 
1.7.2.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 4/7] LED: Add LED trigger support
  2010-12-18 15:15 LED framework Sascha Hauer
                   ` (2 preceding siblings ...)
  2010-12-18 15:15 ` [PATCH 3/7] LED: Add gpio " Sascha Hauer
@ 2010-12-18 15:15 ` Sascha Hauer
  2010-12-18 16:51   ` Belisko Marek
  2010-12-18 15:15 ` [PATCH 5/7] LED: Add led command Sascha Hauer
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Sascha Hauer @ 2010-12-18 15:15 UTC (permalink / raw)
  To: barebox

This patch allows to associate LEDs with certain triggers, such
as heartbeat or network activity.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/led/Kconfig  |    3 +++
 drivers/led/Makefile |    1 +
 include/led.h        |   26 ++++++++++++++++++++++++++
 include/net.h        |    3 +++
 lib/vsprintf.c       |    4 ++++
 net/eth.c            |   20 +++++++++++++++++++-
 net/net.c            |   21 ++++++++++++++++-----
 7 files changed, 72 insertions(+), 6 deletions(-)

diff --git a/drivers/led/Kconfig b/drivers/led/Kconfig
index b5e2f97..d8540c5 100644
--- a/drivers/led/Kconfig
+++ b/drivers/led/Kconfig
@@ -7,4 +7,7 @@ config LED_GPIO
 	bool "gpio LED support"
 	depends on GENERIC_GPIO
 
+config LED_TRIGGERS
+	bool "LED triggers support"
+
 endif
diff --git a/drivers/led/Makefile b/drivers/led/Makefile
index 29b9fd5..6aafa6d 100644
--- a/drivers/led/Makefile
+++ b/drivers/led/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_LED) += core.o
 obj-$(CONFIG_LED_GPIO) += led-gpio.o
+obj-$(CONFIG_LED_TRIGGERS) += led-triggers.o
diff --git a/include/led.h b/include/led.h
index 98b8c2e..2dc0f9d 100644
--- a/include/led.h
+++ b/include/led.h
@@ -22,6 +22,32 @@ int led_register(struct led *led);
 void led_unregister(struct led *led);
 void led_unregister(struct led *led);
 
+/* LED trigger support */
+enum led_trigger {
+	led_trigger_panic,
+	led_trigger_heartbeat,
+	led_trigger_net_rx,
+	led_trigger_net_tx,
+	led_trigger_net_txrx,
+	led_trigger_max,
+};
+
+#ifdef CONFIG_LED_TRIGGERS
+int led_set_trigger(enum led_trigger trigger, struct led *led);
+void led_trigger(enum led_trigger trigger, bool enable);
+#else
+static inline int led_set_trigger(enum led_trigger trigger, struct led *led)
+{
+	return 0;
+}
+
+static inline void led_trigger(enum led_trigger trigger, bool enable)
+{
+}
+#endif
+
+int led_get_trigger(enum led_trigger trigger);
+
 /* gpio LED support */
 struct gpio_led {
 	int gpio;
diff --git a/include/net.h b/include/net.h
index c695e5f..71c314e 100644
--- a/include/net.h
+++ b/include/net.h
@@ -18,6 +18,7 @@
 #include <malloc.h>
 #include <stdlib.h>
 #include <clock.h>
+#include <led.h>
 #include <asm/byteorder.h>	/* for nton* / ntoh* stuff */
 
 
@@ -417,4 +418,6 @@ static inline unsigned char *net_udp_get_payload(struct net_connection *con)
 int net_udp_send(struct net_connection *con, int len);
 int net_icmp_send(struct net_connection *con, int len);
 
+void led_trigger_network(enum led_trigger trigger, int active);
+
 #endif /* __NET_H__ */
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index e2ea84d..6117337 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -17,6 +17,7 @@
 #include <malloc.h>
 
 #include <common.h>
+#include <led.h>
 #include <reloc.h>
 
 unsigned long simple_strtoul(const char *cp,char **endp,unsigned int base)
@@ -625,6 +626,9 @@ void __noreturn panic(const char *fmt, ...)
 	vprintf(fmt, args);
 	putchar('\n');
 	va_end(args);
+
+	led_trigger(led_trigger_panic, 1);
+
 #if defined (CONFIG_PANIC_HANG)
 	hang();
 #else
diff --git a/net/eth.c b/net/eth.c
index a82a263..6ac4f09 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -77,7 +77,13 @@ int eth_send(void *packet, int length)
 		eth_current->active = 1;
 	}
 
-	return eth_current->send(eth_current, packet, length);
+	led_trigger_network(led_trigger_net_tx, 1);
+
+	ret = eth_current->send(eth_current, packet, length);
+
+	led_trigger_network(led_trigger_net_tx, 0);
+
+	return ret;
 }
 
 int eth_rx(void)
@@ -183,4 +189,16 @@ void eth_unregister(struct eth_device *edev)
 	list_del(&edev->list);
 }
 
+void led_trigger_network(enum led_trigger trigger, int enable)
+{
+	static bool rx_active, tx_active;
 
+	led_trigger(trigger, enable);
+
+	if (trigger == led_trigger_net_tx)
+		tx_active = enable;
+	if (trigger == led_trigger_net_rx)
+		rx_active = enable;
+
+	led_trigger(led_trigger_net_txrx, tx_active || rx_active);
+}
diff --git a/net/net.c b/net/net.c
index a613d1d..afd5964 100644
--- a/net/net.c
+++ b/net/net.c
@@ -628,19 +628,30 @@ int net_receive(unsigned char *pkt, int len)
 {
 	struct ethernet *et = (struct ethernet *)pkt;
 	int et_protlen = ntohs(et->et_protlen);
+	int ret;
 
-	if (len < ETHER_HDR_SIZE)
-		return 0;
+	led_trigger_network(led_trigger_net_rx, 1);
+
+	if (len < ETHER_HDR_SIZE) {
+		ret = 0;
+		goto out;
+	}
 
 	switch (et_protlen) {
 	case PROT_ARP:
-		return net_handle_arp(pkt, len);
+		ret = net_handle_arp(pkt, len);
+		break;
 	case PROT_IP:
-		return net_handle_ip(pkt, len);
+		ret = net_handle_ip(pkt, len);
+		break;
 	default:
 		debug("%s: got unknown protocol type: %d\n", __func__, et_protlen);
-		return 1;
+		ret = 1;
+		break;
 	}
+out:
+	led_trigger_network(led_trigger_net_rx, 0);
+	return ret;
 }
 
 static int net_init(void)
-- 
1.7.2.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 5/7] LED: Add led command
  2010-12-18 15:15 LED framework Sascha Hauer
                   ` (3 preceding siblings ...)
  2010-12-18 15:15 ` [PATCH 4/7] LED: Add LED trigger support Sascha Hauer
@ 2010-12-18 15:15 ` Sascha Hauer
  2010-12-18 16:45   ` Jean-Christophe PLAGNIOL-VILLARD
  2010-12-18 15:15 ` [PATCH 6/7] LED: Add trigger command Sascha Hauer
  2010-12-18 15:15 ` [PATCH 7/7] pcm038: led testing. Not to be committed Sascha Hauer
  6 siblings, 1 reply; 21+ messages in thread
From: Sascha Hauer @ 2010-12-18 15:15 UTC (permalink / raw)
  To: barebox

This patch allows controlling LEDs via the command line.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/Kconfig  |    7 ++++++
 commands/Makefile |    1 +
 commands/led.c    |   60 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 0 deletions(-)
 create mode 100644 commands/led.c

diff --git a/commands/Kconfig b/commands/Kconfig
index 5416073..64e08bb 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -372,4 +372,11 @@ config CMD_I2C
 	  include i2c_probe, i2c_read and i2c_write commands to communicate
 	  on i2c bus.
 
+config CMD_LED
+	bool
+	depends on LED
+	prompt "led command"
+	help
+	  include led command to control LEDs
+
 endmenu
diff --git a/commands/Makefile b/commands/Makefile
index ca30b5f..0820483 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -53,3 +53,4 @@ obj-$(CONFIG_CMD_UBI)		+= ubi.o
 obj-$(CONFIG_CMD_MENU)		+= menu.o
 obj-$(CONFIG_CMD_PASSWD)	+= passwd.o
 obj-$(CONFIG_CMD_LOGIN)		+= login.o
+obj-$(CONFIG_CMD_LED)		+= led.o
diff --git a/commands/led.c b/commands/led.c
new file mode 100644
index 0000000..ef6250f
--- /dev/null
+++ b/commands/led.c
@@ -0,0 +1,60 @@
+/*
+ * LED command support for barebox
+ *
+ * (C) Copyright 2010 Sascha Hauer, Pengutronix
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 <led.h>
+#include <command.h>
+#include <getopt.h>
+#include <errno.h>
+
+static int do_led(struct command *cmdtp, int argc, char *argv[])
+{
+	unsigned long led, value;
+	int ret;
+
+	if (argc != 3)
+		return COMMAND_ERROR_USAGE;
+
+	led = simple_strtoul(argv[optind], NULL, 0);
+	value = simple_strtoul(argv[optind + 1], NULL, 0);
+
+	ret = led_set_num(led, value);
+	if (ret < 0) {
+		perror("led");
+		return 1;
+	}
+
+	return 0;
+}
+
+BAREBOX_CMD_HELP_START(led)
+BAREBOX_CMD_HELP_USAGE("led <led> <value>\n")
+BAREBOX_CMD_HELP_SHORT("control the value of a LED. a value of 0 means disabled\n")
+BAREBOX_CMD_HELP_END
+
+BAREBOX_CMD_START(led)
+	.cmd		= do_led,
+	.usage		= "led <led> <value>",
+	BAREBOX_CMD_HELP(cmd_led_help)
+BAREBOX_CMD_END
-- 
1.7.2.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 6/7] LED: Add trigger command
  2010-12-18 15:15 LED framework Sascha Hauer
                   ` (4 preceding siblings ...)
  2010-12-18 15:15 ` [PATCH 5/7] LED: Add led command Sascha Hauer
@ 2010-12-18 15:15 ` Sascha Hauer
  2010-12-18 15:15 ` [PATCH 7/7] pcm038: led testing. Not to be committed Sascha Hauer
  6 siblings, 0 replies; 21+ messages in thread
From: Sascha Hauer @ 2010-12-18 15:15 UTC (permalink / raw)
  To: barebox

This patch allows controlling LED triggers vie the command line.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/Kconfig   |    8 ++++
 commands/Makefile  |    1 +
 commands/trigger.c |  106 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 115 insertions(+), 0 deletions(-)
 create mode 100644 commands/trigger.c

diff --git a/commands/Kconfig b/commands/Kconfig
index 64e08bb..f137f47 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -379,4 +379,12 @@ config CMD_LED
 	help
 	  include led command to control LEDs
 
+config CMD_LED_TRIGGER
+	bool
+	depends on LED_TRIGGERS
+	prompt "trigger command"
+	help
+	  The trigger command allows to control LED triggers from the command
+	  line.
+
 endmenu
diff --git a/commands/Makefile b/commands/Makefile
index 0820483..c89adcf 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -54,3 +54,4 @@ obj-$(CONFIG_CMD_MENU)		+= menu.o
 obj-$(CONFIG_CMD_PASSWD)	+= passwd.o
 obj-$(CONFIG_CMD_LOGIN)		+= login.o
 obj-$(CONFIG_CMD_LED)		+= led.o
+obj-$(CONFIG_CMD_LED_TRIGGER)	+= trigger.o
diff --git a/commands/trigger.c b/commands/trigger.c
new file mode 100644
index 0000000..51d4ce3
--- /dev/null
+++ b/commands/trigger.c
@@ -0,0 +1,106 @@
+/*
+ * LED trigger command support for barebox
+ *
+ * (C) Copyright 2010 Sascha Hauer, Pengutronix
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 <led.h>
+#include <command.h>
+#include <getopt.h>
+#include <errno.h>
+
+#define LED_COMMAND_SET_TRIGGER		1
+#define LED_COMMAND_SHOW_INFO		2
+#define	LED_COMMAND_DISABLE_TRIGGER	3
+
+static char *trigger_names[] = {
+	[led_trigger_panic] = "panic",
+	[led_trigger_heartbeat] = "heartbeat",
+	[led_trigger_net_rx] = "net rx",
+	[led_trigger_net_tx] = "net tx",
+	[led_trigger_net_txrx] = "net",
+};
+
+static int do_trigger(struct command *cmdtp, int argc, char *argv[])
+{
+	struct led *led;
+	int i, opt, ret = 0;
+	int cmd = LED_COMMAND_SHOW_INFO;
+	unsigned long trigger = 0, ledno;
+
+	while((opt = getopt(argc, argv, "t:d:")) > 0) {
+		switch(opt) {
+		case 't':
+			trigger = simple_strtoul(optarg, NULL, 0);
+			cmd = LED_COMMAND_SET_TRIGGER;
+			break;
+		case 'd':
+			trigger = simple_strtoul(optarg, NULL, 0);
+			cmd = LED_COMMAND_DISABLE_TRIGGER;
+		}
+	}
+
+	switch (cmd) {
+	case LED_COMMAND_SHOW_INFO:
+		for (i = 0; i < led_trigger_max; i++) {
+			int led = led_get_trigger(i);
+			printf("%d: %s", i, trigger_names[i]);
+			if (led >= 0)
+				printf(" (led %d)", led);
+			printf("\n");
+		}
+		break;
+
+	case LED_COMMAND_DISABLE_TRIGGER:
+		led_set_trigger(trigger, NULL);
+		return 0;
+	case LED_COMMAND_SET_TRIGGER:
+		if (argc - optind != 1)
+			return COMMAND_ERROR_USAGE;
+		ledno = simple_strtoul(argv[optind], NULL, 0);
+		led = led_by_number(ledno);
+
+		if (!led) {
+			printf("no such led: %d\n", ledno);
+			return 1;
+		}
+
+		ret = led_set_trigger(trigger, led);
+		break;
+	}
+
+	return ret ? 1 : 0;
+}
+
+BAREBOX_CMD_HELP_START(trigger)
+BAREBOX_CMD_HELP_USAGE("trigger [OPTIONS]\n")
+BAREBOX_CMD_HELP_SHORT("control a LED trigger. Without options the currently assigned triggers are shown.\n")
+BAREBOX_CMD_HELP_OPT  ("-t <trigger> <led>",  "set a trigger for a led\n")
+BAREBOX_CMD_HELP_OPT  ("-d <trigger>",  "disable a trigger\n")
+BAREBOX_CMD_HELP_END
+
+BAREBOX_CMD_START(trigger)
+	.cmd		= do_trigger,
+	.usage		= "handle LED triggers",
+	BAREBOX_CMD_HELP(cmd_trigger_help)
+BAREBOX_CMD_END
+
-- 
1.7.2.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 7/7] pcm038: led testing. Not to be committed
  2010-12-18 15:15 LED framework Sascha Hauer
                   ` (5 preceding siblings ...)
  2010-12-18 15:15 ` [PATCH 6/7] LED: Add trigger command Sascha Hauer
@ 2010-12-18 15:15 ` Sascha Hauer
  6 siblings, 0 replies; 21+ messages in thread
From: Sascha Hauer @ 2010-12-18 15:15 UTC (permalink / raw)
  To: barebox

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/boards/pcm038/pcm038.c |   24 +++++++++++++++++++++++-
 1 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/arch/arm/boards/pcm038/pcm038.c b/arch/arm/boards/pcm038/pcm038.c
index 1dbc6b6..cfca7d3 100644
--- a/arch/arm/boards/pcm038/pcm038.c
+++ b/arch/arm/boards/pcm038/pcm038.c
@@ -44,6 +44,7 @@
 #include <mach/spi.h>
 #include <mach/iomux-mx27.h>
 #include <mach/devices-imx27.h>
+#include <led.h>
 
 #include "pll.h"
 
@@ -192,11 +193,26 @@ static void pcm038_mmu_init(void)
 }
 #endif
 
+struct gpio_led led1 = {
+	.gpio = 10,
+};
+
+struct gpio_led led2 = {
+	.gpio = 11,
+	.active_low = 1,
+};
+
+struct gpio_rgb_led led3 = {
+	.gpio_r = 12,
+	.gpio_g = 13,
+	.gpio_b = 14,
+	.active_low = 1,
+};
+
 static int pcm038_devices_init(void)
 {
 	int i;
 	char *envdev;
-
 	unsigned int mode[] = {
 		PD0_AIN_FEC_TXD0,
 		PD1_AIN_FEC_TXD1,
@@ -270,6 +286,12 @@ static int pcm038_devices_init(void)
 
 	pcm038_mmu_init();
 
+	led_gpio_register(&led1);
+	led_gpio_register(&led2);
+	led_gpio_rgb_register(&led3);
+	led_set_trigger(led_trigger_net_tx, &led1.led);
+	led_set_trigger(led_trigger_net_rx, &led2.led);
+
 	/* configure 16 bit nor flash on cs0 */
 	CS0U = 0x0000CC03;
 	CS0L = 0xa0330D01;
-- 
1.7.2.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/7] Add generic poll infrastructure
  2010-12-18 15:15 ` [PATCH 1/7] Add generic poll infrastructure Sascha Hauer
@ 2010-12-18 15:28   ` Sascha Hauer
  0 siblings, 0 replies; 21+ messages in thread
From: Sascha Hauer @ 2010-12-18 15:28 UTC (permalink / raw)
  To: barebox

Hi Marc (Kleine-Budde),

I took the version of this patch I had lying around, please let
me know if you have an updated version.

Sascha

On Sat, Dec 18, 2010 at 04:15:03PM +0100, Sascha Hauer wrote:
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> 
> Barebox does not have interrupt functionality. Nevertheless it's
> sometimes useful to periodically call functions, like for example
> a heartbeat LED or watchdog reset. Instead of cluttering the code
> with calls to these functions this patch adds a generic polling
> infrastructure. Code which might run for longer now can call
> poller_call() periodically which in turn will call all registered
> pollers.
> This patch adds a call to poller_call in two generic pathes. First
> of them is getc() which covers waiting for uart input. Second is
> ctrlc() which should be called anyway from code which might run
> for longer. So instead adding poller_call directly to your code,
> consider checking ctrlc instead which also gives additional
> convenience to the user.
> The poller code is safe against reentrancy which means that it's
> safe to call poller_call inside a poller.
> 
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  common/Kconfig   |    3 +++
>  common/Makefile  |    1 +
>  common/console.c |    5 +++++
>  common/poller.c  |   45 +++++++++++++++++++++++++++++++++++++++++++++
>  include/poller.h |   31 +++++++++++++++++++++++++++++++
>  5 files changed, 85 insertions(+), 0 deletions(-)
>  create mode 100644 common/poller.c
>  create mode 100644 include/poller.h
> 
> diff --git a/common/Kconfig b/common/Kconfig
> index 617f640..02bc67e 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -414,6 +414,9 @@ config DEFAULT_ENVIRONMENT_PATH
>  	  Relative pathes will be relative to the barebox Toplevel dir, but absolute
>  	  pathes are fine aswell.
>  
> +config POLLER
> +	bool "generic polling infrastructure"
> +
>  endmenu
>  
>  menu "Debugging                     "
> diff --git a/common/Makefile b/common/Makefile
> index 753455b..98c9d36 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_OF_FLAT_TREE)	+= ft_build.o
>  obj-$(CONFIG_KALLSYMS)		+= kallsyms.o
>  obj-$(CONFIG_ENV_HANDLING)	+= environment.o
>  obj-$(CONFIG_AUTO_COMPLETE)	+= complete.o
> +obj-$(CONFIG_POLLER)		+= poller.o
>  
>  obj-y += dlmalloc.o
>  obj-y += clock.o
> diff --git a/common/console.c b/common/console.c
> index 82786f2..39ead4b 100644
> --- a/common/console.c
> +++ b/common/console.c
> @@ -34,6 +34,7 @@
>  #include <clock.h>
>  #include <kfifo.h>
>  #include <module.h>
> +#include <poller.h>
>  #include <linux/list.h>
>  
>  LIST_HEAD(console_list);
> @@ -205,6 +206,8 @@ int getc(void)
>  	 */
>  	start = get_time_ns();
>  	while (1) {
> +		poller_call();
> +
>  		if (tstc()) {
>  			kfifo_putc(console_input_buffer, getc_raw());
>  
> @@ -397,6 +400,8 @@ EXPORT_SYMBOL(vprintf);
>  /* test if ctrl-c was pressed */
>  int ctrlc (void)
>  {
> +	poller_call();
> +
>  	if (tstc() && getc() == 3)
>  		return 1;
>  	return 0;
> diff --git a/common/poller.c b/common/poller.c
> new file mode 100644
> index 0000000..0583a53
> --- /dev/null
> +++ b/common/poller.c
> @@ -0,0 +1,45 @@
> +/*
> + * Copyright (C) 2010 Marc Kleine-Budde <mkl@pengutronix.de>
> + *
> + * This file is released under the GPLv2
> + *
> + */
> +
> +#include <common.h>
> +#include <driver.h>
> +#include <malloc.h>
> +#include <module.h>
> +#include <param.h>
> +#include <poller.h>
> +
> +static LIST_HEAD(poller_list);
> +static int poller_active;
> +
> +int poller_register(struct poller_struct *poller)
> +{
> +	list_add_tail(&poller->list, &poller_list);
> +
> +	return 0;
> +}
> +
> +int poller_unregister(struct poller_struct *poller)
> +{
> +	list_del(&poller->list);
> +
> +	return 0;
> +}
> +
> +void poller_call(void)
> +{
> +	struct poller_struct *poller, *tmp;
> +
> +	if (poller_active)
> +		return;
> +
> +	poller_active = 1;
> +
> +	list_for_each_entry_safe(poller, tmp, &poller_list, list)
> +		poller->func(poller);
> +
> +	poller_active = 0;
> +}
> diff --git a/include/poller.h b/include/poller.h
> new file mode 100644
> index 0000000..dc98155
> --- /dev/null
> +++ b/include/poller.h
> @@ -0,0 +1,31 @@
> +/*
> + * Copyright (C) 2010 Marc Kleine-Budde <mkl@pengutronix.de>
> + *
> + * This file is released under the GPLv2
> + *
> + */
> +
> +#ifndef POLLER_H
> +#define POLLER_H
> +
> +#include <linux/list.h>
> +
> +struct poller_struct {
> +	void (*func)(struct poller_struct *poller);
> +
> +	struct list_head list;
> +};
> +
> +int poller_register(struct poller_struct *poller);
> +int poller_unregister(struct poller_struct *poller);
> +
> +
> +#ifdef CONFIG_POLLER
> +void poller_call(void);
> +#else
> +static inline void poller_call(void)
> +{
> +}
> +#endif	/* CONFIG_POLLER */
> +
> +#endif	/* !POLLER_H */
> -- 
> 1.7.2.3
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/7] basic LED support
  2010-12-18 15:15 ` [PATCH 2/7] basic LED support Sascha Hauer
@ 2010-12-18 16:38   ` Jean-Christophe PLAGNIOL-VILLARD
  2010-12-18 17:18     ` Sascha Hauer
  2010-12-18 16:48   ` Jean-Christophe PLAGNIOL-VILLARD
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 21+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2010-12-18 16:38 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

> +/**
> + * led_set - set the value of a LED
> + * @param led	the led
> + * @param value	the value of the LED (0 is disabled)
> + */
> +int led_set(struct led *led, unsigned int value)
> +{
> +	if (value > led->max_value)
> +		value = led->max_value;
> +
> +	led->set(led, value);
> +
> +	return 0;
why always return 0?
> +}
> +
> +/**
> + * led_set_num - set the value of a LED
> + * @param num	the number of the LED
> + * @param value	the value of the LED (0 is disabled)
> + */
> +int led_set_num(int num, unsigned int value)
> +{
> +	struct led *led = led_by_number(num);
> +
> +	if (!led)
> +		return -ENODEV;
> +
> +	return led_set(led, value);
> +}
> +
> +/**
> + * led_register - Register a LED
> + * @param led	the led
> + */
> +int led_register(struct led *led)
> +{
no safe check?
> +	led->num = num_leds++;
> +
> +	list_add_tail(&led->list, &leds);
> +
> +	return 0;
> +}
> +
> +/**
> + * led_unregister - Unegister a LED
> + * @param led	the led
> + */
> +void led_unregister(struct led *led)
> +{
> +	list_del(&led->list);
> +}
> diff --git a/include/led.h b/include/led.h
> new file mode 100644
> index 0000000..62d0d08
> --- /dev/null
> +++ b/include/led.h
> @@ -0,0 +1,25 @@
> +#ifndef __LED_H
> +#define __LED_H
> +
> +struct led {
> +	unsigned long triger;
> +	void (*set)(struct led *, unsigned int value);
return a int will good to known the result
> +	int max_value;
> +	int num;
> +	struct list_head list;
> +};
> +
> +struct led *led_by_number(int no);
> +
> +static inline int led_get_number(struct led *led)
> +{
> +	return led->num;
> +}
> +
> +int led_set_num(int num, unsigned int value);
> +int led_set(struct led *led, unsigned int value);
> +int led_register(struct led *led);
> +void led_unregister(struct led *led);
> +void led_unregister(struct led *led);
twice

Best Regards,
J.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/7] LED: Add gpio LED support
  2010-12-18 15:15 ` [PATCH 3/7] LED: Add gpio " Sascha Hauer
@ 2010-12-18 16:41   ` Jean-Christophe PLAGNIOL-VILLARD
  2010-12-18 17:18     ` Sascha Hauer
  0 siblings, 1 reply; 21+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2010-12-18 16:41 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

> +
> +/**
> + * led_gpio_unregister - remove a gpio controlled LED from the framework
> + * @param led	The gpio LED
> + */
> +void led_gpio_unregister(struct gpio_led *led)
> +{
> +	led_unregister(&led->led);
> +}
> +
how abaout make the rgb support optional?
> +static void led_gpio_rgb_set(struct led *led, unsigned int value)
> +{
> +	struct gpio_rgb_led *rgb = container_of(led, struct gpio_rgb_led, led);
> +	int al = rgb->active_low;
> +
> +	gpio_direction_output(rgb->gpio_r, !!(value & 4) ^ al);
> +	gpio_direction_output(rgb->gpio_g, !!(value & 2) ^ al);
> +	gpio_direction_output(rgb->gpio_b, !!(value & 1) ^ al);
> +}
> +
> +/**
> + * led_gpio_rgb_register - register three gpios as a rgb LED
> + * @param led	The gpio rg LED
> + *
> + * This function registers three gpios as a rgb LED. led->gpio[rgb]
> + * should be initialized to the gpios to control.
> + */
> +int led_gpio_rgb_register(struct gpio_rgb_led *led)
no safe check?
> +{
> +	led->led.set = led_gpio_rgb_set;
> +	led->led.max_value = 7;
> +
> +	return led_register(&led->led);
> +}
> +
> +/**
> + * led_gpio_rgb_unregister - remove a gpio controlled rgb LED from the framework
> + * @param led	The gpio LED
> + */
> +void led_gpio_rgb_unregister(struct gpio_led *led)
> +{
> +	led_unregister(&led->led);
> +}
> +
Best Regards,
J.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 5/7] LED: Add led command
  2010-12-18 15:15 ` [PATCH 5/7] LED: Add led command Sascha Hauer
@ 2010-12-18 16:45   ` Jean-Christophe PLAGNIOL-VILLARD
  2010-12-18 17:24     ` Sascha Hauer
  0 siblings, 1 reply; 21+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2010-12-18 16:45 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On 16:15 Sat 18 Dec     , Sascha Hauer wrote:
> This patch allows controlling LEDs via the command line.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  commands/Kconfig  |    7 ++++++
>  commands/Makefile |    1 +
>  commands/led.c    |   60 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 68 insertions(+), 0 deletions(-)
>  create mode 100644 commands/led.c
> 
> diff --git a/commands/Kconfig b/commands/Kconfig
> index 5416073..64e08bb 100644
> --- a/commands/Kconfig
> +++ b/commands/Kconfig
> @@ -372,4 +372,11 @@ config CMD_I2C
>  	  include i2c_probe, i2c_read and i2c_write commands to communicate
>  	  on i2c bus.
>  
> +config CMD_LED
> +	bool
> +	depends on LED
> +	prompt "led command"
> +	help
> +	  include led command to control LEDs
> +
>  endmenu
> diff --git a/commands/Makefile b/commands/Makefile
> index ca30b5f..0820483 100644
> --- a/commands/Makefile
> +++ b/commands/Makefile
> @@ -53,3 +53,4 @@ obj-$(CONFIG_CMD_UBI)		+= ubi.o
>  obj-$(CONFIG_CMD_MENU)		+= menu.o
>  obj-$(CONFIG_CMD_PASSWD)	+= passwd.o
>  obj-$(CONFIG_CMD_LOGIN)		+= login.o
> +obj-$(CONFIG_CMD_LED)		+= led.o
> diff --git a/commands/led.c b/commands/led.c
> new file mode 100644
> index 0000000..ef6250f
> --- /dev/null
> +++ b/commands/led.c
> @@ -0,0 +1,60 @@
> +/*
> + * LED command support for barebox
> + *
> + * (C) Copyright 2010 Sascha Hauer, Pengutronix
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * 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 <led.h>
> +#include <command.h>
> +#include <getopt.h>
> +#include <errno.h>
> +
> +static int do_led(struct command *cmdtp, int argc, char *argv[])
> +{
> +	unsigned long led, value;
> +	int ret;
> +
> +	if (argc != 3)
> +		return COMMAND_ERROR_USAGE;
> +
> +	led = simple_strtoul(argv[optind], NULL, 0);
> +	value = simple_strtoul(argv[optind + 1], NULL, 0);
> +
> +	ret = led_set_num(led, value);
> +	if (ret < 0) {
> +		perror("led");
> +		return 1;
> +	}
> +
> +	return 0;
> +}
how about list the available led?

Best Regards,
J.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/7] basic LED support
  2010-12-18 15:15 ` [PATCH 2/7] basic LED support Sascha Hauer
  2010-12-18 16:38   ` Jean-Christophe PLAGNIOL-VILLARD
@ 2010-12-18 16:48   ` Jean-Christophe PLAGNIOL-VILLARD
  2010-12-18 19:06   ` Belisko Marek
  2010-12-19 21:31   ` Marc Reilly
  3 siblings, 0 replies; 21+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2010-12-18 16:48 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

Hi,

	how to manage hw led?

	via pwm or hw ip?

Best Regards,
J.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/7] LED: Add LED trigger support
  2010-12-18 15:15 ` [PATCH 4/7] LED: Add LED trigger support Sascha Hauer
@ 2010-12-18 16:51   ` Belisko Marek
  2010-12-18 17:21     ` Sascha Hauer
  0 siblings, 1 reply; 21+ messages in thread
From: Belisko Marek @ 2010-12-18 16:51 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

Hi,

On Sat, Dec 18, 2010 at 4:15 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> This patch allows to associate LEDs with certain triggers, such
> as heartbeat or network activity.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/led/Kconfig  |    3 +++
>  drivers/led/Makefile |    1 +
>  include/led.h        |   26 ++++++++++++++++++++++++++
>  include/net.h        |    3 +++
>  lib/vsprintf.c       |    4 ++++
>  net/eth.c            |   20 +++++++++++++++++++-
>  net/net.c            |   21 ++++++++++++++++-----
>  7 files changed, 72 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/led/Kconfig b/drivers/led/Kconfig
> index b5e2f97..d8540c5 100644
> --- a/drivers/led/Kconfig
> +++ b/drivers/led/Kconfig
> @@ -7,4 +7,7 @@ config LED_GPIO
>        bool "gpio LED support"
>        depends on GENERIC_GPIO
>
> +config LED_TRIGGERS
> +       bool "LED triggers support"
> +
>  endif
> diff --git a/drivers/led/Makefile b/drivers/led/Makefile
> index 29b9fd5..6aafa6d 100644
> --- a/drivers/led/Makefile
> +++ b/drivers/led/Makefile
> @@ -1,2 +1,3 @@
>  obj-$(CONFIG_LED) += core.o
>  obj-$(CONFIG_LED_GPIO) += led-gpio.o
> +obj-$(CONFIG_LED_TRIGGERS) += led-triggers.o
> diff --git a/include/led.h b/include/led.h
> index 98b8c2e..2dc0f9d 100644
> --- a/include/led.h
> +++ b/include/led.h
> @@ -22,6 +22,32 @@ int led_register(struct led *led);
>  void led_unregister(struct led *led);
>  void led_unregister(struct led *led);
>
> +/* LED trigger support */
> +enum led_trigger {
> +       led_trigger_panic,
> +       led_trigger_heartbeat,
> +       led_trigger_net_rx,
> +       led_trigger_net_tx,
> +       led_trigger_net_txrx,
> +       led_trigger_max,
> +};
shouldn't be enums uppercase?
> +
> +#ifdef CONFIG_LED_TRIGGERS
> +int led_set_trigger(enum led_trigger trigger, struct led *led);
> +void led_trigger(enum led_trigger trigger, bool enable);
> +#else
> +static inline int led_set_trigger(enum led_trigger trigger, struct led *led)
> +{
> +       return 0;
> +}
> +
> +static inline void led_trigger(enum led_trigger trigger, bool enable)
> +{
> +}
> +#endif
> +
> +int led_get_trigger(enum led_trigger trigger);
> +
>  /* gpio LED support */
>  struct gpio_led {
>        int gpio;
> diff --git a/include/net.h b/include/net.h
> index c695e5f..71c314e 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -18,6 +18,7 @@
>  #include <malloc.h>
>  #include <stdlib.h>
>  #include <clock.h>
> +#include <led.h>
>  #include <asm/byteorder.h>     /* for nton* / ntoh* stuff */
>
>
> @@ -417,4 +418,6 @@ static inline unsigned char *net_udp_get_payload(struct net_connection *con)
>  int net_udp_send(struct net_connection *con, int len);
>  int net_icmp_send(struct net_connection *con, int len);
>
> +void led_trigger_network(enum led_trigger trigger, int active);
> +
>  #endif /* __NET_H__ */
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index e2ea84d..6117337 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -17,6 +17,7 @@
>  #include <malloc.h>
>
>  #include <common.h>
> +#include <led.h>
>  #include <reloc.h>
>
>  unsigned long simple_strtoul(const char *cp,char **endp,unsigned int base)
> @@ -625,6 +626,9 @@ void __noreturn panic(const char *fmt, ...)
>        vprintf(fmt, args);
>        putchar('\n');
>        va_end(args);
> +
> +       led_trigger(led_trigger_panic, 1);
> +
>  #if defined (CONFIG_PANIC_HANG)
>        hang();
>  #else
> diff --git a/net/eth.c b/net/eth.c
> index a82a263..6ac4f09 100644
> --- a/net/eth.c
> +++ b/net/eth.c
> @@ -77,7 +77,13 @@ int eth_send(void *packet, int length)
>                eth_current->active = 1;
>        }
>
> -       return eth_current->send(eth_current, packet, length);
> +       led_trigger_network(led_trigger_net_tx, 1);
> +
> +       ret = eth_current->send(eth_current, packet, length);
> +
> +       led_trigger_network(led_trigger_net_tx, 0);
> +
> +       return ret;
>  }
>
>  int eth_rx(void)
> @@ -183,4 +189,16 @@ void eth_unregister(struct eth_device *edev)
>        list_del(&edev->list);
>  }
>
> +void led_trigger_network(enum led_trigger trigger, int enable)
> +{
> +       static bool rx_active, tx_active;
>
> +       led_trigger(trigger, enable);
> +
> +       if (trigger == led_trigger_net_tx)
> +               tx_active = enable;
> +       if (trigger == led_trigger_net_rx)
> +               rx_active = enable;
> +
> +       led_trigger(led_trigger_net_txrx, tx_active || rx_active);
> +}
> diff --git a/net/net.c b/net/net.c
> index a613d1d..afd5964 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -628,19 +628,30 @@ int net_receive(unsigned char *pkt, int len)
>  {
>        struct ethernet *et = (struct ethernet *)pkt;
>        int et_protlen = ntohs(et->et_protlen);
> +       int ret;
>
> -       if (len < ETHER_HDR_SIZE)
> -               return 0;
> +       led_trigger_network(led_trigger_net_rx, 1);
> +
> +       if (len < ETHER_HDR_SIZE) {
> +               ret = 0;
> +               goto out;
> +       }
>
>        switch (et_protlen) {
>        case PROT_ARP:
> -               return net_handle_arp(pkt, len);
> +               ret = net_handle_arp(pkt, len);
> +               break;
>        case PROT_IP:
> -               return net_handle_ip(pkt, len);
> +               ret = net_handle_ip(pkt, len);
> +               break;
>        default:
>                debug("%s: got unknown protocol type: %d\n", __func__, et_protlen);
> -               return 1;
> +               ret = 1;
> +               break;
>        }
> +out:
> +       led_trigger_network(led_trigger_net_rx, 0);
> +       return ret;
>  }
>
>  static int net_init(void)
> --
> 1.7.2.3
>
>
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
>

thanks,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
icq: 290551086
web: http://open-nandra.com

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/7] basic LED support
  2010-12-18 16:38   ` Jean-Christophe PLAGNIOL-VILLARD
@ 2010-12-18 17:18     ` Sascha Hauer
  0 siblings, 0 replies; 21+ messages in thread
From: Sascha Hauer @ 2010-12-18 17:18 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox

Hi J,

On Sat, Dec 18, 2010 at 05:38:13PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > +/**
> > + * led_set - set the value of a LED
> > + * @param led	the led
> > + * @param value	the value of the LED (0 is disabled)
> > + */
> > +int led_set(struct led *led, unsigned int value)
> > +{
> > +	if (value > led->max_value)
> > +		value = led->max_value;
> > +
> > +	led->set(led, value);
> > +
> > +	return 0;
> why always return 0?

Should make this void, see below.

> > +}
> > +
> > +/**
> > + * led_set_num - set the value of a LED
> > + * @param num	the number of the LED
> > + * @param value	the value of the LED (0 is disabled)
> > + */
> > +int led_set_num(int num, unsigned int value)
> > +{
> > +	struct led *led = led_by_number(num);
> > +
> > +	if (!led)
> > +		return -ENODEV;
> > +
> > +	return led_set(led, value);
> > +}
> > +
> > +/**
> > + * led_register - Register a LED
> > + * @param led	the led
> > + */
> > +int led_register(struct led *led)
> > +{
> no safe check?

What should we check for? NUll pointers? That's not worth it.

> > +	led->num = num_leds++;
> > +
> > +	list_add_tail(&led->list, &leds);
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * led_unregister - Unegister a LED
> > + * @param led	the led
> > + */
> > +void led_unregister(struct led *led)
> > +{
> > +	list_del(&led->list);
> > +}
> > diff --git a/include/led.h b/include/led.h
> > new file mode 100644
> > index 0000000..62d0d08
> > --- /dev/null
> > +++ b/include/led.h
> > @@ -0,0 +1,25 @@
> > +#ifndef __LED_H
> > +#define __LED_H
> > +
> > +struct led {
> > +	unsigned long triger;
> > +	void (*set)(struct led *, unsigned int value);
> return a int will good to known the result

I intentionally made this void because setting a LED is not really
supposed to fail. And even if does, I don't know if it's worth anything
to tell the user "setting a LED failed", when this may be even in a loop
and the user is bothered with a hundred of these messages.

> > +	int max_value;
> > +	int num;
> > +	struct list_head list;
> > +};
> > +
> > +struct led *led_by_number(int no);
> > +
> > +static inline int led_get_number(struct led *led)
> > +{
> > +	return led->num;
> > +}
> > +
> > +int led_set_num(int num, unsigned int value);
> > +int led_set(struct led *led, unsigned int value);
> > +int led_register(struct led *led);
> > +void led_unregister(struct led *led);
> > +void led_unregister(struct led *led);
> twice
> 
> Best Regards,
> J.
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/7] LED: Add gpio LED support
  2010-12-18 16:41   ` Jean-Christophe PLAGNIOL-VILLARD
@ 2010-12-18 17:18     ` Sascha Hauer
  0 siblings, 0 replies; 21+ messages in thread
From: Sascha Hauer @ 2010-12-18 17:18 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox

On Sat, Dec 18, 2010 at 05:41:48PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > +
> > +/**
> > + * led_gpio_unregister - remove a gpio controlled LED from the framework
> > + * @param led	The gpio LED
> > + */
> > +void led_gpio_unregister(struct gpio_led *led)
> > +{
> > +	led_unregister(&led->led);
> > +}
> > +
> how abaout make the rgb support optional?

Good point, most boards do not have this kind of stuff.

> > +static void led_gpio_rgb_set(struct led *led, unsigned int value)
> > +{
> > +	struct gpio_rgb_led *rgb = container_of(led, struct gpio_rgb_led, led);
> > +	int al = rgb->active_low;
> > +
> > +	gpio_direction_output(rgb->gpio_r, !!(value & 4) ^ al);
> > +	gpio_direction_output(rgb->gpio_g, !!(value & 2) ^ al);
> > +	gpio_direction_output(rgb->gpio_b, !!(value & 1) ^ al);
> > +}
> > +
> > +/**
> > + * led_gpio_rgb_register - register three gpios as a rgb LED
> > + * @param led	The gpio rg LED
> > + *
> > + * This function registers three gpios as a rgb LED. led->gpio[rgb]
> > + * should be initialized to the gpios to control.
> > + */
> > +int led_gpio_rgb_register(struct gpio_rgb_led *led)
> no safe check?

no.

> > +{
> > +	led->led.set = led_gpio_rgb_set;
> > +	led->led.max_value = 7;
> > +
> > +	return led_register(&led->led);
> > +}
> > +
> > +/**
> > + * led_gpio_rgb_unregister - remove a gpio controlled rgb LED from the framework
> > + * @param led	The gpio LED
> > + */
> > +void led_gpio_rgb_unregister(struct gpio_led *led)
> > +{
> > +	led_unregister(&led->led);
> > +}
> > +
> Best Regards,
> J.
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/7] LED: Add LED trigger support
  2010-12-18 16:51   ` Belisko Marek
@ 2010-12-18 17:21     ` Sascha Hauer
  0 siblings, 0 replies; 21+ messages in thread
From: Sascha Hauer @ 2010-12-18 17:21 UTC (permalink / raw)
  To: Belisko Marek; +Cc: barebox

Hi Marek,

On Sat, Dec 18, 2010 at 05:51:56PM +0100, Belisko Marek wrote:
> Hi,
> 
> On Sat, Dec 18, 2010 at 4:15 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > This patch allows to associate LEDs with certain triggers, such
> > as heartbeat or network activity.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/led/Kconfig  |    3 +++
> >  drivers/led/Makefile |    1 +
> >  include/led.h        |   26 ++++++++++++++++++++++++++
> >  include/net.h        |    3 +++
> >  lib/vsprintf.c       |    4 ++++
> >  net/eth.c            |   20 +++++++++++++++++++-
> >  net/net.c            |   21 ++++++++++++++++-----
> >  7 files changed, 72 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/led/Kconfig b/drivers/led/Kconfig
> > index b5e2f97..d8540c5 100644
> > --- a/drivers/led/Kconfig
> > +++ b/drivers/led/Kconfig
> > @@ -7,4 +7,7 @@ config LED_GPIO
> >        bool "gpio LED support"
> >        depends on GENERIC_GPIO
> >
> > +config LED_TRIGGERS
> > +       bool "LED triggers support"
> > +
> >  endif
> > diff --git a/drivers/led/Makefile b/drivers/led/Makefile
> > index 29b9fd5..6aafa6d 100644
> > --- a/drivers/led/Makefile
> > +++ b/drivers/led/Makefile
> > @@ -1,2 +1,3 @@
> >  obj-$(CONFIG_LED) += core.o
> >  obj-$(CONFIG_LED_GPIO) += led-gpio.o
> > +obj-$(CONFIG_LED_TRIGGERS) += led-triggers.o
> > diff --git a/include/led.h b/include/led.h
> > index 98b8c2e..2dc0f9d 100644
> > --- a/include/led.h
> > +++ b/include/led.h
> > @@ -22,6 +22,32 @@ int led_register(struct led *led);
> >  void led_unregister(struct led *led);
> >  void led_unregister(struct led *led);
> >
> > +/* LED trigger support */
> > +enum led_trigger {
> > +       led_trigger_panic,
> > +       led_trigger_heartbeat,
> > +       led_trigger_net_rx,
> > +       led_trigger_net_tx,
> > +       led_trigger_net_txrx,
> > +       led_trigger_max,
> > +};
> shouldn't be enums uppercase?

You're right. Will change this.

Sascha

> > +
> > +#ifdef CONFIG_LED_TRIGGERS
> > +int led_set_trigger(enum led_trigger trigger, struct led *led);
> > +void led_trigger(enum led_trigger trigger, bool enable);
> > +#else
> > +static inline int led_set_trigger(enum led_trigger trigger, struct led *led)
> > +{
> > +       return 0;
> > +}
> > +
> > +static inline void led_trigger(enum led_trigger trigger, bool enable)
> > +{
> > +}
> > +#endif
> > +
> > +int led_get_trigger(enum led_trigger trigger);
> > +
> >  /* gpio LED support */
> >  struct gpio_led {
> >        int gpio;
> > diff --git a/include/net.h b/include/net.h
> > index c695e5f..71c314e 100644
> > --- a/include/net.h
> > +++ b/include/net.h
> > @@ -18,6 +18,7 @@
> >  #include <malloc.h>
> >  #include <stdlib.h>
> >  #include <clock.h>
> > +#include <led.h>
> >  #include <asm/byteorder.h>     /* for nton* / ntoh* stuff */
> >
> >
> > @@ -417,4 +418,6 @@ static inline unsigned char *net_udp_get_payload(struct net_connection *con)
> >  int net_udp_send(struct net_connection *con, int len);
> >  int net_icmp_send(struct net_connection *con, int len);
> >
> > +void led_trigger_network(enum led_trigger trigger, int active);
> > +
> >  #endif /* __NET_H__ */
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > index e2ea84d..6117337 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -17,6 +17,7 @@
> >  #include <malloc.h>
> >
> >  #include <common.h>
> > +#include <led.h>
> >  #include <reloc.h>
> >
> >  unsigned long simple_strtoul(const char *cp,char **endp,unsigned int base)
> > @@ -625,6 +626,9 @@ void __noreturn panic(const char *fmt, ...)
> >        vprintf(fmt, args);
> >        putchar('\n');
> >        va_end(args);
> > +
> > +       led_trigger(led_trigger_panic, 1);
> > +
> >  #if defined (CONFIG_PANIC_HANG)
> >        hang();
> >  #else
> > diff --git a/net/eth.c b/net/eth.c
> > index a82a263..6ac4f09 100644
> > --- a/net/eth.c
> > +++ b/net/eth.c
> > @@ -77,7 +77,13 @@ int eth_send(void *packet, int length)
> >                eth_current->active = 1;
> >        }
> >
> > -       return eth_current->send(eth_current, packet, length);
> > +       led_trigger_network(led_trigger_net_tx, 1);
> > +
> > +       ret = eth_current->send(eth_current, packet, length);
> > +
> > +       led_trigger_network(led_trigger_net_tx, 0);
> > +
> > +       return ret;
> >  }
> >
> >  int eth_rx(void)
> > @@ -183,4 +189,16 @@ void eth_unregister(struct eth_device *edev)
> >        list_del(&edev->list);
> >  }
> >
> > +void led_trigger_network(enum led_trigger trigger, int enable)
> > +{
> > +       static bool rx_active, tx_active;
> >
> > +       led_trigger(trigger, enable);
> > +
> > +       if (trigger == led_trigger_net_tx)
> > +               tx_active = enable;
> > +       if (trigger == led_trigger_net_rx)
> > +               rx_active = enable;
> > +
> > +       led_trigger(led_trigger_net_txrx, tx_active || rx_active);
> > +}
> > diff --git a/net/net.c b/net/net.c
> > index a613d1d..afd5964 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -628,19 +628,30 @@ int net_receive(unsigned char *pkt, int len)
> >  {
> >        struct ethernet *et = (struct ethernet *)pkt;
> >        int et_protlen = ntohs(et->et_protlen);
> > +       int ret;
> >
> > -       if (len < ETHER_HDR_SIZE)
> > -               return 0;
> > +       led_trigger_network(led_trigger_net_rx, 1);
> > +
> > +       if (len < ETHER_HDR_SIZE) {
> > +               ret = 0;
> > +               goto out;
> > +       }
> >
> >        switch (et_protlen) {
> >        case PROT_ARP:
> > -               return net_handle_arp(pkt, len);
> > +               ret = net_handle_arp(pkt, len);
> > +               break;
> >        case PROT_IP:
> > -               return net_handle_ip(pkt, len);
> > +               ret = net_handle_ip(pkt, len);
> > +               break;
> >        default:
> >                debug("%s: got unknown protocol type: %d\n", __func__, et_protlen);
> > -               return 1;
> > +               ret = 1;
> > +               break;
> >        }
> > +out:
> > +       led_trigger_network(led_trigger_net_rx, 0);
> > +       return ret;
> >  }
> >
> >  static int net_init(void)
> > --
> > 1.7.2.3
> >
> >
> > _______________________________________________
> > barebox mailing list
> > barebox@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/barebox
> >
> 
> thanks,
> 
> marek
> 
> -- 
> as simple and primitive as possible
> -------------------------------------------------
> Marek Belisko - OPEN-NANDRA
> Freelance Developer
> 
> Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
> Tel: +421 915 052 184
> skype: marekwhite
> icq: 290551086
> web: http://open-nandra.com
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 5/7] LED: Add led command
  2010-12-18 16:45   ` Jean-Christophe PLAGNIOL-VILLARD
@ 2010-12-18 17:24     ` Sascha Hauer
  0 siblings, 0 replies; 21+ messages in thread
From: Sascha Hauer @ 2010-12-18 17:24 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox

On Sat, Dec 18, 2010 at 05:45:54PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 16:15 Sat 18 Dec     , Sascha Hauer wrote:
> > This patch allows controlling LEDs via the command line.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  commands/Kconfig  |    7 ++++++
> >  commands/Makefile |    1 +
> >  commands/led.c    |   60 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 68 insertions(+), 0 deletions(-)
> >  create mode 100644 commands/led.c
> > 
> > diff --git a/commands/Kconfig b/commands/Kconfig
> > index 5416073..64e08bb 100644
> > --- a/commands/Kconfig
> > +++ b/commands/Kconfig
> > @@ -372,4 +372,11 @@ config CMD_I2C
> >  	  include i2c_probe, i2c_read and i2c_write commands to communicate
> >  	  on i2c bus.
> >  
> > +config CMD_LED
> > +	bool
> > +	depends on LED
> > +	prompt "led command"
> > +	help
> > +	  include led command to control LEDs
> > +
> >  endmenu
> > diff --git a/commands/Makefile b/commands/Makefile
> > index ca30b5f..0820483 100644
> > --- a/commands/Makefile
> > +++ b/commands/Makefile
> > @@ -53,3 +53,4 @@ obj-$(CONFIG_CMD_UBI)		+= ubi.o
> >  obj-$(CONFIG_CMD_MENU)		+= menu.o
> >  obj-$(CONFIG_CMD_PASSWD)	+= passwd.o
> >  obj-$(CONFIG_CMD_LOGIN)		+= login.o
> > +obj-$(CONFIG_CMD_LED)		+= led.o
> > diff --git a/commands/led.c b/commands/led.c
> > new file mode 100644
> > index 0000000..ef6250f
> > --- /dev/null
> > +++ b/commands/led.c
> > @@ -0,0 +1,60 @@
> > +/*
> > + * LED command support for barebox
> > + *
> > + * (C) Copyright 2010 Sascha Hauer, Pengutronix
> > + *
> > + * See file CREDITS for list of people who contributed to this
> > + * project.
> > + *
> > + * 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 <led.h>
> > +#include <command.h>
> > +#include <getopt.h>
> > +#include <errno.h>
> > +
> > +static int do_led(struct command *cmdtp, int argc, char *argv[])
> > +{
> > +	unsigned long led, value;
> > +	int ret;
> > +
> > +	if (argc != 3)
> > +		return COMMAND_ERROR_USAGE;
> > +
> > +	led = simple_strtoul(argv[optind], NULL, 0);
> > +	value = simple_strtoul(argv[optind + 1], NULL, 0);
> > +
> > +	ret = led_set_num(led, value);
> > +	if (ret < 0) {
> > +		perror("led");
> > +		return 1;
> > +	}
> > +
> > +	return 0;
> > +}
> how about list the available led?

What do we want to list here? LEDs do not really have properties other
then the max value (which is probably one for most LEDs). The number of
LEDs would indeed be a good information to have. I also thought about
giving the LEDs names; what do you think?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/7] basic LED support
  2010-12-18 15:15 ` [PATCH 2/7] basic LED support Sascha Hauer
  2010-12-18 16:38   ` Jean-Christophe PLAGNIOL-VILLARD
  2010-12-18 16:48   ` Jean-Christophe PLAGNIOL-VILLARD
@ 2010-12-18 19:06   ` Belisko Marek
  2010-12-19 21:31   ` Marc Reilly
  3 siblings, 0 replies; 21+ messages in thread
From: Belisko Marek @ 2010-12-18 19:06 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On Sat, Dec 18, 2010 at 4:15 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> This patch adds core functionality for controlling LEDs.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/Kconfig      |    1 +
>  drivers/Makefile     |    1 +
>  drivers/led/Kconfig  |    6 +++
>  drivers/led/Makefile |    1 +
>  drivers/led/core.c   |  119 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/led.h        |   25 ++++++++++
>  6 files changed, 153 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/led/Kconfig
>  create mode 100644 drivers/led/Makefile
>  create mode 100644 drivers/led/core.c
>  create mode 100644 include/led.h
>
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index d94017b..86d8fb5 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -12,5 +12,6 @@ source "drivers/video/Kconfig"
>  source "drivers/mci/Kconfig"
>  source "drivers/clk/Kconfig"
>  source "drivers/mfd/Kconfig"
> +source "drivers/led/Kconfig"
>
>  endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 242a564..b1b402f 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -10,3 +10,4 @@ obj-$(CONFIG_MCI) += mci/
>  obj-$(CONFIG_VIDEO) += video/
>  obj-y  += clk/
>  obj-y  += mfd/
> +obj-$(CONFIG_LED) += led/
> diff --git a/drivers/led/Kconfig b/drivers/led/Kconfig
> new file mode 100644
> index 0000000..964626c
> --- /dev/null
> +++ b/drivers/led/Kconfig
> @@ -0,0 +1,6 @@
> +menuconfig LED
> +       bool "LED support"
> +
> +if LED
> +
> +endif
> diff --git a/drivers/led/Makefile b/drivers/led/Makefile
> new file mode 100644
> index 0000000..0c1a6b6
> --- /dev/null
> +++ b/drivers/led/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_LED) += core.o
> diff --git a/drivers/led/core.c b/drivers/led/core.c
> new file mode 100644
> index 0000000..4a0f0a6
> --- /dev/null
> +++ b/drivers/led/core.c
> @@ -0,0 +1,119 @@
> +/*
> + * core LED support for barebox
> + *
> + * (C) Copyright 2010 Sascha Hauer, Pengutronix
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * 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 <command.h>
> +#include <linux/list.h>
> +#include <errno.h>
> +#include <asm/gpio.h>
> +#include <led.h>
> +#include <poller.h>
Is necessary to include poller.h?
Non of poller function is used there.
> +#include <clock.h>
> +
> +/**
> + * @file
> + * @brief LED framework
> + *
> + * This file contains the core LED framework for barebox.
> + *
> + * Each LED can be set to a value where 0 means disabled and values
> + * > 0 mean enabled. LEDs can have different enable values where the
> + * exact meaning depends on the LED, for example a gpio controlled rgb
> + * LED can have enable values from 1 to 7 which correspond to different
> + * colors. value could also mean a brightness.
> + * Each LED is assigned a number. numbers start with 0 and are increased
> + * with each registered LED. The number stays the same during lifecycle,
> + * gaps because of unregistered LEDs are not filled up.
> + */
> +
> +static LIST_HEAD(leds);
> +static int num_leds;
> +
> +/**
> + * led_by_number - get the number of a LED
> + * @param num number of the LED to return
> + */
> +struct led *led_by_number(int num)
> +{
> +       struct led *led;
> +
> +       list_for_each_entry(led, &leds, list) {
> +               if (led->num == num)
> +                       return led;
> +       }
> +
> +       return NULL;
> +}
> +
> +/**
> + * led_set - set the value of a LED
> + * @param led  the led
> + * @param value        the value of the LED (0 is disabled)
> + */
> +int led_set(struct led *led, unsigned int value)
> +{
> +       if (value > led->max_value)
> +               value = led->max_value;
> +
> +       led->set(led, value);
> +
> +       return 0;
> +}
> +
> +/**
> + * led_set_num - set the value of a LED
> + * @param num  the number of the LED
> + * @param value        the value of the LED (0 is disabled)
> + */
> +int led_set_num(int num, unsigned int value)
> +{
> +       struct led *led = led_by_number(num);
> +
> +       if (!led)
> +               return -ENODEV;
> +
> +       return led_set(led, value);
> +}
> +
> +/**
> + * led_register - Register a LED
> + * @param led  the led
> + */
> +int led_register(struct led *led)
> +{
> +       led->num = num_leds++;
> +
> +       list_add_tail(&led->list, &leds);
> +
> +       return 0;
> +}
> +
> +/**
> + * led_unregister - Unegister a LED
> + * @param led  the led
> + */
> +void led_unregister(struct led *led)
> +{
> +       list_del(&led->list);
> +}
> diff --git a/include/led.h b/include/led.h
> new file mode 100644
> index 0000000..62d0d08
> --- /dev/null
> +++ b/include/led.h
> @@ -0,0 +1,25 @@
> +#ifndef __LED_H
> +#define __LED_H
> +
> +struct led {
> +       unsigned long triger;
> +       void (*set)(struct led *, unsigned int value);
> +       int max_value;
> +       int num;
> +       struct list_head list;
> +};
> +
> +struct led *led_by_number(int no);
> +
> +static inline int led_get_number(struct led *led)
> +{
> +       return led->num;
> +}
> +
> +int led_set_num(int num, unsigned int value);
> +int led_set(struct led *led, unsigned int value);
> +int led_register(struct led *led);
> +void led_unregister(struct led *led);
> +void led_unregister(struct led *led);
> +
> +#endif /* __LED_H */
> --
> 1.7.2.3
>
>
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
>

thanks,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
icq: 290551086
web: http://open-nandra.com

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/7] basic LED support
  2010-12-18 15:15 ` [PATCH 2/7] basic LED support Sascha Hauer
                     ` (2 preceding siblings ...)
  2010-12-18 19:06   ` Belisko Marek
@ 2010-12-19 21:31   ` Marc Reilly
  2010-12-20  8:27     ` Sascha Hauer
  3 siblings, 1 reply; 21+ messages in thread
From: Marc Reilly @ 2010-12-19 21:31 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

Hi Sascha,

Thanks, this works well for my board. (tri colour led).

Was drivers/led/led-triggers.c missing from the patch set? 
I haven't been able to compile with trigger support...

> +/**
> + * led_register - Register a LED
> + * @param led	the led
> + */
> +int led_register(struct led *led)
> +{
> +	led->num = num_leds++;
> +
> +	list_add_tail(&led->list, &leds);
> +
> +	return 0;
> +}

How about being able to register a led at a specific number? This means that 
boot/init scripts that want to interact with leds don't rely on the order that 
they are registered.

I also like the suggestion of associating a name to LEDs. This may even be a 
better fit for scripting than reserved numbering.


> +
> +/**
> + * led_unregister - Unegister a LED
> + * @param led	the led
> + */
> +void led_unregister(struct led *led)
> +{
> +	list_del(&led->list);
> +}
> diff --git a/include/led.h b/include/led.h
> new file mode 100644
> index 0000000..62d0d08
> --- /dev/null
> +++ b/include/led.h
> @@ -0,0 +1,25 @@
> +#ifndef __LED_H
> +#define __LED_H
> +
> +struct led {
> +	unsigned long triger;

Spelling typo? (Should this be trigger ?)


> +	void (*set)(struct led *, unsigned int value);
> +	int max_value;
> +	int num;
> +	struct list_head list;
> +};
> +
> +struct led *led_by_number(int no);
> +


Cheers
Marc

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/7] basic LED support
  2010-12-19 21:31   ` Marc Reilly
@ 2010-12-20  8:27     ` Sascha Hauer
  0 siblings, 0 replies; 21+ messages in thread
From: Sascha Hauer @ 2010-12-20  8:27 UTC (permalink / raw)
  To: Marc Reilly; +Cc: barebox

Hi Marc,

On Mon, Dec 20, 2010 at 08:31:34AM +1100, Marc Reilly wrote:
> Hi Sascha,
> 
> Thanks, this works well for my board. (tri colour led).
> 
> Was drivers/led/led-triggers.c missing from the patch set? 
> I haven't been able to compile with trigger support...

Yes, it's missing. I'll send a new series shortly.

> 
> > +/**
> > + * led_register - Register a LED
> > + * @param led	the led
> > + */
> > +int led_register(struct led *led)
> > +{
> > +	led->num = num_leds++;
> > +
> > +	list_add_tail(&led->list, &leds);
> > +
> > +	return 0;
> > +}
> 
> How about being able to register a led at a specific number? This means that 
> boot/init scripts that want to interact with leds don't rely on the order that 
> they are registered.
> 
> I also like the suggestion of associating a name to LEDs. This may even be a 
> better fit for scripting than reserved numbering.

I'll add a name for the LEDs

> 
> 
> > +
> > +/**
> > + * led_unregister - Unegister a LED
> > + * @param led	the led
> > + */
> > +void led_unregister(struct led *led)
> > +{
> > +	list_del(&led->list);
> > +}
> > diff --git a/include/led.h b/include/led.h
> > new file mode 100644
> > index 0000000..62d0d08
> > --- /dev/null
> > +++ b/include/led.h
> > @@ -0,0 +1,25 @@
> > +#ifndef __LED_H
> > +#define __LED_H
> > +
> > +struct led {
> > +	unsigned long triger;
> 
> Spelling typo? (Should this be trigger ?)

Indeed. It turns out I don't need this line add all, so I removed it.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2010-12-20  8:28 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-18 15:15 LED framework Sascha Hauer
2010-12-18 15:15 ` [PATCH 1/7] Add generic poll infrastructure Sascha Hauer
2010-12-18 15:28   ` Sascha Hauer
2010-12-18 15:15 ` [PATCH 2/7] basic LED support Sascha Hauer
2010-12-18 16:38   ` Jean-Christophe PLAGNIOL-VILLARD
2010-12-18 17:18     ` Sascha Hauer
2010-12-18 16:48   ` Jean-Christophe PLAGNIOL-VILLARD
2010-12-18 19:06   ` Belisko Marek
2010-12-19 21:31   ` Marc Reilly
2010-12-20  8:27     ` Sascha Hauer
2010-12-18 15:15 ` [PATCH 3/7] LED: Add gpio " Sascha Hauer
2010-12-18 16:41   ` Jean-Christophe PLAGNIOL-VILLARD
2010-12-18 17:18     ` Sascha Hauer
2010-12-18 15:15 ` [PATCH 4/7] LED: Add LED trigger support Sascha Hauer
2010-12-18 16:51   ` Belisko Marek
2010-12-18 17:21     ` Sascha Hauer
2010-12-18 15:15 ` [PATCH 5/7] LED: Add led command Sascha Hauer
2010-12-18 16:45   ` Jean-Christophe PLAGNIOL-VILLARD
2010-12-18 17:24     ` Sascha Hauer
2010-12-18 15:15 ` [PATCH 6/7] LED: Add trigger command Sascha Hauer
2010-12-18 15:15 ` [PATCH 7/7] pcm038: led testing. Not to be committed Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox