Re: [PATCH] bus: ti-sysc: Fix gpt12 system timer issue with reserved status

[Tony Lindgren]

Hi,

* Kevin Hilman <khilman@xxxxxxxxxxxx> [220304 17:39]:
> Hi Tony,
> 
> Tony Lindgren <tony@xxxxxxxxxxx> writes:
> 
> > Jarkko Nikula <jarkko.nikula@xxxxxxxxxx> reported that Beagleboard
> > revision c2 stopped booting. Jarkko bisected the issue down to
> > commit 6cfcd5563b4f ("clocksource/drivers/timer-ti-dm: Fix suspend
> > and resume for am3 and am4").
> >
> > Let's fix the issue by tagging system timers as reserved rather than
> > ignoring them. And let's not probe any interconnect target module child
> > devices for reserved modules.
> >
> > This allows PM runtime to keep track of clocks and clockdomains for
> > the interconnect target module, and prevent the system timer from idling
> > as we already have SYSC_QUIRK_NO_IDLE and SYSC_QUIRK_NO_IDLE_ON_INIT
> > flags set for system timers.
> >
> > Fixes: 6cfcd5563b4f ("clocksource/drivers/timer-ti-dm: Fix suspend and resume for am3 and am4")
> > Reported-by: Jarkko Nikula <jarkko.nikula@xxxxxxxxxx>
> > Signed-off-by: Tony Lindgren <tony@xxxxxxxxxxx>
> 
> I'm debugging why suspend/resume on AM3x and AM4x are mostly working,
> but getting the warning that not all powerdomains are transitioning:
> 
>    pm33xx pm33xx: PM: Could not transition all powerdomains to target state
> 
> I bisected it down to $SUBJECT patch, and verified that reverting it
> makes both on am335x-boneblack and am437x-gp-evm fully suspend, and I'm
> now seeing:
> 
>    pm33xx pm33xx: PM: Successfully put all powerdomains to target state
> 
> Note that it doesn't revert cleanly due to some other changes, but this
> one-liner[1] effectively reverts the behavior of $SUBJECT patch, and
> also makes things work again.
> 
> I verified the revert (and hack[1]) on both v5.10 stable and mainline
> v5.16 but TBH, I'm still not 100% sure what's going on so looking for
> some guidance from you Tony on what the "real" fix should be.

Thanks for debugging the issue Kevin. It seems the issue is caused by the
extra runtime PM usage count done for modules tagged no-idle. However,
this causes issues for am335x timers as the PM coprocessor needs all
the domains idled for system suspend despite the system timers tagged
with no-idle.

We could patch ti-sysc.c for more timer workarounds, but I don't know if
that really makes sense. It would add further dependencies between the
system timer code and the interconnect code, and I'd rather go back to
no dependencies between the system timers and the interconnect code :)

So I suggest we make the omap3 gpt12 quirk checks SoC specific as below
for now, they are not needed for the other SoCs.

Then at some point we can plan on dropping support for the old beagleboard
revisions A to B4, and then reverting commit 3ff340e24c9d ("bus: ti-sysc:
Fix gpt12 system timer issue with reserved status").

Note that we now have commit 23885389dbbb ("ARM: dts: Fix timer regression
for beagleboard revision c"), so there no need to (wrongly) enable the
old timer quirks for working omap3 revision C and later boards.

Regards,

Tony

8< ----------------------