Hi Geert-san,
Thank you for the review!
> Sent: Thursday, May 21, 2015 5:30 PM
< snip >
> > --- /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.
Oops. I will remove it.
> > +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.
Thank you for the point. I will use it.
> > + do_div(tmp, one_cycle);
> > + cyc = ((u32)tmp << RCAR_PWMCNT_CYC0_SHIFT) & RCAR_PWMCNT_CYC0_MASK;
>
> I think the cast is not needed.
Thank you for the point. I will remove it.
> > +
> > + tmp = duty_ns * 100;
>
> Same here: "tmp = duty_ns * 100ULL;".
Thank you again.
> > + do_div(tmp, one_cycle);
> > + ph = (u32)tmp & RCAR_PWMCNT_PH0_MASK;
>
> I think the cast is not needed.
Thank you again.
Best regards,
Yoshihiro Shimoda
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@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
