linux-kernel - Re: [RFC PATCH] ASoC: tas2552: Support TI TAS2552 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]

Date:	Wed, 18 Jun 2014 19:44:40 +0100
From:	Mark Brown <broonie@...nel.org>
To:	Dan Murphy <dmurphy@...com>
Cc:	linux-sound@...r.kernel.org, linux-kernel@...r.kernel.org,
	alsa-devel@...a-project.org
Subject: Re: [RFC PATCH] ASoC: tas2552: Support TI TAS2552 Amplifier

On Wed, Jun 18, 2014 at 12:45:46PM -0500, Dan Murphy wrote:

> +static int tas2552_i2c_read(struct tas2552_data *tas_data, int reg)
> +{
> +	int val;
> +
> +	if (WARN_ON(!tas_data->tas2552_client))
> +		return -EINVAL;
> +
> +	/* If powered off, return the cached value */
> +	mutex_lock(&tas_data->mutex);
> +	if (tas_data->power_state) {
> +		val = i2c_smbus_read_byte_data(tas_data->tas2552_client, reg);

Don't open code your I/O, use regmap like any other CODEC driver.  This
will remove a very large proportion of the code in the driver which
replicates features of the regmap framework.

> +static int tas2552_startup(struct snd_pcm_substream *substream,
> +			   struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_codec *codec = dai->codec;
> +	struct tas2552_data *tas2552 = snd_soc_codec_get_drvdata(codec);
> +
> +	tas2552_sw_shutdown(tas2552, 1);
> +	tas2552_power(tas2552, 1);
> +	/* Turn on Class D amplifier */
> +	tas2552_i2c_write(tas2552, TAS2552_CFG_2, 0x80);
> +	tas2552_sw_shutdown(tas2552, 0);

I'd expect this to be done using DAPM.

> +static const struct regmap_config tas2552_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +
> +	.max_register = TAS2552_MAX_REG,
> +	.reg_defaults = NULL,
> +	.num_reg_defaults = 0,

No need to set things to zero or NULL in static variables, that's the
default.

> +	dev = &client->dev;
> +	data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
> +	if (data == NULL) {
> +		dev_err(dev, "Can not allocate memory\n");

No need to print an error, OOM messages are already quite noisy.

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