[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAAfSe-t68Jq+mKaSdzz5P2tPetxup7w1UoMsk+Z__3YWtV6EhQ@mail.gmail.com>
Date: Thu, 25 Jan 2024 15:20:04 +0800
From: Chunyan Zhang <zhang.lyra@...il.com>
To: Wenhua Lin <Wenhua.Lin@...soc.com>
Cc: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>,
Orson Zhai <orsonzhai@...il.com>, Baolin Wang <baolin.wang@...ux.alibaba.com>,
linux-pwm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, wenhua lin <wenhua.lin1994@...il.com>,
Xiongpeng Wu <xiongpeng.wu@...soc.com>, zhaochen su <zhaochen.su29@...il.com>,
Zhaochen Su <Zhaochen.Su@...soc.com>, Xiaolong Wang <Xiaolong.Wang@...soc.com>
Subject: Re: [PATCH V2 2/6] pwm: sprd: Improve the pwm backlight control function
On Thu, 25 Jan 2024 at 11:09, Wenhua Lin <Wenhua.Lin@...soc.com> wrote:
>
> The pwm-sprd driver support only 8-bit linear control of backlight. Now,
> new requests of supporting 9-bit, 10-bit, 11-bit and 12-bit linear
> control of backlight are proposed. Besides, different channels of pwm
> could be configured into different linear control of backlight. Thus,
> sprd,mod attribute is introduced into dts for every channel of pwm
> device. This attribute would determine the value of MOD and eventually
> realize the new requirements.
>
> Signed-off-by: Wenhua Lin <Wenhua.Lin@...soc.com>
> ---
> drivers/pwm/pwm-sprd.c | 42 ++++++++++++++++++++++++++++++++++--------
> 1 file changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/pwm/pwm-sprd.c b/drivers/pwm/pwm-sprd.c
> index bc1e3ed13528..cc54aa77c7e6 100644
> --- a/drivers/pwm/pwm-sprd.c
> +++ b/drivers/pwm/pwm-sprd.c
> @@ -18,7 +18,8 @@
> #define SPRD_PWM_DUTY 0x8
> #define SPRD_PWM_ENABLE 0x18
>
> -#define SPRD_PWM_MOD_MAX GENMASK(7, 0)
> +#define SPRD_PWM_MOD_MAX GENMASK(15, 0)
> +#define SPRD_PWM_MOD_DEFAULT GENMASK(9, 0)
> #define SPRD_PWM_DUTY_MSK GENMASK(15, 0)
> #define SPRD_PWM_PRESCALE_MSK GENMASK(7, 0)
> #define SPRD_PWM_ENABLE_BIT BIT(0)
> @@ -43,6 +44,7 @@ struct sprd_pwm_chip {
> const struct sprd_pwm_data *pdata;
> int num_pwms;
> struct sprd_pwm_chn chn[SPRD_PWM_CHN_NUM];
> + u32 mod[SPRD_PWM_CHN_NUM];
> };
>
> static const struct sprd_pwm_data ums512_data = {
> @@ -120,7 +122,7 @@ static int sprd_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
> */
> val = sprd_pwm_read(spc, pwm->hwpwm, SPRD_PWM_PRESCALE);
> prescale = val & SPRD_PWM_PRESCALE_MSK;
> - tmp = (prescale + 1) * NSEC_PER_SEC * SPRD_PWM_MOD_MAX;
> + tmp = (prescale + 1) * NSEC_PER_SEC * spc->mod[pwm->hwpwm];
> state->period = DIV_ROUND_CLOSEST_ULL(tmp, chn->clk_rate);
>
> val = sprd_pwm_read(spc, pwm->hwpwm, SPRD_PWM_DUTY);
> @@ -140,7 +142,7 @@ static int sprd_pwm_config(struct sprd_pwm_chip *spc, struct pwm_device *pwm,
> int duty_ns, int period_ns)
> {
> struct sprd_pwm_chn *chn = &spc->chn[pwm->hwpwm];
> - u32 prescale, duty;
> + u32 prescale, duty, mod;
> u64 tmp;
>
> /*
> @@ -148,16 +150,21 @@ static int sprd_pwm_config(struct sprd_pwm_chip *spc, struct pwm_device *pwm,
> * The period length is (PRESCALE + 1) * MOD counter steps.
> * The duty cycle length is (PRESCALE + 1) * DUTY counter steps.
> *
> - * To keep the maths simple we're always using MOD = SPRD_PWM_MOD_MAX.
> + * The value for MOD is obtained from dts.
> * The value for PRESCALE is selected such that the resulting period
> * gets the maximal length not bigger than the requested one with the
> - * given settings (MOD = SPRD_PWM_MOD_MAX and input clock).
> + * given settings (MOD and input clock).
> */
> - duty = duty_ns * SPRD_PWM_MOD_MAX / period_ns;
> + mod = spc->mod[pwm->hwpwm];
> + duty = duty_ns * mod / period_ns;
>
> tmp = (u64)chn->clk_rate * period_ns;
> do_div(tmp, NSEC_PER_SEC);
> - prescale = DIV_ROUND_CLOSEST_ULL(tmp, SPRD_PWM_MOD_MAX) - 1;
> + prescale = DIV_ROUND_CLOSEST_ULL(tmp, mod);
> + if (prescale < 1)
> + prescale = 1;
> + prescale--;
> +
> if (prescale > SPRD_PWM_PRESCALE_MSK)
> prescale = SPRD_PWM_PRESCALE_MSK;
>
> @@ -170,7 +177,7 @@ static int sprd_pwm_config(struct sprd_pwm_chip *spc, struct pwm_device *pwm,
> * before changing a new configuration to avoid mixed settings.
> */
> sprd_pwm_write(spc, pwm->hwpwm, SPRD_PWM_PRESCALE, prescale);
> - sprd_pwm_write(spc, pwm->hwpwm, SPRD_PWM_MOD, SPRD_PWM_MOD_MAX);
> + sprd_pwm_write(spc, pwm->hwpwm, SPRD_PWM_MOD, mod);
> sprd_pwm_write(spc, pwm->hwpwm, SPRD_PWM_DUTY, duty);
>
> return 0;
> @@ -263,6 +270,21 @@ static int sprd_pwm_clk_init(struct sprd_pwm_chip *spc)
> return 0;
> }
>
> +static int sprd_pwm_get_mod(struct platform_device *pdev)
> +{
> + int i, ret;
> + struct sprd_pwm_chip *spc = platform_get_drvdata(pdev);
Before using platform_get_drvdata(), you have to call
platform_set_drvdata, otherwise spc is NULL here and it will cause a
crash.
> +
> + ret = of_property_read_u32_array(pdev->dev.of_node,
> + "sprd,mod", spc->mod, spc->num_pwms);
> + if (ret) {
> + for (i = 0; i < spc->num_pwms; i++)
> + spc->mod[i] = SPRD_PWM_MOD_DEFAULT;
> + }
> +
> + return ret;
> +}
> +
> static int sprd_pwm_probe(struct platform_device *pdev)
> {
> struct sprd_pwm_chip *spc;
> @@ -288,6 +310,10 @@ static int sprd_pwm_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> + ret = sprd_pwm_get_mod(pdev);
> + if (ret)
> + dev_info(&pdev->dev, "get pwm mod failed! Use default setting\n");
> +
> spc->chip.dev = &pdev->dev;
> spc->chip.ops = &sprd_pwm_ops;
> spc->chip.npwm = spc->num_pwms;
> --
> 2.17.1
>
Powered by blists - more mailing lists