samples: drivers: adc: adc_sequence: wrong usage of adc_raw_to_millivolts

[nordic-piks]

Describe the bug

Looking at: https://github.com/zephyrproject-rtos/zephyr/blob/main/samples/drivers/adc/adc_sequence/src/main.c#L85C11-L85C32

                err = adc_raw_to_millivolts(channel_cfgs[channel_index].reference,
                                channel_cfgs[channel_index].gain,
                                CONFIG_SEQUENCE_RESOLUTION, &val_mv);

Function adc_raw_to_millivolts is used with value which is a enum (passed as a first argument) channel_cfgs[channel_index].reference https://github.com/zephyrproject-rtos/zephyr/blob/main/include/zephyr/drivers/adc.h#L95

struct adc_channel_cfg {
    /** Gain selection. */
    enum adc_gain gain;

    /** Reference selection. */
    enum adc_reference reference;

With values:

enum adc_reference {
    ADC_REF_VDD_1,     /**< VDD. */
    ADC_REF_VDD_1_2,   /**< VDD/2. */
    ADC_REF_VDD_1_3,   /**< VDD/3. */
    ADC_REF_VDD_1_4,   /**< VDD/4. */
    ADC_REF_INTERNAL,  /**< Internal. */
    ADC_REF_EXTERNAL0, /**< External, input 0. */
    ADC_REF_EXTERNAL1, /**< External, input 1. */
};

https://github.com/zephyrproject-rtos/zephyr/blob/main/include/zephyr/drivers/adc.h#L77

However adc_raw_to_millivolts expects as a first arguments millivolts value:

static inline int adc_raw_to_millivolts(int32_t ref_mv,
                    enum adc_gain gain,
                    uint8_t resolution,
                    int32_t *valp)

 * @param ref_mv the reference voltage used for the measurement, in
 * millivolts.  This may be from adc_ref_internal() or a known
 * external reference.

https://github.com/zephyrproject-rtos/zephyr/blob/main/include/zephyr/drivers/adc.h#L878

Which lead to incorrect conversion from adc readings into millivolts for this sample.

Expected behavior Possible solution would be to obtain correct reference value as in adc_raw_to_millivolts_dt: https://github.com/zephyrproject-rtos/zephyr/blob/main/include/zephyr/drivers/adc.h#L916C1-L920C3

    if (spec->channel_cfg.reference == ADC_REF_INTERNAL) {
        vref_mv = (int32_t)adc_ref_internal(spec->dev);
    } else {
        vref_mv = spec->vref_mv;
    }

however, with this approach only ADC_REF_INTERNAL can be handled easily. Other values, especially external reference are handled in adc_dt with vref_mv definition, but such is missing at adc_channel_cfg.

Impact Local, only this sample is affected.

Environment (please complete the following information):

• v3.7.0-rc2-109-gcac70373aa81

[nordic-piks]

@wkhadgar Please have a look on that.

[wkhadgar]

Please have a look on that.

Greatly noticed, will take on look on that and make the refered fixes!

[wkhadgar]

@nordic-piks I would argue that moving the rogue values of adc_dt_spec like resolution and vref_mv to adc_channel_cfg would be a good move, but the implications are at large scale, so not sure if it's the best choice here. As of now, fixed the issue by creating an auxiliary array of vrefs, since it's an exclusively local change and easier to maintain.

[wkhadgar]

@anangl @decsny What are your thoughts on this?

[nordic-piks]

@nordic-piks I would argue that moving the rogue values of adc_dt_spec like resolution and vref_mv to adc_channel_cfg would be a good move, but the implications are at large scale, so not sure if it's the best choice here. As of now, fixed the issue by creating an auxiliary array of vrefs, since it's an exclusively local change and easier to maintain.

I like your choice! Already tested with HW (with internal ref), working fine.