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
| ||
|
Date: Fri, 14 Jun 2019 03:42:23 +0200 From: Andrew Lunn <andrew@...n.ch> To: Ioana Ciornei <ioana.ciornei@....com> Cc: linux@...linux.org.uk, hkallweit1@...il.com, f.fainelli@...il.com, davem@...emloft.net, netdev@...r.kernel.org, alexandru.marginean@....com, ruxandra.radulescu@....com Subject: Re: [PATCH RFC 4/6] dpaa2-mac: add initial driver > +static phy_interface_t phy_mode(enum dpmac_eth_if eth_if) > +{ > + switch (eth_if) { > + case DPMAC_ETH_IF_RGMII: > + return PHY_INTERFACE_MODE_RGMII; So the MAC cannot insert RGMII delays? I didn't see anything in the PHY object about configuring the delays. Does the PCB need to add delays via squiggles in the tracks? > +static void dpaa2_mac_validate(struct phylink_config *config, > + unsigned long *supported, > + struct phylink_link_state *state) > +{ > + struct dpaa2_mac_priv *priv = to_dpaa2_mac_priv(phylink_config); > + struct dpmac_link_state *dpmac_state = &priv->state; > + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, }; > + > + phylink_set(mask, Autoneg); > + phylink_set_port_modes(mask); > + > + switch (state->interface) { > + case PHY_INTERFACE_MODE_10GKR: > + phylink_set(mask, 10baseT_Full); > + phylink_set(mask, 100baseT_Full); > + phylink_set(mask, 1000baseT_Full); > + phylink_set(mask, 10000baseT_Full); > + break; > + case PHY_INTERFACE_MODE_QSGMII: > + case PHY_INTERFACE_MODE_RGMII: > + case PHY_INTERFACE_MODE_RGMII_ID: > + case PHY_INTERFACE_MODE_RGMII_RXID: > + case PHY_INTERFACE_MODE_RGMII_TXID: > + phylink_set(mask, 10baseT_Full); > + phylink_set(mask, 100baseT_Full); > + phylink_set(mask, 1000baseT_Full); > + break; > + case PHY_INTERFACE_MODE_USXGMII: > + phylink_set(mask, 10baseT_Full); > + phylink_set(mask, 100baseT_Full); > + phylink_set(mask, 1000baseT_Full); > + phylink_set(mask, 10000baseT_Full); > + break; > + default: > + goto empty_set; > + } I think this is wrong. This is about validating what the MAC can do. The state->interface should not matter. The PHY will indicate what interface mode should be used when auto-neg has completed. The MAC is then expected to change its interface to fit. But lets see what Russell says. > +static void dpaa2_mac_config(struct phylink_config *config, unsigned int mode, > + const struct phylink_link_state *state) > +{ > + struct dpaa2_mac_priv *priv = to_dpaa2_mac_priv(phylink_config); > + struct dpmac_link_state *dpmac_state = &priv->state; > + struct device *dev = &priv->mc_dev->dev; > + int err; > + > + if (state->speed == SPEED_UNKNOWN && state->duplex == DUPLEX_UNKNOWN) > + return; > + > + dpmac_state->up = !!state->link; > + if (dpmac_state->up) { > + dpmac_state->rate = state->speed; > + > + 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; As Russell pointed out, this auto-neg is only valid in a limited context. The MAC generally does not perform auto-neg. The MAC is only involved in auto-neg when inband signalling is used between the MAC and PHY in 802.3z. As the name says, dpaa2_mac_config is about the MAC. Andrew
Powered by blists - more mailing lists