* [PATCH net-next v4 1/2] net: macb: implement ethtool_ops.get|set_channels()
2026-03-17 16:19 [PATCH net-next v4 0/2] net: macb: implement ethtool set channels count operation Théo Lebrun
@ 2026-03-17 16:19 ` Théo Lebrun
2026-03-19 2:38 ` Jakub Kicinski
2026-03-17 16:19 ` [PATCH net-next v4 2/2] net: macb: distribute evenly Tx SRAM segments Théo Lebrun
1 sibling, 1 reply; 4+ messages in thread
From: Théo Lebrun @ 2026-03-17 16:19 UTC (permalink / raw)
To: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel, Vladimir Kondratiev, Gregory CLEMENT,
Benoît Monin, Tawfik Bayouk, Thomas Petazzoni, Paolo Valerio,
Théo Lebrun
bp->num_queues is the total number of queues and is constant from probe.
Introduce bp->max_num_queues which takes the current role of
bp->num_queues and allow `0 < bp->num_queues <= bp->max_num_queues`.
MACB/GEM does not know about rx/tx specific queues; it only has
combined queues.
.set_channels() is reserved to devices with MACB_CAPS_QUEUE_DISABLE.
The tieoff workaround would not work as packets would still be routed
into queues with a tieoff descriptor.
Implement .set_channels() operation by refusing if netif_running().
The reason to implement .set_channels() is memory savings, we cannot
preallocate and swap at runtime.
Nit: fix an alignment issue inside gem_ethtool_ops which does not
deserve its own patch.
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
drivers/net/ethernet/cadence/macb.h | 1 +
drivers/net/ethernet/cadence/macb_main.c | 100 ++++++++++++++++++++++++++-----
2 files changed, 85 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index c69828b27dae..b08afe340996 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1309,6 +1309,7 @@ struct macb {
unsigned int tx_ring_size;
unsigned int num_queues;
+ unsigned int max_num_queues;
struct macb_queue queues[MACB_MAX_QUEUES];
spinlock_t lock;
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index abd328428c94..dd066f481200 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -469,9 +469,26 @@ static void macb_init_buffers(struct macb *bp)
upper_32_bits(bp->queues[0].tx_ring_dma));
}
- for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
- queue_writel(queue, RBQP, lower_32_bits(queue->rx_ring_dma));
- queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
+ for (q = 0, queue = bp->queues; q < bp->max_num_queues; ++q, ++queue) {
+ if (q < bp->num_queues) {
+ queue_writel(queue, RBQP,
+ lower_32_bits(queue->rx_ring_dma));
+ queue_writel(queue, TBQP,
+ lower_32_bits(queue->tx_ring_dma));
+ } else {
+ /*
+ * macb_set_channels(), which is the only way of writing
+ * to bp->num_queues, is only allowed if
+ * MACB_CAPS_QUEUE_DISABLE.
+ */
+ queue_writel(queue, RBQP, MACB_BIT(QUEUE_DISABLE));
+
+ /* Disable all interrupts */
+ queue_writel(queue, IDR, -1);
+ queue_readl(queue, ISR);
+ if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+ queue_writel(queue, ISR, -1);
+ }
}
}
@@ -4121,8 +4138,8 @@ static int gem_set_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd)
switch (cmd->cmd) {
case ETHTOOL_SRXCLSRLINS:
- if ((cmd->fs.location >= bp->max_tuples)
- || (cmd->fs.ring_cookie >= bp->num_queues)) {
+ if (cmd->fs.location >= bp->max_tuples ||
+ cmd->fs.ring_cookie >= bp->max_num_queues) {
ret = -EINVAL;
break;
}
@@ -4140,6 +4157,51 @@ static int gem_set_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd)
return ret;
}
+static void macb_get_channels(struct net_device *netdev,
+ struct ethtool_channels *ch)
+{
+ struct macb *bp = netdev_priv(netdev);
+
+ ch->max_combined = bp->max_num_queues;
+ ch->combined_count = bp->num_queues;
+}
+
+static int macb_set_channels(struct net_device *netdev,
+ struct ethtool_channels *ch)
+{
+ struct macb *bp = netdev_priv(netdev);
+ unsigned int old_count = bp->num_queues;
+ unsigned int count = ch->combined_count;
+ int ret;
+
+ /*
+ * MACB_CAPS_QUEUE_DISABLE means that the field QUEUE_DISABLE/BIT0 in
+ * the per-queue RBQP register disables queue Rx. If we don't have that
+ * capability we can have multiple queues but we must always run with
+ * all enabled.
+ */
+ if (!(bp->caps & MACB_CAPS_QUEUE_DISABLE))
+ return -EOPNOTSUPP;
+
+ /*
+ * An ideal .set_channels() implementation uses upfront allocated
+ * resources and swaps them in, bringing reliability under memory
+ * pressure. However, here we implement it for memory savings in
+ * setups with less than max number of queues active.
+ *
+ * Signal it by refusing .set_channels() once interface is opened.
+ */
+ if (netif_running(bp->dev))
+ return -EBUSY;
+
+ ret = netif_set_real_num_queues(bp->dev, count, count);
+ if (ret)
+ return ret;
+
+ bp->num_queues = count;
+ return 0;
+}
+
static const struct ethtool_ops macb_ethtool_ops = {
.get_regs_len = macb_get_regs_len,
.get_regs = macb_get_regs,
@@ -4155,6 +4217,8 @@ static const struct ethtool_ops macb_ethtool_ops = {
.set_link_ksettings = macb_set_link_ksettings,
.get_ringparam = macb_get_ringparam,
.set_ringparam = macb_set_ringparam,
+ .get_channels = macb_get_channels,
+ .set_channels = macb_set_channels,
};
static int macb_get_eee(struct net_device *dev, struct ethtool_keee *eee)
@@ -4189,12 +4253,14 @@ static const struct ethtool_ops gem_ethtool_ops = {
.set_link_ksettings = macb_set_link_ksettings,
.get_ringparam = macb_get_ringparam,
.set_ringparam = macb_set_ringparam,
- .get_rxnfc = gem_get_rxnfc,
- .set_rxnfc = gem_set_rxnfc,
- .get_rx_ring_count = gem_get_rx_ring_count,
- .nway_reset = phy_ethtool_nway_reset,
+ .get_rxnfc = gem_get_rxnfc,
+ .set_rxnfc = gem_set_rxnfc,
+ .get_rx_ring_count = gem_get_rx_ring_count,
+ .nway_reset = phy_ethtool_nway_reset,
.get_eee = macb_get_eee,
.set_eee = macb_set_eee,
+ .get_channels = macb_get_channels,
+ .set_channels = macb_set_channels,
};
static int macb_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
@@ -4335,9 +4401,9 @@ static int macb_taprio_setup_replace(struct net_device *ndev,
size_t i;
int err;
- if (conf->num_entries > bp->num_queues) {
+ if (conf->num_entries > bp->max_num_queues) {
netdev_err(ndev, "Too many TAPRIO entries: %zu > %d queues\n",
- conf->num_entries, bp->num_queues);
+ conf->num_entries, bp->max_num_queues);
return -EINVAL;
}
@@ -4385,9 +4451,9 @@ static int macb_taprio_setup_replace(struct net_device *ndev,
/* gate_mask must not select queues outside the valid queues */
queue_id = order_base_2(entry->gate_mask);
- if (queue_id >= bp->num_queues) {
+ if (queue_id >= bp->max_num_queues) {
netdev_err(ndev, "Entry %zu: gate_mask 0x%x exceeds queue range (max_queues=%d)\n",
- i, entry->gate_mask, bp->num_queues);
+ i, entry->gate_mask, bp->max_num_queues);
err = -EINVAL;
goto cleanup;
}
@@ -4447,7 +4513,7 @@ static int macb_taprio_setup_replace(struct net_device *ndev,
/* All validations passed - proceed with hardware configuration */
scoped_guard(spinlock_irqsave, &bp->lock) {
/* Disable ENST queues if running before configuring */
- queue_mask = BIT_U32(bp->num_queues) - 1;
+ queue_mask = BIT_U32(bp->max_num_queues) - 1;
gem_writel(bp, ENST_CONTROL,
queue_mask << GEM_ENST_DISABLE_QUEUE_OFFSET);
@@ -4482,7 +4548,7 @@ static void macb_taprio_destroy(struct net_device *ndev)
unsigned int q;
netdev_reset_tc(ndev);
- queue_mask = BIT_U32(bp->num_queues) - 1;
+ queue_mask = BIT_U32(bp->max_num_queues) - 1;
scoped_guard(spinlock_irqsave, &bp->lock) {
/* Single disable command for all queues */
@@ -4490,7 +4556,8 @@ static void macb_taprio_destroy(struct net_device *ndev)
queue_mask << GEM_ENST_DISABLE_QUEUE_OFFSET);
/* Clear all queue ENST registers in batch */
- for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
+ for (q = 0, queue = bp->queues; q < bp->max_num_queues;
+ ++q, ++queue) {
queue_writel(queue, ENST_START_TIME, 0);
queue_writel(queue, ENST_ON_TIME, 0);
queue_writel(queue, ENST_OFF_TIME, 0);
@@ -5735,6 +5802,7 @@ static int macb_probe(struct platform_device *pdev)
bp->macb_reg_writel = hw_writel;
}
bp->num_queues = num_queues;
+ bp->max_num_queues = num_queues;
bp->dma_burst_length = macb_config->dma_burst_length;
bp->pclk = pclk;
bp->hclk = hclk;
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH net-next v4 2/2] net: macb: distribute evenly Tx SRAM segments
2026-03-17 16:19 [PATCH net-next v4 0/2] net: macb: implement ethtool set channels count operation Théo Lebrun
2026-03-17 16:19 ` [PATCH net-next v4 1/2] net: macb: implement ethtool_ops.get|set_channels() Théo Lebrun
@ 2026-03-17 16:19 ` Théo Lebrun
1 sibling, 0 replies; 4+ messages in thread
From: Théo Lebrun @ 2026-03-17 16:19 UTC (permalink / raw)
To: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel, Vladimir Kondratiev, Gregory CLEMENT,
Benoît Monin, Tawfik Bayouk, Thomas Petazzoni, Paolo Valerio,
Théo Lebrun
GEM has registers to configure the Tx SRAM segments distribution across
queues. The reset value is apprioriate (even spread) but we need to
care if/when number of active queues is modified (or if we inherited
unevenly initialised hardware from bootloader).
To distribute segments, we take as input the number of queues
(bp->num_queues) and the number of segments (found inside DCFG6).
Its output is a number of segments for each queue, formatted as
powers-of-two (eg 2 for queue 0 means it has 2^2=4 segments).
As the distribution logic is quite complex (at least its initial
versions had bugs), it is kunit-tested in macb_kunit.c and the
implementation lives in macb_utils.c. To test:
⟩ env --unset=CROSS_COMPILE make ARCH=um mrproper
⟩ env --unset=CROSS_COMPILE ./tools/testing/kunit/kunit.py run \
--kconfig_add CONFIG_NET=y \
--kconfig_add CONFIG_COMMON_CLK=y macb
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
drivers/net/ethernet/cadence/Kconfig | 12 +++++
drivers/net/ethernet/cadence/Makefile | 5 +-
drivers/net/ethernet/cadence/macb.h | 8 ++++
drivers/net/ethernet/cadence/macb_kunit.c | 77 +++++++++++++++++++++++++++++++
drivers/net/ethernet/cadence/macb_main.c | 15 ++++++
drivers/net/ethernet/cadence/macb_utils.c | 56 ++++++++++++++++++++++
6 files changed, 172 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
index 5b2a461dfd28..a901f74fd4ab 100644
--- a/drivers/net/ethernet/cadence/Kconfig
+++ b/drivers/net/ethernet/cadence/Kconfig
@@ -51,4 +51,16 @@ config MACB_PCI
To compile this driver as a module, choose M here: the module
will be called macb_pci.
+config MACB_KUNIT_TEST
+ tristate "KUnit test for MACB" if !KUNIT_ALL_TESTS
+ depends on KUNIT
+ default KUNIT_ALL_TESTS
+ help
+ Build KUnit tests for the MACB driver.
+
+ For more information on KUnit and unit tests in general,
+ please refer to the KUnit documentation.
+
+ If unsure, say N.
+
endif # NET_VENDOR_CADENCE
diff --git a/drivers/net/ethernet/cadence/Makefile b/drivers/net/ethernet/cadence/Makefile
index 1f33cdca9a3c..a0740da68649 100644
--- a/drivers/net/ethernet/cadence/Makefile
+++ b/drivers/net/ethernet/cadence/Makefile
@@ -2,7 +2,7 @@
#
# Makefile for the Atmel network device drivers.
#
-macb-y := macb_main.o
+macb-y := macb_main.o macb_utils.o
ifeq ($(CONFIG_MACB_USE_HWSTAMP),y)
macb-y += macb_ptp.o
@@ -10,3 +10,6 @@ endif
obj-$(CONFIG_MACB) += macb.o
obj-$(CONFIG_MACB_PCI) += macb_pci.o
+
+obj-$(CONFIG_MACB_KUNIT_TEST) += macb_test.o
+macb_test-y := macb_kunit.o macb_utils.o
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index b08afe340996..0464e774273a 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -197,6 +197,9 @@
#define GEM_TXBDCTRL 0x04cc /* TX Buffer Descriptor control register */
#define GEM_RXBDCTRL 0x04d0 /* RX Buffer Descriptor control register */
+#define GEM_TXQSEGALLOC_LOWER 0x05A0 /* Tx queue segment allocation (low) */
+#define GEM_TXQSEGALLOC_UPPER 0x05A4 /* Tx queue segment allocation (high) */
+
/* Screener Type 2 match registers */
#define GEM_SCRT2 0x540
@@ -549,6 +552,8 @@
#define GEM_PBUF_CUTTHRU_SIZE 1
#define GEM_DAW64_OFFSET 23
#define GEM_DAW64_SIZE 1
+#define GEM_SEGMENTS_BIT_SIZE_OFFSET 16
+#define GEM_SEGMENTS_BIT_SIZE_SIZE 3
/* Bitfields in DCFG8. */
#define GEM_T1SCR_OFFSET 24
@@ -1494,4 +1499,7 @@ struct macb_queue_enst_config {
u8 queue_id;
};
+u64 gem_sram_distribute_segments(unsigned int num_queues,
+ unsigned int num_segments);
+
#endif /* _MACB_H */
diff --git a/drivers/net/ethernet/cadence/macb_kunit.c b/drivers/net/ethernet/cadence/macb_kunit.c
new file mode 100644
index 000000000000..b91a10bda623
--- /dev/null
+++ b/drivers/net/ethernet/cadence/macb_kunit.c
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <kunit/test.h>
+#include <linux/netdevice.h>
+#include "macb.h"
+
+struct macb_sram_segments_case {
+ unsigned int num_queues, num_segments;
+};
+
+static void macb_sram_segments_test(struct kunit *test)
+{
+ const struct macb_sram_segments_case *p = test->param_value;
+ u64 val = gem_sram_distribute_segments(p->num_queues, p->num_segments);
+ unsigned int i, sum_segments = 0, max_assigned_segments;
+ unsigned int num_queues = min(p->num_queues, p->num_segments);
+
+ for (i = 0; i < num_queues; i++) {
+ unsigned int q_segments = (val >> (i * 4)) & 0b11;
+
+ q_segments = 1U << q_segments;
+ sum_segments += q_segments;
+ KUNIT_ASSERT_GT_MSG(test, q_segments, 0, "queue %d, val %#llx",
+ i, val);
+ }
+
+ for (i = num_queues; i < 16; i++) {
+ unsigned int pow = (val >> (i * 4)) & 0b11;
+
+ KUNIT_ASSERT_EQ_MSG(test, pow, 0, "queue %d, val %#llx",
+ i, val);
+ }
+
+ max_assigned_segments = min(p->num_segments, 8 * p->num_queues);
+ KUNIT_ASSERT_EQ_MSG(test, sum_segments, max_assigned_segments,
+ "val %#llx", val);
+}
+
+struct macb_sram_segments_case macb_sram_segments_cases[] = {
+ /* num_segments can only be powers of two. */
+ { .num_queues = 4, .num_segments = 2 },
+ { .num_queues = 1, .num_segments = 16 },
+ { .num_queues = 4, .num_segments = 16 },
+ { .num_queues = 5, .num_segments = 16 },
+ { .num_queues = 15, .num_segments = 16 },
+ { .num_queues = 16, .num_segments = 16 },
+};
+
+static void macb_sram_segments_case_desc(struct macb_sram_segments_case *t,
+ char *desc)
+{
+ u64 val = gem_sram_distribute_segments(t->num_queues, t->num_segments);
+
+ snprintf(desc, KUNIT_PARAM_DESC_SIZE,
+ "num_queues=%d num_segments=%d TXQSEGALLOC=%#llx",
+ t->num_queues, t->num_segments, val);
+}
+
+KUNIT_ARRAY_PARAM(macb_sram_segments,
+ macb_sram_segments_cases,
+ macb_sram_segments_case_desc);
+
+static struct kunit_case macb_test_cases[] = {
+ KUNIT_CASE_PARAM(macb_sram_segments_test,
+ macb_sram_segments_gen_params),
+ {}
+};
+
+static struct kunit_suite macb_test_suite = {
+ .name = "macb",
+ .test_cases = macb_test_cases,
+};
+
+kunit_test_suite(macb_test_suite);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Cadence MACB/GEM Ethernet driver kunit tests");
+MODULE_AUTHOR("Théo Lebrun");
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index dd066f481200..f6c4b31ef75a 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -3053,6 +3053,21 @@ static void macb_init_hw(struct macb *bp)
if (bp->caps & MACB_CAPS_JUMBO)
bp->rx_frm_len_mask = MACB_RX_JFRMLEN_MASK;
+ /*
+ * Distribute Tx SRAM segments evenly based on active number of queues.
+ */
+ if (macb_is_gem(bp)) {
+ unsigned int num_segments;
+ u64 val;
+
+ num_segments = 1U << GEM_BFEXT(SEGMENTS_BIT_SIZE,
+ gem_readl(bp, DCFG6));
+ val = gem_sram_distribute_segments(bp->num_queues,
+ num_segments);
+ gem_writel(bp, TXQSEGALLOC_LOWER, val);
+ gem_writel(bp, TXQSEGALLOC_UPPER, val >> 32);
+ }
+
macb_configure_dma(bp);
/* Enable RX partial store and forward and set watermark */
diff --git a/drivers/net/ethernet/cadence/macb_utils.c b/drivers/net/ethernet/cadence/macb_utils.c
new file mode 100644
index 000000000000..77e0b5c1df86
--- /dev/null
+++ b/drivers/net/ethernet/cadence/macb_utils.c
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/netdevice.h>
+#include "macb.h"
+
+/*
+ * Distribute evenly available segments across queues. The computation is
+ * complex because (1) segments are counted in powers of two and (2) a queue
+ * can only use up to 8 segments. There are four types of cases:
+ * - Sharing all segments equally is doable. Take num_queues=4 and
+ * num_segments=16. Each queue will get 2^2=4 segments.
+ * - Sharing all segments is doable. Take num_queues=5 and num_segments=16.
+ * Three queues will get 2^2=4 segments and two will get 2^1=2 segments.
+ * - Sharing all segments is not doable because not enough queues are
+ * available. Take num_queues=1 and num_segments=16; queue 0 can only have 8
+ * segments.
+ * - Sharing all segments is not doable because not enough segments are
+ * available. Take num_queues=4 and num_segments=2.
+ *
+ * We start by computing the power each queue will have. For num_queues=5 and
+ * num_segments=16, each queue will have at least 2^1 segments. That leaves us
+ * with remaining_segments=6. If we increase the power for a queue, we get a
+ * delta of 2 (2^2-2^1). The first three queues will therefore be advantaged
+ * and each have 2^2 segments. The remaining 2 queues will only have 2^1
+ * segments.
+ */
+u64 gem_sram_distribute_segments(unsigned int num_queues,
+ unsigned int num_segments)
+{
+ unsigned int pow, remaining_segments, i;
+ unsigned int num_advantaged_queues = 0;
+ u64 val = 0;
+
+ /* pow=0 for all queues. ilog2(0) is dangerous. */
+ if (num_queues >= num_segments)
+ return 0;
+
+ pow = min(ilog2(num_segments / num_queues), 3);
+ remaining_segments = num_segments - num_queues * (1U << pow);
+
+ /*
+ * We can only distribute remaining segments if (1) there are remaining
+ * segments and (2) we did not reach the max segments per queue (2^3).
+ */
+ if (remaining_segments != 0 && pow != 3) {
+ unsigned int delta = (1U << (pow + 1)) - (1U << pow);
+
+ num_advantaged_queues = remaining_segments / delta;
+ }
+
+ for (i = 0; i < num_advantaged_queues; i++)
+ val |= ((pow + 1) & 0b11) << (i * 4);
+ for (i = num_advantaged_queues; i < num_queues; i++)
+ val |= (pow & 0b11) << (i * 4);
+
+ return val;
+}
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread