All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/4] dpaa2-eth: add MAC/PHY support through phylink
@ 2019-10-21 22:50 Ioana Ciornei
  2019-10-21 22:50 ` [PATCH net-next 1/4] dpaa2-eth: update the TX frame queues on DPNI_IRQ_EVENT_ENDPOINT_CHANGED Ioana Ciornei
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Ioana Ciornei @ 2019-10-21 22:50 UTC (permalink / raw)
  To: davem, netdev; +Cc: laurentiu.tudor, andrew, f.fainelli, rmk, Ioana Ciornei

The dpaa2-eth driver now has support for connecting to its associated PHY
device found through standard OF bindings. The PHY interraction is handled
by PHYLINK and even though, at the moment, only RGMII_* phy modes are
supported by the driver, this is just the first step into adding the
necessary changes to support the entire spectrum of capabilities.

This comes after feedback on the initial DPAA2 MAC RFC submitted here:
https://lwn.net/Articles/791182/

The notable change is that now, the DPMAC is not a separate driver, and
communication between the DPMAC and DPNI no longer happens through
firmware. Rather, the DPMAC is now a set of API functions that other
net_device drivers (DPNI, DPSW, etc) can use for PHY management.

The change is incremental, because the DPAA2 architecture has many modes of
connecting net devices in hardware loopback (for example DPNI to DPNI).
Those operating modes do not have a DPMAC and phylink instance.

The documentation patch provides a more complete view of the software
architecture and the current implementation.

Ioana Ciornei (4):
  dpaa2-eth: update the TX frame queues on
    DPNI_IRQ_EVENT_ENDPOINT_CHANGED
  bus: fsl-mc: add the fsl_mc_get_endpoint function
  dpaa2-eth: add MAC/PHY support through phylink
  net: documentation: add docs for MAC/PHY support in DPAA2

 .../device_drivers/freescale/dpaa2/index.rst       |   1 +
 .../freescale/dpaa2/mac-phy-support.rst            | 191 ++++++++++++
 MAINTAINERS                                        |   4 +
 drivers/bus/fsl-mc/dprc-driver.c                   |   6 +-
 drivers/bus/fsl-mc/dprc.c                          |  53 ++++
 drivers/bus/fsl-mc/fsl-mc-bus.c                    |  33 ++
 drivers/bus/fsl-mc/fsl-mc-private.h                |  42 +++
 drivers/net/ethernet/freescale/dpaa2/Makefile      |   2 +-
 drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c   | 124 ++++++--
 drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h   |   3 +
 .../net/ethernet/freescale/dpaa2/dpaa2-ethtool.c   |  25 ++
 drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c   | 332 +++++++++++++++++++++
 drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h   |  32 ++
 drivers/net/ethernet/freescale/dpaa2/dpmac-cmd.h   |  62 ++++
 drivers/net/ethernet/freescale/dpaa2/dpmac.c       | 149 +++++++++
 drivers/net/ethernet/freescale/dpaa2/dpmac.h       | 144 +++++++++
 include/linux/fsl/mc.h                             |   2 +
 17 files changed, 1176 insertions(+), 29 deletions(-)
 create mode 100644 Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-support.rst
 create mode 100644 drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
 create mode 100644 drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
 create mode 100644 drivers/net/ethernet/freescale/dpaa2/dpmac-cmd.h
 create mode 100644 drivers/net/ethernet/freescale/dpaa2/dpmac.c
 create mode 100644 drivers/net/ethernet/freescale/dpaa2/dpmac.h

-- 
1.9.1


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

* [PATCH net-next 1/4] dpaa2-eth: update the TX frame queues on DPNI_IRQ_EVENT_ENDPOINT_CHANGED
  2019-10-21 22:50 [PATCH net-next 0/4] dpaa2-eth: add MAC/PHY support through phylink Ioana Ciornei
@ 2019-10-21 22:50 ` Ioana Ciornei
  2019-10-21 22:50 ` [PATCH net-next 2/4] bus: fsl-mc: add the fsl_mc_get_endpoint function Ioana Ciornei
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 18+ messages in thread
From: Ioana Ciornei @ 2019-10-21 22:50 UTC (permalink / raw)
  To: davem, netdev; +Cc: laurentiu.tudor, andrew, f.fainelli, rmk, Ioana Ciornei

Currently the function is called at every link up event, although the
FQID values will only change when the DPNI is disconnected from the
current object and reconnected to a different one.

The patch also avoids the forward declaration of update_tx_fqids.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
index 90fc79b3fd0a..602d5118e928 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
@@ -1255,8 +1255,6 @@ static void dpaa2_eth_set_rx_taildrop(struct dpaa2_eth_priv *priv, bool enable)
 	priv->rx_td_enabled = enable;
 }
 
-static void update_tx_fqids(struct dpaa2_eth_priv *priv);
-
 static int link_state_update(struct dpaa2_eth_priv *priv)
 {
 	struct dpni_link_state state = {0};
@@ -1283,7 +1281,6 @@ static int link_state_update(struct dpaa2_eth_priv *priv)
 		goto out;
 
 	if (state.up) {
-		update_tx_fqids(priv);
 		netif_carrier_on(priv->net_dev);
 		netif_tx_start_all_queues(priv->net_dev);
 	} else {
@@ -3363,8 +3360,10 @@ static irqreturn_t dpni_irq0_handler_thread(int irq_num, void *arg)
 	if (status & DPNI_IRQ_EVENT_LINK_CHANGED)
 		link_state_update(netdev_priv(net_dev));
 
-	if (status & DPNI_IRQ_EVENT_ENDPOINT_CHANGED)
+	if (status & DPNI_IRQ_EVENT_ENDPOINT_CHANGED) {
 		set_mac_addr(netdev_priv(net_dev));
+		update_tx_fqids(priv);
+	}
 
 	return IRQ_HANDLED;
 }
-- 
1.9.1


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

* [PATCH net-next 2/4] bus: fsl-mc: add the fsl_mc_get_endpoint function
  2019-10-21 22:50 [PATCH net-next 0/4] dpaa2-eth: add MAC/PHY support through phylink Ioana Ciornei
  2019-10-21 22:50 ` [PATCH net-next 1/4] dpaa2-eth: update the TX frame queues on DPNI_IRQ_EVENT_ENDPOINT_CHANGED Ioana Ciornei
@ 2019-10-21 22:50 ` Ioana Ciornei
  2019-10-21 23:13   ` Andrew Lunn
  2019-10-21 22:50 ` [PATCH net-next 3/4] dpaa2-eth: add MAC/PHY support through phylink Ioana Ciornei
  2019-10-21 22:50 ` [PATCH net-next 4/4] net: documentation: add docs for MAC/PHY support in DPAA2 Ioana Ciornei
  3 siblings, 1 reply; 18+ messages in thread
From: Ioana Ciornei @ 2019-10-21 22:50 UTC (permalink / raw)
  To: davem, netdev; +Cc: laurentiu.tudor, andrew, f.fainelli, rmk, Ioana Ciornei

Using the newly added fsl_mc_get_endpoint function a fsl-mc driver can
find its associated endpoint (another object at the other link of a MC
firmware link).

The API will be used in the following patch in order to discover the
connected DPMAC object of a DPNI.

Also, the fsl_mc_device_lookup function is made available to the entire
fsl-mc bus driver and not just for the dprc driver.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/bus/fsl-mc/dprc-driver.c    |  6 ++---
 drivers/bus/fsl-mc/dprc.c           | 53 +++++++++++++++++++++++++++++++++++++
 drivers/bus/fsl-mc/fsl-mc-bus.c     | 33 +++++++++++++++++++++++
 drivers/bus/fsl-mc/fsl-mc-private.h | 42 +++++++++++++++++++++++++++++
 include/linux/fsl/mc.h              |  2 ++
 5 files changed, 132 insertions(+), 4 deletions(-)

diff --git a/drivers/bus/fsl-mc/dprc-driver.c b/drivers/bus/fsl-mc/dprc-driver.c
index 52c7e15143d6..c8b1c3842c1a 100644
--- a/drivers/bus/fsl-mc/dprc-driver.c
+++ b/drivers/bus/fsl-mc/dprc-driver.c
@@ -104,10 +104,8 @@ static int __fsl_mc_device_match(struct device *dev, void *data)
 	return fsl_mc_device_match(mc_dev, obj_desc);
 }
 
-static struct fsl_mc_device *fsl_mc_device_lookup(struct fsl_mc_obj_desc
-								*obj_desc,
-						  struct fsl_mc_device
-								*mc_bus_dev)
+struct fsl_mc_device *fsl_mc_device_lookup(struct fsl_mc_obj_desc *obj_desc,
+					   struct fsl_mc_device *mc_bus_dev)
 {
 	struct device *dev;
 
diff --git a/drivers/bus/fsl-mc/dprc.c b/drivers/bus/fsl-mc/dprc.c
index 0fe3f52ae0de..599240e20a4c 100644
--- a/drivers/bus/fsl-mc/dprc.c
+++ b/drivers/bus/fsl-mc/dprc.c
@@ -554,3 +554,56 @@ int dprc_get_container_id(struct fsl_mc_io *mc_io,
 
 	return 0;
 }
+
+/**
+ * dprc_get_connection() - Get connected endpoint and link status if connection
+ *			exists.
+ * @mc_io:	Pointer to MC portal's I/O object
+ * @cmd_flags:	Command flags; one or more of 'MC_CMD_FLAG_'
+ * @token:	Token of DPRC object
+ * @endpoint1:	Endpoint 1 configuration parameters
+ * @endpoint2:	Returned endpoint 2 configuration parameters
+ * @state:	Returned link state:
+ *		1 - link is up;
+ *		0 - link is down;
+ *		-1 - no connection (endpoint2 information is irrelevant)
+ *
+ * Return:     '0' on Success; -ENAVAIL if connection does not exist.
+ */
+int dprc_get_connection(struct fsl_mc_io *mc_io,
+			u32 cmd_flags,
+			u16 token,
+			const struct dprc_endpoint *endpoint1,
+			struct dprc_endpoint *endpoint2,
+			int *state)
+{
+	struct dprc_cmd_get_connection *cmd_params;
+	struct dprc_rsp_get_connection *rsp_params;
+	struct fsl_mc_command cmd = { 0 };
+	int err, i;
+
+	/* prepare command */
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_CONNECTION,
+					  cmd_flags,
+					  token);
+	cmd_params = (struct dprc_cmd_get_connection *)cmd.params;
+	cmd_params->ep1_id = cpu_to_le32(endpoint1->id);
+	cmd_params->ep1_interface_id = cpu_to_le16(endpoint1->if_id);
+	for (i = 0; i < 16; i++)
+		cmd_params->ep1_type[i] = endpoint1->type[i];
+
+	/* send command to mc*/
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+	/* retrieve response parameters */
+	rsp_params = (struct dprc_rsp_get_connection *)cmd.params;
+	endpoint2->id = le32_to_cpu(rsp_params->ep2_id);
+	endpoint2->if_id = le16_to_cpu(rsp_params->ep2_interface_id);
+	*state = le32_to_cpu(rsp_params->state);
+	for (i = 0; i < 16; i++)
+		endpoint2->type[i] = rsp_params->ep2_type[i];
+
+	return 0;
+}
diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
index 5c9bf2e06552..7a7ee25175fa 100644
--- a/drivers/bus/fsl-mc/fsl-mc-bus.c
+++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
@@ -702,6 +702,39 @@ void fsl_mc_device_remove(struct fsl_mc_device *mc_dev)
 }
 EXPORT_SYMBOL_GPL(fsl_mc_device_remove);
 
+struct fsl_mc_device *fsl_mc_get_endpoint(struct fsl_mc_device *mc_dev)
+{
+	struct fsl_mc_device *mc_bus_dev, *endpoint;
+	struct fsl_mc_obj_desc endpoint_desc = { 0 };
+	struct dprc_endpoint endpoint1 = { 0 };
+	struct dprc_endpoint endpoint2 = { 0 };
+	int state, err;
+
+	mc_bus_dev = to_fsl_mc_device(mc_dev->dev.parent);
+	strcpy(endpoint1.type, mc_dev->obj_desc.type);
+	endpoint1.id = mc_dev->obj_desc.id;
+
+	err = dprc_get_connection(mc_bus_dev->mc_io, 0,
+				  mc_bus_dev->mc_handle,
+				  &endpoint1, &endpoint2,
+				  &state);
+
+	if (err == -EINVAL || state == -1)
+		return NULL;
+
+	if (err < 0) {
+		dev_err(&mc_bus_dev->dev, "dprc_get_connection() = %d\n", err);
+		return NULL;
+	}
+
+	strcpy(endpoint_desc.type, endpoint2.type);
+	endpoint_desc.id = endpoint2.id;
+	endpoint = fsl_mc_device_lookup(&endpoint_desc, mc_bus_dev);
+
+	return endpoint;
+}
+EXPORT_SYMBOL_GPL(fsl_mc_get_endpoint);
+
 static int parse_mc_ranges(struct device *dev,
 			   int *paddr_cells,
 			   int *mc_addr_cells,
diff --git a/drivers/bus/fsl-mc/fsl-mc-private.h b/drivers/bus/fsl-mc/fsl-mc-private.h
index 020fcc04ec8b..21ca8c756ee7 100644
--- a/drivers/bus/fsl-mc/fsl-mc-private.h
+++ b/drivers/bus/fsl-mc/fsl-mc-private.h
@@ -105,6 +105,8 @@ int dpmcp_reset(struct fsl_mc_io *mc_io,
 #define DPRC_CMDID_GET_OBJ_REG_V2               DPRC_CMD_V2(0x15E)
 #define DPRC_CMDID_SET_OBJ_IRQ                  DPRC_CMD(0x15F)
 
+#define DPRC_CMDID_GET_CONNECTION               DPRC_CMD(0x16C)
+
 struct dprc_cmd_open {
 	__le32 container_id;
 };
@@ -228,6 +230,22 @@ struct dprc_cmd_set_obj_irq {
 	u8 obj_type[16];
 };
 
+struct dprc_cmd_get_connection {
+	__le32 ep1_id;
+	__le16 ep1_interface_id;
+	u8 pad[2];
+	u8 ep1_type[16];
+};
+
+struct dprc_rsp_get_connection {
+	__le64 pad[3];
+	__le32 ep2_id;
+	__le16 ep2_interface_id;
+	__le16 pad1;
+	u8 ep2_type[16];
+	__le32 state;
+};
+
 /*
  * DPRC API for managing and querying DPAA resources
  */
@@ -392,6 +410,27 @@ int dprc_get_container_id(struct fsl_mc_io *mc_io,
 			  u32 cmd_flags,
 			  int *container_id);
 
+/**
+ * struct dprc_endpoint - Endpoint description for link connect/disconnect
+ *			operations
+ * @type:	Endpoint object type: NULL terminated string
+ * @id:		Endpoint object ID
+ * @if_id:	Interface ID; should be set for endpoints with multiple
+ *		interfaces ("dpsw", "dpdmux"); for others, always set to 0
+ */
+struct dprc_endpoint {
+	char type[16];
+	int id;
+	u16 if_id;
+};
+
+int dprc_get_connection(struct fsl_mc_io *mc_io,
+			u32 cmd_flags,
+			u16 token,
+			const struct dprc_endpoint *endpoint1,
+			struct dprc_endpoint *endpoint2,
+			int *state);
+
 /*
  * Data Path Buffer Pool (DPBP) API
  */
@@ -574,4 +613,7 @@ int __must_check fsl_create_mc_io(struct device *dev,
 
 bool fsl_mc_is_root_dprc(struct device *dev);
 
+struct fsl_mc_device *fsl_mc_device_lookup(struct fsl_mc_obj_desc *obj_desc,
+					   struct fsl_mc_device *mc_bus_dev);
+
 #endif /* _FSL_MC_PRIVATE_H_ */
diff --git a/include/linux/fsl/mc.h b/include/linux/fsl/mc.h
index 975553a9f75d..54d9436600c7 100644
--- a/include/linux/fsl/mc.h
+++ b/include/linux/fsl/mc.h
@@ -403,6 +403,8 @@ struct irq_domain *fsl_mc_msi_create_irq_domain(struct fwnode_handle *fwnode,
 
 void fsl_mc_free_irqs(struct fsl_mc_device *mc_dev);
 
+struct fsl_mc_device *fsl_mc_get_endpoint(struct fsl_mc_device *mc_dev);
+
 extern struct bus_type fsl_mc_bus_type;
 
 extern struct device_type fsl_mc_bus_dprc_type;
-- 
1.9.1


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

* [PATCH net-next 3/4] dpaa2-eth: add MAC/PHY support through phylink
  2019-10-21 22:50 [PATCH net-next 0/4] dpaa2-eth: add MAC/PHY support through phylink Ioana Ciornei
  2019-10-21 22:50 ` [PATCH net-next 1/4] dpaa2-eth: update the TX frame queues on DPNI_IRQ_EVENT_ENDPOINT_CHANGED Ioana Ciornei
  2019-10-21 22:50 ` [PATCH net-next 2/4] bus: fsl-mc: add the fsl_mc_get_endpoint function Ioana Ciornei
@ 2019-10-21 22:50 ` Ioana Ciornei
  2019-10-22  1:06   ` Andrew Lunn
                     ` (2 more replies)
  2019-10-21 22:50 ` [PATCH net-next 4/4] net: documentation: add docs for MAC/PHY support in DPAA2 Ioana Ciornei
  3 siblings, 3 replies; 18+ messages in thread
From: Ioana Ciornei @ 2019-10-21 22:50 UTC (permalink / raw)
  To: davem, netdev; +Cc: laurentiu.tudor, andrew, f.fainelli, rmk, Ioana Ciornei

The dpaa2-eth driver now has support for connecting to its associated
PHY device found through standard OF bindings.

This happens when the DPNI object (that the driver probes on) gets
connected to a DPMAC. When that happens, the device tree is looked up by
the DPMAC ID, and the associated PHY bindings are found.

The old logic of handling the net device's link state by hand still
needs to be kept, as the DPNI can be connected to other devices on the
bus than a DPMAC: other DPNI, DPSW ports, etc. This logic is only
engaged when there is no DPMAC (and therefore no phylink instance)
attached.

The MC firmware support multiple type of DPMAC links: TYPE_FIXED,
TYPE_PHY. The TYPE_FIXED mode does not require any DPMAC management from
Linux side, and as such, the driver will not handle such a DPMAC.

Although PHYLINK typically handles SFP cages and in-band AN modes, for
the moment the driver only supports the RGMII interfaces found on the
LX2160A. Support for other modes will come later.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 MAINTAINERS                                        |   2 +
 drivers/net/ethernet/freescale/dpaa2/Makefile      |   2 +-
 drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c   | 117 ++++++--
 drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h   |   3 +
 .../net/ethernet/freescale/dpaa2/dpaa2-ethtool.c   |  25 ++
 drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c   | 332 +++++++++++++++++++++
 drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h   |  32 ++
 drivers/net/ethernet/freescale/dpaa2/dpmac-cmd.h   |  62 ++++
 drivers/net/ethernet/freescale/dpaa2/dpmac.c       | 149 +++++++++
 drivers/net/ethernet/freescale/dpaa2/dpmac.h       | 144 +++++++++
 10 files changed, 847 insertions(+), 21 deletions(-)
 create mode 100644 drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
 create mode 100644 drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
 create mode 100644 drivers/net/ethernet/freescale/dpaa2/dpmac-cmd.h
 create mode 100644 drivers/net/ethernet/freescale/dpaa2/dpmac.c
 create mode 100644 drivers/net/ethernet/freescale/dpaa2/dpmac.h

diff --git a/MAINTAINERS b/MAINTAINERS
index aaa6ee71c000..d0e562d3ce5b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5046,7 +5046,9 @@ M:	Ioana Radulescu <ruxandra.radulescu@nxp.com>
 L:	netdev@vger.kernel.org
 S:	Maintained
 F:	drivers/net/ethernet/freescale/dpaa2/dpaa2-eth*
+F:	drivers/net/ethernet/freescale/dpaa2/dpaa2-mac*
 F:	drivers/net/ethernet/freescale/dpaa2/dpni*
+F:	drivers/net/ethernet/freescale/dpaa2/dpmac*
 F:	drivers/net/ethernet/freescale/dpaa2/dpkg.h
 F:	drivers/net/ethernet/freescale/dpaa2/Makefile
 F:	drivers/net/ethernet/freescale/dpaa2/Kconfig
diff --git a/drivers/net/ethernet/freescale/dpaa2/Makefile b/drivers/net/ethernet/freescale/dpaa2/Makefile
index d1e78cdd512f..69184ca3b7b9 100644
--- a/drivers/net/ethernet/freescale/dpaa2/Makefile
+++ b/drivers/net/ethernet/freescale/dpaa2/Makefile
@@ -6,7 +6,7 @@
 obj-$(CONFIG_FSL_DPAA2_ETH)		+= fsl-dpaa2-eth.o
 obj-$(CONFIG_FSL_DPAA2_PTP_CLOCK)	+= fsl-dpaa2-ptp.o
 
-fsl-dpaa2-eth-objs	:= dpaa2-eth.o dpaa2-ethtool.o dpni.o
+fsl-dpaa2-eth-objs	:= dpaa2-eth.o dpaa2-ethtool.o dpni.o dpaa2-mac.o dpmac.o
 fsl-dpaa2-eth-${CONFIG_DEBUG_FS} += dpaa2-eth-debugfs.o
 fsl-dpaa2-ptp-objs	:= dpaa2-ptp.o dprtc.o
 
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
index 602d5118e928..3fd580b427d1 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
 /* Copyright 2014-2016 Freescale Semiconductor Inc.
- * Copyright 2016-2017 NXP
+ * Copyright 2016-2019 NXP
  */
 #include <linux/init.h>
 #include <linux/module.h>
@@ -1276,6 +1276,12 @@ static int link_state_update(struct dpaa2_eth_priv *priv)
 		   !!(state.options & DPNI_LINK_OPT_ASYM_PAUSE);
 	dpaa2_eth_set_rx_taildrop(priv, !tx_pause);
 
+	/* When we manage the MAC/PHY using phylink there is no need
+	 * to manually update the netif_carrier.
+	 */
+	if (priv->mac)
+		goto out;
+
 	/* Chech link state; speed / duplex changes are not treated yet */
 	if (priv->link_state.up == state.up)
 		goto out;
@@ -1312,17 +1318,21 @@ static int dpaa2_eth_open(struct net_device *net_dev)
 			   priv->dpbp_dev->obj_desc.id, priv->bpid);
 	}
 
-	/* We'll only start the txqs when the link is actually ready; make sure
-	 * we don't race against the link up notification, which may come
-	 * immediately after dpni_enable();
-	 */
-	netif_tx_stop_all_queues(net_dev);
+	if (!priv->mac) {
+		/* We'll only start the txqs when the link is actually ready;
+		 * make sure we don't race against the link up notification,
+		 * which may come immediately after dpni_enable();
+		 */
+		netif_tx_stop_all_queues(net_dev);
+
+		/* Also, explicitly set carrier off, otherwise
+		 * netif_carrier_ok() will return true and cause 'ip link show'
+		 * to report the LOWER_UP flag, even though the link
+		 * notification wasn't even received.
+		 */
+		netif_carrier_off(net_dev);
+	}
 	enable_ch_napi(priv);
-	/* Also, explicitly set carrier off, otherwise netif_carrier_ok() will
-	 * return true and cause 'ip link show' to report the LOWER_UP flag,
-	 * even though the link notification wasn't even received.
-	 */
-	netif_carrier_off(net_dev);
 
 	err = dpni_enable(priv->mc_io, 0, priv->mc_token);
 	if (err < 0) {
@@ -1330,13 +1340,17 @@ static int dpaa2_eth_open(struct net_device *net_dev)
 		goto enable_err;
 	}
 
-	/* If the DPMAC object has already processed the link up interrupt,
-	 * we have to learn the link state ourselves.
-	 */
-	err = link_state_update(priv);
-	if (err < 0) {
-		netdev_err(net_dev, "Can't update link state\n");
-		goto link_state_err;
+	if (!priv->mac) {
+		/* If the DPMAC object has already processed the link up
+		 * interrupt, we have to learn the link state ourselves.
+		 */
+		err = link_state_update(priv);
+		if (err < 0) {
+			netdev_err(net_dev, "Can't update link state\n");
+			goto link_state_err;
+		}
+	} else {
+		phylink_start(priv->mac->phylink);
 	}
 
 	return 0;
@@ -1411,8 +1425,12 @@ static int dpaa2_eth_stop(struct net_device *net_dev)
 	int dpni_enabled = 0;
 	int retries = 10;
 
-	netif_tx_stop_all_queues(net_dev);
-	netif_carrier_off(net_dev);
+	if (!priv->mac) {
+		netif_tx_stop_all_queues(net_dev);
+		netif_carrier_off(net_dev);
+	} else {
+		phylink_stop(priv->mac->phylink);
+	}
 
 	/* On dpni_disable(), the MC firmware will:
 	 * - stop MAC Rx and wait for all Rx frames to be enqueued to software
@@ -3342,12 +3360,58 @@ static int poll_link_state(void *arg)
 	return 0;
 }
 
+static int dpaa2_eth_connect_mac(struct dpaa2_eth_priv *priv)
+{
+	struct fsl_mc_device *dpni_dev, *dpmac_dev;
+	struct dpaa2_mac *mac;
+	int err;
+
+	dpni_dev = to_fsl_mc_device(priv->net_dev->dev.parent);
+	dpmac_dev = fsl_mc_get_endpoint(dpni_dev);
+	if (!dpmac_dev || dpmac_dev->dev.type != &fsl_mc_bus_dpmac_type)
+		return 0;
+
+	if (dpaa2_mac_is_type_fixed(dpmac_dev, priv->mc_io))
+		return 0;
+
+	mac = kzalloc(sizeof(struct dpaa2_mac), GFP_KERNEL);
+	if (!mac)
+		return -ENOMEM;
+
+	mac->mc_dev = dpmac_dev;
+	mac->mc_io = priv->mc_io;
+	mac->net_dev = priv->net_dev;
+
+	err = dpaa2_mac_connect(mac);
+	if (err) {
+		netdev_err(priv->net_dev, "Error connecting to the MAC endpoint\n");
+		kfree(mac);
+		return err;
+	}
+	priv->mac = mac;
+
+	return 0;
+}
+
+static void dpaa2_eth_disconnect_mac(struct dpaa2_eth_priv *priv)
+{
+	if (!priv->mac)
+		return;
+
+	rtnl_lock();
+	dpaa2_mac_disconnect(priv->mac);
+	kfree(priv->mac);
+	priv->mac = NULL;
+	rtnl_unlock();
+}
+
 static irqreturn_t dpni_irq0_handler_thread(int irq_num, void *arg)
 {
 	u32 status = ~0;
 	struct device *dev = (struct device *)arg;
 	struct fsl_mc_device *dpni_dev = to_fsl_mc_device(dev);
 	struct net_device *net_dev = dev_get_drvdata(dev);
+	struct dpaa2_eth_priv *priv = netdev_priv(net_dev);
 	int err;
 
 	err = dpni_get_irq_status(dpni_dev->mc_io, 0, dpni_dev->mc_handle,
@@ -3363,6 +3427,11 @@ static irqreturn_t dpni_irq0_handler_thread(int irq_num, void *arg)
 	if (status & DPNI_IRQ_EVENT_ENDPOINT_CHANGED) {
 		set_mac_addr(netdev_priv(net_dev));
 		update_tx_fqids(priv);
+
+		if (priv->mac)
+			dpaa2_eth_disconnect_mac(priv);
+		else
+			dpaa2_eth_connect_mac(priv);
 	}
 
 	return IRQ_HANDLED;
@@ -3539,6 +3608,10 @@ static int dpaa2_eth_probe(struct fsl_mc_device *dpni_dev)
 		priv->do_link_poll = true;
 	}
 
+	err = dpaa2_eth_connect_mac(priv);
+	if (err)
+		goto err_connect_mac;
+
 	err = register_netdev(net_dev);
 	if (err < 0) {
 		dev_err(dev, "register_netdev() failed\n");
@@ -3553,6 +3626,8 @@ static int dpaa2_eth_probe(struct fsl_mc_device *dpni_dev)
 	return 0;
 
 err_netdev_reg:
+	dpaa2_eth_disconnect_mac(priv);
+err_connect_mac:
 	if (priv->do_link_poll)
 		kthread_stop(priv->poll_thread);
 	else
@@ -3595,6 +3670,8 @@ static int dpaa2_eth_remove(struct fsl_mc_device *ls_dev)
 #ifdef CONFIG_DEBUG_FS
 	dpaa2_dbg_remove(priv);
 #endif
+	dpaa2_eth_disconnect_mac(priv);
+
 	unregister_netdev(net_dev);
 
 	if (priv->do_link_poll)
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h
index 686b651edcb2..7635db3ef903 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h
@@ -17,6 +17,7 @@
 
 #include "dpaa2-eth-trace.h"
 #include "dpaa2-eth-debugfs.h"
+#include "dpaa2-mac.h"
 
 #define DPAA2_WRIOP_VERSION(x, y, z) ((x) << 10 | (y) << 5 | (z) << 0)
 
@@ -415,6 +416,8 @@ struct dpaa2_eth_priv {
 #ifdef CONFIG_DEBUG_FS
 	struct dpaa2_debugfs dbg;
 #endif
+
+	struct dpaa2_mac *mac;
 };
 
 #define DPAA2_RXH_SUPPORTED	(RXH_L2DA | RXH_VLAN | RXH_L3_PROTO \
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c
index dc9a6c36cac0..0883620631b8 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c
@@ -85,6 +85,10 @@ static void dpaa2_eth_get_drvinfo(struct net_device *net_dev,
 {
 	struct dpaa2_eth_priv *priv = netdev_priv(net_dev);
 
+	if (priv->mac)
+		return phylink_ethtool_ksettings_get(priv->mac->phylink,
+						     link_settings);
+
 	link_settings->base.autoneg = AUTONEG_DISABLE;
 	if (!(priv->link_state.options & DPNI_LINK_OPT_HALF_DUPLEX))
 		link_settings->base.duplex = DUPLEX_FULL;
@@ -93,12 +97,29 @@ static void dpaa2_eth_get_drvinfo(struct net_device *net_dev,
 	return 0;
 }
 
+static int
+dpaa2_eth_set_link_ksettings(struct net_device *net_dev,
+			     const struct ethtool_link_ksettings *link_settings)
+{
+	struct dpaa2_eth_priv *priv = netdev_priv(net_dev);
+
+	if (!priv->mac)
+		return -ENOTSUPP;
+
+	return phylink_ethtool_ksettings_set(priv->mac->phylink, link_settings);
+}
+
 static void dpaa2_eth_get_pauseparam(struct net_device *net_dev,
 				     struct ethtool_pauseparam *pause)
 {
 	struct dpaa2_eth_priv *priv = netdev_priv(net_dev);
 	u64 link_options = priv->link_state.options;
 
+	if (priv->mac) {
+		phylink_ethtool_get_pauseparam(priv->mac->phylink, pause);
+		return;
+	}
+
 	pause->rx_pause = !!(link_options & DPNI_LINK_OPT_PAUSE);
 	pause->tx_pause = pause->rx_pause ^
 			  !!(link_options & DPNI_LINK_OPT_ASYM_PAUSE);
@@ -118,6 +139,9 @@ static int dpaa2_eth_set_pauseparam(struct net_device *net_dev,
 		return -EOPNOTSUPP;
 	}
 
+	if (priv->mac)
+		return phylink_ethtool_set_pauseparam(priv->mac->phylink,
+						      pause);
 	if (pause->autoneg)
 		return -EOPNOTSUPP;
 
@@ -728,6 +752,7 @@ static int dpaa2_eth_get_ts_info(struct net_device *dev,
 	.get_drvinfo = dpaa2_eth_get_drvinfo,
 	.get_link = ethtool_op_get_link,
 	.get_link_ksettings = dpaa2_eth_get_link_ksettings,
+	.set_link_ksettings = dpaa2_eth_set_link_ksettings,
 	.get_pauseparam = dpaa2_eth_get_pauseparam,
 	.set_pauseparam = dpaa2_eth_set_pauseparam,
 	.get_sset_count = dpaa2_eth_get_sset_count,
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
new file mode 100644
index 000000000000..3b71ae1e3966
--- /dev/null
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
@@ -0,0 +1,332 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
+/* Copyright 2019 NXP */
+
+#include "dpaa2-eth.h"
+#include "dpaa2-mac.h"
+
+#define phylink_to_dpaa2_mac(config) \
+	container_of((config), struct dpaa2_mac, phylink_config)
+
+static phy_interface_t phy_mode(enum dpmac_eth_if eth_if)
+{
+	switch (eth_if) {
+	case DPMAC_ETH_IF_RGMII:
+		return PHY_INTERFACE_MODE_RGMII;
+	default:
+		return -EINVAL;
+	}
+}
+
+/* Caller must call of_node_put on the returned value */
+static struct device_node *dpaa2_mac_get_node(u16 dpmac_id)
+{
+	struct device_node *dpmacs, *dpmac = NULL;
+	u32 id;
+	int err;
+
+	dpmacs = of_find_node_by_name(NULL, "dpmacs");
+	if (!dpmacs)
+		return NULL;
+
+	while ((dpmac = of_get_next_child(dpmacs, dpmac)) != NULL) {
+		err = of_property_read_u32(dpmac, "reg", &id);
+		if (err)
+			continue;
+		if (id == dpmac_id)
+			break;
+	}
+
+	of_node_put(dpmacs);
+
+	return dpmac;
+}
+
+static int dpaa2_mac_get_if_mode(struct device_node *node,
+				 struct dpmac_attr attr)
+{
+	int if_mode;
+
+	if_mode = of_get_phy_mode(node);
+	if (if_mode >= 0)
+		return if_mode;
+
+	if_mode = phy_mode(attr.eth_if);
+	if (if_mode >= 0)
+		return if_mode;
+
+	return -ENODEV;
+}
+
+static bool dpaa2_mac_phy_mode_mismatch(struct dpaa2_mac *mac,
+					phy_interface_t interface)
+{
+	switch (interface) {
+	case PHY_INTERFACE_MODE_RGMII:
+	case PHY_INTERFACE_MODE_RGMII_ID:
+	case PHY_INTERFACE_MODE_RGMII_RXID:
+	case PHY_INTERFACE_MODE_RGMII_TXID:
+		return (interface != mac->if_mode);
+	default:
+		return true;
+	}
+}
+
+struct dpaa2_mac_link_mode_map {
+	u64 dpmac_lm;
+	enum ethtool_link_mode_bit_indices ethtool_lm;
+};
+
+static const struct dpaa2_mac_link_mode_map dpaa2_mac_lm_map[] = {
+	{DPMAC_ADVERTISED_10BASET_FULL, ETHTOOL_LINK_MODE_10baseT_Full_BIT},
+	{DPMAC_ADVERTISED_100BASET_FULL, ETHTOOL_LINK_MODE_100baseT_Full_BIT},
+	{DPMAC_ADVERTISED_1000BASET_FULL, ETHTOOL_LINK_MODE_1000baseT_Full_BIT},
+	{DPMAC_ADVERTISED_10000BASET_FULL, ETHTOOL_LINK_MODE_10000baseT_Full_BIT},
+	{DPMAC_ADVERTISED_AUTONEG, ETHTOOL_LINK_MODE_Autoneg_BIT},
+};
+
+static void dpaa2_mac_linkmode2dpmac(unsigned long *phydev_lm,
+				     u64 *dpmac_lm)
+{
+	enum ethtool_link_mode_bit_indices link_mode;
+	int i;
+
+	*dpmac_lm = 0;
+	for (i = 0; i < ARRAY_SIZE(dpaa2_mac_lm_map); i++) {
+		link_mode = dpaa2_mac_lm_map[i].ethtool_lm;
+		if (linkmode_test_bit(link_mode, phydev_lm))
+			*dpmac_lm |= dpaa2_mac_lm_map[i].dpmac_lm;
+	}
+}
+
+static void dpaa2_mac_validate(struct phylink_config *config,
+			       unsigned long *supported,
+			       struct phylink_link_state *state)
+{
+	struct dpaa2_mac *mac = phylink_to_dpaa2_mac(config);
+	struct dpmac_link_state *dpmac_state = &mac->state;
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
+
+	if (state->interface != PHY_INTERFACE_MODE_NA &&
+	    dpaa2_mac_phy_mode_mismatch(mac, state->interface)) {
+		goto empty_set;
+	}
+
+	phylink_set_port_modes(mask);
+	phylink_set(mask, Autoneg);
+	phylink_set(mask, Pause);
+	phylink_set(mask, Asym_Pause);
+
+	switch (state->interface) {
+	case PHY_INTERFACE_MODE_RGMII:
+	case PHY_INTERFACE_MODE_RGMII_ID:
+	case PHY_INTERFACE_MODE_RGMII_RXID:
+	case PHY_INTERFACE_MODE_RGMII_TXID:
+		phylink_set(mask, 10baseT_Full);
+		phylink_set(mask, 100baseT_Full);
+		phylink_set(mask, 1000baseT_Full);
+		break;
+	default:
+		goto empty_set;
+	}
+
+	linkmode_and(supported, supported, mask);
+	linkmode_and(state->advertising, state->advertising, mask);
+
+	dpaa2_mac_linkmode2dpmac(supported, &dpmac_state->supported);
+	dpaa2_mac_linkmode2dpmac(state->advertising, &dpmac_state->advertising);
+
+	return;
+
+empty_set:
+	linkmode_zero(supported);
+}
+
+static void dpaa2_mac_config(struct phylink_config *config, unsigned int mode,
+			     const struct phylink_link_state *state)
+{
+	struct dpaa2_mac *mac = phylink_to_dpaa2_mac(config);
+	struct dpmac_link_state *dpmac_state = &mac->state;
+	int err;
+
+	if (state->speed != SPEED_UNKNOWN)
+		dpmac_state->rate = state->speed;
+
+	if (state->duplex != DUPLEX_UNKNOWN) {
+		if (!state->duplex)
+			dpmac_state->options |= DPMAC_LINK_OPT_HALF_DUPLEX;
+		else
+			dpmac_state->options &= ~DPMAC_LINK_OPT_HALF_DUPLEX;
+	}
+
+	if (state->an_enabled)
+		dpmac_state->options |= DPMAC_LINK_OPT_AUTONEG;
+	else
+		dpmac_state->options &= ~DPMAC_LINK_OPT_AUTONEG;
+
+	if (state->pause & MLO_PAUSE_RX)
+		dpmac_state->options |= DPMAC_LINK_OPT_PAUSE;
+	else
+		dpmac_state->options &= ~DPMAC_LINK_OPT_PAUSE;
+
+	if (!!(state->pause & MLO_PAUSE_RX) ^ !!(state->pause & MLO_PAUSE_TX))
+		dpmac_state->options |= DPMAC_LINK_OPT_ASYM_PAUSE;
+	else
+		dpmac_state->options &= ~DPMAC_LINK_OPT_ASYM_PAUSE;
+
+	err = dpmac_set_link_state(mac->mc_io, 0,
+				   mac->mc_dev->mc_handle, dpmac_state);
+	if (err)
+		netdev_err(mac->net_dev, "dpmac_set_link_state() = %d\n", err);
+}
+
+static void dpaa2_mac_link_up(struct phylink_config *config, unsigned int mode,
+			      phy_interface_t interface, struct phy_device *phy)
+{
+	struct dpaa2_mac *mac = phylink_to_dpaa2_mac(config);
+	struct dpmac_link_state *dpmac_state = &mac->state;
+	int err;
+
+	dpmac_state->up = 1;
+	err = dpmac_set_link_state(mac->mc_io, 0,
+				   mac->mc_dev->mc_handle, dpmac_state);
+	if (err)
+		netdev_err(mac->net_dev, "dpmac_set_link_state() = %d\n", err);
+}
+
+static void dpaa2_mac_link_down(struct phylink_config *config,
+				unsigned int mode,
+				phy_interface_t interface)
+{
+	struct dpaa2_mac *mac = phylink_to_dpaa2_mac(config);
+	struct dpmac_link_state *dpmac_state = &mac->state;
+	int err;
+
+	dpmac_state->up = 0;
+	err = dpmac_set_link_state(mac->mc_io, 0,
+				   mac->mc_dev->mc_handle, dpmac_state);
+	if (err)
+		netdev_err(mac->net_dev, "dpmac_set_link_state() = %d\n", err);
+}
+
+static const struct phylink_mac_ops dpaa2_mac_phylink_ops = {
+	.validate = dpaa2_mac_validate,
+	.mac_config = dpaa2_mac_config,
+	.mac_link_up = dpaa2_mac_link_up,
+	.mac_link_down = dpaa2_mac_link_down,
+};
+
+bool dpaa2_mac_is_type_fixed(struct fsl_mc_device *dpmac_dev,
+			     struct fsl_mc_io *mc_io)
+{
+	struct dpmac_attr attr;
+	bool fixed = false;
+	u16 mc_handle = 0;
+	int err;
+
+	err = dpmac_open(mc_io, 0, dpmac_dev->obj_desc.id,
+			 &mc_handle);
+	if (err || !mc_handle)
+		return false;
+
+	err = dpmac_get_attributes(mc_io, 0, mc_handle, &attr);
+	if (err)
+		goto out;
+
+	if (attr.link_type == DPMAC_LINK_TYPE_FIXED)
+		fixed = true;
+
+out:
+	dpmac_close(mc_io, 0, mc_handle);
+
+	return fixed;
+}
+
+int dpaa2_mac_connect(struct dpaa2_mac *mac)
+{
+	struct fsl_mc_device *dpmac_dev = mac->mc_dev;
+	struct net_device *net_dev = mac->net_dev;
+	struct device_node *dpmac_node;
+	struct phylink *phylink;
+	struct dpmac_attr attr;
+	int err;
+
+	err = dpmac_open(mac->mc_io, 0, dpmac_dev->obj_desc.id,
+			 &dpmac_dev->mc_handle);
+	if (err || !dpmac_dev->mc_handle) {
+		netdev_err(net_dev, "dpmac_open() = %d\n", err);
+		return -ENODEV;
+	}
+
+	err = dpmac_get_attributes(mac->mc_io, 0, dpmac_dev->mc_handle, &attr);
+	if (err) {
+		netdev_err(net_dev, "dpmac_get_attributes() = %d\n", err);
+		goto err_close_dpmac;
+	}
+
+	dpmac_node = dpaa2_mac_get_node(attr.id);
+	if (!dpmac_node) {
+		netdev_err(net_dev, "No dpmac@%d node found.\n", attr.id);
+		err = -ENODEV;
+		goto err_close_dpmac;
+	}
+
+	err = dpaa2_mac_get_if_mode(dpmac_node, attr);
+	if (err < 0) {
+		err = -EINVAL;
+		goto err_put_node;
+	}
+	mac->if_mode = err;
+
+	/* The MAC does not have the capability to add RGMII delays so
+	 * error out if the interface mode requests them and there is no PHY
+	 * to act upon them
+	 */
+	if (of_phy_is_fixed_link(dpmac_node) &&
+	    (mac->if_mode == PHY_INTERFACE_MODE_RGMII_ID ||
+	     mac->if_mode == PHY_INTERFACE_MODE_RGMII_RXID ||
+	     mac->if_mode == PHY_INTERFACE_MODE_RGMII_TXID)) {
+		netdev_err(net_dev, "RGMII delay not supported\n");
+		err = -EINVAL;
+		goto err_put_node;
+	}
+
+	mac->phylink_config.dev = &net_dev->dev;
+	mac->phylink_config.type = PHYLINK_NETDEV;
+
+	phylink = phylink_create(&mac->phylink_config,
+				 of_fwnode_handle(dpmac_node), mac->if_mode,
+				 &dpaa2_mac_phylink_ops);
+	if (IS_ERR(phylink)) {
+		err = PTR_ERR(phylink);
+		goto err_put_node;
+	}
+	mac->phylink = phylink;
+
+	err = phylink_of_phy_connect(mac->phylink, dpmac_node, 0);
+	if (err) {
+		netdev_err(net_dev, "phylink_of_phy_connect() = %d\n", err);
+		goto err_phylink_destroy;
+	}
+
+	of_node_put(dpmac_node);
+
+	return 0;
+
+err_phylink_destroy:
+	phylink_destroy(mac->phylink);
+err_put_node:
+	of_node_put(dpmac_node);
+err_close_dpmac:
+	dpmac_close(mac->mc_io, 0, dpmac_dev->mc_handle);
+	return err;
+}
+
+void dpaa2_mac_disconnect(struct dpaa2_mac *mac)
+{
+	if (!mac->phylink)
+		return;
+
+	phylink_disconnect_phy(mac->phylink);
+	phylink_destroy(mac->phylink);
+	dpmac_close(mac->mc_io, 0, mac->mc_dev->mc_handle);
+}
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
new file mode 100644
index 000000000000..8634d0de7ef3
--- /dev/null
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
+/* Copyright 2019 NXP */
+#ifndef DPAA2_MAC_H
+#define DPAA2_MAC_H
+
+#include <linux/of.h>
+#include <linux/of_mdio.h>
+#include <linux/of_net.h>
+#include <linux/phylink.h>
+
+#include "dpmac.h"
+#include "dpmac-cmd.h"
+
+struct dpaa2_mac {
+	struct fsl_mc_device *mc_dev;
+	struct dpmac_link_state state;
+	struct net_device *net_dev;
+	struct fsl_mc_io *mc_io;
+
+	struct phylink_config phylink_config;
+	struct phylink *phylink;
+	phy_interface_t if_mode;
+};
+
+bool dpaa2_mac_is_type_fixed(struct fsl_mc_device *dpmac_dev,
+			     struct fsl_mc_io *mc_io);
+
+int dpaa2_mac_connect(struct dpaa2_mac *mac);
+
+void dpaa2_mac_disconnect(struct dpaa2_mac *mac);
+
+#endif /* DPAA2_MAC_H */
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpmac-cmd.h b/drivers/net/ethernet/freescale/dpaa2/dpmac-cmd.h
new file mode 100644
index 000000000000..96a9b0d0992e
--- /dev/null
+++ b/drivers/net/ethernet/freescale/dpaa2/dpmac-cmd.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
+/* Copyright 2013-2016 Freescale Semiconductor Inc.
+ * Copyright 2019 NXP
+ */
+#ifndef _FSL_DPMAC_CMD_H
+#define _FSL_DPMAC_CMD_H
+
+/* DPMAC Version */
+#define DPMAC_VER_MAJOR				4
+#define DPMAC_VER_MINOR				4
+#define DPMAC_CMD_BASE_VERSION			1
+#define DPMAC_CMD_2ND_VERSION			2
+#define DPMAC_CMD_ID_OFFSET			4
+
+#define DPMAC_CMD(id)	(((id) << DPMAC_CMD_ID_OFFSET) | DPMAC_CMD_BASE_VERSION)
+#define DPMAC_CMD_V2(id) (((id) << DPMAC_CMD_ID_OFFSET) | DPMAC_CMD_2ND_VERSION)
+
+/* Command IDs */
+#define DPMAC_CMDID_CLOSE		DPMAC_CMD(0x800)
+#define DPMAC_CMDID_OPEN		DPMAC_CMD(0x80c)
+
+#define DPMAC_CMDID_GET_ATTR		DPMAC_CMD(0x004)
+#define DPMAC_CMDID_SET_LINK_STATE	DPMAC_CMD_V2(0x0c3)
+
+/* Macros for accessing command fields smaller than 1byte */
+#define DPMAC_MASK(field)        \
+	GENMASK(DPMAC_##field##_SHIFT + DPMAC_##field##_SIZE - 1, \
+		DPMAC_##field##_SHIFT)
+
+#define dpmac_set_field(var, field, val) \
+	((var) |= (((val) << DPMAC_##field##_SHIFT) & DPMAC_MASK(field)))
+#define dpmac_get_field(var, field)      \
+	(((var) & DPMAC_MASK(field)) >> DPMAC_##field##_SHIFT)
+
+struct dpmac_cmd_open {
+	__le32 dpmac_id;
+};
+
+struct dpmac_rsp_get_attributes {
+	u8 eth_if;
+	u8 link_type;
+	__le16 id;
+	__le32 max_rate;
+};
+
+#define DPMAC_STATE_SIZE	1
+#define DPMAC_STATE_SHIFT	0
+#define DPMAC_STATE_VALID_SIZE	1
+#define DPMAC_STATE_VALID_SHIFT	1
+
+struct dpmac_cmd_set_link_state {
+	__le64 options;
+	__le32 rate;
+	__le32 pad0;
+	/* from lsb: up:1, state_valid:1 */
+	u8 state;
+	u8 pad1[7];
+	__le64 supported;
+	__le64 advertising;
+};
+
+#endif /* _FSL_DPMAC_CMD_H */
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpmac.c b/drivers/net/ethernet/freescale/dpaa2/dpmac.c
new file mode 100644
index 000000000000..b75189deffb1
--- /dev/null
+++ b/drivers/net/ethernet/freescale/dpaa2/dpmac.c
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
+/* Copyright 2013-2016 Freescale Semiconductor Inc.
+ * Copyright 2019 NXP
+ */
+#include <linux/fsl/mc.h>
+#include "dpmac.h"
+#include "dpmac-cmd.h"
+
+/**
+ * dpmac_open() - Open a control session for the specified object.
+ * @mc_io:	Pointer to MC portal's I/O object
+ * @cmd_flags:	Command flags; one or more of 'MC_CMD_FLAG_'
+ * @dpmac_id:	DPMAC unique ID
+ * @token:	Returned token; use in subsequent API calls
+ *
+ * This function can be used to open a control session for an
+ * already created object; an object may have been declared in
+ * the DPL or by calling the dpmac_create function.
+ * This function returns a unique authentication token,
+ * associated with the specific object ID and the specific MC
+ * portal; this token must be used in all subsequent commands for
+ * this specific object
+ *
+ * Return:	'0' on Success; Error code otherwise.
+ */
+int dpmac_open(struct fsl_mc_io *mc_io,
+	       u32 cmd_flags,
+	       int dpmac_id,
+	       u16 *token)
+{
+	struct dpmac_cmd_open *cmd_params;
+	struct fsl_mc_command cmd = { 0 };
+	int err;
+
+	/* prepare command */
+	cmd.header = mc_encode_cmd_header(DPMAC_CMDID_OPEN,
+					  cmd_flags,
+					  0);
+	cmd_params = (struct dpmac_cmd_open *)cmd.params;
+	cmd_params->dpmac_id = cpu_to_le32(dpmac_id);
+
+	/* send command to mc*/
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+	/* retrieve response parameters */
+	*token = mc_cmd_hdr_read_token(&cmd);
+
+	return err;
+}
+
+/**
+ * dpmac_close() - Close the control session of the object
+ * @mc_io:	Pointer to MC portal's I/O object
+ * @cmd_flags:	Command flags; one or more of 'MC_CMD_FLAG_'
+ * @token:	Token of DPMAC object
+ *
+ * After this function is called, no further operations are
+ * allowed on the object without opening a new control session.
+ *
+ * Return:	'0' on Success; Error code otherwise.
+ */
+int dpmac_close(struct fsl_mc_io *mc_io,
+		u32 cmd_flags,
+		u16 token)
+{
+	struct fsl_mc_command cmd = { 0 };
+
+	/* prepare command */
+	cmd.header = mc_encode_cmd_header(DPMAC_CMDID_CLOSE, cmd_flags,
+					  token);
+
+	/* send command to mc*/
+	return mc_send_command(mc_io, &cmd);
+}
+
+/**
+ * dpmac_get_attributes - Retrieve DPMAC attributes.
+ *
+ * @mc_io:	Pointer to MC portal's I/O object
+ * @cmd_flags:	Command flags; one or more of 'MC_CMD_FLAG_'
+ * @token:	Token of DPMAC object
+ * @attr:	Returned object's attributes
+ *
+ * Return:	'0' on Success; Error code otherwise.
+ */
+int dpmac_get_attributes(struct fsl_mc_io *mc_io,
+			 u32 cmd_flags,
+			 u16 token,
+			 struct dpmac_attr *attr)
+{
+	struct dpmac_rsp_get_attributes *rsp_params;
+	struct fsl_mc_command cmd = { 0 };
+	int err;
+
+	/* prepare command */
+	cmd.header = mc_encode_cmd_header(DPMAC_CMDID_GET_ATTR,
+					  cmd_flags,
+					  token);
+
+	/* send command to mc*/
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+	/* retrieve response parameters */
+	rsp_params = (struct dpmac_rsp_get_attributes *)cmd.params;
+	attr->eth_if = rsp_params->eth_if;
+	attr->link_type = rsp_params->link_type;
+	attr->id = le16_to_cpu(rsp_params->id);
+	attr->max_rate = le32_to_cpu(rsp_params->max_rate);
+
+	return 0;
+}
+
+/**
+ * dpmac_set_link_state() - Set the Ethernet link status
+ * @mc_io:      Pointer to opaque I/O object
+ * @cmd_flags:  Command flags; one or more of 'MC_CMD_FLAG_'
+ * @token:      Token of DPMAC object
+ * @link_state: Link state configuration
+ *
+ * Return:      '0' on Success; Error code otherwise.
+ */
+int dpmac_set_link_state(struct fsl_mc_io *mc_io,
+			 u32 cmd_flags,
+			 u16 token,
+			 struct dpmac_link_state *link_state)
+{
+	struct dpmac_cmd_set_link_state *cmd_params;
+	struct fsl_mc_command cmd = { 0 };
+
+	/* prepare command */
+	cmd.header = mc_encode_cmd_header(DPMAC_CMDID_SET_LINK_STATE,
+					  cmd_flags,
+					  token);
+	cmd_params = (struct dpmac_cmd_set_link_state *)cmd.params;
+	cmd_params->options = cpu_to_le64(link_state->options);
+	cmd_params->rate = cpu_to_le32(link_state->rate);
+	dpmac_set_field(cmd_params->state, STATE, link_state->up);
+	dpmac_set_field(cmd_params->state, STATE_VALID,
+			link_state->state_valid);
+	cmd_params->supported = cpu_to_le64(link_state->supported);
+	cmd_params->advertising = cpu_to_le64(link_state->advertising);
+
+	/* send command to mc*/
+	return mc_send_command(mc_io, &cmd);
+}
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpmac.h b/drivers/net/ethernet/freescale/dpaa2/dpmac.h
new file mode 100644
index 000000000000..4efc410a479e
--- /dev/null
+++ b/drivers/net/ethernet/freescale/dpaa2/dpmac.h
@@ -0,0 +1,144 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
+/* Copyright 2013-2016 Freescale Semiconductor Inc.
+ * Copyright 2019 NXP
+ */
+#ifndef __FSL_DPMAC_H
+#define __FSL_DPMAC_H
+
+/* Data Path MAC API
+ * Contains initialization APIs and runtime control APIs for DPMAC
+ */
+
+struct fsl_mc_io;
+
+int dpmac_open(struct fsl_mc_io *mc_io,
+	       u32 cmd_flags,
+	       int dpmac_id,
+	       u16 *token);
+
+int dpmac_close(struct fsl_mc_io *mc_io,
+		u32 cmd_flags,
+		u16 token);
+
+/**
+ * enum dpmac_link_type -  DPMAC link type
+ * @DPMAC_LINK_TYPE_NONE: No link
+ * @DPMAC_LINK_TYPE_FIXED: Link is fixed type
+ * @DPMAC_LINK_TYPE_PHY: Link by PHY ID
+ * @DPMAC_LINK_TYPE_BACKPLANE: Backplane link type
+ */
+enum dpmac_link_type {
+	DPMAC_LINK_TYPE_NONE,
+	DPMAC_LINK_TYPE_FIXED,
+	DPMAC_LINK_TYPE_PHY,
+	DPMAC_LINK_TYPE_BACKPLANE
+};
+
+/**
+ * enum dpmac_eth_if - DPMAC Ethrnet interface
+ * @DPMAC_ETH_IF_MII: MII interface
+ * @DPMAC_ETH_IF_RMII: RMII interface
+ * @DPMAC_ETH_IF_SMII: SMII interface
+ * @DPMAC_ETH_IF_GMII: GMII interface
+ * @DPMAC_ETH_IF_RGMII: RGMII interface
+ * @DPMAC_ETH_IF_SGMII: SGMII interface
+ * @DPMAC_ETH_IF_QSGMII: QSGMII interface
+ * @DPMAC_ETH_IF_XAUI: XAUI interface
+ * @DPMAC_ETH_IF_XFI: XFI interface
+ * @DPMAC_ETH_IF_CAUI: CAUI interface
+ * @DPMAC_ETH_IF_1000BASEX: 1000BASEX interface
+ * @DPMAC_ETH_IF_USXGMII: USXGMII interface
+ */
+enum dpmac_eth_if {
+	DPMAC_ETH_IF_MII,
+	DPMAC_ETH_IF_RMII,
+	DPMAC_ETH_IF_SMII,
+	DPMAC_ETH_IF_GMII,
+	DPMAC_ETH_IF_RGMII,
+	DPMAC_ETH_IF_SGMII,
+	DPMAC_ETH_IF_QSGMII,
+	DPMAC_ETH_IF_XAUI,
+	DPMAC_ETH_IF_XFI,
+	DPMAC_ETH_IF_CAUI,
+	DPMAC_ETH_IF_1000BASEX,
+	DPMAC_ETH_IF_USXGMII,
+};
+
+/**
+ * struct dpmac_attr - Structure representing DPMAC attributes
+ * @id:		DPMAC object ID
+ * @max_rate:	Maximum supported rate - in Mbps
+ * @eth_if:	Ethernet interface
+ * @link_type:	link type
+ */
+struct dpmac_attr {
+	u16 id;
+	u32 max_rate;
+	enum dpmac_eth_if eth_if;
+	enum dpmac_link_type link_type;
+};
+
+int dpmac_get_attributes(struct fsl_mc_io *mc_io,
+			 u32 cmd_flags,
+			 u16 token,
+			 struct dpmac_attr *attr);
+
+/**
+ * DPMAC link configuration/state options
+ */
+
+/**
+ * Enable auto-negotiation
+ */
+#define DPMAC_LINK_OPT_AUTONEG			BIT_ULL(0)
+/**
+ * Enable half-duplex mode
+ */
+#define DPMAC_LINK_OPT_HALF_DUPLEX		BIT_ULL(1)
+/**
+ * Enable pause frames
+ */
+#define DPMAC_LINK_OPT_PAUSE			BIT_ULL(2)
+/**
+ * Enable a-symmetric pause frames
+ */
+#define DPMAC_LINK_OPT_ASYM_PAUSE		BIT_ULL(3)
+
+/**
+ * Advertised link speeds
+ */
+#define DPMAC_ADVERTISED_10BASET_FULL		BIT_ULL(0)
+#define DPMAC_ADVERTISED_100BASET_FULL		BIT_ULL(1)
+#define DPMAC_ADVERTISED_1000BASET_FULL		BIT_ULL(2)
+#define DPMAC_ADVERTISED_10000BASET_FULL	BIT_ULL(4)
+#define DPMAC_ADVERTISED_2500BASEX_FULL		BIT_ULL(5)
+
+/**
+ * Advertise auto-negotiation enable
+ */
+#define DPMAC_ADVERTISED_AUTONEG		BIT_ULL(3)
+
+/**
+ * struct dpmac_link_state - DPMAC link configuration request
+ * @rate: Rate in Mbps
+ * @options: Enable/Disable DPMAC link cfg features (bitmap)
+ * @up: Link state
+ * @state_valid: Ignore/Update the state of the link
+ * @supported: Speeds capability of the phy (bitmap)
+ * @advertising: Speeds that are advertised for autoneg (bitmap)
+ */
+struct dpmac_link_state {
+	u32 rate;
+	u64 options;
+	int up;
+	int state_valid;
+	u64 supported;
+	u64 advertising;
+};
+
+int dpmac_set_link_state(struct fsl_mc_io *mc_io,
+			 u32 cmd_flags,
+			 u16 token,
+			 struct dpmac_link_state *link_state);
+
+#endif /* __FSL_DPMAC_H */
-- 
1.9.1


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

* [PATCH net-next 4/4] net: documentation: add docs for MAC/PHY support in DPAA2
  2019-10-21 22:50 [PATCH net-next 0/4] dpaa2-eth: add MAC/PHY support through phylink Ioana Ciornei
                   ` (2 preceding siblings ...)
  2019-10-21 22:50 ` [PATCH net-next 3/4] dpaa2-eth: add MAC/PHY support through phylink Ioana Ciornei
@ 2019-10-21 22:50 ` Ioana Ciornei
  2019-10-22  8:14   ` Russell King - ARM Linux admin
  3 siblings, 1 reply; 18+ messages in thread
From: Ioana Ciornei @ 2019-10-21 22:50 UTC (permalink / raw)
  To: davem, netdev; +Cc: laurentiu.tudor, andrew, f.fainelli, rmk, Ioana Ciornei

Add documentation file for the MAC/PHY support in the DPAA2
architecture. This describes the architecture and implementation of the
interface between phylink and a DPAA2 network driver.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 .../device_drivers/freescale/dpaa2/index.rst       |   1 +
 .../freescale/dpaa2/mac-phy-support.rst            | 191 +++++++++++++++++++++
 MAINTAINERS                                        |   2 +
 3 files changed, 194 insertions(+)
 create mode 100644 Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-support.rst

diff --git a/Documentation/networking/device_drivers/freescale/dpaa2/index.rst b/Documentation/networking/device_drivers/freescale/dpaa2/index.rst
index 67bd87fe6c53..ee40fcc5ddff 100644
--- a/Documentation/networking/device_drivers/freescale/dpaa2/index.rst
+++ b/Documentation/networking/device_drivers/freescale/dpaa2/index.rst
@@ -8,3 +8,4 @@ DPAA2 Documentation
    overview
    dpio-driver
    ethernet-driver
+   mac-phy-support
diff --git a/Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-support.rst b/Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-support.rst
new file mode 100644
index 000000000000..51e6624fb774
--- /dev/null
+++ b/Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-support.rst
@@ -0,0 +1,191 @@
+.. SPDX-License-Identifier: GPL-2.0
+.. include:: <isonum.txt>
+
+=======================
+DPAA2 MAC / PHY support
+=======================
+
+:Copyright: |copy| 2019 NXP
+
+Overview
+--------
+
+The DPAA2 MAC / PHY support consists of a set of APIs that help DPAA2 network
+drivers (dpaa2-eth, dpaa2-ethsw) interract with the PHY library.
+
+DPAA2 Software Architecture
+---------------------------
+
+Among other DPAA2 objects, the fsl-mc bus exports DPNI objects (abstracting a
+network interface) and DPMAC objects (abstracting a MAC). The dpaa2-eth driver
+probes on the DPNI object and connects to and configures a DPMAC object with
+the help of phylink.
+
+Data connections may be established between a DPNI and a DPMAC, or between two
+DPNIs. Depending on the connection type, the netif_carrier_[on/off] is handled
+directly by the dpaa2-eth driver or by phylink.
+
+.. code-block:: none
+
+  Sources of abstracted link state information presented by the MC firmware
+
+                                               +--------------------------------------+
+  +------------+                  +---------+  |                           xgmac_mdio |
+  | net_device |                  | phylink |--|  +-----+  +-----+  +-----+  +-----+  |
+  +------------+                  +---------+  |  | PHY |  | PHY |  | PHY |  | PHY |  |
+        |                             |        |  +-----+  +-----+  +-----+  +-----+  |
+      +------------------------------------+   |                    External MDIO bus |
+      |            dpaa2-eth               |   +--------------------------------------+
+      +------------------------------------+
+        |                             |                                           Linux
+  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+        |                             |                                     MC firmware
+        |              /|             V
+  +----------+        / |       +----------+
+  |          |       /  |       |          |
+  |          |       |  |       |          |
+  |   DPNI   |<------|  |<------|   DPMAC  |
+  |          |       |  |       |          |
+  |          |       \  |<---+  |          |
+  +----------+        \ |    |  +----------+
+                       \|    |
+                             |
+           +--------------------------------------+
+           | MC firmware polling MAC PCS for link |
+           |  +-----+  +-----+  +-----+  +-----+  |
+           |  | PCS |  | PCS |  | PCS |  | PCS |  |
+           |  +-----+  +-----+  +-----+  +-----+  |
+           |                    Internal MDIO bus |
+           +--------------------------------------+
+
+
+Depending on an MC firmware configuration setting, each MAC may be in one of two modes:
+
+- DPMAC_LINK_TYPE_FIXED: the link state management is handled exclusively by
+  the MC firmware by polling the MAC PCS. Without the need to register a
+  phylink instance, the dpaa2-eth driver will not bind to the connected dpmac
+  object at all.
+
+- DPMAC_LINK_TYPE_PHY: The MC firmware is left waiting for link state update
+  events, but those are in fact passed strictly between the dpaa2-mac (based on
+  phylink) and its attached net_device driver (dpaa2-eth, dpaa2-ethsw),
+  effectively bypassing the firmware.
+
+Implementation
+--------------
+
+At probe time or when a DPNI's endpoint is dynamically changed, the dpaa2-eth
+is responsible to find out if the peer object is a DPMAC and if this is the
+case, to integrate it with PHYLINK using the dpaa2_mac_connect() API, which
+will do the following:
+
+ - look up the device tree for PHYLINK-compatible of binding (phy-handle)
+ - will create a PHYLINK instance associated with the received net_device
+ - connect to the PHY using phylink_of_phy_connect()
+
+The following phylink_mac_ops callback are implemented:
+
+ - .validate() will populate the supported linkmodes with the MAC capabilities
+   only when the phy_interface_t is RGMII_* (at the moment, this is the only
+   link type supported by the driver).
+
+ - .mac_config() will configure the MAC in the new configuration using the
+   dpmac_set_link_state() MC firmware API.
+
+ - .mac_link_up() / .mac_link_down() will update the MAC link using the same
+   API described above.
+
+At driver unbind() or when the DPNI object is disconnected from the DPMAC, the
+dpaa2-eth driver calls dpaa2_mac_disconnect() which will, in turn, disconnect
+from the PHY and destroy the PHYLINK instance.
+
+In case of a DPNI-DPMAC connection, an 'ip link set dev eth0 up' would start
+the following sequence of operations:
+
+(1) phylink_start() called from .dev_open().
+(2) The .mac_config() and .mac_link_up() callbacks are called by PHYLINK.
+(3) In order to configure the HW MAC, the MC Firmware API
+    dpmac_set_link_state() is called.
+(4) The firmware will eventually setup the HW MAC in the new configuration.
+(5) A netif_carrier_on() call is made directly from PHYLINK on the associated
+    net_device.
+(6) The dpaa2-eth driver handles the LINK_STATE_CHANGE irq in order to
+    enable/disable Rx taildrop based on the pause frame settings.
+
+.. code-block:: none
+
+  +---------+               +---------+
+  | PHYLINK |-------------->|  eth0   |
+  +---------+           (5) +---------+
+  (1) ^  |
+      |  |
+      |  v (2)
+  +-----------------------------------+
+  |             dpaa2-eth             |
+  +-----------------------------------+
+         |                    ^ (6)
+         |                    |
+         v (3)                |
+  +---------+---------------+---------+
+  |  DPMAC  |               |  DPNI   |
+  +---------+               +---------+
+  |            MC Firmware            |
+  +-----------------------------------+
+         |
+         |
+         v (4)
+  +-----------------------------------+
+  |             HW MAC                |
+  +-----------------------------------+
+
+In case of a DPNI-DPNI connection, a usual sequence of operations looks like
+the following:
+
+(1) ip link set dev eth0 up
+(2) The dpni_enable() MC API called on the associated fsl_mc_device.
+(3) ip link set dev eth1 up
+(4) The dpni_enable() MC API called on the associated fsl_mc_device.
+(5) The LINK_STATE_CHANGED irq is received by both instances of the dpaa2-eth
+    driver because now the operational link state is up.
+(6) The netif_carrier_on() is called on the exported net_device from
+    link_state_update().
+
+.. code-block:: none
+
+  +---------+               +---------+
+  |  eth0   |               |  eth1   |
+  +---------+               +---------+
+      |  ^                     ^  |
+      |  |                     |  |
+  (1) v  | (6)             (6) |  v (3)
+  +---------+               +---------+
+  |dpaa2-eth|               |dpaa2-eth|
+  +---------+               +---------+
+      |  ^                     ^  |
+      |  |                     |  |
+  (2) v  | (5)             (5) |  v (4)
+  +---------+---------------+---------+
+  |  DPNI   |               |  DPNI   |
+  +---------+               +---------+
+  |            MC Firmware            |
+  +-----------------------------------+
+
+
+Exported API
+------------
+
+Any DPAA2 driver that drivers endpoints of DPMAC objects should service its
+_EVENT_ENDPOINT_CHANGED irq and connect/disconnect from the associated DPMAC
+when necessary using the below listed API::
+
+ - int dpaa2_mac_connect(struct dpaa2_mac *mac);
+ - void dpaa2_mac_disconnect(struct dpaa2_mac *mac);
+
+A phylink integration is necessary only when the partner DPMAC is not of TYPE_FIXED.
+One can check for this condition using the below API::
+
+ - bool dpaa2_mac_is_type_fixed(struct fsl_mc_device *dpmac_dev,struct fsl_mc_io *mc_io);
+
+Before connection to a MAC, the caller must allocate and populate the
+dpaa2_mac structure with the associated net_device, a pointer to the MC portal
+to be used and the actual fsl_mc_device structure of the DPMAC.
diff --git a/MAINTAINERS b/MAINTAINERS
index d0e562d3ce5b..fdc3c89a4a6d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5052,6 +5052,8 @@ F:	drivers/net/ethernet/freescale/dpaa2/dpmac*
 F:	drivers/net/ethernet/freescale/dpaa2/dpkg.h
 F:	drivers/net/ethernet/freescale/dpaa2/Makefile
 F:	drivers/net/ethernet/freescale/dpaa2/Kconfig
+F:	Documentation/networking/device_drivers/freescale/dpaa2/ethernet-driver.rst
+F:	Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-support.rst
 
 DPAA2 ETHERNET SWITCH DRIVER
 M:	Ioana Radulescu <ruxandra.radulescu@nxp.com>
-- 
1.9.1


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

* Re: [PATCH net-next 2/4] bus: fsl-mc: add the fsl_mc_get_endpoint function
  2019-10-21 22:50 ` [PATCH net-next 2/4] bus: fsl-mc: add the fsl_mc_get_endpoint function Ioana Ciornei
@ 2019-10-21 23:13   ` Andrew Lunn
  2019-10-22  9:47     ` Ioana Ciornei
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Lunn @ 2019-10-21 23:13 UTC (permalink / raw)
  To: Ioana Ciornei; +Cc: davem, netdev, laurentiu.tudor, f.fainelli, rmk

Hi Ioana

> +/**
> + * dprc_get_connection() - Get connected endpoint and link status if connection
> + *			exists.
> + * @mc_io:	Pointer to MC portal's I/O object
> + * @cmd_flags:	Command flags; one or more of 'MC_CMD_FLAG_'
> + * @token:	Token of DPRC object
> + * @endpoint1:	Endpoint 1 configuration parameters
> + * @endpoint2:	Returned endpoint 2 configuration parameters
> + * @state:	Returned link state:
> + *		1 - link is up;
> + *		0 - link is down;
> + *		-1 - no connection (endpoint2 information is irrelevant)
> + *
> + * Return:     '0' on Success; -ENAVAIL if connection does not exist.

#define	ENAVAIL		119	/* No XENIX semaphores available */

This is not a semaphore.

How about

#define	ENOTCONN	107	/* Transport endpoint is not connected */

	Andrew

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

* Re: [PATCH net-next 3/4] dpaa2-eth: add MAC/PHY support through phylink
  2019-10-21 22:50 ` [PATCH net-next 3/4] dpaa2-eth: add MAC/PHY support through phylink Ioana Ciornei
@ 2019-10-22  1:06   ` Andrew Lunn
  2019-10-22 12:08     ` Ioana Ciornei
  2019-10-22 16:24     ` Russell King - ARM Linux admin
  2019-10-22  7:36     ` kbuild test robot
  2019-10-22 10:39   ` Russell King - ARM Linux admin
  2 siblings, 2 replies; 18+ messages in thread
From: Andrew Lunn @ 2019-10-22  1:06 UTC (permalink / raw)
  To: Ioana Ciornei; +Cc: davem, netdev, laurentiu.tudor, f.fainelli, rmk

Hi Ioana

> +static int dpaa2_eth_connect_mac(struct dpaa2_eth_priv *priv)
> +{
> +	struct fsl_mc_device *dpni_dev, *dpmac_dev;
> +	struct dpaa2_mac *mac;
> +	int err;
> +
> +	dpni_dev = to_fsl_mc_device(priv->net_dev->dev.parent);
> +	dpmac_dev = fsl_mc_get_endpoint(dpni_dev);
> +	if (!dpmac_dev || dpmac_dev->dev.type != &fsl_mc_bus_dpmac_type)
> +		return 0;
> +
> +	if (dpaa2_mac_is_type_fixed(dpmac_dev, priv->mc_io))
> +		return 0;
> +
> +	mac = kzalloc(sizeof(struct dpaa2_mac), GFP_KERNEL);
> +	if (!mac)
> +		return -ENOMEM;
> +
> +	mac->mc_dev = dpmac_dev;
> +	mac->mc_io = priv->mc_io;
> +	mac->net_dev = priv->net_dev;
> +
> +	err = dpaa2_mac_connect(mac);
> +	if (err) {
> +		netdev_err(priv->net_dev, "Error connecting to the MAC endpoint\n");
> +		kfree(mac);
> +		return err;
> +	}
> +	priv->mac = mac;
> +
> +	return 0;
> +}
> +
> +static void dpaa2_eth_disconnect_mac(struct dpaa2_eth_priv *priv)
> +{
> +	if (!priv->mac)
> +		return;
> +
> +	rtnl_lock();
> +	dpaa2_mac_disconnect(priv->mac);
> +	kfree(priv->mac);
> +	priv->mac = NULL;
> +	rtnl_unlock();
> +}

dpaa2_eth_connect_mac() does not take the rtnl lock.
dpaa2_eth_disconnect_mac() does. This asymmetry makes me think
something is wrong. But it could be correct....

> +/* Caller must call of_node_put on the returned value */
> +static struct device_node *dpaa2_mac_get_node(u16 dpmac_id)
> +{
> +	struct device_node *dpmacs, *dpmac = NULL;
> +	u32 id;
> +	int err;
> +
> +	dpmacs = of_find_node_by_name(NULL, "dpmacs");
> +	if (!dpmacs)
> +		return NULL;
> +
> +	while ((dpmac = of_get_next_child(dpmacs, dpmac)) != NULL) {
> +		err = of_property_read_u32(dpmac, "reg", &id);
> +		if (err)
> +			continue;
> +		if (id == dpmac_id)
> +			break;
> +	}

of_get_next_child() takes a reference on the child. So you need to
release that reference. It is better to make use of something like
for_each_child_of_node() or for_each_available_child_of_node() which
release the reference at the end of each loop, so long as you don't
break/return out of the loop.

> +
> +static void dpaa2_mac_validate(struct phylink_config *config,
> +			       unsigned long *supported,
> +			       struct phylink_link_state *state)
> +{
> +	struct dpaa2_mac *mac = phylink_to_dpaa2_mac(config);
> +	struct dpmac_link_state *dpmac_state = &mac->state;
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> +
> +	if (state->interface != PHY_INTERFACE_MODE_NA &&
> +	    dpaa2_mac_phy_mode_mismatch(mac, state->interface)) {
> +		goto empty_set;
> +	}
> +
> +	phylink_set_port_modes(mask);
> +	phylink_set(mask, Autoneg);
> +	phylink_set(mask, Pause);
> +	phylink_set(mask, Asym_Pause);
> +
> +	switch (state->interface) {
> +	case PHY_INTERFACE_MODE_RGMII:
> +	case PHY_INTERFACE_MODE_RGMII_ID:
> +	case PHY_INTERFACE_MODE_RGMII_RXID:
> +	case PHY_INTERFACE_MODE_RGMII_TXID:
> +		phylink_set(mask, 10baseT_Full);
> +		phylink_set(mask, 100baseT_Full);
> +		phylink_set(mask, 1000baseT_Full);
> +		break;
> +	default:
> +		goto empty_set;
> +	}
> +
> +	linkmode_and(supported, supported, mask);
> +	linkmode_and(state->advertising, state->advertising, mask);
> +
> +	dpaa2_mac_linkmode2dpmac(supported, &dpmac_state->supported);
> +	dpaa2_mac_linkmode2dpmac(state->advertising, &dpmac_state->advertising);

Humm. Not sure about these last two lines. Validate should be about if
the MAC can support something. I don't think you should be setting any
state here. That should happen in mac_config, when the state really is
configured.

> +
> +	return;
> +
> +empty_set:
> +	linkmode_zero(supported);
> +}
> +

  Andrew

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

* Re: [PATCH net-next 3/4] dpaa2-eth: add MAC/PHY support through phylink
  2019-10-21 22:50 ` [PATCH net-next 3/4] dpaa2-eth: add MAC/PHY support through phylink Ioana Ciornei
@ 2019-10-22  7:36     ` kbuild test robot
  2019-10-22  7:36     ` kbuild test robot
  2019-10-22 10:39   ` Russell King - ARM Linux admin
  2 siblings, 0 replies; 18+ messages in thread
From: kbuild test robot @ 2019-10-22  7:36 UTC (permalink / raw)
  To: Ioana Ciornei
  Cc: kbuild-all, davem, netdev, laurentiu.tudor, andrew, f.fainelli,
	rmk, Ioana Ciornei

[-- Attachment #1: Type: text/plain, Size: 918 bytes --]

Hi Ioana,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Ioana-Ciornei/dpaa2-eth-add-MAC-PHY-support-through-phylink/20191022-075637
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 3e78815f753ee131477ff50df82cbb2e87afda63
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-14) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> ERROR: "fsl_mc_bus_dpmac_type" [drivers/net/ethernet/freescale/dpaa2/fsl-dpaa2-eth.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 70180 bytes --]

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

* Re: [PATCH net-next 3/4] dpaa2-eth: add MAC/PHY support through phylink
@ 2019-10-22  7:36     ` kbuild test robot
  0 siblings, 0 replies; 18+ messages in thread
From: kbuild test robot @ 2019-10-22  7:36 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 942 bytes --]

Hi Ioana,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Ioana-Ciornei/dpaa2-eth-add-MAC-PHY-support-through-phylink/20191022-075637
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 3e78815f753ee131477ff50df82cbb2e87afda63
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-14) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> ERROR: "fsl_mc_bus_dpmac_type" [drivers/net/ethernet/freescale/dpaa2/fsl-dpaa2-eth.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 70180 bytes --]

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

* Re: [PATCH net-next 4/4] net: documentation: add docs for MAC/PHY support in DPAA2
  2019-10-21 22:50 ` [PATCH net-next 4/4] net: documentation: add docs for MAC/PHY support in DPAA2 Ioana Ciornei
@ 2019-10-22  8:14   ` Russell King - ARM Linux admin
  2019-10-22  9:41     ` Ioana Ciornei
  0 siblings, 1 reply; 18+ messages in thread
From: Russell King - ARM Linux admin @ 2019-10-22  8:14 UTC (permalink / raw)
  To: Ioana Ciornei; +Cc: davem, netdev, laurentiu.tudor, andrew, f.fainelli

This mentions phylink, but I never got the other patches of the series
which presumably implement this idea.

Please also note that I gave up waiting for another set, and as I now
have LX2160A hardware, I ended up writing my own version, which can be
found in my cex7 branch at:

 git://git.armlinux.org.uk/~rmk/linux-arm.git cex7

 http://git.armlinux.org.uk/cgit/linux-arm.git/log/?h=cex7

On Tue, Oct 22, 2019 at 01:50:28AM +0300, Ioana Ciornei wrote:
> Add documentation file for the MAC/PHY support in the DPAA2
> architecture. This describes the architecture and implementation of the
> interface between phylink and a DPAA2 network driver.
> 
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
>  .../device_drivers/freescale/dpaa2/index.rst       |   1 +
>  .../freescale/dpaa2/mac-phy-support.rst            | 191 +++++++++++++++++++++
>  MAINTAINERS                                        |   2 +
>  3 files changed, 194 insertions(+)
>  create mode 100644 Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-support.rst
> 
> diff --git a/Documentation/networking/device_drivers/freescale/dpaa2/index.rst b/Documentation/networking/device_drivers/freescale/dpaa2/index.rst
> index 67bd87fe6c53..ee40fcc5ddff 100644
> --- a/Documentation/networking/device_drivers/freescale/dpaa2/index.rst
> +++ b/Documentation/networking/device_drivers/freescale/dpaa2/index.rst
> @@ -8,3 +8,4 @@ DPAA2 Documentation
>     overview
>     dpio-driver
>     ethernet-driver
> +   mac-phy-support
> diff --git a/Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-support.rst b/Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-support.rst
> new file mode 100644
> index 000000000000..51e6624fb774
> --- /dev/null
> +++ b/Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-support.rst
> @@ -0,0 +1,191 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +.. include:: <isonum.txt>
> +
> +=======================
> +DPAA2 MAC / PHY support
> +=======================
> +
> +:Copyright: |copy| 2019 NXP
> +
> +Overview
> +--------
> +
> +The DPAA2 MAC / PHY support consists of a set of APIs that help DPAA2 network
> +drivers (dpaa2-eth, dpaa2-ethsw) interract with the PHY library.
> +
> +DPAA2 Software Architecture
> +---------------------------
> +
> +Among other DPAA2 objects, the fsl-mc bus exports DPNI objects (abstracting a
> +network interface) and DPMAC objects (abstracting a MAC). The dpaa2-eth driver
> +probes on the DPNI object and connects to and configures a DPMAC object with
> +the help of phylink.
> +
> +Data connections may be established between a DPNI and a DPMAC, or between two
> +DPNIs. Depending on the connection type, the netif_carrier_[on/off] is handled
> +directly by the dpaa2-eth driver or by phylink.
> +
> +.. code-block:: none
> +
> +  Sources of abstracted link state information presented by the MC firmware
> +
> +                                               +--------------------------------------+
> +  +------------+                  +---------+  |                           xgmac_mdio |
> +  | net_device |                  | phylink |--|  +-----+  +-----+  +-----+  +-----+  |
> +  +------------+                  +---------+  |  | PHY |  | PHY |  | PHY |  | PHY |  |
> +        |                             |        |  +-----+  +-----+  +-----+  +-----+  |
> +      +------------------------------------+   |                    External MDIO bus |
> +      |            dpaa2-eth               |   +--------------------------------------+
> +      +------------------------------------+
> +        |                             |                                           Linux
> +  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +        |                             |                                     MC firmware
> +        |              /|             V
> +  +----------+        / |       +----------+
> +  |          |       /  |       |          |
> +  |          |       |  |       |          |
> +  |   DPNI   |<------|  |<------|   DPMAC  |
> +  |          |       |  |       |          |
> +  |          |       \  |<---+  |          |
> +  +----------+        \ |    |  +----------+
> +                       \|    |
> +                             |
> +           +--------------------------------------+
> +           | MC firmware polling MAC PCS for link |
> +           |  +-----+  +-----+  +-----+  +-----+  |
> +           |  | PCS |  | PCS |  | PCS |  | PCS |  |
> +           |  +-----+  +-----+  +-----+  +-----+  |
> +           |                    Internal MDIO bus |
> +           +--------------------------------------+
> +
> +
> +Depending on an MC firmware configuration setting, each MAC may be in one of two modes:
> +
> +- DPMAC_LINK_TYPE_FIXED: the link state management is handled exclusively by
> +  the MC firmware by polling the MAC PCS. Without the need to register a
> +  phylink instance, the dpaa2-eth driver will not bind to the connected dpmac
> +  object at all.
> +
> +- DPMAC_LINK_TYPE_PHY: The MC firmware is left waiting for link state update
> +  events, but those are in fact passed strictly between the dpaa2-mac (based on
> +  phylink) and its attached net_device driver (dpaa2-eth, dpaa2-ethsw),
> +  effectively bypassing the firmware.
> +
> +Implementation
> +--------------
> +
> +At probe time or when a DPNI's endpoint is dynamically changed, the dpaa2-eth
> +is responsible to find out if the peer object is a DPMAC and if this is the
> +case, to integrate it with PHYLINK using the dpaa2_mac_connect() API, which
> +will do the following:
> +
> + - look up the device tree for PHYLINK-compatible of binding (phy-handle)
> + - will create a PHYLINK instance associated with the received net_device
> + - connect to the PHY using phylink_of_phy_connect()
> +
> +The following phylink_mac_ops callback are implemented:
> +
> + - .validate() will populate the supported linkmodes with the MAC capabilities
> +   only when the phy_interface_t is RGMII_* (at the moment, this is the only
> +   link type supported by the driver).
> +
> + - .mac_config() will configure the MAC in the new configuration using the
> +   dpmac_set_link_state() MC firmware API.
> +
> + - .mac_link_up() / .mac_link_down() will update the MAC link using the same
> +   API described above.
> +
> +At driver unbind() or when the DPNI object is disconnected from the DPMAC, the
> +dpaa2-eth driver calls dpaa2_mac_disconnect() which will, in turn, disconnect
> +from the PHY and destroy the PHYLINK instance.
> +
> +In case of a DPNI-DPMAC connection, an 'ip link set dev eth0 up' would start
> +the following sequence of operations:
> +
> +(1) phylink_start() called from .dev_open().
> +(2) The .mac_config() and .mac_link_up() callbacks are called by PHYLINK.
> +(3) In order to configure the HW MAC, the MC Firmware API
> +    dpmac_set_link_state() is called.
> +(4) The firmware will eventually setup the HW MAC in the new configuration.
> +(5) A netif_carrier_on() call is made directly from PHYLINK on the associated
> +    net_device.
> +(6) The dpaa2-eth driver handles the LINK_STATE_CHANGE irq in order to
> +    enable/disable Rx taildrop based on the pause frame settings.
> +
> +.. code-block:: none
> +
> +  +---------+               +---------+
> +  | PHYLINK |-------------->|  eth0   |
> +  +---------+           (5) +---------+
> +  (1) ^  |
> +      |  |
> +      |  v (2)
> +  +-----------------------------------+
> +  |             dpaa2-eth             |
> +  +-----------------------------------+
> +         |                    ^ (6)
> +         |                    |
> +         v (3)                |
> +  +---------+---------------+---------+
> +  |  DPMAC  |               |  DPNI   |
> +  +---------+               +---------+
> +  |            MC Firmware            |
> +  +-----------------------------------+
> +         |
> +         |
> +         v (4)
> +  +-----------------------------------+
> +  |             HW MAC                |
> +  +-----------------------------------+
> +
> +In case of a DPNI-DPNI connection, a usual sequence of operations looks like
> +the following:
> +
> +(1) ip link set dev eth0 up
> +(2) The dpni_enable() MC API called on the associated fsl_mc_device.
> +(3) ip link set dev eth1 up
> +(4) The dpni_enable() MC API called on the associated fsl_mc_device.
> +(5) The LINK_STATE_CHANGED irq is received by both instances of the dpaa2-eth
> +    driver because now the operational link state is up.
> +(6) The netif_carrier_on() is called on the exported net_device from
> +    link_state_update().
> +
> +.. code-block:: none
> +
> +  +---------+               +---------+
> +  |  eth0   |               |  eth1   |
> +  +---------+               +---------+
> +      |  ^                     ^  |
> +      |  |                     |  |
> +  (1) v  | (6)             (6) |  v (3)
> +  +---------+               +---------+
> +  |dpaa2-eth|               |dpaa2-eth|
> +  +---------+               +---------+
> +      |  ^                     ^  |
> +      |  |                     |  |
> +  (2) v  | (5)             (5) |  v (4)
> +  +---------+---------------+---------+
> +  |  DPNI   |               |  DPNI   |
> +  +---------+               +---------+
> +  |            MC Firmware            |
> +  +-----------------------------------+
> +
> +
> +Exported API
> +------------
> +
> +Any DPAA2 driver that drivers endpoints of DPMAC objects should service its
> +_EVENT_ENDPOINT_CHANGED irq and connect/disconnect from the associated DPMAC
> +when necessary using the below listed API::
> +
> + - int dpaa2_mac_connect(struct dpaa2_mac *mac);
> + - void dpaa2_mac_disconnect(struct dpaa2_mac *mac);
> +
> +A phylink integration is necessary only when the partner DPMAC is not of TYPE_FIXED.
> +One can check for this condition using the below API::
> +
> + - bool dpaa2_mac_is_type_fixed(struct fsl_mc_device *dpmac_dev,struct fsl_mc_io *mc_io);
> +
> +Before connection to a MAC, the caller must allocate and populate the
> +dpaa2_mac structure with the associated net_device, a pointer to the MC portal
> +to be used and the actual fsl_mc_device structure of the DPMAC.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d0e562d3ce5b..fdc3c89a4a6d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5052,6 +5052,8 @@ F:	drivers/net/ethernet/freescale/dpaa2/dpmac*
>  F:	drivers/net/ethernet/freescale/dpaa2/dpkg.h
>  F:	drivers/net/ethernet/freescale/dpaa2/Makefile
>  F:	drivers/net/ethernet/freescale/dpaa2/Kconfig
> +F:	Documentation/networking/device_drivers/freescale/dpaa2/ethernet-driver.rst
> +F:	Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-support.rst
>  
>  DPAA2 ETHERNET SWITCH DRIVER
>  M:	Ioana Radulescu <ruxandra.radulescu@nxp.com>
> -- 
> 1.9.1
> 
> 
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

* RE: [PATCH net-next 4/4] net: documentation: add docs for MAC/PHY support in DPAA2
  2019-10-22  8:14   ` Russell King - ARM Linux admin
@ 2019-10-22  9:41     ` Ioana Ciornei
  2019-10-22  9:49       ` Russell King - ARM Linux admin
  0 siblings, 1 reply; 18+ messages in thread
From: Ioana Ciornei @ 2019-10-22  9:41 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: davem, netdev, Laurentiu Tudor, andrew, f.fainelli

> Subject: Re: [PATCH net-next 4/4] net: documentation: add docs for MAC/PHY
> support in DPAA2
> 
> This mentions phylink, but I never got the other patches of the series which
> presumably implement this idea.

Hi Russell,

I copied you to the entire series. Anyhow, here is a link to the entire patch set - https://www.spinics.net/lists/netdev/msg606466.html.

> 
> Please also note that I gave up waiting for another set, and as I now have
> LX2160A hardware, I ended up writing my own version, which can be found in
> my cex7 branch at:

I looked through the branch and it seems that the approach you are going for is similar (if not exactly the same) as the one from my previous patch set that was shut down.
Also, sorry for the delay on sending another set but it takes a bit of time to consider all the implications of changing the entire architecture of the solution to better fit the upstream model.

Ioana

> 
>  git://git.armlinux.org.uk/~rmk/linux-arm.git cex7
> 
> 
> http://git.armlinux.org.uk/cgit/linux-arm.git/log/?h=cex7
> 
> On Tue, Oct 22, 2019 at 01:50:28AM +0300, Ioana Ciornei wrote:
> > Add documentation file for the MAC/PHY support in the DPAA2
> > architecture. This describes the architecture and implementation of
> > the interface between phylink and a DPAA2 network driver.
> >
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > ---
> >  .../device_drivers/freescale/dpaa2/index.rst       |   1 +
> >  .../freescale/dpaa2/mac-phy-support.rst            | 191
> +++++++++++++++++++++
> >  MAINTAINERS                                        |   2 +
> >  3 files changed, 194 insertions(+)
> >  create mode 100644
> > Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-suppor
> > t.rst
> >
> > diff --git
> > a/Documentation/networking/device_drivers/freescale/dpaa2/index.rst
> > b/Documentation/networking/device_drivers/freescale/dpaa2/index.rst
> > index 67bd87fe6c53..ee40fcc5ddff 100644
> > ---
> > a/Documentation/networking/device_drivers/freescale/dpaa2/index.rst
> > +++ b/Documentation/networking/device_drivers/freescale/dpaa2/index.rs
> > +++ t
> > @@ -8,3 +8,4 @@ DPAA2 Documentation
> >     overview
> >     dpio-driver
> >     ethernet-driver
> > +   mac-phy-support
> > diff --git
> > a/Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-supp
> > ort.rst
> > b/Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-supp
> > ort.rst
> > new file mode 100644
> > index 000000000000..51e6624fb774
> > --- /dev/null
> > +++ b/Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-
> > +++ support.rst
> > @@ -0,0 +1,191 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +.. include:: <isonum.txt>
> > +
> > +=======================
> > +DPAA2 MAC / PHY support
> > +=======================
> > +
> > +:Copyright: |copy| 2019 NXP
> > +
> > +Overview
> > +--------
> > +
> > +The DPAA2 MAC / PHY support consists of a set of APIs that help DPAA2
> > +network drivers (dpaa2-eth, dpaa2-ethsw) interract with the PHY library.
> > +
> > +DPAA2 Software Architecture
> > +---------------------------
> > +
> > +Among other DPAA2 objects, the fsl-mc bus exports DPNI objects
> > +(abstracting a network interface) and DPMAC objects (abstracting a
> > +MAC). The dpaa2-eth driver probes on the DPNI object and connects to
> > +and configures a DPMAC object with the help of phylink.
> > +
> > +Data connections may be established between a DPNI and a DPMAC, or
> > +between two DPNIs. Depending on the connection type, the
> > +netif_carrier_[on/off] is handled directly by the dpaa2-eth driver or by
> phylink.
> > +
> > +.. code-block:: none
> > +
> > +  Sources of abstracted link state information presented by the MC
> > + firmware
> > +
> > +                                               +--------------------------------------+
> > +  +------------+                  +---------+  |                           xgmac_mdio |
> > +  | net_device |                  | phylink |--|  +-----+  +-----+  +-----+  +-----+  |
> > +  +------------+                  +---------+  |  | PHY |  | PHY |  | PHY |  | PHY |  |
> > +        |                             |        |  +-----+  +-----+  +-----+  +-----+  |
> > +      +------------------------------------+   |                    External MDIO bus |
> > +      |            dpaa2-eth               |   +--------------------------------------+
> > +      +------------------------------------+
> > +        |                             |                                           Linux
> > +
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~~~~~~~~~~~~~~
> > +        |                             |                                     MC firmware
> > +        |              /|             V
> > +  +----------+        / |       +----------+
> > +  |          |       /  |       |          |
> > +  |          |       |  |       |          |
> > +  |   DPNI   |<------|  |<------|   DPMAC  |
> > +  |          |       |  |       |          |
> > +  |          |       \  |<---+  |          |
> > +  +----------+        \ |    |  +----------+
> > +                       \|    |
> > +                             |
> > +           +--------------------------------------+
> > +           | MC firmware polling MAC PCS for link |
> > +           |  +-----+  +-----+  +-----+  +-----+  |
> > +           |  | PCS |  | PCS |  | PCS |  | PCS |  |
> > +           |  +-----+  +-----+  +-----+  +-----+  |
> > +           |                    Internal MDIO bus |
> > +           +--------------------------------------+
> > +
> > +
> > +Depending on an MC firmware configuration setting, each MAC may be in
> one of two modes:
> > +
> > +- DPMAC_LINK_TYPE_FIXED: the link state management is handled
> > +exclusively by
> > +  the MC firmware by polling the MAC PCS. Without the need to
> > +register a
> > +  phylink instance, the dpaa2-eth driver will not bind to the
> > +connected dpmac
> > +  object at all.
> > +
> > +- DPMAC_LINK_TYPE_PHY: The MC firmware is left waiting for link state
> > +update
> > +  events, but those are in fact passed strictly between the dpaa2-mac
> > +(based on
> > +  phylink) and its attached net_device driver (dpaa2-eth,
> > +dpaa2-ethsw),
> > +  effectively bypassing the firmware.
> > +
> > +Implementation
> > +--------------
> > +
> > +At probe time or when a DPNI's endpoint is dynamically changed, the
> > +dpaa2-eth is responsible to find out if the peer object is a DPMAC
> > +and if this is the case, to integrate it with PHYLINK using the
> > +dpaa2_mac_connect() API, which will do the following:
> > +
> > + - look up the device tree for PHYLINK-compatible of binding
> > + (phy-handle)
> > + - will create a PHYLINK instance associated with the received
> > + net_device
> > + - connect to the PHY using phylink_of_phy_connect()
> > +
> > +The following phylink_mac_ops callback are implemented:
> > +
> > + - .validate() will populate the supported linkmodes with the MAC capabilities
> > +   only when the phy_interface_t is RGMII_* (at the moment, this is the only
> > +   link type supported by the driver).
> > +
> > + - .mac_config() will configure the MAC in the new configuration using the
> > +   dpmac_set_link_state() MC firmware API.
> > +
> > + - .mac_link_up() / .mac_link_down() will update the MAC link using the same
> > +   API described above.
> > +
> > +At driver unbind() or when the DPNI object is disconnected from the
> > +DPMAC, the dpaa2-eth driver calls dpaa2_mac_disconnect() which will,
> > +in turn, disconnect from the PHY and destroy the PHYLINK instance.
> > +
> > +In case of a DPNI-DPMAC connection, an 'ip link set dev eth0 up'
> > +would start the following sequence of operations:
> > +
> > +(1) phylink_start() called from .dev_open().
> > +(2) The .mac_config() and .mac_link_up() callbacks are called by PHYLINK.
> > +(3) In order to configure the HW MAC, the MC Firmware API
> > +    dpmac_set_link_state() is called.
> > +(4) The firmware will eventually setup the HW MAC in the new configuration.
> > +(5) A netif_carrier_on() call is made directly from PHYLINK on the associated
> > +    net_device.
> > +(6) The dpaa2-eth driver handles the LINK_STATE_CHANGE irq in order to
> > +    enable/disable Rx taildrop based on the pause frame settings.
> > +
> > +.. code-block:: none
> > +
> > +  +---------+               +---------+
> > +  | PHYLINK |-------------->|  eth0   |
> > +  +---------+           (5) +---------+
> > +  (1) ^  |
> > +      |  |
> > +      |  v (2)
> > +  +-----------------------------------+
> > +  |             dpaa2-eth             |
> > +  +-----------------------------------+
> > +         |                    ^ (6)
> > +         |                    |
> > +         v (3)                |
> > +  +---------+---------------+---------+
> > +  |  DPMAC  |               |  DPNI   |
> > +  +---------+               +---------+
> > +  |            MC Firmware            |
> > +  +-----------------------------------+
> > +         |
> > +         |
> > +         v (4)
> > +  +-----------------------------------+
> > +  |             HW MAC                |
> > +  +-----------------------------------+
> > +
> > +In case of a DPNI-DPNI connection, a usual sequence of operations
> > +looks like the following:
> > +
> > +(1) ip link set dev eth0 up
> > +(2) The dpni_enable() MC API called on the associated fsl_mc_device.
> > +(3) ip link set dev eth1 up
> > +(4) The dpni_enable() MC API called on the associated fsl_mc_device.
> > +(5) The LINK_STATE_CHANGED irq is received by both instances of the dpaa2-
> eth
> > +    driver because now the operational link state is up.
> > +(6) The netif_carrier_on() is called on the exported net_device from
> > +    link_state_update().
> > +
> > +.. code-block:: none
> > +
> > +  +---------+               +---------+
> > +  |  eth0   |               |  eth1   |
> > +  +---------+               +---------+
> > +      |  ^                     ^  |
> > +      |  |                     |  |
> > +  (1) v  | (6)             (6) |  v (3)
> > +  +---------+               +---------+
> > +  |dpaa2-eth|               |dpaa2-eth|
> > +  +---------+               +---------+
> > +      |  ^                     ^  |
> > +      |  |                     |  |
> > +  (2) v  | (5)             (5) |  v (4)
> > +  +---------+---------------+---------+
> > +  |  DPNI   |               |  DPNI   |
> > +  +---------+               +---------+
> > +  |            MC Firmware            |
> > +  +-----------------------------------+
> > +
> > +
> > +Exported API
> > +------------
> > +
> > +Any DPAA2 driver that drivers endpoints of DPMAC objects should
> > +service its _EVENT_ENDPOINT_CHANGED irq and connect/disconnect from
> > +the associated DPMAC when necessary using the below listed API::
> > +
> > + - int dpaa2_mac_connect(struct dpaa2_mac *mac);
> > + - void dpaa2_mac_disconnect(struct dpaa2_mac *mac);
> > +
> > +A phylink integration is necessary only when the partner DPMAC is not of
> TYPE_FIXED.
> > +One can check for this condition using the below API::
> > +
> > + - bool dpaa2_mac_is_type_fixed(struct fsl_mc_device
> > + *dpmac_dev,struct fsl_mc_io *mc_io);
> > +
> > +Before connection to a MAC, the caller must allocate and populate the
> > +dpaa2_mac structure with the associated net_device, a pointer to the
> > +MC portal to be used and the actual fsl_mc_device structure of the DPMAC.
> > diff --git a/MAINTAINERS b/MAINTAINERS index
> > d0e562d3ce5b..fdc3c89a4a6d 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -5052,6 +5052,8 @@ F:	drivers/net/ethernet/freescale/dpaa2/dpmac*
> >  F:	drivers/net/ethernet/freescale/dpaa2/dpkg.h
> >  F:	drivers/net/ethernet/freescale/dpaa2/Makefile
> >  F:	drivers/net/ethernet/freescale/dpaa2/Kconfig
> > +F:	Documentation/networking/device_drivers/freescale/dpaa2/ethernet-
> driver.rst
> > +F:	Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-
> support.rst
> >
> >  DPAA2 ETHERNET SWITCH DRIVER
> >  M:	Ioana Radulescu <ruxandra.radulescu@nxp.com>
> > --
> > 1.9.1
> >
> >
> >
> >
> 
> --
> RMK's Patch system:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.ar
> mlinux.org.uk%2Fdeveloper%2Fpatches%2F&amp;data=02%7C01%7Cioana.cior
> nei%40nxp.com%7C3d68212cba5c486cd92208d756c7d35d%7C686ea1d3bc2b4c
> 6fa92cd99c5c301635%7C0%7C0%7C637073288560452478&amp;sdata=JOi4zc4
> WROKWPD085JMZw4mo8a6pyVZa4JXo6wiaD%2BI%3D&amp;reserved=0
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up

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

* RE: [PATCH net-next 2/4] bus: fsl-mc: add the fsl_mc_get_endpoint function
  2019-10-21 23:13   ` Andrew Lunn
@ 2019-10-22  9:47     ` Ioana Ciornei
  0 siblings, 0 replies; 18+ messages in thread
From: Ioana Ciornei @ 2019-10-22  9:47 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: davem, netdev, Laurentiu Tudor, f.fainelli, rmk

> Hi Ioana

Hi Andrew, 

> 
> > +/**
> > + * dprc_get_connection() - Get connected endpoint and link status if
> connection
> > + *			exists.
> > + * @mc_io:	Pointer to MC portal's I/O object
> > + * @cmd_flags:	Command flags; one or more of 'MC_CMD_FLAG_'
> > + * @token:	Token of DPRC object
> > + * @endpoint1:	Endpoint 1 configuration parameters
> > + * @endpoint2:	Returned endpoint 2 configuration parameters
> > + * @state:	Returned link state:
> > + *		1 - link is up;
> > + *		0 - link is down;
> > + *		-1 - no connection (endpoint2 information is irrelevant)
> > + *
> > + * Return:     '0' on Success; -ENAVAIL if connection does not exist.
> 
> #define	ENAVAIL		119	/* No XENIX semaphores
> available */
> 
> This is not a semaphore.
> 
> How about
> 
> #define	ENOTCONN	107	/* Transport endpoint is not
> connected */
> 
> 	Andrew

ENOTCONN is a better fit here. Will change. 

Thanks,
Ioana

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

* Re: [PATCH net-next 4/4] net: documentation: add docs for MAC/PHY support in DPAA2
  2019-10-22  9:41     ` Ioana Ciornei
@ 2019-10-22  9:49       ` Russell King - ARM Linux admin
  2019-10-22 10:03         ` Ioana Ciornei
  0 siblings, 1 reply; 18+ messages in thread
From: Russell King - ARM Linux admin @ 2019-10-22  9:49 UTC (permalink / raw)
  To: Ioana Ciornei; +Cc: davem, netdev, Laurentiu Tudor, andrew, f.fainelli

On Tue, Oct 22, 2019 at 09:41:45AM +0000, Ioana Ciornei wrote:
> > Subject: Re: [PATCH net-next 4/4] net: documentation: add docs for MAC/PHY
> > support in DPAA2
> > 
> > This mentions phylink, but I never got the other patches of the series which
> > presumably implement this idea.
> 
> Hi Russell,
> 
> I copied you to the entire series. Anyhow, here is a link to the entire patch set - https://www.spinics.net/lists/netdev/msg606466.html.

Please avoid using rmk@armlinux.org.uk for patches.  dspam doesn't
know about Linux stuff there, and so files emails that contain
patches into spam.  Please use linux@armlinux.org.uk for all kernel
related work, as per my addresses in the MAINTAINERS file.

I now need to manually transfer all those emails...

> > Please also note that I gave up waiting for another set, and as I now have
> > LX2160A hardware, I ended up writing my own version, which can be found in
> > my cex7 branch at:
> 
> I looked through the branch and it seems that the approach you are going for is similar (if not exactly the same) as the one from my previous patch set that was shut down.
> Also, sorry for the delay on sending another set but it takes a bit of time to consider all the implications of changing the entire architecture of the solution to better fit the upstream model.
> 
> Ioana
> 
> > 
> >  git://git.armlinux.org.uk/~rmk/linux-arm.git cex7
> > 
> > 
> > http://git.armlinux.org.uk/cgit/linux-arm.git/log/?h=cex7
> > 
> > On Tue, Oct 22, 2019 at 01:50:28AM +0300, Ioana Ciornei wrote:
> > > Add documentation file for the MAC/PHY support in the DPAA2
> > > architecture. This describes the architecture and implementation of
> > > the interface between phylink and a DPAA2 network driver.
> > >
> > > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > > ---
> > >  .../device_drivers/freescale/dpaa2/index.rst       |   1 +
> > >  .../freescale/dpaa2/mac-phy-support.rst            | 191
> > +++++++++++++++++++++
> > >  MAINTAINERS                                        |   2 +
> > >  3 files changed, 194 insertions(+)
> > >  create mode 100644
> > > Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-suppor
> > > t.rst
> > >
> > > diff --git
> > > a/Documentation/networking/device_drivers/freescale/dpaa2/index.rst
> > > b/Documentation/networking/device_drivers/freescale/dpaa2/index.rst
> > > index 67bd87fe6c53..ee40fcc5ddff 100644
> > > ---
> > > a/Documentation/networking/device_drivers/freescale/dpaa2/index.rst
> > > +++ b/Documentation/networking/device_drivers/freescale/dpaa2/index.rs
> > > +++ t
> > > @@ -8,3 +8,4 @@ DPAA2 Documentation
> > >     overview
> > >     dpio-driver
> > >     ethernet-driver
> > > +   mac-phy-support
> > > diff --git
> > > a/Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-supp
> > > ort.rst
> > > b/Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-supp
> > > ort.rst
> > > new file mode 100644
> > > index 000000000000..51e6624fb774
> > > --- /dev/null
> > > +++ b/Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-
> > > +++ support.rst
> > > @@ -0,0 +1,191 @@
> > > +.. SPDX-License-Identifier: GPL-2.0
> > > +.. include:: <isonum.txt>
> > > +
> > > +=======================
> > > +DPAA2 MAC / PHY support
> > > +=======================
> > > +
> > > +:Copyright: |copy| 2019 NXP
> > > +
> > > +Overview
> > > +--------
> > > +
> > > +The DPAA2 MAC / PHY support consists of a set of APIs that help DPAA2
> > > +network drivers (dpaa2-eth, dpaa2-ethsw) interract with the PHY library.
> > > +
> > > +DPAA2 Software Architecture
> > > +---------------------------
> > > +
> > > +Among other DPAA2 objects, the fsl-mc bus exports DPNI objects
> > > +(abstracting a network interface) and DPMAC objects (abstracting a
> > > +MAC). The dpaa2-eth driver probes on the DPNI object and connects to
> > > +and configures a DPMAC object with the help of phylink.
> > > +
> > > +Data connections may be established between a DPNI and a DPMAC, or
> > > +between two DPNIs. Depending on the connection type, the
> > > +netif_carrier_[on/off] is handled directly by the dpaa2-eth driver or by
> > phylink.
> > > +
> > > +.. code-block:: none
> > > +
> > > +  Sources of abstracted link state information presented by the MC
> > > + firmware
> > > +
> > > +                                               +--------------------------------------+
> > > +  +------------+                  +---------+  |                           xgmac_mdio |
> > > +  | net_device |                  | phylink |--|  +-----+  +-----+  +-----+  +-----+  |
> > > +  +------------+                  +---------+  |  | PHY |  | PHY |  | PHY |  | PHY |  |
> > > +        |                             |        |  +-----+  +-----+  +-----+  +-----+  |
> > > +      +------------------------------------+   |                    External MDIO bus |
> > > +      |            dpaa2-eth               |   +--------------------------------------+
> > > +      +------------------------------------+
> > > +        |                             |                                           Linux
> > > +
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > ~~~~~~~~~~~~~~~~~~~~
> > > +        |                             |                                     MC firmware
> > > +        |              /|             V
> > > +  +----------+        / |       +----------+
> > > +  |          |       /  |       |          |
> > > +  |          |       |  |       |          |
> > > +  |   DPNI   |<------|  |<------|   DPMAC  |
> > > +  |          |       |  |       |          |
> > > +  |          |       \  |<---+  |          |
> > > +  +----------+        \ |    |  +----------+
> > > +                       \|    |
> > > +                             |
> > > +           +--------------------------------------+
> > > +           | MC firmware polling MAC PCS for link |
> > > +           |  +-----+  +-----+  +-----+  +-----+  |
> > > +           |  | PCS |  | PCS |  | PCS |  | PCS |  |
> > > +           |  +-----+  +-----+  +-----+  +-----+  |
> > > +           |                    Internal MDIO bus |
> > > +           +--------------------------------------+
> > > +
> > > +
> > > +Depending on an MC firmware configuration setting, each MAC may be in
> > one of two modes:
> > > +
> > > +- DPMAC_LINK_TYPE_FIXED: the link state management is handled
> > > +exclusively by
> > > +  the MC firmware by polling the MAC PCS. Without the need to
> > > +register a
> > > +  phylink instance, the dpaa2-eth driver will not bind to the
> > > +connected dpmac
> > > +  object at all.
> > > +
> > > +- DPMAC_LINK_TYPE_PHY: The MC firmware is left waiting for link state
> > > +update
> > > +  events, but those are in fact passed strictly between the dpaa2-mac
> > > +(based on
> > > +  phylink) and its attached net_device driver (dpaa2-eth,
> > > +dpaa2-ethsw),
> > > +  effectively bypassing the firmware.
> > > +
> > > +Implementation
> > > +--------------
> > > +
> > > +At probe time or when a DPNI's endpoint is dynamically changed, the
> > > +dpaa2-eth is responsible to find out if the peer object is a DPMAC
> > > +and if this is the case, to integrate it with PHYLINK using the
> > > +dpaa2_mac_connect() API, which will do the following:
> > > +
> > > + - look up the device tree for PHYLINK-compatible of binding
> > > + (phy-handle)
> > > + - will create a PHYLINK instance associated with the received
> > > + net_device
> > > + - connect to the PHY using phylink_of_phy_connect()
> > > +
> > > +The following phylink_mac_ops callback are implemented:
> > > +
> > > + - .validate() will populate the supported linkmodes with the MAC capabilities
> > > +   only when the phy_interface_t is RGMII_* (at the moment, this is the only
> > > +   link type supported by the driver).
> > > +
> > > + - .mac_config() will configure the MAC in the new configuration using the
> > > +   dpmac_set_link_state() MC firmware API.
> > > +
> > > + - .mac_link_up() / .mac_link_down() will update the MAC link using the same
> > > +   API described above.
> > > +
> > > +At driver unbind() or when the DPNI object is disconnected from the
> > > +DPMAC, the dpaa2-eth driver calls dpaa2_mac_disconnect() which will,
> > > +in turn, disconnect from the PHY and destroy the PHYLINK instance.
> > > +
> > > +In case of a DPNI-DPMAC connection, an 'ip link set dev eth0 up'
> > > +would start the following sequence of operations:
> > > +
> > > +(1) phylink_start() called from .dev_open().
> > > +(2) The .mac_config() and .mac_link_up() callbacks are called by PHYLINK.
> > > +(3) In order to configure the HW MAC, the MC Firmware API
> > > +    dpmac_set_link_state() is called.
> > > +(4) The firmware will eventually setup the HW MAC in the new configuration.
> > > +(5) A netif_carrier_on() call is made directly from PHYLINK on the associated
> > > +    net_device.
> > > +(6) The dpaa2-eth driver handles the LINK_STATE_CHANGE irq in order to
> > > +    enable/disable Rx taildrop based on the pause frame settings.
> > > +
> > > +.. code-block:: none
> > > +
> > > +  +---------+               +---------+
> > > +  | PHYLINK |-------------->|  eth0   |
> > > +  +---------+           (5) +---------+
> > > +  (1) ^  |
> > > +      |  |
> > > +      |  v (2)
> > > +  +-----------------------------------+
> > > +  |             dpaa2-eth             |
> > > +  +-----------------------------------+
> > > +         |                    ^ (6)
> > > +         |                    |
> > > +         v (3)                |
> > > +  +---------+---------------+---------+
> > > +  |  DPMAC  |               |  DPNI   |
> > > +  +---------+               +---------+
> > > +  |            MC Firmware            |
> > > +  +-----------------------------------+
> > > +         |
> > > +         |
> > > +         v (4)
> > > +  +-----------------------------------+
> > > +  |             HW MAC                |
> > > +  +-----------------------------------+
> > > +
> > > +In case of a DPNI-DPNI connection, a usual sequence of operations
> > > +looks like the following:
> > > +
> > > +(1) ip link set dev eth0 up
> > > +(2) The dpni_enable() MC API called on the associated fsl_mc_device.
> > > +(3) ip link set dev eth1 up
> > > +(4) The dpni_enable() MC API called on the associated fsl_mc_device.
> > > +(5) The LINK_STATE_CHANGED irq is received by both instances of the dpaa2-
> > eth
> > > +    driver because now the operational link state is up.
> > > +(6) The netif_carrier_on() is called on the exported net_device from
> > > +    link_state_update().
> > > +
> > > +.. code-block:: none
> > > +
> > > +  +---------+               +---------+
> > > +  |  eth0   |               |  eth1   |
> > > +  +---------+               +---------+
> > > +      |  ^                     ^  |
> > > +      |  |                     |  |
> > > +  (1) v  | (6)             (6) |  v (3)
> > > +  +---------+               +---------+
> > > +  |dpaa2-eth|               |dpaa2-eth|
> > > +  +---------+               +---------+
> > > +      |  ^                     ^  |
> > > +      |  |                     |  |
> > > +  (2) v  | (5)             (5) |  v (4)
> > > +  +---------+---------------+---------+
> > > +  |  DPNI   |               |  DPNI   |
> > > +  +---------+               +---------+
> > > +  |            MC Firmware            |
> > > +  +-----------------------------------+
> > > +
> > > +
> > > +Exported API
> > > +------------
> > > +
> > > +Any DPAA2 driver that drivers endpoints of DPMAC objects should
> > > +service its _EVENT_ENDPOINT_CHANGED irq and connect/disconnect from
> > > +the associated DPMAC when necessary using the below listed API::
> > > +
> > > + - int dpaa2_mac_connect(struct dpaa2_mac *mac);
> > > + - void dpaa2_mac_disconnect(struct dpaa2_mac *mac);
> > > +
> > > +A phylink integration is necessary only when the partner DPMAC is not of
> > TYPE_FIXED.
> > > +One can check for this condition using the below API::
> > > +
> > > + - bool dpaa2_mac_is_type_fixed(struct fsl_mc_device
> > > + *dpmac_dev,struct fsl_mc_io *mc_io);
> > > +
> > > +Before connection to a MAC, the caller must allocate and populate the
> > > +dpaa2_mac structure with the associated net_device, a pointer to the
> > > +MC portal to be used and the actual fsl_mc_device structure of the DPMAC.
> > > diff --git a/MAINTAINERS b/MAINTAINERS index
> > > d0e562d3ce5b..fdc3c89a4a6d 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -5052,6 +5052,8 @@ F:	drivers/net/ethernet/freescale/dpaa2/dpmac*
> > >  F:	drivers/net/ethernet/freescale/dpaa2/dpkg.h
> > >  F:	drivers/net/ethernet/freescale/dpaa2/Makefile
> > >  F:	drivers/net/ethernet/freescale/dpaa2/Kconfig
> > > +F:	Documentation/networking/device_drivers/freescale/dpaa2/ethernet-
> > driver.rst
> > > +F:	Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-
> > support.rst
> > >
> > >  DPAA2 ETHERNET SWITCH DRIVER
> > >  M:	Ioana Radulescu <ruxandra.radulescu@nxp.com>
> > > --
> > > 1.9.1
> > >
> > >
> > >
> > >
> > 
> > --
> > RMK's Patch system:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.ar
> > mlinux.org.uk%2Fdeveloper%2Fpatches%2F&amp;data=02%7C01%7Cioana.cior
> > nei%40nxp.com%7C3d68212cba5c486cd92208d756c7d35d%7C686ea1d3bc2b4c
> > 6fa92cd99c5c301635%7C0%7C0%7C637073288560452478&amp;sdata=JOi4zc4
> > WROKWPD085JMZw4mo8a6pyVZa4JXo6wiaD%2BI%3D&amp;reserved=0
> > FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> > According to speedtest.net: 11.9Mbps down 500kbps up
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

* RE: [PATCH net-next 4/4] net: documentation: add docs for MAC/PHY support in DPAA2
  2019-10-22  9:49       ` Russell King - ARM Linux admin
@ 2019-10-22 10:03         ` Ioana Ciornei
  0 siblings, 0 replies; 18+ messages in thread
From: Ioana Ciornei @ 2019-10-22 10:03 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: davem, netdev, Laurentiu Tudor, andrew, f.fainelli

> > > This mentions phylink, but I never got the other patches of the
> > > series which presumably implement this idea.
> >
> > Hi Russell,
> >
> > I copied you to the entire series. Anyhow, here is a link to the entire patch
> set -
> https://www.spinics.net/lists/netdev/msg606466.html.
> 
> Please avoid using rmk@armlinux.org.uk for patches.  dspam doesn't know
> about Linux stuff there, and so files emails that contain patches into spam.
> Please use linux@armlinux.org.uk for all kernel related work, as per my
> addresses in the MAINTAINERS file.
> 
> I now need to manually transfer all those emails...
> 

I'll keep that in mind for the future. Sorry for the trouble.

> > > Please also note that I gave up waiting for another set, and as I
> > > now have LX2160A hardware, I ended up writing my own version, which
> > > can be found in my cex7 branch at:
> >
> > I looked through the branch and it seems that the approach you are going
> for is similar (if not exactly the same) as the one from my previous patch set
> that was shut down.
> > Also, sorry for the delay on sending another set but it takes a bit of time to
> consider all the implications of changing the entire architecture of the
> solution to better fit the upstream model.
> >
> > Ioana
> >
> > >
> > >  git://git.armlinux.org.uk/~rmk/linux-arm.git cex7
> > >
> > >
> > > http://git.armlinux.org.uk/cgit/linux-arm.git/log/?h=cex7
> > >
> > > On Tue, Oct 22, 2019 at 01:50:28AM +0300, Ioana Ciornei wrote:
> > > > Add documentation file for the MAC/PHY support in the DPAA2
> > > > architecture. This describes the architecture and implementation
> > > > of the interface between phylink and a DPAA2 network driver.
> > > >
> > > > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > > > ---
> > > >  .../device_drivers/freescale/dpaa2/index.rst       |   1 +
> > > >  .../freescale/dpaa2/mac-phy-support.rst            | 191
> > > +++++++++++++++++++++
> > > >  MAINTAINERS                                        |   2 +
> > > >  3 files changed, 194 insertions(+)  create mode 100644
> > > > Documentation/networking/device_drivers/freescale/dpaa2/mac-phy-
> su
> > > > ppor
> > > > t.rst
> > > >
> > > > diff --git
> > > >
> a/Documentation/networking/device_drivers/freescale/dpaa2/index.rs
> > > > t
> > > >
> b/Documentation/networking/device_drivers/freescale/dpaa2/index.rs
> > > > t index 67bd87fe6c53..ee40fcc5ddff 100644
> > > > ---
> > > >
> a/Documentation/networking/device_drivers/freescale/dpaa2/index.rs
> > > > t
> > > > +++
> b/Documentation/networking/device_drivers/freescale/dpaa2/inde
> > > > +++ x.rs
> > > > +++ t
> > > > @@ -8,3 +8,4 @@ DPAA2 Documentation
> > > >     overview
> > > >     dpio-driver
> > > >     ethernet-driver
> > > > +   mac-phy-support
> > > > diff --git
> > > > a/Documentation/networking/device_drivers/freescale/dpaa2/mac-
> phy-
> > > > supp
> > > > ort.rst
> > > > b/Documentation/networking/device_drivers/freescale/dpaa2/mac-
> phy-
> > > > supp
> > > > ort.rst
> > > > new file mode 100644
> > > > index 000000000000..51e6624fb774
> > > > --- /dev/null
> > > > +++
> b/Documentation/networking/device_drivers/freescale/dpaa2/mac-
> > > > +++ phy-
> > > > +++ support.rst
> > > > @@ -0,0 +1,191 @@
> > > > +.. SPDX-License-Identifier: GPL-2.0 .. include:: <isonum.txt>
> > > > +
> > > > +=======================
> > > > +DPAA2 MAC / PHY support
> > > > +=======================
> > > > +
> > > > +:Copyright: |copy| 2019 NXP
> > > > +
> > > > +Overview
> > > > +--------
> > > > +
> > > > +The DPAA2 MAC / PHY support consists of a set of APIs that help
> > > > +DPAA2 network drivers (dpaa2-eth, dpaa2-ethsw) interract with the
> PHY library.
> > > > +
> > > > +DPAA2 Software Architecture
> > > > +---------------------------
> > > > +
> > > > +Among other DPAA2 objects, the fsl-mc bus exports DPNI objects
> > > > +(abstracting a network interface) and DPMAC objects (abstracting
> > > > +a MAC). The dpaa2-eth driver probes on the DPNI object and
> > > > +connects to and configures a DPMAC object with the help of phylink.
> > > > +
> > > > +Data connections may be established between a DPNI and a DPMAC,
> > > > +or between two DPNIs. Depending on the connection type, the
> > > > +netif_carrier_[on/off] is handled directly by the dpaa2-eth
> > > > +driver or by
> > > phylink.
> > > > +
> > > > +.. code-block:: none
> > > > +
> > > > +  Sources of abstracted link state information presented by the
> > > > + MC firmware
> > > > +
> > > > +                                               +--------------------------------------+
> > > > +  +------------+                  +---------+  |                           xgmac_mdio |
> > > > +  | net_device |                  | phylink |--|  +-----+  +-----+  +-----+  +-----+
> |
> > > > +  +------------+                  +---------+  |  | PHY |  | PHY |  | PHY |  | PHY |  |
> > > > +        |                             |        |  +-----+  +-----+  +-----+  +-----+  |
> > > > +      +------------------------------------+   |                    External MDIO bus |
> > > > +      |            dpaa2-eth               |   +--------------------------------------+
> > > > +      +------------------------------------+
> > > > +        |                             |                                           Linux
> > > > +
> > >
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~
> > > ~~~~~~~~~~~~~~~~~~~~
> > > > +        |                             |                                     MC firmware
> > > > +        |              /|             V
> > > > +  +----------+        / |       +----------+
> > > > +  |          |       /  |       |          |
> > > > +  |          |       |  |       |          |
> > > > +  |   DPNI   |<------|  |<------|   DPMAC  |
> > > > +  |          |       |  |       |          |
> > > > +  |          |       \  |<---+  |          |
> > > > +  +----------+        \ |    |  +----------+
> > > > +                       \|    |
> > > > +                             |
> > > > +           +--------------------------------------+
> > > > +           | MC firmware polling MAC PCS for link |
> > > > +           |  +-----+  +-----+  +-----+  +-----+  |
> > > > +           |  | PCS |  | PCS |  | PCS |  | PCS |  |
> > > > +           |  +-----+  +-----+  +-----+  +-----+  |
> > > > +           |                    Internal MDIO bus |
> > > > +           +--------------------------------------+
> > > > +
> > > > +
> > > > +Depending on an MC firmware configuration setting, each MAC may
> > > > +be in
> > > one of two modes:
> > > > +
> > > > +- DPMAC_LINK_TYPE_FIXED: the link state management is handled
> > > > +exclusively by
> > > > +  the MC firmware by polling the MAC PCS. Without the need to
> > > > +register a
> > > > +  phylink instance, the dpaa2-eth driver will not bind to the
> > > > +connected dpmac
> > > > +  object at all.
> > > > +
> > > > +- DPMAC_LINK_TYPE_PHY: The MC firmware is left waiting for link
> > > > +state update
> > > > +  events, but those are in fact passed strictly between the
> > > > +dpaa2-mac (based on
> > > > +  phylink) and its attached net_device driver (dpaa2-eth,
> > > > +dpaa2-ethsw),
> > > > +  effectively bypassing the firmware.
> > > > +
> > > > +Implementation
> > > > +--------------
> > > > +
> > > > +At probe time or when a DPNI's endpoint is dynamically changed,
> > > > +the dpaa2-eth is responsible to find out if the peer object is a
> > > > +DPMAC and if this is the case, to integrate it with PHYLINK using
> > > > +the
> > > > +dpaa2_mac_connect() API, which will do the following:
> > > > +
> > > > + - look up the device tree for PHYLINK-compatible of binding
> > > > + (phy-handle)
> > > > + - will create a PHYLINK instance associated with the received
> > > > + net_device
> > > > + - connect to the PHY using phylink_of_phy_connect()
> > > > +
> > > > +The following phylink_mac_ops callback are implemented:
> > > > +
> > > > + - .validate() will populate the supported linkmodes with the MAC
> capabilities
> > > > +   only when the phy_interface_t is RGMII_* (at the moment, this is
> the only
> > > > +   link type supported by the driver).
> > > > +
> > > > + - .mac_config() will configure the MAC in the new configuration using
> the
> > > > +   dpmac_set_link_state() MC firmware API.
> > > > +
> > > > + - .mac_link_up() / .mac_link_down() will update the MAC link using
> the same
> > > > +   API described above.
> > > > +
> > > > +At driver unbind() or when the DPNI object is disconnected from
> > > > +the DPMAC, the dpaa2-eth driver calls dpaa2_mac_disconnect()
> > > > +which will, in turn, disconnect from the PHY and destroy the PHYLINK
> instance.
> > > > +
> > > > +In case of a DPNI-DPMAC connection, an 'ip link set dev eth0 up'
> > > > +would start the following sequence of operations:
> > > > +
> > > > +(1) phylink_start() called from .dev_open().
> > > > +(2) The .mac_config() and .mac_link_up() callbacks are called by
> PHYLINK.
> > > > +(3) In order to configure the HW MAC, the MC Firmware API
> > > > +    dpmac_set_link_state() is called.
> > > > +(4) The firmware will eventually setup the HW MAC in the new
> configuration.
> > > > +(5) A netif_carrier_on() call is made directly from PHYLINK on the
> associated
> > > > +    net_device.
> > > > +(6) The dpaa2-eth driver handles the LINK_STATE_CHANGE irq in
> order to
> > > > +    enable/disable Rx taildrop based on the pause frame settings.
> > > > +
> > > > +.. code-block:: none
> > > > +
> > > > +  +---------+               +---------+
> > > > +  | PHYLINK |-------------->|  eth0   |
> > > > +  +---------+           (5) +---------+
> > > > +  (1) ^  |
> > > > +      |  |
> > > > +      |  v (2)
> > > > +  +-----------------------------------+
> > > > +  |             dpaa2-eth             |
> > > > +  +-----------------------------------+
> > > > +         |                    ^ (6)
> > > > +         |                    |
> > > > +         v (3)                |
> > > > +  +---------+---------------+---------+
> > > > +  |  DPMAC  |               |  DPNI   |
> > > > +  +---------+               +---------+
> > > > +  |            MC Firmware            |
> > > > +  +-----------------------------------+
> > > > +         |
> > > > +         |
> > > > +         v (4)
> > > > +  +-----------------------------------+
> > > > +  |             HW MAC                |
> > > > +  +-----------------------------------+
> > > > +
> > > > +In case of a DPNI-DPNI connection, a usual sequence of operations
> > > > +looks like the following:
> > > > +
> > > > +(1) ip link set dev eth0 up
> > > > +(2) The dpni_enable() MC API called on the associated fsl_mc_device.
> > > > +(3) ip link set dev eth1 up
> > > > +(4) The dpni_enable() MC API called on the associated fsl_mc_device.
> > > > +(5) The LINK_STATE_CHANGED irq is received by both instances of
> > > > +the dpaa2-
> > > eth
> > > > +    driver because now the operational link state is up.
> > > > +(6) The netif_carrier_on() is called on the exported net_device from
> > > > +    link_state_update().
> > > > +
> > > > +.. code-block:: none
> > > > +
> > > > +  +---------+               +---------+
> > > > +  |  eth0   |               |  eth1   |
> > > > +  +---------+               +---------+
> > > > +      |  ^                     ^  |
> > > > +      |  |                     |  |
> > > > +  (1) v  | (6)             (6) |  v (3)
> > > > +  +---------+               +---------+
> > > > +  |dpaa2-eth|               |dpaa2-eth|
> > > > +  +---------+               +---------+
> > > > +      |  ^                     ^  |
> > > > +      |  |                     |  |
> > > > +  (2) v  | (5)             (5) |  v (4)
> > > > +  +---------+---------------+---------+
> > > > +  |  DPNI   |               |  DPNI   |
> > > > +  +---------+               +---------+
> > > > +  |            MC Firmware            |
> > > > +  +-----------------------------------+
> > > > +
> > > > +
> > > > +Exported API
> > > > +------------
> > > > +
> > > > +Any DPAA2 driver that drivers endpoints of DPMAC objects should
> > > > +service its _EVENT_ENDPOINT_CHANGED irq and connect/disconnect
> > > > +from the associated DPMAC when necessary using the below listed
> API::
> > > > +
> > > > + - int dpaa2_mac_connect(struct dpaa2_mac *mac);
> > > > + - void dpaa2_mac_disconnect(struct dpaa2_mac *mac);
> > > > +
> > > > +A phylink integration is necessary only when the partner DPMAC is
> > > > +not of
> > > TYPE_FIXED.
> > > > +One can check for this condition using the below API::
> > > > +
> > > > + - bool dpaa2_mac_is_type_fixed(struct fsl_mc_device
> > > > + *dpmac_dev,struct fsl_mc_io *mc_io);
> > > > +
> > > > +Before connection to a MAC, the caller must allocate and populate
> > > > +the dpaa2_mac structure with the associated net_device, a pointer
> > > > +to the MC portal to be used and the actual fsl_mc_device structure of
> the DPMAC.
> > > > diff --git a/MAINTAINERS b/MAINTAINERS index
> > > > d0e562d3ce5b..fdc3c89a4a6d 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -5052,6 +5052,8 @@ F:
> 	drivers/net/ethernet/freescale/dpaa2/dpmac*
> > > >  F:	drivers/net/ethernet/freescale/dpaa2/dpkg.h
> > > >  F:	drivers/net/ethernet/freescale/dpaa2/Makefile
> > > >  F:	drivers/net/ethernet/freescale/dpaa2/Kconfig
> > > > +F:
> 	Documentation/networking/device_drivers/freescale/dpaa2/ethern
> et-
> > > driver.rst
> > > > +F:
> 	Documentation/networking/device_drivers/freescale/dpaa2/mac-
> phy-
> > > support.rst
> > > >
> > > >  DPAA2 ETHERNET SWITCH DRIVER
> > > >  M:	Ioana Radulescu <ruxandra.radulescu@nxp.com>
> > > > --
> > > > 1.9.1
> > > >
> > > >
> > > >
> > > >
> > >
> > > --
> > > RMK's Patch system:
> > >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fww
> > > w.ar
> > >
> mlinux.org.uk%2Fdeveloper%2Fpatches%2F&amp;data=02%7C01%7Cioana.c
> ior
> > >
> nei%40nxp.com%7C3d68212cba5c486cd92208d756c7d35d%7C686ea1d3bc2b4
> c
> > >
> 6fa92cd99c5c301635%7C0%7C0%7C637073288560452478&amp;sdata=JOi4zc4
> > > WROKWPD085JMZw4mo8a6pyVZa4JXo6wiaD%2BI%3D&amp;reserved=0
> > > FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down
> > > 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
> >
> 
> --
> RMK's Patch system:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww
> .armlinux.org.uk%2Fdeveloper%2Fpatches%2F&amp;data=02%7C01%7Cioan
> a.ciornei%40nxp.com%7Cde221afe23be4819e4a008d756d53174%7C686ea1d3
> bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637073345987966962&amp;sdata=
> TlljIAmAnmc9nIfnAwXZmIO03XbO94PL9x9P0oI94Zg%3D&amp;reserved=0
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps
> up According to speedtest.net: 11.9Mbps down 500kbps up

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

* Re: [PATCH net-next 3/4] dpaa2-eth: add MAC/PHY support through phylink
  2019-10-21 22:50 ` [PATCH net-next 3/4] dpaa2-eth: add MAC/PHY support through phylink Ioana Ciornei
  2019-10-22  1:06   ` Andrew Lunn
  2019-10-22  7:36     ` kbuild test robot
@ 2019-10-22 10:39   ` Russell King - ARM Linux admin
  2019-10-22 10:52     ` Ioana Ciornei
  2 siblings, 1 reply; 18+ messages in thread
From: Russell King - ARM Linux admin @ 2019-10-22 10:39 UTC (permalink / raw)
  To: Ioana Ciornei; +Cc: davem, netdev, laurentiu.tudor, andrew, f.fainelli

On Tue, Oct 22, 2019 at 01:50:27AM +0300, Ioana Ciornei wrote:
> +	mac->phylink_config.dev = &net_dev->dev;
> +	mac->phylink_config.type = PHYLINK_NETDEV;
> +
> +	phylink = phylink_create(&mac->phylink_config,
> +				 of_fwnode_handle(dpmac_node), mac->if_mode,
> +				 &dpaa2_mac_phylink_ops);

Does this mean we didn't need to go through the removal of netdev from
phylink after all?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

* RE: [PATCH net-next 3/4] dpaa2-eth: add MAC/PHY support through phylink
  2019-10-22 10:39   ` Russell King - ARM Linux admin
@ 2019-10-22 10:52     ` Ioana Ciornei
  0 siblings, 0 replies; 18+ messages in thread
From: Ioana Ciornei @ 2019-10-22 10:52 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: davem, netdev, Laurentiu Tudor, andrew, f.fainelli

> Subject: Re: [PATCH net-next 3/4] dpaa2-eth: add MAC/PHY support through
> phylink
> 
> On Tue, Oct 22, 2019 at 01:50:27AM +0300, Ioana Ciornei wrote:
> > +	mac->phylink_config.dev = &net_dev->dev;
> > +	mac->phylink_config.type = PHYLINK_NETDEV;
> > +
> > +	phylink = phylink_create(&mac->phylink_config,
> > +				 of_fwnode_handle(dpmac_node), mac-
> >if_mode,
> > +				 &dpaa2_mac_phylink_ops);
> 
> Does this mean we didn't need to go through the removal of netdev from
> phylink after all?
> 

No, it doesn't mean that.

The change was accepted because it added support for multi-gbps speeds on
DSA CPU ports by using a phylink instance based on the dsa_switch device.
Maxime's initial mail is here - https://www.spinics.net/lists/netdev/msg570450.html.

Ioana


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

* RE: [PATCH net-next 3/4] dpaa2-eth: add MAC/PHY support through phylink
  2019-10-22  1:06   ` Andrew Lunn
@ 2019-10-22 12:08     ` Ioana Ciornei
  2019-10-22 16:24     ` Russell King - ARM Linux admin
  1 sibling, 0 replies; 18+ messages in thread
From: Ioana Ciornei @ 2019-10-22 12:08 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: davem, netdev, Laurentiu Tudor, f.fainelli, rmk

> Hi Ioana
> 

Hi Andrew

> > +static int dpaa2_eth_connect_mac(struct dpaa2_eth_priv *priv) {
> > +	struct fsl_mc_device *dpni_dev, *dpmac_dev;
> > +	struct dpaa2_mac *mac;
> > +	int err;
> > +
> > +	dpni_dev = to_fsl_mc_device(priv->net_dev->dev.parent);
> > +	dpmac_dev = fsl_mc_get_endpoint(dpni_dev);
> > +	if (!dpmac_dev || dpmac_dev->dev.type !=
> &fsl_mc_bus_dpmac_type)
> > +		return 0;
> > +
> > +	if (dpaa2_mac_is_type_fixed(dpmac_dev, priv->mc_io))
> > +		return 0;
> > +
> > +	mac = kzalloc(sizeof(struct dpaa2_mac), GFP_KERNEL);
> > +	if (!mac)
> > +		return -ENOMEM;
> > +
> > +	mac->mc_dev = dpmac_dev;
> > +	mac->mc_io = priv->mc_io;
> > +	mac->net_dev = priv->net_dev;
> > +
> > +	err = dpaa2_mac_connect(mac);
> > +	if (err) {
> > +		netdev_err(priv->net_dev, "Error connecting to the MAC
> endpoint\n");
> > +		kfree(mac);
> > +		return err;
> > +	}
> > +	priv->mac = mac;
> > +
> > +	return 0;
> > +}
> > +
> > +static void dpaa2_eth_disconnect_mac(struct dpaa2_eth_priv *priv) {
> > +	if (!priv->mac)
> > +		return;
> > +
> > +	rtnl_lock();
> > +	dpaa2_mac_disconnect(priv->mac);
> > +	kfree(priv->mac);
> > +	priv->mac = NULL;
> > +	rtnl_unlock();
> > +}
> 
> dpaa2_eth_connect_mac() does not take the rtnl lock.
> dpaa2_eth_disconnect_mac() does. This asymmetry makes me think
> something is wrong. But it could be correct....

It seems that phylink_of_phy_connect() does not do an ASSERT_RTNL() as phylink_disconnect_phy() does and that is why I didn't catch this.

Should I submit a patch adding the assert in phylink_of_phy_connect() and  phylink_of_phy_connect() ?
Also, I'll fix the asymmetry in the dpaa2-eth in v2.

> 
> > +/* Caller must call of_node_put on the returned value */ static
> > +struct device_node *dpaa2_mac_get_node(u16 dpmac_id) {
> > +	struct device_node *dpmacs, *dpmac = NULL;
> > +	u32 id;
> > +	int err;
> > +
> > +	dpmacs = of_find_node_by_name(NULL, "dpmacs");
> > +	if (!dpmacs)
> > +		return NULL;
> > +
> > +	while ((dpmac = of_get_next_child(dpmacs, dpmac)) != NULL) {
> > +		err = of_property_read_u32(dpmac, "reg", &id);
> > +		if (err)
> > +			continue;
> > +		if (id == dpmac_id)
> > +			break;
> > +	}
> 
> of_get_next_child() takes a reference on the child. So you need to release
> that reference. It is better to make use of something like
> for_each_child_of_node() or for_each_available_child_of_node() which
> release the reference at the end of each loop, so long as you don't
> break/return out of the loop.

It seems that of_get_next_child() also releases the reference took on the
previous node in each iteration of the loop. At the end of this function,
the only reference still held is the one to the node to be returned which
is ok because the caller should release it after usage.
( A function comment is added to specify this.)

Am I missing something here?

> 
> > +
> > +static void dpaa2_mac_validate(struct phylink_config *config,
> > +			       unsigned long *supported,
> > +			       struct phylink_link_state *state) {
> > +	struct dpaa2_mac *mac = phylink_to_dpaa2_mac(config);
> > +	struct dpmac_link_state *dpmac_state = &mac->state;
> > +	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> > +
> > +	if (state->interface != PHY_INTERFACE_MODE_NA &&
> > +	    dpaa2_mac_phy_mode_mismatch(mac, state->interface)) {
> > +		goto empty_set;
> > +	}
> > +
> > +	phylink_set_port_modes(mask);
> > +	phylink_set(mask, Autoneg);
> > +	phylink_set(mask, Pause);
> > +	phylink_set(mask, Asym_Pause);
> > +
> > +	switch (state->interface) {
> > +	case PHY_INTERFACE_MODE_RGMII:
> > +	case PHY_INTERFACE_MODE_RGMII_ID:
> > +	case PHY_INTERFACE_MODE_RGMII_RXID:
> > +	case PHY_INTERFACE_MODE_RGMII_TXID:
> > +		phylink_set(mask, 10baseT_Full);
> > +		phylink_set(mask, 100baseT_Full);
> > +		phylink_set(mask, 1000baseT_Full);
> > +		break;
> > +	default:
> > +		goto empty_set;
> > +	}
> > +
> > +	linkmode_and(supported, supported, mask);
> > +	linkmode_and(state->advertising, state->advertising, mask);
> > +
> > +	dpaa2_mac_linkmode2dpmac(supported, &dpmac_state-
> >supported);
> > +	dpaa2_mac_linkmode2dpmac(state->advertising,
> > +&dpmac_state->advertising);
> 
> Humm. Not sure about these last two lines. Validate should be about if the
> MAC can support something. I don't think you should be setting any state
> here. That should happen in mac_config, when the state really is configured.
> 

I agree with the fact that .validate() is not the best place to setup these. 
Those two lines are there more because the API requests a proper supported/advertised (which had a specific purpose in the previous solution with two separate drivers).
I'll check if we can just omit them.

Thanks,
Ioana

> > +
> > +	return;
> > +
> > +empty_set:
> > +	linkmode_zero(supported);
> > +}
> > +
> 
>   Andrew

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

* Re: [PATCH net-next 3/4] dpaa2-eth: add MAC/PHY support through phylink
  2019-10-22  1:06   ` Andrew Lunn
  2019-10-22 12:08     ` Ioana Ciornei
@ 2019-10-22 16:24     ` Russell King - ARM Linux admin
  1 sibling, 0 replies; 18+ messages in thread
From: Russell King - ARM Linux admin @ 2019-10-22 16:24 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Ioana Ciornei, davem, netdev, laurentiu.tudor, f.fainelli

On Tue, Oct 22, 2019 at 03:06:49AM +0200, Andrew Lunn wrote:
> Hi Ioana
> 
> > +static int dpaa2_eth_connect_mac(struct dpaa2_eth_priv *priv)
> > +{
> > +	struct fsl_mc_device *dpni_dev, *dpmac_dev;
> > +	struct dpaa2_mac *mac;
> > +	int err;
> > +
> > +	dpni_dev = to_fsl_mc_device(priv->net_dev->dev.parent);
> > +	dpmac_dev = fsl_mc_get_endpoint(dpni_dev);
> > +	if (!dpmac_dev || dpmac_dev->dev.type != &fsl_mc_bus_dpmac_type)
> > +		return 0;
> > +
> > +	if (dpaa2_mac_is_type_fixed(dpmac_dev, priv->mc_io))
> > +		return 0;
> > +
> > +	mac = kzalloc(sizeof(struct dpaa2_mac), GFP_KERNEL);
> > +	if (!mac)
> > +		return -ENOMEM;
> > +
> > +	mac->mc_dev = dpmac_dev;
> > +	mac->mc_io = priv->mc_io;
> > +	mac->net_dev = priv->net_dev;
> > +
> > +	err = dpaa2_mac_connect(mac);
> > +	if (err) {
> > +		netdev_err(priv->net_dev, "Error connecting to the MAC endpoint\n");
> > +		kfree(mac);
> > +		return err;
> > +	}
> > +	priv->mac = mac;
> > +
> > +	return 0;
> > +}
> > +
> > +static void dpaa2_eth_disconnect_mac(struct dpaa2_eth_priv *priv)
> > +{
> > +	if (!priv->mac)
> > +		return;
> > +
> > +	rtnl_lock();
> > +	dpaa2_mac_disconnect(priv->mac);
> > +	kfree(priv->mac);
> > +	priv->mac = NULL;
> > +	rtnl_unlock();
> > +}
> 
> dpaa2_eth_connect_mac() does not take the rtnl lock.
> dpaa2_eth_disconnect_mac() does. This asymmetry makes me think
> something is wrong. But it could be correct....

The way the driver is written, it's fine.

dpaa2_eth_connect_mac() is called prior to the netdev being registered.
At that point, nothing is published.

dpaa2_eth_disconnect_mac() is called _prior_ to the netdev being
unregistered, so there could be live accesses happening to the phy
and phylink.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

end of thread, other threads:[~2019-10-22 16:24 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-21 22:50 [PATCH net-next 0/4] dpaa2-eth: add MAC/PHY support through phylink Ioana Ciornei
2019-10-21 22:50 ` [PATCH net-next 1/4] dpaa2-eth: update the TX frame queues on DPNI_IRQ_EVENT_ENDPOINT_CHANGED Ioana Ciornei
2019-10-21 22:50 ` [PATCH net-next 2/4] bus: fsl-mc: add the fsl_mc_get_endpoint function Ioana Ciornei
2019-10-21 23:13   ` Andrew Lunn
2019-10-22  9:47     ` Ioana Ciornei
2019-10-21 22:50 ` [PATCH net-next 3/4] dpaa2-eth: add MAC/PHY support through phylink Ioana Ciornei
2019-10-22  1:06   ` Andrew Lunn
2019-10-22 12:08     ` Ioana Ciornei
2019-10-22 16:24     ` Russell King - ARM Linux admin
2019-10-22  7:36   ` kbuild test robot
2019-10-22  7:36     ` kbuild test robot
2019-10-22 10:39   ` Russell King - ARM Linux admin
2019-10-22 10:52     ` Ioana Ciornei
2019-10-21 22:50 ` [PATCH net-next 4/4] net: documentation: add docs for MAC/PHY support in DPAA2 Ioana Ciornei
2019-10-22  8:14   ` Russell King - ARM Linux admin
2019-10-22  9:41     ` Ioana Ciornei
2019-10-22  9:49       ` Russell King - ARM Linux admin
2019-10-22 10:03         ` Ioana Ciornei

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.