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

Rustam Adilov
2026-03-31T16:48:11Z

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

I can change it to msleep instead.

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

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

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

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