* [PATCH net-next v4 0/4] net: mdio: implement optional PHY reset before MDIO access
@ 2025-10-22 9:08 Buday Csaba
2025-10-22 9:08 ` [PATCH net-next v4 1/4] net: mdio: common handling of phy reset properties Buday Csaba
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Buday Csaba @ 2025-10-22 9:08 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Philipp Zabel, netdev,
linux-kernel
Cc: Buday Csaba
Some Ethernet PHY devices require a hard reset before any MDIO access can
be safely performed. This includes the auto-detection of the PHY ID, which
is necessary to bind the correct driver to the device.
The kernel currently does not provide a way to assert the reset before
reading the ID, making these devices usable only when the ID is hardcoded
in the Device Tree 'compatible' string.
(One notable exception is the FEC driver and its now deprecated
`phy-reset-gpios` property).
This patchset implements an optional reset before reading of the PHY ID
register, allowing such PHYs to be used with auto-detected ID. The reset
is only asserted when the current logic fails to detect the ID, ensuring
compatibility with existing systems.
There have been several earlier attempts to implement such functionality,
of which I have collected a few in the links section.
The links to my own v1, v2 and v3 versions are also provided.
The most notable change compared to v3, is that the DT binding is removed,
and the hard-reset works as a fallback logic now.
Link: https://lore.kernel.org/lkml/1499346330-12166-2-git-send-email-richard.leitner@skidata.com/
Link: https://lore.kernel.org/all/20230405-net-next-topic-net-phy-reset-v1-0-7e5329f08002@pengutronix.de/
Link: https://lore.kernel.org/netdev/20250709133222.48802-4-buday.csaba@prolan.hu/
Link: https://lore.kernel.org/all/20251013135557.62949-1-buday.csaba@prolan.hu/
Link: https://lore.kernel.org/all/20251015134503.107925-1-buday.csaba@prolan.hu/
Link: https://lore.kernel.org/netdev/cover.1760620093.git.buday.csaba@prolan.hu/
Buday Csaba (4):
net: mdio: common handling of phy reset properties
net: mdio: change property read from fwnode_property_read_u32() to
device_property_read_u32()
net: mdio: introduce mdio_device_has_reset()
net: mdio: reset PHY before attempting to access registers in
fwnode_mdiobus_register_phy
drivers/net/mdio/fwnode_mdio.c | 46 ++++++++++++++++++++----
drivers/net/phy/mdio_bus.c | 39 ++------------------
drivers/net/phy/mdio_device.c | 65 ++++++++++++++++++++++++++++++++++
include/linux/mdio.h | 3 ++
4 files changed, 110 insertions(+), 43 deletions(-)
base-commit: 00922eeaca3c5c2001781bcad40e0bd54d0fdbb6
--
2.39.5
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next v4 1/4] net: mdio: common handling of phy reset properties
2025-10-22 9:08 [PATCH net-next v4 0/4] net: mdio: implement optional PHY reset before MDIO access Buday Csaba
@ 2025-10-22 9:08 ` Buday Csaba
2025-10-29 1:22 ` Jakub Kicinski
2025-10-22 9:08 ` [PATCH net-next v4 2/4] net: mdio: change property read from fwnode_property_read_u32() to device_property_read_u32() Buday Csaba
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Buday Csaba @ 2025-10-22 9:08 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Philipp Zabel, netdev,
linux-kernel
Cc: Buday Csaba
Reset properties of an `mdio_device` are initialized in multiple
source files and multiple functions:
- `reset_assert_delay` and `reset_deassert_delay` are in
fwnode_mdio.c
- `reset_gpio` and `reset_ctrl` are in mdio_bus.c, but handled by
different functions
This patch unifies the handling of all these properties into two
functions.
mdiobus_register_gpiod() and mdiobus_register_reset() are removed,
while mdio_device_register_reset() and mdio_device_unregister_reset()
are introduced instead.
These functions handle both reset-controllers and reset-gpios, and
also read the corresponding properties from the device tree.
These changes should make tracking the reset properties easier.
The reset logic is unaltered, and should work as it did before.
Signed-off-by: Buday Csaba <buday.csaba@prolan.hu>
---
V3 -> V4: unmodified
V2 -> V3: fixed kernel-doc warnings
V1 -> V2: the return value of mdio_device_unregister_reset() is made void
---
drivers/net/mdio/fwnode_mdio.c | 5 ----
drivers/net/phy/mdio_bus.c | 39 ++-----------------------
drivers/net/phy/mdio_device.c | 52 ++++++++++++++++++++++++++++++++++
include/linux/mdio.h | 2 ++
4 files changed, 56 insertions(+), 42 deletions(-)
diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index 9b41d4697..ba7091518 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -92,11 +92,6 @@ int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
if (fwnode_property_read_bool(child, "broken-turn-around"))
mdio->phy_ignore_ta_mask |= 1 << addr;
- fwnode_property_read_u32(child, "reset-assert-us",
- &phy->mdio.reset_assert_delay);
- fwnode_property_read_u32(child, "reset-deassert-us",
- &phy->mdio.reset_deassert_delay);
-
/* Associate the fwnode with the device structure so it
* can be looked up later
*/
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index cad6ed3aa..cc3f9cfb1 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -33,33 +33,6 @@
#define CREATE_TRACE_POINTS
#include <trace/events/mdio.h>
-static int mdiobus_register_gpiod(struct mdio_device *mdiodev)
-{
- /* Deassert the optional reset signal */
- mdiodev->reset_gpio = gpiod_get_optional(&mdiodev->dev,
- "reset", GPIOD_OUT_LOW);
- if (IS_ERR(mdiodev->reset_gpio))
- return PTR_ERR(mdiodev->reset_gpio);
-
- if (mdiodev->reset_gpio)
- gpiod_set_consumer_name(mdiodev->reset_gpio, "PHY reset");
-
- return 0;
-}
-
-static int mdiobus_register_reset(struct mdio_device *mdiodev)
-{
- struct reset_control *reset;
-
- reset = reset_control_get_optional_exclusive(&mdiodev->dev, "phy");
- if (IS_ERR(reset))
- return PTR_ERR(reset);
-
- mdiodev->reset_ctrl = reset;
-
- return 0;
-}
-
int mdiobus_register_device(struct mdio_device *mdiodev)
{
int err;
@@ -68,16 +41,9 @@ int mdiobus_register_device(struct mdio_device *mdiodev)
return -EBUSY;
if (mdiodev->flags & MDIO_DEVICE_FLAG_PHY) {
- err = mdiobus_register_gpiod(mdiodev);
+ err = mdio_device_register_reset(mdiodev);
if (err)
return err;
-
- err = mdiobus_register_reset(mdiodev);
- if (err)
- return err;
-
- /* Assert the reset signal */
- mdio_device_reset(mdiodev, 1);
}
mdiodev->bus->mdio_map[mdiodev->addr] = mdiodev;
@@ -91,8 +57,7 @@ int mdiobus_unregister_device(struct mdio_device *mdiodev)
if (mdiodev->bus->mdio_map[mdiodev->addr] != mdiodev)
return -EINVAL;
- gpiod_put(mdiodev->reset_gpio);
- reset_control_put(mdiodev->reset_ctrl);
+ mdio_device_unregister_reset(mdiodev);
mdiodev->bus->mdio_map[mdiodev->addr] = NULL;
diff --git a/drivers/net/phy/mdio_device.c b/drivers/net/phy/mdio_device.c
index f64176e0e..fb1cb7a26 100644
--- a/drivers/net/phy/mdio_device.c
+++ b/drivers/net/phy/mdio_device.c
@@ -74,6 +74,58 @@ struct mdio_device *mdio_device_create(struct mii_bus *bus, int addr)
}
EXPORT_SYMBOL(mdio_device_create);
+/**
+ * mdio_device_register_reset - Read and initialize the reset properties of
+ * an mdio device
+ * @mdiodev: mdio_device structure
+ *
+ * Return: Zero if successful, negative error code on failure
+ */
+int mdio_device_register_reset(struct mdio_device *mdiodev)
+{
+ struct reset_control *reset;
+
+ /* Read optional firmware properties */
+ fwnode_property_read_u32(dev_fwnode(&mdiodev->dev), "reset-assert-us",
+ &mdiodev->reset_assert_delay);
+ fwnode_property_read_u32(dev_fwnode(&mdiodev->dev), "reset-deassert-us",
+ &mdiodev->reset_deassert_delay);
+
+ /* reset-gpio, bring up deasserted */
+ mdiodev->reset_gpio = gpiod_get_optional(&mdiodev->dev, "reset",
+ GPIOD_OUT_LOW);
+
+ if (IS_ERR(mdiodev->reset_gpio))
+ return PTR_ERR(mdiodev->reset_gpio);
+
+ if (mdiodev->reset_gpio)
+ gpiod_set_consumer_name(mdiodev->reset_gpio, "PHY reset");
+
+ reset = reset_control_get_optional_exclusive(&mdiodev->dev, "phy");
+ if (IS_ERR(reset))
+ return PTR_ERR(reset);
+
+ mdiodev->reset_ctrl = reset;
+
+ /* Assert the reset signal */
+ mdio_device_reset(mdiodev, 1);
+
+ return 0;
+}
+EXPORT_SYMBOL(mdio_device_register_reset);
+
+/**
+ * mdio_device_unregister_reset - uninitialize the reset properties of
+ * an mdio device
+ * @mdiodev: mdio_device structure
+ */
+void mdio_device_unregister_reset(struct mdio_device *mdiodev)
+{
+ gpiod_put(mdiodev->reset_gpio);
+ reset_control_put(mdiodev->reset_ctrl);
+}
+EXPORT_SYMBOL(mdio_device_unregister_reset);
+
/**
* mdio_device_register - Register the mdio device on the MDIO bus
* @mdiodev: mdio_device structure to be added to the MDIO bus
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index 42d6d47e4..d81b63fc7 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -90,6 +90,8 @@ static inline void *mdiodev_get_drvdata(struct mdio_device *mdio)
void mdio_device_free(struct mdio_device *mdiodev);
struct mdio_device *mdio_device_create(struct mii_bus *bus, int addr);
+int mdio_device_register_reset(struct mdio_device *mdiodev);
+void mdio_device_unregister_reset(struct mdio_device *mdiodev);
int mdio_device_register(struct mdio_device *mdiodev);
void mdio_device_remove(struct mdio_device *mdiodev);
void mdio_device_reset(struct mdio_device *mdiodev, int value);
--
2.39.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net-next v4 2/4] net: mdio: change property read from fwnode_property_read_u32() to device_property_read_u32()
2025-10-22 9:08 [PATCH net-next v4 0/4] net: mdio: implement optional PHY reset before MDIO access Buday Csaba
2025-10-22 9:08 ` [PATCH net-next v4 1/4] net: mdio: common handling of phy reset properties Buday Csaba
@ 2025-10-22 9:08 ` Buday Csaba
2025-10-22 9:08 ` [PATCH net-next v4 3/4] net: mdio: introduce mdio_device_has_reset() Buday Csaba
2025-10-22 9:08 ` [PATCH net-next v4 4/4] net: mdio: reset PHY before attempting to access registers in fwnode_mdiobus_register_phy Buday Csaba
3 siblings, 0 replies; 8+ messages in thread
From: Buday Csaba @ 2025-10-22 9:08 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Philipp Zabel, netdev,
linux-kernel
Cc: Buday Csaba
Changed fwnode_property_read_u32() in mdio_device_register_reset()
to device_property_read_u32(), which is more appropriate here.
Signed-off-by: Buday Csaba <buday.csaba@prolan.hu>
---
V3 -> V4: unmodified
V2 -> V3: unmodified
V1 -> V2: added new patch based on maintainer request
---
drivers/net/phy/mdio_device.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/mdio_device.c b/drivers/net/phy/mdio_device.c
index fb1cb7a26..5d39b25b7 100644
--- a/drivers/net/phy/mdio_device.c
+++ b/drivers/net/phy/mdio_device.c
@@ -86,9 +86,9 @@ int mdio_device_register_reset(struct mdio_device *mdiodev)
struct reset_control *reset;
/* Read optional firmware properties */
- fwnode_property_read_u32(dev_fwnode(&mdiodev->dev), "reset-assert-us",
+ device_property_read_u32(&mdiodev->dev, "reset-assert-us",
&mdiodev->reset_assert_delay);
- fwnode_property_read_u32(dev_fwnode(&mdiodev->dev), "reset-deassert-us",
+ device_property_read_u32(&mdiodev->dev, "reset-deassert-us",
&mdiodev->reset_deassert_delay);
/* reset-gpio, bring up deasserted */
--
2.39.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net-next v4 3/4] net: mdio: introduce mdio_device_has_reset()
2025-10-22 9:08 [PATCH net-next v4 0/4] net: mdio: implement optional PHY reset before MDIO access Buday Csaba
2025-10-22 9:08 ` [PATCH net-next v4 1/4] net: mdio: common handling of phy reset properties Buday Csaba
2025-10-22 9:08 ` [PATCH net-next v4 2/4] net: mdio: change property read from fwnode_property_read_u32() to device_property_read_u32() Buday Csaba
@ 2025-10-22 9:08 ` Buday Csaba
2025-10-22 9:08 ` [PATCH net-next v4 4/4] net: mdio: reset PHY before attempting to access registers in fwnode_mdiobus_register_phy Buday Csaba
3 siblings, 0 replies; 8+ messages in thread
From: Buday Csaba @ 2025-10-22 9:08 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Philipp Zabel, netdev,
linux-kernel
Cc: Buday Csaba
Introduced mdio_device_has_reset() to check if an MDIO
device has any reset properties defined.
Signed-off-by: Buday Csaba <buday.csaba@prolan.hu>
---
V3 -> V4: new commit
---
drivers/net/phy/mdio_device.c | 13 +++++++++++++
include/linux/mdio.h | 1 +
2 files changed, 14 insertions(+)
diff --git a/drivers/net/phy/mdio_device.c b/drivers/net/phy/mdio_device.c
index 5d39b25b7..b1122bab8 100644
--- a/drivers/net/phy/mdio_device.c
+++ b/drivers/net/phy/mdio_device.c
@@ -170,6 +170,19 @@ void mdio_device_remove(struct mdio_device *mdiodev)
}
EXPORT_SYMBOL(mdio_device_remove);
+/**
+ * mdio_device_has_reset - Check if an MDIO device has reset properties defined
+ * @mdiodev: mdio_device structure
+ *
+ * Return: non-zero if the device has a reset GPIO or reset controller,
+ * zero otherwise.
+ */
+int mdio_device_has_reset(struct mdio_device *mdiodev)
+{
+ return (mdiodev->reset_gpio || mdiodev->reset_ctrl);
+}
+EXPORT_SYMBOL(mdio_device_has_reset);
+
void mdio_device_reset(struct mdio_device *mdiodev, int value)
{
unsigned int d;
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index d81b63fc7..83cfc051e 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -92,6 +92,7 @@ void mdio_device_free(struct mdio_device *mdiodev);
struct mdio_device *mdio_device_create(struct mii_bus *bus, int addr);
int mdio_device_register_reset(struct mdio_device *mdiodev);
void mdio_device_unregister_reset(struct mdio_device *mdiodev);
+int mdio_device_has_reset(struct mdio_device *mdiodev);
int mdio_device_register(struct mdio_device *mdiodev);
void mdio_device_remove(struct mdio_device *mdiodev);
void mdio_device_reset(struct mdio_device *mdiodev, int value);
--
2.39.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net-next v4 4/4] net: mdio: reset PHY before attempting to access registers in fwnode_mdiobus_register_phy
2025-10-22 9:08 [PATCH net-next v4 0/4] net: mdio: implement optional PHY reset before MDIO access Buday Csaba
` (2 preceding siblings ...)
2025-10-22 9:08 ` [PATCH net-next v4 3/4] net: mdio: introduce mdio_device_has_reset() Buday Csaba
@ 2025-10-22 9:08 ` Buday Csaba
2025-10-29 1:26 ` Jakub Kicinski
3 siblings, 1 reply; 8+ messages in thread
From: Buday Csaba @ 2025-10-22 9:08 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Philipp Zabel, netdev,
linux-kernel
Cc: Buday Csaba
When the ID of an Ethernet PHY is not provided by the 'compatible'
string in the device tree, its actual ID is read via the MDIO bus.
For some PHYs this could be unsafe, since a hard reset may be
necessary to safely access the MDIO registers.
This patch adds a fallback mechanism for these devices:
when reading the ID fails, the reset will be asserted, and the
ID read is retried.
When the reset is asserted, an info level message is printed.
Signed-off-by: Buday Csaba <buday.csaba@prolan.hu>
---
V3 -> V4: dropped DT property `phy-id-read-needs-reset` to control
the reset behaviour, asserting reset as a fallback
mechanism instead. Added info level message for resetting.
V2 -> V3: kernel-doc replaced with a comment (fixed warning)
V1 -> V2:
- renamed fwnode_reset_phy_before_probe() to
fwnode_reset_phy()
- added kernel-doc for fwnode_reset_phy()
- improved error handling in fwnode_reset_phy()
---
drivers/net/mdio/fwnode_mdio.c | 41 +++++++++++++++++++++++++++++++++-
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index ba7091518..623120e3b 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -114,6 +114,40 @@ int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
}
EXPORT_SYMBOL(fwnode_mdiobus_phy_device_register);
+/* Hard-reset a PHY before registration */
+static int fwnode_reset_phy(struct mii_bus *bus, u32 addr,
+ struct fwnode_handle *phy_node)
+{
+ struct mdio_device *tmpdev;
+ int err;
+
+ tmpdev = mdio_device_create(bus, addr);
+ if (IS_ERR(tmpdev))
+ return PTR_ERR(tmpdev);
+
+ fwnode_handle_get(phy_node);
+ device_set_node(&tmpdev->dev, phy_node);
+ err = mdio_device_register_reset(tmpdev);
+ if (err) {
+ mdio_device_free(tmpdev);
+ return err;
+ }
+
+ if (mdio_device_has_reset(tmpdev)) {
+ dev_info(&bus->dev,
+ "PHY device at address %d not detected, resetting PHY.",
+ addr);
+ mdio_device_reset(tmpdev, 1);
+ mdio_device_reset(tmpdev, 0);
+ }
+
+ mdio_device_unregister_reset(tmpdev);
+ mdio_device_free(tmpdev);
+ fwnode_handle_put(phy_node);
+
+ return 0;
+}
+
int fwnode_mdiobus_register_phy(struct mii_bus *bus,
struct fwnode_handle *child, u32 addr)
{
@@ -129,8 +163,13 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
return PTR_ERR(mii_ts);
is_c45 = fwnode_device_is_compatible(child, "ethernet-phy-ieee802.3-c45");
- if (is_c45 || fwnode_get_phy_id(child, &phy_id))
+ if (is_c45 || fwnode_get_phy_id(child, &phy_id)) {
phy = get_phy_device(bus, addr, is_c45);
+ if (IS_ERR(phy)) {
+ fwnode_reset_phy(bus, addr, child);
+ phy = get_phy_device(bus, addr, is_c45);
+ }
+ }
else
phy = phy_device_create(bus, addr, phy_id, 0, NULL);
if (IS_ERR(phy)) {
--
2.39.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v4 1/4] net: mdio: common handling of phy reset properties
2025-10-22 9:08 ` [PATCH net-next v4 1/4] net: mdio: common handling of phy reset properties Buday Csaba
@ 2025-10-29 1:22 ` Jakub Kicinski
0 siblings, 0 replies; 8+ messages in thread
From: Jakub Kicinski @ 2025-10-29 1:22 UTC (permalink / raw)
To: Buday Csaba
Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Paolo Abeni, Philipp Zabel, netdev, linux-kernel
Let me give you some nit picky comments, maybe v5 will have more luck
attracting area experts :(
On Wed, 22 Oct 2025 11:08:50 +0200 Buday Csaba wrote:
> Reset properties of an `mdio_device` are initialized in multiple
> source files and multiple functions:
> - `reset_assert_delay` and `reset_deassert_delay` are in
> fwnode_mdio.c
> - `reset_gpio` and `reset_ctrl` are in mdio_bus.c, but handled by
> different functions
>
> This patch unifies the handling of all these properties into two
> functions.
Use imperative mood. "This patch unifies" -> "Unify"
Please check all commits msgs for this (eg glancing at patch 2
"Changed" -> "Change")
> mdiobus_register_gpiod() and mdiobus_register_reset() are removed,
> while mdio_device_register_reset() and mdio_device_unregister_reset()
> are introduced instead.
> These functions handle both reset-controllers and reset-gpios, and
> also read the corresponding properties from the device tree.
> These changes should make tracking the reset properties easier.
> + /* reset-gpio, bring up deasserted */
> + mdiodev->reset_gpio = gpiod_get_optional(&mdiodev->dev, "reset",
> + GPIOD_OUT_LOW);
> +
no empty lines between call and its error check pls
> + if (IS_ERR(mdiodev->reset_gpio))
> + return PTR_ERR(mdiodev->reset_gpio);
> +
> + if (mdiodev->reset_gpio)
> + gpiod_set_consumer_name(mdiodev->reset_gpio, "PHY reset");
> +
> + reset = reset_control_get_optional_exclusive(&mdiodev->dev, "phy");
> + if (IS_ERR(reset))
> + return PTR_ERR(reset);
> +
> + mdiodev->reset_ctrl = reset;
> +
> + /* Assert the reset signal */
> + mdio_device_reset(mdiodev, 1);
> +
> + return 0;
> +}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v4 4/4] net: mdio: reset PHY before attempting to access registers in fwnode_mdiobus_register_phy
2025-10-22 9:08 ` [PATCH net-next v4 4/4] net: mdio: reset PHY before attempting to access registers in fwnode_mdiobus_register_phy Buday Csaba
@ 2025-10-29 1:26 ` Jakub Kicinski
2025-10-29 7:24 ` Buday Csaba
0 siblings, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2025-10-29 1:26 UTC (permalink / raw)
To: Buday Csaba
Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Paolo Abeni, Philipp Zabel, netdev, linux-kernel
On Wed, 22 Oct 2025 11:08:53 +0200 Buday Csaba wrote:
> +/* Hard-reset a PHY before registration */
> +static int fwnode_reset_phy(struct mii_bus *bus, u32 addr,
> + struct fwnode_handle *phy_node)
> +{
> + struct mdio_device *tmpdev;
> + int err;
> +
> + tmpdev = mdio_device_create(bus, addr);
> + if (IS_ERR(tmpdev))
> + return PTR_ERR(tmpdev);
> +
> + fwnode_handle_get(phy_node);
> + device_set_node(&tmpdev->dev, phy_node);
> + err = mdio_device_register_reset(tmpdev);
> + if (err) {
> + mdio_device_free(tmpdev);
> + return err;
Should we worry about -EPROBE_DEFER on any of the error paths here?
If not maybe consider making this function void? We can add errors
back if the caller starts to care.
> + }
> +
> + if (mdio_device_has_reset(tmpdev)) {
> + dev_info(&bus->dev,
> + "PHY device at address %d not detected, resetting PHY.",
> + addr);
IDK if this is still strictly necessary but I guess \n at the end of
the info msg would be idiomatic
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v4 4/4] net: mdio: reset PHY before attempting to access registers in fwnode_mdiobus_register_phy
2025-10-29 1:26 ` Jakub Kicinski
@ 2025-10-29 7:24 ` Buday Csaba
0 siblings, 0 replies; 8+ messages in thread
From: Buday Csaba @ 2025-10-29 7:24 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Paolo Abeni, Philipp Zabel, netdev, linux-kernel
On Tue, Oct 28, 2025 at 06:26:05PM -0700, Jakub Kicinski wrote:
> On Wed, 22 Oct 2025 11:08:53 +0200 Buday Csaba wrote:
> > +/* Hard-reset a PHY before registration */
> > +static int fwnode_reset_phy(struct mii_bus *bus, u32 addr,
> > + struct fwnode_handle *phy_node)
> > +{
> > + struct mdio_device *tmpdev;
> > + int err;
> > +
> > + tmpdev = mdio_device_create(bus, addr);
> > + if (IS_ERR(tmpdev))
> > + return PTR_ERR(tmpdev);
> > +
> > + fwnode_handle_get(phy_node);
> > + device_set_node(&tmpdev->dev, phy_node);
> > + err = mdio_device_register_reset(tmpdev);
> > + if (err) {
> > + mdio_device_free(tmpdev);
> > + return err;
>
> Should we worry about -EPROBE_DEFER on any of the error paths here?
> If not maybe consider making this function void? We can add errors
> back if the caller starts to care.
That is a very valid point, thanks! I think I can handle that correctly,
by propagating that (or any other) error codes to the caller of
fwnode_mdiobus_register_phy(). fwnode_reset_phy() does nothing that
would not be expected to occur during the normal registration, so if
it fails here, it would fail later as well.
>
> > + }
> > +
> > + if (mdio_device_has_reset(tmpdev)) {
> > + dev_info(&bus->dev,
> > + "PHY device at address %d not detected, resetting PHY.",
> > + addr);
>
> IDK if this is still strictly necessary but I guess \n at the end of
> the info msg would be idiomatic
>
I will separate the error message to another patch, then the maintainers
can decide wheter to merge it or not.
Thanks for the feedback!
I should be able to send v5 soon.
Csaba
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-10-29 7:24 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-22 9:08 [PATCH net-next v4 0/4] net: mdio: implement optional PHY reset before MDIO access Buday Csaba
2025-10-22 9:08 ` [PATCH net-next v4 1/4] net: mdio: common handling of phy reset properties Buday Csaba
2025-10-29 1:22 ` Jakub Kicinski
2025-10-22 9:08 ` [PATCH net-next v4 2/4] net: mdio: change property read from fwnode_property_read_u32() to device_property_read_u32() Buday Csaba
2025-10-22 9:08 ` [PATCH net-next v4 3/4] net: mdio: introduce mdio_device_has_reset() Buday Csaba
2025-10-22 9:08 ` [PATCH net-next v4 4/4] net: mdio: reset PHY before attempting to access registers in fwnode_mdiobus_register_phy Buday Csaba
2025-10-29 1:26 ` Jakub Kicinski
2025-10-29 7:24 ` Buday Csaba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).