| Message ID | 20190510023657.8960-1-yuehaibing@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: SOF: Fix build error with CONFIG_SND_SOC_SOF_NOCODEC=m | expand |
On Fri, 10 May 2019 04:36:57 +0200, YueHaibing wrote: > > Fix gcc build error while CONFIG_SND_SOC_SOF_NOCODEC=m > > sound/soc/sof/core.o: In function `snd_sof_device_probe': > core.c:(.text+0x4af): undefined reference to `sof_nocodec_setup' > > Change SND_SOC_SOF_NOCODEC to bool to fix this. > > Reported-by: Hulk Robot <hulkci@huawei.com> > Fixes: c16211d6226d ("ASoC: SOF: Add Sound Open Firmware driver core") > Signed-off-by: YueHaibing <yuehaibing@huawei.com> This change would break things severely. This won't allow to build it as a module any longer. A better fix would be to somehow restrict the SND_SOC_SOF_NOCODEC to align with SND_SOC_SOF, i.e. disallow SND_SOC_SOF=y && SND_SOC_SOF_NOCODEC=m. Because of the complex mix of select and depends-on in SOF, I'm afraid that it's not that trivial, though. There might be something I overlooked, hopefully... An easier alternative would be to replace IS_ENABLED(CONFIG_SND_SOC_SOF_NOCODEC) with IS_REACHABLE(CONFIG_SND_SOC_SOF_NOCODEC). This assures the condition at the build time, although the error at probe might be a surprising to some users that don't know this hidden dependency. thanks, Takashi > --- > sound/soc/sof/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig > index b204c65..9c280c9 100644 > --- a/sound/soc/sof/Kconfig > +++ b/sound/soc/sof/Kconfig > @@ -44,7 +44,7 @@ config SND_SOC_SOF_OPTIONS > if SND_SOC_SOF_OPTIONS > > config SND_SOC_SOF_NOCODEC > - tristate "SOF nocodec mode Support" > + bool "SOF nocodec mode Support" > help > This adds support for a dummy/nocodec machine driver fallback > option if no known codec is detected. This is typically only > -- > 2.7.4 > > >
On 5/10/19 2:12 AM, Takashi Iwai wrote: > On Fri, 10 May 2019 04:36:57 +0200, > YueHaibing wrote: >> >> Fix gcc build error while CONFIG_SND_SOC_SOF_NOCODEC=m >> >> sound/soc/sof/core.o: In function `snd_sof_device_probe': >> core.c:(.text+0x4af): undefined reference to `sof_nocodec_setup' >> >> Change SND_SOC_SOF_NOCODEC to bool to fix this. >> >> Reported-by: Hulk Robot <hulkci@huawei.com> >> Fixes: c16211d6226d ("ASoC: SOF: Add Sound Open Firmware driver core") >> Signed-off-by: YueHaibing <yuehaibing@huawei.com> > > This change would break things severely. This won't allow to build it > as a module any longer. Isn't this fixed already? See the patch 'ASoC: SOF: core: fix undefined nocodec reference' and Takashi's follow-up to fix the unused variable warning. > > A better fix would be to somehow restrict the SND_SOC_SOF_NOCODEC to > align with SND_SOC_SOF, i.e. disallow SND_SOC_SOF=y && > SND_SOC_SOF_NOCODEC=m. Because of the complex mix of select and > depends-on in SOF, I'm afraid that it's not that trivial, though. > There might be something I overlooked, hopefully... > > An easier alternative would be to replace > IS_ENABLED(CONFIG_SND_SOC_SOF_NOCODEC) with > IS_REACHABLE(CONFIG_SND_SOC_SOF_NOCODEC). This assures the condition > at the build time, although the error at probe might be a surprising > to some users that don't know this hidden dependency. > > > thanks, > > Takashi > > >> --- >> sound/soc/sof/Kconfig | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig >> index b204c65..9c280c9 100644 >> --- a/sound/soc/sof/Kconfig >> +++ b/sound/soc/sof/Kconfig >> @@ -44,7 +44,7 @@ config SND_SOC_SOF_OPTIONS >> if SND_SOC_SOF_OPTIONS >> >> config SND_SOC_SOF_NOCODEC >> - tristate "SOF nocodec mode Support" >> + bool "SOF nocodec mode Support" >> help >> This adds support for a dummy/nocodec machine driver fallback >> option if no known codec is detected. This is typically only >> -- >> 2.7.4 >> >> >>
On Fri, 10 May 2019 14:56:29 +0200, Pierre-Louis Bossart wrote: > > On 5/10/19 2:12 AM, Takashi Iwai wrote: > > On Fri, 10 May 2019 04:36:57 +0200, > > YueHaibing wrote: > >> > >> Fix gcc build error while CONFIG_SND_SOC_SOF_NOCODEC=m > >> > >> sound/soc/sof/core.o: In function `snd_sof_device_probe': > >> core.c:(.text+0x4af): undefined reference to `sof_nocodec_setup' > >> > >> Change SND_SOC_SOF_NOCODEC to bool to fix this. > >> > >> Reported-by: Hulk Robot <hulkci@huawei.com> > >> Fixes: c16211d6226d ("ASoC: SOF: Add Sound Open Firmware driver core") > >> Signed-off-by: YueHaibing <yuehaibing@huawei.com> > > > > This change would break things severely. This won't allow to build it > > as a module any longer. > > Isn't this fixed already? > See the patch 'ASoC: SOF: core: fix undefined nocodec reference' and > Takashi's follow-up to fix the unused variable warning. Possibly the problem still persists although I haven't seen through my local build tests with randconfig. You can set SND_SOC_SOF=y and SND_SOC_NOCODEC=m, i.e. built-in sof-core while nocodec is a module. Takashi > > > > A better fix would be to somehow restrict the SND_SOC_SOF_NOCODEC to > > align with SND_SOC_SOF, i.e. disallow SND_SOC_SOF=y && > > SND_SOC_SOF_NOCODEC=m. Because of the complex mix of select and > > depends-on in SOF, I'm afraid that it's not that trivial, though. > > There might be something I overlooked, hopefully... > > > > An easier alternative would be to replace > > IS_ENABLED(CONFIG_SND_SOC_SOF_NOCODEC) with > > IS_REACHABLE(CONFIG_SND_SOC_SOF_NOCODEC). This assures the condition > > at the build time, although the error at probe might be a surprising > > to some users that don't know this hidden dependency. > > > > > > thanks, > > > > Takashi > > > > > >> --- > >> sound/soc/sof/Kconfig | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig > >> index b204c65..9c280c9 100644 > >> --- a/sound/soc/sof/Kconfig > >> +++ b/sound/soc/sof/Kconfig > >> @@ -44,7 +44,7 @@ config SND_SOC_SOF_OPTIONS > >> if SND_SOC_SOF_OPTIONS > >> config SND_SOC_SOF_NOCODEC > >> - tristate "SOF nocodec mode Support" > >> + bool "SOF nocodec mode Support" > >> help > >> This adds support for a dummy/nocodec machine driver fallback > >> option if no known codec is detected. This is typically only > >> -- > >> 2.7.4 > >> > >> > >> >
On 2019/5/10 15:12, Takashi Iwai wrote: > On Fri, 10 May 2019 04:36:57 +0200, > YueHaibing wrote: >> >> Fix gcc build error while CONFIG_SND_SOC_SOF_NOCODEC=m >> >> sound/soc/sof/core.o: In function `snd_sof_device_probe': >> core.c:(.text+0x4af): undefined reference to `sof_nocodec_setup' >> >> Change SND_SOC_SOF_NOCODEC to bool to fix this. >> >> Reported-by: Hulk Robot <hulkci@huawei.com> >> Fixes: c16211d6226d ("ASoC: SOF: Add Sound Open Firmware driver core") >> Signed-off-by: YueHaibing <yuehaibing@huawei.com> > > This change would break things severely. This won't allow to build it > as a module any longer. > > A better fix would be to somehow restrict the SND_SOC_SOF_NOCODEC to > align with SND_SOC_SOF, i.e. disallow SND_SOC_SOF=y && > SND_SOC_SOF_NOCODEC=m. Because of the complex mix of select and > depends-on in SOF, I'm afraid that it's not that trivial, though. > There might be something I overlooked, hopefully... > > An easier alternative would be to replace > IS_ENABLED(CONFIG_SND_SOC_SOF_NOCODEC) with > IS_REACHABLE(CONFIG_SND_SOC_SOF_NOCODEC). This assures the condition > at the build time, although the error at probe might be a surprising > to some users that don't know this hidden dependency. > Yes, I prefer to use IS_REACHABLE, thanks! > > thanks, > > Takashi > > >> --- >> sound/soc/sof/Kconfig | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig >> index b204c65..9c280c9 100644 >> --- a/sound/soc/sof/Kconfig >> +++ b/sound/soc/sof/Kconfig >> @@ -44,7 +44,7 @@ config SND_SOC_SOF_OPTIONS >> if SND_SOC_SOF_OPTIONS >> >> config SND_SOC_SOF_NOCODEC >> - tristate "SOF nocodec mode Support" >> + bool "SOF nocodec mode Support" >> help >> This adds support for a dummy/nocodec machine driver fallback >> option if no known codec is detected. This is typically only >> -- >> 2.7.4 >> >> >> > > . >
On 5/10/19 8:04 AM, Takashi Iwai wrote: > On Fri, 10 May 2019 14:56:29 +0200, > Pierre-Louis Bossart wrote: >> >> On 5/10/19 2:12 AM, Takashi Iwai wrote: >>> On Fri, 10 May 2019 04:36:57 +0200, >>> YueHaibing wrote: >>>> >>>> Fix gcc build error while CONFIG_SND_SOC_SOF_NOCODEC=m >>>> >>>> sound/soc/sof/core.o: In function `snd_sof_device_probe': >>>> core.c:(.text+0x4af): undefined reference to `sof_nocodec_setup' >>>> >>>> Change SND_SOC_SOF_NOCODEC to bool to fix this. >>>> >>>> Reported-by: Hulk Robot <hulkci@huawei.com> >>>> Fixes: c16211d6226d ("ASoC: SOF: Add Sound Open Firmware driver core") >>>> Signed-off-by: YueHaibing <yuehaibing@huawei.com> >>> >>> This change would break things severely. This won't allow to build it >>> as a module any longer. >> >> Isn't this fixed already? >> See the patch 'ASoC: SOF: core: fix undefined nocodec reference' and >> Takashi's follow-up to fix the unused variable warning. > > Possibly the problem still persists although I haven't seen through my > local build tests with randconfig. You can set SND_SOC_SOF=y and > SND_SOC_NOCODEC=m, i.e. built-in sof-core while nocodec is a module. YueHiabing, can you share the config and SHA1 so that we can double check? If the problem persists, we can do something like config SND_SOF_NOCODEC_SUPPORT bool "SOF nocodec mode support" config SND_SOF_NOCODEC tristate confir SND_SOC_SOF tristate select SND_SOF_NOCODEC if SND_SOF_NOCODEC_SUPPORT that way you propagate the required dependencies > > Takashi > >>> >>> A better fix would be to somehow restrict the SND_SOC_SOF_NOCODEC to >>> align with SND_SOC_SOF, i.e. disallow SND_SOC_SOF=y && >>> SND_SOC_SOF_NOCODEC=m. Because of the complex mix of select and >>> depends-on in SOF, I'm afraid that it's not that trivial, though. >>> There might be something I overlooked, hopefully... >>> >>> An easier alternative would be to replace >>> IS_ENABLED(CONFIG_SND_SOC_SOF_NOCODEC) with >>> IS_REACHABLE(CONFIG_SND_SOC_SOF_NOCODEC). This assures the condition >>> at the build time, although the error at probe might be a surprising >>> to some users that don't know this hidden dependency. >>> >>> >>> thanks, >>> >>> Takashi >>> >>> >>>> --- >>>> sound/soc/sof/Kconfig | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig >>>> index b204c65..9c280c9 100644 >>>> --- a/sound/soc/sof/Kconfig >>>> +++ b/sound/soc/sof/Kconfig >>>> @@ -44,7 +44,7 @@ config SND_SOC_SOF_OPTIONS >>>> if SND_SOC_SOF_OPTIONS >>>> config SND_SOC_SOF_NOCODEC >>>> - tristate "SOF nocodec mode Support" >>>> + bool "SOF nocodec mode Support" >>>> help >>>> This adds support for a dummy/nocodec machine driver fallback >>>> option if no known codec is detected. This is typically only >>>> -- >>>> 2.7.4 >>>> >>>> >>>> >> > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel >
On Fri, 10 May 2019 15:34:03 +0200, Pierre-Louis Bossart wrote: > > On 5/10/19 8:04 AM, Takashi Iwai wrote: > > On Fri, 10 May 2019 14:56:29 +0200, > > Pierre-Louis Bossart wrote: > >> > >> On 5/10/19 2:12 AM, Takashi Iwai wrote: > >>> On Fri, 10 May 2019 04:36:57 +0200, > >>> YueHaibing wrote: > >>>> > >>>> Fix gcc build error while CONFIG_SND_SOC_SOF_NOCODEC=m > >>>> > >>>> sound/soc/sof/core.o: In function `snd_sof_device_probe': > >>>> core.c:(.text+0x4af): undefined reference to `sof_nocodec_setup' > >>>> > >>>> Change SND_SOC_SOF_NOCODEC to bool to fix this. > >>>> > >>>> Reported-by: Hulk Robot <hulkci@huawei.com> > >>>> Fixes: c16211d6226d ("ASoC: SOF: Add Sound Open Firmware driver core") > >>>> Signed-off-by: YueHaibing <yuehaibing@huawei.com> > >>> > >>> This change would break things severely. This won't allow to build it > >>> as a module any longer. > >> > >> Isn't this fixed already? > >> See the patch 'ASoC: SOF: core: fix undefined nocodec reference' and > >> Takashi's follow-up to fix the unused variable warning. > > > > Possibly the problem still persists although I haven't seen through my > > local build tests with randconfig. You can set SND_SOC_SOF=y and > > SND_SOC_NOCODEC=m, i.e. built-in sof-core while nocodec is a module. > > YueHiabing, can you share the config and SHA1 so that we can double check? > > If the problem persists, we can do something like > > config SND_SOF_NOCODEC_SUPPORT > bool "SOF nocodec mode support" > > config SND_SOF_NOCODEC > tristate > > confir SND_SOC_SOF > tristate > select SND_SOF_NOCODEC if SND_SOF_NOCODEC_SUPPORT > > that way you propagate the required dependencies Yes, that would work. OTOH, I see no merit to build an extra module for nocodec. nocodec.c can be built together with sof-core stuff. thanks, Takashi
On Fri, 10 May 2019 15:41:10 +0200, Takashi Iwai wrote: > > On Fri, 10 May 2019 15:34:03 +0200, > Pierre-Louis Bossart wrote: > > > > On 5/10/19 8:04 AM, Takashi Iwai wrote: > > > On Fri, 10 May 2019 14:56:29 +0200, > > > Pierre-Louis Bossart wrote: > > >> > > >> On 5/10/19 2:12 AM, Takashi Iwai wrote: > > >>> On Fri, 10 May 2019 04:36:57 +0200, > > >>> YueHaibing wrote: > > >>>> > > >>>> Fix gcc build error while CONFIG_SND_SOC_SOF_NOCODEC=m > > >>>> > > >>>> sound/soc/sof/core.o: In function `snd_sof_device_probe': > > >>>> core.c:(.text+0x4af): undefined reference to `sof_nocodec_setup' > > >>>> > > >>>> Change SND_SOC_SOF_NOCODEC to bool to fix this. > > >>>> > > >>>> Reported-by: Hulk Robot <hulkci@huawei.com> > > >>>> Fixes: c16211d6226d ("ASoC: SOF: Add Sound Open Firmware driver core") > > >>>> Signed-off-by: YueHaibing <yuehaibing@huawei.com> > > >>> > > >>> This change would break things severely. This won't allow to build it > > >>> as a module any longer. > > >> > > >> Isn't this fixed already? > > >> See the patch 'ASoC: SOF: core: fix undefined nocodec reference' and > > >> Takashi's follow-up to fix the unused variable warning. > > > > > > Possibly the problem still persists although I haven't seen through my > > > local build tests with randconfig. You can set SND_SOC_SOF=y and > > > SND_SOC_NOCODEC=m, i.e. built-in sof-core while nocodec is a module. > > > > YueHiabing, can you share the config and SHA1 so that we can double check? > > > > If the problem persists, we can do something like > > > > config SND_SOF_NOCODEC_SUPPORT > > bool "SOF nocodec mode support" > > > > config SND_SOF_NOCODEC > > tristate > > > > confir SND_SOC_SOF > > tristate > > select SND_SOF_NOCODEC if SND_SOF_NOCODEC_SUPPORT > > > > that way you propagate the required dependencies > > Yes, that would work. OTOH, I see no merit to build an extra module > for nocodec. nocodec.c can be built together with sof-core stuff. I mean a patch like below. Takashi diff --git a/include/sound/sof.h b/include/sound/sof.h index 4640566b54fe..1af70800f6dc 100644 --- a/include/sound/sof.h +++ b/include/sound/sof.h @@ -92,9 +92,4 @@ struct sof_dev_desc { const struct sof_arch_ops *arch_ops; }; -int sof_nocodec_setup(struct device *dev, - struct snd_sof_pdata *sof_pdata, - struct snd_soc_acpi_mach *mach, - const struct sof_dev_desc *desc, - const struct snd_sof_dsp_ops *ops); #endif diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig index b204c65698f9..9c280c977d55 100644 --- a/sound/soc/sof/Kconfig +++ b/sound/soc/sof/Kconfig @@ -44,7 +44,7 @@ config SND_SOC_SOF_OPTIONS if SND_SOC_SOF_OPTIONS config SND_SOC_SOF_NOCODEC - tristate "SOF nocodec mode Support" + bool "SOF nocodec mode Support" help This adds support for a dummy/nocodec machine driver fallback option if no known codec is detected. This is typically only diff --git a/sound/soc/sof/Makefile b/sound/soc/sof/Makefile index 8f14c9d2950b..09222be6d1b8 100644 --- a/sound/soc/sof/Makefile +++ b/sound/soc/sof/Makefile @@ -2,14 +2,12 @@ snd-sof-objs := core.o ops.o loader.o ipc.o pcm.o pm.o debug.o topology.o\ control.o trace.o utils.o +snd-sof-$(CONFIG_SND_SOC_SOF_NOCODEC) += nocodec.o snd-sof-pci-objs := sof-pci-dev.o snd-sof-acpi-objs := sof-acpi-dev.o -snd-sof-nocodec-objs := nocodec.o obj-$(CONFIG_SND_SOC_SOF) += snd-sof.o -obj-$(CONFIG_SND_SOC_SOF_NOCODEC) += snd-sof-nocodec.o - obj-$(CONFIG_SND_SOC_SOF_ACPI) += sof-acpi-dev.o obj-$(CONFIG_SND_SOC_SOF_PCI) += sof-pci-dev.o diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c index 32105e0fabe8..744f7f465173 100644 --- a/sound/soc/sof/core.c +++ b/sound/soc/sof/core.c @@ -502,6 +502,18 @@ int snd_sof_device_remove(struct device *dev) } EXPORT_SYMBOL(snd_sof_device_remove); +static int __init snd_sof_init(void) +{ + return platform_driver_register(&sof_nocodec_audio); +} +module_init(snd_sof_init); + +static void __exit snd_sof_exit(void) +{ + platform_driver_unregister(&sof_nocodec_audio); +} +module_exit(snd_sof_exit); + MODULE_AUTHOR("Liam Girdwood"); MODULE_DESCRIPTION("Sound Open Firmware (SOF) Core"); MODULE_LICENSE("Dual BSD/GPL"); diff --git a/sound/soc/sof/nocodec.c b/sound/soc/sof/nocodec.c index f84b4344dcc3..11811591bd81 100644 --- a/sound/soc/sof/nocodec.c +++ b/sound/soc/sof/nocodec.c @@ -8,7 +8,6 @@ // Author: Liam Girdwood <liam.r.girdwood@linux.intel.com> // -#include <linux/module.h> #include <sound/sof.h> #include "sof-priv.h" @@ -77,7 +76,6 @@ int sof_nocodec_setup(struct device *dev, &sof_nocodec_card); return ret; } -EXPORT_SYMBOL(sof_nocodec_setup); static int sof_nocodec_probe(struct platform_device *pdev) { @@ -93,7 +91,7 @@ static int sof_nocodec_remove(struct platform_device *pdev) return 0; } -static struct platform_driver sof_nocodec_audio = { +struct platform_driver sof_nocodec_audio = { .probe = sof_nocodec_probe, .remove = sof_nocodec_remove, .driver = { @@ -101,9 +99,3 @@ static struct platform_driver sof_nocodec_audio = { .pm = &snd_soc_pm_ops, }, }; -module_platform_driver(sof_nocodec_audio) - -MODULE_DESCRIPTION("ASoC sof nocodec"); -MODULE_AUTHOR("Liam Girdwood"); -MODULE_LICENSE("Dual BSD/GPL"); -MODULE_ALIAS("platform:sof-nocodec"); diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 1e85d6f9c5c3..3ae55d15820f 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -559,6 +559,17 @@ int snd_sof_init_trace_ipc(struct snd_sof_dev *sdev); */ extern struct snd_compr_ops sof_compressed_ops; +/* + * nocodec platform binding + */ +extern struct platform_driver sof_nocodec_audio; + +int sof_nocodec_setup(struct device *dev, + struct snd_sof_pdata *sof_pdata, + struct snd_soc_acpi_mach *mach, + const struct sof_dev_desc *desc, + const struct snd_sof_dsp_ops *ops); + /* * Kcontrols. */
On 5/10/19 8:56 AM, Takashi Iwai wrote: > On Fri, 10 May 2019 15:41:10 +0200, > Takashi Iwai wrote: >> >> On Fri, 10 May 2019 15:34:03 +0200, >> Pierre-Louis Bossart wrote: >>> >>> On 5/10/19 8:04 AM, Takashi Iwai wrote: >>>> On Fri, 10 May 2019 14:56:29 +0200, >>>> Pierre-Louis Bossart wrote: >>>>> >>>>> On 5/10/19 2:12 AM, Takashi Iwai wrote: >>>>>> On Fri, 10 May 2019 04:36:57 +0200, >>>>>> YueHaibing wrote: >>>>>>> >>>>>>> Fix gcc build error while CONFIG_SND_SOC_SOF_NOCODEC=m >>>>>>> >>>>>>> sound/soc/sof/core.o: In function `snd_sof_device_probe': >>>>>>> core.c:(.text+0x4af): undefined reference to `sof_nocodec_setup' >>>>>>> >>>>>>> Change SND_SOC_SOF_NOCODEC to bool to fix this. >>>>>>> >>>>>>> Reported-by: Hulk Robot <hulkci@huawei.com> >>>>>>> Fixes: c16211d6226d ("ASoC: SOF: Add Sound Open Firmware driver core") >>>>>>> Signed-off-by: YueHaibing <yuehaibing@huawei.com> >>>>>> >>>>>> This change would break things severely. This won't allow to build it >>>>>> as a module any longer. >>>>> >>>>> Isn't this fixed already? >>>>> See the patch 'ASoC: SOF: core: fix undefined nocodec reference' and >>>>> Takashi's follow-up to fix the unused variable warning. >>>> >>>> Possibly the problem still persists although I haven't seen through my >>>> local build tests with randconfig. You can set SND_SOC_SOF=y and >>>> SND_SOC_NOCODEC=m, i.e. built-in sof-core while nocodec is a module. >>> >>> YueHiabing, can you share the config and SHA1 so that we can double check? >>> >>> If the problem persists, we can do something like >>> >>> config SND_SOF_NOCODEC_SUPPORT >>> bool "SOF nocodec mode support" >>> >>> config SND_SOF_NOCODEC >>> tristate >>> >>> confir SND_SOC_SOF >>> tristate >>> select SND_SOF_NOCODEC if SND_SOF_NOCODEC_SUPPORT >>> >>> that way you propagate the required dependencies >> >> Yes, that would work. OTOH, I see no merit to build an extra module >> for nocodec. nocodec.c can be built together with sof-core stuff. the module has its benefits. Today nocodec includes all possible DAIs, I wanted to add module parameters to restrict things a bit for tests/debug. It'll be e.g. very helpful for SoundWire to avoid exposing the SSP DAIs. Also nocodec is incompatible with hdaudio/hdmi support at the moment, we had all sorts of issues with suspend/resume. > > I mean a patch like below. > > > Takashi > > diff --git a/include/sound/sof.h b/include/sound/sof.h > index 4640566b54fe..1af70800f6dc 100644 > --- a/include/sound/sof.h > +++ b/include/sound/sof.h > @@ -92,9 +92,4 @@ struct sof_dev_desc { > const struct sof_arch_ops *arch_ops; > }; > > -int sof_nocodec_setup(struct device *dev, > - struct snd_sof_pdata *sof_pdata, > - struct snd_soc_acpi_mach *mach, > - const struct sof_dev_desc *desc, > - const struct snd_sof_dsp_ops *ops); > #endif > diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig > index b204c65698f9..9c280c977d55 100644 > --- a/sound/soc/sof/Kconfig > +++ b/sound/soc/sof/Kconfig > @@ -44,7 +44,7 @@ config SND_SOC_SOF_OPTIONS > if SND_SOC_SOF_OPTIONS > > config SND_SOC_SOF_NOCODEC > - tristate "SOF nocodec mode Support" > + bool "SOF nocodec mode Support" > help > This adds support for a dummy/nocodec machine driver fallback > option if no known codec is detected. This is typically only > diff --git a/sound/soc/sof/Makefile b/sound/soc/sof/Makefile > index 8f14c9d2950b..09222be6d1b8 100644 > --- a/sound/soc/sof/Makefile > +++ b/sound/soc/sof/Makefile > @@ -2,14 +2,12 @@ > > snd-sof-objs := core.o ops.o loader.o ipc.o pcm.o pm.o debug.o topology.o\ > control.o trace.o utils.o > +snd-sof-$(CONFIG_SND_SOC_SOF_NOCODEC) += nocodec.o > > snd-sof-pci-objs := sof-pci-dev.o > snd-sof-acpi-objs := sof-acpi-dev.o > -snd-sof-nocodec-objs := nocodec.o > > obj-$(CONFIG_SND_SOC_SOF) += snd-sof.o > -obj-$(CONFIG_SND_SOC_SOF_NOCODEC) += snd-sof-nocodec.o > - > > obj-$(CONFIG_SND_SOC_SOF_ACPI) += sof-acpi-dev.o > obj-$(CONFIG_SND_SOC_SOF_PCI) += sof-pci-dev.o > diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c > index 32105e0fabe8..744f7f465173 100644 > --- a/sound/soc/sof/core.c > +++ b/sound/soc/sof/core.c > @@ -502,6 +502,18 @@ int snd_sof_device_remove(struct device *dev) > } > EXPORT_SYMBOL(snd_sof_device_remove); > > +static int __init snd_sof_init(void) > +{ > + return platform_driver_register(&sof_nocodec_audio); > +} > +module_init(snd_sof_init); > + > +static void __exit snd_sof_exit(void) > +{ > + platform_driver_unregister(&sof_nocodec_audio); > +} > +module_exit(snd_sof_exit); > + > MODULE_AUTHOR("Liam Girdwood"); > MODULE_DESCRIPTION("Sound Open Firmware (SOF) Core"); > MODULE_LICENSE("Dual BSD/GPL"); > diff --git a/sound/soc/sof/nocodec.c b/sound/soc/sof/nocodec.c > index f84b4344dcc3..11811591bd81 100644 > --- a/sound/soc/sof/nocodec.c > +++ b/sound/soc/sof/nocodec.c > @@ -8,7 +8,6 @@ > // Author: Liam Girdwood <liam.r.girdwood@linux.intel.com> > // > > -#include <linux/module.h> > #include <sound/sof.h> > #include "sof-priv.h" > > @@ -77,7 +76,6 @@ int sof_nocodec_setup(struct device *dev, > &sof_nocodec_card); > return ret; > } > -EXPORT_SYMBOL(sof_nocodec_setup); > > static int sof_nocodec_probe(struct platform_device *pdev) > { > @@ -93,7 +91,7 @@ static int sof_nocodec_remove(struct platform_device *pdev) > return 0; > } > > -static struct platform_driver sof_nocodec_audio = { > +struct platform_driver sof_nocodec_audio = { > .probe = sof_nocodec_probe, > .remove = sof_nocodec_remove, > .driver = { > @@ -101,9 +99,3 @@ static struct platform_driver sof_nocodec_audio = { > .pm = &snd_soc_pm_ops, > }, > }; > -module_platform_driver(sof_nocodec_audio) > - > -MODULE_DESCRIPTION("ASoC sof nocodec"); > -MODULE_AUTHOR("Liam Girdwood"); > -MODULE_LICENSE("Dual BSD/GPL"); > -MODULE_ALIAS("platform:sof-nocodec"); > diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h > index 1e85d6f9c5c3..3ae55d15820f 100644 > --- a/sound/soc/sof/sof-priv.h > +++ b/sound/soc/sof/sof-priv.h > @@ -559,6 +559,17 @@ int snd_sof_init_trace_ipc(struct snd_sof_dev *sdev); > */ > extern struct snd_compr_ops sof_compressed_ops; > > +/* > + * nocodec platform binding > + */ > +extern struct platform_driver sof_nocodec_audio; > + > +int sof_nocodec_setup(struct device *dev, > + struct snd_sof_pdata *sof_pdata, > + struct snd_soc_acpi_mach *mach, > + const struct sof_dev_desc *desc, > + const struct snd_sof_dsp_ops *ops); > + > /* > * Kcontrols. > */ > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel >
On Fri, 10 May 2019 17:29:58 +0200, Pierre-Louis Bossart wrote: > > > > On 5/10/19 8:56 AM, Takashi Iwai wrote: > > On Fri, 10 May 2019 15:41:10 +0200, > > Takashi Iwai wrote: > >> > >> On Fri, 10 May 2019 15:34:03 +0200, > >> Pierre-Louis Bossart wrote: > >>> > >>> On 5/10/19 8:04 AM, Takashi Iwai wrote: > >>>> On Fri, 10 May 2019 14:56:29 +0200, > >>>> Pierre-Louis Bossart wrote: > >>>>> > >>>>> On 5/10/19 2:12 AM, Takashi Iwai wrote: > >>>>>> On Fri, 10 May 2019 04:36:57 +0200, > >>>>>> YueHaibing wrote: > >>>>>>> > >>>>>>> Fix gcc build error while CONFIG_SND_SOC_SOF_NOCODEC=m > >>>>>>> > >>>>>>> sound/soc/sof/core.o: In function `snd_sof_device_probe': > >>>>>>> core.c:(.text+0x4af): undefined reference to `sof_nocodec_setup' > >>>>>>> > >>>>>>> Change SND_SOC_SOF_NOCODEC to bool to fix this. > >>>>>>> > >>>>>>> Reported-by: Hulk Robot <hulkci@huawei.com> > >>>>>>> Fixes: c16211d6226d ("ASoC: SOF: Add Sound Open Firmware driver core") > >>>>>>> Signed-off-by: YueHaibing <yuehaibing@huawei.com> > >>>>>> > >>>>>> This change would break things severely. This won't allow to build it > >>>>>> as a module any longer. > >>>>> > >>>>> Isn't this fixed already? > >>>>> See the patch 'ASoC: SOF: core: fix undefined nocodec reference' and > >>>>> Takashi's follow-up to fix the unused variable warning. > >>>> > >>>> Possibly the problem still persists although I haven't seen through my > >>>> local build tests with randconfig. You can set SND_SOC_SOF=y and > >>>> SND_SOC_NOCODEC=m, i.e. built-in sof-core while nocodec is a module. > >>> > >>> YueHiabing, can you share the config and SHA1 so that we can double check? > >>> > >>> If the problem persists, we can do something like > >>> > >>> config SND_SOF_NOCODEC_SUPPORT > >>> bool "SOF nocodec mode support" > >>> > >>> config SND_SOF_NOCODEC > >>> tristate > >>> > >>> confir SND_SOC_SOF > >>> tristate > >>> select SND_SOF_NOCODEC if SND_SOF_NOCODEC_SUPPORT > >>> > >>> that way you propagate the required dependencies > >> > >> Yes, that would work. OTOH, I see no merit to build an extra module > >> for nocodec. nocodec.c can be built together with sof-core stuff. > > the module has its benefits. Today nocodec includes all possible DAIs, > I wanted to add module parameters to restrict things a bit for > tests/debug. It'll be e.g. very helpful for SoundWire to avoid > exposing the SSP DAIs. > > Also nocodec is incompatible with hdaudio/hdmi support at the moment, > we had all sorts of issues with suspend/resume. Well, in the case of SOF, the core code calls directly soc_nocodec_setup(), hence it's rather a direct link. So it makes little sense to make the nocodec code split from sof-core, unless the nocodec code is used / linked by components other than SOF. I doubt the possibility because the current DAI is clearly only for SOF... The module option can be still be there; it'll be applied just to sof-core instead of sof-nocodec. thanks, Takashi > > > > > I mean a patch like below. > > > > > > Takashi > > > > diff --git a/include/sound/sof.h b/include/sound/sof.h > > index 4640566b54fe..1af70800f6dc 100644 > > --- a/include/sound/sof.h > > +++ b/include/sound/sof.h > > @@ -92,9 +92,4 @@ struct sof_dev_desc { > > const struct sof_arch_ops *arch_ops; > > }; > > -int sof_nocodec_setup(struct device *dev, > > - struct snd_sof_pdata *sof_pdata, > > - struct snd_soc_acpi_mach *mach, > > - const struct sof_dev_desc *desc, > > - const struct snd_sof_dsp_ops *ops); > > #endif > > diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig > > index b204c65698f9..9c280c977d55 100644 > > --- a/sound/soc/sof/Kconfig > > +++ b/sound/soc/sof/Kconfig > > @@ -44,7 +44,7 @@ config SND_SOC_SOF_OPTIONS > > if SND_SOC_SOF_OPTIONS > > config SND_SOC_SOF_NOCODEC > > - tristate "SOF nocodec mode Support" > > + bool "SOF nocodec mode Support" > > help > > This adds support for a dummy/nocodec machine driver fallback > > option if no known codec is detected. This is typically only > > diff --git a/sound/soc/sof/Makefile b/sound/soc/sof/Makefile > > index 8f14c9d2950b..09222be6d1b8 100644 > > --- a/sound/soc/sof/Makefile > > +++ b/sound/soc/sof/Makefile > > @@ -2,14 +2,12 @@ > > snd-sof-objs := core.o ops.o loader.o ipc.o pcm.o pm.o debug.o > > topology.o\ > > control.o trace.o utils.o > > +snd-sof-$(CONFIG_SND_SOC_SOF_NOCODEC) += nocodec.o > > snd-sof-pci-objs := sof-pci-dev.o > > snd-sof-acpi-objs := sof-acpi-dev.o > > -snd-sof-nocodec-objs := nocodec.o > > obj-$(CONFIG_SND_SOC_SOF) += snd-sof.o > > -obj-$(CONFIG_SND_SOC_SOF_NOCODEC) += snd-sof-nocodec.o > > - > > obj-$(CONFIG_SND_SOC_SOF_ACPI) += sof-acpi-dev.o > > obj-$(CONFIG_SND_SOC_SOF_PCI) += sof-pci-dev.o > > diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c > > index 32105e0fabe8..744f7f465173 100644 > > --- a/sound/soc/sof/core.c > > +++ b/sound/soc/sof/core.c > > @@ -502,6 +502,18 @@ int snd_sof_device_remove(struct device *dev) > > } > > EXPORT_SYMBOL(snd_sof_device_remove); > > +static int __init snd_sof_init(void) > > +{ > > + return platform_driver_register(&sof_nocodec_audio); > > +} > > +module_init(snd_sof_init); > > + > > +static void __exit snd_sof_exit(void) > > +{ > > + platform_driver_unregister(&sof_nocodec_audio); > > +} > > +module_exit(snd_sof_exit); > > + > > MODULE_AUTHOR("Liam Girdwood"); > > MODULE_DESCRIPTION("Sound Open Firmware (SOF) Core"); > > MODULE_LICENSE("Dual BSD/GPL"); > > diff --git a/sound/soc/sof/nocodec.c b/sound/soc/sof/nocodec.c > > index f84b4344dcc3..11811591bd81 100644 > > --- a/sound/soc/sof/nocodec.c > > +++ b/sound/soc/sof/nocodec.c > > @@ -8,7 +8,6 @@ > > // Author: Liam Girdwood <liam.r.girdwood@linux.intel.com> > > // > > -#include <linux/module.h> > > #include <sound/sof.h> > > #include "sof-priv.h" > > @@ -77,7 +76,6 @@ int sof_nocodec_setup(struct device *dev, > > &sof_nocodec_card); > > return ret; > > } > > -EXPORT_SYMBOL(sof_nocodec_setup); > > static int sof_nocodec_probe(struct platform_device *pdev) > > { > > @@ -93,7 +91,7 @@ static int sof_nocodec_remove(struct platform_device *pdev) > > return 0; > > } > > -static struct platform_driver sof_nocodec_audio = { > > +struct platform_driver sof_nocodec_audio = { > > .probe = sof_nocodec_probe, > > .remove = sof_nocodec_remove, > > .driver = { > > @@ -101,9 +99,3 @@ static struct platform_driver sof_nocodec_audio = { > > .pm = &snd_soc_pm_ops, > > }, > > }; > > -module_platform_driver(sof_nocodec_audio) > > - > > -MODULE_DESCRIPTION("ASoC sof nocodec"); > > -MODULE_AUTHOR("Liam Girdwood"); > > -MODULE_LICENSE("Dual BSD/GPL"); > > -MODULE_ALIAS("platform:sof-nocodec"); > > diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h > > index 1e85d6f9c5c3..3ae55d15820f 100644 > > --- a/sound/soc/sof/sof-priv.h > > +++ b/sound/soc/sof/sof-priv.h > > @@ -559,6 +559,17 @@ int snd_sof_init_trace_ipc(struct snd_sof_dev *sdev); > > */ > > extern struct snd_compr_ops sof_compressed_ops; > > +/* > > + * nocodec platform binding > > + */ > > +extern struct platform_driver sof_nocodec_audio; > > + > > +int sof_nocodec_setup(struct device *dev, > > + struct snd_sof_pdata *sof_pdata, > > + struct snd_soc_acpi_mach *mach, > > + const struct sof_dev_desc *desc, > > + const struct snd_sof_dsp_ops *ops); > > + > > /* > > * Kcontrols. > > */ > > _______________________________________________ > > Alsa-devel mailing list > > Alsa-devel@alsa-project.org > > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel > > >
>>>> Yes, that would work. OTOH, I see no merit to build an extra module >>>> for nocodec. nocodec.c can be built together with sof-core stuff. >> >> the module has its benefits. Today nocodec includes all possible DAIs, >> I wanted to add module parameters to restrict things a bit for >> tests/debug. It'll be e.g. very helpful for SoundWire to avoid >> exposing the SSP DAIs. >> >> Also nocodec is incompatible with hdaudio/hdmi support at the moment, >> we had all sorts of issues with suspend/resume. > > Well, in the case of SOF, the core code calls directly > soc_nocodec_setup(), hence it's rather a direct link. So it makes > little sense to make the nocodec code split from sof-core, unless the > nocodec code is used / linked by components other than SOF. I doubt > the possibility because the current DAI is clearly only for SOF... > > The module option can be still be there; it'll be applied just to > sof-core instead of sof-nocodec. I see your point and this SOF core/nocodec dependency is a conceptual miss on our side. Thanks for bringing our attention on this. The core is really supposed to be about the DSP side of things. It shouldn't be burdened with machine driver stuff, but it unfortunately is at two levels. Initially the nocodec code was handled at the soc-acpi-dev or soc-pci-dev level, and it's still there that the FORCE_CODEC mode is handled, along with the calls to check the codec ACPI IDs. Now when we enabled the HDaudio case, we somehow ended-up moving parts of the nocodec support in the SOF core to simplify our life but created a dependency that wasn't intentional at all. we collectively missed it while we were struggling with nocodec/hdaudio compatibility. The second issue is that we create a platform_device for the machine driver in the SOF core. This is a shortcut that we took and that works for Intel, but for DeviceTree-based platforms this will have to change. So long story short, I'd rather have a simple Kconfig fix to avoid compilation issues for now and revisit all the machine driver support, e.g. when the i.MX patches show up, than strengthen a dependency that we introduced by accident rather than by design.
On Fri, 10 May 2019 19:56:51 +0200, Pierre-Louis Bossart wrote: > > > >>>> Yes, that would work. OTOH, I see no merit to build an extra module > >>>> for nocodec. nocodec.c can be built together with sof-core stuff. > >> > >> the module has its benefits. Today nocodec includes all possible DAIs, > >> I wanted to add module parameters to restrict things a bit for > >> tests/debug. It'll be e.g. very helpful for SoundWire to avoid > >> exposing the SSP DAIs. > >> > >> Also nocodec is incompatible with hdaudio/hdmi support at the moment, > >> we had all sorts of issues with suspend/resume. > > > > Well, in the case of SOF, the core code calls directly > > soc_nocodec_setup(), hence it's rather a direct link. So it makes > > little sense to make the nocodec code split from sof-core, unless the > > nocodec code is used / linked by components other than SOF. I doubt > > the possibility because the current DAI is clearly only for SOF... > > > > The module option can be still be there; it'll be applied just to > > sof-core instead of sof-nocodec. > > I see your point and this SOF core/nocodec dependency is a conceptual > miss on our side. Thanks for bringing our attention on this. > > The core is really supposed to be about the DSP side of things. It > shouldn't be burdened with machine driver stuff, but it unfortunately > is at two levels. > > Initially the nocodec code was handled at the soc-acpi-dev or > soc-pci-dev level, and it's still there that the FORCE_CODEC mode is > handled, along with the calls to check the codec ACPI IDs. Now when we > enabled the HDaudio case, we somehow ended-up moving parts of the > nocodec support in the SOF core to simplify our life but created a > dependency that wasn't intentional at all. we collectively missed it > while we were struggling with nocodec/hdaudio compatibility. > > The second issue is that we create a platform_device for the machine > driver in the SOF core. This is a shortcut that we took and that works > for Intel, but for DeviceTree-based platforms this will have to > change. > > So long story short, I'd rather have a simple Kconfig fix to avoid > compilation issues for now and revisit all the machine driver support, > e.g. when the i.MX patches show up, than strengthen a dependency that > we introduced by accident rather than by design. OK, thanks, that's convincing enough. My proposal was just some minor optimization, and as long as there is a planned change in future, it's fine to leave in the current or similar form. thanks, Takashi
diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig index b204c65..9c280c9 100644 --- a/sound/soc/sof/Kconfig +++ b/sound/soc/sof/Kconfig @@ -44,7 +44,7 @@ config SND_SOC_SOF_OPTIONS if SND_SOC_SOF_OPTIONS config SND_SOC_SOF_NOCODEC - tristate "SOF nocodec mode Support" + bool "SOF nocodec mode Support" help This adds support for a dummy/nocodec machine driver fallback option if no known codec is detected. This is typically only
Fix gcc build error while CONFIG_SND_SOC_SOF_NOCODEC=m sound/soc/sof/core.o: In function `snd_sof_device_probe': core.c:(.text+0x4af): undefined reference to `sof_nocodec_setup' Change SND_SOC_SOF_NOCODEC to bool to fix this. Reported-by: Hulk Robot <hulkci@huawei.com> Fixes: c16211d6226d ("ASoC: SOF: Add Sound Open Firmware driver core") Signed-off-by: YueHaibing <yuehaibing@huawei.com> --- sound/soc/sof/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)