Use a dedicated "mmio-sram" and the genpool allocator instead of
open-coding SRAM allocation for DMA rings.
Keep support for legacy device trees but notify the user via a
warning.
Co-developed-by: Frank Wunderlich <frank-w@public-files.de>
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 119 +++++++++++++-------
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 4 +-
2 files changed, 83 insertions(+), 40 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 8f55069441f4..adae59a3dfa4 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -27,6 +27,7 @@
#include <net/dsa.h>
#include <net/dst_metadata.h>
#include <net/page_pool/helpers.h>
+#include <linux/genalloc.h>
#include "mtk_eth_soc.h"
#include "mtk_wed.h"
@@ -1267,6 +1268,44 @@ static void *mtk_max_lro_buf_alloc(gfp_t gfp_mask)
return (void *)data;
}
+static bool mtk_use_legacy_sram(struct mtk_eth *eth)
+{
+ return !eth->sram_pool && MTK_HAS_CAPS(eth->soc->caps, MTK_SRAM);
+}
+
+static void *mtk_dma_ring_alloc(struct mtk_eth *eth, size_t size,
+ dma_addr_t *dma_handle)
+{
+ void *dma_ring;
+
+ if (WARN_ON(mtk_use_legacy_sram(eth)))
+ return -ENOMEM;
+
+ if (eth->sram_pool) {
+ dma_ring = (void *)gen_pool_alloc(eth->sram_pool, size);
+ if (!dma_ring)
+ return dma_ring;
+ *dma_handle = gen_pool_virt_to_phys(eth->sram_pool, (unsigned long)dma_ring);
+ } else {
+ dma_ring = dma_alloc_coherent(eth->dma_dev, size, dma_handle,
+ GFP_KERNEL);
+ }
+
+ return dma_ring;
+}
+
+static void mtk_dma_ring_free(struct mtk_eth *eth, size_t size, void *dma_ring,
+ dma_addr_t dma_handle)
+{
+ if (WARN_ON(mtk_use_legacy_sram(eth)))
+ return;
+
+ if (eth->sram_pool)
+ gen_pool_free(eth->sram_pool, (unsigned long)dma_ring, size);
+ else
+ dma_free_coherent(eth->dma_dev, size, dma_ring, dma_handle);
+}
+
/* the qdma core needs scratch memory to be setup */
static int mtk_init_fq_dma(struct mtk_eth *eth)
{
@@ -1276,13 +1315,12 @@ static int mtk_init_fq_dma(struct mtk_eth *eth)
dma_addr_t dma_addr;
int i, j, len;
- if (MTK_HAS_CAPS(eth->soc->caps, MTK_SRAM))
+ if (!mtk_use_legacy_sram(eth)) {
+ eth->scratch_ring = mtk_dma_ring_alloc(eth, cnt * soc->tx.desc_size,
+ ð->phy_scratch_ring);
+ } else {
eth->scratch_ring = eth->sram_base;
- else
- eth->scratch_ring = dma_alloc_coherent(eth->dma_dev,
- cnt * soc->tx.desc_size,
- ð->phy_scratch_ring,
- GFP_KERNEL);
+ }
if (unlikely(!eth->scratch_ring))
return -ENOMEM;
@@ -2620,12 +2658,11 @@ static int mtk_tx_alloc(struct mtk_eth *eth)
if (!ring->buf)
goto no_tx_mem;
- if (MTK_HAS_CAPS(soc->caps, MTK_SRAM)) {
+ if (!mtk_use_legacy_sram(eth)) {
+ ring->dma = mtk_dma_ring_alloc(eth, ring_size * sz, &ring->phys);
+ } else {
ring->dma = eth->sram_base + soc->tx.fq_dma_size * sz;
ring->phys = eth->phy_scratch_ring + soc->tx.fq_dma_size * (dma_addr_t)sz;
- } else {
- ring->dma = dma_alloc_coherent(eth->dma_dev, ring_size * sz,
- &ring->phys, GFP_KERNEL);
}
if (!ring->dma)
@@ -2726,9 +2763,9 @@ static void mtk_tx_clean(struct mtk_eth *eth)
kfree(ring->buf);
ring->buf = NULL;
}
- if (!MTK_HAS_CAPS(soc->caps, MTK_SRAM) && ring->dma) {
- dma_free_coherent(eth->dma_dev,
- ring->dma_size * soc->tx.desc_size,
+
+ if (!mtk_use_legacy_sram(eth) && ring->dma) {
+ mtk_dma_ring_free(eth, ring->dma_size * soc->tx.desc_size,
ring->dma, ring->phys);
ring->dma = NULL;
}
@@ -2793,6 +2830,9 @@ static int mtk_rx_alloc(struct mtk_eth *eth, int ring_no, int rx_flag)
ring->dma = dma_alloc_coherent(eth->dma_dev,
rx_dma_size * eth->soc->rx.desc_size,
&ring->phys, GFP_KERNEL);
+ } else if (eth->sram_pool) {
+ ring->dma = mtk_dma_ring_alloc(eth, rx_dma_size * eth->soc->rx.desc_size,
+ &ring->phys);
} else {
struct mtk_tx_ring *tx_ring = ð->tx_ring;
@@ -2921,6 +2961,11 @@ static void mtk_rx_clean(struct mtk_eth *eth, struct mtk_rx_ring *ring, bool in_
ring->dma_size * eth->soc->rx.desc_size,
ring->dma, ring->phys);
ring->dma = NULL;
+ } else if (!mtk_use_legacy_sram(eth) && ring->dma) {
+ mtk_dma_ring_free(eth,
+ ring->dma_size * eth->soc->rx.desc_size,
+ ring->dma, ring->phys);
+ ring->dma = NULL;
}
if (ring->page_pool) {
@@ -3287,9 +3332,8 @@ static void mtk_dma_free(struct mtk_eth *eth)
netdev_tx_reset_subqueue(eth->netdev[i], j);
}
- if (!MTK_HAS_CAPS(soc->caps, MTK_SRAM) && eth->scratch_ring) {
- dma_free_coherent(eth->dma_dev,
- MTK_QDMA_RING_SIZE * soc->tx.desc_size,
+ if (!mtk_use_legacy_sram(eth) && eth->scratch_ring) {
+ mtk_dma_ring_free(eth, soc->tx.fq_dma_size * soc->tx.desc_size,
eth->scratch_ring, eth->phy_scratch_ring);
eth->scratch_ring = NULL;
eth->phy_scratch_ring = 0;
@@ -5009,7 +5053,7 @@ static int mtk_sgmii_init(struct mtk_eth *eth)
static int mtk_probe(struct platform_device *pdev)
{
- struct resource *res = NULL, *res_sram;
+ struct resource *res = NULL;
struct device_node *mac_np;
struct mtk_eth *eth;
int err, i;
@@ -5029,20 +5073,6 @@ static int mtk_probe(struct platform_device *pdev)
if (MTK_HAS_CAPS(eth->soc->caps, MTK_SOC_MT7628))
eth->ip_align = NET_IP_ALIGN;
- if (MTK_HAS_CAPS(eth->soc->caps, MTK_SRAM)) {
- /* SRAM is actual memory and supports transparent access just like DRAM.
- * Hence we don't require __iomem being set and don't need to use accessor
- * functions to read from or write to SRAM.
- */
- if (mtk_is_netsys_v3_or_greater(eth)) {
- eth->sram_base = (void __force *)devm_platform_ioremap_resource(pdev, 1);
- if (IS_ERR(eth->sram_base))
- return PTR_ERR(eth->sram_base);
- } else {
- eth->sram_base = (void __force *)eth->base + MTK_ETH_SRAM_OFFSET;
- }
- }
-
if (MTK_HAS_CAPS(eth->soc->caps, MTK_36BIT_DMA)) {
err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(36));
if (!err)
@@ -5117,16 +5147,27 @@ static int mtk_probe(struct platform_device *pdev)
err = -EINVAL;
goto err_destroy_sgmii;
}
+
if (MTK_HAS_CAPS(eth->soc->caps, MTK_SRAM)) {
- if (mtk_is_netsys_v3_or_greater(eth)) {
- res_sram = platform_get_resource(pdev, IORESOURCE_MEM, 1);
- if (!res_sram) {
- err = -EINVAL;
- goto err_destroy_sgmii;
+ eth->sram_pool = of_gen_pool_get(pdev->dev.of_node, "sram", 0);
+ if (!eth->sram_pool) {
+ if (!mtk_is_netsys_v3_or_greater(eth)) {
+ /*
+ * Legacy support for missing 'sram' node in DT.
+ * SRAM is actual memory and supports transparent access
+ * just like DRAM. Hence we don't require __iomem being
+ * set and don't need to use accessor functions to read from
+ * or write to SRAM.
+ */
+ eth->sram_base = (void __force *)eth->base +
+ MTK_ETH_SRAM_OFFSET;
+ eth->phy_scratch_ring = res->start + MTK_ETH_SRAM_OFFSET;
+ dev_warn(&pdev->dev,
+ "legacy DT: using hard-coded SRAM offset.\n");
+ } else {
+ dev_err(&pdev->dev, "Could not get SRAM pool\n");
+ return -ENODEV;
}
- eth->phy_scratch_ring = res_sram->start;
- } else {
- eth->phy_scratch_ring = res->start + MTK_ETH_SRAM_OFFSET;
}
}
}
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 1ad9075a9b69..0104659e37f0 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -1245,7 +1245,8 @@ struct mtk_soc_data {
* @dev: The device pointer
* @dma_dev: The device pointer used for dma mapping/alloc
* @base: The mapped register i/o base
- * @sram_base: The mapped SRAM base
+ * @sram_base: The mapped SRAM base (deprecated)
+ * @sram_pool: Pointer to SRAM pool used for DMA descriptor rings
* @page_lock: Make sure that register operations are atomic
* @tx_irq__lock: Make sure that IRQ register operations are atomic
* @rx_irq__lock: Make sure that IRQ register operations are atomic
@@ -1292,6 +1293,7 @@ struct mtk_eth {
struct device *dma_dev;
void __iomem *base;
void *sram_base;
+ struct gen_pool *sram_pool;
spinlock_t page_lock;
spinlock_t tx_irq_lock;
spinlock_t rx_irq_lock;
--
2.50.0Hi Daniel,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
[also build test ERROR on next-20250627]
[cannot apply to net/main linus/master v6.16-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Daniel-Golle/net-ethernet-mtk_eth_soc-improve-support-for-named-interrupts/20250628-093324
base: net-next/main
patch link: https://lore.kernel.org/r/566ca90fc59ad0d3aff8bc8dc22ebaf0544bce47.1751072868.git.daniel%40makrotopia.org
patch subject: [PATCH net/next 3/3] net: ethernet: mtk_eth_soc: use genpool allocator for SRAM
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20250629/202506290627.8dPD2PJ1-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250629/202506290627.8dPD2PJ1-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506290627.8dPD2PJ1-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/net/ethernet/mediatek/mtk_eth_soc.c: In function 'mtk_dma_ring_alloc':
>> drivers/net/ethernet/mediatek/mtk_eth_soc.c:1282:24: error: returning 'int' from a function with return type 'void *' makes pointer from integer without a cast [-Wint-conversion]
1282 | return -ENOMEM;
| ^
vim +1282 drivers/net/ethernet/mediatek/mtk_eth_soc.c
1275
1276 static void *mtk_dma_ring_alloc(struct mtk_eth *eth, size_t size,
1277 dma_addr_t *dma_handle)
1278 {
1279 void *dma_ring;
1280
1281 if (WARN_ON(mtk_use_legacy_sram(eth)))
> 1282 return -ENOMEM;
1283
1284 if (eth->sram_pool) {
1285 dma_ring = (void *)gen_pool_alloc(eth->sram_pool, size);
1286 if (!dma_ring)
1287 return dma_ring;
1288 *dma_handle = gen_pool_virt_to_phys(eth->sram_pool, (unsigned long)dma_ring);
1289 } else {
1290 dma_ring = dma_alloc_coherent(eth->dma_dev, size, dma_handle,
1291 GFP_KERNEL);
1292 }
1293
1294 return dma_ring;
1295 }
1296
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Daniel,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
[also build test WARNING on next-20250627]
[cannot apply to net/main linus/master v6.16-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Daniel-Golle/net-ethernet-mtk_eth_soc-improve-support-for-named-interrupts/20250628-093324
base: net-next/main
patch link: https://lore.kernel.org/r/566ca90fc59ad0d3aff8bc8dc22ebaf0544bce47.1751072868.git.daniel%40makrotopia.org
patch subject: [PATCH net/next 3/3] net: ethernet: mtk_eth_soc: use genpool allocator for SRAM
config: arm-randconfig-003-20250629 (https://download.01.org/0day-ci/archive/20250629/202506290403.FwUOUzZq-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250629/202506290403.FwUOUzZq-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506290403.FwUOUzZq-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/net/ethernet/mediatek/mtk_eth_soc.c: In function 'mtk_dma_ring_alloc':
>> drivers/net/ethernet/mediatek/mtk_eth_soc.c:1282:10: warning: returning 'int' from a function with return type 'void *' makes pointer from integer without a cast [-Wint-conversion]
return -ENOMEM;
^
vim +1282 drivers/net/ethernet/mediatek/mtk_eth_soc.c
1275
1276 static void *mtk_dma_ring_alloc(struct mtk_eth *eth, size_t size,
1277 dma_addr_t *dma_handle)
1278 {
1279 void *dma_ring;
1280
1281 if (WARN_ON(mtk_use_legacy_sram(eth)))
> 1282 return -ENOMEM;
1283
1284 if (eth->sram_pool) {
1285 dma_ring = (void *)gen_pool_alloc(eth->sram_pool, size);
1286 if (!dma_ring)
1287 return dma_ring;
1288 *dma_handle = gen_pool_virt_to_phys(eth->sram_pool, (unsigned long)dma_ring);
1289 } else {
1290 dma_ring = dma_alloc_coherent(eth->dma_dev, size, dma_handle,
1291 GFP_KERNEL);
1292 }
1293
1294 return dma_ring;
1295 }
1296
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
> +static void *mtk_dma_ring_alloc(struct mtk_eth *eth, size_t size,
> + dma_addr_t *dma_handle)
> +{
> + void *dma_ring;
> +
> + if (WARN_ON(mtk_use_legacy_sram(eth)))
> + return -ENOMEM;
> +
> + if (eth->sram_pool) {
> + dma_ring = (void *)gen_pool_alloc(eth->sram_pool, size);
> + if (!dma_ring)
> + return dma_ring;
> + *dma_handle = gen_pool_virt_to_phys(eth->sram_pool, (unsigned long)dma_ring);
I don't particularly like all the casting backwards and forwards
between unsigned long and void *. These two APIs are not really
compatible with each other. So any sort of wrapping is going to be
messy.
Maybe define a cookie union:
struct mtk_dma_cookie {
union {
unsigned long gen_pool;
void *coherent;
}
}
Only dma_handle appears to be used by the rest of the code, so only
the _alloc and _free need to know about the union.
Andrew
Hi Andrew,
> Gesendet: Samstag, 28. Juni 2025 um 10:13
> Von: "Andrew Lunn" <andrew@lunn.ch>
> Betreff: Re: [PATCH net/next 3/3] net: ethernet: mtk_eth_soc: use genpool allocator for SRAM
>
> > +static void *mtk_dma_ring_alloc(struct mtk_eth *eth, size_t size,
> > + dma_addr_t *dma_handle)
> > +{
> > + void *dma_ring;
> > +
> > + if (WARN_ON(mtk_use_legacy_sram(eth)))
> > + return -ENOMEM;
> > +
> > + if (eth->sram_pool) {
> > + dma_ring = (void *)gen_pool_alloc(eth->sram_pool, size);
> > + if (!dma_ring)
> > + return dma_ring;
> > + *dma_handle = gen_pool_virt_to_phys(eth->sram_pool, (unsigned long)dma_ring);
>
> I don't particularly like all the casting backwards and forwards
> between unsigned long and void *. These two APIs are not really
> compatible with each other. So any sort of wrapping is going to be
> messy.
>
> Maybe define a cookie union:
>
> struct mtk_dma_cookie {
> union {
> unsigned long gen_pool;
> void *coherent;
> }
> }
>
> Only dma_handle appears to be used by the rest of the code, so only
> the _alloc and _free need to know about the union.
do you mean use the union only for the casts or using it globally for all the access
to the dma struct (and so changing the return type of the alloc function)?
first i made here [1]
second was tried by daniel and is much more change.
OT: btw. have you took a look in the PCS decision case [1]?
regards Frank
[1] https://github.com/frank-w/BPI-Router-Linux/commit/ea963012375e4ac98947a703b5eaf21fdf221ee1
[2] https://lore.kernel.org/netdev/24c4dfe9-ae3a-4126-b4ec-baac7754a669@linux.dev/
On Sat, Jun 28, 2025 at 10:13:51AM +0200, Andrew Lunn wrote:
> > +static void *mtk_dma_ring_alloc(struct mtk_eth *eth, size_t size,
> > + dma_addr_t *dma_handle)
> > +{
> > + void *dma_ring;
> > +
> > + if (WARN_ON(mtk_use_legacy_sram(eth)))
> > + return -ENOMEM;
> > +
> > + if (eth->sram_pool) {
> > + dma_ring = (void *)gen_pool_alloc(eth->sram_pool, size);
> > + if (!dma_ring)
> > + return dma_ring;
> > + *dma_handle = gen_pool_virt_to_phys(eth->sram_pool, (unsigned long)dma_ring);
>
> I don't particularly like all the casting backwards and forwards
> between unsigned long and void *. These two APIs are not really
> compatible with each other. So any sort of wrapping is going to be
> messy.
>
> Maybe define a cookie union:
>
> struct mtk_dma_cookie {
> union {
> unsigned long gen_pool;
> void *coherent;
> }
> }
I've implemented that idea and the diffstat grew quite a lot. Also,
it didn't really make the code more readable (see below why).
>
> Only dma_handle appears to be used by the rest of the code, so only
> the _alloc and _free need to know about the union.
That's not true. The void* ring->dma is used to access the RX and TX
descriptors, so keeping it void* is useful and using the struct you
are suggesting will make things even more messy than they already are.
See all the places in the code where we assume ring->dma being void*.
Converting all of those to use struct mtk_dma_cookie will not make things
better imho.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/mediatek/mtk_eth_soc.c#n1337
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/mediatek/mtk_eth_soc.c#n1345
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/mediatek/mtk_eth_soc.c#n1358
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/mediatek/mtk_eth_soc.c#n1804
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/mediatek/mtk_eth_soc.c#n2172
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/mediatek/mtk_eth_soc.c#n2490
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/mediatek/mtk_eth_soc.c#n2638
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/mediatek/mtk_eth_soc.c#n2668
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/mediatek/mtk_eth_soc.c#n2904
I think keeping the two casts in mtk_dma_ring_alloc() and
mtk_dma_ring_free() is the better option.
On Sun, Jun 29, 2025 at 03:25:25PM +0100, Daniel Golle wrote:
> On Sat, Jun 28, 2025 at 10:13:51AM +0200, Andrew Lunn wrote:
> > > +static void *mtk_dma_ring_alloc(struct mtk_eth *eth, size_t size,
> > > + dma_addr_t *dma_handle)
> > > +{
> > > + void *dma_ring;
> > > +
> > > + if (WARN_ON(mtk_use_legacy_sram(eth)))
> > > + return -ENOMEM;
> > > +
> > > + if (eth->sram_pool) {
> > > + dma_ring = (void *)gen_pool_alloc(eth->sram_pool, size);
> > > + if (!dma_ring)
> > > + return dma_ring;
> > > + *dma_handle = gen_pool_virt_to_phys(eth->sram_pool, (unsigned long)dma_ring);
> >
> > I don't particularly like all the casting backwards and forwards
> > between unsigned long and void *. These two APIs are not really
> > compatible with each other. So any sort of wrapping is going to be
> > messy.
> >
> > Maybe define a cookie union:
> >
> > struct mtk_dma_cookie {
> > union {
> > unsigned long gen_pool;
> > void *coherent;
> > }
> > }
>
> I've implemented that idea and the diffstat grew quite a lot. Also,
> it didn't really make the code more readable (see below why).
O.K, thanks for trying. Please keep with the casts back and forth.
Andrew
© 2016 - 2025 Red Hat, Inc.