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

[refactoring] BSD-License-compliant flashloader rewrite #932

Merged
merged 26 commits into from
May 6, 2020

Conversation

chenguokai
Copy link
Collaborator

This will finally close #915

@chenguokai
Copy link
Collaborator Author

Hope this rebase will help

@chenguokai chenguokai changed the title Add cleanroom note draft Cleanroom flashloader assembly Apr 24, 2020
@chenguokai
Copy link
Collaborator Author

chenguokai commented Apr 25, 2020

My local test shows that the current code is known to be working on stm32f103 and stm32f401. I am dealing with some minor formatting issues and a critical improvement about the compile process. After that the code will be ready for test.

Edit: All done.

@chenguokai chenguokai marked this pull request as ready for review April 25, 2020 07:26
@Nightwalker-87 Nightwalker-87 changed the title Cleanroom flashloader assembly - ready [refactoring] BSD-License-compliant flashloader rewrite Apr 26, 2020
@Nightwalker-87
Copy link
Member

I think we should have more people looking at this as it is a key component.
What exactly do you mean with "cleanroom" - why did you choose this naming?
I don't think that explanations should go to /doc/developer.txt as this is only a quick and dirty copy-paste-doc created "once upon a time". I actually feel we should have a separate document /doc/flashloaders.md for it. Explanations which were present as comments in the old flashloader files could go into this document, maybe in form of a parameter | explanation table format - something like this. Maybe someone even has a better idea for that. In the end users should find a place to gain knowledge on how this works.

@chenguokai
Copy link
Collaborator Author

What exactly do you mean with "cleanroom" - why did you choose this naming?

Because it is what kind of method this PR have followed.

From https://opensource.stackexchange.com/questions/6216/can-i-cleanroom-code-by-myself-if-public-specifications-already-exist

A clean room design is a preventative defence strategy in anticipation of copyright infringement claims. The idea is that a new creative work cannot infringe on the copyright of an earlier work if the new work's authors did not view the earlier work. A clean room design process implements this isolation by having different people handle reverse-engineering of the earlier work and creating the new work, with the two groups only communicating via a spec that can be more easily reviewed for non-infringement.

I don't think that explanations should go to /doc/developer.txt as this is only a quick and dirty copy-paste-doc created "once upon a time". I actually feel we should have a separate document /doc/flashloaders.md for it. Explanations which were present as comments in the old flashloader files could go into this document, maybe in form of a parameter | explanation table format - something like this.

That's what I wanted, too. This clean room document provide only essential information for another one to implement a BSD code. What I wanted to add to any new document (either /doc/flashloaders.md or some file with another proper name) will and should include a more detailed explanation of why we should follow those restrictions.

@Nightwalker-87
Copy link
Member

Nightwalker-87 commented Apr 26, 2020

Ah I understand. To be honest I have never come across this topic before, thus I could not associate the name. Ok, so this reads as being a somehow intermediate step towards a complete solution of this issue. I'm fine with that. Thx for explaining. 👍
It would be desirable though to merge a complete solution if possible.

@chenguokai
Copy link
Collaborator Author

Hope the latest commit will help

@Nightwalker-87
Copy link
Member

Nightwalker-87 commented Apr 27, 2020

I have the impression that someone seems to have deeply focussed on this topic. 👀
If this whole stuff works (what I can't verify by myself here) we can go with it, apart from some minor changes.

@chenguokai
Copy link
Collaborator Author

All "my"s have been replaced.
Added a note at the head of flash_loader.c

@Nightwalker-87 Nightwalker-87 removed the request for review from grevaillot May 6, 2020 20:46
@Nightwalker-87
Copy link
Member

I'm tired of waiting any longer for a second review. 👎
If nobody cares, I don't do either...

@Nightwalker-87 Nightwalker-87 merged commit 9b207de into stlink-org:develop May 6, 2020
@chenguokai
Copy link
Collaborator Author

Hope that untested devices work well🤔

@Nightwalker-87
Copy link
Member

It's ok and if so, to me it's not in your responsibility.

@stlink-org stlink-org locked as resolved and limited conversation to collaborators May 29, 2020
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.

Rewrite of flashloader source files (BSD license)
3 participants