Re: [PATCH v5 3/6] leds: triggers: Add support for read-only triggers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Hans,

Thanks for the new patch set.

On 11/17/2016 11:24 PM, Hans de Goede wrote:
In some cases an LED is controlled through a hardwired (taken care of
in firmware outside of the kernels control) trigger.

Add an LED_TRIGGER_READ_ONLY flag for this and disallow user-space
changing the trigger when this flag is set.

Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
Changes in v5:
-This is a new patch in v5 of this patch-set
---
 drivers/leds/led-class.c    | 2 +-
 drivers/leds/led-triggers.c | 5 +++++
 include/linux/leds.h        | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 326ee6e..56f32cc 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -54,7 +54,7 @@ static ssize_t brightness_store(struct device *dev,
 	if (ret)
 		goto unlock;

-	if (state == LED_OFF)
+	if (state == LED_OFF && !(led_cdev->flags & LED_TRIGGER_READ_ONLY))
 		led_trigger_remove(led_cdev);
 	led_set_brightness(led_cdev, state);

diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index d2ed9c2..9669104 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -37,6 +37,11 @@ ssize_t led_trigger_store(struct device *dev, struct device_attribute *attr,
 	struct led_trigger *trig;
 	int ret = count;

+	if (led_cdev->flags & LED_TRIGGER_READ_ONLY) {
+		dev_err(led_cdev->dev, "Error this led triggers is hardwired and cannot be changed\n");
+		return -EINVAL;

It means that after a trigger is once assigned to a LED class device
it will be impossible to deactivate it. Why not leaving it to the
user's decision whether they want to have hw brightness changes
notifications? This way we disable the possibility to set different
trigger like e.g. timer, after this one is set, which is not
a non-realistic scenario.

Generally it is quite odd to add a functionality that once
set is latched. If one will set such a trigger by mistake, then
system restart will be required to reset this (unless the driver
is built as a module).

+	}
+
 	mutex_lock(&led_cdev->led_access);

 	if (led_sysfs_is_disabled(led_cdev)) {
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 870b8c2..e076b74 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -47,6 +47,7 @@ struct led_classdev {
 #define LED_DEV_CAP_FLASH	(1 << 18)
 #define LED_HW_PLUGGABLE	(1 << 19)
 #define LED_PANIC_INDICATOR	(1 << 20)
+#define LED_TRIGGER_READ_ONLY   (1 << 21)

 	/* set_brightness_work / blink_timer flags, atomic, private. */
 	unsigned long		work_flags;



--
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux