All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/9] soundwire: intel: revisit SHIM programming
From: Bard Liao @ 2020-07-16 15:09 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: pierre-louis.bossart, vinod.koul, tiwai, gregkh, linux-kernel,
	ranjani.sridharan, hui.wang, broonie, srinivas.kandagatla, jank,
	mengdong.lin, slawomir.blauciak, sanyog.r.kale, rander.wang,
	bard.liao

This series does some cleanup, revisits SHIM programming sequences,
and merges Soundwire interrupt handlers/threads.

changes in v2:
 - Resume M device instead of S device in intel_master_process_wakeen_event
   function. See the comment in intel_master_process_wakeen_event() in
   detail.

Bard Liao (2):
  soundwire: intel/cadence: merge Soundwire interrupt handlers/threads
  Soundwire: intel_init: save Slave(s) _ADR info in sdw_intel_ctx

Pierre-Louis Bossart (6):
  soundwire: intel: reuse code for wait loops to set/clear bits
  soundwire: intel: revisit SHIM programming sequences.
  soundwire: intel: introduce a helper to arm link synchronization
  soundwire: intel: introduce helper for link synchronization
  soundwire: intel_init: add implementation of sdw_intel_enable_irq()
  soundwire: intel_init: use EXPORT_SYMBOL_NS

Rander Wang (1):
  soundwire: intel: add wake interrupt support

 drivers/soundwire/cadence_master.c  |  18 +-
 drivers/soundwire/cadence_master.h  |   4 +
 drivers/soundwire/intel.c           | 381 +++++++++++++++++++++-------
 drivers/soundwire/intel.h           |   9 +
 drivers/soundwire/intel_init.c      | 101 +++++++-
 include/linux/soundwire/sdw_intel.h |   2 +
 6 files changed, 417 insertions(+), 98 deletions(-)

-- 
2.17.1


^ permalink raw reply

* [PATCH v4 4/7] arch/x86: Implement text_alloc() and text_free()
From: Jarkko Sakkinen @ 2020-07-17  3:04 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jarkko Sakkinen, Masami Hiramatsu, Andi Kleen, Peter Zijlstra,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Miroslav Benes, Paul E. McKenney, Josh Poimboeuf, Babu Moger,
	Nayna Jain, Omar Sandoval, Marco Elver, Andy Lutomirski,
	Brian Gerst
In-Reply-To: <20200717030422.679972-1-jarkko.sakkinen@linux.intel.com>

Implement text_alloc() and text_free() with vmalloc() and vfree(), thus
dropping the dependency to the module subsystem.

Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Andi Kleen <ak@linux.intel.com>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 arch/x86/Kconfig             |  3 +++
 arch/x86/kernel/Makefile     |  1 +
 arch/x86/kernel/text_alloc.c | 41 ++++++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+)
 create mode 100644 arch/x86/kernel/text_alloc.c

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0dea7fdd7a00..a4ee5d1300f6 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2035,6 +2035,9 @@ config KEXEC_FILE
 config ARCH_HAS_KEXEC_PURGATORY
 	def_bool KEXEC_FILE
 
+config ARCH_HAS_TEXT_ALLOC
+	def_bool y
+
 config KEXEC_SIG
 	bool "Verify kernel signature during kexec_file_load() syscall"
 	depends on KEXEC_FILE
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index e77261db2391..fa1415424ae3 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -68,6 +68,7 @@ obj-y			+= tsc.o tsc_msr.o io_delay.o rtc.o
 obj-y			+= pci-iommu_table.o
 obj-y			+= resource.o
 obj-y			+= irqflags.o
+obj-y			+= text_alloc.o
 
 obj-y				+= process.o
 obj-y				+= fpu/
diff --git a/arch/x86/kernel/text_alloc.c b/arch/x86/kernel/text_alloc.c
new file mode 100644
index 000000000000..c8e5296ebceb
--- /dev/null
+++ b/arch/x86/kernel/text_alloc.c
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *  Kernel module help for x86.
+ *  Copyright (C) 2001 Rusty Russell.
+ */
+
+#include <linux/kasan.h>
+#include <linux/mm.h>
+#include <linux/moduleloader.h>
+#include <linux/vmalloc.h>
+#include <asm/setup.h>
+
+void *text_alloc(unsigned long size)
+{
+	void *p;
+
+	if (PAGE_ALIGN(size) > MODULES_LEN)
+		return NULL;
+
+	p = __vmalloc_node_range(size, MODULE_ALIGN, MODULES_VADDR, MODULES_END,
+				 GFP_KERNEL, PAGE_KERNEL, 0, NUMA_NO_NODE,
+				 __builtin_return_address(0));
+
+	if (p && (kasan_module_alloc(p, size) < 0)) {
+		vfree(p);
+		return NULL;
+	}
+
+	return p;
+}
+
+void text_free(void *region)
+{
+	/*
+	 * This memory may be RO, and freeing RO memory in an interrupt is not
+	 * supported by vmalloc.
+	 */
+	WARN_ON(in_interrupt());
+
+	vfree(region);
+}
-- 
2.25.1


^ permalink raw reply related

* [PATCH] drm/amdgpu: enable xgmi support for sienna cichlid
From: Clements, John @ 2020-07-17  3:05 UTC (permalink / raw)
  To: amd-gfx list, Zhang, Hawking


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

[AMD Public Use]

Submitting patch to enable XGMI support for Sienna Cichlid

[-- Attachment #1.2: Type: text/html, Size: 1801 bytes --]

[-- Attachment #2: 0002-drm-amdgpu-enable-xgmi-support-for-sienna-cichlid.patch --]
[-- Type: application/octet-stream, Size: 1011 bytes --]

From 4a30c975851ab22f34b2c135394f8f12477535d0 Mon Sep 17 00:00:00 2001
From: John Clements <john.clements@amd.com>
Date: Fri, 17 Jul 2020 10:58:14 +0800
Subject: [PATCH] drm/amdgpu: enable xgmi support for sienna cichlid

set xgmi support flag suring nv ip init sequence

Signed-off-by: John Clements <john.clements@amd.com>
Change-Id: I4935a07807efe76979ff0c94a97e5065a801492f
---
 drivers/gpu/drm/amd/amdgpu/nv.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
index 479991b71295..ea69ae76773e 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -446,6 +446,9 @@ int nv_set_ip_blocks(struct amdgpu_device *adev)
 	adev->nbio.funcs = &nbio_v2_3_funcs;
 	adev->nbio.hdp_flush_reg = &nbio_v2_3_hdp_flush_reg;
 
+	if (adev->asic_type == CHIP_SIENNA_CICHLID)
+		adev->gmc.xgmi.supported = true;
+
 	/* Set IP register base before any HW register access */
 	r = nv_reg_base_init(adev);
 	if (r)
-- 
2.17.1


[-- Attachment #3: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related

* [PATCH v4 3/7] vmalloc: Add text_alloc() and text_free()
From: Jarkko Sakkinen @ 2020-07-17  3:04 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jarkko Sakkinen, Andi Kleen, Masami Hiramatsu, Peter Zijlstra,
	Andrew Morton, open list:MEMORY MANAGEMENT
In-Reply-To: <20200717030422.679972-1-jarkko.sakkinen@linux.intel.com>

Introduce functions for allocating memory for dynamic trampolines, such
as kprobes. An arch can promote the availability of these functions with
CONFIG_ARCH_HAS_TEXT_ALLOC. Provide default/fallback implementation
wrapping module_alloc() and module_memfree().

Cc: Andi Kleen <ak@linux.intel.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 include/linux/vmalloc.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 0221f852a7e1..e981436e30b6 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -9,6 +9,7 @@
 #include <asm/page.h>		/* pgprot_t */
 #include <linux/rbtree.h>
 #include <linux/overflow.h>
+#include <linux/moduleloader.h>
 
 #include <asm/vmalloc.h>
 
@@ -249,4 +250,26 @@ pcpu_free_vm_areas(struct vm_struct **vms, int nr_vms)
 int register_vmap_purge_notifier(struct notifier_block *nb);
 int unregister_vmap_purge_notifier(struct notifier_block *nb);
 
+#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
+/*
+ * Allocate memory to be used for dynamic trampoline code.
+ */
+void *text_alloc(unsigned long size);
+
+/*
+ * Free memory returned from text_alloc().
+ */
+void text_free(void *region);
+#else
+static inline void *text_alloc(unsigned long size)
+{
+	return module_alloc(size);
+}
+
+static inline void text_free(void *region)
+{
+	module_memfree(region);
+}
+#endif
+
 #endif /* _LINUX_VMALLOC_H */
-- 
2.25.1



^ permalink raw reply related

* [PATCH v2 8/9] soundwire: intel: add wake interrupt support
From: Bard Liao @ 2020-07-16 15:09 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
	srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
	pierre-louis.bossart, sanyog.r.kale, slawomir.blauciak,
	mengdong.lin, bard.liao
In-Reply-To: <20200716150947.22119-1-yung-chuan.liao@linux.intel.com>

From: Rander Wang <rander.wang@intel.com>

When system is suspended in clock stop mode on intel platforms, both
master and slave are in clock stop mode and soundwire bus is taken
over by a glue hardware. The bus message for jack event is processed
by this glue hardware, which will trigger an interrupt to resume audio
pci device. Then audio pci driver will resume soundwire master and slave,
transfer bus ownership to master, finally slave will report jack event
to master and codec driver is triggered to check jack status.

if a slave has been attached to a bus, the slave->dev_num_sticky
should be non-zero, so we can check this value to skip the
ghost devices defined in ACPI table but not populated in hardware.

Signed-off-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel.c      | 40 +++++++++++++++++++++++++++++++++-
 drivers/soundwire/intel.h      |  1 +
 drivers/soundwire/intel_init.c | 22 +++++++++++++++++++
 3 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 06c553d94890..23b66dcf9966 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -13,6 +13,7 @@
 #include <linux/io.h>
 #include <linux/platform_device.h>
 #include <sound/pcm_params.h>
+#include <linux/pm_runtime.h>
 #include <sound/soc.h>
 #include <linux/soundwire/sdw_registers.h>
 #include <linux/soundwire/sdw.h>
@@ -436,7 +437,7 @@ static int intel_shim_init(struct sdw_intel *sdw, bool clock_stop)
 	return ret;
 }
 
-static void __maybe_unused intel_shim_wake(struct sdw_intel *sdw, bool wake_enable)
+static void intel_shim_wake(struct sdw_intel *sdw, bool wake_enable)
 {
 	void __iomem *shim = sdw->link_res->shim;
 	unsigned int link_id = sdw->instance;
@@ -1337,6 +1338,43 @@ static int intel_master_remove(struct platform_device *pdev)
 	return 0;
 }
 
+int intel_master_process_wakeen_event(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct sdw_intel *sdw;
+	struct sdw_bus *bus;
+	void __iomem *shim;
+	u16 wake_sts;
+
+	sdw = platform_get_drvdata(pdev);
+	bus = &sdw->cdns.bus;
+
+	if (bus->prop.hw_disabled) {
+		dev_dbg(dev, "SoundWire master %d is disabled, ignoring\n", bus->link_id);
+		return 0;
+	}
+
+	shim = sdw->link_res->shim;
+	wake_sts = intel_readw(shim, SDW_SHIM_WAKESTS);
+
+	if (!(wake_sts & BIT(sdw->instance)))
+		return 0;
+
+	/* disable WAKEEN interrupt ASAP to prevent interrupt flood */
+	intel_shim_wake(sdw, false);
+
+	/*
+	 * resume the Master, which will generate a bus reset and result in
+	 * Slaves re-attaching and be re-enumerated. The SoundWire physical
+	 * device which generated the wake will trigger an interrupt, which
+	 * will in turn cause the corresponding Linux Slave device to be
+	 * resumed and the Slave codec driver to check the status.
+	 */
+	pm_request_resume(dev);
+
+	return 0;
+}
+
 static struct platform_driver sdw_intel_drv = {
 	.probe = intel_master_probe,
 	.remove = intel_master_remove,
diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h
index bf127c88eb51..4ea3d262d249 100644
--- a/drivers/soundwire/intel.h
+++ b/drivers/soundwire/intel.h
@@ -47,5 +47,6 @@ struct sdw_intel {
 #define SDW_INTEL_QUIRK_MASK_BUS_DISABLE      BIT(1)
 
 int intel_master_startup(struct platform_device *pdev);
+int intel_master_process_wakeen_event(struct platform_device *pdev);
 
 #endif /* __SDW_INTEL_LOCAL_H */
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index a97d3577eb57..8d3992f9fcdd 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -415,5 +415,27 @@ void sdw_intel_exit(struct sdw_intel_ctx *ctx)
 }
 EXPORT_SYMBOL_NS(sdw_intel_exit, SOUNDWIRE_INTEL_INIT);
 
+void sdw_intel_process_wakeen_event(struct sdw_intel_ctx *ctx)
+{
+	struct sdw_intel_link_res *link;
+	u32 link_mask;
+	int i;
+
+	if (!ctx->links)
+		return;
+
+	link = ctx->links;
+	link_mask = ctx->link_mask;
+
+	/* Startup SDW Master devices */
+	for (i = 0; i < ctx->count; i++, link++) {
+		if (!(link_mask & BIT(i)))
+			continue;
+
+		intel_master_process_wakeen_event(link->pdev);
+	}
+}
+EXPORT_SYMBOL_NS(sdw_intel_process_wakeen_event, SOUNDWIRE_INTEL_INIT);
+
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_DESCRIPTION("Intel Soundwire Init Library");
-- 
2.17.1


^ permalink raw reply related

* [PATCH v4 2/7] kprobes: Use lock_modules() and unlock_modules()
From: Jarkko Sakkinen @ 2020-07-17  3:04 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jarkko Sakkinen, Andi Kleen, Masami Hiramatsu, Naveen N. Rao,
	Anil S Keshavamurthy, David S. Miller, Steven Rostedt,
	Ingo Molnar
In-Reply-To: <20200717030422.679972-1-jarkko.sakkinen@linux.intel.com>

Use lock_modules() and unlock_modules() in order to remove compile time
dependency to the module subsystem.

Cc: Andi Kleen <ak@linux.intel.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 kernel/kprobes.c            | 4 ++--
 kernel/trace/trace_kprobe.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 2e97febeef77..4e46d96d4e16 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -564,7 +564,7 @@ static void kprobe_optimizer(struct work_struct *work)
 	cpus_read_lock();
 	mutex_lock(&text_mutex);
 	/* Lock modules while optimizing kprobes */
-	mutex_lock(&module_mutex);
+	lock_modules();
 
 	/*
 	 * Step 1: Unoptimize kprobes and collect cleaned (unused and disarmed)
@@ -589,7 +589,7 @@ static void kprobe_optimizer(struct work_struct *work)
 	/* Step 4: Free cleaned kprobes after quiesence period */
 	do_free_cleaned_kprobes();
 
-	mutex_unlock(&module_mutex);
+	unlock_modules();
 	mutex_unlock(&text_mutex);
 	cpus_read_unlock();
 
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index aefb6065b508..710ec6a6aa8f 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -122,9 +122,9 @@ static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
 	if (!p)
 		return true;
 	*p = '\0';
-	mutex_lock(&module_mutex);
+	lock_modules();
 	ret = !!find_module(tk->symbol);
-	mutex_unlock(&module_mutex);
+	unlock_modules();
 	*p = ':';
 
 	return ret;
-- 
2.25.1


^ permalink raw reply related

* [PATCH v2 9/9] Soundwire: intel_init: save Slave(s) _ADR info in sdw_intel_ctx
From: Bard Liao @ 2020-07-16 15:09 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
	srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
	pierre-louis.bossart, sanyog.r.kale, slawomir.blauciak,
	mengdong.lin, bard.liao
In-Reply-To: <20200716150947.22119-1-yung-chuan.liao@linux.intel.com>

Save ACPI information in context so that we can match machine driver
with sdw _ADR matching tables.

Suggested-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/intel_init.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index 8d3992f9fcdd..047252a91c9e 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -188,7 +188,11 @@ static struct sdw_intel_ctx
 	struct sdw_intel_link_res *link;
 	struct sdw_intel_ctx *ctx;
 	struct acpi_device *adev;
+	struct sdw_slave *slave;
+	struct list_head *node;
+	struct sdw_bus *bus;
 	u32 link_mask;
+	int num_slaves = 0;
 	int count;
 	int i;
 
@@ -265,6 +269,26 @@ static struct sdw_intel_ctx
 		link->cdns = platform_get_drvdata(pdev);
 
 		list_add_tail(&link->list, &ctx->link_list);
+		bus = &link->cdns->bus;
+		/* Calculate number of slaves */
+		list_for_each(node, &bus->slaves)
+			num_slaves++;
+	}
+
+	ctx->ids = devm_kcalloc(&adev->dev, num_slaves,
+				sizeof(*ctx->ids), GFP_KERNEL);
+	if (!ctx->ids)
+		goto err;
+
+	ctx->num_slaves = num_slaves;
+	i = 0;
+	list_for_each_entry(link, &ctx->link_list, list) {
+		bus = &link->cdns->bus;
+		list_for_each_entry(slave, &bus->slaves, node) {
+			ctx->ids[i].id = slave->id;
+			ctx->ids[i].link_id = bus->link_id;
+			i++;
+		}
 	}
 
 	return ctx;
-- 
2.17.1


^ permalink raw reply related

* [PATCH v4 1/7] module: Add lock_modules() and unlock_modules()
From: Jarkko Sakkinen @ 2020-07-17  3:04 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jarkko Sakkinen, Andi Kleen, Masami Hiramatsu, Jessica Yu
In-Reply-To: <20200717030422.679972-1-jarkko.sakkinen@linux.intel.com>

Add wrapper functions for acquiring module_mutex so that the locking can
be implicitly compiled out when CONFIG_MODULES is not enabled.

Cc: Andi Kleen <ak@linux.intel.com>
Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 include/linux/module.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/include/linux/module.h b/include/linux/module.h
index 2e6670860d27..8850b9692b8f 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -705,6 +705,16 @@ static inline bool is_livepatch_module(struct module *mod)
 bool is_module_sig_enforced(void);
 void set_module_sig_enforced(void);
 
+static inline void lock_modules(void)
+{
+	mutex_lock(&module_mutex);
+}
+
+static inline void unlock_modules(void)
+{
+	mutex_unlock(&module_mutex);
+}
+
 #else /* !CONFIG_MODULES... */
 
 static inline struct module *__module_address(unsigned long addr)
@@ -852,6 +862,14 @@ void *dereference_module_function_descriptor(struct module *mod, void *ptr)
 	return ptr;
 }
 
+static inline void lock_modules(void)
+{
+}
+
+static inline void unlock_modules(void)
+{
+}
+
 #endif /* CONFIG_MODULES */
 
 #ifdef CONFIG_SYSFS
-- 
2.25.1


^ permalink raw reply related

* [PATCH v2 7/9] soundwire: intel/cadence: merge Soundwire interrupt handlers/threads
From: Bard Liao @ 2020-07-16 15:09 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
	srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
	pierre-louis.bossart, sanyog.r.kale, slawomir.blauciak,
	mengdong.lin, bard.liao
In-Reply-To: <20200716150947.22119-1-yung-chuan.liao@linux.intel.com>

The existing code uses one pair of interrupt handler/thread per link
but at the hardware level the interrupt is shared. This works fine for
legacy PCI interrupts, but leads to timeouts in MSI (Message-Signaled
Interrupt) mode, likely due to edges being lost.

This patch unifies interrupt handling for all links. The dedicated
handler is removed since we use a common one for all shared interrupt
sources, and the thread function takes care of dealing with interrupt
sources. This partition follows the model used for the SOF IPC on
HDaudio platforms, where similar timeout issues were noticed and doing
all the interrupt handling/clearing in the thread improved
reliability/stability.

Validation results with 4 links active in parallel show a night-and-day
improvement with no timeouts noticed even during stress tests. Latency
and quality of service are not affected by the change - mostly because
events on a SoundWire link are throttled by the bus frame rate
(typically 8..48kHz).

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/cadence_master.c | 18 ++++++++++--------
 drivers/soundwire/cadence_master.h |  4 ++++
 drivers/soundwire/intel.c          | 15 ---------------
 drivers/soundwire/intel.h          |  4 ++++
 drivers/soundwire/intel_init.c     | 19 +++++++++++++++++++
 5 files changed, 37 insertions(+), 23 deletions(-)

diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index 613dbd415b91..24eafe0aa1c3 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -17,6 +17,7 @@
 #include <linux/soundwire/sdw.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
+#include <linux/workqueue.h>
 #include "bus.h"
 #include "cadence_master.h"
 
@@ -790,7 +791,7 @@ irqreturn_t sdw_cdns_irq(int irq, void *dev_id)
 			     CDNS_MCP_INT_SLAVE_MASK, 0);
 
 		int_status &= ~CDNS_MCP_INT_SLAVE_MASK;
-		ret = IRQ_WAKE_THREAD;
+		schedule_work(&cdns->work);
 	}
 
 	cdns_writel(cdns, CDNS_MCP_INTSTAT, int_status);
@@ -799,13 +800,15 @@ irqreturn_t sdw_cdns_irq(int irq, void *dev_id)
 EXPORT_SYMBOL(sdw_cdns_irq);
 
 /**
- * sdw_cdns_thread() - Cadence irq thread handler
- * @irq: irq number
- * @dev_id: irq context
+ * To update slave status in a work since we will need to handle
+ * other interrupts eg. CDNS_MCP_INT_RX_WL during the update slave
+ * process.
+ * @work: cdns worker thread
  */
-irqreturn_t sdw_cdns_thread(int irq, void *dev_id)
+static void cdns_update_slave_status_work(struct work_struct *work)
 {
-	struct sdw_cdns *cdns = dev_id;
+	struct sdw_cdns *cdns =
+		container_of(work, struct sdw_cdns, work);
 	u32 slave0, slave1;
 
 	dev_dbg_ratelimited(cdns->dev, "Slave status change\n");
@@ -822,9 +825,7 @@ irqreturn_t sdw_cdns_thread(int irq, void *dev_id)
 	cdns_updatel(cdns, CDNS_MCP_INTMASK,
 		     CDNS_MCP_INT_SLAVE_MASK, CDNS_MCP_INT_SLAVE_MASK);
 
-	return IRQ_HANDLED;
 }
-EXPORT_SYMBOL(sdw_cdns_thread);
 
 /*
  * init routines
@@ -1427,6 +1428,7 @@ int sdw_cdns_probe(struct sdw_cdns *cdns)
 	init_completion(&cdns->tx_complete);
 	cdns->bus.port_ops = &cdns_port_ops;
 
+	INIT_WORK(&cdns->work, cdns_update_slave_status_work);
 	return 0;
 }
 EXPORT_SYMBOL(sdw_cdns_probe);
diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h
index b410656f8194..7638858397df 100644
--- a/drivers/soundwire/cadence_master.h
+++ b/drivers/soundwire/cadence_master.h
@@ -129,6 +129,10 @@ struct sdw_cdns {
 
 	bool link_up;
 	unsigned int msg_count;
+
+	struct work_struct work;
+
+	struct list_head list;
 };
 
 #define bus_to_cdns(_bus) container_of(_bus, struct sdw_cdns, bus)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 0a4fc7f65743..06c553d94890 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1258,21 +1258,7 @@ static int intel_master_probe(struct platform_device *pdev)
 			 "SoundWire master %d is disabled, will be ignored\n",
 			 bus->link_id);
 
-	/* Acquire IRQ */
-	ret = request_threaded_irq(sdw->link_res->irq,
-				   sdw_cdns_irq, sdw_cdns_thread,
-				   IRQF_SHARED, KBUILD_MODNAME, cdns);
-	if (ret < 0) {
-		dev_err(dev, "unable to grab IRQ %d, disabling device\n",
-			sdw->link_res->irq);
-		goto err_init;
-	}
-
 	return 0;
-
-err_init:
-	sdw_bus_master_delete(bus);
-	return ret;
 }
 
 int intel_master_startup(struct platform_device *pdev)
@@ -1344,7 +1330,6 @@ static int intel_master_remove(struct platform_device *pdev)
 	if (!bus->prop.hw_disabled) {
 		intel_debugfs_exit(sdw);
 		sdw_cdns_enable_interrupt(cdns, false);
-		free_irq(sdw->link_res->irq, sdw);
 		snd_soc_unregister_component(dev);
 	}
 	sdw_bus_master_delete(bus);
diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h
index d6bdd4d63e08..bf127c88eb51 100644
--- a/drivers/soundwire/intel.h
+++ b/drivers/soundwire/intel.h
@@ -17,6 +17,8 @@
  * @dev: device implementing hw_params and free callbacks
  * @shim_lock: mutex to handle access to shared SHIM registers
  * @shim_mask: global pointer to check SHIM register initialization
+ * @cdns: Cadence master descriptor
+ * @list: used to walk-through all masters exposed by the same controller
  */
 struct sdw_intel_link_res {
 	struct platform_device *pdev;
@@ -29,6 +31,8 @@ struct sdw_intel_link_res {
 	struct device *dev;
 	struct mutex *shim_lock; /* protect shared registers */
 	u32 *shim_mask;
+	struct sdw_cdns *cdns;
+	struct list_head list;
 };
 
 struct sdw_intel {
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index ad3175272e88..a97d3577eb57 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -9,6 +9,7 @@
 
 #include <linux/acpi.h>
 #include <linux/export.h>
+#include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
@@ -166,6 +167,19 @@ void sdw_intel_enable_irq(void __iomem *mmio_base, bool enable)
 }
 EXPORT_SYMBOL_NS(sdw_intel_enable_irq, SOUNDWIRE_INTEL_INIT);
 
+irqreturn_t sdw_intel_thread(int irq, void *dev_id)
+{
+	struct sdw_intel_ctx *ctx = dev_id;
+	struct sdw_intel_link_res *link;
+
+	list_for_each_entry(link, &ctx->link_list, list)
+		sdw_cdns_irq(irq, link->cdns);
+
+	sdw_intel_enable_irq(ctx->mmio_base, true);
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL_NS(sdw_intel_thread, SOUNDWIRE_INTEL_INIT);
+
 static struct sdw_intel_ctx
 *sdw_intel_probe_controller(struct sdw_intel_res *res)
 {
@@ -209,6 +223,8 @@ static struct sdw_intel_ctx
 	link = ctx->links;
 	link_mask = ctx->link_mask;
 
+	INIT_LIST_HEAD(&ctx->link_list);
+
 	/* Create SDW Master devices */
 	for (i = 0; i < count; i++, link++) {
 		if (!(link_mask & BIT(i))) {
@@ -246,6 +262,9 @@ static struct sdw_intel_ctx
 			goto err;
 		}
 		link->pdev = pdev;
+		link->cdns = platform_get_drvdata(pdev);
+
+		list_add_tail(&link->list, &ctx->link_list);
 	}
 
 	return ctx;
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 6/9] soundwire: intel_init: use EXPORT_SYMBOL_NS
From: Bard Liao @ 2020-07-16 15:09 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
	srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
	pierre-louis.bossart, sanyog.r.kale, slawomir.blauciak,
	mengdong.lin, bard.liao
In-Reply-To: <20200716150947.22119-1-yung-chuan.liao@linux.intel.com>

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Make sure all symbols in this soundwire-intel-init module are exported
with a namespace.

The MODULE_IMPORT_NS will be used in Intel/SOF HDaudio modules to be
posted in a separate series.

Namespaces are only introduced for the Intel parts of the SoundWire
code at this time, in future patches we should also add namespaces for
Cadence parts and the SoundWire core.

Suggested-by: Greg KH <gregkh@linuxfoundation.org>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel_init.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index d8f0c1472f1f..ad3175272e88 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -164,7 +164,7 @@ void sdw_intel_enable_irq(void __iomem *mmio_base, bool enable)
 
 	writel(val, mmio_base + HDA_DSP_REG_ADSPIC2);
 }
-EXPORT_SYMBOL(sdw_intel_enable_irq);
+EXPORT_SYMBOL_NS(sdw_intel_enable_irq, SOUNDWIRE_INTEL_INIT);
 
 static struct sdw_intel_ctx
 *sdw_intel_probe_controller(struct sdw_intel_res *res)
@@ -353,7 +353,7 @@ int sdw_intel_acpi_scan(acpi_handle *parent_handle,
 
 	return sdw_intel_scan_controller(info);
 }
-EXPORT_SYMBOL(sdw_intel_acpi_scan);
+EXPORT_SYMBOL_NS(sdw_intel_acpi_scan, SOUNDWIRE_INTEL_INIT);
 
 /**
  * sdw_intel_probe() - SoundWire Intel probe routine
@@ -370,7 +370,7 @@ struct sdw_intel_ctx
 {
 	return sdw_intel_probe_controller(res);
 }
-EXPORT_SYMBOL(sdw_intel_probe);
+EXPORT_SYMBOL_NS(sdw_intel_probe, SOUNDWIRE_INTEL_INIT);
 
 /**
  * sdw_intel_startup() - SoundWire Intel startup
@@ -383,7 +383,7 @@ int sdw_intel_startup(struct sdw_intel_ctx *ctx)
 {
 	return sdw_intel_startup_controller(ctx);
 }
-EXPORT_SYMBOL(sdw_intel_startup);
+EXPORT_SYMBOL_NS(sdw_intel_startup, SOUNDWIRE_INTEL_INIT);
 /**
  * sdw_intel_exit() - SoundWire Intel exit
  * @ctx: SoundWire context allocated in the probe
@@ -394,7 +394,7 @@ void sdw_intel_exit(struct sdw_intel_ctx *ctx)
 {
 	sdw_intel_cleanup(ctx);
 }
-EXPORT_SYMBOL(sdw_intel_exit);
+EXPORT_SYMBOL_NS(sdw_intel_exit, SOUNDWIRE_INTEL_INIT);
 
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_DESCRIPTION("Intel Soundwire Init Library");
-- 
2.17.1


^ permalink raw reply related

* [PATCH v4 0/7] arch/x86: kprobes: Remove MODULES dependency
From: Jarkko Sakkinen @ 2020-07-17  3:04 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jarkko Sakkinen

Remove MODULES dependency by migrating from module_alloc() to the new
text_alloc() API. Essentially these changes provide preliminaries for
allowing to compile a static kernel with a proper tracing support.

The same API can be used later on in other sites that allocate space for
trampolines, and trivially scaled to other arch's. An arch can inform
with CONFIG_ARCH_HAS_TEXT_ALLOC that it's providing implementation for
text_alloc().

I tested this by creating a trivial (x86_64_defconfig) kernel and initrd
(BusyBox) and then run the most basic kprobe:

# ./kprobe p:do_sys_open
Tracing kprobe do_sys_open. Ctrl-C to end.
             cat-1018  [000] ....   277.635966: do_sys_open: (do_sys_open+0x0/0x80)
             cat-1018  [000] ....   277.635966: do_sys_open: (do_sys_open+0x0/0x80)
             cat-1018  [000] ....   277.636966: do_sys_open: (do_sys_open+0x0/0x80)
             cat-1018  [000] ....   277.636966: do_sys_open: (do_sys_open+0x0/0x80)
             cat-1018  [000] ....   277.636966: do_sys_open: (do_sys_open+0x0/0x80)
             cat-1018  [000] ....   277.636966: do_sys_open: (do_sys_open+0x0/0x80)
             cat-1018  [000] ....   277.636966: do_sys_open: (do_sys_open+0x0/0x80)
             cat-1018  [000] ....   277.636966: do_sys_open: (do_sys_open+0x0/0x80)
             cat-1018  [000] ....   277.636966: do_sys_open: (do_sys_open+0x0/0x80)
             cat-1018  [000] ....   277.640966: do_sys_open: (do_sys_open+0x0/0x80)
             cat-1018  [000] ....   277.654963: do_sys_open: (do_sys_open+0x0/0x80)

I did only "sed -i 's/=m/=y/' .config" and disabled CONFIG_MODULES. The
test was run under QEMU:

qemu-system-x86_64 -kernel output/images/bzImage  \
-m 1G -initrd output/images/rootfs.cpio \
-append "root=/dev/sda rw console=ttyS0,115200 acpi=off nokaslr" \
-serial stdio -display none

v3:
* Make text_alloc() API disjoint.
* Remove all the possible extra clutter not absolutely required and
  split into more logical pieces.

Jarkko Sakkinen (7):
  module: Add lock_modules() and unlock_modules()
  kprobes: Use lock_modules() and unlock_modules()
  vmalloc: Add text_alloc() and text_free()
  arch/x86: Implement text_alloc() and text_free()
  arch/x86: kprobes: Use text_alloc() in alloc_insn_page()
  kprobes: Use text_alloc() and text_free()
  kprobes: Flag out CONFIG_MODULES dependent code

 arch/Kconfig                   |  2 +-
 arch/x86/Kconfig               |  3 ++
 arch/x86/kernel/Makefile       |  1 +
 arch/x86/kernel/kprobes/core.c |  8 +----
 arch/x86/kernel/text_alloc.c   | 41 ++++++++++++++++++++++++
 include/linux/module.h         | 32 ++++++++++++++-----
 include/linux/vmalloc.h        | 23 ++++++++++++++
 kernel/kprobes.c               | 57 +++++++++++++++++++++-------------
 kernel/trace/trace_kprobe.c    | 20 +++++++++---
 9 files changed, 146 insertions(+), 41 deletions(-)
 create mode 100644 arch/x86/kernel/text_alloc.c

-- 
2.25.1


^ permalink raw reply

* [PATCH v2 2/9] soundwire: intel: revisit SHIM programming sequences.
From: Bard Liao @ 2020-07-16 15:09 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
	srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
	pierre-louis.bossart, sanyog.r.kale, slawomir.blauciak,
	mengdong.lin, bard.liao
In-Reply-To: <20200716150947.22119-1-yung-chuan.liao@linux.intel.com>

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Somehow the existing code is not aligned with the steps described in
the documentation, refactor code and make sure the register
programming sequences are correct. Also add missing power-up,
power-down and wake capabilities (the last two are used in follow-up
patches but introduced here for consistency).

Some of the SHIM registers exposed fields that are link specific, and
in addition some of the power-related registers (SPA/CPA) take time to
be updated. Uncontrolled access leads to timeouts or errors. Add a
mutex, shared by all links, so that all accesses to such registers are
serialized, and follow a pattern of read-modify-write.

This includes making sure SHIM_SYNC is programmed only once, before
the first master is powered on. We use a 'shim_mask' field, shared
between all links and protected by a mutex, to deal with power-up and
power-down sequences.

Note that the SYNCPRD value is tied only to the XTAL value and not the
current bus frequency or the frame rate.

BugLink: https://github.com/thesofproject/linux/issues/1555
Signed-off-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel.c           | 241 +++++++++++++++++++++++-----
 drivers/soundwire/intel.h           |   4 +
 drivers/soundwire/intel_init.c      |   4 +
 include/linux/soundwire/sdw_intel.h |   2 +
 4 files changed, 215 insertions(+), 36 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 8c7ae07c0fe1..4792613e8e5a 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -46,7 +46,8 @@
 #define SDW_SHIM_LCTL_SPA		BIT(0)
 #define SDW_SHIM_LCTL_CPA		BIT(8)
 
-#define SDW_SHIM_SYNC_SYNCPRD_VAL	0x176F
+#define SDW_SHIM_SYNC_SYNCPRD_VAL_24	(24000 / SDW_CADENCE_GSYNC_KHZ - 1)
+#define SDW_SHIM_SYNC_SYNCPRD_VAL_38_4	(38400 / SDW_CADENCE_GSYNC_KHZ - 1)
 #define SDW_SHIM_SYNC_SYNCPRD		GENMASK(14, 0)
 #define SDW_SHIM_SYNC_SYNCCPU		BIT(15)
 #define SDW_SHIM_SYNC_CMDSYNC_MASK	GENMASK(19, 16)
@@ -272,8 +273,46 @@ static int intel_link_power_up(struct sdw_intel *sdw)
 {
 	unsigned int link_id = sdw->instance;
 	void __iomem *shim = sdw->link_res->shim;
+	u32 *shim_mask = sdw->link_res->shim_mask;
+	struct sdw_bus *bus = &sdw->cdns.bus;
+	struct sdw_master_prop *prop = &bus->prop;
 	int spa_mask, cpa_mask;
-	int link_control, ret;
+	int link_control;
+	int ret = 0;
+	u32 syncprd;
+	u32 sync_reg;
+
+	mutex_lock(sdw->link_res->shim_lock);
+
+	/*
+	 * The hardware relies on an internal counter, typically 4kHz,
+	 * to generate the SoundWire SSP - which defines a 'safe'
+	 * synchronization point between commands and audio transport
+	 * and allows for multi link synchronization. The SYNCPRD value
+	 * is only dependent on the oscillator clock provided to
+	 * the IP, so adjust based on _DSD properties reported in DSDT
+	 * tables. The values reported are based on either 24MHz
+	 * (CNL/CML) or 38.4 MHz (ICL/TGL+).
+	 */
+	if (prop->mclk_freq % 6000000)
+		syncprd = SDW_SHIM_SYNC_SYNCPRD_VAL_38_4;
+	else
+		syncprd = SDW_SHIM_SYNC_SYNCPRD_VAL_24;
+
+	if (!*shim_mask) {
+		/* we first need to program the SyncPRD/CPU registers */
+		dev_dbg(sdw->cdns.dev,
+			"%s: first link up, programming SYNCPRD\n", __func__);
+
+		/* set SyncPRD period */
+		sync_reg = intel_readl(shim, SDW_SHIM_SYNC);
+		sync_reg |= (syncprd <<
+			     SDW_REG_SHIFT(SDW_SHIM_SYNC_SYNCPRD));
+
+		/* Set SyncCPU bit */
+		sync_reg |= SDW_SHIM_SYNC_SYNCCPU;
+		intel_writel(shim, SDW_SHIM_SYNC, sync_reg);
+	}
 
 	/* Link power up sequence */
 	link_control = intel_readl(shim, SDW_SHIM_LCTL);
@@ -282,70 +321,182 @@ static int intel_link_power_up(struct sdw_intel *sdw)
 	link_control |=  spa_mask;
 
 	ret = intel_set_bit(shim, SDW_SHIM_LCTL, link_control, cpa_mask);
-	if (ret < 0)
-		return ret;
+	if (ret < 0) {
+		dev_err(sdw->cdns.dev, "Failed to power up link: %d\n", ret);
+		goto out;
+	}
+
+	if (!*shim_mask) {
+		/* SyncCPU will change once link is active */
+		ret = intel_wait_bit(shim, SDW_SHIM_SYNC,
+				     SDW_SHIM_SYNC_SYNCCPU, 0);
+		if (ret < 0) {
+			dev_err(sdw->cdns.dev,
+				"Failed to set SHIM_SYNC: %d\n", ret);
+			goto out;
+		}
+	}
+
+	*shim_mask |= BIT(link_id);
 
 	sdw->cdns.link_up = true;
-	return 0;
+out:
+	mutex_unlock(sdw->link_res->shim_lock);
+
+	return ret;
 }
 
-static int intel_shim_init(struct sdw_intel *sdw)
+/* this needs to be called with shim_lock */
+static void intel_shim_glue_to_master_ip(struct sdw_intel *sdw)
 {
 	void __iomem *shim = sdw->link_res->shim;
 	unsigned int link_id = sdw->instance;
-	int sync_reg, ret;
-	u16 ioctl = 0, act = 0;
-
-	/* Initialize Shim */
-	ioctl |= SDW_SHIM_IOCTL_BKE;
-	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
-
-	ioctl |= SDW_SHIM_IOCTL_WPDD;
-	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
-
-	ioctl |= SDW_SHIM_IOCTL_DO;
-	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
-
-	ioctl |= SDW_SHIM_IOCTL_DOE;
-	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	u16 ioctl;
 
 	/* Switch to MIP from Glue logic */
 	ioctl = intel_readw(shim,  SDW_SHIM_IOCTL(link_id));
 
 	ioctl &= ~(SDW_SHIM_IOCTL_DOE);
 	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	usleep_range(10, 15);
 
 	ioctl &= ~(SDW_SHIM_IOCTL_DO);
 	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	usleep_range(10, 15);
 
 	ioctl |= (SDW_SHIM_IOCTL_MIF);
 	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	usleep_range(10, 15);
 
 	ioctl &= ~(SDW_SHIM_IOCTL_BKE);
 	ioctl &= ~(SDW_SHIM_IOCTL_COE);
+	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	usleep_range(10, 15);
+
+	/* at this point Master IP has full control of the I/Os */
+}
+
+/* this needs to be called with shim_lock */
+static void intel_shim_master_ip_to_glue(struct sdw_intel *sdw)
+{
+	unsigned int link_id = sdw->instance;
+	void __iomem *shim = sdw->link_res->shim;
+	u16 ioctl;
+
+	/* Glue logic */
+	ioctl = intel_readw(shim, SDW_SHIM_IOCTL(link_id));
+	ioctl |= SDW_SHIM_IOCTL_BKE;
+	ioctl |= SDW_SHIM_IOCTL_COE;
+	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	usleep_range(10, 15);
 
+	ioctl &= ~(SDW_SHIM_IOCTL_MIF);
 	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	usleep_range(10, 15);
+
+	/* at this point Integration Glue has full control of the I/Os */
+}
+
+static int intel_shim_init(struct sdw_intel *sdw, bool clock_stop)
+{
+	void __iomem *shim = sdw->link_res->shim;
+	unsigned int link_id = sdw->instance;
+	int ret = 0;
+	u16 ioctl = 0, act = 0;
+
+	mutex_lock(sdw->link_res->shim_lock);
+
+	/* Initialize Shim */
+	ioctl |= SDW_SHIM_IOCTL_BKE;
+	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	usleep_range(10, 15);
+
+	ioctl |= SDW_SHIM_IOCTL_WPDD;
+	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	usleep_range(10, 15);
+
+	ioctl |= SDW_SHIM_IOCTL_DO;
+	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	usleep_range(10, 15);
+
+	ioctl |= SDW_SHIM_IOCTL_DOE;
+	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	usleep_range(10, 15);
+
+	intel_shim_glue_to_master_ip(sdw);
 
 	act |= 0x1 << SDW_REG_SHIFT(SDW_SHIM_CTMCTL_DOAIS);
 	act |= SDW_SHIM_CTMCTL_DACTQE;
 	act |= SDW_SHIM_CTMCTL_DODS;
 	intel_writew(shim, SDW_SHIM_CTMCTL(link_id), act);
+	usleep_range(10, 15);
 
-	/* Now set SyncPRD period */
-	sync_reg = intel_readl(shim, SDW_SHIM_SYNC);
-	sync_reg |= (SDW_SHIM_SYNC_SYNCPRD_VAL <<
-			SDW_REG_SHIFT(SDW_SHIM_SYNC_SYNCPRD));
-
-	/* Set SyncCPU bit */
-	sync_reg |= SDW_SHIM_SYNC_SYNCCPU;
-	ret = intel_clear_bit(shim, SDW_SHIM_SYNC, sync_reg,
-			      SDW_SHIM_SYNC_SYNCCPU);
-	if (ret < 0)
-		dev_err(sdw->cdns.dev, "Failed to set sync period: %d\n", ret);
+	mutex_unlock(sdw->link_res->shim_lock);
 
 	return ret;
 }
 
+static void __maybe_unused intel_shim_wake(struct sdw_intel *sdw, bool wake_enable)
+{
+	void __iomem *shim = sdw->link_res->shim;
+	unsigned int link_id = sdw->instance;
+	u16 wake_en, wake_sts;
+
+	mutex_lock(sdw->link_res->shim_lock);
+	wake_en = intel_readw(shim, SDW_SHIM_WAKEEN);
+
+	if (wake_enable) {
+		/* Enable the wakeup */
+		wake_en |= (SDW_SHIM_WAKEEN_ENABLE << link_id);
+		intel_writew(shim, SDW_SHIM_WAKEEN, wake_en);
+	} else {
+		/* Disable the wake up interrupt */
+		wake_en &= ~(SDW_SHIM_WAKEEN_ENABLE << link_id);
+		intel_writew(shim, SDW_SHIM_WAKEEN, wake_en);
+
+		/* Clear wake status */
+		wake_sts = intel_readw(shim, SDW_SHIM_WAKESTS);
+		wake_sts |= (SDW_SHIM_WAKEEN_ENABLE << link_id);
+		intel_writew(shim, SDW_SHIM_WAKESTS_STATUS, wake_sts);
+	}
+	mutex_unlock(sdw->link_res->shim_lock);
+}
+
+static int __maybe_unused intel_link_power_down(struct sdw_intel *sdw)
+{
+	int link_control, spa_mask, cpa_mask;
+	unsigned int link_id = sdw->instance;
+	void __iomem *shim = sdw->link_res->shim;
+	u32 *shim_mask = sdw->link_res->shim_mask;
+	int ret = 0;
+
+	mutex_lock(sdw->link_res->shim_lock);
+
+	intel_shim_master_ip_to_glue(sdw);
+
+	/* Link power down sequence */
+	link_control = intel_readl(shim, SDW_SHIM_LCTL);
+	spa_mask = ~(SDW_SHIM_LCTL_SPA << link_id);
+	cpa_mask = (SDW_SHIM_LCTL_CPA << link_id);
+	link_control &=  spa_mask;
+
+	ret = intel_clear_bit(shim, SDW_SHIM_LCTL, link_control, cpa_mask);
+
+	if (!(*shim_mask & BIT(link_id)))
+		dev_err(sdw->cdns.dev,
+			"%s: Unbalanced power-up/down calls\n", __func__);
+
+	*shim_mask &= ~BIT(link_id);
+
+	mutex_unlock(sdw->link_res->shim_lock);
+
+	if (ret < 0)
+		return ret;
+
+	sdw->cdns.link_up = false;
+	return 0;
+}
+
 /*
  * PDI routines
  */
@@ -566,11 +717,15 @@ static int intel_pre_bank_switch(struct sdw_bus *bus)
 	if (!bus->multi_link)
 		return 0;
 
+	mutex_lock(sdw->link_res->shim_lock);
+
 	/* Read SYNC register */
 	sync_reg = intel_readl(shim, SDW_SHIM_SYNC);
 	sync_reg |= SDW_SHIM_SYNC_CMDSYNC << sdw->instance;
 	intel_writel(shim, SDW_SHIM_SYNC, sync_reg);
 
+	mutex_unlock(sdw->link_res->shim_lock);
+
 	return 0;
 }
 
@@ -585,6 +740,8 @@ static int intel_post_bank_switch(struct sdw_bus *bus)
 	if (!bus->multi_link)
 		return 0;
 
+	mutex_lock(sdw->link_res->shim_lock);
+
 	/* Read SYNC register */
 	sync_reg = intel_readl(shim, SDW_SHIM_SYNC);
 
@@ -596,9 +753,10 @@ static int intel_post_bank_switch(struct sdw_bus *bus)
 	 *
 	 * So, set the SYNCGO bit only if CMDSYNC bit is set for any Master.
 	 */
-	if (!(sync_reg & SDW_SHIM_SYNC_CMDSYNC_MASK))
-		return 0;
-
+	if (!(sync_reg & SDW_SHIM_SYNC_CMDSYNC_MASK)) {
+		ret = 0;
+		goto unlock;
+	}
 	/*
 	 * Set SyncGO bit to synchronously trigger a bank switch for
 	 * all the masters. A write to SYNCGO bit clears CMDSYNC bit for all
@@ -608,6 +766,9 @@ static int intel_post_bank_switch(struct sdw_bus *bus)
 
 	ret = intel_clear_bit(shim, SDW_SHIM_SYNC, sync_reg,
 			      SDW_SHIM_SYNC_SYNCGO);
+unlock:
+	mutex_unlock(sdw->link_res->shim_lock);
+
 	if (ret < 0)
 		dev_err(sdw->cdns.dev, "Post bank switch failed: %d\n", ret);
 
@@ -1011,9 +1172,17 @@ static struct sdw_master_ops sdw_intel_ops = {
 
 static int intel_init(struct sdw_intel *sdw)
 {
+	bool clock_stop;
+
 	/* Initialize shim and controller */
 	intel_link_power_up(sdw);
-	intel_shim_init(sdw);
+
+	clock_stop = sdw_cdns_is_clock_stop(&sdw->cdns);
+
+	intel_shim_init(sdw, clock_stop);
+
+	if (clock_stop)
+		return 0;
 
 	return sdw_cdns_init(&sdw->cdns);
 }
diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h
index 694117370ac3..d6bdd4d63e08 100644
--- a/drivers/soundwire/intel.h
+++ b/drivers/soundwire/intel.h
@@ -15,6 +15,8 @@
  * @irq: Interrupt line
  * @ops: Shim callback ops
  * @dev: device implementing hw_params and free callbacks
+ * @shim_lock: mutex to handle access to shared SHIM registers
+ * @shim_mask: global pointer to check SHIM register initialization
  */
 struct sdw_intel_link_res {
 	struct platform_device *pdev;
@@ -25,6 +27,8 @@ struct sdw_intel_link_res {
 	int irq;
 	const struct sdw_intel_ops *ops;
 	struct device *dev;
+	struct mutex *shim_lock; /* protect shared registers */
+	u32 *shim_mask;
 };
 
 struct sdw_intel {
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index 3f2e884b4f6d..f50a93130d12 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -180,6 +180,7 @@ static struct sdw_intel_ctx
 	ctx->mmio_base = res->mmio_base;
 	ctx->link_mask = res->link_mask;
 	ctx->handle = res->handle;
+	mutex_init(&ctx->shim_lock);
 
 	link = ctx->links;
 	link_mask = ctx->link_mask;
@@ -201,6 +202,9 @@ static struct sdw_intel_ctx
 		link->ops = res->ops;
 		link->dev = res->dev;
 
+		link->shim_lock = &ctx->shim_lock;
+		link->shim_mask = &ctx->shim_mask;
+
 		memset(&pdevinfo, 0, sizeof(pdevinfo));
 
 		pdevinfo.parent = res->parent;
diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h
index 979b41b5dcb4..120ffddc03d2 100644
--- a/include/linux/soundwire/sdw_intel.h
+++ b/include/linux/soundwire/sdw_intel.h
@@ -115,6 +115,7 @@ struct sdw_intel_slave_id {
  * links
  * @link_list: list to handle interrupts across all links
  * @shim_lock: mutex to handle concurrent rmw access to shared SHIM registers.
+ * @shim_mask: flags to track initialization of SHIM shared registers
  */
 struct sdw_intel_ctx {
 	int count;
@@ -126,6 +127,7 @@ struct sdw_intel_ctx {
 	struct sdw_intel_slave_id *ids;
 	struct list_head link_list;
 	struct mutex shim_lock; /* lock for access to shared SHIM registers */
+	u32 shim_mask;
 };
 
 /**
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 5/9] soundwire: intel_init: add implementation of sdw_intel_enable_irq()
From: Bard Liao @ 2020-07-16 15:09 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
	srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
	pierre-louis.bossart, sanyog.r.kale, slawomir.blauciak,
	mengdong.lin, bard.liao
In-Reply-To: <20200716150947.22119-1-yung-chuan.liao@linux.intel.com>

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

This function is required to enable all interrupts across all links.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel_init.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index f50a93130d12..d8f0c1472f1f 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -142,6 +142,30 @@ sdw_intel_scan_controller(struct sdw_intel_acpi_info *info)
 	return 0;
 }
 
+#define HDA_DSP_REG_ADSPIC2             (0x10)
+#define HDA_DSP_REG_ADSPIS2             (0x14)
+#define HDA_DSP_REG_ADSPIC2_SNDW        BIT(5)
+
+/**
+ * sdw_intel_enable_irq() - enable/disable Intel SoundWire IRQ
+ * @mmio_base: The mmio base of the control register
+ * @enable: true if enable
+ */
+void sdw_intel_enable_irq(void __iomem *mmio_base, bool enable)
+{
+	u32 val;
+
+	val = readl(mmio_base + HDA_DSP_REG_ADSPIC2);
+
+	if (enable)
+		val |= HDA_DSP_REG_ADSPIC2_SNDW;
+	else
+		val &= ~HDA_DSP_REG_ADSPIC2_SNDW;
+
+	writel(val, mmio_base + HDA_DSP_REG_ADSPIC2);
+}
+EXPORT_SYMBOL(sdw_intel_enable_irq);
+
 static struct sdw_intel_ctx
 *sdw_intel_probe_controller(struct sdw_intel_res *res)
 {
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 4/9] soundwire: intel: introduce helper for link synchronization
From: Bard Liao @ 2020-07-16 15:09 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
	srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
	pierre-louis.bossart, sanyog.r.kale, slawomir.blauciak,
	mengdong.lin, bard.liao
In-Reply-To: <20200716150947.22119-1-yung-chuan.liao@linux.intel.com>

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

After arming the synchronization, the SYNCGO field controls the
hardware-based synchronization between links.

Move the programming and wait for clear of SYNCGO to dedicated helper.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel.c | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 6a745602c9cc..0a4fc7f65743 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -512,6 +512,31 @@ static void intel_shim_sync_arm(struct sdw_intel *sdw)
 	mutex_unlock(sdw->link_res->shim_lock);
 }
 
+static int intel_shim_sync_go_unlocked(struct sdw_intel *sdw)
+{
+	void __iomem *shim = sdw->link_res->shim;
+	u32 sync_reg;
+	int ret;
+
+	/* Read SYNC register */
+	sync_reg = intel_readl(shim, SDW_SHIM_SYNC);
+
+	/*
+	 * Set SyncGO bit to synchronously trigger a bank switch for
+	 * all the masters. A write to SYNCGO bit clears CMDSYNC bit for all
+	 * the Masters.
+	 */
+	sync_reg |= SDW_SHIM_SYNC_SYNCGO;
+
+	ret = intel_clear_bit(shim, SDW_SHIM_SYNC, sync_reg,
+			      SDW_SHIM_SYNC_SYNCGO);
+
+	if (ret < 0)
+		dev_err(sdw->cdns.dev, "SyncGO clear failed: %d\n", ret);
+
+	return ret;
+}
+
 /*
  * PDI routines
  */
@@ -763,15 +788,8 @@ static int intel_post_bank_switch(struct sdw_bus *bus)
 		ret = 0;
 		goto unlock;
 	}
-	/*
-	 * Set SyncGO bit to synchronously trigger a bank switch for
-	 * all the masters. A write to SYNCGO bit clears CMDSYNC bit for all
-	 * the Masters.
-	 */
-	sync_reg |= SDW_SHIM_SYNC_SYNCGO;
 
-	ret = intel_clear_bit(shim, SDW_SHIM_SYNC, sync_reg,
-			      SDW_SHIM_SYNC_SYNCGO);
+	ret = intel_shim_sync_go_unlocked(sdw);
 unlock:
 	mutex_unlock(sdw->link_res->shim_lock);
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 3/9] soundwire: intel: introduce a helper to arm link synchronization
From: Bard Liao @ 2020-07-16 15:09 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
	srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
	pierre-louis.bossart, sanyog.r.kale, slawomir.blauciak,
	mengdong.lin, bard.liao
In-Reply-To: <20200716150947.22119-1-yung-chuan.liao@linux.intel.com>

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Move code from pre_bank_switch to dedicated helper, will be used in
follow-up patches as recommended by programming flows.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 4792613e8e5a..6a745602c9cc 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -497,6 +497,21 @@ static int __maybe_unused intel_link_power_down(struct sdw_intel *sdw)
 	return 0;
 }
 
+static void intel_shim_sync_arm(struct sdw_intel *sdw)
+{
+	void __iomem *shim = sdw->link_res->shim;
+	u32 sync_reg;
+
+	mutex_lock(sdw->link_res->shim_lock);
+
+	/* update SYNC register */
+	sync_reg = intel_readl(shim, SDW_SHIM_SYNC);
+	sync_reg |= (SDW_SHIM_SYNC_CMDSYNC << sdw->instance);
+	intel_writel(shim, SDW_SHIM_SYNC, sync_reg);
+
+	mutex_unlock(sdw->link_res->shim_lock);
+}
+
 /*
  * PDI routines
  */
@@ -710,21 +725,12 @@ static int intel_pre_bank_switch(struct sdw_bus *bus)
 {
 	struct sdw_cdns *cdns = bus_to_cdns(bus);
 	struct sdw_intel *sdw = cdns_to_intel(cdns);
-	void __iomem *shim = sdw->link_res->shim;
-	int sync_reg;
 
 	/* Write to register only for multi-link */
 	if (!bus->multi_link)
 		return 0;
 
-	mutex_lock(sdw->link_res->shim_lock);
-
-	/* Read SYNC register */
-	sync_reg = intel_readl(shim, SDW_SHIM_SYNC);
-	sync_reg |= SDW_SHIM_SYNC_CMDSYNC << sdw->instance;
-	intel_writel(shim, SDW_SHIM_SYNC, sync_reg);
-
-	mutex_unlock(sdw->link_res->shim_lock);
+	intel_shim_sync_arm(sdw);
 
 	return 0;
 }
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 1/9] soundwire: intel: reuse code for wait loops to set/clear bits
From: Bard Liao @ 2020-07-16 15:09 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
	srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
	pierre-louis.bossart, sanyog.r.kale, slawomir.blauciak,
	mengdong.lin, bard.liao
In-Reply-To: <20200716150947.22119-1-yung-chuan.liao@linux.intel.com>

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Refactor code and use same routines on set/clear

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel.c | 45 +++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 7a65414e5714..8c7ae07c0fe1 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -123,40 +123,33 @@ static inline void intel_writew(void __iomem *base, int offset, u16 value)
 	writew(value, base + offset);
 }
 
+static int intel_wait_bit(void __iomem *base, int offset, u32 mask, u32 target)
+{
+	int timeout = 10;
+	u32 reg_read;
+
+	do {
+		reg_read = readl(base + offset);
+		if ((reg_read & mask) == target)
+			return 0;
+
+		timeout--;
+		usleep_range(50, 100);
+	} while (timeout != 0);
+
+	return -EAGAIN;
+}
+
 static int intel_clear_bit(void __iomem *base, int offset, u32 value, u32 mask)
 {
-	int timeout = 10;
-	u32 reg_read;
-
 	writel(value, base + offset);
-	do {
-		reg_read = readl(base + offset);
-		if (!(reg_read & mask))
-			return 0;
-
-		timeout--;
-		udelay(50);
-	} while (timeout != 0);
-
-	return -EAGAIN;
+	return intel_wait_bit(base, offset, mask, 0);
 }
 
 static int intel_set_bit(void __iomem *base, int offset, u32 value, u32 mask)
 {
-	int timeout = 10;
-	u32 reg_read;
-
 	writel(value, base + offset);
-	do {
-		reg_read = readl(base + offset);
-		if (reg_read & mask)
-			return 0;
-
-		timeout--;
-		udelay(50);
-	} while (timeout != 0);
-
-	return -EAGAIN;
+	return intel_wait_bit(base, offset, mask, mask);
 }
 
 /*
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 0/9] soundwire: intel: revisit SHIM programming
From: Bard Liao @ 2020-07-16 15:09 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
	srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
	pierre-louis.bossart, sanyog.r.kale, slawomir.blauciak,
	mengdong.lin, bard.liao

This series does some cleanup, revisits SHIM programming sequences,
and merges Soundwire interrupt handlers/threads.

changes in v2:
 - Resume M device instead of S device in intel_master_process_wakeen_event
   function. See the comment in intel_master_process_wakeen_event() in
   detail.

Bard Liao (2):
  soundwire: intel/cadence: merge Soundwire interrupt handlers/threads
  Soundwire: intel_init: save Slave(s) _ADR info in sdw_intel_ctx

Pierre-Louis Bossart (6):
  soundwire: intel: reuse code for wait loops to set/clear bits
  soundwire: intel: revisit SHIM programming sequences.
  soundwire: intel: introduce a helper to arm link synchronization
  soundwire: intel: introduce helper for link synchronization
  soundwire: intel_init: add implementation of sdw_intel_enable_irq()
  soundwire: intel_init: use EXPORT_SYMBOL_NS

Rander Wang (1):
  soundwire: intel: add wake interrupt support

 drivers/soundwire/cadence_master.c  |  18 +-
 drivers/soundwire/cadence_master.h  |   4 +
 drivers/soundwire/intel.c           | 381 +++++++++++++++++++++-------
 drivers/soundwire/intel.h           |   9 +
 drivers/soundwire/intel_init.c      | 101 +++++++-
 include/linux/soundwire/sdw_intel.h |   2 +
 6 files changed, 417 insertions(+), 98 deletions(-)

-- 
2.17.1


^ permalink raw reply

* [PATCH] drm/amdgpu: load asd for sienna cichlid
From: Clements, John @ 2020-07-17  3:03 UTC (permalink / raw)
  To: amd-gfx list, Zhang, Hawking


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

[AMD Public Use]

Submitting patch to enable ASD loading for Sienna Cichlid

[-- Attachment #1.2: Type: text/html, Size: 1800 bytes --]

[-- Attachment #2: 0001-drm-amdgpu-load-asd-for-sienna-cichlid.patch --]
[-- Type: application/octet-stream, Size: 974 bytes --]

From 32c8d6018fa48a36b68bc6ebea090819395ad3ef Mon Sep 17 00:00:00 2001
From: John Clements <john.clements@amd.com>
Date: Fri, 17 Jul 2020 10:56:53 +0800
Subject: [PATCH] drm/amdgpu: load asd for sienna cichlid

do not abort psp asd load sequence for sienna cichlid

Signed-off-by: John Clements <john.clements@amd.com>
Change-Id: Ic6945439526249b44e6c02f9fc519a296862dd97
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index aa80cf799e42..8034111acd9a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -500,7 +500,6 @@ static int psp_asd_load(struct psp_context *psp)
 	 * TODO: add version check to make it common
 	 */
 	if (amdgpu_sriov_vf(psp->adev) ||
-	    (psp->adev->asic_type == CHIP_SIENNA_CICHLID) ||
 	    (psp->adev->asic_type == CHIP_NAVY_FLOUNDER))
 		return 0;
 
-- 
2.17.1


[-- Attachment #3: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related

* linux-next: build failure after merge of the sound tree
From: Stephen Rothwell @ 2020-07-17  3:01 UTC (permalink / raw)
  To: Takashi Iwai, Christoph Hellwig
  Cc: Linux Next Mailing List, Linux Kernel Mailing List

[-- Attachment #1: Type: text/plain, Size: 1720 bytes --]

Hi all,

After merging the sound tree, today's linux-next build (x86_64
allmodconfig) failed like this:

sound/core/pcm_memory.c: In function 'do_alloc_pages':
sound/core/pcm_memory.c:47:7: error: implicit declaration of function 'dma_is_direct'; did you mean 'd_is_dir'? [-Werror=implicit-function-declaration]
   47 |      !dma_is_direct(get_dma_ops(dev))) {
      |       ^~~~~~~~~~~~~
      |       d_is_dir

Caused by commit

  3ad796cbc36a ("ALSA: pcm: Use SG-buffer only when direct DMA is available")

interacting with commit

  d3fa60d7bfdc ("dma-mapping: move the remaining DMA API calls out of line")

from the dma-mapping tree.

I am not sure of the best fix here, so I just added this hack to make
it build for now.  Suggestions welcome.

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Fri, 17 Jul 2020 12:57:38 +1000
Subject: [PATCH] merge fix for dma_is_direct() removal

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 sound/core/pcm_memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/core/pcm_memory.c b/sound/core/pcm_memory.c
index 795af1b88051..89480f12086a 100644
--- a/sound/core/pcm_memory.c
+++ b/sound/core/pcm_memory.c
@@ -44,7 +44,7 @@ static int do_alloc_pages(struct snd_card *card, int type, struct device *dev,
 
 #ifdef CONFIG_SND_DMA_SGBUF
 	if ((type == SNDRV_DMA_TYPE_DEV_SG || type == SNDRV_DMA_TYPE_DEV_UC_SG) &&
-	    !dma_is_direct(get_dma_ops(dev))) {
+	    1 /* !dma_is_direct(get_dma_ops(dev)) */) {
 		/* mutate to continuous page allocation */
 		dev_dbg(dev, "Use continuous page allocator\n");
 		if (type == SNDRV_DMA_TYPE_DEV_SG)
-- 
2.27.0

-- 
Cheers,
Stephen Rothwell

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

^ permalink raw reply related

* Re: [PATCH 1/1] irqchip: Add config MTK_SYSIRQ and MTK_CIRQ
From: Saravana Kannan @ 2020-07-17  2:55 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: CC Hwang, Jason Cooper, wsd_upstream, Hanks Chen, Loda Chou, LKML,
	moderated list:ARM/Mediatek SoC support, Matthias Brugger,
	Thomas Gleixner, linux-arm-kernel
In-Reply-To: <3dc953265ed4cd4300bd9600bf7e33d6@kernel.org>

On Tue, Jun 23, 2020 at 1:37 AM Marc Zyngier <maz@kernel.org> wrote:
>
> + Saravana
>
> On 2020-06-23 07:36, Hanks Chen wrote:
>
> [...]
>
> > Hi Marc,
> >
> > I want to break the dependency between ARCH_MEDIATEK and CIRQ/SYSIRQ,
> > because we need to follow the GKI.
>
> This isn't what GKI mandates. GKI requires that the SoC code is compiled
> as modules, not that it is dissociated from its platform (which would
> be completely pointless).
>
> > Could I add the MTK_CIRQ and MTK_SYSIRQ into defconfig?
> > (arch/arm64/config/defconfig)
>
> I don't deal with defconfig at all. That's a patch for the arm-soc
> people.
>
> >
> > It would ensures all platform are executable.
> >
> > e.g.
> > --- a/arch/arm64/configs/defconfig
> > +++ b/arch/arm64/configs/defconfig
> >
> >  CONFIG_QCOM_PDC=y
> > +CONFIG_MTK_SYSIRQ=m
> > +CONFIG_MTK_CIRQ=y
> >  CONFIG_RESET_QCOM_AOSS=y
> >
> > --- a/drivers/irqchip/Kconfig
> > +++ b/drivers/irqchip/Kconfig
> > @@ -572,4 +572,18 @@ config LOONGSON_PCH_MSI
> >         help
> >           Support for the Loongson PCH MSI Controller.
> >
> > +config MTK_SYSIRQ
> > +       tristate "Mediatek interrupt polarity controller"
> > +       depends on ARCH_MEDIATEK || COMPILE_TEST
> > +       help
> > +         Interrupt polarity controller driver to swap polarity for
> > +         interrupts for Mediatek mobile chips.
> > +
> > +config MTK_CIRQ
> > +       bool "Mediatek low-power interrupt controller"
> > +       depends on ARCH_MEDIATEK || COMPILE_TEST
> > +       help
> > +         Low-power interrupt controller driver to monitor IRQS
> > +         in the sleep mode for Mediatek mobile chips.
> > +
> >
> >
> > P.S I'll make the irq-mtk-sysirq driver as a loadable kernel module for
> > GKI
>
> You might as well turn both drivers into modules. Saravana was working
> on a set of patches to ease this transition.

Sent out v2 of that patch now.
https://lore.kernel.org/lkml/20200717024447.3128361-1-saravanak@google.com/T/#u

-Saravana

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 1/1] irqchip: Add config MTK_SYSIRQ and MTK_CIRQ
From: Saravana Kannan @ 2020-07-17  2:55 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: CC Hwang, Jason Cooper, wsd_upstream, Hanks Chen, Loda Chou, LKML,
	moderated list:ARM/Mediatek SoC support, Matthias Brugger,
	Thomas Gleixner, linux-arm-kernel
In-Reply-To: <3dc953265ed4cd4300bd9600bf7e33d6@kernel.org>

On Tue, Jun 23, 2020 at 1:37 AM Marc Zyngier <maz@kernel.org> wrote:
>
> + Saravana
>
> On 2020-06-23 07:36, Hanks Chen wrote:
>
> [...]
>
> > Hi Marc,
> >
> > I want to break the dependency between ARCH_MEDIATEK and CIRQ/SYSIRQ,
> > because we need to follow the GKI.
>
> This isn't what GKI mandates. GKI requires that the SoC code is compiled
> as modules, not that it is dissociated from its platform (which would
> be completely pointless).
>
> > Could I add the MTK_CIRQ and MTK_SYSIRQ into defconfig?
> > (arch/arm64/config/defconfig)
>
> I don't deal with defconfig at all. That's a patch for the arm-soc
> people.
>
> >
> > It would ensures all platform are executable.
> >
> > e.g.
> > --- a/arch/arm64/configs/defconfig
> > +++ b/arch/arm64/configs/defconfig
> >
> >  CONFIG_QCOM_PDC=y
> > +CONFIG_MTK_SYSIRQ=m
> > +CONFIG_MTK_CIRQ=y
> >  CONFIG_RESET_QCOM_AOSS=y
> >
> > --- a/drivers/irqchip/Kconfig
> > +++ b/drivers/irqchip/Kconfig
> > @@ -572,4 +572,18 @@ config LOONGSON_PCH_MSI
> >         help
> >           Support for the Loongson PCH MSI Controller.
> >
> > +config MTK_SYSIRQ
> > +       tristate "Mediatek interrupt polarity controller"
> > +       depends on ARCH_MEDIATEK || COMPILE_TEST
> > +       help
> > +         Interrupt polarity controller driver to swap polarity for
> > +         interrupts for Mediatek mobile chips.
> > +
> > +config MTK_CIRQ
> > +       bool "Mediatek low-power interrupt controller"
> > +       depends on ARCH_MEDIATEK || COMPILE_TEST
> > +       help
> > +         Low-power interrupt controller driver to monitor IRQS
> > +         in the sleep mode for Mediatek mobile chips.
> > +
> >
> >
> > P.S I'll make the irq-mtk-sysirq driver as a loadable kernel module for
> > GKI
>
> You might as well turn both drivers into modules. Saravana was working
> on a set of patches to ease this transition.

Sent out v2 of that patch now.
https://lore.kernel.org/lkml/20200717024447.3128361-1-saravanak@google.com/T/#u

-Saravana

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply

* Re: [PATCH 1/1] irqchip: Add config MTK_SYSIRQ and MTK_CIRQ
From: Saravana Kannan @ 2020-07-17  2:55 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Hanks Chen, CC Hwang, Jason Cooper, wsd_upstream, Loda Chou, LKML,
	moderated list:ARM/Mediatek SoC support, Matthias Brugger,
	Thomas Gleixner, linux-arm-kernel
In-Reply-To: <3dc953265ed4cd4300bd9600bf7e33d6@kernel.org>

On Tue, Jun 23, 2020 at 1:37 AM Marc Zyngier <maz@kernel.org> wrote:
>
> + Saravana
>
> On 2020-06-23 07:36, Hanks Chen wrote:
>
> [...]
>
> > Hi Marc,
> >
> > I want to break the dependency between ARCH_MEDIATEK and CIRQ/SYSIRQ,
> > because we need to follow the GKI.
>
> This isn't what GKI mandates. GKI requires that the SoC code is compiled
> as modules, not that it is dissociated from its platform (which would
> be completely pointless).
>
> > Could I add the MTK_CIRQ and MTK_SYSIRQ into defconfig?
> > (arch/arm64/config/defconfig)
>
> I don't deal with defconfig at all. That's a patch for the arm-soc
> people.
>
> >
> > It would ensures all platform are executable.
> >
> > e.g.
> > --- a/arch/arm64/configs/defconfig
> > +++ b/arch/arm64/configs/defconfig
> >
> >  CONFIG_QCOM_PDC=y
> > +CONFIG_MTK_SYSIRQ=m
> > +CONFIG_MTK_CIRQ=y
> >  CONFIG_RESET_QCOM_AOSS=y
> >
> > --- a/drivers/irqchip/Kconfig
> > +++ b/drivers/irqchip/Kconfig
> > @@ -572,4 +572,18 @@ config LOONGSON_PCH_MSI
> >         help
> >           Support for the Loongson PCH MSI Controller.
> >
> > +config MTK_SYSIRQ
> > +       tristate "Mediatek interrupt polarity controller"
> > +       depends on ARCH_MEDIATEK || COMPILE_TEST
> > +       help
> > +         Interrupt polarity controller driver to swap polarity for
> > +         interrupts for Mediatek mobile chips.
> > +
> > +config MTK_CIRQ
> > +       bool "Mediatek low-power interrupt controller"
> > +       depends on ARCH_MEDIATEK || COMPILE_TEST
> > +       help
> > +         Low-power interrupt controller driver to monitor IRQS
> > +         in the sleep mode for Mediatek mobile chips.
> > +
> >
> >
> > P.S I'll make the irq-mtk-sysirq driver as a loadable kernel module for
> > GKI
>
> You might as well turn both drivers into modules. Saravana was working
> on a set of patches to ease this transition.

Sent out v2 of that patch now.
https://lore.kernel.org/lkml/20200717024447.3128361-1-saravanak@google.com/T/#u

-Saravana

^ permalink raw reply

* RE: [RFC PATCH V7 1/3] media: platform: mtk-isp: Add Mediatek sensor interface driver
From: Louis Kuo (郭德寧) @ 2020-07-17  2:55 UTC (permalink / raw)
  To: Chun-Kuang Hu
  Cc: hans.verkuil@cisco.com, laurent.pinchart+renesas@ideasonboard.com,
	Tomasz Figa, keiichiw@chromium.org, Matthias Brugger,
	Mauro Carvalho Chehab, devicetree@vger.kernel.org,
	Sean Cheng (鄭昇弘), srv_heupstream,
	Jerry-ch Chen (陳敬憲),
	Jungo Lin (林明俊),
	Sj Huang (黃信璋), yuzhao@chromium.org,
	moderated list:ARM/Mediatek SoC support, zwisler@chromium.org,
	Christie Yu (游雅惠),
	Frederic Chen (陳俊元), Linux ARM,
	linux-media@vger.kernel.org
In-Reply-To: <CAAOTY_-+v_t3Vv-Ms7k9jCxJ+0B9qb93tBkL=3OmpMLeyAdV-g@mail.gmail.com>

Hi Chun-Kuang,

Since phy driver is not belong to V4L2 scope

Should I need to upsteam 8183 mipi phy driver with new a patch other than this one ?

BRs
Louis

-----Original Message-----
From: Chun-Kuang Hu [mailto:chunkuang.hu@kernel.org] 
Sent: Thursday, July 9, 2020 9:13 PM
To: Louis Kuo (郭德寧)
Cc: hans.verkuil@cisco.com; laurent.pinchart+renesas@ideasonboard.com; Tomasz Figa; keiichiw@chromium.org; Matthias Brugger; Mauro Carvalho Chehab; devicetree@vger.kernel.org; Sean Cheng (鄭昇弘); srv_heupstream; Jerry-ch Chen (陳敬憲); Jungo Lin (林明俊); Sj Huang (黃信璋); yuzhao@chromium.org; moderated list:ARM/Mediatek SoC support; zwisler@chromium.org; Christie Yu (游雅惠); Frederic Chen (陳俊元); Linux ARM; linux-media@vger.kernel.org
Subject: Re: [RFC PATCH V7 1/3] media: platform: mtk-isp: Add Mediatek sensor interface driver

Hi, Louis:

Louis Kuo <louis.kuo@mediatek.com> 於 2020年7月8日 週三 下午6:41寫道:
>
> This patch adds Mediatek's sensor interface driver. Sensor interface 
> driver is a MIPI-CSI2 host driver, namely, a HW camera interface controller.
> It support a widely adopted, simple, high-speed protocol primarily 
> intended for point-to-point image and video transmission between 
> cameras and host devices. The mtk-isp directory will contain drivers 
> for multiple IP blocks found in Mediatek ISP system. It will include 
> ISP Pass 1 driver, sensor interface driver, DIP driver and face detection driver.
>
> Signed-off-by: Louis Kuo <louis.kuo@mediatek.com>
> ---
>  drivers/media/platform/Makefile               |    1 +
>  drivers/media/platform/mtk-isp/Kconfig        |   18 +
>  drivers/media/platform/mtk-isp/Makefile       |    3 +
>  .../media/platform/mtk-isp/seninf/Makefile    |    7 +
>  .../platform/mtk-isp/seninf/mtk_seninf.c      |  974 +++++++++++
>  .../platform/mtk-isp/seninf/mtk_seninf_dphy.c |  353 ++++

I think phy driver should be placed in drivers/phy/mediatek and separate phy driver to an independent patch.

>  .../platform/mtk-isp/seninf/mtk_seninf_reg.h  | 1491 +++++++++++++++++
>  .../mtk-isp/seninf/mtk_seninf_rx_reg.h        |  515 ++++++
>  8 files changed, 3362 insertions(+)
>  create mode 100644 drivers/media/platform/mtk-isp/Kconfig
>  create mode 100644 drivers/media/platform/mtk-isp/Makefile
>  create mode 100644 drivers/media/platform/mtk-isp/seninf/Makefile
>  create mode 100644 drivers/media/platform/mtk-isp/seninf/mtk_seninf.c
>  create mode 100644 
> drivers/media/platform/mtk-isp/seninf/mtk_seninf_dphy.c
>  create mode 100644 
> drivers/media/platform/mtk-isp/seninf/mtk_seninf_reg.h
>  create mode 100644 
> drivers/media/platform/mtk-isp/seninf/mtk_seninf_rx_reg.h
>

[snip]

> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of_graph.h>
> +#include <linux/of_irq.h>

No irq handler, so remove this.

> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/slab.h>
> +#include <linux/videodev2.h>
> +#include <media/v4l2-async.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-event.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-subdev.h>
> +#include <linux/phy/phy.h>
> +#include "mtk_seninf_reg.h"
> +

[snip]

> +
> +static int seninf_set_ctrl(struct v4l2_ctrl *ctrl) {
> +       struct mtk_seninf *priv = container_of(ctrl->handler,
> +                                            struct mtk_seninf, 
> +ctrl_handler);
> +
> +       switch (ctrl->id) {
> +       case V4L2_CID_TEST_PATTERN:
> +               if (ctrl->val == TEST_GEN_PATTERN)
> +                       return seninf_enable_test_pattern(priv);

Without this, this driver still works, so move this to an independent patch.

> +               else if (ctrl->val == TEST_DUMP_DEBUG_INFO)
> +                       return seninf_dump_debug_info(priv);

Ditto.

> +               else
> +                       return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +

[snip]

> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id mtk_mipi_dphy_of_match[] = {
> +       {.compatible = "mediatek,mt8183-mipi_dphy"},

Where is the definition of "mediatek,mt8183-mipi_dphy"?

Regards,
Chun-Kuang.

> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, mtk_mipi_dphy_of_match); #endif
> +
> +static struct platform_driver mipi_dphy_pdrv = {
> +       .probe  = mipi_dphy_probe,
> +       .driver = {
> +               .name   = "mipi_dphy",
> +               .of_match_table = of_match_ptr(mtk_mipi_dphy_of_match),
> +       },
> +};
> +
> +module_platform_driver(mipi_dphy_pdrv);
> +

^ permalink raw reply

* Re: [RFC PATCH v1] irqchip: Add IRQCHIP_MODULE_BEGIN/END helper macros
From: Saravana Kannan @ 2020-07-17  2:55 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: John Stultz, Thomas Gleixner, Jason Cooper, Android Kernel Team,
	lkml
In-Reply-To: <d17908a4313ed0f5ccfa8265611738b2@kernel.org>

On Wed, Jun 3, 2020 at 3:12 AM Marc Zyngier <maz@kernel.org> wrote:
>
> Hi Saravana,
>
> On 2020-05-01 21:23, Saravana Kannan wrote:
> > On Fri, May 1, 2020 at 1:48 AM Marc Zyngier <maz@kernel.org> wrote:
> >>
> >> On 2020-04-29 20:04, Saravana Kannan wrote:
> >> > On Wed, Apr 29, 2020 at 2:28 AM Marc Zyngier <maz@kernel.org> wrote:
> >>
> >> [...]
> >>
> >> >> One thing though: this seems to be exclusively DT driven. Have you
> >> >> looked into how that would look like for other firmware types such as
> >> >> ACPI?
> >> >
> >> > I'm not very familiar with ACPI at all. I've just started to learn
> >> > about how it works in the past few months poking at code when I have
> >> > some time. So I haven't tried to get this to work with ACPI nor do I
> >> > think I'll be able to do that anytime in the near future. I hope that
> >> > doesn't block this from being used for DT based platforms.
> >>
> >> As long as you don't try to modularise a driver that does both DT and
> >> ACPI, you'll be safe. I'm also actively trying to discourage people
> >> from inventing custom irqchips on ACPI platforms (the spec almost
> >> forbids them, but not quite).
> >>
> >> >> Another thing is the handling of dependencies. Statically built
> >> >> irqchips are initialized in the right order based on the topology
> >> >> described in DT, and are initialized early enough that client devices
> >> >> will find their irqchip This doesn't work here, obviously.
> >> >
> >> > Yeah, I read that code thoroughly :)
> >> >
> >> >> How do you
> >> >> propose we handle these dependencies, both between irqchip drivers and
> >> >> client drivers?
> >> >
> >> > For client drivers, we don't need to do anything. The IRQ apis seem to
> >> > already handle -EPROBE_DEFER correctly in this case.
> >> >
> >> > For irqchip drivers, the easy answer can be: Load the IRQ modules
> >> > early if you make them modules.
> >>
> >> Uhuh. I'm afraid that's not a practical solution. We need to offer the
> >> same behaviour for both and not rely on the user to understand the
> >> topology of the SoC.
> >>
> >> > But in my case, I've been testing this with fw_devlink=on. The TL;DR
> >> > of "fw_devlink=on" in this context is that the IRQ devices will get
> >> > device links created based on "interrupt-parent" property. So, with
> >> > the magic of device links, these IRQ devices will probe in the right
> >> > topological order without any wasted deferred probe attempts. For
> >> > cases without fw_devlink=on, I think I can improve
> >> > platform_irqchip_probe() in my patch to check if the parent device has
> >> > probed and defer if it hasn't.
> >>
> >> Seems like an interesting option. Two things then:
> >>
> >> - Can we enforce the use of fw_devlink for modularized irqchips?
> >
> > fw_devlink doesn't have any config and it's a command line option. So
> > not sure how you can enforce that.
>
> By having a config option that forces it on if that option is selected
> by modular irqchips?

Hmmm... not a bad idea. I think this could be useful in general. I'll
look into that separately.

> More importantly, what is the drawback of having
> fw_devlink on at all times? It definitely looks like the best thing
> since sliced bread (with cheese), so what is the catch?

Lol, thanks for the compliment :) My goal is to eventually enable it by default.

But there are a few corner cases on some boards where the DT has a
cycle and if we just blindly create device links for those, some of
the devices will end up not probing. I've slowly been identifying
different corner cases and making sure we drop the "bad link" that's
causing a cycle so that all the devices still probe properly.
Hopefully I can enable it by default soon.

> >
> >> - For those irqchips that can be modularized, it is apparent that they
> >>    should have been written as platform devices the first place. Maybe
> >>    we should just do that (long term, though).
> >
> > I agree. If they can be platform devices, they should be. But when
> > those platform device drivers are built in, you'll either need:
> > 1) fw_devlink=on to enforce the topological init order
>
> That would have my preference, provided that there is no drawbacks.

Since this can't happen yet I went with option 2.

> > Or
> > 2) have a generic irqchip probe helper function that ensures that.
> > My patch with some additional checks added to platform_irqchip_probe()
> > can provide (2).
> >
> > In the short term, my patch series also makes it easier to convert
> > existing non-platform drivers into platform drivers.
> >
> > So if I fix up platform_irqchip_probe() to also do -EPROBE_DEFER to
> > enforce topology, will that make this patch acceptable?
>
> That'd be a lot better. We also need some guards for things that
> cannot be a driver (primary interrupt controllers don't have a struct
> device).

Just sent out v2 of this patch.
https://lore.kernel.org/lkml/20200717024447.3128361-1-saravanak@google.com/T/#u

-Saravana

^ permalink raw reply

* [PATCH v2] PCI: hv: Fix a timing issue which causes kdump to fail occasionally
From: Wei Hu @ 2020-07-17  2:55 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin, wei.liu, lorenzo.pieralisi, robh,
	bhelgaas, linux-hyperv, linux-pci, linux-kernel, decui, mikelley
  Cc: Wei Hu

Kdump could fail sometime on HyperV guest over Accerlated Network
interface. This is because the retry in hv_pci_enter_d0() relies on
an asynchronous host event to arrive guest before calling
hv_send_resources_allocated(). This fixes the problem by moving retry
to hv_pci_probe(), removing this dependence and making the calling
sequence synchronous.

v2: Adding Fixes tag according to Michael Kelley's review comment.

Fixes: c81992e7f4aa ("PCI: hv: Retry PCI bus D0 entry on invalid device state")
Signed-off-by: Wei Hu <weh@microsoft.com>
---
 drivers/pci/controller/pci-hyperv.c | 66 ++++++++++++++---------------
 1 file changed, 32 insertions(+), 34 deletions(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index bf40ff09c99d..738ee30f3334 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -2759,10 +2759,8 @@ static int hv_pci_enter_d0(struct hv_device *hdev)
 	struct pci_bus_d0_entry *d0_entry;
 	struct hv_pci_compl comp_pkt;
 	struct pci_packet *pkt;
-	bool retry = true;
 	int ret;
 
-enter_d0_retry:
 	/*
 	 * Tell the host that the bus is ready to use, and moved into the
 	 * powered-on state.  This includes telling the host which region
@@ -2789,38 +2787,6 @@ static int hv_pci_enter_d0(struct hv_device *hdev)
 	if (ret)
 		goto exit;
 
-	/*
-	 * In certain case (Kdump) the pci device of interest was
-	 * not cleanly shut down and resource is still held on host
-	 * side, the host could return invalid device status.
-	 * We need to explicitly request host to release the resource
-	 * and try to enter D0 again.
-	 */
-	if (comp_pkt.completion_status < 0 && retry) {
-		retry = false;
-
-		dev_err(&hdev->device, "Retrying D0 Entry\n");
-
-		/*
-		 * Hv_pci_bus_exit() calls hv_send_resource_released()
-		 * to free up resources of its child devices.
-		 * In the kdump kernel we need to set the
-		 * wslot_res_allocated to 255 so it scans all child
-		 * devices to release resources allocated in the
-		 * normal kernel before panic happened.
-		 */
-		hbus->wslot_res_allocated = 255;
-
-		ret = hv_pci_bus_exit(hdev, true);
-
-		if (ret == 0) {
-			kfree(pkt);
-			goto enter_d0_retry;
-		}
-		dev_err(&hdev->device,
-			"Retrying D0 failed with ret %d\n", ret);
-	}
-
 	if (comp_pkt.completion_status < 0) {
 		dev_err(&hdev->device,
 			"PCI Pass-through VSP failed D0 Entry with status %x\n",
@@ -3058,6 +3024,7 @@ static int hv_pci_probe(struct hv_device *hdev,
 	struct hv_pcibus_device *hbus;
 	u16 dom_req, dom;
 	char *name;
+	bool enter_d0_retry = true;
 	int ret;
 
 	/*
@@ -3178,11 +3145,42 @@ static int hv_pci_probe(struct hv_device *hdev,
 	if (ret)
 		goto free_fwnode;
 
+retry:
 	ret = hv_pci_query_relations(hdev);
 	if (ret)
 		goto free_irq_domain;
 
 	ret = hv_pci_enter_d0(hdev);
+	/*
+	 * In certain case (Kdump) the pci device of interest was
+	 * not cleanly shut down and resource is still held on host
+	 * side, the host could return invalid device status.
+	 * We need to explicitly request host to release the resource
+	 * and try to enter D0 again.
+	 * The retry should start from hv_pci_query_relations() call.
+	 */
+	if (ret == -EPROTO && enter_d0_retry) {
+		enter_d0_retry = false;
+
+		dev_err(&hdev->device, "Retrying D0 Entry\n");
+
+		/*
+		 * Hv_pci_bus_exit() calls hv_send_resources_released()
+		 * to free up resources of its child devices.
+		 * In the kdump kernel we need to set the
+		 * wslot_res_allocated to 255 so it scans all child
+		 * devices to release resources allocated in the
+		 * normal kernel before panic happened.
+		 */
+		hbus->wslot_res_allocated = 255;
+		ret = hv_pci_bus_exit(hdev, true);
+
+		if (ret == 0)
+			goto retry;
+
+		dev_err(&hdev->device,
+			"Retrying D0 failed with ret %d\n", ret);
+	}
 	if (ret)
 		goto free_irq_domain;
 
-- 
2.20.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.