Fix SC59x Ethernet PHY descriptions by ozan956 · Pull Request #3444 · analogdevicesinc/linux

ozan956 · 2026-07-19T17:50:25Z

Resolves #2883

ozan956 · 2026-07-19T17:53:38Z

I will test it on the hardware and mark it as ready for review afterwards, changes themselves are fairly straightforward.

ozan956 · 2026-07-19T20:54:07Z

I tested the changes on the U-Boot side first since it was easier to validate for me 😛.
The ADIN1300 PHY was detected successfully and eth. negotiation completed as expected.

I also found that the U-Boot ADIN PHY driver was outdated or old bit and did not register ADIN1200 support. I updated the driver to support things we need and align Linux driver.

The U-Boot pull request is available here:
analogdevicesinc/u-boot#130

github-actions · 2026-07-19T21:47:50Z

LLM review

This series migrates the SC594 and SC598 Ethernet device-tree nodes from stale TI DP83867/DP83848 descriptions to the correct ADI ADIN1300/ADIN1200 PHYs, moves reset management from GPIO hogs to reset-gpios, and enables CONFIG_ADIN_PHY in the affected defconfigs.

run: 29704096847

`b9db378` - arm: dts: adi: fix SC594 EZLITE Ethernet reset

Title mismatch: the commit modifies both sc594-som-ezlite.dts and sc594-som-ezkit.dts, but the title only names EZLITE. The body correctly describes both boards. The title should be updated to reflect both targets (e.g., "fix SC594 EZLITE and EZKIT Ethernet reset").

`0f904f5` / `3973355` - configure ADIN PHY parameters

Missing reset-assert-us: The ADIN1300 datasheet (Table 3, Hardware Reset Timing) specifies that RESET_N must be held low for a minimum of 10 μs (t1). The reset-gpios property was correctly added to the ADIN1300 PHY nodes, but reset-assert-us = <10000> was omitted. Without it, mdio_device_reset() in drivers/net/phy/mdio_device.c applies no post-assert delay. In practice, the I2C GPIO expander (MCP23017) adds sufficient inherent latency, but the spec should be met explicitly per Documentation/devicetree/bindings/net/ethernet-phy.yaml.

Affected: sc594-som-ezkit.dts, sc594-som-ezlite.dts, sc598-som-ezkit.dtsi, sc598-som-ezlite.dts.

CI warnings

All DT checker warnings in the CI log (simple_bus_reg, unit_address_vs_reg, unique_unit_address, avoid_unnecessary_addr_size) are pre-existing on lines not touched by this series. The checkpatch "Duplicate signature" warnings are false positives (same author and signer).

Verification data

ADIN1300 datasheet (adin1300.md) obtained from the docling mirror at https://raw.githubusercontent.com/analogdevicesinc/doctools/refs/heads/docling/media/en/technical-documentation/data-sheets/adin1300.md and used to verify: RESET_N active-low polarity (confirmed), 2 ns RGMII default delay (confirmed), and 10 μs minimum reset pulse (t1 parameter).
ADIN1200 datasheet (adin1200.md) used to verify RMII FIFO depth default = 8 bits (GE_RMII_CFG[6:4] = 001, confirmed).
Built SC594 (gcc_arm, sc594-som-ezkit_defconfig) and SC598 (gcc_aarch64, sc598-som-ezkit_defconfig) — DTBs compile without errors.
Ran dt_binding_check with adi,adin.yaml — no binding errors.

Suggested patches

Apply the suggested patches with:

cd path/to/repository
export GITHUB_TOKEN=ghp_***
apply-patches --repo=analogdevicesinc/linux 29704096847

Install instructions

The following one-liner installs the script if not present already:

curl -fSsL "https://raw.githubusercontent.com/analogdevicesinc/doctools/refs/heads/main/ci/scripts/apply-patches.sh" \
     -o ~/.local/bin/apply-patches.sh && \
  grep -q "/apply-patches.sh" ~/.bashrc || echo "source ~/.local/bin/apply-patches.sh" >> $_ ; . $_

More information at AI Usage.

sipraga · 2026-07-20T10:23:54Z

 		adin1200: ethernet-phy@1 {
 			compatible = "ethernet-phy-ieee802.3-c22";
 			reg = <1>;
+			adi,fifo-depth-bits = <8>;


I get that the default is 8, but what's the point of adding this explicitly? just curious.

I added it only to make the RMII configuration explicit, just to make it seen when someone just check the devicetree.

I think RMII is clear from the &emac1 :: phy-mode = "rmii" property.

I would advise not to explicitly fill this property in unless you can explain why a FIFO of 8 as opposed to, say, 16, is somehow preferable.

If omitted, it's clear to me as a reader that no particular thought went into the FIFO config.

If I see FIFO size = 8, I imagine there must be a specific reason.

Filling in what is already default is not a specific reason IMO, and potentially misleading as it suggests somebody thought "it really must be this value and not another".

Yes, for the same reason I also removed the internal delays since they were also the default value for "rgmii-id" mode.

sipraga · 2026-07-21T10:51:36Z

@@ -175,6 +168,9 @@
 		#size-cells = <0>;
 		adin1300: ethernet-phy@0 {


since it's not part of your PR diff, no need to fix the missing newline here IMO

Since I change mdio namings, maybe I can also do that?

go ahead if you're up for it, I just didn't want to ask you to fix yet more things ;D

sipraga · 2026-07-21T11:06:31Z


 &emac1 {
-	phy-handle = <&dp83848>;
+	phy-handle = <&adin1200>;


just my 2c here, but the commits are a bit confusingly structured IMO. in one commit you rename the label from dp83848 to adin1200, but you only update this phandle in the following commit. I don't really see the point. I would suggest to just have commits like:

"describe the EMAC1 PHY" (where you're adding new PHY)

"describe the EMAC0 PHY properly" (where you're changing from TI to ADI PHYs)
(on a per-board basis)

and in both cases, put the phy-handles and reset-gpios/timings in the relevant commit.

Yep, I fixed the commits, I was just doin the fixes and was waiting for approval of functionality to commit-cleanup 👷‍♂️

Move the ADIN1300 reset GPIO out of its GPIO hog so the PHY driver controls reset. The ADIN1300 data sheet, Table 3, requires RESET_N to be asserted for at least 10 us. It also specifies a 5 ms delay before MDIO registers are accessible after reset deassertion. Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>

The SC594 EZKIT connects emac0 to an ADIN1300 and emac1 to an ADIN1200. Replace the stale TI PHY descriptions with the corresponding ADI PHY nodes and remove the unused DP83867 binding. Move the ADIN1300 reset GPIO out of its GPIO hog so the PHY driver controls reset. The ADIN1300 data sheet, Table 3, requires RESET_N to be asserted for at least 10 us and specifies a 5 ms delay before MDIO registers are accessible after reset deassertion. Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>

Move the ADIN1300 reset GPIO out of its GPIO hog so the PHY driver controls reset. The ADIN1300 data sheet, Table 3, requires RESET_N to be asserted for at least 10 us. It also specifies a 5 ms delay before MDIO registers are accessible after reset deassertion. Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>

The SC598 EZKIT connects emac0 to an ADIN1300 and emac1 to an ADIN1200. Replace the stale TI PHY descriptions with the corresponding ADI PHY nodes and remove the unused DP83867 binding. Move the ADIN1300 reset GPIO out of its GPIO hog so the PHY driver controls reset. The ADIN1300 data sheet, Table 3, requires RESET_N to be asserted for at least 10 us and specifies a 5 ms delay before MDIO registers are accessible after reset deassertion. Keep emac1 disabled because its board routing is shared with USB0. Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>

Enable the ADIN PHY driver for the SC594 and SC598 board defconfigs. The board device trees use ADIN1300 and ADIN1200 PHYs; remove the obsolete TI PHY selections from the SC594 defconfigs. Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>

pamolloy · 2026-07-21T11:50:16Z

I've been waiting on fixing this because the delays are not trivial to calculate and it is a great candidate for hardware testing. We should really have a test in place that at least does an iperf3 to measure the impact on performance. But there are likely better tests, especially when modifying the delays.

sipraga · 2026-07-21T12:34:49Z

I usually just asked the hardware guy and he would tell me what the appropriate setting is to program into the PHY. But if trace lengths are equal and the MAC isn't introducing its own delay, then "rgmii-id" with default 2ns TX/RX delay is normally fine (tskewT is +-500 ps on both ends, so it should land within the required [1, 2.6 ns] window).

The (SC598 & SC58x) HRMs actually says the EZKITs expect the PHY to handle the delays:

pamolloy · 2026-07-21T13:16:25Z

Please confirm with David. Also explain what was tested and the results both before and after applying this change. Without hardware testing we need to make sure there isn't a regression.

ozan956 self-assigned this Jul 19, 2026

ozan956 requested a review from pamolloy July 19, 2026 17:50

ozan956 linked an issue Jul 19, 2026 that may be closed by this pull request

EV-SOMCRR-EZKIT devicetree is configured for TI rather than ADIN PHY #2883

Open

ozan956 force-pushed the fix-phy-sc5xx branch from bc81266 to 214c83d Compare July 19, 2026 17:53

ozan956 changed the title ~~Fix SC598 Ethernet PHY descriptions~~ Fix SC5xx Ethernet PHY descriptions Jul 19, 2026

ozan956 changed the title ~~Fix SC5xx Ethernet PHY descriptions~~ Fix SC59x Ethernet PHY descriptions Jul 19, 2026

ozan956 force-pushed the fix-phy-sc5xx branch from fd793da to 3973355 Compare July 19, 2026 20:49

ozan956 added this to ADSP Jul 19, 2026

ozan956 moved this to In Development in ADSP Jul 19, 2026

ozan956 marked this pull request as ready for review July 19, 2026 20:59

ozan956 added the llm review Request a review from a LLM Reviewer label Jul 19, 2026

ozan956 force-pushed the fix-phy-sc5xx branch from 3973355 to 17ee202 Compare July 19, 2026 22:10

ozan956 requested review from a team and removed request for pamolloy July 20, 2026 06:37

sipraga reviewed Jul 20, 2026

View reviewed changes

ozan956 force-pushed the fix-phy-sc5xx branch 2 times, most recently from 9700331 to 2efab48 Compare July 21, 2026 07:08

sipraga reviewed Jul 21, 2026

View reviewed changes

ozan956 force-pushed the fix-phy-sc5xx branch 2 times, most recently from 0d52983 to 795a2ed Compare July 21, 2026 11:24

ozan956 added 5 commits July 21, 2026 13:26

configs: sc5xx: enable ADIN PHY support

19b7214

Enable the ADIN PHY driver for the SC594 and SC598 board defconfigs. The board device trees use ADIN1300 and ADIN1200 PHYs; remove the obsolete TI PHY selections from the SC594 defconfigs. Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>

ozan956 force-pushed the fix-phy-sc5xx branch from 3979231 to 19b7214 Compare July 21, 2026 11:30

sipraga approved these changes Jul 21, 2026

View reviewed changes

		@@ -175,6 +168,9 @@
		#size-cells = <0>;
		adin1300: ethernet-phy@0 {

Uh oh!

Conversation

ozan956 commented Jul 19, 2026

Uh oh!

ozan956 commented Jul 19, 2026

Uh oh!

ozan956 commented Jul 19, 2026

Uh oh!

github-actions Bot commented Jul 19, 2026

LLM review

b9db378 - arm: dts: adi: fix SC594 EZLITE Ethernet reset

0f904f5 / 3973355 - configure ADIN PHY parameters

CI warnings

Verification data

Suggested patches

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pamolloy commented Jul 21, 2026

Uh oh!

sipraga commented Jul 21, 2026

Uh oh!

pamolloy commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

`b9db378` - arm: dts: adi: fix SC594 EZLITE Ethernet reset

`0f904f5` / `3973355` - configure ADIN PHY parameters