-
Notifications
You must be signed in to change notification settings - Fork 8.3k
RFC: ADC: add support for injected channels for STM32 #99787
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
base: main
Are you sure you want to change the base?
RFC: ADC: add support for injected channels for STM32 #99787
Conversation
9e9f4ab to
bbd2b6c
Compare
drivers/adc/adc_stm32.c
Outdated
| LL_ADC_ClearFlag_JEOS(adc); | ||
| for (uint8_t i = 0; i < data->channel_count_injected; i++) { | ||
| *data->buffer_injected++ = LL_ADC_INJ_ReadConversionData32(adc, | ||
| table_rank_injected[i]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add an indentation tab?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
drivers/adc/Kconfig.stm32
Outdated
| config ADC_STM32_INJECTED_CHANNELS | ||
| bool "STM32 ADC Injected channels support" | ||
| select ADC_INJECTED_CHANNELS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If ADC_INJECTED_CHANNELS is part of generic ADC API, it should be possible to use in a portable application and hence I'd expect the dependency to be in the other way round:
User selects ADC_INJECTED_CHANNELS in his application and ADC_STM32_INJECTED_CHANNELS is selected in turn.
Alternatively ADC_STM32_INJECTED_CHANNELS may not be required at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
|
||
| #if ANY_ADC_SEQUENCER_TYPE_IS(SEQUENCER_PROGRAMMABLE) | ||
| if (config->sequencer_type == SEQUENCER_PROGRAMMABLE) { | ||
| #if ANY_ADC_HAS_CHANNEL_PRESELECTION |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part of the change is unclear. Should it be in a dedicated commit ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Writing to the preselection register cannot be done if any sequence (regular or injected) is on-going. So it is necessary to set it up at the "channel setup" stage rather than at the start stage. That's also why this register is not reset when sampling is done (when injected is enabled).
That said, the code should be kept here when injected is not enabled, or else some current tests/samples will fail (because presel will be reset at the end of the sampling, and channel_setup is not necessarily called each time before starting conversions)
mathieuchopstm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems overall OK; some comments:
drivers/adc/adc_stm32.c
Outdated
| #if ANY_ADC_HAS_CHANNEL_PRESELECTION | ||
| if (config->has_channel_preselection) { | ||
| /* | ||
| * Each channel in the sequence must be previously enabled in PCSEL. | ||
| * This register controls the analog switch integrated in the IO | ||
| * level. | ||
| */ | ||
| uint32_t channel = __LL_ADC_DECIMAL_NB_TO_CHANNEL(channel_cfg->channel_id); | ||
|
|
||
| LL_ADC_SetChannelPreselection(adc, channel); | ||
| } | ||
| #endif /* ANY_ADC_HAS_CHANNEL_PRESELECTION */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how this change relates to injected channels. Maybe it should be moved to a dedicated commit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Writing to the preselection register cannot be done if any sequence (regular or injected) is on-going. So it is necessary to set it up at the "channel setup" stage rather than at the start stage. That's also why this register is not reset when sampling is done (when injected is enabled).
| depends on SOC_FAMILY_STM32 && !SOC_SERIES_STM32F1X | ||
| select ADC | ||
| help | ||
| Enable driver for STM32 Vbat sensor and then also ADC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Outside PR scope: inconsistency here vs. other drivers
|
@anangl Unfortunately we miss and ADC maintainer. As the unique contributor to this subsystem, would you mind having a look to the api change and see if acceptable ? |
| TC_PRINT("Samples read: "); | ||
| for (i = 0; i < BUFFER_SIZE; i++) { | ||
| adc_data_size_t sample_value = m_sample_buffer[i]; | ||
| for (i = 0; i < MAX(BUFFER_SIZE, expected_count); i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will not test values were not filled above expected_count offset for inject channels' large buffer.
Should pass the buffer size as argument and use instead of BUFFER_SIZE here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
bbd2b6c to
8e70d05
Compare
956a746 to
b2be6ee
Compare
Add injected mode support in the ADC API. For hardware supporting it, this allows to inject an ADC sequence in the middle of another one. The injected sequence takes precedence over the regular sequence, interrupting it to make its own conversions. Once the injected sequence is finished, the regular sequence will start back from where it was interrupted. Signed-off-by: Guillaume Gautier <[email protected]>
b2be6ee to
e50014e
Compare
|
Last push was to fix conflicts for the release notes. |
e50014e to
f5386d9
Compare
2ea924e to
0c20dfc
Compare
mathieuchopstm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise LGTM
drivers/adc/Kconfig.stm32
Outdated
| bool | ||
| def_bool y |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| bool | |
| def_bool y | |
| def_bool y |
(def_bool y is equivalent to bool followed by default y)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Add injected mode support for STM32 ADC driver. To avoid potential conflicts when configuring the ADC registers, some settings (like presel) are moved into the channel_setup function when injected is enabled. Also configures an ISR when DMA and injected mode are enabled. The reason is that DMA is not available for injected sequence, so an ISR is still necessary in this case. Factorizes the "start_read_xxx" functions to avoid needless duplication between sync/async and regular/injected mode. Signed-off-by: Guillaume Gautier <[email protected]>
In order to write into some ADC registers, it is necessary to have no on-going conversions (either regular, injected, or both). Until now, each time such a write was needed, the ADC was disabled unconditionally, stopping any potential on-going conversion. With the introduction of the injected mode, this could lead to problems if for example an injected is configured differently than a running regular one. In that case it would stop the regular conversion. If the application is waiting the result before starting another one, it will be stuck waiting forever. To avoid cases like this, return an error rather than stopping conversion and disabling the ADC. This reworks many functions to return an error in case it was not possible to safely disable the ADC. It stays possible to force an ADC disable even though a conversion is running, this is useful at init where we don't want this to fail. This also creates a dedicated function for the preselection register to avoid duplication. Signed-off-by: Guillaume Gautier <[email protected]>
Add injected mode support for STM32 temperature sensor. When in injected mode, this configures the sensor channel and sensor enable bit at start to prevent any conflict later: some registers can't be written while a conversion is on-going. Signed-off-by: Guillaume Gautier <[email protected]>
Add injected mode support for STM32 Vbat sensor. When in injected mode, this configures the sensor channel and sensor enable bit at start to prevent any conflict later: some registers can't be written while a conversion is on-going. Signed-off-by: Guillaume Gautier <[email protected]>
Add injected mode support for STM32 Vref sensor. When in injected mode, this configures the sensor channel and sensor enable bit at start to prevent any conflict later: some registers can't be written while a conversion is on-going. Signed-off-by: Guillaume Gautier <[email protected]>
Defines the io-injected-channels property to be able to define a seconde sequence used for injected channels. Signed-off-by: Guillaume Gautier <[email protected]>
Add a test for injected channels. The test first starts a regular asynchronous sampling of several channels, then starts an injected sampling. The test checks that both the regular and injected samplings run without issues. Signed-off-by: Guillaume Gautier <[email protected]>
The ADC has a new API to enable injected mode (Kconfig + struct member). Signed-off-by: Guillaume Gautier <[email protected]>
0c20dfc to
4064a88
Compare
|



This PR adds the support for the STM32 ADC injected channels. Starting an injected sequence means that the sequence is executed immediately. If a regular sequence is being executed, it will be paused and resumed once the injected one is done.
It makes a slight change to the ADC API by adding a boolean in the
adc_sequencestructure to indicate if the sequence should be injected or not. This boolean is itself hidden behind a specific Kconfig. That way, there is no difference for ADC that do not support this feature.An ADC test is added to test the feature on an STM32U5 board. The PR is functional but the implementation stays open for discussion.