Hi Richard, Nishanth (adding Ari back in to cc, also Tony, Kevin) On Sat, 31 Oct 2009, Woodruff, Richard wrote: > > 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... Great - so can we count on you and/or Nishanth to update this patch taking this and Ari's comments into account and to resend to the list? > > 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. OK. > Thanks for looking over. You're welcome. - Paul -- 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
