public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed

* [PATCH net-next v2 0/3] net: dsa: use slave device phydev
@ 2017-09-22 19:40 Vivien Didelot
  2017-09-22 19:40 ` [PATCH net-next v2 1/3] " Vivien Didelot
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Vivien Didelot @ 2017-09-22 19:40 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

This patchset removes the private phy_device in favor of the one
provided by the slave net_device, makes slave open and close symmetrical
and finally provides helpers for enabling or disabling a DSA port.

Changes in v2:
  - do not remove the phy argument from port enable/disable

Vivien Didelot (3):
  net: dsa: use slave device phydev
  net: dsa: make slave close symmetrical to open
  net: dsa: add port enable and disable helpers

 net/dsa/dsa_priv.h |   3 +-
 net/dsa/port.c     |  31 +++++++++++-
 net/dsa/slave.c    | 143 +++++++++++++++++++++++------------------------------
 3 files changed, 94 insertions(+), 83 deletions(-)

-- 
2.14.1

^ permalink raw reply	[flat|nested] 7+ messages in thread

• * [PATCH net-next v2 1/3] net: dsa: use slave device phydev
    2017-09-22 19:40 [PATCH net-next v2 0/3] net: dsa: use slave device phydev Vivien Didelot
  @ 2017-09-22 19:40 ` Vivien Didelot
    2017-09-22 20:00   ` Florian Fainelli
    2017-09-22 20:03   ` Andrew Lunn
    2017-09-22 19:40 ` [PATCH net-next v2 2/3] net: dsa: make slave close symmetrical to open Vivien Didelot
    2017-09-22 19:40 ` [PATCH net-next v2 3/3] net: dsa: add port enable and disable helpers Vivien Didelot
    2 siblings, 2 replies; 7+ messages in thread
  From: Vivien Didelot @ 2017-09-22 19:40 UTC (permalink / raw)
    To: netdev
    Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
  	Andrew Lunn, Vivien Didelot
  
  There is no need to store a phy_device in dsa_slave_priv since
  net_device already provides one. Simply s/p->phy/dev->phydev/.
  
  While at it, return -ENODEV when it is NULL instead of -EOPNOTSUPP.
  
  Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux•com>
  ---
   net/dsa/slave.c | 126 ++++++++++++++++++++++++++------------------------------
   1 file changed, 58 insertions(+), 68 deletions(-)
  
  diff --git a/net/dsa/slave.c b/net/dsa/slave.c
  index 02ace7d462c4..3760472bf41d 100644
  --- a/net/dsa/slave.c
  +++ b/net/dsa/slave.c
  @@ -99,15 +99,15 @@ static int dsa_slave_open(struct net_device *dev)
   	}
   
   	if (ds->ops->port_enable) {
  -		err = ds->ops->port_enable(ds, p->dp->index, p->phy);
  +		err = ds->ops->port_enable(ds, p->dp->index, dev->phydev);
   		if (err)
   			goto clear_promisc;
   	}
   
   	dsa_port_set_state_now(p->dp, stp_state);
   
  -	if (p->phy)
  -		phy_start(p->phy);
  +	if (dev->phydev)
  +		phy_start(dev->phydev);
   
   	return 0;
   
  @@ -130,8 +130,8 @@ static int dsa_slave_close(struct net_device *dev)
   	struct net_device *master = dsa_master_netdev(p);
   	struct dsa_switch *ds = p->dp->ds;
   
  -	if (p->phy)
  -		phy_stop(p->phy);
  +	if (dev->phydev)
  +		phy_stop(dev->phydev);
   
   	dev_mc_unsync(master, dev);
   	dev_uc_unsync(master, dev);
  @@ -144,7 +144,7 @@ static int dsa_slave_close(struct net_device *dev)
   		dev_uc_del(master, dev->dev_addr);
   
   	if (ds->ops->port_disable)
  -		ds->ops->port_disable(ds, p->dp->index, p->phy);
  +		ds->ops->port_disable(ds, p->dp->index, dev->phydev);
   
   	dsa_port_set_state_now(p->dp, BR_STATE_DISABLED);
   
  @@ -273,12 +273,10 @@ dsa_slave_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb,
   
   static int dsa_slave_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
   {
  -	struct dsa_slave_priv *p = netdev_priv(dev);
  +	if (!dev->phydev)
  +		return -ENODEV;
   
  -	if (p->phy != NULL)
  -		return phy_mii_ioctl(p->phy, ifr, cmd);
  -
  -	return -EOPNOTSUPP;
  +	return phy_mii_ioctl(dev->phydev, ifr, cmd);
   }
   
   static int dsa_slave_port_attr_set(struct net_device *dev,
  @@ -435,12 +433,10 @@ static int
   dsa_slave_get_link_ksettings(struct net_device *dev,
   			     struct ethtool_link_ksettings *cmd)
   {
  -	struct dsa_slave_priv *p = netdev_priv(dev);
  +	if (!dev->phydev)
  +		return -ENODEV;
   
  -	if (!p->phy)
  -		return -EOPNOTSUPP;
  -
  -	phy_ethtool_ksettings_get(p->phy, cmd);
  +	phy_ethtool_ksettings_get(dev->phydev, cmd);
   
   	return 0;
   }
  @@ -449,12 +445,10 @@ static int
   dsa_slave_set_link_ksettings(struct net_device *dev,
   			     const struct ethtool_link_ksettings *cmd)
   {
  -	struct dsa_slave_priv *p = netdev_priv(dev);
  +	if (!dev->phydev)
  +		return -ENODEV;
   
  -	if (p->phy != NULL)
  -		return phy_ethtool_ksettings_set(p->phy, cmd);
  -
  -	return -EOPNOTSUPP;
  +	return phy_ethtool_ksettings_set(dev->phydev, cmd);
   }
   
   static void dsa_slave_get_drvinfo(struct net_device *dev,
  @@ -488,24 +482,20 @@ dsa_slave_get_regs(struct net_device *dev, struct ethtool_regs *regs, void *_p)
   
   static int dsa_slave_nway_reset(struct net_device *dev)
   {
  -	struct dsa_slave_priv *p = netdev_priv(dev);
  +	if (!dev->phydev)
  +		return -ENODEV;
   
  -	if (p->phy != NULL)
  -		return genphy_restart_aneg(p->phy);
  -
  -	return -EOPNOTSUPP;
  +	return genphy_restart_aneg(dev->phydev);
   }
   
   static u32 dsa_slave_get_link(struct net_device *dev)
   {
  -	struct dsa_slave_priv *p = netdev_priv(dev);
  +	if (!dev->phydev)
  +		return -ENODEV;
   
  -	if (p->phy != NULL) {
  -		genphy_update_link(p->phy);
  -		return p->phy->link;
  -	}
  +	genphy_update_link(dev->phydev);
   
  -	return -EOPNOTSUPP;
  +	return dev->phydev->link;
   }
   
   static int dsa_slave_get_eeprom_len(struct net_device *dev)
  @@ -640,7 +630,7 @@ static int dsa_slave_set_eee(struct net_device *dev, struct ethtool_eee *e)
   	int ret;
   
   	/* Port's PHY and MAC both need to be EEE capable */
  -	if (!p->phy)
  +	if (!dev->phydev)
   		return -ENODEV;
   
   	if (!ds->ops->set_mac_eee)
  @@ -651,12 +641,12 @@ static int dsa_slave_set_eee(struct net_device *dev, struct ethtool_eee *e)
   		return ret;
   
   	if (e->eee_enabled) {
  -		ret = phy_init_eee(p->phy, 0);
  +		ret = phy_init_eee(dev->phydev, 0);
   		if (ret)
   			return ret;
   	}
   
  -	return phy_ethtool_set_eee(p->phy, e);
  +	return phy_ethtool_set_eee(dev->phydev, e);
   }
   
   static int dsa_slave_get_eee(struct net_device *dev, struct ethtool_eee *e)
  @@ -666,7 +656,7 @@ static int dsa_slave_get_eee(struct net_device *dev, struct ethtool_eee *e)
   	int ret;
   
   	/* Port's PHY and MAC both need to be EEE capable */
  -	if (!p->phy)
  +	if (!dev->phydev)
   		return -ENODEV;
   
   	if (!ds->ops->get_mac_eee)
  @@ -676,7 +666,7 @@ static int dsa_slave_get_eee(struct net_device *dev, struct ethtool_eee *e)
   	if (ret)
   		return ret;
   
  -	return phy_ethtool_get_eee(p->phy, e);
  +	return phy_ethtool_get_eee(dev->phydev, e);
   }
   
   #ifdef CONFIG_NET_POLL_CONTROLLER
  @@ -985,26 +975,26 @@ static void dsa_slave_adjust_link(struct net_device *dev)
   	struct dsa_switch *ds = p->dp->ds;
   	unsigned int status_changed = 0;
   
  -	if (p->old_link != p->phy->link) {
  +	if (p->old_link != dev->phydev->link) {
   		status_changed = 1;
  -		p->old_link = p->phy->link;
  +		p->old_link = dev->phydev->link;
   	}
   
  -	if (p->old_duplex != p->phy->duplex) {
  +	if (p->old_duplex != dev->phydev->duplex) {
   		status_changed = 1;
  -		p->old_duplex = p->phy->duplex;
  +		p->old_duplex = dev->phydev->duplex;
   	}
   
  -	if (p->old_pause != p->phy->pause) {
  +	if (p->old_pause != dev->phydev->pause) {
   		status_changed = 1;
  -		p->old_pause = p->phy->pause;
  +		p->old_pause = dev->phydev->pause;
   	}
   
   	if (ds->ops->adjust_link && status_changed)
  -		ds->ops->adjust_link(ds, p->dp->index, p->phy);
  +		ds->ops->adjust_link(ds, p->dp->index, dev->phydev);
   
   	if (status_changed)
  -		phy_print_status(p->phy);
  +		phy_print_status(dev->phydev);
   }
   
   static int dsa_slave_fixed_link_update(struct net_device *dev,
  @@ -1029,17 +1019,18 @@ static int dsa_slave_phy_connect(struct net_device *slave_dev, int addr)
   	struct dsa_slave_priv *p = netdev_priv(slave_dev);
   	struct dsa_switch *ds = p->dp->ds;
   
  -	p->phy = mdiobus_get_phy(ds->slave_mii_bus, addr);
  -	if (!p->phy) {
  +	slave_dev->phydev = mdiobus_get_phy(ds->slave_mii_bus, addr);
  +	if (!slave_dev->phydev) {
   		netdev_err(slave_dev, "no phy at %d\n", addr);
   		return -ENODEV;
   	}
   
   	/* Use already configured phy mode */
   	if (p->phy_interface == PHY_INTERFACE_MODE_NA)
  -		p->phy_interface = p->phy->interface;
  -	return phy_connect_direct(slave_dev, p->phy, dsa_slave_adjust_link,
  -				  p->phy_interface);
  +		p->phy_interface = slave_dev->phydev->interface;
  +
  +	return phy_connect_direct(slave_dev, slave_dev->phydev,
  +				  dsa_slave_adjust_link, p->phy_interface);
   }
   
   static int dsa_slave_phy_setup(struct net_device *slave_dev)
  @@ -1091,22 +1082,23 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev)
   				return ret;
   			}
   		} else {
  -			p->phy = of_phy_connect(slave_dev, phy_dn,
  -						dsa_slave_adjust_link,
  -						phy_flags,
  -						p->phy_interface);
  +			slave_dev->phydev = of_phy_connect(slave_dev, phy_dn,
  +							   dsa_slave_adjust_link,
  +							   phy_flags,
  +							   p->phy_interface);
   		}
   
   		of_node_put(phy_dn);
   	}
   
  -	if (p->phy && phy_is_fixed)
  -		fixed_phy_set_link_update(p->phy, dsa_slave_fixed_link_update);
  +	if (slave_dev->phydev && phy_is_fixed)
  +		fixed_phy_set_link_update(slave_dev->phydev,
  +					  dsa_slave_fixed_link_update);
   
   	/* We could not connect to a designated PHY, so use the switch internal
   	 * MDIO bus instead
   	 */
  -	if (!p->phy) {
  +	if (!slave_dev->phydev) {
   		ret = dsa_slave_phy_connect(slave_dev, p->dp->index);
   		if (ret) {
   			netdev_err(slave_dev, "failed to connect to port %d: %d\n",
  @@ -1117,7 +1109,7 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev)
   		}
   	}
   
  -	phy_attached_info(p->phy);
  +	phy_attached_info(slave_dev->phydev);
   
   	return 0;
   }
  @@ -1137,12 +1129,12 @@ int dsa_slave_suspend(struct net_device *slave_dev)
   
   	netif_device_detach(slave_dev);
   
  -	if (p->phy) {
  -		phy_stop(p->phy);
  +	if (slave_dev->phydev) {
  +		phy_stop(slave_dev->phydev);
   		p->old_pause = -1;
   		p->old_link = -1;
   		p->old_duplex = -1;
  -		phy_suspend(p->phy);
  +		phy_suspend(slave_dev->phydev);
   	}
   
   	return 0;
  @@ -1150,13 +1142,11 @@ int dsa_slave_suspend(struct net_device *slave_dev)
   
   int dsa_slave_resume(struct net_device *slave_dev)
   {
  -	struct dsa_slave_priv *p = netdev_priv(slave_dev);
  -
   	netif_device_attach(slave_dev);
   
  -	if (p->phy) {
  -		phy_resume(p->phy);
  -		phy_start(p->phy);
  +	if (slave_dev->phydev) {
  +		phy_resume(slave_dev->phydev);
  +		phy_start(slave_dev->phydev);
   	}
   
   	return 0;
  @@ -1249,8 +1239,8 @@ void dsa_slave_destroy(struct net_device *slave_dev)
   	port_dn = p->dp->dn;
   
   	netif_carrier_off(slave_dev);
  -	if (p->phy) {
  -		phy_disconnect(p->phy);
  +	if (slave_dev->phydev) {
  +		phy_disconnect(slave_dev->phydev);
   
   		if (of_phy_is_fixed_link(port_dn))
   			of_phy_deregister_fixed_link(port_dn);
  -- 
  2.14.1
  
  ^ permalink raw reply related	[flat|nested] 7+ messages in thread

• • * Re: [PATCH net-next v2 1/3] net: dsa: use slave device phydev
      2017-09-22 19:40 ` [PATCH net-next v2 1/3] " Vivien Didelot
    @ 2017-09-22 20:00   ` Florian Fainelli
      2017-09-22 20:11     ` Vivien Didelot
      2017-09-22 20:03   ` Andrew Lunn
      1 sibling, 1 reply; 7+ messages in thread
    From: Florian Fainelli @ 2017-09-22 20:00 UTC (permalink / raw)
      To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
    
    On 09/22/2017 12:40 PM, Vivien Didelot wrote:
    > There is no need to store a phy_device in dsa_slave_priv since
    > net_device already provides one. Simply s/p->phy/dev->phydev/.
    
    You can therefore remove the phy_device from dsa_slave_priv, see below
    for more comments. I will have to regress test the heck out of this,
    this should take a few hours.
    
    > 
    > While at it, return -ENODEV when it is NULL instead of -EOPNOTSUPP.
    > 
    > Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux•com>
    > ---
    
    >  static int dsa_slave_port_attr_set(struct net_device *dev,
    > @@ -435,12 +433,10 @@ static int
    >  dsa_slave_get_link_ksettings(struct net_device *dev,
    >  			     struct ethtool_link_ksettings *cmd)
    >  {
    > -	struct dsa_slave_priv *p = netdev_priv(dev);
    > +	if (!dev->phydev)
    > +		return -ENODEV;
    >  
    > -	if (!p->phy)
    > -		return -EOPNOTSUPP;
    > -
    > -	phy_ethtool_ksettings_get(p->phy, cmd);
    > +	phy_ethtool_ksettings_get(dev->phydev, cmd);
    
    This can be replaced by phy_ethtool_get_link_ksettings()
    
    >  
    >  	return 0;
    >  }
    > @@ -449,12 +445,10 @@ static int
    >  dsa_slave_set_link_ksettings(struct net_device *dev,
    >  			     const struct ethtool_link_ksettings *cmd)
    >  {
    > -	struct dsa_slave_priv *p = netdev_priv(dev);
    > +	if (!dev->phydev)
    > +		return -ENODEV;
    >  
    > -	if (p->phy != NULL)
    > -		return phy_ethtool_ksettings_set(p->phy, cmd);
    > -
    > -	return -EOPNOTSUPP;
    > +	return phy_ethtool_ksettings_set(dev->phydev, cmd);
    >  }
    
    This can disappear and you can assign this ethtool operation to
    phy_ethtool_set_link_ksettings()
    
    >  
    >  static void dsa_slave_get_drvinfo(struct net_device *dev,
    > @@ -488,24 +482,20 @@ dsa_slave_get_regs(struct net_device *dev, struct ethtool_regs *regs, void *_p)
    >  
    >  static int dsa_slave_nway_reset(struct net_device *dev)
    >  {
    > -	struct dsa_slave_priv *p = netdev_priv(dev);
    > +	if (!dev->phydev)
    > +		return -ENODEV;
    >  
    > -	if (p->phy != NULL)
    > -		return genphy_restart_aneg(p->phy);
    > -
    > -	return -EOPNOTSUPP;
    > +	return genphy_restart_aneg(dev->phydev);
    >  }
    
    This can now disappear and you can use phy_ethtool_nway_reset() directly
    in ethtool_ops
    
    >  
    >  static u32 dsa_slave_get_link(struct net_device *dev)
    >  {
    > -	struct dsa_slave_priv *p = netdev_priv(dev);
    > +	if (!dev->phydev)
    > +		return -ENODEV;
    >  
    > -	if (p->phy != NULL) {
    > -		genphy_update_link(p->phy);
    > -		return p->phy->link;
    > -	}
    > +	genphy_update_link(dev->phydev);
    >  
    > -	return -EOPNOTSUPP;
    > +	return dev->phydev->link;
    >  }
    
    This should certainly be just ethtool_op_get_link(), not sure why we
    kept that around here...
    -- 
    Florian
    
    ^ permalink raw reply	[flat|nested] 7+ messages in thread

  • • * Re: [PATCH net-next v2 1/3] net: dsa: use slave device phydev
        2017-09-22 20:00   ` Florian Fainelli
      @ 2017-09-22 20:11     ` Vivien Didelot
        0 siblings, 0 replies; 7+ messages in thread
      From: Vivien Didelot @ 2017-09-22 20:11 UTC (permalink / raw)
        To: Florian Fainelli, netdev
        Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
      
      Hi Florian,
      
      Florian Fainelli <f.fainelli@gmail•com> writes:
      
      > On 09/22/2017 12:40 PM, Vivien Didelot wrote:
      >> There is no need to store a phy_device in dsa_slave_priv since
      >> net_device already provides one. Simply s/p->phy/dev->phydev/.
      >
      > You can therefore remove the phy_device from dsa_slave_priv, see below
      > for more comments. I will have to regress test the heck out of this,
      > this should take a few hours.
      
      OK, since this is a sensible topic, I will respin a v3 without this
      patch, so that a future patchset can address your comments below and
      also gives you time to test this one patch alone.
      
      >>  static int dsa_slave_port_attr_set(struct net_device *dev,
      >> @@ -435,12 +433,10 @@ static int
      >>  dsa_slave_get_link_ksettings(struct net_device *dev,
      >>  			     struct ethtool_link_ksettings *cmd)
      >>  {
      >> -	struct dsa_slave_priv *p = netdev_priv(dev);
      >> +	if (!dev->phydev)
      >> +		return -ENODEV;
      >>  
      >> -	if (!p->phy)
      >> -		return -EOPNOTSUPP;
      >> -
      >> -	phy_ethtool_ksettings_get(p->phy, cmd);
      >> +	phy_ethtool_ksettings_get(dev->phydev, cmd);
      >
      > This can be replaced by phy_ethtool_get_link_ksettings()
      >
      >>  
      >>  	return 0;
      >>  }
      >> @@ -449,12 +445,10 @@ static int
      >>  dsa_slave_set_link_ksettings(struct net_device *dev,
      >>  			     const struct ethtool_link_ksettings *cmd)
      >>  {
      >> -	struct dsa_slave_priv *p = netdev_priv(dev);
      >> +	if (!dev->phydev)
      >> +		return -ENODEV;
      >>  
      >> -	if (p->phy != NULL)
      >> -		return phy_ethtool_ksettings_set(p->phy, cmd);
      >> -
      >> -	return -EOPNOTSUPP;
      >> +	return phy_ethtool_ksettings_set(dev->phydev, cmd);
      >>  }
      >
      > This can disappear and you can assign this ethtool operation to
      > phy_ethtool_set_link_ksettings()
      >
      >>  
      >>  static void dsa_slave_get_drvinfo(struct net_device *dev,
      >> @@ -488,24 +482,20 @@ dsa_slave_get_regs(struct net_device *dev, struct ethtool_regs *regs, void *_p)
      >>  
      >>  static int dsa_slave_nway_reset(struct net_device *dev)
      >>  {
      >> -	struct dsa_slave_priv *p = netdev_priv(dev);
      >> +	if (!dev->phydev)
      >> +		return -ENODEV;
      >>  
      >> -	if (p->phy != NULL)
      >> -		return genphy_restart_aneg(p->phy);
      >> -
      >> -	return -EOPNOTSUPP;
      >> +	return genphy_restart_aneg(dev->phydev);
      >>  }
      >
      > This can now disappear and you can use phy_ethtool_nway_reset() directly
      > in ethtool_ops
      >
      >>  
      >>  static u32 dsa_slave_get_link(struct net_device *dev)
      >>  {
      >> -	struct dsa_slave_priv *p = netdev_priv(dev);
      >> +	if (!dev->phydev)
      >> +		return -ENODEV;
      >>  
      >> -	if (p->phy != NULL) {
      >> -		genphy_update_link(p->phy);
      >> -		return p->phy->link;
      >> -	}
      >> +	genphy_update_link(dev->phydev);
      >>  
      >> -	return -EOPNOTSUPP;
      >> +	return dev->phydev->link;
      >>  }
      >
      > This should certainly be just ethtool_op_get_link(), not sure why we
      > kept that around here...
      
      Haaa, good to read that! I wasn't sure about this, but with this patch
      the slave phy ethtool functions seemed indeed quite generic...
      
      
      Thanks,
      
              Vivien
      
      ^ permalink raw reply	[flat|nested] 7+ messages in thread

  • * Re: [PATCH net-next v2 1/3] net: dsa: use slave device phydev
      2017-09-22 19:40 ` [PATCH net-next v2 1/3] " Vivien Didelot
      2017-09-22 20:00   ` Florian Fainelli
    @ 2017-09-22 20:03   ` Andrew Lunn
      1 sibling, 0 replies; 7+ messages in thread
    From: Andrew Lunn @ 2017-09-22 20:03 UTC (permalink / raw)
      To: Vivien Didelot
      Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
    
    On Fri, Sep 22, 2017 at 03:40:43PM -0400, Vivien Didelot wrote:
    > There is no need to store a phy_device in dsa_slave_priv since
    > net_device already provides one. Simply s/p->phy/dev->phydev/.
    > 
    > While at it, return -ENODEV when it is NULL instead of -EOPNOTSUPP.
    
    I just did a quick poll for calling phy_mii_ioctl(). ENODEV seems the
    most popular, second to EINVAL. Marvell drivers all use EOPNOTSUPP.
    
    >  static int dsa_slave_nway_reset(struct net_device *dev)
    >  {
    > -	struct dsa_slave_priv *p = netdev_priv(dev);
    > +	if (!dev->phydev)
    > +		return -ENODEV;
    >  
    > -	if (p->phy != NULL)
    > -		return genphy_restart_aneg(p->phy);
    > -
    > -	return -EOPNOTSUPP;
    > +	return genphy_restart_aneg(dev->phydev);
    >  }
    
    It looks like this can now be replaced with phy_ethtool_nway_reset().
    
    It could be there are other phy_ethtool_ helpers which can be used,
    now that we have phydev in ndev.
    
        Andrew
    
    ^ permalink raw reply	[flat|nested] 7+ messages in thread

• * [PATCH net-next v2 2/3] net: dsa: make slave close symmetrical to open
    2017-09-22 19:40 [PATCH net-next v2 0/3] net: dsa: use slave device phydev Vivien Didelot
    2017-09-22 19:40 ` [PATCH net-next v2 1/3] " Vivien Didelot
  @ 2017-09-22 19:40 ` Vivien Didelot
    2017-09-22 19:40 ` [PATCH net-next v2 3/3] net: dsa: add port enable and disable helpers Vivien Didelot
    2 siblings, 0 replies; 7+ messages in thread
  From: Vivien Didelot @ 2017-09-22 19:40 UTC (permalink / raw)
    To: netdev
    Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
  	Andrew Lunn, Vivien Didelot
  
  The DSA slave open function configures the unicast MAC addresses on the
  master device, enable the switch port, change its STP state, then start
  the PHY device.
  
  Make the close function symmetric, by first stopping the PHY device,
  then changing the STP state, disabling the switch port and restore the
  master device.
  
  Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux•com>
  Reviewed-by: Florian Fainelli <f.fainelli@gmail•com>
  Reviewed-by: Andrew Lunn <andrew@lunn•ch>
  ---
   net/dsa/slave.c | 10 +++++-----
   1 file changed, 5 insertions(+), 5 deletions(-)
  
  diff --git a/net/dsa/slave.c b/net/dsa/slave.c
  index 3760472bf41d..0aab29928152 100644
  --- a/net/dsa/slave.c
  +++ b/net/dsa/slave.c
  @@ -133,6 +133,11 @@ static int dsa_slave_close(struct net_device *dev)
   	if (dev->phydev)
   		phy_stop(dev->phydev);
   
  +	dsa_port_set_state_now(p->dp, BR_STATE_DISABLED);
  +
  +	if (ds->ops->port_disable)
  +		ds->ops->port_disable(ds, p->dp->index, dev->phydev);
  +
   	dev_mc_unsync(master, dev);
   	dev_uc_unsync(master, dev);
   	if (dev->flags & IFF_ALLMULTI)
  @@ -143,11 +148,6 @@ static int dsa_slave_close(struct net_device *dev)
   	if (!ether_addr_equal(dev->dev_addr, master->dev_addr))
   		dev_uc_del(master, dev->dev_addr);
   
  -	if (ds->ops->port_disable)
  -		ds->ops->port_disable(ds, p->dp->index, dev->phydev);
  -
  -	dsa_port_set_state_now(p->dp, BR_STATE_DISABLED);
  -
   	return 0;
   }
   
  -- 
  2.14.1
  
  ^ permalink raw reply related	[flat|nested] 7+ messages in thread

• * [PATCH net-next v2 3/3] net: dsa: add port enable and disable helpers
    2017-09-22 19:40 [PATCH net-next v2 0/3] net: dsa: use slave device phydev Vivien Didelot
    2017-09-22 19:40 ` [PATCH net-next v2 1/3] " Vivien Didelot
    2017-09-22 19:40 ` [PATCH net-next v2 2/3] net: dsa: make slave close symmetrical to open Vivien Didelot
  @ 2017-09-22 19:40 ` Vivien Didelot
    2 siblings, 0 replies; 7+ messages in thread
  From: Vivien Didelot @ 2017-09-22 19:40 UTC (permalink / raw)
    To: netdev
    Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
  	Andrew Lunn, Vivien Didelot
  
  Provide dsa_port_enable and dsa_port_disable helpers to respectively
  enable and disable a switch port. This makes the dsa_port_set_state_now
  helper static.
  
  Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux•com>
  Reviewed-by: Florian Fainelli <f.fainelli@gmail•com>
  Reviewed-by: Andrew Lunn <andrew@lunn•ch>
  ---
   net/dsa/dsa_priv.h |  3 ++-
   net/dsa/port.c     | 31 ++++++++++++++++++++++++++++++-
   net/dsa/slave.c    | 19 +++++--------------
   3 files changed, 37 insertions(+), 16 deletions(-)
  
  diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
  index 9803952a5b40..0298a0f6a349 100644
  --- a/net/dsa/dsa_priv.h
  +++ b/net/dsa/dsa_priv.h
  @@ -117,7 +117,8 @@ void dsa_master_ethtool_restore(struct net_device *dev);
   /* port.c */
   int dsa_port_set_state(struct dsa_port *dp, u8 state,
   		       struct switchdev_trans *trans);
  -void dsa_port_set_state_now(struct dsa_port *dp, u8 state);
  +int dsa_port_enable(struct dsa_port *dp, struct phy_device *phy);
  +void dsa_port_disable(struct dsa_port *dp, struct phy_device *phy);
   int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br);
   void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br);
   int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
  diff --git a/net/dsa/port.c b/net/dsa/port.c
  index 76d43a82d397..72c8dbd3d3f2 100644
  --- a/net/dsa/port.c
  +++ b/net/dsa/port.c
  @@ -56,7 +56,7 @@ int dsa_port_set_state(struct dsa_port *dp, u8 state,
   	return 0;
   }
   
  -void dsa_port_set_state_now(struct dsa_port *dp, u8 state)
  +static void dsa_port_set_state_now(struct dsa_port *dp, u8 state)
   {
   	int err;
   
  @@ -65,6 +65,35 @@ void dsa_port_set_state_now(struct dsa_port *dp, u8 state)
   		pr_err("DSA: failed to set STP state %u (%d)\n", state, err);
   }
   
  +int dsa_port_enable(struct dsa_port *dp, struct phy_device *phy)
  +{
  +	u8 stp_state = dp->bridge_dev ? BR_STATE_BLOCKING : BR_STATE_FORWARDING;
  +	struct dsa_switch *ds = dp->ds;
  +	int port = dp->index;
  +	int err;
  +
  +	if (ds->ops->port_enable) {
  +		err = ds->ops->port_enable(ds, port, phy);
  +		if (err)
  +			return err;
  +	}
  +
  +	dsa_port_set_state_now(dp, stp_state);
  +
  +	return 0;
  +}
  +
  +void dsa_port_disable(struct dsa_port *dp, struct phy_device *phy)
  +{
  +	struct dsa_switch *ds = dp->ds;
  +	int port = dp->index;
  +
  +	dsa_port_set_state_now(dp, BR_STATE_DISABLED);
  +
  +	if (ds->ops->port_disable)
  +		ds->ops->port_disable(ds, port, phy);
  +}
  +
   int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br)
   {
   	struct dsa_notifier_bridge_info info = {
  diff --git a/net/dsa/slave.c b/net/dsa/slave.c
  index 0aab29928152..4ea1c6eb0da8 100644
  --- a/net/dsa/slave.c
  +++ b/net/dsa/slave.c
  @@ -73,9 +73,7 @@ static int dsa_slave_open(struct net_device *dev)
   {
   	struct dsa_slave_priv *p = netdev_priv(dev);
   	struct dsa_port *dp = p->dp;
  -	struct dsa_switch *ds = dp->ds;
   	struct net_device *master = dsa_master_netdev(p);
  -	u8 stp_state = dp->bridge_dev ? BR_STATE_BLOCKING : BR_STATE_FORWARDING;
   	int err;
   
   	if (!(master->flags & IFF_UP))
  @@ -98,13 +96,9 @@ static int dsa_slave_open(struct net_device *dev)
   			goto clear_allmulti;
   	}
   
  -	if (ds->ops->port_enable) {
  -		err = ds->ops->port_enable(ds, p->dp->index, dev->phydev);
  -		if (err)
  -			goto clear_promisc;
  -	}
  -
  -	dsa_port_set_state_now(p->dp, stp_state);
  +	err = dsa_port_enable(dp, dev->phydev);
  +	if (err)
  +		goto clear_promisc;
   
   	if (dev->phydev)
   		phy_start(dev->phydev);
  @@ -128,15 +122,12 @@ static int dsa_slave_close(struct net_device *dev)
   {
   	struct dsa_slave_priv *p = netdev_priv(dev);
   	struct net_device *master = dsa_master_netdev(p);
  -	struct dsa_switch *ds = p->dp->ds;
  +	struct dsa_port *dp = p->dp;
   
   	if (dev->phydev)
   		phy_stop(dev->phydev);
   
  -	dsa_port_set_state_now(p->dp, BR_STATE_DISABLED);
  -
  -	if (ds->ops->port_disable)
  -		ds->ops->port_disable(ds, p->dp->index, dev->phydev);
  +	dsa_port_disable(dp, dev->phydev);
   
   	dev_mc_unsync(master, dev);
   	dev_uc_unsync(master, dev);
  -- 
  2.14.1
  
  ^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-09-22 20:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-22 19:40 [PATCH net-next v2 0/3] net: dsa: use slave device phydev Vivien Didelot
2017-09-22 19:40 ` [PATCH net-next v2 1/3] " Vivien Didelot
2017-09-22 20:00   ` Florian Fainelli
2017-09-22 20:11     ` Vivien Didelot
2017-09-22 20:03   ` Andrew Lunn
2017-09-22 19:40 ` [PATCH net-next v2 2/3] net: dsa: make slave close symmetrical to open Vivien Didelot
2017-09-22 19:40 ` [PATCH net-next v2 3/3] net: dsa: add port enable and disable helpers Vivien Didelot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox