From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753311AbdGSLJE (ORCPT ); Wed, 19 Jul 2017 07:09:04 -0400 Received: from mezzanine.sirena.org.uk ([106.187.55.193]:41020 "EHLO mezzanine.sirena.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751754AbdGSLJC (ORCPT ); Wed, 19 Jul 2017 07:09:02 -0400 Date: Wed, 19 Jul 2017 12:08:42 +0100 From: Mark Brown To: Michael Stecklein Cc: Rob Herring , Mark Rutland , Jaroslav Kysela , Takashi Iwai , alsa-devel@alsa-project.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, "Andrew F . Davis" Message-ID: <20170719110842.lb2f5cqawnkenhsb@sirena.org.uk> References: <1500405604-23511-1-git-send-email-m-stecklein@ti.com> <1500405604-23511-3-git-send-email-m-stecklein@ti.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="dhu4rczr3qhcndvx" Content-Disposition: inline In-Reply-To: <1500405604-23511-3-git-send-email-m-stecklein@ti.com> X-Cookie: Your fault -- core dumped User-Agent: NeoMutt/20170609 (1.8.3) X-SA-Exim-Connect-IP: 2001:470:1f1d:6b5::3 X-SA-Exim-Mail-From: broonie@sirena.org.uk Subject: Re: [PATCH v3 2/2] ASoC: add support for TAS6424 digital amplifier X-SA-Exim-Version: 4.2.1 (built Tue, 02 Aug 2016 21:08:31 +0000) X-SA-Exim-Scanned: No (on mezzanine.sirena.org.uk); Unknown failure Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --dhu4rczr3qhcndvx Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Jul 18, 2017 at 02:20:04PM -0500, Michael Stecklein wrote: > + if (!tx_mask) { > + dev_err(codec->dev, "tdm mask must not be 0\n"); > + return -EINVAL; > + } Setting the mask to 0 is used when turning off TDM. > + if ((reg & TAS6424_WARN_VDD_UV) && !(tas6424->last_warn & TAS6424_WARN_VDD_UV)) > + dev_warn(dev, "experienced a VDD under voltage condition\n"); > + > + if ((reg & TAS6424_WARN_VDD_POR) && !(tas6424->last_warn & TAS6424_WARN_VDD_POR)) > + dev_warn(dev, "experienced a VDD POR condition\n"); These look like they are errors rather than warnings and should be critical prints like the other errors. > + /* Store current warn value so we can detect any changes next time */ > + tas6424->last_warn = reg; It would be better to clear these at some point, perhaps after a delay. Especially with thermal issues they could come and go over device operation. > +static int tas6424_codec_probe(struct snd_soc_codec *codec) > +{ > + struct tas6424_data *tas6424 = snd_soc_codec_get_drvdata(codec); > + int ret; > + > + tas6424->codec = codec; > + > + ret = regulator_bulk_enable(ARRAY_SIZE(tas6424->supplies), > + tas6424->supplies); > + if (ret != 0) { > + dev_err(codec->dev, "failed to enable supplies: %d\n", ret); > + return ret; > + } This init stuff should be in either the main I2C probe, DAPM or bias level management. The CODEC probe should be very minimal. > + if (event & SND_SOC_DAPM_POST_PMU) { > + } else if (event & SND_SOC_DAPM_PRE_PMD) { You're trying to write a switch statement here. > + switch (level) { > + case SND_SOC_BIAS_ON: > + case SND_SOC_BIAS_PREPARE: > + msleep(500); > + break; You need a 500ms sleep in all these cases? That's a lot of sleeping. > + case SND_SOC_BIAS_STANDBY: > + ret = snd_soc_update_bits(codec, TAS6424_CH_STATE_CTRL_REG, > + 0xff, TAS6424_CH1_STATE_MUTE | TAS6424_CH2_STATE_MUTE | > + TAS6424_CH3_STATE_MUTE | TAS6424_CH4_STATE_MUTE); > + > + if (ret < 0) { > + dev_err(codec->dev, "error resuming device: %d\n", ret); > + return ret; > + } > + break; > + case SND_SOC_BIAS_OFF: > + ret = snd_soc_update_bits(codec, TAS6424_CH_STATE_CTRL_REG, > + 0xff, TAS6424_CH1_STATE_HIZ | TAS6424_CH2_STATE_HIZ | > + TAS6424_CH3_STATE_HIZ | TAS6424_CH4_STATE_HIZ); > + > + if (ret < 0) { > + dev_err(codec->dev, "error suspending device: %d\n", ret); > + return ret; > + } These mutes seem very random disassociated from anything else? --dhu4rczr3qhcndvx Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAllvPbkACgkQJNaLcl1U h9Dxugf/eJpPWVzXAnLcrdkQ2IJqfzWjJ4RSDiykX8ScdeYpR5osQswoiuWrwJFg 2o6/3MXHlYZOhpJSKQhkJJqG6nNlrBCFuTd7Jyz8ugl2Lei7qTwD1dzsNCfjTcNz t2zekW6RjvMya+7j/6j1qNXS1+cNup5rSTQdv1/iYXsWsNtSG2PT+aDhLHTHd8OV TnMuRObJO5Iz2DHjKklKKE3NpvpW5I+naD2ssT97evp6Fds2GQSazW98fuPu00LZ 5FZFANZMdCMvrcGzgSsvFgYu33R+t7SyS3RlamnWxcSTU+n3jFpz2e8wyDxl9TbL 862VOQQAkM6ANHKcrEtYVDhf6OiNcQ== =d82M -----END PGP SIGNATURE----- --dhu4rczr3qhcndvx--