RE: [PATCH V1 net-next 0/3] net: stmmac: implement clocks

-----Original Message-----
From: Florian Fainelli <f.fainelli@gmail.com>
Sent: 2021年2月24日 11:54
To: Joakim Zhang <redacted>; Jakub Kicinski
[off-list ref]
Cc: peppe.cavallaro@st.com; alexandre.torgue@st.com;
joabreu@synopsys.com; davem@davemloft.net; netdev@vger.kernel.org;
dl-linux-imx [off-list ref]
Subject: Re: [PATCH V1 net-next 0/3] net: stmmac: implement clocks



On 2/23/2021 6:47 PM, Joakim Zhang wrote:

quoted

quoted

-----Original Message-----
From: Jakub Kicinski <kuba@kernel.org>
Sent: 2021年2月24日 10:35
To: Joakim Zhang <redacted>
Cc: peppe.cavallaro@st.com; alexandre.torgue@st.com;
joabreu@synopsys.com; davem@davemloft.net; netdev@vger.kernel.org;
dl-linux-imx [off-list ref]
Subject: Re: [PATCH V1 net-next 0/3] net: stmmac: implement clocks

On Wed, 24 Feb 2021 02:13:05 +0000 Joakim Zhang wrote:

quoted

quoted

quoted

The aim is to enable clocks when it needs, others keep clocks disabled.

Understood. Please double check ethtool callbacks work fine. People
often forget about those when disabling clocks in .close.

Hi Jakub,

If NIC is open then clocks are always enabled, so all ethtool
callbacks should be okay.

Could you point me which ethtool callbacks could be invoked when NIC
is closed? I'm not very familiar with ethtool use case. Thanks.

Well, all of them - ethtool does not check if the device is open.
User can access and configure the device when it's closed.
Often the callbacks access only driver data, but it's implementation
specific so you'll need to validate the callbacks stmmac implements.

Thanks Jakub, I will check these callbacks.

You can implement ethtool_ops::begin and ethtool_ops::complete where you
would enable the clock, and respectively disable it just for the time of the
operation. The ethtool framework guarantees that begin is called at the
beginning and complete at the end. You can also make sure that if the interface
is disabled you only return a cached copy of the settings/MIB counters (they
are not updating since the HW is disabled) and conversely only store
parameters in a cached structure and apply those when the network device
gets opened again. Either way would work.

Hi Florian,

Thanks for you hint. Yes, I noticed stmmac driver has implemented ethtool_ops::begin, which let ethtool only can be used when interface is enabled. Thanks a lot.

Best Regards,
Joakim Zhang

--
Florian