Re: [EXTERNAL] Re: [PATCH net-next] net: mana: Support HW link state events

[Andrew Lunn <andrew@lunn.ch>]

> > > +		if (link_up) {
> > > +			netif_carrier_on(ndev);
> > > +
> > > +			if (apc->port_is_up)
> > > +				netif_tx_wake_all_queues(ndev);
> > > +
> > > +			__netdev_notify_peers(ndev);
> > > +		} else {
> > > +			if (netif_carrier_ok(ndev)) {
> > > +				netif_tx_disable(ndev);
> > > +				netif_carrier_off(ndev);
> > > +			}
> > > +		}
> > 
> > It is odd this is asymmetric. Up and down should really be opposites.
> For the up event, we need to delay the wake up queues if the 
> mana_close() is called, or mana_open() isn't called yet.
> 
> Also, we notify peers only when link up.

But why is this not symmetric?

On down, if port_is_up is not true, there is no need to disable tx and
set the carrier off. There are also counters associated with
netif_carrier_off() and netif_carrier_on(), and if you don't call them
in symmetric pairs, the counters are going to look odd.

> cancel_work_sync()'s doc says "This function can be used
> even if the work re-queues itself".
> cancel_delayed_work_sync() calls the same underlying function but 
> with WORK_CANCEL_DELAYED flag. So it should be OK.

O.K, thanks

	Andrew