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: 24e2d05 ("leds: Add driver for Qualcomm LPG")
Signed-off-by: Bjorn Andersson <[email protected]>
Signed-off-by: Pavel Machek <[email protected]>

Author: andersson

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"
 
         ^
         |

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;
 }