NordicSemiconductor / nrfx

Standalone drivers for peripherals present in Nordic SoCs
Other
261 stars 137 forks source link

NRFX_COMP_DEFAULT_CONFIG uses wrong internal reference voltage #126

Open LastLockJoe opened 1 month ago

LastLockJoe commented 1 month ago

I was looking at the default configuration for the comparator and I noticed that the reference voltage in the comment does not match the reference voltage in the code.

/**
 * @brief COMP driver default configuration.
 *
 * This configuration sets up COMP with the following options:
 * - single-ended mode
 * - reference voltage: internal 1.8 V
 * - lower threshold: 0.5 V
 * - upper threshold: 1.5 V
 * - high speed mode
 * - hysteresis disabled
 * - current source disabled
 *
 * @param[in] _input Analog input.
 */
#define NRFX_COMP_DEFAULT_CONFIG(_input)                                           \
{                                                                                  \
    .reference          = NRF_COMP_REF_INT_1V2,                                    \
    .main_mode          = NRF_COMP_MAIN_MODE_SE,                                   \
    .threshold          = NRFX_COMP_CONFIG_TH,                                     \
    .speed_mode         = NRF_COMP_SP_MODE_HIGH,                                   \
    .hyst               = NRF_COMP_HYST_NO_HYST,                                   \
    NRFX_COND_CODE_1(NRF_COMP_HAS_ISOURCE, (.isource = NRF_COMP_ISOURCE_OFF,), ()) \
    .input              = (nrf_comp_input_t)_input,                                \
    .interrupt_priority = NRFX_COMP_DEFAULT_CONFIG_IRQ_PRIORITY                    \
}

Given the default threshold configuration uses 1.8, I assume the reference voltage should be changed to NRF_COMP_REF_INT_1V8.

/** @brief COMP threshold default configuration. */
#define NRFX_COMP_CONFIG_TH                                  \
{                                                            \
    .th_down = NRFX_COMP_VOLTAGE_THRESHOLD_TO_INT(0.5, 1.8), \
    .th_up   = NRFX_COMP_VOLTAGE_THRESHOLD_TO_INT(1.5, 1.8)  \
}

From my understanding of the comparator peripheral, setting the default internal ref to 1V8 would result in the comparator not working properly for boards with VDD = 1V8, ex: nRF7002 DK. Perhaps a warning should be added to note this?

nika-nordic commented 3 weeks ago

hello @LastLockJoe , thanks for reaching out.

Looks like we have updated default configuration settings for the COMP driver but missed the doc alignment. 1.2 V reference voltage was chosen specifically as it is a common denominator across all nRF devices.

We will update the doc in next release