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

[Elson Serrao <elson.serrao@oss.qualcomm.com>]

On 9/29/2026 2:47 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] usb: qcom_eud: missing hardware and state rollback on role switch failure
> --
> 
> commit ac5ffb8b40f5366751bb4683118798fac6f6bb75
> Author: Elson Serrao <elson.serrao@oss.qualcomm.com>
> 
> usb: misc: qcom_eud: add role-based EUD control
> 
> EUD functions by presenting itself as a USB device to the host PC and is
> therefore only applicable when the port operates in device role. This patch
> addresses this by treating EUD as usable only in device role, enabling or
> disabling it as the port enters or leaves device role.
> 
>> diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
>> index e227d05cfecd4..5a21a2dd0b0cd 100644
>> --- a/drivers/usb/misc/qcom_eud.c
>> +++ b/drivers/usb/misc/qcom_eud.c
> 
> [ ... ]
> 
>> @@ -306,7 +331,30 @@ static irqreturn_t handle_eud_irq_thread(int irq, void *data)
>>  static int eud_role_switch_set(struct usb_role_switch *sw, enum usb_role role)

[...]

>>  	/* curr_role tracks the role from EUD's point of view */
>>  	path->curr_role = role;
>>
>>  	/* Forward the role request to the USB controller */
>>  	ret = usb_role_switch_set_role(path->controller_sw, role);
>>  	if (ret) {
>>  		dev_err(path->chip->dev, "failed to set role %s for port %u: %d\n",
>>  			usb_role_string(role), path->num, ret);
>>  		return ret;
>>  	}
> 
> [Severity: High]
> Does this error path leave the hardware and state permanently desynchronized
> if the downstream controller switch fails?
> 
> If a role switch to USB_ROLE_DEVICE is requested, enable_eud() succeeds and
> path->curr_role is updated.  Then usb_role_switch_set_role() is called to
> forward the switch to the controller.
> 
> If this downstream switch fails and returns an error, the function returns
> early without reverting path->curr_role or powering down the EUD via
> disable_eud().
> 
> As noted in the commit message, enabling EUD in host role can cause the USB
> controller to misbehave. Could this sequence leave the EUD hardware enabled
> while the underlying USB controller remains in its previous role?
> 
This is intentional. As documented path->curr_role tracks the role requested
for the path, so EUD follows that request rather than the controller's
outcome. The host-role concern applies when the port is in host role.
Here device role was requested, so the link partner is a host, which is
exactly the setup EUD is meant for. Rolling back would not fix the controller,
which is already in an error state, and the failure is logged and returned.
Since EUD operates independently of the controller, keeping it enabled
also preserves debug access for investigating such a failure.

Thanks
Elson