Messages in this thread | | | Date | Sat, 4 Nov 2023 16:00:52 +0000 | Subject | Re: [RFC 2/8] usb: dwc3: core: Register vendor hooks for dwc3-qcom | From | Bryan O'Donoghue <> |
| |
On 03/11/2023 18:49, Krishna Kurapati PSSNV wrote: > > > On 11/4/2023 12:15 AM, Krishna Kurapati PSSNV wrote: >> >> >> On 11/3/2023 8:44 PM, Bryan O'Donoghue wrote: >>> On 17/10/2023 14:18, Krishna Kurapati wrote: >>>> >>>> The following are the requirements aimed in this implementation: >>>> >>>> 1. When enum in device mode, Glue/core must stay active. >>>> >>>> 2. When cable is connected but UDC is not written yet, then glue/core >>>> must be suspended. >>>> >>>> 3. Upon removing cable in device mode, the disconnect event must be >>>> generated and unblock runtime suspend for dwc3 core. >>>> >>>> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> >>> >> >> 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.
| |