* [PATCH] ASoC: snd_soc_tas5086: Reinit register values on probe
@ 2015-03-25 15:29 Pascal Huerst
2015-03-25 16:20 ` Mark Brown
0 siblings, 1 reply; 5+ messages in thread
From: Pascal Huerst @ 2015-03-25 15:29 UTC (permalink / raw
To: zonque; +Cc: alsa-devel, lars, tiwai, lgirdwood, broonie, ckeepax,
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.
Signed-off-by: Pascal Huerst <pascal.huerst@gmail.com>
---
sound/soc/codecs/tas5086.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/sound/soc/codecs/tas5086.c b/sound/soc/codecs/tas5086.c
index 249ef5c..cd19171 100644
--- a/sound/soc/codecs/tas5086.c
+++ b/sound/soc/codecs/tas5086.c
@@ -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)
--
1.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ASoC: snd_soc_tas5086: Reinit register values on probe
2015-03-25 15:29 [PATCH] ASoC: snd_soc_tas5086: Reinit register values on probe Pascal Huerst
@ 2015-03-25 16:20 ` Mark Brown
2015-03-25 17:29 ` Pascal Huerst
0 siblings, 1 reply; 5+ messages in thread
From: Mark Brown @ 2015-03-25 16:20 UTC (permalink / raw
To: Pascal Huerst; +Cc: alsa-devel, lars, tiwai, lgirdwood, zonque, ckeepax
[-- Attachment #1.1: Type: text/plain, Size: 1038 bytes --]
On Wed, Mar 25, 2015 at 04:29:39PM +0100, Pascal Huerst wrote:
> 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...
> tas5086_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.
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ASoC: snd_soc_tas5086: Reinit register values on probe
2015-03-25 16:20 ` Mark Brown
@ 2015-03-25 17:29 ` Pascal Huerst
2015-03-25 17:48 ` Mark Brown
0 siblings, 1 reply; 5+ messages in thread
From: Pascal Huerst @ 2015-03-25 17:29 UTC (permalink / raw
To: Mark Brown; +Cc: alsa-devel, lars, tiwai, lgirdwood, zonque, ckeepax
On 25.03.2015 17:20, Mark Brown wrote:
> On Wed, Mar 25, 2015 at 04:29:39PM +0100, Pascal Huerst wrote:
>
>> 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.
> I do think a version of this is useful regardless of the above...
>
>> tas5086_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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ASoC: snd_soc_tas5086: Reinit register values on probe
2015-03-25 17:29 ` Pascal Huerst
@ 2015-03-25 17:48 ` Mark Brown
2015-03-26 12:17 ` Pascal Huerst
0 siblings, 1 reply; 5+ messages in thread
From: Mark Brown @ 2015-03-25 17:48 UTC (permalink / raw
To: Pascal Huerst; +Cc: alsa-devel, lars, tiwai, lgirdwood, zonque, ckeepax
[-- Attachment #1.1: Type: text/plain, Size: 2303 bytes --]
On Wed, Mar 25, 2015 at 06:29:15PM +0100, Pascal Huerst wrote:
> On 25.03.2015 17:20, Mark Brown wrote:
> > On Wed, Mar 25, 2015 at 04:29:39PM +0100, Pascal Huerst wrote:
> >> 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.
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?
> > I do think a version of this is useful regardless of the above...
> >> tas5086_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.
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...
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ASoC: snd_soc_tas5086: Reinit register values on probe
2015-03-25 17:48 ` Mark Brown
@ 2015-03-26 12:17 ` Pascal Huerst
0 siblings, 0 replies; 5+ messages in thread
From: Pascal Huerst @ 2015-03-26 12:17 UTC (permalink / raw
To: Mark Brown; +Cc: alsa-devel, lars, tiwai, lgirdwood, zonque, ckeepax
On 25.03.2015 18:48, Mark Brown wrote:
> On Wed, Mar 25, 2015 at 06:29:15PM +0100, Pascal Huerst wrote:
>> On 25.03.2015 17:20, Mark Brown wrote:
>>> On Wed, Mar 25, 2015 at 04:29:39PM +0100, Pascal Huerst wrote:
>
>>>> 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.
>
> 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?
>
>>> I do think a version of this is useful regardless of the above...
>
>>>> tas5086_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.
>
> 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!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-03-26 12:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-25 15:29 [PATCH] ASoC: snd_soc_tas5086: Reinit register values on probe Pascal Huerst
2015-03-25 16:20 ` Mark Brown
2015-03-25 17:29 ` Pascal Huerst
2015-03-25 17:48 ` Mark Brown
2015-03-26 12:17 ` Pascal Huerst
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).