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

Flash_loader: increase wait rounds for slow boards #1085

Merged
merged 1 commit into from Mar 6, 2021

Conversation

seeseemelk
Copy link
Contributor

Certain systems can take a while to boot and flash (because of a slow power supply or because the core is running at a lower clock speed).
Commit b8813fa broke st-link for several of our boards. Increasing the WAIT_ROUNDS define by a little fixes the issues.

This was tested on a custom board with an STM32L471 and a custom board with an STM32L496.

@Nightwalker-87
Copy link
Member

I'll probably not change this unless there has been widespread testing with various other board types and / or programmers.
This parameter has been changed several times which not only lead to improvements but also introduced side effects.
I'll leave this open for others in the community to have their say. Should there be a common sense on this, we may change it.

@chenguokai
Copy link
Collaborator

Commit b8813fa broke st-link for several of our boards. Increasing the WAIT_ROUNDS define by a little fixes the issues.

I am a bit confused. It seems that this particular commit does not change the wait time ideally, without considering the operating system scheduler and the function call cost which may cause the sleep time to be longer than expected.

Anyway, since there are some boards that may be affected by this timing issue, I think increasing this period is fine.

Copy link
Collaborator

@Ant-ON Ant-ON left a comment

Choose a reason for hiding this comment

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

@seeseemelk I think it's better to rewrite this part to use time_ms(). This will make possible to set the timeout in ms.

unsigned time_ms();

@Nightwalker-87
Copy link
Member

@Ant-ON Sounds like a good idea to me. 👍

Release v1.7.0 automation moved this from In progress to Review in progress Feb 18, 2021
Copy link
Member

@Nightwalker-87 Nightwalker-87 left a comment

Choose a reason for hiding this comment

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

I agree on the idea to rewrite this part in order to have to have a flexible solution.

@Nightwalker-87
Copy link
Member

@slyshykO: What's your opinion on this?

@slyshykO
Copy link
Collaborator

@slyshykO: What's your opinion on this?

I am Ok with increasing wait time.

@Nightwalker-87
Copy link
Member

@seeseemelk: Are you going to address the proposal from @Ant-ON ?

@seeseemelk
Copy link
Contributor Author

I'm afraid I don't really have the time to focus on this at the moment, especially given that we are switching from ST-Links to J-Links.

Release v1.7.0 automation moved this from Review in progress to Reviewer approved Mar 6, 2021
Copy link
Member

@Nightwalker-87 Nightwalker-87 left a comment

Choose a reason for hiding this comment

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

I'm going to let this pass, but shall there be any regression bug reports, that point back to this, we should not continue to tune this setting frequently.

@Nightwalker-87 Nightwalker-87 merged commit 9923be6 into stlink-org:develop Mar 6, 2021
Release v1.7.0 automation moved this from Reviewer approved to Done Mar 6, 2021
@stlink-org stlink-org locked as resolved and limited conversation to collaborators Mar 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants