[RFC PATCH 4/5] ASoC: remove those unnecessary #ifdef CONFIG_PXA3xx .. #endif

[RFC PATCH 4/5] ASoC: remove those unnecessary #ifdef CONFIG_PXA3xx .. #endif

[Eric Miao]

These are unnecessary since the compiler is smart enough to optimize
the code away if CONFIG_PXA3xx is not defined, when cpu_is_pxa3xx()
will just be expanded to constant 0.

Signed-off-by: Eric Miao <eric.miao@marvell.com>
---
 sound/soc/pxa/pxa-ssp.c |   15 ---------------
 1 files changed, 0 insertions(+), 15 deletions(-)

diff --git a/sound/soc/pxa/pxa-ssp.c b/sound/soc/pxa/pxa-ssp.c
index a65993c..2ce4fb6 100644
--- a/sound/soc/pxa/pxa-ssp.c
+++ b/sound/soc/pxa/pxa-ssp.c
@@ -360,24 +360,20 @@ static int pxa_ssp_set_dai_clkdiv(struct
snd_soc_dai *cpu_dai,
 	case PXA_SSP_AUDIO_DIV_SCDB:
 		val = ssp_read_reg(ssp, SSACD);
 		val &= ~SSACD_SCDB;
-#if defined(CONFIG_PXA3xx)
 		if (cpu_is_pxa3xx())
 			val &= ~SSACD_SCDX8;
-#endif
 		switch (div) {
 		case PXA_SSP_CLK_SCDB_1:
 			val |= SSACD_SCDB;
 			break;
 		case PXA_SSP_CLK_SCDB_4:
 			break;
-#if defined(CONFIG_PXA3xx)
 		case PXA_SSP_CLK_SCDB_8:
 			if (cpu_is_pxa3xx())
 				val |= SSACD_SCDX8;
 			else
 				return -EINVAL;
 			break;
-#endif
 		default:
 			return -EINVAL;
 		}
@@ -403,10 +399,8 @@ static int pxa_ssp_set_dai_pll(struct snd_soc_dai *cpu_dai,
 	struct ssp_device *ssp = priv->ssp;
 	u32 ssacd = ssp_read_reg(ssp, SSACD) & ~0x70;

-#if defined(CONFIG_PXA3xx)
 	if (cpu_is_pxa3xx())
 		ssp_write_reg(ssp, SSACDD, 0);
-#endif

 	switch (freq_out) {
 	case 5622000:
@@ -431,7 +425,6 @@ static int pxa_ssp_set_dai_pll(struct snd_soc_dai *cpu_dai,
 		break;

 	default:
-#ifdef CONFIG_PXA3xx
 		/* PXA3xx has a clock ditherer which can be used to generate
 		 * a wider range of frequencies - calculate a value for it.
 		 */
@@ -452,8 +445,6 @@ static int pxa_ssp_set_dai_pll(struct snd_soc_dai *cpu_dai,
 				val, freq_out);
 			break;
 		}
-#endif
-
 		return -EINVAL;
 	}

@@ -652,10 +643,8 @@ static int pxa_ssp_hw_params(struct
snd_pcm_substream *substream,
 	sscr0 = ssp_read_reg(ssp, SSCR0);
 	switch (params_format(params)) {
 	case SNDRV_PCM_FORMAT_S16_LE:
-#ifdef CONFIG_PXA3xx
 		if (cpu_is_pxa3xx())
 			sscr0 |= SSCR0_FPCKE;
-#endif
 		sscr0 |= SSCR0_DataSize(16);
 		break;
 	case SNDRV_PCM_FORMAT_S24_LE:
@@ -682,7 +671,6 @@ static int pxa_ssp_hw_params(struct
snd_pcm_substream *substream,
 			* needed for that mode are only available on PXA3xx.
 			*/

-#ifdef CONFIG_PXA3xx
 			if (!cpu_is_pxa3xx())
 				return -EINVAL;

@@ -691,9 +679,6 @@ static int pxa_ssp_hw_params(struct
snd_pcm_substream *substream,
 			sspsp |= SSPSP_EDMYSTOP(3);
 			sspsp |= SSPSP_DMYSTOP(3);
 			sspsp |= SSPSP_DMYSTRT(1);
-#else
-			return -EINVAL;
-#endif
 		} else {
 			/* The frame width is the width the LRCLK is
 			 * asserted for; the delay is expressed in
-- 
1.6.0.4

-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php

Re: [RFC PATCH 4/5] ASoC: remove those unnecessary #ifdef CONFIG_PXA3xx .. #endif

[Daniel Mack]

Hi Eric,

On Thu, Apr 23, 2009 at 01:06:55PM +0800, Eric Miao wrote:
> @@ -652,10 +643,8 @@ static int pxa_ssp_hw_params(struct
> snd_pcm_substream *substream,
>  	sscr0 = ssp_read_reg(ssp, SSCR0);
>  	switch (params_format(params)) {
>  	case SNDRV_PCM_FORMAT_S16_LE:
> -#ifdef CONFIG_PXA3xx
>  		if (cpu_is_pxa3xx())
>  			sscr0 |= SSCR0_FPCKE;
> -#endif
>  		sscr0 |= SSCR0_DataSize(16);
>  		break;
>  	case SNDRV_PCM_FORMAT_S24_LE:
> @@ -682,7 +671,6 @@ static int pxa_ssp_hw_params(struct
> snd_pcm_substream *substream,
>  			* needed for that mode are only available on PXA3xx.
>  			*/
> 
> -#ifdef CONFIG_PXA3xx
>  			if (!cpu_is_pxa3xx())
>  				return -EINVAL;
> 
> @@ -691,9 +679,6 @@ static int pxa_ssp_hw_params(struct
> snd_pcm_substream *substream,
>  			sspsp |= SSPSP_EDMYSTOP(3);
>  			sspsp |= SSPSP_DMYSTOP(3);
>  			sspsp |= SSPSP_DMYSTRT(1);

These macros are only defined for PXA3xx since the register bits are not
used on other PXAs. Hence, the usage of these macros must be conditional
as well. Was this ever compiled for PXA2xx?

Daniel

Re: [RFC PATCH 4/5] ASoC: remove those unnecessary #ifdef CONFIG_PXA3xx .. #endif

[Mark Brown]

On Thu, Apr 23, 2009 at 01:06:55PM +0800, Eric Miao wrote:
> These are unnecessary since the compiler is smart enough to optimize
> the code away if CONFIG_PXA3xx is not defined, when cpu_is_pxa3xx()
> will just be expanded to constant 0.

This isn't done for compiler optimisation - it's done because the
register bit macros are only defined if PXA3xx support is being built
in.  I've no problem with removing those guards but without that you'll
get build failures on PXA2xx.

Re: [RFC PATCH 4/5] ASoC: remove those unnecessary #ifdef CONFIG_PXA3xx .. #endif

[Eric Miao]

On Thu, Apr 23, 2009 at 4:17 PM, Mark Brown <broonie@sirena.org.uk> wrote:
> On Thu, Apr 23, 2009 at 01:06:55PM +0800, Eric Miao wrote:
>> These are unnecessary since the compiler is smart enough to optimize
>> the code away if CONFIG_PXA3xx is not defined, when cpu_is_pxa3xx()
>> will just be expanded to constant 0.
>
> This isn't done for compiler optimisation - it's done because the
> register bit macros are only defined if PXA3xx support is being built
> in.  I've no problem with removing those guards but without that you'll
> get build failures on PXA2xx.
>

Yeah, indeed. This is a rush (so the title is RFC), sorry. I'll get those
conditional #ifdef .. #endif removed as well in the ssp-regs.h, it
always makes me upset. And possibly to get ssp-regs.h merged
into ssp.h. No one else now wants to use either one of these
header files.

-- 
Cheers
- eric
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [RFC PATCH 4/5] ASoC: remove those unnecessary #ifdef CONFIG_PXA3xx .. #endif

[Daniel Mack]

On Thu, Apr 23, 2009 at 04:33:34PM +0800, Eric Miao wrote:
> On Thu, Apr 23, 2009 at 4:17 PM, Mark Brown <broonie@sirena.org.uk> wrote:
> > This isn't done for compiler optimisation - it's done because the
> > register bit macros are only defined if PXA3xx support is being built
> > in.  I've no problem with removing those guards but without that you'll
> > get build failures on PXA2xx.
> >
> 
> Yeah, indeed. This is a rush (so the title is RFC), sorry. I'll get those
> conditional #ifdef .. #endif removed as well in the ssp-regs.h, it
> always makes me upset.

Well, I put them in there to clearly state which CPUs have support
for this particular feature. Especially for PXAs, where even register
definitions are considered confidential, this might help people fishing
in muddy waters. Some comment would do as well, though.

Daniel

Re: [RFC PATCH 4/5] ASoC: remove those unnecessary #ifdef CONFIG_PXA3xx .. #endif

[Eric Miao]

On Thu, Apr 23, 2009 at 5:04 PM, Daniel Mack <daniel@caiaq.de> wrote:
> On Thu, Apr 23, 2009 at 04:33:34PM +0800, Eric Miao wrote:
>> On Thu, Apr 23, 2009 at 4:17 PM, Mark Brown <broonie@sirena.org.uk> wrote:
>> > This isn't done for compiler optimisation - it's done because the
>> > register bit macros are only defined if PXA3xx support is being built
>> > in.  I've no problem with removing those guards but without that you'll
>> > get build failures on PXA2xx.
>> >
>>
>> Yeah, indeed. This is a rush (so the title is RFC), sorry. I'll get those
>> conditional #ifdef .. #endif removed as well in the ssp-regs.h, it
>> always makes me upset.
>
> Well, I put them in there to clearly state which CPUs have support
> for this particular feature. Especially for PXAs, where even register
> definitions are considered confidential, this might help people fishing
> in muddy waters. Some comment would do as well, though.
>

Yeah, I fully understand that. Actually these are inclusive #ifdef ..
which actually doesn't hurt. (Unlike #ifdef .. #else .. #endif).

But since the compiler can actually optimize it away, and it's quite
obvious pxa3xx specific by if (cpu_is_pxa3xx()) .. , I'm a bit intended
to remove them away, no?
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel