Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Bjorn Andersson <bjorn.andersson@linaro.org>
To:     Jonathan Corbet <corbet@lwn.net>, Pavel Machek <pavel@ucw.cz>,
        Marijn Suijten <marijn.suijten@somainline.org>
Cc:     Matthias Kaehlcke <mka@chromium.org>,
        Stephen Boyd <swboyd@chromium.org>,
        Satya Priya <quic_c_skakit@quicinc.com>,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-leds@vger.kernel.org, linux-arm-msm@vger.kernel.org
Subject: [PATCH] leds: qcom-lpg: Require pattern to follow documentation
Date:   Mon, 23 May 2022 16:37:19 -0700
Message-Id: <20220523233719.1496297-1-bjorn.andersson@linaro.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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 <bjorn.andersson@linaro.org>
---
 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;
 }
-- 
2.35.1