Thread (37 messages) 37 messages, 4 authors, 2021-11-10

Re: [RFC PATCH v3 2/8] leds: add function to configure hardware controlled LED

From: Andrew Lunn <andrew@lunn.ch>
Date: 2021-11-09 20:49:46
Also in: linux-devicetree, linux-doc, linux-leds, lkml

quoted
quoted
+#ifdef CONFIG_LEDS_HARDWARE_CONTROL
+enum blink_mode_cmd {
+	BLINK_MODE_ENABLE, /* Enable the hardware blink mode */
+	BLINK_MODE_DISABLE, /* Disable the hardware blink mode */
+	BLINK_MODE_READ, /* Read the status of the hardware blink mode */
+	BLINK_MODE_SUPPORTED, /* Ask the driver if the hardware blink mode is supported */
+	BLINK_MODE_ZERO, /* Disable any hardware blink active */
+};
+#endif
this is a strange proposal for the API.

Anyway, led_classdev already has the blink_set() method, which is documented as
	/*
	  * Activate hardware accelerated blink, delays are in milliseconds
	  * and if both are zero then a sensible default should be chosen.
	  * The call should adjust the timings in that case and if it can't
	  * match the values specified exactly.
	  * Deactivate blinking again when the brightness is set to LED_OFF
	  * via the brightness_set() callback.
	  */
	int		(*blink_set)(struct led_classdev *led_cdev,
				     unsigned long *delay_on,
				     unsigned long *delay_off);

So we already have a method to set hardware blkinking, we don't need
another one.

Marek
But that is about hardware blink, not a LED controlled by hardware based
on some rules/modes.
Doesn't really match the use for the hardware control.
Blink_set makes the LED blink contantly at the declared delay.
The blink_mode_cmd are used to request stuff to a LED in hardware mode.

Doesn't seem correct to change/enhance the blink_set function with
something that would do something completely different.
Humm. I can see merits for both.

What i like about reusing blink_set() is that it is well understood.
There is a defined sysfs API for it. ledtrig-oneshot.c also uses it,
for a non-repeating blink. So i think that also fits the PHY LED use
case.

	Andrew
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help