The PWM allow configuring the PWM resolution from 8 bits PWM values up to 15 bits values, for the Hi-Res PWMs, and then either 6-bit or 9-bit for the normal PWMs. The current implementation loops through all possible resolutions (PWM sizes), for the PWM subtype, on top of the already existing process of determining the prediv, exponent and refclk.
The first and second issues are related to capping the computed PWM value.
The third issue is that it uses the wrong maximum possible PWM value for determining the best matched period.
Fix all of them.
Signed-off-by: Abel Vesa abel.vesa@linaro.org --- Changes in v4: - Rebased on next-20250305 - Re-worded the commit messages for the first two patches to include an example that should better explain what the issue that is being fixed is. As per Uwe's request. - Link to v3: https://lore.kernel.org/r/20250303-leds-qcom-lpg-fix-max-pwm-on-hi-res-v3-0-...
Changes in v3: - Added a new patch that fixes the normal PWMs, since they now support 6-bit resolution as well. Added it as first patch. - Re-worded the second patch. Included Bjorn's suggestion and R-b tag. - Link to v2: https://lore.kernel.org/r/20250226-leds-qcom-lpg-fix-max-pwm-on-hi-res-v2-0-...
Changes in v2: - Re-worded the commit to drop the details that are not important w.r.t. what the patch is fixing. - Added another patch which fixes the resolution used for determining best matched period and PWM config. - Link to v1: https://lore.kernel.org/r/20250220-leds-qcom-lpg-fix-max-pwm-on-hi-res-v1-1-...
--- Abel Vesa (3): leds: rgb: leds-qcom-lpg: Fix pwm resolution max for normal PWMs leds: rgb: leds-qcom-lpg: Fix pwm resolution max for Hi-Res PWMs leds: rgb: leds-qcom-lpg: Fix calculation of best period Hi-Res PWMs
drivers/leds/rgb/leds-qcom-lpg.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) --- base-commit: 7ec162622e66a4ff886f8f28712ea1b13069e1aa change-id: 20250220-leds-qcom-lpg-fix-max-pwm-on-hi-res-067e8782a79b
Best regards,
Ideally, the requested duty cycle should never translate to a PWM value higher than the selected resolution (PWM size), but currently the best matched period is never reported back to the PWM consumer, so the consumer will still be using the requested period which is higher than the best matched one. This will result in PWM consumer requesting duty cycle values higher than the allowed PWM value.
For example, a consumer might request a period of 5ms while the best (closest) period the PWM hardware will do is 4.26ms. For this best matched resolution, if the selected resolution is 8-bit wide, when the consumer asks for a duty cycle of 5ms, the PWM value will be 300, which is outside of what the resolution allows. This will happen with all possible resolutions when selected.
Since for these Hi-Res PWMs, the current implementation is capping the PWM value at a 15-bit resolution, even when lower resolutions are selected, the value will be wrapped around by the HW internal logic to the selected resolution.
Fix the issue by capping the PWM value to the maximum value allowed by the selected resolution.
Cc: stable@vger.kernel.org # 6.4 Fixes: b00d2ed37617 ("leds: rgb: leds-qcom-lpg: Add support for high resolution PWM") Reviewed-by: Bjorn Andersson andersson@kernel.org Reviewed-by: Sebastian Reichel sre@kernel.org Signed-off-by: Abel Vesa abel.vesa@linaro.org --- drivers/leds/rgb/leds-qcom-lpg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c index 4454fc6a38480b61916318dd170f3eddc32976d6..0b6310184988c299d82ee7181982c03d306407a4 100644 --- a/drivers/leds/rgb/leds-qcom-lpg.c +++ b/drivers/leds/rgb/leds-qcom-lpg.c @@ -530,7 +530,7 @@ static void lpg_calc_duty(struct lpg_channel *chan, uint64_t duty) unsigned int clk_rate;
if (chan->subtype == LPG_SUBTYPE_HI_RES_PWM) { - max = LPG_RESOLUTION_15BIT - 1; + max = BIT(lpg_pwm_resolution_hi_res[chan->pwm_resolution_sel]) - 1; clk_rate = lpg_clk_rates_hi_res[chan->clk_sel]; } else { max = BIT(lpg_pwm_resolution[chan->pwm_resolution_sel]) - 1;
When determining the actual best period by looping through all possible PWM configs, the resolution currently used is based on bit shift value which is off-by-one above the possible maximum PWM value allowed.
So subtract one from the resolution before determining the best period so that the maximum duty cycle requested by the PWM user won't result in a value above the maximum allowed by the selected resolution.
Cc: stable@vger.kernel.org # 6.4 Fixes: b00d2ed37617 ("leds: rgb: leds-qcom-lpg: Add support for high resolution PWM") Reviewed-by: Sebastian Reichel sre@kernel.org Signed-off-by: Abel Vesa abel.vesa@linaro.org --- drivers/leds/rgb/leds-qcom-lpg.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c index 0b6310184988c299d82ee7181982c03d306407a4..4f2a178e3d265a2cc88e651d3e2ca6ae3dfac2e2 100644 --- a/drivers/leds/rgb/leds-qcom-lpg.c +++ b/drivers/leds/rgb/leds-qcom-lpg.c @@ -462,7 +462,7 @@ static int lpg_calc_freq(struct lpg_channel *chan, uint64_t period) max_res = LPG_RESOLUTION_9BIT; }
- min_period = div64_u64((u64)NSEC_PER_SEC * (1 << pwm_resolution_arr[0]), + min_period = div64_u64((u64)NSEC_PER_SEC * ((1 << pwm_resolution_arr[0]) - 1), clk_rate_arr[clk_len - 1]); if (period <= min_period) return -EINVAL; @@ -483,7 +483,7 @@ static int lpg_calc_freq(struct lpg_channel *chan, uint64_t period) */
for (i = 0; i < pwm_resolution_count; i++) { - resolution = 1 << pwm_resolution_arr[i]; + resolution = (1 << pwm_resolution_arr[i]) - 1; for (clk_sel = 1; clk_sel < clk_len; clk_sel++) { u64 numerator = period * clk_rate_arr[clk_sel];
@@ -1292,7 +1292,7 @@ static int lpg_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, if (ret) return ret;
- state->period = DIV_ROUND_UP_ULL((u64)NSEC_PER_SEC * (1 << resolution) * + state->period = DIV_ROUND_UP_ULL((u64)NSEC_PER_SEC * ((1 << resolution) - 1) * pre_div * (1 << m), refclk); state->duty_cycle = DIV_ROUND_UP_ULL((u64)NSEC_PER_SEC * pwm_value * pre_div * (1 << m), refclk); } else {
On Wed, 05 Mar 2025 15:09:03 +0200, Abel Vesa wrote:
The PWM allow configuring the PWM resolution from 8 bits PWM values up to 15 bits values, for the Hi-Res PWMs, and then either 6-bit or 9-bit for the normal PWMs. The current implementation loops through all possible resolutions (PWM sizes), for the PWM subtype, on top of the already existing process of determining the prediv, exponent and refclk.
[...]
Applied, thanks!
[1/3] leds: rgb: leds-qcom-lpg: Fix pwm resolution max for normal PWMs commit: d3fd20cecf7fcdada938429ad525daf5b2217a7a [2/3] leds: rgb: leds-qcom-lpg: Fix pwm resolution max for Hi-Res PWMs commit: 638fc32c056aa62c7add071205de6acc479ee37d [3/3] leds: rgb: leds-qcom-lpg: Fix calculation of best period Hi-Res PWMs commit: 227fc065ae9e707af9c7d346458f43fd25cf310a
-- Lee Jones [李琼斯]
linux-stable-mirror@lists.linaro.org
-
Abel Vesa -
Lee Jones