Re: [PATCH v14 28/38] phy: core: add notifier infrastructure

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] AB-BA lock inversion deadlock between PHY consumer state locks and phy->mutex via synchronous notifier.
--

--- Patch [28]: [PATCH v14 28/38] phy: core: add notifier infrastructure ---
commit 140d5ba0c3927a01cec06534c2dc22cf14579ba0
Author: Sebastian Reichel <[email protected]>

phy: core: add notifier infrastructure

This commit introduces a notifier API for PHY devices, allowing consumer
drivers to hook into PHY reset events. This provides a mechanism for consumers
to gracefully handle the PHY going down during configuration or orientation
changes.

> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 21aaf2f76e53e..51d261daae7a9 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c

[ ... ]

> @@ -542,6 +542,70 @@ int phy_notify_state(struct phy *phy, union phy_notify state)

[ ... ]

> +/**
> + * phy_notify_reset() - notify consumers of a PHY reset event
> + * @phy: the phy that is being reset
> + * @event: the notification event (PRE_RESET or POST_RESET)
> + *
> + * Called by PHY providers to notify consumers that the PHY is about to
> + * be reset or has completed a reset. This allows consumers to quiesce
> + * hardware before the PHY becomes unavailable.
> + *
> + * This may be called from within PHY provider callbacks (e.g. set_mode,
> + * power_on) where phy->mutex is held. Consumer notification handlers must
> + * therefore NOT call back into the PHY framework (e.g. phy_power_off,
> + * phy_exit) on the same PHY, as this would result in a deadlock.
> + *
> + * Returns: %0 if successful or no notifiers registered, a negative error
> + * code if a notifier returns an error (for PRE_RESET only)
> + */
> +int phy_notify_reset(struct phy *phy, enum phy_notification event)
> +{
> +	int ret;
> +
> +	if (!phy)
> +		return 0;
> +
> +	ret = blocking_notifier_call_chain(&phy->notifier, event, phy);

[Severity: High]
Does this synchronous notification under phy->mutex introduce an AB-BA lock
inversion risk with consumer driver state locks?

For example, if a multi-port PHY has multiple consumers (like DP and USB),
Consumer A could acquire its driver state lock (e.g., dp_lock) and call
phy_power_on(phy), blocking on phy->mutex.

Concurrently, if Consumer B triggers a phy_set_mode(phy) call, the PHY core
acquires phy->mutex and invokes the provider's callback, which in turn calls
phy_notify_reset(). This synchronously invokes Consumer A's notifier callback
via blocking_notifier_call_chain().

If Consumer A attempts to acquire its state lock (dp_lock) within the notifier
callback to safely quiesce its hardware, it will deadlock. Is it safe to execute
this blocking notifier chain while holding phy->mutex?

> +	return notifier_to_errno(ret);
> +}
> +EXPORT_SYMBOL_GPL(phy_notify_reset);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=28

-- 
linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy