[PATCH] ASoC: intel: Fix sst-dsp dependency on dw stuff

[PATCH] ASoC: intel: Fix sst-dsp dependency on dw stuff

[Takashi Iwai]

The recent commit [a92ea59b74e2: ASoC: Intel: sst: only select
sst-firmware when DW DMAC is built-in] introduced more strict kconfig
dependency (depends on DW_DMAC_CORE=y) for avoiding the build failures
due to dependency messes in intel-sst.  This makes, however, it
impossible to use this driver with the modularized systems,
i.e. typically on Linux distros.

The problem addressed in the commit above is that sst_dsp_new() and
sst_dsp_free() includes the firmware init / finish that call dw_*()
functions.  Thus building it as built-in with DW_DMAC_CORE module
results in the missing symbols.

However, these sst_dsp functions are basically called only from the
drivers that depend on DW_DMAC_CORE already.  That is, once when these
functions are split out, the rest can be independent from dw stuff.

This patch attempts to solve the issue by the following:
- Split sst-dsp stuff into two modules: snd-soc-sst-dsp and
  snd-soc-sst-firmware.
- Move sst_dsp_new() and sst_dsp_free() to the latter module so that
  the former module can be independent from DW_DMAC_CORE.
- Add a new kconfig SND_SOC_INTEL_SST_FIRMWARE to select the latter
  module by machine drivers.

One only remaining pitfall is that each machine driver has to select
SND_SOC_INTEL_SST_FIRMWARE carefully depending on DW_DMAC_CORE.
This can't be done cleanly due to the restriction of the current
kbuild.

Bugzilla: https://bugzilla.opensuse.org/show_bug.cgi?id=988117
Fixes: a92ea59b74e2 ('ASoC: Intel: sst: only select sst-firmware when DW DMAC is built-in')
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/soc/intel/Kconfig               | 18 +++++++---
 sound/soc/intel/common/Makefile       |  4 +--
 sound/soc/intel/common/sst-dsp-priv.h |  4 ---
 sound/soc/intel/common/sst-dsp.c      | 67 ----------------------------------
 sound/soc/intel/common/sst-dsp.h      |  2 +-
 sound/soc/intel/common/sst-firmware.c | 68 +++++++++++++++++++++++++++++++++++
 6 files changed, 85 insertions(+), 78 deletions(-)

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index 9c86459d0fc3..a20c3dfbcb5d 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -32,6 +32,12 @@ config SND_SOC_INTEL_SST
 	select SND_SOC_INTEL_SST_MATCH if ACPI
 	depends on (X86 || COMPILE_TEST)
 
+# firmware stuff depends DW_DMAC_CORE; since there is no depends-on from
+# the reverse selection, each machine driver needs to select
+# SND_SOC_INTEL_SST_FIRMWARE carefully depending on DW_DMAC_CORE
+config SND_SOC_INTEL_SST_FIRMWARE
+	tristate
+
 config SND_SOC_INTEL_SST_ACPI
 	tristate
 
@@ -47,8 +53,9 @@ config SND_SOC_INTEL_BAYTRAIL
 config SND_SOC_INTEL_HASWELL_MACH
 	tristate "ASoC Audio DSP support for Intel Haswell Lynxpoint"
 	depends on X86_INTEL_LPSS && I2C && I2C_DESIGNWARE_PLATFORM
-	depends on DW_DMAC_CORE=y
+	depends on DW_DMAC_CORE
 	select SND_SOC_INTEL_SST
+	select SND_SOC_INTEL_SST_FIRMWARE
 	select SND_SOC_INTEL_HASWELL
 	select SND_SOC_RT5640
 	help
@@ -91,8 +98,9 @@ config SND_SOC_INTEL_BXT_RT298_MACH
 config SND_SOC_INTEL_BYT_RT5640_MACH
 	tristate "ASoC Audio driver for Intel Baytrail with RT5640 codec"
 	depends on X86_INTEL_LPSS && I2C
-	depends on DW_DMAC_CORE=y && (SND_SST_IPC_ACPI = n)
+	depends on DW_DMAC_CORE && (SND_SST_IPC_ACPI = n)
 	select SND_SOC_INTEL_SST
+	select SND_SOC_INTEL_SST_FIRMWARE
 	select SND_SOC_INTEL_BAYTRAIL
 	select SND_SOC_RT5640
 	help
@@ -103,8 +111,9 @@ config SND_SOC_INTEL_BYT_RT5640_MACH
 config SND_SOC_INTEL_BYT_MAX98090_MACH
 	tristate "ASoC Audio driver for Intel Baytrail with MAX98090 codec"
 	depends on X86_INTEL_LPSS && I2C
-	depends on DW_DMAC_CORE=y && (SND_SST_IPC_ACPI = n)
+	depends on DW_DMAC_CORE && (SND_SST_IPC_ACPI = n)
 	select SND_SOC_INTEL_SST
+	select SND_SOC_INTEL_SST_FIRMWARE
 	select SND_SOC_INTEL_BAYTRAIL
 	select SND_SOC_MAX98090
 	help
@@ -115,8 +124,9 @@ config SND_SOC_INTEL_BROADWELL_MACH
 	tristate "ASoC Audio DSP support for Intel Broadwell Wildcatpoint"
 	depends on X86_INTEL_LPSS && I2C && DW_DMAC && \
 		   I2C_DESIGNWARE_PLATFORM
-	depends on DW_DMAC_CORE=y
+	depends on DW_DMAC_CORE
 	select SND_SOC_INTEL_SST
+	select SND_SOC_INTEL_SST_FIRMWARE
 	select SND_SOC_INTEL_HASWELL
 	select SND_SOC_RT286
 	help
diff --git a/sound/soc/intel/common/Makefile b/sound/soc/intel/common/Makefile
index fbbb25c2ceed..1a35149bcad7 100644
--- a/sound/soc/intel/common/Makefile
+++ b/sound/soc/intel/common/Makefile
@@ -2,9 +2,9 @@ snd-soc-sst-dsp-objs := sst-dsp.o
 snd-soc-sst-acpi-objs := sst-acpi.o
 snd-soc-sst-match-objs := sst-match-acpi.o
 snd-soc-sst-ipc-objs := sst-ipc.o
-
-snd-soc-sst-dsp-$(CONFIG_DW_DMAC_CORE) += sst-firmware.o
+snd-soc-sst-firmware-objs := sst-firmware.o
 
 obj-$(CONFIG_SND_SOC_INTEL_SST) += snd-soc-sst-dsp.o snd-soc-sst-ipc.o
 obj-$(CONFIG_SND_SOC_INTEL_SST_ACPI) += snd-soc-sst-acpi.o
 obj-$(CONFIG_SND_SOC_INTEL_SST_MATCH) += snd-soc-sst-match.o
+obj-$(CONFIG_SND_SOC_INTEL_SST_FIRMWARE) += snd-soc-sst-firmware.o
diff --git a/sound/soc/intel/common/sst-dsp-priv.h b/sound/soc/intel/common/sst-dsp-priv.h
index 97dc1ae05e69..d13c84364c3c 100644
--- a/sound/soc/intel/common/sst-dsp-priv.h
+++ b/sound/soc/intel/common/sst-dsp-priv.h
@@ -383,10 +383,6 @@ struct sst_mem_block *sst_mem_block_register(struct sst_dsp *dsp, u32 offset,
 	u32 index, void *private);
 void sst_mem_block_unregister_all(struct sst_dsp *dsp);
 
-/* Create/Free DMA resources */
-int sst_dma_new(struct sst_dsp *sst);
-void sst_dma_free(struct sst_dma *dma);
-
 u32 sst_dsp_get_offset(struct sst_dsp *dsp, u32 offset,
 	enum sst_mem_type type);
 #endif
diff --git a/sound/soc/intel/common/sst-dsp.c b/sound/soc/intel/common/sst-dsp.c
index ff2196ef359f..c00ede4ea4d7 100644
--- a/sound/soc/intel/common/sst-dsp.c
+++ b/sound/soc/intel/common/sst-dsp.c
@@ -420,73 +420,6 @@ void sst_dsp_inbox_read(struct sst_dsp *sst, void *message, size_t bytes)
 }
 EXPORT_SYMBOL_GPL(sst_dsp_inbox_read);
 
-#ifdef CONFIG_DW_DMAC_CORE
-struct sst_dsp *sst_dsp_new(struct device *dev,
-	struct sst_dsp_device *sst_dev, struct sst_pdata *pdata)
-{
-	struct sst_dsp *sst;
-	int err;
-
-	dev_dbg(dev, "initialising audio DSP id 0x%x\n", pdata->id);
-
-	sst = devm_kzalloc(dev, sizeof(*sst), GFP_KERNEL);
-	if (sst == NULL)
-		return NULL;
-
-	spin_lock_init(&sst->spinlock);
-	mutex_init(&sst->mutex);
-	sst->dev = dev;
-	sst->dma_dev = pdata->dma_dev;
-	sst->thread_context = sst_dev->thread_context;
-	sst->sst_dev = sst_dev;
-	sst->id = pdata->id;
-	sst->irq = pdata->irq;
-	sst->ops = sst_dev->ops;
-	sst->pdata = pdata;
-	INIT_LIST_HEAD(&sst->used_block_list);
-	INIT_LIST_HEAD(&sst->free_block_list);
-	INIT_LIST_HEAD(&sst->module_list);
-	INIT_LIST_HEAD(&sst->fw_list);
-	INIT_LIST_HEAD(&sst->scratch_block_list);
-
-	/* Initialise SST Audio DSP */
-	if (sst->ops->init) {
-		err = sst->ops->init(sst, pdata);
-		if (err < 0)
-			return NULL;
-	}
-
-	/* Register the ISR */
-	err = request_threaded_irq(sst->irq, sst->ops->irq_handler,
-		sst_dev->thread, IRQF_SHARED, "AudioDSP", sst);
-	if (err)
-		goto irq_err;
-
-	err = sst_dma_new(sst);
-	if (err)
-		dev_warn(dev, "sst_dma_new failed %d\n", err);
-
-	return sst;
-
-irq_err:
-	if (sst->ops->free)
-		sst->ops->free(sst);
-
-	return NULL;
-}
-EXPORT_SYMBOL_GPL(sst_dsp_new);
-
-void sst_dsp_free(struct sst_dsp *sst)
-{
-	free_irq(sst->irq, sst);
-	if (sst->ops->free)
-		sst->ops->free(sst);
-
-	sst_dma_free(sst->dma);
-}
-EXPORT_SYMBOL_GPL(sst_dsp_free);
-#endif
-
 /* Module information */
 MODULE_AUTHOR("Liam Girdwood");
 MODULE_DESCRIPTION("Intel SST Core");
diff --git a/sound/soc/intel/common/sst-dsp.h b/sound/soc/intel/common/sst-dsp.h
index 0b84c719ec48..859f0de00339 100644
--- a/sound/soc/intel/common/sst-dsp.h
+++ b/sound/soc/intel/common/sst-dsp.h
@@ -216,7 +216,7 @@ struct sst_pdata {
 	void *dsp;
 };
 
-#ifdef CONFIG_DW_DMAC_CORE
+#if IS_ENABLED(CONFIG_DW_DMAC_CORE)
 /* Initialization */
 struct sst_dsp *sst_dsp_new(struct device *dev,
 	struct sst_dsp_device *sst_dev, struct sst_pdata *pdata);
diff --git a/sound/soc/intel/common/sst-firmware.c b/sound/soc/intel/common/sst-firmware.c
index 25993527370b..a086c35f91bb 100644
--- a/sound/soc/intel/common/sst-firmware.c
+++ b/sound/soc/intel/common/sst-firmware.c
@@ -1211,3 +1211,71 @@ u32 sst_dsp_get_offset(struct sst_dsp *dsp, u32 offset,
 	}
 }
 EXPORT_SYMBOL_GPL(sst_dsp_get_offset);
+
+struct sst_dsp *sst_dsp_new(struct device *dev,
+	struct sst_dsp_device *sst_dev, struct sst_pdata *pdata)
+{
+	struct sst_dsp *sst;
+	int err;
+
+	dev_dbg(dev, "initialising audio DSP id 0x%x\n", pdata->id);
+
+	sst = devm_kzalloc(dev, sizeof(*sst), GFP_KERNEL);
+	if (sst == NULL)
+		return NULL;
+
+	spin_lock_init(&sst->spinlock);
+	mutex_init(&sst->mutex);
+	sst->dev = dev;
+	sst->dma_dev = pdata->dma_dev;
+	sst->thread_context = sst_dev->thread_context;
+	sst->sst_dev = sst_dev;
+	sst->id = pdata->id;
+	sst->irq = pdata->irq;
+	sst->ops = sst_dev->ops;
+	sst->pdata = pdata;
+	INIT_LIST_HEAD(&sst->used_block_list);
+	INIT_LIST_HEAD(&sst->free_block_list);
+	INIT_LIST_HEAD(&sst->module_list);
+	INIT_LIST_HEAD(&sst->fw_list);
+	INIT_LIST_HEAD(&sst->scratch_block_list);
+
+	/* Initialise SST Audio DSP */
+	if (sst->ops->init) {
+		err = sst->ops->init(sst, pdata);
+		if (err < 0)
+			return NULL;
+	}
+
+	/* Register the ISR */
+	err = request_threaded_irq(sst->irq, sst->ops->irq_handler,
+		sst_dev->thread, IRQF_SHARED, "AudioDSP", sst);
+	if (err)
+		goto irq_err;
+
+	err = sst_dma_new(sst);
+	if (err)
+		dev_warn(dev, "sst_dma_new failed %d\n", err);
+
+	return sst;
+
+irq_err:
+	if (sst->ops->free)
+		sst->ops->free(sst);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(sst_dsp_new);
+
+void sst_dsp_free(struct sst_dsp *sst)
+{
+	free_irq(sst->irq, sst);
+	if (sst->ops->free)
+		sst->ops->free(sst);
+
+	sst_dma_free(sst->dma);
+}
+EXPORT_SYMBOL_GPL(sst_dsp_free);
+
+MODULE_DESCRIPTION("Intel SST Firmware Loader");
+MODULE_LICENSE("GPL v2");
-- 
2.9.0

Applied "ASoC: intel: Fix sst-dsp dependency on dw stuff" to the asoc tree

[Mark Brown]

The patch

   ASoC: intel: Fix sst-dsp dependency on dw stuff

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From a395bdd6b24b692adbce0df6510ec9f2af57573e Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
Date: Mon, 11 Jul 2016 10:39:11 +0200
Subject: [PATCH] ASoC: intel: Fix sst-dsp dependency on dw stuff

The recent commit [a92ea59b74e2: ASoC: Intel: sst: only select
sst-firmware when DW DMAC is built-in] introduced more strict kconfig
dependency (depends on DW_DMAC_CORE=y) for avoiding the build failures
due to dependency messes in intel-sst.  This makes, however, it
impossible to use this driver with the modularized systems,
i.e. typically on Linux distros.

The problem addressed in the commit above is that sst_dsp_new() and
sst_dsp_free() includes the firmware init / finish that call dw_*()
functions.  Thus building it as built-in with DW_DMAC_CORE module
results in the missing symbols.

However, these sst_dsp functions are basically called only from the
drivers that depend on DW_DMAC_CORE already.  That is, once when these
functions are split out, the rest can be independent from dw stuff.

This patch attempts to solve the issue by the following:
- Split sst-dsp stuff into two modules: snd-soc-sst-dsp and
  snd-soc-sst-firmware.
- Move sst_dsp_new() and sst_dsp_free() to the latter module so that
  the former module can be independent from DW_DMAC_CORE.
- Add a new kconfig SND_SOC_INTEL_SST_FIRMWARE to select the latter
  module by machine drivers.

One only remaining pitfall is that each machine driver has to select
SND_SOC_INTEL_SST_FIRMWARE carefully depending on DW_DMAC_CORE.
This can't be done cleanly due to the restriction of the current
kbuild.

Bugzilla: https://bugzilla.opensuse.org/show_bug.cgi?id=988117
Fixes: a92ea59b74e2 ('ASoC: Intel: sst: only select sst-firmware when DW DMAC is built-in')
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/Kconfig               | 18 +++++++---
 sound/soc/intel/common/Makefile       |  4 +--
 sound/soc/intel/common/sst-dsp-priv.h |  4 ---
 sound/soc/intel/common/sst-dsp.c      | 67 ----------------------------------
 sound/soc/intel/common/sst-dsp.h      |  2 +-
 sound/soc/intel/common/sst-firmware.c | 68 +++++++++++++++++++++++++++++++++++
 6 files changed, 85 insertions(+), 78 deletions(-)

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index 9c86459d0fc3..a20c3dfbcb5d 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -32,6 +32,12 @@ config SND_SOC_INTEL_SST
 	select SND_SOC_INTEL_SST_MATCH if ACPI
 	depends on (X86 || COMPILE_TEST)
 
+# firmware stuff depends DW_DMAC_CORE; since there is no depends-on from
+# the reverse selection, each machine driver needs to select
+# SND_SOC_INTEL_SST_FIRMWARE carefully depending on DW_DMAC_CORE
+config SND_SOC_INTEL_SST_FIRMWARE
+	tristate
+
 config SND_SOC_INTEL_SST_ACPI
 	tristate
 
@@ -47,8 +53,9 @@ config SND_SOC_INTEL_BAYTRAIL
 config SND_SOC_INTEL_HASWELL_MACH
 	tristate "ASoC Audio DSP support for Intel Haswell Lynxpoint"
 	depends on X86_INTEL_LPSS && I2C && I2C_DESIGNWARE_PLATFORM
-	depends on DW_DMAC_CORE=y
+	depends on DW_DMAC_CORE
 	select SND_SOC_INTEL_SST
+	select SND_SOC_INTEL_SST_FIRMWARE
 	select SND_SOC_INTEL_HASWELL
 	select SND_SOC_RT5640
 	help
@@ -91,8 +98,9 @@ config SND_SOC_INTEL_BXT_RT298_MACH
 config SND_SOC_INTEL_BYT_RT5640_MACH
 	tristate "ASoC Audio driver for Intel Baytrail with RT5640 codec"
 	depends on X86_INTEL_LPSS && I2C
-	depends on DW_DMAC_CORE=y && (SND_SST_IPC_ACPI = n)
+	depends on DW_DMAC_CORE && (SND_SST_IPC_ACPI = n)
 	select SND_SOC_INTEL_SST
+	select SND_SOC_INTEL_SST_FIRMWARE
 	select SND_SOC_INTEL_BAYTRAIL
 	select SND_SOC_RT5640
 	help
@@ -103,8 +111,9 @@ config SND_SOC_INTEL_BYT_RT5640_MACH
 config SND_SOC_INTEL_BYT_MAX98090_MACH
 	tristate "ASoC Audio driver for Intel Baytrail with MAX98090 codec"
 	depends on X86_INTEL_LPSS && I2C
-	depends on DW_DMAC_CORE=y && (SND_SST_IPC_ACPI = n)
+	depends on DW_DMAC_CORE && (SND_SST_IPC_ACPI = n)
 	select SND_SOC_INTEL_SST
+	select SND_SOC_INTEL_SST_FIRMWARE
 	select SND_SOC_INTEL_BAYTRAIL
 	select SND_SOC_MAX98090
 	help
@@ -115,8 +124,9 @@ config SND_SOC_INTEL_BROADWELL_MACH
 	tristate "ASoC Audio DSP support for Intel Broadwell Wildcatpoint"
 	depends on X86_INTEL_LPSS && I2C && DW_DMAC && \
 		   I2C_DESIGNWARE_PLATFORM
-	depends on DW_DMAC_CORE=y
+	depends on DW_DMAC_CORE
 	select SND_SOC_INTEL_SST
+	select SND_SOC_INTEL_SST_FIRMWARE
 	select SND_SOC_INTEL_HASWELL
 	select SND_SOC_RT286
 	help
diff --git a/sound/soc/intel/common/Makefile b/sound/soc/intel/common/Makefile
index fbbb25c2ceed..1a35149bcad7 100644
--- a/sound/soc/intel/common/Makefile
+++ b/sound/soc/intel/common/Makefile
@@ -2,9 +2,9 @@ snd-soc-sst-dsp-objs := sst-dsp.o
 snd-soc-sst-acpi-objs := sst-acpi.o
 snd-soc-sst-match-objs := sst-match-acpi.o
 snd-soc-sst-ipc-objs := sst-ipc.o
-
-snd-soc-sst-dsp-$(CONFIG_DW_DMAC_CORE) += sst-firmware.o
+snd-soc-sst-firmware-objs := sst-firmware.o
 
 obj-$(CONFIG_SND_SOC_INTEL_SST) += snd-soc-sst-dsp.o snd-soc-sst-ipc.o
 obj-$(CONFIG_SND_SOC_INTEL_SST_ACPI) += snd-soc-sst-acpi.o
 obj-$(CONFIG_SND_SOC_INTEL_SST_MATCH) += snd-soc-sst-match.o
+obj-$(CONFIG_SND_SOC_INTEL_SST_FIRMWARE) += snd-soc-sst-firmware.o
diff --git a/sound/soc/intel/common/sst-dsp-priv.h b/sound/soc/intel/common/sst-dsp-priv.h
index 97dc1ae05e69..d13c84364c3c 100644
--- a/sound/soc/intel/common/sst-dsp-priv.h
+++ b/sound/soc/intel/common/sst-dsp-priv.h
@@ -383,10 +383,6 @@ struct sst_mem_block *sst_mem_block_register(struct sst_dsp *dsp, u32 offset,
 	u32 index, void *private);
 void sst_mem_block_unregister_all(struct sst_dsp *dsp);
 
-/* Create/Free DMA resources */
-int sst_dma_new(struct sst_dsp *sst);
-void sst_dma_free(struct sst_dma *dma);
-
 u32 sst_dsp_get_offset(struct sst_dsp *dsp, u32 offset,
 	enum sst_mem_type type);
 #endif
diff --git a/sound/soc/intel/common/sst-dsp.c b/sound/soc/intel/common/sst-dsp.c
index ff2196ef359f..c00ede4ea4d7 100644
--- a/sound/soc/intel/common/sst-dsp.c
+++ b/sound/soc/intel/common/sst-dsp.c
@@ -420,73 +420,6 @@ void sst_dsp_inbox_read(struct sst_dsp *sst, void *message, size_t bytes)
 }
 EXPORT_SYMBOL_GPL(sst_dsp_inbox_read);
 
-#ifdef CONFIG_DW_DMAC_CORE
-struct sst_dsp *sst_dsp_new(struct device *dev,
-	struct sst_dsp_device *sst_dev, struct sst_pdata *pdata)
-{
-	struct sst_dsp *sst;
-	int err;
-
-	dev_dbg(dev, "initialising audio DSP id 0x%x\n", pdata->id);
-
-	sst = devm_kzalloc(dev, sizeof(*sst), GFP_KERNEL);
-	if (sst == NULL)
-		return NULL;
-
-	spin_lock_init(&sst->spinlock);
-	mutex_init(&sst->mutex);
-	sst->dev = dev;
-	sst->dma_dev = pdata->dma_dev;
-	sst->thread_context = sst_dev->thread_context;
-	sst->sst_dev = sst_dev;
-	sst->id = pdata->id;
-	sst->irq = pdata->irq;
-	sst->ops = sst_dev->ops;
-	sst->pdata = pdata;
-	INIT_LIST_HEAD(&sst->used_block_list);
-	INIT_LIST_HEAD(&sst->free_block_list);
-	INIT_LIST_HEAD(&sst->module_list);
-	INIT_LIST_HEAD(&sst->fw_list);
-	INIT_LIST_HEAD(&sst->scratch_block_list);
-
-	/* Initialise SST Audio DSP */
-	if (sst->ops->init) {
-		err = sst->ops->init(sst, pdata);
-		if (err < 0)
-			return NULL;
-	}
-
-	/* Register the ISR */
-	err = request_threaded_irq(sst->irq, sst->ops->irq_handler,
-		sst_dev->thread, IRQF_SHARED, "AudioDSP", sst);
-	if (err)
-		goto irq_err;
-
-	err = sst_dma_new(sst);
-	if (err)
-		dev_warn(dev, "sst_dma_new failed %d\n", err);
-
-	return sst;
-
-irq_err:
-	if (sst->ops->free)
-		sst->ops->free(sst);
-
-	return NULL;
-}
-EXPORT_SYMBOL_GPL(sst_dsp_new);
-
-void sst_dsp_free(struct sst_dsp *sst)
-{
-	free_irq(sst->irq, sst);
-	if (sst->ops->free)
-		sst->ops->free(sst);
-
-	sst_dma_free(sst->dma);
-}
-EXPORT_SYMBOL_GPL(sst_dsp_free);
-#endif
-
 /* Module information */
 MODULE_AUTHOR("Liam Girdwood");
 MODULE_DESCRIPTION("Intel SST Core");
diff --git a/sound/soc/intel/common/sst-dsp.h b/sound/soc/intel/common/sst-dsp.h
index 0b84c719ec48..859f0de00339 100644
--- a/sound/soc/intel/common/sst-dsp.h
+++ b/sound/soc/intel/common/sst-dsp.h
@@ -216,7 +216,7 @@ struct sst_pdata {
 	void *dsp;
 };
 
-#ifdef CONFIG_DW_DMAC_CORE
+#if IS_ENABLED(CONFIG_DW_DMAC_CORE)
 /* Initialization */
 struct sst_dsp *sst_dsp_new(struct device *dev,
 	struct sst_dsp_device *sst_dev, struct sst_pdata *pdata);
diff --git a/sound/soc/intel/common/sst-firmware.c b/sound/soc/intel/common/sst-firmware.c
index 25993527370b..a086c35f91bb 100644
--- a/sound/soc/intel/common/sst-firmware.c
+++ b/sound/soc/intel/common/sst-firmware.c
@@ -1211,3 +1211,71 @@ u32 sst_dsp_get_offset(struct sst_dsp *dsp, u32 offset,
 	}
 }
 EXPORT_SYMBOL_GPL(sst_dsp_get_offset);
+
+struct sst_dsp *sst_dsp_new(struct device *dev,
+	struct sst_dsp_device *sst_dev, struct sst_pdata *pdata)
+{
+	struct sst_dsp *sst;
+	int err;
+
+	dev_dbg(dev, "initialising audio DSP id 0x%x\n", pdata->id);
+
+	sst = devm_kzalloc(dev, sizeof(*sst), GFP_KERNEL);
+	if (sst == NULL)
+		return NULL;
+
+	spin_lock_init(&sst->spinlock);
+	mutex_init(&sst->mutex);
+	sst->dev = dev;
+	sst->dma_dev = pdata->dma_dev;
+	sst->thread_context = sst_dev->thread_context;
+	sst->sst_dev = sst_dev;
+	sst->id = pdata->id;
+	sst->irq = pdata->irq;
+	sst->ops = sst_dev->ops;
+	sst->pdata = pdata;
+	INIT_LIST_HEAD(&sst->used_block_list);
+	INIT_LIST_HEAD(&sst->free_block_list);
+	INIT_LIST_HEAD(&sst->module_list);
+	INIT_LIST_HEAD(&sst->fw_list);
+	INIT_LIST_HEAD(&sst->scratch_block_list);
+
+	/* Initialise SST Audio DSP */
+	if (sst->ops->init) {
+		err = sst->ops->init(sst, pdata);
+		if (err < 0)
+			return NULL;
+	}
+
+	/* Register the ISR */
+	err = request_threaded_irq(sst->irq, sst->ops->irq_handler,
+		sst_dev->thread, IRQF_SHARED, "AudioDSP", sst);
+	if (err)
+		goto irq_err;
+
+	err = sst_dma_new(sst);
+	if (err)
+		dev_warn(dev, "sst_dma_new failed %d\n", err);
+
+	return sst;
+
+irq_err:
+	if (sst->ops->free)
+		sst->ops->free(sst);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(sst_dsp_new);
+
+void sst_dsp_free(struct sst_dsp *sst)
+{
+	free_irq(sst->irq, sst);
+	if (sst->ops->free)
+		sst->ops->free(sst);
+
+	sst_dma_free(sst->dma);
+}
+EXPORT_SYMBOL_GPL(sst_dsp_free);
+
+MODULE_DESCRIPTION("Intel SST Firmware Loader");
+MODULE_LICENSE("GPL v2");
-- 
2.8.1

Re: [PATCH] ASoC: intel: Fix sst-dsp dependency on dw stuff

[Yang Jie]

On 2016年07月11日 16:39, Takashi Iwai wrote:
> The recent commit [a92ea59b74e2: ASoC: Intel: sst: only select
> sst-firmware when DW DMAC is built-in] introduced more strict kconfig
> dependency (depends on DW_DMAC_CORE=y) for avoiding the build failures
> due to dependency messes in intel-sst.  This makes, however, it
> impossible to use this driver with the modularized systems,
> i.e. typically on Linux distros.
>
> The problem addressed in the commit above is that sst_dsp_new() and
> sst_dsp_free() includes the firmware init / finish that call dw_*()
> functions.  Thus building it as built-in with DW_DMAC_CORE module
> results in the missing symbols.
>
> However, these sst_dsp functions are basically called only from the
> drivers that depend on DW_DMAC_CORE already.  That is, once when these
> functions are split out, the rest can be independent from dw stuff.
>
> This patch attempts to solve the issue by the following:
> - Split sst-dsp stuff into two modules: snd-soc-sst-dsp and
>    snd-soc-sst-firmware.
> - Move sst_dsp_new() and sst_dsp_free() to the latter module so that
>    the former module can be independent from DW_DMAC_CORE.
> - Add a new kconfig SND_SOC_INTEL_SST_FIRMWARE to select the latter
>    module by machine drivers.
>
> One only remaining pitfall is that each machine driver has to select
> SND_SOC_INTEL_SST_FIRMWARE carefully depending on DW_DMAC_CORE.
> This can't be done cleanly due to the restriction of the current
> kbuild.

is it good to let SND_SOC_INTEL_SST_FIRMWARE select DW_DMAC_CORE in your 
opinion? That may fix this pitfall, but I am not sure if it is comply 
with convention -- a module should not select another upper layer one?

>
> Bugzilla: https://bugzilla.opensuse.org/show_bug.cgi?id=988117
> Fixes: a92ea59b74e2 ('ASoC: Intel: sst: only select sst-firmware when DW DMAC is built-in')
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

Acked-by: Jie Yang <yang.jie@intel.com>

thanks,
~Keyon

> ---
>   sound/soc/intel/Kconfig               | 18 +++++++---
>   sound/soc/intel/common/Makefile       |  4 +--
>   sound/soc/intel/common/sst-dsp-priv.h |  4 ---
>   sound/soc/intel/common/sst-dsp.c      | 67 ----------------------------------
>   sound/soc/intel/common/sst-dsp.h      |  2 +-
>   sound/soc/intel/common/sst-firmware.c | 68 +++++++++++++++++++++++++++++++++++
>   6 files changed, 85 insertions(+), 78 deletions(-)
>
> diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
> index 9c86459d0fc3..a20c3dfbcb5d 100644
> --- a/sound/soc/intel/Kconfig
> +++ b/sound/soc/intel/Kconfig
> @@ -32,6 +32,12 @@ config SND_SOC_INTEL_SST
>   	select SND_SOC_INTEL_SST_MATCH if ACPI
>   	depends on (X86 || COMPILE_TEST)
>
> +# firmware stuff depends DW_DMAC_CORE; since there is no depends-on from
> +# the reverse selection, each machine driver needs to select
> +# SND_SOC_INTEL_SST_FIRMWARE carefully depending on DW_DMAC_CORE
> +config SND_SOC_INTEL_SST_FIRMWARE
> +	tristate
> +
>   config SND_SOC_INTEL_SST_ACPI
>   	tristate
>
> @@ -47,8 +53,9 @@ config SND_SOC_INTEL_BAYTRAIL
>   config SND_SOC_INTEL_HASWELL_MACH
>   	tristate "ASoC Audio DSP support for Intel Haswell Lynxpoint"
>   	depends on X86_INTEL_LPSS && I2C && I2C_DESIGNWARE_PLATFORM
> -	depends on DW_DMAC_CORE=y
> +	depends on DW_DMAC_CORE
>   	select SND_SOC_INTEL_SST
> +	select SND_SOC_INTEL_SST_FIRMWARE
>   	select SND_SOC_INTEL_HASWELL
>   	select SND_SOC_RT5640
>   	help
> @@ -91,8 +98,9 @@ config SND_SOC_INTEL_BXT_RT298_MACH
>   config SND_SOC_INTEL_BYT_RT5640_MACH
>   	tristate "ASoC Audio driver for Intel Baytrail with RT5640 codec"
>   	depends on X86_INTEL_LPSS && I2C
> -	depends on DW_DMAC_CORE=y && (SND_SST_IPC_ACPI = n)
> +	depends on DW_DMAC_CORE && (SND_SST_IPC_ACPI = n)
>   	select SND_SOC_INTEL_SST
> +	select SND_SOC_INTEL_SST_FIRMWARE
>   	select SND_SOC_INTEL_BAYTRAIL
>   	select SND_SOC_RT5640
>   	help
> @@ -103,8 +111,9 @@ config SND_SOC_INTEL_BYT_RT5640_MACH
>   config SND_SOC_INTEL_BYT_MAX98090_MACH
>   	tristate "ASoC Audio driver for Intel Baytrail with MAX98090 codec"
>   	depends on X86_INTEL_LPSS && I2C
> -	depends on DW_DMAC_CORE=y && (SND_SST_IPC_ACPI = n)
> +	depends on DW_DMAC_CORE && (SND_SST_IPC_ACPI = n)
>   	select SND_SOC_INTEL_SST
> +	select SND_SOC_INTEL_SST_FIRMWARE
>   	select SND_SOC_INTEL_BAYTRAIL
>   	select SND_SOC_MAX98090
>   	help
> @@ -115,8 +124,9 @@ config SND_SOC_INTEL_BROADWELL_MACH
>   	tristate "ASoC Audio DSP support for Intel Broadwell Wildcatpoint"
>   	depends on X86_INTEL_LPSS && I2C && DW_DMAC && \
>   		   I2C_DESIGNWARE_PLATFORM
> -	depends on DW_DMAC_CORE=y
> +	depends on DW_DMAC_CORE
>   	select SND_SOC_INTEL_SST
> +	select SND_SOC_INTEL_SST_FIRMWARE
>   	select SND_SOC_INTEL_HASWELL
>   	select SND_SOC_RT286
>   	help
> diff --git a/sound/soc/intel/common/Makefile b/sound/soc/intel/common/Makefile
> index fbbb25c2ceed..1a35149bcad7 100644
> --- a/sound/soc/intel/common/Makefile
> +++ b/sound/soc/intel/common/Makefile
> @@ -2,9 +2,9 @@ snd-soc-sst-dsp-objs := sst-dsp.o
>   snd-soc-sst-acpi-objs := sst-acpi.o
>   snd-soc-sst-match-objs := sst-match-acpi.o
>   snd-soc-sst-ipc-objs := sst-ipc.o
> -
> -snd-soc-sst-dsp-$(CONFIG_DW_DMAC_CORE) += sst-firmware.o
> +snd-soc-sst-firmware-objs := sst-firmware.o
>
>   obj-$(CONFIG_SND_SOC_INTEL_SST) += snd-soc-sst-dsp.o snd-soc-sst-ipc.o
>   obj-$(CONFIG_SND_SOC_INTEL_SST_ACPI) += snd-soc-sst-acpi.o
>   obj-$(CONFIG_SND_SOC_INTEL_SST_MATCH) += snd-soc-sst-match.o
> +obj-$(CONFIG_SND_SOC_INTEL_SST_FIRMWARE) += snd-soc-sst-firmware.o
> diff --git a/sound/soc/intel/common/sst-dsp-priv.h b/sound/soc/intel/common/sst-dsp-priv.h
> index 97dc1ae05e69..d13c84364c3c 100644
> --- a/sound/soc/intel/common/sst-dsp-priv.h
> +++ b/sound/soc/intel/common/sst-dsp-priv.h
> @@ -383,10 +383,6 @@ struct sst_mem_block *sst_mem_block_register(struct sst_dsp *dsp, u32 offset,
>   	u32 index, void *private);
>   void sst_mem_block_unregister_all(struct sst_dsp *dsp);
>
> -/* Create/Free DMA resources */
> -int sst_dma_new(struct sst_dsp *sst);
> -void sst_dma_free(struct sst_dma *dma);
> -
>   u32 sst_dsp_get_offset(struct sst_dsp *dsp, u32 offset,
>   	enum sst_mem_type type);
>   #endif
> diff --git a/sound/soc/intel/common/sst-dsp.c b/sound/soc/intel/common/sst-dsp.c
> index ff2196ef359f..c00ede4ea4d7 100644
> --- a/sound/soc/intel/common/sst-dsp.c
> +++ b/sound/soc/intel/common/sst-dsp.c
> @@ -420,73 +420,6 @@ void sst_dsp_inbox_read(struct sst_dsp *sst, void *message, size_t bytes)
>   }
>   EXPORT_SYMBOL_GPL(sst_dsp_inbox_read);
>
> -#ifdef CONFIG_DW_DMAC_CORE
> -struct sst_dsp *sst_dsp_new(struct device *dev,
> -	struct sst_dsp_device *sst_dev, struct sst_pdata *pdata)
> -{
> -	struct sst_dsp *sst;
> -	int err;
> -
> -	dev_dbg(dev, "initialising audio DSP id 0x%x\n", pdata->id);
> -
> -	sst = devm_kzalloc(dev, sizeof(*sst), GFP_KERNEL);
> -	if (sst == NULL)
> -		return NULL;
> -
> -	spin_lock_init(&sst->spinlock);
> -	mutex_init(&sst->mutex);
> -	sst->dev = dev;
> -	sst->dma_dev = pdata->dma_dev;
> -	sst->thread_context = sst_dev->thread_context;
> -	sst->sst_dev = sst_dev;
> -	sst->id = pdata->id;
> -	sst->irq = pdata->irq;
> -	sst->ops = sst_dev->ops;
> -	sst->pdata = pdata;
> -	INIT_LIST_HEAD(&sst->used_block_list);
> -	INIT_LIST_HEAD(&sst->free_block_list);
> -	INIT_LIST_HEAD(&sst->module_list);
> -	INIT_LIST_HEAD(&sst->fw_list);
> -	INIT_LIST_HEAD(&sst->scratch_block_list);
> -
> -	/* Initialise SST Audio DSP */
> -	if (sst->ops->init) {
> -		err = sst->ops->init(sst, pdata);
> -		if (err < 0)
> -			return NULL;
> -	}
> -
> -	/* Register the ISR */
> -	err = request_threaded_irq(sst->irq, sst->ops->irq_handler,
> -		sst_dev->thread, IRQF_SHARED, "AudioDSP", sst);
> -	if (err)
> -		goto irq_err;
> -
> -	err = sst_dma_new(sst);
> -	if (err)
> -		dev_warn(dev, "sst_dma_new failed %d\n", err);
> -
> -	return sst;
> -
> -irq_err:
> -	if (sst->ops->free)
> -		sst->ops->free(sst);
> -
> -	return NULL;
> -}
> -EXPORT_SYMBOL_GPL(sst_dsp_new);
> -
> -void sst_dsp_free(struct sst_dsp *sst)
> -{
> -	free_irq(sst->irq, sst);
> -	if (sst->ops->free)
> -		sst->ops->free(sst);
> -
> -	sst_dma_free(sst->dma);
> -}
> -EXPORT_SYMBOL_GPL(sst_dsp_free);
> -#endif
> -
>   /* Module information */
>   MODULE_AUTHOR("Liam Girdwood");
>   MODULE_DESCRIPTION("Intel SST Core");
> diff --git a/sound/soc/intel/common/sst-dsp.h b/sound/soc/intel/common/sst-dsp.h
> index 0b84c719ec48..859f0de00339 100644
> --- a/sound/soc/intel/common/sst-dsp.h
> +++ b/sound/soc/intel/common/sst-dsp.h
> @@ -216,7 +216,7 @@ struct sst_pdata {
>   	void *dsp;
>   };
>
> -#ifdef CONFIG_DW_DMAC_CORE
> +#if IS_ENABLED(CONFIG_DW_DMAC_CORE)
>   /* Initialization */
>   struct sst_dsp *sst_dsp_new(struct device *dev,
>   	struct sst_dsp_device *sst_dev, struct sst_pdata *pdata);
> diff --git a/sound/soc/intel/common/sst-firmware.c b/sound/soc/intel/common/sst-firmware.c
> index 25993527370b..a086c35f91bb 100644
> --- a/sound/soc/intel/common/sst-firmware.c
> +++ b/sound/soc/intel/common/sst-firmware.c
> @@ -1211,3 +1211,71 @@ u32 sst_dsp_get_offset(struct sst_dsp *dsp, u32 offset,
>   	}
>   }
>   EXPORT_SYMBOL_GPL(sst_dsp_get_offset);
> +
> +struct sst_dsp *sst_dsp_new(struct device *dev,
> +	struct sst_dsp_device *sst_dev, struct sst_pdata *pdata)
> +{
> +	struct sst_dsp *sst;
> +	int err;
> +
> +	dev_dbg(dev, "initialising audio DSP id 0x%x\n", pdata->id);
> +
> +	sst = devm_kzalloc(dev, sizeof(*sst), GFP_KERNEL);
> +	if (sst == NULL)
> +		return NULL;
> +
> +	spin_lock_init(&sst->spinlock);
> +	mutex_init(&sst->mutex);
> +	sst->dev = dev;
> +	sst->dma_dev = pdata->dma_dev;
> +	sst->thread_context = sst_dev->thread_context;
> +	sst->sst_dev = sst_dev;
> +	sst->id = pdata->id;
> +	sst->irq = pdata->irq;
> +	sst->ops = sst_dev->ops;
> +	sst->pdata = pdata;
> +	INIT_LIST_HEAD(&sst->used_block_list);
> +	INIT_LIST_HEAD(&sst->free_block_list);
> +	INIT_LIST_HEAD(&sst->module_list);
> +	INIT_LIST_HEAD(&sst->fw_list);
> +	INIT_LIST_HEAD(&sst->scratch_block_list);
> +
> +	/* Initialise SST Audio DSP */
> +	if (sst->ops->init) {
> +		err = sst->ops->init(sst, pdata);
> +		if (err < 0)
> +			return NULL;
> +	}
> +
> +	/* Register the ISR */
> +	err = request_threaded_irq(sst->irq, sst->ops->irq_handler,
> +		sst_dev->thread, IRQF_SHARED, "AudioDSP", sst);
> +	if (err)
> +		goto irq_err;
> +
> +	err = sst_dma_new(sst);
> +	if (err)
> +		dev_warn(dev, "sst_dma_new failed %d\n", err);
> +
> +	return sst;
> +
> +irq_err:
> +	if (sst->ops->free)
> +		sst->ops->free(sst);
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(sst_dsp_new);
> +
> +void sst_dsp_free(struct sst_dsp *sst)
> +{
> +	free_irq(sst->irq, sst);
> +	if (sst->ops->free)
> +		sst->ops->free(sst);
> +
> +	sst_dma_free(sst->dma);
> +}
> +EXPORT_SYMBOL_GPL(sst_dsp_free);
> +
> +MODULE_DESCRIPTION("Intel SST Firmware Loader");
> +MODULE_LICENSE("GPL v2");
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ASoC: intel: Fix sst-dsp dependency on dw stuff

[Takashi Iwai]

On Tue, 12 Jul 2016 04:02:25 +0200,
Yang Jie wrote:
> 
> On 2016年07月11日 16:39, Takashi Iwai wrote:
> > The recent commit [a92ea59b74e2: ASoC: Intel: sst: only select
> > sst-firmware when DW DMAC is built-in] introduced more strict kconfig
> > dependency (depends on DW_DMAC_CORE=y) for avoiding the build failures
> > due to dependency messes in intel-sst.  This makes, however, it
> > impossible to use this driver with the modularized systems,
> > i.e. typically on Linux distros.
> >
> > The problem addressed in the commit above is that sst_dsp_new() and
> > sst_dsp_free() includes the firmware init / finish that call dw_*()
> > functions.  Thus building it as built-in with DW_DMAC_CORE module
> > results in the missing symbols.
> >
> > However, these sst_dsp functions are basically called only from the
> > drivers that depend on DW_DMAC_CORE already.  That is, once when these
> > functions are split out, the rest can be independent from dw stuff.
> >
> > This patch attempts to solve the issue by the following:
> > - Split sst-dsp stuff into two modules: snd-soc-sst-dsp and
> >    snd-soc-sst-firmware.
> > - Move sst_dsp_new() and sst_dsp_free() to the latter module so that
> >    the former module can be independent from DW_DMAC_CORE.
> > - Add a new kconfig SND_SOC_INTEL_SST_FIRMWARE to select the latter
> >    module by machine drivers.
> >
> > One only remaining pitfall is that each machine driver has to select
> > SND_SOC_INTEL_SST_FIRMWARE carefully depending on DW_DMAC_CORE.
> > This can't be done cleanly due to the restriction of the current
> > kbuild.
> 
> is it good to let SND_SOC_INTEL_SST_FIRMWARE select DW_DMAC_CORE in
> your opinion? That may fix this pitfall, but I am not sure if it is
> comply with convention -- a module should not select another upper
> layer one?

It depends.  But in this case, DW_DMAC_CORE alone won't help to make
it functional.  Thus it might be rather confusing for users.


Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel