Thread (58 messages) 58 messages, 4 authors, 9d ago

Re: [PATCH v13 28/35] phy: core: add notifier infrastructure

From: sashiko-bot@kernel.org
Date: 2026-07-14 20:10:12
Also in: linux-phy

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing rollback mechanism in PHY reset notifier chain
--
--- Patch [28]: [PATCH v13 28/35] phy: core: add notifier infrastructure ---
commit ec60b7ba41c261654c48fb56546b6b42f913f6e5
Author: Sebastian Reichel [off-list ref]

phy: core: add notifier infrastructure

This commit introduces a new PHY notifier API to allow consumer drivers to
hook into reset events from a PHY device. This helps multi-port PHY devices
handle configuration or cable orientation changes without causing undefined
behavior in consumers.
quoted hunk ↗ jump to hunk
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
[ ... ]
+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);
+	return notifier_to_errno(ret);
+}
[Severity: Medium]
Since blocking_notifier_call_chain() aborts on the first error, does this
leave earlier consumers permanently suspended?

If phy_notify_reset() is called with PHY_NOTIFY_PRE_RESET on a multi-consumer
PHY, and the notifier chain stops halfway through due to an error, the provider
receives a negative error code. The earlier consumers in the chain will have
already handled PHY_NOTIFY_PRE_RESET and quiesced their hardware.

Would it be safer to introduce a rollback mechanism to notify the successfully
suspended consumers that the reset has been aborted, so they don't remain
stuck indefinitely?
quoted hunk ↗ jump to hunk
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index ea47975e288ae..3779a4d0a02c3 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
[ ... ]
+/**
+ * enum phy_notification - PHY notification events
+ * @PHY_NOTIFY_PRE_RESET: PHY is about to be reset, consumers should quiesce
+ * @PHY_NOTIFY_POST_RESET: PHY reset is complete, consumers may resume
+ */
+enum phy_notification {
+	PHY_NOTIFY_PRE_RESET,
+	PHY_NOTIFY_POST_RESET,
+};
[Severity: Medium]
Is there a missing abort event here to support the rollback path mentioned
above?

If a pre-reset notification fails, broadcasting PHY_NOTIFY_POST_RESET might
break consumers that were never notified of the pre-reset in the first place.
Adding a dedicated abort event might give consumers a safe way to recover when
a provider has to cancel a planned reset.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714-rockchip-usbdp-cleanup-v13-0-6cb3e769d4c5@collabora.com?part=28
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help