linux-kernel - Re: [alsa-devel] [PATCH 1/2] ALSA: ASoc: TLV320AIC3X: ad SPI and clock on GPIO2 or BCLK

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: <20110402082604.GE21737@sirena.org.uk>
Date:	Sat, 2 Apr 2011 09:26:04 +0100
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	Prchal Ji?? <jiri.prchal@...ignal.cz>
Cc:	alsa-devel@...r.kernel.org, alsa-devel@...a-project.org,
	vbarinov@...eddedalley.com, linux-kernel@...r.kernel.org
Subject: Re: [alsa-devel] [PATCH 1/2] ALSA: ASoc: TLV320AIC3X: ad SPI and
	clock on GPIO2 or BCLK

On Thu, Mar 24, 2011 at 11:20:45AM +0100, Prchal Ji?? wrote:
> Hi,
> this patch ads SPI communication for codecs TLV320AIC3X and clock input on GPIO2 or BCLK.
> Tested on TLV320AIC3106 and AT91SAM9260.
> TODO: Set the model in SPI probe the right way. I don't know how.

Register two SPI drivers with different names.

> This codec communicates on SPI in 1st byte: 7 MSB is register address, 1 LSB is read/write, 2nd byte data. For this
> reason there is new functions in soc-cache.c snd_soc_7_8_*.

This looks mostly good but there's some cleanups needed, mostly from
extra stuff which snuck in there, and you need to rebase against current
code.

As covered in SubmittingPatches you should always CC maintainers on
patches.

>  config SND_SOC_AD73311
>  	tristate
> -
> +

Random whitespace change, should be removed.

>   * Author:      Vladimir Barinov, <vbarinov@...eddedalley.com>
> + *              Jiri Prchal, <jiri.prchal@...ignal.cz

If you're going to do this you probably want a > in there.

>  {
>  	struct snd_soc_codec *codec = codec_dai->codec;
>  	struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);
> +	u8 data;
> +
> +	/* set external clock on GPIO2 or BCLK */
> +	data = snd_soc_read(codec, AIC3X_CLKGEN_CTRL_REG);
> +	data &= 0x0f;
> +	data |= ((clk_id << PLLCLK_IN_SHIFT) | (clk_id << CLKDIV_IN_SHIFT));
> +	snd_soc_write(codec, AIC3X_CLKGEN_CTRL_REG, data);

This looks like an unrelated change which is specific to your board.
You should add an interface for configuring this functionality .

>  	if (ret != 0) {
> -		dev_err(codec->dev, "Failed to set cache I/O: %d\n", ret);
> +		dev_err(codec->dev, "failed to set cache I/O: %d\n", ret);

Why are you decapitalising th estart of the sentance here?

> +static struct spi_driver aic3x_spi_driver = {
> +	.driver = {
> +		.name	= "tlv320aic3x-codec",
> +		.owner	= THIS_MODULE,

Remove the -codec from the driver name.

> +/* special functions for codecs with 7 bit register address and LSB read/write (like TLV320AIC3X) */
> +static unsigned int snd_soc_7_8_read(struct snd_soc_codec *codec,
> +				     unsigned int reg)
> +{
> +	int ret;
> +	unsigned int val;

This won't apply against the current kernel and should be a separate
patch, it's a generic thing rather than part of the CODEC driver.
--
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/