Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1580293

Re: [PATCH 1/6] clocksource: sh_cmt: compute rate before registration again

Show all headers | View raw

On Mon, Feb 6, 2017 at 1:11 PM, Nicolai Stange <nicstange@gmail.com> wrote:
> With the upcoming NTP correction related rate adjustments to be implemented
> in the clockevents core, the latter needs to get informed about every rate
> change of a clockevent device made after its registration.
>
> Currently, sh_cmt violates this requirement in that it registers its
> clockevent device with a dummy rate and sets its final ->mult and ->shift
> values from its ->set_state_oneshot() and ->set_state_periodic() functions
> respectively.
>
> This patch moves the setting of the clockevent device's ->mult and ->shift
> values to before its registration.
>
> Note that there has been some back and forth regarding this question with
> respect to the clocksource also provided by this driver:
>   commit f4d7c3565c16 ("clocksource: sh_cmt: compute mult and shift before
>                         registration")
> moves the rate determination from the clocksource's ->enable() function to
> before its registration. OTOH, the later
>   commit 3593f5fe40a1 ("clocksource: sh_cmt: __clocksource_updatefreq_hz()
>                         update")
> basically reverts this, saying
>   "Without this patch the old code uses clocksource_register() together
>    with a hack that assumes a never changing clock rate."
>
> However, I checked all current sh_cmt users in arch/sh as well as in
> arch/arm/mach-shmobile carefully and right now, none of them changes any
> rate in any clock tree relevant to sh_cmt after their respective
> time_init(). Since all sh_cmt instances are created after time_init(), none
> of them should ever observe any clock rate changes.
>
> What's more, both, a clocksource as well as a clockevent device, can
> immediately get selected for use at their registration and thus, enabled
> at this point already. So it's probably safer to assume a "never changing
> clock rate" here.
>
> - Move the struct sh_cmt_channel's ->rate member to struct sh_cmt_device:
>   it's a property of the underlying clock which is in turn specific to
>   the sh_cmt_device.
> - Determine the ->rate value in sh_cmt_setup() at device probing rather
>   than at first usage.
> - Set the clockevent device's ->mult and ->shift values right before its
>   registration.
> - Although not strictly necessary for the upcoming clockevent core changes,
>   set the clocksource's rate at its registration for consistency.

Magnus: Do you have any objection to the above?  I know we reworked
this a few times in the past.

thanks
-john

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 0/6] adapt clockevents frequencies to mono clock: prerequisites 1 Nicolai Stange <nicstange@gmail.com> - 2017-02-06 22:20 +0100
  [PATCH 5/6] clocksource: h8300_timer8: don't reset rate in ->set_state_oneshot() Nicolai Stange <nicstange@gmail.com> - 2017-02-06 22:20 +0100
  [PATCH 1/6] clocksource: sh_cmt: compute rate before registration again Nicolai Stange <nicstange@gmail.com> - 2017-02-06 22:20 +0100
    Re: [PATCH 1/6] clocksource: sh_cmt: compute rate before registration again John Stultz <john.stultz@linaro.org> - 2017-02-14 05:40 +0100
  [PATCH 4/6] clocksource: em_sti: compute rate before registration Nicolai Stange <nicstange@gmail.com> - 2017-02-06 22:20 +0100
  [PATCH 6/6] clockevents: make clockevents_config() static Nicolai Stange <nicstange@gmail.com> - 2017-02-06 22:20 +0100
  [PATCH 2/6] clocksource: sh_tmu: compute rate before registration again Nicolai Stange <nicstange@gmail.com> - 2017-02-06 22:20 +0100
  [PATCH 3/6] clocksource: em_sti: split clock prepare and enable steps Nicolai Stange <nicstange@gmail.com> - 2017-02-06 22:20 +0100

csiph-web