Skip to content

ST: Change the LSI_VALUE according to documentation #11454

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 18, 2019
Merged

ST: Change the LSI_VALUE according to documentation #11454

merged 1 commit into from
Sep 18, 2019

Conversation

Tharazi97
Copy link
Contributor

Description

Changed the LSI_VALUE of STM32F1, STM32L0, STM32L1 to match the typical value of their documentation.

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@jamesbeyond @maciejbocianski @fkjagodzinski @jeromecoutant

Release Notes

@ciarmcom
Copy link
Member

@Tharazi97, thank you for your changes.
@fkjagodzinski @maciejbocianski @jamesbeyond @ARMmbed/mbed-os-maintainers please review.

@LMESTM
Copy link
Contributor

LMESTM commented Sep 11, 2019

@Tharazi97 can we ask why you're doing this change ? Have you met any test issue for instance ?

@Tharazi97
Copy link
Contributor Author

@LMESTM I was looking for informations about LSI for this PR #11203 and I found some inconsistency.

jeromecoutant
jeromecoutant reviewed Sep 11, 2019
View reviewed changes
targets/TARGET_STM/TARGET_STM32F1/device/stm32f1xx_hal_conf.h
@@ -114,7 +114,7 @@ extern "C" {
* @brief Internal Low Speed oscillator (LSI) value.
*/
#if !defined (LSI_VALUE)
#define LSI_VALUE 32000U /*!< LSI Typical Value in Hz */
#define LSI_VALUE 40000U /*!< LSI Typical Value in Hz */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

targets/TARGET_STM/TARGET_STM32F1/device/stm32f1xx_ll_rcc.h
@@ -120,7 +120,7 @@ typedef struct
#endif /* LSE_VALUE */

#if !defined (LSI_VALUE)
#define LSI_VALUE 32000U /*!< Value of the LSI oscillator in Hz */
#define LSI_VALUE 40000U /*!< Value of the LSI oscillator in Hz */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

targets/TARGET_STM/TARGET_STM32L0/device/stm32l0xx_hal_conf.h Outdated
@@ -132,7 +132,7 @@
* @brief Internal Low Speed oscillator (LSI) value.
*/
#if !defined (LSI_VALUE)
#define LSI_VALUE (37000U) /*!< LSI Typical Value in Hz*/
#define LSI_VALUE (38000U) /*!< LSI Typical Value in Hz*/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reference Manual says: The clock frequency is around 37 kHz
Data Sheet adds details about the value: min 26 / Typ 38 / Max 56
So you could change value from 37 to 38, this will not change anything... Some boards will be at 26, other at 56...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right and on some diagrams it's marked as 40kHz. Won't change anything, but it will be consistent to data sheet.

@LMESTM
Copy link
Contributor

LMESTM commented Sep 11, 2019

@Tharazi97

@LMESTM I was looking for informations about LSI for this PR #11203 and I found some inconsistency.

Ok but do we agree that this change will not be enough to meet the requirement below:
"The watchdog should trigger at or after the timeout value.".
The LSI is unprecise and can have many different values like min 26 / Typ 38 / Max 56.

To meet the requirement, you'd have to consider the worst case or introduce a precision information about the clock frequency ... ?

@Tharazi97
Copy link
Contributor Author

@LMESTM Yes, that won't be enough, this is only to keep consistency between datasheet and implementation.
Right now I'm working on adding the information about a precision of these clocks frequency.

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 17, 2019

Right now I'm working on adding the information about a precision of these clocks frequency.

@ARMmbed/team-st-mcd can you please review. Should we close this one or go separately and the other PR #11203 implements the proper fix.

@jeromecoutant
Copy link
Collaborator

I think this PR could be closed.

@Tharazi97
Copy link
Contributor Author

Shouldn't we at least keep the changes to F1 series?

@jeromecoutant
Copy link
Collaborator

Looks not needed with #11203 change I can see.
But if you keep this F1 change, I will approve it :-)

@Tharazi97
Copy link
Contributor Author

Done

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 18, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Sep 18, 2019

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_dynamic-memory-usage

@Tharazi97
Copy link
Contributor Author

No fails in CI build artefacts

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 18, 2019

CI restarted

@0xc0170 0xc0170 merged commit 83fca60 into ARMmbed:master Sep 18, 2019
This was referenced Sep 20, 2019
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 approved these changes

@jeromecoutant jeromecoutant jeromecoutant approved these changes

@fkjagodzinski fkjagodzinski Awaiting requested review from fkjagodzinski

@jamesbeyond jamesbeyond Awaiting requested review from jamesbeyond

@maciejbocianski maciejbocianski Awaiting requested review from maciejbocianski

Assignees
No one assigned
Labels
release-version: 5.14.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Tharazi97 @ciarmcom @LMESTM @0xc0170 @jeromecoutant @mbed-ci