Hi Paul, > From: Paul Walmsley [mailto:paul@xxxxxxxxx] > Sent: Wednesday, October 28, 2009 9:39 PM > > +static void lookup_dco_sddiv(struct clk *clk, u8 *dco, u8 *sd_div, u16 m, > u8 n) > > +{ > > + unsigned long fint, clkinp, sd; /* watch out for overflow */ > > + int mod1, mod2; > > + > > + n++; /* always n+1 below */ > > The N that's passed into lookup_dco_sddiv() is the real N -- that is, it's > the register bitfield plus one. That can be seen below in this line: > > > v |= (n - 1) << __ffs(dd->div1_mask); > > Given this, is the "n++;" above correct? Probably not... Actually much of the original function path (setting dpll4) is a bit suspect. Reprogramming of PER DPLL at kernel time would seem risky. The service provided in this function maybe should only be available at early init time. There is no notification in clock receiving drivers. For instance an active UART would crash. > The rest of the code looks fine. (Of course, I can't review the > technical basis of the code since I don't have the 3630 docs yet, but I'm > fine with taking it in the meantime.) > > I might change the 'u8 jtype;' field below into 'u8 flags;'; please let me > know if you foresee a problem with that. Don't care. Jtype seemed description but flag |= jtype is just as descriptive and can hold a few more. Thanks for looking over. Regards, Richard W. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
