linux-kernel - Re: [PATCH v3 7/8] ASoC: wm8998: Initial WM8998 codec driver

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 for Android: free password hash cracker in your pocket

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

Date:	Tue, 5 May 2015 11:53:27 +0100
From:	Mark Brown <broonie@...nel.org>
To:	Richard Fitzgerald <rf@...nsource.wolfsonmicro.com>
Cc:	lee.jones@...aro.org, linus.walleij@...aro.org, gnurou@...il.com,
	cw00.choi@...sung.com, myungjoo.ham@...sung.com,
	devicetree@...r.kernel.org, alsa-devel@...a-project.org,
	patches@...nsource.wolfsonmicro.com, linux-kernel@...r.kernel.org,
	linux-gpio@...r.kernel.org, ckeepax@...nsource.wolfsonmicro.com
Subject: Re: [PATCH v3 7/8] ASoC: wm8998: Initial WM8998 codec driver

On Tue, May 05, 2015 at 10:24:12AM +0100, Richard Fitzgerald wrote:
> On Mon, May 04, 2015 at 12:39:43PM +0100, Mark Brown wrote:
> > On Fri, May 01, 2015 at 04:15:18PM +0100, Richard Fitzgerald wrote:

> > > +		val = snd_soc_read(codec, ARIZONA_ASYNC_SAMPLE_RATE_1 + val);
> > > +		if (val >= 0x11)
> > > +			dev_warn(codec->dev, "Unsupported ASRC rate2\n");

> > ...some random other register.

> ARIZONA_ASRC_RATE2 register can only have the values 8 or 9 and we don't
> provide a way to set it to illegal values.

That's trusting of future maintainers of this code, and it doesn't look
great when bounds checking and validation code has some fairly obviously
missing bounds checking and validation.

> > > +static const char * const wm8998_inmux_texts[] = {
> > > +	"A",
> > > +	"B",

> > Those are some fun input names...

> ... that's what the mux positions are called and it seems nice to have consistent
> naming for all similar muxes across channels and across future codecs. Like for
> example to switch both muxes of a stereo input you set them both to the same
> position name (both "A" or both "B") is clearer, and quicker to spot in a dump
> of ALSA control settings, than having the two muxes set to different enumeration
> strings that actually mean the same mux position.

Equally when setting the muxes settings called A and B that's not really
telling people what they do.

Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)