> > > + /* 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html