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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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 -- 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
��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux