linux-kernel - Re: [PATCH 3/3] ASoC: codecs: add driver for max9768 amplifier

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: <20120127162501.GA18572@opensource.wolfsonmicro.com>
Date:	Fri, 27 Jan 2012 16:25:01 +0000
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	Wolfram Sang <w.sang@...gutronix.de>
Cc:	alsa-devel@...a-project.org, linux-kernel@...r.kernel.org,
	Liam Girdwood <lrg@...com>,
	Lars-Peter Clausen <lars@...afoo.de>
Subject: Re: [PATCH 3/3] ASoC: codecs: add driver for max9768 amplifier

On Fri, Jan 27, 2012 at 04:10:23PM +0100, Wolfram Sang wrote:

> Mark: Still no DAPM support. We don't have PM on this board, and since I
> couldn't easily figure what is expected from me here, I'd like to leave this
> for people who have real interest in that.

I don't understand what was unclear here...

You should at least use set_bias_level() to manage the shutdown GPIO I
guess.

> +static const struct snd_kcontrol_new max9768_volume[] = {
> +	SOC_SINGLE_TLV("Playback Volume", MAX9768_VOL, 0, 63, 0, volume_tlv),
> +};

> +static const struct snd_kcontrol_new max9768_mute[] = {
> +	SOC_SINGLE_BOOL_EXT("Mute Switch", 0, max9768_get_gpio, max9768_set_gpio),
> +};

Should be "Playback Switch" to match the volume control - that way
userspace can present a single control to applications.

> +static bool max9768_always_false(struct device *dev, unsigned int reg)
> +{
> +	return false;
> +}

This should be the default in regmap for formats that use format_write
(and hence can't read back), please fix in regmap rather than add this.  

> +	if (pdata) {
> +		/* Mute on powerup to avoid clicks */
> +		err = gpio_request_one(pdata->mute_gpio, GPIOF_INIT_HIGH, "MAX9768 Mute");

This need to avoid clicks is the reason I was asking for the DAPM stuff.

> +		/* Activate chip by releasing shutdown, enables I2C */
> +		err = gpio_request_one(pdata->shdn_gpio, GPIOF_INIT_HIGH, "MAX9768 Shutdown");
> +		max9768->shdn_gpio = err ?: pdata->shdn_gpio;

Eeew.

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