diff mbox

ASoC: tegra: depend on ARCH_TEGRA, not ARCH_TEGRA_*

Message ID 1374013667-21435-1-git-send-email-swarren@wwwdotorg.org (mailing list archive)
State New, archived
Headers show

Commit Message

Stephen Warren July 16, 2013, 10:27 p.m. UTC 
From: Stephen Warren <swarren@nvidia.com>

I'm planning to remove CONFIG_ARCH_TEGRA_*_SOC, leaving just ARCH_TEGRA.
This will reduce the number of configurations that need to be supported
by core Tegra code, e.g. CPU idle, hotplug. As a precursor, we need to
update Kconfig for all Tegra drivers not to reference ARCH_TEGRA_*_SOC.

Stop selecting I2S/AC97/SPDIF controller drivers from the machine driver
config options; this doesn't really work if we don't know which specific
SoC(s) we're building for. However, set their default values based on
SND_SOC_TEGRA, since most people will probably want to enable support for
all SoCs. This also avoids the need to change any defconfig files.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 sound/soc/tegra/Kconfig | 36 +++++++++++++++---------------------
 1 file changed, 15 insertions(+), 21 deletions(-)

Comments

Mark Brown July 17, 2013, 8:30 a.m. UTC | #1 
On Tue, Jul 16, 2013 at 04:27:47PM -0600, Stephen Warren wrote:

> Stop selecting I2S/AC97/SPDIF controller drivers from the machine driver
> config options; this doesn't really work if we don't know which specific
> SoC(s) we're building for. However, set their default values based on
> SND_SOC_TEGRA, since most people will probably want to enable support for
> all SoCs. This also avoids the need to change any defconfig files.

This doesn't seem terribly clever and is definitely not idiomatic for
ASoC.  If you want to just select all CPUs that'd be fine but forcing
the user to select the individual components isn't the style anything
else uses.
Stephen Warren July 17, 2013, 5:01 p.m. UTC | #2 
On 07/17/2013 02:30 AM, Mark Brown wrote:
> On Tue, Jul 16, 2013 at 04:27:47PM -0600, Stephen Warren wrote:
> 
>> Stop selecting I2S/AC97/SPDIF controller drivers from the machine
>> driver config options; this doesn't really work if we don't know
>> which specific SoC(s) we're building for. However, set their
>> default values based on SND_SOC_TEGRA, since most people will
>> probably want to enable support for all SoCs. This also avoids
>> the need to change any defconfig files.
> 
> This doesn't seem terribly clever and is definitely not idiomatic
> for ASoC.  If you want to just select all CPUs that'd be fine but
> forcing the user to select the individual components isn't the
> style anything else uses.

So I think what you're saying is that machine drivers should be
changed like:

-       select SND_SOC_TEGRA20_I2S if ARCH_TEGRA_2x_SOC
-       select SND_SOC_TEGRA30_I2S if ARCH_TEGRA_3x_SOC
+       select SND_SOC_TEGRA20_I2S
+       select SND_SOC_TEGRA30_I2S

But then, it won't be possible to disable support for older SoCs,
since simply enabling a machine driver that might support Tegra20
would force ASoC support for Tegra20 to be enabled, even if the user
only cares about Tegra30.
Mark Brown July 17, 2013, 5:57 p.m. UTC | #3 
On Wed, Jul 17, 2013 at 11:01:30AM -0600, Stephen Warren wrote:
> On 07/17/2013 02:30 AM, Mark Brown wrote:

> -       select SND_SOC_TEGRA20_I2S if ARCH_TEGRA_2x_SOC
> -       select SND_SOC_TEGRA30_I2S if ARCH_TEGRA_3x_SOC
> +       select SND_SOC_TEGRA20_I2S
> +       select SND_SOC_TEGRA30_I2S

> But then, it won't be possible to disable support for older SoCs,
> since simply enabling a machine driver that might support Tegra20
> would force ASoC support for Tegra20 to be enabled, even if the user
> only cares about Tegra30.

So how do they disable the core support for the older SoCs with the new
model?
Stephen Warren July 17, 2013, 6:23 p.m. UTC | #4 
On 07/17/2013 11:57 AM, Mark Brown wrote:
> On Wed, Jul 17, 2013 at 11:01:30AM -0600, Stephen Warren wrote:
>> On 07/17/2013 02:30 AM, Mark Brown wrote:
> 
>> -       select SND_SOC_TEGRA20_I2S if ARCH_TEGRA_2x_SOC -
>> select SND_SOC_TEGRA30_I2S if ARCH_TEGRA_3x_SOC +       select
>> SND_SOC_TEGRA20_I2S +       select SND_SOC_TEGRA30_I2S
> 
>> But then, it won't be possible to disable support for older
>> SoCs, since simply enabling a machine driver that might support
>> Tegra20 would force ASoC support for Tegra20 to be enabled, even
>> if the user only cares about Tegra30.
> 
> So how do they disable the core support for the older SoCs with the
> new model?

They don't; the core support is so small it's not worth having the
ifdefs in it; just a few K. As such, it seems simpler to just always
compile in the core support, and remove the need for all the ifdef
nests in mach-tegra/. The bulk of the differences are different
drivers for different chips.
Mark Brown July 17, 2013, 10:33 p.m. UTC | #5 
On Wed, Jul 17, 2013 at 12:23:03PM -0600, Stephen Warren wrote:
> On 07/17/2013 11:57 AM, Mark Brown wrote:

> > So how do they disable the core support for the older SoCs with the
> > new model?

> They don't; the core support is so small it's not worth having the
> ifdefs in it; just a few K. As such, it seems simpler to just always
> compile in the core support, and remove the need for all the ifdef
> nests in mach-tegra/. The bulk of the differences are different
> drivers for different chips.

So my take on that is that it seems like it's simpler to just have the
core selection since that's just one option for the user to choose and
save all the space for whatever device they're not interested in rather
than having to go through individual drivers disabling the boring SoCs.
It seems more straightfoward from a UI point of view but perhaps I'm not
thinking about it from the right angle?
Stephen Warren July 17, 2013, 10:52 p.m. UTC | #6 
On 07/17/2013 04:33 PM, Mark Brown wrote:
> On Wed, Jul 17, 2013 at 12:23:03PM -0600, Stephen Warren wrote:
>> On 07/17/2013 11:57 AM, Mark Brown wrote:
> 
>>> So how do they disable the core support for the older SoCs with
>>> the new model?
> 
>> They don't; the core support is so small it's not worth having
>> the ifdefs in it; just a few K. As such, it seems simpler to just
>> always compile in the core support, and remove the need for all
>> the ifdef nests in mach-tegra/. The bulk of the differences are
>> different drivers for different chips.
> 
> So my take on that is that it seems like it's simpler to just have
> the core selection since that's just one option for the user to
> choose and save all the space for whatever device they're not
> interested in rather than having to go through individual drivers
> disabling the boring SoCs. It seems more straightfoward from a UI
> point of view but perhaps I'm not thinking about it from the right
> angle?

Ah, so you mean keep the options in mach-tegra/Kconfig so that there's
a single place to configure per-SoC support, but just ignore those
options in code (like the Tegra CPU reset vector) where it's not worth
it. That sounds reasonable.

The one remaining issue is that we have plenty of HW module whose
driver works across multiple HW generations. For example,
tegra20_i2s.c supports only Tegra20, yet tegra30_i2s.c supports both
Tegra30 and Tegra114, and likely will support other generations too.
The key to enable tegra30_i2s.c should really be ARCH_TEGRA_3x_SOC ||
ARCH_TEGRA_114_SOC, which gets a bit unwieldy once you have, say, 4
different SoCs in that list and a similar list starts to apply to a
lot of different HW-modules/drivers. Even creating an
ARCH_TEGRA_30_OR_LATER_SOC will get problematic, since defining "or
later" will likely become progressively more complex over time, as we
start putting our more SoCs. What are your thoughts on the best way to
solve that?
Mark Brown July 18, 2013, 9:58 a.m. UTC | #7 
On Wed, Jul 17, 2013 at 04:52:42PM -0600, Stephen Warren wrote:

> The key to enable tegra30_i2s.c should really be ARCH_TEGRA_3x_SOC ||
> ARCH_TEGRA_114_SOC, which gets a bit unwieldy once you have, say, 4
> different SoCs in that list and a similar list starts to apply to a
> lot of different HW-modules/drivers. Even creating an
> ARCH_TEGRA_30_OR_LATER_SOC will get problematic, since defining "or
> later" will likely become progressively more complex over time, as we
> start putting our more SoCs. What are your thoughts on the best way to
> solve that?

Can't you just say Tegra 20 or later is Tegra 20 plus Tegra 30 or later
and so on?
Stephen Warren July 18, 2013, 4:58 p.m. UTC | #8 
On 07/18/2013 03:58 AM, Mark Brown wrote:
> On Wed, Jul 17, 2013 at 04:52:42PM -0600, Stephen Warren wrote:
> 
>> The key to enable tegra30_i2s.c should really be
>> ARCH_TEGRA_3x_SOC || ARCH_TEGRA_114_SOC, which gets a bit
>> unwieldy once you have, say, 4 different SoCs in that list and a
>> similar list starts to apply to a lot of different
>> HW-modules/drivers. Even creating an ARCH_TEGRA_30_OR_LATER_SOC
>> will get problematic, since defining "or later" will likely
>> become progressively more complex over time, as we start putting
>> our more SoCs. What are your thoughts on the best way to solve
>> that?
> 
> Can't you just say Tegra 20 or later is Tegra 20 plus Tegra 30 or
> later and so on?

For the chips that are supported upstream right now, that would work.
However, I'm not sure that life will be quite so linear in the future.
In other words, what is "or later" for, say, the SDHCI controller may
not be the same as what is "or later" for, say, the I2C controller.
(feel free to substitute arbitrary HW modules into that statement).
diff mbox

Patch

diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
index 68fd6d0..321a6c9 100644
--- a/sound/soc/tegra/Kconfig
+++ b/sound/soc/tegra/Kconfig
@@ -7,8 +7,9 @@  config SND_SOC_TEGRA
 	  Say Y or M here if you want support for SoC audio on Tegra.
 
 config SND_SOC_TEGRA20_AC97
-	tristate
-	depends on SND_SOC_TEGRA && ARCH_TEGRA_2x_SOC
+	tristate "Tegra20 AC97 controller"
+	depends on SND_SOC_TEGRA
+	default SND_SOC_TEGRA
 	select SND_SOC_AC97_BUS
 	select SND_SOC_TEGRA20_DAS
 	help
@@ -18,15 +19,16 @@  config SND_SOC_TEGRA20_AC97
 
 config SND_SOC_TEGRA20_DAS
 	tristate
-	depends on SND_SOC_TEGRA && ARCH_TEGRA_2x_SOC
+	depends on SND_SOC_TEGRA
 	help
 	  Say Y or M if you want to add support for the Tegra20 DAS module.
 	  You will also need to select the individual machine drivers to
 	  support below.
 
 config SND_SOC_TEGRA20_I2S
-	tristate
-	depends on SND_SOC_TEGRA && ARCH_TEGRA_2x_SOC
+	tristate "Tegra20 I2C controller"
+	depends on SND_SOC_TEGRA
+	default SND_SOC_TEGRA
 	select SND_SOC_TEGRA20_DAS
 	help
 	  Say Y or M if you want to add support for codecs attached to the
@@ -34,9 +36,9 @@  config SND_SOC_TEGRA20_I2S
 	  machine drivers to support below.
 
 config SND_SOC_TEGRA20_SPDIF
-	tristate
-	depends on SND_SOC_TEGRA && ARCH_TEGRA_2x_SOC
-	default m
+	tristate "Tegra20 SPDIF controller"
+	default SND_SOC_TEGRA
+	depends on SND_SOC_TEGRA
 	help
 	  Say Y or M if you want to add support for the Tegra20 SPDIF interface.
 	  You will also need to select the individual machine drivers to support
@@ -44,15 +46,16 @@  config SND_SOC_TEGRA20_SPDIF
 
 config SND_SOC_TEGRA30_AHUB
 	tristate
-	depends on SND_SOC_TEGRA && ARCH_TEGRA_3x_SOC
+	depends on SND_SOC_TEGRA
 	help
 	  Say Y or M if you want to add support for the Tegra20 AHUB module.
 	  You will also need to select the individual machine drivers to
 	  support below.
 
 config SND_SOC_TEGRA30_I2S
-	tristate
-	depends on SND_SOC_TEGRA && ARCH_TEGRA_3x_SOC
+	tristate "Tegra30 I2S controller"
+	depends on SND_SOC_TEGRA
+	default SND_SOC_TEGRA
 	select SND_SOC_TEGRA30_AHUB
 	help
 	  Say Y or M if you want to add support for codecs attached to the
@@ -62,8 +65,6 @@  config SND_SOC_TEGRA30_I2S
 config SND_SOC_TEGRA_RT5640
 	tristate "SoC Audio support for Tegra boards using an RT5640 codec"
 	depends on SND_SOC_TEGRA && I2C && GPIOLIB
-	select SND_SOC_TEGRA20_I2S if ARCH_TEGRA_2x_SOC
-	select SND_SOC_TEGRA30_I2S if ARCH_TEGRA_3x_SOC
 	select SND_SOC_RT5640
 	help
 	  Say Y or M here if you want to add support for SoC audio on Tegra
@@ -72,8 +73,6 @@  config SND_SOC_TEGRA_RT5640
 config SND_SOC_TEGRA_WM8753
 	tristate "SoC Audio support for Tegra boards using a WM8753 codec"
 	depends on SND_SOC_TEGRA && I2C && GPIOLIB
-	select SND_SOC_TEGRA20_I2S if ARCH_TEGRA_2x_SOC
-	select SND_SOC_TEGRA30_I2S if ARCH_TEGRA_3x_SOC
 	select SND_SOC_WM8753
 	help
 	  Say Y or M here if you want to add support for SoC audio on Tegra
@@ -82,8 +81,6 @@  config SND_SOC_TEGRA_WM8753
 config SND_SOC_TEGRA_WM8903
 	tristate "SoC Audio support for Tegra boards using a WM8903 codec"
 	depends on SND_SOC_TEGRA && I2C && GPIOLIB
-	select SND_SOC_TEGRA20_I2S if ARCH_TEGRA_2x_SOC
-	select SND_SOC_TEGRA30_I2S if ARCH_TEGRA_3x_SOC
 	select SND_SOC_WM8903
 	help
 	  Say Y or M here if you want to add support for SoC audio on Tegra
@@ -92,8 +89,7 @@  config SND_SOC_TEGRA_WM8903
 
 config SND_SOC_TEGRA_WM9712
 	tristate "SoC Audio support for Tegra boards using a WM9712 codec"
-	depends on SND_SOC_TEGRA && ARCH_TEGRA_2x_SOC && GPIOLIB
-	select SND_SOC_TEGRA20_AC97
+	depends on SND_SOC_TEGRA && GPIOLIB
 	select SND_SOC_WM9712
 	help
 	  Say Y or M here if you want to add support for SoC audio on Tegra
@@ -102,7 +98,6 @@  config SND_SOC_TEGRA_WM9712
 config SND_SOC_TEGRA_TRIMSLICE
 	tristate "SoC Audio support for TrimSlice board"
 	depends on SND_SOC_TEGRA && I2C
-	select SND_SOC_TEGRA20_I2S if ARCH_TEGRA_2x_SOC
 	select SND_SOC_TLV320AIC23
 	help
 	  Say Y or M here if you want to add support for SoC audio on the
@@ -111,7 +106,6 @@  config SND_SOC_TEGRA_TRIMSLICE
 config SND_SOC_TEGRA_ALC5632
 	tristate "SoC Audio support for Tegra boards using an ALC5632 codec"
 	depends on SND_SOC_TEGRA && I2C && GPIOLIB
-	select SND_SOC_TEGRA20_I2S if ARCH_TEGRA_2x_SOC
 	select SND_SOC_ALC5632
 	help
 	  Say Y or M here if you want to add support for SoC audio on the