Re: [PATCH v3 3/3] leds: rgb: leds-qcom-lpg: Fix calculation of best period Hi-Res PWMs

Sebastian Reichel
2025-03-03T22:47:33Z

[-- Attachment #1: Type: text/plain, Size: 2379 bytes --]

Hi,

On Mon, Mar 03, 2025 at 01:52:52PM +0200, Abel Vesa wrote:
> 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.
>
> Cc: stable@vger.kernel.org # 6.4
> Fixes: b00d2ed37617 ("leds: rgb: leds-qcom-lpg: Add support for high resolution PWM")
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---

Reviewed-by: Sebastian Reichel <sre@kernel.org>

Greetings,

-- Sebastian

> 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 {
>
> --
> 2.34.1
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]