On 8/10/2020 5:13 AM, Felipe Balbi wrote: > > Hi, > > Wesley Cheng <wcheng@xxxxxxxxxxxxxx> writes: >> @@ -190,6 +195,73 @@ static int dwc3_qcom_register_extcon(struct dwc3_qcom *qcom) >> return 0; >> } >> >> +static int dwc3_qcom_usb_role_switch_set(struct usb_role_switch *sw, >> + enum usb_role role) >> +{ >> + struct dwc3_qcom *qcom = usb_role_switch_get_drvdata(sw); >> + struct fwnode_handle *child; >> + bool enable = false; >> + >> + if (!qcom->dwc3_drd_sw) { >> + child = device_get_next_child_node(qcom->dev, NULL); >> + if (child) { >> + qcom->dwc3_drd_sw = usb_role_switch_find_by_fwnode(child); >> + fwnode_handle_put(child); >> + if (IS_ERR(qcom->dwc3_drd_sw)) { >> + qcom->dwc3_drd_sw = NULL; >> + return 0; >> + } >> + } >> + } >> + >> + usb_role_switch_set_role(qcom->dwc3_drd_sw, role); > > why is this done at the glue layer instead of core.c? > Hi Felipe, Thanks for the feedback. So the DWC3 DRD driver already registers a role switch device for receiving external events. However, the DWC3 glue (dwc3-qcom) needs to also know of the role changes, so that it can set the override bits accordingly in the controller. I've seen a few implementations, ie using a notifier block to notify the glue of these events, but that placed a dependency on the DWC3 core being available to the DWC3 glue at probe time. If the DWC3 core was not available at that time, the dwc3-qcom driver will finish its probing routine, and since the notifier was never registered, the role change events would not be received. By registering another role switch device in the DWC3 glue, this gives us a place to attempt initializing a channel w/ the DWC3 core if it wasn't ready during probe(). For example... usb_conn_detect_cable(role=USB_ROLE_DEVICE) -->usb_role_switch_set_role(sw=dwc3-qcom) -->dwc3_qcom_usb_role_switch_set() -- IF DWC3 core role switch available -->usb_role_switch_set_role(sw=drd) -- ELSE --> do nothing. So basically, the goal is to just propagate the role change event down to the DWC3 core, while breaking the dependency of it being available at probe. >> + if (role == USB_ROLE_DEVICE) >> + enable = true; >> + else >> + enable = false; >> + >> + qcom->mode = (role == USB_ROLE_HOST) ? USB_DR_MODE_HOST : >> + USB_DR_MODE_PERIPHERAL; >> + dwc3_qcom_vbus_overrride_enable(qcom, enable); > > could you add a patch fixing this typo? > Sure, I'll submit a separate patch to remove that extra 'r' Thanks Wesley -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project