The soundwire update_status() callback may be called multiple times with
the same ATTACHED status but initialisation should only be done when
transitioning from UNATTACHED to ATTACHED.

Fix the inverted hw_init flag which was set to false instead of true
after initialisation which defeats its purpose and may result in
repeated unnecessary initialisation.

Similarly, the initial state of the flag was also inverted so that the
codec would only be initialised and brought out of regmap cache only
mode if its status first transitions to UNATTACHED.

Fixes: aa21a7d4f68a ("ASoC: codecs: wsa884x: Add WSA884x family of speakers")
Cc: stable@vger.kernel.org	# 6.5
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 sound/soc/codecs/wsa884x.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/soc/codecs/wsa884x.c b/sound/soc/codecs/wsa884x.c
index 887edd2be705..6c6b497657d0 100644
--- a/sound/soc/codecs/wsa884x.c
+++ b/sound/soc/codecs/wsa884x.c
@@ -1534,7 +1534,7 @@ static void wsa884x_init(struct wsa884x_priv *wsa884x)
 
 	wsa884x_set_gain_parameters(wsa884x);
 
-	wsa884x->hw_init = false;
+	wsa884x->hw_init = true;
 }
 
 static int wsa884x_update_status(struct sdw_slave *slave,
@@ -2109,7 +2109,6 @@ static int wsa884x_probe(struct sdw_slave *pdev,
 
 	/* Start in cache-only until device is enumerated */
 	regcache_cache_only(wsa884x->regmap, true);
-	wsa884x->hw_init = true;
 
 	if (IS_REACHABLE(CONFIG_HWMON)) {
 		struct device *hwmon;
-- 
2.51.2

On 02/01/2026 12:14, Johan Hovold wrote:
> The soundwire update_status() callback may be called multiple times with
> the same ATTACHED status but initialisation should only be done when
> transitioning from UNATTACHED to ATTACHED.
> 
> Fix the inverted hw_init flag which was set to false instead of true
> after initialisation which defeats its purpose and may result in
> repeated unnecessary initialisation.
> 
> Similarly, the initial state of the flag was also inverted so that the
> codec would only be initialised and brought out of regmap cache only
> mode if its status first transitions to UNATTACHED.
> 
> Fixes: aa21a7d4f68a ("ASoC: codecs: wsa884x: Add WSA884x family of speakers")
> Cc: stable@vger.kernel.org	# 6.5
> Cc: Krzysztof Kozlowski <krzk@kernel.org>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Tested on Lenovo T14s:
Tested-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>


Best regards,
Krzysztof

On 02/01/2026 12:14, Johan Hovold wrote:
> The soundwire update_status() callback may be called multiple times with
> the same ATTACHED status but initialisation should only be done when
> transitioning from UNATTACHED to ATTACHED.
> 
> Fix the inverted hw_init flag which was set to false instead of true
> after initialisation which defeats its purpose and may result in
> repeated unnecessary initialisation.

Either it results or it does not, not "may". If the device moves to
UNATTACHED state flag should be probably set to "true". This is the bug.

> 
> Similarly, the initial state of the flag was also inverted so that the
> codec would only be initialised and brought out of regmap cache only
> mode if its status first transitions to UNATTACHED.


Maybe that's confusing wording but existing code was intentional and IMO
almost correct. The flag is saying - we need hw init - that's why it is
set to true in the probe and to false AFTER the proper hw initialization
which is done after ATTACHED state.

If you find naming confusing, maybe rename it to other style or fix the
false -> true when going to unattached stat, but not everything at once.


Best regards,
Krzysztof

On Fri, Jan 02, 2026 at 12:31:21PM +0100, Krzysztof Kozlowski wrote:
> On 02/01/2026 12:14, Johan Hovold wrote:
> > The soundwire update_status() callback may be called multiple times with
> > the same ATTACHED status but initialisation should only be done when
> > transitioning from UNATTACHED to ATTACHED.
> > 
> > Fix the inverted hw_init flag which was set to false instead of true
> > after initialisation which defeats its purpose and may result in
> > repeated unnecessary initialisation.
> 
> Either it results or it does not, not "may". 

No, it depends on whether update_status() is called with the same status
more than once. So "may" is correct here.

> If the device moves to
> UNATTACHED state flag should be probably set to "true". This is the bug.

No, update_status() has:

	if (wsa884x->hw_init || status != SDW_SLAVE_ATTACHED)
		return 0;

	...

	wsa884x_init(wsa884x);

so if you set hw_init to true then init is never called when status is
changed to ATTACHED.

> > 
> > Similarly, the initial state of the flag was also inverted so that the
> > codec would only be initialised and brought out of regmap cache only
> > mode if its status first transitions to UNATTACHED.
> 
> Maybe that's confusing wording but existing code was intentional and IMO
> almost correct. The flag is saying - we need hw init - that's why it is
> set to true in the probe and to false AFTER the proper hw initialization
> which is done after ATTACHED state.

All other codec drivers have hw_init mean that init has been done and
the check in update_status() reflects that too so this driver is still
broken.

Johan

On 02/01/2026 12:43, Johan Hovold wrote:
> On Fri, Jan 02, 2026 at 12:31:21PM +0100, Krzysztof Kozlowski wrote:
>> On 02/01/2026 12:14, Johan Hovold wrote:
>>> The soundwire update_status() callback may be called multiple times with
>>> the same ATTACHED status but initialisation should only be done when
>>> transitioning from UNATTACHED to ATTACHED.
>>>
>>> Fix the inverted hw_init flag which was set to false instead of true
>>> after initialisation which defeats its purpose and may result in
>>> repeated unnecessary initialisation.
>>
>> Either it results or it does not, not "may". 
> 
> No, it depends on whether update_status() is called with the same status
> more than once. So "may" is correct here.
> 
>> If the device moves to
>> UNATTACHED state flag should be probably set to "true". This is the bug.
> 
> No, update_status() has:
> 
> 	if (wsa884x->hw_init || status != SDW_SLAVE_ATTACHED)
> 		return 0;
> 
> 	...
> 
> 	wsa884x_init(wsa884x);
> 
> so if you set hw_init to true then init is never called when status is
> changed to ATTACHED.

Uh, indeed, so this was supposed to be !wsa884x->hw_init... or indeed
your meaning. This also means that this was never passing above if() and
the init() was never called.

regcache was probably synced via runtime PM, so at least that part worked.

Did you test this driver on actual device how it affects the behavior?

> 
>>>
>>> Similarly, the initial state of the flag was also inverted so that the
>>> codec would only be initialised and brought out of regmap cache only
>>> mode if its status first transitions to UNATTACHED.
>>
>> Maybe that's confusing wording but existing code was intentional and IMO
>> almost correct. The flag is saying - we need hw init - that's why it is
>> set to true in the probe and to false AFTER the proper hw initialization
>> which is done after ATTACHED state.
> 
> All other codec drivers have hw_init mean that init has been done and
> the check in update_status() reflects that too so this driver is still
> broken.

ok


Best regards,
Krzysztof

On Fri, Jan 02, 2026 at 12:50:45PM +0100, Krzysztof Kozlowski wrote:
> On 02/01/2026 12:43, Johan Hovold wrote:
> > On Fri, Jan 02, 2026 at 12:31:21PM +0100, Krzysztof Kozlowski wrote:
> >> On 02/01/2026 12:14, Johan Hovold wrote:
> >>> The soundwire update_status() callback may be called multiple times with
> >>> the same ATTACHED status but initialisation should only be done when
> >>> transitioning from UNATTACHED to ATTACHED.
> >>>
> >>> Fix the inverted hw_init flag which was set to false instead of true
> >>> after initialisation which defeats its purpose and may result in
> >>> repeated unnecessary initialisation.

> > No, update_status() has:
> > 
> > 	if (wsa884x->hw_init || status != SDW_SLAVE_ATTACHED)
> > 		return 0;
> > 
> > 	...
> > 
> > 	wsa884x_init(wsa884x);
> > 
> > so if you set hw_init to true then init is never called when status is
> > changed to ATTACHED.
> 
> Uh, indeed, so this was supposed to be !wsa884x->hw_init... or indeed
> your meaning. This also means that this was never passing above if() and
> the init() was never called.
> 
> regcache was probably synced via runtime PM, so at least that part worked.
> 
> Did you test this driver on actual device how it affects the behavior?

No, I don't have a device that uses this codec anymore.

If you could give it a spin that would be great. It seems we have been
depending on reset values or an UNATTACHED => ATTACHED transition so
far.

Johan

On 02/01/2026 13:17, Johan Hovold wrote:
>>> 	...
>>>
>>> 	wsa884x_init(wsa884x);
>>>
>>> so if you set hw_init to true then init is never called when status is
>>> changed to ATTACHED.
>>
>> Uh, indeed, so this was supposed to be !wsa884x->hw_init... or indeed
>> your meaning. This also means that this was never passing above if() and
>> the init() was never called.
>>
>> regcache was probably synced via runtime PM, so at least that part worked.
>>
>> Did you test this driver on actual device how it affects the behavior?
> 
> No, I don't have a device that uses this codec anymore.
> 
> If you could give it a spin that would be great. It seems we have been
> depending on reset values or an UNATTACHED => ATTACHED transition so
> far.

I can test it, maybe after the weekend.

Best regards,
Krzysztof