* [PATCH 2/3] usb: phy: mxs: fix getting wrong state with mxs_phy_is_otg_host()
2023-06-27 11:03 [PATCH 1/3] usb: chipidea: add USB PHY event Xu Yang
@ 2023-06-27 11:03 ` Xu Yang
2023-07-10 3:20 ` Peter Chen
2023-07-26 6:08 ` Francesco Dolcini
2023-06-27 11:03 ` [PATCH 3/3] usb: phy: mxs: disconnect line when USB charger is attached Xu Yang
` (4 subsequent siblings)
5 siblings, 2 replies; 14+ messages in thread
From: Xu Yang @ 2023-06-27 11:03 UTC (permalink / raw)
To: peter.chen, gregkh, shawnguo, s.hauer
Cc: kernel, festevam, linux-imx, balbi, linux-usb, jun.li, xu.yang_2
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 2/3] usb: phy: mxs: fix getting wrong state with mxs_phy_is_otg_host()
2023-06-27 11:03 ` [PATCH 2/3] usb: phy: mxs: fix getting wrong state with mxs_phy_is_otg_host() Xu Yang
@ 2023-07-10 3:20 ` Peter Chen
2023-07-26 6:08 ` Francesco Dolcini
1 sibling, 0 replies; 14+ messages in thread
From: Peter Chen @ 2023-07-10 3:20 UTC (permalink / raw)
To: Xu Yang
Cc: gregkh, shawnguo, s.hauer, kernel, festevam, linux-imx, balbi,
linux-usb, jun.li
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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 2/3] usb: phy: mxs: fix getting wrong state with mxs_phy_is_otg_host()
2023-06-27 11:03 ` [PATCH 2/3] usb: phy: mxs: fix getting wrong state with mxs_phy_is_otg_host() Xu Yang
2023-07-10 3:20 ` Peter Chen
@ 2023-07-26 6:08 ` Francesco Dolcini
2023-07-26 6:40 ` [EXT] " Xu Yang
1 sibling, 1 reply; 14+ messages in thread
From: Francesco Dolcini @ 2023-07-26 6:08 UTC (permalink / raw)
To: Xu Yang
Cc: peter.chen, gregkh, shawnguo, s.hauer, kernel, festevam,
linux-imx, balbi, linux-usb, jun.li
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;
?
^ permalink raw reply [flat|nested] 14+ messages in thread* RE: [EXT] Re: [PATCH 2/3] usb: phy: mxs: fix getting wrong state with mxs_phy_is_otg_host()
2023-07-26 6:08 ` Francesco Dolcini
@ 2023-07-26 6:40 ` Xu Yang
0 siblings, 0 replies; 14+ messages in thread
From: Xu Yang @ 2023-07-26 6:40 UTC (permalink / raw)
To: Francesco Dolcini
Cc: peter.chen@kernel.org, gregkh@linuxfoundation.org,
shawnguo@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com, dl-linux-imx,
balbi@ti.com, linux-usb@vger.kernel.org, Jun Li
> 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;
>
>
> ?
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] usb: phy: mxs: disconnect line when USB charger is attached
2023-06-27 11:03 [PATCH 1/3] usb: chipidea: add USB PHY event Xu Yang
2023-06-27 11:03 ` [PATCH 2/3] usb: phy: mxs: fix getting wrong state with mxs_phy_is_otg_host() Xu Yang
@ 2023-06-27 11:03 ` Xu Yang
2023-07-10 3:21 ` Peter Chen
2023-07-10 3:19 ` [PATCH 1/3] usb: chipidea: add USB PHY event Peter Chen
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Xu Yang @ 2023-06-27 11:03 UTC (permalink / raw)
To: peter.chen, gregkh, shawnguo, s.hauer
Cc: kernel, festevam, linux-imx, balbi, linux-usb, jun.li, xu.yang_2
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 3/3] usb: phy: mxs: disconnect line when USB charger is attached
2023-06-27 11:03 ` [PATCH 3/3] usb: phy: mxs: disconnect line when USB charger is attached Xu Yang
@ 2023-07-10 3:21 ` Peter Chen
0 siblings, 0 replies; 14+ messages in thread
From: Peter Chen @ 2023-07-10 3:21 UTC (permalink / raw)
To: Xu Yang
Cc: gregkh, shawnguo, s.hauer, kernel, festevam, linux-imx, balbi,
linux-usb, jun.li
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] usb: chipidea: add USB PHY event
2023-06-27 11:03 [PATCH 1/3] usb: chipidea: add USB PHY event Xu Yang
2023-06-27 11:03 ` [PATCH 2/3] usb: phy: mxs: fix getting wrong state with mxs_phy_is_otg_host() Xu Yang
2023-06-27 11:03 ` [PATCH 3/3] usb: phy: mxs: disconnect line when USB charger is attached Xu Yang
@ 2023-07-10 3:19 ` Peter Chen
2023-07-18 12:27 ` Luca Ceresoli
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Peter Chen @ 2023-07-10 3:19 UTC (permalink / raw)
To: Xu Yang
Cc: gregkh, shawnguo, s.hauer, kernel, festevam, linux-imx, balbi,
linux-usb, jun.li
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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/3] usb: chipidea: add USB PHY event
2023-06-27 11:03 [PATCH 1/3] usb: chipidea: add USB PHY event Xu Yang
` (2 preceding siblings ...)
2023-07-10 3:19 ` [PATCH 1/3] usb: chipidea: add USB PHY event Peter Chen
@ 2023-07-18 12:27 ` Luca Ceresoli
2024-08-27 6:10 ` Pu, Hui
2024-09-02 7:59 ` Xu Yang
5 siblings, 0 replies; 14+ messages in thread
From: Luca Ceresoli @ 2023-07-18 12:27 UTC (permalink / raw)
To: Xu Yang
Cc: peter.chen, gregkh, shawnguo, s.hauer, kernel, festevam,
linux-imx, balbi, linux-usb, jun.li, Francesco Dolcini
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
^ permalink raw reply [flat|nested] 14+ messages in thread* RE: [PATCH 1/3] usb: chipidea: add USB PHY event
2023-06-27 11:03 [PATCH 1/3] usb: chipidea: add USB PHY event Xu Yang
` (3 preceding siblings ...)
2023-07-18 12:27 ` Luca Ceresoli
@ 2024-08-27 6:10 ` Pu, Hui
2024-08-29 9:17 ` Xu Yang
2024-09-02 7:59 ` Xu Yang
5 siblings, 1 reply; 14+ messages in thread
From: Pu, Hui @ 2024-08-27 6:10 UTC (permalink / raw)
To: Xu Yang, peter.chen@kernel.org, gregkh@linuxfoundation.org,
shawnguo@kernel.org, s.hauer@pengutronix.de
Cc: kernel@pengutronix.de, festevam@gmail.com, linux-imx@nxp.com,
balbi@ti.com, linux-usb@vger.kernel.org, jun.li@nxp.com, Ray, Ian
> 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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/3] usb: chipidea: add USB PHY event
2024-08-27 6:10 ` Pu, Hui
@ 2024-08-29 9:17 ` Xu Yang
2024-08-29 9:59 ` Pu, Hui
0 siblings, 1 reply; 14+ messages in thread
From: Xu Yang @ 2024-08-29 9:17 UTC (permalink / raw)
To: Pu, Hui
Cc: peter.chen@kernel.org, gregkh@linuxfoundation.org,
shawnguo@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com, linux-imx@nxp.com,
balbi@ti.com, linux-usb@vger.kernel.org, jun.li@nxp.com, Ray, Ian
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
^ permalink raw reply [flat|nested] 14+ messages in thread* RE: [PATCH 1/3] usb: chipidea: add USB PHY event
2024-08-29 9:17 ` Xu Yang
@ 2024-08-29 9:59 ` Pu, Hui
0 siblings, 0 replies; 14+ messages in thread
From: Pu, Hui @ 2024-08-29 9:59 UTC (permalink / raw)
To: Xu Yang
Cc: peter.chen@kernel.org, gregkh@linuxfoundation.org,
shawnguo@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com, linux-imx@nxp.com,
balbi@ti.com, linux-usb@vger.kernel.org, jun.li@nxp.com, Ray, Ian
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] usb: chipidea: add USB PHY event
2023-06-27 11:03 [PATCH 1/3] usb: chipidea: add USB PHY event Xu Yang
` (4 preceding siblings ...)
2024-08-27 6:10 ` Pu, Hui
@ 2024-09-02 7:59 ` Xu Yang
2024-09-02 8:49 ` Greg KH
5 siblings, 1 reply; 14+ messages in thread
From: Xu Yang @ 2024-09-02 7:59 UTC (permalink / raw)
To: gregkh, sashal
Cc: peter.chen, shawnguo, s.hauer, kernel, festevam, linux-imx, balbi,
linux-usb, jun.li
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
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/3] usb: chipidea: add USB PHY event
2024-09-02 7:59 ` Xu Yang
@ 2024-09-02 8:49 ` Greg KH
0 siblings, 0 replies; 14+ messages in thread
From: Greg KH @ 2024-09-02 8:49 UTC (permalink / raw)
To: Xu Yang
Cc: sashal, peter.chen, shawnguo, s.hauer, kernel, festevam,
linux-imx, balbi, linux-usb, jun.li
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
^ permalink raw reply [flat|nested] 14+ messages in thread