[PATCH net/next 0/3] net: ethernet: mtk_eth_soc: improve device tree handling

[PATCH net/next 0/3] net: ethernet: mtk_eth_soc: improve device tree handling

[Daniel Golle]

The series further improves the mtk_eth_soc driver in preparation to
complete upstream support for the MT7988 SoC family.

Frank Wunderlich's previous attempt to have the ethernet node included
in mt7988a.dtsi and cover support for MT7988 in the device tree bindings
was criticized for the way mtk_eth_soc references SRAM in device tree[1].

Having a 2nd 'reg' property, like introduced by commit ebb1e4f9cf38
("net: ethernet: mtk_eth_soc: add support for in-SoC SRAM") isn't acceptable
and a dedicated "mmio-sram" node should be used instead.

Support for the hard-coded offset and including the SRAM region as part of
the Ethernet's "reg" MMIO space will still be required in order to support
existing legacy device trees of the MT7981 and MT7986 SoC families.

While at it also replace confusing error messages when using legacy device
trees without "interrupt-names" with a warning informing users that they
are using a legacy device tree.

[1]: https://patchwork.ozlabs.org/comment/3533543/

Daniel Golle (3):
  net: ethernet: mtk_eth_soc: improve support for named interrupts
  net: ethernet: mtk_eth_soc: fix kernel-doc comment
  net: ethernet: mtk_eth_soc: use genpool allocator for SRAM

 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 134 +++++++++++++-------
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |   5 +-
 2 files changed, 94 insertions(+), 45 deletions(-)

-- 
2.50.0

[PATCH net/next 1/3] net: ethernet: mtk_eth_soc: improve support for named interrupts

[Daniel Golle]

Use platform_get_irq_byname_optional() to avoid outputting error
messages when using legacy device trees which rely identifying
interrupts only by index. Instead, output a warning notifying the user
to update their device tree.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index f8a907747db4..8f55069441f4 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -3341,17 +3341,22 @@ static int mtk_get_irqs(struct platform_device *pdev, struct mtk_eth *eth)
 	int i;
 
 	/* future SoCs beginning with MT7988 should use named IRQs in dts */
-	eth->irq[MTK_FE_IRQ_TX] = platform_get_irq_byname(pdev, "fe1");
-	eth->irq[MTK_FE_IRQ_RX] = platform_get_irq_byname(pdev, "fe2");
+	eth->irq[MTK_FE_IRQ_TX] = platform_get_irq_byname_optional(pdev, "fe1");
+	eth->irq[MTK_FE_IRQ_RX] = platform_get_irq_byname_optional(pdev, "fe2");
 	if (eth->irq[MTK_FE_IRQ_TX] >= 0 && eth->irq[MTK_FE_IRQ_RX] >= 0)
 		return 0;
 
-	/* only use legacy mode if platform_get_irq_byname returned -ENXIO */
+	/* only use legacy mode if platform_get_irq_byname_optional returned -ENXIO */
 	if (eth->irq[MTK_FE_IRQ_TX] != -ENXIO)
-		return eth->irq[MTK_FE_IRQ_TX];
+		return dev_err_probe(&pdev->dev, eth->irq[MTK_FE_IRQ_TX],
+				     "Error requesting FE TX IRQ\n");
 
 	if (eth->irq[MTK_FE_IRQ_RX] != -ENXIO)
-		return eth->irq[MTK_FE_IRQ_RX];
+		return dev_err_probe(&pdev->dev, eth->irq[MTK_FE_IRQ_RX],
+				     "Error requesting FE RX IRQ\n");
+
+	if (!MTK_HAS_CAPS(eth->soc->caps, MTK_SHARED_INT))
+		dev_warn(&pdev->dev, "legacy DT: missing interrupt-names.");
 
 	/* legacy way:
 	 * On MTK_SHARED_INT SoCs (MT7621 + MT7628) the first IRQ is taken
-- 
2.50.0

[PATCH net/next 2/3] net: ethernet: mtk_eth_soc: fix kernel-doc comment

[Daniel Golle]

Fix and add some missing field descriptions to kernel-doc comment of
struct mtk_eth.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 9261c0e13b59..1ad9075a9b69 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -1243,8 +1243,9 @@ struct mtk_soc_data {
 /* struct mtk_eth -	This is the main datasructure for holding the state
  *			of the driver
  * @dev:		The device pointer
- * @dev:		The device pointer used for dma mapping/alloc
+ * @dma_dev:		The device pointer used for dma mapping/alloc
  * @base:		The mapped register i/o base
+ * @sram_base:		The mapped SRAM base
  * @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
-- 
2.50.0

[PATCH net/next 3/3] net: ethernet: mtk_eth_soc: use genpool allocator for SRAM

[Daniel Golle]

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,
+						       &eth->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,
-						       &eth->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 = &eth->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.0

Re: [PATCH net/next 1/3] net: ethernet: mtk_eth_soc: improve support for named interrupts

[Andrew Lunn]

On Sat, Jun 28, 2025 at 02:30:23AM +0100, Daniel Golle wrote:
> Use platform_get_irq_byname_optional() to avoid outputting error
> messages when using legacy device trees which rely identifying
> interrupts only by index. Instead, output a warning notifying the user
> to update their device tree.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net/next 2/3] net: ethernet: mtk_eth_soc: fix kernel-doc comment

[Andrew Lunn]

On Sat, Jun 28, 2025 at 02:30:31AM +0100, Daniel Golle wrote:
> Fix and add some missing field descriptions to kernel-doc comment of
> struct mtk_eth.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net/next 3/3] net: ethernet: mtk_eth_soc: use genpool allocator for SRAM

[Andrew Lunn]

> +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

Re: [PATCH net/next 3/3] net: ethernet: mtk_eth_soc: use genpool allocator for SRAM

[kernel test robot]

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

Re: [PATCH net/next 3/3] net: ethernet: mtk_eth_soc: use genpool allocator for SRAM

[kernel test robot]

Hi 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

Re: [PATCH net/next 3/3] net: ethernet: mtk_eth_soc: use genpool allocator for SRAM

[Daniel Golle]

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.

Re: [PATCH net/next 3/3] net: ethernet: mtk_eth_soc: use genpool allocator for SRAM

[Frank Wunderlich]

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/

Re: [PATCH net/next 3/3] net: ethernet: mtk_eth_soc: use genpool allocator for SRAM

[Andrew Lunn]

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