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

[Charles Keepax <ckeepax@opensource.cirrus.com>]

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