* [PATCH 1/2] ASoC: max98088: fix dai1/2_hw_params access
@ 2022-12-13 9:53 Rouven Czerwinski
2022-12-13 9:53 ` [PATCH 2/2] ASoC: max98088: fix initial dai mute state Rouven Czerwinski
2022-12-13 12:04 ` [PATCH 1/2] ASoC: max98088: fix dai1/2_hw_params access Mark Brown
0 siblings, 2 replies; 6+ messages in thread
From: Rouven Czerwinski @ 2022-12-13 9:53 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai
Cc: kernel, Marco Felsch, alsa-devel, linux-kernel
From: Marco Felsch <m.felsch@pengutronix.de>
According the reference manuals [1], [2] updating register 0x14/1c
should only be done after #shdn bit is set to 0.
[1] https://datasheets.maximintegrated.com/en/ds/MAX98089.pdf
[2] https://datasheets.maximintegrated.com/en/ds/MAX98088.pdf
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
sound/soc/codecs/max98088.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/soc/codecs/max98088.c b/sound/soc/codecs/max98088.c
index 405ec16be2b6..7f108e147355 100644
--- a/sound/soc/codecs/max98088.c
+++ b/sound/soc/codecs/max98088.c
@@ -977,6 +977,8 @@ static int max98088_dai1_hw_params(struct snd_pcm_substream *substream,
rate = params_rate(params);
+ snd_soc_component_update_bits(component, M98088_REG_51_PWR_SYS, M98088_SHDNRUN, 0);
+
switch (params_width(params)) {
case 16:
snd_soc_component_update_bits(component, M98088_REG_14_DAI1_FORMAT,
@@ -990,8 +992,6 @@ static int max98088_dai1_hw_params(struct snd_pcm_substream *substream,
return -EINVAL;
}
- snd_soc_component_update_bits(component, M98088_REG_51_PWR_SYS, M98088_SHDNRUN, 0);
-
if (rate_value(rate, ®val))
return -EINVAL;
@@ -1047,6 +1047,8 @@ static int max98088_dai2_hw_params(struct snd_pcm_substream *substream,
rate = params_rate(params);
+ snd_soc_component_update_bits(component, M98088_REG_51_PWR_SYS, M98088_SHDNRUN, 0);
+
switch (params_width(params)) {
case 16:
snd_soc_component_update_bits(component, M98088_REG_1C_DAI2_FORMAT,
@@ -1060,8 +1062,6 @@ static int max98088_dai2_hw_params(struct snd_pcm_substream *substream,
return -EINVAL;
}
- snd_soc_component_update_bits(component, M98088_REG_51_PWR_SYS, M98088_SHDNRUN, 0);
-
if (rate_value(rate, ®val))
return -EINVAL;
--
2.38.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] ASoC: max98088: fix initial dai mute state
2022-12-13 9:53 [PATCH 1/2] ASoC: max98088: fix dai1/2_hw_params access Rouven Czerwinski
@ 2022-12-13 9:53 ` Rouven Czerwinski
2022-12-13 12:01 ` Mark Brown
2022-12-13 12:04 ` [PATCH 1/2] ASoC: max98088: fix dai1/2_hw_params access Mark Brown
1 sibling, 1 reply; 6+ messages in thread
From: Rouven Czerwinski @ 2022-12-13 9:53 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai
Cc: kernel, Marco Felsch, alsa-devel, linux-kernel
From: Marco Felsch <m.felsch@pengutronix.de>
According the datasheets [1], [2] the initial value of register
0x2f/0x31 (dai1/dai2) is 0x00 which means that dai is unmuted. So upon
the first playback request the register is not touched since it is
cached by regmap. But the device output keeps silent. After ending the
playback session the mute() callback updates the register. Now the 2nd
playback request updates the register again (-> unmute the device) and
now we can really hear the output signal.
I've checked the register initial value which is '0x00' so the driver is
correct. Accroding the above inspections it seems that the hardware does
not update the register correctly on power up because the output is
muted.
To fix that we need to explicit set the mute state. Now the first
playback request gets played correctly.
[1] https://datasheets.maximintegrated.com/en/ds/MAX98089.pdf
[2] https://datasheets.maximintegrated.com/en/ds/MAX98088.pdf
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
sound/soc/codecs/max98088.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/sound/soc/codecs/max98088.c b/sound/soc/codecs/max98088.c
index 7f108e147355..c00d7726ac04 100644
--- a/sound/soc/codecs/max98088.c
+++ b/sound/soc/codecs/max98088.c
@@ -1710,6 +1710,11 @@ static int max98088_probe(struct snd_soc_component *component)
snd_soc_component_write(component, M98088_REG_1E_DAI2_IOCFG,
M98088_S2NORMAL|M98088_SDATA);
+ snd_soc_component_update_bits(component, M98088_REG_2F_LVL_DAI1_PLAY,
+ M98088_DAI_MUTE_MASK, M98088_DAI_MUTE);
+ snd_soc_component_update_bits(component, M98088_REG_31_LVL_DAI2_PLAY,
+ M98088_DAI_MUTE_MASK, M98088_DAI_MUTE);
+
max98088_handle_pdata(component);
err_access:
--
2.38.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] ASoC: max98088: fix initial dai mute state
2022-12-13 9:53 ` [PATCH 2/2] ASoC: max98088: fix initial dai mute state Rouven Czerwinski
@ 2022-12-13 12:01 ` Mark Brown
2022-12-15 9:17 ` Marco Felsch
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2022-12-13 12:01 UTC (permalink / raw)
To: Rouven Czerwinski
Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, kernel,
Marco Felsch, alsa-devel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 953 bytes --]
On Tue, Dec 13, 2022 at 10:53:28AM +0100, Rouven Czerwinski wrote:
> To fix that we need to explicit set the mute state. Now the first
> playback request gets played correctly.
> +++ b/sound/soc/codecs/max98088.c
> @@ -1710,6 +1710,11 @@ static int max98088_probe(struct snd_soc_component *component)
> snd_soc_component_write(component, M98088_REG_1E_DAI2_IOCFG,
> M98088_S2NORMAL|M98088_SDATA);
>
> + snd_soc_component_update_bits(component, M98088_REG_2F_LVL_DAI1_PLAY,
> + M98088_DAI_MUTE_MASK, M98088_DAI_MUTE);
> + snd_soc_component_update_bits(component, M98088_REG_31_LVL_DAI2_PLAY,
> + M98088_DAI_MUTE_MASK, M98088_DAI_MUTE);
> +
Won't this be broken again after suspend? The device gets powered off
over suspend, then when it powers on again with the output unmuted
nothing will do another write since the register is already in the state
in the cache.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] ASoC: max98088: fix initial dai mute state
2022-12-13 12:01 ` Mark Brown
@ 2022-12-15 9:17 ` Marco Felsch
2022-12-15 12:00 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Marco Felsch @ 2022-12-15 9:17 UTC (permalink / raw)
To: Mark Brown
Cc: Rouven Czerwinski, Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
kernel, alsa-devel, linux-kernel
Hi Mark,
On 22-12-13, Mark Brown wrote:
> On Tue, Dec 13, 2022 at 10:53:28AM +0100, Rouven Czerwinski wrote:
>
> > To fix that we need to explicit set the mute state. Now the first
> > playback request gets played correctly.
>
> > +++ b/sound/soc/codecs/max98088.c
> > @@ -1710,6 +1710,11 @@ static int max98088_probe(struct snd_soc_component *component)
> > snd_soc_component_write(component, M98088_REG_1E_DAI2_IOCFG,
> > M98088_S2NORMAL|M98088_SDATA);
> >
> > + snd_soc_component_update_bits(component, M98088_REG_2F_LVL_DAI1_PLAY,
> > + M98088_DAI_MUTE_MASK, M98088_DAI_MUTE);
> > + snd_soc_component_update_bits(component, M98088_REG_31_LVL_DAI2_PLAY,
> > + M98088_DAI_MUTE_MASK, M98088_DAI_MUTE);
> > +
>
> Won't this be broken again after suspend? The device gets powered off
> over suspend, then when it powers on again with the output unmuted
> nothing will do another write since the register is already in the state
> in the cache.
I didn't found any suspend logic within the driver. Is this handled
within the ASoC core?
Regards,
Marco
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] ASoC: max98088: fix initial dai mute state
2022-12-15 9:17 ` Marco Felsch
@ 2022-12-15 12:00 ` Mark Brown
0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2022-12-15 12:00 UTC (permalink / raw)
To: Marco Felsch
Cc: Rouven Czerwinski, Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
kernel, alsa-devel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 785 bytes --]
On Thu, Dec 15, 2022 at 10:17:47AM +0100, Marco Felsch wrote:
> On 22-12-13, Mark Brown wrote:
> > > + snd_soc_component_update_bits(component, M98088_REG_2F_LVL_DAI1_PLAY,
> > > + M98088_DAI_MUTE_MASK, M98088_DAI_MUTE);
> > > + snd_soc_component_update_bits(component, M98088_REG_31_LVL_DAI2_PLAY,
> > > + M98088_DAI_MUTE_MASK, M98088_DAI_MUTE);
> > > +
> > Won't this be broken again after suspend? The device gets powered off
> > over suspend, then when it powers on again with the output unmuted
> > nothing will do another write since the register is already in the state
> > in the cache.
> I didn't found any suspend logic within the driver. Is this handled
> within the ASoC core?
Register save and restore for the device won't be.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] ASoC: max98088: fix dai1/2_hw_params access
2022-12-13 9:53 [PATCH 1/2] ASoC: max98088: fix dai1/2_hw_params access Rouven Czerwinski
2022-12-13 9:53 ` [PATCH 2/2] ASoC: max98088: fix initial dai mute state Rouven Czerwinski
@ 2022-12-13 12:04 ` Mark Brown
1 sibling, 0 replies; 6+ messages in thread
From: Mark Brown @ 2022-12-13 12:04 UTC (permalink / raw)
To: Rouven Czerwinski
Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, kernel,
Marco Felsch, alsa-devel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 679 bytes --]
On Tue, Dec 13, 2022 at 10:53:27AM +0100, Rouven Czerwinski wrote:
> From: Marco Felsch <m.felsch@pengutronix.de>
>
> According the reference manuals [1], [2] updating register 0x14/1c
> should only be done after #shdn bit is set to 0.
>
> [1] https://datasheets.maximintegrated.com/en/ds/MAX98089.pdf
> [2] https://datasheets.maximintegrated.com/en/ds/MAX98088.pdf
>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
> sound/soc/codecs/max98088.c | 8 ++++----
You've not provided a Signed-off-by for this so I can't do anything with
it, please see Documentation/process/submitting-patches.rst for details
on what this is and why it's important.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-12-15 12:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-13 9:53 [PATCH 1/2] ASoC: max98088: fix dai1/2_hw_params access Rouven Czerwinski
2022-12-13 9:53 ` [PATCH 2/2] ASoC: max98088: fix initial dai mute state Rouven Czerwinski
2022-12-13 12:01 ` Mark Brown
2022-12-15 9:17 ` Marco Felsch
2022-12-15 12:00 ` Mark Brown
2022-12-13 12:04 ` [PATCH 1/2] ASoC: max98088: fix dai1/2_hw_params access Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox