[PATCH net-next v8 0/2] net: phy: add X-Powers AC200/AC300 EPHY support

[PATCH net-next v8 0/2] net: phy: add X-Powers AC200/AC300 EPHY support

[James Hilliard]

This submission contains only the networking binding and PHY driver and
is intended for net-next. The AC200 I2C register provider was submitted
separately for the MFD tree:

  https://patch.msgid.link/20260811-submit-ac200-mfd-v7-0-8b06f552a4d7@gmail.com

Neither series has a compile-time dependency or required merge order.
Their shared design point is the DT representation: the MDIO PHY-package
node references the separately described AC200 I2C node for access to
package-control registers.

The AC200 and AC300 contain compatible Fast Ethernet link PHYs which
report the same Clause 22 identifier and use the same link-side register
layout. The link endpoint is inaccessible until package-specific control
registers have powered and configured it.

Both variants are modeled as standard Ethernet PHY packages with one
forced-ID link-PHY child. The package reg value is the link address.
AC300's control range is a fixed package-relative offset of 16 and is
accessed with the PHY package helpers. AC200 reaches its control registers
through a phandle to the companion IC's I2C register interface.

For AC200, the driver resolves the referenced I2C device, establishes a
managed device link, and takes the supplier device lock before checking
that the provider is fully bound and retrieving the regmap attached to
that device. It waits for transient lock holders instead of treating lock
contention as a missing dependency. The link then keeps the provider bound
until the PHY consumer has unbound. This uses the generic device and
regmap interfaces and supports multiple AC200 instances without syscon or
a private cross-subsystem API.

Fixed hardware uses an AC200- or AC300-specific package compatible.
Systems which can contain either package use the ACx00 package compatible
and one packed SID configuration field. Bits 3 through 0 carry the analog
calibration, bit 8 selects AC300, and bit 9 selects its low-calibration
tuning. The driver chooses the backend before acquiring any
backend-specific resource.

One xpowers-acx00 module binds the link child, joins the parent package and
runs the selected backend. It manages the package supply, MII or RMII mode,
vendor analog initialization and package state across suspend and resume.
The common link implementation preserves automatic MDI/MDI-X and standard
MAC-managed EEE advertisement while disabling only the vendor
PHY-autonomous Intelligent EEE mode.

The two patches add the AC200/AC300 PHY-package binding and combined PHY
driver. Board Device Trees and optional PHY features remain outside this
initial series.

The AC200 portions build on earlier work by Jernej Skrabec and Andre
Przywara:

  https://github.com/jernejsk/linux-1/commits/ac200-v4

Public AC200 and AC300 documentation is linked from:

  https://linux-sunxi.org/AC200

Validation completed for v8:

  - dt_binding_check for the PHY-package schema;
  - an x86_64 allmodconfig composite-object build with W=1;
  - a built-in AC300-only configuration with I2C and MFD_SYSCON disabled,
    built with W=1; and
  - strict checkpatch checks with no errors and only the generic new-file
    MAINTAINERS prompts.

The final AC200 v8 path was hardware-tested on an H616 board through a
clean Buildroot kernel, FIT, rootfs and SWU build with source overrides
disabled. The resulting SWU was installed on the board. The provider
reported revision 0x101 in package 1, the PHY resolved its regmap after
establishing the supplier device link, and RMII negotiated a 100 Mbps
full-duplex link. Eight MiB passed in each direction and 100 consecutive
pings completed without loss both before and after the rebind stress
tests.

An administrative down/up cycle changed the package regulator consumer
count from one to zero and back to one, then restored the same link mode.
Five AC200 supplier unbind/rebind cycles each unbound the PHY consumer,
recreated the device link and recovered the 100 Mbps full-duplex link and
ping traffic.

A separate 100-iteration test started each PHY bind while the AC200
supplier was still inside its 40 ms probe delay and holding the supplier
device lock. Every PHY bind waited for the supplier, recovered with the
correct PHY ID and device-link relationships, and the final link came up
at 100 Mbps full duplex without new warnings or probe errors.

The AC300 results carry forward from v5. On the connected H616 board the
driver selected AC300 without accessing the AC200 I2C path, negotiated a
100 Mbps full-duplex RMII link, passed eight MiB in each direction and
restored the regulator, package state and link across an administrative
down/up cycle.

Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
Changes in v8:
- replace the AC200 supplier trylock/defer path with a blocking device lock
- add Krzysztof's Reviewed-by tag to the PHY-package binding
- rebase onto current net-next
- Link to v7: https://patch.msgid.link/20260811-submit-acx00-of-dynamic-v1-v7-0-03f5b91775d5@gmail.com

Changes in v7:
- link to the separately submitted MFD v7 series:
  https://patch.msgid.link/20260811-submit-ac200-mfd-v7-0-8b06f552a4d7@gmail.com
- replace syscon publication and lookup with direct AC200 device/regmap lookup
- require the referenced AC200 provider to be fully bound under its device lock
- keep teardown ordered with the managed device link
- rebase onto current net-next
- Link to v6: https://patch.msgid.link/20260811-submit-acx00-of-dynamic-v1-v6-0-0229b99a32f6@gmail.com

Changes in v6:
- split the networking work from the independently mergeable MFD series
- add the posting link for the separately submitted MFD series
- remove the redundant explicit schema selector
- describe the AC200 phandle in hardware terms and state its purpose
- simplify the clock description and keep only the needed NVMEM dependency
- reduce the binding examples to one
- rebase onto current net-next
- Link to v5: https://patch.msgid.link/20260809-submit-acx00-of-dynamic-v1-v5-0-bfa1f8518b28@gmail.com

To: Andrew Lunn <andrew+netdev@lunn.ch>
To: "David S. Miller" <davem@davemloft.net>
To: Eric Dumazet <edumazet@google.com>
To: Jakub Kicinski <kuba@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: James Hilliard <james.hilliard1@gmail.com>
To: Andrew Lunn <andrew@lunn.ch>
To: Heiner Kallweit <hkallweit1@gmail.com>
To: Russell King <linux@armlinux.org.uk>
Cc: netdev@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
James Hilliard (2):
      dt-bindings: net: x-powers: add AC200/AC300 EPHY packages
      net: phy: add X-Powers AC200/AC300 EPHY driver

 .../bindings/net/x-powers,acx00-ephy-package.yaml  | 154 ++++++
 drivers/net/phy/Kconfig                            |  11 +
 drivers/net/phy/Makefile                           |   1 +
 drivers/net/phy/xpowers/Makefile                   |   3 +
 drivers/net/phy/xpowers/ac200.c                    | 308 ++++++++++++
 drivers/net/phy/xpowers/ac300.c                    | 387 +++++++++++++++
 drivers/net/phy/xpowers/acx00.c                    | 536 +++++++++++++++++++++
 drivers/net/phy/xpowers/acx00.h                    |  28 ++
 8 files changed, 1428 insertions(+)
---
base-commit: 3da8c3c8b8fa99505624b65ef590482f48e766b6
change-id: 20260802-submit-acx00-of-dynamic-v1-94a0dc15f282

Best regards,
--  
James Hilliard <james.hilliard1@gmail.com>

[PATCH net-next v8 1/2] dt-bindings: net: x-powers: add AC200/AC300 EPHY packages

[James Hilliard]

The AC200 and AC300 contain compatible Fast Ethernet link PHYs which
report the same Clause 22 identifier and use the same link-side register
layout. Their package control paths differ: AC200 control registers are
accessed through the I2C register interface of the companion IC, while
the AC300 control range is at a second Clause 22 address fixed at 16 plus
the link address.

Describe both variants as Ethernet PHY packages. The package reg value is
the link PHY base address and the child supplies the documented PHY ID,
because the identification registers cannot be read until package setup
has completed. The AC300 driver reaches its control range with the PHY
package offset helpers. The AC200 package instead references the
companion IC whose I2C interface provides access to the package controls.

Give fixed hardware variant-specific compatibles. Also provide an ACx00
compatible for systems which can contain either package, with one packed
NVMEM field containing calibration in bits 3 through 0, the AC300 selector
in bit 8 and its low-calibration selector in bit 9. Such a description
contains the potential resources for both backends; the driver consumes
only those selected by the configuration field.

Describe the package clock, supply and optional xMII receive-clock
inversion needed by some integrations. A fixed AC200 package may omit the
NVMEM field and use its internal calibration copy. Limit AC300-capable link
addresses to 0 through 7 so the control address remains in the documented
range 16 through 23.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
 .../bindings/net/x-powers,acx00-ephy-package.yaml  | 154 +++++++++++++++++++++
 1 file changed, 154 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/x-powers,acx00-ephy-package.yaml b/Documentation/devicetree/bindings/net/x-powers,acx00-ephy-package.yaml
new file mode 100644
index 000000000000..afe57bd163dd
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/x-powers,acx00-ephy-package.yaml
@@ -0,0 +1,154 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/x-powers,acx00-ephy-package.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: X-Powers AC200 and AC300 Ethernet PHY packages
+
+maintainers:
+  - James Hilliard <james.hilliard1@gmail.com>
+
+description:
+  The AC200 and AC300 contain compatible Fast Ethernet PHY blocks which report
+  the same Clause 22 PHY ID and use the same link-side register layout. The
+  package control paths differ. AC200 package control registers are accessed
+  through the companion IC's I2C register interface, while AC300 control
+  registers occupy a second Clause 22 address fixed at 16 plus the link PHY
+  address.
+
+  Systems with a known package use the corresponding AC200 or AC300 compatible.
+  Systems which can contain either package use the ACx00 compatible and a
+  packed NVMEM configuration field to select the backend at runtime.
+
+  The link PHY cannot report its ID until the package control sequence has
+  completed, so its child node supplies the documented PHY ID. The package and
+  child use the same base address. The AC200 driver programs that address into
+  the package, while the AC300 driver derives its control address from it.
+
+properties:
+  compatible:
+    enum:
+      - x-powers,ac200-ephy-package
+      - x-powers,ac300-ephy-package
+      - x-powers,acx00-ephy-package
+
+  reg:
+    maximum: 31
+
+  clocks:
+    maxItems: 1
+    description: AC300 input clock, running at 24, 25, or 27 MHz
+
+  vcc-supply:
+    description:
+      3.3 V package supply. This is connected to EPHY_VCC on AC200 and VCC1
+      on AC300.
+
+  nvmem-cells:
+    maxItems: 1
+    description:
+      Packed EPHY configuration field. Bits 3 through 0 contain the analog
+      calibration code, bits 7 through 4 are reserved, bit 8 selects AC300
+      when set and AC200 when clear, and bit 9 requests the AC300
+      low-calibration tuning sequence. A fixed AC200 package may provide only
+      the low calibration bits or omit the cell and use its internal eFuse
+      copy instead.
+
+  nvmem-cell-names:
+    const: configuration
+
+  x-powers,ac200:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Phandle to the AC200 companion IC providing I2C access to the package
+      control registers used to power and configure the Ethernet PHY.
+
+  x-powers,xmii-rx-clock-inverted:
+    type: boolean
+    description:
+      Invert the xMII receive clock inside the PHY. This is required by some
+      SoC integrations, including the Allwinner H6 AC200 connection.
+
+patternProperties:
+  ^ethernet-phy@[a-f0-9]+$:
+    $ref: ethernet-phy.yaml#
+    properties:
+      compatible:
+        const: ethernet-phy-id0044.1400
+    required:
+      - compatible
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - vcc-supply
+
+dependentRequired:
+  nvmem-cells: [ nvmem-cell-names ]
+
+allOf:
+  - $ref: ethernet-phy-package.yaml#
+  - if:
+      properties:
+        compatible:
+          const: x-powers,ac200-ephy-package
+    then:
+      properties:
+        clocks: false
+      required:
+        - x-powers,ac200
+  - if:
+      properties:
+        compatible:
+          const: x-powers,ac300-ephy-package
+    then:
+      properties:
+        reg:
+          maximum: 7
+        x-powers,ac200: false
+      required:
+        - clocks
+        - nvmem-cells
+        - nvmem-cell-names
+  - if:
+      properties:
+        compatible:
+          const: x-powers,acx00-ephy-package
+    then:
+      properties:
+        reg:
+          maximum: 7
+      required:
+        - clocks
+        - nvmem-cells
+        - nvmem-cell-names
+        - x-powers,ac200
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    mdio {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ethernet-phy-package@0 {
+            compatible = "x-powers,acx00-ephy-package";
+            reg = <0>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+            clocks = <&pwm 5>;
+            vcc-supply = <&reg_3v3>;
+            nvmem-cells = <&ephy_configuration>;
+            nvmem-cell-names = "configuration";
+            x-powers,ac200 = <&ac200>;
+
+            ethernet-phy@0 {
+                compatible = "ethernet-phy-id0044.1400";
+                reg = <0>;
+            };
+        };
+    };
+
+...

-- 
2.53.0

[PATCH net-next v8 2/2] net: phy: add X-Powers AC200/AC300 EPHY driver

[James Hilliard]

The AC200 and AC300 contain compatible Fast Ethernet link PHYs, but the
link endpoint is inaccessible until package-specific control registers
have powered and configured it.

Add one PHY driver which binds the link child and joins its parent
Ethernet PHY package. Fixed package compatibles select the corresponding
backend. The generic ACx00 compatible reads one packed configuration field
and selects the backend before touching backend-specific resources. The
AC300 path therefore does not resolve or access the candidate AC200 I2C
device.

AC200 locates the I2C device referenced by the package and establishes a
managed device link. It verifies that the provider is fully bound under
the supplier device lock before retrieving the regmap attached to the I2C
device. The link keeps the supplier bound until the PHY consumer has
unbound. Firmware must make the selected AC200 provider path available
before the PHY probes.

AC300 uses the PHY package helpers to access the control range at base
address plus 16. Keeping both backends in the same module lets the common
link implementation own the complete PHY without registering artificial
control devices.

Obtain and manage the selected package supply, calibration and input
clock, validate or program the link address, and apply the required reset,
clock, I/O and shutdown sequences. Start in the hardware-default MII mode
so the forced PHY device can probe before a MAC attaches, then apply the
MAC-provided MII or RMII mode before the normal PHY soft reset.

Apply the common vendor analog initialization, optional AC300
low-calibration tuning and board-selected receive-clock inversion. Keep
MDI/MDI-X in automatic mode. Preserve standard MAC-managed EEE support
while disabling the vendor PHY-autonomous Intelligent EEE mode.

Power down the package control block and its supply during PHY suspend,
then restore them and the vendor configuration during resume. If
shutdown fails, attempt to recover the running PHY before returning the
original error.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/net/phy/Kconfig          |  11 +
 drivers/net/phy/Makefile         |   1 +
 drivers/net/phy/xpowers/Makefile |   3 +
 drivers/net/phy/xpowers/ac200.c  | 308 ++++++++++++++++++++++
 drivers/net/phy/xpowers/ac300.c  | 387 ++++++++++++++++++++++++++++
 drivers/net/phy/xpowers/acx00.c  | 536 +++++++++++++++++++++++++++++++++++++++
 drivers/net/phy/xpowers/acx00.h  |  28 ++
 7 files changed, 1274 insertions(+)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index b4ef927fd4a6..d3835597e379 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -484,6 +484,17 @@ config VITESSE_PHY
 	help
 	  Currently supports the vsc8244
 
+config XPOWERS_ACX00_PHY
+	tristate "X-Powers AC200/AC300 Ethernet PHY"
+	depends on COMMON_CLK && NVMEM && OF_MDIO && REGULATOR
+	depends on I2C || !I2C
+	select PHY_PACKAGE
+	help
+	  Enable the Fast Ethernet PHY driver shared by the X-Powers AC200
+	  and AC300 companion ICs. The driver configures their common MDIO PHY
+	  registers and the package-specific AC200 I2C or AC300 MDIO control
+	  registers.
+
 config XILINX_GMII2RGMII
 	tristate "Xilinx GMII2RGMII converter driver"
 	help
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 25c4a3c2429f..49575fa18522 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -101,4 +101,5 @@ obj-$(CONFIG_SMSC_PHY)		+= smsc.o
 obj-$(CONFIG_STE10XP)		+= ste10Xp.o
 obj-$(CONFIG_TERANETICS_PHY)	+= teranetics.o
 obj-$(CONFIG_VITESSE_PHY)	+= vitesse.o
+obj-$(CONFIG_XPOWERS_ACX00_PHY)	+= xpowers/
 obj-$(CONFIG_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o
diff --git a/drivers/net/phy/xpowers/Makefile b/drivers/net/phy/xpowers/Makefile
new file mode 100644
index 000000000000..ae977a795832
--- /dev/null
+++ b/drivers/net/phy/xpowers/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+xpowers-acx00-y := acx00.o ac200.o ac300.o
+obj-$(CONFIG_XPOWERS_ACX00_PHY) += xpowers-acx00.o
diff --git a/drivers/net/phy/xpowers/ac200.c b/drivers/net/phy/xpowers/ac200.c
new file mode 100644
index 000000000000..17011b5f5e21
--- /dev/null
+++ b/drivers/net/phy/xpowers/ac200.c
@@ -0,0 +1,308 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * X-Powers AC200 Ethernet PHY package backend
+ *
+ * Copyright (c) 2022 Arm Ltd. (Andre Przywara <andre.przywara@arm.com>)
+ * Copyright (C) 2026 James Hilliard <james.hilliard1@gmail.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/i2c.h>
+#include <linux/of.h>
+#include <linux/phy.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+#include "acx00.h"
+
+#define AC200_EPHY_BPS_EFFUSE_OFFSET	3
+#define AC200_EPHY_CLK_RATE_24_MHZ	24000000
+#define AC200_EPHY_CLK_RATE_27_MHZ	27000000
+
+#define AC200_SYS_EPHY_CTL0_REG			0x0014
+#define AC200_EPHY_RESET_DEASSERT		BIT(0)
+#define AC200_EPHY_SYSCLK_ENABLE			BIT(1)
+
+#define AC200_SYS_EPHY_CTL1_REG			0x0016
+#define AC200_EPHY_MII_IO_ENABLE			BIT(0)
+
+/* AC200-internal copy of the Ethernet PHY calibration eFuse. */
+#define AC200_EFUSE_EPHY_REG			0x8004
+
+#define AC200_EPHY_CTL_REG			0x6000
+#define AC200_EPHY_SHUTDOWN			BIT(0)
+#define AC200_EPHY_CLK_SEL_24_MHZ		BIT(2)
+#define AC200_EPHY_PHY_ADDR_MASK			GENMASK(8, 4)
+#define AC200_EPHY_RMII_SEL			BIT(11)
+#define AC200_EPHY_BPS_EFFUSE_MASK		GENMASK(15, 12)
+
+struct ac200_ephy_ctl {
+	struct acx00_ephy_control control;
+	struct regmap *regmap;
+	struct regulator *vcc;
+	struct device *dev;
+	u16 ephy_ctl;
+	unsigned int phy_addr;
+	phy_interface_t interface;
+	bool supply_enabled;
+	bool powered;
+};
+
+static u16 ac200_ephy_ctl_config(const struct ac200_ephy_ctl *priv)
+{
+	return priv->ephy_ctl |
+		(priv->interface == PHY_INTERFACE_MODE_RMII ?
+		 AC200_EPHY_RMII_SEL : 0) |
+		FIELD_PREP(AC200_EPHY_PHY_ADDR_MASK, priv->phy_addr);
+}
+
+static int ac200_ephy_ctl_write(struct ac200_ephy_ctl *priv,
+				unsigned int reg, u16 value)
+{
+	int ret;
+
+	ret = regmap_write(priv->regmap, reg, value);
+	if (ret)
+		dev_err(priv->dev, "failed to write register %#x: %pe\n",
+			reg, ERR_PTR(ret));
+
+	return ret;
+}
+
+static int ac200_ephy_ctl_disable(struct ac200_ephy_ctl *priv)
+{
+	int err;
+	int ret = 0;
+
+	if (priv->powered) {
+		ret = ac200_ephy_ctl_write(priv, AC200_EPHY_CTL_REG,
+					   ac200_ephy_ctl_config(priv) |
+					   AC200_EPHY_SHUTDOWN);
+		err = ac200_ephy_ctl_write(priv, AC200_SYS_EPHY_CTL1_REG, 0);
+		if (!ret)
+			ret = err;
+		err = ac200_ephy_ctl_write(priv, AC200_SYS_EPHY_CTL0_REG, 0);
+		if (!ret)
+			ret = err;
+
+		priv->powered = false;
+	}
+
+	if (priv->supply_enabled) {
+		err = regulator_disable(priv->vcc);
+		if (!err)
+			priv->supply_enabled = false;
+		else if (!ret)
+			ret = err;
+	}
+
+	return ret;
+}
+
+static int ac200_ephy_ctl_power_off(struct acx00_ephy_control *control)
+{
+	struct ac200_ephy_ctl *priv =
+		container_of(control, struct ac200_ephy_ctl, control);
+
+	return ac200_ephy_ctl_disable(priv);
+}
+
+static int
+ac200_ephy_ctl_set_interface(struct acx00_ephy_control *control,
+			     phy_interface_t interface)
+{
+	struct ac200_ephy_ctl *priv =
+		container_of(control, struct ac200_ephy_ctl, control);
+	int ret = 0;
+
+	if (interface != PHY_INTERFACE_MODE_MII &&
+	    interface != PHY_INTERFACE_MODE_RMII)
+		return -EINVAL;
+
+	if (priv->interface == interface)
+		return 0;
+
+	if (priv->powered)
+		ret = regmap_update_bits(priv->regmap, AC200_EPHY_CTL_REG,
+					 AC200_EPHY_RMII_SEL,
+					 interface == PHY_INTERFACE_MODE_RMII ?
+					 AC200_EPHY_RMII_SEL : 0);
+	if (!ret)
+		priv->interface = interface;
+
+	return ret;
+}
+
+static int ac200_ephy_ctl_power_on(struct acx00_ephy_control *control,
+				   unsigned int phy_addr)
+{
+	struct ac200_ephy_ctl *priv =
+		container_of(control, struct ac200_ephy_ctl, control);
+	u16 ephy_ctl;
+	int ret;
+
+	if (phy_addr > FIELD_MAX(AC200_EPHY_PHY_ADDR_MASK))
+		return -EINVAL;
+
+	if (priv->powered && priv->phy_addr == phy_addr)
+		return 0;
+
+	if (priv->powered) {
+		ret = ac200_ephy_ctl_disable(priv);
+		if (ret)
+			return ret;
+	}
+	priv->phy_addr = phy_addr;
+
+	if (!priv->supply_enabled) {
+		ret = regulator_enable(priv->vcc);
+		if (ret)
+			return ret;
+		priv->supply_enabled = true;
+	}
+
+	ephy_ctl = ac200_ephy_ctl_config(priv);
+
+	/* Start from a disabled state before applying the configuration. */
+	ret = ac200_ephy_ctl_write(priv, AC200_SYS_EPHY_CTL0_REG, 0);
+	if (ret)
+		goto err_disable;
+
+	ret = ac200_ephy_ctl_write(priv, AC200_SYS_EPHY_CTL1_REG,
+				   AC200_EPHY_MII_IO_ENABLE);
+	if (ret)
+		goto err_disable;
+
+	ret = ac200_ephy_ctl_write(priv, AC200_EPHY_CTL_REG,
+				   ephy_ctl | AC200_EPHY_SHUTDOWN);
+	if (ret)
+		goto err_disable;
+
+	ret = ac200_ephy_ctl_write(priv, AC200_SYS_EPHY_CTL0_REG,
+				   AC200_EPHY_RESET_DEASSERT |
+				   AC200_EPHY_SYSCLK_ENABLE);
+	if (ret)
+		goto err_disable;
+
+	ret = ac200_ephy_ctl_write(priv, AC200_EPHY_CTL_REG, ephy_ctl);
+	if (ret)
+		goto err_disable;
+
+	priv->powered = true;
+	return 0;
+
+err_disable:
+	/* Attempt every step of the shutdown sequence after a partial start. */
+	priv->powered = true;
+	ac200_ephy_ctl_disable(priv);
+
+	return ret;
+}
+
+struct acx00_ephy_control *
+ac200_ephy_ctl_create(struct phy_device *phydev,
+		      struct device_node *package_node,
+		      bool has_calibration, u8 calibration)
+{
+	struct device *dev = &phydev->mdio.dev;
+	unsigned int internal_calibration;
+	struct device_node *ac200_node;
+	struct ac200_ephy_ctl *priv;
+	struct i2c_client *client;
+	unsigned long clk_rate;
+	u8 bps_effuse_code;
+	struct clk *clk;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return ERR_PTR(-ENOMEM);
+	priv->dev = dev;
+	priv->vcc = devm_of_regulator_get(dev, package_node, "vcc");
+	if (IS_ERR(priv->vcc))
+		return ERR_PTR(dev_err_probe(dev, PTR_ERR(priv->vcc),
+					     "failed to get package supply\n"));
+
+	ac200_node = of_parse_phandle(package_node, "x-powers,ac200", 0);
+	if (!ac200_node)
+		return ERR_PTR(dev_err_probe(dev, -EINVAL,
+					     "missing x-powers,ac200 reference\n"));
+	client = of_find_i2c_device_by_node(ac200_node);
+	of_node_put(ac200_node);
+	if (!client) {
+		ret = IS_ENABLED(CONFIG_I2C) ? -EPROBE_DEFER : -ENODEV;
+		return ERR_PTR(dev_err_probe(dev, ret,
+					     "AC200 device is not registered\n"));
+	}
+
+	if (!device_link_add(dev, &client->dev,
+			     DL_FLAG_AUTOREMOVE_CONSUMER)) {
+		ret = dev_err_probe(dev, -EINVAL,
+				    "failed to link AC200 device\n");
+		goto out_put_client;
+	}
+
+	device_lock(&client->dev);
+	if (device_is_bound(&client->dev))
+		priv->regmap = dev_get_regmap(&client->dev, NULL);
+	device_unlock(&client->dev);
+	if (!priv->regmap) {
+		ret = dev_err_probe(dev, -EPROBE_DEFER,
+				    "AC200 driver is not ready\n");
+		goto out_put_client;
+	}
+
+	if (!has_calibration) {
+		ret = regmap_read(priv->regmap, AC200_EFUSE_EPHY_REG,
+				  &internal_calibration);
+		if (ret)
+			goto out_error;
+		calibration = internal_calibration;
+	}
+
+	/* The vendor driver supplies no transfer function beyond this offset. */
+	bps_effuse_code = (calibration + AC200_EPHY_BPS_EFFUSE_OFFSET) &
+			   FIELD_MAX(AC200_EPHY_BPS_EFFUSE_MASK);
+	priv->ephy_ctl =
+		FIELD_PREP(AC200_EPHY_BPS_EFFUSE_MASK, bps_effuse_code);
+	/* EPHY_MODE and BIST_CLK_EN stay clear for normal operation. */
+
+	clk = clk_get(&client->dev, NULL);
+	if (IS_ERR(clk)) {
+		ret = PTR_ERR(clk);
+		goto out_error;
+	}
+
+	clk_rate = clk_get_rate(clk);
+	clk_put(clk);
+
+	switch (clk_rate) {
+	case AC200_EPHY_CLK_RATE_24_MHZ:
+		priv->ephy_ctl |= AC200_EPHY_CLK_SEL_24_MHZ;
+		break;
+	case AC200_EPHY_CLK_RATE_27_MHZ:
+		break;
+	default:
+		ret = dev_err_probe(dev, -EINVAL,
+				    "unsupported AC200 clock rate %lu Hz\n",
+				    clk_rate);
+		goto out_put_client;
+	}
+
+	priv->control.power_on = ac200_ephy_ctl_power_on;
+	priv->control.power_off = ac200_ephy_ctl_power_off;
+	priv->control.set_interface = ac200_ephy_ctl_set_interface;
+	/* MII is the reset default used until the MAC supplies its interface. */
+	priv->interface = PHY_INTERFACE_MODE_MII;
+	put_device(&client->dev);
+
+	return &priv->control;
+
+out_error:
+	ret = dev_err_probe(dev, ret, "failed to initialize AC200 control\n");
+out_put_client:
+	put_device(&client->dev);
+	return ERR_PTR(ret);
+}
diff --git a/drivers/net/phy/xpowers/ac300.c b/drivers/net/phy/xpowers/ac300.c
new file mode 100644
index 000000000000..aa560432a14c
--- /dev/null
+++ b/drivers/net/phy/xpowers/ac300.c
@@ -0,0 +1,387 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * X-Powers AC300 Ethernet PHY package backend
+ *
+ * Copyright (C) 2026 James Hilliard <james.hilliard1@gmail.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/phy.h>
+#include <linux/regulator/consumer.h>
+
+#include "../phylib.h"
+#include "acx00.h"
+
+#define AC300_EPHY_BGS_EFFUSE_OFFSET	3
+#define AC300_EPHY_CLK_RATE_24_MHZ	24000000
+#define AC300_EPHY_CLK_RATE_25_MHZ	25000000
+#define AC300_EPHY_CLK_RATE_27_MHZ	27000000
+#define AC300_SYS_CONTROL_REG			0x00
+#define AC300_PACKAGE_STATUS_MASK		GENMASK(11, 8)
+#define AC300_EPHY_CLK_SEL_MASK			GENMASK(7, 6)
+#define AC300_EPHY_CLK_SEL_25_MHZ		FIELD_PREP(AC300_EPHY_CLK_SEL_MASK, 0)
+#define AC300_EPHY_CLK_SEL_27_MHZ		FIELD_PREP(AC300_EPHY_CLK_SEL_MASK, 1)
+#define AC300_EPHY_CLK_SEL_24_MHZ		FIELD_PREP(AC300_EPHY_CLK_SEL_MASK, 2)
+#define AC300_EFUSE_CLK_ENABLE			BIT(5)
+#define AC300_EPHY_REG_CLK_ENABLE		BIT(4)
+#define AC300_CLKIN_GATING_ENABLE		BIT(2)
+#define AC300_EPHY_RESET_DEASSERT		BIT(1)
+#define AC300_CHIP_RESET_DEASSERT		BIT(0)
+
+#define AC300_PACKAGE_POR_INTERNAL_DLDO		BIT(3)
+#define AC300_PACKAGE_PHY_ADDR_MASK		GENMASK(2, 0)
+
+#define AC300_SYS_BIAS1_REG			0x02
+#define AC300_INTERNAL_DLDO_ENABLE		BIT(15)
+
+#define AC300_SYS_IO_REG			0x05
+#define AC300_MDIO_DRV_MASK			GENMASK(15, 14)
+#define AC300_MII_DRV_MASK			GENMASK(11, 10)
+#define AC300_IO_DRV_LEVEL_2			2
+#define AC300_CLKIN_PAD_ENABLE			BIT(4)
+#define AC300_EPHY_MII_IO_ENABLE			BIT(0)
+
+#define AC300_EPHY_CONFIG_REG			0x06
+#define AC300_EPHY_BGS_EFFUSE_MASK		GENMASK(15, 12)
+#define AC300_EPHY_RMII_SEL			BIT(11)
+#define AC300_EPHY_SHUTDOWN			BIT(0)
+
+#define AC300_SYS_CONTROL_ENABLE_BITS \
+	(AC300_EFUSE_CLK_ENABLE | AC300_EPHY_REG_CLK_ENABLE | \
+	 AC300_CLKIN_GATING_ENABLE | AC300_EPHY_RESET_DEASSERT | \
+	 AC300_CHIP_RESET_DEASSERT)
+
+#define AC300_SYS_IO_VALUE \
+	(FIELD_PREP(AC300_MDIO_DRV_MASK, AC300_IO_DRV_LEVEL_2) | \
+	 FIELD_PREP(AC300_MII_DRV_MASK, AC300_IO_DRV_LEVEL_2) | \
+	 AC300_CLKIN_PAD_ENABLE | AC300_EPHY_MII_IO_ENABLE)
+
+struct ac300_ephy_ctl {
+	struct acx00_ephy_control control;
+	struct phy_device *phydev;
+	struct regulator *vcc;
+	struct clk *clk;
+	u16 sys_control;
+	u16 ephy_config;
+	phy_interface_t interface;
+	bool package_known;
+	bool internal_dldo;
+	bool supply_enabled;
+	bool powered;
+};
+
+static unsigned int
+ac300_ephy_ctl_link_addr(const struct ac300_ephy_ctl *priv)
+{
+	return priv->phydev->mdio.addr;
+}
+
+static int ac300_ephy_ctl_read(struct ac300_ephy_ctl *priv, u32 regnum)
+{
+	int ret;
+
+	phy_lock_mdio_bus(priv->phydev);
+	ret = __phy_package_read(priv->phydev,
+				 AC300_EPHY_CONTROL_ADDR_OFFSET, regnum);
+	phy_unlock_mdio_bus(priv->phydev);
+
+	return ret;
+}
+
+static int ac300_ephy_ctl_write(struct ac300_ephy_ctl *priv, u32 regnum,
+				u16 val)
+{
+	int ret;
+
+	phy_lock_mdio_bus(priv->phydev);
+	ret = __phy_package_write(priv->phydev,
+				  AC300_EPHY_CONTROL_ADDR_OFFSET, regnum, val);
+	phy_unlock_mdio_bus(priv->phydev);
+
+	return ret;
+}
+
+static int ac300_ephy_ctl_modify(struct ac300_ephy_ctl *priv, u32 regnum,
+				 u16 mask, u16 set)
+{
+	int ret;
+
+	phy_lock_mdio_bus(priv->phydev);
+	ret = __phy_package_read(priv->phydev,
+				 AC300_EPHY_CONTROL_ADDR_OFFSET, regnum);
+	if (ret >= 0) {
+		u16 val = (ret & ~mask) | (set & mask);
+
+		ret = val == ret ? 0 :
+			__phy_package_write(priv->phydev,
+					    AC300_EPHY_CONTROL_ADDR_OFFSET,
+					    regnum, val);
+	}
+	phy_unlock_mdio_bus(priv->phydev);
+
+	return ret;
+}
+
+static u16 ac300_ephy_ctl_config(const struct ac300_ephy_ctl *priv)
+{
+	return priv->ephy_config |
+		(priv->interface == PHY_INTERFACE_MODE_RMII ?
+		 AC300_EPHY_RMII_SEL : 0);
+}
+
+static u16 ac300_ephy_ctl_reset_value(const struct ac300_ephy_ctl *priv)
+{
+	/*
+	 * A chip reset restores DLDOEN to one. Until PKG_STATUS has identified
+	 * the supply arrangement, preserve any external-VDD setup left by the
+	 * bootloader by asserting only the EPHY reset.
+	 */
+	return !priv->package_known || !priv->internal_dldo ?
+		AC300_CHIP_RESET_DEASSERT : 0;
+}
+
+static int ac300_ephy_ctl_disable(struct ac300_ephy_ctl *priv)
+{
+	int err;
+	int ret = 0;
+
+	if (priv->powered) {
+		ret = ac300_ephy_ctl_write(priv, AC300_EPHY_CONFIG_REG,
+					   ac300_ephy_ctl_config(priv) |
+					   AC300_EPHY_SHUTDOWN);
+		err = ac300_ephy_ctl_write(priv, AC300_SYS_IO_REG, 0);
+		if (!ret)
+			ret = err;
+		err = ac300_ephy_ctl_write(priv, AC300_SYS_CONTROL_REG,
+					   ac300_ephy_ctl_reset_value(priv));
+		if (!ret)
+			ret = err;
+
+		clk_disable_unprepare(priv->clk);
+		priv->powered = false;
+	}
+
+	if (priv->supply_enabled) {
+		err = regulator_disable(priv->vcc);
+		if (!err)
+			priv->supply_enabled = false;
+		else if (!ret)
+			ret = err;
+	}
+
+	return ret;
+}
+
+static int ac300_ephy_ctl_power_off(struct acx00_ephy_control *control)
+{
+	struct ac300_ephy_ctl *priv =
+		container_of(control, struct ac300_ephy_ctl, control);
+
+	return ac300_ephy_ctl_disable(priv);
+}
+
+static int
+ac300_ephy_ctl_set_interface(struct acx00_ephy_control *control,
+			     phy_interface_t interface)
+{
+	struct ac300_ephy_ctl *priv =
+		container_of(control, struct ac300_ephy_ctl, control);
+	int ret = 0;
+
+	if (interface != PHY_INTERFACE_MODE_MII &&
+	    interface != PHY_INTERFACE_MODE_RMII)
+		return -EINVAL;
+
+	if (priv->interface == interface)
+		return 0;
+
+	if (priv->powered)
+		ret = ac300_ephy_ctl_modify(priv, AC300_EPHY_CONFIG_REG,
+					    AC300_EPHY_RMII_SEL,
+					    interface == PHY_INTERFACE_MODE_RMII ?
+					    AC300_EPHY_RMII_SEL : 0);
+	if (!ret)
+		priv->interface = interface;
+
+	return ret;
+}
+
+static int ac300_ephy_ctl_power_on(struct acx00_ephy_control *control,
+				   unsigned int phy_addr)
+{
+	struct ac300_ephy_ctl *priv =
+		container_of(control, struct ac300_ephy_ctl, control);
+	u8 package_status;
+	u16 reset_value;
+	int sys_control;
+	int ret;
+
+	if (phy_addr != ac300_ephy_ctl_link_addr(priv))
+		return -EINVAL;
+
+	if (priv->powered)
+		return 0;
+
+	if (!priv->supply_enabled) {
+		ret = regulator_enable(priv->vcc);
+		if (ret)
+			return ret;
+		priv->supply_enabled = true;
+
+		/* Wait for the power-on reset interval specified by the manual. */
+		fsleep(10000);
+	}
+
+	ret = clk_prepare_enable(priv->clk);
+	if (ret)
+		goto err_power_off;
+	priv->powered = true;
+
+	/* Keep the external-supply configuration across chip resets. */
+	reset_value = ac300_ephy_ctl_reset_value(priv);
+	ret = ac300_ephy_ctl_write(priv, AC300_SYS_CONTROL_REG, reset_value);
+	if (ret)
+		goto err_power_off;
+
+	/* The manual requires both resets to be released before the clocks. */
+	ret = ac300_ephy_ctl_write(priv, AC300_SYS_CONTROL_REG,
+				   AC300_EPHY_RESET_DEASSERT |
+				  AC300_CHIP_RESET_DEASSERT);
+	if (ret)
+		goto err_power_off;
+
+	/* Retain the vendor clock-enable defaults, including the eFuse clock. */
+	ret = ac300_ephy_ctl_write(priv, AC300_SYS_CONTROL_REG,
+				   priv->sys_control);
+	if (ret)
+		goto err_power_off;
+
+	sys_control = ac300_ephy_ctl_read(priv, AC300_SYS_CONTROL_REG);
+	if (sys_control < 0) {
+		ret = sys_control;
+		goto err_power_off;
+	}
+
+	package_status = FIELD_GET(AC300_PACKAGE_STATUS_MASK, sys_control);
+	if ((~package_status & AC300_PACKAGE_PHY_ADDR_MASK) !=
+	    ac300_ephy_ctl_link_addr(priv)) {
+		ret = -EINVAL;
+		goto err_power_off;
+	}
+
+	priv->internal_dldo = package_status & AC300_PACKAGE_POR_INTERNAL_DLDO;
+	priv->package_known = true;
+	ret = ac300_ephy_ctl_modify(priv, AC300_SYS_BIAS1_REG,
+				    AC300_INTERNAL_DLDO_ENABLE,
+				   priv->internal_dldo ?
+				   AC300_INTERNAL_DLDO_ENABLE : 0);
+	if (ret)
+		goto err_power_off;
+
+	/* Keep the documented default drive level and leave the IRQ disabled. */
+	ret = ac300_ephy_ctl_write(priv, AC300_SYS_IO_REG,
+				   AC300_SYS_IO_VALUE);
+	if (ret)
+		goto err_power_off;
+
+	fsleep(10000);
+
+	ret = ac300_ephy_ctl_write(priv, AC300_EPHY_CONFIG_REG,
+				   ac300_ephy_ctl_config(priv) |
+				  AC300_EPHY_SHUTDOWN);
+	if (ret)
+		goto err_power_off;
+
+	fsleep(10000);
+
+	ret = ac300_ephy_ctl_write(priv, AC300_EPHY_CONFIG_REG,
+				   ac300_ephy_ctl_config(priv));
+	if (ret)
+		goto err_power_off;
+
+	return 0;
+
+err_power_off:
+	ac300_ephy_ctl_disable(priv);
+
+	return ret;
+}
+
+static void ac300_ephy_clk_put(void *data)
+{
+	clk_put(data);
+}
+
+struct acx00_ephy_control *
+ac300_ephy_ctl_create(struct phy_device *phydev,
+		      struct device_node *package_node, u8 calibration)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct ac300_ephy_ctl *priv;
+	unsigned long clk_rate;
+	u8 bgs_effuse_code;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return ERR_PTR(-ENOMEM);
+	if (phydev->mdio.addr > FIELD_MAX(AC300_PACKAGE_PHY_ADDR_MASK))
+		return ERR_PTR(dev_err_probe(dev, -EINVAL,
+					     "link address is outside the package range\n"));
+	priv->phydev = phydev;
+
+	priv->vcc = devm_of_regulator_get(dev, package_node, "vcc");
+	if (IS_ERR(priv->vcc))
+		return ERR_PTR(dev_err_probe(dev, PTR_ERR(priv->vcc),
+					     "failed to get package supply\n"));
+
+	priv->clk = of_clk_get(package_node, 0);
+	if (IS_ERR(priv->clk))
+		return ERR_PTR(dev_err_probe(dev, PTR_ERR(priv->clk),
+					     "failed to get input clock\n"));
+
+	ret = devm_add_action_or_reset(dev, ac300_ephy_clk_put, priv->clk);
+	if (ret)
+		return ERR_PTR(ret);
+
+	ret = devm_clk_rate_exclusive_get(dev, priv->clk);
+	if (ret)
+		return ERR_PTR(dev_err_probe(dev, ret,
+					     "failed to lock clock rate\n"));
+
+	clk_rate = clk_get_rate(priv->clk);
+	switch (clk_rate) {
+	case AC300_EPHY_CLK_RATE_24_MHZ:
+		priv->sys_control = AC300_EPHY_CLK_SEL_24_MHZ;
+		break;
+	case AC300_EPHY_CLK_RATE_25_MHZ:
+		priv->sys_control = AC300_EPHY_CLK_SEL_25_MHZ;
+		break;
+	case AC300_EPHY_CLK_RATE_27_MHZ:
+		priv->sys_control = AC300_EPHY_CLK_SEL_27_MHZ;
+		break;
+	default:
+		return ERR_PTR(dev_err_probe(dev, -EINVAL,
+					     "unsupported input clock rate %lu Hz\n",
+					     clk_rate));
+	}
+	priv->sys_control |= AC300_SYS_CONTROL_ENABLE_BITS;
+
+	/* The vendor driver supplies no transfer function beyond this offset. */
+	bgs_effuse_code = (calibration + AC300_EPHY_BGS_EFFUSE_OFFSET) &
+			   FIELD_MAX(AC300_EPHY_BGS_EFFUSE_MASK);
+	priv->ephy_config =
+		FIELD_PREP(AC300_EPHY_BGS_EFFUSE_MASK, bgs_effuse_code);
+	/* EPHY_MODE and BIST_CLK_EN stay clear for normal operation. */
+
+	priv->control.power_on = ac300_ephy_ctl_power_on;
+	priv->control.power_off = ac300_ephy_ctl_power_off;
+	priv->control.set_interface = ac300_ephy_ctl_set_interface;
+	/* MII is the reset default used until the MAC supplies its interface. */
+	priv->interface = PHY_INTERFACE_MODE_MII;
+
+	return &priv->control;
+}
diff --git a/drivers/net/phy/xpowers/acx00.c b/drivers/net/phy/xpowers/acx00.c
new file mode 100644
index 000000000000..98f2b0b63909
--- /dev/null
+++ b/drivers/net/phy/xpowers/acx00.c
@@ -0,0 +1,536 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * X-Powers AC200/AC300 Ethernet PHY driver
+ *
+ * Copyright (C) 2019 Jernej Skrabec <jernej.skrabec@gmail.com>
+ * Copyright (C) 2026 James Hilliard <james.hilliard1@gmail.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/phy.h>
+#include <linux/slab.h>
+
+#include "../phylib.h"
+#include "acx00.h"
+
+#define ACX00_EPHY_ID				0x00441400
+
+#define ACX00_EPHY_CONFIG_CALIBRATION_MASK	GENMASK(3, 0)
+#define ACX00_EPHY_CONFIG_VARIANT_AC300		BIT(8)
+#define ACX00_EPHY_CONFIG_CALIBRATION_LOW	BIT(9)
+
+#define ACX00_PAGE_SELECT_REG			0x1f
+#define ACX00_PAGE_SELECT_MASK			GENMASK(12, 8)
+#define ACX00_PAGE_0				0
+#define ACX00_PAGE_1				1
+#define ACX00_PAGE_2				2
+#define ACX00_PAGE_6				6
+#define ACX00_PAGE_8				8
+
+#define ACX00_PAGE0_GLOBAL_CONFIG_REG		0x13
+#define ACX00_PAGE0_XMII_RX_CLOCK_INVERT	BIT(12)
+#define ACX00_PAGE0_MDI_MODE_MASK		GENMASK(1, 0)
+#define ACX00_PAGE0_MDI_MODE_AUTO		2
+
+#define ACX00_PAGE1_APS_CONTROL_REG		0x12
+#define ACX00_PAGE1_APS_DISABLED_4S_VALUE	0x4824
+#define ACX00_PAGE1_UAPS_CONTROL_REG		0x13
+#define ACX00_PAGE1_UAPS_ENABLE			BIT(15)
+#define ACX00_PAGE1_INTELLIGENT_EEE_CONTROL_REG	0x17
+#define ACX00_PAGE1_INTELLIGENT_EEE_ENABLE	BIT(3)
+
+#define ACX00_PAGE2_TX_DATA_CONTROL_REG		0x18
+#define ACX00_PAGE2_10BT_FIR_SELECT_MASK		GENMASK(14, 12)
+#define ACX00_PAGE2_10BT_FIR_SELECT_DEFAULT	0
+
+#define ACX00_PAGE6_ADC_CONTROL_REG		0x10
+#define ACX00_PAGE6_ADC_CONTROL_LOW_CAL_VALUE	0x5523
+#define ACX00_PAGE6_AFE_RX_CONTROL_REG		0x13
+#define ACX00_PAGE6_AFE_RX_CONTROL_VALUE		0xf000
+#define ACX00_PAGE6_AFE_EQ_RX_DETECT_CONTROL_REG	0x14
+#define AC200_PAGE6_AFE_EQ_RX_DETECT_VALUE	0x708f
+#define AC300_PAGE6_AFE_EQ_RX_DETECT_VALUE	0x708b
+#define ACX00_PAGE6_AFE_EQ_RX_DETECT_LOW_CAL_VALUE 0x7809
+#define ACX00_PAGE6_TX_LEVEL_REG			0x15
+#define ACX00_PAGE6_TX_LEVEL_100M_MASK		GENMASK(15, 8)
+#define ACX00_PAGE6_TX_LEVEL_10M_MASK		GENMASK(7, 0)
+#define ACX00_PAGE6_TX_LEVEL_VALUE(_100m, _10m) \
+	(FIELD_PREP(ACX00_PAGE6_TX_LEVEL_100M_MASK, (_100m)) | \
+	 FIELD_PREP(ACX00_PAGE6_TX_LEVEL_10M_MASK, (_10m)))
+#define ACX00_PAGE6_TX_LEVEL_DEFAULT_VALUE \
+	ACX00_PAGE6_TX_LEVEL_VALUE(0x15, 0x30)
+#define ACX00_PAGE6_TX_LEVEL_LOW_CAL_VALUE \
+	ACX00_PAGE6_TX_LEVEL_VALUE(0x35, 0x33)
+
+#define ACX00_PAGE8_AFE_CONTROL_REG		0x18
+#define ACX00_PAGE8_AFE_CONTROL_VALUE		0x00bc
+#define ACX00_PAGE8_AUTO_CAL_CONTROL_REG		0x1d
+#define ACX00_PAGE8_AUTO_CAL_TX_LEVEL_ADJUST_BYPASS BIT(11)
+#define ACX00_PAGE8_AUTO_CAL_LOW_CAL_OPAQUE_BITS	0x0044
+#define ACX00_PAGE8_AUTO_CAL_LOW_VALUE \
+	(ACX00_PAGE8_AUTO_CAL_TX_LEVEL_ADJUST_BYPASS | \
+	 ACX00_PAGE8_AUTO_CAL_LOW_CAL_OPAQUE_BITS)
+
+/*
+ * Another integration of this exact-ID PHY documents its digital vendor
+ * register map, which also matches the observed ACx00 reset values. The ACx00
+ * analog-page field encodings remain unpublished, so keep those as opaque
+ * vendor initialization values instead of inventing bit definitions.
+ */
+
+struct acx00_ephy_priv {
+	struct phy_device *phydev;
+	struct acx00_ephy_control *control;
+	bool is_ac300;
+	bool use_low_calibration_tuning;
+	bool xmii_rx_clock_inverted;
+};
+
+static int acx00_ephy_read_page(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = __phy_read(phydev, ACX00_PAGE_SELECT_REG);
+	if (ret < 0)
+		return ret;
+
+	return FIELD_GET(ACX00_PAGE_SELECT_MASK, ret);
+}
+
+static int acx00_ephy_write_page(struct phy_device *phydev, int page)
+{
+	return __phy_write(phydev, ACX00_PAGE_SELECT_REG,
+			   FIELD_PREP(ACX00_PAGE_SELECT_MASK, page));
+}
+
+static int acx00_ephy_control_power_on(struct acx00_ephy_priv *priv)
+{
+	return priv->control->power_on(priv->control,
+				       priv->phydev->mdio.addr);
+}
+
+static int acx00_ephy_control_power_off(struct acx00_ephy_priv *priv)
+{
+	return priv->control->power_off(priv->control);
+}
+
+static int acx00_ephy_set_interface(struct phy_device *phydev)
+{
+	struct acx00_ephy_priv *priv = phydev->priv;
+
+	if (phydev->interface == PHY_INTERFACE_MODE_NA)
+		return 0;
+	if (phydev->interface != PHY_INTERFACE_MODE_MII &&
+	    phydev->interface != PHY_INTERFACE_MODE_RMII)
+		return -EINVAL;
+
+	return priv->control->set_interface(priv->control,
+					    phydev->interface);
+}
+
+static void acx00_ephy_control_release(void *data)
+{
+	struct acx00_ephy_priv *priv = data;
+	int ret;
+
+	ret = acx00_ephy_control_power_off(priv);
+	if (ret)
+		phydev_warn(priv->phydev,
+			    "failed to power off control block: %pe\n",
+			    ERR_PTR(ret));
+}
+
+static int acx00_ephy_read_nvmem_u16(struct device_node *node,
+				     const char *name, u16 *value)
+{
+	struct nvmem_cell *cell;
+	size_t i;
+	size_t len;
+	u8 *buf;
+	u16 val = 0;
+
+	cell = of_nvmem_cell_get(node, name);
+	if (IS_ERR(cell))
+		return PTR_ERR(cell);
+
+	buf = nvmem_cell_read(cell, &len);
+	nvmem_cell_put(cell);
+	if (IS_ERR(buf))
+		return PTR_ERR(buf);
+	if (!len || len > sizeof(*value)) {
+		kfree(buf);
+		return len ? -ERANGE : -EINVAL;
+	}
+
+	for (i = 0; i < len; i++)
+		val |= (u16)buf[i] << (8 * i);
+	kfree(buf);
+	*value = val;
+
+	return 0;
+}
+
+static int acx00_ephy_init_package(struct phy_device *phydev,
+				   struct acx00_ephy_priv *priv)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct device_node *package_node;
+	bool selectable;
+	bool fixed_ac300;
+	bool has_configuration;
+	u8 calibration;
+	u32 base_addr;
+	u16 configuration = 0;
+	int ret;
+
+	package_node = of_get_parent(dev->of_node);
+	if (!package_node)
+		return -EINVAL;
+	if (!of_node_name_eq(package_node, "ethernet-phy-package")) {
+		ret = dev_err_probe(dev, -EINVAL,
+				    "PHY is not in an Ethernet PHY package\n");
+		goto out_put_node;
+	}
+
+	ret = of_property_read_u32(package_node, "reg", &base_addr);
+	if (ret || base_addr != phydev->mdio.addr) {
+		ret = dev_err_probe(dev, -EINVAL,
+				    "package and link PHY addresses differ\n");
+		goto out_put_node;
+	}
+
+	selectable = of_device_is_compatible(package_node,
+					     "x-powers,acx00-ephy-package");
+	fixed_ac300 = of_device_is_compatible(package_node,
+					      "x-powers,ac300-ephy-package");
+	if (!selectable && !fixed_ac300 &&
+	    !of_device_is_compatible(package_node,
+				     "x-powers,ac200-ephy-package")) {
+		ret = dev_err_probe(dev, -EINVAL,
+				    "unsupported Ethernet PHY package\n");
+		goto out_put_node;
+	}
+
+	ret = devm_of_phy_package_join(dev, phydev, 0);
+	if (ret)
+		goto out_put_node;
+
+	has_configuration =
+		of_property_match_string(package_node, "nvmem-cell-names",
+					 "configuration") >= 0;
+	if (has_configuration) {
+		ret = acx00_ephy_read_nvmem_u16(package_node, "configuration",
+						&configuration);
+		if (ret) {
+			ret = dev_err_probe(dev, ret,
+					    "failed to read package configuration\n");
+			goto out_put_node;
+		}
+	} else if (selectable || fixed_ac300) {
+		ret = dev_err_probe(dev, -EINVAL,
+				    "package configuration is required\n");
+		goto out_put_node;
+	}
+
+	if (selectable) {
+		priv->is_ac300 =
+			configuration & ACX00_EPHY_CONFIG_VARIANT_AC300;
+	} else {
+		priv->is_ac300 = fixed_ac300;
+		if (has_configuration &&
+		    !!(configuration & ACX00_EPHY_CONFIG_VARIANT_AC300) !=
+		    priv->is_ac300) {
+			ret = dev_err_probe(dev, -EINVAL,
+					    "configuration does not match package\n");
+			goto out_put_node;
+		}
+	}
+
+	calibration = FIELD_GET(ACX00_EPHY_CONFIG_CALIBRATION_MASK,
+				configuration);
+
+	priv->use_low_calibration_tuning =
+		priv->is_ac300 &&
+		!!(configuration & ACX00_EPHY_CONFIG_CALIBRATION_LOW);
+	priv->xmii_rx_clock_inverted =
+		of_property_read_bool(package_node,
+				      "x-powers,xmii-rx-clock-inverted");
+
+	if (priv->is_ac300)
+		priv->control =
+			ac300_ephy_ctl_create(phydev, package_node,
+					      calibration);
+	else
+		priv->control =
+			ac200_ephy_ctl_create(phydev, package_node,
+					      has_configuration,
+					      calibration);
+	ret = PTR_ERR_OR_ZERO(priv->control);
+
+out_put_node:
+	of_node_put(package_node);
+	return ret;
+}
+
+static int acx00_ephy_disable_autonomous_eee(struct phy_device *phydev)
+{
+	return phy_modify_paged(phydev, ACX00_PAGE_1,
+				ACX00_PAGE1_INTELLIGENT_EEE_CONTROL_REG,
+				ACX00_PAGE1_INTELLIGENT_EEE_ENABLE, 0);
+}
+
+static int acx00_ephy_probe(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct acx00_ephy_priv *priv;
+	int ret;
+
+	if (!dev->of_node)
+		return -ENODEV;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->phydev = phydev;
+	ret = acx00_ephy_init_package(phydev, priv);
+	if (ret)
+		return ret;
+
+	phydev->priv = priv;
+	ret = devm_add_action_or_reset(dev, acx00_ephy_control_release, priv);
+	if (ret)
+		return ret;
+
+	ret = acx00_ephy_control_power_on(priv);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to power on control block\n");
+
+	return 0;
+}
+
+static int acx00_ephy_soft_reset(struct phy_device *phydev)
+{
+	struct acx00_ephy_priv *priv = phydev->priv;
+	int ret;
+
+	ret = acx00_ephy_set_interface(phydev);
+	if (ret)
+		return ret;
+
+	ret = acx00_ephy_control_power_on(priv);
+	if (ret)
+		return ret;
+
+	/* ACx00 can acknowledge reset in power-down without restarting. */
+	ret = genphy_resume(phydev);
+	if (ret)
+		return ret;
+
+	return genphy_soft_reset(phydev);
+}
+
+static int acx00_ephy_config_init(struct phy_device *phydev)
+{
+	struct acx00_ephy_priv *priv = phydev->priv;
+	u16 afe_eq_rx_detect = priv->is_ac300 ?
+		AC300_PAGE6_AFE_EQ_RX_DETECT_VALUE :
+		AC200_PAGE6_AFE_EQ_RX_DETECT_VALUE;
+	u16 tx_level_value = ACX00_PAGE6_TX_LEVEL_DEFAULT_VALUE;
+	u16 global_config;
+	int oldpage;
+	int ret;
+
+	global_config = FIELD_PREP(ACX00_PAGE0_MDI_MODE_MASK,
+				   ACX00_PAGE0_MDI_MODE_AUTO);
+	if (priv->xmii_rx_clock_inverted)
+		global_config |= ACX00_PAGE0_XMII_RX_CLOCK_INVERT;
+
+	ret = phy_modify_paged(phydev, ACX00_PAGE_0,
+			       ACX00_PAGE0_GLOBAL_CONFIG_REG,
+			       ACX00_PAGE0_XMII_RX_CLOCK_INVERT |
+			       ACX00_PAGE0_MDI_MODE_MASK, global_config);
+	if (ret)
+		return ret;
+
+	if (priv->is_ac300 && priv->use_low_calibration_tuning) {
+		afe_eq_rx_detect =
+			ACX00_PAGE6_AFE_EQ_RX_DETECT_LOW_CAL_VALUE;
+		tx_level_value = ACX00_PAGE6_TX_LEVEL_LOW_CAL_VALUE;
+	}
+
+	oldpage = phy_select_page(phydev, ACX00_PAGE_1);
+	if (oldpage < 0)
+		goto out_restore_page;
+
+	ret = __phy_write(phydev, ACX00_PAGE1_APS_CONTROL_REG,
+			  ACX00_PAGE1_APS_DISABLED_4S_VALUE);
+	if (ret)
+		goto out_restore_page;
+	ret = __phy_modify(phydev, ACX00_PAGE1_UAPS_CONTROL_REG,
+			   ACX00_PAGE1_UAPS_ENABLE, 0);
+	if (ret)
+		goto out_restore_page;
+	ret = __phy_modify(phydev, ACX00_PAGE1_INTELLIGENT_EEE_CONTROL_REG,
+			   ACX00_PAGE1_INTELLIGENT_EEE_ENABLE, 0);
+	if (ret)
+		goto out_restore_page;
+
+	ret = acx00_ephy_write_page(phydev, ACX00_PAGE_2);
+	if (ret)
+		goto out_restore_page;
+	ret = __phy_modify(phydev, ACX00_PAGE2_TX_DATA_CONTROL_REG,
+			   ACX00_PAGE2_10BT_FIR_SELECT_MASK,
+			   FIELD_PREP(ACX00_PAGE2_10BT_FIR_SELECT_MASK,
+				      ACX00_PAGE2_10BT_FIR_SELECT_DEFAULT));
+	if (ret)
+		goto out_restore_page;
+
+	ret = acx00_ephy_write_page(phydev, ACX00_PAGE_6);
+	if (ret)
+		goto out_restore_page;
+	ret = __phy_write(phydev, ACX00_PAGE6_AFE_EQ_RX_DETECT_CONTROL_REG,
+			  afe_eq_rx_detect);
+	if (ret)
+		goto out_restore_page;
+	ret = __phy_write(phydev, ACX00_PAGE6_AFE_RX_CONTROL_REG,
+			  ACX00_PAGE6_AFE_RX_CONTROL_VALUE);
+	if (ret)
+		goto out_restore_page;
+	if (priv->is_ac300 && priv->use_low_calibration_tuning) {
+		ret = __phy_write(phydev, ACX00_PAGE6_ADC_CONTROL_REG,
+				  ACX00_PAGE6_ADC_CONTROL_LOW_CAL_VALUE);
+		if (ret)
+			goto out_restore_page;
+	}
+	ret = __phy_write(phydev, ACX00_PAGE6_TX_LEVEL_REG, tx_level_value);
+	if (ret)
+		goto out_restore_page;
+
+	ret = acx00_ephy_write_page(phydev, ACX00_PAGE_8);
+	if (ret)
+		goto out_restore_page;
+	if (priv->is_ac300 && priv->use_low_calibration_tuning) {
+		ret = __phy_write(phydev, ACX00_PAGE8_AUTO_CAL_CONTROL_REG,
+				  ACX00_PAGE8_AUTO_CAL_LOW_VALUE);
+		if (ret)
+			goto out_restore_page;
+	}
+	ret = __phy_write(phydev, ACX00_PAGE8_AFE_CONTROL_REG,
+			  ACX00_PAGE8_AFE_CONTROL_VALUE);
+
+out_restore_page:
+	ret = phy_restore_page(phydev, oldpage, ret);
+	if (ret)
+		return ret;
+
+	/* Restore the standard EEE policy retained by phylib across resets. */
+	return genphy_c45_an_config_eee_aneg(phydev);
+}
+
+static int acx00_ephy_power_on_and_resume(struct phy_device *phydev)
+{
+	struct acx00_ephy_priv *priv = phydev->priv;
+	int ret;
+
+	ret = acx00_ephy_set_interface(phydev);
+	if (ret)
+		return ret;
+
+	ret = acx00_ephy_control_power_on(priv);
+	if (ret)
+		return ret;
+
+	ret = genphy_resume(phydev);
+	if (ret) {
+		acx00_ephy_control_power_off(priv);
+		return ret;
+	}
+
+	/* Powering off the control block loses the vendor-page state. */
+	return acx00_ephy_config_init(phydev);
+}
+
+static int acx00_ephy_resume(struct phy_device *phydev)
+{
+	return acx00_ephy_power_on_and_resume(phydev);
+}
+
+static int acx00_ephy_suspend(struct phy_device *phydev)
+{
+	struct acx00_ephy_priv *priv = phydev->priv;
+	int resume_ret;
+	int ret;
+
+	ret = genphy_suspend(phydev);
+	if (ret)
+		return ret;
+
+	ret = acx00_ephy_control_power_off(priv);
+	if (ret) {
+		resume_ret = acx00_ephy_power_on_and_resume(phydev);
+		if (resume_ret)
+			phydev_warn(phydev,
+				    "failed to recover from suspend error: %pe\n",
+				    ERR_PTR(resume_ret));
+	}
+
+	return ret;
+}
+
+static int acx00_ephy_match_phy_device(struct phy_device *phydev,
+				       const struct phy_driver *phydrv)
+{
+	struct device_node *node = phydev->mdio.dev.of_node;
+	struct device_node *package_node;
+	bool match;
+
+	if (!genphy_match_phy_device(phydev, phydrv) || !node)
+		return 0;
+
+	/* RK630 reports the same PHY ID, so also match the package identity. */
+	package_node = of_get_parent(node);
+	if (!package_node)
+		return 0;
+	match = of_device_is_compatible(package_node,
+					"x-powers,ac200-ephy-package") ||
+		of_device_is_compatible(package_node,
+					"x-powers,ac300-ephy-package") ||
+		of_device_is_compatible(package_node,
+					"x-powers,acx00-ephy-package");
+	of_node_put(package_node);
+
+	return match;
+}
+
+static struct phy_driver acx00_ephy_driver[] = {
+	{
+		PHY_ID_MATCH_MODEL(ACX00_EPHY_ID),
+		.name = "X-Powers AC200/AC300 EPHY",
+		.match_phy_device = acx00_ephy_match_phy_device,
+		.probe = acx00_ephy_probe,
+		.read_page = acx00_ephy_read_page,
+		.write_page = acx00_ephy_write_page,
+		.soft_reset = acx00_ephy_soft_reset,
+		.config_init = acx00_ephy_config_init,
+		.disable_autonomous_eee = acx00_ephy_disable_autonomous_eee,
+		.suspend = acx00_ephy_suspend,
+		.resume = acx00_ephy_resume,
+	},
+};
+module_phy_driver(acx00_ephy_driver);
+
+static const struct mdio_device_id __maybe_unused acx00_ephy_tbl[] = {
+	{ PHY_ID_MATCH_MODEL(ACX00_EPHY_ID) },
+	{ }
+};
+MODULE_DEVICE_TABLE(mdio, acx00_ephy_tbl);
+
+MODULE_AUTHOR("James Hilliard <james.hilliard1@gmail.com>");
+MODULE_DESCRIPTION("X-Powers AC200/AC300 Ethernet PHY driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/phy/xpowers/acx00.h b/drivers/net/phy/xpowers/acx00.h
new file mode 100644
index 000000000000..7df6084c7a77
--- /dev/null
+++ b/drivers/net/phy/xpowers/acx00.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __DRIVERS_NET_PHY_XPOWERS_ACX00_H
+#define __DRIVERS_NET_PHY_XPOWERS_ACX00_H
+
+#include <linux/err.h>
+#include <linux/phy.h>
+
+#define AC300_EPHY_CONTROL_ADDR_OFFSET	16
+
+struct device_node;
+
+struct acx00_ephy_control {
+	int (*power_on)(struct acx00_ephy_control *control,
+			unsigned int phy_addr);
+	int (*power_off)(struct acx00_ephy_control *control);
+	int (*set_interface)(struct acx00_ephy_control *control,
+			     phy_interface_t interface);
+};
+
+struct acx00_ephy_control *
+ac200_ephy_ctl_create(struct phy_device *phydev,
+		      struct device_node *package_node,
+		      bool has_calibration, u8 calibration);
+struct acx00_ephy_control *
+ac300_ephy_ctl_create(struct phy_device *phydev,
+		      struct device_node *package_node, u8 calibration);
+
+#endif

-- 
2.53.0

Re: [PATCH net-next v8 1/2] dt-bindings: net: x-powers: add AC200/AC300 EPHY packages

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
dt-bindings: net: x-powers: add AC200/AC300 EPHY packages

This adds a new binding describing the X-Powers AC200 and AC300 Ethernet
PHY packages, with separate compatibles for each fixed variant plus an
ACx00 compatible for boards that can carry either part. It documents the
package clock, supply, packed NVMEM configuration field and the phandle
used to reach the AC200 companion IC.

> diff --git a/Documentation/devicetree/bindings/net/x-powers,acx00-ephy-package.yaml b/Documentation/devicetree/bindings/net/x-powers,acx00-ephy-package.yaml
> new file mode 100644
> index 0000000000000..afe57bd163dd7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/x-powers,acx00-ephy-package.yaml
> @@ -0,0 +1,154 @@

[ ... ]

> +  x-powers,ac200:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Phandle to the AC200 companion IC providing I2C access to the package
> +      control registers used to power and configure the Ethernet PHY.

[Severity: Low]
Is there a binding for the node this phandle is supposed to point at?

A case-insensitive grep for ac200 under Documentation/devicetree/bindings at
the end of this series matches only this new file; the x-powers mfd directory
contains just x-powers,ac100.yaml and x-powers,axp152.yaml. So the
compatible, bus position (I2C child) and properties of the phandle target
appear to be undocumented, and any device tree using the ac200 or acx00
compatibles would carry a node that dtbs_check reports as undocumented.

The same gap seems to exist on the driver side. ac200_ephy_ctl_create() in
drivers/net/phy/xpowers/ac200.c resolves the phandle and then needs a bound
I2C client with a regmap and a clock:

  client = of_find_i2c_device_by_node(ac200_node);
  ...
  device_lock(&client->dev);
  if (device_is_bound(&client->dev))
          priv->regmap = dev_get_regmap(&client->dev, NULL);
  device_unlock(&client->dev);
  if (!priv->regmap) {
          ret = dev_err_probe(dev, -EPROBE_DEFER, "AC200 driver is not ready\n");

A grep for ac200 in drivers/mfd returns nothing at the end of this series,
so is there any in-tree driver that can bind such an I2C device and provide
that regmap and clock? If not, this path can only ever return -EPROBE_DEFER,
and the AC200/ACx00 half of the new ABI, including the packed NVMEM bit
layout consumed only through that path, cannot be exercised or validated by
a conforming device tree.

Could the companion IC binding be added (or referenced) alongside this file?

> +
> +  x-powers,xmii-rx-clock-inverted:
> +    type: boolean
> +    description:
> +      Invert the xMII receive clock inside the PHY. This is required by some
> +      SoC integrations, including the Allwinner H6 AC200 connection.
> +
> +patternProperties:
> +  ^ethernet-phy@[a-f0-9]+$:
> +    $ref: ethernet-phy.yaml#
> +    properties:
> +      compatible:
> +        const: ethernet-phy-id0044.1400
> +    required:
> +      - compatible
> +    unevaluatedProperties: false

[Severity: Low]
This isn't a bug, but should the child node's reg be constrained here too?

The child schema overrides only compatible, so the link PHY's reg keeps
ethernet-phy.yaml's 0..31 range. The description in this same file says "The
package and child use the same base address", and the ac300/acx00
conditionals below limit the package reg to 7, but neither constraint reaches
the child. A tree like:

  ethernet-phy-package@0 {
          reg = <0>;
          ethernet-phy@1f {
                  reg = <0x1f>;
          };
  };

passes dt_binding_check, as does an AC300 child above address 7.

Nothing is misprogrammed at runtime: acx00_ephy_init_package() in
drivers/net/phy/xpowers/acx00.c rejects the mismatch before joining the
package,

  ret = of_property_read_u32(package_node, "reg", &base_addr);
  if (ret || base_addr != phydev->mdio.addr) {
          ret = dev_err_probe(dev, -EINVAL,
                              "package and link PHY addresses differ\n");

and ac300_ephy_ctl_create() in drivers/net/phy/xpowers/ac300.c rejects
addresses above 7 before any control-range access:

  if (phydev->mdio.addr > FIELD_MAX(AC300_PACKAGE_PHY_ADDR_MASK))
          return ERR_PTR(dev_err_probe(dev, -EINVAL,
                                       "link address is outside the package range\n"));

So the only effect is that an invalid tree fails PHY probe instead of failing
schema validation. Bounding the child reg per variant in the added
patternProperties block would move that detection to dt_binding_check. Note
the existing package binding qcom,qca807x.yaml adds no child-address
constraints either, so the omission matches current convention.

> +
> +required:
> +  - compatible
> +  - vcc-supply

[ ... ]

> +  - if:
> +      properties:
> +        compatible:
> +          const: x-powers,acx00-ephy-package
> +    then:
> +      properties:
> +        reg:
> +          maximum: 7
> +      required:
> +        - clocks
> +        - nvmem-cells
> +        - nvmem-cell-names
> +        - x-powers,ac200

[ ... ]
-- 
pw-bot: cr

Re: [PATCH net-next v8 2/2] net: phy: add X-Powers AC200/AC300 EPHY driver

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: phy: add X-Powers AC200/AC300 EPHY driver

This adds a single PHY driver for the Fast Ethernet link PHY found in the
X-Powers AC200 and AC300 companion ICs, with two package backends: AC200
control registers reached over an I2C regmap, and AC300 control registers
reached through the PHY package helpers at base address plus 16.

A few questions below about the AC200 backend, the resume/suspend return
values and the power-state bookkeeping.

> diff --git a/drivers/net/phy/xpowers/ac200.c b/drivers/net/phy/xpowers/ac200.c
> new file mode 100644
> index 0000000000000..17011b5f5e21c
> --- /dev/null
> +++ b/drivers/net/phy/xpowers/ac200.c
> @@ -0,0 +1,308 @@

[ ... ]

> +static int ac200_ephy_ctl_power_on(struct acx00_ephy_control *control,
> +				   unsigned int phy_addr)
> +{
> +	struct ac200_ephy_ctl *priv =
> +		container_of(control, struct ac200_ephy_ctl, control);
> +	u16 ephy_ctl;
> +	int ret;
> +
> +	if (phy_addr > FIELD_MAX(AC200_EPHY_PHY_ADDR_MASK))
> +		return -EINVAL;
> +
> +	if (priv->powered && priv->phy_addr == phy_addr)
> +		return 0;
> +
> +	if (priv->powered) {
> +		ret = ac200_ephy_ctl_disable(priv);
> +		if (ret)
> +			return ret;
> +	}
> +	priv->phy_addr = phy_addr;
> +
> +	if (!priv->supply_enabled) {
> +		ret = regulator_enable(priv->vcc);
> +		if (ret)
> +			return ret;
> +		priv->supply_enabled = true;
> +	}

[Severity: Medium]

What serializes these check-then-set updates of priv->supply_enabled and
priv->powered?

The same flags are read and written from ac200_ephy_ctl_power_on(),
ac200_ephy_ctl_disable() and ac200_ephy_ctl_set_interface(), reached from
four phylib entry points with different lock contexts:

  .resume    -> acx00_ephy_resume()        (phydev->lock held,
                                            __phy_resume() has
                                            lockdep_assert_held())
  .suspend   -> acx00_ephy_suspend()       (no phylib lock; phy_suspend()
                                            takes none, and
                                            _phy_state_machine_post_work()
                                            calls it after
                                            phy_state_machine() unlocked)
  .soft_reset-> acx00_ephy_soft_reset()    (via exported phy_init_hw(),
                                            no phylib lock)
  devres     -> acx00_ephy_control_release() (unbind, no lock)

If two of those interleave, the regulator enable count (and for AC300 the
clk_prepare_enable() count guarded by the same priv->powered flag) becomes
unbalanced, and the multi-register bring-up and shutdown sequences
interleave since only the regmap or MDIO bus lock is held per access. Would
a mutex in the backend private data be appropriate here?

> +
> +	ephy_ctl = ac200_ephy_ctl_config(priv);
> +
> +	/* Start from a disabled state before applying the configuration. */
> +	ret = ac200_ephy_ctl_write(priv, AC200_SYS_EPHY_CTL0_REG, 0);
> +	if (ret)
> +		goto err_disable;
> +
> +	ret = ac200_ephy_ctl_write(priv, AC200_SYS_EPHY_CTL1_REG,
> +				   AC200_EPHY_MII_IO_ENABLE);
> +	if (ret)
> +		goto err_disable;
> +
> +	ret = ac200_ephy_ctl_write(priv, AC200_EPHY_CTL_REG,
> +				   ephy_ctl | AC200_EPHY_SHUTDOWN);
> +	if (ret)
> +		goto err_disable;
> +
> +	ret = ac200_ephy_ctl_write(priv, AC200_SYS_EPHY_CTL0_REG,
> +				   AC200_EPHY_RESET_DEASSERT |
> +				   AC200_EPHY_SYSCLK_ENABLE);
> +	if (ret)
> +		goto err_disable;
> +
> +	ret = ac200_ephy_ctl_write(priv, AC200_EPHY_CTL_REG, ephy_ctl);
> +	if (ret)
> +		goto err_disable;

[Severity: Low]

Is the AC200 power-on sequence really free of timing requirements?

ac200_ephy_ctl_power_on() writes the control registers immediately after
regulator_enable(), and clears AC200_EPHY_SHUTDOWN in the write directly
following the reset-deassert plus sysclk-enable write. The callers
(acx00_ephy_soft_reset() and acx00_ephy_power_on_and_resume() ->
genphy_resume()) then start Clause 22 accesses to the link PHY right away.

The AC300 backend, driving the same PHY block, treats the equivalent steps
as timed:

	/* Wait for the power-on reset interval specified by the manual. */
	fsleep(10000);
	...
	ret = ac300_ephy_ctl_write(priv, AC300_EPHY_CONFIG_REG,
				   ac300_ephy_ctl_config(priv) |
				  AC300_EPHY_SHUTDOWN);
	...
	fsleep(10000);

Is the asymmetry intentional, i.e. does the AC200 datasheet specify no
power-on-reset interval and no settling time around the shutdown
transition? I could not confirm the AC200 timing from anything in tree.

[ ... ]

> +	ac200_node = of_parse_phandle(package_node, "x-powers,ac200", 0);
> +	if (!ac200_node)
> +		return ERR_PTR(dev_err_probe(dev, -EINVAL,
> +					     "missing x-powers,ac200 reference\n"));
> +	client = of_find_i2c_device_by_node(ac200_node);
> +	of_node_put(ac200_node);
> +	if (!client) {
> +		ret = IS_ENABLED(CONFIG_I2C) ? -EPROBE_DEFER : -ENODEV;
> +		return ERR_PTR(dev_err_probe(dev, ret,
> +					     "AC200 device is not registered\n"));
> +	}
> +
> +	if (!device_link_add(dev, &client->dev,
> +			     DL_FLAG_AUTOREMOVE_CONSUMER)) {
> +		ret = dev_err_probe(dev, -EINVAL,
> +				    "failed to link AC200 device\n");
> +		goto out_put_client;
> +	}
> +
> +	device_lock(&client->dev);
> +	if (device_is_bound(&client->dev))
> +		priv->regmap = dev_get_regmap(&client->dev, NULL);
> +	device_unlock(&client->dev);
> +	if (!priv->regmap) {
> +		ret = dev_err_probe(dev, -EPROBE_DEFER,
> +				    "AC200 driver is not ready\n");
> +		goto out_put_client;
> +	}

[Severity: Low]

Can this gate ever be satisfied in mainline?

The AC200 path requires that the I2C device referenced by the
x-powers,ac200 phandle is bound to a driver which has registered a regmap.
Searching the tree at this commit, x-powers,ac200 matches only the new
binding and the two new driver files, there is no AC200 companion-IC driver
(nothing in drivers/mfd), and no DTS instantiates such a node. The only
other AC200-ish DTS matches are unrelated (regulator-name = "vcc-ac200",
allwinner,sun55i-a523-gmac200).

So on any DT using x-powers,ac200-ephy-package or
x-powers,acx00-ephy-package with an AC200, of_i2c_register_devices() will
create the i2c_client, but device_is_bound() can never become true and the
probe defers forever. Everything below this point (the 0x0014, 0x0016,
0x6000 and 0x8004 register contract, the eFuse calibration read, the clock
rate selection and the whole power sequence) is then unreachable.

The Kconfig help text says:

	  Enable the Fast Ethernet PHY driver shared by the X-Powers AC200
	  and AC300 companion ICs.

and the commit message says:

    Firmware must make the selected AC200 provider path available
    before the PHY probes.

Would it be more accurate to state that the AC200 half depends on a
companion-IC MFD driver that is not merged yet, or to post that driver in
the same series so the AC200 code is testable?

> +
> +	if (!has_calibration) {
> +		ret = regmap_read(priv->regmap, AC200_EFUSE_EPHY_REG,
> +				  &internal_calibration);
> +		if (ret)
> +			goto out_error;
> +		calibration = internal_calibration;
> +	}

[ ... ]

> +	clk = clk_get(&client->dev, NULL);
> +	if (IS_ERR(clk)) {
> +		ret = PTR_ERR(clk);
> +		goto out_error;
> +	}
> +
> +	clk_rate = clk_get_rate(clk);
> +	clk_put(clk);
> +
> +	switch (clk_rate) {
> +	case AC200_EPHY_CLK_RATE_24_MHZ:
> +		priv->ephy_ctl |= AC200_EPHY_CLK_SEL_24_MHZ;
> +		break;
> +	case AC200_EPHY_CLK_RATE_27_MHZ:
> +		break;
> +	default:
> +		ret = dev_err_probe(dev, -EINVAL,
> +				    "unsupported AC200 clock rate %lu Hz\n",
> +				    clk_rate);
> +		goto out_put_client;
> +	}

[Severity: Medium]

Can the sampled rate go stale here?

ac200_ephy_ctl_create() reads the rate once, folds the result into
priv->ephy_ctl as AC200_EPHY_CLK_SEL_24_MHZ, drops the clock reference, and
every later ac200_ephy_ctl_power_on() re-writes that latched value. There is
no devm_clk_rate_exclusive_get() and no clk_prepare_enable() anywhere in
ac200.c, so if another consumer calls clk_set_rate() on the shared provider
after probe, CLK_SEL no longer describes the frequency reaching the EPHY and
the link timing is off with no error reported.

The AC300 backend handles the identical constraint the other way:

	ret = devm_add_action_or_reset(dev, ac300_ephy_clk_put, priv->clk);
	...
	ret = devm_clk_rate_exclusive_get(dev, priv->clk);

plus clk_prepare_enable() and clk_disable_unprepare() around the powered
state. Should the AC200 path pin the rate the same way? Related, the commit
message says:

    Obtain and manage the selected package supply, calibration and input
    clock

which as written seems to hold only for the AC300 backend. I could not check
whether the (unmerged) AC200 companion driver is expected to own and enable
that clock instead.

[Severity: Low]

Where is this AC200 clock requirement documented?

The package binding describes clocks as "AC300 input clock, running at 24,
25, or 27 MHz", requires it for x-powers,ac300-ephy-package and
x-powers,acx00-ephy-package, and forbids it for
x-powers,ac200-ephy-package. But the AC200 backend never looks at the
package clocks; it does clk_get(&client->dev, NULL) on the AC200 companion
I2C device and fails probe with -EINVAL unless that clock reports exactly
24 MHz or 27 MHz (and propagates the clk_get() error if the companion node
has no clock at all).

So a DT author has no in-tree description of the AC200 companion clock or
its permitted rates. Could the binding for the AC200 companion node (or at
least the package binding text) state that constraint?

> diff --git a/drivers/net/phy/xpowers/acx00.c b/drivers/net/phy/xpowers/acx00.c
> new file mode 100644
> index 0000000000000..98f2b0b639096
> --- /dev/null
> +++ b/drivers/net/phy/xpowers/acx00.c
> @@ -0,0 +1,536 @@

[ ... ]

> +static int acx00_ephy_soft_reset(struct phy_device *phydev)
> +{
> +	struct acx00_ephy_priv *priv = phydev->priv;
> +	int ret;
> +
> +	ret = acx00_ephy_set_interface(phydev);
> +	if (ret)
> +		return ret;
> +
> +	ret = acx00_ephy_control_power_on(priv);
> +	if (ret)
> +		return ret;
> +
> +	/* ACx00 can acknowledge reset in power-down without restarting. */
> +	ret = genphy_resume(phydev);
> +	if (ret)
> +		return ret;
> +
> +	return genphy_soft_reset(phydev);
> +}

[ ... ]

> +out_restore_page:
> +	ret = phy_restore_page(phydev, oldpage, ret);
> +	if (ret)
> +		return ret;
> +
> +	/* Restore the standard EEE policy retained by phylib across resets. */
> +	return genphy_c45_an_config_eee_aneg(phydev);
> +}

[Severity: Medium]

Should this positive return value be consumed rather than propagated?

genphy_c45_an_config_eee_aneg() returns 1 when it actually changed an EEE
advertisement register, because genphy_c45_write_eee_adv() returns changed:

drivers/net/phy/phy-c45.c:genphy_c45_write_eee_adv() {
	...
		if (val > 0)
			changed = 1;
	...
	return changed;
}

acx00_ephy_config_init() forwards that 1 to acx00_ephy_power_on_and_resume()
and from there to acx00_ephy_resume(), which is the .resume callback. In
phylib:

drivers/net/phy/phy_device.c:__phy_resume() {
	...
	ret = phydrv->resume(phydev);
	if (!ret)
		phydev->suspended = false;
	...
}

so a fully successful resume that happened to change the advertisement is
reported as an error and leaves phydev->suspended set. phy_suspend() then
short-circuits:

drivers/net/phy/phy_device.c:phy_suspend() {
	if (phydev->suspended || !phydrv)
		return 0;
	...
}

which means acx00_ephy_suspend(), the only path that powers the package
down, is skipped for the rest of the device's life. Note phy_init_hw() does
not always mask this, since phylink_prepare_resume() calls phy_resume()
directly when phydev->suspended is set.

Separately, if the advertisement did change, does this path need to restart
autonegotiation for the new advertisement?

> +
> +static int acx00_ephy_power_on_and_resume(struct phy_device *phydev)
> +{
> +	struct acx00_ephy_priv *priv = phydev->priv;
> +	int ret;
> +
> +	ret = acx00_ephy_set_interface(phydev);
> +	if (ret)
> +		return ret;
> +
> +	ret = acx00_ephy_control_power_on(priv);
> +	if (ret)
> +		return ret;
> +
> +	ret = genphy_resume(phydev);
> +	if (ret) {
> +		acx00_ephy_control_power_off(priv);
> +		return ret;
> +	}
> +
> +	/* Powering off the control block loses the vendor-page state. */
> +	return acx00_ephy_config_init(phydev);
> +}

[Severity: Medium]

Does the acx00_ephy_config_init() error path leave the package powered?

The genphy_resume() failure branch calls acx00_ephy_control_power_off(), but
a failure in the trailing acx00_ephy_config_init() returns with
priv->powered and priv->supply_enabled still true. acx00_ephy_soft_reset()
above has the same shape: it powers on, then returns genphy_resume() or
genphy_soft_reset() errors without a matching power off.

Because phylib only clears the suspended flag after a successful
.soft_reset:

drivers/net/phy/phy_device.c:phy_init_hw() {
	if (phydev->drv->soft_reset) {
		ret = phydev->drv->soft_reset(phydev);
		if (ret < 0)
			return ret;

		/* see comment in genphy_soft_reset for an explanation */
		phydev->suspended = 0;
	}
	...
}

and mdio_bus_phy_resume() aborts before phy_resume() when phy_init_hw()
fails, a single failed MDIO or I2C transaction on this path leaves
phydev->suspended set. phy_suspend() then returns early on that flag, so
the package supply (and the AC300 input clock enabled via
clk_prepare_enable()) stays on for all later suspend cycles until unbind
runs the devres release action. Should both paths power the control block
back off on failure?

> +
> +static int acx00_ephy_resume(struct phy_device *phydev)
> +{
> +	return acx00_ephy_power_on_and_resume(phydev);
> +}
> +
> +static int acx00_ephy_suspend(struct phy_device *phydev)
> +{
> +	struct acx00_ephy_priv *priv = phydev->priv;
> +	int resume_ret;
> +	int ret;
> +
> +	ret = genphy_suspend(phydev);
> +	if (ret)
> +		return ret;
> +
> +	ret = acx00_ephy_control_power_off(priv);
> +	if (ret) {
> +		resume_ret = acx00_ephy_power_on_and_resume(phydev);
> +		if (resume_ret)
> +			phydev_warn(phydev,
> +				    "failed to recover from suspend error: %pe\n",
> +				    ERR_PTR(resume_ret));
> +	}
> +
> +	return ret;
> +}

[Severity: Medium]

Is it safe to run the resume sequence from the .suspend callback?

acx00_ephy_power_on_and_resume() performs genphy_resume() (BMCR modify) and
acx00_ephy_config_init(), which does phy_select_page() plus paged vendor
writes and then genphy_c45_an_config_eee_aneg(), reading phydev->autoneg,
phydev->eee_cfg.eee_enabled and phydev->advertising_eee.

phylib requires phydev->lock for the resume operation:

drivers/net/phy/phy_device.c:__phy_resume() {
	lockdep_assert_held(&phydev->lock);
	...
}

but .suspend runs with no phylib lock held (phy_suspend() takes none, and
_phy_state_machine_post_work() invokes it after phy_state_machine() has
dropped the lock). So when the control-block write fails, this recovery
executes resume code unlocked, concurrently with phydev->lock holders such
as the state machine or phy_ethtool_set_eee(), which write the same fields
and registers it reads and rewrites. Should the recovery be restricted to
the control-block power-on, or moved to a context that holds phydev->lock?