Re: [PATCH v3 2/2] pwm: Add support for R-Car PWM Timer

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Shimoda-san,

On Thu, May 21, 2015 at 4:50 AM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> This patch adds support for R-Car SoCs PWM Timer.

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/pwm/pwm-rcar.c

> +#define RCAR_PWM_CH_OFFSET     0x1000

This is no longer used, now each pwm instance has its own device node.

> +static void rcar_pwm_set_counter(struct rcar_pwm_chip *rp, int div,
> +                                int duty_ns, int period_ns)
> +{
> +       unsigned long long one_cycle, tmp;      /* 0.01 nanoseconds */
> +       unsigned long clk_rate = clk_get_rate(rp->clk);
> +       u32 cyc, ph;
> +
> +       one_cycle = (unsigned long long)NSEC_PER_SEC * 100 * (1 << div);
> +       do_div(one_cycle, clk_rate);
> +
> +       tmp = period_ns * 100;

period_ns and the constant 100 are both int, hence the multiplication may
still overflow.
Please use e.g. "tmp = period_ns * 100ULL" instead.

> +       do_div(tmp, one_cycle);
> +       cyc = ((u32)tmp << RCAR_PWMCNT_CYC0_SHIFT) & RCAR_PWMCNT_CYC0_MASK;

I think the cast is not needed.

> +
> +       tmp = duty_ns * 100;

Same here: "tmp = duty_ns * 100ULL;".

> +       do_div(tmp, one_cycle);
> +       ph = (u32)tmp & RCAR_PWMCNT_PH0_MASK;

I think the cast is not needed.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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