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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 24 Apr 2017 17:07:26 +0200
From:   Paul Kocialkowski <contact@...lk.fr>
To:     Stephen Warren <swarren@...dotorg.org>,
        Thierry Reding <thierry.reding@...il.com>
Cc:     linux-kernel@...r.kernel.org, alsa-devel@...a-project.org,
        linux-tegra@...r.kernel.org, Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        Alexandre Courbot <gnurou@...il.com>,
        Marcel Ziswiler <marcel.ziswiler@...adex.com>,
        Rob Herring <robh@...nel.org>
Subject: Re: [PATCH 3/3] sound: ASoC: tegra: Select tegra30 i2s and ahub for
 tegra124 SoC

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/
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)