* [PATCH 1/5] dt-bindings: net: ftgmac100: Add AST2700 compatible
2026-09-27 7:16 [PATCH 0/5] net: ftgmac100: Add AST2700 support Jacky Chou
@ 2026-09-27 7:16 ` Jacky Chou
2026-09-30 10:06 ` Krzysztof Kozlowski
2026-09-30 21:17 ` netdev-bot+sashiko
2026-09-27 7:16 ` [PATCH 2/5] net: ftgmac100: Add AST2700 compatible support Jacky Chou
` (4 subsequent siblings)
5 siblings, 2 replies; 17+ messages in thread
From: Jacky Chou @ 2026-09-27 7:16 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Po-Yu Chuang
Cc: netdev, devicetree, linux-kernel, 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.
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 1/5] dt-bindings: net: ftgmac100: Add AST2700 compatible
2026-09-27 7:16 ` [PATCH 1/5] dt-bindings: net: ftgmac100: Add AST2700 compatible Jacky Chou
@ 2026-09-30 10:06 ` Krzysztof Kozlowski
2026-09-30 21:17 ` netdev-bot+sashiko
1 sibling, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2026-09-30 10:06 UTC (permalink / raw)
To: Jacky Chou
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Po-Yu Chuang, netdev, devicetree, linux-kernel
On Sun, Sep 27, 2026 at 03:16:20PM +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.
>
> Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
> ---
> Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/5] dt-bindings: net: ftgmac100: Add AST2700 compatible
2026-09-27 7:16 ` [PATCH 1/5] dt-bindings: net: ftgmac100: Add AST2700 compatible Jacky Chou
2026-09-30 10:06 ` Krzysztof Kozlowski
@ 2026-09-30 21:17 ` netdev-bot+sashiko
1 sibling, 0 replies; 17+ messages in thread
From: netdev-bot+sashiko @ 2026-09-30 21:17 UTC (permalink / raw)
To: jacky_chou
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, ratbert, netdev, devicetree, linux-kernel
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] Concern: at this commit, "aspeed,ast2700-mac" is in
Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml…
--
dt-bindings: net: ftgmac100: Add AST2700 compatible
This commit adds aspeed,ast2700-mac to the compatible list in the FTGMAC100
binding. It also extends the RMII RCLK clock description to cover AST2700,
and it allows the resets property for AST2700 the same way it is allowed
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]
At this commit, ftgmac100_of_match[] in
drivers/net/ethernet/faraday/ftgmac100.c has no entry for
aspeed,ast2700-mac. Would an AST2700 node bind through the
faraday,ftgmac100 fallback and be treated as FTGMAC100_FARADAY?
If it did:
- is_aspeed would stay false.
- EDORR/EDOTR would use BIT(15) instead of BIT(30).
- ftgmac100_setup_clk() would never run.
- The in-MAC MDIO would be set up.
- The RMII reset path would be skipped.
Later patches in the series seem to fix this. The patch "net: ftgmac100:
Add AST2700 compatible support" adds:
{ .compatible = "aspeed,ast2700-mac",
.data = &ftgmac100_match_data_ast2700 },
The same patch adds FTGMAC100_AST2700 to the Aspeed branch in
ftgmac100_probe(), which sets BIT(30) and is_aspeed. Separately, "net:
ftgmac100: Add AST2700 upper DMA address support" adds the high address
bits.
No in-tree AST2700 DTS under arch/arm64/boot/dts/aspeed/ has an ethernet
node. So nothing in the tree should be affected between the binding patch
and the driver patch.
One small leftover: this patch changes the binding text to "RMII RCLK
gate for AST2500/2600/2700". The rclk comment in struct ftgmac100 still
says:
/* AST2500/AST2600 RMII ref clock gate */
Could that comment be updated to include AST2700 too?
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927-support_ast2700-v1-0-9f03f29d2c82%40aspeedtech.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/5] net: ftgmac100: Add AST2700 compatible support
2026-09-27 7:16 [PATCH 0/5] net: ftgmac100: Add AST2700 support Jacky Chou
2026-09-27 7:16 ` [PATCH 1/5] dt-bindings: net: ftgmac100: Add AST2700 compatible Jacky Chou
@ 2026-09-27 7:16 ` Jacky Chou
2026-09-30 21:17 ` netdev-bot+sashiko
2026-09-27 7:16 ` [PATCH 3/5] net: ftgmac100: Enable AST2700 RMII support Jacky Chou
` (3 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Jacky Chou @ 2026-09-27 7:16 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Po-Yu Chuang
Cc: netdev, devicetree, linux-kernel, 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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/5] net: ftgmac100: Add AST2700 compatible support
2026-09-27 7:16 ` [PATCH 2/5] net: ftgmac100: Add AST2700 compatible support Jacky Chou
@ 2026-09-30 21:17 ` netdev-bot+sashiko
0 siblings, 0 replies; 17+ messages in thread
From: netdev-bot+sashiko @ 2026-09-30 21:17 UTC (permalink / raw)
To: jacky_chou
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, ratbert, netdev, devicetree, linux-kernel
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] Baseline decision: false (resolved later in the series).
- [Low] Several driver comments listing which SoCs share a behaviour now
leave out AST2700, although the code and binding include it.
Pre-existing issues:
- [High] This bug was not introduced by this patch, but the patch extends
it to the new AST2700 compatible.
--
net: ftgmac100: Add AST2700 compatible support
This commit adds match data for the Aspeed AST2700 FTGMAC100 controller
and adds the aspeed,ast2700-mac compatible to the OF match table. In
ftgmac100_probe(), AST2700 is handled like the other Aspeed MACs. That
sets is_aspeed and skips the internal MDIO setup.
> 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: High]
This is a pre-existing issue, but this change extends it to the new
AST2700 compatible. When is_aspeed is set, ftgmac100_reset_and_config_mac()
dereferences phydev without a NULL check:
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) {
...
}
This check came from commit af350ee72e9dda.
ftgmac100_probe_dt() can return 0 with no PHY attached. Take a node
with no use-ncsi, no fixed-link and no phy-handle, but with an mdio
child. For that node, ftgmac100_mii_probe() is skipped:
ftgmac100_probe_dt() {
...
if (!ftgmac100_has_child_node(np, "mdio")) {
/* Support legacy ASPEED devicetree descriptions that
...
}
The binding does not require phy-handle and allows an mdio child for
all compatibles. So an aspeed,ast2700-mac node like this would register
successfully with netdev->phydev == NULL.
Could bringing the interface up then oops here? ftgmac100_open() treats
phydev as optional for phy_start(), but only after this call:
ftgmac100_open()
ftgmac100_reset_and_config_mac()
priv->netdev->phydev->interface <-- NULL phydev
The reset task reaches the same helper:
ftgmac100_reset_task()
ftgmac100_reset()
ftgmac100_reset_and_config_mac()
Later in the series, "net: ftgmac100: Enable AST2700 RMII support" adds
another unguarded phydev->interface dereference in ftgmac100_start_hw()
for the AST2700 RMII quirk. This problem is still there at the end of
the series.
[Severity: Low]
This isn't a bug, but AST2700 now sets is_aspeed and skips
ftgmac100_setup_mdio(). A few comments no longer list every SoC that
takes these paths:
struct ftgmac100:
/* AST2500/AST2600 RMII ref clock gate */
struct clk *rclk;
ftgmac100_setup_mdio():
/* The AST2600 has a separate MDIO controller */
ftgmac100_probe_dt():
* them. 2600 has an independent MDIO controller, not
* part of the MAC.
The binding already describes the RCLK gate as being for
AST2500/2600/2700, and ftgmac100_setup_clk() now also runs for AST2700.
Should these comments mention AST2700 too? No later patch in the series
updates them.
[ ... ]
> @@ -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
> +};
> +
[ ... ]
> @@ -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 },
[Severity: Low]
At this point in the series, can AST2700 DMA to its memory? All AST2700
DRAM is above 4 GiB:
arch/arm64/boot/dts/aspeed/ast2700-evb.dts:
memory@400000000 {
reg = <0x4 0x00000000 0x0 0x40000000>;
The driver only handles 32-bit DMA addresses here. No dma_set_mask*()
call is made for AST2700, and only the low 32 bits get programmed:
ftgmac100_init_hw():
iowrite32(priv->rxdes_dma, priv->base + FTGMAC100_OFFSET_RXR_BADR);
ftgmac100_alloc_rx_buf():
rxdes->rxdes3 = cpu_to_le32(map);
ftgmac100_hard_start_xmit():
txdes->txdes3 = cpu_to_le32(map);
With the default 32-bit mask, ftgmac100_alloc_rings() would fail and
ftgmac100_open() would return -ENOMEM on AST2700.
A later patch in the series, "net: ftgmac100: Add AST2700 upper DMA
address support", fixes this. It adds FTGMAC100_QUIRK_DMA_35BIT to the
AST2700 match data, sets a 35-bit DMA mask in probe, and programs the
upper address bits for the rings, the descriptors and the unmap paths.
Also, at this commit the driver cannot be built for ARM64 without
BROKEN.
Would it make sense to reorder the series so the compatible is only
added once DMA support is in place?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927-support_ast2700-v1-0-9f03f29d2c82%40aspeedtech.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/5] net: ftgmac100: Enable AST2700 RMII support
2026-09-27 7:16 [PATCH 0/5] net: ftgmac100: Add AST2700 support Jacky Chou
2026-09-27 7:16 ` [PATCH 1/5] dt-bindings: net: ftgmac100: Add AST2700 compatible Jacky Chou
2026-09-27 7:16 ` [PATCH 2/5] net: ftgmac100: Add AST2700 compatible support Jacky Chou
@ 2026-09-27 7:16 ` Jacky Chou
2026-09-30 21:18 ` netdev-bot+sashiko
2026-09-27 7:16 ` [PATCH 4/5] net: ftgmac100: Add AST2700 upper DMA address support Jacky Chou
` (2 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Jacky Chou @ 2026-09-27 7:16 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Po-Yu Chuang
Cc: netdev, devicetree, linux-kernel, 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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/5] net: ftgmac100: Enable AST2700 RMII support
2026-09-27 7:16 ` [PATCH 3/5] net: ftgmac100: Enable AST2700 RMII support Jacky Chou
@ 2026-09-30 21:18 ` netdev-bot+sashiko
0 siblings, 0 replies; 17+ messages in thread
From: netdev-bot+sashiko @ 2026-09-30 21:18 UTC (permalink / raw)
To: jacky_chou
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, ratbert, netdev, devicetree, linux-kernel
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()
Pre-existing issues:
- [High] An Aspeed MAC (AST2400/2500/2600, and AST2700 once the series
adds it) can probe successfully with no PHY attached, and opening it…
--
net: ftgmac100: Enable AST2700 RMII support
This commit adds an FTGMAC100_QUIRK_RMII_ENABLE match-data quirk and sets
it for AST2700. When the quirk is set and the attached PHY uses RMII,
ftgmac100_start_hw() sets the new FTGMAC100_MACCR_RMII_ENABLE bit in
MACCR.
> 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) {
> + maccr |= FTGMAC100_MACCR_RMII_ENABLE;
> + }
[Severity: High]
Can phydev be NULL here on AST2700?
ftgmac100_probe_dt() can return 0 without connecting a PHY. That
happens when the MAC node has an "mdio" child node but no "use-ncsi",
"fixed-link" or "phy-handle" property. The mdio child check only
skips the legacy ftgmac100_mii_probe() scan, so nothing sets
netdev->phydev and register_netdev() still succeeds.
When the interface is later brought up, ftgmac100_open() reaches
ftgmac100_start_hw() through ftgmac100_init_all(). Because
FTGMAC100_QUIRK_RMII_ENABLE is set for AST2700, the new check reads
phydev->interface without testing phydev first. The same applies to
the other callers of ftgmac100_start_hw(): ftgmac100_set_rx_mode() and
the need_mac_restart path in ftgmac100_poll().
The rest of the driver treats a missing PHY as a valid state.
ftgmac100_open() calls phy_start() only when netdev->phydev is set, and
ftgmac100_reset() and ftgmac100_phy_disconnect() also check for NULL.
A devicetree mistake like this should either fail probe or be handled.
It should not oops the kernel with rtnl_lock held from dev_open().
The is_aspeed RMII check in ftgmac100_reset_and_config_mac() makes the
same assumption, so guarding only this spot may not be enough.
Would it make sense to reject the PHY-less configuration in
ftgmac100_probe_dt() when neither NCSI nor a PHY is present? Another
option is to cache the interface mode when the PHY is connected and
use the cached value in both places.
[Severity: High]
This isn't a bug introduced by this patch, but can phydev be NULL here?
ftgmac100_probe_dt() can return 0 without attaching a PHY. This happens
when the MAC node has an "mdio" child but no use-ncsi, fixed-link or
phy-handle property:
ftgmac100_probe_dt() {
...
if (!ftgmac100_has_child_node(np, "mdio")) {
...
err = ftgmac100_mii_probe(netdev);
...
}
return 0;
}
In that case register_netdev() succeeds and netdev->phydev is NULL.
The existing check in ftgmac100_reset_and_config_mac() already
dereferences the pointer without a NULL check:
ftgmac100_reset_and_config_mac() {
...
if (priv->is_aspeed && priv->netdev->phydev->interface == PHY_INTERFACE_MODE_RMII) {
...
}
On every path that reaches ftgmac100_start_hw() (ftgmac100_init_all() on
open and reset, ftgmac100_set_rx_mode(), and the need_mac_restart path in
ftgmac100_poll()), ftgmac100_reset_and_config_mac() either runs first or
has already succeeded. This means the NULL dereference is already
reachable on AST2400/2500/2600 through
ftgmac100_open()->ftgmac100_reset_and_config_mac(). The new check in
ftgmac100_start_hw() makes the same assumption for AST2700.
Elsewhere the driver treats a NULL phydev as valid. ftgmac100_open()
calls phy_start() only when netdev->phydev is set, and ftgmac100_reset()
and ftgmac100_phy_disconnect() also check for NULL.
dev_open() holds rtnl_lock, so an oops here would leave RTNL held.
Could probe reject PHY-less configurations instead? Another option is to
guard both dereferences, or to cache the interface mode when the PHY is
connected.
> +
> /* Hit the HW */
> iowrite32(maccr, priv->base + FTGMAC100_OFFSET_MACCR);
> }
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927-support_ast2700-v1-0-9f03f29d2c82%40aspeedtech.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/5] net: ftgmac100: Add AST2700 upper DMA address support
2026-09-27 7:16 [PATCH 0/5] net: ftgmac100: Add AST2700 support Jacky Chou
` (2 preceding siblings ...)
2026-09-27 7:16 ` [PATCH 3/5] net: ftgmac100: Enable AST2700 RMII support Jacky Chou
@ 2026-09-27 7:16 ` Jacky Chou
2026-09-30 21:18 ` netdev-bot+sashiko
2026-09-27 7:16 ` [PATCH 5/5] net: ftgmac100: Allow building on ARM64 Jacky Chou
2026-09-27 17:39 ` [PATCH 0/5] net: ftgmac100: Add AST2700 support Andrew Lunn
5 siblings, 1 reply; 17+ messages in thread
From: Jacky Chou @ 2026-09-27 7:16 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Po-Yu Chuang
Cc: netdev, devicetree, linux-kernel, 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 b835472da360..4becd28f805b 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);
@@ -2094,6 +2129,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) {
@@ -2146,7 +2189,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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 4/5] net: ftgmac100: Add AST2700 upper DMA address support
2026-09-27 7:16 ` [PATCH 4/5] net: ftgmac100: Add AST2700 upper DMA address support Jacky Chou
@ 2026-09-30 21:18 ` netdev-bot+sashiko
0 siblings, 0 replies; 17+ messages in thread
From: netdev-bot+sashiko @ 2026-09-30 21:18 UTC (permalink / raw)
To: jacky_chou
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, ratbert, netdev, devicetree, linux-kernel
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 descriptor layout comments in
drivers/net/ethernet/faraday/ftgmac100.h now contradict the code.
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but TX completion
reads the producer-published ring index without a paired read barrier.
- [High] This problem wasn't introduced by this patch, but the NC-SI
failure path in ftgmac100_open() frees DMA memory while the MAC's DMA…
--
net: ftgmac100: Add AST2700 upper DMA address support
This commit adds 35-bit DMA addressing for the AST2700. It programs the
high RX and TX ring base registers and stores buffer address bits 34:32
in descriptor word 2. All of this is gated by a new
FTGMAC100_QUIRK_DMA_35BIT match-data quirk, which also sets a 35-bit DMA
mask at probe time.
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index b835472da3604..4becd28f805bc 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: High]
This is a pre-existing issue, but the txdes2 read added here has the same
ordering problem as the existing txdes0 and tx_skbs[] reads on the TX
completion path. Is there a read barrier that pairs with the smp_wmb() in
ftgmac100_hard_start_xmit()?
The producer side does:
ftgmac100_hard_start_xmit()
txdes->txdes2 = ...;
txdes->txdes3 = ...;
...
dma_wmb();
first->txdes0 = cpu_to_le32(f_ctl_stat);
smp_wmb();
priv->tx_pointer = pointer;
The NAPI consumer side does:
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_bytes += skb->len;
ftgmac100_free_tx_packet(priv, pointer, skb, txdes, ctl_stat);
The smp_mb() in ftgmac100_tx_complete() only runs after this loop.
xmit and NAPI poll can run on different CPUs of an SMP AST2600/AST2700.
Could the NAPI CPU see the new tx_pointer while still reading the stale
txdes0 of that slot, with TXDMA_OWN clear from the previous clean, and a
stale tx_skbs[] entry?
If that happens, a packet that was just queued would be handled as
completed. skb could be NULL when skb->len is read. The buffer could also
be unmapped and freed while the MAC is about to DMA from it, and txdes0
would be overwritten, clearing the TXDMA_OWN bit that the producer just
set.
Would an smp_rmb() after ftgmac100_tx_buf_cleanable(), or an
smp_load_acquire() of tx_pointer, close this window?
> if (ctl_stat & FTGMAC100_TXDES0_FTS) {
> len = skb_headlen(skb);
> dma_unmap_single(priv->dev, map, len, DMA_TO_DEVICE);
[ ... ]
> @@ -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;
[Severity: High]
This is a pre-existing issue, and this patch did not introduce it.
ftgmac100_open() can reach ftgmac100_free_buffers() while the MAC's RX
and TX DMA engines are still enabled.
ftgmac100_open()->ftgmac100_init_all()->ftgmac100_start_hw() sets:
maccr |= FTGMAC100_MACCR_TXDMA_EN |
FTGMAC100_MACCR_RXDMA_EN |
...
If ncsi_start_dev() fails after that, the error path is:
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);
Only IER is cleared here. ftgmac100_stop_hw(), which clears MACCR, is
never called on this path, although ftgmac100_stop() does call it.
ncsi_start_dev() can fail on a reopen with NCSI_DEV_PROBED set. In that
case ncsi_reset_dev()->ncsi_choose_active_channel() returns -ENODEV when
no channel is found.
After that, can the RX engine keep polling the freed descriptor ring and
DMA incoming frames into the freed skb buffers, whose addresses are still
in rxdes3 and rxdes2?
Would calling ftgmac100_stop_hw() on this error path prevent that?
[ ... ]
> 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)
> /*
> * 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)
> +
[Severity: Low]
This isn't a bug, but the descriptor layout comments in this header
still describe these words differently:
struct ftgmac100_txdes {
...
__le32 txdes2; /* Reserved */
...
};
struct ftgmac100_rxdes {
...
__le32 rxdes2; /* length/type on AST2500 */
...
};
When FTGMAC100_QUIRK_DMA_35BIT is set, the driver now writes buffer
address bits 34:32 into txdes2 and rxdes2. The writes happen in
ftgmac100_alloc_rx_buf(), ftgmac100_init_rings() and
ftgmac100_hard_start_xmit(). The bits are read back in
ftgmac100_rx_packet(), ftgmac100_free_buffers() and
ftgmac100_free_tx_packet().
Could these comments be updated to mention the AST2700 upper address
bits? The last patch in the series does not update them either.
> #endif /* __FTGMAC100_H */
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927-support_ast2700-v1-0-9f03f29d2c82%40aspeedtech.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/5] net: ftgmac100: Allow building on ARM64
2026-09-27 7:16 [PATCH 0/5] net: ftgmac100: Add AST2700 support Jacky Chou
` (3 preceding siblings ...)
2026-09-27 7:16 ` [PATCH 4/5] net: ftgmac100: Add AST2700 upper DMA address support Jacky Chou
@ 2026-09-27 7:16 ` Jacky Chou
2026-09-30 21:18 ` netdev-bot+sashiko
2026-09-27 17:39 ` [PATCH 0/5] net: ftgmac100: Add AST2700 support Andrew Lunn
5 siblings, 1 reply; 17+ messages in thread
From: Jacky Chou @ 2026-09-27 7:16 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Po-Yu Chuang
Cc: netdev, devicetree, linux-kernel, 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.
Keep selecting the Aspeed MDIO controller for MACH_ASPEED_G6 and also
select it for ARCH_ASPEED. This preserves the existing AST2600
dependency while covering newer Aspeed SoCs that use the common ARCH
symbol.
Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
drivers/net/ethernet/faraday/Kconfig | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/faraday/Kconfig b/drivers/net/ethernet/faraday/Kconfig
index 474073c7f94d..dea6dcfc9ab5 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 MACH_ASPEED_G6 || ARCH_ASPEED
select CRC32
help
This driver supports the FTGMAC100 Gigabit Ethernet controller
--
2.34.1
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 5/5] net: ftgmac100: Allow building on ARM64
2026-09-27 7:16 ` [PATCH 5/5] net: ftgmac100: Allow building on ARM64 Jacky Chou
@ 2026-09-30 21:18 ` netdev-bot+sashiko
0 siblings, 0 replies; 17+ messages in thread
From: netdev-bot+sashiko @ 2026-09-30 21:18 UTC (permalink / raw)
To: jacky_chou
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, ratbert, netdev, devicetree, linux-kernel
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · 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).
- [Medium] In the FTGMAC100 Kconfig entry
(drivers/net/ethernet/faraday/Kconfig, line 34), the new condition…
--
net: ftgmac100: Allow building on ARM64
This commit lets the Faraday vendor menu and the FTGMAC100 driver be
selected on ARM64 so AST2700 platforms can use the MAC. It also removes
the !64BIT || BROKEN dependency. The select of MDIO_ASPEED is widened
from MACH_ASPEED_G6 to MACH_ASPEED_G6 || ARCH_ASPEED.
> diff --git a/drivers/net/ethernet/faraday/Kconfig b/drivers/net/ethernet/faraday/Kconfig
> index 474073c7f94d7..dea6dcfc9ab57 100644
> --- a/drivers/net/ethernet/faraday/Kconfig
> +++ b/drivers/net/ethernet/faraday/Kconfig
[ ... ]
> @@ -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 MACH_ASPEED_G6 || ARCH_ASPEED
[Severity: Medium]
Does this select now cover every 32-bit Aspeed generation, not only the
newer SoCs?
On ARM32, arch/arm/mach-aspeed/Kconfig puts all generations under
ARCH_ASPEED:
menuconfig ARCH_ASPEED
bool "Aspeed BMC architectures"
...
if ARCH_ASPEED
config MACH_ASPEED_G4
...
config MACH_ASPEED_G5
...
config MACH_ASPEED_G6
...
endif
So MACH_ASPEED_G6 already implies ARCH_ASPEED, and the condition becomes
"if ARCH_ASPEED". The MACH_ASPEED_G6 term has no effect, and MDIO_ASPEED
is now forced on in AST2400/AST2500-only kernels.
Two in-tree configs change this way. aspeed_g4_defconfig has
CONFIG_MACH_ASPEED_G4=y and CONFIG_FTGMAC100=y with no G6, so it now gets
MDIO_ASPEED=y. multi_v5_defconfig (MACH_ASPEED_G4=y, FTGMAC100=m) now
gets MDIO_ASPEED=m. Neither had MDIO_ASPEED before. Also,
mdio-aspeed.c only matches "aspeed,ast2600-mdio", so those kernels build
a driver that never binds.
The MDIO_ASPEED help text in drivers/net/mdio/Kconfig says:
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.
With this select, users can no longer choose N on those platforms. The
help text also still does not mention AST2700.
The commit message says:
Keep selecting the Aspeed MDIO controller for MACH_ASPEED_G6 and also
select it for ARCH_ASPEED. This preserves the existing AST2600
dependency while covering newer Aspeed SoCs that use the common ARCH
symbol.
It does not say that older 32-bit AST2400/AST2500 configs are affected
too.
Would a narrower condition keep the old ARM32 behaviour? For example:
select MDIO_ASPEED if MACH_ASPEED_G6 || (ARM64 && ARCH_ASPEED)
If the wider scope is intended, could the redundant MACH_ASPEED_G6 term
be dropped, and the commit message and MDIO_ASPEED help text updated to
match?
> select CRC32
> help
> This driver supports the FTGMAC100 Gigabit Ethernet controller
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927-support_ast2700-v1-0-9f03f29d2c82%40aspeedtech.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/5] net: ftgmac100: Add AST2700 support
2026-09-27 7:16 [PATCH 0/5] net: ftgmac100: Add AST2700 support Jacky Chou
` (4 preceding siblings ...)
2026-09-27 7:16 ` [PATCH 5/5] net: ftgmac100: Allow building on ARM64 Jacky Chou
@ 2026-09-27 17:39 ` Andrew Lunn
2026-09-29 7:12 ` 回覆: " Jacky Chou
5 siblings, 1 reply; 17+ messages in thread
From: Andrew Lunn @ 2026-09-27 17:39 UTC (permalink / raw)
To: Jacky Chou
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Po-Yu Chuang, netdev, devicetree, linux-kernel
On Sun, Sep 27, 2026 at 03:16:19PM +0800, Jacky Chou wrote:
> 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.
There is no mention of RGMII here or RGMII delays. Given the mess that
the AST2600 is, i would expect to see some explanation how that has
been solved.
Andrew
^ permalink raw reply [flat|nested] 17+ messages in thread* 回覆: [PATCH 0/5] net: ftgmac100: Add AST2700 support
2026-09-27 17:39 ` [PATCH 0/5] net: ftgmac100: Add AST2700 support Andrew Lunn
@ 2026-09-29 7:12 ` Jacky Chou
2026-09-29 17:08 ` Andrew Lunn
0 siblings, 1 reply; 17+ messages in thread
From: Jacky Chou @ 2026-09-29 7:12 UTC (permalink / raw)
To: Andrew Lunn
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Po-Yu Chuang, netdev, devicetree, linux-kernel
Hi Andrew,
Thank you for your reply.
> > 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.
>
> There is no mention of RGMII here or RGMII delays. Given the mess that the
> AST2600 is, i would expect to see some explanation how that has been solved.
>
Currently, we provide our custom or user to configure RGMII delay in bootloader stage,
it is the same as AST2600 in bootloader. So, when booting to kernel, the RGMII delay will
be kept. The ftgmac100 driver with AST2600 and the next generation AST2700 does not
need to configure the RGMII delay.
I will add more descriptions about the RGMII delay configuration in this first mail in next
version.
Thanks,
Jacky
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: 回覆: [PATCH 0/5] net: ftgmac100: Add AST2700 support
2026-09-29 7:12 ` 回覆: " Jacky Chou
@ 2026-09-29 17:08 ` Andrew Lunn
2026-09-30 1:53 ` 回覆: " Jacky Chou
0 siblings, 1 reply; 17+ messages in thread
From: Andrew Lunn @ 2026-09-29 17:08 UTC (permalink / raw)
To: Jacky Chou
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Po-Yu Chuang, netdev, devicetree, linux-kernel
> Currently, we provide our custom or user to configure RGMII delay in bootloader stage,
> it is the same as AST2600 in bootloader.
From what i understand, openBMC have changed their bootloader to not
insert delays, for the ATS2600. That allows them to have DTS files
with the correct 'rgmii-id'.
> So, when booting to kernel, the RGMII delay will
> be kept. The ftgmac100 driver with AST2600 and the next generation AST2700 does not
> need to configure the RGMII delay.
Does your AST2700 bootloader also have the delays off by default?
Andrew
^ permalink raw reply [flat|nested] 17+ messages in thread* 回覆: 回覆: [PATCH 0/5] net: ftgmac100: Add AST2700 support
2026-09-29 17:08 ` Andrew Lunn
@ 2026-09-30 1:53 ` Jacky Chou
2026-09-30 12:55 ` Andrew Lunn
0 siblings, 1 reply; 17+ messages in thread
From: Jacky Chou @ 2026-09-30 1:53 UTC (permalink / raw)
To: Andrew Lunn
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Po-Yu Chuang, netdev, devicetree, linux-kernel
> > Currently, we provide our custom or user to configure RGMII delay in
> > bootloader stage, it is the same as AST2600 in bootloader.
>
> From what i understand, openBMC have changed their bootloader to not insert
> delays, for the ATS2600. That allows them to have DTS files with the correct
> 'rgmii-id'.
>
Yes. We also change our SDK to configure the phy-mode to "rgmii-id" to suggest
our customer or user in AST2600 and AST2700.
> > So, when booting to kernel, the RGMII delay will be kept. The
> > ftgmac100 driver with AST2600 and the next generation AST2700 does not
> > need to configure the RGMII delay.
>
> Does your AST2700 bootloader also have the delays off by default?
>
Yes. All of MACs in AST2700 are configured to disable RGMII delay with "rgmii-id".
Thanks,
Jacky
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: 回覆: 回覆: [PATCH 0/5] net: ftgmac100: Add AST2700 support
2026-09-30 1:53 ` 回覆: " Jacky Chou
@ 2026-09-30 12:55 ` Andrew Lunn
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Lunn @ 2026-09-30 12:55 UTC (permalink / raw)
To: Jacky Chou
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Po-Yu Chuang, netdev, devicetree, linux-kernel
> > Does your AST2700 bootloader also have the delays off by default?
> >
>
> Yes. All of MACs in AST2700 are configured to disable RGMII delay with "rgmii-id".
static int ftgmac100_mii_probe(struct net_device *netdev)
{
struct ftgmac100 *priv = netdev_priv(netdev);
struct platform_device *pdev = to_platform_device(priv->dev);
struct device_node *np = pdev->dev.of_node;
struct phy_device *phydev;
phy_interface_t phy_intf;
int err;
if (!priv->mii_bus) {
dev_err(priv->dev, "No MDIO bus available\n");
return -ENODEV;
}
/* Default to RGMII. It's a gigabit part after all */
err = of_get_phy_mode(np, &phy_intf);
if (err)
phy_intf = PHY_INTERFACE_MODE_RGMII;
This default is now incorrect for the 2700.
I would suggest that for the 2700 you make phy-mode a required DT
property, and fail the probe if it is missing.
Andrew
^ permalink raw reply [flat|nested] 17+ messages in thread