* Re: [PATCH][RESEND] ASoC: Intel: atom: fix ACPI/PCI Kconfig
2018-04-02 17:06 [PATCH][RESEND] ASoC: Intel: atom: fix ACPI/PCI Kconfig Pierre-Louis Bossart
@ 2018-04-02 17:28 ` Andy Shevchenko
2018-04-03 4:55 ` Vinod Koul
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2018-04-02 17:28 UTC (permalink / raw)
To: Pierre-Louis Bossart, alsa-devel
Cc: Arnd Bergmann, Mark Brown, liam.r.girdwood, Vinod Koul,
linux-kernel, Takashi Iwai, Harsha Priya N, Naveen M,
Daniel Drake
On Mon, 2018-04-02 at 12:06 -0500, Pierre-Louis Bossart wrote:
> The split between ACPI and PCI platforms generated issues with
> randconfig:
>
> 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!
>
> To keep things simple, let's expose two configs for
> SND_SST_ATOM_HIFI2_PLATFORM_PCI and SND_SST_ATOM_HIFI2_PLATFORM_ACPI,
> which select a common SND_SST_ATOM_HIFI2_PLATFORM option. To avoid
> breaking existing solutions with the semantics change,
> SND_SST_ATOM_HIFI2_PLATFORM_ACPI uses "default ACPI" so that "make
> oldnoconfig" and "make olddefconfig" still work as expected.
>
> Also remove mentions of Medfield while we are at it since it was
> removed recently.
>
Assuming we will see no more kbuild bot complains,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 4772c16ede52 ("ASoC: Intel: Kconfig: Simplify-clarify ACPI/PCI
> dependencies")
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.
> com>
> ---
> already sent on January 23 but apparently missed
>
>
> sound/soc/intel/Kconfig | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
> index ceb105cbd461..addac2a8e52a 100644
> --- a/sound/soc/intel/Kconfig
> +++ b/sound/soc/intel/Kconfig
> @@ -72,24 +72,28 @@ 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
> + tristate
> + select SND_SOC_COMPRESS
> +
> config SND_SST_ATOM_HIFI2_PLATFORM_PCI
> - tristate "PCI HiFi2 (Medfield, Merrifield) Platforms"
> + tristate "PCI HiFi2 (Merrifield) Platforms"
> depends on X86 && PCI
> select SND_SST_IPC_PCI
> - select SND_SOC_COMPRESS
> + select SND_SST_ATOM_HIFI2_PLATFORM
> help
> - If you have a Intel Medfield or Merrifield/Edison platform,
> then
> + If you have a Intel 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
> + 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
>
> -config SND_SST_ATOM_HIFI2_PLATFORM
> +config SND_SST_ATOM_HIFI2_PLATFORM_ACPI
> tristate "ACPI HiFi2 (Baytrail, Cherrytrail) Platforms"
> + default ACPI
> depends on X86 && ACPI
> select SND_SST_IPC_ACPI
> - select SND_SOC_COMPRESS
> + select SND_SST_ATOM_HIFI2_PLATFORM
> select SND_SOC_ACPI_INTEL_MATCH
> select IOSF_MBI
> help
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH][RESEND] ASoC: Intel: atom: fix ACPI/PCI Kconfig
2018-04-02 17:06 [PATCH][RESEND] ASoC: Intel: atom: fix ACPI/PCI Kconfig Pierre-Louis Bossart
2018-04-02 17:28 ` Andy Shevchenko
@ 2018-04-03 4:55 ` Vinod Koul
2018-04-06 20:05 ` Sasha Levin
2018-04-25 18:15 ` Takashi Iwai
3 siblings, 0 replies; 5+ messages in thread
From: Vinod Koul @ 2018-04-03 4:55 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: alsa-devel, Arnd Bergmann, Mark Brown, Andy Shevchenko,
liam.r.girdwood, linux-kernel, Takashi Iwai, Harsha Priya N,
Naveen M, Daniel Drake
On Mon, Apr 02, 2018 at 12:06:14PM -0500, Pierre-Louis Bossart wrote:
> The split between ACPI and PCI platforms generated issues with randconfig:
>
> 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!
>
> To keep things simple, let's expose two configs for
> SND_SST_ATOM_HIFI2_PLATFORM_PCI and SND_SST_ATOM_HIFI2_PLATFORM_ACPI,
> which select a common SND_SST_ATOM_HIFI2_PLATFORM option. To avoid
> breaking existing solutions with the semantics change,
> SND_SST_ATOM_HIFI2_PLATFORM_ACPI uses "default ACPI" so that "make
> oldnoconfig" and "make olddefconfig" still work as expected.
>
> Also remove mentions of Medfield while we are at it since it was
> removed recently.
Acked-By: Vinod Koul <vinod.koul@intel.com>
--
~Vinod
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][RESEND] ASoC: Intel: atom: fix ACPI/PCI Kconfig
2018-04-02 17:06 [PATCH][RESEND] ASoC: Intel: atom: fix ACPI/PCI Kconfig Pierre-Louis Bossart
2018-04-02 17:28 ` Andy Shevchenko
2018-04-03 4:55 ` Vinod Koul
@ 2018-04-06 20:05 ` Sasha Levin
2018-04-25 18:15 ` Takashi Iwai
3 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2018-04-06 20:05 UTC (permalink / raw)
To: Sasha Levin, Pierre-Louis Bossart, alsa-devel@alsa-project.org
Cc: Arnd Bergmann, Mark Brown, stable@vger.kernel.org
Hi,
[This is an automated email]
This commit has been processed because it contains a "Fixes:" tag,
fixing commit: 4772c16ede52 ASoC: Intel: Kconfig: Simplify-clarify ACPI/PCI dependencies.
The bot has also determined it's probably a bug fixing patch. (score: 3.8189)
The bot has tested the following trees: v4.16.
v4.16: Build OK!
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][RESEND] ASoC: Intel: atom: fix ACPI/PCI Kconfig
2018-04-02 17:06 [PATCH][RESEND] ASoC: Intel: atom: fix ACPI/PCI Kconfig Pierre-Louis Bossart
` (2 preceding siblings ...)
2018-04-06 20:05 ` Sasha Levin
@ 2018-04-25 18:15 ` Takashi Iwai
3 siblings, 0 replies; 5+ messages in thread
From: Takashi Iwai @ 2018-04-25 18:15 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: alsa-devel, Arnd Bergmann, Daniel Drake, Harsha Priya N, Naveen M,
Vinod Koul, Mark Brown, Andy Shevchenko, liam.r.girdwood,
linux-kernel
On Mon, 02 Apr 2018 19:06:14 +0200,
Pierre-Louis Bossart wrote:
>
> The split between ACPI and PCI platforms generated issues with randconfig:
>
> 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!
>
> To keep things simple, let's expose two configs for
> SND_SST_ATOM_HIFI2_PLATFORM_PCI and SND_SST_ATOM_HIFI2_PLATFORM_ACPI,
> which select a common SND_SST_ATOM_HIFI2_PLATFORM option. To avoid
> breaking existing solutions with the semantics change,
> SND_SST_ATOM_HIFI2_PLATFORM_ACPI uses "default ACPI" so that "make
> oldnoconfig" and "make olddefconfig" still work as expected.
So now it reached to my tree, and noticed this "default ACPI".
After reading the patch description, I understand the reason behind
it, but still I'd say this would confuse users. For example, I was
quite surprised and almost proceeded to build this unnecessary just
because of the expectation to be default "N" in a standard config.
The distros would enable in anyway, so you don't have to care much.
The question is which target should we satisfy more: users who don't
need to turn this on, or users who need this. In probability, I'd bet
the former :)
Takashi
^ permalink raw reply [flat|nested] 5+ messages in thread