ChibiOS Free Embedded RTOS

Posted: **Fri Jan 30, 2026 2:49 pm**

Hi,

context : last ChibiOS 21.11 stable and hit a compile error when calling bsioReadI().

File: os/hal/lib/complex/buffered_sio/hal_buffered_sio.h

Current macro:

Code: Select all

  #define bsioReadI(bsiop, b, n) iqReadI(&(bsiop)->iqueue, b, n, TIME_INFINITE)

But iqReadI() is declared as:

Code: Select all

 size_t iqReadI(input_queue_t *iqp, uint8_t *bp, size_t n);

So the macro passes one extra argument and fails to compile
It seems the macro should be either:

Code: Select all

  #define bsioReadI(bsiop, b, n) iqReadI(&(bsiop)->iqueue, b, n)

or (if timeout is intended, then in thread context):

Code: Select all

  #define bsioReadI(bsiop, b, n) iqReadTimeout(&(bsiop)->iqueue, b, n, TIME_INFINITE)

I worked around it locally by calling iqReadI() directly.

I am converting a project that use SD and USART driver to only use SIO, so I will probably discover another subtleties, should I create a new topic each time, or is it better to keep all SIO/BSIO report in the same topic ?

Alexandre

Posted: **Fri Jan 30, 2026 2:54 pm**

You may use this one if you find more problems.

Giovanni

Posted: **Sat Jan 31, 2026 12:27 am**

You may use this one if you find more problems.

I did not, but the the new trendy thing did ... Not trying to paraphrase, I copy paste here the finding :

• Findings (ordered by severity)

- Critical: sioAsyncReadX / sioAsyncWriteX macro argument order is reversed.
- sio_lld_read / sio_lld_write signatures are (SIODriver*, uint8_t *buffer, size_t n) but macros pass (siop, size, buffer), so callers pass a pointer as size and size_t as buffer.
- File: ChibiOS_21.11_stable/os/hal/include/hal_sio.h:437 and :453.

- High: bsioStart() doesn’t restore siop->arg after sioStop() clears it.
- sioStop() sets siop->arg = NULL (hal_sio.c:301–305).
- bsioStart() only sets the callback, not arg (hal_buffered_sio.c:202–207).
- __bsio_default_cb() dereferences siop->arg without a NULL check (hal_buffered_sio.c:70–72).
- Result: stop → start → first IRQ dereferences NULL in ISR.
- Files: hal_sio.c:301–305, hal_buffered_sio.c:70–72, hal_buffered_sio.c:202–207.

- Low: extension field guards are misspelled (*_FIELS).
- SIO_CONFIG_EXT_FIELS / SIO_DRIVER_EXT_FIELS won’t match the expected *_FIELDS macro name, so user extensions are never included.
- File: hal_sio.h:171–173, :243–245.

I also have, not a bug report, but a small change request
Using SIO with continuous DMA, I'm using the SIO_EV_RXIDLE event to detect the end of a frame (variable length). My callback looks something like this:

Code: Select all

    static void sioIdleCb(SIODriver *siop) {
      // ... handle DMA partial transfer ...
   
      // Need to clear the hardware IDLE flag before re-enabling the event
      siop->usart->ICR = USART_ICR_IDLECF; // <--- Direct hardware access
   
      sioSetEnableFlagsX(siop, SIO_EV_RXIDLE);
    }

The Issue:
Currently, there is no portable way to clear a specific event flag in the SIO API.
* sioGetAndClearEventsX(siop) exists, but it is a "read-and-clear-all" operation, I would prefer to selectively clear one event (IDLE here).
* Directly writing to ICR violates usage of abstractions

Proposal:
add a sioClearEvents API to the SIO driver

High Level (hal_sio.h):

Code: Select all

 /**
     * @brief   Clears specific SIO event flags.
     * @param[in] siop      pointer to the @p SIODriver object
     * @param[in] mask      event flags to be cleared
     * @xclass
     */
    #define sioClearEventsX(siop, mask) sio_lld_clear_events(siop, mask)

Low Level (STM32 LLD example):

Code: Select all

   void sio_lld_clear_events(SIODriver *siop, sioevents_t mask) {
      uint32_t icr = 0;
      if ((mask & SIO_EV_RXIDLE) != 0U) icr |= USART_ICR_IDLECF;
      if ((mask & SIO_EV_TXDONE) != 0U) icr |= USART_ICR_TCCF;
     // ... map other flags ...
      siop->usart->ICR = icr;
    }

This would allow for cleaner, more portable application code:

Code: Select all

    sioClearEventsX(siop, SIO_EV_RXIDLE);
    sioSetEnableFlagsX(siop, SIO_EV_RXIDLE);

This is just a proposal, and any abstraction will be fine for me.

Alexandre

Posted: **Sat Jan 31, 2026 7:14 am**

Hi,

My trendy thing (Codex-CLI) found this:

- High: os/hal/src/hal_sio.c:121 and os/hal/src/hal_sio.c:146 return
sioSynchronizeRX() results directly; when it returns SIO_MSG_ERRORS (defined
as positive 1 in os/hal/include/hal_sio.h:83), the stream API (__get/__gett)
returns 0x01 as if it were valid data and does not clear/read the FIFO,
which can silently corrupt input on RX error events. Consider treating
SIO_MSG_ERRORS like sync_read does (clear + retry) or mapping to a negative
error code.
- Medium: os/hal/include/hal_sio.h:171 and os/hal/include/hal_sio.h:243 guard
extension fields with SIO_CONFIG_EXT_FIELS / SIO_DRIVER_EXT_FIELS (typo). If
users define the expected *_FIELDS macros (consistent with other HAL
drivers), their extra fields never compile in.
- Low: os/hal/include/hal_sio.h:423–os/hal/include/hal_sio.h:453 documents
sioAsyncReadX/sioAsyncWriteX as (siop, size, buffer) and uses that parameter
naming, while the non-X API and all LLDs are (siop, buffer, n). Current call
sites pass (buffer, n) so it expands “correctly,” but the signature/docs are
misleading and easy to misuse.

Should have done a pass earlier but I was focusing on LLDs mainly.

Giovanni

Posted: **Sat Jan 31, 2026 7:58 am**

Hi,

Committed all fixes, please try again. I am doing a mass check on all HAL/XHAL HLDs.

Giovanni

Posted: **Sat Jan 31, 2026 9:57 am**

Committed all fixes, please try again. I am doing a mass check on all HAL/XHAL HLDs.

The comparison between my patched stable and trunk seems good, I will port my firmware (https://github.com/alex31/paparazzi_minican) on trunk and run on actual hardware on monday and report results.

What is your feeling about adding a sioClearEvents API to the SIO driver ?

A.

Posted: **Sat Jan 31, 2026 11:54 am**

Frankly seems redundant, you could just alias sioGetAndClearEventsX().

Giovanni

Posted: **Mon Feb 02, 2026 4:50 pm**

Made some real tests with trunk, it works fine, although I get some compiler warnings :

Code: Select all

os/hal/ports/STM32/STM32G4xx/hal_efl_lld.c: In function 'stm32_flash_dual_bank':
os/hal/ports/STM32/STM32G4xx/hal_efl_lld.c:114:56: warning: unused parameter 'eflp' [-Wunused-parameter]
  114 | static inline bool stm32_flash_dual_bank(EFlashDriver *eflp) {
      |                                          ~~~~~~~~~~~~~~^~~~
os/hal/ports/STM32/STM32G4xx/hal_efl_lld.c: In function 'efl_lld_read':
os/hal/ports/STM32/STM32G4xx/hal_efl_lld.c:245:36: warning: pointer of type 'void *' used in arithmetic [-Wpointer-arith]
  245 |                                    + offset, n);
      |                                    ^

In function 'usart_init',
    inlined from 'sio_lld_start' at ../../../../ChibiOS_mirror/os/hal/ports/STM32/LLD/USARTv3/hal_sio_lld.c:555:3:
os/hal/ports/STM32/LLD/USARTv3/hal_sio_lld.c:322:29: warning: 'clock' may be used uninitialized [-Wmaybe-uninitialized]
  322 |     brr = (uint32_t)((clock / presc) / siop->config->baud);
      |                      ~~~~~~~^~~~~~~~
os/hal/ports/STM32/LLD/USARTv3/hal_sio_lld.c: In function 'sio_lld_start':
os/hal/ports/STM32/LLD/USARTv3/hal_sio_lld.c:240:24: note: 'clock' was declared here
  240 |   uint32_t presc, brr, clock, cr3;

Frankly seems redundant, you could just alias sioGetAndClearEventsX().

I just re plugged my brain to think a little bit, and yes, you are right :-)

A.

ChibiOS Free Embedded RTOS

incorrect macro in buffered SIO

incorrect macro in buffered SIO

Re: incorrect macro in buffered SIO

Re: incorrect macro in buffered SIO

Re: incorrect macro in buffered SIO

Re: incorrect macro in buffered SIO

Re: incorrect macro in buffered SIO

Re: incorrect macro in buffered SIO

Re: incorrect macro in buffered SIO