Re: [alsa-devel] [PATCH] ASoC: SOF: imx: fix reverse CONFIG_SND_SOC_SOF_OF dependency - Pierre-Louis Bossart

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Mark Brown <broonie@kernel.org>, Hulk Robot <hulkci@huawei.com>,
	YueHaibing <yuehaibing@huawei.com>,
	Daniel Baluta <daniel.baluta@nxp.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	ALSA Development Mailing List <alsa-devel@alsa-project.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [alsa-devel] [PATCH] ASoC: SOF: imx: fix reverse CONFIG_SND_SOC_SOF_OF dependency
Date: Tue, 1 Oct 2019 11:03:45 -0500	[thread overview]
Message-ID: <e4b90233-846c-bfc1-68a3-a7b7c28b60bd@linux.intel.com> (raw)
In-Reply-To: <CAK8P3a3Js2dNhnRhP7PLadWZ69DZr1mz6DowN9HDJL4CFDAAFw@mail.gmail.com>

On 10/1/19 10:41 AM, Arnd Bergmann wrote:
> On Tue, Oct 1, 2019 at 5:32 PM Pierre-Louis Bossart
> <pierre-louis.bossart@linux.intel.com> wrote:
>> On 10/1/19 9:20 AM, Arnd Bergmann wrote:
>>> CONFIG_SND_SOC_SOF_IMX depends on CONFIG_SND_SOC_SOF, but is in
>>> turn referenced by the sof-of-dev driver. This creates a reverse
>>> dependency that manifests in a link error when CONFIG_SND_SOC_SOF_OF
>>> is built-in but CONFIG_SND_SOC_SOF_IMX=m:
>>>
>>> sound/soc/sof/sof-of-dev.o:(.data+0x118): undefined reference to `sof_imx8_ops'
>>>
>>> Make the latter a 'bool' symbol and change the Makefile so the imx8
>>> driver is compiled the same way as the driver using it.
>>>
>>> A nicer way would be to reverse the layering and move all
>>> the imx specific bits of sof-of-dev.c into the imx driver
>>> itself, which can then call into the common code. Doing this
>>> would need more testing and can be done if we add another
>>> driver like the first one.
>>
>> Or use something like
>>
>> config SND_SOC_SOF_IMX8_SUPPORT
>>          bool "SOF support for i.MX8"
>>          depends on IMX_SCU
>>          depends on IMX_DSP
>>
>> config SND_SOC_SOF_IMX8
>>          tristate
>>          <i.mx selects>
>>
>> config SND_SOC_SOF_OF
>>          depends on OF
>>          select SND_SOC_SOF_IMX8 if SND_SOC_SOF_IMX8_SUPPORT
>>
>> That way you propagate the module/built-in information. That's how we
>> fixed those issues for the Intel parts.
> 
> Yes, I think that would work here as well, but it keeps even more
> information about the specific drivers in the generic code. It also
> requires adding more 'select' statements that tend to cause more
> problems.
> 
> The same could be done with a Kconfig-only solution avoiding
> 'select' such as:
> 
> config SND_SOC_SOF_IMX8_SUPPORT
>           bool "SOF support for i.MX8"
>           depends on IMX_SCU
>           depends on IMX_DSP
> 
>   config SND_SOC_SOF_IMX8
>           def_tristate SND_SOC_SOF_OF
>           depends on SND_SOC_SOF_IMX8_SUPPORT

Ah, nice, thanks for the suggestion! That would be my preference, we 
have a similar select for PCI and ACPI parts in sound/soc/sof/Kconfig 
and I was looking for a means to do this more elegantly.
I can submit a new fix or let you sent a v2, whatever is more convenient.