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

[Rustam Adilov <adilov@disroot.org>]

On 2026-08-12 15:05, Manivannan Sadhasivam wrote:
> On Wed, May 20, 2026 at 10:57:27PM +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. 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);
> 
> If you really want to make sure the delay gets completed within this if()
> condition, then add a read.
> 
> - Mani

Hey,

Thanks for reviews, i have already seen the other comments but i have a question
about this one. What do you mean by adding a read? As far as i know, it is just
time to "allow IP to startup" and not like a status bit i have to continuously
read if that is what you were referring to.

Best,
Rustam