ROM constants address defines for ADC calibration values not defined as address of const data

[unsigned_char_array]

I noticed that the ROM constants for ADC calibration values in the LL headers (*_ll_adc.h) are not defined as addresses of const data.

e.g.

 

#define VREFINT_CAL_ADDR ((uint16_t*) (0x1FF07A2A))
#define TEMPSENSOR_CAL1_ADDR ((uint16_t*) (0x1FF07A2C))
#define TEMPSENSOR_CAL2_ADDR ((uint16_t*) (0x1FF07A2E))

 

And it should be:

 

#define VREFINT_CAL_ADDR ((const uint16_t*) (0x1FF07A2A))
#define TEMPSENSOR_CAL1_ADDR ((const uint16_t*) (0x1FF07A2C))
#define TEMPSENSOR_CAL2_ADDR ((const uint16_t*) (0x1FF07A2E))

 

Trying to write to it probably results in hardfault errors, but it should raise compile errors too in my opinion.

[gbm]

For me, the more interesting aspect is why these are defined as pointers instead of constant data:

#define VREFINT_CAL (*(volatile const uint16_t*) (0x1FF07A2A))
#define TEMPSENSOR_CAL1 (*(volatile const uint16_t*) (0x1FF07A2C))
#define TEMPSENSOR_CAL2 (*(volatile const uint16_t*) (0x1FF07A2E))

[unsigned_char_array]

---

@gbm wrote:

 why these are defined as pointers instead of constant data

Because they are not compile time constants, but ROM constants (run-time constants). The compiler doesn't know their values. Or do you mean that they could be defined using an attribute to be in a constant data section defined in the linker file?

I don't think they need to be marked as volatile as these values cannot change during run of the program and order of reading doesn't matter. So if they are read once and then assumed unchanging by the compiler it should be fine. const volatile would only make sense for gpio input registers for instance since they can change.

[Saket_Om]

Hello @unsigned_char_array  and @gbm 

Thank you for bringing this to our attention.

I have reported your requests internally and will get back to you as soon as possible.

Internal ticket number: 202867 (This is an internal tracking number and is not accessible or usable by customers).

Thank you.

[David0384]

---

@unsigned_char_array wrote:

I noticed that the ROM constants for ADC calibration values in the LL headers (*_ll_adc.h) are not defined as const.

e.g.

 

#define VREFINT_CAL_ADDR ((uint16_t*) (0x1FF07A2A))
#define TEMPSENSOR_CAL1_ADDR ((uint16_t*) (0x1FF07A2C))
#define TEMPSENSOR_CAL2_ADDR ((uint16_t*) (0x1FF07A2E))

 

 

And it should be:

 

#define VREFINT_CAL_ADDR ((const uint16_t*) (0x1FF07A2A))
#define TEMPSENSOR_CAL1_ADDR ((const uint16_t*) (0x1FF07A2C))
#define TEMPSENSOR_CAL2_ADDR ((const uint16_t*) (0x1FF07A2E))

 

In some microcontroller architectures, ADC calibration values stored in ROM are not defined as const pointers, leading to potential unintended modifications. This can result in unexpected behavior or incorrect ADC readings if the values are altered during runtime. Defining them as const ensures they remain immutable, preserving calibration accuracy. Proper handling of these pointers prevents accidental writes and maintains system reliability. Always check the microcontroller’s reference manual to verify how calibration values should be accessed safely.

trying to write to it probably results in hard fault errors, but I think it should raise compile errors too.

---

 

[Saket_Om]

Hello @unsigned_char_array 

Defining a constant pointer means that the pointer points to a fixed memory location and cannot be modified. However, the value inside this location can usually be modified during runtime.

The value inside these variables defined with '#define' will not be stored in a memory region. It will just be replaced by their value during the compilation steps

 

[unsigned_char_array]

---

@Saket_Om wrote:

Hello @unsigned_char_array 

Defining a constant pointer means that the pointer points to a fixed memory location and cannot be modified.

---

First of all the define is not a pointer. It is a macro containing an address (an integer typecasted with as an address type). And a constant pointer is different than a pointer to a constant anyway.
Since the data currently is not defined as const it means the compiler won't complain if it is modified. That's the issue I have with it. Trying to modify this ROM value should result in a compile error.

 

---

@Saket_Om wrote:

 However, the value inside this location can usually be modified during runtime.

---

Correct. The const keyword just means you only have read access. It doesn't mean it cannot change. Though in this case it will remain constant for the run of the program unless there is a serious silicon bug.

 

---

@Saket_Om wrote:

The value inside these variables defined with '#define' will not be stored in a memory region. It will just be replaced by their value during the compilation steps

---

These are not variables. Neither the define, nor the address of the ROM value is a variable. You are right the define won't be stored in memory as it is not a pointer. It will be inlined wherever it is used. The compiler will turn de-referencing the define into a memory access.
In peripheral definitions structs are used with "__I" example for read-only members e.g.:

 

 

typedef struct
{
...
 __I uint32_t PCSR; /* Offset: 0x01C (R/ ) Program Counter Sample Register */
...
} DWT_Type;

#define DWT_BASE (0xE0001000UL) /*!< DWT Base Address */


#define DWT ((DWT_Type *) DWT_BASE ) /*!< DWT configuration struct */

 

 

Using a struct type would be even cleaner.

[Saket_Om]

Hello @unsigned_char_array 

This issue has been resolved, and the fix is now available on GitHub. 

[gbm]

@Saket_Om  So how about putting this into every main MCU header file (not some LL or HAL header):

#define SYSROM_ADDR	0x1fff0000	// System ROM base address
// Calibration values stored in ROM
#define VREFINT_CAL_mV	3000u	// calibration voltage
#define VREFINT_CAL	(*(const uint16_t *)0x1fff756a)

#define TS_CAL1	(*(const uint16_t *)0x1fff7568)	// at 30 C
#define TS_CAL1_T	30

#define TS_CAL2	(*(const uint16_t *)0x1fff75xx)	// at 110 C
#define TS_CAL2_T	110

@unsigned_char_arrayYou're right, volatile is an overkill here.