linux-kernel - Re: [PATCH 2/4] ASoC: rockchip: add rockchip machine driver for max98090

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: <20150512192636.GA3066@sirena.org.uk>
Date:	Tue, 12 May 2015 20:26:36 +0100
From:	Mark Brown <broonie@...nel.org>
To:	Xing Zheng <acgzxing@...il.com>
Cc:	dgreid@...omium.org, dianders@...omium.org, heiko@...ech.de,
	sonnyrao@...omium.org, linux-rockchip@...ts.infradead.org,
	zhengxing <zhengxing@...k-chips.com>,
	Liam Girdwood <lgirdwood@...il.com>,
	Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.de>,
	linux-kernel@...r.kernel.org, alsa-devel@...a-project.org,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 2/4] ASoC: rockchip: add rockchip machine driver for
 max98090

On Tue, May 12, 2015 at 05:26:35PM +0800, Xing Zheng wrote:

>  sound/soc/rockchip/Kconfig                   |   10 ++
>  sound/soc/rockchip/Makefile                  |    2 +
>  sound/soc/rockchip/rockchip_machine_driver.c |    6 +
>  sound/soc/rockchip/rockchip_machine_driver.h |    6 +
>  sound/soc/rockchip/rockchip_max98090.c       |  185 ++++++++++++++++++++++++++

This looks more like a normal and reasonable machine driver but then why
have you created the generic rockchip machine driver?  It seems like
this should just be a regular machine driver like other platforms have,
were it not for that this would be mostly fine apart from a couple of
nitpicks below.

> +config SND_SOC_ROCKCHIP_MAX98090
> +	tristate "ASoC support for Rockchip boards using a MAX98090 codec"
> +	depends on SND_SOC_ROCKCHIP && I2C && GPIOLIB
> +	select SND_SOC_ROCKCHIP_I2S
> +	select SND_SOC_MAX98090
> +	select SND_SOC_TS3A227E

This looks like it's a driver specific to Chromebooks (possibly even
specific Chromeooks) and so should be named as such.

> +	card->dapm.idle_bias_off = true;

Just set this when declaring the card, don't do it at runtime.

> +	snd_soc_dapm_enable_pin(dapm, "Headset Mic");
> +	snd_soc_dapm_enable_pin(dapm, "Headphone");
> +	snd_soc_dapm_enable_pin(dapm, "Speaker");
> +	snd_soc_dapm_enable_pin(dapm, "Int Mic");

No need to do this, all pins are enabled by default.

> +	snd_soc_dapm_sync(dapm);

This has no effect during initialization.

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