From e98a860f65428a3cae7ed7b3e8ebcf6320d7fc5e Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Mon, 23 May 2022 16:37:19 -0700 Subject: [PATCH] leds: qcom-lpg: Require pattern to follow documentation The leds-trigger-pattern documentation describes how the brightness of the LED should transition linearly from one brightness value to the next, over the given delta_t. But the pattern engine in the Qualcomm LPG hardware only supports holding the brightness for each entry for the period. This subset of patterns can be represented in the leds-trigger-pattern by injecting zero-time transitions after each entry in the pattern, resulting in a pattern that pattern that can be rendered by the LPG. Rework LPG pattern interface to require these zero-time transitions, to make it comply with this subset of patterns and reject the patterns it can't render. Fixes: 24e2d05d1b68 ("leds: Add driver for Qualcomm LPG") Signed-off-by: Bjorn Andersson Signed-off-by: Pavel Machek --- Documentation/leds/leds-qcom-lpg.rst | 8 ++++-- drivers/leds/rgb/leds-qcom-lpg.c | 43 ++++++++++++++++++++++++---- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/Documentation/leds/leds-qcom-lpg.rst b/Documentation/leds/leds-qcom-lpg.rst index f12416f02dd8..de7ceead9337 100644 --- a/Documentation/leds/leds-qcom-lpg.rst +++ b/Documentation/leds/leds-qcom-lpg.rst @@ -35,11 +35,13 @@ Specify a hardware pattern for a Qualcomm LPG LED. The pattern is a series of brightness and hold-time pairs, with the hold-time expressed in milliseconds. The hold time is a property of the pattern and must therefor be identical for each element in the pattern (except for the pauses -described below). +described below). As the LPG hardware is not able to perform the linear +transitions expected by the leds-trigger-pattern format, each entry in the +pattern must be followed a zero-length entry of the same brightness. Simple pattern:: - "255 500 0 500" + "255 500 255 0 0 500 0 0" ^ | @@ -54,7 +56,7 @@ in the pattern, the so called "low pause" and "high pause". Low-pause pattern:: - "255 1000 0 500 255 500 0 500" + "255 1000 255 0 0 500 0 0 255 500 255 0 0 500 0 0" ^ | diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c index cfa3362b2457..02f51cc61837 100644 --- a/drivers/leds/rgb/leds-qcom-lpg.c +++ b/drivers/leds/rgb/leds-qcom-lpg.c @@ -704,11 +704,12 @@ static int lpg_blink_mc_set(struct led_classdev *cdev, return ret; } -static int lpg_pattern_set(struct lpg_led *led, struct led_pattern *pattern, +static int lpg_pattern_set(struct lpg_led *led, struct led_pattern *led_pattern, u32 len, int repeat) { struct lpg_channel *chan; struct lpg *lpg = led->lpg; + struct led_pattern *pattern; unsigned int brightness_a; unsigned int brightness_b; unsigned int actual_len; @@ -719,18 +720,48 @@ static int lpg_pattern_set(struct lpg_led *led, struct led_pattern *pattern, unsigned int hi_idx; unsigned int i; bool ping_pong = true; - int ret; + int ret = -EINVAL; /* Hardware only support oneshot or indefinite loops */ if (repeat != -1 && repeat != 1) return -EINVAL; + /* + * The standardized leds-trigger-pattern format defines that the + * brightness of the LED follows a linear transition from one entry + * in the pattern to the next, over the given delta_t time. It + * describes that the way to perform instant transitions a zero-length + * entry should be added following a pattern entry. + * + * The LPG hardware is only able to perform the latter (no linear + * transitions), so require each entry in the pattern to be followed by + * a zero-length transition. + */ + if (len % 2) + return -EINVAL; + + pattern = kcalloc(len / 2, sizeof(*pattern), GFP_KERNEL); + if (!pattern) + return -ENOMEM; + + for (i = 0; i < len; i += 2) { + if (led_pattern[i].brightness != led_pattern[i + 1].brightness) + goto out_free_pattern; + if (led_pattern[i + 1].delta_t != 0) + goto out_free_pattern; + + pattern[i / 2].brightness = led_pattern[i].brightness; + pattern[i / 2].delta_t = led_pattern[i].delta_t; + } + + len /= 2; + /* * Specifying a pattern of length 1 causes the hardware to iterate * through the entire LUT, so prohibit this. */ if (len < 2) - return -EINVAL; + goto out_free_pattern; /* * The LPG plays patterns with at a fixed pace, a "low pause" can be @@ -781,13 +812,13 @@ static int lpg_pattern_set(struct lpg_led *led, struct led_pattern *pattern, * specify hi pause. Reject other variations. */ if (i != actual_len - 1) - return -EINVAL; + goto out_free_pattern; } } /* LPG_RAMP_DURATION_REG is a 9bit */ if (delta_t >= BIT(9)) - return -EINVAL; + goto out_free_pattern; /* Find "low pause" and "high pause" in the pattern */ lo_pause = pattern[0].delta_t; @@ -814,6 +845,8 @@ static int lpg_pattern_set(struct lpg_led *led, struct led_pattern *pattern, out_unlock: mutex_unlock(&lpg->lock); +out_free_pattern: + kfree(pattern); return ret; }