Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IPA upstreaming #93

Closed
alikates opened this issue Apr 28, 2023 · 13 comments
Closed

IPA upstreaming #93

alikates opened this issue Apr 28, 2023 · 13 comments

Comments

@alikates
Copy link
Member

alikates commented Apr 28, 2023

Last time I rebased IPA patches was with kernel version 6.1 and it was half-working.
Let's try tu update them to kernel-next/6.3 with the objective of upstreaming.

@spasswolf
Copy link

spasswolf commented May 9, 2023

I just rebased these patches https://www.spinics.net/lists/kernel/msg4081927.html to linux-msm8953-mainline branch msm8953-6.1.0/main and was able to send and receive SMS on a fairphone3 (with a standard debian sid installed on a compact flash card). Calls are probably working, too, but sound isn't so testing this has to be postponed.
Here's a diff to commit b5b016f of msm8953-6.1.0/main.

@spasswolf
Copy link

@alikates
Copy link
Member Author

alikates commented May 9, 2023

Hi, thank you for your help. There's already a branch for 6.1 with all IPA patches (https://github.com/msm8953-mainline/linux/commits/msm8953-6.1.0/ipa). However some stuff didn't work properly and at that time I didn't have much time to debug it, so it would be nice to know what you changed to make it work.

The issue now is rebasing on top of 6.3 or even 6.4-rcX because there has been a big refactor. Luckily this refactor maybe benefits us in some parts of the driver because improves separation between the DMA driver and the actual IPA driver.

@spasswolf
Copy link

Just booted the 6.1.0/ipa branch on my fairphone3 and got a NULL pointer dereference
`
3.790627] ipa 7940000.ipa: required memory region 14 missing
[ 3.790960] ipa 7940000.ipa: required memory region 15 missing
[ 3.812088] ipa 7940000.ipa: IPA driver initialized
[ 3.812430] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008
[ 3.816612] Mem abort info:
[ 3.826241] ESR = 0x0000000096000004
[ 3.829458] EC = 0x25: DABT (current EL), IL = 32 bits
[ 3.833975] SET = 0, FnV = 0
[ 3.840118] EA = 0, S1PTW = 0
[ 3.843677] FSC = 0x04: level 0 translation fault
[ 3.847432] Data abort info:
[ 3.852975] ISV = 0, ISS = 0x00000004
[ 3.856792] CM = 0, WnR = 0
[ 3.861039] user pgtable: 4k pages, 48-bit VAs, pgdp=00000000186a4000
[ 3.864883] [0000000000000008] pgd=0000000000000000, p4d=0000000000000000
[ 3.872012] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
[ 3.879462] Modules linked in: ipa(+) qcom_common

`
This is good news as I had that too, this comes from mem->size being used when mem is NULL. This requires a few additional ipa->version checks.

@spasswolf
Copy link

spasswolf commented May 9, 2023

You were setting the wrong IPA_VERSION for IPA v2.6L. This patch fixes this (at least for me...) and enables the modem on the fairphone-fp3:

diff --git a/arch/arm64/boot/dts/qcom/sdm632-fairphone-fp3.dts b/arch/arm64/boot/dts/qcom/sdm632-fairphone-fp3.dts
index b9173607b1b1..5bc6c69c43bb 100644
--- a/arch/arm64/boot/dts/qcom/sdm632-fairphone-fp3.dts
+++ b/arch/arm64/boot/dts/qcom/sdm632-fairphone-fp3.dts
@@ -125,6 +125,7 @@ &lpass {
 };
 
 &modem {
+	status = "okay";
 	compatible = "qcom,msm8953-alt-mss-pil";
 
 	power-domains = <&rpmpd MSM8953_VDDCX>, <&rpmpd MSM8953_VDDMX>, <&rpmpd MSM8953_VDDMD>;
diff --git a/drivers/net/ipa/data/ipa_data-v2.6l.c b/drivers/net/ipa/data/ipa_data-v2.6l.c
index 2dda7b8e54ea..e35d8c3c3a85 100644
--- a/drivers/net/ipa/data/ipa_data-v2.6l.c
+++ b/drivers/net/ipa/data/ipa_data-v2.6l.c
@@ -224,7 +224,7 @@ static struct ipa_power_data ipa_power_data = {
 
 /* Configuration data for IPA v2.6l */
 const struct ipa_data ipa_data_v2_6l = {
-	.version	= IPA_VERSION_2_5,
+	.version	= IPA_VERSION_2_6L,
 	.endpoint_count	= ARRAY_SIZE(ipa_endpoint_data),
 	.endpoint_data	= ipa_endpoint_data,
 	.mem_data	= &ipa_mem_data,
diff --git a/drivers/net/ipa/ipa_mem.c b/drivers/net/ipa/ipa_mem.c
index 4023092182c7..d524d4b7fcbe 100644
--- a/drivers/net/ipa/ipa_mem.c
+++ b/drivers/net/ipa/ipa_mem.c
@@ -211,7 +211,8 @@ static bool ipa_mem_id_required(struct ipa *ipa, enum ipa_mem_id mem_id)
 		return true;
 	case IPA_MEM_MODEM_PROC_CTX:
 	case IPA_MEM_AP_PROC_CTX:
-		return ipa->version != IPA_VERSION_2_6L;
+		return ipa->version == IPA_VERSION_2_5 || ipa->version >= IPA_VERSION_3_0;
+		//return ipa->version != IPA_VERSION_2_6L;
 
 	case IPA_MEM_V4_FILTER_HASHED:
 	case IPA_MEM_V6_FILTER_HASHED:
@@ -458,7 +459,8 @@ int ipa_mem_zero_modem(struct ipa *ipa)
 	}
 
 	ipa_mem_zero_region_add(trans, IPA_MEM_MODEM_HEADER);
-	ipa_mem_zero_region_add(trans, IPA_MEM_MODEM_PROC_CTX);
+	if (ipa->version == IPA_VERSION_2_5 || ipa->version >= IPA_VERSION_3_0)
+		ipa_mem_zero_region_add(trans, IPA_MEM_MODEM_PROC_CTX);
 	ipa_mem_zero_region_add(trans, IPA_MEM_MODEM);
 
 	trans->ipa_dma->ops->trans_commit_wait(trans);

Edit: Sending SMS with mmcli works now, required packages (for debian) are modemmanager, rmtfs and probably qrtr-tools.

@alikates
Copy link
Member Author

alikates commented May 9, 2023

Thank you! IIRC SMS and calls don't go through IPA so they should work OOTB. However ModemManager requires a working mobile data endpoint so it will only pick up the modem if IPA at least exposes that endpoint, even if really doesn't work.

@spasswolf
Copy link

spasswolf commented May 9, 2023

At least for me mmcli (as part of ModemManager) only works when the ipa module is loaded. gnome-calls also seems to need ipa to communicate with the modem. This is probably related to modem init.

Here is another patch that removes possible problem:

diff --git a/drivers/net/ipa/ipa_table.c b/drivers/net/ipa/ipa_table.c
index 2c96353319ea..5da8cd1c6456 100644
--- a/drivers/net/ipa/ipa_table.c
+++ b/drivers/net/ipa/ipa_table.c
@@ -361,12 +361,13 @@ static int ipa_route_reset(struct ipa *ipa, bool modem)
        }
 
        ipa_table_reset_add(trans, false, first, count, IPA_MEM_V4_ROUTE);
-       ipa_table_reset_add(trans, false, first, count,
-                           IPA_MEM_V4_ROUTE_HASHED);
-
        ipa_table_reset_add(trans, false, first, count, IPA_MEM_V6_ROUTE
-       ipa_table_reset_add(trans, false, first, count,
-                           IPA_MEM_V6_ROUTE_HASHED);
+       if (ipa->version >= IPA_VERSION_3_0) {
+               ipa_table_reset_add(trans, false, first, count,
+                                       IPA_MEM_V4_ROUTE_HASHED);
+               ipa_table_reset_add(trans, false, first, count,
+                                       IPA_MEM_V6_ROUTE_HASHED);
+       }
 
        trans->ipa_dma->ops->trans_commit_wait(trans);
 

Edit: Also unloading and reloading the ipa module does not give a rmnet_ipa0 interface again:

[  451.224994] ipa 7940000.ipa: IPA driver removed
[  504.961596] ipa 7940000.ipa: IPA driver initialized
[  504.964161] ipa 7940000.ipa: IPA driver setup completed successfully
[  566.230231] ipa 7940000.ipa: error -110 awaiting init driver response

Edit2: SMS still work with no rmnet_ipa0, but if I build a kernel without the ipa module mmcli can't find the modem.

@spasswolf
Copy link

spasswolf commented May 13, 2023

I've finished the rebasing of IPA v2 to branch origin/6.3.0/ipa starting from commit dbfb5bd. It seems to work as good as the 6.1 version, but linux-6.3.0
has other problem on msm8953:

[    4.682114] cacheinfo: Unable to detect cache hierarchy for CPU 0
[    4.682158] CPU0: Booted secondary processor 0x0000000000 [0x51af8014]
[    4.700324] Detected VIPT I-cache on CPU1
[    4.700373] cacheinfo: Unable to detect cache hierarchy for CPU 1
[    4.700414] CPU1: Booted secondary processor 0x0000000001 [0x51af8014]
[    4.719364] Detected VIPT I-cache on CPU2
[    4.719402] cacheinfo: Unable to detect cache hierarchy for CPU 2
[    4.719436] CPU2: Booted secondary processor 0x0000000002 [0x51af8014]
[    4.737968] Detected VIPT I-cache on CPU3
[    4.738006] cacheinfo: Unable to detect cache hierarchy for CPU 3
[    4.738042] CPU3: Booted secondary processor 0x0000000003 [0x51af8014]
[    4.756919] Detected VIPT I-cache on CPU4
[    4.756970] cacheinfo: Unable to detect cache hierarchy for CPU 4
[    4.757009] CPU4: Booted secondary processor 0x0000000100 [0x51af8002] 
[    4.773856] Detected VIPT I-cache on CPU5
[    4.773892] cacheinfo: Unable to detect cache hierarchy for CPU 5
[    4.773922] CPU5: Booted secondary processor 0x0000000101 [0x51af8002]
[    4.791914] Detected VIPT I-cache on CPU6
[    4.791951] cacheinfo: Unable to detect cache hierarchy for CPU 6
[    4.791981] CPU6: Booted secondary processor 0x0000000102 [0x51af8002]

These are unrelated to the ipa port and also occur for linux-6.3.0 form kernel.org.

Edit: How can I best upload the patch, attaching files does not seem to work and I can't post 10000 lines in-line.

Edit2: Rebasing to linux-6.4 should not pose a problem as the only change in drivers/net/ipa from 6.3 to 6.4-rc1 is a more complete support of IPA v5.0.

@spasswolf
Copy link

spasswolf commented May 13, 2023

Now here's the patch, which had to be renamed to *.txt:
ipa-v2-6.3.0-msm8953.txt
This also seems to hang on reboot/shutdown, but I'm not sure if this is related to IPA.

@alikates
Copy link
Member Author

Can you open a PR for it? Also please keep the commit history. It's easier for sending it upstream and getting patches accepted.

@spasswolf spasswolf linked a pull request May 14, 2023 that will close this issue
telent added a commit to telent/mobile-nixos that referenced this issue Jan 15, 2024
This Soc uses an earlier version of the Qualcomm IPA thing
which is not supported by the existing drivers/net/ipa
(only works on SDM854)

msm8953-mainline/linux#93
@barni2000
Copy link
Member

barni2000 commented May 11, 2024

@vldly do you plan upstream ipa2-lite? If no i think it can be closed.

@vldly
Copy link
Member

vldly commented May 12, 2024

@vldly do you plan upstream ipa2-lite? If no i think it can be closed.

If/when it's polished (as much as it can be) and well tested then why not? I don't like approach of sending lower quality code and expecting maintainers/reviewers to point out flaws or hoping they won't notice or ignore it...

@barni2000 barni2000 mentioned this issue May 12, 2024
7 tasks
@barni2000
Copy link
Member

I have closed this in favor of #196

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants