[PATCH 1/2] usb: dwc3: drd: extend dwc3_pre_set_role() to extcon and otg usecase

[PATCH 1/2] usb: dwc3: drd: extend dwc3_pre_set_role() to extcon and otg usecase

[Xu Yang]

Extend dwc3_pre_set_role() to extcon and otg usecase, so that the glue
driver can do proper action in case of role changes.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 drivers/usb/dwc3/drd.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index 589bbeb27454..031cfd12300a 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -381,6 +381,7 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
 		dwc3_otgregs_init(dwc);
 		dwc3_otg_host_init(dwc);
 		spin_unlock_irqrestore(&dwc->lock, flags);
+		dwc3_pre_set_role(dwc, USB_ROLE_HOST);
 		ret = dwc3_host_init(dwc);
 		if (ret) {
 			dev_err(dwc->dev, "failed to initialize host\n");
@@ -406,6 +407,7 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
 			otg_set_vbus(dwc->usb2_phy->otg, false);
 		if (dwc->usb2_generic_phy[0])
 			phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
+		dwc3_pre_set_role(dwc, USB_ROLE_DEVICE);
 		ret = dwc3_gadget_init(dwc);
 		if (ret)
 			dev_err(dwc->dev, "failed to initialize peripheral\n");
@@ -433,10 +435,12 @@ static int dwc3_drd_notifier(struct notifier_block *nb,
 			     unsigned long event, void *ptr)
 {
 	struct dwc3 *dwc = container_of(nb, struct dwc3, edev_nb);
+	u32 mode = event ? DWC3_GCTL_PRTCAP_HOST : DWC3_GCTL_PRTCAP_DEVICE;
+	enum usb_role role = mode == DWC3_GCTL_PRTCAP_HOST ?
+				     USB_ROLE_HOST : USB_ROLE_DEVICE;
 
-	dwc3_set_mode(dwc, event ?
-		      DWC3_GCTL_PRTCAP_HOST :
-		      DWC3_GCTL_PRTCAP_DEVICE);
+	dwc3_pre_set_role(dwc, role);
+	dwc3_set_mode(dwc, mode);
 
 	return NOTIFY_DONE;
 }
-- 
2.34.1

[PATCH 2/2] usb: dwc3: imx8mp: disable auto suspend for host role

[Xu Yang]

Do dwc3 core auto suspend enable for device and disable for host
, this can make sure dwc3 core device auto suspend setting is
correct all the time, the background of disable dwc3 core device
auto suspend is to make its parent device suspend immediately
(so wakeup enable can be enabled) after xhci-plat device suspended,
for device mode, we keep the dwc3 core device auto suspend is to
give some wait for gadget to be enumerated.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 drivers/usb/dwc3/dwc3-imx8mp.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
index bce6af82f54c..80a431cb6fed 100644
--- a/drivers/usb/dwc3/dwc3-imx8mp.c
+++ b/drivers/usb/dwc3/dwc3-imx8mp.c
@@ -158,11 +158,31 @@ static irqreturn_t dwc3_imx8mp_interrupt(int irq, void *_dwc3_imx)
 	return IRQ_HANDLED;
 }
 
+static void dwc3_imx_pre_set_role(struct dwc3 *dwc, enum usb_role role)
+{
+	if (role == USB_ROLE_HOST)
+		/*
+		 * For xhci host, we need disable dwc core auto
+		 * suspend, because during this auto suspend delay(5s),
+		 * xhci host RUN_STOP is cleared and wakeup is not
+		 * enabled, if device is inserted, xhci host can't
+		 * response the connection.
+		 */
+		pm_runtime_dont_use_autosuspend(dwc->dev);
+	else
+		pm_runtime_use_autosuspend(dwc->dev);
+}
+
+struct dwc3_glue_ops dwc3_imx_glue_ops = {
+	.pre_set_role   = dwc3_imx_pre_set_role,
+};
+
 static int dwc3_imx8mp_probe(struct platform_device *pdev)
 {
 	struct device		*dev = &pdev->dev;
 	struct device_node	*node = dev->of_node;
 	struct dwc3_imx8mp	*dwc3_imx;
+	struct dwc3		*dwc3;
 	struct resource		*res;
 	int			err, irq;
 
@@ -240,6 +260,17 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
 		goto depopulate;
 	}
 
+	dwc3 = platform_get_drvdata(dwc3_imx->dwc3);
+	if (!dwc3) {
+		err = dev_err_probe(dev, -EPROBE_DEFER, "failed to get dwc3 platform data\n");
+		goto depopulate;
+	}
+
+	dwc3->glue_ops = &dwc3_imx_glue_ops;
+
+	if (dwc3->dr_mode == USB_DR_MODE_HOST)
+		pm_runtime_dont_use_autosuspend(dwc3->dev);
+
 	err = devm_request_threaded_irq(dev, irq, NULL, dwc3_imx8mp_interrupt,
 					IRQF_ONESHOT, dev_name(dev), dwc3_imx);
 	if (err) {
-- 
2.34.1

Re: [PATCH 1/2] usb: dwc3: drd: extend dwc3_pre_set_role() to extcon and otg usecase

[Frank Li]

On Wed, Nov 05, 2025 at 03:45:02PM +0800, Xu Yang wrote:
> Extend dwc3_pre_set_role() to extcon and otg usecase, so that the glue
> driver can do proper action in case of role changes.

Nit: look likes

Call dwc3_pre_set_role() to support both extcon and otg usecase, ...

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  drivers/usb/dwc3/drd.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
> index 589bbeb27454..031cfd12300a 100644
> --- a/drivers/usb/dwc3/drd.c
> +++ b/drivers/usb/dwc3/drd.c
> @@ -381,6 +381,7 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
>  		dwc3_otgregs_init(dwc);
>  		dwc3_otg_host_init(dwc);
>  		spin_unlock_irqrestore(&dwc->lock, flags);
> +		dwc3_pre_set_role(dwc, USB_ROLE_HOST);
>  		ret = dwc3_host_init(dwc);
>  		if (ret) {
>  			dev_err(dwc->dev, "failed to initialize host\n");
> @@ -406,6 +407,7 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
>  			otg_set_vbus(dwc->usb2_phy->otg, false);
>  		if (dwc->usb2_generic_phy[0])
>  			phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
> +		dwc3_pre_set_role(dwc, USB_ROLE_DEVICE);
>  		ret = dwc3_gadget_init(dwc);
>  		if (ret)
>  			dev_err(dwc->dev, "failed to initialize peripheral\n");
> @@ -433,10 +435,12 @@ static int dwc3_drd_notifier(struct notifier_block *nb,
>  			     unsigned long event, void *ptr)
>  {
>  	struct dwc3 *dwc = container_of(nb, struct dwc3, edev_nb);
> +	u32 mode = event ? DWC3_GCTL_PRTCAP_HOST : DWC3_GCTL_PRTCAP_DEVICE;
> +	enum usb_role role = mode == DWC3_GCTL_PRTCAP_HOST ?
> +				     USB_ROLE_HOST : USB_ROLE_DEVICE;
>
> -	dwc3_set_mode(dwc, event ?
> -		      DWC3_GCTL_PRTCAP_HOST :
> -		      DWC3_GCTL_PRTCAP_DEVICE);
> +	dwc3_pre_set_role(dwc, role);
> +	dwc3_set_mode(dwc, mode);
>
>  	return NOTIFY_DONE;
>  }
> --
> 2.34.1
>

Re: [PATCH 2/2] usb: dwc3: imx8mp: disable auto suspend for host role

[Frank Li]

On Wed, Nov 05, 2025 at 03:45:03PM +0800, Xu Yang wrote:
> Do dwc3 core auto suspend enable for device and disable for host
> , this can make sure dwc3 core device auto suspend setting is
> correct all the time, the background of disable dwc3 core device
> auto suspend is to make its parent device suspend immediately
> (so wakeup enable can be enabled) after xhci-plat device suspended,

Does wakeup only enable at runtime pm suspend? why core delay runtime
pm suspend impact wakeup function?

Frank

> for device mode, we keep the dwc3 core device auto suspend is to
> give some wait for gadget to be enumerated.
>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  drivers/usb/dwc3/dwc3-imx8mp.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
> index bce6af82f54c..80a431cb6fed 100644
> --- a/drivers/usb/dwc3/dwc3-imx8mp.c
> +++ b/drivers/usb/dwc3/dwc3-imx8mp.c
> @@ -158,11 +158,31 @@ static irqreturn_t dwc3_imx8mp_interrupt(int irq, void *_dwc3_imx)
>  	return IRQ_HANDLED;
>  }
>
> +static void dwc3_imx_pre_set_role(struct dwc3 *dwc, enum usb_role role)
> +{
> +	if (role == USB_ROLE_HOST)
> +		/*
> +		 * For xhci host, we need disable dwc core auto
> +		 * suspend, because during this auto suspend delay(5s),
> +		 * xhci host RUN_STOP is cleared and wakeup is not
> +		 * enabled, if device is inserted, xhci host can't
> +		 * response the connection.
> +		 */
> +		pm_runtime_dont_use_autosuspend(dwc->dev);
> +	else
> +		pm_runtime_use_autosuspend(dwc->dev);
> +}
> +
> +struct dwc3_glue_ops dwc3_imx_glue_ops = {
> +	.pre_set_role   = dwc3_imx_pre_set_role,
> +};
> +
>  static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  {
>  	struct device		*dev = &pdev->dev;
>  	struct device_node	*node = dev->of_node;
>  	struct dwc3_imx8mp	*dwc3_imx;
> +	struct dwc3		*dwc3;
>  	struct resource		*res;
>  	int			err, irq;
>
> @@ -240,6 +260,17 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  		goto depopulate;
>  	}
>
> +	dwc3 = platform_get_drvdata(dwc3_imx->dwc3);
> +	if (!dwc3) {
> +		err = dev_err_probe(dev, -EPROBE_DEFER, "failed to get dwc3 platform data\n");
> +		goto depopulate;
> +	}
> +
> +	dwc3->glue_ops = &dwc3_imx_glue_ops;
> +
> +	if (dwc3->dr_mode == USB_DR_MODE_HOST)
> +		pm_runtime_dont_use_autosuspend(dwc3->dev);
> +
>  	err = devm_request_threaded_irq(dev, irq, NULL, dwc3_imx8mp_interrupt,
>  					IRQF_ONESHOT, dev_name(dev), dwc3_imx);
>  	if (err) {
> --
> 2.34.1
>

Re: [PATCH 2/2] usb: dwc3: imx8mp: disable auto suspend for host role

[Xu Yang]

On Wed, Nov 05, 2025 at 10:44:57AM -0500, Frank Li wrote:
> On Wed, Nov 05, 2025 at 03:45:03PM +0800, Xu Yang wrote:
> > Do dwc3 core auto suspend enable for device and disable for host
> > , this can make sure dwc3 core device auto suspend setting is
> > correct all the time, the background of disable dwc3 core device
> > auto suspend is to make its parent device suspend immediately
> > (so wakeup enable can be enabled) after xhci-plat device suspended,
> 
> Does wakeup only enable at runtime pm suspend? why core delay runtime

Yes.

> pm suspend impact wakeup function?

If xhci is suspended, I mean the wakeup function comes from external glue logic.

When xhci is suspended, it will stop host controller (RUN_STOP = 0) which means
it can't detect connect/disconnect event. Meanwhile, if dwc3 is active, the
wakeup IRQs (in glue) is still disabled. when a device is attached, xhci can't
detect this event and the external wakeup logic is still unfunctional. When dwc3
is suspended, the previous connect event can't be fired again even though wakeup
IRQ is enabled. So that event will be lost. 

Thanks,
Xu Yang

> 
> Frank
> 
> > for device mode, we keep the dwc3 core device auto suspend is to
> > give some wait for gadget to be enumerated.

Re: [PATCH 1/2] usb: dwc3: drd: extend dwc3_pre_set_role() to extcon and otg usecase

[Thinh Nguyen]

On Wed, Nov 05, 2025, Xu Yang wrote:
> Extend dwc3_pre_set_role() to extcon and otg usecase, so that the glue
> driver can do proper action in case of role changes.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  drivers/usb/dwc3/drd.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
> index 589bbeb27454..031cfd12300a 100644
> --- a/drivers/usb/dwc3/drd.c
> +++ b/drivers/usb/dwc3/drd.c
> @@ -381,6 +381,7 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
>  		dwc3_otgregs_init(dwc);
>  		dwc3_otg_host_init(dwc);
>  		spin_unlock_irqrestore(&dwc->lock, flags);
> +		dwc3_pre_set_role(dwc, USB_ROLE_HOST);
>  		ret = dwc3_host_init(dwc);
>  		if (ret) {
>  			dev_err(dwc->dev, "failed to initialize host\n");
> @@ -406,6 +407,7 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
>  			otg_set_vbus(dwc->usb2_phy->otg, false);
>  		if (dwc->usb2_generic_phy[0])
>  			phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
> +		dwc3_pre_set_role(dwc, USB_ROLE_DEVICE);
>  		ret = dwc3_gadget_init(dwc);
>  		if (ret)
>  			dev_err(dwc->dev, "failed to initialize peripheral\n");
> @@ -433,10 +435,12 @@ static int dwc3_drd_notifier(struct notifier_block *nb,
>  			     unsigned long event, void *ptr)
>  {
>  	struct dwc3 *dwc = container_of(nb, struct dwc3, edev_nb);
> +	u32 mode = event ? DWC3_GCTL_PRTCAP_HOST : DWC3_GCTL_PRTCAP_DEVICE;
> +	enum usb_role role = mode == DWC3_GCTL_PRTCAP_HOST ?
> +				     USB_ROLE_HOST : USB_ROLE_DEVICE;
>  
> -	dwc3_set_mode(dwc, event ?
> -		      DWC3_GCTL_PRTCAP_HOST :
> -		      DWC3_GCTL_PRTCAP_DEVICE);
> +	dwc3_pre_set_role(dwc, role);
> +	dwc3_set_mode(dwc, mode);
>  
>  	return NOTIFY_DONE;
>  }
> -- 
> 2.34.1
> 

I don't like how the ternary operator is used here and how we've done it
previously, especially when we're treating the event as if it's a
boolean.

Regardless, it's a minor nit.

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

BR,
Thinh

Re: [PATCH 2/2] usb: dwc3: imx8mp: disable auto suspend for host role

[Thinh Nguyen]

On Wed, Nov 05, 2025, Xu Yang wrote:
> Do dwc3 core auto suspend enable for device and disable for host
> , this can make sure dwc3 core device auto suspend setting is
> correct all the time, the background of disable dwc3 core device
> auto suspend is to make its parent device suspend immediately
> (so wakeup enable can be enabled) after xhci-plat device suspended,
> for device mode, we keep the dwc3 core device auto suspend is to
> give some wait for gadget to be enumerated.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  drivers/usb/dwc3/dwc3-imx8mp.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
> index bce6af82f54c..80a431cb6fed 100644
> --- a/drivers/usb/dwc3/dwc3-imx8mp.c
> +++ b/drivers/usb/dwc3/dwc3-imx8mp.c
> @@ -158,11 +158,31 @@ static irqreturn_t dwc3_imx8mp_interrupt(int irq, void *_dwc3_imx)
>  	return IRQ_HANDLED;
>  }
>  
> +static void dwc3_imx_pre_set_role(struct dwc3 *dwc, enum usb_role role)
> +{
> +	if (role == USB_ROLE_HOST)
> +		/*
> +		 * For xhci host, we need disable dwc core auto
> +		 * suspend, because during this auto suspend delay(5s),
> +		 * xhci host RUN_STOP is cleared and wakeup is not
> +		 * enabled, if device is inserted, xhci host can't
> +		 * response the connection.
> +		 */
> +		pm_runtime_dont_use_autosuspend(dwc->dev);
> +	else
> +		pm_runtime_use_autosuspend(dwc->dev);

Would this override the user setting everytime there's a role switch?

> +}
> +
> +struct dwc3_glue_ops dwc3_imx_glue_ops = {
> +	.pre_set_role   = dwc3_imx_pre_set_role,
> +};
> +
>  static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  {
>  	struct device		*dev = &pdev->dev;
>  	struct device_node	*node = dev->of_node;
>  	struct dwc3_imx8mp	*dwc3_imx;
> +	struct dwc3		*dwc3;
>  	struct resource		*res;
>  	int			err, irq;
>  
> @@ -240,6 +260,17 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  		goto depopulate;
>  	}
>  
> +	dwc3 = platform_get_drvdata(dwc3_imx->dwc3);

It's confusing how dwc3_imx->dwc3 is also named dwc3...

> +	if (!dwc3) {
> +		err = dev_err_probe(dev, -EPROBE_DEFER, "failed to get dwc3 platform data\n");
> +		goto depopulate;
> +	}
> +
> +	dwc3->glue_ops = &dwc3_imx_glue_ops;

If you want to use glue_ops, please use the new flatten model. I
don't want dwc3 to be initialized again after of_platform_populate().

BR,
Thinh

> +
> +	if (dwc3->dr_mode == USB_DR_MODE_HOST)
> +		pm_runtime_dont_use_autosuspend(dwc3->dev);
> +
>  	err = devm_request_threaded_irq(dev, irq, NULL, dwc3_imx8mp_interrupt,
>  					IRQF_ONESHOT, dev_name(dev), dwc3_imx);
>  	if (err) {
> -- 
> 2.34.1
> 

Re: [PATCH 2/2] usb: dwc3: imx8mp: disable auto suspend for host role

[Xu Yang]

On Thu, Nov 13, 2025 at 11:05:43PM +0000, Thinh Nguyen wrote:
> On Wed, Nov 05, 2025, Xu Yang wrote:
> > Do dwc3 core auto suspend enable for device and disable for host
> > , this can make sure dwc3 core device auto suspend setting is
> > correct all the time, the background of disable dwc3 core device
> > auto suspend is to make its parent device suspend immediately
> > (so wakeup enable can be enabled) after xhci-plat device suspended,
> > for device mode, we keep the dwc3 core device auto suspend is to
> > give some wait for gadget to be enumerated.
> > 
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> >  drivers/usb/dwc3/dwc3-imx8mp.c | 31 +++++++++++++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> > 
> > diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
> > index bce6af82f54c..80a431cb6fed 100644
> > --- a/drivers/usb/dwc3/dwc3-imx8mp.c
> > +++ b/drivers/usb/dwc3/dwc3-imx8mp.c
> > @@ -158,11 +158,31 @@ static irqreturn_t dwc3_imx8mp_interrupt(int irq, void *_dwc3_imx)
> >  	return IRQ_HANDLED;
> >  }
> >  
> > +static void dwc3_imx_pre_set_role(struct dwc3 *dwc, enum usb_role role)
> > +{
> > +	if (role == USB_ROLE_HOST)
> > +		/*
> > +		 * For xhci host, we need disable dwc core auto
> > +		 * suspend, because during this auto suspend delay(5s),
> > +		 * xhci host RUN_STOP is cleared and wakeup is not
> > +		 * enabled, if device is inserted, xhci host can't
> > +		 * response the connection.
> > +		 */
> > +		pm_runtime_dont_use_autosuspend(dwc->dev);
> > +	else
> > +		pm_runtime_use_autosuspend(dwc->dev);
> 
> Would this override the user setting everytime there's a role switch?

From what I know, the user can't control whether to enable or disable
autosuspend feature. So there should be no overriding problem. When
user change autosuspend_delay_ms, the delay setting will be kept
everytime there's a role switch.

> 
> > +}
> > +
> > +struct dwc3_glue_ops dwc3_imx_glue_ops = {
> > +	.pre_set_role   = dwc3_imx_pre_set_role,
> > +};
> > +
> >  static int dwc3_imx8mp_probe(struct platform_device *pdev)
> >  {
> >  	struct device		*dev = &pdev->dev;
> >  	struct device_node	*node = dev->of_node;
> >  	struct dwc3_imx8mp	*dwc3_imx;
> > +	struct dwc3		*dwc3;
> >  	struct resource		*res;
> >  	int			err, irq;
> >  
> > @@ -240,6 +260,17 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
> >  		goto depopulate;
> >  	}
> >  
> > +	dwc3 = platform_get_drvdata(dwc3_imx->dwc3);
> 
> It's confusing how dwc3_imx->dwc3 is also named dwc3...

I will rename it later.

> 
> > +	if (!dwc3) {
> > +		err = dev_err_probe(dev, -EPROBE_DEFER, "failed to get dwc3 platform data\n");
> > +		goto depopulate;
> > +	}
> > +
> > +	dwc3->glue_ops = &dwc3_imx_glue_ops;
> 
> If you want to use glue_ops, please use the new flatten model. I
> don't want dwc3 to be initialized again after of_platform_populate().

I understand the new flatten model is a more suitable way. Considering that
many i.MX devices are using this legacy unflatten model, do you mind allow
this change firstly and we will switch to the new flatten model in the future?

Thanks,
Xu Yang

> 
> BR,
> Thinh
> 
> > +
> > +	if (dwc3->dr_mode == USB_DR_MODE_HOST)
> > +		pm_runtime_dont_use_autosuspend(dwc3->dev);
> > +
> >  	err = devm_request_threaded_irq(dev, irq, NULL, dwc3_imx8mp_interrupt,
> >  					IRQF_ONESHOT, dev_name(dev), dwc3_imx);
> >  	if (err) {
> > -- 
> > 2.34.1
> > 

Re: [PATCH 1/2] usb: dwc3: drd: extend dwc3_pre_set_role() to extcon and otg usecase

[Xu Yang]

On Thu, Nov 13, 2025 at 10:37:40PM +0000, Thinh Nguyen wrote:
> On Wed, Nov 05, 2025, Xu Yang wrote:
> > Extend dwc3_pre_set_role() to extcon and otg usecase, so that the glue
> > driver can do proper action in case of role changes.
> > 
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> >  drivers/usb/dwc3/drd.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
> > index 589bbeb27454..031cfd12300a 100644
> > --- a/drivers/usb/dwc3/drd.c
> > +++ b/drivers/usb/dwc3/drd.c
> > @@ -381,6 +381,7 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
> >  		dwc3_otgregs_init(dwc);
> >  		dwc3_otg_host_init(dwc);
> >  		spin_unlock_irqrestore(&dwc->lock, flags);
> > +		dwc3_pre_set_role(dwc, USB_ROLE_HOST);
> >  		ret = dwc3_host_init(dwc);
> >  		if (ret) {
> >  			dev_err(dwc->dev, "failed to initialize host\n");
> > @@ -406,6 +407,7 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
> >  			otg_set_vbus(dwc->usb2_phy->otg, false);
> >  		if (dwc->usb2_generic_phy[0])
> >  			phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
> > +		dwc3_pre_set_role(dwc, USB_ROLE_DEVICE);
> >  		ret = dwc3_gadget_init(dwc);
> >  		if (ret)
> >  			dev_err(dwc->dev, "failed to initialize peripheral\n");
> > @@ -433,10 +435,12 @@ static int dwc3_drd_notifier(struct notifier_block *nb,
> >  			     unsigned long event, void *ptr)
> >  {
> >  	struct dwc3 *dwc = container_of(nb, struct dwc3, edev_nb);
> > +	u32 mode = event ? DWC3_GCTL_PRTCAP_HOST : DWC3_GCTL_PRTCAP_DEVICE;
> > +	enum usb_role role = mode == DWC3_GCTL_PRTCAP_HOST ?
> > +				     USB_ROLE_HOST : USB_ROLE_DEVICE;
> >  
> > -	dwc3_set_mode(dwc, event ?
> > -		      DWC3_GCTL_PRTCAP_HOST :
> > -		      DWC3_GCTL_PRTCAP_DEVICE);
> > +	dwc3_pre_set_role(dwc, role);
> > +	dwc3_set_mode(dwc, mode);
> >  
> >  	return NOTIFY_DONE;
> >  }
> > -- 
> > 2.34.1
> > 
> 
> I don't like how the ternary operator is used here and how we've done it
> previously, especially when we're treating the event as if it's a
> boolean.
> 
> Regardless, it's a minor nit.
> 
> Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

OK, Will avoid such expression in the future.

Thanks,
Xu Yang

> 
> BR,
> Thinh

Re: [PATCH 2/2] usb: dwc3: imx8mp: disable auto suspend for host role

[Thinh Nguyen]

On Mon, Nov 17, 2025, Xu Yang wrote:
> On Thu, Nov 13, 2025 at 11:05:43PM +0000, Thinh Nguyen wrote:
> > On Wed, Nov 05, 2025, Xu Yang wrote:
> > > Do dwc3 core auto suspend enable for device and disable for host
> > > , this can make sure dwc3 core device auto suspend setting is
> > > correct all the time, the background of disable dwc3 core device
> > > auto suspend is to make its parent device suspend immediately
> > > (so wakeup enable can be enabled) after xhci-plat device suspended,
> > > for device mode, we keep the dwc3 core device auto suspend is to
> > > give some wait for gadget to be enumerated.
> > > 
> > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > > ---
> > >  drivers/usb/dwc3/dwc3-imx8mp.c | 31 +++++++++++++++++++++++++++++++
> > >  1 file changed, 31 insertions(+)
> > > 
> > > diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
> > > index bce6af82f54c..80a431cb6fed 100644
> > > --- a/drivers/usb/dwc3/dwc3-imx8mp.c
> > > +++ b/drivers/usb/dwc3/dwc3-imx8mp.c
> > > @@ -158,11 +158,31 @@ static irqreturn_t dwc3_imx8mp_interrupt(int irq, void *_dwc3_imx)
> > >  	return IRQ_HANDLED;
> > >  }
> > >  
> > > +static void dwc3_imx_pre_set_role(struct dwc3 *dwc, enum usb_role role)
> > > +{
> > > +	if (role == USB_ROLE_HOST)
> > > +		/*
> > > +		 * For xhci host, we need disable dwc core auto
> > > +		 * suspend, because during this auto suspend delay(5s),
> > > +		 * xhci host RUN_STOP is cleared and wakeup is not
> > > +		 * enabled, if device is inserted, xhci host can't
> > > +		 * response the connection.
> > > +		 */
> > > +		pm_runtime_dont_use_autosuspend(dwc->dev);
> > > +	else
> > > +		pm_runtime_use_autosuspend(dwc->dev);
> > 
> > Would this override the user setting everytime there's a role switch?
> 
> From what I know, the user can't control whether to enable or disable
> autosuspend feature. So there should be no overriding problem. When
> user change autosuspend_delay_ms, the delay setting will be kept
> everytime there's a role switch.

Should we set pm_runtime_mark_last_busy()?

Also, if pm_runtime_use_autosuspend() is set, make sure to unwind with
pm_runtime_dont_use_autosuspend() on remove.

> 
> > 
> > > +}
> > > +
> > > +struct dwc3_glue_ops dwc3_imx_glue_ops = {
> > > +	.pre_set_role   = dwc3_imx_pre_set_role,
> > > +};
> > > +
> > >  static int dwc3_imx8mp_probe(struct platform_device *pdev)
> > >  {
> > >  	struct device		*dev = &pdev->dev;
> > >  	struct device_node	*node = dev->of_node;
> > >  	struct dwc3_imx8mp	*dwc3_imx;
> > > +	struct dwc3		*dwc3;
> > >  	struct resource		*res;
> > >  	int			err, irq;
> > >  
> > > @@ -240,6 +260,17 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
> > >  		goto depopulate;
> > >  	}
> > >  
> > > +	dwc3 = platform_get_drvdata(dwc3_imx->dwc3);
> > 
> > It's confusing how dwc3_imx->dwc3 is also named dwc3...
> 
> I will rename it later.
> 
> > 
> > > +	if (!dwc3) {
> > > +		err = dev_err_probe(dev, -EPROBE_DEFER, "failed to get dwc3 platform data\n");
> > > +		goto depopulate;
> > > +	}
> > > +
> > > +	dwc3->glue_ops = &dwc3_imx_glue_ops;
> > 
> > If you want to use glue_ops, please use the new flatten model. I
> > don't want dwc3 to be initialized again after of_platform_populate().
> 
> I understand the new flatten model is a more suitable way. Considering that
> many i.MX devices are using this legacy unflatten model, do you mind allow
> this change firstly and we will switch to the new flatten model in the future?
> 

Do you have plans for this transition? Perhaps this is a good time to
initiate the process so to avoid having and maintaining this improper
code while we still don't have it yet.

BR,
Thinh

Re: [PATCH 2/2] usb: dwc3: imx8mp: disable auto suspend for host role

[Xu Yang]

On Fri, Nov 21, 2025 at 12:37:47AM +0000, Thinh Nguyen wrote:
> On Mon, Nov 17, 2025, Xu Yang wrote:
> > On Thu, Nov 13, 2025 at 11:05:43PM +0000, Thinh Nguyen wrote:
> > > On Wed, Nov 05, 2025, Xu Yang wrote:
> > > > Do dwc3 core auto suspend enable for device and disable for host
> > > > , this can make sure dwc3 core device auto suspend setting is
> > > > correct all the time, the background of disable dwc3 core device
> > > > auto suspend is to make its parent device suspend immediately
> > > > (so wakeup enable can be enabled) after xhci-plat device suspended,
> > > > for device mode, we keep the dwc3 core device auto suspend is to
> > > > give some wait for gadget to be enumerated.
> > > > 
> > > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > > > ---
> > > >  drivers/usb/dwc3/dwc3-imx8mp.c | 31 +++++++++++++++++++++++++++++++
> > > >  1 file changed, 31 insertions(+)
> > > > 
> > > > diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
> > > > index bce6af82f54c..80a431cb6fed 100644
> > > > --- a/drivers/usb/dwc3/dwc3-imx8mp.c
> > > > +++ b/drivers/usb/dwc3/dwc3-imx8mp.c
> > > > @@ -158,11 +158,31 @@ static irqreturn_t dwc3_imx8mp_interrupt(int irq, void *_dwc3_imx)
> > > >  	return IRQ_HANDLED;
> > > >  }
> > > >  
> > > > +static void dwc3_imx_pre_set_role(struct dwc3 *dwc, enum usb_role role)
> > > > +{
> > > > +	if (role == USB_ROLE_HOST)
> > > > +		/*
> > > > +		 * For xhci host, we need disable dwc core auto
> > > > +		 * suspend, because during this auto suspend delay(5s),
> > > > +		 * xhci host RUN_STOP is cleared and wakeup is not
> > > > +		 * enabled, if device is inserted, xhci host can't
> > > > +		 * response the connection.
> > > > +		 */
> > > > +		pm_runtime_dont_use_autosuspend(dwc->dev);
> > > > +	else
> > > > +		pm_runtime_use_autosuspend(dwc->dev);
> > > 
> > > Would this override the user setting everytime there's a role switch?
> > 
> > From what I know, the user can't control whether to enable or disable
> > autosuspend feature. So there should be no overriding problem. When
> > user change autosuspend_delay_ms, the delay setting will be kept
> > everytime there's a role switch.
> 
> Should we set pm_runtime_mark_last_busy()?

I think we needn't explicitly call pm_runtime_mark_last_busy().
Since commit "08071e64cb64 PM: runtime: Mark last busy stamp in pm_runtime_autosuspend()",
pm_runtime_autosuspend() will do that thing automatically once dwc3 device is idle.

> 
> Also, if pm_runtime_use_autosuspend() is set, make sure to unwind with
> pm_runtime_dont_use_autosuspend() on remove.
> 
> > 
> > > 
> > > > +}
> > > > +
> > > > +struct dwc3_glue_ops dwc3_imx_glue_ops = {
> > > > +	.pre_set_role   = dwc3_imx_pre_set_role,
> > > > +};
> > > > +
> > > >  static int dwc3_imx8mp_probe(struct platform_device *pdev)
> > > >  {
> > > >  	struct device		*dev = &pdev->dev;
> > > >  	struct device_node	*node = dev->of_node;
> > > >  	struct dwc3_imx8mp	*dwc3_imx;
> > > > +	struct dwc3		*dwc3;
> > > >  	struct resource		*res;
> > > >  	int			err, irq;
> > > >  
> > > > @@ -240,6 +260,17 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
> > > >  		goto depopulate;
> > > >  	}
> > > >  
> > > > +	dwc3 = platform_get_drvdata(dwc3_imx->dwc3);
> > > 
> > > It's confusing how dwc3_imx->dwc3 is also named dwc3...
> > 
> > I will rename it later.
> > 
> > > 
> > > > +	if (!dwc3) {
> > > > +		err = dev_err_probe(dev, -EPROBE_DEFER, "failed to get dwc3 platform data\n");
> > > > +		goto depopulate;
> > > > +	}
> > > > +
> > > > +	dwc3->glue_ops = &dwc3_imx_glue_ops;
> > > 
> > > If you want to use glue_ops, please use the new flatten model. I
> > > don't want dwc3 to be initialized again after of_platform_populate().
> > 
> > I understand the new flatten model is a more suitable way. Considering that
> > many i.MX devices are using this legacy unflatten model, do you mind allow
> > this change firstly and we will switch to the new flatten model in the future?
> > 
> 
> Do you have plans for this transition? Perhaps this is a good time to
> initiate the process so to avoid having and maintaining this improper
> code while we still don't have it yet.

OK. It's a trend to use new flatten model. I do have plan for it. I'll
prepare for it later.

Thanks,
Xu Yang

> 
> BR,
> Thinh

Re: [PATCH 2/2] usb: dwc3: imx8mp: disable auto suspend for host role

[Thinh Nguyen]

On Tue, Nov 25, 2025, Xu Yang wrote:
> On Fri, Nov 21, 2025 at 12:37:47AM +0000, Thinh Nguyen wrote:
> > On Mon, Nov 17, 2025, Xu Yang wrote:
> > > On Thu, Nov 13, 2025 at 11:05:43PM +0000, Thinh Nguyen wrote:
> > > > On Wed, Nov 05, 2025, Xu Yang wrote:
> > > > > Do dwc3 core auto suspend enable for device and disable for host
> > > > > , this can make sure dwc3 core device auto suspend setting is
> > > > > correct all the time, the background of disable dwc3 core device
> > > > > auto suspend is to make its parent device suspend immediately
> > > > > (so wakeup enable can be enabled) after xhci-plat device suspended,
> > > > > for device mode, we keep the dwc3 core device auto suspend is to
> > > > > give some wait for gadget to be enumerated.
> > > > > 
> > > > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > > > > ---
> > > > >  drivers/usb/dwc3/dwc3-imx8mp.c | 31 +++++++++++++++++++++++++++++++
> > > > >  1 file changed, 31 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
> > > > > index bce6af82f54c..80a431cb6fed 100644
> > > > > --- a/drivers/usb/dwc3/dwc3-imx8mp.c
> > > > > +++ b/drivers/usb/dwc3/dwc3-imx8mp.c
> > > > > @@ -158,11 +158,31 @@ static irqreturn_t dwc3_imx8mp_interrupt(int irq, void *_dwc3_imx)
> > > > >  	return IRQ_HANDLED;
> > > > >  }
> > > > >  
> > > > > +static void dwc3_imx_pre_set_role(struct dwc3 *dwc, enum usb_role role)
> > > > > +{
> > > > > +	if (role == USB_ROLE_HOST)
> > > > > +		/*
> > > > > +		 * For xhci host, we need disable dwc core auto
> > > > > +		 * suspend, because during this auto suspend delay(5s),
> > > > > +		 * xhci host RUN_STOP is cleared and wakeup is not
> > > > > +		 * enabled, if device is inserted, xhci host can't
> > > > > +		 * response the connection.
> > > > > +		 */
> > > > > +		pm_runtime_dont_use_autosuspend(dwc->dev);
> > > > > +	else
> > > > > +		pm_runtime_use_autosuspend(dwc->dev);
> > > > 
> > > > Would this override the user setting everytime there's a role switch?
> > > 
> > > From what I know, the user can't control whether to enable or disable
> > > autosuspend feature. So there should be no overriding problem. When
> > > user change autosuspend_delay_ms, the delay setting will be kept
> > > everytime there's a role switch.
> > 
> > Should we set pm_runtime_mark_last_busy()?
> 
> I think we needn't explicitly call pm_runtime_mark_last_busy().
> Since commit "08071e64cb64 PM: runtime: Mark last busy stamp in pm_runtime_autosuspend()",
> pm_runtime_autosuspend() will do that thing automatically once dwc3 device is idle.

Ok.

> 
> > 
> > Also, if pm_runtime_use_autosuspend() is set, make sure to unwind with
> > pm_runtime_dont_use_autosuspend() on remove.
> > 
> > > 
> > > > 
> > > > > +}
> > > > > +
> > > > > +struct dwc3_glue_ops dwc3_imx_glue_ops = {
> > > > > +	.pre_set_role   = dwc3_imx_pre_set_role,
> > > > > +};
> > > > > +
> > > > >  static int dwc3_imx8mp_probe(struct platform_device *pdev)
> > > > >  {
> > > > >  	struct device		*dev = &pdev->dev;
> > > > >  	struct device_node	*node = dev->of_node;
> > > > >  	struct dwc3_imx8mp	*dwc3_imx;
> > > > > +	struct dwc3		*dwc3;
> > > > >  	struct resource		*res;
> > > > >  	int			err, irq;
> > > > >  
> > > > > @@ -240,6 +260,17 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
> > > > >  		goto depopulate;
> > > > >  	}
> > > > >  
> > > > > +	dwc3 = platform_get_drvdata(dwc3_imx->dwc3);
> > > > 
> > > > It's confusing how dwc3_imx->dwc3 is also named dwc3...
> > > 
> > > I will rename it later.
> > > 
> > > > 
> > > > > +	if (!dwc3) {
> > > > > +		err = dev_err_probe(dev, -EPROBE_DEFER, "failed to get dwc3 platform data\n");
> > > > > +		goto depopulate;
> > > > > +	}
> > > > > +
> > > > > +	dwc3->glue_ops = &dwc3_imx_glue_ops;
> > > > 
> > > > If you want to use glue_ops, please use the new flatten model. I
> > > > don't want dwc3 to be initialized again after of_platform_populate().
> > > 
> > > I understand the new flatten model is a more suitable way. Considering that
> > > many i.MX devices are using this legacy unflatten model, do you mind allow
> > > this change firstly and we will switch to the new flatten model in the future?
> > > 
> > 
> > Do you have plans for this transition? Perhaps this is a good time to
> > initiate the process so to avoid having and maintaining this improper
> > code while we still don't have it yet.
> 
> OK. It's a trend to use new flatten model. I do have plan for it. I'll
> prepare for it later.
> 

Ok. Can we add a note that this is a temporary solution until we switch
to using the new model in the commit message.

Thanks,
Thinh

Re: [PATCH 2/2] usb: dwc3: imx8mp: disable auto suspend for host role

[Xu Yang]

On Tue, Dec 02, 2025 at 12:00:16AM +0000, Thinh Nguyen wrote:
> On Tue, Nov 25, 2025, Xu Yang wrote:
> > On Fri, Nov 21, 2025 at 12:37:47AM +0000, Thinh Nguyen wrote:
> > > On Mon, Nov 17, 2025, Xu Yang wrote:
> > > > On Thu, Nov 13, 2025 at 11:05:43PM +0000, Thinh Nguyen wrote:
> > > > > On Wed, Nov 05, 2025, Xu Yang wrote:
> > > > > > Do dwc3 core auto suspend enable for device and disable for host
> > > > > > , this can make sure dwc3 core device auto suspend setting is
> > > > > > correct all the time, the background of disable dwc3 core device
> > > > > > auto suspend is to make its parent device suspend immediately
> > > > > > (so wakeup enable can be enabled) after xhci-plat device suspended,
> > > > > > for device mode, we keep the dwc3 core device auto suspend is to
> > > > > > give some wait for gadget to be enumerated.
> > > > > > 
> > > > > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > > > > > ---
> > > > > >  drivers/usb/dwc3/dwc3-imx8mp.c | 31 +++++++++++++++++++++++++++++++
> > > > > >  1 file changed, 31 insertions(+)
> > > > > > 
> > > > > > diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
> > > > > > index bce6af82f54c..80a431cb6fed 100644
> > > > > > --- a/drivers/usb/dwc3/dwc3-imx8mp.c
> > > > > > +++ b/drivers/usb/dwc3/dwc3-imx8mp.c
> > > > > > @@ -158,11 +158,31 @@ static irqreturn_t dwc3_imx8mp_interrupt(int irq, void *_dwc3_imx)
> > > > > >  	return IRQ_HANDLED;
> > > > > >  }
> > > > > >  
> > > > > > +static void dwc3_imx_pre_set_role(struct dwc3 *dwc, enum usb_role role)
> > > > > > +{
> > > > > > +	if (role == USB_ROLE_HOST)
> > > > > > +		/*
> > > > > > +		 * For xhci host, we need disable dwc core auto
> > > > > > +		 * suspend, because during this auto suspend delay(5s),
> > > > > > +		 * xhci host RUN_STOP is cleared and wakeup is not
> > > > > > +		 * enabled, if device is inserted, xhci host can't
> > > > > > +		 * response the connection.
> > > > > > +		 */
> > > > > > +		pm_runtime_dont_use_autosuspend(dwc->dev);
> > > > > > +	else
> > > > > > +		pm_runtime_use_autosuspend(dwc->dev);
> > > > > 
> > > > > Would this override the user setting everytime there's a role switch?
> > > > 
> > > > From what I know, the user can't control whether to enable or disable
> > > > autosuspend feature. So there should be no overriding problem. When
> > > > user change autosuspend_delay_ms, the delay setting will be kept
> > > > everytime there's a role switch.
> > > 
> > > Should we set pm_runtime_mark_last_busy()?
> > 
> > I think we needn't explicitly call pm_runtime_mark_last_busy().
> > Since commit "08071e64cb64 PM: runtime: Mark last busy stamp in pm_runtime_autosuspend()",
> > pm_runtime_autosuspend() will do that thing automatically once dwc3 device is idle.
> 
> Ok.
> 
> > 
> > > 
> > > Also, if pm_runtime_use_autosuspend() is set, make sure to unwind with
> > > pm_runtime_dont_use_autosuspend() on remove.
> > > 
> > > > 
> > > > > 
> > > > > > +}
> > > > > > +
> > > > > > +struct dwc3_glue_ops dwc3_imx_glue_ops = {
> > > > > > +	.pre_set_role   = dwc3_imx_pre_set_role,
> > > > > > +};
> > > > > > +
> > > > > >  static int dwc3_imx8mp_probe(struct platform_device *pdev)
> > > > > >  {
> > > > > >  	struct device		*dev = &pdev->dev;
> > > > > >  	struct device_node	*node = dev->of_node;
> > > > > >  	struct dwc3_imx8mp	*dwc3_imx;
> > > > > > +	struct dwc3		*dwc3;
> > > > > >  	struct resource		*res;
> > > > > >  	int			err, irq;
> > > > > >  
> > > > > > @@ -240,6 +260,17 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
> > > > > >  		goto depopulate;
> > > > > >  	}
> > > > > >  
> > > > > > +	dwc3 = platform_get_drvdata(dwc3_imx->dwc3);
> > > > > 
> > > > > It's confusing how dwc3_imx->dwc3 is also named dwc3...
> > > > 
> > > > I will rename it later.
> > > > 
> > > > > 
> > > > > > +	if (!dwc3) {
> > > > > > +		err = dev_err_probe(dev, -EPROBE_DEFER, "failed to get dwc3 platform data\n");
> > > > > > +		goto depopulate;
> > > > > > +	}
> > > > > > +
> > > > > > +	dwc3->glue_ops = &dwc3_imx_glue_ops;
> > > > > 
> > > > > If you want to use glue_ops, please use the new flatten model. I
> > > > > don't want dwc3 to be initialized again after of_platform_populate().
> > > > 
> > > > I understand the new flatten model is a more suitable way. Considering that
> > > > many i.MX devices are using this legacy unflatten model, do you mind allow
> > > > this change firstly and we will switch to the new flatten model in the future?
> > > > 
> > > 
> > > Do you have plans for this transition? Perhaps this is a good time to
> > > initiate the process so to avoid having and maintaining this improper
> > > code while we still don't have it yet.
> > 
> > OK. It's a trend to use new flatten model. I do have plan for it. I'll
> > prepare for it later.
> > 
> 
> Ok. Can we add a note that this is a temporary solution until we switch
> to using the new model in the commit message.

Ok. Will do. Thank you!

Thanks,
Xu Yang

> 
> Thanks,
> Thinh