From: Xu Yang <xu.yang_2@nxp.com>
To: peter.chen@kernel.org, gregkh@linuxfoundation.org, jun.li@nxp.com
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
imx@lists.linux.dev
Subject: [PATCH v2 2/3] usb: chipidea: udc: support dynamic gadget add/remove
Date: Thu, 23 Apr 2026 18:20:01 +0800 [thread overview]
Message-ID: <20260423102002.2675414-2-xu.yang_2@nxp.com> (raw)
In-Reply-To: <20260423102002.2675414-1-xu.yang_2@nxp.com>
An asynchronous vbus_event_work() keep running when switch the role from
device to host. This affects EHCI host controller initialization.
USBCMD.RUNSTOP bit is set at ehci_run() and cleared by following
vbus_event_work() if bus_event_work() run after ehci_run().
The log below shows what happens:
[ 87.819925] ci_hdrc ci_hdrc.0: EHCI Host Controller
[ 87.819963] ci_hdrc ci_hdrc.0: new USB bus registered, assigned bus number 1
[ 87.955634] ci_hdrc ci_hdrc.0: USB 2.0, controller refused to start: -110
[ 87.955658] ci_hdrc ci_hdrc.0: startup error -110
[ 87.955682] ci_hdrc ci_hdrc.0: USB bus 1 deregistered
The problem is that the chipidea UDC driver call usb_udc_vbus_handler() to
pull down data line but it don't wait for completion before host controller
starts running.
Now UDC core can properly delete usb gadget device and make sure that vbus
work is cancelled or completed after usb_del_gadget_udc() is returned. But
the udc.c only call usb_del_gadget_udc() in ci_hdrc_gadget_destroy(). To
avoid above issue, add/remove the gadget device dynamically during USB role
switching.
To support dynamic gadget add/remove, do below steps:
- clear ci->gadget and ci->ci_hw_ep at initialization.
- assign udc_[start|stop]() to rdrv->[start|stop] and properly merge the
operations in udc_id_switch_for_[device|host]() to udc_[start|stop]()
Adjust the order ci_handle_vbus_change() and ci_role_start() to avoid NULL
pointer reference since ci_hdrc_gadget_init() doesn't add gadget anymore.
Acked-by: Peter Chen <peter.chen@kernel.org>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
Changes in v2:
- add A-b tag
- refine the commit message
---
drivers/usb/chipidea/core.c | 11 +++----
drivers/usb/chipidea/udc.c | 65 +++++++++++++++++++------------------
2 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 7cfabb04a4fb..95d9db159ce8 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -1191,19 +1191,16 @@ static int ci_hdrc_probe(struct platform_device *pdev)
ci->role = ci_get_role(ci);
if (!ci_otg_is_fsm_mode(ci)) {
- /* only update vbus status for peripheral */
- if (ci->role == CI_ROLE_GADGET) {
- /* Pull down DP for possible charger detection */
- hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
- ci_handle_vbus_change(ci);
- }
-
ret = ci_role_start(ci, ci->role);
if (ret) {
dev_err(dev, "can't start %s role\n",
ci_role(ci)->name);
goto stop;
}
+
+ /* only update vbus status for peripheral */
+ if (ci->role == CI_ROLE_GADGET)
+ ci_handle_vbus_change(ci);
}
ret = devm_request_irq(dev, ci->irq, ci_irq_handler, IRQF_SHARED,
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index d4277d6611ee..d52f89489893 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -2044,6 +2044,8 @@ static int init_eps(struct ci_hdrc *ci)
{
int retval = 0, i, j;
+ memset(ci->ci_hw_ep, 0, sizeof(ci->ci_hw_ep));
+
for (i = 0; i < ci->hw_ep_max/2; i++)
for (j = RX; j <= TX; j++) {
int k = i + j * ci->hw_ep_max/2;
@@ -2289,6 +2291,8 @@ static int udc_start(struct ci_hdrc *ci)
struct usb_otg_caps *otg_caps = &ci->platdata->ci_otg_caps;
int retval = 0;
+ memset(&ci->gadget, 0, sizeof(ci->gadget));
+
ci->gadget.ops = &usb_gadget_ops;
ci->gadget.speed = USB_SPEED_UNKNOWN;
ci->gadget.max_speed = USB_SPEED_HIGH;
@@ -2327,10 +2331,15 @@ static int udc_start(struct ci_hdrc *ci)
ci->gadget.ep0 = &ci->ep0in->ep;
+ if (ci->platdata->pins_device)
+ pinctrl_select_state(ci->platdata->pctl,
+ ci->platdata->pins_device);
+
retval = usb_add_gadget_udc(dev, &ci->gadget);
if (retval)
goto destroy_eps;
+ ci_udc_enable_vbus_irq(ci, true);
return retval;
destroy_eps:
@@ -2342,38 +2351,20 @@ static int udc_start(struct ci_hdrc *ci)
return retval;
}
-/*
- * ci_hdrc_gadget_destroy: parent remove must call this to remove UDC
- *
- * No interrupts active, the IRQ has been released
+/**
+ * udc_stop: deinitialize gadget role
+ * @ci: chipidea controller
*/
-void ci_hdrc_gadget_destroy(struct ci_hdrc *ci)
+static void udc_stop(struct ci_hdrc *ci)
{
- if (!ci->roles[CI_ROLE_GADGET])
- return;
-
+ ci_udc_enable_vbus_irq(ci, false);
usb_del_gadget_udc(&ci->gadget);
+ ci->vbus_active = 0;
destroy_eps(ci);
dma_pool_destroy(ci->td_pool);
dma_pool_destroy(ci->qh_pool);
-}
-
-static int udc_id_switch_for_device(struct ci_hdrc *ci)
-{
- if (ci->platdata->pins_device)
- pinctrl_select_state(ci->platdata->pctl,
- ci->platdata->pins_device);
-
- ci_udc_enable_vbus_irq(ci, true);
- return 0;
-}
-
-static void udc_id_switch_for_host(struct ci_hdrc *ci)
-{
- ci_udc_enable_vbus_irq(ci, false);
- ci->vbus_active = 0;
if (ci->platdata->pins_device && ci->platdata->pins_default)
pinctrl_select_state(ci->platdata->pctl,
@@ -2422,7 +2413,6 @@ static void udc_resume(struct ci_hdrc *ci, bool power_lost)
int ci_hdrc_gadget_init(struct ci_hdrc *ci)
{
struct ci_role_driver *rdrv;
- int ret;
if (!hw_read(ci, CAP_DCCPARAMS, DCCPARAMS_DC))
return -ENXIO;
@@ -2431,8 +2421,8 @@ int ci_hdrc_gadget_init(struct ci_hdrc *ci)
if (!rdrv)
return -ENOMEM;
- rdrv->start = udc_id_switch_for_device;
- rdrv->stop = udc_id_switch_for_host;
+ rdrv->start = udc_start;
+ rdrv->stop = udc_stop;
#ifdef CONFIG_PM_SLEEP
rdrv->suspend = udc_suspend;
rdrv->resume = udc_resume;
@@ -2440,9 +2430,22 @@ int ci_hdrc_gadget_init(struct ci_hdrc *ci)
rdrv->irq = udc_irq;
rdrv->name = "gadget";
- ret = udc_start(ci);
- if (!ret)
- ci->roles[CI_ROLE_GADGET] = rdrv;
+ ci->roles[CI_ROLE_GADGET] = rdrv;
- return ret;
+ /* Pull down DP for possible charger detection */
+ hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
+ return 0;
+}
+
+/*
+ * ci_hdrc_gadget_destroy: parent remove must call this to remove UDC
+ *
+ * No interrupts active, the IRQ has been released
+ */
+void ci_hdrc_gadget_destroy(struct ci_hdrc *ci)
+{
+ struct device *dev = &ci->gadget.dev;
+
+ if (ci->roles[CI_ROLE_GADGET] && device_is_registered(dev))
+ udc_stop(ci);
}
--
2.34.1
next prev parent reply other threads:[~2026-04-23 10:17 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-23 10:20 [PATCH v2 1/3] usb: chipidea: udc: add a helper ci_udc_enable_vbus_irq() Xu Yang
2026-04-23 10:20 ` Xu Yang [this message]
2026-04-23 10:20 ` [PATCH v2 3/3] usb: chipidea: core: convert ci_role_switch to local variable Xu Yang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260423102002.2675414-2-xu.yang_2@nxp.com \
--to=xu.yang_2@nxp.com \
--cc=gregkh@linuxfoundation.org \
--cc=imx@lists.linux.dev \
--cc=jun.li@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=peter.chen@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox