[alsa-devel] [PATCH 0/5] ASoC: tas571x: fix of several issues for TAS5717
An extensive usage of the TAS571x driver with the TAS5717 chip revealed several issues which in the first place caused audio to fail to play. This set of one-line and trivial patches fixes them.
The issues were: * improper handling of the power-down signal * some timings not obeyed * bias level not saved in the driver * channel mixer volume controls in the wrong chip structure ;-)
Petr Kulhavy (5): ASoC: tas571x: fix missing bias save level ASoC: tas571x: extend the t_i2c time to comply with TAS5721 ASoC: tas571x: remove improper PDN signal usage in set_bias_level ASoC: tas571x: wait 50ms after oscillator trim ASoC: tas571x: move mixer volume controls from TAS5711 to TAS5717
sound/soc/codecs/tas571x.c | 38 +++++++++++++------------------------- 1 file changed, 13 insertions(+), 25 deletions(-)
The tas571x_set_bias_level() was not saving the new bias level to codec->dapm.bias_level. This had the effect that bias was switching between standby and off levels and never entered the on level.
This fix saves the bias level before returning from the function.
Signed-off-by: Petr Kulhavy brain@jikos.cz --- sound/soc/codecs/tas571x.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/soc/codecs/tas571x.c b/sound/soc/codecs/tas571x.c index df5e5cb..e996313 100644 --- a/sound/soc/codecs/tas571x.c +++ b/sound/soc/codecs/tas571x.c @@ -360,6 +360,7 @@ static int tas571x_set_bias_level(struct snd_soc_codec *codec, break; }
+ codec->dapm.bias_level = level; return 0; }
On 10/03/2016 04:40 PM, Petr Kulhavy wrote:
The tas571x_set_bias_level() was not saving the new bias level to codec->dapm.bias_level. This had the effect that bias was switching between standby and off levels and never entered the on level.
This fix saves the bias level before returning from the function.
This is done by the ASoC core after the function completes, so this should not be necessary.
On 03/10/16 19:09, Lars-Peter Clausen wrote:
On 10/03/2016 04:40 PM, Petr Kulhavy wrote:
The tas571x_set_bias_level() was not saving the new bias level to codec->dapm.bias_level. This had the effect that bias was switching between standby and off levels and never entered the on level.
This fix saves the bias level before returning from the function.
This is done by the ASoC core after the function completes, so this should not be necessary.
Hi Lars,
since what kernel version is it handled by the ASoC core? I've been testing it on a 4.1 based iMX kernel and there the bias levels were handled improperly...
Thanks Petr
On 10/03/2016 09:00 PM, Petr Kulhavy wrote:
On 03/10/16 19:09, Lars-Peter Clausen wrote:
On 10/03/2016 04:40 PM, Petr Kulhavy wrote:
The tas571x_set_bias_level() was not saving the new bias level to codec->dapm.bias_level. This had the effect that bias was switching between standby and off levels and never entered the on level.
This fix saves the bias level before returning from the function.
This is done by the ASoC core after the function completes, so this should not be necessary.
Hi Lars,
since what kernel version is it handled by the ASoC core? I've been testing it on a 4.1 based iMX kernel and there the bias levels
Since v4.2. This is the commit: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=fa...
On 10/03/2016 09:11 PM, Lars-Peter Clausen wrote:
On 10/03/2016 09:00 PM, Petr Kulhavy wrote:
On 03/10/16 19:09, Lars-Peter Clausen wrote:
On 10/03/2016 04:40 PM, Petr Kulhavy wrote:
The tas571x_set_bias_level() was not saving the new bias level to codec->dapm.bias_level. This had the effect that bias was switching between standby and off levels and never entered the on level.
This fix saves the bias level before returning from the function.
This is done by the ASoC core after the function completes, so this should not be necessary.
Hi Lars,
since what kernel version is it handled by the ASoC core? I've been testing it on a 4.1 based iMX kernel and there the bias levels
Since v4.2. This is the commit: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=fa...
Sorry, wrong ID, this one is it: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=f4...
On 03/10/16 21:13, Lars-Peter Clausen wrote:
On 10/03/2016 09:11 PM, Lars-Peter Clausen wrote:
On 10/03/2016 09:00 PM, Petr Kulhavy wrote:
On 03/10/16 19:09, Lars-Peter Clausen wrote:
On 10/03/2016 04:40 PM, Petr Kulhavy wrote:
The tas571x_set_bias_level() was not saving the new bias level to codec->dapm.bias_level. This had the effect that bias was switching between standby and off levels and never entered the on level.
This fix saves the bias level before returning from the function.
This is done by the ASoC core after the function completes, so this should not be necessary.
Hi Lars,
since what kernel version is it handled by the ASoC core? I've been testing it on a 4.1 based iMX kernel and there the bias levels
Since v4.2. This is the commit: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=fa...
Sorry, wrong ID, this one is it: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=f4...
OK, that explains. Then this patch can be dropped.
Thanks Petr
TAS5721 datasheet recommends to wait at least 13.5ms after deasserting the RESET signal. For TAS5717 this time is only 12ms, which was the original value in the code.
Extend the wait time after deasserting RESET from 12 to 13.5ms to comply with the TAS5721 specification.
Signed-off-by: Petr Kulhavy brain@jikos.cz --- sound/soc/codecs/tas571x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/tas571x.c b/sound/soc/codecs/tas571x.c index e996313..4d61357 100644 --- a/sound/soc/codecs/tas571x.c +++ b/sound/soc/codecs/tas571x.c @@ -748,7 +748,7 @@ static int tas571x_i2c_probe(struct i2c_client *client, /* pulse the active low reset line for ~100us */ usleep_range(100, 200); gpiod_set_value(priv->reset_gpio, 0); - usleep_range(12000, 20000); + usleep_range(13500, 20000); }
ret = regmap_write(priv->regmap, TAS571X_OSC_TRIM_REG, 0);
The patch
ASoC: tas571x: extend the t_i2c time to comply with TAS5721
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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
From e54de7f555970915c3856e7709a7ff74d352f315 Mon Sep 17 00:00:00 2001
From: Petr Kulhavy brain@jikos.cz Date: Mon, 3 Oct 2016 16:40:47 +0200 Subject: [PATCH] ASoC: tas571x: extend the t_i2c time to comply with TAS5721
TAS5721 datasheet recommends to wait at least 13.5ms after deasserting the RESET signal. For TAS5717 this time is only 12ms, which was the original value in the code.
Extend the wait time after deasserting RESET from 12 to 13.5ms to comply with the TAS5721 specification.
Signed-off-by: Petr Kulhavy brain@jikos.cz Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/tas571x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/tas571x.c b/sound/soc/codecs/tas571x.c index 34e4ce6f23d8..512b9e663e89 100644 --- a/sound/soc/codecs/tas571x.c +++ b/sound/soc/codecs/tas571x.c @@ -747,7 +747,7 @@ static int tas571x_i2c_probe(struct i2c_client *client, /* pulse the active low reset line for ~100us */ usleep_range(100, 200); gpiod_set_value(priv->reset_gpio, 0); - usleep_range(12000, 20000); + usleep_range(13500, 20000); }
ret = regmap_write(priv->regmap, TAS571X_OSC_TRIM_REG, 0);
The set_bias_level toggles the PDN signal when entering SND_SOC_BIAS_STANDBY and SND_SOC_BIAS_OFF. However this has no effect and actually breaks things down (tested with TAS5717) due to the following reasons:
1) holding down PDN does not save power but holding down RST does 2) now hard mute via register 0x5 is implemented and therefore it is no longer needed to toggle PDN to enter all channel shut down 3) in order to leave PDN it is required to toggle the RST signal (see TAS5721 datasheet), which was not implemented 4) toggling PDN as implemented actually mutes PWMs and there is no audio output (tested on TAS5717)
For these reasons remove the PDN signal toggling and just initialize it to inactive in probe().
Signed-off-by: Petr Kulhavy brain@jikos.cz --- sound/soc/codecs/tas571x.c | 14 -------------- 1 file changed, 14 deletions(-)
diff --git a/sound/soc/codecs/tas571x.c b/sound/soc/codecs/tas571x.c index 4d61357..a8a3279 100644 --- a/sound/soc/codecs/tas571x.c +++ b/sound/soc/codecs/tas571x.c @@ -341,20 +341,9 @@ static int tas571x_set_bias_level(struct snd_soc_codec *codec, return ret; } } - - gpiod_set_value(priv->pdn_gpio, 0); - usleep_range(5000, 6000); - - regcache_cache_only(priv->regmap, false); - ret = regcache_sync(priv->regmap); - if (ret) - return ret; } break; case SND_SOC_BIAS_OFF: - regcache_cache_only(priv->regmap, true); - gpiod_set_value(priv->pdn_gpio, 1); - if (!IS_ERR(priv->mclk)) clk_disable_unprepare(priv->mclk); break; @@ -771,9 +760,6 @@ static int tas571x_i2c_probe(struct i2c_client *client, return ret; }
- regcache_cache_only(priv->regmap, true); - gpiod_set_value(priv->pdn_gpio, 1); - return snd_soc_register_codec(&client->dev, &priv->codec_driver, &tas571x_dai, 1); }
The patch
ASoC: tas571x: remove improper PDN signal usage in set_bias_level
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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
From 8b0678dc914271e9ad4317fe82555ead92ce1cb9 Mon Sep 17 00:00:00 2001
From: Petr Kulhavy brain@jikos.cz Date: Mon, 3 Oct 2016 16:40:48 +0200 Subject: [PATCH] ASoC: tas571x: remove improper PDN signal usage in set_bias_level
The set_bias_level toggles the PDN signal when entering SND_SOC_BIAS_STANDBY and SND_SOC_BIAS_OFF. However this has no effect and actually breaks things down (tested with TAS5717) due to the following reasons:
1) holding down PDN does not save power but holding down RST does 2) now hard mute via register 0x5 is implemented and therefore it is no longer needed to toggle PDN to enter all channel shut down 3) in order to leave PDN it is required to toggle the RST signal (see TAS5721 datasheet), which was not implemented 4) toggling PDN as implemented actually mutes PWMs and there is no audio output (tested on TAS5717)
For these reasons remove the PDN signal toggling and just initialize it to inactive in probe().
Signed-off-by: Petr Kulhavy brain@jikos.cz Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/tas571x.c | 14 -------------- 1 file changed, 14 deletions(-)
diff --git a/sound/soc/codecs/tas571x.c b/sound/soc/codecs/tas571x.c index 512b9e663e89..810369f687d7 100644 --- a/sound/soc/codecs/tas571x.c +++ b/sound/soc/codecs/tas571x.c @@ -341,20 +341,9 @@ static int tas571x_set_bias_level(struct snd_soc_codec *codec, return ret; } } - - gpiod_set_value(priv->pdn_gpio, 0); - usleep_range(5000, 6000); - - regcache_cache_only(priv->regmap, false); - ret = regcache_sync(priv->regmap); - if (ret) - return ret; } break; case SND_SOC_BIAS_OFF: - regcache_cache_only(priv->regmap, true); - gpiod_set_value(priv->pdn_gpio, 1); - if (!IS_ERR(priv->mclk)) clk_disable_unprepare(priv->mclk); break; @@ -771,9 +760,6 @@ static int tas571x_i2c_probe(struct i2c_client *client, return ret; }
- regcache_cache_only(priv->regmap, true); - gpiod_set_value(priv->pdn_gpio, 1); - return snd_soc_register_codec(&client->dev, &priv->codec_driver, &tas571x_dai, 1); }
Wait extra 50ms after writing the oscillator trim register in probe(), as recommended by the TAS5721 and TAS5711 datasheets.
Signed-off-by: Petr Kulhavy brain@jikos.cz --- sound/soc/codecs/tas571x.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/soc/codecs/tas571x.c b/sound/soc/codecs/tas571x.c index a8a3279..f3252af 100644 --- a/sound/soc/codecs/tas571x.c +++ b/sound/soc/codecs/tas571x.c @@ -744,6 +744,7 @@ static int tas571x_i2c_probe(struct i2c_client *client, if (ret) return ret;
+ usleep_range(50000, 60000);
memcpy(&priv->codec_driver, &tas571x_codec, sizeof(priv->codec_driver)); priv->codec_driver.component_driver.controls = priv->chip->controls;
The patch
ASoC: tas571x: wait 50ms after oscillator trim
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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
From 5e549a0c18fc0321c18e7b6842cb7afc90aab396 Mon Sep 17 00:00:00 2001
From: Petr Kulhavy brain@jikos.cz Date: Mon, 3 Oct 2016 16:40:49 +0200 Subject: [PATCH] ASoC: tas571x: wait 50ms after oscillator trim
Wait extra 50ms after writing the oscillator trim register in probe(), as recommended by the TAS5721 and TAS5711 datasheets.
Signed-off-by: Petr Kulhavy brain@jikos.cz Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/tas571x.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/soc/codecs/tas571x.c b/sound/soc/codecs/tas571x.c index df5e5cb33baa..de65ecf534af 100644 --- a/sound/soc/codecs/tas571x.c +++ b/sound/soc/codecs/tas571x.c @@ -754,6 +754,7 @@ static int tas571x_i2c_probe(struct i2c_client *client, if (ret) return ret;
+ usleep_range(50000, 60000);
memcpy(&priv->codec_driver, &tas571x_codec, sizeof(priv->codec_driver)); priv->codec_driver.component_driver.controls = priv->chip->controls;
Channel 1 and 2 Mixer Volume controls (registers 0x72/0x73 and 0x76/0x77) were wrongly assigned to tas5711_controls in commit f252d2346022 ("ASoC: tas571x: add input channel mixer for TAS5717/19")
Therefore move them to tas5717_controls.
Signed-off-by: Petr Kulhavy brain@jikos.cz --- sound/soc/codecs/tas571x.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/sound/soc/codecs/tas571x.c b/sound/soc/codecs/tas571x.c index f3252af..71790a1 100644 --- a/sound/soc/codecs/tas571x.c +++ b/sound/soc/codecs/tas571x.c @@ -391,16 +391,6 @@ static const struct snd_kcontrol_new tas5711_controls[] = { TAS571X_SOFT_MUTE_REG, TAS571X_SOFT_MUTE_CH1_SHIFT, TAS571X_SOFT_MUTE_CH2_SHIFT, 1, 1), - - SOC_DOUBLE_R_RANGE("CH1 Mixer Volume", - TAS5717_CH1_LEFT_CH_MIX_REG, - TAS5717_CH1_RIGHT_CH_MIX_REG, - 16, 0, 0x80, 0), - - SOC_DOUBLE_R_RANGE("CH2 Mixer Volume", - TAS5717_CH2_LEFT_CH_MIX_REG, - TAS5717_CH2_RIGHT_CH_MIX_REG, - 16, 0, 0x80, 0), };
static const struct regmap_range tas571x_readonly_regs_range[] = { @@ -478,6 +468,16 @@ static const struct snd_kcontrol_new tas5717_controls[] = { TAS571X_SOFT_MUTE_CH1_SHIFT, TAS571X_SOFT_MUTE_CH2_SHIFT, 1, 1),
+ SOC_DOUBLE_R_RANGE("CH1 Mixer Volume", + TAS5717_CH1_LEFT_CH_MIX_REG, + TAS5717_CH1_RIGHT_CH_MIX_REG, + 16, 0, 0x80, 0), + + SOC_DOUBLE_R_RANGE("CH2 Mixer Volume", + TAS5717_CH2_LEFT_CH_MIX_REG, + TAS5717_CH2_RIGHT_CH_MIX_REG, + 16, 0, 0x80, 0), + /* * The biquads are named according to the register names. * Please note that TI's TAS57xx Graphical Development Environment
The patch
ASoC: tas571x: move mixer volume controls from TAS5711 to TAS5717
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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
From 960695670502afa153c2294e15ca790dbe086f4d Mon Sep 17 00:00:00 2001
From: Petr Kulhavy brain@jikos.cz Date: Mon, 3 Oct 2016 16:40:50 +0200 Subject: [PATCH] ASoC: tas571x: move mixer volume controls from TAS5711 to TAS5717
Channel 1 and 2 Mixer Volume controls (registers 0x72/0x73 and 0x76/0x77) were wrongly assigned to tas5711_controls in commit f252d2346022 ("ASoC: tas571x: add input channel mixer for TAS5717/19")
Therefore move them to tas5717_controls.
Signed-off-by: Petr Kulhavy brain@jikos.cz Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/tas571x.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/sound/soc/codecs/tas571x.c b/sound/soc/codecs/tas571x.c index de65ecf534af..34e4ce6f23d8 100644 --- a/sound/soc/codecs/tas571x.c +++ b/sound/soc/codecs/tas571x.c @@ -401,16 +401,6 @@ static const struct snd_kcontrol_new tas5711_controls[] = { TAS571X_SOFT_MUTE_REG, TAS571X_SOFT_MUTE_CH1_SHIFT, TAS571X_SOFT_MUTE_CH2_SHIFT, 1, 1), - - SOC_DOUBLE_R_RANGE("CH1 Mixer Volume", - TAS5717_CH1_LEFT_CH_MIX_REG, - TAS5717_CH1_RIGHT_CH_MIX_REG, - 16, 0, 0x80, 0), - - SOC_DOUBLE_R_RANGE("CH2 Mixer Volume", - TAS5717_CH2_LEFT_CH_MIX_REG, - TAS5717_CH2_RIGHT_CH_MIX_REG, - 16, 0, 0x80, 0), };
static const struct regmap_range tas571x_readonly_regs_range[] = { @@ -488,6 +478,16 @@ static const struct snd_kcontrol_new tas5717_controls[] = { TAS571X_SOFT_MUTE_CH1_SHIFT, TAS571X_SOFT_MUTE_CH2_SHIFT, 1, 1),
+ SOC_DOUBLE_R_RANGE("CH1 Mixer Volume", + TAS5717_CH1_LEFT_CH_MIX_REG, + TAS5717_CH1_RIGHT_CH_MIX_REG, + 16, 0, 0x80, 0), + + SOC_DOUBLE_R_RANGE("CH2 Mixer Volume", + TAS5717_CH2_LEFT_CH_MIX_REG, + TAS5717_CH2_RIGHT_CH_MIX_REG, + 16, 0, 0x80, 0), + /* * The biquads are named according to the register names. * Please note that TI's TAS57xx Graphical Development Environment
participants (3)
-
Lars-Peter Clausen -
Mark Brown -
Petr Kulhavy