[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;
> 
> 
> ?