On Fri, Nov 03, 2023 at 03:34:52PM +0530, Krishna Kurapati PSSNV wrote:
On 10/24/2023 12:26 PM, Johan Hovold wrote:
On Mon, Oct 23, 2023 at 10:42:31PM +0530, Krishna Kurapati PSSNV wrote:
On 10/23/2023 7:37 PM, Johan Hovold wrote:
Right. And I assume there are hs_phy_irqs also for the first two USB
controllers on sc8280xp?
There are, I can dig through and find out. Atleast in downstream I
don't
see any use of them.
Yes, please do post how these are wired as well for completeness.
Did you find these two interrupts as well?
Regarding the points of discussion we had last week on [1], here are
some clarifications:
1. We do have hs_phy_irq 1/2/3/4 for tertiary port of Sc8280 as
mentioned. Why do we need them and would we use it in multiport
targets ?
DPSE and DMSE are single ended line state of DP and DM lines. The DP
line and DM line stay in steady High or Low during suspend and they flip
when there is a RESUME or REMOTE WAKE. This is what we do/check in
dwc3_qcom_enable_interrupts call for dp/dm irq's based on usb2_speed.
Right, this bit is clear.
Initially in QUSB2 targets, the interrupts were enabled and configured
in phy and the wakeup was interrupt was read on hs_phy_irq vector - [2].
In that case, we modify DP/DM interrupts in phy registers, specifically
QUSB2PHY_INTR_CTRL and when wakeup signal comes in, hs_phy_irq is
triggered. But in femto targets, this is done via DP/DM interrupts and
there is no use of hs_phy_irq. Even hw folks confirmed they dont use
hs_ph_irq in femto phy targets.
Ok, thanks for pointing to QUSB2. The same mechanism is apparently used
in phy-qcom-usb-hs-28nm.c as well (even if the dtsi currently does not
define the wakeup interrupts).
Furthermore, that implementation is broken and has never worked due to
another half-arsed, incomplete Qualcomm implementation. Specifically, no
one is changing the PHY mode based on the current speed before suspend
as commits like
3b3cd24ae61b ("phy: Add USB speed related PHY modes")
and
891a96f65ac3 ("phy: qcom-qusb2: Add support for runtime PM")
depend on. Guess I should go revert that mess too...
As an experiment, I tried to test wakeup by pressing buttons on
connected keyboard when in suspend state or connecting/disconnecting
keyboard in suspended state on different ports and only see dp/dm IRQ's
getting fired although we register for hs_phy_irq as well:
/ # cat /proc/interrupts |grep phy_
171: 1 0 0 0 0 0 0 0 PDC 127 Edge dp_hs_phy_1
172: 2 0 0 0 0 0 0 0 PDC 126 Edge dm_hs_phy_1
173: 3 0 0 0 0 0 0 0 PDC 129 Edge dp_hs_phy_2
174: 4 0 0 0 0 0 0 0 PDC 128 Edge dm_hs_phy_2
175: 0 0 0 0 0 0 0 0 PDC 131 Edge dp_hs_phy_3
176: 2 0 0 0 0 0 0 0 PDC 130 Edge dm_hs_phy_3
177: 2 0 0 0 0 0 0 0 PDC 133 Edge dp_hs_phy_4
178: 5 0 0 0 0 0 0 0 PDC 132 Edge dm_hs_phy_4
179: 0 0 0 0 0 0 0 0 PDC 16 Level ss_phy_1
180: 0 0 0 0 0 0 0 0 PDC 17 Level ss_phy_2
181: 0 0 0 0 0 0 0 0 GICv3 163 Level hs_phy_1
182: 0 0 0 0 0 0 0 0 GICv3 168 Level hs_phy_2
183: 0 0 0 0 0 0 0 0 GICv3 892 Level hs_phy_3
184: 0 0 0 0 0 0 0 0 GICv3 891 Level hs_phy_4
Yes, but that doesn't really say much since you never enable the hs_phy
interrupt in the PHY on suspend.
- qcom,ipq4019-dwc3
- qcom,ipq6018-dwc3
- qcom,ipq8064-dwc3
- qcom,ipq8074-dwc3
- qcom,msm8994-dwc3
- qcom,qcs404-dwc3
- qcom,sc7180-dwc3
- qcom,sc7280-dwc3
- qcom,sdm670-dwc3
- qcom,sdm845-dwc3
- qcom,sdx55-dwc3
- qcom,sdx65-dwc3
- qcom,sm4250-dwc3
- qcom,sm6125-dwc3
- qcom,sm6350-dwc3
- qcom,sm8150-dwc3
- qcom,sm8250-dwc3
- qcom,sm8350-dwc3
- qcom,sm8450-dwc3
- qcom,sm8550-dwc3
Some of those use QUSB2 PHYs and some use "femto" PHYs.
> And this comes from Qualcomm through commits like:
0b766e7fe5a2 ("arm64: dts: qcom: sc7180: Add USB related nodes")
bb9efa59c665 ("arm64: dts: qcom: sc7280: Add USB related nodes")
3. ctrl_irq[1] usage:
This is a feature of SNPS controller, not qcom glue wrapper, and is
present on all targets (non-QC as well probably). As mentioned before on
[3], this is used for HW acceleration.
In host mode, XHCI spec does allow for multiple interrupters when
multiple event rings are used. A possible usage is multiple execution
environments something like what we are doing on mobile with ADSP audio
offload [4]. Another possibility could be some of virtualization where
host/hyp would manage the first interrupter and could allow a guest to
operate only with the second (though current design does not go far
enough to offer true isolation for real VM type workloads). The
additional interrupts (ones other than ctrl_irq[0]) are either for
virtualization use cases, or for our various “hw offload” features. In
device mode, these are used for offloading tethering functionality to
IPA FW.
Ok, thanks for clarifying what you meant by "HW acceleration".
Since the DeviceTree passed to the OS, should describe the hardware to
the OS, and should represent the hardware from the point-of-view of the
OS, adding one interrupt (ctrl_irq[0]) might be sufficient as Linux
would not use the other interrupts.
I've only skimmed the virtualisation bits in xHCI spec, but it seems
Linux as VMM would still be involved in assigning these interrupts to
VMs.