| 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
| ||
|
Message-ID: <DB8PR04MB68282F710FB598B977C36F99E0ED0@DB8PR04MB6828.eurprd04.prod.outlook.com>
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