[PATCH net-next v4 0/7] net: ftgmac100: Add AST2700 support

[PATCH net-next v4 0/7] 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 makes the existing aspeed,ast2400-mac, aspeed,ast2500-mac,
  and aspeed,ast2600-mac compatibles standalone, dropping the
  requirement to pair them with the generic "faraday,ftgmac100"
  fallback compatible.
- patch 2 adds the aspeed,ast2700-mac compatible to the binding, with
  its RMII RCLK gate, reset support, and the "phy-mode" requirement.
- patch 3 adds the AST2700 match data and OF compatible.
- patch 4 programs the AST2700 RMII enable bit and documents why
  netdev->phydev can never be NULL at that point for AST2700.
- patch 5 makes "phy-mode" a required device tree property for AST2700
  and fails probe if it is missing.
- patch 6 adds AST2700 upper DMA address handling for rings and
  descriptors.
- patch 7 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 v4:
- Split the dt-bindings patch into two: one making the existing
  aspeed,ast2400-mac, aspeed,ast2500-mac, and aspeed,ast2600-mac
  compatibles standalone, and a second adding the new
  aspeed,ast2700-mac compatible, per review feedback that these are
  two separate concerns.
- Link to v3: https://lore.kernel.org/r/20261015-support_ast2700-v3-0-8d7999171ac4@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 (7):
      dt-bindings: net: ftgmac100: Make Aspeed compatibles standalone
      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: f09123082704b08d56fd044a2166e3e39fa68563
change-id: 20260623-support_ast2700-68fb164da45c

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

[PATCH net-next v4 1/7] dt-bindings: net: ftgmac100: Make Aspeed compatibles standalone

[Jacky Chou]

The aspeed,ast2400-mac, aspeed,ast2500-mac, and aspeed,ast2600-mac
compatibles are currently only valid when paired 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.

Make each of them a standalone compatible instead, so none of the
Aspeed compatibles require the generic fallback alongside them.

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

---
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.
---
 .../devicetree/bindings/net/faraday,ftgmac100.yaml         | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
index d14410018bcf..c870dd0bda7b 100644
--- a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
+++ b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
@@ -13,12 +13,10 @@ 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
 
   reg:
     maxItems: 1
@@ -94,14 +92,14 @@ 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

Re: [PATCH net-next v4 1/7] dt-bindings: net: ftgmac100: Make Aspeed compatibles standalone

[Rob Herring]

On Tue, Oct 20, 2026 at 07:34:12PM +0800, Jacky Chou wrote:
> The aspeed,ast2400-mac, aspeed,ast2500-mac, and aspeed,ast2600-mac
> compatibles are currently only valid when paired 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.
> 
> Make each of them a standalone compatible instead, so none of the
> Aspeed compatibles require the generic fallback alongside them.
> 
> Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
> 
> ---
> 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.

No. This just creates unnecessary warnings even if only temporary.

What a driver uses doesn't matter. That can evolve over time, but that 
doesn't mean we change the DT schema.

Rob

回覆: [PATCH net-next v4 1/7] dt-bindings: net: ftgmac100: Make Aspeed compatibles standalone

[Jacky Chou]

Hi Rob,

> > The aspeed,ast2400-mac, aspeed,ast2500-mac, and aspeed,ast2600-mac
> > compatibles are currently only valid when paired 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.
> >
> > Make each of them a standalone compatible instead, so none of the
> > Aspeed compatibles require the generic fallback alongside them.
> >
> > Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
> >
> > ---
> > 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.
> 
> No. This just creates unnecessary warnings even if only temporary.
> 
> What a driver uses doesn't matter. That can evolve over time, but that doesn't
> mean we change the DT schema.
> 

Thanks for the quick look.

Just so I don't end up re-spinning this back and forth: this patch
was written in response to Conor's comment on the previous round,
where he suggested folding the AST2400/2500/2600 fix into this
series rather than deferring it:

  "I'd fix the binding here, since you're gonna generate conflicts
   with yourself otherwise. The dts changes need to be sent
   separately anyway (or at least applied separately) since binding
   changes don't go via soc maintainers."

My original plan was actually closer to what you're asking for: only
touch aspeed,ast2700-mac (zero migration cost, no existing dts uses
it yet) and leave AST2400/2500/2600 paired with the faraday,ftgmac100
fallback as-is, specifically to avoid the kind of transitional
dtbs_check warning you're pointing out here. Conor asked me to widen
the scope to all four compatibles in this same series instead.

Since the two of you seem to want different things here, could you
sync up on which approach you'd prefer? I'm happy to go either way:

 (a) revert to my original scope - keep AST2400/2500/2600 paired
     with the fallback, and only make aspeed,ast2700-mac a standalone
     compatible, since it's the one case with no existing dts users
     to break, or

 (b) keep the standalone-compatible change for all four as in this
     patch, and coordinate with the Aspeed/arm-soc maintainers so the
     aspeed-g4/g5/g6.dtsi fallback removal lands in the same cycle
     (e.g. via an immutable branch), so dtbs_check never shows a
     warning in an actual tagged release.

Let me know which you'd like and I'll send the next version
accordingly.

Thanks,
Jacky

Re: [PATCH net-next v4 1/7] dt-bindings: net: ftgmac100: Make Aspeed compatibles standalone

[Conor Dooley]

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

On Tue, Oct 20, 2026 at 01:14:41PM -0500, Rob Herring wrote:
> On Tue, Oct 20, 2026 at 07:34:12PM +0800, Jacky Chou wrote:
> > The aspeed,ast2400-mac, aspeed,ast2500-mac, and aspeed,ast2600-mac
> > compatibles are currently only valid when paired 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.
> > 
> > Make each of them a standalone compatible instead, so none of the
> > Aspeed compatibles require the generic fallback alongside them.
> > 
> > Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
> > 
> > ---
> > 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.
> 
> No. This just creates unnecessary warnings even if only temporary.
> 
> What a driver uses doesn't matter. That can evolve over time, but that 
> doesn't mean we change the DT schema.

The reason I suggested changing this is because the device used as a
fallback compatible is apparently not actually compatible at all and
were it to be used the device would malfunction.
The original code (from 2016) only worked because it checked the machine
compatible and slightly later on matched on the ftgmac compatible but
then explicitly checked for "ast2400-mac" etc using
of_device_is_compatible().

If you don't want the current stuff deleted, then at the very least it
should be marked deprecated and the standalone aspeed compatible stuff
added alongside it.

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

Re: [PATCH net-next v4 1/7] dt-bindings: net: ftgmac100: Make Aspeed compatibles standalone

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 3 · 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] The commit message's reason for the change does not hold up
  for the code at this commit.
- [Medium] The binding now asks for a standalone "aspeed,ast2400-mac" /
  "aspeed,ast2500-mac" / "aspeed,ast2600-mac" compatible with no…
- [Medium] The patch deletes the `items: [enum: [aspeed,ast2400-mac,
  aspeed,ast2500-mac, aspeed,ast2600-mac], const: faraday,ftgmac100]`…

--

dt-bindings: net: ftgmac100: Make Aspeed compatibles standalone

This patch changes the faraday,ftgmac100 binding so that aspeed,ast2400-mac,
aspeed,ast2500-mac and aspeed,ast2600-mac are standalone compatibles. It
drops the two-string form that carried the "faraday,ftgmac100" fallback.
The examples now use "aspeed,ast2500-mac" on its own.

> The aspeed,ast2400-mac, aspeed,ast2500-mac, and aspeed,ast2600-mac
> compatibles are currently only valid when paired 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.

[Severity: Medium]
Is this rationale accurate for the driver at this commit?

In drivers/net/ethernet/faraday/ftgmac100.c, ftgmac100_of_match[] has
dedicated Aspeed entries ahead of the generic one:

    { .compatible = "aspeed,ast2400-mac",
      .data = &ftgmac100_match_data_ast2400},
    ...
    { .compatible = "faraday,ftgmac100",
      .data = &ftgmac100_match_data_faraday },

The most specific compatible wins. So a node with "aspeed,astXXXX-mac",
"faraday,ftgmac100" always gets the Aspeed mac_id, and ftgmac100_probe()
then does:

    if (priv->mac_id == FTGMAC100_AST2400 ||
        priv->mac_id == FTGMAC100_AST2500 ||
        priv->mac_id == FTGMAC100_AST2600) {
        priv->rxdes0_edorr_mask = BIT(30);
        priv->txdes0_edotr_mask = BIT(30);
        priv->is_aspeed = true;

The two-string form this patch removes therefore can't reach the BIT(15)
and no-clock path. The only node that could reach it is one with just
"faraday,ftgmac100", and the binding still accepts that through
"- const: faraday,ftgmac100".

The DMA mask claim also doesn't seem to match the driver. Nothing under
drivers/net/ethernet/faraday/ calls dma_set_mask*() at this commit. Even at
the end of the series, the only DMA mask set is the 35-bit one under
FTGMAC100_QUIRK_DMA_35BIT, and only the AST2700 match data selects it.

The note below the --- line says there is no known combination "where a
node has actually bound through the generic entry and gotten the wrong
EDORR bit". Before commit 03a2aba50b2c ("net: ftgmac100: List all
compatibles"), though, "faraday,ftgmac100" was the driver's only of_match
entry. Every Aspeed MAC bound through it, and the driver detected Aspeed
afterwards with of_device_is_compatible(). In those kernels the fallback
was how the device bound, not a hazard.

Could the commit message give the real reason for the change and drop the
DMA mask claim?

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

[Severity: Medium]
Should this items branch stay in the oneOf with deprecated: true instead of
being removed?

All eight Aspeed MAC nodes in the tree still use 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 to mac3):
    compatible = "aspeed,ast2600-mac", "faraday,ftgmac100";

Every remaining oneOf branch matches a single string. Won't every
AST2400/AST2500/AST2600 board DTB now fail dtbs_check? The schema would
also stop describing the DT form that is deployed in the field.

No later patch in this series touches arch/ or brings the old form back.
The "dt-bindings: net: ftgmac100: Add AST2700 compatible" patch only adds
aspeed,ast2700-mac to the enum plus new if blocks. The tree stays
inconsistent until a separate series lands through another tree.

Runtime binding on this kernel is not affected, because of_match prefers
the first compatible.

> +      - enum:
> +          - aspeed,ast2400-mac
> +          - aspeed,ast2500-mac
> +          - aspeed,ast2600-mac

[ ... ]

> @@ -94,14 +92,14 @@ unevaluatedProperties: false
>  examples:
>    - |
>      ethernet@1e660000 {
> -        compatible = "aspeed,ast2500-mac", "faraday,ftgmac100";
> +        compatible = "aspeed,ast2500-mac";
>          reg = <0x1e660000 0x180>;
>          interrupts = <2>;
>          use-ncsi;
>      };

[Severity: Medium]
What happens when a DTB written to this binding boots on a kernel that
doesn't have commit 03a2aba50b2c ("net: ftgmac100: List all compatibles")?

Before that commit, ftgmac100_of_match[] had only:

    { .compatible = "faraday,ftgmac100" },

Aspeed variants were identified after binding, with
of_device_is_compatible().

On those kernels, a node with only "aspeed,ast2500-mac" matches nothing.
That includes stable and LTS branches without 03a2aba50b2c. The Ethernet
MAC would never probe, which on BMCs usually means losing NC-SI and network
management. Other DT consumers that match only the generic string would be
affected the same way.

The fallback is what lets new DTBs work with older software. Could the
commit message mention this forward compatibility cost, instead of calling
the change only "a binding correctness fix"?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261020-support_ast2700-v4-0-4d36b40939b3%40aspeedtech.com

[PATCH net-next v4 2/7] 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.

Add aspeed,ast2700-mac as a standalone compatible, consistent with
the other Aspeed compatibles.

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
index c870dd0bda7b..0a376b849a91 100644
--- a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
+++ b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
@@ -17,6 +17,7 @@ properties:
           - aspeed,ast2400-mac
           - aspeed,ast2500-mac
           - aspeed,ast2600-mac
+          - aspeed,ast2700-mac
 
   reg:
     maxItems: 1
@@ -28,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
@@ -80,12 +81,21 @@ 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
 

-- 
2.34.1

Re: [PATCH net-next v4 2/7] dt-bindings: net: ftgmac100: Add AST2700 compatible

[Rob Herring (Arm)]

On Tue, 20 Oct 2026 19:34:13 +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.
> 
> Add aspeed,ast2700-mac as a standalone compatible, consistent with
> the other Aspeed compatibles.
> 
> Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
> ---
>  Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH net-next v4 2/7] dt-bindings: net: ftgmac100: Add AST2700 compatible

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 2 · 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 this binding patch, the new AST2700 rule `if compatible
  contains aspeed,ast2700-mac then required: [phy-mode]`…
- [Medium] This patch makes `aspeed,ast2700-mac` a standalone compatible,
  "consistent with the other Aspeed compatibles".
- [Low] Concern (baseline: not a valid finding): the commit message says
  the AST2700 phy-mode requirement matches "the driver's probe-time…
- [Low] Concern (baseline: not a valid finding): the binding documents
  `aspeed,ast2700-mac` as standalone, but ftgmac100_of_match[] has no…
- [Low] Concern (baseline: not a valid finding): for AST2700 the binding
  only allows `resets`, as for AST2600, instead of requiring it.

--

dt-bindings: net: ftgmac100: Add AST2700 compatible

This adds aspeed,ast2700-mac as a standalone compatible to the ftgmac100
binding. It allows the resets property for it, as is already done for
AST2600. It also adds a conditional block that requires phy-mode whenever
the AST2700 compatible is used.

> diff --git a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> index c870dd0bda7b2..0a376b849a916 100644
> --- a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> +++ b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> @@ -17,6 +17,7 @@ properties:
>            - aspeed,ast2400-mac
>            - aspeed,ast2500-mac
>            - aspeed,ast2600-mac
> +          - aspeed,ast2700-mac

[Severity: Medium]
This builds on the previous patch in the series, commit c64ed4df5e64
("dt-bindings: net: ftgmac100: Make Aspeed compatibles standalone"). That
patch turned the compatible into a oneOf of single strings. Do the existing
in-tree Aspeed MAC nodes still pass validation with that?

aspeed-g4.dtsi, aspeed-g5.dtsi and aspeed-g6.dtsi all use the two-string
form. For example, in aspeed-g6.dtsi:

    compatible = "aspeed,ast2600-mac", "faraday,ftgmac100";

A compatible with two entries matches neither oneOf branch. So dtbs_check
would now fail for the MAC nodes on every AST2400/2500/2600 board. The
series doesn't update those dtsi files, and neither commit message
mentions this.

The reason given in the previous patch also looks doubtful. In
__of_device_is_compatible() the score depends on the position in the
compatible list:

    if (of_compat_cmp(cp, compat, strlen(compat)) == 0) {
            score = INT_MAX/2 - (index << 2);
            break;
    }

So the more specific "aspeed,ast2x00-mac" entry always wins over
"faraday,ftgmac100". A two-string node can't bind through the generic
entry alone. The bare "faraday,ftgmac100" alternative is also still
accepted, so the hazard described there is still present.

Could the two-string fallback form stay as an accepted alternative, or
could the dtsi files be updated as part of this series?

[Severity: Low]
At this commit, ftgmac100_of_match[] has no entry for aspeed,ast2700-mac
and there is no FTGMAC100_AST2700 mac_id. Would a node that follows this
binding fail to bind at this point?

A later patch in the series, "net: ftgmac100: Add AST2700 compatible
support", seems to handle this. It adds:

    { .compatible = "aspeed,ast2700-mac",
      .data = &ftgmac100_match_data_ast2700 },

It also adds FTGMAC100_AST2700 to the is_aspeed / BIT(30) selection in
ftgmac100_probe(). So this only affects the intermediate state of the
series, and putting the binding before the driver is the usual DT order.

[ ... ]

> @@ -80,12 +81,21 @@ allOf:
>            contains:
>              enum:
>                - aspeed,ast2600-mac
> +              - aspeed,ast2700-mac
>      then:
>        properties:
>          resets: true
>      else:
>        properties:
>          resets: false

[Severity: Low]
This isn't a bug, but for AST2700 resets is only allowed here, not
required. ftgmac100_probe() gets the reset with
devm_reset_control_get_optional_exclusive(). If resets is missing, the
RMII SCU reset in ftgmac100_reset_and_config_mac() silently does nothing.

This matches the AST2600 handling and the commit message. AST2400/2500 run
the same is_aspeed && RMII reset path while their binding forbids resets,
so this is probably intended.

Does AST2700 hardware need the reset in a way AST2600 does not, so that
resets should be required?

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: aspeed,ast2700-mac
> +    then:
> +      required:
> +        - phy-mode

[Severity: Medium]
Should NC-SI nodes be exempt from this rule? There is no use-ncsi
exception, so an AST2700 node with use-ncsi and no phy-mode now fails
dtbs_check.

That is the same shape as the first example in this binding, which uses
aspeed,ast2500-mac. The use-ncsi description also says "Currently assumes
rmii (100bT)".

The driver doesn't read phy-mode in NC-SI mode, even at the end of the
series. ftgmac100_probe_dt() returns early:

    if (of_get_property(np, "use-ncsi", NULL))
            return ftgmac100_probe_ncsi(netdev, priv, pdev);

and ftgmac100_probe_ncsi() hardcodes the interface:

    err = phy_connect_direct(netdev, phydev, ftgmac100_adjust_link,
                             PHY_INTERFACE_MODE_RMII);

The phy-mode check that "net: ftgmac100: Require phy-mode for AST2700"
adds to ftgmac100_mii_probe() also looks unreachable on AST2700. Only
ftgmac100_setup_mdio() sets priv->mii_bus, and it is called only for
FARADAY/AST2400/AST2500. So ftgmac100_mii_probe() returns -ENODEV with
"No MDIO bus available" before it gets to the phy-mode test.

That leaves the generic requirement in of_phy_get_and_connect() on the
phy-handle/fixed-link path. AST2600 goes through the same path, but this
binding does not require phy-mode for AST2600.

Given this, is "matching the driver's probe-time requirement" accurate?
Could the rule require phy-mode only when use-ncsi is absent?

[Severity: Low]
On the same topic, 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.

At this commit the driver has no AST2700 support yet, and a missing
phy-mode falls back to RGMII. Is the driver requirement referred to here
only the one added later in the series?

After "net: ftgmac100: Add AST2700 compatible support" and "net:
ftgmac100: Require phy-mode for AST2700", the AST2700
phy-handle/fixed-link path does require phy-mode, through
of_phy_get_and_connect():

    ret = of_get_phy_mode(np, &iface);
    if (ret)
            return NULL;

The RGMII default no longer applies to AST2700, because
ftgmac100_mii_probe() returns -ENODEV before reaching it. The remaining
gap is the NC-SI case above.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261020-support_ast2700-v4-0-4d36b40939b3%40aspeedtech.com

[PATCH net-next v4 3/7] 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

Re: [PATCH net-next v4 3/7] 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] (Baseline decision: false positive.

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

--

net: ftgmac100: Add AST2700 compatible support

This patch adds an FTGMAC100_AST2700 mac_id, match data for it, and an
"aspeed,ast2700-mac" entry in ftgmac100_of_match. In ftgmac100_probe(),
AST2700 is handled like the other Aspeed variants: is_aspeed is set and the
BIT(30) end-of-ring masks are used.

> 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 isn't a bug introduced by this patch, but setting is_aspeed for
AST2700 exposes an existing NULL phydev dereference on this variant.

Suppose an aspeed,ast2700-mac node has an mdio child but no phy-handle,
fixed-link or use-ncsi. In that case ftgmac100_probe_dt() skips both PHY
paths and returns success with netdev->phydev still NULL:

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

	return 0;
}

ftgmac100_probe() only calls ftgmac100_setup_mdio() for FARADAY, AST2400
and AST2500, so on AST2700 the mdio child is ignored entirely.

register_netdev() then succeeds. When the interface comes up,
ftgmac100_open() calls ftgmac100_reset_and_config_mac() before it checks
phydev:

drivers/net/ethernet/faraday/ftgmac100.c:ftgmac100_reset_and_config_mac() {
	...
	if (priv->is_aspeed && priv->netdev->phydev->interface == PHY_INTERFACE_MODE_RMII) {
	...
}

Can this oops in ndo_open with RTNL held? The binding only requires
phy-mode for ast2700-mac and doesn't restrict mdio, so a DT like this
passes dt_binding_check. The same path already exists for AST2600.

The later patch "net: ftgmac100: Enable AST2700 RMII support" makes the
same assumption in ftgmac100_start_hw(). Its commit message says
"ftgmac100_probe_dt() always attaches a phy_device before register_netdev()
can succeed", but that does not hold for this configuration.

Could ftgmac100_probe_dt() reject the case with no PHY and no NC-SI before
register_netdev()? Alternatively, or in addition, the binding could forbid
mdio and require a PHY source for ast2600/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]
Once aspeed,ast2700-mac binds here, AST2700 uses a datapath that only
handles 32-bit DMA addresses.

ftgmac100_match_data_ast2700 has no FTGMAC100_QUIRK_DMA_35BIT, so
ftgmac100_probe() never widens the DMA mask. ftgmac100_init_hw() also
writes only the low 32 bits of the ring bases:

	iowrite32(priv->rxdes_dma, priv->base + FTGMAC100_OFFSET_RXR_BADR);
	...
	iowrite32(priv->txdes_dma, priv->base + FTGMAC100_OFFSET_NPTXR_BADR);

Buffer addresses are likewise written into the descriptors with
cpu_to_le32(map).

As far as I can tell, this can't be reached at this commit. The FTGMAC100
Kconfig entry still has "depends on ARM || COMPILE_TEST" and "depends on
!64BIT || BROKEN", so the driver can't be built for the arm64 AST2700.

Later in the series, "net: ftgmac100: Add AST2700 upper DMA address
support" adds the 35-bit quirk, programs the RXR/TXR BADDR_HIGH registers
and fills in the upper descriptor bits. "net: ftgmac100: Allow building on
ARM64" enables arm64 only after that, so the final state of the series
looks fine.

Is it intended that AST2700 binds to the driver before the upper DMA
address support is in place?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261020-support_ast2700-v4-0-4d36b40939b3%40aspeedtech.com

[PATCH net-next v4 4/7] 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

Re: [PATCH net-next v4 4/7] net: ftgmac100: Enable AST2700 RMII support

[netdev-bot+sashiko]

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

Critical: 0 · High: 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] The commit message and the new comment in ftgmac100_start_hw()
  (drivers/net/ethernet/faraday/ftgmac100.c) both say that an opened…

--

net: ftgmac100: Enable AST2700 RMII support

This patch adds a FTGMAC100_QUIRK_RMII_ENABLE match-data quirk for
AST2700. When the attached PHY uses RMII, ftgmac100_start_hw() now sets
FTGMAC100_MACCR_RMII_ENABLE. The commit message argues that
netdev->phydev is always populated by the time ftgmac100_start_hw() runs.

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

[ ... ]

> @@ -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;

[ ... ]

> @@ -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]
Can phydev be NULL here on AST2700?

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

ftgmac100_probe_dt() also has a fallback path. If the node has an "mdio"
child and no use-ncsi, fixed-link or phy-handle property, it returns
success without connecting a PHY:

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

	return 0;
}

ftgmac100_probe() then calls register_netdev() with netdev->phydev ==
NULL. The aspeed,ast2700-mac binding in faraday,ftgmac100.yaml appears to
allow this layout. phy-handle is optional, an mdio child is allowed, and
only phy-mode is required.

The rest of the driver also seems to treat phydev as optional. For
example, ftgmac100_open() does:

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

ftgmac100_stop(), ftgmac100_reset_task(), ftgmac100_set_pauseparam() and
ftgmac100_phy_disconnect() check for NULL in the same way.

The quirk bit is always set for AST2700, so the && does not
short-circuit. With such a DT, this path would dereference a NULL phydev:

ftgmac100_open()
  ftgmac100_init_all()
    ftgmac100_start_hw()
      phydev->interface

Today the same open path oopses earlier. It hits the unguarded check in
ftgmac100_reset_and_config_mac() that the commit message cites as
precedent:

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

That existing check looks like a latent NULL dereference itself, so it
doesn't prove the invariant. This patch adds a second dereference, which
would become the crash point if the earlier check were fixed or
reordered.

The later patch in this series, "net: ftgmac100: Require phy-mode for
AST2700", only changes ftgmac100_mii_probe(). That function isn't called
when an mdio child exists, so the gap seems to remain at the end of the
series.

Would it make sense to check phydev && phydev->interface ==
PHY_INTERFACE_MODE_RMII here? Another option is to have probe fail when
no PHY is attached on Aspeed MACs, which would also cover
ftgmac100_reset_and_config_mac(). Either way, the new comment above the
phydev declaration and the commit message would need updating to match.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261020-support_ast2700-v4-0-4d36b40939b3%40aspeedtech.com

[PATCH net-next v4 5/7] 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.

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

Re: [PATCH net-next v4 5/7] net: ftgmac100: Require phy-mode for AST2700

[netdev-bot+sashiko]

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

Critical: 0 · High: 2 (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] The new AST2700 "phy-mode is required" check in
  ftgmac100_mii_probe() (drivers/net/ethernet/faraday/ftgmac100.c) can…
- [High] The commit message says that on AST2700 the driver will "fail
  probe with an error message if [phy-mode] is not specified", and the…

Pre-existing issues:
- [High] NULL pointer dereference in ftgmac100_reset_and_config_mac() on
  ndo_open for an Aspeed MAC with no PHY attached.

--

net: ftgmac100: Require phy-mode for AST2700

This patch changes ftgmac100_mii_probe() so that on AST2700 a missing
phy-mode property returns -EINVAL. Previously the driver fell back to
RGMII. The goal is to make phy-mode a required property for AST2700,
because that SoC also supports RMII.

> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 9b7becb017262..5a578a62df673 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 AST2700 ever reach this check? Earlier in ftgmac100_mii_probe()
there is:

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

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

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

On AST2700, ftgmac100_probe()->ftgmac100_probe_dt()->ftgmac100_mii_probe()
returns -ENODEV before of_get_phy_mode() is called. That makes the new
branch look like dead code.

The phy-handle and fixed-link paths go through of_phy_get_and_connect().
That function already returns NULL when phy-mode is missing:

drivers/net/mdio/of_mdio.c:of_phy_get_and_connect() {
	...
	ret = of_get_phy_mode(np, &iface);
	if (ret)
		return NULL;
	...
}

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

  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

As far as I can tell, AST2700 never reaches the RGMII fallback, and this
patch does not change how AST2700 probes. The companion patch
"dt-bindings: net: ftgmac100: Add AST2700 compatible" says the binding
matches "the driver's probe-time requirement", so it depends on this
check working.

Could the check move somewhere AST2700 actually reaches, such as the
start of ftgmac100_probe_dt() or ftgmac100_probe()? The later patches in
the series, "net: ftgmac100: Add AST2700 upper DMA address support" and
"net: ftgmac100: Allow building on ARM64", don't seem to touch this logic.

[Severity: High]
Two other AST2700 probe paths in ftgmac100_probe_dt() still succeed
without phy-mode.

The first is use-ncsi:

	if (of_get_property(np, "use-ncsi", NULL))
		return ftgmac100_probe_ncsi(netdev, priv, pdev);

ftgmac100_probe_ncsi() never reads phy-mode and always connects with
RMII:

	err = phy_connect_direct(netdev, phydev, ftgmac100_adjust_link,
				 PHY_INTERFACE_MODE_RMII);

An AST2700 NC-SI node without phy-mode therefore probes successfully. A
node with phy-mode = "rgmii" passes binding validation, but the driver
ignores that value and runs in RMII.

The second is a node with an mdio child but no phy-handle, fixed-link or
use-ncsi. ftgmac100_probe_dt() skips the ftgmac100_mii_probe() block and
returns 0 without reading phy-mode:

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

	return 0;

Doesn't this conflict with "fail probe with an error message if it is
not specified" in the commit message? The binding also requires phy-mode
for every aspeed,ast2700-mac node.

Should the binding allow a use-ncsi exception, or should the driver check
phy-mode on all of these paths?

>  		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: High]
This is a pre-existing issue and was not introduced by this patch. Can
ftgmac100_open() dereference a NULL phydev on the mdio child path
described above?

Take an Aspeed node with an mdio child but no phy-handle, fixed-link or
use-ncsi. ftgmac100_probe_dt() returns 0 without connecting a PHY, so the
netdev is registered with netdev->phydev == NULL.

On ip link set up, ftgmac100_open()->ftgmac100_reset_and_config_mac()
does:

	/* Aspeed RMII needs SCU reset to clear status */
	if (priv->is_aspeed && priv->netdev->phydev->interface == PHY_INTERFACE_MODE_RMII) {

is_aspeed is true for every Aspeed MAC. ftgmac100_open() only checks
netdev->phydev later, just before phy_start().

This dereference came from commit af350ee72e9dda, before this series.
The series extends it to AST2700.

The earlier patch "net: ftgmac100: Enable AST2700 RMII support" adds
another unguarded read in ftgmac100_start_hw():

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

That patch's comment says ftgmac100_probe_dt() always attaches a
phy_device, but this path shows it doesn't. The phy-mode check added
here doesn't cover this path either.

Should these reads check for a NULL phydev, or should
ftgmac100_probe_dt() fail when no PHY is attached?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261020-support_ast2700-v4-0-4d36b40939b3%40aspeedtech.com

[PATCH net-next v4 6/7] 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.

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

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

[netdev-bot+sashiko]

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

Low priority / nit picks for your consideration:

- [Low] The descriptor struct comments are now wrong.

--

net: ftgmac100: Add AST2700 upper DMA address support

This patch programs the high RX and TX descriptor ring base registers on
AST2700. It also stores DMA address bits 34:32 in bits 18:16 of txdes2 and
rxdes2, and requests a 35-bit DMA mask. All of the extended address
handling is gated by a new FTGMAC100_QUIRK_DMA_35BIT match-data quirk.

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

[ ... ]

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

[Severity: Low]
This isn't a bug, but the descriptor struct comments just above this hunk
no longer match the new layout:

struct ftgmac100_txdes {
	__le32	txdes0; /* Control & status bits */
	__le32	txdes1; /* Irq, checksum and vlan control */
	__le32	txdes2; /* Reserved */
	__le32	txdes3; /* DMA buffer address */
} __attribute__ ((aligned(16)));

With FTGMAC100_QUIRK_DMA_35BIT set, ftgmac100_hard_start_xmit() now writes
buffer address bits 34:32 into txdes2 for both head and fragment
descriptors. ftgmac100_free_tx_packet() reads them back to rebuild the
unmap address, and txdes3 only holds the low 32 bits.

struct ftgmac100_rxdes has the same problem. rxdes3 is still described as
the full "DMA buffer address", and rxdes2 is only described as
"length/type on AST2500".

Could the comments for txdes2/txdes3 and rxdes2/rxdes3 be updated? It
might also help to note on the new GENMASK defines that they only apply
to the 35-bit (AST2700) layout. Otherwise a future descriptor writer or
reset path could treat txdes2 as reserved and drop the upper address
bits.

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

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261020-support_ast2700-v4-0-4d36b40939b3%40aspeedtech.com

[PATCH net-next v4 7/7] 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.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
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 v4 0/7] net: ftgmac100: Add AST2700 support

[Jakub Kicinski]

On Tue, 20 Oct 2026 19:34:11 +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.

## Form letter - net-next-closed

The merge window for Linux v7.4 has begun, and the net-next
tree is now closed for new drivers, features, code refactoring
and optimizations. We are currently accepting bug fixes only.

Please repost when net-next reopens. There is some uncertainty
about how long this merge window will last, so please check
https://netdev.bots.linux.dev/net-next.html
before reposting. net-next _may_ reopen on Nov 2nd.

See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
-- 
pw-bot: defer
pv-bot: closed