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

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