mbox series

[RFC,0/8] Enable runtime suspend resume for QCOM devices

Message ID 20231017131851.8299-1-quic_kriskura@quicinc.com
Headers show
Series Enable runtime suspend resume for QCOM devices | expand

Message

Krishna Kurapati Oct. 17, 2023, 1:18 p.m. UTC
Currently for all dwc3 devices, runtime is forbidden in kernel code.
Although it can be configured from userspace, qualcomm devices using
role switching with dr_mode "otg" would need to modify glue specific
qscratch registers without which connection done/ disconnect events
wouldn't be generated in device mode thus blocking suspend entry.
Also when in host mode, xhci and roothubs needs to be configured
to use_autosuspend accrordingly. More information regarding disconnect
event generation on QC targets is in [1].

The series introduces vendor hooks for passing on notifications
from core to glue layers. Since these hooks are to be registered
by glue to core, glue needs to have some sort of indication that
core probe hasn't been deferred or core probe hasn't failed. So
this series banks upon Bjorn's flattened device tree implementation [2]
for dwc3 core and dwc3-qcom. Since these patches ensure that we use
core layer as a library and probe is invoked in a controlled fashion
from qcom glue layer, we can register vendor hooks in this probe call.

While in host mode, usb-core notifiers have been utilised to ensure
we use_autosuspend for connected devices and roothubs.

[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/

Tests done:
1. Enumeration in device mode:
After creating symlinks to ffs.adb and writing to UDC node, ADB is up and
working in a stable way.

2. When none is written to UDC, device enters suspend.

3. When cable is removed, cable disconnect notification comes and when
qscratch registers are cleared properly, it is generating disconnect event

4. Device enters suspend upon removing cable (host and device mode)

5. In host mode, when autosuspend and wakeup are enabled from userspace
via the following commands, device enters runtime suspend:

echo enabled > /sys/bus/platform/devices/xhci-hcd.0.auto/power/wakeup
echo auto > /sys/bus/platform/devices/xhci-hcd.0.auto/power/control
echo 5000 > /sys/bus/platform/devices/xhci-hcd.0.auto/usb1/1-1/power/autosuspend_delay_ms
echo enabled > /sys/bus/platform/devices/xhci-hcd.0.auto/usb1/1-1/power/wakeup
echo auto > /sys/bus/platform/devices/xhci-hcd.0.auto/usb1/1-1/power/control

6. Upon removing cable in host mode, setmode brings back usb to device
mode (which is default setting), it enters suspend as cable is still
disconnected

7. When in host mode, if we enter runtime suspend with wakeup enabled,
clicking on buttons of headset are resuming the controller.

Issues still present:
During bootup if UDC is not written before dwc3 is suspended, then device
enters suspend and although writing to UDC is resuming the device and
setting  run stop, controller is not generating events. At this stage if
a simple plug-in/plug-out is done, everything works fine.

Krishna Kurapati (8):
  dt-bindings: usb: qcom,dwc3: Add bindings to enable runtime
  usb: dwc3: core: Register vendor hooks for dwc3-qcom
  usb: dwc3: qcom: Enable autosuspend for host mode
  clk: qcom: gcc-sm8450: Keep usb30 prim gdsc on during runtime suspend
  arm64: dts: qcom: Flatten sm8450 usb device node
  arm: dts: qcom: Add pmic glink support for sm8450-qrd
  arm: dts: qcom: Enable runtime for SM8450 QRD
  usb: dwc3: core: Skip set_mode notification if cable is disconnected

 .../devicetree/bindings/usb/qcom,dwc3.yaml    |   5 +
 arch/arm64/boot/dts/qcom/sm8450-hdk.dts       |   3 -
 arch/arm64/boot/dts/qcom/sm8450-qrd.dts       |  48 ++++++-
 .../dts/qcom/sm8450-sony-xperia-nagara.dtsi   |   3 -
 arch/arm64/boot/dts/qcom/sm8450.dtsi          |  44 +++----
 drivers/clk/qcom/gcc-sm8450.c                 |   1 +
 drivers/usb/dwc3/core.c                       |  19 ++-
 drivers/usb/dwc3/core.h                       |  50 ++++++-
 drivers/usb/dwc3/drd.c                        |  13 ++
 drivers/usb/dwc3/dwc3-qcom.c                  | 122 +++++++++++++++++-
 10 files changed, 271 insertions(+), 37 deletions(-)

Comments

Krzysztof Kozlowski Oct. 17, 2023, 5:20 p.m. UTC | #1
On 17/10/2023 15:18, Krishna Kurapati wrote:
> Add Pmic Glink support for sm8450-qrd to facilitate passing
> of roe switch notifications generated by ADSP to dwc3 core
> via ucsi and pmic glink's.
> 
> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching.

> ---
>  arch/arm64/boot/dts/qcom/sm8450-qrd.dts | 46 ++++++++++++++++++++++++-
>  1 file changed, 45 insertions(+), 1 deletion(-)
> 

With subject fixes:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Krzysztof Kozlowski Oct. 17, 2023, 5:21 p.m. UTC | #2
On 17/10/2023 15:18, Krishna Kurapati wrote:
> Enable runtime and wakeup source for SM8450 QRD platform.
> 
> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sm8450-qrd.dts | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8450-qrd.dts b/arch/arm64/boot/dts/qcom/sm8450-qrd.dts
> index aec47e45284e..d3e8fe7a37ec 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450-qrd.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8450-qrd.dts
> @@ -490,6 +490,9 @@ &usb_1 {
>  
>  	dr_mode = "otg";
>  	usb-role-switch;
> +
> +	qcom,enable-rt;

NAK. Not a HW property.


Best regards,
Krzysztof
Konrad Dybcio Oct. 26, 2023, 7:41 p.m. UTC | #3
On 10/17/23 15:18, Krishna Kurapati wrote:
> Add Pmic Glink support for sm8450-qrd to facilitate passing
> of roe switch notifications generated by ADSP to dwc3 core
> via ucsi and pmic glink's.
> 
> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
> ---
No phy+redriver+dp configuration?

Konrad
Krishna Kurapati Oct. 27, 2023, 3:30 a.m. UTC | #4
On 10/27/2023 1:11 AM, Konrad Dybcio wrote:
> 
> 
> On 10/17/23 15:18, Krishna Kurapati wrote:
>> Add Pmic Glink support for sm8450-qrd to facilitate passing
>> of roe switch notifications generated by ADSP to dwc3 core
>> via ucsi and pmic glink's.
>>
>> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
>> ---
> No phy+redriver+dp configuration?
> 
Hi Konrad,

  Did you mean adding the following node:

         typec-mux@42 {
                 compatible = "fcs,fsa4480";
                 reg = <0x42>;

                 interrupts-extended = <&tlmm 2 IRQ_TYPE_LEVEL_LOW>;

                 vcc-supply = <&vreg_bob>;
                 mode-switch;
                 orientation-switch;

                 port {
                         fsa4480_sbu_mux: endpoint {
                                 remote-endpoint = <&pmic_glink_sbu>;
                         };
                 };
         };


and then adding port-2 for pmic_glink ?

Usually for role-switch the port-0/1 defined in this patch are 
sufficient. Also if I added it, I don't have a way to currently test it. 
So skipped this node. I will try and see if I can test it and add it if 
possible.

Regards,
Krishna,
Bryan O'Donoghue Nov. 3, 2023, 3:14 p.m. UTC | #5
On 17/10/2023 14:18, Krishna Kurapati wrote:
> Currently on QC targets, the conndone/disconnect events in device mode are
> generated by controller when software writes to QSCRATCH registers in qcom
> glue layer rather than the vbus line being routed to dwc3 core IP for it
> to recognize and generate these events.
> 
> We need to write '1' to  UTMI_OTG_VBUS_VALID bit of QSCRATCH_HS_PHY_CTRL
> register to generate a connection done event and "0" if we need to
> generate a disconnect event during cable removal or mode switch. Exactly
> what is done by "dwc3_qcom_vbus_override_enable" call in dwc3-qcom.
> 
> When the disconnect is not generated upon cable removal, the connected
> flag of dwc3 is left marked as "true" and it blocks runtime suspend.
> 
> The goal of these vendor hooks is to let the mode change and cable removal
> notifications from core reach the  glue layers so that glue can take
> necessary action.
> 
> Before flattening the device tree, glue driver is not sure when the core
> probe was completed as core probe can be deferred. In this case, glue is
> not sure when to register vendor hooks. So mandate enabling runtime only
> for flattened device node platforms so that glue can know when to register
> vendor hooks.
> 
> 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>

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()

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.

---
bod
Krishna Kurapati Nov. 3, 2023, 6:45 p.m. UTC | #6
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 ?

Regards,
Krishna,
Krishna Kurapati Nov. 3, 2023, 6:49 p.m. UTC | #7
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.

Regards,
Krishna,
Bryan O'Donoghue Nov. 4, 2023, 4 p.m. UTC | #8
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.
Krishna Kurapati Nov. 4, 2023, 5:02 p.m. UTC | #9
>>> 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,
Krishna Kurapati Nov. 7, 2023, 8:33 a.m. UTC | #10
On 11/4/2023 10:32 PM, Krishna Kurapati PSSNV wrote:
>>
>> 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.
> 

Hi Bryan,

   Are you able to apply the series after including Bjorn's patches ? 
Also can you confirm if the comments provided to your queries on [1] are 
proper and if you have any other comments w.r.t probe deferral.

[1]: 
https://lore.kernel.org/all/e700133b-58f7-4a4d-8e5c-0d04441b789b@linaro.org/

Regards,
Krishna,
Bryan O'Donoghue Nov. 7, 2023, 10:41 a.m. UTC | #11
On 07/11/2023 08:33, Krishna Kurapati PSSNV wrote:
> 
> 
> On 11/4/2023 10:32 PM, Krishna Kurapati PSSNV wrote:
>>>
>>> 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.
>>
> 
> Hi Bryan,
> 
>    Are you able to apply the series after including Bjorn's patches ? 
> Also can you confirm if the comments provided to your queries on [1] are 
> proper and if you have any other comments w.r.t probe deferral.
> 
> [1]: 
> https://lore.kernel.org/all/e700133b-58f7-4a4d-8e5c-0d04441b789b@linaro.org/
> 
> Regards,
> Krishna,

I wonder could you give a base SHA to apply the various series on ?

Your referenced precursor doesn't apply to usb-next

deckard@sagittarius-a:~/Development/qualcomm/qlt-kernel$ b4 shazam 
20231016-dwc3-refactor-v1-0-ab4a84165470@quicinc.com
Grabbing thread from 
lore.kernel.org/all/20231016-dwc3-refactor-v1-0-ab4a84165470@quicinc.com/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Analyzing 27 messages in the thread
Checking attestation on all messages, may take a moment...
---
   [PATCH 1/12] dt-bindings: usb: qcom,dwc3: Add qcom,sc8180x-dwc3
   [PATCH 2/12] usb: dwc3: qcom: Rename dwc3 platform_device reference
     + Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
   [PATCH 3/12] usb: dwc3: qcom: Merge resources from urs_usb device
   [PATCH 4/12] usb: dwc3: Expose core driver as library
   [PATCH 5/12] usb: dwc3: Override end of dwc3 memory resource
     + Acked-by: Konrad Dybcio <konrad.dybcio@linaro.org>
   [PATCH 6/12] usb: dwc3: qcom: Add dwc3 core reference in driver state
   [PATCH 7/12] usb: dwc3: qcom: Instantiate dwc3 core directly
   [PATCH 8/12] usb: dwc3: qcom: Inline the qscratch constants
     + Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
   [PATCH 9/12] dt-bindings: usb: qcom,dwc3: Rename to "glue"
     + Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
   [PATCH 10/12] dt-bindings: usb: qcom,dwc3: Introduce flattened 
qcom,dwc3 binding
   [PATCH 11/12] usb: dwc3: qcom: Flatten the Qualcomm dwc3 binding and 
implementation
   [PATCH 12/12] arm64: dts: qcom: sc8180x: flatten usb_sec node
   ---
   ✗ No key: ed25519/quic_bjorande@quicinc.com
   ---
   NOTE: install dkimpy for DKIM signature verification
---
Total patches: 12
---
  Base: using specified base-commit 4d0515b235dec789578d135a5db586b25c5870cb
Applying: dt-bindings: usb: qcom,dwc3: Add qcom,sc8180x-dwc3
Patch failed at 0001 dt-bindings: usb: qcom,dwc3: Add qcom,sc8180x-dwc3
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
error: patch failed: Documentation/devicetree/bindings/usb/qcom,dwc3.yaml:29
error: Documentation/devicetree/bindings/usb/qcom,dwc3.yaml: patch does 
not apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch
deckard@sagittarius-a:~/Development/qualcomm/qlt-kernel$ git diff
deckard@sagittarius-a:~/Development/qualcomm/qlt-kernel$ git am --skip
Applying: usb: dwc3: qcom: Rename dwc3 platform_device reference
error: patch failed: drivers/usb/dwc3/dwc3-qcom.c:67
error: drivers/usb/dwc3/dwc3-qcom.c: patch does not apply
Patch failed at 0002 usb: dwc3: qcom: Rename dwc3 platform_device reference
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
deckard@sagittarius-a:~/Development/qualcomm/qlt-kernel$ git am --skip
Applying: usb: dwc3: qcom: Merge resources from urs_usb device
error: patch failed: drivers/usb/dwc3/dwc3-qcom.c:68
error: drivers/usb/dwc3/dwc3-qcom.c: patch does not apply
Patch failed at 0003 usb: dwc3: qcom: Merge resources from urs_usb device
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
deckard@sagittarius-a:~/Development/qualcomm/qlt-kernel$ git am --skip
Applying: usb: dwc3: Expose core driver as library
error: patch failed: drivers/usb/dwc3/core.c:1876
error: drivers/usb/dwc3/core.c: patch does not apply
error: patch failed: drivers/usb/dwc3/core.h:1568
error: drivers/usb/dwc3/core.h: patch does not apply
Patch failed at 0004 usb: dwc3: Expose core driver as library
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
deckard@sagittarius-a:~/Development/qualcomm/qlt-kernel$ git am --skip
Applying: usb: dwc3: Override end of dwc3 memory resource
error: patch failed: drivers/usb/dwc3/core.c:1908
error: drivers/usb/dwc3/core.c: patch does not apply
Patch failed at 0005 usb: dwc3: Override end of dwc3 memory resource
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
deckard@sagittarius-a:~/Development/qualcomm/qlt-kernel$ git am --skip
Applying: usb: dwc3: qcom: Add dwc3 core reference in driver state
error: patch failed: drivers/usb/dwc3/dwc3-qcom.c:67
error: drivers/usb/dwc3/dwc3-qcom.c: patch does not apply
Patch failed at 0006 usb: dwc3: qcom: Add dwc3 core reference in driver 
state
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
deckard@sagittarius-a:~/Development/qualcomm/qlt-kernel$ git am --abort

---
bod
Bryan O'Donoghue Nov. 7, 2023, 10:55 a.m. UTC | #12
On 07/11/2023 10:41, Bryan O'Donoghue wrote:
> On 07/11/2023 08:33, Krishna Kurapati PSSNV wrote:
>>
>>
>> On 11/4/2023 10:32 PM, Krishna Kurapati PSSNV wrote:
>>>>
>>>> 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.
>>>
>>
>> Hi Bryan,
>>
>>    Are you able to apply the series after including Bjorn's patches ? 
>> Also can you confirm if the comments provided to your queries on [1] 
>> are proper and if you have any other comments w.r.t probe deferral.
>>
>> [1]: 
>> https://lore.kernel.org/all/e700133b-58f7-4a4d-8e5c-0d04441b789b@linaro.org/
>>
>> Regards,
>> Krishna,
> 
> I wonder could you give a base SHA to apply the various series on ?
> 
> Your referenced precursor doesn't apply to usb-next

Well now, that doesn't point where I thought it pointed usb-next/master 
is extremely old

  b3a9e3b9622ae - (HEAD -> usb-next-23-10-07-usb-glue-test, tag: 
v5.8-rc1, usb-next/master, origin/tracking-qcomlt-sm8150-gcc, 
linaro/tracking-qcomlt-sm8150-gcc, fecked-old, delete-this-branch2, 
delete-this-branch) Linux 5.8-rc1 (3 years, 5 months ago)

I want usb-next/main

*   d2f51b3516dad - (usb-next/usb-testing, usb-next/usb-next, 
usb-next/main) Merge tag 'rtc-6.7' of 
git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux (32 hours ago)

Everything applies there.

Anyway, your pointing to Bjorn's series answers my question re: 
sequencing of the probe.

---
bod
Krishna Kurapati Nov. 7, 2023, 11:02 a.m. UTC | #13
On 11/7/2023 4:25 PM, Bryan O'Donoghue wrote:
> On 07/11/2023 10:41, Bryan O'Donoghue wrote:
>> On 07/11/2023 08:33, Krishna Kurapati PSSNV wrote:
>>>
>>>
>>> On 11/4/2023 10:32 PM, Krishna Kurapati PSSNV wrote:
>>>>>
>>>>> 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.
>>>>
>>>
>>> Hi Bryan,
>>>
>>>    Are you able to apply the series after including Bjorn's patches ? 
>>> Also can you confirm if the comments provided to your queries on [1] 
>>> are proper and if you have any other comments w.r.t probe deferral.
>>>
>>> [1]: 
>>> https://lore.kernel.org/all/e700133b-58f7-4a4d-8e5c-0d04441b789b@linaro.org/
>>>
>>> Regards,
>>> Krishna,
>>
>> I wonder could you give a base SHA to apply the various series on ?
>>
>> Your referenced precursor doesn't apply to usb-next
> 
> Well now, that doesn't point where I thought it pointed usb-next/master 
> is extremely old
> 
>   b3a9e3b9622ae - (HEAD -> usb-next-23-10-07-usb-glue-test, tag: 
> v5.8-rc1, usb-next/master, origin/tracking-qcomlt-sm8150-gcc, 
> linaro/tracking-qcomlt-sm8150-gcc, fecked-old, delete-this-branch2, 
> delete-this-branch) Linux 5.8-rc1 (3 years, 5 months ago)
> 
> I want usb-next/main
> 
> *   d2f51b3516dad - (usb-next/usb-testing, usb-next/usb-next, 
> usb-next/main) Merge tag 'rtc-6.7' of 
> git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux (32 hours ago)
> 
> Everything applies there.

Hi Bryan,

   I should have mentioned that series is pushed on top of usb-next. 
Apologies.

> 
> Anyway, your pointing to Bjorn's series answers my question re: 
> sequencing of the probe.

Perfect. Thanks for the confirmation.

Regards,
Krishna,