@dpgeorge WiFi is almost ready I think, most of the networking tests pass, however it doesn't seem to work very well with OOB IRQ (CYW43_PIN_WL_HOST_WAKE enabled) I might be missing something, I could take another look at that later.

However I'm more concerned with Bluetooth, it doesn't work at all because cywbt.c is not portable, I tried to enable it but the port is missing named pins (for BT regulator control, cts etc..), and cywbt.c uses some STM specific functions, so I didn't link it in the Makefile, the weak functions in mpbthciport.c are used. Although I'm almost sure I saw bluetooth working once or twice when I started testing this, not sure how, maybe by initializing WiFi first,

Unrelated question, why is the ticks management in this port so complex ? Why use a GPT timer at all to manage ticks, delay etc.. and not just use the systick like in stm32 ?

Networking tests (note this port doesn't have cert validity checks):

20 tests performed
15 tests passed
2 tests skipped: multi_net/tcp_client_rst.py multi_net/uasyncio_tcp_client_rst.py
3 tests failed: multi_net/ssl_cert_rsa.py multi_net/uasyncio_tcp_readall.py multi_net/udp_data.py

Unrelated question, why is the ticks management in this port so complex ? Why use a GPT timer at all to manage ticks, delay etc.. and not just use the systick like in stm32 ?

That's a question for @robert-hh

That's a question for @robert-hh

I had seen the question but thought it was about the STM32 port. If we are talking about ticks.c: that existed when Philipp and me started with the MIMXRT port, and I kept is as it is. But it is a good timing source for µs resolution. It is well made (by @dpgeorge, as git tells), not overly complicated. The calls to the NXP lib look a little bit bulky.

Yes I was referring to ticks.c, I don't think it's needed at all, it uses an extra resource (GPT timer) to handle something that can be handled by the existing and running SysTick, so more code and more power, and it also assumes the crystal is 24MHz, and disables global IRQs in more than one place, and makes a lot of calls to the SDK. IMHO I think it should just be replaced by something like stm32/systick.c, or that moved into /shared and reused.

systick is not suitable for the ticks_us() part, because it has just a ms resolution. ticks.c could be simplified. The ticks_delay_us64() with ticks_wake_after_us32() part may not be needed. But it's working, and as such it follows the rule "if it ain't broken, don't fix it". The GPT timers are not used for anything else. I used them initially for the timer class, but moved that to SoftTimer.

But it's working, and as such it follows the rule "if it ain't broken, don't fix it".

I was not going to try to fix anything, just pointing out that there's room for improvement, I'm mainly concerned with WiFi/BT support right now, but you know that rule doesn't really apply here, code changes all the time to improve time/space complexity, even if it's to save a few bytes.

@dpgeorge I think the bluetooth issue might actually be faulty hardware or a power supply issue with the evk/breakout I'm using, I will get prototypes of the actual hardware next week and will retest Bluetooth. In the meantime, I think cywbt.c can be made more portable by using the mp_bluetooth_hci_uart_* functions instead of the stm32-uart, like I did in the Nina HCI driver, and by defining the Bluetooth control pins in cyw43_configport.h.

Regarding the OOB issue, it was actually one of those reads failing and locking up the bus, I've now added code to clear the error after failure, and it works normally now just like the SDIO interrupt mode. Just a thought, it might be valuable to write a porting guide for the driver at some point.

static void cyw43_kso_set(cyw43_int_t *self, int value) {
    // these can fail and it's still ok
    cyw43_write_reg_u8(self, BACKPLANE_FUNCTION, SDIO_SLEEP_CSR, write_value);
    cyw43_write_reg_u8(self, BACKPLANE_FUNCTION, SDIO_SLEEP_CSR, write_value);
``

@dpgeorge Do we have to switch the CTS pin to GPIO in cywbt_wait_cts_low function ? Please see the note in my last commit in this PR for context, alternatively can we move cywbt_wait_cts_low to <port>/mpbthciport.c with a prototype in extmod/mpbthci.h (for example bool mp_bluetooth_hci_uart_poll_cts(void);) ?

One more question, I tested WiFi with a 25MHz initial clock and enumeration, firmware loading etc.. everything seems to work fine at 25MHz, I'm wondering if the initial 400KHz clock is really required ? It makes a big difference in boot time.

@robert-hh I noticed that the processor doesn't wake up from WFI() called from CYW43_DO_IOCTL_WAIT (see trace below) without SysTick priority set to a higher priority other than the default (lowest), GPT IRQ in ticks.c doesn't interrupt WFI() probably because it triggers when delay is needed, I guess it's fine for ticks source but SysTick is still needed. Note I also moved the priorities and irq functions to irq.h in the last commit to clean up mpconfigport.h

cyw43_do_ioctl (self=self@entry=0x2000779c <cyw43_state>, kind=kind@entry=2, cmd=cmd@entry=263, len=len@entry=1560, 
    buf=buf@entry=0x200077d4 <cyw43_state+56> "clmload", iface=iface@entry=0) at ../../lib/cyw43-driver/src/cyw43_ll.c:1223
1223	        CYW43_DO_IOCTL_WAIT;

You may have found the reason for a long standing issue in the mimxrt port, in that WFI() was not interrupted by systick. That may have been one reason why ticks.c was implemented this way with the compare IRQ on the GPT. The other reason is µs-precision for delays. Just the ticks_ms_upper counter & handling is a little bit of over-engineering.
Moving priorities and definitions to irq.h is fine, tidying up mpconfigport.h a little bit.
So it seems that what started as "just" a CYW43 port gets a rework for both STM32 and MIMXRT port.

So it seems that what started as "just" a CYW43 port gets a rework for both STM32 and MIMXRT port.

Yeah but I had to get a few things in order first, mainly cywbt.c, but hopefully I'm done for this PR.

b.t.w. Which board do you use for testing? I looked back into the various PR about the WFI()/WFE() topic, and it seems only remained for the 117x MCU.

b.t.w. Which board do you use for testing? I looked back into the various PR about the WFI()/WFE() topic, and it seems only remained for the 117x MCU.

MIMXRT1060-EVKB

Final update, WiFi and Bluetooth are both working fine now, see test results below, although for BT nothing above 115200 is reliable, since there's no hardware flow control support in machine_uart.c. Fixing that I think is beyond the scope of this PR, as much as I would like to refactor the whole port I have other things to do ;)

Bluetooth

./run-multitests.py -i pyb:a0 -i pyb:a1 multi_bluetooth/*py
18 tests performed
16 tests passed
1 tests skipped: multi_bluetooth/ble_deepsleep.py
1 tests failed: multi_bluetooth/stress_log_filesystem.py

WiFi

./run-multitests.py -i pyb:a0 -i pyb:a1 multi_net/*
20 tests performed
14 tests passed
2 tests skipped: multi_net/tcp_client_rst.py multi_net/uasyncio_tcp_client_rst.py
4 tests failed: multi_net/ssl_cert_rsa.py multi_net/uasyncio_tcp_close_write.py multi_net/uasyncio_tcp_readall.py multi_net/udp_data.py

@dpgeorge Maybe this one can be merged too ? Possibly after the next release, but it's not as invasive as it seems.

Are we any closer to resolving the pending conversations here ?

I tested this on PYBD-SF2/SF6 with the BLE multitests, and it works.

Pending issues:

* The `cywbt_wait_cts_low` function in `cywbt.c` should not change IO config, because the pin is still readable.

* In `cywbt.c` every `mp_hal_pin` should be replaced with `cyw43_hal_pin`

@iabdalkader I agree these points can be improved (but they were not necessary to get this PR merged). Feel free to have a go at fixing them, otherwise I'll eventually get to them.

Well at least now the driver is usable by other ports so it should be fine for now, but may get around to it. For the first point, adding mp_bluetooth_hci_uart_poll_cts might be the safest option.

Are there any plans (or reasons not) to move cywbt.c to lib/cyw43-driver/ ?

Are there any plans (or reasons not) to move cywbt.c to lib/cyw43-driver/ ?

We can do that. It changes the licensing but I don't think there's really a problem with that. Would be neater to keep everything in lib/cyw43-driver. (Also rp2 uses a completely different BT backend, but again that doesn't impact moving this file there.)

Are there any plans (or reasons not) to move cywbt.c to lib/cyw43-driver/ ?

We can do that. It changes the licensing but I don't think there's really a problem with that. Would be neater to keep everything in lib/cyw43-driver. (Also rp2 uses a completely different BT backend, but again that doesn't impact moving this file there.)

Following-up on this, I moved cywbt.c to cyw43-driver in PR georgerobotics/cyw43-driver#123. The PR also makes the firmware address and size configurable. This allows us to move the bt firmware blob to QSPI for example (if needed) to save flash space.