Re: [PATCH net-next v5 3/3] tsnep: Add TSN endpoint Ethernet MAC driver

From
Andrew Lunn <andrew@lunn.ch>
Date
2021-11-15 23:06:46
Reply to
Gerhard Engleder, 2021-11-15
quoted hunk ↗ jump to hunk
+static int tsnep_ethtool_get_regs_len(struct net_device *netdev)
+{
+	struct tsnep_adapter *adapter = netdev_priv(netdev);
+	int len;
+	int num_queues;
+
+	len = TSNEP_MAC_SIZE;
+	num_queues = max(adapter->num_tx_queues, adapter->num_rx_queues);
+	len += TSNEP_QUEUE_SIZE * (num_queues - 1);
Why the num_queues - 1 ? A comment here might be good explaining it.
quoted hunk ↗ jump to hunk
+
+	return len;
+}
+
+static void tsnep_ethtool_get_regs(struct net_device *netdev,
+				   struct ethtool_regs *regs,
+				   void *p)
+{
+	struct tsnep_adapter *adapter = netdev_priv(netdev);
+
+	regs->version = 1;
+
+	memcpy_fromio(p, adapter->addr, regs->len);
+}
So the registers and the queues are contiguous?
quoted hunk ↗ jump to hunk
+static int tsnep_ethtool_get_ts_info(struct net_device *dev,
+				     struct ethtool_ts_info *info)
+{
+	struct tsnep_adapter *adapter = netdev_priv(dev);
+
+	info->so_timestamping = SOF_TIMESTAMPING_TX_SOFTWARE |
+				SOF_TIMESTAMPING_RX_SOFTWARE |
+				SOF_TIMESTAMPING_SOFTWARE |
+				SOF_TIMESTAMPING_TX_HARDWARE |
+				SOF_TIMESTAMPING_RX_HARDWARE |
+				SOF_TIMESTAMPING_RAW_HARDWARE;
+
+	if (adapter->ptp_clock)
+		info->phc_index = ptp_clock_index(adapter->ptp_clock);
+	else
+		info->phc_index = -1;
+
+	info->tx_types = BIT(HWTSTAMP_TX_OFF) |
+			 BIT(HWTSTAMP_TX_ON);
+	info->rx_filters = BIT(HWTSTAMP_FILTER_NONE) |
+			   BIT(HWTSTAMP_FILTER_ALL);
+
+	return 0;
+}
You should Cc: Richard Cochran [off-list ref] for the PTP
parts.
quoted hunk ↗ jump to hunk
+static int tsnep_mdio_init(struct tsnep_adapter *adapter)
+{
+	struct device_node *np = adapter->pdev->dev.of_node;
+	int retval;
+
+	if (np) {
+		np = of_get_child_by_name(np, "mdio");
+		if (!np)
+			return 0;
+
+		adapter->suppress_preamble =
+			of_property_read_bool(np, "suppress-preamble");
+	}
+
+	adapter->mdiobus = devm_mdiobus_alloc(&adapter->pdev->dev);
+	if (!adapter->mdiobus) {
+		retval = -ENOMEM;
+
+		goto out;
+	}
+
+	adapter->mdiobus->priv = (void *)adapter;
+	adapter->mdiobus->parent = &adapter->pdev->dev;
+	adapter->mdiobus->read = tsnep_mdiobus_read;
+	adapter->mdiobus->write = tsnep_mdiobus_write;
+	adapter->mdiobus->name = TSNEP "-mdiobus";
+	snprintf(adapter->mdiobus->id, MII_BUS_ID_SIZE, "%s",
+		 adapter->pdev->name);
+
+	if (np) {
+		retval = of_mdiobus_register(adapter->mdiobus, np);
+
+		of_node_put(np);
+	} else {
+		/* do not scan broadcast address */
+		adapter->mdiobus->phy_mask = 0x0000001;
+
+		retval = mdiobus_register(adapter->mdiobus);
You can probably simply this. of_mdiobus_register() is happy to take a
NULL pointer for np, and will fall back to mdiobus_register().

quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/engleder/tsnep_test.c b/drivers/net/ethernet/engleder/tsnep_test.c
You have quite a lot of code in this file. Could it either be

1) A loadable module which extends the base driver?
2) A build time configuration option?

What percentage of the overall driver binary does this test code take
up?

Apart from the minor comments above, ethtool, mdio, phy all looks
good.

	Andrew
Thread (12 messages) 12 messages, 4 authors, last reply 2021-11-18
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help