lkml.org 
[lkml]   [2023]   [Nov]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [RFC 2/8] usb: dwc3: core: Register vendor hooks for dwc3-qcom
From



>>> 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,

\
 
 \ /
  Last update: 2023-11-20 13:48    [W:0.223 / U:0.100 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site