linux-kernel - Re: [PATCH v19 10/15] clocksource/drivers/arm_arch_timer: Refactor the timer init code to prepare for GTDT

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170116183032.GL5908@leverpostej>
Date:   Mon, 16 Jan 2017 18:30:32 +0000
From:   Mark Rutland <mark.rutland@....com>
To:     fu.wei@...aro.org
Cc:     rjw@...ysocki.net, lenb@...nel.org, daniel.lezcano@...aro.org,
        tglx@...utronix.de, marc.zyngier@....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 v19 10/15] clocksource/drivers/arm_arch_timer: Refactor
 the timer init code to prepare for GTDT

On Wed, Dec 21, 2016 at 02:45:58PM +0800, fu.wei@...aro.org wrote:
> From: Fu Wei <fu.wei@...aro.org>
> 
> The patch refactor original memory-mapped timer init code:
>     (1) Refactor "arch_timer_mem_init", make it become a common code for
>         memory-mapped timer init.
>     (2) Add a new function "arch_timer_mem_of_init" for DT init.

As a general note, please write proper commit messages, describing what
the problem is, and why we are making the changes. These bullet points
don't add anything to what can be derived from a glance at the code.

For this patch, you can use:

  clocksource: arm_arch_timer: refactor MMIO timer probing

  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.

[...]

> +	for_each_available_child_of_node(np, frame_node) {
> +		int n;
> +		struct arch_timer_mem_frame *frame = &timer_mem->frame[i];
> +
> +		if (of_property_read_u32(frame_node, "frame-number", &n)) {
> +			pr_err("Missing frame-number\n");
> +			of_node_put(frame_node);
> +			goto out;
> +		}
> +		frame->frame_nr = n;
> +
> +		if (of_address_to_resource(frame_node, 0, &res)) {
> +			of_node_put(frame_node);
> +			goto out;
> +		}
> +		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);
>  
> -	if (!arch_timer_needs_of_probing())
> +		if (++i >= ARCH_TIMER_MEM_MAX_FRAMES)
> +			break;
> +	}

It would be good if we could warn upon seeing more than
ARCH_TIMER_MEM_MAX_FRAMES children, since that's obviously an error.

Thanks,
Mark.