-
Notifications
You must be signed in to change notification settings - Fork 148
nRF52832 and nRF52 DK Support #326
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also pulls in several (compatibly licensed, I think) header files and a single C file from the vendor. This handles chip startup and errata workarounds.
Thank you for your contribution @nocko :) I see the Just by quickly looking at your pull-request, it looks like there is a lot of code duplication from the You also copied all the For the ravenscar-sfp support, what do you have in mind? Because if the drivers work on ZFP they will also work on the ravenscar-sfp. For Did you use startup-gen to generate the crt0 and linker script? |
Hi Fabien! De-duplicationI thought it over last night and de-duplication is still the [obviously] better case though (esp. because I am planning nRF52840 and 810 support as well). I am thinking that I'll split into family/chip specific source directories, but keep everything under a common package (NRF? Nordic?). That should avoid programmers having to manually remember a package path for each peripheral, and limit changes to managing the source directories RTS gpr file. This wouldn't work (cleanly) if two chips had different peripherals (required different drivers) but the same name. I don't think this is currently the case... ExamplesI only ported three (digital_out, BLE_beacon, and buttons) examples from Microbit and all three are working on the nRF52 DK. I think this should be okay. startup-genI didn't use startup-gen. I manually edited the linker script and ported crt0 from the microbit (and included the vendor files for the [huge] list of errata. I'll investigate, but see question 1 below. Questions
|
Yes, that's what we do for
Actually you can then differentiate the drivers by putting them in different directories:
If they work yes, that's ok. But in that case please update the readmes because they are mentioning
You should use
I do think that we should implement the errata in Ada. The good way to integrate them without editing the package body nRF is
begin
if Errata_X then
-- Fix errata
end if;
end nRF; This code will be automatically executed when Ada is initialized.
Yes they do, I seems similar to what we do for STM32. You should have a look at that. Don't hesitate to ask me for feedback as soon as you can. Here or in the gitter char. Thanks, |
e71521a
to
2288b45
Compare
@Fabien-Chouteau CI is caught up in svd2ada generated files. Example:
Others are records whose contents don't fill the entire Size:
Should I hand edit these for pedantics, or file bugs against svd2ada? I doubt there are actual bugs in svd2ada... it's just a question if it's in scope for svd2ada to try to completely work around the badness in vendor files. As an aside, almost all the driver changes between nrf51 and nrf52 are working around the differences in naming in the svd files, rather than any substantial changes. |
Status
|
I think this is ready for review. There's an opertunity to combine some of the microbit board-level drivers with the nrf52 board drivers, but this patch is already so big that I think that should be addressed in a follow up after this work lands. @Fabien-Chouteau |
Rebased on master after the cm0 NVIC function renaming. |
Thanks for the update @nocko. I will send a PR to your branch for a few type changes on the NVIC drivers. For |
Either the SVD file of svd2ada should also be fixed, but this will do for the time being.
Various fixes for you nRF52 branch.
@Fabien-Chouteau Shall I squash these commits? |
Don't worry I will probably do a squash and merge on the PR anyway. |
Various nRF5x modifications
Hi @nocko, excuse me for the delay I was not available the last couple of weeks. I will have a look at the PR as soon as possible. |
Thanks @Fabien-Chouteau! I understand that life often "gets in the way". Whenever you are ready is fine with me. |
Thank you @nocko ! It's great to have nRF52 support in Ada Drivers Library :) |
This is working (examples included), but rather less complete than nRF51.
Controversial (?) removal of System.Interrupts from nvic_cm4_cm7 in favour of locally defined types (like cm0)... due to only supporting ZFP.Many peripherals are the same between nrf51 and 52, but this implementation copies all peripheral drivers. There may be a way to combine them intelligently instead.Feedback appreciated.