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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 17 Jul 2014 17:07:07 +0000 From: "Murphy, Dan" <dmurphy@...com> To: Mark Brown <broonie@...nel.org> CC: "linux-sound@...r.kernel.org" <linux-sound@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "alsa-devel@...a-project.org" <alsa-devel@...a-project.org>, "devicetree@...r.kernel.org" <devicetree@...r.kernel.org> Subject: Re: [PATCH v7] ASoC: tas2552: Support TI TAS2552 Amplifier Mark On 07/17/2014 11:58 AM, Mark Brown wrote: > On Mon, Jul 14, 2014 at 03:10:45PM -0500, Dan Murphy wrote: > > There's a few smallish issues below but this is basically good so I've > applied it, please send incremental fixed for the things below. > >> + /* Turn on Class D amplifier */ >> + snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_CLASSD_EN_MASK, >> + TAS2552_CLASSD_EN); >> + > Why is this being done in hw_params() and not using DAPM? > >> +static int tas2552_runtime_suspend(struct device *dev) >> +{ >> + struct tas2552_data *tas2552 = dev_get_drvdata(dev); >> + >> + tas2552_sw_shutdown(tas2552, 0); >> + >> + if (tas2552->enable_gpio) >> + gpiod_set_value(tas2552->enable_gpio, 0); >> + >> + regcache_cache_only(tas2552->regmap, true); >> + regcache_mark_dirty(tas2552->regmap); > It's better to do the GPIO set after making the device cache only in > order to be sure nothing can come in and try to use the register map > between the two. > >> +static void tas2552_shutdown(struct snd_pcm_substream *substream, >> + struct snd_soc_dai *dai) >> +{ >> + struct snd_soc_codec *codec = dai->codec; >> + >> + snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE, 0); >> +} > I'd also expect the PLL power to be managed via DAPM. > >> + ret = pm_runtime_get_sync(codec->dev); >> + if (ret < 0) { >> + dev_err(codec->dev, "Enabling device failed: %d\n", >> + ret); >> + goto probe_fail; >> + } > There's no matching put for this in remove(). > >> + snd_soc_write(codec, TAS2552_CFG_2, TAS2552_CLASSD_EN | >> + TAS2552_BOOST_EN | TAS2552_APT_EN | >> + TAS2552_LIM_EN); >> + return 0; > The class D is still being enabled here. Thanks will send updates in a day or two. -- ------------------ Dan Murphy -- 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/
Powered by blists - more mailing lists