Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v16 14/15] clocksource/drivers/arm_arch_timer: Add GTDT support for memory-mapped timer
From: Mark Rutland @ 2016-11-18 20:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479304148-2965-15-git-send-email-fu.wei@linaro.org>

On Wed, Nov 16, 2016 at 09:49:07PM +0800, fu.wei at linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
> 
> 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@linaro.org>
> ---
>  drivers/clocksource/arm_arch_timer.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index c494ca8..0aad60a 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -1067,7 +1067,28 @@ CLOCKSOURCE_OF_DECLARE(armv7_arch_timer_mem, "arm,armv7-timer-mem",
>  		       arch_timer_mem_of_init);
>  
>  #ifdef CONFIG_ACPI_GTDT
> -/* Initialize per-processor generic timer */
> +static int __init arch_timer_mem_acpi_init(void)
> +{
> +	struct arch_timer_mem *timer_mem;
> +	int ret = 0;
> +	int i = 0;
> +
> +	timer_mem = kzalloc(sizeof(*timer_mem), GFP_KERNEL);

Why do you need it zeroed? You don't clear it between iterations, so
either you don't need this, or you have a bug in the loop.

> +	if (!timer_mem)
> +		return -ENOMEM;
> +
> +	while (!gtdt_arch_timer_mem_init(timer_mem, i)) {

Huh?

Why doesn't GTDT expose a function to fill in the entire arch_timer_mem
in one go?

There shouldn't be multiple instances, as far as I am aware. Am I
mistaken?

> +		ret = arch_timer_mem_init(timer_mem);
> +		if (ret)
> +			break;
> +		i++;
> +	}
> +
> +	kfree(timer_mem);
> +	return ret;
> +}


Regardless, arch_timer_mem is small enough that you don't need dynamic
allocaiton. Just put it on the stack, e.g.

static int __init arch_timer_mem_acpi_init(void)
{
	int i = 0;
	struct arch_timer_mem timer_mem;

	while (!gtdt_arch_timer_mem_init(timer_mem, i)) {
		int ret = arch_timer_mem_init();
		if (ret)
			return ret;
		i++
	}

	return 0;
}

Thanks,
Mark.

^ permalink raw reply

* [PATCH v16 11/15] acpi/arm64: Add GTDT table parse driver
From: Mark Rutland @ 2016-11-18 20:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479304148-2965-12-git-send-email-fu.wei@linaro.org>

On Wed, Nov 16, 2016 at 09:49:04PM +0800, fu.wei at linaro.org wrote:

> +#define for_each_platform_timer(_g) for (; _g; _g = next_platform_timer(_g))

This doesn't fit the usual for_each_* pattern, since _g has to be
manually initialised first. Either come up with a way of maknig this fit
the usual pattern, or get rid of this, and use:

	t = however_you_get_the_first_timer();

	if (!t)
		bailt_out_somehow();
	
	do { 
		...
	} while (t = next_platform_timer(t));

> +/*
> + * Release the memory we have allocated in acpi_gtdt_init.
> + * This should be called, when the driver who called "acpi_gtdt_init" previously
> + * doesn't need the GTDT info anymore.
> + */
> +void __init acpi_gtdt_release(void)
> +{
> +	kfree(timer_block);
> +	kfree(watchdog);
> +	timer_block = NULL;
> +	watchdog = NULL;
> +}

Why is this not simply in the error path of acpi_gtdt_init()?

> +
> +/*
> + * Get some basic info from GTDT table, and init the global variables above
> + * for all timers initialization of Generic Timer.
> + * This function does some validation on GTDT table.
> + */
> +int __init acpi_gtdt_init(struct acpi_table_header *table)
> +{

> +	timer_block = kcalloc(timer_count,
> +			      sizeof(struct acpi_gtdt_timer_block *),
> +			      GFP_KERNEL);
> +	if (!timer_block)
> +		return -ENOMEM;
> +
> +	watchdog = kcalloc(timer_count, sizeof(struct acpi_gtdt_watchdog *),
> +			   GFP_KERNEL);
> +	if (!watchdog) {
> +		kfree(timer_block);
> +		timer_block = NULL;
> +		return -ENOMEM;
> +	}

Please have a common error path below, and branch to that when you need
to free these.

> +error:
> +	acpi_gtdt_release();
> +	return -EINVAL;
> +}

[...]

> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 61a3d90..a1611d1 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -577,6 +577,13 @@ enum acpi_reconfig_event  {
>  int acpi_reconfig_notifier_register(struct notifier_block *nb);
>  int acpi_reconfig_notifier_unregister(struct notifier_block *nb);
>  
> +#ifdef CONFIG_ACPI_GTDT
> +int acpi_gtdt_init(struct acpi_table_header *table);
> +int acpi_gtdt_map_ppi(int type);
> +bool acpi_gtdt_c3stop(int type);
> +void acpi_gtdt_release(void);

Why do these need to be here?

What possible value is ther in exporting acpi_gtdt_release() !?

Thanks,
Mark.

^ permalink raw reply

* [PATCH] arm64: dts: qcom: msm8996: Fixup smp2p node
From: Bjorn Andersson @ 2016-11-18 20:06 UTC (permalink / raw)
  To: linux-arm-kernel

The SMEM state property name changes between the integration branch and
mainline, update to use the correct one.

Fixes: 2f45d9fcd531 ("arm64: dts: msm8996: Add SMP2P and APCS nodes")
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Seems we're still carrying this discrepancy somewhere and I missed it in my
review, sorry about that.

 arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index cde4114bae5a..712d5d043105 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -530,7 +530,7 @@
 
 		adsp_smp2p_out: master-kernel {
 			qcom,entry-name = "master-kernel";
-			#qcom,state-cells = <1>;
+			#qcom,smem-state-cells = <1>;
 		};
 
 		adsp_smp2p_in: slave-kernel {
-- 
2.5.0

^ permalink raw reply related

* [PATCH v16 10/15] clocksource/drivers/arm_arch_timer: Refactor the timer init code to prepare for GTDT
From: Mark Rutland @ 2016-11-18 20:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479304148-2965-11-git-send-email-fu.wei@linaro.org>

On Wed, Nov 16, 2016 at 09:49:03PM +0800, fu.wei at linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
> 
> The patch refactor original memory-mapped timer init code:
>     (1) Extract a subfunction for detecting a bast time frame:
>         is_best_frame.

Please leave this logic in arch_timer_mem_init(). Pulling it out gains
us nothing, but makes the patch harder to review.

>     (2) Refactor "arch_timer_mem_init", make it become a common code for
>         memory-mapped timer init.
>     (3) Add a new function "arch_timer_mem_of_init" for DT init.

These generally look fine.

Thanks,
Mark.

> Signed-off-by: Fu Wei <fu.wei@linaro.org>
> ---
>  drivers/clocksource/arm_arch_timer.c | 162 +++++++++++++++++++++++------------
>  1 file changed, 107 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 9ddc091..0836bb9 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -923,17 +923,35 @@ static int __init arch_timer_of_init(struct device_node *np)
>  CLOCKSOURCE_OF_DECLARE(armv7_arch_timer, "arm,armv7-timer", arch_timer_of_init);
>  CLOCKSOURCE_OF_DECLARE(armv8_arch_timer, "arm,armv8-timer", arch_timer_of_init);
>  
> -static int __init arch_timer_mem_init(struct device_node *np)
> +static bool __init is_best_frame(void __iomem *cntctlbase, u32 cnttidr, int n)
> +{
> +	u32 cntacr = CNTACR_RFRQ | CNTACR_RWPT | CNTACR_RPCT | CNTACR_RWVT |
> +		     CNTACR_RVOFF | CNTACR_RVCT;
> +
> +	/* Try enabling everything, and see what sticks */
> +	writel_relaxed(cntacr, cntctlbase + CNTACR(n));
> +	cntacr = readl_relaxed(cntctlbase + CNTACR(n));
> +
> +	if ((cnttidr & CNTTIDR_VIRT(n)) &&
> +	    !(~cntacr & (CNTACR_RWVT | CNTACR_RVCT)))
> +		arch_timer_mem_use_virtual = true;
> +	else if (~cntacr & (CNTACR_RWPT | CNTACR_RPCT))
> +		return false;
> +
> +	return true;
> +}
> +
> +static int __init arch_timer_mem_init(struct arch_timer_mem *timer_mem)
>  {
> -	struct device_node *frame, *best_frame = NULL;
>  	void __iomem *cntctlbase, *base;
> -	unsigned int irq, ret = -EINVAL;
> +	struct arch_timer_mem_frame *best_frame = NULL;
> +	unsigned int irq;
>  	u32 cnttidr;
> +	int i, ret;
>  
> -	arch_timers_present |= ARCH_TIMER_TYPE_MEM;
> -	cntctlbase = of_iomap(np, 0);
> +	cntctlbase = ioremap(timer_mem->cntctlbase, timer_mem->size);
>  	if (!cntctlbase) {
> -		pr_err("Can't find CNTCTLBase\n");
> +		pr_err("Can't map CNTCTLBase.\n");
>  		return -ENXIO;
>  	}
>  
> @@ -943,76 +961,110 @@ static int __init arch_timer_mem_init(struct device_node *np)
>  	 * Try to find a virtual capable frame. Otherwise fall back to a
>  	 * physical capable frame.
>  	 */
> -	for_each_available_child_of_node(np, frame) {
> -		int n;
> -		u32 cntacr;
> -
> -		if (of_property_read_u32(frame, "frame-number", &n)) {
> -			pr_err("Missing frame-number\n");
> -			of_node_put(frame);
> -			goto out;
> -		}
> -
> -		/* 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;
> -			arch_timer_mem_use_virtual = true;
> -			break;
> +	for (i = 0; i < timer_mem->num_frames; i++) {
> +		if (is_best_frame(cntctlbase, cnttidr,
> +				  timer_mem->frame[i].frame_nr)) {
> +			best_frame = &timer_mem->frame[i];
> +			if (arch_timer_mem_use_virtual)
> +				break;
>  		}
> -
> -		if (~cntacr & (CNTACR_RWPT | CNTACR_RPCT))
> -			continue;
> -
> -		of_node_put(best_frame);
> -		best_frame = of_node_get(frame);
>  	}
> +	iounmap(cntctlbase);
>  
> -	ret= -ENXIO;
> -	base = arch_counter_base = of_iomap(best_frame, 0);
> -	if (!base) {
> -		pr_err("Can't map frame's registers\n");
> -		goto out;
> +	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",
>  		       arch_timer_mem_use_virtual ? "virt" : "phys");
> -		goto out;
> +		return -EINVAL;
>  	}
>  
> -	/*
> -	 * Try to determine the frequency from the device tree,
> -	 * if fail, get the frequency from CNTFRQ.
> -	 */
> -	if (!arch_timer_rate &&
> -	    of_property_read_u32(np, "clock-frequency", &arch_timer_rate))
> -		arch_timer_detect_rate(base);
> +	base = ioremap(best_frame->cntbase, best_frame->size);
> +	if (!base) {
> +		pr_err("Can't map frame's registers\n");
> +		return -ENXIO;
> +	}
> +
> +	arch_timer_detect_rate(base);
>  
>  	ret = arch_timer_mem_register(base, irq);
> -	if (ret)
> +	if (ret) {
> +		iounmap(base);
> +		return ret;
> +	}
> +
> +	arch_counter_base = base;
> +	arch_timers_present |= ARCH_TIMER_TYPE_MEM;
> +
> +	return 0;
> +}
> +
> +static int __init arch_timer_mem_of_init(struct device_node *np)
> +{
> +	struct arch_timer_mem *timer_mem;
> +	struct device_node *frame_node;
> +	struct resource res;
> +	int i, ret = -EINVAL;
> +
> +	timer_mem = kzalloc(sizeof(*timer_mem), GFP_KERNEL);
> +	if (!timer_mem)
> +		return -ENOMEM;
> +
> +	if (of_address_to_resource(np, 0, &res))
>  		goto out;
> +	timer_mem->cntctlbase = res.start;
> +	timer_mem->size = resource_size(&res);
>  
> -	return arch_timer_common_init();
> +	i = 0;
> +	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 (++i >= ARCH_TIMER_MEM_MAX_FRAMES)
> +			break;
> +	}
> +	timer_mem->num_frames = i;
> +
> +	/* Try to determine the frequency from the device tree */
> +	if (!arch_timer_rate)
> +		of_property_read_u32(np, "clock-frequency", &arch_timer_rate);
> +
> +	ret = arch_timer_mem_init(timer_mem);
> +	if (!ret)
> +		ret = arch_timer_common_init();
>  out:
> -	iounmap(cntctlbase);
> -	of_node_put(best_frame);
> +	kfree(timer_mem);
>  	return ret;
>  }
>  CLOCKSOURCE_OF_DECLARE(armv7_arch_timer_mem, "arm,armv7-timer-mem",
> -		       arch_timer_mem_init);
> +		       arch_timer_mem_of_init);
>  
>  #ifdef CONFIG_ACPI
>  static int __init map_generic_timer_interrupt(u32 interrupt, u32 flags)
> -- 
> 2.7.4
> 

^ permalink raw reply

* [PATCH v16 08/15] clocksource/drivers/arm_arch_timer: Refactor arch_timer_needs_probing, and call it only if acpi disabled.
From: Mark Rutland @ 2016-11-18 19:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479304148-2965-9-git-send-email-fu.wei@linaro.org>

On Wed, Nov 16, 2016 at 09:49:01PM +0800, fu.wei at linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
> 
> The patch refactor original arch_timer_needs_probing function:
>     (1) Separate out arch_timer_needs_probing from arch_timer_common_init,
>     and call it only if acpi disabled.
>     (2) Rename arch_timer_needs_probing to arch_timer_needs_of_probing.

Please write a real commit message.

> Signed-off-by: Fu Wei <fu.wei@linaro.org>
> ---
>  drivers/clocksource/arm_arch_timer.c | 34 +++++++++++++++++++---------------
>  1 file changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index fe4e812..9ddc091 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -792,15 +792,28 @@ static const struct of_device_id arch_timer_mem_of_match[] __initconst = {
>  	{},
>  };
>  
> -static bool __init
> -arch_timer_needs_probing(int type, const struct of_device_id *matches)
> +static bool __init arch_timer_needs_of_probing(void)
>  {
>  	struct device_node *dn;
>  	bool needs_probing = false;
> +	unsigned int mask = ARCH_TIMER_TYPE_CP15 | ARCH_TIMER_TYPE_MEM;
> +
> +	/* We have two timers, and both device-tree nodes are probed. */
> +	if ((arch_timers_present & mask) == mask)
> +		return false;
> +
> +	/*
> +	 * Only one type of timer is probed,
> +	 * check if we have another type of timer node in device-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);
>  
> -	dn = of_find_matching_node(NULL, matches);
> -	if (dn && of_device_is_available(dn) && !(arch_timers_present & type))
> +	if (dn && of_device_is_available(dn))
>  		needs_probing = true;
> +
>  	of_node_put(dn);
>  
>  	return needs_probing;
> @@ -808,17 +821,8 @@ arch_timer_needs_probing(int type, const struct of_device_id *matches)
>  
>  static int __init arch_timer_common_init(void)
>  {
> -	unsigned int mask = 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))
> -			return 0;
> -		if (arch_timer_needs_probing(ARCH_TIMER_TYPE_CP15,
> -					     arch_timer_of_match))
> -			return 0;
> -	}

Why can't we just move this into the DT-specific caller of
arch_timer_common_init()?

Thanks
Mark.

> +	if (acpi_disabled && arch_timer_needs_of_probing())
> +		return 0;
>  
>  	arch_timer_banner(arch_timers_present);
>  	arch_counter_register(arch_timers_present);
> -- 
> 2.7.4
> 

^ permalink raw reply

* [PATCH v3 1/3] dt-bindings: firmware: scm: Add MSM8996 DT bindings
From: Bjorn Andersson @ 2016-11-18 19:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479259165-1601-2-git-send-email-spjoshi@codeaurora.org>

On Tue 15 Nov 17:19 PST 2016, Sarangdhar Joshi wrote:

> Add SCM DT bindings for Qualcomm's MSM8996 platform.
> 
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Sarangdhar Joshi <spjoshi@codeaurora.org>

Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> ---
>  Documentation/devicetree/bindings/firmware/qcom,scm.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.txt b/Documentation/devicetree/bindings/firmware/qcom,scm.txt
> index 3b4436e..20f26fb 100644
> --- a/Documentation/devicetree/bindings/firmware/qcom,scm.txt
> +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.txt
> @@ -10,8 +10,10 @@ Required properties:
>   * "qcom,scm-apq8064" for APQ8064 platforms
>   * "qcom,scm-msm8660" for MSM8660 platforms
>   * "qcom,scm-msm8690" for MSM8690 platforms
> + * "qcom,scm-msm8996" for MSM8996 platforms
>   * "qcom,scm" for later processors (MSM8916, APQ8084, MSM8974, etc)
>  - clocks: One to three clocks may be required based on compatible.
> + * No clock required for "qcom,scm-msm8996"
>   * Only core clock required for "qcom,scm-apq8064", "qcom,scm-msm8660", and "qcom,scm-msm8960"
>   * Core, iface, and bus clocks required for "qcom,scm"
>  - clock-names: Must contain "core" for the core clock, "iface" for the interface
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

^ permalink raw reply

* [PATCH v16 07/15] clocksource/drivers/arm_arch_timer: Refactor arch_timer_detect_rate to keep dt code in *_of_init
From: Mark Rutland @ 2016-11-18 19:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479304148-2965-8-git-send-email-fu.wei@linaro.org>

On Wed, Nov 16, 2016 at 09:49:00PM +0800, fu.wei at 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.

^ permalink raw reply

* [PATCH 2/2] i2c: designware: fix rx fifo depth tracking
From: Russell King @ 2016-11-18 19:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161118193542.GO1041@n2100.armlinux.org.uk>

When loading the TX fifo to receive bytes on the I2C bus, we incorrectly
count the number of bytes:

	rx_limit = dev->rx_fifo_depth - dw_readl(dev, DW_IC_RXFLR);

	while (buf_len > 0 && tx_limit > 0 && rx_limit > 0) {
		if (rx_limit - dev->rx_outstanding <= 0)
			break;
		rx_limit--;
		dev->rx_outstanding++;
	}

DW_IC_RXFLR indicates how many bytes are available to be read in the
FIFO, dev->rx_fifo_depth is the FIFO size, and dev->rx_outstanding is
the number of bytes that we've requested to be read so far, but which
have not been read.

Firstly, increasing dev->rx_outstanding and decreasing rx_limit and then
comparing them results in each byte consuming "two" bytes in this
tracking, so this is obviously wrong.

Secondly, the number of bytes that _could_ be received into the FIFO at
any time is the number of bytes we have so far requested but not yet
read from the FIFO - in other words dev->rx_outstanding.

So, in order to request enough bytes to fill the RX FIFO, we need to
request dev->rx_fifo_depth - dev->rx_outstanding bytes.

Modifying the code thusly results in us reaching the maximum number of
bytes outstanding each time we queue more "receive" operations, provided
the transfer allows that to happen.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
With this applied, I now get:

[    1.726388] i2c_designware 7ffa0000.i2c: transfer terminated early - interrupt latency too high?
[    1.733813] tda998x 0-0070: Error -5 reading from 0x900
[    1.737708] tda998x 0-0070: failed to read edid block 0: -5
[    1.743683] tda998x 0-0070: failed to read EDID

which is a more graceful failure than letting DRM detect the bad
transfer by checking the EDID checksum and hoping that the untransferred
bytes don't result in the EDID checksum succeeding.

 drivers/i2c/busses/i2c-designware-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 11e866d05368..9703fe392543 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -611,7 +611,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 			if (msgs[dev->msg_write_idx].flags & I2C_M_RD) {
 
 				/* avoid rx buffer overrun */
-				if (rx_limit - dev->rx_outstanding <= 0)
+				if (dev->rx_outstanding >= dev->rx_fifo_depth)
 					break;
 
 				dw_writel(dev, cmd | 0x100, DW_IC_DATA_CMD);
-- 
2.7.4

^ permalink raw reply related

* [PATCH 1/2] i2c: designware: report short transfers
From: Russell King @ 2016-11-18 19:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161118193542.GO1041@n2100.armlinux.org.uk>

Rather than reporting success for a short transfer due to interrupt
latency, report an error both to the caller, as well as to the kernel
log.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/i2c/busses/i2c-designware-core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 9703fe392543..c53058d6139c 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -758,7 +758,7 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	}
 
 	/* no error */
-	if (likely(!dev->cmd_err)) {
+	if (likely(!dev->cmd_err && !dev->status)) {
 		ret = num;
 		goto done;
 	}
@@ -768,6 +768,11 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 		ret = i2c_dw_handle_tx_abort(dev);
 		goto done;
 	}
+
+	if (dev->status)
+		dev_err(dev->dev,
+			"transfer terminated early - interrupt latency too high?\n");
+
 	ret = -EIO;
 
 done:
-- 
2.7.4

^ permalink raw reply related

* [BUG] i2c-designware silently fails on long transfers
From: Russell King - ARM Linux @ 2016-11-18 19:35 UTC (permalink / raw)
  To: linux-arm-kernel

With reference to this commit:

commit d39f77b06a712fcba6185a20bb209e357923d980
Author: Andrew Jackson <Andrew.Jackson@arm.com>
Date:   Fri Nov 7 12:10:44 2014 +0000

    i2c: designware: prevent early stop on TX FIFO empty

    If the Designware core is configured with IC_EMPTYFIFO_HOLD_MASTER_EN
    set to zero, allowing the TX FIFO to become empty causes a STOP
    condition to be generated on the I2C bus. If the transmit FIFO
    threshold is set too high, an erroneous STOP condition can be
    generated on long transfers - particularly where the interrupt
    latency is extended.

    Signed-off-by: Andrew Jackson <Andrew.Jackson@arm.com>
    Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
    Tested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
    Signed-off-by: Wolfram Sang <wsa@the-dreams.de>

The TDA998x driver issues long I2C transfers to read the EDID from the
device - and userspace can also issue large transfers too.  However,
if a DW core is configured with IC_EMPTYFIFO_HOLD_MASTER_EN set as
zero, the above commit doesn't seem to solve the problem.  During
boot, with the patch below, I see:

[    1.736549] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x10
[    1.736564] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x510
[    1.736608] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x504
[    1.736799] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x514
[    1.736819] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x510
...
[    1.737986] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x504
[    1.738010] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x514
[    1.738034] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x504
[    1.738039] random: fast init done
[    1.740120] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x714
[    1.740231] i2c_dw_xfer: ffffffc97657b770:1 -> ffffffc97657b770:1 (0:0) [0 0 3 0] 8 [tx:ffffffc976682380:47] [rx:ffffffc9766823c9:55]
[    1.740249] [drm:drm_edid_block_valid] *ERROR* EDID checksum is invalid, remainder is 93
[    1.746979] Raw EDID:
[    1.747934]          00 ff ff ff ff ff ff 00 34 a9 96 a2 01 01 01 01
[    1.752342]          00 17 01 03 80 80 48 78 0a da ff a3 58 4a a2 29
[    1.756748]          17 49 4b 21 08 00 31 40 45 40 61 40 81 80 01 01
[    1.761153]          01 01 01 01 01 01 02 3a 80 d0 72 38 2d 40 10 2c
[    1.765555]          45 80 ba 88 21 00 00 1e 02 00 d0 4e 30 09 12 54
[    1.769958]          01 08 02 00 23 36 01 40 01 05 00 80 a1 4c 4b 49
[    1.774361]          22 00 00 40 03 00 28 00 23 01 20 00 01 88 00 01
[    1.778762]          08 00 00 40 00 02 03 04 0a 00 80 00 02 00 00 40

The significant thing is the "i2c_dw_xfer" line, where I add a print of
the current state.  Here, we can see that the transfer is mid-way, but
a stop condition has been generated by the hardware, leaving 55 bytes
to be received.

Unfortunately, the i2c-designware driver ignores this, and believes that
the transfer completed both fully and successfully, but returns bogus
data to userspace or the kernel driver.  That's really _bad_ behaviour
by the driver - it should at least return an error.

This problem is _soo_ bad that on my Juno, I can't run Xorg (it hits
this every time we try to read the EDID) nor can I boot with the TV
connected (it hits this every boot as well.)

I'd go as far as to say that the i2c-designware hardware, when
configured with this option set to zero, is fundamentally broken for OS
which do not provide any guarantee for interrupt latency, such as Linux.

The commit above tries to mitigate this by reducing the Tx FIFO
threshold, so the interrupt is raised sooner, but that's clearly not
enough for reliable operation.

Another mitigation would be to lower the I2C bus frequency on Juno from
400kHz to 100kHz, so that there's 4x longer IRQ latency possible.
However, even that isn't going to be reliable - even going to 100kHz
isn't going to allow the above case to be solved - the interrupt is
delayed by around 2ms, and it takes about 1.4ms to send/receive 16 bytes
at 100kHz.  (9 * 16 / (100*10^3)).

So, I think all hope is lost for i2c-designware on Juno to cope with
reading the EDID from TDA998x reliably.

I have one patch which solves a problem in the accounting of bytes, and
another to ensure that we return an error for an incomplete transfer,
both will be sent threaded to this mail.

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 11e866d05368..060ae9e5a916 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -752,6 +752,15 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 		goto done;
 	}
 
+	printk(KERN_DEBUG "%s: %p:%d -> %p:%d (%d:%d) [%x %x %x %x] %d [tx:%p:%d] [rx:%p:%d]\n",
+		__func__, msgs, num,
+		dev->msgs, dev->msgs_num,
+		dev->msg_write_idx, dev->msg_read_idx,
+		dev->cmd_err, dev->msg_err, dev->status, dev->abort_source,
+		dev->rx_outstanding,
+		dev->tx_buf, dev->tx_buf_len,
+		dev->rx_buf, dev->rx_buf_len);
+
 	if (dev->msg_err) {
 		ret = dev->msg_err;
 		goto done;
@@ -857,7 +866,7 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
 
 	enabled = dw_readl(dev, DW_IC_ENABLE);
 	stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
-	dev_dbg(dev->dev, "%s: enabled=%#x stat=%#x\n", __func__, enabled, stat);
+	dev_printk(KERN_DEBUG, dev->dev, "%s: enabled=%#x stat=%#x\n", __func__, enabled, stat);
 	if (!enabled || !(stat & ~DW_IC_INTR_ACTIVITY))
 		return IRQ_NONE;
 


-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply related

* [PATCH v16 06/15] clocksource/drivers/arm_arch_timer: separate out arch_timer_uses_ppi init code to prepare for GTDT.
From: Mark Rutland @ 2016-11-18 19:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479304148-2965-7-git-send-email-fu.wei@linaro.org>

On Wed, Nov 16, 2016 at 09:48:59PM +0800, fu.wei at linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
> 
> The patch refactor original arch_timer_uses_ppi init code:
> (1) Extract a subfunction: arch_timer_uses_ppi_init
> (2) Use the new subfunction in arch_timer_of_init and
> arch_timer_acpi_init

This isn't a strict refactoring, since this now assigns
ARCH_TIMER_PHYS_NONSECURE_PPI to arch_timer_uses_ppi, which we didn't do
previously.

As a general note, please write your commit messages as prose rather
than a list of bullet points. Please also explain the rationale for the
change, rather than enumerating the changes. Call out things which are
important and/or likely to surprise reviewers, for example:

* Can 32-bit ARM still use non-secure interrupts afer this change?

* Does the "arm,cpu-registers-not-fw-configured"  proeprty still work?

That will make it vastly easier to have this code reviewed, and it will
be far more helpful for anyone looking at this in future.

For example:

  arm_arch_timer: rework PPI determination

  Currently, the arch timer driver uses ARCH_TIMER_PHYS_SECURE_PPI to
  mean the driver will use the secure PPI *and* potentialy also use the
  non-secure PPI. This is somewhat confusing.

  For arm64, where it never makes sense to use the secure PPI, this
  means we must always request the useless secure PPI, adding to the
  confusion. For ACPI, where we may not even have a valid secure PPI
  number, this is additionally problematic. We need the driver to be
  able to use *only* the non-secure PPI.

  The logic to choose which PPI to use is intertwined with other logic
  in arch_timer_init(). This patch factors the PPI determination out
  into a new function, and then reworks it so that we can handle having
  only a non-secure PPI.

[...]

> +/*
> + * If HYP mode is available, we know that the physical timer
> + * has been configured to be accessible from PL1. Use it, so
> + * that a guest can use the virtual timer instead.
> + *
> + * If no interrupt provided for virtual timer, we'll have to
> + * stick to the physical timer. It'd better be accessible...
> + * On ARM64, we we only use ARCH_TIMER_PHYS_NONSECURE_PPI in Linux.

It would be better to say that for arm64 we never use the secure
interrupt.

> + *
> + * On ARMv8.1 with VH extensions, the kernel runs in HYP. VHE
> + * accesses to CNTP_*_EL1 registers are silently redirected to
> + * their CNTHP_*_EL2 counterparts, and use a different PPI
> + * number.
> + */
> +static int __init arch_timer_uses_ppi_init(void)

It would be better to call this something like arch_timer_select_ppi().
As it stands, the name is difficult to read.

> @@ -902,6 +904,10 @@ static int __init arch_timer_of_init(struct device_node *np)
>  	    of_property_read_bool(np, "arm,cpu-registers-not-fw-configured"))
>  		arch_timer_uses_ppi = ARCH_TIMER_PHYS_SECURE_PPI;
>  
> +	ret = arch_timer_uses_ppi_init();
> +	if (ret)
> +		return ret;

This is clearly broken if you consider what the statement above is
doing.

Thanks,
Mark.

^ permalink raw reply

* [PATCHv3 6/6] arm64: Add support for CONFIG_DEBUG_VIRTUAL
From: Laura Abbott @ 2016-11-18 19:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161118190531.GJ1197@leverpostej>

On 11/18/2016 11:05 AM, Mark Rutland wrote:
> On Fri, Nov 18, 2016 at 10:42:56AM -0800, Laura Abbott wrote:
>> On 11/18/2016 09:53 AM, Mark Rutland wrote:
>>> On Thu, Nov 17, 2016 at 05:16:56PM -0800, Laura Abbott wrote:
> 
>>>> +#define __virt_to_phys_nodebug(x) ({					\
>>>>  	phys_addr_t __x = (phys_addr_t)(x);				\
>>>> -	__x & BIT(VA_BITS - 1) ? (__x & ~PAGE_OFFSET) + PHYS_OFFSET :	\
>>>> -				 (__x - kimage_voffset); })
>>>> +	((__x & ~PAGE_OFFSET) + PHYS_OFFSET);				\
>>>> +})
>>>
>>> Given the KASAN failure, and the strong possibility that there's even
>>> more stuff lurking in common code, I think we should retain the logic to
>>> handle kernel image addresses for the timebeing (as x86 does). Once
>>> we've merged DEBUG_VIRTUAL, it will be easier to track those down.
>>
>> Agreed. I might see about adding another option DEBUG_STRICT_VIRTUAL
>> for catching bad __pa vs __pa_symbol usage and keep DEBUG_VIRTUAL for
>> catching addresses that will work in neither case.
> 
> I think it makes sense for DEBUG_VIRTUAL to do both, so long as the
> default behaviour (and fallback after a WARN for virt_to_phys()) matches
> what we currently do. We'll get useful diagnostics, but a graceful
> fallback.
> 

I was suggesting making the WARN optional for having this be more useful
before all the __pa_symbol stuff gets cleaned up. Maybe the WARN won't
actually be a hindrance.

Thanks,
Laura

> I think the helpers I suggested below do that?  Or have I misunderstood,
> and you mean something stricter (e.g. checking whether a lm address is
> is backed by something)?
> 
>>> phys_addr_t __virt_to_phys(unsigned long x)
>>> {
>>> 	WARN(!__is_lm_address(x),
>>> 	     "virt_to_phys() used for non-linear address: %pK\n",
>>> 	     (void*)x);
>>> 	
>>> 	return __virt_to_phys_nodebug(x);
>>> }
>>> EXPORT_SYMBOL(__virt_to_phys);
> 
>>> phys_addr_t __phys_addr_symbol(unsigned long x)
>>> {
>>> 	/*
>>> 	 * This is bounds checking against the kernel image only.
>>> 	 * __pa_symbol should only be used on kernel symbol addresses.
>>> 	 */
>>> 	VIRTUAL_BUG_ON(x < (unsigned long) KERNEL_START ||
>>> 		       x > (unsigned long) KERNEL_END);
>>>
>>> 	return __pa_symbol_nodebug(x);
>>> }
>>> EXPORT_SYMBOL(__phys_addr_symbol);
> 
> Thanks,
> Mark.
> 

^ permalink raw reply

* [PATCHv3 6/6] arm64: Add support for CONFIG_DEBUG_VIRTUAL
From: Mark Rutland @ 2016-11-18 19:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <16e4b3da-c552-252d-108a-0681b71b12ef@redhat.com>

On Fri, Nov 18, 2016 at 10:42:56AM -0800, Laura Abbott wrote:
> On 11/18/2016 09:53 AM, Mark Rutland wrote:
> > On Thu, Nov 17, 2016 at 05:16:56PM -0800, Laura Abbott wrote:

> >> +#define __virt_to_phys_nodebug(x) ({					\
> >>  	phys_addr_t __x = (phys_addr_t)(x);				\
> >> -	__x & BIT(VA_BITS - 1) ? (__x & ~PAGE_OFFSET) + PHYS_OFFSET :	\
> >> -				 (__x - kimage_voffset); })
> >> +	((__x & ~PAGE_OFFSET) + PHYS_OFFSET);				\
> >> +})
> > 
> > Given the KASAN failure, and the strong possibility that there's even
> > more stuff lurking in common code, I think we should retain the logic to
> > handle kernel image addresses for the timebeing (as x86 does). Once
> > we've merged DEBUG_VIRTUAL, it will be easier to track those down.
> 
> Agreed. I might see about adding another option DEBUG_STRICT_VIRTUAL
> for catching bad __pa vs __pa_symbol usage and keep DEBUG_VIRTUAL for
> catching addresses that will work in neither case.

I think it makes sense for DEBUG_VIRTUAL to do both, so long as the
default behaviour (and fallback after a WARN for virt_to_phys()) matches
what we currently do. We'll get useful diagnostics, but a graceful
fallback.

I think the helpers I suggested below do that?  Or have I misunderstood,
and you mean something stricter (e.g. checking whether a lm address is
is backed by something)?

> > phys_addr_t __virt_to_phys(unsigned long x)
> > {
> > 	WARN(!__is_lm_address(x),
> > 	     "virt_to_phys() used for non-linear address: %pK\n",
> > 	     (void*)x);
> > 	
> > 	return __virt_to_phys_nodebug(x);
> > }
> > EXPORT_SYMBOL(__virt_to_phys);

> > phys_addr_t __phys_addr_symbol(unsigned long x)
> > {
> > 	/*
> > 	 * This is bounds checking against the kernel image only.
> > 	 * __pa_symbol should only be used on kernel symbol addresses.
> > 	 */
> > 	VIRTUAL_BUG_ON(x < (unsigned long) KERNEL_START ||
> > 		       x > (unsigned long) KERNEL_END);
> > 
> > 	return __pa_symbol_nodebug(x);
> > }
> > EXPORT_SYMBOL(__phys_addr_symbol);

Thanks,
Mark.

^ permalink raw reply

* [PATCH] mfd: twl-core: make driver DT only
From: Lee Jones @ 2016-11-18 19:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478952153-26592-1-git-send-email-Nicolae_Rosia@mentor.com>

On Sat, 12 Nov 2016, Nicolae Rosia wrote:

> All users are DT-only and it makes no sense to keep
> unused code
> 
> Signed-off-by: Nicolae Rosia <Nicolae_Rosia@mentor.com>
> ---
>  drivers/mfd/Kconfig    |   1 +
>  drivers/mfd/twl-core.c | 395 ++-----------------------------------------------
>  2 files changed, 10 insertions(+), 386 deletions(-)

I think it would be courteous to add some of the serious contributors
to the review list.

There may still be some out of tree users who might wish for a
heads-up that this change is about to be merged.

> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index c6df644..c180f8b 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1333,6 +1333,7 @@ config MFD_TPS80031
>  config TWL4030_CORE
>  	bool "TI TWL4030/TWL5030/TWL6030/TPS659x0 Support"
>  	depends on I2C=y
> +	depends on OF
>  	select IRQ_DOMAIN
>  	select REGMAP_I2C
>  	help
> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
> index c64615d..2025326 100644
> --- a/drivers/mfd/twl-core.c
> +++ b/drivers/mfd/twl-core.c
> @@ -13,6 +13,9 @@
>   * Code cleanup and modifications to IRQ handler.
>   * by syed khasim <x0khasim@ti.com>
>   *
> + * Code cleanup and modifications:
> + * Copyright (C) 2016 Nicolae Rosia <nicolae.rosia@gmail.com>
> + *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
>   * the Free Software Foundation; either version 2 of the License, or
> @@ -604,376 +607,6 @@ int twl_get_hfclk_rate(void)
>  }
>  EXPORT_SYMBOL_GPL(twl_get_hfclk_rate);
>  
> -static struct device *
> -add_numbered_child(unsigned mod_no, const char *name, int num,
> -		void *pdata, unsigned pdata_len,
> -		bool can_wakeup, int irq0, int irq1)
> -{
> -	struct platform_device	*pdev;
> -	struct twl_client	*twl;
> -	int			status, sid;
> -
> -	if (unlikely(mod_no >= twl_get_last_module())) {
> -		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
> -		return ERR_PTR(-EPERM);
> -	}
> -	sid = twl_priv->twl_map[mod_no].sid;
> -	twl = &twl_priv->twl_modules[sid];
> -
> -	pdev = platform_device_alloc(name, num);
> -	if (!pdev)
> -		return ERR_PTR(-ENOMEM);
> -
> -	pdev->dev.parent = &twl->client->dev;
> -
> -	if (pdata) {
> -		status = platform_device_add_data(pdev, pdata, pdata_len);
> -		if (status < 0) {
> -			dev_dbg(&pdev->dev, "can't add platform_data\n");
> -			goto put_device;
> -		}
> -	}
> -
> -	if (irq0) {
> -		struct resource r[2] = {
> -			{ .start = irq0, .flags = IORESOURCE_IRQ, },
> -			{ .start = irq1, .flags = IORESOURCE_IRQ, },
> -		};
> -
> -		status = platform_device_add_resources(pdev, r, irq1 ? 2 : 1);
> -		if (status < 0) {
> -			dev_dbg(&pdev->dev, "can't add irqs\n");
> -			goto put_device;
> -		}
> -	}
> -
> -	status = platform_device_add(pdev);
> -	if (status)
> -		goto put_device;
> -
> -	device_init_wakeup(&pdev->dev, can_wakeup);
> -
> -	return &pdev->dev;
> -
> -put_device:
> -	platform_device_put(pdev);
> -	dev_err(&twl->client->dev, "failed to add device %s\n", name);
> -	return ERR_PTR(status);
> -}
> -
> -static inline struct device *add_child(unsigned mod_no, const char *name,
> -		void *pdata, unsigned pdata_len,
> -		bool can_wakeup, int irq0, int irq1)
> -{
> -	return add_numbered_child(mod_no, name, -1, pdata, pdata_len,
> -		can_wakeup, irq0, irq1);
> -}
> -
> -static struct device *
> -add_regulator_linked(int num, struct regulator_init_data *pdata,
> -		struct regulator_consumer_supply *consumers,
> -		unsigned num_consumers, unsigned long features)
> -{
> -	struct twl_regulator_driver_data drv_data;
> -
> -	/* regulator framework demands init_data ... */
> -	if (!pdata)
> -		return NULL;
> -
> -	if (consumers) {
> -		pdata->consumer_supplies = consumers;
> -		pdata->num_consumer_supplies = num_consumers;
> -	}
> -
> -	if (pdata->driver_data) {
> -		/* If we have existing drv_data, just add the flags */
> -		struct twl_regulator_driver_data *tmp;
> -		tmp = pdata->driver_data;
> -		tmp->features |= features;
> -	} else {
> -		/* add new driver data struct, used only during init */
> -		drv_data.features = features;
> -		drv_data.set_voltage = NULL;
> -		drv_data.get_voltage = NULL;
> -		drv_data.data = NULL;
> -		pdata->driver_data = &drv_data;
> -	}
> -
> -	/* NOTE:  we currently ignore regulator IRQs, e.g. for short circuits */
> -	return add_numbered_child(TWL_MODULE_PM_MASTER, "twl_reg", num,
> -		pdata, sizeof(*pdata), false, 0, 0);
> -}
> -
> -static struct device *
> -add_regulator(int num, struct regulator_init_data *pdata,
> -		unsigned long features)
> -{
> -	return add_regulator_linked(num, pdata, NULL, 0, features);
> -}
> -
> -/*
> - * NOTE:  We know the first 8 IRQs after pdata->base_irq are
> - * for the PIH, and the next are for the PWR_INT SIH, since
> - * that's how twl_init_irq() sets things up.
> - */
> -
> -static int
> -add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
> -		unsigned long features)
> -{
> -	struct device	*child;
> -
> -	if (IS_ENABLED(CONFIG_GPIO_TWL4030) && pdata->gpio) {
> -		child = add_child(TWL4030_MODULE_GPIO, "twl4030_gpio",
> -				pdata->gpio, sizeof(*pdata->gpio),
> -				false, irq_base + GPIO_INTR_OFFSET, 0);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -	}
> -
> -	if (IS_ENABLED(CONFIG_KEYBOARD_TWL4030) && pdata->keypad) {
> -		child = add_child(TWL4030_MODULE_KEYPAD, "twl4030_keypad",
> -				pdata->keypad, sizeof(*pdata->keypad),
> -				true, irq_base + KEYPAD_INTR_OFFSET, 0);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -	}
> -
> -	if (IS_ENABLED(CONFIG_TWL4030_MADC) && pdata->madc &&
> -	    twl_class_is_4030()) {
> -		child = add_child(TWL4030_MODULE_MADC, "twl4030_madc",
> -				pdata->madc, sizeof(*pdata->madc),
> -				true, irq_base + MADC_INTR_OFFSET, 0);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -	}
> -
> -	if (IS_ENABLED(CONFIG_RTC_DRV_TWL4030)) {
> -		/*
> -		 * REVISIT platform_data here currently might expose the
> -		 * "msecure" line ... but for now we just expect board
> -		 * setup to tell the chip "it's always ok to SET_TIME".
> -		 * Eventually, Linux might become more aware of such
> -		 * HW security concerns, and "least privilege".
> -		 */
> -		child = add_child(TWL_MODULE_RTC, "twl_rtc", NULL, 0,
> -				true, irq_base + RTC_INTR_OFFSET, 0);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -	}
> -
> -	if (IS_ENABLED(CONFIG_PWM_TWL)) {
> -		child = add_child(TWL_MODULE_PWM, "twl-pwm", NULL, 0,
> -				  false, 0, 0);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -	}
> -
> -	if (IS_ENABLED(CONFIG_PWM_TWL_LED)) {
> -		child = add_child(TWL_MODULE_LED, "twl-pwmled", NULL, 0,
> -				  false, 0, 0);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -	}
> -
> -	if (IS_ENABLED(CONFIG_TWL4030_USB) && pdata->usb &&
> -	    twl_class_is_4030()) {
> -
> -		static struct regulator_consumer_supply usb1v5 = {
> -			.supply =	"usb1v5",
> -		};
> -		static struct regulator_consumer_supply usb1v8 = {
> -			.supply =	"usb1v8",
> -		};
> -		static struct regulator_consumer_supply usb3v1 = {
> -			.supply =	"usb3v1",
> -		};
> -
> -	/* First add the regulators so that they can be used by transceiver */
> -		if (IS_ENABLED(CONFIG_REGULATOR_TWL4030)) {
> -			/* this is a template that gets copied */
> -			struct regulator_init_data usb_fixed = {
> -				.constraints.valid_modes_mask =
> -					REGULATOR_MODE_NORMAL
> -					| REGULATOR_MODE_STANDBY,
> -				.constraints.valid_ops_mask =
> -					REGULATOR_CHANGE_MODE
> -					| REGULATOR_CHANGE_STATUS,
> -			};
> -
> -			child = add_regulator_linked(TWL4030_REG_VUSB1V5,
> -						      &usb_fixed, &usb1v5, 1,
> -						      features);
> -			if (IS_ERR(child))
> -				return PTR_ERR(child);
> -
> -			child = add_regulator_linked(TWL4030_REG_VUSB1V8,
> -						      &usb_fixed, &usb1v8, 1,
> -						      features);
> -			if (IS_ERR(child))
> -				return PTR_ERR(child);
> -
> -			child = add_regulator_linked(TWL4030_REG_VUSB3V1,
> -						      &usb_fixed, &usb3v1, 1,
> -						      features);
> -			if (IS_ERR(child))
> -				return PTR_ERR(child);
> -
> -		}
> -
> -		child = add_child(TWL_MODULE_USB, "twl4030_usb",
> -				pdata->usb, sizeof(*pdata->usb), true,
> -				/* irq0 = USB_PRES, irq1 = USB */
> -				irq_base + USB_PRES_INTR_OFFSET,
> -				irq_base + USB_INTR_OFFSET);
> -
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -
> -		/* we need to connect regulators to this transceiver */
> -		if (IS_ENABLED(CONFIG_REGULATOR_TWL4030) && child) {
> -			usb1v5.dev_name = dev_name(child);
> -			usb1v8.dev_name = dev_name(child);
> -			usb3v1.dev_name = dev_name(child);
> -		}
> -	}
> -
> -	if (IS_ENABLED(CONFIG_TWL4030_WATCHDOG) && twl_class_is_4030()) {
> -		child = add_child(TWL_MODULE_PM_RECEIVER, "twl4030_wdt", NULL,
> -				  0, false, 0, 0);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -	}
> -
> -	if (IS_ENABLED(CONFIG_INPUT_TWL4030_PWRBUTTON) && twl_class_is_4030()) {
> -		child = add_child(TWL_MODULE_PM_MASTER, "twl4030_pwrbutton",
> -				  NULL, 0, true, irq_base + 8 + 0, 0);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -	}
> -
> -	if (IS_ENABLED(CONFIG_MFD_TWL4030_AUDIO) && pdata->audio &&
> -	    twl_class_is_4030()) {
> -		child = add_child(TWL4030_MODULE_AUDIO_VOICE, "twl4030-audio",
> -				pdata->audio, sizeof(*pdata->audio),
> -				false, 0, 0);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -	}
> -
> -	/* twl4030 regulators */
> -	if (IS_ENABLED(CONFIG_REGULATOR_TWL4030) && twl_class_is_4030()) {
> -		child = add_regulator(TWL4030_REG_VPLL1, pdata->vpll1,
> -					features);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -
> -		child = add_regulator(TWL4030_REG_VIO, pdata->vio,
> -					features);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -
> -		child = add_regulator(TWL4030_REG_VDD1, pdata->vdd1,
> -					features);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -
> -		child = add_regulator(TWL4030_REG_VDD2, pdata->vdd2,
> -					features);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -
> -		child = add_regulator(TWL4030_REG_VMMC1, pdata->vmmc1,
> -					features);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -
> -		child = add_regulator(TWL4030_REG_VDAC, pdata->vdac,
> -					features);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -
> -		child = add_regulator((features & TWL4030_VAUX2)
> -					? TWL4030_REG_VAUX2_4030
> -					: TWL4030_REG_VAUX2,
> -				pdata->vaux2, features);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -
> -		child = add_regulator(TWL4030_REG_VINTANA1, pdata->vintana1,
> -					features);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -
> -		child = add_regulator(TWL4030_REG_VINTANA2, pdata->vintana2,
> -					features);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -
> -		child = add_regulator(TWL4030_REG_VINTDIG, pdata->vintdig,
> -					features);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -	}
> -
> -	/* maybe add LDOs that are omitted on cost-reduced parts */
> -	if (IS_ENABLED(CONFIG_REGULATOR_TWL4030) && !(features & TPS_SUBSET)
> -	  && twl_class_is_4030()) {
> -		child = add_regulator(TWL4030_REG_VPLL2, pdata->vpll2,
> -					features);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -
> -		child = add_regulator(TWL4030_REG_VMMC2, pdata->vmmc2,
> -					features);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -
> -		child = add_regulator(TWL4030_REG_VSIM, pdata->vsim,
> -					features);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -
> -		child = add_regulator(TWL4030_REG_VAUX1, pdata->vaux1,
> -					features);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -
> -		child = add_regulator(TWL4030_REG_VAUX3, pdata->vaux3,
> -					features);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -
> -		child = add_regulator(TWL4030_REG_VAUX4, pdata->vaux4,
> -					features);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -	}
> -
> -	if (IS_ENABLED(CONFIG_CHARGER_TWL4030) && pdata->bci &&
> -			!(features & (TPS_SUBSET | TWL5031))) {
> -		child = add_child(TWL_MODULE_MAIN_CHARGE, "twl4030_bci",
> -				pdata->bci, sizeof(*pdata->bci), false,
> -				/* irq0 = CHG_PRES, irq1 = BCI */
> -				irq_base + BCI_PRES_INTR_OFFSET,
> -				irq_base + BCI_INTR_OFFSET);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -	}
> -
> -	if (IS_ENABLED(CONFIG_TWL4030_POWER) && pdata->power) {
> -		child = add_child(TWL_MODULE_PM_MASTER, "twl4030_power",
> -				  pdata->power, sizeof(*pdata->power), false,
> -				  0, 0);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -	}
> -
> -	return 0;
> -}
> -
> -/*----------------------------------------------------------------------*/
> -
>  /*
>   * These three functions initialize the on-chip clock framework,
>   * letting it generate the right frequencies for USB, MADC, and
> @@ -1000,8 +633,7 @@ static inline int __init unprotect_pm_master(void)
>  	return e;
>  }
>  
> -static void clocks_init(struct device *dev,
> -			struct twl4030_clock_init_data *clock)
> +static void clocks_init(struct device *dev)
>  {
>  	int e = 0;
>  	struct clk *osc;
> @@ -1031,8 +663,6 @@ static void clocks_init(struct device *dev,
>  	}
>  
>  	ctrl |= HIGH_PERF_SQ;
> -	if (clock && clock->ck32k_lowpwr_enable)
> -		ctrl |= CK32K_LOWPWR_EN;
>  
>  	e |= unprotect_pm_master();
>  	/* effect->MADC+USB ck en */
> @@ -1080,7 +710,6 @@ static struct of_dev_auxdata twl_auxdata_lookup[] = {
>  static int
>  twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  {
> -	struct twl4030_platform_data	*pdata = dev_get_platdata(&client->dev);
>  	struct device_node		*node = client->dev.of_node;
>  	struct platform_device		*pdev;
>  	const struct regmap_config	*twl_regmap_config;
> @@ -1088,8 +717,8 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  	int				status;
>  	unsigned			i, num_slaves;
>  
> -	if (!node && !pdata) {
> -		dev_err(&client->dev, "no platform data\n");
> +	if (!node) {
> +		dev_err(&client->dev, "no DT info\n");
>  		return -EINVAL;
>  	}
>  
> @@ -1177,7 +806,7 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  	twl_priv->ready = true;
>  
>  	/* setup clock framework */
> -	clocks_init(&pdev->dev, pdata ? pdata->clock : NULL);
> +	clocks_init(&pdev->dev);
>  
>  	/* read TWL IDCODE Register */
>  	if (twl_class_is_4030()) {
> @@ -1225,14 +854,8 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  				 TWL4030_DCDC_GLOBAL_CFG);
>  	}
>  
> -	if (node) {
> -		if (pdata)
> -			twl_auxdata_lookup[0].platform_data = pdata->gpio;
> -		status = of_platform_populate(node, NULL, twl_auxdata_lookup,
> -					      &client->dev);
> -	} else {
> -		status = add_children(pdata, irq_base, id->driver_data);
> -	}
> +	status = of_platform_populate(node, NULL, twl_auxdata_lookup,
> +					&client->dev);
>  
>  fail:
>  	if (status < 0)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [GIT PULL 4/4] bcm2835-defconfig-64-next-2016-11-18
From: Eric Anholt @ 2016-11-18 18:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161118185835.14452-1-eric@anholt.net>

  Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)

are available in the git repository at:

  https://github.com/anholt/linux tags/bcm2835-defconfig-64-next-2016-11-18

for you to fetch changes up to ac178e4280e65f4d0d14b13a7bfec3a43ff90e66:

  ARM64: bcm2835: add thermal driver to default config (2016-11-11 09:00:00 -0800)

----------------------------------------------------------------
This pull enables the BCM2837 (Pi 3) thermal driver in the defconfig.

----------------------------------------------------------------
Martin Sperl (1):
      ARM64: bcm2835: add thermal driver to default config

 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

^ permalink raw reply

* [GIT PULL 3/4] bcm2835-defconfig-next-2016-11-18
From: Eric Anholt @ 2016-11-18 18:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161118185835.14452-1-eric@anholt.net>

  Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)

are available in the git repository at:

  https://github.com/anholt/linux tags/bcm2835-defconfig-next-2016-11-18

for you to fetch changes up to bab0cb90550467c71f4e1b73da406a2280c4f418:

  ARM: bcm2835: add thermal driver to default config (2016-11-11 09:00:37 -0800)

----------------------------------------------------------------
This pull request enables the BCM2835 (Raspberry Pi) thermal driver in
the Pi1 defconfig.

----------------------------------------------------------------
Martin Sperl (1):
      ARM: bcm2835: add thermal driver to default config

 arch/arm/configs/bcm2835_defconfig | 2 ++
 1 file changed, 2 insertions(+)

^ permalink raw reply

* [GIT PULL 2/4] bcm2835-dt-64-next-2016-11-18
From: Eric Anholt @ 2016-11-18 18:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161118185835.14452-1-eric@anholt.net>

  Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)

are available in the git repository at:

  https://github.com/anholt/linux tags/bcm2835-dt-64-next-2016-11-18

for you to fetch changes up to a44e87b47148c6ee6b78509f47e6a15c0fae890a:

  ARM64: dts: bcm2837-rpi-3-b: remove incorrect pwr LED (2016-11-16 13:49:38 -0800)

----------------------------------------------------------------
This pull request brings thermal support to the BCM2837 DT, and a few
other fixes.

In order to get the thermal node that we're adjusting the compatible
string on, we have to merge in the bcm2835-dt-next branch.

----------------------------------------------------------------
Andrea Merello (1):
      ARM64: dts: bcm2837-rpi-3-b: remove incorrect pwr LED

Andreas F?rber (1):
      ARM64: dts: bcm2835: Fix bcm2837 compatible string

Eric Anholt (2):
      ARM: dts: bcm283x: Define standard pinctrl groups in the gpio node.
      Merge branch 'bcm2835-dt-next' into bcm2835-dt-64-next

Gerd Hoffmann (6):
      pinctrl: bcm2835: add pull defines to dt bindings
      ARM: dts: bcm283x: add pinctrl group to &pwm, drop pins from &gpio
      ARM: dts: bcm283x: add pinctrl group to &i2c0, drop pins from &gpio
      ARM: dts: bcm283x: add pinctrl group to &i2c1, drop pins from &gpio
      ARM: dts: bcm283x: add pinctrl group to &sdhci, drop pins from &gpio
      ARM: dts: bcm283x: drop alt3 from &gpio

Linus Walleij (1):
      ARM: bcm2835: Add names for the Raspberry Pi GPIO lines

Martin Sperl (3):
      dt: bindings: add thermal device driver for bcm2835
      ARM: bcm2835: dts: add thermal node to device-tree of bcm283x
      ARM64: bcm2835: dts: add thermal node to device-tree of bcm2837

Stefan Wahren (3):
      ARM64: dts: bcm283x: Use dtsi for USB host mode
      DT: binding: bcm2835-mbox: fix address typo in example
      ARM: dts: bcm283x: fix typo in mailbox address

 .../bindings/mailbox/brcm,bcm2835-mbox.txt         |   2 +-
 .../bindings/thermal/brcm,bcm2835-thermal.txt      |  17 ++
 arch/arm/boot/dts/bcm2835-rpi-a-plus.dts           |  67 ++++++-
 arch/arm/boot/dts/bcm2835-rpi-a.dts                |  69 ++++++-
 arch/arm/boot/dts/bcm2835-rpi-b-plus.dts           |  68 ++++++-
 arch/arm/boot/dts/bcm2835-rpi-b-rev2.dts           |  68 ++++++-
 arch/arm/boot/dts/bcm2835-rpi-b.dts                |  69 ++++++-
 arch/arm/boot/dts/bcm2835-rpi-zero.dts             |   2 +-
 arch/arm/boot/dts/bcm2835-rpi.dtsi                 |  15 +-
 arch/arm/boot/dts/bcm2835.dtsi                     |   6 +
 arch/arm/boot/dts/bcm2836-rpi-2-b.dts              |   2 +-
 arch/arm/boot/dts/bcm2836.dtsi                     |   6 +
 arch/arm/boot/dts/bcm283x.dtsi                     | 212 ++++++++++++++++++++-
 arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts   |   8 +-
 arch/arm64/boot/dts/broadcom/bcm2837.dtsi          |   8 +-
 .../boot/dts/broadcom/bcm283x-rpi-usb-host.dtsi    |   1 +
 drivers/pinctrl/bcm/pinctrl-bcm2835.c              |   6 -
 include/dt-bindings/pinctrl/bcm2835.h              |   5 +
 18 files changed, 602 insertions(+), 29 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/thermal/brcm,bcm2835-thermal.txt
 create mode 120000 arch/arm64/boot/dts/broadcom/bcm283x-rpi-usb-host.dtsi

^ permalink raw reply

* [GIT PULL 1/4] bcm2835-dt-next-2016-11-18
From: Eric Anholt @ 2016-11-18 18:58 UTC (permalink / raw)
  To: linux-arm-kernel

  Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)

are available in the git repository at:

  https://github.com/anholt/linux tags/bcm2835-dt-next-2016-11-18

for you to fetch changes up to 3a1689ea752436917c5ce4487527ed6c444630ee:

  ARM: bcm2835: Add names for the RPi Zero GPIO lines (2016-11-16 13:54:36 -0800)

----------------------------------------------------------------
This pull request brings in DT changes for BCM2835: pinctrl setup
cleanups, GPIO line naming, and the node for the new thermal driver.

----------------------------------------------------------------
Eric Anholt (1):
      ARM: dts: bcm283x: Define standard pinctrl groups in the gpio node.

Gerd Hoffmann (6):
      pinctrl: bcm2835: add pull defines to dt bindings
      ARM: dts: bcm283x: add pinctrl group to &pwm, drop pins from &gpio
      ARM: dts: bcm283x: add pinctrl group to &i2c0, drop pins from &gpio
      ARM: dts: bcm283x: add pinctrl group to &i2c1, drop pins from &gpio
      ARM: dts: bcm283x: add pinctrl group to &sdhci, drop pins from &gpio
      ARM: dts: bcm283x: drop alt3 from &gpio

Linus Walleij (1):
      ARM: bcm2835: Add names for the Raspberry Pi GPIO lines

Martin Sperl (2):
      dt: bindings: add thermal device driver for bcm2835
      ARM: bcm2835: dts: add thermal node to device-tree of bcm283x

Stefan Wahren (4):
      DT: binding: bcm2835-mbox: fix address typo in example
      ARM: dts: bcm283x: fix typo in mailbox address
      ARM: bcm2835: Fix names for the Raspberry Pi GPIO lines
      ARM: bcm2835: Add names for the RPi Zero GPIO lines

 .../bindings/mailbox/brcm,bcm2835-mbox.txt         |   2 +-
 .../bindings/thermal/brcm,bcm2835-thermal.txt      |  17 ++
 arch/arm/boot/dts/bcm2835-rpi-a-plus.dts           |  67 ++++++-
 arch/arm/boot/dts/bcm2835-rpi-a.dts                |  69 ++++++-
 arch/arm/boot/dts/bcm2835-rpi-b-plus.dts           |  68 ++++++-
 arch/arm/boot/dts/bcm2835-rpi-b-rev2.dts           |  68 ++++++-
 arch/arm/boot/dts/bcm2835-rpi-b.dts                |  69 ++++++-
 arch/arm/boot/dts/bcm2835-rpi-zero.dts             |  67 ++++++-
 arch/arm/boot/dts/bcm2835-rpi.dtsi                 |  15 +-
 arch/arm/boot/dts/bcm2835.dtsi                     |   6 +
 arch/arm/boot/dts/bcm2836-rpi-2-b.dts              |   2 +-
 arch/arm/boot/dts/bcm2836.dtsi                     |   6 +
 arch/arm/boot/dts/bcm283x.dtsi                     | 212 ++++++++++++++++++++-
 drivers/pinctrl/bcm/pinctrl-bcm2835.c              |   6 -
 include/dt-bindings/pinctrl/bcm2835.h              |   5 +
 15 files changed, 658 insertions(+), 21 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/thermal/brcm,bcm2835-thermal.txt

^ permalink raw reply

* [GIT PULL] add #pinctrl-cells for v4.10
From: Tony Lindgren @ 2016-11-18 18:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161118175629.GC8882@localhost>

* Olof Johansson <olof@lixom.net> [161118 10:49]:
> Hi,
> 
> On Mon, Nov 14, 2016 at 04:34:19PM -0800, Tony Lindgren wrote:
> > The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
> > 
> >   Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
> > 
> > are available in the git repository at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap tags/omap-for-v4.10/pinctrl-cells-signed
> > 
> > for you to fetch changes up to be76fd3197df608e1b010bf5ab90377205f54344:
> > 
> >   ARM: dts: Add #pinctrl-cells for pinctrl-single instances (2016-11-07 08:27:49 -0700)
> > 
> > ----------------------------------------------------------------
> > Add #pinctrl-cells for pinctrl-single using dts files. This allows
> > us to use generic parser later on. Note that the driver supports
> > handling the legacy binding also with no #pinctrl-cells so these
> > changes can be queued separately from the driver changes.
> 
> You forgot to make one commit per file to maximize your patch contribution
> count! :-)

Heh I've got plenty of churn commits already :)

> Merged, but note that we normally keep arm64 and arm DT updates
> separate. I'm gambling on not seeing any conflicts on the hi6220 dtsi
> this time around though, and took it in as-is.

Oh right, sorry about that. I'll split n * arch ways next time.

Regards,

Tony

^ permalink raw reply

* [PATCH] mfd: twl-core: export twl_get_regmap
From: Lee Jones @ 2016-11-18 18:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478950693-13959-1-git-send-email-Nicolae_Rosia@mentor.com>

On Sat, 12 Nov 2016, Nicolae Rosia wrote:

> We want to get rid of global twl_i2c_{write/read}.
> As a first step, allow clients to get the regmap and write directly

What's stopping you from passing it through device data?

> Signed-off-by: Nicolae Rosia <Nicolae_Rosia@mentor.com>
> ---
>  drivers/mfd/twl-core.c  | 3 ++-
>  include/linux/i2c/twl.h | 2 ++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
> index c64615d..49e6a4b 100644
> --- a/drivers/mfd/twl-core.c
> +++ b/drivers/mfd/twl-core.c
> @@ -421,7 +421,7 @@ EXPORT_SYMBOL(twl_rev);
>   *
>   * Returns the regmap pointer or NULL in case of failure.
>   */
> -static struct regmap *twl_get_regmap(u8 mod_no)
> +struct regmap *twl_get_regmap(u8 mod_no)
>  {
>  	int sid;
>  	struct twl_client *twl;
> @@ -440,6 +440,7 @@ static struct regmap *twl_get_regmap(u8 mod_no)
>  
>  	return twl->regmap;
>  }
> +EXPORT_SYMBOL(twl_get_regmap);
>  
>  /**
>   * twl_i2c_write - Writes a n bit register in TWL4030/TWL5030/TWL60X0
> diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h
> index 9ad7828..4c43cdb3 100644
> --- a/include/linux/i2c/twl.h
> +++ b/include/linux/i2c/twl.h
> @@ -174,6 +174,8 @@ static inline int twl_class_is_ ##class(void)	\
>  TWL_CLASS_IS(4030, TWL4030_CLASS_ID)
>  TWL_CLASS_IS(6030, TWL6030_CLASS_ID)
>  
> +struct regmap *twl_get_regmap(u8 mod_no);
> +
>  /* Set the regcache bypass for the regmap associated with the nodule */
>  int twl_set_regcache_bypass(u8 mod_no, bool enable);
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH v16 05/15] clocksource/drivers/arm_arch_timer: fix a bug in arch_timer_register about arch_timer_uses_ppi
From: Mark Rutland @ 2016-11-18 18:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479304148-2965-6-git-send-email-fu.wei@linaro.org>

On Wed, Nov 16, 2016 at 09:48:58PM +0800, fu.wei at linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
> 
> The patch fix a potential bug about arch_timer_uses_ppi in
> arch_timer_register.
> On ARM64, we don't use ARCH_TIMER_PHYS_SECURE_PPI in Linux, so we will
> just igorne it in init code. 

That's not currently the case. I assume you mean we will in later
patches? If so, please make that clear in the commit message.

> If arch_timer_uses_ppi is ARCH_TIMER_PHYS_NONSECURE_PPI, the orignal
> code of arch_timer_uses_ppi may go wrong.

How? What specifically happens?

We don't currently assign ARCH_TIMER_PHYS_NONSECURE_PPI to
arch_timer_uses_ppi, so I assume a later patch changes this. This change
should be folded into said patch; it doesn't make sense in isolation.

Thanks,
Mark.

> Signed-off-by: Fu Wei <fu.wei@linaro.org>
> ---
>  drivers/clocksource/arm_arch_timer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index dd1040d..6de164f 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -699,7 +699,7 @@ static int __init arch_timer_register(void)
>  	case ARCH_TIMER_PHYS_NONSECURE_PPI:
>  		err = request_percpu_irq(ppi, arch_timer_handler_phys,
>  					 "arch_timer", arch_timer_evt);
> -		if (!err && arch_timer_ppi[ARCH_TIMER_PHYS_NONSECURE_PPI]) {
> +		if (!err && arch_timer_has_nonsecure_ppi()) {
>  			ppi = arch_timer_ppi[ARCH_TIMER_PHYS_NONSECURE_PPI];
>  			err = request_percpu_irq(ppi, arch_timer_handler_phys,
>  						 "arch_timer", arch_timer_evt);
> -- 
> 2.7.4
> 

^ permalink raw reply

* [PATCH] ARM: dts: socfpga: fine-tune L2 cache configuration
From: Marek Vasut @ 2016-11-18 18:52 UTC (permalink / raw)
  To: linux-arm-kernel

Enable double-linefill and increase prefetch offset, which gives
considerable read performance boost. The following numbers were
obtained using lmbench 3.0 bw_mem tool, for easier comparison, the
numbers are pasted in two columns. The test machine has Cyclone V
SoC running at 800MHz MPU clock and 512MiB 333MHz 16bit DDR3 DRAM.

Without patch	| With patch
$ for i in rd wr rdwr cp fwr frd fcp bzero bcopy ; do echo $i ; bw_mem 64M $i ; done
rd		| rd
64.00 526.46	| 64.00 1151.06
wr		| wr
64.00 329.95	| 64.00 346.14
rdwr		| rdwr
64.00 342.07	| 64.00 367.24
cp		| cp
64.00 239.79	| 64.00 322.47
fwr		| fwr
64.00 1027.90	| 64.00 1025.38
frd		| frd
64.00 322.36	| 64.00 641.89
fcp		| fcp
64.00 256.99	| 64.00 408.41
bzero		| bzero
64.00 1028.43	| 64.00 1025.07
bcopy		| bcopy
64.00 294.73	| 64.00 357.19

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
---
 arch/arm/boot/dts/socfpga.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index f2f74b5..d645bef 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -694,6 +694,11 @@
 			prefetch-data = <1>;
 			prefetch-instr = <1>;
 			arm,shared-override;
+			arm,double-linefill = <1>;
+			arm,double-linefill-incr = <0>;
+			arm,double-linefill-wrap = <1>;
+			arm,prefetch-drop = <0>;
+			arm,prefetch-offset = <7>;
 		};
 
 		mmc: dwmmc0 at ff704000 {
-- 
2.10.2

^ permalink raw reply related

* [PATCH 2/2] mfd: axp20x: Fix AXP806 access errors on cold boot
From: Lee Jones @ 2016-11-18 18:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161111032953.20849-3-wens@csie.org>

On Fri, 11 Nov 2016, Chen-Yu Tsai wrote:

> The AXP806 supports either master/standalone or slave mode.
> Slave mode allows sharing the serial bus, even with multiple
> AXP806 which all have the same hardware address.
> 
> This is done with extra "serial interface address extension",
> or AXP806_BUS_ADDR_EXT, and "register address extension", or
> AXP806_REG_ADDR_EXT, registers. The former is read-only, with
> 1 bit customizable at the factory, and 1 bit depending on the
> state of an external pin. The latter is writable. Only when
> the these device addressing bits (in the upper 4 bits of the
> registers) match, will the device respond to operations on
> its other registers.
> 
> The AXP806_REG_ADDR_EXT was previously configured by Allwinner's
> bootloader. Work on U-boot SPL support now allows us to switch
> to mainline U-boot, which doesn't do this for us. There might
> be other bare minimum bootloaders out there which don't to this
> either. It's best to handle this in the kernel.
> 
> This patch sets AXP806_REG_ADDR_EXT to 0x10, which is what we
> know to be the proper value for a standard AXP806 in slave mode.
> Afterwards it will reinitialize the regmap cache, to purge any
> invalid stale values.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  drivers/mfd/axp20x.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index cdaeb34a9a38..2f5d46f28511 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -829,6 +829,41 @@ int axp20x_device_probe(struct axp20x_dev *axp20x)
>  {
>  	int ret;
>  
> +	/*
> +	 * The AXP806 supports either master/standalone or slave mode.
> +	 * Slave mode allows sharing the serial bus, even with multiple
> +	 * AXP806 which all have the same hardware address.
> +	 *
> +	 * This is done with extra "serial interface address extension",
> +	 * or AXP806_BUS_ADDR_EXT, and "register address extension", or
> +	 * AXP806_REG_ADDR_EXT, registers. The former is read-only, with
> +	 * 1 bit customizable at the factory, and 1 bit depending on the
> +	 * state of an external pin. The latter is writable. Only when
> +	 * the these device addressing bits (in the upper 4 bits of the
> +	 * registers) match, will the device respond to operations on its
> +	 * other registers.
> +	 *
> +	 * Since we only support an AXP806 chained to an AXP809 in slave
> +	 * mode, and there isn't any existing hardware which uses AXP806
> +	 * in master mode, or has 2 AXP806s in the same system, we can
> +	 * just program the register address extension to the slave mode
> +	 * address.
> +	 */
> +	if (axp20x->variant == AXP806_ID) {
> +		/* Write to the register address extension register */
> +		regmap_write(axp20x->regmap, AXP806_REG_ADDR_EXT, 0x10);

Please define 0x10.

> +		/* Make sure the write hits the device */
> +		regcache_sync_region(axp20x->regmap, AXP806_REG_ADDR_EXT,
> +				     AXP806_REG_ADDR_EXT);
> +
> +		/*
> +		 * Reinitialize the regmap cache in case the device didn't
> +		 * properly respond to our reads before.
> +		 */
> +		regmap_reinit_cache(axp20x->regmap, axp20x->regmap_cfg);
> +	}
> +
>  	ret = regmap_add_irq_chip(axp20x->regmap, axp20x->irq,
>  				  IRQF_ONESHOT | IRQF_SHARED, -1,
>  				  axp20x->regmap_irq_chip,

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH v16 04/15] clocksource/drivers/arm_arch_timer: rename some enums and defines, and some cleanups.
From: Mark Rutland @ 2016-11-18 18:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479304148-2965-5-git-send-email-fu.wei@linaro.org>

On Wed, Nov 16, 2016 at 09:48:57PM +0800, fu.wei at linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
> 
> Rename some enums and defines, to unify the format of enums and defines
> in arm_arch_timer.h, also update all the users of these enums and defines:
>     drivers/clocksource/arm_arch_timer.c
>     virt/kvm/arm/hyp/timer-sr.c

I'm happy with making definitions use a consistent ARCH_TIMER_ prefix,
given they're exposed in headers...

> And do some cleanups, according to the suggestion from checkpatch.pl:
> (1) using BIT(nr) instead of (1 << nr)
> (2) using 'unsigned int' instead of 'unsigned'

... but these changes are pointless churn. They make the patch larger,
hardwer to review, and more painful to merge.

Please leave these as they are unless there is a functional problem. If
there will be a functional problem unless these are changed, describe
that in the commit message.

Thanks,
Mark.

> 
> No functional change.
> 
> Signed-off-by: Fu Wei <fu.wei@linaro.org>
> ---
>  drivers/clocksource/arm_arch_timer.c | 111 ++++++++++++++++++-----------------
>  include/clocksource/arm_arch_timer.h |  40 ++++++-------
>  virt/kvm/arm/hyp/timer-sr.c          |   6 +-
>  3 files changed, 81 insertions(+), 76 deletions(-)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 15341cf..dd1040d 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -66,11 +66,11 @@ struct arch_timer {
>  #define to_arch_timer(e) container_of(e, struct arch_timer, evt)
>  
>  static u32 arch_timer_rate;
> -static int arch_timer_ppi[MAX_TIMER_PPI];
> +static int arch_timer_ppi[ARCH_TIMER_MAX_TIMER_PPI];
>  
>  static struct clock_event_device __percpu *arch_timer_evt;
>  
> -static enum arch_timer_ppi_nr arch_timer_uses_ppi = VIRT_PPI;
> +static enum arch_timer_ppi_nr arch_timer_uses_ppi = ARCH_TIMER_VIRT_PPI;
>  static bool arch_timer_c3stop;
>  static bool arch_timer_mem_use_virtual;
>  
> @@ -340,7 +340,7 @@ static void fsl_a008585_set_sne(struct clock_event_device *clk)
>  	if (!static_branch_unlikely(&arch_timer_read_ool_enabled))
>  		return;
>  
> -	if (arch_timer_uses_ppi == VIRT_PPI)
> +	if (arch_timer_uses_ppi == ARCH_TIMER_VIRT_PPI)
>  		clk->set_next_event = fsl_a008585_set_next_event_virt;
>  	else
>  		clk->set_next_event = fsl_a008585_set_next_event_phys;
> @@ -352,7 +352,7 @@ static void __arch_timer_setup(unsigned type,
>  {
>  	clk->features = CLOCK_EVT_FEAT_ONESHOT;
>  
> -	if (type == ARCH_CP15_TIMER) {
> +	if (type == ARCH_TIMER_TYPE_CP15) {
>  		if (arch_timer_c3stop)
>  			clk->features |= CLOCK_EVT_FEAT_C3STOP;
>  		clk->name = "arch_sys_timer";
> @@ -360,14 +360,14 @@ static void __arch_timer_setup(unsigned type,
>  		clk->cpumask = cpumask_of(smp_processor_id());
>  		clk->irq = arch_timer_ppi[arch_timer_uses_ppi];
>  		switch (arch_timer_uses_ppi) {
> -		case VIRT_PPI:
> +		case ARCH_TIMER_VIRT_PPI:
>  			clk->set_state_shutdown = arch_timer_shutdown_virt;
>  			clk->set_state_oneshot_stopped = arch_timer_shutdown_virt;
>  			clk->set_next_event = arch_timer_set_next_event_virt;
>  			break;
> -		case PHYS_SECURE_PPI:
> -		case PHYS_NONSECURE_PPI:
> -		case HYP_PPI:
> +		case ARCH_TIMER_PHYS_SECURE_PPI:
> +		case ARCH_TIMER_PHYS_NONSECURE_PPI:
> +		case ARCH_TIMER_HYP_PPI:
>  			clk->set_state_shutdown = arch_timer_shutdown_phys;
>  			clk->set_state_oneshot_stopped = arch_timer_shutdown_phys;
>  			clk->set_next_event = arch_timer_set_next_event_phys;
> @@ -447,8 +447,8 @@ static void arch_counter_set_user_access(void)
>  
>  static bool arch_timer_has_nonsecure_ppi(void)
>  {
> -	return (arch_timer_uses_ppi == PHYS_SECURE_PPI &&
> -		arch_timer_ppi[PHYS_NONSECURE_PPI]);
> +	return (arch_timer_uses_ppi == ARCH_TIMER_PHYS_SECURE_PPI &&
> +		arch_timer_ppi[ARCH_TIMER_PHYS_NONSECURE_PPI]);
>  }
>  
>  static u32 check_ppi_trigger(int irq)
> @@ -469,14 +469,15 @@ static int arch_timer_starting_cpu(unsigned int cpu)
>  	struct clock_event_device *clk = this_cpu_ptr(arch_timer_evt);
>  	u32 flags;
>  
> -	__arch_timer_setup(ARCH_CP15_TIMER, clk);
> +	__arch_timer_setup(ARCH_TIMER_TYPE_CP15, clk);
>  
>  	flags = check_ppi_trigger(arch_timer_ppi[arch_timer_uses_ppi]);
>  	enable_percpu_irq(arch_timer_ppi[arch_timer_uses_ppi], flags);
>  
>  	if (arch_timer_has_nonsecure_ppi()) {
> -		flags = check_ppi_trigger(arch_timer_ppi[PHYS_NONSECURE_PPI]);
> -		enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], flags);
> +		flags = check_ppi_trigger(arch_timer_ppi[ARCH_TIMER_PHYS_NONSECURE_PPI]);
> +		enable_percpu_irq(arch_timer_ppi[ARCH_TIMER_PHYS_NONSECURE_PPI],
> +				  flags);
>  	}
>  
>  	arch_counter_set_user_access();
> @@ -513,16 +514,17 @@ arch_timer_detect_rate(void __iomem *cntbase, struct device_node *np)
>  static void arch_timer_banner(unsigned type)
>  {
>  	pr_info("%s%s%s timer(s) running at %lu.%02luMHz (%s%s%s).\n",
> -		type & ARCH_CP15_TIMER ? "cp15" : "",
> -		type == (ARCH_CP15_TIMER | ARCH_MEM_TIMER) ?  " and " : "",
> -		type & ARCH_MEM_TIMER ? "mmio" : "",
> +		type & ARCH_TIMER_TYPE_CP15 ? "cp15" : "",
> +		type == (ARCH_TIMER_TYPE_CP15 | ARCH_TIMER_TYPE_MEM) ?
> +			" and " : "",
> +		type & ARCH_TIMER_TYPE_MEM ? "mmio" : "",
>  		(unsigned long)arch_timer_rate / 1000000,
>  		(unsigned long)(arch_timer_rate / 10000) % 100,
> -		type & ARCH_CP15_TIMER ?
> -			(arch_timer_uses_ppi == VIRT_PPI) ? "virt" : "phys" :
> +		type & ARCH_TIMER_TYPE_CP15 ?
> +			(arch_timer_uses_ppi == ARCH_TIMER_VIRT_PPI) ? "virt" : "phys" :
>  			"",
> -		type == (ARCH_CP15_TIMER | ARCH_MEM_TIMER) ?  "/" : "",
> -		type & ARCH_MEM_TIMER ?
> +		type == (ARCH_TIMER_TYPE_CP15 | ARCH_TIMER_TYPE_MEM) ? "/" : "",
> +		type & ARCH_TIMER_TYPE_MEM ?
>  			arch_timer_mem_use_virtual ? "virt" : "phys" :
>  			"");
>  }
> @@ -588,8 +590,9 @@ static void __init arch_counter_register(unsigned type)
>  	u64 start_count;
>  
>  	/* Register the CP15 based counter if we have one */
> -	if (type & ARCH_CP15_TIMER) {
> -		if (IS_ENABLED(CONFIG_ARM64) || arch_timer_uses_ppi == VIRT_PPI)
> +	if (type & ARCH_TIMER_TYPE_CP15) {
> +		if (IS_ENABLED(CONFIG_ARM64) ||
> +		    arch_timer_uses_ppi == ARCH_TIMER_VIRT_PPI)
>  			arch_timer_read_counter = arch_counter_get_cntvct;
>  		else
>  			arch_timer_read_counter = arch_counter_get_cntpct;
> @@ -625,7 +628,7 @@ static void arch_timer_stop(struct clock_event_device *clk)
>  
>  	disable_percpu_irq(arch_timer_ppi[arch_timer_uses_ppi]);
>  	if (arch_timer_has_nonsecure_ppi())
> -		disable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI]);
> +		disable_percpu_irq(arch_timer_ppi[ARCH_TIMER_PHYS_NONSECURE_PPI]);
>  
>  	clk->set_state_shutdown(clk);
>  }
> @@ -688,24 +691,24 @@ static int __init arch_timer_register(void)
>  
>  	ppi = arch_timer_ppi[arch_timer_uses_ppi];
>  	switch (arch_timer_uses_ppi) {
> -	case VIRT_PPI:
> +	case ARCH_TIMER_VIRT_PPI:
>  		err = request_percpu_irq(ppi, arch_timer_handler_virt,
>  					 "arch_timer", arch_timer_evt);
>  		break;
> -	case PHYS_SECURE_PPI:
> -	case PHYS_NONSECURE_PPI:
> +	case ARCH_TIMER_PHYS_SECURE_PPI:
> +	case ARCH_TIMER_PHYS_NONSECURE_PPI:
>  		err = request_percpu_irq(ppi, arch_timer_handler_phys,
>  					 "arch_timer", arch_timer_evt);
> -		if (!err && arch_timer_ppi[PHYS_NONSECURE_PPI]) {
> -			ppi = arch_timer_ppi[PHYS_NONSECURE_PPI];
> +		if (!err && arch_timer_ppi[ARCH_TIMER_PHYS_NONSECURE_PPI]) {
> +			ppi = arch_timer_ppi[ARCH_TIMER_PHYS_NONSECURE_PPI];
>  			err = request_percpu_irq(ppi, arch_timer_handler_phys,
>  						 "arch_timer", arch_timer_evt);
>  			if (err)
> -				free_percpu_irq(arch_timer_ppi[PHYS_SECURE_PPI],
> +				free_percpu_irq(arch_timer_ppi[ARCH_TIMER_PHYS_SECURE_PPI],
>  						arch_timer_evt);
>  		}
>  		break;
> -	case HYP_PPI:
> +	case ARCH_TIMER_HYP_PPI:
>  		err = request_percpu_irq(ppi, arch_timer_handler_phys,
>  					 "arch_timer", arch_timer_evt);
>  		break;
> @@ -737,7 +740,7 @@ static int __init arch_timer_register(void)
>  out_unreg_notify:
>  	free_percpu_irq(arch_timer_ppi[arch_timer_uses_ppi], arch_timer_evt);
>  	if (arch_timer_has_nonsecure_ppi())
> -		free_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI],
> +		free_percpu_irq(arch_timer_ppi[ARCH_TIMER_PHYS_NONSECURE_PPI],
>  				arch_timer_evt);
>  
>  out_free:
> @@ -758,7 +761,7 @@ static int __init arch_timer_mem_register(void __iomem *base, unsigned int irq)
>  
>  	t->base = base;
>  	t->evt.irq = irq;
> -	__arch_timer_setup(ARCH_MEM_TIMER, &t->evt);
> +	__arch_timer_setup(ARCH_TIMER_TYPE_MEM, &t->evt);
>  
>  	if (arch_timer_mem_use_virtual)
>  		func = arch_timer_handler_virt_mem;
> @@ -801,13 +804,15 @@ arch_timer_needs_probing(int type, const struct of_device_id *matches)
>  
>  static int __init arch_timer_common_init(void)
>  {
> -	unsigned mask = ARCH_CP15_TIMER | ARCH_MEM_TIMER;
> +	unsigned int mask = 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_MEM_TIMER, arch_timer_mem_of_match))
> +		if (arch_timer_needs_probing(ARCH_TIMER_TYPE_MEM,
> +					     arch_timer_mem_of_match))
>  			return 0;
> -		if (arch_timer_needs_probing(ARCH_CP15_TIMER, arch_timer_of_match))
> +		if (arch_timer_needs_probing(ARCH_TIMER_TYPE_CP15,
> +					     arch_timer_of_match))
>  			return 0;
>  	}
>  
> @@ -832,16 +837,16 @@ static int __init arch_timer_init(void)
>  	 * their CNTHP_*_EL2 counterparts, and use a different PPI
>  	 * number.
>  	 */
> -	if (is_hyp_mode_available() || !arch_timer_ppi[VIRT_PPI]) {
> +	if (is_hyp_mode_available() || !arch_timer_ppi[ARCH_TIMER_VIRT_PPI]) {
>  		bool has_ppi;
>  
>  		if (is_kernel_in_hyp_mode()) {
> -			arch_timer_uses_ppi = HYP_PPI;
> -			has_ppi = !!arch_timer_ppi[HYP_PPI];
> +			arch_timer_uses_ppi = ARCH_TIMER_HYP_PPI;
> +			has_ppi = !!arch_timer_ppi[ARCH_TIMER_HYP_PPI];
>  		} else {
> -			arch_timer_uses_ppi = PHYS_SECURE_PPI;
> -			has_ppi = (!!arch_timer_ppi[PHYS_SECURE_PPI] ||
> -				   !!arch_timer_ppi[PHYS_NONSECURE_PPI]);
> +			arch_timer_uses_ppi = ARCH_TIMER_PHYS_SECURE_PPI;
> +			has_ppi = (!!arch_timer_ppi[ARCH_TIMER_PHYS_SECURE_PPI] ||
> +				   !!arch_timer_ppi[ARCH_TIMER_PHYS_NONSECURE_PPI]);
>  		}
>  
>  		if (!has_ppi) {
> @@ -858,7 +863,7 @@ static int __init arch_timer_init(void)
>  	if (ret)
>  		return ret;
>  
> -	arch_timer_kvm_info.virtual_irq = arch_timer_ppi[VIRT_PPI];
> +	arch_timer_kvm_info.virtual_irq = arch_timer_ppi[ARCH_TIMER_VIRT_PPI];
>  
>  	return 0;
>  }
> @@ -867,13 +872,13 @@ static int __init arch_timer_of_init(struct device_node *np)
>  {
>  	int i;
>  
> -	if (arch_timers_present & ARCH_CP15_TIMER) {
> +	if (arch_timers_present & ARCH_TIMER_TYPE_CP15) {
>  		pr_warn("multiple nodes in dt, skipping\n");
>  		return 0;
>  	}
>  
> -	arch_timers_present |= ARCH_CP15_TIMER;
> -	for (i = PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++)
> +	arch_timers_present |= ARCH_TIMER_TYPE_CP15;
> +	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);
> @@ -895,7 +900,7 @@ static int __init arch_timer_of_init(struct device_node *np)
>  	 */
>  	if (IS_ENABLED(CONFIG_ARM) &&
>  	    of_property_read_bool(np, "arm,cpu-registers-not-fw-configured"))
> -		arch_timer_uses_ppi = PHYS_SECURE_PPI;
> +		arch_timer_uses_ppi = ARCH_TIMER_PHYS_SECURE_PPI;
>  
>  	return arch_timer_init();
>  }
> @@ -909,7 +914,7 @@ static int __init arch_timer_mem_init(struct device_node *np)
>  	unsigned int irq, ret = -EINVAL;
>  	u32 cnttidr;
>  
> -	arch_timers_present |= ARCH_MEM_TIMER;
> +	arch_timers_present |= ARCH_TIMER_TYPE_MEM;
>  	cntctlbase = of_iomap(np, 0);
>  	if (!cntctlbase) {
>  		pr_err("Can't find CNTCTLBase\n");
> @@ -1008,28 +1013,28 @@ static int __init arch_timer_acpi_init(struct acpi_table_header *table)
>  {
>  	struct acpi_table_gtdt *gtdt;
>  
> -	if (arch_timers_present & ARCH_CP15_TIMER) {
> +	if (arch_timers_present & ARCH_TIMER_TYPE_CP15) {
>  		pr_warn("already initialized, skipping\n");
>  		return -EINVAL;
>  	}
>  
>  	gtdt = container_of(table, struct acpi_table_gtdt, header);
>  
> -	arch_timers_present |= ARCH_CP15_TIMER;
> +	arch_timers_present |= ARCH_TIMER_TYPE_CP15;
>  
> -	arch_timer_ppi[PHYS_SECURE_PPI] =
> +	arch_timer_ppi[ARCH_TIMER_PHYS_SECURE_PPI] =
>  		map_generic_timer_interrupt(gtdt->secure_el1_interrupt,
>  		gtdt->secure_el1_flags);
>  
> -	arch_timer_ppi[PHYS_NONSECURE_PPI] =
> +	arch_timer_ppi[ARCH_TIMER_PHYS_NONSECURE_PPI] =
>  		map_generic_timer_interrupt(gtdt->non_secure_el1_interrupt,
>  		gtdt->non_secure_el1_flags);
>  
> -	arch_timer_ppi[VIRT_PPI] =
> +	arch_timer_ppi[ARCH_TIMER_VIRT_PPI] =
>  		map_generic_timer_interrupt(gtdt->virtual_timer_interrupt,
>  		gtdt->virtual_timer_flags);
>  
> -	arch_timer_ppi[HYP_PPI] =
> +	arch_timer_ppi[ARCH_TIMER_HYP_PPI] =
>  		map_generic_timer_interrupt(gtdt->non_secure_el2_interrupt,
>  		gtdt->non_secure_el2_flags);
>  
> diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
> index d23c381..2625ff1 100644
> --- a/include/clocksource/arm_arch_timer.h
> +++ b/include/clocksource/arm_arch_timer.h
> @@ -20,18 +20,18 @@
>  #include <linux/timecounter.h>
>  #include <linux/types.h>
>  
> -#define ARCH_CP15_TIMER			BIT(0)
> -#define ARCH_MEM_TIMER			BIT(1)
> +#define ARCH_TIMER_TYPE_CP15		BIT(0)
> +#define ARCH_TIMER_TYPE_MEM		BIT(1)
>  
> -#define ARCH_TIMER_CTRL_ENABLE		(1 << 0)
> -#define ARCH_TIMER_CTRL_IT_MASK		(1 << 1)
> -#define ARCH_TIMER_CTRL_IT_STAT		(1 << 2)
> +#define ARCH_TIMER_CTRL_ENABLE		BIT(0)
> +#define ARCH_TIMER_CTRL_IT_MASK		BIT(1)
> +#define ARCH_TIMER_CTRL_IT_STAT		BIT(2)
>  
> -#define CNTHCTL_EL1PCTEN		(1 << 0)
> -#define CNTHCTL_EL1PCEN			(1 << 1)
> -#define CNTHCTL_EVNTEN			(1 << 2)
> -#define CNTHCTL_EVNTDIR			(1 << 3)
> -#define CNTHCTL_EVNTI			(0xF << 4)
> +#define ARCH_TIMER_CNTHCTL_EL1PCTEN	BIT(0)
> +#define ARCH_TIMER_CNTHCTL_EL1PCEN	BIT(1)
> +#define ARCH_TIMER_CNTHCTL_EVNTEN	BIT(2)
> +#define ARCH_TIMER_CNTHCTL_EVNTDIR	BIT(3)
> +#define ARCH_TIMER_CNTHCTL_EVNTI	(0xF << 4)
>  
>  enum arch_timer_reg {
>  	ARCH_TIMER_REG_CTRL,
> @@ -39,11 +39,11 @@ enum arch_timer_reg {
>  };
>  
>  enum arch_timer_ppi_nr {
> -	PHYS_SECURE_PPI,
> -	PHYS_NONSECURE_PPI,
> -	VIRT_PPI,
> -	HYP_PPI,
> -	MAX_TIMER_PPI
> +	ARCH_TIMER_PHYS_SECURE_PPI,
> +	ARCH_TIMER_PHYS_NONSECURE_PPI,
> +	ARCH_TIMER_VIRT_PPI,
> +	ARCH_TIMER_HYP_PPI,
> +	ARCH_TIMER_MAX_TIMER_PPI
>  };
>  
>  enum arch_timer_spi_nr {
> @@ -57,13 +57,13 @@ enum arch_timer_spi_nr {
>  #define ARCH_TIMER_MEM_PHYS_ACCESS	2
>  #define ARCH_TIMER_MEM_VIRT_ACCESS	3
>  
> -#define ARCH_TIMER_USR_PCT_ACCESS_EN	(1 << 0) /* physical counter */
> -#define ARCH_TIMER_USR_VCT_ACCESS_EN	(1 << 1) /* virtual counter */
> -#define ARCH_TIMER_VIRT_EVT_EN		(1 << 2)
> +#define ARCH_TIMER_USR_PCT_ACCESS_EN	BIT(0) /* physical counter */
> +#define ARCH_TIMER_USR_VCT_ACCESS_EN	BIT(1) /* virtual counter */
> +#define ARCH_TIMER_VIRT_EVT_EN		BIT(2)
>  #define ARCH_TIMER_EVT_TRIGGER_SHIFT	(4)
>  #define ARCH_TIMER_EVT_TRIGGER_MASK	(0xF << ARCH_TIMER_EVT_TRIGGER_SHIFT)
> -#define ARCH_TIMER_USR_VT_ACCESS_EN	(1 << 8) /* virtual timer registers */
> -#define ARCH_TIMER_USR_PT_ACCESS_EN	(1 << 9) /* physical timer registers */
> +#define ARCH_TIMER_USR_VT_ACCESS_EN	BIT(8) /* virtual timer registers */
> +#define ARCH_TIMER_USR_PT_ACCESS_EN	BIT(9) /* physical timer registers */
>  
>  #define ARCH_TIMER_EVT_STREAM_FREQ	10000	/* 100us */
>  
> diff --git a/virt/kvm/arm/hyp/timer-sr.c b/virt/kvm/arm/hyp/timer-sr.c
> index 798866a..695b9d9 100644
> --- a/virt/kvm/arm/hyp/timer-sr.c
> +++ b/virt/kvm/arm/hyp/timer-sr.c
> @@ -37,7 +37,7 @@ void __hyp_text __timer_save_state(struct kvm_vcpu *vcpu)
>  
>  	/* Allow physical timer/counter access for the host */
>  	val = read_sysreg(cnthctl_el2);
> -	val |= CNTHCTL_EL1PCTEN | CNTHCTL_EL1PCEN;
> +	val |= ARCH_TIMER_CNTHCTL_EL1PCTEN | ARCH_TIMER_CNTHCTL_EL1PCEN;
>  	write_sysreg(val, cnthctl_el2);
>  
>  	/* Clear cntvoff for the host */
> @@ -55,8 +55,8 @@ void __hyp_text __timer_restore_state(struct kvm_vcpu *vcpu)
>  	 * Physical counter access is allowed
>  	 */
>  	val = read_sysreg(cnthctl_el2);
> -	val &= ~CNTHCTL_EL1PCEN;
> -	val |= CNTHCTL_EL1PCTEN;
> +	val &= ~ARCH_TIMER_CNTHCTL_EL1PCEN;
> +	val |= ARCH_TIMER_CNTHCTL_EL1PCTEN;
>  	write_sysreg(val, cnthctl_el2);
>  
>  	if (timer->enabled) {
> -- 
> 2.7.4
> 

^ permalink raw reply

* [PATCH v8 6/7] arm/arm64: vgic: Implement VGICv3 CPU interface access
From: Vijay Kilari @ 2016-11-18 18:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161117160951.GB23588@cbox>

On Thu, Nov 17, 2016 at 9:39 PM, Christoffer Dall
<christoffer.dall@linaro.org> wrote:
> On Thu, Nov 17, 2016 at 09:25:59PM +0530, Vijay Kilari wrote:
>> On Thu, Nov 17, 2016 at 12:22 AM, Christoffer Dall
>> <christoffer.dall@linaro.org> wrote:
>> > On Fri, Nov 04, 2016 at 04:43:32PM +0530, vijay.kilari at gmail.com wrote:
>> >> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>> >>
>> >> VGICv3 CPU interface registers are accessed using
>> >> KVM_DEV_ARM_VGIC_CPU_SYSREGS ioctl. These registers are accessed
>> >> as 64-bit. The cpu MPIDR value is passed along with register id.
>> >> is used to identify the cpu for registers access.
>> >>
>> >> The version of VGIC v3 specification is define here
>> >> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-July/445611.html
>> >>
>> >> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
>> >> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>> >> ---
>> >>  arch/arm64/include/uapi/asm/kvm.h   |   3 +
>> >>  arch/arm64/kvm/Makefile             |   1 +
>> >>  include/kvm/arm_vgic.h              |   9 +
>> >>  virt/kvm/arm/vgic/vgic-kvm-device.c |  27 +++
>> >>  virt/kvm/arm/vgic/vgic-mmio-v3.c    |  19 +++
>> >>  virt/kvm/arm/vgic/vgic-sys-reg-v3.c | 324 ++++++++++++++++++++++++++++++++++++
>> >>  virt/kvm/arm/vgic/vgic-v3.c         |   8 +
>> >>  virt/kvm/arm/vgic/vgic.h            |   4 +
>> >>  8 files changed, 395 insertions(+)
>> >>
>> >> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
>> >> index 56dc08d..91c7137 100644
>> >> --- a/arch/arm64/include/uapi/asm/kvm.h
>> >> +++ b/arch/arm64/include/uapi/asm/kvm.h
>> >> @@ -206,9 +206,12 @@ struct kvm_arch_memory_slot {
>> >>                       (0xffffffffULL << KVM_DEV_ARM_VGIC_V3_MPIDR_SHIFT)
>> >>  #define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT      0
>> >>  #define   KVM_DEV_ARM_VGIC_OFFSET_MASK       (0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
>> >> +#define   KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK (0xffff)
>> >>  #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS 3
>> >>  #define KVM_DEV_ARM_VGIC_GRP_CTRL    4
>> >>  #define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
>> >> +#define KVM_DEV_ARM_VGIC_CPU_SYSREGS    6
>> >> +
>> >>  #define   KVM_DEV_ARM_VGIC_CTRL_INIT 0
>> >>
>> >>  /* Device Control API on vcpu fd */
>> >> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
>> >> index d50a82a..1a14e29 100644
>> >> --- a/arch/arm64/kvm/Makefile
>> >> +++ b/arch/arm64/kvm/Makefile
>> >> @@ -32,5 +32,6 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v3.o
>> >>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-kvm-device.o
>> >>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-its.o
>> >>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/irqchip.o
>> >> +kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-sys-reg-v3.o
>> >
>> > Thi is making me wonder:  Are we properly handling GICv3 save/restore
>> > for AArch32 now that we have GICv3 support for AArch32?  By properly I
>> > mean that either it is clearly only supported on AArch64 systems or it's
>> > supported on both AArch64 and AArch32, but it shouldn't break randomly
>> > on AArch32.
>>
>> It supports both AArch64 and AArch64 in handling of system registers
>> save/restore.
>> All system registers that we save/restore are 32-bit for both aarch64
>> and aarch32.
>> Though opcode op0 should be zero for aarch32, the remaining Op and CRn codes
>> are same. However the codes sent by qemu is matched and register
>> are handled properly irrespective of AArch32 or AArch64.
>>
>> I don't have platform which support AArch32 guests to verify.
>
> Actually this is not about the guest, it's about an ARMv8 AArch32 host
> that has a GICv3.
>
> I just tried to do a v7 compile with your patches, and it results in an
> epic failure, so there's something for you to look at.
>

Could you please share you config file?. I tried with multi_v7 defconfig with
CONFIG KVM and CONFIG_KVM_ARM_HOST enabled. it compiled for me.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox