On Tue, 2015-03-24 at 09:59 -0700, Mark Brown wrote:
> On Tue, Mar 24, 2015 at 05:43:22PM +0200, Andy Shevchenko wrote:
> > The Quark SoC data sheet describes the baud rate setting using fractional
> > divider. The subset of possible values represented by a table suggests that
> > the
> > divisor has one block that could divide by 5. This explains a satan number
> > in
> > some cases in the table Thus, in this particular case the divisor can be
> > evaluated as
>
> What is a "satan number"?
Number of the beast 666.
>
> > + /* Quark SoC has 200MHz xtal */
> > + unsigned long xtal = 200000000, fref = xtal / 2;
> > + /* Define two reference frequencies */
> > + unsigned long fref1 = fref / 2, fref2 = fref * 2 / 5;
>
> The Linux coding style generally frowns on multiple initializaitons on a
> single line for legibility reasons.
Not a problem, however they are tighten to each other.
>
> > + /* Choose the best between two */
> > + if (r2 >= r1 || q2 > 256) {
> > + r = r1;
> > + q = q1;
> > + mul = mul1;
> > + } else {
> > + r = r2;
> > + q = q2;
> > + mul = (1 << 24) * 2 / 5; /* 0x666666 */
> > + }
>
> Making the comments on this a bit more explicit might be helpful - a
> mention of why it's better to pick pick one of the settings or the other
> would help.
Okay, I will make it more verbose.
>
> > + /* In case the divisor is big enough */
> > + if (fref / rate >= 80) {
> > + u64 fssp;
> > +
> > + /* Calculate initial quot */
> > + q1 = DIV_ROUND_CLOSEST(fref, rate);
> > + mul1 = (1 << 24) / q1;
> > +
> > + /* Get the remainder */
> > + fssp = (u64)fref * mul1;
> > + do_div(fssp, 1 << 24);
> > + r1 = abs(fssp - rate);
> > +
> > + /* Choose this one if it suits better */
> > + if (r1 < r) {
> > + q = 1;
> > + mul = mul1;
> > + }
> > + }
>
> So what do we do if the divisor is not big enough?
Then we choose one of the first two variants.
> I'm not sure that
> one could reasonably expect someone to follow this code without doing
> detective work. I think the biggest thing is that the comments aren't
> saying why the code is doing what it's doing.
Should I cite data sheet in case to explain some branches?
--
Andy Shevchenko <[email protected]>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
