| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20170328145814.GE2123@mai>
Date: Tue, 28 Mar 2017 16:58:14 +0200
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: fu.wei@...aro.org
Cc: rjw@...ysocki.net, lenb@...nel.org, tglx@...utronix.de,
marc.zyngier@....com, mark.rutland@....com,
lorenzo.pieralisi@....com, sudeep.holla@....com,
hanjun.guo@...aro.org, linux-arm-kernel@...ts.infradead.org,
linaro-acpi@...ts.linaro.org, linux-kernel@...r.kernel.org,
linux-acpi@...r.kernel.org, rruigrok@...eaurora.org,
harba@...eaurora.org, cov@...eaurora.org, timur@...eaurora.org,
graeme.gregory@...aro.org, al.stone@...aro.org, jcm@...hat.com,
wei@...hat.com, arnd@...db.de, catalin.marinas@....com,
will.deacon@....com, Suravee.Suthikulpanit@....com,
leo.duran@....com, wim@...ana.be, linux@...ck-us.net,
linux-watchdog@...r.kernel.org, tn@...ihalf.com,
christoffer.dall@...aro.org, julien.grall@....com
Subject: Re: [PATCH v22 02/11] clocksource: arm_arch_timer: separate out
device-tree code and remove arch_timer_detect_rate
On Wed, Mar 22, 2017 at 12:31:13AM +0800, fu.wei@...aro.org wrote:
> From: Fu Wei <fu.wei@...aro.org>
>
> Currently, the counter frequency detection call(arch_timer_detect_rate)
> includes getting the frequency from the device-tree property, the per-cpu
> arch-timer and the memory-mapped (MMIO) timer interfaces.
> But reading device-tree property will be needed only when system boot with
> device-tree, and reading from the per-cpu arch-timer and the memory-mapped
> (MMIO) timer interfaces will be needed only when the system initializes
> the relevant timer.
>
> This patch separates out device-tree code, keep them in device-tree init
> function, and removes arch_timer_detect_rate founction, then uses the
> arch_timer_get_cntfrq and arch_timer_mem_get_cntfrq directly.
>
> Signed-off-by: Fu Wei <fu.wei@...aro.org>
> ---
> drivers/clocksource/arm_arch_timer.c | 58 +++++++++++++++++++-----------------
> 1 file changed, 30 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 843f923..29ca7d6 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -560,30 +560,6 @@ static u32 arch_timer_mem_get_cntfrq(void __iomem *cntbase)
> return readl_relaxed(cntbase + CNTFRQ);
> }
>
> -static void
> -arch_timer_detect_rate(void __iomem *cntbase, struct device_node *np)
> -{
> - /* 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 (!acpi_disabled ||
> - of_property_read_u32(np, "clock-frequency", &arch_timer_rate)) {
> - if (cntbase)
> - arch_timer_rate = arch_timer_mem_get_cntfrq(cntbase);
> - else
> - arch_timer_rate = arch_timer_get_cntfrq();
> - }
> -
> - /* Check the timer frequency. */
> - if (arch_timer_rate == 0)
> - pr_warn("frequency not available\n");
> -}
> -
> static void arch_timer_banner(unsigned type)
> {
> pr_info("%s%s%s timer(s) running at %lu.%02luMHz (%s%s%s).\n",
> @@ -958,7 +934,17 @@ static int __init arch_timer_of_init(struct device_node *np)
> for (i = ARCH_TIMER_PHYS_SECURE_PPI; i < ARCH_TIMER_MAX_TIMER_PPI; i++)
> arch_timer_ppi[i] = irq_of_parse_and_map(np, i);
>
> - arch_timer_detect_rate(NULL, np);
> + /*
> + * Try to determine the frequency from the device tree,
> + * if fail, get the frequency from the sysreg CNTFRQ.
> + */
> + if (!arch_timer_rate &&
This variable is set only if "arm,armv7-timer" and "arm,armv7-timer-mem" are
declared together in the DT, right ?
Two declarations for a single variable ? Ignore the !arch_timer_rate.
> + of_property_read_u32(np, "clock-frequency", &arch_timer_rate))
> + arch_timer_rate = arch_timer_get_cntfrq();
> + if (!arch_timer_rate) {
> + pr_err(FW_BUG "frequency not available.\n");
> + return -EINVAL;
> + }
Please, clarify this block, the conditions are unclear.
>
> arch_timer_c3stop = !of_property_read_bool(np, "always-on");
>
> @@ -1069,7 +1055,19 @@ static int __init arch_timer_mem_init(struct device_node *np)
> goto out;
> }
>
> - arch_timer_detect_rate(base, np);
> + /*
> + * Try to determine the frequency from the device tree,
> + * if fail, get the frequency from the CNTFRQ reg of MMIO timer.
> + */
> + if (!arch_timer_rate &&
> + of_property_read_u32(np, "clock-frequency", &arch_timer_rate))
> + arch_timer_rate = arch_timer_mem_get_cntfrq(base);
> + if (!arch_timer_rate) {
> + pr_err(FW_BUG "MMIO frequency not available.\n");
> + ret = -EINVAL;
> + goto out;
> + }
> +
> ret = arch_timer_mem_register(base, irq);
> if (ret)
> goto out;
> @@ -1130,8 +1128,12 @@ static int __init arch_timer_acpi_init(struct acpi_table_header *table)
> map_generic_timer_interrupt(gtdt->non_secure_el2_interrupt,
> gtdt->non_secure_el2_flags);
>
> - /* Get the frequency from CNTFRQ */
> - arch_timer_detect_rate(NULL, NULL);
> + /* Get the frequency from the sysreg CNTFRQ */
> + arch_timer_rate = arch_timer_get_cntfrq();
> + if (!arch_timer_rate) {
> + pr_err(FW_BUG "frequency not available.\n");
> + return -EINVAL;
> + }
>
> arch_timer_uses_ppi = arch_timer_select_ppi();
> if (!arch_timer_ppi[arch_timer_uses_ppi]) {
> --
> 2.9.3
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
Powered by blists - more mailing lists