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

["Péter Ujfalusi" <peter.ujfalusi@linux.intel.com>]

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