lkml.org 
[lkml]   [2024]   [Apr]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH net V2 2/2] net: lan743x: support WOL in MAC even when PHY does not
    Date
    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]
    \
     
     \ /
      Last update: 2024-05-27 17:58    [W:3.779 / U:0.400 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site