[PATCH v1 net] net: lan743x: Restore SGMII CTRL register on resume

Thangaraj Samynathan posted 1 patch 7 months, 1 week ago
drivers/net/ethernet/microchip/lan743x_main.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
[PATCH v1 net] net: lan743x: Restore SGMII CTRL register on resume
Posted by Thangaraj Samynathan 7 months, 1 week ago
SGMII_CTRL register, which specifies the active interface, was not 
properly restored when resuming from suspend. This led to incorrect
interface selection after resume particularly in scenarios involving
the FPGA.

To fix this:
- Move the SGMII_CTRL setup out of the probe function.
- Initialize the register in the hardware initialization helper function,
which is called during both device initialization and resume.

This ensures the interface configuration is consistently restored after
suspend/resume cycles.

Fixes: a46d9d37c4f4f ("net: lan743x: Add support for SGMII interface")
Signed-off-by: Thangaraj Samynathan <thangaraj.s@microchip.com>
---
 drivers/net/ethernet/microchip/lan743x_main.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index 880681085df2..7e71579632f3 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -3495,6 +3495,7 @@ static int lan743x_hardware_init(struct lan743x_adapter *adapter,
 				 struct pci_dev *pdev)
 {
 	struct lan743x_tx *tx;
+	u32 sgmii_ctl;
 	int index;
 	int ret;
 
@@ -3507,6 +3508,15 @@ static int lan743x_hardware_init(struct lan743x_adapter *adapter,
 		spin_lock_init(&adapter->eth_syslock_spinlock);
 		mutex_init(&adapter->sgmii_rw_lock);
 		pci11x1x_set_rfe_rd_fifo_threshold(adapter);
+		sgmii_ctl = lan743x_csr_read(adapter, SGMII_CTL);
+		if (adapter->is_sgmii_en) {
+			sgmii_ctl |= SGMII_CTL_SGMII_ENABLE_;
+			sgmii_ctl &= ~SGMII_CTL_SGMII_POWER_DN_;
+		} else {
+			sgmii_ctl &= ~SGMII_CTL_SGMII_ENABLE_;
+			sgmii_ctl |= SGMII_CTL_SGMII_POWER_DN_;
+		}
+		lan743x_csr_write(adapter, SGMII_CTL, sgmii_ctl);
 	} else {
 		adapter->max_tx_channels = LAN743X_MAX_TX_CHANNELS;
 		adapter->used_tx_channels = LAN743X_USED_TX_CHANNELS;
@@ -3558,7 +3568,6 @@ static int lan743x_hardware_init(struct lan743x_adapter *adapter,
 
 static int lan743x_mdiobus_init(struct lan743x_adapter *adapter)
 {
-	u32 sgmii_ctl;
 	int ret;
 
 	adapter->mdiobus = devm_mdiobus_alloc(&adapter->pdev->dev);
@@ -3570,10 +3579,6 @@ static int lan743x_mdiobus_init(struct lan743x_adapter *adapter)
 	adapter->mdiobus->priv = (void *)adapter;
 	if (adapter->is_pci11x1x) {
 		if (adapter->is_sgmii_en) {
-			sgmii_ctl = lan743x_csr_read(adapter, SGMII_CTL);
-			sgmii_ctl |= SGMII_CTL_SGMII_ENABLE_;
-			sgmii_ctl &= ~SGMII_CTL_SGMII_POWER_DN_;
-			lan743x_csr_write(adapter, SGMII_CTL, sgmii_ctl);
 			netif_dbg(adapter, drv, adapter->netdev,
 				  "SGMII operation\n");
 			adapter->mdiobus->read = lan743x_mdiobus_read_c22;
@@ -3584,10 +3589,6 @@ static int lan743x_mdiobus_init(struct lan743x_adapter *adapter)
 			netif_dbg(adapter, drv, adapter->netdev,
 				  "lan743x-mdiobus-c45\n");
 		} else {
-			sgmii_ctl = lan743x_csr_read(adapter, SGMII_CTL);
-			sgmii_ctl &= ~SGMII_CTL_SGMII_ENABLE_;
-			sgmii_ctl |= SGMII_CTL_SGMII_POWER_DN_;
-			lan743x_csr_write(adapter, SGMII_CTL, sgmii_ctl);
 			netif_dbg(adapter, drv, adapter->netdev,
 				  "RGMII operation\n");
 			// Only C22 support when RGMII I/F
-- 
2.25.1
Re: [PATCH v1 net] net: lan743x: Restore SGMII CTRL register on resume
Posted by Jakub Kicinski 7 months ago
On Fri, 16 May 2025 09:27:19 +0530 Thangaraj Samynathan wrote:
> SGMII_CTRL register, which specifies the active interface, was not 
> properly restored when resuming from suspend. This led to incorrect
> interface selection after resume particularly in scenarios involving
> the FPGA.
> 
> To fix this:
> - Move the SGMII_CTRL setup out of the probe function.
> - Initialize the register in the hardware initialization helper function,
> which is called during both device initialization and resume.
> 
> This ensures the interface configuration is consistently restored after
> suspend/resume cycles.

Is there a reason you're not CCing Raju Lakkaraju on this?
Having a review tag from the author of the change under Fixes 
is always great.
Re: [PATCH v1 net] net: lan743x: Restore SGMII CTRL register on resume
Posted by Thangaraj.S@microchip.com 7 months ago
Hi Jakub,
On Fri, 2025-05-16 at 16:40 -0700, Jakub Kicinski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Fri, 16 May 2025 09:27:19 +0530 Thangaraj Samynathan wrote:
> > SGMII_CTRL register, which specifies the active interface, was not
> > properly restored when resuming from suspend. This led to incorrect
> > interface selection after resume particularly in scenarios
> > involving
> > the FPGA.
> > 
> > To fix this:
> > - Move the SGMII_CTRL setup out of the probe function.
> > - Initialize the register in the hardware initialization helper
> > function,
> > which is called during both device initialization and resume.
> > 
> > This ensures the interface configuration is consistently restored
> > after
> > suspend/resume cycles.
> 
> Is there a reason you're not CCing Raju Lakkaraju on this?
> Having a review tag from the author of the change under Fixes
> is always great.
Thanks for pointing this out.
Raju Lakkaraju is no longer with the company, and as I am currentlymanaging this driver, I have not included him in the CC.

Thanks,
Thangaraj Samynathan


Re: [PATCH v1 net] net: lan743x: Restore SGMII CTRL register on resume
Posted by Jakub Kicinski 7 months ago
On Mon, 19 May 2025 09:23:56 +0000 Thangaraj.S@microchip.com wrote:
> Raju Lakkaraju is no longer with the company, and as I am
> currentlymanaging this driver, I have not included him in the CC.

Understood, let me add their address to the ignore list.