Re: [PATCH 1/2] phy: qcom: qmp-combo: do not decrement init_count on a forced init failure

[Oleg Keri <okerixx@gmail.com>]

You are right, and this patch should be dropped.  Please do not apply it.

The diagnosis in the commit message holds - a forced qmp_combo_com_init()
never takes the reference, because && short-circuits on !force, so the
unconditional decrement in the error path drops one that was never taken.
The remedy does not.

Tracing a consumer that holds phy_init(), so init_count is 1 and the block
is up, through a typec callback that does com_exit(force) then a com_init(force)
which fails:

  before this patch   init_count 0, hardware down.
                      a later com_exit(false) does --init_count -> -1, which
                      is non-zero, so it returns early: no double disable,
                      but the reference is lost and the count stays negative.

  with this patch     init_count 1, hardware down.
                      a later com_exit(false) does --init_count -> 0 and
                      proceeds to the full teardown, so
                      clk_disable_unprepare(), clk_bulk_disable_unprepare()
                      and regulator_bulk_disable() all run against resources
                      the error path already released.

So it swaps a silently negative counter for a real unbalanced disable, which
is worse than what it replaces.  Exactly what you asked.

The underlying problem is that init_count is being used for two different
things - how many consumers hold a reference, and whether the common block
is currently powered - and after a failed forced re-init those two diverge.
No adjustment to the single counter can express that, so I would rather send
nothing here than send another half fix.  I will come back to it with the
hardware state tracked separately, once I can convince myself the teardown
paths are balanced in every case.

Patch 2/2 does not touch the counter at all - it only stops both callbacks
carrying on to write PHY registers after the bringup has already failed and
left the clocks disabled - so it stands on its own and can still be applied.

The runtime PM patch it was a follow-up to is likewise independent:
https://lore.kernel.org/all/20260909140345.4239-1-okerixx@gmail.com/

pw-bot: changes-requested

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy