diff mbox

ASoC: wm8962: Correct the bit offset to enable mono speaker output

Message ID 1407412549-14983-1-git-send-email-nicoleotsuka@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Nicolin Chen Aug. 7, 2014, 11:55 a.m. UTC
As WM8962 datasheet describes for SPK_MONO bit of R51: When SPK_MONO
is set to '1', both speakers output the signal from the left channel.

So for mono speaker widget, we shall enable Left Channel whose enable
bit is 6 instead of 7 (Right Channel).

This patches just simply corrects the bit offset.

Signed-off-by: Peter Chan <B18700@freescale.com>
Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
---

@Peter
I cced you just to let you know that I'm merging your fix to upstream.
And because you patch can't be applied to the upstream tree directly,
I used my own commit with your signed-off. Thank you for the fix.

 sound/soc/codecs/wm8962.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Mark Brown Aug. 7, 2014, 4:58 p.m. UTC | #1
On Thu, Aug 07, 2014 at 07:55:49PM +0800, Nicolin Chen wrote:
> As WM8962 datasheet describes for SPK_MONO bit of R51: When SPK_MONO
> is set to '1', both speakers output the signal from the left channel.

> So for mono speaker widget, we shall enable Left Channel whose enable
> bit is 6 instead of 7 (Right Channel).
> 
> This patches just simply corrects the bit offset.

>  		   spkmixl, ARRAY_SIZE(spkmixl)),
>  SND_SOC_DAPM_MUX_E("Speaker PGA", WM8962_PWR_MGMT_2, 4, 0, &spkoutl_mux,
>  		   out_pga_event, SND_SOC_DAPM_POST_PMU),
> -SND_SOC_DAPM_PGA("Speaker Output", WM8962_CLASS_D_CONTROL_1, 7, 0, NULL, 0),
> +SND_SOC_DAPM_PGA("Speaker Output", WM8962_CLASS_D_CONTROL_1, 6, 0, NULL, 0),
>  SND_SOC_DAPM_OUTPUT("SPKOUT"),
>  };

Someone who's name I'd need to look up submitted an identical fix off
list recently - I checked the datasheet and it seems that in mono mode
the speaker output actually wants both left and right channels active so
there is a bug here but this isn't the fix.
Nicolin Chen Aug. 8, 2014, 8:20 a.m. UTC | #2
On Thu, Aug 07, 2014 at 05:58:36PM +0100, Mark Brown wrote:
> On Thu, Aug 07, 2014 at 07:55:49PM +0800, Nicolin Chen wrote:
> > As WM8962 datasheet describes for SPK_MONO bit of R51: When SPK_MONO
> > is set to '1', both speakers output the signal from the left channel.
> 
> > So for mono speaker widget, we shall enable Left Channel whose enable
> > bit is 6 instead of 7 (Right Channel).
> > 
> > This patches just simply corrects the bit offset.
> 
> >  		   spkmixl, ARRAY_SIZE(spkmixl)),
> >  SND_SOC_DAPM_MUX_E("Speaker PGA", WM8962_PWR_MGMT_2, 4, 0, &spkoutl_mux,
> >  		   out_pga_event, SND_SOC_DAPM_POST_PMU),
> > -SND_SOC_DAPM_PGA("Speaker Output", WM8962_CLASS_D_CONTROL_1, 7, 0, NULL, 0),
> > +SND_SOC_DAPM_PGA("Speaker Output", WM8962_CLASS_D_CONTROL_1, 6, 0, NULL, 0),
> >  SND_SOC_DAPM_OUTPUT("SPKOUT"),
> >  };
> 
> Someone who's name I'd need to look up submitted an identical fix off
> list recently - I checked the datasheet and it seems that in mono mode
> the speaker output actually wants both left and right channels active so
> there is a bug here but this isn't the fix.

I rechecked the datasheet and found that the hardware needs to tie
both outputs and enable both bits. So the patch should be invalid.

But our customer reported that they did try to set both SPKOUTR_ENA
and SPKOUTL_ENA bits in mono mode but the sound is lousy. If setting
SPKOUTR_ENA alone, the speaker has no sound. Only setting SPKOUTL_ENA
bit get the correct speaker output in mono mode.

And I just confirmed with our engineer who supports the customer
that they are using the correct hardware design as the datasheet
suggests. So I'm wondering if there's any other part we're missing.

Thank you
Nicolin
Charles Keepax Aug. 11, 2014, 1:19 p.m. UTC | #3
On Fri, Aug 08, 2014 at 09:55:43AM +0000, Peter Chan wrote:
> I have a question about the wm8962 mono speaker mode. In SPK_MONO description at table 73, it says "When SPK_MONO is enabled, both speakers output the signal from the left channel". Does it mean only the left channel control the output in mono speaker mode?
> 
> Thanks,
> Peter
> 
> -----Original Message-----
> From: Nicolin Chen [mailto:Guangyu.Chen@freescale.com] 
> Sent: Friday, August 08, 2014 4:21 PM
> To: Mark Brown
> Cc: Nicolin Chen; alsa-devel@alsa-project.org; tiwai@suse.de; patches@opensource.wolfsonmicro.com; Wang Shengjiu-B02247; lgirdwood@gmail.com; linux-kernel@vger.kernel.org; CHAN Peter-B18700; ckeepax@opensource.wolfsonmicro.com; rf@opensource.wolfsonmicro.com
> Subject: Re: [alsa-devel] [PATCH] ASoC: wm8962: Correct the bit offset to enable mono speaker output
> 
> On Thu, Aug 07, 2014 at 05:58:36PM +0100, Mark Brown wrote:
> > On Thu, Aug 07, 2014 at 07:55:49PM +0800, Nicolin Chen wrote:
> > > As WM8962 datasheet describes for SPK_MONO bit of R51: When SPK_MONO 
> > > is set to '1', both speakers output the signal from the left channel.
> > 
> > > So for mono speaker widget, we shall enable Left Channel whose 
> > > enable bit is 6 instead of 7 (Right Channel).
> > > 
> > > This patches just simply corrects the bit offset.
> > 
> > >  		   spkmixl, ARRAY_SIZE(spkmixl)),  SND_SOC_DAPM_MUX_E("Speaker 
> > > PGA", WM8962_PWR_MGMT_2, 4, 0, &spkoutl_mux,
> > >  		   out_pga_event, SND_SOC_DAPM_POST_PMU), 
> > > -SND_SOC_DAPM_PGA("Speaker Output", WM8962_CLASS_D_CONTROL_1, 7, 0, 
> > > NULL, 0),
> > > +SND_SOC_DAPM_PGA("Speaker Output", WM8962_CLASS_D_CONTROL_1, 6, 0, 
> > > +NULL, 0),
> > >  SND_SOC_DAPM_OUTPUT("SPKOUT"),
> > >  };
> > 
> > Someone who's name I'd need to look up submitted an identical fix off 
> > list recently - I checked the datasheet and it seems that in mono mode 
> > the speaker output actually wants both left and right channels active 
> > so there is a bug here but this isn't the fix.
> 
> I rechecked the datasheet and found that the hardware needs to tie both outputs and enable both bits. So the patch should be invalid.
> 
> But our customer reported that they did try to set both SPKOUTR_ENA and SPKOUTL_ENA bits in mono mode but the sound is lousy. If setting SPKOUTR_ENA alone, the speaker has no sound. Only setting SPKOUTL_ENA bit get the correct speaker output in mono mode.

From the wording of "tie both outputs together" in the datasheet
I would assume that means both should be fed the same audio? Was
this done? I will try to give the hardware guys here a shout and
see if I can get some clarification on mono mode on this device.

Thanks,
Charles
Charles Keepax Aug. 12, 2014, 8:24 a.m. UTC | #4
On Mon, Aug 11, 2014 at 02:19:49PM +0100, Charles Keepax wrote:
> On Fri, Aug 08, 2014 at 09:55:43AM +0000, Peter Chan wrote:
> > 
> > But our customer reported that they did try to set both SPKOUTR_ENA and SPKOUTL_ENA bits in mono mode but the sound is lousy. If setting SPKOUTR_ENA alone, the speaker has no sound. Only setting SPKOUTL_ENA bit get the correct speaker output in mono mode.
> 
> From the wording of "tie both outputs together" in the datasheet
> I would assume that means both should be fed the same audio? Was
> this done? I will try to give the hardware guys here a shout and
> see if I can get some clarification on mono mode on this device.

I have had a word with the hardware team here and both channels
do need to be enabled but audio should only be fed to the left
channel. So the driver definitely needs updated for the mono
case, but I am unclear as to what is causing your poor audio
quality issue.

Thanks,
Charles
Mark Brown Aug. 12, 2014, 9:57 p.m. UTC | #5
On Tue, Aug 12, 2014 at 09:24:16AM +0100, Charles Keepax wrote:
> On Mon, Aug 11, 2014 at 02:19:49PM +0100, Charles Keepax wrote:

> > From the wording of "tie both outputs together" in the datasheet
> > I would assume that means both should be fed the same audio? Was
> > this done? I will try to give the hardware guys here a shout and
> > see if I can get some clarification on mono mode on this device.

> I have had a word with the hardware team here and both channels
> do need to be enabled but audio should only be fed to the left
> channel. So the driver definitely needs updated for the mono
> case, but I am unclear as to what is causing your poor audio
> quality issue.

Overdriving the speaker perhaps?
Peter Chan Aug. 13, 2014, 10:10 a.m. UTC | #6
I found that in mono speaker mode, only the left channel is enable. The SPKOUTR_PGA_ENA bit remains 0 in this case. Thus when SPKOUTR_ENA is set, poor sound quality results.

-----Original Message-----
From: Mark Brown [mailto:broonie@kernel.org] 
Sent: Wednesday, August 13, 2014 5:58 AM
To: Charles Keepax
Cc: CHAN Peter-B18700; alsa-devel@alsa-project.org; tiwai@suse.de; linux-kernel@vger.kernel.org; patches@opensource.wolfsonmicro.com; Wang Shengjiu-B02247; lgirdwood@gmail.com; Nicolin Chen; Chen Guangyu-B42378; rf@opensource.wolfsonmicro.com
Subject: Re: [alsa-devel] [PATCH] ASoC: wm8962: Correct the bit offset to enable mono speaker output

On Tue, Aug 12, 2014 at 09:24:16AM +0100, Charles Keepax wrote:
> On Mon, Aug 11, 2014 at 02:19:49PM +0100, Charles Keepax wrote:

> > From the wording of "tie both outputs together" in the datasheet I 
> > would assume that means both should be fed the same audio? Was this 
> > done? I will try to give the hardware guys here a shout and see if I 
> > can get some clarification on mono mode on this device.

> I have had a word with the hardware team here and both channels do 
> need to be enabled but audio should only be fed to the left channel. 
> So the driver definitely needs updated for the mono case, but I am 
> unclear as to what is causing your poor audio quality issue.

Overdriving the speaker perhaps?
diff mbox

Patch

diff --git a/sound/soc/codecs/wm8962.c b/sound/soc/codecs/wm8962.c
index 1098ae3..105deec 100644
--- a/sound/soc/codecs/wm8962.c
+++ b/sound/soc/codecs/wm8962.c
@@ -2185,7 +2185,7 @@  SND_SOC_DAPM_MIXER("Speaker Mixer", WM8962_MIXER_ENABLES, 1, 0,
 		   spkmixl, ARRAY_SIZE(spkmixl)),
 SND_SOC_DAPM_MUX_E("Speaker PGA", WM8962_PWR_MGMT_2, 4, 0, &spkoutl_mux,
 		   out_pga_event, SND_SOC_DAPM_POST_PMU),
-SND_SOC_DAPM_PGA("Speaker Output", WM8962_CLASS_D_CONTROL_1, 7, 0, NULL, 0),
+SND_SOC_DAPM_PGA("Speaker Output", WM8962_CLASS_D_CONTROL_1, 6, 0, NULL, 0),
 SND_SOC_DAPM_OUTPUT("SPKOUT"),
 };