Skip to content
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

sys/fmt: optimize scn_u32_dec scn_u32_hex #19027

Merged
merged 1 commit into from
Feb 17, 2023
Merged

Conversation

kfessel
Copy link
Contributor

@kfessel kfessel commented Dec 8, 2022

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 ks) 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

@github-actions github-actions bot added the Area: sys Area: System label Dec 8, 2022
@kfessel kfessel added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: low priority If set, builds of this PR will be queued behind others labels Dec 8, 2022
@kfessel kfessel requested a review from maribu December 8, 2022 13:20
@kaspar030
Copy link
Contributor

Improves the compilation result for scn_u32_dec scn_u32_hex especially on cortex-m reducing either stack usage and or code size.

Comparing just the code size of tests-fmt, my gcc 12.2.0 produces identical size w/wo this PR. How did you measure the stack usage?

In godbolt, do I compare e.g., scn_u32_hexo with scn_u32_hexk?

@kaspar030 kaspar030 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Dec 8, 2022
@kfessel
Copy link
Contributor Author

kfessel commented Dec 8, 2022

this pr is hexkk (compare to hexo) and deck (compare to deco)

arm-gcc 12.2(linux) and arm-gcc 11.2.1 (none) -Os -mcpu=cortex-m4

scn_u32_hexkk(char const*, unsigned int):
        mov     r2, r0
        add     r1, r1, r0
        movs    r0, #0
.L2:
        cmp     r2, r1
        bne     .L7
.L1:
        bx      lr
.L7:
        ldrb    r3, [r2], #1    @ zero_extendqisi2
        cmp     r3, #57
        bhi     .L3
        subs    r3, r3, #48
.L4:
        orr     r0, r3, r0, lsl #4
        b       .L2
.L3:
        cmp     r3, #70
        bhi     .L5
        subs    r3, r3, #55
        b       .L4
.L5:
        cmp     r3, #102
        bhi     .L1
        subs    r3, r3, #87
        b       .L4

vs

scn_u32_hexo(char const*, unsigned int):
        push    {r4, r5, lr}
        mov     r2, r0
        add     r1, r1, r0
        movs    r0, #0
.L2:
        cmp     r2, r1
        bne     .L7
.L1:
        pop     {r4, r5, pc}
.L7:
        ldrb    r3, [r2], #1    @ zero_extendqisi2
        sub     r4, r3, #48
        uxtb    r5, r4
        cmp     r5, #9
        bls     .L3
        sub     r4, r3, #65
        cmp     r4, #25
        itt     ls
        addls   r3, r3, #32
        uxtbls  r3, r3
        subs    r4, r3, #1
        cmp     r4, #101
        bhi     .L1
        subs    r3, r3, #87
        orr     r0, r3, r0, lsl #4
        b       .L2
.L3:
        orr     r0, r4, r0, lsl #4
        b       .L2

there is obvious stack usage (push pop) by hexo and non by hexk

Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See inline. Please squash right away.

To me, the code is roughly equally readable. So let's go with what the compiler likes more.

sys/fmt/fmt.c Outdated Show resolved Hide resolved
sys/fmt/fmt.c Outdated Show resolved Hide resolved
sys/fmt/fmt.c Outdated Show resolved Hide resolved
sys/fmt/fmt.c Outdated Show resolved Hide resolved
sys/fmt/fmt.c Outdated
char c = *str++;
if (!fmt_is_digit(c)) {
unsigned char c = *str++;
unsigned d = c - '0';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least in C99, char literals are of type int, so this does not in fact make use of unsigned integer overflows, but instead unleashes undefined behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

many thanks for pointing that out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to trigger a UB sanatizer that and i could not.
Found out it is not UB but a promotion issue (-> it is well defined to go wrong since if c is unsigned char it may be promoted to int and the literal being int is just the icing to trigger that).

I also ran the test before I uploaded this PR but the char was a addition to the code after that.

sys/fmt/fmt.c Outdated
res *= 10;
res += (c - '0');
}
res *= 10;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This integer literal is signed as well; undefined behavior again.

@riot-ci
Copy link

riot-ci commented Dec 8, 2022

Murdock results

✔️ PASSED

18a783d sys/fmt: optimize scn_u32_dec scn_u32_hex

Success Failures Total Runtime
6865 0 6865 11m:33s

Artifacts

@Teufelchen1
Copy link
Contributor

Teufelchen1 commented Dec 8, 2022

Please note, we got lucky that murdock catched this one. The fmt unittest is lacking in quality. Maybe this PR can also work on that for the affected two functions?


Maribu wrote:

To me, the code is roughly equally readable.

I would disagree; I perceived this as a downgrade in readability regards.

@kfessel
Copy link
Contributor Author

kfessel commented Dec 8, 2022

I would disagree; I perceived this as a downgrade in readability regards.

I changed for the n version which i think more readable than the original and yields better code

@pyropeter please have another look for UB

Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see inline

sys/fmt/fmt.c Outdated Show resolved Hide resolved
sys/fmt/fmt.c Outdated Show resolved Hide resolved
sys/fmt/fmt.c Outdated Show resolved Hide resolved
sys/fmt/fmt.c Outdated
d = c - 'A' + 0xa;
}
else if ( 'a' <= c && c <= 'f' ) {
d = c - 'a' + 0xa;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
d = c - 'a' + 0xa;
d = c - (unsigned)'a' + 0xa;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there shall never be an overflow (passed the condition) -> this is defined even if a is promoted to int.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could add it for peace of mind

@kfessel kfessel force-pushed the p-fmt-scn-opt branch 2 times, most recently from 439425a to 2b27f00 Compare December 9, 2022 13:46
@kfessel
Copy link
Contributor Author

kfessel commented Dec 9, 2022

added the peace of mind interger Suffixes and unsigned casts

@benpicco
Copy link
Contributor

benpicco commented Jan 3, 2023

Please squash

@benpicco
Copy link
Contributor

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>
@bors
Copy link
Contributor

bors bot commented Feb 17, 2023

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)

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


Co-authored-by: Karl Fessel <karl.fessel@ovgu.de>
Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
@bors
Copy link
Contributor

bors bot commented Feb 17, 2023

This PR was included in a batch that was canceled, it will be automatically retried

@benpicco
Copy link
Contributor

bors merge

@bors
Copy link
Contributor

bors bot commented Feb 17, 2023

Already running a review

@benpicco
Copy link
Contributor

bors cancel
bors merge

@bors
Copy link
Contributor

bors bot commented Feb 17, 2023

Canceled.

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)

Co-authored-by: Karl Fessel <karl.fessel@ovgu.de>
@benpicco
Copy link
Contributor

bors cancel
bors merge

@bors
Copy link
Contributor

bors bot commented Feb 17, 2023

Canceled.

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>
@bors
Copy link
Contributor

bors bot commented Feb 17, 2023

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>
@bors
Copy link
Contributor

bors bot commented Feb 17, 2023

This PR was included in a batch that was canceled, it will be automatically retried

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>
@bors
Copy link
Contributor

bors bot commented Feb 17, 2023

Build failed (retrying...):

@bors
Copy link
Contributor

bors bot commented Feb 17, 2023

Build succeeded:

@bors bors bot merged commit 8c2a4b4 into RIOT-OS:master Feb 17, 2023
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: sys Area: System CI: low priority If set, builds of this PR will be queued behind others CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants