lkml.org 
[lkml]   [2014]   [Jul]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v7] ASoC: tas2552: Support TI TAS2552 Amplifier
Date
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



\
 
 \ /
  Last update: 2014-07-17 19:41    [W:0.042 / U:0.320 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site