i get why you'd want to wait a while for things to settle, but is there a more reliable way? eg is there some bit that will return a specific value once core power/reset has stabilised?

Good call — there is a deterministic signal, and I switched to it.

I instrumented the resume path on Alder/Raptor Lake-P (Framework Laptop 12):

• At the original pause() location (after the DEVIDLE un-idle, before the core reset is deasserted) the entire DesignWare register bank still reads back 0x0: the core reset asserted in ig4iic_suspend() is still asserted there, so there is nothing to poll yet. I polled COMP_TYPE for 10 ms and never saw it change.

• The core only becomes readable after the RESETS_SKL deassert in ig4iic_set_config(). Immediately after that deassert, COMP_TYPE reads its fixed signature 0x44570140 (and COMP_VER/COMP_PARAM1 are valid) with zero wait, on every cold boot and every resume.

So I dropped the blind pause() entirely and instead poll COMP_TYPE right
after the core reset is deasserted, breaking out as soon as it returns
the signature (bounded to ~1 ms for slower silicon). Measured settle on
this hardware is 0 us. This also covers the existing RESTORE_REQUIRED
path, since both reach that reset.

I left the pre-existing pause() in the RESTORE_REQUIRED branch (between
the un-idle and the reset) untouched to keep the change scoped — happy to
convert that one too if you'd prefer.

Updated the diff.

So, see here how it's reprogramming some stuff based on hardware type / config and if it was reset above?
And looking at it now, the reset path looks suspiciously like what you're doing.

I wonder if it would be cleaner to modify ig4iic_set_config() to take a third parameter, call it "force restore",
which is false except in the restore path you're modifying where you'd call it set to true.

Then the if statement becomes something like:

if (IG4_HAS_ADDREGS(sc->version) && ((force_restore == true) || (v & IG4_RESTORE_REQUIRED))) {

.. which would force do the reset/restore here, set reset = true (which happens to be the case in your current diff).

Then your resume handler would be:

bool force_restore = false;
...
if (IG4_HAS_ADDREGS(sc->version) 
  force_restore = true;
...
	if (ig4iic_set_config(sc, IG4_HAS_ADDREGS(sc->version), force_restore))
``

That way we have a way to force a reset /and/ force a full restore whilst keeping the actual reset/restore handshake in one place.

What do you think?

This is good; I'd add a final check after the loop to see if it's fully woken up and if not print a nice big warning.
(Later on after this lands we can refactor the check out, return an error if it doesn't wake up, etc..)

You may want to put that pause() back here just to match the other reset logic being done in set_config().

Thanks! Will do

👍

I am fine with this approach. I discussed with Deb that I only intend to MFC this to stable/15 and also likely releng/15.1 for a p-release to propagate it to other Framework users (I usually don't like to change API, but it's file-local and static, so MFC friction should be low -- especially bounded to just the stable/releng 15 series)