RE: [PATCH net-next v2 4/8] net: phy: add X-Powers AC200 EPHY control driver

["Jagielski, Jedrzej" <jedrzej.jagielski@intel.com>]

From: James Hilliard <james.hilliard1@gmail.com> 
Sent: Wednesday, August 5, 2026 4:27 AM

>The AC200 Fast Ethernet PHY needs package registers in the parent I2C
>regmap to be configured before its Clause 22 endpoint becomes usable.
>
>Add a control driver which obtains the calibration value, applies the
>vendor offset, selects the documented 24 or 27 MHz input clock, programs
>the link PHY address and performs the required reset, clock and shutdown
>sequence. Use the optional SoC SID cell when supplied and otherwise read
>the AC200's internal calibration eFuse.
>
>Early PHY creation occurs before a MAC has attached, so use reset-default
>MII initially. A separate operation later applies phydev->interface; when
>the block is already powered it changes only RMII_SEL before the normal
>PHY soft reset.
>
>Expose serialized, idempotent power and interface operations to the
>common PHY driver. Enable only the MII I/O pads in this basic driver;
>dedicated LED outputs remain disabled until a later LED patch describes
>and manages them.
>
>Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
>---
> drivers/net/phy/Kconfig             |   9 ++
> drivers/net/phy/Makefile            |   1 +
> drivers/net/phy/xpowers-ac200-ctl.c | 294 ++++++++++++++++++++++++++++++++++++
> drivers/net/phy/xpowers-acx00.h     |  15 ++
> 4 files changed, 319 insertions(+)
>
>diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
>index a29d3fed8a05..6119f4de880d 100644
>--- a/drivers/net/phy/Kconfig
>+++ b/drivers/net/phy/Kconfig
>@@ -475,6 +475,15 @@ config VITESSE_PHY
> 	help
> 	  Currently supports the vsc8244
> 
>+config XPOWERS_AC200_PHY_CTL
>+	tristate "X-Powers AC200 Ethernet PHY control"
>+	depends on MFD_AC200
>+	help
>+	  Enable the control driver for the Fast Ethernet PHY function in
>+	  the X-Powers AC200 mixed-signal companion IC. It programs the PHY
>+	  address, interface mode, calibration and I/O controls through the
>+	  parent AC200 I2C regmap.
>+
> config XILINX_GMII2RGMII
> 	tristate "Xilinx GMII2RGMII converter driver"
> 	help
>diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
>index e23df5e836e9..f81854fc9f12 100644
>--- a/drivers/net/phy/Makefile
>+++ b/drivers/net/phy/Makefile
>@@ -100,4 +100,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_AC200_PHY_CTL) += xpowers-ac200-ctl.o
> obj-$(CONFIG_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o
>diff --git a/drivers/net/phy/xpowers-ac200-ctl.c b/drivers/net/phy/xpowers-ac200-ctl.c
>new file mode 100644
>index 000000000000..c69e6a8c71a4
>--- /dev/null
>+++ b/drivers/net/phy/xpowers-ac200-ctl.c
>@@ -0,0 +1,294 @@
>+// SPDX-License-Identifier: GPL-2.0-only
>+/*
>+ * X-Powers AC200 Ethernet PHY control driver
>+ *
>+ * Copyright (c) 2022 Arm Ltd. (Andre Przywara <andre.przywara@arm.com>)
>+ * Copyright (C) 2026 James Hilliard <james.hilliard1@gmail.com>

Quite unclear for me what's Andre's role here
if he worked on that shouldn't he be mentioned in the
commit msg anyhow?

>+ */
>+
>+#include <linux/bitfield.h>
>+#include <linux/clk.h>
>+#include <linux/delay.h>
>+#include <linux/module.h>
>+#include <linux/mutex.h>
>+#include <linux/nvmem-consumer.h>
>+#include <linux/phy.h>
>+#include <linux/platform_device.h>
>+#include <linux/property.h>
>+#include <linux/regmap.h>
>+
>+#include "xpowers-acx00.h"
>+
>+#define AC200_EPHY_BPS_EFFUSE_OFFSET	3
>+
>+#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 mutex lock; /* Serializes power sequencing and state. */
>+	u16 ephy_ctl;
>+	unsigned int phy_addr;
>+	phy_interface_t interface;
>+	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_power_off_locked(struct ac200_ephy_ctl *priv)
>+{
>+	int err;
>+	int ret;
>+
>+	if (!priv->powered)
>+		return 0;
>+
>+	ret = regmap_write(priv->regmap, AC200_EPHY_CTL_REG,
>+			   ac200_ephy_ctl_config(priv) | AC200_EPHY_SHUTDOWN);

how about creating wrapper logging which register write failed?
currently there's no info at the end what's not ideal when debugging
especially as the're called in a sequence

>+	err = regmap_write(priv->regmap, AC200_SYS_EPHY_CTL1_REG, 0);
>+	if (!ret)
>+		ret = err;
>+	err = regmap_write(priv->regmap, AC200_SYS_EPHY_CTL0_REG, 0);
>+	if (!ret)
>+		ret = err;
>+
>+	priv->powered = false;
>+
>+	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);
>+	int ret;
>+
>+	mutex_lock(&priv->lock);
>+	ret = ac200_ephy_ctl_power_off_locked(priv);
>+	mutex_unlock(&priv->lock);
>+
>+	return ret;
>+}
>+
>+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);
>+	u16 value;
>+	int ret = 0;
>+
>+	switch (interface) {
>+	case PHY_INTERFACE_MODE_MII:
>+		value = 0;
>+		break;
>+	case PHY_INTERFACE_MODE_RMII:
>+		value = AC200_EPHY_RMII_SEL;
>+		break;
>+	default:
>+		return -EINVAL;
>+	}

'value' can be moved into priv->powered branch since
it's used only there

>+
>+	mutex_lock(&priv->lock);
>+	if (priv->interface == interface)
>+		goto out_unlock;
>+
>+	if (priv->powered)
>+		ret = regmap_update_bits(priv->regmap, AC200_EPHY_CTL_REG,
>+					 AC200_EPHY_RMII_SEL, value);
>+	if (!ret)
>+		priv->interface = interface;
>+
>+out_unlock:
>+	mutex_unlock(&priv->lock);
>+
>+	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;
>+
>+	mutex_lock(&priv->lock);
>+	if (priv->powered && priv->phy_addr == phy_addr) {
>+		ret = 0;
>+		goto out_unlock;
>+	}
>+	if (priv->powered) {
>+		ret = ac200_ephy_ctl_power_off_locked(priv);
>+		if (ret)
>+			goto out_unlock;
>+	}
>+	priv->phy_addr = phy_addr;
>+
>+	ephy_ctl = ac200_ephy_ctl_config(priv);
>+
>+	/* Start from a disabled state before applying the configuration. */
>+	ret = regmap_write(priv->regmap, AC200_SYS_EPHY_CTL0_REG, 0);
>+	if (ret)
>+		goto err_disable;
>+
>+	ret = regmap_write(priv->regmap, AC200_SYS_EPHY_CTL1_REG,
>+			   AC200_EPHY_MII_IO_ENABLE);
>+	if (ret)
>+		goto err_disable;
>+
>+	ret = regmap_write(priv->regmap, AC200_EPHY_CTL_REG,
>+			   ephy_ctl | AC200_EPHY_SHUTDOWN);
>+	if (ret)
>+		goto err_disable;
>+
>+	ret = regmap_write(priv->regmap, AC200_SYS_EPHY_CTL0_REG,
>+			   AC200_EPHY_RESET_DEASSERT |
>+			   AC200_EPHY_SYSCLK_ENABLE);
>+	if (ret)
>+		goto err_disable;
>+

what does the value comes from?

>+	fsleep(10000);
>+
>+	ret = regmap_write(priv->regmap, AC200_EPHY_CTL_REG, ephy_ctl);
>+	if (ret)
>+		goto err_disable;
>+
>+	priv->powered = true;
>+	goto out_unlock;
>+
>+err_disable:
>+	/* Attempt every step of the shutdown sequence after a partial start. */
>+	priv->powered = true;
>+	ac200_ephy_ctl_power_off_locked(priv);
>+out_unlock:
>+	mutex_unlock(&priv->lock);
>+
>+	return ret;
>+}
>+
>+static int ac200_ephy_ctl_probe(struct platform_device *pdev)
>+{
>+	struct device *dev = &pdev->dev;
>+	struct ac200_ephy_ctl *priv;
>+	unsigned long clk_rate;
>+	unsigned int calibration;
>+	u8 nvmem_calibration;
>+	u8 bps_effuse_code;
>+	struct clk *clk;
>+	int ret;
>+
>+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>+	if (!priv)
>+		return -ENOMEM;
>+	mutex_init(&priv->lock);
>+
>+	priv->regmap = dev_get_regmap(dev->parent, NULL);
>+	if (!priv->regmap)
>+		return dev_err_probe(dev, -EPROBE_DEFER,
>+				     "parent regmap is not ready\n");
>+
>+	if (device_property_present(dev, "nvmem-cells")) {
>+		ret = nvmem_cell_read_u8(dev, "calibration",
>+					 &nvmem_calibration);
>+		if (ret)
>+			return dev_err_probe(dev, ret,
>+					     "failed to read calibration data\n");
>+		calibration = nvmem_calibration;
>+	} else {
>+		ret = regmap_read(priv->regmap, AC200_EFUSE_EPHY_REG,
>+				  &calibration);
>+		if (ret)
>+			return dev_err_probe(dev, ret,
>+					     "failed to read on-chip calibration data\n");
>+	}
>+
>+	/* 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(dev->parent, NULL);
>+	if (IS_ERR(clk))
>+		return dev_err_probe(dev, PTR_ERR(clk),
>+				     "failed to get input clock\n");
>+
>+	clk_rate = clk_get_rate(clk);
>+	clk_put(clk);
>+
>+	switch (clk_rate) {
>+	case 24000000:
>+		priv->ephy_ctl |= AC200_EPHY_CLK_SEL_24_MHZ;
>+		break;
>+	case 27000000:
>+		break;
>+	default:
>+		return dev_err_probe(dev, -EINVAL,
>+				     "unsupported input clock rate %lu Hz\n",
>+				     clk_rate);
>+	}
>+
>+	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;
>+	platform_set_drvdata(pdev, &priv->control);
>+
>+	return 0;
>+}
>+
>+static void ac200_ephy_ctl_remove(struct platform_device *pdev)
>+{
>+	struct acx00_ephy_control *control = platform_get_drvdata(pdev);
>+
>+	control->power_off(control);
>+}
>+
>+static const struct of_device_id ac200_ephy_ctl_of_match[] = {
>+	{ .compatible = "x-powers,ac200-ephy-ctl" },
>+	{ }
>+};
>+MODULE_DEVICE_TABLE(of, ac200_ephy_ctl_of_match);
>+
>+static struct platform_driver ac200_ephy_ctl_driver = {
>+	.probe = ac200_ephy_ctl_probe,
>+	.remove = ac200_ephy_ctl_remove,
>+	.shutdown = ac200_ephy_ctl_remove,
>+	.driver = {
>+		.name = "ac200-ephy-ctl",
>+		.of_match_table = ac200_ephy_ctl_of_match,
>+	},
>+};
>+module_platform_driver(ac200_ephy_ctl_driver);
>+
>+MODULE_AUTHOR("James Hilliard <james.hilliard1@gmail.com>");
>+MODULE_DESCRIPTION("X-Powers AC200 Ethernet PHY control 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..482ba6faba6e
>--- /dev/null
>+++ b/drivers/net/phy/xpowers-acx00.h
>@@ -0,0 +1,15 @@
>+/* SPDX-License-Identifier: GPL-2.0-only */
>+#ifndef __DRIVERS_NET_PHY_XPOWERS_ACX00_H
>+#define __DRIVERS_NET_PHY_XPOWERS_ACX00_H
>+
>+#include <linux/phy.h>
>+
>+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);
>+};
>+
>+#endif
>
>-- 
>2.53.0