>> @@ -645,7 +646,7 @@ static long si5351_msynth_round_rate(struct clk_hw *hw, >> unsigned long rate, >> struct si5351_hw_data *hwdata = >> container_of(hw, struct si5351_hw_data, hw); >> unsigned long long lltmp; >> - unsigned long a, b, c; >> + unsigned long a, b = 0, c = 1; > > Actually, moving b,c initialization up here is neither related > to the patch itself nor is it mentioned in the commit log. > > Please do not mix different patches.
I agree. I just thought we could avoid repeating the b,c initialization in the code, but You are right, that would be a different patch, if any at all. >> *parent_rate = a * rate; > >> + if (rfrac) >> + rational_best_approximation(rfrac, denom, >> + SI5351_MULTISYNTH_B_MAX, >> + SI5351_MULTISYNTH_C_MAX, >> + &b, &c); >> + } > > I am still not convinced that this is the right way to calculate the > _best_ integer divider for ms6,7. > > The code above is written to (a) find the _largest_ integer "a" that > will match > > a * rate <= parent_rate > > and then (b) determines the fractional part of the divider. > > As you correctly stated, ms6,7 do not support (b) but if we use (a) for > those msynths, we will determine an "a" so that "a * rate" is always > smaller than parent_rate. > > What we actually want is the smallest error between generated and > requested rate, so we have to find the closest integer with > > a = DIV_ROUND_CLOSEST(parent_rate, rate) Agreed, that's probably better. > IMHO, the special divider calculation for ms6,7 is best dealt with > in an extra else-if branch after the check for CLK_SET_RATE_PARENT > flag. Agreed. > hwdata->params.p3 = 0; > hwdata->params.p2 = 0; Agreed. > Out of curiosity, do you actually have a device that uses ms6,7 and can > you measure the generated frequency - or did you just read the code as > a bedtime story? ;) :) I do have a device and have to use ms6,7, those are connected to pllb. I have tested some integer pll dividers in a range from 30 to 130. Measured frequencies and the frequencies shown in <debugfs>/clk/clk_summery are the same. I am going to send a patch v3. Thanks for your feedback. Sergej -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
