[PATCH 1/3] leds:triggers:Extend the kernel panic LED trigger

[PATCH 1/3] leds:triggers:Extend the kernel panic LED trigger

[chaochao2021666]

From: chao zeng <chao.zeng@siemens.com>

This commit extend panic trigger, add two new panic trigger
"panic_on" and "panic_off" and keep the "panic" compatible with
"panic_blink".

All the led on the "panic_on" would light and on
the "panic_off" would turn off

Expand the panic state of led to meet more requirements

Signed-off-by: chao zeng <chao.zeng@siemens.com>
---
 drivers/leds/trigger/ledtrig-panic.c | 39 ++++++++++++++++++++++------
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/drivers/leds/trigger/ledtrig-panic.c b/drivers/leds/trigger/ledtrig-panic.c
index 64abf2e91608..1274bc94b5dd 100644
--- a/drivers/leds/trigger/ledtrig-panic.c
+++ b/drivers/leds/trigger/ledtrig-panic.c
@@ -12,19 +12,26 @@
 #include <linux/leds.h>
 #include "../leds.h"
 
-static struct led_trigger *trigger;
+enum led_display_type {
+	ON,
+	OFF,
+	BLINK,
+	DISPLAY_TYPE_COUNT,
+};
+
+static struct led_trigger *panic_trigger[DISPLAY_TYPE_COUNT];
 
 /*
  * This is called in a special context by the atomic panic
  * notifier. This means the trigger can be changed without
  * worrying about locking.
  */
-static void led_trigger_set_panic(struct led_classdev *led_cdev)
+static void led_trigger_set_panic(struct led_classdev *led_cdev, const char *type)
 {
 	struct led_trigger *trig;
 
 	list_for_each_entry(trig, &trigger_list, next_trig) {
-		if (strcmp("panic", trig->name))
+		if (strcmp(type, trig->name))
 			continue;
 		if (led_cdev->trigger)
 			list_del(&led_cdev->trig_list);
@@ -37,6 +44,10 @@ static void led_trigger_set_panic(struct led_classdev *led_cdev)
 		led_cdev->trigger = trig;
 		if (trig->activate)
 			trig->activate(led_cdev);
+
+		/*Clear current brightness work*/
+		led_cdev->work_flags = 0;
+
 		break;
 	}
 }
@@ -48,7 +59,12 @@ static int led_trigger_panic_notifier(struct notifier_block *nb,
 
 	list_for_each_entry(led_cdev, &leds_list, node)
 		if (led_cdev->flags & LED_PANIC_INDICATOR)
-			led_trigger_set_panic(led_cdev);
+			led_trigger_set_panic(led_cdev, "panic");
+		else if (led_cdev->flags & LED_PANIC_INDICATOR_ON)
+			led_trigger_set_panic(led_cdev, "panic_on");
+		else if (led_cdev->flags & LED_PANIC_INDICATOR_OFF)
+			led_trigger_set_panic(led_cdev, "panic_off");
+
 	return NOTIFY_DONE;
 }
 
@@ -56,9 +72,12 @@ static struct notifier_block led_trigger_panic_nb = {
 	.notifier_call = led_trigger_panic_notifier,
 };
 
-static long led_panic_blink(int state)
+static long led_panic_activity(int state)
 {
-	led_trigger_event(trigger, state ? LED_FULL : LED_OFF);
+	led_trigger_event(panic_trigger[BLINK], state ? LED_FULL : LED_OFF);
+	led_trigger_event(panic_trigger[ON], LED_FULL);
+	led_trigger_event(panic_trigger[OFF], LED_OFF);
+
 	return 0;
 }
 
@@ -67,8 +86,12 @@ static int __init ledtrig_panic_init(void)
 	atomic_notifier_chain_register(&panic_notifier_list,
 				       &led_trigger_panic_nb);
 
-	led_trigger_register_simple("panic", &trigger);
-	panic_blink = led_panic_blink;
+	led_trigger_register_simple("panic", &panic_trigger[BLINK]);
+	led_trigger_register_simple("panic_on", &panic_trigger[ON]);
+	led_trigger_register_simple("panic_off", &panic_trigger[OFF]);
+
+	panic_blink = led_panic_activity;
+
 	return 0;
 }
 device_initcall(ledtrig_panic_init);
-- 
2.17.1

[PATCH 2/3] leds:gpio:Add the support for "panic-indicator-on" and "panic-indicator-off"

[chaochao2021666]

From: chao zeng <chao.zeng@siemens.com>

This commit extend the panic indicator,allowing to set LED a variety of
different of states on a kernel panic.There are on and off state
in addition to blink for a given LED.

Signed-off-by: chao zeng <chao.zeng@siemens.com>
---
 drivers/leds/leds-gpio.c | 9 +++++++++
 include/linux/leds.h     | 4 ++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index 092eb59a7d32..d77ec57f3f71 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -103,6 +103,11 @@ static int create_gpio_led(const struct gpio_led *template,
 		led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
 	if (template->panic_indicator)
 		led_dat->cdev.flags |= LED_PANIC_INDICATOR;
+	else if (template->panic_indicator_on)
+		led_dat->cdev.flags |= LED_PANIC_INDICATOR_ON;
+	else if (template->panic_indicator_off)
+		led_dat->cdev.flags |= LED_PANIC_INDICATOR_OFF;
+
 	if (template->retain_state_shutdown)
 		led_dat->cdev.flags |= LED_RETAIN_AT_SHUTDOWN;
 
@@ -169,6 +174,10 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
 			led.retain_state_shutdown = 1;
 		if (fwnode_property_present(child, "panic-indicator"))
 			led.panic_indicator = 1;
+		else if (fwnode_property_present(child, "panic-indicator-on"))
+			led.panic_indicator_on = 1;
+		else if (fwnode_property_present(child, "panic-indicator-off"))
+			led.panic_indicator_off = 1;
 
 		ret = create_gpio_led(&led, led_dat, dev, child, NULL);
 		if (ret < 0) {
diff --git a/include/linux/leds.h b/include/linux/leds.h
index a0b730be40ad..6b3ae1dbc65f 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -85,6 +85,8 @@ struct led_classdev {
 #define LED_BRIGHT_HW_CHANGED	BIT(21)
 #define LED_RETAIN_AT_SHUTDOWN	BIT(22)
 #define LED_INIT_DEFAULT_TRIGGER BIT(23)
+#define LED_PANIC_INDICATOR_ON	BIT(24)
+#define LED_PANIC_INDICATOR_OFF	BIT(25)
 
 	/* set_brightness_work / blink_timer flags, atomic, private. */
 	unsigned long		work_flags;
@@ -521,6 +523,8 @@ struct gpio_led {
 	unsigned	active_low : 1;
 	unsigned	retain_state_suspended : 1;
 	unsigned	panic_indicator : 1;
+	unsigned	panic_indicator_on : 1;
+	unsigned	panic_indicator_off : 1;
 	unsigned	default_state : 2;
 	unsigned	retain_state_shutdown : 1;
 	/* default_state should be one of LEDS_GPIO_DEFSTATE_(ON|OFF|KEEP) */
-- 
2.17.1

[PATCH 3/3] dt-bindings:leds:Extend panic led state

[chaochao2021666]

From: chao zeng <chao.zeng@siemens.com>

Add extra "panic-indicator-on" and "panic-indicator-off"
to extend the panic led status.Not only blink to indicate
the panic

Signed-off-by: chao zeng <chao.zeng@siemens.com>
---
 Documentation/devicetree/bindings/leds/common.yaml | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
index 697102707703..7eb367063155 100644
--- a/Documentation/devicetree/bindings/leds/common.yaml
+++ b/Documentation/devicetree/bindings/leds/common.yaml
@@ -125,7 +125,19 @@ properties:
   panic-indicator:
     description:
       This property specifies that the LED should be used, if at all possible,
-      as a panic indicator.
+      using blink as a panic indicator.
+    type: boolean
+
+  panic-indicator-on:
+    description:
+      This property specifies that the LED should be used, if at all possible,
+      using led on as a panic indicator.
+    type: boolean
+
+  panic-indicator-off:
+    description:
+      This property specifies that the LED should be used, if at all possible,
+      using led off as a panic indicator.
     type: boolean
 
   retain-state-shutdown:
-- 
2.17.1

Re: [PATCH 1/3] leds:triggers:Extend the kernel panic LED trigger

[Marek Behún]

On Mon,  6 Sep 2021 21:53:18 +0800
chaochao2021666@163.com wrote:

> From: chao zeng <chao.zeng@siemens.com>
> 
> This commit extend panic trigger, add two new panic trigger
> "panic_on" and "panic_off" and keep the "panic" compatible with
> "panic_blink".
> 
> All the led on the "panic_on" would light and on
> the "panic_off" would turn off

We don't wont gazillion triggers, each for every possible setting.

Instead extend the existing panic trigger to have another sysfs setting
where you can set this behavior.
  echo panic >trigger
  echo blink >on_panic
So the on_panic file can accept "on", "off" or "blink".

Alternatively a pattern could be set as in the ledtrig-pattern trigger.

Also your patches do not use correct spacing in commit titles:
  leds:triggers:Extend the kernel panic LED trigger
should instead be
  leds: triggers: Extend the kernel panic LED trigger

Marek

Re: [PATCH 1/3] leds:triggers:Extend the kernel panic LED trigger

[Jan Kiszka]

On 08.09.21 03:45, chaochao2021666 wrote:
> Dear Marek
> 
> 
> For other types of led could be set at the userspace level. But for the
> panic,
> maybe it would trigger at kernel space during the kernel boot up.
> 
> And currently only blink to indicate the error. we need more kinds of
> type to indicate the error.
> 
> we have two leds in the panic trigger group, all in the panic only one
> behavior-- blink.
> we need different panic led behavior, so extend the led behavior. I
> think add more types of 
> LED behavior could be helpful.
> 

To make it even clearer, there are three issues to solve for us:

One is that we have two LEDs mixing a color, red and green, and the
obviously desired panic color it red, not orange.

The other is that the desired state in an error case is non-blinking,
just on (in line with what our U-Boot will do in case the boot fails).

And as we need that behavior prior to userspace, it should be
configurable via DT. But that does not exclude extending the sysfs
interface as well with the new options.

Jan

> BRs
> Chao
> 
> 
> At 2021-09-07 20:20:18, "Marek Behún" <kabel@kernel.org> wrote:
>>On Mon,  6 Sep 2021 21:53:18 +0800
>>chaochao2021666@163.com wrote:
>>
>>> From: chao zeng <chao.zeng@siemens.com>
>>> 
>>> This commit extend panic trigger, add two new panic trigger
>>> "panic_on" and "panic_off" and keep the "panic" compatible with
>>> "panic_blink".
>>> 
>>> All the led on the "panic_on" would light and on
>>> the "panic_off" would turn off
>>
>>We don't wont gazillion triggers, each for every possible setting.
>>
>>Instead extend the existing panic trigger to have another sysfs setting
>>where you can set this behavior.
>>  echo panic >trigger
>>  echo blink >on_panic
>>So the on_panic file can accept "on", "off" or "blink".
>>
>>Alternatively a pattern could be set as in the ledtrig-pattern trigger.
>>
>>Also your patches do not use correct spacing in commit titles:
>>  leds:triggers:Extend the kernel panic LED trigger
>>should instead be
>>  leds: triggers: Extend the kernel panic LED trigger
>>
>>Marek
> 

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux