Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1525687
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v16 07/15] clocksource/drivers/arm_arch_timer: Refactor arch_timer_detect_rate to keep dt code in *_of_init |
| Date | 2016-11-18 21:00 +0100 |
| Message-ID | <sEXuh-32H-19@gated-at.bofh.it> (permalink) |
| References | <sE8L7-3uw-15@gated-at.bofh.it> <sE8UN-3xZ-5@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Wed, Nov 16, 2016 at 09:49:00PM +0800, fu.wei@linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
>
> The patch refactor original arch_timer_detect_rate function:
> (1) Separate out device-tree code, keep them in device-tree init
> function:
> arch_timer_of_init,
> arch_timer_mem_init;
Please write a real commit message.
> (2) Improve original mechanism, if getting from memory-mapped timer
> fail, try arch_timer_get_cntfrq() again.
This is *not* a refactoring. It's completely unrelated to the supposed
refactoring from point (1), and if necessary, should be a separate
patch.
*Why* are you maknig this change? Does some ACPI platform have an MMIO
timer with an ill-configured CNTFRQ register? If so, report that to the
vendor. Don't add yet another needless bodge.
I'd really like to split the MMIO and CP15 timers, and this is yet
another hack that'll make it harder to do so.
> Signed-off-by: Fu Wei <fu.wei@linaro.org>
> ---
> drivers/clocksource/arm_arch_timer.c | 45 +++++++++++++++++++++++-------------
> 1 file changed, 29 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index af22953..fe4e812 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -487,27 +487,31 @@ static int arch_timer_starting_cpu(unsigned int cpu)
> return 0;
> }
>
> -static void
> -arch_timer_detect_rate(void __iomem *cntbase, struct device_node *np)
> +static void arch_timer_detect_rate(void __iomem *cntbase)
> {
> /* Who has more than one independent system counter? */
> if (arch_timer_rate)
> return;
> -
> /*
> - * Try to determine the frequency from the device tree or CNTFRQ,
> - * if ACPI is enabled, get the frequency from CNTFRQ ONLY.
> + * If we got memory-mapped timer(cntbase != NULL),
> + * try to determine the frequency from CNTFRQ in memory-mapped timer.
> */
*WHY* ?
If we're sharing arch_timer_rate across MMIO and sysreg timers, the
sysreg value is alreayd sufficient.
If we're not, they should be completely independent.
> - if (!acpi_disabled ||
> - of_property_read_u32(np, "clock-frequency", &arch_timer_rate)) {
> - if (cntbase)
> - arch_timer_rate = readl_relaxed(cntbase + CNTFRQ);
> - else
> - arch_timer_rate = arch_timer_get_cntfrq();
> - }
> + if (cntbase)
> + arch_timer_rate = readl_relaxed(cntbase + CNTFRQ);
> + /*
> + * Because in a system that implements both Secure and
> + * Non-secure states, CNTFRQ is only accessible in Secure state.
That's true for the CNTCTLBase frame, but that doesn't matter.
The CNTBase<n> frames should have a readable CNTFRQ.
> + * So the operation above may fail, even if (cntbase != NULL),
> + * especially on ARM64.
> + * In this case, we can try cntfrq_el0(system coprocessor register).
> + */
> + if (!arch_timer_rate)
> + arch_timer_rate = arch_timer_get_cntfrq();
> + else
> + return;
Urrgh.
Please have separate paths to determine the MMIO frequency and the
sysreg frequency, and use the appropriate one for the counter you want
to know the frequency of.
Thanks,
Mark.
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH v16 00/15] acpi, clocksource: add GTDT driver and GTDT support in arm_arch_timer fu.wei@linaro.org - 2016-11-16 14:50 +0100
[PATCH v16 07/15] clocksource/drivers/arm_arch_timer: Refactor arch_timer_detect_rate to keep dt code in *_of_init fu.wei@linaro.org - 2016-11-16 15:00 +0100
Re: [PATCH v16 07/15] clocksource/drivers/arm_arch_timer: Refactor arch_timer_detect_rate to keep dt code in *_of_init Mark Rutland <mark.rutland@arm.com> - 2016-11-18 21:00 +0100
Re: [PATCH v16 07/15] clocksource/drivers/arm_arch_timer: Refactor arch_timer_detect_rate to keep dt code in *_of_init Fu Wei <fu.wei@linaro.org> - 2016-11-21 15:10 +0100
[PATCH v16 12/15] clocksource/drivers/arm_arch_timer: Simplify ACPI support code. fu.wei@linaro.org - 2016-11-16 15:00 +0100
[PATCH v16 13/15] acpi/arm64: Add memory-mapped timer support in GTDT driver fu.wei@linaro.org - 2016-11-16 15:00 +0100
Re: [PATCH v16 13/15] acpi/arm64: Add memory-mapped timer support in GTDT driver Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2016-11-18 15:30 +0100
Re: [PATCH v16 13/15] acpi/arm64: Add memory-mapped timer support in GTDT driver Fu Wei <fu.wei@linaro.org> - 2016-11-23 13:00 +0100
Re: [PATCH v16 13/15] acpi/arm64: Add memory-mapped timer support in GTDT driver Fu Wei <fu.wei@linaro.org> - 2016-11-24 05:00 +0100
[PATCH v16 10/15] clocksource/drivers/arm_arch_timer: Refactor the timer init code to prepare for GTDT fu.wei@linaro.org - 2016-11-16 15:00 +0100
Re: [PATCH v16 10/15] clocksource/drivers/arm_arch_timer: Refactor the timer init code to prepare for GTDT Mark Rutland <mark.rutland@arm.com> - 2016-11-18 21:10 +0100
Re: [PATCH v16 10/15] clocksource/drivers/arm_arch_timer: Refactor the timer init code to prepare for GTDT Fu Wei <fu.wei@linaro.org> - 2016-11-23 07:20 +0100
[PATCH v16 11/15] acpi/arm64: Add GTDT table parse driver fu.wei@linaro.org - 2016-11-16 15:00 +0100
Re: [PATCH v16 11/15] acpi/arm64: Add GTDT table parse driver Mark Rutland <mark.rutland@arm.com> - 2016-11-18 21:20 +0100
Re: [PATCH v16 11/15] acpi/arm64: Add GTDT table parse driver Fu Wei <fu.wei@linaro.org> - 2016-11-23 13:10 +0100
[PATCH v16 04/15] clocksource/drivers/arm_arch_timer: rename some enums and defines, and some cleanups. fu.wei@linaro.org - 2016-11-16 15:00 +0100
Re: [PATCH v16 04/15] clocksource/drivers/arm_arch_timer: rename some enums and defines, and some cleanups. Mark Rutland <mark.rutland@arm.com> - 2016-11-18 20:00 +0100
Re: [PATCH v16 04/15] clocksource/drivers/arm_arch_timer: rename some enums and defines, and some cleanups. Fu Wei <fu.wei@linaro.org> - 2016-11-21 07:20 +0100
[PATCH v16 01/15] clocksource/drivers/arm_arch_timer: Move enums and defines to header file fu.wei@linaro.org - 2016-11-16 15:00 +0100
[PATCH v16 15/15] acpi/arm64: Add SBSA Generic Watchdog support in GTDT driver fu.wei@linaro.org - 2016-11-16 15:00 +0100
[PATCH v16 14/15] clocksource/drivers/arm_arch_timer: Add GTDT support for memory-mapped timer fu.wei@linaro.org - 2016-11-16 15:00 +0100
Re: [PATCH v16 14/15] clocksource/drivers/arm_arch_timer: Add GTDT support for memory-mapped timer Mark Rutland <mark.rutland@arm.com> - 2016-11-18 21:30 +0100
Re: [PATCH v16 14/15] clocksource/drivers/arm_arch_timer: Add GTDT support for memory-mapped timer Fu Wei <fu.wei@linaro.org> - 2016-11-23 13:20 +0100
[PATCH v16 08/15] clocksource/drivers/arm_arch_timer: Refactor arch_timer_needs_probing, and call it only if acpi disabled. fu.wei@linaro.org - 2016-11-16 15:00 +0100
Re: [PATCH v16 08/15] clocksource/drivers/arm_arch_timer: Refactor arch_timer_needs_probing, and call it only if acpi disabled. Mark Rutland <mark.rutland@arm.com> - 2016-11-18 21:00 +0100
Re: [PATCH v16 08/15] clocksource/drivers/arm_arch_timer: Refactor arch_timer_needs_probing, and call it only if acpi disabled. Fu Wei <fu.wei@linaro.org> - 2016-11-21 15:40 +0100
[PATCH v16 06/15] clocksource/drivers/arm_arch_timer: separate out arch_timer_uses_ppi init code to prepare for GTDT. fu.wei@linaro.org - 2016-11-16 15:00 +0100
Re: [PATCH v16 06/15] clocksource/drivers/arm_arch_timer: separate out arch_timer_uses_ppi init code to prepare for GTDT. Mark Rutland <mark.rutland@arm.com> - 2016-11-18 20:40 +0100
Re: [PATCH v16 06/15] clocksource/drivers/arm_arch_timer: separate out arch_timer_uses_ppi init code to prepare for GTDT. Fu Wei <fu.wei@linaro.org> - 2016-11-21 10:50 +0100
[PATCH v16 03/15] clocksource/drivers/arm_arch_timer: Improve printk relevant code fu.wei@linaro.org - 2016-11-16 15:00 +0100
[PATCH v16 02/15] clocksource/drivers/arm_arch_timer: Add a new enum for spi type fu.wei@linaro.org - 2016-11-16 15:00 +0100
[PATCH v16 09/15] clocksource/drivers/arm_arch_timer: Introduce some new structs to prepare for GTDT fu.wei@linaro.org - 2016-11-16 15:00 +0100
[PATCH v16 05/15] clocksource/drivers/arm_arch_timer: fix a bug in arch_timer_register about arch_timer_uses_ppi fu.wei@linaro.org - 2016-11-16 15:00 +0100
Re: [PATCH v16 05/15] clocksource/drivers/arm_arch_timer: fix a bug in arch_timer_register about arch_timer_uses_ppi Mark Rutland <mark.rutland@arm.com> - 2016-11-18 20:00 +0100
Re: [PATCH v16 05/15] clocksource/drivers/arm_arch_timer: fix a bug in arch_timer_register about arch_timer_uses_ppi Fu Wei <fu.wei@linaro.org> - 2016-11-21 08:40 +0100
Re: [PATCH v16 00/15] acpi, clocksource: add GTDT driver and GTDT support in arm_arch_timer Xiongfeng Wang <wangxiongfeng2@huawei.com> - 2016-11-17 04:50 +0100
Re: [PATCH v16 00/15] acpi, clocksource: add GTDT driver and GTDT support in arm_arch_timer Xiongfeng Wang <wangxiongfeng2@huawei.com> - 2016-11-17 10:40 +0100
Re: [PATCH v16 00/15] acpi, clocksource: add GTDT driver and GTDT support in arm_arch_timer Fu Wei <fu.wei@linaro.org> - 2016-11-17 12:20 +0100
Re: [PATCH v16 00/15] acpi, clocksource: add GTDT driver and GTDT support in arm_arch_timer Daniel Lezcano <daniel.lezcano@linaro.org> - 2016-11-17 10:50 +0100
csiph-web