[PATCH 1/3] usb: chipidea: add USB PHY event

[PATCH 1/3] usb: chipidea: add USB PHY event

[Xu Yang]

Add USB PHY event for below situation:
- usb role changed
- vbus connect
- vbus disconnect
- gadget driver is enumerated

USB PHY driver can get the last event after above situation occurs
and deal with different situations.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 drivers/usb/chipidea/ci.h  | 18 ++++++++++++++++--
 drivers/usb/chipidea/udc.c | 10 ++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index f210b7489fd5..d262b9df7b3d 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -281,8 +281,19 @@ static inline int ci_role_start(struct ci_hdrc *ci, enum ci_role role)
 		return -ENXIO;
 
 	ret = ci->roles[role]->start(ci);
-	if (!ret)
-		ci->role = role;
+	if (ret)
+		return ret;
+
+	ci->role = role;
+
+	if (ci->usb_phy) {
+		if (role == CI_ROLE_HOST)
+			usb_phy_set_event(ci->usb_phy, USB_EVENT_ID);
+		else
+			/* in device mode but vbus is invalid*/
+			usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
+	}
+
 	return ret;
 }
 
@@ -296,6 +307,9 @@ static inline void ci_role_stop(struct ci_hdrc *ci)
 	ci->role = CI_ROLE_END;
 
 	ci->roles[role]->stop(ci);
+
+	if (ci->usb_phy)
+		usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
 }
 
 static inline enum usb_role ci_role_to_usb_role(struct ci_hdrc *ci)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 54c09245ad05..d58355427eeb 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1718,6 +1718,13 @@ static int ci_udc_vbus_session(struct usb_gadget *_gadget, int is_active)
 		ret = ci->platdata->notify_event(ci,
 				CI_HDRC_CONTROLLER_VBUS_EVENT);
 
+	if (ci->usb_phy) {
+		if (is_active)
+			usb_phy_set_event(ci->usb_phy, USB_EVENT_VBUS);
+		else
+			usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
+	}
+
 	if (ci->driver)
 		ci_hdrc_gadget_connect(_gadget, is_active);
 
@@ -2034,6 +2041,9 @@ static irqreturn_t udc_irq(struct ci_hdrc *ci)
 		if (USBi_PCI & intr) {
 			ci->gadget.speed = hw_port_is_high_speed(ci) ?
 				USB_SPEED_HIGH : USB_SPEED_FULL;
+			if (ci->usb_phy)
+				usb_phy_set_event(ci->usb_phy,
+					USB_EVENT_ENUMERATED);
 			if (ci->suspended) {
 				if (ci->driver->resume) {
 					spin_unlock(&ci->lock);
-- 
2.34.1

[PATCH 2/3] usb: phy: mxs: fix getting wrong state with mxs_phy_is_otg_host()

[Xu Yang]

The function mxs_phy_is_otg_host() will return true if OTG_ID_VALUE is
0 at USBPHY_CTRL register. However, OTG_ID_VALUE will not reflect the real
state if the ID pin is float, such as Host-only or Type-C cases. The value
of OTG_ID_VALUE is always 1 which means device mode.
This patch will fix the issue by judging the current mode based on
last_event. The controller will update last_event in time.

Fixes: 7b09e67639d6 ("usb: phy: mxs: refine mxs_phy_disconnect_line")
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 drivers/usb/phy/phy-mxs-usb.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index 036bb58a3a71..f484c79efa6c 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -388,14 +388,8 @@ static void __mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool disconnect)
 
 static bool mxs_phy_is_otg_host(struct mxs_phy *mxs_phy)
 {
-	void __iomem *base = mxs_phy->phy.io_priv;
-	u32 phyctrl = readl(base + HW_USBPHY_CTRL);
-
-	if (IS_ENABLED(CONFIG_USB_OTG) &&
-			!(phyctrl & BM_USBPHY_CTRL_OTG_ID_VALUE))
-		return true;
-
-	return false;
+	return IS_ENABLED(CONFIG_USB_OTG) &&
+		mxs_phy->phy.last_event == USB_EVENT_ID;
 }
 
 static void mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool on)
-- 
2.34.1

[PATCH 3/3] usb: phy: mxs: disconnect line when USB charger is attached

[Xu Yang]

For mxs PHY, if there is a vbus but the bus is not enumerated, we need
to force the dp/dm as SE0 from the controller side. If not, there is
possible USB wakeup due to unstable dp/dm, since there is possible no
pull on dp/dm, such as there is a USB charger on the port.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 drivers/usb/phy/phy-mxs-usb.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index f484c79efa6c..619411aab86c 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -395,6 +395,7 @@ static bool mxs_phy_is_otg_host(struct mxs_phy *mxs_phy)
 static void mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool on)
 {
 	bool vbus_is_on = false;
+	enum usb_phy_events last_event = mxs_phy->phy.last_event;
 
 	/* If the SoCs don't need to disconnect line without vbus, quit */
 	if (!(mxs_phy->data->flags & MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS))
@@ -406,7 +407,8 @@ static void mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool on)
 
 	vbus_is_on = mxs_phy_get_vbus_status(mxs_phy);
 
-	if (on && !vbus_is_on && !mxs_phy_is_otg_host(mxs_phy))
+	if (on && ((!vbus_is_on && !mxs_phy_is_otg_host(mxs_phy))
+		|| (last_event == USB_EVENT_VBUS)))
 		__mxs_phy_disconnect_line(mxs_phy, true);
 	else
 		__mxs_phy_disconnect_line(mxs_phy, false);
-- 
2.34.1

Re: [PATCH 1/3] usb: chipidea: add USB PHY event

[Peter Chen]

On 23-06-27 19:03:51, Xu Yang wrote:
> Add USB PHY event for below situation:
> - usb role changed
> - vbus connect
> - vbus disconnect
> - gadget driver is enumerated
> 
> USB PHY driver can get the last event after above situation occurs
> and deal with different situations.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

Acked-by: Peter Chen <peter.chen@kernel.org>

Peter
> ---
>  drivers/usb/chipidea/ci.h  | 18 ++++++++++++++++--
>  drivers/usb/chipidea/udc.c | 10 ++++++++++
>  2 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index f210b7489fd5..d262b9df7b3d 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -281,8 +281,19 @@ static inline int ci_role_start(struct ci_hdrc *ci, enum ci_role role)
>  		return -ENXIO;
>  
>  	ret = ci->roles[role]->start(ci);
> -	if (!ret)
> -		ci->role = role;
> +	if (ret)
> +		return ret;
> +
> +	ci->role = role;
> +
> +	if (ci->usb_phy) {
> +		if (role == CI_ROLE_HOST)
> +			usb_phy_set_event(ci->usb_phy, USB_EVENT_ID);
> +		else
> +			/* in device mode but vbus is invalid*/
> +			usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
> +	}
> +
>  	return ret;
>  }
>  
> @@ -296,6 +307,9 @@ static inline void ci_role_stop(struct ci_hdrc *ci)
>  	ci->role = CI_ROLE_END;
>  
>  	ci->roles[role]->stop(ci);
> +
> +	if (ci->usb_phy)
> +		usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
>  }
>  
>  static inline enum usb_role ci_role_to_usb_role(struct ci_hdrc *ci)
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 54c09245ad05..d58355427eeb 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -1718,6 +1718,13 @@ static int ci_udc_vbus_session(struct usb_gadget *_gadget, int is_active)
>  		ret = ci->platdata->notify_event(ci,
>  				CI_HDRC_CONTROLLER_VBUS_EVENT);
>  
> +	if (ci->usb_phy) {
> +		if (is_active)
> +			usb_phy_set_event(ci->usb_phy, USB_EVENT_VBUS);
> +		else
> +			usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
> +	}
> +
>  	if (ci->driver)
>  		ci_hdrc_gadget_connect(_gadget, is_active);
>  
> @@ -2034,6 +2041,9 @@ static irqreturn_t udc_irq(struct ci_hdrc *ci)
>  		if (USBi_PCI & intr) {
>  			ci->gadget.speed = hw_port_is_high_speed(ci) ?
>  				USB_SPEED_HIGH : USB_SPEED_FULL;
> +			if (ci->usb_phy)
> +				usb_phy_set_event(ci->usb_phy,
> +					USB_EVENT_ENUMERATED);
>  			if (ci->suspended) {
>  				if (ci->driver->resume) {
>  					spin_unlock(&ci->lock);
> -- 
> 2.34.1
> 

-- 

Thanks,
Peter Chen

Re: [PATCH 2/3] usb: phy: mxs: fix getting wrong state with mxs_phy_is_otg_host()

[Peter Chen]

On 23-06-27 19:03:52, Xu Yang wrote:
> The function mxs_phy_is_otg_host() will return true if OTG_ID_VALUE is
> 0 at USBPHY_CTRL register. However, OTG_ID_VALUE will not reflect the real
> state if the ID pin is float, such as Host-only or Type-C cases. The value
> of OTG_ID_VALUE is always 1 which means device mode.
> This patch will fix the issue by judging the current mode based on
> last_event. The controller will update last_event in time.
> 
> Fixes: 7b09e67639d6 ("usb: phy: mxs: refine mxs_phy_disconnect_line")
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

Acked-by: Peter Chen <peter.chen@kernel.org>

Peter
> ---
>  drivers/usb/phy/phy-mxs-usb.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index 036bb58a3a71..f484c79efa6c 100644
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -388,14 +388,8 @@ static void __mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool disconnect)
>  
>  static bool mxs_phy_is_otg_host(struct mxs_phy *mxs_phy)
>  {
> -	void __iomem *base = mxs_phy->phy.io_priv;
> -	u32 phyctrl = readl(base + HW_USBPHY_CTRL);
> -
> -	if (IS_ENABLED(CONFIG_USB_OTG) &&
> -			!(phyctrl & BM_USBPHY_CTRL_OTG_ID_VALUE))
> -		return true;
> -
> -	return false;
> +	return IS_ENABLED(CONFIG_USB_OTG) &&
> +		mxs_phy->phy.last_event == USB_EVENT_ID;
>  }
>  
>  static void mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool on)
> -- 
> 2.34.1
> 

-- 

Thanks,
Peter Chen

Re: [PATCH 3/3] usb: phy: mxs: disconnect line when USB charger is attached

[Peter Chen]

On 23-06-27 19:03:53, Xu Yang wrote:
> For mxs PHY, if there is a vbus but the bus is not enumerated, we need
> to force the dp/dm as SE0 from the controller side. If not, there is
> possible USB wakeup due to unstable dp/dm, since there is possible no
> pull on dp/dm, such as there is a USB charger on the port.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

Acked-by: Peter Chen <peter.chen@kernel.org>

Peter
> ---
>  drivers/usb/phy/phy-mxs-usb.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index f484c79efa6c..619411aab86c 100644
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -395,6 +395,7 @@ static bool mxs_phy_is_otg_host(struct mxs_phy *mxs_phy)
>  static void mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool on)
>  {
>  	bool vbus_is_on = false;
> +	enum usb_phy_events last_event = mxs_phy->phy.last_event;
>  
>  	/* If the SoCs don't need to disconnect line without vbus, quit */
>  	if (!(mxs_phy->data->flags & MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS))
> @@ -406,7 +407,8 @@ static void mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool on)
>  
>  	vbus_is_on = mxs_phy_get_vbus_status(mxs_phy);
>  
> -	if (on && !vbus_is_on && !mxs_phy_is_otg_host(mxs_phy))
> +	if (on && ((!vbus_is_on && !mxs_phy_is_otg_host(mxs_phy))
> +		|| (last_event == USB_EVENT_VBUS)))
>  		__mxs_phy_disconnect_line(mxs_phy, true);
>  	else
>  		__mxs_phy_disconnect_line(mxs_phy, false);
> -- 
> 2.34.1
> 

-- 

Thanks,
Peter Chen

Re: [PATCH 1/3] usb: chipidea: add USB PHY event

[Luca Ceresoli]

Hello Xu,

On Tue, 27 Jun 2023 19:03:51 +0800
Xu Yang <xu.yang_2@nxp.com> wrote:

> Add USB PHY event for below situation:
> - usb role changed
> - vbus connect
> - vbus disconnect
> - gadget driver is enumerated
> 
> USB PHY driver can get the last event after above situation occurs
> and deal with different situations.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

I tested this patchset on top of v6.5-rc2 and USB detection
is still broken on the Colibri iMX6ULL. With or without the patches
the behavior is the same: USB devices are detected only during boot,
and anything connected after boot is never detected.

Is there anything I can test for you do understand what's going wrong
here?

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 2/3] usb: phy: mxs: fix getting wrong state with mxs_phy_is_otg_host()

[Francesco Dolcini]

On Tue, Jun 27, 2023 at 07:03:52PM +0800, Xu Yang wrote:
> The function mxs_phy_is_otg_host() will return true if OTG_ID_VALUE is
> 0 at USBPHY_CTRL register. However, OTG_ID_VALUE will not reflect the real
> state if the ID pin is float, such as Host-only or Type-C cases. The value
> of OTG_ID_VALUE is always 1 which means device mode.
> This patch will fix the issue by judging the current mode based on
> last_event. The controller will update last_event in time.
> 
> Fixes: 7b09e67639d6 ("usb: phy: mxs: refine mxs_phy_disconnect_line")
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  drivers/usb/phy/phy-mxs-usb.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index 036bb58a3a71..f484c79efa6c 100644
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -388,14 +388,8 @@ static void __mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool disconnect)
>  
>  static bool mxs_phy_is_otg_host(struct mxs_phy *mxs_phy)
>  {
> -	void __iomem *base = mxs_phy->phy.io_priv;
> -	u32 phyctrl = readl(base + HW_USBPHY_CTRL);
> -
> -	if (IS_ENABLED(CONFIG_USB_OTG) &&
> -			!(phyctrl & BM_USBPHY_CTRL_OTG_ID_VALUE))
> -		return true;
> -
> -	return false;
> +	return IS_ENABLED(CONFIG_USB_OTG) &&
> +		mxs_phy->phy.last_event == USB_EVENT_ID;

The logic here is not working when CONFIG_USB_OTG, should we always
return true when !IS_ENABLED(CONFIG_USB_OTG) ?

so something like 

if (!IS_ENABLED(CONFIG_USB_OTG))
	return true;

return mxs_phy->phy.last_event == USB_EVENT_ID;


?

RE: [EXT] Re: [PATCH 2/3] usb: phy: mxs: fix getting wrong state with mxs_phy_is_otg_host()

[Xu Yang]

> On Tue, Jun 27, 2023 at 07:03:52PM +0800, Xu Yang wrote:
> > The function mxs_phy_is_otg_host() will return true if OTG_ID_VALUE is
> > 0 at USBPHY_CTRL register. However, OTG_ID_VALUE will not reflect the real
> > state if the ID pin is float, such as Host-only or Type-C cases. The value
> > of OTG_ID_VALUE is always 1 which means device mode.
> > This patch will fix the issue by judging the current mode based on
> > last_event. The controller will update last_event in time.
> >
> > Fixes: 7b09e67639d6 ("usb: phy: mxs: refine mxs_phy_disconnect_line")
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> >  drivers/usb/phy/phy-mxs-usb.c | 10 ++--------
> >  1 file changed, 2 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> > index 036bb58a3a71..f484c79efa6c 100644
> > --- a/drivers/usb/phy/phy-mxs-usb.c
> > +++ b/drivers/usb/phy/phy-mxs-usb.c
> > @@ -388,14 +388,8 @@ static void __mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool disconnect)
> >
> >  static bool mxs_phy_is_otg_host(struct mxs_phy *mxs_phy)
> >  {
> > -     void __iomem *base = mxs_phy->phy.io_priv;
> > -     u32 phyctrl = readl(base + HW_USBPHY_CTRL);
> > -
> > -     if (IS_ENABLED(CONFIG_USB_OTG) &&
> > -                     !(phyctrl & BM_USBPHY_CTRL_OTG_ID_VALUE))
> > -             return true;
> > -
> > -     return false;
> > +     return IS_ENABLED(CONFIG_USB_OTG) &&
> > +             mxs_phy->phy.last_event == USB_EVENT_ID;
> 
> The logic here is not working when CONFIG_USB_OTG, should we always
> return true when !IS_ENABLED(CONFIG_USB_OTG) ?

No. 

> 
> so something like
> 
> if (!IS_ENABLED(CONFIG_USB_OTG))
>         return true;
> 

Below code should be enough. We don't need to judge 
CONFIG_USB_OTG here since last_event always be updated
when starting new role.

> return mxs_phy->phy.last_event == USB_EVENT_ID;
> 
> 
> ?

RE: [PATCH 1/3] usb: chipidea: add USB PHY event

[Pu, Hui]

> Add USB PHY event for below situation:
> - usb role changed
> - vbus connect
> - vbus disconnect
> - gadget driver is enumerated
> 
> USB PHY driver can get the last event after above situation occurs and deal with
> different situations.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  drivers/usb/chipidea/ci.h  | 18 ++++++++++++++++--
> drivers/usb/chipidea/udc.c | 10 ++++++++++
>  2 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h index
> f210b7489fd5..d262b9df7b3d 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -281,8 +281,19 @@ static inline int ci_role_start(struct ci_hdrc *ci, enum
> ci_role role)
>  		return -ENXIO;
> 
>  	ret = ci->roles[role]->start(ci);
> -	if (!ret)
> -		ci->role = role;
> +	if (ret)
> +		return ret;
> +
> +	ci->role = role;
> +
> +	if (ci->usb_phy) {
> +		if (role == CI_ROLE_HOST)
> +			usb_phy_set_event(ci->usb_phy, USB_EVENT_ID);
> +		else
> +			/* in device mode but vbus is invalid*/
> +			usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
> +	}
> +
>  	return ret;
>  }
> 
> @@ -296,6 +307,9 @@ static inline void ci_role_stop(struct ci_hdrc *ci)
>  	ci->role = CI_ROLE_END;
> 
>  	ci->roles[role]->stop(ci);
> +
> +	if (ci->usb_phy)
> +		usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
>  }
> 
>  static inline enum usb_role ci_role_to_usb_role(struct ci_hdrc *ci) diff --git
> a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index
> 54c09245ad05..d58355427eeb 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -1718,6 +1718,13 @@ static int ci_udc_vbus_session(struct usb_gadget
> *_gadget, int is_active)
>  		ret = ci->platdata->notify_event(ci,
>  				CI_HDRC_CONTROLLER_VBUS_EVENT);
> 
> +	if (ci->usb_phy) {
> +		if (is_active)
> +			usb_phy_set_event(ci->usb_phy, USB_EVENT_VBUS);
> +		else
> +			usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
> +	}
> +
>  	if (ci->driver)
>  		ci_hdrc_gadget_connect(_gadget, is_active);
> 
> @@ -2034,6 +2041,9 @@ static irqreturn_t udc_irq(struct ci_hdrc *ci)
>  		if (USBi_PCI & intr) {
>  			ci->gadget.speed = hw_port_is_high_speed(ci) ?
>  				USB_SPEED_HIGH : USB_SPEED_FULL;
> +			if (ci->usb_phy)
> +				usb_phy_set_event(ci->usb_phy,
> +					USB_EVENT_ENUMERATED);
>  			if (ci->suspended) {
>  				if (ci->driver->resume) {
>  					spin_unlock(&ci->lock);
> --
> 2.34.1


Hi guys,

I'm not sure if I'm replying correctly, please correct me if any mistake.
(I didn't find the cover letter in this thread.)

This patchset has been merged on master branch, but only the 2/3 patch on linux-5.15.y and linux-6.1.y.
So, on 5.15.y and 6.1.y, there's a degradation on the i.MX6 devices that the usb hub cannot work well.

- Paul

Re: [PATCH 1/3] usb: chipidea: add USB PHY event

[Xu Yang]

Hi Paul,

On Tue, Aug 27, 2024 at 06:10:09AM +0000, Pu, Hui wrote:
> > Add USB PHY event for below situation:
> > - usb role changed
> > - vbus connect
> > - vbus disconnect
> > - gadget driver is enumerated
> > 
> > USB PHY driver can get the last event after above situation occurs and deal with
> > different situations.
> > 
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> >  drivers/usb/chipidea/ci.h  | 18 ++++++++++++++++--
> > drivers/usb/chipidea/udc.c | 10 ++++++++++
> >  2 files changed, 26 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h index
> > f210b7489fd5..d262b9df7b3d 100644
> > --- a/drivers/usb/chipidea/ci.h
> > +++ b/drivers/usb/chipidea/ci.h
> > @@ -281,8 +281,19 @@ static inline int ci_role_start(struct ci_hdrc *ci, enum
> > ci_role role)
> >  		return -ENXIO;
> > 
> >  	ret = ci->roles[role]->start(ci);
> > -	if (!ret)
> > -		ci->role = role;
> > +	if (ret)
> > +		return ret;
> > +
> > +	ci->role = role;
> > +
> > +	if (ci->usb_phy) {
> > +		if (role == CI_ROLE_HOST)
> > +			usb_phy_set_event(ci->usb_phy, USB_EVENT_ID);
> > +		else
> > +			/* in device mode but vbus is invalid*/
> > +			usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
> > +	}
> > +
> >  	return ret;
> >  }
> > 
> > @@ -296,6 +307,9 @@ static inline void ci_role_stop(struct ci_hdrc *ci)
> >  	ci->role = CI_ROLE_END;
> > 
> >  	ci->roles[role]->stop(ci);
> > +
> > +	if (ci->usb_phy)
> > +		usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
> >  }
> > 
> >  static inline enum usb_role ci_role_to_usb_role(struct ci_hdrc *ci) diff --git
> > a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index
> > 54c09245ad05..d58355427eeb 100644
> > --- a/drivers/usb/chipidea/udc.c
> > +++ b/drivers/usb/chipidea/udc.c
> > @@ -1718,6 +1718,13 @@ static int ci_udc_vbus_session(struct usb_gadget
> > *_gadget, int is_active)
> >  		ret = ci->platdata->notify_event(ci,
> >  				CI_HDRC_CONTROLLER_VBUS_EVENT);
> > 
> > +	if (ci->usb_phy) {
> > +		if (is_active)
> > +			usb_phy_set_event(ci->usb_phy, USB_EVENT_VBUS);
> > +		else
> > +			usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
> > +	}
> > +
> >  	if (ci->driver)
> >  		ci_hdrc_gadget_connect(_gadget, is_active);
> > 
> > @@ -2034,6 +2041,9 @@ static irqreturn_t udc_irq(struct ci_hdrc *ci)
> >  		if (USBi_PCI & intr) {
> >  			ci->gadget.speed = hw_port_is_high_speed(ci) ?
> >  				USB_SPEED_HIGH : USB_SPEED_FULL;
> > +			if (ci->usb_phy)
> > +				usb_phy_set_event(ci->usb_phy,
> > +					USB_EVENT_ENUMERATED);
> >  			if (ci->suspended) {
> >  				if (ci->driver->resume) {
> >  					spin_unlock(&ci->lock);
> > --
> > 2.34.1
> 
> 
> Hi guys,
> 
> I'm not sure if I'm replying correctly, please correct me if any mistake.
> (I didn't find the cover letter in this thread.)
> 
> This patchset has been merged on master branch, but only the 2/3 patch on linux-5.15.y and linux-6.1.y.
> So, on 5.15.y and 6.1.y, there's a degradation on the i.MX6 devices that the usb hub cannot work well.

Thanks for reporting this. 
Could the usb hub work well if you apply patch 1 and patch 3 to your kernel?

Thanks,
Xu Yang

> 
> - Paul

RE: [PATCH 1/3] usb: chipidea: add USB PHY event

[Pu, Hui]

Hi Xu Yang,

Thanks for reply.


> Hi Paul,
> 
> On Tue, Aug 27, 2024 at 06:10:09AM +0000, Pu, Hui wrote:
> > > Add USB PHY event for below situation:
> > > - usb role changed
> > > - vbus connect
> > > - vbus disconnect
> > > - gadget driver is enumerated
> > >
> > > USB PHY driver can get the last event after above situation occurs
> > > and deal with different situations.
> > >
> > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > > ---
> > >  drivers/usb/chipidea/ci.h  | 18 ++++++++++++++++--
> > > drivers/usb/chipidea/udc.c | 10 ++++++++++
> > >  2 files changed, 26 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> > > index f210b7489fd5..d262b9df7b3d 100644
> > > --- a/drivers/usb/chipidea/ci.h
> > > +++ b/drivers/usb/chipidea/ci.h
> > > @@ -281,8 +281,19 @@ static inline int ci_role_start(struct ci_hdrc
> > > *ci, enum ci_role role)
> > >             return -ENXIO;
> > >
> > >     ret = ci->roles[role]->start(ci);
> > > -   if (!ret)
> > > -           ci->role = role;
> > > +   if (ret)
> > > +           return ret;
> > > +
> > > +   ci->role = role;
> > > +
> > > +   if (ci->usb_phy) {
> > > +           if (role == CI_ROLE_HOST)
> > > +                   usb_phy_set_event(ci->usb_phy, USB_EVENT_ID);
> > > +           else
> > > +                   /* in device mode but vbus is invalid*/
> > > +                   usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
> > > +   }
> > > +
> > >     return ret;
> > >  }
> > >
> > > @@ -296,6 +307,9 @@ static inline void ci_role_stop(struct ci_hdrc *ci)
> > >     ci->role = CI_ROLE_END;
> > >
> > >     ci->roles[role]->stop(ci);
> > > +
> > > +   if (ci->usb_phy)
> > > +           usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
> > >  }
> > >
> > >  static inline enum usb_role ci_role_to_usb_role(struct ci_hdrc *ci)
> > > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> > > index 54c09245ad05..d58355427eeb 100644
> > > --- a/drivers/usb/chipidea/udc.c
> > > +++ b/drivers/usb/chipidea/udc.c
> > > @@ -1718,6 +1718,13 @@ static int ci_udc_vbus_session(struct
> > > usb_gadget *_gadget, int is_active)
> > >             ret = ci->platdata->notify_event(ci,
> > >                             CI_HDRC_CONTROLLER_VBUS_EVENT);
> > >
> > > +   if (ci->usb_phy) {
> > > +           if (is_active)
> > > +                   usb_phy_set_event(ci->usb_phy, USB_EVENT_VBUS);
> > > +           else
> > > +                   usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
> > > +   }
> > > +
> > >     if (ci->driver)
> > >             ci_hdrc_gadget_connect(_gadget, is_active);
> > >
> > > @@ -2034,6 +2041,9 @@ static irqreturn_t udc_irq(struct ci_hdrc *ci)
> > >             if (USBi_PCI & intr) {
> > >                     ci->gadget.speed = hw_port_is_high_speed(ci) ?
> > >                             USB_SPEED_HIGH : USB_SPEED_FULL;
> > > +                   if (ci->usb_phy)
> > > +                           usb_phy_set_event(ci->usb_phy,
> > > +                                   USB_EVENT_ENUMERATED);
> > >                     if (ci->suspended) {
> > >                             if (ci->driver->resume) {
> > >                                     spin_unlock(&ci->lock);
> > > --
> > > 2.34.1
> >
> >
> > Hi guys,
> >
> > I'm not sure if I'm replying correctly, please correct me if any mistake.
> > (I didn't find the cover letter in this thread.)
> >
> > This patchset has been merged on master branch, but only the 2/3 patch on
> linux-5.15.y and linux-6.1.y.
> > So, on 5.15.y and 6.1.y, there's a degradation on the i.MX6 devices that the
> usb hub cannot work well.
> 
> Thanks for reporting this.
> Could the usb hub work well if you apply patch 1 and patch 3 to your kernel?


Yes, work well.

- Paul


> 
> Thanks,
> Xu Yang
> 
> >
> > - Paul

Re: [PATCH 1/3] usb: chipidea: add USB PHY event

[Xu Yang]

Hi Greg and Sasha,

On Tue, Jun 27, 2023 at 07:03:51PM +0800, Xu Yang wrote:
> Add USB PHY event for below situation:
> - usb role changed
> - vbus connect
> - vbus disconnect
> - gadget driver is enumerated
> 
> USB PHY driver can get the last event after above situation occurs
> and deal with different situations.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

These 3 patches had already merged to usb tree. But I didn't add fix tag
to patch #1 and #3, therefore, only patch #2 went to stable tree. Now
the stable tree linux-5.15.y and linux-6.1.y have issue due to patch #2
depends on patch #1. So could you please add patch #1 and patch #3 to
linux-5.15.y and linux-6.1.y? Or should I post a normal request to achieve
this? 

Thanks in advance!

Best Regards,
Xu Yang

> ---
>  drivers/usb/chipidea/ci.h  | 18 ++++++++++++++++--
>  drivers/usb/chipidea/udc.c | 10 ++++++++++
>  2 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index f210b7489fd5..d262b9df7b3d 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -281,8 +281,19 @@ static inline int ci_role_start(struct ci_hdrc *ci, enum ci_role role)
>  		return -ENXIO;
>  
>  	ret = ci->roles[role]->start(ci);
> -	if (!ret)
> -		ci->role = role;
> +	if (ret)
> +		return ret;
> +
> +	ci->role = role;
> +
> +	if (ci->usb_phy) {
> +		if (role == CI_ROLE_HOST)
> +			usb_phy_set_event(ci->usb_phy, USB_EVENT_ID);
> +		else
> +			/* in device mode but vbus is invalid*/
> +			usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
> +	}
> +
>  	return ret;
>  }
>  
> @@ -296,6 +307,9 @@ static inline void ci_role_stop(struct ci_hdrc *ci)
>  	ci->role = CI_ROLE_END;
>  
>  	ci->roles[role]->stop(ci);
> +
> +	if (ci->usb_phy)
> +		usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
>  }
>  
>  static inline enum usb_role ci_role_to_usb_role(struct ci_hdrc *ci)
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 54c09245ad05..d58355427eeb 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -1718,6 +1718,13 @@ static int ci_udc_vbus_session(struct usb_gadget *_gadget, int is_active)
>  		ret = ci->platdata->notify_event(ci,
>  				CI_HDRC_CONTROLLER_VBUS_EVENT);
>  
> +	if (ci->usb_phy) {
> +		if (is_active)
> +			usb_phy_set_event(ci->usb_phy, USB_EVENT_VBUS);
> +		else
> +			usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
> +	}
> +
>  	if (ci->driver)
>  		ci_hdrc_gadget_connect(_gadget, is_active);
>  
> @@ -2034,6 +2041,9 @@ static irqreturn_t udc_irq(struct ci_hdrc *ci)
>  		if (USBi_PCI & intr) {
>  			ci->gadget.speed = hw_port_is_high_speed(ci) ?
>  				USB_SPEED_HIGH : USB_SPEED_FULL;
> +			if (ci->usb_phy)
> +				usb_phy_set_event(ci->usb_phy,
> +					USB_EVENT_ENUMERATED);
>  			if (ci->suspended) {
>  				if (ci->driver->resume) {
>  					spin_unlock(&ci->lock);
> -- 
> 2.34.1
> 

Re: [PATCH 1/3] usb: chipidea: add USB PHY event

[Greg KH]

On Mon, Sep 02, 2024 at 03:59:12PM +0800, Xu Yang wrote:
> Hi Greg and Sasha,
> 
> On Tue, Jun 27, 2023 at 07:03:51PM +0800, Xu Yang wrote:
> > Add USB PHY event for below situation:
> > - usb role changed
> > - vbus connect
> > - vbus disconnect
> > - gadget driver is enumerated
> > 
> > USB PHY driver can get the last event after above situation occurs
> > and deal with different situations.
> > 
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> These 3 patches had already merged to usb tree. But I didn't add fix tag
> to patch #1 and #3, therefore, only patch #2 went to stable tree. Now
> the stable tree linux-5.15.y and linux-6.1.y have issue due to patch #2
> depends on patch #1. So could you please add patch #1 and patch #3 to
> linux-5.15.y and linux-6.1.y? Or should I post a normal request to achieve
> this? 

Please send a "normal" request as I have no idea what to do here, nor
what the git commit ids are.

thanks,

greg k-h