InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.68k stars 916 forks source link

Access MMIO without going through HAL or drivers #1599

Open szsam opened 1 year ago

szsam commented 1 year ago

Verification

Introduce the issue

I have noticed that some code accesses memory-mapped I/O (MMIO) without going through Hardware Abstraction Layer (HAL) or drivers. This type of access may have several drawbacks, including poor portability and reusability.

I would like to open this issue to investigate why some MMIO access does not follow the conventional path through HAL or drivers. Are there specific scenarios where non-conventional MMIO access is preferred or a must (e.g. because, for example, SDK does not provide the necessary API)? Is it possible to fix the code by accessing MMIO conventionally (i.e. through HAL/drivers)?

Code snippets with MMIO access that does not go through HAL or drivers:

  1. NRF_TIMER2->CC[0] in https://github.com/InfiniTimeOrg/InfiniTime/blob/d968bcb1f3afd2364558e41013dbd2d6d5c286aa/src/main.cpp#L345
  2. https://github.com/InfiniTimeOrg/InfiniTime/blob/d968bcb1f3afd2364558e41013dbd2d6d5c286aa/src/components/firmwarevalidator/FirmwareValidator.cpp#L8-L11 On the contrary, FirmwareValidator::Validate() uses driver API to write to validBitAdress: https://github.com/InfiniTimeOrg/InfiniTime/blob/d968bcb1f3afd2364558e41013dbd2d6d5c286aa/src/components/firmwarevalidator/FirmwareValidator.cpp#L15

Preferred solution

No response

Version

No response

JF002 commented 1 year ago

NRF_TIMER2->CC[0] : Is used to store the version of the bootloader : the bootloader write its own version in that register, and the application reads that register before doing anything else with this register or TIM2. In this case, we would rather not use a HAL driver that might clear the register before we have the chance to read the value of the register.

validBitValue is set when the image is validated (this is used by the bootloader to automatically revert to the previous image if the current one is not validated). Since the internal flash memory is mapped into the memory range, we can read it the same way we read variables in RAM, so we don't need any specific driver to read the value. However, special care must be taken when writing it (disable IRQ, wait loop) which was implemented in Pinetime::Drivers::InternalFlash::WriteWord.

Both of them can (and probably should if we want to support multiple hardware) obviously be implemented in their own drivers (abstraction).

szsam commented 1 year ago

Thanks for your explanation.

In this case, we would rather not use a HAL driver that might clear the register before we have the chance to read the value of the register.

Could you clarify which HAL/driver API might clear the register before we have the chance to read?

JF002 commented 1 year ago

The timer driver from the NRF SDK, I guess.