ASoC: snd_soc_tas5086: Reinit register values on probe
PATCH 1/1 ASoC: snd_soc_tas5086: Reinit register values on probe
If the machine driver has been un/reloaded, the register values of the codec driver have to be reinitialized in order to run properly.
--- sound/soc/codecs/tas5086.c | 6 ++++++ 1 file changed, 6 insertions(+)
sound/soc/codecs/tas5086.c +6 -0
@@ -851,10 +851,16 @@ static int tas5086_probe(struct snd_soc_codec *codec) }tas5086_reset (priv); +regcache_mark_dirty (priv->regmap); + ret =tas5086_init (codec->dev, priv);if (ret <0 ) goto exit_disable_regulators; + ret =regcache_sync (priv->regmap); +if (ret <0 ) + goto exit_disable_regulators; +/* set master volume to 0 dB */ ret =regmap_write (priv->regmap,TAS5086_MASTER_VOL ,0x30 );if (ret <0 )
Signature · 1 line
1.9.3
Mark Brown
maintainerQuoted 2 lines · Pascal Huerst
If the machine driver has been un/reloaded, the register values of the codec driver have to be reinitialized in order to run properly.
Hrm. This isn't something that I'd expect to be required - I'd expect that as part of the machine driver teardown to put the hardware into a reasonable default state so that things come back as normal. Can you be a bit more specific about the problem that you are seeing? We probably shouldn't need the existing reset that's in the ASoC level probe either.
I do think a version of this is useful regardless of the above...
sound/soc/codecs/tas5086.c static int tas5086_probe(struct snd_soc_codec *codec) · line 854
Quoted 2 lines
sound/soc/codecs/tas5086.c +1
-0tas5086_reset (priv); +regcache_mark_dirty (priv->regmap);
Since the device has hardware reset support we really ought to be able to do the register cache resync only if the reset GPIO is missing. How about putting your code into the reset function and doing it in the case where the reset GPIO is missing? That way anything that thinks it's resetting the device will get the benefit of your code.
Quoted 10 lines · Mark Brown
Quoted 2 lines · Pascal Huerst
If the machine driver has been un/reloaded, the register values of the codec driver have to be reinitialized in order to run properly.
Hrm. This isn't something that I'd expect to be required - I'd expect that as part of the machine driver teardown to put the hardware into a reasonable default state so that things come back as normal. Can you be a bit more specific about the problem that you are seeing? We probably shouldn't need the existing reset that's in the ASoC level probe either.
The symptom is, that if I rmmod the machine driver and then modprobe it again. The codec does not play audio at all. I can call aplay without any problems, but there is no output. My guess was that I have to rewrite the default values after a reset. May be regmap_reinit_cache() is the better choice to reinit the register values? Not sure about that.
Quoted 10 lines
I do think a version of this is useful regardless of the above...
sound/soc/codecs/tas5086.c static int tas5086_probe(struct snd_soc_codec *codec) · line 854
Quoted 2 lines
sound/soc/codecs/tas5086.c +1
-0tas5086_reset (priv); +regcache_mark_dirty (priv->regmap);Since the device has hardware reset support we really ought to be able to do the register cache resync only if the reset GPIO is missing. How about putting your code into the reset function and doing it in the case where the reset GPIO is missing? That way anything that thinks it's resetting the device will get the benefit of your code.
I thing this would not fix the issue. I do have a hardware reset. In that case my code would not be called at all and I would face the same issue (no audio output) again.
Mark Brown
maintainerQuoted 12 lines · Pascal Huerst
Quoted 10 lines · Mark Brown
Quoted 2 lines · Pascal Huerst
If the machine driver has been un/reloaded, the register values of the codec driver have to be reinitialized in order to run properly.
Hrm. This isn't something that I'd expect to be required - I'd expect that as part of the machine driver teardown to put the hardware into a reasonable default state so that things come back as normal. Can you be a bit more specific about the problem that you are seeing? We probably shouldn't need the existing reset that's in the ASoC level probe either.
Quoted 5 lines
The symptom is, that if I rmmod the machine driver and then modprobe it again. The codec does not play audio at all. I can call aplay without any problems, but there is no output. My guess was that I have to rewrite the default values after a reset. May be
regmap_reinit_cache()is the better choice to reinit the register values? Not sure about that.
That's not really answering my question at a low enough level - what exactly is changed in the device as a result of rewriting all the registers?
Quoted 11 lines
Quoted 10 lines
I do think a version of this is useful regardless of the above...
sound/soc/codecs/tas5086.c static int tas5086_probe(struct snd_soc_codec *codec) · line 854
Quoted 2 lines
sound/soc/codecs/tas5086.c +1
-0tas5086_reset (priv); +regcache_mark_dirty (priv->regmap);Since the device has hardware reset support we really ought to be able to do the register cache resync only if the reset GPIO is missing. How about putting your code into the reset function and doing it in the case where the reset GPIO is missing? That way anything that thinks it's resetting the device will get the benefit of your code.
Quoted 3 lines
I thing this would not fix the issue. I do have a hardware reset. In that case my code would not be called at all and I would face the same issue (no audio output) again.
So you're saying that doing a hardware reset of the device isn't actually resetting the device? That suggests that either the hardware reset code isn't working for some reason or there's a bug somewhere else.
If the rewrite of all the registers is doing anything other than writing the values that the registers already have that suggests we're just randomly bashing on the device in the hope that it works...
Quoted 38 lines · Mark Brown
Quoted 12 lines · Pascal Huerst
Quoted 10 lines · Mark Brown
Quoted 2 lines · Pascal Huerst
If the machine driver has been un/reloaded, the register values of the codec driver have to be reinitialized in order to run properly.
Hrm. This isn't something that I'd expect to be required - I'd expect that as part of the machine driver teardown to put the hardware into a reasonable default state so that things come back as normal. Can you be a bit more specific about the problem that you are seeing? We probably shouldn't need the existing reset that's in the ASoC level probe either.
Quoted 5 lines
The symptom is, that if I rmmod the machine driver and then modprobe it again. The codec does not play audio at all. I can call aplay without any problems, but there is no output. My guess was that I have to rewrite the default values after a reset. May be
regmap_reinit_cache()is the better choice to reinit the register values? Not sure about that.That's not really answering my question at a low enough level - what exactly is changed in the device as a result of rewriting all the registers?
Quoted 11 lines
Quoted 10 lines
I do think a version of this is useful regardless of the above...
sound/soc/codecs/tas5086.c static int tas5086_probe(struct snd_soc_codec *codec) · line 854
Quoted 2 lines
sound/soc/codecs/tas5086.c +1
-0tas5086_reset (priv); +regcache_mark_dirty (priv->regmap);Since the device has hardware reset support we really ought to be able to do the register cache resync only if the reset GPIO is missing. How about putting your code into the reset function and doing it in the case where the reset GPIO is missing? That way anything that thinks it's resetting the device will get the benefit of your code.
Quoted 3 lines
I thing this would not fix the issue. I do have a hardware reset. In that case my code would not be called at all and I would face the same issue (no audio output) again.
Quoted 8 lines
So you're saying that doing a hardware reset of the device isn't actually resetting the device? That suggests that either the hardware reset code isn't working for some reason or there's a bug somewhere else.
If the rewrite of all the registers is doing anything other than writing the values that the registers already have that suggests we're just randomly bashing on the device in the hope that it works...
Aargh... I have had this patch in use for quite some time and assumed it's working properly, but after some more testing now, I have to admit that the cause for my problem is something else... I does not even make a difference anymore, if applied or not... Sorry for bothering!