[PATCH v2 0/6] phy: realtek: usb2: support for RTL9607C USB2 PHY

[PATCH v2 0/6] phy: realtek: usb2: support for RTL9607C USB2 PHY

[Rustam Adilov]

This patch series for Realtek USB2 PHY driver adds support for RTL9607C
USB2 PHY.

RTL9607C is a big endian MIPS CPU which is quite far from RTD series SoCs
supported by realtek usb2 phy driver, but the phy initilization is found
to be very indentical in most areas.

Most of the code was based on the Realtek's usb driver from the GPL tarball
in [1] and adjusted to fit into the realtek usb2 phy driver code format.

The patch series was split into smaller patches that add/change something
in the driver that are not exactly related to RTL9607C and that also
helps for easier review. That also means, patch 5 depends on all the prior
patches that come before it.

USB2 PHY on RTL9607C is primarly used for its internal OHCI/EHCI controllers.

[1] - https://github.com/jameywine/GPL-for-GP3000/blob/main/linux-5.10.x/arch/mips/rtl9607c/usb.c

---
Changelog in v2:
- Patch 5
 - removed the line about OHCI/EHCI controllers from description.
 - set the resets to false for RTD SoC devices and changed the
   commit message to reflect that.
- Link to v1: https://lore.kernel.org/linux-phy/20260326193419.48419-1-adilov@disroot.org/

Rustam Adilov (6):
  phy: realtek: usb2: introduce vstatus/new_reg_req variables to driver
    data
  phy: realtek: usb2: introduce read and write functions to driver data
  dt-bindings: phy: realtek,usb2phy.yaml: extend for resets and RTL9607C
    support
  phy: realtek: usb2: introduce reset controller struct
  phy: realtek: usb2: add support for RTL9607C USB2 PHY
  phy: realtek: usb2: Make configs available for MACH_REALTEK_RTL

 .../bindings/phy/realtek,usb2phy.yaml         |  25 ++-
 drivers/phy/realtek/Kconfig                   |   2 +-
 drivers/phy/realtek/phy-rtk-usb2.c            | 189 ++++++++++++++++--
 3 files changed, 196 insertions(+), 20 deletions(-)

-- 
2.53.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v2 1/6] phy: realtek: usb2: introduce vstatus/new_reg_req variables to driver data

[Rustam Adilov]

In RTL9607C SoC, the vstatus register is located at a certain offset from
the base and so introduce the vstatus_offset to handle it.

Busy bit of the vstatus and new_reg_req bit are also different and so
introduce these variables to the driver data as well.

Add these variables to the pre-existing phy cfg structs for RTD SoCs and
assign them the default values.

Co-developed-by: Michael Zavertkin <misha.zavertkin@mail.ru>
Signed-off-by: Michael Zavertkin <misha.zavertkin@mail.ru>
Signed-off-by: Rustam Adilov <adilov@disroot.org>
---
 drivers/phy/realtek/phy-rtk-usb2.c | 59 ++++++++++++++++++++++++------
 1 file changed, 48 insertions(+), 11 deletions(-)

diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
index 248550ef98ca..f5d2f0c3376a 100644
--- a/drivers/phy/realtek/phy-rtk-usb2.c
+++ b/drivers/phy/realtek/phy-rtk-usb2.c
@@ -64,6 +64,9 @@ struct phy_reg {
 	void __iomem *reg_wrap_vstatus;
 	void __iomem *reg_gusb2phyacc0;
 	int vstatus_index;
+	int vstatus_offset;
+	int vstatus_busy;
+	int new_reg_req;
 };
 
 struct phy_data {
@@ -96,6 +99,9 @@ struct phy_cfg {
 	bool do_toggle_driving;
 	bool use_default_parameter;
 	bool is_double_sensitivity_mode;
+	int vstatus_offset;
+	int vstatus_busy;
+	int new_reg_req;
 };
 
 struct phy_parameter {
@@ -162,21 +168,21 @@ static char rtk_phy_read(struct phy_reg *phy_reg, char addr)
 	addr -= OFFEST_PHY_READ;
 
 	/* polling until VBusy == 0 */
-	ret = utmi_wait_register(reg_gusb2phyacc0, PHY_VSTS_BUSY, 0);
+	ret = utmi_wait_register(reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
 	if (ret)
 		return (char)ret;
 
 	/* VCtrl = low nibble of addr, and set PHY_NEW_REG_REQ */
-	val = PHY_NEW_REG_REQ | (GET_LOW_NIBBLE(addr) << PHY_VCTRL_SHIFT);
+	val = phy_reg->new_reg_req | (GET_LOW_NIBBLE(addr) << PHY_VCTRL_SHIFT);
 	writel(val, reg_gusb2phyacc0);
-	ret = utmi_wait_register(reg_gusb2phyacc0, PHY_VSTS_BUSY, 0);
+	ret = utmi_wait_register(reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
 	if (ret)
 		return (char)ret;
 
 	/* VCtrl = high nibble of addr, and set PHY_NEW_REG_REQ */
-	val = PHY_NEW_REG_REQ | (GET_HIGH_NIBBLE(addr) << PHY_VCTRL_SHIFT);
+	val = phy_reg->new_reg_req | (GET_HIGH_NIBBLE(addr) << PHY_VCTRL_SHIFT);
 	writel(val, reg_gusb2phyacc0);
-	ret = utmi_wait_register(reg_gusb2phyacc0, PHY_VSTS_BUSY, 0);
+	ret = utmi_wait_register(reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
 	if (ret)
 		return (char)ret;
 
@@ -194,25 +200,25 @@ static int rtk_phy_write(struct phy_reg *phy_reg, char addr, char data)
 	int ret = 0;
 
 	/* write data to VStatusOut2 (data output to phy) */
-	writel((u32)data << shift_bits, reg_wrap_vstatus);
+	writel((u32)data << shift_bits, reg_wrap_vstatus + phy_reg->vstatus_offset);
 
-	ret = utmi_wait_register(reg_gusb2phyacc0, PHY_VSTS_BUSY, 0);
+	ret = utmi_wait_register(reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
 	if (ret)
 		return ret;
 
 	/* VCtrl = low nibble of addr, set PHY_NEW_REG_REQ */
-	val = PHY_NEW_REG_REQ | (GET_LOW_NIBBLE(addr) << PHY_VCTRL_SHIFT);
+	val = phy_reg->new_reg_req | (GET_LOW_NIBBLE(addr) << PHY_VCTRL_SHIFT);
 
 	writel(val, reg_gusb2phyacc0);
-	ret = utmi_wait_register(reg_gusb2phyacc0, PHY_VSTS_BUSY, 0);
+	ret = utmi_wait_register(reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
 	if (ret)
 		return ret;
 
 	/* VCtrl = high nibble of addr, set PHY_NEW_REG_REQ */
-	val = PHY_NEW_REG_REQ | (GET_HIGH_NIBBLE(addr) << PHY_VCTRL_SHIFT);
+	val = phy_reg->new_reg_req | (GET_HIGH_NIBBLE(addr) << PHY_VCTRL_SHIFT);
 
 	writel(val, reg_gusb2phyacc0);
-	ret = utmi_wait_register(reg_gusb2phyacc0, PHY_VSTS_BUSY, 0);
+	ret = utmi_wait_register(reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
 	if (ret)
 		return ret;
 
@@ -957,6 +963,7 @@ static int get_phy_data_by_efuse(struct rtk_phy *rtk_phy,
 
 static int parse_phy_data(struct rtk_phy *rtk_phy)
 {
+	struct phy_cfg *phy_cfg = rtk_phy->phy_cfg;
 	struct device *dev = rtk_phy->dev;
 	struct device_node *np = dev->of_node;
 	struct phy_parameter *phy_parameter;
@@ -974,6 +981,9 @@ static int parse_phy_data(struct rtk_phy *rtk_phy)
 		phy_parameter->phy_reg.reg_wrap_vstatus = of_iomap(np, 0);
 		phy_parameter->phy_reg.reg_gusb2phyacc0 = of_iomap(np, 1) + index;
 		phy_parameter->phy_reg.vstatus_index = index;
+		phy_parameter->phy_reg.vstatus_offset = phy_cfg->vstatus_offset;
+		phy_parameter->phy_reg.vstatus_busy = phy_cfg->vstatus_busy;
+		phy_parameter->phy_reg.new_reg_req = phy_cfg->new_reg_req;
 
 		if (of_property_read_bool(np, "realtek,inverse-hstx-sync-clock"))
 			phy_parameter->inverse_hstx_sync_clock = true;
@@ -1085,6 +1095,9 @@ static const struct phy_cfg rtd1295_phy_cfg = {
 	.driving_updated_for_dev_dis = 0xf,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = false,
+	.vstatus_offset = 0,
+	.vstatus_busy = PHY_VSTS_BUSY,
+	.new_reg_req = PHY_NEW_REG_REQ,
 };
 
 static const struct phy_cfg rtd1395_phy_cfg = {
@@ -1109,6 +1122,9 @@ static const struct phy_cfg rtd1395_phy_cfg = {
 	.driving_updated_for_dev_dis = 0xf,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = false,
+	.vstatus_offset = 0,
+	.vstatus_busy = PHY_VSTS_BUSY,
+	.new_reg_req = PHY_NEW_REG_REQ,
 };
 
 static const struct phy_cfg rtd1395_phy_cfg_2port = {
@@ -1133,6 +1149,9 @@ static const struct phy_cfg rtd1395_phy_cfg_2port = {
 	.driving_updated_for_dev_dis = 0xf,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = false,
+	.vstatus_offset = 0,
+	.vstatus_busy = PHY_VSTS_BUSY,
+	.new_reg_req = PHY_NEW_REG_REQ,
 };
 
 static const struct phy_cfg rtd1619_phy_cfg = {
@@ -1155,6 +1174,9 @@ static const struct phy_cfg rtd1619_phy_cfg = {
 	.driving_updated_for_dev_dis = 0xf,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = false,
+	.vstatus_offset = 0,
+	.vstatus_busy = PHY_VSTS_BUSY,
+	.new_reg_req = PHY_NEW_REG_REQ,
 };
 
 static const struct phy_cfg rtd1319_phy_cfg = {
@@ -1181,6 +1203,9 @@ static const struct phy_cfg rtd1319_phy_cfg = {
 	.driving_updated_for_dev_dis = 0xf,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = true,
+	.vstatus_offset = 0,
+	.vstatus_busy = PHY_VSTS_BUSY,
+	.new_reg_req = PHY_NEW_REG_REQ,
 };
 
 static const struct phy_cfg rtd1312c_phy_cfg = {
@@ -1206,6 +1231,9 @@ static const struct phy_cfg rtd1312c_phy_cfg = {
 	.driving_updated_for_dev_dis = 0xf,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = true,
+	.vstatus_offset = 0,
+	.vstatus_busy = PHY_VSTS_BUSY,
+	.new_reg_req = PHY_NEW_REG_REQ,
 };
 
 static const struct phy_cfg rtd1619b_phy_cfg = {
@@ -1231,6 +1259,9 @@ static const struct phy_cfg rtd1619b_phy_cfg = {
 	.driving_updated_for_dev_dis = 0x8,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = true,
+	.vstatus_offset = 0,
+	.vstatus_busy = PHY_VSTS_BUSY,
+	.new_reg_req = PHY_NEW_REG_REQ,
 };
 
 static const struct phy_cfg rtd1319d_phy_cfg = {
@@ -1256,6 +1287,9 @@ static const struct phy_cfg rtd1319d_phy_cfg = {
 	.driving_updated_for_dev_dis = 0x8,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = true,
+	.vstatus_offset = 0,
+	.vstatus_busy = PHY_VSTS_BUSY,
+	.new_reg_req = PHY_NEW_REG_REQ,
 };
 
 static const struct phy_cfg rtd1315e_phy_cfg = {
@@ -1282,6 +1316,9 @@ static const struct phy_cfg rtd1315e_phy_cfg = {
 	.driving_updated_for_dev_dis = 0x8,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = true,
+	.vstatus_offset = 0,
+	.vstatus_busy = PHY_VSTS_BUSY,
+	.new_reg_req = PHY_NEW_REG_REQ,
 };
 
 static const struct of_device_id usbphy_rtk_dt_match[] = {
-- 
2.53.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v2 2/6] phy: realtek: usb2: introduce read and write functions to driver data

[Rustam Adilov]

RTL9607C is a big endian SoC but has little endian USB host controller and
thus, reads and writes to the reg_gusb2phyacc0 should go through
le32_to_cpu and cpu_to_le32 functions respectively. This doesn't apply to
vstatus register though.

To handle this situation, introduce read and write functions to the driver
data and create 2 variations of reads and write function with le32 function
in it and without.

Adjust all instances of utmi_wait_register function to now include the read
function as one of its arguments.

Assign the existing phy configuration for RTD SoCs to the default phy_read
and phy_write functions.

Co-developed-by: Michael Zavertkin <misha.zavertkin@mail.ru>
Signed-off-by: Michael Zavertkin <misha.zavertkin@mail.ru>
Signed-off-by: Rustam Adilov <adilov@disroot.org>
---
 drivers/phy/realtek/phy-rtk-usb2.c | 73 ++++++++++++++++++++++++------
 1 file changed, 60 insertions(+), 13 deletions(-)

diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
index f5d2f0c3376a..e65b8525b88b 100644
--- a/drivers/phy/realtek/phy-rtk-usb2.c
+++ b/drivers/phy/realtek/phy-rtk-usb2.c
@@ -67,6 +67,9 @@ struct phy_reg {
 	int vstatus_offset;
 	int vstatus_busy;
 	int new_reg_req;
+
+	u32 (*read)(void __iomem *reg);
+	void (*write)(u32 val, void __iomem *reg);
 };
 
 struct phy_data {
@@ -102,6 +105,9 @@ struct phy_cfg {
 	int vstatus_offset;
 	int vstatus_busy;
 	int new_reg_req;
+
+	u32 (*read)(void __iomem *reg);
+	void (*write)(u32 val, void __iomem *reg);
 };
 
 struct phy_parameter {
@@ -128,6 +134,26 @@ struct rtk_phy {
 	struct dentry *debug_dir;
 };
 
+static inline u32 phy_read(void __iomem *reg)
+{
+	return readl(reg);
+}
+
+static inline u32 phy_read_le(void __iomem *reg)
+{
+	return le32_to_cpu(readl(reg));
+}
+
+static inline void phy_write(u32 val, void __iomem *reg)
+{
+	writel(val, reg);
+}
+
+static inline void phy_write_le(u32 val, void __iomem *reg)
+{
+	writel(cpu_to_le32(val), reg);
+}
+
 /* mapping 0xE0 to 0 ... 0xE7 to 7, 0xF0 to 8 ,,, 0xF7 to 15 */
 static inline int page_addr_to_array_index(u8 addr)
 {
@@ -144,12 +170,13 @@ static inline u8 array_index_to_page_addr(int index)
 #define PHY_IO_TIMEOUT_USEC		(50000)
 #define PHY_IO_DELAY_US			(100)
 
-static inline int utmi_wait_register(void __iomem *reg, u32 mask, u32 result)
+static inline int utmi_wait_register(u32 (*read)(void __iomem *reg), void __iomem *reg, u32 mask,
+				     u32 result)
 {
 	int ret;
 	unsigned int val;
 
-	ret = read_poll_timeout(readl, val, ((val & mask) == result),
+	ret = read_poll_timeout(read, val, ((val & mask) == result),
 				PHY_IO_DELAY_US, PHY_IO_TIMEOUT_USEC, false, reg);
 	if (ret) {
 		pr_err("%s can't program USB phy\n", __func__);
@@ -168,25 +195,25 @@ static char rtk_phy_read(struct phy_reg *phy_reg, char addr)
 	addr -= OFFEST_PHY_READ;
 
 	/* polling until VBusy == 0 */
-	ret = utmi_wait_register(reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
+	ret = utmi_wait_register(phy_reg->read, reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
 	if (ret)
 		return (char)ret;
 
 	/* VCtrl = low nibble of addr, and set PHY_NEW_REG_REQ */
 	val = phy_reg->new_reg_req | (GET_LOW_NIBBLE(addr) << PHY_VCTRL_SHIFT);
-	writel(val, reg_gusb2phyacc0);
-	ret = utmi_wait_register(reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
+	phy_reg->write(val, reg_gusb2phyacc0);
+	ret = utmi_wait_register(phy_reg->read, reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
 	if (ret)
 		return (char)ret;
 
 	/* VCtrl = high nibble of addr, and set PHY_NEW_REG_REQ */
 	val = phy_reg->new_reg_req | (GET_HIGH_NIBBLE(addr) << PHY_VCTRL_SHIFT);
-	writel(val, reg_gusb2phyacc0);
-	ret = utmi_wait_register(reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
+	phy_reg->write(val, reg_gusb2phyacc0);
+	ret = utmi_wait_register(phy_reg->read, reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
 	if (ret)
 		return (char)ret;
 
-	val = readl(reg_gusb2phyacc0);
+	val = phy_reg->read(reg_gusb2phyacc0);
 
 	return (char)(val & PHY_REG_DATA_MASK);
 }
@@ -202,23 +229,23 @@ static int rtk_phy_write(struct phy_reg *phy_reg, char addr, char data)
 	/* write data to VStatusOut2 (data output to phy) */
 	writel((u32)data << shift_bits, reg_wrap_vstatus + phy_reg->vstatus_offset);
 
-	ret = utmi_wait_register(reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
+	ret = utmi_wait_register(phy_reg->read, reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
 	if (ret)
 		return ret;
 
 	/* VCtrl = low nibble of addr, set PHY_NEW_REG_REQ */
 	val = phy_reg->new_reg_req | (GET_LOW_NIBBLE(addr) << PHY_VCTRL_SHIFT);
 
-	writel(val, reg_gusb2phyacc0);
-	ret = utmi_wait_register(reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
+	phy_reg->write(val, reg_gusb2phyacc0);
+	ret = utmi_wait_register(phy_reg->read, reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
 	if (ret)
 		return ret;
 
 	/* VCtrl = high nibble of addr, set PHY_NEW_REG_REQ */
 	val = phy_reg->new_reg_req | (GET_HIGH_NIBBLE(addr) << PHY_VCTRL_SHIFT);
 
-	writel(val, reg_gusb2phyacc0);
-	ret = utmi_wait_register(reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
+	phy_reg->write(val, reg_gusb2phyacc0);
+	ret = utmi_wait_register(phy_reg->read, reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
 	if (ret)
 		return ret;
 
@@ -984,6 +1011,8 @@ static int parse_phy_data(struct rtk_phy *rtk_phy)
 		phy_parameter->phy_reg.vstatus_offset = phy_cfg->vstatus_offset;
 		phy_parameter->phy_reg.vstatus_busy = phy_cfg->vstatus_busy;
 		phy_parameter->phy_reg.new_reg_req = phy_cfg->new_reg_req;
+		phy_parameter->phy_reg.read = phy_cfg->read;
+		phy_parameter->phy_reg.write = phy_cfg->write;
 
 		if (of_property_read_bool(np, "realtek,inverse-hstx-sync-clock"))
 			phy_parameter->inverse_hstx_sync_clock = true;
@@ -1098,6 +1127,8 @@ static const struct phy_cfg rtd1295_phy_cfg = {
 	.vstatus_offset = 0,
 	.vstatus_busy = PHY_VSTS_BUSY,
 	.new_reg_req = PHY_NEW_REG_REQ,
+	.read = phy_read,
+	.write = phy_write,
 };
 
 static const struct phy_cfg rtd1395_phy_cfg = {
@@ -1125,6 +1156,8 @@ static const struct phy_cfg rtd1395_phy_cfg = {
 	.vstatus_offset = 0,
 	.vstatus_busy = PHY_VSTS_BUSY,
 	.new_reg_req = PHY_NEW_REG_REQ,
+	.read = phy_read,
+	.write = phy_write,
 };
 
 static const struct phy_cfg rtd1395_phy_cfg_2port = {
@@ -1152,6 +1185,8 @@ static const struct phy_cfg rtd1395_phy_cfg_2port = {
 	.vstatus_offset = 0,
 	.vstatus_busy = PHY_VSTS_BUSY,
 	.new_reg_req = PHY_NEW_REG_REQ,
+	.read = phy_read,
+	.write = phy_write,
 };
 
 static const struct phy_cfg rtd1619_phy_cfg = {
@@ -1177,6 +1212,8 @@ static const struct phy_cfg rtd1619_phy_cfg = {
 	.vstatus_offset = 0,
 	.vstatus_busy = PHY_VSTS_BUSY,
 	.new_reg_req = PHY_NEW_REG_REQ,
+	.read = phy_read,
+	.write = phy_write,
 };
 
 static const struct phy_cfg rtd1319_phy_cfg = {
@@ -1206,6 +1243,8 @@ static const struct phy_cfg rtd1319_phy_cfg = {
 	.vstatus_offset = 0,
 	.vstatus_busy = PHY_VSTS_BUSY,
 	.new_reg_req = PHY_NEW_REG_REQ,
+	.read = phy_read,
+	.write = phy_write,
 };
 
 static const struct phy_cfg rtd1312c_phy_cfg = {
@@ -1234,6 +1273,8 @@ static const struct phy_cfg rtd1312c_phy_cfg = {
 	.vstatus_offset = 0,
 	.vstatus_busy = PHY_VSTS_BUSY,
 	.new_reg_req = PHY_NEW_REG_REQ,
+	.read = phy_read,
+	.write = phy_write,
 };
 
 static const struct phy_cfg rtd1619b_phy_cfg = {
@@ -1262,6 +1303,8 @@ static const struct phy_cfg rtd1619b_phy_cfg = {
 	.vstatus_offset = 0,
 	.vstatus_busy = PHY_VSTS_BUSY,
 	.new_reg_req = PHY_NEW_REG_REQ,
+	.read = phy_read,
+	.write = phy_write,
 };
 
 static const struct phy_cfg rtd1319d_phy_cfg = {
@@ -1290,6 +1333,8 @@ static const struct phy_cfg rtd1319d_phy_cfg = {
 	.vstatus_offset = 0,
 	.vstatus_busy = PHY_VSTS_BUSY,
 	.new_reg_req = PHY_NEW_REG_REQ,
+	.read = phy_read,
+	.write = phy_write,
 };
 
 static const struct phy_cfg rtd1315e_phy_cfg = {
@@ -1319,6 +1364,8 @@ static const struct phy_cfg rtd1315e_phy_cfg = {
 	.vstatus_offset = 0,
 	.vstatus_busy = PHY_VSTS_BUSY,
 	.new_reg_req = PHY_NEW_REG_REQ,
+	.read = phy_read,
+	.write = phy_write,
 };
 
 static const struct of_device_id usbphy_rtk_dt_match[] = {
-- 
2.53.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 2/6] phy: realtek: usb2: introduce read and write functions to driver data

[Vladimir Oltean]

On Fri, Mar 27, 2026 at 09:06:34PM +0500, Rustam Adilov wrote:
> +static inline u32 phy_read(void __iomem *reg)
> +{
> +	return readl(reg);
> +}
> +
> +static inline u32 phy_read_le(void __iomem *reg)
> +{
> +	return le32_to_cpu(readl(reg));
> +}
> +
> +static inline void phy_write(u32 val, void __iomem *reg)
> +{
> +	writel(val, reg);
> +}
> +
> +static inline void phy_write_le(u32 val, void __iomem *reg)
> +{
> +	writel(cpu_to_le32(val), reg);
> +}

Please don't name driver-level functions phy_read() and phy_write().
That will collide with networking API functions of the same name and
will make grep-based code searching more difficult.

Also, have you looked at regmap? It has native support for endianness;
it supports regmap_field_read()/regmap_field_write() for abstracting
registers which may be found at different places for different HW;
it offers regmap_read_poll_timeout() so you don't have to pass the
function pointer to utmi_wait_register(). It seems the result would be a
bit more elegant.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 2/6] phy: realtek: usb2: introduce read and write functions to driver data

[Vladimir Oltean]

On Tue, Mar 31, 2026 at 12:19:18AM +0300, Vladimir Oltean wrote:
> On Fri, Mar 27, 2026 at 09:06:34PM +0500, Rustam Adilov wrote:
> > +static inline u32 phy_read(void __iomem *reg)
> > +{
> > +	return readl(reg);
> > +}
> > +
> > +static inline u32 phy_read_le(void __iomem *reg)
> > +{
> > +	return le32_to_cpu(readl(reg));
> > +}
> > +
> > +static inline void phy_write(u32 val, void __iomem *reg)
> > +{
> > +	writel(val, reg);
> > +}
> > +
> > +static inline void phy_write_le(u32 val, void __iomem *reg)
> > +{
> > +	writel(cpu_to_le32(val), reg);
> > +}
> 
> Please don't name driver-level functions phy_read() and phy_write().
> That will collide with networking API functions of the same name and
> will make grep-based code searching more difficult.
> 
> Also, have you looked at regmap? It has native support for endianness;
> it supports regmap_field_read()/regmap_field_write() for abstracting
> registers which may be found at different places for different HW;
> it offers regmap_read_poll_timeout() so you don't have to pass the
> function pointer to utmi_wait_register(). It seems the result would be a
> bit more elegant.

Even if you decide not to use regmap. I thought I should let you know
that LLM review says:

  Are these double byte-swaps intentional?

  Since readl() and writel() inherently perform little-endian memory accesses
  and handle byte-swapping on big-endian architectures automatically, won't
  wrapping them in le32_to_cpu() and cpu_to_le32() apply a second, redundant
  byte-swap?

  On big-endian systems, wouldn't these double swaps cancel each other out
  and result in a native big-endian access instead of the intended
  little-endian access? If the SoC bus bridge implicitly swaps and requires
  a native access, should __raw_readl() and __raw_writel() (or ioread32be /
  iowrite32be) be used instead to avoid obfuscating it with double-swaps?

  Also, does passing the __le32 restricted type returned by cpu_to_le32()
  into writel() (which expects a native u32) trigger Sparse static analysis
  warnings for an incorrect type in argument?

For reference:
https://elixir.bootlin.com/linux/v6.19.10/source/include/asm-generic/io.h#L184
/*
 * {read,write}{b,w,l,q}() access little endian memory and return result in
 * native endianness.
 */

and yes, your patch does trigger sparse warnings:
../drivers/phy/realtek/phy-rtk-usb2.c:153:16: warning: cast to restricted __le32
../drivers/phy/realtek/phy-rtk-usb2.c:163:16: warning: incorrect type in argument 1 (different base types)
../drivers/phy/realtek/phy-rtk-usb2.c:163:16:    expected unsigned int val
../drivers/phy/realtek/phy-rtk-usb2.c:163:16:    got restricted __le32 [usertype]

Furthermore, please drop the 'inline' keyword from C files and let the
compiler decide. Your use of this keyword has no value - you declare
phy_read(), phy_read_le() etc as inline but then assign function
pointers to them. How can the compiler inline the indirect calls?

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 2/6] phy: realtek: usb2: introduce read and write functions to driver data

[Rustam Adilov]

On 2026-03-30 21:32, Vladimir Oltean wrote:
> On Tue, Mar 31, 2026 at 12:19:18AM +0300, Vladimir Oltean wrote:
>> On Fri, Mar 27, 2026 at 09:06:34PM +0500, Rustam Adilov wrote:
>> > +static inline u32 phy_read(void __iomem *reg)
>> > +{
>> > +	return readl(reg);
>> > +}
>> > +
>> > +static inline u32 phy_read_le(void __iomem *reg)
>> > +{
>> > +	return le32_to_cpu(readl(reg));
>> > +}
>> > +
>> > +static inline void phy_write(u32 val, void __iomem *reg)
>> > +{
>> > +	writel(val, reg);
>> > +}
>> > +
>> > +static inline void phy_write_le(u32 val, void __iomem *reg)
>> > +{
>> > +	writel(cpu_to_le32(val), reg);
>> > +}
>> 
>> Please don't name driver-level functions phy_read() and phy_write().
>> That will collide with networking API functions of the same name and
>> will make grep-based code searching more difficult.
>> 
>> Also, have you looked at regmap? It has native support for endianness;
>> it supports regmap_field_read()/regmap_field_write() for abstracting
>> registers which may be found at different places for different HW;
>> it offers regmap_read_poll_timeout() so you don't have to pass the
>> function pointer to utmi_wait_register(). It seems the result would be a
>> bit more elegant.
> 
> Even if you decide not to use regmap. I thought I should let you know
> that LLM review says:
> 
>   Are these double byte-swaps intentional?
> 
>   Since readl() and writel() inherently perform little-endian memory accesses
>   and handle byte-swapping on big-endian architectures automatically, won't
>   wrapping them in le32_to_cpu() and cpu_to_le32() apply a second, redundant
>   byte-swap?

From my experience (and also understanding), readl returns value in native endian
and doesn't do byte swapping. The same goes writel.
Thus wrapping le32_to_cpu() and cpu_to_le32() around them correctly swaps the
bytes from big endian to little endian, without the double byte-swaps.

The comment even mentions it "{read,write}{b,w,l,q}() access little endian memory
and return result in native endianness." which was provided for reference later on.

>   On big-endian systems, wouldn't these double swaps cancel each other out
>   and result in a native big-endian access instead of the intended
>   little-endian access? If the SoC bus bridge implicitly swaps and requires
>   a native access, should __raw_readl() and __raw_writel() (or ioread32be /
>   iowrite32be) be used instead to avoid obfuscating it with double-swaps?

The __raw_readl() and __raw_writel() are indeed in native endian, and for
RTL9607C SoC it is in big endian.

The way Realtek did it is by using the volatile and wrapping them around
le32_to_cpu() and cpu_to_le32() respectively, which is certainly hacky.

 static inline unsigned int ehci_readl(const struct ehci_hcd *ehci,
 		__u32 __iomem *regs)
 {
+#if defined(CONFIG_RTK_MIPS_SOC)
+	return (le32_to_cpu((*(volatile unsigned long *)(regs))));
+#else
[...]

 static inline void ehci_writel(const struct ehci_hcd *ehci,
 		const unsigned int val, __u32 __iomem *regs)
 {
+#if defined(CONFIG_RTK_MIPS_SOC)
+	((*(volatile unsigned long *)(regs))=cpu_to_le32(val));
+#else

We did a bit of debugging with usb some time ago and and printed the value
of the readl result from the base usb address and got this
[    1.327473] ehci_setup:694 ehci caps: 0xb8021000, value: 0x10000001
[    1.334478] ehci_setup:695 ehci regs: 0xb8021001
[    1.339706] ehci_halt:187 ehci regs: 0xb8021001

"0x10000001" is supposed to be "0x01000010". Otherwise, it would take 0x01
for the cap length and result in immediate halt from timeout.

Even though it is for linux-usb and not the phy subsystem, it was worth
mentioning as it is very much related to this issue.

What is surprising, the ioread32be and iowrite32be functions actually do
swap bytes, thus resulting in little endian (ironic). But using it here
would be just incorrect as it is intended for accessing big endian mmio, 
not for big endian CPU and its little endian USB phy/host.

So, that is a conundrum we have here. Let me know if what you think of it
and maybe even how to better solve it.

Will hold on posting v3 until this hopefully is more or less solved..

>   Also, does passing the __le32 restricted type returned by cpu_to_le32()
>   into writel() (which expects a native u32) trigger Sparse static analysis
>   warnings for an incorrect type in argument?
> 
> For reference:
> https://elixir.bootlin.com/linux/v6.19.10/source/include/asm-generic/io.h#L184
> /*
>  * {read,write}{b,w,l,q}() access little endian memory and return result in
>  * native endianness.
>  */
> 
> and yes, your patch does trigger sparse warnings:
> ../drivers/phy/realtek/phy-rtk-usb2.c:153:16: warning: cast to restricted __le32
> ../drivers/phy/realtek/phy-rtk-usb2.c:163:16: warning: incorrect type in argument 1 (different base types)
> ../drivers/phy/realtek/phy-rtk-usb2.c:163:16:    expected unsigned int val
> ../drivers/phy/realtek/phy-rtk-usb2.c:163:16:    got restricted __le32 [usertype]

I can alleviate it by creating a temp u32 variable to hold the cpu_to_le32() and then
using that temp variable for writel.

> Furthermore, please drop the 'inline' keyword from C files and let the
> compiler decide. Your use of this keyword has no value - you declare
> phy_read(), phy_read_le() etc as inline but then assign function
> pointers to them. How can the compiler inline the indirect calls?

Will drop these.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 2/6] phy: realtek: usb2: introduce read and write functions to driver data

[Michael Zavertkin]

On Tue, Mar 31, 2026 at 12:32:27AM +0300, Vladimir Oltean wrote:
> On Tue, Mar 31, 2026 at 12:19:18AM +0300, Vladimir Oltean wrote:
> > On Fri, Mar 27, 2026 at 09:06:34PM +0500, Rustam Adilov wrote:
> > > +static inline u32 phy_read(void __iomem *reg)
> > > +{
> > > +	return readl(reg);
> > > +}
> > > +
> > > +static inline u32 phy_read_le(void __iomem *reg)
> > > +{
> > > +	return le32_to_cpu(readl(reg));
> > > +}
> > > +
> > > +static inline void phy_write(u32 val, void __iomem *reg)
> > > +{
> > > +	writel(val, reg);
> > > +}
> > > +
> > > +static inline void phy_write_le(u32 val, void __iomem *reg)
> > > +{
> > > +	writel(cpu_to_le32(val), reg);
> > > +}
> > 
> > Please don't name driver-level functions phy_read() and phy_write().
> > That will collide with networking API functions of the same name and
> > will make grep-based code searching more difficult.
> > 
> > Also, have you looked at regmap? It has native support for endianness;
> > it supports regmap_field_read()/regmap_field_write() for abstracting
> > registers which may be found at different places for different HW;
> > it offers regmap_read_poll_timeout() so you don't have to pass the
> > function pointer to utmi_wait_register(). It seems the result would be a
> > bit more elegant.
> 
> Even if you decide not to use regmap. I thought I should let you know
> that LLM review says:
> 
>   Are these double byte-swaps intentional?
> 
>   Since readl() and writel() inherently perform little-endian memory accesses
>   and handle byte-swapping on big-endian architectures automatically, won't
>   wrapping them in le32_to_cpu() and cpu_to_le32() apply a second, redundant
>   byte-swap?
> 
>   On big-endian systems, wouldn't these double swaps cancel each other out
>   and result in a native big-endian access instead of the intended
>   little-endian access? If the SoC bus bridge implicitly swaps and requires
>   a native access, should __raw_readl() and __raw_writel() (or ioread32be /
>   iowrite32be) be used instead to avoid obfuscating it with double-swaps?
> 
>   Also, does passing the __le32 restricted type returned by cpu_to_le32()
>   into writel() (which expects a native u32) trigger Sparse static analysis
>   warnings for an incorrect type in argument?
> 
> For reference:
> https://elixir.bootlin.com/linux/v6.19.10/source/include/asm-generic/io.h#L184
> /*
>  * {read,write}{b,w,l,q}() access little endian memory and return result in
>  * native endianness.
>  */
There is readl() (and family) function. For most hosts it returns native
endian value (because most systems LE nowadays).
I don't know all context, and don't know exactly why... but for MIPS readl()
returns value in native endianess.
Reference - https://elixir.bootlin.com/linux/v6.19.10/source/arch/mips/include/asm/io.h

So we are in interesting situation, when readl() returns native-endian values (BE in our case),
readl_be() returns BE too, because cpu_to_be32() does nothing on BE system, ioread32() returns BE,
but ioread32be() returns LE because of unconditional swab32(). Using *be
to get LE value... kinda weird. That's why we ended up with these two
functions.

Not a perfect solution, so it would be good to hear others opinions and
come to a more proper solution.
UPD. regmap looks like a pretty good solution for endianess stuff, but
it seems to interfere with current register handling (using struct
offsets as register addresses). So it has to be a big work.

For example, Realtek solves this issue that way -
https://github.com/jameywine/GPL-for-GP3000/blob/5090fbcb6ba743dd7b1314811ef557bad0460147/linux-5.10.x/drivers/usb/host/ehci.h#L742
> 
> and yes, your patch does trigger sparse warnings:
> ../drivers/phy/realtek/phy-rtk-usb2.c:153:16: warning: cast to restricted __le32
> ../drivers/phy/realtek/phy-rtk-usb2.c:163:16: warning: incorrect type in argument 1 (different base types)
> ../drivers/phy/realtek/phy-rtk-usb2.c:163:16:    expected unsigned int val
> ../drivers/phy/realtek/phy-rtk-usb2.c:163:16:    got restricted __le32 [usertype]
> 
> Furthermore, please drop the 'inline' keyword from C files and let the
> compiler decide. Your use of this keyword has no value - you declare
> phy_read(), phy_read_le() etc as inline but then assign function
> pointers to them. How can the compiler inline the indirect calls?

Thanks for all other review comments, going to be fixed in next patch
series version.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 2/6] phy: realtek: usb2: introduce read and write functions to driver data

[Michael Zavertkin]

On Tue, Mar 31, 2026 at 12:32:27AM +0300, Vladimir Oltean wrote:
> On Tue, Mar 31, 2026 at 12:19:18AM +0300, Vladimir Oltean wrote:
> > On Fri, Mar 27, 2026 at 09:06:34PM +0500, Rustam Adilov wrote:
> > > +static inline u32 phy_read(void __iomem *reg)
> > > +{
> > > +	return readl(reg);
> > > +}
> > > +
> > > +static inline u32 phy_read_le(void __iomem *reg)
> > > +{
> > > +	return le32_to_cpu(readl(reg));
> > > +}
> > > +
> > > +static inline void phy_write(u32 val, void __iomem *reg)
> > > +{
> > > +	writel(val, reg);
> > > +}
> > > +
> > > +static inline void phy_write_le(u32 val, void __iomem *reg)
> > > +{
> > > +	writel(cpu_to_le32(val), reg);
> > > +}
> > 
> > Please don't name driver-level functions phy_read() and phy_write().
> > That will collide with networking API functions of the same name and
> > will make grep-based code searching more difficult.
> > 
> > Also, have you looked at regmap? It has native support for endianness;
> > it supports regmap_field_read()/regmap_field_write() for abstracting
> > registers which may be found at different places for different HW;
> > it offers regmap_read_poll_timeout() so you don't have to pass the
> > function pointer to utmi_wait_register(). It seems the result would be a
> > bit more elegant.
> 
> Even if you decide not to use regmap. I thought I should let you know
> that LLM review says:
> 
>   Are these double byte-swaps intentional?
> 
>   Since readl() and writel() inherently perform little-endian memory accesses
>   and handle byte-swapping on big-endian architectures automatically, won't
>   wrapping them in le32_to_cpu() and cpu_to_le32() apply a second, redundant
>   byte-swap?
> 
>   On big-endian systems, wouldn't these double swaps cancel each other out
>   and result in a native big-endian access instead of the intended
>   little-endian access? If the SoC bus bridge implicitly swaps and requires
>   a native access, should __raw_readl() and __raw_writel() (or ioread32be /
>   iowrite32be) be used instead to avoid obfuscating it with double-swaps?
> 
>   Also, does passing the __le32 restricted type returned by cpu_to_le32()
>   into writel() (which expects a native u32) trigger Sparse static analysis
>   warnings for an incorrect type in argument?
> 
> For reference:
> https://elixir.bootlin.com/linux/v6.19.10/source/include/asm-generic/io.h#L184
> /*
>  * {read,write}{b,w,l,q}() access little endian memory and return result in
>  * native endianness.
>  */
There is readl() (and family) function. For most hosts it returns native
endian value (because most systems LE nowadays).
I don't know all context, and don't know exactly why... but for MIPS readl()
returns value in native endianess.
Reference - https://elixir.bootlin.com/linux/v6.19.10/source/arch/mips/include/asm/io.h

So we are in interesting situation, when readl() returns native-endian values (BE in our case),
readl_be() returns BE too, because cpu_to_be32() does nothing on BE system, ioread32() returns BE,
but ioread32be() returns LE because of unconditional swab32(). Using *be
to get LE value... kinda weird. That's why we ended up with these two
functions.

Not a perfect solution, so it would be good to hear others opinions and
come to a more proper solution.
UPD. regmap looks like a pretty good solution for endianess stuff, but
it seems to interfere with current register handling (using struct
offsets as register addresses). So it has to be a big work.

For example, Realtek solves this issue that way -
https://github.com/jameywine/GPL-for-GP3000/blob/5090fbcb6ba743dd7b1314811ef557bad0460147/linux-5.10.x/drivers/usb/host/ehci.h#L742
> 
> and yes, your patch does trigger sparse warnings:
> ../drivers/phy/realtek/phy-rtk-usb2.c:153:16: warning: cast to restricted __le32
> ../drivers/phy/realtek/phy-rtk-usb2.c:163:16: warning: incorrect type in argument 1 (different base types)
> ../drivers/phy/realtek/phy-rtk-usb2.c:163:16:    expected unsigned int val
> ../drivers/phy/realtek/phy-rtk-usb2.c:163:16:    got restricted __le32 [usertype]
> 
> Furthermore, please drop the 'inline' keyword from C files and let the
> compiler decide. Your use of this keyword has no value - you declare
> phy_read(), phy_read_le() etc as inline but then assign function
> pointers to them. How can the compiler inline the indirect calls?

Thanks for all other review comments, going to be fixed in next patch
series version.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 2/6] phy: realtek: usb2: introduce read and write functions to driver data

[Rustam Adilov]

Hello,
On 2026-03-30 21:19, Vladimir Oltean wrote:
> On Fri, Mar 27, 2026 at 09:06:34PM +0500, Rustam Adilov wrote:
>> +static inline u32 phy_read(void __iomem *reg)
>> +{
>> +	return readl(reg);
>> +}
>> +
>> +static inline u32 phy_read_le(void __iomem *reg)
>> +{
>> +	return le32_to_cpu(readl(reg));
>> +}
>> +
>> +static inline void phy_write(u32 val, void __iomem *reg)
>> +{
>> +	writel(val, reg);
>> +}
>> +
>> +static inline void phy_write_le(u32 val, void __iomem *reg)
>> +{
>> +	writel(cpu_to_le32(val), reg);
>> +}
> 
> Please don't name driver-level functions phy_read() and phy_write().
> That will collide with networking API functions of the same name and
> will make grep-based code searching more difficult.

I can change it to something like "rtk_phy_read" or "usb2phy_read" then.

> Also, have you looked at regmap? It has native support for endianness;
> it supports regmap_field_read()/regmap_field_write() for abstracting
> registers which may be found at different places for different HW;
> it offers regmap_read_poll_timeout() so you don't have to pass the
> function pointer to utmi_wait_register(). It seems the result would be a
> bit more elegant.

In fact, I did not because it would involve in way more refactoring for patch 
series that is supposed to simply add RTL9607C support. And unfortunately, 
the regmap is not going to the solve the issue, which i will explain in the
later email to your LLM review on readl/writel.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v2 3/6] dt-bindings: phy: realtek,usb2phy.yaml: extend for resets and RTL9607C support

[Rustam Adilov]

Add the "realtek,rtl9607-usb2phy" compatible for USB2 PHY on the RTL9607C
SoC series.

Add a resets property to properties to describe the usb2phy reset line.

In RTL9607C, USB2 PHY reset line is from "IP Enable controller" which is
multipurpose and handle activating various SoC peripherals.

It is unclear whether RTD SoCs have something similar to that so set
the resets to false for these devices.

RTL9607C requires the "resets" to be specified so add the corresponding
if check for the "realtek,rtl9607-usb2phy" compatible.

Signed-off-by: Rustam Adilov <adilov@disroot.org>
---
 .../bindings/phy/realtek,usb2phy.yaml         | 25 ++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml b/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
index 9911ada39ee7..7b50833c8e19 100644
--- a/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
+++ b/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
@@ -11,7 +11,8 @@ maintainers:
   - Stanley Chang <stanley_chang@realtek.com>
 
 description: |
-  Realtek USB 2.0 PHY support the digital home center (DHC) RTD series SoCs.
+  Realtek USB 2.0 PHY support the digital home center (DHC) RTD and
+  RTL9607C series SoCs.
   The USB 2.0 PHY driver is designed to support the XHCI controller. The SoCs
   support multiple XHCI controllers. One PHY device node maps to one XHCI
   controller.
@@ -57,6 +58,12 @@ description: |
   XHCI controller#1 -- usb2phy -- phy#0
   XHCI controller#2 -- usb2phy -- phy#0
 
+  RTL9607C SoCs USB
+  The USB architecture includes OHCI and EHCI controllers.
+  Both of them map to one USB2.0 PHY.
+  OHCI controller#0 -- usb2phy -- phy#0
+  EHCI controller#0 -- usb2phy -- phy#0
+
 properties:
   compatible:
     enum:
@@ -69,6 +76,7 @@ properties:
       - realtek,rtd1395-usb2phy-2port
       - realtek,rtd1619-usb2phy
       - realtek,rtd1619b-usb2phy
+      - realtek,rtl9607-usb2phy
 
   reg:
     items:
@@ -130,6 +138,9 @@ properties:
     minimum: -8
     maximum: 8
 
+  resets:
+    maxItems: 1
+
 required:
   - compatible
   - reg
@@ -157,6 +168,18 @@ allOf:
     then:
       properties:
         realtek,driving-level-compensate: false
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - realtek,rtl9607-usb2phy
+    then:
+      required:
+        - resets
+    else:
+      properties:
+        resets: false
 
 additionalProperties: false
 
-- 
2.53.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v2 4/6] phy: realtek: usb2: introduce reset controller struct

[Rustam Adilov]

In RTL9607C, there is so called "IP Enable Controller" which resemble
reset controller with reset lines and is used for various things like
USB, PCIE, GMAC and such.

Introduce the reset_control struct to this driver to handle deasserting
usb2 phy reset line.

Make use of the function devm_reset_control_array_get_optional_exclusive()
function to get the reset controller and since existing RTD SoCs don't
specify the resets we can have a cleaner code.

Co-developed-by: Michael Zavertkin <misha.zavertkin@mail.ru>
Signed-off-by: Michael Zavertkin <misha.zavertkin@mail.ru>
Signed-off-by: Rustam Adilov <adilov@disroot.org>
---
 drivers/phy/realtek/phy-rtk-usb2.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
index e65b8525b88b..070cba1e0e0a 100644
--- a/drivers/phy/realtek/phy-rtk-usb2.c
+++ b/drivers/phy/realtek/phy-rtk-usb2.c
@@ -17,6 +17,7 @@
 #include <linux/sys_soc.h>
 #include <linux/mfd/syscon.h>
 #include <linux/phy/phy.h>
+#include <linux/reset.h>
 #include <linux/usb.h>
 
 /* GUSB2PHYACCn register */
@@ -130,6 +131,7 @@ struct rtk_phy {
 	struct phy_cfg *phy_cfg;
 	int num_phy;
 	struct phy_parameter *phy_parameter;
+	struct reset_control *phy_rst;
 
 	struct dentry *debug_dir;
 };
@@ -602,6 +604,10 @@ static int do_rtk_phy_init(struct rtk_phy *rtk_phy, int index)
 	phy_parameter = &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
 	phy_reg = &phy_parameter->phy_reg;
 
+	reset_control_deassert(rtk_phy->phy_rst);
+
+	mdelay(5);
+
 	if (phy_cfg->use_default_parameter) {
 		dev_dbg(rtk_phy->dev, "%s phy#%d use default parameter\n",
 			__func__, index);
@@ -1069,6 +1075,12 @@ static int rtk_usb2phy_probe(struct platform_device *pdev)
 
 	rtk_phy->num_phy = phy_cfg->num_phy;
 
+	rtk_phy->phy_rst = devm_reset_control_array_get_optional_exclusive(dev);
+	if (IS_ERR(rtk_phy->phy_rst)) {
+		dev_err(dev, "usb2 phy resets are not working\n");
+		return PTR_ERR(rtk_phy->phy_rst);
+	}
+
 	ret = parse_phy_data(rtk_phy);
 	if (ret)
 		goto err;
-- 
2.53.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 4/6] phy: realtek: usb2: introduce reset controller struct

[Vladimir Oltean]

On Fri, Mar 27, 2026 at 09:06:36PM +0500, Rustam Adilov wrote:
> In RTL9607C, there is so called "IP Enable Controller" which resemble
> reset controller with reset lines and is used for various things like
> USB, PCIE, GMAC and such.
> 
> Introduce the reset_control struct to this driver to handle deasserting
> usb2 phy reset line.
> 
> Make use of the function devm_reset_control_array_get_optional_exclusive()
> function to get the reset controller and since existing RTD SoCs don't
> specify the resets we can have a cleaner code.
> 
> Co-developed-by: Michael Zavertkin <misha.zavertkin@mail.ru>
> Signed-off-by: Michael Zavertkin <misha.zavertkin@mail.ru>
> Signed-off-by: Rustam Adilov <adilov@disroot.org>
> ---
>  drivers/phy/realtek/phy-rtk-usb2.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
> index e65b8525b88b..070cba1e0e0a 100644
> --- a/drivers/phy/realtek/phy-rtk-usb2.c
> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
> @@ -17,6 +17,7 @@
>  #include <linux/sys_soc.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/phy/phy.h>
> +#include <linux/reset.h>
>  #include <linux/usb.h>
>  
>  /* GUSB2PHYACCn register */
> @@ -130,6 +131,7 @@ struct rtk_phy {
>  	struct phy_cfg *phy_cfg;
>  	int num_phy;
>  	struct phy_parameter *phy_parameter;
> +	struct reset_control *phy_rst;
>  
>  	struct dentry *debug_dir;
>  };
> @@ -602,6 +604,10 @@ static int do_rtk_phy_init(struct rtk_phy *rtk_phy, int index)
>  	phy_parameter = &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
>  	phy_reg = &phy_parameter->phy_reg;
>  
> +	reset_control_deassert(rtk_phy->phy_rst);

LLM review says:

(less important)
Can reset_control_deassert() fail here? If there is a hardware communication
error with the reset controller, should this check the return value and
propagate the error up instead of proceeding to configure the PHY?
Additionally, since the exclusive reset line is deasserted here, does this
code need a corresponding reset_control_assert() in the driver's teardown
or exit path? Leaving the IP block permanently enabled after shutdown could
lead to power leaks and prevent proper hardware re-initialization.

> +
> +	mdelay(5);

(more important)
This code unnecessarily penalizes existing platforms. If rtk_phy->phy_rst
is NULL (as on older platforms where the optional reset is not defined), the
delay still executes.

Also, since PHY initialization callbacks run in a sleepable context, would it
be better to use a sleep-based delay like usleep_range(5000, 6000) to yield
the CPU instead of busy-waiting with mdelay(5)?

> +
>  	if (phy_cfg->use_default_parameter) {
>  		dev_dbg(rtk_phy->dev, "%s phy#%d use default parameter\n",
>  			__func__, index);
> @@ -1069,6 +1075,12 @@ static int rtk_usb2phy_probe(struct platform_device *pdev)
>  
>  	rtk_phy->num_phy = phy_cfg->num_phy;
>  
> +	rtk_phy->phy_rst = devm_reset_control_array_get_optional_exclusive(dev);
> +	if (IS_ERR(rtk_phy->phy_rst)) {
> +		dev_err(dev, "usb2 phy resets are not working\n");
> +		return PTR_ERR(rtk_phy->phy_rst);
> +	}
> +

(still LLM review)
If the reset controller driver is not yet ready, this will return
-EPROBE_DEFER and print an error message to the kernel log.
Should this use dev_err_probe() to silently handle probe deferral while
correctly logging actual errors?

>  	ret = parse_phy_data(rtk_phy);
>  	if (ret)
>  		goto err;
> -- 
> 2.53.0
> 
> 


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 4/6] phy: realtek: usb2: introduce reset controller struct

[Rustam Adilov]

On 2026-03-30 21:39, Vladimir Oltean wrote:
> On Fri, Mar 27, 2026 at 09:06:36PM +0500, Rustam Adilov wrote:
>> In RTL9607C, there is so called "IP Enable Controller" which resemble
>> reset controller with reset lines and is used for various things like
>> USB, PCIE, GMAC and such.
>> 
>> Introduce the reset_control struct to this driver to handle deasserting
>> usb2 phy reset line.
>> 
>> Make use of the function devm_reset_control_array_get_optional_exclusive()
>> function to get the reset controller and since existing RTD SoCs don't
>> specify the resets we can have a cleaner code.
>> 
>> Co-developed-by: Michael Zavertkin <misha.zavertkin@mail.ru>
>> Signed-off-by: Michael Zavertkin <misha.zavertkin@mail.ru>
>> Signed-off-by: Rustam Adilov <adilov@disroot.org>
>> ---
>>  drivers/phy/realtek/phy-rtk-usb2.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>> 
>> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
>> index e65b8525b88b..070cba1e0e0a 100644
>> --- a/drivers/phy/realtek/phy-rtk-usb2.c
>> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
>> @@ -17,6 +17,7 @@
>>  #include <linux/sys_soc.h>
>>  #include <linux/mfd/syscon.h>
>>  #include <linux/phy/phy.h>
>> +#include <linux/reset.h>
>>  #include <linux/usb.h>
>>  
>>  /* GUSB2PHYACCn register */
>> @@ -130,6 +131,7 @@ struct rtk_phy {
>>  	struct phy_cfg *phy_cfg;
>>  	int num_phy;
>>  	struct phy_parameter *phy_parameter;
>> +	struct reset_control *phy_rst;
>>  
>>  	struct dentry *debug_dir;
>>  };
>> @@ -602,6 +604,10 @@ static int do_rtk_phy_init(struct rtk_phy *rtk_phy, int index)
>>  	phy_parameter = &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
>>  	phy_reg = &phy_parameter->phy_reg;
>>  
>> +	reset_control_deassert(rtk_phy->phy_rst);
> 
> LLM review says:
> 
> (less important)
> Can reset_control_deassert() fail here? If there is a hardware communication
> error with the reset controller, should this check the return value and
> propagate the error up instead of proceeding to configure the PHY?
> Additionally, since the exclusive reset line is deasserted here, does this
> code need a corresponding reset_control_assert() in the driver's teardown
> or exit path? Leaving the IP block permanently enabled after shutdown could
> lead to power leaks and prevent proper hardware re-initialization.

It realistically shouldn't fail. But I can add the return error for this.
And no, it doesn't need the reset_control_assert.

>> +
>> +	mdelay(5);
> 
> (more important)
> This code unnecessarily penalizes existing platforms. If rtk_phy->phy_rst
> is NULL (as on older platforms where the optional reset is not defined), the
> delay still executes.
> 
> Also, since PHY initialization callbacks run in a sleepable context, would it
> be better to use a sleep-based delay like usleep_range(5000, 6000) to yield
> the CPU instead of busy-waiting with mdelay(5)?

I can change mdelay to msleep and wrap it around something like if (rtk_phy->phy_rst).

>> +
>>  	if (phy_cfg->use_default_parameter) {
>>  		dev_dbg(rtk_phy->dev, "%s phy#%d use default parameter\n",
>>  			__func__, index);
>> @@ -1069,6 +1075,12 @@ static int rtk_usb2phy_probe(struct platform_device *pdev)
>>  
>>  	rtk_phy->num_phy = phy_cfg->num_phy;
>>  
>> +	rtk_phy->phy_rst = devm_reset_control_array_get_optional_exclusive(dev);
>> +	if (IS_ERR(rtk_phy->phy_rst)) {
>> +		dev_err(dev, "usb2 phy resets are not working\n");
>> +		return PTR_ERR(rtk_phy->phy_rst);
>> +	}
>> +
> 
> (still LLM review)
> If the reset controller driver is not yet ready, this will return
> -EPROBE_DEFER and print an error message to the kernel log.
> Should this use dev_err_probe() to silently handle probe deferral while
> correctly logging actual errors?

I can change it to dev_err_probe, no problem with that.

>>  	ret = parse_phy_data(rtk_phy);
>>  	if (ret)
>>  		goto err;
>> -- 
>> 2.53.0
>> 
>>

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v2 5/6] phy: realtek: usb2: add support for RTL9607C USB2 PHY

[Rustam Adilov]

Add support for the usb2 phy of RTL9607C series based SoCs.
Add the macros and phy config struct for rtl9607.

RTL9607C requires to clear a "force host disconnect" bit in the
specific register (which is at an offset from reg_wrap_vstatus)
before proceeding with phy parameter writes.

Add the bool variable to the driver data struct and hide this whole
procedure under the if statement that checks this new variable.

Co-developed-by: Michael Zavertkin <misha.zavertkin@mail.ru>
Signed-off-by: Michael Zavertkin <misha.zavertkin@mail.ru>
Signed-off-by: Rustam Adilov <adilov@disroot.org>
---
 drivers/phy/realtek/phy-rtk-usb2.c | 57 ++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
index 070cba1e0e0a..bf22d12681dc 100644
--- a/drivers/phy/realtek/phy-rtk-usb2.c
+++ b/drivers/phy/realtek/phy-rtk-usb2.c
@@ -26,6 +26,12 @@
 #define PHY_VCTRL_SHIFT 8
 #define PHY_REG_DATA_MASK 0xff
 
+#define PHY_9607_VSTS_BUSY BIT(17)
+#define PHY_9607_NEW_REG_REQ BIT(13)
+
+#define PHY_9607_FORCE_DISCONNECT_REG 0x10
+#define PHY_9607_FORCE_DISCONNECT_BIT BIT(5)
+
 #define GET_LOW_NIBBLE(addr) ((addr) & 0x0f)
 #define GET_HIGH_NIBBLE(addr) (((addr) & 0xf0) >> 4)
 
@@ -109,6 +115,7 @@ struct phy_cfg {
 
 	u32 (*read)(void __iomem *reg);
 	void (*write)(u32 val, void __iomem *reg);
+	bool force_host_disconnect;
 };
 
 struct phy_parameter {
@@ -614,6 +621,16 @@ static int do_rtk_phy_init(struct rtk_phy *rtk_phy, int index)
 		goto do_toggle;
 	}
 
+	if (phy_cfg->force_host_disconnect) {
+		/* disable force-host-disconnect */
+		u32 temp = readl(phy_reg->reg_wrap_vstatus + PHY_9607_FORCE_DISCONNECT_REG);
+
+		temp &= ~PHY_9607_FORCE_DISCONNECT_BIT;
+		writel(temp, phy_reg->reg_wrap_vstatus + PHY_9607_FORCE_DISCONNECT_REG);
+
+		mdelay(10);
+	}
+
 	/* Set page 0 */
 	phy_data_page = phy_cfg->page0;
 	rtk_phy_set_page(phy_reg, 0);
@@ -1141,6 +1158,7 @@ static const struct phy_cfg rtd1295_phy_cfg = {
 	.new_reg_req = PHY_NEW_REG_REQ,
 	.read = phy_read,
 	.write = phy_write,
+	.force_host_disconnect = false,
 };
 
 static const struct phy_cfg rtd1395_phy_cfg = {
@@ -1170,6 +1188,7 @@ static const struct phy_cfg rtd1395_phy_cfg = {
 	.new_reg_req = PHY_NEW_REG_REQ,
 	.read = phy_read,
 	.write = phy_write,
+	.force_host_disconnect = false,
 };
 
 static const struct phy_cfg rtd1395_phy_cfg_2port = {
@@ -1199,6 +1218,7 @@ static const struct phy_cfg rtd1395_phy_cfg_2port = {
 	.new_reg_req = PHY_NEW_REG_REQ,
 	.read = phy_read,
 	.write = phy_write,
+	.force_host_disconnect = false,
 };
 
 static const struct phy_cfg rtd1619_phy_cfg = {
@@ -1226,6 +1246,7 @@ static const struct phy_cfg rtd1619_phy_cfg = {
 	.new_reg_req = PHY_NEW_REG_REQ,
 	.read = phy_read,
 	.write = phy_write,
+	.force_host_disconnect = false,
 };
 
 static const struct phy_cfg rtd1319_phy_cfg = {
@@ -1257,6 +1278,7 @@ static const struct phy_cfg rtd1319_phy_cfg = {
 	.new_reg_req = PHY_NEW_REG_REQ,
 	.read = phy_read,
 	.write = phy_write,
+	.force_host_disconnect = false,
 };
 
 static const struct phy_cfg rtd1312c_phy_cfg = {
@@ -1287,6 +1309,7 @@ static const struct phy_cfg rtd1312c_phy_cfg = {
 	.new_reg_req = PHY_NEW_REG_REQ,
 	.read = phy_read,
 	.write = phy_write,
+	.force_host_disconnect = false,
 };
 
 static const struct phy_cfg rtd1619b_phy_cfg = {
@@ -1317,6 +1340,7 @@ static const struct phy_cfg rtd1619b_phy_cfg = {
 	.new_reg_req = PHY_NEW_REG_REQ,
 	.read = phy_read,
 	.write = phy_write,
+	.force_host_disconnect = false,
 };
 
 static const struct phy_cfg rtd1319d_phy_cfg = {
@@ -1347,6 +1371,7 @@ static const struct phy_cfg rtd1319d_phy_cfg = {
 	.new_reg_req = PHY_NEW_REG_REQ,
 	.read = phy_read,
 	.write = phy_write,
+	.force_host_disconnect = false,
 };
 
 static const struct phy_cfg rtd1315e_phy_cfg = {
@@ -1378,6 +1403,37 @@ static const struct phy_cfg rtd1315e_phy_cfg = {
 	.new_reg_req = PHY_NEW_REG_REQ,
 	.read = phy_read,
 	.write = phy_write,
+	.force_host_disconnect = false,
+};
+
+static const struct phy_cfg rtl9607_phy_cfg = {
+	.page0_size = MAX_USB_PHY_PAGE0_DATA_SIZE,
+	.page0 = { [0] = {0xe0, 0x95},
+		   [4] = {0xe4, 0x6a},
+		  [12] = {0xf3, 0x31}, },
+	.page1_size = MAX_USB_PHY_PAGE1_DATA_SIZE,
+	.page1 = { [0] = {0xe0, 0x26}, },
+	.page2_size = MAX_USB_PHY_PAGE2_DATA_SIZE,
+	.page2 = { [7] = {0xe7, 0x33}, },
+	.num_phy = 1,
+	.check_efuse = false,
+	.check_efuse_version = CHECK_EFUSE_V2,
+	.efuse_dc_driving_rate = EFUS_USB_DC_CAL_RATE,
+	.dc_driving_mask = 0x1f,
+	.efuse_dc_disconnect_rate = EFUS_USB_DC_DIS_RATE,
+	.dc_disconnect_mask = 0xf,
+	.usb_dc_disconnect_at_page0 = true,
+	.do_toggle = true,
+	.do_toggle_driving = false,
+	.driving_updated_for_dev_dis = 0x8,
+	.use_default_parameter = false,
+	.is_double_sensitivity_mode = true,
+	.vstatus_offset = 0xc,
+	.vstatus_busy = PHY_9607_VSTS_BUSY,
+	.new_reg_req = PHY_9607_NEW_REG_REQ,
+	.read = phy_read_le,
+	.write = phy_write_le,
+	.force_host_disconnect = true,
 };
 
 static const struct of_device_id usbphy_rtk_dt_match[] = {
@@ -1390,6 +1446,7 @@ static const struct of_device_id usbphy_rtk_dt_match[] = {
 	{ .compatible = "realtek,rtd1395-usb2phy-2port", .data = &rtd1395_phy_cfg_2port },
 	{ .compatible = "realtek,rtd1619-usb2phy", .data = &rtd1619_phy_cfg },
 	{ .compatible = "realtek,rtd1619b-usb2phy", .data = &rtd1619b_phy_cfg },
+	{ .compatible = "realtek,rtl9607-usb2phy", .data = &rtl9607_phy_cfg },
 	{},
 };
 MODULE_DEVICE_TABLE(of, usbphy_rtk_dt_match);
-- 
2.53.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 5/6] phy: realtek: usb2: add support for RTL9607C USB2 PHY

[Vladimir Oltean]

On Fri, Mar 27, 2026 at 09:06:37PM +0500, Rustam Adilov wrote:
> Add support for the usb2 phy of RTL9607C series based SoCs.
> Add the macros and phy config struct for rtl9607.
> 
> RTL9607C requires to clear a "force host disconnect" bit in the
> specific register (which is at an offset from reg_wrap_vstatus)
> before proceeding with phy parameter writes.
> 
> Add the bool variable to the driver data struct and hide this whole
> procedure under the if statement that checks this new variable.
> 
> Co-developed-by: Michael Zavertkin <misha.zavertkin@mail.ru>
> Signed-off-by: Michael Zavertkin <misha.zavertkin@mail.ru>
> Signed-off-by: Rustam Adilov <adilov@disroot.org>
> ---
>  drivers/phy/realtek/phy-rtk-usb2.c | 57 ++++++++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
> 
> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
> index 070cba1e0e0a..bf22d12681dc 100644
> --- a/drivers/phy/realtek/phy-rtk-usb2.c
> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
> @@ -26,6 +26,12 @@
>  #define PHY_VCTRL_SHIFT 8
>  #define PHY_REG_DATA_MASK 0xff
>  
> +#define PHY_9607_VSTS_BUSY BIT(17)
> +#define PHY_9607_NEW_REG_REQ BIT(13)
> +
> +#define PHY_9607_FORCE_DISCONNECT_REG 0x10
> +#define PHY_9607_FORCE_DISCONNECT_BIT BIT(5)
> +
>  #define GET_LOW_NIBBLE(addr) ((addr) & 0x0f)
>  #define GET_HIGH_NIBBLE(addr) (((addr) & 0xf0) >> 4)
>  
> @@ -109,6 +115,7 @@ struct phy_cfg {
>  
>  	u32 (*read)(void __iomem *reg);
>  	void (*write)(u32 val, void __iomem *reg);
> +	bool force_host_disconnect;
>  };
>  
>  struct phy_parameter {
> @@ -614,6 +621,16 @@ static int do_rtk_phy_init(struct rtk_phy *rtk_phy, int index)
>  		goto do_toggle;
>  	}
>  
> +	if (phy_cfg->force_host_disconnect) {
> +		/* disable force-host-disconnect */
> +		u32 temp = readl(phy_reg->reg_wrap_vstatus + PHY_9607_FORCE_DISCONNECT_REG);
> +
> +		temp &= ~PHY_9607_FORCE_DISCONNECT_BIT;
> +		writel(temp, phy_reg->reg_wrap_vstatus + PHY_9607_FORCE_DISCONNECT_REG);
> +
> +		mdelay(10);

LLM review:

Could we use msleep(10) or usleep_range(10000, 11000) here instead of
mdelay(10)?
Since do_rtk_phy_init() executes as part of the phy_ops->init callback
with a mutex held from a sleepable process context, spinning the CPU for
10ms wastes CPU resources and increases scheduling latency.

> +	}
> +
>  	/* Set page 0 */
>  	phy_data_page = phy_cfg->page0;
>  	rtk_phy_set_page(phy_reg, 0);
> @@ -1141,6 +1158,7 @@ static const struct phy_cfg rtd1295_phy_cfg = {
>  	.new_reg_req = PHY_NEW_REG_REQ,
>  	.read = phy_read,
>  	.write = phy_write,
> +	.force_host_disconnect = false,

You don't need to initialize rodata struct fields with false/0/NULL.

>  };
>  
>  static const struct phy_cfg rtd1395_phy_cfg = {
> @@ -1170,6 +1188,7 @@ static const struct phy_cfg rtd1395_phy_cfg = {
>  	.new_reg_req = PHY_NEW_REG_REQ,
>  	.read = phy_read,
>  	.write = phy_write,
> +	.force_host_disconnect = false,
>  };
>  
>  static const struct phy_cfg rtd1395_phy_cfg_2port = {
> @@ -1199,6 +1218,7 @@ static const struct phy_cfg rtd1395_phy_cfg_2port = {
>  	.new_reg_req = PHY_NEW_REG_REQ,
>  	.read = phy_read,
>  	.write = phy_write,
> +	.force_host_disconnect = false,
>  };
>  
>  static const struct phy_cfg rtd1619_phy_cfg = {
> @@ -1226,6 +1246,7 @@ static const struct phy_cfg rtd1619_phy_cfg = {
>  	.new_reg_req = PHY_NEW_REG_REQ,
>  	.read = phy_read,
>  	.write = phy_write,
> +	.force_host_disconnect = false,
>  };
>  
>  static const struct phy_cfg rtd1319_phy_cfg = {
> @@ -1257,6 +1278,7 @@ static const struct phy_cfg rtd1319_phy_cfg = {
>  	.new_reg_req = PHY_NEW_REG_REQ,
>  	.read = phy_read,
>  	.write = phy_write,
> +	.force_host_disconnect = false,
>  };
>  
>  static const struct phy_cfg rtd1312c_phy_cfg = {
> @@ -1287,6 +1309,7 @@ static const struct phy_cfg rtd1312c_phy_cfg = {
>  	.new_reg_req = PHY_NEW_REG_REQ,
>  	.read = phy_read,
>  	.write = phy_write,
> +	.force_host_disconnect = false,
>  };
>  
>  static const struct phy_cfg rtd1619b_phy_cfg = {
> @@ -1317,6 +1340,7 @@ static const struct phy_cfg rtd1619b_phy_cfg = {
>  	.new_reg_req = PHY_NEW_REG_REQ,
>  	.read = phy_read,
>  	.write = phy_write,
> +	.force_host_disconnect = false,
>  };
>  
>  static const struct phy_cfg rtd1319d_phy_cfg = {
> @@ -1347,6 +1371,7 @@ static const struct phy_cfg rtd1319d_phy_cfg = {
>  	.new_reg_req = PHY_NEW_REG_REQ,
>  	.read = phy_read,
>  	.write = phy_write,
> +	.force_host_disconnect = false,
>  };
>  
>  static const struct phy_cfg rtd1315e_phy_cfg = {
> @@ -1378,6 +1403,37 @@ static const struct phy_cfg rtd1315e_phy_cfg = {
>  	.new_reg_req = PHY_NEW_REG_REQ,
>  	.read = phy_read,
>  	.write = phy_write,
> +	.force_host_disconnect = false,
> +};
> +
> +static const struct phy_cfg rtl9607_phy_cfg = {
> +	.page0_size = MAX_USB_PHY_PAGE0_DATA_SIZE,
> +	.page0 = { [0] = {0xe0, 0x95},
> +		   [4] = {0xe4, 0x6a},
> +		  [12] = {0xf3, 0x31}, },
> +	.page1_size = MAX_USB_PHY_PAGE1_DATA_SIZE,
> +	.page1 = { [0] = {0xe0, 0x26}, },
> +	.page2_size = MAX_USB_PHY_PAGE2_DATA_SIZE,
> +	.page2 = { [7] = {0xe7, 0x33}, },
> +	.num_phy = 1,
> +	.check_efuse = false,

Similar for these (+do_toggle_driving, use_default_parameter).

> +	.check_efuse_version = CHECK_EFUSE_V2,
> +	.efuse_dc_driving_rate = EFUS_USB_DC_CAL_RATE,
> +	.dc_driving_mask = 0x1f,
> +	.efuse_dc_disconnect_rate = EFUS_USB_DC_DIS_RATE,
> +	.dc_disconnect_mask = 0xf,
> +	.usb_dc_disconnect_at_page0 = true,
> +	.do_toggle = true,
> +	.do_toggle_driving = false,
> +	.driving_updated_for_dev_dis = 0x8,
> +	.use_default_parameter = false,
> +	.is_double_sensitivity_mode = true,
> +	.vstatus_offset = 0xc,
> +	.vstatus_busy = PHY_9607_VSTS_BUSY,
> +	.new_reg_req = PHY_9607_NEW_REG_REQ,
> +	.read = phy_read_le,
> +	.write = phy_write_le,
> +	.force_host_disconnect = true,
>  };
>  
>  static const struct of_device_id usbphy_rtk_dt_match[] = {
> @@ -1390,6 +1446,7 @@ static const struct of_device_id usbphy_rtk_dt_match[] = {
>  	{ .compatible = "realtek,rtd1395-usb2phy-2port", .data = &rtd1395_phy_cfg_2port },
>  	{ .compatible = "realtek,rtd1619-usb2phy", .data = &rtd1619_phy_cfg },
>  	{ .compatible = "realtek,rtd1619b-usb2phy", .data = &rtd1619b_phy_cfg },
> +	{ .compatible = "realtek,rtl9607-usb2phy", .data = &rtl9607_phy_cfg },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, usbphy_rtk_dt_match);
> -- 
> 2.53.0
> 
> 


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 5/6] phy: realtek: usb2: add support for RTL9607C USB2 PHY

[Rustam Adilov]

On 2026-03-30 21:50, Vladimir Oltean wrote:
> On Fri, Mar 27, 2026 at 09:06:37PM +0500, Rustam Adilov wrote:
>> Add support for the usb2 phy of RTL9607C series based SoCs.
>> Add the macros and phy config struct for rtl9607.
>> 
>> RTL9607C requires to clear a "force host disconnect" bit in the
>> specific register (which is at an offset from reg_wrap_vstatus)
>> before proceeding with phy parameter writes.
>> 
>> Add the bool variable to the driver data struct and hide this whole
>> procedure under the if statement that checks this new variable.
>> 
>> Co-developed-by: Michael Zavertkin <misha.zavertkin@mail.ru>
>> Signed-off-by: Michael Zavertkin <misha.zavertkin@mail.ru>
>> Signed-off-by: Rustam Adilov <adilov@disroot.org>
>> ---
>>  drivers/phy/realtek/phy-rtk-usb2.c | 57 ++++++++++++++++++++++++++++++
>>  1 file changed, 57 insertions(+)
>> 
>> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
>> index 070cba1e0e0a..bf22d12681dc 100644
>> --- a/drivers/phy/realtek/phy-rtk-usb2.c
>> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
>> @@ -26,6 +26,12 @@
>>  #define PHY_VCTRL_SHIFT 8
>>  #define PHY_REG_DATA_MASK 0xff
>>  
>> +#define PHY_9607_VSTS_BUSY BIT(17)
>> +#define PHY_9607_NEW_REG_REQ BIT(13)
>> +
>> +#define PHY_9607_FORCE_DISCONNECT_REG 0x10
>> +#define PHY_9607_FORCE_DISCONNECT_BIT BIT(5)
>> +
>>  #define GET_LOW_NIBBLE(addr) ((addr) & 0x0f)
>>  #define GET_HIGH_NIBBLE(addr) (((addr) & 0xf0) >> 4)
>>  
>> @@ -109,6 +115,7 @@ struct phy_cfg {
>>  
>>  	u32 (*read)(void __iomem *reg);
>>  	void (*write)(u32 val, void __iomem *reg);
>> +	bool force_host_disconnect;
>>  };
>>  
>>  struct phy_parameter {
>> @@ -614,6 +621,16 @@ static int do_rtk_phy_init(struct rtk_phy *rtk_phy, int index)
>>  		goto do_toggle;
>>  	}
>>  
>> +	if (phy_cfg->force_host_disconnect) {
>> +		/* disable force-host-disconnect */
>> +		u32 temp = readl(phy_reg->reg_wrap_vstatus + PHY_9607_FORCE_DISCONNECT_REG);
>> +
>> +		temp &= ~PHY_9607_FORCE_DISCONNECT_BIT;
>> +		writel(temp, phy_reg->reg_wrap_vstatus + PHY_9607_FORCE_DISCONNECT_REG);
>> +
>> +		mdelay(10);
> 
> LLM review:
> 
> Could we use msleep(10) or usleep_range(10000, 11000) here instead of
> mdelay(10)?
> Since do_rtk_phy_init() executes as part of the phy_ops->init callback
> with a mutex held from a sleepable process context, spinning the CPU for
> 10ms wastes CPU resources and increases scheduling latency.

I can change it to msleep instead.

>> +	}
>> +
>>  	/* Set page 0 */
>>  	phy_data_page = phy_cfg->page0;
>>  	rtk_phy_set_page(phy_reg, 0);
>> @@ -1141,6 +1158,7 @@ static const struct phy_cfg rtd1295_phy_cfg = {
>>  	.new_reg_req = PHY_NEW_REG_REQ,
>>  	.read = phy_read,
>>  	.write = phy_write,
>> +	.force_host_disconnect = false,
> 
> You don't need to initialize rodata struct fields with false/0/NULL.

From what i can see, it lines up with other phy_cfg structs, and thats how they
did it and it did get accepted. You can check the rtd1295_phy_cfg as an example.

I am personally fine with removing the "force_host_disconnect = false" and other
falses in rtl9607_phy_cfg but i am debating because it wouldn't line up with the rest.

>>  };
>>  
>>  static const struct phy_cfg rtd1395_phy_cfg = {
>> @@ -1170,6 +1188,7 @@ static const struct phy_cfg rtd1395_phy_cfg = {
>>  	.new_reg_req = PHY_NEW_REG_REQ,
>>  	.read = phy_read,
>>  	.write = phy_write,
>> +	.force_host_disconnect = false,
>>  };
>>  
>>  static const struct phy_cfg rtd1395_phy_cfg_2port = {
>> @@ -1199,6 +1218,7 @@ static const struct phy_cfg rtd1395_phy_cfg_2port = {
>>  	.new_reg_req = PHY_NEW_REG_REQ,
>>  	.read = phy_read,
>>  	.write = phy_write,
>> +	.force_host_disconnect = false,
>>  };
>>  
>>  static const struct phy_cfg rtd1619_phy_cfg = {
>> @@ -1226,6 +1246,7 @@ static const struct phy_cfg rtd1619_phy_cfg = {
>>  	.new_reg_req = PHY_NEW_REG_REQ,
>>  	.read = phy_read,
>>  	.write = phy_write,
>> +	.force_host_disconnect = false,
>>  };
>>  
>>  static const struct phy_cfg rtd1319_phy_cfg = {
>> @@ -1257,6 +1278,7 @@ static const struct phy_cfg rtd1319_phy_cfg = {
>>  	.new_reg_req = PHY_NEW_REG_REQ,
>>  	.read = phy_read,
>>  	.write = phy_write,
>> +	.force_host_disconnect = false,
>>  };
>>  
>>  static const struct phy_cfg rtd1312c_phy_cfg = {
>> @@ -1287,6 +1309,7 @@ static const struct phy_cfg rtd1312c_phy_cfg = {
>>  	.new_reg_req = PHY_NEW_REG_REQ,
>>  	.read = phy_read,
>>  	.write = phy_write,
>> +	.force_host_disconnect = false,
>>  };
>>  
>>  static const struct phy_cfg rtd1619b_phy_cfg = {
>> @@ -1317,6 +1340,7 @@ static const struct phy_cfg rtd1619b_phy_cfg = {
>>  	.new_reg_req = PHY_NEW_REG_REQ,
>>  	.read = phy_read,
>>  	.write = phy_write,
>> +	.force_host_disconnect = false,
>>  };
>>  
>>  static const struct phy_cfg rtd1319d_phy_cfg = {
>> @@ -1347,6 +1371,7 @@ static const struct phy_cfg rtd1319d_phy_cfg = {
>>  	.new_reg_req = PHY_NEW_REG_REQ,
>>  	.read = phy_read,
>>  	.write = phy_write,
>> +	.force_host_disconnect = false,
>>  };
>>  
>>  static const struct phy_cfg rtd1315e_phy_cfg = {
>> @@ -1378,6 +1403,37 @@ static const struct phy_cfg rtd1315e_phy_cfg = {
>>  	.new_reg_req = PHY_NEW_REG_REQ,
>>  	.read = phy_read,
>>  	.write = phy_write,
>> +	.force_host_disconnect = false,
>> +};
>> +
>> +static const struct phy_cfg rtl9607_phy_cfg = {
>> +	.page0_size = MAX_USB_PHY_PAGE0_DATA_SIZE,
>> +	.page0 = { [0] = {0xe0, 0x95},
>> +		   [4] = {0xe4, 0x6a},
>> +		  [12] = {0xf3, 0x31}, },
>> +	.page1_size = MAX_USB_PHY_PAGE1_DATA_SIZE,
>> +	.page1 = { [0] = {0xe0, 0x26}, },
>> +	.page2_size = MAX_USB_PHY_PAGE2_DATA_SIZE,
>> +	.page2 = { [7] = {0xe7, 0x33}, },
>> +	.num_phy = 1,
>> +	.check_efuse = false,
> 
> Similar for these (+do_toggle_driving, use_default_parameter).
> 
>> +	.check_efuse_version = CHECK_EFUSE_V2,
>> +	.efuse_dc_driving_rate = EFUS_USB_DC_CAL_RATE,
>> +	.dc_driving_mask = 0x1f,
>> +	.efuse_dc_disconnect_rate = EFUS_USB_DC_DIS_RATE,
>> +	.dc_disconnect_mask = 0xf,
>> +	.usb_dc_disconnect_at_page0 = true,
>> +	.do_toggle = true,
>> +	.do_toggle_driving = false,
>> +	.driving_updated_for_dev_dis = 0x8,
>> +	.use_default_parameter = false,
>> +	.is_double_sensitivity_mode = true,
>> +	.vstatus_offset = 0xc,
>> +	.vstatus_busy = PHY_9607_VSTS_BUSY,
>> +	.new_reg_req = PHY_9607_NEW_REG_REQ,
>> +	.read = phy_read_le,
>> +	.write = phy_write_le,
>> +	.force_host_disconnect = true,
>>  };
>>  
>>  static const struct of_device_id usbphy_rtk_dt_match[] = {
>> @@ -1390,6 +1446,7 @@ static const struct of_device_id usbphy_rtk_dt_match[] = {
>>  	{ .compatible = "realtek,rtd1395-usb2phy-2port", .data = &rtd1395_phy_cfg_2port },
>>  	{ .compatible = "realtek,rtd1619-usb2phy", .data = &rtd1619_phy_cfg },
>>  	{ .compatible = "realtek,rtd1619b-usb2phy", .data = &rtd1619b_phy_cfg },
>> +	{ .compatible = "realtek,rtl9607-usb2phy", .data = &rtl9607_phy_cfg },
>>  	{},
>>  };
>>  MODULE_DEVICE_TABLE(of, usbphy_rtk_dt_match);
>> -- 
>> 2.53.0
>> 
>>

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 5/6] phy: realtek: usb2: add support for RTL9607C USB2 PHY

[Vladimir Oltean]

On Tue, Mar 31, 2026 at 04:48:08PM +0000, Rustam Adilov wrote:
> I am personally fine with removing the "force_host_disconnect = false" and other
> falses in rtl9607_phy_cfg but i am debating because it wouldn't line up with the rest.

You can also remove the other unnecessary initializations.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 5/6] phy: realtek: usb2: add support for RTL9607C USB2 PHY

[Rustam Adilov]

On 2026-03-31 19:36, Vladimir Oltean wrote:
> On Tue, Mar 31, 2026 at 04:48:08PM +0000, Rustam Adilov wrote:
>> I am personally fine with removing the "force_host_disconnect = false" and other
>> falses in rtl9607_phy_cfg but i am debating because it wouldn't line up with the rest.
> 
> You can also remove the other unnecessary initializations.

After thinking about it, removing other unnecessary initializations would be out of scope
for this patch series but i can remove unnecessary things introduced by my patch series,
i.e force_host_disconnect, and specifically things inside the rtl9607_phy_cfg.

Later on, someone else can clean up the rest as a separate patch.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v2 6/6] phy: realtek: usb2: Make configs available for MACH_REALTEK_RTL

[Rustam Adilov]

Add the MACH_REALTEK_RTL to the if statement to make the config
options available for Realtek RTL SoCs as well.

Signed-off-by: Rustam Adilov <adilov@disroot.org>
---
 drivers/phy/realtek/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/realtek/Kconfig b/drivers/phy/realtek/Kconfig
index 75ac7e7c31ae..f9eadffacd18 100644
--- a/drivers/phy/realtek/Kconfig
+++ b/drivers/phy/realtek/Kconfig
@@ -3,7 +3,7 @@
 # Phy drivers for Realtek platforms
 #
 
-if ARCH_REALTEK || COMPILE_TEST
+if ARCH_REALTEK || MACH_REALTEK_RTL || COMPILE_TEST
 
 config PHY_RTK_RTD_USB2PHY
 	tristate "Realtek RTD USB2 PHY Transceiver Driver"
-- 
2.53.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 6/6] phy: realtek: usb2: Make configs available for MACH_REALTEK_RTL

[Vladimir Oltean]

On Fri, Mar 27, 2026 at 09:06:38PM +0500, Rustam Adilov wrote:
> Add the MACH_REALTEK_RTL to the if statement to make the config
> options available for Realtek RTL SoCs as well.
> 
> Signed-off-by: Rustam Adilov <adilov@disroot.org>
> ---
>  drivers/phy/realtek/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/realtek/Kconfig b/drivers/phy/realtek/Kconfig
> index 75ac7e7c31ae..f9eadffacd18 100644
> --- a/drivers/phy/realtek/Kconfig
> +++ b/drivers/phy/realtek/Kconfig
> @@ -3,7 +3,7 @@
>  # Phy drivers for Realtek platforms
>  #
>  
> -if ARCH_REALTEK || COMPILE_TEST
> +if ARCH_REALTEK || MACH_REALTEK_RTL || COMPILE_TEST
>  
>  config PHY_RTK_RTD_USB2PHY
>  	tristate "Realtek RTD USB2 PHY Transceiver Driver"
> -- 
> 2.53.0
> 
> 

The file now reads:

if ARCH_REALTEK || MACH_REALTEK_RTL || COMPILE_TEST

...

endif # ARCH_REALTEK || COMPILE_TEST

Please update the end comment as well.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 6/6] phy: realtek: usb2: Make configs available for MACH_REALTEK_RTL

[Rustam Adilov]

On 2026-03-30 21:52, Vladimir Oltean wrote:
> On Fri, Mar 27, 2026 at 09:06:38PM +0500, Rustam Adilov wrote:
>> Add the MACH_REALTEK_RTL to the if statement to make the config
>> options available for Realtek RTL SoCs as well.
>> 
>> Signed-off-by: Rustam Adilov <adilov@disroot.org>
>> ---
>>  drivers/phy/realtek/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/phy/realtek/Kconfig b/drivers/phy/realtek/Kconfig
>> index 75ac7e7c31ae..f9eadffacd18 100644
>> --- a/drivers/phy/realtek/Kconfig
>> +++ b/drivers/phy/realtek/Kconfig
>> @@ -3,7 +3,7 @@
>>  # Phy drivers for Realtek platforms
>>  #
>>  
>> -if ARCH_REALTEK || COMPILE_TEST
>> +if ARCH_REALTEK || MACH_REALTEK_RTL || COMPILE_TEST
>>  
>>  config PHY_RTK_RTD_USB2PHY
>>  	tristate "Realtek RTD USB2 PHY Transceiver Driver"
>> -- 
>> 2.53.0
>> 
>> 
> 
> The file now reads:
> 
> if ARCH_REALTEK || MACH_REALTEK_RTL || COMPILE_TEST
> 
> ...
> 
> endif # ARCH_REALTEK || COMPILE_TEST
> 
> Please update the end comment as well.

Good notice, will change it.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy