Re: [PATCH v8 6/8] usb: misc: qcom_eud: add role-based EUD control

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Mixing goto-based exit logic with scope-based guard() cleanup in `handle_eud_irq_thread`.
--

commit 5bca652d5a5a406c9aee75922062a73bc36dc72b
Author: Elson Serrao <elson.serrao@oss.qualcomm.com>

usb: misc: qcom_eud: add role-based EUD control

This commit updates the QCOM EUD driver to only operate when the port is in
device role, preserving hardware resources and preventing controller misbehavior
in host role. It uses a mutex to serialize state management across sysfs, IRQ,
and role switch paths.

> diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
> --- a/drivers/usb/misc/qcom_eud.c
> +++ b/drivers/usb/misc/qcom_eud.c
[ ... ]
> @@ -285,8 +306,13 @@ static irqreturn_t handle_eud_irq_thread(int irq, void *data)
>  	struct eud_path *path;
>  	int ret;
>  
> +	guard(mutex)(&chip->state_lock);
> +
>  	path = chip->paths[chip->port_idx];
>  
> +	if (!chip->enabled || path->curr_role != USB_ROLE_DEVICE)
> +		goto out;
> +

[Severity: Low]
Does this code mix a goto-based exit label (out) with a scope-based
guard(mutex) in handle_eud_irq_thread()?

The Cleanup and Guard Subsystem architectural guidelines explicitly ban mixing
these patterns in the same function to prevent confusing ownership semantics.
While this might not lead to an immediate runtime failure here since the goto
just skips to clearing an interrupt and returning, the subsystem documentation
states that for a given routine, all resources must either be converted to
scope-based cleanup, or none of them should be.

>  	if (chip->usb_attached)
>  		ret = usb_role_switch_set_role(path->controller_sw, USB_ROLE_DEVICE);
>  	else
> @@ -294,6 +320,7 @@ static irqreturn_t handle_eud_irq_thread(int irq, void *data)
>  	if (ret)
>  		dev_err(chip->dev, "failed to set role switch\n");
>  
> +out:
>  	/* set and clear vbus_int_clr[0] to clear interrupt */
>  	writel(BIT(0), chip->base + EUD_REG_VBUS_INT_CLR);
>  	writel(0, chip->base + EUD_REG_VBUS_INT_CLR);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912225301.3396556-1-elson.serrao@oss.qualcomm.com?part=6