Re: [PATCH v12 7/8] clocksource/drivers/arm_arch_timer: Add GTDT support for memory-mapped timer

[Marc Zyngier]

Argh, new version...

On 13/09/16 11:39, fu.wei@xxxxxxxxxx wrote:
> From: Fu Wei <fu.wei@xxxxxxxxxx>
> 
> 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 <fu.wei@xxxxxxxxxx>
> ---
>  drivers/clocksource/arm_arch_timer.c | 127 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 124 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 0197ef9..d33802b 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -888,7 +888,128 @@ CLOCKSOURCE_OF_DECLARE(armv7_arch_timer_mem, "arm,armv7-timer-mem",
>  		       arch_timer_mem_init);
>  
>  #ifdef CONFIG_ACPI_GTDT
> -/* Initialize per-processor generic timer */
> +static struct gt_timer_data __init *arch_timer_mem_get_timer(
> +						struct gt_block_data *gt_blocks)
> +{
> +	struct gt_block_data *gt_block = gt_blocks;
> +	struct gt_timer_data *best_frame = NULL;
> +	void __iomem *cntctlbase;
> +	u32 cnttidr;
> +	int i;
> +
> +	/*
> +	 * According to ARMv8 Architecture Reference Manual(ARM),
> +	 * the size of CNTCTLBase frame of memory-mapped timer
> +	 * is SZ_4K(Offset 0x000 â 0xFFF).
> +	 */
> +	cntctlbase = ioremap(gt_block->cntctlbase_phy, SZ_4K);
> +	if (!cntctlbase) {
> +		pr_err("Failed to map mem timer control frame base address\n");
> +		return NULL;
> +	}
> +	cnttidr = readl_relaxed(cntctlbase + CNTTIDR);
> +
> +	/*
> +	 * Try to find a virtual capable frame. Otherwise fall back to a
> +	 * physical capable frame.
> +	 */
> +	for (i = 0; i < gt_block->timer_count; i++) {
> +		int n;
> +		u32 cntacr;
> +
> +		n = gt_block->timer[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))) {
> +			best_frame = &gt_block->timer[i];
> +			arch_timer_mem_use_virtual = true;
> +			break;
> +		}
> +
> +		if (~cntacr & (CNTACR_RWPT | CNTACR_RPCT))
> +			continue;
> +
> +		best_frame = &gt_block->timer[i];
> +	}
> +	iounmap(cntctlbase);

As I just said in my reply to the same patch in v11, all of this is
duplicating existing infrastructure that already exists for DT. Please
consider decoupling the core driver code from the firmware side and make
this reusable.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...