Re: [PATCH] gianfar: Add gfar_change_carrier()

[Joakim Tjernlund]

On Fri, 2018-12-07 at 12:40 -0800, Florian Fainelli wrote:
> 
> On 12/7/18 9:26 AM, Andrew Lunn wrote:
> > > Would you be happier if .ndo_change_carrier() only acted on Fixed PHYs?
> > 
> > I think it makes sense to allow a fixed phy carrier to be changed from
> > user space. However, i don't think you can easily plumb that to
> > .ndo_change_carrier(), since that is a MAC feature. You need to change
> > the fixed_phy_status to indicate the PHY has lost link, and then let
> > the usual mechanisms tell the MAC it is down and change the carrier
> > status.
> 
> Joakim, I still don't understand what did not work with:
> 
> - adding a ndo_change_carrier() interface which keeps a boolean flag
> whether the link was up or not
> 
> - register a fixed_link_update callback for your fixed PHY, which just
> propagates that flag back to the fixed PHY
> 
> and that should take care of having the carrier go down, as driven by
> the PHY state machine, for that fixed device.
> --
> Florian

Florian,I guess I did misunderstand you and I am still not sure. Consider
this pseudo impl., is that what you mean ? if so, is Andrew happy with this too?

--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -491,7 +491,28 @@ static int gfar_set_mac_addr(struct net_device *dev, void 
*p)
 
        return 0;
 }
+static int gfar_fixed_phy_link_update(struct net_device *dev,
+                                     struct fixed_phy_status *status)
+{
+       struct gfar_priviate *priv;
+
+       if (dev && dev->phydev && status) {
+               priv = netdev_priv(dev);
+               status->link = !priv->no_carrier;
+       }
+       return 0;
+}
+static int gfar_change_carrier(struct net_device *dev, bool new_carrier)
+{
+       struct phy_device *phydev = dev->phydev;
+       struct gfar_priviate *priv;
 
+       if (!phy_is_pseudo_fixed_link(phydev))
+               return -EINVAL;
+       priv = netdev_priv(dev);
+       priv->no_carrier = !new_carrier;
+       return 0;
+}
 static const struct net_device_ops gfar_netdev_ops = {
        .ndo_open = gfar_enet_open,
        .ndo_start_xmit = gfar_start_xmit,
@@ -502,6 +523,7 @@ static const struct net_device_ops gfar_netdev_ops = {
        .ndo_tx_timeout = gfar_timeout,
        .ndo_do_ioctl = gfar_ioctl,
        .ndo_get_stats = gfar_get_stats,
+       .ndo_change_carrier = gfar_change_carrier,
        .ndo_set_mac_address = gfar_set_mac_addr,
        .ndo_validate_addr = eth_validate_addr,
 #ifdef CONFIG_NET_POLL_CONTROLLER
@@ -1807,6 +1829,10 @@ static int init_phy(struct net_device *dev)
                return -ENODEV;
        }
 
+       if (phy_is_pseudo_fixed_link(phydev))
+               fixed_phy_set_link_update(phydev,
+                                         gfar_fixed_phy_link_update);
+
        if (interface == PHY_INTERFACE_MODE_SGMII)
                gfar_configure_serdes(dev);