Subject: Re: [PATCH net-next v4 11/12] microchip: lan865x: add driver support for Microchip's LAN865X MAC-PHY

From: Ramón Nordin Rodriguez <[email-masked]>
Date: Sat, 27 Apr 2024 21:35:06 +0200

I'm running a dual lan8650 setup, neither IC passed the sw reset in the
oa_tc.c module, I need to pull the reset pin low to reset the pin before
the rest of the init stuff happens.

The datasheet recommends not doing a sw reset, excerpt from section
4.1.1.3 Software Reset
"Note: The SW_RESET bit of the Clause 22 Basic Control register will reset only the internal PHY, not
the entire device. This PHY only reset is not recommended for use. If such a reset is detected, by
reading the RESETC bit of the STS2 register, reset the entire device."

Doing a hw reset followed by a sw reset seems to work fine though. I
added the folloing patch to get things moving.

diff --git a/drivers/net/ethernet/microchip/lan865x/lan865x.c b/drivers/net/ethernet/microchip/lan865x/lan865x.c
index 72a663f14f50..993c4f9dec7e 100644
--- a/drivers/net/ethernet/microchip/lan865x/lan865x.c
+++ b/drivers/net/ethernet/microchip/lan865x/lan865x.c
@@ -9,6 +9,7 @@
 #include <linux/kernel.h>
 #include <linux/phy.h>
 #include <linux/oa_tc6.h>
+#include <linux/gpio/driver.h>

 #define DRV_NAME                       "lan865x"

@@ -36,6 +37,7 @@ struct lan865x_priv {
        struct net_device *netdev;
        struct spi_device *spi;
        struct oa_tc6 *tc6;
+       struct gpio_desc *reset_gpio;
 };

 static int lan865x_set_hw_macaddr_low_bytes(struct oa_tc6 *tc6, const u8 *mac)
@@ -283,6 +285,29 @@ static int lan865x_set_zarfe(struct lan865x_priv *priv)
        return oa_tc6_write_register(priv->tc6, OA_TC6_REG_CONFIG0, regval);
 }

+static int lan865x_probe_reset_gpio(struct lan865x_priv *priv)
+{
+       priv->reset_gpio = devm_gpiod_get_optional(&priv->spi->dev,
+                                           "reset",
+                                           GPIOD_OUT_HIGH);
+       if (IS_ERR(priv->reset_gpio)) {
+               dev_err(&priv->spi->dev, "failed to parse reset gpio from dt");
+               return PTR_ERR(priv->reset_gpio);
+       }
+
+       return 0;
+}
+
+static void lan865x_hw_reset(struct lan865x_priv *priv)
+{
+       dev_info(&priv->spi->dev, "resetting device");
+       gpiod_set_value(priv->reset_gpio, 1);
+       // the datasheet specifies a minimum 5µs hold time
+       usleep_range(5,10);
+       gpiod_set_value(priv->reset_gpio, 0);
+       dev_info(&priv->spi->dev, "reset completed");
+}
+
 static int lan865x_probe(struct spi_device *spi)
 {
        struct net_device *netdev;
@@ -297,6 +322,9 @@ static int lan865x_probe(struct spi_device *spi)
        priv->netdev = netdev;
        priv->spi = spi;
        spi_set_drvdata(spi, priv);
+       lan865x_probe_reset_gpio(priv);
+       if(priv->reset_gpio)
+               lan865x_hw_reset(priv);
        INIT_WORK(&priv->multicast_work, lan865x_multicast_work_handler);

        priv->tc6 = oa_tc6_init(spi, netdev);

Since the chip does have a HW reset pin I think it would be nice to
at least expose this as an optional dt binding.
Maybe ignore the prints I forgot to remove :)