| Message ID | 20200714151822.250783-2-philippe.schenker@toradex.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
> The Toradex Colibri iMX6ULL board has a special USB hardware design. > With runtime-pm enabled USB reset itself continuously. Furthermore the OTG port > is also not enumerating devices if the Chipidea IP is in runtime sleep mode and a > device or host gets plugged in. > Hi Philippe, You may describe the detail what's the special USB hardware design for your board, and why it causes the problem, and why disable runtime pm could fix this issue, then, the other users could know if it could apply to their platforms or not in future. Peter > This patch adds the opportunity to disable Runtime Power Management from > devicetree > > Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com> > > --- > > Changes in v2: > - Change commit message to tell the use case for Colibri iMX6ULL > > drivers/usb/chipidea/ci_hdrc_imx.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c > index 5ae16368a0c7..5078d0695eb7 100644 > --- a/drivers/usb/chipidea/ci_hdrc_imx.c > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c > @@ -434,6 +434,9 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev) > usb_phy_init(pdata.usb_phy); > } > > + if (of_property_read_bool(np, "disable-runtime-pm")) > + pdata.flags &= ~CI_HDRC_SUPPORTS_RUNTIME_PM; > + > if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM) > data->supports_runtime_pm = true; > > -- > 2.27.0
On Wed, 2020-07-15 at 00:51 +0000, Peter Chen wrote: > > > The Toradex Colibri iMX6ULL board has a special USB hardware design. > > With runtime-pm enabled USB reset itself continuously. Furthermore > > the OTG port > > is also not enumerating devices if the Chipidea IP is in runtime > > sleep mode and a > > device or host gets plugged in. > > > > Hi Philippe, > > You may describe the detail what's the special USB hardware design for > your board, If I only knew the root-cause of that problem - unfortunately I don't. That's also why I have such a hard time to describe it. > and why it causes the problem, and why disable runtime pm could fix > this issue, then, I cannot provide the 'why' part yet. I'll try something more and hope I can provide you guys with the exact description. > the other users could know if it could apply to their platforms or not > in future. I only found out about it because you were pointing me in that direction. I debugged for hours now and didn't came to the root-cause of the issue. I think to really understand it I would need to know much more about the Chipidea IP. I'll get back to you guys with a proposal for a new description. Philippe > > Peter > > > This patch adds the opportunity to disable Runtime Power Management > > from > > devicetree > > > > Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com> > > > > --- > > > > Changes in v2: > > - Change commit message to tell the use case for Colibri iMX6ULL > > > > drivers/usb/chipidea/ci_hdrc_imx.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c > > b/drivers/usb/chipidea/ci_hdrc_imx.c > > index 5ae16368a0c7..5078d0695eb7 100644 > > --- a/drivers/usb/chipidea/ci_hdrc_imx.c > > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c > > @@ -434,6 +434,9 @@ static int ci_hdrc_imx_probe(struct > > platform_device *pdev) > > usb_phy_init(pdata.usb_phy); > > } > > > > + if (of_property_read_bool(np, "disable-runtime-pm")) > > + pdata.flags &= ~CI_HDRC_SUPPORTS_RUNTIME_PM; > > + > > if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM) > > data->supports_runtime_pm = true; > > > > -- > > 2.27.0
> On Wed, 2020-07-15 at 00:51 +0000, Peter Chen wrote: > > > > > The Toradex Colibri iMX6ULL board has a special USB hardware design. > > > With runtime-pm enabled USB reset itself continuously. Furthermore > > > the OTG port is also not enumerating devices if the Chipidea IP is > > > in runtime sleep mode and a device or host gets plugged in. > > > > > > > Hi Philippe, > > > > You may describe the detail what's the special USB hardware design for > > your board, > > If I only knew the root-cause of that problem - unfortunately I don't. > That's also why I have such a hard time to describe it. > > > and why it causes the problem, and why disable runtime pm could fix > > this issue, then, > > I cannot provide the 'why' part yet. I'll try something more and hope I can provide > you guys with the exact description. > > > the other users could know if it could apply to their platforms or not > > in future. > > I only found out about it because you were pointing me in that direction. I debugged > for hours now and didn't came to the root-cause of the issue. I think to really > understand it I would need to know much more about the Chipidea IP. > > I'll get back to you guys with a proposal for a new description. > Philippe, is it possible to share your USB hardware design at 6ULL? And how ci_hdrc_gadget_connect is called when the runtime pm is disabled? Thanks, Peter
On Mon, 2020-07-20 at 03:44 +0000, Peter Chen wrote: > > > On Wed, 2020-07-15 at 00:51 +0000, Peter Chen wrote: > > > > The Toradex Colibri iMX6ULL board has a special USB hardware > > > > design. > > > > With runtime-pm enabled USB reset itself continuously. > > > > Furthermore > > > > the OTG port is also not enumerating devices if the Chipidea IP > > > > is > > > > in runtime sleep mode and a device or host gets plugged in. > > > > > > > > > > Hi Philippe, > > > > > > You may describe the detail what's the special USB hardware design > > > for > > > your board, > > > > If I only knew the root-cause of that problem - unfortunately I > > don't. > > That's also why I have such a hard time to describe it. > > > > > and why it causes the problem, and why disable runtime pm could > > > fix > > > this issue, then, > > > > I cannot provide the 'why' part yet. I'll try something more and > > hope I can provide > > you guys with the exact description. > > > > > the other users could know if it could apply to their platforms or > > > not > > > in future. > > > > I only found out about it because you were pointing me in that > > direction. I debugged > > for hours now and didn't came to the root-cause of the issue. I > > think to really > > understand it I would need to know much more about the Chipidea IP. > > > > I'll get back to you guys with a proposal for a new description. > > > > Philippe, is it possible to share your USB hardware design at 6ULL? It's actually pretty simple: We have on USB_OTG1_VBUS a 1uF capacitor and +3.0V on VDD_USB_CAP together with 100n and 10u bypass caps. Now the big problem is that the driver can not detect the 5V on VBUS signal. I tried to 'inject' 5V to that pin last week and things got really better with runtime-pm. But I still thinks disabling it for our board would make sense. I'll send a new description today where I try to point to VBUS signal not connected. Philippe > And how ci_hdrc_gadget_connect is called when the runtime pm is > disabled? > > Thanks, > Peter >
> On Mon, 2020-07-20 at 03:44 +0000, Peter Chen wrote: > > > > > On Wed, 2020-07-15 at 00:51 +0000, Peter Chen wrote: > > > > > The Toradex Colibri iMX6ULL board has a special USB hardware > > > > > design. > > > > > With runtime-pm enabled USB reset itself continuously. > > > > > Furthermore > > > > > the OTG port is also not enumerating devices if the Chipidea IP > > > > > is in runtime sleep mode and a device or host gets plugged in. > > > > > > > > > > > > > Hi Philippe, > > > > > > > > You may describe the detail what's the special USB hardware design > > > > for your board, > > > > > > If I only knew the root-cause of that problem - unfortunately I > > > don't. > > > That's also why I have such a hard time to describe it. > > > > > > > and why it causes the problem, and why disable runtime pm could > > > > fix this issue, then, > > > > > > I cannot provide the 'why' part yet. I'll try something more and > > > hope I can provide you guys with the exact description. > > > > > > > the other users could know if it could apply to their platforms or > > > > not in future. > > > > > > I only found out about it because you were pointing me in that > > > direction. I debugged for hours now and didn't came to the > > > root-cause of the issue. I think to really understand it I would > > > need to know much more about the Chipidea IP. > > > > > > I'll get back to you guys with a proposal for a new description. > > > > > > > Philippe, is it possible to share your USB hardware design at 6ULL? > > It's actually pretty simple: We have on USB_OTG1_VBUS a 1uF capacitor and > +3.0V on VDD_USB_CAP together with 100n and 10u bypass caps. Now the big > problem is that the driver can not detect the 5V on VBUS signal. > Could you confirm it does not see VBUS at register OTGSC? If it is, how can it work with runtime disabled, the USBCMD.RS setting (ci_hdrc_gadget_connect is called) depends on VBUS. Peter > I tried to 'inject' 5V to that pin last week and things got really better with runtime-pm. > But I still thinks disabling it for our board would make sense. > > I'll send a new description today where I try to point to VBUS signal not connected. > > Philippe > > > And how ci_hdrc_gadget_connect is called when the runtime pm is > > disabled? > > > > Thanks, > > Peter > >
On Mon, 2020-07-20 at 08:06 +0000, Peter Chen wrote: > > > On Mon, 2020-07-20 at 03:44 +0000, Peter Chen wrote: > > > > On Wed, 2020-07-15 at 00:51 +0000, Peter Chen wrote: > > > > > > The Toradex Colibri iMX6ULL board has a special USB hardware > > > > > > design. > > > > > > With runtime-pm enabled USB reset itself continuously. > > > > > > Furthermore > > > > > > the OTG port is also not enumerating devices if the Chipidea > > > > > > IP > > > > > > is in runtime sleep mode and a device or host gets plugged > > > > > > in. > > > > > > > > > > > > > > > > Hi Philippe, > > > > > > > > > > You may describe the detail what's the special USB hardware > > > > > design > > > > > for your board, > > > > > > > > If I only knew the root-cause of that problem - unfortunately I > > > > don't. > > > > That's also why I have such a hard time to describe it. > > > > > > > > > and why it causes the problem, and why disable runtime pm > > > > > could > > > > > fix this issue, then, > > > > > > > > I cannot provide the 'why' part yet. I'll try something more and > > > > hope I can provide you guys with the exact description. > > > > > > > > > the other users could know if it could apply to their > > > > > platforms or > > > > > not in future. > > > > > > > > I only found out about it because you were pointing me in that > > > > direction. I debugged for hours now and didn't came to the > > > > root-cause of the issue. I think to really understand it I would > > > > need to know much more about the Chipidea IP. > > > > > > > > I'll get back to you guys with a proposal for a new description. > > > > > > > > > > Philippe, is it possible to share your USB hardware design at > > > 6ULL? > > > > It's actually pretty simple: We have on USB_OTG1_VBUS a 1uF > > capacitor and > > +3.0V on VDD_USB_CAP together with 100n and 10u bypass caps. Now the > > big > > problem is that the driver can not detect the 5V on VBUS signal. > > > > Could you confirm it does not see VBUS at register OTGSC? If it is, > how can it work with runtime > disabled, the USBCMD.RS setting (ci_hdrc_gadget_connect is called) > depends on VBUS. > > Peter For this reason I'm using a workaround in extcon like this: extcon = <&extcon_usbc_det>, <&extcon_usbc_det>; I know that this is undocumented and wrong, but it works for our hardware. With this and enabled runtime-pm devices do not get enumerated. But with runtime-pm disabled, devices get enumerated. Further with this workaround the VBUS signal gets 'simulated' in hw_read_otgsc. Another problem with runtime-pm enabled is that with no devices plugged into USB it resets itself every ~1 second. Philippe. > > > I tried to 'inject' 5V to that pin last week and things got really > > better with runtime-pm. > > But I still thinks disabling it for our board would make sense. > > > > I'll send a new description today where I try to point to VBUS > > signal not connected. > > > > Philippe > > > > > And how ci_hdrc_gadget_connect is called when the runtime pm is > > > disabled? > > > > > > Thanks, > > > Peter > > >
Hello Philippe,
I found this thread after several hours spent in debugging why USB host is
not detecting new devices on a custom board using the iMX6ULL Colibri
SoM.
My best workaround at the moment is:
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -56,7 +58,7 @@ static const struct ci_hdrc_imx_platform_flag imx6sx_usb_data = {
};
static const struct ci_hdrc_imx_platform_flag imx6ul_usb_data = {
- .flags = CI_HDRC_SUPPORTS_RUNTIME_PM |
+ .flags = //CI_HDRC_SUPPORTS_RUNTIME_PM |
CI_HDRC_TURN_VBUS_EARLY_ON |
CI_HDRC_DISABLE_DEVICE_STREAMING,
};
so it's equivalent to yours, but hard-coded.
I haven't found any follow-up patches from you, so I'm wondering whether
you have made any progress on this issue.
As I see it, a quirk in the driver would make sense. What needs to be
sorted out is how to enable it from device tree.
I think a DT boolean as you propose would do, but with a name describing
the hardware aspects of it ("chipidea,vbus-detection-is-broken"?) and not
the driver behaviour.
Otherwise looking for the "toradex,colibri-imx6ul" prefix in the top-level
/compatible strings seems doable. I don't know how acceptable this is from
a driver maintainer's point of view though.
Best regards,
Luca
Hello Luca, I guess your mail to Philippe bounced, let me try to answer since I am aware of the issue here. On Thu, May 04, 2023 at 06:23:12PM +0200, Luca Ceresoli wrote: > I found this thread after several hours spent in debugging why USB host is > not detecting new devices on a custom board using the iMX6ULL Colibri > SoM. > > My best workaround at the moment is: We have the same workaround in our BSP since quite some time, see https://git.toradex.com/cgit/meta-toradex-bsp-common.git/tree/recipes-kernel/linux/linux-toradex-mainline-git/0002-drivers-chipidea-disable-runtime-pm-for-imx6ul.patch > I haven't found any follow-up patches from you, so I'm wondering whether > you have made any progress on this issue. You can find the latest discussion on that regard here https://lore.kernel.org/all/Y1vLpaxpc5WBCuGD@francesco-nb.int.toradex.com/ > As I see it, a quirk in the driver would make sense. I am not sure. The reason this is not working is that the VBUS is not directly connected to the SOC and the USB IP is powered in a different way (all of that was reviewed/acked by NXP when the board was designed). The issue is in drivers/usb/phy/phy-mxs-usb.c:mxs_phy_disconnect_line(), however: 1 - checking for the VBUS when in host mode is not correct. 2 - checking for the VBUS when in OTG/dual-role mode should be done using extcon or equivalent. In this case on the colibri-imx6ull you would get the correct behavior. The NXP downstream kernel has some improvements on that regard, for instance 1 would work, 2 I am not 100% sure. Here the related patches: From 60422d8301fc354e69ec0184831468c3b65cc26a Mon Sep 17 00:00:00 2001 From: Li Jun <jun.li@nxp.com> Date: Wed, 12 Apr 2017 05:31:17 +0800 Subject: [PATCH 1/3] MLK-14285-1 usb: phy: add usb mode for usb_phy USB phy driver may need to know the current working mode of the controller, and does some different settings according to host mode or device mode. Signed-off-by: Li Jun <jun.li@nxp.com> (cherry picked from commit 2286cb30feedd6f4a5cb82a0f0af5aa3a04ab698) Signed-off-by: Vipul Kumar <vipul_kumar@mentor.com> Signed-off-by: Srikanth Krishnakar <Srikanth_Krishnakar@mentor.com> --- include/linux/usb/phy.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h index e4de6bc1f69b..93963a018a13 100644 --- a/include/linux/usb/phy.h +++ b/include/linux/usb/phy.h @@ -63,6 +63,13 @@ enum usb_otg_state { OTG_STATE_A_VBUS_ERR, }; +/* The usb role of phy to be working with */ +enum usb_current_mode { + CUR_USB_MODE_NONE, + CUR_USB_MODE_HOST, + CUR_USB_MODE_DEVICE, +}; + struct usb_phy; struct usb_otg; @@ -155,6 +162,10 @@ struct usb_phy { * manually detect the charger type. */ enum usb_charger_type (*charger_detect)(struct usb_phy *x); + + int (*set_mode)(struct usb_phy *x, + enum usb_current_mode mode); + }; /* for board-specific init logic */ @@ -213,6 +224,15 @@ usb_phy_vbus_off(struct usb_phy *x) return x->set_vbus(x, false); } +static inline int +usb_phy_set_mode(struct usb_phy *x, enum usb_current_mode mode) +{ + if (!x || !x->set_mode) + return 0; + + return x->set_mode(x, mode); +} + /* for usb host and peripheral controller drivers */ #if IS_ENABLED(CONFIG_USB_PHY) extern struct usb_phy *usb_get_phy(enum usb_phy_type type); From cf3f741f4b7ab5d139938c4c0cd65d4547d386ff Mon Sep 17 00:00:00 2001 From: Li Jun <jun.li@nxp.com> Date: Wed, 12 Apr 2017 05:41:21 +0800 Subject: [PATCH 2/3] MLK-14285-3 usb: phy: mxs: optimize disconnect line condition We only have below cases to disconnect line when suspend: 1. Device mode without connection to any host/charger(no vbus). 2. Device mode connect to a charger(w/ vbus), usb suspend when system is entering suspend. This patch can fix usb phy wrongly does disconnect line in case some usb host enters suspend but vbus is off. Signed-off-by: Li Jun <jun.li@nxp.com> (cherry picked from commit 2af48913f77cec3658f5863b13f63619d8101279) --- drivers/usb/phy/phy-mxs-usb.c | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c index 8a262c5a0408..31f40dbb71c0 100644 --- a/drivers/usb/phy/phy-mxs-usb.c +++ b/drivers/usb/phy/phy-mxs-usb.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0+ /* - * Copyright 2012-2014 Freescale Semiconductor, Inc. + * Copyright 2012-2016 Freescale Semiconductor, Inc. + * Copyright 2017 NXP * Copyright (C) 2012 Marek Vasut <marex@denx.de> * on behalf of DENX Software Engineering GmbH */ @@ -204,6 +205,7 @@ struct mxs_phy { int port_id; u32 tx_reg_set; u32 tx_reg_mask; + enum usb_current_mode mode; }; static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy) @@ -386,18 +388,6 @@ static void __mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool disconnect) usleep_range(500, 1000); } -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; -} - static void mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool on) { bool vbus_is_on = false; @@ -412,7 +402,7 @@ 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->mode != CUR_USB_MODE_HOST))) __mxs_phy_disconnect_line(mxs_phy, true); else __mxs_phy_disconnect_line(mxs_phy, false); @@ -708,6 +698,19 @@ static enum usb_charger_type mxs_phy_charger_detect(struct usb_phy *phy) return chgr_type; } +/* + * Set the usb current role for phy. + */ +static int mxs_phy_set_mode(struct usb_phy *phy, + enum usb_current_mode mode) +{ + struct mxs_phy *mxs_phy = to_mxs_phy(phy); + + mxs_phy->mode = mode; + + return 0; +} + static int mxs_phy_probe(struct platform_device *pdev) { void __iomem *base; @@ -793,6 +796,7 @@ static int mxs_phy_probe(struct platform_device *pdev) mxs_phy->clk = clk; mxs_phy->data = of_device_get_match_data(&pdev->dev); + mxs_phy->phy.set_mode = mxs_phy_set_mode; platform_set_drvdata(pdev, mxs_phy); From d4c4385b997f32b081e859d491f25f5beb738ae9 Mon Sep 17 00:00:00 2001 From: Li Jun <jun.li@nxp.com> Date: Wed, 12 Apr 2017 05:38:48 +0800 Subject: [PATCH 3/3] MLK-14285-2 usb: chipidea: set mode for usb phy driver After enters one specific role, notify usb phy driver. Signed-off-by: Li Jun <jun.li@nxp.com> (cherry picked from commit d3aa2a13f4e47bc7fae7f2eee1e86291d7513312) --- drivers/usb/chipidea/ci.h | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h index 99440baa6458..9d7ba902246d 100644 --- a/drivers/usb/chipidea/ci.h +++ b/drivers/usb/chipidea/ci.h @@ -274,9 +274,21 @@ 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; - return ret; + if (ret) + return ret; + + ci->role = role; + + if (ci->usb_phy) { + if (role == CI_ROLE_HOST) + usb_phy_set_mode(ci->usb_phy, + CUR_USB_MODE_HOST); + else + usb_phy_set_mode(ci->usb_phy, + CUR_USB_MODE_DEVICE); + } + + return 0; } static inline void ci_role_stop(struct ci_hdrc *ci) @@ -289,6 +301,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_mode(ci->usb_phy, CUR_USB_MODE_NONE); } static inline enum usb_role ci_role_to_usb_role(struct ci_hdrc *ci) Francesco
Hello Francesco, On Thu, 4 May 2023 18:50:14 +0200 Francesco Dolcini <francesco@dolcini.it> wrote: > Hello Luca, > I guess your mail to Philippe bounced, let me try to answer since I am aware > of the issue here. > > On Thu, May 04, 2023 at 06:23:12PM +0200, Luca Ceresoli wrote: > > I found this thread after several hours spent in debugging why USB host is > > not detecting new devices on a custom board using the iMX6ULL Colibri > > SoM. > > > > My best workaround at the moment is: > We have the same workaround in our BSP since quite some time, see > https://git.toradex.com/cgit/meta-toradex-bsp-common.git/tree/recipes-kernel/linux/linux-toradex-mainline-git/0002-drivers-chipidea-disable-runtime-pm-for-imx6ul.patch > > > I haven't found any follow-up patches from you, so I'm wondering whether > > you have made any progress on this issue. > You can find the latest discussion on that regard here > https://lore.kernel.org/all/Y1vLpaxpc5WBCuGD@francesco-nb.int.toradex.com/ Thanks for this pointer! I have read the discussion and it was a bit confusing, especially about whether the hardware can work at all. Are you planning to continue on that work? I would be very glad to test on product based on the i.MX6ULL Colibri module I am currently working on. Best regards, Luca
> -----Original Message----- > From: Francesco Dolcini <francesco@dolcini.it> > Sent: Friday, May 5, 2023 12:50 AM > To: Luca Ceresoli <luca.ceresoli@bootlin.com> > Cc: devicetree@vger.kernel.org; festevam@gmail.com; > gregkh@linuxfoundation.org; Jun Li <jun.li@nxp.com>; > kernel@pengutronix.de; linux-arm-kernel@lists.infradead.org; > dl-linux-imx <linux-imx@nxp.com>; linux-kernel@vger.kernel.org; > linux-usb@vger.kernel.org; peter.chen@nxp.com; robh+dt@kernel.org; > s.hauer@pengutronix.de; shawnguo@kernel.org; Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org>; Francesco Dolcini > <francesco.dolcini@toradex.com> > Subject: Re: [PATCH v2 2/3] usb: chipidea: imx: support disabling runtime-pm > > Hello Luca, > I guess your mail to Philippe bounced, let me try to answer since I am aware > of the issue here. > > On Thu, May 04, 2023 at 06:23:12PM +0200, Luca Ceresoli wrote: > > I found this thread after several hours spent in debugging why USB host > is > > not detecting new devices on a custom board using the iMX6ULL Colibri > > SoM. > > > > My best workaround at the moment is: > We have the same workaround in our BSP since quite some time, see > https://git.t/ > oradex.com%2Fcgit%2Fmeta-toradex-bsp-common.git%2Ftree%2Frecipes-kernel > %2Flinux%2Flinux-toradex-mainline-git%2F0002-drivers-chipidea-disable-r > untime-pm-for-imx6ul.patch&data=05%7C01%7Cjun.li%40nxp.com%7C776dc1e71a > 554ee20ed908db4cbfa5aa%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638 > 188158228795706%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l > uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3cFsUurgKBfic > LNfRxMOErukGiMczhJREFGvaFYTH34%3D&reserved=0 > > > I haven't found any follow-up patches from you, so I'm wondering whether > > you have made any progress on this issue. > You can find the latest discussion on that regard here > https://lore/. > kernel.org%2Fall%2FY1vLpaxpc5WBCuGD%40francesco-nb.int.toradex.com%2F&d > ata=05%7C01%7Cjun.li%40nxp.com%7C776dc1e71a554ee20ed908db4cbfa5aa%7C686 > ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638188158228795706%7CUnknown%7C > TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI > 6Mn0%3D%7C3000%7C%7C%7C&sdata=xgIAlY5Az9zQgOBaUTHeGJDXNIyNRmEBlcR49JOVB > sI%3D&reserved=0 > > > As I see it, a quirk in the driver would make sense. > I am not sure. > > The reason this is not working is that the VBUS is not directly > connected to the SOC and the USB IP is powered in a different way (all > of that was reviewed/acked by NXP when the board was designed). Hi Luca, Is your board design similar like Francesco's as below? "VDD_USB_CAP in our design is connected to a 3.0V external LDO, voltage on both VBUS pads is 0V" I am going to fix this by update the PHY power supply check to cover Francesco's case(PHY is powered by VDD_USB_CAP). Thanks Li Jun > > The issue is in drivers/usb/phy/phy-mxs-usb.c:mxs_phy_disconnect_line(), > however: > 1 - checking for the VBUS when in host mode is not correct. > 2 - checking for the VBUS when in OTG/dual-role mode should be done using > extcon or equivalent. In this case on the colibri-imx6ull you would get > the correct behavior. > > The NXP downstream kernel has some improvements on that regard, for > instance 1 would work, 2 I am not 100% sure. > > Here the related patches: > > From 60422d8301fc354e69ec0184831468c3b65cc26a Mon Sep 17 00:00:00 2001 > From: Li Jun <jun.li@nxp.com> > Date: Wed, 12 Apr 2017 05:31:17 +0800 > Subject: [PATCH 1/3] MLK-14285-1 usb: phy: add usb mode for usb_phy > > USB phy driver may need to know the current working mode of > the controller, and does some different settings according to > host mode or device mode. > > Signed-off-by: Li Jun <jun.li@nxp.com> > (cherry picked from commit 2286cb30feedd6f4a5cb82a0f0af5aa3a04ab698) > Signed-off-by: Vipul Kumar <vipul_kumar@mentor.com> > Signed-off-by: Srikanth Krishnakar <Srikanth_Krishnakar@mentor.com> > --- > include/linux/usb/phy.h | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h > index e4de6bc1f69b..93963a018a13 100644 > --- a/include/linux/usb/phy.h > +++ b/include/linux/usb/phy.h > @@ -63,6 +63,13 @@ enum usb_otg_state { > OTG_STATE_A_VBUS_ERR, > }; > > +/* The usb role of phy to be working with */ > +enum usb_current_mode { > + CUR_USB_MODE_NONE, > + CUR_USB_MODE_HOST, > + CUR_USB_MODE_DEVICE, > +}; > + > struct usb_phy; > struct usb_otg; > > @@ -155,6 +162,10 @@ struct usb_phy { > * manually detect the charger type. > */ > enum usb_charger_type (*charger_detect)(struct usb_phy *x); > + > + int (*set_mode)(struct usb_phy *x, > + enum usb_current_mode mode); > + > }; > > /* for board-specific init logic */ > @@ -213,6 +224,15 @@ usb_phy_vbus_off(struct usb_phy *x) > return x->set_vbus(x, false); > } > > +static inline int > +usb_phy_set_mode(struct usb_phy *x, enum usb_current_mode mode) > +{ > + if (!x || !x->set_mode) > + return 0; > + > + return x->set_mode(x, mode); > +} > + > /* for usb host and peripheral controller drivers */ > #if IS_ENABLED(CONFIG_USB_PHY) > extern struct usb_phy *usb_get_phy(enum usb_phy_type type); > > > From cf3f741f4b7ab5d139938c4c0cd65d4547d386ff Mon Sep 17 00:00:00 2001 > From: Li Jun <jun.li@nxp.com> > Date: Wed, 12 Apr 2017 05:41:21 +0800 > Subject: [PATCH 2/3] MLK-14285-3 usb: phy: mxs: optimize disconnect line > condition > > We only have below cases to disconnect line when suspend: > 1. Device mode without connection to any host/charger(no vbus). > 2. Device mode connect to a charger(w/ vbus), usb suspend when > system is entering suspend. > This patch can fix usb phy wrongly does disconnect line in case > some usb host enters suspend but vbus is off. > > Signed-off-by: Li Jun <jun.li@nxp.com> > (cherry picked from commit 2af48913f77cec3658f5863b13f63619d8101279) > --- > drivers/usb/phy/phy-mxs-usb.c | 32 ++++++++++++++++++-------------- > 1 file changed, 18 insertions(+), 14 deletions(-) > > diff --git a/drivers/usb/phy/phy-mxs-usb.c > b/drivers/usb/phy/phy-mxs-usb.c > index 8a262c5a0408..31f40dbb71c0 100644 > --- a/drivers/usb/phy/phy-mxs-usb.c > +++ b/drivers/usb/phy/phy-mxs-usb.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0+ > /* > - * Copyright 2012-2014 Freescale Semiconductor, Inc. > + * Copyright 2012-2016 Freescale Semiconductor, Inc. > + * Copyright 2017 NXP > * Copyright (C) 2012 Marek Vasut <marex@denx.de> > * on behalf of DENX Software Engineering GmbH > */ > @@ -204,6 +205,7 @@ struct mxs_phy { > int port_id; > u32 tx_reg_set; > u32 tx_reg_mask; > + enum usb_current_mode mode; > }; > > static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy) > @@ -386,18 +388,6 @@ static void __mxs_phy_disconnect_line(struct mxs_phy > *mxs_phy, bool disconnect) > usleep_range(500, 1000); > } > > -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; > -} > - > static void mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool on) > { > bool vbus_is_on = false; > @@ -412,7 +402,7 @@ 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->mode != CUR_USB_MODE_HOST))) > __mxs_phy_disconnect_line(mxs_phy, true); > else > __mxs_phy_disconnect_line(mxs_phy, false); > @@ -708,6 +698,19 @@ static enum usb_charger_type > mxs_phy_charger_detect(struct usb_phy *phy) > return chgr_type; > } > > +/* > + * Set the usb current role for phy. > + */ > +static int mxs_phy_set_mode(struct usb_phy *phy, > + enum usb_current_mode mode) > +{ > + struct mxs_phy *mxs_phy = to_mxs_phy(phy); > + > + mxs_phy->mode = mode; > + > + return 0; > +} > + > static int mxs_phy_probe(struct platform_device *pdev) > { > void __iomem *base; > @@ -793,6 +796,7 @@ static int mxs_phy_probe(struct platform_device *pdev) > > mxs_phy->clk = clk; > mxs_phy->data = of_device_get_match_data(&pdev->dev); > + mxs_phy->phy.set_mode = mxs_phy_set_mode; > > platform_set_drvdata(pdev, mxs_phy); > > > From d4c4385b997f32b081e859d491f25f5beb738ae9 Mon Sep 17 00:00:00 2001 > From: Li Jun <jun.li@nxp.com> > Date: Wed, 12 Apr 2017 05:38:48 +0800 > Subject: [PATCH 3/3] MLK-14285-2 usb: chipidea: set mode for usb phy driver > > After enters one specific role, notify usb phy driver. > > Signed-off-by: Li Jun <jun.li@nxp.com> > (cherry picked from commit d3aa2a13f4e47bc7fae7f2eee1e86291d7513312) > --- > drivers/usb/chipidea/ci.h | 21 ++++++++++++++++++--- > 1 file changed, 18 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h > index 99440baa6458..9d7ba902246d 100644 > --- a/drivers/usb/chipidea/ci.h > +++ b/drivers/usb/chipidea/ci.h > @@ -274,9 +274,21 @@ 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; > - return ret; > + if (ret) > + return ret; > + > + ci->role = role; > + > + if (ci->usb_phy) { > + if (role == CI_ROLE_HOST) > + usb_phy_set_mode(ci->usb_phy, > + CUR_USB_MODE_HOST); > + else > + usb_phy_set_mode(ci->usb_phy, > + CUR_USB_MODE_DEVICE); > + } > + > + return 0; > } > > static inline void ci_role_stop(struct ci_hdrc *ci) > @@ -289,6 +301,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_mode(ci->usb_phy, CUR_USB_MODE_NONE); > } > > static inline enum usb_role ci_role_to_usb_role(struct ci_hdrc *ci) > > > Francesco
Hello Jun, On Fri, 5 May 2023 09:49:16 +0000 Jun Li <jun.li@nxp.com> wrote: > > -----Original Message----- > > From: Francesco Dolcini <francesco@dolcini.it> > > Sent: Friday, May 5, 2023 12:50 AM > > To: Luca Ceresoli <luca.ceresoli@bootlin.com> > > Cc: devicetree@vger.kernel.org; festevam@gmail.com; > > gregkh@linuxfoundation.org; Jun Li <jun.li@nxp.com>; > > kernel@pengutronix.de; linux-arm-kernel@lists.infradead.org; > > dl-linux-imx <linux-imx@nxp.com>; linux-kernel@vger.kernel.org; > > linux-usb@vger.kernel.org; peter.chen@nxp.com; robh+dt@kernel.org; > > s.hauer@pengutronix.de; shawnguo@kernel.org; Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org>; Francesco Dolcini > > <francesco.dolcini@toradex.com> > > Subject: Re: [PATCH v2 2/3] usb: chipidea: imx: support disabling runtime-pm > > > > Hello Luca, > > I guess your mail to Philippe bounced, let me try to answer since I am aware > > of the issue here. > > > > On Thu, May 04, 2023 at 06:23:12PM +0200, Luca Ceresoli wrote: > > > I found this thread after several hours spent in debugging why USB host > > is > > > not detecting new devices on a custom board using the iMX6ULL Colibri > > > SoM. > > > > > > My best workaround at the moment is: > > We have the same workaround in our BSP since quite some time, see > > https://git.t/ > > oradex.com%2Fcgit%2Fmeta-toradex-bsp-common.git%2Ftree%2Frecipes-kernel > > %2Flinux%2Flinux-toradex-mainline-git%2F0002-drivers-chipidea-disable-r > > untime-pm-for-imx6ul.patch&data=05%7C01%7Cjun.li%40nxp.com%7C776dc1e71a > > 554ee20ed908db4cbfa5aa%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638 > > 188158228795706%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l > > uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3cFsUurgKBfic > > LNfRxMOErukGiMczhJREFGvaFYTH34%3D&reserved=0 > > > > > I haven't found any follow-up patches from you, so I'm wondering whether > > > you have made any progress on this issue. > > You can find the latest discussion on that regard here > > https://lore/. > > kernel.org%2Fall%2FY1vLpaxpc5WBCuGD%40francesco-nb.int.toradex.com%2F&d > > ata=05%7C01%7Cjun.li%40nxp.com%7C776dc1e71a554ee20ed908db4cbfa5aa%7C686 > > ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638188158228795706%7CUnknown%7C > > TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI > > 6Mn0%3D%7C3000%7C%7C%7C&sdata=xgIAlY5Az9zQgOBaUTHeGJDXNIyNRmEBlcR49JOVB > > sI%3D&reserved=0 > > > > > As I see it, a quirk in the driver would make sense. > > I am not sure. > > > > The reason this is not working is that the VBUS is not directly > > connected to the SOC and the USB IP is powered in a different way (all > > of that was reviewed/acked by NXP when the board was designed). > > Hi Luca, > > Is your board design similar like Francesco's as below? Possibly, but I'm afraid I can't say: I am using the Toradex Colibri i.MX6ULL SoM, whose schematics are not public. Best regards, Luca
On Fri, May 05, 2023 at 11:23:51AM +0200, Luca Ceresoli wrote: > On Thu, 4 May 2023 18:50:14 +0200 > Francesco Dolcini <francesco@dolcini.it> wrote: > > > Hello Luca, > > I guess your mail to Philippe bounced, let me try to answer since I am aware > > of the issue here. > > > > On Thu, May 04, 2023 at 06:23:12PM +0200, Luca Ceresoli wrote: > > > I found this thread after several hours spent in debugging why USB host is > > > not detecting new devices on a custom board using the iMX6ULL Colibri > > > SoM. > > > > > > My best workaround at the moment is: > > We have the same workaround in our BSP since quite some time, see > > https://git.toradex.com/cgit/meta-toradex-bsp-common.git/tree/recipes-kernel/linux/linux-toradex-mainline-git/0002-drivers-chipidea-disable-runtime-pm-for-imx6ul.patch > > > > > I haven't found any follow-up patches from you, so I'm wondering whether > > > you have made any progress on this issue. > > You can find the latest discussion on that regard here > > https://lore.kernel.org/all/Y1vLpaxpc5WBCuGD@francesco-nb.int.toradex.com/ > > Thanks for this pointer! I have read the discussion and it was a bit > confusing, especially about whether the hardware can work at all. It can work. Hardware wise is just fine, software wise I tried to explain how in my previous answer to you. Worst case we would have to describe the hardware in a different way in the DTS, but I do not expect this to be required. > Are you planning to continue on that work? Hopefully NXP / Li Jun will be able to help. Francesco
On Fri, May 05, 2023 at 12:06:18PM +0200, Luca Ceresoli wrote: > On Fri, 5 May 2023 09:49:16 +0000 > Jun Li <jun.li@nxp.com> wrote: > > Is your board design similar like Francesco's as below? > > Possibly, but I'm afraid I can't say: I am using the Toradex Colibri > i.MX6ULL SoM, whose schematics are not public. I can confirm that it's the same. Francesco
> -----Original Message----- > From: Francesco Dolcini <francesco@dolcini.it> > Sent: Friday, May 5, 2023 7:00 PM > To: Luca Ceresoli <luca.ceresoli@bootlin.com>; Jun Li <jun.li@nxp.com> > Cc: Francesco Dolcini <francesco@dolcini.it>; devicetree@vger.kernel.org; > festevam@gmail.com; gregkh@linuxfoundation.org; kernel@pengutronix.de; > linux-arm-kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>; > linux-kernel@vger.kernel.org; linux-usb@vger.kernel.org; > peter.chen@nxp.com; robh+dt@kernel.org; s.hauer@pengutronix.de; > shawnguo@kernel.org; Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>; > Francesco Dolcini <francesco.dolcini@toradex.com> > Subject: Re: [PATCH v2 2/3] usb: chipidea: imx: support disabling runtime-pm > > On Fri, May 05, 2023 at 12:06:18PM +0200, Luca Ceresoli wrote: > > On Fri, 5 May 2023 09:49:16 +0000 > > Jun Li <jun.li@nxp.com> wrote: > > > Is your board design similar like Francesco's as below? > > > > Possibly, but I'm afraid I can't say: I am using the Toradex Colibri > > i.MX6ULL SoM, whose schematics are not public. > > I can confirm that it's the same. Thanks Francesco for the confirmation, had a check with design team, there is no status bit which can be used to judge the VDD_USB_CAP is powered or not, so we have to add a board level dts property to tell this usb phy driver to bypass MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS. Before send a formal patch, I want to confirm this should work for your HW design, like below simple hack: diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c index e1a2b2ea098b..ec5ee790455e 100644 --- a/drivers/usb/phy/phy-mxs-usb.c +++ b/drivers/usb/phy/phy-mxs-usb.c @@ -178,7 +178,6 @@ static const struct mxs_phy_data imx6sx_phy_data = { }; static const struct mxs_phy_data imx6ul_phy_data = { - .flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS, }; static const struct mxs_phy_data imx7ulp_phy_data = { Thanks Li Jun > > Francesco
Hello Jun, On Mon, 8 May 2023 15:17:56 +0200 Luca Ceresoli <luca.ceresoli@bootlin.com> wrote: > Hello Jun, Francesco, > > On Mon, 8 May 2023 13:17:11 +0200 > Francesco Dolcini <francesco@dolcini.it> wrote: > > > On Sat, May 06, 2023 at 09:02:39AM +0000, Jun Li wrote: > > > > -----Original Message----- > > > > From: Francesco Dolcini <francesco@dolcini.it> > > > > Sent: Friday, May 5, 2023 7:00 PM > > > > To: Luca Ceresoli <luca.ceresoli@bootlin.com>; Jun Li <jun.li@nxp.com> > > > > Cc: Francesco Dolcini <francesco@dolcini.it>; devicetree@vger.kernel.org; > > > > festevam@gmail.com; gregkh@linuxfoundation.org; kernel@pengutronix.de; > > > > linux-arm-kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>; > > > > linux-kernel@vger.kernel.org; linux-usb@vger.kernel.org; > > > > peter.chen@nxp.com; robh+dt@kernel.org; s.hauer@pengutronix.de; > > > > shawnguo@kernel.org; Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>; > > > > Francesco Dolcini <francesco.dolcini@toradex.com> > > > > Subject: Re: [PATCH v2 2/3] usb: chipidea: imx: support disabling runtime-pm > > > > > > > > On Fri, May 05, 2023 at 12:06:18PM +0200, Luca Ceresoli wrote: > > > > > On Fri, 5 May 2023 09:49:16 +0000 > > > > > Jun Li <jun.li@nxp.com> wrote: > > > > > > Is your board design similar like Francesco's as below? > > > > > > > > > > Possibly, but I'm afraid I can't say: I am using the Toradex Colibri > > > > > i.MX6ULL SoM, whose schematics are not public. > > > > > > > > I can confirm that it's the same. > > > > > > Thanks Francesco for the confirmation, had a check with design team, > > > there is no status bit which can be used to judge the VDD_USB_CAP is > > > powered or not, so we have to add a board level dts property to tell > > > this usb phy driver to bypass MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS. > > > > > > Before send a formal patch, I want to confirm this should work for your > > > HW design, like below simple hack: > > > > Thanks Li Jun, I tested it with v6.3.1 kernel and it's all good. > > I would be happy to test the patch as soon as you send it. > > Thanks Jun, it works here as well, on 6.1.27! Have you managed to make progress on the patch after Francesco's and my tests? As I see it, a proper fix for mainline could be as simple as a new DT property to describe this specific hardware configuration and a patch to ignore the flag when the property is present. Is my understanding correct? Best regards, Luca
> -----Original Message----- > From: Luca Ceresoli <luca.ceresoli@bootlin.com> > Sent: Monday, May 29, 2023 6:18 PM > To: Jun Li <jun.li@nxp.com> > Cc: Francesco Dolcini <francesco@dolcini.it>; devicetree@vger.kernel.org; > festevam@gmail.com; gregkh@linuxfoundation.org; kernel@pengutronix.de; > linux-arm-kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>; > linux-kernel@vger.kernel.org; linux-usb@vger.kernel.org; > robh+dt@kernel.org; s.hauer@pengutronix.de; shawnguo@kernel.org; > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>; Francesco Dolcini > <francesco.dolcini@toradex.com>; Xu Yang <xu.yang_2@nxp.com> > Subject: Re: [PATCH v2 2/3] usb: chipidea: imx: support disabling runtime-pm > > Hello Jun, > > On Mon, 8 May 2023 15:17:56 +0200 > Luca Ceresoli <luca.ceresoli@bootlin.com> wrote: > > > Hello Jun, Francesco, > > > > On Mon, 8 May 2023 13:17:11 +0200 > > Francesco Dolcini <francesco@dolcini.it> wrote: > > > > > On Sat, May 06, 2023 at 09:02:39AM +0000, Jun Li wrote: > > > > > -----Original Message----- > > > > > From: Francesco Dolcini <francesco@dolcini.it> > > > > > Sent: Friday, May 5, 2023 7:00 PM > > > > > To: Luca Ceresoli <luca.ceresoli@bootlin.com>; Jun Li > <jun.li@nxp.com> > > > > > Cc: Francesco Dolcini <francesco@dolcini.it>; > devicetree@vger.kernel.org; > > > > > festevam@gmail.com; gregkh@linuxfoundation.org; > kernel@pengutronix.de; > > > > > linux-arm-kernel@lists.infradead.org; dl-linux-imx > <linux-imx@nxp.com>; > > > > > linux-kernel@vger.kernel.org; linux-usb@vger.kernel.org; > > > > > peter.chen@nxp.com; robh+dt@kernel.org; s.hauer@pengutronix.de; > > > > > shawnguo@kernel.org; Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org>; > > > > > Francesco Dolcini <francesco.dolcini@toradex.com> > > > > > Subject: Re: [PATCH v2 2/3] usb: chipidea: imx: support disabling > runtime-pm > > > > > > > > > > On Fri, May 05, 2023 at 12:06:18PM +0200, Luca Ceresoli wrote: > > > > > > On Fri, 5 May 2023 09:49:16 +0000 > > > > > > Jun Li <jun.li@nxp.com> wrote: > > > > > > > Is your board design similar like Francesco's as below? > > > > > > > > > > > > Possibly, but I'm afraid I can't say: I am using the Toradex Colibri > > > > > > i.MX6ULL SoM, whose schematics are not public. > > > > > > > > > > I can confirm that it's the same. > > > > > > > > Thanks Francesco for the confirmation, had a check with design team, > > > > there is no status bit which can be used to judge the VDD_USB_CAP is > > > > powered or not, so we have to add a board level dts property to tell > > > > this usb phy driver to bypass MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS. > > > > > > > > Before send a formal patch, I want to confirm this should work for > your > > > > HW design, like below simple hack: > > > > > > Thanks Li Jun, I tested it with v6.3.1 kernel and it's all good. > > > I would be happy to test the patch as soon as you send it. > > > > Thanks Jun, it works here as well, on 6.1.27! > > Have you managed to make progress on the patch after Francesco's and my > tests? > > As I see it, a proper fix for mainline could be as simple as a new DT > property to describe this specific hardware configuration and a patch > to ignore the flag when the property is present. Is my understanding > correct? Yes, your understanding is correct, talked with Xu(in CC), he will take this soon. Thanks Li Jun > > Best regards, > Luca > > -- > Luca Ceresoli, Bootlin > Embedded Linux and Kernel engineering > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootl > in.com%2F&data=05%7C01%7Cjun.li%40nxp.com%7C54d7d4bc194545db610608db602 > e0ca6%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638209523121178887%7 > CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h > aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3SbXTUvvRwcsJoBIhPoypWDLDMpj2m > z09JC34t3rNh4%3D&reserved=0
Hello Luca, On Tue, May 30, 2023 at 11:22:51AM +0000, Jun Li wrote: > Yes, your understanding is correct, talked with Xu(in CC), he will take this > soon. A series was posted https://lore.kernel.org/all/20230627110353.1879477-1-xu.yang_2@nxp.com/, I had no time to try or look at it yet. Francesco
Ciao Francesco, On Thu, 6 Jul 2023 12:23:43 +0200 Francesco Dolcini <francesco@dolcini.it> wrote: > Hello Luca, > > On Tue, May 30, 2023 at 11:22:51AM +0000, Jun Li wrote: > > Yes, your understanding is correct, talked with Xu(in CC), he will take this > > soon. > > A series was posted > https://lore.kernel.org/all/20230627110353.1879477-1-xu.yang_2@nxp.com/, > I had no time to try or look at it yet. Thanks for keeping me up to date on this topic, which is still totally relevant to me. I looked at the series, but it does not seem to be addressing the problem with USB host not detecting new devices when VBUS is not directly connected, e.g. in the Colibri imx6ull SoM. Xu, do you confirm the series at the link is _not_ solving the problem being discussed here? Best regards, Luca
On Mon, Jul 17, 2023 at 06:45:37PM +0200, Luca Ceresoli wrote: > On Thu, 6 Jul 2023 12:23:43 +0200 > Francesco Dolcini <francesco@dolcini.it> wrote: > > On Tue, May 30, 2023 at 11:22:51AM +0000, Jun Li wrote: > > > Yes, your understanding is correct, talked with Xu(in CC), he will take this > > > soon. > > > > A series was posted > > https://lore.kernel.org/all/20230627110353.1879477-1-xu.yang_2@nxp.com/, > > I had no time to try or look at it yet. > > Thanks for keeping me up to date on this topic, which is still totally > relevant to me. > > I looked at the series, but it does not seem to be addressing the > problem with USB host not detecting new devices when VBUS is not > directly connected, e.g. in the Colibri imx6ull SoM. I recall having tried something similar, it could handle also the colibri imx6ull use case. Were you able to try it? These days I am busy with other tasks and I was not able to find the time (yet). Francesco
> -----Original Message----- > > Ciao Francesco, > > On Thu, 6 Jul 2023 12:23:43 +0200 > Francesco Dolcini <francesco@dolcini.it> wrote: > > > Hello Luca, > > > > On Tue, May 30, 2023 at 11:22:51AM +0000, Jun Li wrote: > > > Yes, your understanding is correct, talked with Xu(in CC), he will take this > > > soon. > > > > A series was posted > > > > I had no time to try or look at it yet. > > Thanks for keeping me up to date on this topic, which is still totally > relevant to me. > > I looked at the series, but it does not seem to be addressing the > problem with USB host not detecting new devices when VBUS is not > directly connected, e.g. in the Colibri imx6ull SoM. > > Xu, do you confirm the series at the link is _not_ solving the problem > being discussed here? Have you tried this patchset? The upstream driver couldn't get correct USB role from HW_USBPHY_CTRL register when the ID pin is float. This is what this patchset is trying to fix. With this patch, condition "(!vbus_is_on && !mxs_phy_is_otg_host(mxs_phy)" will always be false when controller acts as host role, then __mxs_phy_disconnect_line(phy, true) will never be called. So I think it doesn't matter whether VBUS is connected or not when act as host mode. If you still have issue after apply this patchset, please let me know. Thanks, Xu Yang > > Best regards, > > Luca >
Hello Xu, On Tue, 18 Jul 2023 08:31:48 +0000 Xu Yang <xu.yang_2@nxp.com> wrote: > > -----Original Message----- > > > > Ciao Francesco, > > > > On Thu, 6 Jul 2023 12:23:43 +0200 > > Francesco Dolcini <francesco@dolcini.it> wrote: > > > > > Hello Luca, > > > > > > On Tue, May 30, 2023 at 11:22:51AM +0000, Jun Li wrote: > > > > Yes, your understanding is correct, talked with Xu(in CC), he will take this > > > > soon. > > > > > > A series was posted > > > > > > I had no time to try or look at it yet. > > > > Thanks for keeping me up to date on this topic, which is still totally > > relevant to me. > > > > I looked at the series, but it does not seem to be addressing the > > problem with USB host not detecting new devices when VBUS is not > > directly connected, e.g. in the Colibri imx6ull SoM. > > > > Xu, do you confirm the series at the link is _not_ solving the problem > > being discussed here? > > Have you tried this patchset? The upstream driver couldn't get correct > USB role from HW_USBPHY_CTRL register when the ID pin is float. This is > what this patchset is trying to fix. With this patch, condition > "(!vbus_is_on && !mxs_phy_is_otg_host(mxs_phy)" will always be false when > controller acts as host role, then __mxs_phy_disconnect_line(phy, true) > will never be called. So I think it doesn't matter whether VBUS is connected > or not when act as host mode. If you still have issue after apply this patchset, > please let me know. I tested this patchset on top of v6.5-rc2 and I confirm 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 are never detected. For the archives, I'm replying also to the patch series. Luca
Hi Luca, > -----Original Message----- > > Hello Xu, > > On Tue, 18 Jul 2023 08:31:48 +0000 > Xu Yang <xu.yang_2@nxp.com> wrote: > > > > -----Original Message----- > > > > > > Ciao Francesco, > > > > > > On Thu, 6 Jul 2023 12:23:43 +0200 > > > Francesco Dolcini <francesco@dolcini.it> wrote: > > > > > > > Hello Luca, > > > > > > > > On Tue, May 30, 2023 at 11:22:51AM +0000, Jun Li wrote: > > > > > Yes, your understanding is correct, talked with Xu(in CC), he will take this > > > > > soon. > > > > > > > > A series was posted > > > > > > > > I had no time to try or look at it yet. > > > > > > Thanks for keeping me up to date on this topic, which is still totally > > > relevant to me. > > > > > > I looked at the series, but it does not seem to be addressing the > > > problem with USB host not detecting new devices when VBUS is not > > > directly connected, e.g. in the Colibri imx6ull SoM. > > > > > > Xu, do you confirm the series at the link is _not_ solving the problem > > > being discussed here? > > > > Have you tried this patchset? The upstream driver couldn't get correct > > USB role from HW_USBPHY_CTRL register when the ID pin is float. This is > > what this patchset is trying to fix. With this patch, condition > > "(!vbus_is_on && !mxs_phy_is_otg_host(mxs_phy)" will always be false when > > controller acts as host role, then __mxs_phy_disconnect_line(phy, true) > > will never be called. So I think it doesn't matter whether VBUS is connected > > or not when act as host mode. If you still have issue after apply this patchset, > > please let me know. > > I tested this patchset on top of v6.5-rc2 and I confirm 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 are never detected. Thanks for your feedback. As you said this issue will disappear with below change, right? diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c index e1a2b2ea098b..ec5ee790455e 100644 --- a/drivers/usb/phy/phy-mxs-usb.c +++ b/drivers/usb/phy/phy-mxs-usb.c @@ -178,7 +178,6 @@ static const struct mxs_phy_data imx6sx_phy_data = { }; static const struct mxs_phy_data imx6ul_phy_data = { - .flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS, }; static const struct mxs_phy_data imx7ulp_phy_data = { So I guess something in __mxs_phy_disconnect_line(mxs_phy, true) is causing this behavior. Could you please help to find which line to comment to make this issue disappear? Thanks, Xu Yang > > For the archives, I'm replying also to the patch series. > > Luca >
Hello Xu, thanks for the follow up. On Wed, 19 Jul 2023 11:23:26 +0000 Xu Yang <xu.yang_2@nxp.com> wrote: > Hi Luca, > > > -----Original Message----- > > > > Hello Xu, > > > > On Tue, 18 Jul 2023 08:31:48 +0000 > > Xu Yang <xu.yang_2@nxp.com> wrote: > > > > > > -----Original Message----- > > > > > > > > Ciao Francesco, > > > > > > > > On Thu, 6 Jul 2023 12:23:43 +0200 > > > > Francesco Dolcini <francesco@dolcini.it> wrote: > > > > > > > > > Hello Luca, > > > > > > > > > > On Tue, May 30, 2023 at 11:22:51AM +0000, Jun Li wrote: > > > > > > Yes, your understanding is correct, talked with Xu(in CC), he will take this > > > > > > soon. > > > > > > > > > > A series was posted > > > > > > > > > > I had no time to try or look at it yet. > > > > > > > > Thanks for keeping me up to date on this topic, which is still totally > > > > relevant to me. > > > > > > > > I looked at the series, but it does not seem to be addressing the > > > > problem with USB host not detecting new devices when VBUS is not > > > > directly connected, e.g. in the Colibri imx6ull SoM. > > > > > > > > Xu, do you confirm the series at the link is _not_ solving the problem > > > > being discussed here? > > > > > > Have you tried this patchset? The upstream driver couldn't get correct > > > USB role from HW_USBPHY_CTRL register when the ID pin is float. This is > > > what this patchset is trying to fix. With this patch, condition > > > "(!vbus_is_on && !mxs_phy_is_otg_host(mxs_phy)" will always be false when > > > controller acts as host role, then __mxs_phy_disconnect_line(phy, true) > > > will never be called. So I think it doesn't matter whether VBUS is connected > > > or not when act as host mode. If you still have issue after apply this patchset, > > > please let me know. > > > > I tested this patchset on top of v6.5-rc2 and I confirm 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 are never detected. > > Thanks for your feedback. As you said this issue will disappear with below change, right? > > diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c > index e1a2b2ea098b..ec5ee790455e 100644 > --- a/drivers/usb/phy/phy-mxs-usb.c > +++ b/drivers/usb/phy/phy-mxs-usb.c > @@ -178,7 +178,6 @@ static const struct mxs_phy_data imx6sx_phy_data = { > }; > > static const struct mxs_phy_data imx6ul_phy_data = { > - .flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS, > }; > > static const struct mxs_phy_data imx7ulp_phy_data = { Exactly. > So I guess something in __mxs_phy_disconnect_line(mxs_phy, true) is causing this behavior. > Could you please help to find which line to comment to make this issue disappear? I did some tests and detection works by doing _any_ of the following two changes (or both of them). Change 1: --- a/drivers/usb/phy/phy-mxs-usb.c +++ b/drivers/usb/phy/phy-mxs-usb.c @@ -359,10 +359,6 @@ static void __mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool disconnect) void __iomem *base = mxs_phy->phy.io_priv; u32 reg; - if (disconnect) - writel_relaxed(BM_USBPHY_DEBUG_CLKGATE, - base + HW_USBPHY_DEBUG_CLR); - if (mxs_phy->port_id == 0) { reg = disconnect ? ANADIG_USB1_LOOPBACK_SET : ANADIG_USB1_LOOPBACK_CLR; Change 2: --- a/drivers/usb/phy/phy-mxs-usb.c +++ b/drivers/usb/phy/phy-mxs-usb.c @@ -372,9 +372,6 @@ static void __mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool disconnect) } else if (mxs_phy->port_id == 1) { reg = disconnect ? ANADIG_USB2_LOOPBACK_SET : ANADIG_USB2_LOOPBACK_CLR; - regmap_write(mxs_phy->regmap_anatop, reg, - BM_ANADIG_USB2_LOOPBACK_UTMI_DIG_TST1 | - BM_ANADIG_USB2_LOOPBACK_TSTI_TX_EN); } if (!disconnect) I hope this clarifies something to you. Luca
Hi Luca, > > > -----Original Message----- > > > > > > Hello Xu, > > > > > > On Tue, 18 Jul 2023 08:31:48 +0000 > > > Xu Yang <xu.yang_2@nxp.com> wrote: > > > > > > > > -----Original Message----- > > > > > > > > > > Ciao Francesco, > > > > > > > > > > On Thu, 6 Jul 2023 12:23:43 +0200 > > > > > Francesco Dolcini <francesco@dolcini.it> wrote: > > > > > > > > > > > Hello Luca, > > > > > > > > > > > > On Tue, May 30, 2023 at 11:22:51AM +0000, Jun Li wrote: > > > > > > > Yes, your understanding is correct, talked with Xu(in CC), he will take this > > > > > > > soon. > > > > > > > > > > > > A series was posted > > > > > > > > > > > > I had no time to try or look at it yet. > > > > > > > > > > Thanks for keeping me up to date on this topic, which is still totally > > > > > relevant to me. > > > > > > > > > > I looked at the series, but it does not seem to be addressing the > > > > > problem with USB host not detecting new devices when VBUS is not > > > > > directly connected, e.g. in the Colibri imx6ull SoM. > > > > > > > > > > Xu, do you confirm the series at the link is _not_ solving the problem > > > > > being discussed here? > > > > > > > > Have you tried this patchset? The upstream driver couldn't get correct > > > > USB role from HW_USBPHY_CTRL register when the ID pin is float. This is > > > > what this patchset is trying to fix. With this patch, condition > > > > "(!vbus_is_on && !mxs_phy_is_otg_host(mxs_phy)" will always be false when > > > > controller acts as host role, then __mxs_phy_disconnect_line(phy, true) > > > > will never be called. So I think it doesn't matter whether VBUS is connected > > > > or not when act as host mode. If you still have issue after apply this patchset, > > > > please let me know. > > > > > > I tested this patchset on top of v6.5-rc2 and I confirm 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 are never detected. > > > > Thanks for your feedback. As you said this issue will disappear with below change, right? > > > > diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c > > index e1a2b2ea098b..ec5ee790455e 100644 > > --- a/drivers/usb/phy/phy-mxs-usb.c > > +++ b/drivers/usb/phy/phy-mxs-usb.c > > @@ -178,7 +178,6 @@ static const struct mxs_phy_data imx6sx_phy_data = { > > }; > > > > static const struct mxs_phy_data imx6ul_phy_data = { > > - .flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS, > > }; > > > > static const struct mxs_phy_data imx7ulp_phy_data = { > > Exactly. > > > So I guess something in __mxs_phy_disconnect_line(mxs_phy, true) is causing this behavior. > > Could you please help to find which line to comment to make this issue disappear? To correct what I said: __mxs_phy_disconnect_line(mxs_phy, false) should be called. I think the enable wakeup sequence should be follow for host-only port: mxs_phy_set_wakeup(mxs_phy, true) mxs_phy_disconnect_line(mxs_phy, true); __mxs_phy_disconnect_line(mxs_phy, false); And disable wakeup sequence: mxs_phy_set_wakeup(mxs_phy, false) mxs_phy_disconnect_line(mxs_phy, false); __mxs_phy_disconnect_line(mxs_phy, false); So "bool variable disconnect is false" all the time. > > I did some tests and detection works by doing _any_ of the following > two changes (or both of them). > > Change 1: > > --- a/drivers/usb/phy/phy-mxs-usb.c > +++ b/drivers/usb/phy/phy-mxs-usb.c > @@ -359,10 +359,6 @@ static void __mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool disconnect) > void __iomem *base = mxs_phy->phy.io_priv; > u32 reg; > > - if (disconnect) > - writel_relaxed(BM_USBPHY_DEBUG_CLKGATE, > - base + HW_USBPHY_DEBUG_CLR); > - Since disconnect = false, this code didn't get executed all the time. Remove this will have no impact. But your test results didn't align to this. Could you please help check the sequence? Is disconnect true or false when __mxs_phy_disconnect_line is called? Thanks, Xu Yang > if (mxs_phy->port_id == 0) { > reg = disconnect ? ANADIG_USB1_LOOPBACK_SET > : ANADIG_USB1_LOOPBACK_CLR; > > Change 2: > > --- a/drivers/usb/phy/phy-mxs-usb.c > +++ b/drivers/usb/phy/phy-mxs-usb.c > @@ -372,9 +372,6 @@ static void __mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool disconnect) > } else if (mxs_phy->port_id == 1) { > reg = disconnect ? ANADIG_USB2_LOOPBACK_SET > : ANADIG_USB2_LOOPBACK_CLR; > - regmap_write(mxs_phy->regmap_anatop, reg, > - BM_ANADIG_USB2_LOOPBACK_UTMI_DIG_TST1 | > - BM_ANADIG_USB2_LOOPBACK_TSTI_TX_EN); > } > > if (!disconnect) > > I hope this clarifies something to you. > > Luca >
Hi Xu, On Thu, 20 Jul 2023 10:13:57 +0000 Xu Yang <xu.yang_2@nxp.com> wrote: > Hi Luca, > > > > > -----Original Message----- > > > > > > > > Hello Xu, > > > > > > > > On Tue, 18 Jul 2023 08:31:48 +0000 > > > > Xu Yang <xu.yang_2@nxp.com> wrote: > > > > > > > > > > -----Original Message----- > > > > > > > > > > > > Ciao Francesco, > > > > > > > > > > > > On Thu, 6 Jul 2023 12:23:43 +0200 > > > > > > Francesco Dolcini <francesco@dolcini.it> wrote: > > > > > > > > > > > > > Hello Luca, > > > > > > > > > > > > > > On Tue, May 30, 2023 at 11:22:51AM +0000, Jun Li wrote: > > > > > > > > Yes, your understanding is correct, talked with Xu(in CC), he will take this > > > > > > > > soon. > > > > > > > > > > > > > > A series was posted > > > > > > > > > > > > > > I had no time to try or look at it yet. > > > > > > > > > > > > Thanks for keeping me up to date on this topic, which is still totally > > > > > > relevant to me. > > > > > > > > > > > > I looked at the series, but it does not seem to be addressing the > > > > > > problem with USB host not detecting new devices when VBUS is not > > > > > > directly connected, e.g. in the Colibri imx6ull SoM. > > > > > > > > > > > > Xu, do you confirm the series at the link is _not_ solving the problem > > > > > > being discussed here? > > > > > > > > > > Have you tried this patchset? The upstream driver couldn't get correct > > > > > USB role from HW_USBPHY_CTRL register when the ID pin is float. This is > > > > > what this patchset is trying to fix. With this patch, condition > > > > > "(!vbus_is_on && !mxs_phy_is_otg_host(mxs_phy)" will always be false when > > > > > controller acts as host role, then __mxs_phy_disconnect_line(phy, true) > > > > > will never be called. So I think it doesn't matter whether VBUS is connected > > > > > or not when act as host mode. If you still have issue after apply this patchset, > > > > > please let me know. > > > > > > > > I tested this patchset on top of v6.5-rc2 and I confirm 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 are never detected. > > > > > > Thanks for your feedback. As you said this issue will disappear with below change, right? > > > > > > diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c > > > index e1a2b2ea098b..ec5ee790455e 100644 > > > --- a/drivers/usb/phy/phy-mxs-usb.c > > > +++ b/drivers/usb/phy/phy-mxs-usb.c > > > @@ -178,7 +178,6 @@ static const struct mxs_phy_data imx6sx_phy_data = { > > > }; > > > > > > static const struct mxs_phy_data imx6ul_phy_data = { > > > - .flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS, > > > }; > > > > > > static const struct mxs_phy_data imx7ulp_phy_data = { > > > > Exactly. > > > > > So I guess something in __mxs_phy_disconnect_line(mxs_phy, true) is causing this behavior. > > > Could you please help to find which line to comment to make this issue disappear? > > To correct what I said: __mxs_phy_disconnect_line(mxs_phy, false) should > be called. > > I think the enable wakeup sequence should be follow for host-only port: > mxs_phy_set_wakeup(mxs_phy, true) > mxs_phy_disconnect_line(mxs_phy, true); > __mxs_phy_disconnect_line(mxs_phy, false); > > And disable wakeup sequence: > mxs_phy_set_wakeup(mxs_phy, false) > mxs_phy_disconnect_line(mxs_phy, false); > __mxs_phy_disconnect_line(mxs_phy, false); > > So "bool variable disconnect is false" all the time. > > > > > I did some tests and detection works by doing _any_ of the following > > two changes (or both of them). > > > > Change 1: > > > > --- a/drivers/usb/phy/phy-mxs-usb.c > > +++ b/drivers/usb/phy/phy-mxs-usb.c > > @@ -359,10 +359,6 @@ static void __mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool disconnect) > > void __iomem *base = mxs_phy->phy.io_priv; > > u32 reg; > > > > - if (disconnect) > > - writel_relaxed(BM_USBPHY_DEBUG_CLKGATE, > > - base + HW_USBPHY_DEBUG_CLR); > > - > > Since disconnect = false, this code didn't get executed all the time. > Remove this will have no impact. But your test results didn't align > to this. Could you please help check the sequence? Is disconnect > true or false when __mxs_phy_disconnect_line is called? What I observe is that __mxs_phy_disconnect_line(..., true) is called. This happens during boot and after unplugging a device in case one was detected during boot. This is because in mxs_phy_disconnect_line() [0]: - on = 1 - !vbus_is_on = 1 - !mxs_phy_is_otg_host(mxs_phy) = 1 Which one(s) of those three would you expect to be 0? Some additional info about the !mxs_phy_is_otg_host(mxs_phy) value: with or without CONFIG_USB_OTG, it always has the same value because phyctrl always has the BM_USBPHY_CTRL_OTG_ID_VALUE bit set. [0] https://elixir.bootlin.com/linux/v6.5-rc2/source/drivers/usb/phy/phy-mxs-usb.c#L415 Luca
Hi Luca, > -----Original Message----- > > Hi Xu, > > On Thu, 20 Jul 2023 10:13:57 +0000 > Xu Yang <xu.yang_2@nxp.com> wrote: > > > Hi Luca, > > > > > > > -----Original Message----- > > > > > > > > > > Hello Xu, > > > > > > > > > > On Tue, 18 Jul 2023 08:31:48 +0000 > > > > > Xu Yang <xu.yang_2@nxp.com> wrote: > > > > > > > > > > > > -----Original Message----- > > > > > > > > > > > > > > Ciao Francesco, > > > > > > > > > > > > > > On Thu, 6 Jul 2023 12:23:43 +0200 > > > > > > > Francesco Dolcini <francesco@dolcini.it> wrote: > > > > > > > > > > > > > > > Hello Luca, > > > > > > > > > > > > > > > > On Tue, May 30, 2023 at 11:22:51AM +0000, Jun Li wrote: > > > > > > > > > Yes, your understanding is correct, talked with Xu(in CC), he will take this > > > > > > > > > soon. > > > > > > > > > > > > > > > > A series was posted > > > > > > > > > > > > > > > > I had no time to try or look at it yet. > > > > > > > > > > > > > > Thanks for keeping me up to date on this topic, which is still totally > > > > > > > relevant to me. > > > > > > > > > > > > > > I looked at the series, but it does not seem to be addressing the > > > > > > > problem with USB host not detecting new devices when VBUS is not > > > > > > > directly connected, e.g. in the Colibri imx6ull SoM. > > > > > > > > > > > > > > Xu, do you confirm the series at the link is _not_ solving the problem > > > > > > > being discussed here? > > > > > > > > > > > > Have you tried this patchset? The upstream driver couldn't get correct > > > > > > USB role from HW_USBPHY_CTRL register when the ID pin is float. This is > > > > > > what this patchset is trying to fix. With this patch, condition > > > > > > "(!vbus_is_on && !mxs_phy_is_otg_host(mxs_phy)" will always be false when > > > > > > controller acts as host role, then __mxs_phy_disconnect_line(phy, true) > > > > > > will never be called. So I think it doesn't matter whether VBUS is connected > > > > > > or not when act as host mode. If you still have issue after apply this patchset, > > > > > > please let me know. > > > > > > > > > > I tested this patchset on top of v6.5-rc2 and I confirm 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 are never detected. > > > > > > > > Thanks for your feedback. As you said this issue will disappear with below change, right? > > > > > > > > diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c > > > > index e1a2b2ea098b..ec5ee790455e 100644 > > > > --- a/drivers/usb/phy/phy-mxs-usb.c > > > > +++ b/drivers/usb/phy/phy-mxs-usb.c > > > > @@ -178,7 +178,6 @@ static const struct mxs_phy_data imx6sx_phy_data = { > > > > }; > > > > > > > > static const struct mxs_phy_data imx6ul_phy_data = { > > > > - .flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS, > > > > }; > > > > > > > > static const struct mxs_phy_data imx7ulp_phy_data = { > > > > > > Exactly. > > > > > > > So I guess something in __mxs_phy_disconnect_line(mxs_phy, true) is causing this behavior. > > > > Could you please help to find which line to comment to make this issue disappear? > > > > To correct what I said: __mxs_phy_disconnect_line(mxs_phy, false) should > > be called. > > > > I think the enable wakeup sequence should be follow for host-only port: > > mxs_phy_set_wakeup(mxs_phy, true) > > mxs_phy_disconnect_line(mxs_phy, true); > > __mxs_phy_disconnect_line(mxs_phy, false); > > > > And disable wakeup sequence: > > mxs_phy_set_wakeup(mxs_phy, false) > > mxs_phy_disconnect_line(mxs_phy, false); > > __mxs_phy_disconnect_line(mxs_phy, false); > > > > So "bool variable disconnect is false" all the time. > > > > > > > > I did some tests and detection works by doing _any_ of the following > > > two changes (or both of them). > > > > > > Change 1: > > > > > > --- a/drivers/usb/phy/phy-mxs-usb.c > > > +++ b/drivers/usb/phy/phy-mxs-usb.c > > > @@ -359,10 +359,6 @@ static void __mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool disconnect) > > > void __iomem *base = mxs_phy->phy.io_priv; > > > u32 reg; > > > > > > - if (disconnect) > > > - writel_relaxed(BM_USBPHY_DEBUG_CLKGATE, > > > - base + HW_USBPHY_DEBUG_CLR); > > > - > > > > Since disconnect = false, this code didn't get executed all the time. > > Remove this will have no impact. But your test results didn't align > > to this. Could you please help check the sequence? Is disconnect > > true or false when __mxs_phy_disconnect_line is called? > > What I observe is that __mxs_phy_disconnect_line(..., true) is called. > This happens during boot and after unplugging a device in case one was > detected during boot. > > This is because in mxs_phy_disconnect_line() [0]: > - on = 1 > - !vbus_is_on = 1 > - !mxs_phy_is_otg_host(mxs_phy) = 1 > > Which one(s) of those three would you expect to be 0? > > Some additional info about the !mxs_phy_is_otg_host(mxs_phy) value: > with or without CONFIG_USB_OTG, it always has the same value because > phyctrl always has the BM_USBPHY_CTRL_OTG_ID_VALUE bit set. > I guess you didn't apply my whole patchset. [1/3] usb: chipidea: add USB PHY event [2/3] usb: phy: mxs: fix getting wrong state with mxs_phy_is_otg_host() [3/3] usb: phy: mxs: disconnect line when USB charger is attached The 2nd patch is going to fix the issue of mxs_phy_is_otg_host(): https://lore.kernel.org/all/20230627110353.1879477-2-xu.yang_2@nxp.com/ Our design is not to disconnect line when the controller is acting as host mode. Thanks, Xu Yang
Hi Luca, > Hello Xu, > > On Tue, 25 Jul 2023 12:23:43 +0000 > Xu Yang <xu.yang_2@nxp.com> wrote: > ... > > > I tested again now with the 3 patches applied and found that with > > > CONFIG_USB_OTG=y it works, and detects a device when plugged. Good! > > > > > > However with CONFIG_USB_OTG disabled it is still not working. In this > > > case obviously mxs_phy_is_otg_host() returns always false, even though > > > las_event is 2 (USB_EVENT_ID). This is what I get during boot with no > > > device connected: > > > > Yes, I may need to remove CONFIG_USB_OTG condifion. Will handle this later. > > Did you manage to make progress with this work? It looked like very > close to be fixed in the proper way with your latest patch iteration. I'm a little busy these days. I may handle this when I'm free or you can also fix it. Thanks, Xu Yang > > I'd definitely happy to test new patches as soon as you have any. > > Best regards, > Luca >
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c index 5ae16368a0c7..5078d0695eb7 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.c +++ b/drivers/usb/chipidea/ci_hdrc_imx.c @@ -434,6 +434,9 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev) usb_phy_init(pdata.usb_phy); } + if (of_property_read_bool(np, "disable-runtime-pm")) + pdata.flags &= ~CI_HDRC_SUPPORTS_RUNTIME_PM; + if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM) data->supports_runtime_pm = true;
The Toradex Colibri iMX6ULL board has a special USB hardware design. With runtime-pm enabled USB reset itself continuously. Furthermore the OTG port is also not enumerating devices if the Chipidea IP is in runtime sleep mode and a device or host gets plugged in. This patch adds the opportunity to disable Runtime Power Management from devicetree Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com> --- Changes in v2: - Change commit message to tell the use case for Colibri iMX6ULL drivers/usb/chipidea/ci_hdrc_imx.c | 3 +++ 1 file changed, 3 insertions(+)