[PATCH 1/5] clk: mvebu: Add core-divider clock
Andrew Lunn
andrew at lunn.ch
Thu Sep 26 11:56:56 EDT 2013
> > > + /* Valid ratio are 1:4, 1:5, 1:6 and 1:8 */
> > > + u32 div;
> > > +
> > > + div = *parent_rate / rate;
> > > + if (div <= 4)
> > > + div = 4;
> > > + else if (div <= 5)
> > > + div = 5;
> > > + else if (div <= 6)
> > > + div = 6;
> > > + else
> > > + div = 8;
> > > +
> > > + return *parent_rate / div;
> > > +}
> >
> > This looks odd. Is not the following clearer?
> >
> > div = *parent_rate / rate;
> > if (div < 5)
> > div = 4;
> > else if (div > 6)
> > div = 8;
> >
>
> Mmmm... no, it's not at all clearer to me.
> IMHO, the original construction explicitly show the possible ratios:
>
> /* If it's smaller than or equal to 4, set to 4 */
> if (div <= 4)
> div = 4;
>
> /* Otherwise, if it's between 4 and 5, set to 5 */
> else if (div <= 5)
> div = 5;
If div was a float or double, i would probably agree. But its a u32.
It cannot be between 4 and 5. It must be 5. So it becomes
if (div <= 4)
div = 4;
else if (div == 5)
div = 5;
else if (div == 6)
div = 6
else
div = 8
Those two middle statements look odd to me...
Andrew
More information about the linux-arm-kernel
mailing list