subject:"\[PATCH 3\/3\] sound\: ASoC\: tegra\: Select tegra30 i2s and ahub for tegra124 SoC"

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-05-31 Thread Paul Kocialkowski

Hi,

Le lundi 08 mai 2017 à 09:58 -0600, Stephen Warren a écrit :
> On 05/07/2017 12:12 PM, Paul Kocialkowski wrote:
> > Hi,
> > 
> > Le mardi 25 avril 2017 à 10:57 -0600, Stephen Warren a écrit :
> > > On 04/24/2017 12:41 PM, Paul Kocialkowski wrote:
> > > > Le lundi 24 avril 2017 à 09:35 -0600, Stephen Warren a écrit :
> > > > > On 04/24/2017 09:07 AM, Paul Kocialkowski wrote:
> > > > > > Le mercredi 19 avril 2017 à 16:00 -0600, Stephen Warren a écrit :
> > > > > > > On 04/18/2017 10:38 AM, Paul Kocialkowski wrote:
> > > > > > > > Le mardi 18 avril 2017 à 10:15 -0600, Stephen Warren a écrit :
> > > > > > > > > On 04/18/2017 09:11 AM, Paul Kocialkowski wrote:
> > > > > > > > > > This selects the tegra30 i2s and ahub controllers for the
> > > > > > > > > > tegra124
> > > > > > > > > > SoC.
> > > > > > > > > > These are needed when building without ARCH_TEGRA_3x_SOC
> > > > > > > > > > set.
> > > > > > > > > > diff --git a/sound/soc/tegra/Kconfig
> > > > > > > > > > b/sound/soc/tegra/Kconfig
> > > > > > > > > > index efbe8d4c019e..bcd18d2cf7a7 100644
> > > > > > > > > > --- a/sound/soc/tegra/Kconfig
> > > > > > > > > > +++ b/sound/soc/tegra/Kconfig
> > > > > > > > > > @@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC ||
> > > > > > > > > > ARCH_TEGRA_124_SOC)
> > > > > > > > > 
> > > > > > > > > Is this really a compile-time dependency?
> > > > > > > > 
> > > > > > > > From a quick look at the code, I doubt this is really a build
> > > > > > > > dependency.
> > > > > > > > 
> > > > > > > > > If so, don't we need to add T210 and T186 entries into that ||
> > > > > > > > > condition
> > > > > > > > > too,
> > > > > > > > > since we could be building a kernel with just T210/T186
> > > > > > > > > support
> > > > > > > > > and no
> > > > > > > > > T124
> > > > > > > > > support?
> > > > > > > > 
> > > > > > > > In the spirit of this patch, adding entries for other tegra
> > > > > > > > platforms
> > > > > > > > would
> > > > > > > > make
> > > > > > > > sense. Would you prefer that we leave out the dependency from
> > > > > > > > SND_SOC_TEGRA30_*
> > > > > > > > and only select the right I2S driver to use in each codec
> > > > > > > > driver?
> > > > > > > > 
> > > > > > > > If not, we'd have to list all relevant platforms both in the
> > > > > > > > I2S/AHUB
> > > > > > > > drivers
> > > > > > > > and in each codec's rules (which is not necessarily and issue,
> > > > > > > > but
> > > > > > > > there's
> > > > > > > > no
> > > > > > > > need to have artificial platform dependencies).
> > > > > > > > 
> > > > > > > > What do you think?
> > > > > > > 
> > > > > > > I think we should just remove most of these "depends on" since
> > > > > > > they're
> > > > > > > mostly set up to reflect runtime requirements rather than build
> > > > > > > time
> > > > > > > requirements. The only points I'd make are:
> > > > > > 
> > > > > > I definitely agree we should do that for all the codec Kconfig
> > > > > > options.
> > > > > > 
> > > > > > > 1)
> > > > > > > 
> > > > > > > Everything should "depends on SND_SOC_TEGRA" simply so the options
> > > > > > > don't
> > > > > > > show up and clutter menuconfig menus unless SND_SOC_TEGRA is
> > > > > > > enabled.
> > > > > > 
> > > > > > Agreed.
> > > > > > 
> > > > > > > 2)
> > > > > > > 
> > > > > > > SND_SOC_TEGRA30_I2S does need the Tegra30 AHUB driver in order to
> > > > > > > compile/link, since it directly calls functions in that driver.
> > > > > > > This
> > > > > > > is
> > > > > > > already handled by SND_SOC_TEGRA30_I2S doing "select
> > > > > > > SND_SOC_TEGRA30_AHUB".
> > > > > > 
> > > > > > Agreed.
> > > > > > 
> > > > > > > 3)
> > > > > > > 
> > > > > > > The machine drivers all do e.g. "select SND_SOC_TEGRA30_I2S if
> > > > > > > ARCH_TEGRA_3x_SOC". This was an attempt to make the machine
> > > > > > > drivers
> > > > > > > only
> > > > > > > pull in the relevant drivers for the SoC(s) being compiled for.
> > > > > > > I'm
> > > > > > > not
> > > > > > > sure this still makes sense; this won't work on kernels that only
> > > > > > > support T124/T210/T186 since ARCH_TEGRA_3x_SOC isn't enabled then.
> > > > > > > Should we just remove all those and make sure the defconfigs are
> > > > > > > updated
> > > > > > > to make sure the relevant I2S/AHUB/SPDIF/AC97 drivers are
> > > > > > > explicitly
> > > > > > > enabled? Perhaps we should default all the I2S/AHUB/SPDIF/AC97 to
> > > > > > > y
> > > > > > > (which will only apply if SND_SOC_TEGRA is enabled)?
> > > > > > 
> > > > > > I think it would be easier for everyone to just auto-select the
> > > > > > machine
> > > > > > drivers
> > > > > > automatically based on the architecture (so we could have the list
> > > > > > of
> > > > > > ARCH_TEGRA_*_SOC here) when

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-05-31 Thread Paul Kocialkowski

Hi,

Le lundi 08 mai 2017 à 09:58 -0600, Stephen Warren a écrit :
> On 05/07/2017 12:12 PM, Paul Kocialkowski wrote:
> > Hi,
> > 
> > Le mardi 25 avril 2017 à 10:57 -0600, Stephen Warren a écrit :
> > > On 04/24/2017 12:41 PM, Paul Kocialkowski wrote:
> > > > Le lundi 24 avril 2017 à 09:35 -0600, Stephen Warren a écrit :
> > > > > On 04/24/2017 09:07 AM, Paul Kocialkowski wrote:
> > > > > > Le mercredi 19 avril 2017 à 16:00 -0600, Stephen Warren a écrit :
> > > > > > > On 04/18/2017 10:38 AM, Paul Kocialkowski wrote:
> > > > > > > > Le mardi 18 avril 2017 à 10:15 -0600, Stephen Warren a écrit :
> > > > > > > > > On 04/18/2017 09:11 AM, Paul Kocialkowski wrote:
> > > > > > > > > > This selects the tegra30 i2s and ahub controllers for the
> > > > > > > > > > tegra124
> > > > > > > > > > SoC.
> > > > > > > > > > These are needed when building without ARCH_TEGRA_3x_SOC
> > > > > > > > > > set.
> > > > > > > > > > diff --git a/sound/soc/tegra/Kconfig
> > > > > > > > > > b/sound/soc/tegra/Kconfig
> > > > > > > > > > index efbe8d4c019e..bcd18d2cf7a7 100644
> > > > > > > > > > --- a/sound/soc/tegra/Kconfig
> > > > > > > > > > +++ b/sound/soc/tegra/Kconfig
> > > > > > > > > > @@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC ||
> > > > > > > > > > ARCH_TEGRA_124_SOC)
> > > > > > > > > 
> > > > > > > > > Is this really a compile-time dependency?
> > > > > > > > 
> > > > > > > > From a quick look at the code, I doubt this is really a build
> > > > > > > > dependency.
> > > > > > > > 
> > > > > > > > > If so, don't we need to add T210 and T186 entries into that ||
> > > > > > > > > condition
> > > > > > > > > too,
> > > > > > > > > since we could be building a kernel with just T210/T186
> > > > > > > > > support
> > > > > > > > > and no
> > > > > > > > > T124
> > > > > > > > > support?
> > > > > > > > 
> > > > > > > > In the spirit of this patch, adding entries for other tegra
> > > > > > > > platforms
> > > > > > > > would
> > > > > > > > make
> > > > > > > > sense. Would you prefer that we leave out the dependency from
> > > > > > > > SND_SOC_TEGRA30_*
> > > > > > > > and only select the right I2S driver to use in each codec
> > > > > > > > driver?
> > > > > > > > 
> > > > > > > > If not, we'd have to list all relevant platforms both in the
> > > > > > > > I2S/AHUB
> > > > > > > > drivers
> > > > > > > > and in each codec's rules (which is not necessarily and issue,
> > > > > > > > but
> > > > > > > > there's
> > > > > > > > no
> > > > > > > > need to have artificial platform dependencies).
> > > > > > > > 
> > > > > > > > What do you think?
> > > > > > > 
> > > > > > > I think we should just remove most of these "depends on" since
> > > > > > > they're
> > > > > > > mostly set up to reflect runtime requirements rather than build
> > > > > > > time
> > > > > > > requirements. The only points I'd make are:
> > > > > > 
> > > > > > I definitely agree we should do that for all the codec Kconfig
> > > > > > options.
> > > > > > 
> > > > > > > 1)
> > > > > > > 
> > > > > > > Everything should "depends on SND_SOC_TEGRA" simply so the options
> > > > > > > don't
> > > > > > > show up and clutter menuconfig menus unless SND_SOC_TEGRA is
> > > > > > > enabled.
> > > > > > 
> > > > > > Agreed.
> > > > > > 
> > > > > > > 2)
> > > > > > > 
> > > > > > > SND_SOC_TEGRA30_I2S does need the Tegra30 AHUB driver in order to
> > > > > > > compile/link, since it directly calls functions in that driver.
> > > > > > > This
> > > > > > > is
> > > > > > > already handled by SND_SOC_TEGRA30_I2S doing "select
> > > > > > > SND_SOC_TEGRA30_AHUB".
> > > > > > 
> > > > > > Agreed.
> > > > > > 
> > > > > > > 3)
> > > > > > > 
> > > > > > > The machine drivers all do e.g. "select SND_SOC_TEGRA30_I2S if
> > > > > > > ARCH_TEGRA_3x_SOC". This was an attempt to make the machine
> > > > > > > drivers
> > > > > > > only
> > > > > > > pull in the relevant drivers for the SoC(s) being compiled for.
> > > > > > > I'm
> > > > > > > not
> > > > > > > sure this still makes sense; this won't work on kernels that only
> > > > > > > support T124/T210/T186 since ARCH_TEGRA_3x_SOC isn't enabled then.
> > > > > > > Should we just remove all those and make sure the defconfigs are
> > > > > > > updated
> > > > > > > to make sure the relevant I2S/AHUB/SPDIF/AC97 drivers are
> > > > > > > explicitly
> > > > > > > enabled? Perhaps we should default all the I2S/AHUB/SPDIF/AC97 to
> > > > > > > y
> > > > > > > (which will only apply if SND_SOC_TEGRA is enabled)?
> > > > > > 
> > > > > > I think it would be easier for everyone to just auto-select the
> > > > > > machine
> > > > > > drivers
> > > > > > automatically based on the architecture (so we could have the list
> > > > > > of
> > > > > > ARCH_TEGRA_*_SOC here) when

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-05-08 Thread Stephen Warren


On 05/07/2017 12:12 PM, Paul Kocialkowski wrote:

Hi,

Le mardi 25 avril 2017 à 10:57 -0600, Stephen Warren a écrit :

On 04/24/2017 12:41 PM, Paul Kocialkowski wrote:

Le lundi 24 avril 2017 à 09:35 -0600, Stephen Warren a écrit :

On 04/24/2017 09:07 AM, Paul Kocialkowski wrote:

Le mercredi 19 avril 2017 à 16:00 -0600, Stephen Warren a écrit :

On 04/18/2017 10:38 AM, Paul Kocialkowski wrote:

Le mardi 18 avril 2017 à 10:15 -0600, Stephen Warren a écrit :

On 04/18/2017 09:11 AM, Paul Kocialkowski wrote:

This selects the tegra30 i2s and ahub controllers for the
tegra124
SoC.
These are needed when building without ARCH_TEGRA_3x_SOC set.
diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
index efbe8d4c019e..bcd18d2cf7a7 100644
--- a/sound/soc/tegra/Kconfig
+++ b/sound/soc/tegra/Kconfig
@@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC ||
ARCH_TEGRA_124_SOC)


Is this really a compile-time dependency?


From a quick look at the code, I doubt this is really a build
dependency.


If so, don't we need to add T210 and T186 entries into that ||
condition
too,
since we could be building a kernel with just T210/T186 support
and no
T124
support?


In the spirit of this patch, adding entries for other tegra
platforms
would
make
sense. Would you prefer that we leave out the dependency from
SND_SOC_TEGRA30_*
and only select the right I2S driver to use in each codec driver?

If not, we'd have to list all relevant platforms both in the
I2S/AHUB
drivers
and in each codec's rules (which is not necessarily and issue, but
there's
no
need to have artificial platform dependencies).

What do you think?


I think we should just remove most of these "depends on" since they're
mostly set up to reflect runtime requirements rather than build time
requirements. The only points I'd make are:


I definitely agree we should do that for all the codec Kconfig options.


1)

Everything should "depends on SND_SOC_TEGRA" simply so the options
don't
show up and clutter menuconfig menus unless SND_SOC_TEGRA is enabled.


Agreed.


2)

SND_SOC_TEGRA30_I2S does need the Tegra30 AHUB driver in order to
compile/link, since it directly calls functions in that driver. This
is
already handled by SND_SOC_TEGRA30_I2S doing "select
SND_SOC_TEGRA30_AHUB".


Agreed.


3)

The machine drivers all do e.g. "select SND_SOC_TEGRA30_I2S if
ARCH_TEGRA_3x_SOC". This was an attempt to make the machine drivers
only
pull in the relevant drivers for the SoC(s) being compiled for. I'm
not
sure this still makes sense; this won't work on kernels that only
support T124/T210/T186 since ARCH_TEGRA_3x_SOC isn't enabled then.
Should we just remove all those and make sure the defconfigs are
updated
to make sure the relevant I2S/AHUB/SPDIF/AC97 drivers are explicitly
enabled? Perhaps we should default all the I2S/AHUB/SPDIF/AC97 to y
(which will only apply if SND_SOC_TEGRA is enabled)?


I think it would be easier for everyone to just auto-select the machine
drivers
automatically based on the architecture (so we could have the list of
ARCH_TEGRA_*_SOC here) when SND_SOC_TEGRA is selected.


I don't think selecting the machine drivers is the correct approach,
since then they can't be disabled.

Making certain machine drivers "default y if ARCH_TEGRA_nn_SOC" would
address that,


That's right, my mistake. Let's take that as the solution I'm backing then.


 but still isn't very scalable since we need to go back and
edit the Kconfig every time we define a new SoC, in order to add that
SoC into the default statement.


Well, that's what platform bringup is all about, isn't it? I think it makes
a
lot more sense to have to add a new platform once (and it's not like one
will
forget to look at the sound part when adding a new platform) rather than
requiring users to hand-pick the option.


Personally I'd expect all the Tegra machine drivers to be enabled in the
upstream defconfig files all the time, such that we don't need to make
the Kconfig options default y, nor have users manually turn the relevant
options on, at all.


I think I was previously mistaken about what the machine drivers are, so I got a
good occasion to find out about them an learn something new. Thanks!

Agreed regarding the machine drivers, they should indeed all be enabled in the
defconfig by default.


Not only does this preserve existing configs (including external ones
that
aren't part of the kernel tree), it also clearly maps which machine
driver
to use for which SoC instead of having users do it by hand.


The machine drivers aren't terribly tied to SoCs by design; most of them
would work on pretty much any SoC. They're only tied to SoCs as a
side-effect of a machine driver being tied to a certain CODEC, and
certain CODECS just by chance are only used (so far) on specific boards,
which have specific SoCs.


I'm a bit confused: aren't the

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-05-08 Thread Stephen Warren


On 05/07/2017 12:12 PM, Paul Kocialkowski wrote:

Hi,

Le mardi 25 avril 2017 à 10:57 -0600, Stephen Warren a écrit :

On 04/24/2017 12:41 PM, Paul Kocialkowski wrote:

Le lundi 24 avril 2017 à 09:35 -0600, Stephen Warren a écrit :

On 04/24/2017 09:07 AM, Paul Kocialkowski wrote:

Le mercredi 19 avril 2017 à 16:00 -0600, Stephen Warren a écrit :

On 04/18/2017 10:38 AM, Paul Kocialkowski wrote:

Le mardi 18 avril 2017 à 10:15 -0600, Stephen Warren a écrit :

On 04/18/2017 09:11 AM, Paul Kocialkowski wrote:

This selects the tegra30 i2s and ahub controllers for the
tegra124
SoC.
These are needed when building without ARCH_TEGRA_3x_SOC set.
diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
index efbe8d4c019e..bcd18d2cf7a7 100644
--- a/sound/soc/tegra/Kconfig
+++ b/sound/soc/tegra/Kconfig
@@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC ||
ARCH_TEGRA_124_SOC)


Is this really a compile-time dependency?


From a quick look at the code, I doubt this is really a build
dependency.


If so, don't we need to add T210 and T186 entries into that ||
condition
too,
since we could be building a kernel with just T210/T186 support
and no
T124
support?


In the spirit of this patch, adding entries for other tegra
platforms
would
make
sense. Would you prefer that we leave out the dependency from
SND_SOC_TEGRA30_*
and only select the right I2S driver to use in each codec driver?

If not, we'd have to list all relevant platforms both in the
I2S/AHUB
drivers
and in each codec's rules (which is not necessarily and issue, but
there's
no
need to have artificial platform dependencies).

What do you think?


I think we should just remove most of these "depends on" since they're
mostly set up to reflect runtime requirements rather than build time
requirements. The only points I'd make are:


I definitely agree we should do that for all the codec Kconfig options.


1)

Everything should "depends on SND_SOC_TEGRA" simply so the options
don't
show up and clutter menuconfig menus unless SND_SOC_TEGRA is enabled.


Agreed.


2)

SND_SOC_TEGRA30_I2S does need the Tegra30 AHUB driver in order to
compile/link, since it directly calls functions in that driver. This
is
already handled by SND_SOC_TEGRA30_I2S doing "select
SND_SOC_TEGRA30_AHUB".


Agreed.


3)

The machine drivers all do e.g. "select SND_SOC_TEGRA30_I2S if
ARCH_TEGRA_3x_SOC". This was an attempt to make the machine drivers
only
pull in the relevant drivers for the SoC(s) being compiled for. I'm
not
sure this still makes sense; this won't work on kernels that only
support T124/T210/T186 since ARCH_TEGRA_3x_SOC isn't enabled then.
Should we just remove all those and make sure the defconfigs are
updated
to make sure the relevant I2S/AHUB/SPDIF/AC97 drivers are explicitly
enabled? Perhaps we should default all the I2S/AHUB/SPDIF/AC97 to y
(which will only apply if SND_SOC_TEGRA is enabled)?


I think it would be easier for everyone to just auto-select the machine
drivers
automatically based on the architecture (so we could have the list of
ARCH_TEGRA_*_SOC here) when SND_SOC_TEGRA is selected.


I don't think selecting the machine drivers is the correct approach,
since then they can't be disabled.

Making certain machine drivers "default y if ARCH_TEGRA_nn_SOC" would
address that,


That's right, my mistake. Let's take that as the solution I'm backing then.


 but still isn't very scalable since we need to go back and
edit the Kconfig every time we define a new SoC, in order to add that
SoC into the default statement.


Well, that's what platform bringup is all about, isn't it? I think it makes
a
lot more sense to have to add a new platform once (and it's not like one
will
forget to look at the sound part when adding a new platform) rather than
requiring users to hand-pick the option.


Personally I'd expect all the Tegra machine drivers to be enabled in the
upstream defconfig files all the time, such that we don't need to make
the Kconfig options default y, nor have users manually turn the relevant
options on, at all.


I think I was previously mistaken about what the machine drivers are, so I got a
good occasion to find out about them an learn something new. Thanks!

Agreed regarding the machine drivers, they should indeed all be enabled in the
defconfig by default.


Not only does this preserve existing configs (including external ones
that
aren't part of the kernel tree), it also clearly maps which machine
driver
to use for which SoC instead of having users do it by hand.


The machine drivers aren't terribly tied to SoCs by design; most of them
would work on pretty much any SoC. They're only tied to SoCs as a
side-effect of a machine driver being tied to a certain CODEC, and
certain CODECS just by chance are only used (so far) on specific boards,
which have specific SoCs.


I'm a bit confused: aren't the

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-05-07 Thread Paul Kocialkowski

Hi,

Le mardi 25 avril 2017 à 10:57 -0600, Stephen Warren a écrit :
> On 04/24/2017 12:41 PM, Paul Kocialkowski wrote:
> > Le lundi 24 avril 2017 à 09:35 -0600, Stephen Warren a écrit :
> > > On 04/24/2017 09:07 AM, Paul Kocialkowski wrote:
> > > > Le mercredi 19 avril 2017 à 16:00 -0600, Stephen Warren a écrit :
> > > > > On 04/18/2017 10:38 AM, Paul Kocialkowski wrote:
> > > > > > Le mardi 18 avril 2017 à 10:15 -0600, Stephen Warren a écrit :
> > > > > > > On 04/18/2017 09:11 AM, Paul Kocialkowski wrote:
> > > > > > > > This selects the tegra30 i2s and ahub controllers for the
> > > > > > > > tegra124
> > > > > > > > SoC.
> > > > > > > > These are needed when building without ARCH_TEGRA_3x_SOC set.
> > > > > > > > diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
> > > > > > > > index efbe8d4c019e..bcd18d2cf7a7 100644
> > > > > > > > --- a/sound/soc/tegra/Kconfig
> > > > > > > > +++ b/sound/soc/tegra/Kconfig
> > > > > > > > @@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC ||
> > > > > > > > ARCH_TEGRA_124_SOC)
> > > > > > > 
> > > > > > > Is this really a compile-time dependency?
> > > > > > 
> > > > > > From a quick look at the code, I doubt this is really a build
> > > > > > dependency.
> > > > > > 
> > > > > > > If so, don't we need to add T210 and T186 entries into that ||
> > > > > > > condition
> > > > > > > too,
> > > > > > > since we could be building a kernel with just T210/T186 support
> > > > > > > and no
> > > > > > > T124
> > > > > > > support?
> > > > > > 
> > > > > > In the spirit of this patch, adding entries for other tegra
> > > > > > platforms
> > > > > > would
> > > > > > make
> > > > > > sense. Would you prefer that we leave out the dependency from
> > > > > > SND_SOC_TEGRA30_*
> > > > > > and only select the right I2S driver to use in each codec driver?
> > > > > > 
> > > > > > If not, we'd have to list all relevant platforms both in the
> > > > > > I2S/AHUB
> > > > > > drivers
> > > > > > and in each codec's rules (which is not necessarily and issue, but
> > > > > > there's
> > > > > > no
> > > > > > need to have artificial platform dependencies).
> > > > > > 
> > > > > > What do you think?
> > > > > 
> > > > > I think we should just remove most of these "depends on" since they're
> > > > > mostly set up to reflect runtime requirements rather than build time
> > > > > requirements. The only points I'd make are:
> > > > 
> > > > I definitely agree we should do that for all the codec Kconfig options.
> > > > 
> > > > > 1)
> > > > > 
> > > > > Everything should "depends on SND_SOC_TEGRA" simply so the options
> > > > > don't
> > > > > show up and clutter menuconfig menus unless SND_SOC_TEGRA is enabled.
> > > > 
> > > > Agreed.
> > > > 
> > > > > 2)
> > > > > 
> > > > > SND_SOC_TEGRA30_I2S does need the Tegra30 AHUB driver in order to
> > > > > compile/link, since it directly calls functions in that driver. This
> > > > > is
> > > > > already handled by SND_SOC_TEGRA30_I2S doing "select
> > > > > SND_SOC_TEGRA30_AHUB".
> > > > 
> > > > Agreed.
> > > > 
> > > > > 3)
> > > > > 
> > > > > The machine drivers all do e.g. "select SND_SOC_TEGRA30_I2S if
> > > > > ARCH_TEGRA_3x_SOC". This was an attempt to make the machine drivers
> > > > > only
> > > > > pull in the relevant drivers for the SoC(s) being compiled for. I'm
> > > > > not
> > > > > sure this still makes sense; this won't work on kernels that only
> > > > > support T124/T210/T186 since ARCH_TEGRA_3x_SOC isn't enabled then.
> > > > > Should we just remove all those and make sure the defconfigs are
> > > > > updated
> > > > > to make sure the relevant I2S/AHUB/SPDIF/AC97 drivers are explicitly
> > > > > enabled? Perhaps we should default all the I2S/AHUB/SPDIF/AC97 to y
> > > > > (which will only apply if SND_SOC_TEGRA is enabled)?
> > > > 
> > > > I think it would be easier for everyone to just auto-select the machine
> > > > drivers
> > > > automatically based on the architecture (so we could have the list of
> > > > ARCH_TEGRA_*_SOC here) when SND_SOC_TEGRA is selected.
> > > 
> > > I don't think selecting the machine drivers is the correct approach,
> > > since then they can't be disabled.
> > > 
> > > Making certain machine drivers "default y if ARCH_TEGRA_nn_SOC" would
> > > address that,
> > 
> > That's right, my mistake. Let's take that as the solution I'm backing then.
> > 
> > >  but still isn't very scalable since we need to go back and
> > > edit the Kconfig every time we define a new SoC, in order to add that
> > > SoC into the default statement.
> > 
> > Well, that's what platform bringup is all about, isn't it? I think it makes
> > a
> > lot more sense to have to add a new platform once (and it's not like one
> > will
> > forget to look at the sound part when adding a new

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-05-07 Thread Paul Kocialkowski

Hi,

Le mardi 25 avril 2017 à 10:57 -0600, Stephen Warren a écrit :
> On 04/24/2017 12:41 PM, Paul Kocialkowski wrote:
> > Le lundi 24 avril 2017 à 09:35 -0600, Stephen Warren a écrit :
> > > On 04/24/2017 09:07 AM, Paul Kocialkowski wrote:
> > > > Le mercredi 19 avril 2017 à 16:00 -0600, Stephen Warren a écrit :
> > > > > On 04/18/2017 10:38 AM, Paul Kocialkowski wrote:
> > > > > > Le mardi 18 avril 2017 à 10:15 -0600, Stephen Warren a écrit :
> > > > > > > On 04/18/2017 09:11 AM, Paul Kocialkowski wrote:
> > > > > > > > This selects the tegra30 i2s and ahub controllers for the
> > > > > > > > tegra124
> > > > > > > > SoC.
> > > > > > > > These are needed when building without ARCH_TEGRA_3x_SOC set.
> > > > > > > > diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
> > > > > > > > index efbe8d4c019e..bcd18d2cf7a7 100644
> > > > > > > > --- a/sound/soc/tegra/Kconfig
> > > > > > > > +++ b/sound/soc/tegra/Kconfig
> > > > > > > > @@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC ||
> > > > > > > > ARCH_TEGRA_124_SOC)
> > > > > > > 
> > > > > > > Is this really a compile-time dependency?
> > > > > > 
> > > > > > From a quick look at the code, I doubt this is really a build
> > > > > > dependency.
> > > > > > 
> > > > > > > If so, don't we need to add T210 and T186 entries into that ||
> > > > > > > condition
> > > > > > > too,
> > > > > > > since we could be building a kernel with just T210/T186 support
> > > > > > > and no
> > > > > > > T124
> > > > > > > support?
> > > > > > 
> > > > > > In the spirit of this patch, adding entries for other tegra
> > > > > > platforms
> > > > > > would
> > > > > > make
> > > > > > sense. Would you prefer that we leave out the dependency from
> > > > > > SND_SOC_TEGRA30_*
> > > > > > and only select the right I2S driver to use in each codec driver?
> > > > > > 
> > > > > > If not, we'd have to list all relevant platforms both in the
> > > > > > I2S/AHUB
> > > > > > drivers
> > > > > > and in each codec's rules (which is not necessarily and issue, but
> > > > > > there's
> > > > > > no
> > > > > > need to have artificial platform dependencies).
> > > > > > 
> > > > > > What do you think?
> > > > > 
> > > > > I think we should just remove most of these "depends on" since they're
> > > > > mostly set up to reflect runtime requirements rather than build time
> > > > > requirements. The only points I'd make are:
> > > > 
> > > > I definitely agree we should do that for all the codec Kconfig options.
> > > > 
> > > > > 1)
> > > > > 
> > > > > Everything should "depends on SND_SOC_TEGRA" simply so the options
> > > > > don't
> > > > > show up and clutter menuconfig menus unless SND_SOC_TEGRA is enabled.
> > > > 
> > > > Agreed.
> > > > 
> > > > > 2)
> > > > > 
> > > > > SND_SOC_TEGRA30_I2S does need the Tegra30 AHUB driver in order to
> > > > > compile/link, since it directly calls functions in that driver. This
> > > > > is
> > > > > already handled by SND_SOC_TEGRA30_I2S doing "select
> > > > > SND_SOC_TEGRA30_AHUB".
> > > > 
> > > > Agreed.
> > > > 
> > > > > 3)
> > > > > 
> > > > > The machine drivers all do e.g. "select SND_SOC_TEGRA30_I2S if
> > > > > ARCH_TEGRA_3x_SOC". This was an attempt to make the machine drivers
> > > > > only
> > > > > pull in the relevant drivers for the SoC(s) being compiled for. I'm
> > > > > not
> > > > > sure this still makes sense; this won't work on kernels that only
> > > > > support T124/T210/T186 since ARCH_TEGRA_3x_SOC isn't enabled then.
> > > > > Should we just remove all those and make sure the defconfigs are
> > > > > updated
> > > > > to make sure the relevant I2S/AHUB/SPDIF/AC97 drivers are explicitly
> > > > > enabled? Perhaps we should default all the I2S/AHUB/SPDIF/AC97 to y
> > > > > (which will only apply if SND_SOC_TEGRA is enabled)?
> > > > 
> > > > I think it would be easier for everyone to just auto-select the machine
> > > > drivers
> > > > automatically based on the architecture (so we could have the list of
> > > > ARCH_TEGRA_*_SOC here) when SND_SOC_TEGRA is selected.
> > > 
> > > I don't think selecting the machine drivers is the correct approach,
> > > since then they can't be disabled.
> > > 
> > > Making certain machine drivers "default y if ARCH_TEGRA_nn_SOC" would
> > > address that,
> > 
> > That's right, my mistake. Let's take that as the solution I'm backing then.
> > 
> > >  but still isn't very scalable since we need to go back and
> > > edit the Kconfig every time we define a new SoC, in order to add that
> > > SoC into the default statement.
> > 
> > Well, that's what platform bringup is all about, isn't it? I think it makes
> > a
> > lot more sense to have to add a new platform once (and it's not like one
> > will
> > forget to look at the sound part when adding a new

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-04-25 Thread Stephen Warren


On 04/24/2017 12:41 PM, Paul Kocialkowski wrote:

Le lundi 24 avril 2017 à 09:35 -0600, Stephen Warren a écrit :

On 04/24/2017 09:07 AM, Paul Kocialkowski wrote:

Le mercredi 19 avril 2017 à 16:00 -0600, Stephen Warren a écrit :

On 04/18/2017 10:38 AM, Paul Kocialkowski wrote:

Le mardi 18 avril 2017 à 10:15 -0600, Stephen Warren a écrit :

On 04/18/2017 09:11 AM, Paul Kocialkowski wrote:

This selects the tegra30 i2s and ahub controllers for the tegra124
SoC.
These are needed when building without ARCH_TEGRA_3x_SOC set.
diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
index efbe8d4c019e..bcd18d2cf7a7 100644
--- a/sound/soc/tegra/Kconfig
+++ b/sound/soc/tegra/Kconfig
@@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC ||
ARCH_TEGRA_124_SOC)


Is this really a compile-time dependency?


From a quick look at the code, I doubt this is really a build
dependency.


If so, don't we need to add T210 and T186 entries into that ||
condition
too,
since we could be building a kernel with just T210/T186 support and no
T124
support?


In the spirit of this patch, adding entries for other tegra platforms
would
make
sense. Would you prefer that we leave out the dependency from
SND_SOC_TEGRA30_*
and only select the right I2S driver to use in each codec driver?

If not, we'd have to list all relevant platforms both in the I2S/AHUB
drivers
and in each codec's rules (which is not necessarily and issue, but
there's
no
need to have artificial platform dependencies).

What do you think?


I think we should just remove most of these "depends on" since they're
mostly set up to reflect runtime requirements rather than build time
requirements. The only points I'd make are:


I definitely agree we should do that for all the codec Kconfig options.


1)

Everything should "depends on SND_SOC_TEGRA" simply so the options don't
show up and clutter menuconfig menus unless SND_SOC_TEGRA is enabled.


Agreed.


2)

SND_SOC_TEGRA30_I2S does need the Tegra30 AHUB driver in order to
compile/link, since it directly calls functions in that driver. This is
already handled by SND_SOC_TEGRA30_I2S doing "select
SND_SOC_TEGRA30_AHUB".


Agreed.


3)

The machine drivers all do e.g. "select SND_SOC_TEGRA30_I2S if
ARCH_TEGRA_3x_SOC". This was an attempt to make the machine drivers only
pull in the relevant drivers for the SoC(s) being compiled for. I'm not
sure this still makes sense; this won't work on kernels that only
support T124/T210/T186 since ARCH_TEGRA_3x_SOC isn't enabled then.
Should we just remove all those and make sure the defconfigs are updated
to make sure the relevant I2S/AHUB/SPDIF/AC97 drivers are explicitly
enabled? Perhaps we should default all the I2S/AHUB/SPDIF/AC97 to y
(which will only apply if SND_SOC_TEGRA is enabled)?


I think it would be easier for everyone to just auto-select the machine
drivers
automatically based on the architecture (so we could have the list of
ARCH_TEGRA_*_SOC here) when SND_SOC_TEGRA is selected.


I don't think selecting the machine drivers is the correct approach,
since then they can't be disabled.

Making certain machine drivers "default y if ARCH_TEGRA_nn_SOC" would
address that,


That's right, my mistake. Let's take that as the solution I'm backing then.


 but still isn't very scalable since we need to go back and
edit the Kconfig every time we define a new SoC, in order to add that
SoC into the default statement.


Well, that's what platform bringup is all about, isn't it? I think it makes a
lot more sense to have to add a new platform once (and it's not like one will
forget to look at the sound part when adding a new platform) rather than
requiring users to hand-pick the option.


Personally I'd expect all the Tegra machine drivers to be enabled in the 
upstream defconfig files all the time, such that we don't need to make 
the Kconfig options default y, nor have users manually turn the relevant 
options on, at all.



Not only does this preserve existing configs (including external ones that
aren't part of the kernel tree), it also clearly maps which machine driver
to use for which SoC instead of having users do it by hand.


The machine drivers aren't terribly tied to SoCs by design; most of them
would work on pretty much any SoC. They're only tied to SoCs as a
side-effect of a machine driver being tied to a certain CODEC, and
certain CODECS just by chance are only used (so far) on specific boards,
which have specific SoCs.


I'm a bit confused: aren't the machine driver (i2s/ahub/spdif/ac97) tied to
specific hardware blocks that are found in specific SoCs and not in others? I
can see these blocks haven't evolved much across generations, but they're still
either part of a specific SoC or not, aren't they?

The compatible strings in the common SoC dts seem to indicate that only one of
these blocks is found at a time.


There

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-04-25 Thread Stephen Warren


On 04/24/2017 12:41 PM, Paul Kocialkowski wrote:

Le lundi 24 avril 2017 à 09:35 -0600, Stephen Warren a écrit :

On 04/24/2017 09:07 AM, Paul Kocialkowski wrote:

Le mercredi 19 avril 2017 à 16:00 -0600, Stephen Warren a écrit :

On 04/18/2017 10:38 AM, Paul Kocialkowski wrote:

Le mardi 18 avril 2017 à 10:15 -0600, Stephen Warren a écrit :

On 04/18/2017 09:11 AM, Paul Kocialkowski wrote:

This selects the tegra30 i2s and ahub controllers for the tegra124
SoC.
These are needed when building without ARCH_TEGRA_3x_SOC set.
diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
index efbe8d4c019e..bcd18d2cf7a7 100644
--- a/sound/soc/tegra/Kconfig
+++ b/sound/soc/tegra/Kconfig
@@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC ||
ARCH_TEGRA_124_SOC)


Is this really a compile-time dependency?


From a quick look at the code, I doubt this is really a build
dependency.


If so, don't we need to add T210 and T186 entries into that ||
condition
too,
since we could be building a kernel with just T210/T186 support and no
T124
support?


In the spirit of this patch, adding entries for other tegra platforms
would
make
sense. Would you prefer that we leave out the dependency from
SND_SOC_TEGRA30_*
and only select the right I2S driver to use in each codec driver?

If not, we'd have to list all relevant platforms both in the I2S/AHUB
drivers
and in each codec's rules (which is not necessarily and issue, but
there's
no
need to have artificial platform dependencies).

What do you think?


I think we should just remove most of these "depends on" since they're
mostly set up to reflect runtime requirements rather than build time
requirements. The only points I'd make are:


I definitely agree we should do that for all the codec Kconfig options.


1)

Everything should "depends on SND_SOC_TEGRA" simply so the options don't
show up and clutter menuconfig menus unless SND_SOC_TEGRA is enabled.


Agreed.


2)

SND_SOC_TEGRA30_I2S does need the Tegra30 AHUB driver in order to
compile/link, since it directly calls functions in that driver. This is
already handled by SND_SOC_TEGRA30_I2S doing "select
SND_SOC_TEGRA30_AHUB".


Agreed.


3)

The machine drivers all do e.g. "select SND_SOC_TEGRA30_I2S if
ARCH_TEGRA_3x_SOC". This was an attempt to make the machine drivers only
pull in the relevant drivers for the SoC(s) being compiled for. I'm not
sure this still makes sense; this won't work on kernels that only
support T124/T210/T186 since ARCH_TEGRA_3x_SOC isn't enabled then.
Should we just remove all those and make sure the defconfigs are updated
to make sure the relevant I2S/AHUB/SPDIF/AC97 drivers are explicitly
enabled? Perhaps we should default all the I2S/AHUB/SPDIF/AC97 to y
(which will only apply if SND_SOC_TEGRA is enabled)?


I think it would be easier for everyone to just auto-select the machine
drivers
automatically based on the architecture (so we could have the list of
ARCH_TEGRA_*_SOC here) when SND_SOC_TEGRA is selected.


I don't think selecting the machine drivers is the correct approach,
since then they can't be disabled.

Making certain machine drivers "default y if ARCH_TEGRA_nn_SOC" would
address that,


That's right, my mistake. Let's take that as the solution I'm backing then.


 but still isn't very scalable since we need to go back and
edit the Kconfig every time we define a new SoC, in order to add that
SoC into the default statement.


Well, that's what platform bringup is all about, isn't it? I think it makes a
lot more sense to have to add a new platform once (and it's not like one will
forget to look at the sound part when adding a new platform) rather than
requiring users to hand-pick the option.


Personally I'd expect all the Tegra machine drivers to be enabled in the 
upstream defconfig files all the time, such that we don't need to make 
the Kconfig options default y, nor have users manually turn the relevant 
options on, at all.



Not only does this preserve existing configs (including external ones that
aren't part of the kernel tree), it also clearly maps which machine driver
to use for which SoC instead of having users do it by hand.


The machine drivers aren't terribly tied to SoCs by design; most of them
would work on pretty much any SoC. They're only tied to SoCs as a
side-effect of a machine driver being tied to a certain CODEC, and
certain CODECS just by chance are only used (so far) on specific boards,
which have specific SoCs.


I'm a bit confused: aren't the machine driver (i2s/ahub/spdif/ac97) tied to
specific hardware blocks that are found in specific SoCs and not in others? I
can see these blocks haven't evolved much across generations, but they're still
either part of a specific SoC or not, aren't they?

The compatible strings in the common SoC dts seem to indicate that only one of
these blocks is found at a time.


There

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-04-24 Thread Paul Kocialkowski

Le lundi 24 avril 2017 à 09:35 -0600, Stephen Warren a écrit :
> On 04/24/2017 09:07 AM, Paul Kocialkowski wrote:
> > Le mercredi 19 avril 2017 à 16:00 -0600, Stephen Warren a écrit :
> > > On 04/18/2017 10:38 AM, Paul Kocialkowski wrote:
> > > > Le mardi 18 avril 2017 à 10:15 -0600, Stephen Warren a écrit :
> > > > > On 04/18/2017 09:11 AM, Paul Kocialkowski wrote:
> > > > > > This selects the tegra30 i2s and ahub controllers for the tegra124
> > > > > > SoC.
> > > > > > These are needed when building without ARCH_TEGRA_3x_SOC set.
> > > > > > diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
> > > > > > index efbe8d4c019e..bcd18d2cf7a7 100644
> > > > > > --- a/sound/soc/tegra/Kconfig
> > > > > > +++ b/sound/soc/tegra/Kconfig
> > > > > > @@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC ||
> > > > > > ARCH_TEGRA_124_SOC)
> > > > > 
> > > > > Is this really a compile-time dependency?
> > > > 
> > > > From a quick look at the code, I doubt this is really a build
> > > > dependency.
> > > > 
> > > > > If so, don't we need to add T210 and T186 entries into that ||
> > > > > condition
> > > > > too,
> > > > > since we could be building a kernel with just T210/T186 support and no
> > > > > T124
> > > > > support?
> > > > 
> > > > In the spirit of this patch, adding entries for other tegra platforms
> > > > would
> > > > make
> > > > sense. Would you prefer that we leave out the dependency from
> > > > SND_SOC_TEGRA30_*
> > > > and only select the right I2S driver to use in each codec driver?
> > > > 
> > > > If not, we'd have to list all relevant platforms both in the I2S/AHUB
> > > > drivers
> > > > and in each codec's rules (which is not necessarily and issue, but
> > > > there's
> > > > no
> > > > need to have artificial platform dependencies).
> > > > 
> > > > What do you think?
> > > 
> > > I think we should just remove most of these "depends on" since they're
> > > mostly set up to reflect runtime requirements rather than build time
> > > requirements. The only points I'd make are:
> > 
> > I definitely agree we should do that for all the codec Kconfig options.
> > 
> > > 1)
> > > 
> > > Everything should "depends on SND_SOC_TEGRA" simply so the options don't
> > > show up and clutter menuconfig menus unless SND_SOC_TEGRA is enabled.
> > 
> > Agreed.
> > 
> > > 2)
> > > 
> > > SND_SOC_TEGRA30_I2S does need the Tegra30 AHUB driver in order to
> > > compile/link, since it directly calls functions in that driver. This is
> > > already handled by SND_SOC_TEGRA30_I2S doing "select
> > > SND_SOC_TEGRA30_AHUB".
> > 
> > Agreed.
> > 
> > > 3)
> > > 
> > > The machine drivers all do e.g. "select SND_SOC_TEGRA30_I2S if
> > > ARCH_TEGRA_3x_SOC". This was an attempt to make the machine drivers only
> > > pull in the relevant drivers for the SoC(s) being compiled for. I'm not
> > > sure this still makes sense; this won't work on kernels that only
> > > support T124/T210/T186 since ARCH_TEGRA_3x_SOC isn't enabled then.
> > > Should we just remove all those and make sure the defconfigs are updated
> > > to make sure the relevant I2S/AHUB/SPDIF/AC97 drivers are explicitly
> > > enabled? Perhaps we should default all the I2S/AHUB/SPDIF/AC97 to y
> > > (which will only apply if SND_SOC_TEGRA is enabled)?
> > 
> > I think it would be easier for everyone to just auto-select the machine
> > drivers
> > automatically based on the architecture (so we could have the list of
> > ARCH_TEGRA_*_SOC here) when SND_SOC_TEGRA is selected.
> 
> I don't think selecting the machine drivers is the correct approach, 
> since then they can't be disabled.
>
> Making certain machine drivers "default y if ARCH_TEGRA_nn_SOC" would 
> address that,

That's right, my mistake. Let's take that as the solution I'm backing then.

>  but still isn't very scalable since we need to go back and 
> edit the Kconfig every time we define a new SoC, in order to add that 
> SoC into the default statement.

Well, that's what platform bringup is all about, isn't it? I think it makes a
lot more sense to have to add a new platform once (and it's not like one will
forget to look at the sound part when adding a new platform) rather than
requiring users to hand-pick the option.

> > Not only does this preserve existing configs (including external ones that
> > aren't part of the kernel tree), it also clearly maps which machine driver
> > to use for which SoC instead of having users do it by hand.
> 
> The machine drivers aren't terribly tied to SoCs by design; most of them 
> would work on pretty much any SoC. They're only tied to SoCs as a 
> side-effect of a machine driver being tied to a certain CODEC, and 
> certain CODECS just by chance are only used (so far) on specific boards, 
> which have specific SoCs.

I'm a bit confused: aren't the machine

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-04-24 Thread Paul Kocialkowski

Le lundi 24 avril 2017 à 09:35 -0600, Stephen Warren a écrit :
> On 04/24/2017 09:07 AM, Paul Kocialkowski wrote:
> > Le mercredi 19 avril 2017 à 16:00 -0600, Stephen Warren a écrit :
> > > On 04/18/2017 10:38 AM, Paul Kocialkowski wrote:
> > > > Le mardi 18 avril 2017 à 10:15 -0600, Stephen Warren a écrit :
> > > > > On 04/18/2017 09:11 AM, Paul Kocialkowski wrote:
> > > > > > This selects the tegra30 i2s and ahub controllers for the tegra124
> > > > > > SoC.
> > > > > > These are needed when building without ARCH_TEGRA_3x_SOC set.
> > > > > > diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
> > > > > > index efbe8d4c019e..bcd18d2cf7a7 100644
> > > > > > --- a/sound/soc/tegra/Kconfig
> > > > > > +++ b/sound/soc/tegra/Kconfig
> > > > > > @@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC ||
> > > > > > ARCH_TEGRA_124_SOC)
> > > > > 
> > > > > Is this really a compile-time dependency?
> > > > 
> > > > From a quick look at the code, I doubt this is really a build
> > > > dependency.
> > > > 
> > > > > If so, don't we need to add T210 and T186 entries into that ||
> > > > > condition
> > > > > too,
> > > > > since we could be building a kernel with just T210/T186 support and no
> > > > > T124
> > > > > support?
> > > > 
> > > > In the spirit of this patch, adding entries for other tegra platforms
> > > > would
> > > > make
> > > > sense. Would you prefer that we leave out the dependency from
> > > > SND_SOC_TEGRA30_*
> > > > and only select the right I2S driver to use in each codec driver?
> > > > 
> > > > If not, we'd have to list all relevant platforms both in the I2S/AHUB
> > > > drivers
> > > > and in each codec's rules (which is not necessarily and issue, but
> > > > there's
> > > > no
> > > > need to have artificial platform dependencies).
> > > > 
> > > > What do you think?
> > > 
> > > I think we should just remove most of these "depends on" since they're
> > > mostly set up to reflect runtime requirements rather than build time
> > > requirements. The only points I'd make are:
> > 
> > I definitely agree we should do that for all the codec Kconfig options.
> > 
> > > 1)
> > > 
> > > Everything should "depends on SND_SOC_TEGRA" simply so the options don't
> > > show up and clutter menuconfig menus unless SND_SOC_TEGRA is enabled.
> > 
> > Agreed.
> > 
> > > 2)
> > > 
> > > SND_SOC_TEGRA30_I2S does need the Tegra30 AHUB driver in order to
> > > compile/link, since it directly calls functions in that driver. This is
> > > already handled by SND_SOC_TEGRA30_I2S doing "select
> > > SND_SOC_TEGRA30_AHUB".
> > 
> > Agreed.
> > 
> > > 3)
> > > 
> > > The machine drivers all do e.g. "select SND_SOC_TEGRA30_I2S if
> > > ARCH_TEGRA_3x_SOC". This was an attempt to make the machine drivers only
> > > pull in the relevant drivers for the SoC(s) being compiled for. I'm not
> > > sure this still makes sense; this won't work on kernels that only
> > > support T124/T210/T186 since ARCH_TEGRA_3x_SOC isn't enabled then.
> > > Should we just remove all those and make sure the defconfigs are updated
> > > to make sure the relevant I2S/AHUB/SPDIF/AC97 drivers are explicitly
> > > enabled? Perhaps we should default all the I2S/AHUB/SPDIF/AC97 to y
> > > (which will only apply if SND_SOC_TEGRA is enabled)?
> > 
> > I think it would be easier for everyone to just auto-select the machine
> > drivers
> > automatically based on the architecture (so we could have the list of
> > ARCH_TEGRA_*_SOC here) when SND_SOC_TEGRA is selected.
> 
> I don't think selecting the machine drivers is the correct approach, 
> since then they can't be disabled.
>
> Making certain machine drivers "default y if ARCH_TEGRA_nn_SOC" would 
> address that,

That's right, my mistake. Let's take that as the solution I'm backing then.

>  but still isn't very scalable since we need to go back and 
> edit the Kconfig every time we define a new SoC, in order to add that 
> SoC into the default statement.

Well, that's what platform bringup is all about, isn't it? I think it makes a
lot more sense to have to add a new platform once (and it's not like one will
forget to look at the sound part when adding a new platform) rather than
requiring users to hand-pick the option.

> > Not only does this preserve existing configs (including external ones that
> > aren't part of the kernel tree), it also clearly maps which machine driver
> > to use for which SoC instead of having users do it by hand.
> 
> The machine drivers aren't terribly tied to SoCs by design; most of them 
> would work on pretty much any SoC. They're only tied to SoCs as a 
> side-effect of a machine driver being tied to a certain CODEC, and 
> certain CODECS just by chance are only used (so far) on specific boards, 
> which have specific SoCs.

I'm a bit confused: aren't the machine

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-04-24 Thread Stephen Warren


On 04/24/2017 09:07 AM, Paul Kocialkowski wrote:

Le mercredi 19 avril 2017 à 16:00 -0600, Stephen Warren a écrit :

On 04/18/2017 10:38 AM, Paul Kocialkowski wrote:

Le mardi 18 avril 2017 à 10:15 -0600, Stephen Warren a écrit :

On 04/18/2017 09:11 AM, Paul Kocialkowski wrote:

This selects the tegra30 i2s and ahub controllers for the tegra124 SoC.
These are needed when building without ARCH_TEGRA_3x_SOC set.
diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
index efbe8d4c019e..bcd18d2cf7a7 100644
--- a/sound/soc/tegra/Kconfig
+++ b/sound/soc/tegra/Kconfig
@@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC ||
ARCH_TEGRA_124_SOC)


Is this really a compile-time dependency?


From a quick look at the code, I doubt this is really a build dependency.


If so, don't we need to add T210 and T186 entries into that || condition
too,
since we could be building a kernel with just T210/T186 support and no
T124
support?


In the spirit of this patch, adding entries for other tegra platforms would
make
sense. Would you prefer that we leave out the dependency from
SND_SOC_TEGRA30_*
and only select the right I2S driver to use in each codec driver?

If not, we'd have to list all relevant platforms both in the I2S/AHUB
drivers
and in each codec's rules (which is not necessarily and issue, but there's
no
need to have artificial platform dependencies).

What do you think?


I think we should just remove most of these "depends on" since they're
mostly set up to reflect runtime requirements rather than build time
requirements. The only points I'd make are:


I definitely agree we should do that for all the codec Kconfig options.


1)

Everything should "depends on SND_SOC_TEGRA" simply so the options don't
show up and clutter menuconfig menus unless SND_SOC_TEGRA is enabled.


Agreed.


2)

SND_SOC_TEGRA30_I2S does need the Tegra30 AHUB driver in order to
compile/link, since it directly calls functions in that driver. This is
already handled by SND_SOC_TEGRA30_I2S doing "select SND_SOC_TEGRA30_AHUB".


Agreed.


3)

The machine drivers all do e.g. "select SND_SOC_TEGRA30_I2S if
ARCH_TEGRA_3x_SOC". This was an attempt to make the machine drivers only
pull in the relevant drivers for the SoC(s) being compiled for. I'm not
sure this still makes sense; this won't work on kernels that only
support T124/T210/T186 since ARCH_TEGRA_3x_SOC isn't enabled then.
Should we just remove all those and make sure the defconfigs are updated
to make sure the relevant I2S/AHUB/SPDIF/AC97 drivers are explicitly
enabled? Perhaps we should default all the I2S/AHUB/SPDIF/AC97 to y
(which will only apply if SND_SOC_TEGRA is enabled)?


I think it would be easier for everyone to just auto-select the machine drivers
automatically based on the architecture (so we could have the list of
ARCH_TEGRA_*_SOC here) when SND_SOC_TEGRA is selected.


I don't think selecting the machine drivers is the correct approach, 
since then they can't be disabled.


Making certain machine drivers "default y if ARCH_TEGRA_nn_SOC" would 
address that, but still isn't very scalable since we need to go back and 
edit the Kconfig every time we define a new SoC, in order to add that 
SoC into the default statement.



Not only does this preserve existing configs (including external ones that
aren't part of the kernel tree), it also clearly maps which machine driver to
use for which SoC instead of having users do it by hand.


The machine drivers aren't terribly tied to SoCs by design; most of them 
would work on pretty much any SoC. They're only tied to SoCs as a 
side-effect of a machine driver being tied to a certain CODEC, and 
certain CODECS just by chance are only used (so far) on specific boards, 
which have specific SoCs.



I'm also opposed to auto-selecting them all, because I don't really like the
idea of auto-including things that might not be needed.

Would that be agreeable?

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-04-24 Thread Stephen Warren


On 04/24/2017 09:07 AM, Paul Kocialkowski wrote:

Le mercredi 19 avril 2017 à 16:00 -0600, Stephen Warren a écrit :

On 04/18/2017 10:38 AM, Paul Kocialkowski wrote:

Le mardi 18 avril 2017 à 10:15 -0600, Stephen Warren a écrit :

On 04/18/2017 09:11 AM, Paul Kocialkowski wrote:

This selects the tegra30 i2s and ahub controllers for the tegra124 SoC.
These are needed when building without ARCH_TEGRA_3x_SOC set.
diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
index efbe8d4c019e..bcd18d2cf7a7 100644
--- a/sound/soc/tegra/Kconfig
+++ b/sound/soc/tegra/Kconfig
@@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC ||
ARCH_TEGRA_124_SOC)


Is this really a compile-time dependency?


From a quick look at the code, I doubt this is really a build dependency.


If so, don't we need to add T210 and T186 entries into that || condition
too,
since we could be building a kernel with just T210/T186 support and no
T124
support?


In the spirit of this patch, adding entries for other tegra platforms would
make
sense. Would you prefer that we leave out the dependency from
SND_SOC_TEGRA30_*
and only select the right I2S driver to use in each codec driver?

If not, we'd have to list all relevant platforms both in the I2S/AHUB
drivers
and in each codec's rules (which is not necessarily and issue, but there's
no
need to have artificial platform dependencies).

What do you think?


I think we should just remove most of these "depends on" since they're
mostly set up to reflect runtime requirements rather than build time
requirements. The only points I'd make are:


I definitely agree we should do that for all the codec Kconfig options.


1)

Everything should "depends on SND_SOC_TEGRA" simply so the options don't
show up and clutter menuconfig menus unless SND_SOC_TEGRA is enabled.


Agreed.


2)

SND_SOC_TEGRA30_I2S does need the Tegra30 AHUB driver in order to
compile/link, since it directly calls functions in that driver. This is
already handled by SND_SOC_TEGRA30_I2S doing "select SND_SOC_TEGRA30_AHUB".


Agreed.


3)

The machine drivers all do e.g. "select SND_SOC_TEGRA30_I2S if
ARCH_TEGRA_3x_SOC". This was an attempt to make the machine drivers only
pull in the relevant drivers for the SoC(s) being compiled for. I'm not
sure this still makes sense; this won't work on kernels that only
support T124/T210/T186 since ARCH_TEGRA_3x_SOC isn't enabled then.
Should we just remove all those and make sure the defconfigs are updated
to make sure the relevant I2S/AHUB/SPDIF/AC97 drivers are explicitly
enabled? Perhaps we should default all the I2S/AHUB/SPDIF/AC97 to y
(which will only apply if SND_SOC_TEGRA is enabled)?


I think it would be easier for everyone to just auto-select the machine drivers
automatically based on the architecture (so we could have the list of
ARCH_TEGRA_*_SOC here) when SND_SOC_TEGRA is selected.


I don't think selecting the machine drivers is the correct approach, 
since then they can't be disabled.


Making certain machine drivers "default y if ARCH_TEGRA_nn_SOC" would 
address that, but still isn't very scalable since we need to go back and 
edit the Kconfig every time we define a new SoC, in order to add that 
SoC into the default statement.



Not only does this preserve existing configs (including external ones that
aren't part of the kernel tree), it also clearly maps which machine driver to
use for which SoC instead of having users do it by hand.


The machine drivers aren't terribly tied to SoCs by design; most of them 
would work on pretty much any SoC. They're only tied to SoCs as a 
side-effect of a machine driver being tied to a certain CODEC, and 
certain CODECS just by chance are only used (so far) on specific boards, 
which have specific SoCs.



I'm also opposed to auto-selecting them all, because I don't really like the
idea of auto-including things that might not be needed.

Would that be agreeable?

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-04-24 Thread Paul Kocialkowski

Le mercredi 19 avril 2017 à 16:00 -0600, Stephen Warren a écrit :
> On 04/18/2017 10:38 AM, Paul Kocialkowski wrote:
> > Le mardi 18 avril 2017 à 10:15 -0600, Stephen Warren a écrit :
> > > On 04/18/2017 09:11 AM, Paul Kocialkowski wrote:
> > > > This selects the tegra30 i2s and ahub controllers for the tegra124 SoC.
> > > > These are needed when building without ARCH_TEGRA_3x_SOC set.
> > > > diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
> > > > index efbe8d4c019e..bcd18d2cf7a7 100644
> > > > --- a/sound/soc/tegra/Kconfig
> > > > +++ b/sound/soc/tegra/Kconfig
> > > > @@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC ||
> > > > ARCH_TEGRA_124_SOC)
> > > 
> > > Is this really a compile-time dependency?
> > 
> > From a quick look at the code, I doubt this is really a build dependency.
> > 
> > > If so, don't we need to add T210 and T186 entries into that || condition
> > > too,
> > > since we could be building a kernel with just T210/T186 support and no
> > > T124
> > > support?
> > 
> > In the spirit of this patch, adding entries for other tegra platforms would
> > make
> > sense. Would you prefer that we leave out the dependency from
> > SND_SOC_TEGRA30_*
> > and only select the right I2S driver to use in each codec driver?
> > 
> > If not, we'd have to list all relevant platforms both in the I2S/AHUB
> > drivers
> > and in each codec's rules (which is not necessarily and issue, but there's
> > no
> > need to have artificial platform dependencies).
> > 
> > What do you think?
> 
> I think we should just remove most of these "depends on" since they're 
> mostly set up to reflect runtime requirements rather than build time 
> requirements. The only points I'd make are:

I definitely agree we should do that for all the codec Kconfig options.

> 1)
> 
> Everything should "depends on SND_SOC_TEGRA" simply so the options don't 
> show up and clutter menuconfig menus unless SND_SOC_TEGRA is enabled.

Agreed.

> 2)
> 
> SND_SOC_TEGRA30_I2S does need the Tegra30 AHUB driver in order to 
> compile/link, since it directly calls functions in that driver. This is 
> already handled by SND_SOC_TEGRA30_I2S doing "select SND_SOC_TEGRA30_AHUB".

Agreed.

> 3)
> 
> The machine drivers all do e.g. "select SND_SOC_TEGRA30_I2S if 
> ARCH_TEGRA_3x_SOC". This was an attempt to make the machine drivers only 
> pull in the relevant drivers for the SoC(s) being compiled for. I'm not 
> sure this still makes sense; this won't work on kernels that only 
> support T124/T210/T186 since ARCH_TEGRA_3x_SOC isn't enabled then. 
> Should we just remove all those and make sure the defconfigs are updated 
> to make sure the relevant I2S/AHUB/SPDIF/AC97 drivers are explicitly 
> enabled? Perhaps we should default all the I2S/AHUB/SPDIF/AC97 to y 
> (which will only apply if SND_SOC_TEGRA is enabled)?

I think it would be easier for everyone to just auto-select the machine drivers
automatically based on the architecture (so we could have the list of
ARCH_TEGRA_*_SOC here) when SND_SOC_TEGRA is selected.

Not only does this preserve existing configs (including external ones that
aren't part of the kernel tree), it also clearly maps which machine driver to
use for which SoC instead of having users do it by hand.

I'm also opposed to auto-selecting them all, because I don't really like the
idea of auto-including things that might not be needed.

Would that be agreeable?

-- 
Paul Kocialkowski, developer of free digital technology and hardware support

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

signature.asc
Description: This is a digitally signed message part

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-04-24 Thread Paul Kocialkowski

Le mercredi 19 avril 2017 à 16:00 -0600, Stephen Warren a écrit :
> On 04/18/2017 10:38 AM, Paul Kocialkowski wrote:
> > Le mardi 18 avril 2017 à 10:15 -0600, Stephen Warren a écrit :
> > > On 04/18/2017 09:11 AM, Paul Kocialkowski wrote:
> > > > This selects the tegra30 i2s and ahub controllers for the tegra124 SoC.
> > > > These are needed when building without ARCH_TEGRA_3x_SOC set.
> > > > diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
> > > > index efbe8d4c019e..bcd18d2cf7a7 100644
> > > > --- a/sound/soc/tegra/Kconfig
> > > > +++ b/sound/soc/tegra/Kconfig
> > > > @@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC ||
> > > > ARCH_TEGRA_124_SOC)
> > > 
> > > Is this really a compile-time dependency?
> > 
> > From a quick look at the code, I doubt this is really a build dependency.
> > 
> > > If so, don't we need to add T210 and T186 entries into that || condition
> > > too,
> > > since we could be building a kernel with just T210/T186 support and no
> > > T124
> > > support?
> > 
> > In the spirit of this patch, adding entries for other tegra platforms would
> > make
> > sense. Would you prefer that we leave out the dependency from
> > SND_SOC_TEGRA30_*
> > and only select the right I2S driver to use in each codec driver?
> > 
> > If not, we'd have to list all relevant platforms both in the I2S/AHUB
> > drivers
> > and in each codec's rules (which is not necessarily and issue, but there's
> > no
> > need to have artificial platform dependencies).
> > 
> > What do you think?
> 
> I think we should just remove most of these "depends on" since they're 
> mostly set up to reflect runtime requirements rather than build time 
> requirements. The only points I'd make are:

I definitely agree we should do that for all the codec Kconfig options.

> 1)
> 
> Everything should "depends on SND_SOC_TEGRA" simply so the options don't 
> show up and clutter menuconfig menus unless SND_SOC_TEGRA is enabled.

Agreed.

> 2)
> 
> SND_SOC_TEGRA30_I2S does need the Tegra30 AHUB driver in order to 
> compile/link, since it directly calls functions in that driver. This is 
> already handled by SND_SOC_TEGRA30_I2S doing "select SND_SOC_TEGRA30_AHUB".

Agreed.

> 3)
> 
> The machine drivers all do e.g. "select SND_SOC_TEGRA30_I2S if 
> ARCH_TEGRA_3x_SOC". This was an attempt to make the machine drivers only 
> pull in the relevant drivers for the SoC(s) being compiled for. I'm not 
> sure this still makes sense; this won't work on kernels that only 
> support T124/T210/T186 since ARCH_TEGRA_3x_SOC isn't enabled then. 
> Should we just remove all those and make sure the defconfigs are updated 
> to make sure the relevant I2S/AHUB/SPDIF/AC97 drivers are explicitly 
> enabled? Perhaps we should default all the I2S/AHUB/SPDIF/AC97 to y 
> (which will only apply if SND_SOC_TEGRA is enabled)?

I think it would be easier for everyone to just auto-select the machine drivers
automatically based on the architecture (so we could have the list of
ARCH_TEGRA_*_SOC here) when SND_SOC_TEGRA is selected.

Not only does this preserve existing configs (including external ones that
aren't part of the kernel tree), it also clearly maps which machine driver to
use for which SoC instead of having users do it by hand.

I'm also opposed to auto-selecting them all, because I don't really like the
idea of auto-including things that might not be needed.

Would that be agreeable?

-- 
Paul Kocialkowski, developer of free digital technology and hardware support

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

signature.asc
Description: This is a digitally signed message part

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-04-19 Thread Stephen Warren


On 04/18/2017 10:38 AM, Paul Kocialkowski wrote:

Le mardi 18 avril 2017 à 10:15 -0600, Stephen Warren a écrit :

On 04/18/2017 09:11 AM, Paul Kocialkowski wrote:

This selects the tegra30 i2s and ahub controllers for the tegra124 SoC.
These are needed when building without ARCH_TEGRA_3x_SOC set.
diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
index efbe8d4c019e..bcd18d2cf7a7 100644
--- a/sound/soc/tegra/Kconfig
+++ b/sound/soc/tegra/Kconfig
@@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC ||
ARCH_TEGRA_124_SOC)


Is this really a compile-time dependency?


From a quick look at the code, I doubt this is really a build dependency.


If so, don't we need to add T210 and T186 entries into that || condition too,
since we could be building a kernel with just T210/T186 support and no T124
support?


In the spirit of this patch, adding entries for other tegra platforms would make
sense. Would you prefer that we leave out the dependency from SND_SOC_TEGRA30_*
and only select the right I2S driver to use in each codec driver?

If not, we'd have to list all relevant platforms both in the I2S/AHUB drivers
and in each codec's rules (which is not necessarily and issue, but there's no
need to have artificial platform dependencies).

What do you think?


I think we should just remove most of these "depends on" since they're 
mostly set up to reflect runtime requirements rather than build time 
requirements. The only points I'd make are:


1)

Everything should "depends on SND_SOC_TEGRA" simply so the options don't 
show up and clutter menuconfig menus unless SND_SOC_TEGRA is enabled.


2)

SND_SOC_TEGRA30_I2S does need the Tegra30 AHUB driver in order to 
compile/link, since it directly calls functions in that driver. This is 
already handled by SND_SOC_TEGRA30_I2S doing "select SND_SOC_TEGRA30_AHUB".


3)

The machine drivers all do e.g. "select SND_SOC_TEGRA30_I2S if 
ARCH_TEGRA_3x_SOC". This was an attempt to make the machine drivers only 
pull in the relevant drivers for the SoC(s) being compiled for. I'm not 
sure this still makes sense; this won't work on kernels that only 
support T124/T210/T186 since ARCH_TEGRA_3x_SOC isn't enabled then. 
Should we just remove all those and make sure the defconfigs are updated 
to make sure the relevant I2S/AHUB/SPDIF/AC97 drivers are explicitly 
enabled? Perhaps we should default all the I2S/AHUB/SPDIF/AC97 to y 
(which will only apply if SND_SOC_TEGRA is enabled)?

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-04-19 Thread Stephen Warren


On 04/18/2017 10:38 AM, Paul Kocialkowski wrote:

Le mardi 18 avril 2017 à 10:15 -0600, Stephen Warren a écrit :

On 04/18/2017 09:11 AM, Paul Kocialkowski wrote:

This selects the tegra30 i2s and ahub controllers for the tegra124 SoC.
These are needed when building without ARCH_TEGRA_3x_SOC set.
diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
index efbe8d4c019e..bcd18d2cf7a7 100644
--- a/sound/soc/tegra/Kconfig
+++ b/sound/soc/tegra/Kconfig
@@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC ||
ARCH_TEGRA_124_SOC)


Is this really a compile-time dependency?


From a quick look at the code, I doubt this is really a build dependency.


If so, don't we need to add T210 and T186 entries into that || condition too,
since we could be building a kernel with just T210/T186 support and no T124
support?


In the spirit of this patch, adding entries for other tegra platforms would make
sense. Would you prefer that we leave out the dependency from SND_SOC_TEGRA30_*
and only select the right I2S driver to use in each codec driver?

If not, we'd have to list all relevant platforms both in the I2S/AHUB drivers
and in each codec's rules (which is not necessarily and issue, but there's no
need to have artificial platform dependencies).

What do you think?


I think we should just remove most of these "depends on" since they're 
mostly set up to reflect runtime requirements rather than build time 
requirements. The only points I'd make are:


1)

Everything should "depends on SND_SOC_TEGRA" simply so the options don't 
show up and clutter menuconfig menus unless SND_SOC_TEGRA is enabled.


2)

SND_SOC_TEGRA30_I2S does need the Tegra30 AHUB driver in order to 
compile/link, since it directly calls functions in that driver. This is 
already handled by SND_SOC_TEGRA30_I2S doing "select SND_SOC_TEGRA30_AHUB".


3)

The machine drivers all do e.g. "select SND_SOC_TEGRA30_I2S if 
ARCH_TEGRA_3x_SOC". This was an attempt to make the machine drivers only 
pull in the relevant drivers for the SoC(s) being compiled for. I'm not 
sure this still makes sense; this won't work on kernels that only 
support T124/T210/T186 since ARCH_TEGRA_3x_SOC isn't enabled then. 
Should we just remove all those and make sure the defconfigs are updated 
to make sure the relevant I2S/AHUB/SPDIF/AC97 drivers are explicitly 
enabled? Perhaps we should default all the I2S/AHUB/SPDIF/AC97 to y 
(which will only apply if SND_SOC_TEGRA is enabled)?

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-04-18 Thread Paul Kocialkowski

Le mardi 18 avril 2017 à 10:15 -0600, Stephen Warren a écrit :
> On 04/18/2017 09:11 AM, Paul Kocialkowski wrote:
> > This selects the tegra30 i2s and ahub controllers for the tegra124 SoC.
> > These are needed when building without ARCH_TEGRA_3x_SOC set.
> > diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
> > index efbe8d4c019e..bcd18d2cf7a7 100644
> > --- a/sound/soc/tegra/Kconfig
> > +++ b/sound/soc/tegra/Kconfig
> > @@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC ||
> > ARCH_TEGRA_124_SOC)
> 
> Is this really a compile-time dependency?

From a quick look at the code, I doubt this is really a build dependency.

> If so, don't we need to add T210 and T186 entries into that || condition too,
> since we could be building a kernel with just T210/T186 support and no T124
> support?

In the spirit of this patch, adding entries for other tegra platforms would make
sense. Would you prefer that we leave out the dependency from SND_SOC_TEGRA30_*
and only select the right I2S driver to use in each codec driver?

If not, we'd have to list all relevant platforms both in the I2S/AHUB drivers
and in each codec's rules (which is not necessarily and issue, but there's no
need to have artificial platform dependencies).

What do you think?

-- 
Paul Kocialkowski, developer of free digital technology at the lower levels

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

signature.asc
Description: This is a digitally signed message part

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-04-18 Thread Paul Kocialkowski

Le mardi 18 avril 2017 à 10:15 -0600, Stephen Warren a écrit :
> On 04/18/2017 09:11 AM, Paul Kocialkowski wrote:
> > This selects the tegra30 i2s and ahub controllers for the tegra124 SoC.
> > These are needed when building without ARCH_TEGRA_3x_SOC set.
> > diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
> > index efbe8d4c019e..bcd18d2cf7a7 100644
> > --- a/sound/soc/tegra/Kconfig
> > +++ b/sound/soc/tegra/Kconfig
> > @@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC ||
> > ARCH_TEGRA_124_SOC)
> 
> Is this really a compile-time dependency?

From a quick look at the code, I doubt this is really a build dependency.

> If so, don't we need to add T210 and T186 entries into that || condition too,
> since we could be building a kernel with just T210/T186 support and no T124
> support?

In the spirit of this patch, adding entries for other tegra platforms would make
sense. Would you prefer that we leave out the dependency from SND_SOC_TEGRA30_*
and only select the right I2S driver to use in each codec driver?

If not, we'd have to list all relevant platforms both in the I2S/AHUB drivers
and in each codec's rules (which is not necessarily and issue, but there's no
need to have artificial platform dependencies).

What do you think?

-- 
Paul Kocialkowski, developer of free digital technology at the lower levels

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

signature.asc
Description: This is a digitally signed message part

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-04-18 Thread Stephen Warren


On 04/18/2017 09:11 AM, Paul Kocialkowski wrote:

This selects the tegra30 i2s and ahub controllers for the tegra124 SoC.
These are needed when building without ARCH_TEGRA_3x_SOC set.



diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
index efbe8d4c019e..bcd18d2cf7a7 100644
--- a/sound/soc/tegra/Kconfig
+++ b/sound/soc/tegra/Kconfig
@@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC || ARCH_TEGRA_124_SOC)


Is this really a compile-time dependency? If so, don't we need to add 
T210 and T186 entries into that || condition too, since we could be 
building a kernel with just T210/T186 support and no T124 support?

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-04-18 Thread Stephen Warren


On 04/18/2017 09:11 AM, Paul Kocialkowski wrote:

This selects the tegra30 i2s and ahub controllers for the tegra124 SoC.
These are needed when building without ARCH_TEGRA_3x_SOC set.



diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
index efbe8d4c019e..bcd18d2cf7a7 100644
--- a/sound/soc/tegra/Kconfig
+++ b/sound/soc/tegra/Kconfig
@@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC || ARCH_TEGRA_124_SOC)


Is this really a compile-time dependency? If so, don't we need to add 
T210 and T186 entries into that || condition too, since we could be 
building a kernel with just T210/T186 support and no T124 support?

[PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-04-18 Thread Paul Kocialkowski

This selects the tegra30 i2s and ahub controllers for the tegra124 SoC.
These are needed when building without ARCH_TEGRA_3x_SOC set.

Signed-off-by: Paul Kocialkowski 
---
 sound/soc/tegra/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
index efbe8d4c019e..bcd18d2cf7a7 100644
--- a/sound/soc/tegra/Kconfig
+++ b/sound/soc/tegra/Kconfig
@@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC || ARCH_TEGRA_124_SOC)
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
@@ -54,7 +54,7 @@ config SND_SOC_TEGRA30_AHUB
 
 config SND_SOC_TEGRA30_I2S
tristate
-   depends on SND_SOC_TEGRA && ARCH_TEGRA_3x_SOC
+   depends on SND_SOC_TEGRA && (ARCH_TEGRA_3x_SOC || ARCH_TEGRA_124_SOC)
select SND_SOC_TEGRA30_AHUB
help
  Say Y or M if you want to add support for codecs attached to the
@@ -123,7 +123,7 @@ config SND_SOC_TEGRA_MAX98090
tristate "SoC Audio support for Tegra boards using a MAX98090 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_TEGRA30_I2S if ARCH_TEGRA_3x_SOC || ARCH_TEGRA_124_SOC
select SND_SOC_MAX98090
help
  Say Y or M here if you want to add support for SoC audio on Tegra
-- 
2.12.2

[PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

2017-04-18 Thread Paul Kocialkowski

This selects the tegra30 i2s and ahub controllers for the tegra124 SoC.
These are needed when building without ARCH_TEGRA_3x_SOC set.

Signed-off-by: Paul Kocialkowski 
---
 sound/soc/tegra/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
index efbe8d4c019e..bcd18d2cf7a7 100644
--- a/sound/soc/tegra/Kconfig
+++ b/sound/soc/tegra/Kconfig
@@ -46,7 +46,7 @@ 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 && (ARCH_TEGRA_3x_SOC || ARCH_TEGRA_124_SOC)
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
@@ -54,7 +54,7 @@ config SND_SOC_TEGRA30_AHUB
 
 config SND_SOC_TEGRA30_I2S
tristate
-   depends on SND_SOC_TEGRA && ARCH_TEGRA_3x_SOC
+   depends on SND_SOC_TEGRA && (ARCH_TEGRA_3x_SOC || ARCH_TEGRA_124_SOC)
select SND_SOC_TEGRA30_AHUB
help
  Say Y or M if you want to add support for codecs attached to the
@@ -123,7 +123,7 @@ config SND_SOC_TEGRA_MAX98090
tristate "SoC Audio support for Tegra boards using a MAX98090 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_TEGRA30_I2S if ARCH_TEGRA_3x_SOC || ARCH_TEGRA_124_SOC
select SND_SOC_MAX98090
help
  Say Y or M here if you want to add support for SoC audio on Tegra
-- 
2.12.2

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

[PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

[PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for tegra124 SoC

22 matches

Site Navigation

Mail list logo

Footer information