Messages in this thread | ![/](/images/icornerl.gif) | | From | <> | Subject | RE: [PATCH net V2 2/2] net: lan743x: support WOL in MAC even when PHY does not | Date | Tue, 23 Apr 2024 11:05:44 +0000 |
| |
Hi Andrew,
> -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: Friday, April 5, 2024 10:42 PM > To: Raju Lakkaraju - I30499 <Raju.Lakkaraju@microchip.com> > Cc: netdev@vger.kernel.org; davem@davemloft.net; kuba@kernel.org; > pabeni@redhat.com; edumazet@google.com; linux-kernel@vger.kernel.org; > Bryan Whitehead - C21958 <Bryan.Whitehead@microchip.com>; > UNGLinuxDriver <UNGLinuxDriver@microchip.com> > Subject: Re: [PATCH net V2 2/2] net: lan743x: support WOL in MAC even when > PHY does not > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > On Fri, Apr 05, 2024 at 08:17:22AM +0000, Raju.Lakkaraju@microchip.com > wrote: > > Hi Andrew, > > > > Sorry for delayed response. > > > > > -----Original Message----- > > > From: Andrew Lunn <andrew@lunn.ch> > > > Sent: Thursday, March 21, 2024 4:23 AM > > > To: Raju Lakkaraju - I30499 <Raju.Lakkaraju@microchip.com> > > > Cc: netdev@vger.kernel.org; davem@davemloft.net; kuba@kernel.org; > > > pabeni@redhat.com; edumazet@google.com; > > > linux-kernel@vger.kernel.org; Bryan Whitehead - C21958 > > > <Bryan.Whitehead@microchip.com>; UNGLinuxDriver > > > <UNGLinuxDriver@microchip.com> > > > Subject: Re: [PATCH net V2 2/2] net: lan743x: support WOL in MAC > > > even when PHY does not > > > > > > EXTERNAL EMAIL: Do not click links or open attachments unless you > > > know the content is safe > > > > > > > + if (netdev->phydev) { > > > > + ret = phy_ethtool_set_wol(netdev->phydev, wol); > > > > + if (ret != -EOPNOTSUPP && ret != 0) > > > > + return ret; > > > > > > I'm not sure this condition is correct. > > > > > > If there is an error, and the error is not EOPNOTSUPP, you want to > > > report that error. However, if the PHY can support the WoL > > > configuration, it will return 0, and this function should exit, WoL > > > in the MAC is not needed. And doing WoL in the PHY consumes less > power since you can suspend the MAC. > > > > > > So i think it should simply be: > > > > > > > + if (ret != -EOPNOTSUPP) > > > > + return ret; > > > > > > Do you have a board with this MAC with a PHY which does have some > > > WoL support. Could you test PHY WoL is used when appropriate? > > > > Yes. > > We have external PHY (Max Linear GPY211C) attach to MAC of PCI11x1x > > (PCIe Ethernet chip) If I don't register the Ethernet module in wakeup > source, WOL is not working. Ethernet device power state shows as disable. > > So i'm talking about the case where the PHY is doing the wakeup, without > help from the MAC. In that case, "Ethernet device power state shows as > disable." is sensible. >
If we don't enable/register the PCI11x1x's ethernet device in wake list by calling " device_set_wakeup_enable( )" function, device power state shows as disable. When I refer the other vendor pcie's ethernet drivers, " device_set_wakeup_enable( )" function called in ethernet driver Power Management's suspend and resume functions. But LAN743x driver call this function when Ethtool configure the WOL.
> > i.e. > > > /sys/devices/pci0000:00/0000:00:1c.4/0000:05:00.0/0000:06:03.0/0000:09:00. > 0/power/wakeup <-- disabled
When I call the device_set_wakeup_enable( ) with enable = 1, wakeup (/sys/devices/pci0000:00/0000:00:1c.4/0000:05:00.0/0000:06:03.0/0000:09:00.0/power/wakeup) shows enable. Also observe that device (PCI11x1x's ethernet device : 0000:09:00.0) add in wake list. i.e. device: 'wakeup34': device_add
> > > > PCI11x1x is PCIe bridge device between PCIe and Ethernet along with > > other peripherals (i.e. UART, SPI, I2C, USB and PCIe devices) > > 0000:09:00.0 - Ethernet device > > 0000:05:00.0 - PCIe Bridge Up link > > How does the PHY indicate wake up? It could be you can power off the MAC, > but you need to keep parts of the PCIe bridge up, in order the wake up gets > delivered? >
PHY (GPY211C)'s interrupt pin (MDINT) connect to PCI11x1x's ethernet device. When I configure the WAKE_PHY or WAKE_MAGIC on GPY211C PHY, Interrupt generation observed when magic packet or link activity (link down or link up). If wakeup enable in PCI11x1x's ethernet device, System resumes from sleep.
> > When I test the WOL_PHY option on setup (PCI11x1x MAC + GPY211C PHY), > > observe the following: > > > 1. When enable WOL_PHY by using ethtool (i.e. ethtool -s enp9s0 wol > > p), GPY211 PHY configure the WOL. After resume from sleep, GPY211 WOL > > configuration vanish. Observed that gpy_config_init( ) function reset. > > Is it expected behaviour ? In other mail thread, we discussed that > > Ethtool configuration should retain after resume from sleep. > > The whole point of suspend/resume is that the configuration is retained. So i > would expect from the users perspective WoL is still enabled. > > As you point out, we might have a logic issue here, that on resume we hit the > PHY with a hardware reset. That could be clearing out WoL? We might need > to cache the PHY WoL configuration in phydev, and on resume re-apply it. > WoL is complex so we need to be careful who is actually managing it. But this > seems like something which could be done in the phylib core. >
Yes. You are correct. On resume we hit the PHY with a hardware reset. That could be clearing out WoL.
On resume, phy_init_hw( ) calls the PHY's configuration and interrupt functions to resets. As you suggested, I add the wolopts flag in phydev and update this when set_wol function execute. This change fix the issue. i.e. Re-apply WOL configuration on PHY in phy_suspend( ) function.
Please find the attached prototype code change (Temporary patch)for reference. I will submit this patch separately.
> > 2. when WOL configure with ethtool, Either Link-down and Link-up on > > CLI, WOL configuration vanish. Is it expected behaviour ? Due to this, > > every time we have to configure WOL through Ethtool. > > This is unexpected. I would expect WoL to remain configured until the > configuration is changed. >
Observe that magic packet configuration(ethtool's wol g option) did not vanish. But when Link flap occur for WAKE_PHY case, this issue observes. This might be recent "netlink" changes issue. I will debug and confirm.
> > Based on above information, We need to check for return < 0 condition > > and return the error. Else enable the wakeup by calling > > "device_set_wakeup_enable( )" function. > > Once it is clear how the PHY does the wakeup, we can look at this. However, if > the PHY can support the WoL, you should not be configuring the MAC as well > to do the same WoL, you should be putting the MAC into a low power mode. >
When PHY support's the WOL (WAKE_PHY, WAKE_MAGIC), it's generated only MDINT interrupt which can handle by MAC. But we must configure the wake phy interrupt bit in MAC's Power Management register.
PCI11x1x's ethernet device MAC supports "Secure-ON" features, but GPY211 does not support "Secure-ON". So, we have to use MAC WOL for secure-on feature.
I tested this part. When enable the MAC's WAKE_PHY_INT bit, when PHY generate the interrupt, System resumes from sleep.
> Andrew
Thanks, Raju [unhandled content-type:application/octet-stream] | ![\](/images/icornerr.gif) |