Enable MPM interrupt controller #153
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The wakeirq_map data is taken from drivers/irqchip/qcom/mpm-8953.c in the fairphone-fp3 downstream kernel. Link: https://code.fairphone.com/projects/fairphone-3/gpl.html Signed-off-by: Bert Karwatzki <spasswolf@web.de>
The MPM hardware is accessible from the ARM CPUs through a shared memory
region (RPM MSG RAM) which is also concurrently accessed by other kinds of
cores on the system like modem, ADSP etc.
Modeling this relation in a (somewhat) sane manner in the device tree
requires to
- either present the MPM as a child of said memory region, which
makes little sense, as a mapped memory carveout is not a bus.
- define nodes which bleed their register spaces into one another
- or passing their slice of the MSG RAM through a property
Go with the third option and add a way to map a region passed through the
"qcom,rpm-msg-ram" property as register space for the MPM interrupt
controller.
The current way of using 'reg' is preserved for backwards compatibility
reasons.
[ tglx: Massaged changelog ]
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Acked-by: Shawn Guo <shawn.guo@linaro.org>
Link: https://lore.kernel.org/r/20230328-topic-msgram_mpm-v7-2-6ee2bfeaac2c@linaro.org
This adds a devicetree node for the mpm interrupt controller and a node for the sram needed by the controller. The interrupt mapping data is taken from the fairphone-fp3 downstream kernel (drivers/irqchip/qcom/mpm-8953.c) and the mpm node for msm8996 was used as a template. Link: https://code.fairphone.com/projects/fairphone-3/gpl.html Link: https://lore.kernel.org/r/20231215-topic-mpm_dt-v1-2-c6636fc75ce3@linaro.org Signed-off-by: Bert Karwatzki <spasswolf@web.de>
|
Cherry-picked the ipa_legacy patches on top of this branch, no luck so far: Edit: For testing purposes I pulled in the old ipa v2 patches (which have been rebased to 6.6.0 in my msm8953_v6.6_2 branch where they are working) and got a similar result: Edit2: So it seems that doe to some issue ipa_probe is not even called here. |
|
Or you get -EPROBE_DEFER, if you want verbose messages CONFIG_DEBUG_DRIVER can be quite useful |
|
No, I inserted a printk at the top of ipa_probe which does not show up in dmesg. Trying CONFIG_DEBUG_DRIVER next. Tested with CONFIG_DEBUG_DRIVER=y and ipa legacy patch, the probe deferral seems to be caused by cpr: There seems to be a problem with the cpr init here ... EDIT: No cpr init seems to work, but the messages did not show up because of the overly verbose CONFIG_DEBUG_DRIVER. |
|
The probe deferral is caused by this part of device_links_check_supplier: I monitored this in the branch where IPA is working and found out that in that case the supplier is NULL: |
|
When I try to work around this with |
|
RPM interconnects on 6.6.0+ take extra cell. Did you adjust ipa node? |
|
No, I just pulled the patch from the 6.5 branch. Thank you, this is most likely the solution. |
|
Adjusted the interconnects in the ipa node to: ipa is running now (without fw_devlink=permissive): Should some of the _TAGS be other tags than RPM_ACTIVE_TAG? |
|
It's still not possible to wake the phone from suspend by calling it. |
barni2000
reviewed
Sep 28, 2024
| #power-domain-cells = <0>; | ||
| interrupt-parent = <&intc>; | ||
| qcom,mpm-pin-count = <96>; | ||
| qcom,mpm-pin-map = <2 184>, /* tsens_upper_lower_int */ |
There was a problem hiding this comment.
It seems it is not matching with this https://git.codelinaro.org/clo/la/kernel/msm-4.9/-/blob/LA.UM.9.6.2.r1-04800-89xx.0/drivers/irqchip/qcom/mpm-8953.c#L16
There was a problem hiding this comment.
This offset IS confusing, but this is due to the the different mpm drivers in mainline and downstream kernel. The downstream kernel routes every interrupt through the mpm.
The irq numbers in mpm-pin-map should differ by 32 due to this, I think my template for this was
commit 09896da07315cce07b019ab00750c8a57e1b53a3
Author: Konrad Dybcio <konradybcio@kernel.org>
Date: Fri Dec 15 01:01:09 2023 +0100
arm64: dts: qcom: msm8996: Hook up MPM
Wire up MPM and the interrupts it provides.
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Link: https://lore.kernel.org/r/20231215-topic-mpm_dt-v1-2-c6636fc75ce3@linaro.org
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
This has since then been reverted
commit 4f423c4cbe26d79d8974936eb01e0d6574c5d2ac
Author: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Date: Wed Feb 21 01:07:21 2024 +0200
Revert "arm64: dts: qcom: msm8996: Hook up MPM"
Commit 09896da07315 ("arm64: dts: qcom: msm8996: Hook up MPM") has
hooked up the MPM irq chip on the MSM8996 platform. However this causes
my Dragonboard 820c crash during bootup (usually when probing IOMMUs).
Revert the offending commit for now. Quick debug shows that making
tlmm's wakeup-parent point to the MPM is enough to trigger the crash.
Fixes: 09896da07315 ("arm64: dts: qcom: msm8996: Hook up MPM")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Link: https://lore.kernel.org/r/20240221-msm8996-revert-mpm-v1-1-cdca9e30c9b4@linaro.org
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
I think I had the mentioned problem, too, (crash on boot when using mpm as tlmm's wakeup-parent) but solved it (as in avoiding the crash, wakeup is still not working). I will look into it but first have to rebase these to the newest branch, I havent't booted my FP3 into debian for a long time.
There was a problem hiding this comment.
I tried to find an example (i.e. an android kernel for msm8996 where the wakegic is used in the devicetree) but couldn't find one, so I tried to use the data from mpm-8953.c in my devicetree without shifting:
qcom,mpm-pin-map = <2 216>, /* tsens_upper_lower_int */
<37 252>, /* qmp_usb3_lfps_rxterm_irq -> ss_phy_irq */
<49 168>, /* qusb2phy_dpse_hv -> hs_phy_irq*/
<53 104>, /* mdss_irq */
<58 168>, /* qusb2phy_dmse_hv -> hs_phy_irq*/
<88 222>; /* ee0_krait_hlos_spmi_periph_irq */
and with this the kernel hangs when booting. I take this is a hint that the shift is correct.
barni2000
reviewed
Sep 28, 2024
| interrupts = <GIC_SPI 171 IRQ_TYPE_EDGE_RISING>; | ||
| mboxes = <&apcs 1>; | ||
| interrupt-controller; | ||
| #interrupt-cells = <2>; |
There was a problem hiding this comment.
Suggested change
| #interrupt-cells = <2>; | |
| #interrupt-cells = <3>; |
There was a problem hiding this comment.
But i am not totally sure because of this https://git.codelinaro.org/clo/la/kernel/msm-4.9/-/blob/LA.UM.9.6.2.r1-04800-89xx.0/arch/arm64/boot/dts/qcom/msm8953.dtsi#L275
There was a problem hiding this comment.
I think the downstream mpm controller "#interrupt-cells = <3>" only because it is used as interrupt-parent of the whole device-tree. In the examples in the mainline devicetree "qcom,mpm" is used with "#interrupt-cells = <2>", while the interrupt controller uses 3 cells (in sm6375.dtsi, qcm2290.dtsi).
barni2000
reviewed
Sep 28, 2024
Co-authored-by: Barnabás Czémán <barni2000@users.noreply.github.com>
This isn't because of MPM. MPM is responsible for deep sleep state which is hard to enter and will need a lot of changes. |
|
I've noticed that pressing the power button on my fairphone-fp3 (with this ms8953 kernel and debian stable(bookworm)) only suspends to idle (s2idle). I guess we need suspend-to-ram for the mpm do the waking-up. |
|
I was able to get working wakeup for calls (sms not tested, yet) with this (on top of 6.11.1/main from here). |
|
That's pretty awesome! :) |
|
This has to be enabled in sysfs like this (remoteproc0 can also be remoteproc{1,2} here, this seems to change without reason) @z3ntu: As you have acccess to a lot of different phone models, can you check if the qrtr ports for voice and messaging are universal using qrtr-lookup? Also, this is not the way the device is woken up in the downstream kernel, there the smd-edge interrupts do not have the flag IRQF_NO_SUSPEND. |
There is in |
|
Ah, thanks I had that mixed up. |
|
Receiving SMS works, too. |
|
It should be definitely sent to upstream (at least pinctrl part). |
|
The working wakeup for calls and SMS is done without mpm, it's just the qrtr callback called by the smd interrupt handler which is waking up the phone from s2idle. |
|
The qrtr ports are probably not universal, qrtr-lookup receives the port values from an AF_QIPCRTR socket (in qrtr-1.1/src/lookup.c). If they are indeed not universal the ports needed for wakeup could be specified in the devicetree, perhaps as properties of the smd-edge for the modem. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This enables the MPM interrupt controller which is supposed to be able to wake up the system from suspend. Next I'll put the IPA v2 patches on top of this to see if it's possible to receive SMS when suspended.