linux-kernel - RE: [PATCH] asoc: tegra: wm8903: Simplify pin disconnect

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]

Message-ID: <74CDBE0F657A3D45AFBB94109FB122FF173EDABA68@HQMAIL01.nvidia.com>
Date:	Tue, 1 Nov 2011 10:05:15 -0700
From:	Stephen Warren <swarren@...dia.com>
To:	Taylor Hutt <thutt@...omium.org>
CC:	Liam Girdwood <lrg@...mlogic.co.uk>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.de>,
	"alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>
Subject: RE: [PATCH] asoc: tegra: wm8903: Simplify pin disconnect

Taylor Hutt wrote at Tuesday, November 01, 2011 10:26 AM:
> (Note that this patch fails the kernel patch checker because
>  the 'M' macro produces an error that it's value should be enclosed
>  in parenthesis; this is not actually possible for this use & expansion.)
> 
> Detail
> 
>   This change is a simplification, both in implementation and for
>   reasoning about whcih pins are connected and disconnected.
> 
>   The impetus for the change was the addition of new boards, and the
>   difficulty in reasoning about the previous code to disconnect pins.
> 
>   Now, unless a connection is specified for the current board, the pin
>   will be disconnected.
> 
>   A follow-on change will add the 'asymptote' board.
> 
> Testing
> 
>   Visual inspection from original code.
>   Built kernel.

As I mentioned in the ChromeOS review, I don't like this approach.

The main issue is that all the connectivity information is already present
in the ${machine}_audio_maps[] arrays, and this change duplicates it all
in the data-structures inside tegra_wm8903_disconnect_pins().

Instead, I believe a function should be added to the ASoC core that reads
the machine's dapm routes, and does the same thing based on that. Mark
Brown pointed out that this should be an optional utility function that
machine drivers should call, since applying it automatically might cause
issues for some other machines.

The second issue here is that the new code calls machine_is_ventana()
and the equivalent for other machines, and some of those functions will go
away next time Russell updates mach-types and removes entries for boards
that aren't supported with an explicit board file.

-- 
nvpublic

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/