lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Wed, 28 Mar 2018 10:50:03 +0100
From:   Russell King - ARM Linux <linux@...linux.org.uk>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     netdev@...r.kernel.org, privat@...l-hjelmeland.no, andrew@...n.ch,
        vivien.didelot@...oirfairelinux.com, davem@...emloft.net,
        sean.wang@...iatek.com, Woojung.Huh@...rochip.com,
        john@...ozen.org, cphealy@...il.com
Subject: Re: [PATCH net-next 2/4] net: phy: phylink: Provide PHY interface to
 mac_link_{up,down}

On Sun, Mar 18, 2018 at 11:52:44AM -0700, Florian Fainelli wrote:
> In preparation for having DSA transition entirely to PHYLINK, we need to pass a
> PHY interface type to the mac_link_{up,down} callbacks because we may have to
> make decisions on that (e.g: turn on/off RGMII interfaces etc.). We do not pass
> an entire phylink_link_state because not all parameters (pause, duplex etc.) are
> defined when the link is down, only link and interface are.

If we're going to make this change, we ought to decide whether David
should pick this up for the coming merge window or not independently
of the remaining patches - there are other users of phylink in the
pipeline (bootlin are working on mvpp2 support, so this will be a
minor source of build error pain for folk.)

To that end,

Acked-by: Russell King <rmk+kernel@...linux.org.uk>

However, the documentation probably ought to make it clear that the
configuration of the interface mode of the MAC should always happen
in the mac_config() callback, not in the mac_link_*() functions.

Thanks.

> Update mvneta accordingly since it currently implements phylink_mac_ops.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
> ---
>  drivers/net/ethernet/marvell/mvneta.c |  4 +++-
>  drivers/net/phy/phylink.c             |  6 +++++-
>  include/linux/phylink.h               | 10 ++++++++--
>  3 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index 25e9a551cc8c..60de9b8d62c2 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -3396,7 +3396,8 @@ static void mvneta_set_eee(struct mvneta_port *pp, bool enable)
>  	mvreg_write(pp, MVNETA_LPI_CTRL_1, lpi_ctl1);
>  }
>  
> -static void mvneta_mac_link_down(struct net_device *ndev, unsigned int mode)
> +static void mvneta_mac_link_down(struct net_device *ndev, unsigned int mode,
> +				 phy_interface_t interface)
>  {
>  	struct mvneta_port *pp = netdev_priv(ndev);
>  	u32 val;
> @@ -3415,6 +3416,7 @@ static void mvneta_mac_link_down(struct net_device *ndev, unsigned int mode)
>  }
>  
>  static void mvneta_mac_link_up(struct net_device *ndev, unsigned int mode,
> +			       phy_interface_t interface,
>  			       struct phy_device *phy)
>  {
>  	struct mvneta_port *pp = netdev_priv(ndev);
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 51a011a349fe..cef3c1356a8c 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -423,8 +423,10 @@ static void phylink_resolve(struct work_struct *w)
>  	if (pl->phylink_disable_state) {
>  		pl->mac_link_dropped = false;
>  		link_state.link = false;
> +		link_state.interface = pl->phy_state.interface;
>  	} else if (pl->mac_link_dropped) {
>  		link_state.link = false;
> +		link_state.interface = pl->phy_state.interface;
>  	} else {
>  		switch (pl->link_an_mode) {
>  		case MLO_AN_PHY:
> @@ -470,10 +472,12 @@ static void phylink_resolve(struct work_struct *w)
>  	if (link_state.link != netif_carrier_ok(ndev)) {
>  		if (!link_state.link) {
>  			netif_carrier_off(ndev);
> -			pl->ops->mac_link_down(ndev, pl->link_an_mode);
> +			pl->ops->mac_link_down(ndev, pl->link_an_mode,
> +					       pl->phy_state.interface);
>  			netdev_info(ndev, "Link is Down\n");
>  		} else {
>  			pl->ops->mac_link_up(ndev, pl->link_an_mode,
> +					     pl->phy_state.interface,
>  					     pl->phydev);
>  
>  			netif_carrier_on(ndev);
> diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> index bd137c273d38..f29a40947de9 100644
> --- a/include/linux/phylink.h
> +++ b/include/linux/phylink.h
> @@ -73,8 +73,10 @@ struct phylink_mac_ops {
>  	void (*mac_config)(struct net_device *ndev, unsigned int mode,
>  			   const struct phylink_link_state *state);
>  	void (*mac_an_restart)(struct net_device *ndev);
> -	void (*mac_link_down)(struct net_device *ndev, unsigned int mode);
> +	void (*mac_link_down)(struct net_device *ndev, unsigned int mode,
> +			      phy_interface_t interface);
>  	void (*mac_link_up)(struct net_device *ndev, unsigned int mode,
> +			    phy_interface_t interface,
>  			    struct phy_device *phy);
>  };
>  
> @@ -161,17 +163,20 @@ void mac_an_restart(struct net_device *ndev);
>   * mac_link_down() - take the link down
>   * @ndev: a pointer to a &struct net_device for the MAC.
>   * @mode: link autonegotiation mode
> + * @interface: link &typedef phy_interface_t mode
>   *
>   * If @mode is not an in-band negotiation mode (as defined by
>   * phylink_autoneg_inband()), force the link down and disable any
>   * Energy Efficient Ethernet MAC configuration.
>   */
> -void mac_link_down(struct net_device *ndev, unsigned int mode);
> +void mac_link_down(struct net_device *ndev, unsigned int mode,
> +		   phy_interface_t interface);
>  
>  /**
>   * mac_link_up() - allow the link to come up
>   * @ndev: a pointer to a &struct net_device for the MAC.
>   * @mode: link autonegotiation mode
> + * @interface: link &typedef phy_interface_t mode
>   * @phy: any attached phy
>   *
>   * If @mode is not an in-band negotiation mode (as defined by
> @@ -180,6 +185,7 @@ void mac_link_down(struct net_device *ndev, unsigned int mode);
>   * phy_init_eee() and perform appropriate MAC configuration for EEE.
>   */
>  void mac_link_up(struct net_device *ndev, unsigned int mode,
> +		 phy_interface_t interface,
>  		 struct phy_device *phy);
>  #endif
>  
> -- 
> 2.14.1
> 

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ