Re: [PATCH] ASoC: intel: clean up CONFIG_SND_SST_IPC handling

[Andy Shevchenko]

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@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
Intel Finland Oy