* [PATCH v6 1/6] phy: realtek: usb2: introduce vstatus/new_reg_req variables to driver data
2026-05-20 17:57 [PATCH v6 0/6] phy: realtek: usb2: support for RTL9607C USB2 PHY Rustam Adilov
@ 2026-05-20 17:57 ` Rustam Adilov
2026-05-20 18:23 ` sashiko-bot
2026-05-20 17:57 ` [PATCH v6 2/6] phy: realtek: usb2: introduce read and write functions " Rustam Adilov
` (4 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Rustam Adilov @ 2026-05-20 17:57 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Stanley Chang, Philipp Zabel, linux-phy, devicetree,
linux-kernel
Cc: Rustam Adilov, Michael Zavertkin
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 | 68 +++++++++++++++++++++++-------
1 file changed, 53 insertions(+), 15 deletions(-)
diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
index 248550ef98ca..bd9ee895b9f9 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;
+ u32 vstatus_busy;
+ u32 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;
+ u32 vstatus_busy;
+ u32 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);
+ /* VCtrl = low nibble of addr, and set phy_reg->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_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);
+ /* VCtrl = high nibble of addr, and set phy_reg->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_VSTS_BUSY, 0);
+ ret = utmi_wait_register(reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
if (ret)
return (char)ret;
@@ -194,25 +200,26 @@ 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);
+ /* VCtrl = low nibble of addr, set phy_reg->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_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);
+ /* VCtrl = high nibble of addr, set phy_reg->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_VSTS_BUSY, 0);
+ ret = utmi_wait_register(reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
if (ret)
return ret;
@@ -957,6 +964,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 +982,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 +1096,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 +1123,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 +1150,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 +1175,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 +1204,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 +1232,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 +1260,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 +1288,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 +1317,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.54.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v6 1/6] phy: realtek: usb2: introduce vstatus/new_reg_req variables to driver data
2026-05-20 17:57 ` [PATCH v6 1/6] phy: realtek: usb2: introduce vstatus/new_reg_req variables to driver data Rustam Adilov
@ 2026-05-20 18:23 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-05-20 18:23 UTC (permalink / raw)
To: Rustam Adilov
Cc: neil.armstrong, robh, devicetree, olteanv, linux-phy, vkoul,
conor+dt
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the driver fails to check the return value of `of_iomap()`, which can lead to a NULL pointer dereference.
- [High] This problem wasn't introduced by this patch, but incorrect pointer arithmetic on a `void __iomem *` pointer results in unaligned I/O memory accesses for multi-port setups.
- [High] This problem wasn't introduced by this patch, but I/O memory mappings created by `of_iomap()` are never unmapped, leading to resource leaks.
- [High] This problem wasn't introduced by this patch, but multi-port PHY configurations concurrently modify shared hardware registers without synchronization or a read-modify-write cycle.
--
commit 734507efb75a6be151dcde389bfc7f0e1df95132
Author: Rustam Adilov <adilov@disroot.org>
phy: realtek: usb2: introduce vstatus/new_reg_req variables to driver data
This commit introduces vstatus_offset, vstatus_busy, and new_reg_req variables
to the driver data to handle different register offsets and busy bit
definitions in the RTL9607C SoC. These variables are added to the existing
PHY configuration structures and assigned default values.
> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
> index 248550ef98cab..bd9ee895b9f90 100644
> --- a/drivers/phy/realtek/phy-rtk-usb2.c
> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
[ ... ]
> @@ -194,25 +200,26 @@ 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);
[Severity: High]
This is a pre-existing issue, but does writing directly to this register
without a read-modify-write cycle overwrite the configuration for other
ports?
In multi-port setups, all PHY instances appear to share this same base
address, and a direct writel() could clear the bits belonging to other PHYs.
Is there also a need for synchronization to prevent concurrent writes across
different ports?
>
> - 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;
[ ... ]
> @@ -974,6 +982,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;
[Severity: High]
This isn't a bug introduced by this patch, but does the driver need to
check the return value of of_iomap() here?
If the mapping fails, this would leave the pointers as NULL and cause a
dereference during I/O operations.
[Severity: High]
This is a pre-existing issue, but does adding index directly to the
void __iomem * returned by of_iomap() result in exactly a 1-byte offset
when index > 0?
If so, this could cause an unaligned memory access fault on ARM
architectures. Should the index be scaled appropriately?
[Severity: High]
This is also a pre-existing issue, but does the driver leak these I/O
mappings?
The driver maps the memory here using of_iomap(), but there does not
appear to be a matching iounmap() in the driver cleanup paths. Would
using devm_of_iomap() prevent this leak?
> 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"))
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520175728.720877-1-adilov@disroot.org?part=1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v6 2/6] phy: realtek: usb2: introduce read and write functions to driver data
2026-05-20 17:57 [PATCH v6 0/6] phy: realtek: usb2: support for RTL9607C USB2 PHY Rustam Adilov
2026-05-20 17:57 ` [PATCH v6 1/6] phy: realtek: usb2: introduce vstatus/new_reg_req variables to driver data Rustam Adilov
@ 2026-05-20 17:57 ` Rustam Adilov
2026-05-20 18:40 ` sashiko-bot
2026-05-20 17:57 ` [PATCH v6 3/6] dt-bindings: phy: realtek,usb2phy.yaml: extend for resets and RTL9607C support Rustam Adilov
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Rustam Adilov @ 2026-05-20 17:57 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Stanley Chang, Philipp Zabel, linux-phy, devicetree,
linux-kernel
Cc: Rustam Adilov, Michael Zavertkin
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
driver data and create a default variation for the current RTD SoCs.
Assign the existing phy configuration for RTD SoCs to the default
read and write function.
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 | 40 ++++++++++++++++++++++++++++--
1 file changed, 38 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
index bd9ee895b9f9..a9e29d2f7e6f 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;
u32 vstatus_busy;
u32 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;
u32 vstatus_busy;
u32 new_reg_req;
+
+ u32 (*read)(void __iomem *reg);
+ void (*write)(u32 val, void __iomem *reg);
};
struct phy_parameter {
@@ -128,6 +134,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)
{
@@ -200,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 + phy_reg->vstatus_offset);
+ phy_reg->write((u32)data << shift_bits,
+ reg_wrap_vstatus + phy_reg->vstatus_offset);
ret = utmi_wait_register(reg_gusb2phyacc0, phy_reg->vstatus_busy, 0);
if (ret)
@@ -985,6 +1001,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;
@@ -1099,6 +1117,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 = rtk_usb2phy_read,
+ .write = rtk_usb2phy_write,
};
static const struct phy_cfg rtd1395_phy_cfg = {
@@ -1126,6 +1146,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 = rtk_usb2phy_read,
+ .write = rtk_usb2phy_write,
};
static const struct phy_cfg rtd1395_phy_cfg_2port = {
@@ -1153,6 +1175,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 = rtk_usb2phy_read,
+ .write = rtk_usb2phy_write,
};
static const struct phy_cfg rtd1619_phy_cfg = {
@@ -1178,6 +1202,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 = rtk_usb2phy_read,
+ .write = rtk_usb2phy_write,
};
static const struct phy_cfg rtd1319_phy_cfg = {
@@ -1207,6 +1233,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 = rtk_usb2phy_read,
+ .write = rtk_usb2phy_write,
};
static const struct phy_cfg rtd1312c_phy_cfg = {
@@ -1235,6 +1263,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 = rtk_usb2phy_read,
+ .write = rtk_usb2phy_write,
};
static const struct phy_cfg rtd1619b_phy_cfg = {
@@ -1263,6 +1293,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 = rtk_usb2phy_read,
+ .write = rtk_usb2phy_write,
};
static const struct phy_cfg rtd1319d_phy_cfg = {
@@ -1291,6 +1323,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 = rtk_usb2phy_read,
+ .write = rtk_usb2phy_write,
};
static const struct phy_cfg rtd1315e_phy_cfg = {
@@ -1320,6 +1354,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 = rtk_usb2phy_read,
+ .write = rtk_usb2phy_write,
};
static const struct of_device_id usbphy_rtk_dt_match[] = {
--
2.54.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v6 2/6] phy: realtek: usb2: introduce read and write functions to driver data
2026-05-20 17:57 ` [PATCH v6 2/6] phy: realtek: usb2: introduce read and write functions " Rustam Adilov
@ 2026-05-20 18:40 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-05-20 18:40 UTC (permalink / raw)
To: Rustam Adilov
Cc: vkoul, olteanv, robh, linux-phy, conor+dt, devicetree,
neil.armstrong
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] The return values of `of_iomap()` are not checked, which can lead to a NULL pointer dereference.
- [High] MMIO regions mapped with `of_iomap()` are never unmapped, and are redundantly mapped in a loop.
- [High] Incorrect pointer arithmetic on a `void __iomem *` leads to unaligned memory accesses for multi-PHY configurations.
--
commit cc0b13f36e44c7fb7f45ae90cdfa2f1c10a50993
Author: Rustam Adilov <adilov@disroot.org>
phy: realtek: usb2: introduce read and write functions to driver data
This commit introduces read and write function pointers to the driver data
to handle RTL9607C, which has a little endian USB host controller but a big
endian vstatus register. It also provides default accessors for existing SoCs.
> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
> index bd9ee895b9f90..a9e29d2f7e6fd 100644
> --- a/drivers/phy/realtek/phy-rtk-usb2.c
> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
[ ... ]
> @@ -985,6 +1001,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;
[Severity: High]
This isn't a bug introduced by this patch, but looking at the earlier lines in
this loop:
phy_parameter->phy_reg.reg_wrap_vstatus = of_iomap(np, 0);
phy_parameter->phy_reg.reg_gusb2phyacc0 = of_iomap(np, 1) + index;
Could we add a check to verify if of_iomap() returns NULL? Dereferencing a
NULL pointer during subsequent I/O operations will cause a kernel panic
during device probe.
[Severity: High]
This is a pre-existing issue, but does this code cause a resource leak?
The of_iomap() function is called inside a for loop that iterates
rtk_phy->num_phy times. For configurations with multiple PHYs (like
rtd1395_phy_cfg_2port), this maps the exact same device tree resources
multiple times.
There appear to be no corresponding iounmap() calls in the driver's error
handling paths or the remove function, which could lead to a permanent
vmalloc space resource leak upon driver unbind or probe failure.
[Severity: High]
This is a pre-existing issue, but could this pointer arithmetic lead to
unaligned memory accesses?
phy_parameter->phy_reg.reg_gusb2phyacc0 = of_iomap(np, 1) + index;
Since of_iomap() returns a void __iomem *, pointer arithmetic is performed
in single bytes. For multi-PHY configurations where index is 1, this
increments the base address by exactly 1 byte, resulting in an unaligned
32-bit offset that could cause an alignment fault during writel().
> + 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;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520175728.720877-1-adilov@disroot.org?part=2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v6 3/6] dt-bindings: phy: realtek,usb2phy.yaml: extend for resets and RTL9607C support
2026-05-20 17:57 [PATCH v6 0/6] phy: realtek: usb2: support for RTL9607C USB2 PHY Rustam Adilov
2026-05-20 17:57 ` [PATCH v6 1/6] phy: realtek: usb2: introduce vstatus/new_reg_req variables to driver data Rustam Adilov
2026-05-20 17:57 ` [PATCH v6 2/6] phy: realtek: usb2: introduce read and write functions " Rustam Adilov
@ 2026-05-20 17:57 ` Rustam Adilov
2026-05-20 17:57 ` [PATCH v6 4/6] phy: realtek: usb2: introduce reset controller struct Rustam Adilov
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Rustam Adilov @ 2026-05-20 17:57 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Stanley Chang, Philipp Zabel, linux-phy, devicetree,
linux-kernel
Cc: Rustam Adilov, Krzysztof Kozlowski
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>
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.54.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v6 4/6] phy: realtek: usb2: introduce reset controller struct
2026-05-20 17:57 [PATCH v6 0/6] phy: realtek: usb2: support for RTL9607C USB2 PHY Rustam Adilov
` (2 preceding siblings ...)
2026-05-20 17:57 ` [PATCH v6 3/6] dt-bindings: phy: realtek,usb2phy.yaml: extend for resets and RTL9607C support Rustam Adilov
@ 2026-05-20 17:57 ` Rustam Adilov
2026-05-20 19:16 ` sashiko-bot
2026-05-20 17:57 ` [PATCH v6 5/6] phy: realtek: usb2: add support for RTL9607C USB2 PHY Rustam Adilov
2026-05-20 17:57 ` [PATCH v6 6/6] phy: realtek: Make configs available for MACH_REALTEK_RTL Rustam Adilov
5 siblings, 1 reply; 11+ messages in thread
From: Rustam Adilov @ 2026-05-20 17:57 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Stanley Chang, Philipp Zabel, linux-phy, devicetree,
linux-kernel
Cc: Rustam Adilov, Michael Zavertkin
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.
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 a9e29d2f7e6f..16c5fc3191de 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;
};
@@ -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);
@@ -1059,6 +1070,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_array_get_optional_exclusive(dev);
+ if (IS_ERR(rtk_phy->phy_rst))
+ return dev_err_probe(dev, PTR_ERR(rtk_phy->phy_rst),
+ "usb2 phy resets are not working\n");
+
ret = parse_phy_data(rtk_phy);
if (ret)
goto err;
--
2.54.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v6 4/6] phy: realtek: usb2: introduce reset controller struct
2026-05-20 17:57 ` [PATCH v6 4/6] phy: realtek: usb2: introduce reset controller struct Rustam Adilov
@ 2026-05-20 19:16 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-05-20 19:16 UTC (permalink / raw)
To: Rustam Adilov
Cc: olteanv, neil.armstrong, linux-phy, devicetree, vkoul, conor+dt,
robh
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [High] The driver attempts to access the PHY's memory-mapped registers in `probe()` before the newly introduced reset control is deasserted.
Pre-existing issues:
- [Critical] An off-by-one error in `rtk_phy_toggle` allows out-of-bounds access to the `phy_parameter` array.
- [High] The loop in `rtk_phy_init()` silently ignores initialization errors from all but the last PHY.
- [High] Pointer arithmetic on `void __iomem *` leads to unaligned I/O access and duplicated memory mappings.
- [High] Per-port PHY properties are parsed in a loop and written directly into the globally shared `rtk_phy->phy_cfg` array, overwriting settings from earlier ports.
--
commit adbf8b08353248c649e3e87e6816cbc9f57abfab
Author: Rustam Adilov <adilov@disroot.org>
phy: realtek: usb2: introduce reset controller struct
In RTL9607C, there is an IP Enable Controller which acts like a
reset controller. This introduces a reset_control struct to handle
deasserting the usb2 phy reset line using the reset framework.
> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
> index a9e29d2f7e6fd..16c5fc3191dec 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: High]
This is a pre-existing issue, but does this loop silently ignore
initialization errors from all but the last PHY?
If an error occurs on any PHY other than the last one, the ret variable
is overwritten by the result of the subsequent iteration. Could this
falsely return success even if earlier ports failed to initialize?
[Severity: Critical]
This is a pre-existing issue, but is there an out-of-bounds access
in the toggle function called during initialization?
When do_rtk_phy_init() calls the toggle function for a given index, the
target function checks if index > rtk_phy->num_phy instead of
index >= rtk_phy->num_phy. Since the phy_parameter array is allocated
with rtk_phy->num_phy elements, could passing index == num_phy bypass
the bounds check and lead to memory corruption or arbitrary memory
access via I/O operations?
[ ... ]
> @@ -1059,6 +1070,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_array_get_optional_exclusive(dev);
> + if (IS_ERR(rtk_phy->phy_rst))
> + return dev_err_probe(dev, PTR_ERR(rtk_phy->phy_rst),
> + "usb2 phy resets are not working\n");
> +
> ret = parse_phy_data(rtk_phy);
[Severity: High]
Does parse_phy_data() attempt to access the PHY's memory-mapped registers
before the newly introduced reset control is deasserted?
It appears that the driver acquires the reset in probe() and then
immediately calls parse_phy_data(), which executes hardware read/write
operations via update_dc_driving_level() and update_hs_clk_select().
Because the reset is only deasserted later in rtk_phy_init(), could
accessing registers of an IP block held in reset trigger a synchronous
bus error or silently return zeroes, preventing proper initialization?
[Severity: High]
This is a pre-existing issue, but does pointer arithmetic on void __iomem *
lead to unaligned I/O access and duplicated memory mappings in
parse_phy_data()?
Looking at the loop in parse_phy_data():
for (index = 0; index < rtk_phy->num_phy; index++) {
...
phy_parameter->phy_reg.reg_gusb2phyacc0 = of_iomap(np, 1) + index;
...
}
Since of_iomap() returns a void __iomem *, does adding index advance the
pointer by exactly 1 byte? Could this result in an unaligned pointer for
subsequent 32-bit readl() or writel() operations and trigger an alignment
fault?
Also, does calling of_iomap() repeatedly inside the loop create redundant,
duplicate mappings of the exact same physical resource?
[Severity: High]
This is a pre-existing issue, but are per-port PHY properties parsed in a
loop and written directly into the globally shared rtk_phy->phy_cfg array,
overwriting settings from earlier ports?
In parse_phy_data(), the driver iterates over all PHY ports and retrieves
port-specific parameters. Functions like update_dc_driving_level() modify
the globally shared array.
Will properties computed for port 1 silently overwrite those computed
for port 0? When rtk_phy_init() is later called, do_rtk_phy_init() applies
this globally shared array to all ports, which seems like it might break
per-port configurations for any SoC with num_phy > 1.
> if (ret)
> goto err;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520175728.720877-1-adilov@disroot.org?part=4
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v6 5/6] phy: realtek: usb2: add support for RTL9607C USB2 PHY
2026-05-20 17:57 [PATCH v6 0/6] phy: realtek: usb2: support for RTL9607C USB2 PHY Rustam Adilov
` (3 preceding siblings ...)
2026-05-20 17:57 ` [PATCH v6 4/6] phy: realtek: usb2: introduce reset controller struct Rustam Adilov
@ 2026-05-20 17:57 ` Rustam Adilov
2026-05-20 19:51 ` sashiko-bot
2026-05-20 17:57 ` [PATCH v6 6/6] phy: realtek: Make configs available for MACH_REALTEK_RTL Rustam Adilov
5 siblings, 1 reply; 11+ messages in thread
From: Rustam Adilov @ 2026-05-20 17:57 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Stanley Chang, Philipp Zabel, linux-phy, devicetree,
linux-kernel
Cc: Rustam Adilov, Michael Zavertkin
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 phy config 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 | 58 ++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
index 16c5fc3191de..69f0f5279b5e 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 {
@@ -146,6 +153,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,19 @@ 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 */
+ void __iomem *vstatus = phy_reg->reg_wrap_vstatus;
+ u32 temp;
+
+ temp = phy_reg->read(vstatus + PHY_9607_FORCE_DISCONNECT_REG);
+
+ temp &= ~PHY_9607_FORCE_DISCONNECT_BIT;
+ phy_reg->write(temp, vstatus + PHY_9607_FORCE_DISCONNECT_REG);
+
+ usleep_range(10000, 11000);
+ }
+
/* Set page 0 */
phy_data_page = phy_cfg->page0;
rtk_phy_set_page(phy_reg, 0);
@@ -1374,6 +1404,33 @@ static const struct phy_cfg rtd1315e_phy_cfg = {
.write = rtk_usb2phy_write,
};
+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_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,
+ .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,
+ .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 },
@@ -1384,6 +1441,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.54.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v6 5/6] phy: realtek: usb2: add support for RTL9607C USB2 PHY
2026-05-20 17:57 ` [PATCH v6 5/6] phy: realtek: usb2: add support for RTL9607C USB2 PHY Rustam Adilov
@ 2026-05-20 19:51 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-05-20 19:51 UTC (permalink / raw)
To: Rustam Adilov
Cc: linux-phy, vkoul, neil.armstrong, robh, conor+dt, devicetree,
olteanv
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing error handling for of_iomap() leads to NULL pointer dereference
- [High] Incorrect pointer arithmetic on void __iomem * causes unaligned memory accesses
- [High] Resource leak of I/O memory mappings
--
commit 45e50610a9cf3e4d1ba9824c0450199938805d5b
Author: Rustam Adilov <adilov@disroot.org>
phy: realtek: usb2: add support for RTL9607C USB2 PHY
This commit adds support for the USB2 PHY of RTL9607C-based SoCs. It includes
macros, a PHY config structure, and logic to clear a "force host disconnect"
bit before writing parameters.
> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
> index 16c5fc3191dec..69f0f5279b5ef 100644
> --- a/drivers/phy/realtek/phy-rtk-usb2.c
> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
[ ... ]
> @@ -600,6 +617,19 @@ 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 */
> + void __iomem *vstatus = phy_reg->reg_wrap_vstatus;
> + u32 temp;
> +
> + temp = phy_reg->read(vstatus + PHY_9607_FORCE_DISCONNECT_REG);
[Severity: High]
This isn't a bug introduced by this patch, but missing error handling for
of_iomap() in parse_phy_data() could lead to a NULL pointer dereference here.
drivers/phy/realtek/phy-rtk-usb2.c: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;
...
}
If the mapping fails, phy_reg->reg_wrap_vstatus will be NULL.
Does this new code risk dereferencing a NULL vstatus pointer if the device
tree is invalid or memory is exhausted?
[Severity: High]
This is a pre-existing issue, but does the driver leak the I/O memory mapped
by of_iomap()?
Since devm_of_iomap() is not used and there are no corresponding iounmap()
calls anywhere in the driver, could this exhaust I/O memory upon probe
failure or driver unbinding?
> +
> + temp &= ~PHY_9607_FORCE_DISCONNECT_BIT;
> + phy_reg->write(temp, vstatus + PHY_9607_FORCE_DISCONNECT_REG);
> +
> + usleep_range(10000, 11000);
> + }
> +
> /* Set page 0 */
> phy_data_page = phy_cfg->page0;
> rtk_phy_set_page(phy_reg, 0);
[ ... ]
> @@ -1374,6 +1404,33 @@ static const struct phy_cfg rtd1315e_phy_cfg = {
> .write = rtk_usb2phy_write,
> };
>
> +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,
[Severity: High]
This isn't a bug introduced by this patch, but does the pointer arithmetic
incorrectly misalign the base pointer when num_phy > 1?
drivers/phy/realtek/phy-rtk-usb2.c:parse_phy_data() {
...
for (index = 0; index < rtk_phy->num_phy; index++) {
...
phy_parameter->phy_reg.reg_gusb2phyacc0 = of_iomap(np, 1) + index;
...
}
Since of_iomap() returns a void __iomem *, adding index increments the
pointer by exactly index bytes.
Could this cause an unaligned memory access exception (leading to a kernel
panic) or fail to map the correct register index from the device tree?
> + .check_efuse_version = CHECK_EFUSE_V2,
> + .efuse_dc_driving_rate = EFUS_USB_DC_CAL_RATE,
> + .dc_driving_mask = 0x1f,
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520175728.720877-1-adilov@disroot.org?part=5
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v6 6/6] phy: realtek: Make configs available for MACH_REALTEK_RTL
2026-05-20 17:57 [PATCH v6 0/6] phy: realtek: usb2: support for RTL9607C USB2 PHY Rustam Adilov
` (4 preceding siblings ...)
2026-05-20 17:57 ` [PATCH v6 5/6] phy: realtek: usb2: add support for RTL9607C USB2 PHY Rustam Adilov
@ 2026-05-20 17:57 ` Rustam Adilov
5 siblings, 0 replies; 11+ messages in thread
From: Rustam Adilov @ 2026-05-20 17:57 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Stanley Chang, Philipp Zabel, linux-phy, devicetree,
linux-kernel
Cc: 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 | 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.54.0
^ permalink raw reply related [flat|nested] 11+ messages in thread