cpu/gd32v/periph_i2c: interrupt based driver #19269
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gschorcht
force-pushed
the
cpu/gd32v/periph_i2c_interrupt_driven
branch
2 times, most recently
from
f8a6656 to
d966f6a
Compare
gschorcht
added
Type: enhancement
gschorcht
force-pushed
the
cpu/gd32v/periph_i2c_interrupt_driven
branch
from
d966f6a to
7a9e2c4
Compare
github-actions
bot
removed
the
Platform: RISC-V
gschorcht
added
the
State: WIP
gschorcht
force-pushed
the
cpu/gd32v/periph_i2c_interrupt_driven
branch
from
7a9e2c4 to
75547d3
Compare
gschorcht
removed
the
State: WIP
benpicco
reviewed
Feb 13, 2023
| { | ||
| uint16_t tick = TICK_TIMEOUT; | ||
| while ((i2c->STAT1 & I2C_STAT1_I2CBSY_Msk) && tick--) {} | ||
| /* short busy waiting for the bus becoming free (I2CBSY is cleared) */ | ||
| while ((i2c_config[dev].dev->STAT1 & I2C_STAT1_I2CBSY_Msk) && tick--) {} |
There was a problem hiding this comment.
So that's just the delay to power on the I2C peripheral, right?
There was a problem hiding this comment.
No, it is only a very short delay from setting the STOP bit to sending the STOP condition on the bus. I2C_STAT1_I2CBSY is cleared by the controller as soon as the STOP condition is sent. Unfortunately this does not trigger an interrupt, so polling is the only way.
gschorcht
commented
Feb 13, 2023
|
|
||
| static void _wait_for_irq(i2c_t dev) | ||
| { | ||
| #if defined(MODULE_ZTIMER_USEC) |
There was a problem hiding this comment.
Timeouts are realized either
- by means of
ztimer_usectimers, if theztimer_usecmodule is already activated, - by
ztimer_msectimer if theztimer_msecmodule is already activated, or - not at all.
The driver itself does not pull a timer module by itself.
However, the use of ztimer_msec seems to make no sense at all, since it uses the RTC, for which a write operation to an RTC register requires at least 3 RTC clock cycles (i.e. about 91 us). This means that setting and removing the timer again takes about 180 us in total, which is about twice the duration for sending/receiving a byte at 100 kHz clock. As a result, there seems to be an idle phase between the sent and received bytes, which is about the duration of a sent or received byte and thus reduces the speed of the I2C transmission to about half, see the image below.
Even worse, during this time (the entire 180 us) the MCU is constantly polling the RTC's status register to determine when the write access for the timer set/unset is complete. So instead of letting the MCU sleep while waiting for an interrupt, the MCU is in a waiting state the whole time, which makes the interrupt-driven I2C driver completely absurd
With timeout handling using ztimer_usec it looks as it should:
So the question is whether we remove the timeout handling to avoid using ztimer_usec, or whether we enable ztimer_usec for the I2C driver. If we use timeout handling, it makes only sense with ztimer_usec.
There was a problem hiding this comment.
I think ztimer_usec is fine.
There is no requirement to avoid it except for long-running timers. If you only need a short delay there is nothing wrong with ztimer_usec.
I understood it so that you added the ztimer_msec as a fallback for an application that would otherwise not use ztimer_usec, but I think this is rare.
There was a problem hiding this comment.
I think
ztimer_usecis fine.
There is no requirement to avoid it except for long-running timers. If you only need a short delay there is nothing wrong withztimer_usec.
But doesn't this mean that the timer peripheral will run all the time even if I2C is not used and no timer is set, at least if the ztimer_timer_ondemand module is not used?
There was a problem hiding this comment.
I didn't follow the discussion there tbh, but afaik the (long term) goal is that the timer should be only running if it has alarms scheduled or if it was acquired somehow to measure time.
Since we are not there yet (there is plenty of code that relies on the timer to be always running to measure time intervals) this is indeed the case, but your code should not bend over backwards to accommodate a temporary (let's hope so!) shortcoming of the ztimer implementation.
It already does the right thing by scheduling an alarm, so once we flip the switch (Is ztimer_timer_ondemand that switch?) it will just behave as it should and only enable the clock when needed.
gschorcht
requested review from
leandrolanzieri,
jia200x and
MrKevinWeiss
as code owners
|
still needs squashing 😉 |
gschorcht
force-pushed
the
cpu/gd32v/periph_i2c_interrupt_driven
branch
from
7bad813 to
3fd04b7
Compare
|
bors merge |
bors bot
added a commit
that referenced
this pull request
Feb 17, 2023
19027: sys/fmt: optimize scn_u32_dec scn_u32_hex r=benpicco a=kfessel ### Contribution description Improves the compilation result for `scn_u32_dec` `scn_u32_hex` especially on `cortex-m` reducing either stack usage and or code size. This makes use of unsigned int overflow (slightly less better without doing that `hexn`). See godbolt (original versions got an `o` attached, modified versions got `k`s) all functions are marked `_S_` defined to `static` by assigning the global at end the compiled function can be changed (`deco deck hexo hexk hexkk hexn`) this PR is `hexkk` and `deck` ### Testing procedure run unit-test/test-fmt ``` <RIOT>/tests/unittests$ make tests-fmt <RIOT>/tests/unittests$ make term ``` ### Issues/PRs references [godbolt](https://godbolt.org/z/MzT1zh4q1) 19256: pkg/tinyusb: add GD32VF103 support r=benpicco a=gschorcht ### Contribution description This PR provides the tinyUSB support for GD32VF103 and enables the `tinyusb_device` feature as well as `stdio_tinyusb_cdc_acm` for GD32VF103 boards. ### Testing procedure ``` BOARD=sipeeed-longan-nano make -C tests/shell flash term ``` should work ### Issues/PRs references 19269: cpu/gd32v/periph_i2c: interrupt based driver r=benpicco a=gschorcht ### Contribution description This PR provides an interrupt-driven version of the I2C low-level driver. The existing I2C low-level driver for GDVF103 uses a busy-waiting approach where the status register is continuously polled while waiting for a certain status when sending or receiving. The MCU is thus occupied the whole time during a send or receive operation. The driver provided with this PR uses an interrupt-driven approach. This is, while waiting for a certain status when sending or receiving, the calling thread is suspended and woken up by interrupts. Since the I2C controller allows to receive up to two bytes before the application has to react, receiving a single byte, two bytes or more than two bytes needs a different handling for correct receiption. This requires a tricky implementation which distinguish a number of different case. There the driver requires 860 byte more ROM and 8 byte more RAM. ### Testing procedure The driver should work with any I2C sensor/actuator. It was tested with - `tests/driver_bmp180` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) BMP180 test application +------------Initializing------------+ Initialization successful +------------Calibration------------+ AC1: 8448 AC2: -1208 AC3: -14907 AC4: 33310 AC5: 24774 AC6: 19213 B1: 6515 B2: 49 MB: -32768 MC: -11786 MD: 2958 +--------Starting Measurements--------+ Temperature [°C]: 22.0 Pressure [hPa]: 1006.49 Pressure at see level [hPa]: 1025.55 Altitude [m]: 157 +-------------------------------------+ Temperature [°C]: 22.0 Pressure [hPa]: 1006.56 Pressure at see level [hPa]: 1025.58 Altitude [m]: 157 +-------------------------------------+ ``` </details> - `tests/driver_ccs811` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) CCS811 test application +------------Initializing------------+ +--------Starting Measurements--------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ ``` </details> - `tests/driver_sht3x` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) SHT3X test application +------------Initializing------------+ Initialization successful +--------Starting Measurements--------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.50 +-------------------------------------+ Temperature [°C]: 21.47 Relative Humidity [%]: 54.53 +-------------------------------------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.48 +-------------------------------------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.47 +-------------------------------------+ ``` </details> - `tests/driver_l3gxxxx` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-375-g75547-cpu/gd32v/periph_i2c_interrupt_driven) L3GXXXX gyroscope driver test application Initializing L3GXXXX sensor [OK] gyro [dps] x: +0, y: -1, z: -2 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: -1, y: +0, z: +4 gyro [dps] x: +0, y: +0, z: -21 gyro [dps] x: +0, y: +0, z: +6 gyro [dps] x: -43, y: +0, z: -13 gyro [dps] x: -21, y: -2, z: +0 gyro [dps] x: +0, y: +1, z: +3 gyro [dps] x: +25, y: +0, z: +0 ``` </details> - `tests/driver_hd44780` with `pcf8574a` I2C interface ### Issues/PRs references 19286: cpu/esp_common: use generic WIFI_SSID/WIFI_PASS defines r=benpicco a=benpicco Co-authored-by: Karl Fessel <karl.fessel@ovgu.de> Co-authored-by: Gunar Schorcht <gunar@schorcht.net> Co-authored-by: Benjamin Valentin <benpicco@beuth-hochschule.de>
|
Build failed (retrying...): |
bors bot
added a commit
that referenced
this pull request
Feb 17, 2023
19269: cpu/gd32v/periph_i2c: interrupt based driver r=benpicco a=gschorcht
### Contribution description
This PR provides an interrupt-driven version of the I2C low-level driver.
The existing I2C low-level driver for GDVF103 uses a busy-waiting approach where the status register is continuously polled while waiting for a certain status when sending or receiving. The MCU is thus occupied the whole time during a send or receive operation.
The driver provided with this PR uses an interrupt-driven approach. This is, while waiting for a certain status when sending or receiving, the calling thread is suspended and woken up by interrupts.
Since the I2C controller allows to receive up to two bytes before the application has to react, receiving a single byte, two bytes or more than two bytes needs a different handling for correct receiption. This requires a tricky implementation which distinguish a number of different case. There the driver requires 860 byte more ROM and 8 byte more RAM.
### Testing procedure
The driver should work with any I2C sensor/actuator. It was tested with
- `tests/driver_bmp180`
<details>
```
main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven)
BMP180 test application
+------------Initializing------------+
Initialization successful
+------------Calibration------------+
AC1: 8448
AC2: -1208
AC3: -14907
AC4: 33310
AC5: 24774
AC6: 19213
B1: 6515
B2: 49
MB: -32768
MC: -11786
MD: 2958
+--------Starting Measurements--------+
Temperature [°C]: 22.0
Pressure [hPa]: 1006.49
Pressure at see level [hPa]: 1025.55
Altitude [m]: 157
+-------------------------------------+
Temperature [°C]: 22.0
Pressure [hPa]: 1006.56
Pressure at see level [hPa]: 1025.58
Altitude [m]: 157
+-------------------------------------+
```
</details>
- `tests/driver_ccs811`
<details>
```
main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven)
CCS811 test application
+------------Initializing------------+
+--------Starting Measurements--------+
TVOC [ppb]: 0
eCO2 [ppm]: 0
+-------------------------------------+
TVOC [ppb]: 0
eCO2 [ppm]: 0
+-------------------------------------+
TVOC [ppb]: 0
eCO2 [ppm]: 0
+-------------------------------------+
TVOC [ppb]: 0
eCO2 [ppm]: 400
+-------------------------------------+
TVOC [ppb]: 0
eCO2 [ppm]: 400
+-------------------------------------+
TVOC [ppb]: 0
eCO2 [ppm]: 400
+-------------------------------------+
TVOC [ppb]: 7
eCO2 [ppm]: 446
+-------------------------------------+
TVOC [ppb]: 7
eCO2 [ppm]: 446
+-------------------------------------+
TVOC [ppb]: 7
eCO2 [ppm]: 446
+-------------------------------------+
TVOC [ppb]: 7
eCO2 [ppm]: 446
+-------------------------------------+
```
</details>
- `tests/driver_sht3x`
<details>
```
main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven)
SHT3X test application
+------------Initializing------------+
Initialization successful
+--------Starting Measurements--------+
Temperature [°C]: 21.46
Relative Humidity [%]: 54.50
+-------------------------------------+
Temperature [°C]: 21.47
Relative Humidity [%]: 54.53
+-------------------------------------+
Temperature [°C]: 21.46
Relative Humidity [%]: 54.48
+-------------------------------------+
Temperature [°C]: 21.46
Relative Humidity [%]: 54.47
+-------------------------------------+
```
</details>
- `tests/driver_l3gxxxx`
<details>
```
main(): This is RIOT! (Version: 2023.04-devel-375-g75547-cpu/gd32v/periph_i2c_interrupt_driven)
L3GXXXX gyroscope driver test application
Initializing L3GXXXX sensor
[OK]
gyro [dps] x: +0, y: -1, z: -2
gyro [dps] x: +0, y: +0, z: +0
gyro [dps] x: +0, y: +0, z: +0
gyro [dps] x: +0, y: +0, z: +0
gyro [dps] x: +0, y: +0, z: +0
gyro [dps] x: +0, y: +0, z: +0
gyro [dps] x: -1, y: +0, z: +4
gyro [dps] x: +0, y: +0, z: -21
gyro [dps] x: +0, y: +0, z: +6
gyro [dps] x: -43, y: +0, z: -13
gyro [dps] x: -21, y: -2, z: +0
gyro [dps] x: +0, y: +1, z: +3
gyro [dps] x: +25, y: +0, z: +0
```
</details>
- `tests/driver_hd44780` with `pcf8574a` I2C interface
### Issues/PRs references
19286: cpu/esp_common: use generic WIFI_SSID/WIFI_PASS defines r=benpicco a=benpicco
Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
Co-authored-by: Benjamin Valentin <benpicco@beuth-hochschule.de>
|
This PR was included in a batch that was canceled, it will be automatically retried |
|
bors cancel |
|
Canceled. |
|
bors merge |
|
Already running a review |
bors bot
added a commit
that referenced
this pull request
Feb 17, 2023
19027: sys/fmt: optimize scn_u32_dec scn_u32_hex r=benpicco a=kfessel ### Contribution description Improves the compilation result for `scn_u32_dec` `scn_u32_hex` especially on `cortex-m` reducing either stack usage and or code size. This makes use of unsigned int overflow (slightly less better without doing that `hexn`). See godbolt (original versions got an `o` attached, modified versions got `k`s) all functions are marked `_S_` defined to `static` by assigning the global at end the compiled function can be changed (`deco deck hexo hexk hexkk hexn`) this PR is `hexkk` and `deck` ### Testing procedure run unit-test/test-fmt ``` <RIOT>/tests/unittests$ make tests-fmt <RIOT>/tests/unittests$ make term ``` ### Issues/PRs references [godbolt](https://godbolt.org/z/MzT1zh4q1) 19269: cpu/gd32v/periph_i2c: interrupt based driver r=benpicco a=gschorcht ### Contribution description This PR provides an interrupt-driven version of the I2C low-level driver. The existing I2C low-level driver for GDVF103 uses a busy-waiting approach where the status register is continuously polled while waiting for a certain status when sending or receiving. The MCU is thus occupied the whole time during a send or receive operation. The driver provided with this PR uses an interrupt-driven approach. This is, while waiting for a certain status when sending or receiving, the calling thread is suspended and woken up by interrupts. Since the I2C controller allows to receive up to two bytes before the application has to react, receiving a single byte, two bytes or more than two bytes needs a different handling for correct receiption. This requires a tricky implementation which distinguish a number of different case. There the driver requires 860 byte more ROM and 8 byte more RAM. ### Testing procedure The driver should work with any I2C sensor/actuator. It was tested with - `tests/driver_bmp180` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) BMP180 test application +------------Initializing------------+ Initialization successful +------------Calibration------------+ AC1: 8448 AC2: -1208 AC3: -14907 AC4: 33310 AC5: 24774 AC6: 19213 B1: 6515 B2: 49 MB: -32768 MC: -11786 MD: 2958 +--------Starting Measurements--------+ Temperature [°C]: 22.0 Pressure [hPa]: 1006.49 Pressure at see level [hPa]: 1025.55 Altitude [m]: 157 +-------------------------------------+ Temperature [°C]: 22.0 Pressure [hPa]: 1006.56 Pressure at see level [hPa]: 1025.58 Altitude [m]: 157 +-------------------------------------+ ``` </details> - `tests/driver_ccs811` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) CCS811 test application +------------Initializing------------+ +--------Starting Measurements--------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ ``` </details> - `tests/driver_sht3x` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) SHT3X test application +------------Initializing------------+ Initialization successful +--------Starting Measurements--------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.50 +-------------------------------------+ Temperature [°C]: 21.47 Relative Humidity [%]: 54.53 +-------------------------------------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.48 +-------------------------------------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.47 +-------------------------------------+ ``` </details> - `tests/driver_l3gxxxx` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-375-g75547-cpu/gd32v/periph_i2c_interrupt_driven) L3GXXXX gyroscope driver test application Initializing L3GXXXX sensor [OK] gyro [dps] x: +0, y: -1, z: -2 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: -1, y: +0, z: +4 gyro [dps] x: +0, y: +0, z: -21 gyro [dps] x: +0, y: +0, z: +6 gyro [dps] x: -43, y: +0, z: -13 gyro [dps] x: -21, y: -2, z: +0 gyro [dps] x: +0, y: +1, z: +3 gyro [dps] x: +25, y: +0, z: +0 ``` </details> - `tests/driver_hd44780` with `pcf8574a` I2C interface ### Issues/PRs references 19284: boards: support for the LILYGO TTGO T8 ESP32-S2 board r=benpicco a=gschorcht ### Contribution description This PR provides the support for the LILYGO TTGO T8 ESP32-S2 board which has a OLED display (not yet supported) and a SD-Card slot on board. The board is equipped with a USB-C connector that connects either to a USB-to-UART bridge or to the USB-OTG/JTAG interface of the ESP32-S2 via some DIP switches. The PR includes a very small fix of printf format string in `tests/malloc`. I can split it off. ### Testing procedure t.b.d. ### Issues/PRs references 19286: cpu/esp_common: use generic WIFI_SSID/WIFI_PASS defines r=benpicco a=benpicco Co-authored-by: Karl Fessel <karl.fessel@ovgu.de> Co-authored-by: Gunar Schorcht <gunar@schorcht.net> Co-authored-by: Benjamin Valentin <benpicco@beuth-hochschule.de>
|
Build failed (retrying...): |
bors bot
added a commit
that referenced
this pull request
Feb 17, 2023
19027: sys/fmt: optimize scn_u32_dec scn_u32_hex r=benpicco a=kfessel ### Contribution description Improves the compilation result for `scn_u32_dec` `scn_u32_hex` especially on `cortex-m` reducing either stack usage and or code size. This makes use of unsigned int overflow (slightly less better without doing that `hexn`). See godbolt (original versions got an `o` attached, modified versions got `k`s) all functions are marked `_S_` defined to `static` by assigning the global at end the compiled function can be changed (`deco deck hexo hexk hexkk hexn`) this PR is `hexkk` and `deck` ### Testing procedure run unit-test/test-fmt ``` <RIOT>/tests/unittests$ make tests-fmt <RIOT>/tests/unittests$ make term ``` ### Issues/PRs references [godbolt](https://godbolt.org/z/MzT1zh4q1) 19269: cpu/gd32v/periph_i2c: interrupt based driver r=benpicco a=gschorcht ### Contribution description This PR provides an interrupt-driven version of the I2C low-level driver. The existing I2C low-level driver for GDVF103 uses a busy-waiting approach where the status register is continuously polled while waiting for a certain status when sending or receiving. The MCU is thus occupied the whole time during a send or receive operation. The driver provided with this PR uses an interrupt-driven approach. This is, while waiting for a certain status when sending or receiving, the calling thread is suspended and woken up by interrupts. Since the I2C controller allows to receive up to two bytes before the application has to react, receiving a single byte, two bytes or more than two bytes needs a different handling for correct receiption. This requires a tricky implementation which distinguish a number of different case. There the driver requires 860 byte more ROM and 8 byte more RAM. ### Testing procedure The driver should work with any I2C sensor/actuator. It was tested with - `tests/driver_bmp180` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) BMP180 test application +------------Initializing------------+ Initialization successful +------------Calibration------------+ AC1: 8448 AC2: -1208 AC3: -14907 AC4: 33310 AC5: 24774 AC6: 19213 B1: 6515 B2: 49 MB: -32768 MC: -11786 MD: 2958 +--------Starting Measurements--------+ Temperature [°C]: 22.0 Pressure [hPa]: 1006.49 Pressure at see level [hPa]: 1025.55 Altitude [m]: 157 +-------------------------------------+ Temperature [°C]: 22.0 Pressure [hPa]: 1006.56 Pressure at see level [hPa]: 1025.58 Altitude [m]: 157 +-------------------------------------+ ``` </details> - `tests/driver_ccs811` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) CCS811 test application +------------Initializing------------+ +--------Starting Measurements--------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ ``` </details> - `tests/driver_sht3x` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) SHT3X test application +------------Initializing------------+ Initialization successful +--------Starting Measurements--------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.50 +-------------------------------------+ Temperature [°C]: 21.47 Relative Humidity [%]: 54.53 +-------------------------------------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.48 +-------------------------------------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.47 +-------------------------------------+ ``` </details> - `tests/driver_l3gxxxx` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-375-g75547-cpu/gd32v/periph_i2c_interrupt_driven) L3GXXXX gyroscope driver test application Initializing L3GXXXX sensor [OK] gyro [dps] x: +0, y: -1, z: -2 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: -1, y: +0, z: +4 gyro [dps] x: +0, y: +0, z: -21 gyro [dps] x: +0, y: +0, z: +6 gyro [dps] x: -43, y: +0, z: -13 gyro [dps] x: -21, y: -2, z: +0 gyro [dps] x: +0, y: +1, z: +3 gyro [dps] x: +25, y: +0, z: +0 ``` </details> - `tests/driver_hd44780` with `pcf8574a` I2C interface ### Issues/PRs references Co-authored-by: Karl Fessel <karl.fessel@ovgu.de> Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
|
Kconfig doesn't like the timer selection bors cancel |
|
Canceled. |
To be honest, dependency modelling is becoming more and more a nightmare in Kconfig. Why the hell do compiled modules differ for Kconfig if the |
Huh, why do I have to select diff --git a/cpu/gd32v/Kconfig b/cpu/gd32v/Kconfig
index e732c6eb85..43714dc786 100644
--- a/cpu/gd32v/Kconfig
+++ b/cpu/gd32v/Kconfig
@@ -30,7 +30,7 @@ config CPU_FAM_GD32V
select MODULE_PERIPH_CLIC if TEST_KCONFIG
select MODULE_PERIPH_WDT if MODULE_PERIPH_PM && HAS_PERIPH_WDT
- select MODULE_ZTIMER_USEC if MODULE_PERIPH_I2C
+ select ZTIMER_USEC if MODULE_PERIPH_I2C
select PACKAGE_NMSIS_SDK
menu "GD32V configuration" |
|
I'm hoping that laze will turn into a viable replacement with a less painful migration path, but we'll see. |
Is it already decided to switch to |
|
I sqashed it directly. Let's try again. bors merge |
|
No, nothing is decided yet, I'm just equally frustrated with the Kconfig migration and look for a silver lining. |
You didn't push it yet though 😉 |
bors bot
added a commit
that referenced
this pull request
Feb 17, 2023
19027: sys/fmt: optimize scn_u32_dec scn_u32_hex r=benpicco a=kfessel ### Contribution description Improves the compilation result for `scn_u32_dec` `scn_u32_hex` especially on `cortex-m` reducing either stack usage and or code size. This makes use of unsigned int overflow (slightly less better without doing that `hexn`). See godbolt (original versions got an `o` attached, modified versions got `k`s) all functions are marked `_S_` defined to `static` by assigning the global at end the compiled function can be changed (`deco deck hexo hexk hexkk hexn`) this PR is `hexkk` and `deck` ### Testing procedure run unit-test/test-fmt ``` <RIOT>/tests/unittests$ make tests-fmt <RIOT>/tests/unittests$ make term ``` ### Issues/PRs references [godbolt](https://godbolt.org/z/MzT1zh4q1) 19269: cpu/gd32v/periph_i2c: interrupt based driver r=gschorcht a=gschorcht ### Contribution description This PR provides an interrupt-driven version of the I2C low-level driver. The existing I2C low-level driver for GDVF103 uses a busy-waiting approach where the status register is continuously polled while waiting for a certain status when sending or receiving. The MCU is thus occupied the whole time during a send or receive operation. The driver provided with this PR uses an interrupt-driven approach. This is, while waiting for a certain status when sending or receiving, the calling thread is suspended and woken up by interrupts. Since the I2C controller allows to receive up to two bytes before the application has to react, receiving a single byte, two bytes or more than two bytes needs a different handling for correct receiption. This requires a tricky implementation which distinguish a number of different case. There the driver requires 860 byte more ROM and 8 byte more RAM. ### Testing procedure The driver should work with any I2C sensor/actuator. It was tested with - `tests/driver_bmp180` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) BMP180 test application +------------Initializing------------+ Initialization successful +------------Calibration------------+ AC1: 8448 AC2: -1208 AC3: -14907 AC4: 33310 AC5: 24774 AC6: 19213 B1: 6515 B2: 49 MB: -32768 MC: -11786 MD: 2958 +--------Starting Measurements--------+ Temperature [°C]: 22.0 Pressure [hPa]: 1006.49 Pressure at see level [hPa]: 1025.55 Altitude [m]: 157 +-------------------------------------+ Temperature [°C]: 22.0 Pressure [hPa]: 1006.56 Pressure at see level [hPa]: 1025.58 Altitude [m]: 157 +-------------------------------------+ ``` </details> - `tests/driver_ccs811` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) CCS811 test application +------------Initializing------------+ +--------Starting Measurements--------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ ``` </details> - `tests/driver_sht3x` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) SHT3X test application +------------Initializing------------+ Initialization successful +--------Starting Measurements--------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.50 +-------------------------------------+ Temperature [°C]: 21.47 Relative Humidity [%]: 54.53 +-------------------------------------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.48 +-------------------------------------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.47 +-------------------------------------+ ``` </details> - `tests/driver_l3gxxxx` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-375-g75547-cpu/gd32v/periph_i2c_interrupt_driven) L3GXXXX gyroscope driver test application Initializing L3GXXXX sensor [OK] gyro [dps] x: +0, y: -1, z: -2 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: -1, y: +0, z: +4 gyro [dps] x: +0, y: +0, z: -21 gyro [dps] x: +0, y: +0, z: +6 gyro [dps] x: -43, y: +0, z: -13 gyro [dps] x: -21, y: -2, z: +0 gyro [dps] x: +0, y: +1, z: +3 gyro [dps] x: +25, y: +0, z: +0 ``` </details> - `tests/driver_hd44780` with `pcf8574a` I2C interface ### Issues/PRs references Co-authored-by: Karl Fessel <karl.fessel@ovgu.de> Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
|
Build failed (retrying...): |
bors bot
added a commit
that referenced
this pull request
Feb 17, 2023
19269: cpu/gd32v/periph_i2c: interrupt based driver r=gschorcht a=gschorcht
### Contribution description
This PR provides an interrupt-driven version of the I2C low-level driver.
The existing I2C low-level driver for GDVF103 uses a busy-waiting approach where the status register is continuously polled while waiting for a certain status when sending or receiving. The MCU is thus occupied the whole time during a send or receive operation.
The driver provided with this PR uses an interrupt-driven approach. This is, while waiting for a certain status when sending or receiving, the calling thread is suspended and woken up by interrupts.
Since the I2C controller allows to receive up to two bytes before the application has to react, receiving a single byte, two bytes or more than two bytes needs a different handling for correct receiption. This requires a tricky implementation which distinguish a number of different case. There the driver requires 860 byte more ROM and 8 byte more RAM.
### Testing procedure
The driver should work with any I2C sensor/actuator. It was tested with
- `tests/driver_bmp180`
<details>
```
main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven)
BMP180 test application
+------------Initializing------------+
Initialization successful
+------------Calibration------------+
AC1: 8448
AC2: -1208
AC3: -14907
AC4: 33310
AC5: 24774
AC6: 19213
B1: 6515
B2: 49
MB: -32768
MC: -11786
MD: 2958
+--------Starting Measurements--------+
Temperature [°C]: 22.0
Pressure [hPa]: 1006.49
Pressure at see level [hPa]: 1025.55
Altitude [m]: 157
+-------------------------------------+
Temperature [°C]: 22.0
Pressure [hPa]: 1006.56
Pressure at see level [hPa]: 1025.58
Altitude [m]: 157
+-------------------------------------+
```
</details>
- `tests/driver_ccs811`
<details>
```
main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven)
CCS811 test application
+------------Initializing------------+
+--------Starting Measurements--------+
TVOC [ppb]: 0
eCO2 [ppm]: 0
+-------------------------------------+
TVOC [ppb]: 0
eCO2 [ppm]: 0
+-------------------------------------+
TVOC [ppb]: 0
eCO2 [ppm]: 0
+-------------------------------------+
TVOC [ppb]: 0
eCO2 [ppm]: 400
+-------------------------------------+
TVOC [ppb]: 0
eCO2 [ppm]: 400
+-------------------------------------+
TVOC [ppb]: 0
eCO2 [ppm]: 400
+-------------------------------------+
TVOC [ppb]: 7
eCO2 [ppm]: 446
+-------------------------------------+
TVOC [ppb]: 7
eCO2 [ppm]: 446
+-------------------------------------+
TVOC [ppb]: 7
eCO2 [ppm]: 446
+-------------------------------------+
TVOC [ppb]: 7
eCO2 [ppm]: 446
+-------------------------------------+
```
</details>
- `tests/driver_sht3x`
<details>
```
main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven)
SHT3X test application
+------------Initializing------------+
Initialization successful
+--------Starting Measurements--------+
Temperature [°C]: 21.46
Relative Humidity [%]: 54.50
+-------------------------------------+
Temperature [°C]: 21.47
Relative Humidity [%]: 54.53
+-------------------------------------+
Temperature [°C]: 21.46
Relative Humidity [%]: 54.48
+-------------------------------------+
Temperature [°C]: 21.46
Relative Humidity [%]: 54.47
+-------------------------------------+
```
</details>
- `tests/driver_l3gxxxx`
<details>
```
main(): This is RIOT! (Version: 2023.04-devel-375-g75547-cpu/gd32v/periph_i2c_interrupt_driven)
L3GXXXX gyroscope driver test application
Initializing L3GXXXX sensor
[OK]
gyro [dps] x: +0, y: -1, z: -2
gyro [dps] x: +0, y: +0, z: +0
gyro [dps] x: +0, y: +0, z: +0
gyro [dps] x: +0, y: +0, z: +0
gyro [dps] x: +0, y: +0, z: +0
gyro [dps] x: +0, y: +0, z: +0
gyro [dps] x: -1, y: +0, z: +4
gyro [dps] x: +0, y: +0, z: -21
gyro [dps] x: +0, y: +0, z: +6
gyro [dps] x: -43, y: +0, z: -13
gyro [dps] x: -21, y: -2, z: +0
gyro [dps] x: +0, y: +1, z: +3
gyro [dps] x: +25, y: +0, z: +0
```
</details>
- `tests/driver_hd44780` with `pcf8574a` I2C interface
### Issues/PRs references
Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
|
Build failed: |
gschorcht
force-pushed
the
cpu/gd32v/periph_i2c_interrupt_driven
branch
from
3fd04b7 to
5433dac
Compare
Indeed. |
gschorcht
added
CI: ready for build
|
bors merge |
|
Build succeeded: |
|
@benpicco Thanks for reviewing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contribution description
This PR provides an interrupt-driven version of the I2C low-level driver.
The existing I2C low-level driver for GDVF103 uses a busy-waiting approach where the status register is continuously polled while waiting for a certain status when sending or receiving. The MCU is thus occupied the whole time during a send or receive operation.
The driver provided with this PR uses an interrupt-driven approach. This is, while waiting for a certain status when sending or receiving, the calling thread is suspended and woken up by interrupts.
Since the I2C controller allows to receive up to two bytes before the application has to react, receiving a single byte, two bytes or more than two bytes needs a different handling for correct receiption. This requires a tricky implementation which distinguish a number of different case. There the driver requires 860 byte more ROM and 8 byte more RAM.
Testing procedure
The driver should work with any I2C sensor/actuator. It was tested with
tests/driver_bmp180tests/driver_ccs811tests/driver_sht3xtests/driver_l3gxxxxtests/driver_hd44780withpcf8574aI2C interfaceIssues/PRs references