[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250220145522.GA778229@google.com>
Date: Thu, 20 Feb 2025 14:55:22 +0000
From: Lee Jones <lee@...nel.org>
To: Anjelique Melendez <anjelique.melendez@....qualcomm.com>
Cc: pavel@...nel.org, andersson@...nel.org, krzysztof.kozlowski@...aro.org,
morf3089@...il.com, u.kleine-koenig@...gutronix.de,
marijn.suijten@...ainline.org, linux-leds@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
Zejiong Huang <zejiongh@....qualcomm.com>
Subject: Re: [PATCH] leds: rgb: leds-qcom-lpg: Add support for 6-bit PWM
resolution
On Wed, 12 Feb 2025, Anjelique Melendez wrote:
> Currently, driver only allows for PWM modules to use 9-bit resolution.
> However, PWM modules can support 6-bit and 9-bit resolution. Add support
> for 6-bit resolution.
>
> Suggested-by: Zejiong Huang <zejiongh@....qualcomm.com>
> Signed-off-by: Anjelique Melendez <anjelique.melendez@....qualcomm.com>
> ---
> drivers/leds/rgb/leds-qcom-lpg.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c
> index f3c9ef2bfa57..4e5c56ded1f0 100644
> --- a/drivers/leds/rgb/leds-qcom-lpg.c
> +++ b/drivers/leds/rgb/leds-qcom-lpg.c
> @@ -24,6 +24,7 @@
> #define LPG_PATTERN_CONFIG_REG 0x40
> #define LPG_SIZE_CLK_REG 0x41
> #define PWM_CLK_SELECT_MASK GENMASK(1, 0)
> +#define PWM_SIZE_SELECT_MASK BIT(2)
Are you sure you want to shove this between 2 seemingly related defines?
> #define PWM_CLK_SELECT_HI_RES_MASK GENMASK(2, 0)
> #define PWM_SIZE_HI_RES_MASK GENMASK(6, 4)
> #define LPG_PREDIV_CLK_REG 0x42
> @@ -412,8 +413,8 @@ static int lpg_lut_sync(struct lpg *lpg, unsigned int mask)
> static const unsigned int lpg_clk_rates[] = {0, 1024, 32768, 19200000};
> static const unsigned int lpg_clk_rates_hi_res[] = {0, 1024, 32768, 19200000, 76800000};
> static const unsigned int lpg_pre_divs[] = {1, 3, 5, 6};
> -static const unsigned int lpg_pwm_resolution[] = {9};
> -static const unsigned int lpg_pwm_resolution_hi_res[] = {8, 9, 10, 11, 12, 13, 14, 15};
> +static const unsigned int lpg_pwm_resolution[] = {6, 9};
> +static const unsigned int lpg_pwm_resolution_hi_res[] = {8, 9, 10, 11, 12, 13, 14, 15};
>
> static int lpg_calc_freq(struct lpg_channel *chan, uint64_t period)
> {
> @@ -436,12 +437,12 @@ static int lpg_calc_freq(struct lpg_channel *chan, uint64_t period)
> * period = --------------------------
> * refclk
> *
> - * Resolution = 2^9 bits for PWM or
> + * Resolution = 2^{6 or 9} bits for PWM or
> * 2^{8, 9, 10, 11, 12, 13, 14, 15} bits for high resolution PWM
> * pre_div = {1, 3, 5, 6} and
> * M = [0..7].
> *
> - * This allows for periods between 27uS and 384s for PWM channels and periods between
> + * This allows for periods between 3uS and 384s for PWM channels and periods between
> * 3uS and 24576s for high resolution PWMs.
> * The PWM framework wants a period of equal or lower length than requested,
> * reject anything below minimum period.
> @@ -558,7 +559,7 @@ static void lpg_apply_freq(struct lpg_channel *chan)
> val |= GENMASK(5, 4);
> break;
> case LPG_SUBTYPE_PWM:
> - val |= BIT(2);
> + val |= FIELD_PREP(PWM_SIZE_SELECT_MASK, chan->pwm_resolution_sel);
> break;
> case LPG_SUBTYPE_HI_RES_PWM:
> val |= FIELD_PREP(PWM_SIZE_HI_RES_MASK, chan->pwm_resolution_sel);
> @@ -1276,7 +1277,7 @@ static int lpg_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
> resolution = lpg_pwm_resolution_hi_res[FIELD_GET(PWM_SIZE_HI_RES_MASK, val)];
> } else {
> refclk = lpg_clk_rates[FIELD_GET(PWM_CLK_SELECT_MASK, val)];
> - resolution = 9;
> + resolution = lpg_pwm_resolution[FIELD_GET(PWM_SIZE_SELECT_MASK, val)];
> }
>
> if (refclk) {
> --
> 2.34.1
>
>
--
Lee Jones [李琼斯]
Powered by blists - more mailing lists