mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
To: Alexey Klimov <alexey.klimov@linaro.org>,
	Srinivas Kandagatla <srini@kernel.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Stephen Boyd <sboyd@kernel.org>
Cc: Lee Jones <lee@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>,
	linux-arm-msm@vger.kernel.org, linux-sound@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Subject: Re: [PATCH v2 2/3] ASoC: codecs: add new pm4125 audio codec driver
Date: Fri, 11 Jul 2025 14:32:36 +0100	[thread overview]
Message-ID: <87a56d1e-2bbd-4abf-91c8-5129835d8d87@oss.qualcomm.com> (raw)
In-Reply-To: <20250711-pm4125_audio_codec_v1-v2-2-13e6f835677a@linaro.org>

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@oss.qualcomm.com>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.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 ]---
> +		break;
> +	case SND_SOC_DAPM_PRE_PMD:
> +		disable_irq_nosync(pm4125->hphl_pdm_wd_int);
> +		break;
> +	case SND_SOC_DAPM_POST_PMD:
> +		usleep_range(2000, 2010);
> +		snd_soc_component_write_field(component, PM4125_ANA_COMBOPA_CTL,
> +					      PM4125_ANA_COMBO_PA_SELECT_MASK,
> +					      PM4125_ANA_COMBO_PA_SELECT_EAR);
> +		usleep_range(5000, 5100);
> +		snd_soc_component_write_field(component, PM4125_DIG_SWR_PDM_WD_CTL0,
> +					      PM4125_WDT_ENABLE_MASK, 0x00);
> +		break;
> +	}
> +
> +	return 0;
> +}
> +thanks,
Srini

  parent reply	other threads:[~2025-07-11 13:32 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-11  3:00 [PATCH v2 0/3] Add PM4125 " Alexey Klimov
2025-07-11  3:00 ` [PATCH v2 1/3] dt-bindings: sound: add bindings for pm4125 audio codec Alexey Klimov
2025-07-11  8:27   ` Krzysztof Kozlowski
2025-07-18 13:43   ` Lee Jones
2025-07-24 15:17     ` Alexey Klimov
2025-07-24 15:48       ` Krzysztof Kozlowski
2025-08-13 22:28         ` Alexey Klimov
2025-07-11  3:00 ` [PATCH v2 2/3] ASoC: codecs: add new pm4125 audio codec driver Alexey Klimov
2025-07-11  8:29   ` Krzysztof Kozlowski
2025-07-24 15:41     ` Alexey Klimov
2025-08-13 23:08     ` Alexey Klimov
2025-07-11 13:32   ` Srinivas Kandagatla [this message]
2025-08-13 23:07     ` Alexey Klimov
2025-07-11 15:15   ` Christophe JAILLET
2025-08-13 23:04     ` Alexey Klimov
2025-07-11  3:00 ` [PATCH v2 3/3] MAINTAINERS: add Qualcomm PM4125 audio codec to drivers list Alexey Klimov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87a56d1e-2bbd-4abf-91c8-5129835d8d87@oss.qualcomm.com \
    --to=srinivas.kandagatla@oss.qualcomm.com \
    --cc=alexey.klimov@linaro.org \
    --cc=broonie@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.baryshkov@oss.qualcomm.com \
    --cc=krzk+dt@kernel.org \
    --cc=lee@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=perex@perex.cz \
    --cc=robh@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=srini@kernel.org \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®