[PATCH v2 0/3] phy: Realtek Otto SerDes: add new driver

[PATCH v2 0/3] phy: Realtek Otto SerDes: add new driver

[Markus Stockhausen]

This patch series adds support for the SerDes in the Realtek Otto platform.
These are 8-52 port switch SoCs of the RTL83xx and RTL93xx series with MIPS
cores. The ports are based on 1-8 port PHYs (e.g. RTL8218D) that are connected
via multiple SerDes.

The driver is based on the GPL source drops from the different switch vendors.
It supports all 4 SoC series and was developed and tested on the following
devices:

RTL838x - codename maple - Linksys LGS310C
RTL839x - codename cypress - Zyxel GS1920-24
RTL930x - codename langon - Zyxel XGS1210-10
RTL931x - codename mango - Linksys LGS352C

Due to very little information and fundamentally different I/O handling and
port ranges of the devices the driver assumes and translates some handling
to provide a common consistent interface.

Currently only provide the most basic operations for mode set and device
reset as well as some debugging information if enabled. The strength lies in
the fact that the driver can run patch sequences for the SerDes registers at
certain events, e.g. during setup. This allows to run configuration
operations to get the SerDes up and running. For now provide setup sequences
for RTL838x and RTL839x devices.

For more information see:

https://svanheule.net/switches/gpl_source_drops
https://svanheule.net/realtek/

[PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding

[Markus Stockhausen]

Add bindings for the SerDes of the Realtek Otto platform. These are
network Switch SoCs with up to 52 ports divided into four different
model lines.

Changes in v2:
- new subject
- removed patch command sequences
- renamed parameter controlled-ports to realtek,controlled-ports

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 .../bindings/phy/realtek,otto-serdes.yaml     | 95 +++++++++++++++++++
 1 file changed, 95 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml

diff --git a/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml
new file mode 100644
index 000000000000..a72ac206b35f
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml
@@ -0,0 +1,95 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/realtek,otto-serdes.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Realtek Otto SerDes controller
+
+maintainers:
+  - Markus Stockhausen <markus.stockhausen@gmx.de>
+
+description:
+  The MIPS based Realtek Switch SoCs of the Realtek RTL838x, RTL839x, RTL930x and RTL931x series
+  have multiple SerDes built in. They are linked to single, quad or octa PHYs like the RTL8218B,
+  RTL8218D or RTL8214FC and are one of the integral part of the up-to-52-port switch architecture.
+
+  Although these SerDes controllers have common basics they behave differently on the SoC families
+  and rely on heavy register magic. To keep the driver clean it can load patch sequences from
+  devictree and execute them during the controller actions like phy_init(), ...
+
+  The driver exposes the SerDes registers different from the hardware but instead gives a
+  consistent view and programming interface. So the RTL838x series has 6 ports and 4 pages, the
+  RTL839x has 14 ports and 12 pages, the RTL930x has 12 ports and 64 pages and the RTL931x has
+  14 ports and 192 pages.
+
+properties:
+  $nodename:
+    pattern: "^serdes@[0-9a-f]+$"
+
+  compatible:
+    items:
+      - enum:
+          - realtek,rtl8380-serdes
+          - realtek,rtl8390-serdes
+          - realtek,rtl9300-serdes
+          - realtek,rtl9310-serdes
+
+  reg:
+    items:
+      description:
+        The primary SerDes paged register memory location. Other SerDes control and management
+        registers are distributed all over the I/O memory space and are identified by the driver.
+
+  "#phy-cells":
+    const: 4
+    description:
+      The first number defines the SerDes to use. The second number a linked SerDes. E.g. if a octa
+      1G PHY is attached to two QSGMII SerDes. The third number is the first switch port this
+      SerDes is working for, the fourth number is the last switch port the SerDes is working for.
+
+  realtek,controlled-ports:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      A bit mask defining the ports that are actively controlled by the driver. In case a bit is
+      not set the driver will only process read operations on the SerDes. This is just in case the
+      SerDes has been setup by U-Boot correctly and the driver malfunctions. If not set the driver
+      will control all SerDes.
+
+reguired:
+  - compatible
+  - reg
+  - "#phy-cells"
+
+additionalProperties:
+  false
+
+examples:
+  - |
+    serdes: serdes@1b00e780 {
+      compatible = "realtek,rtl8380-serdes", "realtek,otto-serdes";
+      reg = <0x1b00e780 0x1200>;
+      controlled-ports = <0x003f>;
+      #phy-cells = <4>;
+    };
+  - |
+    serdes: serdes@1b00a000 {
+      compatible = "realtek,rtl8390-serdes", "realtek,otto-serdes";
+      reg = <0x1b00a000 0x1c00>;
+      controlled-ports = <0x3fff>;
+      #phy-cells = <4>;
+    };
+  - |
+    serdes: serdes@1b0003b0 {
+      compatible = "realtek,rtl9300-serdes", "realtek,otto-serdes";
+      reg = <0x1b0003b0 0x8>;
+      controlled-ports = <0x0fff>;
+      #phy-cells = <4>;
+    };
+  - |
+    serdes: serdes@1b005638 {
+      compatible = "realtek,rtl9310-serdes", "realtek,otto-serdes";
+      reg = <0x1b005638 0x8>;
+      controlled-ports = <0x3fff>;
+      #phy-cells = <4>;
+    };
--
2.46.2

[PATCH v2 2/3] phy: Realtek Otto SerDes driver

[Markus Stockhausen]

The Realtek Otto platform is a series of 4 different MIPS32 based network
switch SoCs. They consist of:

- RTL838x: 500MHz single core, up to 28 ports 20GBps switching capacity
- RTL839x: 700MHz single core, up to 52 ports 100GBps switching capacity
- RTL930x: 700MHz single core, up to 28 ports 120GBps switching capacity
- RTL931x: 1.4GHz dual core, up to 52 ports 170GBps switching capacity

The SoCs have 6-14 SerDes that provide the interconnect between several
one, quad or octa port attached PHYs like the RTL8218D or RTL8214FC.

This driver builts on top of several GPL source drops from different switch
vendors and harmonizes the different programming models. The common basics
are:

- Each SerDes is controlled through registers that are organized into pages
- A page consists of 32x 16 bit registers that cover various functions
- Registers are either accessed through I/O addresses or an MDIO style bus
- The SerDes operate on different MII variants (mostly QSGMII & XGMII)

The SerDes rely on heavy register modifications with lots of undocumented
features. Provide a patching sequence feature that allows to configure
the system.

Changes in v2:
- switched logic to internal patch sequences
- added setup sequences for RTL838x and RTL839x
- moved includes from header to source file
- added helpers for better readability
- minor fixes from checkpatch

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 drivers/phy/realtek/phy-rtk-otto-serdes.c | 1312 +++++++++++++++++++++
 drivers/phy/realtek/phy-rtk-otto-serdes.h |  117 ++
 2 files changed, 1429 insertions(+)
 create mode 100644 drivers/phy/realtek/phy-rtk-otto-serdes.c
 create mode 100644 drivers/phy/realtek/phy-rtk-otto-serdes.h

diff --git a/drivers/phy/realtek/phy-rtk-otto-serdes.c b/drivers/phy/realtek/phy-rtk-otto-serdes.c
new file mode 100644
index 000000000000..34715ee3ea37
--- /dev/null
+++ b/drivers/phy/realtek/phy-rtk-otto-serdes.c
@@ -0,0 +1,1312 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Realtek RTL838x, RTL839x, RTL930x & RTL931x SerDes PHY driver
+ * Copyright (c) 2024 Markus Stockhausen <markus.stockhausen@gmx.de>
+ */
+
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+
+#include "phy-rtk-otto-serdes.h"
+
+/* common helpers */
+
+static inline bool rtsds_invalid_reg(struct rtsds_ctrl *ctrl, u32 sid, u32 page, u32 reg)
+{
+	return (sid >= ctrl->conf->sds_cnt || page >= ctrl->conf->page_cnt || reg > 31);
+}
+
+static inline bool rtsds_invalid_sds(struct rtsds_ctrl *ctrl, u32 sid)
+{
+	return (sid >= ctrl->conf->sds_cnt);
+}
+
+static inline bool rtsds_readonly(struct rtsds_ctrl *ctrl, u32 sid)
+{
+	return !(ctrl->sds_mask & BIT(sid));
+}
+
+static int rtsds_hwmode_to_phymode(struct rtsds_ctrl *ctrl, int hwmode)
+{
+	for (int m = 0; m < PHY_INTERFACE_MODE_MAX; m++)
+		if (ctrl->conf->mode_map[m] == hwmode)
+			return m;
+
+	return PHY_INTERFACE_MODE_MAX;
+}
+
+static const char *rtsds_events[RTSDS_EVENT_CNT] = {
+	[RTSDS_EVENT_SETUP] = "setup",
+};
+
+static int rtsds_run_event(struct rtsds_ctrl *ctrl, u32 sid, int evt)
+{
+	struct rtsds_seq *seq;
+	int ret, step = 1, delay = 0;
+
+	if (evt >= RTSDS_EVENT_CNT || sid >= ctrl->conf->sds_cnt)
+		return -EINVAL;
+
+	seq = ctrl->conf->sequence[evt];
+
+	if (!seq)
+		return 0;
+
+	while (seq->action != RTSDS_SEQ_STOP) {
+		if (!(seq->ports & BIT(sid)) ||
+		    ((seq->mode != PHY_INTERFACE_MODE_NA) &&
+		     (seq->mode != ctrl->sds[sid].mode))) {
+			step++;	seq++;
+			continue;
+		}
+
+		if (seq->action == RTSDS_SEQ_WAIT)
+			delay = seq->val;
+
+		if (delay)
+			usleep_range(delay << 10, (delay << 10) + 1000);
+
+		if (seq->action == RTSDS_SEQ_MASK) {
+			ret = ctrl->conf->mask(ctrl, sid, seq->page,
+					 seq->reg, seq->val, seq->mask);
+			if (ret) {
+				dev_err(ctrl->dev, "sequence %s failed at step %d",
+					rtsds_events[evt], step);
+				return -EIO;
+			}
+		}
+
+		step++; seq++;
+	}
+
+	return 0;
+}
+
+/* common RTL838x and RTL839x helpers */
+
+static void rtsds_83xx_digital_reset(struct rtsds_ctrl *ctrl, u32 sid, u32 cnt)
+{
+	/* software reset */
+	for (u32 i = sid; i <= sid + cnt; i++)
+		ctrl->conf->mask(ctrl, i, RTSDS_PAGE_SDS,
+				 0x03, RTSDS_BITS_SOFT_RST, RTSDS_BITS_SOFT_RST);
+	usleep_range(100000, 101000);
+	for (u32 i = sid; i <= sid + cnt; i++)
+		ctrl->conf->mask(ctrl, i, RTSDS_PAGE_SDS,
+				 0x03, 0x0000, RTSDS_BITS_SOFT_RST);
+	/* RX/TX reset */
+	for (u32 i = sid; i <= sid + cnt; i++) {
+		ctrl->conf->mask(ctrl, i, RTSDS_PAGE_SDS,
+				 0x00, 0x0000, RTSDS_BITS_SDS_EN);
+		ctrl->conf->mask(ctrl, i, RTSDS_PAGE_SDS,
+				 0x00, RTSDS_BITS_SDS_EN, RTSDS_BITS_SDS_EN);
+	}
+}
+
+/* common RTL930x and RTL931x helpers */
+
+static int rtsds_93xx_read(struct rtsds_ctrl *ctrl, u32 sid, u32 page, u32 reg)
+{
+	int cnt = 100, cmd = (sid << 2) | (page << 7) | (reg << 13) | RTSDS_93XX_SDS_READ;
+
+	iowrite32(cmd, ctrl->base);
+	while (--cnt && (ioread32(ctrl->base) & RTSDS_93XX_SDS_BUSY))
+		usleep_range(50, 60);
+
+	return cnt ? ioread32(ctrl->base + 4) & 0xffff : -EIO;
+}
+
+static int rtsds_93xx_mask(struct rtsds_ctrl *ctrl, u32 sid, u32 page, u32 reg, u32 val, u32 mask)
+{
+	int oldval, cnt = 100, cmd = (sid << 2) | (page << 7) | (reg << 13) | RTSDS_93XX_SDS_WRITE;
+
+	if (mask != 0xffff) {
+		oldval = rtsds_93xx_read(ctrl, sid, page, reg);
+		if (oldval < 0)
+			return -EIO;
+		oldval &= ~mask;
+		val |= oldval;
+	}
+
+	iowrite32(val, ctrl->base + 4);
+	iowrite32(cmd, ctrl->base);
+
+	while (--cnt && (ioread32(ctrl->base) & RTSDS_93XX_SDS_BUSY))
+		usleep_range(50, 60);
+
+	return cnt ? 0 : -EIO;
+}
+
+/*
+ * The RTL838x has 6 SerDes. The 16 bit registers start at 0xbb00e780 and are mapped directly into
+ * 32 bit memory addresses. High 16 bits are always empty. A "lower memory block serves pages 0/3
+ * a higher one pages 1/2.
+ */
+
+static int rtsds_838x_reg_offset(u32 sid, u32 page, u32 reg)
+{
+	if (page == 0 || page == 3)
+		return (sid << 9) + (page << 7) + (reg << 2);
+	else
+		return 0xb80 + (sid << 8) + (page << 7) + (reg << 2);
+}
+
+static int rtsds_838x_read(struct rtsds_ctrl *ctrl, u32 sid, u32 page, u32 reg)
+{
+	int offs;
+
+	if (rtsds_invalid_reg(ctrl, sid, page, reg))
+		return -EINVAL;
+
+	offs = rtsds_838x_reg_offset(sid, page, reg);
+
+	/* read twice for link status latch */
+	if (page == RTSDS_PAGE_FIB && reg == 1)
+		ioread32(ctrl->base + offs);
+
+	return ioread32(ctrl->base + offs);
+}
+
+static int rtsds_838x_mask(struct rtsds_ctrl *ctrl, u32 sid, u32 page, u32 reg, u32 val, u32 mask)
+{
+	int offs;
+
+	if (rtsds_invalid_reg(ctrl, sid, page, reg))
+		return -EINVAL;
+
+	offs = rtsds_838x_reg_offset(sid, page, reg);
+
+	/* read twice for link status latch */
+	if (page == RTSDS_PAGE_FIB && reg == 1)
+		ioread32(ctrl->base + offs);
+
+	iomask32(mask, val, ctrl->base + offs);
+
+	return 0;
+}
+
+static void rtsds_838x_rx_reset(struct rtsds_ctrl *ctrl, u32 sid)
+{
+	/* reset with a selftest by enabling/disabling RX_EN_SELF */
+	rtsds_838x_mask(ctrl, sid, 0x01, 0x09, 0x0200, 0x0200);
+	usleep_range(100000, 101000);
+	rtsds_838x_mask(ctrl, sid, 0x01, 0x09, 0x0000, 0x0200);
+}
+
+static void rtsds_838x_cmu_reset(struct rtsds_ctrl *ctrl, u32 sid)
+{
+	rtsds_838x_mask(ctrl, sid, 0x01, 0x00, 0x4040, 0xffff);
+	rtsds_838x_mask(ctrl, sid, 0x01, 0x00, 0x4740, 0xffff);
+	rtsds_838x_mask(ctrl, sid, 0x01, 0x00, 0x47c0, 0xffff);
+	rtsds_838x_mask(ctrl, sid, 0x01, 0x00, 0x4000, 0xffff);
+}
+
+static int rtsds_838x_reset(struct rtsds_ctrl *ctrl, u32 sid)
+{
+	if (rtsds_invalid_sds(ctrl, sid))
+		return -EINVAL;
+
+	rtsds_838x_rx_reset(ctrl, sid);
+	rtsds_838x_cmu_reset(ctrl, sid);
+	rtsds_83xx_digital_reset(ctrl, sid, 1);
+
+	return 0;
+}
+
+static int rtsds_838x_set_mode(struct rtsds_ctrl *ctrl, u32 sid, int combomode)
+{
+	int shift, mode = RTSDS_MODE(combomode), submode = RTSDS_SUBMODE(combomode);
+
+	if (rtsds_invalid_sds(ctrl, sid))
+		return -EINVAL;
+
+	if (sid >= 4) {
+		shift = (sid - 4) * 3;
+		iomask32(0x7 << shift, (submode & 0x7) << shift, RTSDS_838X_INT_MODE_CTRL);
+	} else if (submode != 0)
+		return -EINVAL;
+
+	shift = 25 - sid * 5;
+	iomask32(0x1f << shift, (mode & 0x1f) << shift, RTSDS_838X_SDS_MODE_SEL);
+
+	return 0;
+}
+
+static int rtsds_838x_get_mode(struct rtsds_ctrl *ctrl, u32 sid)
+{
+	int shift, mode, submode = 0;
+
+	if (rtsds_invalid_sds(ctrl, sid))
+		return -EINVAL;
+
+	if (sid >= 4) {
+		shift = (sid - 4) * 3;
+		submode = (ioread32(RTSDS_838X_INT_MODE_CTRL) >> shift) & 0x7;
+	}
+
+	shift = 25 - sid * 5;
+	mode = (ioread32(RTSDS_838X_SDS_MODE_SEL) >> shift) & 0x1f;
+
+	return RTSDS_COMBOMODE(mode, submode);
+}
+
+/*
+ * The RLT839x has 14 SerDes starting at 0xbb00a000. 0-7, 10, 11 are 5GBit, 8, 9, 12, 13 are
+ * 10GIt. Two adjacent SerDes are tightly coupled and share a 1024 bytes register area. Per 32 bit
+ * address two registers are stored. The first register is stored in the lower 2 bytes ("on the
+ * right" due to big endian) and the second register in the upper 2 bytes. We know the following
+ * register areas:
+ *
+ * - XSG0	(4 pages @ offset 0x000): for even SerDes
+ * - XSG1	(4 pages @ offset 0x100): for odd SerDes
+ * - TGRX	(4 pages @ offset 0x200): for even 10G SerDes
+ * - ANA_RG	(2 pages @ offset 0x300): for even 5G SerDes
+ * - ANA_RG	(2 pages @ offset 0x380): for odd 5G SerDes
+ * - ANA_TG	(2 pages @ offset 0x300): for even 10G SerDes
+ * - ANA_TG	(2 pages @ offset 0x380): for odd 10G SerDes
+ *
+ * The most consistent mapping that aligns to the RTL93xx devices is:
+ *
+ *		even 5G SerDes	odd 5G SerDes	even 10G SerDes	odd 10G SerDes
+ * Page 0:	XSG0/0		XSG1/0		XSG0/0		XSG1/0
+ * Page 1:	XSG0/1		XSG1/1		XSG0/1		XSG1/1
+ * Page 2:	XSG0/2		XSG1/2		XSG0/2		XSG1/2
+ * Page 3:	XSG0/3		XSG1/3		XSG0/3		XSG1/3
+ * Page 4:	<zero>		<zero>		TGRX/0		<zero>
+ * Page 5:	<zero>		<zero>		TGRX/1		<zero>
+ * Page 6:	<zero>		<zero>		TGRX/2		<zero>
+ * Page 7:	<zero>		<zero>		TGRX/3		<zero>
+ * Page 8:	ANA_RG		ANA_RG		<zero>		<zero>
+ * Page 9:	ANA_RG_EXT	ANA_RG_EXT	<zero>		<zero>
+ * Page 10:	<zero>		<zero>		ANA_TG		ANA_TG
+ * Page 11:	<zero>		<zero>		ANA_TG_EXT	ANA_TG_EXT
+ */
+
+static int rtsds_839x_reg_offset(u32 sid, u32 page, u32 reg)
+{
+	int offs = ((sid & 0xfe) << 9) + ((reg & 0xfe) << 1);
+
+	if (page < 4) {
+		offs += ((sid & 1) << 8) + (page << 6);
+	} else if (page < 8) {
+		if (sid != 8 && sid != 12)
+			return -1;
+		offs += 0x100 + (page << 6);
+	} else if (page < 10) {
+		if (sid == 8 || sid == 9 || sid == 12 || sid == 13)
+			return -1;
+		offs += 0x100 + ((sid & 1) << 7) + (page << 6);
+	} else {
+		if (sid != 8 && sid != 9 && sid != 12 && sid != 13)
+			return -1;
+		offs += 0x100 + ((sid & 1) << 7) + ((page - 2) << 6);
+	}
+
+	return offs;
+}
+
+static int rtsds_839x_read(struct rtsds_ctrl *ctrl, u32 sid, u32 page, u32 reg)
+{
+	int offs, shift = (reg << 4) & 0x10;
+
+	if (rtsds_invalid_reg(ctrl, sid, page, reg))
+		return -EINVAL;
+
+	offs = rtsds_839x_reg_offset(sid, page, reg);
+	if (offs < 0)
+		return 0;
+
+	/* read twice for link status latch */
+	if (page == RTSDS_PAGE_FIB && reg == 1)
+		ioread32(ctrl->base + offs);
+
+	return (ioread32(ctrl->base + offs) >> shift) & 0xffff;
+}
+
+static int rtsds_839x_mask(struct rtsds_ctrl *ctrl, u32 sid, u32 page, u32 reg, u32 val, u32 mask)
+{
+	int oldval, offs;
+
+	if (rtsds_invalid_reg(ctrl, sid, page, reg))
+		return -EINVAL;
+
+	offs = rtsds_839x_reg_offset(sid, page, reg);
+	if (offs < 0)
+		return 0;
+
+	/* read twice for link status latch */
+	if (page == RTSDS_PAGE_FIB && reg == 1)
+		ioread32(ctrl->base + offs);
+
+	oldval = ioread32(ctrl->base + offs);
+	val = reg & 1 ? (oldval & ~(mask << 16)) | (val << 16) : (oldval & ~mask) | val;
+	iowrite32(val, ctrl->base + offs);
+
+	return 0;
+}
+
+static int rtsds_839x_set_mode(struct rtsds_ctrl *ctrl, u32 sid, int combomode)
+{
+	int shift = (sid & 7) << 2, offs = (sid >> 1) & ~3;
+	int mode = RTSDS_MODE(combomode), submode = RTSDS_SUBMODE(combomode);
+
+	if (rtsds_invalid_sds(ctrl, sid))
+		return -EINVAL;
+
+	rtsds_839x_mask(ctrl, sid, RTSDS_PAGE_SDS, 0x04, (submode << 12) & 0xf000, 0xf000);
+	iomask32(0xf << shift, (mode & 0xf) << shift, RTSDS_839X_MAC_SERDES_IF_CTRL + offs);
+
+	return 0;
+}
+
+static int rtsds_839x_get_mode(struct rtsds_ctrl *ctrl, u32 sid)
+{
+	int mode, submode, shift = (sid & 7) << 2, offs = (sid >> 1) & ~3;
+
+	if (rtsds_invalid_sds(ctrl, sid))
+		return -EINVAL;
+
+	submode = (rtsds_839x_read(ctrl, sid, RTSDS_PAGE_SDS, 0x04) >> 12) & 0xf;
+	mode = (ioread32(RTSDS_839X_MAC_SERDES_IF_CTRL + offs) >> shift) & 0xf;
+
+	return RTSDS_COMBOMODE(mode, submode);
+}
+
+static void rtsds_839x_rx_reset(struct rtsds_ctrl *ctrl, u32 sid)
+{
+	u32 i, page, reg, pre, post, mask, lo = sid & ~1, hi = sid | 1;
+
+	/* reset with a selftest by enabling/disabling RX_EN_SELF */
+	if (lo < 8 || lo == 10) {
+		page = 0x08; reg = 0x14; pre = 0x0200; post = 0x0000; mask = 0x0201;
+	} else {
+		page = 0x0b; reg = 0x00; pre = 0x8000; post = 0x0000; mask = 0x8008;
+	}
+
+	for (i = lo; i <= hi; i++)
+		rtsds_839x_mask(ctrl, i, page, reg, pre, mask);
+	usleep_range(100000, 101000);
+	for (i = lo; i <= hi; i++)
+		rtsds_839x_mask(ctrl, i, page, reg, post, mask);
+}
+
+static void rtsds_839x_cmu_reset(struct rtsds_ctrl *ctrl, u32 sid)
+{
+	int hi = sid | 1;
+
+	if (hi == 9 || hi == 13)
+		return;
+
+	rtsds_839x_mask(ctrl, hi, 0x09, 0x01, 0x0050, 0xffff);
+	rtsds_839x_mask(ctrl, hi, 0x09, 0x01, 0x00f0, 0xffff);
+	rtsds_839x_mask(ctrl, hi, 0x09, 0x01, 0x0000, 0xffff);
+}
+
+static int rtsds_839x_reset(struct rtsds_ctrl *ctrl, u32 sid)
+{
+	int lo = sid & ~1;
+
+	if (rtsds_invalid_sds(ctrl, sid))
+		return -EINVAL;
+
+	/* Registers are shared between adjacent SerDes. Reset both. */
+	rtsds_839x_cmu_reset(ctrl, sid);
+	rtsds_839x_rx_reset(ctrl, sid);
+	rtsds_83xx_digital_reset(ctrl, lo, 2);
+
+	return 0;
+}
+
+/*
+ * The RTL930x family has 12 SerdDes. They are accessed through two IO registers at 0xbb0003b0
+ * which simulate commands to an internal MDIO bus. From the current observation there are 3 types
+ * of SerDes:
+ *
+ * - SerDes 0,1 exist on the RLT9301 and 9302B and are QSGMII capable
+ * - SerDes 2-9 are USXGMII capabable with either quad or single configuration
+ * - SerDes 10-11 are 10GBase-R capable
+ */
+
+static int rtsds_930x_read(struct rtsds_ctrl *ctrl, u32 sid, u32 page, u32 reg)
+{
+	if (rtsds_invalid_reg(ctrl, sid, page, reg))
+		return -EINVAL;
+
+	return rtsds_93xx_read(ctrl, sid, page, reg);
+}
+
+static int rtsds_930x_mask(struct rtsds_ctrl *ctrl, u32 sid, u32 page, u32 reg, u32 val, u32 mask)
+{
+	if (rtsds_invalid_reg(ctrl, sid, page, reg))
+		return -EINVAL;
+
+	return rtsds_93xx_mask(ctrl, sid, page, reg, val, mask);
+}
+
+static void rtsds_930x_mode_offset(int sid,
+				   void __iomem __force **modereg, int *modeshift,
+				   void __iomem __force **subreg, int *subshift)
+{
+	if (sid > 3) {
+		*subreg = RTSDS_930X_SDS_SUBMODE_CTRL1;
+		*subshift = (sid - 4) * 5;
+	} else {
+		*subreg = RTSDS_930X_SDS_SUBMODE_CTRL0;
+		*subshift = (sid - 2) * 5;
+	}
+
+	if (sid < 4) {
+		*modeshift = sid * 6;
+		*modereg = RTSDS_930X_SDS_MODE_SEL_0;
+	} else if (sid < 8) {
+		*modeshift = (sid - 4) * 6;
+		*modereg = RTSDS_930X_SDS_MODE_SEL_1;
+	} else if (sid < 10) {
+		*modeshift = (sid - 8) * 6;
+		*modereg = RTSDS_930X_SDS_MODE_SEL_2;
+	} else {
+		*modeshift = (sid - 10) * 6;
+		*modereg = RTSDS_930X_SDS_MODE_SEL_3;
+	}
+}
+
+static int rtsds_930x_set_mode(struct rtsds_ctrl *ctrl, u32 sid, int combomode)
+{
+	int modeshift, subshift;
+	int mode = RTSDS_MODE(combomode);
+	int submode = RTSDS_SUBMODE(combomode);
+	void __iomem __force *modereg;
+	void __iomem __force *subreg;
+
+	if (rtsds_invalid_sds(ctrl, sid))
+		return -EINVAL;
+
+	rtsds_930x_mode_offset(sid, &modereg, &modeshift, &subreg, &subshift);
+	if (sid >= 2 || sid <= 9)
+		iomask32(0x1f << subshift, (submode & 0x1f) << subshift, subreg);
+	else if (submode != 0)
+		return -EINVAL;
+	iomask32(0x1f << modeshift, (mode & 0x1f) << modeshift, modereg);
+
+	return 0;
+}
+
+static int rtsds_930x_get_mode(struct rtsds_ctrl *ctrl, u32 sid)
+{
+	int modeshift, subshift, mode, submode = 0;
+	void __iomem __force *modereg;
+	void __iomem __force *subreg;
+
+	if (rtsds_invalid_sds(ctrl, sid))
+		return -EINVAL;
+
+	rtsds_930x_mode_offset(sid, &modereg, &modeshift, &subreg, &subshift);
+	if (sid >= 2 || sid <= 9)
+		submode = (ioread32(subreg) >> subshift) & 0x1f;
+	mode = ioread32(modereg) >> modeshift & 0x1f;
+
+	return RTSDS_COMBOMODE(mode, submode);
+}
+
+static int rtsds_930x_reset(struct rtsds_ctrl *ctrl, u32 sid)
+{
+	int modecur, modeoff = ctrl->conf->mode_map[PHY_INTERFACE_MODE_NA];
+
+	if (rtsds_invalid_sds(ctrl, sid))
+		return -EINVAL;
+
+	modecur = rtsds_930x_get_mode(ctrl, sid);
+
+	/* It is enough to power off SerDes and set to old mode again */
+	if (modecur != modeoff) {
+		rtsds_930x_set_mode(ctrl, sid, modeoff);
+		rtsds_930x_set_mode(ctrl, sid, modecur);
+	}
+
+	return 0;
+}
+
+/*
+ * The RTL931x family has 14 "frontend" SerDes that are cascaded. All operations (e.g. reset) work
+ * on this frontend view while their registers are distributed over a total of 32 background
+ * SerDes. Two types of SerDes have been identified:
+ *
+ * A "even" SerDes with numbers 0, 1, 2, 4, 6, 8, 10, 12 works on two background SerDes. 64 analog
+ * and 64 XGMII data pages are coming from a first background SerDes while another 64 XGMII pages
+ * are served from a second SerDes.
+ *
+ * The "odd" SerDes with numbers 3, 5, 7, 9, 11 & 13 SerDes consist of a total of 3 background
+ * SerDes (one analog and two XGMII) each with an own page/register set.
+ *
+ * To align this and improve readability the driver will simulate a total of 576 pages and mix
+ * them as follows.
+ *
+ * frontend page		"even" frontend SerDes	"odd" frontend SerDes
+ * page 0-63 (analog):		back SDS page 0-63	back SDS page 0-63
+ * page 64-127 (XGMII1):	back SDS page 0-63	back SDS+1 page 0-63
+ * page 128-191 (XGMII2):	back SDS+1 page 0-63	back SDS+2 page 0-63
+ */
+
+static int rtsds_931x_reg_offset(u32 sid, u32 page)
+{
+	int map[] = {0, 1, 2, 3, 6, 7, 10, 11, 14, 15, 18, 19, 22, 23};
+	int offs = map[sid];
+
+	if ((sid & 1) && (sid != 1))
+		offs += (page >> 6); /* distribute "odd" to 3 background SerDes */
+	else if (page >= 128)
+		offs += 1; /* "distribute "even" to 2 background SerDes */
+
+	return offs;
+}
+
+static int rtsds_931x_read(struct rtsds_ctrl *ctrl, u32 sid, u32 page, u32 reg)
+{
+	int offs;
+
+	if (rtsds_invalid_reg(ctrl, sid, page, reg))
+		return -EINVAL;
+
+	offs = rtsds_931x_reg_offset(sid, page);
+	if (offs < 0)
+		return 0;
+
+	return rtsds_93xx_read(ctrl, offs, page, reg);
+}
+
+static int rtsds_931x_mask(struct rtsds_ctrl *ctrl, u32 sid, u32 page, u32 reg, u32 val, u32 mask)
+{
+	int offs;
+
+	if (rtsds_invalid_reg(ctrl, sid, page, reg))
+		return -EINVAL;
+
+	offs = rtsds_931x_reg_offset(sid, page);
+	if (offs < 0)
+		return 0;
+
+	return rtsds_93xx_mask(ctrl, offs, page, reg, val, mask);
+}
+
+static int rtsds_931x_set_mode(struct rtsds_ctrl *ctrl, u32 sid, int combomode)
+{
+	int shift = (sid & 3) << 3, offs = sid & ~3;
+	int mode = RTSDS_MODE(combomode);
+	int submode = RTSDS_SUBMODE(combomode);
+
+	if (rtsds_invalid_sds(ctrl, sid))
+		return -EINVAL;
+
+	rtsds_931x_mask(ctrl, sid, 0x1f, 0x09, (submode & 0x3f << 6), 0x0fc0);
+	iomask32(0xff << shift, ((mode | RTSDS_931X_SDS_FORCE_SETUP) & 0xff) << shift,
+		 RTSDS_931X_SERDES_MODE_CTRL + offs);
+
+	return 0;
+}
+
+static int rtsds_931x_get_mode(struct rtsds_ctrl *ctrl, u32 sid)
+{
+	int mode, submode, shift = (sid & 3) << 3, offs = sid & ~3;
+
+	if (rtsds_invalid_sds(ctrl, sid))
+		return -EINVAL;
+
+	submode = (rtsds_931x_read(ctrl, sid, 0x1f, 0x09) >> 6) & 0x3f;
+	mode = (ioread32(RTSDS_931X_SERDES_MODE_CTRL + offs) >> shift) & 0x1f;
+
+	return RTSDS_COMBOMODE(mode, submode);
+}
+
+static int rtsds_931x_reset(struct rtsds_ctrl *ctrl, u32 sid)
+{
+	int pwr, modecur, modeoff = ctrl->conf->mode_map[PHY_INTERFACE_MODE_NA];
+
+	if (rtsds_invalid_sds(ctrl, sid))
+		return -EINVAL;
+
+	modecur = rtsds_931x_get_mode(ctrl, sid);
+
+	if (modecur != modeoff) {
+		/* reset with mode switch cycle while being powered off */
+		pwr = ioread32(RTSDS_931X_PS_SERDES_OFF_MODE_CTRL);
+		iowrite32(pwr | BIT(sid), RTSDS_931X_PS_SERDES_OFF_MODE_CTRL);
+		rtsds_931x_set_mode(ctrl, sid, modeoff);
+		rtsds_931x_set_mode(ctrl, sid, modecur);
+		iowrite32(pwr, RTSDS_931X_PS_SERDES_OFF_MODE_CTRL);
+	}
+
+	return 0;
+}
+
+int rtsds_read(struct phy *phy, u32 page, u32 reg)
+{
+	struct rtsds_macro *macro = phy_get_drvdata(phy);
+	struct rtsds_ctrl *ctrl = macro->ctrl;
+	u32 sid = macro->sid;
+
+	return ctrl->conf->read(ctrl, sid, page, reg);
+}
+
+int rtsds_mask(struct phy *phy, u32 page, u32 reg, u32 val, u32 mask)
+{
+	struct rtsds_macro *macro = phy_get_drvdata(phy);
+	struct rtsds_ctrl *ctrl = macro->ctrl;
+	u32 sid = macro->sid;
+
+	if (rtsds_readonly(ctrl, sid))
+		return 0;
+
+	return ctrl->conf->mask(ctrl, sid, page, reg, val, mask);
+}
+
+int rtsds_write(struct phy *phy, u32 page, u32 reg, u32 val)
+{
+	return rtsds_mask(phy, page, reg, val, 0xffff);
+}
+
+static int rtsds_phy_init(struct phy *phy)
+{
+	struct rtsds_macro *macro = phy_get_drvdata(phy);
+	struct rtsds_ctrl *ctrl = macro->ctrl;
+	u32 sid = macro->sid;
+	int ret;
+
+	if (rtsds_readonly(ctrl, sid))
+		return 0;
+
+	mutex_lock(&ctrl->lock);
+//	ret = rtsds_run_event(ctrl, sid, RTSDS_EVENT_INIT);
+	mutex_unlock(&ctrl->lock);
+
+	if (ret)
+		dev_err(ctrl->dev, "init failed for SerDes %d\n", sid);
+
+	return ret;
+}
+
+static int rtsds_phy_power_on(struct phy *phy)
+{
+	struct rtsds_macro *macro = phy_get_drvdata(phy);
+	struct rtsds_ctrl *ctrl = macro->ctrl;
+	u32 sid = macro->sid;
+	int ret;
+
+	if (rtsds_readonly(ctrl, sid))
+		return 0;
+
+	mutex_lock(&ctrl->lock);
+//	ret = rtsds_run_event(ctrl, sid, RTSDS_EVENT_POWER_ON);
+	mutex_unlock(&ctrl->lock);
+
+	if (ret)
+		dev_err(ctrl->dev, "power on failed for SerDes %d\n", sid);
+
+	return ret;
+}
+
+static int rtsds_phy_power_off(struct phy *phy)
+{
+	struct rtsds_macro *macro = phy_get_drvdata(phy);
+	struct rtsds_ctrl *ctrl = macro->ctrl;
+	u32 sid = macro->sid;
+	int ret;
+
+	if (rtsds_readonly(ctrl, sid))
+		return 0;
+
+	mutex_lock(&ctrl->lock);
+	if (!ret)
+		ret = ctrl->conf->set_mode(ctrl, sid, ctrl->conf->mode_map[PHY_INTERFACE_MODE_NA]);
+	mutex_unlock(&ctrl->lock);
+
+	if (ret)
+		dev_err(ctrl->dev, "power off failed for SerDes %d\n", sid);
+
+	return ret;
+}
+
+static int rtsds_phy_set_mode_int(struct rtsds_ctrl *ctrl, u32 sid, int phymode, int hwmode)
+{
+	int ret;
+
+	mutex_lock(&ctrl->lock);
+	ret = ctrl->conf->set_mode(ctrl, sid, hwmode);
+	if (!ret)
+		ctrl->sds[sid].mode = phymode;
+	mutex_unlock(&ctrl->lock);
+
+	if (ret)
+		dev_err(ctrl->dev, "set mode failed for SerDes %d\n", sid);
+
+	return ret;
+}
+
+static int rtsds_phy_set_mode(struct phy *phy, enum phy_mode mode, int submode)
+{
+	struct rtsds_macro *macro = phy_get_drvdata(phy);
+	struct rtsds_ctrl *ctrl = macro->ctrl;
+	u32 sid = macro->sid;
+
+	if (rtsds_readonly(ctrl, sid))
+		return 0;
+
+	if (mode != PHY_MODE_ETHERNET)
+		return -EINVAL;
+
+	return rtsds_phy_set_mode_int(ctrl, sid, submode, ctrl->conf->mode_map[submode]);
+}
+
+static int rtsds_phy_reset_int(struct rtsds_ctrl *ctrl, u32 sid)
+{
+	int ret;
+
+	mutex_lock(&ctrl->lock);
+	ret = ctrl->conf->reset(ctrl, sid);
+	mutex_unlock(&ctrl->lock);
+
+	if (ret)
+		dev_err(ctrl->dev, "reset failed for SerDes %d\n", sid);
+
+	return ret;
+}
+
+static int rtsds_phy_reset(struct phy *phy)
+{
+	struct rtsds_macro *macro = phy_get_drvdata(phy);
+	struct rtsds_ctrl *ctrl = macro->ctrl;
+	u32 sid = macro->sid;
+
+	if (rtsds_readonly(ctrl, sid))
+		return 0;
+
+	return rtsds_phy_reset_int(ctrl, sid);
+}
+
+static const struct phy_ops rtsds_phy_ops = {
+	.init		= rtsds_phy_init,
+	.power_on	= rtsds_phy_power_on,
+	.power_off	= rtsds_phy_power_off,
+	.reset		= rtsds_phy_reset,
+	.set_mode	= rtsds_phy_set_mode,
+	.owner		= THIS_MODULE,
+};
+
+/*
+ * The SerDes offer a lot of magic that sill needs to be uncovered. To help further development
+ * provide some basic debugging about registers, modes and polarity. The mode can be changed on
+ * the fly and executes the normal setter including events.
+ */
+
+#ifdef CONFIG_DEBUG_FS
+
+#define RTSDS_PAGE_NAMES 48
+
+static const char *rtsds_page_name[RTSDS_PAGE_NAMES] = {
+	[0] = "SDS",		[1] = "SDS_EXT",
+	[2] = "FIB",		[3] = "FIB_EXT",
+	[4] = "DTE",		[5] = "DTE_EXT",
+	[6] = "TGX",		[7] = "TGX_EXT",
+	[8] = "ANA_RG",		[9] = "ANA_RG_EXT",
+	[10] = "ANA_TG",	[11] = "ANA_TG_EXT",
+	[31] = "ANA_WDIG",
+	[32] = "ANA_MISC",	[33] = "ANA_COM",
+	[34] = "ANA_SP",	[35] = "ANA_SP_EXT",
+	[36] = "ANA_1G",	[37] = "ANA_1G_EXT",
+	[38] = "ANA_2G",	[39] = "ANA_2G_EXT",
+	[40] = "ANA_3G",	[41] = "ANA_3G_EXT",
+	[42] = "ANA_5G",	[43] = "ANA_5G_EXT",
+	[44] = "ANA_6G",	[45] = "ANA_6G_EXT",
+	[46] = "ANA_10G",	[47] = "ANA_10G_EXT",
+};
+
+static ssize_t rtsds_dbg_mode_show(struct seq_file *seqf, void *unused)
+{
+	struct rtsds_macro *macro = dev_get_drvdata(seqf->private);
+	struct rtsds_ctrl *ctrl = macro->ctrl;
+	int mode, sid = macro->sid;
+
+	mutex_lock(&ctrl->lock);
+	mode = ctrl->conf->get_mode(ctrl, sid);
+	mutex_unlock(&ctrl->lock);
+
+	seq_printf(seqf, "hw mode: 0x%X\n", mode);
+	seq_puts(seqf, "phy mode: ");
+
+	if (ctrl->sds[sid].mode == PHY_INTERFACE_MODE_NA)
+		seq_puts(seqf, "off\n");
+	else
+		seq_printf(seqf, "%s\n", phy_modes(ctrl->sds[sid].mode));
+
+	return 0;
+}
+
+static ssize_t rtsds_dbg_mode_write(struct file *file, const char __user *userbuf,
+				size_t count, loff_t *ppos)
+{
+	struct seq_file *seqf = file->private_data;
+	struct rtsds_macro *macro = dev_get_drvdata(seqf->private);
+	struct rtsds_ctrl *ctrl = macro->ctrl;
+	int ret, hwmode, phymode, sid = macro->sid;
+
+	ret = kstrtou32_from_user(userbuf, count, 16, &hwmode);
+	if (ret)
+		return ret;
+	/*
+	 * Allow to set arbitrary modes into the SerDes to improve error analysis. Accept that
+	 * this might confuse the internal state tracking.
+	 */
+	phymode = rtsds_hwmode_to_phymode(ctrl, hwmode);
+	rtsds_phy_set_mode_int(ctrl, sid, phymode, hwmode);
+
+	return count;
+}
+DEFINE_SHOW_STORE_ATTRIBUTE(rtsds_dbg_mode);
+
+static ssize_t rtsds_dbg_reset_show(struct seq_file *seqf, void *unused)
+{
+	return 0;
+}
+
+static ssize_t rtsds_dbg_reset_write(struct file *file, const char __user *userbuf,
+				size_t count, loff_t *ppos)
+{
+	struct seq_file *seqf = file->private_data;
+	struct rtsds_macro *macro = dev_get_drvdata(seqf->private);
+	struct rtsds_ctrl *ctrl = macro->ctrl;
+	int ret, reset, sid = macro->sid;
+
+	ret = kstrtou32_from_user(userbuf, count, 10, &reset);
+	if (ret || reset != 1)
+		return ret;
+
+	rtsds_phy_reset_int(ctrl, sid);
+
+	return count;
+}
+DEFINE_SHOW_STORE_ATTRIBUTE(rtsds_dbg_reset);
+
+static int rtsds_dbg_registers_show(struct seq_file *seqf, void *unused)
+{
+	struct rtsds_macro *macro = dev_get_drvdata(seqf->private);
+	struct rtsds_ctrl *ctrl = macro->ctrl;
+	u32 page = 0, reg, sid = macro->sid;
+
+	seq_printf(seqf, "%*s", 12, "");
+	for (int i = 0; i < 32; i++)
+		seq_printf(seqf, "   %02X", i);
+
+	while (page < ctrl->conf->page_cnt) {
+		if (page < RTSDS_PAGE_NAMES && rtsds_page_name[page])
+			seq_printf(seqf, "\n%*s: ", -11, rtsds_page_name[page]);
+		else if (page == 64 || page == 128)
+			seq_printf(seqf, "\n\nXGMII_%d    : ", page >> 8);
+		else
+			seq_printf(seqf, "\nPAGE_%03d   : ", page);
+		for (reg = 0; reg < 32; reg++)
+			seq_printf(seqf, "%04X ", ctrl->conf->read(ctrl, sid, page, reg));
+
+		page++;
+	}
+	seq_puts(seqf, "\n");
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(rtsds_dbg_registers);
+
+static int rtsds_dbg_polarity_show(struct seq_file *seqf, void *unused)
+{
+	struct rtsds_macro *macro = dev_get_drvdata(seqf->private);
+	struct rtsds_ctrl *ctrl = macro->ctrl;
+	u32 reg, sid = macro->sid;
+
+	reg = ctrl->conf->read(ctrl, sid, RTSDS_PAGE_SDS, 0);
+
+	seq_puts(seqf, "tx polarity: ");
+	seq_puts(seqf, reg & RTSDS_BITS_INV_HSO ? "inverse" : "normal");
+	seq_puts(seqf, "\nrx polarity: ");
+	seq_puts(seqf, reg & RTSDS_BITS_INV_HSI ? "inverse" : "normal");
+	seq_puts(seqf, "\n");
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(rtsds_dbg_polarity);
+
+static void rtsds_dbg_init(struct rtsds_ctrl *ctrl, u32 sid)
+{
+	debugfs_create_file("mode", 0600, ctrl->sds[sid].phy->debugfs,
+			    &ctrl->sds[sid].phy->dev, &rtsds_dbg_mode_fops);
+
+	debugfs_create_file("reset", 0200, ctrl->sds[sid].phy->debugfs,
+			    &ctrl->sds[sid].phy->dev, &rtsds_dbg_reset_fops);
+
+	debugfs_create_file("polarity", 0400, ctrl->sds[sid].phy->debugfs,
+			    &ctrl->sds[sid].phy->dev, &rtsds_dbg_polarity_fops);
+
+	debugfs_create_file("registers", 0400, ctrl->sds[sid].phy->debugfs,
+			    &ctrl->sds[sid].phy->dev, &rtsds_dbg_registers_fops);
+}
+#endif /* CONFIG_DEBUG_FS */
+
+static void rtsds_setup(struct rtsds_ctrl *ctrl)
+{
+	int hwmode, ret;
+
+	for (u32 sid = 0; sid < ctrl->conf->sds_cnt; sid++) {
+		if (ctrl->sds_mask & BIT(sid)) {
+			/* power off controlled SerDes */
+			hwmode = ctrl->conf->mode_map[PHY_INTERFACE_MODE_NA];
+			ret = ctrl->conf->set_mode(ctrl, sid, hwmode);
+			if (!ret)
+				ret = rtsds_run_event(ctrl, sid, RTSDS_EVENT_SETUP);
+			if (ret)
+				dev_err(ctrl->dev, "setup failed for SerDes %d\n", sid);
+		}
+		/* in any case sync back hardware status */
+		hwmode = ctrl->conf->get_mode(ctrl, sid);
+		ctrl->sds[sid].mode = rtsds_hwmode_to_phymode(ctrl, hwmode);
+	}
+}
+
+static struct phy *rtsds_simple_xlate(struct device *dev,
+				      const struct of_phandle_args *args)
+{
+	struct rtsds_ctrl *ctrl = dev_get_drvdata(dev);
+	int sid, sid2, min_port, max_port;
+
+	/*
+	 * Some Realtek Ethernet transceivers (e.g. RLT8218B) will be attached via a bonded 2x
+	 * QSGMII link to two SerDes. Others (e.g. RTL8218D) allow to make use of single XGMII or
+	 * dual QSGMII links. When a switch port tries to lookup the SerDes it is attached to
+	 * honour that by an enhanced mapping. Allow two possible configuration options. Either
+	 * standalone or linked to another. E.g.
+	 *
+	 * Single: port@24 { phys = <&serdes 4 -1 MinPort MaxPort>; };
+	 * Dual:   port@24 { phys = <&serdes 4  5 MinPort MaxPort>; };
+	 *
+	 * This function will return the primary PHY of the link. The secondary can be identified
+	 * later on by the link attribute in the controller structure.
+	 */
+
+	if (args->args_count != 4)
+		return ERR_PTR(-EINVAL);
+
+	sid = args->args[0];
+	if (sid < 0 || sid >= ctrl->conf->sds_cnt)
+		return ERR_PTR(-EINVAL);
+
+	sid2 = args->args[1];
+	if (sid2 < -1 || sid2 >= ctrl->conf->sds_cnt)
+		return ERR_PTR(-EINVAL);
+
+	/*
+	 * Additionally to a linked SerDes also get the ports whose traffic is going through this
+	 * SerDes. As of now the driver does not care about that but later on it will be helpful.
+	 */
+
+	min_port = args->args[2];
+	if (min_port < 0)
+		return ERR_PTR(-EINVAL);
+
+	max_port = args->args[3];
+	if (max_port < min_port)
+		return ERR_PTR(-EINVAL);
+
+	ctrl->sds[sid].link = sid2;
+	if (sid2 >= 0)
+		ctrl->sds[sid2].link = sid;
+
+	ctrl->sds[sid].min_port = min_port;
+	ctrl->sds[sid].max_port = max_port;
+
+	return ctrl->sds[sid].phy;
+}
+
+static int rtsds_phy_create(struct rtsds_ctrl *ctrl, u32 sid)
+{
+	struct rtsds_macro *macro;
+
+	ctrl->sds[sid].phy = devm_phy_create(ctrl->dev, NULL, &rtsds_phy_ops);
+	if (IS_ERR(ctrl->sds[sid].phy))
+		return PTR_ERR(ctrl->sds[sid].phy);
+
+	macro = devm_kzalloc(ctrl->dev, sizeof(*macro), GFP_KERNEL);
+	if (!macro)
+		return -ENOMEM;
+
+	macro->sid = sid;
+	macro->ctrl = ctrl;
+	phy_set_drvdata(ctrl->sds[sid].phy, macro);
+
+	ctrl->sds[sid].link = -1;
+	ctrl->sds[sid].min_port = -1;
+	ctrl->sds[sid].max_port = -1;
+
+#ifdef CONFIG_DEBUG_FS
+	rtsds_dbg_init(ctrl, sid);
+#endif
+	return 0;
+}
+
+static int rtsds_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
+	struct phy_provider *provider;
+	struct rtsds_ctrl *ctrl;
+	int ret;
+
+	if (!np)
+		return -EINVAL;
+
+	ctrl = devm_kzalloc(dev, sizeof(*ctrl), GFP_KERNEL);
+	if (!ctrl)
+		return -ENOMEM;
+
+	ctrl->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(ctrl->base)) {
+		dev_err(dev, "failed to map SerDes memory\n");
+		return PTR_ERR(ctrl->base);
+	}
+
+	ctrl->dev = dev;
+	ctrl->conf = (struct rtsds_conf *)of_device_get_match_data(dev);
+
+	ret = of_property_read_u32(np, "realtek,controlled-ports", &ctrl->sds_mask);
+	if (ret)
+		ctrl->sds_mask = GENMASK(ctrl->conf->sds_cnt, 0);
+
+	for (u32 sid = 0; sid < ctrl->conf->sds_cnt; sid++) {
+		ret = rtsds_phy_create(ctrl, sid);
+		if (ret) {
+			dev_err(dev, "failed to create PHY for SerDes %d\n", sid);
+			return ret;
+		}
+	}
+
+	mutex_init(&ctrl->lock);
+	dev_set_drvdata(dev, ctrl);
+	provider = devm_of_phy_provider_register(dev, rtsds_simple_xlate);
+	rtsds_setup(ctrl);
+	dev_info(dev, "initialized (%d SerDes, %d pages, 32 registers, mask 0x%04x)",
+		 ctrl->conf->sds_cnt, ctrl->conf->page_cnt, ctrl->sds_mask);
+
+	return PTR_ERR_OR_ZERO(provider);
+}
+
+#define MSK_ALL(a, b, c, d, e)	{ RTSDS_SEQ_MASK, PHY_INTERFACE_MODE_NA, a, b, c, d, e }
+#define MSK_QSG(a, b, c, d, e)	{ RTSDS_SEQ_MASK, PHY_INTERFACE_MODE_QSGMII, a, b, c, d, e }
+#define WAIT_MS(v)		{ RTSDS_SEQ_WAIT, 0, 0, 0, 0, v, 0 }
+#define END_SEQ			{ RTSDS_SEQ_STOP, 0, 0, 0, 0, 0, 0 }
+
+static struct rtsds_seq rtsds_838x_seq_setup[] = {
+	WAIT_MS(1),
+	/* all ports */
+	MSK_ALL(0x003f, 0x02, 0x19, 0x0303, 0xffff),	MSK_ALL(0x003f, 0x01, 0x02, 0x85fa, 0xffff),
+	/* ports 0-3 */
+	MSK_ALL(0x0005, 0x01, 0x11, 0xb7c9, 0xffff),	MSK_ALL(0x000a, 0x01, 0x11, 0x4208, 0xffff),
+	MSK_ALL(0x0005, 0x01, 0x12, 0xab8e, 0xffff),	MSK_ALL(0x000a, 0x01, 0x12, 0xc208, 0xffff),
+	MSK_ALL(0x0005, 0x01, 0x13, 0x24ab, 0xffff),	MSK_ALL(0x0001, 0x01, 0x03, 0xf46f, 0xffff),
+	MSK_ALL(0x0002, 0x01, 0x0a, 0x80c7, 0xffff),	MSK_ALL(0x0004, 0x01, 0x03, 0xf46d, 0xffff),
+	MSK_ALL(0x000f, 0x01, 0x0b, 0x0482, 0xffff),	MSK_ALL(0x000f, 0x01, 0x0e, 0xfcc2, 0xffff),
+	MSK_ALL(0x000f, 0x00, 0x01, 0x0f00, 0xffff),	MSK_ALL(0x000f, 0x00, 0x02, 0x7060, 0xffff),
+	MSK_ALL(0x000f, 0x01, 0x06, 0x20d8, 0xffff),
+	/* port 4 */
+	MSK_ALL(0x0010, 0x01, 0x06, 0x20d8, 0xffff),	MSK_ALL(0x0010, 0x01, 0x0a, 0x00c3, 0xffff),
+	MSK_ALL(0x0010, 0x01, 0x0b, 0x1482, 0xffff),	MSK_ALL(0x0010, 0x01, 0x0e, 0xfcc2, 0xffff),
+	MSK_ALL(0x0010, 0x01, 0x11, 0xb7c9, 0xffff),	MSK_ALL(0x0010, 0x01, 0x12, 0xab8e, 0xffff),
+	/* port 5 */
+	MSK_ALL(0x0020, 0x01, 0x03, 0x0000, 0xffff),	MSK_ALL(0x0020, 0x01, 0x04, 0xdccc, 0xffff),
+	MSK_ALL(0x0020, 0x01, 0x05, 0x0000, 0xffff),	MSK_ALL(0x0020, 0x01, 0x06, 0x3600, 0xffff),
+	MSK_ALL(0x0020, 0x01, 0x07, 0x0003, 0xffff),	MSK_ALL(0x0020, 0x01, 0x08, 0x79aa, 0xffff),
+	MSK_ALL(0x0020, 0x01, 0x09, 0x8c64, 0xffff),	MSK_ALL(0x0020, 0x01, 0x0a, 0x00c3, 0xffff),
+	MSK_ALL(0x0020, 0x01, 0x0b, 0x1482, 0xffff),	MSK_ALL(0x0020, 0x01, 0x0e, 0xf002, 0xffff),
+	MSK_ALL(0x0020, 0x02, 0x1b, 0x04bf, 0xffff),	MSK_ALL(0x0020, 0x02, 0x18, 0x14aa, 0xffff),
+	MSK_ALL(0x0020, 0x02, 0x1b, 0x04bf, 0xffff),	END_SEQ,
+};
+
+static struct rtsds_seq rtsds_839x_seq_setup[] = {
+	/* ports 0-7, 10-11 (5G) */
+	MSK_ALL(0x0cff, 0x08, 0x11, 0x0000, 0x8000),	MSK_ALL(0x0cff, 0x08, 0x11, 0x1000, 0x7000),
+	MSK_ALL(0x0cff, 0x08, 0x11, 0x0400, 0x0e00),	MSK_ALL(0x0cff, 0x08, 0x11, 0x00c0, 0x01c0),
+	MSK_ALL(0x0cff, 0x08, 0x16, 0x0000, 0x8000),	MSK_ALL(0x0cff, 0x08, 0x08, 0x0000, 0x0008),
+	MSK_ALL(0x0cff, 0x08, 0x07, 0x0000, 0x0200),	MSK_ALL(0x0cff, 0x08, 0x07, 0x0000, 0x0100),
+	MSK_ALL(0x0cff, 0x08, 0x14, 0x0002, 0x0002),	MSK_ALL(0x0cff, 0x08, 0x15, 0xc000, 0xf000),
+	MSK_ALL(0x0cff, 0x08, 0x16, 0x0000, 0x1000),	MSK_ALL(0x0cff, 0x08, 0x18, 0x0006, 0x003f),
+	MSK_ALL(0x0cff, 0x08, 0x08, 0x0000, 0x0040),	MSK_ALL(0x0cff, 0x08, 0x08, 0x0000, 0x0800),
+	MSK_ALL(0x0cff, 0x08, 0x08, 0x8000, 0xf000),	MSK_ALL(0x0cff, 0x08, 0x08, 0x0400, 0x0780),
+	/* ports 10-11 (clock edge)			ports 4, 10 (full swing)) */
+	MSK_ALL(0x0c00, 0x00, 0x07, 0x4000, 0x4000),	MSK_ALL(0x0410, 0x08, 0x08, 0x8c6a, 0xffff),
+	/* ports 8-9, 12-13 (10G digital patch) */
+	MSK_ALL(0x3300, 0x0a, 0x00, 0x5800, 0xffff),	MSK_ALL(0x3300, 0x0a, 0x01, 0x4000, 0xffff),
+	MSK_ALL(0x1100, 0x0a, 0x02, 0x5400, 0xffff),	MSK_ALL(0x2200, 0x0a, 0x02, 0x5000, 0xffff),
+	MSK_ALL(0x3300, 0x0a, 0x03, 0x0000, 0xffff),	MSK_ALL(0x3300, 0x0a, 0x04, 0x0000, 0xffff),
+	MSK_ALL(0x3300, 0x0a, 0x05, 0x4000, 0xffff),	MSK_ALL(0x3300, 0x0a, 0x06, 0x4000, 0xffff),
+	MSK_ALL(0x3300, 0x0a, 0x07, 0xffff, 0xffff),	MSK_ALL(0x3300, 0x0a, 0x08, 0xffff, 0xffff),
+	MSK_ALL(0x3300, 0x0a, 0x09, 0x806f, 0xffff),	MSK_ALL(0x3300, 0x0a, 0x0a, 0x0004, 0xffff),
+	MSK_ALL(0x3300, 0x0a, 0x0b, 0x0000, 0xffff),	MSK_ALL(0x3300, 0x0a, 0x0c, 0x0000, 0xffff),
+	MSK_ALL(0x3300, 0x0a, 0x0d, 0x0000, 0xffff),	MSK_ALL(0x3300, 0x0a, 0x0e, 0x0a00, 0xffff),
+	MSK_ALL(0x3300, 0x0a, 0x0f, 0x2000, 0xffff),	MSK_ALL(0x3300, 0x0a, 0x10, 0xf00e, 0xffff),
+	MSK_ALL(0x1100, 0x0a, 0x11, 0xf04a, 0xffff),	MSK_ALL(0x2200, 0x0a, 0x11, 0xfdab, 0xffff),
+	MSK_ALL(0x1100, 0x0a, 0x12, 0x97b3, 0xffff),	MSK_ALL(0x2200, 0x0a, 0x12, 0x96ea, 0xffff),
+	MSK_ALL(0x3300, 0x0a, 0x13, 0x5318, 0xffff),	MSK_ALL(0x3300, 0x0a, 0x14, 0x0f03, 0xffff),
+	MSK_ALL(0x3300, 0x0a, 0x15, 0x0000, 0xffff),	MSK_ALL(0x3300, 0x0a, 0x16, 0x0000, 0xffff),
+	MSK_ALL(0x3300, 0x0a, 0x17, 0x0000, 0xffff),	MSK_ALL(0x3300, 0x0a, 0x18, 0x0000, 0xffff),
+	MSK_ALL(0x3300, 0x0a, 0x19, 0x0000, 0xffff),	MSK_ALL(0x3300, 0x0a, 0x1a, 0xffff, 0xffff),
+	MSK_ALL(0x3300, 0x0a, 0x1b, 0x0000, 0xffff),	MSK_ALL(0x3300, 0x0a, 0x1c, 0x1203, 0xffff),
+	MSK_ALL(0x3300, 0x0a, 0x1d, 0x0000, 0xffff),	MSK_ALL(0x3300, 0x0a, 0x1e, 0xa052, 0xffff),
+	MSK_ALL(0x3300, 0x0a, 0x1f, 0x9a00, 0xffff),	MSK_ALL(0x3300, 0x0b, 0x00, 0x00f5, 0xffff),
+	MSK_ALL(0x3300, 0x0b, 0x01, 0xf000, 0xffff),	MSK_ALL(0x1100, 0x0b, 0x02, 0x41ff, 0xffff),
+	MSK_ALL(0x2200, 0x0b, 0x02, 0x4079, 0xffff),	MSK_ALL(0x3300, 0x0b, 0x03, 0x0000, 0xffff),
+	MSK_ALL(0x1100, 0x0b, 0x04, 0x39ff, 0xffff),	MSK_ALL(0x2200, 0x0b, 0x04, 0x93fa, 0xffff),
+	MSK_ALL(0x3300, 0x0b, 0x05, 0x3340, 0xffff),	MSK_ALL(0x1100, 0x0b, 0x06, 0x40aa, 0xffff),
+	MSK_ALL(0x2200, 0x0b, 0x06, 0x4280, 0xffff),	MSK_ALL(0x3300, 0x0b, 0x07, 0x0000, 0xffff),
+	MSK_ALL(0x3300, 0x0b, 0x08, 0x801f, 0xffff),	MSK_ALL(0x3300, 0x0b, 0x09, 0x0000, 0xffff),
+	MSK_ALL(0x3300, 0x0b, 0x0a, 0x619c, 0xffff),	MSK_ALL(0x3300, 0x0b, 0x0b, 0xffed, 0xffff),
+	MSK_ALL(0x3300, 0x0b, 0x0c, 0x29ff, 0xffff),	MSK_ALL(0x3300, 0x0b, 0x0d, 0x29ff, 0xffff),
+	MSK_ALL(0x1100, 0x0b, 0x0e, 0x4e10, 0xffff),	MSK_ALL(0x2200, 0x0b, 0x0e, 0x4c50, 0xffff),
+	MSK_ALL(0x1100, 0x0b, 0x0f, 0x4e10, 0xffff),	MSK_ALL(0x2200, 0x0b, 0x0f, 0x4c50, 0xffff),
+	MSK_ALL(0x3300, 0x0b, 0x10, 0x0000, 0xffff),	MSK_ALL(0x3300, 0x0b, 0x11, 0x0000, 0xffff),
+	MSK_ALL(0x3300, 0x00, 0x0c, 0x08ec, 0xffff),	MSK_ALL(0x2200, 0x0b, 0x1f, 0x003f, 0xffff),
+	MSK_ALL(0x3300, 0x00, 0x07, 0x4000, 0x4000),
+	/* ports 8-9, 12-13 (10G analog patch) */
+	MSK_ALL(0x3300, 0x0b, 0x09, 0x417f, 0xffff),	MSK_ALL(0x3300, 0x0a, 0x1c, 0x0000, 0x0200),
+	MSK_ALL(0x3300, 0x0a, 0x1c, 0x0000, 0x1c00),	MSK_ALL(0x3300, 0x0a, 0x1c, 0x0028, 0x0038),
+	MSK_ALL(0x3300, 0x0a, 0x1c, 0x0000, 0x01c0),	MSK_ALL(0x3300, 0x0a, 0x1c, 0x0002, 0x0007),
+	MSK_ALL(0x3300, 0x0b, 0x01, 0xc440, 0xffff),	MSK_ALL(0x1100, 0x0b, 0x06, 0x0000, 0x0008),
+	MSK_ALL(0x3300, 0x0a, 0x05, 0x8000, 0xffff),	MSK_ALL(0x3300, 0x0a, 0x06, 0x8000, 0xffff),
+	MSK_ALL(0x3300, 0x0a, 0x0a, 0x0000, 0xffff),	MSK_ALL(0x3300, 0x0a, 0x1e, 0x0002, 0xffff),
+	MSK_ALL(0x3300, 0x0a, 0x1f, 0xbe00, 0xffff),	MSK_ALL(0x1100, 0x0b, 0x0e, 0x0000, 0x0400),
+	MSK_ALL(0x1100, 0x0b, 0x0f, 0x0000, 0x0400),	MSK_ALL(0x1100, 0x0b, 0x0e, 0x0000, 0x4000),
+	MSK_ALL(0x1100, 0x0b, 0x0f, 0x0000, 0x4000),	MSK_ALL(0x3300, 0x0a, 0x10, 0x0000, 0x0020),
+	MSK_ALL(0x3300, 0x0b, 0x09, 0x0000, 0x0100),	MSK_ALL(0x3300, 0x0a, 0x03, 0xf000, 0xf000),
+	MSK_ALL(0x3300, 0x0a, 0x1f, 0x3000, 0x3000),	MSK_ALL(0x3300, 0x0a, 0x1f, 0x0e00, 0x0e00),
+	MSK_ALL(0x3300, 0x0b, 0x01, 0x8000, 0x8000),	MSK_ALL(0x3300, 0x0b, 0x01, 0x4000, 0x4000),
+	MSK_ALL(0x3300, 0x0b, 0x01, 0x0000, 0x2000),	MSK_ALL(0x3300, 0x0b, 0x01, 0x0000, 0x1000),
+	MSK_ALL(0x3300, 0x0b, 0x01, 0x0400, 0x0e00),	MSK_ALL(0x3300, 0x0b, 0x01, 0x0080, 0x01c0),
+	MSK_ALL(0x3300, 0x0b, 0x01, 0x0000, 0x0038),	MSK_ALL(0x3300, 0x0b, 0x01, 0x0000, 0x0007),
+	MSK_ALL(0x3300, 0x0b, 0x0c, 0x0200, 0x0200),	MSK_ALL(0x3300, 0x0b, 0x0d, 0x0200, 0x0200),
+	MSK_ALL(0x3300, 0x0b, 0x08, 0x0020, 0x0020),	MSK_ALL(0x3300, 0x0b, 0x08, 0x0000, 0x0040),
+	MSK_ALL(0x3300, 0x0a, 0x1c, 0x0000, 0x8000),	MSK_ALL(0x3300, 0x0a, 0x10, 0x0000, 0xf000),
+	MSK_ALL(0x3300, 0x0a, 0x13, 0x0000, 0x0010),	MSK_ALL(0x3300, 0x0a, 0x13, 0x0000, 0x0200),
+	MSK_ALL(0x3300, 0x0a, 0x13, 0x0008, 0x000f),	MSK_ALL(0x3300, 0x0a, 0x13, 0x0100, 0x01e0),
+	END_SEQ,
+};
+
+static const struct rtsds_conf rtsds_838x_conf = {
+	.sds_cnt	= RTSDS_838X_SDS_CNT,
+	.page_cnt	= RTSDS_838X_PAGE_CNT,
+	.mask		= rtsds_838x_mask,
+	.read		= rtsds_838x_read,
+	.reset		= rtsds_838x_reset,
+	.set_mode	= rtsds_838x_set_mode,
+	.get_mode	= rtsds_838x_get_mode,
+	.mode_map = {
+		[PHY_INTERFACE_MODE_NA]		= RTSDS_COMBOMODE(0x00, 0x00),
+		[PHY_INTERFACE_MODE_1000BASEX]	= RTSDS_COMBOMODE(0x04, 0x01),
+		[PHY_INTERFACE_MODE_100BASEX]	= RTSDS_COMBOMODE(0x05, 0x01),
+		[PHY_INTERFACE_MODE_QSGMII]	= RTSDS_COMBOMODE(0x06, 0x00),
+	},
+	.sequence = {
+		[RTSDS_EVENT_SETUP]		= rtsds_838x_seq_setup,
+	}
+};
+
+static const struct rtsds_conf rtsds_839x_conf = {
+	.sds_cnt	= RTSDS_839X_SDS_CNT,
+	.page_cnt	= RTSDS_839X_PAGE_CNT,
+	.mask		= rtsds_839x_mask,
+	.read		= rtsds_839x_read,
+	.reset		= rtsds_839x_reset,
+	.set_mode	= rtsds_839x_set_mode,
+	.get_mode	= rtsds_839x_get_mode,
+	.mode_map = {
+		[PHY_INTERFACE_MODE_NA]		= RTSDS_COMBOMODE(0x00, 0x00),
+		[PHY_INTERFACE_MODE_10GBASER]	= RTSDS_COMBOMODE(0x01, 0x00),
+		[PHY_INTERFACE_MODE_1000BASEX]	= RTSDS_COMBOMODE(0x07, 0x00),
+		[PHY_INTERFACE_MODE_100BASEX]	= RTSDS_COMBOMODE(0x08, 0x00),
+		[PHY_INTERFACE_MODE_QSGMII]	= RTSDS_COMBOMODE(0x06, 0x00),
+		[PHY_INTERFACE_MODE_SGMII]	= RTSDS_COMBOMODE(0x07, 0x05),
+	},
+	.sequence = {
+		[RTSDS_EVENT_SETUP]		= rtsds_839x_seq_setup,
+	}
+};
+
+static const struct rtsds_conf rtsds_930x_conf = {
+	.sds_cnt	= RTSDS_930X_SDS_CNT,
+	.page_cnt	= RTSDS_930X_PAGE_CNT,
+	.mask		= rtsds_930x_mask,
+	.read		= rtsds_930x_read,
+	.reset		= rtsds_930x_reset,
+	.set_mode	= rtsds_930x_set_mode,
+	.get_mode	= rtsds_930x_get_mode,
+	.mode_map = {
+		[PHY_INTERFACE_MODE_NA]		= RTSDS_COMBOMODE(0x1f, 0x00),
+		[PHY_INTERFACE_MODE_10GBASER]	= RTSDS_COMBOMODE(0x1a, 0x00),
+		[PHY_INTERFACE_MODE_2500BASEX]  = RTSDS_COMBOMODE(0x16, 0x00),
+		[PHY_INTERFACE_MODE_1000BASEX]	= RTSDS_COMBOMODE(0x04, 0x00),
+		[PHY_INTERFACE_MODE_USXGMII]	= RTSDS_COMBOMODE(0x0d, 0x00),
+		[PHY_INTERFACE_MODE_QUSGMII]	= RTSDS_COMBOMODE(0x0d, 0x02),
+		[PHY_INTERFACE_MODE_QSGMII]	= RTSDS_COMBOMODE(0x06, 0x00),
+	},
+	.sequence = {
+		[RTSDS_EVENT_SETUP]		= NULL,
+	}
+};
+
+static const struct rtsds_conf rtsds_931x_conf = {
+	.sds_cnt	= RTSDS_931X_SDS_CNT,
+	.page_cnt	= RTSDS_931X_PAGE_CNT,
+	.mask		= rtsds_931x_mask,
+	.read		= rtsds_931x_read,
+	.reset		= rtsds_931x_reset,
+	.set_mode	= rtsds_931x_set_mode,
+	.get_mode	= rtsds_931x_get_mode,
+	.mode_map = {
+		[PHY_INTERFACE_MODE_NA]		= RTSDS_COMBOMODE(0x1f, 0x3f),
+		[PHY_INTERFACE_MODE_10GBASER]	= RTSDS_COMBOMODE(0x1f, 0x35),
+		[PHY_INTERFACE_MODE_1000BASEX]	= RTSDS_COMBOMODE(0x1f, 0x39),
+		[PHY_INTERFACE_MODE_USXGMII]	= RTSDS_COMBOMODE(0x0d, 0x00),
+		[PHY_INTERFACE_MODE_XGMII]	= RTSDS_COMBOMODE(0x0a, 0x00),
+		[PHY_INTERFACE_MODE_QSGMII]	= RTSDS_COMBOMODE(0x06, 0x00),
+	},
+	.sequence = {
+		[RTSDS_EVENT_SETUP]		= NULL,
+	}
+};
+
+static const struct of_device_id rtsds_compatible_ids[] = {
+	{ .compatible = "realtek,rtl8380-serdes",
+	  .data = &rtsds_838x_conf,
+	},
+	{ .compatible = "realtek,rtl8390-serdes",
+	  .data = &rtsds_839x_conf,
+	},
+	{ .compatible = "realtek,rtl9300-serdes",
+	  .data = &rtsds_930x_conf,
+	},
+	{ .compatible = "realtek,rtl9310-serdes",
+	  .data = &rtsds_931x_conf,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, rtsds_compatible_ids);
+
+static struct platform_driver rtsds_platform_driver = {
+	.probe		= rtsds_probe,
+	.driver		= {
+		.name	= "realtek,otto-serdes",
+		.of_match_table = of_match_ptr(rtsds_compatible_ids),
+	},
+};
+
+module_platform_driver(rtsds_platform_driver);
+
+MODULE_AUTHOR("Markus Stockhausen <markus.stockhausen@gmx.de>");
+MODULE_DESCRIPTION("SerDes driver for Realtek RTL83xx, RTL93xx switch SoCs");
+MODULE_LICENSE("GPL");
diff --git a/drivers/phy/realtek/phy-rtk-otto-serdes.h b/drivers/phy/realtek/phy-rtk-otto-serdes.h
new file mode 100644
index 000000000000..f1566d9393c6
--- /dev/null
+++ b/drivers/phy/realtek/phy-rtk-otto-serdes.h
@@ -0,0 +1,117 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Realtek RTL838x, RTL839x, RTL930x & RTL931x SerDes PHY driver
+ * Copyright (c) 2024 Markus Stockhausen <markus.stockhausen@gmx.de>
+ */
+
+#ifndef _PHY_RTK_OTTO_SERDES_H
+#define _PHY_RTK_OTTO_SERDES_H
+
+#define RTSDS_SWITCH_ADDR_BASE		(0xbb000000)
+#define RTSDS_REG(x)			((void __iomem __force *)RTSDS_SWITCH_ADDR_BASE + (x))
+#define iomask32(mask, value, addr)	iowrite32((ioread32(addr) & ~(mask)) | (value), addr)
+
+#define RTSDS_PAGE_SDS				0x00
+#define RTSDS_PAGE_SDS_EXT			0x01
+#define RTSDS_PAGE_FIB				0x02
+#define RTSDS_PAGE_FIB_EXT			0x03
+
+#define RTSDS_BITS_INV_HSO			BIT(8)
+#define RTSDS_BITS_INV_HSI			BIT(9)
+#define RTSDS_BITS_SOFT_RST			BIT(6)
+#define RTSDS_BITS_SDS_EN			GENMASK(1, 0)
+
+#define RTSDS_EVENT_SETUP			0
+#define RTSDS_EVENT_CNT				1
+
+#define RTSDS_SEQ_STOP				0
+#define RTSDS_SEQ_MASK				1
+#define RTSDS_SEQ_WAIT				2
+
+#define RTSDS_838X_SDS_CNT			6
+#define RTSDS_838X_PAGE_CNT			4
+#define RTSDS_838X_SDS_MODE_SEL			RTSDS_REG(0x0028)
+#define RTSDS_838X_INT_MODE_CTRL		RTSDS_REG(0x005c)
+
+#define RTSDS_839X_SDS_CNT			14
+#define RTSDS_839X_PAGE_CNT			12
+#define RTSDS_839X_MAC_SERDES_IF_CTRL		RTSDS_REG(0x0008)
+
+#define RTSDS_930X_SDS_CNT			12
+#define RTSDS_930X_PAGE_CNT			64
+#define RTSDS_930X_SDS_MODE_SEL_0		RTSDS_REG(0x0194)
+#define RTSDS_930X_SDS_MODE_SEL_1		RTSDS_REG(0x02a0)
+#define RTSDS_930X_SDS_MODE_SEL_2		RTSDS_REG(0x02a4)
+#define RTSDS_930X_SDS_MODE_SEL_3		RTSDS_REG(0x0198)
+#define RTSDS_930X_SDS_SUBMODE_CTRL0		RTSDS_REG(0x01cc)
+#define RTSDS_930X_SDS_SUBMODE_CTRL1		RTSDS_REG(0x02d8)
+
+#define RTSDS_931X_SDS_CNT			14
+#define RTSDS_931X_PAGE_CNT			192
+#define RTSDS_931X_SERDES_MODE_CTRL		RTSDS_REG(0x13cc)
+#define RTSDS_931X_PS_SERDES_OFF_MODE_CTRL	RTSDS_REG(0x13f4)
+#define RTSDS_931X_SDS_FORCE_SETUP		0x80
+
+#define RTSDS_93XX_SDS_READ			0x1
+#define RTSDS_93XX_SDS_WRITE			0x3
+#define RTSDS_93XX_SDS_BUSY			0x1
+
+#define RTSDS_COMBOMODE(mode, submode)		(0x10000 | (mode << 8) | submode)
+#define RTSDS_MODE(combomode)			((combomode >> 8) & 0xff)
+#define RTSDS_SUBMODE(combomode)		(combomode & 0xff)
+
+struct __packed rtsds_seq {
+	u8 action;
+	u8 mode;
+	u16 ports;
+	u16 page;
+	u16 reg;
+	u16 val;
+	u16 mask;
+};
+
+struct rtsds_sds {
+	struct phy *phy;
+	int mode;
+	int link;
+	int min_port;
+	int max_port;
+};
+
+struct rtsds_ctrl {
+	struct device *dev;
+	void __iomem *base;
+	struct mutex lock;
+	u32 sds_mask;
+	struct rtsds_conf *conf;
+	struct rtsds_sds sds[RTSDS_931X_SDS_CNT];
+	struct rtsds_seq *sequence[RTSDS_EVENT_CNT];
+};
+
+struct rtsds_macro {
+	struct rtsds_ctrl *ctrl;
+	u32 sid;
+};
+
+struct rtsds_conf {
+	u32 sds_cnt;
+	u32 page_cnt;
+	int (*read)(struct rtsds_ctrl *ctrl, u32 idx, u32 page, u32 reg);
+	int (*mask)(struct rtsds_ctrl *ctrl, u32 idx, u32 page, u32 reg, u32 val, u32 mask);
+	int (*reset)(struct rtsds_ctrl *ctrl, u32 idx);
+	int (*set_mode)(struct rtsds_ctrl *ctrl, u32 idx, int mode);
+	int (*get_mode)(struct rtsds_ctrl *ctrl, u32 idx);
+	int mode_map[PHY_INTERFACE_MODE_MAX];
+	struct rtsds_seq *sequence[RTSDS_EVENT_CNT];
+};
+
+/*
+ * This SerDes module should be written in quite a clean way so that direct calls are
+ * not needed. The following functions are provided just in case ...
+ */
+
+int rtsds_read(struct phy *phy, u32 page, u32 reg);
+int rtsds_write(struct phy *phy, u32 page, u32 reg, u32 val);
+int rtsds_mask(struct phy *phy, u32 page, u32 reg, u32 val, u32 mask);
+
+#endif /* _PHY_RTK_OTTO_SERDES_H */
--
2.46.2

[PATCH v2 3/3] phy: Integrate Realtek Otto SerDes driver into build system

[Markus Stockhausen]

Add the driver to the build system. The Otto platform currently has
only some drivers upstream and is missing a lot of platform bits.
Use only the bare minimum of dependencies.

Changes in v2:
- Change naming convention
- Add more help text

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 drivers/phy/realtek/Kconfig  | 10 ++++++++++
 drivers/phy/realtek/Makefile |  1 +
 2 files changed, 11 insertions(+)

diff --git a/drivers/phy/realtek/Kconfig b/drivers/phy/realtek/Kconfig
index 75ac7e7c31ae..021b4c4e700a 100644
--- a/drivers/phy/realtek/Kconfig
+++ b/drivers/phy/realtek/Kconfig
@@ -30,3 +30,13 @@ config PHY_RTK_RTD_USB3PHY
 	  of the parameters.

 endif # ARCH_REALTEK || COMPILE_TEST
+
+config PHY_RTK_OTTO_SERDES
+	tristate "SerDes driver for the Realtek Otto platform"
+	depends on OF
+	select GENERIC_PHY
+	help
+	  Enable this to support Realtek SerDes in the RTL83xx and
+	  RTL93xx network SoCs. These are based on MIPS32 architecture
+	  and the SerDes connect to one to octa transceivers to build
+	  up switches with up to 52 ports.
diff --git a/drivers/phy/realtek/Makefile b/drivers/phy/realtek/Makefile
index ed7b47ff8a26..34e607f33961 100644
--- a/drivers/phy/realtek/Makefile
+++ b/drivers/phy/realtek/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_PHY_RTK_RTD_USB2PHY)	+= phy-rtk-usb2.o
 obj-$(CONFIG_PHY_RTK_RTD_USB3PHY)	+= phy-rtk-usb3.o
+obj-$(CONFIG_PHY_RTK_OTTO_SERDES)	+= phy-rtk-otto-serdes.o
--
2.46.2

Re: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding

[Rob Herring (Arm)]

On Mon, 07 Oct 2024 12:36:21 -0400, Markus Stockhausen wrote:
> Add bindings for the SerDes of the Realtek Otto platform. These are
> network Switch SoCs with up to 52 ports divided into four different
> model lines.
> 
> Changes in v2:
> - new subject
> - removed patch command sequences
> - renamed parameter controlled-ports to realtek,controlled-ports
> 
> Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
> ---
>  .../bindings/phy/realtek,otto-serdes.yaml     | 95 +++++++++++++++++++
>  1 file changed, 95 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml: 'reguired' is not one of ['$id', '$schema', 'title', 'description', 'examples', 'required', 'allOf', 'anyOf', 'oneOf', 'definitions', '$defs', 'additionalProperties', 'dependencies', 'dependentRequired', 'dependentSchemas', 'patternProperties', 'properties', 'not', 'if', 'then', 'else', 'unevaluatedProperties', 'deprecated', 'maintainers', 'select', '$ref']
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
Documentation/devicetree/bindings/phy/realtek,otto-serdes.example.dts:41.33-46.11: ERROR (duplicate_label): /example-1/serdes@1b00a000: Duplicate label 'serdes' on /example-1/serdes@1b00a000 and /example-0/serdes@1b00e780
Documentation/devicetree/bindings/phy/realtek,otto-serdes.example.dts:64.33-69.11: ERROR (duplicate_label): /example-2/serdes@1b0003b0: Duplicate label 'serdes' on /example-2/serdes@1b0003b0 and /example-0/serdes@1b00e780
Documentation/devicetree/bindings/phy/realtek,otto-serdes.example.dts:87.33-92.11: ERROR (duplicate_label): /example-3/serdes@1b005638: Duplicate label 'serdes' on /example-3/serdes@1b005638 and /example-0/serdes@1b00e780
ERROR: Input tree has errors, aborting (use -f to force output)
make[2]: *** [scripts/Makefile.dtbs:129: Documentation/devicetree/bindings/phy/realtek,otto-serdes.example.dtb] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1442: dt_binding_check] Error 2
make: *** [Makefile:224: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241007163623.3274510-2-markus.stockhausen@gmx.de

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding

[Krzysztof Kozlowski]

On 07/10/2024 18:36, Markus Stockhausen wrote:
> Add bindings for the SerDes of the Realtek Otto platform. These are
> network Switch SoCs with up to 52 ports divided into four different
> model lines.
> 
> Changes in v2:
> - new subject
> - removed patch command sequences
> - renamed parameter controlled-ports to realtek,controlled-ports

Changelog goes under ---.


> 
> Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
> ---
>  .../bindings/phy/realtek,otto-serdes.yaml     | 95 +++++++++++++++++++
>  1 file changed, 95 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml
> new file mode 100644
> index 000000000000..a72ac206b35f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml

Nothing improved.

> @@ -0,0 +1,95 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/realtek,otto-serdes.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Realtek Otto SerDes controller
> +
> +maintainers:
> +  - Markus Stockhausen <markus.stockhausen@gmx.de>
> +
> +description:
> +  The MIPS based Realtek Switch SoCs of the Realtek RTL838x, RTL839x, RTL930x and RTL931x series
> +  have multiple SerDes built in. They are linked to single, quad or octa PHYs like the RTL8218B,
> +  RTL8218D or RTL8214FC and are one of the integral part of the up-to-52-port switch architecture.
> +
> +  Although these SerDes controllers have common basics they behave differently on the SoC families
> +  and rely on heavy register magic. To keep the driver clean it can load patch sequences from
> +  devictree and execute them during the controller actions like phy_init(), ...
> +
> +  The driver exposes the SerDes registers different from the hardware but instead gives a
> +  consistent view and programming interface. So the RTL838x series has 6 ports and 4 pages, the
> +  RTL839x has 14 ports and 12 pages, the RTL930x has 12 ports and 64 pages and the RTL931x has
> +  14 ports and 192 pages.

Totally messed wrapping. Please wrap your code as Linux coding style.

> +
> +properties:
> +  $nodename:
> +    pattern: "^serdes@[0-9a-f]+$"
> +
> +  compatible:
> +    items:
> +      - enum:
> +          - realtek,rtl8380-serdes
> +          - realtek,rtl8390-serdes
> +          - realtek,rtl9300-serdes
> +          - realtek,rtl9310-serdes
> +
> +  reg:
> +    items:
> +      description:
> +        The primary SerDes paged register memory location. Other SerDes control and management
> +        registers are distributed all over the I/O memory space and are identified by the driver.

What happened here? I asked only about |. Why are you adding unrelated
changes?

Anyway, still not tested and still does not look any other binding.

> +
> +  "#phy-cells":
> +    const: 4
> +    description:
> +      The first number defines the SerDes to use. The second number a linked SerDes. E.g. if a octa
> +      1G PHY is attached to two QSGMII SerDes. The third number is the first switch port this
> +      SerDes is working for, the fourth number is the last switch port the SerDes is working for.
> +
> +  realtek,controlled-ports:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      A bit mask defining the ports that are actively controlled by the driver. In case a bit is

Driver? Bindings are not about drivers. Drop.

I don't think you implemented my feedback.

> +      not set the driver will only process read operations on the SerDes. This is just in case the
> +      SerDes has been setup by U-Boot correctly and the driver malfunctions. If not set the driver
> +      will control all SerDes.



> +
> +reguired:
> +  - compatible
> +  - reg
> +  - "#phy-cells"
> +
> +additionalProperties:
> +  false

Please open any existing binding and do it like there. Or start from
scratch from example-schema.

> +
> +examples:
> +  - |
> +    serdes: serdes@1b00e780 {
> +      compatible = "realtek,rtl8380-serdes", "realtek,otto-serdes";
> +      reg = <0x1b00e780 0x1200>;
> +      controlled-ports = <0x003f>;
> +      #phy-cells = <4>;
> +    };

One example is enough.

... and still not tested. Sending untested code is waste of our time.

Best regards,
Krzysztof

Re: [PATCH v2 3/3] phy: Integrate Realtek Otto SerDes driver into build system

[Krzysztof Kozlowski]

On 07/10/2024 18:36, Markus Stockhausen wrote:
> Add the driver to the build system. The Otto platform currently has
> only some drivers upstream and is missing a lot of platform bits.
> Use only the bare minimum of dependencies.
> 
> Changes in v2:
> - Change naming convention
> - Add more help text
> 
> Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
> ---
>  drivers/phy/realtek/Kconfig  | 10 ++++++++++
>  drivers/phy/realtek/Makefile |  1 +
>  2 files changed, 11 insertions(+)

Still no point in splitting this.

<form letter>
This is a friendly reminder during the review process.

It seems my or other reviewer's previous comments were not fully
addressed. Maybe the feedback got lost between the quotes, maybe you
just forgot to apply it. Please go back to the previous discussion and
either implement all requested changes or keep discussing them.

Thank you.
</form letter>

Best regards,
Krzysztof

Re: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding

[Rob Herring]

On Mon, Oct 07, 2024 at 12:36:21PM -0400, Markus Stockhausen wrote:
> Add bindings for the SerDes of the Realtek Otto platform. These are
> network Switch SoCs with up to 52 ports divided into four different
> model lines.
> 
> Changes in v2:
> - new subject
> - removed patch command sequences
> - renamed parameter controlled-ports to realtek,controlled-ports
> 
> Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
> ---
>  .../bindings/phy/realtek,otto-serdes.yaml     | 95 +++++++++++++++++++
>  1 file changed, 95 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml
> new file mode 100644
> index 000000000000..a72ac206b35f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml
> @@ -0,0 +1,95 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/realtek,otto-serdes.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Realtek Otto SerDes controller
> +
> +maintainers:
> +  - Markus Stockhausen <markus.stockhausen@gmx.de>
> +
> +description:

You need '>' on the end if you want to preserve paragraphs.

> +  The MIPS based Realtek Switch SoCs of the Realtek RTL838x, RTL839x, RTL930x and RTL931x series
> +  have multiple SerDes built in. They are linked to single, quad or octa PHYs like the RTL8218B,
> +  RTL8218D or RTL8214FC and are one of the integral part of the up-to-52-port switch architecture.
> +
> +  Although these SerDes controllers have common basics they behave differently on the SoC families
> +  and rely on heavy register magic. To keep the driver clean it can load patch sequences from
> +  devictree and execute them during the controller actions like phy_init(), ...
> +
> +  The driver exposes the SerDes registers different from the hardware but instead gives a
> +  consistent view and programming interface. So the RTL838x series has 6 ports and 4 pages, the
> +  RTL839x has 14 ports and 12 pages, the RTL930x has 12 ports and 64 pages and the RTL931x has
> +  14 ports and 192 pages.
> +
> +properties:
> +  $nodename:
> +    pattern: "^serdes@[0-9a-f]+$"

The node name for phy providers is 'phy'.

> +
> +  compatible:
> +    items:
> +      - enum:
> +          - realtek,rtl8380-serdes
> +          - realtek,rtl8390-serdes
> +          - realtek,rtl9300-serdes
> +          - realtek,rtl9310-serdes
> +
> +  reg:
> +    items:
> +      description:
> +        The primary SerDes paged register memory location. Other SerDes control and management
> +        registers are distributed all over the I/O memory space and are identified by the driver.
> +
> +  "#phy-cells":
> +    const: 4
> +    description:
> +      The first number defines the SerDes to use. The second number a linked SerDes. E.g. if a octa
> +      1G PHY is attached to two QSGMII SerDes. The third number is the first switch port this
> +      SerDes is working for, the fourth number is the last switch port the SerDes is working for.
> +
> +  realtek,controlled-ports:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      A bit mask defining the ports that are actively controlled by the driver. In case a bit is
> +      not set the driver will only process read operations on the SerDes. This is just in case the
> +      SerDes has been setup by U-Boot correctly and the driver malfunctions. If not set the driver
> +      will control all SerDes.
> +
> +reguired:
> +  - compatible
> +  - reg
> +  - "#phy-cells"
> +
> +additionalProperties:
> +  false

One line like *everywhere* else.

> +
> +examples:
> +  - |
> +    serdes: serdes@1b00e780 {

Drop unused labels.

> +      compatible = "realtek,rtl8380-serdes", "realtek,otto-serdes";
> +      reg = <0x1b00e780 0x1200>;
> +      controlled-ports = <0x003f>;
> +      #phy-cells = <4>;
> +    };
> +  - |
> +    serdes: serdes@1b00a000 {
> +      compatible = "realtek,rtl8390-serdes", "realtek,otto-serdes";
> +      reg = <0x1b00a000 0x1c00>;
> +      controlled-ports = <0x3fff>;
> +      #phy-cells = <4>;
> +    };
> +  - |
> +    serdes: serdes@1b0003b0 {
> +      compatible = "realtek,rtl9300-serdes", "realtek,otto-serdes";
> +      reg = <0x1b0003b0 0x8>;
> +      controlled-ports = <0x0fff>;
> +      #phy-cells = <4>;
> +    };
> +  - |
> +    serdes: serdes@1b005638 {
> +      compatible = "realtek,rtl9310-serdes", "realtek,otto-serdes";
> +      reg = <0x1b005638 0x8>;
> +      controlled-ports = <0x3fff>;
> +      #phy-cells = <4>;
> +    };

You don't need 4 examples that are essentially the same.

> --
> 2.46.2
> 

Re: [PATCH v2 2/3] phy: Realtek Otto SerDes driver

[Krzysztof Kozlowski]

On 07/10/2024 18:36, Markus Stockhausen wrote:
> The Realtek Otto platform is a series of 4 different MIPS32 based network
> switch SoCs. They consist of:
> 
> - RTL838x: 500MHz single core, up to 28 ports 20GBps switching capacity
> - RTL839x: 700MHz single core, up to 52 ports 100GBps switching capacity
> - RTL930x: 700MHz single core, up to 28 ports 120GBps switching capacity
> - RTL931x: 1.4GHz dual core, up to 52 ports 170GBps switching capacity
> 

...

> +
> +static int rtsds_phy_power_on(struct phy *phy)
> +{
> +	struct rtsds_macro *macro = phy_get_drvdata(phy);
> +	struct rtsds_ctrl *ctrl = macro->ctrl;
> +	u32 sid = macro->sid;
> +	int ret;
> +
> +	if (rtsds_readonly(ctrl, sid))
> +		return 0;
> +
> +	mutex_lock(&ctrl->lock);
> +//	ret = rtsds_run_event(ctrl, sid, RTSDS_EVENT_POWER_ON);


Dead code? What is the point of this?

> +	mutex_unlock(&ctrl->lock);
> +
> +	if (ret)
> +		dev_err(ctrl->dev, "power on failed for SerDes %d\n", sid);
> +
> +	return ret;
> +}
> +


...

> +
> +static ssize_t rtsds_dbg_mode_write(struct file *file, const char __user *userbuf,
> +				size_t count, loff_t *ppos)
> +{
> +	struct seq_file *seqf = file->private_data;
> +	struct rtsds_macro *macro = dev_get_drvdata(seqf->private);
> +	struct rtsds_ctrl *ctrl = macro->ctrl;
> +	int ret, hwmode, phymode, sid = macro->sid;
> +
> +	ret = kstrtou32_from_user(userbuf, count, 16, &hwmode);
> +	if (ret)
> +		return ret;
> +	/*
> +	 * Allow to set arbitrary modes into the SerDes to improve error analysis. Accept that
> +	 * this might confuse the internal state tracking.
> +	 */
> +	phymode = rtsds_hwmode_to_phymode(ctrl, hwmode);
> +	rtsds_phy_set_mode_int(ctrl, sid, phymode, hwmode);

Interfasce which confuses internal state is a bad interface. Drop.

> +
> +	return count;
> +}
> +DEFINE_SHOW_STORE_ATTRIBUTE(rtsds_dbg_mode);
> +
> +static ssize_t rtsds_dbg_reset_show(struct seq_file *seqf, void *unused)
> +{
> +	return 0;
> +}
> +
> +static ssize_t rtsds_dbg_reset_write(struct file *file, const char __user *userbuf,
> +				size_t count, loff_t *ppos)
> +{
> +	struct seq_file *seqf = file->private_data;
> +	struct rtsds_macro *macro = dev_get_drvdata(seqf->private);
> +	struct rtsds_ctrl *ctrl = macro->ctrl;
> +	int ret, reset, sid = macro->sid;
> +
> +	ret = kstrtou32_from_user(userbuf, count, 10, &reset);
> +	if (ret || reset != 1)
> +		return ret;
> +
> +	rtsds_phy_reset_int(ctrl, sid);
> +
> +	return count;
> +}
> +DEFINE_SHOW_STORE_ATTRIBUTE(rtsds_dbg_reset);

That's not a debugfs interface. Drop reset.

> +
> +static int rtsds_dbg_registers_show(struct seq_file *seqf, void *unused)
> +{
> +	struct rtsds_macro *macro = dev_get_drvdata(seqf->private);
> +	struct rtsds_ctrl *ctrl = macro->ctrl;
> +	u32 page = 0, reg, sid = macro->sid;
> +
> +	seq_printf(seqf, "%*s", 12, "");
> +	for (int i = 0; i < 32; i++)
> +		seq_printf(seqf, "   %02X", i);
> +
> +	while (page < ctrl->conf->page_cnt) {
> +		if (page < RTSDS_PAGE_NAMES && rtsds_page_name[page])
> +			seq_printf(seqf, "\n%*s: ", -11, rtsds_page_name[page]);
> +		else if (page == 64 || page == 128)
> +			seq_printf(seqf, "\n\nXGMII_%d    : ", page >> 8);
> +		else
> +			seq_printf(seqf, "\nPAGE_%03d   : ", page);
> +		for (reg = 0; reg < 32; reg++)
> +			seq_printf(seqf, "%04X ", ctrl->conf->read(ctrl, sid, page, reg));
> +
> +		page++;
> +	}
> +	seq_puts(seqf, "\n");
> +
> +	return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(rtsds_dbg_registers);
> +
> +static int rtsds_dbg_polarity_show(struct seq_file *seqf, void *unused)
> +{
> +	struct rtsds_macro *macro = dev_get_drvdata(seqf->private);
> +	struct rtsds_ctrl *ctrl = macro->ctrl;
> +	u32 reg, sid = macro->sid;
> +
> +	reg = ctrl->conf->read(ctrl, sid, RTSDS_PAGE_SDS, 0);
> +
> +	seq_puts(seqf, "tx polarity: ");
> +	seq_puts(seqf, reg & RTSDS_BITS_INV_HSO ? "inverse" : "normal");
> +	seq_puts(seqf, "\nrx polarity: ");
> +	seq_puts(seqf, reg & RTSDS_BITS_INV_HSI ? "inverse" : "normal");
> +	seq_puts(seqf, "\n");
> +
> +	return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(rtsds_dbg_polarity);
> +
> +static void rtsds_dbg_init(struct rtsds_ctrl *ctrl, u32 sid)
> +{
> +	debugfs_create_file("mode", 0600, ctrl->sds[sid].phy->debugfs,
> +			    &ctrl->sds[sid].phy->dev, &rtsds_dbg_mode_fops);
> +
> +	debugfs_create_file("reset", 0200, ctrl->sds[sid].phy->debugfs,
> +			    &ctrl->sds[sid].phy->dev, &rtsds_dbg_reset_fops);
> +
> +	debugfs_create_file("polarity", 0400, ctrl->sds[sid].phy->debugfs,
> +			    &ctrl->sds[sid].phy->dev, &rtsds_dbg_polarity_fops);
> +
> +	debugfs_create_file("registers", 0400, ctrl->sds[sid].phy->debugfs,
> +			    &ctrl->sds[sid].phy->dev, &rtsds_dbg_registers_fops);
> +}
> +#endif /* CONFIG_DEBUG_FS */
> +


> +
> +static int rtsds_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct device *dev = &pdev->dev;
> +	struct phy_provider *provider;
> +	struct rtsds_ctrl *ctrl;
> +	int ret;
> +
> +	if (!np)
> +		return -EINVAL;

How is this possible? Drop.

> +
> +	ctrl = devm_kzalloc(dev, sizeof(*ctrl), GFP_KERNEL);
> +	if (!ctrl)
> +		return -ENOMEM;
> +
> +	ctrl->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(ctrl->base)) {
> +		dev_err(dev, "failed to map SerDes memory\n");
> +		return PTR_ERR(ctrl->base);
> +	}
> +
> +	ctrl->dev = dev;
> +	ctrl->conf = (struct rtsds_conf *)of_device_get_match_data(dev);
> +
> +	ret = of_property_read_u32(np, "realtek,controlled-ports", &ctrl->sds_mask);
> +	if (ret)
> +		ctrl->sds_mask = GENMASK(ctrl->conf->sds_cnt, 0);
> +
> +	for (u32 sid = 0; sid < ctrl->conf->sds_cnt; sid++) {
> +		ret = rtsds_phy_create(ctrl, sid);
> +		if (ret) {
> +			dev_err(dev, "failed to create PHY for SerDes %d\n", sid);
> +			return ret;
> +		}
> +	}
> +
> +	mutex_init(&ctrl->lock);
> +	dev_set_drvdata(dev, ctrl);
> +	provider = devm_of_phy_provider_register(dev, rtsds_simple_xlate);
> +	rtsds_setup(ctrl);
> +	dev_info(dev, "initialized (%d SerDes, %d pages, 32 registers, mask 0x%04x)",
> +		 ctrl->conf->sds_cnt, ctrl->conf->page_cnt, ctrl->sds_mask);
> +
> +	return PTR_ERR_OR_ZERO(provider);
> +}
> +

...

> +static const struct of_device_id rtsds_compatible_ids[] = {
> +	{ .compatible = "realtek,rtl8380-serdes",
> +	  .data = &rtsds_838x_conf,
> +	},
> +	{ .compatible = "realtek,rtl8390-serdes",
> +	  .data = &rtsds_839x_conf,
> +	},
> +	{ .compatible = "realtek,rtl9300-serdes",
> +	  .data = &rtsds_930x_conf,
> +	},
> +	{ .compatible = "realtek,rtl9310-serdes",
> +	  .data = &rtsds_931x_conf,
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, rtsds_compatible_ids);
> +
> +static struct platform_driver rtsds_platform_driver = {
> +	.probe		= rtsds_probe,
> +	.driver		= {
> +		.name	= "realtek,otto-serdes",
> +		.of_match_table = of_match_ptr(rtsds_compatible_ids),

Drop of_match_ptr, you have warning because of it.

> +	},
> +};
> +
> +module_platform_driver(rtsds_platform_driver);
> +
> +MODULE_AUTHOR("Markus Stockhausen <markus.stockhausen@gmx.de>");
> +MODULE_DESCRIPTION("SerDes driver for Realtek RTL83xx, RTL93xx switch SoCs");
> +MODULE_LICENSE("GPL");

...

> +
> +struct rtsds_macro {
> +	struct rtsds_ctrl *ctrl;
> +	u32 sid;
> +};
> +
> +struct rtsds_conf {
> +	u32 sds_cnt;
> +	u32 page_cnt;
> +	int (*read)(struct rtsds_ctrl *ctrl, u32 idx, u32 page, u32 reg);
> +	int (*mask)(struct rtsds_ctrl *ctrl, u32 idx, u32 page, u32 reg, u32 val, u32 mask);
> +	int (*reset)(struct rtsds_ctrl *ctrl, u32 idx);
> +	int (*set_mode)(struct rtsds_ctrl *ctrl, u32 idx, int mode);
> +	int (*get_mode)(struct rtsds_ctrl *ctrl, u32 idx);
> +	int mode_map[PHY_INTERFACE_MODE_MAX];
> +	struct rtsds_seq *sequence[RTSDS_EVENT_CNT];
> +};
> +
> +/*
> + * This SerDes module should be written in quite a clean way so that direct calls are
> + * not needed. The following functions are provided just in case ...
> + */

Drop code "just in case".



Best regards,
Krzysztof

AW: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding

[markus.stockhausen]

> -----Ursprüngliche Nachricht-----
> Von: Krzysztof Kozlowski <krzk@kernel.org> 
> Gesendet: Montag, 7. Oktober 2024 21:26
> An: Markus Stockhausen <markus.stockhausen@gmx.de>; linux-phy@lists.infradead.org; chris.packham@alliedtelesis.co.nz; devicetree@vger.kernel.org
> Betreff: Re: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding
>
> ... and still not tested. Sending untested code is waste of our time.

Hi Krzysztof,

appreciate your feedback and I do not want to waste your time. My fixes where a mix
of your feedback and some half-baked "make dt_binding_check" feedbacks (because
packages where missing). My fault and sorry fort he noise.

To get next version in better shape two questions regarding your feedback:

1. "Messed wrapping": According to checkpatch 100 chars/line are accepted. 
So I designed the comments in the driver. Does devicetree differ from that?

2 "Bindings vs drivers". The idea about controlled ports came from other bindings.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/interrupt-controller/st,stih407-irq-syscfg.yaml?h=v6.12-rc2
E.g. st,invert-ext. Something like this will be needed in the future because the
SerDes allow to swap polarity which must be changed depending on the switch
design. How to do this?

Best regards.

Markus

Re: AW: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding

[Krzysztof Kozlowski]

On 08/10/2024 07:38, markus.stockhausen@gmx.de wrote:
>> -----Ursprüngliche Nachricht-----
>> Von: Krzysztof Kozlowski <krzk@kernel.org> 
>> Gesendet: Montag, 7. Oktober 2024 21:26
>> An: Markus Stockhausen <markus.stockhausen@gmx.de>; linux-phy@lists.infradead.org; chris.packham@alliedtelesis.co.nz; devicetree@vger.kernel.org
>> Betreff: Re: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding
>>
>> ... and still not tested. Sending untested code is waste of our time.
> 
> Hi Krzysztof,
> 
> appreciate your feedback and I do not want to waste your time. My fixes where a mix
> of your feedback and some half-baked "make dt_binding_check" feedbacks (because
> packages where missing). My fault and sorry fort he noise.
> 
> To get next version in better shape two questions regarding your feedback:
> 
> 1. "Messed wrapping": According to checkpatch 100 chars/line are accepted. 
> So I designed the comments in the driver. Does devicetree differ from that?

checkpatch is not a coding style. I asked to follow coding style, please
read entire document in Documentation/process.

> 
> 2 "Bindings vs drivers". The idea about controlled ports came from other bindings.

Entire property description speaks about driver, not bindings.

> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/interrupt-controller/st,stih407-irq-syscfg.yaml?h=v6.12-rc2

stih is rather poor example to use. The property was added in 2015 (!)
without review (!!!).


> E.g. st,invert-ext. Something like this will be needed in the future because the
> SerDes allow to swap polarity which must be changed depending on the switch
> design. How to do this?

I do not understand the hardware aspect discussed in the property
description... probably because there is no hardware description at all,
but instead you speak about driver.

I do not understand how polarity has anything to do with U-Boot
configuring serdes.

Best regards,
Krzysztof

Re: [PATCH v2 3/3] phy: Integrate Realtek Otto SerDes driver into build system

[kernel test robot]

Hi Markus,

kernel test robot noticed the following build warnings:

[auto build test WARNING on robh/for-next]
[also build test WARNING on krzk-dt/for-next linus/master v6.12-rc2 next-20241004]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Markus-Stockhausen/dt-bindings-phy-add-realtek-otto-serdes-PHY-binding/20241008-003929
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20241007163623.3274510-4-markus.stockhausen%40gmx.de
patch subject: [PATCH v2 3/3] phy: Integrate Realtek Otto SerDes driver into build system
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20241008/202410081436.fFz8vjTK-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project fef3566a25ff0e34fb87339ba5e13eca17cec00f)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241008/202410081436.fFz8vjTK-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410081436.fFz8vjTK-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/phy/realtek/phy-rtk-otto-serdes.c:11:
   In file included from include/linux/phy.h:16:
   In file included from include/linux/ethtool.h:18:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:10:
   In file included from include/linux/mm.h:2213:
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   In file included from drivers/phy/realtek/phy-rtk-otto-serdes.c:11:
   In file included from include/linux/phy.h:16:
   In file included from include/linux/ethtool.h:18:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from drivers/phy/realtek/phy-rtk-otto-serdes.c:11:
   In file included from include/linux/phy.h:16:
   In file included from include/linux/ethtool.h:18:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from drivers/phy/realtek/phy-rtk-otto-serdes.c:11:
   In file included from include/linux/phy.h:16:
   In file included from include/linux/ethtool.h:18:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
>> drivers/phy/realtek/phy-rtk-otto-serdes.c:490:15: warning: overlapping comparisons always evaluate to true [-Wtautological-overlap-compare]
     490 |         if (sid >= 2 || sid <= 9)
         |             ~~~~~~~~~^~~~~~~~~~~
   drivers/phy/realtek/phy-rtk-otto-serdes.c:509:15: warning: overlapping comparisons always evaluate to true [-Wtautological-overlap-compare]
     509 |         if (sid >= 2 || sid <= 9)
         |             ~~~~~~~~~^~~~~~~~~~~
>> drivers/phy/realtek/phy-rtk-otto-serdes.c:686:6: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
     686 |         if (ret)
         |             ^~~
   drivers/phy/realtek/phy-rtk-otto-serdes.c:677:9: note: initialize the variable 'ret' to silence this warning
     677 |         int ret;
         |                ^
         |                 = 0
   drivers/phy/realtek/phy-rtk-otto-serdes.c:706:6: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
     706 |         if (ret)
         |             ^~~
   drivers/phy/realtek/phy-rtk-otto-serdes.c:697:9: note: initialize the variable 'ret' to silence this warning
     697 |         int ret;
         |                ^
         |                 = 0
   drivers/phy/realtek/phy-rtk-otto-serdes.c:723:7: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
     723 |         if (!ret)
         |              ^~~
   drivers/phy/realtek/phy-rtk-otto-serdes.c:717:9: note: initialize the variable 'ret' to silence this warning
     717 |         int ret;
         |                ^
         |                 = 0
   12 warnings generated.

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for MODVERSIONS
   Depends on [n]: MODULES [=y] && !COMPILE_TEST [=y]
   Selected by [y]:
   - RANDSTRUCT_FULL [=y] && (CC_HAS_RANDSTRUCT [=y] || GCC_PLUGINS [=n]) && MODULES [=y]
   WARNING: unmet direct dependencies detected for GET_FREE_REGION
   Depends on [n]: SPARSEMEM [=n]
   Selected by [m]:
   - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]


vim +490 drivers/phy/realtek/phy-rtk-otto-serdes.c

40f1aea80b53b8 Markus Stockhausen 2024-10-07  477  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  478  static int rtsds_930x_set_mode(struct rtsds_ctrl *ctrl, u32 sid, int combomode)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  479  {
40f1aea80b53b8 Markus Stockhausen 2024-10-07  480  	int modeshift, subshift;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  481  	int mode = RTSDS_MODE(combomode);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  482  	int submode = RTSDS_SUBMODE(combomode);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  483  	void __iomem __force *modereg;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  484  	void __iomem __force *subreg;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  485  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  486  	if (rtsds_invalid_sds(ctrl, sid))
40f1aea80b53b8 Markus Stockhausen 2024-10-07  487  		return -EINVAL;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  488  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  489  	rtsds_930x_mode_offset(sid, &modereg, &modeshift, &subreg, &subshift);
40f1aea80b53b8 Markus Stockhausen 2024-10-07 @490  	if (sid >= 2 || sid <= 9)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  491  		iomask32(0x1f << subshift, (submode & 0x1f) << subshift, subreg);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  492  	else if (submode != 0)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  493  		return -EINVAL;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  494  	iomask32(0x1f << modeshift, (mode & 0x1f) << modeshift, modereg);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  495  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  496  	return 0;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  497  }
40f1aea80b53b8 Markus Stockhausen 2024-10-07  498  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  499  static int rtsds_930x_get_mode(struct rtsds_ctrl *ctrl, u32 sid)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  500  {
40f1aea80b53b8 Markus Stockhausen 2024-10-07  501  	int modeshift, subshift, mode, submode = 0;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  502  	void __iomem __force *modereg;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  503  	void __iomem __force *subreg;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  504  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  505  	if (rtsds_invalid_sds(ctrl, sid))
40f1aea80b53b8 Markus Stockhausen 2024-10-07  506  		return -EINVAL;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  507  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  508  	rtsds_930x_mode_offset(sid, &modereg, &modeshift, &subreg, &subshift);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  509  	if (sid >= 2 || sid <= 9)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  510  		submode = (ioread32(subreg) >> subshift) & 0x1f;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  511  	mode = ioread32(modereg) >> modeshift & 0x1f;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  512  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  513  	return RTSDS_COMBOMODE(mode, submode);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  514  }
40f1aea80b53b8 Markus Stockhausen 2024-10-07  515  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  516  static int rtsds_930x_reset(struct rtsds_ctrl *ctrl, u32 sid)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  517  {
40f1aea80b53b8 Markus Stockhausen 2024-10-07  518  	int modecur, modeoff = ctrl->conf->mode_map[PHY_INTERFACE_MODE_NA];
40f1aea80b53b8 Markus Stockhausen 2024-10-07  519  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  520  	if (rtsds_invalid_sds(ctrl, sid))
40f1aea80b53b8 Markus Stockhausen 2024-10-07  521  		return -EINVAL;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  522  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  523  	modecur = rtsds_930x_get_mode(ctrl, sid);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  524  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  525  	/* It is enough to power off SerDes and set to old mode again */
40f1aea80b53b8 Markus Stockhausen 2024-10-07  526  	if (modecur != modeoff) {
40f1aea80b53b8 Markus Stockhausen 2024-10-07  527  		rtsds_930x_set_mode(ctrl, sid, modeoff);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  528  		rtsds_930x_set_mode(ctrl, sid, modecur);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  529  	}
40f1aea80b53b8 Markus Stockhausen 2024-10-07  530  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  531  	return 0;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  532  }
40f1aea80b53b8 Markus Stockhausen 2024-10-07  533  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  534  /*
40f1aea80b53b8 Markus Stockhausen 2024-10-07  535   * The RTL931x family has 14 "frontend" SerDes that are cascaded. All operations (e.g. reset) work
40f1aea80b53b8 Markus Stockhausen 2024-10-07  536   * on this frontend view while their registers are distributed over a total of 32 background
40f1aea80b53b8 Markus Stockhausen 2024-10-07  537   * SerDes. Two types of SerDes have been identified:
40f1aea80b53b8 Markus Stockhausen 2024-10-07  538   *
40f1aea80b53b8 Markus Stockhausen 2024-10-07  539   * A "even" SerDes with numbers 0, 1, 2, 4, 6, 8, 10, 12 works on two background SerDes. 64 analog
40f1aea80b53b8 Markus Stockhausen 2024-10-07  540   * and 64 XGMII data pages are coming from a first background SerDes while another 64 XGMII pages
40f1aea80b53b8 Markus Stockhausen 2024-10-07  541   * are served from a second SerDes.
40f1aea80b53b8 Markus Stockhausen 2024-10-07  542   *
40f1aea80b53b8 Markus Stockhausen 2024-10-07  543   * The "odd" SerDes with numbers 3, 5, 7, 9, 11 & 13 SerDes consist of a total of 3 background
40f1aea80b53b8 Markus Stockhausen 2024-10-07  544   * SerDes (one analog and two XGMII) each with an own page/register set.
40f1aea80b53b8 Markus Stockhausen 2024-10-07  545   *
40f1aea80b53b8 Markus Stockhausen 2024-10-07  546   * To align this and improve readability the driver will simulate a total of 576 pages and mix
40f1aea80b53b8 Markus Stockhausen 2024-10-07  547   * them as follows.
40f1aea80b53b8 Markus Stockhausen 2024-10-07  548   *
40f1aea80b53b8 Markus Stockhausen 2024-10-07  549   * frontend page		"even" frontend SerDes	"odd" frontend SerDes
40f1aea80b53b8 Markus Stockhausen 2024-10-07  550   * page 0-63 (analog):		back SDS page 0-63	back SDS page 0-63
40f1aea80b53b8 Markus Stockhausen 2024-10-07  551   * page 64-127 (XGMII1):	back SDS page 0-63	back SDS+1 page 0-63
40f1aea80b53b8 Markus Stockhausen 2024-10-07  552   * page 128-191 (XGMII2):	back SDS+1 page 0-63	back SDS+2 page 0-63
40f1aea80b53b8 Markus Stockhausen 2024-10-07  553   */
40f1aea80b53b8 Markus Stockhausen 2024-10-07  554  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  555  static int rtsds_931x_reg_offset(u32 sid, u32 page)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  556  {
40f1aea80b53b8 Markus Stockhausen 2024-10-07  557  	int map[] = {0, 1, 2, 3, 6, 7, 10, 11, 14, 15, 18, 19, 22, 23};
40f1aea80b53b8 Markus Stockhausen 2024-10-07  558  	int offs = map[sid];
40f1aea80b53b8 Markus Stockhausen 2024-10-07  559  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  560  	if ((sid & 1) && (sid != 1))
40f1aea80b53b8 Markus Stockhausen 2024-10-07  561  		offs += (page >> 6); /* distribute "odd" to 3 background SerDes */
40f1aea80b53b8 Markus Stockhausen 2024-10-07  562  	else if (page >= 128)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  563  		offs += 1; /* "distribute "even" to 2 background SerDes */
40f1aea80b53b8 Markus Stockhausen 2024-10-07  564  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  565  	return offs;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  566  }
40f1aea80b53b8 Markus Stockhausen 2024-10-07  567  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  568  static int rtsds_931x_read(struct rtsds_ctrl *ctrl, u32 sid, u32 page, u32 reg)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  569  {
40f1aea80b53b8 Markus Stockhausen 2024-10-07  570  	int offs;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  571  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  572  	if (rtsds_invalid_reg(ctrl, sid, page, reg))
40f1aea80b53b8 Markus Stockhausen 2024-10-07  573  		return -EINVAL;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  574  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  575  	offs = rtsds_931x_reg_offset(sid, page);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  576  	if (offs < 0)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  577  		return 0;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  578  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  579  	return rtsds_93xx_read(ctrl, offs, page, reg);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  580  }
40f1aea80b53b8 Markus Stockhausen 2024-10-07  581  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  582  static int rtsds_931x_mask(struct rtsds_ctrl *ctrl, u32 sid, u32 page, u32 reg, u32 val, u32 mask)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  583  {
40f1aea80b53b8 Markus Stockhausen 2024-10-07  584  	int offs;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  585  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  586  	if (rtsds_invalid_reg(ctrl, sid, page, reg))
40f1aea80b53b8 Markus Stockhausen 2024-10-07  587  		return -EINVAL;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  588  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  589  	offs = rtsds_931x_reg_offset(sid, page);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  590  	if (offs < 0)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  591  		return 0;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  592  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  593  	return rtsds_93xx_mask(ctrl, offs, page, reg, val, mask);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  594  }
40f1aea80b53b8 Markus Stockhausen 2024-10-07  595  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  596  static int rtsds_931x_set_mode(struct rtsds_ctrl *ctrl, u32 sid, int combomode)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  597  {
40f1aea80b53b8 Markus Stockhausen 2024-10-07  598  	int shift = (sid & 3) << 3, offs = sid & ~3;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  599  	int mode = RTSDS_MODE(combomode);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  600  	int submode = RTSDS_SUBMODE(combomode);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  601  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  602  	if (rtsds_invalid_sds(ctrl, sid))
40f1aea80b53b8 Markus Stockhausen 2024-10-07  603  		return -EINVAL;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  604  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  605  	rtsds_931x_mask(ctrl, sid, 0x1f, 0x09, (submode & 0x3f << 6), 0x0fc0);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  606  	iomask32(0xff << shift, ((mode | RTSDS_931X_SDS_FORCE_SETUP) & 0xff) << shift,
40f1aea80b53b8 Markus Stockhausen 2024-10-07  607  		 RTSDS_931X_SERDES_MODE_CTRL + offs);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  608  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  609  	return 0;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  610  }
40f1aea80b53b8 Markus Stockhausen 2024-10-07  611  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  612  static int rtsds_931x_get_mode(struct rtsds_ctrl *ctrl, u32 sid)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  613  {
40f1aea80b53b8 Markus Stockhausen 2024-10-07  614  	int mode, submode, shift = (sid & 3) << 3, offs = sid & ~3;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  615  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  616  	if (rtsds_invalid_sds(ctrl, sid))
40f1aea80b53b8 Markus Stockhausen 2024-10-07  617  		return -EINVAL;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  618  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  619  	submode = (rtsds_931x_read(ctrl, sid, 0x1f, 0x09) >> 6) & 0x3f;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  620  	mode = (ioread32(RTSDS_931X_SERDES_MODE_CTRL + offs) >> shift) & 0x1f;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  621  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  622  	return RTSDS_COMBOMODE(mode, submode);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  623  }
40f1aea80b53b8 Markus Stockhausen 2024-10-07  624  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  625  static int rtsds_931x_reset(struct rtsds_ctrl *ctrl, u32 sid)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  626  {
40f1aea80b53b8 Markus Stockhausen 2024-10-07  627  	int pwr, modecur, modeoff = ctrl->conf->mode_map[PHY_INTERFACE_MODE_NA];
40f1aea80b53b8 Markus Stockhausen 2024-10-07  628  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  629  	if (rtsds_invalid_sds(ctrl, sid))
40f1aea80b53b8 Markus Stockhausen 2024-10-07  630  		return -EINVAL;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  631  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  632  	modecur = rtsds_931x_get_mode(ctrl, sid);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  633  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  634  	if (modecur != modeoff) {
40f1aea80b53b8 Markus Stockhausen 2024-10-07  635  		/* reset with mode switch cycle while being powered off */
40f1aea80b53b8 Markus Stockhausen 2024-10-07  636  		pwr = ioread32(RTSDS_931X_PS_SERDES_OFF_MODE_CTRL);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  637  		iowrite32(pwr | BIT(sid), RTSDS_931X_PS_SERDES_OFF_MODE_CTRL);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  638  		rtsds_931x_set_mode(ctrl, sid, modeoff);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  639  		rtsds_931x_set_mode(ctrl, sid, modecur);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  640  		iowrite32(pwr, RTSDS_931X_PS_SERDES_OFF_MODE_CTRL);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  641  	}
40f1aea80b53b8 Markus Stockhausen 2024-10-07  642  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  643  	return 0;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  644  }
40f1aea80b53b8 Markus Stockhausen 2024-10-07  645  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  646  int rtsds_read(struct phy *phy, u32 page, u32 reg)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  647  {
40f1aea80b53b8 Markus Stockhausen 2024-10-07  648  	struct rtsds_macro *macro = phy_get_drvdata(phy);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  649  	struct rtsds_ctrl *ctrl = macro->ctrl;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  650  	u32 sid = macro->sid;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  651  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  652  	return ctrl->conf->read(ctrl, sid, page, reg);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  653  }
40f1aea80b53b8 Markus Stockhausen 2024-10-07  654  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  655  int rtsds_mask(struct phy *phy, u32 page, u32 reg, u32 val, u32 mask)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  656  {
40f1aea80b53b8 Markus Stockhausen 2024-10-07  657  	struct rtsds_macro *macro = phy_get_drvdata(phy);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  658  	struct rtsds_ctrl *ctrl = macro->ctrl;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  659  	u32 sid = macro->sid;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  660  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  661  	if (rtsds_readonly(ctrl, sid))
40f1aea80b53b8 Markus Stockhausen 2024-10-07  662  		return 0;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  663  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  664  	return ctrl->conf->mask(ctrl, sid, page, reg, val, mask);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  665  }
40f1aea80b53b8 Markus Stockhausen 2024-10-07  666  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  667  int rtsds_write(struct phy *phy, u32 page, u32 reg, u32 val)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  668  {
40f1aea80b53b8 Markus Stockhausen 2024-10-07  669  	return rtsds_mask(phy, page, reg, val, 0xffff);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  670  }
40f1aea80b53b8 Markus Stockhausen 2024-10-07  671  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  672  static int rtsds_phy_init(struct phy *phy)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  673  {
40f1aea80b53b8 Markus Stockhausen 2024-10-07  674  	struct rtsds_macro *macro = phy_get_drvdata(phy);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  675  	struct rtsds_ctrl *ctrl = macro->ctrl;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  676  	u32 sid = macro->sid;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  677  	int ret;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  678  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  679  	if (rtsds_readonly(ctrl, sid))
40f1aea80b53b8 Markus Stockhausen 2024-10-07  680  		return 0;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  681  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  682  	mutex_lock(&ctrl->lock);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  683  //	ret = rtsds_run_event(ctrl, sid, RTSDS_EVENT_INIT);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  684  	mutex_unlock(&ctrl->lock);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  685  
40f1aea80b53b8 Markus Stockhausen 2024-10-07 @686  	if (ret)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  687  		dev_err(ctrl->dev, "init failed for SerDes %d\n", sid);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  688  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  689  	return ret;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  690  }
40f1aea80b53b8 Markus Stockhausen 2024-10-07  691  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

AW: AW: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding

[markus.stockhausen]

> -----Ursprüngliche Nachricht-----
> Von: Krzysztof Kozlowski <krzk@kernel.org> 
> Gesendet: Dienstag, 8. Oktober 2024 08:17
> An: markus.stockhausen@gmx.de; linux-phy@lists.infradead.org; chris.packham@alliedtelesis.co.nz; devicetree@vger.kernel.org
> Betreff: Re: AW: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding
>
> On 08/10/2024 07:38, markus.stockhausen@gmx.de wrote:
> >> -----Ursprüngliche Nachricht-----
> >> Von: Krzysztof Kozlowski <krzk@kernel.org>
> >> Gesendet: Montag, 7. Oktober 2024 21:26
> >> An: Markus Stockhausen <markus.stockhausen@gmx.de>; 
> >> linux-phy@lists.infradead.org; chris.packham@alliedtelesis.co.nz; 
> >> devicetree@vger.kernel.org
> >> Betreff: Re: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes 
> >> PHY binding
> >>
> >> ... and still not tested. Sending untested code is waste of our time.
> > 
> > Hi Krzysztof,
> > 
> > appreciate your feedback and I do not want to waste your time. My 
> > fixes where a mix of your feedback and some half-baked "make 
> > dt_binding_check" feedbacks (because packages where missing). My fault and sorry fort he noise.
> > 
> > To get next version in better shape two questions regarding your feedback:
> > 
> > 1. "Messed wrapping": According to checkpatch 100 chars/line are accepted. 
> > So I designed the comments in the driver. Does devicetree differ from that?
>
> checkpatch is not a coding style. I asked to follow coding style, please read entire document in Documentation/process.

Understood.

> > 
> > 2 "Bindings vs drivers". The idea about controlled ports came from other bindings.
>
> Entire property description speaks about driver, not bindings.
>
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tre
> > e/Documentation/devicetree/bindings/interrupt-controller/st,stih407-ir
> > q-syscfg.yaml?h=v6.12-rc2
>
> stih is rather poor example to use. The property was added in 2015 (!) without review (!!!).
>
>
> > E.g. st,invert-ext. Something like this will be needed in the future 
> > because the SerDes allow to swap polarity which must be changed 
> > depending on the switch design. How to do this?
>
> I do not understand the hardware aspect discussed in the property description... probably because there is no hardware description at all, but instead you speak about driver.
>
> I do not understand how polarity has anything to do with U-Boot configuring serdes.

Maybe my lack of knowledge in platform driver programming or the naming
conventions leads to confusion. I'm searching for knobs to control the behaviour 
of the SerDes depending on the hardware. Two examples are (more may come):

- "ignore SerDes X": because the provided patch sequence confuses the SerDes
and overwrites registers with wrong values that vendor patched U-Boot has setup
correctly before. 

- "reverse polarity of SerDes X": same goes here. Some boards need inverted
signalling on some of the SerDes to work properly. This must be configurable
somehow.

Looking at some more modern implementation/documentation I need soemthing 
like in realtek,usb2phy.yaml - e.g. realtek,driving-level-compensate.

Should I just leave "driver" out of the description?

Best regards.

Markus

Re: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding

[Krzysztof Kozlowski]

On 07/10/2024 18:36, Markus Stockhausen wrote:
> Add bindings for the SerDes of the Realtek Otto platform. These are
> network Switch SoCs with up to 52 ports divided into four different
> model lines.
> 
> Changes in v2:
> - new subject
> - removed patch command sequences
> - renamed parameter controlled-ports to realtek,controlled-ports

<form letter>
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC (and consider --no-git-fallback argument, so you will
not CC people just because they made one commit years ago). It might
happen, that command when run on an older kernel, gives you outdated
entries. Therefore please be sure you base your patches on recent Linux
kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.
</form letter>

I already commented on this. The Cc list is still wrong. Please start
using tools which do it automatically and you do not have to perform any
choice of addresses.

Best regards,
Krzysztof

Re: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding

[Krzysztof Kozlowski]

On 07/10/2024 18:36, Markus Stockhausen wrote:
> +  compatible:
> +    items:
> +      - enum:
> +          - realtek,rtl8380-serdes
> +          - realtek,rtl8390-serdes
> +          - realtek,rtl9300-serdes

It turns out there is no chip like 9300. Align with Chris Packham,
because it is not a superposition-SoC (exists and does not exist the
same time, unless you check in given binding).

Best regards,
Krzysztof

Re: [PATCH v2 3/3] phy: Integrate Realtek Otto SerDes driver into build system

[kernel test robot]

Hi Markus,

kernel test robot noticed the following build errors:

[auto build test ERROR on robh/for-next]
[also build test ERROR on krzk-dt/for-next linus/master v6.12-rc2 next-20241004]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Markus-Stockhausen/dt-bindings-phy-add-realtek-otto-serdes-PHY-binding/20241008-003929
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20241007163623.3274510-4-markus.stockhausen%40gmx.de
patch subject: [PATCH v2 3/3] phy: Integrate Realtek Otto SerDes driver into build system
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20241008/202410081449.rLWpQtUj-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241008/202410081449.rLWpQtUj-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410081449.rLWpQtUj-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/linux/debugfs.h:16,
                    from drivers/phy/realtek/phy-rtk-otto-serdes.c:7:
   drivers/phy/realtek/phy-rtk-otto-serdes.c: In function 'rtsds_dbg_mode_open':
>> drivers/phy/realtek/phy-rtk-otto-serdes.c:868:29: error: passing argument 2 of 'single_open' from incompatible pointer type [-Werror=incompatible-pointer-types]
     868 | DEFINE_SHOW_STORE_ATTRIBUTE(rtsds_dbg_mode);
         |                             ^~~~~~~~~~~~~~
         |                             |
         |                             ssize_t (*)(struct seq_file *, void *) {aka long int (*)(struct seq_file *, void *)}
   include/linux/seq_file.h:223:34: note: in definition of macro 'DEFINE_SHOW_STORE_ATTRIBUTE'
     223 |         return single_open(file, __name ## _show, inode->i_private);    \
         |                                  ^~~~~~
   include/linux/seq_file.h:176:32: note: expected 'int (*)(struct seq_file *, void *)' but argument is of type 'ssize_t (*)(struct seq_file *, void *)' {aka 'long int (*)(struct seq_file *, void *)'}
     176 | int single_open(struct file *, int (*)(struct seq_file *, void *), void *);
         |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/phy/realtek/phy-rtk-otto-serdes.c: In function 'rtsds_dbg_reset_open':
   drivers/phy/realtek/phy-rtk-otto-serdes.c:891:29: error: passing argument 2 of 'single_open' from incompatible pointer type [-Werror=incompatible-pointer-types]
     891 | DEFINE_SHOW_STORE_ATTRIBUTE(rtsds_dbg_reset);
         |                             ^~~~~~~~~~~~~~~
         |                             |
         |                             ssize_t (*)(struct seq_file *, void *) {aka long int (*)(struct seq_file *, void *)}
   include/linux/seq_file.h:223:34: note: in definition of macro 'DEFINE_SHOW_STORE_ATTRIBUTE'
     223 |         return single_open(file, __name ## _show, inode->i_private);    \
         |                                  ^~~~~~
   include/linux/seq_file.h:176:32: note: expected 'int (*)(struct seq_file *, void *)' but argument is of type 'ssize_t (*)(struct seq_file *, void *)' {aka 'long int (*)(struct seq_file *, void *)'}
     176 | int single_open(struct file *, int (*)(struct seq_file *, void *), void *);
         |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/single_open +868 drivers/phy/realtek/phy-rtk-otto-serdes.c

40f1aea80b53b8 Markus Stockhausen 2024-10-07  847  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  848  static ssize_t rtsds_dbg_mode_write(struct file *file, const char __user *userbuf,
40f1aea80b53b8 Markus Stockhausen 2024-10-07  849  				size_t count, loff_t *ppos)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  850  {
40f1aea80b53b8 Markus Stockhausen 2024-10-07  851  	struct seq_file *seqf = file->private_data;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  852  	struct rtsds_macro *macro = dev_get_drvdata(seqf->private);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  853  	struct rtsds_ctrl *ctrl = macro->ctrl;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  854  	int ret, hwmode, phymode, sid = macro->sid;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  855  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  856  	ret = kstrtou32_from_user(userbuf, count, 16, &hwmode);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  857  	if (ret)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  858  		return ret;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  859  	/*
40f1aea80b53b8 Markus Stockhausen 2024-10-07  860  	 * Allow to set arbitrary modes into the SerDes to improve error analysis. Accept that
40f1aea80b53b8 Markus Stockhausen 2024-10-07  861  	 * this might confuse the internal state tracking.
40f1aea80b53b8 Markus Stockhausen 2024-10-07  862  	 */
40f1aea80b53b8 Markus Stockhausen 2024-10-07  863  	phymode = rtsds_hwmode_to_phymode(ctrl, hwmode);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  864  	rtsds_phy_set_mode_int(ctrl, sid, phymode, hwmode);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  865  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  866  	return count;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  867  }
40f1aea80b53b8 Markus Stockhausen 2024-10-07 @868  DEFINE_SHOW_STORE_ATTRIBUTE(rtsds_dbg_mode);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  869  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 3/3] phy: Integrate Realtek Otto SerDes driver into build system

[kernel test robot]

Hi Markus,

kernel test robot noticed the following build errors:

[auto build test ERROR on robh/for-next]
[also build test ERROR on krzk-dt/for-next linus/master v6.12-rc2 next-20241004]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Markus-Stockhausen/dt-bindings-phy-add-realtek-otto-serdes-PHY-binding/20241008-003929
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20241007163623.3274510-4-markus.stockhausen%40gmx.de
patch subject: [PATCH v2 3/3] phy: Integrate Realtek Otto SerDes driver into build system
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20241008/202410081607.EKE62jfx-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241008/202410081607.EKE62jfx-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410081607.EKE62jfx-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/phy/realtek/phy-rtk-otto-serdes.c:490:15: warning: overlapping comparisons always evaluate to true [-Wtautological-overlap-compare]
     490 |         if (sid >= 2 || sid <= 9)
         |             ~~~~~~~~~^~~~~~~~~~~
   drivers/phy/realtek/phy-rtk-otto-serdes.c:509:15: warning: overlapping comparisons always evaluate to true [-Wtautological-overlap-compare]
     509 |         if (sid >= 2 || sid <= 9)
         |             ~~~~~~~~~^~~~~~~~~~~
   drivers/phy/realtek/phy-rtk-otto-serdes.c:686:6: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
     686 |         if (ret)
         |             ^~~
   drivers/phy/realtek/phy-rtk-otto-serdes.c:677:9: note: initialize the variable 'ret' to silence this warning
     677 |         int ret;
         |                ^
         |                 = 0
   drivers/phy/realtek/phy-rtk-otto-serdes.c:706:6: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
     706 |         if (ret)
         |             ^~~
   drivers/phy/realtek/phy-rtk-otto-serdes.c:697:9: note: initialize the variable 'ret' to silence this warning
     697 |         int ret;
         |                ^
         |                 = 0
   drivers/phy/realtek/phy-rtk-otto-serdes.c:723:7: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
     723 |         if (!ret)
         |              ^~~
   drivers/phy/realtek/phy-rtk-otto-serdes.c:717:9: note: initialize the variable 'ret' to silence this warning
     717 |         int ret;
         |                ^
         |                 = 0
>> drivers/phy/realtek/phy-rtk-otto-serdes.c:868:1: error: incompatible function pointer types passing 'ssize_t (struct seq_file *, void *)' (aka 'long (struct seq_file *, void *)') to parameter of type 'int (*)(struct seq_file *, void *)' [-Wincompatible-function-pointer-types]
     868 | DEFINE_SHOW_STORE_ATTRIBUTE(rtsds_dbg_mode);
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/seq_file.h:223:27: note: expanded from macro 'DEFINE_SHOW_STORE_ATTRIBUTE'
     223 |         return single_open(file, __name ## _show, inode->i_private);    \
         |                                  ^~~~~~~~~~~~~~~
   <scratch space>:49:1: note: expanded from here
      49 | rtsds_dbg_mode_show
         | ^~~~~~~~~~~~~~~~~~~
   include/linux/seq_file.h:176:38: note: passing argument to parameter here
     176 | int single_open(struct file *, int (*)(struct seq_file *, void *), void *);
         |                                      ^
   drivers/phy/realtek/phy-rtk-otto-serdes.c:891:1: error: incompatible function pointer types passing 'ssize_t (struct seq_file *, void *)' (aka 'long (struct seq_file *, void *)') to parameter of type 'int (*)(struct seq_file *, void *)' [-Wincompatible-function-pointer-types]
     891 | DEFINE_SHOW_STORE_ATTRIBUTE(rtsds_dbg_reset);
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/seq_file.h:223:27: note: expanded from macro 'DEFINE_SHOW_STORE_ATTRIBUTE'
     223 |         return single_open(file, __name ## _show, inode->i_private);    \
         |                                  ^~~~~~~~~~~~~~~
   <scratch space>:54:1: note: expanded from here
      54 | rtsds_dbg_reset_show
         | ^~~~~~~~~~~~~~~~~~~~
   include/linux/seq_file.h:176:38: note: passing argument to parameter here
     176 | int single_open(struct file *, int (*)(struct seq_file *, void *), void *);
         |                                      ^
   5 warnings and 2 errors generated.

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for MODVERSIONS
   Depends on [n]: MODULES [=y] && !COMPILE_TEST [=y]
   Selected by [y]:
   - RANDSTRUCT_FULL [=y] && (CC_HAS_RANDSTRUCT [=y] || GCC_PLUGINS [=n]) && MODULES [=y]


vim +868 drivers/phy/realtek/phy-rtk-otto-serdes.c

40f1aea80b53b8 Markus Stockhausen 2024-10-07  847  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  848  static ssize_t rtsds_dbg_mode_write(struct file *file, const char __user *userbuf,
40f1aea80b53b8 Markus Stockhausen 2024-10-07  849  				size_t count, loff_t *ppos)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  850  {
40f1aea80b53b8 Markus Stockhausen 2024-10-07  851  	struct seq_file *seqf = file->private_data;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  852  	struct rtsds_macro *macro = dev_get_drvdata(seqf->private);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  853  	struct rtsds_ctrl *ctrl = macro->ctrl;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  854  	int ret, hwmode, phymode, sid = macro->sid;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  855  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  856  	ret = kstrtou32_from_user(userbuf, count, 16, &hwmode);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  857  	if (ret)
40f1aea80b53b8 Markus Stockhausen 2024-10-07  858  		return ret;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  859  	/*
40f1aea80b53b8 Markus Stockhausen 2024-10-07  860  	 * Allow to set arbitrary modes into the SerDes to improve error analysis. Accept that
40f1aea80b53b8 Markus Stockhausen 2024-10-07  861  	 * this might confuse the internal state tracking.
40f1aea80b53b8 Markus Stockhausen 2024-10-07  862  	 */
40f1aea80b53b8 Markus Stockhausen 2024-10-07  863  	phymode = rtsds_hwmode_to_phymode(ctrl, hwmode);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  864  	rtsds_phy_set_mode_int(ctrl, sid, phymode, hwmode);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  865  
40f1aea80b53b8 Markus Stockhausen 2024-10-07  866  	return count;
40f1aea80b53b8 Markus Stockhausen 2024-10-07  867  }
40f1aea80b53b8 Markus Stockhausen 2024-10-07 @868  DEFINE_SHOW_STORE_ATTRIBUTE(rtsds_dbg_mode);
40f1aea80b53b8 Markus Stockhausen 2024-10-07  869  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: AW: AW: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding

[Krzysztof Kozlowski]

On 08/10/2024 08:56, markus.stockhausen@gmx.de wrote:
>>
>>> E.g. st,invert-ext. Something like this will be needed in the future 
>>> because the SerDes allow to swap polarity which must be changed 
>>> depending on the switch design. How to do this?
>>
>> I do not understand the hardware aspect discussed in the property description... probably because there is no hardware description at all, but instead you speak about driver.
>>
>> I do not understand how polarity has anything to do with U-Boot configuring serdes.
> 
> Maybe my lack of knowledge in platform driver programming or the naming
> conventions leads to confusion. I'm searching for knobs to control the behaviour 
> of the SerDes depending on the hardware. Two examples are (more may come):
> 
> - "ignore SerDes X": because the provided patch sequence confuses the SerDes
> and overwrites registers with wrong values that vendor patched U-Boot has setup
> correctly before. 

And if someone updates the bootloader to a bit different one, the DTS
becomes wrong? How do you handle then same board with two different
bootloaders requiring two different DTS? DTS is software-independent
description of the hardware, so this does not look like DTS property.

> 
> - "reverse polarity of SerDes X": same goes here. Some boards need inverted
> signalling on some of the SerDes to work properly. This must be configurable
> somehow.

I do not see how this is related to "control ports" property. There are
few bindings which already do this, so look at them.

Best regards,
Krzysztof

AW: AW: AW: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding

[markus.stockhausen]

> -----Ursprüngliche Nachricht-----
> Von: Krzysztof Kozlowski <krzk@kernel.org> 
> Gesendet: Dienstag, 8. Oktober 2024 10:32
> An: markus.stockhausen@gmx.de; linux-phy@lists.infradead.org; chris.packham@alliedtelesis.co.nz; devicetree@vger.kernel.org
> Betreff: Re: AW: AW: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding
>
> On 08/10/2024 08:56, markus.stockhausen@gmx.de wrote:
> >>
> >>> E.g. st,invert-ext. Something like this will be needed in the future 
> >>> because the SerDes allow to swap polarity which must be changed 
> >>> depending on the switch design. How to do this?
> >>
> >> I do not understand the hardware aspect discussed in the property description... probably because there is no hardware description at all, but instead you speak about driver.
> >>
> >> I do not understand how polarity has anything to do with U-Boot configuring serdes.
> > 
> > Maybe my lack of knowledge in platform driver programming or the 
> > naming conventions leads to confusion. I'm searching for knobs to 
> > control the behaviour of the SerDes depending on the hardware. Two examples are (more may come):
> > 
> > - "ignore SerDes X": because the provided patch sequence confuses the 
> > SerDes and overwrites registers with wrong values that vendor patched 
> > U-Boot has setup correctly before.
>
> And if someone updates the bootloader to a bit different one, the DTS 
> becomes wrong? How do you handle then same board with two different 
> bootloaders requiring two different DTS? DTS is software-independent
> description of the hardware, so this does not look like DTS property.

Good point. So the initial idea to provide dynamic patch sequences was
the right direction but storing in devicetree is wrong. Like Chris mentioned
I would change the code to make use of a dynamically loaded firmware file.

- if exist: run sequences from there
- if not exist: keep registers as is.

Reasonable idea?

Best regards.

Markus

AW: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding

[markus.stockhausen]

> -----Ursprüngliche Nachricht-----
> Von: Rob Herring <robh@kernel.org> 
> Gesendet: Montag, 7. Oktober 2024 21:31
> An: Markus Stockhausen <markus.stockhausen@gmx.de>
> Cc: linux-phy@lists.infradead.org; chris.packham@alliedtelesis.co.nz;
devicetree@vger.kernel.org; krzk@kernel.org
> Betreff: Re: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY
binding
> ...
> > +
> > +properties:
> > +  $nodename:
> > +    pattern: "^serdes@[0-9a-f]+$"
>
> The node name for phy providers is 'phy'.

Hi Rob,

I found different configs in other files. E.g.

- torrent-phy@f0fb500000
- serdes: serdes@e2004010
- serdes_phy: phy@8901000

Do I understand correctly that I should go with "serdes: phy@1b00e780"? 
If yes, adapt the $nodename pattern accordingly or drop it as in most other
files?

Best regards.

Markus

AW: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding

[markus.stockhausen]

Hi Krzysztof,

with your feedback on the latest version I will take up the issues from 
v2 once again. To be sure that I do not miss anything in upcoming v5 
I will comment on all your feedback. 

> > ....
> > Changes in v2:
> > - new subject
> > - removed patch command sequences
> > - renamed parameter controlled-ports to realtek,controlled-ports
>
> Changelog goes under ---.

After reading this another 4 times now I think I understand. You mean
"put changelog below signed-off-by". Will do with next patch.

> > ....
> > diff --git 
> > a/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml 
> > b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml
> > new file mode 100644
> > index 000000000000..a72ac206b35f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml
>
> Nothing improved.

In between renamed to compatible "realtek,rtl8380m-serdes.yaml". I hope
that fits the requested naming convention.

> > +  The driver exposes the SerDes registers different from the hardware 
> > + but instead gives a  consistent view and programming interface. So 
> > + the RTL838x series has 6 ports and 4 pages, the  RTL839x has 14 
> > + ports and 12 pages, the RTL930x has 12 ports and 64 pages and the 
> > + RTL931x has
> > +  14 ports and 192 pages.
>
> Totally messed wrapping. Please wrap your code as Linux coding style.

Was restyled in between. If this is still an issue in latest version, please advise.

> > +  reg:
> > +    items:
> > +      description:
> > +        The primary SerDes paged register memory location. Other SerDes control and management
> > +        registers are distributed all over the I/O memory space and are identified by the driver.
>
> What happened here? I asked only about |. Why are you adding unrelated changes?
>
> Anyway, still not tested and still does not look any other binding.

Has been tested in between with "make dt_binding_check".

> > +  realtek,controlled-ports:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      A bit mask defining the ports that are actively controlled by 
> > + the driver. In case a bit is
>
> Driver? Bindings are not about drivers. Drop.
>
> I don't think you implemented my feedback.

All these have been removed.

> > +additionalProperties:
> > +  false
>
> Please open any existing binding and do it like there. Or start from scratch from example-schema.

Was converted to one line.

> > +
> > +examples:
> > +  - |
> > +    serdes: serdes@1b00e780 {
> > +      compatible = "realtek,rtl8380-serdes", "realtek,otto-serdes";
> > +      reg = <0x1b00e780 0x1200>;
> > +      controlled-ports = <0x003f>;
> > +      #phy-cells = <4>;
> > +    };
>
> One example is enough.

Only one example left.

Best regards.

Markus

Re: AW: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding

[Krzysztof Kozlowski]

On 16/10/2024 17:30, markus.stockhausen@gmx.de wrote:
> Hi Krzysztof,
> 
> with your feedback on the latest version I will take up the issues from 
> v2 once again. To be sure that I do not miss anything in upcoming v5 
> I will comment on all your feedback. 
> 
>>> ....
>>> Changes in v2:
>>> - new subject
>>> - removed patch command sequences
>>> - renamed parameter controlled-ports to realtek,controlled-ports
>>
>> Changelog goes under ---.
> 
> After reading this another 4 times now I think I understand. You mean
> "put changelog below signed-off-by". Will do with next patch.

--- is under Signed-off-by, so yes, but more importantly under ---.

> 
>>> ....
>>> diff --git 
>>> a/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml 
>>> b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml
>>> new file mode 100644
>>> index 000000000000..a72ac206b35f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml
>>
>> Nothing improved.
> 
> In between renamed to compatible "realtek,rtl8380m-serdes.yaml". I hope
> that fits the requested naming convention.

Yes.

> 
>>> +  The driver exposes the SerDes registers different from the hardware 
>>> + but instead gives a  consistent view and programming interface. So 
>>> + the RTL838x series has 6 ports and 4 pages, the  RTL839x has 14 
>>> + ports and 12 pages, the RTL930x has 12 ports and 64 pages and the 
>>> + RTL931x has
>>> +  14 ports and 192 pages.
>>
>> Totally messed wrapping. Please wrap your code as Linux coding style.
> 
> Was restyled in between. If this is still an issue in latest version, please advise.

It's ok, the rest as well.


Best regards,
Krzysztof