[PATCH] ASoC: tlv320aic31xx: Fix jack detection after suspend
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(+)
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; }
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-j... 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/fd727e56e60de06a923175ce246e965e27c6... 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
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-j... 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/fd727e56e60de06a923175ce246e965e27c6... 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
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
participants (2)
-
kernel test robot -
Mark Brown