ChibiOS Free Embedded RTOS

Hi,

Currently we have several drivers operating on circular buffers: ADC, DAC, I2S and SPI. Those drivers all use different conventions for callbacks, I decided to give all them the same callbacks.

I will implement the following changes:
- All drivers will have two separate callbacks, one for 1st half buffer completion, the other for 2nd half buffer completion.
- Callbacks will have no parameters except for the pointer to the driver instance.

This will improve HAL consistency and also make callbacks processing faster (no if/decisions to take in the processing path).

Suggestions? feedback?

Giovanni

Thanks for addressing this!

FYI, here is a link to a post I made on this topic a while back:
viewtopic.php?t=4414#p31788

If the callback will only take a parameter to the driver instance, the length of the circular buffer and the current pointer to the buffer will be stored in the driver instance struct then, right? Will there be two pointers stored in the driver struct, first half and second half? Or will be there be a single pointer to the buffer, and the user will have to do the the arithmetic to figure out the current "half"?

I'm okay with the current callback function definition which has three arguments, driver instance pointer, buffer pointer, and length.

As this is going in the HAL, how about those processors which don't have half transfer and transfer complete interrupts? Would the burden be on the HAL driver to emulate it, or were you thinking about a configuration symbol to constrain the behavior?

I hope the code for handling the circular callbacks will be made into a common library, and reused by the various drivers - instead of the current state of essentially the same code duplicated in multiple places.

Another thing to look into is the error handling code and conventions, as noted in the link I posted above to an old post I made.

Giovanni wrote :

I will implement the following changes:
- All drivers will have two separate callbacks, one for 1st half buffer completion, the other for 2nd half buffer completion.
- Callbacks will have no parameters except for the pointer to the driver instance.

This will improve HAL consistency and also make callbacks processing faster (no if/decisions to take in the processing path).

Hello,

Of course, improving consistency is a good thing, but i appreciate the actual implementation with one callback, i am afraid that two function definitions that will do nearly the same thing will clobber the code, probably pushing users to write a third function that does all the common work.

For the processing faster part, i am not that sure, modern compiler can do constant propagation, automatic inlining to mitigate overhead, and for the processor that have I-cache, twice the callback functions can impact performance negatively because of cache miss.

The two solutions should be benchmarked to see if there is a real advantage on the efficiency side to compensate more verbose API.

Alexandre

The other option is to have a single callback then check the "state" variable to understand if it is the first half (XXX_ACTIVE) or the second half (XXX_COMPLETE). It is what the SPI driver does right now. All impacted drivers have those states so it is just matter of removing extra parameters.

About pointer to buffer and size of the data, the application, where the callback exists, already knows those:
1st half: buffer, sizeof(buffer) / 2 / sizeof(sample_t).
2nd half: buffer + sizeof(buffer) / 2 / sizeof(sample_t), sizeof(buffer) / 2 / sizeof(sample_t).

A couple of macros/inlne_functions could be added that return those values after checking the state, for example: ADC_GET_SAMPLES_PTR(adcp, buffer), ADC_GET_SAMPLES_SIZE(adcp, buffer).
This would allow to not store pointer and size into the driver structure which would take space and slow down the driver.

About the point of DMAs not supporting half transfer interrupts, usually all DMAs have that or chained blocks or scatter gather or other similar mechanisms, all could be used to implement the half buffer thing. A DMA without such features simply could not be used for circular buffers without SW tricks (2 DMA operations with SW restart ad buffer half).

Giovanni

Ok, for me it's a good compromise.

Don't you think that this consistency campaign is a good opportunity to bring half buffer callback enhancement for the UART driver ?

Alexandre

Hi,

What is the use case of circular buffers for UART driver?

Giovanni

problem arose when trying to receive a continuous stream of data.

with the actual driver, i did not found a way to not miss bytes.

there have been a thread about this problem : exemple of using uart driver

Seems to me that a classic half buffer callback scheme should solve the problem.

For the sending part, i did not test if actual API permits ton send a stream without glitches, but if a half buffer scheme is chosen for reception, it should perhaps be symmetrical for sending.

Alexandre

Hi,

I changed ADC, DAC, I2S and SPI, now all callbacks are similar and macros are very similar, remaining differences are because intrinsic differences in drivers. I HOPE to not have introduced regressions, please report any strangeness.

About UART, I need to think about it, I am not yet convinced.

Giovanni

Hi,

I changed ADC, DAC, I2S and SPI, now all callbacks are similar and macros are very similar, remaining differences are because intrinsic differences in drivers. I HOPE to not have introduced regressions, please report any strangeness.

Thanks for your work !
I will have to port some of my 18.2 projects to trunk to test ...

About UART, I need to think about it, I am not yet convinced.

IMHO, actual driver make strong assumption on datagram oriented communication.

On the other side, a double buffer scheme is well adapted to bytes stream, less to datagram,
perhaps we need, for UART, a double buffer scheme with timout management that could be used on both use case.

Alexandre