diff mbox series

ASoC: intel: clean up CONFIG_SND_SST_IPC handling

Message ID 20180121221505.196163-1-arnd@arndb.de
State New
Headers show
Series ASoC: intel: clean up CONFIG_SND_SST_IPC handling | expand

Commit Message

Arnd Bergmann Jan. 21, 2018, 10:14 p.m. UTC
In a configuration with SND_SST_ATOM_HIFI2_PLATFORM_PCI=y and
SND_SST_ATOM_HIFI2_PLATFORM=m, we get this module link failure:

ERROR: "sst_context_init" [sound/soc/intel/atom/sst/snd-intel-sst-acpi.ko] undefined!
ERROR: "sst_context_cleanup" [sound/soc/intel/atom/sst/snd-intel-sst-acpi.ko] undefined!
ERROR: "sst_alloc_drv_context" [sound/soc/intel/atom/sst/snd-intel-sst-acpi.ko] undefined!
ERROR: "intel_sst_pm" [sound/soc/intel/atom/sst/snd-intel-sst-acpi.ko] undefined!
ERROR: "sst_configure_runtime_pm" [sound/soc/intel/atom/sst/snd-intel-sst-acpi.ko] undefined!

The problem is that the sound/soc/intel/atom/ directory only gets
entered by Kbuild when SND_SST_ATOM_HIFI2_PLATFORM is set, so we
only build modules (obj-m) under here but not built-in files (obj-y)
including the snd-intel-sst-core that we clearly want need here.

Before commit 4772c16ede52 ("ASoC: Intel: Kconfig: Simplify-clarify ACPI/PCI
dependencies"), this could not happen as all files in sound/soc/intel/atom/
depended on CONFIG_SND_SST_ATOM_HIFI2_PLATFORM anyway.

Slightly later, commit 05f4434bc130 ("ASoC: Intel: remove mfld_machine")
was added, which then removed the Merrifield machine code that happened
to be the only user of SND_SST_ATOM_HIFI2_PLATFORM_PCI. With that gone,
it appears that we can completely remove that symbol as well, along with
the SND_SST_IPC_PCI code and everything associated with it that is now
unused.

For clarify, this also removes the SND_SST_IPC_ACPI symbol that is now
synonymous with SND_SST_IPC, and links both into a single loadable module.

Fixes: 4772c16ede52 ("ASoC: Intel: Kconfig: Simplify-clarify ACPI/PCI dependencies")
Fixes: 05f4434bc130 ("ASoC: Intel: remove mfld_machine")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
I checked that this patch leads to a clean compilation, and that the
code I'm removing is only used for the now-obsolete Merrifield PCI ID.

If I got something else wrong, or you want a different fix, please
just send a replacement patch and treat this as a bug report.
---
 sound/soc/intel/Kconfig            |  27 +----
 sound/soc/intel/atom/sst/Makefile  |   6 +-
 sound/soc/intel/atom/sst/sst_pci.c | 209 -------------------------------------
 sound/soc/intel/common/sst-acpi.c  |   4 +-
 4 files changed, 5 insertions(+), 241 deletions(-)
 delete mode 100644 sound/soc/intel/atom/sst/sst_pci.c

-- 
2.9.0

Comments

Andy Shevchenko Jan. 22, 2018, 9:51 a.m. UTC | #1
On Sun, 2018-01-21 at 23:14 +0100, Arnd Bergmann wrote:
> In a configuration with SND_SST_ATOM_HIFI2_PLATFORM_PCI=y and

> SND_SST_ATOM_HIFI2_PLATFORM=m, we get this module link failure:

> 

> ERROR: "sst_context_init" [sound/soc/intel/atom/sst/snd-intel-sst-

> acpi.ko] undefined!

> ERROR: "sst_context_cleanup" [sound/soc/intel/atom/sst/snd-intel-sst-

> acpi.ko] undefined!

> ERROR: "sst_alloc_drv_context" [sound/soc/intel/atom/sst/snd-intel-

> sst-acpi.ko] undefined!

> ERROR: "intel_sst_pm" [sound/soc/intel/atom/sst/snd-intel-sst-acpi.ko] 

> undefined!

> ERROR: "sst_configure_runtime_pm" [sound/soc/intel/atom/sst/snd-intel-

> sst-acpi.ko] undefined!

> 

> The problem is that the sound/soc/intel/atom/ directory only gets

> entered by Kbuild when SND_SST_ATOM_HIFI2_PLATFORM is set, so we

> only build modules (obj-m) under here but not built-in files (obj-y)

> including the snd-intel-sst-core that we clearly want need here.

> 

> Before commit 4772c16ede52 ("ASoC: Intel: Kconfig: Simplify-clarify

> ACPI/PCI

> dependencies"), this could not happen as all files in

> sound/soc/intel/atom/

> depended on CONFIG_SND_SST_ATOM_HIFI2_PLATFORM anyway.

> 

> Slightly later, commit 05f4434bc130 ("ASoC: Intel: remove

> mfld_machine")

> was added, which then removed the Merrifield machine code that

> happened

> to be the only user of SND_SST_ATOM_HIFI2_PLATFORM_PCI. With that

> gone,


That's what I afraid of. Intel Merrifield *should* be there. What Vinod
did, AFAIU, is removal of Intel Medfield support, which is fine with me.

So, before this can go, we need to get confirmation from Vinod and
Pierre, that Merrifield still stays there.

> it appears that we can completely remove that symbol as well, along

> with

> the SND_SST_IPC_PCI code and everything associated with it that is now

> unused.

> 

> For clarify, this also removes the SND_SST_IPC_ACPI symbol that is now

> synonymous with SND_SST_IPC, and links both into a single loadable

> module.


> I checked that this patch leads to a clean compilation, and that the

> code I'm removing is only used for the now-obsolete Merrifield PCI ID.


NACK for this part.

> If I got something else wrong, or you want a different fix, please

> just send a replacement patch and treat this as a bug report.


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
Arnd Bergmann Jan. 22, 2018, 10:58 a.m. UTC | #2
On Mon, Jan 22, 2018 at 10:51 AM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Sun, 2018-01-21 at 23:14 +0100, Arnd Bergmann wrote:

>> In a configuration with SND_SST_ATOM_HIFI2_PLATFORM_PCI=y and

>> SND_SST_ATOM_HIFI2_PLATFORM=m, we get this module link failure:

>>

>> ERROR: "sst_context_init" [sound/soc/intel/atom/sst/snd-intel-sst-

>> acpi.ko] undefined!

>> ERROR: "sst_context_cleanup" [sound/soc/intel/atom/sst/snd-intel-sst-

>> acpi.ko] undefined!

>> ERROR: "sst_alloc_drv_context" [sound/soc/intel/atom/sst/snd-intel-

>> sst-acpi.ko] undefined!

>> ERROR: "intel_sst_pm" [sound/soc/intel/atom/sst/snd-intel-sst-acpi.ko]

>> undefined!

>> ERROR: "sst_configure_runtime_pm" [sound/soc/intel/atom/sst/snd-intel-

>> sst-acpi.ko] undefined!

>>

>> The problem is that the sound/soc/intel/atom/ directory only gets

>> entered by Kbuild when SND_SST_ATOM_HIFI2_PLATFORM is set, so we

>> only build modules (obj-m) under here but not built-in files (obj-y)

>> including the snd-intel-sst-core that we clearly want need here.

>>

>> Before commit 4772c16ede52 ("ASoC: Intel: Kconfig: Simplify-clarify

>> ACPI/PCI

>> dependencies"), this could not happen as all files in

>> sound/soc/intel/atom/

>> depended on CONFIG_SND_SST_ATOM_HIFI2_PLATFORM anyway.

>>

>> Slightly later, commit 05f4434bc130 ("ASoC: Intel: remove

>> mfld_machine")

>> was added, which then removed the Merrifield machine code that

>> happened

>> to be the only user of SND_SST_ATOM_HIFI2_PLATFORM_PCI. With that

>> gone,

>

> That's what I afraid of. Intel Merrifield *should* be there. What Vinod

> did, AFAIU, is removal of Intel Medfield support, which is fine with me.

>

> So, before this can go, we need to get confirmation from Vinod and

> Pierre, that Merrifield still stays there.


Ok, I see. Checking further, I see that SND_SST_ATOM_HIFI2_PLATFORM_PCI
cannot be built without SND_SST_ATOM_HIFI2_PLATFORM:

sound/soc/intel/atom/sst/sst_drv_interface.o: In function `sst_register':
sst_drv_interface.c:(.text+0xc3e): undefined reference to `sst_register_dsp'
sound/soc/intel/atom/sst/sst_drv_interface.o: In function `sst_unregister':
sst_drv_interface.c:(.text+0xc67): undefined reference to `sst_unregister_dsp'

How about this instead:

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index f2c9e8c5970a..16344bd24eb0 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -72,21 +72,8 @@ config SND_SOC_INTEL_BAYTRAIL
          for Baytrail Chromebooks but this option is now deprecated and is
          not recommended, use SND_SST_ATOM_HIFI2_PLATFORM instead.

-config SND_SST_ATOM_HIFI2_PLATFORM_PCI
-       tristate "PCI HiFi2 (Medfield, Merrifield) Platforms"
-       depends on X86 && PCI
-       select SND_SST_IPC_PCI
-       select SND_SOC_COMPRESS
-       help
-         If you have a Intel Medfield or Merrifield/Edison platform, then
-         enable this option by saying Y or m. Distros will typically not
-         enable this option: Medfield devices are not available to
-         developers and while Merrifield/Edison can run a mainline kernel with
-         limited functionality it will require a firmware file which
-         is not in the standard firmware tree
-
 config SND_SST_ATOM_HIFI2_PLATFORM
-       tristate "ACPI HiFi2 (Baytrail, Cherrytrail) Platforms"
+       tristate "ACPI HiFi2 (Baytrail, Cherrytrail, Merrifield) Platforms"
        depends on X86 && ACPI
        select SND_SST_IPC_ACPI
        select SND_SOC_COMPRESS
diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig
index d4e103615f51..c73b19292fda 100644
--- a/sound/soc/intel/boards/Kconfig
+++ b/sound/soc/intel/boards/Kconfig
@@ -159,6 +159,19 @@ config SND_SOC_INTEL_BYT_CHT_NOCODEC_MACH

          If unsure select "N".

+config SND_SOC_INTEL_MRFLD_MACH
+       tristate "Merrifield/Edison platform"
+       depends on X86_INTEL_LPSS && I2C && PCI
+       select SND_SST_IPC_PCI
+       help
+         This adds support for ASoC PCI driver for the Merrifield
+         (platform) used e.g. on Intel Edison.  If you have
+         Merrifield/Edison platform, then enable this option by saying
+         Y or m. Distros will typically not enable this option: while
+         Merrifield/Edison can run a mainline kernel with limited
+         functionality it will require a firmware file which is not in
+         the standard firmware tree.
+
 endif ## SND_SST_ATOM_HIFI2_PLATFORM

 if SND_SOC_INTEL_SKYLAKE
Andy Shevchenko Jan. 22, 2018, 11:39 a.m. UTC | #3
On Mon, 2018-01-22 at 11:58 +0100, Arnd Bergmann wrote:
> On Mon, Jan 22, 2018 at 10:51 AM, Andy Shevchenko

> <andriy.shevchenko@linux.intel.com> wrote:

> > On Sun, 2018-01-21 at 23:14 +0100, Arnd Bergmann wrote:


> > > Slightly later, commit 05f4434bc130 ("ASoC: Intel: remove

> > > mfld_machine")

> > > was added, which then removed the Merrifield machine code that

> > > happened

> > > to be the only user of SND_SST_ATOM_HIFI2_PLATFORM_PCI. With that

> > > gone,

> > 

> > That's what I afraid of. Intel Merrifield *should* be there. What

> > Vinod

> > did, AFAIU, is removal of Intel Medfield support, which is fine with

> > me.

> > 

> > So, before this can go, we need to get confirmation from Vinod and

> > Pierre, that Merrifield still stays there.

> 

> Ok, I see. Checking further, I see that

> SND_SST_ATOM_HIFI2_PLATFORM_PCI

> cannot be built without SND_SST_ATOM_HIFI2_PLATFORM:

> 

> sound/soc/intel/atom/sst/sst_drv_interface.o: In function

> `sst_register':

> sst_drv_interface.c:(.text+0xc3e): undefined reference to

> `sst_register_dsp'

> sound/soc/intel/atom/sst/sst_drv_interface.o: In function

> `sst_unregister':

> sst_drv_interface.c:(.text+0xc67): undefined reference to

> `sst_unregister_dsp'


Oh.

> 

> How about this instead:

> 

> diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig

> index f2c9e8c5970a..16344bd24eb0 100644

> --- a/sound/soc/intel/Kconfig

> +++ b/sound/soc/intel/Kconfig

> @@ -72,21 +72,8 @@ config SND_SOC_INTEL_BAYTRAIL

>           for Baytrail Chromebooks but this option is now deprecated

> and is

>           not recommended, use SND_SST_ATOM_HIFI2_PLATFORM instead.

> 

> -config SND_SST_ATOM_HIFI2_PLATFORM_PCI

> -       tristate "PCI HiFi2 (Medfield, Merrifield) Platforms"

> -       depends on X86 && PCI

> -       select SND_SST_IPC_PCI

> -       select SND_SOC_COMPRESS

> -       help

> -         If you have a Intel Medfield or Merrifield/Edison platform,

> then

> -         enable this option by saying Y or m. Distros will typically

> not

> -         enable this option: Medfield devices are not available to

> -         developers and while Merrifield/Edison can run a mainline

> kernel with

> -         limited functionality it will require a firmware file which

> -         is not in the standard firmware tree

> -

>  config SND_SST_ATOM_HIFI2_PLATFORM

> -       tristate "ACPI HiFi2 (Baytrail, Cherrytrail) Platforms"

> +       tristate "ACPI HiFi2 (Baytrail, Cherrytrail, Merrifield)

> Platforms"


Perhaps it makes sense to do something like _HIFI2 and on top
HIFI2_PLATFORM and HIFI2_PCI, but it seems like a current split.

So, it means the split itself is not accurate in the first place.
Pierre, Vinod?

> +config SND_SOC_INTEL_MRFLD_MACH

> +       tristate "Merrifield/Edison platform"


Edison should not be here (it's a board, while Merrifield is a platform)

> +       depends on X86_INTEL_LPSS && I2C && PCI


X86_INTEL_LPSS has nothing to do with Merrifield. :-)

> +       select SND_SST_IPC_PCI

> +       help

> +         This adds support for ASoC PCI driver for the Merrifield

> +         (platform) used e.g. on Intel Edison.  If you have

> +         Merrifield/Edison platform, then enable this option by

> saying

> +         Y or m. Distros will typically not enable this option: while

> +         Merrifield/Edison can run a mainline kernel with limited

> +         functionality it will require a firmware file which is not

> in

> +         the standard firmware tree.


Above looks like a solution to me, although I'm not familiar with ASoC
code, so, I would rely on Pierre, Vinod and Liam suggestions.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
Pierre-Louis Bossart Jan. 22, 2018, 4:16 p.m. UTC | #4
On 1/21/18 4:14 PM, Arnd Bergmann wrote:
> In a configuration with SND_SST_ATOM_HIFI2_PLATFORM_PCI=y and

> SND_SST_ATOM_HIFI2_PLATFORM=m, we get this module link failure:

> 

> ERROR: "sst_context_init" [sound/soc/intel/atom/sst/snd-intel-sst-acpi.ko] undefined!

> ERROR: "sst_context_cleanup" [sound/soc/intel/atom/sst/snd-intel-sst-acpi.ko] undefined!

> ERROR: "sst_alloc_drv_context" [sound/soc/intel/atom/sst/snd-intel-sst-acpi.ko] undefined!

> ERROR: "intel_sst_pm" [sound/soc/intel/atom/sst/snd-intel-sst-acpi.ko] undefined!

> ERROR: "sst_configure_runtime_pm" [sound/soc/intel/atom/sst/snd-intel-sst-acpi.ko] undefined!

> 

> The problem is that the sound/soc/intel/atom/ directory only gets

> entered by Kbuild when SND_SST_ATOM_HIFI2_PLATFORM is set, so we

> only build modules (obj-m) under here but not built-in files (obj-y)

> including the snd-intel-sst-core that we clearly want need here.

> 

> Before commit 4772c16ede52 ("ASoC: Intel: Kconfig: Simplify-clarify ACPI/PCI

> dependencies"), this could not happen as all files in sound/soc/intel/atom/

> depended on CONFIG_SND_SST_ATOM_HIFI2_PLATFORM anyway.

> 

> Slightly later, commit 05f4434bc130 ("ASoC: Intel: remove mfld_machine")

> was added, which then removed the Merrifield machine code that happened

> to be the only user of SND_SST_ATOM_HIFI2_PLATFORM_PCI. With that gone,

> it appears that we can completely remove that symbol as well, along with

> the SND_SST_IPC_PCI code and everything associated with it that is now

> unused.


there's a misunderstanding here. The Medfield code was removed. We want 
to keep the PCI stuff to support Merrifield aka Tangier/Edison - at 
least for now.

> For clarify, this also removes the SND_SST_IPC_ACPI symbol that is now

> synonymous with SND_SST_IPC, and links both into a single loadable module.

> 

> Fixes: 4772c16ede52 ("ASoC: Intel: Kconfig: Simplify-clarify ACPI/PCI dependencies")

> Fixes: 05f4434bc130 ("ASoC: Intel: remove mfld_machine")

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> ---

> I checked that this patch leads to a clean compilation, and that the

> code I'm removing is only used for the now-obsolete Merrifield PCI ID.

> 

> If I got something else wrong, or you want a different fix, please

> just send a replacement patch and treat this as a bug report.

> ---

>   sound/soc/intel/Kconfig            |  27 +----

>   sound/soc/intel/atom/sst/Makefile  |   6 +-

>   sound/soc/intel/atom/sst/sst_pci.c | 209 -------------------------------------

>   sound/soc/intel/common/sst-acpi.c  |   4 +-

>   4 files changed, 5 insertions(+), 241 deletions(-)

>   delete mode 100644 sound/soc/intel/atom/sst/sst_pci.c

> 

> diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig

> index f2c9e8c5970a..2dc8cda7a7cd 100644

> --- a/sound/soc/intel/Kconfig

> +++ b/sound/soc/intel/Kconfig

> @@ -17,21 +17,11 @@ if SND_SOC_INTEL_SST_TOPLEVEL

>   config SND_SST_IPC

>   	tristate

>   	# This option controls the IPC core for HiFi2 platforms

> -

> -config SND_SST_IPC_PCI

> -	tristate

> -	select SND_SST_IPC

> -	# This option controls the PCI-based IPC for HiFi2 platforms

> -	#  (Medfield, Merrifield).

> -

> -config SND_SST_IPC_ACPI

> -	tristate

> -	select SND_SST_IPC

> -	# This option controls the ACPI-based IPC for HiFi2 platforms

>   	# (Baytrail, Cherrytrail)

>   

>   config SND_SOC_INTEL_SST_ACPI

>   	tristate

> +	select SND_SST_IPC

>   	# This option controls ACPI-based probing on

>   	# Haswell/Broadwell/Baytrail legacy and will be set

>   	# when these platforms are enabled

> @@ -72,23 +62,10 @@ config SND_SOC_INTEL_BAYTRAIL

>   	  for Baytrail Chromebooks but this option is now deprecated and is

>   	  not recommended, use SND_SST_ATOM_HIFI2_PLATFORM instead.

>   

> -config SND_SST_ATOM_HIFI2_PLATFORM_PCI

> -	tristate "PCI HiFi2 (Medfield, Merrifield) Platforms"

> -	depends on X86 && PCI

> -	select SND_SST_IPC_PCI

> -	select SND_SOC_COMPRESS

> -	help

> -	  If you have a Intel Medfield or Merrifield/Edison platform, then

> -	  enable this option by saying Y or m. Distros will typically not

> -	  enable this option: Medfield devices are not available to

> -	  developers and while Merrifield/Edison can run a mainline kernel with

> -	  limited functionality it will require a firmware file which

> -	  is not in the standard firmware tree

> -

>   config SND_SST_ATOM_HIFI2_PLATFORM

>   	tristate "ACPI HiFi2 (Baytrail, Cherrytrail) Platforms"

>   	depends on X86 && ACPI

> -	select SND_SST_IPC_ACPI

> +	select SND_SST_IPC

>   	select SND_SOC_COMPRESS

>   	select SND_SOC_ACPI_INTEL_MATCH

>   	select IOSF_MBI

> diff --git a/sound/soc/intel/atom/sst/Makefile b/sound/soc/intel/atom/sst/Makefile

> index 795d1cf8f386..f5b7008b4cea 100644

> --- a/sound/soc/intel/atom/sst/Makefile

> +++ b/sound/soc/intel/atom/sst/Makefile

> @@ -1,8 +1,4 @@

>   # SPDX-License-Identifier: GPL-2.0

> -snd-intel-sst-core-objs := sst.o sst_ipc.o sst_stream.o sst_drv_interface.o sst_loader.o sst_pvt.o

> -snd-intel-sst-pci-objs += sst_pci.o

> -snd-intel-sst-acpi-objs += sst_acpi.o

> +snd-intel-sst-core-objs := sst.o sst_ipc.o sst_stream.o sst_drv_interface.o sst_loader.o sst_pvt.o sst_acpi.o

>   

>   obj-$(CONFIG_SND_SST_IPC) += snd-intel-sst-core.o

> -obj-$(CONFIG_SND_SST_IPC_PCI) += snd-intel-sst-pci.o

> -obj-$(CONFIG_SND_SST_IPC_ACPI) += snd-intel-sst-acpi.o

> diff --git a/sound/soc/intel/atom/sst/sst_pci.c b/sound/soc/intel/atom/sst/sst_pci.c

> deleted file mode 100644

> index 6906ee624cf6..000000000000

> --- a/sound/soc/intel/atom/sst/sst_pci.c

> +++ /dev/null

> @@ -1,209 +0,0 @@

> -/*

> - *  sst_pci.c - SST (LPE) driver init file for pci enumeration.

> - *

> - *  Copyright (C) 2008-14	Intel Corp

> - *  Authors:	Vinod Koul <vinod.koul@intel.com>

> - *		Harsha Priya <priya.harsha@intel.com>

> - *		Dharageswari R <dharageswari.r@intel.com>

> - *		KP Jeeja <jeeja.kp@intel.com>

> - *  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> - *

> - *  This program is free software; you can redistribute it and/or modify

> - *  it under the terms of the GNU General Public License as published by

> - *  the Free Software Foundation; version 2 of the License.

> - *

> - *  This program is distributed in the hope that it will be useful, but

> - *  WITHOUT ANY WARRANTY; without even the implied warranty of

> - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU

> - *  General Public License for more details.

> - *

> - * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> - */

> -#include <linux/module.h>

> -#include <linux/pci.h>

> -#include <linux/fs.h>

> -#include <linux/firmware.h>

> -#include <linux/pm_runtime.h>

> -#include <sound/core.h>

> -#include <sound/soc.h>

> -#include <asm/platform_sst_audio.h>

> -#include "../sst-mfld-platform.h"

> -#include "sst.h"

> -

> -static int sst_platform_get_resources(struct intel_sst_drv *ctx)

> -{

> -	int ddr_base, ret = 0;

> -	struct pci_dev *pci = ctx->pci;

> -

> -	ret = pci_request_regions(pci, SST_DRV_NAME);

> -	if (ret)

> -		return ret;

> -

> -	/* map registers */

> -	/* DDR base */

> -	if (ctx->dev_id == SST_MRFLD_PCI_ID) {

> -		ctx->ddr_base = pci_resource_start(pci, 0);

> -		/* check that the relocated IMR base matches with FW Binary */

> -		ddr_base = relocate_imr_addr_mrfld(ctx->ddr_base);

> -		if (!ctx->pdata->lib_info) {

> -			dev_err(ctx->dev, "lib_info pointer NULL\n");

> -			ret = -EINVAL;

> -			goto do_release_regions;

> -		}

> -		if (ddr_base != ctx->pdata->lib_info->mod_base) {

> -			dev_err(ctx->dev,

> -					"FW LSP DDR BASE does not match with IFWI\n");

> -			ret = -EINVAL;

> -			goto do_release_regions;

> -		}

> -		ctx->ddr_end = pci_resource_end(pci, 0);

> -

> -		ctx->ddr = pcim_iomap(pci, 0,

> -					pci_resource_len(pci, 0));

> -		if (!ctx->ddr) {

> -			ret = -EINVAL;

> -			goto do_release_regions;

> -		}

> -		dev_dbg(ctx->dev, "sst: DDR Ptr %p\n", ctx->ddr);

> -	} else {

> -		ctx->ddr = NULL;

> -	}

> -	/* SHIM */

> -	ctx->shim_phy_add = pci_resource_start(pci, 1);

> -	ctx->shim = pcim_iomap(pci, 1, pci_resource_len(pci, 1));

> -	if (!ctx->shim) {

> -		ret = -EINVAL;

> -		goto do_release_regions;

> -	}

> -	dev_dbg(ctx->dev, "SST Shim Ptr %p\n", ctx->shim);

> -

> -	/* Shared SRAM */

> -	ctx->mailbox_add = pci_resource_start(pci, 2);

> -	ctx->mailbox = pcim_iomap(pci, 2, pci_resource_len(pci, 2));

> -	if (!ctx->mailbox) {

> -		ret = -EINVAL;

> -		goto do_release_regions;

> -	}

> -	dev_dbg(ctx->dev, "SRAM Ptr %p\n", ctx->mailbox);

> -

> -	/* IRAM */

> -	ctx->iram_end = pci_resource_end(pci, 3);

> -	ctx->iram_base = pci_resource_start(pci, 3);

> -	ctx->iram = pcim_iomap(pci, 3, pci_resource_len(pci, 3));

> -	if (!ctx->iram) {

> -		ret = -EINVAL;

> -		goto do_release_regions;

> -	}

> -	dev_dbg(ctx->dev, "IRAM Ptr %p\n", ctx->iram);

> -

> -	/* DRAM */

> -	ctx->dram_end = pci_resource_end(pci, 4);

> -	ctx->dram_base = pci_resource_start(pci, 4);

> -	ctx->dram = pcim_iomap(pci, 4, pci_resource_len(pci, 4));

> -	if (!ctx->dram) {

> -		ret = -EINVAL;

> -		goto do_release_regions;

> -	}

> -	dev_dbg(ctx->dev, "DRAM Ptr %p\n", ctx->dram);

> -do_release_regions:

> -	pci_release_regions(pci);

> -	return 0;

> -}

> -

> -/*

> - * intel_sst_probe - PCI probe function

> - *

> - * @pci:	PCI device structure

> - * @pci_id: PCI device ID structure

> - *

> - */

> -static int intel_sst_probe(struct pci_dev *pci,

> -			const struct pci_device_id *pci_id)

> -{

> -	int ret = 0;

> -	struct intel_sst_drv *sst_drv_ctx;

> -	struct sst_platform_info *sst_pdata = pci->dev.platform_data;

> -

> -	dev_dbg(&pci->dev, "Probe for DID %x\n", pci->device);

> -	ret = sst_alloc_drv_context(&sst_drv_ctx, &pci->dev, pci->device);

> -	if (ret < 0)

> -		return ret;

> -

> -	sst_drv_ctx->pdata = sst_pdata;

> -	sst_drv_ctx->irq_num = pci->irq;

> -	snprintf(sst_drv_ctx->firmware_name, sizeof(sst_drv_ctx->firmware_name),

> -			"%s%04x%s", "fw_sst_",

> -			sst_drv_ctx->dev_id, ".bin");

> -

> -	ret = sst_context_init(sst_drv_ctx);

> -	if (ret < 0)

> -		return ret;

> -

> -	/* Init the device */

> -	ret = pcim_enable_device(pci);

> -	if (ret) {

> -		dev_err(sst_drv_ctx->dev,

> -			"device can't be enabled. Returned err: %d\n", ret);

> -		goto do_free_drv_ctx;

> -	}

> -	sst_drv_ctx->pci = pci_dev_get(pci);

> -	ret = sst_platform_get_resources(sst_drv_ctx);

> -	if (ret < 0)

> -		goto do_free_drv_ctx;

> -

> -	pci_set_drvdata(pci, sst_drv_ctx);

> -	sst_configure_runtime_pm(sst_drv_ctx);

> -

> -	return ret;

> -

> -do_free_drv_ctx:

> -	sst_context_cleanup(sst_drv_ctx);

> -	dev_err(sst_drv_ctx->dev, "Probe failed with %d\n", ret);

> -	return ret;

> -}

> -

> -/**

> - * intel_sst_remove - PCI remove function

> - *

> - * @pci:	PCI device structure

> - *

> - * This function is called by OS when a device is unloaded

> - * This frees the interrupt etc

> - */

> -static void intel_sst_remove(struct pci_dev *pci)

> -{

> -	struct intel_sst_drv *sst_drv_ctx = pci_get_drvdata(pci);

> -

> -	sst_context_cleanup(sst_drv_ctx);

> -	pci_dev_put(sst_drv_ctx->pci);

> -	pci_release_regions(pci);

> -	pci_set_drvdata(pci, NULL);

> -}

> -

> -/* PCI Routines */

> -static const struct pci_device_id intel_sst_ids[] = {

> -	{ PCI_VDEVICE(INTEL, SST_MRFLD_PCI_ID), 0},

> -	{ 0, }

> -};

> -

> -static struct pci_driver sst_driver = {

> -	.name = SST_DRV_NAME,

> -	.id_table = intel_sst_ids,

> -	.probe = intel_sst_probe,

> -	.remove = intel_sst_remove,

> -#ifdef CONFIG_PM

> -	.driver = {

> -		.pm = &intel_sst_pm,

> -	},

> -#endif

> -};

> -

> -module_pci_driver(sst_driver);

> -

> -MODULE_DESCRIPTION("Intel (R) SST(R) Audio Engine PCI Driver");

> -MODULE_AUTHOR("Vinod Koul <vinod.koul@intel.com>");

> -MODULE_AUTHOR("Harsha Priya <priya.harsha@intel.com>");

> -MODULE_AUTHOR("Dharageswari R <dharageswari.r@intel.com>");

> -MODULE_AUTHOR("KP Jeeja <jeeja.kp@intel.com>");

> -MODULE_LICENSE("GPL v2");

> -MODULE_ALIAS("sst");

> diff --git a/sound/soc/intel/common/sst-acpi.c b/sound/soc/intel/common/sst-acpi.c

> index cf6fbbd4e378..3b669d5adbb6 100644

> --- a/sound/soc/intel/common/sst-acpi.c

> +++ b/sound/soc/intel/common/sst-acpi.c

> @@ -206,7 +206,7 @@ static struct sst_acpi_desc sst_acpi_broadwell_desc = {

>   	.dma_size = SST_LPT_DSP_DMA_SIZE,

>   };

>   

> -#if !IS_ENABLED(CONFIG_SND_SST_IPC_ACPI)

> +#if !IS_ENABLED(CONFIG_SND_SST_IPC)

>   static struct sst_acpi_desc sst_acpi_baytrail_desc = {

>   	.drv_name = "baytrail-pcm-audio",

>   	.machines = snd_soc_acpi_intel_baytrail_legacy_machines,

> @@ -222,7 +222,7 @@ static struct sst_acpi_desc sst_acpi_baytrail_desc = {

>   static const struct acpi_device_id sst_acpi_match[] = {

>   	{ "INT33C8", (unsigned long)&sst_acpi_haswell_desc },

>   	{ "INT3438", (unsigned long)&sst_acpi_broadwell_desc },

> -#if !IS_ENABLED(CONFIG_SND_SST_IPC_ACPI)

> +#if !IS_ENABLED(CONFIG_SND_SST_IPC)

>   	{ "80860F28", (unsigned long)&sst_acpi_baytrail_desc },

>   #endif

>   	{ }

>
Pierre-Louis Bossart Jan. 22, 2018, 4:37 p.m. UTC | #5
On 1/22/18 5:39 AM, Andy Shevchenko wrote:
> On Mon, 2018-01-22 at 11:58 +0100, Arnd Bergmann wrote:

>> On Mon, Jan 22, 2018 at 10:51 AM, Andy Shevchenko

>> <andriy.shevchenko@linux.intel.com> wrote:

>>> On Sun, 2018-01-21 at 23:14 +0100, Arnd Bergmann wrote:

> 

>>>> Slightly later, commit 05f4434bc130 ("ASoC: Intel: remove

>>>> mfld_machine")

>>>> was added, which then removed the Merrifield machine code that

>>>> happened

>>>> to be the only user of SND_SST_ATOM_HIFI2_PLATFORM_PCI. With that

>>>> gone,

>>>

>>> That's what I afraid of. Intel Merrifield *should* be there. What

>>> Vinod

>>> did, AFAIU, is removal of Intel Medfield support, which is fine with

>>> me.

>>>

>>> So, before this can go, we need to get confirmation from Vinod and

>>> Pierre, that Merrifield still stays there.

>>

>> Ok, I see. Checking further, I see that

>> SND_SST_ATOM_HIFI2_PLATFORM_PCI

>> cannot be built without SND_SST_ATOM_HIFI2_PLATFORM:

>>

>> sound/soc/intel/atom/sst/sst_drv_interface.o: In function

>> `sst_register':

>> sst_drv_interface.c:(.text+0xc3e): undefined reference to

>> `sst_register_dsp'

>> sound/soc/intel/atom/sst/sst_drv_interface.o: In function

>> `sst_unregister':

>> sst_drv_interface.c:(.text+0xc67): undefined reference to

>> `sst_unregister_dsp'

> 

> Oh.

> 

>>

>> How about this instead:

>>

>> diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig

>> index f2c9e8c5970a..16344bd24eb0 100644

>> --- a/sound/soc/intel/Kconfig

>> +++ b/sound/soc/intel/Kconfig

>> @@ -72,21 +72,8 @@ config SND_SOC_INTEL_BAYTRAIL

>>            for Baytrail Chromebooks but this option is now deprecated

>> and is

>>            not recommended, use SND_SST_ATOM_HIFI2_PLATFORM instead.

>>

>> -config SND_SST_ATOM_HIFI2_PLATFORM_PCI

>> -       tristate "PCI HiFi2 (Medfield, Merrifield) Platforms"

>> -       depends on X86 && PCI

>> -       select SND_SST_IPC_PCI

>> -       select SND_SOC_COMPRESS

>> -       help

>> -         If you have a Intel Medfield or Merrifield/Edison platform,

>> then

>> -         enable this option by saying Y or m. Distros will typically

>> not

>> -         enable this option: Medfield devices are not available to

>> -         developers and while Merrifield/Edison can run a mainline

>> kernel with

>> -         limited functionality it will require a firmware file which

>> -         is not in the standard firmware tree

>> -

>>   config SND_SST_ATOM_HIFI2_PLATFORM

>> -       tristate "ACPI HiFi2 (Baytrail, Cherrytrail) Platforms"

>> +       tristate "ACPI HiFi2 (Baytrail, Cherrytrail, Merrifield)

>> Platforms"

> 

> Perhaps it makes sense to do something like _HIFI2 and on top

> HIFI2_PLATFORM and HIFI2_PCI, but it seems like a current split.

> 

> So, it means the split itself is not accurate in the first place.

> Pierre, Vinod?

> 

>> +config SND_SOC_INTEL_MRFLD_MACH

>> +       tristate "Merrifield/Edison platform"

> 

> Edison should not be here (it's a board, while Merrifield is a platform)

> 

>> +       depends on X86_INTEL_LPSS && I2C && PCI

> 

> X86_INTEL_LPSS has nothing to do with Merrifield. :-)

> 

>> +       select SND_SST_IPC_PCI

>> +       help

>> +         This adds support for ASoC PCI driver for the Merrifield

>> +         (platform) used e.g. on Intel Edison.  If you have

>> +         Merrifield/Edison platform, then enable this option by

>> saying

>> +         Y or m. Distros will typically not enable this option: while

>> +         Merrifield/Edison can run a mainline kernel with limited

>> +         functionality it will require a firmware file which is not

>> in

>> +         the standard firmware tree.

> 

> Above looks like a solution to me, although I'm not familiar with ASoC

> code, so, I would rely on Pierre, Vinod and Liam suggestions.


I'd suggest that we instead add SND_SST_ATOM_HIFI2_PLATFORM_ACPI (for 
symmetry with PCI) and keep the SND_SST_ATOM_HIFI2_PLATFORM as a common 
part to solve this coexistence.

e.g (untested - just idea)

config SND_SST_ATOM_HIFI2_PLATFORM_PCI
	tristate "PCI HiFi2 (Merrifield) Platforms"
	depends on X86 && PCI
	select SND_SST_IPC_PCI
	select SND_SST_ATOM_HIFI2_PLATFORM

config SND_SST_ATOM_HIFI2_PLATFORM_ACPI
	tristate "ACPI HiFi2 (Baytrail, Cherrytrail) Platforms"
	depends on X86 && ACPI
	select SND_SST_IPC_ACPI
	select SND_SST_ATOM_HIFI2_PLATFORM

config SND_SST_ATOM_HIFI2_PLATFORM
	tristate
	select SND_SOC_COMPRESS

That said changing names would break oldnoconfig so maybe something 
similar that just adds the common layer.
Arnd Bergmann Jan. 22, 2018, 8:59 p.m. UTC | #6
On Mon, Jan 22, 2018 at 5:37 PM, Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
> On 1/22/18 5:39 AM, Andy Shevchenko wrote:

>> On Mon, 2018-01-22 at 11:58 +0100, Arnd Bergmann wrote:

>>> On Mon, Jan 22, 2018 at 10:51 AM, Andy Shevchenko

>>> <andriy.shevchenko@linux.intel.com> wrote:

>>>> On Sun, 2018-01-21 at 23:14 +0100, Arnd Bergmann wrote:


>> Above looks like a solution to me, although I'm not familiar with ASoC

>> code, so, I would rely on Pierre, Vinod and Liam suggestions.

>

>

> I'd suggest that we instead add SND_SST_ATOM_HIFI2_PLATFORM_ACPI (for

> symmetry with PCI) and keep the SND_SST_ATOM_HIFI2_PLATFORM as a common part

> to solve this coexistence.

>

> e.g (untested - just idea)

>

> config SND_SST_ATOM_HIFI2_PLATFORM_PCI

>         tristate "PCI HiFi2 (Merrifield) Platforms"

>         depends on X86 && PCI

>         select SND_SST_IPC_PCI

>         select SND_SST_ATOM_HIFI2_PLATFORM

>

> config SND_SST_ATOM_HIFI2_PLATFORM_ACPI

>         tristate "ACPI HiFi2 (Baytrail, Cherrytrail) Platforms"

>         depends on X86 && ACPI

>         select SND_SST_IPC_ACPI

>         select SND_SST_ATOM_HIFI2_PLATFORM

>

> config SND_SST_ATOM_HIFI2_PLATFORM

>         tristate

>         select SND_SOC_COMPRESS

>

> That said changing names would break oldnoconfig so maybe something similar

> that just adds the common layer.


It sounds like a good idea, at least if it can be done without a
larger code rework.
For the new SND_SST_ATOM_HIFI2_PLATFORM_ACPI symbol, you could simply
make that 'default ACPI' to make at least 'oldconfig' and 'olddefconfig' work.

See https://pastebin.com/GKtkgW99 for my randconfig file for testing the
configuration that I hit. With the current state of linux-next, there are two
configurations that are broken AFAICT

- SND_SST_ATOM_HIFI2_PLATFORM_PCI=y && SND_SST_ATOM_HIFI2_PLATFORM=m
  (because of the Makefile thing I mentioned)

- SND_SST_ATOM_HIFI2_PLATFORM_PCI=y && SND_SST_ATOM_HIFI2_PLATFORM=n
  (because of missing symbols)

       Arnd
diff mbox series

Patch

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index f2c9e8c5970a..2dc8cda7a7cd 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -17,21 +17,11 @@  if SND_SOC_INTEL_SST_TOPLEVEL
 config SND_SST_IPC
 	tristate
 	# This option controls the IPC core for HiFi2 platforms
-
-config SND_SST_IPC_PCI
-	tristate
-	select SND_SST_IPC
-	# This option controls the PCI-based IPC for HiFi2 platforms
-	#  (Medfield, Merrifield).
-
-config SND_SST_IPC_ACPI
-	tristate
-	select SND_SST_IPC
-	# This option controls the ACPI-based IPC for HiFi2 platforms
 	# (Baytrail, Cherrytrail)
 
 config SND_SOC_INTEL_SST_ACPI
 	tristate
+	select SND_SST_IPC
 	# This option controls ACPI-based probing on
 	# Haswell/Broadwell/Baytrail legacy and will be set
 	# when these platforms are enabled
@@ -72,23 +62,10 @@  config SND_SOC_INTEL_BAYTRAIL
 	  for Baytrail Chromebooks but this option is now deprecated and is
 	  not recommended, use SND_SST_ATOM_HIFI2_PLATFORM instead.
 
-config SND_SST_ATOM_HIFI2_PLATFORM_PCI
-	tristate "PCI HiFi2 (Medfield, Merrifield) Platforms"
-	depends on X86 && PCI
-	select SND_SST_IPC_PCI
-	select SND_SOC_COMPRESS
-	help
-	  If you have a Intel Medfield or Merrifield/Edison platform, then
-	  enable this option by saying Y or m. Distros will typically not
-	  enable this option: Medfield devices are not available to
-	  developers and while Merrifield/Edison can run a mainline kernel with
-	  limited functionality it will require a firmware file which
-	  is not in the standard firmware tree
-
 config SND_SST_ATOM_HIFI2_PLATFORM
 	tristate "ACPI HiFi2 (Baytrail, Cherrytrail) Platforms"
 	depends on X86 && ACPI
-	select SND_SST_IPC_ACPI
+	select SND_SST_IPC
 	select SND_SOC_COMPRESS
 	select SND_SOC_ACPI_INTEL_MATCH
 	select IOSF_MBI
diff --git a/sound/soc/intel/atom/sst/Makefile b/sound/soc/intel/atom/sst/Makefile
index 795d1cf8f386..f5b7008b4cea 100644
--- a/sound/soc/intel/atom/sst/Makefile
+++ b/sound/soc/intel/atom/sst/Makefile
@@ -1,8 +1,4 @@ 
 # SPDX-License-Identifier: GPL-2.0
-snd-intel-sst-core-objs := sst.o sst_ipc.o sst_stream.o sst_drv_interface.o sst_loader.o sst_pvt.o
-snd-intel-sst-pci-objs += sst_pci.o
-snd-intel-sst-acpi-objs += sst_acpi.o
+snd-intel-sst-core-objs := sst.o sst_ipc.o sst_stream.o sst_drv_interface.o sst_loader.o sst_pvt.o sst_acpi.o
 
 obj-$(CONFIG_SND_SST_IPC) += snd-intel-sst-core.o
-obj-$(CONFIG_SND_SST_IPC_PCI) += snd-intel-sst-pci.o
-obj-$(CONFIG_SND_SST_IPC_ACPI) += snd-intel-sst-acpi.o
diff --git a/sound/soc/intel/atom/sst/sst_pci.c b/sound/soc/intel/atom/sst/sst_pci.c
deleted file mode 100644
index 6906ee624cf6..000000000000
--- a/sound/soc/intel/atom/sst/sst_pci.c
+++ /dev/null
@@ -1,209 +0,0 @@ 
-/*
- *  sst_pci.c - SST (LPE) driver init file for pci enumeration.
- *
- *  Copyright (C) 2008-14	Intel Corp
- *  Authors:	Vinod Koul <vinod.koul@intel.com>
- *		Harsha Priya <priya.harsha@intel.com>
- *		Dharageswari R <dharageswari.r@intel.com>
- *		KP Jeeja <jeeja.kp@intel.com>
- *  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; version 2 of the License.
- *
- *  This program is distributed in the hope that it will be useful, but
- *  WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- *  General Public License for more details.
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- */
-#include <linux/module.h>
-#include <linux/pci.h>
-#include <linux/fs.h>
-#include <linux/firmware.h>
-#include <linux/pm_runtime.h>
-#include <sound/core.h>
-#include <sound/soc.h>
-#include <asm/platform_sst_audio.h>
-#include "../sst-mfld-platform.h"
-#include "sst.h"
-
-static int sst_platform_get_resources(struct intel_sst_drv *ctx)
-{
-	int ddr_base, ret = 0;
-	struct pci_dev *pci = ctx->pci;
-
-	ret = pci_request_regions(pci, SST_DRV_NAME);
-	if (ret)
-		return ret;
-
-	/* map registers */
-	/* DDR base */
-	if (ctx->dev_id == SST_MRFLD_PCI_ID) {
-		ctx->ddr_base = pci_resource_start(pci, 0);
-		/* check that the relocated IMR base matches with FW Binary */
-		ddr_base = relocate_imr_addr_mrfld(ctx->ddr_base);
-		if (!ctx->pdata->lib_info) {
-			dev_err(ctx->dev, "lib_info pointer NULL\n");
-			ret = -EINVAL;
-			goto do_release_regions;
-		}
-		if (ddr_base != ctx->pdata->lib_info->mod_base) {
-			dev_err(ctx->dev,
-					"FW LSP DDR BASE does not match with IFWI\n");
-			ret = -EINVAL;
-			goto do_release_regions;
-		}
-		ctx->ddr_end = pci_resource_end(pci, 0);
-
-		ctx->ddr = pcim_iomap(pci, 0,
-					pci_resource_len(pci, 0));
-		if (!ctx->ddr) {
-			ret = -EINVAL;
-			goto do_release_regions;
-		}
-		dev_dbg(ctx->dev, "sst: DDR Ptr %p\n", ctx->ddr);
-	} else {
-		ctx->ddr = NULL;
-	}
-	/* SHIM */
-	ctx->shim_phy_add = pci_resource_start(pci, 1);
-	ctx->shim = pcim_iomap(pci, 1, pci_resource_len(pci, 1));
-	if (!ctx->shim) {
-		ret = -EINVAL;
-		goto do_release_regions;
-	}
-	dev_dbg(ctx->dev, "SST Shim Ptr %p\n", ctx->shim);
-
-	/* Shared SRAM */
-	ctx->mailbox_add = pci_resource_start(pci, 2);
-	ctx->mailbox = pcim_iomap(pci, 2, pci_resource_len(pci, 2));
-	if (!ctx->mailbox) {
-		ret = -EINVAL;
-		goto do_release_regions;
-	}
-	dev_dbg(ctx->dev, "SRAM Ptr %p\n", ctx->mailbox);
-
-	/* IRAM */
-	ctx->iram_end = pci_resource_end(pci, 3);
-	ctx->iram_base = pci_resource_start(pci, 3);
-	ctx->iram = pcim_iomap(pci, 3, pci_resource_len(pci, 3));
-	if (!ctx->iram) {
-		ret = -EINVAL;
-		goto do_release_regions;
-	}
-	dev_dbg(ctx->dev, "IRAM Ptr %p\n", ctx->iram);
-
-	/* DRAM */
-	ctx->dram_end = pci_resource_end(pci, 4);
-	ctx->dram_base = pci_resource_start(pci, 4);
-	ctx->dram = pcim_iomap(pci, 4, pci_resource_len(pci, 4));
-	if (!ctx->dram) {
-		ret = -EINVAL;
-		goto do_release_regions;
-	}
-	dev_dbg(ctx->dev, "DRAM Ptr %p\n", ctx->dram);
-do_release_regions:
-	pci_release_regions(pci);
-	return 0;
-}
-
-/*
- * intel_sst_probe - PCI probe function
- *
- * @pci:	PCI device structure
- * @pci_id: PCI device ID structure
- *
- */
-static int intel_sst_probe(struct pci_dev *pci,
-			const struct pci_device_id *pci_id)
-{
-	int ret = 0;
-	struct intel_sst_drv *sst_drv_ctx;
-	struct sst_platform_info *sst_pdata = pci->dev.platform_data;
-
-	dev_dbg(&pci->dev, "Probe for DID %x\n", pci->device);
-	ret = sst_alloc_drv_context(&sst_drv_ctx, &pci->dev, pci->device);
-	if (ret < 0)
-		return ret;
-
-	sst_drv_ctx->pdata = sst_pdata;
-	sst_drv_ctx->irq_num = pci->irq;
-	snprintf(sst_drv_ctx->firmware_name, sizeof(sst_drv_ctx->firmware_name),
-			"%s%04x%s", "fw_sst_",
-			sst_drv_ctx->dev_id, ".bin");
-
-	ret = sst_context_init(sst_drv_ctx);
-	if (ret < 0)
-		return ret;
-
-	/* Init the device */
-	ret = pcim_enable_device(pci);
-	if (ret) {
-		dev_err(sst_drv_ctx->dev,
-			"device can't be enabled. Returned err: %d\n", ret);
-		goto do_free_drv_ctx;
-	}
-	sst_drv_ctx->pci = pci_dev_get(pci);
-	ret = sst_platform_get_resources(sst_drv_ctx);
-	if (ret < 0)
-		goto do_free_drv_ctx;
-
-	pci_set_drvdata(pci, sst_drv_ctx);
-	sst_configure_runtime_pm(sst_drv_ctx);
-
-	return ret;
-
-do_free_drv_ctx:
-	sst_context_cleanup(sst_drv_ctx);
-	dev_err(sst_drv_ctx->dev, "Probe failed with %d\n", ret);
-	return ret;
-}
-
-/**
- * intel_sst_remove - PCI remove function
- *
- * @pci:	PCI device structure
- *
- * This function is called by OS when a device is unloaded
- * This frees the interrupt etc
- */
-static void intel_sst_remove(struct pci_dev *pci)
-{
-	struct intel_sst_drv *sst_drv_ctx = pci_get_drvdata(pci);
-
-	sst_context_cleanup(sst_drv_ctx);
-	pci_dev_put(sst_drv_ctx->pci);
-	pci_release_regions(pci);
-	pci_set_drvdata(pci, NULL);
-}
-
-/* PCI Routines */
-static const struct pci_device_id intel_sst_ids[] = {
-	{ PCI_VDEVICE(INTEL, SST_MRFLD_PCI_ID), 0},
-	{ 0, }
-};
-
-static struct pci_driver sst_driver = {
-	.name = SST_DRV_NAME,
-	.id_table = intel_sst_ids,
-	.probe = intel_sst_probe,
-	.remove = intel_sst_remove,
-#ifdef CONFIG_PM
-	.driver = {
-		.pm = &intel_sst_pm,
-	},
-#endif
-};
-
-module_pci_driver(sst_driver);
-
-MODULE_DESCRIPTION("Intel (R) SST(R) Audio Engine PCI Driver");
-MODULE_AUTHOR("Vinod Koul <vinod.koul@intel.com>");
-MODULE_AUTHOR("Harsha Priya <priya.harsha@intel.com>");
-MODULE_AUTHOR("Dharageswari R <dharageswari.r@intel.com>");
-MODULE_AUTHOR("KP Jeeja <jeeja.kp@intel.com>");
-MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("sst");
diff --git a/sound/soc/intel/common/sst-acpi.c b/sound/soc/intel/common/sst-acpi.c
index cf6fbbd4e378..3b669d5adbb6 100644
--- a/sound/soc/intel/common/sst-acpi.c
+++ b/sound/soc/intel/common/sst-acpi.c
@@ -206,7 +206,7 @@  static struct sst_acpi_desc sst_acpi_broadwell_desc = {
 	.dma_size = SST_LPT_DSP_DMA_SIZE,
 };
 
-#if !IS_ENABLED(CONFIG_SND_SST_IPC_ACPI)
+#if !IS_ENABLED(CONFIG_SND_SST_IPC)
 static struct sst_acpi_desc sst_acpi_baytrail_desc = {
 	.drv_name = "baytrail-pcm-audio",
 	.machines = snd_soc_acpi_intel_baytrail_legacy_machines,
@@ -222,7 +222,7 @@  static struct sst_acpi_desc sst_acpi_baytrail_desc = {
 static const struct acpi_device_id sst_acpi_match[] = {
 	{ "INT33C8", (unsigned long)&sst_acpi_haswell_desc },
 	{ "INT3438", (unsigned long)&sst_acpi_broadwell_desc },
-#if !IS_ENABLED(CONFIG_SND_SST_IPC_ACPI)
+#if !IS_ENABLED(CONFIG_SND_SST_IPC)
 	{ "80860F28", (unsigned long)&sst_acpi_baytrail_desc },
 #endif
 	{ }