Re: [PATCH v9 05/10] usb: dwc3: core: Refactor PHY logic to support Multiport Controller

[Krishna Kurapati PSSNV <quic_kriskura@xxxxxxxxxxx>]

On 6/24/2023 4:25 AM, Thinh Nguyen wrote:
> On Wed, Jun 21, 2023, Krishna Kurapati wrote:
> > Currently the DWC3 driver supports only single port controller
> > which requires at most one HS and one SS PHY.
> >
> > But the DWC3 USB controller can be connected to multiple ports and
> > each port can have their own PHYs. Each port of the multiport
> > controller can either be HS+SS capable or HS only capable
> > Proper quantification of them is required to modify GUSB2PHYCFG
> > and GUSB3PIPECTL registers appropriately.
> >
> > Add support for detecting, obtaining and configuring phy's supported
> > by a multiport controller and limit the max number of ports
> > supported to 4.
> >
> > Signed-off-by: Harsh Agarwal <quic_harshq@xxxxxxxxxxx>
> > [Krishna: Modifed logic for generic phy and rebased the patch]
> > Signed-off-by: Krishna Kurapati <quic_kriskura@xxxxxxxxxxx>
> > ---
> >   drivers/usb/dwc3/core.c | 252 ++++++++++++++++++++++++++++------------
> >   drivers/usb/dwc3/core.h |  11 +-
> >   drivers/usb/dwc3/drd.c  |  15 ++-
> >   3 files changed, 192 insertions(+), 86 deletions(-)
> >
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index e1ebae54454f..2ac72525de7d 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -120,10 +120,11 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode)
> >   static void __dwc3_set_mode(struct work_struct *work)
> >   {
> >   	struct dwc3 *dwc = work_to_dwc(work);
> > +	u32 desired_dr_role;
> >   	unsigned long flags;
> >   	int ret;
> >   	u32 reg;
> > -	u32 desired_dr_role;
> > +	int i;
> >   
> >   	mutex_lock(&dwc->mutex);
> >   	spin_lock_irqsave(&dwc->lock, flags);
> > @@ -201,8 +202,10 @@ static void __dwc3_set_mode(struct work_struct *work)
> >   		} else {
> >   			if (dwc->usb2_phy)
> >   				otg_set_vbus(dwc->usb2_phy->otg, true);
> > -			phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
> > -			phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
> > +			for (i = 0; i < dwc->num_usb2_ports; i++) {
> > +				phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
> > +				phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
> > +			}
> >   			if (dwc->dis_split_quirk) {
> >   				reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
> >   				reg |= DWC3_GUCTL3_SPLITDISABLE;
> > @@ -217,8 +220,8 @@ static void __dwc3_set_mode(struct work_struct *work)
> >   
> >   		if (dwc->usb2_phy)
> >   			otg_set_vbus(dwc->usb2_phy->otg, false);
> > -		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
> > -		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE);
> > +		phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
> > +		phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
> >   
> >   		ret = dwc3_gadget_init(dwc);
> >   		if (ret)
> > @@ -587,22 +590,14 @@ static int dwc3_core_ulpi_init(struct dwc3 *dwc)
> >   	return ret;
> >   }
> >   
> > -/**
> > - * dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core
> > - * @dwc: Pointer to our controller context structure
> > - *
> > - * Returns 0 on success. The USB PHY interfaces are configured but not
> > - * initialized. The PHY interfaces and the PHYs get initialized together with
> > - * the core in dwc3_core_init.
> > - */
> > -static int dwc3_phy_setup(struct dwc3 *dwc)
> > +static int dwc3_ss_phy_setup(struct dwc3 *dwc, int index)
> >   {
> >   	unsigned int hw_mode;
> >   	u32 reg;
> >   
> >   	hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
> >   
> > -	reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
> > +	reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(index));
> >   
> >   	/*
> >   	 * Make sure UX_EXIT_PX is cleared as that causes issues with some
> > @@ -657,9 +652,19 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
> >   	if (dwc->dis_del_phy_power_chg_quirk)
> >   		reg &= ~DWC3_GUSB3PIPECTL_DEPOCHANGE;
> >   
> > -	dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
> > +	dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(index), reg);
> > +
> > +	return 0;
> > +}
> > +
> > +static int dwc3_hs_phy_setup(struct dwc3 *dwc, int index)
> > +{
> > +	unsigned int hw_mode;
> > +	u32 reg;
> >   
> > -	reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> > +	hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
> > +
> > +	reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(index));
> >   
> >   	/* Select the HS PHY interface */
> >   	switch (DWC3_GHWPARAMS3_HSPHY_IFC(dwc->hwparams.hwparams3)) {
> > @@ -671,7 +676,7 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
> >   		} else if (dwc->hsphy_interface &&
> >   				!strncmp(dwc->hsphy_interface, "ulpi", 4)) {
> >   			reg |= DWC3_GUSB2PHYCFG_ULPI_UTMI;
> > -			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> > +			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(index), reg);
> >   		} else {
> >   			/* Relying on default value. */
> >   			if (!(reg & DWC3_GUSB2PHYCFG_ULPI_UTMI))
> > @@ -738,7 +743,35 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
> >   	if (dwc->ulpi_ext_vbus_drv)
> >   		reg |= DWC3_GUSB2PHYCFG_ULPIEXTVBUSDRV;
> >   
> > -	dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> > +	dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(index), reg);
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core
> > + * @dwc: Pointer to our controller context structure
> > + *
> > + * Returns 0 on success. The USB PHY interfaces are configured but not
> > + * initialized. The PHY interfaces and the PHYs get initialized together with
> > + * the core in dwc3_core_init.
> > + */
> > +static int dwc3_phy_setup(struct dwc3 *dwc)
> > +{
> > +	int i;
> > +	int ret;
> > +
> > +	for (i = 0; i < dwc->num_usb3_ports; i++) {
> > +		ret = dwc3_ss_phy_setup(dwc, i);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	for (i = 0; i < dwc->num_usb2_ports; i++) {
> > +		ret = dwc3_hs_phy_setup(dwc, i);
> > +		if (ret)
> > +			return ret;
> > +	}
> >   
> >   	return 0;
> >   }
> > @@ -746,23 +779,34 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
> >   static int dwc3_phy_init(struct dwc3 *dwc)
> >   {
> >   	int ret;
> > +	int i;
> > +	int j;
> >   
> >   	usb_phy_init(dwc->usb2_phy);
> >   	usb_phy_init(dwc->usb3_phy);
> >   
> > -	ret = phy_init(dwc->usb2_generic_phy);
> > -	if (ret < 0)
> > -		goto err_shutdown_usb3_phy;
> > +	for (i = 0; i < dwc->num_usb2_ports; i++) {
> > +		ret = phy_init(dwc->usb2_generic_phy[i]);
> > +		if (ret < 0)
> > +			goto err_exit_usb2_phy;
> > +	}
> >   
> > -	ret = phy_init(dwc->usb3_generic_phy);
> > -	if (ret < 0)
> > -		goto err_exit_usb2_phy;
> > +	for (i = 0; i < dwc->num_usb2_ports; i++) {
> > +		ret = phy_init(dwc->usb3_generic_phy[i]);
> > +		if (ret < 0)
> > +			goto err_exit_usb3_phy;
> > +	}
> >   
> >   	return 0;
> >   
> > +err_exit_usb3_phy:
> > +	for (j = i-1; j >= 0; j--)
>
> Minor nits, can we add spacing around the '-' for here and other places.
> Checkpatch should be able to catch this.
Hi Thinh,

 Checkpatch actually caught this, but the only reason I didn't fix this 
single nit is it looked like too much spaces in a single for loop, (I 
know it doesn't sound convincing). I will fix this nit in next version.

Regards,
Krishna,

> > +		phy_exit(dwc->usb3_generic_phy[j]);
> > +	i = dwc->num_usb2_ports;
> >   err_exit_usb2_phy:
> > -	phy_exit(dwc->usb2_generic_phy);
> > -err_shutdown_usb3_phy:
> > +	for (j = i-1; j >= 0; j--)
> > +		phy_exit(dwc->usb2_generic_phy[j]);
> > +
> >   	usb_phy_shutdown(dwc->usb3_phy);
> >   	usb_phy_shutdown(dwc->usb2_phy);
> >   
> > @@ -771,8 +815,12 @@ static int dwc3_phy_init(struct dwc3 *dwc)
> >   
> >   static void dwc3_phy_exit(struct dwc3 *dwc)
> >   {
> > -	phy_exit(dwc->usb3_generic_phy);
> > -	phy_exit(dwc->usb2_generic_phy);
> > +	int i;
> > +
> > +	for (i = 0; i < dwc->num_usb2_ports; i++) {
> > +		phy_exit(dwc->usb3_generic_phy[i]);
> > +		phy_exit(dwc->usb2_generic_phy[i]);
> > +	}
> >   
> >   	usb_phy_shutdown(dwc->usb3_phy);
> >   	usb_phy_shutdown(dwc->usb2_phy);
> > @@ -781,23 +829,34 @@ static void dwc3_phy_exit(struct dwc3 *dwc)
> >   static int dwc3_phy_power_on(struct dwc3 *dwc)
> >   {
> >   	int ret;
> > +	int i;
> > +	int j;
> >   
> >   	usb_phy_set_suspend(dwc->usb2_phy, 0);
> >   	usb_phy_set_suspend(dwc->usb3_phy, 0);
> >   
> > -	ret = phy_power_on(dwc->usb2_generic_phy);
> > -	if (ret < 0)
> > -		goto err_suspend_usb3_phy;
> > +	for (i = 0; i < dwc->num_usb2_ports; i++) {
> > +		ret = phy_power_on(dwc->usb2_generic_phy[i]);
> > +		if (ret < 0)
> > +			goto err_power_off_usb2_phy;
> > +	}
> >   
> > -	ret = phy_power_on(dwc->usb3_generic_phy);
> > -	if (ret < 0)
> > -		goto err_power_off_usb2_phy;
> > +	for (i = 0; i < dwc->num_usb2_ports; i++) {
> > +		ret = phy_power_on(dwc->usb3_generic_phy[i]);
> > +		if (ret < 0)
> > +			goto err_power_off_usb3_phy;
> > +	}
> >   
> >   	return 0;
> >   
> > +err_power_off_usb3_phy:
> > +	for (j = i-1; j >= 0; j--)
> > +		phy_power_off(dwc->usb3_generic_phy[i]);
> > +	i = dwc->num_usb2_ports;
> >   err_power_off_usb2_phy:
> > -	phy_power_off(dwc->usb2_generic_phy);
> > -err_suspend_usb3_phy:
> > +	for (j = i-1; j >= 0; j--)
> > +		phy_power_off(dwc->usb2_generic_phy[i]);
> > +
> >   	usb_phy_set_suspend(dwc->usb3_phy, 1);
> >   	usb_phy_set_suspend(dwc->usb2_phy, 1);
> >   
> > @@ -806,8 +865,12 @@ static int dwc3_phy_power_on(struct dwc3 *dwc)
> >   
> >   static void dwc3_phy_power_off(struct dwc3 *dwc)
> >   {
> > -	phy_power_off(dwc->usb3_generic_phy);
> > -	phy_power_off(dwc->usb2_generic_phy);
> > +	int i;
> > +
> > +	for (i = 0; i < dwc->num_usb2_ports; i++) {
> > +		phy_power_off(dwc->usb3_generic_phy[i]);
> > +		phy_power_off(dwc->usb2_generic_phy[i]);
> > +	}
> >   
> >   	usb_phy_set_suspend(dwc->usb3_phy, 1);
> >   	usb_phy_set_suspend(dwc->usb2_phy, 1);
> > @@ -1080,6 +1143,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
> >   	unsigned int		hw_mode;
> >   	u32			reg;
> >   	int			ret;
> > +	int			i;
> >   
> >   	hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
> >   
> > @@ -1123,15 +1187,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
> >   	if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD &&
> >   	    !DWC3_VER_IS_WITHIN(DWC3, ANY, 194A)) {
> >   		if (!dwc->dis_u3_susphy_quirk) {
> > -			reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
> > -			reg |= DWC3_GUSB3PIPECTL_SUSPHY;
> > -			dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
> > +			for (i = 0; i < dwc->num_usb3_ports; i++) {
> > +				reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(i));
> > +				reg |= DWC3_GUSB3PIPECTL_SUSPHY;
> > +				dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(i), reg);
> > +			}
> >   		}
> >   
> >   		if (!dwc->dis_u2_susphy_quirk) {
> > -			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> > -			reg |= DWC3_GUSB2PHYCFG_SUSPHY;
> > -			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> > +			for (i = 0; i < dwc->num_usb2_ports; i++) {
> > +				reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
> > +				reg |= DWC3_GUSB2PHYCFG_SUSPHY;
> > +				dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
> > +			}
> >   		}
> >   	}
> >   
> > @@ -1290,7 +1358,9 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
> >   {
> >   	struct device		*dev = dwc->dev;
> >   	struct device_node	*node = dev->of_node;
> > +	char phy_name[11];
> >   	int ret;
> > +	int i;
> >   
> >   	if (node) {
> >   		dwc->usb2_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 0);
> > @@ -1316,22 +1386,36 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
> >   			return dev_err_probe(dev, ret, "no usb3 phy configured\n");
> >   	}
> >   
> > -	dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
> > -	if (IS_ERR(dwc->usb2_generic_phy)) {
> > -		ret = PTR_ERR(dwc->usb2_generic_phy);
> > -		if (ret == -ENOSYS || ret == -ENODEV)
> > -			dwc->usb2_generic_phy = NULL;
> > +	for (i = 0; i < dwc->num_usb2_ports; i++) {
> > +		if (dwc->num_usb2_ports == 1)
> > +			sprintf(phy_name, "usb2-phy");
> >   		else
> > -			return dev_err_probe(dev, ret, "no usb2 phy configured\n");
> > -	}
> > +			sprintf(phy_name, "usb2-port%d", i);
> > +
> > +		dwc->usb2_generic_phy[i] = devm_phy_get(dev, phy_name);
> > +		if (IS_ERR(dwc->usb2_generic_phy[i])) {
> > +			ret = PTR_ERR(dwc->usb2_generic_phy[i]);
> > +			if (ret == -ENOSYS || ret == -ENODEV)
> > +				dwc->usb2_generic_phy[i] = NULL;
> > +			else
> > +				return dev_err_probe(dev, ret,
> > +					"no %s phy configured\n", phy_name);
> > +		}
> >   
> > -	dwc->usb3_generic_phy = devm_phy_get(dev, "usb3-phy");
> > -	if (IS_ERR(dwc->usb3_generic_phy)) {
> > -		ret = PTR_ERR(dwc->usb3_generic_phy);
> > -		if (ret == -ENOSYS || ret == -ENODEV)
> > -			dwc->usb3_generic_phy = NULL;
> > +		if (dwc->num_usb2_ports == 1)
> > +			sprintf(phy_name, "usb3-phy");
> >   		else
> > -			return dev_err_probe(dev, ret, "no usb3 phy configured\n");
> > +			sprintf(phy_name, "usb3-port%d", i);
> > +
> > +		dwc->usb3_generic_phy[i] = devm_phy_get(dev, phy_name);
> > +		if (IS_ERR(dwc->usb3_generic_phy[i])) {
> > +			ret = PTR_ERR(dwc->usb3_generic_phy[i]);
> > +			if (ret == -ENOSYS || ret == -ENODEV)
> > +				dwc->usb3_generic_phy[i] = NULL;
> > +			else
> > +				return dev_err_probe(dev, ret,
> > +					"no %s phy configured\n", phy_name);
> > +		}
> >   	}
> >   
> >   	return 0;
> > @@ -1341,6 +1425,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
> >   {
> >   	struct device *dev = dwc->dev;
> >   	int ret;
> > +	int i;
> >   
> >   	switch (dwc->dr_mode) {
> >   	case USB_DR_MODE_PERIPHERAL:
> > @@ -1348,8 +1433,8 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
> >   
> >   		if (dwc->usb2_phy)
> >   			otg_set_vbus(dwc->usb2_phy->otg, false);
> > -		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
> > -		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE);
> > +		phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
> > +		phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
> >   
> >   		ret = dwc3_gadget_init(dwc);
> >   		if (ret)
> > @@ -1360,8 +1445,10 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
> >   
> >   		if (dwc->usb2_phy)
> >   			otg_set_vbus(dwc->usb2_phy->otg, true);
> > -		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
> > -		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
> > +		for (i = 0; i < dwc->num_usb2_ports; i++) {
> > +			phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
> > +			phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
> > +		}
> >   
> >   		ret = dwc3_host_init(dwc);
> >   		if (ret)
> > @@ -1821,6 +1908,9 @@ static int dwc3_read_port_info(struct dwc3 *dwc)
> >   	dev_dbg(dwc->dev, "hs-ports: %u ss-ports: %u\n",
> >   			dwc->num_usb2_ports, dwc->num_usb3_ports);
> >   
> > +	if (dwc->num_usb2_ports > DWC3_MAX_PORTS)
> > +		ret = -ENOMEM;
> > +
> >   	iounmap(base);
> >   	return ret;
> >   }
> > @@ -2057,6 +2147,7 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
> >   {
> >   	unsigned long	flags;
> >   	u32 reg;
> > +	int i;
> >   
> >   	switch (dwc->current_dr_role) {
> >   	case DWC3_GCTL_PRTCAP_DEVICE:
> > @@ -2075,17 +2166,21 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
> >   		/* Let controller to suspend HSPHY before PHY driver suspends */
> >   		if (dwc->dis_u2_susphy_quirk ||
> >   		    dwc->dis_enblslpm_quirk) {
> > -			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> > -			reg |=  DWC3_GUSB2PHYCFG_ENBLSLPM |
> > -				DWC3_GUSB2PHYCFG_SUSPHY;
> > -			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> > +			for (i = 0; i < dwc->num_usb2_ports; i++) {
> > +				reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
> > +				reg |=  DWC3_GUSB2PHYCFG_ENBLSLPM |
> > +					DWC3_GUSB2PHYCFG_SUSPHY;
> > +				dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
> > +			}
> >   
> >   			/* Give some time for USB2 PHY to suspend */
> >   			usleep_range(5000, 6000);
> >   		}
> >   
> > -		phy_pm_runtime_put_sync(dwc->usb2_generic_phy);
> > -		phy_pm_runtime_put_sync(dwc->usb3_generic_phy);
> > +		for (i = 0; i < dwc->num_usb2_ports; i++) {
> > +			phy_pm_runtime_put_sync(dwc->usb2_generic_phy[i]);
> > +			phy_pm_runtime_put_sync(dwc->usb3_generic_phy[i]);
> > +		}
> >   		break;
> >   	case DWC3_GCTL_PRTCAP_OTG:
> >   		/* do nothing during runtime_suspend */
> > @@ -2115,6 +2210,7 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
> >   	unsigned long	flags;
> >   	int		ret;
> >   	u32		reg;
> > +	int		i;
> >   
> >   	switch (dwc->current_dr_role) {
> >   	case DWC3_GCTL_PRTCAP_DEVICE:
> > @@ -2134,17 +2230,21 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
> >   			break;
> >   		}
> >   		/* Restore GUSB2PHYCFG bits that were modified in suspend */
> > -		reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> > -		if (dwc->dis_u2_susphy_quirk)
> > -			reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
> > +		 for (i = 0; i < dwc->num_usb2_ports; i++) {
>
> Nit: Extra spacing before the "for"?
>
> > +			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
> > +			if (dwc->dis_u2_susphy_quirk)
> > +				reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
> >   
> > -		if (dwc->dis_enblslpm_quirk)
> > -			reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
> > +			if (dwc->dis_enblslpm_quirk)
> > +				reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
> >   
> > -		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> > +			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
> > +		}
> >   
> > -		phy_pm_runtime_get_sync(dwc->usb2_generic_phy);
> > -		phy_pm_runtime_get_sync(dwc->usb3_generic_phy);
> > +		for (i = 0; i < dwc->num_usb2_ports; i++) {
> > +			phy_pm_runtime_get_sync(dwc->usb2_generic_phy[i]);
> > +			phy_pm_runtime_get_sync(dwc->usb3_generic_phy[i]);
> > +		}
> >   		break;
> >   	case DWC3_GCTL_PRTCAP_OTG:
> >   		/* nothing to do on runtime_resume */
> > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> > index 42fb17aa66fa..b2bab23ca22b 100644
> > --- a/drivers/usb/dwc3/core.h
> > +++ b/drivers/usb/dwc3/core.h
> > @@ -37,6 +37,9 @@
> >   #define XHCI_EXT_PORT_MINOR(x)	(((x) >> 16) & 0xff)
> >   #define XHCI_EXT_PORT_COUNT(x)	(((x) >> 8) & 0xff)
> >   
> > +/* Number of ports supported by a multiport controller */
> > +#define DWC3_MAX_PORTS 4
> > +
> >   #define DWC3_MSG_MAX	500
> >   
> >   /* Global constants */
> > @@ -1031,8 +1034,8 @@ struct dwc3_scratchpad_array {
> >    * @usb_psy: pointer to power supply interface.
> >    * @usb2_phy: pointer to USB2 PHY
> >    * @usb3_phy: pointer to USB3 PHY
> > - * @usb2_generic_phy: pointer to USB2 PHY
> > - * @usb3_generic_phy: pointer to USB3 PHY
> > + * @usb2_generic_phy: pointer to array of USB2 PHY
> > + * @usb3_generic_phy: pointer to array of USB3 PHY
> >    * @num_usb2_ports: number of USB2 ports.
> >    * @num_usb3_ports: number of USB3 ports.
> >    * @phys_ready: flag to indicate that PHYs are ready
> > @@ -1171,8 +1174,8 @@ struct dwc3 {
> >   	struct usb_phy		*usb2_phy;
> >   	struct usb_phy		*usb3_phy;
> >   
> > -	struct phy		*usb2_generic_phy;
> > -	struct phy		*usb3_generic_phy;
> > +	struct phy		*usb2_generic_phy[DWC3_MAX_PORTS];
> > +	struct phy		*usb3_generic_phy[DWC3_MAX_PORTS];
> >   
> >   	u8			num_usb2_ports;
> >   	u8			num_usb3_ports;
> > diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
> > index 039bf241769a..18a247ff75ac 100644
> > --- a/drivers/usb/dwc3/drd.c
> > +++ b/drivers/usb/dwc3/drd.c
> > @@ -327,10 +327,11 @@ static void dwc3_otg_device_exit(struct dwc3 *dwc)
> >   
> >   void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
> >   {
> > +	unsigned long flags;
> >   	int ret;
> >   	u32 reg;
> >   	int id;
> > -	unsigned long flags;
> > +	int i;
> >   
> >   	if (dwc->dr_mode != USB_DR_MODE_OTG)
> >   		return;
> > @@ -386,9 +387,11 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
> >   		} else {
> >   			if (dwc->usb2_phy)
> >   				otg_set_vbus(dwc->usb2_phy->otg, true);
> > -			if (dwc->usb2_generic_phy)
> > -				phy_set_mode(dwc->usb2_generic_phy,
> > -					     PHY_MODE_USB_HOST);
> > +			for (i = 0; i < dwc->num_usb2_ports; i++) {
> > +				if (dwc->usb2_generic_phy[i])
> > +					phy_set_mode(dwc->usb2_generic_phy[i],
> > +						     PHY_MODE_USB_HOST);
> > +			}
> >   		}
> >   		break;
> >   	case DWC3_OTG_ROLE_DEVICE:
> > @@ -400,8 +403,8 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
> >   
> >   		if (dwc->usb2_phy)
> >   			otg_set_vbus(dwc->usb2_phy->otg, false);
> > -		if (dwc->usb2_generic_phy)
> > -			phy_set_mode(dwc->usb2_generic_phy,
> > +		if (dwc->usb2_generic_phy[0])
> > +			phy_set_mode(dwc->usb2_generic_phy[0],
> >   				     PHY_MODE_USB_DEVICE);
> >   		ret = dwc3_gadget_init(dwc);
> >   		if (ret)
> > --
> > 2.40.0
>
> After fixing the minor nits mentioned above, you can add my Ack:
>
> Acked-by: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx>
>
> Thanks,
> Thinh