[PATCH net V4 1/3] net: lan743x: disable WOL upon resume to restore full data path operation

Raju Lakkaraju posted 3 patches 1 year, 8 months ago
There is a newer version of this series
[PATCH net V4 1/3] net: lan743x: disable WOL upon resume to restore full data path operation
Posted by Raju Lakkaraju 1 year, 8 months ago
When Wake-on-LAN (WoL) is active and the system is in suspend mode, triggering
a system event can wake the system from sleep, which may block the data path.
To restore normal data path functionality after waking, disable all wake-up
events. Furthermore, clear all Write 1 to Clear (W1C) status bits by writing
1's to them.

Fixes: 4d94282afd95 ("lan743x: Add power management support")
Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microchip.com>
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
---
Change List:                                                                    
------------                                                                    
V3 -> V4:
  - No change
V2 -> V3:
  - No change
V1 -> V2:
  - Repost - No change
V0 -> V1:
  - Variable "data" change from "int" to "unsigned int"
 drivers/net/ethernet/microchip/lan743x_main.c | 30 ++++++++++++++++---
 drivers/net/ethernet/microchip/lan743x_main.h | 24 +++++++++++++++
 2 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index 6be8a43c908a..6a40b961fafb 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -3575,7 +3575,7 @@ static void lan743x_pm_set_wol(struct lan743x_adapter *adapter)
 
 	/* clear wake settings */
 	pmtctl = lan743x_csr_read(adapter, PMT_CTL);
-	pmtctl |= PMT_CTL_WUPS_MASK_;
+	pmtctl |= PMT_CTL_WUPS_MASK_ | PMT_CTL_RES_CLR_WKP_MASK_;
 	pmtctl &= ~(PMT_CTL_GPIO_WAKEUP_EN_ | PMT_CTL_EEE_WAKEUP_EN_ |
 		PMT_CTL_WOL_EN_ | PMT_CTL_MAC_D3_RX_CLK_OVR_ |
 		PMT_CTL_RX_FCT_RFE_D3_CLK_OVR_ | PMT_CTL_ETH_PHY_WAKE_EN_);
@@ -3710,6 +3710,7 @@ static int lan743x_pm_resume(struct device *dev)
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct lan743x_adapter *adapter = netdev_priv(netdev);
+	u32 data;
 	int ret;
 
 	pci_set_power_state(pdev, PCI_D0);
@@ -3728,6 +3729,30 @@ static int lan743x_pm_resume(struct device *dev)
 		return ret;
 	}
 
+	ret = lan743x_csr_read(adapter, MAC_WK_SRC);
+	netif_info(adapter, drv, adapter->netdev,
+		   "Wakeup source : 0x%08X\n", ret);
+
+	/* Clear the wol configuration and status bits. Note that
+	 * the status bits are "Write One to Clear (W1C)"
+	 */
+	data = MAC_WUCSR_EEE_TX_WAKE_ | MAC_WUCSR_EEE_RX_WAKE_ |
+	       MAC_WUCSR_RFE_WAKE_FR_ | MAC_WUCSR_PFDA_FR_ | MAC_WUCSR_WUFR_ |
+	       MAC_WUCSR_MPR_ | MAC_WUCSR_BCAST_FR_;
+	lan743x_csr_write(adapter, MAC_WUCSR, data);
+
+	data = MAC_WUCSR2_NS_RCD_ | MAC_WUCSR2_ARP_RCD_ |
+	       MAC_WUCSR2_IPV6_TCPSYN_RCD_ | MAC_WUCSR2_IPV4_TCPSYN_RCD_;
+	lan743x_csr_write(adapter, MAC_WUCSR2, data);
+
+	data = MAC_WK_SRC_ETH_PHY_WK_ | MAC_WK_SRC_IPV6_TCPSYN_RCD_WK_ |
+	       MAC_WK_SRC_IPV4_TCPSYN_RCD_WK_ | MAC_WK_SRC_EEE_TX_WK_ |
+	       MAC_WK_SRC_EEE_RX_WK_ | MAC_WK_SRC_RFE_FR_WK_ |
+	       MAC_WK_SRC_PFDA_FR_WK_ | MAC_WK_SRC_MP_FR_WK_ |
+	       MAC_WK_SRC_BCAST_FR_WK_ | MAC_WK_SRC_WU_FR_WK_ |
+	       MAC_WK_SRC_WK_FR_SAVED_;
+	lan743x_csr_write(adapter, MAC_WK_SRC, data);
+
 	/* open netdev when netdev is at running state while resume.
 	 * For instance, it is true when system wakesup after pm-suspend
 	 * However, it is false when system wakes up after suspend GUI menu
@@ -3736,9 +3761,6 @@ static int lan743x_pm_resume(struct device *dev)
 		lan743x_netdev_open(netdev);
 
 	netif_device_attach(netdev);
-	ret = lan743x_csr_read(adapter, MAC_WK_SRC);
-	netif_info(adapter, drv, adapter->netdev,
-		   "Wakeup source : 0x%08X\n", ret);
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/microchip/lan743x_main.h b/drivers/net/ethernet/microchip/lan743x_main.h
index 645bc048e52e..fac0f33d10b2 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.h
+++ b/drivers/net/ethernet/microchip/lan743x_main.h
@@ -61,6 +61,7 @@
 #define PMT_CTL_RX_FCT_RFE_D3_CLK_OVR_		BIT(18)
 #define PMT_CTL_GPIO_WAKEUP_EN_			BIT(15)
 #define PMT_CTL_EEE_WAKEUP_EN_			BIT(13)
+#define PMT_CTL_RES_CLR_WKP_MASK_		GENMASK(9, 8)
 #define PMT_CTL_READY_				BIT(7)
 #define PMT_CTL_ETH_PHY_RST_			BIT(4)
 #define PMT_CTL_WOL_EN_				BIT(3)
@@ -227,12 +228,31 @@
 #define MAC_WUCSR				(0x140)
 #define MAC_MP_SO_EN_				BIT(21)
 #define MAC_WUCSR_RFE_WAKE_EN_			BIT(14)
+#define MAC_WUCSR_EEE_TX_WAKE_			BIT(13)
+#define MAC_WUCSR_EEE_RX_WAKE_			BIT(11)
+#define MAC_WUCSR_RFE_WAKE_FR_			BIT(9)
+#define MAC_WUCSR_PFDA_FR_			BIT(7)
+#define MAC_WUCSR_WUFR_				BIT(6)
+#define MAC_WUCSR_MPR_				BIT(5)
+#define MAC_WUCSR_BCAST_FR_			BIT(4)
 #define MAC_WUCSR_PFDA_EN_			BIT(3)
 #define MAC_WUCSR_WAKE_EN_			BIT(2)
 #define MAC_WUCSR_MPEN_				BIT(1)
 #define MAC_WUCSR_BCST_EN_			BIT(0)
 
 #define MAC_WK_SRC				(0x144)
+#define MAC_WK_SRC_ETH_PHY_WK_			BIT(17)
+#define MAC_WK_SRC_IPV6_TCPSYN_RCD_WK_		BIT(16)
+#define MAC_WK_SRC_IPV4_TCPSYN_RCD_WK_		BIT(15)
+#define MAC_WK_SRC_EEE_TX_WK_			BIT(14)
+#define MAC_WK_SRC_EEE_RX_WK_			BIT(13)
+#define MAC_WK_SRC_RFE_FR_WK_			BIT(12)
+#define MAC_WK_SRC_PFDA_FR_WK_			BIT(11)
+#define MAC_WK_SRC_MP_FR_WK_			BIT(10)
+#define MAC_WK_SRC_BCAST_FR_WK_			BIT(9)
+#define MAC_WK_SRC_WU_FR_WK_			BIT(8)
+#define MAC_WK_SRC_WK_FR_SAVED_			BIT(7)
+
 #define MAC_MP_SO_HI				(0x148)
 #define MAC_MP_SO_LO				(0x14C)
 
@@ -295,6 +315,10 @@
 #define RFE_INDX(index)			(0x580 + (index << 2))
 
 #define MAC_WUCSR2			(0x600)
+#define MAC_WUCSR2_NS_RCD_		BIT(7)
+#define MAC_WUCSR2_ARP_RCD_		BIT(6)
+#define MAC_WUCSR2_IPV6_TCPSYN_RCD_	BIT(5)
+#define MAC_WUCSR2_IPV4_TCPSYN_RCD_	BIT(4)
 
 #define SGMII_ACC			(0x720)
 #define SGMII_ACC_SGMII_BZY_		BIT(31)
-- 
2.34.1
Re: [PATCH net V4 1/3] net: lan743x: disable WOL upon resume to restore full data path operation
Posted by Russell King (Oracle) 1 year, 8 months ago
On Wed, Jun 12, 2024 at 10:55:37PM +0530, Raju Lakkaraju wrote:
> @@ -3728,6 +3729,30 @@ static int lan743x_pm_resume(struct device *dev)
>  		return ret;
>  	}
>  
> +	ret = lan743x_csr_read(adapter, MAC_WK_SRC);
> +	netif_info(adapter, drv, adapter->netdev,
> +		   "Wakeup source : 0x%08X\n", ret);

Does this need to be printed at info level, or is it a debug message?

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net V4 1/3] net: lan743x: disable WOL upon resume to restore full data path operation
Posted by Raju Lakkaraju 1 year, 8 months ago
Hi Russell King,

The 06/13/2024 08:44, Russell King (Oracle) wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Wed, Jun 12, 2024 at 10:55:37PM +0530, Raju Lakkaraju wrote:
> > @@ -3728,6 +3729,30 @@ static int lan743x_pm_resume(struct device *dev)
> >               return ret;
> >       }
> >
> > +     ret = lan743x_csr_read(adapter, MAC_WK_SRC);
> > +     netif_info(adapter, drv, adapter->netdev,
> > +                "Wakeup source : 0x%08X\n", ret);
> 
> Does this need to be printed at info level, or is it a debug message?

Print at info level helps the tester/sqa team to identify the root cause of
the wake and confirm the test cases.
In general, tester does not enable debug level messages for testing.

Still, if we need to change from info to debug, i can change.
Please let me know.

> 
> Thanks.
> 
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

-- 
Thanks,                                                                         
Raju
Re: [PATCH net V4 1/3] net: lan743x: disable WOL upon resume to restore full data path operation
Posted by Andrew Lunn 1 year, 8 months ago
On Fri, Jun 14, 2024 at 10:00:58AM +0530, Raju Lakkaraju wrote:
> Hi Russell King,
> 
> The 06/13/2024 08:44, Russell King (Oracle) wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > On Wed, Jun 12, 2024 at 10:55:37PM +0530, Raju Lakkaraju wrote:
> > > @@ -3728,6 +3729,30 @@ static int lan743x_pm_resume(struct device *dev)
> > >               return ret;
> > >       }
> > >
> > > +     ret = lan743x_csr_read(adapter, MAC_WK_SRC);
> > > +     netif_info(adapter, drv, adapter->netdev,
> > > +                "Wakeup source : 0x%08X\n", ret);
> > 
> > Does this need to be printed at info level, or is it a debug message?
> 
> Print at info level helps the tester/sqa team to identify the root cause of
> the wake and confirm the test cases.
> In general, tester does not enable debug level messages for testing.
> 
> Still, if we need to change from info to debug, i can change.
> Please let me know.

We are not really writing a kernel for the tester/SQA team, but the
end users. Do the end users find this log message useful? Can they
decode some hex value into something meaningful?

I'm surprised the test case cares what caused the wakeup. So long as
it does wake up, does it really matter what the source was?

	Andrew
Re: [PATCH net V4 1/3] net: lan743x: disable WOL upon resume to restore full data path operation
Posted by Raju Lakkaraju 1 year, 8 months ago
Hi Andrew,

The 06/14/2024 16:17, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Fri, Jun 14, 2024 at 10:00:58AM +0530, Raju Lakkaraju wrote:
> > Hi Russell King,
> >
> > The 06/13/2024 08:44, Russell King (Oracle) wrote:
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > >
> > > On Wed, Jun 12, 2024 at 10:55:37PM +0530, Raju Lakkaraju wrote:
> > > > @@ -3728,6 +3729,30 @@ static int lan743x_pm_resume(struct device *dev)
> > > >               return ret;
> > > >       }
> > > >
> > > > +     ret = lan743x_csr_read(adapter, MAC_WK_SRC);
> > > > +     netif_info(adapter, drv, adapter->netdev,
> > > > +                "Wakeup source : 0x%08X\n", ret);
> > >
> > > Does this need to be printed at info level, or is it a debug message?
> >
> > Print at info level helps the tester/sqa team to identify the root cause of
> > the wake and confirm the test cases.
> > In general, tester does not enable debug level messages for testing.
> >
> > Still, if we need to change from info to debug, i can change.
> > Please let me know.
> 
> We are not really writing a kernel for the tester/SQA team, but the
> end users. Do the end users find this log message useful? Can they
> decode some hex value into something meaningful?
> 

Yes. You are correct. We are writing code for end users.

But, our chip support different wake options which are testing by SQA team to
make sure all the options should work as per specifications and the output
respective input.
This message is not new and move from bottom.

As per Linux community review comment, I will move from "info" to "debug" in my
next patch series.

> I'm surprised the test case cares what caused the wakeup. So long as
> it does wake up, does it really matter what the source was?
> 
>         Andrew

-- 
Thanks,                                                                         
Raju