Merge patch series "led: implement software blinking"

Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> says:

v2 changes:
 * Drop sw_blink_state structure, move its necessary fields to
   led_uc_plat structure.
 * Add cyclic_info pointer to led_uc_plat structure. This
   simplify code a lot.
 * Remove cyclic function search logic. Not needed anymore.
 * Fix blinking period. It was twice large.
 * Other cleanups.

v3 changes:
 * Adapt code to recent cyclic function changes
 * Move software blinking functions to separate file
 * Other small changes

v4 changes:
 * Refactoring of led_set_period() function

v5 changes
 * Fix compilation if CONFIG_LED_BLINK is not defined

v6 changes:
 * Enable LEDST_BLINK state unconditionally.
 * Function led_set_period() becomes available when CONFIG_LED_BLINK
   is disabled. This makes led code simpler.
 * Software blinking requires about 100 bytes of data for a led. It's
   not a good idea to allocate so much memory for each supported led.
   Change the code to allocate blinking data only for required leds.

cmd/led.c

@@ -15,9 +15,7 @@ static const char *const state_label[] = {
 	[LEDST_OFF]	= "off",
 	[LEDST_ON]	= "on",
 	[LEDST_TOGGLE]	= "toggle",
-#ifdef CONFIG_LED_BLINK
 	[LEDST_BLINK]	= "blink",
-#endif
 };
 
 enum led_state_t get_led_cmd(char *var)
@@ -75,9 +73,7 @@ int do_led(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 	enum led_state_t cmd;
 	const char *led_label;
 	struct udevice *dev;
-#ifdef CONFIG_LED_BLINK
 	int freq_ms = 0;
-#endif
 	int ret;
 
 	/* Validate arguments */
@@ -88,13 +84,11 @@ int do_led(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 		return list_leds();
 
 	cmd = argc > 2 ? get_led_cmd(argv[2]) : LEDST_COUNT;
-#ifdef CONFIG_LED_BLINK
 	if (cmd == LEDST_BLINK) {
 		if (argc < 4)
 			return CMD_RET_USAGE;
 		freq_ms = dectoul(argv[3], NULL);
 	}
-#endif
 	ret = led_get_by_label(led_label, &dev);
 	if (ret) {
 		printf("LED '%s' not found (err=%d)\n", led_label, ret);
@@ -106,13 +100,11 @@ int do_led(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 	case LEDST_TOGGLE:
 		ret = led_set_state(dev, cmd);
 		break;
-#ifdef CONFIG_LED_BLINK
 	case LEDST_BLINK:
 		ret = led_set_period(dev, freq_ms);
 		if (!ret)
 			ret = led_set_state(dev, LEDST_BLINK);
 		break;
-#endif
 	case LEDST_COUNT:
 		printf("LED '%s': ", led_label);
 		ret = show_led_state(dev);

drivers/led/Kconfig

@@ -65,7 +65,7 @@ config LED_PWM
 	  Linux compatible ofdata.
 
 config LED_BLINK
-	bool "Support LED blinking"
+	bool "Support hardware LED blinking"
 	depends on LED
 	help
 	  Some drivers can support automatic blinking of LEDs with a given
@@ -73,6 +73,20 @@ config LED_BLINK
 	  This option enables support for this which adds slightly to the
 	  code size.
 
+config LED_SW_BLINK
+	bool "Support software LED blinking"
+	depends on LED
+	select CYCLIC
+	help
+	  Turns on led blinking implemented in the software, useful when
+	  the hardware doesn't support led blinking. Half of the period
+	  led will be ON and the rest time it will be OFF. Standard
+	  led commands can be used to configure blinking. Does nothing
+	  if driver supports hardware blinking.
+	  WARNING: Blinking may be inaccurate during execution of time
+	  consuming commands (ex. flash reading). Also it completely
+	  stops during OS booting.
+
 config SPL_LED
 	bool "Enable LED support in SPL"
 	depends on SPL_DM

drivers/led/Makefile

@@ -4,6 +4,7 @@
 # Written by Simon Glass <sjg@chromium.org>
 
 obj-y += led-uclass.o
+obj-$(CONFIG_LED_SW_BLINK) += led_sw_blink.o
 obj-$(CONFIG_LED_BCM6328) += led_bcm6328.o
 obj-$(CONFIG_LED_BCM6358) += led_bcm6358.o
 obj-$(CONFIG_LED_BCM6753) += led_bcm6753.o

drivers/led/led-uclass.c

@@ -58,6 +58,10 @@ int led_set_state(struct udevice *dev, enum led_state_t state)
 	if (!ops->set_state)
 		return -ENOSYS;
 
+	if (IS_ENABLED(CONFIG_LED_SW_BLINK) &&
+	    led_sw_on_state_change(dev, state))
+		return 0;
+
 	return ops->set_state(dev, state);
 }
 
@@ -68,21 +72,28 @@ enum led_state_t led_get_state(struct udevice *dev)
 	if (!ops->get_state)
 		return -ENOSYS;
 
+	if (IS_ENABLED(CONFIG_LED_SW_BLINK) &&
+	    led_sw_is_blinking(dev))
+		return LEDST_BLINK;
+
 	return ops->get_state(dev);
 }
 
-#ifdef CONFIG_LED_BLINK
 int led_set_period(struct udevice *dev, int period_ms)
 {
+#ifdef CONFIG_LED_BLINK
 	struct led_ops *ops = led_get_ops(dev);
 
-	if (!ops->set_period)
+	if (ops->set_period)
-		return -ENOSYS;
+		return ops->set_period(dev, period_ms);
-
-	return ops->set_period(dev, period_ms);
-}
 #endif
 
+	if (IS_ENABLED(CONFIG_LED_SW_BLINK))
+		return led_sw_set_period(dev, period_ms);
+
+	return -ENOSYS;
+}
+
 static int led_post_bind(struct udevice *dev)
 {
 	struct led_uc_plat *uc_plat = dev_get_uclass_plat(dev);
@@ -107,6 +118,14 @@ static int led_post_bind(struct udevice *dev)
 	else
 		return 0;
 
+	if (IS_ENABLED(CONFIG_LED_BLINK)) {
+		const char *trigger;
+
+		trigger = dev_read_string(dev, "linux,default-trigger");
+		if (trigger && !strncmp(trigger, "pattern", 7))
+			uc_plat->default_state = LEDST_BLINK;
+	}
+
 	/*
 	 * In case the LED has default-state DT property, trigger
 	 * probe() to configure its default state during startup.
@@ -119,12 +138,24 @@ static int led_post_bind(struct udevice *dev)
 static int led_post_probe(struct udevice *dev)
 {
 	struct led_uc_plat *uc_plat = dev_get_uclass_plat(dev);
+	int default_period_ms = 1000;
+	int ret = 0;
 
-	if (uc_plat->default_state == LEDST_ON ||
+	switch (uc_plat->default_state) {
-	    uc_plat->default_state == LEDST_OFF)
+	case LEDST_ON:
-		led_set_state(dev, uc_plat->default_state);
+	case LEDST_OFF:
+		ret = led_set_state(dev, uc_plat->default_state);
+		break;
+	case LEDST_BLINK:
+		ret = led_set_period(dev, default_period_ms);
+		if (!ret)
+			ret = led_set_state(dev, uc_plat->default_state);
+		break;
+	default:
+		break;
+	}
 
-	return 0;
+	return ret;
 }
 
 UCLASS_DRIVER(led) = {

drivers/led/led_sw_blink.c

@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Software blinking helpers
+ * Copyright (C) 2024 IOPSYS Software Solutions AB
+ * Author: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
+ */
+
+#include <dm.h>
+#include <led.h>
+#include <time.h>
+#include <stdlib.h>
+
+#define CYCLIC_NAME_PREFIX	"led_sw_blink_"
+
+static void led_sw_blink(struct cyclic_info *c)
+{
+	struct led_sw_blink *sw_blink;
+	struct udevice *dev;
+	struct led_ops *ops;
+
+	sw_blink = container_of(c, struct led_sw_blink, cyclic);
+	dev = sw_blink->dev;
+	ops = led_get_ops(dev);
+
+	switch (sw_blink->state) {
+	case LED_SW_BLINK_ST_OFF:
+		sw_blink->state = LED_SW_BLINK_ST_ON;
+		ops->set_state(dev, LEDST_ON);
+		break;
+	case LED_SW_BLINK_ST_ON:
+		sw_blink->state = LED_SW_BLINK_ST_OFF;
+		ops->set_state(dev, LEDST_OFF);
+		break;
+	case LED_SW_BLINK_ST_NOT_READY:
+		/*
+		 * led_set_period has been called, but
+		 * led_set_state(LDST_BLINK) has not yet,
+		 * so doing nothing
+		 */
+		break;
+	default:
+		break;
+	}
+}
+
+int led_sw_set_period(struct udevice *dev, int period_ms)
+{
+	struct led_uc_plat *uc_plat = dev_get_uclass_plat(dev);
+	struct led_sw_blink *sw_blink = uc_plat->sw_blink;
+	struct led_ops *ops = led_get_ops(dev);
+	int half_period_us;
+
+	half_period_us = period_ms * 1000 / 2;
+
+	if (!sw_blink) {
+		int len = sizeof(struct led_sw_blink) +
+			  strlen(CYCLIC_NAME_PREFIX) +
+			  strlen(uc_plat->label) + 1;
+
+		sw_blink = calloc(1, len);
+		if (!sw_blink)
+			return -ENOMEM;
+
+		sw_blink->dev = dev;
+		sw_blink->state = LED_SW_BLINK_ST_DISABLED;
+		strcpy((char *)sw_blink->cyclic_name, CYCLIC_NAME_PREFIX);
+		strcat((char *)sw_blink->cyclic_name, uc_plat->label);
+
+		uc_plat->sw_blink = sw_blink;
+	}
+
+	if (sw_blink->state == LED_SW_BLINK_ST_DISABLED) {
+		cyclic_register(&sw_blink->cyclic, led_sw_blink,
+				half_period_us, sw_blink->cyclic_name);
+	} else {
+		sw_blink->cyclic.delay_us = half_period_us;
+		sw_blink->cyclic.start_time_us = timer_get_us();
+	}
+
+	sw_blink->state = LED_SW_BLINK_ST_NOT_READY;
+	ops->set_state(dev, LEDST_OFF);
+
+	return 0;
+}
+
+bool led_sw_is_blinking(struct udevice *dev)
+{
+	struct led_uc_plat *uc_plat = dev_get_uclass_plat(dev);
+	struct led_sw_blink *sw_blink = uc_plat->sw_blink;
+
+	if (!sw_blink)
+		return false;
+
+	return sw_blink->state > LED_SW_BLINK_ST_NOT_READY;
+}
+
+bool led_sw_on_state_change(struct udevice *dev, enum led_state_t state)
+{
+	struct led_uc_plat *uc_plat = dev_get_uclass_plat(dev);
+	struct led_sw_blink *sw_blink = uc_plat->sw_blink;
+
+	if (!sw_blink || sw_blink->state == LED_SW_BLINK_ST_DISABLED)
+		return false;
+
+	if (state == LEDST_BLINK) {
+		/* start blinking on next led_sw_blink() call */
+		sw_blink->state = LED_SW_BLINK_ST_OFF;
+		return true;
+	}
+
+	/* stop blinking */
+	uc_plat->sw_blink = NULL;
+	cyclic_unregister(&sw_blink->cyclic);
+	free(sw_blink);
+
+	return false;
+}

include/led.h

@@ -7,19 +7,34 @@
 #ifndef __LED_H
 #define __LED_H
 
+#include <stdbool.h>
+#include <cyclic.h>
+
 struct udevice;
 
 enum led_state_t {
 	LEDST_OFF = 0,
 	LEDST_ON = 1,
 	LEDST_TOGGLE,
-#ifdef CONFIG_LED_BLINK
 	LEDST_BLINK,
-#endif
 
 	LEDST_COUNT,
 };
 
+enum led_sw_blink_state_t {
+	LED_SW_BLINK_ST_DISABLED,
+	LED_SW_BLINK_ST_NOT_READY,
+	LED_SW_BLINK_ST_OFF,
+	LED_SW_BLINK_ST_ON,
+};
+
+struct led_sw_blink {
+	enum led_sw_blink_state_t state;
+	struct udevice *dev;
+	struct cyclic_info cyclic;
+	const char cyclic_name[0];
+};
+
 /**
  * struct led_uc_plat - Platform data the uclass stores about each device
  *
@@ -29,6 +44,9 @@ enum led_state_t {
 struct led_uc_plat {
 	const char *label;
 	enum led_state_t default_state;
+#ifdef CONFIG_LED_SW_BLINK
+	struct led_sw_blink *sw_blink;
+#endif
 };
 
 /**
@@ -118,4 +136,9 @@ int led_set_period(struct udevice *dev, int period_ms);
  */
 int led_bind_generic(struct udevice *parent, const char *driver_name);
 
+/* Internal functions for software blinking. Do not use them in your code */
+int led_sw_set_period(struct udevice *dev, int period_ms);
+bool led_sw_is_blinking(struct udevice *dev);
+bool led_sw_on_state_change(struct udevice *dev, enum led_state_t state);
+
 #endif