Skip to content

Unified magic number storage? #13

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
fpistm opened this issue May 20, 2019 · 11 comments
Closed

Unified magic number storage? #13

fpistm opened this issue May 20, 2019 · 11 comments

Comments

@fpistm
Copy link

fpistm commented May 20, 2019

Hi @Serasidis

First of all, thank you very much for your excellent work!

Currently, STM32F1 handle the magic number thanks the battery backed register (DR10):

uint16_t value = READ_REG(BKP->DR10);

and STM32F4 thanks the battery backed SRAM region at 0.

magic_val = *(__IO uint32_t *) (BKPSRAM_BASE);

I wonder if it would not be better to unified them and use only battery backed register which is available across all series while backup SRAM not ?

And in this case, this could use a common DRx index across all series, DR4 should be fine.
This allow to unified cores support.

@Serasidis
Copy link
Owner

Serasidis commented May 21, 2019

Thanks @fpistm !

I did some tests on F407 mcu + Arduino IDE 1.8.9 + STM Core, to use the backed register instead of using the battery backed SRAM region.
It seems the following function does not work correct. It is not pointing to the correct register address.
For example, the following code (modified file stm32f4xx_hal_rtc_ex.c) should return the register address 0x40002860 that is the register RTC_BKP_DR4 address. instead of this address, it returns the value 0x60.

uint32_t magic_val = HAL_RTCEx_BKUPRead(&hrtc, RTC_BKP_DR4);
magic_val should have value 0x40002860 but it has 0x60

uint32_t HAL_RTCEx_BKUPRead(RTC_HandleTypeDef *hrtc, uint32_t BackupRegister)
{
  uint32_t tmp = 0U;
  
  /* Check the parameters */
  assert_param(IS_RTC_BKP(BackupRegister));

  tmp = (uint32_t)&(hrtc->Instance->BKP0R);
  tmp += (BackupRegister * 4U);
  
  /* Read the specified register */
  return tmp; //(*(__IO uint32_t *)tmp);
}

When I try to write directly to the register address the value I write is saved successfully to the register.

This reads successfully the RTC_BKP_DR4 register value
uint32_t magic_val = *(__IO uint32_t *) (0x40002850 + (RTC_BKP_DR4 * 4U));

I have also successfully wrote the RTC_BKP_DR4 register by directly writing.

...
HAL_PWR_EnableBkUpAccess();
*(__IO uint32_t *) (0x40002850 + (RTC_BKP_DR4 * 4U)) = 0x12345678;
HAL_PWR_DisableBkUpAccess();
....

Maybe I am missing something ...

@fpistm
Copy link
Author

fpistm commented May 21, 2019

Humm... right strange behaviour.
I'm using the LL access to avoid this kind of issue:

set

https://github.com/stm32duino/Arduino_Core_STM32/blob/9f757b44954e60db665aa41ed904d5e8e0073bd4/system/Drivers/STM32F4xx_HAL_Driver/Inc/stm32f4xx_ll_rtc.h#L3051

get

https://github.com/stm32duino/Arduino_Core_STM32/blob/9f757b44954e60db665aa41ed904d5e8e0073bd4/system/Drivers/STM32F4xx_HAL_Driver/Inc/stm32f4xx_ll_rtc.h#L3089

You can check here how I handle this in the core:
https://github.com/stm32duino/Arduino_Core_STM32/blob/master/cores/arduino/stm32/backup.h#L102

I use it for HID bootloader for STM32F1 without any issue on the PR stm32duino/Arduino_Core_STM32#525:

#if defined (HIDBL_F1)
        setBackupRegister(HID_MAGIC_NUMBER_BKP_INDEX, HID_MAGIC_NUMBER_BKP_VALUE);
#endif

So if HID BL for F4 use the same backup register, I do not need handle specific access across series ;)

@Serasidis
Copy link
Owner

Serasidis commented May 21, 2019

So if HID BL for F4 use the same backup register, I do not need handle specific access across series ;)

It is not needed to the new HID-BL version.

Please, do not upload any code related to the HID-BL to the repo. I have made a lot of changes so far to my local HID-BL copy. When I finish it, we will discuss it and send you a PR.

@fpistm
Copy link
Author

fpistm commented May 21, 2019

Ok.
That's why I've split the PR from @BennehBoy:
One commit to add Maple BL support, the second the HID BL ;)

@Serasidis
Copy link
Owner

Thanks !

@Serasidis
Copy link
Owner

The LL_RTC_BAK_SetRegister and LL_RTC_BAK_GetRegister did the job !
Now it Reads/Writes to the correct address (0x40002860)

@Serasidis
Copy link
Owner

  • From now on, HIB BL uses the battery-backed DR4 register on all STM32 MCUs.
  • The magic word 1EAF has been removed. It's used a DTR pulse loop instead, to let the USB Serial reception work better.

Tests were successful on Bluepill F103 and DIYMROE STM32F407VGT boards

@fpistm
Copy link
Author

fpistm commented May 22, 2019

Nice. Great job @Serasidis

@BennehBoy
Copy link

No change required to the DTR/magic word handling in the core then I guess, magic word still applies to Maple DFU, and DTR will continue to function...

@Serasidis
Copy link
Owner

Serasidis commented May 24, 2019

@BennehBoy , The reboot feature that is based only on DTR signal works ok in both DFU Bootloader 2.0 and HID bootloader as-is (without modifying any bootloader file).

For avoiding any malfunction it is better to leave this process as is (with the magic word 1EAF).

@Serasidis
Copy link
Owner

Serasidis commented Jun 11, 2019

From now on, the DR4 battery backup register is used on both F1 and F4 MCUs to store the Magic Number 0x424C

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants