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] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 25 Feb 2020 16:36:32 +0000
From:   Ioana Ciornei <ioana.ciornei@....com>
To:     Russell King <rmk+kernel@...linux.org.uk>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>
CC:     Alexandre Torgue <alexandre.torgue@...com>,
        "David S. Miller" <davem@...emloft.net>,
        Felix Fietkau <nbd@....name>,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Hauke Mehrtens <hauke@...ke-m.de>,
        Jakub Kicinski <kuba@...nel.org>,
        John Crispin <john@...ozen.org>,
        Jonathan Corbet <corbet@....net>,
        Jose Abreu <joabreu@...opsys.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        "linux-stm32@...md-mailman.stormreply.com" 
        <linux-stm32@...md-mailman.stormreply.com>,
        Mark Lee <Mark-MC.Lee@...iatek.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Michal Simek <michal.simek@...inx.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Radhey Shyam Pandey <radhey.shyam.pandey@...inx.com>,
        Sean Wang <sean.wang@...iatek.com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Vladimir Oltean <olteanv@...il.com>
Subject: RE: [PATCH net-next 5/8] net: dpaa2-mac: use resolved link config in
 mac_link_up()

> Subject: [PATCH net-next 5/8] net: dpaa2-mac: use resolved link config in
> mac_link_up()
> 
> Convert the DPAA2 ethernet driver to use the finalised link parameters in
> mac_link_up() rather than the parameters in mac_config(), which are more
> suited to the needs of the DPAA2 MC firmware than those available via
> mac_config().
> 
> Signed-off-by: Russell King <rmk+kernel@...linux.org.uk>

Tested-by: Ioana Ciornei <ioana.ciornei@....com>

> ---
>  .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  | 54 +++++++++++--------
> .../net/ethernet/freescale/dpaa2/dpaa2-mac.h  |  1 +
>  2 files changed, 33 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> index 3a75c5b58f95..3ee236c5fc37 100644
> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> @@ -123,35 +123,16 @@ static void dpaa2_mac_config(struct phylink_config
> *config, unsigned int mode,
>  	struct dpmac_link_state *dpmac_state = &mac->state;
>  	int err;
> 
> -	if (state->speed != SPEED_UNKNOWN)
> -		dpmac_state->rate = state->speed;
> -
> -	if (state->duplex != DUPLEX_UNKNOWN) {
> -		if (!state->duplex)
> -			dpmac_state->options |=
> DPMAC_LINK_OPT_HALF_DUPLEX;
> -		else
> -			dpmac_state->options &=
> ~DPMAC_LINK_OPT_HALF_DUPLEX;
> -	}
> -
>  	if (state->an_enabled)
>  		dpmac_state->options |= DPMAC_LINK_OPT_AUTONEG;
>  	else
>  		dpmac_state->options &= ~DPMAC_LINK_OPT_AUTONEG;
> 
> -	if (state->pause & MLO_PAUSE_RX)
> -		dpmac_state->options |= DPMAC_LINK_OPT_PAUSE;
> -	else
> -		dpmac_state->options &= ~DPMAC_LINK_OPT_PAUSE;
> -
> -	if (!!(state->pause & MLO_PAUSE_RX) ^ !!(state->pause &
> MLO_PAUSE_TX))
> -		dpmac_state->options |= DPMAC_LINK_OPT_ASYM_PAUSE;
> -	else
> -		dpmac_state->options &= ~DPMAC_LINK_OPT_ASYM_PAUSE;
> -
>  	err = dpmac_set_link_state(mac->mc_io, 0,
>  				   mac->mc_dev->mc_handle, dpmac_state);
>  	if (err)
> -		netdev_err(mac->net_dev, "dpmac_set_link_state() = %d\n",
> err);
> +		netdev_err(mac->net_dev, "%s: dpmac_set_link_state() =
> %d\n",
> +			   __func__, err);
>  }
> 
>  static void dpaa2_mac_link_up(struct phylink_config *config, @@ -165,10
> +146,37 @@ static void dpaa2_mac_link_up(struct phylink_config *config,
>  	int err;
> 
>  	dpmac_state->up = 1;
> +
> +	if (mac->if_link_type == DPMAC_LINK_TYPE_PHY) {
> +		/* If the DPMAC is configured for PHY mode, we need
> +		 * to pass the link parameters to the MC firmware.
> +		 */
> +		dpmac_state->rate = speed;
> +
> +		if (duplex == DUPLEX_HALF)
> +			dpmac_state->options |=
> DPMAC_LINK_OPT_HALF_DUPLEX;
> +		else if (duplex == DUPLEX_FULL)
> +			dpmac_state->options &=
> ~DPMAC_LINK_OPT_HALF_DUPLEX;
> +
> +		/* This is lossy; the firmware really should take the pause
> +		 * enablement status rather than pause/asym pause status.
> +		 */

In what sense it's lossy? I cannot see how information can be lost by translating the rx/tx pause state to pause/asym.
If it's just about the unnecessary double translation, then I agree.. this could have been done in an easier manner.


> +		if (rx_pause)
> +			dpmac_state->options |= DPMAC_LINK_OPT_PAUSE;
> +		else
> +			dpmac_state->options &= ~DPMAC_LINK_OPT_PAUSE;
> +
> +		if (rx_pause ^ tx_pause)
> +			dpmac_state->options |=
> DPMAC_LINK_OPT_ASYM_PAUSE;
> +		else
> +			dpmac_state->options &=
> ~DPMAC_LINK_OPT_ASYM_PAUSE;
> +	}
> +
>  	err = dpmac_set_link_state(mac->mc_io, 0,
>  				   mac->mc_dev->mc_handle, dpmac_state);
>  	if (err)
> -		netdev_err(mac->net_dev, "dpmac_set_link_state() = %d\n",
> err);
> +		netdev_err(mac->net_dev, "%s: dpmac_set_link_state() =
> %d\n",
> +			   __func__, err);
>  }
> 
>  static void dpaa2_mac_link_down(struct phylink_config *config, @@ -241,6
> +249,8 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)
>  		goto err_close_dpmac;
>  	}
> 
> +	mac->if_link_type = attr.link_type;
> +
>  	dpmac_node = dpaa2_mac_get_node(attr.id);
>  	if (!dpmac_node) {
>  		netdev_err(net_dev, "No dpmac@%d node found.\n", attr.id);
> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> index 4da8079b9155..2130d9c7d40e 100644
> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> @@ -20,6 +20,7 @@ struct dpaa2_mac {
>  	struct phylink_config phylink_config;
>  	struct phylink *phylink;
>  	phy_interface_t if_mode;
> +	enum dpmac_link_type if_link_type;
>  };
> 
>  bool dpaa2_mac_is_type_fixed(struct fsl_mc_device *dpmac_dev,
> --
> 2.20.1

Powered by blists - more mailing lists