From: Fu Wei <redacted>
This patchset:
(1)Preparation for adding GTDT support in arm_arch_timer:
1. Introduce two functions to get the frequency from mmio and sysreg.
2. separate out device-tree code from arch_timer_detect_rate
3. remove arch_timer_detect_rate use arch_timer_get_*_freq directly
4. split arch_timer_rate for different types of timer
5. Refactor arch_timer_needs_probing, and move it into DT init call
6. Introduce some new structs and refactor the MMIO timer init code
for reusing some common code.
(2)Introduce ACPI GTDT parser: drivers/acpi/arm64/acpi_gtdt.c
Parse all kinds of timer in GTDT table of ACPI:arch timer,
memory-mapped timer and SBSA Generic Watchdog timer.
This driver can help to simplify all the relevant timer drivers,
and separate all the ACPI GTDT knowledge from them.
(3)Simplify ACPI code for arm_arch_timer
(4)Add GTDT support for ARM memory-mapped timer.
This patchset has been tested on the following platforms with ACPI enabled:
(1)ARM Foundation v8 model
Changelog:
v21: https://lkml.org/lkml/2017/2/6/
Introduce two functions to get the frequency from mmio and sysreg.
Remove arch_timer_detect_rate use arch_timer_get_*_freq directly
Split arch_timer_rate for different types of timer.
Skip secure timer frame in GTDT driver.
Rebase to git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git arch-timer/cleanup
(The first 6 patches in v20 have been merged into arch-timer/cleanup branch)
v20: https://lkml.org/lkml/2017/1/18/534
Reorder the first 4 patches and split the 4th patches.
Leave CNTHCTL_* as they originally were.
Fix the bug in arch_timer_select_ppi.
Split "Rework counter frequency detection" patch.
Rework the arch_timer_detect_rate function.
Improve the commit message of "Refactor MMIO timer probing".
Rebase to 4.10.0-rc4
v19: https://lkml.org/lkml/2016/12/21/25
Fix a '\n' missing in a error message in arch_timer_mem_init.
Add "request_mem_region" for ioremapping cntbase, according to
f947ee1 clocksource/drivers/arm_arch_timer: Map frame with of_io_request_and_map()
Rebase to 4.9.0-gfb779ff
v18: https://lkml.org/lkml/2016/12/8/446
Fix 8/15 patch problem of "int ret;" in arch_timer_acpi_init.
Rebase to 4.9.0-rc8-g9269898
v17: https://lkml.org/lkml/2016/11/25/140
Take out some cleanups from 4/15.
Merge 5/15 and 6/15, improve PPI determination code,
improve commit message.
Rework counter frequency detection.
Move arch_timer_needs_of_probing into DT init call.
Move Platform Timer scan loop back to timer init call to avoid allocating
and free memory.
Improve all the exported functions' comment.
v16: https://lkml.org/lkml/2016/11/16/268
Fix patchset problem about static enum ppi_nr of 01/13 in v15.
Refactor arch_timer_detect_rate.
Refactor arch_timer_needs_probing.
v15: https://lkml.org/lkml/2016/11/15/366
Re-order patches
Add arm_arch_timer refactoring patches to prepare for GTDT:
1. rename some enums and defines, and some cleanups
2. separate out arch_timer_uses_ppi init code and fix a potential bug
3. Improve some new structs, refactor the timer init code.
Since the some structs have been changed, GTDT parser for memory-mapped
timer and SBSA Generic Watchdog timer have been update.
v14: https://lkml.org/lkml/2016/9/28/573
Separate memory-mapped timer GTDT support into two patches
1. Refactor the timer init code to prepare for GTDT
2. Add GTDT support for memory-mapped timer
v13: http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1231717.html
Improve arm_arch_timer code for memory-mapped
timer GTDT support, refactor original memory-mapped timer
dt support for reusing some common code.
v12: https://lkml.org/lkml/2016/9/13/250
Rebase to latest Linux 4.8-rc6
Delete the confusing "skipping" in the error message.
V11: https://lkml.org/lkml/2016/9/6/354
Rebase to latest Linux 4.8-rc5
Delete typedef (suggested by checkpatch.pl)
V10: https://lkml.org/lkml/2016/7/26/215
Drop the "readq" patch.
Rebase to latest Linux 4.7.
V9: https://lkml.org/lkml/2016/7/25/345
Improve pr_err message in acpi gtdt driver.
Update Commit message for 7/9
shorten the irq mapping function name
Improve GTDT driver for memory-mapped timer
v8: https://lkml.org/lkml/2016/7/19/660
Improve "pr_fmt(fmt)" definition: add "ACPI" in front of "GTDT",
and also improve printk message.
Simplify is_timer_block and is_watchdog.
Merge acpi_gtdt_desc_init and gtdt_arch_timer_init into acpi_gtdt_init();
Delete __init in include/linux/acpi.h for GTDT API
Make ARM64 select GTDT.
Delete "#include <linux/module.h>" from acpi_gtdt.c
Simplify GT block parse code.
v7: https://lkml.org/lkml/2016/7/13/769
Move the GTDT driver to drivers/acpi/arm64
Add add the ARM64-specific ACPI Support maintainers in MAINTAINERS
Merge 3 patches of GTDT parser driver.
Fix the for_each_platform_timer bug.
v6: https://lkml.org/lkml/2016/6/29/580
split the GTDT driver to 4 parts: basic, arch_timer, memory-mapped timer,
and SBSA Generic Watchdog timer
Improve driver by suggestions and example code from Daniel Lezcano
v5: https://lkml.org/lkml/2016/5/24/356
Sorting out all patches, simplify the API of GTDT driver:
GTDT driver just fills the data struct for arm_arch_timer driver.
v4: https://lists.linaro.org/pipermail/linaro-acpi/2016-March/006667.html
Delete the kvm relevant patches
Separate two patches for sorting out the code for arm_arch_timer.
Improve irq info export code to allow missing irq info in GTDT table.
v3: https://lkml.org/lkml/2016/2/1/658
Improve GTDT driver code:
(1)improve pr_* by defining pr_fmt(fmt)
(2)simplify gtdt_sbsa_gwdt_init
(3)improve gtdt_arch_timer_data_init, if table is NULL, it will try
to get GTDT table.
Move enum ppi_nr to arm_arch_timer.h, and add enum spi_nr.
Add arm_arch_timer get ppi from DT and GTDT support for kvm.
v2: https://lkml.org/lkml/2015/12/2/10
Rebase to latest kernel version(4.4-rc3).
Fix the bug about the config problem,
use CONFIG_ACPI_GTDT instead of CONFIG_ACPI in arm_arch_timer.c
v1: The first upstreaming version: https://lkml.org/lkml/2015/10/28/553
Fu Wei (13):
clocksource: arm_arch_timer: introduce two functions to get the
frequency from mmio and sysreg.
clocksource: arm_arch_timer: separate out device-tree code from
arch_timer_detect_rate
clocksource: arm_arch_timer: remove arch_timer_detect_rate
clocksource: arm_arch_timer: split arch_timer_rate for different types
of timer
clocksource: arm_arch_timer: refactor arch_timer_needs_probing
clocksource: arm_arch_timer: move arch_timer_needs_of_probing into DT
init call
clocksource: arm_arch_timer: introduce some new structs to prepare for
GTDT
clocksource: arm_arch_timer: refactor MMIO timer probing.
acpi/arm64: Add GTDT table parse driver
clocksource: arm_arch_timer: simplify ACPI support code.
acpi/arm64: Add memory-mapped timer support in GTDT driver
clocksource: arm_arch_timer: add GTDT support for memory-mapped timer
acpi/arm64: Add SBSA Generic Watchdog support in GTDT driver
arch/arm64/Kconfig | 1 +
drivers/acpi/arm64/Kconfig | 3 +
drivers/acpi/arm64/Makefile | 1 +
drivers/acpi/arm64/gtdt.c | 378 +++++++++++++++++++++++++++++++++++
drivers/clocksource/arm_arch_timer.c | 354 ++++++++++++++++++++------------
drivers/watchdog/Kconfig | 1 +
include/clocksource/arm_arch_timer.h | 17 ++
include/linux/acpi.h | 7 +
8 files changed, 635 insertions(+), 127 deletions(-)
create mode 100644 drivers/acpi/arm64/gtdt.c
--
2.9.3
From: Fu Wei <redacted>
The patch introduce two new functions: arch_timer_get_sysreg_freq and
arch_timer_get_mmio_freq, and applys them in arch_timer_detect_rate.
These will be used for getting the frequency from mmio and sysreg to
prepare for reworking counter frequency detection.
Signed-off-by: Fu Wei <redacted>
---
drivers/clocksource/arm_arch_timer.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
@@ -554,6 +554,23 @@ static int arch_timer_starting_cpu(unsigned int cpu)return0;}+staticu32arch_timer_get_sysreg_freq(void)+{+/*+*TrytogetthefrequencyfromtheCNTFRQofsysreg.+*/+returnarch_timer_get_cntfrq();+}++staticu32arch_timer_get_mmio_freq(void__iomem*cntbase)+{+/*+*TrytogetthefrequencyfromtheCNTFRQoftimerframeregisters.+*Note:pleaseverifycntbaseincaller.+*/+returnreadl_relaxed(cntbase+CNTFRQ);+}+staticvoidarch_timer_detect_rate(void__iomem*cntbase,structdevice_node*np){
From: Fu Wei <redacted>
Currently, the counter frequency detection call(arch_timer_detect_rate)
include getting the frequency from the device-tree property.
But reading device-tree property will be needed only when system boot with
device-tree.
This patch separate out device-tree code, keep them in device-tree init
function.
Signed-off-by: Fu Wei <redacted>
Reviewed-by: Hanjun Guo <redacted>
Tested-by: Hanjun Guo <redacted>
---
drivers/clocksource/arm_arch_timer.c | 38 ++++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 15 deletions(-)
@@ -571,24 +571,19 @@ static u32 arch_timer_get_mmio_freq(void __iomem *cntbase)returnreadl_relaxed(cntbase+CNTFRQ);}-staticvoid-arch_timer_detect_rate(void__iomem*cntbase,structdevice_node*np)+staticvoidarch_timer_detect_rate(void__iomem*cntbase){/* Who has more than one independent system counter? */if(arch_timer_rate)return;/*-*TrytodeterminethefrequencyfromthedevicetreeorCNTFRQ,-*ifACPIisenabled,getthefrequencyfromCNTFRQONLY.+*TrytodeterminethefrequencyfromtheMMIOtimerorthesysreg.*/-if(!acpi_disabled||-of_property_read_u32(np,"clock-frequency",&arch_timer_rate)){-if(cntbase)-arch_timer_rate=arch_timer_get_mmio_freq(cntbase);-else-arch_timer_rate=arch_timer_get_sysreg_freq();-}+if(cntbase)+arch_timer_rate=arch_timer_get_mmio_freq(cntbase);+else+arch_timer_rate=arch_timer_get_sysreg_freq();/* Check the timer frequency. */if(arch_timer_rate==0)
@@ -969,7 +964,13 @@ 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);+/*+*Trytodeterminethefrequencyfromthedevicetree,+*iffail,getthefrequencyfromthesysregCNTFRQ.+*/+if(!arch_timer_rate&&+of_property_read_u32(np,"clock-frequency",&arch_timer_rate))+arch_timer_detect_rate(NULL);arch_timer_c3stop=!of_property_read_bool(np,"always-on");
@@ -1080,7 +1081,14 @@ static int __init arch_timer_mem_init(struct device_node *np)gotoout;}-arch_timer_detect_rate(base,np);+/*+*Trytodeterminethefrequencyfromthedevicetree,+*iffail,getthefrequencyfromtheCNTFRQregofMMIOtimer.+*/+if(!arch_timer_rate&&+of_property_read_u32(np,"clock-frequency",&arch_timer_rate))+arch_timer_detect_rate(base);+ret=arch_timer_mem_register(base,irq);if(ret)gotoout;
@@ -1141,8 +1149,8 @@ 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_detect_rate(NULL);arch_timer_uses_ppi=arch_timer_select_ppi();if(!arch_timer_ppi[arch_timer_uses_ppi]){
From: Fu Wei <redacted>
The original counter frequency detection call(arch_timer_detect_rate)
include getting the frequency from the per-cpu arch-timer and the
memory-mapped (MMIO) timer interfaces. But they will be needed only when
the system initializes the relevant timer.
This patch remove arch_timer_detect_rate founction, and use the
arch_timer_get_sysreg_freq and arch_timer_get_mmio_freq directly.
Signed-off-by: Fu Wei <redacted>
---
drivers/clocksource/arm_arch_timer.c | 38 +++++++++++++++---------------------
1 file changed, 16 insertions(+), 22 deletions(-)
@@ -571,25 +571,6 @@ static u32 arch_timer_get_mmio_freq(void __iomem *cntbase)returnreadl_relaxed(cntbase+CNTFRQ);}-staticvoidarch_timer_detect_rate(void__iomem*cntbase)-{-/* Who has more than one independent system counter? */-if(arch_timer_rate)-return;--/*-*TrytodeterminethefrequencyfromtheMMIOtimerorthesysreg.-*/-if(cntbase)-arch_timer_rate=arch_timer_get_mmio_freq(cntbase);-else-arch_timer_rate=arch_timer_get_sysreg_freq();--/* Check the timer frequency. */-if(arch_timer_rate==0)-pr_warn("frequency not available\n");-}-staticvoidarch_timer_banner(unsignedtype){pr_info("%s%s%s timer(s) running at %lu.%02luMHz (%s%s%s).\n",
@@ -970,7 +951,11 @@ static int __init arch_timer_of_init(struct device_node *np)*/if(!arch_timer_rate&&of_property_read_u32(np,"clock-frequency",&arch_timer_rate))-arch_timer_detect_rate(NULL);+arch_timer_rate=arch_timer_get_sysreg_freq();+if(!arch_timer_rate){+pr_err(FW_BUG"frequency not available.\n");+return-EINVAL;+}arch_timer_c3stop=!of_property_read_bool(np,"always-on");
@@ -1087,7 +1072,12 @@ static int __init arch_timer_mem_init(struct device_node *np)*/if(!arch_timer_rate&&of_property_read_u32(np,"clock-frequency",&arch_timer_rate))-arch_timer_detect_rate(base);+arch_timer_rate=arch_timer_get_mmio_freq(base);+if(!arch_timer_rate){+pr_err(FW_BUG"frequency not available for MMIO timer.\n");+ret=-EINVAL;+gotoout;+}ret=arch_timer_mem_register(base,irq);if(ret)
@@ -1150,7 +1140,11 @@ static int __init arch_timer_acpi_init(struct acpi_table_header *table)gtdt->non_secure_el2_flags);/* Get the frequency from the sysreg CNTFRQ */-arch_timer_detect_rate(NULL);+arch_timer_rate=arch_timer_get_sysreg_freq();+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]){
From: Fu Wei <redacted>
Currently, arch_timer_rate is used to store the frequency got from per-cpu
arch-timer or the memory-mapped (MMIO) timers. But those values come from
different registers which should all be initialized by firmware.
This patch remove arch_timer_rate, and use arch_timer_sysreg_freq and
arch_timer_mmio_freq instead.
Signed-off-by: Fu Wei <redacted>
---
drivers/clocksource/arm_arch_timer.c | 42 ++++++++++++++++++++----------------
1 file changed, 24 insertions(+), 18 deletions(-)
@@ -487,7 +491,7 @@ static void arch_timer_configure_evtstream(void)intevt_stream_div,pos;/* Find the closest power of two to the divisor */-evt_stream_div=arch_timer_rate/ARCH_TIMER_EVT_STREAM_FREQ;+evt_stream_div=arch_timer_sysreg_freq/ARCH_TIMER_EVT_STREAM_FREQ;pos=fls(evt_stream_div);if(pos>1&&!(evt_stream_div&(1<<(pos-2))))pos--;
@@ -578,8 +582,8 @@ static void arch_timer_banner(unsigned type)type==(ARCH_TIMER_TYPE_CP15|ARCH_TIMER_TYPE_MEM)?" and ":"",type&ARCH_TIMER_TYPE_MEM?"mmio":"",-(unsignedlong)arch_timer_rate/1000000,-(unsignedlong)(arch_timer_rate/10000)%100,+(unsignedlong)arch_timer_sysreg_freq/1000000,+(unsignedlong)(arch_timer_sysreg_freq/10000)%100,type&ARCH_TIMER_TYPE_CP15?(arch_timer_uses_ppi==ARCH_TIMER_VIRT_PPI)?"virt":"phys":"",
@@ -648,6 +652,7 @@ struct arch_timer_kvm_info *arch_timer_get_kvm_info(void)staticvoid__initarch_counter_register(unsignedtype){u64start_count;+u32freq;/* Register the CP15 based counter if we have one */if(type&ARCH_TIMER_TYPE_CP15){
@@ -669,19 +676,20 @@ static void __init arch_counter_register(unsigned type)#endif}else{arch_timer_read_counter=arch_counter_get_cntvct_mem;+freq=arch_timer_mmio_freq;}if(!arch_counter_suspend_stop)clocksource_counter.flags|=CLOCK_SOURCE_SUSPEND_NONSTOP;start_count=arch_timer_read_counter();-clocksource_register_hz(&clocksource_counter,arch_timer_rate);+clocksource_register_hz(&clocksource_counter,freq);cyclecounter.mult=clocksource_counter.mult;cyclecounter.shift=clocksource_counter.shift;timecounter_init(&arch_timer_kvm_info.timecounter,&cyclecounter,start_count);/* 56 bits minimum, so we assume worst case rollover */-sched_clock_register(arch_timer_read_counter,56,arch_timer_rate);+sched_clock_register(arch_timer_read_counter,56,freq);}staticvoidarch_timer_stop(structclock_event_device*clk)
@@ -949,10 +957,9 @@ static int __init arch_timer_of_init(struct device_node *np)*Trytodeterminethefrequencyfromthedevicetree,*iffail,getthefrequencyfromthesysregCNTFRQ.*/-if(!arch_timer_rate&&-of_property_read_u32(np,"clock-frequency",&arch_timer_rate))-arch_timer_rate=arch_timer_get_sysreg_freq();-if(!arch_timer_rate){+if(of_property_read_u32(np,"clock-frequency",&arch_timer_sysreg_freq))+arch_timer_sysreg_freq=arch_timer_get_sysreg_freq();+if(!arch_timer_sysreg_freq){pr_err(FW_BUG"frequency not available.\n");return-EINVAL;}
@@ -1070,10 +1077,9 @@ static int __init arch_timer_mem_init(struct device_node *np)*Trytodeterminethefrequencyfromthedevicetree,*iffail,getthefrequencyfromtheCNTFRQregofMMIOtimer.*/-if(!arch_timer_rate&&-of_property_read_u32(np,"clock-frequency",&arch_timer_rate))-arch_timer_rate=arch_timer_get_mmio_freq(base);-if(!arch_timer_rate){+if(of_property_read_u32(np,"clock-frequency",&arch_timer_mmio_freq))+arch_timer_mmio_freq=arch_timer_get_mmio_freq(base);+if(!arch_timer_mmio_freq){pr_err(FW_BUG"frequency not available for MMIO timer.\n");ret=-EINVAL;gotoout;
@@ -1140,8 +1146,8 @@ static int __init arch_timer_acpi_init(struct acpi_table_header *table)gtdt->non_secure_el2_flags);/* Get the frequency from the sysreg CNTFRQ */-arch_timer_rate=arch_timer_get_sysreg_freq();-if(!arch_timer_rate){+arch_timer_sysreg_freq=arch_timer_get_sysreg_freq();+if(!arch_timer_sysreg_freq){pr_err(FW_BUG"frequency not available.\n");return-EINVAL;}
From: Fu Wei <redacted>
Because arch_timer_needs_of_probing is only for booting with device-tree,
but arch_timer_common_init is a generic init call which shouldn't include
the FW-specific code. It's better to put arch_timer_needs_of_probing into
DT init function.
But for per-cpu timer, the arch_timer_common_init is called from
arch_timer_init. For reaching the goal above, this patch disassemble
arch_timer_init and use arch_timer_register and arch_timer_common_init
directly, just like arch_timer_mem init code is doing.
By this way, all the DT relevant code are only called from DT init call.
Signed-off-by: Fu Wei <redacted>
Reviewed-by: Hanjun Guo <redacted>
---
drivers/clocksource/arm_arch_timer.c | 46 ++++++++++++++++--------------------
1 file changed, 21 insertions(+), 25 deletions(-)
@@ -1149,6 +1140,8 @@ 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);+arch_timer_kvm_info.virtual_irq=arch_timer_ppi[ARCH_TIMER_VIRT_PPI];+/* Get the frequency from the sysreg CNTFRQ */arch_timer_sysreg_freq=arch_timer_get_sysreg_freq();if(!arch_timer_sysreg_freq){
From: Fu Wei <redacted>
When system init with device-tree, we don't know which node will be
initialized first. And the code in arch_timer_common_init should wait
until per-cpu timer and MMIO timer are both initialized. So we need
arch_timer_needs_probing to detect the init status of system.
But currently the code is dispersed in arch_timer_needs_probing and
arch_timer_common_init. And the function name doesn't specify that
it's only for device-tree. This is somewhat confusing.
This patch move all related code from arch_timer_common_init to
arch_timer_needs_probing, refactor it, and rename it to
arch_timer_needs_of_probing. And make sure that it will be called
only if acpi is disabled.
Signed-off-by: Fu Wei <redacted>
Reviewed-by: Hanjun Guo <redacted>
---
drivers/clocksource/arm_arch_timer.c | 34 +++++++++++++++++++---------------
1 file changed, 19 insertions(+), 15 deletions(-)
@@ -858,15 +858,28 @@ static const struct of_device_id arch_timer_mem_of_match[] __initconst = {{},};-staticbool__init-arch_timer_needs_probing(inttype,conststructof_device_id*matches)+staticbool__initarch_timer_needs_of_probing(void){structdevice_node*dn;boolneeds_probing=false;+unsignedintmask=ARCH_TIMER_TYPE_CP15|ARCH_TIMER_TYPE_MEM;-dn=of_find_matching_node(NULL,matches);-if(dn&&of_device_is_available(dn)&&!(arch_timers_present&type))+/* We have two timers, and both device-tree nodes are probed. */+if((arch_timers_present&mask)==mask)+returnfalse;++/*+*Onlyonetypeoftimerisprobed,+*checkifwehaveanothertypeoftimernodeindevice-tree.+*/+if(arch_timers_present&ARCH_TIMER_TYPE_CP15)+dn=of_find_matching_node(NULL,arch_timer_mem_of_match);+else+dn=of_find_matching_node(NULL,arch_timer_of_match);++if(dn&&of_device_is_available(dn))needs_probing=true;+of_node_put(dn);returnneeds_probing;
@@ -874,17 +887,8 @@ arch_timer_needs_probing(int type, const struct of_device_id *matches)staticint__initarch_timer_common_init(void){-unsignedmask=ARCH_TIMER_TYPE_CP15|ARCH_TIMER_TYPE_MEM;--/* Wait until both nodes are probed if we have two timers */-if((arch_timers_present&mask)!=mask){-if(arch_timer_needs_probing(ARCH_TIMER_TYPE_MEM,-arch_timer_mem_of_match))-return0;-if(arch_timer_needs_probing(ARCH_TIMER_TYPE_CP15,-arch_timer_of_match))-return0;-}+if(acpi_disabled&&arch_timer_needs_of_probing())+return0;arch_timer_banner(arch_timers_present);arch_counter_register(arch_timers_present);
From: Fu Wei <redacted>
The patch introduce two new structs: arch_timer_mem, arch_timer_mem_frame.
And also introduce a new define: ARCH_TIMER_MEM_MAX_FRAMES
These will be used for refactoring the memory-mapped timer init code to
prepare for GTDT
Signed-off-by: Fu Wei <redacted>
Reviewed-by: Hanjun Guo <redacted>
---
include/clocksource/arm_arch_timer.h | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
From: Fu Wei <redacted>
Currently the code to probe MMIO architected timers mixes DT parsing with
actual poking of hardware. This makes the code harder than necessary to
understand, and makes it difficult to add support for probing via ACPI.
This patch factors all the DT-specific logic out of arch_timer_mem_init(),
into a new function arch_timer_mem_of_init().
The former pokes the hardware and determines the suitablility of frames
based on a datastructure populated by the latter.
This cleanly separates the two and will make it possible to add probing
using the ACPI GTDT in subsequent patches.
Signed-off-by: Fu Wei <redacted>
Reviewed-by: Hanjun Guo <redacted>
---
drivers/clocksource/arm_arch_timer.c | 156 +++++++++++++++++++++++++----------
1 file changed, 111 insertions(+), 45 deletions(-)
@@ -1015,26 +1018,18 @@ static int __init arch_timer_mem_init(struct device_node *np)*Trytofindavirtualcapableframe.Otherwisefallbacktoa*physicalcapableframe.*/-for_each_available_child_of_node(np,frame){-intn;-u32cntacr;--if(of_property_read_u32(frame,"frame-number",&n)){-pr_err("Missing frame-number\n");-of_node_put(frame);-gotoout;-}+for(i=0;i<timer_mem->num_frames;i++){+u32cntacr=CNTACR_RFRQ|CNTACR_RWPT|CNTACR_RPCT|+CNTACR_RWVT|CNTACR_RVOFF|CNTACR_RVCT;+intn=timer_mem->frame[i].frame_nr;/* Try enabling everything, and see what sticks */-cntacr=CNTACR_RFRQ|CNTACR_RWPT|CNTACR_RPCT|-CNTACR_RWVT|CNTACR_RVOFF|CNTACR_RVCT;writel_relaxed(cntacr,cntctlbase+CNTACR(n));cntacr=readl_relaxed(cntctlbase+CNTACR(n));if((cnttidr&CNTTIDR_VIRT(n))&&!(~cntacr&(CNTACR_RWVT|CNTACR_RVCT))){-of_node_put(best_frame);-best_frame=frame;+best_frame=&timer_mem->frame[i];arch_timer_mem_use_virtual=true;break;}
@@ -1042,55 +1037,126 @@ static int __init arch_timer_mem_init(struct device_node *np)if(~cntacr&(CNTACR_RWPT|CNTACR_RPCT))continue;-of_node_put(best_frame);-best_frame=of_node_get(frame);+best_frame=&timer_mem->frame[i];}+iounmap(cntctlbase);-ret=-ENXIO;-base=arch_counter_base=of_io_request_and_map(best_frame,0,-"arch_mem_timer");-if(IS_ERR(base)){-pr_err("Can't map frame's registers\n");-gotoout;+if(!best_frame){+pr_err("Can't find frame for register\n");+return-EINVAL;}if(arch_timer_mem_use_virtual)-irq=irq_of_parse_and_map(best_frame,ARCH_TIMER_VIRT_SPI);+irq=best_frame->virt_irq;else-irq=irq_of_parse_and_map(best_frame,ARCH_TIMER_PHYS_SPI);+irq=best_frame->phys_irq;-ret=-EINVAL;if(!irq){pr_err("Frame missing %s irq.\n",arch_timer_mem_use_virtual?"virt":"phys");-gotoout;+return-EINVAL;}-/*-*Trytodeterminethefrequencyfromthedevicetree,-*iffail,getthefrequencyfromtheCNTFRQregofMMIOtimer.-*/-if(of_property_read_u32(np,"clock-frequency",&arch_timer_mmio_freq))+if(!request_mem_region(best_frame->cntbase,best_frame->size,+"arch_mem_timer"))+return-EBUSY;++base=ioremap(best_frame->cntbase,best_frame->size);+if(!base){+pr_err("Can't map frame's registers\n");+return-ENXIO;+}++if(!arch_timer_mmio_freq)arch_timer_mmio_freq=arch_timer_get_mmio_freq(base);-if(!arch_timer_mmio_freq){-pr_err(FW_BUG"frequency not available for MMIO timer.\n");-ret=-EINVAL;-gotoout;+if(!arch_timer_mmio_freq||+(arch_timer_sysreg_freq&&+arch_timer_mmio_freq!=arch_timer_sysreg_freq)){+pr_err(FW_BUG"invalid frequency for MMIO timer.\n");+arch_timer_mmio_freq=0;+iounmap(base);+return-EINVAL;}ret=arch_timer_mem_register(base,irq);-if(ret)+if(ret){+iounmap(base);+returnret;+}++arch_counter_base=base;+arch_timers_present|=ARCH_TIMER_TYPE_MEM;++return0;+}++staticint__initarch_timer_mem_of_init(structdevice_node*np)+{+structarch_timer_mem*timer_mem;+structdevice_node*frame_node;+structresourceres;+inti,ret=-EINVAL;++timer_mem=kzalloc(sizeof(*timer_mem),GFP_KERNEL);+if(!timer_mem)+return-ENOMEM;++if(of_address_to_resource(np,0,&res))gotoout;+timer_mem->cntctlbase=res.start;+timer_mem->size=resource_size(&res);++i=0;+for_each_available_child_of_node(np,frame_node){+intn;+structarch_timer_mem_frame*frame;++if(i>=ARCH_TIMER_MEM_MAX_FRAMES){+pr_err(FW_BUG"too many frames, only %u are permitted.\n",+ARCH_TIMER_MEM_MAX_FRAMES);+gotoout;+}-if(!arch_timer_needs_of_probing())+frame=&timer_mem->frame[i];++if(of_property_read_u32(frame_node,"frame-number",&n)){+pr_err(FW_BUG"Missing frame-number\n");+of_node_put(frame_node);+gotoout;+}+frame->frame_nr=n;++if(of_address_to_resource(frame_node,0,&res)){+of_node_put(frame_node);+gotoout;+}+frame->cntbase=res.start;+frame->size=resource_size(&res);++frame->virt_irq=irq_of_parse_and_map(frame_node,+ARCH_TIMER_VIRT_SPI);+frame->phys_irq=irq_of_parse_and_map(frame_node,+ARCH_TIMER_PHYS_SPI);++i++;+}+timer_mem->num_frames=i;++/*+*Trytogetthefrequencyfromthedevicetree,+*iffail,wewilltrytheCNTFRQregisterinarch_timer_mem_init.+*/+of_property_read_u32(np,"clock-frequency",&arch_timer_mmio_freq);++ret=arch_timer_mem_init(timer_mem);+if(!ret&&!arch_timer_needs_of_probing())ret=arch_timer_common_init();out:-iounmap(cntctlbase);-of_node_put(best_frame);+kfree(timer_mem);returnret;}CLOCKSOURCE_OF_DECLARE(armv7_arch_timer_mem,"arm,armv7-timer-mem",-arch_timer_mem_init);+arch_timer_mem_of_init);#ifdef CONFIG_ACPIstaticint__initmap_generic_timer_interrupt(u32interrupt,u32flags)
From: Fu Wei <redacted>
This patch adds support for parsing arch timer info in GTDT,
provides some kernel APIs to parse all the PPIs and
always-on info in GTDT and export them.
By this driver, we can simplify arm_arch_timer drivers, and
separate the ACPI GTDT knowledge from it.
Signed-off-by: Fu Wei <redacted>
Signed-off-by: Hanjun Guo <redacted>
Acked-by: Rafael J. Wysocki <redacted>
Tested-by: Xiongfeng Wang <redacted>
Reviewed-by: Hanjun Guo <redacted>
Tested-by: Hanjun Guo <redacted>
---
arch/arm64/Kconfig | 1 +
drivers/acpi/arm64/Kconfig | 3 +
drivers/acpi/arm64/Makefile | 1 +
drivers/acpi/arm64/gtdt.c | 157 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/acpi.h | 6 ++
5 files changed, 168 insertions(+)
From: Fu Wei <redacted>
The patch update arm_arch_timer driver to use the function
provided by the new GTDT driver of ACPI.
By this way, arm_arch_timer.c can be simplified, and separate
all the ACPI GTDT knowledge from this timer driver.
Signed-off-by: Fu Wei <redacted>
Signed-off-by: Hanjun Guo <redacted>
Tested-by: Xiongfeng Wang <redacted>
Reviewed-by: Hanjun Guo <redacted>
Tested-by: Hanjun Guo <redacted>
---
drivers/clocksource/arm_arch_timer.c | 54 ++++++++++++------------------------
1 file changed, 17 insertions(+), 37 deletions(-)
@@ -1158,63 +1158,36 @@ static int __init arch_timer_mem_of_init(struct device_node *np)CLOCKSOURCE_OF_DECLARE(armv7_arch_timer_mem,"arm,armv7-timer-mem",arch_timer_mem_of_init);-#ifdef CONFIG_ACPI-staticint__initmap_generic_timer_interrupt(u32interrupt,u32flags)-{-inttrigger,polarity;--if(!interrupt)-return0;--trigger=(flags&ACPI_GTDT_INTERRUPT_MODE)?ACPI_EDGE_SENSITIVE-:ACPI_LEVEL_SENSITIVE;--polarity=(flags&ACPI_GTDT_INTERRUPT_POLARITY)?ACPI_ACTIVE_LOW-:ACPI_ACTIVE_HIGH;--returnacpi_register_gsi(NULL,interrupt,trigger,polarity);-}-+#ifdef CONFIG_ACPI_GTDT/* Initialize per-processor generic timer */staticint__initarch_timer_acpi_init(structacpi_table_header*table){intret;-structacpi_table_gtdt*gtdt;if(arch_timers_present&ARCH_TIMER_TYPE_CP15){pr_warn("already initialized, skipping\n");return-EINVAL;}-gtdt=container_of(table,structacpi_table_gtdt,header);-arch_timers_present|=ARCH_TIMER_TYPE_CP15;-arch_timer_ppi[ARCH_TIMER_PHYS_SECURE_PPI]=-map_generic_timer_interrupt(gtdt->secure_el1_interrupt,-gtdt->secure_el1_flags);+ret=acpi_gtdt_init(table,NULL);+if(ret){+pr_err("Failed to init GTDT table.\n");+returnret;+}arch_timer_ppi[ARCH_TIMER_PHYS_NONSECURE_PPI]=-map_generic_timer_interrupt(gtdt->non_secure_el1_interrupt,-gtdt->non_secure_el1_flags);+acpi_gtdt_map_ppi(ARCH_TIMER_PHYS_NONSECURE_PPI);arch_timer_ppi[ARCH_TIMER_VIRT_PPI]=-map_generic_timer_interrupt(gtdt->virtual_timer_interrupt,-gtdt->virtual_timer_flags);+acpi_gtdt_map_ppi(ARCH_TIMER_VIRT_PPI);arch_timer_ppi[ARCH_TIMER_HYP_PPI]=-map_generic_timer_interrupt(gtdt->non_secure_el2_interrupt,-gtdt->non_secure_el2_flags);+acpi_gtdt_map_ppi(ARCH_TIMER_HYP_PPI);arch_timer_kvm_info.virtual_irq=arch_timer_ppi[ARCH_TIMER_VIRT_PPI];-/* Get the frequency from the sysreg CNTFRQ */-arch_timer_sysreg_freq=arch_timer_get_sysreg_freq();-if(!arch_timer_sysreg_freq){-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]){pr_err("No interrupt available, giving up\n");
@@ -1222,7 +1195,14 @@ static int __init arch_timer_acpi_init(struct acpi_table_header *table)}/* Always-on capability */-arch_timer_c3stop=!(gtdt->non_secure_el1_flags&ACPI_GTDT_ALWAYS_ON);+arch_timer_c3stop=acpi_gtdt_c3stop(arch_timer_uses_ppi);++/* Get the frequency from the sysreg CNTFRQ */+arch_timer_sysreg_freq=arch_timer_get_sysreg_freq();+if(!arch_timer_sysreg_freq){+pr_err(FW_BUG"frequency not available.\n");+return-EINVAL;+}ret=arch_timer_register();if(ret)
From: Fu Wei <redacted>
On platforms booting with ACPI, architected memory-mapped timers'
configuration data is provided by firmware through the ACPI GTDT
static table.
The clocksource architected timer kernel driver requires a firmware
interface to collect timer configuration and configure its driver.
this infrastructure is present for device tree systems, but it is
missing on systems booting with ACPI.
Implement the kernel infrastructure required to parse the static
ACPI GTDT table so that the architected timer clocksource driver can
make use of it on systems booting with ACPI, therefore enabling
the corresponding timers configuration.
Signed-off-by: Fu Wei <redacted>
Signed-off-by: Hanjun Guo <redacted>
---
drivers/acpi/arm64/gtdt.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/acpi.h | 1 +
2 files changed, 129 insertions(+)
From: Fu Wei <redacted>
This driver adds support for parsing SBSA Generic Watchdog timer
in GTDT, parse all info in SBSA Generic Watchdog Structure in GTDT,
and creating a platform device with that information.
This allows the operating system to obtain device data from the
resource of platform device. The platform device named "sbsa-gwdt"
can be used by the ARM SBSA Generic Watchdog driver.
Signed-off-by: Fu Wei <redacted>
Signed-off-by: Hanjun Guo <redacted>
Tested-by: Xiongfeng Wang <redacted>
---
drivers/acpi/arm64/gtdt.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++
drivers/watchdog/Kconfig | 1 +
2 files changed, 94 insertions(+)
@@ -283,3 +291,88 @@ int __init acpi_arch_timer_mem_init(struct arch_timer_mem *data,return0;}++/*+*InitializeaSBSAgenericWatchdogplatformdeviceinfofromGTDT+*/+staticint__initgtdt_import_sbsa_gwdt(structacpi_gtdt_watchdog*wd,+intindex)+{+structplatform_device*pdev;+intirq=map_gt_gsi(wd->timer_interrupt,wd->timer_flags);+intno_irq=1;++/*+*AccordingtoSBSAspecificationthesizeofrefreshandcontrol+*framesofSBSAGenericWatchdogisSZ_4K(Offset0x000?0xFFF).+*/+structresourceres[]={+DEFINE_RES_MEM(wd->control_frame_address,SZ_4K),+DEFINE_RES_MEM(wd->refresh_frame_address,SZ_4K),+DEFINE_RES_IRQ(irq),+};++pr_debug("found a Watchdog (0x%llx/0x%llx gsi:%u flags:0x%x).\n",+wd->refresh_frame_address,wd->control_frame_address,+wd->timer_interrupt,wd->timer_flags);++if(!(wd->refresh_frame_address&&wd->control_frame_address)){+pr_err(FW_BUG"failed to get the Watchdog base address.\n");+return-EINVAL;+}++if(!wd->timer_interrupt)+pr_warn(FW_BUG"failed to get the Watchdog interrupt.\n");+elseif(irq<=0)+pr_warn("failed to map the Watchdog interrupt.\n");+else+no_irq=0;++/*+*Addaplatformdevicenamed"sbsa-gwdt"tomatchtheplatformdriver.+*"sbsa-gwdt":SBSA(ServerBaseSystemArchitecture)GenericWatchdog+*Theplatformdriver(likedrivers/watchdog/sbsa_gwdt.c)cangetdevice+*infobelowbymatchingthisname.+*/+pdev=platform_device_register_simple("sbsa-gwdt",index,res,+ARRAY_SIZE(res)-no_irq);+if(IS_ERR(pdev)){+acpi_unregister_gsi(wd->timer_interrupt);+returnPTR_ERR(pdev);+}++return0;+}++staticint__initgtdt_sbsa_gwdt_init(void)+{+intret,i=0;+void*platform_timer;+structacpi_table_header*table;++if(acpi_disabled)+return0;++if(ACPI_FAILURE(acpi_get_table(ACPI_SIG_GTDT,0,&table)))+return-EINVAL;++ret=acpi_gtdt_init(table,NULL);+if(ret)+returnret;++for_each_platform_timer(platform_timer){+if(is_watchdog(platform_timer)){+ret=gtdt_import_sbsa_gwdt(platform_timer,i);+if(ret)+break;+i++;+}+}++if(i)+pr_info("found %d SBSA generic Watchdog(s).\n",i);++returnret;+}++device_initcall(gtdt_sbsa_gwdt_init);
From: Fu Wei <redacted>
The patch add memory-mapped timer register support by using the
information provided by the new GTDT driver of ACPI.
Signed-off-by: Fu Wei <redacted>
Reviewed-by: Hanjun Guo <redacted>
---
drivers/clocksource/arm_arch_timer.c | 35 ++++++++++++++++++++++++++++++++---
1 file changed, 32 insertions(+), 3 deletions(-)
Hi Mark, Marc
On 7 February 2017 at 02:50, [off-list ref] wrote:
From: Fu Wei <redacted>
This patchset:
(1)Preparation for adding GTDT support in arm_arch_timer:
1. Introduce two functions to get the frequency from mmio and sysreg.
2. separate out device-tree code from arch_timer_detect_rate
3. remove arch_timer_detect_rate use arch_timer_get_*_freq directly
4. split arch_timer_rate for different types of timer
5. Refactor arch_timer_needs_probing, and move it into DT init call
6. Introduce some new structs and refactor the MMIO timer init code
for reusing some common code.
(2)Introduce ACPI GTDT parser: drivers/acpi/arm64/acpi_gtdt.c
Parse all kinds of timer in GTDT table of ACPI:arch timer,
memory-mapped timer and SBSA Generic Watchdog timer.
This driver can help to simplify all the relevant timer drivers,
and separate all the ACPI GTDT knowledge from them.
(3)Simplify ACPI code for arm_arch_timer
(4)Add GTDT support for ARM memory-mapped timer.
This patchset has been tested on the following platforms with ACPI enabled:
(1)ARM Foundation v8 model
Changelog:
v21: https://lkml.org/lkml/2017/2/6/
Introduce two functions to get the frequency from mmio and sysreg.
Remove arch_timer_detect_rate use arch_timer_get_*_freq directly
Split arch_timer_rate for different types of timer.
Skip secure timer frame in GTDT driver.
Rebase to git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git arch-timer/cleanup
(The first 6 patches in v20 have been merged into arch-timer/cleanup branch)
Did I follow your suggestion correctly?
I wonder how to improve this patchset. May I have your some suggestion
or feedback on v21? :-)
Great thanks for your help!
v20: https://lkml.org/lkml/2017/1/18/534
Reorder the first 4 patches and split the 4th patches.
Leave CNTHCTL_* as they originally were.
Fix the bug in arch_timer_select_ppi.
Split "Rework counter frequency detection" patch.
Rework the arch_timer_detect_rate function.
Improve the commit message of "Refactor MMIO timer probing".
Rebase to 4.10.0-rc4
v19: https://lkml.org/lkml/2016/12/21/25
Fix a '\n' missing in a error message in arch_timer_mem_init.
Add "request_mem_region" for ioremapping cntbase, according to
f947ee1 clocksource/drivers/arm_arch_timer: Map frame with of_io_request_and_map()
Rebase to 4.9.0-gfb779ff
v18: https://lkml.org/lkml/2016/12/8/446
Fix 8/15 patch problem of "int ret;" in arch_timer_acpi_init.
Rebase to 4.9.0-rc8-g9269898
v17: https://lkml.org/lkml/2016/11/25/140
Take out some cleanups from 4/15.
Merge 5/15 and 6/15, improve PPI determination code,
improve commit message.
Rework counter frequency detection.
Move arch_timer_needs_of_probing into DT init call.
Move Platform Timer scan loop back to timer init call to avoid allocating
and free memory.
Improve all the exported functions' comment.
v16: https://lkml.org/lkml/2016/11/16/268
Fix patchset problem about static enum ppi_nr of 01/13 in v15.
Refactor arch_timer_detect_rate.
Refactor arch_timer_needs_probing.
v15: https://lkml.org/lkml/2016/11/15/366
Re-order patches
Add arm_arch_timer refactoring patches to prepare for GTDT:
1. rename some enums and defines, and some cleanups
2. separate out arch_timer_uses_ppi init code and fix a potential bug
3. Improve some new structs, refactor the timer init code.
Since the some structs have been changed, GTDT parser for memory-mapped
timer and SBSA Generic Watchdog timer have been update.
v14: https://lkml.org/lkml/2016/9/28/573
Separate memory-mapped timer GTDT support into two patches
1. Refactor the timer init code to prepare for GTDT
2. Add GTDT support for memory-mapped timer
v13: http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1231717.html
Improve arm_arch_timer code for memory-mapped
timer GTDT support, refactor original memory-mapped timer
dt support for reusing some common code.
v12: https://lkml.org/lkml/2016/9/13/250
Rebase to latest Linux 4.8-rc6
Delete the confusing "skipping" in the error message.
V11: https://lkml.org/lkml/2016/9/6/354
Rebase to latest Linux 4.8-rc5
Delete typedef (suggested by checkpatch.pl)
V10: https://lkml.org/lkml/2016/7/26/215
Drop the "readq" patch.
Rebase to latest Linux 4.7.
V9: https://lkml.org/lkml/2016/7/25/345
Improve pr_err message in acpi gtdt driver.
Update Commit message for 7/9
shorten the irq mapping function name
Improve GTDT driver for memory-mapped timer
v8: https://lkml.org/lkml/2016/7/19/660
Improve "pr_fmt(fmt)" definition: add "ACPI" in front of "GTDT",
and also improve printk message.
Simplify is_timer_block and is_watchdog.
Merge acpi_gtdt_desc_init and gtdt_arch_timer_init into acpi_gtdt_init();
Delete __init in include/linux/acpi.h for GTDT API
Make ARM64 select GTDT.
Delete "#include <linux/module.h>" from acpi_gtdt.c
Simplify GT block parse code.
v7: https://lkml.org/lkml/2016/7/13/769
Move the GTDT driver to drivers/acpi/arm64
Add add the ARM64-specific ACPI Support maintainers in MAINTAINERS
Merge 3 patches of GTDT parser driver.
Fix the for_each_platform_timer bug.
v6: https://lkml.org/lkml/2016/6/29/580
split the GTDT driver to 4 parts: basic, arch_timer, memory-mapped timer,
and SBSA Generic Watchdog timer
Improve driver by suggestions and example code from Daniel Lezcano
v5: https://lkml.org/lkml/2016/5/24/356
Sorting out all patches, simplify the API of GTDT driver:
GTDT driver just fills the data struct for arm_arch_timer driver.
v4: https://lists.linaro.org/pipermail/linaro-acpi/2016-March/006667.html
Delete the kvm relevant patches
Separate two patches for sorting out the code for arm_arch_timer.
Improve irq info export code to allow missing irq info in GTDT table.
v3: https://lkml.org/lkml/2016/2/1/658
Improve GTDT driver code:
(1)improve pr_* by defining pr_fmt(fmt)
(2)simplify gtdt_sbsa_gwdt_init
(3)improve gtdt_arch_timer_data_init, if table is NULL, it will try
to get GTDT table.
Move enum ppi_nr to arm_arch_timer.h, and add enum spi_nr.
Add arm_arch_timer get ppi from DT and GTDT support for kvm.
v2: https://lkml.org/lkml/2015/12/2/10
Rebase to latest kernel version(4.4-rc3).
Fix the bug about the config problem,
use CONFIG_ACPI_GTDT instead of CONFIG_ACPI in arm_arch_timer.c
v1: The first upstreaming version: https://lkml.org/lkml/2015/10/28/553
Fu Wei (13):
clocksource: arm_arch_timer: introduce two functions to get the
frequency from mmio and sysreg.
clocksource: arm_arch_timer: separate out device-tree code from
arch_timer_detect_rate
clocksource: arm_arch_timer: remove arch_timer_detect_rate
clocksource: arm_arch_timer: split arch_timer_rate for different types
of timer
clocksource: arm_arch_timer: refactor arch_timer_needs_probing
clocksource: arm_arch_timer: move arch_timer_needs_of_probing into DT
init call
clocksource: arm_arch_timer: introduce some new structs to prepare for
GTDT
clocksource: arm_arch_timer: refactor MMIO timer probing.
acpi/arm64: Add GTDT table parse driver
clocksource: arm_arch_timer: simplify ACPI support code.
acpi/arm64: Add memory-mapped timer support in GTDT driver
clocksource: arm_arch_timer: add GTDT support for memory-mapped timer
acpi/arm64: Add SBSA Generic Watchdog support in GTDT driver
arch/arm64/Kconfig | 1 +
drivers/acpi/arm64/Kconfig | 3 +
drivers/acpi/arm64/Makefile | 1 +
drivers/acpi/arm64/gtdt.c | 378 +++++++++++++++++++++++++++++++++++
drivers/clocksource/arm_arch_timer.c | 354 ++++++++++++++++++++------------
drivers/watchdog/Kconfig | 1 +
include/clocksource/arm_arch_timer.h | 17 ++
include/linux/acpi.h | 7 +
8 files changed, 635 insertions(+), 127 deletions(-)
create mode 100644 drivers/acpi/arm64/gtdt.c
--
2.9.3
Hi Mark, Marc,
I have tried to rebase all the 19(6+13) patches on 4.11-rc1,
all the patchse can directly apply on 4.11-rc1,
Could you help to review the patches, and see if there is anywhere I
can improve ?
Great thanks ! :-)
On 20 February 2017 at 17:20, Fu Wei [off-list ref] wrote:
Hi Mark, Marc
On 7 February 2017 at 02:50, [off-list ref] wrote:
quoted
From: Fu Wei <redacted>
This patchset:
(1)Preparation for adding GTDT support in arm_arch_timer:
1. Introduce two functions to get the frequency from mmio and sysreg.
2. separate out device-tree code from arch_timer_detect_rate
3. remove arch_timer_detect_rate use arch_timer_get_*_freq directly
4. split arch_timer_rate for different types of timer
5. Refactor arch_timer_needs_probing, and move it into DT init call
6. Introduce some new structs and refactor the MMIO timer init code
for reusing some common code.
(2)Introduce ACPI GTDT parser: drivers/acpi/arm64/acpi_gtdt.c
Parse all kinds of timer in GTDT table of ACPI:arch timer,
memory-mapped timer and SBSA Generic Watchdog timer.
This driver can help to simplify all the relevant timer drivers,
and separate all the ACPI GTDT knowledge from them.
(3)Simplify ACPI code for arm_arch_timer
(4)Add GTDT support for ARM memory-mapped timer.
This patchset has been tested on the following platforms with ACPI enabled:
(1)ARM Foundation v8 model
Changelog:
v21: https://lkml.org/lkml/2017/2/6/
Introduce two functions to get the frequency from mmio and sysreg.
Remove arch_timer_detect_rate use arch_timer_get_*_freq directly
Split arch_timer_rate for different types of timer.
Skip secure timer frame in GTDT driver.
Rebase to git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git arch-timer/cleanup
(The first 6 patches in v20 have been merged into arch-timer/cleanup branch)
Did I follow your suggestion correctly?
I wonder how to improve this patchset. May I have your some suggestion
or feedback on v21? :-)
Great thanks for your help!
quoted
v20: https://lkml.org/lkml/2017/1/18/534
Reorder the first 4 patches and split the 4th patches.
Leave CNTHCTL_* as they originally were.
Fix the bug in arch_timer_select_ppi.
Split "Rework counter frequency detection" patch.
Rework the arch_timer_detect_rate function.
Improve the commit message of "Refactor MMIO timer probing".
Rebase to 4.10.0-rc4
v19: https://lkml.org/lkml/2016/12/21/25
Fix a '\n' missing in a error message in arch_timer_mem_init.
Add "request_mem_region" for ioremapping cntbase, according to
f947ee1 clocksource/drivers/arm_arch_timer: Map frame with of_io_request_and_map()
Rebase to 4.9.0-gfb779ff
v18: https://lkml.org/lkml/2016/12/8/446
Fix 8/15 patch problem of "int ret;" in arch_timer_acpi_init.
Rebase to 4.9.0-rc8-g9269898
v17: https://lkml.org/lkml/2016/11/25/140
Take out some cleanups from 4/15.
Merge 5/15 and 6/15, improve PPI determination code,
improve commit message.
Rework counter frequency detection.
Move arch_timer_needs_of_probing into DT init call.
Move Platform Timer scan loop back to timer init call to avoid allocating
and free memory.
Improve all the exported functions' comment.
v16: https://lkml.org/lkml/2016/11/16/268
Fix patchset problem about static enum ppi_nr of 01/13 in v15.
Refactor arch_timer_detect_rate.
Refactor arch_timer_needs_probing.
v15: https://lkml.org/lkml/2016/11/15/366
Re-order patches
Add arm_arch_timer refactoring patches to prepare for GTDT:
1. rename some enums and defines, and some cleanups
2. separate out arch_timer_uses_ppi init code and fix a potential bug
3. Improve some new structs, refactor the timer init code.
Since the some structs have been changed, GTDT parser for memory-mapped
timer and SBSA Generic Watchdog timer have been update.
v14: https://lkml.org/lkml/2016/9/28/573
Separate memory-mapped timer GTDT support into two patches
1. Refactor the timer init code to prepare for GTDT
2. Add GTDT support for memory-mapped timer
v13: http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1231717.html
Improve arm_arch_timer code for memory-mapped
timer GTDT support, refactor original memory-mapped timer
dt support for reusing some common code.
v12: https://lkml.org/lkml/2016/9/13/250
Rebase to latest Linux 4.8-rc6
Delete the confusing "skipping" in the error message.
V11: https://lkml.org/lkml/2016/9/6/354
Rebase to latest Linux 4.8-rc5
Delete typedef (suggested by checkpatch.pl)
V10: https://lkml.org/lkml/2016/7/26/215
Drop the "readq" patch.
Rebase to latest Linux 4.7.
V9: https://lkml.org/lkml/2016/7/25/345
Improve pr_err message in acpi gtdt driver.
Update Commit message for 7/9
shorten the irq mapping function name
Improve GTDT driver for memory-mapped timer
v8: https://lkml.org/lkml/2016/7/19/660
Improve "pr_fmt(fmt)" definition: add "ACPI" in front of "GTDT",
and also improve printk message.
Simplify is_timer_block and is_watchdog.
Merge acpi_gtdt_desc_init and gtdt_arch_timer_init into acpi_gtdt_init();
Delete __init in include/linux/acpi.h for GTDT API
Make ARM64 select GTDT.
Delete "#include <linux/module.h>" from acpi_gtdt.c
Simplify GT block parse code.
v7: https://lkml.org/lkml/2016/7/13/769
Move the GTDT driver to drivers/acpi/arm64
Add add the ARM64-specific ACPI Support maintainers in MAINTAINERS
Merge 3 patches of GTDT parser driver.
Fix the for_each_platform_timer bug.
v6: https://lkml.org/lkml/2016/6/29/580
split the GTDT driver to 4 parts: basic, arch_timer, memory-mapped timer,
and SBSA Generic Watchdog timer
Improve driver by suggestions and example code from Daniel Lezcano
v5: https://lkml.org/lkml/2016/5/24/356
Sorting out all patches, simplify the API of GTDT driver:
GTDT driver just fills the data struct for arm_arch_timer driver.
v4: https://lists.linaro.org/pipermail/linaro-acpi/2016-March/006667.html
Delete the kvm relevant patches
Separate two patches for sorting out the code for arm_arch_timer.
Improve irq info export code to allow missing irq info in GTDT table.
v3: https://lkml.org/lkml/2016/2/1/658
Improve GTDT driver code:
(1)improve pr_* by defining pr_fmt(fmt)
(2)simplify gtdt_sbsa_gwdt_init
(3)improve gtdt_arch_timer_data_init, if table is NULL, it will try
to get GTDT table.
Move enum ppi_nr to arm_arch_timer.h, and add enum spi_nr.
Add arm_arch_timer get ppi from DT and GTDT support for kvm.
v2: https://lkml.org/lkml/2015/12/2/10
Rebase to latest kernel version(4.4-rc3).
Fix the bug about the config problem,
use CONFIG_ACPI_GTDT instead of CONFIG_ACPI in arm_arch_timer.c
v1: The first upstreaming version: https://lkml.org/lkml/2015/10/28/553
Fu Wei (13):
clocksource: arm_arch_timer: introduce two functions to get the
frequency from mmio and sysreg.
clocksource: arm_arch_timer: separate out device-tree code from
arch_timer_detect_rate
clocksource: arm_arch_timer: remove arch_timer_detect_rate
clocksource: arm_arch_timer: split arch_timer_rate for different types
of timer
clocksource: arm_arch_timer: refactor arch_timer_needs_probing
clocksource: arm_arch_timer: move arch_timer_needs_of_probing into DT
init call
clocksource: arm_arch_timer: introduce some new structs to prepare for
GTDT
clocksource: arm_arch_timer: refactor MMIO timer probing.
acpi/arm64: Add GTDT table parse driver
clocksource: arm_arch_timer: simplify ACPI support code.
acpi/arm64: Add memory-mapped timer support in GTDT driver
clocksource: arm_arch_timer: add GTDT support for memory-mapped timer
acpi/arm64: Add SBSA Generic Watchdog support in GTDT driver
arch/arm64/Kconfig | 1 +
drivers/acpi/arm64/Kconfig | 3 +
drivers/acpi/arm64/Makefile | 1 +
drivers/acpi/arm64/gtdt.c | 378 +++++++++++++++++++++++++++++++++++
drivers/clocksource/arm_arch_timer.c | 354 ++++++++++++++++++++------------
drivers/watchdog/Kconfig | 1 +
include/clocksource/arm_arch_timer.h | 17 ++
include/linux/acpi.h | 7 +
8 files changed, 635 insertions(+), 127 deletions(-)
create mode 100644 drivers/acpi/arm64/gtdt.c
--
2.9.3
From: Mark Rutland <mark.rutland@arm.com> Date: 2017-03-17 18:06:08
On Tue, Feb 07, 2017 at 02:50:03AM +0800, fu.wei at linaro.org wrote:
+static u32 arch_timer_get_sysreg_freq(void)
+{
+ /*
+ * Try to get the frequency from the CNTFRQ of sysreg.
+ */
+ return arch_timer_get_cntfrq();
+}
We already have arch_timer_get_cntfrq(), so I don't see the point in
this wrapper.
+static u32 arch_timer_get_mmio_freq(void __iomem *cntbase)
+{
+ /*
+ * Try to get the frequency from the CNTFRQ of timer frame registers.
+ * Note: please verify cntbase in caller.
+ */
+ return readl_relaxed(cntbase + CNTFRQ);
+}
Wrapping the MMIO read makes sense if we're going to do this in more
than one place, so I'm happy with this wrapper.
If you can s/arch_timer_get_mmio_freq/arch_timer_get_cntfrq/, and drop
the comments, then this looks fine to me.
Thanks,
Mark.
From: Mark Rutland <mark.rutland@arm.com> Date: 2017-03-17 19:05:33
On Tue, Feb 07, 2017 at 02:50:05AM +0800, fu.wei at linaro.org wrote:
From: Fu Wei <redacted>
The original counter frequency detection call(arch_timer_detect_rate)
include getting the frequency from the per-cpu arch-timer and the
memory-mapped (MMIO) timer interfaces. But they will be needed only when
the system initializes the relevant timer.
This patch remove arch_timer_detect_rate founction, and use the
arch_timer_get_sysreg_freq and arch_timer_get_mmio_freq directly.
Signed-off-by: Fu Wei <redacted>
Could you please fold this with the prior patch?
quoted hunk
@@ -1087,7 +1072,12 @@ static int __init arch_timer_mem_init(struct device_node *np) */ if (!arch_timer_rate && of_property_read_u32(np, "clock-frequency", &arch_timer_rate))- arch_timer_detect_rate(base);+ arch_timer_rate = arch_timer_get_mmio_freq(base);+ if (!arch_timer_rate) {+ pr_err(FW_BUG "frequency not available for MMIO timer.\n");
It would be better to say "MMIO frequency not available.\n" here.
Thanks,
Mark.
From: Mark Rutland <mark.rutland@arm.com> Date: 2017-03-17 19:15:50
On Tue, Feb 07, 2017 at 02:50:06AM +0800, fu.wei at linaro.org wrote:
From: Fu Wei <redacted>
Currently, arch_timer_rate is used to store the frequency got from per-cpu
arch-timer or the memory-mapped (MMIO) timers. But those values come from
different registers which should all be initialized by firmware.
This patch remove arch_timer_rate, and use arch_timer_sysreg_freq and
arch_timer_mmio_freq instead.
Signed-off-by: Fu Wei <redacted>
Thanks for attacking this. Generally, I do think this is the right thing
to do.
However...
quoted hunk
@@ -1070,10 +1077,9 @@ static int __init arch_timer_mem_init(struct device_node *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_get_mmio_freq(base);- if (!arch_timer_rate) {+ if (of_property_read_u32(np, "clock-frequency", &arch_timer_mmio_freq))+ arch_timer_mmio_freq = arch_timer_get_mmio_freq(base);+ if (!arch_timer_mmio_freq) { pr_err(FW_BUG "frequency not available for MMIO timer.\n"); ret = -EINVAL; goto out;
... unfortunately, I believe that this will break some DT platforms that
have been (unintentionally) relying on the way currently allow the
frequency to be probed from either the MMIO timer or the sysreg timer.
So while the above was my suggestion, it was not my best.
For the timebeing, let's leave the single arch_timer_rate, but ensure
that it doesn't get in the way fo the ACPI code, by making the ACPI
probe path:
* Probe the sysreg timers first, using the sysreg cntfrq().
* Probe the MMIO timers second, verifying that each MMIO cntfrq matches
the already-probed sysreg cntfrq.
... which is what I believe you suggested previously.
Thanks,
Mark.
From: Mark Rutland <mark.rutland@arm.com> Date: 2017-03-17 19:41:18
Hi,
On Tue, Feb 07, 2017 at 02:50:13AM +0800, fu.wei at linaro.org wrote:
+static int __init gtdt_parse_timer_block(struct acpi_gtdt_timer_block *block,
+ struct arch_timer_mem *data)
Please s/data/timer_mem/ here, to match the rest of the timer code.
+{
+ int i, j;
+ struct acpi_gtdt_timer_entry *frame;
So as to make it clear what this is, and to make things a litlte simpler
below, please s/frame/gtdt_frame/ here.
+
+ if (!block->timer_count) {
+ pr_err(FW_BUG "GT block present, but frame count is zero.");
+ return -ENODEV;
+ }
+
+ if (block->timer_count > ARCH_TIMER_MEM_MAX_FRAMES) {
+ pr_err(FW_BUG "GT block lists %d frames, ACPI spec only allows 8\n",
+ block->timer_count);
+ return -EINVAL;
+ }
+
+ data->cntctlbase = (phys_addr_t)block->block_address;
+ /*
+ * According to "Table * CNTCTLBase memory map" of
+ * <ARM Architecture Reference Manual> for ARMv8,
+ * The size of the CNTCTLBase frame is 4KB(Offset 0x000 ? 0xFFC).
+ */
As a general thing, please cite the version of the ARM ARM you're
referring to, as over time the internal numbering (and the headings)
change.
e.g.
/*
* The CNTCTLBase frame is 4KB (register offsets 0x000 - 0xFFC).
* See ARM DDI 0487A.k_iss10775, page I1-5129, Table I1-3
* "CNTCTLBase memory map".
*/
+ data->size = SZ_4K;
+
+ frame = (void *)block + block->timer_offset;
+ if (frame + block->timer_count != (void *)block + block->header.length)
+ return -EINVAL;
+
+ /*
+ * Get the GT timer Frame data for every GT Block Timer
+ */
+ for (i = 0, j = 0; i < block->timer_count; i++, frame++) {
With the gtdt_frame rename as above, here we can do:
struct arch_timer_mem_frame *frame = &timer_mem->frame[j];
From: Mark Rutland <mark.rutland@arm.com> Date: 2017-03-17 20:04:30
On Thu, Mar 09, 2017 at 11:47:16PM +0100, Fu Wei wrote:
Hi Mark, Marc,
Hi,
I have tried to rebase all the 19(6+13) patches on 4.11-rc1,
all the patchse can directly apply on 4.11-rc1,
Could you help to review the patches, and see if there is anywhere I
can improve ?
I'm sorry I have taken so long to review this posting.
This is generally looking much better. I'm not sure about a few details
relating to the watchdog, but I'm largely happy to pick up the rest of
the series, if you can address my comments.
Thanks,
Mark.
From: Mark Rutland <mark.rutland@arm.com> Date: 2017-03-17 20:11:10
On Tue, Feb 07, 2017 at 02:50:15AM +0800, fu.wei at linaro.org wrote:
+static int __init gtdt_import_sbsa_gwdt(struct acpi_gtdt_watchdog *wd,
+ int index)
+{
+ struct platform_device *pdev;
+ int irq = map_gt_gsi(wd->timer_interrupt, wd->timer_flags);
+ int no_irq = 1;
+
+ /*
+ * According to SBSA specification the size of refresh and control
+ * frames of SBSA Generic Watchdog is SZ_4K(Offset 0x000 ? 0xFFF).
+ */
+ struct resource res[] = {
+ DEFINE_RES_MEM(wd->control_frame_address, SZ_4K),
+ DEFINE_RES_MEM(wd->refresh_frame_address, SZ_4K),
+ DEFINE_RES_IRQ(irq),
+ };
+
+ pr_debug("found a Watchdog (0x%llx/0x%llx gsi:%u flags:0x%x).\n",
+ wd->refresh_frame_address, wd->control_frame_address,
+ wd->timer_interrupt, wd->timer_flags);
+
+ if (!(wd->refresh_frame_address && wd->control_frame_address)) {
+ pr_err(FW_BUG "failed to get the Watchdog base address.\n");
+ return -EINVAL;
+ }
+
+ if (!wd->timer_interrupt)
+ pr_warn(FW_BUG "failed to get the Watchdog interrupt.\n");
I've not been able to find where the ACPI spec says that zero is not a
valid GSIV. This may simply be an oversight/ambiguity in the spec.
Is there any statement to that effect?
+ else if (irq <= 0)
+ pr_warn("failed to map the Watchdog interrupt.\n");
+ else
+ no_irq = 0;
+
+ /*
+ * Add a platform device named "sbsa-gwdt" to match the platform driver.
+ * "sbsa-gwdt": SBSA(Server Base System Architecture) Generic Watchdog
+ * The platform driver (like drivers/watchdog/sbsa_gwdt.c)can get device
+ * info below by matching this name.
+ */
+ pdev = platform_device_register_simple("sbsa-gwdt", index, res,
+ ARRAY_SIZE(res) - no_irq);
This no_irq variable is messy and confusing.
Get rid of no_irq, and replace it with nr_res, initialised to
ARRAY_SIZE(res). If there's no interrupt, subtract one.
[...]
+ for_each_platform_timer(platform_timer) {
+ if (is_watchdog(platform_timer)) {
+ ret = gtdt_import_sbsa_gwdt(platform_timer, i);
+ if (ret)
+ break;
+ i++;
+ }
+ }
+
+ if (i)
+ pr_info("found %d SBSA generic Watchdog(s).\n", i);
My reading of SBSA is that there is one watchdog in the system.
Is that not the case?
[...]
Hi Mark,
On 18 March 2017 at 04:03, Mark Rutland [off-list ref] wrote:
On Thu, Mar 09, 2017 at 11:47:16PM +0100, Fu Wei wrote:
quoted
Hi Mark, Marc,
Hi,
quoted
I have tried to rebase all the 19(6+13) patches on 4.11-rc1,
all the patchse can directly apply on 4.11-rc1,
Could you help to review the patches, and see if there is anywhere I
can improve ?
I'm sorry I have taken so long to review this posting.
This is generally looking much better. I'm not sure about a few details
relating to the watchdog, but I'm largely happy to pick up the rest of
the series, if you can address my comments.
Great thanks for your review, I will post a new v22 ASAP(before your Tuesday).
Hi Mark,
On 18 March 2017 at 02:07, Mark Rutland [off-list ref] wrote:
On Tue, Feb 07, 2017 at 02:50:05AM +0800, fu.wei at linaro.org wrote:
quoted
From: Fu Wei <redacted>
The original counter frequency detection call(arch_timer_detect_rate)
include getting the frequency from the per-cpu arch-timer and the
memory-mapped (MMIO) timer interfaces. But they will be needed only when
the system initializes the relevant timer.
This patch remove arch_timer_detect_rate founction, and use the
arch_timer_get_sysreg_freq and arch_timer_get_mmio_freq directly.
Signed-off-by: Fu Wei <redacted>
Could you please fold this with the prior patch?
Sure, will do
quoted
@@ -1087,7 +1072,12 @@ static int __init arch_timer_mem_init(struct device_node *np) */ if (!arch_timer_rate && of_property_read_u32(np, "clock-frequency", &arch_timer_rate))- arch_timer_detect_rate(base);+ arch_timer_rate = arch_timer_get_mmio_freq(base);+ if (!arch_timer_rate) {+ pr_err(FW_BUG "frequency not available for MMIO timer.\n");
It would be better to say "MMIO frequency not available.\n" here.
Hi Mark,
On 18 March 2017 at 02:05, Mark Rutland [off-list ref] wrote:
On Tue, Feb 07, 2017 at 02:50:03AM +0800, fu.wei at linaro.org wrote:
quoted
+static u32 arch_timer_get_sysreg_freq(void)
+{
+ /*
+ * Try to get the frequency from the CNTFRQ of sysreg.
+ */
+ return arch_timer_get_cntfrq();
+}
We already have arch_timer_get_cntfrq(), so I don't see the point in
this wrapper.
quoted
+static u32 arch_timer_get_mmio_freq(void __iomem *cntbase)
+{
+ /*
+ * Try to get the frequency from the CNTFRQ of timer frame registers.
+ * Note: please verify cntbase in caller.
+ */
+ return readl_relaxed(cntbase + CNTFRQ);
+}
Wrapping the MMIO read makes sense if we're going to do this in more
than one place, so I'm happy with this wrapper.
If you can s/arch_timer_get_mmio_freq/arch_timer_get_cntfrq/, and drop
sorry, May I guess that is
"s/arch_timer_get_mmio_freq/arch_timer_get_mmio_cntfrq/"
or
"s/arch_timer_get_mmio_freq/arch_timer_mem_get_cntfrq/"
which one do you prefer? :-)
the comments, then this looks fine to me.
Thanks,
Mark.
Hi Mark,
On 20 March 2017 at 15:36, Fu Wei [off-list ref] wrote:
Hi Mark,
On 18 March 2017 at 02:05, Mark Rutland [off-list ref] wrote:
quoted
On Tue, Feb 07, 2017 at 02:50:03AM +0800, fu.wei at linaro.org wrote:
quoted
+static u32 arch_timer_get_sysreg_freq(void)
+{
+ /*
+ * Try to get the frequency from the CNTFRQ of sysreg.
+ */
+ return arch_timer_get_cntfrq();
+}
We already have arch_timer_get_cntfrq(), so I don't see the point in
this wrapper.
quoted
+static u32 arch_timer_get_mmio_freq(void __iomem *cntbase)
+{
+ /*
+ * Try to get the frequency from the CNTFRQ of timer frame registers.
+ * Note: please verify cntbase in caller.
+ */
+ return readl_relaxed(cntbase + CNTFRQ);
+}
Wrapping the MMIO read makes sense if we're going to do this in more
than one place, so I'm happy with this wrapper.
If you can s/arch_timer_get_mmio_freq/arch_timer_get_cntfrq/, and drop
sorry, May I guess that is
"s/arch_timer_get_mmio_freq/arch_timer_get_mmio_cntfrq/"
or
"s/arch_timer_get_mmio_freq/arch_timer_mem_get_cntfrq/"
which one do you prefer? :-)
keeping using arch_timer_get_cntfrq(); for per-CPU arch timer, then
+static u32 arch_timer_mem_get_cntfrq(void __iomem *cntbase)
+{
+ return readl_relaxed(cntbase + CNTFRQ);
+}
+
Is that OK for you?
quoted
the comments, then this looks fine to me.
Thanks,
Mark.
From: Mark Rutland <mark.rutland@arm.com> Date: 2017-03-20 10:43:24
On Mon, Mar 20, 2017 at 05:43:29PM +0800, Fu Wei wrote:
On 20 March 2017 at 15:36, Fu Wei [off-list ref] wrote:
quoted
On 18 March 2017 at 02:05, Mark Rutland [off-list ref] wrote:
quoted
On Tue, Feb 07, 2017 at 02:50:03AM +0800, fu.wei at linaro.org wrote:
quoted
quoted
quoted
+static u32 arch_timer_get_mmio_freq(void __iomem *cntbase)
+{
+ /*
+ * Try to get the frequency from the CNTFRQ of timer frame registers.
+ * Note: please verify cntbase in caller.
+ */
+ return readl_relaxed(cntbase + CNTFRQ);
+}
Wrapping the MMIO read makes sense if we're going to do this in more
than one place, so I'm happy with this wrapper.
If you can s/arch_timer_get_mmio_freq/arch_timer_get_cntfrq/, and drop
sorry, May I guess that is
"s/arch_timer_get_mmio_freq/arch_timer_get_mmio_cntfrq/"
or
"s/arch_timer_get_mmio_freq/arch_timer_mem_get_cntfrq/"
which one do you prefer? :-)
keeping using arch_timer_get_cntfrq(); for per-CPU arch timer, then
+static u32 arch_timer_mem_get_cntfrq(void __iomem *cntbase)
+{
+ return readl_relaxed(cntbase + CNTFRQ);
+}
+
That looks perfect to me.
Sorry for the confusion above!
Mark.
Hi Mark,
On 20 March 2017 at 18:41, Mark Rutland [off-list ref] wrote:
On Mon, Mar 20, 2017 at 05:43:29PM +0800, Fu Wei wrote:
quoted
On 20 March 2017 at 15:36, Fu Wei [off-list ref] wrote:
quoted
On 18 March 2017 at 02:05, Mark Rutland [off-list ref] wrote:
quoted
On Tue, Feb 07, 2017 at 02:50:03AM +0800, fu.wei at linaro.org wrote:
quoted
quoted
quoted
quoted
+static u32 arch_timer_get_mmio_freq(void __iomem *cntbase)
+{
+ /*
+ * Try to get the frequency from the CNTFRQ of timer frame registers.
+ * Note: please verify cntbase in caller.
+ */
+ return readl_relaxed(cntbase + CNTFRQ);
+}
Wrapping the MMIO read makes sense if we're going to do this in more
than one place, so I'm happy with this wrapper.
If you can s/arch_timer_get_mmio_freq/arch_timer_get_cntfrq/, and drop
sorry, May I guess that is
"s/arch_timer_get_mmio_freq/arch_timer_get_mmio_cntfrq/"
or
"s/arch_timer_get_mmio_freq/arch_timer_mem_get_cntfrq/"
which one do you prefer? :-)
keeping using arch_timer_get_cntfrq(); for per-CPU arch timer, then
+static u32 arch_timer_mem_get_cntfrq(void __iomem *cntbase)
+{
+ return readl_relaxed(cntbase + CNTFRQ);
+}
+
That looks perfect to me.
Sorry for the confusion above!
Hi Mark,
On 18 March 2017 at 03:05, Mark Rutland [off-list ref] wrote:
On Tue, Feb 07, 2017 at 02:50:06AM +0800, fu.wei at linaro.org wrote:
quoted
From: Fu Wei <redacted>
Currently, arch_timer_rate is used to store the frequency got from per-cpu
arch-timer or the memory-mapped (MMIO) timers. But those values come from
different registers which should all be initialized by firmware.
This patch remove arch_timer_rate, and use arch_timer_sysreg_freq and
arch_timer_mmio_freq instead.
Signed-off-by: Fu Wei <redacted>
Thanks for attacking this. Generally, I do think this is the right thing
to do.
However...
quoted
@@ -1070,10 +1077,9 @@ static int __init arch_timer_mem_init(struct device_node *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_get_mmio_freq(base);- if (!arch_timer_rate) {+ if (of_property_read_u32(np, "clock-frequency", &arch_timer_mmio_freq))+ arch_timer_mmio_freq = arch_timer_get_mmio_freq(base);+ if (!arch_timer_mmio_freq) { pr_err(FW_BUG "frequency not available for MMIO timer.\n"); ret = -EINVAL; goto out;
... unfortunately, I believe that this will break some DT platforms that
have been (unintentionally) relying on the way currently allow the
frequency to be probed from either the MMIO timer or the sysreg timer.
Ah, I 'm really not aware of this. Thanks for pointing it out.
So while the above was my suggestion, it was not my best.
For the timebeing, let's leave the single arch_timer_rate, but ensure
that it doesn't get in the way fo the ACPI code, by making the ACPI
probe path:
* Probe the sysreg timers first, using the sysreg cntfrq().
* Probe the MMIO timers second, verifying that each MMIO cntfrq matches
the already-probed sysreg cntfrq.
... which is what I believe you suggested previously.
Hi Mark,
On 18 March 2017 at 03:40, Mark Rutland [off-list ref] wrote:
Hi,
On Tue, Feb 07, 2017 at 02:50:13AM +0800, fu.wei at linaro.org wrote:
quoted
+static int __init gtdt_parse_timer_block(struct acpi_gtdt_timer_block *block,
+ struct arch_timer_mem *data)
Please s/data/timer_mem/ here, to match the rest of the timer code.
quoted
+{
+ int i, j;
+ struct acpi_gtdt_timer_entry *frame;
So as to make it clear what this is, and to make things a litlte simpler
below, please s/frame/gtdt_frame/ here.
quoted
+
+ if (!block->timer_count) {
+ pr_err(FW_BUG "GT block present, but frame count is zero.");
+ return -ENODEV;
+ }
+
+ if (block->timer_count > ARCH_TIMER_MEM_MAX_FRAMES) {
+ pr_err(FW_BUG "GT block lists %d frames, ACPI spec only allows 8\n",
+ block->timer_count);
+ return -EINVAL;
+ }
+
+ data->cntctlbase = (phys_addr_t)block->block_address;
+ /*
+ * According to "Table * CNTCTLBase memory map" of
+ * <ARM Architecture Reference Manual> for ARMv8,
+ * The size of the CNTCTLBase frame is 4KB(Offset 0x000 ? 0xFFC).
+ */
As a general thing, please cite the version of the ARM ARM you're
referring to, as over time the internal numbering (and the headings)
change.
e.g.
/*
* The CNTCTLBase frame is 4KB (register offsets 0x000 - 0xFFC).
* See ARM DDI 0487A.k_iss10775, page I1-5129, Table I1-3
* "CNTCTLBase memory map".
*/
quoted
+ data->size = SZ_4K;
+
+ frame = (void *)block + block->timer_offset;
+ if (frame + block->timer_count != (void *)block + block->header.length)
+ return -EINVAL;
+
+ /*
+ * Get the GT timer Frame data for every GT Block Timer
+ */
+ for (i = 0, j = 0; i < block->timer_count; i++, frame++) {
With the gtdt_frame rename as above, here we can do:
struct arch_timer_mem_frame *frame = &timer_mem->frame[j];
From: Mark Rutland <mark.rutland@arm.com> Date: 2017-03-20 18:10:26
On Tue, Mar 21, 2017 at 01:57:58AM +0800, Fu Wei wrote:
On 18 March 2017 at 04:01, Mark Rutland [off-list ref] wrote:
quoted
On Tue, Feb 07, 2017 at 02:50:15AM +0800, fu.wei at linaro.org wrote:
quoted
I've not been able to find where the ACPI spec says that zero is not a
valid GSIV. This may simply be an oversight/ambiguity in the spec.
Is there any statement to that effect?
you are right, zero is a valid GSIV, I will delete this check. Thanks
That being the case, how does one describe a watchdog that does not have
an interrupt?
As I mentioned, I think this is an oversight/ambiguity in the spec tat
we should address.
quoted
My reading of SBSA is that there is one watchdog in the system.
Is that not the case?
do you mean:
---------------
4.2.4 Watchdogs
The base server system implements a Generic Watchdog as specified in
APPENDIX A: Generic Watchdog.
---------------
I am not sure about that if this is saying "we only have one SBSA
watchdog in a system"
would you let me know where mention it? Do I miss something?
My reading was that the 'a' above meant a single element. i.e.
The base server system implements _a_ Generic Watchdog as
specified in APPENDIX A: Generic Watchdog.
Subsequently in 4.2.5, it is stated:
In this scenario, the system wakeup timer or generic watchdog is
still required to send its interrupt.
... which only makes sense if there is a single watchdog in the system.
Perhaps this is an oversight in the specification.
Thanks,
Mark.
Hi Mark
On 18 March 2017 at 04:01, Mark Rutland [off-list ref] wrote:
On Tue, Feb 07, 2017 at 02:50:15AM +0800, fu.wei at linaro.org wrote:
quoted
+static int __init gtdt_import_sbsa_gwdt(struct acpi_gtdt_watchdog *wd,
+ int index)
+{
+ struct platform_device *pdev;
+ int irq = map_gt_gsi(wd->timer_interrupt, wd->timer_flags);
+ int no_irq = 1;
+
+ /*
+ * According to SBSA specification the size of refresh and control
+ * frames of SBSA Generic Watchdog is SZ_4K(Offset 0x000 ? 0xFFF).
+ */
+ struct resource res[] = {
+ DEFINE_RES_MEM(wd->control_frame_address, SZ_4K),
+ DEFINE_RES_MEM(wd->refresh_frame_address, SZ_4K),
+ DEFINE_RES_IRQ(irq),
+ };
+
+ pr_debug("found a Watchdog (0x%llx/0x%llx gsi:%u flags:0x%x).\n",
+ wd->refresh_frame_address, wd->control_frame_address,
+ wd->timer_interrupt, wd->timer_flags);
+
+ if (!(wd->refresh_frame_address && wd->control_frame_address)) {
+ pr_err(FW_BUG "failed to get the Watchdog base address.\n");
+ return -EINVAL;
+ }
+
+ if (!wd->timer_interrupt)
+ pr_warn(FW_BUG "failed to get the Watchdog interrupt.\n");
I've not been able to find where the ACPI spec says that zero is not a
valid GSIV. This may simply be an oversight/ambiguity in the spec.
Is there any statement to that effect?
you are right, zero is a valid GSIV, I will delete this check. Thanks
quoted
+ else if (irq <= 0)
+ pr_warn("failed to map the Watchdog interrupt.\n");
+ else
+ no_irq = 0;
+
+ /*
+ * Add a platform device named "sbsa-gwdt" to match the platform driver.
+ * "sbsa-gwdt": SBSA(Server Base System Architecture) Generic Watchdog
+ * The platform driver (like drivers/watchdog/sbsa_gwdt.c)can get device
+ * info below by matching this name.
+ */
+ pdev = platform_device_register_simple("sbsa-gwdt", index, res,
+ ARRAY_SIZE(res) - no_irq);
This no_irq variable is messy and confusing.
Get rid of no_irq, and replace it with nr_res, initialised to
ARRAY_SIZE(res). If there's no interrupt, subtract one.
Sure, you are right ,will do
[...]
quoted
+ for_each_platform_timer(platform_timer) {
+ if (is_watchdog(platform_timer)) {
+ ret = gtdt_import_sbsa_gwdt(platform_timer, i);
+ if (ret)
+ break;
+ i++;
+ }
+ }
+
+ if (i)
+ pr_info("found %d SBSA generic Watchdog(s).\n", i);
My reading of SBSA is that there is one watchdog in the system.
Is that not the case?
do you mean:
---------------
4.2.4 Watchdogs
The base server system implements a Generic Watchdog as specified in
APPENDIX A: Generic Watchdog.
---------------
I am not sure about that if this is saying "we only have one SBSA
watchdog in a system"
would you let me know where mention it? Do I miss something?
Thanks :-)
From: Lurndal, Scott <hidden> Date: 2017-03-20 18:50:16
On Mon, Mar 20, 2017 at 06:09:50PM +0000, Mark Rutland wrote:
On Tue, Mar 21, 2017 at 01:57:58AM +0800, Fu Wei wrote:
quoted
On 18 March 2017 at 04:01, Mark Rutland [off-list ref] wrote:
quoted
On Tue, Feb 07, 2017 at 02:50:15AM +0800, fu.wei at linaro.org wrote:
quoted
quoted
I've not been able to find where the ACPI spec says that zero is not a
valid GSIV. This may simply be an oversight/ambiguity in the spec.
Is there any statement to that effect?
you are right, zero is a valid GSIV, I will delete this check. Thanks
That being the case, how does one describe a watchdog that does not have
an interrupt?
As I mentioned, I think this is an oversight/ambiguity in the spec tat
we should address.
quoted
quoted
My reading of SBSA is that there is one watchdog in the system.
Is that not the case?
do you mean:
---------------
4.2.4 Watchdogs
The base server system implements a Generic Watchdog as specified in
APPENDIX A: Generic Watchdog.
---------------
I am not sure about that if this is saying "we only have one SBSA
watchdog in a system"
would you let me know where mention it? Do I miss something?
My reading was that the 'a' above meant a single element. i.e.
The base server system implements _a_ Generic Watchdog as
specified in APPENDIX A: Generic Watchdog.
It is a requirement of a conforming implementation that there
be a generic watchdog (impl as per the appendix). That doesn't preclude
an implmentation from providing additional watchdogs (for example, if
the processor implements EL3, it is likely that an implementation
will include a secure watchdog as well as a non-secure watchdog).
The SBSA describes the minimal hardware requirements for a
compliant server.
scott
Hi Mark, Lurndal,
On 21 March 2017 at 02:50, Lurndal, Scott [off-list ref] wrote:
On Mon, Mar 20, 2017 at 06:09:50PM +0000, Mark Rutland wrote:
quoted
On Tue, Mar 21, 2017 at 01:57:58AM +0800, Fu Wei wrote:
quoted
On 18 March 2017 at 04:01, Mark Rutland [off-list ref] wrote:
quoted
On Tue, Feb 07, 2017 at 02:50:15AM +0800, fu.wei at linaro.org wrote:
quoted
quoted
I've not been able to find where the ACPI spec says that zero is not a
valid GSIV. This may simply be an oversight/ambiguity in the spec.
Is there any statement to that effect?
you are right, zero is a valid GSIV, I will delete this check. Thanks
That being the case, how does one describe a watchdog that does not have
an interrupt?
As I mentioned, I think this is an oversight/ambiguity in the spec tat
we should address.
quoted
quoted
My reading of SBSA is that there is one watchdog in the system.
Is that not the case?
do you mean:
---------------
4.2.4 Watchdogs
The base server system implements a Generic Watchdog as specified in
APPENDIX A: Generic Watchdog.
---------------
I am not sure about that if this is saying "we only have one SBSA
watchdog in a system"
would you let me know where mention it? Do I miss something?
My reading was that the 'a' above meant a single element. i.e.
The base server system implements _a_ Generic Watchdog as
specified in APPENDIX A: Generic Watchdog.
It is a requirement of a conforming implementation that there
be a generic watchdog (impl as per the appendix). That doesn't preclude
an implmentation from providing additional watchdogs (for example, if
the processor implements EL3, it is likely that an implementation
will include a secure watchdog as well as a non-secure watchdog).
The SBSA describes the minimal hardware requirements for a
compliant server.
So I think, for the SBSA watchdog:
(1) there maybe more then one non-secure watchdog in GTDT
(2) we may also need to skip secure watchdogs in GTDT,
and only register non-secure watchdogs into platform resources.
Please correct me, if I misunderstand something.
Hi Mark,
On 21 March 2017 at 02:09, Mark Rutland [off-list ref] wrote:
On Tue, Mar 21, 2017 at 01:57:58AM +0800, Fu Wei wrote:
quoted
On 18 March 2017 at 04:01, Mark Rutland [off-list ref] wrote:
quoted
On Tue, Feb 07, 2017 at 02:50:15AM +0800, fu.wei at linaro.org wrote:
quoted
quoted
I've not been able to find where the ACPI spec says that zero is not a
valid GSIV. This may simply be an oversight/ambiguity in the spec.
Is there any statement to that effect?
you are right, zero is a valid GSIV, I will delete this check. Thanks
That being the case, how does one describe a watchdog that does not have
an interrupt?
I think we may can use "Timer Flags", because all the GSIV come with a flag,
if we can define a bit field called "valid" for all GSIV
Bit Field Bit Offset Number of bits Description
Valid 31 1 This bit
indicates the validity of the timer interrupt
1:
Interrupt is valid
0:
Interrupt is invalid
Then we don't need to test the value of GSIV, just test this bit instead.
Just my thought, hope this makes sense to all of you :-)
As I mentioned, I think this is an oversight/ambiguity in the spec tat
we should address.
quoted
quoted
My reading of SBSA is that there is one watchdog in the system.
Is that not the case?
do you mean:
---------------
4.2.4 Watchdogs
The base server system implements a Generic Watchdog as specified in
APPENDIX A: Generic Watchdog.
---------------
I am not sure about that if this is saying "we only have one SBSA
watchdog in a system"
would you let me know where mention it? Do I miss something?
My reading was that the 'a' above meant a single element. i.e.
The base server system implements _a_ Generic Watchdog as
specified in APPENDIX A: Generic Watchdog.
Subsequently in 4.2.5, it is stated:
In this scenario, the system wakeup timer or generic watchdog is
still required to send its interrupt.
... which only makes sense if there is a single watchdog in the system.
Perhaps this is an oversight in the specification.
Thanks,
Mark.
From: Lurndal, Scott <hidden> Date: 2017-03-21 12:48:26
On Tue, Mar 21, 2017 at 11:48:02AM +0800, Fu Wei wrote:
Hi Mark, Lurndal,
quoted
quoted
The base server system implements _a_ Generic Watchdog as
specified in APPENDIX A: Generic Watchdog.
It is a requirement of a conforming implementation that there
be a generic watchdog (impl as per the appendix). That doesn't preclude
an implmentation from providing additional watchdogs (for example, if
the processor implements EL3, it is likely that an implementation
will include a secure watchdog as well as a non-secure watchdog).
The SBSA describes the minimal hardware requirements for a
compliant server.
So I think, for the SBSA watchdog:
(1) there maybe more then one non-secure watchdog in GTDT
I suppose a firmware vendor could chose to expose more than the
platform minimum to linux. Not quite sure what linux would
do with it :-)
(2) we may also need to skip secure watchdogs in GTDT,
and only register non-secure watchdogs into platform resources.
The secure watchdog will, be definition, never be accessible
to non-secure EL2 or non-secure EL1, so only in the case of
a secure EL1 operating system would the secure watchdog ever
be conveyed through ACPI. I don't think secure EL1 is in
the scope of the SBSA, or something that needs to be accomodated
in the GTDT at this time.
scott