drivers/net/mdio/fwnode_mdio.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
When the reset gpio is defined within the node of the device tree
describing the PHY, the reset is initialized and managed only after
calling the fwnode_mdiobus_phy_device_register function.
However, before calling it, the MDIO communication is checked by the
get_phy_device function.
When this happen and the reset GPIO was somehow previously set down,
the get_phy_device function fails, preventing the PHY detection.
These changes force the deassert of the MDIO reset signal before
checking the MDIO channel.
The PHY may require a minimum deassert time before being responsive:
use a reasonable sleep time after forcing the deassert of the MDIO
reset signal.
Once done, free the gpio descriptor to allow managing it later.
Signed-off-by: Pierluigi Passaro <pierluigi.p@variscite.com>
Signed-off-by: FrancescoFerraro <francesco.f@variscite.com>
---
drivers/net/mdio/fwnode_mdio.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index b782c35c4ac1..1f4b8c4c1f60 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -8,6 +8,8 @@
#include <linux/acpi.h>
#include <linux/fwnode_mdio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
#include <linux/of.h>
#include <linux/phy.h>
#include <linux/pse-pd/pse.h>
@@ -118,6 +120,8 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
bool is_c45 = false;
u32 phy_id;
int rc;
+ int reset_deassert_delay = 0;
+ struct gpio_desc *reset_gpio;
psec = fwnode_find_pse_control(child);
if (IS_ERR(psec))
@@ -134,10 +138,31 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
if (rc >= 0)
is_c45 = true;
+ reset_gpio = fwnode_gpiod_get_index(child, "reset", 0, GPIOD_OUT_LOW, "PHY reset");
+ if (reset_gpio == ERR_PTR(-EPROBE_DEFER)) {
+ dev_dbg(&bus->dev, "reset signal for PHY@%u not ready\n", addr);
+ return -EPROBE_DEFER;
+ } else if (IS_ERR(reset_gpio)) {
+ if (reset_gpio == ERR_PTR(-ENOENT))
+ dev_dbg(&bus->dev, "reset signal for PHY@%u not defined\n", addr);
+ else
+ dev_dbg(&bus->dev, "failed to request reset for PHY@%u, error %ld\n", addr, PTR_ERR(reset_gpio));
+ reset_gpio = NULL;
+ } else {
+ dev_dbg(&bus->dev, "deassert reset signal for PHY@%u\n", addr);
+ fwnode_property_read_u32(child, "reset-deassert-us",
+ &reset_deassert_delay);
+ if (reset_deassert_delay)
+ fsleep(reset_deassert_delay);
+ }
+
if (is_c45 || fwnode_get_phy_id(child, &phy_id))
phy = get_phy_device(bus, addr, is_c45);
else
phy = phy_device_create(bus, addr, phy_id, 0, NULL);
+
+ gpiochip_free_own_desc(reset_gpio);
+
if (IS_ERR(phy)) {
rc = PTR_ERR(phy);
goto clean_mii_ts;
--
2.37.2On Sun, Jan 15, 2023 at 10:37:46PM +0100, Pierluigi Passaro wrote:
> When the reset gpio is defined within the node of the device tree
> describing the PHY, the reset is initialized and managed only after
> calling the fwnode_mdiobus_phy_device_register function.
> However, before calling it, the MDIO communication is checked by the
> get_phy_device function.
> When this happen and the reset GPIO was somehow previously set down,
> the get_phy_device function fails, preventing the PHY detection.
> These changes force the deassert of the MDIO reset signal before
> checking the MDIO channel.
> The PHY may require a minimum deassert time before being responsive:
> use a reasonable sleep time after forcing the deassert of the MDIO
> reset signal.
> Once done, free the gpio descriptor to allow managing it later.
>
> Signed-off-by: Pierluigi Passaro <pierluigi.p@variscite.com>
> Signed-off-by: FrancescoFerraro <francesco.f@variscite.com>
> ---
> drivers/net/mdio/fwnode_mdio.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
> index b782c35c4ac1..1f4b8c4c1f60 100644
> --- a/drivers/net/mdio/fwnode_mdio.c
> +++ b/drivers/net/mdio/fwnode_mdio.c
> @@ -8,6 +8,8 @@
>
> #include <linux/acpi.h>
> #include <linux/fwnode_mdio.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/gpio/driver.h>
> #include <linux/of.h>
> #include <linux/phy.h>
> #include <linux/pse-pd/pse.h>
> @@ -118,6 +120,8 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
> bool is_c45 = false;
> u32 phy_id;
> int rc;
> + int reset_deassert_delay = 0;
nit: it looks like scope of reset_deassert_delay could be reduced
to the else clause where it is used.
> + struct gpio_desc *reset_gpio;
nit: reverse xmas tree for local variable declarations
...
Also, if reposting, the target tree for this should be in the subject.
Assuming net-next, that would mean "[PATCH net-next v3]"
On Mon, Jan 16, 2023 at 12:10 PM Simon Horman <simon.horman@corigine.com> wrote: > On Sun, Jan 15, 2023 at 10:37:46PM +0100, Pierluigi Passaro wrote: > > When the reset gpio is defined within the node of the device tree > > describing the PHY, the reset is initialized and managed only after > > calling the fwnode_mdiobus_phy_device_register function. > > However, before calling it, the MDIO communication is checked by the > > get_phy_device function. > > When this happens and the reset GPIO was somehow previously set down, > > the get_phy_device function fails, preventing the PHY detection. > > These changes force the deassert of the MDIO reset signal before > > checking the MDIO channel. > > The PHY may require a minimum deassert time before being responsive: > > use a reasonable sleep time after forcing the deassert of the MDIO > > reset signal. > > Once done, free the gpio descriptor to allow managing it later. > > > > Signed-off-by: Pierluigi Passaro <pierluigi.p@variscite.com> > > Signed-off-by: FrancescoFerraro <francesco.f@variscite.com> > > --- > > drivers/net/mdio/fwnode_mdio.c | 25 +++++++++++++++++++++++++ > > 1 file changed, 25 insertions(+) > > > > diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c > > index b782c35c4ac1..1f4b8c4c1f60 100644 > > --- a/drivers/net/mdio/fwnode_mdio.c > > +++ b/drivers/net/mdio/fwnode_mdio.c > > @@ -8,6 +8,8 @@ > > > > #include <linux/acpi.h> > > #include <linux/fwnode_mdio.h> > > +#include <linux/gpio/consumer.h> > > +#include <linux/gpio/driver.h> > > #include <linux/of.h> > > #include <linux/phy.h> > > #include <linux/pse-pd/pse.h> > > @@ -118,6 +120,8 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus, > > bool is_c45 = false; > > u32 phy_id; > > int rc; > > + int reset_deassert_delay = 0; > > nit: it looks like scope of reset_deassert_delay could be reduced > to the else clause where it is used. > no problem. > > > + struct gpio_desc *reset_gpio; > > nit: reverse xmas tree for local variable declarations > I'm worried I'm asking something stupid: what do you mean by "reverse xmas tree" ?> ... > > Also, if reposting, the target tree for this should be in the subject. > Assuming net-next, that would mean "[PATCH net-next v3]" > no problem
On Mon, Jan 16, 2023 at 01:13:49PM +0000, Pierluigi Passaro wrote: > I'm worried I'm asking something stupid: what do you mean by > "reverse xmas tree" ?> ... Ordering them so that the longest is first, followed by the next longest all the way down to the shortest. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
On Mon, Jan 16, 2023 at 01:36:17PM +0000, Russell King (Oracle) wrote: > On Mon, Jan 16, 2023 at 01:13:49PM +0000, Pierluigi Passaro wrote: > > I'm worried I'm asking something stupid: what do you mean by > > "reverse xmas tree" ?> ... > > Ordering them so that the longest is first, followed by the next longest > all the way down to the shortest. Yes, something a bit like this.
© 2016 - 2025 Red Hat, Inc.