Re: [RFC][PATCH 3/3] Try to convert non-trivial clocksources to clocksource_register_hz

[Jim Cromie]

On Mon, May 10, 2010 at 8:04 PM, john stultz <johnstul@xxxxxxxxxx> wrote:
> On Mon, 2010-05-10 at 17:33 -0400, Jim Cromie wrote:
>> On Fri, Apr 30, 2010 at 9:36 PM, John Stultz <johnstul@xxxxxxxxxx> wrote:
>> hi John,
>>
>> On casual inspection, this looks good, however
>> I wont be able to try it on the hardware til early June earliest.
>>
>> I have a few qs, (idle musings really)
>>
>> - scx200_hrt cannot be rmmod'd, due iirc to the clocksource design.
>> This is not a problem, but I wonder if it might be added later, and what
>> it might mean to the drivers (in a new module_exit())
>
>
> I've not really heard much of a compelling argument for clocksource
> driver removal, nor has anyone wanted to put much effort there so far.
> That said, I'm not opposed to it right off.  It just not a priority.
>

OK.  Just to be clear, Im not gonna make that argument.
I was imagining dragons there anyway.

>
>> - HRT_SHIFT_1/_27 macros are removed, and now calculated in clocksource code.
>> I chose a 1:16 shift ratio for the 1:27 input clocks.  I suppose I could have
>> used 1:32, I dont recall thinking of it or trying it then.
>> What would/should your new code do ?
>
> I'm not sure I'm following what you mean with the ratios, but the new
> code will select the largest shift value possible, where the resulting
> mult won't overflow 64bits when multiplied against 5 seconds (that
> interval length may be changed in the future) worth of cycles.
>

I hesitate to pursue this (musings), but:
the SHIFT_1, _27 values were 26,22, hence the 16:1 ratio.
the fastclock is 27* the slowclock, which is closer to 32:1 than 16:1,
so one of my SHIFT values is probably suboptimal.
Your new code will fix my error :-)
I'll probably add a printk just to see what shift, mult terms you compute.

>
>> - when writing scx200_hrt, I chose to adjust by ppm, not +/- hz
>> partly cuz it was easier to describe in a 1-line mod-desc, and partly
>> cuz 1/27ppm vernier-knob is overkill on a $0.25 crystal.
>> The new API name (with _hz suffix) suggests that I could change
>> the mod-param-name, and add it in after the 1:27 multiplier.
>
> I believe its still mathematically identical to what you had before, but
> let me know if I'm mistaken here.

Your changes look faithful to the original.
what I meant was changing the meaning of the param, and the computation:

+       freq = HRT_FREQ;
+       if (mhz27)
+               freq *= 27;
+       freq += hz_adjust;

This would give finer (apparent) control when using 27MHz clock,
though temp swings would still move real frequency much more
than the minimum adjustment (hence only an appearance of accuracy).

So, is this proposed parameter-name change an improvement, or needless churn ?
Obviously, I should ask this Q with a patch on top of yours :-}

>
> thanks
> -john
>

no, thank you
-jim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/