On Thu, 2023-12-07 at 18:20 +0000, Mark Brown wrote: > On Thu, Dec 07, 2023 at 12:59:44AM +0100, Gergo Koteles wrote: > > > The amp has 3 level addressing (BOOK, PAGE, REG). > > The regcache couldn't handle it. > > So the books aren't currently used so the driver actually works? > It writes to the book 0 and 8c. The initialization works with regcache, because it writes also the i2c devices. > > static int tas2781_system_suspend(struct device *dev) > > @@ -770,10 +758,7 @@ static int tas2781_system_suspend(struct device *dev) > > return ret; > > > > /* Shutdown chip before system suspend */ > > - regcache_cache_only(tas_priv->regmap, false); > > tasdevice_tuning_switch(tas_priv, 1); > > - regcache_cache_only(tas_priv->regmap, true); > > - regcache_mark_dirty(tas_priv->regmap); > > > > /* > > * Reset GPIO may be shared, so cannot reset here. > > How can this work over system suspend? This just removes the cache with > no replacement so if the device looses power over suspend (which seems > likely) then all the register state will be lost. A similar issue may > potentially exist over runtime suspend on an ACPI system with > sufficiently heavily optimised power management. In runtime_resume, only one of the two amplifiers goes back. The runtime_suspend sets the current book/prog/conf to -1 on all devices, and tas2781_hda_playback_hook will restore the program/configuration/profile with tasdevice_tuning_switch. And only one, because tasdevice_change_chn_book directly changes the address of i2c_client, so the unlucky one gets invalid values in its actual book from regcache_sync. system_restore doesn't work at all, because regcache_cache_only stays true since system_suspend. It works without the regcache functions.
