samples: wifi: fix wifi connect functionality on NXP frdm_rw612

[andrewnyland]

This change enables hardware entropy support on NXP's frdm_rw612 board to enable wifi connection to an access point in the wifi sample. It also disables a Kconfig flag which was causing incorrect entropy initialization in mbedtls.

Fixes #81025.

[github-actions]

Hello @andrewnyland, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

[danieldegrasse]

@andrewnyland thank you for this fix- can you rebase this change to resolve conflicts?

If you'd like to merge this change before 4.0 (which we should, IMO), then we need to create an issue report describing how to reproduce the failure on the frdm_rw612 board. If you'd like help with this, please let me know. Once the issue is created, you need to reference the issue number (present in the github URL) from the commit message in this PR, as well as the PR text

[danieldegrasse]

Do we need to manually set this to n, or could we just remove this line? It seems like this will be disabled by default.

[andrewnyland]

I was going to ask for clarity on this from anyone who is more familiar with it. The other changes I've made in this PR have a logical basis + can be shown from the DT files, but this one in particular was more of a brute force approach. I'm not sure if mbedtls/entropy initialization is just being called incorrect/hasn't been updated to support this convention (like maybe it has been but is on a feature branch and not mainstream zephyr), and maybe should be added again later for security reasons. I made this PR simply to fix basic functionality of this board for this sample (and we were still concerned the drivers might not have been added correctly from NXP yet but that appears to not be the case).

Removing it could make sense to me, but in the grand scheme of things I was assuming that changes like that should be another PR. That sample/conf file has a few other Kconfig flags being set that I assume are to support future features, as some of them have extremely limited documentation or are not used in Zephyr code as far as I could see. So my thinking was to leave this as n for now so that someone might know/remember to ensure that feature does work later - if it was intended correctly by someone or will be supported in the future. Hope that makes sense.

[krish2718]

Entropy should be using the PSA APIs not the legacy MbedTLS APIs.

[andrewnyland]

Ok, I think that makes sense, I'm not fully familiar with what the difference there is (on the implementation details side of things). Do you mind expanding on that more?

@krish2718 Or do you have any input on whether that Kconfig flag should persist here, and/or is it a library change that needs to happen unrelated to this sample? I'm just wondering how that difference affects the scope/content of this PR. Thanks.

[krish2718]

I don't know the original issues, so, cannot comment much. But we shouldn't disable PSA, instead we should fix the entropy to use PSA. Can you please share details or logs of the original problem?

[andrewnyland]

Yeah, I just finished writing an issue for this PR, I can link it to you. I'll copy in the relevant logs here in case it helps at all.

The bootup logs are as below:

Set BGA tx power table data 
[00:00:01.220,419] <dbg> nxp_wifi: nxp_wifi_wlan_event_callback: WLAN: received event 14
uart:~$ *** Booting Zephyr OS build v4.0.0-rc1-76-g743761d7d1d7 ***
[00:00:01.237,361] <dbg> nxp_wifi: nxp_wifi_wlan_event_callback: WLAN initialized
uart:~$ supp_psa_crypto_init failed [00:00:01.251,870] <inf> wifi_supplicant: wpa_supplicant initialized
[00:00:01.265,358] <dbg> nxp_wifi: nxp_wifi_wlan_event_callback: WLAN: received event 16
[00:00:01.276,122] <dbg> nxp_wifi: nxp_wifi_wlan_event_callback: WLAN: PS_ENTER
[00:00:01.286,156] <dbg> nxp_wifi: nxp_wifi_wlan_event_callback: WLAN: received event 16
[00:00:01.296,955] <dbg> nxp_wifi: nxp_wifi_wlan_event_callback: WLAN: PS_ENTER
[00:00:01.308,682] <err> wpa_supp: Init of random number generator failed

Which corresponds to code in ~/zephyrproject/modules/hostap/src/supp_main.c:1126:

#ifdef CONFIG_WIFI_NM_WPA_SUPPLICANT_CRYPTO_MBEDTLS_PSA
	supp_psa_crypto_init();
#endif

which calls psa_crypto_init() in ~/zephyrproject/modules/lib/hostap/port/mbedtls/supp_psa_api.c:468.

Hope this helps. My initial impression is that a real fix for this aspect is out of scope for this PR, but I'm happy to help or try to fix it you think it is worth doing here.

[krish2718]

Thanks. mbedtls_ctr_drbg_seed CTR-DRBG is a legacy way to get random numbers, so, when PSA is enabled it should using PSA APIs. Anyways, as you say fixing it properly is out of scope of this PR.

FYI @MaochenWang1 @fengming-ye from a quick look we need to add proper guards to compiled out legacy stuff when PSA is enabled. This could affect multiple platforms, so, please take a look

@andrewnyland I would suggest to raise another bug for getting Wi-Fi connection working with PSA enabled.

[andrewnyland]

@krish2718 Ok, good to know - thanks for the input. I'll make some notes and make one next week, I have another PR I'd like to submit for the wifi shell that I think I'll do first. And partially as I'm more unfamiliar with that realm (the base security system on-board with mbedtls, I've done a lot with TCP conns recently however). I'll try to at least make an issue for it this week.

Would you suggest removing that line for this PR or just deal with it later?

[krish2718]

Would you suggest removing that line for this PR or just deal with it later?

Does it work without that line? If no, then we can leave that in.

[andrewnyland]

@danieldegrasse Yeah sure, I'll try to throw one together now. I'm quite new to this board (we're switching to it from the nrf7002dk for more RAM/etc) and to issues here, but have far more familiarity with Zephyr's code base at least. Will comment here or tag you here/on the issue, whatever ends up being appropriate.

Sounds good on the timeline, I wasn't sure when the next big merge and such will be.

[zephyrbot]

The following west manifest projects have been modified in this Pull Request:

Name	Old Revision	New Revision	Diff

Note: This message is automatically posted and updated by the Manifest GitHub Action.

[andrewnyland]

@danieldegrasse Just did the rebase, I think I've done it correctly but I'm not sure so please let me know if I did do something wrong and if so what to do to fix it. Also, as I haven't made any major changes, git won't allow me to re-sign a commit, only push the updated repo. Thanks.

[andrewnyland]

I completely forgot to put the issue # in that merge, github's UI confused me with it. I'll try to add that now.

#81025

[dleach02]

@andrewnyland you have something weird with this PR as it shows 157 commits. Try cleaning up your branch to the tip of main with just the commit with the one file change.

[andrewnyland]

@dleach02 Ok, yeah I thought that was weird. I'll look into it, this was my first time rebasing something in a while. I pulled main into my fork and then rebased my branch to that.

[andrewnyland]

@dleach02 I think I fixed the extra commits, I try to avoid force pushing but I followed a tutorial online for fixing this specific issue - please let me know if I did that incorrectly.

I saw in my email that you mentioned you fixed me missing having the issue number in a commit - do I still need to fix that? I know how to do it with github in the future (the merge conflict wasn't showing up in local code but now that I know how to update forks I think that's fine going forward). If so, I could remove the Kconfig flag in the above conversation and commit that with the issue #. Thanks for the help.

[dleach02]

force pushing to your branch is how we handle PRs. You get used to it after a while

[andrewnyland]

@dleach02 Ok, good to know. Is there anything else I should do on this PR or just wait for reviewers to approve?

[dleach02]

@dleach02 Ok, good to know. Is there anything else I should do on this PR or just wait for reviewers to approve?

Wait for the reviewers. I reached out to the internal NXP WiFi team and there might be a different way to address this that retains the original settings. I'm the assignee so I'll make sure we reach some sort of resolution.