In the new code introduced with commit cf4c87abe238ec17cd0255b4e21abd949d7f811e, "OMAP: McBSP: implement McBSP CLKR and FSR signal muxing via mach-omap2/mcbsp.c", the way omap1 build is supposed to bypass omap2 specific functionality doesn't optimize out all omap2 specific stuff. This breaks linking phase for omap1 machines, giving "undefined reference to `omap2_mcbsp1_mux_clkr_src'" and "undefined reference to `omap2_mcbsp1_mux_fsr_src'" errors. Fix it.
Created and tested against linux-2.6.37-rc1.
Signed-off-by: Janusz Krzysztofik jkrzyszt@tis.icnet.pl ---
sound/soc/omap/omap-mcbsp.c | 8 ++++++++ 1 file changed, 8 insertions(+)
--- linux-2.6.37-rc1/sound/soc/omap/omap-mcbsp.c.orig 2010-11-01 22:43:16.000000000 +0100 +++ linux-2.6.37-rc1/sound/soc/omap/omap-mcbsp.c 2010-11-02 15:41:42.000000000 +0100 @@ -644,15 +644,23 @@ static int omap_mcbsp_dai_set_dai_sysclk
case OMAP_MCBSP_CLKR_SRC_CLKR: + if (cpu_class_is_omap1()) + break; omap2_mcbsp1_mux_clkr_src(CLKR_SRC_CLKR); break; case OMAP_MCBSP_CLKR_SRC_CLKX: + if (cpu_class_is_omap1()) + break; omap2_mcbsp1_mux_clkr_src(CLKR_SRC_CLKX); break; case OMAP_MCBSP_FSR_SRC_FSR: + if (cpu_class_is_omap1()) + break; omap2_mcbsp1_mux_fsr_src(FSR_SRC_FSR); break; case OMAP_MCBSP_FSR_SRC_FSX: + if (cpu_class_is_omap1()) + break; omap2_mcbsp1_mux_fsr_src(FSR_SRC_FSX); break; default:
On Tue, Nov 02, 2010 at 03:50:32PM +0100, Janusz Krzysztofik wrote:
In the new code introduced with commit cf4c87abe238ec17cd0255b4e21abd949d7f811e, "OMAP: McBSP: implement McBSP CLKR and FSR signal muxing via mach-omap2/mcbsp.c", the way omap1 build is supposed to bypass omap2 specific functionality doesn't optimize out all omap2 specific stuff. This breaks linking phase for omap1 machines, giving "undefined reference to `omap2_mcbsp1_mux_clkr_src'" and "undefined reference to `omap2_mcbsp1_mux_fsr_src'" errors. Fix it.
Acked-by: Mark Brown broonie@opensource.wolfsonmicro.com
case OMAP_MCBSP_CLKR_SRC_CLKR:
if (cpu_class_is_omap1())break;
I guess this will still fail with optimisation diabled, but I'm not sure anyone ever builds the kernel like that.
On Tue, 2 Nov 2010, Janusz Krzysztofik wrote:
In the new code introduced with commit cf4c87abe238ec17cd0255b4e21abd949d7f811e, "OMAP: McBSP: implement McBSP CLKR and FSR signal muxing via mach-omap2/mcbsp.c", the way omap1 build is supposed to bypass omap2 specific functionality doesn't optimize out all omap2 specific stuff. This breaks linking phase for omap1 machines, giving "undefined reference to `omap2_mcbsp1_mux_clkr_src'" and "undefined reference to `omap2_mcbsp1_mux_fsr_src'" errors. Fix it.
Created and tested against linux-2.6.37-rc1.
Signed-off-by: Janusz Krzysztofik jkrzyszt@tis.icnet.pl
Acked-by: Paul Walmsley paul@pwsan.com
Thanks Janusz.
- Paul
On Tue, 2 Nov 2010 10:58:59 -0600 (MDT) Paul Walmsley paul@pwsan.com wrote:
On Tue, 2 Nov 2010, Janusz Krzysztofik wrote:
In the new code introduced with commit cf4c87abe238ec17cd0255b4e21abd949d7f811e, "OMAP: McBSP: implement McBSP CLKR and FSR signal muxing via mach-omap2/mcbsp.c", the way omap1 build is supposed to bypass omap2 specific functionality doesn't optimize out all omap2 specific stuff. This breaks linking phase for omap1 machines, giving "undefined reference to `omap2_mcbsp1_mux_clkr_src'" and "undefined reference to `omap2_mcbsp1_mux_fsr_src'" errors. Fix it.
Created and tested against linux-2.6.37-rc1.
Signed-off-by: Janusz Krzysztofik jkrzyszt@tis.icnet.pl
Acked-by: Paul Walmsley paul@pwsan.com
Thanks Janusz.
Yep, this must go to 2.6.37.
Acked-by: Jarkko Nikula jhnikula@gmail.com
On Wed, 2010-11-03 at 10:20 +0200, Jarkko Nikula wrote:
On Tue, 2 Nov 2010 10:58:59 -0600 (MDT) Paul Walmsley paul@pwsan.com wrote:
On Tue, 2 Nov 2010, Janusz Krzysztofik wrote:
In the new code introduced with commit cf4c87abe238ec17cd0255b4e21abd949d7f811e, "OMAP: McBSP: implement McBSP CLKR and FSR signal muxing via mach-omap2/mcbsp.c", the way omap1 build is supposed to bypass omap2 specific functionality doesn't optimize out all omap2 specific stuff. This breaks linking phase for omap1 machines, giving "undefined reference to `omap2_mcbsp1_mux_clkr_src'" and "undefined reference to `omap2_mcbsp1_mux_fsr_src'" errors. Fix it.
Created and tested against linux-2.6.37-rc1.
Signed-off-by: Janusz Krzysztofik jkrzyszt@tis.icnet.pl
Acked-by: Paul Walmsley paul@pwsan.com
Thanks Janusz.
Yep, this must go to 2.6.37.
Acked-by: Jarkko Nikula jhnikula@gmail.com
Applied.
Thanks
Liam
Hello Janusz, Mark,
On Tue, 2 Nov 2010, Janusz Krzysztofik wrote:
In the new code introduced with commit cf4c87abe238ec17cd0255b4e21abd949d7f811e, "OMAP: McBSP: implement McBSP CLKR and FSR signal muxing via mach-omap2/mcbsp.c", the way omap1 build is supposed to bypass omap2 specific functionality doesn't optimize out all omap2 specific stuff. This breaks linking phase for omap1 machines, giving "undefined reference to `omap2_mcbsp1_mux_clkr_src'" and "undefined reference to `omap2_mcbsp1_mux_fsr_src'" errors. Fix it.
Created and tested against linux-2.6.37-rc1.
Signed-off-by: Janusz Krzysztofik jkrzyszt@tis.icnet.pl
Thanks for fixing this. What do you think about the following patch instead? It should avoid any compiler issues.
- Paul
From: Paul Walmsley paul@pwsan.com Subject: [PATCH] ASoC: OMAP: McBSP: fix build breakage on OMAP1
After commits d13586574d373ef40acd4725c9a269daa355e412 ("OMAP: McBSP: implement functional clock switching via clock framework") and cf4c87abe238ec17cd0255b4e21abd949d7f811e ("OMAP: McBSP: implement McBSP CLKR and FSR signal muxing via mach-omap2/mcbsp.c"), any OMAP1 board (such as the AMS Delta) that uses the ASoC McBSP driver will no longer build:
sound/built-in.o: In function `omap_mcbsp_dai_set_dai_sysclk': last.c:(.text+0x24ff8): undefined reference to `omap2_mcbsp1_mux_clkr_src' last.c:(.text+0x2500c): undefined reference to `omap2_mcbsp1_mux_fsr_src' make: *** [vmlinux] Error 1
Fix by defining three OMAP1-only dummy functions for omap2_mcbsp1_mux_clkr_src(), omap2_mcbsp1_mux_fsr_src(), and omap2_mcbsp_set_clks_src().
Normally, code that is OMAP SoC-revision-specific like this should go under the arch/arm/*omap* directories, and get abstracted away from drivers via struct platform_data function pointers. This doesn't work in this case since there doesn't appear to be any convenient way to access struct platform_data (or something like it) in the current design of the sound/soc/omap/omap-mcbsp.c driver.
Reported by Janusz Krzysztofik jkrzyszt@tis.icnet.pl and Tony Lindgren tony@atomide.com. Janusz also posted a patch to fix this at:
http://www.spinics.net/lists/linux-omap/msg39560.html
(among other places), but the following approach seems less dependent on compiler behavior.
This patch passes build tests for ams_delta_defconfig and omap2plus_defconfig, but since I don't have an AMS Delta here, I can't boot test it on that platform.
Signed-off-by: Paul Walmsley paul@pwsan.com Cc: Janusz Krzysztofik jkrzyszt@tis.icnet.pl Cc: Tony Lindgren tony@atomide.com Cc: Jarkko Nikula jhnikula@gmail.com Cc: Peter Ujfalusi peter.ujfalusi@nokia.com Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Liam Girdwood lrg@slimlogic.co.uk --- arch/arm/plat-omap/mcbsp.c | 27 +++++++++++++++++++++++++++ 1 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c index fdecd33..1fbfcf3 100644 --- a/arch/arm/plat-omap/mcbsp.c +++ b/arch/arm/plat-omap/mcbsp.c @@ -1466,6 +1466,33 @@ void omap_mcbsp_set_spi_mode(unsigned int id, } EXPORT_SYMBOL(omap_mcbsp_set_spi_mode);
+/* + * The following functions are only required on an OMAP1-only build. + * mach-omap2/mcbsp.c contains the real functions + */ +#ifndef CONFIG_ARCH_OMAP2PLUS +int omap2_mcbsp_set_clks_src(u8 id, u8 fck_src_id) +{ + WARN(1, "%s: should never be called on an OMAP1-only kernel\n", + __func__); + return -EINVAL; +} + +void omap2_mcbsp1_mux_clkr_src(u8 mux) +{ + WARN(1, "%s: should never be called on an OMAP1-only kernel\n", + __func__); + return; +} + +void omap2_mcbsp1_mux_fsr_src(u8 mux) +{ + WARN(1, "%s: should never be called on an OMAP1-only kernel\n", + __func__); + return; +} +#endif + #ifdef CONFIG_ARCH_OMAP3 #define max_thres(m) (mcbsp->pdata->buffer_size) #define valid_threshold(m, val) ((val) <= max_thres(m))
On Mon, 22 Nov 2010 17:48:24 -0700 (MST) Paul Walmsley paul@pwsan.com wrote:
Signed-off-by: Janusz Krzysztofik jkrzyszt@tis.icnet.pl
Thanks for fixing this. What do you think about the following patch instead? It should avoid any compiler issues.
Hmm.. looks like Janusz's patch is still in Liam's for-2.6.37 branch only.
+/*
- The following functions are only required on an OMAP1-only build.
- mach-omap2/mcbsp.c contains the real functions
- */
+#ifndef CONFIG_ARCH_OMAP2PLUS +int omap2_mcbsp_set_clks_src(u8 id, u8 fck_src_id) +{
Would that create a new problem if we are able to compile some day omap1 and omap2 support into same kernel? I agree with you that passing these via platform_data sounds the right solution.
On Tue, 2010-11-23 at 09:26 +0200, Jarkko Nikula wrote:
On Mon, 22 Nov 2010 17:48:24 -0700 (MST) Paul Walmsley paul@pwsan.com wrote:
Signed-off-by: Janusz Krzysztofik jkrzyszt@tis.icnet.pl
Thanks for fixing this. What do you think about the following patch instead? It should avoid any compiler issues.
Hmm.. looks like Janusz's patch is still in Liam's for-2.6.37 branch only.
Sorry, this pull request was waiting on Takashi's return from vacation. I'll be doing a pull request shortly.
Thanks
Liam
* Jarkko Nikula jhnikula@gmail.com [101122 23:16]:
On Mon, 22 Nov 2010 17:48:24 -0700 (MST) Paul Walmsley paul@pwsan.com wrote:
Signed-off-by: Janusz Krzysztofik jkrzyszt@tis.icnet.pl
Thanks for fixing this. What do you think about the following patch instead? It should avoid any compiler issues.
Hmm.. looks like Janusz's patch is still in Liam's for-2.6.37 branch only.
+/*
- The following functions are only required on an OMAP1-only build.
- mach-omap2/mcbsp.c contains the real functions
- */
+#ifndef CONFIG_ARCH_OMAP2PLUS +int omap2_mcbsp_set_clks_src(u8 id, u8 fck_src_id) +{
Would that create a new problem if we are able to compile some day omap1 and omap2 support into same kernel? I agree with you that passing these via platform_data sounds the right solution.
This should not be a problem because there's no ifdef else and in that case the functions would be there.
In general, the way to avoid problems like this is to make all code under drivers arch independent and pass the platform data from arch specific code.
In this case the ASoC platform data should be passed from arch/arm/*omap*/ board-*.c files to make the drivers generic.
We are going to get rid of cpu_is_omapxxxx macros in the drivers, so please consider this when patching the ASoC drivers.
Anyways, Paul's patch should be merged during the -rc cycle because it is currently blocking Amstrad delta being built:
http://armlinux.simtec.co.uk/kautobuild/
Acked-by: Tony Lindgren tony@atomide.com
* Tony Lindgren tony@atomide.com [101124 15:25]:
Anyways, Paul's patch should be merged during the -rc cycle because it is currently blocking Amstrad delta being built:
Turns out I'm several hours behind.. The build is working with Janusz commit 233538501f707b0176f09af7039fec1e3fcac6e7 now in mainline.
I guess no need to merge Paul's fix then?
Tony
On Wed, 24 Nov 2010 15:53:42 -0800 Tony Lindgren tony@atomide.com wrote:
- Tony Lindgren tony@atomide.com [101124 15:25]:
Anyways, Paul's patch should be merged during the -rc cycle because it is currently blocking Amstrad delta being built:
Turns out I'm several hours behind.. The build is working with Janusz commit 233538501f707b0176f09af7039fec1e3fcac6e7 now in mainline.
I guess no need to merge Paul's fix then?
Not necessary I think if compilers are happy with Janusz's fix. If not, then very strong reasong to start looking for platform data solution as soon as possible :-)
By the way, ASoC and OMAP audio-hackers, I deeply regret how much trouble that SCM cleanup series has caused. It does not meet my personal standards for a patch-series...
- Paul
On Mon, 22 Nov 2010 17:50:54 -0700 (MST) Paul Walmsley paul@pwsan.com wrote:
By the way, ASoC and OMAP audio-hackers, I deeply regret how much trouble that SCM cleanup series has caused. It does not meet my personal standards for a patch-series...
Hey, small compile breaks happens to everyone and word trouble doesn't come to my mind when speaking about your set.
Your set actually did one major step forward with ASoC OMAP drivers - now it's possible to have modular build with them so your work has lot of benefits to developers and distro builders.
-
Janusz Krzysztofik -
Jarkko Nikula -
Liam Girdwood -
Mark Brown -
Paul Walmsley -
Tony Lindgren