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
[MAINTAINERS] Changes to circular callbacks
- Giovanni
- Site Admin
- Posts: 14457
- Joined: Wed May 27, 2009 8:48 am
- Location: Salerno, Italy
- Has thanked: 1076 times
- Been thanked: 922 times
- Contact:
Re: [MAINTAINERS] Changes to circular callbacks
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
FYI, here is a link to a post I made on this topic a while back:
viewtopic.php?t=4414#p31788
Re: [MAINTAINERS] Changes to circular callbacks
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.
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.
- alex31
- Posts: 379
- Joined: Fri May 25, 2012 10:23 am
- Location: toulouse, france
- Has thanked: 38 times
- Been thanked: 62 times
- Contact:
Re: [MAINTAINERS] Changes to circular callbacks
Giovanni wrote :
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
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
- Giovanni
- Site Admin
- Posts: 14457
- Joined: Wed May 27, 2009 8:48 am
- Location: Salerno, Italy
- Has thanked: 1076 times
- Been thanked: 922 times
- Contact:
Re: [MAINTAINERS] Changes to circular callbacks
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
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
- alex31
- Posts: 379
- Joined: Fri May 25, 2012 10:23 am
- Location: toulouse, france
- Has thanked: 38 times
- Been thanked: 62 times
- Contact:
Re: [MAINTAINERS] Changes to circular callbacks
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
Don't you think that this consistency campaign is a good opportunity to bring half buffer callback enhancement for the UART driver ?
Alexandre
- Giovanni
- Site Admin
- Posts: 14457
- Joined: Wed May 27, 2009 8:48 am
- Location: Salerno, Italy
- Has thanked: 1076 times
- Been thanked: 922 times
- Contact:
Re: [MAINTAINERS] Changes to circular callbacks
Hi,
What is the use case of circular buffers for UART driver?
Giovanni
What is the use case of circular buffers for UART driver?
Giovanni
- alex31
- Posts: 379
- Joined: Fri May 25, 2012 10:23 am
- Location: toulouse, france
- Has thanked: 38 times
- Been thanked: 62 times
- Contact:
Re: [MAINTAINERS] Changes to circular callbacks
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
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
- Giovanni
- Site Admin
- Posts: 14457
- Joined: Wed May 27, 2009 8:48 am
- Location: Salerno, Italy
- Has thanked: 1076 times
- Been thanked: 922 times
- Contact:
Re: [MAINTAINERS] Changes to circular callbacks
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
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
- alex31
- Posts: 379
- Joined: Fri May 25, 2012 10:23 am
- Location: toulouse, france
- Has thanked: 38 times
- Been thanked: 62 times
- Contact:
Re: [MAINTAINERS] Changes to circular callbacks
Hi,
Thanks for your work !
I will have to port some of my 18.2 projects to trunk to test ...
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
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
Return to “Development and Feedback”
Who is online
Users browsing this forum: No registered users and 48 guests