Re: [PATCH V5 net-next 05/11] net: hibmcge: Implement some .ndo functions

[Andrew Lunn <andrew@lunn.ch>]

On Thu, Aug 29, 2024 at 07:43:39AM -0700, Jakub Kicinski wrote:
> On Thu, 29 Aug 2024 10:40:07 +0800 Jijie Shao wrote:
> > on 2024/8/29 9:39, Jakub Kicinski wrote:
> > > On Tue, 27 Aug 2024 21:14:49 +0800 Jijie Shao wrote:  
> > >> +static int hbg_net_open(struct net_device *dev)
> > >> +{
> > >> +	struct hbg_priv *priv = netdev_priv(dev);
> > >> +
> > >> +	if (test_and_set_bit(HBG_NIC_STATE_OPEN, &priv->state))
> > >> +		return 0;
> > >> +
> > >> +	netif_carrier_off(dev);  
> > > Why clear the carrier during open? You should probably clear it once on
> > > the probe path and then on stop.  
> > 
> > In net_open(), the GMAC is not ready to receive or transmit packets.
> > Therefore, netif_carrier_off() is called.
> > 
> > Packets can be received or transmitted only after the PHY is linked.
> > Therefore, netif_carrier_on() should be called in adjust_link.
> 
> But why are you calling _off() during .ndo_open() ?
> Surely the link is also off before ndo_open is called?

I wounder what driver they copied?

The general trend is .probe() calls netif_carrier_off(). After than,
phylib/phylink is in control of the carrier and the MAC driver does
not touch it. in fact, when using phylink, if you try to change the
carrier, you will get SHOUTED at from Russell.

	Andrew