All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Petar Stepanovic" <pstepanovic@axiado.com>
Cc: linux-usb@vger.kernel.org, devicetree@vger.kernel.org,
	robh@kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH v4 6/6] usb: gadget: udc: crg: Add core driver and build integration
Date: Fri, 30 Oct 2026 08:46:23 +0000	[thread overview]
Message-ID: <sashiko-outbox-190713@kernel.org> (raw)
In-Reply-To: <20261030-axiado-ax3000-usb-device-controller-v4-6-c5267b36a777@axiado.com>

Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
- [High] usb: udc: crg: dequeue fails in crg_udc_issue_command() when stopped
- [High] usb: gadget: crg: controller left stopped after disconnect reset
- [High] usb: gadget: crg: deferred crg_udc_prepare_for_setup() races with SETUP
- [High] usb: gadget: crg: GFP_KERNEL allocation in crg_udc_ep_enable()
- [High] usb: gadget: crg: race conditions when crg_udc_reset() drops udc_lock
- [Medium] usb: udc: crg: stale reconnect and ep0 state across crg_udc_reset()
- [Medium] usb: gadget: udc: crg: state inconsistency across crg_udc_suspend() and resume
- [Medium] usb: gadget: udc: crg: race in crg_udc_suspend() re-enables pullup
- [Medium] usb: gadget: crg: race with IRQ thread in crg_udc_shutdown() and teardown
- [Low] usb: gadget: crg: uncapped and unvalidated max_speed in crg_udc_probe()
--

commit ff1f5c0791ff275876830e2f2710870a97b34e7c
Author: Petar Stepanovic <pstepanovic@axiado.com>

usb: gadget: udc: crg: Add core driver and build integration

This commit introduces the core probe, remove, power management, interrupt
handling, and controller reset functions for the Axiado AX3000/AX3005 USB
Device Controller driver, and integrates it into Kconfig and Makefile.

> diff --git a/drivers/usb/gadget/udc/crg/crg-udc-core.c b/drivers/usb/gadget/udc/crg/crg-udc-core.c
> new file mode 100644
> index 0000000000000..33be5c920ebee
> --- /dev/null
> +++ b/drivers/usb/gadget/udc/crg/crg-udc-core.c
> @@ -0,0 +1,723 @@

[ ... ]

> +int crg_udc_issue_command(struct crg_gadget_dev *crg_udc,
> +			  enum crg_cmd_type type, u32 param0, u32 param1)
> +{
> +	struct crg_uccr __iomem *uccr = crg_udc->uccr;
> +	u32 status;
> +	u32 tmp;
> +	bool running;
> +
> +	running = !!(readl(&uccr->control) & CRG_U3DC_CTRL_RUN);
> +	/* Only INIT_EP0 may be staged while the controller is stopped. */
> +	if (!running && type != CRG_CMD_INIT_EP0)
> +		return -ESHUTDOWN;

[Severity: High]
As reported by Sashiko on v3 [1], rejecting all commands other than
CRG_CMD_INIT_EP0 when CRG_U3DC_CTRL_RUN is clear causes crg_udc_ep_dequeue()
to fail with -ESHUTDOWN whenever the pullup has been disabled.

During gadget unbind (gadget_unbind_driver() -> usb_gadget_disconnect_locked()
-> crg_gadget_pullup()), soft_connect disable, or crg_udc_suspend(),
crg_udc_set_pullup(crg_udc, false) clears CRG_U3DC_CTRL_RUN in uccr->control
while enabled endpoints remain in EP_STATE_RUNNING with active requests on
udc_ep->queue:

drivers/usb/gadget/udc/crg/crg-udc-gadget.c:crg_udc_ep_dequeue() {
	...
	if (old_ep_state == EP_STATE_RUNNING) {
		int ret = crg_udc_issue_command(crg_udc, CRG_CMD_STOP_EP,
						BIT(dci), 0);

		if (ret) {
			spin_unlock_irqrestore(&crg_udc->udc_lock, flags);
			return ret;
		}
	...
}

When a function driver's disconnect() or unbind() callback (or a FunctionFS
endpoint close) calls usb_ep_dequeue() -> crg_udc_ep_dequeue() before
crg_gadget_stop() runs, crg_udc_issue_command() returns -ESHUTDOWN for
CRG_CMD_STOP_EP (and would also reject CRG_CMD_SET_TR_DQPTR in
crg_udc_epcx_update_dqptr()). Because crg_udc_ep_dequeue() aborts without
unlinking or giving back the request, a subsequent usb_ep_free_request()
frees the request while it is still linked on udc_ep->queue, leading to a
use-after-free when crg_gadget_stop() -> crg_udc_reset() -> crg_udc_ep_nuke()
later traverses udc_ep->queue.

How should crg_udc_ep_dequeue() unlink and give back queued requests when
CRG_U3DC_CTRL_RUN is already clear?

[ ... ]

> +static int crg_udc_probe(struct platform_device *pdev)
> +{

[ ... ]

> +	crg_udc->gadget.ops = &crg_gadget_ops;
> +	crg_udc->gadget.ep0 = &crg_udc->udc_ep[0].usb_ep;
> +	INIT_LIST_HEAD(&crg_udc->gadget.ep_list);
> +	/*
> +	 * The controller supports up to SuperSpeed. Honour the standard
> +	 * "maximum-speed" property to cap it; default to the hardware
> +	 * maximum when the property is absent.
> +	 */
> +	crg_udc->gadget.max_speed = usb_get_maximum_speed(&pdev->dev);
> +	if (crg_udc->gadget.max_speed == USB_SPEED_UNKNOWN)
> +		crg_udc->gadget.max_speed = USB_SPEED_SUPER;
> +	crg_udc->max_speed = crg_udc->gadget.max_speed;

[Severity: Low]
The comment, Kconfig, and DT binding state that the controller supports
High-Speed and SuperSpeed and that the maximum-speed property caps the speed
up to SuperSpeed, but crg_udc_probe() only checks for USB_SPEED_UNKNOWN.

If the DT maximum-speed property is set to super-speed-plus
(USB_SPEED_SUPER_PLUS), crg_udc->gadget.max_speed is not capped to
USB_SPEED_SUPER and crg_udc_program_speed() writes CRG_U3DC_CFG0_MAXSPEED_SSP.
Conversely, if maximum-speed (or a gadget driver's max_speed passed to
crg_udc_set_speed()) is USB_SPEED_LOW, crg_udc_program_speed() returns
-EINVAL and aborts crg_udc_reset_data_struct().

Should crg_udc_probe() and crg_udc_set_speed() clamp speeds above
USB_SPEED_SUPER (or outside the supported range) to USB_SPEED_SUPER?

[ ... ]

> +static void crg_udc_shutdown(struct platform_device *pdev)
> +{
> +	struct crg_gadget_dev *crg_udc = platform_get_drvdata(pdev);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&crg_udc->udc_lock, flags);
> +	crg_udc->pullup_on = false;
> +	crg_udc->resetting = true;
> +	crg_udc->device_state = USB_STATE_ATTACHED;
> +	crg_udc_set_pullup(crg_udc, false);
> +	crg_udc_set_vbus_detect(crg_udc, false);
> +	if (crg_udc->uccr) {
> +		u32 val = readl(&crg_udc->uccr->control);
> +
> +		val &= ~(CRG_U3DC_CTRL_INT_EN | CRG_U3DC_CTRL_RUN);
> +		writel(val, &crg_udc->uccr->control);
> +	}
> +	spin_unlock_irqrestore(&crg_udc->udc_lock, flags);
> +	if (crg_udc->irq >= 0)
> +		synchronize_irq(crg_udc->irq);
> +	cancel_work_sync(&crg_udc->udc_reset_work);
> +}

[Severity: Medium]
As reported by Sashiko on v3 [2], what happens if crg_udc_irq_thread() is
already mid-recovery in crg_gadget_handle_interrupt() (or in the disconnect
reset path of crg_udc_handle_port_status()) and has dropped udc_lock inside
crg_udc_notify_disconnect() or crg_udc_reset() when crg_udc_shutdown() runs?

Once crg_udc_shutdown() releases udc_lock and waits in synchronize_irq(), the
in-flight IRQ thread reacquires udc_lock, runs crg_udc_reset_data_struct() and
crg_udc_init_ep0(), and sets crg_udc->resetting = false, undoing the shutdown
gate.

Similarly, in crg_udc_free_resources(), uccr->control is modified without
holding udc_lock before devm_free_irq() is called, while crg_udc_irq_thread()
can still be running after crg_gadget_stop() dropped udc_lock during
crg_udc_reset(). Couldn't that race on uccr->control or with resource
teardown?

> +
> +static int crg_udc_suspend(struct device *dev)
> +{
> +	struct crg_gadget_dev *crg_udc = dev_get_drvdata(dev);
> +	unsigned long flags;
> +
> +	/* Stop the pullup while the controller is suspended. */
> +	spin_lock_irqsave(&crg_udc->udc_lock, flags);
> +	if (!crg_udc->resetting)
> +		crg_udc_set_pullup(crg_udc, false);
> +	spin_unlock_irqrestore(&crg_udc->udc_lock, flags);
> +	return 0;
> +}

[Severity: Medium]
Can an in-flight CRG_U3DC_STATUS_SYS_ERR recovery in
crg_gadget_handle_interrupt() re-enable the pullup after crg_udc_suspend()
returns?

During system suspend, crg_udc_suspend() runs before device IRQs are disabled.
If crg_gadget_handle_interrupt() is handling CRG_U3DC_STATUS_SYS_ERR and has
dropped udc_lock in crg_udc_notify_disconnect() or crg_udc_reset(),
crg_udc_suspend() either sees crg_udc->resetting == true and skips
crg_udc_set_pullup(crg_udc, false), or clears CRG_U3DC_CTRL_RUN and returns
while crg_udc->pullup_on remains true. When the IRQ thread finishes reset
recovery, it checks:

	if (!ret && crg_udc->pullup_on)
		crg_udc_set_pullup(crg_udc, true);

which re-asserts CRG_U3DC_CTRL_RUN while the controller is suspended. Should
crg_udc_suspend() synchronize with the IRQ/reset path and track a suspended
state so recovery does not re-assert CRG_U3DC_CTRL_RUN?

> +
> +static int crg_udc_resume(struct device *dev)
> +{
> +	struct crg_gadget_dev *crg_udc = dev_get_drvdata(dev);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&crg_udc->udc_lock, flags);
> +	if (!crg_udc->resetting && crg_udc->gadget_driver && crg_udc->pullup_on)
> +		crg_udc_set_pullup(crg_udc, true);
> +	spin_unlock_irqrestore(&crg_udc->udc_lock, flags);
> +
> +	return 0;
> +}

[Severity: Medium]
When crg_udc_suspend() clears CRG_U3DC_CTRL_RUN via
crg_udc_set_pullup(crg_udc, false) while the gadget is enumerated, the
controller halts and drops bus terminations so the host sees a disconnect,
but crg_udc_suspend() does not notify the gadget driver of disconnect or
reset crg_udc->connected, crg_udc->device_state, crg_udc->dev_addr, or
enabled endpoints.

When crg_udc_resume() re-asserts CRG_U3DC_CTRL_RUN:

1. If the cable was unplugged while suspended, CCS is already 0 when
   CRG_U3DC_CTRL_RUN is set, so no CSC event is generated and the gadget
   driver remains stuck in a connected/configured state with queued requests
   never completing.

2. If the cable remains connected to a SuperSpeed host, link training to U0
   raises CRG_U3DC_PORTSC_CSC without CRG_U3DC_PORTSC_PRC. In
   crg_udc_handle_port_status(), the CSC connect branch does not call
   crg_udc_reinit(), and because crg_udc->device_state is still
   USB_STATE_CONFIGURED or USB_STATE_ADDRESS (>= USB_STATE_RECONNECTING),
   neither crg_udc_enable_setup() nor crg_udc_prepare_for_setup() runs, the
   stale dev_addr and endpoints remain active, and setaddressrequest() stalls
   SET_ADDRESS in USB_STATE_CONFIGURED.

Should crg_udc_suspend() disconnect the gadget session and reset controller
state before suspending?

[ ... ]

> +static void crg_gadget_handle_interrupt(struct crg_gadget_dev *crg_udc)
> +{
> +	struct crg_uccr __iomem *uccr = crg_udc->uccr;
> +	unsigned long flags = 0;
> +	u32 tmp_status;
> +	int ret;
> +
> +	spin_lock_irqsave(&crg_udc->udc_lock, flags);
> +
> +	tmp_status = readl(&uccr->status);
> +
> +	if (tmp_status & CRG_U3DC_STATUS_SYS_ERR) {
> +		bool was_connected = crg_udc->connected;
> +
> +		dev_err(crg_udc->dev, "controller system error\n");
> +		writel(CRG_U3DC_STATUS_SYS_ERR, &uccr->status);
> +		/* Function drivers must see enabled endpoints on disconnect. */
> +		if (was_connected)
> +			crg_udc_notify_disconnect(crg_udc, &flags);
> +		crg_udc->connected = 0;
> +		ret = crg_udc_reset(crg_udc, &flags);
> +		if (!ret)
> +			ret = crg_udc_reset_data_struct(crg_udc);
> +		if (!ret)
> +			ret = crg_udc_init_ep0(crg_udc);
> +		if (ret)
> +			dev_err(crg_udc->dev,
> +				"failed to recover from controller error: %d\n",
> +				ret);
> +		crg_udc->gadget.speed = USB_SPEED_UNKNOWN;
> +		if (!ret)
> +			crg_udc->resetting = false;
> +		if (!ret && crg_udc->pullup_on)
> +			crg_udc_set_pullup(crg_udc, true);

[Severity: High]
Here in the CRG_U3DC_STATUS_SYS_ERR recovery path, after crg_udc_reset(),
crg_udc_reset_data_struct(), and crg_udc_init_ep0() complete, the driver
restores the controller's running state via crg_udc_set_pullup(crg_udc, true)
when crg_udc->pullup_on is set.

However, in the cable disconnect (!cable_connected) branch of
crg_udc_handle_port_status(), the driver runs the same crg_udc_reset(),
crg_udc_reset_data_struct(), and crg_udc_init_ep0() sequence (which clears
uccr->config1 as well as CRG_U3DC_CTRL_INT_EN and CRG_U3DC_CTRL_RUN in
uccr->control, and stages CRG_CMD_INIT_EP0 while stopped) and then returns
-ECONNRESET without calling crg_udc_set_pullup(crg_udc, true):

drivers/usb/gadget/udc/crg/crg-udc-gadget.c:crg_udc_handle_port_status() {
	...
		} else if (!cable_connected) {
			dev_dbg(crg_udc->dev,
				"cable disconnected, rst controller\n");
			if (was_connected)
				crg_udc_notify_disconnect(crg_udc, flags);
			ret = crg_udc_reset(crg_udc, flags);
			if (!ret)
				ret = crg_udc_reset_data_struct(crg_udc);
			if (!ret)
				ret = crg_udc_init_ep0(crg_udc);
			if (ret)
				dev_err(crg_udc->dev,
					"controller reset failed on disconnect: %d\n",
					ret);
			else
				crg_udc->resetting = false;
			crg_udc->connected = 0;

			crg_udc->device_state = USB_STATE_ATTACHED;
			return -ECONNRESET;
		}
}

Because gadget disconnect callbacks do not re-invoke the pullup callback,
won't unplugging the USB cable leave CRG_U3DC_CTRL_RUN, CRG_U3DC_CTRL_INT_EN,
and uccr->config1 cleared so the controller never consumes CRG_CMD_INIT_EP0
or detects a subsequent cable reconnection?

[ ... ]

> +	if (tmp_status & CRG_U3DC_STATUS_EINT) {
> +		int i;
> +
> +		writel(CRG_U3DC_STATUS_EINT, &uccr->status);
> +
> +		for (i = 0; i < CRG_RING_NUM; i++) {
> +			ret = crg_udc_process_event_ring(crg_udc, i, &flags);

[Severity: High]
When crg_udc_process_event_ring() dispatches a SET_CONFIGURATION or
SET_INTERFACE control transfer via crg_udc_handle_event() ->
crg_udc_handle_setup_pkt(), it drops crg_udc->udc_lock and calls
driver->setup(&crg_udc->gadget, setup_pkt).

In composite_setup(), set_config() and f->set_alt() run under
spin_lock(&cdev->lock) and call usb_ep_enable() -> crg_udc_ep_enable().
However, crg_udc_ep_enable() drops udc_lock and allocates the transfer ring
with GFP_KERNEL:

drivers/usb/gadget/udc/crg/crg-udc-gadget.c:crg_udc_ep_enable() {
	...
		spin_unlock_irqrestore(&crg_udc->udc_lock, flags);
		new_ring = dma_alloc_coherent(crg_udc->dev, len, &new_dma,
					      GFP_KERNEL);
		spin_lock_irqsave(&crg_udc->udc_lock, flags);
	...
}

Since usb_ep_enable() is called while composite holds cdev->lock (and the
gadget API requires ep->ops->enable() to be callable in atomic context),
won't dma_alloc_coherent(..., GFP_KERNEL) sleep in atomic context?

> +			if (ret)
> +				dev_err(crg_udc->dev,
> +					"process event ring error: %d\n", ret);
> +		}
> +	}
> +
> +	if (crg_udc->device_state == USB_STATE_RECONNECTING &&
> +	    crg_udc->portsc_on_reconnecting == 1 &&
> +	    crg_udc_is_event_rings_empty(crg_udc)) {
> +		crg_udc->portsc_on_reconnecting = 0;
> +		crg_udc_handle_port_status(crg_udc, &flags);
> +	}
> +
> +	if (crg_udc->device_state == USB_STATE_RECONNECTING &&
> +	    crg_udc->connected == 1)
> +		crg_udc_prepare_for_setup(crg_udc);

[Severity: High]
When a bus reset (CRG_U3DC_PORTSC_PRC) completes while crg_udc->device_state
is at least USB_STATE_DEFAULT, crg_udc_handle_port_status() calls
crg_udc_reinit(), which sets crg_udc->device_state = USB_STATE_RECONNECTING,
and sets crg_udc->connected = 1 while CRG_U3DC_CFG1_SETUP_EVENT_EN remains
enabled in uccr->config1 from the prior connection.

Because crg_udc_prepare_for_setup() is only called here after
crg_udc_process_event_ring() finishes (and returns -EBUSY if the event ring
is not empty), what happens if the host's first post-reset SETUP packet is
already in the event ring behind the port status event, or arrives before
crg_udc_prepare_for_setup() checks crg_udc_is_event_rings_empty()?

In that case, crg_udc_process_event_ring() processes TRB_TYPE_EVT_SETUP_PKT
while crg_udc->device_state is still USB_STATE_RECONNECTING:
setaddressrequest() rejects USB_REQ_SET_ADDRESS and stalls EP0 because
device_state is neither USB_STATE_DEFAULT nor USB_STATE_ADDRESS, or another
control request queues a transfer on EP0 only for crg_udc_prepare_for_setup()
to immediately stop EP0 and nuke the newly queued request with -ESHUTDOWN.

[ ... ]

> +int crg_udc_reset(struct crg_gadget_dev *crg_udc, unsigned long *flags)
> +{
> +	struct crg_uccr __iomem *uccr = crg_udc->uccr;
> +	u32 i, tmp;
> +	struct crg_udc_ep *udc_ep_ptr;
> +	u32 val;
> +
> +	/*
> +	 * Keep all endpoint operations gated until the caller has recreated
> +	 * the event rings, device context and EP0 after this reset.
> +	 */
> +	crg_udc->resetting = true;
> +	val = readl(&uccr->control);
> +	val |= CRG_U3DC_CTRL_SWRST;
> +	writel(val, &uccr->control);
> +	spin_unlock_irqrestore(&crg_udc->udc_lock, *flags);
> +	/* Poll until SWRST clears; hardware reset takes up to 10 ms */
> +	if (readl_poll_timeout_atomic(&uccr->control, tmp,
> +			       !(tmp & CRG_U3DC_CTRL_SWRST), 10, 10000)) {
> +		spin_lock_irqsave(&crg_udc->udc_lock, *flags);
> +		dev_err(crg_udc->dev, "reset error\n");
> +		return -ETIMEDOUT;
> +	}
> +	spin_lock_irqsave(&crg_udc->udc_lock, *flags);
> +
> +	crg_udc_clear_portpm(crg_udc);
> +
> +	crg_udc->setup_status = WAIT_FOR_SETUP;
> +	/* Base on Figure 9-1, default USB_STATE is attached */
> +	crg_udc->device_state = USB_STATE_ATTACHED;
> +	crg_udc->dev_addr = 0;

[Severity: Medium]
When crg_udc_reset() resets crg_udc->device_state to USB_STATE_ATTACHED, it
does not clear crg_udc->portsc_on_reconnecting, crg_udc->setup_fn_call_back,
or crg_udc->set_tm (nor does crg_udc_reset_data_struct()).

If a port status event sets crg_udc->portsc_on_reconnecting = 1 while
crg_udc->device_state == USB_STATE_RECONNECTING and crg_udc_reset() runs
before that flag is consumed (for example due to a cable disconnect,
CRG_U3DC_STATUS_SYS_ERR, or crg_gadget_stop()), won't the stale
portsc_on_reconnecting == 1 persist until the next connection's bus reset
sets device_state = USB_STATE_RECONNECTING, causing crg_udc_prepare_for_setup()
to fail with -EBUSY or crg_gadget_handle_interrupt() to call
crg_udc_handle_port_status() spuriously?

> +
> +	/*
> +	 * Mark all endpoints unavailable before request callbacks drop the
> +	 * lock. A callback must not enqueue on a later, not-yet-nuked EP.
> +	 */
> +	for (i = 2; i < CRG_NUM_EP_CX; i++) {
> +		udc_ep_ptr = &crg_udc->udc_ep[i];
> +		udc_ep_ptr->generation++;
> +		udc_ep_ptr->desc = NULL;
> +		udc_ep_ptr->comp_desc = NULL;
> +		udc_ep_ptr->tran_ring_full = false;
> +		udc_ep_ptr->ep_state = EP_STATE_DISABLED;
> +		/*
> +		 * This bypasses the generic usb_ep_disable() API, so the
> +		 * framework's own usb_ep.enabled bookkeeping needs to be
> +		 * cleared directly here too (crg_udc_reinit() already does
> +		 * this for the live bus-reset case). Otherwise a later
> +		 * usb_ep_enable() call for this endpoint short-circuits on
> +		 * the stale "already enabled" flag without ever calling our
> +		 * .enable callback, leaving desc/the ring never set up for
> +		 * the new session.
> +		 */
> +		udc_ep_ptr->usb_ep.enabled = 0;
> +	}
> +	crg_udc->num_enabled_eps = 0;
> +
> +	udc_ep_ptr = &crg_udc->udc_ep[0];
> +	udc_ep_ptr->generation++;
> +	udc_ep_ptr->desc = NULL;
> +	udc_ep_ptr->comp_desc = NULL;
> +	udc_ep_ptr->ep_state = EP_STATE_DISABLED;
> +
> +	for (i = 2; i < CRG_NUM_EP_CX; i++)
> +		crg_udc_ep_nuke(&crg_udc->udc_ep[i], -ESHUTDOWN);
> +	crg_udc_ep_nuke(udc_ep_ptr, -ESHUTDOWN);

[Severity: High]
As reported by Sashiko on v3 [3], dropping udc_lock inside crg_udc_reset()
(both during the 10 ms CRG_U3DC_CTRL_SWRST poll and inside crg_udc_ep_nuke()
-> crg_udc_req_done()) introduces several races:

1. Neither crg_gadget_handle_interrupt() nor crg_udc_process_event_ring()
   checks crg_udc->resetting after acquiring or reacquiring udc_lock, and
   udc_event->generation is not incremented until crg_udc_reset_data_struct()
   -> crg_udc_init_event_ring() runs after crg_udc_reset() returns. If
   crg_udc_irq_thread() acquires udc_lock while crg_udc_reset() is polling
   CRG_U3DC_CTRL_SWRST or giving back requests, it can access controller
   registers and process stale event ring entries mid-reset.

2. Callers of crg_udc_reset() (crg_gadget_handle_interrupt(),
   crg_udc_handle_port_status(), and crg_gadget_stop()) do not serialize
   against an already-running reset. If two reset paths overlap across the
   udc_lock drops, one can run crg_udc_reset_data_struct() and
   crg_udc_init_ep0() while the other's CRG_U3DC_CTRL_SWRST is active, or
   the second crg_udc_init_ep0() can see CRG_U3DC_CMD_CTRL_ACTIVE still set
   from the first staged CRG_CMD_INIT_EP0 while CRG_U3DC_CTRL_RUN is 0 and
   fail with -EBUSY.

3. During the 10 ms SWRST poll window, crg_udc->resetting is true while
   usb_ep.enabled is still 1 and requests remain on udc_ep[i].queue; then
   crg_udc_reset() clears udc_ep_ptr->usb_ep.enabled = 0 for all endpoints
   before calling crg_udc_ep_nuke() (which drops udc_lock per request). If a
   function driver concurrently calls usb_ep_disable() on a higher-numbered
   endpoint while an earlier endpoint is being nuked (or during the SWRST
   poll), usb_ep_disable() returns immediately without draining that
   endpoint's queue; when the function driver then calls
   usb_ep_free_request(), the freed request remains on udc_ep[i].queue and is
   dereferenced by crg_udc_ep_nuke().

Can reset execution and endpoint teardown be serialized so concurrent callers
wait for the reset and request givebacks to complete?

[1] https://lore.kernel.org/r/20261020-axiado-ax3000-usb-device-controller-v3-6-3012ecfc0a93@axiado.com
[2] https://lore.kernel.org/r/20261020-axiado-ax3000-usb-device-controller-v3-6-3012ecfc0a93@axiado.com
[3] https://lore.kernel.org/r/20261020-axiado-ax3000-usb-device-controller-v3-6-3012ecfc0a93@axiado.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030-axiado-ax3000-usb-device-controller-v4-0-c5267b36a777@axiado.com?part=6

  reply	other threads:[~2026-10-30  8:46 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-30  8:10 [PATCH v4 0/6] Axiado AX3000 and AX3005 USB Device Controller (UDC) Petar Stepanovic
2026-10-30  8:10 ` [PATCH v4 1/6] dt-bindings: usb: gadget: udc: add Axiado AX3000 and AX3005 UDC Petar Stepanovic
2026-10-30  8:14   ` sashiko-bot
2026-10-30  8:10 ` [PATCH v4 2/6] usb: gadget: udc: crg: Add hardware definitions and structs Petar Stepanovic
2026-10-30  8:21   ` sashiko-bot
2026-10-30  8:10 ` [PATCH v4 3/6] usb: gadget: udc: crg: Add ring and transfer management Petar Stepanovic
2026-10-30  8:29   ` sashiko-bot
2026-10-30  8:10 ` [PATCH v4 4/6] usb: gadget: udc: crg: Add endpoint 0 handling Petar Stepanovic
2026-10-30  8:40   ` sashiko-bot
2026-10-30  8:10 ` [PATCH v4 5/6] usb: gadget: udc: crg: Add gadget and endpoint operations Petar Stepanovic
2026-10-30  8:50   ` sashiko-bot
2026-10-30  8:10 ` [PATCH v4 6/6] usb: gadget: udc: crg: Add core driver and build integration Petar Stepanovic
2026-10-30  8:46   ` sashiko-bot [this message]
2026-10-30  9:34   ` Philipp Zabel

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=sashiko-outbox-190713@kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=pstepanovic@axiado.com \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.