Re: [PATCH 1/5] clk: mvebu: Add core-divider clock

[Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>]

> > > +	/* 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html