Messages in this thread | | | Date | Sat, 4 Nov 2023 22:32:55 +0530 | Subject | Re: [RFC 2/8] usb: dwc3: core: Register vendor hooks for dwc3-qcom | From | Krishna Kurapati PSSNV <> |
| |
>>> Hi Bryan, >>> >>>> What happens to this code if you >>>> >>>> static int count; >>>> >>>> 1. sleep in dwc3_probe for 10 milliseconds >>>> 2. return -EPROBE_DEFER >>>> 3. if count++ < 5 goto 1 >>>> >>>> i.e. if we simulate say waiting on a PHY driver to probe in >>>> dwc3_probe() >>>> >>> The vendor hooks are used in __dwc3_set_mode and role_switch_set >>> calls in core and drd files respectively. These are invoked only if >>> we are OTG capable. The drd_work is initialized in core_init_mode >>> which is called at the end of dwc3_probe. If dwc3_probe fails and >>> gets deferred before that, none of the vendor hooks will be fired and >>> dwc3_qcom_probe is also deferred. >>> >>> However I see that if core_init_mode fails (the cleanup is already >>> done in drd to prevent set_role from getting invoked already), I >>> need to cleanup vendor hooks in error path of dwc3_probe(). >>> >>>> and what happens if we introduce a 100 millsecond sleep into >>>> dwc3_qcom_probe() - and run a fake disconnect event from >>>> dwc3_qcom_probe_core() directly ? >>>> >>>> In other words if make it that dwc3_probe() completes and struct >>>> dwc3_glue_ops->notify_cable_disconnect() fires prior to >>>> dwc3_qcom_probe_core() completing ? >>>> >>>> i.e. I don't immediately see how you've solved the probe() >>>> completion race condition here. >>>> >>> Just wanted to understand the situation clearly. Is this the sequence >>> you are referring to ? >>> >>> 1. dwc3_probe is successful and role switch is registered properly. >>> 2. added delay after dwc3_qcom_probe_core and before interconnect_init >>> 3. Between this delay, we got a disconnect notificiation from glink >>> 4. We are clearing the qscratch reg in case of device mode and >>> un-registering notifier in case of host mode. >>> >>> If so, firstly I don't see any issue if we process disconnect event >>> before qcom probe is complete. If we reached this stage, the >>> clocks/gdsc is definitely ON and register accesses are good to go. >>> >>> If we are in host mode at this point, we would just unregister to >>> usb-core notifier and mark last busy. If we are in device mode, we >>> would just clear the hs_phy_ctrl reg of qscratch. After the 100ms >>> delay you mentioned we would call dwc3_remove anyways and cleanup the >>> vendor hooks. But is the concern here that, what if we enter >>> runtime_suspend at this point ? >>> >> >> Just to clarify one more thing. The probe completion requirement came >> in because, before the device tree was flattened, dwc3-qcom and core >> are two different platform devices. And if the dwc3 core device probe >> got deferred, dwc3-qcom probe still gets successfully completed. The >> glue would never know when to register vendor hook callbacks to >> dwc3-core as it would never know when the core probe was completed. >> >> That is the reason we wanted to find out accurate point where core >> probe is done to ensure we can properly register these callbacks. > > Are you saying to you require/rely on both of these series being applied > first ? > > [1]: > https://lore.kernel.org/all/af60c05b-4a0f-51b8-486a-1fc601602515@quicinc.com/ > [2]: > https://lore.kernel.org/all/20231016-dwc3-refactor-v1-0-ab4a84165470@quicinc.com/ > > Must be, nothing applies for me in this series.
The first one is not a patch. It is just a discussion thread I started to get community's opinion before on disconnect interrupt handling. The current series is based on top of [2] made by Bjorn (as you already found out) and as I mentioned in cover letter of my series.
Regards, Krishna,
| |