All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/6] Introduce DSA Ethernet switch class and Felix driver
@ 2019-12-03 14:56 Alex Marginean
  2019-12-03 14:56 ` [PATCH v3 1/6] net: introduce DSA class for Ethernet switches Alex Marginean
                   ` (6 more replies)
  0 siblings, 7 replies; 25+ messages in thread
From: Alex Marginean @ 2019-12-03 14:56 UTC (permalink / raw)
  To: u-boot

DSA stands for Distributed Switch Architecture and it is a subsystem
introduced in the Linux kernel to support switches that:
- have an Ethernet link up to the CPU
- use some form of tagging to identify the source/destination port for
  Rx/Tx
- may be cascaded in tree-like structures.

DSA is described in depth here:
https://www.kernel.org/doc/Documentation/networking/dsa/dsa.txt

From the doc:

	Summarized, this is basically how DSA looks like from a network device
	perspective:


		    |---------------------------
		    | CPU network device (eth0)|
		    ----------------------------
		    | <tag added by switch     |
		    |                          |
		    |                          |
		    |        tag added by CPU> |
		|--------------------------------------------|
		| Switch driver				     |
		|--------------------------------------------|
		    ||        ||         ||
		|-------|  |-------|  |-------|
		| sw0p0 |  | sw0p1 |  | sw0p2 |
		|-------|  |-------|  |-------|

This patch set introduces a DSA class in U-Boot to support drivers of such
switches.  DSA drivers have to implement the following ops:
- enable/disable of switch ports,
- insert a tag in frames being transmitted, used by the switch to select
  the egress port,
- parse a tag in frames being received, used for Rx traffic.

DSA class code deals with presentation of switch ports as Ethernet
interfaces, deals with the master Ethernet device for I/O and helps with
parsing of the DT assuming the structure follows the DSA kernel binding.

Support for switch cascading is not included yet.

This patch set also introduces a driver for the Ethernet switch integrated
into NXP LS1028A, called Felix.  The switch has 4 front panel ports, I/O
to/fom it is done though an ENETC Ethernet interface and meta-data is
carried between the switch and the driver though an additional header
pre-pended to the original frame.
Network commands like tftp can be used on these front panel ports.  The
ports are disabled unless used so they do not cause issues on network
topologies that include loops.

Felix as seen on LS1028A RDB:
=> dm tree
 Class     Index  Probed  Driver                Name
-----------------------------------------------------------
......
 dsa           0  [ + ]   felix-switch          |   |-- felix-switch
 eth           4  [ + ]   dsa-port              |   |   |-- swp0
 eth           5  [ + ]   dsa-port              |   |   |-- swp1
 eth           6  [ + ]   dsa-port              |   |   |-- swp2
 eth           7  [ + ]   dsa-port              |   |   `-- swp3

=> mdio list
......
10 - Vitesse VSC8514 <--> swp0
11 - Vitesse VSC8514 <--> swp1
12 - Vitesse VSC8514 <--> swp2
13 - Vitesse VSC8514 <--> swp3

=> tftp 80000000 test
Using swp2 device
TFTP from server 192.168.100.1; our IP address is 192.168.100.100
Filename 'test'.
Load address: 0x80000000
Loading: #################################################################
         #################################################################
         ########################################################
         6.8 MiB/s
done
Bytes transferred = 949880 (e7e78 hex)

Changes in v3:
 - fix Felix platdata size
 - move include/dsa.h to include/net/dsa.h
 - updated TODO in dsa.h
 - other minor fixes

Changes in v2:
 - Don't use NULL PHY in Felix driver
 - guard dsa.h with #ifndef __DSA__H__, somehow I missed that in v1
 - added a TODO for setting master Eth in promiscuous mode
 - Minor fixes in patch descriptions, API comments
 - Added address/size-cells to LS1028A DT ports node

This patch set replaces v2:
https://patchwork.ozlabs.org/project/uboot/list/?series=144912
and depends on:
https://patchwork.ozlabs.org/project/uboot/list/?series=144907
https://patchwork.ozlabs.org/project/uboot/list/?series=142879

Alex Marginean (6):
  net: introduce DSA class for Ethernet switches
  drivers: net: add a DSA sandbox driver
  test: dm: add a simple unit test for DSA class
  drivers: net: add Felix DSA switch driver
  arm: dts: ls1028a: adds Ethernet switch node and its dependencies
  configs: ls1028a: enable the Ethernet switch driver in defconfig

 arch/Kconfig                                 |   1 +
 arch/arm/dts/fsl-ls1028a-rdb.dts             |  36 ++
 arch/arm/dts/fsl-ls1028a.dtsi                |  44 +-
 arch/sandbox/dts/test.dts                    |  49 ++
 configs/ls1028aqds_tfa_SECURE_BOOT_defconfig |   3 +-
 configs/ls1028aqds_tfa_defconfig             |   3 +-
 configs/ls1028ardb_tfa_SECURE_BOOT_defconfig |   3 +-
 configs/ls1028ardb_tfa_defconfig             |   3 +-
 drivers/net/Kconfig                          |  21 +
 drivers/net/Makefile                         |   1 +
 drivers/net/dsa_sandbox.c                    | 272 +++++++++++
 drivers/net/fsl_enetc.h                      |   5 +
 drivers/net/mscc_eswitch/Kconfig             |   8 +
 drivers/net/mscc_eswitch/Makefile            |   1 +
 drivers/net/mscc_eswitch/felix_switch.c      | 454 +++++++++++++++++++
 include/configs/sandbox.h                    |   4 +
 include/dm/uclass-id.h                       |   1 +
 include/dsa.h                                | 140 ++++++
 net/Makefile                                 |   1 +
 net/dsa-uclass.c                             | 369 +++++++++++++++
 test/dm/Makefile                             |   1 +
 test/dm/dsa.c                                |  58 +++
 test/dm/test-fdt.c                           |   2 +-
 23 files changed, 1474 insertions(+), 6 deletions(-)
 create mode 100644 drivers/net/dsa_sandbox.c
 create mode 100644 drivers/net/mscc_eswitch/felix_switch.c
 create mode 100644 include/dsa.h
 create mode 100644 net/dsa-uclass.c
 create mode 100644 test/dm/dsa.c

-- 
2.17.1

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 1/6] net: introduce DSA class for Ethernet switches
  2019-12-03 14:56 [PATCH v3 0/6] Introduce DSA Ethernet switch class and Felix driver Alex Marginean
@ 2019-12-03 14:56 ` Alex Marginean
  2019-12-15 12:44   ` Vladimir Oltean
                     ` (2 more replies)
  2019-12-03 14:56 ` [PATCH v3 2/6] drivers: net: add a DSA sandbox driver Alex Marginean
                   ` (5 subsequent siblings)
  6 siblings, 3 replies; 25+ messages in thread
From: Alex Marginean @ 2019-12-03 14:56 UTC (permalink / raw)
  To: u-boot

DSA stands for Distributed Switch Architecture and it covers switches that
are connected to the CPU through an Ethernet link and generally use frame
tags to pass information about the source/destination ports to/from CPU.
Front panel ports are presented as regular ethernet devices in U-Boot and
they are expected to support the typical networking commands.
DSA switches may be cascaded, DSA class code does not currently support
this.

Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
---
 drivers/net/Kconfig    |  13 ++
 include/dm/uclass-id.h |   1 +
 include/net/dsa.h      | 141 ++++++++++++++++
 net/Makefile           |   1 +
 net/dsa-uclass.c       | 369 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 525 insertions(+)
 create mode 100644 include/net/dsa.h
 create mode 100644 net/dsa-uclass.c

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 4182897d89..a4157cb122 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -37,6 +37,19 @@ config DM_MDIO_MUX
 	  This is currently implemented in net/mdio-mux-uclass.c
 	  Look in include/miiphy.h for details.
 
+config DM_DSA
+	bool "Enable Driver Model for DSA switches"
+	depends on DM_ETH && DM_MDIO
+	help
+	  Enable Driver Model for DSA switches
+
+	  Adds UCLASS_DSA class supporting switches that follow the Distributed
+	  Switch Architecture (DSA).  These switches rely on the presence of a
+	  management switch port connected to an Ethernet controller capable of
+	  receiving frames from the switch.  This host Ethernet controller is
+	  called "master" and "cpu" in DSA terminology.
+	  This is currently implemented in net/dsa-uclass.c
+
 config MDIO_SANDBOX
 	depends on DM_MDIO && SANDBOX
 	default y
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 0c563d898b..8f37a91488 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -42,6 +42,7 @@ enum uclass_id {
 	UCLASS_DISPLAY,		/* Display (e.g. DisplayPort, HDMI) */
 	UCLASS_DSI_HOST,	/* Display Serial Interface host */
 	UCLASS_DMA,		/* Direct Memory Access */
+	UCLASS_DSA,		/* Distributed (Ethernet) Switch Architecture */
 	UCLASS_EFI,		/* EFI managed devices */
 	UCLASS_ETH,		/* Ethernet device */
 	UCLASS_FIRMWARE,	/* Firmware */
diff --git a/include/net/dsa.h b/include/net/dsa.h
new file mode 100644
index 0000000000..2387419b9d
--- /dev/null
+++ b/include/net/dsa.h
@@ -0,0 +1,141 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2019 NXP
+ */
+
+#ifndef __DSA_H__
+#define __DSA_H__
+
+#include <common.h>
+#include <dm.h>
+#include <phy.h>
+
+/**
+ * DSA stands for Distributed Switch Architecture and it is infrastructure
+ * intended to support drivers for Switches that rely on an intermediary
+ * Ethernet device for I/O.  These switches may support cascading allowing
+ * them to be arranged as a tree.
+ * DSA is documented in detail in the Linux kernel documentation under
+ * Documentation/networking/dsa/dsa.txt
+ * The network layout of such a switch is shown below:
+ *
+ *	    |---------------------------
+ *	    | CPU network device (eth0)|
+ *	    ----------------------------
+ *	    | <tag added by switch     |
+ *	    |                          |
+ *	    |                          |
+ *	    |        tag added by CPU> |
+ *	|--------------------------------------------|
+ *	| Switch driver				     |
+ *	|--------------------------------------------|
+ *	    ||        ||         ||
+ *	|-------|  |-------|  |-------|
+ *	| sw0p0 |  | sw0p1 |  | sw0p2 |
+ *	|-------|  |-------|  |-------|
+ *
+ * In U-Boot the intent is to allow access to front panel ports (shown at the
+ * bottom of the picture) though the master Ethernet port (eth0 in the picture).
+ * Front panel ports are presented as regular Ethernet devices in U-Boot and
+ * they are expected to support the typical networking commands.
+ * In general DSA switches require the use of tags, extra headers added both by
+ * software on Tx and by the switch on Rx.  These tags carry at a minimum port
+ * information and switch information for cascaded set-ups.
+ * In U-Boot these tags are inserted and parsed by the DSA switch driver, the
+ * class code helps with headroom/tailroom for the extra headers.
+ *
+ * TODO:
+ * - handle switch cascading, for now U-Boot only supports stand-alone switches.
+ * - propagate the master Eth MAC address to switch ports, this is used in
+ * Linux to avoid using additional MAC addresses on master Eth.
+ * - Add support to probe DSA switches connected to a MDIO bus, this is needed
+ * to convert switch drivers that are now under drivers/net/phy.
+ */
+
+#define DSA_PORT_NAME_LENGTH	16
+
+/* Maximum number of ports each DSA device can have */
+#define DSA_MAX_PORTS		12
+/* Used to size internal buffers, no support for jumbo yet */
+#define DSA_MAX_FRAME_SIZE	2048
+
+/**
+ * struct dsa_ops - DSA operations
+ *
+ * @port_enable:  Initialize a switch port for I/O
+ * @port_disable: Disable a port
+ * @xmit:         Insert the DSA tag for transmission
+ *                DSA drivers receive a copy of the packet with headroom and
+ *                tailroom reserved and set to 0.
+ *                Packet points to headroom and length is updated to include
+ *                both headroom and tailroom
+ * @rcv:          Process the DSA tag on reception
+ *                Packet and length describe the frame as received from master
+ *                including any additional headers
+ */
+struct dsa_ops {
+	int (*port_enable)(struct udevice *dev, int port,
+			   struct phy_device *phy);
+	void (*port_disable)(struct udevice *dev, int port,
+			     struct phy_device *phy);
+	int (*xmit)(struct udevice *dev, int port, void *packet, int length);
+	int (*rcv)(struct udevice *dev, int *port, void *packet, int length);
+};
+
+#define dsa_get_ops(dev) ((struct dsa_ops *)(dev)->driver->ops)
+
+/**
+ * struct dsa_port_platdata - DSA port platform data
+ *
+ * @dev :  Port u-device
+ *         Uclass code sets this field for all ports
+ * @phy:   PHY device associated with this port
+ *         Uclass code sets this field for all ports except CPU port, based on
+ *         DT information.  It may be NULL.
+ * @node:  Port DT node, if any.  Uclass code sets this field.
+ * @index: Port index in the DSA switch, set by class code.
+ * @name:  Name of the port Eth device.  If a label property is present in the
+ *         port DT node, it is used as name.  Drivers can use custom names by
+ *         populating this field, otherwise class code generates a default.
+ */
+struct dsa_port_platdata {
+	struct udevice *dev;
+	struct phy_device *phy;
+	ofnode node;
+	int index;
+	char name[DSA_PORT_NAME_LENGTH];
+};
+
+/**
+ * struct dsa_perdev_platdata - Per-device platform data for DSA DM
+ *
+ * @num_ports:   Number of ports the device has, must be <= DSA_MAX_PORTS
+ *               All DSA drivers must set this at _bind
+ * @headroom:    Size, in bytes, of headroom needed for the DSA tag
+ *               All DSA drivers must set this at _bind or _probe
+ * @tailroom:    Size, in bytes, of tailroom needed for the DSA tag
+ *               DSA class code allocates headroom and tailroom on Tx before
+ *               calling DSA driver xmit function
+ *               All DSA drivers must set this@_bind or _probe
+ * @master_node: DT node of the master Ethernet.  DT is optional so this may be
+ *               null.
+ * @master_dev:  Ethernet device to be used as master.  Uclass code sets this
+ *               based on DT information if present, otherwise drivers must set
+ *               this field in _probe.
+ * @cpu_port:    Index of switch port linked to master Ethernet.
+ *               Uclass code sets this based on DT information if present,
+ *               otherwise drivers must set this field in _bind.
+ * @port:        per-port data
+ */
+struct dsa_perdev_platdata {
+	int num_ports;
+	int headroom;
+	int tailroom;
+
+	ofnode master_node;
+	struct udevice *master_dev;
+	int cpu_port;
+	struct dsa_port_platdata port[DSA_MAX_PORTS];
+};
+
+#endif /* __DSA_H__ */
diff --git a/net/Makefile b/net/Makefile
index 2a700c8401..fac8c8beb9 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_CMD_SNTP) += sntp.o
 obj-$(CONFIG_CMD_TFTPBOOT) += tftp.o
 obj-$(CONFIG_UDP_FUNCTION_FASTBOOT)  += fastboot.o
 obj-$(CONFIG_CMD_WOL)  += wol.o
+obj-$(CONFIG_DM_DSA)   += dsa-uclass.o
 
 # Disable this warning as it is triggered by:
 # sprintf(buf, index ? "foo%d" : "foo", index)
diff --git a/net/dsa-uclass.c b/net/dsa-uclass.c
new file mode 100644
index 0000000000..3790a72841
--- /dev/null
+++ b/net/dsa-uclass.c
@@ -0,0 +1,369 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2019, NXP
+ */
+
+#include <net/dsa.h>
+#include <dm/lists.h>
+#include <dm/device-internal.h>
+#include <dm/uclass-internal.h>
+#include <miiphy.h>
+
+#define DSA_PORT_CHILD_DRV_NAME "dsa-port"
+
+/* helper that returns the DSA master Ethernet device. */
+static struct udevice *dsa_port_get_master(struct udevice *pdev, bool probe)
+{
+	struct udevice *dev = dev_get_parent(pdev);
+	struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
+
+	if (probe)
+		device_probe(platdata->master_dev);
+
+	return platdata->master_dev;
+}
+
+/*
+ * Start the desired port, the CPU port and the master Eth interface.
+ * TODO: if cascaded we may need to _start ports in other switches too
+ */
+static int dsa_port_start(struct udevice *pdev)
+{
+	struct udevice *dev = dev_get_parent(pdev);
+	struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
+	struct udevice *master = dsa_port_get_master(pdev, true);
+	struct dsa_port_platdata *ppriv = dev_get_priv(pdev);
+	struct dsa_ops *ops = dsa_get_ops(dev);
+	int err;
+
+	if (!ppriv || !platdata)
+		return -EINVAL;
+
+	if (!master) {
+		dev_err(pdev, "DSA master Ethernet device not found!\n");
+		return -EINVAL;
+	}
+
+	if (ops->port_enable) {
+		err = ops->port_enable(dev, ppriv->index, ppriv->phy);
+		if (err)
+			return err;
+		err = ops->port_enable(dev, platdata->cpu_port,
+				       platdata->port[platdata->cpu_port].phy);
+		if (err)
+			return err;
+	}
+
+	return eth_get_ops(master)->start(master);
+}
+
+/* Stop the desired port, the CPU port and the master Eth interface */
+static void dsa_port_stop(struct udevice *pdev)
+{
+	struct udevice *dev = dev_get_parent(pdev);
+	struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
+	struct udevice *master = dsa_port_get_master(pdev, false);
+	struct dsa_port_platdata *ppriv = dev_get_priv(pdev);
+	struct dsa_ops *ops = dsa_get_ops(dev);
+
+	if (!ppriv || !platdata)
+		return;
+
+	if (ops->port_disable) {
+		ops->port_disable(dev, ppriv->index, ppriv->phy);
+		ops->port_disable(dev, platdata->cpu_port,
+				  platdata->port[platdata->cpu_port].phy);
+	}
+
+	/*
+	 * stop master only if it's active, don't probe it otherwise.
+	 * Under normal usage it would be active because we're using it, but
+	 * during tear-down it may have been removed ahead of us.
+	 */
+	if (master && device_active(master))
+		eth_get_ops(master)->stop(master);
+}
+
+/*
+ * Insert a DSA tag and call master Ethernet send on the resulting packet
+ * We copy the frame to a stack buffer where we have reserved headroom and
+ * tailroom space.  Headroom and tailroom are set to 0.
+ */
+static int dsa_port_send(struct udevice *pdev, void *packet, int length)
+{
+	struct udevice *dev = dev_get_parent(pdev);
+	struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
+	struct udevice *master = dsa_port_get_master(pdev, true);
+	struct dsa_port_platdata *ppriv = dev_get_priv(pdev);
+	struct dsa_ops *ops = dsa_get_ops(dev);
+	uchar dsa_packet[DSA_MAX_FRAME_SIZE];
+	int head = platdata->headroom, tail = platdata->tailroom;
+	int err;
+
+	if (!master)
+		return -EINVAL;
+
+	if (length + head + tail > DSA_MAX_FRAME_SIZE)
+		return -EINVAL;
+
+	memset(dsa_packet, 0, head);
+	memset(dsa_packet + head + length, 0, tail);
+	memcpy(dsa_packet + head, packet, length);
+	length += head + tail;
+
+	err = ops->xmit(dev, ppriv->index, dsa_packet, length);
+	if (err)
+		return err;
+
+	return eth_get_ops(master)->send(master, dsa_packet, length);
+}
+
+/* Receive a frame from master Ethernet, process it and pass it on */
+static int dsa_port_recv(struct udevice *pdev, int flags, uchar **packetp)
+{
+	struct udevice *dev = dev_get_parent(pdev);
+	struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
+	struct udevice *master = dsa_port_get_master(pdev, true);
+	struct dsa_port_platdata *ppriv = dev_get_priv(pdev);
+	struct dsa_ops *ops = dsa_get_ops(dev);
+	int head = platdata->headroom, tail = platdata->tailroom;
+	int length, port_index, err;
+
+	if (!master)
+		return -EINVAL;
+
+	length = eth_get_ops(master)->recv(master, flags, packetp);
+	if (length <= 0)
+		return length;
+
+	/*
+	 * if we receive frames from a different port or frames that DSA driver
+	 * doesn't like we discard them here.
+	 * In case of discard we return with no frame and expect to be called
+	 * again instead of looping here, so upper layer can deal with timeouts
+	 * and ctrl-c
+	 */
+	err = ops->rcv(dev, &port_index, *packetp, length);
+	if (err || port_index != ppriv->index || (length <= head + tail)) {
+		if (eth_get_ops(master)->free_pkt)
+			eth_get_ops(master)->free_pkt(master, *packetp, length);
+		return -EAGAIN;
+	}
+
+	/*
+	 * We move the pointer over headroom here to avoid a copy.  If free_pkt
+	 * gets called we move the pointer back before calling master free_pkt.
+	 */
+	*packetp += head;
+
+	return length - head - tail;
+}
+
+static int dsa_port_free_pkt(struct udevice *pdev, uchar *packet, int length)
+{
+	struct udevice *dev = dev_get_parent(pdev);
+	struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
+	struct udevice *master = dsa_port_get_master(pdev, true);
+
+	if (!master)
+		return -EINVAL;
+
+	if (eth_get_ops(master)->free_pkt) {
+		/* return the original pointer and length to master Eth */
+		packet -= platdata->headroom;
+		length += platdata->headroom - platdata->tailroom;
+
+		return eth_get_ops(master)->free_pkt(master, packet, length);
+	}
+
+	return 0;
+}
+
+static const struct eth_ops dsa_port_ops = {
+	.start		= dsa_port_start,
+	.send		= dsa_port_send,
+	.recv		= dsa_port_recv,
+	.stop		= dsa_port_stop,
+	.free_pkt	= dsa_port_free_pkt,
+};
+
+U_BOOT_DRIVER(dsa_port) = {
+	.name	= DSA_PORT_CHILD_DRV_NAME,
+	.id	= UCLASS_ETH,
+	.ops	= &dsa_port_ops,
+	.platdata_auto_alloc_size = sizeof(struct eth_pdata),
+};
+
+/*
+ * reads the DT properties of the given DSA port.
+ * If the return value is != 0 then the port is skipped
+ */
+static int dsa_port_parse_dt(struct udevice *dev, int port_index,
+			     ofnode ports_node, bool *is_cpu)
+{
+	struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
+	struct dsa_port_platdata *port = &platdata->port[port_index];
+	ofnode temp_node;
+	u32 ethernet;
+
+	/*
+	 * if we don't have a DT we don't do anything here but the port is
+	 * registered normally
+	 */
+	if (!ofnode_valid(ports_node))
+		return 0;
+
+	ofnode_for_each_subnode(temp_node, ports_node) {
+		const char *port_label;
+		u32 reg;
+
+		if (ofnode_read_u32(temp_node, "reg", &reg) ||
+		    reg != port_index)
+			continue;
+
+		/* if the port is explicitly disabled in DT skip it */
+		if (!ofnode_is_available(temp_node))
+			return -ENODEV;
+
+		port->node = temp_node;
+
+		dev_dbg(dev, "port %d node %s\n", port->index,
+			ofnode_get_name(port->node));
+
+		/* Use 'label' if present in DT */
+		port_label = ofnode_read_string(port->node, "label");
+		if (port_label)
+			strncpy(port->name, port_label, DSA_PORT_NAME_LENGTH);
+
+		*is_cpu = !ofnode_read_u32(port->node, "ethernet",
+					   &ethernet);
+
+		if (*is_cpu) {
+			platdata->master_node =
+				ofnode_get_by_phandle(ethernet);
+			platdata->cpu_port = port_index;
+
+			dev_dbg(dev, "master node %s on port %d\n",
+				ofnode_get_name(platdata->master_node),
+				port_index);
+		}
+		break;
+	}
+
+	return 0;
+}
+
+/**
+ * This function mostly deals with pulling information out of the device tree
+ * into the platdata structure.
+ * It goes through the list of switch ports, registers an Eth device for each
+ * front panel port and identifies the cpu port connected to master Eth device.
+ * TODO: support cascaded switches
+ */
+static int dm_dsa_post_bind(struct udevice *dev)
+{
+	struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
+	ofnode ports_node = ofnode_null();
+	int first_err = 0, err = 0, i;
+
+	if (!platdata) {
+		dev_err(dev, "missing plaform data\n");
+		return -EINVAL;
+	}
+
+	if (platdata->num_ports <= 0 || platdata->num_ports > DSA_MAX_PORTS) {
+		dev_err(dev, "unexpected num_ports value (%d)\n",
+			platdata->num_ports);
+		return -EINVAL;
+	}
+
+	platdata->master_node = ofnode_null();
+
+	if (!ofnode_valid(dev->node)) {
+		dev_dbg(dev, "Device doesn't have a valid DT node!\n");
+	} else {
+		ports_node = ofnode_find_subnode(dev->node, "ports");
+		if (!ofnode_valid(ports_node))
+			dev_dbg(dev,
+				"ports node is missing under DSA device!\n");
+	}
+
+	for (i = 0; i < platdata->num_ports; i++) {
+		struct dsa_port_platdata *port = &platdata->port[i];
+		bool skip_port, is_cpu = false;
+
+		port->index = i;
+
+		/*
+		 * If the driver set up port names in _bind use those, otherwise
+		 * use default ones.
+		 * If present, DT label is used as name and overrides anything
+		 * we may have here.
+		 */
+		if (!strlen(port->name))
+			snprintf(port->name, DSA_PORT_NAME_LENGTH, "%s@%d",
+				 dev->name, i);
+
+		skip_port = !!dsa_port_parse_dt(dev, i, ports_node, &is_cpu);
+
+		/*
+		 * if this is the CPU port don't register it as an ETH device,
+		 * we skip it on purpose since I/O to/from it from the CPU
+		 * isn't useful
+		 * TODO: cpu port may have a PHY and we don't handle that yet.
+		 */
+		if (is_cpu || skip_port)
+			continue;
+
+		err = device_bind_driver_to_node(dev, DSA_PORT_CHILD_DRV_NAME,
+						 port->name, port->node,
+						 &port->dev);
+
+		/* try to bind all ports but keep 1st error */
+		if (err && !first_err)
+			first_err = err;
+	}
+
+	if (!ofnode_valid(platdata->master_node))
+		dev_dbg(dev, "DSA master Eth device is missing!\n");
+
+	return first_err;
+}
+
+/**
+ * This function deals with additional devices around the switch as these should
+ * have been bound to drivers by now.
+ * TODO: pick up references to other switch devices here, if we're cascaded.
+ */
+static int dm_dsa_pre_probe(struct udevice *dev)
+{
+	struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
+	int i;
+
+	if (!platdata)
+		return -EINVAL;
+
+	if (ofnode_valid(platdata->master_node))
+		uclass_find_device_by_ofnode(UCLASS_ETH, platdata->master_node,
+					     &platdata->master_dev);
+
+	for (i = 0; i < platdata->num_ports; i++) {
+		struct dsa_port_platdata *port = &platdata->port[i];
+
+		if (port->dev) {
+			port->dev->priv = port;
+			port->phy = dm_eth_phy_connect(port->dev);
+		}
+	}
+
+	return 0;
+}
+
+UCLASS_DRIVER(dsa) = {
+	.id = UCLASS_DSA,
+	.name = "dsa",
+	.post_bind  = dm_dsa_post_bind,
+	.pre_probe = dm_dsa_pre_probe,
+	.per_device_platdata_auto_alloc_size =
+			sizeof(struct dsa_perdev_platdata),
+};
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3 2/6] drivers: net: add a DSA sandbox driver
  2019-12-03 14:56 [PATCH v3 0/6] Introduce DSA Ethernet switch class and Felix driver Alex Marginean
  2019-12-03 14:56 ` [PATCH v3 1/6] net: introduce DSA class for Ethernet switches Alex Marginean
@ 2019-12-03 14:56 ` Alex Marginean
  2019-12-03 14:56 ` [PATCH v3 3/6] test: dm: add a simple unit test for DSA class Alex Marginean
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 25+ messages in thread
From: Alex Marginean @ 2019-12-03 14:56 UTC (permalink / raw)
  To: u-boot

The DSA sandbox driver is used for DSA unit testing.  It implements a
simple 4 port switch that uses a very simple tag to identify the ports.
The DSA driver comes paired with an Ethernet driver that loops packets
back and can selectively filter traffic on DSA switch ports.

Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
---
 drivers/net/Kconfig       |   8 ++
 drivers/net/Makefile      |   1 +
 drivers/net/dsa_sandbox.c | 272 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 281 insertions(+)
 create mode 100644 drivers/net/dsa_sandbox.c

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index a4157cb122..ac420cb474 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -70,6 +70,14 @@ config MDIO_MUX_SANDBOX
 
 	  This driver is used for testing in test/dm/mdio.c
 
+config DSA_SANDBOX
+	depends on DM_DSA && SANDBOX
+	default y
+	bool "Sandbox: Mocked DSA driver"
+	help
+	  This driver implements a dummy switch and a dummy Ethernet device used
+	  to test DSA class code.
+
 menuconfig NETDEVICES
 	bool "Network device support"
 	depends on NET
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 30991834ec..8be49f6335 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -82,3 +82,4 @@ obj-y += mscc_eswitch/
 obj-$(CONFIG_HIGMACV300_ETH) += higmacv300.o
 obj-$(CONFIG_MDIO_SANDBOX) += mdio_sandbox.o
 obj-$(CONFIG_FSL_ENETC) += fsl_enetc.o fsl_enetc_mdio.o
+obj-$(CONFIG_DSA_SANDBOX) += dsa_sandbox.o
diff --git a/drivers/net/dsa_sandbox.c b/drivers/net/dsa_sandbox.c
new file mode 100644
index 0000000000..66882ed7de
--- /dev/null
+++ b/drivers/net/dsa_sandbox.c
@@ -0,0 +1,272 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2019 NXP
+ */
+
+#include <net/dsa.h>
+
+#define DSA_SANDBOX_MAGIC	0x00415344
+#define DSA_SANDBOX_TAG_LEN	sizeof(struct dsa_sandbox_tag)
+/*
+ * This global flag is used to enable DSA just for DSA test so it doesn't affect
+ * the existing eth unit test.
+ */
+int dsa_sandbox_port_mask;
+
+struct dsa_sandbox_priv {
+	int enabled;
+	int port_enabled;
+};
+
+struct dsa_sandbox_tag {
+	u32 magic;
+	u32 port;
+};
+
+static int dsa_sandbox_port_enable(struct udevice *dev, int port,
+				   struct phy_device *phy)
+{
+	struct dsa_sandbox_priv *priv = dev->priv;
+
+	if (!priv->enabled)
+		return -EFAULT;
+
+	priv->port_enabled |= BIT(port);
+
+	return 0;
+}
+
+static void dsa_sandbox_port_disable(struct udevice *dev, int port,
+				     struct phy_device *phy)
+{
+	struct dsa_sandbox_priv *priv = dev->priv;
+
+	if (!priv->enabled)
+		return;
+
+	priv->port_enabled &= ~BIT(port);
+}
+
+static int dsa_sandbox_xmit(struct udevice *dev, int port, void *packet,
+			    int length)
+{
+	struct dsa_sandbox_priv *priv = dev->priv;
+	struct dsa_sandbox_tag *tag = packet;
+
+	if (!priv->enabled)
+		return -EFAULT;
+
+	if (!(priv->port_enabled & BIT(port)))
+		return -EFAULT;
+
+	tag->magic = DSA_SANDBOX_MAGIC;
+	tag->port = port;
+
+	return 0;
+}
+
+static int dsa_sandbox_rcv(struct udevice *dev, int *port, void *packet,
+			   int length)
+{
+	struct dsa_sandbox_priv *priv = dev->priv;
+	struct dsa_sandbox_tag *tag = packet;
+
+	if (!priv->enabled)
+		return -EFAULT;
+
+	if (tag->magic != DSA_SANDBOX_MAGIC)
+		return -EFAULT;
+
+	*port = tag->port;
+	if (!(priv->port_enabled & BIT(*port)))
+		return -EFAULT;
+
+	return 0;
+}
+
+static const struct dsa_ops dsa_sandbox_ops = {
+	.port_enable = dsa_sandbox_port_enable,
+	.port_disable = dsa_sandbox_port_disable,
+	.xmit = dsa_sandbox_xmit,
+	.rcv = dsa_sandbox_rcv,
+};
+
+static int dsa_sandbox_bind(struct udevice *dev)
+{
+	struct dsa_perdev_platdata *pdata = dev->platdata;
+
+	/* must be at least 4 to match sandbox test DT */
+	pdata->num_ports = 4;
+	pdata->headroom = DSA_SANDBOX_TAG_LEN;
+
+	return 0;
+}
+
+static int dsa_sandbox_probe(struct udevice *dev)
+{
+	struct dsa_sandbox_priv *priv = dev_get_priv(dev);
+
+	/*
+	 * return error if DSA is not being tested so we don't break existing
+	 * eth test.
+	 */
+	if (!dsa_sandbox_port_mask)
+		return -EINVAL;
+
+	priv->enabled = 1;
+
+	return 0;
+}
+
+static int dsa_sandbox_remove(struct udevice *dev)
+{
+	struct dsa_sandbox_priv *priv = dev_get_priv(dev);
+
+	priv->enabled = 0;
+
+	return 0;
+}
+
+static const struct udevice_id dsa_sandbox_ids[] = {
+	{ .compatible = "sandbox,dsa" },
+	{ }
+};
+
+U_BOOT_DRIVER(dsa_sandbox) = {
+	.name		= "dsa_sandbox",
+	.id		= UCLASS_DSA,
+	.of_match	= dsa_sandbox_ids,
+	.bind		= dsa_sandbox_bind,
+	.probe		= dsa_sandbox_probe,
+	.remove		= dsa_sandbox_remove,
+	.ops		= &dsa_sandbox_ops,
+	.priv_auto_alloc_size = sizeof(struct dsa_sandbox_priv),
+	.platdata_auto_alloc_size = sizeof(struct dsa_perdev_platdata),
+};
+
+struct dsa_sandbox_eth_priv {
+	int enabled;
+	int started;
+	int packet_length;
+	uchar packet[DSA_MAX_FRAME_SIZE];
+};
+
+static int dsa_eth_sandbox_start(struct udevice *dev)
+{
+	struct dsa_sandbox_eth_priv *priv = dev->priv;
+
+	if (!priv->enabled)
+		return -EFAULT;
+
+	priv->started = 1;
+
+	return 0;
+}
+
+static void dsa_eth_sandbox_stop(struct udevice *dev)
+{
+	struct dsa_sandbox_eth_priv *priv = dev->priv;
+
+	if (!priv->enabled)
+		return;
+
+	priv->started = 0;
+}
+
+static int dsa_eth_sandbox_send(struct udevice *dev, void *packet, int length)
+{
+	struct dsa_sandbox_eth_priv *priv = dev->priv;
+	struct dsa_sandbox_tag *tag = packet;
+
+	if (!priv->enabled || !priv->started)
+		return -EFAULT;
+
+	memcpy(priv->packet, packet, length);
+	priv->packet_length = length;
+
+	/*
+	 * for DSA test frames we only respond if the associated port is enabled
+	 * in the dsa test port mask
+	 */
+
+	if (tag->magic == DSA_SANDBOX_MAGIC) {
+		int port = tag->port;
+
+		if (!(dsa_sandbox_port_mask & BIT(port)))
+			/* drop the frame, port is not enabled */
+			priv->packet_length = 0;
+	}
+
+	return 0;
+}
+
+static int dsa_eth_sandbox_recv(struct udevice *dev, int flags, uchar **packetp)
+{
+	struct dsa_sandbox_eth_priv *priv = dev->priv;
+	int length = priv->packet_length;
+
+	if (!priv->enabled || !priv->started)
+		return -EFAULT;
+
+	if (!length) {
+		/* no frames pending, force a time-out */
+		timer_test_add_offset(100);
+		return -EAGAIN;
+	}
+
+	*packetp = priv->packet;
+	priv->packet_length = 0;
+
+	return length;
+}
+
+static const struct eth_ops dsa_eth_sandbox_ops = {
+	.start	= dsa_eth_sandbox_start,
+	.send	= dsa_eth_sandbox_send,
+	.recv	= dsa_eth_sandbox_recv,
+	.stop	= dsa_eth_sandbox_stop,
+};
+
+static int dsa_eth_sandbox_bind(struct udevice *dev)
+{
+	return 0;
+}
+
+static int dsa_eth_sandbox_probe(struct udevice *dev)
+{
+	struct dsa_sandbox_eth_priv *priv = dev->priv;
+
+	priv->enabled = 1;
+
+	/*
+	 * return error if DSA is not being tested do we don't break existing
+	 * eth test.
+	 */
+	return dsa_sandbox_port_mask ? 0 : -EINVAL;
+}
+
+static int dsa_eth_sandbox_remove(struct udevice *dev)
+{
+	struct dsa_sandbox_eth_priv *priv = dev->priv;
+
+	priv->enabled = 0;
+
+	return 0;
+}
+
+static const struct udevice_id dsa_eth_sandbox_ids[] = {
+	{ .compatible = "sandbox,dsa-eth" },
+	{ }
+};
+
+U_BOOT_DRIVER(dsa_eth_sandbox) = {
+	.name		= "dsa_eth_sandbox",
+	.id		= UCLASS_ETH,
+	.of_match	= dsa_eth_sandbox_ids,
+	.bind		= dsa_eth_sandbox_bind,
+	.probe		= dsa_eth_sandbox_probe,
+	.remove		= dsa_eth_sandbox_remove,
+	.ops		= &dsa_eth_sandbox_ops,
+	.platdata_auto_alloc_size = sizeof(struct eth_pdata),
+	.priv_auto_alloc_size = sizeof(struct dsa_sandbox_eth_priv),
+};
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3 3/6] test: dm: add a simple unit test for DSA class
  2019-12-03 14:56 [PATCH v3 0/6] Introduce DSA Ethernet switch class and Felix driver Alex Marginean
  2019-12-03 14:56 ` [PATCH v3 1/6] net: introduce DSA class for Ethernet switches Alex Marginean
  2019-12-03 14:56 ` [PATCH v3 2/6] drivers: net: add a DSA sandbox driver Alex Marginean
@ 2019-12-03 14:56 ` Alex Marginean
  2019-12-04  4:14   ` Priyanka Jain
  2019-12-03 14:56 ` [PATCH v3 4/6] drivers: net: add Felix DSA switch driver Alex Marginean
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 25+ messages in thread
From: Alex Marginean @ 2019-12-03 14:56 UTC (permalink / raw)
  To: u-boot

The test pings the local IP address though different ports of a sandbox
DSA device.  Port traffic is filtered and the test verifies that ping
works only on enabled ports.
The additional interfaces require MAC addresses, these have been added
to sandbox default environment.

Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
---
 arch/Kconfig              |  1 +
 arch/sandbox/dts/test.dts | 49 +++++++++++++++++++++++++++++++++
 include/configs/sandbox.h |  4 +++
 test/dm/Makefile          |  1 +
 test/dm/dsa.c             | 58 +++++++++++++++++++++++++++++++++++++++
 test/dm/test-fdt.c        |  2 +-
 6 files changed, 114 insertions(+), 1 deletion(-)
 create mode 100644 test/dm/dsa.c

diff --git a/arch/Kconfig b/arch/Kconfig
index 141e48bc43..70907d69a1 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -133,6 +133,7 @@ config SANDBOX
 	imply PHYLIB
 	imply DM_MDIO
 	imply DM_MDIO_MUX
+	imply DM_DSA
 
 config SH
 	bool "SuperH architecture"
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index fdb08f2111..0f565f066a 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -40,6 +40,10 @@
 		usb2 = &usb_2;
 		axi0 = &axi;
 		osd0 = "/osd";
+		eth8 = &swp_0;
+		eth9 = &swp_1;
+		eth10 = &swp_2;
+		eth11 = &dsa_eth0;
 	};
 
 	audio: audio-codec {
@@ -889,6 +893,51 @@
 	mdio: mdio-test {
 		compatible = "sandbox,mdio";
 	};
+
+	dsa_eth0: dsa-test-eth {
+		compatible = "sandbox,dsa-eth";
+	};
+
+	dsa-test {
+		compatible = "sandbox,dsa";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			swp_0: port at 0 {
+				reg = <0>;
+				label = "lan0";
+			};
+
+			swp_1: port at 1 {
+				reg = <1>;
+				label = "lan1";
+				phy-mode = "rgmii-txid";
+				fixed-link {
+					speed = <1000>;
+					full-duplex;
+				};
+			};
+
+			swp_2: port at 2 {
+				reg = <2>;
+				label = "lan2";
+				fixed-link {
+					speed = <100>;
+					full-duplex;
+				};
+			};
+
+			port at 3 {
+				reg = <3>;
+				ethernet = <&dsa_eth0>;
+				fixed-link {
+					speed = <100>;
+					full-duplex;
+				};
+			};
+		};
+	};
 };
 
 #include "sandbox_pmic.dtsi"
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
index 1c13055cdc..35a5676eb9 100644
--- a/include/configs/sandbox.h
+++ b/include/configs/sandbox.h
@@ -100,6 +100,10 @@
 					"eth1addr=00:00:11:22:33:45\0" \
 					"eth3addr=00:00:11:22:33:46\0" \
 					"eth5addr=00:00:11:22:33:47\0" \
+					"eth8addr=00:00:11:22:33:48\0" \
+					"eth9addr=00:00:11:22:33:49\0" \
+					"eth10addr=00:00:11:22:33:4a\0" \
+					"eth11addr=00:00:11:22:33:4b\0" \
 					"ipaddr=1.2.3.4\0"
 
 #define MEM_LAYOUT_ENV_SETTINGS \
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 0c2fd5cb5e..69e9feed91 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -65,4 +65,5 @@ obj-$(CONFIG_VIRTIO_SANDBOX) += virtio.o
 obj-$(CONFIG_DMA) += dma.o
 obj-$(CONFIG_DM_MDIO) += mdio.o
 obj-$(CONFIG_DM_MDIO_MUX) += mdio_mux.o
+obj-$(CONFIG_DM_DSA) += dsa.o
 endif
diff --git a/test/dm/dsa.c b/test/dm/dsa.c
new file mode 100644
index 0000000000..5aa3847fe5
--- /dev/null
+++ b/test/dm/dsa.c
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019 NXP
+ */
+
+#include <net/dsa.h>
+#include <dm/test.h>
+#include <test/ut.h>
+
+extern int dsa_sandbox_port_mask;
+
+/* this test sends ping requests with the local address through each DSA port
+ * via the dummy DSA master Eth.
+ * The dummy Eth filters traffic based on DSA port used to Tx and the port
+ * mask set here, so we can check that port information gets trough correctly.
+ */
+static int dm_test_dsa(struct unit_test_state *uts)
+{
+	dsa_sandbox_port_mask = 0x5;
+
+	env_set("ethrotate", "no");
+	net_ping_ip = string_to_ip("1.2.3.4");
+
+	env_set("ethact", "dsa-test-eth");
+	ut_assertok(net_loop(PING));
+
+	dsa_sandbox_port_mask = 0x7;
+	env_set("ethact", "lan0");
+	ut_assertok(net_loop(PING));
+	env_set("ethact", "lan1");
+	ut_assertok(net_loop(PING));
+	env_set("ethact", "lan2");
+	ut_assertok(net_loop(PING));
+
+	dsa_sandbox_port_mask = 0x1;
+	env_set("ethact", "lan0");
+	ut_assertok(net_loop(PING));
+	env_set("ethact", "lan1");
+	ut_assert(net_loop(PING) != 0);
+	env_set("ethact", "lan2");
+	ut_assert(net_loop(PING) != 0);
+
+	dsa_sandbox_port_mask = 0x6;
+	env_set("ethact", "lan0");
+	ut_assert(net_loop(PING) != 0);
+	env_set("ethact", "lan1");
+	ut_assertok(net_loop(PING));
+	env_set("ethact", "lan2");
+	ut_assertok(net_loop(PING));
+
+	dsa_sandbox_port_mask = 0;
+	env_set("ethact", "");
+	env_set("ethrotate", "yes");
+
+	return 0;
+}
+
+DM_TEST(dm_test_dsa, DM_TESTF_SCAN_FDT);
diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c
index 1fb8b5c248..0d7dd053e2 100644
--- a/test/dm/test-fdt.c
+++ b/test/dm/test-fdt.c
@@ -224,7 +224,7 @@ static int dm_test_alias_highest_id(struct unit_test_state *uts)
 	int ret;
 
 	ret = dev_read_alias_highest_id("eth");
-	ut_asserteq(5, ret);
+	ut_asserteq(11, ret);
 
 	ret = dev_read_alias_highest_id("gpio");
 	ut_asserteq(2, ret);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3 4/6] drivers: net: add Felix DSA switch driver
  2019-12-03 14:56 [PATCH v3 0/6] Introduce DSA Ethernet switch class and Felix driver Alex Marginean
                   ` (2 preceding siblings ...)
  2019-12-03 14:56 ` [PATCH v3 3/6] test: dm: add a simple unit test for DSA class Alex Marginean
@ 2019-12-03 14:56 ` Alex Marginean
  2019-12-15 12:53   ` Vladimir Oltean
  2019-12-03 14:56 ` [PATCH v3 5/6] arm: dts: ls1028a: adds Ethernet switch node and its dependencies Alex Marginean
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 25+ messages in thread
From: Alex Marginean @ 2019-12-03 14:56 UTC (permalink / raw)
  To: u-boot

This driver is used for the Ethernet switch integrated into LS1028A NXP.
Felix on LS1028A has 4 front panel ports and two internal ports, I/O
to/from the switch is done through an ENETC Ethernet interface.
The 4 front panel ports are available as Ethernet interfaces and can be
used with the typical network commands like tftp.

Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
Tested-by: Michael Walle <michael@walle.cc>
---
 drivers/net/fsl_enetc.h                 |   5 +
 drivers/net/mscc_eswitch/Kconfig        |   8 +
 drivers/net/mscc_eswitch/Makefile       |   1 +
 drivers/net/mscc_eswitch/felix_switch.c | 454 ++++++++++++++++++++++++
 4 files changed, 468 insertions(+)
 create mode 100644 drivers/net/mscc_eswitch/felix_switch.c

diff --git a/drivers/net/fsl_enetc.h b/drivers/net/fsl_enetc.h
index 9a36cdad80..29e7781b5e 100644
--- a/drivers/net/fsl_enetc.h
+++ b/drivers/net/fsl_enetc.h
@@ -200,6 +200,11 @@ struct enetc_priv {
 /* PCS replicator block for USXGMII */
 #define ENETC_PCS_DEVAD_REPL		0x1f
 
+#define ENETC_PCS_REPL_LINK_TIMER_1	0x12
+#define  ENETC_PCS_REPL_LINK_TIMER_1_DEF	0x0003
+#define ENETC_PCS_REPL_LINK_TIMER_2	0x13
+#define  ENETC_PCS_REPL_LINK_TIMER_2_DEF	0x06a0
+
 /* ENETC external MDIO registers */
 #define ENETC_MDIO_BASE		0x1c00
 #define ENETC_MDIO_CFG		0x00
diff --git a/drivers/net/mscc_eswitch/Kconfig b/drivers/net/mscc_eswitch/Kconfig
index 80dd22f98b..11fb08edaa 100644
--- a/drivers/net/mscc_eswitch/Kconfig
+++ b/drivers/net/mscc_eswitch/Kconfig
@@ -36,3 +36,11 @@ config MSCC_SERVAL_SWITCH
 	select PHYLIB
 	help
 	  This driver supports the Serval network switch device.
+
+config MSCC_FELIX_SWITCH
+	bool "Felix switch driver"
+	depends on DM_DSA && DM_PCI
+	select FSL_ENETC
+	help
+	  This driver supports the Ethernet switch integrated in LS1028A NXP
+	  SoC.
diff --git a/drivers/net/mscc_eswitch/Makefile b/drivers/net/mscc_eswitch/Makefile
index d583fe9fc4..22342ed114 100644
--- a/drivers/net/mscc_eswitch/Makefile
+++ b/drivers/net/mscc_eswitch/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_MSCC_LUTON_SWITCH) += luton_switch.o mscc_xfer.o mscc_mac_table.o m
 obj-$(CONFIG_MSCC_JR2_SWITCH) += jr2_switch.o mscc_xfer.o mscc_miim.o
 obj-$(CONFIG_MSCC_SERVALT_SWITCH) += servalt_switch.o mscc_xfer.o mscc_miim.o
 obj-$(CONFIG_MSCC_SERVAL_SWITCH) += serval_switch.o mscc_xfer.o mscc_mac_table.o mscc_miim.o
+obj-$(CONFIG_MSCC_FELIX_SWITCH) += felix_switch.o
diff --git a/drivers/net/mscc_eswitch/felix_switch.c b/drivers/net/mscc_eswitch/felix_switch.c
new file mode 100644
index 0000000000..3723aad4b4
--- /dev/null
+++ b/drivers/net/mscc_eswitch/felix_switch.c
@@ -0,0 +1,454 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Felix Ethernet switch driver
+ * Copyright 2018-2019 NXP
+ */
+
+/*
+ * This driver is used for the Ethernet switch integrated into LS1028A NXP.
+ * Felix switch is derived from Microsemi Ocelot but there are several NXP
+ * adaptations that makes the two U-Boot drivers largely incompatible.
+ *
+ * Felix on LS1028A has 4 front panel ports and two internal ports, connected
+ * to ENETC interfaces.  We're using one of the ENETC interfaces to push traffic
+ * into the switch.  Injection/extraction headers are used to identify
+ * egress/ingress ports in the switch for Tx/Rx.
+ */
+
+#include <common.h>
+#include <net/dsa.h>
+#include <asm/io.h>
+#include <pci.h>
+#include <miiphy.h>
+
+/* defines especially around PCS are reused from enetc */
+#include "../fsl_enetc.h"
+
+#define PCI_DEVICE_ID_FELIX_ETHSW	0xEEF0
+
+/* Felix has in fact 6 ports, but we don't use the last internal one */
+#define FELIX_PORT_COUNT		5
+/* Front panel port mask */
+#define FELIX_FP_PORT_MASK		0xf
+
+/* Register map for BAR4 */
+#define FELIX_SYS			0x010000
+#define FELIX_ES0			0x040000
+#define FELIX_IS1			0x050000
+#define FELIX_IS2			0x060000
+#define FELIX_GMII(port)		(0x100000 + (port) * 0x10000)
+#define FELIX_QSYS			0x200000
+
+#define FELIX_SYS_SYSTEM		(FELIX_SYS + 0x00000E00)
+#define  FELIX_SYS_SYSTEM_EN		BIT(0)
+#define FELIX_SYS_RAM_CTRL		(FELIX_SYS + 0x00000F24)
+#define  FELIX_SYS_RAM_CTRL_INIT	BIT(1)
+#define FELIX_SYS_SYSTEM_PORT_MODE(a)	(FELIX_SYS_SYSTEM + 0xC + (a) * 4)
+#define  FELIX_SYS_SYSTEM_PORT_MODE_CPU	0x0000001e
+
+#define FELIX_ES0_TCAM_CTRL		(FELIX_ES0 + 0x000003C0)
+#define  FELIX_ES0_TCAM_CTRL_EN		BIT(0)
+#define FELIX_IS1_TCAM_CTRL		(FELIX_IS1 + 0x000003C0)
+#define  FELIX_IS1_TCAM_CTRL_EN		BIT(0)
+#define FELIX_IS2_TCAM_CTRL		(FELIX_IS2 + 0x000003C0)
+#define  FELIX_IS2_TCAM_CTRL_EN		BIT(0)
+
+#define FELIX_GMII_CLOCK_CFG(port)	(FELIX_GMII(port) + 0x00000000)
+#define  FELIX_GMII_CLOCK_CFG_LINK_1G	1
+#define  FELIX_GMII_CLOCK_CFG_LINK_100M	2
+#define  FELIX_GMII_CLOCK_CFG_LINK_10M	3
+#define FELIX_GMII_MAC_ENA_CFG(port)	(FELIX_GMII(port) + 0x0000001C)
+#define  FELIX_GMII_MAX_ENA_CFG_TX	BIT(0)
+#define  FELIX_GMII_MAX_ENA_CFG_RX	BIT(4)
+#define FELIX_GMII_MAC_IFG_CFG(port)	(FELIX_GMII(port) + 0x0000001C + 0x14)
+#define  FELIX_GMII_MAC_IFG_CFG_DEF	0x515
+
+#define FELIX_QSYS_SYSTEM		(FELIX_QSYS + 0x0000F460)
+#define FELIX_QSYS_SYSTEM_SW_PORT_MODE(a)	\
+					(FELIX_QSYS_SYSTEM + 0x20 + (a) * 4)
+#define  FELIX_QSYS_SYSTEM_SW_PORT_ENA		BIT(14)
+#define  FELIX_QSYS_SYSTEM_SW_PORT_LOSSY	BIT(9)
+#define  FELIX_QSYS_SYSTEM_SW_PORT_SCH(a)	(((a) & 0x3800) << 11)
+#define FELIX_QSYS_SYSTEM_EXT_CPU_CFG	(FELIX_QSYS_SYSTEM + 0x80)
+#define  FELIX_QSYS_SYSTEM_EXT_CPU_PORT(a)	(((a) & 0xf) << 8 | 0xff)
+
+/* internal MDIO in BAR0 */
+#define FELIX_PM_IMDIO_BASE		0x8030
+
+/* Serdes block on LS1028A */
+#define FELIX_SERDES_BASE		0x1ea0000L
+#define FELIX_SERDES_LNATECR0(lane)	(FELIX_SERDES_BASE + 0x818 + \
+					 (lane) * 0x40)
+#define  FELIX_SERDES_LNATECR0_ADPT_EQ	0x00003000
+#define FELIX_SERDES_SGMIICR1(lane)	(FELIX_SERDES_BASE + 0x1804 + \
+					 (lane) * 0x10)
+#define  FELIX_SERDES_SGMIICR1_SGPCS	BIT(11)
+#define  FELIX_SERDES_SGMIICR1_MDEV(a)	(((a) & 0x1f) << 27)
+
+#define FELIX_PCS_CTRL			0
+#define  FELIX_PCS_CTRL_RST		BIT(15)
+
+/*
+ * The long prefix format used here contains two dummy MAC addresses, a magic
+ * value in place of a VLAN tag followed by the extraction/injection header and
+ * the original L2 frame.  Out of all this we only use the port ID.
+ */
+
+#define FELIX_DSA_TAG_LEN		sizeof(struct felix_dsa_tag)
+#define FELIX_DSA_TAG_MAGIC		0x0a008088
+#define FELIX_DSA_TAG_INJ_PORT		7
+#define  FELIX_DSA_TAG_INJ_PORT_SET(a)	(0x1 << ((a) & FELIX_FP_PORT_MASK))
+#define FELIX_DSA_TAG_EXT_PORT		10
+#define  FELIX_DSA_TAG_EXT_PORT_GET(a)	((a) >> 3)
+
+struct felix_dsa_tag {
+	uchar d_mac[6];
+	uchar s_mac[6];
+	u32   magic;
+	uchar meta[16];
+};
+
+struct felix_priv {
+	void *regs_base;
+	void *imdio_base;
+	struct mii_dev imdio;
+	struct udevice *port[FELIX_PORT_COUNT];
+};
+
+/* MDIO wrappers, we're using these to drive internal MDIO to get to serdes */
+static int felix_mdio_read(struct mii_dev *bus, int addr, int devad, int reg)
+{
+	struct enetc_mdio_priv priv;
+
+	priv.regs_base = bus->priv;
+	return enetc_mdio_read_priv(&priv, addr, devad, reg);
+}
+
+static int felix_mdio_write(struct mii_dev *bus, int addr, int devad, int reg,
+			    u16 val)
+{
+	struct enetc_mdio_priv priv;
+
+	priv.regs_base = bus->priv;
+	return enetc_mdio_write_priv(&priv, addr, devad, reg, val);
+}
+
+/* set up serdes for SGMII */
+static int felix_init_sgmii(struct udevice *dev, int port, int if_type)
+{
+	struct felix_priv *priv = dev_get_priv(dev);
+	bool is2500 = false;
+	u16 reg;
+
+	/* set up PCS lane address */
+	out_le32(FELIX_SERDES_SGMIICR1(port), FELIX_SERDES_SGMIICR1_SGPCS |
+		 FELIX_SERDES_SGMIICR1_MDEV(port));
+
+	if (!priv->imdio.priv)
+		return 0;
+
+	if (if_type == PHY_INTERFACE_MODE_SGMII_2500)
+		is2500 = true;
+
+	/*
+	 * Set to SGMII mode, for 1Gbps enable AN, for 2.5Gbps set fixed speed.
+	 * Although fixed speed is 1Gbps, we could be running at 2.5Gbps based
+	 * on PLL configuration.  Setting 1G for 2.5G here is counter intuitive
+	 * but intentional.
+	 */
+	reg = ENETC_PCS_IF_MODE_SGMII;
+	reg |= is2500 ? ENETC_PCS_IF_MODE_SPEED_1G : ENETC_PCS_IF_MODE_SGMII_AN;
+	felix_mdio_write(&priv->imdio, port, MDIO_DEVAD_NONE,
+			 ENETC_PCS_IF_MODE, reg);
+
+	/* Dev ability - SGMII */
+	felix_mdio_write(&priv->imdio, port, MDIO_DEVAD_NONE,
+			 ENETC_PCS_DEV_ABILITY, ENETC_PCS_DEV_ABILITY_SGMII);
+
+	/* Adjust link timer for SGMII */
+	felix_mdio_write(&priv->imdio, port, MDIO_DEVAD_NONE,
+			 ENETC_PCS_LINK_TIMER1, ENETC_PCS_LINK_TIMER1_VAL);
+	felix_mdio_write(&priv->imdio, port, MDIO_DEVAD_NONE,
+			 ENETC_PCS_LINK_TIMER2, ENETC_PCS_LINK_TIMER2_VAL);
+
+	reg = ENETC_PCS_CR_DEF_VAL;
+	reg |= is2500 ? ENETC_PCS_CR_RST : ENETC_PCS_CR_RESET_AN;
+	/* restart PCS AN */
+	felix_mdio_write(&priv->imdio, port, MDIO_DEVAD_NONE,
+			 ENETC_PCS_CR, reg);
+
+	return 0;
+}
+
+/* set up MAC and serdes for (Q)SXGMII */
+static int felix_init_sxgmii(struct udevice *dev, int port)
+{
+	struct felix_priv *priv = dev_get_priv(dev);
+	int to = 1000;
+
+	/* set up transit equalization control on serdes lane */
+	out_le32(FELIX_SERDES_LNATECR0(1), FELIX_SERDES_LNATECR0_ADPT_EQ);
+
+	if (!priv->imdio.priv)
+		return 0;
+
+	/*reset lane */
+	felix_mdio_write(&priv->imdio, port, MDIO_MMD_PCS, FELIX_PCS_CTRL,
+			 FELIX_PCS_CTRL_RST);
+	while (felix_mdio_read(&priv->imdio, port, MDIO_MMD_PCS,
+			       FELIX_PCS_CTRL) & FELIX_PCS_CTRL_RST &&
+			--to) {
+		mdelay(10);
+	}
+	if (felix_mdio_read(&priv->imdio, port, MDIO_MMD_PCS,
+			    FELIX_PCS_CTRL) & FELIX_PCS_CTRL_RST)
+		dev_dbg(port, "PCS reset time-out\n");
+
+	/* Dev ability - SXGMII */
+	felix_mdio_write(&priv->imdio, port, ENETC_PCS_DEVAD_REPL,
+			 ENETC_PCS_DEV_ABILITY, ENETC_PCS_DEV_ABILITY_SXGMII);
+
+	/* Restart PCS AN */
+	felix_mdio_write(&priv->imdio, port, ENETC_PCS_DEVAD_REPL,
+			 ENETC_PCS_CR,
+			 ENETC_PCS_CR_RST | ENETC_PCS_CR_RESET_AN);
+	felix_mdio_write(&priv->imdio, port, ENETC_PCS_DEVAD_REPL,
+			 ENETC_PCS_REPL_LINK_TIMER_1,
+			 ENETC_PCS_REPL_LINK_TIMER_1_DEF);
+	felix_mdio_write(&priv->imdio, port, ENETC_PCS_DEVAD_REPL,
+			 ENETC_PCS_REPL_LINK_TIMER_2,
+			 ENETC_PCS_REPL_LINK_TIMER_2_DEF);
+
+	return 0;
+}
+
+/* Apply protocol specific configuration to MAC, serdes as needed */
+static void felix_start_pcs(struct udevice *dev, int port)
+{
+	struct dsa_perdev_platdata *platdata = dev->platdata;
+	struct felix_priv *priv = dev_get_priv(dev);
+	const char *if_str;
+	int if_type;
+
+	if_type = PHY_INTERFACE_MODE_NONE;
+
+	priv->imdio.read = felix_mdio_read;
+	priv->imdio.write = felix_mdio_write;
+	priv->imdio.priv = priv->imdio_base + FELIX_PM_IMDIO_BASE;
+	strncpy(priv->imdio.name, dev->name, MDIO_NAME_LEN);
+
+	if_str = ofnode_read_string(platdata->port[port].node, "phy-mode");
+	if (if_str)
+		if_type = phy_get_interface_by_name(if_str);
+	else
+		dev_dbg(port,
+			"phy-mode property not found, defaulting to NONE\n");
+	if (if_type < 0)
+		if_type = PHY_INTERFACE_MODE_NONE;
+
+	switch (if_type) {
+	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_SGMII_2500:
+	case PHY_INTERFACE_MODE_QSGMII:
+		felix_init_sgmii(dev, port, if_type);
+		break;
+	case PHY_INTERFACE_MODE_XGMII:
+	case PHY_INTERFACE_MODE_XFI:
+	case PHY_INTERFACE_MODE_USXGMII:
+		felix_init_sxgmii(dev, port);
+		break;
+	}
+}
+
+void felix_init(struct udevice *dev)
+{
+	struct dsa_perdev_platdata *platdata = dev->platdata;
+	struct felix_priv *priv = dev_get_priv(dev);
+	int supported, to = 100, port;
+	void *base = priv->regs_base;
+	struct phy_device *phy;
+
+	dev_dbg(dev, "trying to set up L2 switch\n");
+
+	/* Init core memories */
+	out_le32(base + FELIX_SYS_RAM_CTRL, FELIX_SYS_RAM_CTRL_INIT);
+	while (in_le32(base + FELIX_SYS_RAM_CTRL) & FELIX_SYS_RAM_CTRL_INIT &&
+	       --to)
+		udelay(10);
+	if (in_le32(base + FELIX_SYS_RAM_CTRL) & FELIX_SYS_RAM_CTRL_INIT)
+		dev_dbg(dev, "Time-out waiting for switch memories\n");
+
+	/* Start switch core, set up ES0, IS1, IS2 */
+	out_le32(base + FELIX_SYS_SYSTEM, FELIX_SYS_SYSTEM_EN);
+	out_le32(base + FELIX_ES0_TCAM_CTRL, FELIX_ES0_TCAM_CTRL_EN);
+	out_le32(base + FELIX_IS1_TCAM_CTRL, FELIX_IS1_TCAM_CTRL_EN);
+	out_le32(base + FELIX_IS2_TCAM_CTRL, FELIX_IS2_TCAM_CTRL_EN);
+	udelay(20);
+
+	supported = PHY_GBIT_FEATURES | SUPPORTED_2500baseX_Full;
+
+	for (port = 0; port < FELIX_PORT_COUNT; port++) {
+		/* Set up MAC registers */
+		out_le32(base + FELIX_GMII_CLOCK_CFG(port),
+			 FELIX_GMII_CLOCK_CFG_LINK_1G);
+
+		out_le32(base + FELIX_GMII_MAC_IFG_CFG(port),
+			 FELIX_GMII_MAC_IFG_CFG_DEF);
+
+		felix_start_pcs(dev, port);
+
+		phy = platdata->port[port].phy;
+		if (phy) {
+			phy->supported &= supported;
+			phy->advertising &= supported;
+			phy_config(phy);
+		}
+	}
+
+	/* set up CPU port */
+	out_le32(base + FELIX_QSYS_SYSTEM_EXT_CPU_CFG,
+		 FELIX_QSYS_SYSTEM_EXT_CPU_PORT(platdata->cpu_port));
+	out_le32(base + FELIX_SYS_SYSTEM_PORT_MODE(platdata->cpu_port),
+		 FELIX_SYS_SYSTEM_PORT_MODE_CPU);
+}
+
+static int felix_bind(struct udevice *dev)
+{
+	struct dsa_perdev_platdata *pdata = dev->platdata;
+
+	pdata->num_ports = FELIX_PORT_COUNT;
+	pdata->headroom = FELIX_DSA_TAG_LEN;
+
+	return 0;
+}
+
+/*
+ * Probe Felix:
+ * - enable the PCI function
+ * - map BAR 4
+ * - init switch core and port registers
+ */
+static int felix_probe(struct udevice *dev)
+{
+	struct felix_priv *priv = dev_get_priv(dev);
+
+	if (ofnode_valid(dev->node) && !ofnode_is_available(dev->node)) {
+		dev_dbg(dev, "switch disabled\n");
+		return -ENODEV;
+	}
+
+	priv->imdio_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0);
+	if (!priv->imdio_base) {
+		dev_dbg(dev, "failed to map BAR0\n");
+		return -EINVAL;
+	}
+
+	priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_4, 0);
+	if (!priv->regs_base) {
+		dev_dbg(dev, "failed to map BAR4\n");
+		return -EINVAL;
+	}
+
+	/* register internal MDIO for debug */
+	if (!miiphy_get_dev_by_name(dev->name)) {
+		struct mii_dev *mii_bus;
+
+		mii_bus = mdio_alloc();
+		mii_bus->read = felix_mdio_read;
+		mii_bus->write = felix_mdio_write;
+		mii_bus->priv = priv->imdio_base + FELIX_PM_IMDIO_BASE;
+		strncpy(mii_bus->name, dev->name, MDIO_NAME_LEN);
+		mdio_register(mii_bus);
+	}
+
+	dm_pci_clrset_config16(dev, PCI_COMMAND, 0, PCI_COMMAND_MEMORY);
+
+	/* set up registers */
+	felix_init(dev);
+
+	return 0;
+}
+
+static int felix_port_enable(struct udevice *dev, int port,
+			     struct phy_device *phy)
+{
+	struct felix_priv *priv = dev_get_priv(dev);
+	void *base = priv->regs_base;
+
+	out_le32(base + FELIX_GMII_MAC_ENA_CFG(port),
+		 FELIX_GMII_MAX_ENA_CFG_TX | FELIX_GMII_MAX_ENA_CFG_RX);
+
+	out_le32(base + FELIX_QSYS_SYSTEM_SW_PORT_MODE(port),
+		 FELIX_QSYS_SYSTEM_SW_PORT_ENA |
+		 FELIX_QSYS_SYSTEM_SW_PORT_LOSSY |
+		 FELIX_QSYS_SYSTEM_SW_PORT_SCH(1));
+
+	if (phy)
+		phy_startup(phy);
+	return 0;
+}
+
+static void felix_port_disable(struct udevice *dev, int port,
+			       struct phy_device *phy)
+{
+	struct felix_priv *priv = dev_get_priv(dev);
+	void *base = priv->regs_base;
+
+	out_le32(base + FELIX_GMII_MAC_ENA_CFG(port), 0);
+
+	out_le32(base + FELIX_QSYS_SYSTEM_SW_PORT_MODE(port),
+		 FELIX_QSYS_SYSTEM_SW_PORT_LOSSY |
+		 FELIX_QSYS_SYSTEM_SW_PORT_SCH(1));
+
+	/*
+	 * we don't call phy_shutdown here to avoind waiting next time we use
+	 * the port, but the downside is that remote side will think we're
+	 * actively processing traffic although we are not.
+	 */
+}
+
+static int felix_xmit(struct udevice *dev, int port, void *packet, int length)
+{
+	struct felix_dsa_tag *tag = packet;
+
+	tag->magic = FELIX_DSA_TAG_MAGIC;
+	tag->meta[FELIX_DSA_TAG_INJ_PORT] = FELIX_DSA_TAG_INJ_PORT_SET(port);
+
+	return 0;
+}
+
+static int felix_rcv(struct udevice *dev, int *port, void *packet, int length)
+{
+	struct felix_dsa_tag *tag = packet;
+
+	if (tag->magic != FELIX_DSA_TAG_MAGIC)
+		return -EINVAL;
+
+	*port = FELIX_DSA_TAG_EXT_PORT_GET(tag->meta[FELIX_DSA_TAG_EXT_PORT]);
+
+	return 0;
+}
+
+static const struct dsa_ops felix_dsa_ops = {
+	.port_enable  = felix_port_enable,
+	.port_disable = felix_port_disable,
+	.xmit         = felix_xmit,
+	.rcv          = felix_rcv,
+};
+
+U_BOOT_DRIVER(felix_ethsw) = {
+	.name	= "felix-switch",
+	.id	= UCLASS_DSA,
+	.bind	= felix_bind,
+	.probe	= felix_probe,
+	.ops    = &felix_dsa_ops,
+	.priv_auto_alloc_size = sizeof(struct felix_priv),
+	.platdata_auto_alloc_size = sizeof(struct dsa_perdev_platdata),
+};
+
+static struct pci_device_id felix_ethsw_ids[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, PCI_DEVICE_ID_FELIX_ETHSW) },
+	{}
+};
+
+U_BOOT_PCI_DEVICE(felix_ethsw, felix_ethsw_ids);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3 5/6] arm: dts: ls1028a: adds Ethernet switch node and its dependencies
  2019-12-03 14:56 [PATCH v3 0/6] Introduce DSA Ethernet switch class and Felix driver Alex Marginean
                   ` (3 preceding siblings ...)
  2019-12-03 14:56 ` [PATCH v3 4/6] drivers: net: add Felix DSA switch driver Alex Marginean
@ 2019-12-03 14:56 ` Alex Marginean
  2019-12-03 14:56 ` [PATCH v3 6/6] configs: ls1028a: enable the Ethernet switch driver in defconfig Alex Marginean
  2020-02-08 13:19 ` [PATCH v3 0/6] Introduce DSA Ethernet switch class and Felix driver Vladimir Oltean
  6 siblings, 0 replies; 25+ messages in thread
From: Alex Marginean @ 2019-12-03 14:56 UTC (permalink / raw)
  To: u-boot

The definition follows the DSA binding in kernel and describes the switch,
its ports and PHYs.
ENETC PF6 is the 2nd Eth controller linked to the switch on LS1028A, it is
not used in U-Boot and was disabled.

Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
Tested-by: Michael Walle <michael@walle.cc>
---
 arch/arm/dts/fsl-ls1028a-rdb.dts | 36 ++++++++++++++++++++++++++
 arch/arm/dts/fsl-ls1028a.dtsi    | 44 +++++++++++++++++++++++++++++++-
 2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/arch/arm/dts/fsl-ls1028a-rdb.dts b/arch/arm/dts/fsl-ls1028a-rdb.dts
index 3d5e8ade21..700fc067a4 100644
--- a/arch/arm/dts/fsl-ls1028a-rdb.dts
+++ b/arch/arm/dts/fsl-ls1028a-rdb.dts
@@ -114,9 +114,45 @@
 	phy-handle = <&rdb_phy0>;
 };
 
+&ethsw_ports {
+	port at 0 {
+		status = "okay";
+		phy-mode = "qsgmii";
+		phy-handle = <&sw_phy0>;
+	};
+	port at 1 {
+		status = "okay";
+		phy-mode = "qsgmii";
+		phy-handle = <&sw_phy1>;
+	};
+	port at 2 {
+		status = "okay";
+		phy-mode = "qsgmii";
+		phy-handle = <&sw_phy2>;
+	};
+	port at 3 {
+		status = "okay";
+		phy-mode = "qsgmii";
+		phy-handle = <&sw_phy3>;
+	};
+};
+
 &mdio0 {
 	status = "okay";
 	rdb_phy0: phy at 2 {
 		reg = <2>;
 	};
+
+	sw_phy0: phy at 10 {
+		reg = <0x10>;
+	};
+	sw_phy1: phy at 11 {
+		reg = <0x11>;
+	};
+	sw_phy2: phy at 12 {
+		reg = <0x12>;
+	};
+	sw_phy3: phy at 13 {
+		reg = <0x13>;
+	};
 };
diff --git a/arch/arm/dts/fsl-ls1028a.dtsi b/arch/arm/dts/fsl-ls1028a.dtsi
index 43a154e8e7..97c7d4de4d 100644
--- a/arch/arm/dts/fsl-ls1028a.dtsi
+++ b/arch/arm/dts/fsl-ls1028a.dtsi
@@ -136,9 +136,51 @@
 			reg = <0x000300 0 0 0 0>;
 			status = "disabled";
 		};
+		ethsw: pci at 0,5 {
+			#address-cells=<0>;
+			#size-cells=<1>;
+			reg = <0x000500 0 0 0 0>;
+
+			ethsw_ports: ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port at 0 {
+					reg = <0>;
+					status = "disabled";
+					label = "swp0";
+				};
+				port at 1 {
+					reg = <1>;
+					status = "disabled";
+					label = "swp1";
+				};
+				port at 2 {
+					reg = <2>;
+					status = "disabled";
+					label = "swp2";
+				};
+				port at 3 {
+					reg = <3>;
+					status = "disabled";
+					label = "swp3";
+				};
+				port at 4 {
+					reg = <4>;
+					phy-mode = "internal";
+					status = "okay";
+					ethernet = <&enetc2>;
+				};
+				port at 5 {
+					reg = <5>;
+					phy-mode = "internal";
+					status = "disabled";
+				};
+			};
+		};
 		enetc6: pci at 0,6 {
 			reg = <0x000600 0 0 0 0>;
-			status = "okay";
+			status = "disabled";
 			phy-mode = "internal";
 		};
 	};
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3 6/6] configs: ls1028a: enable the Ethernet switch driver in defconfig
  2019-12-03 14:56 [PATCH v3 0/6] Introduce DSA Ethernet switch class and Felix driver Alex Marginean
                   ` (4 preceding siblings ...)
  2019-12-03 14:56 ` [PATCH v3 5/6] arm: dts: ls1028a: adds Ethernet switch node and its dependencies Alex Marginean
@ 2019-12-03 14:56 ` Alex Marginean
  2020-03-13 14:33   ` Vladimir Oltean
  2020-02-08 13:19 ` [PATCH v3 0/6] Introduce DSA Ethernet switch class and Felix driver Vladimir Oltean
  6 siblings, 1 reply; 25+ messages in thread
From: Alex Marginean @ 2019-12-03 14:56 UTC (permalink / raw)
  To: u-boot

The switch driver for LS1028A Ethernet switch is now compiled in for
both LS1028A boards.

Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
---
 configs/ls1028aqds_tfa_SECURE_BOOT_defconfig | 3 ++-
 configs/ls1028aqds_tfa_defconfig             | 3 ++-
 configs/ls1028ardb_tfa_SECURE_BOOT_defconfig | 3 ++-
 configs/ls1028ardb_tfa_defconfig             | 3 ++-
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig b/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig
index 4a01cd6715..65e467817e 100644
--- a/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig
+++ b/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig
@@ -50,8 +50,9 @@ CONFIG_PHY_ATHEROS=y
 CONFIG_PHY_VITESSE=y
 CONFIG_DM_ETH=y
 CONFIG_DM_MDIO=y
+CONFIG_DM_DSA=y
 CONFIG_E1000=y
-CONFIG_FSL_ENETC=y
+CONFIG_MSCC_FELIX_SWITCH=y
 CONFIG_PCI=y
 CONFIG_DM_PCI=y
 CONFIG_DM_PCI_COMPAT=y
diff --git a/configs/ls1028aqds_tfa_defconfig b/configs/ls1028aqds_tfa_defconfig
index 1307f0d951..40d259d907 100644
--- a/configs/ls1028aqds_tfa_defconfig
+++ b/configs/ls1028aqds_tfa_defconfig
@@ -56,8 +56,9 @@ CONFIG_PHY_ATHEROS=y
 CONFIG_PHY_VITESSE=y
 CONFIG_DM_ETH=y
 CONFIG_DM_MDIO=y
+CONFIG_DM_DSA=y
 CONFIG_E1000=y
-CONFIG_FSL_ENETC=y
+CONFIG_MSCC_FELIX_SWITCH=y
 CONFIG_PCI=y
 CONFIG_DM_PCI=y
 CONFIG_DM_PCI_COMPAT=y
diff --git a/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig b/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig
index d0a3310a4c..f54a6da31b 100644
--- a/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig
+++ b/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig
@@ -49,9 +49,10 @@ CONFIG_PHY_ATHEROS=y
 CONFIG_PHY_VITESSE=y
 CONFIG_DM_ETH=y
 CONFIG_DM_MDIO=y
+CONFIG_DM_DSA=y
 CONFIG_PHY_GIGE=y
 CONFIG_E1000=y
-CONFIG_FSL_ENETC=y
+CONFIG_MSCC_FELIX_SWITCH=y
 CONFIG_PCI=y
 CONFIG_DM_PCI=y
 CONFIG_DM_PCI_COMPAT=y
diff --git a/configs/ls1028ardb_tfa_defconfig b/configs/ls1028ardb_tfa_defconfig
index 4ec7ed0920..e018e5a50e 100644
--- a/configs/ls1028ardb_tfa_defconfig
+++ b/configs/ls1028ardb_tfa_defconfig
@@ -56,9 +56,10 @@ CONFIG_PHY_ATHEROS=y
 CONFIG_PHY_VITESSE=y
 CONFIG_DM_ETH=y
 CONFIG_DM_MDIO=y
+CONFIG_DM_DSA=y
 CONFIG_PHY_GIGE=y
 CONFIG_E1000=y
-CONFIG_FSL_ENETC=y
+CONFIG_MSCC_FELIX_SWITCH=y
 CONFIG_PCI=y
 CONFIG_DM_PCI=y
 CONFIG_DM_PCI_COMPAT=y
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3 3/6] test: dm: add a simple unit test for DSA class
  2019-12-03 14:56 ` [PATCH v3 3/6] test: dm: add a simple unit test for DSA class Alex Marginean
@ 2019-12-04  4:14   ` Priyanka Jain
  0 siblings, 0 replies; 25+ messages in thread
From: Priyanka Jain @ 2019-12-04  4:14 UTC (permalink / raw)
  To: u-boot



>-----Original Message-----
>From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Alex Marginean
>Sent: Tuesday, December 3, 2019 8:27 PM
>To: u-boot at lists.denx.de
>Cc: Joe Hershberger <joe.hershberger@ni.com>; Claudiu Manoil
><claudiu.manoil@nxp.com>; Vladimir Oltean <vladimir.oltean@nxp.com>
>Subject: [PATCH v3 3/6] test: dm: add a simple unit test for DSA class
>
>The test pings the local IP address though different ports of a sandbox DSA
>device.  Port traffic is filtered and the test verifies that ping works only on
>enabled ports.
>The additional interfaces require MAC addresses, these have been added to
>sandbox default environment.
>
>Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
>---
> arch/Kconfig              |  1 +
> arch/sandbox/dts/test.dts | 49 +++++++++++++++++++++++++++++++++
>include/configs/sandbox.h |  4 +++
> test/dm/Makefile          |  1 +
> test/dm/dsa.c             | 58 +++++++++++++++++++++++++++++++++++++++
> test/dm/test-fdt.c        |  2 +-
> 6 files changed, 114 insertions(+), 1 deletion(-)  create mode 100644
>test/dm/dsa.c
>
>diff --git a/arch/Kconfig b/arch/Kconfig index 141e48bc43..70907d69a1 100644
>--- a/arch/Kconfig
>+++ b/arch/Kconfig
>@@ -133,6 +133,7 @@ config SANDBOX
> 	imply PHYLIB
> 	imply DM_MDIO
> 	imply DM_MDIO_MUX
>+	imply DM_DSA
>
> config SH
> 	bool "SuperH architecture"
>diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index
>fdb08f2111..0f565f066a 100644
>--- a/arch/sandbox/dts/test.dts
>+++ b/arch/sandbox/dts/test.dts
>@@ -40,6 +40,10 @@
> 		usb2 = &usb_2;
> 		axi0 = &axi;
> 		osd0 = "/osd";
>+		eth8 = &swp_0;
>+		eth9 = &swp_1;
>+		eth10 = &swp_2;
>+		eth11 = &dsa_eth0;
> 	};
>
> 	audio: audio-codec {
>@@ -889,6 +893,51 @@
> 	mdio: mdio-test {
> 		compatible = "sandbox,mdio";
> 	};
>+
>+	dsa_eth0: dsa-test-eth {
>+		compatible = "sandbox,dsa-eth";
>+	};
>+
>+	dsa-test {
>+		compatible = "sandbox,dsa";
>+
>+		ports {
>+			#address-cells = <1>;
>+			#size-cells = <0>;
>+			swp_0: port at 0 {
>+				reg = <0>;
>+				label = "lan0";
>+			};
>+
>+			swp_1: port at 1 {
>+				reg = <1>;
>+				label = "lan1";
>+				phy-mode = "rgmii-txid";
>+				fixed-link {
>+					speed = <1000>;
>+					full-duplex;
>+				};
>+			};
>+
>+			swp_2: port at 2 {
>+				reg = <2>;
>+				label = "lan2";
>+				fixed-link {
>+					speed = <100>;
>+					full-duplex;
>+				};
>+			};
>+
>+			port at 3 {
>+				reg = <3>;
>+				ethernet = <&dsa_eth0>;
>+				fixed-link {
>+					speed = <100>;
>+					full-duplex;
>+				};
>+			};
>+		};
>+	};
> };
>
> #include "sandbox_pmic.dtsi"
>diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h index
>1c13055cdc..35a5676eb9 100644
>--- a/include/configs/sandbox.h
>+++ b/include/configs/sandbox.h
>@@ -100,6 +100,10 @@
> 					"eth1addr=00:00:11:22:33:45\0" \
> 					"eth3addr=00:00:11:22:33:46\0" \
> 					"eth5addr=00:00:11:22:33:47\0" \
>+					"eth8addr=00:00:11:22:33:48\0" \
>+					"eth9addr=00:00:11:22:33:49\0" \
>+					"eth10addr=00:00:11:22:33:4a\0" \
>+					"eth11addr=00:00:11:22:33:4b\0" \
> 					"ipaddr=1.2.3.4\0"
>
> #define MEM_LAYOUT_ENV_SETTINGS \
>diff --git a/test/dm/Makefile b/test/dm/Makefile index 0c2fd5cb5e..69e9feed91
>100644
>--- a/test/dm/Makefile
>+++ b/test/dm/Makefile
>@@ -65,4 +65,5 @@ obj-$(CONFIG_VIRTIO_SANDBOX) += virtio.o
> obj-$(CONFIG_DMA) += dma.o
> obj-$(CONFIG_DM_MDIO) += mdio.o
> obj-$(CONFIG_DM_MDIO_MUX) += mdio_mux.o
>+obj-$(CONFIG_DM_DSA) += dsa.o
> endif
>diff --git a/test/dm/dsa.c b/test/dm/dsa.c new file mode 100644 index
>0000000000..5aa3847fe5
>--- /dev/null
>+++ b/test/dm/dsa.c
>@@ -0,0 +1,58 @@
>+// SPDX-License-Identifier: GPL-2.0
>+/*
>+ * Copyright (c) 2019 NXP
Please use Copyright 2019 NXP
>+ */
>+
>+#include <net/dsa.h>
>+#include <dm/test.h>
>+#include <test/ut.h>
>+
>+extern int dsa_sandbox_port_mask;
>+
>+/* this test sends ping requests with the local address through each
>+DSA port
>+ * via the dummy DSA master Eth.
>+ * The dummy Eth filters traffic based on DSA port used to Tx and the
>+port
>+ * mask set here, so we can check that port information gets trough correctly.
>+ */
>+static int dm_test_dsa(struct unit_test_state *uts) {
>+	dsa_sandbox_port_mask = 0x5;
>+
>+	env_set("ethrotate", "no");
>+	net_ping_ip = string_to_ip("1.2.3.4");
>+
>+	env_set("ethact", "dsa-test-eth");
>+	ut_assertok(net_loop(PING));
>+
>+	dsa_sandbox_port_mask = 0x7;
>+	env_set("ethact", "lan0");
>+	ut_assertok(net_loop(PING));
>+	env_set("ethact", "lan1");
>+	ut_assertok(net_loop(PING));
>+	env_set("ethact", "lan2");
>+	ut_assertok(net_loop(PING));
>+
>+	dsa_sandbox_port_mask = 0x1;
>+	env_set("ethact", "lan0");
>+	ut_assertok(net_loop(PING));
>+	env_set("ethact", "lan1");
>+	ut_assert(net_loop(PING) != 0);
>+	env_set("ethact", "lan2");
>+	ut_assert(net_loop(PING) != 0);
>+
>+	dsa_sandbox_port_mask = 0x6;
>+	env_set("ethact", "lan0");
>+	ut_assert(net_loop(PING) != 0);
>+	env_set("ethact", "lan1");
>+	ut_assertok(net_loop(PING));
>+	env_set("ethact", "lan2");
>+	ut_assertok(net_loop(PING));
>+
>+	dsa_sandbox_port_mask = 0;
>+	env_set("ethact", "");
>+	env_set("ethrotate", "yes");
>+
>+	return 0;
>+}
>+
>+DM_TEST(dm_test_dsa, DM_TESTF_SCAN_FDT);
>diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 1fb8b5c248..0d7dd053e2
>100644
>--- a/test/dm/test-fdt.c
>+++ b/test/dm/test-fdt.c
>@@ -224,7 +224,7 @@ static int dm_test_alias_highest_id(struct unit_test_state
>*uts)
> 	int ret;
>
> 	ret = dev_read_alias_highest_id("eth");
>-	ut_asserteq(5, ret);
>+	ut_asserteq(11, ret);
>
> 	ret = dev_read_alias_highest_id("gpio");
> 	ut_asserteq(2, ret);
>--
>2.17.1
Priyanka

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 1/6] net: introduce DSA class for Ethernet switches
  2019-12-03 14:56 ` [PATCH v3 1/6] net: introduce DSA class for Ethernet switches Alex Marginean
@ 2019-12-15 12:44   ` Vladimir Oltean
  2019-12-17  7:11     ` Alexandru Marginean
  2019-12-15 13:08   ` Vladimir Oltean
  2020-02-09 13:10   ` Vladimir Oltean
  2 siblings, 1 reply; 25+ messages in thread
From: Vladimir Oltean @ 2019-12-15 12:44 UTC (permalink / raw)
  To: u-boot

Hi Alex,

On Tue, 3 Dec 2019 at 17:32, Alex Marginean <alexandru.marginean@nxp.com> wrote:
>
> DSA stands for Distributed Switch Architecture and it covers switches that
> are connected to the CPU through an Ethernet link and generally use frame
> tags to pass information about the source/destination ports to/from CPU.
> Front panel ports are presented as regular ethernet devices in U-Boot and
> they are expected to support the typical networking commands.
> DSA switches may be cascaded, DSA class code does not currently support
> this.
>
> Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
> ---
>  drivers/net/Kconfig    |  13 ++
>  include/dm/uclass-id.h |   1 +
>  include/net/dsa.h      | 141 ++++++++++++++++
>  net/Makefile           |   1 +
>  net/dsa-uclass.c       | 369 +++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 525 insertions(+)
>  create mode 100644 include/net/dsa.h
>  create mode 100644 net/dsa-uclass.c
>
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 4182897d89..a4157cb122 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -37,6 +37,19 @@ config DM_MDIO_MUX
>           This is currently implemented in net/mdio-mux-uclass.c
>           Look in include/miiphy.h for details.
>
> +config DM_DSA
> +       bool "Enable Driver Model for DSA switches"
> +       depends on DM_ETH && DM_MDIO
> +       help
> +         Enable Driver Model for DSA switches
> +
> +         Adds UCLASS_DSA class supporting switches that follow the Distributed
> +         Switch Architecture (DSA).  These switches rely on the presence of a
> +         management switch port connected to an Ethernet controller capable of
> +         receiving frames from the switch.  This host Ethernet controller is
> +         called "master" and "cpu" in DSA terminology.
> +         This is currently implemented in net/dsa-uclass.c
> +
>  config MDIO_SANDBOX
>         depends on DM_MDIO && SANDBOX
>         default y
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 0c563d898b..8f37a91488 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -42,6 +42,7 @@ enum uclass_id {
>         UCLASS_DISPLAY,         /* Display (e.g. DisplayPort, HDMI) */
>         UCLASS_DSI_HOST,        /* Display Serial Interface host */
>         UCLASS_DMA,             /* Direct Memory Access */
> +       UCLASS_DSA,             /* Distributed (Ethernet) Switch Architecture */
>         UCLASS_EFI,             /* EFI managed devices */
>         UCLASS_ETH,             /* Ethernet device */
>         UCLASS_FIRMWARE,        /* Firmware */
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> new file mode 100644
> index 0000000000..2387419b9d
> --- /dev/null
> +++ b/include/net/dsa.h
> @@ -0,0 +1,141 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (c) 2019 NXP
> + */
> +
> +#ifndef __DSA_H__
> +#define __DSA_H__
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <phy.h>
> +
> +/**
> + * DSA stands for Distributed Switch Architecture and it is infrastructure
> + * intended to support drivers for Switches that rely on an intermediary
> + * Ethernet device for I/O.  These switches may support cascading allowing
> + * them to be arranged as a tree.
> + * DSA is documented in detail in the Linux kernel documentation under
> + * Documentation/networking/dsa/dsa.txt
> + * The network layout of such a switch is shown below:
> + *
> + *         |---------------------------
> + *         | CPU network device (eth0)|
> + *         ----------------------------
> + *         | <tag added by switch     |
> + *         |                          |
> + *         |                          |
> + *         |        tag added by CPU> |
> + *     |--------------------------------------------|
> + *     | Switch driver                              |
> + *     |--------------------------------------------|
> + *         ||        ||         ||
> + *     |-------|  |-------|  |-------|
> + *     | sw0p0 |  | sw0p1 |  | sw0p2 |
> + *     |-------|  |-------|  |-------|
> + *
> + * In U-Boot the intent is to allow access to front panel ports (shown at the
> + * bottom of the picture) though the master Ethernet port (eth0 in the picture).
> + * Front panel ports are presented as regular Ethernet devices in U-Boot and
> + * they are expected to support the typical networking commands.
> + * In general DSA switches require the use of tags, extra headers added both by
> + * software on Tx and by the switch on Rx.  These tags carry at a minimum port
> + * information and switch information for cascaded set-ups.
> + * In U-Boot these tags are inserted and parsed by the DSA switch driver, the
> + * class code helps with headroom/tailroom for the extra headers.
> + *
> + * TODO:
> + * - handle switch cascading, for now U-Boot only supports stand-alone switches.
> + * - propagate the master Eth MAC address to switch ports, this is used in
> + * Linux to avoid using additional MAC addresses on master Eth.

Any idea how this would be done?
The DSA master port needs to have the MAC address of the switch eth
device in its RX filter (eth_get_ops(dev)->write_hwaddr), or otherwise
be in promiscuous mode to receive packets destined to its MAC address.
Either that, or the switch eth devices need to inherit the MAC address
of the DSA master eth device, but that implies a strict probing order
between switch ports and the DSA master, which is not enforced
currently, and custom logic for DSA switches in eth_post_probe().
So it's not so much of a TODO as it is something to sort out from the
beginning. On the LS1028A, this is not a problem because you use a
tagging prefix that circumvents the need to put the DSA master in
promiscuous mode (6 bytes of ff:ff:ff:ff:ff:ff in the headroom).
However a more generic solution is needed.
For the LS1021A-TSN board, I need to hardcode promiscuous mode in
startup_tsec in order to get any other traffic than broadcast from the
sja1105 driver.
And by the way, if CONFIG_NET_RANDOM_ETHADDR is not set and the DSA
master port does not have a MAC address, U-Boot crashes pretty badly
for me.

> + * - Add support to probe DSA switches connected to a MDIO bus, this is needed
> + * to convert switch drivers that are now under drivers/net/phy.
> + */
> +
> +#define DSA_PORT_NAME_LENGTH   16
> +
> +/* Maximum number of ports each DSA device can have */
> +#define DSA_MAX_PORTS          12
> +/* Used to size internal buffers, no support for jumbo yet */
> +#define DSA_MAX_FRAME_SIZE     2048
> +
> +/**
> + * struct dsa_ops - DSA operations
> + *
> + * @port_enable:  Initialize a switch port for I/O
> + * @port_disable: Disable a port
> + * @xmit:         Insert the DSA tag for transmission
> + *                DSA drivers receive a copy of the packet with headroom and
> + *                tailroom reserved and set to 0.
> + *                Packet points to headroom and length is updated to include
> + *                both headroom and tailroom
> + * @rcv:          Process the DSA tag on reception
> + *                Packet and length describe the frame as received from master
> + *                including any additional headers
> + */
> +struct dsa_ops {
> +       int (*port_enable)(struct udevice *dev, int port,
> +                          struct phy_device *phy);
> +       void (*port_disable)(struct udevice *dev, int port,
> +                            struct phy_device *phy);
> +       int (*xmit)(struct udevice *dev, int port, void *packet, int length);
> +       int (*rcv)(struct udevice *dev, int *port, void *packet, int length);
> +};
> +
> +#define dsa_get_ops(dev) ((struct dsa_ops *)(dev)->driver->ops)
> +
> +/**
> + * struct dsa_port_platdata - DSA port platform data
> + *
> + * @dev :  Port u-device
> + *         Uclass code sets this field for all ports
> + * @phy:   PHY device associated with this port
> + *         Uclass code sets this field for all ports except CPU port, based on
> + *         DT information.  It may be NULL.
> + * @node:  Port DT node, if any.  Uclass code sets this field.
> + * @index: Port index in the DSA switch, set by class code.
> + * @name:  Name of the port Eth device.  If a label property is present in the
> + *         port DT node, it is used as name.  Drivers can use custom names by
> + *         populating this field, otherwise class code generates a default.
> + */
> +struct dsa_port_platdata {
> +       struct udevice *dev;
> +       struct phy_device *phy;
> +       ofnode node;
> +       int index;
> +       char name[DSA_PORT_NAME_LENGTH];
> +};
> +
> +/**
> + * struct dsa_perdev_platdata - Per-device platform data for DSA DM
> + *
> + * @num_ports:   Number of ports the device has, must be <= DSA_MAX_PORTS
> + *               All DSA drivers must set this at _bind
> + * @headroom:    Size, in bytes, of headroom needed for the DSA tag
> + *               All DSA drivers must set this at _bind or _probe
> + * @tailroom:    Size, in bytes, of tailroom needed for the DSA tag
> + *               DSA class code allocates headroom and tailroom on Tx before
> + *               calling DSA driver xmit function
> + *               All DSA drivers must set this at _bind or _probe
> + * @master_node: DT node of the master Ethernet.  DT is optional so this may be
> + *               null.
> + * @master_dev:  Ethernet device to be used as master.  Uclass code sets this
> + *               based on DT information if present, otherwise drivers must set
> + *               this field in _probe.
> + * @cpu_port:    Index of switch port linked to master Ethernet.
> + *               Uclass code sets this based on DT information if present,
> + *               otherwise drivers must set this field in _bind.
> + * @port:        per-port data
> + */
> +struct dsa_perdev_platdata {
> +       int num_ports;
> +       int headroom;
> +       int tailroom;
> +
> +       ofnode master_node;
> +       struct udevice *master_dev;
> +       int cpu_port;
> +       struct dsa_port_platdata port[DSA_MAX_PORTS];
> +};
> +
> +#endif /* __DSA_H__ */
> diff --git a/net/Makefile b/net/Makefile
> index 2a700c8401..fac8c8beb9 100644
> --- a/net/Makefile
> +++ b/net/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_CMD_SNTP) += sntp.o
>  obj-$(CONFIG_CMD_TFTPBOOT) += tftp.o
>  obj-$(CONFIG_UDP_FUNCTION_FASTBOOT)  += fastboot.o
>  obj-$(CONFIG_CMD_WOL)  += wol.o
> +obj-$(CONFIG_DM_DSA)   += dsa-uclass.o
>
>  # Disable this warning as it is triggered by:
>  # sprintf(buf, index ? "foo%d" : "foo", index)
> diff --git a/net/dsa-uclass.c b/net/dsa-uclass.c
> new file mode 100644
> index 0000000000..3790a72841
> --- /dev/null
> +++ b/net/dsa-uclass.c
> @@ -0,0 +1,369 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2019, NXP
> + */
> +
> +#include <net/dsa.h>
> +#include <dm/lists.h>
> +#include <dm/device-internal.h>
> +#include <dm/uclass-internal.h>
> +#include <miiphy.h>
> +
> +#define DSA_PORT_CHILD_DRV_NAME "dsa-port"
> +
> +/* helper that returns the DSA master Ethernet device. */
> +static struct udevice *dsa_port_get_master(struct udevice *pdev, bool probe)
> +{
> +       struct udevice *dev = dev_get_parent(pdev);
> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
> +
> +       if (probe)
> +               device_probe(platdata->master_dev);
> +
> +       return platdata->master_dev;
> +}
> +
> +/*
> + * Start the desired port, the CPU port and the master Eth interface.
> + * TODO: if cascaded we may need to _start ports in other switches too
> + */
> +static int dsa_port_start(struct udevice *pdev)
> +{
> +       struct udevice *dev = dev_get_parent(pdev);
> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
> +       struct udevice *master = dsa_port_get_master(pdev, true);
> +       struct dsa_port_platdata *ppriv = dev_get_priv(pdev);
> +       struct dsa_ops *ops = dsa_get_ops(dev);
> +       int err;
> +
> +       if (!ppriv || !platdata)
> +               return -EINVAL;
> +
> +       if (!master) {
> +               dev_err(pdev, "DSA master Ethernet device not found!\n");
> +               return -EINVAL;
> +       }
> +
> +       if (ops->port_enable) {
> +               err = ops->port_enable(dev, ppriv->index, ppriv->phy);
> +               if (err)
> +                       return err;
> +               err = ops->port_enable(dev, platdata->cpu_port,
> +                                      platdata->port[platdata->cpu_port].phy);
> +               if (err)
> +                       return err;
> +       }
> +
> +       return eth_get_ops(master)->start(master);
> +}
> +
> +/* Stop the desired port, the CPU port and the master Eth interface */
> +static void dsa_port_stop(struct udevice *pdev)
> +{
> +       struct udevice *dev = dev_get_parent(pdev);
> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
> +       struct udevice *master = dsa_port_get_master(pdev, false);
> +       struct dsa_port_platdata *ppriv = dev_get_priv(pdev);
> +       struct dsa_ops *ops = dsa_get_ops(dev);
> +
> +       if (!ppriv || !platdata)
> +               return;
> +
> +       if (ops->port_disable) {
> +               ops->port_disable(dev, ppriv->index, ppriv->phy);
> +               ops->port_disable(dev, platdata->cpu_port,
> +                                 platdata->port[platdata->cpu_port].phy);
> +       }
> +
> +       /*
> +        * stop master only if it's active, don't probe it otherwise.
> +        * Under normal usage it would be active because we're using it, but
> +        * during tear-down it may have been removed ahead of us.
> +        */
> +       if (master && device_active(master))
> +               eth_get_ops(master)->stop(master);
> +}
> +
> +/*
> + * Insert a DSA tag and call master Ethernet send on the resulting packet
> + * We copy the frame to a stack buffer where we have reserved headroom and
> + * tailroom space.  Headroom and tailroom are set to 0.
> + */
> +static int dsa_port_send(struct udevice *pdev, void *packet, int length)
> +{
> +       struct udevice *dev = dev_get_parent(pdev);
> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
> +       struct udevice *master = dsa_port_get_master(pdev, true);
> +       struct dsa_port_platdata *ppriv = dev_get_priv(pdev);
> +       struct dsa_ops *ops = dsa_get_ops(dev);
> +       uchar dsa_packet[DSA_MAX_FRAME_SIZE];
> +       int head = platdata->headroom, tail = platdata->tailroom;
> +       int err;
> +
> +       if (!master)
> +               return -EINVAL;
> +
> +       if (length + head + tail > DSA_MAX_FRAME_SIZE)
> +               return -EINVAL;
> +
> +       memset(dsa_packet, 0, head);
> +       memset(dsa_packet + head + length, 0, tail);
> +       memcpy(dsa_packet + head, packet, length);
> +       length += head + tail;
> +
> +       err = ops->xmit(dev, ppriv->index, dsa_packet, length);
> +       if (err)
> +               return err;
> +
> +       return eth_get_ops(master)->send(master, dsa_packet, length);
> +}
> +
> +/* Receive a frame from master Ethernet, process it and pass it on */
> +static int dsa_port_recv(struct udevice *pdev, int flags, uchar **packetp)
> +{
> +       struct udevice *dev = dev_get_parent(pdev);
> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
> +       struct udevice *master = dsa_port_get_master(pdev, true);
> +       struct dsa_port_platdata *ppriv = dev_get_priv(pdev);
> +       struct dsa_ops *ops = dsa_get_ops(dev);
> +       int head = platdata->headroom, tail = platdata->tailroom;
> +       int length, port_index, err;
> +
> +       if (!master)
> +               return -EINVAL;
> +
> +       length = eth_get_ops(master)->recv(master, flags, packetp);
> +       if (length <= 0)
> +               return length;
> +
> +       /*
> +        * if we receive frames from a different port or frames that DSA driver
> +        * doesn't like we discard them here.
> +        * In case of discard we return with no frame and expect to be called
> +        * again instead of looping here, so upper layer can deal with timeouts
> +        * and ctrl-c
> +        */
> +       err = ops->rcv(dev, &port_index, *packetp, length);
> +       if (err || port_index != ppriv->index || (length <= head + tail)) {
> +               if (eth_get_ops(master)->free_pkt)
> +                       eth_get_ops(master)->free_pkt(master, *packetp, length);
> +               return -EAGAIN;
> +       }
> +
> +       /*
> +        * We move the pointer over headroom here to avoid a copy.  If free_pkt
> +        * gets called we move the pointer back before calling master free_pkt.
> +        */
> +       *packetp += head;
> +
> +       return length - head - tail;
> +}
> +
> +static int dsa_port_free_pkt(struct udevice *pdev, uchar *packet, int length)
> +{
> +       struct udevice *dev = dev_get_parent(pdev);
> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
> +       struct udevice *master = dsa_port_get_master(pdev, true);
> +
> +       if (!master)
> +               return -EINVAL;
> +
> +       if (eth_get_ops(master)->free_pkt) {
> +               /* return the original pointer and length to master Eth */
> +               packet -= platdata->headroom;
> +               length += platdata->headroom - platdata->tailroom;
> +
> +               return eth_get_ops(master)->free_pkt(master, packet, length);
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct eth_ops dsa_port_ops = {
> +       .start          = dsa_port_start,
> +       .send           = dsa_port_send,
> +       .recv           = dsa_port_recv,
> +       .stop           = dsa_port_stop,
> +       .free_pkt       = dsa_port_free_pkt,
> +};
> +
> +U_BOOT_DRIVER(dsa_port) = {
> +       .name   = DSA_PORT_CHILD_DRV_NAME,
> +       .id     = UCLASS_ETH,
> +       .ops    = &dsa_port_ops,
> +       .platdata_auto_alloc_size = sizeof(struct eth_pdata),
> +};
> +
> +/*
> + * reads the DT properties of the given DSA port.
> + * If the return value is != 0 then the port is skipped
> + */
> +static int dsa_port_parse_dt(struct udevice *dev, int port_index,
> +                            ofnode ports_node, bool *is_cpu)
> +{
> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
> +       struct dsa_port_platdata *port = &platdata->port[port_index];
> +       ofnode temp_node;
> +       u32 ethernet;
> +
> +       /*
> +        * if we don't have a DT we don't do anything here but the port is
> +        * registered normally
> +        */
> +       if (!ofnode_valid(ports_node))
> +               return 0;
> +
> +       ofnode_for_each_subnode(temp_node, ports_node) {
> +               const char *port_label;
> +               u32 reg;
> +
> +               if (ofnode_read_u32(temp_node, "reg", &reg) ||
> +                   reg != port_index)
> +                       continue;
> +
> +               /* if the port is explicitly disabled in DT skip it */
> +               if (!ofnode_is_available(temp_node))
> +                       return -ENODEV;
> +
> +               port->node = temp_node;
> +
> +               dev_dbg(dev, "port %d node %s\n", port->index,
> +                       ofnode_get_name(port->node));
> +
> +               /* Use 'label' if present in DT */
> +               port_label = ofnode_read_string(port->node, "label");
> +               if (port_label)
> +                       strncpy(port->name, port_label, DSA_PORT_NAME_LENGTH);
> +
> +               *is_cpu = !ofnode_read_u32(port->node, "ethernet",
> +                                          &ethernet);
> +
> +               if (*is_cpu) {
> +                       platdata->master_node =
> +                               ofnode_get_by_phandle(ethernet);
> +                       platdata->cpu_port = port_index;
> +
> +                       dev_dbg(dev, "master node %s on port %d\n",
> +                               ofnode_get_name(platdata->master_node),
> +                               port_index);
> +               }
> +               break;
> +       }
> +
> +       return 0;
> +}
> +
> +/**
> + * This function mostly deals with pulling information out of the device tree
> + * into the platdata structure.
> + * It goes through the list of switch ports, registers an Eth device for each
> + * front panel port and identifies the cpu port connected to master Eth device.
> + * TODO: support cascaded switches
> + */
> +static int dm_dsa_post_bind(struct udevice *dev)
> +{
> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
> +       ofnode ports_node = ofnode_null();
> +       int first_err = 0, err = 0, i;
> +
> +       if (!platdata) {
> +               dev_err(dev, "missing plaform data\n");
> +               return -EINVAL;
> +       }
> +
> +       if (platdata->num_ports <= 0 || platdata->num_ports > DSA_MAX_PORTS) {
> +               dev_err(dev, "unexpected num_ports value (%d)\n",
> +                       platdata->num_ports);
> +               return -EINVAL;
> +       }
> +
> +       platdata->master_node = ofnode_null();
> +
> +       if (!ofnode_valid(dev->node)) {
> +               dev_dbg(dev, "Device doesn't have a valid DT node!\n");
> +       } else {
> +               ports_node = ofnode_find_subnode(dev->node, "ports");
> +               if (!ofnode_valid(ports_node))
> +                       dev_dbg(dev,
> +                               "ports node is missing under DSA device!\n");
> +       }
> +
> +       for (i = 0; i < platdata->num_ports; i++) {
> +               struct dsa_port_platdata *port = &platdata->port[i];
> +               bool skip_port, is_cpu = false;
> +
> +               port->index = i;
> +
> +               /*
> +                * If the driver set up port names in _bind use those, otherwise
> +                * use default ones.
> +                * If present, DT label is used as name and overrides anything
> +                * we may have here.
> +                */
> +               if (!strlen(port->name))
> +                       snprintf(port->name, DSA_PORT_NAME_LENGTH, "%s@%d",
> +                                dev->name, i);
> +
> +               skip_port = !!dsa_port_parse_dt(dev, i, ports_node, &is_cpu);
> +
> +               /*
> +                * if this is the CPU port don't register it as an ETH device,
> +                * we skip it on purpose since I/O to/from it from the CPU
> +                * isn't useful
> +                * TODO: cpu port may have a PHY and we don't handle that yet.
> +                */
> +               if (is_cpu || skip_port)
> +                       continue;
> +
> +               err = device_bind_driver_to_node(dev, DSA_PORT_CHILD_DRV_NAME,
> +                                                port->name, port->node,
> +                                                &port->dev);
> +
> +               /* try to bind all ports but keep 1st error */
> +               if (err && !first_err)
> +                       first_err = err;
> +       }
> +
> +       if (!ofnode_valid(platdata->master_node))
> +               dev_dbg(dev, "DSA master Eth device is missing!\n");
> +
> +       return first_err;
> +}
> +
> +/**
> + * This function deals with additional devices around the switch as these should
> + * have been bound to drivers by now.
> + * TODO: pick up references to other switch devices here, if we're cascaded.
> + */
> +static int dm_dsa_pre_probe(struct udevice *dev)
> +{
> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
> +       int i;
> +
> +       if (!platdata)
> +               return -EINVAL;
> +
> +       if (ofnode_valid(platdata->master_node))
> +               uclass_find_device_by_ofnode(UCLASS_ETH, platdata->master_node,
> +                                            &platdata->master_dev);
> +
> +       for (i = 0; i < platdata->num_ports; i++) {
> +               struct dsa_port_platdata *port = &platdata->port[i];
> +
> +               if (port->dev) {
> +                       port->dev->priv = port;
> +                       port->phy = dm_eth_phy_connect(port->dev);
> +               }
> +       }
> +
> +       return 0;
> +}
> +
> +UCLASS_DRIVER(dsa) = {
> +       .id = UCLASS_DSA,
> +       .name = "dsa",
> +       .post_bind  = dm_dsa_post_bind,
> +       .pre_probe = dm_dsa_pre_probe,
> +       .per_device_platdata_auto_alloc_size =
> +                       sizeof(struct dsa_perdev_platdata),
> +};
> --
> 2.17.1
>

Thanks,
-Vladimir

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 4/6] drivers: net: add Felix DSA switch driver
  2019-12-03 14:56 ` [PATCH v3 4/6] drivers: net: add Felix DSA switch driver Alex Marginean
@ 2019-12-15 12:53   ` Vladimir Oltean
  2019-12-15 12:53     ` Vladimir Oltean
                       ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Vladimir Oltean @ 2019-12-15 12:53 UTC (permalink / raw)
  To: u-boot

Hi Alex,

On Tue, 3 Dec 2019 at 18:18, Alex Marginean <alexandru.marginean@nxp.com> wrote:
> +static int felix_port_enable(struct udevice *dev, int port,
> +                            struct phy_device *phy)
> +{
> +       struct felix_priv *priv = dev_get_priv(dev);
> +       void *base = priv->regs_base;
> +
> +       out_le32(base + FELIX_GMII_MAC_ENA_CFG(port),
> +                FELIX_GMII_MAX_ENA_CFG_TX | FELIX_GMII_MAX_ENA_CFG_RX);
> +
> +       out_le32(base + FELIX_QSYS_SYSTEM_SW_PORT_MODE(port),
> +                FELIX_QSYS_SYSTEM_SW_PORT_ENA |
> +                FELIX_QSYS_SYSTEM_SW_PORT_LOSSY |
> +                FELIX_QSYS_SYSTEM_SW_PORT_SCH(1));
> +
> +       if (phy)
> +               phy_startup(phy);
> +       return 0;
> +}
> +
> +static void felix_port_disable(struct udevice *dev, int port,
> +                              struct phy_device *phy)
> +{
> +       struct felix_priv *priv = dev_get_priv(dev);
> +       void *base = priv->regs_base;
> +
> +       out_le32(base + FELIX_GMII_MAC_ENA_CFG(port), 0);
> +
> +       out_le32(base + FELIX_QSYS_SYSTEM_SW_PORT_MODE(port),
> +                FELIX_QSYS_SYSTEM_SW_PORT_LOSSY |
> +                FELIX_QSYS_SYSTEM_SW_PORT_SCH(1));
> +
> +       /*
> +        * we don't call phy_shutdown here to avoind waiting next time we use
> +        * the port, but the downside is that remote side will think we're
> +        * actively processing traffic although we are not.
> +        */
> +}
> --
> 2.17.1
>

What is the correct general procedure here, is it to call phy_startup
so late (felix_port_enable)? I'm trying to take this driver as an
example for sja1105, which has RGMII so PCS and no autonomous in-band
AN like felix does. On this switch, it is too late to do phy_startup
now. Instead, I would need to look at phy->speed which should have
been settled by now, and reprogram my MAC with it.
My question is: don't you think phy_startup() and phy_shutdown()
belong in the DSA uclass code?

Thanks,
-Vladimir

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 4/6] drivers: net: add Felix DSA switch driver
  2019-12-15 12:53   ` Vladimir Oltean
@ 2019-12-15 12:53     ` Vladimir Oltean
  2019-12-15 14:16     ` Vladimir Oltean
  2019-12-17  7:28     ` Alexandru Marginean
  2 siblings, 0 replies; 25+ messages in thread
From: Vladimir Oltean @ 2019-12-15 12:53 UTC (permalink / raw)
  To: u-boot

On Sun, 15 Dec 2019 at 14:53, Vladimir Oltean <olteanv@gmail.com> wrote:
>
> Hi Alex,
>
> On Tue, 3 Dec 2019 at 18:18, Alex Marginean <alexandru.marginean@nxp.com> wrote:
> > +static int felix_port_enable(struct udevice *dev, int port,
> > +                            struct phy_device *phy)
> > +{
> > +       struct felix_priv *priv = dev_get_priv(dev);
> > +       void *base = priv->regs_base;
> > +
> > +       out_le32(base + FELIX_GMII_MAC_ENA_CFG(port),
> > +                FELIX_GMII_MAX_ENA_CFG_TX | FELIX_GMII_MAX_ENA_CFG_RX);
> > +
> > +       out_le32(base + FELIX_QSYS_SYSTEM_SW_PORT_MODE(port),
> > +                FELIX_QSYS_SYSTEM_SW_PORT_ENA |
> > +                FELIX_QSYS_SYSTEM_SW_PORT_LOSSY |
> > +                FELIX_QSYS_SYSTEM_SW_PORT_SCH(1));
> > +
> > +       if (phy)
> > +               phy_startup(phy);
> > +       return 0;
> > +}
> > +
> > +static void felix_port_disable(struct udevice *dev, int port,
> > +                              struct phy_device *phy)
> > +{
> > +       struct felix_priv *priv = dev_get_priv(dev);
> > +       void *base = priv->regs_base;
> > +
> > +       out_le32(base + FELIX_GMII_MAC_ENA_CFG(port), 0);
> > +
> > +       out_le32(base + FELIX_QSYS_SYSTEM_SW_PORT_MODE(port),
> > +                FELIX_QSYS_SYSTEM_SW_PORT_LOSSY |
> > +                FELIX_QSYS_SYSTEM_SW_PORT_SCH(1));
> > +
> > +       /*
> > +        * we don't call phy_shutdown here to avoind waiting next time we use
> > +        * the port, but the downside is that remote side will think we're
> > +        * actively processing traffic although we are not.
> > +        */
> > +}
> > --
> > 2.17.1
> >
>
> What is the correct general procedure here, is it to call phy_startup
> so late (felix_port_enable)? I'm trying to take this driver as an
> example for sja1105, which has RGMII so PCS and no autonomous in-band

RGMII, so no* PCS

> AN like felix does. On this switch, it is too late to do phy_startup
> now. Instead, I would need to look at phy->speed which should have
> been settled by now, and reprogram my MAC with it.
> My question is: don't you think phy_startup() and phy_shutdown()
> belong in the DSA uclass code?
>
> Thanks,
> -Vladimir

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 1/6] net: introduce DSA class for Ethernet switches
  2019-12-03 14:56 ` [PATCH v3 1/6] net: introduce DSA class for Ethernet switches Alex Marginean
  2019-12-15 12:44   ` Vladimir Oltean
@ 2019-12-15 13:08   ` Vladimir Oltean
  2019-12-17  7:18     ` Alexandru Marginean
  2020-02-09 13:10   ` Vladimir Oltean
  2 siblings, 1 reply; 25+ messages in thread
From: Vladimir Oltean @ 2019-12-15 13:08 UTC (permalink / raw)
  To: u-boot

On Tue, 3 Dec 2019 at 17:32, Alex Marginean <alexandru.marginean@nxp.com> wrote:
> +/**
> + * This function deals with additional devices around the switch as these should
> + * have been bound to drivers by now.
> + * TODO: pick up references to other switch devices here, if we're cascaded.
> + */
> +static int dm_dsa_pre_probe(struct udevice *dev)
> +{
> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
> +       int i;
> +
> +       if (!platdata)
> +               return -EINVAL;
> +
> +       if (ofnode_valid(platdata->master_node))
> +               uclass_find_device_by_ofnode(UCLASS_ETH, platdata->master_node,
> +                                            &platdata->master_dev);
> +
> +       for (i = 0; i < platdata->num_ports; i++) {
> +               struct dsa_port_platdata *port = &platdata->port[i];
> +
> +               if (port->dev) {
> +                       port->dev->priv = port;
> +                       port->phy = dm_eth_phy_connect(port->dev);

Fixed-link interfaces don't work with DM_MDIO. That is somewhat
natural as there is no MDIO bus for a fixed-link. However the legacy
phy_connect function can be made rather easily to work with
fixed-link, since it has the necessary code for dealing with it
already. I am not, however, sure how it ever worked in the absence of
an MDIO bus.

commit 1b7e23cc7e6d0dc3fe7ae9c55390b40717ff3c3a
Author: Vladimir Oltean <olteanv@gmail.com>
Date:   Sat Dec 14 23:25:40 2019 +0200

    phy: make phy_connect_fixed work with a null mdio bus

    It is utterly pointless to require an MDIO bus pointer for a fixed PHY
    device. The fixed.c implementation does not require it, only
    phy_device_create. Fix that.

    Signed-off-by: Vladimir Oltean <olteanv@gmail.com>

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 80a7664e4978..8ea5c9005291 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -658,7 +658,7 @@ static struct phy_device *phy_device_create(struct
mii_dev *bus, int addr,
        dev = malloc(sizeof(*dev));
        if (!dev) {
                printf("Failed to allocate PHY device for %s:%d\n",
-                      bus->name, addr);
+                      bus ? bus->name : "(null bus)", addr);
                return NULL;
        }

@@ -686,7 +686,7 @@ static struct phy_device *phy_device_create(struct
mii_dev *bus, int addr,
                return NULL;
        }

-       if (addr >= 0 && addr < PHY_MAX_ADDR)
+       if (addr >= 0 && addr < PHY_MAX_ADDR && phy_id != PHY_FIXED_ID)
                bus->phymap[addr] = dev;

        return dev;

With the patch above in place, fixed-link can also be made to work
with some logic similar to what can be seen below:

    if (ofnode_valid(ofnode_find_subnode(port->dev->node, "fixed-link")))
        port->phy = phy_connect(NULL, 0, port->dev, phy_mode); //
phy_mode needs to be pre-parsed somewhere else as well
    else
        port->phy = dm_eth_phy_connect(port->dev);

How would you see fixed-link interfaces being treated? My question so
far is in the context of front-panel ports but I am interested in your
view of the CPU port situation as well.

> +               }
> +       }
> +
> +       return 0;
> +}

Thanks,
-Vladimir

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3 4/6] drivers: net: add Felix DSA switch driver
  2019-12-15 12:53   ` Vladimir Oltean
  2019-12-15 12:53     ` Vladimir Oltean
@ 2019-12-15 14:16     ` Vladimir Oltean
  2019-12-17  7:29       ` Alexandru Marginean
  2019-12-17  7:28     ` Alexandru Marginean
  2 siblings, 1 reply; 25+ messages in thread
From: Vladimir Oltean @ 2019-12-15 14:16 UTC (permalink / raw)
  To: u-boot

On Sun, 15 Dec 2019 at 14:53, Vladimir Oltean <olteanv@gmail.com> wrote:
>
> Hi Alex,
>
> On Tue, 3 Dec 2019 at 18:18, Alex Marginean <alexandru.marginean@nxp.com> wrote:
> > +static int felix_port_enable(struct udevice *dev, int port,
> > +                            struct phy_device *phy)
> > +{
> > +       struct felix_priv *priv = dev_get_priv(dev);
> > +       void *base = priv->regs_base;
> > +
> > +       out_le32(base + FELIX_GMII_MAC_ENA_CFG(port),
> > +                FELIX_GMII_MAX_ENA_CFG_TX | FELIX_GMII_MAX_ENA_CFG_RX);
> > +
> > +       out_le32(base + FELIX_QSYS_SYSTEM_SW_PORT_MODE(port),
> > +                FELIX_QSYS_SYSTEM_SW_PORT_ENA |
> > +                FELIX_QSYS_SYSTEM_SW_PORT_LOSSY |
> > +                FELIX_QSYS_SYSTEM_SW_PORT_SCH(1));
> > +
> > +       if (phy)
> > +               phy_startup(phy);
> > +       return 0;
> > +}
> > +
> > +static void felix_port_disable(struct udevice *dev, int port,
> > +                              struct phy_device *phy)
> > +{
> > +       struct felix_priv *priv = dev_get_priv(dev);
> > +       void *base = priv->regs_base;
> > +
> > +       out_le32(base + FELIX_GMII_MAC_ENA_CFG(port), 0);
> > +
> > +       out_le32(base + FELIX_QSYS_SYSTEM_SW_PORT_MODE(port),
> > +                FELIX_QSYS_SYSTEM_SW_PORT_LOSSY |
> > +                FELIX_QSYS_SYSTEM_SW_PORT_SCH(1));
> > +
> > +       /*
> > +        * we don't call phy_shutdown here to avoind waiting next time we use
> > +        * the port, but the downside is that remote side will think we're
> > +        * actively processing traffic although we are not.
> > +        */
> > +}
> > --
> > 2.17.1
> >
>
> What is the correct general procedure here, is it to call phy_startup
> so late (felix_port_enable)? I'm trying to take this driver as an
> example for sja1105, which has RGMII so PCS and no autonomous in-band
> AN like felix does. On this switch, it is too late to do phy_startup
> now. Instead, I would need to look at phy->speed which should have
> been settled by now, and reprogram my MAC with it.
> My question is: don't you think phy_startup() and phy_shutdown()
> belong in the DSA uclass code?
>

My bad, phy_startup() is synchronous and waits for PHY AN to complete.
So this is fine.

> Thanks,
> -Vladimir

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 1/6] net: introduce DSA class for Ethernet switches
  2019-12-15 12:44   ` Vladimir Oltean
@ 2019-12-17  7:11     ` Alexandru Marginean
  0 siblings, 0 replies; 25+ messages in thread
From: Alexandru Marginean @ 2019-12-17  7:11 UTC (permalink / raw)
  To: u-boot

Hi Vladimir,

On 12/15/2019 1:44 PM, Vladimir Oltean wrote:
> Hi Alex,
> 
> On Tue, 3 Dec 2019 at 17:32, Alex Marginean <alexandru.marginean@nxp.com> wrote:
>>
>> DSA stands for Distributed Switch Architecture and it covers switches that
>> are connected to the CPU through an Ethernet link and generally use frame
>> tags to pass information about the source/destination ports to/from CPU.
>> Front panel ports are presented as regular ethernet devices in U-Boot and
>> they are expected to support the typical networking commands.
>> DSA switches may be cascaded, DSA class code does not currently support
>> this.
>>
>> Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
>> ---
>>   drivers/net/Kconfig    |  13 ++
>>   include/dm/uclass-id.h |   1 +
>>   include/net/dsa.h      | 141 ++++++++++++++++
>>   net/Makefile           |   1 +
>>   net/dsa-uclass.c       | 369 +++++++++++++++++++++++++++++++++++++++++
>>   5 files changed, 525 insertions(+)
>>   create mode 100644 include/net/dsa.h
>>   create mode 100644 net/dsa-uclass.c
>>
>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
>> index 4182897d89..a4157cb122 100644
>> --- a/drivers/net/Kconfig
>> +++ b/drivers/net/Kconfig
>> @@ -37,6 +37,19 @@ config DM_MDIO_MUX
>>            This is currently implemented in net/mdio-mux-uclass.c
>>            Look in include/miiphy.h for details.
>>
>> +config DM_DSA
>> +       bool "Enable Driver Model for DSA switches"
>> +       depends on DM_ETH && DM_MDIO
>> +       help
>> +         Enable Driver Model for DSA switches
>> +
>> +         Adds UCLASS_DSA class supporting switches that follow the Distributed
>> +         Switch Architecture (DSA).  These switches rely on the presence of a
>> +         management switch port connected to an Ethernet controller capable of
>> +         receiving frames from the switch.  This host Ethernet controller is
>> +         called "master" and "cpu" in DSA terminology.
>> +         This is currently implemented in net/dsa-uclass.c
>> +
>>   config MDIO_SANDBOX
>>          depends on DM_MDIO && SANDBOX
>>          default y
>> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
>> index 0c563d898b..8f37a91488 100644
>> --- a/include/dm/uclass-id.h
>> +++ b/include/dm/uclass-id.h
>> @@ -42,6 +42,7 @@ enum uclass_id {
>>          UCLASS_DISPLAY,         /* Display (e.g. DisplayPort, HDMI) */
>>          UCLASS_DSI_HOST,        /* Display Serial Interface host */
>>          UCLASS_DMA,             /* Direct Memory Access */
>> +       UCLASS_DSA,             /* Distributed (Ethernet) Switch Architecture */
>>          UCLASS_EFI,             /* EFI managed devices */
>>          UCLASS_ETH,             /* Ethernet device */
>>          UCLASS_FIRMWARE,        /* Firmware */
>> diff --git a/include/net/dsa.h b/include/net/dsa.h
>> new file mode 100644
>> index 0000000000..2387419b9d
>> --- /dev/null
>> +++ b/include/net/dsa.h
>> @@ -0,0 +1,141 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +/*
>> + * Copyright (c) 2019 NXP
>> + */
>> +
>> +#ifndef __DSA_H__
>> +#define __DSA_H__
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <phy.h>
>> +
>> +/**
>> + * DSA stands for Distributed Switch Architecture and it is infrastructure
>> + * intended to support drivers for Switches that rely on an intermediary
>> + * Ethernet device for I/O.  These switches may support cascading allowing
>> + * them to be arranged as a tree.
>> + * DSA is documented in detail in the Linux kernel documentation under
>> + * Documentation/networking/dsa/dsa.txt
>> + * The network layout of such a switch is shown below:
>> + *
>> + *         |---------------------------
>> + *         | CPU network device (eth0)|
>> + *         ----------------------------
>> + *         | <tag added by switch     |
>> + *         |                          |
>> + *         |                          |
>> + *         |        tag added by CPU> |
>> + *     |--------------------------------------------|
>> + *     | Switch driver                              |
>> + *     |--------------------------------------------|
>> + *         ||        ||         ||
>> + *     |-------|  |-------|  |-------|
>> + *     | sw0p0 |  | sw0p1 |  | sw0p2 |
>> + *     |-------|  |-------|  |-------|
>> + *
>> + * In U-Boot the intent is to allow access to front panel ports (shown at the
>> + * bottom of the picture) though the master Ethernet port (eth0 in the picture).
>> + * Front panel ports are presented as regular Ethernet devices in U-Boot and
>> + * they are expected to support the typical networking commands.
>> + * In general DSA switches require the use of tags, extra headers added both by
>> + * software on Tx and by the switch on Rx.  These tags carry at a minimum port
>> + * information and switch information for cascaded set-ups.
>> + * In U-Boot these tags are inserted and parsed by the DSA switch driver, the
>> + * class code helps with headroom/tailroom for the extra headers.
>> + *
>> + * TODO:
>> + * - handle switch cascading, for now U-Boot only supports stand-alone switches.
>> + * - propagate the master Eth MAC address to switch ports, this is used in
>> + * Linux to avoid using additional MAC addresses on master Eth.
> 
> Any idea how this would be done?
> The DSA master port needs to have the MAC address of the switch eth
> device in its RX filter (eth_get_ops(dev)->write_hwaddr), or otherwise
> be in promiscuous mode to receive packets destined to its MAC address.
> Either that, or the switch eth devices need to inherit the MAC address
> of the DSA master eth device, but that implies a strict probing order
> between switch ports and the DSA master, which is not enforced
> currently, and custom logic for DSA switches in eth_post_probe().
> So it's not so much of a TODO as it is something to sort out from the
> beginning. On the LS1028A, this is not a problem because you use a
> tagging prefix that circumvents the need to put the DSA master in
> promiscuous mode (6 bytes of ff:ff:ff:ff:ff:ff in the headroom).
> However a more generic solution is needed.
> For the LS1021A-TSN board, I need to hardcode promiscuous mode in
> startup_tsec in order to get any other traffic than broadcast from the
> sja1105 driver.
> And by the way, if CONFIG_NET_RANDOM_ETHADDR is not set and the DSA
> master port does not have a MAC address, U-Boot crashes pretty badly
> for me.

For DSA ports in particular U-Boot shouldn't need to pass MAC addresses 
to Linux.
I'm thinking to add some sort of flag to ETH devices to skip the MAC 
address set-up part in eth_initialize, this would be used for the switch 
ports as they don't need their own private addresses.  The MAC address 
can be resolved for them later in dsa uclass code when the ports are 
used and by that time master Eth should also be probed.

Alex

>> + * - Add support to probe DSA switches connected to a MDIO bus, this is needed
>> + * to convert switch drivers that are now under drivers/net/phy.
>> + */
>> +
>> +#define DSA_PORT_NAME_LENGTH   16
>> +
>> +/* Maximum number of ports each DSA device can have */
>> +#define DSA_MAX_PORTS          12
>> +/* Used to size internal buffers, no support for jumbo yet */
>> +#define DSA_MAX_FRAME_SIZE     2048
>> +
>> +/**
>> + * struct dsa_ops - DSA operations
>> + *
>> + * @port_enable:  Initialize a switch port for I/O
>> + * @port_disable: Disable a port
>> + * @xmit:         Insert the DSA tag for transmission
>> + *                DSA drivers receive a copy of the packet with headroom and
>> + *                tailroom reserved and set to 0.
>> + *                Packet points to headroom and length is updated to include
>> + *                both headroom and tailroom
>> + * @rcv:          Process the DSA tag on reception
>> + *                Packet and length describe the frame as received from master
>> + *                including any additional headers
>> + */
>> +struct dsa_ops {
>> +       int (*port_enable)(struct udevice *dev, int port,
>> +                          struct phy_device *phy);
>> +       void (*port_disable)(struct udevice *dev, int port,
>> +                            struct phy_device *phy);
>> +       int (*xmit)(struct udevice *dev, int port, void *packet, int length);
>> +       int (*rcv)(struct udevice *dev, int *port, void *packet, int length);
>> +};
>> +
>> +#define dsa_get_ops(dev) ((struct dsa_ops *)(dev)->driver->ops)
>> +
>> +/**
>> + * struct dsa_port_platdata - DSA port platform data
>> + *
>> + * @dev :  Port u-device
>> + *         Uclass code sets this field for all ports
>> + * @phy:   PHY device associated with this port
>> + *         Uclass code sets this field for all ports except CPU port, based on
>> + *         DT information.  It may be NULL.
>> + * @node:  Port DT node, if any.  Uclass code sets this field.
>> + * @index: Port index in the DSA switch, set by class code.
>> + * @name:  Name of the port Eth device.  If a label property is present in the
>> + *         port DT node, it is used as name.  Drivers can use custom names by
>> + *         populating this field, otherwise class code generates a default.
>> + */
>> +struct dsa_port_platdata {
>> +       struct udevice *dev;
>> +       struct phy_device *phy;
>> +       ofnode node;
>> +       int index;
>> +       char name[DSA_PORT_NAME_LENGTH];
>> +};
>> +
>> +/**
>> + * struct dsa_perdev_platdata - Per-device platform data for DSA DM
>> + *
>> + * @num_ports:   Number of ports the device has, must be <= DSA_MAX_PORTS
>> + *               All DSA drivers must set this at _bind
>> + * @headroom:    Size, in bytes, of headroom needed for the DSA tag
>> + *               All DSA drivers must set this at _bind or _probe
>> + * @tailroom:    Size, in bytes, of tailroom needed for the DSA tag
>> + *               DSA class code allocates headroom and tailroom on Tx before
>> + *               calling DSA driver xmit function
>> + *               All DSA drivers must set this at _bind or _probe
>> + * @master_node: DT node of the master Ethernet.  DT is optional so this may be
>> + *               null.
>> + * @master_dev:  Ethernet device to be used as master.  Uclass code sets this
>> + *               based on DT information if present, otherwise drivers must set
>> + *               this field in _probe.
>> + * @cpu_port:    Index of switch port linked to master Ethernet.
>> + *               Uclass code sets this based on DT information if present,
>> + *               otherwise drivers must set this field in _bind.
>> + * @port:        per-port data
>> + */
>> +struct dsa_perdev_platdata {
>> +       int num_ports;
>> +       int headroom;
>> +       int tailroom;
>> +
>> +       ofnode master_node;
>> +       struct udevice *master_dev;
>> +       int cpu_port;
>> +       struct dsa_port_platdata port[DSA_MAX_PORTS];
>> +};
>> +
>> +#endif /* __DSA_H__ */
>> diff --git a/net/Makefile b/net/Makefile
>> index 2a700c8401..fac8c8beb9 100644
>> --- a/net/Makefile
>> +++ b/net/Makefile
>> @@ -28,6 +28,7 @@ obj-$(CONFIG_CMD_SNTP) += sntp.o
>>   obj-$(CONFIG_CMD_TFTPBOOT) += tftp.o
>>   obj-$(CONFIG_UDP_FUNCTION_FASTBOOT)  += fastboot.o
>>   obj-$(CONFIG_CMD_WOL)  += wol.o
>> +obj-$(CONFIG_DM_DSA)   += dsa-uclass.o
>>
>>   # Disable this warning as it is triggered by:
>>   # sprintf(buf, index ? "foo%d" : "foo", index)
>> diff --git a/net/dsa-uclass.c b/net/dsa-uclass.c
>> new file mode 100644
>> index 0000000000..3790a72841
>> --- /dev/null
>> +++ b/net/dsa-uclass.c
>> @@ -0,0 +1,369 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * (C) Copyright 2019, NXP
>> + */
>> +
>> +#include <net/dsa.h>
>> +#include <dm/lists.h>
>> +#include <dm/device-internal.h>
>> +#include <dm/uclass-internal.h>
>> +#include <miiphy.h>
>> +
>> +#define DSA_PORT_CHILD_DRV_NAME "dsa-port"
>> +
>> +/* helper that returns the DSA master Ethernet device. */
>> +static struct udevice *dsa_port_get_master(struct udevice *pdev, bool probe)
>> +{
>> +       struct udevice *dev = dev_get_parent(pdev);
>> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
>> +
>> +       if (probe)
>> +               device_probe(platdata->master_dev);
>> +
>> +       return platdata->master_dev;
>> +}
>> +
>> +/*
>> + * Start the desired port, the CPU port and the master Eth interface.
>> + * TODO: if cascaded we may need to _start ports in other switches too
>> + */
>> +static int dsa_port_start(struct udevice *pdev)
>> +{
>> +       struct udevice *dev = dev_get_parent(pdev);
>> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
>> +       struct udevice *master = dsa_port_get_master(pdev, true);
>> +       struct dsa_port_platdata *ppriv = dev_get_priv(pdev);
>> +       struct dsa_ops *ops = dsa_get_ops(dev);
>> +       int err;
>> +
>> +       if (!ppriv || !platdata)
>> +               return -EINVAL;
>> +
>> +       if (!master) {
>> +               dev_err(pdev, "DSA master Ethernet device not found!\n");
>> +               return -EINVAL;
>> +       }
>> +
>> +       if (ops->port_enable) {
>> +               err = ops->port_enable(dev, ppriv->index, ppriv->phy);
>> +               if (err)
>> +                       return err;
>> +               err = ops->port_enable(dev, platdata->cpu_port,
>> +                                      platdata->port[platdata->cpu_port].phy);
>> +               if (err)
>> +                       return err;
>> +       }
>> +
>> +       return eth_get_ops(master)->start(master);
>> +}
>> +
>> +/* Stop the desired port, the CPU port and the master Eth interface */
>> +static void dsa_port_stop(struct udevice *pdev)
>> +{
>> +       struct udevice *dev = dev_get_parent(pdev);
>> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
>> +       struct udevice *master = dsa_port_get_master(pdev, false);
>> +       struct dsa_port_platdata *ppriv = dev_get_priv(pdev);
>> +       struct dsa_ops *ops = dsa_get_ops(dev);
>> +
>> +       if (!ppriv || !platdata)
>> +               return;
>> +
>> +       if (ops->port_disable) {
>> +               ops->port_disable(dev, ppriv->index, ppriv->phy);
>> +               ops->port_disable(dev, platdata->cpu_port,
>> +                                 platdata->port[platdata->cpu_port].phy);
>> +       }
>> +
>> +       /*
>> +        * stop master only if it's active, don't probe it otherwise.
>> +        * Under normal usage it would be active because we're using it, but
>> +        * during tear-down it may have been removed ahead of us.
>> +        */
>> +       if (master && device_active(master))
>> +               eth_get_ops(master)->stop(master);
>> +}
>> +
>> +/*
>> + * Insert a DSA tag and call master Ethernet send on the resulting packet
>> + * We copy the frame to a stack buffer where we have reserved headroom and
>> + * tailroom space.  Headroom and tailroom are set to 0.
>> + */
>> +static int dsa_port_send(struct udevice *pdev, void *packet, int length)
>> +{
>> +       struct udevice *dev = dev_get_parent(pdev);
>> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
>> +       struct udevice *master = dsa_port_get_master(pdev, true);
>> +       struct dsa_port_platdata *ppriv = dev_get_priv(pdev);
>> +       struct dsa_ops *ops = dsa_get_ops(dev);
>> +       uchar dsa_packet[DSA_MAX_FRAME_SIZE];
>> +       int head = platdata->headroom, tail = platdata->tailroom;
>> +       int err;
>> +
>> +       if (!master)
>> +               return -EINVAL;
>> +
>> +       if (length + head + tail > DSA_MAX_FRAME_SIZE)
>> +               return -EINVAL;
>> +
>> +       memset(dsa_packet, 0, head);
>> +       memset(dsa_packet + head + length, 0, tail);
>> +       memcpy(dsa_packet + head, packet, length);
>> +       length += head + tail;
>> +
>> +       err = ops->xmit(dev, ppriv->index, dsa_packet, length);
>> +       if (err)
>> +               return err;
>> +
>> +       return eth_get_ops(master)->send(master, dsa_packet, length);
>> +}
>> +
>> +/* Receive a frame from master Ethernet, process it and pass it on */
>> +static int dsa_port_recv(struct udevice *pdev, int flags, uchar **packetp)
>> +{
>> +       struct udevice *dev = dev_get_parent(pdev);
>> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
>> +       struct udevice *master = dsa_port_get_master(pdev, true);
>> +       struct dsa_port_platdata *ppriv = dev_get_priv(pdev);
>> +       struct dsa_ops *ops = dsa_get_ops(dev);
>> +       int head = platdata->headroom, tail = platdata->tailroom;
>> +       int length, port_index, err;
>> +
>> +       if (!master)
>> +               return -EINVAL;
>> +
>> +       length = eth_get_ops(master)->recv(master, flags, packetp);
>> +       if (length <= 0)
>> +               return length;
>> +
>> +       /*
>> +        * if we receive frames from a different port or frames that DSA driver
>> +        * doesn't like we discard them here.
>> +        * In case of discard we return with no frame and expect to be called
>> +        * again instead of looping here, so upper layer can deal with timeouts
>> +        * and ctrl-c
>> +        */
>> +       err = ops->rcv(dev, &port_index, *packetp, length);
>> +       if (err || port_index != ppriv->index || (length <= head + tail)) {
>> +               if (eth_get_ops(master)->free_pkt)
>> +                       eth_get_ops(master)->free_pkt(master, *packetp, length);
>> +               return -EAGAIN;
>> +       }
>> +
>> +       /*
>> +        * We move the pointer over headroom here to avoid a copy.  If free_pkt
>> +        * gets called we move the pointer back before calling master free_pkt.
>> +        */
>> +       *packetp += head;
>> +
>> +       return length - head - tail;
>> +}
>> +
>> +static int dsa_port_free_pkt(struct udevice *pdev, uchar *packet, int length)
>> +{
>> +       struct udevice *dev = dev_get_parent(pdev);
>> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
>> +       struct udevice *master = dsa_port_get_master(pdev, true);
>> +
>> +       if (!master)
>> +               return -EINVAL;
>> +
>> +       if (eth_get_ops(master)->free_pkt) {
>> +               /* return the original pointer and length to master Eth */
>> +               packet -= platdata->headroom;
>> +               length += platdata->headroom - platdata->tailroom;
>> +
>> +               return eth_get_ops(master)->free_pkt(master, packet, length);
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct eth_ops dsa_port_ops = {
>> +       .start          = dsa_port_start,
>> +       .send           = dsa_port_send,
>> +       .recv           = dsa_port_recv,
>> +       .stop           = dsa_port_stop,
>> +       .free_pkt       = dsa_port_free_pkt,
>> +};
>> +
>> +U_BOOT_DRIVER(dsa_port) = {
>> +       .name   = DSA_PORT_CHILD_DRV_NAME,
>> +       .id     = UCLASS_ETH,
>> +       .ops    = &dsa_port_ops,
>> +       .platdata_auto_alloc_size = sizeof(struct eth_pdata),
>> +};
>> +
>> +/*
>> + * reads the DT properties of the given DSA port.
>> + * If the return value is != 0 then the port is skipped
>> + */
>> +static int dsa_port_parse_dt(struct udevice *dev, int port_index,
>> +                            ofnode ports_node, bool *is_cpu)
>> +{
>> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
>> +       struct dsa_port_platdata *port = &platdata->port[port_index];
>> +       ofnode temp_node;
>> +       u32 ethernet;
>> +
>> +       /*
>> +        * if we don't have a DT we don't do anything here but the port is
>> +        * registered normally
>> +        */
>> +       if (!ofnode_valid(ports_node))
>> +               return 0;
>> +
>> +       ofnode_for_each_subnode(temp_node, ports_node) {
>> +               const char *port_label;
>> +               u32 reg;
>> +
>> +               if (ofnode_read_u32(temp_node, "reg", &reg) ||
>> +                   reg != port_index)
>> +                       continue;
>> +
>> +               /* if the port is explicitly disabled in DT skip it */
>> +               if (!ofnode_is_available(temp_node))
>> +                       return -ENODEV;
>> +
>> +               port->node = temp_node;
>> +
>> +               dev_dbg(dev, "port %d node %s\n", port->index,
>> +                       ofnode_get_name(port->node));
>> +
>> +               /* Use 'label' if present in DT */
>> +               port_label = ofnode_read_string(port->node, "label");
>> +               if (port_label)
>> +                       strncpy(port->name, port_label, DSA_PORT_NAME_LENGTH);
>> +
>> +               *is_cpu = !ofnode_read_u32(port->node, "ethernet",
>> +                                          &ethernet);
>> +
>> +               if (*is_cpu) {
>> +                       platdata->master_node =
>> +                               ofnode_get_by_phandle(ethernet);
>> +                       platdata->cpu_port = port_index;
>> +
>> +                       dev_dbg(dev, "master node %s on port %d\n",
>> +                               ofnode_get_name(platdata->master_node),
>> +                               port_index);
>> +               }
>> +               break;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +/**
>> + * This function mostly deals with pulling information out of the device tree
>> + * into the platdata structure.
>> + * It goes through the list of switch ports, registers an Eth device for each
>> + * front panel port and identifies the cpu port connected to master Eth device.
>> + * TODO: support cascaded switches
>> + */
>> +static int dm_dsa_post_bind(struct udevice *dev)
>> +{
>> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
>> +       ofnode ports_node = ofnode_null();
>> +       int first_err = 0, err = 0, i;
>> +
>> +       if (!platdata) {
>> +               dev_err(dev, "missing plaform data\n");
>> +               return -EINVAL;
>> +       }
>> +
>> +       if (platdata->num_ports <= 0 || platdata->num_ports > DSA_MAX_PORTS) {
>> +               dev_err(dev, "unexpected num_ports value (%d)\n",
>> +                       platdata->num_ports);
>> +               return -EINVAL;
>> +       }
>> +
>> +       platdata->master_node = ofnode_null();
>> +
>> +       if (!ofnode_valid(dev->node)) {
>> +               dev_dbg(dev, "Device doesn't have a valid DT node!\n");
>> +       } else {
>> +               ports_node = ofnode_find_subnode(dev->node, "ports");
>> +               if (!ofnode_valid(ports_node))
>> +                       dev_dbg(dev,
>> +                               "ports node is missing under DSA device!\n");
>> +       }
>> +
>> +       for (i = 0; i < platdata->num_ports; i++) {
>> +               struct dsa_port_platdata *port = &platdata->port[i];
>> +               bool skip_port, is_cpu = false;
>> +
>> +               port->index = i;
>> +
>> +               /*
>> +                * If the driver set up port names in _bind use those, otherwise
>> +                * use default ones.
>> +                * If present, DT label is used as name and overrides anything
>> +                * we may have here.
>> +                */
>> +               if (!strlen(port->name))
>> +                       snprintf(port->name, DSA_PORT_NAME_LENGTH, "%s@%d",
>> +                                dev->name, i);
>> +
>> +               skip_port = !!dsa_port_parse_dt(dev, i, ports_node, &is_cpu);
>> +
>> +               /*
>> +                * if this is the CPU port don't register it as an ETH device,
>> +                * we skip it on purpose since I/O to/from it from the CPU
>> +                * isn't useful
>> +                * TODO: cpu port may have a PHY and we don't handle that yet.
>> +                */
>> +               if (is_cpu || skip_port)
>> +                       continue;
>> +
>> +               err = device_bind_driver_to_node(dev, DSA_PORT_CHILD_DRV_NAME,
>> +                                                port->name, port->node,
>> +                                                &port->dev);
>> +
>> +               /* try to bind all ports but keep 1st error */
>> +               if (err && !first_err)
>> +                       first_err = err;
>> +       }
>> +
>> +       if (!ofnode_valid(platdata->master_node))
>> +               dev_dbg(dev, "DSA master Eth device is missing!\n");
>> +
>> +       return first_err;
>> +}
>> +
>> +/**
>> + * This function deals with additional devices around the switch as these should
>> + * have been bound to drivers by now.
>> + * TODO: pick up references to other switch devices here, if we're cascaded.
>> + */
>> +static int dm_dsa_pre_probe(struct udevice *dev)
>> +{
>> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
>> +       int i;
>> +
>> +       if (!platdata)
>> +               return -EINVAL;
>> +
>> +       if (ofnode_valid(platdata->master_node))
>> +               uclass_find_device_by_ofnode(UCLASS_ETH, platdata->master_node,
>> +                                            &platdata->master_dev);
>> +
>> +       for (i = 0; i < platdata->num_ports; i++) {
>> +               struct dsa_port_platdata *port = &platdata->port[i];
>> +
>> +               if (port->dev) {
>> +                       port->dev->priv = port;
>> +                       port->phy = dm_eth_phy_connect(port->dev);
>> +               }
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +UCLASS_DRIVER(dsa) = {
>> +       .id = UCLASS_DSA,
>> +       .name = "dsa",
>> +       .post_bind  = dm_dsa_post_bind,
>> +       .pre_probe = dm_dsa_pre_probe,
>> +       .per_device_platdata_auto_alloc_size =
>> +                       sizeof(struct dsa_perdev_platdata),
>> +};
>> --
>> 2.17.1
>>
> 
> Thanks,
> -Vladimir
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 1/6] net: introduce DSA class for Ethernet switches
  2019-12-15 13:08   ` Vladimir Oltean
@ 2019-12-17  7:18     ` Alexandru Marginean
  2019-12-17  9:41       ` Vladimir Oltean
  0 siblings, 1 reply; 25+ messages in thread
From: Alexandru Marginean @ 2019-12-17  7:18 UTC (permalink / raw)
  To: u-boot

On 12/15/2019 2:08 PM, Vladimir Oltean wrote:
> On Tue, 3 Dec 2019 at 17:32, Alex Marginean <alexandru.marginean@nxp.com> wrote:
>> +/**
>> + * This function deals with additional devices around the switch as these should
>> + * have been bound to drivers by now.
>> + * TODO: pick up references to other switch devices here, if we're cascaded.
>> + */
>> +static int dm_dsa_pre_probe(struct udevice *dev)
>> +{
>> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
>> +       int i;
>> +
>> +       if (!platdata)
>> +               return -EINVAL;
>> +
>> +       if (ofnode_valid(platdata->master_node))
>> +               uclass_find_device_by_ofnode(UCLASS_ETH, platdata->master_node,
>> +                                            &platdata->master_dev);
>> +
>> +       for (i = 0; i < platdata->num_ports; i++) {
>> +               struct dsa_port_platdata *port = &platdata->port[i];
>> +
>> +               if (port->dev) {
>> +                       port->dev->priv = port;
>> +                       port->phy = dm_eth_phy_connect(port->dev);
> 
> Fixed-link interfaces don't work with DM_MDIO. That is somewhat
> natural as there is no MDIO bus for a fixed-link. However the legacy
> phy_connect function can be made rather easily to work with
> fixed-link, since it has the necessary code for dealing with it
> already. I am not, however, sure how it ever worked in the absence of
> an MDIO bus.
> 
> commit 1b7e23cc7e6d0dc3fe7ae9c55390b40717ff3c3a
> Author: Vladimir Oltean <olteanv@gmail.com>
> Date:   Sat Dec 14 23:25:40 2019 +0200
> 
>      phy: make phy_connect_fixed work with a null mdio bus
> 
>      It is utterly pointless to require an MDIO bus pointer for a fixed PHY
>      device. The fixed.c implementation does not require it, only
>      phy_device_create. Fix that.
> 
>      Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 80a7664e4978..8ea5c9005291 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -658,7 +658,7 @@ static struct phy_device *phy_device_create(struct
> mii_dev *bus, int addr,
>          dev = malloc(sizeof(*dev));
>          if (!dev) {
>                  printf("Failed to allocate PHY device for %s:%d\n",
> -                      bus->name, addr);
> +                      bus ? bus->name : "(null bus)", addr);
>                  return NULL;
>          }
> 
> @@ -686,7 +686,7 @@ static struct phy_device *phy_device_create(struct
> mii_dev *bus, int addr,
>                  return NULL;
>          }
> 
> -       if (addr >= 0 && addr < PHY_MAX_ADDR)
> +       if (addr >= 0 && addr < PHY_MAX_ADDR && phy_id != PHY_FIXED_ID)
>                  bus->phymap[addr] = dev;
> 
>          return dev;
> 
> With the patch above in place, fixed-link can also be made to work
> with some logic similar to what can be seen below:
> 
>      if (ofnode_valid(ofnode_find_subnode(port->dev->node, "fixed-link")))
>          port->phy = phy_connect(NULL, 0, port->dev, phy_mode); //
> phy_mode needs to be pre-parsed somewhere else as well
>      else
>          port->phy = dm_eth_phy_connect(port->dev);
> 
> How would you see fixed-link interfaces being treated? My question so
> far is in the context of front-panel ports but I am interested in your
> view of the CPU port situation as well.

I was thinking turning dm_eth_phy_connect into a more generic helper 
that also deals with fixed links, which it does not yet.  That would 
move the "fixed-link" if out of the driver code.  Ideally the driver 
should be able to call a single helper and, if the device has a DT node, 
it would get back a PHY handle to either a proper PHY or to a fixed link 
(from phy_connect_fixed).

Alex

> 
>> +               }
>> +       }
>> +
>> +       return 0;
>> +}
> 
> Thanks,
> -Vladimir
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 4/6] drivers: net: add Felix DSA switch driver
  2019-12-15 12:53   ` Vladimir Oltean
  2019-12-15 12:53     ` Vladimir Oltean
  2019-12-15 14:16     ` Vladimir Oltean
@ 2019-12-17  7:28     ` Alexandru Marginean
  2 siblings, 0 replies; 25+ messages in thread
From: Alexandru Marginean @ 2019-12-17  7:28 UTC (permalink / raw)
  To: u-boot

On 12/15/2019 1:53 PM, Vladimir Oltean wrote:
> Hi Alex,
> 
> On Tue, 3 Dec 2019 at 18:18, Alex Marginean <alexandru.marginean@nxp.com> wrote:
>> +static int felix_port_enable(struct udevice *dev, int port,
>> +                            struct phy_device *phy)
>> +{
>> +       struct felix_priv *priv = dev_get_priv(dev);
>> +       void *base = priv->regs_base;
>> +
>> +       out_le32(base + FELIX_GMII_MAC_ENA_CFG(port),
>> +                FELIX_GMII_MAX_ENA_CFG_TX | FELIX_GMII_MAX_ENA_CFG_RX);
>> +
>> +       out_le32(base + FELIX_QSYS_SYSTEM_SW_PORT_MODE(port),
>> +                FELIX_QSYS_SYSTEM_SW_PORT_ENA |
>> +                FELIX_QSYS_SYSTEM_SW_PORT_LOSSY |
>> +                FELIX_QSYS_SYSTEM_SW_PORT_SCH(1));
>> +
>> +       if (phy)
>> +               phy_startup(phy);
>> +       return 0;
>> +}
>> +
>> +static void felix_port_disable(struct udevice *dev, int port,
>> +                              struct phy_device *phy)
>> +{
>> +       struct felix_priv *priv = dev_get_priv(dev);
>> +       void *base = priv->regs_base;
>> +
>> +       out_le32(base + FELIX_GMII_MAC_ENA_CFG(port), 0);
>> +
>> +       out_le32(base + FELIX_QSYS_SYSTEM_SW_PORT_MODE(port),
>> +                FELIX_QSYS_SYSTEM_SW_PORT_LOSSY |
>> +                FELIX_QSYS_SYSTEM_SW_PORT_SCH(1));
>> +
>> +       /*
>> +        * we don't call phy_shutdown here to avoind waiting next time we use
>> +        * the port, but the downside is that remote side will think we're
>> +        * actively processing traffic although we are not.
>> +        */
>> +}
>> --
>> 2.17.1
>>
> 
> What is the correct general procedure here, is it to call phy_startup
> so late (felix_port_enable)? I'm trying to take this driver as an
> example for sja1105, which has RGMII so PCS and no autonomous in-band
> AN like felix does. On this switch, it is too late to do phy_startup
> now.

Why is it too late?  Is it a functional problem, or you're looking to 
reduce the waiting time?

> Instead, I would need to look at phy->speed which should have
> been settled by now, and reprogram my MAC with it.
> My question is: don't you think phy_startup() and phy_shutdown()
> belong in the DSA uclass code?
> 
> Thanks,
> -Vladimir
> 

The API is similar to the one in Linux, plus I didn't want to force a 
specific PHY related behavior to drivers.  Sometimes it's fine to start 
up the PHYs at probe and then just send/receive as needed, but that 
behavior is not always acceptable.  Assume there is some other host 
connected to one of the front panel ports, if it sees link up it may 
start doing DHCP, IPv6 discovery.  I think it's generally better to keep 
links down unless the port is actually in use for the benefit of the 
remote end.  I didn't want to force this by moving the PHY calls to 
uclass code.  This approach is also similar to eth uclass, it doesn't 
handle phy calls for the driver either.

Alex

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 4/6] drivers: net: add Felix DSA switch driver
  2019-12-15 14:16     ` Vladimir Oltean
@ 2019-12-17  7:29       ` Alexandru Marginean
  0 siblings, 0 replies; 25+ messages in thread
From: Alexandru Marginean @ 2019-12-17  7:29 UTC (permalink / raw)
  To: u-boot

On 12/15/2019 3:16 PM, Vladimir Oltean wrote:
> On Sun, 15 Dec 2019 at 14:53, Vladimir Oltean <olteanv@gmail.com> wrote:
>>
>> Hi Alex,
>>
>> On Tue, 3 Dec 2019 at 18:18, Alex Marginean <alexandru.marginean@nxp.com> wrote:
>>> +static int felix_port_enable(struct udevice *dev, int port,
>>> +                            struct phy_device *phy)
>>> +{
>>> +       struct felix_priv *priv = dev_get_priv(dev);
>>> +       void *base = priv->regs_base;
>>> +
>>> +       out_le32(base + FELIX_GMII_MAC_ENA_CFG(port),
>>> +                FELIX_GMII_MAX_ENA_CFG_TX | FELIX_GMII_MAX_ENA_CFG_RX);
>>> +
>>> +       out_le32(base + FELIX_QSYS_SYSTEM_SW_PORT_MODE(port),
>>> +                FELIX_QSYS_SYSTEM_SW_PORT_ENA |
>>> +                FELIX_QSYS_SYSTEM_SW_PORT_LOSSY |
>>> +                FELIX_QSYS_SYSTEM_SW_PORT_SCH(1));
>>> +
>>> +       if (phy)
>>> +               phy_startup(phy);
>>> +       return 0;
>>> +}
>>> +
>>> +static void felix_port_disable(struct udevice *dev, int port,
>>> +                              struct phy_device *phy)
>>> +{
>>> +       struct felix_priv *priv = dev_get_priv(dev);
>>> +       void *base = priv->regs_base;
>>> +
>>> +       out_le32(base + FELIX_GMII_MAC_ENA_CFG(port), 0);
>>> +
>>> +       out_le32(base + FELIX_QSYS_SYSTEM_SW_PORT_MODE(port),
>>> +                FELIX_QSYS_SYSTEM_SW_PORT_LOSSY |
>>> +                FELIX_QSYS_SYSTEM_SW_PORT_SCH(1));
>>> +
>>> +       /*
>>> +        * we don't call phy_shutdown here to avoind waiting next time we use
>>> +        * the port, but the downside is that remote side will think we're
>>> +        * actively processing traffic although we are not.
>>> +        */
>>> +}
>>> --
>>> 2.17.1
>>>
>>
>> What is the correct general procedure here, is it to call phy_startup
>> so late (felix_port_enable)? I'm trying to take this driver as an
>> example for sja1105, which has RGMII so PCS and no autonomous in-band
>> AN like felix does. On this switch, it is too late to do phy_startup
>> now. Instead, I would need to look at phy->speed which should have
>> been settled by now, and reprogram my MAC with it.
>> My question is: don't you think phy_startup() and phy_shutdown()
>> belong in the DSA uclass code?
>>
> 
> My bad, phy_startup() is synchronous and waits for PHY AN to complete.
> So this is fine.
> 
>> Thanks,
>> -Vladimir

OK, thanks :)
Alex

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 1/6] net: introduce DSA class for Ethernet switches
  2019-12-17  7:18     ` Alexandru Marginean
@ 2019-12-17  9:41       ` Vladimir Oltean
  2019-12-17 10:36         ` Alexandru Marginean
  0 siblings, 1 reply; 25+ messages in thread
From: Vladimir Oltean @ 2019-12-17  9:41 UTC (permalink / raw)
  To: u-boot

On Tue, 17 Dec 2019 at 09:18, Alexandru Marginean
<alexm.osslist@gmail.com> wrote:
>
> On 12/15/2019 2:08 PM, Vladimir Oltean wrote:
> > On Tue, 3 Dec 2019 at 17:32, Alex Marginean <alexandru.marginean@nxp.com> wrote:
> >> +/**
> >> + * This function deals with additional devices around the switch as these should
> >> + * have been bound to drivers by now.
> >> + * TODO: pick up references to other switch devices here, if we're cascaded.
> >> + */
> >> +static int dm_dsa_pre_probe(struct udevice *dev)
> >> +{
> >> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
> >> +       int i;
> >> +
> >> +       if (!platdata)
> >> +               return -EINVAL;
> >> +
> >> +       if (ofnode_valid(platdata->master_node))
> >> +               uclass_find_device_by_ofnode(UCLASS_ETH, platdata->master_node,
> >> +                                            &platdata->master_dev);
> >> +
> >> +       for (i = 0; i < platdata->num_ports; i++) {
> >> +               struct dsa_port_platdata *port = &platdata->port[i];
> >> +
> >> +               if (port->dev) {
> >> +                       port->dev->priv = port;
> >> +                       port->phy = dm_eth_phy_connect(port->dev);
> >
> > Fixed-link interfaces don't work with DM_MDIO. That is somewhat
> > natural as there is no MDIO bus for a fixed-link. However the legacy
> > phy_connect function can be made rather easily to work with
> > fixed-link, since it has the necessary code for dealing with it
> > already. I am not, however, sure how it ever worked in the absence of
> > an MDIO bus.
> >
> > commit 1b7e23cc7e6d0dc3fe7ae9c55390b40717ff3c3a
> > Author: Vladimir Oltean <olteanv@gmail.com>
> > Date:   Sat Dec 14 23:25:40 2019 +0200
> >
> >      phy: make phy_connect_fixed work with a null mdio bus
> >
> >      It is utterly pointless to require an MDIO bus pointer for a fixed PHY
> >      device. The fixed.c implementation does not require it, only
> >      phy_device_create. Fix that.
> >
> >      Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
> >
> > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> > index 80a7664e4978..8ea5c9005291 100644
> > --- a/drivers/net/phy/phy.c
> > +++ b/drivers/net/phy/phy.c
> > @@ -658,7 +658,7 @@ static struct phy_device *phy_device_create(struct
> > mii_dev *bus, int addr,
> >          dev = malloc(sizeof(*dev));
> >          if (!dev) {
> >                  printf("Failed to allocate PHY device for %s:%d\n",
> > -                      bus->name, addr);
> > +                      bus ? bus->name : "(null bus)", addr);
> >                  return NULL;
> >          }
> >
> > @@ -686,7 +686,7 @@ static struct phy_device *phy_device_create(struct
> > mii_dev *bus, int addr,
> >                  return NULL;
> >          }
> >
> > -       if (addr >= 0 && addr < PHY_MAX_ADDR)
> > +       if (addr >= 0 && addr < PHY_MAX_ADDR && phy_id != PHY_FIXED_ID)
> >                  bus->phymap[addr] = dev;
> >
> >          return dev;
> >
> > With the patch above in place, fixed-link can also be made to work
> > with some logic similar to what can be seen below:
> >
> >      if (ofnode_valid(ofnode_find_subnode(port->dev->node, "fixed-link")))
> >          port->phy = phy_connect(NULL, 0, port->dev, phy_mode); //
> > phy_mode needs to be pre-parsed somewhere else as well
> >      else
> >          port->phy = dm_eth_phy_connect(port->dev);
> >
> > How would you see fixed-link interfaces being treated? My question so
> > far is in the context of front-panel ports but I am interested in your
> > view of the CPU port situation as well.
>
> I was thinking turning dm_eth_phy_connect into a more generic helper
> that also deals with fixed links, which it does not yet.  That would

How would you do that? Just put my snippet above in a higher-level
wrapper over dm_eth_phy_connect and phy_connect? Otherwise I think
it's pretty difficult and hacky to pass a null mdiodev pointer through
dm_mdio_phy_connect.

> move the "fixed-link" if out of the driver code.  Ideally the driver
> should be able to call a single helper and, if the device has a DT node,
> it would get back a PHY handle to either a proper PHY or to a fixed link
> (from phy_connect_fixed).
>
> Alex
>
> >
> >> +               }
> >> +       }
> >> +
> >> +       return 0;
> >> +}
> >
> > Thanks,
> > -Vladimir
> >

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 1/6] net: introduce DSA class for Ethernet switches
  2019-12-17  9:41       ` Vladimir Oltean
@ 2019-12-17 10:36         ` Alexandru Marginean
  2019-12-17 10:47           ` Vladimir Oltean
  0 siblings, 1 reply; 25+ messages in thread
From: Alexandru Marginean @ 2019-12-17 10:36 UTC (permalink / raw)
  To: u-boot

On 12/17/2019 10:41 AM, Vladimir Oltean wrote:
> On Tue, 17 Dec 2019 at 09:18, Alexandru Marginean
> <alexm.osslist@gmail.com> wrote:
>>
>> On 12/15/2019 2:08 PM, Vladimir Oltean wrote:
>>> On Tue, 3 Dec 2019 at 17:32, Alex Marginean <alexandru.marginean@nxp.com> wrote:
>>>> +/**
>>>> + * This function deals with additional devices around the switch as these should
>>>> + * have been bound to drivers by now.
>>>> + * TODO: pick up references to other switch devices here, if we're cascaded.
>>>> + */
>>>> +static int dm_dsa_pre_probe(struct udevice *dev)
>>>> +{
>>>> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
>>>> +       int i;
>>>> +
>>>> +       if (!platdata)
>>>> +               return -EINVAL;
>>>> +
>>>> +       if (ofnode_valid(platdata->master_node))
>>>> +               uclass_find_device_by_ofnode(UCLASS_ETH, platdata->master_node,
>>>> +                                            &platdata->master_dev);
>>>> +
>>>> +       for (i = 0; i < platdata->num_ports; i++) {
>>>> +               struct dsa_port_platdata *port = &platdata->port[i];
>>>> +
>>>> +               if (port->dev) {
>>>> +                       port->dev->priv = port;
>>>> +                       port->phy = dm_eth_phy_connect(port->dev);
>>>
>>> Fixed-link interfaces don't work with DM_MDIO. That is somewhat
>>> natural as there is no MDIO bus for a fixed-link. However the legacy
>>> phy_connect function can be made rather easily to work with
>>> fixed-link, since it has the necessary code for dealing with it
>>> already. I am not, however, sure how it ever worked in the absence of
>>> an MDIO bus.
>>>
>>> commit 1b7e23cc7e6d0dc3fe7ae9c55390b40717ff3c3a
>>> Author: Vladimir Oltean <olteanv@gmail.com>
>>> Date:   Sat Dec 14 23:25:40 2019 +0200
>>>
>>>       phy: make phy_connect_fixed work with a null mdio bus
>>>
>>>       It is utterly pointless to require an MDIO bus pointer for a fixed PHY
>>>       device. The fixed.c implementation does not require it, only
>>>       phy_device_create. Fix that.
>>>
>>>       Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
>>>
>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>>> index 80a7664e4978..8ea5c9005291 100644
>>> --- a/drivers/net/phy/phy.c
>>> +++ b/drivers/net/phy/phy.c
>>> @@ -658,7 +658,7 @@ static struct phy_device *phy_device_create(struct
>>> mii_dev *bus, int addr,
>>>           dev = malloc(sizeof(*dev));
>>>           if (!dev) {
>>>                   printf("Failed to allocate PHY device for %s:%d\n",
>>> -                      bus->name, addr);
>>> +                      bus ? bus->name : "(null bus)", addr);
>>>                   return NULL;
>>>           }
>>>
>>> @@ -686,7 +686,7 @@ static struct phy_device *phy_device_create(struct
>>> mii_dev *bus, int addr,
>>>                   return NULL;
>>>           }
>>>
>>> -       if (addr >= 0 && addr < PHY_MAX_ADDR)
>>> +       if (addr >= 0 && addr < PHY_MAX_ADDR && phy_id != PHY_FIXED_ID)
>>>                   bus->phymap[addr] = dev;
>>>
>>>           return dev;
>>>
>>> With the patch above in place, fixed-link can also be made to work
>>> with some logic similar to what can be seen below:
>>>
>>>       if (ofnode_valid(ofnode_find_subnode(port->dev->node, "fixed-link")))
>>>           port->phy = phy_connect(NULL, 0, port->dev, phy_mode); //
>>> phy_mode needs to be pre-parsed somewhere else as well
>>>       else
>>>           port->phy = dm_eth_phy_connect(port->dev);
>>>
>>> How would you see fixed-link interfaces being treated? My question so
>>> far is in the context of front-panel ports but I am interested in your
>>> view of the CPU port situation as well.
>>
>> I was thinking turning dm_eth_phy_connect into a more generic helper
>> that also deals with fixed links, which it does not yet.  That would
> 
> How would you do that? Just put my snippet above in a higher-level
> wrapper over dm_eth_phy_connect and phy_connect? Otherwise I think
> it's pretty difficult and hacky to pass a null mdiodev pointer through
> dm_mdio_phy_connect.

It wouldn't go through mdio_phy_connect, dm_eth_phy_connect would need 
to figure out if it's a fixed link or PHY and then call the proper 
function, like in your snippet.
The MDIO NULL topic is a bit more complicated I think.  Some of the Eth 
drivers just deal with fixed link internally and don't create a phy 
device at all.  If there is a PHY device I think we need to have a MDIO 
bus for it, and we shouldn't piggy-back on real MDIO buses.  I'm 
thinking we could have a dummy MDIO bus created automatically by 
ETH/MDIO uclass code for fixed link PHY devices.

Thoughts?
Alex

> 
>> move the "fixed-link" if out of the driver code.  Ideally the driver
>> should be able to call a single helper and, if the device has a DT node,
>> it would get back a PHY handle to either a proper PHY or to a fixed link
>> (from phy_connect_fixed).
>>
>> Alex
>>
>>>
>>>> +               }
>>>> +       }
>>>> +
>>>> +       return 0;
>>>> +}
>>>
>>> Thanks,
>>> -Vladimir
>>>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 1/6] net: introduce DSA class for Ethernet switches
  2019-12-17 10:36         ` Alexandru Marginean
@ 2019-12-17 10:47           ` Vladimir Oltean
  0 siblings, 0 replies; 25+ messages in thread
From: Vladimir Oltean @ 2019-12-17 10:47 UTC (permalink / raw)
  To: u-boot

On Tue, 17 Dec 2019 at 12:36, Alexandru Marginean
<alexm.osslist@gmail.com> wrote:
> It wouldn't go through mdio_phy_connect, dm_eth_phy_connect would need
> to figure out if it's a fixed link or PHY and then call the proper
> function, like in your snippet.
> The MDIO NULL topic is a bit more complicated I think.  Some of the Eth
> drivers just deal with fixed link internally and don't create a phy
> device at all.  If there is a PHY device I think we need to have a MDIO
> bus for it, and we shouldn't piggy-back on real MDIO buses.  I'm
> thinking we could have a dummy MDIO bus created automatically by
> ETH/MDIO uclass code for fixed link PHY devices.

Well, either one or the other, really. The dummy MDIO bus is not
really needed, but on the other hand, if that's what it takes to make
DM_MDIO support fixed-link with the least amount of special cases,
then ok, fine with me.

>
> Thoughts?
> Alex
>

Regards,
-Vladimir

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 0/6] Introduce DSA Ethernet switch class and Felix driver
  2019-12-03 14:56 [PATCH v3 0/6] Introduce DSA Ethernet switch class and Felix driver Alex Marginean
                   ` (5 preceding siblings ...)
  2019-12-03 14:56 ` [PATCH v3 6/6] configs: ls1028a: enable the Ethernet switch driver in defconfig Alex Marginean
@ 2020-02-08 13:19 ` Vladimir Oltean
  2020-03-17  9:58   ` Alexandru Marginean
  6 siblings, 1 reply; 25+ messages in thread
From: Vladimir Oltean @ 2020-02-08 13:19 UTC (permalink / raw)
  To: u-boot

On Tue, 3 Dec 2019 at 18:18, Alex Marginean <alexandru.marginean@nxp.com> wrote:
>
> DSA stands for Distributed Switch Architecture and it is a subsystem
> introduced in the Linux kernel to support switches that:
> - have an Ethernet link up to the CPU
> - use some form of tagging to identify the source/destination port for
>   Rx/Tx
> - may be cascaded in tree-like structures.
>
> DSA is described in depth here:
> https://www.kernel.org/doc/Documentation/networking/dsa/dsa.txt
>
> From the doc:
>
>         Summarized, this is basically how DSA looks like from a network device
>         perspective:
>
>
>                     |---------------------------
>                     | CPU network device (eth0)|
>                     ----------------------------
>                     | <tag added by switch     |
>                     |                          |
>                     |                          |
>                     |        tag added by CPU> |
>                 |--------------------------------------------|
>                 | Switch driver                              |
>                 |--------------------------------------------|
>                     ||        ||         ||
>                 |-------|  |-------|  |-------|
>                 | sw0p0 |  | sw0p1 |  | sw0p2 |
>                 |-------|  |-------|  |-------|
>
> This patch set introduces a DSA class in U-Boot to support drivers of such
> switches.  DSA drivers have to implement the following ops:
> - enable/disable of switch ports,
> - insert a tag in frames being transmitted, used by the switch to select
>   the egress port,
> - parse a tag in frames being received, used for Rx traffic.
>
> DSA class code deals with presentation of switch ports as Ethernet
> interfaces, deals with the master Ethernet device for I/O and helps with
> parsing of the DT assuming the structure follows the DSA kernel binding.
>
> Support for switch cascading is not included yet.
>
> This patch set also introduces a driver for the Ethernet switch integrated
> into NXP LS1028A, called Felix.  The switch has 4 front panel ports, I/O
> to/fom it is done though an ENETC Ethernet interface and meta-data is
> carried between the switch and the driver though an additional header
> pre-pended to the original frame.
> Network commands like tftp can be used on these front panel ports.  The
> ports are disabled unless used so they do not cause issues on network
> topologies that include loops.
>
> Felix as seen on LS1028A RDB:
> => dm tree
>  Class     Index  Probed  Driver                Name
> -----------------------------------------------------------
> ......
>  dsa           0  [ + ]   felix-switch          |   |-- felix-switch
>  eth           4  [ + ]   dsa-port              |   |   |-- swp0
>  eth           5  [ + ]   dsa-port              |   |   |-- swp1
>  eth           6  [ + ]   dsa-port              |   |   |-- swp2
>  eth           7  [ + ]   dsa-port              |   |   `-- swp3
>
> => mdio list
> ......
> 10 - Vitesse VSC8514 <--> swp0
> 11 - Vitesse VSC8514 <--> swp1
> 12 - Vitesse VSC8514 <--> swp2
> 13 - Vitesse VSC8514 <--> swp3
>
> => tftp 80000000 test
> Using swp2 device
> TFTP from server 192.168.100.1; our IP address is 192.168.100.100
> Filename 'test'.
> Load address: 0x80000000
> Loading: #################################################################
>          #################################################################
>          ########################################################
>          6.8 MiB/s
> done
> Bytes transferred = 949880 (e7e78 hex)
>
> Changes in v3:
>  - fix Felix platdata size
>  - move include/dsa.h to include/net/dsa.h
>  - updated TODO in dsa.h
>  - other minor fixes
>
> Changes in v2:
>  - Don't use NULL PHY in Felix driver
>  - guard dsa.h with #ifndef __DSA__H__, somehow I missed that in v1
>  - added a TODO for setting master Eth in promiscuous mode
>  - Minor fixes in patch descriptions, API comments
>  - Added address/size-cells to LS1028A DT ports node
>
> This patch set replaces v2:
> https://patchwork.ozlabs.org/project/uboot/list/?series=144912
> and depends on:
> https://patchwork.ozlabs.org/project/uboot/list/?series=144907
> https://patchwork.ozlabs.org/project/uboot/list/?series=142879
>
> Alex Marginean (6):
>   net: introduce DSA class for Ethernet switches
>   drivers: net: add a DSA sandbox driver
>   test: dm: add a simple unit test for DSA class
>   drivers: net: add Felix DSA switch driver
>   arm: dts: ls1028a: adds Ethernet switch node and its dependencies
>   configs: ls1028a: enable the Ethernet switch driver in defconfig
>
>  arch/Kconfig                                 |   1 +
>  arch/arm/dts/fsl-ls1028a-rdb.dts             |  36 ++
>  arch/arm/dts/fsl-ls1028a.dtsi                |  44 +-
>  arch/sandbox/dts/test.dts                    |  49 ++
>  configs/ls1028aqds_tfa_SECURE_BOOT_defconfig |   3 +-
>  configs/ls1028aqds_tfa_defconfig             |   3 +-
>  configs/ls1028ardb_tfa_SECURE_BOOT_defconfig |   3 +-
>  configs/ls1028ardb_tfa_defconfig             |   3 +-
>  drivers/net/Kconfig                          |  21 +
>  drivers/net/Makefile                         |   1 +
>  drivers/net/dsa_sandbox.c                    | 272 +++++++++++
>  drivers/net/fsl_enetc.h                      |   5 +
>  drivers/net/mscc_eswitch/Kconfig             |   8 +
>  drivers/net/mscc_eswitch/Makefile            |   1 +
>  drivers/net/mscc_eswitch/felix_switch.c      | 454 +++++++++++++++++++
>  include/configs/sandbox.h                    |   4 +
>  include/dm/uclass-id.h                       |   1 +
>  include/dsa.h                                | 140 ++++++
>  net/Makefile                                 |   1 +
>  net/dsa-uclass.c                             | 369 +++++++++++++++
>  test/dm/Makefile                             |   1 +
>  test/dm/dsa.c                                |  58 +++
>  test/dm/test-fdt.c                           |   2 +-
>  23 files changed, 1474 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/net/dsa_sandbox.c
>  create mode 100644 drivers/net/mscc_eswitch/felix_switch.c
>  create mode 100644 include/dsa.h
>  create mode 100644 net/dsa-uclass.c
>  create mode 100644 test/dm/dsa.c
>
> --
> 2.17.1
>

For the entire series, you are free to add my:

Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com>

I think this looks good enough for now. I've even been able to create
another DSA driver based on this framework. Improvements (and getting
rid of the TODOs) can come further along the way, but this has been
lingering for a while now. Joe, if there aren't any complaints, do you
think you could let this in?

Thanks,
-Vladimir

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 1/6] net: introduce DSA class for Ethernet switches
  2019-12-03 14:56 ` [PATCH v3 1/6] net: introduce DSA class for Ethernet switches Alex Marginean
  2019-12-15 12:44   ` Vladimir Oltean
  2019-12-15 13:08   ` Vladimir Oltean
@ 2020-02-09 13:10   ` Vladimir Oltean
  2 siblings, 0 replies; 25+ messages in thread
From: Vladimir Oltean @ 2020-02-09 13:10 UTC (permalink / raw)
  To: u-boot

Hi Alex,

On Tue, 3 Dec 2019 at 17:32, Alex Marginean <alexandru.marginean@nxp.com> wrote:
>
> DSA stands for Distributed Switch Architecture and it covers switches that
> are connected to the CPU through an Ethernet link and generally use frame
> tags to pass information about the source/destination ports to/from CPU.
> Front panel ports are presented as regular ethernet devices in U-Boot and
> they are expected to support the typical networking commands.
> DSA switches may be cascaded, DSA class code does not currently support
> this.
>
> Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
> ---
>  drivers/net/Kconfig    |  13 ++
>  include/dm/uclass-id.h |   1 +
>  include/net/dsa.h      | 141 ++++++++++++++++
>  net/Makefile           |   1 +
>  net/dsa-uclass.c       | 369 +++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 525 insertions(+)
>  create mode 100644 include/net/dsa.h
>  create mode 100644 net/dsa-uclass.c
>
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 4182897d89..a4157cb122 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -37,6 +37,19 @@ config DM_MDIO_MUX
>           This is currently implemented in net/mdio-mux-uclass.c
>           Look in include/miiphy.h for details.
>
> +config DM_DSA
> +       bool "Enable Driver Model for DSA switches"
> +       depends on DM_ETH && DM_MDIO
> +       help
> +         Enable Driver Model for DSA switches
> +
> +         Adds UCLASS_DSA class supporting switches that follow the Distributed
> +         Switch Architecture (DSA).  These switches rely on the presence of a
> +         management switch port connected to an Ethernet controller capable of
> +         receiving frames from the switch.  This host Ethernet controller is
> +         called "master" and "cpu" in DSA terminology.
> +         This is currently implemented in net/dsa-uclass.c
> +
>  config MDIO_SANDBOX
>         depends on DM_MDIO && SANDBOX
>         default y
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 0c563d898b..8f37a91488 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -42,6 +42,7 @@ enum uclass_id {
>         UCLASS_DISPLAY,         /* Display (e.g. DisplayPort, HDMI) */
>         UCLASS_DSI_HOST,        /* Display Serial Interface host */
>         UCLASS_DMA,             /* Direct Memory Access */
> +       UCLASS_DSA,             /* Distributed (Ethernet) Switch Architecture */
>         UCLASS_EFI,             /* EFI managed devices */
>         UCLASS_ETH,             /* Ethernet device */
>         UCLASS_FIRMWARE,        /* Firmware */
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> new file mode 100644
> index 0000000000..2387419b9d
> --- /dev/null
> +++ b/include/net/dsa.h
> @@ -0,0 +1,141 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (c) 2019 NXP
> + */
> +
> +#ifndef __DSA_H__
> +#define __DSA_H__
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <phy.h>
> +
> +/**
> + * DSA stands for Distributed Switch Architecture and it is infrastructure
> + * intended to support drivers for Switches that rely on an intermediary
> + * Ethernet device for I/O.  These switches may support cascading allowing
> + * them to be arranged as a tree.
> + * DSA is documented in detail in the Linux kernel documentation under
> + * Documentation/networking/dsa/dsa.txt
> + * The network layout of such a switch is shown below:
> + *
> + *         |---------------------------
> + *         | CPU network device (eth0)|
> + *         ----------------------------
> + *         | <tag added by switch     |
> + *         |                          |
> + *         |                          |
> + *         |        tag added by CPU> |
> + *     |--------------------------------------------|
> + *     | Switch driver                              |
> + *     |--------------------------------------------|
> + *         ||        ||         ||
> + *     |-------|  |-------|  |-------|
> + *     | sw0p0 |  | sw0p1 |  | sw0p2 |
> + *     |-------|  |-------|  |-------|
> + *
> + * In U-Boot the intent is to allow access to front panel ports (shown at the
> + * bottom of the picture) though the master Ethernet port (eth0 in the picture).
> + * Front panel ports are presented as regular Ethernet devices in U-Boot and
> + * they are expected to support the typical networking commands.
> + * In general DSA switches require the use of tags, extra headers added both by
> + * software on Tx and by the switch on Rx.  These tags carry at a minimum port
> + * information and switch information for cascaded set-ups.
> + * In U-Boot these tags are inserted and parsed by the DSA switch driver, the
> + * class code helps with headroom/tailroom for the extra headers.
> + *
> + * TODO:
> + * - handle switch cascading, for now U-Boot only supports stand-alone switches.
> + * - propagate the master Eth MAC address to switch ports, this is used in
> + * Linux to avoid using additional MAC addresses on master Eth.
> + * - Add support to probe DSA switches connected to a MDIO bus, this is needed
> + * to convert switch drivers that are now under drivers/net/phy.
> + */
> +
> +#define DSA_PORT_NAME_LENGTH   16
> +
> +/* Maximum number of ports each DSA device can have */
> +#define DSA_MAX_PORTS          12
> +/* Used to size internal buffers, no support for jumbo yet */
> +#define DSA_MAX_FRAME_SIZE     2048
> +
> +/**
> + * struct dsa_ops - DSA operations
> + *
> + * @port_enable:  Initialize a switch port for I/O
> + * @port_disable: Disable a port
> + * @xmit:         Insert the DSA tag for transmission
> + *                DSA drivers receive a copy of the packet with headroom and
> + *                tailroom reserved and set to 0.
> + *                Packet points to headroom and length is updated to include
> + *                both headroom and tailroom
> + * @rcv:          Process the DSA tag on reception
> + *                Packet and length describe the frame as received from master
> + *                including any additional headers
> + */
> +struct dsa_ops {
> +       int (*port_enable)(struct udevice *dev, int port,
> +                          struct phy_device *phy);
> +       void (*port_disable)(struct udevice *dev, int port,
> +                            struct phy_device *phy);
> +       int (*xmit)(struct udevice *dev, int port, void *packet, int length);
> +       int (*rcv)(struct udevice *dev, int *port, void *packet, int length);
> +};
> +
> +#define dsa_get_ops(dev) ((struct dsa_ops *)(dev)->driver->ops)
> +
> +/**
> + * struct dsa_port_platdata - DSA port platform data
> + *
> + * @dev :  Port u-device
> + *         Uclass code sets this field for all ports
> + * @phy:   PHY device associated with this port
> + *         Uclass code sets this field for all ports except CPU port, based on
> + *         DT information.  It may be NULL.
> + * @node:  Port DT node, if any.  Uclass code sets this field.
> + * @index: Port index in the DSA switch, set by class code.
> + * @name:  Name of the port Eth device.  If a label property is present in the
> + *         port DT node, it is used as name.  Drivers can use custom names by
> + *         populating this field, otherwise class code generates a default.
> + */
> +struct dsa_port_platdata {
> +       struct udevice *dev;
> +       struct phy_device *phy;
> +       ofnode node;
> +       int index;
> +       char name[DSA_PORT_NAME_LENGTH];
> +};
> +
> +/**
> + * struct dsa_perdev_platdata - Per-device platform data for DSA DM
> + *
> + * @num_ports:   Number of ports the device has, must be <= DSA_MAX_PORTS
> + *               All DSA drivers must set this at _bind
> + * @headroom:    Size, in bytes, of headroom needed for the DSA tag
> + *               All DSA drivers must set this at _bind or _probe
> + * @tailroom:    Size, in bytes, of tailroom needed for the DSA tag
> + *               DSA class code allocates headroom and tailroom on Tx before
> + *               calling DSA driver xmit function
> + *               All DSA drivers must set this at _bind or _probe
> + * @master_node: DT node of the master Ethernet.  DT is optional so this may be
> + *               null.
> + * @master_dev:  Ethernet device to be used as master.  Uclass code sets this
> + *               based on DT information if present, otherwise drivers must set
> + *               this field in _probe.
> + * @cpu_port:    Index of switch port linked to master Ethernet.
> + *               Uclass code sets this based on DT information if present,
> + *               otherwise drivers must set this field in _bind.
> + * @port:        per-port data
> + */
> +struct dsa_perdev_platdata {
> +       int num_ports;
> +       int headroom;
> +       int tailroom;
> +
> +       ofnode master_node;
> +       struct udevice *master_dev;
> +       int cpu_port;
> +       struct dsa_port_platdata port[DSA_MAX_PORTS];
> +};
> +
> +#endif /* __DSA_H__ */
> diff --git a/net/Makefile b/net/Makefile
> index 2a700c8401..fac8c8beb9 100644
> --- a/net/Makefile
> +++ b/net/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_CMD_SNTP) += sntp.o
>  obj-$(CONFIG_CMD_TFTPBOOT) += tftp.o
>  obj-$(CONFIG_UDP_FUNCTION_FASTBOOT)  += fastboot.o
>  obj-$(CONFIG_CMD_WOL)  += wol.o
> +obj-$(CONFIG_DM_DSA)   += dsa-uclass.o
>
>  # Disable this warning as it is triggered by:
>  # sprintf(buf, index ? "foo%d" : "foo", index)
> diff --git a/net/dsa-uclass.c b/net/dsa-uclass.c
> new file mode 100644
> index 0000000000..3790a72841
> --- /dev/null
> +++ b/net/dsa-uclass.c
> @@ -0,0 +1,369 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2019, NXP
> + */
> +
> +#include <net/dsa.h>
> +#include <dm/lists.h>
> +#include <dm/device-internal.h>
> +#include <dm/uclass-internal.h>
> +#include <miiphy.h>
> +
> +#define DSA_PORT_CHILD_DRV_NAME "dsa-port"
> +
> +/* helper that returns the DSA master Ethernet device. */
> +static struct udevice *dsa_port_get_master(struct udevice *pdev, bool probe)
> +{
> +       struct udevice *dev = dev_get_parent(pdev);
> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
> +
> +       if (probe)
> +               device_probe(platdata->master_dev);
> +
> +       return platdata->master_dev;
> +}
> +
> +/*
> + * Start the desired port, the CPU port and the master Eth interface.
> + * TODO: if cascaded we may need to _start ports in other switches too
> + */
> +static int dsa_port_start(struct udevice *pdev)
> +{
> +       struct udevice *dev = dev_get_parent(pdev);
> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
> +       struct udevice *master = dsa_port_get_master(pdev, true);
> +       struct dsa_port_platdata *ppriv = dev_get_priv(pdev);
> +       struct dsa_ops *ops = dsa_get_ops(dev);
> +       int err;
> +
> +       if (!ppriv || !platdata)
> +               return -EINVAL;
> +
> +       if (!master) {
> +               dev_err(pdev, "DSA master Ethernet device not found!\n");
> +               return -EINVAL;
> +       }
> +
> +       if (ops->port_enable) {
> +               err = ops->port_enable(dev, ppriv->index, ppriv->phy);
> +               if (err)
> +                       return err;
> +               err = ops->port_enable(dev, platdata->cpu_port,
> +                                      platdata->port[platdata->cpu_port].phy);
> +               if (err)
> +                       return err;
> +       }
> +
> +       return eth_get_ops(master)->start(master);
> +}
> +
> +/* Stop the desired port, the CPU port and the master Eth interface */
> +static void dsa_port_stop(struct udevice *pdev)
> +{
> +       struct udevice *dev = dev_get_parent(pdev);
> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
> +       struct udevice *master = dsa_port_get_master(pdev, false);
> +       struct dsa_port_platdata *ppriv = dev_get_priv(pdev);
> +       struct dsa_ops *ops = dsa_get_ops(dev);
> +
> +       if (!ppriv || !platdata)
> +               return;
> +
> +       if (ops->port_disable) {
> +               ops->port_disable(dev, ppriv->index, ppriv->phy);
> +               ops->port_disable(dev, platdata->cpu_port,
> +                                 platdata->port[platdata->cpu_port].phy);
> +       }
> +
> +       /*
> +        * stop master only if it's active, don't probe it otherwise.
> +        * Under normal usage it would be active because we're using it, but
> +        * during tear-down it may have been removed ahead of us.
> +        */
> +       if (master && device_active(master))
> +               eth_get_ops(master)->stop(master);
> +}
> +
> +/*
> + * Insert a DSA tag and call master Ethernet send on the resulting packet
> + * We copy the frame to a stack buffer where we have reserved headroom and
> + * tailroom space.  Headroom and tailroom are set to 0.
> + */
> +static int dsa_port_send(struct udevice *pdev, void *packet, int length)
> +{
> +       struct udevice *dev = dev_get_parent(pdev);
> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
> +       struct udevice *master = dsa_port_get_master(pdev, true);
> +       struct dsa_port_platdata *ppriv = dev_get_priv(pdev);
> +       struct dsa_ops *ops = dsa_get_ops(dev);
> +       uchar dsa_packet[DSA_MAX_FRAME_SIZE];
> +       int head = platdata->headroom, tail = platdata->tailroom;
> +       int err;
> +
> +       if (!master)
> +               return -EINVAL;
> +
> +       if (length + head + tail > DSA_MAX_FRAME_SIZE)
> +               return -EINVAL;
> +
> +       memset(dsa_packet, 0, head);
> +       memset(dsa_packet + head + length, 0, tail);
> +       memcpy(dsa_packet + head, packet, length);
> +       length += head + tail;
> +
> +       err = ops->xmit(dev, ppriv->index, dsa_packet, length);
> +       if (err)
> +               return err;
> +
> +       return eth_get_ops(master)->send(master, dsa_packet, length);
> +}
> +
> +/* Receive a frame from master Ethernet, process it and pass it on */
> +static int dsa_port_recv(struct udevice *pdev, int flags, uchar **packetp)
> +{
> +       struct udevice *dev = dev_get_parent(pdev);
> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
> +       struct udevice *master = dsa_port_get_master(pdev, true);
> +       struct dsa_port_platdata *ppriv = dev_get_priv(pdev);
> +       struct dsa_ops *ops = dsa_get_ops(dev);
> +       int head = platdata->headroom, tail = platdata->tailroom;
> +       int length, port_index, err;
> +
> +       if (!master)
> +               return -EINVAL;
> +
> +       length = eth_get_ops(master)->recv(master, flags, packetp);
> +       if (length <= 0)
> +               return length;
> +
> +       /*
> +        * if we receive frames from a different port or frames that DSA driver
> +        * doesn't like we discard them here.
> +        * In case of discard we return with no frame and expect to be called
> +        * again instead of looping here, so upper layer can deal with timeouts
> +        * and ctrl-c
> +        */
> +       err = ops->rcv(dev, &port_index, *packetp, length);
> +       if (err || port_index != ppriv->index || (length <= head + tail)) {
> +               if (eth_get_ops(master)->free_pkt)
> +                       eth_get_ops(master)->free_pkt(master, *packetp, length);
> +               return -EAGAIN;
> +       }
> +
> +       /*
> +        * We move the pointer over headroom here to avoid a copy.  If free_pkt
> +        * gets called we move the pointer back before calling master free_pkt.
> +        */
> +       *packetp += head;
> +
> +       return length - head - tail;
> +}
> +
> +static int dsa_port_free_pkt(struct udevice *pdev, uchar *packet, int length)
> +{
> +       struct udevice *dev = dev_get_parent(pdev);
> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
> +       struct udevice *master = dsa_port_get_master(pdev, true);
> +
> +       if (!master)
> +               return -EINVAL;
> +
> +       if (eth_get_ops(master)->free_pkt) {
> +               /* return the original pointer and length to master Eth */
> +               packet -= platdata->headroom;
> +               length += platdata->headroom - platdata->tailroom;
> +
> +               return eth_get_ops(master)->free_pkt(master, packet, length);
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct eth_ops dsa_port_ops = {
> +       .start          = dsa_port_start,
> +       .send           = dsa_port_send,
> +       .recv           = dsa_port_recv,
> +       .stop           = dsa_port_stop,
> +       .free_pkt       = dsa_port_free_pkt,
> +};
> +
> +U_BOOT_DRIVER(dsa_port) = {
> +       .name   = DSA_PORT_CHILD_DRV_NAME,
> +       .id     = UCLASS_ETH,
> +       .ops    = &dsa_port_ops,
> +       .platdata_auto_alloc_size = sizeof(struct eth_pdata),
> +};
> +
> +/*
> + * reads the DT properties of the given DSA port.
> + * If the return value is != 0 then the port is skipped
> + */
> +static int dsa_port_parse_dt(struct udevice *dev, int port_index,
> +                            ofnode ports_node, bool *is_cpu)
> +{
> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
> +       struct dsa_port_platdata *port = &platdata->port[port_index];
> +       ofnode temp_node;
> +       u32 ethernet;
> +
> +       /*
> +        * if we don't have a DT we don't do anything here but the port is
> +        * registered normally
> +        */
> +       if (!ofnode_valid(ports_node))
> +               return 0;
> +
> +       ofnode_for_each_subnode(temp_node, ports_node) {
> +               const char *port_label;
> +               u32 reg;
> +
> +               if (ofnode_read_u32(temp_node, "reg", &reg) ||
> +                   reg != port_index)
> +                       continue;
> +
> +               /* if the port is explicitly disabled in DT skip it */
> +               if (!ofnode_is_available(temp_node))
> +                       return -ENODEV;
> +
> +               port->node = temp_node;
> +

I know I said that this is in principle good to go, but actually!
you should swap these 2, and set port->node = temp_node before you
actually skip ports with status = "disabled". The reason is that you
want the driver to be able to check for
ofnode_is_available(platdata->port[port].node) too, for whatever
reason.

> +               dev_dbg(dev, "port %d node %s\n", port->index,
> +                       ofnode_get_name(port->node));
> +
> +               /* Use 'label' if present in DT */
> +               port_label = ofnode_read_string(port->node, "label");
> +               if (port_label)
> +                       strncpy(port->name, port_label, DSA_PORT_NAME_LENGTH);
> +
> +               *is_cpu = !ofnode_read_u32(port->node, "ethernet",
> +                                          &ethernet);
> +
> +               if (*is_cpu) {
> +                       platdata->master_node =
> +                               ofnode_get_by_phandle(ethernet);
> +                       platdata->cpu_port = port_index;
> +
> +                       dev_dbg(dev, "master node %s on port %d\n",
> +                               ofnode_get_name(platdata->master_node),
> +                               port_index);
> +               }
> +               break;
> +       }
> +
> +       return 0;
> +}
> +
> +/**
> + * This function mostly deals with pulling information out of the device tree
> + * into the platdata structure.
> + * It goes through the list of switch ports, registers an Eth device for each
> + * front panel port and identifies the cpu port connected to master Eth device.
> + * TODO: support cascaded switches
> + */
> +static int dm_dsa_post_bind(struct udevice *dev)
> +{
> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
> +       ofnode ports_node = ofnode_null();
> +       int first_err = 0, err = 0, i;
> +
> +       if (!platdata) {
> +               dev_err(dev, "missing plaform data\n");
> +               return -EINVAL;
> +       }
> +
> +       if (platdata->num_ports <= 0 || platdata->num_ports > DSA_MAX_PORTS) {
> +               dev_err(dev, "unexpected num_ports value (%d)\n",
> +                       platdata->num_ports);
> +               return -EINVAL;
> +       }
> +
> +       platdata->master_node = ofnode_null();
> +
> +       if (!ofnode_valid(dev->node)) {
> +               dev_dbg(dev, "Device doesn't have a valid DT node!\n");
> +       } else {
> +               ports_node = ofnode_find_subnode(dev->node, "ports");
> +               if (!ofnode_valid(ports_node))
> +                       dev_dbg(dev,
> +                               "ports node is missing under DSA device!\n");
> +       }
> +
> +       for (i = 0; i < platdata->num_ports; i++) {
> +               struct dsa_port_platdata *port = &platdata->port[i];
> +               bool skip_port, is_cpu = false;
> +
> +               port->index = i;
> +
> +               /*
> +                * If the driver set up port names in _bind use those, otherwise
> +                * use default ones.
> +                * If present, DT label is used as name and overrides anything
> +                * we may have here.
> +                */
> +               if (!strlen(port->name))
> +                       snprintf(port->name, DSA_PORT_NAME_LENGTH, "%s@%d",
> +                                dev->name, i);
> +
> +               skip_port = !!dsa_port_parse_dt(dev, i, ports_node, &is_cpu);
> +
> +               /*
> +                * if this is the CPU port don't register it as an ETH device,
> +                * we skip it on purpose since I/O to/from it from the CPU
> +                * isn't useful
> +                * TODO: cpu port may have a PHY and we don't handle that yet.
> +                */
> +               if (is_cpu || skip_port)
> +                       continue;
> +
> +               err = device_bind_driver_to_node(dev, DSA_PORT_CHILD_DRV_NAME,
> +                                                port->name, port->node,
> +                                                &port->dev);
> +
> +               /* try to bind all ports but keep 1st error */
> +               if (err && !first_err)
> +                       first_err = err;
> +       }
> +
> +       if (!ofnode_valid(platdata->master_node))
> +               dev_dbg(dev, "DSA master Eth device is missing!\n");
> +
> +       return first_err;
> +}
> +
> +/**
> + * This function deals with additional devices around the switch as these should
> + * have been bound to drivers by now.
> + * TODO: pick up references to other switch devices here, if we're cascaded.
> + */
> +static int dm_dsa_pre_probe(struct udevice *dev)
> +{
> +       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
> +       int i;
> +
> +       if (!platdata)
> +               return -EINVAL;
> +
> +       if (ofnode_valid(platdata->master_node))
> +               uclass_find_device_by_ofnode(UCLASS_ETH, platdata->master_node,
> +                                            &platdata->master_dev);
> +
> +       for (i = 0; i < platdata->num_ports; i++) {
> +               struct dsa_port_platdata *port = &platdata->port[i];
> +
> +               if (port->dev) {
> +                       port->dev->priv = port;
> +                       port->phy = dm_eth_phy_connect(port->dev);
> +               }
> +       }
> +
> +       return 0;
> +}
> +
> +UCLASS_DRIVER(dsa) = {
> +       .id = UCLASS_DSA,
> +       .name = "dsa",
> +       .post_bind  = dm_dsa_post_bind,
> +       .pre_probe = dm_dsa_pre_probe,
> +       .per_device_platdata_auto_alloc_size =
> +                       sizeof(struct dsa_perdev_platdata),
> +};
> --
> 2.17.1
>

Regards,
-Vladimir

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 6/6] configs: ls1028a: enable the Ethernet switch driver in defconfig
  2019-12-03 14:56 ` [PATCH v3 6/6] configs: ls1028a: enable the Ethernet switch driver in defconfig Alex Marginean
@ 2020-03-13 14:33   ` Vladimir Oltean
  2020-03-17  9:49     ` Alexandru Marginean
  0 siblings, 1 reply; 25+ messages in thread
From: Vladimir Oltean @ 2020-03-13 14:33 UTC (permalink / raw)
  To: u-boot

On Tue, 3 Dec 2019 at 17:23, Alex Marginean <alexandru.marginean@nxp.com> wrote:
>
> The switch driver for LS1028A Ethernet switch is now compiled in for
> both LS1028A boards.
>
> Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
> ---
>  configs/ls1028aqds_tfa_SECURE_BOOT_defconfig | 3 ++-
>  configs/ls1028aqds_tfa_defconfig             | 3 ++-
>  configs/ls1028ardb_tfa_SECURE_BOOT_defconfig | 3 ++-
>  configs/ls1028ardb_tfa_defconfig             | 3 ++-
>  4 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig b/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig
> index 4a01cd6715..65e467817e 100644
> --- a/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig
> +++ b/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig
> @@ -50,8 +50,9 @@ CONFIG_PHY_ATHEROS=y
>  CONFIG_PHY_VITESSE=y
>  CONFIG_DM_ETH=y
>  CONFIG_DM_MDIO=y
> +CONFIG_DM_DSA=y
>  CONFIG_E1000=y
> -CONFIG_FSL_ENETC=y

You surely didn't want to disable FSL_ENETC here, no?

> +CONFIG_MSCC_FELIX_SWITCH=y
>  CONFIG_PCI=y
>  CONFIG_DM_PCI=y
>  CONFIG_DM_PCI_COMPAT=y
> diff --git a/configs/ls1028aqds_tfa_defconfig b/configs/ls1028aqds_tfa_defconfig
> index 1307f0d951..40d259d907 100644
> --- a/configs/ls1028aqds_tfa_defconfig
> +++ b/configs/ls1028aqds_tfa_defconfig
> @@ -56,8 +56,9 @@ CONFIG_PHY_ATHEROS=y
>  CONFIG_PHY_VITESSE=y
>  CONFIG_DM_ETH=y
>  CONFIG_DM_MDIO=y
> +CONFIG_DM_DSA=y
>  CONFIG_E1000=y
> -CONFIG_FSL_ENETC=y
> +CONFIG_MSCC_FELIX_SWITCH=y
>  CONFIG_PCI=y
>  CONFIG_DM_PCI=y
>  CONFIG_DM_PCI_COMPAT=y
> diff --git a/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig b/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig
> index d0a3310a4c..f54a6da31b 100644
> --- a/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig
> +++ b/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig
> @@ -49,9 +49,10 @@ CONFIG_PHY_ATHEROS=y
>  CONFIG_PHY_VITESSE=y
>  CONFIG_DM_ETH=y
>  CONFIG_DM_MDIO=y
> +CONFIG_DM_DSA=y
>  CONFIG_PHY_GIGE=y
>  CONFIG_E1000=y
> -CONFIG_FSL_ENETC=y
> +CONFIG_MSCC_FELIX_SWITCH=y
>  CONFIG_PCI=y
>  CONFIG_DM_PCI=y
>  CONFIG_DM_PCI_COMPAT=y
> diff --git a/configs/ls1028ardb_tfa_defconfig b/configs/ls1028ardb_tfa_defconfig
> index 4ec7ed0920..e018e5a50e 100644
> --- a/configs/ls1028ardb_tfa_defconfig
> +++ b/configs/ls1028ardb_tfa_defconfig
> @@ -56,9 +56,10 @@ CONFIG_PHY_ATHEROS=y
>  CONFIG_PHY_VITESSE=y
>  CONFIG_DM_ETH=y
>  CONFIG_DM_MDIO=y
> +CONFIG_DM_DSA=y
>  CONFIG_PHY_GIGE=y
>  CONFIG_E1000=y
> -CONFIG_FSL_ENETC=y
> +CONFIG_MSCC_FELIX_SWITCH=y
>  CONFIG_PCI=y
>  CONFIG_DM_PCI=y
>  CONFIG_DM_PCI_COMPAT=y
> --
> 2.17.1
>

Regards,
-Vladimir

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 6/6] configs: ls1028a: enable the Ethernet switch driver in defconfig
  2020-03-13 14:33   ` Vladimir Oltean
@ 2020-03-17  9:49     ` Alexandru Marginean
  0 siblings, 0 replies; 25+ messages in thread
From: Alexandru Marginean @ 2020-03-17  9:49 UTC (permalink / raw)
  To: u-boot

On 3/13/2020 3:33 PM, Vladimir Oltean wrote:
> On Tue, 3 Dec 2019 at 17:23, Alex Marginean <alexandru.marginean@nxp.com> wrote:
>>
>> The switch driver for LS1028A Ethernet switch is now compiled in for
>> both LS1028A boards.
>>
>> Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
>> ---
>>   configs/ls1028aqds_tfa_SECURE_BOOT_defconfig | 3 ++-
>>   configs/ls1028aqds_tfa_defconfig             | 3 ++-
>>   configs/ls1028ardb_tfa_SECURE_BOOT_defconfig | 3 ++-
>>   configs/ls1028ardb_tfa_defconfig             | 3 ++-
>>   4 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig b/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig
>> index 4a01cd6715..65e467817e 100644
>> --- a/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig
>> +++ b/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig
>> @@ -50,8 +50,9 @@ CONFIG_PHY_ATHEROS=y
>>   CONFIG_PHY_VITESSE=y
>>   CONFIG_DM_ETH=y
>>   CONFIG_DM_MDIO=y
>> +CONFIG_DM_DSA=y
>>   CONFIG_E1000=y
>> -CONFIG_FSL_ENETC=y
> 
> You surely didn't want to disable FSL_ENETC here, no?

I think there's a dependency set and ENETC is implied if FELIX_SWITCH is 
enabled.  I saved these with savedefconfig, I didn't manually edit them.

Alex

> 
>> +CONFIG_MSCC_FELIX_SWITCH=y
>>   CONFIG_PCI=y
>>   CONFIG_DM_PCI=y
>>   CONFIG_DM_PCI_COMPAT=y
>> diff --git a/configs/ls1028aqds_tfa_defconfig b/configs/ls1028aqds_tfa_defconfig
>> index 1307f0d951..40d259d907 100644
>> --- a/configs/ls1028aqds_tfa_defconfig
>> +++ b/configs/ls1028aqds_tfa_defconfig
>> @@ -56,8 +56,9 @@ CONFIG_PHY_ATHEROS=y
>>   CONFIG_PHY_VITESSE=y
>>   CONFIG_DM_ETH=y
>>   CONFIG_DM_MDIO=y
>> +CONFIG_DM_DSA=y
>>   CONFIG_E1000=y
>> -CONFIG_FSL_ENETC=y
>> +CONFIG_MSCC_FELIX_SWITCH=y
>>   CONFIG_PCI=y
>>   CONFIG_DM_PCI=y
>>   CONFIG_DM_PCI_COMPAT=y
>> diff --git a/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig b/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig
>> index d0a3310a4c..f54a6da31b 100644
>> --- a/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig
>> +++ b/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig
>> @@ -49,9 +49,10 @@ CONFIG_PHY_ATHEROS=y
>>   CONFIG_PHY_VITESSE=y
>>   CONFIG_DM_ETH=y
>>   CONFIG_DM_MDIO=y
>> +CONFIG_DM_DSA=y
>>   CONFIG_PHY_GIGE=y
>>   CONFIG_E1000=y
>> -CONFIG_FSL_ENETC=y
>> +CONFIG_MSCC_FELIX_SWITCH=y
>>   CONFIG_PCI=y
>>   CONFIG_DM_PCI=y
>>   CONFIG_DM_PCI_COMPAT=y
>> diff --git a/configs/ls1028ardb_tfa_defconfig b/configs/ls1028ardb_tfa_defconfig
>> index 4ec7ed0920..e018e5a50e 100644
>> --- a/configs/ls1028ardb_tfa_defconfig
>> +++ b/configs/ls1028ardb_tfa_defconfig
>> @@ -56,9 +56,10 @@ CONFIG_PHY_ATHEROS=y
>>   CONFIG_PHY_VITESSE=y
>>   CONFIG_DM_ETH=y
>>   CONFIG_DM_MDIO=y
>> +CONFIG_DM_DSA=y
>>   CONFIG_PHY_GIGE=y
>>   CONFIG_E1000=y
>> -CONFIG_FSL_ENETC=y
>> +CONFIG_MSCC_FELIX_SWITCH=y
>>   CONFIG_PCI=y
>>   CONFIG_DM_PCI=y
>>   CONFIG_DM_PCI_COMPAT=y
>> --
>> 2.17.1
>>
> 
> Regards,
> -Vladimir
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 0/6] Introduce DSA Ethernet switch class and Felix driver
  2020-02-08 13:19 ` [PATCH v3 0/6] Introduce DSA Ethernet switch class and Felix driver Vladimir Oltean
@ 2020-03-17  9:58   ` Alexandru Marginean
  0 siblings, 0 replies; 25+ messages in thread
From: Alexandru Marginean @ 2020-03-17  9:58 UTC (permalink / raw)
  To: u-boot

On 2/8/2020 2:19 PM, Vladimir Oltean wrote:
> On Tue, 3 Dec 2019 at 18:18, Alex Marginean <alexandru.marginean@nxp.com> wrote:
>>
>> DSA stands for Distributed Switch Architecture and it is a subsystem
>> introduced in the Linux kernel to support switches that:
>> - have an Ethernet link up to the CPU
>> - use some form of tagging to identify the source/destination port for
>>    Rx/Tx
>> - may be cascaded in tree-like structures.
>>
>> DSA is described in depth here:
>> https://www.kernel.org/doc/Documentation/networking/dsa/dsa.txt
>>
>>  From the doc:
>>
>>          Summarized, this is basically how DSA looks like from a network device
>>          perspective:
>>
>>
>>                      |---------------------------
>>                      | CPU network device (eth0)|
>>                      ----------------------------
>>                      | <tag added by switch     |
>>                      |                          |
>>                      |                          |
>>                      |        tag added by CPU> |
>>                  |--------------------------------------------|
>>                  | Switch driver                              |
>>                  |--------------------------------------------|
>>                      ||        ||         ||
>>                  |-------|  |-------|  |-------|
>>                  | sw0p0 |  | sw0p1 |  | sw0p2 |
>>                  |-------|  |-------|  |-------|
>>
>> This patch set introduces a DSA class in U-Boot to support drivers of such
>> switches.  DSA drivers have to implement the following ops:
>> - enable/disable of switch ports,
>> - insert a tag in frames being transmitted, used by the switch to select
>>    the egress port,
>> - parse a tag in frames being received, used for Rx traffic.
>>
>> DSA class code deals with presentation of switch ports as Ethernet
>> interfaces, deals with the master Ethernet device for I/O and helps with
>> parsing of the DT assuming the structure follows the DSA kernel binding.
>>
>> Support for switch cascading is not included yet.
>>
>> This patch set also introduces a driver for the Ethernet switch integrated
>> into NXP LS1028A, called Felix.  The switch has 4 front panel ports, I/O
>> to/fom it is done though an ENETC Ethernet interface and meta-data is
>> carried between the switch and the driver though an additional header
>> pre-pended to the original frame.
>> Network commands like tftp can be used on these front panel ports.  The
>> ports are disabled unless used so they do not cause issues on network
>> topologies that include loops.
>>
>> Felix as seen on LS1028A RDB:
>> => dm tree
>>   Class     Index  Probed  Driver                Name
>> -----------------------------------------------------------
>> ......
>>   dsa           0  [ + ]   felix-switch          |   |-- felix-switch
>>   eth           4  [ + ]   dsa-port              |   |   |-- swp0
>>   eth           5  [ + ]   dsa-port              |   |   |-- swp1
>>   eth           6  [ + ]   dsa-port              |   |   |-- swp2
>>   eth           7  [ + ]   dsa-port              |   |   `-- swp3
>>
>> => mdio list
>> ......
>> 10 - Vitesse VSC8514 <--> swp0
>> 11 - Vitesse VSC8514 <--> swp1
>> 12 - Vitesse VSC8514 <--> swp2
>> 13 - Vitesse VSC8514 <--> swp3
>>
>> => tftp 80000000 test
>> Using swp2 device
>> TFTP from server 192.168.100.1; our IP address is 192.168.100.100
>> Filename 'test'.
>> Load address: 0x80000000
>> Loading: #################################################################
>>           #################################################################
>>           ########################################################
>>           6.8 MiB/s
>> done
>> Bytes transferred = 949880 (e7e78 hex)
>>
>> Changes in v3:
>>   - fix Felix platdata size
>>   - move include/dsa.h to include/net/dsa.h
>>   - updated TODO in dsa.h
>>   - other minor fixes
>>
>> Changes in v2:
>>   - Don't use NULL PHY in Felix driver
>>   - guard dsa.h with #ifndef __DSA__H__, somehow I missed that in v1
>>   - added a TODO for setting master Eth in promiscuous mode
>>   - Minor fixes in patch descriptions, API comments
>>   - Added address/size-cells to LS1028A DT ports node
>>
>> This patch set replaces v2:
>> https://patchwork.ozlabs.org/project/uboot/list/?series=144912
>> and depends on:
>> https://patchwork.ozlabs.org/project/uboot/list/?series=144907
>> https://patchwork.ozlabs.org/project/uboot/list/?series=142879
>>
>> Alex Marginean (6):
>>    net: introduce DSA class for Ethernet switches
>>    drivers: net: add a DSA sandbox driver
>>    test: dm: add a simple unit test for DSA class
>>    drivers: net: add Felix DSA switch driver
>>    arm: dts: ls1028a: adds Ethernet switch node and its dependencies
>>    configs: ls1028a: enable the Ethernet switch driver in defconfig
>>
>>   arch/Kconfig                                 |   1 +
>>   arch/arm/dts/fsl-ls1028a-rdb.dts             |  36 ++
>>   arch/arm/dts/fsl-ls1028a.dtsi                |  44 +-
>>   arch/sandbox/dts/test.dts                    |  49 ++
>>   configs/ls1028aqds_tfa_SECURE_BOOT_defconfig |   3 +-
>>   configs/ls1028aqds_tfa_defconfig             |   3 +-
>>   configs/ls1028ardb_tfa_SECURE_BOOT_defconfig |   3 +-
>>   configs/ls1028ardb_tfa_defconfig             |   3 +-
>>   drivers/net/Kconfig                          |  21 +
>>   drivers/net/Makefile                         |   1 +
>>   drivers/net/dsa_sandbox.c                    | 272 +++++++++++
>>   drivers/net/fsl_enetc.h                      |   5 +
>>   drivers/net/mscc_eswitch/Kconfig             |   8 +
>>   drivers/net/mscc_eswitch/Makefile            |   1 +
>>   drivers/net/mscc_eswitch/felix_switch.c      | 454 +++++++++++++++++++
>>   include/configs/sandbox.h                    |   4 +
>>   include/dm/uclass-id.h                       |   1 +
>>   include/dsa.h                                | 140 ++++++
>>   net/Makefile                                 |   1 +
>>   net/dsa-uclass.c                             | 369 +++++++++++++++
>>   test/dm/Makefile                             |   1 +
>>   test/dm/dsa.c                                |  58 +++
>>   test/dm/test-fdt.c                           |   2 +-
>>   23 files changed, 1474 insertions(+), 6 deletions(-)
>>   create mode 100644 drivers/net/dsa_sandbox.c
>>   create mode 100644 drivers/net/mscc_eswitch/felix_switch.c
>>   create mode 100644 include/dsa.h
>>   create mode 100644 net/dsa-uclass.c
>>   create mode 100644 test/dm/dsa.c
>>
>> --
>> 2.17.1
>>
> 
> For the entire series, you are free to add my:
> 
> Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> I think this looks good enough for now. I've even been able to create
> another DSA driver based on this framework. Improvements (and getting
> rid of the TODOs) can come further along the way, but this has been
> lingering for a while now. Joe, if there aren't any complaints, do you
> think you could let this in?
> 
> Thanks,
> -Vladimir
> 

Yeah, this has been sitting here for some time and I didn't get to work 
on it more.  One thing I think it would be worth including early is 
dealing with MAC addresses for switch ports.  Linux DSA uses the MAC 
address of master Eth and we should do the same in U-Boot.  That's not 
just for consistency with Linux but also to make this work on arbitrary 
mixes of switches and Eth ports without having to set master Eth in 
promisc mode.
It's not really a big change, I'll try to look into it in the next 
couple of weeks.

Thanks!
Alex

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2020-03-17  9:58 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-03 14:56 [PATCH v3 0/6] Introduce DSA Ethernet switch class and Felix driver Alex Marginean
2019-12-03 14:56 ` [PATCH v3 1/6] net: introduce DSA class for Ethernet switches Alex Marginean
2019-12-15 12:44   ` Vladimir Oltean
2019-12-17  7:11     ` Alexandru Marginean
2019-12-15 13:08   ` Vladimir Oltean
2019-12-17  7:18     ` Alexandru Marginean
2019-12-17  9:41       ` Vladimir Oltean
2019-12-17 10:36         ` Alexandru Marginean
2019-12-17 10:47           ` Vladimir Oltean
2020-02-09 13:10   ` Vladimir Oltean
2019-12-03 14:56 ` [PATCH v3 2/6] drivers: net: add a DSA sandbox driver Alex Marginean
2019-12-03 14:56 ` [PATCH v3 3/6] test: dm: add a simple unit test for DSA class Alex Marginean
2019-12-04  4:14   ` Priyanka Jain
2019-12-03 14:56 ` [PATCH v3 4/6] drivers: net: add Felix DSA switch driver Alex Marginean
2019-12-15 12:53   ` Vladimir Oltean
2019-12-15 12:53     ` Vladimir Oltean
2019-12-15 14:16     ` Vladimir Oltean
2019-12-17  7:29       ` Alexandru Marginean
2019-12-17  7:28     ` Alexandru Marginean
2019-12-03 14:56 ` [PATCH v3 5/6] arm: dts: ls1028a: adds Ethernet switch node and its dependencies Alex Marginean
2019-12-03 14:56 ` [PATCH v3 6/6] configs: ls1028a: enable the Ethernet switch driver in defconfig Alex Marginean
2020-03-13 14:33   ` Vladimir Oltean
2020-03-17  9:49     ` Alexandru Marginean
2020-02-08 13:19 ` [PATCH v3 0/6] Introduce DSA Ethernet switch class and Felix driver Vladimir Oltean
2020-03-17  9:58   ` Alexandru Marginean

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.