Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] ARM: imx31/of: clk: init clock controller with CLK_OF_DECLARE
From: Stephen Boyd @ 2016-09-20 17:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160919055802.dkf2rxcou44kbya5@pengutronix.de>

On 09/19, Uwe Kleine-K?nig wrote:
> Hello,
> 
> On Mon, Sep 19, 2016 at 05:42:17AM +0300, Vladimir Zapolskiy wrote:
> > The change is based on v4.8.0-rc1 and plus it depends on this fixup:
> > 
> >   ARM: dts: imx31: fix clock control module interrupts description
> > 
> > The change is tested on qemu kzm target and mx31lite board, while both
> > targets don't have DTS in upstream, I had to write simple DTS files for
> > them, because the proposed change is for i.MX31 targets with OF support.
> > 
> > The second change in the series compilation time dependent on the first
> > one, while the first change apparently should break runtime execution
> > of i.MX31 boards with OF support, if both changes are applied everything
> > should be fine, regression testing on the same legacy targets does not
> 
> that means this should be done in a single patch.

Yes, please combine the patches. I can ack it then so it can be
routed through arm-soc.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* [PATCH] KVM: arm/arm64: timer: Fix hw sync for user space irqchip path
From: Marc Zyngier @ 2016-09-20 17:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474002971-31434-1-git-send-email-agraf@suse.de>

Alex,

On 16/09/16 06:16, Alexander Graf wrote:
> While adding the new vgic implementation, apparently nobody tested
> the non-vgic path where user space controls the vgic, so two functions
> slipped through the cracks that get called in generic code but don't
> check whether hardware support is enabled.
> 
> This patch guards them with proper checks to ensure we only try to
> use vgic data structures if they are available. Without this, I get
> a stack trace:
> 
> [   74.363037] Unable to handle kernel paging request at virtual address ffffffffffffffe8
> [...]
> [   74.929654] [<ffff000008824bcc>] _raw_spin_lock+0x1c/0x58
> [   74.935133] [<ffff0000080b7f20>] kvm_vgic_flush_hwstate+0x88/0x288
> [   74.941406] [<ffff0000080ab0b4>] kvm_arch_vcpu_ioctl_run+0xfc/0x630
> [   74.947766] [<ffff0000080a15bc>] kvm_vcpu_ioctl+0x2f4/0x710
> [   74.953420] [<ffff0000082788a8>] do_vfs_ioctl+0xb0/0x728
> [   74.958807] [<ffff000008278fb4>] SyS_ioctl+0x94/0xa8
> [   74.963844] [<ffff000008083744>] el0_svc_naked+0x38/0x3c
> 
> Fixes: 0919e84c0
> Cc: stable at vger.kernel.org
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  virt/kvm/arm/vgic/vgic.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
> index e83b7fe..9f312ba 100644
> --- a/virt/kvm/arm/vgic/vgic.c
> +++ b/virt/kvm/arm/vgic/vgic.c
> @@ -645,6 +645,9 @@ next:
>  /* Sync back the hardware VGIC state into our emulation after a guest's run. */
>  void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
>  {
> +	if (!vcpu->kvm->arch.vgic.enabled)
> +		return;
> +
>  	vgic_process_maintenance_interrupt(vcpu);
>  	vgic_fold_lr_state(vcpu);
>  	vgic_prune_ap_list(vcpu);
> @@ -653,6 +656,9 @@ void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
>  /* Flush our emulation state into the GIC hardware before entering the guest. */
>  void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu)
>  {
> +	if (!vcpu->kvm->arch.vgic.enabled)
> +		return;
> +
>  	spin_lock(&vcpu->arch.vgic_cpu.ap_list_lock);
>  	vgic_flush_lr_state(vcpu);
>  	spin_unlock(&vcpu->arch.vgic_cpu.ap_list_lock);
> 

I hate that fix, because it papers over the fact that we have uninitialized
structures all over the shop, and that's not exactly great.

How about the following instead:

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index c94b90d..0961128 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -472,6 +472,9 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
 			return ret;
 	}
 
+	if (unlikely(!irqchip_in_kernel(kvm)))
+		kvm_no_vgic_init(kvm);
+
 	/*
 	 * Enable the arch timers only if we have an in-kernel VGIC
 	 * and it has been properly initialized, since we cannot handle
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index bb46c03..1b70b1e 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -327,4 +327,6 @@ int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi);
  */
 int kvm_vgic_setup_default_irq_routing(struct kvm *kvm);
 
+void kvm_no_vgic_init(struct kvm *kvm);
+
 #endif /* __KVM_ARM_VGIC_H */
diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
index 83777c1..7b8f12b 100644
--- a/virt/kvm/arm/vgic/vgic-init.c
+++ b/virt/kvm/arm/vgic/vgic-init.c
@@ -151,9 +151,11 @@ static int kvm_vgic_dist_init(struct kvm *kvm, unsigned int nr_spis)
 	INIT_LIST_HEAD(&dist->lpi_list_head);
 	spin_lock_init(&dist->lpi_list_lock);
 
-	dist->spis = kcalloc(nr_spis, sizeof(struct vgic_irq), GFP_KERNEL);
-	if (!dist->spis)
-		return  -ENOMEM;
+	if (nr_spis) {
+		dist->spis = kcalloc(nr_spis, sizeof(struct vgic_irq), GFP_KERNEL);
+		if (!dist->spis)
+			return  -ENOMEM;
+	}
 
 	/*
 	 * In the following code we do not take the irq struct lock since
@@ -325,6 +327,21 @@ int vgic_lazy_init(struct kvm *kvm)
 	return ret;
 }
 
+void kvm_no_vgic_init(struct kvm *kvm)
+{
+	mutex_lock(&kvm->lock);
+	if (unlikely(!vgic_initialized(kvm))) {
+		struct kvm_vcpu *vcpu;
+		int i;
+
+		kvm_vgic_dist_init(kvm, 0);
+		kvm_for_each_vcpu(i, vcpu, kvm)
+			kvm_vgic_vcpu_init(vcpu);
+		kvm->arch.vgic.initialized = true;
+	}
+	mutex_unlock(&kvm->lock);
+}
+
 /* RESOURCE MAPPING */
 
 /**

Thanks,

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

^ permalink raw reply related

* [PATCH v3] ASoC: exynos: organize the asoc audio into a menu
From: Krzysztof Kozlowski @ 2016-09-20 17:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474311470-23175-1-git-send-email-ayaka@soulik.info>

On Tue, Sep 20, 2016 at 02:57:50AM +0800, Randy Li wrote:
> It is simple sound card time, we could assign different codec
> to a interface without making a specific driver for it. The SPDIF
> and I2S interface for Samsung would be possible used by
> simple-sound-card, but not sure about the PCM.
> 
> Those S3C time entries are left alone as I don't think any new board
> would need them.
> 
> Signed-off-by: Randy Li <ayaka@soulik.info>
> ---
>  sound/soc/samsung/Kconfig | 57 ++++++++++++++++++++++++-----------------------
>  1 file changed, 29 insertions(+), 28 deletions(-)
> 

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

^ permalink raw reply

* [PATCH 1/4] ARM: tegra: nyan: Use proper IRQ type definitions
From: Jon Hunter @ 2016-09-20 17:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160828173246.32621-1-contact@paulk.fr>


On 28/08/16 18:32, Paul Kocialkowski wrote:
> This switches a few interrupt definitions that were using
> GPIO_ACTIVE_HIGH as IRQ type, which is invalid.

May be you are right, but this does not describe why this is invalid.
Can you elaborate?

> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
>  arch/arm/boot/dts/tegra124-nyan.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/tegra124-nyan.dtsi b/arch/arm/boot/dts/tegra124-nyan.dtsi
> index 271505e..30a77ec 100644
> --- a/arch/arm/boot/dts/tegra124-nyan.dtsi
> +++ b/arch/arm/boot/dts/tegra124-nyan.dtsi
> @@ -59,7 +59,7 @@
>  			compatible = "maxim,max98090";
>  			reg = <0x10>;
>  			interrupt-parent = <&gpio>;
> -			interrupts = <TEGRA_GPIO(H, 4) GPIO_ACTIVE_HIGH>;
> +			interrupts = <TEGRA_GPIO(H, 4) IRQ_TYPE_EDGE_FALLING>;

If this in invalid then the DT binding doc for the max98090 should be
updated as well.

>  		};
>  
>  		temperature-sensor at 4c {
> @@ -325,7 +325,7 @@
>  					reg = <0x9>;
>  					interrupt-parent = <&gpio>;
>  					interrupts = <TEGRA_GPIO(J, 0)
> -							GPIO_ACTIVE_HIGH>;
> +							IRQ_TYPE_EDGE_BOTH>;
>  					ti,ac-detect-gpios = <&gpio
>  							TEGRA_GPIO(J, 0)
>  							GPIO_ACTIVE_HIGH>;
> 

Cheers
Jon

-- 
nvpublic

^ permalink raw reply

* [PATCH 5/5] arm64: Add uprobe support
From: Catalin Marinas @ 2016-09-20 16:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <abaac6ebaa5dd63889803ec568663bc8c8c65b02.1470114993.git.panand@redhat.com>

Hi Pratyush,

On Tue, Aug 02, 2016 at 11:00:09AM +0530, Pratyush Anand wrote:
> --- a/arch/arm64/include/asm/debug-monitors.h
> +++ b/arch/arm64/include/asm/debug-monitors.h
> @@ -70,6 +70,9 @@
>  #define BRK64_ESR_MASK		0xFFFF
>  #define BRK64_ESR_KPROBES	0x0004
>  #define BRK64_OPCODE_KPROBES	(AARCH64_BREAK_MON | (BRK64_ESR_KPROBES << 5))
> +/* uprobes BRK opcodes with ESR encoding  */
> +#define BRK64_ESR_UPROBES	0x0008
> +#define BRK64_OPCODE_UPROBES	(AARCH64_BREAK_MON | (BRK64_ESR_UPROBES << 5))

You can use 0x0005 as immediate, we don't need individual bits here.

> --- a/arch/arm64/include/asm/probes.h
> +++ b/arch/arm64/include/asm/probes.h
> @@ -35,4 +35,8 @@ struct arch_specific_insn {
>  };
>  #endif
>  
> +#ifdef CONFIG_UPROBES
> +typedef u32 uprobe_opcode_t;
> +#endif

We don't need #ifdef around this typedef. Also, all the other
architectures seem to have this typedef in asm/uprobes.h.

> --- a/arch/arm64/include/asm/ptrace.h
> +++ b/arch/arm64/include/asm/ptrace.h
> @@ -217,6 +217,14 @@ int valid_user_regs(struct user_pt_regs *regs, struct task_struct *task);
>  
>  #include <asm-generic/ptrace.h>
>  
> +#define procedure_link_pointer(regs)	((regs)->regs[30])
> +
> +static inline void procedure_link_pointer_set(struct pt_regs *regs,
> +					   unsigned long val)
> +{
> +	procedure_link_pointer(regs) = val;
> +}

Do you need these macro and function here? It seems that they are only
used in arch_uretprobe_hijack_return_addr(), so you can just expand them
in there, no need for additional definitions.

> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -109,6 +109,7 @@ static inline struct thread_info *current_thread_info(void)
>  #define TIF_NEED_RESCHED	1
>  #define TIF_NOTIFY_RESUME	2	/* callback before returning to user */
>  #define TIF_FOREIGN_FPSTATE	3	/* CPU's FP state is not current's */
> +#define TIF_UPROBE		5	/* uprobe breakpoint or singlestep */

Nitpick: you can just use 4 until we cover this gap.

> --- /dev/null
> +++ b/arch/arm64/include/asm/uprobes.h
> @@ -0,0 +1,37 @@
> +/*
> + * Copyright (C) 2014-2015 Pratyush Anand <panand@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _ASM_UPROBES_H
> +#define _ASM_UPROBES_H
> +
> +#include <asm/debug-monitors.h>
> +#include <asm/insn.h>
> +#include <asm/probes.h>
> +
> +#define MAX_UINSN_BYTES		AARCH64_INSN_SIZE
> +
> +#define UPROBE_SWBP_INSN	BRK64_OPCODE_UPROBES
> +#define UPROBE_SWBP_INSN_SIZE	4

Nitpick: for consistency, just use AARCH64_INSN_SIZE.

> +#define UPROBE_XOL_SLOT_BYTES	MAX_UINSN_BYTES
> +
> +struct arch_uprobe_task {
> +	unsigned long saved_fault_code;
> +};
> +
> +struct arch_uprobe {
> +	union {
> +		u8 insn[MAX_UINSN_BYTES];
> +		u8 ixol[MAX_UINSN_BYTES];
> +	};
> +	struct arch_probe_insn api;
> +	bool simulate;
> +};
> +
> +extern void flush_uprobe_xol_access(struct page *page, unsigned long uaddr,
> +		void *kaddr, unsigned long len);

I don't think we need this. It doesn't seem to be used anywhere other
than the arm64 uprobes.c file. So I would rather expose
sync_icache_aliases(), similarly to __sync_icache_dcache().

> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -688,7 +688,8 @@ ret_fast_syscall:
>  	ldr	x1, [tsk, #TI_FLAGS]		// re-check for syscall tracing
>  	and	x2, x1, #_TIF_SYSCALL_WORK
>  	cbnz	x2, ret_fast_syscall_trace
> -	and	x2, x1, #_TIF_WORK_MASK
> +	mov     x2, #_TIF_WORK_MASK
> +	and     x2, x1, x2

Is this needed because _TIF_WORK_MASK cannot work as an immediate value
to 'and'? We could reorder the TIF bits, they are not exposed to user to
have ABI implications.

>  	cbnz	x2, work_pending
>  	enable_step_tsk x1, x2
>  	kernel_exit 0
> @@ -718,7 +719,8 @@ work_resched:
>  ret_to_user:
>  	disable_irq				// disable interrupts
>  	ldr	x1, [tsk, #TI_FLAGS]
> -	and	x2, x1, #_TIF_WORK_MASK
> +	mov     x2, #_TIF_WORK_MASK
> +	and     x2, x1, x2
>  	cbnz	x2, work_pending
>  	enable_step_tsk x1, x2
>  	kernel_exit 0

Same here.

> --- /dev/null
> +++ b/arch/arm64/kernel/probes/uprobes.c
> @@ -0,0 +1,227 @@
> +/*
> + * Copyright (C) 2014-2015 Pratyush Anand <panand@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/highmem.h>
> +#include <linux/ptrace.h>
> +#include <linux/uprobes.h>
> +
> +#include "decode-insn.h"
> +
> +#define UPROBE_INV_FAULT_CODE	UINT_MAX
> +
> +void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
> +		void *src, unsigned long len)
> +{
> +	void *xol_page_kaddr = kmap_atomic(page);
> +	void *dst = xol_page_kaddr + (vaddr & ~PAGE_MASK);
> +
> +	preempt_disable();

kmap_atomic() already disabled preemption.

> +
> +	/* Initialize the slot */
> +	memcpy(dst, src, len);
> +
> +	/* flush caches (dcache/icache) */
> +	flush_uprobe_xol_access(page, vaddr, dst, len);

Just use sync_icache_aliases() here (once exposed in an arm64 header).

> +
> +	preempt_enable();
> +
> +	kunmap_atomic(xol_page_kaddr);
> +}
> +
> +unsigned long uprobe_get_swbp_addr(struct pt_regs *regs)
> +{
> +	return instruction_pointer(regs);
> +}
> +
> +int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
> +		unsigned long addr)
> +{
> +	probe_opcode_t insn;
> +
> +	/* TODO: Currently we do not support AARCH32 instruction probing */

Is there a way to check (not necessarily in this file) that we don't
probe 32-bit tasks?

> +	if (!IS_ALIGNED(addr, AARCH64_INSN_SIZE))
> +		return -EINVAL;
> +
> +	insn = *(probe_opcode_t *)(&auprobe->insn[0]);
> +
> +	switch (arm_probe_decode_insn(insn, &auprobe->api)) {
> +	case INSN_REJECTED:
> +		return -EINVAL;
> +
> +	case INSN_GOOD_NO_SLOT:
> +		auprobe->simulate = true;
> +		break;
> +
> +	case INSN_GOOD:
> +	default:
> +		break;

Nitpick, we don't need case INSN_GOOD as well since default would cover
it (that's unless you want to change default to BUG()).

> +	}
> +
> +	return 0;
> +}
> +
> +int arch_uprobe_pre_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
> +{
> +	struct uprobe_task *utask = current->utask;
> +
> +	/* saved fault code is restored in post_xol */
> +	utask->autask.saved_fault_code = current->thread.fault_code;

Does the current->thread.fault_code has any meaning here? We use it in
the arm64 code when delivering a signal to user but I don't think that's
the case here, we are handling a breakpoint instruction and there isn't
anything that set the fault_code.

> +
> +	/* An invalid fault code between pre/post xol event */
> +	current->thread.fault_code = UPROBE_INV_FAULT_CODE;
> +
> +	/* Instruction point to execute ol */
> +	instruction_pointer_set(regs, utask->xol_vaddr);
> +
> +	user_enable_single_step(current);
> +
> +	return 0;
> +}
> +
> +int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
> +{
> +	struct uprobe_task *utask = current->utask;
> +
> +	WARN_ON_ONCE(current->thread.fault_code != UPROBE_INV_FAULT_CODE);
> +
> +	/* restore fault code */
> +	current->thread.fault_code = utask->autask.saved_fault_code;

Same here, I don't think this needs restoring if it wasn't meaningful in
the first place.


-- 
Catalin

^ permalink raw reply

* [GIT PULL] ARM: mvebu: soc for v4.9 (#1)
From: Gregory CLEMENT @ 2016-09-20 16:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5238448.m4YaC7vVme@wuerfel>

Hi Arnd,
 
 On mar., sept. 20 2016, Arnd Bergmann <arnd@arndb.de> wrote:

> On Tuesday, September 20, 2016 9:25:51 AM CEST Gregory CLEMENT wrote:
>> Hi Arnd,
>>  
>>  On lun., sept. 19 2016, Arnd Bergmann <arnd@arndb.de> wrote:
>> 
>> > On Monday, September 19, 2016 11:46:22 PM CEST Arnd Bergmann wrote:
>> >> On Wednesday, September 14, 2016 5:34:37 PM CEST Gregory CLEMENT wrote:
>> >> > mvebu soc for 4.9 (part 1)
>> >> > 
>> >> > - irq cleanup for old mvebu SoC
>> >> > - Convert orion5x based SoC Netgear WNR854T to devicetree
>> >> > 
>> >> 
>> >> Pulled into next/soc, thanks!
>> >
>> > Sorry, backed out again after seeing the PCI stuff on the WNR854T in
>> > there. I thought the plan was to leave out PCI support from the DT
>> > based machine file, and leave the old board in place, or am I
>> > missing something?
>> 
>> I might have overlooked the thread, I thouhgt the state of the patch was
>> OK as is, and further changes can be done later.
>> 
>> So it seems that it will be 4.10 material.
>> 
>
> We have gained a little more time since Linus delayed the merge window
> by another week, so I think there is still a chance to respin this.
>
> I also really want the NO_IRQ changes to get merged ;-)
>
> Just drop the patch removing wnr854t-setup.c, and add another patch
> on top to remove the PCI initialization from wrt350n-v2-setup.c,
> and I'll take it.

Do you mean board-wnr854t.c?

If it is the case then I can just drop the patch adding this file because
the only things done in this file are about the PCI.

Gregory

>
> 	Arnd

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply

* [PATCH v4 2/2] KVM: arm/arm64: Route vtimer events to user space
From: Marc Zyngier @ 2016-09-20 16:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <57E14829.3060406@suse.de>

On 20/09/16 15:31, Alexander Graf wrote:
> On 09/20/2016 02:37 PM, Marc Zyngier wrote:

[...]

>>>>> We also need to know "timer line low + timer line masked", as otherwise
>>>>> we might get spurious interrupts in the guest, no?
>>>> Yes. Though you can't really know about this one, and you'll have to
>>>> wait until the next natural exit to find out. As long as the spurious is
>>> We can provoke a special exit for it, no?
>> How? The guest decides to disable its timer. That doesn't trigger any
>> exit whatsoever. You'll have to wait until the next exit from the guest
>> to notice it.
> 
> Before we inject a timer interrupt, we can check whether the pending 
> semantics of user space / kernel space match. If they don't match, we 
> can exit before we inject the interrupt and allow user space to disable 
> the pending state again.

Let's rewind a bit, because I've long lost track of what you're trying
to do to handle what.

You need two signals:

(1) TIMER_LEVEL: the output of the timer line, having accounted for the
IMASK bit. This is conveniently the value of timer->irq.level.

(2) TIMER_IRQ_MASK: an indication from userspace that a timer interrupt
is pending, and that the physical line should be masked.

You need a number of rules:

(a) On exit to userspace, the kernel always exposes the value of
TIMER_LEVEL.

(b) On kernel entry, userspace always exposes the required
TIMER_IRQ_MASK, depending on what has been exposed to it by TIMER_LEVEL.

(c) If on guest exit, TIMER_LEVEL==1 and TIMER_IRQ_MASK==0, perform a
userspace exit, because the emulated GIC needs to make the interrupt
pending.

(d) If on guest exit, TIMER_LEVEL==0 and TIMER_IRQ_MASK==1, perform a
userspace exit, because the guest has disabled its timer before taking
the interrupt, and the emulated GIC needs to retire the pending state.

and that's it. Nothing else. The kernel tells userspace the state of the
timer, and userspace drives the masking of the physical interrupt.
Conveniently, this matches what the current code does.

> 
>>
>>>> "spurious in the GIC sense" (hence returning 0x3ff) and not a spurious
>>>> timer interrupt being presented, you're fine.
>>> Imagine
>>>
>>>     * guest configures timer to fire
>>>     * kvm exits to user space because lines mismatch now
>> I suppose you mean timer fired + physical interrupt for the virtual enabled?
> 
> In this scheme, I would simply say "user space thinks it's pending" == 
> "host vtimer irq is masked". So because user space thinks that no timer 
> is pending and the guest managed to trigger a vtimer irq on the host, 
> the guest exited on a physical vtimer irq. Kvm then checks whether the 
> timer expired and sets the vcpu timer pending status to 1. User space 
> status is still 0, so kvm exits to user space to update the status.
> 
>>
>>>     * user space sets gic line up, setting vcpu irq line high
>>>     * guest runs, handles IRQ, calls EOI with IRQs disabled on the core
>> Same thing: you mean physical interrupt for the virtual timer disabled?
>> or something else?
> 
> I mean the guest sets pstate.I = 0.

The I bit is a mask, just like any other exception bit in PSTATE. So
you're *enabling* interrupts here. But let's assume the interrupts are
disabled, as they are in an interrupt handler.

> 
>>
>>>     * user space handles EOI, but still leaves the vcpu line high
>> If there is another pending interrupt, sure. Otherwise, that's a bug.
> 
> Why is that a bug? The line really is still pending at this point.

So what is this "vcpu line"? HCR_EL2.VI? The timer interrupt through the
GIC? The timer output line?

> 
>>
>>>     * guest runs, sets cval, enables IRQs on the core
>> What is that interrupt? Please name things, because that's very confusing.
> 
> The guest sets pstate.I=1.

And?

> 
>>
>>>     * kvm injects a timer interrupt into the guest
>> Why? Has the timer fired?
> 
> cntv.ctl.imask = 0 here, but user space doesn't know that yet. So the 
> "pending" state is still active.

Rule (a) should apply, always. Whether it is pending or not depends on
what userspace decides to do. If userspace doesn't play ball, the guest
will keep exiting.

> 
>>
>>> At this point, the pending state from user space is bogus, as the line
>>> really isn't pending anymore. However, when the guest asks the GIC at
>>> that point what interrupt is pending, the line down state will have
>>> populated into user space before the mmio request gets processed. So you
>>> will see a 0x3ff return I guess.
>>>
>>> Is that reasonable?
>> I'm sorry, but you need to put names on things and precisely describe
>> the various events. because from the above script, I can derive twenty
>> different scenarios...
> 
> I'm sure one of them has to be racy? :)
> 
> (in fact, there are probably more than what I came up with that really 
> are...)

At this stage, I don't have any understanding of what you're trying to
do. I've given you what I think needs to be implemented, please let me
know if that matches your understanding.

Thanks,

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

^ permalink raw reply

* [PATCH 2/3] regulator: axp20x: simplify the treatment of linked regulators
From: Jean-Francois Moine @ 2016-09-20 16:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1474616699.git.moinejf@free.fr>

Using ancillary variables for handling the linked regulators simplifies
the loop of regulator creation and makes easier the addition of new
regulator types.

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 drivers/regulator/axp20x-regulator.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 4e5e7c8..7405f5b 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -511,6 +511,10 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 	u32 workmode;
 	const char *dcdc1_name = axp22x_regulators[AXP22X_DCDC1].name;
 	const char *dcdc5_name = axp22x_regulators[AXP22X_DCDC5].name;
+	s8 dcdc1_ix = -1;
+	s8 dcdc5_ix = -1;
+	s8 dc1sw_ix = -1;
+	s8 dc5ldo_ix = -1;
 	bool drivevbus = false;
 	u32 skip_bitmap = 0;
 
@@ -524,6 +528,10 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 	case AXP223_ID:
 		regulators = axp22x_regulators;
 		nregulators = AXP22X_REG_ID_MAX;
+		dcdc1_ix = AXP22X_DCDC1;
+		dcdc5_ix = AXP22X_DCDC5;
+		dc1sw_ix = AXP22X_DC1SW;
+		dc5ldo_ix = AXP22X_DC5LDO;
 		drivevbus = of_property_read_bool(pdev->dev.parent->of_node,
 						  "x-powers,drive-vbus-en");
 		break;
@@ -541,6 +549,10 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 	case AXP809_ID:
 		regulators = axp809_regulators;
 		nregulators = AXP809_REG_ID_MAX;
+		dcdc1_ix = AXP809_DCDC1;
+		dcdc5_ix = AXP809_DCDC5;
+		dc1sw_ix = AXP809_DC1SW;
+		dc5ldo_ix = AXP809_DC5LDO;
 		break;
 	default:
 		dev_err(&pdev->dev, "Unsupported AXP variant: %ld\n",
@@ -567,8 +579,7 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 		 * part of this loop to see where we save the DT defined
 		 * name.
 		 */
-		if ((regulators == axp22x_regulators && i == AXP22X_DC1SW) ||
-		    (regulators == axp809_regulators && i == AXP809_DC1SW)) {
+		if (i == dc1sw_ix && dcdc1_name) {
 			new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
 						GFP_KERNEL);
 			*new_desc = regulators[i];
@@ -576,8 +587,7 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 			desc = new_desc;
 		}
 
-		if ((regulators == axp22x_regulators && i == AXP22X_DC5LDO) ||
-		    (regulators == axp809_regulators && i == AXP809_DC5LDO)) {
+		if (i == dc5ldo_ix && dcdc5_name) {
 			new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
 						GFP_KERNEL);
 			*new_desc = regulators[i];
@@ -605,14 +615,12 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 		/*
 		 * Save AXP22X DCDC1 / DCDC5 regulator names for later.
 		 */
-		if ((regulators == axp22x_regulators && i == AXP22X_DCDC1) ||
-		    (regulators == axp809_regulators && i == AXP809_DCDC1))
+		if (i == dcdc1_ix)
 			of_property_read_string(rdev->dev.of_node,
 						"regulator-name",
 						&dcdc1_name);
 
-		if ((regulators == axp22x_regulators && i == AXP22X_DCDC5) ||
-		    (regulators == axp809_regulators && i == AXP809_DCDC5))
+		if (i == dcdc5_ix)
 			of_property_read_string(rdev->dev.of_node,
 						"regulator-name",
 						&dcdc5_name);
-- 
2.10.0

^ permalink raw reply related

* [PATCH] pinctrl: mvebu: orion5x: Generalise mv88f5181l support for 88f5181
From: Gregory CLEMENT @ 2016-09-20 16:26 UTC (permalink / raw)
  To: linux-arm-kernel

From: Jamie Lentin <jm@lentin.co.uk>

As far as I'm aware the mv88f5181-b1 and mv88f5181l are the same at the
pinctrl level, so re-use the definitions for both.

[gregory.clement at free-electrons.com: fix commit title]
Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 .../bindings/pinctrl/marvell,orion-pinctrl.txt     |  4 +++-
 drivers/pinctrl/mvebu/pinctrl-orion.c              | 23 +++++++++++-----------
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/marvell,orion-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/marvell,orion-pinctrl.txt
index 27570a3a1741..ec8aa3c6936b 100644
--- a/Documentation/devicetree/bindings/pinctrl/marvell,orion-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/marvell,orion-pinctrl.txt
@@ -4,7 +4,9 @@ Please refer to marvell,mvebu-pinctrl.txt in this directory for common binding
 part and usage.
 
 Required properties:
-- compatible: "marvell,88f5181l-pinctrl", "marvell,88f5182-pinctrl",
+- compatible: "marvell,88f5181-pinctrl",
+              "marvell,88f5181l-pinctrl",
+              "marvell,88f5182-pinctrl",
               "marvell,88f5281-pinctrl"
 
 - reg: two register areas, the first one describing the first two
diff --git a/drivers/pinctrl/mvebu/pinctrl-orion.c b/drivers/pinctrl/mvebu/pinctrl-orion.c
index 345c3df669a0..84e144167b44 100644
--- a/drivers/pinctrl/mvebu/pinctrl-orion.c
+++ b/drivers/pinctrl/mvebu/pinctrl-orion.c
@@ -64,11 +64,11 @@ static int orion_mpp_ctrl_set(unsigned pid, unsigned long config)
 	return 0;
 }
 
-#define V(f5181l, f5182, f5281) \
-	((f5181l << 0) | (f5182 << 1) | (f5281 << 2))
+#define V(f5181, f5182, f5281) \
+	((f5181 << 0) | (f5182 << 1) | (f5281 << 2))
 
 enum orion_variant {
-	V_5181L = V(1, 0, 0),
+	V_5181  = V(1, 0, 0),
 	V_5182  = V(0, 1, 0),
 	V_5281  = V(0, 0, 1),
 	V_ALL   = V(1, 1, 1),
@@ -103,13 +103,13 @@ static struct mvebu_mpp_mode orion_mpp_modes[] = {
 		 MPP_VAR_FUNCTION(0x0, "gpio", NULL,        V_ALL),
 		 MPP_VAR_FUNCTION(0x2, "pci", "req5",       V_ALL),
 		 MPP_VAR_FUNCTION(0x4, "nand", "re0",       V_5182 | V_5281),
-		 MPP_VAR_FUNCTION(0x5, "pci-1", "clk",      V_5181L),
+		 MPP_VAR_FUNCTION(0x5, "pci-1", "clk",      V_5181),
 		 MPP_VAR_FUNCTION(0x5, "sata0", "act",      V_5182)),
 	MPP_MODE(7,
 		 MPP_VAR_FUNCTION(0x0, "gpio", NULL,        V_ALL),
 		 MPP_VAR_FUNCTION(0x2, "pci", "gnt5",       V_ALL),
 		 MPP_VAR_FUNCTION(0x4, "nand", "we0",       V_5182 | V_5281),
-		 MPP_VAR_FUNCTION(0x5, "pci-1", "clk",      V_5181L),
+		 MPP_VAR_FUNCTION(0x5, "pci-1", "clk",      V_5181),
 		 MPP_VAR_FUNCTION(0x5, "sata1", "act",      V_5182)),
 	MPP_MODE(8,
 		 MPP_VAR_FUNCTION(0x0, "gpio", NULL,        V_ALL),
@@ -165,7 +165,7 @@ static struct mvebu_mpp_ctrl orion_mpp_controls[] = {
 	MPP_FUNC_CTRL(0, 19, NULL, orion_mpp_ctrl),
 };
 
-static struct pinctrl_gpio_range mv88f5181l_gpio_ranges[] = {
+static struct pinctrl_gpio_range mv88f5181_gpio_ranges[] = {
 	MPP_GPIO_RANGE(0, 0, 0, 16),
 };
 
@@ -177,14 +177,14 @@ static struct pinctrl_gpio_range mv88f5281_gpio_ranges[] = {
 	MPP_GPIO_RANGE(0, 0, 0, 16),
 };
 
-static struct mvebu_pinctrl_soc_info mv88f5181l_info = {
-	.variant = V_5181L,
+static struct mvebu_pinctrl_soc_info mv88f5181_info = {
+	.variant = V_5181,
 	.controls = orion_mpp_controls,
 	.ncontrols = ARRAY_SIZE(orion_mpp_controls),
 	.modes = orion_mpp_modes,
 	.nmodes = ARRAY_SIZE(orion_mpp_modes),
-	.gpioranges = mv88f5181l_gpio_ranges,
-	.ngpioranges = ARRAY_SIZE(mv88f5181l_gpio_ranges),
+	.gpioranges = mv88f5181_gpio_ranges,
+	.ngpioranges = ARRAY_SIZE(mv88f5181_gpio_ranges),
 };
 
 static struct mvebu_pinctrl_soc_info mv88f5182_info = {
@@ -212,7 +212,8 @@ static struct mvebu_pinctrl_soc_info mv88f5281_info = {
  * muxing, they are identical.
  */
 static const struct of_device_id orion_pinctrl_of_match[] = {
-	{ .compatible = "marvell,88f5181l-pinctrl", .data = &mv88f5181l_info },
+	{ .compatible = "marvell,88f5181-pinctrl", .data = &mv88f5181_info },
+	{ .compatible = "marvell,88f5181l-pinctrl", .data = &mv88f5181_info },
 	{ .compatible = "marvell,88f5182-pinctrl", .data = &mv88f5182_info },
 	{ .compatible = "marvell,88f5281-pinctrl", .data = &mv88f5281_info },
 	{ }
-- 
2.9.3

^ permalink raw reply related

* [PATCH] clk: mvebu: Add clk support for the orion5x SoC mv88f5181
From: Gregory CLEMENT @ 2016-09-20 16:24 UTC (permalink / raw)
  To: linux-arm-kernel

From: Jamie Lentin <jm@lentin.co.uk>

Referring to the u-boot sources for the Netgear WNR854T, add support
for the mv88f5181.

[gregory.clement at free-electrons.com: fix commit title]
Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---

Hi Stephen and Mike,

do you agree to give your acked-by on this patch. It is part of a
convertion of old orion5x Socv to the device tree. If you acked-by
this one, then I will be able to take it in my tree and avoiding
breaking the git bisect.

This patch is really about adding a new variant, teh logic remains the
same and also it won't produce any conflict.

Sorry to not have asked this before.

Thanks,

Gregory


 .../devicetree/bindings/clock/mvebu-core-clock.txt |  1 +
 drivers/clk/mvebu/orion.c                          | 70 ++++++++++++++++++++++
 2 files changed, 71 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt b/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
index 670c2af3e931..eb985a633d59 100644
--- a/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
+++ b/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
@@ -52,6 +52,7 @@ Required properties:
 	"marvell,dove-core-clock" - for Dove SoC core clocks
 	"marvell,kirkwood-core-clock" - for Kirkwood SoC (except mv88f6180)
 	"marvell,mv88f6180-core-clock" - for Kirkwood MV88f6180 SoC
+	"marvell,mv88f5181-core-clock" - for Orion MV88F5181 SoC
 	"marvell,mv88f5182-core-clock" - for Orion MV88F5182 SoC
 	"marvell,mv88f5281-core-clock" - for Orion MV88F5281 SoC
 	"marvell,mv88f6183-core-clock" - for Orion MV88F6183 SoC
diff --git a/drivers/clk/mvebu/orion.c b/drivers/clk/mvebu/orion.c
index fd129566c1ce..a6e5bee23385 100644
--- a/drivers/clk/mvebu/orion.c
+++ b/drivers/clk/mvebu/orion.c
@@ -21,6 +21,76 @@ static const struct coreclk_ratio orion_coreclk_ratios[] __initconst = {
 };
 
 /*
+ * Orion 5181
+ */
+
+#define SAR_MV88F5181_TCLK_FREQ      8
+#define SAR_MV88F5181_TCLK_FREQ_MASK 0x3
+
+static u32 __init mv88f5181_get_tclk_freq(void __iomem *sar)
+{
+	u32 opt = (readl(sar) >> SAR_MV88F5181_TCLK_FREQ) &
+		SAR_MV88F5181_TCLK_FREQ_MASK;
+	if (opt == 0)
+		return 133333333;
+	else if (opt == 1)
+		return 150000000;
+	else if (opt == 2)
+		return 166666667;
+	else
+		return 0;
+}
+
+#define SAR_MV88F5181_CPU_FREQ       4
+#define SAR_MV88F5181_CPU_FREQ_MASK  0xf
+
+static u32 __init mv88f5181_get_cpu_freq(void __iomem *sar)
+{
+	u32 opt = (readl(sar) >> SAR_MV88F5181_CPU_FREQ) &
+		SAR_MV88F5181_CPU_FREQ_MASK;
+	if (opt == 0)
+		return 333333333;
+	else if (opt == 1 || opt == 2)
+		return 400000000;
+	else if (opt == 3)
+		return 500000000;
+	else
+		return 0;
+}
+
+static void __init mv88f5181_get_clk_ratio(void __iomem *sar, int id,
+					   int *mult, int *div)
+{
+	u32 opt = (readl(sar) >> SAR_MV88F5181_CPU_FREQ) &
+		SAR_MV88F5181_CPU_FREQ_MASK;
+	if (opt == 0 || opt == 1) {
+		*mult = 1;
+		*div  = 2;
+	} else if (opt == 2 || opt == 3) {
+		*mult = 1;
+		*div  = 3;
+	} else {
+		*mult = 0;
+		*div  = 1;
+	}
+}
+
+static const struct coreclk_soc_desc mv88f5181_coreclks = {
+	.get_tclk_freq = mv88f5181_get_tclk_freq,
+	.get_cpu_freq = mv88f5181_get_cpu_freq,
+	.get_clk_ratio = mv88f5181_get_clk_ratio,
+	.ratios = orion_coreclk_ratios,
+	.num_ratios = ARRAY_SIZE(orion_coreclk_ratios),
+};
+
+static void __init mv88f5181_clk_init(struct device_node *np)
+{
+	return mvebu_coreclk_setup(np, &mv88f5181_coreclks);
+}
+
+CLK_OF_DECLARE(mv88f5181_clk, "marvell,mv88f5181-core-clock", mv88f5181_clk_init);
+
+/*
  * Orion 5182
  */
 
-- 
2.9.3

^ permalink raw reply related

* [PATCH v5 02/16] dt/bindings: Update binding for PM domain idle states
From: Lina Iyer @ 2016-09-20 16:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <874m5cx6y8.fsf@arm.com>

On Mon, Sep 19 2016 at 09:09 -0600, Brendan Jackman wrote:
>
>On Fri, Sep 16 2016 at 18:39, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> Hi Kevin,
>>
>> Thanks for looking at this and simplifying various discussions we had so
>> far. I was thinking of summarizing something very similar. I couldn't
>> due to lack of time.
>>
>> On 16/09/16 18:13, Kevin Hilman wrote:
>>
>> [...]
>>
>>> I think we're having some terminology issues...
>>>
>>> FWIW, the kernel terminolgy is actually "PM domain", not power domain.
>>> This was intentional because the goal of the PM domain was to group
>>> devices that some PM features.  To be very specific to the kernel, they
>>> us the same set of PM callbacks.  Today, this is most commonly used to
>>> model power domains, where a group of devices share a power rail, but it
>>> does not need to be limited to that.
>>>
>>
>> Agreed/Understood.
>>
>>> That being said, I'm having a hard time understanding the root of the
>>> disagreement.
>>>
>>
>> Yes. I tried to convey the same earlier, but have failed. The only
>> disagreement is about a small part of this DT bindings. We would like to
>> make it completely hierarchical up to CPU nodes. More comments on that
>> below.
>>
>>> It seems that you and Sudeep would like to use domain-idle-states to
>>> replace/superceed cpu-idle-states with the primary goal (and benefit)
>>> being that it simplifies the DT bindings.  Is that correct?
>>>
>>
>> Correct, we want to deprecate cpu-idle-states with the introduction of
>> this hierarchical PM bindings. Yes IMO, it simplifies things and avoids
>> any ABI break we might trigger if we miss to consider some use-case now.
>>
>>> The objections have come in because that means that implies that CPUs
>>> become their own domains, which may not be the case in hardware in the
>>> sense that they share a power rail.
>>>
>>
>> Agreed.
>>
>>> However, IMO, thinking of a CPU as it's own "PM domain" may make some
>>> sense based on the terminology above.
>>>
>>
>> Thanks for that, we do understand that it may not be 100% correct when
>> we strictly considers hardware terminologies instead of above ones.
>> As along as we see no issues with the above terminologies it should be fine.
>>
>>> I think the other objection may be that using a genpd to model domain
>>> with only a single device in it may be overkill, and I agree with that.
>>
>> I too agree with that. Just because we represent that in DT in that way
>> doesn't mean we need to create a genpd to model domain. We can always
>> skip that if not required. That's pure implementation specifics and I
>> have tried to convey the same in my previous emails. I must say you have
>> summarized it very clearly in this email. Thanks again for that.
>>
>>> But, I'm not sure if making CPUs use domain-idle-states implies that
>>> they necessarily have to use genpd is what you are proposing.  Maybe
>>> someone could clarify that?
>>>
>>
>> No, I have not proposing anything around implementation in the whole
>> discussion so far. I have constrained myself just to DT bindings so far.
>> That's the main reason why I was opposed to mentions of OS vs platform
>> co-ordinated modes of CPU suspend in this discussion. IMO that's
>> completely out of scope of this DT binding we are defining here.
>>
Fair. But understand the PM Domain bindings do not impose any
requirements of hierarchy. Domain idle states are defined by the
property domain-idle-states in the domain node. How the DT bindings are
organized is immaterial to the PM Domain core.

It is a different exercise all together to look at CPU PSCI modes and
have a unified way of representing them in DT. The current set of
patches does not dictate where the domain idle states be located (pardon
my example in the patch, which was not updated to reflect that). That
said, I do require that domains that are controlled by the PSCI f/w be
defined under the 'psci' node in DT, which is fair. All the domain needs
are phandles to the idle state definitions; how the nodes are arranged
in DT is not of consequence to the driver.

In my mind providing a structure to CPU PM domains that can be used for
both OSI and PC is a separate effort. It may also club what Brendan
mentions below as part of the effort. The hierarchy that is presented in
[1] is inherent in the PM domain hierarchy and idle states don't have to
duplicate that information.

>> Hope that helps/clarifies the misunderstanding/disagreement.
>
>Indeed. My intention was that the proposal would result in the exact
>same kernel behaviour as Lina's current patchset, i.e. there is one
>genpd per cluster, and CPU-level idle states are still handled by
>cpuidle.
>
>The only change from the current patchset would be in initialisation
>code: some coordination would need to be done to determine which idle
>states go into cpuidle and which go into the genpds (whereas with the
>current bindings, states from cpu-idle-states go into cpuidle and states
>from domain-idle-states go into genpd). So you could say that this would
>be a trade-off between binding simplicity and implementation simplicity.
>
I would not oppose the idea of virtual domains around CPUs (I admit I am
not comfortable with the idea though), if that is the right thing to do.
But the scope of that work is extensive and should not be clubbed as
part of this proposal. It is an extensive code rework spanning cpuidle
drivers and PSCI and there are hooks in this code to help you achieve
that.

Thanks,
Lina

[1]. https://patchwork.kernel.org/patch/9264507/

^ permalink raw reply

* [GIT PULL] ARM: mvebu: fixes for v4.8 (#3)
From: Gregory CLEMENT @ 2016-09-20 16:14 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

Here is the third pull request for fixes for mvebu for v4.8.

Gregory

The following changes since commit c721da1d05760ad0b4e7670896dae31b6b07d8d6:

  ARM: dts: kirkwood: Fix PCIe label on OpenRD (2016-08-26 11:33:16 +0200)

are available in the git repository at:

  git://git.infradead.org/linux-mvebu.git tags/mvebu-fixes-4.8-3

for you to fetch changes up to 51227bf52008bd4c4c50da4b749bbc6e7bbbca52:

  arm64: dts: marvell: fix clocksource for CP110 master SPI0 (2016-09-20 16:55:12 +0200)

----------------------------------------------------------------
mvebu fixes for 4.8 (part 3)

- Select corediv clk for all mvebu v7 SoC
- Fix clocksource for CP110 master SPI0 for Armada 7K/8K

----------------------------------------------------------------
Gregory CLEMENT (1):
      ARM: mvebu: Select corediv clk for all mvebu v7 SoC

Marcin Wojtas (1):
      arm64: dts: marvell: fix clocksource for CP110 master SPI0

 arch/arm/mach-mvebu/Kconfig                          | 4 +---
 arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi | 2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)

^ permalink raw reply

* [PATCH v9 08/10] arm64: pmu: Detect and enable multiple PMUs in an ACPI system
From: Punit Agrawal @ 2016-09-20 16:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <b8b27aa5-e80e-f7cc-3410-4b2ac44c69e5@arm.com>

Jeremy Linton <jeremy.linton@arm.com> writes:

> On 09/16/2016 12:07 PM, Punit Agrawal wrote:
>> Jeremy Linton <jeremy.linton@arm.com> writes:
>>
>>> Hi,
>>>
>>> On 09/16/2016 08:33 AM, Punit Agrawal wrote:
>>>> Jeremy Linton <jeremy.linton@arm.com> writes:
>>>>
>>>>> Its possible that an ACPI system has multiple CPU types in it
>>>>> with differing PMU counters. Iterate the CPU's and make a determination
>>>>> about how many of each type exist in the system. Then take and create
>>>>> a PMU platform device for each type, and assign it the interrupts parsed
>>>>> from the MADT. Creating a platform device is necessary because the PMUs
>>>>> are not described as devices in the DSDT table.
>>>>>
>>>>> This code is loosely based on earlier work by Mark Salter.
>>>
>>> (trimming)
>>>
>>>>> +
>>>>> +	list_for_each_entry_safe(pmu, safe_temp, &pmus, list) {
>>>>> +		if (unused_madt_entries)
>>>>> +			pmu->cpu_count = num_possible_cpus();
>>>>
>>>> So if there is any unbooted cpu ...
>>>>
>>>>> +
>>>>> +		res = kcalloc(pmu->cpu_count,
>>>>> +				  sizeof(struct resource), GFP_KERNEL);
>>>>
>>>> ... we allocate potentially large number (num_possible_cpus()) of
>>>> resources for each PMU.
>>>>
>>>> This is needlessly wasteful. Under what conditions have you found
>>>> reg_midr to be 0?
>>>
>>> Unused MADT entries, in place for potentially unbooted/hotplug
>>> CPUs.
>>
>> Is linux able to deal with booting secondaries that have unused MADT
>> entries?
>
> Uh, yah I think so, that is how i've been testing it, maybe we are
> saying different things. When i'm talking about "unused" I mean MADT
> entries that don't have started/running cpus. Obviously they stop
> being unused when a cpu is booted.
>
>>
>>> In those cases you don't know for sure which PMU the CPU belongs
>>> to until it comes online and the MIDR can be read. I'm open to
>>> suggestions on how to deal with this, outside of pushing my luck and
>>> further breaking the platform device encapsulation by trying to
>>> reallocate the resource structure while its active. Besides its only
>>> wasteful for ACPI+big.little, which at the moment only applies to a
>>> development platform.
>>
>> I don't have any ideas to solve this problem, but in the interest of
>> helping review, please move all the changes arising from hotplug/unused
>> MADT entries in this patch to the next one.
>
> Which is sort of the opposite of the last 3 months, of complaints
> about how hard it was to review this module with multiple patches
> adding features to the code...

My suggestion to move functionality that has concerns and needs rework
to later patch is aimed at helping get stuff that everybody agrees upon
merged. But it's only a suggestion...

> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH 1/3] regulator: axp20x: simplify poly-phase handling
From: Jean-Francois Moine @ 2016-09-20 16:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1474616699.git.moinejf@free.fr>

Building a list (bitmap) of the slaves included in poly-phase groups
at probe startup time simplifies the treatment in the regulator
creation loop.

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 drivers/regulator/axp20x-regulator.c | 45 +++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 24 deletions(-)

diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 54382ef..4e5e7c8 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -477,30 +477,24 @@ static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 work
 }
 
 /*
- * This function checks whether a regulator is part of a poly-phase
- * output setup based on the registers settings. Returns true if it is.
+ * This function checks which regulators are part of poly-phase
+ * output setups based on the registers settings.
+ * Returns a bitmap of these regulators.
  */
-static bool axp20x_is_polyphase_slave(struct axp20x_dev *axp20x, int id)
+static u32 axp20x_polyphase_slave(struct axp20x_dev *axp20x)
 {
-	u32 reg = 0;
-
-	/* Only AXP806 has poly-phase outputs */
-	if (axp20x->variant != AXP806_ID)
-		return false;
+	u32 reg = 0, bitmap = 0;
 
 	regmap_read(axp20x->regmap, AXP806_DCDC_MODE_CTRL2, &reg);
 
-	switch (id) {
-	case AXP806_DCDCB:
-		return (((reg & GENMASK(7, 6)) == BIT(6)) ||
-			((reg & GENMASK(7, 6)) == BIT(7)));
-	case AXP806_DCDCC:
-		return ((reg & GENMASK(7, 6)) == BIT(7));
-	case AXP806_DCDCE:
-		return !!(reg & BIT(5));
-	}
+	if ((reg & GENMASK(7, 6)) == BIT(5))
+		bitmap |= 1 << AXP806_DCDCE;
+	if ((reg & GENMASK(7, 6)) == BIT(6))
+		bitmap |= 1 << AXP806_DCDCB;
+	if ((reg & GENMASK(7, 6)) == BIT(7))
+		bitmap |= (1 << AXP806_DCDCB) | (1 << AXP806_DCDCC);
 
-	return false;
+	return bitmap;
 }
 
 static int axp20x_regulator_probe(struct platform_device *pdev)
@@ -518,6 +512,7 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 	const char *dcdc1_name = axp22x_regulators[AXP22X_DCDC1].name;
 	const char *dcdc5_name = axp22x_regulators[AXP22X_DCDC5].name;
 	bool drivevbus = false;
+	u32 skip_bitmap = 0;
 
 	switch (axp20x->variant) {
 	case AXP202_ID:
@@ -535,6 +530,13 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 	case AXP806_ID:
 		regulators = axp806_regulators;
 		nregulators = AXP806_REG_ID_MAX;
+
+		/*
+		 * The regulators which are slave in a poly-phase setup
+		 * are skipped, as their controls are bound to the master
+		 * regulator and won't work.
+		 */
+		skip_bitmap |= axp20x_polyphase_slave(axp20x);
 		break;
 	case AXP809_ID:
 		regulators = axp809_regulators;
@@ -553,12 +555,7 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 		const struct regulator_desc *desc = &regulators[i];
 		struct regulator_desc *new_desc;
 
-		/*
-		 * If this regulator is a slave in a poly-phase setup,
-		 * skip it, as its controls are bound to the master
-		 * regulator and won't work.
-		 */
-		if (axp20x_is_polyphase_slave(axp20x, i))
+		if (skip_bitmap & (1 << i))
 			continue;
 
 		/*
-- 
2.10.0

^ permalink raw reply related

* [PATCH v6 4/4] ARM: dts: Add EXTI controller node to stm32f429
From: Alexandre TORGUE @ 2016-09-20 16:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474387259-18926-1-git-send-email-alexandre.torgue@st.com>

Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Signed-off-by: Alexandre TORGUE <alexandre.torgue@st.com>

diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index 35df462..1a189d4 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -176,6 +176,14 @@
 			reg = <0x40013800 0x400>;
 		};
 
+		exti: interrupt-controller at 40013c00 {
+			compatible = "st,stm32-exti";
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			reg = <0x40013C00 0x400>;
+			interrupts = <1>, <2>, <3>, <6>, <7>, <8>, <9>, <10>, <23>, <40>, <41>, <42>, <62>, <76>;
+		};
+
 		pin-controller {
 			#address-cells = <1>;
 			#size-cells = <1>;
-- 
1.9.1

^ permalink raw reply related

* [PATCH v6 3/4] ARM: STM32: Select external interrupts controller
From: Alexandre TORGUE @ 2016-09-20 16:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474387259-18926-1-git-send-email-alexandre.torgue@st.com>

Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Signed-off-by: Alexandre TORGUE <alexandre.torgue@st.com>

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 2d601d7..157cea9 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -877,6 +877,7 @@ config ARCH_STM32
 	select CLKSRC_STM32
 	select PINCTRL
 	select RESET_CONTROLLER
+	select STM32_EXTI
 	help
 	  Support for STMicroelectronics STM32 processors.
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH v6 2/4] drivers: irqchip: Add STM32 external interrupts support
From: Alexandre TORGUE @ 2016-09-20 16:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474387259-18926-1-git-send-email-alexandre.torgue@st.com>

The STM32 external interrupt controller consists of edge detectors that
generate interrupts requests or wake-up events.

Each line can be independently configured as interrupt or wake-up source,
and triggers either on rising, falling or both edges. Each line can also
be masked independently.

Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Signed-off-by: Alexandre TORGUE <alexandre.torgue@st.com>

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 7f87289..bc62d1f 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -264,3 +264,7 @@ config EZNPS_GIC
 	select IRQ_DOMAIN
 	help
 	  Support the EZchip NPS400 global interrupt controller
+
+config STM32_EXTI
+	bool
+	select IRQ_DOMAIN
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 4c203b6..96383b2 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -71,3 +71,4 @@ obj-$(CONFIG_MVEBU_ODMI)		+= irq-mvebu-odmi.o
 obj-$(CONFIG_LS_SCFG_MSI)		+= irq-ls-scfg-msi.o
 obj-$(CONFIG_EZNPS_GIC)			+= irq-eznps.o
 obj-$(CONFIG_ARCH_ASPEED)		+= irq-aspeed-vic.o
+obj-$(CONFIG_STM32_EXTI) 		+= irq-stm32-exti.o
diff --git a/drivers/irqchip/irq-stm32-exti.c b/drivers/irqchip/irq-stm32-exti.c
new file mode 100644
index 0000000..491568c
--- /dev/null
+++ b/drivers/irqchip/irq-stm32-exti.c
@@ -0,0 +1,201 @@
+/*
+ * Copyright (C) Maxime Coquelin 2015
+ * Author:  Maxime Coquelin <mcoquelin.stm32@gmail.com>
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+
+#include <linux/bitops.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+#define EXTI_IMR	0x0
+#define EXTI_EMR	0x4
+#define EXTI_RTSR	0x8
+#define EXTI_FTSR	0xc
+#define EXTI_SWIER	0x10
+#define EXTI_PR		0x14
+
+static void stm32_irq_handler(struct irq_desc *desc)
+{
+	struct irq_domain *domain = irq_desc_get_handler_data(desc);
+	struct irq_chip_generic *gc = domain->gc->gc[0];
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	unsigned long pending;
+	int n;
+
+	chained_irq_enter(chip, desc);
+
+	while ((pending = irq_reg_readl(gc, EXTI_PR))) {
+		for_each_set_bit(n, &pending, BITS_PER_LONG) {
+			generic_handle_irq(irq_find_mapping(domain, n));
+			irq_reg_writel(gc, BIT(n), EXTI_PR);
+		}
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+static int stm32_irq_set_type(struct irq_data *data, unsigned int type)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
+	int pin = data->hwirq;
+	u32 rtsr, ftsr;
+
+	irq_gc_lock(gc);
+
+	rtsr = irq_reg_readl(gc, EXTI_RTSR);
+	ftsr = irq_reg_readl(gc, EXTI_FTSR);
+
+	switch (type) {
+	case IRQ_TYPE_EDGE_RISING:
+		rtsr |= BIT(pin);
+		ftsr &= ~BIT(pin);
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		rtsr &= ~BIT(pin);
+		ftsr |= BIT(pin);
+		break;
+	case IRQ_TYPE_EDGE_BOTH:
+		rtsr |= BIT(pin);
+		ftsr |= BIT(pin);
+		break;
+	default:
+		irq_gc_unlock(gc);
+		return -EINVAL;
+	}
+
+	irq_reg_writel(gc, rtsr, EXTI_RTSR);
+	irq_reg_writel(gc, ftsr, EXTI_FTSR);
+
+	irq_gc_unlock(gc);
+
+	return 0;
+}
+
+static int stm32_irq_set_wake(struct irq_data *data, unsigned int on)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
+	int pin = data->hwirq;
+	u32 emr;
+
+	irq_gc_lock(gc);
+
+	emr = irq_reg_readl(gc, EXTI_EMR);
+	if (on)
+		emr |= BIT(pin);
+	else
+		emr &= ~BIT(pin);
+	irq_reg_writel(gc, emr, EXTI_EMR);
+
+	irq_gc_unlock(gc);
+
+	return 0;
+}
+
+static int stm32_exti_alloc(struct irq_domain *d, unsigned int virq,
+			    unsigned int nr_irqs, void *data)
+{
+	struct irq_chip_generic *gc = d->gc->gc[0];
+	struct irq_fwspec *fwspec = data;
+	irq_hw_number_t hwirq;
+
+	hwirq = fwspec->param[0];
+
+	irq_map_generic_chip(d, virq, hwirq);
+	irq_domain_set_info(d, virq, hwirq, &gc->chip_types->chip, gc,
+			    handle_simple_irq, NULL, NULL);
+
+	return 0;
+}
+
+static void stm32_exti_free(struct irq_domain *d, unsigned int virq,
+			    unsigned int nr_irqs)
+{
+	struct irq_data *data = irq_domain_get_irq_data(d, virq);
+
+	irq_domain_reset_irq_data(data);
+}
+
+struct irq_domain_ops irq_exti_domain_ops = {
+	.map	= irq_map_generic_chip,
+	.xlate	= irq_domain_xlate_onetwocell,
+	.alloc  = stm32_exti_alloc,
+	.free	= stm32_exti_free,
+};
+
+static int __init stm32_exti_init(struct device_node *node,
+				  struct device_node *parent)
+{
+	unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
+	int nr_irqs, nr_exti, ret, i;
+	struct irq_chip_generic *gc;
+	struct irq_domain *domain;
+	void *base;
+
+	base = of_iomap(node, 0);
+	if (!base) {
+		pr_err("%s: Unable to map registers\n", node->full_name);
+		return -ENOMEM;
+	}
+
+	/* Determine number of irqs supported */
+	writel_relaxed(~0UL, base + EXTI_RTSR);
+	nr_exti = fls(readl_relaxed(base + EXTI_RTSR));
+	writel_relaxed(0, base + EXTI_RTSR);
+
+	pr_info("%s: %d External IRQs detected\n", node->full_name, nr_exti);
+
+	domain = irq_domain_add_linear(node, nr_exti,
+				       &irq_exti_domain_ops, NULL);
+	if (!domain) {
+		pr_err("%s: Could not register interrupt domain.\n",
+				node->name);
+		ret = -ENOMEM;
+		goto out_unmap;
+	}
+
+	ret = irq_alloc_domain_generic_chips(domain, nr_exti, 1, "exti",
+					     handle_edge_irq, clr, 0, 0);
+	if (ret) {
+		pr_err("%s: Could not allocate generic interrupt chip.\n",
+			node->full_name);
+		goto out_free_domain;
+	}
+
+	gc = domain->gc->gc[0];
+	gc->reg_base                         = base;
+	gc->chip_types->type               = IRQ_TYPE_EDGE_BOTH;
+	gc->chip_types->chip.name          = gc->chip_types[0].chip.name;
+	gc->chip_types->chip.irq_ack       = irq_gc_ack_set_bit;
+	gc->chip_types->chip.irq_mask      = irq_gc_mask_clr_bit;
+	gc->chip_types->chip.irq_unmask    = irq_gc_mask_set_bit;
+	gc->chip_types->chip.irq_set_type  = stm32_irq_set_type;
+	gc->chip_types->chip.irq_set_wake  = stm32_irq_set_wake;
+	gc->chip_types->regs.ack           = EXTI_PR;
+	gc->chip_types->regs.mask          = EXTI_IMR;
+	gc->chip_types->handler            = handle_edge_irq;
+
+	nr_irqs = of_irq_count(node);
+	for (i = 0; i < nr_irqs; i++) {
+		unsigned int irq = irq_of_parse_and_map(node, i);
+
+		irq_set_handler_data(irq, domain);
+		irq_set_chained_handler(irq, stm32_irq_handler);
+	}
+
+	return 0;
+
+out_free_domain:
+	irq_domain_remove(domain);
+out_unmap:
+	iounmap(base);
+	return ret;
+}
+
+IRQCHIP_DECLARE(stm32_exti, "st,stm32-exti", stm32_exti_init);
-- 
1.9.1

^ permalink raw reply related

* [PATCH v6 1/4] Documentation: dt-bindings: Document STM32 EXTI controller bindings
From: Alexandre TORGUE @ 2016-09-20 16:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474387259-18926-1-git-send-email-alexandre.torgue@st.com>

Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Signed-off-by: Alexandre TORGUE <alexandre.torgue@st.com>

diff --git a/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.txt b/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.txt
new file mode 100644
index 0000000..6e7703d
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.txt
@@ -0,0 +1,20 @@
+STM32 External Interrupt Controller
+
+Required properties:
+
+- compatible: Should be "st,stm32-exti"
+- reg: Specifies base physical address and size of the registers
+- interrupt-controller: Indentifies the node as an interrupt controller
+- #interrupt-cells: Specifies the number of cells to encode an interrupt
+  specifier, shall be 2
+- interrupts: interrupts references to primary interrupt controller
+
+Example:
+
+exti: interrupt-controller at 40013c00 {
+	compatible = "st,stm32-exti";
+	interrupt-controller;
+	#interrupt-cells = <2>;
+	reg = <0x40013C00 0x400>;
+	interrupts = <1>, <2>, <3>, <6>, <7>, <8>, <9>, <10>, <23>, <40>, <41>, <42>, <62>, <76>;
+};
-- 
1.9.1

^ permalink raw reply related

* [PATCH v6 0/4] Add STM32 EXTI interrupt controller support
From: Alexandre TORGUE @ 2016-09-20 16:00 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

This v6 fix useless mask in .free callback of stm32 exti driver (thanks
Thomas).
I resend only patches concerning STM32 EXTI driver (as pinctrl part has
already been applied by Linus W.)


The series adds support to EXTI interrupt controller and GPIO IRQ support in
STM32 pinctrl driver.

The STM32 external interrupt controller consists of edge detectors that
generate interrupts requests or wake-up events.

Each line can be independently configured as interrupt or wake-up source,
and triggers either on rising, fallin or both edges. Each line can also
be masked independently.

Regards 

Alex

Changes since v5:
-----------------
 - Fix .free callback according to Thomas

Changes since v4:
-----------------
 - Fix bad copy/paste in stm32 exti driver
 - Remove GPIOLIB_IRQCHIP config in stm32 pinctrl driver 

Changes since v3:
-----------------
 - Review domain dealloc/free irq in stm32 pinctrl driver
 - Review domain dealloc/free irq in stm32 exti driver
 - Fix remarks on coding style

Changes since v2:
-----------------
 - Define irq_chip for GPIO banks
 - Use hierarchical domain for GPIO banks
 - Improve search loop inside stm32_exti handler 
 - Rebased on top of v4.8-rc1

Changes since v1:
-----------------
 - Rebased on top of v4.6-rc1
 - Change variable name from virq to irq (Linus W.)


Alexandre TORGUE (4):
  Documentation: dt-bindings: Document STM32 EXTI controller bindings
  drivers: irqchip: Add STM32 external interrupts support
  ARM: STM32: Select external interrupts controller
  ARM: dts: Add EXTI controller node to stm32f429

 .../interrupt-controller/st,stm32-exti.txt         |  20 ++
 arch/arm/Kconfig                                   |   1 +
 arch/arm/boot/dts/stm32f429.dtsi                   |   8 +
 drivers/irqchip/Kconfig                            |   4 +
 drivers/irqchip/Makefile                           |   1 +
 drivers/irqchip/irq-stm32-exti.c                   | 201 +++++++++++++++++++++
 6 files changed, 235 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.txt
 create mode 100644 drivers/irqchip/irq-stm32-exti.c

-- 
1.9.1

^ permalink raw reply

* [PATCH v3 0/3] ARM: aspeed: add support for the BT IPMI interface
From: Cédric Le Goater @ 2016-09-20 15:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <40bb95a8-b5a6-563b-11c6-73de2f67130d@acm.org>

On 09/20/2016 04:58 PM, Corey Minyard wrote:
> On 09/20/2016 02:01 AM, C?dric Le Goater wrote:
>> Hello,
>>
>> This serie adds support for the iBT interface on Aspeed SOCs (AST2400
>> and AST2500). The BT (Block Transfer) interface is used to perform
>> in-band IPMI communication between a host and its BMC. This driver
>> implements the BMC side.
> 
> This looks good.  I pulled these into my tree, it's ok for 4.9 or
> whatever is next, since it's a new driver.
> 
> Do patches 2 and 3 go through Joel?

Yes. He said he would.

Thanks,

C. 

> 
> -corey
> 
>> Changes since v2:
>>
>>   - limit to one opener
>>   - protect write/read operations with a mutex.
>>
>> Changes since v1:
>>
>>   - the driver is now called 'bt-bmc' and the device node '/dev/ipmi-bt-host'
>>       - the code is now under drivers/char/ipmi/. This required a change of
>>     the top Makefile in drivers/ to compile the bt-bmc driver with the
>>     ipmi handlers not being selected. That might be an issue.
>>       - changed the read/write operations to use a temporary buffer and get
>>     rid of the {get,put}_user calls
>>
>> Thanks,
>>
>> C.
>>
>> Alistair Popple (1):
>>    ipmi: add an Aspeed BT IPMI BMC driver
>>
>> C?dric Le Goater (2):
>>    ARM: aspeed: Add defconfigs for CONFIG_ASPEED_BT_IPMI_BMC
>>    ARM: dts: aspeed: Enable BT IPMI BMC device
>>
>>   .../bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt   |  23 +
>>   arch/arm/boot/dts/aspeed-g4.dtsi                   |   6 +
>>   arch/arm/boot/dts/aspeed-g5.dtsi                   |   6 +
>>   arch/arm/configs/aspeed_g4_defconfig               |   1 +
>>   arch/arm/configs/aspeed_g5_defconfig               |   1 +
>>   drivers/Makefile                                   |   2 +-
>>   drivers/char/ipmi/Kconfig                          |   7 +
>>   drivers/char/ipmi/Makefile                         |   1 +
>>   drivers/char/ipmi/bt-bmc.c                         | 510 +++++++++++++++++++++
>>   include/uapi/linux/Kbuild                          |   1 +
>>   include/uapi/linux/bt-bmc.h                        |  18 +
>>   11 files changed, 575 insertions(+), 1 deletion(-)
>>   create mode 100644 Documentation/devicetree/bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt
>>   create mode 100644 drivers/char/ipmi/bt-bmc.c
>>   create mode 100644 include/uapi/linux/bt-bmc.h
>>
> 

^ permalink raw reply

* [PATCH v5 2/9] drivers: irqchip: Add STM32 external interrupts support
From: Thomas Gleixner @ 2016-09-20 15:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2095bf33-2940-3294-afd6-a28a48762569@st.com>

On Tue, 20 Sep 2016, Alexandre Torgue wrote:
> so if you agree I will resend only patches concerning stm32 exti driver
> [1],[2],[3],[4]

Ok.
 
> Thanks for your time.

Welcome,

	tglx

^ permalink raw reply

* [PATCH v5 2/9] drivers: irqchip: Add STM32 external interrupts support
From: Alexandre Torgue @ 2016-09-20 15:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <alpine.DEB.2.20.1609201600490.6905@nanos>

Thomas,

On 09/20/2016 04:02 PM, Thomas Gleixner wrote:
> On Tue, 20 Sep 2016, Alexandre Torgue wrote:
>> On 09/20/2016 02:44 PM, Thomas Gleixner wrote:
>>> Free will be called when a interrupt in the child domain is torn down,
>>> i.e. when irq_domain_free_irqs() is called. And it will be called for both
>>> domains like the alloc callback is invoked on both domains via
>>> irq_domain_alloc_irqs().
>>
>> Thanks Thomas for this clarification (I'm sure now that we need .free
>> callbacks).
>> irq_domain_free_irqs() is called in 2 scenario:
>> 1- when issue occurs in irq_create_fwspec_mapping()
>> 2- when irq_dispose_mapping() is called
>>
>> Case 2 is the one I tested some times ago. In this case, I need to mask
>> interrupts in .free callback of EXTI (parent) domain to avoid spurious
>> interrupts.
>
> And why would irq_dispose_mapping() be called on an unmasked, i.e. active,
> interrupt? The masking is just papering over that.

Ok. So my test was wrong and irq_dispose_mapping() has to be called when 
irq is masked (for example just  after free_irq()). For sure in this 
case the mask inside exti free callback has no sens (catch :))

I will change .free callback by:

static void stm32_exti_free(struct irq_domain *d, unsigned int virq,
			    unsigned int nr_irqs)
{
	struct irq_data *data = irq_domain_get_irq_data(d, virq);
	irq_domain_reset_irq_data(data);
}


so if you agree I will resend only patches concerning stm32 exti driver 
[1],[2],[3],[4]

Thanks for your time.

alex


>
> Thanks,
>
> 	tglx
>

^ permalink raw reply

* [PATCH] hwrng: omap - Only fail if pm_runtime_get_sync returns < 0
From: Dave Gerlach @ 2016-09-20 15:25 UTC (permalink / raw)
  To: linux-arm-kernel

Currently omap-rng checks the return value of pm_runtime_get_sync and
reports failure if anything is returned, however it should be checking
if ret < 0 as pm_runtime_get_sync return 0 on success but also can return
1 if the device was already active which is not a failure case. Only
values < 0 are actual failures.

Fixes: 61dc0a446e5d ("hwrng: omap - Fix assumption that runtime_get_sync will always succeed")
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 drivers/char/hw_random/omap-rng.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
index 01d4be2c354b..f5c26a5f6875 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -385,7 +385,7 @@ static int omap_rng_probe(struct platform_device *pdev)
 
 	pm_runtime_enable(&pdev->dev);
 	ret = pm_runtime_get_sync(&pdev->dev);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to runtime_get device: %d\n", ret);
 		pm_runtime_put_noidle(&pdev->dev);
 		goto err_ioremap;
@@ -443,7 +443,7 @@ static int __maybe_unused omap_rng_resume(struct device *dev)
 	int ret;
 
 	ret = pm_runtime_get_sync(dev);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(dev, "Failed to runtime_get device: %d\n", ret);
 		pm_runtime_put_noidle(dev);
 		return ret;
-- 
2.9.3

^ permalink raw reply related

* [PATCH V6 3/5] PCI: thunder-pem: Allow to probe PEM-specific register range for ACPI case
From: Ard Biesheuvel @ 2016-09-20 15:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160920140518.GD13855@localhost>

On 20 September 2016 at 15:05, Bjorn Helgaas <helgaas@kernel.org> wrote:
> Hi Ard,
>
> On Tue, Sep 20, 2016 at 02:40:13PM +0100, Ard Biesheuvel wrote:
>> On 20 September 2016 at 14:33, Bjorn Helgaas <helgaas@kernel.org> wrote:
>> > [+cc Rafael (maybe already cc'd; I didn't recognize rafael at kernel.org, Duc]
>> >
>> > On Tue, Sep 20, 2016 at 09:23:21AM +0200, Tomasz Nowicki wrote:
>> >> On 19.09.2016 20:09, Bjorn Helgaas wrote:
>> >> >On Fri, Sep 09, 2016 at 09:24:05PM +0200, Tomasz Nowicki wrote:
>> >> >>thunder-pem driver stands for being ACPI based PCI host controller.
>> >> >>However, there is no standard way to describe its PEM-specific register
>> >> >>ranges in ACPI tables. Thus we add thunder_pem_init() ACPI extension
>> >> >>to obtain hardcoded addresses from static resource array.
>> >> >>Although it is not pretty, it prevents from creating standard mechanism to
>> >> >>handle similar cases in future.
>> >> >>
>> >> >>Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> >> >>---
>> >> >> drivers/pci/host/pci-thunder-pem.c | 61 ++++++++++++++++++++++++++++++--------
>> >> >> 1 file changed, 48 insertions(+), 13 deletions(-)
>> >> >>
>> >> >>diff --git a/drivers/pci/host/pci-thunder-pem.c b/drivers/pci/host/pci-thunder-pem.c
>> >> >>index 6abaf80..b048761 100644
>> >> >>--- a/drivers/pci/host/pci-thunder-pem.c
>> >> >>+++ b/drivers/pci/host/pci-thunder-pem.c
>> >> >>@@ -18,6 +18,7 @@
>> >> >> #include <linux/init.h>
>> >> >> #include <linux/of_address.h>
>> >> >> #include <linux/of_pci.h>
>> >> >>+#include <linux/pci-acpi.h>
>> >> >> #include <linux/pci-ecam.h>
>> >> >> #include <linux/platform_device.h>
>> >> >>
>> >> >>@@ -284,6 +285,40 @@ static int thunder_pem_config_write(struct pci_bus *bus, unsigned int devfn,
>> >> >>    return pci_generic_config_write(bus, devfn, where, size, val);
>> >> >> }
>> >> >>
>> >> >>+#ifdef CONFIG_ACPI
>> >> >>+static struct resource thunder_pem_reg_res[] = {
>> >> >>+   [4] = DEFINE_RES_MEM(0x87e0c0000000UL, SZ_16M),
>> >> >>+   [5] = DEFINE_RES_MEM(0x87e0c1000000UL, SZ_16M),
>> >> >>+   [6] = DEFINE_RES_MEM(0x87e0c2000000UL, SZ_16M),
>> >> >>+   [7] = DEFINE_RES_MEM(0x87e0c3000000UL, SZ_16M),
>> >> >>+   [8] = DEFINE_RES_MEM(0x87e0c4000000UL, SZ_16M),
>> >> >>+   [9] = DEFINE_RES_MEM(0x87e0c5000000UL, SZ_16M),
>> >> >>+   [14] = DEFINE_RES_MEM(0x97e0c0000000UL, SZ_16M),
>> >> >>+   [15] = DEFINE_RES_MEM(0x97e0c1000000UL, SZ_16M),
>> >> >>+   [16] = DEFINE_RES_MEM(0x97e0c2000000UL, SZ_16M),
>> >> >>+   [17] = DEFINE_RES_MEM(0x97e0c3000000UL, SZ_16M),
>> >> >>+   [18] = DEFINE_RES_MEM(0x97e0c4000000UL, SZ_16M),
>> >> >>+   [19] = DEFINE_RES_MEM(0x97e0c5000000UL, SZ_16M),
>> >> >
>> >> >1) The "correct" way to discover the resources consumed by an ACPI
>> >> >   device is to use the _CRS method.  I know there are some issues
>> >> >   there for bridges (not the fault of ThunderX!) because there's not
>> >> >   a good way to distinguish windows from resources consumed directly
>> >> >   by the bridge.
>> >> >
>> >> >   But we should either do this correctly, or include a comment about
>> >> >   why we're doing it wrong, so we don't give the impression that this
>> >> >   is the right way to do it.
>> >> >
>> >> >   I seem to recall some discussion about why we're doing it this way,
>> >> >   but I don't remember the details.  It'd be nice to include a
>> >> >   summary here.
>> >>
>> >> OK I will. The reason why we cannot use _CRS for this case is that
>> >> CONSUMER flag was not use consistently for the bridge so far.
>> >
>> > Yes, I'm aware of that problem, but hard-coding resources into drivers
>> > is just a disaster.  The PCI and ACPI cores need generic ways to learn
>> > what resources are consumed by devices.  For PCI devices, that's done
>> > with BARs.  For ACPI devices, it's done with _CRS.  Without generic
>> > resource discovery, we can't manage resources reliably at the system
>> > level [1].
>> >
>> > You have a PNP0A03/PNP0A08 device for the PCI host bridge.  Because of
>> > the BIOS bugs in CONSUMER flag usage, we assume everything in its _CRS
>> > is a window and not consumed by the bridge itself.  What if you added
>> > a companion ACPI device with a _CRS that contained the bridge
>> > resources?  Then you'd have some driver ugliness to find that device,
>> > but at least the ACPI core could tell what resources were in use.
>> >
>> > Maybe Rafael has a better idea?
>>
>> In the discussions leading up to this, we tried very hard to make this
>> arm64/acpi quirks mechanism just as flexible as we need it to be to
>> cover the current crop of incompatible hardware, but not more so.
>> Going forward, we intend to require all arm64/acpi hardware to be spec
>> compliant, and so any parametrization beyond what is required for the
>> currently known broken hardware is only going to make it easier for
>> others to ship with tweaked ACPI descriptions so that an existing
>> quirk is triggered for hardware that it was not intended for. It also
>> implies that we have to deal with the ACPI descriptions as they were
>> shipped with the current hardware.
>>
>> That does not mean, of course, that we should use bare constants
>> rather than symbolic ones, but anything beyond that exceeds the
>> desired scope of quirks handling.
>
> Symbolic vs bare constants is the least of my worries.  I'm pretty
> happy with the current quirk implementation.  It's pretty simple and
> straightforward.
>

OK, good to know that we are on the right track here.

> Apparently you shipped broken firmware that doesn't accurately
> describe system resource usage.  Presumably that firmware could be
> updated, but maybe it's worthwhile to work around it in the kernel,
> depending on where it got shipped.
>

None of these platforms can be fixed entirely in software, and given
that we will not be adding quirks for new broken hardware, we should
ask ourselves whether having two versions of a quirk, i.e., one for
broken hardware + currently shipping firmware, and one for the same
broken hardware with fixed firmware is really an improvement over what
has been proposed here.

> I'd like to step back and come up with some understanding of how
> non-broken firmware *should* deal with this issue.  Then, if we *do*
> work around this particular broken firmware in the kernel, it would be
> nice to do it in a way that fits in with that understanding.
>
> For example, if a companion ACPI device is the preferred solution, an
> ACPI quirk could fabricate a device with the required resources.  That
> would address the problem closer to the source and make it more likely
> that the rest of the system will work correctly: /proc/iomem could
> make sense, things that look at _CRS generically would work (e.g,
> /sys/, an admittedly hypothetical "lsacpi", etc.)
>
> Hard-coding stuff in drivers is a point solution that doesn't provide
> any guidance for future platforms and makes it likely that the hack
> will get copied into even more drivers.
>

OK, I see. But the guidance for future platforms should be 'do not
rely on quirks', and what I am arguing here is that the more we polish
up this code and make it clean and reusable, the more likely it is
that will end up getting abused by new broken hardware that we set out
to reject entirely in the first place.

So of course, if the quirk involves claiming resources, let's make
sure that this occurs in the cleanest and most compliant way possible.
But any factoring/reuse concerns other than for the current crop of
broken hardware should be avoided imo.

-- 
Ard.

^ permalink raw reply

* [PATCH 2/2] arm64: dts: marvell: fix clocksource for CP110 slave SPI0
From: Gregory CLEMENT @ 2016-09-20 15:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473183672-12028-3-git-send-email-mw@semihalf.com>

Hi Olof and Arnd,
 
 On mar., sept. 06 2016, Marcin Wojtas <mw@semihalf.com> wrote:

> I2C and SPI interfaces share common clock trees within the CP110 HW block.
> It occurred that SPI0 interface has wrong clock assignment in the device
> tree, which is fixed in this commit to a proper value.
>
> Fixes: c749b8d9de32 ("arm64: dts: marvell: add description for the ...")
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

Could you take this patch for 4.9? I didn't realized at first view it
was for 4.9, I though I could put it the PR for fixes for 4.8.

I can do a new PR for dt64 with this single patch if you prefer.

The last option will be to have it as a fix for v4.9-rc2.

Thanks,

Gregory



> ---
>  arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
> index 37bff70..150675c 100644
> --- a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
> @@ -133,7 +133,7 @@
>  				#address-cells = <0x1>;
>  				#size-cells = <0x0>;
>  				cell-index = <1>;
> -				clocks = <&cps_syscon0 0 3>;
> +				clocks = <&cps_syscon0 1 21>;
>  				status = "disabled";
>  			};
>  
> -- 
> 1.8.3.1
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ 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