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

[PATCH net-next v3 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,
  reset support, and requires "phy-mode" when the AST2700 compatible is
  used. It also drops the "faraday,ftgmac100" fallback compatible
  requirement for aspeed,ast2400-mac, aspeed,ast2500-mac, and
  aspeed,ast2600-mac, giving them the same standalone treatment as
  aspeed,ast2700-mac.
- patch 2 adds the AST2700 match data and OF compatible.
- patch 3 programs the AST2700 RMII enable bit and documents why
  netdev->phydev can never be NULL at that point for AST2700.
- 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 v3:
- Require "phy-mode" in the devicetree binding when the AST2700
  compatible is used, matching the driver's probe-time requirement
  added in v2.
- Drop the "faraday,ftgmac100" fallback compatible requirement for
  AST2700; "aspeed,ast2700-mac" now stands on its own in the binding,
  matching how the driver already matches it independently.
- Drop the same "faraday,ftgmac100" fallback requirement for
  aspeed,ast2400-mac, aspeed,ast2500-mac, and aspeed,ast2600-mac as
  well, per review feedback. The same EDORR/EDOTR bit mismatch applies
  to all of them if a node ever bound through the generic
  faraday,ftgmac100 entry alone; there is no evidence this has ever
  happened in practice since ftgmac100_of_match[] has had dedicated
  entries for these SoCs since commit 03a2aba50b2c, but keeping the
  binding correct avoids leaving the same landmine in place. Folded
  into this series rather than sent as a follow-up, to avoid
  self-conflicting with a later series touching the same "compatible"
  block. Note this makes `make dtbs_check` warn against
  arch/arm/boot/dts/aspeed/aspeed-g4.dtsi, aspeed-g5.dtsi, and
  aspeed-g6.dtsi, which still declare the two-string compatible; that
  .dtsi change will be sent separately through the Aspeed/arm-soc tree,
  since dt-bindings changes don't go through that tree.
- Document in the RMII-enable patch why netdev->phydev is always
  non-NULL by the time ftgmac100_start_hw() checks phydev->interface
  for AST2700: ftgmac100_probe_dt() always attaches a phy_device
  before register_netdev() can succeed, via a discrete PHY
  (phy-handle), a fixed-link node, or NC-SI's internal fixed 100Mbit
  PHY, and any failure in that setup aborts probe first.
- Drop Reviewed-by from Krzysztof Kozlowski on the dt-bindings patch
  since its content changed.
- Link to v2: https://lore.kernel.org/r/20261006-support_ast2700-v2-0-36de51fb8066@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 | 26 ++++--
 drivers/net/ethernet/faraday/Kconfig               |  7 +-
 drivers/net/ethernet/faraday/ftgmac100.c           | 98 +++++++++++++++++++---
 drivers/net/ethernet/faraday/ftgmac100.h           |  7 ++
 drivers/net/mdio/Kconfig                           | 16 ++--
 5 files changed, 123 insertions(+), 31 deletions(-)
---
base-commit: 518bf436318e8d1162391cd77d69d65198a4a25b
change-id: 20260623-support_ast2700-68fb164da45c

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

[PATCH net-next v3 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.

Require "phy-mode" to be present in the device tree when the AST2700
compatible is used, matching the driver's probe-time requirement.

Make aspeed,ast2700-mac a standalone compatible instead of pairing it
with the generic "faraday,ftgmac100" fallback: if a node ever bound
through the generic entry alone, ftgmac100_probe() would set
rxdes0_edorr_mask/txdes0_edotr_mask to BIT(15) instead of the Aspeed
BIT(30), skip Aspeed clock setup, and leave the DMA mask too narrow.

The existing aspeed,ast2400-mac, aspeed,ast2500-mac, and
aspeed,ast2600-mac compatibles have the same structural problem: they
are currently only valid when paired with "faraday,ftgmac100", and a
node that bound through the generic entry alone would hit the same
BIT(15) vs BIT(30) mismatch. Give them the same standalone treatment as
AST2700, so none of the Aspeed compatibles require the generic fallback
alongside them.

ftgmac100_of_match[] has had dedicated entries for AST2400, AST2500,
and AST2600 since commit 03a2aba50b2c ("net: ftgmac100: List all
compatibles"), so there is no known kernel + devicetree combination
where a node has actually bound through the generic entry and gotten
the wrong EDORR bit. This part is a binding correctness fix, not a fix
for an observed failure.

Note that arch/arm/boot/dts/aspeed/aspeed-g4.dtsi, aspeed-g5.dtsi, and
aspeed-g6.dtsi still declare their MAC nodes with the two-string
"aspeed,astXXXX-mac", "faraday,ftgmac100" compatible, so this will make
dtbs_check warn against those files until the redundant fallback
compatible is dropped there as well. That .dtsi change will be sent as
a separate series through the Aspeed/arm-soc tree, since dt-bindings
changes don't go through that tree.

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 .../devicetree/bindings/net/faraday,ftgmac100.yaml | 26 ++++++++++++++--------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
index d14410018bcf..0a376b849a91 100644
--- a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
+++ b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
@@ -13,12 +13,11 @@ properties:
   compatible:
     oneOf:
       - const: faraday,ftgmac100
-      - items:
-          - enum:
-              - aspeed,ast2400-mac
-              - aspeed,ast2500-mac
-              - aspeed,ast2600-mac
-          - const: faraday,ftgmac100
+      - enum:
+          - aspeed,ast2400-mac
+          - aspeed,ast2500-mac
+          - aspeed,ast2600-mac
+          - aspeed,ast2700-mac
 
   reg:
     maxItems: 1
@@ -30,7 +29,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,26 +81,35 @@ allOf:
           contains:
             enum:
               - aspeed,ast2600-mac
+              - aspeed,ast2700-mac
     then:
       properties:
         resets: true
     else:
       properties:
         resets: false
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: aspeed,ast2700-mac
+    then:
+      required:
+        - phy-mode
 
 unevaluatedProperties: false
 
 examples:
   - |
     ethernet@1e660000 {
-        compatible = "aspeed,ast2500-mac", "faraday,ftgmac100";
+        compatible = "aspeed,ast2500-mac";
         reg = <0x1e660000 0x180>;
         interrupts = <2>;
         use-ncsi;
     };
 
     ethernet@1e680000 {
-        compatible = "aspeed,ast2500-mac", "faraday,ftgmac100";
+        compatible = "aspeed,ast2500-mac";
         reg = <0x1e680000 0x180>;
         interrupts = <2>;
 

-- 
2.34.1

[PATCH net-next v3 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.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
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 v3 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.

phydev is always populated by the time this runs: ftgmac100_start_hw()
is only reached once the device has been opened, and
ftgmac100_probe_dt() always attaches a phy_device before
register_netdev() can succeed -- via a discrete PHY ("phy-handle"), a
"fixed-link" node, or NC-SI's internal fixed 100Mbit PHY
(ftgmac100_probe_ncsi()). Any failure in that setup aborts probe before
register_netdev(), so an opened AST2700 netdev always has phydev set.
This mirrors the same assumption ftgmac100_reset_and_config_mac()
already makes for every Aspeed SoC earlier in the open path.

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

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 67b1fa464a42..9b7becb01726 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,12 @@ static void ftgmac100_init_hw(struct ftgmac100 *priv)
 static void ftgmac100_start_hw(struct ftgmac100 *priv)
 {
 	u32 maccr = ioread32(priv->base + FTGMAC100_OFFSET_MACCR);
+	/*
+	 * Only reached once the device is open, so a phy_device is already
+	 * attached: a discrete PHY via "phy-handle", a "fixed-link" node, or
+	 * NC-SI's internal fixed 100M PHY.
+	 */
+	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 +393,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 +1957,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 +2002,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 +2150,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 v3 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 9b7becb01726..5a578a62df67 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1526,8 +1526,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 v3 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 5a578a62df67..263594de0e07 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),
@@ -474,7 +482,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();
@@ -601,6 +612,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
@@ -677,9 +691,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);
@@ -833,7 +852,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 */
@@ -861,7 +883,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);
@@ -936,7 +961,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;
@@ -1055,7 +1085,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);
@@ -2104,6 +2139,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) {
@@ -2156,7 +2199,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 v3 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 v3 1/6] dt-bindings: net: ftgmac100: Add AST2700 compatible

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4606 bytes --]

On Thu, Oct 15, 2026 at 02:46:53PM +0800, Jacky Chou wrote:
> 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.
> 
> Require "phy-mode" to be present in the device tree when the AST2700
> compatible is used, matching the driver's probe-time requirement.
> 
> Make aspeed,ast2700-mac a standalone compatible instead of pairing it
> with the generic "faraday,ftgmac100" fallback: if a node ever bound
> through the generic entry alone, ftgmac100_probe() would set
> rxdes0_edorr_mask/txdes0_edotr_mask to BIT(15) instead of the Aspeed
> BIT(30), skip Aspeed clock setup, and leave the DMA mask too narrow.
> 
> The existing aspeed,ast2400-mac, aspeed,ast2500-mac, and
> aspeed,ast2600-mac compatibles have the same structural problem: they
> are currently only valid when paired with "faraday,ftgmac100", and a
> node that bound through the generic entry alone would hit the same
> BIT(15) vs BIT(30) mismatch. Give them the same standalone treatment as
> AST2700, so none of the Aspeed compatibles require the generic fallback
> alongside them.

This needs to be two commits. One fixing the existing devices and one
adding the new one.

> ftgmac100_of_match[] has had dedicated entries for AST2400, AST2500,
> and AST2600 since commit 03a2aba50b2c ("net: ftgmac100: List all
> compatibles"), so there is no known kernel + devicetree combination
> where a node has actually bound through the generic entry and gotten
> the wrong EDORR bit. This part is a binding correctness fix, not a fix
> for an observed failure.

> Note that arch/arm/boot/dts/aspeed/aspeed-g4.dtsi, aspeed-g5.dtsi, and
> aspeed-g6.dtsi still declare their MAC nodes with the two-string
> "aspeed,astXXXX-mac", "faraday,ftgmac100" compatible, so this will make
> dtbs_check warn against those files until the redundant fallback
> compatible is dropped there as well. That .dtsi change will be sent as
> a separate series through the Aspeed/arm-soc tree, since dt-bindings
> changes don't go through that tree.

This should be below the --- line or not even included in the patch.

pw-bot: changes-requested

Thanks,
Conor.

> 
> Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>

> ---
>  .../devicetree/bindings/net/faraday,ftgmac100.yaml | 26 ++++++++++++++--------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> index d14410018bcf..0a376b849a91 100644
> --- a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> +++ b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> @@ -13,12 +13,11 @@ properties:
>    compatible:
>      oneOf:
>        - const: faraday,ftgmac100
> -      - items:
> -          - enum:
> -              - aspeed,ast2400-mac
> -              - aspeed,ast2500-mac
> -              - aspeed,ast2600-mac
> -          - const: faraday,ftgmac100
> +      - enum:
> +          - aspeed,ast2400-mac
> +          - aspeed,ast2500-mac
> +          - aspeed,ast2600-mac
> +          - aspeed,ast2700-mac
>  
>    reg:
>      maxItems: 1
> @@ -30,7 +29,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,26 +81,35 @@ allOf:
>            contains:
>              enum:
>                - aspeed,ast2600-mac
> +              - aspeed,ast2700-mac
>      then:
>        properties:
>          resets: true
>      else:
>        properties:
>          resets: false
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: aspeed,ast2700-mac
> +    then:
> +      required:
> +        - phy-mode
>  
>  unevaluatedProperties: false
>  
>  examples:
>    - |
>      ethernet@1e660000 {
> -        compatible = "aspeed,ast2500-mac", "faraday,ftgmac100";
> +        compatible = "aspeed,ast2500-mac";
>          reg = <0x1e660000 0x180>;
>          interrupts = <2>;
>          use-ncsi;
>      };
>  
>      ethernet@1e680000 {
> -        compatible = "aspeed,ast2500-mac", "faraday,ftgmac100";
> +        compatible = "aspeed,ast2500-mac";
>          reg = <0x1e680000 0x180>;
>          interrupts = <2>;
>  
> 
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Jacky Chou]

Hi Conor,

Thank you for your reply.

> > 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.
> >
> > Require "phy-mode" to be present in the device tree when the AST2700
> > compatible is used, matching the driver's probe-time requirement.
> >
> > Make aspeed,ast2700-mac a standalone compatible instead of pairing it
> > with the generic "faraday,ftgmac100" fallback: if a node ever bound
> > through the generic entry alone, ftgmac100_probe() would set
> > rxdes0_edorr_mask/txdes0_edotr_mask to BIT(15) instead of the Aspeed
> > BIT(30), skip Aspeed clock setup, and leave the DMA mask too narrow.
> >
> > The existing aspeed,ast2400-mac, aspeed,ast2500-mac, and
> > aspeed,ast2600-mac compatibles have the same structural problem: they
> > are currently only valid when paired with "faraday,ftgmac100", and a
> > node that bound through the generic entry alone would hit the same
> > BIT(15) vs BIT(30) mismatch. Give them the same standalone treatment
> > as AST2700, so none of the Aspeed compatibles require the generic
> > fallback alongside them.
> 
> This needs to be two commits. One fixing the existing devices and one adding
> the new one.
> 

Agreed, I will split them to two commits in next version.

> > ftgmac100_of_match[] has had dedicated entries for AST2400, AST2500,
> > and AST2600 since commit 03a2aba50b2c ("net: ftgmac100: List all
> > compatibles"), so there is no known kernel + devicetree combination
> > where a node has actually bound through the generic entry and gotten
> > the wrong EDORR bit. This part is a binding correctness fix, not a fix
> > for an observed failure.
> 
> > Note that arch/arm/boot/dts/aspeed/aspeed-g4.dtsi, aspeed-g5.dtsi, and
> > aspeed-g6.dtsi still declare their MAC nodes with the two-string
> > "aspeed,astXXXX-mac", "faraday,ftgmac100" compatible, so this will
> > make dtbs_check warn against those files until the redundant fallback
> > compatible is dropped there as well. That .dtsi change will be sent as
> > a separate series through the Aspeed/arm-soc tree, since dt-bindings
> > changes don't go through that tree.
> 
> This should be below the --- line or not even included in the patch.
> 

I also update this in next version.

Thanks,
Jacky

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

[Andrew Lunn]

On Thu, Oct 15, 2026 at 02:46:56PM +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.
> 
> Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>

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

    Andrew

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

[Andrew Lunn]

On Thu, Oct 15, 2026 at 02:46:57PM +0800, Jacky Chou wrote:
> 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>

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

    Andrew

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

[Andrew Lunn]

On Thu, Oct 15, 2026 at 02:46:58PM +0800, Jacky Chou wrote:
> 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>

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

    Andrew

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

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 3 · Low: 3

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] In
  Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml…
- [Medium] The rewritten `compatible: oneOf` in faraday,ftgmac100.yaml
  (lines 13-20) no longer accepts the two-string `"aspeed,astXXXX-mac",…
- [Medium] dt-bindings: net: unconditional phy-mode requirement conflicts
  with use-ncsi
- [Low] The commit message says requiring phy-mode 'match[es] the
  driver's probe-time requirement' and talks about AST2700…
- [Low] At this commit, the binding documents a standalone
  `aspeed,ast2700-mac` compatible, but ftgmac100_of_match[] (line 2138)…
- [Low] The new aspeed,ast2700-mac compatible does not require `clocks`
  (top-level `required:` at faraday,ftgmac100.yaml lines 71-74 lists…

--

dt-bindings: net: ftgmac100: Add AST2700 compatible

This adds aspeed,ast2700-mac to the ftgmac100 binding, allows the reset
line for it and requires phy-mode for it. It also changes all of the
Aspeed compatibles from the "aspeed,astXXXX-mac", "faraday,ftgmac100" pair
to standalone strings.

> diff --git a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> index d14410018bcf..0a376b849a91 100644
> --- a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> +++ b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> @@ -13,12 +13,11 @@ properties:
>    compatible:
>      oneOf:
>        - const: faraday,ftgmac100
> -      - items:
> -          - enum:
> -              - aspeed,ast2400-mac
> -              - aspeed,ast2500-mac
> -              - aspeed,ast2600-mac
> -          - const: faraday,ftgmac100
> +      - enum:
> +          - aspeed,ast2400-mac
> +          - aspeed,ast2500-mac
> +          - aspeed,ast2600-mac
> +          - aspeed,ast2700-mac

[Severity: Medium]
Will DTBs written to the new binding still get networking on older
kernels?

Before commit 03a2aba50b2c ("net: ftgmac100: List all compatibles"),
ftgmac100_of_match[] in drivers/net/ethernet/faraday/ftgmac100.c only had:

    { .compatible = "faraday,ftgmac100" },

On those kernels, Aspeed MAC nodes bound only through the fallback string.
A node with only compatible = "aspeed,ast2500-mac", as in the updated
example, matches no driver on such a kernel. That includes stable/LTS
kernels that predate that commit, and any other DT consumer that matches
only the generic string.

The commit message only mentions the dtbs_check warnings and calls this a
binding correctness fix. Could it also mention this forward-compatibility
change? Alternatively, could the two-string form be kept as a deprecated
alternative instead of being removed?

[Severity: Medium]
Without the items branch, both oneOf branches accept only a single
string. Every in-tree Aspeed MAC node still uses the two-string form:

arch/arm/boot/dts/aspeed/aspeed-g4.dtsi (mac0, mac1):
    compatible = "aspeed,ast2400-mac", "faraday,ftgmac100";

arch/arm/boot/dts/aspeed/aspeed-g5.dtsi (mac0, mac1):
    compatible = "aspeed,ast2500-mac", "faraday,ftgmac100";

arch/arm/boot/dts/aspeed/aspeed-g6.dtsi (mac0..mac3):
    compatible = "aspeed,ast2600-mac", "faraday,ftgmac100";

After this patch, these 8 MAC nodes fail dtbs_check on every AST2400,
AST2500 and AST2600 board. Nothing in this series updates the dtsi files,
and DTBs already deployed with this compatible become schema-invalid too.
The two-string form was never marked deprecated before being removed.

Dropping the pair also changes nothing at runtime on current kernels.
of_match_node() scores matches by their position in the node's compatible
list, so the dedicated aspeed,astXXXX-mac entries in ftgmac100_of_match[]
already win over "faraday,ftgmac100".

Could the existing items form stay valid, for example marked deprecated,
next to the new standalone enum?

[Severity: Low]
At this commit, ftgmac100_of_match[] has no aspeed,ast2700-mac entry and
enum ftgmac100_mac_id has no AST2700 value. A schema-valid AST2700 node
would therefore bind to nothing.

Later in the series, "net: ftgmac100: Add AST2700 compatible support"
adds FTGMAC100_AST2700, the match entry and the BIT(30) EDORR selection in
ftgmac100_probe(). So this only affects the intermediate state.

> @@ -30,7 +29,7 @@ properties:
>      minItems: 1
>      items:
>        - description: MAC IP clock
> -      - description: RMII RCLK gate for AST2500/2600
> +      - description: RMII RCLK gate for AST2500/2600/2700

[Severity: Low]
Should clocks also be required for aspeed,ast2700-mac? The top-level
required list has only compatible, reg and interrupts, and no conditional
adds clocks.

With the later driver patches in this series, ftgmac100_probe() calls
ftgmac100_setup_clk() for AST2700, which does:

	clk = devm_clk_get_enabled(priv->dev, NULL /* MACCLK */);
	if (IS_ERR(clk))
		return PTR_ERR(clk);

So an AST2700 node without clocks passes dt_binding_check but then fails
probe.

phy-mode is made required below to match a probe-time requirement. Should
clocks be required for the same reason? AST2400-2600 already have this
gap; this patch extends it to the new compatible.

> @@ -82,26 +81,35 @@ allOf:
>            contains:
>              enum:
>                - aspeed,ast2600-mac
> +              - aspeed,ast2700-mac
>      then:
>        properties:
>          resets: true
>      else:
>        properties:
>          resets: false
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: aspeed,ast2700-mac
> +    then:
> +      required:
> +        - phy-mode

[Severity: Medium]
Should this requirement exclude NC-SI configurations? use-ncsi takes
the place of an MDIO PHY. When it is present, probe goes down the
ftgmac100_probe_ncsi() path and returns before ftgmac100_mii_probe()
is reached. The driver therefore never reads phy-mode for such a node.

With this block, an AST2700 node written like the first example in
this binding fails dt_binding_check / dtbs_check because phy-mode is
missing:

    compatible = "aspeed,ast2700-mac";
    reg = <...>;
    interrupts = <...>;
    use-ncsi;

The only way to make it pass is to add a phy-mode value that nothing
consumes. That value would also not describe the hardware, since
there is no PHY on that MAC.

The commit message says this matches the driver's probe-time
requirement. As far as I can tell, that requirement only applies on
the PHY path, not with use-ncsi.

Could the then: branch be limited to nodes without use-ncsi? For
example, a nested if: with not: required: use-ncsi could carry the
phy-mode requirement. Or is there a reason AST2700 NC-SI boards need
phy-mode as well?

[Severity: Low]
The commit message says:

    Require "phy-mode" to be present in the device tree when the AST2700
    compatible is used, matching the driver's probe-time requirement.

It also describes the AST2700 EDORR, clock and DMA mask handling in
ftgmac100_probe().

At this commit, ftgmac100.c has no AST2700 support and never calls
dma_set_mask*(). ftgmac100_mii_probe() also still treats phy-mode as
optional:

	err = of_get_phy_mode(np, &phy_intf);
	if (err)
		phy_intf = PHY_INTERFACE_MODE_RGMII;

This driver behavior is added later in the series by:

    "net: ftgmac100: Add AST2700 compatible support"
    "net: ftgmac100: Require phy-mode for AST2700"
    "net: ftgmac100: Add AST2700 upper DMA address support"

Putting the binding first is the normal order. Could the commit message
say that it describes the driver once the rest of the series is applied?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261015-support_ast2700-v3-0-8d7999171ac4%40aspeedtech.com

Re: [PATCH net-next v3 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] The comment on struct ftgmac100::rclk still says '/*
  AST2500/AST2600 RMII ref clock gate */' (line 117 at this commit, and…
- [Low] (Baseline: discarded as resolved later in the series.) At this
  commit, AST2700 binds to the driver with no DMA support above 4 GiB.

Pre-existing issues:
- [High] On Aspeed variants, including the newly added AST2700, a NULL
  pointer dereference oopses ftgmac100_reset_and_config_mac() when the…

--

net: ftgmac100: Add AST2700 compatible support

This adds an FTGMAC100_AST2700 mac_id, its match data, and an
"aspeed,ast2700-mac" entry in the OF match table. In ftgmac100_probe(),
AST2700 is handled the same way as the other Aspeed variants: it gets
the BIT(30) EDORR/EDOTR masks and is_aspeed is set.

> 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

[ ... ]

> @@ -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: High]
This is a pre-existing issue, not one introduced by this patch. Setting
is_aspeed for AST2700 does make it reachable with the new compatible,
though. Can ftgmac100_reset_and_config_mac() dereference a NULL phydev
here?

drivers/net/ethernet/faraday/ftgmac100.c:ftgmac100_reset_and_config_mac() {
    ...
	/* Aspeed RMII needs SCU reset to clear status */
	if (priv->is_aspeed && priv->netdev->phydev->interface == PHY_INTERFACE_MODE_RMII) {
    ...
}

Suppose the DT node has an "mdio" child node but no "phy-handle", no
"fixed-link" and no "use-ncsi". In that case ftgmac100_probe_dt() skips
ftgmac100_mii_probe() and returns 0 without attaching a PHY:

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

	return 0;

ftgmac100_probe() also skips ftgmac100_setup_mdio() for AST2700. So
register_netdev() succeeds with netdev->phydev == NULL. The
faraday,ftgmac100.yaml binding allows this DT, because mdio and
phy-handle are both optional.

Bringing the interface up then runs:

ftgmac100_open()
  ftgmac100_reset_and_config_mac()
    priv->netdev->phydev->interface

The reset task reaches the same code through ftgmac100_reset().

Elsewhere, ftgmac100_open() treats phydev as optional:

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

The unchecked dereference came in with commit af350ee72e9dd ("net:
ftgmac100: Add optional reset control for RMII mode on Aspeed SoCs").
It already affects AST2400/2500/2600.

A later patch in this series, "net: ftgmac100: Enable AST2700 RMII
support", adds another unconditional phydev->interface dereference in
ftgmac100_start_hw(). Its commit message and code comment say that
probe_dt always attaches a phy_device before register_netdev(). Doesn't
the path above show that this assumption doesn't always hold?

[Severity: Low]
This isn't a bug, but the comment on the rclk member of struct ftgmac100
still reads:

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

Since is_aspeed is now set for AST2700, ftgmac100_setup_clk() acquires
RCLK for it too. The binding also describes the clock as "RMII RCLK
gate for AST2500/2600/2700". Could the comment be updated to include
AST2700?

[ ... ]

> @@ -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]
Can AST2700 do DMA with only this match data? At this commit no quirks
are set, so dma_set_mask_and_coherent() is never called. In addition:

ftgmac100_init_hw() only programs the 32-bit RXR_BADR/NPTXR_BADR
registers.

ftgmac100_alloc_rx_buf() and the tx path only store the low 32 bits of
the buffer address in rxdes3/txdes3.

The AST2700 EVB puts all of its DRAM above 4 GiB:

	reg = <0x4 0x00000000 0x0 0x40000000>;

This looks to be handled later in the series. "net: ftgmac100: Add
AST2700 upper DMA address support" adds FTGMAC100_QUIRK_DMA_35BIT to
this match data. The same patch adds the RXR/TXR high base register
writes and the upper address bits in rxdes2/txdes2.

The driver also cannot be built for arm64 until "net: ftgmac100: Allow
building on ARM64". The Kconfig has "depends on ARM || COMPILE_TEST"
and "depends on !64BIT || BROKEN", so this entry has no effect at this
point in the series.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261015-support_ast2700-v3-0-8d7999171ac4%40aspeedtech.com

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

[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 commit message and the new comment in ftgmac100_start_hw()
  say a phy_device is always attached once the netdev is open.
- [Low] The struct ftgmac100 field comment above `struct clk *rclk;`
  still reads "AST2500/AST2600 RMII ref clock gate".

--

net: ftgmac100: Enable AST2700 RMII support

This commit adds a match-data quirk, FTGMAC100_QUIRK_RMII_ENABLE, for
AST2700. With the quirk set, ftgmac100_start_hw() sets
FTGMAC100_MACCR_RMII_ENABLE when the attached PHY uses RMII. The commit
message says netdev->phydev is always populated once the device is open.

> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 67b1fa464a42..9b7becb01726 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c

[ ... ]

> @@ -79,6 +82,7 @@ struct ftgmac100 {
>  	void __iomem *base;
>  
>  	enum ftgmac100_mac_id mac_id;
> +	u32 quirks;
>  
>  	/* Rx ring */
>  	unsigned int rx_q_entries;

[Severity: Low]
This isn't a bug, but this patch enables RMII on AST2700. Should the
comment on the rclk field further down in struct ftgmac100 mention
AST2700 too?

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

ftgmac100_setup_clk() requests the optional "RCLK" clock for every
is_aspeed device, and that includes AST2700. The binding also describes
it as "RMII RCLK gate for AST2500/2600/2700".

> @@ -355,6 +359,12 @@ static void ftgmac100_init_hw(struct ftgmac100 *priv)
>  static void ftgmac100_start_hw(struct ftgmac100 *priv)
>  {
>  	u32 maccr = ioread32(priv->base + FTGMAC100_OFFSET_MACCR);
> +	/*
> +	 * Only reached once the device is open, so a phy_device is already
> +	 * attached: a discrete PHY via "phy-handle", a "fixed-link" node, or
> +	 * NC-SI's internal fixed 100M PHY.
> +	 */
> +	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 +393,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;
> +	}

[Severity: Medium]
Is phydev guaranteed to be non-NULL here? The commit message says:

  ftgmac100_probe_dt() always attaches a phy_device before
  register_netdev() can succeed -- via a discrete PHY ("phy-handle"), a
  "fixed-link" node, or NC-SI's internal fixed 100Mbit PHY

Take a node that has an "mdio" child but no "use-ncsi", no "fixed-link"
and no "phy-handle". It reaches the legacy branch in
ftgmac100_probe_dt():

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

	return 0;

Probe then succeeds, and register_netdev() is called with
netdev->phydev == NULL. ftgmac100_setup_mdio() is not called for
AST2700, so the mdio child is simply ignored.

The binding appears to allow this setup. mdio is allowed for every
compatible, and AST2700 only adds phy-mode to the required list. It does
not require phy-handle, fixed-link or use-ncsi.

When such a netdev is opened, the path
ftgmac100_open()->ftgmac100_reset_and_config_mac() would oops first, on
the same dereference the commit message cites as precedent:

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

The new phydev->interface check in ftgmac100_start_hw() has the same
problem.

Other parts of the driver treat a NULL phydev as valid, for example
ftgmac100_open():

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

ftgmac100_stop(), ftgmac100_phy_disconnect(), ftgmac100_set_pauseparam()
and the reset task also check for NULL.

The later patch in this series, "Require phy-mode for AST2700", only
adds a check inside ftgmac100_mii_probe(). That function isn't reached
on the mdio-child path, so both unchecked dereferences are still there
at the end of the series.

Would it make sense to fail probe on Aspeed when no PHY gets attached,
to guard the check with phydev && ..., or to cache the phy-mode at probe
time? At minimum, could the new comment and the commit message be
corrected?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261015-support_ast2700-v3-0-8d7999171ac4%40aspeedtech.com

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

[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 (2)

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] The new AST2700 phy-mode check in ftgmac100_mii_probe() can
  never run, so the patch has no effect.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but it is a
  reachable kernel oops.
- [Low] The AST2700 binding requires phy-mode in every configuration,
  including use-ncsi.
- [Low] In ftgmac100_mii_probe(), the comment "Aspeed only supports
  these" and the netdev_warn("Unsupported PHY mode %s !") fire for any…

--

net: ftgmac100: Require phy-mode for AST2700

This patch changes ftgmac100_mii_probe() so that on AST2700 it fails
with -EINVAL when the phy-mode property is missing, instead of falling
back to RGMII. The goal is to make phy-mode a required property for
AST2700, since that SoC also supports RMII.

> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 9b7becb01726..5a578a62df67 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -1526,8 +1526,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: High]
Can this new check ever be reached on AST2700?

ftgmac100_setup_mdio() is the only function that assigns priv->mii_bus.
ftgmac100_probe() calls it only for FARADAY, AST2400 and AST2500:

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

On AST2700, priv->mii_bus is therefore always NULL. ftgmac100_mii_probe()
returns at this check before it ever calls of_get_phy_mode():

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

That leaves the "phy-mode is required for this SoC" message unreachable.

The commit message says:

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

Is that true for AST2700? These are the probe paths in
ftgmac100_probe_dt():

- phy-handle or fixed-link: of_phy_get_and_connect() already returns
  NULL when of_get_phy_mode() fails, so probe fails with "Failed to
  connect to phy".
- use-ncsi: phy-mode is ignored and RMII is hardcoded.
- no mdio child: ftgmac100_mii_probe() fails with "No MDIO bus
  available".
- an mdio child with no phy-handle, fixed-link or use-ncsi:
  ftgmac100_probe_dt() returns 0 and phy-mode is never checked.

In that last case AST2700 still probes successfully without phy-mode.
That does not match "fail probe with an error message if it is not
specified".

The dt-binding patch in this series, "dt-bindings: net: ftgmac100: Add
AST2700 compatible", justifies required: phy-mode by saying it matches
the driver's probe-time requirement. This patch does not create that
requirement, and no later patch in the series changes
ftgmac100_mii_probe() or ftgmac100_probe_dt().

Would it work better to put the check in ftgmac100_probe_dt() (or
ftgmac100_probe())? It would need to run before the
fixed-link/phy-handle/mdio-child dispatch and outside the use-ncsi path.

>  		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

[Severity: Low]
This isn't a bug introduced by this patch, but the comment and the
warning that follow it in ftgmac100_mii_probe() look out of date:

	if (priv->is_aspeed && !(phy_interface_mode_is_rgmii(phy_intf))) {
		netdev_warn(netdev,
			    "Unsupported PHY mode %s !\n",
			    phy_modes(phy_intf));
	}

The driver handles Aspeed RMII in two places: the SCU reset in
ftgmac100_reset_and_config_mac(), and FTGMAC100_QUIRK_RMII_ENABLE in
ftgmac100_start_hw(). The binding's phy-mode enum also allows rmii.

Do legacy AST2400/AST2500 RMII device trees with no mdio child get a
spurious "Unsupported PHY mode rmii !" warning here? phy_connect() still
gets the RMII mode, so the warning has no functional effect. This path
is not reachable on AST2700.

[Severity: High]
This isn't a bug introduced by this patch, but the mdio child path in
ftgmac100_probe_dt() can also lead to a NULL pointer dereference.

If an Aspeed DT node has an mdio child but no phy-handle, fixed-link or
use-ncsi, ftgmac100_probe_dt() returns 0 without attaching a PHY:

ftgmac100_probe_dt() {
    ...
	if (!ftgmac100_has_child_node(np, "mdio")) {
		...
	}

	return 0;
}

The netdev is then registered with netdev->phydev == NULL. On interface
up:

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

Can this oops in ndo_open while rtnl_lock is held? ftgmac100_reset_task()
reaches the same dereference. ftgmac100_open() checks
if (netdev->phydev) later on, so a NULL phydev looks like an expected
state.

The series patch "net: ftgmac100: Enable AST2700 RMII support" adds a
second unguarded dereference in ftgmac100_start_hw():

	if ((priv->quirks & FTGMAC100_QUIRK_RMII_ENABLE) &&
	    phydev->interface == PHY_INTERFACE_MODE_RMII) {

That patch justifies the dereference with "ftgmac100_probe_dt() always
attaches a phy_device before register_netdev() can succeed". The mdio
child path above contradicts that.

In practice the crash would happen first in
ftgmac100_reset_and_config_mac(), because it runs before
ftgmac100_init_all()->ftgmac100_start_hw(). The phy-mode check in this
patch does not cover this path either.

[Severity: Low]
This is a pre-existing issue, but it bears on the binding requirement
this series adds. The AST2700 binding requires phy-mode in every
configuration, including use-ncsi, yet ftgmac100_probe_ncsi() never
reads phy-mode:

ftgmac100_probe_ncsi() {
    ...
	err = phy_connect_direct(netdev, phydev, ftgmac100_adjust_link,
				 PHY_INTERFACE_MODE_RMII);
    ...
}

A DT with use-ncsi and phy-mode = "rgmii" would pass the schema, and the
driver would silently use RMII. The binding documents use-ncsi as
"Currently assumes rmii (100bT)", so the driver's behaviour matches the
documentation.

Should the binding exempt use-ncsi configurations from the phy-mode
requirement?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261015-support_ast2700-v3-0-8d7999171ac4%40aspeedtech.com

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

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 (2) · 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 patch keeps the only copy of buffer DMA address bits 34:32
  in rxdes2[18:16].
- [Low] After this patch the descriptor layout comments in ftgmac100.h
  contradict the code.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the TX
  completion path has no read-side barrier pairing with the producer's…
- [High] This problem wasn't introduced by this patch, but the
  ftgmac100_open() failure path for ncsi_start_dev() frees the RX…

--

net: ftgmac100: Add AST2700 upper DMA address support

This patch programs the high RX and TX descriptor ring base registers on
AST2700 and stores DMA address bits 34:32 in bits 18:16 of descriptor
word 2. It rebuilds the full 64-bit DMA address from the descriptors
before unmapping and sets a 35-bit DMA mask. All of this is gated by a
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 5a578a62df67..263594de0e07 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c

[ ... ]

> @@ -474,7 +482,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();
> @@ -601,6 +612,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;

[Severity: Medium]
With this patch, rxdes2 bits 18:16 are the only copy of the upper RX
buffer address bits. The driver reads them back here after the MAC has
completed the descriptor. Does the AST2700 MAC always leave
rxdes2[18:16] untouched when it writes the descriptor back?

The header still describes this word as one the hardware writes:

	__le32	rxdes2; /* length/type on AST2500 */

If the MAC does write it, map would no longer match the address that
ftgmac100_alloc_rx_buf() mapped. dma_unmap_single() would then get a
handle that was never mapped, so the real mapping leaks and the cache
maintenance or swiotlb/IOMMU unmap hits the wrong range.
ftgmac100_free_buffers() rebuilds the address the same way.

The drop path in ftgmac100_rx_packet() also gives the descriptor back to
the MAC by rewriting only rxdes0:

drop:
	/* Clean rxdes0 (which resets own bit) */
	rxdes->rxdes0 = cpu_to_le32(status & priv->rxdes0_edorr_mask);

A clobbered rxdes2 would then send the next frame for that slot to
lower_32_bits(map) | (clobbered_hi << 32). AST2700 DRAM starts at
0x4_0000_0000, so every buffer has non-zero high bits.

Would it be safer to keep the full dma_addr_t for each RX slot in driver
memory (for example a priv->rx_dma[] array) and rewrite rxdes2 every
time a descriptor is handed back? If not, could the hardware guarantee
be documented?

[ ... ]

> @@ -677,9 +691,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: High]
This isn't a bug introduced by this patch, but the TX completion path
has no read barrier to pair with the producer's smp_wmb(). This patch
adds the txdes2 read to the same unordered set of loads.

The producer side in ftgmac100_hard_start_xmit():

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

The consumer side in NAPI on another CPU, via
ftgmac100_poll()->ftgmac100_tx_complete():

ftgmac100_tx_buf_cleanable()
	return priv->tx_pointer != priv->tx_clean_pointer;

ftgmac100_tx_complete_packet()
	ctl_stat = le32_to_cpu(txdes->txdes0);
	if (ctl_stat & FTGMAC100_TXDES0_TXDMA_OWN)
		return false;

	skb = priv->tx_skbs[pointer];
	netdev->stats.tx_packets++;
	netdev->stats.tx_bytes += skb->len;

Nothing like smp_rmb() or smp_load_acquire() sits between the
tx_pointer load and the txdes0 and tx_skbs[] loads. There is only a
control dependency, and that does not order a load against later loads
on ARMv7/ARMv8.

Can the consumer see the new tx_pointer together with a stale txdes0?
The stale value would have OWN clear, because ftgmac100_init_rings()
writes 0 and an earlier completion writes ctl_stat & txdes0_edotr_mask.

In that case a stale NULL tx_skbs[pointer] would be dereferenced at
skb->len. Alternatively, a stale txdes3/txdes2 address would be unmapped
and the skb freed while TX DMA is still reading it.

[ ... ]

> @@ -936,7 +961,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;

[Severity: High]
This is a pre-existing issue, but it concerns how this function is used.
When ncsi_start_dev() fails in ftgmac100_open(), are the RX buffers and
rings freed while the MAC DMA engines are still running?

ftgmac100_open()
  ftgmac100_init_all()
    ftgmac100_init_hw()    <- ring bases, RX autopoll via APTC
    ftgmac100_start_hw()   <- MACCR TXDMA_EN | RXDMA_EN | ...
  ncsi_start_dev() fails
    goto err_ncsi

err_ncsi:
	phy_stop(netdev->phydev);
	napi_disable(&priv->napi);
	netif_stop_queue(netdev);
err_alloc:
	ftgmac100_free_buffers(priv);
	free_irq(netdev->irq, netdev);
err_irq:
	netif_napi_del(&priv->napi);
err_hw:
	iowrite32(0, priv->base + FTGMAC100_OFFSET_IER);
	ftgmac100_free_rings(priv);

Unlike ftgmac100_stop(), this path never calls ftgmac100_stop_hw() to
clear MACCR. Could the MAC keep polling the freed descriptor rings and
DMA incoming frames into the freed skb data and the freed rx_scratch
buffer? That would last until the next open resets the MAC, or forever
if the interface is never reopened.

ncsi_start_dev() returns -ENOTTY when nd->state is neither registered
nor functional. An up/down/up sequence while the asynchronous NC-SI
probe from the first open is still running looks like it can reach
this, because ncsi_stop_dev() does not reset the state.

[ ... ]

> 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

[ ... ]

> @@ -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 */

[Severity: Low]
This isn't a bug, but the descriptor layout comments in this header now
disagree with the code:

struct ftgmac100_txdes {
	...
	__le32	txdes2; /* Reserved */
	...
};

struct ftgmac100_rxdes {
	...
	__le32	rxdes2; /* length/type on AST2500 */
	...
};

ftgmac100_hard_start_xmit(), ftgmac100_alloc_rx_buf() and
ftgmac100_init_rings() now write buffer address bits 34:32 into these
words. ftgmac100_free_tx_packet(), ftgmac100_rx_packet() and
ftgmac100_free_buffers() read them back.

Could the comments say that with FTGMAC100_QUIRK_DMA_35BIT (AST2700),
bits 18:16 of word 2 hold buffer address bits 34:32?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261015-support_ast2700-v3-0-8d7999171ac4%40aspeedtech.com