Re: [PATCH v2 2/2] pwm: sifive: change the PWM controlled LED algorithm

[Geert Uytterhoeven]

Hi Nylon,

On Mon, Jan 30, 2023 at 10:32 AM Nylon Chen <nylon.chen@xxxxxxxxxx> wrote:
> The `frac` variable represents the pulse inactive time, and the result of
> this algorithm is the pulse active time. Therefore, we must reverse the
> result.
>
> The reference is SiFive FU740-C000 Manual[0].
>
> [0]: https://sifive.cdn.prismic.io/sifive/1a82e600-1f93-4f41-b2d8-86ed8b16acba_fu740-c000-manual-v1p6.pdf
>
> Signed-off-by: Nylon Chen <nylon.chen@xxxxxxxxxx>

Thanks for your patch!

> --- a/drivers/pwm/pwm-sifive.c
> +++ b/drivers/pwm/pwm-sifive.c
> @@ -158,6 +158,7 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>         frac = DIV64_U64_ROUND_CLOSEST(num, state->period);
>         /* The hardware cannot generate a 100% duty cycle */
>         frac = min(frac, (1U << PWM_SIFIVE_CMPWIDTH) - 1);
> +       frac = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - frac;

Shouldn't the inversion be done before the hardware limitation fixup?

>
>         mutex_lock(&ddata->lock);
>         if (state->period != ddata->approx_period) {

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds