[PATCH v2 phy-next 0/2] phy: ti: add driver for TI DS125DF111 Dual-Channel Retimer

[PATCH v2 phy-next 0/2] phy: ti: add driver for TI DS125DF111 Dual-Channel Retimer

[Ioana Ciornei]

This patch set adds a generic PHY driver and the corresponding DT
binding for the TI DS125DF111 Dual-Channel retimer. The datasheet on
which this driver was based on can be found at -
https://www.ti.com/lit/gpn/DS125DF111.

A separate generic PHY is registered for each of the two channels of the
retimer, so consumers can drive each channel independently. This allows
for independent control of the channels, which is especially important
since each channel can be routed to different SerDes lanes and it is not
guaranteed that the same retimer will do both directions of SerDes lane.

This was tested on a LS1088ARDB board with the Lynx10G SerDes PHY driver
yet to be submitted.

Changes in v2:
- Remove the label from the example
- Rename the node from 'retimer' to 'phy'
- Explicitly include all the needed headers
- Change ds125df111_xlate() so that it returns an error if args_count is
not exactly 1
- Add a MAINTAINERS entry
- Link to v1: https://lore.kernel.org/all/20260513185103.1371809-1-ioana.ciornei@nxp.com/

Ioana Ciornei (2):
  dt-bindings: phy: add PHY bindings for the TI DS125DF111 Retimer PHY
  phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer

 .../bindings/phy/ti,ds125df111.yaml           |  46 ++++
 MAINTAINERS                                   |   7 +
 drivers/phy/ti/Kconfig                        |  10 +
 drivers/phy/ti/Makefile                       |   1 +
 drivers/phy/ti/phy-ds125df111.c               | 252 ++++++++++++++++++
 5 files changed, 316 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/ti,ds125df111.yaml
 create mode 100644 drivers/phy/ti/phy-ds125df111.c

-- 
2.25.1

[PATCH v2 phy-next 1/2] dt-bindings: phy: add PHY bindings for the TI DS125DF111 Retimer PHY

[Ioana Ciornei]

Add device tree binding for the TI DS125DF111 Retimer PHY.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
Changes in v2:
- Remove the label from the example
- Rename the node from 'retimer' to 'phy'
---
 .../bindings/phy/ti,ds125df111.yaml           | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/ti,ds125df111.yaml

diff --git a/Documentation/devicetree/bindings/phy/ti,ds125df111.yaml b/Documentation/devicetree/bindings/phy/ti,ds125df111.yaml
new file mode 100644
index 000000000000..ca4605f1d664
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/ti,ds125df111.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/ti,ds125df111.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI DS125DF111 Retimer PHY
+
+description:
+  This binding describes the TI DS125DF111 multi-protocol Retimer PHY.
+
+maintainers:
+  - Ioana Ciornei <ioana.ciornei@nxp.com>
+
+properties:
+  compatible:
+    const: ti,ds125df111
+
+  reg:
+    maxItems: 1
+
+  "#phy-cells":
+    const: 1
+    description: |
+      The phandle's argument in the PHY specifier selects one of the two
+      channels of the retimer
+
+required:
+  - compatible
+  - reg
+  - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      phy@18 {
+        compatible = "ti,ds125df111";
+        reg = <0x18>;
+        #phy-cells = <1>;
+      };
+    };
-- 
2.25.1

[PATCH v2 phy-next 2/2] phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer

[Ioana Ciornei]

Add a generic PHY driver for the TI DS125DF111 Multi-Protocol
Dual-Channel Retimer. The driver currently supports only 10G and 1G link
speeds but it can easily extended to also cover other usecases.

Since the available datasheet (https://www.ti.com/lit/gpn/DS125DF111)
does not name the registers, the name for the macros were determined by
their usage pattern.

A PHY device is created for each of the two channels present on the
retimer. This allows for independent configuration of the two channels.
This capability is especially important on retimers which have more than
2 channels that can be, depending on the board design, connected in
multiple different ways to the SerDes lanes.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
Changes in v2:
- Explicitly include all the needed headers
- Change ds125df111_xlate() so that it returns an error if args_count is
not exactly 1
- Add a MAINTAINERS entry
---
 MAINTAINERS                     |   7 +
 drivers/phy/ti/Kconfig          |  10 ++
 drivers/phy/ti/Makefile         |   1 +
 drivers/phy/ti/phy-ds125df111.c | 252 ++++++++++++++++++++++++++++++++
 4 files changed, 270 insertions(+)
 create mode 100644 drivers/phy/ti/phy-ds125df111.c

diff --git a/MAINTAINERS b/MAINTAINERS
index f877e5aaf2c7..58f410b666e7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26781,6 +26781,13 @@ T:	git git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git
 F:	drivers/media/platform/ti/davinci/
 F:	include/media/davinci/
 
+TI DS125DF111 RETIMER PHY DRIVER
+M:	Ioana Ciornei <ioana.ciornei@nxp.com>
+L:	linux-phy@lists.infradead.org (moderated for non-subscribers)
+S:	Maintained
+F:	Documentation/devicetree/bindings/phy/ti,ds125df111.yaml
+F:	drivers/phy/ti/phy-ds125df111.c
+
 TI ENHANCED CAPTURE (eCAP) DRIVER
 M:	Vignesh Raghavendra <vigneshr@ti.com>
 R:	Julien Panis <jpanis@baylibre.com>
diff --git a/drivers/phy/ti/Kconfig b/drivers/phy/ti/Kconfig
index b40f28019131..475e80fcd52d 100644
--- a/drivers/phy/ti/Kconfig
+++ b/drivers/phy/ti/Kconfig
@@ -111,3 +111,13 @@ config PHY_TI_GMII_SEL
 	help
 	  This driver supports configuring of the TI CPSW Port mode depending on
 	  the Ethernet PHY connected to the CPSW Port.
+
+config PHY_TI_DS125DF111
+	tristate "DS125DF111 2-Channel Retimer Driver"
+	depends on OF && I2C
+	select GENERIC_PHY
+	help
+	  Enable this to add support for configuration and runtime management
+	  of the TI DS125DF111 Multi-Protocol 2-Channel Retimer.
+	  The retimer is modeled as a Generic PHY and supports both 10G and 1G
+	  link speeds.
diff --git a/drivers/phy/ti/Makefile b/drivers/phy/ti/Makefile
index dcba2571c9bd..e68445ddd848 100644
--- a/drivers/phy/ti/Makefile
+++ b/drivers/phy/ti/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
 obj-$(CONFIG_PHY_AM654_SERDES)		+= phy-am654-serdes.o
 obj-$(CONFIG_PHY_TI_GMII_SEL)		+= phy-gmii-sel.o
 obj-$(CONFIG_PHY_J721E_WIZ)		+= phy-j721e-wiz.o
+obj-$(CONFIG_PHY_TI_DS125DF111)		+= phy-ds125df111.o
diff --git a/drivers/phy/ti/phy-ds125df111.c b/drivers/phy/ti/phy-ds125df111.c
new file mode 100644
index 000000000000..084a94e655c2
--- /dev/null
+++ b/drivers/phy/ti/phy-ds125df111.c
@@ -0,0 +1,252 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright 2026 NXP */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/phy.h>
+#include <linux/phy/phy.h>
+#include <linux/slab.h>
+
+#define DS125DF111_NUM_CH			2
+#define DS125DF111_NUM_VCO_GROUP_REG		5
+
+#define DS125DF111_CH_SELECT			0xff
+#define DS125DF111_CH_SELECT_TARGET_MASK	GENMASK(3, 0)
+#define DS125DF111_CH_SELECT_EN			BIT(2)
+
+#define DS125DF111_CH_CTRL			0x00
+#define DS125DF111_CH_CTRL_RESET		BIT(2) /* self clearing */
+
+#define DS125DF111_VCO_GROUP_BASE		0x60
+
+#define DS125DF111_RATIOS			0x2F
+#define DS125DF111_RATIOS_RATE_MASK		GENMASK(7, 6)
+#define DS125DF111_RATIOS_SUBRATE_MASK		GENMASK(5, 4)
+
+struct ds125df111_ch {
+	struct phy *phy;
+	struct ds125df111_priv *priv;
+	int idx;
+};
+
+struct ds125df111_priv {
+	struct ds125df111_ch ch[DS125DF111_NUM_CH];
+	struct i2c_client *client;
+	struct mutex mutex; /* protects access to shared registers */
+};
+
+enum ds125df111_mode {
+	FREQ_1G,
+	FREQ_10G,
+};
+
+static const struct ds125df111_config {
+	u8 vco_group[DS125DF111_NUM_VCO_GROUP_REG];
+	u8 rate;
+	u8 subrate;
+} ds125df111_cfg[] = {
+	[FREQ_1G] = {
+		/* VCO group #0 = 10GHz, VCO group #1 = 10GHz */
+		.vco_group = {0x00, 0xB2, 0x00, 0xB2, 0xCC},
+		/* By using the following combination of rate and subrate we
+		 * select divide ratios of 1, 2, 4, 8 on both groups
+		 */
+		.rate = 0x1,
+		.subrate = 0x2,
+	},
+
+	[FREQ_10G] = {
+		/* VCO group #0 = 10.3125GHz, VCO group #1 = 10.3125GHz */
+		.vco_group = {0x90, 0xB3, 0x90, 0xB3, 0xCD},
+		/* By using the following combination of rate and subrate we
+		 * select divide ratios of 1 on both groups
+		 */
+		.rate = 0x1,
+		.subrate = 0x3,
+	},
+};
+
+static int ds125df111_configure(struct phy *phy,
+				const struct ds125df111_config *cfg)
+{
+	struct ds125df111_ch *ch = phy_get_drvdata(phy);
+	struct ds125df111_priv *priv = ch->priv;
+	struct i2c_client *i2c = priv->client;
+	struct device *dev = &phy->dev;
+	u8 val;
+	int err, i;
+
+	mutex_lock(&priv->mutex);
+
+	/* Make sure that any subsequent read/write operation will be directed
+	 * only to the registers of the selected channel
+	 */
+	err = i2c_smbus_read_byte_data(i2c, DS125DF111_CH_SELECT);
+	if (err < 0) {
+		dev_err(dev, "Unable to select channel\n");
+		goto out;
+	}
+	val = (u8)err;
+	val &= ~DS125DF111_CH_SELECT_TARGET_MASK;
+	val |= DS125DF111_CH_SELECT_EN | ch->idx;
+	err = i2c_smbus_write_byte_data(i2c, DS125DF111_CH_SELECT, val);
+	if (err < 0) {
+		dev_err(dev, "Unable to select channel\n");
+		goto out;
+	}
+
+	/* Reset Channel Registers */
+	err = i2c_smbus_read_byte_data(i2c, DS125DF111_CH_CTRL);
+	if (err < 0) {
+		dev_err(dev, "Error resetting channel configuration\n");
+		goto out;
+	}
+	val = (u8)err;
+	val |= DS125DF111_CH_CTRL_RESET;
+	err = i2c_smbus_write_byte_data(i2c, DS125DF111_CH_CTRL, val);
+	if (err < 0) {
+		dev_err(dev, "Error resetting channel configuration\n");
+		goto out;
+	}
+
+	/* Program the VCO group frequencies */
+	for (i = 0; i < DS125DF111_NUM_VCO_GROUP_REG; i++) {
+		err = i2c_smbus_write_byte_data(i2c,
+						DS125DF111_VCO_GROUP_BASE + i,
+						cfg->vco_group[i]);
+		if (err < 0) {
+			dev_err(dev, "Error programming VCO group frequencies\n");
+			goto out;
+		}
+	}
+
+	/* Set the Divide Ratios for the VCO Groups*/
+	err = i2c_smbus_read_byte_data(i2c, DS125DF111_RATIOS);
+	if (err < 0) {
+		dev_err(dev, "Error programming the divide ratios\n");
+		goto out;
+	}
+	val = (u8)err;
+	val &= ~(DS125DF111_RATIOS_RATE_MASK | DS125DF111_RATIOS_SUBRATE_MASK);
+	val |= FIELD_PREP(DS125DF111_RATIOS_RATE_MASK, cfg->rate) |
+		FIELD_PREP(DS125DF111_RATIOS_SUBRATE_MASK, cfg->subrate);
+	err = i2c_smbus_write_byte_data(i2c, DS125DF111_RATIOS, val);
+	if (err < 0) {
+		dev_err(dev, "Error programming the divide ratios\n");
+		goto out;
+	}
+
+	mutex_unlock(&priv->mutex);
+
+	return 0;
+
+out:
+	mutex_unlock(&priv->mutex);
+
+	return err;
+}
+
+static int ds125df111_set_mode(struct phy *phy, enum phy_mode mode, int submode)
+{
+	const struct ds125df111_config *cfg;
+
+	if (mode != PHY_MODE_ETHERNET)
+		return -EOPNOTSUPP;
+
+	switch (submode) {
+	case PHY_INTERFACE_MODE_10GBASER:
+		cfg = &ds125df111_cfg[FREQ_10G];
+		break;
+	case PHY_INTERFACE_MODE_1000BASEX:
+	case PHY_INTERFACE_MODE_SGMII:
+		cfg = &ds125df111_cfg[FREQ_1G];
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return ds125df111_configure(phy, cfg);
+}
+
+static const struct phy_ops ds125df111_ops = {
+	.set_mode	= ds125df111_set_mode,
+	.owner		= THIS_MODULE,
+};
+
+static struct phy *ds125df111_xlate(struct device *dev,
+				    const struct of_phandle_args *args)
+{
+	struct ds125df111_priv *priv = dev_get_drvdata(dev);
+	u32 idx;
+
+	if (args->args_count != 1)
+		return ERR_PTR(-EINVAL);
+
+	idx = args->args[0];
+	if (idx >= DS125DF111_NUM_CH) {
+		dev_err(dev, "Maximum number of channels is %d\n",
+			DS125DF111_NUM_CH);
+		return ERR_PTR(-EINVAL);
+	}
+
+	return priv->ch[idx].phy;
+}
+
+static int ds125df111_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct phy_provider *provider;
+	struct ds125df111_priv *priv;
+	int i, err;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	priv->client = client;
+	err = devm_mutex_init(dev, &priv->mutex);
+	if (err)
+		return err;
+
+	i2c_set_clientdata(client, priv);
+
+	for (i = 0; i < DS125DF111_NUM_CH; i++) {
+		struct ds125df111_ch *ch = &priv->ch[i];
+		struct phy *phy;
+
+		phy = devm_phy_create(dev, NULL, &ds125df111_ops);
+		if (IS_ERR(phy))
+			return PTR_ERR(phy);
+
+		ch->idx = i;
+		ch->priv = priv;
+		ch->phy = phy;
+
+		phy_set_drvdata(phy, ch);
+	}
+
+	provider = devm_of_phy_provider_register(dev, ds125df111_xlate);
+
+	return PTR_ERR_OR_ZERO(provider);
+}
+
+static const struct of_device_id ds125df111_dt_ids[] = {
+	{ .compatible = "ti,ds125df111", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ds125df111_dt_ids);
+
+static struct i2c_driver ds125df111_driver = {
+	.driver = {
+		.name = "ds125df111",
+		.of_match_table = ds125df111_dt_ids,
+	},
+	.probe = ds125df111_probe,
+};
+module_i2c_driver(ds125df111_driver);
+
+MODULE_AUTHOR("Ioana Ciornei <ioana.ciornei@nxp.com>");
+MODULE_DESCRIPTION("TI DS125DF111 Retimer driver");
+MODULE_LICENSE("GPL");
-- 
2.25.1

Re: [PATCH v2 phy-next 2/2] phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing delay or polling after setting a self-clearing hardware reset bit.
--

commit 59a5ca7411308a81c146d660b8c04e0be8fe8fbb
Author: Ioana Ciornei <ioana.ciornei@nxp.com>

phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer

This commit adds a generic PHY driver for the TI DS125DF111 Multi-Protocol
Dual-Channel Retimer. It supports 10G and 1G link speeds, modeling each of
the two channels as a separate PHY device.

> diff --git a/drivers/phy/ti/phy-ds125df111.c b/drivers/phy/ti/phy-ds125df111.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/phy/ti/phy-ds125df111.c

[ ... ]

> +static int ds125df111_configure(struct phy *phy,
> +				const struct ds125df111_config *cfg)
> +{

[ ... ]

> +	/* Reset Channel Registers */
> +	err = i2c_smbus_read_byte_data(i2c, DS125DF111_CH_CTRL);
> +	if (err < 0) {
> +		dev_err(dev, "Error resetting channel configuration\n");
> +		goto out;
> +	}
> +	val = (u8)err;
> +	val |= DS125DF111_CH_CTRL_RESET;
> +	err = i2c_smbus_write_byte_data(i2c, DS125DF111_CH_CTRL, val);
> +	if (err < 0) {
> +		dev_err(dev, "Error resetting channel configuration\n");
> +		goto out;
> +	}
> +
> +	/* Program the VCO group frequencies */
> +	for (i = 0; i < DS125DF111_NUM_VCO_GROUP_REG; i++) {
> +		err = i2c_smbus_write_byte_data(i2c,
> +						DS125DF111_VCO_GROUP_BASE + i,
> +						cfg->vco_group[i]);

Is a delay or polling loop needed after triggering the channel reset?

The DS125DF111_CH_CTRL_RESET bit is defined as self-clearing, but the code 
immediately proceeds to program the VCO group frequencies without checking if
the reset has finished.

Since hardware resets usually take a non-zero amount of time, writing to the
registers right away could result in the device NACKing the transactions or
silently dropping the new configuration.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515110145.1925579-1-ioana.ciornei@nxp.com?part=2

Re: [PATCH v2 phy-next 2/2] phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer

[Ioana Ciornei]

On Fri, May 15, 2026 at 11:22:52AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Missing delay or polling after setting a self-clearing hardware reset bit.
> --
> 
> commit 59a5ca7411308a81c146d660b8c04e0be8fe8fbb
> Author: Ioana Ciornei <ioana.ciornei@nxp.com>
> 
> phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer
> 
> This commit adds a generic PHY driver for the TI DS125DF111 Multi-Protocol
> Dual-Channel Retimer. It supports 10G and 1G link speeds, modeling each of
> the two channels as a separate PHY device.
> 
> > diff --git a/drivers/phy/ti/phy-ds125df111.c b/drivers/phy/ti/phy-ds125df111.c
> > new file mode 100644
> > --- /dev/null
> > +++ b/drivers/phy/ti/phy-ds125df111.c
> 
> [ ... ]
> 
> > +static int ds125df111_configure(struct phy *phy,
> > +				const struct ds125df111_config *cfg)
> > +{
> 
> [ ... ]
> 
> > +	/* Reset Channel Registers */
> > +	err = i2c_smbus_read_byte_data(i2c, DS125DF111_CH_CTRL);
> > +	if (err < 0) {
> > +		dev_err(dev, "Error resetting channel configuration\n");
> > +		goto out;
> > +	}
> > +	val = (u8)err;
> > +	val |= DS125DF111_CH_CTRL_RESET;
> > +	err = i2c_smbus_write_byte_data(i2c, DS125DF111_CH_CTRL, val);
> > +	if (err < 0) {
> > +		dev_err(dev, "Error resetting channel configuration\n");
> > +		goto out;
> > +	}
> > +
> > +	/* Program the VCO group frequencies */
> > +	for (i = 0; i < DS125DF111_NUM_VCO_GROUP_REG; i++) {
> > +		err = i2c_smbus_write_byte_data(i2c,
> > +						DS125DF111_VCO_GROUP_BASE + i,
> > +						cfg->vco_group[i]);
> 
> Is a delay or polling loop needed after triggering the channel reset?
> 
> The DS125DF111_CH_CTRL_RESET bit is defined as self-clearing, but the code 
> immediately proceeds to program the VCO group frequencies without checking if
> the reset has finished.
> 
> Since hardware resets usually take a non-zero amount of time, writing to the
> registers right away could result in the device NACKing the transactions or
> silently dropping the new configuration.
> 

The datasheet does not specify a maximum timeout for channel reset. I
will try to use read_poll_timeout() to wait for the DS125DF111_CH_CTRL
bit to be cleared.

Re: [PATCH v2 phy-next 2/2] phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer

[Vladimir Oltean]

On Fri, May 15, 2026 at 02:01:45PM +0300, Ioana Ciornei wrote:
> Add a generic PHY driver for the TI DS125DF111 Multi-Protocol
> Dual-Channel Retimer. The driver currently supports only 10G and 1G link
> speeds but it can easily extended to also cover other usecases.
> 
> Since the available datasheet (https://www.ti.com/lit/gpn/DS125DF111)
> does not name the registers, the name for the macros were determined by
> their usage pattern.
> 
> A PHY device is created for each of the two channels present on the
> retimer. This allows for independent configuration of the two channels.
> This capability is especially important on retimers which have more than
> 2 channels that can be, depending on the board design, connected in
> multiple different ways to the SerDes lanes.
> 
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
> Changes in v2:
> - Explicitly include all the needed headers
> - Change ds125df111_xlate() so that it returns an error if args_count is
> not exactly 1
> - Add a MAINTAINERS entry
> ---
>  MAINTAINERS                     |   7 +
>  drivers/phy/ti/Kconfig          |  10 ++
>  drivers/phy/ti/Makefile         |   1 +
>  drivers/phy/ti/phy-ds125df111.c | 252 ++++++++++++++++++++++++++++++++
>  4 files changed, 270 insertions(+)
>  create mode 100644 drivers/phy/ti/phy-ds125df111.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f877e5aaf2c7..58f410b666e7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -26781,6 +26781,13 @@ T:	git git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git
>  F:	drivers/media/platform/ti/davinci/
>  F:	include/media/davinci/
>  
> +TI DS125DF111 RETIMER PHY DRIVER
> +M:	Ioana Ciornei <ioana.ciornei@nxp.com>
> +L:	linux-phy@lists.infradead.org (moderated for non-subscribers)
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/phy/ti,ds125df111.yaml
> +F:	drivers/phy/ti/phy-ds125df111.c
> +
>  TI ENHANCED CAPTURE (eCAP) DRIVER
>  M:	Vignesh Raghavendra <vigneshr@ti.com>
>  R:	Julien Panis <jpanis@baylibre.com>
> diff --git a/drivers/phy/ti/Kconfig b/drivers/phy/ti/Kconfig
> index b40f28019131..475e80fcd52d 100644
> --- a/drivers/phy/ti/Kconfig
> +++ b/drivers/phy/ti/Kconfig
> @@ -111,3 +111,13 @@ config PHY_TI_GMII_SEL
>  	help
>  	  This driver supports configuring of the TI CPSW Port mode depending on
>  	  the Ethernet PHY connected to the CPSW Port.
> +
> +config PHY_TI_DS125DF111
> +	tristate "DS125DF111 2-Channel Retimer Driver"
> +	depends on OF && I2C
> +	select GENERIC_PHY
> +	help
> +	  Enable this to add support for configuration and runtime management
> +	  of the TI DS125DF111 Multi-Protocol 2-Channel Retimer.
> +	  The retimer is modeled as a Generic PHY and supports both 10G and 1G
> +	  link speeds.
> diff --git a/drivers/phy/ti/Makefile b/drivers/phy/ti/Makefile
> index dcba2571c9bd..e68445ddd848 100644
> --- a/drivers/phy/ti/Makefile
> +++ b/drivers/phy/ti/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
>  obj-$(CONFIG_PHY_AM654_SERDES)		+= phy-am654-serdes.o
>  obj-$(CONFIG_PHY_TI_GMII_SEL)		+= phy-gmii-sel.o
>  obj-$(CONFIG_PHY_J721E_WIZ)		+= phy-j721e-wiz.o
> +obj-$(CONFIG_PHY_TI_DS125DF111)		+= phy-ds125df111.o
> diff --git a/drivers/phy/ti/phy-ds125df111.c b/drivers/phy/ti/phy-ds125df111.c
> new file mode 100644
> index 000000000000..084a94e655c2
> --- /dev/null
> +++ b/drivers/phy/ti/phy-ds125df111.c
> @@ -0,0 +1,252 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright 2026 NXP */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/phy.h>
> +#include <linux/phy/phy.h>
> +#include <linux/slab.h>
> +
> +#define DS125DF111_NUM_CH			2
> +#define DS125DF111_NUM_VCO_GROUP_REG		5
> +
> +#define DS125DF111_CH_SELECT			0xff
> +#define DS125DF111_CH_SELECT_TARGET_MASK	GENMASK(3, 0)
> +#define DS125DF111_CH_SELECT_EN			BIT(2)
> +
> +#define DS125DF111_CH_CTRL			0x00
> +#define DS125DF111_CH_CTRL_RESET		BIT(2) /* self clearing */
> +
> +#define DS125DF111_VCO_GROUP_BASE		0x60
> +
> +#define DS125DF111_RATIOS			0x2F
> +#define DS125DF111_RATIOS_RATE_MASK		GENMASK(7, 6)
> +#define DS125DF111_RATIOS_SUBRATE_MASK		GENMASK(5, 4)
> +
> +struct ds125df111_ch {
> +	struct phy *phy;
> +	struct ds125df111_priv *priv;
> +	int idx;
> +};
> +
> +struct ds125df111_priv {
> +	struct ds125df111_ch ch[DS125DF111_NUM_CH];
> +	struct i2c_client *client;
> +	struct mutex mutex; /* protects access to shared registers */
> +};
> +
> +enum ds125df111_mode {
> +	FREQ_1G,
> +	FREQ_10G,
> +};
> +
> +static const struct ds125df111_config {
> +	u8 vco_group[DS125DF111_NUM_VCO_GROUP_REG];
> +	u8 rate;
> +	u8 subrate;
> +} ds125df111_cfg[] = {
> +	[FREQ_1G] = {
> +		/* VCO group #0 = 10GHz, VCO group #1 = 10GHz */
> +		.vco_group = {0x00, 0xB2, 0x00, 0xB2, 0xCC},
> +		/* By using the following combination of rate and subrate we
> +		 * select divide ratios of 1, 2, 4, 8 on both groups
> +		 */
> +		.rate = 0x1,
> +		.subrate = 0x2,
> +	},
> +
> +	[FREQ_10G] = {
> +		/* VCO group #0 = 10.3125GHz, VCO group #1 = 10.3125GHz */
> +		.vco_group = {0x90, 0xB3, 0x90, 0xB3, 0xCD},
> +		/* By using the following combination of rate and subrate we
> +		 * select divide ratios of 1 on both groups
> +		 */
> +		.rate = 0x1,
> +		.subrate = 0x3,
> +	},
> +};
> +
> +static int ds125df111_configure(struct phy *phy,
> +				const struct ds125df111_config *cfg)
> +{
> +	struct ds125df111_ch *ch = phy_get_drvdata(phy);
> +	struct ds125df111_priv *priv = ch->priv;
> +	struct i2c_client *i2c = priv->client;
> +	struct device *dev = &phy->dev;
> +	u8 val;
> +	int err, i;

Not mandatory, but if the rest of the file uses reverse Christmas tree
variable ordering, could you stick to that here as well?

> +
> +	mutex_lock(&priv->mutex);
> +
> +	/* Make sure that any subsequent read/write operation will be directed
> +	 * only to the registers of the selected channel
> +	 */
> +	err = i2c_smbus_read_byte_data(i2c, DS125DF111_CH_SELECT);
> +	if (err < 0) {
> +		dev_err(dev, "Unable to select channel\n");

Here and everywhere else: could you please print a symbolic description
of the error? %pe, ERR_PTR(err).

> +		goto out;
> +	}
> +	val = (u8)err;
> +	val &= ~DS125DF111_CH_SELECT_TARGET_MASK;
> +	val |= DS125DF111_CH_SELECT_EN | ch->idx;
> +	err = i2c_smbus_write_byte_data(i2c, DS125DF111_CH_SELECT, val);
> +	if (err < 0) {
> +		dev_err(dev, "Unable to select channel\n");
> +		goto out;
> +	}
> +
> +	/* Reset Channel Registers */
> +	err = i2c_smbus_read_byte_data(i2c, DS125DF111_CH_CTRL);
> +	if (err < 0) {
> +		dev_err(dev, "Error resetting channel configuration\n");
> +		goto out;
> +	}
> +	val = (u8)err;
> +	val |= DS125DF111_CH_CTRL_RESET;
> +	err = i2c_smbus_write_byte_data(i2c, DS125DF111_CH_CTRL, val);
> +	if (err < 0) {
> +		dev_err(dev, "Error resetting channel configuration\n");
> +		goto out;
> +	}

Did you consider simplifying this function using a ds125df111_rmw() helper?
All configuration accesses except the VCO group frequencies are
read-modify-write.

> +
> +	/* Program the VCO group frequencies */
> +	for (i = 0; i < DS125DF111_NUM_VCO_GROUP_REG; i++) {
> +		err = i2c_smbus_write_byte_data(i2c,
> +						DS125DF111_VCO_GROUP_BASE + i,
> +						cfg->vco_group[i]);
> +		if (err < 0) {
> +			dev_err(dev, "Error programming VCO group frequencies\n");
> +			goto out;
> +		}
> +	}
> +
> +	/* Set the Divide Ratios for the VCO Groups*/

Space between Groups and */
Also, Divide Ratios, Groups, Channel Registers are not proper nouns,
they don't need to be capitalized.

> +	err = i2c_smbus_read_byte_data(i2c, DS125DF111_RATIOS);
> +	if (err < 0) {
> +		dev_err(dev, "Error programming the divide ratios\n");
> +		goto out;
> +	}
> +	val = (u8)err;
> +	val &= ~(DS125DF111_RATIOS_RATE_MASK | DS125DF111_RATIOS_SUBRATE_MASK);
> +	val |= FIELD_PREP(DS125DF111_RATIOS_RATE_MASK, cfg->rate) |
> +		FIELD_PREP(DS125DF111_RATIOS_SUBRATE_MASK, cfg->subrate);
> +	err = i2c_smbus_write_byte_data(i2c, DS125DF111_RATIOS, val);
> +	if (err < 0) {
> +		dev_err(dev, "Error programming the divide ratios\n");
> +		goto out;
> +	}
> +
> +	mutex_unlock(&priv->mutex);
> +
> +	return 0;
> +
> +out:
> +	mutex_unlock(&priv->mutex);
> +
> +	return err;

You don't need a separate code path for the 'out' label, it can be
common with the normal exit path (err will be 0).

> +}
> +
> +static int ds125df111_set_mode(struct phy *phy, enum phy_mode mode, int submode)
> +{
> +	const struct ds125df111_config *cfg;
> +
> +	if (mode != PHY_MODE_ETHERNET)
> +		return -EOPNOTSUPP;

Please use a different error code like -EINVAL. Let -EOPNOTSUPP mean
that the function is not implemented (when calling phy_set_mode_ext()).

> +
> +	switch (submode) {
> +	case PHY_INTERFACE_MODE_10GBASER:
> +		cfg = &ds125df111_cfg[FREQ_10G];
> +		break;
> +	case PHY_INTERFACE_MODE_1000BASEX:
> +	case PHY_INTERFACE_MODE_SGMII:
> +		cfg = &ds125df111_cfg[FREQ_1G];
> +		break;
> +	default:
> +		return -EOPNOTSUPP;

Same here.

> +	}
> +
> +	return ds125df111_configure(phy, cfg);
> +}
> +
> +static const struct phy_ops ds125df111_ops = {
> +	.set_mode	= ds125df111_set_mode,

Can you please implement .validate() as well? It will be made mandatory
in the future for those who implement .set_mode().

> +	.owner		= THIS_MODULE,
> +};
> +
> +static struct phy *ds125df111_xlate(struct device *dev,
> +				    const struct of_phandle_args *args)
> +{
> +	struct ds125df111_priv *priv = dev_get_drvdata(dev);
> +	u32 idx;
> +
> +	if (args->args_count != 1)
> +		return ERR_PTR(-EINVAL);
> +
> +	idx = args->args[0];
> +	if (idx >= DS125DF111_NUM_CH) {
> +		dev_err(dev, "Maximum number of channels is %d\n",
> +			DS125DF111_NUM_CH);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	return priv->ch[idx].phy;
> +}
> +
> +static int ds125df111_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct phy_provider *provider;
> +	struct ds125df111_priv *priv;
> +	int i, err;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +	priv->client = client;
> +	err = devm_mutex_init(dev, &priv->mutex);
> +	if (err)
> +		return err;
> +
> +	i2c_set_clientdata(client, priv);
> +
> +	for (i = 0; i < DS125DF111_NUM_CH; i++) {
> +		struct ds125df111_ch *ch = &priv->ch[i];
> +		struct phy *phy;
> +
> +		phy = devm_phy_create(dev, NULL, &ds125df111_ops);
> +		if (IS_ERR(phy))
> +			return PTR_ERR(phy);
> +
> +		ch->idx = i;
> +		ch->priv = priv;
> +		ch->phy = phy;
> +
> +		phy_set_drvdata(phy, ch);
> +	}
> +
> +	provider = devm_of_phy_provider_register(dev, ds125df111_xlate);
> +
> +	return PTR_ERR_OR_ZERO(provider);
> +}
> +
> +static const struct of_device_id ds125df111_dt_ids[] = {
> +	{ .compatible = "ti,ds125df111", },
> +	{},

Unnecessary comma after sentinel entry.

> +};
> +MODULE_DEVICE_TABLE(of, ds125df111_dt_ids);
> +
> +static struct i2c_driver ds125df111_driver = {
> +	.driver = {
> +		.name = "ds125df111",
> +		.of_match_table = ds125df111_dt_ids,
> +	},
> +	.probe = ds125df111_probe,
> +};
> +module_i2c_driver(ds125df111_driver);
> +
> +MODULE_AUTHOR("Ioana Ciornei <ioana.ciornei@nxp.com>");
> +MODULE_DESCRIPTION("TI DS125DF111 Retimer driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.25.1
> 
> 

Re: [PATCH v2 phy-next 2/2] phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer

[Ioana Ciornei]

On Fri, May 15, 2026 at 03:24:18PM +0300, Vladimir Oltean wrote:
> On Fri, May 15, 2026 at 02:01:45PM +0300, Ioana Ciornei wrote:
> > Add a generic PHY driver for the TI DS125DF111 Multi-Protocol
> > Dual-Channel Retimer. The driver currently supports only 10G and 1G link
> > speeds but it can easily extended to also cover other usecases.
> > 
> > Since the available datasheet (https://www.ti.com/lit/gpn/DS125DF111)
> > does not name the registers, the name for the macros were determined by
> > their usage pattern.
> > 
> > A PHY device is created for each of the two channels present on the
> > retimer. This allows for independent configuration of the two channels.
> > This capability is especially important on retimers which have more than
> > 2 channels that can be, depending on the board design, connected in
> > multiple different ways to the SerDes lanes.
> > 
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > ---
> > Changes in v2:
> > - Explicitly include all the needed headers
> > - Change ds125df111_xlate() so that it returns an error if args_count is
> > not exactly 1
> > - Add a MAINTAINERS entry
> > ---
> >  MAINTAINERS                     |   7 +
> >  drivers/phy/ti/Kconfig          |  10 ++
> >  drivers/phy/ti/Makefile         |   1 +
> >  drivers/phy/ti/phy-ds125df111.c | 252 ++++++++++++++++++++++++++++++++
> >  4 files changed, 270 insertions(+)
> >  create mode 100644 drivers/phy/ti/phy-ds125df111.c
> > 

(...)

> > +static int ds125df111_configure(struct phy *phy,
> > +				const struct ds125df111_config *cfg)
> > +{
> > +	struct ds125df111_ch *ch = phy_get_drvdata(phy);
> > +	struct ds125df111_priv *priv = ch->priv;
> > +	struct i2c_client *i2c = priv->client;
> > +	struct device *dev = &phy->dev;
> > +	u8 val;
> > +	int err, i;
> 
> Not mandatory, but if the rest of the file uses reverse Christmas tree
> variable ordering, could you stick to that here as well?

Ok, sure.

> 
> > +
> > +	mutex_lock(&priv->mutex);
> > +
> > +	/* Make sure that any subsequent read/write operation will be directed
> > +	 * only to the registers of the selected channel
> > +	 */
> > +	err = i2c_smbus_read_byte_data(i2c, DS125DF111_CH_SELECT);
> > +	if (err < 0) {
> > +		dev_err(dev, "Unable to select channel\n");
> 
> Here and everywhere else: could you please print a symbolic description
> of the error? %pe, ERR_PTR(err).

Nice, I didn't know about the %pe.

> 
> > +		goto out;
> > +	}
> > +	val = (u8)err;
> > +	val &= ~DS125DF111_CH_SELECT_TARGET_MASK;
> > +	val |= DS125DF111_CH_SELECT_EN | ch->idx;
> > +	err = i2c_smbus_write_byte_data(i2c, DS125DF111_CH_SELECT, val);
> > +	if (err < 0) {
> > +		dev_err(dev, "Unable to select channel\n");
> > +		goto out;
> > +	}
> > +
> > +	/* Reset Channel Registers */
> > +	err = i2c_smbus_read_byte_data(i2c, DS125DF111_CH_CTRL);
> > +	if (err < 0) {
> > +		dev_err(dev, "Error resetting channel configuration\n");
> > +		goto out;
> > +	}
> > +	val = (u8)err;
> > +	val |= DS125DF111_CH_CTRL_RESET;
> > +	err = i2c_smbus_write_byte_data(i2c, DS125DF111_CH_CTRL, val);
> > +	if (err < 0) {
> > +		dev_err(dev, "Error resetting channel configuration\n");
> > +		goto out;
> > +	}
> 
> Did you consider simplifying this function using a ds125df111_rmw() helper?
> All configuration accesses except the VCO group frequencies are
> read-modify-write.
> 

I will look into a _rmw helper and see if it helps.

> > +
> > +	/* Program the VCO group frequencies */
> > +	for (i = 0; i < DS125DF111_NUM_VCO_GROUP_REG; i++) {
> > +		err = i2c_smbus_write_byte_data(i2c,
> > +						DS125DF111_VCO_GROUP_BASE + i,
> > +						cfg->vco_group[i]);
> > +		if (err < 0) {
> > +			dev_err(dev, "Error programming VCO group frequencies\n");
> > +			goto out;
> > +		}
> > +	}
> > +
> > +	/* Set the Divide Ratios for the VCO Groups*/
> 
> Space between Groups and */
> Also, Divide Ratios, Groups, Channel Registers are not proper nouns,
> they don't need to be capitalized.

Ok, will fix.

> 
> > +	err = i2c_smbus_read_byte_data(i2c, DS125DF111_RATIOS);
> > +	if (err < 0) {
> > +		dev_err(dev, "Error programming the divide ratios\n");
> > +		goto out;
> > +	}
> > +	val = (u8)err;
> > +	val &= ~(DS125DF111_RATIOS_RATE_MASK | DS125DF111_RATIOS_SUBRATE_MASK);
> > +	val |= FIELD_PREP(DS125DF111_RATIOS_RATE_MASK, cfg->rate) |
> > +		FIELD_PREP(DS125DF111_RATIOS_SUBRATE_MASK, cfg->subrate);
> > +	err = i2c_smbus_write_byte_data(i2c, DS125DF111_RATIOS, val);
> > +	if (err < 0) {
> > +		dev_err(dev, "Error programming the divide ratios\n");
> > +		goto out;
> > +	}
> > +
> > +	mutex_unlock(&priv->mutex);
> > +
> > +	return 0;
> > +
> > +out:
> > +	mutex_unlock(&priv->mutex);
> > +
> > +	return err;
> 
> You don't need a separate code path for the 'out' label, it can be
> common with the normal exit path (err will be 0).

Ok.

> 
> > +}
> > +
> > +static int ds125df111_set_mode(struct phy *phy, enum phy_mode mode, int submode)
> > +{
> > +	const struct ds125df111_config *cfg;
> > +
> > +	if (mode != PHY_MODE_ETHERNET)
> > +		return -EOPNOTSUPP;
> 
> Please use a different error code like -EINVAL. Let -EOPNOTSUPP mean
> that the function is not implemented (when calling phy_set_mode_ext()).
> 
> > +
> > +	switch (submode) {
> > +	case PHY_INTERFACE_MODE_10GBASER:
> > +		cfg = &ds125df111_cfg[FREQ_10G];
> > +		break;
> > +	case PHY_INTERFACE_MODE_1000BASEX:
> > +	case PHY_INTERFACE_MODE_SGMII:
> > +		cfg = &ds125df111_cfg[FREQ_1G];
> > +		break;
> > +	default:
> > +		return -EOPNOTSUPP;
> 
> Same here.

Will change in both instances.

> 
> > +	}
> > +
> > +	return ds125df111_configure(phy, cfg);
> > +}
> > +
> > +static const struct phy_ops ds125df111_ops = {
> > +	.set_mode	= ds125df111_set_mode,
> 
> Can you please implement .validate() as well? It will be made mandatory
> in the future for those who implement .set_mode().
> 

Sure. Will implement the .validate() callback in v3.

(...)

> > +	.owner		= THIS_MODULE,
> > +};
> > +
> > +static const struct of_device_id ds125df111_dt_ids[] =
> > +	{ .compatible = "ti,ds125df111", },
> > +	{},
> 
> Unnecessary comma after sentinel entry.

I will remove it.