diff mbox series

ASoC: tlv320aic31xx: Fix jack detection after suspend

Message ID 20210723180200.25105-1-broonie@kernel.org
State Accepted
Commit 2c39ca6885a2ec03e5c9e7c12a4da2aa8926605a
Headers show
Series ASoC: tlv320aic31xx: Fix jack detection after suspend | expand

Commit Message

Mark Brown July 23, 2021, 6:02 p.m. UTC 
The tlv320aic31xx driver relies on regcache_sync() to restore the register
contents after going to _BIAS_OFF, for example during system suspend. This
does not work for the jack detection configuration since that is configured
via the same register that status is read back from so the register is
volatile and not cached. This can also cause issues during init if the jack
detection ends up getting set up before the CODEC is initially brought out
of _BIAS_OFF, we will reset the CODEC and resync the cache as part of that
process.

Fix this by explicitly reapplying the jack detection configuration after
resyncing the register cache during power on.

This issue was found by an engineer working off-list on a product
kernel, I just wrote up the upstream fix.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/tlv320aic31xx.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

kernel test robot July 24, 2021, 12:21 a.m. UTC | #1 
Hi Mark,

I love your patch! Yet something to improve:

[auto build test ERROR on asoc/for-next]
[also build test ERROR on v5.14-rc2 next-20210723]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Mark-Brown/ASoC-tlv320aic31xx-Fix-jack-detection-after-suspend/20210724-020429
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: x86_64-randconfig-c022-20210723 (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/fd727e56e60de06a923175ce246e965e27c6df88
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Mark-Brown/ASoC-tlv320aic31xx-Fix-jack-detection-after-suspend/20210724-020429
        git checkout fd727e56e60de06a923175ce246e965e27c6df88
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   sound/soc/codecs/tlv320aic31xx.c: In function 'aic31xx_power_on':
>> sound/soc/codecs/tlv320aic31xx.c:1264:2: error: implicit declaration of function 'aic31xx_set_jack'; did you mean 'aic31xx_set_dai_fmt'? [-Werror=implicit-function-declaration]
    1264 |  aic31xx_set_jack(component, aic31xx->jack, NULL);
         |  ^~~~~~~~~~~~~~~~
         |  aic31xx_set_dai_fmt
   sound/soc/codecs/tlv320aic31xx.c: At top level:
>> sound/soc/codecs/tlv320aic31xx.c:1312:12: error: static declaration of 'aic31xx_set_jack' follows non-static declaration
    1312 | static int aic31xx_set_jack(struct snd_soc_component *component,
         |            ^~~~~~~~~~~~~~~~
   sound/soc/codecs/tlv320aic31xx.c:1264:2: note: previous implicit declaration of 'aic31xx_set_jack' was here
    1264 |  aic31xx_set_jack(component, aic31xx->jack, NULL);
         |  ^~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +1264 sound/soc/codecs/tlv320aic31xx.c

  1231	
  1232	static int aic31xx_power_on(struct snd_soc_component *component)
  1233	{
  1234		struct aic31xx_priv *aic31xx = snd_soc_component_get_drvdata(component);
  1235		int ret;
  1236	
  1237		ret = regulator_bulk_enable(ARRAY_SIZE(aic31xx->supplies),
  1238					    aic31xx->supplies);
  1239		if (ret)
  1240			return ret;
  1241	
  1242		regcache_cache_only(aic31xx->regmap, false);
  1243	
  1244		/* Reset device registers for a consistent power-on like state */
  1245		ret = aic31xx_reset(aic31xx);
  1246		if (ret < 0)
  1247			dev_err(aic31xx->dev, "Could not reset device: %d\n", ret);
  1248	
  1249		ret = regcache_sync(aic31xx->regmap);
  1250		if (ret) {
  1251			dev_err(component->dev,
  1252				"Failed to restore cache: %d\n", ret);
  1253			regcache_cache_only(aic31xx->regmap, true);
  1254			regulator_bulk_disable(ARRAY_SIZE(aic31xx->supplies),
  1255					       aic31xx->supplies);
  1256			return ret;
  1257		}
  1258	
  1259		/*
  1260		 * The jack detection configuration is in the same register
  1261		 * that is used to report jack detect status so is volatile
  1262		 * and not covered by the cache sync, restore it separately.
  1263		 */
> 1264		aic31xx_set_jack(component, aic31xx->jack, NULL);
  1265	
  1266		return 0;
  1267	}
  1268	
  1269	static void aic31xx_power_off(struct snd_soc_component *component)
  1270	{
  1271		struct aic31xx_priv *aic31xx = snd_soc_component_get_drvdata(component);
  1272	
  1273		regcache_cache_only(aic31xx->regmap, true);
  1274		regulator_bulk_disable(ARRAY_SIZE(aic31xx->supplies),
  1275				       aic31xx->supplies);
  1276	}
  1277	
  1278	static int aic31xx_set_bias_level(struct snd_soc_component *component,
  1279					  enum snd_soc_bias_level level)
  1280	{
  1281		dev_dbg(component->dev, "## %s: %d -> %d\n", __func__,
  1282			snd_soc_component_get_bias_level(component), level);
  1283	
  1284		switch (level) {
  1285		case SND_SOC_BIAS_ON:
  1286			break;
  1287		case SND_SOC_BIAS_PREPARE:
  1288			if (snd_soc_component_get_bias_level(component) == SND_SOC_BIAS_STANDBY)
  1289				aic31xx_clk_on(component);
  1290			break;
  1291		case SND_SOC_BIAS_STANDBY:
  1292			switch (snd_soc_component_get_bias_level(component)) {
  1293			case SND_SOC_BIAS_OFF:
  1294				aic31xx_power_on(component);
  1295				break;
  1296			case SND_SOC_BIAS_PREPARE:
  1297				aic31xx_clk_off(component);
  1298				break;
  1299			default:
  1300				BUG();
  1301			}
  1302			break;
  1303		case SND_SOC_BIAS_OFF:
  1304			if (snd_soc_component_get_bias_level(component) == SND_SOC_BIAS_STANDBY)
  1305				aic31xx_power_off(component);
  1306			break;
  1307		}
  1308	
  1309		return 0;
  1310	}
  1311	
> 1312	static int aic31xx_set_jack(struct snd_soc_component *component,
  1313				    struct snd_soc_jack *jack, void *data)
  1314	{
  1315		struct aic31xx_priv *aic31xx = snd_soc_component_get_drvdata(component);
  1316	
  1317		aic31xx->jack = jack;
  1318	
  1319		/* Enable/Disable jack detection */
  1320		regmap_write(aic31xx->regmap, AIC31XX_HSDETECT,
  1321			     jack ? AIC31XX_HSD_ENABLE : 0);
  1322	
  1323		return 0;
  1324	}
  1325	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot July 24, 2021, 2:36 a.m. UTC | #2 
Hi Mark,

I love your patch! Yet something to improve:

[auto build test ERROR on asoc/for-next]
[also build test ERROR on v5.14-rc2 next-20210723]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Mark-Brown/ASoC-tlv320aic31xx-Fix-jack-detection-after-suspend/20210724-020429
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: arm-randconfig-r022-20210723 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 9625ca5b602616b2f5584e8a49ba93c52c141e40)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/fd727e56e60de06a923175ce246e965e27c6df88
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Mark-Brown/ASoC-tlv320aic31xx-Fix-jack-detection-after-suspend/20210724-020429
        git checkout fd727e56e60de06a923175ce246e965e27c6df88
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> sound/soc/codecs/tlv320aic31xx.c:1264:2: error: implicit declaration of function 'aic31xx_set_jack' [-Werror,-Wimplicit-function-declaration]
           aic31xx_set_jack(component, aic31xx->jack, NULL);
           ^
   sound/soc/codecs/tlv320aic31xx.c:1312:12: error: static declaration of 'aic31xx_set_jack' follows non-static declaration
   static int aic31xx_set_jack(struct snd_soc_component *component,
              ^
   sound/soc/codecs/tlv320aic31xx.c:1264:2: note: previous implicit declaration is here
           aic31xx_set_jack(component, aic31xx->jack, NULL);
           ^
   2 errors generated.


vim +/aic31xx_set_jack +1264 sound/soc/codecs/tlv320aic31xx.c

  1231	
  1232	static int aic31xx_power_on(struct snd_soc_component *component)
  1233	{
  1234		struct aic31xx_priv *aic31xx = snd_soc_component_get_drvdata(component);
  1235		int ret;
  1236	
  1237		ret = regulator_bulk_enable(ARRAY_SIZE(aic31xx->supplies),
  1238					    aic31xx->supplies);
  1239		if (ret)
  1240			return ret;
  1241	
  1242		regcache_cache_only(aic31xx->regmap, false);
  1243	
  1244		/* Reset device registers for a consistent power-on like state */
  1245		ret = aic31xx_reset(aic31xx);
  1246		if (ret < 0)
  1247			dev_err(aic31xx->dev, "Could not reset device: %d\n", ret);
  1248	
  1249		ret = regcache_sync(aic31xx->regmap);
  1250		if (ret) {
  1251			dev_err(component->dev,
  1252				"Failed to restore cache: %d\n", ret);
  1253			regcache_cache_only(aic31xx->regmap, true);
  1254			regulator_bulk_disable(ARRAY_SIZE(aic31xx->supplies),
  1255					       aic31xx->supplies);
  1256			return ret;
  1257		}
  1258	
  1259		/*
  1260		 * The jack detection configuration is in the same register
  1261		 * that is used to report jack detect status so is volatile
  1262		 * and not covered by the cache sync, restore it separately.
  1263		 */
> 1264		aic31xx_set_jack(component, aic31xx->jack, NULL);
  1265	
  1266		return 0;
  1267	}
  1268	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Mark Brown July 26, 2021, 4:08 p.m. UTC | #3 
On Fri, 23 Jul 2021 19:02:00 +0100, Mark Brown wrote:
> The tlv320aic31xx driver relies on regcache_sync() to restore the register
> contents after going to _BIAS_OFF, for example during system suspend. This
> does not work for the jack detection configuration since that is configured
> via the same register that status is read back from so the register is
> volatile and not cached. This can also cause issues during init if the jack
> detection ends up getting set up before the CODEC is initially brought out
> of _BIAS_OFF, we will reset the CODEC and resync the cache as part of that
> process.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: tlv320aic31xx: Fix jack detection after suspend
      commit: 2c39ca6885a2ec03e5c9e7c12a4da2aa8926605a

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
diff mbox series

Patch

diff --git a/sound/soc/codecs/tlv320aic31xx.c b/sound/soc/codecs/tlv320aic31xx.c
index b504d63385b3..e3da8f41fd1a 100644
--- a/sound/soc/codecs/tlv320aic31xx.c
+++ b/sound/soc/codecs/tlv320aic31xx.c
@@ -1256,6 +1256,13 @@  static int aic31xx_power_on(struct snd_soc_component *component)
 		return ret;
 	}
 
+	/*
+	 * The jack detection configuration is in the same register
+	 * that is used to report jack detect status so is volatile
+	 * and not covered by the cache sync, restore it separately.
+	 */
+	aic31xx_set_jack(component, aic31xx->jack, NULL);
+
 	return 0;
 }