Received-SPF: pass (google.com: domain of linux-kernel+bounces-161151-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Sat, 27 Apr 2024 21:35:06 +0200
From: =?iso-8859-1?Q?Ram=F3n?= Nordin Rodriguez <ramon.nordin.rodriguez@ferroamp.se>
To: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com>
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, horms@kernel.org, saeedm@nvidia.com,
	anthony.l.nguyen@intel.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, andrew@lunn.ch, corbet@lwn.net,
	linux-doc@vger.kernel.org, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org, horatiu.vultur@microchip.com,
	ruanjinjie@huawei.com, steen.hegelund@microchip.com,
	vladimir.oltean@nxp.com, UNGLinuxDriver@microchip.com,
	Thorsten.Kummermehr@microchip.com, Pier.Beruto@onsemi.com,
	Selvamani.Rajagopal@onsemi.com, Nicolas.Ferre@microchip.com,
	benjamin.bigler@bernformulastudent.ch
Subject: Re: [PATCH net-next v4 11/12] microchip: lan865x: add driver support
 for Microchip's LAN865X MAC-PHY
Message-ID: <Zi1Tang5RQMmEFdx@builder>
References: <20240418125648.372526-1-Parthiban.Veerasooran@microchip.com>
 <20240418125648.372526-12-Parthiban.Veerasooran@microchip.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20240418125648.372526-12-Parthiban.Veerasooran@microchip.com>

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 :)