[PATCH 2.6.37-rc1] ASoC: OMAP: fix OMAP1 compilation problem

[PATCH 2.6.37-rc1] ASoC: OMAP: fix OMAP1 compilation problem

[Janusz Krzysztofik]

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:

Re: [PATCH 2.6.37-rc1] ASoC: OMAP: fix OMAP1 compilation problem

[Mark Brown]

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;
>  		omap2_mcbsp1_mux_clkr_src(CLKR_SRC_CLKR);

I guess this will still fail with optimisation diabled, but I'm not sure
anyone ever builds the kernel like that.

Re: [PATCH 2.6.37-rc1] ASoC: OMAP: fix OMAP1 compilation problem

[Paul Walmsley]

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

Re: [PATCH 2.6.37-rc1] ASoC: OMAP: fix OMAP1 compilation problem

[Jarkko Nikula]

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>

Re: [alsa-devel] [PATCH 2.6.37-rc1] ASoC: OMAP: fix OMAP1 compilation problem

[Liam Girdwood]

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
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH 2.6.37-rc1] ASoC: OMAP: fix OMAP1 compilation problem

[Paul Walmsley]

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))
-- 
1.7.2.3

Re: [PATCH 2.6.37-rc1] ASoC: OMAP: fix OMAP1 compilation problem

[Paul Walmsley]

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

Re: [PATCH 2.6.37-rc1] ASoC: OMAP: fix OMAP1 compilation problem

[Paul Walmsley]

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

Re: [PATCH 2.6.37-rc1] ASoC: OMAP: fix OMAP1 compilation problem

[Jarkko Nikula]

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.


-- 
Jarkko

Re: [PATCH 2.6.37-rc1] ASoC: OMAP: fix OMAP1 compilation problem

[Jarkko Nikula]

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.


-- 
Jarkko

Re: [alsa-devel] [PATCH 2.6.37-rc1] ASoC: OMAP: fix OMAP1 compilation problem

[Liam Girdwood]

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
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH 2.6.37-rc1] ASoC: OMAP: fix OMAP1 compilation problem

[Tony Lindgren]

* 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>

Re: [PATCH 2.6.37-rc1] ASoC: OMAP: fix OMAP1 compilation problem

[Tony Lindgren]

* 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:
> 
> http://armlinux.simtec.co.uk/kautobuild/

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

Re: [PATCH 2.6.37-rc1] ASoC: OMAP: fix OMAP1 compilation problem

[Jarkko Nikula]

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:
> > 
> > http://armlinux.simtec.co.uk/kautobuild/
> 
> 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 :-)


-- 
Jarkko

Re: [PATCH 2.6.37-rc1] ASoC: OMAP: fix OMAP1 compilation problem

[Janusz Krzysztofik]

dropping e3-hacking, the problem has been solved from their POV

Thursday 25 November 2010 08:02:01 Jarkko Nikula wrote:
> 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:
> > >
> > > http://armlinux.simtec.co.uk/kautobuild/
> >
> > 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 :-)

Fine with me if you prefere reverting my changes and using Paul's fix as a 
better short term solution instead.

Thanks,
Janusz

Re: [PATCH 2.6.37-rc1] ASoC: OMAP: fix OMAP1 compilation problem

[Paul Walmsley]

On Thu, 25 Nov 2010, Janusz Krzysztofik wrote:

> Thursday 25 November 2010 08:02:01 Jarkko Nikula wrote:
> > 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:
> > > >
> > > > http://armlinux.simtec.co.uk/kautobuild/
> > >
> > > 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 :-)
> 
> Fine with me if you prefere reverting my changes and using Paul's fix as a 
> better short term solution instead.

And I'm fine with Janusz's patch.  My patch was written in response to 
Mark's comments about compilers...


- Paul