[PATCH net-next v2 0/6] net: ftgmac100: Add AST2700 support

[PATCH net-next v2 0/6] net: ftgmac100: Add AST2700 support

[Jacky Chou]

Add the pieces needed for the FTGMAC100 driver to run on Aspeed AST2700
systems.

AST2700 keeps using the FTGMAC100 MAC IP, but the enablement is not
limited to a new compatible string. The SoC can boot with the MAC on
ARM64, needs the Aspeed-specific RMII mode bit programmed when a port is
wired for RMII, and requires the driver to use the upper DMA address
fields in the ring base registers and packet descriptors. Those fields
expose a 35-bit DMA address range on AST2700, so the driver must request
a mask that matches the address bits the hardware can encode.

The series first documents and wires up the aspeed,ast2700-mac
compatible. It then treats AST2700 as an Aspeed MAC in the driver,
enables the RMII mode programming, writes the AST2700 high descriptor-ring
base registers, carries the RX/TX descriptor high address bits, rebuilds
full DMA addresses before unmapping buffers, and requests a 35-bit DMA
mask. The high ring base registers are only touched for AST2700 so older
Aspeed device tree register windows remain unchanged. With that in place,
the FTGMAC100 Kconfig entry can be made available on ARM64 while keeping
the existing Aspeed MDIO dependency for AST2600 and newer ARCH_ASPEED
systems.

RGMII delay is configured at the bootloader stage, the same way it is
handled for AST2600, not by this driver. Aspeed's SDK bootloader
configures both AST2600 and AST2700 boards with the RGMII delay
disabled and advertises "phy-mode = rgmii-id" to match, mirroring the
OpenBMC bootloader change that stopped inserting delays on AST2600 so
its device trees could use the correct "rgmii-id" setting. On AST2700,
every on-chip MAC is likewise configured by the bootloader with RGMII
delay disabled and "rgmii-id" phy-mode. Because the delay is already
resolved before Linux boots, the ftgmac100 driver does not need to
program any RGMII delay for either AST2600 or AST2700.

This series has been validated on AST2700 EVB and AST2600 EVB.

Patch layout:
- patch 1 updates the binding for the AST2700 compatible, RMII RCLK gate,
  and reset support.
- patch 2 adds the AST2700 match data and OF compatible.
- patch 3 programs the AST2700 RMII enable bit.
- patch 4 makes "phy-mode" a required device tree property for AST2700
  and fails probe if it is missing.
- patch 5 adds AST2700 upper DMA address handling for rings and
  descriptors.
- patch 6 enables the Faraday/FTGMAC100 Kconfig options on ARM64 and
  selects MDIO_ASPEED for ARCH_ASPEED.

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
Changes in v2:
- Make "phy-mode" a required device tree property for AST2700 and fail
  probe if it is missing, instead of silently defaulting to RGMII.
- Document in the cover letter how RGMII delay is configured at the
  bootloader stage for AST2600 and AST2700.
- Simplify the MDIO_ASPEED Kconfig select to ARCH_ASPEED only, dropping
  the redundant MACH_ASPEED_G6 condition, and update its help text to
  mention AST2700.
- Pick up Reviewed-by from Krzysztof Kozlowski on the dt-bindings patch.
- Link to v1: https://lore.kernel.org/r/20260927-support_ast2700-v1-0-9f03f29d2c82@aspeedtech.com

---
Jacky Chou (6):
      dt-bindings: net: ftgmac100: Add AST2700 compatible
      net: ftgmac100: Add AST2700 compatible support
      net: ftgmac100: Enable AST2700 RMII support
      net: ftgmac100: Require phy-mode for AST2700
      net: ftgmac100: Add AST2700 upper DMA address support
      net: ftgmac100: Allow building on ARM64

 .../devicetree/bindings/net/faraday,ftgmac100.yaml |  4 +-
 drivers/net/ethernet/faraday/Kconfig               |  7 +-
 drivers/net/ethernet/faraday/ftgmac100.c           | 93 +++++++++++++++++++---
 drivers/net/ethernet/faraday/ftgmac100.h           |  7 ++
 drivers/net/mdio/Kconfig                           | 16 ++--
 5 files changed, 104 insertions(+), 23 deletions(-)
---
base-commit: a5e7d8e446af9803e37a3b6a4d416fb41178348f
change-id: 20260623-support_ast2700-68fb164da45c

Best regards,
-- 
Jacky Chou <jacky_chou@aspeedtech.com>

[PATCH net-next v2 1/6] dt-bindings: net: ftgmac100: Add AST2700 compatible

[Jacky Chou]

Add AST2700 to the FTGMAC100 compatible list and document that its
RMII RCLK gate matches the earlier Aspeed SoCs. Allow the reset line for
AST2700 as it follows the same reset requirements as AST2600.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
index d14410018bcf..bee9570a6f63 100644
--- a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
+++ b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
@@ -18,6 +18,7 @@ properties:
               - aspeed,ast2400-mac
               - aspeed,ast2500-mac
               - aspeed,ast2600-mac
+              - aspeed,ast2700-mac
           - const: faraday,ftgmac100
 
   reg:
@@ -30,7 +31,7 @@ properties:
     minItems: 1
     items:
       - description: MAC IP clock
-      - description: RMII RCLK gate for AST2500/2600
+      - description: RMII RCLK gate for AST2500/2600/2700
 
   resets:
     maxItems: 1
@@ -82,6 +83,7 @@ allOf:
           contains:
             enum:
               - aspeed,ast2600-mac
+              - aspeed,ast2700-mac
     then:
       properties:
         resets: true

-- 
2.34.1

[PATCH net-next v2 2/6] net: ftgmac100: Add AST2700 compatible support

[Jacky Chou]

Add match data for the Aspeed AST2700 FTGMAC100 controller and wire
its compatible string into the OF match table. This lets AST2700 device
tree nodes bind to the ftgmac100 driver.

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 6d2fe5c2f390..67b1fa464a42 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -37,7 +37,8 @@ enum ftgmac100_mac_id {
 	FTGMAC100_FARADAY = 1,
 	FTGMAC100_AST2400,
 	FTGMAC100_AST2500,
-	FTGMAC100_AST2600
+	FTGMAC100_AST2600,
+	FTGMAC100_AST2700
 };
 
 struct ftgmac100_match_data {
@@ -2017,7 +2018,8 @@ static int ftgmac100_probe(struct platform_device *pdev)
 
 	if (priv->mac_id == FTGMAC100_AST2400 ||
 	    priv->mac_id == FTGMAC100_AST2500 ||
-	    priv->mac_id == FTGMAC100_AST2600) {
+	    priv->mac_id == FTGMAC100_AST2600 ||
+	    priv->mac_id == FTGMAC100_AST2700) {
 		priv->rxdes0_edorr_mask = BIT(30);
 		priv->txdes0_edotr_mask = BIT(30);
 		priv->is_aspeed = true;
@@ -2131,6 +2133,10 @@ static const struct ftgmac100_match_data ftgmac100_match_data_ast2600 = {
 	.mac_id = FTGMAC100_AST2600
 };
 
+static const struct ftgmac100_match_data ftgmac100_match_data_ast2700 = {
+	.mac_id = FTGMAC100_AST2700
+};
+
 static const struct ftgmac100_match_data ftgmac100_match_data_faraday = {
 	.mac_id = FTGMAC100_FARADAY
 };
@@ -2142,6 +2148,8 @@ static const struct of_device_id ftgmac100_of_match[] = {
 	  .data = &ftgmac100_match_data_ast2500 },
 	{ .compatible = "aspeed,ast2600-mac",
 	  .data = &ftgmac100_match_data_ast2600 },
+	{ .compatible = "aspeed,ast2700-mac",
+	  .data = &ftgmac100_match_data_ast2700 },
 	{ .compatible = "faraday,ftgmac100",
 	  .data = &ftgmac100_match_data_faraday },
 	{ }

-- 
2.34.1

[PATCH net-next v2 3/6] net: ftgmac100: Enable AST2700 RMII support

[Jacky Chou]

Set the RMII enable bit when an AST2700 port uses RMII so the MAC is
programmed for the selected interface mode. Describe the requirement as
a match-data quirk instead of checking the MAC generation in the data
path, allowing later compatible controllers to opt into the behavior.

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 16 ++++++++++++++--
 drivers/net/ethernet/faraday/ftgmac100.h |  1 +
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 67b1fa464a42..b835472da360 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -41,8 +41,11 @@ enum ftgmac100_mac_id {
 	FTGMAC100_AST2700
 };
 
+#define FTGMAC100_QUIRK_RMII_ENABLE	BIT(0)
+
 struct ftgmac100_match_data {
 	enum ftgmac100_mac_id mac_id;
+	u32 quirks;
 };
 
 /* Arbitrary values, I am not sure the HW has limits */
@@ -79,6 +82,7 @@ struct ftgmac100 {
 	void __iomem *base;
 
 	enum ftgmac100_mac_id mac_id;
+	u32 quirks;
 
 	/* Rx ring */
 	unsigned int rx_q_entries;
@@ -355,6 +359,7 @@ static void ftgmac100_init_hw(struct ftgmac100 *priv)
 static void ftgmac100_start_hw(struct ftgmac100 *priv)
 {
 	u32 maccr = ioread32(priv->base + FTGMAC100_OFFSET_MACCR);
+	struct phy_device *phydev = priv->netdev->phydev;
 
 	/* Keep the original GMAC and FAST bits */
 	maccr &= (FTGMAC100_MACCR_FAST_MODE | FTGMAC100_MACCR_GIGA_MODE);
@@ -383,6 +388,11 @@ static void ftgmac100_start_hw(struct ftgmac100 *priv)
 	if (priv->netdev->features & NETIF_F_HW_VLAN_CTAG_RX)
 		maccr |= FTGMAC100_MACCR_RM_VLAN;
 
+	if ((priv->quirks & FTGMAC100_QUIRK_RMII_ENABLE) &&
+	    phydev->interface == PHY_INTERFACE_MODE_RMII) {
+		maccr |= FTGMAC100_MACCR_RMII_ENABLE;
+	}
+
 	/* Hit the HW */
 	iowrite32(maccr, priv->base + FTGMAC100_OFFSET_MACCR);
 }
@@ -1942,7 +1952,7 @@ static int ftgmac100_probe_dt(struct net_device *netdev,
 
 static int ftgmac100_probe(struct platform_device *pdev)
 {
-	const struct ftgmac100_match_data *match_data;
+	const struct ftgmac100_match_data *match_data = NULL;
 	enum ftgmac100_mac_id mac_id;
 	struct resource *res;
 	int irq;
@@ -1987,6 +1997,7 @@ static int ftgmac100_probe(struct platform_device *pdev)
 	priv->netdev = netdev;
 	priv->dev = &pdev->dev;
 	priv->mac_id = mac_id;
+	priv->quirks = match_data ? match_data->quirks : 0;
 	INIT_WORK(&priv->reset_task, ftgmac100_reset_task);
 
 	/* map io memory */
@@ -2134,7 +2145,8 @@ static const struct ftgmac100_match_data ftgmac100_match_data_ast2600 = {
 };
 
 static const struct ftgmac100_match_data ftgmac100_match_data_ast2700 = {
-	.mac_id = FTGMAC100_AST2700
+	.mac_id = FTGMAC100_AST2700,
+	.quirks = FTGMAC100_QUIRK_RMII_ENABLE,
 };
 
 static const struct ftgmac100_match_data ftgmac100_match_data_faraday = {
diff --git a/drivers/net/ethernet/faraday/ftgmac100.h b/drivers/net/ethernet/faraday/ftgmac100.h
index 4968f6f0bdbc..8c35c895b146 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.h
+++ b/drivers/net/ethernet/faraday/ftgmac100.h
@@ -167,6 +167,7 @@
 #define FTGMAC100_MACCR_RX_BROADPKT	(1 << 17)
 #define FTGMAC100_MACCR_DISCARD_CRCERR	(1 << 18)
 #define FTGMAC100_MACCR_FAST_MODE	(1 << 19)
+#define FTGMAC100_MACCR_RMII_ENABLE	BIT(20)
 #define FTGMAC100_MACCR_SW_RST		(1 << 31)
 
 /*

-- 
2.34.1

[PATCH net-next v2 4/6] net: ftgmac100: Require phy-mode for AST2700

[Jacky Chou]

The driver currently defaults to RGMII when the "phy-mode" property
is missing from the device tree. This default is no longer valid for
AST2700, which also supports RMII via FTGMAC100_QUIRK_RMII_ENABLE.

Make "phy-mode" a required property for AST2700 and fail probe with
an error message if it is not specified, instead of silently
defaulting to RGMII.

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index b835472da360..2d029d1262e6 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1521,8 +1521,13 @@ static int ftgmac100_mii_probe(struct net_device *netdev)
 
 	/* Default to RGMII. It's a gigabit part after all */
 	err = of_get_phy_mode(np, &phy_intf);
-	if (err)
+	if (err) {
+		if (priv->mac_id == FTGMAC100_AST2700) {
+			dev_err(priv->dev, "phy-mode is required for this SoC\n");
+			return -EINVAL;
+		}
 		phy_intf = PHY_INTERFACE_MODE_RGMII;
+	}
 
 	/* Aspeed only supports these. I don't know about other IP
 	 * block vendors so I'm going to just let them through for

-- 
2.34.1

[PATCH net-next v2 5/6] net: ftgmac100: Add AST2700 upper DMA address support

[Jacky Chou]

Program the high RX and TX descriptor ring base registers and carry the
upper DMA address bits in RX and TX descriptors. Rebuild full buffer DMA
addresses in a 64-bit type before unmapping them so bits 34:32 are not
lost to 32-bit arithmetic.

The descriptor format provides three upper buffer address bits, so
request a 35-bit DMA mask instead of advertising an address range that
the hardware cannot encode. Gate all extended address handling with a
match-data quirk so older register layouts remain untouched and later
compatible controllers can opt into the same capability.

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 62 +++++++++++++++++++++++++++-----
 drivers/net/ethernet/faraday/ftgmac100.h |  6 ++++
 2 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 2d029d1262e6..af2e272f32f2 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -8,6 +8,7 @@
 
 #define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
 
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/reset.h>
 #include <linux/dma-mapping.h>
@@ -42,6 +43,7 @@ enum ftgmac100_mac_id {
 };
 
 #define FTGMAC100_QUIRK_RMII_ENABLE	BIT(0)
+#define FTGMAC100_QUIRK_DMA_35BIT	BIT(1)
 
 struct ftgmac100_match_data {
 	enum ftgmac100_mac_id mac_id;
@@ -303,10 +305,16 @@ static void ftgmac100_init_hw(struct ftgmac100 *priv)
 	iowrite32(reg, priv->base + FTGMAC100_OFFSET_ISR);
 
 	/* Setup RX ring buffer base */
-	iowrite32(priv->rxdes_dma, priv->base + FTGMAC100_OFFSET_RXR_BADR);
+	iowrite32(lower_32_bits(priv->rxdes_dma), priv->base + FTGMAC100_OFFSET_RXR_BADR);
+	if (priv->quirks & FTGMAC100_QUIRK_DMA_35BIT)
+		iowrite32(upper_32_bits(priv->rxdes_dma),
+			  priv->base + FTGMAC100_OFFSET_RXR_BADDR_HIGH);
 
 	/* Setup TX ring buffer base */
-	iowrite32(priv->txdes_dma, priv->base + FTGMAC100_OFFSET_NPTXR_BADR);
+	iowrite32(lower_32_bits(priv->txdes_dma), priv->base + FTGMAC100_OFFSET_NPTXR_BADR);
+	if (priv->quirks & FTGMAC100_QUIRK_DMA_35BIT)
+		iowrite32(upper_32_bits(priv->txdes_dma),
+			  priv->base + FTGMAC100_OFFSET_TXR_BADDR_HIGH);
 
 	/* Configure RX buffer size */
 	iowrite32(FTGMAC100_RBSR_SIZE(RX_BUF_SIZE),
@@ -469,7 +477,10 @@ static int ftgmac100_alloc_rx_buf(struct ftgmac100 *priv, unsigned int entry,
 	priv->rx_skbs[entry] = skb;
 
 	/* Store DMA address into RX desc */
-	rxdes->rxdes3 = cpu_to_le32(map);
+	if (priv->quirks & FTGMAC100_QUIRK_DMA_35BIT)
+		rxdes->rxdes2 = cpu_to_le32(FIELD_PREP(FTGMAC100_RXDES2_RXBUF_BADR_HI,
+						       upper_32_bits(map)));
+	rxdes->rxdes3 = cpu_to_le32(lower_32_bits(map));
 
 	/* Ensure the above is ordered vs clearing the OWN bit */
 	dma_wmb();
@@ -596,6 +607,9 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 
 	/* Tear down DMA mapping, do necessary cache management */
 	map = le32_to_cpu(rxdes->rxdes3);
+	if (priv->quirks & FTGMAC100_QUIRK_DMA_35BIT)
+		map |= (u64)FIELD_GET(FTGMAC100_RXDES2_RXBUF_BADR_HI,
+				     le32_to_cpu(rxdes->rxdes2)) << 32;
 
 #if defined(CONFIG_ARM) && !defined(CONFIG_ARM_DMA_USE_IOMMU)
 	/* When we don't have an iommu, we can save cycles by not
@@ -672,9 +686,14 @@ static void ftgmac100_free_tx_packet(struct ftgmac100 *priv,
 				     struct ftgmac100_txdes *txdes,
 				     u32 ctl_stat)
 {
-	dma_addr_t map = le32_to_cpu(txdes->txdes3);
+	dma_addr_t map;
 	size_t len;
 
+	map = le32_to_cpu(txdes->txdes3);
+	if (priv->quirks & FTGMAC100_QUIRK_DMA_35BIT)
+		map |= (u64)FIELD_GET(FTGMAC100_TXDES2_TXBUF_BADR_HI,
+				     le32_to_cpu(txdes->txdes2)) << 32;
+
 	if (ctl_stat & FTGMAC100_TXDES0_FTS) {
 		len = skb_headlen(skb);
 		dma_unmap_single(priv->dev, map, len, DMA_TO_DEVICE);
@@ -828,7 +847,10 @@ static netdev_tx_t ftgmac100_hard_start_xmit(struct sk_buff *skb,
 	f_ctl_stat |= FTGMAC100_TXDES0_FTS;
 	if (nfrags == 0)
 		f_ctl_stat |= FTGMAC100_TXDES0_LTS;
-	txdes->txdes3 = cpu_to_le32(map);
+	if (priv->quirks & FTGMAC100_QUIRK_DMA_35BIT)
+		txdes->txdes2 = cpu_to_le32(FIELD_PREP(FTGMAC100_TXDES2_TXBUF_BADR_HI,
+						       upper_32_bits(map)));
+	txdes->txdes3 = cpu_to_le32(lower_32_bits(map));
 	txdes->txdes1 = cpu_to_le32(csum_vlan);
 
 	/* Next descriptor */
@@ -856,7 +878,10 @@ static netdev_tx_t ftgmac100_hard_start_xmit(struct sk_buff *skb,
 			ctl_stat |= FTGMAC100_TXDES0_LTS;
 		txdes->txdes0 = cpu_to_le32(ctl_stat);
 		txdes->txdes1 = 0;
-		txdes->txdes3 = cpu_to_le32(map);
+		if (priv->quirks & FTGMAC100_QUIRK_DMA_35BIT)
+			txdes->txdes2 = cpu_to_le32(FIELD_PREP(FTGMAC100_TXDES2_TXBUF_BADR_HI,
+							       upper_32_bits(map)));
+		txdes->txdes3 = cpu_to_le32(lower_32_bits(map));
 
 		/* Next one */
 		pointer = ftgmac100_next_tx_pointer(priv, pointer);
@@ -931,7 +956,12 @@ static void ftgmac100_free_buffers(struct ftgmac100 *priv)
 	for (i = 0; i < priv->rx_q_entries; i++) {
 		struct ftgmac100_rxdes *rxdes = &priv->rxdes[i];
 		struct sk_buff *skb = priv->rx_skbs[i];
-		dma_addr_t map = le32_to_cpu(rxdes->rxdes3);
+		dma_addr_t map;
+
+		map = le32_to_cpu(rxdes->rxdes3);
+		if (priv->quirks & FTGMAC100_QUIRK_DMA_35BIT)
+			map |= (u64)FIELD_GET(FTGMAC100_RXDES2_RXBUF_BADR_HI,
+					     le32_to_cpu(rxdes->rxdes2)) << 32;
 
 		if (!skb)
 			continue;
@@ -1050,7 +1080,12 @@ static void ftgmac100_init_rings(struct ftgmac100 *priv)
 	for (i = 0; i < priv->rx_q_entries; i++) {
 		rxdes = &priv->rxdes[i];
 		rxdes->rxdes0 = 0;
-		rxdes->rxdes3 = cpu_to_le32(priv->rx_scratch_dma);
+		if (priv->quirks & FTGMAC100_QUIRK_DMA_35BIT) {
+			u32 hi = upper_32_bits(priv->rx_scratch_dma);
+
+			rxdes->rxdes2 = cpu_to_le32(FIELD_PREP(FTGMAC100_RXDES2_RXBUF_BADR_HI, hi));
+		}
+		rxdes->rxdes3 = cpu_to_le32(lower_32_bits(priv->rx_scratch_dma));
 	}
 	/* Mark the end of the ring */
 	rxdes->rxdes0 |= cpu_to_le32(priv->rxdes0_edorr_mask);
@@ -2099,6 +2134,14 @@ static int ftgmac100_probe(struct platform_device *pdev)
 		netdev->hw_features &= ~(NETIF_F_HW_CSUM | NETIF_F_RXCSUM);
 	netdev->features |= netdev->hw_features;
 
+	if (priv->quirks & FTGMAC100_QUIRK_DMA_35BIT) {
+		err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(35));
+		if (err) {
+			dev_err(&pdev->dev, "35-bit DMA enable failed\n");
+			goto err;
+		}
+	}
+
 	/* register network device */
 	err = register_netdev(netdev);
 	if (err) {
@@ -2151,7 +2194,8 @@ static const struct ftgmac100_match_data ftgmac100_match_data_ast2600 = {
 
 static const struct ftgmac100_match_data ftgmac100_match_data_ast2700 = {
 	.mac_id = FTGMAC100_AST2700,
-	.quirks = FTGMAC100_QUIRK_RMII_ENABLE,
+	.quirks = FTGMAC100_QUIRK_RMII_ENABLE |
+		  FTGMAC100_QUIRK_DMA_35BIT,
 };
 
 static const struct ftgmac100_match_data ftgmac100_match_data_faraday = {
diff --git a/drivers/net/ethernet/faraday/ftgmac100.h b/drivers/net/ethernet/faraday/ftgmac100.h
index 8c35c895b146..0c244b1c0a35 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.h
+++ b/drivers/net/ethernet/faraday/ftgmac100.h
@@ -57,6 +57,9 @@
 #define FTGMAC100_OFFSET_RX_RUNT	0xc0
 #define FTGMAC100_OFFSET_RX_CRCER_FTL	0xc4
 #define FTGMAC100_OFFSET_RX_COL_LOST	0xc8
+/* reserved 0xcc - 0x178 */
+#define FTGMAC100_OFFSET_TXR_BADDR_HIGH	0x17c
+#define FTGMAC100_OFFSET_RXR_BADDR_HIGH	0x18c
 
 /*
  * Interrupt status register & interrupt enable register
@@ -226,6 +229,7 @@ struct ftgmac100_txdes {
 #define FTGMAC100_TXDES1_TX2FIC		(1 << 30)
 #define FTGMAC100_TXDES1_TXIC		(1 << 31)
 
+#define FTGMAC100_TXDES2_TXBUF_BADR_HI	GENMASK(18, 16)
 /*
  * Receive descriptor, aligned to 16 bytes
  */
@@ -272,4 +276,6 @@ struct ftgmac100_rxdes {
 #define FTGMAC100_RXDES1_UDP_CHKSUM_ERR	(1 << 26)
 #define FTGMAC100_RXDES1_IP_CHKSUM_ERR	(1 << 27)
 
+#define FTGMAC100_RXDES2_RXBUF_BADR_HI	GENMASK(18, 16)
+
 #endif /* __FTGMAC100_H */

-- 
2.34.1

[PATCH net-next v2 6/6] net: ftgmac100: Allow building on ARM64

[Jacky Chou]

Allow the Faraday vendor menu and FTGMAC100 driver to be selected on
ARM64 so AST2700 platforms can enable the MAC. The driver now programs
the upper DMA address fields, so it no longer needs to be marked BROKEN
for 64-bit builds.

AST2700 has no dedicated MACH_ASPEED_G7 symbol; unlike the 32-bit
generations it is only gated by the common ARCH_ASPEED symbol. On
32-bit Aspeed platforms, arch/arm/mach-aspeed/Kconfig nests
MACH_ASPEED_G4/G5/G6 under "if ARCH_ASPEED", so MACH_ASPEED_G6 already
implies ARCH_ASPEED and is redundant once ARCH_ASPEED is part of the
condition. Drop it and select MDIO_ASPEED directly on ARCH_ASPEED.

This does widen the MDIO_ASPEED select beyond AST2600: it is now also
selected for AST2400/AST2500, which keep driving their embedded MDIO
controller from FTGMAC100 itself and have no MDIO node for this driver
to bind to, so it is built but stays unused there. Update the
MDIO_ASPEED help text to describe this.

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 drivers/net/ethernet/faraday/Kconfig |  7 +++----
 drivers/net/mdio/Kconfig             | 16 ++++++++++------
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/faraday/Kconfig b/drivers/net/ethernet/faraday/Kconfig
index 474073c7f94d..9acc2d260c1f 100644
--- a/drivers/net/ethernet/faraday/Kconfig
+++ b/drivers/net/ethernet/faraday/Kconfig
@@ -6,7 +6,7 @@
 config NET_VENDOR_FARADAY
 	bool "Faraday devices"
 	default y
-	depends on ARM || COMPILE_TEST
+	depends on ARM || ARM64 || COMPILE_TEST
 	help
 	  If you have a network (Ethernet) card belonging to this class, say Y.
 
@@ -28,11 +28,10 @@ config FTMAC100
 
 config FTGMAC100
 	tristate "Faraday FTGMAC100 Gigabit Ethernet support"
-	depends on ARM || COMPILE_TEST
-	depends on !64BIT || BROKEN
+	depends on ARM || ARM64 || COMPILE_TEST
 	select PHYLIB
 	select FIXED_PHY
-	select MDIO_ASPEED if MACH_ASPEED_G6
+	select MDIO_ASPEED if ARCH_ASPEED
 	select CRC32
 	help
 	  This driver supports the FTGMAC100 Gigabit Ethernet controller
diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
index d44278f26fab..d784782c230f 100644
--- a/drivers/net/mdio/Kconfig
+++ b/drivers/net/mdio/Kconfig
@@ -50,12 +50,16 @@ config MDIO_ASPEED
 	depends on OF_MDIO && HAS_IOMEM
 	help
 	  This module provides a driver for the independent MDIO bus
-	  controllers found in the ASPEED AST2600 SoC. This is a driver for the
-	  third revision of the ASPEED MDIO register interface - the first two
-	  revisions are the "old" and "new" interfaces found in the AST2400 and
-	  AST2500, embedded in the MAC. For legacy reasons, FTGMAC100 driver
-	  continues to drive the embedded MDIO controller for the AST2400 and
-	  AST2500 SoCs, so say N if AST2600 support is not required.
+	  controllers found in the ASPEED AST2600 and AST2700 SoCs. This is a
+	  driver for the third revision of the ASPEED MDIO register interface -
+	  the first two revisions are the "old" and "new" interfaces found in
+	  the AST2400 and AST2500, embedded in the MAC. For legacy reasons,
+	  FTGMAC100 driver continues to drive the embedded MDIO controller for
+	  the AST2400 and AST2500 SoCs. AST2700 has no dedicated MACH_ASPEED_G7
+	  symbol and is only gated by ARCH_ASPEED, so FTGMAC100 now
+	  unconditionally selects this driver on every ARCH_ASPEED platform,
+	  including the older AST2400/AST2500 SoCs where it has no MDIO node to
+	  bind to.
 
 config MDIO_BITBANG
 	tristate "Bitbanged MDIO buses"

-- 
2.34.1

Re: [PATCH net-next v2 4/6] net: ftgmac100: Require phy-mode for AST2700

[Andrew Lunn]

On Tue, Oct 06, 2026 at 03:20:42PM +0800, Jacky Chou wrote:
> The driver currently defaults to RGMII when the "phy-mode" property
> is missing from the device tree. This default is no longer valid for
> AST2700, which also supports RMII via FTGMAC100_QUIRK_RMII_ENABLE.
> 
> Make "phy-mode" a required property for AST2700 and fail probe with
> an error message if it is not specified, instead of silently
> defaulting to RGMII.

You need to update the DT binding, mark it as required for ast2700.

    Andrew

---
pw-bot: cr

Re: [PATCH net-next v2 2/6] net: ftgmac100: Add AST2700 compatible support

[Andrew Lunn]

On Tue, Oct 06, 2026 at 03:20:40PM +0800, Jacky Chou wrote:
> Add match data for the Aspeed AST2700 FTGMAC100 controller and wire
> its compatible string into the OF match table. This lets AST2700 device
> tree nodes bind to the ftgmac100 driver.
> 
> Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>

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

    Andrew

Re: [PATCH net-next v2 3/6] net: ftgmac100: Enable AST2700 RMII support

[Andrew Lunn]

>  static void ftgmac100_start_hw(struct ftgmac100 *priv)
>  {
>  	u32 maccr = ioread32(priv->base + FTGMAC100_OFFSET_MACCR);
> +	struct phy_device *phydev = priv->netdev->phydev;

Is it safe to assume there is a phydev? The AI is better at this sort
of logic, but looking at ftgmac100_probe_dt() is think it is possible
to not have a phydev. And ftgmac100_phy_disconnect() does guard
against not having one.

	Andrew

回覆: [PATCH net-next v2 4/6] net: ftgmac100: Require phy-mode for AST2700

[Jacky Chou]

Hi Andrew,

Thank you for your review.

> > The driver currently defaults to RGMII when the "phy-mode" property is
> > missing from the device tree. This default is no longer valid for
> > AST2700, which also supports RMII via FTGMAC100_QUIRK_RMII_ENABLE.
> >
> > Make "phy-mode" a required property for AST2700 and fail probe with an
> > error message if it is not specified, instead of silently defaulting
> > to RGMII.
> 
> You need to update the DT binding, mark it as required for ast2700.
> 

I will update the dt-binding patch in next version.

Thanks,
Jacky

回覆: [PATCH net-next v2 3/6] net: ftgmac100: Enable AST2700 RMII support

[Jacky Chou]

Hi Andrew,

Thank you for your review.

> >  static void ftgmac100_start_hw(struct ftgmac100 *priv)  {
> >  	u32 maccr = ioread32(priv->base + FTGMAC100_OFFSET_MACCR);
> > +	struct phy_device *phydev = priv->netdev->phydev;
> 
> Is it safe to assume there is a phydev? The AI is better at this sort of logic, but
> looking at ftgmac100_probe_dt() is think it is possible to not have a phydev.
> And ftgmac100_phy_disconnect() does guard against not having one.
> 

I think that needs to adjust the ftgmac100_probe_dt() at the return of the end.
```
static int ftgmac100_probe_dt()
{
...
	if (of_get_property(np, "use-ncsi", NULL))
		return ftgmac100_probe_ncsi(netdev, priv, pdev);

	if (of_phy_is_fixed_link(np) ||
	  	....
		return 0;
	}

	if (!ftgmac100_has_child_node(np, "mdio")) {
		err = ftgmac100_mii_probe(netdev);
		if (err) {
			dev_err(priv->dev, "MII probe failed!\n");
			return err;
		}
		Return 0;                       <-- add new
	}

	return -ENODEV;                     <-- change to -ENODEV
}
```
The applications of device tree in ftgmac100 have 'use-ncsi', 'fixed-link'/'phy-handle'
and legacy mdio probing for AST2400/2500.

The 'use-ncsi' will bind a fixed-link phy device on speed 100 and RMII, so it will include
phydev for netdev.
The 'fixed-link' also bind a fixed-link phy device for phydev in netdev.
The 'phy-handle' will return the actual phy device instance for phydev.
The legacy mdio method also returns phydev if the MAC node includes mdio in dts
and find the phy device by mdc/mdio.

Therefore, in the current code, the devices tree does not include 'phy-handle', 'use-ncsi', 
'fixed-link and 'mdio' properties, the ftgmac100_probe_dt() still returns 0 as success at the 
end, and the phydev in netdev will be NULL.

All applications in ftgmac100 must get the phy device handle, regardless of the actual
phy device or the virtual fixed-link phy device.

I would like to add a patch to adjust the ftgmac100_probe_dt(), once the
device tree lacks one of them will return the corresponding error or no device error to 
make probing failed.

Thanks,
Jacky

Re: 回覆: [PATCH net-next v2 3/6] net: ftgmac100: Enable AST2700 RMII support

[Andrew Lunn]

> The applications of device tree in ftgmac100 have 'use-ncsi', 'fixed-link'/'phy-handle'
> and legacy mdio probing for AST2400/2500.
> 
> The 'use-ncsi' will bind a fixed-link phy device on speed 100 and RMII, so it will include
> phydev for netdev.
> The 'fixed-link' also bind a fixed-link phy device for phydev in netdev.
> The 'phy-handle' will return the actual phy device instance for phydev.
> The legacy mdio method also returns phydev if the MAC node includes mdio in dts
> and find the phy device by mdc/mdio.
> 
> Therefore, in the current code, the devices tree does not include 'phy-handle', 'use-ncsi', 
> 'fixed-link and 'mdio' properties, the ftgmac100_probe_dt() still returns 0 as success at the 
> end, and the phydev in netdev will be NULL.
> 
> All applications in ftgmac100 must get the phy device handle, regardless of the actual
> phy device or the virtual fixed-link phy device.
> 
> I would like to add a patch to adjust the ftgmac100_probe_dt(), once the
> device tree lacks one of them will return the corresponding error or no device error to 
> make probing failed.

You cannot cause regressions with existing device, e.g.

aspeed-ast2500-evb.dts

&mac0 {
        status = "okay";

        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_rgmii1_default &pinctrl_mdio1_default>;
};

No phy-handle, no use-ncsi.

So enforcing these must be limited to 2700.

It looks like you can test that some sort of PHY has been found. But
then please remove all tests which check that phydev is not NULL. And
include a good commit message why this is safe and will not cause
regressions.

	Andrew

回覆: 回覆: [PATCH net-next v2 3/6] net: ftgmac100: Enable AST2700 RMII support

[Jacky Chou]

> > The applications of device tree in ftgmac100 have 'use-ncsi',
> 'fixed-link'/'phy-handle'
> > and legacy mdio probing for AST2400/2500.
> >
> > The 'use-ncsi' will bind a fixed-link phy device on speed 100 and
> > RMII, so it will include phydev for netdev.
> > The 'fixed-link' also bind a fixed-link phy device for phydev in netdev.
> > The 'phy-handle' will return the actual phy device instance for phydev.
> > The legacy mdio method also returns phydev if the MAC node includes
> > mdio in dts and find the phy device by mdc/mdio.
> >
> > Therefore, in the current code, the devices tree does not include
> > 'phy-handle', 'use-ncsi', 'fixed-link and 'mdio' properties, the
> > ftgmac100_probe_dt() still returns 0 as success at the end, and the phydev in
> netdev will be NULL.
> >
> > All applications in ftgmac100 must get the phy device handle,
> > regardless of the actual phy device or the virtual fixed-link phy device.
> >
> > I would like to add a patch to adjust the ftgmac100_probe_dt(), once
> > the device tree lacks one of them will return the corresponding error
> > or no device error to make probing failed.
> 
> You cannot cause regressions with existing device, e.g.
> 
> aspeed-ast2500-evb.dts
> 
> &mac0 {
>         status = "okay";
> 
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_rgmii1_default &pinctrl_mdio1_default>; };
> 
> No phy-handle, no use-ncsi.
> 
> So enforcing these must be limited to 2700.
> 
> It looks like you can test that some sort of PHY has been found. But then please
> remove all tests which check that phydev is not NULL. And include a good
> commit message why this is safe and will not cause regressions.
> 

Thank you for bringing this device to my attention.
I will verify all applications running on the AST2700 and update the commit 
message to explain why this change is safe.

Thanks,
Jacky

Re: [PATCH net-next v2 1/6] dt-bindings: net: ftgmac100: Add AST2700 compatible

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] Already fixed later in this series, so the baseline decision is
  false.

--

dt-bindings: net: ftgmac100: Add AST2700 compatible

This adds aspeed,ast2700-mac to the FTGMAC100 compatible list, which
requires the faraday,ftgmac100 fallback. It also updates the RMII RCLK
clock description to include AST2700, and allows the resets property for
AST2700 in the same way as for AST2600.

> diff --git a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> index d14410018bcf6..bee9570a6f63f 100644
> --- a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> +++ b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> @@ -18,6 +18,7 @@ properties:
>                - aspeed,ast2400-mac
>                - aspeed,ast2500-mac
>                - aspeed,ast2600-mac
> +              - aspeed,ast2700-mac
>            - const: faraday,ftgmac100

[Severity: Low]
With this patch, "aspeed,ast2700-mac", "faraday,ftgmac100" becomes a
valid compatible pair. However, at this commit, ftgmac100_of_match[] in
drivers/net/ethernet/faraday/ftgmac100.c has no aspeed,ast2700-mac entry.
Such a node could only bind through the generic faraday,ftgmac100 entry
(FTGMAC100_FARADAY).

On that path, ftgmac100_probe() would do the following:

  - program the ring-end bits at BIT(15) instead of the Aspeed BIT(30)
  - leave is_aspeed false, so ftgmac100_setup_clk() never claims
    MACCLK/RCLK
  - register the embedded MDIO bus through ftgmac100_setup_mdio()
  - never set a DMA mask wider than 32 bits

Would an AST2700 node work correctly if it fell back to the Faraday
match?

Later patches in this series fix this. "net: ftgmac100: Add AST2700
compatible support" adds FTGMAC100_AST2700 and an aspeed,ast2700-mac
match entry, ahead of the faraday fallback. That entry selects the
BIT(30) and is_aspeed path.

"net: ftgmac100: Add AST2700 upper DMA address support" then adds
dma_set_mask_and_coherent(..., DMA_BIT_MASK(35)).

So the problem only shows up when bisecting to this commit, and only with
an out-of-tree AST2700 DTS on an ARM64 build. ARM64 builds are only
enabled by "net: ftgmac100: Allow building on ARM64". Landing the binding
before the driver match is the usual order for dt-bindings series.

[ ... ]

^ annotation: none needed for the [ ... ] above; snipped hunks are the RCLK description and the resets allOf block.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006-support_ast2700-v2-0-36de51fb8066%40aspeedtech.com

Re: [PATCH net-next v2 2/6] net: ftgmac100: Add AST2700 compatible support

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] Dropped: a later patch in the series fixes this.
- [Low] Comments in the driver are now out of date.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but it is a high-
  severity use-after-free that newly matched AST2700 NCSI ports also hit.
- [Medium] This problem wasn't introduced by this patch, but it now also
  affects AST2700.

--

net: ftgmac100: Add AST2700 compatible support

This commit adds match data for the Aspeed AST2700 FTGMAC100 controller
and adds "aspeed,ast2700-mac" to the OF match table. ftgmac100_probe()
now treats AST2700 as an Aspeed MAC. It gets the BIT(30) EDORR/EDOTR
masks and is_aspeed set.

> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 6d2fe5c2f3903..67b1fa464a421 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c

[ ... ]

> @@ -2017,7 +2018,8 @@ static int ftgmac100_probe(struct platform_device *pdev)
>  
>  	if (priv->mac_id == FTGMAC100_AST2400 ||
>  	    priv->mac_id == FTGMAC100_AST2500 ||
> -	    priv->mac_id == FTGMAC100_AST2600) {
> +	    priv->mac_id == FTGMAC100_AST2600 ||
> +	    priv->mac_id == FTGMAC100_AST2700) {
>  		priv->rxdes0_edorr_mask = BIT(30);
>  		priv->txdes0_edotr_mask = BIT(30);
>  		priv->is_aspeed = true;

[Severity: Medium]
This isn't a bug introduced by this patch, but setting is_aspeed for
AST2700 also exposes it to an existing NULL phydev dereference.

Suppose the MAC node has an "mdio" child but no use-ncsi, fixed-link or
phy-handle. Then ftgmac100_probe_dt() returns 0 without connecting a PHY:

drivers/net/ethernet/faraday/ftgmac100.c:ftgmac100_probe_dt() {
    ...
	if (!ftgmac100_has_child_node(np, "mdio")) {
		...
		err = ftgmac100_mii_probe(netdev);
		...
	}

	return 0;
}

register_netdev() then succeeds with netdev->phydev == NULL. On ifup,
ftgmac100_open()->ftgmac100_reset_and_config_mac() does:

	if (priv->is_aspeed && priv->netdev->phydev->interface == PHY_INTERFACE_MODE_RMII) {

Can this dereference a NULL phydev here?

The binding allows an mdio child for every compatible. It does not
require phy-handle, fixed-link or use-ncsi, so a node like this would
pass dt_binding_check.

The unguarded dereference came from commit af350ee72e9dda and affects all
Aspeed variants.

[Severity: Low]
This isn't a bug, but some comments are now out of date because AST2700
is handled as an Aspeed MAC.

struct ftgmac100 still says:

	/* AST2500/AST2600 RMII ref clock gate */
	struct clk *rclk;

However, ftgmac100_setup_clk() now gets the optional RCLK for AST2700 as
well. The binding already describes it as "RMII RCLK gate for
AST2500/2600/2700".

In the same way, ftgmac100_setup_mdio() ("The AST2600 has a separate MDIO
controller") and ftgmac100_probe_dt() ("2600 has an independent MDIO
controller") only mention the AST2600. AST2700 also skips the embedded
MDIO setup. The rest of the series doesn't update these comments either.

Should these comments mention AST2700 too?

> @@ -2131,6 +2133,10 @@ static const struct ftgmac100_match_data ftgmac100_match_data_ast2600 = {
>  	.mac_id = FTGMAC100_AST2600
>  };
>  
> +static const struct ftgmac100_match_data ftgmac100_match_data_ast2700 = {
> +	.mac_id = FTGMAC100_AST2700
> +};
> +

[Severity: Low]
At this commit, can the driver do DMA on AST2700 at all?

AST2700 DRAM starts at 0x4_0000_0000. The ast2700-evb.dts file has
memory@400000000 with reg = <0x4 0x00000000 0x0 0x40000000>. The match
data here only carries mac_id, and the driver at this point:

  - sets no wider DMA mask
  - writes the ring bases with iowrite32() only in ftgmac100_init_hw()
  - keeps only the low 32 bits in rxdes3/txdes3 in
    ftgmac100_alloc_rx_buf() and ftgmac100_hard_start_xmit()

If the driver were built for arm64 here, dma_alloc_coherent() in
ftgmac100_alloc_rings() would fail under the default 32-bit mask, and
ifup would return an error.

Normal configs can't reach this at this commit, because Kconfig still
has:

	depends on ARM || COMPILE_TEST
	depends on !64BIT || BROKEN

Later patches in the series appear to fix this:

  - "net: ftgmac100: Add AST2700 upper DMA address support" adds
    FTGMAC100_QUIRK_DMA_35BIT to ftgmac100_match_data_ast2700, calls
    dma_set_mask_and_coherent() with DMA_BIT_MASK(35), and programs the
    upper address bits in the ring base registers and descriptors.
  - "net: ftgmac100: Allow building on ARM64" then enables the build.

> @@ -2142,6 +2148,8 @@ static const struct of_device_id ftgmac100_of_match[] = {
>  	  .data = &ftgmac100_match_data_ast2500 },
>  	{ .compatible = "aspeed,ast2600-mac",
>  	  .data = &ftgmac100_match_data_ast2600 },
> +	{ .compatible = "aspeed,ast2700-mac",
> +	  .data = &ftgmac100_match_data_ast2700 },
>  	{ .compatible = "faraday,ftgmac100",
>  	  .data = &ftgmac100_match_data_faraday },
>  	{ }

[Severity: High]
This is a pre-existing issue, but AST2700 NCSI ports matched by this new
compatible will hit it too. ftgmac100_remove() frees the NCSI device
before it unregisters the netdev:

drivers/net/ethernet/faraday/ftgmac100.c:ftgmac100_remove() {
    ...
	if (priv->ndev)
		ncsi_unregister_dev(priv->ndev);
	unregister_netdev(netdev);
    ...
}

ncsi_unregister_dev() ends with kfree(ndp), and that allocation contains
the struct ncsi_dev. Neither priv->ndev nor priv->use_ncsi is cleared.

If the interface is still up when the driver is removed (rmmod or a
sysfs unbind), unregister_netdev() closes it:

unregister_netdev()
  ...
  ftgmac100_stop()
    ncsi_stop_dev(priv->ndev)      <- priv->ndev already freed
      NCSI_FOR_EACH_PACKAGE(ndp, np) ...
      ncsi_report_link(ndp, true)
        nd->state = ncsi_dev_state_functional;
        nd->link_up = 0;
        nd->handler(nd);

Does this write to freed memory? It also calls nd->handler through a
pointer read from that freed memory.

This ordering dates from commit 3d5179458d22. Would calling
unregister_netdev() before ncsi_unregister_dev() in ftgmac100_remove()
avoid this?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006-support_ast2700-v2-0-36de51fb8066%40aspeedtech.com

Re: [PATCH net-next v2 3/6] net: ftgmac100: Enable AST2700 RMII support

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 (1) · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] net: ftgmac100: NULL pointer dereference in ftgmac100_start_hw()
  on missing PHY

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch
  copies it.

--

net: ftgmac100: Enable AST2700 RMII support

This commit adds a FTGMAC100_QUIRK_RMII_ENABLE match-data quirk for
AST2700. ftgmac100_start_hw() uses the quirk to set the new
FTGMAC100_MACCR_RMII_ENABLE bit when the attached PHY uses RMII.

> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 67b1fa464a421..b835472da3604 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c

[ ... ]

> @@ -355,6 +359,7 @@ static void ftgmac100_init_hw(struct ftgmac100 *priv)
>  static void ftgmac100_start_hw(struct ftgmac100 *priv)
>  {
>  	u32 maccr = ioread32(priv->base + FTGMAC100_OFFSET_MACCR);
> +	struct phy_device *phydev = priv->netdev->phydev;
>  
>  	/* Keep the original GMAC and FAST bits */
>  	maccr &= (FTGMAC100_MACCR_FAST_MODE | FTGMAC100_MACCR_GIGA_MODE);
> @@ -383,6 +388,11 @@ static void ftgmac100_start_hw(struct ftgmac100 *priv)
>  	if (priv->netdev->features & NETIF_F_HW_VLAN_CTAG_RX)
>  		maccr |= FTGMAC100_MACCR_RM_VLAN;
>  
> +	if ((priv->quirks & FTGMAC100_QUIRK_RMII_ENABLE) &&
> +	    phydev->interface == PHY_INTERFACE_MODE_RMII) {

[Severity: High]
Can phydev be NULL here?

ftgmac100_probe_dt() returns 0 without attaching a PHY when the node
has an mdio child but no phy-handle, fixed-link or use-ncsi property.
of_phy_get_and_connect() is skipped in that case. The
ftgmac100_mii_probe() fallback is skipped too, because the mdio child
exists. Probe then registers the netdev with netdev->phydev still NULL.

The rest of the driver treats a missing PHY as a valid state. For
example, ftgmac100_open() and ftgmac100_stop() test netdev->phydev
before using it. This check reads phydev->interface whenever the quirk
is set, and ftgmac100_match_data_ast2700 always sets it. On such a
board, bringing the interface up would oops via ftgmac100_init_all()
-> ftgmac100_start_hw(). The MAC restart in ftgmac100_poll() and
ftgmac100_set_rx_mode() reach the same code.

ftgmac100_reset_and_config_mac() already does the same unchecked
priv->netdev->phydev->interface read on Aspeed parts. It runs before
ftgmac100_start_hw() on open, so that site may oops first today. This
is still a second dereference that depends only on the quirk bit. The
quirk is meant to be reused by later controllers, which may not set
is_aspeed.

The later patch requiring phy-mode on AST2700 only changes
ftgmac100_mii_probe(). This path never reaches that function, so
nothing in the series closes the gap.

Could phydev be checked for NULL here and in
ftgmac100_reset_and_config_mac()? Or should probe fail on AST2700 when
no PHY ends up attached?

[Severity: High]
This is a pre-existing issue, but this new check repeats it. Can phydev be
NULL here?

ftgmac100_reset_and_config_mac() already dereferences phydev the same way,
without a check, for every Aspeed MAC. That code came from commit
af350ee72e9dda:

    if (priv->is_aspeed && priv->netdev->phydev->interface == PHY_INTERFACE_MODE_RMII) {

The faraday,ftgmac100.yaml schema seems to allow a node that has an mdio
child but no phy-handle, fixed-link or use-ncsi. For such a node,
ftgmac100_probe_dt() skips of_phy_get_and_connect(). Because the mdio child
is present, it also skips the ftgmac100_mii_probe() fallback:

    if (!ftgmac100_has_child_node(np, "mdio")) {
        ...
        err = ftgmac100_mii_probe(netdev);
        ...
    }

    return 0;

Probe then registers the netdev with no PHY attached.

Other parts of the driver accept a NULL phydev, for example
ftgmac100_open():

    if (netdev->phydev) {
        /* If we have a PHY, start polling */
        phy_start(netdev->phydev);

ftgmac100_open()->ftgmac100_reset_and_config_mac() dereferences phydev
before that check, though, so the oops happens there on ndo_open.
ftgmac100_reset() takes the same path.

On AST2700, every caller of ftgmac100_start_hw() runs after
ftgmac100_reset_and_config_mac(). That covers ftgmac100_init_all(),
ftgmac100_poll() via need_mac_restart, and ftgmac100_set_rx_mode(). So the
new site does not add a crash on its own.

It does add a second unchecked read that depends only on the quirk bit, and
AST2700 always sets that bit. A future controller that sets the quirk
without is_aspeed would crash here.

Later patches in the series leave both sites unchecked. "net: ftgmac100:
Require phy-mode for AST2700" only changes ftgmac100_mii_probe(), and this
path never reaches that function.

Could phydev be checked for NULL at both sites? Another option would be to
fail probe on Aspeed parts when no PHY gets attached.

> +		maccr |= FTGMAC100_MACCR_RMII_ENABLE;
> +	}
> +
>  	/* Hit the HW */
>  	iowrite32(maccr, priv->base + FTGMAC100_OFFSET_MACCR);
>  }

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006-support_ast2700-v2-0-36de51fb8066%40aspeedtech.com

Re: [PATCH net-next v2 4/6] net: ftgmac100: Require phy-mode for AST2700

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The new AST2700 check in ftgmac100_mii_probe() can never run,
  and the commit message describes behaviour that does not exist.
- [Low] The commit says it makes 'phy-mode' a required property for
  AST2700, but the DT binding in the same series does not say so.

--

net: ftgmac100: Require phy-mode for AST2700

This patch makes ftgmac100_mii_probe() return -EINVAL on AST2700 when
the phy-mode property is missing, instead of falling back to RGMII. The
commit message says the RGMII default is no longer valid because
AST2700 also supports RMII.

> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index b835472da3604..2d029d1262e6f 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -1521,8 +1521,13 @@ static int ftgmac100_mii_probe(struct net_device *netdev)
>  
>  	/* Default to RGMII. It's a gigabit part after all */
>  	err = of_get_phy_mode(np, &phy_intf);
> -	if (err)
> +	if (err) {
> +		if (priv->mac_id == FTGMAC100_AST2700) {
> +			dev_err(priv->dev, "phy-mode is required for this SoC\n");
> +			return -EINVAL;
> +		}

[Severity: Medium]
Can this check ever run on AST2700?

priv->mii_bus is set in only one place, ftgmac100_setup_mdio(). That
function is called from ftgmac100_probe() only for the Faraday, AST2400
and AST2500 variants:

ftgmac100_probe() {
    ...
	if (priv->mac_id == FTGMAC100_FARADAY ||
	    priv->mac_id == FTGMAC100_AST2400 ||
	    priv->mac_id == FTGMAC100_AST2500) {
		err = ftgmac100_setup_mdio(netdev);
    ...
}

So on AST2700 priv->mii_bus stays NULL, and ftgmac100_mii_probe()
returns before it reaches of_get_phy_mode():

ftgmac100_mii_probe() {
    ...
	if (!priv->mii_bus) {
		dev_err(priv->dev, "No MDIO bus available\n");
		return -ENODEV;
	}
    ...
}

That leaves the new dev_err() and -EINVAL return unreachable on AST2700.

The other AST2700 PHY path is phy-handle or fixed-link in
ftgmac100_probe_dt(). It goes through of_phy_get_and_connect(), which
already returns NULL when phy-mode is missing:

of_phy_get_and_connect() {
    ...
	ret = of_get_phy_mode(np, &iface);
	if (ret)
		return NULL;
    ...
}

Probe then fails with "Failed to connect to phy" and never uses an RGMII
default.

Given that, is this part of the commit message accurate for AST2700?

  "The driver currently defaults to RGMII when the "phy-mode" property
  is missing from the device tree."

  "... fail probe with an error message if it is not specified, instead
  of silently defaulting to RGMII."

One AST2700 configuration can still probe without phy-mode: an mdio child
node with no phy-handle. The requirement the commit describes is not
enforced there either.

The end of the series ("net: ftgmac100: Allow building on ARM64") has the
same layout. ftgmac100_setup_mdio() is still skipped for AST2700, and the
mii_bus check still comes before the new AST2700 check.

Would it make sense to move this check somewhere reachable, such as
ftgmac100_probe_dt(), and update the commit message to match? Or should
the patch be dropped?

> +		phy_intf = PHY_INTERFACE_MODE_RGMII;
> +	}

[Severity: Low]
The commit message says:

  "Make "phy-mode" a required property for AST2700"

The binding doesn't seem to say that. The top-level required list in
Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml is:

required:
  - compatible
  - reg
  - interrupts

The allOf if/then block for aspeed,ast2600-mac and aspeed,ast2700-mac only
covers resets. That means dtbs_check still accepts an AST2700 node with
no phy-mode.

The series already adds the AST2700 compatible in "dt-bindings: net:
ftgmac100: Add AST2700 compatible". Could the requirement go in that
patch, as an if/then for aspeed,ast2700-mac with required: [phy-mode]?

It would need to exclude use-ncsi nodes. ftgmac100_probe_ncsi() hardcodes
PHY_INTERFACE_MODE_RMII and never reads phy-mode.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006-support_ast2700-v2-0-36de51fb8066%40aspeedtech.com

Re: [PATCH net-next v2 5/6] net: ftgmac100: Add AST2700 upper DMA address support

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] struct ftgmac100_txdes in
  drivers/net/ethernet/faraday/ftgmac100.h still says `__le32 txdes2; /*…

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but TX
  completion has no consumer-side read barrier to pair with the…

--

net: ftgmac100: Add AST2700 upper DMA address support

This patch programs the high RX and TX descriptor ring base registers on
AST2700. It also stores DMA address bits 34:32 in rxdes2/txdes2 and
rebuilds the full address from both words before unmapping. A 35-bit DMA
mask is requested, and all of this is gated by the new
FTGMAC100_QUIRK_DMA_35BIT match-data quirk.

> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 2d029d1262e6f..af2e272f32f2d 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c

[ ... ]

> @@ -672,9 +686,14 @@ static void ftgmac100_free_tx_packet(struct ftgmac100 *priv,
>  				     struct ftgmac100_txdes *txdes,
>  				     u32 ctl_stat)
>  {
> -	dma_addr_t map = le32_to_cpu(txdes->txdes3);
> +	dma_addr_t map;
>  	size_t len;
>  
> +	map = le32_to_cpu(txdes->txdes3);
> +	if (priv->quirks & FTGMAC100_QUIRK_DMA_35BIT)
> +		map |= (u64)FIELD_GET(FTGMAC100_TXDES2_TXBUF_BADR_HI,
> +				     le32_to_cpu(txdes->txdes2)) << 32;
> +

[Severity: Medium]
This is a pre-existing issue and was not introduced by this patch. The new
txdes2 read goes through the same TX completion handoff as the existing
txdes0/txdes3 reads, and that handoff seems to have no read barrier on the
consumer side.

ftgmac100_hard_start_xmit() fills tx_skbs[] and txdes1/2/3, sets OWN in
txdes0, and then publishes the producer index:

	dma_wmb();
	first->txdes0 = cpu_to_le32(f_ctl_stat);
	...
	smp_wmb();
	...
	priv->tx_pointer = pointer;

On another CPU, ftgmac100_poll()->ftgmac100_tx_complete() runs:

	while (ftgmac100_tx_buf_cleanable(priv) &&
	       ftgmac100_tx_complete_packet(priv))
		;

ftgmac100_tx_buf_cleanable() loads priv->tx_pointer. After that,
ftgmac100_tx_complete_packet() loads txdes0 and tx_skbs[pointer], and
ftgmac100_free_tx_packet() loads txdes2/txdes3. Nothing like smp_rmb()
sits between the tx_pointer load and the descriptor loads. The smp_mb()
after the loop runs too late to help.

On weakly ordered SMP CPUs such as ARM/ARM64, can the txdes0 load be
satisfied before the tx_pointer load? If so, it could return the stale
OWN-clear value left by the previous completion, and the slot would be
reclaimed while the device still owns it.

That could unmap a buffer the device is still reading. It could also
dereference skb->len through a stale NULL tx_skbs[] entry.

ftgmac100_poll() keeps NAPI scheduled while ftgmac100_tx_buf_cleanable()
is true, so completion can run on one CPU while xmit runs on another.
Would an smp_rmb() after the tx_pointer check in the completion path be
needed here?

[ ... ]

> diff --git a/drivers/net/ethernet/faraday/ftgmac100.h b/drivers/net/ethernet/faraday/ftgmac100.h
> index 8c35c895b1465..0c244b1c0a351 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.h
> +++ b/drivers/net/ethernet/faraday/ftgmac100.h

[ ... ]

> @@ -226,6 +229,7 @@ struct ftgmac100_txdes {
>  #define FTGMAC100_TXDES1_TX2FIC		(1 << 30)
>  #define FTGMAC100_TXDES1_TXIC		(1 << 31)
>  
> +#define FTGMAC100_TXDES2_TXBUF_BADR_HI	GENMASK(18, 16)

[Severity: Low]
This isn't a bug, but struct ftgmac100_txdes still describes this word as
reserved:

	__le32	txdes2; /* Reserved */

With this patch, ftgmac100_hard_start_xmit() writes the upper buffer
address bits into txdes2 for the head descriptor and for every fragment
descriptor. ftgmac100_free_tx_packet() reads them back to rebuild the
address passed to dma_unmap_single()/dma_unmap_page().

Could the comment be updated to describe the new field? Otherwise a later
change might zero or reuse txdes2 based on that comment.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006-support_ast2700-v2-0-36de51fb8066%40aspeedtech.com