Thread (29 messages) 29 messages, 4 authors, 2022-03-10

Re: [PATCH v8 net-next 06/10] net: dsa: microchip: add support for phylink management

From: Vladimir Oltean <olteanv@gmail.com>
Date: 2022-02-07 17:34:04
Also in: linux-devicetree, lkml

On Mon, Feb 07, 2022 at 10:52:00PM +0530, Prasanna Vengateshan wrote:
+static void lan937x_apply_rgmii_delay(struct ksz_device *dev, int port,
+				      phy_interface_t interface, u8 val)
+{
+	struct ksz_port *p = &dev->ports[port];
+
+	/* Clear Ingress & Egress internal delay enabled bits */
+	val &= ~(PORT_RGMII_ID_EG_ENABLE | PORT_RGMII_ID_IG_ENABLE);
+
+	if (interface == PHY_INTERFACE_MODE_RGMII_TXID ||
+	    interface == PHY_INTERFACE_MODE_RGMII_ID) {
+		/* if the delay is 0, let us not enable DLL */
+		if (p->rgmii_tx_val) {
+			lan937x_update_rgmii_tx_rx_delay(dev, port, true);
+			dev_info(dev->dev, "Applied rgmii tx delay for the port %d\n",
+				 port);
+			val |= PORT_RGMII_ID_EG_ENABLE;
+		}
+	}
+
+	if (interface == PHY_INTERFACE_MODE_RGMII_RXID ||
+	    interface == PHY_INTERFACE_MODE_RGMII_ID) {
+		/* if the delay is 0, let us not enable DLL */
+		if (p->rgmii_rx_val) {
+			lan937x_update_rgmii_tx_rx_delay(dev, port, false);
+			dev_info(dev->dev, "Applied rgmii rx delay for the port %d\n",
+				 port);
+			val |= PORT_RGMII_ID_IG_ENABLE;
+		}
+	}
+
+	/* Enable RGMII internal delays */
+	lan937x_pwrite8(dev, port, REG_PORT_XMII_CTRL_1, val);
+}
+
+void lan937x_mac_config(struct ksz_device *dev, int port,
+			phy_interface_t interface)
+{
+	u8 data8;
+
+	lan937x_pread8(dev, port, REG_PORT_XMII_CTRL_1, &data8);
+
+	/* clear MII selection & set it based on interface later */
+	data8 &= ~PORT_MII_SEL_M;
+
+	/* configure MAC based on interface */
+	switch (interface) {
+	case PHY_INTERFACE_MODE_MII:
+		lan937x_config_gbit(dev, false, &data8);
+		data8 |= PORT_MII_SEL;
+		break;
+	case PHY_INTERFACE_MODE_RMII:
+		lan937x_config_gbit(dev, false, &data8);
+		data8 |= PORT_RMII_SEL;
+		break;
+	case PHY_INTERFACE_MODE_RGMII:
+		lan937x_config_gbit(dev, true, &data8);
+		data8 |= PORT_RGMII_SEL;
+		break;
+	case PHY_INTERFACE_MODE_RGMII_ID:
+	case PHY_INTERFACE_MODE_RGMII_TXID:
+	case PHY_INTERFACE_MODE_RGMII_RXID:
+		lan937x_config_gbit(dev, true, &data8);
+		data8 |= PORT_RGMII_SEL;
+
+		/* Apply rgmii internal delay for the mac */
+		lan937x_apply_rgmii_delay(dev, port, interface, data8);
I think the agreement from previous discussions was to apply RGMII delay
_exclusively_ based on the 'rx-internal-delay-ps' and 'tx-internal-delay-ps'
properties, at least for new drivers with no legacy. You are omitting to
apply delays in phy-mode = "rgmii", which contradicts that agreement.
I think you should treat all 4 RGMII cases the same, and remove the
interface checks from lan937x_apply_rgmii_delay.
+
+		/* rgmii delay configuration is already applied above,
+		 * hence return from here as no changes required
+		 */
+		return;
+	default:
+		dev_err(dev->dev, "Unsupported interface '%s' for port %d\n",
+			phy_modes(interface), port);
+		return;
+	}
+
+	/* Write the updated value */
+	lan937x_pwrite8(dev, port, REG_PORT_XMII_CTRL_1, data8);
+}
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help