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

[Joakim Zhang]

> -----Original Message-----
> From: Jakub Kicinski <k...@kernel.org>
> Sent: 2021年2月24日 10:35
> To: Joakim Zhang <qiangqing.zh...@nxp.com>
> Cc: peppe.cavall...@st.com; alexandre.tor...@st.com;
> joab...@synopsys.com; da...@davemloft.net; netdev@vger.kernel.org;
> dl-linux-imx <linux-...@nxp.com>
> Subject: Re: [PATCH V1 net-next 0/3] net: stmmac: implement clocks
> 
> On Wed, 24 Feb 2021 02:13:05 +0000 Joakim Zhang wrote:
> > > > 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.

Hi Jakub,

I check the code, ethtool from stmmac driver only can be used when net is 
running now, so the clocks are enabled.
net/ethtool/ioctl.c -> dev_ethtool()
        [...]
        if (dev->ethtool_ops->begin) {
                rc = dev->ethtool_ops->begin(dev);
                if (rc < 0)
                        return rc;
        }
        [...]

Stmmac driver implement begin callback like below:
        static int stmmac_check_if_running(struct net_device *dev)
        {
                if (!netif_running(dev))
                        return -EBUSY;
                return 0;
        }

Best Regards,
Joakim Zhang