Re: [PATCH net-next v6 4/4] net: rnpgbe: Add link status handling support

[Andrew Lunn]

> +static int rnpgbe_process_link_event(struct mucse *mucse)
> +{
> +	struct net_device *netdev = mucse->netdev;
> +	struct mucse_hw *hw = &mucse->hw;
> +	unsigned long flags;
> +	bool link;
> +	int speed;
> +	u8 duplex;
> +
> +	/* lockless fast-path ,serv_task retry handles race */
> +	if (!(mucse->flags & M_FLAG_NEED_LINK_UPDATE))
> +		return hw->link;
> +
> +	spin_lock_irqsave(&mucse->link_lock, flags);
> +
> +	link = hw->link;
> +	speed = hw->speed;
> +	duplex = hw->duplex;
> +
> +	mucse->flags &= ~M_FLAG_NEED_LINK_UPDATE;
> +	spin_unlock_irqrestore(&mucse->link_lock, flags);
> +
> +	if (link) {
> +		netdev_info(netdev, "NIC Link is Up %d Mbps, %s Duplex\n",
> +			    speed,
> +			    duplex ? "Full" : "Half");
> +	}

I actually think down is more useful to report than up? Up is
generally the normal state, you don't care about that too
much. However if the link does down, you probably want to know so you
can figure out why.

> + * rnpgbe_link_is_down - Update netif_carrier status and
> + * print link down message
> + * @mucse: pointer to the private structure
> + **/
> +static void rnpgbe_link_is_down(struct mucse *mucse)
> +{
> +	struct net_device *netdev = mucse->netdev;
> +	struct mucse_hw *hw = &mucse->hw;
> +
> +	/* Only continue if link was up previously */
> +	if (!netif_carrier_ok(netdev))
> +		return;
> +	netdev_info(netdev, "NIC Link is Down\n");
> +	rnpgbe_set_link(hw, false);
> +	netif_carrier_off(netdev);

Why asymmetric behaviour.  Report up in one place down somewhere else?

	Andrew