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-20T10:26:07Z

+			reset-assert-us = <10000>;
+			reset-deassert-us = <5000>;
+			adi,rx-internal-delay-ps = <2000>;
+			adi,tx-internal-delay-ps = <2000>;


when you say "documented" 2 ns delay applied by the PHY, where is this documented?

Will refer to the documents in commit messages.

I also removed these delays since they are also default in rgmii-id mode, as fifo-depth-bits for rmii.

The SC598 EZKIT connects emac0 to an ADIN1300, not a DP83867. Replace the stale PHY description and remove the DP83867-specific properties. Move the 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>

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>

Describe the ADIN1200 RMII PHY and MDIO bus connected to emac1. Keep the interface disabled because its board routing is shared with USB0. Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>

Move the EZKIT and EZLITE ADIN1300 reset GPIOs out of GPIO hogs 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 DP83867 and DP83848 descriptions with the corresponding ADIN PHY nodes and remove the unused DP83867 binding. 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>

sipraga · 2026-07-21T10:49:37Z

 		#address-cells = <1>;
 		#size-cells = <0>;
-		dp83867: ethernet-phy@0 {
+		adin1300: ethernet-phy@0 {


newline above node names (previous comment was a general one to be applied everywhere)

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

sipraga · 2026-07-21T10:53:04Z

+	pinctrl-0 = <&eth1_default>;
+
+	/* USB0 and ETH1 share board wiring; keep ETH1 disabled by default. */
+


I would drop the intermediate newline between comment and status, but you can ignore this comment if you're too lazy

sipraga · 2026-07-21T10:54:29Z

+		#size-cells = <0>;
+
+		adin1200: ethernet-phy@1 {
+			compatible = "ethernet-phy-ieee802.3-c22";


the generic compatible isn't needed for c22, and the maintainer actually prefers it be omitted, cf. https://lore.kernel.org/all/c7fe0371-76bc-4cb4-ade8-e22112c1475d@lunn.ch/

sipraga · 2026-07-21T10:56:33Z

-		dp83848: ethernet-phy@1 {
+
+		adin1200: ethernet-phy@1 {
+			compatible = "ethernet-phy-ieee802.3-c22";


sipraga · 2026-07-21T10:58:10Z

 		#address-cells = <1>;
 		#size-cells = <0>;
-		dp83867: ethernet-phy@0 {
+		adin1300: ethernet-phy@0 {


newline above

sipraga · 2026-07-21T10:59:06Z

 	snps,reset-active-low;
 	snps,reset-delays-us = <0 200 500>;
-	phy-handle = <&dp83867>;
+	phy-handle = <&adin1300>;


would be nice to keep order of properties consistent across device trees

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.

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 from 17ee202 to 9700331 Compare July 21, 2026 06:58

ozan956 added 6 commits July 21, 2026 09:05

arm64: dts: adi: describe SC598 EZKIT second Ethernet interface

8c5828b

Describe the ADIN1200 RMII PHY and MDIO bus connected to emac1. Keep the interface disabled because its board routing is shared with USB0. Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>

configs: sc5xx: enable ADIN PHY support

2efab48

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 9700331 to 2efab48 Compare July 21, 2026 07:08

sipraga reviewed Jul 21, 2026

View reviewed changes

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

		pinctrl-0 = <&eth1_default>;

		/* USB0 and ETH1 share board wiring; keep ETH1 disabled by default. */

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!

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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

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

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