Re: [PATCH] ASoC: tas2783-sdw: drop stale regcache on uninitialized re-attach

[Andrey Golovko <andrey.golovko@gmail.com>]

On Mon, Aug 24, 2026 at 11:55:35AM +0000, Xu, Baojun wrote:

> Based on my test, this modification is also needed in
> tas2783_sdca_dev_resume().

(Restoring linux-sound and the rest of the Cc list, since the patch was
posted there.)

Thank you for looking at it.  I agree the same sync is a problem there,
and the ordering makes that unambiguous: sdw_handle_slave_status() calls
the driver's update_status() callback - where the cache is now dropped
and tas_io_init() re-downloads the firmware - and only afterwards does
complete_all(&slave->initialization_complete).  That completion is what
sdw_slave_wait_for_init() at the top of tas2783_sdca_dev_resume() is
waiting for, so by the time regcache_sync() runs the part has already
been soft-reset and re-initialized.  Syncing there writes the cache back
onto a device that has just been brought up from scratch.

What I would not do is drop unconditionally in dev_resume(), because
that function is also the RUNTIME_PM_OPS resume callback.  On a resume
where the peripheral kept its context, or came back through clock stop
without losing state, regcache_sync() is the only thing that restores
the user's settings, and dropping the cache there would silently reset
volume and mute on every runtime resume.

So the shape I have in mind is a flag rather than a second drop: set it
in tas_update_status() on the uninitialized-attach path where the cache
is dropped today, consume it in dev_resume(), and simply skip the sync
when it is set - after tas_io_init() the cache already mirrors the
hardware, so there is nothing worth syncing, only registers that can
fail.  Something like:

	if (test_and_clear_bit(TAS_REINIT, &tas_dev->flags))
		return 0;	/* re-initialized from scratch, cache is fresh */

	regcache_cache_only(tas_dev->regmap, false);
	ret = regcache_sync(tas_dev->regmap);

Would you prefer that, or do you have a different fix in progress on
your side?  I am happy to write and test it either way - I just do not
want us to post two versions of the same thing.

Before I write the changelog, though, I need to describe a failure I can
actually point at, and this is where I have to ask what you saw.  On the
board I have here - ASUS ProArt PX13, AMD ACP7.0, two TAS2783 plus RT721
on link 1 - I cannot reproduce a failure on that path:

- the codec never reaches runtime suspend at all (runtime_status stays
  active, the usage count never drops to zero), so the runtime resume
  path is not exercised;

- on the system resume path, across s2idle cycles where both amplifiers
  genuinely lose power, re-attach and re-download the firmware, the
  journal shows no resume error at all.  My reading is that
  regcache_sync() returns early: regcache_cache_only(true) does not by
  itself set cache_dirty, and if nothing writes through the cache while
  the device is suspended, sync takes the "if (!map->cache_dirty) goto
  out" exit and never touches the bus.  Which would mean the bug is
  latent here and armed only when something does dirty the cache during
  suspend.

That it is armed at all is easy to show: when I force the sync on this
part (through a small debug module, outside of any suspend), it aborts
at 0x40400108, FU23 Mute ch0, with -ENODATA - reg_defaults claims 0x1
while the init sequence writes 0x00, so sync tries to "restore" a value
the device refuses.  A dev_resume() that reaches the sync on this
hardware would therefore fail outright and return -ENODATA to the PM
core, not merely leave the amplifier stale.  That is one more instance
of the reg_defaults question in my other mail of 24 August [1].

So could you tell me a bit more about your test:

1. Which resume path - runtime resume, or system resume from s2idle/S3?
2. Which tree, and does it already contain b627da430357 in
   update_status()?
3. What did you observe - a sync error code in the log, or silent
   speakers with no error at all?
4. Does your board's controller power-gate the link across suspend, so
   the amplifiers re-attach uninitialized, or do they keep context?

With that I can write the patch against a failure that is described
rather than assumed, and test it here by forcing the cache dirty across
suspend.

[1] https://lore.kernel.org/linux-sound/20260824104500.7588-1-andrey.golovko@gmail.com/

Thanks,
Andrey