-
-
Notifications
You must be signed in to change notification settings - Fork 775
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
target: Renesas: support for flash types MF3 and MF4 #1723
base: main
Are you sure you want to change the base?
Conversation
729fb2b
to
1626fc6
Compare
Before we go nuts reviewing this, please run We're a bit uneasy with the Flash interfaces being split out into their own files like this, especially with what the renesas.c implementation is having to do - and as the existing support is left in that file, which means it's inconsistent. Please can you give us your rationale behind this split? The enable variable you're defining in src/Makefile is inconsistent - named |
target: Renesas flash types MF3 and MF4
1a48e71
to
9091034
Compare
clang-format, I didn't think of that. The rationale to split in two files, is that the renesas_ra.c is already about 1000 lines and pretty hard to navigate. So adding another 1000 lines would add to the confusion. Your repo, you rules :-) If you insist. I will make renesas_ra 2000 lines. I guess a solution would be to have individual files, for the two kind of processors, having their own probe function. That would follow you style, but it would also duplicate some of the probe code, hence larger flash image. I am pretty sure main doesn't build with PROBE_HOST=native. After a rebase on main, I get:
When I build, without Renesas support, which now exclude renesas_ra.c. |
Your compiler is too old - Re splitting the files - the problem isn't necessarily the split, it's the inconsistency created by the split how it's done here - you've split the Flash routines out for one processor type but not both, The code you're PR'ing here appears to be adapted from Renesas' MIT-licensed BSPs and other Renesas repos, and you're ascribing the copyright and licensing to us which is a violation of their licensing. Their license headers must be kept intact. The code also needs considerable clean-up, including BSP macros eliminating as those aren't relevant to BMD nor helpful to maintain. There's also some timing stuff that got copied in which doesn't fit with the code base at all nor how it's organised and needs addressing. If you really need a µs delay timing function, something should be added to |
Detailed description
This change implements support for flashing Renesas processors with flash type MF3 and MF4.
I have fixed "native" platform build by moving all Renesas support to be optional ENABLE_RENESAS=1.
This is only tests on a RA2L1 processor, as this is the only one I have access to.
NOTE:
At the moment writing the .IDCode segment 0x1010010, is disabled. Writing a wrong value here bricks the processor beyond repair.
For the boldhearted! remove all
Your checklist for this pull request
make PROBE_HOST=native
)make PROBE_HOST=hosted
)Closing issues
fixes #1243