[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9a0eba2e-9933-43bb-ab3b-0480bf1d34a4@linux.intel.com>
Date: Thu, 18 Jul 2024 11:12:24 +0200
From: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To: Takashi Iwai <tiwai@...e.de>
Cc: Nathan Chancellor <nathan@...nel.org>, kernel test robot <lkp@...el.com>,
llvm@...ts.linux.dev, oe-kbuild-all@...ts.linux.dev,
linux-kernel@...r.kernel.org, Kai Vehmanen <kai.vehmanen@...ux.intel.com>,
Guennadi Liakhovetski <guennadi.liakhovetski@...ux.intel.com>,
Bard Liao <bard.liao@...el.com>
Subject: Re: ld.lld: error: undefined symbol: iosf_mbi_available
>>> --- a/sound/soc/sof/intel/Kconfig
>>> +++ b/sound/soc/sof/intel/Kconfig
>>> @@ -19,6 +19,7 @@ config SND_SOC_SOF_INTEL_ATOM_HIFI_EP
>>> tristate
>>> select SND_SOC_SOF_INTEL_COMMON
>>> select SND_SOC_SOF_INTEL_HIFI_EP_IPC
>>> + select IOSF_MBI if X86 && PCI
>>> help
>>> This option is not user-selectable but automagically handled by
>>> 'select' statements at a higher level.
>>> @@ -44,7 +45,6 @@ config SND_SOC_SOF_BAYTRAIL
>>> select SND_SOC_SOF_INTEL_COMMON
>>> select SND_SOC_SOF_INTEL_ATOM_HIFI_EP
>>> select SND_SOC_SOF_ACPI_DEV
>>> - select IOSF_MBI if X86 && PCI
>>> help
>>> This adds support for Sound Open Firmware for Intel(R) platforms
>>> using the Baytrail, Braswell or Cherrytrail processors.
>>
>> I don't think it's the 'right' fix Takashi.
>>
>> The problem is that we end-up using the iosf_mbi_read() routine by
>> including the soc-intel-quirks.h header file blindly for all X66
>> platforms - even when Baytrail is not used.
>>
>> Adding IOSF support for Tangiger doesn't seem right to me, it's not a
>> real dependency.
>>
>> We can be more restrictive and only use the helper for Baytrail, and use
>> a fallback if Baytrail is not used.
>>
>> diff --git a/sound/soc/intel/common/soc-intel-quirks.h
>> b/sound/soc/intel/common/soc-intel-quirks.h
>> index de4e550c5b34..ae67853f7e2e 100644
>> --- a/sound/soc/intel/common/soc-intel-quirks.h
>> +++ b/sound/soc/intel/common/soc-intel-quirks.h
>> @@ -11,7 +11,9 @@
>>
>> #include <linux/platform_data/x86/soc.h>
>>
>> -#if IS_ENABLED(CONFIG_X86)
>> +#if IS_ENABLED(CONFIG_X86) && \
>> + (IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL) || \
>> + IS_ENABLED(CONFIG_SND_SST_ATOM_HIFI2_PLATFORM_ACPI))
>>
>> #include <linux/dmi.h>
>> #include <asm/iosf_mbi.h>
>>
>> also at https://github.com/thesofproject/linux/pull/5114
>
> I'm afraid it's not enough, either. It's included in
> sound/soc/sof/intel/atom.c, and this one can be built-in by selected
> from others while CONFIG_SND_SOC_SOF_BAYTRAIL=m. And, the reverse
> selection is done from CONFIG_SND_SOC_SOF_BAYTRAIL -- so
> CONFIG_IOSF_MBI can be m as well, and this can lead to the unresolved
> symbol from the built-in atom.c.
Fair point, I was only looking at the reported failure where Baytrail
was completely disabled.
I am not sure though if it makes sense to split hair in N dimensions.
Building Merrifield as y and Baytrail as m is a corner case that
shouldn't exist at all. And it's only an academic compilation issue, in
practice using 'y' would fail at run-time due to the usual firmware load
dependencies...
Powered by blists - more mailing lists