| Message ID | 20210723180200.25105-1-broonie@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: tlv320aic31xx: Fix jack detection after suspend | expand |
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
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
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 --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; }
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(+)