-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
ports: Support UART.irq for the RP2, ESP32, MIMXRT, NRF, Renesas-RA and SAMD ports. #14041
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
Conversation
Thanks for this, it's a great addition! Definitely needed. By chance I was working on the docs side of this today: #14040. |
1c2854a
to
27742b0
Compare
27742b0
to
b18b64b
Compare
ce8fc89
to
7d14aee
Compare
Discussion topics:
|
7d14aee
to
2754f50
Compare
Code size report:
|
9c59a7c
to
0a0831b
Compare
Thanks again for working on this. After thinking more about it here are my general comments. I don't think it makes sense to have both We need to make it as easy as possible to write portable code that runs across all the ports, and having different IRQs, or the same IRQ but with slightly different semantics, makes that hard. So I suggest to not have
The idea is that if you only ever do Even if the hardware/SDK doesn't support this concept we should still be able to synthesize it. Eg if the hardware supports Having For |
So you suggest that IRQ_RX is renamed to IRQ_RXIDLE for those ports that do not support a true IRQ_RXIDLE, and for those that do so IRQ_RX is dropped? B.t.w: I made two Python classes as example that implement IRQ_RXIDLE based on an existing IRQ_RX. The simpler one is below with uart.irq() being a method. The other implements uart.irq() as a class. I considered implementing that in the machine_uart module itself, but dealing with the timer is easier in Python.
|
@dpgeorge Thinking about further, I prefer to support the behaviour with proper names. Looking at the list of ports, three of them support true IRQ_RXIDLE, and eight support IRQ_RX. Both the rp2040 and the MIMXRT port can support IRQ_RX mode. The rp2040 commit would get much simpler and more close to the existing state. Without support for IRQ_RXIDLE, the MIMXRT port would not need a change to the NXP UART driver. |
The change to esp32/machine_timer.c is made and the MIMXRT MCU's re-tested. I removed at the test scripts the restrictions for Teensy 4.x. So any MIMXRT board can be used. |
Supported trigger names: IRQ_RXIDLE, IRQ_TXIDLE, IRQ_BREAK - IRQ_RXIDLE: The handler for IRQ_RXIDLE is called reliably 31 UART bit times after the last incoming data. - IRQ_TXIDLE: This IRQ is triggered after at least >5 characters are sent at once. It is triggered when the TX FIFO falls below 4 elements. At that time, up to 5 bytes may still be in the FIFO and output shift register. - IRQ_BREAK: The IRQ triggers if a BREAK state is detected at RX. Properties & side effects: - After a BREAK, a valid character must be received before another break can be detected. - Each break puts a 0xff character into the input buffer. The irq.flags() value is cleared only with a new wanted event. Do not change the flags otherwise. Signed-off-by: robert-hh <robert@hammelrath.com>
Supported for all SAMD51 devices and SAMD21 with external flash. For interrupt events, IRQ_RX and IRQ_TXIDLE are provided. IRQ_RX is called for every received byte. This may not be useful for high data rates, but can be used to build a wrapper class providing an IRQ_RXIDLE event or to signal just the first byte of a message. IRQ_TXIDLE is called only when messages are longer than 5 bytes and triggers when still 5 bytes are due to be sent. The SAMD hardware does not support implementing IRQ_RXIDLE. Signed-off-by: robert-hh <robert@hammelrath.com>
Supported triggers are: IRQ_RXIDLE and IRQ_TXIDLE. When IRQ_RXIDLE is set, the handler will be called 3 character times after the data in burst stopped. When IRQ_TXIDLE is set, the handler will be called immediately after the data has been sent. This commit requires a change to fsl_lpuart.c, because the existing code does not support under-run appropriately. The irq.flags() value is cleared only at an expected event. Do not change it otherwise. Signed-off-by: robert-hh <robert@hammelrath.com>
The renesas-ra port supports calling a handler to be called on every byte received by UART. For consistency with other ports, the symbol IRQ_RX is added as the trigger name. Side change: Add the received UART data to the REPL input buffer only if it is the REPL UART. Otherwise, every UART would act as REPL input. Signed-off-by: robert-hh <robert@hammelrath.com>
As alternative to RX_ANY to match the names used by the other ports. Signed-off-by: robert-hh <robert@hammelrath.com>
Supported trigger events: IRQ_RX and IRQ_BREAK. Hard IRQ is not supported. Signed-off-by: robert-hh <robert@hammelrath.com>
This commit fixes a bug in the existing driver, that the UART baud rate could not be changed without reset or power cycle. It adds as well functionality to UART.deinit(). Signed-off-by: robert-hh <robert@hammelrath.com>
Supported triggers: UART.IRQ_RX and UART.IRQ_TXIDLE. It will probably work on other boards as well, but so far untested. The irq.flags() value is changed only when requested by a triggered event. Do not change it otherwise. Signed-off-by: robert-hh <robert@hammelrath.com>
Just adding the event symbol. No code change required, and no impact on code execution time when the event is not selected. Tested with STM32F4xx, STM32F7xx and STM32H7xx. Signed-off-by: robert-hh <robert@hammelrath.com>
With the softtimer the minimal delay between the end of a message and the trigger is 2 ms. For baud rates <= 9600 baud it's three character times. Tested with baud rates up tp 115200 baud. The timer used for RXIDLE is running only during UART receive, saving execution cycles when the timer is not needed. The irq.flags() value is changed only with an expected event. Do not change it otherwise. Signed-off-by: robert-hh <robert@hammelrath.com>
Allowing to define the trigger UART.IRQ_RXIDLE as well as UART.IRQ_RX. The delay for the IRQ_RXIDLE interrupt is about 3 character times or 1-2 ms, whichever is larger. The irq.flags() value is changed only with an expected event. Do not change it otherwise. Signed-off-by: robert-hh <robert@hammelrath.com>
The UART.IRQ_IDLE callback is called about two character times after the last byte, or 1 ms, whichever is larger. For the irq, timer 0 is used. machine_timer.c had to be reworked to make it's mechanisms available for machine_uart.c. The irq.flags() value is change only at a requested event. Otherwise keep the state. Signed-off-by: robert-hh <robert@hammelrath.com>
These docs now match the code in `extmod/machine_uart.c`. IRQ trigger support still need to be updated for each port (to be done in a follow-up commit). Signed-off-by: Damien George <damien@micropython.org>
For more ports and trigger options, based on the current state of the code. Signed-off-by: robert-hh <robert@hammelrath.com>
The test checks whether the message created by the IRQ handler appears about at the end of the data sent by UART. Supported MCUs resp. boards: - RP2040 - Teensy 4.x - Adafruit ItsyBitsy M0 - Adafruit ItsyBitsy M4 - NRF52 (Arduino Nano Connect 33 BLE) Signed-off-by: Damien George <damien@micropython.org>
These all require hardware connections, so live in a different directory. Except for the IRQ_BREAK test of ESP32 devices a single UART with loopback is sufficient. General: SAMD21: Due to the limited flash size only SAMD21 devices with external flash support uart.irq(). IRQ_BREAK: ESP32 needs different UART devices for creating and sensing a break. Lacking a second UART the test is skipped for ESP32S2 and ESP32C3. RP2 does not pass the test reliable at 115200 baud, reason to be found. Thus the upper limit is set to 57600 Baud. Coverage: esp32 pass when different UART devices are used. rp2 pass up to 57600 baud IRQ_RX: SAMD21: Being a slow device it needs data to be sent byte-by-byte at 9600 baud, since the IRQ callback is scheduled delayed and then the flags do not match any more. The data matches since it is queued in the FIFO resp. ringbuffer. CC3200: The test cannot be performed since no calls are accepted in the IRQ handler like u.read(). Skipped. Coverage: cc3200 fail due to major differences in the implementation. esp32 pass nrf pass renesas-ra pass samd pass see the notes. stm32 pass IRQ_RXIDLE: STM32: With PyBoard the IRQ is called several times, but only once with the flag IRQ_RXIDLE set. Coverage: esp32 pass mimxrt pass renesas-ra pass rp2 pass samd pass for both SAMD21 and SAMD51 stm32 fail. see notes. Signed-off-by: Damien George <damien@micropython.org> Signed-off-by: robert-hh <robert@hammelrath.com>
dc934f7
to
09d070a
Compare
OK, great. I also removed the test configuration for the samd I also did some very minor fixes to this PR:
And I kept all 16 commits. They are well separated commits making independent changes. Thanks @robert-hh for all your work on this! And @Gadgetoid for testing, and @projectgus for additional reviews. |
Thank you for the improvements and for merging. |
Note this doesn't build anymore with more recent SDKs because the isr variable is now defined:
|
Which port? |
mimxrt, with any recent SDK release. They changed that variable to any array. |
We are still at sdk 2.10, and we use a modified fsl_lpuart.c to get a proper IRQ_RXIDLE behavior. Updating to a newer SDK is still due. When that happens I expect more changes to be done. No too many, as I noticed when trying that last year. |
I understand. I'm just saying if you try to use a more recent SDK this doesn't build anymore. However, it seems everything else works. I'm not sure which SDK I'm using but it's more recent than the one in MicroPython. |
Adds UART IRQ functionality for 4 additional ports: RP2, ESP32, MIMXRT and SAMD. Available triggers:
IRQ_RX: called every time when a byte is received. Not useful for high data rates (like > 9600 baud).
IRQ_RXIDLE: called when the receive line gets silent after a burst of data. Very useful!
IRQ_TXIDLE: called when a block of data has been sent.
Not all ports provide all triggers:
hard=True
is not supported.Tested with RP2040, ESP32. ESP32-S2, ESP32-S3, ESP32-C3, SAMD21, SAMD51, NRF52840, and all supported MIMXRT MCUs except i.MX RT 1064 (I do not have access to a board).
Test observations for other boards: