STM32U5 ADC: Selected channels are not reset between reads

[JoelSchofield]

Describe the bug Performing an ADC read on a channel will leave that channel activated in the channel select register (e.g. ADC4->CHSEL). This will cause issues when attempting to read from a new channel, as the hardware will attempt to read from both.

To Reproduce This can be seen using the adc driver demo. I modified the nucleo_u575zi_q.overlay to use ADC4 and include another channel:

/ {
    zephyr,user {
        /* adjust channel number according to pinmux in board.dts */
        io-channels = <&adc4 2>, <&adc4 3>;
    };
};

&adc4 {
    #address-cells = <1>;
    #size-cells = <0>;

    channel@2 {
        reg = <2>;
        zephyr,gain = "ADC_GAIN_1";
        zephyr,reference = "ADC_REF_INTERNAL";
        // Note the acquisiton time is set long to help demonstrate ISR firing multiple times for single request.
        zephyr,acquisition-time = <ADC_ACQ_TIME_MAX>;
        zephyr,resolution = <12>;
    };

    channel@3 {
        reg = <3>;
        zephyr,gain = "ADC_GAIN_1";
        zephyr,reference = "ADC_REF_INTERNAL";
        zephyr,acquisition-time = <ADC_ACQ_TIME_MAX>;
        zephyr,resolution = <12>;
    };
};

Then build the project with:

west build --pristine -b nucleo_u575zi_q zephyr/samples/drivers/adc

Running the project youll see the following:

1. first call to adc_read reading Channel 2:
   • ADC4->CHSEL register == 0x04 (correct)
   • adc_stm32_isr is hit once for the single conversion.
2. second call to adc_read reading Channel 8:
   • ADC4->CHSEL register == 0x0c (incorrect, should be 0x08)
   • adc_stm32_isris hit twice, once for each channel. The destination buffer will overflow here from second read.

Expected behavior A new read should not be impacted by any previous reads. Channels should be cleared before setting up the new read.

Workaround: Reset the selected channels at the start of the start_read func in adc_stm32.c:

static int start_read(const struct device *dev,
              const struct adc_sequence *sequence)
{
    ...

#if defined(CONFIG_SOC_SERIES_STM32U5X)
    // Note I've only tested with STM32U5 ADC4
    if (adc == ADC4) {
        LL_ADC_REG_SetSequencerChannels(adc, 0);
    }
#endif

    ...
}

Environment:

• OS: MacOS
• Toolchain: Zephyr SDK,
• Zephyr v3.4

Additional info: This issue might be present on other STM32 families as well. I dont any other family of boards to test, however a quick look over the start_read func doesnt seem to have any calls to reset the channels, and only calls to add new channels.

[github-actions[bot]]

Hi @JoelSchofield! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

[gautierg-st]

Hello @JoelSchofield, I have started working on this point, to make sampling several channels at once available on all series, and I've hit this same overrun problem on all series I've tested. After some test I found that it is possible (at least on some series, I've not tested them all yet) to avoid it by setting an appropriate prescaler for the ADC clock.

So first I need this PR #59874 merged, and then I will work again on this part. And this sequencer reset will definitely be part of it ;)

[gautierg-st]

Hi @JoelSchofield, I've pushed PR #62436 to be able to use the sequencer on all series, it should solve the lack of reset that you brought up. If you ever run a sequence and encounter an overrun, I would advise you to fiddle with the acquisition time (increasing it), or with the new clock and prescaler properties (to reduce the clock speed). I would welcome your feedback.

[JoelSchofield]

Thanks @gautierg-st!

I can confirm ADCs that are NOT_FULLY_CONFIGURABLE has this issue fixed for me. The call to LL_ADC_REG_SetSequencerChannels with channel_mask will clear any previous channels that may have been set.

For the FULLY_CONFIGURABLE ADCs I'm wondering if theres any issues from not clearing the ADC_PCSEL and ADC_SQRx registers. Say for the following:

1. Perform a two-channel read sequence with ADC1, e.g. using channels 5 and 6.
2. Then, perform a single channel read with ADC1 using channel 7.

For step 2 the ADC_PCSEL will still have the bits set for channels 5 and 6, as well as setting the bit for channel 7. The ADC_SQR1 register will also have data in the SQ2 bits from the previous sequence, however the L (length) bits are configured correctly so I guess thats fine and SQ2 bits would be ignored.

Are there any downsides to having channels selected with ADC_PCSEL that arent used for a sequence?

Thanks again for all your help and contributions :)

[gautierg-st]

Thanks for your feedback. I'm glad it works for you. You're right about PCSEL, that's an oversight on my part. I don't think it can have an impact functionality-wise, but I suppose it could negatively impact the power consumption. I'll reset it once conversion is complete. For the SQRx registers, it is like you say, the length is what matters, what is after that length have no impact.