Thread (36 messages) 36 messages, 4 authors, 2024-08-05

Re: [RFC PATCH net-next 03/10] net: hibmcge: Add mdio and hardware configuration supported in this module

From: Andrew Lunn <andrew@lunn.ch>
Date: 2024-08-01 00:42:46
Also in: lkml

+int hbg_hw_adjust_link(struct hbg_priv *priv, u32 speed, u32 duplex)
+{
+	if (speed != HBG_PORT_MODE_SGMII_10M &&
+	    speed != HBG_PORT_MODE_SGMII_100M &&
+	    speed != HBG_PORT_MODE_SGMII_1000M)
+		return -EOPNOTSUPP;
+
+	if (duplex != DUPLEX_FULL && duplex != DUPLEX_HALF)
+		return -EOPNOTSUPP;
+
+	if (speed == HBG_PORT_MODE_SGMII_1000M && duplex == DUPLEX_HALF)
+		return -EOPNOTSUPP;
If you tell phylib you don't support 1G/Half, this will not happen.
+/* sgmii autoneg always enable */
+int hbg_hw_sgmii_autoneg(struct hbg_priv *priv)
+{
+	wait_time = 0;
+	do {
+		msleep(HBG_HW_AUTONEG_TIMEOUT_STEP);
+		wait_time += HBG_HW_AUTONEG_TIMEOUT_STEP;
+
+		an_state.bits = hbg_reg_read(priv, HBG_REG_AN_NEG_STATE_ADDR);
+		if (an_state.an_done)
+			break;
+	} while (wait_time < HBG_HW_AUTONEG_TIMEOUT_MS);
include/linux/iopoll.h
+static const u32 hbg_mode_ability[] = {
+	ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+	ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+	ETHTOOL_LINK_MODE_100baseT_Half_BIT,
+	ETHTOOL_LINK_MODE_10baseT_Full_BIT,
+	ETHTOOL_LINK_MODE_10baseT_Half_BIT,
+	ETHTOOL_LINK_MODE_Autoneg_BIT,
+	ETHTOOL_LINK_MODE_TP_BIT,
+};
+
+static int hbg_mac_init(struct hbg_priv *priv)
+{
+	struct hbg_mac *mac = &priv->mac;
+	u32 i;
+
+	for (i = 0; i < ARRAY_SIZE(hbg_mode_ability); i++)
+		linkmode_set_bit(hbg_mode_ability[i], mac->supported);
Humm, odd. Where is this leading...
+#define HBG_MDIO_FREQUENCE_2_5M		0x1
I assume it supports other frequencies. You might want to implement
the DT property 'clock-frequency'. Many modern PHY will work faster
than 2.5Mhz.
+static int hbg_phy_connect(struct hbg_priv *priv)
+{
+	struct phy_device *phydev = priv->mac.phydev;
+	struct hbg_mac *mac = &priv->mac;
+	int ret;
+
+	linkmode_copy(phydev->supported, mac->supported);
+	linkmode_copy(phydev->advertising, mac->supported);
And here it is. Why? Do you see any other MAC driver doing this?

What you probably want is:

phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT);

which is what other MAC drivers do.
+
+	phy_connect_direct(priv->netdev, mac->phydev, hbg_phy_adjust_link,
+			   PHY_INTERFACE_MODE_SGMII);
+	ret = devm_add_action_or_reset(&priv->pdev->dev,
+				       hbg_phy_disconnect, mac->phydev);
+	if (ret)
+		return ret;
+
+	phy_attached_info(phydev);
+	return 0;
+}
+
+/* include phy link and mac link */
+u32 hbg_get_link_status(struct hbg_priv *priv)
+{
+	struct phy_device *phydev = priv->mac.phydev;
+	int ret;
+
+	if (!phydev)
+		return HBG_LINK_DOWN;
+
+	phy_read_status(phydev);
+	if ((phydev->state != PHY_UP && phydev->state != PHY_RUNNING) ||
+	    !phydev->link)
+		return HBG_LINK_DOWN;
+
+	ret = hbg_hw_sgmii_autoneg(priv);
+	if (ret)
+		return HBG_LINK_DOWN;
+
+	return HBG_LINK_UP;
+}
There should not be any need for this. So why do you have it?

	Andrew
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help