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.

Sign up for GitHub

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

Jump to bottom

hal: nordic: add support for nrf52833 #12

Merged
merged 1 commit into from Nov 7, 2019
Merged

hal: nordic: add support for nrf52833 #12

merged 1 commit into from Nov 7, 2019

Conversation

barsok
Copy link
Contributor

@barsok barsok commented Oct 29, 2019

Add nrfx support for nrf52833 to the build system

Signed-off-by: Bartosz Sokolski bartosz.sokolski@nordicsemi.no

@barsok barsok mentioned this pull request Oct 29, 2019
Merged
@ioannisg ioannisg requested a review from anangl October 29, 2019 23:01
anangl
anangl requested changes Oct 30, 2019
View reviewed changes
nrfx_config_nrf52833.h Outdated

// <e> NRFX_CLOCK_ENABLED - nrfx_clock - CLOCK peripheral driver
//==========================================================
#ifdef CONFIG_NRFX_CLOCK
Copy link
Member

Choose a reason for hiding this comment

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

Starting from commit b429968, all Kconfig options are now handled in the common nrfx_config.h file.
Please use the following commit: e136b6501b662a1c5a9ea25412a10fc96c38ca9a for the changes in nrfx_config* files, and the remaining ones place in a commit with a title starting with "CMakeLists".

Copy link
Member

Choose a reason for hiding this comment

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

Is this done now @anangl ?

@anangl
Copy link
Member

anangl commented Oct 30, 2019

A PR in the main Zephyr repository (zephyrproject-rtos/zephyr#20222, for instance) should use this PR head, so that the changes get checked by CI.

@ioannisg ioannisg requested a review from anangl November 1, 2019 10:17
@ioannisg
Copy link
Member

ioannisg commented Nov 1, 2019

We merge this with approval by @anangl and green CI on zephyr tree

@ioannisg
Copy link
Member

ioannisg commented Nov 1, 2019

We merge this with approval by @anangl and green CI on zephyr tree

Is there more to be done here @barsok or simply get @anangl to go through the review once more?

@barsok
Copy link
Contributor Author

barsok commented Nov 4, 2019

We merge this with approval by @anangl and green CI on zephyr tree

Is there more to be done here @barsok or simply get @anangl to go through the review once more?

@ioannisg I believe just another look of @anangl would do

@anangl
Copy link
Member

anangl commented Nov 5, 2019 •
edited

@ioannisg

Is this done now @anangl ?

Not entirely. I asked to split the changes into two commits and to provide more adequate commit messages. The added nrfx_config_nrf52833.h file brings also an extra empty line at its end, what brings an unnecessary difference when compared to the corresponding nrfx template. But I think I can live with these imperfections, so I'll approve this PR as it is.

@carlescufi
Copy link
Member

@barsok could you please rebase and refer to this PR in a PR in the main zephyr repo so we can run CI?

@ioannisg
Copy link
Member

ioannisg commented Nov 7, 2019

@barsok could you please rebase and refer to this PR in a PR in the main zephyr repo so we can run CI?

He already has a PR; it is the 52833 porting :)
As said, however, since we are moving to nrfx 2.0, you better rebase this PR over the 2.0 support

@carlescufi
Copy link
Member

@barsok could you please rebase and refer to this PR in a PR in the main zephyr repo so we can run CI?

He already has a PR; it is the 52833 porting :)
As said, however, since we are moving to nrfx 2.0, you better rebase this PR over the 2.0 support

Sorry was expecting a link here.

@carlescufi
Copy link
Member

Once zephyrproject-rtos/zephyr#20222 is green I will merge this. Please ping me if I don't do it quick, since we are close to code freeze.

ioannisg
ioannisg reviewed Nov 7, 2019
View reviewed changes
CMakeLists.txt Outdated Show resolved Hide resolved
ioannisg
ioannisg reviewed Nov 7, 2019
View reviewed changes
CMakeLists.txt Outdated Show resolved Hide resolved
@barsok
hal: nordic: add support for nrf52833
bc0ed74 
Add nrfx support for nrf52833 to the build system

Signed-off-by: Bartosz Sokolski <bartosz.sokolski@nordicsemi.no>
@ioannisg
Copy link
Member

ioannisg commented Nov 7, 2019

Once zephyrproject-rtos/zephyr#20222 is green I will merge this. Please ping me if I don't do it quick, since we are close to code freeze.

No @carlescufi don't do that zephyrproject-rtos/zephyr#20222 is not running with the latest rebase of this PR

@carlescufi
Copy link
Member

Once zephyrproject-rtos/zephyr#20222 is green I will merge this. Please ping me if I don't do it quick, since we are close to code freeze.

No @carlescufi don't do that zephyrproject-rtos/zephyr#20222 is not running with the latest rebase of this PR

Well, how about we force-push then? We need to know this doesn't break CI.

@ioannisg
Copy link
Member

ioannisg commented Nov 7, 2019

Yes, let's force push the 52833 PR

@ioannisg ioannisg added the DNM Do Not Merge label Nov 7, 2019
@ioannisg
Copy link
Member

ioannisg commented Nov 7, 2019

So let me rebase this now over master...

@ioannisg ioannisg merged commit 64f3473 into zephyrproject-rtos:master Nov 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ioannisg ioannisg ioannisg approved these changes

@anangl anangl anangl approved these changes

Assignees

@anangl anangl

Labels
DNM Do Not Merge
Projects
None yet
Milestone
No milestone
4 participants
@barsok @anangl @ioannisg @carlescufi