Message-ID: <PH8PR11MB7965A7545D86EA3546BFAEE495102@PH8PR11MB7965.namprd11.prod.outlook.com>
Date: Wed, 24 Apr 2024 06:07:52 +0000
From: <Ronnie.Kunin@...rochip.com>
To: <andrew@...n.ch>, <Raju.Lakkaraju@...rochip.com>
CC: <netdev@...r.kernel.org>, <davem@...emloft.net>, <kuba@...nel.org>,
	<pabeni@...hat.com>, <edumazet@...gle.com>, <linux-kernel@...r.kernel.org>,
	<Bryan.Whitehead@...rochip.com>, <UNGLinuxDriver@...rochip.com>
Subject: RE: [PATCH net V2 2/2] net: lan743x: support WOL in MAC even when PHY
 does not

> While the registers that control enabling/disabling processing of the phy interrupt do reside within the MAC
> block's register set in the ethernet controller
Correction, the bits that control processing of the PHY interrupt are NOT within the actual MAC block register set either. 

> -----Original Message-----
> From: Ronnie Kunin - C21729 <Ronnie.Kunin@...rochip.com>
> Sent: Wednesday, April 24, 2024 1:59 AM
> To: Andrew Lunn <andrew@...n.ch>; Raju Lakkaraju - I30499 <Raju.Lakkaraju@...rochip.com>
> Cc: netdev@...r.kernel.org; davem@...emloft.net; kuba@...nel.org; pabeni@...hat.com;
> edumazet@...gle.com; linux-kernel@...r.kernel.org; Bryan Whitehead - C21958
> <Bryan.Whitehead@...rochip.com>; UNGLinuxDriver <UNGLinuxDriver@...rochip.com>
> Subject: RE: [PATCH net V2 2/2] net: lan743x: support WOL in MAC even when PHY does not
> 
> Thanks very much for your feedback and suggestions Andrew, some comments and clarifications below.
> 
> > -----Original Message-----
> > From: Andrew Lunn <andrew@...n.ch>
> > Sent: Tuesday, April 23, 2024 3:11 PM
> > To: Raju Lakkaraju - I30499 <Raju.Lakkaraju@...rochip.com>
> > Cc: netdev@...r.kernel.org; davem@...emloft.net; kuba@...nel.org;
> > pabeni@...hat.com; edumazet@...gle.com; linux-kernel@...r.kernel.org;
> > Bryan Whitehead - C21958 <Bryan.Whitehead@...rochip.com>;
> > UNGLinuxDriver <UNGLinuxDriver@...rochip.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 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.
> >
> > > 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.
> >
> > This is the bit that is missing from your commit message, and maybe it
> > should be in a comment. The interrupt controller is part of the MAC.
> > So you need to leave MAC burning power, maybe even processing packets,
> > because you cannot disable the MAC but leave the interrupt controller functioning, so that it can
> trigger a wake up via PCIe.
> 
> I think there is a terminology problem here. Within MCHP we sometimes call "the MAC" to the whole
> ethernet controller chip that has everything (i.e. actual MAC, FIFOs, filtering engines, offloads, interrupt
> controller, bus interface, etc) except the PHY.
> That is what Raju probably means when he says that the interrupt is handled by "the MAC". While the
> registers that control enabling/disabling processing of the phy interrupt do reside within the MAC
> block's register set in the ethernet controller, it neither means that the extensive parts of the actual MAC
> block need to be kept enabled nor that processing packets has to occur in the MAC in order for the
> PCI11x1x chip to detect an event coming from the PHY that should be processed as a wake event over
> PCIe
> 
> >
> > There are a few things you should consider:
> >
> > Call phy_speed_down().  This will renegotiate the link, dropping it to
> > the slowest speed both link partners support. So hopefully down to
> > 10Mbps. Your MAC will then only need to pointlessly process 10Mbps of
> > packets, rather than 1Gbps.p
> 
> I am Windows driver expert, not Linux so I may be wrong for Linux, but with the advent of dynamic PM
> in modern OSs (connected and then modern standby in Windows, I remember also autosuspend - at
> least in USB, maybe not applicable to PCIe  - in Linux ) we have stayed away from renegotiating the link
> to down speed during suspend - resume events since those interfere with / delay connectivity
> significantly.
> 
> >
> > See if you can disable parts of the MAC, in particularly the receive engine, in order to save power.
> 
> This is already supported by the hardware and I had told Raju to do it as soon as he successfully got the
> host to wake over PCIe from events signaled from the PHY
> 
> >
> > Talk to your hardware engineer and see if the next generation of the
> > hardware can separate the interrupt controller from the MAC, so you
> > can disable the MAC and leave the interrupt controller functioning.
> 
> As mentioned above, for all intents and purposes, that is already the case.
> 
> >
> > > Please find the attached prototype code change (Temporary patch)for reference.
> > > I will submit this patch separately.
> >
> > Please just submit it in the normal way for review.
> >
> >        Andrew

Powered by blists - more mailing lists