[PATCH net-next 0/5] Flow control for NXP ENETC

[PATCH net-next 0/5] Flow control for NXP ENETC

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

This patch series contains logic for enabling the lossless mode on the
RX rings of the ENETC, and the PAUSE thresholds on the internal FIFO
memory.

During testing it was found that, with the default FIFO configuration,
a sender which isn't persuaded by our PAUSE frames and keeps sending
will cause some MAC RX frame errors. To mitigate this, we need to ensure
that the FIFO never runs completely full, so we need to fix up a setting
that was supposed to be configured well out of reset. Unfortunately this
requires the addition of a new mini-driver.

Vladimir Oltean (5):
  net: enetc: create a common enetc_pf_to_port helper
  dt-bindings: net: fsl: enetc: add the IERB documentation
  net: enetc: add a mini driver for the Integrated Endpoint Register
    Block
  arm64: dts: ls1028a: declare the Integrated Endpoint Register Block
    node
  net: enetc: add support for flow control

 .../devicetree/bindings/net/fsl-enetc.txt     |  15 ++
 .../arm64/boot/dts/freescale/fsl-ls1028a.dtsi |   6 +
 drivers/net/ethernet/freescale/enetc/Kconfig  |   9 +
 drivers/net/ethernet/freescale/enetc/Makefile |   3 +
 drivers/net/ethernet/freescale/enetc/enetc.h  |  16 ++
 .../ethernet/freescale/enetc/enetc_ethtool.c  |  18 ++
 .../net/ethernet/freescale/enetc/enetc_hw.h   |   9 +
 .../net/ethernet/freescale/enetc/enetc_ierb.c | 155 ++++++++++++++++++
 .../net/ethernet/freescale/enetc/enetc_ierb.h |  20 +++
 .../net/ethernet/freescale/enetc/enetc_pf.c   |  95 ++++++++++-
 .../net/ethernet/freescale/enetc/enetc_qos.c  |  16 +-
 11 files changed, 349 insertions(+), 13 deletions(-)
 create mode 100644 drivers/net/ethernet/freescale/enetc/enetc_ierb.c
 create mode 100644 drivers/net/ethernet/freescale/enetc/enetc_ierb.h

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH net-next 1/5] net: enetc: create a common enetc_pf_to_port helper

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Even though ENETC interfaces are exposed as individual PCIe PFs with
their own driver instances, the ENETC is still fundamentally a
multi-port Ethernet controller, and some parts of the IP take a port
number (as can be seen in the PSFP implementation).

Create a common helper that can be used outside of the TSN code for
retrieving the ENETC port number based on the PF number. This is only
correct for LS1028A, the only Linux-capable instantiation of ENETC thus
far.

Note that ENETC port 3 is PF 6. The TSN code did not care about this
because ENETC port 3 does not support TSN, so the wrong mapping done by
enetc_get_port for PF 6 could have never been hit.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc.h     | 16 ++++++++++++++++
 drivers/net/ethernet/freescale/enetc/enetc_qos.c | 16 ++++++----------
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h
index 3de71669e317..08b283347d9c 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc.h
@@ -237,6 +237,22 @@ static inline bool enetc_si_is_pf(struct enetc_si *si)
 	return !!(si->hw.port);
 }
 
+static inline int enetc_pf_to_port(struct pci_dev *pf_pdev)
+{
+	switch (pf_pdev->devfn) {
+	case 0:
+		return 0;
+	case 1:
+		return 1;
+	case 2:
+		return 2;
+	case 6:
+		return 3;
+	default:
+		return -1;
+	}
+}
+
 #define ENETC_MAX_NUM_TXQS	8
 #define ENETC_INT_NAME_MAX	(IFNAMSIZ + 8)
 
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_qos.c b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
index cb7fa4bceaf2..af699f2ad095 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_qos.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
@@ -455,11 +455,6 @@ static struct enetc_psfp epsfp = {
 
 static LIST_HEAD(enetc_block_cb_list);
 
-static inline int enetc_get_port(struct enetc_ndev_priv *priv)
-{
-	return priv->si->pdev->devfn & 0x7;
-}
-
 /* Stream Identity Entry Set Descriptor */
 static int enetc_streamid_hw_set(struct enetc_ndev_priv *priv,
 				 struct enetc_streamid *sid,
@@ -504,7 +499,7 @@ static int enetc_streamid_hw_set(struct enetc_ndev_priv *priv,
 
 	si_conf = &cbd.sid_set;
 	/* Only one port supported for one entry, set itself */
-	si_conf->iports = cpu_to_le32(1 << enetc_get_port(priv));
+	si_conf->iports = cpu_to_le32(1 << enetc_pf_to_port(priv->si->pdev));
 	si_conf->id_type = 1;
 	si_conf->oui[2] = 0x0;
 	si_conf->oui[1] = 0x80;
@@ -529,7 +524,7 @@ static int enetc_streamid_hw_set(struct enetc_ndev_priv *priv,
 
 	si_conf->en = 0x80;
 	si_conf->stream_handle = cpu_to_le32(sid->handle);
-	si_conf->iports = cpu_to_le32(1 << enetc_get_port(priv));
+	si_conf->iports = cpu_to_le32(1 << enetc_pf_to_port(priv->si->pdev));
 	si_conf->id_type = sid->filtertype;
 	si_conf->oui[2] = 0x0;
 	si_conf->oui[1] = 0x80;
@@ -591,7 +586,8 @@ static int enetc_streamfilter_hw_set(struct enetc_ndev_priv *priv,
 	}
 
 	sfi_config->sg_inst_table_index = cpu_to_le16(sfi->gate_id);
-	sfi_config->input_ports = cpu_to_le32(1 << enetc_get_port(priv));
+	sfi_config->input_ports =
+		cpu_to_le32(1 << enetc_pf_to_port(priv->si->pdev));
 
 	/* The priority value which may be matched against the
 	 * frame’s priority value to determine a match for this entry.
@@ -1562,10 +1558,10 @@ int enetc_setup_tc_psfp(struct net_device *ndev, void *type_data)
 
 	switch (f->command) {
 	case FLOW_BLOCK_BIND:
-		set_bit(enetc_get_port(priv), &epsfp.dev_bitmap);
+		set_bit(enetc_pf_to_port(priv->si->pdev), &epsfp.dev_bitmap);
 		break;
 	case FLOW_BLOCK_UNBIND:
-		clear_bit(enetc_get_port(priv), &epsfp.dev_bitmap);
+		clear_bit(enetc_pf_to_port(priv->si->pdev), &epsfp.dev_bitmap);
 		if (!epsfp.dev_bitmap)
 			clean_psfp_all();
 		break;
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH net-next 2/5] dt-bindings: net: fsl: enetc: add the IERB documentation

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Mention the required compatible string and base address for the
Integrated Endpoint Register Block node.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 .../devicetree/bindings/net/fsl-enetc.txt         | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/fsl-enetc.txt b/Documentation/devicetree/bindings/net/fsl-enetc.txt
index b7034ccbc1bd..9b9a3f197e2d 100644
--- a/Documentation/devicetree/bindings/net/fsl-enetc.txt
+++ b/Documentation/devicetree/bindings/net/fsl-enetc.txt
@@ -102,3 +102,18 @@ Example:
 			full-duplex;
 		};
 	};
+
+* Integrated Endpoint Register Block bindings
+
+Optionally, the fsl_enetc driver can probe on the Integrated Endpoint Register
+Block, which preconfigures the FIFO limits for the ENETC ports. This is a node
+with the following properties:
+
+- reg		: Specifies the address in the SoC memory space.
+- compatible	: Must be "fsl,ls1028a-enetc-ierb".
+
+Example:
+	ierb@1f0800000 {
+		compatible = "fsl,ls1028a-enetc-ierb";
+		reg = <0x01 0xf0800000 0x0 0x10000>;
+	};
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH net-next 3/5] net: enetc: add a mini driver for the Integrated Endpoint Register Block

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

The NXP ENETC is a 4-port Ethernet controller which 'smells' to
operating systems like 4 distinct PCIe PFs with SR-IOV, each PF having
its own driver instance, but in fact there are some hardware resources
which are shared between all ports, like for example the 256 KB SRAM
FIFO between the MACs and the Host Transfer Agent which DMAs frames to
DRAM.

To hide the stuff that cannot be neatly exposed per port, the hardware
designers came up with this idea of having a dedicated register block
which is supposed to be populated by the bootloader, and contains
everything configuration-related: MAC addresses, FIFO partitioning, etc.

When a port is reset using PCIe Function Level Reset, its defaults are
transferred from the IERB configuration. Most of the time, the settings
made through the IERB are read-only in the port's memory space (if they
are even visible), so they cannot be modified at runtime.

Linux doesn't have any advanced FIFO partitioning requirements at all,
but when reading through the hardware manual, it became clear that, even
though there are many good 'recommendations' for default values, many of
them were not actually put in practice on LS1028A. So we end up with a
default configuration that:

(a) does not have enough TX and RX byte credits to support the max MTU
    of 9600 (which the Linux driver claims already) properly (at full speed)
(b) allows the FIFO to be overrun with RX traffic, potentially
    overwriting internal data structures.

The last part sounds a bit catastrophic, but it isn't. Frames are
supposed to transit the FIFO for a very short time, but they can
actually accumulate there under 2 conditions:

(a) there is very severe congestion on DRAM memory, or
(b) the RX rings visible to the operating system were configured for
    lossless operation, and they just ran out of free buffers to copy
    the frame to. This is what is used to put backpressure onto the MAC
    with flow control.

So since ENETC has not supported flow control thus far, RX FIFO overruns
were never seen with Linux. But with the addition of flow control, we
should configure some registers to prevent this from happening. What we
are trying to protect against are bad actors which continue to send us
traffic despite the fact that we have signaled a PAUSE condition. Of
course we can't be lossless in that case, but it is best to configure
the FIFO to do tail dropping rather than letting it overrun.

So in a nutshell, this driver is a fixup for all the IERB default values
that should have been but aren't.

The IERB configuration needs to be done _before_ the PFs are enabled.
So every PF searches for the presence of the "fsl,ls1028a-enetc-ierb"
node in the device tree, and if it finds it, it "registers" with the
IERB, which means that it requests the IERB to fix up its default
values. This is done through -EPROBE_DEFER. The IERB driver is part of
the fsl_enetc module, but is technically a platform driver, since the
IERB is a good old fashioned MMIO region, as opposed to ENETC ports
which pretend to be PCIe devices.

The driver was already configuring ENETC_PTXMBAR (FIFO allocation for
TX) because due to an omission, TXMBAR is a read/write register in the
PF memory space. But the manual is quite clear that the formula for this
should depend upon the TX byte credits (TXBCR). In turn, the TX byte
credits are only readable/writable through the IERB. So if we want to
ensure that the TXBCR register also has a value that is correct and in
line with TXMBAR, there is simply no way this can be done from the PF
driver, access to the IERB is needed.

I could have modified U-Boot to fix up the IERB values, but that is
quite undesirable, as old U-Boot versions are likely to be floating
around for quite some time from now.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/Kconfig  |   9 +
 drivers/net/ethernet/freescale/enetc/Makefile |   3 +
 .../net/ethernet/freescale/enetc/enetc_ierb.c | 155 ++++++++++++++++++
 .../net/ethernet/freescale/enetc/enetc_ierb.h |  20 +++
 .../net/ethernet/freescale/enetc/enetc_pf.c   |  35 +++-
 5 files changed, 221 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/ethernet/freescale/enetc/enetc_ierb.c
 create mode 100644 drivers/net/ethernet/freescale/enetc/enetc_ierb.h

diff --git a/drivers/net/ethernet/freescale/enetc/Kconfig b/drivers/net/ethernet/freescale/enetc/Kconfig
index ab92382c399a..d88f60c2bb82 100644
--- a/drivers/net/ethernet/freescale/enetc/Kconfig
+++ b/drivers/net/ethernet/freescale/enetc/Kconfig
@@ -2,6 +2,7 @@
 config FSL_ENETC
 	tristate "ENETC PF driver"
 	depends on PCI && PCI_MSI
+	depends on FSL_ENETC_IERB || FSL_ENETC_IERB=n
 	select FSL_ENETC_MDIO
 	select PHYLINK
 	select PCS_LYNX
@@ -25,6 +26,14 @@ config FSL_ENETC_VF
 
 	  If compiled as module (M), the module name is fsl-enetc-vf.
 
+config FSL_ENETC_IERB
+	tristate "ENETC IERB driver"
+	help
+	  This driver configures the Integrated Endpoint Register Block on NXP
+	  LS1028A.
+
+	  If compiled as module (M), the module name is fsl-enetc-ierb.
+
 config FSL_ENETC_MDIO
 	tristate "ENETC MDIO driver"
 	depends on PCI && MDIO_DEVRES && MDIO_BUS
diff --git a/drivers/net/ethernet/freescale/enetc/Makefile b/drivers/net/ethernet/freescale/enetc/Makefile
index 74f7ac253b8b..a139f2e9d59f 100644
--- a/drivers/net/ethernet/freescale/enetc/Makefile
+++ b/drivers/net/ethernet/freescale/enetc/Makefile
@@ -11,6 +11,9 @@ obj-$(CONFIG_FSL_ENETC_VF) += fsl-enetc-vf.o
 fsl-enetc-vf-y := enetc_vf.o $(common-objs)
 fsl-enetc-vf-$(CONFIG_FSL_ENETC_QOS) += enetc_qos.o
 
+obj-$(CONFIG_FSL_ENETC_IERB) += fsl-enetc-ierb.o
+fsl-enetc-ierb-y := enetc_ierb.o
+
 obj-$(CONFIG_FSL_ENETC_MDIO) += fsl-enetc-mdio.o
 fsl-enetc-mdio-y := enetc_pci_mdio.o enetc_mdio.o
 
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_ierb.c b/drivers/net/ethernet/freescale/enetc/enetc_ierb.c
new file mode 100644
index 000000000000..8b356c485507
--- /dev/null
+++ b/drivers/net/ethernet/freescale/enetc/enetc_ierb.c
@@ -0,0 +1,155 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
+/* Copyright 2021 NXP Semiconductors
+ *
+ * The Integrated Endpoint Register Block (IERB) is configured by pre-boot
+ * software and is supposed to be to ENETC what a NVRAM is to a 'real' PCIe
+ * card. Upon FLR, values from the IERB are transferred to the ENETC PFs, and
+ * are read-only in the PF memory space.
+ *
+ * This driver fixes up the power-on reset values for the ENETC shared FIFO,
+ * such that the TX and RX allocations are sufficient for jumbo frames, and
+ * that intelligent FIFO dropping is enabled before the internal data
+ * structures are corrupted.
+ *
+ * Even though not all ports might be used on a given board, we are not
+ * concerned with partitioning the FIFO, because the default values configure
+ * no strict reservations, so the entire FIFO can be used by the RX of a single
+ * port, or the TX of a single port.
+ */
+
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include "enetc.h"
+#include "enetc_ierb.h"
+
+/* IERB registers */
+#define ENETC_IERB_TXMBAR(port)			(((port) * 0x100) + 0x8080)
+#define ENETC_IERB_RXMBER(port)			(((port) * 0x100) + 0x8090)
+#define ENETC_IERB_RXMBLR(port)			(((port) * 0x100) + 0x8094)
+#define ENETC_IERB_RXBCR(port)			(((port) * 0x100) + 0x80a0)
+#define ENETC_IERB_TXBCR(port)			(((port) * 0x100) + 0x80a8)
+#define ENETC_IERB_FMBDTR			0xa000
+
+#define ENETC_RESERVED_FOR_ICM			1024
+
+struct enetc_ierb {
+	void __iomem *regs;
+};
+
+static void enetc_ierb_write(struct enetc_ierb *ierb, u32 offset, u32 val)
+{
+	iowrite32(val, ierb->regs + offset);
+}
+
+int enetc_ierb_register_pf(struct platform_device *pdev,
+			   struct pci_dev *pf_pdev)
+{
+	struct enetc_ierb *ierb = platform_get_drvdata(pdev);
+	int port = enetc_pf_to_port(pf_pdev);
+	u16 tx_credit, rx_credit, tx_alloc;
+
+	if (port < 0)
+		return -ENODEV;
+
+	if (!ierb)
+		return -EPROBE_DEFER;
+
+	/* By default, it is recommended to set the Host Transfer Agent
+	 * per port transmit byte credit to "1000 + max_frame_size/2".
+	 * The power-on reset value (1800 bytes) is rounded up to the nearest
+	 * 100 assuming a maximum frame size of 1536 bytes.
+	 */
+	tx_credit = roundup(1000 + ENETC_MAC_MAXFRM_SIZE / 2, 100);
+
+	/* Internal memory allocated for transmit buffering is guaranteed but
+	 * not reserved; i.e. if the total transmit allocation is not used,
+	 * then the unused portion is not left idle, it can be used for receive
+	 * buffering but it will be reclaimed, if required, from receive by
+	 * intelligently dropping already stored receive frames in the internal
+	 * memory to ensure that the transmit allocation is respected.
+	 *
+	 * PaTXMBAR must be set to a value larger than
+	 *     PaTXBCR + 2 * max_frame_size + 32
+	 * if frame preemption is not enabled, or to
+	 *     2 * PaTXBCR + 2 * p_max_frame_size (pMAC maximum frame size) +
+	 *     2 * np_max_frame_size (eMAC maximum frame size) + 64
+	 * if frame preemption is enabled.
+	 */
+	tx_alloc = roundup(2 * tx_credit + 4 * ENETC_MAC_MAXFRM_SIZE + 64, 16);
+
+	/* Initial credits, in units of 8 bytes, to the Ingress Congestion
+	 * Manager for the maximum amount of bytes the port is allocated for
+	 * pending traffic.
+	 * It is recommended to set the initial credits to 2 times the maximum
+	 * frame size (2 frames of maximum size).
+	 */
+	rx_credit = DIV_ROUND_UP(ENETC_MAC_MAXFRM_SIZE * 2, 8);
+
+	enetc_ierb_write(ierb, ENETC_IERB_TXBCR(port), tx_credit);
+	enetc_ierb_write(ierb, ENETC_IERB_TXMBAR(port), tx_alloc);
+	enetc_ierb_write(ierb, ENETC_IERB_RXBCR(port), rx_credit);
+
+	return 0;
+}
+EXPORT_SYMBOL(enetc_ierb_register_pf);
+
+static int enetc_ierb_probe(struct platform_device *pdev)
+{
+	struct enetc_ierb *ierb;
+	struct resource *res;
+	void __iomem *regs;
+
+	ierb = devm_kzalloc(&pdev->dev, sizeof(*ierb), GFP_KERNEL);
+	if (!ierb)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(regs))
+		return PTR_ERR(regs);
+
+	ierb->regs = regs;
+
+	/* Free buffer depletion threshold in bytes.
+	 * This sets the minimum amount of free buffer memory that should be
+	 * maintained in the datapath sub system, and when the amount of free
+	 * buffer memory falls below this threshold, a depletion indication is
+	 * asserted, which may trigger "intelligent drop" frame releases from
+	 * the ingress queues in the ICM.
+	 * It is recommended to set the free buffer depletion threshold to 1024
+	 * bytes, since the ICM needs some FIFO memory for its own use.
+	 */
+	enetc_ierb_write(ierb, ENETC_IERB_FMBDTR, ENETC_RESERVED_FOR_ICM);
+
+	platform_set_drvdata(pdev, ierb);
+
+	return 0;
+}
+
+static int enetc_ierb_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static const struct of_device_id enetc_ierb_match[] = {
+	{ .compatible = "fsl,ls1028a-enetc-ierb", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, enetc_ierb_match);
+
+static struct platform_driver enetc_ierb_driver = {
+	.driver = {
+		.name = "fsl-enetc-ierb",
+		.of_match_table = enetc_ierb_match,
+	},
+	.probe = enetc_ierb_probe,
+	.remove = enetc_ierb_remove,
+};
+
+module_platform_driver(enetc_ierb_driver);
+
+MODULE_DESCRIPTION("NXP ENETC IERB");
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_ierb.h b/drivers/net/ethernet/freescale/enetc/enetc_ierb.h
new file mode 100644
index 000000000000..b3b774e0998a
--- /dev/null
+++ b/drivers/net/ethernet/freescale/enetc/enetc_ierb.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
+/* Copyright 2021 NXP Semiconductors */
+
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+
+#if IS_ENABLED(CONFIG_FSL_ENETC_IERB)
+
+int enetc_ierb_register_pf(struct platform_device *pdev,
+			   struct pci_dev *pf_pdev);
+
+#else
+
+static inline int enetc_ierb_register_pf(struct platform_device *pdev,
+					 struct pci_dev *pf_pdev)
+{
+	return -EOPNOTSUPP;
+}
+
+#endif
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index be3dde0618e7..1ae2473cbc16 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -4,8 +4,10 @@
 #include <linux/mdio.h>
 #include <linux/module.h>
 #include <linux/fsl/enetc_mdio.h>
+#include <linux/of_platform.h>
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
+#include "enetc_ierb.h"
 #include "enetc_pf.h"
 
 #define ENETC_DRV_NAME_STR "ENETC PF driver"
@@ -518,7 +520,6 @@ static void enetc_configure_port_mac(struct enetc_hw *hw)
 		      ENETC_SET_MAXFRM(ENETC_RX_MAXFRM_SIZE));
 
 	enetc_port_wr(hw, ENETC_PTCMSDUR(0), ENETC_MAC_MAXFRM_SIZE);
-	enetc_port_wr(hw, ENETC_PTXMBAR, 2 * ENETC_MAC_MAXFRM_SIZE);
 
 	enetc_port_wr(hw, ENETC_PM0_CMD_CFG, ENETC_PM0_CMD_PHY_TX_EN |
 		      ENETC_PM0_CMD_TXP	| ENETC_PM0_PROMISC);
@@ -1116,6 +1117,30 @@ static int enetc_init_port_rss_memory(struct enetc_si *si)
 	return err;
 }
 
+static int enetc_pf_register_with_ierb(struct pci_dev *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct platform_device *ierb_pdev;
+	struct device_node *ierb_node;
+
+	/* Don't register with the IERB if the PF itself is disabled */
+	if (!node || !of_device_is_available(node))
+		return 0;
+
+	ierb_node = of_find_compatible_node(NULL, NULL,
+					    "fsl,ls1028a-enetc-ierb");
+	if (!ierb_node || !of_device_is_available(ierb_node))
+		return -ENODEV;
+
+	ierb_pdev = of_find_device_by_node(ierb_node);
+	of_node_put(ierb_node);
+
+	if (!ierb_pdev)
+		return -EPROBE_DEFER;
+
+	return enetc_ierb_register_pf(ierb_pdev, pdev);
+}
+
 static int enetc_pf_probe(struct pci_dev *pdev,
 			  const struct pci_device_id *ent)
 {
@@ -1126,6 +1151,14 @@ static int enetc_pf_probe(struct pci_dev *pdev,
 	struct enetc_pf *pf;
 	int err;
 
+	err = enetc_pf_register_with_ierb(pdev);
+	if (err == -EPROBE_DEFER)
+		return err;
+	if (err)
+		dev_warn(&pdev->dev,
+			 "Could not register with IERB driver: %pe, please update the device tree\n",
+			 ERR_PTR(err));
+
 	err = enetc_pci_probe(pdev, KBUILD_MODNAME, sizeof(*pf));
 	if (err) {
 		dev_err(&pdev->dev, "PCI probing failed\n");
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH net-next 4/5] arm64: dts: ls1028a: declare the Integrated Endpoint Register Block node

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Add a node describing the address in the SoC memory space for the IERB.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
index 7b45fba7a9cc..44e18603e678 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
@@ -1130,6 +1130,12 @@ fixed-link {
 			};
 		};
 
+		/* Integrated Endpoint Register Block */
+		ierb@1f0800000 {
+			compatible = "fsl,ls1028a-enetc-ierb";
+			reg = <0x01 0xf0800000 0x0 0x10000>;
+		};
+
 		rcpm: power-controller@1e34040 {
 			compatible = "fsl,ls1028a-rcpm", "fsl,qoriq-rcpm-2.1+";
 			reg = <0x0 0x1e34040 0x0 0x1c>;
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH net-next 5/5] net: enetc: add support for flow control

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

In the ENETC receive path, a frame received by the MAC is first stored
in a 256KB 'FIFO' memory, then transferred to DRAM when enqueuing it to
the RX ring. The FIFO is a shared resource for all ENETC ports, but
every port keeps track of its own memory utilization, on RX and on TX.

There is a setting for RX rings through which they can either operate in
'lossy' mode (where the lack of a free buffer causes an immediate
discard of the frame) or in 'lossless' mode (where the lack of a free
buffer in the ring makes the frame stay longer in the FIFO).

In turn, when the memory utilization of the FIFO exceeds a certain
margin, the MAC can be configured to emit PAUSE frames.

There is enough FIFO memory to buffer up to 3 MTU-sized frames per RX
port while not jeopardizing the other use cases (jumbo frames), and
also not consume bytes from the port TX allocations. Also, 3 MTU-sized
frames worth of memory is enough to ensure zero loss for 64 byte packets
at 1G line rate.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 .../ethernet/freescale/enetc/enetc_ethtool.c  | 18 ++++++
 .../net/ethernet/freescale/enetc/enetc_hw.h   |  9 +++
 .../net/ethernet/freescale/enetc/enetc_pf.c   | 60 ++++++++++++++++++-
 3 files changed, 85 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
index 49835e878bbb..ebccaf02411c 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
@@ -708,6 +708,22 @@ static int enetc_set_wol(struct net_device *dev,
 	return ret;
 }
 
+static void enetc_get_pauseparam(struct net_device *dev,
+				 struct ethtool_pauseparam *pause)
+{
+	struct enetc_ndev_priv *priv = netdev_priv(dev);
+
+	phylink_ethtool_get_pauseparam(priv->phylink, pause);
+}
+
+static int enetc_set_pauseparam(struct net_device *dev,
+				struct ethtool_pauseparam *pause)
+{
+	struct enetc_ndev_priv *priv = netdev_priv(dev);
+
+	return phylink_ethtool_set_pauseparam(priv->phylink, pause);
+}
+
 static int enetc_get_link_ksettings(struct net_device *dev,
 				    struct ethtool_link_ksettings *cmd)
 {
@@ -754,6 +770,8 @@ static const struct ethtool_ops enetc_pf_ethtool_ops = {
 	.get_ts_info = enetc_get_ts_info,
 	.get_wol = enetc_get_wol,
 	.set_wol = enetc_set_wol,
+	.get_pauseparam = enetc_get_pauseparam,
+	.set_pauseparam = enetc_set_pauseparam,
 };
 
 static const struct ethtool_ops enetc_vf_ethtool_ops = {
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
index 04ac7fc23ead..0f5f081a5baf 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
@@ -109,6 +109,7 @@ enum enetc_bdr_type {TX, RX};
 /* RX BDR reg offsets */
 #define ENETC_RBMR	0
 #define ENETC_RBMR_BDS	BIT(2)
+#define ENETC_RBMR_CM	BIT(4)
 #define ENETC_RBMR_VTE	BIT(5)
 #define ENETC_RBMR_EN	BIT(31)
 #define ENETC_RBSR	0x4
@@ -180,6 +181,8 @@ enum enetc_bdr_type {TX, RX};
 #define ENETC_PSIVLANR(n)	(0x0240 + (n) * 4) /* n = SI index */
 #define ENETC_PSIVLAN_EN	BIT(31)
 #define ENETC_PSIVLAN_SET_QOS(val)	((u32)(val) << 12)
+#define ENETC_PPAUONTR		0x0410
+#define ENETC_PPAUOFFTR		0x0414
 #define ENETC_PTXMBAR		0x0608
 #define ENETC_PCAPR0		0x0900
 #define ENETC_PCAPR0_RXBDR(val)	((val) >> 24)
@@ -227,6 +230,7 @@ enum enetc_bdr_type {TX, RX};
 #define ENETC_PM0_TX_EN		BIT(0)
 #define ENETC_PM0_RX_EN		BIT(1)
 #define ENETC_PM0_PROMISC	BIT(4)
+#define ENETC_PM0_PAUSE_IGN	BIT(8)
 #define ENETC_PM0_CMD_XGLP	BIT(10)
 #define ENETC_PM0_CMD_TXP	BIT(11)
 #define ENETC_PM0_CMD_PHY_TX_EN	BIT(15)
@@ -239,6 +243,11 @@ enum enetc_bdr_type {TX, RX};
 
 #define ENETC_PM_IMDIO_BASE	0x8030
 
+#define ENETC_PM0_PAUSE_QUANTA	0x8054
+#define ENETC_PM0_PAUSE_THRESH	0x8064
+#define ENETC_PM1_PAUSE_QUANTA	0x9054
+#define ENETC_PM1_PAUSE_THRESH	0x9064
+
 #define ENETC_PM0_SINGLE_STEP		0x80c0
 #define ENETC_PM1_SINGLE_STEP		0x90c0
 #define ENETC_PM0_SINGLE_STEP_CH	BIT(7)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 1ae2473cbc16..aff1339442cc 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -1015,7 +1015,12 @@ static void enetc_pl_mac_link_up(struct phylink_config *config,
 				 int duplex, bool tx_pause, bool rx_pause)
 {
 	struct enetc_pf *pf = phylink_to_enetc_pf(config);
+	u32 pause_off_thresh = 0, pause_on_thresh = 0;
+	u32 init_quanta = 0, refresh_quanta = 0;
+	struct enetc_hw *hw = &pf->si->hw;
 	struct enetc_ndev_priv *priv;
+	u32 rbmr, cmd_cfg;
+	int idx;
 
 	priv = netdev_priv(pf->si->ndev);
 	if (priv->active_offloads & ENETC_F_QBV)
@@ -1023,9 +1028,60 @@ static void enetc_pl_mac_link_up(struct phylink_config *config,
 
 	if (!phylink_autoneg_inband(mode) &&
 	    phy_interface_mode_is_rgmii(interface))
-		enetc_force_rgmii_mac(&pf->si->hw, speed, duplex);
+		enetc_force_rgmii_mac(hw, speed, duplex);
+
+	/* Flow control */
+	for (idx = 0; idx < priv->num_rx_rings; idx++) {
+		rbmr = enetc_rxbdr_rd(hw, idx, ENETC_RBMR);
+
+		if (tx_pause)
+			rbmr |= ENETC_RBMR_CM;
+		else
+			rbmr &= ~ENETC_RBMR_CM;
+
+		enetc_rxbdr_wr(hw, idx, ENETC_RBMR, rbmr);
+	}
+
+	if (tx_pause) {
+		/* When the port first enters congestion, send a PAUSE request
+		 * with the maximum number of quanta. When the port exits
+		 * congestion, it will automatically send a PAUSE frame with
+		 * zero quanta.
+		 */
+		init_quanta = 0xffff;
+
+		/* Also, set up the refresh timer to send follow-up PAUSE
+		 * frames at half the quanta value, in case the congestion
+		 * condition persists.
+		 */
+		refresh_quanta = 0xffff / 2;
+
+		/* Start emitting PAUSE frames when 3 large frames (or more
+		 * smaller frames) have accumulated in the FIFO waiting to be
+		 * DMAed to the RX ring.
+		 */
+		pause_on_thresh = 3 * ENETC_MAC_MAXFRM_SIZE;
+		pause_off_thresh = 1 * ENETC_MAC_MAXFRM_SIZE;
+	}
+
+	enetc_port_wr(hw, ENETC_PM0_PAUSE_QUANTA, init_quanta);
+	enetc_port_wr(hw, ENETC_PM1_PAUSE_QUANTA, init_quanta);
+	enetc_port_wr(hw, ENETC_PM0_PAUSE_THRESH, refresh_quanta);
+	enetc_port_wr(hw, ENETC_PM1_PAUSE_THRESH, refresh_quanta);
+	enetc_port_wr(hw, ENETC_PPAUONTR, pause_on_thresh);
+	enetc_port_wr(hw, ENETC_PPAUOFFTR, pause_off_thresh);
+
+	cmd_cfg = enetc_port_rd(hw, ENETC_PM0_CMD_CFG);
+
+	if (rx_pause)
+		cmd_cfg &= ~ENETC_PM0_PAUSE_IGN;
+	else
+		cmd_cfg |= ENETC_PM0_PAUSE_IGN;
+
+	enetc_port_wr(hw, ENETC_PM0_CMD_CFG, cmd_cfg);
+	enetc_port_wr(hw, ENETC_PM1_CMD_CFG, cmd_cfg);
 
-	enetc_mac_enable(&pf->si->hw, true);
+	enetc_mac_enable(hw, true);
 }
 
 static void enetc_pl_mac_link_down(struct phylink_config *config,
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH net-next 5/5] net: enetc: add support for flow control

[Claudiu Manoil]

>-----Original Message-----
>From: Vladimir Oltean <olteanv@gmail.com>
>Sent: Saturday, April 17, 2021 2:42 AM
>To: Jakub Kicinski <kuba@kernel.org>; David S. Miller
><davem@davemloft.net>; netdev@vger.kernel.org; Po Liu
><po.liu@nxp.com>
>Cc: Claudiu Manoil <claudiu.manoil@nxp.com>; Alexandru Marginean
><alexandru.marginean@nxp.com>; Rob Herring <robh+dt@kernel.org>;
>Shawn Guo <shawnguo@kernel.org>; linux-arm-kernel@lists.infradead.org;
>devicetree@vger.kernel.org; Russell King - ARM Linux admin
><linux@armlinux.org.uk>; Andrew Lunn <andrew@lunn.ch>; Michael Walle
><michael@walle.cc>; Vladimir Oltean <vladimir.oltean@nxp.com>
>Subject: [PATCH net-next 5/5] net: enetc: add support for flow control
>
>From: Vladimir Oltean <vladimir.oltean@nxp.com>
>
>In the ENETC receive path, a frame received by the MAC is first stored
>in a 256KB 'FIFO' memory, then transferred to DRAM when enqueuing it to
>the RX ring. The FIFO is a shared resource for all ENETC ports, but
>every port keeps track of its own memory utilization, on RX and on TX.
>
>There is a setting for RX rings through which they can either operate in
>'lossy' mode (where the lack of a free buffer causes an immediate
>discard of the frame) or in 'lossless' mode (where the lack of a free
>buffer in the ring makes the frame stay longer in the FIFO).
>
>In turn, when the memory utilization of the FIFO exceeds a certain
>margin, the MAC can be configured to emit PAUSE frames.
>
>There is enough FIFO memory to buffer up to 3 MTU-sized frames per RX
>port while not jeopardizing the other use cases (jumbo frames), and
>also not consume bytes from the port TX allocations. Also, 3 MTU-sized
>frames worth of memory is enough to ensure zero loss for 64 byte packets
>at 1G line rate.
>
>Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Claudiu Manoil <claudiu.manoil@nxp.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 0/5] Flow control for NXP ENETC

[Jakub Kicinski]

On Sat, 17 Apr 2021 02:42:20 +0300 Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> This patch series contains logic for enabling the lossless mode on the
> RX rings of the ENETC, and the PAUSE thresholds on the internal FIFO
> memory.
> 
> During testing it was found that, with the default FIFO configuration,
> a sender which isn't persuaded by our PAUSE frames and keeps sending
> will cause some MAC RX frame errors. To mitigate this, we need to ensure
> that the FIFO never runs completely full, so we need to fix up a setting
> that was supposed to be configured well out of reset. Unfortunately this
> requires the addition of a new mini-driver.

FWIW back in the day when I was working on more advanced devices than 
I deal with these days I was expecting to eventually run into this as
well and create some form of devlink umbrella. IMHO such "mini driver"
is a natural place for a devlink instance, and not the PFs/ports.
Is this your thinking as well? AFAICT enetc doesn't implement devlink
today so you start from whatever model works best without worrying
about backward compat.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 0/5] Flow control for NXP ENETC

[Vladimir Oltean]

Hi Jakub,

On Mon, Apr 19, 2021 at 02:04:42PM -0700, Jakub Kicinski wrote:
> On Sat, 17 Apr 2021 02:42:20 +0300 Vladimir Oltean wrote:
> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> > 
> > This patch series contains logic for enabling the lossless mode on the
> > RX rings of the ENETC, and the PAUSE thresholds on the internal FIFO
> > memory.
> > 
> > During testing it was found that, with the default FIFO configuration,
> > a sender which isn't persuaded by our PAUSE frames and keeps sending
> > will cause some MAC RX frame errors. To mitigate this, we need to ensure
> > that the FIFO never runs completely full, so we need to fix up a setting
> > that was supposed to be configured well out of reset. Unfortunately this
> > requires the addition of a new mini-driver.
> 
> FWIW back in the day when I was working on more advanced devices than 
> I deal with these days I was expecting to eventually run into this as
> well and create some form of devlink umbrella. IMHO such "mini driver"
> is a natural place for a devlink instance, and not the PFs/ports.
> Is this your thinking as well? AFAICT enetc doesn't implement devlink
> today so you start from whatever model works best without worrying
> about backward compat.

Sorry, but I am not sure if I understood the central idea of what you
were trying to transmit. What is 'a devlink instance and not the PFs'?
I am not aware of how a single devlink instance can be exposed for a
piece of hardware presenting itself as multiple PFs with multiple driver
instances running asynchronously and potentially being assigned to AMP
software execution environments (other cores running non-Linux, and most
probably Linux is not even the privileged execution environment which
has write access to the FIFO parameters).
Are you suggesting that the FIFO size and partitioning characteristics
be exposed through the devlink subsystem? Isn't that what devlink-sb is
for? Also, that would not help with what the IERB driver is trying to
achieve. There isn't anything we want the user to view or fiddle with,
the reality is simply that the FIFO parameters were supposed to be
one-size-fits-all-and-nobody-cares-about-them (the memory usage scheme
of this NIC is smart enough to allow for that, or so I think) but
nonetheless, the hardware defaults need to be touched up. If LS1028A was
a new SoC today we would have probably done this from U-Boot, from the
same logic that already passes the MAC addresses to the PFs through the
IERB, but the ship has kind of sailed for that, bootloaders are stable,
and 'Linux needs this feature' is not a good reason to update them.
So this is all that I would like the IERB driver to do, notice how it's
all writes of predefined values but no reads. For next generation SoCs
with ENETC we'll try our best to not need an IERB driver in Linux at
all. Another option would have been to do these fixups in the arch init
code as a sort of erratum workaround, but I didn't find a place similar
to arch/arm/mach-* for arm64, so I assumed that the arm64 port just
doesn't want to go that route. So here I am with a driver for some
memory writes.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 0/5] Flow control for NXP ENETC

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Sat, 17 Apr 2021 02:42:20 +0300 you wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> This patch series contains logic for enabling the lossless mode on the
> RX rings of the ENETC, and the PAUSE thresholds on the internal FIFO
> memory.
> 
> During testing it was found that, with the default FIFO configuration,
> a sender which isn't persuaded by our PAUSE frames and keeps sending
> will cause some MAC RX frame errors. To mitigate this, we need to ensure
> that the FIFO never runs completely full, so we need to fix up a setting
> that was supposed to be configured well out of reset. Unfortunately this
> requires the addition of a new mini-driver.
> 
> [...]

Here is the summary with links:
  - [net-next,1/5] net: enetc: create a common enetc_pf_to_port helper
    https://git.kernel.org/netdev/net-next/c/87614b931c24
  - [net-next,2/5] dt-bindings: net: fsl: enetc: add the IERB documentation
    https://git.kernel.org/netdev/net-next/c/4ac7acc67f29
  - [net-next,3/5] net: enetc: add a mini driver for the Integrated Endpoint Register Block
    https://git.kernel.org/netdev/net-next/c/e7d48e5fbf30
  - [net-next,4/5] arm64: dts: ls1028a: declare the Integrated Endpoint Register Block node
    https://git.kernel.org/netdev/net-next/c/b764dc6cc1ba
  - [net-next,5/5] net: enetc: add support for flow control
    https://git.kernel.org/netdev/net-next/c/a8648887880f

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 0/5] Flow control for NXP ENETC

[Michael Walle]

Hi Vladimir,

Am 2021-04-17 01:42, schrieb Vladimir Oltean:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> This patch series contains logic for enabling the lossless mode on the
> RX rings of the ENETC, and the PAUSE thresholds on the internal FIFO
> memory.
> 
> During testing it was found that, with the default FIFO configuration,
> a sender which isn't persuaded by our PAUSE frames and keeps sending
> will cause some MAC RX frame errors. To mitigate this, we need to 
> ensure
> that the FIFO never runs completely full, so we need to fix up a 
> setting
> that was supposed to be configured well out of reset. Unfortunately 
> this
> requires the addition of a new mini-driver.

What happens if the mini driver is not enabled? Then the fixes aren't
applied and bad things happen (now with the addition of flow control),
right?

I'm asking because, if you have the arm64 defconfig its not enabled.

shouldn't it be something like:

diff --git a/drivers/net/ethernet/freescale/enetc/Kconfig 
b/drivers/net/ethernet/freescale/enetc/Kconfig
index d88f60c2bb82..cdc0ff89388a 100644
--- a/drivers/net/ethernet/freescale/enetc/Kconfig
+++ b/drivers/net/ethernet/freescale/enetc/Kconfig
@@ -2,7 +2,7 @@
  config FSL_ENETC
         tristate "ENETC PF driver"
         depends on PCI && PCI_MSI
-       depends on FSL_ENETC_IERB || FSL_ENETC_IERB=n
+       select FSL_ENETC_IERB
         select FSL_ENETC_MDIO
         select PHYLINK
         select PCS_LYNX

-michael

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 0/5] Flow control for NXP ENETC

[Vladimir Oltean]

Hi Michael,

On Tue, Apr 20, 2021 at 03:27:24PM +0200, Michael Walle wrote:
> Hi Vladimir,
> 
> Am 2021-04-17 01:42, schrieb Vladimir Oltean:
> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> > 
> > This patch series contains logic for enabling the lossless mode on the
> > RX rings of the ENETC, and the PAUSE thresholds on the internal FIFO
> > memory.
> > 
> > During testing it was found that, with the default FIFO configuration,
> > a sender which isn't persuaded by our PAUSE frames and keeps sending
> > will cause some MAC RX frame errors. To mitigate this, we need to ensure
> > that the FIFO never runs completely full, so we need to fix up a setting
> > that was supposed to be configured well out of reset. Unfortunately this
> > requires the addition of a new mini-driver.
> 
> What happens if the mini driver is not enabled? Then the fixes aren't
> applied and bad things happen (now with the addition of flow control),
> right?
> 
> I'm asking because, if you have the arm64 defconfig its not enabled.
> 
> shouldn't it be something like:
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/Kconfig
> b/drivers/net/ethernet/freescale/enetc/Kconfig
> index d88f60c2bb82..cdc0ff89388a 100644
> --- a/drivers/net/ethernet/freescale/enetc/Kconfig
> +++ b/drivers/net/ethernet/freescale/enetc/Kconfig
> @@ -2,7 +2,7 @@
>  config FSL_ENETC
>         tristate "ENETC PF driver"
>         depends on PCI && PCI_MSI
> -       depends on FSL_ENETC_IERB || FSL_ENETC_IERB=n
> +       select FSL_ENETC_IERB
>         select FSL_ENETC_MDIO
>         select PHYLINK
>         select PCS_LYNX

Yes, ideally the IERB driver and the ENETC PF driver should be built in
the same way, or the IERB driver can be built-in and the PF driver can
be module. I don't know how to express this using Kconfig, sorry.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 0/5] Flow control for NXP ENETC

[Michael Walle]

Hi Vladimir,

Am 2021-04-20 16:04, schrieb Vladimir Oltean:
> On Tue, Apr 20, 2021 at 03:27:24PM +0200, Michael Walle wrote:
>> Hi Vladimir,
>> 
>> Am 2021-04-17 01:42, schrieb Vladimir Oltean:
>> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
>> >
>> > This patch series contains logic for enabling the lossless mode on the
>> > RX rings of the ENETC, and the PAUSE thresholds on the internal FIFO
>> > memory.
>> >
>> > During testing it was found that, with the default FIFO configuration,
>> > a sender which isn't persuaded by our PAUSE frames and keeps sending
>> > will cause some MAC RX frame errors. To mitigate this, we need to ensure
>> > that the FIFO never runs completely full, so we need to fix up a setting
>> > that was supposed to be configured well out of reset. Unfortunately this
>> > requires the addition of a new mini-driver.
>> 
>> What happens if the mini driver is not enabled? Then the fixes aren't
>> applied and bad things happen (now with the addition of flow control),
>> right?
>> 
>> I'm asking because, if you have the arm64 defconfig its not enabled.
>> 
>> shouldn't it be something like:
>> 
>> diff --git a/drivers/net/ethernet/freescale/enetc/Kconfig
>> b/drivers/net/ethernet/freescale/enetc/Kconfig
>> index d88f60c2bb82..cdc0ff89388a 100644
>> --- a/drivers/net/ethernet/freescale/enetc/Kconfig
>> +++ b/drivers/net/ethernet/freescale/enetc/Kconfig
>> @@ -2,7 +2,7 @@
>>  config FSL_ENETC
>>         tristate "ENETC PF driver"
>>         depends on PCI && PCI_MSI
>> -       depends on FSL_ENETC_IERB || FSL_ENETC_IERB=n
>> +       select FSL_ENETC_IERB
>>         select FSL_ENETC_MDIO
>>         select PHYLINK
>>         select PCS_LYNX
> 
> Yes, ideally the IERB driver and the ENETC PF driver should be built in
> the same way, or the IERB driver can be built-in and the PF driver can
> be module. I don't know how to express this using Kconfig, sorry.

With the small patch above it is:
  FSL_ENETC=m -> FSL_ENETC_IERB = m or y
  FSL_ENETC=y -> FSL_ENETC_IERB = y
  FSL_ENETC=n -> FSL_ENETC_IERB = m,y or n

Will you fix it? Should I prepare a patch?

-michael

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 0/5] Flow control for NXP ENETC

[Vladimir Oltean]

On Tue, Apr 20, 2021 at 04:10:34PM +0200, Michael Walle wrote:
> Hi Vladimir,
> 
> Am 2021-04-20 16:04, schrieb Vladimir Oltean:
> > On Tue, Apr 20, 2021 at 03:27:24PM +0200, Michael Walle wrote:
> > > Hi Vladimir,
> > > 
> > > Am 2021-04-17 01:42, schrieb Vladimir Oltean:
> > > > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > >
> > > > This patch series contains logic for enabling the lossless mode on the
> > > > RX rings of the ENETC, and the PAUSE thresholds on the internal FIFO
> > > > memory.
> > > >
> > > > During testing it was found that, with the default FIFO configuration,
> > > > a sender which isn't persuaded by our PAUSE frames and keeps sending
> > > > will cause some MAC RX frame errors. To mitigate this, we need to ensure
> > > > that the FIFO never runs completely full, so we need to fix up a setting
> > > > that was supposed to be configured well out of reset. Unfortunately this
> > > > requires the addition of a new mini-driver.
> > > 
> > > What happens if the mini driver is not enabled? Then the fixes aren't
> > > applied and bad things happen (now with the addition of flow control),
> > > right?
> > > 
> > > I'm asking because, if you have the arm64 defconfig its not enabled.
> > > 
> > > shouldn't it be something like:
> > > 
> > > diff --git a/drivers/net/ethernet/freescale/enetc/Kconfig
> > > b/drivers/net/ethernet/freescale/enetc/Kconfig
> > > index d88f60c2bb82..cdc0ff89388a 100644
> > > --- a/drivers/net/ethernet/freescale/enetc/Kconfig
> > > +++ b/drivers/net/ethernet/freescale/enetc/Kconfig
> > > @@ -2,7 +2,7 @@
> > >  config FSL_ENETC
> > >         tristate "ENETC PF driver"
> > >         depends on PCI && PCI_MSI
> > > -       depends on FSL_ENETC_IERB || FSL_ENETC_IERB=n
> > > +       select FSL_ENETC_IERB
> > >         select FSL_ENETC_MDIO
> > >         select PHYLINK
> > >         select PCS_LYNX
> > 
> > Yes, ideally the IERB driver and the ENETC PF driver should be built in
> > the same way, or the IERB driver can be built-in and the PF driver can
> > be module. I don't know how to express this using Kconfig, sorry.
> 
> With the small patch above it is:
>  FSL_ENETC=m -> FSL_ENETC_IERB = m or y
>  FSL_ENETC=y -> FSL_ENETC_IERB = y
>  FSL_ENETC=n -> FSL_ENETC_IERB = m,y or n
> 
> Will you fix it? Should I prepare a patch?

Could you please send the patch? Thanks.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel