All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 4/5] powerpc/pseries: Query hypervisor for count cache flush settings
From: Michael Ellerman @ 2018-07-23 15:07 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20180723150756.11108-1-mpe@ellerman.id.au>

Use the existing hypercall to determine the appropriate settings for
the count cache flush, and then call the generic powerpc code to set
it up based on the security feature flags.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/hvcall.h      | 2 ++
 arch/powerpc/platforms/pseries/setup.c | 7 +++++++
 2 files changed, 9 insertions(+)

diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index 662c8347d699..a0b17f9f1ea4 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -342,10 +342,12 @@
 #define H_CPU_CHAR_BRANCH_HINTS_HONORED	(1ull << 58) // IBM bit 5
 #define H_CPU_CHAR_THREAD_RECONFIG_CTRL	(1ull << 57) // IBM bit 6
 #define H_CPU_CHAR_COUNT_CACHE_DISABLED	(1ull << 56) // IBM bit 7
+#define H_CPU_CHAR_BCCTR_FLUSH_ASSIST	(1ull << 54) // IBM bit 9
 
 #define H_CPU_BEHAV_FAVOUR_SECURITY	(1ull << 63) // IBM bit 0
 #define H_CPU_BEHAV_L1D_FLUSH_PR	(1ull << 62) // IBM bit 1
 #define H_CPU_BEHAV_BNDS_CHK_SPEC_BAR	(1ull << 61) // IBM bit 2
+#define H_CPU_BEHAV_FLUSH_COUNT_CACHE	(1ull << 58) // IBM bit 5
 
 /* Flag values used in H_REGISTER_PROC_TBL hcall */
 #define PROC_TABLE_OP_MASK	0x18
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 139f0af6c3d9..04805a79cbda 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -484,6 +484,12 @@ static void init_cpu_char_feature_flags(struct h_cpu_char_result *result)
 	if (result->character & H_CPU_CHAR_COUNT_CACHE_DISABLED)
 		security_ftr_set(SEC_FTR_COUNT_CACHE_DISABLED);
 
+	if (result->character & H_CPU_CHAR_BCCTR_FLUSH_ASSIST)
+		security_ftr_set(SEC_FTR_BCCTR_FLUSH_ASSIST);
+
+	if (result->behaviour & H_CPU_BEHAV_FLUSH_COUNT_CACHE)
+		security_ftr_set(SEC_FTR_FLUSH_COUNT_CACHE);
+
 	/*
 	 * The features below are enabled by default, so we instead look to see
 	 * if firmware has *disabled* them, and clear them if so.
@@ -535,6 +541,7 @@ void pseries_setup_rfi_flush(void)
 
 	setup_rfi_flush(types, enable);
 	setup_barrier_nospec();
+	setup_count_cache_flush();
 }
 
 #ifdef CONFIG_PCI_IOV
-- 
2.14.1

^ permalink raw reply related

* [PATCH 3/5] powerpc/64s: Add support for software count cache flush
From: Michael Ellerman @ 2018-07-23 15:07 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20180723150756.11108-1-mpe@ellerman.id.au>

Some CPU revisions support a mode where the count cache needs to be
flushed by software on context switch. Additionally some revisions may
have a hardware accelerated flush, in which case the software flush
sequence can be shortened.

If we detect the appropriate flag from firmware we patch a branch
into _switch() which takes us to a count cache flush sequence.

That sequence in turn may be patched to return early if we detect that
the CPU supports accelerating the flush sequence in hardware.

Add debugfs support for reporting the state of the flush, as well as
runtime disabling it.

And modify the spectre_v2 sysfs file to report the state of the
software flush.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/asm-prototypes.h    |  6 ++
 arch/powerpc/include/asm/security_features.h |  1 +
 arch/powerpc/kernel/entry_64.S               | 54 ++++++++++++++++
 arch/powerpc/kernel/security.c               | 96 ++++++++++++++++++++++++++--
 4 files changed, 152 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
index 769567b66c0c..70fdc5b9b9fb 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -143,4 +143,10 @@ struct kvm_vcpu;
 void _kvmppc_restore_tm_pr(struct kvm_vcpu *vcpu, u64 guest_msr);
 void _kvmppc_save_tm_pr(struct kvm_vcpu *vcpu, u64 guest_msr);
 
+/* Patch sites */
+extern s32 patch__call_flush_count_cache;
+extern s32 patch__flush_count_cache_return;
+
+extern long flush_count_cache;
+
 #endif /* _ASM_POWERPC_ASM_PROTOTYPES_H */
diff --git a/arch/powerpc/include/asm/security_features.h b/arch/powerpc/include/asm/security_features.h
index a0d47bc18a5c..759597bf0fd8 100644
--- a/arch/powerpc/include/asm/security_features.h
+++ b/arch/powerpc/include/asm/security_features.h
@@ -22,6 +22,7 @@ enum stf_barrier_type {
 
 void setup_stf_barrier(void);
 void do_stf_barrier_fixups(enum stf_barrier_type types);
+void setup_count_cache_flush(void);
 
 static inline void security_ftr_set(unsigned long feature)
 {
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 0357f87a013c..017cf70f01d7 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -25,6 +25,7 @@
 #include <asm/page.h>
 #include <asm/mmu.h>
 #include <asm/thread_info.h>
+#include <asm/code-patching-asm.h>
 #include <asm/ppc_asm.h>
 #include <asm/asm-offsets.h>
 #include <asm/cputable.h>
@@ -504,6 +505,57 @@ _GLOBAL(ret_from_kernel_thread)
 	li	r3,0
 	b	.Lsyscall_exit
 
+#ifdef CONFIG_PPC_BOOK3S_64
+
+#define FLUSH_COUNT_CACHE	\
+1:	nop;			\
+	patch_site 1b, patch__call_flush_count_cache
+
+
+#define BCCTR_FLUSH	.long 0x4c400420
+
+.macro nops number
+	.rept \number
+	nop
+	.endr
+.endm
+
+.balign 32
+.global flush_count_cache
+flush_count_cache:
+	/* Save LR into r9 */
+	mflr	r9
+
+	.rept 64
+	bl	.+4
+	.endr
+	b	1f
+	nops	6
+
+	.balign 32
+	/* Restore LR */
+1:	mtlr	r9
+	li	r9,0x7fff
+	mtctr	r9
+
+	BCCTR_FLUSH
+
+2:	nop
+	patch_site 2b patch__flush_count_cache_return
+
+	nops	3
+
+	.rept 278
+	.balign 32
+	BCCTR_FLUSH
+	nops	7
+	.endr
+
+	blr
+#else
+#define FLUSH_COUNT_CACHE
+#endif /* CONFIG_PPC_BOOK3S_64 */
+
 /*
  * This routine switches between two different tasks.  The process
  * state of one is saved on its kernel stack.  Then the state
@@ -535,6 +587,8 @@ _GLOBAL(_switch)
 	std	r23,_CCR(r1)
 	std	r1,KSP(r3)	/* Set old stack pointer */
 
+	FLUSH_COUNT_CACHE
+
 	/*
 	 * On SMP kernels, care must be taken because a task may be
 	 * scheduled off CPUx and on to CPUy. Memory ordering must be
diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index 4cb8f1f7b593..fa9366b53eb7 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -8,6 +8,8 @@
 #include <linux/device.h>
 #include <linux/seq_buf.h>
 
+#include <asm/asm-prototypes.h>
+#include <asm/code-patching.h>
 #include <asm/debugfs.h>
 #include <asm/security_features.h>
 #include <asm/setup.h>
@@ -15,6 +17,13 @@
 
 unsigned long powerpc_security_features __read_mostly = SEC_FTR_DEFAULT;
 
+enum count_cache_flush_type {
+	COUNT_CACHE_FLUSH_NONE	= 0x1,
+	COUNT_CACHE_FLUSH_SW	= 0x2,
+	COUNT_CACHE_FLUSH_HW	= 0x4,
+};
+static enum count_cache_flush_type count_cache_flush_type;
+
 bool barrier_nospec_enabled;
 
 static void enable_barrier_nospec(bool enable)
@@ -147,17 +156,29 @@ ssize_t cpu_show_spectre_v2(struct device *dev, struct device_attribute *attr, c
 	bcs = security_ftr_enabled(SEC_FTR_BCCTRL_SERIALISED);
 	ccd = security_ftr_enabled(SEC_FTR_COUNT_CACHE_DISABLED);
 
-	if (bcs || ccd) {
+	if (bcs || ccd || count_cache_flush_type != COUNT_CACHE_FLUSH_NONE) {
+		bool comma = false;
 		seq_buf_printf(&s, "Mitigation: ");
 
-		if (bcs)
+		if (bcs) {
 			seq_buf_printf(&s, "Indirect branch serialisation (kernel only)");
+			comma = true;
+		}
+
+		if (ccd) {
+			if (comma)
+				seq_buf_printf(&s, ", ");
+			seq_buf_printf(&s, "Indirect branch cache disabled");
+			comma = true;
+		}
 
-		if (bcs && ccd)
+		if (comma)
 			seq_buf_printf(&s, ", ");
 
-		if (ccd)
-			seq_buf_printf(&s, "Indirect branch cache disabled");
+		seq_buf_printf(&s, "Software count cache flush");
+
+		if (count_cache_flush_type == COUNT_CACHE_FLUSH_HW)
+			seq_buf_printf(&s, "(hardware accelerated)");
 	} else
 		seq_buf_printf(&s, "Vulnerable");
 
@@ -313,3 +334,68 @@ static __init int stf_barrier_debugfs_init(void)
 }
 device_initcall(stf_barrier_debugfs_init);
 #endif /* CONFIG_DEBUG_FS */
+
+static void toggle_count_cache_flush(bool enable)
+{
+	if (!enable || !security_ftr_enabled(SEC_FTR_FLUSH_COUNT_CACHE)) {
+		patch_instruction_site(&patch__call_flush_count_cache, PPC_INST_NOP);
+		count_cache_flush_type = COUNT_CACHE_FLUSH_NONE;
+		pr_info("count-cache-flush: software flush disabled.\n");
+		return;
+	}
+
+	patch_branch_site(&patch__call_flush_count_cache,
+			  (u64)&flush_count_cache, BRANCH_SET_LINK);
+
+	if (!security_ftr_enabled(SEC_FTR_BCCTR_FLUSH_ASSIST)) {
+		count_cache_flush_type = COUNT_CACHE_FLUSH_SW;
+		pr_info("count-cache-flush: full software flush sequence enabled.\n");
+		return;
+	}
+
+	patch_instruction_site(&patch__flush_count_cache_return, PPC_INST_BLR);
+	count_cache_flush_type = COUNT_CACHE_FLUSH_HW;
+	pr_info("count-cache-flush: hardware assisted flush sequence enabled\n");
+}
+
+void setup_count_cache_flush(void)
+{
+	toggle_count_cache_flush(true);
+}
+
+#ifdef CONFIG_DEBUG_FS
+static int count_cache_flush_set(void *data, u64 val)
+{
+	bool enable;
+
+	if (val == 1)
+		enable = true;
+	else if (val == 0)
+		enable = false;
+	else
+		return -EINVAL;
+
+	toggle_count_cache_flush(enable);
+
+	return 0;
+}
+
+static int count_cache_flush_get(void *data, u64 *val)
+{
+	if (count_cache_flush_type == COUNT_CACHE_FLUSH_NONE)
+		*val = 0;
+	else
+		*val = 1;
+
+	return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(fops_count_cache_flush, count_cache_flush_get, count_cache_flush_set, "%llu\n");
+
+static __init int count_cache_flush_debugfs_init(void)
+{
+	debugfs_create_file("count_cache_flush", 0600, powerpc_debugfs_root, NULL, &fops_count_cache_flush);
+	return 0;
+}
+device_initcall(count_cache_flush_debugfs_init);
+#endif /* CONFIG_DEBUG_FS */
-- 
2.14.1

^ permalink raw reply related

* [PATCH 2/5] powerpc/64s: Add new security feature flags for count cache flush
From: Michael Ellerman @ 2018-07-23 15:07 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20180723150756.11108-1-mpe@ellerman.id.au>

Add security feature flags to indicate the need for software to flush
the count cache on context switch, and for the presence of a hardware
assisted count cache flush.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/security_features.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/powerpc/include/asm/security_features.h b/arch/powerpc/include/asm/security_features.h
index 44989b22383c..a0d47bc18a5c 100644
--- a/arch/powerpc/include/asm/security_features.h
+++ b/arch/powerpc/include/asm/security_features.h
@@ -59,6 +59,9 @@ static inline bool security_ftr_enabled(unsigned long feature)
 // Indirect branch prediction cache disabled
 #define SEC_FTR_COUNT_CACHE_DISABLED	0x0000000000000020ull
 
+// bcctr 2,0,0 triggers a hardware assisted count cache flush
+#define SEC_FTR_BCCTR_FLUSH_ASSIST	0x0000000000000800ull
+
 
 // Features indicating need for Spectre/Meltdown mitigations
 
@@ -74,6 +77,9 @@ static inline bool security_ftr_enabled(unsigned long feature)
 // Firmware configuration indicates user favours security over performance
 #define SEC_FTR_FAVOUR_SECURITY		0x0000000000000200ull
 
+// Software required to flush count cache on context switch
+#define SEC_FTR_FLUSH_COUNT_CACHE	0x0000000000000400ull
+
 
 // Features enabled by default
 #define SEC_FTR_DEFAULT \
-- 
2.14.1

^ permalink raw reply related

* [PATCH 1/5] powerpc/asm: Add a patch_site macro & helpers for patching instructions
From: Michael Ellerman @ 2018-07-23 15:07 UTC (permalink / raw)
  To: linuxppc-dev

Add a macro and some helper C functions for patching single asm
instructions.

The gas macro means we can do something like:

  1:	nop
  	patch_site 1b, patch__foo

Which is less visually distracting than defining a GLOBAL symbol at 1,
and also doesn't pollute the symbol table which can confuse eg. perf.

These are obviously similar to our existing feature sections, but are
not automatically patched based on CPU/MMU features, rather they are
designed to be manually patched by C code at some arbitrary point.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/code-patching-asm.h | 18 ++++++++++++++++++
 arch/powerpc/include/asm/code-patching.h     |  2 ++
 arch/powerpc/lib/code-patching.c             | 16 ++++++++++++++++
 3 files changed, 36 insertions(+)
 create mode 100644 arch/powerpc/include/asm/code-patching-asm.h

diff --git a/arch/powerpc/include/asm/code-patching-asm.h b/arch/powerpc/include/asm/code-patching-asm.h
new file mode 100644
index 000000000000..ed7b1448493a
--- /dev/null
+++ b/arch/powerpc/include/asm/code-patching-asm.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2018, Michael Ellerman, IBM Corporation.
+ */
+#ifndef _ASM_POWERPC_CODE_PATCHING_ASM_H
+#define _ASM_POWERPC_CODE_PATCHING_ASM_H
+
+/* Define a "site" that can be patched */
+.macro patch_site label name
+	.pushsection ".rodata"
+	.balign 4
+	.global \name
+\name:
+	.4byte	\label - .
+	.popsection
+.endm
+
+#endif /* _ASM_POWERPC_CODE_PATCHING_ASM_H */
diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
index 812535f40124..b2051234ada8 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -32,6 +32,8 @@ unsigned int create_cond_branch(const unsigned int *addr,
 int patch_branch(unsigned int *addr, unsigned long target, int flags);
 int patch_instruction(unsigned int *addr, unsigned int instr);
 int raw_patch_instruction(unsigned int *addr, unsigned int instr);
+int patch_instruction_site(s32 *addr, unsigned int instr);
+int patch_branch_site(s32 *site, unsigned long target, int flags);
 
 int instr_is_relative_branch(unsigned int instr);
 int instr_is_relative_link_branch(unsigned int instr);
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index e0d881ab304e..850f3b8f4da5 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -195,6 +195,22 @@ int patch_branch(unsigned int *addr, unsigned long target, int flags)
 	return patch_instruction(addr, create_branch(addr, target, flags));
 }
 
+int patch_branch_site(s32 *site, unsigned long target, int flags)
+{
+	unsigned int *addr;
+
+	addr = (unsigned int *)((unsigned long)site + *site);
+	return patch_instruction(addr, create_branch(addr, target, flags));
+}
+
+int patch_instruction_site(s32 *site, unsigned int instr)
+{
+	unsigned int *addr;
+
+	addr = (unsigned int *)((unsigned long)site + *site);
+	return patch_instruction(addr, instr);
+}
+
 bool is_offset_in_branch_range(long offset)
 {
 	/*
-- 
2.14.1

^ permalink raw reply related

* [RFC PATCH v6 2/2] mailbox: imx-mailbox: add scu protocol support
From: Jassi Brar @ 2018-07-23 15:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180712070129.tyetqm2yesouiatf@pengutronix.de>

On Thu, Jul 12, 2018 at 12:31 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Thu, Jul 12, 2018 at 12:32:39AM +0800, Dong Aisheng wrote:
>> Add SCU protocol support in the generic mailbox driver.
>>
>> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
>> ---
>>  drivers/mailbox/Kconfig       |   6 +-
>>  drivers/mailbox/imx-mailbox.c | 252 ++++++++++++++++++++++++++++++++++++------
>>  2 files changed, 223 insertions(+), 35 deletions(-)
>>
>> +static bool imx_mu_scu_peek_data(struct mbox_chan *chan)
>> +{
>> +     struct imx_mu_priv *priv = chan->con_priv;
>> +     u8 *raw_data;
>> +     int i, size;
>> +     int ret;
>> +
>> +     ret = imx_mu_scu_receive_msg(chan, 0, priv->msg);
>> +     if (ret)
>> +             return false;
>> +
>> +     raw_data = (u8 *)priv->msg;
>> +     size = raw_data[1];
>> +
>> +     dev_dbg(chan->mbox->dev, "receive data: hdr 0x%x size %d\n",
>> +             *priv->msg, size);
>> +
>> +     for (i = 1; i < size; i++) {
>> +             ret = imx_mu_scu_receive_msg(chan, i % 4, priv->msg + i);
>> +             if (ret)
>> +                     return false;
>> +     }
>> +
>> +     mbox_chan_received_data(chan, (void *)priv->msg);
>> +
>> +     return true;
>> +}
>
> Receiving data now works by calling mbox_client_peek_data(). I think
> your plan is not to use rx_callback as triggered by mbox_chan_received_data,
> but instead exploiting the fact that you have put the receive data into
> the same buffer that you previously used to send a message. That way the
> client knows where to find the answer.
>
> While that should work I don't think this is very clean. That is the
> point where I mentioned I think that the mailbox framework would need a
> way to receive messages synchronously.
>
> Jassi, this is for you probably. Do you have an idea how synchronous
> receive could be implemented? We'll need some mbox_client_receive_data
> function.
>
Most platforms call mbox_chan_received_data() from RX interrupt
handler. Which is actually


> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCH v2 1/2] powerpc/pseries: Avoid blocking rtas polling handling multiple PRRN events
From: John Allen @ 2018-07-23 15:05 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, nfont
In-Reply-To: <87muuihyjn.fsf@concordia.ellerman.id.au>

On Mon, Jul 23, 2018 at 11:27:56PM +1000, Michael Ellerman wrote:
>Hi John,
>
>I'm a bit puzzled by this one.
>
>John Allen <jallen@linux.ibm.com> writes:
>> When a PRRN event is being handled and another PRRN event comes in, the
>> second event will block rtas polling waiting on the first to complete,
>> preventing any further rtas events from being handled. This can be
>> especially problematic in case that PRRN events are continuously being
>> queued in which case rtas polling gets indefinitely blocked completely.
>>
>> This patch introduces a mutex that prevents any subsequent PRRN events from
>> running while there is a prrn event being handled, allowing rtas polling to
>> continue normally.
>>
>> Signed-off-by: John Allen <jallen@linux.ibm.com>
>> ---
>> v2:
>>   -Unlock prrn_lock when PRRN operations are complete, not after handler is
>>    scheduled.
>>   -Remove call to flush_work, the previous broken method of serializing
>>    PRRN events.
>> ---
>>  arch/powerpc/kernel/rtasd.c | 10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/rtasd.c b/arch/powerpc/kernel/rtasd.c
>> index 44d66c33d59d..845fc5aec178 100644
>> --- a/arch/powerpc/kernel/rtasd.c
>> +++ b/arch/powerpc/kernel/rtasd.c
>> @@ -284,15 +286,17 @@ static void prrn_work_fn(struct work_struct *work)
>>  	 */
>>  	pseries_devicetree_update(-prrn_update_scope);
>>  	numa_update_cpu_topology(false);
>> +	mutex_unlock(&prrn_lock);
>>  }
>>
>>  static DECLARE_WORK(prrn_work, prrn_work_fn);
>>
>>  static void prrn_schedule_update(u32 scope)
>>  {
>> -	flush_work(&prrn_work);
>
>This seems like it's actually the core of the change. Previously we were
>basically blocking on the flush before continuing.

The idea here is to replace the blocking flush_work with a non-blocking 
mutex. So rather than waiting on the running PRRN event to complete, we 
bail out since a PRRN event is already running. The situation this is 
meant to address is flooding the workqueue with PRRN events, which like 
the situation in patch 2/2, these can be queued up faster than they can 
actually be handled.

>
>> -	prrn_update_scope = scope;
>
>I don't really understand the scope. With the old code we always ran the
>work function once for call, now we potentially throw away the scope
>value (if the try lock fails).

So anytime we actually want to run with the scope (in the event the 
trylock succeeds), we schedule the work with the scope value set 
accordingly as seen in the code below. In the case that we actually 
don't want to run a PRRN event (if one is already running) we do throw 
away the scope and ignore the request entirely.

>
>> -	schedule_work(&prrn_work);
>> +	if (mutex_trylock(&prrn_lock)) {
>> +		prrn_update_scope = scope;
>> +		schedule_work(&prrn_work);
>> +	}
>
>Ignoring the scope, the addition of the mutex should not actually make
>any difference. If you see the doco for schedule_work() it says:
>
> * This puts a job in the kernel-global workqueue if it was not already
> * queued and leaves it in the same position on the kernel-global
> * workqueue otherwise.
>
>
>So the mutex basically implements that existing behaviour. But maybe the
>scope is the issue? Like I said I don't really understand the scope
>value.
>
>
>So I guess I'm wondering if we just need to drop the flush_work() and
>the rest is not required?

To sum up the above, the behavior without the mutex is not the same as 
with the mutex. Without the mutex, that means that anytime we get a PRRN 
event, it will get queued on the workqueue which can get flooded if PRRN 
events are queued continuously. With the mutex, only one PRRN event can 
be queued for handling at once.

Hope that clears things up!

-John

>
>cheers
>

^ permalink raw reply

* [PATCH net-next] net: phy: prevent PHYs w/o Clause 22 regs from calling genphy_config_aneg
From: Camelia Groza @ 2018-07-23 15:06 UTC (permalink / raw)
  To: andrew, f.fainelli, rmk+kernel, davem; +Cc: netdev, linux-kernel, Camelia Groza

genphy_config_aneg() should be called only by PHYs that implement
the Clause 22 register set. Prevent Clause 45 PHYs that don't implement
the register set from calling the genphy function.

Signed-off-by: Camelia Groza <camelia.groza@nxp.com>
---
 drivers/net/phy/phy.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index efa0a3c..6049652 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -471,8 +471,14 @@ static int phy_config_aneg(struct phy_device *phydev)
 {
 	if (phydev->drv->config_aneg)
 		return phydev->drv->config_aneg(phydev);
-	else
-		return genphy_config_aneg(phydev);
+
+	/* Clause 45 PHYs that don't implement Clause 22 registers are not
+	 * allowed to call genphy_config_aneg()
+	 */
+	if (phydev->is_c45 && !(phydev->c45_ids.devices_in_package & BIT(0)))
+		return -EOPNOTSUPP;
+
+	return genphy_config_aneg(phydev);
 }
 
 /**
-- 
1.9.1


^ permalink raw reply related

* Re: How to tell bitbake my prefered terminal in for devshell and access bitbake datastore?
From: Rudolf J Streif @ 2018-07-23 15:05 UTC (permalink / raw)
  To: MOHAMMAD RASIM, yocto
In-Reply-To: <1e741f3f-f20e-4e38-d9ec-643e22b34e2d@gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 1136 bytes --]

You can use the variable OE_TERMINAL to choose your preferred terminal.
By default it's set to auto and Bitbake figures out what terminal is
available on your system. Please refer to the manual at:
https://www.yoctoproject.org/docs/2.5/mega-manual/mega-manual.html#var-OE_TERMINAL.

> Also, is it possible to access bitbake datastore from inside the
devshell(or devpyshell) ?

You cannot access d if you mean that, however, all the variables are
exported to the shell.

:rjs


On 07/23/2018 02:49 AM, MOHAMMAD RASIM wrote:
> Hi,
>
> when running bitbake with a recipe and giving the devshell as a
> command bitbake runs the devshell inside xterm, on my other machine it
> runs tmux.
>
> looking at the openembedded source code here
> https://github.com/openembedded/openembedded-core/blob/master/meta/lib/oe/terminal.py
>
> I can see there are multiple classes for terminals that bitbake loop
> through, but how can I tell bitbake that I prefer a certain terminal?
>
> Also, is it possible to access bitbake datastore from inside the
> devshell(or devpyshell) ?
>
> Regards
>

-- 
Rudolf J Streif



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 499 bytes --]

^ permalink raw reply

* Re: [PATCH 6/6] x86/shadow: a little bit of style cleanup
From: Tim Deegan @ 2018-07-23 15:05 UTC (permalink / raw)
  To: Jan Beulich; +Cc: George Dunlap, xen-devel, Andrew Cooper
In-Reply-To: <5B506D1702000078001D5D8D@prv1-mh.provo.novell.com>

At 04:51 -0600 on 19 Jul (1531975863), Jan Beulich wrote:
> Correct indentation of a piece of code, adjusting comment style at the
> same time. Constify gl3e pointers and drop a bogus (and useless once
> corrected) cast.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Tim Deegan <tim@xen.org>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply

* Re: [stable-4.14 00/23] block/scsi multiqueue performance enhancement and
From: Jens Axboe @ 2018-07-23 15:05 UTC (permalink / raw)
  To: Jack Wang, Greg Kroah-Hartman, linux-scsi, linux-block
  Cc: Wang Jinpu, stable, Jinpu Wang
In-Reply-To: <CA+res+TE5-mgnQys67zQ8cqRyv+dgNPpTFLOEq81t2izpq1dsw@mail.gmail.com>

On 7/23/18 9:00 AM, Jack Wang wrote:
> Hi Greg,
> 
> Thanks for quick reply. Please see reply inline.
> 
> 
> 
> Greg KH <gregkh@linuxfoundation.org> 于2018年7月23日周一 下午3:34写道:
>>
>> On Mon, Jul 23, 2018 at 03:24:22PM +0200, Jack Wang wrote:
>>> Hi Greg,
>>>
>>> Please consider this patchset, which include block/scsi multiqueue performance
>>> enhancement and bugfix.
>>
>> What exactly is the performance enhancement?  How can you measure it?
>> How did you measure it?
> I'm testing on SRP/IBNBD using fio, I've seen +10% with mix IO load,
> and 50% improvement on small IO (bs=512.) with the patchset.

Big nak on backporting this huge series, it's a lot of core changes.
It's way beyond the scope of a stable fix backport.

-- 
Jens Axboe

^ permalink raw reply

* Re: [PATCH 2/2] xen/pv: Call get_cpu_address_sizes to set x86_virt/phys_bits
From: Boris Ostrovsky @ 2018-07-23 15:04 UTC (permalink / raw)
  To: M. Vefa Bicakci, linux-kernel
  Cc: Kirill A. Shutemov, Andy Lutomirski, Ingo Molnar, H. Peter Anvin,
	Thomas Gleixner, Juergen Gross, xen-devel, x86, stable
In-Reply-To: <13b774d7-bc8c-039a-fbc0-52318f535883@runbox.com>

On 07/22/2018 11:57 AM, M. Vefa Bicakci wrote:
> On 07/21/2018 07:17 PM, M. Vefa Bicakci wrote:
>> On 07/21/2018 05:25 PM, Boris Ostrovsky wrote:
>>> On 07/21/2018 03:49 PM, M. Vefa Bicakci wrote:
>>>> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
>>>> index 439a94bf89ad..87afb000142a 100644
>>>> --- a/arch/x86/xen/enlighten_pv.c
>>>> +++ b/arch/x86/xen/enlighten_pv.c
>>>> @@ -1257,6 +1257,7 @@ asmlinkage __visible void __init
>>>> xen_start_kernel(void)
>>>>       /* Work out if we support NX */
>>>>       get_cpu_cap(&boot_cpu_data);
>>>> +    get_cpu_address_sizes(&boot_cpu_data);
>>>>       x86_configure_nx();
>>>
>>>
>>> Have you observed any problems without this call? get_cpu_cap() is only
>>> called here to set X86_FEATURE_NX, and is then called again, together
>>> with get_cpu_address_sizes(), from early_identify_cpu().
>>
>> Thank you for the reviews! Without the call to get_cpu_address_sizes,
>> paravirtualized virtual machines do not boot up kernels with versions
>> 4.17 and up at all; this includes dom0 and domU. No domU logs are
>> generated in dom0's /var/log/xen/console/ directory either, despite
>> having earlyprintk=xen on the kernel command line for my test domU.
>
> Hello Boris,
>
> I debugged this further with a debugging version of Xen (so that I can
> get early kernel print-outs via the "xen_raw_console_write" function),
> and I found the root cause of the boot up failure.
>
> In summary, the issue is due to the following call path in version
> 4.17 (and higher, I assume), which the kernel goes through /only/ when
> CONFIG_DEBUG_VIRTUAL is enabled:
>
> enlighten_pv.c::xen_start_kernel
>   mmu_pv.c::xen_reserve_special_pages
>     page.h::__pa
>       physaddr.c::__phys_addr
>         physaddr.h::phys_addr_valid // uses boot_cpu_data.x86_phys_bits
>
> The return value of phys_addr_valid is used with the VIRTUAL_BUG_ON
> macro,
> which evaluates to BUG_ON in case CONFIG_DEBUG_VIRTUAL is enabled.


Ah, that's why it hasn't been detected.


>
> It looks like the call to get_cpu_address_size is required in the
> xen_start_kernel function. Perhaps there is a more elegant way to
> resolve this issue as well.
>
> Another approach could be to check in the phys_addr_valid function
> whether
> boot_cpu_data.x86_phys_bits has been initialized or not, I think, but
> I am
> not sure about the correctness of this approach.


No, I think your patch is good. The only thing I'd suggest is to move
the call a few lines down. The way it is placed now may create
impression that we are calling get_cpu_address_sizes() to figure out NX
support.


-boris

^ permalink raw reply

* Re: [PATCH 2/2] xen/pv: Call get_cpu_address_sizes to set x86_virt/phys_bits
From: Boris Ostrovsky @ 2018-07-23 15:04 UTC (permalink / raw)
  To: M. Vefa Bicakci, linux-kernel
  Cc: Juergen Gross, x86, stable, Ingo Molnar, Andy Lutomirski,
	H. Peter Anvin, xen-devel, Thomas Gleixner, Kirill A. Shutemov
In-Reply-To: <13b774d7-bc8c-039a-fbc0-52318f535883@runbox.com>

On 07/22/2018 11:57 AM, M. Vefa Bicakci wrote:
> On 07/21/2018 07:17 PM, M. Vefa Bicakci wrote:
>> On 07/21/2018 05:25 PM, Boris Ostrovsky wrote:
>>> On 07/21/2018 03:49 PM, M. Vefa Bicakci wrote:
>>>> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
>>>> index 439a94bf89ad..87afb000142a 100644
>>>> --- a/arch/x86/xen/enlighten_pv.c
>>>> +++ b/arch/x86/xen/enlighten_pv.c
>>>> @@ -1257,6 +1257,7 @@ asmlinkage __visible void __init
>>>> xen_start_kernel(void)
>>>>       /* Work out if we support NX */
>>>>       get_cpu_cap(&boot_cpu_data);
>>>> +    get_cpu_address_sizes(&boot_cpu_data);
>>>>       x86_configure_nx();
>>>
>>>
>>> Have you observed any problems without this call? get_cpu_cap() is only
>>> called here to set X86_FEATURE_NX, and is then called again, together
>>> with get_cpu_address_sizes(), from early_identify_cpu().
>>
>> Thank you for the reviews! Without the call to get_cpu_address_sizes,
>> paravirtualized virtual machines do not boot up kernels with versions
>> 4.17 and up at all; this includes dom0 and domU. No domU logs are
>> generated in dom0's /var/log/xen/console/ directory either, despite
>> having earlyprintk=xen on the kernel command line for my test domU.
>
> Hello Boris,
>
> I debugged this further with a debugging version of Xen (so that I can
> get early kernel print-outs via the "xen_raw_console_write" function),
> and I found the root cause of the boot up failure.
>
> In summary, the issue is due to the following call path in version
> 4.17 (and higher, I assume), which the kernel goes through /only/ when
> CONFIG_DEBUG_VIRTUAL is enabled:
>
> enlighten_pv.c::xen_start_kernel
>   mmu_pv.c::xen_reserve_special_pages
>     page.h::__pa
>       physaddr.c::__phys_addr
>         physaddr.h::phys_addr_valid // uses boot_cpu_data.x86_phys_bits
>
> The return value of phys_addr_valid is used with the VIRTUAL_BUG_ON
> macro,
> which evaluates to BUG_ON in case CONFIG_DEBUG_VIRTUAL is enabled.


Ah, that's why it hasn't been detected.


>
> It looks like the call to get_cpu_address_size is required in the
> xen_start_kernel function. Perhaps there is a more elegant way to
> resolve this issue as well.
>
> Another approach could be to check in the phys_addr_valid function
> whether
> boot_cpu_data.x86_phys_bits has been initialized or not, I think, but
> I am
> not sure about the correctness of this approach.


No, I think your patch is good. The only thing I'd suggest is to move
the call a few lines down. The way it is placed now may create
impression that we are calling get_cpu_address_sizes() to figure out NX
support.


-boris

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply

* Re: wireguardnl: Go package for interacting with WireGuard via generic netlink
From: Jason A. Donenfeld @ 2018-07-23 15:12 UTC (permalink / raw)
  To: mdlayher; +Cc: WireGuard mailing list
In-Reply-To: <3c167a80-6459-7c0e-8935-a98e226fa023@gmail.com>

Hi Matt,

> This is super interesting and I actually did not discover it until after
> I pushed the first few commits to my package.  I could see it making
> sense to refactor my current package layout to something like three
> packages:
>
> - wireguardnl: netlink-based communication
> - wireguardcfg: text-based userspace configuration protocol communication
> - wireguard: wrapper for both that detects the module in use and
> seamlessly presents a unified interface

No, that's really not a good approach at all. First of all, do not
take the raw name "wireguard". That's going to cause a lot of
confusion. It's really not appropriate.

But more importantly, you shouldn't expose either the netlink or the
xplatform API distinction to users ever. They should be given one
interface, not three, and that one interface should be able to select
the right thing in 100% of cases.

Jason

^ permalink raw reply

* Re: [PATCH v2 07/12] sched/core: uclamp: enforce last task UCLAMP_MAX
From: Patrick Bellasi @ 2018-07-23 15:02 UTC (permalink / raw)
  To: Suren Baghdasaryan
  Cc: linux-kernel, linux-pm, Ingo Molnar, Peter Zijlstra, Tejun Heo,
	Rafael J . Wysocki, Viresh Kumar, Vincent Guittot, Paul Turner,
	Dietmar Eggemann, Morten Rasmussen, Juri Lelli, Todd Kjos,
	Joel Fernandes, Steve Muckle
In-Reply-To: <CAJuCfpH6CsDVG35Z3NV87mUgJW9TmoCuOs8GeW2TEqkP0K0f9g@mail.gmail.com>

On 20-Jul 18:23, Suren Baghdasaryan wrote:
> Hi Patrick,

Hi Sure,
thank!

> On Mon, Jul 16, 2018 at 1:29 AM, Patrick Bellasi
> <patrick.bellasi@arm.com> wrote:

[...]

> > @@ -977,13 +991,21 @@ static inline void uclamp_cpu_get_id(struct task_struct *p,
> >         uc_grp = &rq->uclamp.group[clamp_id][0];
> >         uc_grp[group_id].tasks += 1;
> >
> > +       /* Force clamp update on idle exit */
> > +       uc_cpu = &rq->uclamp;
> > +       clamp_value = p->uclamp[clamp_id].value;
> > +       if (unlikely(uc_cpu->flags & UCLAMP_FLAG_IDLE)) {
> 
> The condition below is not needed because UCLAMP_FLAG_IDLE is set only
> for UCLAMP_MAX clamp_id, therefore the above condition already covers
> the one below.

Not really, this function is called two times, the first time to
update UCLAMP_MIN and a second time to update UCLAMP_MAX.

For both clamp_id we want to force update uc_cpu->value[clamp_id],
thus the UCLAMP_FLAG_IDLE flag has to be cleared only the second time.

Maybe I can had the following comment to better explain the reason of
the check:

		/*
		 * This function is called for both UCLAMP_MIN (before) and
		 * UCLAMP_MAX (after). Let's reset the flag only the when
		 * we know that UCLAMP_MIN has been already updated.
		 */

> > +               if (clamp_id == UCLAMP_MAX)
> > +                       uc_cpu->flags &= ~UCLAMP_FLAG_IDLE;
> > +               uc_cpu->value[clamp_id] = clamp_value;
> > +               return;
> > +       }

[...]

-- 
#include <best/regards.h>

Patrick Bellasi

^ permalink raw reply

* Re: [PATCH 5/7] btrfs: warn for num_devices below 0
From: David Sterba @ 2018-07-23 14:01 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs
In-Reply-To: <20180716145812.20836-6-anand.jain@oracle.com>

On Mon, Jul 16, 2018 at 10:58:10PM +0800, Anand Jain wrote:
> In preparation to de-duplicate a section of code where we deduce the
> num_devices, use warn instead of bug.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/volumes.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 7f4973fc2b52..0f4c512aa6b4 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -3726,7 +3726,7 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
>  	num_devices = fs_info->fs_devices->num_devices;
>  	btrfs_dev_replace_read_lock(&fs_info->dev_replace);
>  	if (btrfs_dev_replace_is_ongoing(&fs_info->dev_replace)) {
> -		BUG_ON(num_devices < 1);
> +		WARN_ON(num_devices < 1);

I wonder if there any valid cases when there are 0 devices when balance
is started, ie. before num_devices gets decremented.

The WARN_ON is either redundant or should be turned to a proper sanity
check.

>  		num_devices--;

^ permalink raw reply

* Re: [PATCH 3/4] perf/x86/intel/ds: Handle PEBS overflow for fixed counters
From: Peter Zijlstra @ 2018-07-23 15:02 UTC (permalink / raw)
  To: kan.liang
  Cc: tglx, mingo, linux-kernel, acme, alexander.shishkin,
	vincent.weaver, jolsa, ak
In-Reply-To: <20180723145944.GB2458@hirez.programming.kicks-ass.net>

On Mon, Jul 23, 2018 at 04:59:44PM +0200, Peter Zijlstra wrote:
> On Thu, Mar 08, 2018 at 06:15:41PM -0800, kan.liang@linux.intel.com wrote:
> > diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> > index ef47a418d819..86149b87cce8 100644
> > --- a/arch/x86/events/intel/core.c
> > +++ b/arch/x86/events/intel/core.c
> > @@ -2280,7 +2280,10 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
> >  	 * counters from the GLOBAL_STATUS mask and we always process PEBS
> >  	 * events via drain_pebs().
> >  	 */
> > -	status &= ~(cpuc->pebs_enabled & PEBS_COUNTER_MASK);
> > +	if (x86_pmu.flags & PMU_FL_PEBS_ALL)
> > +		status &= ~(cpuc->pebs_enabled & EXTENDED_PEBS_COUNTER_MASK);
> > +	else
> > +		status &= ~(cpuc->pebs_enabled & PEBS_COUNTER_MASK);
> >  
> >  	/*
> >  	 * PEBS overflow sets bit 62 in the global status register
> 
> Doesn't this re-introduce the problem fixed in commit fd583ad1563be,
> where pebs_enabled:32-34 are PEBS Load Latency, instead of fixed
> counters?

*GROAN* the MSR definitions now differ between Core and Atom :-(

^ permalink raw reply

* [PATCH 10/10] NFC: st95hf: add of match table
From: Daniel Mack @ 2018-07-23 14:00 UTC (permalink / raw)
  To: sameo; +Cc: linux-wireless, colin.king, shikha.singh, Daniel Mack, devicetree
In-Reply-To: <20180723140015.11663-1-daniel@zonque.org>

Add a match table for device tree compatible strings. Interestingly, a
document describing the bindings already exists since a while, but users
currently reply on the implicit matching in the drivers core.

Signed-off-by: Daniel Mack <daniel@zonque.org>
Cc: devicetree@vger.kernel.org
---
 drivers/nfc/st95hf/core.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/nfc/st95hf/core.c b/drivers/nfc/st95hf/core.c
index 4db3c020921c..5a01b454fbc4 100644
--- a/drivers/nfc/st95hf/core.c
+++ b/drivers/nfc/st95hf/core.c
@@ -1012,6 +1012,12 @@ static const struct spi_device_id st95hf_id[] = {
 };
 MODULE_DEVICE_TABLE(spi, st95hf_id);
 
+static const struct of_device_id st95hf_of_match[] = {
+	{ .compatible = "st,st95hf", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, st95hf_of_match);
+
 static int st95hf_probe(struct spi_device *nfc_spi_dev)
 {
 	int ret;
@@ -1189,6 +1195,7 @@ static struct spi_driver st95hf_driver = {
 	.driver = {
 		.name = "st95hf",
 		.owner = THIS_MODULE,
+		.of_match_table = st95hf_of_match,
 	},
 	.id_table = st95hf_id,
 	.probe = st95hf_probe,
-- 
2.17.1

^ permalink raw reply related

* [PATCH v2 4/4] iio: adc: xilinx: Move request_irq before enabling interrupts
From: Manish Narani @ 2018-07-23 15:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1532358123-23485-1-git-send-email-manish.narani@xilinx.com>

Enabling the Interrupts before registering the irq handler is a bad
idea. This patch corrects the same for XADC driver.

Signed-off-by: Manish Narani <manish.narani@xilinx.com>
---
 drivers/iio/adc/xilinx-xadc-core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index 44a2519..3f6be5a 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -1226,15 +1226,15 @@ static int xadc_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_free_samplerate_trigger;
 
-	ret = xadc->ops->setup(pdev, indio_dev, xadc->irq);
-	if (ret)
-		goto err_clk_disable_unprepare;
-
 	ret = request_irq(xadc->irq, xadc->ops->interrupt_handler, 0,
 			dev_name(&pdev->dev), indio_dev);
 	if (ret)
 		goto err_clk_disable_unprepare;
 
+	ret = xadc->ops->setup(pdev, indio_dev, xadc->irq);
+	if (ret)
+		goto err_free_irq;
+
 	for (i = 0; i < 16; i++)
 		xadc_read_adc_reg(xadc, XADC_REG_THRESHOLD(i),
 			&xadc->threshold[i]);
-- 
2.1.1

^ permalink raw reply related

* [PATCH v2 4/4] iio: adc: xilinx: Move request_irq before enabling interrupts
From: Manish Narani @ 2018-07-23 15:02 UTC (permalink / raw)
  To: lakshmi.sai.krishna.potthuri, manish.narani, michal.simek, pmeerw,
	lars, knaack.h, jic23
  Cc: anirudh, sgoud, linux-kernel, linux-arm-kernel, linux-iio
In-Reply-To: <1532358123-23485-1-git-send-email-manish.narani@xilinx.com>

Enabling the Interrupts before registering the irq handler is a bad
idea. This patch corrects the same for XADC driver.

Signed-off-by: Manish Narani <manish.narani@xilinx.com>
---
 drivers/iio/adc/xilinx-xadc-core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index 44a2519..3f6be5a 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -1226,15 +1226,15 @@ static int xadc_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_free_samplerate_trigger;
 
-	ret = xadc->ops->setup(pdev, indio_dev, xadc->irq);
-	if (ret)
-		goto err_clk_disable_unprepare;
-
 	ret = request_irq(xadc->irq, xadc->ops->interrupt_handler, 0,
 			dev_name(&pdev->dev), indio_dev);
 	if (ret)
 		goto err_clk_disable_unprepare;
 
+	ret = xadc->ops->setup(pdev, indio_dev, xadc->irq);
+	if (ret)
+		goto err_free_irq;
+
 	for (i = 0; i < 16; i++)
 		xadc_read_adc_reg(xadc, XADC_REG_THRESHOLD(i),
 			&xadc->threshold[i]);
-- 
2.1.1

^ permalink raw reply related

* [PATCH v2 3/4] iio: adc: xilinx: Remove platform_get_irq from xadc_remove function
From: Manish Narani @ 2018-07-23 15:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1532358123-23485-1-git-send-email-manish.narani@xilinx.com>

This patch avoids getting irq number in xadc_remove function. Instead
store 'irq' in xadc struct and use xadc->irq wherever needed.
This patch also resolves a warning reported by coverity where it asks to
check return value of platform_get_irq() for any errors in xadc_remove.

Signed-off-by: Manish Narani <manish.narani@xilinx.com>
---
 drivers/iio/adc/xilinx-xadc-core.c | 10 +++++-----
 drivers/iio/adc/xilinx-xadc.h      |  1 +
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index 0dd306d..44a2519 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -1175,6 +1175,7 @@ static int xadc_probe(struct platform_device *pdev)
 
 	xadc = iio_priv(indio_dev);
 	xadc->ops = id->data;
+	xadc->irq = irq;
 	init_completion(&xadc->completion);
 	mutex_init(&xadc->mutex);
 	spin_lock_init(&xadc->lock);
@@ -1225,11 +1226,11 @@ static int xadc_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_free_samplerate_trigger;
 
-	ret = xadc->ops->setup(pdev, indio_dev, irq);
+	ret = xadc->ops->setup(pdev, indio_dev, xadc->irq);
 	if (ret)
 		goto err_clk_disable_unprepare;
 
-	ret = request_irq(irq, xadc->ops->interrupt_handler, 0,
+	ret = request_irq(xadc->irq, xadc->ops->interrupt_handler, 0,
 			dev_name(&pdev->dev), indio_dev);
 	if (ret)
 		goto err_clk_disable_unprepare;
@@ -1288,7 +1289,7 @@ static int xadc_probe(struct platform_device *pdev)
 	return 0;
 
 err_free_irq:
-	free_irq(irq, indio_dev);
+	free_irq(xadc->irq, indio_dev);
 err_clk_disable_unprepare:
 	clk_disable_unprepare(xadc->clk);
 err_free_samplerate_trigger:
@@ -1310,7 +1311,6 @@ static int xadc_remove(struct platform_device *pdev)
 {
 	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
 	struct xadc *xadc = iio_priv(indio_dev);
-	int irq = platform_get_irq(pdev, 0);
 
 	iio_device_unregister(indio_dev);
 	if (xadc->ops->flags & XADC_FLAGS_BUFFERED) {
@@ -1318,7 +1318,7 @@ static int xadc_remove(struct platform_device *pdev)
 		iio_trigger_free(xadc->convst_trigger);
 		iio_triggered_buffer_cleanup(indio_dev);
 	}
-	free_irq(irq, indio_dev);
+	free_irq(xadc->irq, indio_dev);
 	clk_disable_unprepare(xadc->clk);
 	cancel_delayed_work(&xadc->zynq_unmask_work);
 	kfree(xadc->data);
diff --git a/drivers/iio/adc/xilinx-xadc.h b/drivers/iio/adc/xilinx-xadc.h
index 62edbda..8c00095 100644
--- a/drivers/iio/adc/xilinx-xadc.h
+++ b/drivers/iio/adc/xilinx-xadc.h
@@ -68,6 +68,7 @@ struct xadc {
 	spinlock_t lock;
 
 	struct completion completion;
+	int irq;
 };
 
 struct xadc_ops {
-- 
2.1.1

^ permalink raw reply related

* [PATCH 09/10] NFC: st95hf: two small style nits
From: Daniel Mack @ 2018-07-23 14:00 UTC (permalink / raw)
  To: sameo; +Cc: linux-wireless, colin.king, shikha.singh, Daniel Mack
In-Reply-To: <20180723140015.11663-1-daniel@zonque.org>

Address two minor style issues that I came across while working on the
driver.

Signed-off-by: Daniel Mack <daniel@zonque.org>
---
 drivers/nfc/st95hf/core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/nfc/st95hf/core.c b/drivers/nfc/st95hf/core.c
index 835a1e61c817..4db3c020921c 100644
--- a/drivers/nfc/st95hf/core.c
+++ b/drivers/nfc/st95hf/core.c
@@ -662,7 +662,8 @@ static int st95hf_error_handling(struct st95hf_context *stcontext,
 			result = -ETIMEDOUT;
 		else
 			result = -EIO;
-	return  result;
+
+		return result;
 	}
 
 	/* Check for CRC err only if CRC is present in the tag response */
@@ -844,6 +845,7 @@ static irqreturn_t st95hf_irq_thread_handler(int irq, void  *st95hfcontext)
 	/* call of callback with error */
 	cb_arg->complete_cb(stcontext->ddev, cb_arg->cb_usrarg, skb_resp);
 	mutex_unlock(&stcontext->rm_lock);
+
 	return IRQ_HANDLED;
 }
 
-- 
2.17.1

^ permalink raw reply related

* [PATCH 08/10] NFC: st95hf: unify sync/async flags
From: Daniel Mack @ 2018-07-23 14:00 UTC (permalink / raw)
  To: sameo; +Cc: linux-wireless, colin.king, shikha.singh, Daniel Mack
In-Reply-To: <20180723140015.11663-1-daniel@zonque.org>

Keep the information whether a command is asynchronous in a boolean flag
everywhere in the code. This way, the enum can go away.

Signed-off-by: Daniel Mack <daniel@zonque.org>
---
 drivers/nfc/st95hf/core.c | 38 ++++++++++++++++----------------------
 drivers/nfc/st95hf/spi.c  | 12 +++++-------
 drivers/nfc/st95hf/spi.h  |  8 +-------
 3 files changed, 22 insertions(+), 36 deletions(-)

diff --git a/drivers/nfc/st95hf/core.c b/drivers/nfc/st95hf/core.c
index e7ecc57dde8f..835a1e61c817 100644
--- a/drivers/nfc/st95hf/core.c
+++ b/drivers/nfc/st95hf/core.c
@@ -101,7 +101,7 @@ struct cmd {
 	unsigned char cmd_id;
 	unsigned char no_cmd_params;
 	unsigned char cmd_params[MAX_CMD_PARAMS];
-	enum req_type req;
+	bool is_sync;
 };
 
 struct param_list {
@@ -134,63 +134,62 @@ static const struct cmd cmd_array[] = {
 		.cmd_len = 0x2,
 		.cmd_id = ECHO_CMD,
 		.no_cmd_params = 0,
-		.req = SYNC,
+		.is_sync = true,
 	},
 	[CMD_ISO14443A_CONFIG] = {
 		.cmd_len = 0x7,
 		.cmd_id = WRITE_REGISTER_CMD,
 		.no_cmd_params = 0x4,
 		.cmd_params = {0x3A, 0x00, 0x5A, 0x04},
-		.req = SYNC,
+		.is_sync = true,
 	},
 	[CMD_ISO14443A_DEMOGAIN] = {
 		.cmd_len = 0x7,
 		.cmd_id = WRITE_REGISTER_CMD,
 		.no_cmd_params = 0x4,
 		.cmd_params = {0x68, 0x01, 0x01, 0xDF},
-		.req = SYNC,
+		.is_sync = true,
 	},
 	[CMD_ISO14443B_DEMOGAIN] = {
 		.cmd_len = 0x7,
 		.cmd_id = WRITE_REGISTER_CMD,
 		.no_cmd_params = 0x4,
 		.cmd_params = {0x68, 0x01, 0x01, 0x51},
-		.req = SYNC,
+		.is_sync = true,
 	},
 	[CMD_ISO14443A_PROTOCOL_SELECT] = {
 		.cmd_len = 0x7,
 		.cmd_id = PROTOCOL_SELECT_CMD,
 		.no_cmd_params = 0x4,
 		.cmd_params = {ISO14443A_PROTOCOL_CODE, 0x00, 0x01, 0xA0},
-		.req = SYNC,
+		.is_sync = true,
 	},
 	[CMD_ISO14443B_PROTOCOL_SELECT] = {
 		.cmd_len = 0x7,
 		.cmd_id = PROTOCOL_SELECT_CMD,
 		.no_cmd_params = 0x4,
 		.cmd_params = {ISO14443B_PROTOCOL_CODE, 0x01, 0x03, 0xFF},
-		.req = SYNC,
+		.is_sync = true,
 	},
 	[CMD_WTX_RESPONSE] = {
 		.cmd_len = 0x6,
 		.cmd_id = SEND_RECEIVE_CMD,
 		.no_cmd_params = 0x3,
 		.cmd_params = {0xF2, 0x00, TRFLAG_NFCA_STD_FRAME_CRC},
-		.req = ASYNC,
 	},
 	[CMD_FIELD_OFF] = {
 		.cmd_len = 0x5,
 		.cmd_id = PROTOCOL_SELECT_CMD,
 		.no_cmd_params = 0x2,
 		.cmd_params = {0x0, 0x0},
-		.req = SYNC,
+		.is_sync = true,
 	},
 	[CMD_ISO15693_PROTOCOL_SELECT] = {
 		.cmd_len = 0x5,
 		.cmd_id = PROTOCOL_SELECT_CMD,
 		.no_cmd_params = 0x2,
 		.cmd_params = {ISO15693_PROTOCOL_CODE, 0x0D},
-		.req = SYNC,
+		.is_sync = true,
 	},
 };
 
@@ -279,13 +278,13 @@ static int st95hf_send_recv_cmd(struct st95hf_context *st95context,
 	ret = st95hf_spi_send(&st95context->spicontext,
 			      spi_cmd_buffer,
 			      cmd_array[cmd].cmd_len,
-			      cmd_array[cmd].req);
+			      cmd_array[cmd].is_sync);
 	if (ret) {
 		dev_err(dev, "st95hf_spi_send failed with error %d\n", ret);
 		return ret;
 	}
 
-	if (cmd_array[cmd].req == SYNC && recv_res) {
+	if (cmd_array[cmd].is_sync && recv_res) {
 		unsigned char st95hf_response_arr[2];
 
 		ret = st95hf_spi_recv_response(&st95context->spicontext,
@@ -480,10 +479,8 @@ static int st95hf_send_spi_reset_sequence(struct st95hf_context *st95context)
 	int result = 0;
 	unsigned char reset_cmd = ST95HF_COMMAND_RESET;
 
-	result = st95hf_spi_send(&st95context->spicontext,
-				 &reset_cmd,
-				 ST95HF_RESET_CMD_LEN,
-				 ASYNC);
+	result = st95hf_spi_send(&st95context->spicontext, &reset_cmd,
+				 ST95HF_RESET_CMD_LEN, false);
 	if (result) {
 		dev_err(&st95context->spicontext.spidev->dev,
 			"spi reset sequence cmd error = %d", result);
@@ -932,8 +929,7 @@ static int st95hf_in_send_cmd(struct nfc_digital_dev *ddev,
 		stcontext->complete_cb_arg.rats = true;
 
 	rc = st95hf_spi_send(&stcontext->spicontext, skb->data,
-			     skb->len,
-			     ASYNC);
+			     skb->len, false);
 	if (rc) {
 		dev_err(&stcontext->nfcdev->dev,
 			"Error %d trying to perform data_exchange", rc);
@@ -1168,10 +1164,8 @@ static int st95hf_remove(struct spi_device *nfc_spi_dev)
 	mutex_unlock(&stcontext->rm_lock);
 
 	/* next reset the ST95HF controller */
-	result = st95hf_spi_send(&stcontext->spicontext,
-				 &reset_cmd,
-				 ST95HF_RESET_CMD_LEN,
-				 ASYNC);
+	result = st95hf_spi_send(&stcontext->spicontext, &reset_cmd,
+				 ST95HF_RESET_CMD_LEN, false);
 	if (result) {
 		dev_err(&spictx->spidev->dev,
 			"ST95HF reset failed in remove() err = %d\n", result);
diff --git a/drivers/nfc/st95hf/spi.c b/drivers/nfc/st95hf/spi.c
index d5894d4546b1..ef12ed67343f 100644
--- a/drivers/nfc/st95hf/spi.c
+++ b/drivers/nfc/st95hf/spi.c
@@ -23,7 +23,7 @@
 int st95hf_spi_send(struct st95hf_spi_context *spicontext,
 		    unsigned char *buffertx,
 		    int datalen,
-		    enum req_type reqtype)
+		    bool is_sync)
 {
 	struct spi_message m;
 	int result = 0;
@@ -35,12 +35,10 @@ int st95hf_spi_send(struct st95hf_spi_context *spicontext,
 
 	mutex_lock(&spicontext->spi_lock);
 
-	if (reqtype == SYNC) {
-		spicontext->req_issync = true;
+	spicontext->req_issync = is_sync;
+
+	if (is_sync)
 		reinit_completion(&spicontext->done);
-	} else {
-		spicontext->req_issync = false;
-	}
 
 	spi_message_init(&m);
 	spi_message_add_tail(&tx_transfer, &m);
@@ -52,7 +50,7 @@ int st95hf_spi_send(struct st95hf_spi_context *spicontext,
 	}
 
 	/* return for asynchronous or no-wait case */
-	if (reqtype == ASYNC) {
+	if (!is_sync) {
 		mutex_unlock(&spicontext->spi_lock);
 		return 0;
 	}
diff --git a/drivers/nfc/st95hf/spi.h b/drivers/nfc/st95hf/spi.h
index 552d220747cd..ede17eef6ab1 100644
--- a/drivers/nfc/st95hf/spi.h
+++ b/drivers/nfc/st95hf/spi.h
@@ -44,16 +44,10 @@ struct st95hf_spi_context {
 	struct mutex spi_lock;
 };
 
-/* flag to differentiate synchronous & asynchronous spi request */
-enum req_type {
-	SYNC,
-	ASYNC,
-};
-
 int st95hf_spi_send(struct st95hf_spi_context *spicontext,
 		    unsigned char *buffertx,
 		    int datalen,
-		    enum req_type reqtype);
+		    bool is_sync);
 
 int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
 			     unsigned char *receivebuff);
-- 
2.17.1

^ permalink raw reply related

* [PATCH v2 2/4] iio: adc: xilinx: limit pcap clock frequency value
From: Manish Narani @ 2018-07-23 15:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1532358123-23485-1-git-send-email-manish.narani@xilinx.com>

This patch limits the xadc pcap clock frequency value to be less than
200MHz. This fixes the issue when zynq is booted at higher frequency
values, pcap crosses the maximum limit of 200MHz(Fmax) as it is derived
from IOPLL.
If this limit is crossed it is required to alter the WEDGE and REDGE
bits of XADC_CFG register to make timings better in the interface. So to
avoid alteration of these bits every time, the pcap value should not
cross the Fmax limit.

Signed-off-by: Manish Narani <manish.narani@xilinx.com>
---
 drivers/iio/adc/xilinx-xadc-core.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index 23395fc..0dd306d 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -322,6 +322,7 @@ static irqreturn_t xadc_zynq_interrupt_handler(int irq, void *devid)
 
 #define XADC_ZYNQ_TCK_RATE_MAX 50000000
 #define XADC_ZYNQ_IGAP_DEFAULT 20
+#define XADC_ZYNQ_PCAP_RATE_MAX 200000000
 
 static int xadc_zynq_setup(struct platform_device *pdev,
 	struct iio_dev *indio_dev, int irq)
@@ -332,6 +333,7 @@ static int xadc_zynq_setup(struct platform_device *pdev,
 	unsigned int div;
 	unsigned int igap;
 	unsigned int tck_rate;
+	int ret;
 
 	/* TODO: Figure out how to make igap and tck_rate configurable */
 	igap = XADC_ZYNQ_IGAP_DEFAULT;
@@ -343,6 +345,13 @@ static int xadc_zynq_setup(struct platform_device *pdev,
 	if (!pcap_rate)
 		return -EINVAL;
 
+	if (pcap_rate > XADC_ZYNQ_PCAP_RATE_MAX) {
+		ret = clk_set_rate(xadc->clk,
+				   (unsigned long)XADC_ZYNQ_PCAP_RATE_MAX);
+		if (ret)
+			return ret;
+	}
+
 	if (tck_rate > pcap_rate / 2) {
 		div = 2;
 	} else {
@@ -368,6 +377,12 @@ static int xadc_zynq_setup(struct platform_device *pdev,
 			XADC_ZYNQ_CFG_REDGE | XADC_ZYNQ_CFG_WEDGE |
 			tck_div | XADC_ZYNQ_CFG_IGAP(igap));
 
+	if (pcap_rate > XADC_ZYNQ_PCAP_RATE_MAX) {
+		ret = clk_set_rate(xadc->clk, pcap_rate);
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }
 
-- 
2.1.1

^ permalink raw reply related

* [PATCH v2 1/4] iio: adc: xilinx: Check for return values in clk related functions
From: Manish Narani @ 2018-07-23 15:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1532358123-23485-1-git-send-email-manish.narani@xilinx.com>

This patch adds check for return values from clock related functions.
This was reported by static code analysis tool.

Signed-off-by: Manish Narani <manish.narani@xilinx.com>
---
 drivers/iio/adc/xilinx-xadc-core.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index 0127e85..23395fc 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -340,6 +340,8 @@ static int xadc_zynq_setup(struct platform_device *pdev,
 	xadc->zynq_intmask = ~0;
 
 	pcap_rate = clk_get_rate(xadc->clk);
+	if (!pcap_rate)
+		return -EINVAL;
 
 	if (tck_rate > pcap_rate / 2) {
 		div = 2;
@@ -887,6 +889,9 @@ static int xadc_write_raw(struct iio_dev *indio_dev,
 	unsigned long clk_rate = xadc_get_dclk_rate(xadc);
 	unsigned int div;
 
+	if (!clk_rate)
+		return -EINVAL;
+
 	if (info != IIO_CHAN_INFO_SAMP_FREQ)
 		return -EINVAL;
 
@@ -1237,8 +1242,10 @@ static int xadc_probe(struct platform_device *pdev)
 		goto err_free_irq;
 
 	/* Disable all alarms */
-	xadc_update_adc_reg(xadc, XADC_REG_CONF1, XADC_CONF1_ALARM_MASK,
-		XADC_CONF1_ALARM_MASK);
+	ret = xadc_update_adc_reg(xadc, XADC_REG_CONF1, XADC_CONF1_ALARM_MASK,
+				  XADC_CONF1_ALARM_MASK);
+	if (ret)
+		goto err_free_irq;
 
 	/* Set thresholds to min/max */
 	for (i = 0; i < 16; i++) {
-- 
2.1.1

^ permalink raw reply related

* [PATCH 05/10] NFC: st95hf: remove exchange_lock
From: Daniel Mack @ 2018-07-23 14:00 UTC (permalink / raw)
  To: sameo; +Cc: linux-wireless, colin.king, shikha.singh, Daniel Mack
In-Reply-To: <20180723140015.11663-1-daniel@zonque.org>

This patch removes the exchange_lock sempahore. Its intended function
was two-fold:

a) Lock the remove() callback of the driver against the ISR, so that
   the resources only go away after the ISR has finished. This is
   unnecessary though, because `rm_lock' does that already, in
   combination with the nullification of `scontext->ddev'.

b) Indicate whether a command was sent previously. If the semaphore
   is found unused in the threaded ISR, an error is reported.
   This case can be handled much nicer by checking whether `skb_resp'
   is present in the context. For this, nullify the `skb_resp' pointer
   in the callback context.

Signed-off-by: Daniel Mack <daniel@zonque.org>
---
 drivers/nfc/st95hf/core.c | 52 +++++++--------------------------------
 1 file changed, 9 insertions(+), 43 deletions(-)

diff --git a/drivers/nfc/st95hf/core.c b/drivers/nfc/st95hf/core.c
index d857197ec7b2..6761ab90f68d 100644
--- a/drivers/nfc/st95hf/core.c
+++ b/drivers/nfc/st95hf/core.c
@@ -214,8 +214,6 @@ struct st95_digital_cmd_complete_arg {
  * @st95hf_supply: regulator "consumer" for NFC device.
  * @sendrcv_trflag: last byte of frame send by sendrecv command
  *	of st95hf. This byte contains transmission flag info.
- * @exchange_lock: semaphore used for signaling the st95hf_remove
- *	function that the last outstanding async nfc request is finished.
  * @rm_lock: mutex for ensuring safe access of nfc digital object
  *	from threaded ISR. Usage of this mutex avoids any race between
  *	deletion of the object from st95hf_remove() and its access from
@@ -233,7 +231,6 @@ struct st95hf_context {
 	struct st95_digital_cmd_complete_arg complete_cb_arg;
 	struct regulator *st95hf_supply;
 	unsigned char sendrcv_trflag;
-	struct semaphore exchange_lock;
 	struct mutex rm_lock;
 	u8 current_protocol;
 	u8 current_rf_tech;
@@ -785,29 +782,14 @@ static irqreturn_t st95hf_irq_thread_handler(int irq, void  *st95hfcontext)
 	struct st95_digital_cmd_complete_arg *cb_arg;
 
 	spidevice = &stcontext->spicontext.spidev->dev;
+	cb_arg = &stcontext->complete_cb_arg;
+	skb_resp = cb_arg->skb_resp;
 
-	/*
-	 * check semaphore, if not down() already, then we don't
-	 * know in which context the ISR is called and surely it
-	 * will be a bug. Note that down() of the semaphore is done
-	 * in the corresponding st95hf_in_send_cmd() and then
-	 * only this ISR should be called. ISR will up() the
-	 * semaphore before leaving. Hence when the ISR is called
-	 * the correct behaviour is down_trylock() should always
-	 * return 1 (indicating semaphore cant be taken and hence no
-	 * change in semaphore count).
-	 * If not, then we up() the semaphore and crash on
-	 * a BUG() !
-	 */
-	if (!down_trylock(&stcontext->exchange_lock)) {
-		up(&stcontext->exchange_lock);
+	if (unlikely(!skb_resp)) {
 		WARN(1, "unknown context in ST95HF ISR");
 		return IRQ_NONE;
 	}
 
-	cb_arg = &stcontext->complete_cb_arg;
-	skb_resp = cb_arg->skb_resp;
-
 	mutex_lock(&stcontext->rm_lock);
 	res_len = st95hf_spi_recv_response(&stcontext->spicontext,
 					   skb_resp->data);
@@ -856,8 +838,13 @@ static irqreturn_t st95hf_irq_thread_handler(int irq, void  *st95hfcontext)
 	/* call digital layer callback */
 	cb_arg->complete_cb(stcontext->ddev, cb_arg->cb_usrarg, skb_resp);
 
+	/*
+	 * This skb is now owned by the core layer.
+	 * Make sure not to use it again.
+	 */
+	cb_arg->skb_resp = NULL;
+
 	/* up the semaphore before returning */
-	up(&stcontext->exchange_lock);
 	mutex_unlock(&stcontext->rm_lock);
 
 	return IRQ_HANDLED;
@@ -868,8 +855,6 @@ static irqreturn_t st95hf_irq_thread_handler(int irq, void  *st95hfcontext)
 	skb_resp = ERR_PTR(result);
 	/* call of callback with error */
 	cb_arg->complete_cb(stcontext->ddev, cb_arg->cb_usrarg, skb_resp);
-	/* up the semaphore before returning */
-	up(&stcontext->exchange_lock);
 	mutex_unlock(&stcontext->rm_lock);
 	return IRQ_HANDLED;
 }
@@ -965,25 +950,12 @@ static int st95hf_in_send_cmd(struct nfc_digital_dev *ddev,
 	    ddev->curr_protocol == NFC_PROTO_ISO14443)
 		stcontext->complete_cb_arg.rats = true;
 
-	/*
-	 * down the semaphore to indicate to remove func that an
-	 * ISR is pending, note that it will not block here in any case.
-	 * If found blocked, it is a BUG!
-	 */
-	rc = down_killable(&stcontext->exchange_lock);
-	if (rc) {
-		WARN(1, "Semaphore is not found up in st95hf_in_send_cmd\n");
-		return rc;
-	}
-
 	rc = st95hf_spi_send(&stcontext->spicontext, skb->data,
 			     skb->len,
 			     ASYNC);
 	if (rc) {
 		dev_err(&stcontext->nfcdev->dev,
 			"Error %d trying to perform data_exchange", rc);
-		/* up the semaphore since ISR will never come in this case */
-		up(&stcontext->exchange_lock);
 		goto free_skb_resp;
 	}
 
@@ -1104,7 +1076,6 @@ static int st95hf_probe(struct spi_device *nfc_spi_dev)
 		}
 	}
 
-	sema_init(&st95context->exchange_lock, 1);
 	init_completion(&spicontext->done);
 	mutex_init(&spicontext->spi_lock);
 	mutex_init(&st95context->rm_lock);
@@ -1220,11 +1191,6 @@ static int st95hf_remove(struct spi_device *nfc_spi_dev)
 
 	mutex_unlock(&stcontext->rm_lock);
 
-	/* if last in_send_cmd's ISR is pending, wait for it to finish */
-	result = down_killable(&stcontext->exchange_lock);
-	if (result == -EINTR)
-		dev_err(&spictx->spidev->dev, "sleep for semaphore interrupted by signal\n");
-
 	/* next reset the ST95HF controller */
 	result = st95hf_spi_send(&stcontext->spicontext,
 				 &reset_cmd,
-- 
2.17.1

^ permalink raw reply related


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.