[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <DC1OE9UIUPON.2DOURP7NEQS54@linaro.org>
Date: Thu, 14 Aug 2025 00:07:16 +0100
From: "Alexey Klimov" <alexey.klimov@...aro.org>
To: "Srinivas Kandagatla" <srinivas.kandagatla@....qualcomm.com>, "Srinivas
Kandagatla" <srini@...nel.org>, "Liam Girdwood" <lgirdwood@...il.com>,
"Mark Brown" <broonie@...nel.org>, "Rob Herring" <robh@...nel.org>,
"Krzysztof Kozlowski" <krzk+dt@...nel.org>, "Conor Dooley"
<conor+dt@...nel.org>, "Stephen Boyd" <sboyd@...nel.org>
Cc: "Lee Jones" <lee@...nel.org>, "Jaroslav Kysela" <perex@...ex.cz>,
"Takashi Iwai" <tiwai@...e.com>, <linux-arm-msm@...r.kernel.org>,
<linux-sound@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, "Dmitry Baryshkov"
<dmitry.baryshkov@....qualcomm.com>
Subject: Re: [PATCH v2 2/3] ASoC: codecs: add new pm4125 audio codec driver
On Fri Jul 11, 2025 at 2:32 PM BST, Srinivas Kandagatla wrote:
> On 7/11/25 4:00 AM, Alexey Klimov wrote:
>> The audio codec is found in Qualcomm PM2250/PM4125 PMICs and is used on
>> platforms like Qualcomm QCM2290. It has soundwire interface and
>> corresponding RX and TX slave devices.
>>
>> It has only two input channels: HPH left and right. The line output (LO)
>> is linked to HPHL so the hardware has some limitations regarding concurrent
>> playback via HPH and LO for instance.
>>
>> The codec driver also uses WCD MBCH framework. The MBHC functionality is
>> implemented in a minimalistic way to enable IRQs and avoid different
>> issues with IRQs.
>>
>> Co-developed-by: Srinivas Kandagatla <srinivas.kandagatla@....qualcomm.com>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@....qualcomm.com>
>> Signed-off-by: Alexey Klimov <alexey.klimov@...aro.org>
>
> Thanks for doing this Alexey,
>
> I have tested Headset playback, headset recording and Lineout.
>
> While testing I have noticed one issue with the places where enable_irq
> is being used, there is a possiblity of things going wrong in some cases.
>
>
>> ---
>> sound/soc/codecs/Kconfig | 18 +
>> sound/soc/codecs/Makefile | 8 +
>> sound/soc/codecs/pm4125-sdw.c | 546 +++++++++++++
>> sound/soc/codecs/pm4125.c | 1767 +++++++++++++++++++++++++++++++++++++++++
>> sound/soc/codecs/pm4125.h | 314 ++++++++
>> 5 files changed, 2653 insertions(+)
>>
>
> ...
>> +static int pm4125_codec_enable_lo_pa(struct snd_soc_dapm_widget *w,
>> + struct snd_kcontrol *kcontrol, int event)
>> +{
>> + struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
>> + struct pm4125_priv *pm4125 = snd_soc_component_get_drvdata(component);
>> +
>> + switch (event) {
>> + case SND_SOC_DAPM_PRE_PMU:
>> + snd_soc_component_update_bits(component, PM4125_ANA_COMBOPA_CTL_5, 0x04, 0x00);
>> + usleep_range(1000, 1010);
>> + snd_soc_component_update_bits(component, PM4125_ANA_COMBOPA_CTL_4, 0x0F, 0x0F);
>> + usleep_range(1000, 1010);
>> + snd_soc_component_write_field(component, PM4125_ANA_COMBOPA_CTL,
>> + PM4125_ANA_COMBO_PA_SELECT_MASK,
>> + PM4125_ANA_COMBO_PA_SELECT_LO);
>> + snd_soc_component_write_field(component, PM4125_DIG_SWR_PDM_WD_CTL0,
>> + PM4125_WDT_ENABLE_MASK,
>> + (PM4125_WDT_ENABLE_RX0_M | PM4125_WDT_ENABLE_RX0_L));
>> + break;
>> + case SND_SOC_DAPM_POST_PMU:
>> + usleep_range(5000, 5010);
>> + snd_soc_component_update_bits(component, PM4125_ANA_COMBOPA_CTL_4, 0x0F, 0x04);
>> + enable_irq(pm4125->hphl_pdm_wd_int);
>
> if headset playback dapm widgets are powered up or active and you try to
> play on lineout you would get below splat:
>
> [ 1181.352762] ------------[ cut here ]------------
> [ 1181.357435] Unbalanced enable for IRQ 156
> [ 1181.361489] WARNING: CPU: 1 PID: 877 at kernel/irq/manage.c:753
> __enable_irq+0x4c/0x7c
> ...
>
> [ 1181.483541] pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS
> BTYPE=--)
> [ 1181.490534] pc : __enable_irq+0x4c/0x7c
> [ 1181.494405] lr : __enable_irq+0x4c/0x7c
> [ 1181.498278] sp : ffff8000840ab8a0
> [ 1181.501618] x29: ffff8000840ab8a0 x28: ffffd79b9725d588 x27:
> ffff8000840ab948
> [ 1181.508796] x26: 000000000000309b x25: 0000000000000080 x24:
> 0000000000000080
> [ 1181.515980] x23: ffffd79b9698f250 x22: 0000000000000000 x21:
> 0000000000000000
> [ 1181.523153] x20: 000000000000009c x19: ffff23c347fd9400 x18:
> 0000000000000006
> [ 1181.530335] x17: 0000000000000001 x16: ffffd79b94dbcce8 x15:
> ffff8000840ab46f
> [ 1181.537508] x14: 0000000000000000 x13: 3635312051524920 x12:
> ffffd79b96d96768
> [ 1181.544691] x11: 0000000000000058 x10: 0000000000000018 x9 :
> ffffd79b96d96768
> [ 1181.551862] x8 : 000000000000024f x7 : ffffd79b96dee768 x6 :
> ffffd79b96dee768
> [ 1181.559044] x5 : ffff23c3bbbb8508 x4 : 0000000000000000 x3 :
> 0000000000000027
> [ 1181.566217] x2 : 0000000000000000 x1 : 0000000000000000 x0 :
> ffff23c35a0a4600
> [ 1181.573392] Call trace:
> [ 1181.575871] __enable_irq+0x4c/0x7c (P)
> [ 1181.579748] enable_irq+0x60/0xd8
> [ 1181.583098] pm4125_codec_enable_lo_pa+0x6c/0x1a0 [snd_soc_pm4125]
> [ 1181.589319] dapm_seq_check_event+0x11c/0x144
> [ 1181.593720] dapm_seq_run_coalesced+0x130/0x1d0
> [ 1181.598286] dapm_seq_run+0x264/0x374
> [ 1181.601991] dapm_power_widgets+0x664/0x900
> [ 1181.606212] snd_soc_dapm_stream_event+0xf4/0x178
> [ 1181.610957] __soc_pcm_prepare+0x64/0x120
> [ 1181.614998] dpcm_be_dai_prepare+0x120/0x168
> [ 1181.619299] dpcm_fe_dai_prepare+0x94/0x178
> [ 1181.623517] snd_pcm_do_prepare+0x30/0x50
> [ 1181.627567] snd_pcm_action_single+0x48/0xa4
> [ 1181.631872] snd_pcm_action_nonatomic+0x94/0xb0
> [ 1181.636435] snd_pcm_prepare+0x7c/0xe0
> [ 1181.640222] snd_pcm_common_ioctl+0xca4/0x16e0
> [ 1181.644705] snd_pcm_ioctl+0x34/0x4c
> [ 1181.648315] __arm64_sys_ioctl+0xac/0x104
> [ 1181.652355] invoke_syscall+0x48/0x104
> [ 1181.656147] el0_svc_common.constprop.0+0x40/0xe0
> [ 1181.660892] do_el0_svc+0x1c/0x28
> [ 1181.664242] el0_svc+0x30/0xcc
> [ 1181.667340] el0t_64_sync_handler+0x10c/0x138
> [ 1181.671737] el0t_64_sync+0x198/0x19c
> [ 1181.675430] ---[ end trace 0000000000000000 ]---
Yep, I noticed that one time only but was puzzled how to reproduce.
As you suggested during out of email list discussion I will move handling
of IRQs into separate widget.
Thank you,
Alexey
Powered by blists - more mailing lists