add to the stm32-adc internal channel handling #34458
Conversation
changes: - inside the dts the channel 17 can be set up as internal channel - this is done via adding "ch17-internal;" to the adc1 node - the "ch17-internal" flag enables the internal channel routing at the stm32 adc driver note: - this is only tested for the stm32f412 MCU Signed-off-by: Stefan Jaritz <stefan@kokoontech.com>
changed wrong sample
drivers/adc/adc_stm32.c
Outdated
| @@ -248,6 +248,18 @@ static int check_buffer_size(const struct adc_sequence *sequence, | |||
| return 0; | |||
| } | |||
|
|
|||
| static uint32_t adc_stm32_getChannelByIndx(const uint8_t index) { | |||
| uint32_t channel = __LL_ADC_DECIMAL_NB_TO_CHANNEL(index); | |||
| #if DT_N_S_soc_S_adc_40012000_P_ch17_internal == 1 | |||
There was a problem hiding this comment.
You know this part is soc specific, right ?
There was a problem hiding this comment.
I only know the stm32f412 MCU so I don't know how to make my code fitting to others st archs.
In general this code is a demo to show, how to add the internal channels to the stm32f412 by using the dts and some minor code patch of the stm32 adc driver.
I am happy to get some advice and execute the changes to make this commit better ;)
drivers/adc/adc_stm32.c
Outdated
| @@ -716,6 +728,13 @@ static int adc_stm32_init(const struct device *dev) | |||
| } | |||
| #endif | |||
|
|
|||
| // handle internal channels | |||
| #if DT_N_S_soc_S_adc_40012000_P_ch17_internal == 1 | |||
There was a problem hiding this comment.
I will leave the SoC specific parts of this to @erwango, but I am -1 on using the generated devicetree_unfixed.h macros directly. They are implementation details. Use the documented macro APIs provided in devicetree.h only please.
There was a problem hiding this comment.
@mbolivar-nordic Sure. We'll first review the pure driver implementation first (to make sure the proposed changes make sense), then we'll focus on getting the change compliant (and simply functional) vs DT API.
There was a problem hiding this comment.
fyi
I changed the mentioned lines. Now using dts macros
I also used C comments instead of C++ comments
changes:
- the internal channels are now switched on, if
the soc is an stm32f4xx and ch17_internal is
set.
| * for now only the channel 17 (VRef internal) | ||
| * is implemented | ||
| */ | ||
| #if defined(CONFIG_SOC_SERIES_STM32F4X) |
There was a problem hiding this comment.
If using DT macros to control the enabled code, you don't need this anymore.
| * is implemented | ||
| */ | ||
| #if defined(CONFIG_SOC_SERIES_STM32F4X) | ||
| #if (DT_PROP(DT_NODELABEL(adc1), ch17_internal) == 1) |
There was a problem hiding this comment.
Is that only valid for adc1 ?
Also, one problem raised here is that this piece of code will be applied whatever the instance in use, as long as ch17_internal is defined for adc1.
I'd advice to store the information about ch17_internal in a device specific struct (adc_stm32_cfg),
and check for the info in that structure in run time. So you ensure that channel setting is done only on the expected instance (be it adc1, adc2, adcx or any combination)
There was a problem hiding this comment.
thx for the comment
I just dived into DT and its macros. So I am still beginner - if you have an better idea pls let me know.
Storing the internal/external channel info must be handled at one instance. I used macros that are fixed at compile time. The adc_stm32_cfg is a good idea, because its constant and therefore the compiler can resolve this const expr into static code. So that should not cost any performance (because the channel setup is done for every conversation). An other topic is in general the adc API, what translates the sequence into the device specific adc sequence for EVERY adc_read call. IMHO that is a bad design ;)
|
@erwango I quickly checked #34673 From a perspective of an stm32f412 this implementation is missing 2 things:
The commit does not explicit solves the problem that the F4 adc channels are multiplexed (internal/external). The current driver assumes that all channels are external. As long as there is no definition for that via dts/kconfig etc. the driver can't set internal channels up. Maybe we should start there first. My time is limited and I have only a stm32f412 custom board here. So no chance for me for stm32Fx/stm32Lx testing :( |
drivers: adc: enable the stm32 adc driver VRef via internal channel
changes:
- inside the dts the channel 17 can be set up as internal channel
- this is done via adding "ch17-internal;" to the adc1 node
- the "ch17-internal" flag enables the internal channel routing
at the stm32 adc driver
note:
- this is only tested for the stm32f412 MCU
Signed-off-by: Stefan Jaritz stefan@kokoontech.com