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

[PATCH v9 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.

Tested on my RTL9607C based machine as a backport to 6.18 linux in OpenWrt.

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

---
Changelog in v9:
- Patch 3:
 - removed yaml prefix from the commit subject.
- Link to v8: https://lore.kernel.org/linux-phy/20261012173739.36377-1-adilov@disroot.org/

Changelog in v8:
- Patch 5
 - changed 0xf3 mapping to index 11 (sashiko)
- Link to v7: https://lore.kernel.org/linux-phy/20260815091340.26910-1-adilov@disroot.org/

Changelog in v7:
- Patch 1
 - created a phy_reg_desc struct to get rid of duplicated fields across phy_cfg and phy_reg.
 - added a default rtd_phy_reg_desc for all existing SoC phy configs.
- Patch 2
 - moved the read/write to a newly added phy_reg_desc struct.
- Patch 3
 - added an Acked-by tag from Manivannan Sadhasivam.
- Patch 4
 - replaced devm_reset_control_array_get*() with devm_reset_control_get*()
- Patch 5
 - added the read operation after write to FORCE_DISCONNECT_REG.
- Patch 6
 - added a Reviewed-by tag from Manivannan Sadhasivam.
- Link to v6: https://lore.kernel.org/linux-phy/20260520175728.720877-1-adilov@disroot.org/

Changelog in v6:
Driver has been refactored with enabled SWAP_IO_SPACE config in mind.
- Patch 1
 - fixed one line exceeding 80 columns.
- Patch 2
 - changed to use the introduced read/write functions for reg_wrap_vstatus
   instead of reg_gusb2phyacc0.
- Patch 3
 - properly added the review tag from v4 of patch series.
- Patch 5
 - fixed the lines exceeding 80 columns.
 - changed msleep(10) to usleep_range(10000, 11000).
 - changed the read and write functions to use ioread32be instead of __le32 
   now that we are using them for reg_wrap_vstatus.
- Link to v5: https://lore.kernel.org/linux-phy/20260420191941.81834-1-adilov@disroot.org/

Changelog in v5:
Mostly addressing LLM review
- Patch 1
 - changed int to u32 type for new_reg_req and vstatus_busy data fields.
 - changed comments in rtk_phy_read/write from PHY_NEW_REG_REQ to phy_reg->new_reg_req.
- Patch 2
 - explained readl/writel native endianess issue in more detail.
 - explained why vstatus register doesn't need byte swapping.
- Patch 4
 - moved reset_control_deassert to rtk_phy_init function to keep it outside of for loop.
 - changed msleep(5) to usleep_range(5000, 6000).
 - explained why reset_control_assert is not needed.
- Patch 5
 - explained readl/writel native endianess issue here as well.
 - explained why FORCE_DISCONNECT_REG doesn't need byte swapping.
- Link to v4: https://lore.kernel.org/linux-phy/20260406181228.25892-1-adilov@disroot.org/

Changelog in v4:
- Patch 2
 - moved the le variations of read/write functions to Patch 5 where it is actually used because
   otherwise, it results in unused errors when only Patch 2 is applied.
 - updated the commit message to to point the reason for le32 wrappers around readl/writel.
- Patch 3
 - added "Reviewed by Krzysztof Kozlowski"
- Patch 5
 - updated the commit message to include the addition of little endian read/write functions from
   Patch 2.
- Link to v3: https://lore.kernel.org/linux-phy/20260402154414.196012-1-adilov@disroot.org/

Changelog in v3:
- Patch 2
 - renamed phy read and functions to "rtk_usb2phy" to not collide with networking API functions
 - fixed the sparse warnings by creating intermidiate "tmp" variable and then pass it to writel
 - sligtly adjusted commit message to instead use "default read" not "default phy_read"
- Patch 4
 - added the check for reset_control_deassert() just in case
 - changed mdelay(5) to msleep(5)
 - changed dev_err and return combo with one dev_err_probe for phy_rst
- Patch 5
 - changed mdelay(10) under force_host_disconnect to msleep(10)
 - removed struct fields with false like force_host_disconnect and more in rtl9607_phy_cfg
- Patch 6
 - updated the #endif commend to now include MACH_REALTEK_RTL to reflect if on top
- Link to v2: https://lore.kernel.org/linux-phy/20260327160638.15134-1-adilov@disroot.org/

Changelog in v2:
- Patch 3
 - 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 phy_reg_desc struct to the driver
  phy: realtek: usb2: introduce read and write functions to phy_reg_desc
    struct
  dt-bindings: phy: realtek,usb2phy: extend for resets and RTL9607C
    support
  phy: realtek: usb2: introduce reset controller struct
  phy: realtek: usb2: add support for RTL9607C USB2 PHY
  phy: realtek: Make configs available for MACH_REALTEK_RTL

 .../bindings/phy/realtek,usb2phy.yaml         |  25 ++-
 drivers/phy/realtek/Kconfig                   |   4 +-
 drivers/phy/realtek/phy-rtk-usb2.c            | 154 ++++++++++++++++--
 3 files changed, 165 insertions(+), 18 deletions(-)

-- 
2.56.0


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

[PATCH v9 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.

Tested on my RTL9607C based machine as a backport to 6.18 linux in OpenWrt.

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

---
Changelog in v9:
- Patch 3:
 - removed yaml prefix from the commit subject.
- Link to v8: https://lore.kernel.org/linux-phy/20261012173739.36377-1-adilov@disroot.org/

Changelog in v8:
- Patch 5
 - changed 0xf3 mapping to index 11 (sashiko)
- Link to v7: https://lore.kernel.org/linux-phy/20260815091340.26910-1-adilov@disroot.org/

Changelog in v7:
- Patch 1
 - created a phy_reg_desc struct to get rid of duplicated fields across phy_cfg and phy_reg.
 - added a default rtd_phy_reg_desc for all existing SoC phy configs.
- Patch 2
 - moved the read/write to a newly added phy_reg_desc struct.
- Patch 3
 - added an Acked-by tag from Manivannan Sadhasivam.
- Patch 4
 - replaced devm_reset_control_array_get*() with devm_reset_control_get*()
- Patch 5
 - added the read operation after write to FORCE_DISCONNECT_REG.
- Patch 6
 - added a Reviewed-by tag from Manivannan Sadhasivam.
- Link to v6: https://lore.kernel.org/linux-phy/20260520175728.720877-1-adilov@disroot.org/

Changelog in v6:
Driver has been refactored with enabled SWAP_IO_SPACE config in mind.
- Patch 1
 - fixed one line exceeding 80 columns.
- Patch 2
 - changed to use the introduced read/write functions for reg_wrap_vstatus
   instead of reg_gusb2phyacc0.
- Patch 3
 - properly added the review tag from v4 of patch series.
- Patch 5
 - fixed the lines exceeding 80 columns.
 - changed msleep(10) to usleep_range(10000, 11000).
 - changed the read and write functions to use ioread32be instead of __le32 
   now that we are using them for reg_wrap_vstatus.
- Link to v5: https://lore.kernel.org/linux-phy/20260420191941.81834-1-adilov@disroot.org/

Changelog in v5:
Mostly addressing LLM review
- Patch 1
 - changed int to u32 type for new_reg_req and vstatus_busy data fields.
 - changed comments in rtk_phy_read/write from PHY_NEW_REG_REQ to phy_reg->new_reg_req.
- Patch 2
 - explained readl/writel native endianess issue in more detail.
 - explained why vstatus register doesn't need byte swapping.
- Patch 4
 - moved reset_control_deassert to rtk_phy_init function to keep it outside of for loop.
 - changed msleep(5) to usleep_range(5000, 6000).
 - explained why reset_control_assert is not needed.
- Patch 5
 - explained readl/writel native endianess issue here as well.
 - explained why FORCE_DISCONNECT_REG doesn't need byte swapping.
- Link to v4: https://lore.kernel.org/linux-phy/20260406181228.25892-1-adilov@disroot.org/

Changelog in v4:
- Patch 2
 - moved the le variations of read/write functions to Patch 5 where it is actually used because
   otherwise, it results in unused errors when only Patch 2 is applied.
 - updated the commit message to to point the reason for le32 wrappers around readl/writel.
- Patch 3
 - added "Reviewed by Krzysztof Kozlowski"
- Patch 5
 - updated the commit message to include the addition of little endian read/write functions from
   Patch 2.
- Link to v3: https://lore.kernel.org/linux-phy/20260402154414.196012-1-adilov@disroot.org/

Changelog in v3:
- Patch 2
 - renamed phy read and functions to "rtk_usb2phy" to not collide with networking API functions
 - fixed the sparse warnings by creating intermidiate "tmp" variable and then pass it to writel
 - sligtly adjusted commit message to instead use "default read" not "default phy_read"
- Patch 4
 - added the check for reset_control_deassert() just in case
 - changed mdelay(5) to msleep(5)
 - changed dev_err and return combo with one dev_err_probe for phy_rst
- Patch 5
 - changed mdelay(10) under force_host_disconnect to msleep(10)
 - removed struct fields with false like force_host_disconnect and more in rtl9607_phy_cfg
- Patch 6
 - updated the #endif commend to now include MACH_REALTEK_RTL to reflect if on top
- Link to v2: https://lore.kernel.org/linux-phy/20260327160638.15134-1-adilov@disroot.org/

Changelog in v2:
- Patch 3
 - 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 phy_reg_desc struct to the driver
  phy: realtek: usb2: introduce read and write functions to phy_reg_desc
    struct
  dt-bindings: phy: realtek,usb2phy: extend for resets and RTL9607C
    support
  phy: realtek: usb2: introduce reset controller struct
  phy: realtek: usb2: add support for RTL9607C USB2 PHY
  phy: realtek: Make configs available for MACH_REALTEK_RTL

 .../bindings/phy/realtek,usb2phy.yaml         |  25 ++-
 drivers/phy/realtek/Kconfig                   |   4 +-
 drivers/phy/realtek/phy-rtk-usb2.c            | 154 ++++++++++++++++--
 3 files changed, 165 insertions(+), 18 deletions(-)

-- 
2.56.0

[PATCH v9 1/6] phy: realtek: usb2: introduce phy_reg_desc struct to the driver

[Rustam Adilov]

In RTL9607C SoC, there are a few differences in usb2 phy compared
to existing RTD SoCs, like vstatus register is at certain offset
from the base, busy bit and new_reg_req bit are at different bits
of the register.

To avoid duplicating these variables across phy cfg and driver data,
create a per-SoC struct that would hold them and with that add the
default rtd_phy_reg_desc to which all RTD SoCs driver data will
point towards.

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, 44 insertions(+), 15 deletions(-)

diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
index 248550ef98ca..e8d0f0119ec0 100644
--- a/drivers/phy/realtek/phy-rtk-usb2.c
+++ b/drivers/phy/realtek/phy-rtk-usb2.c
@@ -60,10 +60,17 @@
 #define DEFAULT_DC_DISCONNECTION_VALUE (0x6)
 #define HS_CLK_SELECT BIT(6)
 
+struct phy_reg_desc {
+	int vstatus_offset;
+	u32 vstatus_busy;
+	u32 new_reg_req;
+};
+
 struct phy_reg {
 	void __iomem *reg_wrap_vstatus;
 	void __iomem *reg_gusb2phyacc0;
 	int vstatus_index;
+	const struct phy_reg_desc *desc;
 };
 
 struct phy_data {
@@ -96,6 +103,7 @@ struct phy_cfg {
 	bool do_toggle_driving;
 	bool use_default_parameter;
 	bool is_double_sensitivity_mode;
+	const struct phy_reg_desc *reg_desc;
 };
 
 struct phy_parameter {
@@ -156,27 +164,28 @@ static inline int utmi_wait_register(void __iomem *reg, u32 mask, u32 result)
 static char rtk_phy_read(struct phy_reg *phy_reg, char addr)
 {
 	void __iomem *reg_gusb2phyacc0 = phy_reg->reg_gusb2phyacc0;
+	const struct phy_reg_desc *reg_desc = phy_reg->desc;
 	unsigned int val;
 	int ret = 0;
 
 	addr -= OFFEST_PHY_READ;
 
 	/* polling until VBusy == 0 */
-	ret = utmi_wait_register(reg_gusb2phyacc0, PHY_VSTS_BUSY, 0);
+	ret = utmi_wait_register(reg_gusb2phyacc0, reg_desc->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);
+	/* VCtrl = low nibble of addr, and set reg_desc->new_reg_req */
+	val = reg_desc->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, reg_desc->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);
+	/* VCtrl = high nibble of addr, and set reg_desc->new_reg_req */
+	val = reg_desc->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, reg_desc->vstatus_busy, 0);
 	if (ret)
 		return (char)ret;
 
@@ -190,29 +199,31 @@ static int rtk_phy_write(struct phy_reg *phy_reg, char addr, char data)
 	unsigned int val;
 	void __iomem *reg_wrap_vstatus = phy_reg->reg_wrap_vstatus;
 	void __iomem *reg_gusb2phyacc0 = phy_reg->reg_gusb2phyacc0;
+	const struct phy_reg_desc *reg_desc = phy_reg->desc;
 	int shift_bits = phy_reg->vstatus_index * 8;
 	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 + reg_desc->vstatus_offset);
 
-	ret = utmi_wait_register(reg_gusb2phyacc0, PHY_VSTS_BUSY, 0);
+	ret = utmi_wait_register(reg_gusb2phyacc0, reg_desc->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);
+	/* VCtrl = low nibble of addr, set reg_desc->new_reg_req */
+	val = reg_desc->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, reg_desc->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);
+	/* VCtrl = high nibble of addr, set reg_desc->new_reg_req */
+	val = reg_desc->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, reg_desc->vstatus_busy, 0);
 	if (ret)
 		return ret;
 
@@ -957,6 +968,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 +986,8 @@ 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.desc = phy_cfg->reg_desc;
+
 
 		if (of_property_read_bool(np, "realtek,inverse-hstx-sync-clock"))
 			phy_parameter->inverse_hstx_sync_clock = true;
@@ -1060,6 +1074,12 @@ static void rtk_usb2phy_remove(struct platform_device *pdev)
 	remove_debug_files(rtk_phy);
 }
 
+static const struct phy_reg_desc rtd_phy_reg_desc = {
+	.vstatus_offset = 0,
+	.vstatus_busy = PHY_VSTS_BUSY,
+	.new_reg_req = PHY_NEW_REG_REQ,
+};
+
 static const struct phy_cfg rtd1295_phy_cfg = {
 	.page0_size = MAX_USB_PHY_PAGE0_DATA_SIZE,
 	.page0 = { [0] = {0xe0, 0x90},
@@ -1085,6 +1105,7 @@ static const struct phy_cfg rtd1295_phy_cfg = {
 	.driving_updated_for_dev_dis = 0xf,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = false,
+	.reg_desc = &rtd_phy_reg_desc,
 };
 
 static const struct phy_cfg rtd1395_phy_cfg = {
@@ -1109,6 +1130,7 @@ static const struct phy_cfg rtd1395_phy_cfg = {
 	.driving_updated_for_dev_dis = 0xf,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = false,
+	.reg_desc = &rtd_phy_reg_desc,
 };
 
 static const struct phy_cfg rtd1395_phy_cfg_2port = {
@@ -1133,6 +1155,7 @@ static const struct phy_cfg rtd1395_phy_cfg_2port = {
 	.driving_updated_for_dev_dis = 0xf,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = false,
+	.reg_desc = &rtd_phy_reg_desc,
 };
 
 static const struct phy_cfg rtd1619_phy_cfg = {
@@ -1155,6 +1178,7 @@ static const struct phy_cfg rtd1619_phy_cfg = {
 	.driving_updated_for_dev_dis = 0xf,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = false,
+	.reg_desc = &rtd_phy_reg_desc,
 };
 
 static const struct phy_cfg rtd1319_phy_cfg = {
@@ -1181,6 +1205,7 @@ static const struct phy_cfg rtd1319_phy_cfg = {
 	.driving_updated_for_dev_dis = 0xf,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = true,
+	.reg_desc = &rtd_phy_reg_desc,
 };
 
 static const struct phy_cfg rtd1312c_phy_cfg = {
@@ -1206,6 +1231,7 @@ static const struct phy_cfg rtd1312c_phy_cfg = {
 	.driving_updated_for_dev_dis = 0xf,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = true,
+	.reg_desc = &rtd_phy_reg_desc,
 };
 
 static const struct phy_cfg rtd1619b_phy_cfg = {
@@ -1231,6 +1257,7 @@ static const struct phy_cfg rtd1619b_phy_cfg = {
 	.driving_updated_for_dev_dis = 0x8,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = true,
+	.reg_desc = &rtd_phy_reg_desc,
 };
 
 static const struct phy_cfg rtd1319d_phy_cfg = {
@@ -1256,6 +1283,7 @@ static const struct phy_cfg rtd1319d_phy_cfg = {
 	.driving_updated_for_dev_dis = 0x8,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = true,
+	.reg_desc = &rtd_phy_reg_desc,
 };
 
 static const struct phy_cfg rtd1315e_phy_cfg = {
@@ -1282,6 +1310,7 @@ static const struct phy_cfg rtd1315e_phy_cfg = {
 	.driving_updated_for_dev_dis = 0x8,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = true,
+	.reg_desc = &rtd_phy_reg_desc,
 };
 
 static const struct of_device_id usbphy_rtk_dt_match[] = {
-- 
2.56.0


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

[PATCH v9 1/6] phy: realtek: usb2: introduce phy_reg_desc struct to the driver

[Rustam Adilov]

In RTL9607C SoC, there are a few differences in usb2 phy compared
to existing RTD SoCs, like vstatus register is at certain offset
from the base, busy bit and new_reg_req bit are at different bits
of the register.

To avoid duplicating these variables across phy cfg and driver data,
create a per-SoC struct that would hold them and with that add the
default rtd_phy_reg_desc to which all RTD SoCs driver data will
point towards.

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, 44 insertions(+), 15 deletions(-)

diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
index 248550ef98ca..e8d0f0119ec0 100644
--- a/drivers/phy/realtek/phy-rtk-usb2.c
+++ b/drivers/phy/realtek/phy-rtk-usb2.c
@@ -60,10 +60,17 @@
 #define DEFAULT_DC_DISCONNECTION_VALUE (0x6)
 #define HS_CLK_SELECT BIT(6)
 
+struct phy_reg_desc {
+	int vstatus_offset;
+	u32 vstatus_busy;
+	u32 new_reg_req;
+};
+
 struct phy_reg {
 	void __iomem *reg_wrap_vstatus;
 	void __iomem *reg_gusb2phyacc0;
 	int vstatus_index;
+	const struct phy_reg_desc *desc;
 };
 
 struct phy_data {
@@ -96,6 +103,7 @@ struct phy_cfg {
 	bool do_toggle_driving;
 	bool use_default_parameter;
 	bool is_double_sensitivity_mode;
+	const struct phy_reg_desc *reg_desc;
 };
 
 struct phy_parameter {
@@ -156,27 +164,28 @@ static inline int utmi_wait_register(void __iomem *reg, u32 mask, u32 result)
 static char rtk_phy_read(struct phy_reg *phy_reg, char addr)
 {
 	void __iomem *reg_gusb2phyacc0 = phy_reg->reg_gusb2phyacc0;
+	const struct phy_reg_desc *reg_desc = phy_reg->desc;
 	unsigned int val;
 	int ret = 0;
 
 	addr -= OFFEST_PHY_READ;
 
 	/* polling until VBusy == 0 */
-	ret = utmi_wait_register(reg_gusb2phyacc0, PHY_VSTS_BUSY, 0);
+	ret = utmi_wait_register(reg_gusb2phyacc0, reg_desc->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);
+	/* VCtrl = low nibble of addr, and set reg_desc->new_reg_req */
+	val = reg_desc->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, reg_desc->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);
+	/* VCtrl = high nibble of addr, and set reg_desc->new_reg_req */
+	val = reg_desc->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, reg_desc->vstatus_busy, 0);
 	if (ret)
 		return (char)ret;
 
@@ -190,29 +199,31 @@ static int rtk_phy_write(struct phy_reg *phy_reg, char addr, char data)
 	unsigned int val;
 	void __iomem *reg_wrap_vstatus = phy_reg->reg_wrap_vstatus;
 	void __iomem *reg_gusb2phyacc0 = phy_reg->reg_gusb2phyacc0;
+	const struct phy_reg_desc *reg_desc = phy_reg->desc;
 	int shift_bits = phy_reg->vstatus_index * 8;
 	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 + reg_desc->vstatus_offset);
 
-	ret = utmi_wait_register(reg_gusb2phyacc0, PHY_VSTS_BUSY, 0);
+	ret = utmi_wait_register(reg_gusb2phyacc0, reg_desc->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);
+	/* VCtrl = low nibble of addr, set reg_desc->new_reg_req */
+	val = reg_desc->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, reg_desc->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);
+	/* VCtrl = high nibble of addr, set reg_desc->new_reg_req */
+	val = reg_desc->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, reg_desc->vstatus_busy, 0);
 	if (ret)
 		return ret;
 
@@ -957,6 +968,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 +986,8 @@ 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.desc = phy_cfg->reg_desc;
+
 
 		if (of_property_read_bool(np, "realtek,inverse-hstx-sync-clock"))
 			phy_parameter->inverse_hstx_sync_clock = true;
@@ -1060,6 +1074,12 @@ static void rtk_usb2phy_remove(struct platform_device *pdev)
 	remove_debug_files(rtk_phy);
 }
 
+static const struct phy_reg_desc rtd_phy_reg_desc = {
+	.vstatus_offset = 0,
+	.vstatus_busy = PHY_VSTS_BUSY,
+	.new_reg_req = PHY_NEW_REG_REQ,
+};
+
 static const struct phy_cfg rtd1295_phy_cfg = {
 	.page0_size = MAX_USB_PHY_PAGE0_DATA_SIZE,
 	.page0 = { [0] = {0xe0, 0x90},
@@ -1085,6 +1105,7 @@ static const struct phy_cfg rtd1295_phy_cfg = {
 	.driving_updated_for_dev_dis = 0xf,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = false,
+	.reg_desc = &rtd_phy_reg_desc,
 };
 
 static const struct phy_cfg rtd1395_phy_cfg = {
@@ -1109,6 +1130,7 @@ static const struct phy_cfg rtd1395_phy_cfg = {
 	.driving_updated_for_dev_dis = 0xf,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = false,
+	.reg_desc = &rtd_phy_reg_desc,
 };
 
 static const struct phy_cfg rtd1395_phy_cfg_2port = {
@@ -1133,6 +1155,7 @@ static const struct phy_cfg rtd1395_phy_cfg_2port = {
 	.driving_updated_for_dev_dis = 0xf,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = false,
+	.reg_desc = &rtd_phy_reg_desc,
 };
 
 static const struct phy_cfg rtd1619_phy_cfg = {
@@ -1155,6 +1178,7 @@ static const struct phy_cfg rtd1619_phy_cfg = {
 	.driving_updated_for_dev_dis = 0xf,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = false,
+	.reg_desc = &rtd_phy_reg_desc,
 };
 
 static const struct phy_cfg rtd1319_phy_cfg = {
@@ -1181,6 +1205,7 @@ static const struct phy_cfg rtd1319_phy_cfg = {
 	.driving_updated_for_dev_dis = 0xf,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = true,
+	.reg_desc = &rtd_phy_reg_desc,
 };
 
 static const struct phy_cfg rtd1312c_phy_cfg = {
@@ -1206,6 +1231,7 @@ static const struct phy_cfg rtd1312c_phy_cfg = {
 	.driving_updated_for_dev_dis = 0xf,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = true,
+	.reg_desc = &rtd_phy_reg_desc,
 };
 
 static const struct phy_cfg rtd1619b_phy_cfg = {
@@ -1231,6 +1257,7 @@ static const struct phy_cfg rtd1619b_phy_cfg = {
 	.driving_updated_for_dev_dis = 0x8,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = true,
+	.reg_desc = &rtd_phy_reg_desc,
 };
 
 static const struct phy_cfg rtd1319d_phy_cfg = {
@@ -1256,6 +1283,7 @@ static const struct phy_cfg rtd1319d_phy_cfg = {
 	.driving_updated_for_dev_dis = 0x8,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = true,
+	.reg_desc = &rtd_phy_reg_desc,
 };
 
 static const struct phy_cfg rtd1315e_phy_cfg = {
@@ -1282,6 +1310,7 @@ static const struct phy_cfg rtd1315e_phy_cfg = {
 	.driving_updated_for_dev_dis = 0x8,
 	.use_default_parameter = false,
 	.is_double_sensitivity_mode = true,
+	.reg_desc = &rtd_phy_reg_desc,
 };
 
 static const struct of_device_id usbphy_rtk_dt_match[] = {
-- 
2.56.0

Re: [PATCH v9 1/6] phy: realtek: usb2: introduce phy_reg_desc struct to the driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] usb: phy: unaligned IO pointer arithmetic on void __iomem *
- [High] phy: realtek: missing NULL check and iounmap() for of_iomap()
- [High] phy: realtek: off-by-one array bounds check allows out-of-bounds access
--

commit a5449cff40ee54bc6f98436955a56c20a0155c71
Author: Rustam Adilov <adilov@disroot.org>

phy: realtek: usb2: introduce phy_reg_desc struct to the driver

In RTL9607C SoC, there are a few differences in usb2 phy compared
to existing RTD SoCs, like vstatus register is at certain offset
from the base, busy bit and new_reg_req bit are at different bits
of the register.

To avoid duplicating these variables across phy cfg and driver data,
create a per-SoC struct that would hold them and with that add the
default rtd_phy_reg_desc to which all RTD SoCs driver data will
point towards.

> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
> index 248550ef98cab..e8d0f0119ec0a 100644
> --- a/drivers/phy/realtek/phy-rtk-usb2.c
> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
> @@ -156,27 +164,28 @@ static inline int utmi_wait_register(void __iomem *reg, u32 mask, u32 result)
>  static char rtk_phy_read(struct phy_reg *phy_reg, char addr)
>  {
>  	void __iomem *reg_gusb2phyacc0 = phy_reg->reg_gusb2phyacc0;
> +	const struct phy_reg_desc *reg_desc = phy_reg->desc;
>  	unsigned int val;
>  	int ret = 0;
>  
>  	addr -= OFFEST_PHY_READ;
>  
>  	/* polling until VBusy == 0 */
> -	ret = utmi_wait_register(reg_gusb2phyacc0, PHY_VSTS_BUSY, 0);
> +	ret = utmi_wait_register(reg_gusb2phyacc0, reg_desc->vstatus_busy, 0);

[Severity: High]
Can this dereference an out-of-bounds array element?

In rtk_phy_toggle(), the bounds check uses > instead of >=:

    if (index > rtk_phy->num_phy) {

This incorrectly allows index to equal the array size num_phy. When
do_rtk_phy_toggle() uses this out-of-bounds index to access the dynamically
allocated phy_parameter array, an invalid phy_reg element is passed to
rtk_phy_read().

Dereferencing the explicitly added desc pointer from this invalid phy_reg
element will immediately cause a NULL pointer dereference or memory corruption.

[ ... ]

> @@ -957,6 +968,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 +986,8 @@ 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;

[Severity: High]
Does this code handle of_iomap() failures correctly?

of_iomap() can return NULL. The driver fails to check this return value and
proceeds, which can cause a NULL pointer dereference later when
reg_wrap_vstatus is offset and dereferenced in rtk_phy_write().

Additionally, this maps the exact same indices (0 and 1) redundantly in a
loop up to num_phy times, which may leak I/O memory if mapped multiple times.

[Severity: High]
Does this pointer arithmetic result in an unaligned pointer?

Adding index to the void __iomem * returned by of_iomap(np, 1) increments the
address by exactly index bytes (e.g. 1 byte for the second PHY in a dual-port
configuration). This creates an unaligned I/O pointer.

Executing a 32-bit I/O access on this unaligned pointer in rtk_phy_write()
can cause a hardware Alignment Fault.

>  		phy_parameter->phy_reg.vstatus_index = index;
> +		phy_parameter->phy_reg.desc = phy_cfg->reg_desc;
> +
>  
>  		if (of_property_read_bool(np, "realtek,inverse-hstx-sync-clock"))
>  			phy_parameter->inverse_hstx_sync_clock = true;

[ ... ]

> @@ -1060,6 +1074,12 @@ static void rtk_usb2phy_remove(struct platform_device *pdev)
>  	remove_debug_files(rtk_phy);
>  }

[Severity: High]
Does this missing cleanup cause an I/O memory leak?

The teardown function completes without calling iounmap() for the regions
mapped by of_iomap() in parse_phy_data(), resulting in a permanent I/O memory
leak on driver removal.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016180314.31760-1-adilov@disroot.org?part=1

Re: [PATCH v9 1/6] phy: realtek: usb2: introduce phy_reg_desc struct to the driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] usb: phy: unaligned IO pointer arithmetic on void __iomem *
- [High] phy: realtek: missing NULL check and iounmap() for of_iomap()
- [High] phy: realtek: off-by-one array bounds check allows out-of-bounds access
--

commit a5449cff40ee54bc6f98436955a56c20a0155c71
Author: Rustam Adilov <adilov@disroot.org>

phy: realtek: usb2: introduce phy_reg_desc struct to the driver

In RTL9607C SoC, there are a few differences in usb2 phy compared
to existing RTD SoCs, like vstatus register is at certain offset
from the base, busy bit and new_reg_req bit are at different bits
of the register.

To avoid duplicating these variables across phy cfg and driver data,
create a per-SoC struct that would hold them and with that add the
default rtd_phy_reg_desc to which all RTD SoCs driver data will
point towards.

> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
> index 248550ef98cab..e8d0f0119ec0a 100644
> --- a/drivers/phy/realtek/phy-rtk-usb2.c
> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
> @@ -156,27 +164,28 @@ static inline int utmi_wait_register(void __iomem *reg, u32 mask, u32 result)
>  static char rtk_phy_read(struct phy_reg *phy_reg, char addr)
>  {
>  	void __iomem *reg_gusb2phyacc0 = phy_reg->reg_gusb2phyacc0;
> +	const struct phy_reg_desc *reg_desc = phy_reg->desc;
>  	unsigned int val;
>  	int ret = 0;
>  
>  	addr -= OFFEST_PHY_READ;
>  
>  	/* polling until VBusy == 0 */
> -	ret = utmi_wait_register(reg_gusb2phyacc0, PHY_VSTS_BUSY, 0);
> +	ret = utmi_wait_register(reg_gusb2phyacc0, reg_desc->vstatus_busy, 0);

[Severity: High]
Can this dereference an out-of-bounds array element?

In rtk_phy_toggle(), the bounds check uses > instead of >=:

    if (index > rtk_phy->num_phy) {

This incorrectly allows index to equal the array size num_phy. When
do_rtk_phy_toggle() uses this out-of-bounds index to access the dynamically
allocated phy_parameter array, an invalid phy_reg element is passed to
rtk_phy_read().

Dereferencing the explicitly added desc pointer from this invalid phy_reg
element will immediately cause a NULL pointer dereference or memory corruption.

[ ... ]

> @@ -957,6 +968,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 +986,8 @@ 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;

[Severity: High]
Does this code handle of_iomap() failures correctly?

of_iomap() can return NULL. The driver fails to check this return value and
proceeds, which can cause a NULL pointer dereference later when
reg_wrap_vstatus is offset and dereferenced in rtk_phy_write().

Additionally, this maps the exact same indices (0 and 1) redundantly in a
loop up to num_phy times, which may leak I/O memory if mapped multiple times.

[Severity: High]
Does this pointer arithmetic result in an unaligned pointer?

Adding index to the void __iomem * returned by of_iomap(np, 1) increments the
address by exactly index bytes (e.g. 1 byte for the second PHY in a dual-port
configuration). This creates an unaligned I/O pointer.

Executing a 32-bit I/O access on this unaligned pointer in rtk_phy_write()
can cause a hardware Alignment Fault.

>  		phy_parameter->phy_reg.vstatus_index = index;
> +		phy_parameter->phy_reg.desc = phy_cfg->reg_desc;
> +
>  
>  		if (of_property_read_bool(np, "realtek,inverse-hstx-sync-clock"))
>  			phy_parameter->inverse_hstx_sync_clock = true;

[ ... ]

> @@ -1060,6 +1074,12 @@ static void rtk_usb2phy_remove(struct platform_device *pdev)
>  	remove_debug_files(rtk_phy);
>  }

[Severity: High]
Does this missing cleanup cause an I/O memory leak?

The teardown function completes without calling iounmap() for the regions
mapped by of_iomap() in parse_phy_data(), resulting in a permanent I/O memory
leak on driver removal.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016180314.31760-1-adilov@disroot.org?part=1

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

Re: [PATCH v9 1/6] phy: realtek: usb2: introduce phy_reg_desc struct to the driver

[Rustam Adilov]

On 2026-10-16 18:15, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] usb: phy: unaligned IO pointer arithmetic on void __iomem *
> - [High] phy: realtek: missing NULL check and iounmap() for of_iomap()
> - [High] phy: realtek: off-by-one array bounds check allows out-of-bounds access

Pre-existing issues in disguise, not relevant to this patch series at all.

Re: [PATCH v9 1/6] phy: realtek: usb2: introduce phy_reg_desc struct to the driver

[Rustam Adilov]

On 2026-10-16 18:15, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] usb: phy: unaligned IO pointer arithmetic on void __iomem *
> - [High] phy: realtek: missing NULL check and iounmap() for of_iomap()
> - [High] phy: realtek: off-by-one array bounds check allows out-of-bounds access

Pre-existing issues in disguise, not relevant to this patch series at all.

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

[PATCH v9 2/6] phy: realtek: usb2: introduce read and write functions to phy_reg_desc struct

[Rustam Adilov]

RTL9607C is a big endian SoC with little endian USB host controller but
vstatus register is from big endian memory region and thus existing
writel doesn't work. It needs either __raw_writel or iowrite32be
instead.

To handle this situation, introduce read and write function to the
phy_reg_desc struct and add a default variation that is going to be
ponted by the existing rtd_phy_reg_desc.

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 | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
index e8d0f0119ec0..5e483c853f02 100644
--- a/drivers/phy/realtek/phy-rtk-usb2.c
+++ b/drivers/phy/realtek/phy-rtk-usb2.c
@@ -64,6 +64,8 @@ struct phy_reg_desc {
 	int vstatus_offset;
 	u32 vstatus_busy;
 	u32 new_reg_req;
+	u32 (*read)(void __iomem *reg);
+	void (*write)(u32 val, void __iomem *reg);
 };
 
 struct phy_reg {
@@ -130,6 +132,16 @@ struct rtk_phy {
 	struct dentry *debug_dir;
 };
 
+static u32 rtk_usb2phy_read(void __iomem *reg)
+{
+	return readl(reg);
+}
+
+static void rtk_usb2phy_write(u32 val, void __iomem *reg)
+{
+	writel(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)
 {
@@ -204,8 +216,8 @@ 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 + reg_desc->vstatus_offset);
+	reg_desc->write((u32)data << shift_bits,
+		       reg_wrap_vstatus + reg_desc->vstatus_offset);
 
 	ret = utmi_wait_register(reg_gusb2phyacc0, reg_desc->vstatus_busy, 0);
 	if (ret)
@@ -988,7 +1000,6 @@ static int parse_phy_data(struct rtk_phy *rtk_phy)
 		phy_parameter->phy_reg.vstatus_index = index;
 		phy_parameter->phy_reg.desc = phy_cfg->reg_desc;
 
-
 		if (of_property_read_bool(np, "realtek,inverse-hstx-sync-clock"))
 			phy_parameter->inverse_hstx_sync_clock = true;
 		else
@@ -1078,6 +1089,8 @@ static const struct phy_reg_desc rtd_phy_reg_desc = {
 	.vstatus_offset = 0,
 	.vstatus_busy = PHY_VSTS_BUSY,
 	.new_reg_req = PHY_NEW_REG_REQ,
+	.read = rtk_usb2phy_read,
+	.write = rtk_usb2phy_write,
 };
 
 static const struct phy_cfg rtd1295_phy_cfg = {
-- 
2.56.0


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

[PATCH v9 2/6] phy: realtek: usb2: introduce read and write functions to phy_reg_desc struct

[Rustam Adilov]

RTL9607C is a big endian SoC with little endian USB host controller but
vstatus register is from big endian memory region and thus existing
writel doesn't work. It needs either __raw_writel or iowrite32be
instead.

To handle this situation, introduce read and write function to the
phy_reg_desc struct and add a default variation that is going to be
ponted by the existing rtd_phy_reg_desc.

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 | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
index e8d0f0119ec0..5e483c853f02 100644
--- a/drivers/phy/realtek/phy-rtk-usb2.c
+++ b/drivers/phy/realtek/phy-rtk-usb2.c
@@ -64,6 +64,8 @@ struct phy_reg_desc {
 	int vstatus_offset;
 	u32 vstatus_busy;
 	u32 new_reg_req;
+	u32 (*read)(void __iomem *reg);
+	void (*write)(u32 val, void __iomem *reg);
 };
 
 struct phy_reg {
@@ -130,6 +132,16 @@ struct rtk_phy {
 	struct dentry *debug_dir;
 };
 
+static u32 rtk_usb2phy_read(void __iomem *reg)
+{
+	return readl(reg);
+}
+
+static void rtk_usb2phy_write(u32 val, void __iomem *reg)
+{
+	writel(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)
 {
@@ -204,8 +216,8 @@ 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 + reg_desc->vstatus_offset);
+	reg_desc->write((u32)data << shift_bits,
+		       reg_wrap_vstatus + reg_desc->vstatus_offset);
 
 	ret = utmi_wait_register(reg_gusb2phyacc0, reg_desc->vstatus_busy, 0);
 	if (ret)
@@ -988,7 +1000,6 @@ static int parse_phy_data(struct rtk_phy *rtk_phy)
 		phy_parameter->phy_reg.vstatus_index = index;
 		phy_parameter->phy_reg.desc = phy_cfg->reg_desc;
 
-
 		if (of_property_read_bool(np, "realtek,inverse-hstx-sync-clock"))
 			phy_parameter->inverse_hstx_sync_clock = true;
 		else
@@ -1078,6 +1089,8 @@ static const struct phy_reg_desc rtd_phy_reg_desc = {
 	.vstatus_offset = 0,
 	.vstatus_busy = PHY_VSTS_BUSY,
 	.new_reg_req = PHY_NEW_REG_REQ,
+	.read = rtk_usb2phy_read,
+	.write = rtk_usb2phy_write,
 };
 
 static const struct phy_cfg rtd1295_phy_cfg = {
-- 
2.56.0

Re: [PATCH v9 2/6] phy: realtek: usb2: introduce read and write functions to phy_reg_desc struct

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] phy: missing NULL check for of_iomap() in parse_phy_data()
--

commit a9528213c2ade387d2252d526e2e1e2c2162eeba
Author: Rustam Adilov <adilov@disroot.org>

phy: realtek: usb2: introduce read and write functions to phy_reg_desc struct

RTL9607C is a big endian SoC with little endian USB host controller but
vstatus register is from big endian memory region and thus existing
writel doesn't work. It needs either __raw_writel or iowrite32be
instead.

To handle this situation, introduce read and write function to the
phy_reg_desc struct and add a default variation that is going to be
ponted by the existing rtd_phy_reg_desc.

> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
> index e8d0f0119ec0a..5e483c853f02f 100644
> --- a/drivers/phy/realtek/phy-rtk-usb2.c
> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
[ ... ]
> @@ -204,8 +216,8 @@ 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 + reg_desc->vstatus_offset);
> +	reg_desc->write((u32)data << shift_bits,
> +		       reg_wrap_vstatus + reg_desc->vstatus_offset);

[Severity: High]
Does this code risk dereferencing a NULL pointer if of_iomap() fails?

In parse_phy_data(), the results of of_iomap() are assigned to both
reg_wrap_vstatus and reg_gusb2phyacc0 without being checked for NULL:

parse_phy_data() {
    ...
    phy_parameter->phy_reg.reg_wrap_vstatus = of_iomap(np, 0);
    phy_parameter->phy_reg.reg_gusb2phyacc0 = of_iomap(np, 1) + index;
    ...
}

Later, in rtk_phy_write(), the reg_wrap_vstatus pointer is dereferenced here
when writing to the register. Should we verify that the device tree memory
mappings were successful before they are used?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016180314.31760-1-adilov@disroot.org?part=2

Re: [PATCH v9 2/6] phy: realtek: usb2: introduce read and write functions to phy_reg_desc struct

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] phy: missing NULL check for of_iomap() in parse_phy_data()
--

commit a9528213c2ade387d2252d526e2e1e2c2162eeba
Author: Rustam Adilov <adilov@disroot.org>

phy: realtek: usb2: introduce read and write functions to phy_reg_desc struct

RTL9607C is a big endian SoC with little endian USB host controller but
vstatus register is from big endian memory region and thus existing
writel doesn't work. It needs either __raw_writel or iowrite32be
instead.

To handle this situation, introduce read and write function to the
phy_reg_desc struct and add a default variation that is going to be
ponted by the existing rtd_phy_reg_desc.

> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
> index e8d0f0119ec0a..5e483c853f02f 100644
> --- a/drivers/phy/realtek/phy-rtk-usb2.c
> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
[ ... ]
> @@ -204,8 +216,8 @@ 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 + reg_desc->vstatus_offset);
> +	reg_desc->write((u32)data << shift_bits,
> +		       reg_wrap_vstatus + reg_desc->vstatus_offset);

[Severity: High]
Does this code risk dereferencing a NULL pointer if of_iomap() fails?

In parse_phy_data(), the results of of_iomap() are assigned to both
reg_wrap_vstatus and reg_gusb2phyacc0 without being checked for NULL:

parse_phy_data() {
    ...
    phy_parameter->phy_reg.reg_wrap_vstatus = of_iomap(np, 0);
    phy_parameter->phy_reg.reg_gusb2phyacc0 = of_iomap(np, 1) + index;
    ...
}

Later, in rtk_phy_write(), the reg_wrap_vstatus pointer is dereferenced here
when writing to the register. Should we verify that the device tree memory
mappings were successful before they are used?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016180314.31760-1-adilov@disroot.org?part=2

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

[PATCH v9 3/6] dt-bindings: phy: realtek,usb2phy: 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.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
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.56.0


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

[PATCH v9 3/6] dt-bindings: phy: realtek,usb2phy: 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.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
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.56.0

[PATCH v9 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_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.

Since the vendor usb driver developed by Realtek doesn't assert the reset
line (or in their case clear the register bit), we can reasonably assume
reset_control_assert is not needed here.

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 | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
index 5e483c853f02..2ddad7be7353 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 */
@@ -128,6 +129,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;
 };
@@ -676,6 +678,15 @@ static int rtk_phy_init(struct phy *phy)
 	if (!rtk_phy)
 		return -EINVAL;
 
+	if (rtk_phy->phy_rst) {
+		ret = reset_control_deassert(rtk_phy->phy_rst);
+
+		if (ret)
+			return ret;
+
+		usleep_range(5000, 6000);
+	}
+
 	for (i = 0; i < rtk_phy->num_phy; i++)
 		ret = do_rtk_phy_init(rtk_phy, i);
 
@@ -1055,6 +1066,11 @@ static int rtk_usb2phy_probe(struct platform_device *pdev)
 
 	rtk_phy->num_phy = phy_cfg->num_phy;
 
+	rtk_phy->phy_rst = devm_reset_control_get_optional_exclusive(dev, NULL);
+	if (IS_ERR(rtk_phy->phy_rst))
+		return dev_err_probe(dev, PTR_ERR(rtk_phy->phy_rst),
+				     "usb2 phy reset is not working\n");
+
 	ret = parse_phy_data(rtk_phy);
 	if (ret)
 		goto err;
-- 
2.56.0


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

[PATCH v9 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_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.

Since the vendor usb driver developed by Realtek doesn't assert the reset
line (or in their case clear the register bit), we can reasonably assume
reset_control_assert is not needed here.

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 | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
index 5e483c853f02..2ddad7be7353 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 */
@@ -128,6 +129,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;
 };
@@ -676,6 +678,15 @@ static int rtk_phy_init(struct phy *phy)
 	if (!rtk_phy)
 		return -EINVAL;
 
+	if (rtk_phy->phy_rst) {
+		ret = reset_control_deassert(rtk_phy->phy_rst);
+
+		if (ret)
+			return ret;
+
+		usleep_range(5000, 6000);
+	}
+
 	for (i = 0; i < rtk_phy->num_phy; i++)
 		ret = do_rtk_phy_init(rtk_phy, i);
 
@@ -1055,6 +1066,11 @@ static int rtk_usb2phy_probe(struct platform_device *pdev)
 
 	rtk_phy->num_phy = phy_cfg->num_phy;
 
+	rtk_phy->phy_rst = devm_reset_control_get_optional_exclusive(dev, NULL);
+	if (IS_ERR(rtk_phy->phy_rst))
+		return dev_err_probe(dev, PTR_ERR(rtk_phy->phy_rst),
+				     "usb2 phy reset is not working\n");
+
 	ret = parse_phy_data(rtk_phy);
 	if (ret)
 		goto err;
-- 
2.56.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] phy: realtek: register access before reset deassertion causes hang/timeout
- [Medium] phy: missing reset_control_assert in rtk_phy_exit and error paths
--

commit ae1074cfd8fee3a5506c0c9db06f187c80793713
Author: Rustam Adilov <adilov@disroot.org>

phy: realtek: usb2: introduce reset controller struct

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.

> phy: realtek: usb2: introduce reset controller struct
> 
> 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_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.
> 
> Since the vendor usb driver developed by Realtek doesn't assert the reset
> line (or in their case clear the register bit), we can reasonably assume
> reset_control_assert is not needed here.

[Severity: Medium]
Since this is an exclusive reset control, does it need to be asserted on
teardown even if the vendor driver doesn't do it?

If the reset line is never asserted during module removal or driver unbind
(such as in an rtk_phy_exit() function), could this lead to power leakage
or unexpected initial states during subsequent driver reloads?

> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
> index 5e483c853f02f..2ddad7be73534 100644
> --- a/drivers/phy/realtek/phy-rtk-usb2.c
> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
[ ... ]
> @@ -676,6 +678,15 @@ static int rtk_phy_init(struct phy *phy)
>  	if (!rtk_phy)
>  		return -EINVAL;
>  
> +	if (rtk_phy->phy_rst) {
> +		ret = reset_control_deassert(rtk_phy->phy_rst);
> +
> +		if (ret)
> +			return ret;
> +
> +		usleep_range(5000, 6000);
> +	}
> +
>  	for (i = 0; i < rtk_phy->num_phy; i++)
>  		ret = do_rtk_phy_init(rtk_phy, i);

[Severity: Medium]
If do_rtk_phy_init() fails, the reset line remains deasserted on the error
path. Should the reset line be asserted before returning the error?

[ ... ]
> @@ -1055,6 +1066,11 @@ static int rtk_usb2phy_probe(struct platform_device *pdev)
>  
>  	rtk_phy->num_phy = phy_cfg->num_phy;
>  
> +	rtk_phy->phy_rst = devm_reset_control_get_optional_exclusive(dev, NULL);
> +	if (IS_ERR(rtk_phy->phy_rst))
> +		return dev_err_probe(dev, PTR_ERR(rtk_phy->phy_rst),
> +				     "usb2 phy reset is not working\n");
> +
>  	ret = parse_phy_data(rtk_phy);

[Severity: High]
Can calling parse_phy_data() here before deasserting the reset control cause
a hardware hang or timeout?

Inside parse_phy_data(), if the SoC configuration lacks a pre-populated
default value for certain registers, functions like update_hs_clk_select()
and update_dc_driving_level() will attempt to read from the PHY registers:

drivers/phy/realtek/phy-rtk-usb2.c:update_hs_clk_select() {
	if (phy_parameter->inverse_hstx_sync_clock) {
		if (!phy_cfg->page0[6].addr) {
			rtk_phy_set_page(phy_reg, 0);
			phy_cfg->page0[6].addr = PAGE0_0XE6;
			phy_cfg->page0[6].data = rtk_phy_read(phy_reg, PAGE0_0XE6);
		}
	}
}

If the realtek,inverse-hstx-sync-clock DT property is set, it will read
from the unclocked or reset IP block, which could lock up the system bus
on many SoCs.

Would it be safer to defer these register reads until after the reset is
deasserted in rtk_phy_init()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016180314.31760-1-adilov@disroot.org?part=4

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] phy: realtek: register access before reset deassertion causes hang/timeout
- [Medium] phy: missing reset_control_assert in rtk_phy_exit and error paths
--

commit ae1074cfd8fee3a5506c0c9db06f187c80793713
Author: Rustam Adilov <adilov@disroot.org>

phy: realtek: usb2: introduce reset controller struct

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.

> phy: realtek: usb2: introduce reset controller struct
> 
> 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_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.
> 
> Since the vendor usb driver developed by Realtek doesn't assert the reset
> line (or in their case clear the register bit), we can reasonably assume
> reset_control_assert is not needed here.

[Severity: Medium]
Since this is an exclusive reset control, does it need to be asserted on
teardown even if the vendor driver doesn't do it?

If the reset line is never asserted during module removal or driver unbind
(such as in an rtk_phy_exit() function), could this lead to power leakage
or unexpected initial states during subsequent driver reloads?

> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
> index 5e483c853f02f..2ddad7be73534 100644
> --- a/drivers/phy/realtek/phy-rtk-usb2.c
> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
[ ... ]
> @@ -676,6 +678,15 @@ static int rtk_phy_init(struct phy *phy)
>  	if (!rtk_phy)
>  		return -EINVAL;
>  
> +	if (rtk_phy->phy_rst) {
> +		ret = reset_control_deassert(rtk_phy->phy_rst);
> +
> +		if (ret)
> +			return ret;
> +
> +		usleep_range(5000, 6000);
> +	}
> +
>  	for (i = 0; i < rtk_phy->num_phy; i++)
>  		ret = do_rtk_phy_init(rtk_phy, i);

[Severity: Medium]
If do_rtk_phy_init() fails, the reset line remains deasserted on the error
path. Should the reset line be asserted before returning the error?

[ ... ]
> @@ -1055,6 +1066,11 @@ static int rtk_usb2phy_probe(struct platform_device *pdev)
>  
>  	rtk_phy->num_phy = phy_cfg->num_phy;
>  
> +	rtk_phy->phy_rst = devm_reset_control_get_optional_exclusive(dev, NULL);
> +	if (IS_ERR(rtk_phy->phy_rst))
> +		return dev_err_probe(dev, PTR_ERR(rtk_phy->phy_rst),
> +				     "usb2 phy reset is not working\n");
> +
>  	ret = parse_phy_data(rtk_phy);

[Severity: High]
Can calling parse_phy_data() here before deasserting the reset control cause
a hardware hang or timeout?

Inside parse_phy_data(), if the SoC configuration lacks a pre-populated
default value for certain registers, functions like update_hs_clk_select()
and update_dc_driving_level() will attempt to read from the PHY registers:

drivers/phy/realtek/phy-rtk-usb2.c:update_hs_clk_select() {
	if (phy_parameter->inverse_hstx_sync_clock) {
		if (!phy_cfg->page0[6].addr) {
			rtk_phy_set_page(phy_reg, 0);
			phy_cfg->page0[6].addr = PAGE0_0XE6;
			phy_cfg->page0[6].data = rtk_phy_read(phy_reg, PAGE0_0XE6);
		}
	}
}

If the realtek,inverse-hstx-sync-clock DT property is set, it will read
from the unclocked or reset IP block, which could lock up the system bus
on many SoCs.

Would it be safer to defer these register reads until after the reset is
deasserted in rtk_phy_init()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016180314.31760-1-adilov@disroot.org?part=4

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

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

[Rustam Adilov]

On 2026-10-16 18:15, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] phy: realtek: register access before reset deassertion causes hang/timeout
> - [Medium] phy: missing reset_control_assert in rtk_phy_exit and error paths
> --
> 
> commit ae1074cfd8fee3a5506c0c9db06f187c80793713
> Author: Rustam Adilov <adilov@disroot.org>
> 
> phy: realtek: usb2: introduce reset controller struct
> 
> 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.
> 
>> phy: realtek: usb2: introduce reset controller struct
>> 
>> 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_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.
>> 
>> Since the vendor usb driver developed by Realtek doesn't assert the reset
>> line (or in their case clear the register bit), we can reasonably assume
>> reset_control_assert is not needed here.
> 
> [Severity: Medium]
> Since this is an exclusive reset control, does it need to be asserted on
> teardown even if the vendor driver doesn't do it?
> 
> If the reset line is never asserted during module removal or driver unbind
> (such as in an rtk_phy_exit() function), could this lead to power leakage
> or unexpected initial states during subsequent driver reloads?

There is still a lot unknown about the IP enable controller and what it really
does, the reset controller is just my best guess. I would ideally not deviate
from what the vendor usb driver did.

>> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
>> index 5e483c853f02f..2ddad7be73534 100644
>> --- a/drivers/phy/realtek/phy-rtk-usb2.c
>> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
> [ ... ]
>> @@ -676,6 +678,15 @@ static int rtk_phy_init(struct phy *phy)
>>  	if (!rtk_phy)
>>  		return -EINVAL;
>>  
>> +	if (rtk_phy->phy_rst) {
>> +		ret = reset_control_deassert(rtk_phy->phy_rst);
>> +
>> +		if (ret)
>> +			return ret;
>> +
>> +		usleep_range(5000, 6000);
>> +	}
>> +
>>  	for (i = 0; i < rtk_phy->num_phy; i++)
>>  		ret = do_rtk_phy_init(rtk_phy, i);
> 
> [Severity: Medium]
> If do_rtk_phy_init() fails, the reset line remains deasserted on the error
> path. Should the reset line be asserted before returning the error?
> 
> [ ... ]
>> @@ -1055,6 +1066,11 @@ static int rtk_usb2phy_probe(struct platform_device *pdev)
>>  
>>  	rtk_phy->num_phy = phy_cfg->num_phy;
>>  
>> +	rtk_phy->phy_rst = devm_reset_control_get_optional_exclusive(dev, NULL);
>> +	if (IS_ERR(rtk_phy->phy_rst))
>> +		return dev_err_probe(dev, PTR_ERR(rtk_phy->phy_rst),
>> +				     "usb2 phy reset is not working\n");
>> +
>>  	ret = parse_phy_data(rtk_phy);
> 
> [Severity: High]
> Can calling parse_phy_data() here before deasserting the reset control cause
> a hardware hang or timeout?
> 
> Inside parse_phy_data(), if the SoC configuration lacks a pre-populated
> default value for certain registers, functions like update_hs_clk_select()
> and update_dc_driving_level() will attempt to read from the PHY registers:
> 
> drivers/phy/realtek/phy-rtk-usb2.c:update_hs_clk_select() {
> 	if (phy_parameter->inverse_hstx_sync_clock) {
> 		if (!phy_cfg->page0[6].addr) {
> 			rtk_phy_set_page(phy_reg, 0);
> 			phy_cfg->page0[6].addr = PAGE0_0XE6;
> 			phy_cfg->page0[6].data = rtk_phy_read(phy_reg, PAGE0_0XE6);
> 		}
> 	}
> }
> 
> If the realtek,inverse-hstx-sync-clock DT property is set, it will read
> from the unclocked or reset IP block, which could lock up the system bus
> on many SoCs.

realtek,inverse-hstx-sync-clock property is irrelevant to rtl9607c.

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

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

[Rustam Adilov]

On 2026-10-16 18:15, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] phy: realtek: register access before reset deassertion causes hang/timeout
> - [Medium] phy: missing reset_control_assert in rtk_phy_exit and error paths
> --
> 
> commit ae1074cfd8fee3a5506c0c9db06f187c80793713
> Author: Rustam Adilov <adilov@disroot.org>
> 
> phy: realtek: usb2: introduce reset controller struct
> 
> 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.
> 
>> phy: realtek: usb2: introduce reset controller struct
>> 
>> 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_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.
>> 
>> Since the vendor usb driver developed by Realtek doesn't assert the reset
>> line (or in their case clear the register bit), we can reasonably assume
>> reset_control_assert is not needed here.
> 
> [Severity: Medium]
> Since this is an exclusive reset control, does it need to be asserted on
> teardown even if the vendor driver doesn't do it?
> 
> If the reset line is never asserted during module removal or driver unbind
> (such as in an rtk_phy_exit() function), could this lead to power leakage
> or unexpected initial states during subsequent driver reloads?

There is still a lot unknown about the IP enable controller and what it really
does, the reset controller is just my best guess. I would ideally not deviate
from what the vendor usb driver did.

>> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
>> index 5e483c853f02f..2ddad7be73534 100644
>> --- a/drivers/phy/realtek/phy-rtk-usb2.c
>> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
> [ ... ]
>> @@ -676,6 +678,15 @@ static int rtk_phy_init(struct phy *phy)
>>  	if (!rtk_phy)
>>  		return -EINVAL;
>>  
>> +	if (rtk_phy->phy_rst) {
>> +		ret = reset_control_deassert(rtk_phy->phy_rst);
>> +
>> +		if (ret)
>> +			return ret;
>> +
>> +		usleep_range(5000, 6000);
>> +	}
>> +
>>  	for (i = 0; i < rtk_phy->num_phy; i++)
>>  		ret = do_rtk_phy_init(rtk_phy, i);
> 
> [Severity: Medium]
> If do_rtk_phy_init() fails, the reset line remains deasserted on the error
> path. Should the reset line be asserted before returning the error?
> 
> [ ... ]
>> @@ -1055,6 +1066,11 @@ static int rtk_usb2phy_probe(struct platform_device *pdev)
>>  
>>  	rtk_phy->num_phy = phy_cfg->num_phy;
>>  
>> +	rtk_phy->phy_rst = devm_reset_control_get_optional_exclusive(dev, NULL);
>> +	if (IS_ERR(rtk_phy->phy_rst))
>> +		return dev_err_probe(dev, PTR_ERR(rtk_phy->phy_rst),
>> +				     "usb2 phy reset is not working\n");
>> +
>>  	ret = parse_phy_data(rtk_phy);
> 
> [Severity: High]
> Can calling parse_phy_data() here before deasserting the reset control cause
> a hardware hang or timeout?
> 
> Inside parse_phy_data(), if the SoC configuration lacks a pre-populated
> default value for certain registers, functions like update_hs_clk_select()
> and update_dc_driving_level() will attempt to read from the PHY registers:
> 
> drivers/phy/realtek/phy-rtk-usb2.c:update_hs_clk_select() {
> 	if (phy_parameter->inverse_hstx_sync_clock) {
> 		if (!phy_cfg->page0[6].addr) {
> 			rtk_phy_set_page(phy_reg, 0);
> 			phy_cfg->page0[6].addr = PAGE0_0XE6;
> 			phy_cfg->page0[6].data = rtk_phy_read(phy_reg, PAGE0_0XE6);
> 		}
> 	}
> }
> 
> If the realtek,inverse-hstx-sync-clock DT property is set, it will read
> from the unclocked or reset IP block, which could lock up the system bus
> on many SoCs.

realtek,inverse-hstx-sync-clock property is irrelevant to rtl9607c.

[PATCH v9 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. Since it belongs into
the vstatus register region, it requires the use of added read and
write helper functions.

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

Add the appropriate big endian read and write functions for rtl9607
and assign them to its rtl9607_phy_reg_desc struct.

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 | 66 ++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
index 2ddad7be7353..64076938c821 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)
 
@@ -107,6 +113,7 @@ struct phy_cfg {
 	bool use_default_parameter;
 	bool is_double_sensitivity_mode;
 	const struct phy_reg_desc *reg_desc;
+	bool force_host_disconnect;
 };
 
 struct phy_parameter {
@@ -144,6 +151,16 @@ static void rtk_usb2phy_write(u32 val, void __iomem *reg)
 	writel(val, reg);
 }
 
+static u32 rtk_usb2phy_read_be(void __iomem *reg)
+{
+	return ioread32be(reg);
+}
+
+static void rtk_usb2phy_write_be(u32 val, void __iomem *reg)
+{
+	iowrite32be(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)
 {
@@ -600,6 +617,23 @@ static int do_rtk_phy_init(struct rtk_phy *rtk_phy, int index)
 		goto do_toggle;
 	}
 
+	if (phy_cfg->force_host_disconnect) {
+		const struct phy_reg_desc *reg_desc = phy_reg->desc;
+		void __iomem *vstatus = phy_reg->reg_wrap_vstatus;
+		u32 temp;
+
+		temp = reg_desc->read(vstatus + PHY_9607_FORCE_DISCONNECT_REG);
+
+		/* disable force-host-disconnect */
+		temp &= ~PHY_9607_FORCE_DISCONNECT_BIT;
+
+		reg_desc->write(temp, vstatus + PHY_9607_FORCE_DISCONNECT_REG);
+		temp = reg_desc->read(vstatus + PHY_9607_FORCE_DISCONNECT_REG);
+
+		/* allow IP to startup */
+		usleep_range(10000, 11000);
+	}
+
 	/* Set page 0 */
 	phy_data_page = phy_cfg->page0;
 	rtk_phy_set_page(phy_reg, 0);
@@ -1109,6 +1143,14 @@ static const struct phy_reg_desc rtd_phy_reg_desc = {
 	.write = rtk_usb2phy_write,
 };
 
+static const struct phy_reg_desc rtl9607_phy_reg_desc = {
+	.vstatus_offset = 0xc,
+	.vstatus_busy = PHY_9607_VSTS_BUSY,
+	.new_reg_req = PHY_9607_NEW_REG_REQ,
+	.read = rtk_usb2phy_read_be,
+	.write = rtk_usb2phy_write_be,
+};
+
 static const struct phy_cfg rtd1295_phy_cfg = {
 	.page0_size = MAX_USB_PHY_PAGE0_DATA_SIZE,
 	.page0 = { [0] = {0xe0, 0x90},
@@ -1342,6 +1384,29 @@ static const struct phy_cfg rtd1315e_phy_cfg = {
 	.reg_desc = &rtd_phy_reg_desc,
 };
 
+static const struct phy_cfg rtl9607_phy_cfg = {
+	.page0_size = MAX_USB_PHY_PAGE0_DATA_SIZE,
+	.page0 = { [0] = {0xe0, 0x95},
+		   [4] = {0xe4, 0x6a},
+		  [11] = {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_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,
+	.driving_updated_for_dev_dis = 0x8,
+	.is_double_sensitivity_mode = true,
+	.reg_desc = &rtl9607_phy_reg_desc,
+	.force_host_disconnect = true,
+};
+
 static const struct of_device_id usbphy_rtk_dt_match[] = {
 	{ .compatible = "realtek,rtd1295-usb2phy", .data = &rtd1295_phy_cfg },
 	{ .compatible = "realtek,rtd1312c-usb2phy", .data = &rtd1312c_phy_cfg },
@@ -1352,6 +1417,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.56.0


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

[PATCH v9 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. Since it belongs into
the vstatus register region, it requires the use of added read and
write helper functions.

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

Add the appropriate big endian read and write functions for rtl9607
and assign them to its rtl9607_phy_reg_desc struct.

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 | 66 ++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
index 2ddad7be7353..64076938c821 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)
 
@@ -107,6 +113,7 @@ struct phy_cfg {
 	bool use_default_parameter;
 	bool is_double_sensitivity_mode;
 	const struct phy_reg_desc *reg_desc;
+	bool force_host_disconnect;
 };
 
 struct phy_parameter {
@@ -144,6 +151,16 @@ static void rtk_usb2phy_write(u32 val, void __iomem *reg)
 	writel(val, reg);
 }
 
+static u32 rtk_usb2phy_read_be(void __iomem *reg)
+{
+	return ioread32be(reg);
+}
+
+static void rtk_usb2phy_write_be(u32 val, void __iomem *reg)
+{
+	iowrite32be(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)
 {
@@ -600,6 +617,23 @@ static int do_rtk_phy_init(struct rtk_phy *rtk_phy, int index)
 		goto do_toggle;
 	}
 
+	if (phy_cfg->force_host_disconnect) {
+		const struct phy_reg_desc *reg_desc = phy_reg->desc;
+		void __iomem *vstatus = phy_reg->reg_wrap_vstatus;
+		u32 temp;
+
+		temp = reg_desc->read(vstatus + PHY_9607_FORCE_DISCONNECT_REG);
+
+		/* disable force-host-disconnect */
+		temp &= ~PHY_9607_FORCE_DISCONNECT_BIT;
+
+		reg_desc->write(temp, vstatus + PHY_9607_FORCE_DISCONNECT_REG);
+		temp = reg_desc->read(vstatus + PHY_9607_FORCE_DISCONNECT_REG);
+
+		/* allow IP to startup */
+		usleep_range(10000, 11000);
+	}
+
 	/* Set page 0 */
 	phy_data_page = phy_cfg->page0;
 	rtk_phy_set_page(phy_reg, 0);
@@ -1109,6 +1143,14 @@ static const struct phy_reg_desc rtd_phy_reg_desc = {
 	.write = rtk_usb2phy_write,
 };
 
+static const struct phy_reg_desc rtl9607_phy_reg_desc = {
+	.vstatus_offset = 0xc,
+	.vstatus_busy = PHY_9607_VSTS_BUSY,
+	.new_reg_req = PHY_9607_NEW_REG_REQ,
+	.read = rtk_usb2phy_read_be,
+	.write = rtk_usb2phy_write_be,
+};
+
 static const struct phy_cfg rtd1295_phy_cfg = {
 	.page0_size = MAX_USB_PHY_PAGE0_DATA_SIZE,
 	.page0 = { [0] = {0xe0, 0x90},
@@ -1342,6 +1384,29 @@ static const struct phy_cfg rtd1315e_phy_cfg = {
 	.reg_desc = &rtd_phy_reg_desc,
 };
 
+static const struct phy_cfg rtl9607_phy_cfg = {
+	.page0_size = MAX_USB_PHY_PAGE0_DATA_SIZE,
+	.page0 = { [0] = {0xe0, 0x95},
+		   [4] = {0xe4, 0x6a},
+		  [11] = {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_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,
+	.driving_updated_for_dev_dis = 0x8,
+	.is_double_sensitivity_mode = true,
+	.reg_desc = &rtl9607_phy_reg_desc,
+	.force_host_disconnect = true,
+};
+
 static const struct of_device_id usbphy_rtk_dt_match[] = {
 	{ .compatible = "realtek,rtd1295-usb2phy", .data = &rtd1295_phy_cfg },
 	{ .compatible = "realtek,rtd1312c-usb2phy", .data = &rtd1312c_phy_cfg },
@@ -1352,6 +1417,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.56.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] phy: realtek: missing of_iomap() check leads to NULL dereference
--

commit 69e22da529f3cad44ca97c0c6f0a80e03f43ad7e
Author: Rustam Adilov <adilov@disroot.org>

phy: realtek: usb2: add support for RTL9607C USB2 PHY

This commit adds support for the USB2 PHY on RTL9607C series SoCs. It
includes new macros, configuration structures, big-endian access functions,
and logic to clear a force host disconnect bit during initialization.

> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
> index 2ddad7be73534..64076938c821d 100644
> --- a/drivers/phy/realtek/phy-rtk-usb2.c
> +++ b/drivers/phy/realtek/phy-rtk-usb2.c

[ ... ]

> @@ -600,6 +617,23 @@ static int do_rtk_phy_init(struct rtk_phy *rtk_phy, int index)
>  		goto do_toggle;
>  	}
>  
> +	if (phy_cfg->force_host_disconnect) {
> +		const struct phy_reg_desc *reg_desc = phy_reg->desc;
> +		void __iomem *vstatus = phy_reg->reg_wrap_vstatus;
> +		u32 temp;
> +
> +		temp = reg_desc->read(vstatus + PHY_9607_FORCE_DISCONNECT_REG);

[Severity: High]
Can this result in a NULL pointer dereference?

If of_iomap() fails to map device memory, it returns NULL which is stored
without validation:

drivers/phy/realtek/phy-rtk-usb2.c:parse_phy_data() {
    ...
    phy_parameter->phy_reg.reg_wrap_vstatus = of_iomap(np, 0);
    ...
}

This new code in do_rtk_phy_init() relies on this pointer by dereferencing
vstatus via the read function. Does this need an error check during the
initial mapping or before this dereference?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016180314.31760-1-adilov@disroot.org?part=5

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] phy: realtek: missing of_iomap() check leads to NULL dereference
--

commit 69e22da529f3cad44ca97c0c6f0a80e03f43ad7e
Author: Rustam Adilov <adilov@disroot.org>

phy: realtek: usb2: add support for RTL9607C USB2 PHY

This commit adds support for the USB2 PHY on RTL9607C series SoCs. It
includes new macros, configuration structures, big-endian access functions,
and logic to clear a force host disconnect bit during initialization.

> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
> index 2ddad7be73534..64076938c821d 100644
> --- a/drivers/phy/realtek/phy-rtk-usb2.c
> +++ b/drivers/phy/realtek/phy-rtk-usb2.c

[ ... ]

> @@ -600,6 +617,23 @@ static int do_rtk_phy_init(struct rtk_phy *rtk_phy, int index)
>  		goto do_toggle;
>  	}
>  
> +	if (phy_cfg->force_host_disconnect) {
> +		const struct phy_reg_desc *reg_desc = phy_reg->desc;
> +		void __iomem *vstatus = phy_reg->reg_wrap_vstatus;
> +		u32 temp;
> +
> +		temp = reg_desc->read(vstatus + PHY_9607_FORCE_DISCONNECT_REG);

[Severity: High]
Can this result in a NULL pointer dereference?

If of_iomap() fails to map device memory, it returns NULL which is stored
without validation:

drivers/phy/realtek/phy-rtk-usb2.c:parse_phy_data() {
    ...
    phy_parameter->phy_reg.reg_wrap_vstatus = of_iomap(np, 0);
    ...
}

This new code in do_rtk_phy_init() relies on this pointer by dereferencing
vstatus via the read function. Does this need an error check during the
initial mapping or before this dereference?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016180314.31760-1-adilov@disroot.org?part=5

[PATCH v9 6/6] phy: realtek: 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.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Signed-off-by: Rustam Adilov <adilov@disroot.org>
---
 drivers/phy/realtek/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/realtek/Kconfig b/drivers/phy/realtek/Kconfig
index 75ac7e7c31ae..76f9215d8b94 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"
@@ -29,4 +29,4 @@ config PHY_RTK_RTD_USB3PHY
 	  DWC3 USB IP. This driver will do the PHY initialization
 	  of the parameters.
 
-endif # ARCH_REALTEK || COMPILE_TEST
+endif # ARCH_REALTEK || MACH_REALTEK_RTL || COMPILE_TEST
-- 
2.56.0


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

[PATCH v9 6/6] phy: realtek: 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.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Signed-off-by: Rustam Adilov <adilov@disroot.org>
---
 drivers/phy/realtek/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/realtek/Kconfig b/drivers/phy/realtek/Kconfig
index 75ac7e7c31ae..76f9215d8b94 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"
@@ -29,4 +29,4 @@ config PHY_RTK_RTD_USB3PHY
 	  DWC3 USB IP. This driver will do the PHY initialization
 	  of the parameters.
 
-endif # ARCH_REALTEK || COMPILE_TEST
+endif # ARCH_REALTEK || MACH_REALTEK_RTL || COMPILE_TEST
-- 
2.56.0