From 1dd7093742517b4aa362d22eaff8eef133d876ce Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Wed, 9 Jan 2019 15:44:45 +0100 Subject: [PATCH 1/6] dt-bindings: leds: Add pattern initialization from Device Tree Document new led-pattern property for initialization of LED triggers. The property format is trigger-specific (except being array of integers). For pattern trigger, the explanation of pattern format was moved to a common file shared with sysfs ABI. Signed-off-by: Krzysztof Kozlowski Acked-by: Pavel Machek Reviewed-by: Rob Herring Signed-off-by: Jacek Anaszewski --- .../testing/sysfs-class-led-trigger-pattern | 51 ++----------------- .../devicetree/bindings/leds/common.txt | 12 +++++ .../bindings/leds/leds-trigger-pattern.txt | 49 ++++++++++++++++++ 3 files changed, 64 insertions(+), 48 deletions(-) create mode 100644 Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-pattern b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern index 1e5d172e0646..bd92ef9d6faa 100644 --- a/Documentation/ABI/testing/sysfs-class-led-trigger-pattern +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern @@ -7,55 +7,10 @@ Description: timer. It can do gradual dimming and step change of brightness. The pattern is given by a series of tuples, of brightness and - duration (ms). The LED is expected to traverse the series and - each brightness value for the specified duration. Duration of - 0 means brightness should immediately change to new value, and - writing malformed pattern deactivates any active one. + duration (ms). - 1. For gradual dimming, the dimming interval now is set as 50 - milliseconds. So the tuple with duration less than dimming - interval (50ms) is treated as a step change of brightness, - i.e. the subsequent brightness will be applied without adding - intervening dimming intervals. - - The gradual dimming format of the software pattern values should be: - "brightness_1 duration_1 brightness_2 duration_2 brightness_3 - duration_3 ...". For example: - - echo 0 1000 255 2000 > pattern - - It will make the LED go gradually from zero-intensity to max (255) - intensity in 1000 milliseconds, then back to zero intensity in 2000 - milliseconds: - - LED brightness - ^ - 255-| / \ / \ / - | / \ / \ / - | / \ / \ / - | / \ / \ / - 0-| / \/ \/ - +---0----1----2----3----4----5----6------------> time (s) - - 2. To make the LED go instantly from one brightness value to another, - we should use zero-time lengths (the brightness must be same as - the previous tuple's). So the format should be: - "brightness_1 duration_1 brightness_1 0 brightness_2 duration_2 - brightness_2 0 ...". For example: - - echo 0 1000 0 0 255 2000 255 0 > pattern - - It will make the LED stay off for one second, then stay at max brightness - for two seconds: - - LED brightness - ^ - 255-| +---------+ +---------+ - | | | | | - | | | | | - | | | | | - 0-| -----+ +----+ +---- - +---0----1----2----3----4----5----6------------> time (s) + The exact format is described in: + Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt What: /sys/class/leds//hw_pattern Date: September 2018 diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt index aa1399814a2a..70876ac11367 100644 --- a/Documentation/devicetree/bindings/leds/common.txt +++ b/Documentation/devicetree/bindings/leds/common.txt @@ -37,6 +37,18 @@ Optional properties for child nodes: "ide-disk" - LED indicates IDE disk activity (deprecated), in new implementations use "disk-activity" "timer" - LED flashes at a fixed, configurable rate + "pattern" - LED alters the brightness for the specified duration with one + software timer (requires "led-pattern" property) + +- led-pattern : Array of integers with default pattern for certain triggers. + Each trigger may parse this property differently: + - one-shot : two numbers specifying delay on and delay off (in ms), + - timer : two numbers specifying delay on and delay off (in ms), + - pattern : the pattern is given by a series of tuples, of + brightness and duration (in ms). The exact format is + described in: + Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt + - led-max-microamp : Maximum LED supply current in microamperes. This property can be made mandatory for the board configurations diff --git a/Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt b/Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt new file mode 100644 index 000000000000..d3696680bfc8 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt @@ -0,0 +1,49 @@ +* Pattern format for LED pattern trigger + +The pattern is given by a series of tuples, of brightness and duration (ms). +The LED is expected to traverse the series and each brightness value for the +specified duration. Duration of 0 means brightness should immediately change to +new value, and writing malformed pattern deactivates any active one. + +1. For gradual dimming, the dimming interval now is set as 50 milliseconds. So +the tuple with duration less than dimming interval (50ms) is treated as a step +change of brightness, i.e. the subsequent brightness will be applied without +adding intervening dimming intervals. + +The gradual dimming format of the software pattern values should be: +"brightness_1 duration_1 brightness_2 duration_2 brightness_3 duration_3 ...". +For example (using sysfs interface): + +echo 0 1000 255 2000 > pattern + +It will make the LED go gradually from zero-intensity to max (255) intensity in +1000 milliseconds, then back to zero intensity in 2000 milliseconds: + +LED brightness + ^ +255-| / \ / \ / + | / \ / \ / + | / \ / \ / + | / \ / \ / + 0-| / \/ \/ + +---0----1----2----3----4----5----6------------> time (s) + +2. To make the LED go instantly from one brightness value to another, we should +use zero-time lengths (the brightness must be same as the previous tuple's). So +the format should be: "brightness_1 duration_1 brightness_1 0 brightness_2 +duration_2 brightness_2 0 ...". +For example (using sysfs interface): + +echo 0 1000 0 0 255 2000 255 0 > pattern + +It will make the LED stay off for one second, then stay at max brightness for +two seconds: + +LED brightness + ^ +255-| +---------+ +---------+ + | | | | | + | | | | | + | | | | | + 0-| -----+ +----+ +---- + +---0----1----2----3----4----5----6------------> time (s) From 8e1f456129e61371fb190c71ea182a9f6e21282e Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Wed, 9 Jan 2019 15:44:46 +0100 Subject: [PATCH 2/6] leds: Add helper for getting default pattern from Device Tree Multiple LED triggers might need to access default pattern so add a helper for that. Signed-off-by: Krzysztof Kozlowski Acked-by: Pavel Machek Signed-off-by: Jacek Anaszewski --- drivers/leds/led-core.c | 30 ++++++++++++++++++++++++++++++ include/linux/leds.h | 13 +++++++++++++ 2 files changed, 43 insertions(+) diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c index ede4fa0ac2cc..e3da7c03da1b 100644 --- a/drivers/leds/led-core.c +++ b/drivers/leds/led-core.c @@ -16,7 +16,9 @@ #include #include #include +#include #include +#include #include "leds.h" DECLARE_RWSEM(leds_list_lock); @@ -310,6 +312,34 @@ int led_update_brightness(struct led_classdev *led_cdev) } EXPORT_SYMBOL_GPL(led_update_brightness); +u32 *led_get_default_pattern(struct led_classdev *led_cdev, unsigned int *size) +{ + struct device_node *np = dev_of_node(led_cdev->dev); + u32 *pattern; + int count; + + if (!np) + return NULL; + + count = of_property_count_u32_elems(np, "led-pattern"); + if (count < 0) + return NULL; + + pattern = kcalloc(count, sizeof(*pattern), GFP_KERNEL); + if (!pattern) + return NULL; + + if (of_property_read_u32_array(np, "led-pattern", pattern, count)) { + kfree(pattern); + return NULL; + } + + *size = count; + + return pattern; +} +EXPORT_SYMBOL_GPL(led_get_default_pattern); + /* Caller must ensure led_cdev->led_access held */ void led_sysfs_disable(struct led_classdev *led_cdev) { diff --git a/include/linux/leds.h b/include/linux/leds.h index 5263f87e1d2c..78204650fe2a 100644 --- a/include/linux/leds.h +++ b/include/linux/leds.h @@ -218,6 +218,19 @@ extern int led_set_brightness_sync(struct led_classdev *led_cdev, */ extern int led_update_brightness(struct led_classdev *led_cdev); +/** + * led_get_default_pattern - return default pattern + * + * @led_cdev: the LED to get default pattern for + * @size: pointer for storing the number of elements in returned array, + * modified only if return != NULL + * + * Return: Allocated array of integers with default pattern from device tree + * or NULL. Caller is responsible for kfree(). + */ +extern u32 *led_get_default_pattern(struct led_classdev *led_cdev, + unsigned int *size); + /** * led_sysfs_disable - disable LED sysfs interface * @led_cdev: the LED to set From aa6fd10481bdb1af2d4a4b5975e23674e989cd81 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Wed, 9 Jan 2019 15:44:47 +0100 Subject: [PATCH 3/6] leds: trigger: pattern: Add pattern initialization from Device Tree Allow initialization of pattern used in pattern trigger from Device Tree property. This is especially useful for embedded systems where the pattern trigger would be used to indicate the process of boot status in a nice, user-friendly blinking way. This initialization pattern will be used till user-space is brought up and sets its own pattern, indicating the boot status is for example finished. Signed-off-by: Krzysztof Kozlowski Acked-by: Pavel Machek Signed-off-by: Jacek Anaszewski --- drivers/leds/trigger/ledtrig-pattern.c | 99 +++++++++++++++++++++----- 1 file changed, 80 insertions(+), 19 deletions(-) diff --git a/drivers/leds/trigger/ledtrig-pattern.c b/drivers/leds/trigger/ledtrig-pattern.c index 1870cf87afe1..718729c89440 100644 --- a/drivers/leds/trigger/ledtrig-pattern.c +++ b/drivers/leds/trigger/ledtrig-pattern.c @@ -220,12 +220,48 @@ static ssize_t pattern_trig_show_patterns(struct pattern_trig_data *data, return count; } +static int pattern_trig_store_patterns_string(struct pattern_trig_data *data, + const char *buf, size_t count) +{ + int ccount, cr, offset = 0; + + while (offset < count - 1 && data->npatterns < MAX_PATTERNS) { + cr = 0; + ccount = sscanf(buf + offset, "%d %u %n", + &data->patterns[data->npatterns].brightness, + &data->patterns[data->npatterns].delta_t, &cr); + if (ccount != 2) { + data->npatterns = 0; + return -EINVAL; + } + + offset += cr; + data->npatterns++; + } + + return 0; +} + +static int pattern_trig_store_patterns_int(struct pattern_trig_data *data, + const u32 *buf, size_t count) +{ + unsigned int i; + + for (i = 0; i < count; i += 2) { + data->patterns[data->npatterns].brightness = buf[i]; + data->patterns[data->npatterns].delta_t = buf[i + 1]; + data->npatterns++; + } + + return 0; +} + static ssize_t pattern_trig_store_patterns(struct led_classdev *led_cdev, - const char *buf, size_t count, - bool hw_pattern) + const char *buf, const u32 *buf_int, + size_t count, bool hw_pattern) { struct pattern_trig_data *data = led_cdev->trigger_data; - int ccount, cr, offset = 0, err = 0; + int err = 0; mutex_lock(&data->lock); @@ -237,20 +273,12 @@ static ssize_t pattern_trig_store_patterns(struct led_classdev *led_cdev, data->is_hw_pattern = hw_pattern; data->npatterns = 0; - while (offset < count - 1 && data->npatterns < MAX_PATTERNS) { - cr = 0; - ccount = sscanf(buf + offset, "%d %u %n", - &data->patterns[data->npatterns].brightness, - &data->patterns[data->npatterns].delta_t, &cr); - if (ccount != 2) { - data->npatterns = 0; - err = -EINVAL; - goto out; - } - - offset += cr; - data->npatterns++; - } + if (buf) + err = pattern_trig_store_patterns_string(data, buf, count); + else + err = pattern_trig_store_patterns_int(data, buf_int, count); + if (err) + goto out; err = pattern_trig_start_pattern(led_cdev); if (err) @@ -275,7 +303,7 @@ static ssize_t pattern_store(struct device *dev, struct device_attribute *attr, { struct led_classdev *led_cdev = dev_get_drvdata(dev); - return pattern_trig_store_patterns(led_cdev, buf, count, false); + return pattern_trig_store_patterns(led_cdev, buf, NULL, count, false); } static DEVICE_ATTR_RW(pattern); @@ -295,7 +323,7 @@ static ssize_t hw_pattern_store(struct device *dev, { struct led_classdev *led_cdev = dev_get_drvdata(dev); - return pattern_trig_store_patterns(led_cdev, buf, count, true); + return pattern_trig_store_patterns(led_cdev, buf, NULL, count, true); } static DEVICE_ATTR_RW(hw_pattern); @@ -331,6 +359,30 @@ static const struct attribute_group *pattern_trig_groups[] = { NULL, }; +static void pattern_init(struct led_classdev *led_cdev) +{ + unsigned int size = 0; + u32 *pattern; + int err; + + pattern = led_get_default_pattern(led_cdev, &size); + if (!pattern) + return; + + if (size % 2) { + dev_warn(led_cdev->dev, "Expected pattern of tuples\n"); + goto out; + } + + err = pattern_trig_store_patterns(led_cdev, NULL, pattern, size, false); + if (err < 0) + dev_warn(led_cdev->dev, + "Pattern initialization failed with error %d\n", err); + +out: + kfree(pattern); +} + static int pattern_trig_activate(struct led_classdev *led_cdev) { struct pattern_trig_data *data; @@ -354,6 +406,15 @@ static int pattern_trig_activate(struct led_classdev *led_cdev) timer_setup(&data->timer, pattern_trig_timer_function, 0); led_cdev->activated = true; + if (led_cdev->flags & LED_INIT_DEFAULT_TRIGGER) { + pattern_init(led_cdev); + /* + * Mark as initialized even on pattern_init() error because + * any consecutive call to it would produce the same error. + */ + led_cdev->flags &= ~LED_INIT_DEFAULT_TRIGGER; + } + return 0; } From 9c9ab51e86bf1f3a24eb3477cbe5d4fb09d386a5 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Wed, 9 Jan 2019 15:44:48 +0100 Subject: [PATCH 4/6] leds: trigger: oneshot: Add initialization from Device Tree Allow initialization of delays used in oneshot trigger from Device Tree property. This is especially useful for embedded systems where the trigger might be used early, before bringing up user-space. Signed-off-by: Krzysztof Kozlowski Signed-off-by: Jacek Anaszewski --- drivers/leds/trigger/ledtrig-oneshot.c | 38 ++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/drivers/leds/trigger/ledtrig-oneshot.c b/drivers/leds/trigger/ledtrig-oneshot.c index 95c9be4b6e7e..8808f0ad7339 100644 --- a/drivers/leds/trigger/ledtrig-oneshot.c +++ b/drivers/leds/trigger/ledtrig-oneshot.c @@ -130,6 +130,34 @@ static struct attribute *oneshot_trig_attrs[] = { }; ATTRIBUTE_GROUPS(oneshot_trig); +static void pattern_init(struct led_classdev *led_cdev) +{ + u32 *pattern; + unsigned int size = 0; + + pattern = led_get_default_pattern(led_cdev, &size); + if (!pattern) + goto out_default; + + if (size != 2) { + dev_warn(led_cdev->dev, + "Expected 2 but got %u values for delays pattern\n", + size); + goto out_default; + } + + led_cdev->blink_delay_on = pattern[0]; + led_cdev->blink_delay_off = pattern[1]; + kfree(pattern); + + return; + +out_default: + kfree(pattern); + led_cdev->blink_delay_on = DEFAULT_DELAY; + led_cdev->blink_delay_off = DEFAULT_DELAY; +} + static int oneshot_trig_activate(struct led_classdev *led_cdev) { struct oneshot_trig_data *oneshot_data; @@ -140,8 +168,14 @@ static int oneshot_trig_activate(struct led_classdev *led_cdev) led_set_trigger_data(led_cdev, oneshot_data); - led_cdev->blink_delay_on = DEFAULT_DELAY; - led_cdev->blink_delay_off = DEFAULT_DELAY; + if (led_cdev->flags & LED_INIT_DEFAULT_TRIGGER) { + pattern_init(led_cdev); + /* + * Mark as initialized even on pattern_init() error because + * any consecutive call to it would produce the same error. + */ + led_cdev->flags &= ~LED_INIT_DEFAULT_TRIGGER; + } return 0; } From 0f85e8498a482cbfdab91fb2d1b81c6c8c1cfd15 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Wed, 9 Jan 2019 15:44:49 +0100 Subject: [PATCH 5/6] leds: trigger: timer: Add initialization from Device Tree Allow initialization of delays used in timer trigger from Device Tree property. This is especially useful for embedded systems where the trigger might be used early, before bringing up user-space. Signed-off-by: Krzysztof Kozlowski Signed-off-by: Jacek Anaszewski --- drivers/leds/trigger/ledtrig-timer.c | 34 ++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/drivers/leds/trigger/ledtrig-timer.c b/drivers/leds/trigger/ledtrig-timer.c index 7c14983781ee..ca898c1383be 100644 --- a/drivers/leds/trigger/ledtrig-timer.c +++ b/drivers/leds/trigger/ledtrig-timer.c @@ -15,6 +15,7 @@ #include #include #include +#include #include static ssize_t led_delay_on_show(struct device *dev, @@ -77,8 +78,41 @@ static struct attribute *timer_trig_attrs[] = { }; ATTRIBUTE_GROUPS(timer_trig); +static void pattern_init(struct led_classdev *led_cdev) +{ + u32 *pattern; + unsigned int size = 0; + + pattern = led_get_default_pattern(led_cdev, &size); + if (!pattern) + return; + + if (size != 2) { + dev_warn(led_cdev->dev, + "Expected 2 but got %u values for delays pattern\n", + size); + goto out; + } + + led_cdev->blink_delay_on = pattern[0]; + led_cdev->blink_delay_off = pattern[1]; + /* led_blink_set() called by caller */ + +out: + kfree(pattern); +} + static int timer_trig_activate(struct led_classdev *led_cdev) { + if (led_cdev->flags & LED_INIT_DEFAULT_TRIGGER) { + pattern_init(led_cdev); + /* + * Mark as initialized even on pattern_init() error because + * any consecutive call to it would produce the same error. + */ + led_cdev->flags &= ~LED_INIT_DEFAULT_TRIGGER; + } + led_blink_set(led_cdev, &led_cdev->blink_delay_on, &led_cdev->blink_delay_off); From 5ddb0869bfc1bca6cfc592c74c64a026f936638c Mon Sep 17 00:00:00 2001 From: Michal Kazior Date: Mon, 11 Feb 2019 10:29:27 +0100 Subject: [PATCH 6/6] leds: lp55xx: fix null deref on firmware load failure I've stumbled upon a kernel crash and the logs pointed me towards the lp5562 driver: > <4>[306013.841294] lp5562 0-0030: Direct firmware load for lp5562 failed with error -2 > <4>[306013.894990] lp5562 0-0030: Falling back to user helper > ... > <3>[306073.924886] lp5562 0-0030: firmware request failed > <1>[306073.939456] Unable to handle kernel NULL pointer dereference at virtual address 00000000 > <4>[306074.251011] PC is at _raw_spin_lock+0x1c/0x58 > <4>[306074.255539] LR is at release_firmware+0x6c/0x138 > ... After taking a look I noticed firmware_release() could be called with either NULL or a dangling pointer. Fixes: 10c06d178df11 ("leds-lp55xx: support firmware interface") Signed-off-by: Michal Kazior Signed-off-by: Jacek Anaszewski --- drivers/leds/leds-lp55xx-common.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/leds/leds-lp55xx-common.c b/drivers/leds/leds-lp55xx-common.c index 3d79a6380761..723f2f17497a 100644 --- a/drivers/leds/leds-lp55xx-common.c +++ b/drivers/leds/leds-lp55xx-common.c @@ -201,7 +201,7 @@ static void lp55xx_firmware_loaded(const struct firmware *fw, void *context) if (!fw) { dev_err(dev, "firmware request failed\n"); - goto out; + return; } /* handling firmware data is chip dependent */ @@ -214,9 +214,9 @@ static void lp55xx_firmware_loaded(const struct firmware *fw, void *context) mutex_unlock(&chip->lock); -out: /* firmware should be released for other channel use */ release_firmware(chip->fw); + chip->fw = NULL; } static int lp55xx_request_firmware(struct lp55xx_chip *chip)