Bluepill: hse config fix

[LordGuilly]

Notes:

Description

fixed the BLUEPILL (and likely other F1XX devices) oscillator setup
Problem described in issue #4753

Status

READY

Migrations

NO

Todos

• Tests
• Documentation

Deploy notes

as said, it may impact other F1xx branches, but the HSI (fallback of the startup procedure) was doing 64Mhz), so systems may do faster than expected

Steps to test or reproduce

I started the mbed build app in a remote target with GDB, and printed SystemCoreClock

[jeromecoutant]

I don't have BLUEPILL target,
but I could reproduce the issue with another DISCO board (F429ZI) which supports also HSE_XTAL and not HSE_EXTC.
Proposed patch is solving the issue.

[theotherjimmy]

Thanks for making the PR @LordGuilly. I really like the minimal diff.

[0xc0170]

@LordGuilly Can you please sign https://developer.mbed.org/contributor_agreement/ ? And share your nick on mbed?

[LordGuilly]

my mbed user is 'guilly' not sure if it can be linked to the github profile?

[0xc0170]

/morph test

[0xc0170]

/morph test

[mbed-bot]

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 817

All builds and test passed!

[jeromecoutant]

Hi
Note that ST driver team doesn't approve this update...

I suggest to remove the commit in stm32f1xx_hal_rcc.h file from this pul request, and to keep only the patch in targets/TARGET_STM/TARGET_STM32F1/TARGET_BLUEPILL_F103C8/device/system_stm32f1xx.c

This will make Bluepill target works anyway.

In //, I am going to work on a patch which will satisfy everyone and every STM32 family (not only F1)

Thx

[0xc0170]

I suggest to remove the commit in stm32f1xx_hal_rcc.h file from this pul request, and to keep only the patch in targets/TARGET_STM/TARGET_STM32F1/TARGET_BLUEPILL_F103C8/device/system_stm32f1xx.c

This will make Bluepill target works anyway.

@LordGuilly What do you think, can you do the update?

[LordGuilly]

what's the problem, side effects? Or is it also present in other platforms, and want to solve it for all at the same time?
Do you need me to remove the commit, or can be done automatically?

[jeromecoutant]

what's the problem, side effects?

They mention 2 points:

• in some cases, when user call twice HAL_RCC_OscConfig() with HSE On, in each call the HSE will be disabled then enabled

• after clearing HSEON and HSEBYP, and before setting HSEON, it seems that we have to wait on HSERDY= 0 to guarantee the sequence

[LordGuilly]

well, make sense, I guess the best approach would be to replace the macro with an inline call, and add all the extra logic. Doing it with a macro will not be very clear.
Disabling the HSE is needed only if HSEBYP is enabled, because the bit cannot be changed with the HSE on.
I will update the PR to have only the BLUEPILL specific commit, likely will test it tonight at home.

[LordGuilly]

reverted the commit, tested with my board, and definitely works, but only for this platform at the moment.

[theotherjimmy]

@LordGuilly I think the history would be cleaner if you just rebased/squashed and removed the commit that you reverted and the revert.

[LordGuilly]

I think I did it right this time ;-)

[theotherjimmy]

Looks great!

[0xc0170]

/morph test

[studavekar]

/morph test

[mbed-bot]

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 845

Test failed!

[studavekar]

/morph test

[mbed-bot]

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 854

All builds and test passed!

[theotherjimmy]

Sorry @LordGuilly It looks like the pile of merges that got in before this one caused a merge conflict. Could you rebase to resolve this conflict?

[LordGuilly]

done it, the file tree was restructured, and the fix had to be done in another file

[adustm]

Hello @LordGuilly
Thank you for the fix.
I only have a comment on your PR's details:

it may impact other F1xx branches,

As you modify only TARGET_STM32F1/TARGET_BLUEPILL_F103C8/device/system_clock.c, it means that only BLUEPILL_F103C8 device will be affected by your modification.
Kind regards
Armelle

[LordGuilly]

Yes, the comment is a leftover from the original PR, which also included changes in a generic f1xx header. Those changes were removed due to ST driver team objections

[theotherjimmy]

/morph test

[mbed-bot]

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 887

All builds and test passed!

[theotherjimmy]

Thanks for the fix @LordGuilly!