msm8953-mainline / linux

Linux mainline kernel with WIP patches for msm8953 devices
Other
111 stars 59 forks source link

IPA upstreaming #93

Closed alikates closed 4 months ago

alikates commented 1 year ago

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 commented 1 year ago

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 b5b016f951f44dcc5a584d31843ea7e0ce5bf1c2 of msm8953-6.1.0/main.

spasswolf commented 1 year ago

[Uploading linux-msm8953-IPAv2.PATCH…]()

alikates commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

I've finished the rebasing of IPA v2 to branch origin/6.3.0/ipa starting from commit dbfb5bd9330eeba18015dc821f885163813c0058. 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 commented 1 year ago

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 commented 1 year ago

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

barni2000 commented 4 months ago

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

vldly commented 4 months ago

@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 commented 4 months ago

I have closed this in favor of #196