diff mbox series

[v2,4/8] led: status_led: add new activity LED config and functions

Message ID 20240606143024.20024-5-ansuelsmth@gmail.com
State Changes Requested
Delegated to: Tom Rini
Headers show
Series misc: introduce STATUS LED activity function | expand

Commit Message

Christian Marangi June 6, 2024, 2:29 p.m. UTC
Add a new activity LED config and additional functions to implement a
simple software blink feature to signal activity of any kind.

Usual activity might be a file transfer with TFTP, a flash write...

User of this API will call status_led_activity_start/stop() on each
activity and LED will be toggled based on the defined FREQ config value.

With this enabled, cyclic API are also enabled as this makes use of
cyclic API to handle LED blink.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/led/Kconfig       | 16 +++++++++++++++
 drivers/misc/status_led.c | 41 +++++++++++++++++++++++++++++++++++++++
 include/status_led.h      |  2 ++
 3 files changed, 59 insertions(+)

Comments

Quentin Schulz June 7, 2024, 8:43 a.m. UTC | #1
Hi Christian,

On 6/6/24 4:29 PM, Christian Marangi wrote:
> Add a new activity LED config and additional functions to implement a
> simple software blink feature to signal activity of any kind.
> 
> Usual activity might be a file transfer with TFTP, a flash write...
> 
> User of this API will call status_led_activity_start/stop() on each
> activity and LED will be toggled based on the defined FREQ config value.
> 
> With this enabled, cyclic API are also enabled as this makes use of
> cyclic API to handle LED blink.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>   drivers/led/Kconfig       | 16 +++++++++++++++
>   drivers/misc/status_led.c | 41 +++++++++++++++++++++++++++++++++++++++
>   include/status_led.h      |  2 ++
>   3 files changed, 59 insertions(+)
> 
> diff --git a/drivers/led/Kconfig b/drivers/led/Kconfig
> index 6c4f02d71f2..869ed78e87f 100644
> --- a/drivers/led/Kconfig
> +++ b/drivers/led/Kconfig
> @@ -359,6 +359,22 @@ config LED_STATUS_BOOT
>   
>   endif # LED_STATUS_BOOT_ENABLE
>   
> +config LED_STATUS_ACTIVITY_ENABLE
> +	bool "Enable BOOT LED"
> +	select CYCLIC
> +	help
> +	  Enable to turn an LED on when the board is doing some
> +	  activity (flash write, file download).
> +
> +if LED_STATUS_ACTIVITY_ENABLE
> +
> +config LED_STATUS_ACTIVITY
> +	int "LED to light when the board is doing some activity"
> +	help
> +	  Valid enabled LED device number.
> +
> +endif # LED_STATUS_ACTIVITY_ENABLE
> +
>   config LED_STATUS_RED_ENABLE
>   	bool "Enable red LED"
>   	help
> diff --git a/drivers/misc/status_led.c b/drivers/misc/status_led.c
> index 93bfb410662..5ee68c7c77d 100644
> --- a/drivers/misc/status_led.c
> +++ b/drivers/misc/status_led.c
> @@ -5,6 +5,7 @@
>    */
>   
>   #include <common.h>
> +#include <cyclic.h>
>   #include <status_led.h>
>   
>   /*
> @@ -23,6 +24,7 @@ typedef struct {
>   	int state;
>   	int period;
>   	int cnt;
> +	struct cyclic_info *cyclic;
>   } led_dev_t;
>   
>   led_dev_t led_dev[] = {
> @@ -140,3 +142,42 @@ void status_led_toggle(int led)
>   
>   	__led_toggle(ld->mask);
>   }
> +
> +static void status_led_activity_toggle(void *ctx)
> +{
> +	__led_toggle(*(led_id_t *)ctx);
> +}
> +
> +void status_led_activity_start(int led)
> +{
> +	led_dev_t *ld;
> +
> +	ld = status_get_led_dev(led);
> +	if (!ld)
> +		return;

Should we print an error here as well? This seems to mean we are calling 
this function with an LED that doesn't exist?

> +
> +	if (ld->cyclic) {
> +		printf("Cyclic for activity status LED already registered. THIS IS AN ERROR.\n");

What about giving the info of which activity status LED is problematic? 
So that if somehow we have multiple activity status LED, we have some 
idea where to look in the code already without adding more debug code to 
get started?

> +		cyclic_unregister(ld->cyclic);
> +	}
> +
> +	status_led_set(led, CONFIG_LED_STATUS_BLINKING);
> +
> +	ld->cyclic = cyclic_register(status_led_activity_toggle,
> +				     ld->period * 500, "activity", &ld->mask);
> +	if (!ld->cyclic)
> +		printf("Registering of cyclic function for activity status LED failed\n");

Ditto.

> +}
> +
> +void status_led_activity_stop(int led)
> +{
> +	led_dev_t *ld;
> +
> +	ld = status_get_led_dev(led);
> +	if (!ld)
> +		return;

Ditto.

Cheers,
Quentin
diff mbox series

Patch

diff --git a/drivers/led/Kconfig b/drivers/led/Kconfig
index 6c4f02d71f2..869ed78e87f 100644
--- a/drivers/led/Kconfig
+++ b/drivers/led/Kconfig
@@ -359,6 +359,22 @@  config LED_STATUS_BOOT
 
 endif # LED_STATUS_BOOT_ENABLE
 
+config LED_STATUS_ACTIVITY_ENABLE
+	bool "Enable BOOT LED"
+	select CYCLIC
+	help
+	  Enable to turn an LED on when the board is doing some
+	  activity (flash write, file download).
+
+if LED_STATUS_ACTIVITY_ENABLE
+
+config LED_STATUS_ACTIVITY
+	int "LED to light when the board is doing some activity"
+	help
+	  Valid enabled LED device number.
+
+endif # LED_STATUS_ACTIVITY_ENABLE
+
 config LED_STATUS_RED_ENABLE
 	bool "Enable red LED"
 	help
diff --git a/drivers/misc/status_led.c b/drivers/misc/status_led.c
index 93bfb410662..5ee68c7c77d 100644
--- a/drivers/misc/status_led.c
+++ b/drivers/misc/status_led.c
@@ -5,6 +5,7 @@ 
  */
 
 #include <common.h>
+#include <cyclic.h>
 #include <status_led.h>
 
 /*
@@ -23,6 +24,7 @@  typedef struct {
 	int state;
 	int period;
 	int cnt;
+	struct cyclic_info *cyclic;
 } led_dev_t;
 
 led_dev_t led_dev[] = {
@@ -140,3 +142,42 @@  void status_led_toggle(int led)
 
 	__led_toggle(ld->mask);
 }
+
+static void status_led_activity_toggle(void *ctx)
+{
+	__led_toggle(*(led_id_t *)ctx);
+}
+
+void status_led_activity_start(int led)
+{
+	led_dev_t *ld;
+
+	ld = status_get_led_dev(led);
+	if (!ld)
+		return;
+
+	if (ld->cyclic) {
+		printf("Cyclic for activity status LED already registered. THIS IS AN ERROR.\n");
+		cyclic_unregister(ld->cyclic);
+	}
+
+	status_led_set(led, CONFIG_LED_STATUS_BLINKING);
+
+	ld->cyclic = cyclic_register(status_led_activity_toggle,
+				     ld->period * 500, "activity", &ld->mask);
+	if (!ld->cyclic)
+		printf("Registering of cyclic function for activity status LED failed\n");
+}
+
+void status_led_activity_stop(int led)
+{
+	led_dev_t *ld;
+
+	ld = status_get_led_dev(led);
+	if (!ld)
+		return;
+
+	cyclic_unregister(ld->cyclic);
+	ld->cyclic = NULL
+	status_led_set(led, CONFIG_LED_STATUS_OFF);
+}
diff --git a/include/status_led.h b/include/status_led.h
index fe0c84fb4b4..7de40551621 100644
--- a/include/status_led.h
+++ b/include/status_led.h
@@ -39,6 +39,8 @@  void status_led_init(void);
 void status_led_tick(unsigned long timestamp);
 void status_led_set(int led, int state);
 void status_led_toggle(int led);
+void status_led_activity_start(int led);
+void status_led_activity_stop(int led);
 
 /*****  MVS v1  **********************************************************/
 #if (defined(CONFIG_MVS) && CONFIG_MVS < 2)