[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cf553cd8-45f8-4a61-b016-69e7a80eee9f@linaro.org>
Date: Sat, 4 Nov 2023 16:00:52 +0000
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: Krishna Kurapati PSSNV <quic_kriskura@...cinc.com>,
Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Conor Dooley <conor+dt@...nel.org>
Cc: linux-usb@...r.kernel.org,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
linux-kernel@...r.kernel.org,
Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
Rob Herring <robh+dt@...nel.org>,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
quic_ppratap@...cinc.com, quic_jackp@...cinc.com,
quic_wcheng@...cinc.com, Andy Gross <agross@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Philipp Zabel <p.zabel@...gutronix.de>
Subject: Re: [RFC 2/8] usb: dwc3: core: Register vendor hooks for dwc3-qcom
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@...cinc.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.
Powered by blists - more mailing lists