Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 12/16] KVM: arm64: Save host SVE context as appropriate
From: Dave Martin @ 2018-05-09 16:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525882385-29181-1-git-send-email-Dave.Martin@arm.com>

This patch adds SVE context saving to the hyp FPSIMD context switch
path.  This means that it is no longer necessary to save the host
SVE state in advance of entering the guest, when in use.

In order to avoid adding pointless complexity to the code, VHE is
assumed if SVE is in use.  VHE is an architectural prerequisite for
SVE, so there is no good reason to turn CONFIG_ARM64_VHE off in
kernels that support both SVE and KVM.

Historically, software models exist that can expose the
architecturally invalid configuration of SVE without VHE, so if
this situation is detected at kvm_init() time then KVM will be
disabled.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/Kconfig          |  7 +++++++
 arch/arm64/kvm/fpsimd.c     |  1 -
 arch/arm64/kvm/hyp/switch.c | 20 +++++++++++++++++++-
 virt/kvm/arm/arm.c          | 18 ++++++++++++++++++
 4 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index eb2cf49..b0d3820 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1130,6 +1130,7 @@ endmenu
 config ARM64_SVE
 	bool "ARM Scalable Vector Extension support"
 	default y
+	depends on !KVM || ARM64_VHE
 	help
 	  The Scalable Vector Extension (SVE) is an extension to the AArch64
 	  execution state which complements and extends the SIMD functionality
@@ -1155,6 +1156,12 @@ config ARM64_SVE
 	  booting the kernel.  If unsure and you are not observing these
 	  symptoms, you should assume that it is safe to say Y.
 
+	  CPUs that support SVE are architecturally required to support the
+	  Virtualization Host Extensions (VHE), so the kernel makes no
+	  provision for supporting SVE alongside KVM without VHE enabled.
+	  Thus, you will need to enable CONFIG_ARM64_VHE if you want to support
+	  KVM in the same kernel image.
+
 config ARM64_MODULE_PLTS
 	bool
 	select HAVE_MOD_ARCH_SPECIFIC
diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
index cdd78a77..c50a8e9 100644
--- a/arch/arm64/kvm/fpsimd.c
+++ b/arch/arm64/kvm/fpsimd.c
@@ -59,7 +59,6 @@ int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu)
  */
 void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
 {
-	BUG_ON(system_supports_sve());
 	BUG_ON(!current->mm);
 
 	vcpu->arch.flags &= ~(KVM_ARM64_FP_ENABLED | KVM_ARM64_HOST_SVE_IN_USE);
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 118f300..a6a8c7d 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -21,6 +21,7 @@
 
 #include <kvm/arm_psci.h>
 
+#include <asm/cpufeature.h>
 #include <asm/kvm_asm.h>
 #include <asm/kvm_emulate.h>
 #include <asm/kvm_host.h>
@@ -28,6 +29,7 @@
 #include <asm/kvm_mmu.h>
 #include <asm/fpsimd.h>
 #include <asm/debug-monitors.h>
+#include <asm/processor.h>
 #include <asm/thread_info.h>
 
 /* Check whether the FP regs were dirtied while in the host-side run loop: */
@@ -329,6 +331,8 @@ static bool __hyp_text __skip_instr(struct kvm_vcpu *vcpu)
 void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
 				    struct kvm_vcpu *vcpu)
 {
+	struct user_fpsimd_state *host_fpsimd = vcpu->arch.host_fpsimd_state;
+
 	if (has_vhe())
 		write_sysreg(read_sysreg(cpacr_el1) | CPACR_EL1_FPEN,
 			     cpacr_el1);
@@ -339,7 +343,21 @@ void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
 	isb();
 
 	if (vcpu->arch.flags & KVM_ARM64_FP_HOST) {
-		__fpsimd_save_state(vcpu->arch.host_fpsimd_state);
+		/*
+		 * In the SVE case, VHE is assumed: it is enforced by
+		 * Kconfig and kvm_arch_init().
+		 */
+		if (system_supports_sve() &&
+		    (vcpu->arch.flags & KVM_ARM64_HOST_SVE_IN_USE)) {
+			struct thread_struct *thread = container_of(
+				host_fpsimd,
+				struct thread_struct, uw.fpsimd_state);
+
+			sve_save_state(sve_pffr(thread), &host_fpsimd->fpsr);
+		} else {
+			__fpsimd_save_state(host_fpsimd);
+		}
+
 		vcpu->arch.flags &= ~KVM_ARM64_FP_HOST;
 	}
 
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index bee226c..379e8a9 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -16,6 +16,7 @@
  * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 
+#include <linux/bug.h>
 #include <linux/cpu_pm.h>
 #include <linux/errno.h>
 #include <linux/err.h>
@@ -41,6 +42,7 @@
 #include <asm/mman.h>
 #include <asm/tlbflush.h>
 #include <asm/cacheflush.h>
+#include <asm/cpufeature.h>
 #include <asm/virt.h>
 #include <asm/kvm_arm.h>
 #include <asm/kvm_asm.h>
@@ -1582,6 +1584,22 @@ int kvm_arch_init(void *opaque)
 		}
 	}
 
+	/*
+	 * VHE is a prerequisite for SVE in the Arm architecture, and
+	 * Kconfig ensures that if system_supports_sve() here then
+	 * CONFIG_ARM64_VHE is enabled, so if VHE support wasn't already
+	 * detected and enabled, the CPU is architecturally
+	 * noncompliant.
+	 *
+	 * Just in case this mismatch is seen, detect it, warn and give
+	 * up.  Supporting this forbidden configuration in Hyp would be
+	 * pointless.
+	 */
+	if (system_supports_sve() && !has_vhe()) {
+		kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
+		return -ENODEV;
+	}
+
 	err = init_common_resources();
 	if (err)
 		return err;
-- 
2.1.4

^ permalink raw reply related

* [PATCH v7 13/16] KVM: arm64: Remove eager host SVE state saving
From: Dave Martin @ 2018-05-09 16:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525882385-29181-1-git-send-email-Dave.Martin@arm.com>

Now that the host SVE context can be saved on demand from Hyp,
there is no longer any need to save this state in advance before
entering the guest.

This patch removes the relevant call to
kvm_fpsimd_flush_cpu_state().

Since the problem that function was intended to solve now no longer
exists, the function and its dependencies are also deleted.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/kvm_host.h   |  3 ---
 arch/arm64/include/asm/kvm_host.h | 10 ----------
 arch/arm64/kernel/fpsimd.c        | 21 ---------------------
 virt/kvm/arm/arm.c                |  3 ---
 4 files changed, 37 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 4cac8d1..39f051f 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -311,9 +311,6 @@ static inline void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu) {}
 static inline void kvm_arch_vcpu_park_fp(struct kvm_vcpu *vcpu) {}
 static inline void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu) {}
 
-/* All host FP/SIMD state is restored on guest exit, so nothing to save: */
-static inline void kvm_fpsimd_flush_cpu_state(void) {}
-
 static inline void kvm_arm_vhe_guest_enter(void) {}
 static inline void kvm_arm_vhe_guest_exit(void) {}
 
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index b3fe730..2629093 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -444,16 +444,6 @@ static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
 }
 #endif
 
-/*
- * All host FP/SIMD state is restored on guest exit, so nothing needs
- * doing here except in the SVE case:
-*/
-static inline void kvm_fpsimd_flush_cpu_state(void)
-{
-	if (system_supports_sve())
-		sve_flush_cpu_state();
-}
-
 static inline void kvm_arm_vhe_guest_enter(void)
 {
 	local_daif_mask();
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 13fa681..32898a8 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -120,7 +120,6 @@
  */
 struct fpsimd_last_state_struct {
 	struct user_fpsimd_state *st;
-	bool sve_in_use;
 };
 
 static DEFINE_PER_CPU(struct fpsimd_last_state_struct, fpsimd_last_state);
@@ -1010,7 +1009,6 @@ void fpsimd_bind_task_to_cpu(void)
 		this_cpu_ptr(&fpsimd_last_state);
 
 	last->st = &current->thread.uw.fpsimd_state;
-	last->sve_in_use = test_thread_flag(TIF_SVE);
 	current->thread.fpsimd_cpu = smp_processor_id();
 
 	if (system_supports_sve()) {
@@ -1032,7 +1030,6 @@ void fpsimd_bind_state_to_cpu(struct user_fpsimd_state *st)
 	WARN_ON(!in_softirq() && !irqs_disabled());
 
 	last->st = st;
-	last->sve_in_use = false;
 }
 
 /*
@@ -1092,24 +1089,6 @@ void fpsimd_flush_cpu_state(void)
 	__this_cpu_write(fpsimd_last_state.st, NULL);
 }
 
-/*
- * Invalidate any task SVE state currently held in this CPU's regs.
- *
- * This is used to prevent the kernel from trying to reuse SVE register data
- * that is detroyed by KVM guest enter/exit.  This function should go away when
- * KVM SVE support is implemented.  Don't use it for anything else.
- */
-#ifdef CONFIG_ARM64_SVE
-void sve_flush_cpu_state(void)
-{
-	struct fpsimd_last_state_struct const *last =
-		this_cpu_ptr(&fpsimd_last_state);
-
-	if (last->st && last->sve_in_use)
-		fpsimd_flush_cpu_state();
-}
-#endif /* CONFIG_ARM64_SVE */
-
 #ifdef CONFIG_KERNEL_MODE_NEON
 
 DEFINE_PER_CPU(bool, kernel_neon_busy);
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 379e8a9..9cef525 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -682,9 +682,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
 		 */
 		preempt_disable();
 
-		/* Flush FP/SIMD state that can't survive guest entry/exit */
-		kvm_fpsimd_flush_cpu_state();
-
 		kvm_pmu_flush_hwstate(vcpu);
 
 		local_irq_disable();
-- 
2.1.4

^ permalink raw reply related

* [PATCH v7 14/16] KVM: arm64: Remove redundant *exit_code changes in fpsimd_guest_exit()
From: Dave Martin @ 2018-05-09 16:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525882385-29181-1-git-send-email-Dave.Martin@arm.com>

In fixup_guest_exit(), there are a couple of cases where after
checking what the exit code was, we assign it explicitly with the
value it already had.

Assuming this is not indicative of a bug, these assignments are not
needed.

This patch removes the redundant assignments, and simplifies some
if-nesting that becomes trivial as a result.

No functional change.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kvm/hyp/switch.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index a6a8c7d..18d0faa 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -403,12 +403,8 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 		if (valid) {
 			int ret = __vgic_v2_perform_cpuif_access(vcpu);
 
-			if (ret == 1) {
-				if (__skip_instr(vcpu))
-					return true;
-				else
-					*exit_code = ARM_EXCEPTION_TRAP;
-			}
+			if (ret ==  1 && __skip_instr(vcpu))
+				return true;
 
 			if (ret == -1) {
 				/* Promote an illegal access to an
@@ -430,12 +426,8 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 	     kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_CP15_32)) {
 		int ret = __vgic_v3_perform_cpuif_access(vcpu);
 
-		if (ret == 1) {
-			if (__skip_instr(vcpu))
-				return true;
-			else
-				*exit_code = ARM_EXCEPTION_TRAP;
-		}
+		if (ret == 1 && __skip_instr(vcpu))
+			return true;
 	}
 
 	/* Return to the host kernel and handle the exit */
-- 
2.1.4

^ permalink raw reply related

* [PATCH v7 15/16] KVM: arm64: Fold redundant exit code checks out of fixup_guest_exit()
From: Dave Martin @ 2018-05-09 16:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525882385-29181-1-git-send-email-Dave.Martin@arm.com>

The entire tail of fixup_guest_exit() is contained in if statements
of the form if (x && *exit_code == ARM_EXCEPTION_TRAP).  As a result,
we can check just once and bail out of the function early, allowing
the remaining if conditions to be simplified.

The only awkward case is where *exit_code is changed to
ARM_EXCEPTION_EL1_SERROR in the case of an illegal GICv2 CPU
interface access: in that case, the GICv3 trap handling code is
skipped using a goto.  This avoids pointlessly evaluating the
static branch check for the GICv3 case, even though we can't have
vgic_v2_cpuif_trap and vgic_v3_cpuif_trap true simultaneously
unless we have a GICv3 and GICv2 on the host: that sounds stupid,
but I haven't satisfied myself that it can't happen.

No functional change.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kvm/hyp/switch.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 18d0faa..4fbee95 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -387,11 +387,13 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 	 * same PC once the SError has been injected, and replay the
 	 * trapping instruction.
 	 */
-	if (*exit_code == ARM_EXCEPTION_TRAP && !__populate_fault_info(vcpu))
+	if (*exit_code != ARM_EXCEPTION_TRAP)
+		goto exit;
+
+	if (!__populate_fault_info(vcpu))
 		return true;
 
-	if (static_branch_unlikely(&vgic_v2_cpuif_trap) &&
-	    *exit_code == ARM_EXCEPTION_TRAP) {
+	if (static_branch_unlikely(&vgic_v2_cpuif_trap)) {
 		bool valid;
 
 		valid = kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_DABT_LOW &&
@@ -417,11 +419,12 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 					*vcpu_cpsr(vcpu) &= ~DBG_SPSR_SS;
 				*exit_code = ARM_EXCEPTION_EL1_SERROR;
 			}
+
+			goto exit;
 		}
 	}
 
 	if (static_branch_unlikely(&vgic_v3_cpuif_trap) &&
-	    *exit_code == ARM_EXCEPTION_TRAP &&
 	    (kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_SYS64 ||
 	     kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_CP15_32)) {
 		int ret = __vgic_v3_perform_cpuif_access(vcpu);
@@ -430,6 +433,7 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 			return true;
 	}
 
+exit:
 	/* Return to the host kernel and handle the exit */
 	return false;
 }
-- 
2.1.4

^ permalink raw reply related

* [PATCH v7 16/16] KVM: arm64: Invoke FPSIMD context switch trap from C
From: Dave Martin @ 2018-05-09 16:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525882385-29181-1-git-send-email-Dave.Martin@arm.com>

The conversion of the FPSIMD context switch trap code to C has added
some overhead to calling it, due to the need to save registers that
the procedure call standard defines as caller-saved.

So, perhaps it is no longer worth invoking this trap handler quite
so early.

Instead, we can invoke it from fixup_guest_exit(), with little
likelihood of increasing the overhead much further.

As a convenience, this patch gives __hyp_switch_fpsimd() the same
return semantics fixup_guest_exit().  For now there is no
possibility of a spurious FPSIMD trap, so the function always
returns true, but this allows it to be tail-called with a single
return statement.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kvm/hyp/entry.S     | 30 ------------------------------
 arch/arm64/kvm/hyp/hyp-entry.S | 19 -------------------
 arch/arm64/kvm/hyp/switch.c    | 15 +++++++++++++--
 3 files changed, 13 insertions(+), 51 deletions(-)

diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
index 40f349b..fad1e16 100644
--- a/arch/arm64/kvm/hyp/entry.S
+++ b/arch/arm64/kvm/hyp/entry.S
@@ -166,33 +166,3 @@ abort_guest_exit_end:
 	orr	x0, x0, x5
 1:	ret
 ENDPROC(__guest_exit)
-
-ENTRY(__fpsimd_guest_restore)
-	// x0: esr
-	// x1: vcpu
-	// x2-x29,lr: vcpu regs
-	// vcpu x0-x1 on the stack
-	stp	x2, x3, [sp, #-144]!
-	stp	x4, x5, [sp, #16]
-	stp	x6, x7, [sp, #32]
-	stp	x8, x9, [sp, #48]
-	stp	x10, x11, [sp, #64]
-	stp	x12, x13, [sp, #80]
-	stp	x14, x15, [sp, #96]
-	stp	x16, x17, [sp, #112]
-	stp	x18, lr, [sp, #128]
-
-	bl	__hyp_switch_fpsimd
-
-	ldp	x4, x5, [sp, #16]
-	ldp	x6, x7, [sp, #32]
-	ldp	x8, x9, [sp, #48]
-	ldp	x10, x11, [sp, #64]
-	ldp	x12, x13, [sp, #80]
-	ldp	x14, x15, [sp, #96]
-	ldp	x16, x17, [sp, #112]
-	ldp	x18, lr, [sp, #128]
-	ldp	x0, x1, [sp, #144]
-	ldp	x2, x3, [sp], #160
-	eret
-ENDPROC(__fpsimd_guest_restore)
diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S
index bffece2..753b9d2 100644
--- a/arch/arm64/kvm/hyp/hyp-entry.S
+++ b/arch/arm64/kvm/hyp/hyp-entry.S
@@ -113,25 +113,6 @@ el1_hvc_guest:
 
 el1_trap:
 	get_vcpu_ptr	x1, x0
-
-	mrs		x0, esr_el2
-	lsr		x0, x0, #ESR_ELx_EC_SHIFT
-	/*
-	 * x0: ESR_EC
-	 * x1: vcpu pointer
-	 */
-
-	/*
-	 * We trap the first access to the FP/SIMD to save the host context
-	 * and restore the guest context lazily.
-	 * If FP/SIMD is not implemented, handle the trap and inject an
-	 * undefined instruction exception to the guest.
-	 */
-alternative_if_not ARM64_HAS_NO_FPSIMD
-	cmp	x0, #ESR_ELx_EC_FP_ASIMD
-	b.eq	__fpsimd_guest_restore
-alternative_else_nop_endif
-
 	mov	x0, #ARM_EXCEPTION_TRAP
 	b	__guest_exit
 
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 4fbee95..2d45bd7 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -328,8 +328,7 @@ static bool __hyp_text __skip_instr(struct kvm_vcpu *vcpu)
 	}
 }
 
-void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
-				    struct kvm_vcpu *vcpu)
+static bool __hyp_text __hyp_switch_fpsimd(struct kvm_vcpu *vcpu)
 {
 	struct user_fpsimd_state *host_fpsimd = vcpu->arch.host_fpsimd_state;
 
@@ -369,6 +368,8 @@ void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
 			     fpexc32_el2);
 
 	vcpu->arch.flags |= KVM_ARM64_FP_ENABLED;
+
+	return true;
 }
 
 /*
@@ -390,6 +391,16 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 	if (*exit_code != ARM_EXCEPTION_TRAP)
 		goto exit;
 
+	/*
+	 * We trap the first access to the FP/SIMD to save the host context
+	 * and restore the guest context lazily.
+	 * If FP/SIMD is not implemented, handle the trap and inject an
+	 * undefined instruction exception to the guest.
+	 */
+	if (system_supports_fpsimd() &&
+	    kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_FP_ASIMD)
+		return __hyp_switch_fpsimd(vcpu);
+
 	if (!__populate_fault_info(vcpu))
 		return true;
 
-- 
2.1.4

^ permalink raw reply related

* [PATCH v7 02/16] arm64: Use update{,_tsk}_thread_flag()
From: Will Deacon @ 2018-05-09 16:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525882385-29181-3-git-send-email-Dave.Martin@arm.com>

Hi Dave,

On Wed, May 09, 2018 at 05:12:51PM +0100, Dave Martin wrote:
> This patch uses the new update_thread_flag() helpers to simplify a
> couple of if () set; else clear; constructs.
> 
> No functional change.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/kernel/fpsimd.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index 87a3536..0c4e7e0 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -618,10 +618,8 @@ int sve_set_vector_length(struct task_struct *task,
>  	task->thread.sve_vl = vl;
>  
>  out:
> -	if (flags & PR_SVE_VL_INHERIT)
> -		set_tsk_thread_flag(task, TIF_SVE_VL_INHERIT);
> -	else
> -		clear_tsk_thread_flag(task, TIF_SVE_VL_INHERIT);
> +	update_tsk_thread_flag(task, TIF_SVE_VL_INHERIT,
> +			       flags & PR_SVE_VL_INHERIT);
>  
>  	return 0;
>  }
> @@ -902,7 +900,7 @@ void fpsimd_thread_switch(struct task_struct *next)
>  	if (current->mm)
>  		task_fpsimd_save();
>  
> -	if (next->mm) {
> +	if (next->mm)
>  		/*
>  		 * If we are switching to a task whose most recent userland
>  		 * FPSIMD state is already in the registers of *this* cpu,
> @@ -910,13 +908,10 @@ void fpsimd_thread_switch(struct task_struct *next)
>  		 * the TIF_FOREIGN_FPSTATE flag so the state will be loaded
>  		 * upon the next return to userland.
>  		 */
> -		if (__this_cpu_read(fpsimd_last_state.st) ==
> -			&next->thread.uw.fpsimd_state
> -		    && next->thread.fpsimd_cpu == smp_processor_id())
> -			clear_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE);
> -		else
> -			set_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE);
> -	}
> +		update_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE,
> +			__this_cpu_read(fpsimd_last_state.st) !=
> +				&next->thread.uw.fpsimd_state ||
> +			next->thread.fpsimd_cpu != smp_processor_id());

Given the multi-line comment and this multi-line call, I'd be inclined to
leave the curlies in place and then use a local bool for the complex
condition.

With that:

Acked-by: Will Deacon <will.deacon@arm.com>

Will

^ permalink raw reply

* [PATCH v7 02/16] arm64: Use update{,_tsk}_thread_flag()
From: Dave Martin @ 2018-05-09 16:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180509161728.GC26671@arm.com>

On Wed, May 09, 2018 at 05:17:28PM +0100, Will Deacon wrote:
> Hi Dave,
> 
> On Wed, May 09, 2018 at 05:12:51PM +0100, Dave Martin wrote:
> > This patch uses the new update_thread_flag() helpers to simplify a
> > couple of if () set; else clear; constructs.
> > 
> > No functional change.
> > 
> > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> > Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > ---
> >  arch/arm64/kernel/fpsimd.c | 19 +++++++------------
> >  1 file changed, 7 insertions(+), 12 deletions(-)
> > 
> > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> > index 87a3536..0c4e7e0 100644
> > --- a/arch/arm64/kernel/fpsimd.c
> > +++ b/arch/arm64/kernel/fpsimd.c

[...]

> > @@ -902,7 +900,7 @@ void fpsimd_thread_switch(struct task_struct *next)
> >  	if (current->mm)
> >  		task_fpsimd_save();
> >  
> > -	if (next->mm) {
> > +	if (next->mm)
> >  		/*
> >  		 * If we are switching to a task whose most recent userland
> >  		 * FPSIMD state is already in the registers of *this* cpu,
> > @@ -910,13 +908,10 @@ void fpsimd_thread_switch(struct task_struct *next)
> >  		 * the TIF_FOREIGN_FPSTATE flag so the state will be loaded
> >  		 * upon the next return to userland.
> >  		 */
> > -		if (__this_cpu_read(fpsimd_last_state.st) ==
> > -			&next->thread.uw.fpsimd_state
> > -		    && next->thread.fpsimd_cpu == smp_processor_id())
> > -			clear_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE);
> > -		else
> > -			set_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE);
> > -	}
> > +		update_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE,
> > +			__this_cpu_read(fpsimd_last_state.st) !=
> > +				&next->thread.uw.fpsimd_state ||
> > +			next->thread.fpsimd_cpu != smp_processor_id());
> 
> Given the multi-line comment and this multi-line call, I'd be inclined to
> leave the curlies in place and then use a local bool for the complex
> condition.

Hey, curlies cost money, you know.

> With that:
> 
> Acked-by: Will Deacon <will.deacon@arm.com>

Are you content to see this merged without the change?  It doesn't seem
worth a respin of the whole series just for this.  I agree the code
would be clearer, but this patch doesn't actually make it worse IMHO.

If I respin for some reason though, I can address this and add your Ack.

Cheers
---Dave

^ permalink raw reply

* [PATCH] arm64: dts: juno: fix OF graph endpoint node names
From: Sudeep Holla @ 2018-05-09 16:34 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180508150952.24562-1-robh@kernel.org>



On 08/05/18 16:09, Rob Herring wrote:
> OF graph endpoint node names should be 'endpoint'. Fix the following
> warnings found by dtc:
> 
> Warning (graph_endpoint): /hdlcd at 7ff50000/port/hdlcd1-endpoint: graph endpont node nameshould be 'endpoint'
> Warning (graph_endpoint): /hdlcd at 7ff60000/port/hdlcd0-endpoint: graph endpont node nameshould be 'endpoint'
> Warning (graph_endpoint): /i2c at 7ffa0000/hdmi-transmitter at 70/port/tda998x-0-endpoint: graph endpont node name should be 'endpoint'
> Warning (graph_endpoint): /i2c at 7ffa0000/hdmi-transmitter at 71/port/tda998x-1-endpoint: graph endpont node name should be 'endpoint'
> 
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Rob Herring <robh@kernel.org>

Thanks Robs, both patches applied.

-- 
Regards,
Sudeep

^ permalink raw reply

* [PATCH v7 06/16] arm64/sve: Refactor user SVE trap maintenance for external use
From: Marc Zyngier @ 2018-05-09 16:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525882385-29181-7-git-send-email-Dave.Martin@arm.com>

On 09/05/18 17:12, Dave Martin wrote:
> In preparation for optimising the way KVM manages switching the
> guest and host FPSIMD state, it is necessary to provide a means for
> code outside arch/arm64/kernel/fpsimd.c to restore the user trap
> configuration for SVE correctly for the current task.
> 
> Rather than requiring external code to duplicate the maintenance
> explicitly, this patch wraps moves the trap maintenenace to
> fpsimd_bind_to_cpu(), since it is logically part of the work of
> associating the current task with the cpu.
> 
> Because fpsimd_bind_to_cpu() is rather a cryptic name to publish
> alongside fpsimd_bind_state_to_cpu(), the former function is
> renamed to fpsimd_bind_task_to_cpu() to make its purpose more
> explicit.
> 
> This patch makes appropriate changes to ensure that
> fpsimd_bind_task_to_cpu() is always called alongside
> task_fpsimd_load(), so that the trap maintenance continues to be
> done in every situation where it was done prior to this patch.
> 
> As a side-effect, the metadata updates done by
> fpsimd_bind_task_to_cpu() now change from conditional to
> unconditional in the "already bound" case of sigreturn.  This is
> harmless, and a couple of extra stores on this slow path will not
> impact performance.  I consider this a reasonable price to pay for
> a slightly cleaner interface.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> ---
>  arch/arm64/kernel/fpsimd.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index 5fc0595..60404eb 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -257,16 +257,6 @@ static void task_fpsimd_load(void)
>  			       sve_vq_from_vl(current->thread.sve_vl) - 1);
>  	else
>  		fpsimd_load_state(&current->thread.uw.fpsimd_state);
> -
> -	if (system_supports_sve()) {
> -		/* Toggle SVE trapping for userspace if needed */
> -		if (test_thread_flag(TIF_SVE))
> -			sve_user_enable();
> -		else
> -			sve_user_disable();
> -
> -		/* Serialised by exception return to user */
> -	}
>  }
>  
>  /*
> @@ -998,7 +988,7 @@ void fpsimd_signal_preserve_current_state(void)
>   * Associate current's FPSIMD context with this cpu
>   * Preemption must be disabled when calling this function.
>   */
> -static void fpsimd_bind_to_cpu(void)
> +static void fpsimd_bind_task_to_cpu(void)
>  {
>  	struct fpsimd_last_state_struct *last =
>  		this_cpu_ptr(&fpsimd_last_state);
> @@ -1006,6 +996,16 @@ static void fpsimd_bind_to_cpu(void)
>  	last->st = &current->thread.uw.fpsimd_state;
>  	last->sve_in_use = test_thread_flag(TIF_SVE);
>  	current->thread.fpsimd_cpu = smp_processor_id();
> +
> +	if (system_supports_sve()) {

Is it worth moving the last->sve_in_use assignment here? If the system
doesn't have SVE, the assignment is always 0, and we probably don't need
to do it at all. It could avoid the double call to test_thread_flag.

> +		/* Toggle SVE trapping for userspace if needed */
> +		if (test_thread_flag(TIF_SVE))
> +			sve_user_enable();
> +		else
> +			sve_user_disable();
> +
> +		/* Serialised by exception return to user */
> +	}
>  }
>  
>  /*
> @@ -1022,7 +1022,7 @@ void fpsimd_restore_current_state(void)
>  
>  	if (test_and_clear_thread_flag(TIF_FOREIGN_FPSTATE)) {
>  		task_fpsimd_load();
> -		fpsimd_bind_to_cpu();
> +		fpsimd_bind_task_to_cpu();
>  	}
>  
>  	local_bh_enable();
> @@ -1045,9 +1045,9 @@ void fpsimd_update_current_state(struct user_fpsimd_state const *state)
>  		fpsimd_to_sve(current);
>  
>  	task_fpsimd_load();
> +	fpsimd_bind_task_to_cpu();
>  
> -	if (test_and_clear_thread_flag(TIF_FOREIGN_FPSTATE))
> -		fpsimd_bind_to_cpu();
> +	clear_thread_flag(TIF_FOREIGN_FPSTATE);
>  
>  	local_bh_enable();
>  }
> 

Thanks,

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

^ permalink raw reply

* [PATCH v7 06/16] arm64/sve: Refactor user SVE trap maintenance for external use
From: Dave Martin @ 2018-05-09 16:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <dad74a47-26f4-d38f-a9c4-e60fc9d1a07a@arm.com>

On Wed, May 09, 2018 at 05:36:30PM +0100, Marc Zyngier wrote:
> On 09/05/18 17:12, Dave Martin wrote:
> > In preparation for optimising the way KVM manages switching the
> > guest and host FPSIMD state, it is necessary to provide a means for
> > code outside arch/arm64/kernel/fpsimd.c to restore the user trap
> > configuration for SVE correctly for the current task.
> > 
> > Rather than requiring external code to duplicate the maintenance
> > explicitly, this patch wraps moves the trap maintenenace to
> > fpsimd_bind_to_cpu(), since it is logically part of the work of
> > associating the current task with the cpu.
> > 
> > Because fpsimd_bind_to_cpu() is rather a cryptic name to publish
> > alongside fpsimd_bind_state_to_cpu(), the former function is
> > renamed to fpsimd_bind_task_to_cpu() to make its purpose more
> > explicit.
> > 
> > This patch makes appropriate changes to ensure that
> > fpsimd_bind_task_to_cpu() is always called alongside
> > task_fpsimd_load(), so that the trap maintenance continues to be
> > done in every situation where it was done prior to this patch.
> > 
> > As a side-effect, the metadata updates done by
> > fpsimd_bind_task_to_cpu() now change from conditional to
> > unconditional in the "already bound" case of sigreturn.  This is
> > harmless, and a couple of extra stores on this slow path will not
> > impact performance.  I consider this a reasonable price to pay for
> > a slightly cleaner interface.
> > 
> > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> > ---
> >  arch/arm64/kernel/fpsimd.c | 28 ++++++++++++++--------------
> >  1 file changed, 14 insertions(+), 14 deletions(-)
> > 
> > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c

[...]

> > @@ -1006,6 +996,16 @@ static void fpsimd_bind_to_cpu(void)
> >  	last->st = &current->thread.uw.fpsimd_state;
> >  	last->sve_in_use = test_thread_flag(TIF_SVE);
> >  	current->thread.fpsimd_cpu = smp_processor_id();
> > +
> > +	if (system_supports_sve()) {
> 
> Is it worth moving the last->sve_in_use assignment here? If the system
> doesn't have SVE, the assignment is always 0, and we probably don't need
> to do it at all. It could avoid the double call to test_thread_flag.

Could do now that you point it out ... but patch 13 gets rid of it
entirely anyway.  This is (was) part of the (now legacy) mechanism for
working round KVM's inability to lazily save the host SVE state.

If I respin I could tweak it here, but it would be purely cosmetic from
the whole-series point of view.

[...]

Cheers
---Dave

^ permalink raw reply

* [PATCH 1/2] ARM: dts: Restructure Vexpress motherboard includes
From: Sudeep Holla @ 2018-05-09 16:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180427185404.19205-1-linus.walleij@linaro.org>



On 27/04/18 19:54, Linus Walleij wrote:
> It is a bit unorthodox to just include a file in the middle
> of a another DTS file, it breaks the pattern from other device
> trees and also makes it really hard to reference things
> across the files with phandles.
> 
> Restructure the include for the Versatile Express motherboards
> to happen at the top of the file, reference the target nodes
> directly, and indent the motherboard .dtsi files to reflect
> their actual depth in the hiearchy.
> 
> This is a purely syntactic change that result in the same
> DTB files from the DTS/DTSI files.
> 

Please copy me and Lorenzo also in future.
Applied now(with typo in hierarchy fixed), thanks.

-- 
Regards,
Sudeep

^ permalink raw reply

* [PATCH 1/3] arm64: dts: juno: Fix "debounce-interval" property misspelling
From: Sudeep Holla @ 2018-05-09 16:44 UTC (permalink / raw)
  To: linux-arm-kernel

"debounce_interval" was never supported in the bindings. It should be
"debounce-interval". Moreover, latest DTC complains the following:

Warning (property_name_chars_strict): debounce_interval: Character '_' not recommended in property name

This patch fixes the above warning by using the correct property as
per the bindings.

Cc: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/boot/dts/arm/juno-motherboard.dtsi | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/arm/juno-motherboard.dtsi b/arch/arm64/boot/dts/arm/juno-motherboard.dtsi
index 69804c5f1197..4b38195a8bea 100644
--- a/arch/arm64/boot/dts/arm/juno-motherboard.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-motherboard.dtsi
@@ -58,42 +58,42 @@
 				compatible = "gpio-keys";
 
 				power-button {
-					debounce_interval = <50>;
+					debounce-interval = <50>;
 					wakeup-source;
 					linux,code = <116>;
 					label = "POWER";
 					gpios = <&iofpga_gpio0 0 0x4>;
 				};
 				home-button {
-					debounce_interval = <50>;
+					debounce-interval = <50>;
 					wakeup-source;
 					linux,code = <102>;
 					label = "HOME";
 					gpios = <&iofpga_gpio0 1 0x4>;
 				};
 				rlock-button {
-					debounce_interval = <50>;
+					debounce-interval = <50>;
 					wakeup-source;
 					linux,code = <152>;
 					label = "RLOCK";
 					gpios = <&iofpga_gpio0 2 0x4>;
 				};
 				vol-up-button {
-					debounce_interval = <50>;
+					debounce-interval = <50>;
 					wakeup-source;
 					linux,code = <115>;
 					label = "VOL+";
 					gpios = <&iofpga_gpio0 3 0x4>;
 				};
 				vol-down-button {
-					debounce_interval = <50>;
+					debounce-interval = <50>;
 					wakeup-source;
 					linux,code = <114>;
 					label = "VOL-";
 					gpios = <&iofpga_gpio0 4 0x4>;
 				};
 				nmi-button {
-					debounce_interval = <50>;
+					debounce-interval = <50>;
 					wakeup-source;
 					linux,code = <99>;
 					label = "NMI";
-- 
2.7.4

^ permalink raw reply related

* [PATCH 2/3] arm64: dts: juno: replace '_' with '-' in node names
From: Sudeep Holla @ 2018-05-09 16:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525884291-18851-1-git-send-email-sudeep.holla@arm.com>

The latest DTC throws warnings for character '_' in the node names.

Warning (node_name_chars_strict): /thermal-zones/big_cluster: Character '_' not recommended in node name
Warning (node_name_chars_strict): /thermal-zones/little_cluster: Character '_' not recommended in node name
Warning (node_name_chars_strict): /smb at 8000000/motherboard/gpio_keys: Character '_' not recommended in node name
Warning (node_name_chars_strict): /pmu_a57: Character '_' not recommended in node name
Warning (node_name_chars_strict): /pmu_a53: Character '_' not recommended in node name

The general recommendation is to use character '-' for all the node names.
This patch fixes the warnings following the recommendation.

Cc: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/boot/dts/arm/juno-base.dtsi        | 4 ++--
 arch/arm64/boot/dts/arm/juno-motherboard.dtsi | 2 +-
 arch/arm64/boot/dts/arm/juno-r1.dts           | 4 ++--
 arch/arm64/boot/dts/arm/juno-r2.dts           | 4 ++--
 arch/arm64/boot/dts/arm/juno.dts              | 4 ++--
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
index b2fedaab3fcc..b74e462c6658 100644
--- a/arch/arm64/boot/dts/arm/juno-base.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
@@ -572,14 +572,14 @@
 			thermal-sensors = <&scpi_sensors0 3>;
 		};
 
-		big_cluster_thermal_zone: big_cluster {
+		big_cluster_thermal_zone: big-cluster {
 			polling-delay = <1000>;
 			polling-delay-passive = <100>;
 			thermal-sensors = <&scpi_sensors0 21>;
 			status = "disabled";
 		};
 
-		little_cluster_thermal_zone: little_cluster {
+		little_cluster_thermal_zone: little-cluster {
 			polling-delay = <1000>;
 			polling-delay-passive = <100>;
 			thermal-sensors = <&scpi_sensors0 22>;
diff --git a/arch/arm64/boot/dts/arm/juno-motherboard.dtsi b/arch/arm64/boot/dts/arm/juno-motherboard.dtsi
index 4b38195a8bea..70e3409d86a3 100644
--- a/arch/arm64/boot/dts/arm/juno-motherboard.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-motherboard.dtsi
@@ -54,7 +54,7 @@
 				regulator-always-on;
 			};
 
-			gpio_keys {
+			gpio-keys {
 				compatible = "gpio-keys";
 
 				power-button {
diff --git a/arch/arm64/boot/dts/arm/juno-r1.dts b/arch/arm64/boot/dts/arm/juno-r1.dts
index c52c5c40dede..2c5db03f226c 100644
--- a/arch/arm64/boot/dts/arm/juno-r1.dts
+++ b/arch/arm64/boot/dts/arm/juno-r1.dts
@@ -201,7 +201,7 @@
 		};
 	};
 
-	pmu_a57 {
+	pmu-a57 {
 		compatible = "arm,cortex-a57-pmu";
 		interrupts = <GIC_SPI 02 IRQ_TYPE_LEVEL_HIGH>,
 			     <GIC_SPI 06 IRQ_TYPE_LEVEL_HIGH>;
@@ -209,7 +209,7 @@
 				     <&A57_1>;
 	};
 
-	pmu_a53 {
+	pmu-a53 {
 		compatible = "arm,cortex-a53-pmu";
 		interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
 			     <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>,
diff --git a/arch/arm64/boot/dts/arm/juno-r2.dts b/arch/arm64/boot/dts/arm/juno-r2.dts
index a90982ceda35..c51950f4a1b6 100644
--- a/arch/arm64/boot/dts/arm/juno-r2.dts
+++ b/arch/arm64/boot/dts/arm/juno-r2.dts
@@ -201,7 +201,7 @@
 		};
 	};
 
-	pmu_a72 {
+	pmu-a72 {
 		compatible = "arm,cortex-a72-pmu";
 		interrupts = <GIC_SPI 02 IRQ_TYPE_LEVEL_HIGH>,
 			     <GIC_SPI 06 IRQ_TYPE_LEVEL_HIGH>;
@@ -209,7 +209,7 @@
 				     <&A72_1>;
 	};
 
-	pmu_a53 {
+	pmu-a53 {
 		compatible = "arm,cortex-a53-pmu";
 		interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
 			     <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>,
diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
index c9236c4b967d..2b2bf39c30ef 100644
--- a/arch/arm64/boot/dts/arm/juno.dts
+++ b/arch/arm64/boot/dts/arm/juno.dts
@@ -200,7 +200,7 @@
 		};
 	};
 
-	pmu_a57 {
+	pmu-a57 {
 		compatible = "arm,cortex-a57-pmu";
 		interrupts = <GIC_SPI 02 IRQ_TYPE_LEVEL_HIGH>,
 			     <GIC_SPI 06 IRQ_TYPE_LEVEL_HIGH>;
@@ -208,7 +208,7 @@
 				     <&A57_1>;
 	};
 
-	pmu_a53 {
+	pmu-a53 {
 		compatible = "arm,cortex-a53-pmu";
 		interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
 			     <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>,
-- 
2.7.4

^ permalink raw reply related

* [PATCH 3/3] arm64: dts: juno/rtsm: re-structure motherboard includes
From: Sudeep Holla @ 2018-05-09 16:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525884291-18851-1-git-send-email-sudeep.holla@arm.com>

It is a bit unorthodox to just include a file in the middle of a another
DTS file, it breaks the pattern from other device trees and also makes
it really hard to reference things across the files with phandles.

Restructure the include for the Juno/RTSM motherboards to happen at the
top of the file, reference the target nodes directly, and indent the
motherboard .dtsi files to reflect their actual depth in the hierarchy.

This is a purely syntactic change that result in the same DTB files from
the DTS/DTSI files. This is based on similar patch from Linus Walleij
for ARM Vexpress platforms.

Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/boot/dts/arm/juno-base.dtsi           | 3 +--
 arch/arm64/boot/dts/arm/juno-motherboard.dtsi    | 4 ++++
 arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts       | 4 ++--
 arch/arm64/boot/dts/arm/rtsm_ve-motherboard.dtsi | 5 ++++-
 4 files changed, 11 insertions(+), 5 deletions(-)

Hi,

Posting this patch with -b(aka ignore-spaces option for review purpose.

Regards,
Sudeep

diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
index b74e462c6658..ce56a4acda4f 100644
--- a/arch/arm64/boot/dts/arm/juno-base.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 #include "juno-clocks.dtsi"
+#include "juno-motherboard.dtsi"
 
 / {
 	/*
@@ -795,8 +796,6 @@
 				<0 0 10 &gic 0 0 0 167 IRQ_TYPE_LEVEL_HIGH>,
 				<0 0 11 &gic 0 0 0 168 IRQ_TYPE_LEVEL_HIGH>,
 				<0 0 12 &gic 0 0 0 169 IRQ_TYPE_LEVEL_HIGH>;
-
-		/include/ "juno-motherboard.dtsi"
 	};
 
 	site2: tlx at 60000000 {
diff --git a/arch/arm64/boot/dts/arm/juno-motherboard.dtsi b/arch/arm64/boot/dts/arm/juno-motherboard.dtsi
index 70e3409d86a3..1792b074e9a3 100644
--- a/arch/arm64/boot/dts/arm/juno-motherboard.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-motherboard.dtsi
@@ -7,6 +7,8 @@
  *
  */
 
+/ {
+	smb at 8000000 {
 		mb_clk24mhz: clk24mhz {
 			compatible = "fixed-clock";
 			#clock-cells = <0>;
@@ -287,3 +289,5 @@
 				};
 			};
 		};
+	};
+};
diff --git a/arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts b/arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts
index 06c8117e812a..602f63f72c37 100644
--- a/arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts
+++ b/arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts
@@ -12,6 +12,8 @@
 
 /memreserve/ 0x80000000 0x00010000;
 
+#include "rtsm_ve-motherboard.dtsi"
+
 / {
 	model = "RTSM_VE_AEMv8A";
 	compatible = "arm,rtsm_ve,aemv8a", "arm,vexpress";
@@ -162,7 +164,5 @@
 				<0 0 40 &gic 0 40 4>,
 				<0 0 41 &gic 0 41 4>,
 				<0 0 42 &gic 0 42 4>;
-
-		/include/ "rtsm_ve-motherboard.dtsi"
 	};
 };
diff --git a/arch/arm64/boot/dts/arm/rtsm_ve-motherboard.dtsi b/arch/arm64/boot/dts/arm/rtsm_ve-motherboard.dtsi
index 1134e5d8df18..d2dbc3f39263 100644
--- a/arch/arm64/boot/dts/arm/rtsm_ve-motherboard.dtsi
+++ b/arch/arm64/boot/dts/arm/rtsm_ve-motherboard.dtsi
@@ -7,7 +7,8 @@
  *
  * VEMotherBoard.lisa
  */
-
+/ {
+	smb at 8000000 {
 		motherboard {
 			arm,v2m-memory-map = "rs1";
 			compatible = "arm,vexpress,v2m-p1", "simple-bus";
@@ -274,3 +275,5 @@
 				};
 			};
 		};
+	};
+};
-- 
2.7.4

^ permalink raw reply related

* [PATCH v7 06/16] arm64/sve: Refactor user SVE trap maintenance for external use
From: Marc Zyngier @ 2018-05-09 16:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180509164125.GQ7753@e103592.cambridge.arm.com>

On 09/05/18 17:41, Dave Martin wrote:
> On Wed, May 09, 2018 at 05:36:30PM +0100, Marc Zyngier wrote:
>> On 09/05/18 17:12, Dave Martin wrote:
>>> In preparation for optimising the way KVM manages switching the
>>> guest and host FPSIMD state, it is necessary to provide a means for
>>> code outside arch/arm64/kernel/fpsimd.c to restore the user trap
>>> configuration for SVE correctly for the current task.
>>>
>>> Rather than requiring external code to duplicate the maintenance
>>> explicitly, this patch wraps moves the trap maintenenace to
>>> fpsimd_bind_to_cpu(), since it is logically part of the work of
>>> associating the current task with the cpu.
>>>
>>> Because fpsimd_bind_to_cpu() is rather a cryptic name to publish
>>> alongside fpsimd_bind_state_to_cpu(), the former function is
>>> renamed to fpsimd_bind_task_to_cpu() to make its purpose more
>>> explicit.
>>>
>>> This patch makes appropriate changes to ensure that
>>> fpsimd_bind_task_to_cpu() is always called alongside
>>> task_fpsimd_load(), so that the trap maintenance continues to be
>>> done in every situation where it was done prior to this patch.
>>>
>>> As a side-effect, the metadata updates done by
>>> fpsimd_bind_task_to_cpu() now change from conditional to
>>> unconditional in the "already bound" case of sigreturn.  This is
>>> harmless, and a couple of extra stores on this slow path will not
>>> impact performance.  I consider this a reasonable price to pay for
>>> a slightly cleaner interface.
>>>
>>> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
>>> ---
>>>  arch/arm64/kernel/fpsimd.c | 28 ++++++++++++++--------------
>>>  1 file changed, 14 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> 
> [...]
> 
>>> @@ -1006,6 +996,16 @@ static void fpsimd_bind_to_cpu(void)
>>>  	last->st = &current->thread.uw.fpsimd_state;
>>>  	last->sve_in_use = test_thread_flag(TIF_SVE);
>>>  	current->thread.fpsimd_cpu = smp_processor_id();
>>> +
>>> +	if (system_supports_sve()) {
>>
>> Is it worth moving the last->sve_in_use assignment here? If the system
>> doesn't have SVE, the assignment is always 0, and we probably don't need
>> to do it at all. It could avoid the double call to test_thread_flag.
> 
> Could do now that you point it out ... but patch 13 gets rid of it
> entirely anyway.  This is (was) part of the (now legacy) mechanism for
> working round KVM's inability to lazily save the host SVE state.
> 
> If I respin I could tweak it here, but it would be purely cosmetic from
> the whole-series point of view.
Nah, don't even bother.

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

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

^ permalink raw reply

* [PATCH] ARM: dts: vexpress: Replace '_' with '-' in node names
From: Sudeep Holla @ 2018-05-09 16:48 UTC (permalink / raw)
  To: linux-arm-kernel

The latest DTC throws warnings for character '_' in the node names.

Warning (node_name_chars_strict): /sysreg at 10000/sys_led: Character '_' not recommended in node name
Warning (node_name_chars_strict): /sysreg at 10000/sys_mci: Character '_' not recommended in node name
Warning (node_name_chars_strict): /sysreg at 10000/sys_flash: Character '_' not recommended in node name

The general recommendation is to use character '-' for all the node names.
This patch fixes the warnings following the recommendation.

Cc: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
index 7b8ff5b3b912..58e73131ecef 100644
--- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
+++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
@@ -77,19 +77,19 @@
 					compatible = "arm,vexpress-sysreg";
 					reg = <0x010000 0x1000>;
 
-					v2m_led_gpios: sys_led {
+					v2m_led_gpios: sys-led {
 						compatible = "arm,vexpress-sysreg,sys_led";
 						gpio-controller;
 						#gpio-cells = <2>;
 					};
 
-					v2m_mmc_gpios: sys_mci {
+					v2m_mmc_gpios: sys-mci {
 						compatible = "arm,vexpress-sysreg,sys_mci";
 						gpio-controller;
 						#gpio-cells = <2>;
 					};
 
-					v2m_flash_gpios: sys_flash {
+					v2m_flash_gpios: sys-flash {
 						compatible = "arm,vexpress-sysreg,sys_flash";
 						gpio-controller;
 						#gpio-cells = <2>;
-- 
2.7.4

^ permalink raw reply related

* [PATCH v7 08/16] KVM: arm64: Optimise FPSIMD handling to reduce guest/host thrashing
From: Marc Zyngier @ 2018-05-09 16:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525882385-29181-9-git-send-email-Dave.Martin@arm.com>

On 09/05/18 17:12, Dave Martin wrote:
> This patch refactors KVM to align the host and guest FPSIMD
> save/restore logic with each other for arm64.  This reduces the
> number of redundant save/restore operations that must occur, and
> reduces the common-case IRQ blackout time during guest exit storms
> by saving the host state lazily and optimising away the need to
> restore the host state before returning to the run loop.
> 
> Four hooks are defined in order to enable this:
> 
>  * kvm_arch_vcpu_run_map_fp():
>    Called on PID change to map necessary bits of current to Hyp.
> 
>  * kvm_arch_vcpu_load_fp():
>    Set up FP/SIMD for entering the KVM run loop (parse as
>    "vcpu_load fp").
> 
>  * kvm_arch_vcpu_ctxsync_fp():
>    Get FP/SIMD into a safe state for re-enabling interrupts after a
>    guest exit back to the run loop.
> 
>    For arm64 specifically, this involves updating the host kernel's
>    FPSIMD context tracking metadata so that kernel-mode NEON use
>    will cause the vcpu's FPSIMD state to be saved back correctly
>    into the vcpu struct.  This must be done before re-enabling
>    interrupts because kernel-mode NEON may be used by softirqs.
> 
>  * kvm_arch_vcpu_put_fp():
>    Save guest FP/SIMD state back to memory and dissociate from the
>    CPU ("vcpu_put fp").
> 
> Also, the arm64 FPSIMD context switch code is updated to enable it
> to save back FPSIMD state for a vcpu, not just current.  A few
> helpers drive this:
> 
>  * fpsimd_bind_state_to_cpu(struct user_fpsimd_state *fp):
>    mark this CPU as having context fp (which may belong to a vcpu)
>    currently loaded in its registers.  This is the non-task
>    equivalent of the static function fpsimd_bind_to_cpu() in
>    fpsimd.c.
> 
>  * task_fpsimd_save():
>    exported to allow KVM to save the guest's FPSIMD state back to
>    memory on exit from the run loop.
> 
>  * fpsimd_flush_state():
>    invalidate any context's FPSIMD state that is currently loaded.
>    Used to disassociate the vcpu from the CPU regs on run loop exit.
> 
> These changes allow the run loop to enable interrupts (and thus
> softirqs that may use kernel-mode NEON) without having to save the
> guest's FPSIMD state eagerly.
> 
> Some new vcpu_arch fields are added to make all this work.  Because
> host FPSIMD state can now be saved back directly into current's
> thread_struct as appropriate, host_cpu_context is no longer used
> for preserving the FPSIMD state.  However, it is still needed for
> preserving other things such as the host's system registers.  To
> avoid ABI churn, the redundant storage space in host_cpu_context is
> not removed for now.
> 
> arch/arm is not addressed by this patch and continues to use its
> current save/restore logic.  It could provide implementations of
> the helpers later if desired.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> 
> ---
> 
> Dropped Reviewed-bys due to non-trivial changes.
> 
> Changes since v6:
> 
>  * Don't define kvm_arch_vcpu_run_pid_change() unless CONFIG_KVM=y.
> 
>    <asm/kvm_host.h> may be used for its declarations even with
>    CONFIG_KVM=n (e.g., in asm-offsets.c).
> 
>    This patch avoids conflicts with the core headers in this config.
> 
>  * Rebind current's FP state to the cpu in vcpu_put() if it is
>    still loaded, to ensure that the SVE trapping setup for userspace is
>    properly restored.
> 
> Requested by Marc Zyngier:
> 
>  * Add a comment to explain the purpose of update_fp_enabled().
> 
>  * Migrate vcpu_arch.flags definitions to kvm_host.h.
> 
>  * Eliminate magic NULL semantics for vcpu_arch.host_fpsimd_state so
>    that we can just assign this pointer once in the pid_change hook.
> 
>    A new flag KVM_ARM64_FP_HOST flag is added to capture the former
>    semantics of vcpu->arch.host_fpsimd_state != NULL.
Thanks for the additional rework, that looks much better now.

Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>

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

^ permalink raw reply

* [PATCH v7 02/16] arm64: Use update{,_tsk}_thread_flag()
From: Marc Zyngier @ 2018-05-09 16:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180509162753.GP7753@e103592.cambridge.arm.com>

On 09/05/18 17:27, Dave Martin wrote:
> On Wed, May 09, 2018 at 05:17:28PM +0100, Will Deacon wrote:
>> Hi Dave,
>>
>> On Wed, May 09, 2018 at 05:12:51PM +0100, Dave Martin wrote:
>>> This patch uses the new update_thread_flag() helpers to simplify a
>>> couple of if () set; else clear; constructs.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
>>> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: Will Deacon <will.deacon@arm.com>
>>> ---
>>>  arch/arm64/kernel/fpsimd.c | 19 +++++++------------
>>>  1 file changed, 7 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
>>> index 87a3536..0c4e7e0 100644
>>> --- a/arch/arm64/kernel/fpsimd.c
>>> +++ b/arch/arm64/kernel/fpsimd.c
> 
> [...]
> 
>>> @@ -902,7 +900,7 @@ void fpsimd_thread_switch(struct task_struct *next)
>>>  	if (current->mm)
>>>  		task_fpsimd_save();
>>>  
>>> -	if (next->mm) {
>>> +	if (next->mm)
>>>  		/*
>>>  		 * If we are switching to a task whose most recent userland
>>>  		 * FPSIMD state is already in the registers of *this* cpu,
>>> @@ -910,13 +908,10 @@ void fpsimd_thread_switch(struct task_struct *next)
>>>  		 * the TIF_FOREIGN_FPSTATE flag so the state will be loaded
>>>  		 * upon the next return to userland.
>>>  		 */
>>> -		if (__this_cpu_read(fpsimd_last_state.st) ==
>>> -			&next->thread.uw.fpsimd_state
>>> -		    && next->thread.fpsimd_cpu == smp_processor_id())
>>> -			clear_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE);
>>> -		else
>>> -			set_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE);
>>> -	}
>>> +		update_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE,
>>> +			__this_cpu_read(fpsimd_last_state.st) !=
>>> +				&next->thread.uw.fpsimd_state ||
>>> +			next->thread.fpsimd_cpu != smp_processor_id());
>>
>> Given the multi-line comment and this multi-line call, I'd be inclined to
>> leave the curlies in place and then use a local bool for the complex
>> condition.
> 
> Hey, curlies cost money, you know.
> 
>> With that:
>>
>> Acked-by: Will Deacon <will.deacon@arm.com>
> 
> Are you content to see this merged without the change?  It doesn't seem
> worth a respin of the whole series just for this.  I agree the code
> would be clearer, but this patch doesn't actually make it worse IMHO.
> 
> If I respin for some reason though, I can address this and add your Ack.

I'm happy to perform the change myself when applying the series if there
is no additional comment that would lead to a respin.

Thanks,

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

^ permalink raw reply

* [PATCH v7 08/16] KVM: arm64: Optimise FPSIMD handling to reduce guest/host thrashing
From: Dave Martin @ 2018-05-09 17:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <b53227b8-8532-a395-184d-dc2b3654f607@arm.com>

On Wed, May 09, 2018 at 05:54:25PM +0100, Marc Zyngier wrote:
> On 09/05/18 17:12, Dave Martin wrote:
> > This patch refactors KVM to align the host and guest FPSIMD
> > save/restore logic with each other for arm64.  This reduces the
> > number of redundant save/restore operations that must occur, and
> > reduces the common-case IRQ blackout time during guest exit storms
> > by saving the host state lazily and optimising away the need to
> > restore the host state before returning to the run loop.
> > 
> > Four hooks are defined in order to enable this:
> > 
> >  * kvm_arch_vcpu_run_map_fp():
> >    Called on PID change to map necessary bits of current to Hyp.
> > 
> >  * kvm_arch_vcpu_load_fp():
> >    Set up FP/SIMD for entering the KVM run loop (parse as
> >    "vcpu_load fp").
> > 
> >  * kvm_arch_vcpu_ctxsync_fp():
> >    Get FP/SIMD into a safe state for re-enabling interrupts after a
> >    guest exit back to the run loop.
> > 
> >    For arm64 specifically, this involves updating the host kernel's
> >    FPSIMD context tracking metadata so that kernel-mode NEON use
> >    will cause the vcpu's FPSIMD state to be saved back correctly
> >    into the vcpu struct.  This must be done before re-enabling
> >    interrupts because kernel-mode NEON may be used by softirqs.
> > 
> >  * kvm_arch_vcpu_put_fp():
> >    Save guest FP/SIMD state back to memory and dissociate from the
> >    CPU ("vcpu_put fp").
> > 
> > Also, the arm64 FPSIMD context switch code is updated to enable it
> > to save back FPSIMD state for a vcpu, not just current.  A few
> > helpers drive this:
> > 
> >  * fpsimd_bind_state_to_cpu(struct user_fpsimd_state *fp):
> >    mark this CPU as having context fp (which may belong to a vcpu)
> >    currently loaded in its registers.  This is the non-task
> >    equivalent of the static function fpsimd_bind_to_cpu() in
> >    fpsimd.c.
> > 
> >  * task_fpsimd_save():
> >    exported to allow KVM to save the guest's FPSIMD state back to
> >    memory on exit from the run loop.
> > 
> >  * fpsimd_flush_state():
> >    invalidate any context's FPSIMD state that is currently loaded.
> >    Used to disassociate the vcpu from the CPU regs on run loop exit.
> > 
> > These changes allow the run loop to enable interrupts (and thus
> > softirqs that may use kernel-mode NEON) without having to save the
> > guest's FPSIMD state eagerly.
> > 
> > Some new vcpu_arch fields are added to make all this work.  Because
> > host FPSIMD state can now be saved back directly into current's
> > thread_struct as appropriate, host_cpu_context is no longer used
> > for preserving the FPSIMD state.  However, it is still needed for
> > preserving other things such as the host's system registers.  To
> > avoid ABI churn, the redundant storage space in host_cpu_context is
> > not removed for now.
> > 
> > arch/arm is not addressed by this patch and continues to use its
> > current save/restore logic.  It could provide implementations of
> > the helpers later if desired.
> > 
> > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> > 
> > ---
> > 
> > Dropped Reviewed-bys due to non-trivial changes.
> > 
> > Changes since v6:
> > 
> >  * Don't define kvm_arch_vcpu_run_pid_change() unless CONFIG_KVM=y.
> > 
> >    <asm/kvm_host.h> may be used for its declarations even with
> >    CONFIG_KVM=n (e.g., in asm-offsets.c).
> > 
> >    This patch avoids conflicts with the core headers in this config.
> > 
> >  * Rebind current's FP state to the cpu in vcpu_put() if it is
> >    still loaded, to ensure that the SVE trapping setup for userspace is
> >    properly restored.
> > 
> > Requested by Marc Zyngier:
> > 
> >  * Add a comment to explain the purpose of update_fp_enabled().
> > 
> >  * Migrate vcpu_arch.flags definitions to kvm_host.h.
> > 
> >  * Eliminate magic NULL semantics for vcpu_arch.host_fpsimd_state so
> >    that we can just assign this pointer once in the pid_change hook.
> > 
> >    A new flag KVM_ARM64_FP_HOST flag is added to capture the former
> >    semantics of vcpu->arch.host_fpsimd_state != NULL.
> Thanks for the additional rework, that looks much better now.
> 
> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>

Cool, thanks for wrapping your head around it.

Cheers
---Dave

^ permalink raw reply

* [PATCH 0/8] firmware: arm_scmi: trivial cleanups
From: Sudeep Holla @ 2018-05-09 17:07 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

This contains all of the trivial review comments that were not addressed
as the series was queued for v4.17 and were not critical.

Regards,
Sudeep

Sudeep Holla (8):
  firmware: arm_scmi: improve code readability using bitfield accessor
    macros
  firmware: arm_scmi: fix kernel-docs documentation
  firmware: arm_scmi: rename get_transition_latency and
    add_opps_to_device
  firmware: arm_scmi: rename scmi_xfer_{init,get,put}
  firmware: arm_scmi: drop unused `con_priv` structure member
  firmware: arm_scmi: remove unnecessary bitmap_zero
  firmware: arm_scmi: improve exit paths and code readability
  firmware: arm_scmi: simplify exit path by returning on error

 drivers/cpufreq/scmi-cpufreq.c      |   4 +-
 drivers/firmware/arm_scmi/base.c    |  43 +++++++-------
 drivers/firmware/arm_scmi/bus.c     |  22 ++++----
 drivers/firmware/arm_scmi/clock.c   |  24 ++++----
 drivers/firmware/arm_scmi/common.h  |  22 ++++----
 drivers/firmware/arm_scmi/driver.c  | 109 ++++++++++++++++++------------------
 drivers/firmware/arm_scmi/perf.c    |  38 ++++++-------
 drivers/firmware/arm_scmi/power.c   |  16 +++---
 drivers/firmware/arm_scmi/sensors.c |  20 +++----
 include/linux/scmi_protocol.h       |  18 ++++--
 10 files changed, 166 insertions(+), 150 deletions(-)

--
2.7.4

^ permalink raw reply

* [PATCH 1/8] firmware: arm_scmi: improve code readability using bitfield accessor macros
From: Sudeep Holla @ 2018-05-09 17:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525885634-22348-1-git-send-email-sudeep.holla@arm.com>

By using FIELD_{FIT,GET,PREP} and GENMASK macro accessors we can avoid
some clumpsy custom shifting and masking macros and also improve the
code better readability.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scmi/common.h |  9 +++++----
 drivers/firmware/arm_scmi/driver.c | 31 ++++++++++++++-----------------
 2 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 0c30234f9098..e8f332c9c469 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -7,6 +7,7 @@
  * Copyright (C) 2018 ARM Ltd.
  */
 
+#include <linux/bitfield.h>
 #include <linux/completion.h>
 #include <linux/device.h>
 #include <linux/errno.h>
@@ -14,10 +15,10 @@
 #include <linux/scmi_protocol.h>
 #include <linux/types.h>
 
-#define PROTOCOL_REV_MINOR_BITS	16
-#define PROTOCOL_REV_MINOR_MASK	((1U << PROTOCOL_REV_MINOR_BITS) - 1)
-#define PROTOCOL_REV_MAJOR(x)	((x) >> PROTOCOL_REV_MINOR_BITS)
-#define PROTOCOL_REV_MINOR(x)	((x) & PROTOCOL_REV_MINOR_MASK)
+#define PROTOCOL_REV_MINOR_MASK	GENMASK(15, 0)
+#define PROTOCOL_REV_MAJOR_MASK	GENMASK(31, 16)
+#define PROTOCOL_REV_MAJOR(x)	(u16)(FIELD_GET(PROTOCOL_REV_MAJOR_MASK, (x)))
+#define PROTOCOL_REV_MINOR(x)	(u16)(FIELD_GET(PROTOCOL_REV_MINOR_MASK, (x)))
 #define MAX_PROTOCOLS_IMP	16
 #define MAX_OPPS		16
 
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 14b147135a0c..917786d91f55 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -29,16 +29,12 @@
 
 #include "common.h"
 
-#define MSG_ID_SHIFT		0
-#define MSG_ID_MASK		0xff
-#define MSG_TYPE_SHIFT		8
-#define MSG_TYPE_MASK		0x3
-#define MSG_PROTOCOL_ID_SHIFT	10
-#define MSG_PROTOCOL_ID_MASK	0xff
-#define MSG_TOKEN_ID_SHIFT	18
-#define MSG_TOKEN_ID_MASK	0x3ff
-#define MSG_XTRACT_TOKEN(header)	\
-	(((header) >> MSG_TOKEN_ID_SHIFT) & MSG_TOKEN_ID_MASK)
+#define MSG_ID_MASK		GENMASK(7, 0)
+#define MSG_TYPE_MASK		GENMASK(9, 8)
+#define MSG_PROTOCOL_ID_MASK	GENMASK(17, 10)
+#define MSG_TOKEN_ID_MASK	GENMASK(27, 18)
+#define MSG_XTRACT_TOKEN(hdr)	FIELD_GET(MSG_TOKEN_ID_MASK, (hdr))
+#define MSG_TOKEN_MAX		(MSG_XTRACT_TOKEN(MSG_TOKEN_ID_MASK) + 1)
 
 enum scmi_error_codes {
 	SCMI_SUCCESS = 0,	/* Success */
@@ -255,9 +251,9 @@ static void scmi_rx_callback(struct mbox_client *cl, void *m)
  */
 static inline u32 pack_scmi_header(struct scmi_msg_hdr *hdr)
 {
-	return ((hdr->id & MSG_ID_MASK) << MSG_ID_SHIFT) |
-	   ((hdr->seq & MSG_TOKEN_ID_MASK) << MSG_TOKEN_ID_SHIFT) |
-	   ((hdr->protocol_id & MSG_PROTOCOL_ID_MASK) << MSG_PROTOCOL_ID_SHIFT);
+	return FIELD_PREP(MSG_ID_MASK, hdr->id) |
+		FIELD_PREP(MSG_TOKEN_ID_MASK, hdr->seq) |
+		FIELD_PREP(MSG_PROTOCOL_ID_MASK, hdr->protocol_id);
 }
 
 /**
@@ -621,9 +617,9 @@ static int scmi_xfer_info_init(struct scmi_info *sinfo)
 	struct scmi_xfers_info *info = &sinfo->minfo;
 
 	/* Pre-allocated messages, no more than what hdr.seq can support */
-	if (WARN_ON(desc->max_msg >= (MSG_TOKEN_ID_MASK + 1))) {
-		dev_err(dev, "Maximum message of %d exceeds supported %d\n",
-			desc->max_msg, MSG_TOKEN_ID_MASK + 1);
+	if (WARN_ON(desc->max_msg >= MSG_TOKEN_MAX)) {
+		dev_err(dev, "Maximum message of %d exceeds supported %ld\n",
+			desc->max_msg, MSG_TOKEN_MAX);
 		return -EINVAL;
 	}
 
@@ -840,7 +836,8 @@ static int scmi_probe(struct platform_device *pdev)
 		if (of_property_read_u32(child, "reg", &prot_id))
 			continue;
 
-		prot_id &= MSG_PROTOCOL_ID_MASK;
+		if (!FIELD_FIT(MSG_PROTOCOL_ID_MASK, prot_id))
+			dev_err(dev, "Out of range protocol %d\n", prot_id);
 
 		if (!scmi_is_protocol_implemented(handle, prot_id)) {
 			dev_err(dev, "SCMI protocol %d not implemented\n",
-- 
2.7.4

^ permalink raw reply related

* [PATCH 2/8] firmware: arm_scmi: fix kernel-docs documentation
From: Sudeep Holla @ 2018-05-09 17:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525885634-22348-1-git-send-email-sudeep.holla@arm.com>

There are few missing descriptions for function parameters and structure
members along with certain instances where excessive function parameters
or structure members are described.

This patch fixes all of those warnings.

Reported-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scmi/base.c   | 20 +++++++++--------
 drivers/firmware/arm_scmi/common.h |  7 ++++--
 drivers/firmware/arm_scmi/driver.c | 45 ++++++++++++++++++++------------------
 include/linux/scmi_protocol.h      |  8 +++++++
 4 files changed, 48 insertions(+), 32 deletions(-)

diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c
index 0d3806c0d432..c36ded9dbb83 100644
--- a/drivers/firmware/arm_scmi/base.c
+++ b/drivers/firmware/arm_scmi/base.c
@@ -26,7 +26,7 @@ struct scmi_msg_resp_base_attributes {
  * scmi_base_attributes_get() - gets the implementation details
  *	that are associated with the base protocol.
  *
- * @handle - SCMI entity handle
+ * @handle: SCMI entity handle
  *
  * Return: 0 on success, else appropriate SCMI error.
  */
@@ -50,14 +50,15 @@ static int scmi_base_attributes_get(const struct scmi_handle *handle)
 	}
 
 	scmi_one_xfer_put(handle, t);
+
 	return ret;
 }
 
 /**
  * scmi_base_vendor_id_get() - gets vendor/subvendor identifier ASCII string.
  *
- * @handle - SCMI entity handle
- * @sub_vendor - specify true if sub-vendor ID is needed
+ * @handle: SCMI entity handle
+ * @sub_vendor: specify true if sub-vendor ID is needed
  *
  * Return: 0 on success, else appropriate SCMI error.
  */
@@ -97,7 +98,7 @@ scmi_base_vendor_id_get(const struct scmi_handle *handle, bool sub_vendor)
  *	implementation 32-bit version. The format of the version number is
  *	vendor-specific
  *
- * @handle - SCMI entity handle
+ * @handle: SCMI entity handle
  *
  * Return: 0 on success, else appropriate SCMI error.
  */
@@ -128,8 +129,8 @@ scmi_base_implementation_version_get(const struct scmi_handle *handle)
  * scmi_base_implementation_list_get() - gets the list of protocols it is
  *	OSPM is allowed to access
  *
- * @handle - SCMI entity handle
- * @protocols_imp - pointer to hold the list of protocol identifiers
+ * @handle: SCMI entity handle
+ * @protocols_imp: pointer to hold the list of protocol identifiers
  *
  * Return: 0 on success, else appropriate SCMI error.
  */
@@ -173,15 +174,16 @@ static int scmi_base_implementation_list_get(const struct scmi_handle *handle,
 	} while (loop_num_ret);
 
 	scmi_one_xfer_put(handle, t);
+
 	return ret;
 }
 
 /**
  * scmi_base_discover_agent_get() - discover the name of an agent
  *
- * @handle - SCMI entity handle
- * @id - Agent identifier
- * @name - Agent identifier ASCII string
+ * @handle: SCMI entity handle
+ * @id: Agent identifier
+ * @name: Agent identifier ASCII string
  *
  * An agent id of 0 is reserved to identify the platform itself.
  * Generally operating system is represented as "OSPM"
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index e8f332c9c469..0821662a4633 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -51,8 +51,11 @@ struct scmi_msg_resp_prot_version {
  * @id: The identifier of the command being sent
  * @protocol_id: The identifier of the protocol used to send @id command
  * @seq: The token to identify the message. when a message/command returns,
- *       the platform returns the whole message header unmodified including
- *	 the token.
+ *	the platform returns the whole message header unmodified including
+ *	the token
+ * @status: Status of the transfer once it's complete
+ * @poll_completion: Indicate if the transfer needs to be polled for
+ *	completion or interrupt mode is used
  */
 struct scmi_msg_hdr {
 	u8 id;
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 917786d91f55..6fee11f06a66 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -51,7 +51,7 @@ enum scmi_error_codes {
 	SCMI_ERR_MAX
 };
 
-/* List of all  SCMI devices active in system */
+/* List of all SCMI devices active in system */
 static LIST_HEAD(scmi_list);
 /* Protection for the entire list */
 static DEFINE_MUTEX(scmi_list_mutex);
@@ -68,7 +68,6 @@ static DEFINE_MUTEX(scmi_list_mutex);
 struct scmi_xfers_info {
 	struct scmi_xfer *xfer_block;
 	unsigned long *xfer_alloc_table;
-	/* protect transfer allocation */
 	spinlock_t xfer_lock;
 };
 
@@ -94,6 +93,7 @@ struct scmi_desc {
  * @payload: Transmit/Receive mailbox channel payload area
  * @dev: Reference to device in the SCMI hierarchy corresponding to this
  *	 channel
+ * @handle: Pointer to SCMI entity handle
  */
 struct scmi_chan_info {
 	struct mbox_client cl;
@@ -104,7 +104,7 @@ struct scmi_chan_info {
 };
 
 /**
- * struct scmi_info - Structure representing a  SCMI instance
+ * struct scmi_info - Structure representing a SCMI instance
  *
  * @dev: Device pointer
  * @desc: SoC description for this instance
@@ -113,9 +113,9 @@ struct scmi_chan_info {
  *	implementation version and (sub-)vendor identification.
  * @minfo: Message info
  * @tx_idr: IDR object to map protocol id to channel info pointer
- * @protocols_imp: list of protocols implemented, currently maximum of
+ * @protocols_imp: List of protocols implemented, currently maximum of
  *	MAX_PROTOCOLS_IMP elements allocated by the base protocol
- * @node: list head
+ * @node: List head
  * @users: Number of users of this instance
  */
 struct scmi_info {
@@ -221,9 +221,7 @@ static void scmi_rx_callback(struct mbox_client *cl, void *m)
 
 	xfer_id = MSG_XTRACT_TOKEN(ioread32(&mem->msg_header));
 
-	/*
-	 * Are we even expecting this?
-	 */
+	/* Are we even expecting this? */
 	if (!test_bit(xfer_id, minfo->xfer_alloc_table)) {
 		dev_err(dev, "message for %d is not expected!\n", xfer_id);
 		return;
@@ -248,6 +246,8 @@ static void scmi_rx_callback(struct mbox_client *cl, void *m)
  *
  * @hdr: pointer to header containing all the information on message id,
  *	protocol id and sequence id.
+ *
+ * Return: 32-bit packed command header to be sent to the platform.
  */
 static inline u32 pack_scmi_header(struct scmi_msg_hdr *hdr)
 {
@@ -282,9 +282,9 @@ static void scmi_tx_prepare(struct mbox_client *cl, void *m)
 }
 
 /**
- * scmi_one_xfer_get() - Allocate one message
+ * scmi_xfer_get() - Allocate one message
  *
- * @handle: SCMI entity handle
+ * @handle: Pointer to SCMI entity handle
  *
  * Helper function which is used by various command functions that are
  * exposed to clients of this driver for allocating a message traffic event.
@@ -326,8 +326,8 @@ static struct scmi_xfer *scmi_one_xfer_get(const struct scmi_handle *handle)
 /**
  * scmi_one_xfer_put() - Release a message
  *
- * @minfo: transfer info pointer
- * @xfer: message that was reserved by scmi_one_xfer_get
+ * @handle: Pointer to SCMI entity handle
+ * @xfer: message that was reserved by scmi_xfer_get
  *
  * This holds a spinlock to maintain integrity of internal data structures.
  */
@@ -374,12 +374,12 @@ static bool scmi_xfer_done_no_timeout(const struct scmi_chan_info *cinfo,
 /**
  * scmi_do_xfer() - Do one transfer
  *
- * @info: Pointer to SCMI entity information
+ * @handle: Pointer to SCMI entity handle
  * @xfer: Transfer to initiate and wait for response
  *
  * Return: -ETIMEDOUT in case of no response, if transmit error,
- *   return corresponding error, else if all goes well,
- *   return 0.
+ *	return corresponding error, else if all goes well,
+ *	return 0.
  */
 int scmi_do_xfer(const struct scmi_handle *handle, struct scmi_xfer *xfer)
 {
@@ -438,9 +438,9 @@ int scmi_do_xfer(const struct scmi_handle *handle, struct scmi_xfer *xfer)
 /**
  * scmi_one_xfer_init() - Allocate and initialise one message
  *
- * @handle: SCMI entity handle
+ * @handle: Pointer to SCMI entity handle
  * @msg_id: Message identifier
- * @msg_prot_id: Protocol identifier for the message
+ * @prot_id: Protocol identifier for the message
  * @tx_size: transmit message size
  * @rx_size: receive message size
  * @p: pointer to the allocated and initialised message
@@ -478,13 +478,16 @@ int scmi_one_xfer_init(const struct scmi_handle *handle, u8 msg_id, u8 prot_id,
 	xfer->hdr.poll_completion = false;
 
 	*p = xfer;
+
 	return 0;
 }
 
 /**
  * scmi_version_get() - command to get the revision of the SCMI entity
  *
- * @handle: Handle to SCMI entity information
+ * @handle: Pointer to SCMI entity handle
+ * @protocol: Protocol identifier for the message
+ * @version: Holds returned version of protocol.
  *
  * Updates the SCMI information in the internal data structure.
  *
@@ -541,7 +544,7 @@ scmi_is_protocol_implemented(const struct scmi_handle *handle, u8 prot_id)
  * @dev: pointer to device for which we want SCMI handle
  *
  * NOTE: The function does not track individual clients of the framework
- * and is expected to be maintained by caller of  SCMI protocol library.
+ * and is expected to be maintained by caller of SCMI protocol library.
  * scmi_handle_put must be balanced with successful scmi_handle_get
  *
  * Return: pointer to handle if successful, NULL on error
@@ -572,7 +575,7 @@ struct scmi_handle *scmi_handle_get(struct device *dev)
  * @handle: handle acquired by scmi_handle_get
  *
  * NOTE: The function does not track individual clients of the framework
- * and is expected to be maintained by caller of  SCMI protocol library.
+ * and is expected to be maintained by caller of SCMI protocol library.
  * scmi_handle_put must be balanced with successful scmi_handle_get
  *
  * Return: 0 is successfully released
@@ -595,7 +598,7 @@ int scmi_handle_put(const struct scmi_handle *handle)
 }
 
 static const struct scmi_desc scmi_generic_desc = {
-	.max_rx_timeout_ms = 30,	/* we may increase this if required */
+	.max_rx_timeout_ms = 30,	/* We may increase this if required */
 	.max_msg = 20,		/* Limited by MBOX_TX_QUEUE_LEN */
 	.max_msg_size = 128,
 };
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index b458c87b866c..a171c1e293e8 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -189,6 +189,14 @@ struct scmi_sensor_ops {
  * @perf_ops: pointer to set of performance protocol operations
  * @clk_ops: pointer to set of clock protocol operations
  * @sensor_ops: pointer to set of sensor protocol operations
+ * @perf_priv: pointer to private data structure specific to performance
+ *	protocol(for internal use only)
+ * @clk_priv: pointer to private data structure specific to clock
+ *	protocol(for internal use only)
+ * @power_priv: pointer to private data structure specific to power
+ *	protocol(for internal use only)
+ * @sensor_priv: pointer to private data structure specific to sensors
+ *	protocol(for internal use only)
  */
 struct scmi_handle {
 	struct device *dev;
-- 
2.7.4

^ permalink raw reply related

* [PATCH 3/8] firmware: arm_scmi: rename get_transition_latency and add_opps_to_device
From: Sudeep Holla @ 2018-05-09 17:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525885634-22348-1-git-send-email-sudeep.holla@arm.com>

Most of the scmi code follows the suggestion from Greg KH on a totally
different thread[0] to have the subsystem name first, followed by the
noun and finally the verb with couple of these exceptions.

This patch fixes them so that all the functions names are aligned to
on practice.

[0] https://www.spinics.net/lists/arm-kernel/msg583673.html

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/cpufreq/scmi-cpufreq.c   |  4 ++--
 drivers/firmware/arm_scmi/perf.c | 10 +++++-----
 include/linux/scmi_protocol.h    | 10 +++++-----
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c
index b4dbc77459b6..50b1551ba894 100644
--- a/drivers/cpufreq/scmi-cpufreq.c
+++ b/drivers/cpufreq/scmi-cpufreq.c
@@ -117,7 +117,7 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy)
 		return -ENODEV;
 	}
 
-	ret = handle->perf_ops->add_opps_to_device(handle, cpu_dev);
+	ret = handle->perf_ops->device_opps_add(handle, cpu_dev);
 	if (ret) {
 		dev_warn(cpu_dev, "failed to add opps to the device\n");
 		return ret;
@@ -164,7 +164,7 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy)
 	/* SCMI allows DVFS request for any domain from any CPU */
 	policy->dvfs_possible_from_any_cpu = true;
 
-	latency = handle->perf_ops->get_transition_latency(handle, cpu_dev);
+	latency = handle->perf_ops->transition_latency_get(handle, cpu_dev);
 	if (!latency)
 		latency = CPUFREQ_ETERNAL;
 
diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
index 987c64d19801..611ab08e6174 100644
--- a/drivers/firmware/arm_scmi/perf.c
+++ b/drivers/firmware/arm_scmi/perf.c
@@ -349,8 +349,8 @@ static int scmi_dev_domain_id(struct device *dev)
 	return clkspec.args[0];
 }
 
-static int scmi_dvfs_add_opps_to_device(const struct scmi_handle *handle,
-					struct device *dev)
+static int scmi_dvfs_device_opps_add(const struct scmi_handle *handle,
+				     struct device *dev)
 {
 	int idx, ret, domain;
 	unsigned long freq;
@@ -383,7 +383,7 @@ static int scmi_dvfs_add_opps_to_device(const struct scmi_handle *handle,
 	return 0;
 }
 
-static int scmi_dvfs_get_transition_latency(const struct scmi_handle *handle,
+static int scmi_dvfs_transition_latency_get(const struct scmi_handle *handle,
 					    struct device *dev)
 {
 	struct perf_dom_info *dom;
@@ -432,8 +432,8 @@ static struct scmi_perf_ops perf_ops = {
 	.level_set = scmi_perf_level_set,
 	.level_get = scmi_perf_level_get,
 	.device_domain_id = scmi_dev_domain_id,
-	.get_transition_latency = scmi_dvfs_get_transition_latency,
-	.add_opps_to_device = scmi_dvfs_add_opps_to_device,
+	.transition_latency_get = scmi_dvfs_transition_latency_get,
+	.device_opps_add = scmi_dvfs_device_opps_add,
 	.freq_set = scmi_dvfs_freq_set,
 	.freq_get = scmi_dvfs_freq_get,
 };
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index a171c1e293e8..f4c9fc0fc755 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -85,8 +85,8 @@ struct scmi_clk_ops {
  * @level_set: sets the performance level of a domain
  * @level_get: gets the performance level of a domain
  * @device_domain_id: gets the scmi domain id for a given device
- * @get_transition_latency: gets the DVFS transition latency for a given device
- * @add_opps_to_device: adds all the OPPs for a given device
+ * @transition_latency_get: gets the DVFS transition latency for a given device
+ * @device_opps_add: adds all the OPPs for a given device
  * @freq_set: sets the frequency for a given device using sustained frequency
  *	to sustained performance level mapping
  * @freq_get: gets the frequency for a given device using sustained frequency
@@ -102,10 +102,10 @@ struct scmi_perf_ops {
 	int (*level_get)(const struct scmi_handle *handle, u32 domain,
 			 u32 *level, bool poll);
 	int (*device_domain_id)(struct device *dev);
-	int (*get_transition_latency)(const struct scmi_handle *handle,
+	int (*transition_latency_get)(const struct scmi_handle *handle,
 				      struct device *dev);
-	int (*add_opps_to_device)(const struct scmi_handle *handle,
-				  struct device *dev);
+	int (*device_opps_add)(const struct scmi_handle *handle,
+			       struct device *dev);
 	int (*freq_set)(const struct scmi_handle *handle, u32 domain,
 			unsigned long rate, bool poll);
 	int (*freq_get)(const struct scmi_handle *handle, u32 domain,
-- 
2.7.4

^ permalink raw reply related

* [PATCH 4/8] firmware: arm_scmi: rename scmi_xfer_{init,get,put}
From: Sudeep Holla @ 2018-05-09 17:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525885634-22348-1-git-send-email-sudeep.holla@arm.com>

Just after the initial patches were queued, Jonathan Cameron mentioned
that scmi_one_xfer_{get_put} were not very clear and suggested to use
scmi_xfer_{alloc,free}. While I agree to some extent, the reason not to
have alloc/free as these are preallocated buffers and these functions
just returns a reference to free slot in that preallocated array.
However it was agreed to drop "_one" as it's implicit that we are always
dealing with one slot anyways.

This patch updates the name accordingly dropping "_one" in both {get,put}
functions. Also scmi_one_xfer_init is renamed as scmi_xfer_get_init to
reflect the fact that it gets the free slots and then initialise it.

Reported-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scmi/base.c    | 23 +++++++++++++----------
 drivers/firmware/arm_scmi/clock.c   | 24 ++++++++++++------------
 drivers/firmware/arm_scmi/common.h  |  4 ++--
 drivers/firmware/arm_scmi/driver.c  | 20 ++++++++++----------
 drivers/firmware/arm_scmi/perf.c    | 28 ++++++++++++++--------------
 drivers/firmware/arm_scmi/power.c   | 16 ++++++++--------
 drivers/firmware/arm_scmi/sensors.c | 20 ++++++++++----------
 7 files changed, 69 insertions(+), 66 deletions(-)

diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c
index c36ded9dbb83..9dff33ea6416 100644
--- a/drivers/firmware/arm_scmi/base.c
+++ b/drivers/firmware/arm_scmi/base.c
@@ -37,7 +37,7 @@ static int scmi_base_attributes_get(const struct scmi_handle *handle)
 	struct scmi_msg_resp_base_attributes *attr_info;
 	struct scmi_revision_info *rev = handle->version;
 
-	ret = scmi_one_xfer_init(handle, PROTOCOL_ATTRIBUTES,
+	ret = scmi_xfer_get_init(handle, PROTOCOL_ATTRIBUTES,
 				 SCMI_PROTOCOL_BASE, 0, sizeof(*attr_info), &t);
 	if (ret)
 		return ret;
@@ -49,7 +49,7 @@ static int scmi_base_attributes_get(const struct scmi_handle *handle)
 		rev->num_agents = attr_info->num_agents;
 	}
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 
 	return ret;
 }
@@ -81,7 +81,7 @@ scmi_base_vendor_id_get(const struct scmi_handle *handle, bool sub_vendor)
 		size = ARRAY_SIZE(rev->vendor_id);
 	}
 
-	ret = scmi_one_xfer_init(handle, cmd, SCMI_PROTOCOL_BASE, 0, size, &t);
+	ret = scmi_xfer_get_init(handle, cmd, SCMI_PROTOCOL_BASE, 0, size, &t);
 	if (ret)
 		return ret;
 
@@ -89,7 +89,8 @@ scmi_base_vendor_id_get(const struct scmi_handle *handle, bool sub_vendor)
 	if (!ret)
 		memcpy(vendor_id, t->rx.buf, size);
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
+
 	return ret;
 }
 
@@ -110,7 +111,7 @@ scmi_base_implementation_version_get(const struct scmi_handle *handle)
 	struct scmi_xfer *t;
 	struct scmi_revision_info *rev = handle->version;
 
-	ret = scmi_one_xfer_init(handle, BASE_DISCOVER_IMPLEMENT_VERSION,
+	ret = scmi_xfer_get_init(handle, BASE_DISCOVER_IMPLEMENT_VERSION,
 				 SCMI_PROTOCOL_BASE, 0, sizeof(*impl_ver), &t);
 	if (ret)
 		return ret;
@@ -121,7 +122,8 @@ scmi_base_implementation_version_get(const struct scmi_handle *handle)
 		rev->impl_ver = le32_to_cpu(*impl_ver);
 	}
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
+
 	return ret;
 }
 
@@ -144,7 +146,7 @@ static int scmi_base_implementation_list_get(const struct scmi_handle *handle,
 	u32 tot_num_ret = 0, loop_num_ret;
 	struct device *dev = handle->dev;
 
-	ret = scmi_one_xfer_init(handle, BASE_DISCOVER_LIST_PROTOCOLS,
+	ret = scmi_xfer_get_init(handle, BASE_DISCOVER_LIST_PROTOCOLS,
 				 SCMI_PROTOCOL_BASE, sizeof(*num_skip), 0, &t);
 	if (ret)
 		return ret;
@@ -173,7 +175,7 @@ static int scmi_base_implementation_list_get(const struct scmi_handle *handle,
 		tot_num_ret += loop_num_ret;
 	} while (loop_num_ret);
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 
 	return ret;
 }
@@ -196,7 +198,7 @@ static int scmi_base_discover_agent_get(const struct scmi_handle *handle,
 	int ret;
 	struct scmi_xfer *t;
 
-	ret = scmi_one_xfer_init(handle, BASE_DISCOVER_AGENT,
+	ret = scmi_xfer_get_init(handle, BASE_DISCOVER_AGENT,
 				 SCMI_PROTOCOL_BASE, sizeof(__le32),
 				 SCMI_MAX_STR_SIZE, &t);
 	if (ret)
@@ -208,7 +210,8 @@ static int scmi_base_discover_agent_get(const struct scmi_handle *handle,
 	if (!ret)
 		memcpy(name, t->rx.buf, SCMI_MAX_STR_SIZE);
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
+
 	return ret;
 }
 
diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
index e6f17825db79..3874666a8a14 100644
--- a/drivers/firmware/arm_scmi/clock.c
+++ b/drivers/firmware/arm_scmi/clock.c
@@ -77,7 +77,7 @@ static int scmi_clock_protocol_attributes_get(const struct scmi_handle *handle,
 	struct scmi_xfer *t;
 	struct scmi_msg_resp_clock_protocol_attributes *attr;
 
-	ret = scmi_one_xfer_init(handle, PROTOCOL_ATTRIBUTES,
+	ret = scmi_xfer_get_init(handle, PROTOCOL_ATTRIBUTES,
 				 SCMI_PROTOCOL_CLOCK, 0, sizeof(*attr), &t);
 	if (ret)
 		return ret;
@@ -90,7 +90,7 @@ static int scmi_clock_protocol_attributes_get(const struct scmi_handle *handle,
 		ci->max_async_req = attr->max_async_req;
 	}
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
@@ -101,7 +101,7 @@ static int scmi_clock_attributes_get(const struct scmi_handle *handle,
 	struct scmi_xfer *t;
 	struct scmi_msg_resp_clock_attributes *attr;
 
-	ret = scmi_one_xfer_init(handle, CLOCK_ATTRIBUTES, SCMI_PROTOCOL_CLOCK,
+	ret = scmi_xfer_get_init(handle, CLOCK_ATTRIBUTES, SCMI_PROTOCOL_CLOCK,
 				 sizeof(clk_id), sizeof(*attr), &t);
 	if (ret)
 		return ret;
@@ -115,7 +115,7 @@ static int scmi_clock_attributes_get(const struct scmi_handle *handle,
 	else
 		clk->name[0] = '\0';
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
@@ -132,7 +132,7 @@ scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id,
 	struct scmi_msg_clock_describe_rates *clk_desc;
 	struct scmi_msg_resp_clock_describe_rates *rlist;
 
-	ret = scmi_one_xfer_init(handle, CLOCK_DESCRIBE_RATES,
+	ret = scmi_xfer_get_init(handle, CLOCK_DESCRIBE_RATES,
 				 SCMI_PROTOCOL_CLOCK, sizeof(*clk_desc), 0, &t);
 	if (ret)
 		return ret;
@@ -186,7 +186,7 @@ scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id,
 		clk->list.num_rates = tot_rate_cnt;
 
 err:
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
@@ -196,7 +196,7 @@ scmi_clock_rate_get(const struct scmi_handle *handle, u32 clk_id, u64 *value)
 	int ret;
 	struct scmi_xfer *t;
 
-	ret = scmi_one_xfer_init(handle, CLOCK_RATE_GET, SCMI_PROTOCOL_CLOCK,
+	ret = scmi_xfer_get_init(handle, CLOCK_RATE_GET, SCMI_PROTOCOL_CLOCK,
 				 sizeof(__le32), sizeof(u64), &t);
 	if (ret)
 		return ret;
@@ -211,7 +211,7 @@ scmi_clock_rate_get(const struct scmi_handle *handle, u32 clk_id, u64 *value)
 		*value |= (u64)le32_to_cpu(*(pval + 1)) << 32;
 	}
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
@@ -222,7 +222,7 @@ static int scmi_clock_rate_set(const struct scmi_handle *handle, u32 clk_id,
 	struct scmi_xfer *t;
 	struct scmi_clock_set_rate *cfg;
 
-	ret = scmi_one_xfer_init(handle, CLOCK_RATE_SET, SCMI_PROTOCOL_CLOCK,
+	ret = scmi_xfer_get_init(handle, CLOCK_RATE_SET, SCMI_PROTOCOL_CLOCK,
 				 sizeof(*cfg), 0, &t);
 	if (ret)
 		return ret;
@@ -235,7 +235,7 @@ static int scmi_clock_rate_set(const struct scmi_handle *handle, u32 clk_id,
 
 	ret = scmi_do_xfer(handle, t);
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
@@ -246,7 +246,7 @@ scmi_clock_config_set(const struct scmi_handle *handle, u32 clk_id, u32 config)
 	struct scmi_xfer *t;
 	struct scmi_clock_set_config *cfg;
 
-	ret = scmi_one_xfer_init(handle, CLOCK_CONFIG_SET, SCMI_PROTOCOL_CLOCK,
+	ret = scmi_xfer_get_init(handle, CLOCK_CONFIG_SET, SCMI_PROTOCOL_CLOCK,
 				 sizeof(*cfg), 0, &t);
 	if (ret)
 		return ret;
@@ -257,7 +257,7 @@ scmi_clock_config_set(const struct scmi_handle *handle, u32 clk_id, u32 config)
 
 	ret = scmi_do_xfer(handle, t);
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 0821662a4633..41b03e46cca8 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -95,9 +95,9 @@ struct scmi_xfer {
 	struct completion done;
 };
 
-void scmi_one_xfer_put(const struct scmi_handle *h, struct scmi_xfer *xfer);
+void scmi_xfer_put(const struct scmi_handle *h, struct scmi_xfer *xfer);
 int scmi_do_xfer(const struct scmi_handle *h, struct scmi_xfer *xfer);
-int scmi_one_xfer_init(const struct scmi_handle *h, u8 msg_id, u8 prot_id,
+int scmi_xfer_get_init(const struct scmi_handle *h, u8 msg_id, u8 prot_id,
 		       size_t tx_size, size_t rx_size, struct scmi_xfer **p);
 int scmi_handle_put(const struct scmi_handle *handle);
 struct scmi_handle *scmi_handle_get(struct device *dev);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 6fee11f06a66..33d2b78af3ff 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -295,7 +295,7 @@ static void scmi_tx_prepare(struct mbox_client *cl, void *m)
  *
  * Return: 0 if all went fine, else corresponding error.
  */
-static struct scmi_xfer *scmi_one_xfer_get(const struct scmi_handle *handle)
+static struct scmi_xfer *scmi_xfer_get(const struct scmi_handle *handle)
 {
 	u16 xfer_id;
 	struct scmi_xfer *xfer;
@@ -324,14 +324,14 @@ static struct scmi_xfer *scmi_one_xfer_get(const struct scmi_handle *handle)
 }
 
 /**
- * scmi_one_xfer_put() - Release a message
+ * scmi_xfer_put() - Release a message
  *
  * @handle: Pointer to SCMI entity handle
  * @xfer: message that was reserved by scmi_xfer_get
  *
  * This holds a spinlock to maintain integrity of internal data structures.
  */
-void scmi_one_xfer_put(const struct scmi_handle *handle, struct scmi_xfer *xfer)
+void scmi_xfer_put(const struct scmi_handle *handle, struct scmi_xfer *xfer)
 {
 	unsigned long flags;
 	struct scmi_info *info = handle_to_scmi_info(handle);
@@ -436,7 +436,7 @@ int scmi_do_xfer(const struct scmi_handle *handle, struct scmi_xfer *xfer)
 }
 
 /**
- * scmi_one_xfer_init() - Allocate and initialise one message
+ * scmi_xfer_get_init() - Allocate and initialise one message
  *
  * @handle: Pointer to SCMI entity handle
  * @msg_id: Message identifier
@@ -445,13 +445,13 @@ int scmi_do_xfer(const struct scmi_handle *handle, struct scmi_xfer *xfer)
  * @rx_size: receive message size
  * @p: pointer to the allocated and initialised message
  *
- * This function allocates the message using @scmi_one_xfer_get and
+ * This function allocates the message using @scmi_xfer_get and
  * initialise the header.
  *
  * Return: 0 if all went fine with @p pointing to message, else
  *	corresponding error.
  */
-int scmi_one_xfer_init(const struct scmi_handle *handle, u8 msg_id, u8 prot_id,
+int scmi_xfer_get_init(const struct scmi_handle *handle, u8 msg_id, u8 prot_id,
 		       size_t tx_size, size_t rx_size, struct scmi_xfer **p)
 {
 	int ret;
@@ -464,7 +464,7 @@ int scmi_one_xfer_init(const struct scmi_handle *handle, u8 msg_id, u8 prot_id,
 	    tx_size > info->desc->max_msg_size)
 		return -ERANGE;
 
-	xfer = scmi_one_xfer_get(handle);
+	xfer = scmi_xfer_get(handle);
 	if (IS_ERR(xfer)) {
 		ret = PTR_ERR(xfer);
 		dev_err(dev, "failed to get free message slot(%d)\n", ret);
@@ -500,7 +500,7 @@ int scmi_version_get(const struct scmi_handle *handle, u8 protocol,
 	__le32 *rev_info;
 	struct scmi_xfer *t;
 
-	ret = scmi_one_xfer_init(handle, PROTOCOL_VERSION, protocol, 0,
+	ret = scmi_xfer_get_init(handle, PROTOCOL_VERSION, protocol, 0,
 				 sizeof(*version), &t);
 	if (ret)
 		return ret;
@@ -511,7 +511,7 @@ int scmi_version_get(const struct scmi_handle *handle, u8 protocol,
 		*version = le32_to_cpu(*rev_info);
 	}
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
@@ -539,7 +539,7 @@ scmi_is_protocol_implemented(const struct scmi_handle *handle, u8 prot_id)
 }
 
 /**
- * scmi_handle_get() - Get the  SCMI handle for a device
+ * scmi_handle_get() - Get the SCMI handle for a device
  *
  * @dev: pointer to device for which we want SCMI handle
  *
diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
index 611ab08e6174..2a219b1261b1 100644
--- a/drivers/firmware/arm_scmi/perf.c
+++ b/drivers/firmware/arm_scmi/perf.c
@@ -115,7 +115,7 @@ static int scmi_perf_attributes_get(const struct scmi_handle *handle,
 	struct scmi_xfer *t;
 	struct scmi_msg_resp_perf_attributes *attr;
 
-	ret = scmi_one_xfer_init(handle, PROTOCOL_ATTRIBUTES,
+	ret = scmi_xfer_get_init(handle, PROTOCOL_ATTRIBUTES,
 				 SCMI_PROTOCOL_PERF, 0, sizeof(*attr), &t);
 	if (ret)
 		return ret;
@@ -133,7 +133,7 @@ static int scmi_perf_attributes_get(const struct scmi_handle *handle,
 		pi->stats_size = le32_to_cpu(attr->stats_size);
 	}
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
@@ -145,7 +145,7 @@ scmi_perf_domain_attributes_get(const struct scmi_handle *handle, u32 domain,
 	struct scmi_xfer *t;
 	struct scmi_msg_resp_perf_domain_attributes *attr;
 
-	ret = scmi_one_xfer_init(handle, PERF_DOMAIN_ATTRIBUTES,
+	ret = scmi_xfer_get_init(handle, PERF_DOMAIN_ATTRIBUTES,
 				 SCMI_PROTOCOL_PERF, sizeof(domain),
 				 sizeof(*attr), &t);
 	if (ret)
@@ -171,7 +171,7 @@ scmi_perf_domain_attributes_get(const struct scmi_handle *handle, u32 domain,
 		memcpy(dom_info->name, attr->name, SCMI_MAX_STR_SIZE);
 	}
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
@@ -194,7 +194,7 @@ scmi_perf_describe_levels_get(const struct scmi_handle *handle, u32 domain,
 	struct scmi_msg_perf_describe_levels *dom_info;
 	struct scmi_msg_resp_perf_describe_levels *level_info;
 
-	ret = scmi_one_xfer_init(handle, PERF_DESCRIBE_LEVELS,
+	ret = scmi_xfer_get_init(handle, PERF_DESCRIBE_LEVELS,
 				 SCMI_PROTOCOL_PERF, sizeof(*dom_info), 0, &t);
 	if (ret)
 		return ret;
@@ -237,7 +237,7 @@ scmi_perf_describe_levels_get(const struct scmi_handle *handle, u32 domain,
 	} while (num_returned && num_remaining);
 
 	perf_dom->opp_count = tot_opp_cnt;
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 
 	sort(perf_dom->opp, tot_opp_cnt, sizeof(*opp), opp_cmp_func, NULL);
 	return ret;
@@ -250,7 +250,7 @@ static int scmi_perf_limits_set(const struct scmi_handle *handle, u32 domain,
 	struct scmi_xfer *t;
 	struct scmi_perf_set_limits *limits;
 
-	ret = scmi_one_xfer_init(handle, PERF_LIMITS_SET, SCMI_PROTOCOL_PERF,
+	ret = scmi_xfer_get_init(handle, PERF_LIMITS_SET, SCMI_PROTOCOL_PERF,
 				 sizeof(*limits), 0, &t);
 	if (ret)
 		return ret;
@@ -262,7 +262,7 @@ static int scmi_perf_limits_set(const struct scmi_handle *handle, u32 domain,
 
 	ret = scmi_do_xfer(handle, t);
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
@@ -273,7 +273,7 @@ static int scmi_perf_limits_get(const struct scmi_handle *handle, u32 domain,
 	struct scmi_xfer *t;
 	struct scmi_perf_get_limits *limits;
 
-	ret = scmi_one_xfer_init(handle, PERF_LIMITS_GET, SCMI_PROTOCOL_PERF,
+	ret = scmi_xfer_get_init(handle, PERF_LIMITS_GET, SCMI_PROTOCOL_PERF,
 				 sizeof(__le32), 0, &t);
 	if (ret)
 		return ret;
@@ -288,7 +288,7 @@ static int scmi_perf_limits_get(const struct scmi_handle *handle, u32 domain,
 		*min_perf = le32_to_cpu(limits->min_level);
 	}
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
@@ -299,7 +299,7 @@ static int scmi_perf_level_set(const struct scmi_handle *handle, u32 domain,
 	struct scmi_xfer *t;
 	struct scmi_perf_set_level *lvl;
 
-	ret = scmi_one_xfer_init(handle, PERF_LEVEL_SET, SCMI_PROTOCOL_PERF,
+	ret = scmi_xfer_get_init(handle, PERF_LEVEL_SET, SCMI_PROTOCOL_PERF,
 				 sizeof(*lvl), 0, &t);
 	if (ret)
 		return ret;
@@ -311,7 +311,7 @@ static int scmi_perf_level_set(const struct scmi_handle *handle, u32 domain,
 
 	ret = scmi_do_xfer(handle, t);
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
@@ -321,7 +321,7 @@ static int scmi_perf_level_get(const struct scmi_handle *handle, u32 domain,
 	int ret;
 	struct scmi_xfer *t;
 
-	ret = scmi_one_xfer_init(handle, PERF_LEVEL_GET, SCMI_PROTOCOL_PERF,
+	ret = scmi_xfer_get_init(handle, PERF_LEVEL_GET, SCMI_PROTOCOL_PERF,
 				 sizeof(u32), sizeof(u32), &t);
 	if (ret)
 		return ret;
@@ -333,7 +333,7 @@ static int scmi_perf_level_get(const struct scmi_handle *handle, u32 domain,
 	if (!ret)
 		*level = le32_to_cpu(*(__le32 *)t->rx.buf);
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
diff --git a/drivers/firmware/arm_scmi/power.c b/drivers/firmware/arm_scmi/power.c
index 087c2876cdf2..cfa033b05aed 100644
--- a/drivers/firmware/arm_scmi/power.c
+++ b/drivers/firmware/arm_scmi/power.c
@@ -63,7 +63,7 @@ static int scmi_power_attributes_get(const struct scmi_handle *handle,
 	struct scmi_xfer *t;
 	struct scmi_msg_resp_power_attributes *attr;
 
-	ret = scmi_one_xfer_init(handle, PROTOCOL_ATTRIBUTES,
+	ret = scmi_xfer_get_init(handle, PROTOCOL_ATTRIBUTES,
 				 SCMI_PROTOCOL_POWER, 0, sizeof(*attr), &t);
 	if (ret)
 		return ret;
@@ -78,7 +78,7 @@ static int scmi_power_attributes_get(const struct scmi_handle *handle,
 		pi->stats_size = le32_to_cpu(attr->stats_size);
 	}
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
@@ -90,7 +90,7 @@ scmi_power_domain_attributes_get(const struct scmi_handle *handle, u32 domain,
 	struct scmi_xfer *t;
 	struct scmi_msg_resp_power_domain_attributes *attr;
 
-	ret = scmi_one_xfer_init(handle, POWER_DOMAIN_ATTRIBUTES,
+	ret = scmi_xfer_get_init(handle, POWER_DOMAIN_ATTRIBUTES,
 				 SCMI_PROTOCOL_POWER, sizeof(domain),
 				 sizeof(*attr), &t);
 	if (ret)
@@ -109,7 +109,7 @@ scmi_power_domain_attributes_get(const struct scmi_handle *handle, u32 domain,
 		memcpy(dom_info->name, attr->name, SCMI_MAX_STR_SIZE);
 	}
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
@@ -120,7 +120,7 @@ scmi_power_state_set(const struct scmi_handle *handle, u32 domain, u32 state)
 	struct scmi_xfer *t;
 	struct scmi_power_set_state *st;
 
-	ret = scmi_one_xfer_init(handle, POWER_STATE_SET, SCMI_PROTOCOL_POWER,
+	ret = scmi_xfer_get_init(handle, POWER_STATE_SET, SCMI_PROTOCOL_POWER,
 				 sizeof(*st), 0, &t);
 	if (ret)
 		return ret;
@@ -132,7 +132,7 @@ scmi_power_state_set(const struct scmi_handle *handle, u32 domain, u32 state)
 
 	ret = scmi_do_xfer(handle, t);
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
@@ -142,7 +142,7 @@ scmi_power_state_get(const struct scmi_handle *handle, u32 domain, u32 *state)
 	int ret;
 	struct scmi_xfer *t;
 
-	ret = scmi_one_xfer_init(handle, POWER_STATE_GET, SCMI_PROTOCOL_POWER,
+	ret = scmi_xfer_get_init(handle, POWER_STATE_GET, SCMI_PROTOCOL_POWER,
 				 sizeof(u32), sizeof(u32), &t);
 	if (ret)
 		return ret;
@@ -153,7 +153,7 @@ scmi_power_state_get(const struct scmi_handle *handle, u32 domain, u32 *state)
 	if (!ret)
 		*state = le32_to_cpu(*(__le32 *)t->rx.buf);
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
index bbb469fea0ed..27f2092b9882 100644
--- a/drivers/firmware/arm_scmi/sensors.c
+++ b/drivers/firmware/arm_scmi/sensors.c
@@ -79,7 +79,7 @@ static int scmi_sensor_attributes_get(const struct scmi_handle *handle,
 	struct scmi_xfer *t;
 	struct scmi_msg_resp_sensor_attributes *attr;
 
-	ret = scmi_one_xfer_init(handle, PROTOCOL_ATTRIBUTES,
+	ret = scmi_xfer_get_init(handle, PROTOCOL_ATTRIBUTES,
 				 SCMI_PROTOCOL_SENSOR, 0, sizeof(*attr), &t);
 	if (ret)
 		return ret;
@@ -95,7 +95,7 @@ static int scmi_sensor_attributes_get(const struct scmi_handle *handle,
 		si->reg_size = le32_to_cpu(attr->reg_size);
 	}
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
@@ -108,7 +108,7 @@ static int scmi_sensor_description_get(const struct scmi_handle *handle,
 	struct scmi_xfer *t;
 	struct scmi_msg_resp_sensor_description *buf;
 
-	ret = scmi_one_xfer_init(handle, SENSOR_DESCRIPTION_GET,
+	ret = scmi_xfer_get_init(handle, SENSOR_DESCRIPTION_GET,
 				 SCMI_PROTOCOL_SENSOR, sizeof(__le32), 0, &t);
 	if (ret)
 		return ret;
@@ -150,7 +150,7 @@ static int scmi_sensor_description_get(const struct scmi_handle *handle,
 		 */
 	} while (num_returned && num_remaining);
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
@@ -162,7 +162,7 @@ scmi_sensor_configuration_set(const struct scmi_handle *handle, u32 sensor_id)
 	struct scmi_xfer *t;
 	struct scmi_msg_set_sensor_config *cfg;
 
-	ret = scmi_one_xfer_init(handle, SENSOR_CONFIG_SET,
+	ret = scmi_xfer_get_init(handle, SENSOR_CONFIG_SET,
 				 SCMI_PROTOCOL_SENSOR, sizeof(*cfg), 0, &t);
 	if (ret)
 		return ret;
@@ -173,7 +173,7 @@ scmi_sensor_configuration_set(const struct scmi_handle *handle, u32 sensor_id)
 
 	ret = scmi_do_xfer(handle, t);
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
@@ -185,7 +185,7 @@ static int scmi_sensor_trip_point_set(const struct scmi_handle *handle,
 	struct scmi_xfer *t;
 	struct scmi_msg_set_sensor_trip_point *trip;
 
-	ret = scmi_one_xfer_init(handle, SENSOR_TRIP_POINT_SET,
+	ret = scmi_xfer_get_init(handle, SENSOR_TRIP_POINT_SET,
 				 SCMI_PROTOCOL_SENSOR, sizeof(*trip), 0, &t);
 	if (ret)
 		return ret;
@@ -198,7 +198,7 @@ static int scmi_sensor_trip_point_set(const struct scmi_handle *handle,
 
 	ret = scmi_do_xfer(handle, t);
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
@@ -209,7 +209,7 @@ static int scmi_sensor_reading_get(const struct scmi_handle *handle,
 	struct scmi_xfer *t;
 	struct scmi_msg_sensor_reading_get *sensor;
 
-	ret = scmi_one_xfer_init(handle, SENSOR_READING_GET,
+	ret = scmi_xfer_get_init(handle, SENSOR_READING_GET,
 				 SCMI_PROTOCOL_SENSOR, sizeof(*sensor),
 				 sizeof(u64), &t);
 	if (ret)
@@ -227,7 +227,7 @@ static int scmi_sensor_reading_get(const struct scmi_handle *handle,
 		*value |= (u64)le32_to_cpu(*(pval + 1)) << 32;
 	}
 
-	scmi_one_xfer_put(handle, t);
+	scmi_xfer_put(handle, t);
 	return ret;
 }
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH 5/8] firmware: arm_scmi: drop unused `con_priv` structure member
From: Sudeep Holla @ 2018-05-09 17:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525885634-22348-1-git-send-email-sudeep.holla@arm.com>

Initially con_priv was supposedly used for transport specific data when
the SCMI driver had an abstraction to communicate with different mailbox
controllers. But after some discussions, the idea was dropped but this
variable slipped through the cracks.

This patch gets rid of this unused variable.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scmi/common.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 41b03e46cca8..937a930ce87d 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -86,9 +86,7 @@ struct scmi_msg {
  *	buffer for the rx path as we use for the tx path.
  * @done: completion event
  */
-
 struct scmi_xfer {
-	void *con_priv;
 	struct scmi_msg_hdr hdr;
 	struct scmi_msg tx;
 	struct scmi_msg rx;
-- 
2.7.4

^ permalink raw reply related


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