[PATCH] ASoC: cs42l43-jack: Remove manual pm_runtime get/put from tip_sense_work

[PATCH] ASoC: cs42l43-jack: Remove manual pm_runtime get/put from tip_sense_work

[Peter Ujfalusi]

When a jack is inserted the forced pm_runtime_get() will keep the codec,
soundwire bus and it's parent active as long as the jack is connected.
This makes for example the DSP and firmware booted up on Intel platforms.

If the module is removed while the jack is connected we will also have
unbalanced runtime PM state.

Without the manual get/put, the button detection still works correctly and
the system can reach lower power state while the jack is connected like
in the case when there is no jack connected.

Fixes: fc918cbe874e ("ASoC: cs42l43: Add support for the cs42l43")
Cc: stable@vger.kernel.org
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 sound/soc/codecs/cs42l43-jack.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/sound/soc/codecs/cs42l43-jack.c b/sound/soc/codecs/cs42l43-jack.c
index 3e04e6897b14..d90a13a55845 100644
--- a/sound/soc/codecs/cs42l43-jack.c
+++ b/sound/soc/codecs/cs42l43-jack.c
@@ -756,10 +756,8 @@ void cs42l43_tip_sense_work(struct work_struct *work)
 	ring = (sts >> CS42L43_RINGSENSE_PLUG_DB_STS_SHIFT) & CS42L43_JACK_PRESENT;
 
 	if (tip == CS42L43_JACK_PRESENT) {
-		if (cs42l43->sdw && !priv->jack_present) {
+		if (cs42l43->sdw && !priv->jack_present)
 			priv->jack_present = true;
-			pm_runtime_get(priv->dev);
-		}
 
 		if (priv->use_ring_sense && ring == CS42L43_JACK_ABSENT) {
 			report = CS42L43_JACK_OPTICAL;
@@ -779,10 +777,8 @@ void cs42l43_tip_sense_work(struct work_struct *work)
 
 		snd_soc_jack_report(priv->jack_hp, 0, 0xFFFF);
 
-		if (cs42l43->sdw && priv->jack_present) {
-			pm_runtime_put(priv->dev);
+		if (cs42l43->sdw && priv->jack_present)
 			priv->jack_present = false;
-		}
 	}
 
 error:
-- 
2.53.0

Re: [PATCH] ASoC: cs42l43-jack: Remove manual pm_runtime get/put from tip_sense_work

[Charles Keepax]

On Mon, Mar 16, 2026 at 02:49:24PM +0200, Peter Ujfalusi wrote:
> When a jack is inserted the forced pm_runtime_get() will keep the codec,
> soundwire bus and it's parent active as long as the jack is connected.
> This makes for example the DSP and firmware booted up on Intel platforms.
> 
> If the module is removed while the jack is connected we will also have
> unbalanced runtime PM state.
> 
> Without the manual get/put, the button detection still works correctly and
> the system can reach lower power state while the jack is connected like
> in the case when there is no jack connected.
> 
> Fixes: fc918cbe874e ("ASoC: cs42l43: Add support for the cs42l43")
> Cc: stable@vger.kernel.org
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
> ---
>  sound/soc/codecs/cs42l43-jack.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/soc/codecs/cs42l43-jack.c b/sound/soc/codecs/cs42l43-jack.c
> index 3e04e6897b14..d90a13a55845 100644
> --- a/sound/soc/codecs/cs42l43-jack.c
> +++ b/sound/soc/codecs/cs42l43-jack.c
> @@ -756,10 +756,8 @@ void cs42l43_tip_sense_work(struct work_struct *work)
>  	ring = (sts >> CS42L43_RINGSENSE_PLUG_DB_STS_SHIFT) & CS42L43_JACK_PRESENT;
>  
>  	if (tip == CS42L43_JACK_PRESENT) {
> -		if (cs42l43->sdw && !priv->jack_present) {
> +		if (cs42l43->sdw && !priv->jack_present)
>  			priv->jack_present = true;
> -			pm_runtime_get(priv->dev);
> -		}

Hmm... yes, I have this feeling this was in here for a reason I
should probably have left a comment here. I somewhat agree it
looks a bit mad with fresh eyes. The variable is also only used
for tracking this pm_runtime_get so you can drop the jack_present
variable from the struct as well, if we take the patch forward.

Best I can come up with was it was some interaction with the
Intel host's doing a bus reset when coming out of clock stop. I
think that might have caused something important to get
clobbered in some situations. But anyway will do some testing and
thinking and report back.

Thanks,
Charles

Re: [PATCH] ASoC: cs42l43-jack: Remove manual pm_runtime get/put from tip_sense_work

[Péter Ujfalusi]

On 16/03/2026 16:27, Charles Keepax wrote:
> On Mon, Mar 16, 2026 at 02:49:24PM +0200, Peter Ujfalusi wrote:
>> When a jack is inserted the forced pm_runtime_get() will keep the codec,
>> soundwire bus and it's parent active as long as the jack is connected.
>> This makes for example the DSP and firmware booted up on Intel platforms.
>>
>> If the module is removed while the jack is connected we will also have
>> unbalanced runtime PM state.
>>
>> Without the manual get/put, the button detection still works correctly and
>> the system can reach lower power state while the jack is connected like
>> in the case when there is no jack connected.
>>
>> Fixes: fc918cbe874e ("ASoC: cs42l43: Add support for the cs42l43")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
>> ---
>>  sound/soc/codecs/cs42l43-jack.c | 8 ++------
>>  1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/sound/soc/codecs/cs42l43-jack.c b/sound/soc/codecs/cs42l43-jack.c
>> index 3e04e6897b14..d90a13a55845 100644
>> --- a/sound/soc/codecs/cs42l43-jack.c
>> +++ b/sound/soc/codecs/cs42l43-jack.c
>> @@ -756,10 +756,8 @@ void cs42l43_tip_sense_work(struct work_struct *work)
>>  	ring = (sts >> CS42L43_RINGSENSE_PLUG_DB_STS_SHIFT) & CS42L43_JACK_PRESENT;
>>  
>>  	if (tip == CS42L43_JACK_PRESENT) {
>> -		if (cs42l43->sdw && !priv->jack_present) {
>> +		if (cs42l43->sdw && !priv->jack_present)
>>  			priv->jack_present = true;
>> -			pm_runtime_get(priv->dev);
>> -		}
> 
> Hmm... yes, I have this feeling this was in here for a reason I
> should probably have left a comment here. I somewhat agree it
> looks a bit mad with fresh eyes. The variable is also only used
> for tracking this pm_runtime_get so you can drop the jack_present
> variable from the struct as well, if we take the patch forward.

That was my thinking as well, but then when the headset buttons did
worked after the patch on an idle system (ARL laptop) then I thought
that this might no longer be needed?

Fwiw, I have been banging my head on why the DSP is not suspending ever
on the laptop and the system is not hitting lower C state because of
this when I had some spare time and studied the code and then removed
the jack and boom, the DSP suspended right away :o

> Best I can come up with was it was some interaction with the
> Intel host's doing a bus reset when coming out of clock stop. I
> think that might have caused something important to get
> clobbered in some situations. But anyway will do some testing and
> thinking and report back.

Sure, but draining battery when the jack is connected is not a great
added feature of a codec driver.
The type-Cs are on the other side of the laptop, so taping the jack and
power together is not a workable solution - to disconnect jack if power
is removed ;)

It would be great if you can find the reason for forcing the system up
if jack is connected.
Even then there is the issue of unbalance in runtime get on module
removal when the jack is connected...

-- 
Péter

Re: [PATCH] ASoC: cs42l43-jack: Remove manual pm_runtime get/put from tip_sense_work

[Charles Keepax]

On Mon, Mar 16, 2026 at 04:37:28PM +0200, Péter Ujfalusi wrote:
> On 16/03/2026 16:27, Charles Keepax wrote:
> > On Mon, Mar 16, 2026 at 02:49:24PM +0200, Peter Ujfalusi wrote:
> > Hmm... yes, I have this feeling this was in here for a reason I
> > should probably have left a comment here. I somewhat agree it
> > looks a bit mad with fresh eyes. The variable is also only used
> > for tracking this pm_runtime_get so you can drop the jack_present
> > variable from the struct as well, if we take the patch forward.
> 
> That was my thinking as well, but then when the headset buttons did
> worked after the patch on an idle system (ARL laptop) then I thought
> that this might no longer be needed?

Hmm... are you sure that was working? Tried removing it (on
MTL here) and I see quite a few issues.

> Fwiw, I have been banging my head on why the DSP is not suspending ever
> on the laptop and the system is not hitting lower C state because of
> this when I had some spare time and studied the code and then removed
> the jack and boom, the DSP suspended right away :o

Apologies for that :-)

> Sure, but draining battery when the jack is connected is not a great
> added feature of a codec driver.
> The type-Cs are on the other side of the laptop, so taping the jack and
> power together is not a workable solution - to disconnect jack if power
> is removed ;)

I am not sure there are many other solutions. I will burn a few
cycles investigating here, but I suspect its going to come down
to a choice of two solutions:

 1) The one already in the code.
 2) Stop the host from reseting the codec.

Fundamentally reseting a device right before checking what state
it was in is always going to be hard, so would be awesome if you
could have a look at how much of a problem removing that bus
reset would be.

> Even then there is the issue of unbalance in runtime get on module
> removal when the jack is connected...

Yeah that is a good spot, if we stick with the current code I
will get that fixed up.

Thanks,
Charles

Re: [PATCH] ASoC: cs42l43-jack: Remove manual pm_runtime get/put from tip_sense_work

[Péter Ujfalusi]

On 16/03/2026 18:40, Charles Keepax wrote:
> On Mon, Mar 16, 2026 at 04:37:28PM +0200, Péter Ujfalusi wrote:
>> On 16/03/2026 16:27, Charles Keepax wrote:
>>> On Mon, Mar 16, 2026 at 02:49:24PM +0200, Peter Ujfalusi wrote:
>>> Hmm... yes, I have this feeling this was in here for a reason I
>>> should probably have left a comment here. I somewhat agree it
>>> looks a bit mad with fresh eyes. The variable is also only used
>>> for tracking this pm_runtime_get so you can drop the jack_present
>>> variable from the struct as well, if we take the patch forward.
>>
>> That was my thinking as well, but then when the headset buttons did
>> worked after the patch on an idle system (ARL laptop) then I thought
>> that this might no longer be needed?
> 
> Hmm... are you sure that was working? Tried removing it (on
> MTL here) and I see quite a few issues.

Yes, it is working on ARL (which is MTL in this regards).
There are several
soundwire_intel soundwire_intel.link.0: Slave status change: 0x2000000
soundwire_intel soundwire_intel.link.0: Slave status change: 0x4000000

back and forth, but the button press got handled fine.

Note 1: the same status change sequence happens when I plug the headset.
Note 2: the same status change sequence happens if I play audio to the
headset and then press the buttons
Note 3: without this patch I see the same status change churn when the
headset is plugged and button is pressed.

>> Fwiw, I have been banging my head on why the DSP is not suspending ever
>> on the laptop and the system is not hitting lower C state because of
>> this when I had some spare time and studied the code and then removed
>> the jack and boom, the DSP suspended right away :o
> 
> Apologies for that :-)

None of the other laptops that I have did similar thing, only this with
cs42l43, all others allows the DSP to suspend while headset is connected.

>> Sure, but draining battery when the jack is connected is not a great
>> added feature of a codec driver.
>> The type-Cs are on the other side of the laptop, so taping the jack and
>> power together is not a workable solution - to disconnect jack if power
>> is removed ;)
> 
> I am not sure there are many other solutions. I will burn a few
> cycles investigating here, but I suspect its going to come down
> to a choice of two solutions:
> 
>  1) The one already in the code.
>  2) Stop the host from reseting the codec.

The issue with 1 (how it is atm) is that it is done in a completely
wrong place. I think the cs42l43 can be used with other than Intel MTL,
let's say Qualcomm or AMD?
If there is a workaround needed for something on the platform, it has to
be done in the platform code.

> Fundamentally reseting a device right before checking what state
> it was in is always going to be hard, so would be awesome if you
> could have a look at how much of a problem removing that bus
> reset would be.

I'm still not sure if I get the whole picture, but by the hints it looks
like that on systems with cs42l43 we cannot suspend the DSP since the
codec cannot be suspended?
What is the difference between detecting the jack insert compared to
detecting the jack removal and/or the HS button detection?
Under the hood it is the same soundwire wake event then do what needs to
be done to read the cause of the event, right?
So, why it is OK to suspend the DSP when the jack is not inserted and it
is not OK if it is inserted?

>> Even then there is the issue of unbalance in runtime get on module
>> removal when the jack is connected...
> 
> Yeah that is a good spot, if we stick with the current code I
> will get that fixed up.
> 
> Thanks,
> Charles

-- 
Péter

Re: [PATCH] ASoC: cs42l43-jack: Remove manual pm_runtime get/put from tip_sense_work

[Péter Ujfalusi]

On 17/03/2026 08:21, Péter Ujfalusi wrote:
>> Fundamentally reseting a device right before checking what state
>> it was in is always going to be hard, so would be awesome if you
>> could have a look at how much of a problem removing that bus
>> reset would be.
> 
> I'm still not sure if I get the whole picture, but by the hints it looks
> like that on systems with cs42l43 we cannot suspend the DSP since the
> codec cannot be suspended?
> What is the difference between detecting the jack insert compared to
> detecting the jack removal and/or the HS button detection?
> Under the hood it is the same soundwire wake event then do what needs to
> be done to read the cause of the event, right?
> So, why it is OK to suspend the DSP when the jack is not inserted and it
> is not OK if it is inserted?

Using UI shows what you might be referring to.
with the codec powered on and pressing the button:
snd_soc_cs42l43:cs42l43_button_press: cs42l43-codec cs42l43-codec:
Detected button 0 at 8 Ohms
...
snd_soc_cs42l43:cs42l43_button_release: cs42l43-codec cs42l43-codec:
Button release IRQ

when the codec and DSP suspends when audio is idle:
snd_soc_cs42l43:cs42l43_stop_button_detect: cs42l43-codec cs42l43-codec:
Stop button detect
...
snd_soc_cs42l43:cs42l43_start_button_detect: cs42l43-codec
cs42l43-codec: Start button detect
...
snd_soc_cs42l43:cs42l43_button_press: cs42l43-codec cs42l43-codec:
Button ignored due to bias sense
...
snd_soc_cs42l43:cs42l43_button_release: cs42l43-codec cs42l43-codec:
Button release IRQ

and the first button press is ignored  - which wakes the DSP, soundwire
and codec up

So yes, there seams to be an issue with the headset button handling here.

> 
>>> Even then there is the issue of unbalance in runtime get on module
>>> removal when the jack is connected...
>>
>> Yeah that is a good spot, if we stick with the current code I
>> will get that fixed up.
>>
>> Thanks,
>> Charles
> 

-- 
Péter

Re: [PATCH] ASoC: cs42l43-jack: Remove manual pm_runtime get/put from tip_sense_work

[Péter Ujfalusi]

On 17/03/2026 11:11, Péter Ujfalusi wrote:
> 
> 
> On 17/03/2026 08:21, Péter Ujfalusi wrote:
>>> Fundamentally reseting a device right before checking what state
>>> it was in is always going to be hard, so would be awesome if you
>>> could have a look at how much of a problem removing that bus
>>> reset would be.
>>
>> I'm still not sure if I get the whole picture, but by the hints it looks
>> like that on systems with cs42l43 we cannot suspend the DSP since the
>> codec cannot be suspended?
>> What is the difference between detecting the jack insert compared to
>> detecting the jack removal and/or the HS button detection?
>> Under the hood it is the same soundwire wake event then do what needs to
>> be done to read the cause of the event, right?
>> So, why it is OK to suspend the DSP when the jack is not inserted and it
>> is not OK if it is inserted?
> 
> Using UI shows what you might be referring to.

no need for UI, evtest also shows it.

> with the codec powered on and pressing the button:
> snd_soc_cs42l43:cs42l43_button_press: cs42l43-codec cs42l43-codec:
> Detected button 0 at 8 Ohms
> ...
> snd_soc_cs42l43:cs42l43_button_release: cs42l43-codec cs42l43-codec:
> Button release IRQ

Event: time 1773739467.877146, type 1 (EV_KEY), code 164 (KEY_PLAYPAUSE), value 1
Event: time 1773739467.877146, -------------- SYN_REPORT ------------
Event: time 1773739468.040281, type 1 (EV_KEY), code 164 (KEY_PLAYPAUSE), value 0
Event: time 1773739468.040281, -------------- SYN_REPORT ------------

> 
> when the codec and DSP suspends when audio is idle:
> snd_soc_cs42l43:cs42l43_stop_button_detect: cs42l43-codec cs42l43-codec:
> Stop button detect
> ...
> snd_soc_cs42l43:cs42l43_start_button_detect: cs42l43-codec
> cs42l43-codec: Start button detect
> ...
> snd_soc_cs42l43:cs42l43_button_press: cs42l43-codec cs42l43-codec:
> Button ignored due to bias sense
> ...
> snd_soc_cs42l43:cs42l43_button_release: cs42l43-codec cs42l43-codec:
> Button release IRQ

Event: time 1773739475.147018, type 5 (EV_SW), code 2 (SW_HEADPHONE_INSERT), value 0
Event: time 1773739475.147018, type 5 (EV_SW), code 4 (SW_MICROPHONE_INSERT), value 0
Event: time 1773739475.147018, type 5 (EV_SW), code 7 (SW_JACK_PHYSICAL_INSERT), value 0
Event: time 1773739475.147018, -------------- SYN_REPORT ------------
Event: time 1773739476.220776, type 5 (EV_SW), code 2 (SW_HEADPHONE_INSERT), value 1
Event: time 1773739476.220776, type 5 (EV_SW), code 4 (SW_MICROPHONE_INSERT), value 1
Event: time 1773739476.220776, type 5 (EV_SW), code 7 (SW_JACK_PHYSICAL_INSERT), value 1
Event: time 1773739476.220776, -------------- SYN_REPORT ------------


> 
> and the first button press is ignored  - which wakes the DSP, soundwire
> and codec up
> 
> So yes, there seams to be an issue with the headset button handling here.

To note: on another MTL laptop with codec from different vendor I see the same thing.
On LNL laptop (different codec) this works correctly.

> 
>>
>>>> Even then there is the issue of unbalance in runtime get on module
>>>> removal when the jack is connected...
>>>
>>> Yeah that is a good spot, if we stick with the current code I
>>> will get that fixed up.
>>>
>>> Thanks,
>>> Charles
>>
> 

-- 
Péter

Re: [PATCH] ASoC: cs42l43-jack: Remove manual pm_runtime get/put from tip_sense_work

[Charles Keepax]

On Tue, Mar 17, 2026 at 11:36:10AM +0200, Péter Ujfalusi wrote:
> On 17/03/2026 11:11, Péter Ujfalusi wrote:
> > On 17/03/2026 08:21, Péter Ujfalusi wrote:
> >>> Fundamentally reseting a device right before checking what state
> >>> it was in is always going to be hard, so would be awesome if you
> >>> could have a look at how much of a problem removing that bus
> >>> reset would be.
> >>
> >> I'm still not sure if I get the whole picture, but by the hints it looks
> >> like that on systems with cs42l43 we cannot suspend the DSP since the
> >> codec cannot be suspended?

At least currently yeah, the problem is better phrased from the
codec side you can't reset the codec whilst some IRQs are
pending and then expect those IRQs to report correctly. The
device has no problem with the clock stop, its the reset that
causes issues.

> >> What is the difference between detecting the jack insert compared to
> >> detecting the jack removal and/or the HS button detection?
> >> Under the hood it is the same soundwire wake event then do what needs to
> >> be done to read the cause of the event, right?
> >> So, why it is OK to suspend the DSP when the jack is not inserted and it
> >> is not OK if it is inserted?

There are differences here, firstly between buttons and the jack
insert/removal, jack presence is a much more static event. This
means whilst all the state is lost on the reset, the codec can
easily rebuild that state. Buttons are more ephemeral, if it takes
too long to rebuild state the button will no longer be pressed.

Regarding differences between insert/remove, this mostly comes
down to the device issuing a jack removal IRQ on boot. As you see
below:

> > when the codec and DSP suspends when audio is idle:
> > snd_soc_cs42l43:cs42l43_stop_button_detect: cs42l43-codec cs42l43-codec:
> > Stop button detect
> > ...
> > snd_soc_cs42l43:cs42l43_start_button_detect: cs42l43-codec
> > cs42l43-codec: Start button detect
> > ...
> > snd_soc_cs42l43:cs42l43_button_press: cs42l43-codec cs42l43-codec:
> > Button ignored due to bias sense
> > ...
> > snd_soc_cs42l43:cs42l43_button_release: cs42l43-codec cs42l43-codec:
> > Button release IRQ
> 
> Event: time 1773739475.147018, type 5 (EV_SW), code 2 (SW_HEADPHONE_INSERT), value 0
> Event: time 1773739475.147018, type 5 (EV_SW), code 4 (SW_MICROPHONE_INSERT), value 0
> Event: time 1773739475.147018, type 5 (EV_SW), code 7 (SW_JACK_PHYSICAL_INSERT), value 0
> Event: time 1773739475.147018, -------------- SYN_REPORT ------------
> Event: time 1773739476.220776, type 5 (EV_SW), code 2 (SW_HEADPHONE_INSERT), value 1
> Event: time 1773739476.220776, type 5 (EV_SW), code 4 (SW_MICROPHONE_INSERT), value 1
> Event: time 1773739476.220776, type 5 (EV_SW), code 7 (SW_JACK_PHYSICAL_INSERT), value 1
> Event: time 1773739476.220776, -------------- SYN_REPORT ------------

This tends to means the reset will cause a full redetect of the
jack. Which is also quite undesirable, and will almost certainly
take too long to see the button. Not to mention doing the detect
whilst buttons are pressed is far from ideal, as it changes the
impedances seen.

I think I was occasionally seeing the jack detection get stuck as
well, although I would imagine that is just a driver issue
related to us not having fully thought through this flow
including the reset.

> > and the first button press is ignored  - which wakes the DSP, soundwire
> > and codec up
> > 
> > So yes, there seams to be an issue with the headset button handling here.
> 
> To note: on another MTL laptop with codec from different vendor I see the same thing.
> On LNL laptop (different codec) this works correctly.

I would not be surprised to see other vendors have some issues
here, I think it mostly comes down to what the device does with a
SoundWire bus reset, if the device treats it as a full reset,
which is heavily implied it should be by the spec, then problems
are very likely without special consideration.

Our newer devices are more friendly in handling this, but alas
cs42l43 is not. I will keep poking/thinking a bit, but I suspect
at least getting the first button detect out of a clock stop to
work reliably might not be feasible if we keep the bus reset.

Thanks,
Charles

Re: [PATCH] ASoC: cs42l43-jack: Remove manual pm_runtime get/put from tip_sense_work

[Charles Keepax]

On Tue, Mar 17, 2026 at 08:21:12AM +0200, Péter Ujfalusi wrote:
> On 16/03/2026 18:40, Charles Keepax wrote:
> > On Mon, Mar 16, 2026 at 04:37:28PM +0200, Péter Ujfalusi wrote:
> >> On 16/03/2026 16:27, Charles Keepax wrote:
> >>> On Mon, Mar 16, 2026 at 02:49:24PM +0200, Peter Ujfalusi wrote:
> >  1) The one already in the code.
> >  2) Stop the host from reseting the codec.
> 
> The issue with 1 (how it is atm) is that it is done in a completely
> wrong place. I think the cs42l43 can be used with other than Intel MTL,
> let's say Qualcomm or AMD?
> If there is a workaround needed for something on the platform, it has to
> be done in the platform code.

There is probably a discussion to be had here, its far from clear
to me this is the wrong place to do this. Generally the codec
controls when the codec wants to mark itself as runtime active.
For example on our phone devices where far more of the chip
powered down in runtime suspend having a jack in would always
keep the device powered up so the button detect could run,
as the lowest power states disabled that.

It is also appears the specification doesn't prohibit issuing
a bus reset when coming out of a mode 0 clock stop (which seems
bonkers to me, given literally the only difference between that
and a mode 1 clock stop is that the mode 1 clock stop resets the
device). But without the specification prohibiting this then
the device can't rely on the host not to do it, so doing this
could be required on any platform.

I can however see the argument that it would be nice to only
force the codec out of runtime suspend on platforms where this is
necessary but its not obvious to me what the sensible mechanism
would be.

Thanks,
Charles

Re: [PATCH] ASoC: cs42l43-jack: Remove manual pm_runtime get/put from tip_sense_work

[Péter Ujfalusi]

On 17/03/2026 13:44, Charles Keepax wrote:
> On Tue, Mar 17, 2026 at 08:21:12AM +0200, Péter Ujfalusi wrote:
>> On 16/03/2026 18:40, Charles Keepax wrote:
>>> On Mon, Mar 16, 2026 at 04:37:28PM +0200, Péter Ujfalusi wrote:
>>>> On 16/03/2026 16:27, Charles Keepax wrote:
>>>>> On Mon, Mar 16, 2026 at 02:49:24PM +0200, Peter Ujfalusi wrote:
>>>  1) The one already in the code.
>>>  2) Stop the host from reseting the codec.
>>
>> The issue with 1 (how it is atm) is that it is done in a completely
>> wrong place. I think the cs42l43 can be used with other than Intel MTL,
>> let's say Qualcomm or AMD?
>> If there is a workaround needed for something on the platform, it has to
>> be done in the platform code.
> 
> There is probably a discussion to be had here, its far from clear
> to me this is the wrong place to do this. Generally the codec
> controls when the codec wants to mark itself as runtime active.
> For example on our phone devices where far more of the chip
> powered down in runtime suspend having a jack in would always
> keep the device powered up so the button detect could run,
> as the lowest power states disabled that.

I see, what about this:
if the inserted accessory is CS42L43_JACK_HEADSET (SND_JACK_HEADSET)
then do one more pm_runtime_get() to allow the button presses to be handled?
This would allow the laptop to hit lower power state if a headphone is
connected, headphones do not have buttons as they don't have mic ring.

The jack_plugged would be renamed as jack_is_headset or something and
drop the rpm on jack removal for headset ( and on module remove).

> It is also appears the specification doesn't prohibit issuing
> a bus reset when coming out of a mode 0 clock stop (which seems
> bonkers to me, given literally the only difference between that
> and a mode 1 clock stop is that the mode 1 clock stop resets the
> device). But without the specification prohibiting this then
> the device can't rely on the host not to do it, so doing this
> could be required on any platform.
> 
> I can however see the argument that it would be nice to only
> force the codec out of runtime suspend on platforms where this is
> necessary but its not obvious to me what the sensible mechanism
> would be.
> 
> Thanks,
> Charles

-- 
Péter

Re: [PATCH] ASoC: cs42l43-jack: Remove manual pm_runtime get/put from tip_sense_work

[Charles Keepax]

On Tue, Mar 17, 2026 at 02:07:29PM +0200, Péter Ujfalusi wrote:
> On 17/03/2026 13:44, Charles Keepax wrote:
> > On Tue, Mar 17, 2026 at 08:21:12AM +0200, Péter Ujfalusi wrote:
> >> On 16/03/2026 18:40, Charles Keepax wrote:
> >>> On Mon, Mar 16, 2026 at 04:37:28PM +0200, Péter Ujfalusi wrote:
> >>>> On 16/03/2026 16:27, Charles Keepax wrote:
> > There is probably a discussion to be had here, its far from clear
> > to me this is the wrong place to do this. Generally the codec
> > controls when the codec wants to mark itself as runtime active.
> > For example on our phone devices where far more of the chip
> > powered down in runtime suspend having a jack in would always
> > keep the device powered up so the button detect could run,
> > as the lowest power states disabled that.
> 
> I see, what about this:
> if the inserted accessory is CS42L43_JACK_HEADSET (SND_JACK_HEADSET)
> then do one more pm_runtime_get() to allow the button presses to be handled?
> This would allow the laptop to hit lower power state if a headphone is
> connected, headphones do not have buttons as they don't have mic ring.
> 
> The jack_plugged would be renamed as jack_is_headset or something and
> drop the rpm on jack removal for headset ( and on module remove).

Yeah it might be possible to not hold the reference for
headphones as we don't need the button detect. We will need to
have a little think about the re-running of the jack detection. I
am not super keen on it running a full jack detect each time we
exit a clock stop. But we might be able to cache that and ignore
the new IRQs after the clock stop.

Thanks,
Charles

Re: [PATCH] ASoC: cs42l43-jack: Remove manual pm_runtime get/put from tip_sense_work

[Charles Keepax]

On Mon, Mar 16, 2026 at 02:27:40PM +0000, Charles Keepax wrote:
> On Mon, Mar 16, 2026 at 02:49:24PM +0200, Peter Ujfalusi wrote:
> > When a jack is inserted the forced pm_runtime_get() will keep the codec,
> > soundwire bus and it's parent active as long as the jack is connected.
> > This makes for example the DSP and firmware booted up on Intel platforms.
> > 
> > If the module is removed while the jack is connected we will also have
> > unbalanced runtime PM state.
> > 
> > Without the manual get/put, the button detection still works correctly and
> > the system can reach lower power state while the jack is connected like
> > in the case when there is no jack connected.
> > 
> > Fixes: fc918cbe874e ("ASoC: cs42l43: Add support for the cs42l43")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
> > ---
> >  sound/soc/codecs/cs42l43-jack.c | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> > 
> > diff --git a/sound/soc/codecs/cs42l43-jack.c b/sound/soc/codecs/cs42l43-jack.c
> > index 3e04e6897b14..d90a13a55845 100644
> > --- a/sound/soc/codecs/cs42l43-jack.c
> > +++ b/sound/soc/codecs/cs42l43-jack.c
> > @@ -756,10 +756,8 @@ void cs42l43_tip_sense_work(struct work_struct *work)
> >  	ring = (sts >> CS42L43_RINGSENSE_PLUG_DB_STS_SHIFT) & CS42L43_JACK_PRESENT;
> >  
> >  	if (tip == CS42L43_JACK_PRESENT) {
> > -		if (cs42l43->sdw && !priv->jack_present) {
> > +		if (cs42l43->sdw && !priv->jack_present)
> >  			priv->jack_present = true;
> > -			pm_runtime_get(priv->dev);
> > -		}
> 
> Hmm... yes, I have this feeling this was in here for a reason I
> should probably have left a comment here. I somewhat agree it
> looks a bit mad with fresh eyes. The variable is also only used
> for tracking this pm_runtime_get so you can drop the jack_present
> variable from the struct as well, if we take the patch forward.
> 
> Best I can come up with was it was some interaction with the
> Intel host's doing a bus reset when coming out of clock stop. I
> think that might have caused something important to get
> clobbered in some situations. But anyway will do some testing and
> thinking and report back.

Wait, I think it was the headset button detection. I think the
problem went roughly like you get a button press on the headset
whilst the host is powered down, the device wakes the host, the
host resets the device, wiping the button event. So to the user
the button press is just ignored.

I will try to find time to retest to confirm this, but I have
a good feeling that was the problem that caused us to add this
runtime get. I think maybe we could get away with dropping down to
only doing it for 4-pole headsets so one could still get the power
savings on 3-pole headphones, since the buttons are not required.

But fundamentally I think the issue was with the bus reset coming
out of clock stop. We had erroneously assumed on our end that only
clock stop mode 1 would include a reset of the device.

Thanks,
Charles