Re: [PATCH v9 4/8] usb: misc: qcom_eud: add per-path role switch support

[Konrad Dybcio]

On 10/17/26 3:14 AM, Elson Serrao wrote:
> 
> 
> On 10/15/2026 7:44 AM, Konrad Dybcio wrote:
>> On 9/29/26 11:35 PM, Elson Serrao wrote:
>>> The EUD hardware can support multiple High-Speed USB paths, each connected
>>> to different USB controllers. The current implementation uses a single
>>> chip-level role switch, which cannot properly handle multi-path
>>> configurations where each path needs independent role management. Since
>>> EUD is physically present between the USB connector and the controller,
>>> it should also relay the role change requests from the connector.
>>>
>>> Restructure the driver to support per-path role switches and remove the
>>> chip-level role switch. Additionally, as EUD need not modify the USB
>>> role upon enabling, remove the unnecessary role switch call from
>>> enable_eud().
>>>
>>> Reviewed-by: Peter Chen <peter.chen@xxxxxxxxxx>
>>> Signed-off-by: Elson Serrao <elson.serrao@xxxxxxxxxxxxxxxx>
>>> ---
>>
>> [...]
>>
>>> -static void eud_role_switch_release(void *data)
>>> +static int eud_role_switch_set(struct usb_role_switch *sw, enum usb_role role)
>>>  {
>>> -	struct eud_chip *chip = data;
>>> +	struct eud_path *path = usb_role_switch_get_drvdata(sw);
>>> +	int ret;
>>> +
>>> +	/* curr_role tracks the role from EUD's point of view */
>>> +	path->curr_role = role;
>>
>> This should be assigned after the fallible call below
>>
> My thinking here was that `curr_role` should track the role requested by
> TCPM, rather than the outcome of the downstream controller role switch.
> 
> When TCPM requests `USB_ROLE_DEVICE` (e.g. cable insertion), EUD should be
> enabled regardless of whether `usb_role_switch_set_role()` eventually
> succeeds or fails. EUD access is tied to the requested device-mode
> operation, not to the controller's ability to complete the transition.
> Likewise, when the port exits `USB_ROLE_DEVICE`, EUD should be disabled
> irrespective of the controller's state, since device-mode EUD access is no
> longer needed.
> 
> If `curr_role` is updated based on the controller's outcome, EUD state and
> `curr_role` can become inconsistent. For example, TCPM may request
> `USB_ROLE_DEVICE` and we enable_eud() as per Patch6, while the downstream
> controller subsequently fails to switch roles. In that case EUD remains
> enabled, but `curr_role` would still be `USB_ROLE_NONE` (see also Sashiko's
> observation with this approach 
> https://lore.kernel.org/all/20260811215406.7FD161F00A3A@xxxxxxxxxxxxxxx/).
> 
> We could keep them synchronized by rolling back the EUD state when the
> controller role switch fails. However, I don't see a strong reason to tie
> EUD state to the controller outcome. EUD is intended to operate
> independently of the controller in device mode. Preserving EUD access in
> this scenario also provides a means to investigate why the downstream
> controller role switch failed.

Okay, I think we can tackle this if any issues show up in the field.
Otherwise, I think it's high time to get this merged!

Konrad