2024-06-05 10:19:24

by Raju Lakkaraju

[permalink] [raw]
Subject: [PATCH net V3 0/3] net: lan743x: Fixes for multiple WOL related issues

This patch series implement the following fixes:
1. Disable WOL upon resume in order to restore full data path operation
2. Support WOL at both the PHY and MAC appropriately
3. Remove interrupt mask clearing from config_init

Patch-3 was sent seperately earlier. Review comments in link:
https://lore.kernel.org/lkml/[email protected]/T/

Raju Lakkaraju (3):
net: lan743x: disable WOL upon resume to restore full data path
operation
net: lan743x: Support WOL at both the PHY and MAC appropriately
net: phy: mxl-gpy: Remove interrupt mask clearing from config_init

.../net/ethernet/microchip/lan743x_ethtool.c | 44 ++++++++++++--
drivers/net/ethernet/microchip/lan743x_main.c | 46 ++++++++++++---
drivers/net/ethernet/microchip/lan743x_main.h | 28 +++++++++
drivers/net/phy/mxl-gpy.c | 58 ++++++++++++-------
4 files changed, 144 insertions(+), 32 deletions(-)

--
2.34.1



2024-06-05 10:19:42

by Raju Lakkaraju

[permalink] [raw]
Subject: [PATCH net V3 1/3] net: lan743x: disable WOL upon resume to restore full data path operation

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 <[email protected]>
---
Change List:
------------
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


2024-06-05 10:19:57

by Raju Lakkaraju

[permalink] [raw]
Subject: [PATCH net V3 2/3] net: lan743x: Support WOL at both the PHY and MAC appropriately

Prevent options not supported by the PHY from being requested to it by the MAC
Whenever a WOL option is supported by both, the PHY is given priority
since that usually leads to better power savings

Fixes: e9e13b6adc338 ("lan743x: fix for potential NULL pointer dereference with bare card")
Signed-off-by: Raju Lakkaraju <[email protected]>
---
Change List:
------------
V2 -> V3:
- Remove the "phy does not support WOL" debug message which is not required
- Remove WAKE_PHY support option from Ethernet MAC (LAN743x/PCI11x1x) driver
- Add "phy_wol_supported" and "phy_wolopts" variables to hold PHY's WOL config
V1 -> V2:
- Repost - No change
V0 -> V1:
- Change the "phy does not support WOL" print from netif_info() to
netif_dbg()

.../net/ethernet/microchip/lan743x_ethtool.c | 44 +++++++++++++++++--
drivers/net/ethernet/microchip/lan743x_main.c | 16 +++++--
drivers/net/ethernet/microchip/lan743x_main.h | 4 ++
3 files changed, 56 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan743x_ethtool.c b/drivers/net/ethernet/microchip/lan743x_ethtool.c
index d0f4ff4ee075..0d1740d64676 100644
--- a/drivers/net/ethernet/microchip/lan743x_ethtool.c
+++ b/drivers/net/ethernet/microchip/lan743x_ethtool.c
@@ -1127,8 +1127,12 @@ static void lan743x_ethtool_get_wol(struct net_device *netdev,
if (netdev->phydev)
phy_ethtool_get_wol(netdev->phydev, wol);

- wol->supported |= WAKE_BCAST | WAKE_UCAST | WAKE_MCAST |
- WAKE_MAGIC | WAKE_PHY | WAKE_ARP;
+ if (wol->supported != adapter->phy_wol_supported)
+ netif_warn(adapter, drv, adapter->netdev,
+ "PHY changed its supported WOL! old=%x, new=%x\n",
+ adapter->phy_wol_supported, wol->supported);
+
+ wol->supported |= MAC_SUPPORTED_WAKES;

if (adapter->is_pci11x1x)
wol->supported |= WAKE_MAGICSECURE;
@@ -1143,7 +1147,39 @@ static int lan743x_ethtool_set_wol(struct net_device *netdev,
{
struct lan743x_adapter *adapter = netdev_priv(netdev);

+ /* WAKE_MAGICSEGURE is a modifier of and only valid together with
+ * WAKE_MAGIC
+ */
+ if ((wol->wolopts & WAKE_MAGICSECURE) && !(wol->wolopts & WAKE_MAGIC))
+ return -EINVAL;
+
+ if (netdev->phydev) {
+ struct ethtool_wolinfo phy_wol;
+ int ret;
+
+ phy_wol.wolopts = wol->wolopts & adapter->phy_wol_supported;
+
+ /* If WAKE_MAGICSECURE was requested, filter out WAKE_MAGIC
+ * for PHYs that do not support WAKE_MAGICSECURE
+ */
+ if (wol->wolopts & WAKE_MAGICSECURE &&
+ !(adapter->phy_wol_supported & WAKE_MAGICSECURE))
+ phy_wol.wolopts &= ~WAKE_MAGIC;
+
+ ret = phy_ethtool_set_wol(netdev->phydev, &phy_wol);
+ if (ret && (ret != -EOPNOTSUPP))
+ return ret;
+
+ if (ret == -EOPNOTSUPP)
+ adapter->phy_wolopts = 0;
+ else
+ adapter->phy_wolopts = phy_wol.wolopts;
+ } else {
+ adapter->phy_wolopts = 0;
+ }
+
adapter->wolopts = 0;
+ wol->wolopts &= ~adapter->phy_wolopts;
if (wol->wolopts & WAKE_UCAST)
adapter->wolopts |= WAKE_UCAST;
if (wol->wolopts & WAKE_MCAST)
@@ -1164,10 +1200,10 @@ static int lan743x_ethtool_set_wol(struct net_device *netdev,
memset(adapter->sopass, 0, sizeof(u8) * SOPASS_MAX);
}

+ wol->wolopts = adapter->wolopts | adapter->phy_wolopts;
device_set_wakeup_enable(&adapter->pdev->dev, (bool)wol->wolopts);

- return netdev->phydev ? phy_ethtool_set_wol(netdev->phydev, wol)
- : -ENETDOWN;
+ return 0;
}
#endif /* CONFIG_PM */

diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index 6a40b961fafb..b6810840bc61 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -3118,6 +3118,15 @@ static int lan743x_netdev_open(struct net_device *netdev)
if (ret)
goto close_tx;
}
+
+ if (adapter->netdev->phydev) {
+ struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
+
+ phy_ethtool_get_wol(netdev->phydev, &wol);
+ adapter->phy_wol_supported = wol.supported;
+ adapter->phy_wolopts = wol.wolopts;
+ }
+
return 0;

close_tx:
@@ -3587,10 +3596,9 @@ static void lan743x_pm_set_wol(struct lan743x_adapter *adapter)

pmtctl |= PMT_CTL_ETH_PHY_D3_COLD_OVR_ | PMT_CTL_ETH_PHY_D3_OVR_;

- if (adapter->wolopts & WAKE_PHY) {
- pmtctl |= PMT_CTL_ETH_PHY_EDPD_PLL_CTL_;
+ if (adapter->phy_wolopts)
pmtctl |= PMT_CTL_ETH_PHY_WAKE_EN_;
- }
+
if (adapter->wolopts & WAKE_MAGIC) {
wucsr |= MAC_WUCSR_MPEN_;
macrx |= MAC_RX_RXEN_;
@@ -3686,7 +3694,7 @@ static int lan743x_pm_suspend(struct device *dev)
lan743x_csr_write(adapter, MAC_WUCSR2, 0);
lan743x_csr_write(adapter, MAC_WK_SRC, 0xFFFFFFFF);

- if (adapter->wolopts)
+ if (adapter->wolopts || adapter->phy_wolopts)
lan743x_pm_set_wol(adapter);

if (adapter->is_pci11x1x) {
diff --git a/drivers/net/ethernet/microchip/lan743x_main.h b/drivers/net/ethernet/microchip/lan743x_main.h
index fac0f33d10b2..3b2585a384e2 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.h
+++ b/drivers/net/ethernet/microchip/lan743x_main.h
@@ -1042,6 +1042,8 @@ enum lan743x_sgmii_lsd {
LINK_2500_SLAVE
};

+#define MAC_SUPPORTED_WAKES (WAKE_BCAST | WAKE_UCAST | WAKE_MCAST | \
+ WAKE_MAGIC | WAKE_ARP)
struct lan743x_adapter {
struct net_device *netdev;
struct mii_bus *mdiobus;
@@ -1049,6 +1051,8 @@ struct lan743x_adapter {
#ifdef CONFIG_PM
u32 wolopts;
u8 sopass[SOPASS_MAX];
+ u32 phy_wolopts;
+ u32 phy_wol_supported;
#endif
struct pci_dev *pdev;
struct lan743x_csr csr;
--
2.34.1


2024-06-05 10:20:19

by Raju Lakkaraju

[permalink] [raw]
Subject: [PATCH net V3 3/3] net: phy: mxl-gpy: Remove interrupt mask clearing from config_init

When the system resumes from sleep, the phy_init_hw() function invokes
config_init(), which clears all interrupt masks and causes wake events to be
lost in subsequent wake sequences. Remove interrupt mask clearing from
config_init() and preserve relevant masks in config_intr()

Fixes: 7d901a1e878a ("net: phy: add Maxlinear GPY115/21x/24x driver")
Signed-off-by: Raju Lakkaraju <[email protected]>
---
Change List:
------------
V0 -> V3:
- Address the https://lore.kernel.org/lkml/[email protected]/T/
review comments

drivers/net/phy/mxl-gpy.c | 58 +++++++++++++++++++++++++--------------
1 file changed, 38 insertions(+), 20 deletions(-)

diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
index b2d36a3a96f1..e5f8ac4b4604 100644
--- a/drivers/net/phy/mxl-gpy.c
+++ b/drivers/net/phy/mxl-gpy.c
@@ -107,6 +107,7 @@ struct gpy_priv {

u8 fw_major;
u8 fw_minor;
+ u32 wolopts;

/* It takes 3 seconds to fully switch out of loopback mode before
* it can safely re-enter loopback mode. Record the time when
@@ -221,6 +222,15 @@ static int gpy_hwmon_register(struct phy_device *phydev)
}
#endif

+static int gpy_ack_interrupt(struct phy_device *phydev)
+{
+ int ret;
+
+ /* Clear all pending interrupts */
+ ret = phy_read(phydev, PHY_ISTAT);
+ return ret < 0 ? ret : 0;
+}
+
static int gpy_mbox_read(struct phy_device *phydev, u32 addr)
{
struct gpy_priv *priv = phydev->priv;
@@ -262,16 +272,8 @@ static int gpy_mbox_read(struct phy_device *phydev, u32 addr)

static int gpy_config_init(struct phy_device *phydev)
{
- int ret;
-
- /* Mask all interrupts */
- ret = phy_write(phydev, PHY_IMASK, 0);
- if (ret)
- return ret;
-
- /* Clear all pending interrupts */
- ret = phy_read(phydev, PHY_ISTAT);
- return ret < 0 ? ret : 0;
+ /* Nothing to configure. Configuration Requirement Placeholder */
+ return 0;
}

static int gpy21x_config_init(struct phy_device *phydev)
@@ -627,11 +629,23 @@ static int gpy_read_status(struct phy_device *phydev)

static int gpy_config_intr(struct phy_device *phydev)
{
+ struct gpy_priv *priv = phydev->priv;
u16 mask = 0;
+ int ret;
+
+ ret = gpy_ack_interrupt(phydev);
+ if (ret)
+ return ret;

if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
mask = PHY_IMASK_MASK;

+ if (priv->wolopts & WAKE_MAGIC)
+ mask |= PHY_IMASK_WOL;
+
+ if (priv->wolopts & WAKE_PHY)
+ mask |= PHY_IMASK_LSTC;
+
return phy_write(phydev, PHY_IMASK, mask);
}

@@ -678,6 +692,7 @@ static int gpy_set_wol(struct phy_device *phydev,
struct ethtool_wolinfo *wol)
{
struct net_device *attach_dev = phydev->attached_dev;
+ struct gpy_priv *priv = phydev->priv;
int ret;

if (wol->wolopts & WAKE_MAGIC) {
@@ -725,6 +740,8 @@ static int gpy_set_wol(struct phy_device *phydev,
ret = phy_read(phydev, PHY_ISTAT);
if (ret < 0)
return ret;
+
+ priv->wolopts |= WAKE_MAGIC;
} else {
/* Disable magic packet matching */
ret = phy_clear_bits_mmd(phydev, MDIO_MMD_VEND2,
@@ -732,6 +749,13 @@ static int gpy_set_wol(struct phy_device *phydev,
WOL_EN);
if (ret < 0)
return ret;
+
+ /* Disable the WOL interrupt */
+ ret = phy_clear_bits(phydev, PHY_IMASK, PHY_IMASK_WOL);
+ if (ret < 0)
+ return ret;
+
+ priv->wolopts &= ~WAKE_MAGIC;
}

if (wol->wolopts & WAKE_PHY) {
@@ -748,9 +772,11 @@ static int gpy_set_wol(struct phy_device *phydev,
if (ret & (PHY_IMASK_MASK & ~PHY_IMASK_LSTC))
phy_trigger_machine(phydev);

+ priv->wolopts |= WAKE_PHY;
return 0;
}

+ priv->wolopts &= ~WAKE_PHY;
/* Disable the link state change interrupt */
return phy_clear_bits(phydev, PHY_IMASK, PHY_IMASK_LSTC);
}
@@ -758,18 +784,10 @@ static int gpy_set_wol(struct phy_device *phydev,
static void gpy_get_wol(struct phy_device *phydev,
struct ethtool_wolinfo *wol)
{
- int ret;
+ struct gpy_priv *priv = phydev->priv;

wol->supported = WAKE_MAGIC | WAKE_PHY;
- wol->wolopts = 0;
-
- ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, VPSPEC2_WOL_CTL);
- if (ret & WOL_EN)
- wol->wolopts |= WAKE_MAGIC;
-
- ret = phy_read(phydev, PHY_IMASK);
- if (ret & PHY_IMASK_LSTC)
- wol->wolopts |= WAKE_PHY;
+ wol->wolopts = priv->wolopts;
}

static int gpy_loopback(struct phy_device *phydev, bool enable)
--
2.34.1


2024-06-05 11:19:11

by Wojciech Drewek

[permalink] [raw]
Subject: Re: [PATCH net V3 1/3] net: lan743x: disable WOL upon resume to restore full data path operation



On 05.06.2024 12:16, Raju Lakkaraju wrote:
> 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 <[email protected]>
> ---

Reviewed-by: Wojciech Drewek <[email protected]>

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

2024-06-05 11:21:08

by Wojciech Drewek

[permalink] [raw]
Subject: Re: [PATCH net V3 2/3] net: lan743x: Support WOL at both the PHY and MAC appropriately



On 05.06.2024 12:16, Raju Lakkaraju wrote:
> Prevent options not supported by the PHY from being requested to it by the MAC
> Whenever a WOL option is supported by both, the PHY is given priority
> since that usually leads to better power savings
>
> Fixes: e9e13b6adc338 ("lan743x: fix for potential NULL pointer dereference with bare card")
> Signed-off-by: Raju Lakkaraju <[email protected]>
> ---

Reviewed-by: Wojciech Drewek <[email protected]>

> Change List:
> ------------
> V2 -> V3:
> - Remove the "phy does not support WOL" debug message which is not required
> - Remove WAKE_PHY support option from Ethernet MAC (LAN743x/PCI11x1x) driver
> - Add "phy_wol_supported" and "phy_wolopts" variables to hold PHY's WOL config
> V1 -> V2:
> - Repost - No change
> V0 -> V1:
> - Change the "phy does not support WOL" print from netif_info() to
> netif_dbg()
>
> .../net/ethernet/microchip/lan743x_ethtool.c | 44 +++++++++++++++++--
> drivers/net/ethernet/microchip/lan743x_main.c | 16 +++++--
> drivers/net/ethernet/microchip/lan743x_main.h | 4 ++
> 3 files changed, 56 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/microchip/lan743x_ethtool.c b/drivers/net/ethernet/microchip/lan743x_ethtool.c
> index d0f4ff4ee075..0d1740d64676 100644
> --- a/drivers/net/ethernet/microchip/lan743x_ethtool.c
> +++ b/drivers/net/ethernet/microchip/lan743x_ethtool.c
> @@ -1127,8 +1127,12 @@ static void lan743x_ethtool_get_wol(struct net_device *netdev,
> if (netdev->phydev)
> phy_ethtool_get_wol(netdev->phydev, wol);
>
> - wol->supported |= WAKE_BCAST | WAKE_UCAST | WAKE_MCAST |
> - WAKE_MAGIC | WAKE_PHY | WAKE_ARP;
> + if (wol->supported != adapter->phy_wol_supported)
> + netif_warn(adapter, drv, adapter->netdev,
> + "PHY changed its supported WOL! old=%x, new=%x\n",
> + adapter->phy_wol_supported, wol->supported);
> +
> + wol->supported |= MAC_SUPPORTED_WAKES;
>
> if (adapter->is_pci11x1x)
> wol->supported |= WAKE_MAGICSECURE;
> @@ -1143,7 +1147,39 @@ static int lan743x_ethtool_set_wol(struct net_device *netdev,
> {
> struct lan743x_adapter *adapter = netdev_priv(netdev);
>
> + /* WAKE_MAGICSEGURE is a modifier of and only valid together with
> + * WAKE_MAGIC
> + */
> + if ((wol->wolopts & WAKE_MAGICSECURE) && !(wol->wolopts & WAKE_MAGIC))
> + return -EINVAL;
> +
> + if (netdev->phydev) {
> + struct ethtool_wolinfo phy_wol;
> + int ret;
> +
> + phy_wol.wolopts = wol->wolopts & adapter->phy_wol_supported;
> +
> + /* If WAKE_MAGICSECURE was requested, filter out WAKE_MAGIC
> + * for PHYs that do not support WAKE_MAGICSECURE
> + */
> + if (wol->wolopts & WAKE_MAGICSECURE &&
> + !(adapter->phy_wol_supported & WAKE_MAGICSECURE))
> + phy_wol.wolopts &= ~WAKE_MAGIC;
> +
> + ret = phy_ethtool_set_wol(netdev->phydev, &phy_wol);
> + if (ret && (ret != -EOPNOTSUPP))
> + return ret;
> +
> + if (ret == -EOPNOTSUPP)
> + adapter->phy_wolopts = 0;
> + else
> + adapter->phy_wolopts = phy_wol.wolopts;
> + } else {
> + adapter->phy_wolopts = 0;
> + }
> +
> adapter->wolopts = 0;
> + wol->wolopts &= ~adapter->phy_wolopts;
> if (wol->wolopts & WAKE_UCAST)
> adapter->wolopts |= WAKE_UCAST;
> if (wol->wolopts & WAKE_MCAST)
> @@ -1164,10 +1200,10 @@ static int lan743x_ethtool_set_wol(struct net_device *netdev,
> memset(adapter->sopass, 0, sizeof(u8) * SOPASS_MAX);
> }
>
> + wol->wolopts = adapter->wolopts | adapter->phy_wolopts;
> device_set_wakeup_enable(&adapter->pdev->dev, (bool)wol->wolopts);
>
> - return netdev->phydev ? phy_ethtool_set_wol(netdev->phydev, wol)
> - : -ENETDOWN;
> + return 0;
> }
> #endif /* CONFIG_PM */
>
> diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
> index 6a40b961fafb..b6810840bc61 100644
> --- a/drivers/net/ethernet/microchip/lan743x_main.c
> +++ b/drivers/net/ethernet/microchip/lan743x_main.c
> @@ -3118,6 +3118,15 @@ static int lan743x_netdev_open(struct net_device *netdev)
> if (ret)
> goto close_tx;
> }
> +
> + if (adapter->netdev->phydev) {
> + struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
> +
> + phy_ethtool_get_wol(netdev->phydev, &wol);
> + adapter->phy_wol_supported = wol.supported;
> + adapter->phy_wolopts = wol.wolopts;
> + }
> +
> return 0;
>
> close_tx:
> @@ -3587,10 +3596,9 @@ static void lan743x_pm_set_wol(struct lan743x_adapter *adapter)
>
> pmtctl |= PMT_CTL_ETH_PHY_D3_COLD_OVR_ | PMT_CTL_ETH_PHY_D3_OVR_;
>
> - if (adapter->wolopts & WAKE_PHY) {
> - pmtctl |= PMT_CTL_ETH_PHY_EDPD_PLL_CTL_;
> + if (adapter->phy_wolopts)
> pmtctl |= PMT_CTL_ETH_PHY_WAKE_EN_;
> - }
> +
> if (adapter->wolopts & WAKE_MAGIC) {
> wucsr |= MAC_WUCSR_MPEN_;
> macrx |= MAC_RX_RXEN_;
> @@ -3686,7 +3694,7 @@ static int lan743x_pm_suspend(struct device *dev)
> lan743x_csr_write(adapter, MAC_WUCSR2, 0);
> lan743x_csr_write(adapter, MAC_WK_SRC, 0xFFFFFFFF);
>
> - if (adapter->wolopts)
> + if (adapter->wolopts || adapter->phy_wolopts)
> lan743x_pm_set_wol(adapter);
>
> if (adapter->is_pci11x1x) {
> diff --git a/drivers/net/ethernet/microchip/lan743x_main.h b/drivers/net/ethernet/microchip/lan743x_main.h
> index fac0f33d10b2..3b2585a384e2 100644
> --- a/drivers/net/ethernet/microchip/lan743x_main.h
> +++ b/drivers/net/ethernet/microchip/lan743x_main.h
> @@ -1042,6 +1042,8 @@ enum lan743x_sgmii_lsd {
> LINK_2500_SLAVE
> };
>
> +#define MAC_SUPPORTED_WAKES (WAKE_BCAST | WAKE_UCAST | WAKE_MCAST | \
> + WAKE_MAGIC | WAKE_ARP)
> struct lan743x_adapter {
> struct net_device *netdev;
> struct mii_bus *mdiobus;
> @@ -1049,6 +1051,8 @@ struct lan743x_adapter {
> #ifdef CONFIG_PM
> u32 wolopts;
> u8 sopass[SOPASS_MAX];
> + u32 phy_wolopts;
> + u32 phy_wol_supported;
> #endif
> struct pci_dev *pdev;
> struct lan743x_csr csr;

2024-06-05 11:25:16

by Wojciech Drewek

[permalink] [raw]
Subject: Re: [PATCH net V3 3/3] net: phy: mxl-gpy: Remove interrupt mask clearing from config_init



On 05.06.2024 12:16, Raju Lakkaraju wrote:
> When the system resumes from sleep, the phy_init_hw() function invokes
> config_init(), which clears all interrupt masks and causes wake events to be
> lost in subsequent wake sequences. Remove interrupt mask clearing from
> config_init() and preserve relevant masks in config_intr()
>
> Fixes: 7d901a1e878a ("net: phy: add Maxlinear GPY115/21x/24x driver")
> Signed-off-by: Raju Lakkaraju <[email protected]>
> ---

One nit, other than that:
Reviewed-by: Wojciech Drewek <[email protected]>

> Change List:
> ------------
> V0 -> V3:
> - Address the https://lore.kernel.org/lkml/[email protected]/T/
> review comments
>
> drivers/net/phy/mxl-gpy.c | 58 +++++++++++++++++++++++++--------------
> 1 file changed, 38 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
> index b2d36a3a96f1..e5f8ac4b4604 100644
> --- a/drivers/net/phy/mxl-gpy.c
> +++ b/drivers/net/phy/mxl-gpy.c
> @@ -107,6 +107,7 @@ struct gpy_priv {
>
> u8 fw_major;
> u8 fw_minor;
> + u32 wolopts;
>
> /* It takes 3 seconds to fully switch out of loopback mode before
> * it can safely re-enter loopback mode. Record the time when
> @@ -221,6 +222,15 @@ static int gpy_hwmon_register(struct phy_device *phydev)
> }
> #endif
>
> +static int gpy_ack_interrupt(struct phy_device *phydev)
> +{
> + int ret;
> +
> + /* Clear all pending interrupts */
> + ret = phy_read(phydev, PHY_ISTAT);
> + return ret < 0 ? ret : 0;

Can we just return phy_read?

> +}
> +
> static int gpy_mbox_read(struct phy_device *phydev, u32 addr)
> {
> struct gpy_priv *priv = phydev->priv;
> @@ -262,16 +272,8 @@ static int gpy_mbox_read(struct phy_device *phydev, u32 addr)
>
> static int gpy_config_init(struct phy_device *phydev)
> {
> - int ret;
> -
> - /* Mask all interrupts */
> - ret = phy_write(phydev, PHY_IMASK, 0);
> - if (ret)
> - return ret;
> -
> - /* Clear all pending interrupts */
> - ret = phy_read(phydev, PHY_ISTAT);
> - return ret < 0 ? ret : 0;
> + /* Nothing to configure. Configuration Requirement Placeholder */
> + return 0;
> }
>
> static int gpy21x_config_init(struct phy_device *phydev)
> @@ -627,11 +629,23 @@ static int gpy_read_status(struct phy_device *phydev)
>
> static int gpy_config_intr(struct phy_device *phydev)
> {
> + struct gpy_priv *priv = phydev->priv;
> u16 mask = 0;
> + int ret;
> +
> + ret = gpy_ack_interrupt(phydev);
> + if (ret)
> + return ret;
>
> if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
> mask = PHY_IMASK_MASK;
>
> + if (priv->wolopts & WAKE_MAGIC)
> + mask |= PHY_IMASK_WOL;
> +
> + if (priv->wolopts & WAKE_PHY)
> + mask |= PHY_IMASK_LSTC;
> +
> return phy_write(phydev, PHY_IMASK, mask);
> }
>
> @@ -678,6 +692,7 @@ static int gpy_set_wol(struct phy_device *phydev,
> struct ethtool_wolinfo *wol)
> {
> struct net_device *attach_dev = phydev->attached_dev;
> + struct gpy_priv *priv = phydev->priv;
> int ret;
>
> if (wol->wolopts & WAKE_MAGIC) {
> @@ -725,6 +740,8 @@ static int gpy_set_wol(struct phy_device *phydev,
> ret = phy_read(phydev, PHY_ISTAT);
> if (ret < 0)
> return ret;
> +
> + priv->wolopts |= WAKE_MAGIC;
> } else {
> /* Disable magic packet matching */
> ret = phy_clear_bits_mmd(phydev, MDIO_MMD_VEND2,
> @@ -732,6 +749,13 @@ static int gpy_set_wol(struct phy_device *phydev,
> WOL_EN);
> if (ret < 0)
> return ret;
> +
> + /* Disable the WOL interrupt */
> + ret = phy_clear_bits(phydev, PHY_IMASK, PHY_IMASK_WOL);
> + if (ret < 0)
> + return ret;
> +
> + priv->wolopts &= ~WAKE_MAGIC;
> }
>
> if (wol->wolopts & WAKE_PHY) {
> @@ -748,9 +772,11 @@ static int gpy_set_wol(struct phy_device *phydev,
> if (ret & (PHY_IMASK_MASK & ~PHY_IMASK_LSTC))
> phy_trigger_machine(phydev);
>
> + priv->wolopts |= WAKE_PHY;
> return 0;
> }
>
> + priv->wolopts &= ~WAKE_PHY;
> /* Disable the link state change interrupt */
> return phy_clear_bits(phydev, PHY_IMASK, PHY_IMASK_LSTC);
> }
> @@ -758,18 +784,10 @@ static int gpy_set_wol(struct phy_device *phydev,
> static void gpy_get_wol(struct phy_device *phydev,
> struct ethtool_wolinfo *wol)
> {
> - int ret;
> + struct gpy_priv *priv = phydev->priv;
>
> wol->supported = WAKE_MAGIC | WAKE_PHY;
> - wol->wolopts = 0;
> -
> - ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, VPSPEC2_WOL_CTL);
> - if (ret & WOL_EN)
> - wol->wolopts |= WAKE_MAGIC;
> -
> - ret = phy_read(phydev, PHY_IMASK);
> - if (ret & PHY_IMASK_LSTC)
> - wol->wolopts |= WAKE_PHY;
> + wol->wolopts = priv->wolopts;
> }
>
> static int gpy_loopback(struct phy_device *phydev, bool enable)

2024-06-05 14:57:07

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH net V3 2/3] net: lan743x: Support WOL at both the PHY and MAC appropriately

Hi Raju,

kernel test robot noticed the following build errors:

[auto build test ERROR on net/main]

url: https://github.com/intel-lab-lkp/linux/commits/Raju-Lakkaraju/net-lan743x-disable-WOL-upon-resume-to-restore-full-data-path-operation/20240605-182110
base: net/main
patch link: https://lore.kernel.org/r/20240605101611.18791-3-Raju.Lakkaraju%40microchip.com
patch subject: [PATCH net V3 2/3] net: lan743x: Support WOL at both the PHY and MAC appropriately
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240605/[email protected]/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240605/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

drivers/net/ethernet/microchip/lan743x_main.c: In function 'lan743x_netdev_open':
>> drivers/net/ethernet/microchip/lan743x_main.c:3126:24: error: 'struct lan743x_adapter' has no member named 'phy_wol_supported'
3126 | adapter->phy_wol_supported = wol.supported;
| ^~
>> drivers/net/ethernet/microchip/lan743x_main.c:3127:24: error: 'struct lan743x_adapter' has no member named 'phy_wolopts'
3127 | adapter->phy_wolopts = wol.wolopts;
| ^~


vim +3126 drivers/net/ethernet/microchip/lan743x_main.c

3085
3086 static int lan743x_netdev_open(struct net_device *netdev)
3087 {
3088 struct lan743x_adapter *adapter = netdev_priv(netdev);
3089 int index;
3090 int ret;
3091
3092 ret = lan743x_intr_open(adapter);
3093 if (ret)
3094 goto return_error;
3095
3096 ret = lan743x_mac_open(adapter);
3097 if (ret)
3098 goto close_intr;
3099
3100 ret = lan743x_phy_open(adapter);
3101 if (ret)
3102 goto close_mac;
3103
3104 ret = lan743x_ptp_open(adapter);
3105 if (ret)
3106 goto close_phy;
3107
3108 lan743x_rfe_open(adapter);
3109
3110 for (index = 0; index < LAN743X_USED_RX_CHANNELS; index++) {
3111 ret = lan743x_rx_open(&adapter->rx[index]);
3112 if (ret)
3113 goto close_rx;
3114 }
3115
3116 for (index = 0; index < adapter->used_tx_channels; index++) {
3117 ret = lan743x_tx_open(&adapter->tx[index]);
3118 if (ret)
3119 goto close_tx;
3120 }
3121
3122 if (adapter->netdev->phydev) {
3123 struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
3124
3125 phy_ethtool_get_wol(netdev->phydev, &wol);
> 3126 adapter->phy_wol_supported = wol.supported;
> 3127 adapter->phy_wolopts = wol.wolopts;
3128 }
3129
3130 return 0;
3131
3132 close_tx:
3133 for (index = 0; index < adapter->used_tx_channels; index++) {
3134 if (adapter->tx[index].ring_cpu_ptr)
3135 lan743x_tx_close(&adapter->tx[index]);
3136 }
3137
3138 close_rx:
3139 for (index = 0; index < LAN743X_USED_RX_CHANNELS; index++) {
3140 if (adapter->rx[index].ring_cpu_ptr)
3141 lan743x_rx_close(&adapter->rx[index]);
3142 }
3143 lan743x_ptp_close(adapter);
3144
3145 close_phy:
3146 lan743x_phy_close(adapter);
3147
3148 close_mac:
3149 lan743x_mac_close(adapter);
3150
3151 close_intr:
3152 lan743x_intr_close(adapter);
3153
3154 return_error:
3155 netif_warn(adapter, ifup, adapter->netdev,
3156 "Error opening LAN743x\n");
3157 return ret;
3158 }
3159

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-06-05 22:31:14

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH net V3 2/3] net: lan743x: Support WOL at both the PHY and MAC appropriately

Hi Raju,

kernel test robot noticed the following build errors:

[auto build test ERROR on net/main]

url: https://github.com/intel-lab-lkp/linux/commits/Raju-Lakkaraju/net-lan743x-disable-WOL-upon-resume-to-restore-full-data-path-operation/20240605-182110
base: net/main
patch link: https://lore.kernel.org/r/20240605101611.18791-3-Raju.Lakkaraju%40microchip.com
patch subject: [PATCH net V3 2/3] net: lan743x: Support WOL at both the PHY and MAC appropriately
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20240606/[email protected]/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project d7d2d4f53fc79b4b58e8d8d08151b577c3699d4a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240606/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

In file included from drivers/net/ethernet/microchip/lan743x_main.c:4:
In file included from include/linux/module.h:19:
In file included from include/linux/elf.h:6:
In file included from arch/s390/include/asm/elf.h:173:
In file included from arch/s390/include/asm/mmu_context.h:11:
In file included from arch/s390/include/asm/pgalloc.h:18:
In file included from include/linux/mm.h:2253:
include/linux/vmstat.h:500:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
500 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
501 | item];
| ~~~~
include/linux/vmstat.h:507:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
507 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
508 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
include/linux/vmstat.h:514:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
514 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
include/linux/vmstat.h:519:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
519 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
520 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
include/linux/vmstat.h:528:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
528 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
529 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
In file included from drivers/net/ethernet/microchip/lan743x_main.c:5:
In file included from include/linux/pci.h:39:
In file included from include/linux/io.h:14:
In file included from arch/s390/include/asm/io.h:93:
include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
548 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
561 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
| ^
include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
102 | #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
| ^
In file included from drivers/net/ethernet/microchip/lan743x_main.c:5:
In file included from include/linux/pci.h:39:
In file included from include/linux/io.h:14:
In file included from arch/s390/include/asm/io.h:93:
include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
574 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
| ^
include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
115 | #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
| ^
In file included from drivers/net/ethernet/microchip/lan743x_main.c:5:
In file included from include/linux/pci.h:39:
In file included from include/linux/io.h:14:
In file included from arch/s390/include/asm/io.h:93:
include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
585 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
595 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
605 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:693:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
693 | readsb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:701:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
701 | readsw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:709:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
709 | readsl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:718:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
718 | writesb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:727:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
727 | writesw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:736:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
736 | writesl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
>> drivers/net/ethernet/microchip/lan743x_main.c:3126:12: error: no member named 'phy_wol_supported' in 'struct lan743x_adapter'
3126 | adapter->phy_wol_supported = wol.supported;
| ~~~~~~~ ^
>> drivers/net/ethernet/microchip/lan743x_main.c:3127:12: error: no member named 'phy_wolopts' in 'struct lan743x_adapter'
3127 | adapter->phy_wolopts = wol.wolopts;
| ~~~~~~~ ^
17 warnings and 2 errors generated.


vim +3126 drivers/net/ethernet/microchip/lan743x_main.c

3085
3086 static int lan743x_netdev_open(struct net_device *netdev)
3087 {
3088 struct lan743x_adapter *adapter = netdev_priv(netdev);
3089 int index;
3090 int ret;
3091
3092 ret = lan743x_intr_open(adapter);
3093 if (ret)
3094 goto return_error;
3095
3096 ret = lan743x_mac_open(adapter);
3097 if (ret)
3098 goto close_intr;
3099
3100 ret = lan743x_phy_open(adapter);
3101 if (ret)
3102 goto close_mac;
3103
3104 ret = lan743x_ptp_open(adapter);
3105 if (ret)
3106 goto close_phy;
3107
3108 lan743x_rfe_open(adapter);
3109
3110 for (index = 0; index < LAN743X_USED_RX_CHANNELS; index++) {
3111 ret = lan743x_rx_open(&adapter->rx[index]);
3112 if (ret)
3113 goto close_rx;
3114 }
3115
3116 for (index = 0; index < adapter->used_tx_channels; index++) {
3117 ret = lan743x_tx_open(&adapter->tx[index]);
3118 if (ret)
3119 goto close_tx;
3120 }
3121
3122 if (adapter->netdev->phydev) {
3123 struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
3124
3125 phy_ethtool_get_wol(netdev->phydev, &wol);
> 3126 adapter->phy_wol_supported = wol.supported;
> 3127 adapter->phy_wolopts = wol.wolopts;
3128 }
3129
3130 return 0;
3131
3132 close_tx:
3133 for (index = 0; index < adapter->used_tx_channels; index++) {
3134 if (adapter->tx[index].ring_cpu_ptr)
3135 lan743x_tx_close(&adapter->tx[index]);
3136 }
3137
3138 close_rx:
3139 for (index = 0; index < LAN743X_USED_RX_CHANNELS; index++) {
3140 if (adapter->rx[index].ring_cpu_ptr)
3141 lan743x_rx_close(&adapter->rx[index]);
3142 }
3143 lan743x_ptp_close(adapter);
3144
3145 close_phy:
3146 lan743x_phy_close(adapter);
3147
3148 close_mac:
3149 lan743x_mac_close(adapter);
3150
3151 close_intr:
3152 lan743x_intr_close(adapter);
3153
3154 return_error:
3155 netif_warn(adapter, ifup, adapter->netdev,
3156 "Error opening LAN743x\n");
3157 return ret;
3158 }
3159

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-06-06 06:44:21

by Raju Lakkaraju

[permalink] [raw]
Subject: Re: [PATCH net V3 3/3] net: phy: mxl-gpy: Remove interrupt mask clearing from config_init

Hi Wojciech,

The 06/05/2024 13:24, Wojciech Drewek wrote:
> [Some people who received this message don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 05.06.2024 12:16, Raju Lakkaraju wrote:
> > When the system resumes from sleep, the phy_init_hw() function invokes
> > config_init(), which clears all interrupt masks and causes wake events to be
> > lost in subsequent wake sequences. Remove interrupt mask clearing from
> > config_init() and preserve relevant masks in config_intr()
> >
> > Fixes: 7d901a1e878a ("net: phy: add Maxlinear GPY115/21x/24x driver")
> > Signed-off-by: Raju Lakkaraju <[email protected]>
> > ---
>
> One nit, other than that:
> Reviewed-by: Wojciech Drewek <[email protected]>
>
> > Change List:
> > ------------
> > V0 -> V3:
> > - Address the https://lore.kernel.org/lkml/[email protected]/T/
> > review comments
> >
> > drivers/net/phy/mxl-gpy.c | 58 +++++++++++++++++++++++++--------------
> > 1 file changed, 38 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
> > index b2d36a3a96f1..e5f8ac4b4604 100644
> > --- a/drivers/net/phy/mxl-gpy.c
> > +++ b/drivers/net/phy/mxl-gpy.c
> > @@ -107,6 +107,7 @@ struct gpy_priv {
> >
> > u8 fw_major;
> > u8 fw_minor;
> > + u32 wolopts;
> >
> > /* It takes 3 seconds to fully switch out of loopback mode before
> > * it can safely re-enter loopback mode. Record the time when
> > @@ -221,6 +222,15 @@ static int gpy_hwmon_register(struct phy_device *phydev)
> > }
> > #endif
> >
> > +static int gpy_ack_interrupt(struct phy_device *phydev)
> > +{
> > + int ret;
> > +
> > + /* Clear all pending interrupts */
> > + ret = phy_read(phydev, PHY_ISTAT);
> > + return ret < 0 ? ret : 0;
>
> Can we just return phy_read?
>

No.
PHY_ISTAT (i.e.16 bit) is Max Linear's GPY211 PHY interrupt status register.
These bits are ROSC (i.e. "Read Only, Self-Clearing) bits.
Read Status gives us the interrupts details
Any negative number is indicate the error in accessing PHY registgers.
Return either success (i.e. 0) or Error ( < 0)

> > +}
> > +
> > static int gpy_mbox_read(struct phy_device *phydev, u32 addr)
> > {
> > struct gpy_priv *priv = phydev->priv;
> > @@ -262,16 +272,8 @@ static int gpy_mbox_read(struct phy_device *phydev, u32 addr)
> >

--
Thanks,
Raju

2024-06-06 10:26:23

by Raju Lakkaraju

[permalink] [raw]
Subject: Re: [PATCH net V3 2/3] net: lan743x: Support WOL at both the PHY and MAC appropriately

The target architecture of alpha's config file miss the "CONFIG_PM=y"
cofiguration.

"phy_wolopts" and "phy_wol_supported" variable define in struct
lan743x_adapter under CONFIG_PM compiler option.

these variable define in drivers/net/ethernet/microchip/lan743x_main.h file.

Thanks,
Raju

The 06/05/2024 22:56, kernel test robot wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Hi Raju,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on net/main]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Raju-Lakkaraju/net-lan743x-disable-WOL-upon-resume-to-restore-full-data-path-operation/20240605-182110
> base: net/main
> patch link: https://lore.kernel.org/r/20240605101611.18791-3-Raju.Lakkaraju%40microchip.com
> patch subject: [PATCH net V3 2/3] net: lan743x: Support WOL at both the PHY and MAC appropriately
> config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240605/[email protected]/config)
> compiler: alpha-linux-gcc (GCC) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240605/[email protected]/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <[email protected]>
> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> All errors (new ones prefixed by >>):
>
> drivers/net/ethernet/microchip/lan743x_main.c: In function 'lan743x_netdev_open':
> >> drivers/net/ethernet/microchip/lan743x_main.c:3126:24: error: 'struct lan743x_adapter' has no member named 'phy_wol_supported'
> 3126 | adapter->phy_wol_supported = wol.supported;
> | ^~
> >> drivers/net/ethernet/microchip/lan743x_main.c:3127:24: error: 'struct lan743x_adapter' has no member named 'phy_wolopts'
> 3127 | adapter->phy_wolopts = wol.wolopts;
> | ^~
>
>
> vim +3126 drivers/net/ethernet/microchip/lan743x_main.c
>
> 3085
> 3086 static int lan743x_netdev_open(struct net_device *netdev)
> 3087 {
> 3088 struct lan743x_adapter *adapter = netdev_priv(netdev);
> 3089 int index;
> 3090 int ret;
> 3091
> 3092 ret = lan743x_intr_open(adapter);
> 3093 if (ret)
> 3094 goto return_error;
> 3095
> 3096 ret = lan743x_mac_open(adapter);
> 3097 if (ret)
> 3098 goto close_intr;
> 3099
> 3100 ret = lan743x_phy_open(adapter);
> 3101 if (ret)
> 3102 goto close_mac;
> 3103
> 3104 ret = lan743x_ptp_open(adapter);
> 3105 if (ret)
> 3106 goto close_phy;
> 3107
> 3108 lan743x_rfe_open(adapter);
> 3109
> 3110 for (index = 0; index < LAN743X_USED_RX_CHANNELS; index++) {
> 3111 ret = lan743x_rx_open(&adapter->rx[index]);
> 3112 if (ret)
> 3113 goto close_rx;
> 3114 }
> 3115
> 3116 for (index = 0; index < adapter->used_tx_channels; index++) {
> 3117 ret = lan743x_tx_open(&adapter->tx[index]);
> 3118 if (ret)
> 3119 goto close_tx;
> 3120 }
> 3121
> 3122 if (adapter->netdev->phydev) {
> 3123 struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
> 3124
> 3125 phy_ethtool_get_wol(netdev->phydev, &wol);
> > 3126 adapter->phy_wol_supported = wol.supported;
> > 3127 adapter->phy_wolopts = wol.wolopts;
> 3128 }
> 3129
> 3130 return 0;
> 3131
> 3132 close_tx:
> 3133 for (index = 0; index < adapter->used_tx_channels; index++) {
> 3134 if (adapter->tx[index].ring_cpu_ptr)
> 3135 lan743x_tx_close(&adapter->tx[index]);
> 3136 }
> 3137
> 3138 close_rx:
> 3139 for (index = 0; index < LAN743X_USED_RX_CHANNELS; index++) {
> 3140 if (adapter->rx[index].ring_cpu_ptr)
> 3141 lan743x_rx_close(&adapter->rx[index]);
> 3142 }
> 3143 lan743x_ptp_close(adapter);
> 3144
> 3145 close_phy:
> 3146 lan743x_phy_close(adapter);
> 3147
> 3148 close_mac:
> 3149 lan743x_mac_close(adapter);
> 3150
> 3151 close_intr:
> 3152 lan743x_intr_close(adapter);
> 3153
> 3154 return_error:
> 3155 netif_warn(adapter, ifup, adapter->netdev,
> 3156 "Error opening LAN743x\n");
> 3157 return ret;
> 3158 }
> 3159
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki

--
--------
Thanks,
Raju

2024-06-06 10:33:19

by Raju Lakkaraju

[permalink] [raw]
Subject: Re: [PATCH net V3 2/3] net: lan743x: Support WOL at both the PHY and MAC appropriately

"phy_wolopts" and "phy_wol_supported" variable define in struct lan743x_adapter
under CONFIG_PM compiler build option.

These variable define in drivers/net/ethernet/microchip/lan743x_main.h file.
Can you please add the config (CONFIG_PM=y) to build?

Thanks,
Raju

The 06/06/2024 06:25, kernel test robot wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Hi Raju,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on net/main]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Raju-Lakkaraju/net-lan743x-disable-WOL-upon-resume-to-restore-full-data-path-operation/20240605-182110
> base: net/main
> patch link: https://lore.kernel.org/r/20240605101611.18791-3-Raju.Lakkaraju%40microchip.com
> patch subject: [PATCH net V3 2/3] net: lan743x: Support WOL at both the PHY and MAC appropriately
> config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20240606/[email protected]/config)
> compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project d7d2d4f53fc79b4b58e8d8d08151b577c3699d4a)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240606/[email protected]/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <[email protected]>
> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> All errors (new ones prefixed by >>):
>
> In file included from drivers/net/ethernet/microchip/lan743x_main.c:4:
> In file included from include/linux/module.h:19:
> In file included from include/linux/elf.h:6:
> In file included from arch/s390/include/asm/elf.h:173:
> In file included from arch/s390/include/asm/mmu_context.h:11:
> In file included from arch/s390/include/asm/pgalloc.h:18:
> In file included from include/linux/mm.h:2253:
> include/linux/vmstat.h:500:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
> 500 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
> | ~~~~~~~~~~~~~~~~~~~~~ ^
> 501 | item];
> | ~~~~
> include/linux/vmstat.h:507:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
> 507 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
> | ~~~~~~~~~~~~~~~~~~~~~ ^
> 508 | NR_VM_NUMA_EVENT_ITEMS +
> | ~~~~~~~~~~~~~~~~~~~~~~
> include/linux/vmstat.h:514:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
> 514 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
> | ~~~~~~~~~~~ ^ ~~~
> include/linux/vmstat.h:519:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
> 519 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
> | ~~~~~~~~~~~~~~~~~~~~~ ^
> 520 | NR_VM_NUMA_EVENT_ITEMS +
> | ~~~~~~~~~~~~~~~~~~~~~~
> include/linux/vmstat.h:528:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
> 528 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
> | ~~~~~~~~~~~~~~~~~~~~~ ^
> 529 | NR_VM_NUMA_EVENT_ITEMS +
> | ~~~~~~~~~~~~~~~~~~~~~~
> In file included from drivers/net/ethernet/microchip/lan743x_main.c:5:
> In file included from include/linux/pci.h:39:
> In file included from include/linux/io.h:14:
> In file included from arch/s390/include/asm/io.h:93:
> include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> 548 | val = __raw_readb(PCI_IOBASE + addr);
> | ~~~~~~~~~~ ^
> include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> 561 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
> | ~~~~~~~~~~ ^
> include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
> 37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
> | ^
> include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
> 102 | #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
> | ^
> In file included from drivers/net/ethernet/microchip/lan743x_main.c:5:
> In file included from include/linux/pci.h:39:
> In file included from include/linux/io.h:14:
> In file included from arch/s390/include/asm/io.h:93:
> include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> 574 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
> | ~~~~~~~~~~ ^
> include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
> 35 | #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
> | ^
> include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
> 115 | #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
> | ^
> In file included from drivers/net/ethernet/microchip/lan743x_main.c:5:
> In file included from include/linux/pci.h:39:
> In file included from include/linux/io.h:14:
> In file included from arch/s390/include/asm/io.h:93:
> include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> 585 | __raw_writeb(value, PCI_IOBASE + addr);
> | ~~~~~~~~~~ ^
> include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> 595 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
> | ~~~~~~~~~~ ^
> include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> 605 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
> | ~~~~~~~~~~ ^
> include/asm-generic/io.h:693:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> 693 | readsb(PCI_IOBASE + addr, buffer, count);
> | ~~~~~~~~~~ ^
> include/asm-generic/io.h:701:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> 701 | readsw(PCI_IOBASE + addr, buffer, count);
> | ~~~~~~~~~~ ^
> include/asm-generic/io.h:709:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> 709 | readsl(PCI_IOBASE + addr, buffer, count);
> | ~~~~~~~~~~ ^
> include/asm-generic/io.h:718:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> 718 | writesb(PCI_IOBASE + addr, buffer, count);
> | ~~~~~~~~~~ ^
> include/asm-generic/io.h:727:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> 727 | writesw(PCI_IOBASE + addr, buffer, count);
> | ~~~~~~~~~~ ^
> include/asm-generic/io.h:736:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> 736 | writesl(PCI_IOBASE + addr, buffer, count);
> | ~~~~~~~~~~ ^
> >> drivers/net/ethernet/microchip/lan743x_main.c:3126:12: error: no member named 'phy_wol_supported' in 'struct lan743x_adapter'
> 3126 | adapter->phy_wol_supported = wol.supported;
> | ~~~~~~~ ^
> >> drivers/net/ethernet/microchip/lan743x_main.c:3127:12: error: no member named 'phy_wolopts' in 'struct lan743x_adapter'
> 3127 | adapter->phy_wolopts = wol.wolopts;
> | ~~~~~~~ ^
> 17 warnings and 2 errors generated.
>
>
> vim +3126 drivers/net/ethernet/microchip/lan743x_main.c
>
> 3085
> 3086 static int lan743x_netdev_open(struct net_device *netdev)
> 3087 {
> 3088 struct lan743x_adapter *adapter = netdev_priv(netdev);
> 3089 int index;
> 3090 int ret;
> 3091
> 3092 ret = lan743x_intr_open(adapter);
> 3093 if (ret)
> 3094 goto return_error;
> 3095
> 3096 ret = lan743x_mac_open(adapter);
> 3097 if (ret)
> 3098 goto close_intr;
> 3099
> 3100 ret = lan743x_phy_open(adapter);
> 3101 if (ret)
> 3102 goto close_mac;
> 3103
> 3104 ret = lan743x_ptp_open(adapter);
> 3105 if (ret)
> 3106 goto close_phy;
> 3107
> 3108 lan743x_rfe_open(adapter);
> 3109
> 3110 for (index = 0; index < LAN743X_USED_RX_CHANNELS; index++) {
> 3111 ret = lan743x_rx_open(&adapter->rx[index]);
> 3112 if (ret)
> 3113 goto close_rx;
> 3114 }
> 3115
> 3116 for (index = 0; index < adapter->used_tx_channels; index++) {
> 3117 ret = lan743x_tx_open(&adapter->tx[index]);
> 3118 if (ret)
> 3119 goto close_tx;
> 3120 }
> 3121
> 3122 if (adapter->netdev->phydev) {
> 3123 struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
> 3124
> 3125 phy_ethtool_get_wol(netdev->phydev, &wol);
> > 3126 adapter->phy_wol_supported = wol.supported;
> > 3127 adapter->phy_wolopts = wol.wolopts;
> 3128 }
> 3129
> 3130 return 0;
> 3131
> 3132 close_tx:
> 3133 for (index = 0; index < adapter->used_tx_channels; index++) {
> 3134 if (adapter->tx[index].ring_cpu_ptr)
> 3135 lan743x_tx_close(&adapter->tx[index]);
> 3136 }
> 3137
> 3138 close_rx:
> 3139 for (index = 0; index < LAN743X_USED_RX_CHANNELS; index++) {
> 3140 if (adapter->rx[index].ring_cpu_ptr)
> 3141 lan743x_rx_close(&adapter->rx[index]);
> 3142 }
> 3143 lan743x_ptp_close(adapter);
> 3144
> 3145 close_phy:
> 3146 lan743x_phy_close(adapter);
> 3147
> 3148 close_mac:
> 3149 lan743x_mac_close(adapter);
> 3150
> 3151 close_intr:
> 3152 lan743x_intr_close(adapter);
> 3153
> 3154 return_error:
> 3155 netif_warn(adapter, ifup, adapter->netdev,
> 3156 "Error opening LAN743x\n");
> 3157 return ret;
> 3158 }
> 3159
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki

--
--------
Thanks,
Raju

2024-06-06 13:30:49

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net V3 2/3] net: lan743x: Support WOL at both the PHY and MAC appropriately

On Thu, Jun 06, 2024 at 03:52:51PM +0530, Raju Lakkaraju wrote:
> The target architecture of alpha's config file miss the "CONFIG_PM=y"
> cofiguration.

No. Your patch is missing support for CONFIG_PM = n. Or you need to
add a depends on PM.

We expect the kernel to build for any configuration. There are build
bots which create random configurations and see if they build. Not
having PM is a valid configuration, especially for a big iron server
which never sleeps.

Andrew

2024-06-07 06:51:07

by Raju Lakkaraju

[permalink] [raw]
Subject: [PATCH net V3 2/3] net: lan743x: Support WOL at both the PHY and MAC appropriately

Prevent options not supported by the PHY from being requested to it by the MAC
Whenever a WOL option is supported by both, the PHY is given priority
since that usually leads to better power savings.

Fixes: e9e13b6adc338 ("lan743x: fix for potential NULL pointer dereference with bare card")
Signed-off-by: Raju Lakkaraju <[email protected]>
Reviewed-by: Wojciech Drewek <[email protected]>
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
---
Change List:
------------
V2 -> V3:
- Remove the "phy does not support WOL" debug message which is not required
- Remove WAKE_PHY support option from Ethernet MAC (LAN743x/PCI11x1x) driver
- Add "phy_wol_supported" and "phy_wolopts" variables to hold PHY's WOL config
V1 -> V2:
- Repost - No change
V0 -> V1:
- Change the "phy does not support WOL" print from netif_info() to
netif_dbg()

.../net/ethernet/microchip/lan743x_ethtool.c | 44 +++++++++++++++++--
drivers/net/ethernet/microchip/lan743x_main.c | 18 ++++++--
drivers/net/ethernet/microchip/lan743x_main.h | 4 ++
3 files changed, 58 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan743x_ethtool.c b/drivers/net/ethernet/microchip/lan743x_ethtool.c
index d0f4ff4ee075..0d1740d64676 100644
--- a/drivers/net/ethernet/microchip/lan743x_ethtool.c
+++ b/drivers/net/ethernet/microchip/lan743x_ethtool.c
@@ -1127,8 +1127,12 @@ static void lan743x_ethtool_get_wol(struct net_device *netdev,
if (netdev->phydev)
phy_ethtool_get_wol(netdev->phydev, wol);

- wol->supported |= WAKE_BCAST | WAKE_UCAST | WAKE_MCAST |
- WAKE_MAGIC | WAKE_PHY | WAKE_ARP;
+ if (wol->supported != adapter->phy_wol_supported)
+ netif_warn(adapter, drv, adapter->netdev,
+ "PHY changed its supported WOL! old=%x, new=%x\n",
+ adapter->phy_wol_supported, wol->supported);
+
+ wol->supported |= MAC_SUPPORTED_WAKES;

if (adapter->is_pci11x1x)
wol->supported |= WAKE_MAGICSECURE;
@@ -1143,7 +1147,39 @@ static int lan743x_ethtool_set_wol(struct net_device *netdev,
{
struct lan743x_adapter *adapter = netdev_priv(netdev);

+ /* WAKE_MAGICSEGURE is a modifier of and only valid together with
+ * WAKE_MAGIC
+ */
+ if ((wol->wolopts & WAKE_MAGICSECURE) && !(wol->wolopts & WAKE_MAGIC))
+ return -EINVAL;
+
+ if (netdev->phydev) {
+ struct ethtool_wolinfo phy_wol;
+ int ret;
+
+ phy_wol.wolopts = wol->wolopts & adapter->phy_wol_supported;
+
+ /* If WAKE_MAGICSECURE was requested, filter out WAKE_MAGIC
+ * for PHYs that do not support WAKE_MAGICSECURE
+ */
+ if (wol->wolopts & WAKE_MAGICSECURE &&
+ !(adapter->phy_wol_supported & WAKE_MAGICSECURE))
+ phy_wol.wolopts &= ~WAKE_MAGIC;
+
+ ret = phy_ethtool_set_wol(netdev->phydev, &phy_wol);
+ if (ret && (ret != -EOPNOTSUPP))
+ return ret;
+
+ if (ret == -EOPNOTSUPP)
+ adapter->phy_wolopts = 0;
+ else
+ adapter->phy_wolopts = phy_wol.wolopts;
+ } else {
+ adapter->phy_wolopts = 0;
+ }
+
adapter->wolopts = 0;
+ wol->wolopts &= ~adapter->phy_wolopts;
if (wol->wolopts & WAKE_UCAST)
adapter->wolopts |= WAKE_UCAST;
if (wol->wolopts & WAKE_MCAST)
@@ -1164,10 +1200,10 @@ static int lan743x_ethtool_set_wol(struct net_device *netdev,
memset(adapter->sopass, 0, sizeof(u8) * SOPASS_MAX);
}

+ wol->wolopts = adapter->wolopts | adapter->phy_wolopts;
device_set_wakeup_enable(&adapter->pdev->dev, (bool)wol->wolopts);

- return netdev->phydev ? phy_ethtool_set_wol(netdev->phydev, wol)
- : -ENETDOWN;
+ return 0;
}
#endif /* CONFIG_PM */

diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index 6a40b961fafb..90572e780d9f 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -3118,6 +3118,17 @@ static int lan743x_netdev_open(struct net_device *netdev)
if (ret)
goto close_tx;
}
+
+#ifdef CONFIG_PM
+ if (adapter->netdev->phydev) {
+ struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
+
+ phy_ethtool_get_wol(netdev->phydev, &wol);
+ adapter->phy_wol_supported = wol.supported;
+ adapter->phy_wolopts = wol.wolopts;
+ }
+#endif
+
return 0;

close_tx:
@@ -3587,10 +3598,9 @@ static void lan743x_pm_set_wol(struct lan743x_adapter *adapter)

pmtctl |= PMT_CTL_ETH_PHY_D3_COLD_OVR_ | PMT_CTL_ETH_PHY_D3_OVR_;

- if (adapter->wolopts & WAKE_PHY) {
- pmtctl |= PMT_CTL_ETH_PHY_EDPD_PLL_CTL_;
+ if (adapter->phy_wolopts)
pmtctl |= PMT_CTL_ETH_PHY_WAKE_EN_;
- }
+
if (adapter->wolopts & WAKE_MAGIC) {
wucsr |= MAC_WUCSR_MPEN_;
macrx |= MAC_RX_RXEN_;
@@ -3686,7 +3696,7 @@ static int lan743x_pm_suspend(struct device *dev)
lan743x_csr_write(adapter, MAC_WUCSR2, 0);
lan743x_csr_write(adapter, MAC_WK_SRC, 0xFFFFFFFF);

- if (adapter->wolopts)
+ if (adapter->wolopts || adapter->phy_wolopts)
lan743x_pm_set_wol(adapter);

if (adapter->is_pci11x1x) {
diff --git a/drivers/net/ethernet/microchip/lan743x_main.h b/drivers/net/ethernet/microchip/lan743x_main.h
index fac0f33d10b2..3b2585a384e2 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.h
+++ b/drivers/net/ethernet/microchip/lan743x_main.h
@@ -1042,6 +1042,8 @@ enum lan743x_sgmii_lsd {
LINK_2500_SLAVE
};

+#define MAC_SUPPORTED_WAKES (WAKE_BCAST | WAKE_UCAST | WAKE_MCAST | \
+ WAKE_MAGIC | WAKE_ARP)
struct lan743x_adapter {
struct net_device *netdev;
struct mii_bus *mdiobus;
@@ -1049,6 +1051,8 @@ struct lan743x_adapter {
#ifdef CONFIG_PM
u32 wolopts;
u8 sopass[SOPASS_MAX];
+ u32 phy_wolopts;
+ u32 phy_wol_supported;
#endif
struct pci_dev *pdev;
struct lan743x_csr csr;
--
2.34.1


2024-06-11 07:11:25

by Raju Lakkaraju

[permalink] [raw]
Subject: [PATCH net V3 0/3] net: lan743x: Fixes for multiple WOL related issues

This patch series implement the following fixes:
1. Disable WOL upon resume in order to restore full data path operation
2. Support WOL at both the PHY and MAC appropriately
3. Remove interrupt mask clearing from config_init

Patch-3 was sent seperately earlier. Review comments in link:
https://lore.kernel.org/lkml/[email protected]/T/

Reviewed-by: Wojciech Drewek <[email protected]>
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

Raju Lakkaraju (3):
net: lan743x: disable WOL upon resume to restore full data path
operation
net: lan743x: Support WOL at both the PHY and MAC appropriately
net: phy: mxl-gpy: Remove interrupt mask clearing from config_init

.../net/ethernet/microchip/lan743x_ethtool.c | 44 ++++++++++++--
drivers/net/ethernet/microchip/lan743x_main.c | 46 ++++++++++++---
drivers/net/ethernet/microchip/lan743x_main.h | 28 +++++++++
drivers/net/phy/mxl-gpy.c | 58 ++++++++++++-------
4 files changed, 144 insertions(+), 32 deletions(-)

--
2.34.1


2024-06-11 07:12:58

by Raju Lakkaraju

[permalink] [raw]
Subject: [PATCH net V3 2/3] net: lan743x: Support WOL at both the PHY and MAC appropriately

Prevent options not supported by the PHY from being requested to it by the MAC
Whenever a WOL option is supported by both, the PHY is given priority
since that usually leads to better power savings.

Fixes: e9e13b6adc338 ("lan743x: fix for potential NULL pointer dereference with bare card")
Signed-off-by: Raju Lakkaraju <[email protected]>
Reviewed-by: Wojciech Drewek <[email protected]>
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
---
Change List:
------------
V2 -> V3:
- Remove the "phy does not support WOL" debug message which is not required
- Remove WAKE_PHY support option from Ethernet MAC (LAN743x/PCI11x1x) driver
- Add "phy_wol_supported" and "phy_wolopts" variables to hold PHY's WOL config
V1 -> V2:
- Repost - No change
V0 -> V1:
- Change the "phy does not support WOL" print from netif_info() to
netif_dbg()

.../net/ethernet/microchip/lan743x_ethtool.c | 44 +++++++++++++++++--
drivers/net/ethernet/microchip/lan743x_main.c | 18 ++++++--
drivers/net/ethernet/microchip/lan743x_main.h | 4 ++
3 files changed, 58 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan743x_ethtool.c b/drivers/net/ethernet/microchip/lan743x_ethtool.c
index d0f4ff4ee075..0d1740d64676 100644
--- a/drivers/net/ethernet/microchip/lan743x_ethtool.c
+++ b/drivers/net/ethernet/microchip/lan743x_ethtool.c
@@ -1127,8 +1127,12 @@ static void lan743x_ethtool_get_wol(struct net_device *netdev,
if (netdev->phydev)
phy_ethtool_get_wol(netdev->phydev, wol);

- wol->supported |= WAKE_BCAST | WAKE_UCAST | WAKE_MCAST |
- WAKE_MAGIC | WAKE_PHY | WAKE_ARP;
+ if (wol->supported != adapter->phy_wol_supported)
+ netif_warn(adapter, drv, adapter->netdev,
+ "PHY changed its supported WOL! old=%x, new=%x\n",
+ adapter->phy_wol_supported, wol->supported);
+
+ wol->supported |= MAC_SUPPORTED_WAKES;

if (adapter->is_pci11x1x)
wol->supported |= WAKE_MAGICSECURE;
@@ -1143,7 +1147,39 @@ static int lan743x_ethtool_set_wol(struct net_device *netdev,
{
struct lan743x_adapter *adapter = netdev_priv(netdev);

+ /* WAKE_MAGICSEGURE is a modifier of and only valid together with
+ * WAKE_MAGIC
+ */
+ if ((wol->wolopts & WAKE_MAGICSECURE) && !(wol->wolopts & WAKE_MAGIC))
+ return -EINVAL;
+
+ if (netdev->phydev) {
+ struct ethtool_wolinfo phy_wol;
+ int ret;
+
+ phy_wol.wolopts = wol->wolopts & adapter->phy_wol_supported;
+
+ /* If WAKE_MAGICSECURE was requested, filter out WAKE_MAGIC
+ * for PHYs that do not support WAKE_MAGICSECURE
+ */
+ if (wol->wolopts & WAKE_MAGICSECURE &&
+ !(adapter->phy_wol_supported & WAKE_MAGICSECURE))
+ phy_wol.wolopts &= ~WAKE_MAGIC;
+
+ ret = phy_ethtool_set_wol(netdev->phydev, &phy_wol);
+ if (ret && (ret != -EOPNOTSUPP))
+ return ret;
+
+ if (ret == -EOPNOTSUPP)
+ adapter->phy_wolopts = 0;
+ else
+ adapter->phy_wolopts = phy_wol.wolopts;
+ } else {
+ adapter->phy_wolopts = 0;
+ }
+
adapter->wolopts = 0;
+ wol->wolopts &= ~adapter->phy_wolopts;
if (wol->wolopts & WAKE_UCAST)
adapter->wolopts |= WAKE_UCAST;
if (wol->wolopts & WAKE_MCAST)
@@ -1164,10 +1200,10 @@ static int lan743x_ethtool_set_wol(struct net_device *netdev,
memset(adapter->sopass, 0, sizeof(u8) * SOPASS_MAX);
}

+ wol->wolopts = adapter->wolopts | adapter->phy_wolopts;
device_set_wakeup_enable(&adapter->pdev->dev, (bool)wol->wolopts);

- return netdev->phydev ? phy_ethtool_set_wol(netdev->phydev, wol)
- : -ENETDOWN;
+ return 0;
}
#endif /* CONFIG_PM */

diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index 6a40b961fafb..90572e780d9f 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -3118,6 +3118,17 @@ static int lan743x_netdev_open(struct net_device *netdev)
if (ret)
goto close_tx;
}
+
+#ifdef CONFIG_PM
+ if (adapter->netdev->phydev) {
+ struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
+
+ phy_ethtool_get_wol(netdev->phydev, &wol);
+ adapter->phy_wol_supported = wol.supported;
+ adapter->phy_wolopts = wol.wolopts;
+ }
+#endif
+
return 0;

close_tx:
@@ -3587,10 +3598,9 @@ static void lan743x_pm_set_wol(struct lan743x_adapter *adapter)

pmtctl |= PMT_CTL_ETH_PHY_D3_COLD_OVR_ | PMT_CTL_ETH_PHY_D3_OVR_;

- if (adapter->wolopts & WAKE_PHY) {
- pmtctl |= PMT_CTL_ETH_PHY_EDPD_PLL_CTL_;
+ if (adapter->phy_wolopts)
pmtctl |= PMT_CTL_ETH_PHY_WAKE_EN_;
- }
+
if (adapter->wolopts & WAKE_MAGIC) {
wucsr |= MAC_WUCSR_MPEN_;
macrx |= MAC_RX_RXEN_;
@@ -3686,7 +3696,7 @@ static int lan743x_pm_suspend(struct device *dev)
lan743x_csr_write(adapter, MAC_WUCSR2, 0);
lan743x_csr_write(adapter, MAC_WK_SRC, 0xFFFFFFFF);

- if (adapter->wolopts)
+ if (adapter->wolopts || adapter->phy_wolopts)
lan743x_pm_set_wol(adapter);

if (adapter->is_pci11x1x) {
diff --git a/drivers/net/ethernet/microchip/lan743x_main.h b/drivers/net/ethernet/microchip/lan743x_main.h
index fac0f33d10b2..3b2585a384e2 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.h
+++ b/drivers/net/ethernet/microchip/lan743x_main.h
@@ -1042,6 +1042,8 @@ enum lan743x_sgmii_lsd {
LINK_2500_SLAVE
};

+#define MAC_SUPPORTED_WAKES (WAKE_BCAST | WAKE_UCAST | WAKE_MCAST | \
+ WAKE_MAGIC | WAKE_ARP)
struct lan743x_adapter {
struct net_device *netdev;
struct mii_bus *mdiobus;
@@ -1049,6 +1051,8 @@ struct lan743x_adapter {
#ifdef CONFIG_PM
u32 wolopts;
u8 sopass[SOPASS_MAX];
+ u32 phy_wolopts;
+ u32 phy_wol_supported;
#endif
struct pci_dev *pdev;
struct lan743x_csr csr;
--
2.34.1


2024-06-11 07:43:46

by Horatiu Vultur

[permalink] [raw]
Subject: Re: [PATCH net V3 0/3] net: lan743x: Fixes for multiple WOL related issues

Hi Raju,

Is this not supposed to be v4?
Because I can see v3 here:
https://www.spinics.net/lists/netdev/msg1002225.html

The 06/11/2024 11:57, Raju Lakkaraju wrote:
> This patch series implement the following fixes:
> 1. Disable WOL upon resume in order to restore full data path operation
> 2. Support WOL at both the PHY and MAC appropriately
> 3. Remove interrupt mask clearing from config_init
>
> Patch-3 was sent seperately earlier. Review comments in link:
> https://lore.kernel.org/lkml/[email protected]/T/
>
> Reviewed-by: Wojciech Drewek <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

I think you should drop the 'Reported-by' and 'Closes' tags because the
issue that is getting closed is the one that you introduced in one of
your previous version of the patch series.

>
> Raju Lakkaraju (3):
> net: lan743x: disable WOL upon resume to restore full data path
> operation
> net: lan743x: Support WOL at both the PHY and MAC appropriately
> net: phy: mxl-gpy: Remove interrupt mask clearing from config_init
>
> .../net/ethernet/microchip/lan743x_ethtool.c | 44 ++++++++++++--
> drivers/net/ethernet/microchip/lan743x_main.c | 46 ++++++++++++---
> drivers/net/ethernet/microchip/lan743x_main.h | 28 +++++++++
> drivers/net/phy/mxl-gpy.c | 58 ++++++++++++-------
> 4 files changed, 144 insertions(+), 32 deletions(-)
>
> --
> 2.34.1
>
>

--
/Horatiu

2024-06-11 08:05:24

by Raju Lakkaraju

[permalink] [raw]
Subject: Re: [PATCH net V3 0/3] net: lan743x: Fixes for multiple WOL related issues

Hi Horatiu,

There is no new changes except "kernel test robot" reported issue.

I fix the issue and sent the patch along with other old patches.
Also add "Reported-by" and "Closes" tags to all patches and
[email protected].
i.e.
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

Is it sufficient? or
Do you need to generete new version of patches ?

Thanks,
Raju
The 06/11/2024 09:10, Horatiu Vultur wrote:
> Hi Raju,
>
> Is this not supposed to be v4?
> Because I can see v3 here:
> https://www.spinics.net/lists/netdev/msg1002225.html
>
> The 06/11/2024 11:57, Raju Lakkaraju wrote:
> > This patch series implement the following fixes:
> > 1. Disable WOL upon resume in order to restore full data path operation
> > 2. Support WOL at both the PHY and MAC appropriately
> > 3. Remove interrupt mask clearing from config_init
> >
> > Patch-3 was sent seperately earlier. Review comments in link:
> > https://lore.kernel.org/lkml/[email protected]/T/
> >
> > Reviewed-by: Wojciech Drewek <[email protected]>
> > Reported-by: kernel test robot <[email protected]>
> > Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> I think you should drop the 'Reported-by' and 'Closes' tags because the
> issue that is getting closed is the one that you introduced in one of
> your previous version of the patch series.
>
> >
> > Raju Lakkaraju (3):
> > net: lan743x: disable WOL upon resume to restore full data path
> > operation
> > net: lan743x: Support WOL at both the PHY and MAC appropriately
> > net: phy: mxl-gpy: Remove interrupt mask clearing from config_init
> >
> > .../net/ethernet/microchip/lan743x_ethtool.c | 44 ++++++++++++--
> > drivers/net/ethernet/microchip/lan743x_main.c | 46 ++++++++++++---
> > drivers/net/ethernet/microchip/lan743x_main.h | 28 +++++++++
> > drivers/net/phy/mxl-gpy.c | 58 ++++++++++++-------
> > 4 files changed, 144 insertions(+), 32 deletions(-)
> >
> > --
> > 2.34.1
> >
> >
>
> --
> /Horatiu

--
Thanks,
Raju

2024-06-11 10:46:30

by Horatiu Vultur

[permalink] [raw]
Subject: Re: [PATCH net V3 0/3] net: lan743x: Fixes for multiple WOL related issues

The 06/11/2024 13:31, Raju Lakkaraju wrote:
> Hi Horatiu,
>
> There is no new changes except "kernel test robot" reported issue.

So there is no change in the code, you just added the tags between this
version and the previous one where the robot complained?
Because to me it looks like you added an extra #ifdef in 2/3 patch.

>
> I fix the issue and sent the patch along with other old patches.
> Also add "Reported-by" and "Closes" tags to all patches and
> [email protected].
> i.e.
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <[email protected]>
> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

It doesn't say to add only if you fix the issue in a separate patch and
not just for a new version of the patch/commit.
Or I miss reading this?

>
> Is it sufficient? or
> Do you need to generete new version of patches ?

Every time when you do a change in your patch until is accepted, you
will need to generate a new version.
Don't forget about 24h rule. That you need to wait 24h before you can
send a new version.

>
> Thanks,
> Raju
> The 06/11/2024 09:10, Horatiu Vultur wrote:
> > Hi Raju,
> >
> > Is this not supposed to be v4?
> > Because I can see v3 here:
> > https://www.spinics.net/lists/netdev/msg1002225.html
> >
> > The 06/11/2024 11:57, Raju Lakkaraju wrote:
> > > This patch series implement the following fixes:
> > > 1. Disable WOL upon resume in order to restore full data path operation
> > > 2. Support WOL at both the PHY and MAC appropriately
> > > 3. Remove interrupt mask clearing from config_init
> > >
> > > Patch-3 was sent seperately earlier. Review comments in link:
> > > https://lore.kernel.org/lkml/[email protected]/T/
> > >
> > > Reviewed-by: Wojciech Drewek <[email protected]>
> > > Reported-by: kernel test robot <[email protected]>
> > > Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> >
> > I think you should drop the 'Reported-by' and 'Closes' tags because the
> > issue that is getting closed is the one that you introduced in one of
> > your previous version of the patch series.
> >
> > >
> > > Raju Lakkaraju (3):
> > > net: lan743x: disable WOL upon resume to restore full data path
> > > operation
> > > net: lan743x: Support WOL at both the PHY and MAC appropriately
> > > net: phy: mxl-gpy: Remove interrupt mask clearing from config_init
> > >
> > > .../net/ethernet/microchip/lan743x_ethtool.c | 44 ++++++++++++--
> > > drivers/net/ethernet/microchip/lan743x_main.c | 46 ++++++++++++---
> > > drivers/net/ethernet/microchip/lan743x_main.h | 28 +++++++++
> > > drivers/net/phy/mxl-gpy.c | 58 ++++++++++++-------
> > > 4 files changed, 144 insertions(+), 32 deletions(-)
> > >
> > > --
> > > 2.34.1
> > >
> > >
> >
> > --
> > /Horatiu
>
> --
> Thanks,
> Raju

--
/Horatiu