> +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
