Re: [PATCH V2 2/6] pwm: sprd: Improve the pwm backlight control function

[Chunyan Zhang]

On Thu, 25 Jan 2024 at 11:09, Wenhua Lin <Wenhua.Lin@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
>  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
>