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

[Manivannan Sadhasivam]

On Thu, Aug 13, 2026 at 07:43:00AM +0000, Rustam Adilov wrote:
> 
> 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@xxxxxxx>
> >> Signed-off-by: Michael Zavertkin <misha.zavertkin@xxxxxxx>
> >> Signed-off-by: Rustam Adilov <adilov@xxxxxxxxxxx>
> >> ---
> >>  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.
> 

So you do a write to PHY_9607_FORCE_DISCONNECT_REG register and allow the IP to
start and then continue the operation. What if the write to
PHY_9607_FORCE_DISCONNECT_REG register gets stored in a hardware Write Buffer?
The write instruction will succeed, and the CPU will wait for the delay. But the
write may never reach the hardware and if the CPU starts executing successive
operations before enough delay, it may lead to erratic behaviour.

So that's why it is recommended to flush out the write before delay (if the
delay is critical) by doing a read of that register. The read will ensure that
the write has reached the hardware before the delay:

	write(PHY_9607_FORCE_DISCONNECT_REG);
	read(PHY_9607_FORCE_DISCONNECT_REG) # flush the write
	delay()
	# continue

This read back is not needed in all the cases, but only where the delay is
critical.

- Mani

-- 
மணிவண்ணன் சதாசிவம்