Skip to content
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

Decouple system timer and drivers: SoC Initialization #15505

Conversation

pizi-nordic
Copy link
Collaborator

During our work in timer area, we found that system clock frequency is used incorrectly in several places.
For example, it is used to obtain CPU frequency needed for SoC initialization.
If we select different system timer device, all these calculations become invalid.

This PR fixes the problem by obtaining all needed information directly from the DTS.
Brings us closer to: #15363

@@ -44,6 +44,7 @@

#include <stdint.h>
#include <ti/devices/msp432p4xx/inc/msp.h>
#include <generated_dts_board.h>
Copy link
Member

Choose a reason for hiding this comment

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

I am not so happy with including generated headers in ext.
Could this be done in soc.h, instead?

Copy link
Member

Choose a reason for hiding this comment

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

that's what we do for nRF

Copy link
Member

@ioannisg ioannisg left a comment

Choose a reason for hiding this comment

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

Did a pass - it looks ok to me. I left a comment about including auto-generated headers.
The PR does not include changed to nRF platform. Needs review from more collaborators.

@pizi-nordic pizi-nordic force-pushed the decouple-system-timer-and-clock-soc branch from 72030f8 to 4964110 Compare April 24, 2019 11:15
@galak galak added the dev-review To be discussed in dev-review meeting label Apr 24, 2019
Copy link
Contributor

@andrewboie andrewboie left a comment

Choose a reason for hiding this comment

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

question I have...we are now setting the CPU clock in terms of DT_CPUS_CPU_0_CLOCK_FREQUENCY, but does CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC also derive from this value or is it still manually set in defconfig?

@pizi-nordic
Copy link
Collaborator Author

@andrewboie: The CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC is still set manually in defconfig. However all these changes are preparation for removal of of this define. I noticed, that it is widely used in wrong context as well as in many places we are assuming that system clock frequency is known at compile time which is not always true.

@zephyrbot
Copy link
Collaborator

zephyrbot commented Apr 26, 2019

All checks are passing now.

Review history of this comment for details about previous failed status.
Note that some checks might have not completed yet.

anangl and others added 12 commits April 26, 2019 12:37
This patch adds generation of DT_ macros for values assigned to
the `clock-frequency` property in `/cpus/cpu*` nodes.

Signed-off-by: Andrzej Głąbek <[email protected]>
The SoC initialization code used system clock frequency
as a CPU clock frequency. This commit corrects that by
obtaining the needed value from DTS.

Signed-off-by: Piotr Zięcik <[email protected]>
The SoC initialization code used system clock frequency
as a CPU clock frequency. This commit corrects that by
obtaining the needed value from DTS.

Signed-off-by: Piotr Zięcik <[email protected]>
The SoC initialization code used system clock frequency
as a CPU clock frequency. This commit corrects that by
obtaining the needed value from DTS.

Signed-off-by: Piotr Zięcik <[email protected]>
The SoC initialization code used system clock frequency
as a CPU clock frequency. This commit corrects that by
obtaining the needed value from DTS.

Signed-off-by: Piotr Zięcik <[email protected]>
The SoC initialization code used system clock frequency
as a CPU clock frequency. This commit corrects that by
obtaining the needed value from DTS.

Signed-off-by: Piotr Zięcik <[email protected]>
The SoC initialization code used system clock frequency
as a CPU clock frequency. This commit corrects that by
obtaining the needed value from DTS.

Signed-off-by: Piotr Zięcik <[email protected]>
The SoC initialization code used system clock frequency
as a CPU clock frequency. This commit corrects that by
obtaining the needed value from DTS.

Signed-off-by: Piotr Zięcik <[email protected]>
The SoC initialization code used system clock frequency
as a CPU clock frequency. This commit corrects that by
obtaining the needed value from DTS.

Signed-off-by: Piotr Zięcik <[email protected]>
The SoC initialization code used system clock frequency
as a CPU clock frequency. This commit corrects that by
obtaining the needed value from DTS.

Signed-off-by: Piotr Zięcik <[email protected]>
The SoC initialization code used system clock frequency
as a CPU clock frequency. This commit corrects that by
obtaining the needed value from DTS.

Signed-off-by: Piotr Zięcik <[email protected]>
The SoC initialization code used system clock frequency
as a CPU clock frequency. This commit corrects that by
obtaining the needed value from DTS.

Signed-off-by: Piotr Zięcik <[email protected]>
The clock control initialization code used system clock
frequency as a CPU clock frequency. This commit corrects
that by obtaining the needed value from DTS.

Signed-off-by: Piotr Zięcik <[email protected]>
The log_backend_swo used system clock frequency
as a base for SWO clock calculation. This commit
corrects that by obtaining the needed value from DTS.

Signed-off-by: Piotr Zięcik <[email protected]>
@pizi-nordic pizi-nordic force-pushed the decouple-system-timer-and-clock-soc branch from ba9f81a to d11b92d Compare April 26, 2019 10:37
@nashif nashif removed dev-review To be discussed in dev-review meeting labels May 2, 2019
@pizi-nordic pizi-nordic deleted the decouple-system-timer-and-clock-soc branch May 29, 2019 09:22
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

Successfully merging this pull request may close these issues.

7 participants