-
Notifications
You must be signed in to change notification settings - Fork 8.2k
boards: dts: nrf: Remove SoC compatible in top-level compatible #24774
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
boards: dts: nrf: Remove SoC compatible in top-level compatible #24774
Conversation
The SoC node has compatibles for the specific SoC in place, having the same compatible at the top level is technically a conflict and the top-level one should really just be about the board. Remove the SoC related compatibles at the top-level. Signed-off-by: Kumar Gala <[email protected]>
|
All checks passed. checkpatch (informational only, not a failure)Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages. |
| / { | ||
| model = "QEMU Cortex-M0"; | ||
| compatible = "nordic,nrf51822-qfaa", "nordic,nrf51822"; | ||
| compatible = "bbc,qemu-microbit"; |
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.
@ioannisg is this OK?
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.
I think so , yes
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.
qemu_cortex_m0 is emulating a bbc-micro bit
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.
that's why I called in qemu-microbit, otherwise I'd argue we could have just used the microbit board port.
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.
Yes. FWIW, the reason we did not use the microbit board port directly, is that QEMU does not emulate RTC but just the TIMER nRF peripheral, so we needed a new timer driver; this made me thing we could just have a separate qemu board with a local timer driver :)
mbolivar-nordic
left a comment
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.
Seems fine; AFAIK we don't use these anywhere
ioannisg
left a comment
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.
Looks fine to me
|
This isn't going to pass CI because of the large number of boards touched. As mentioned we don't use the top-level compat for anything so this will have no impact. Going to merge. |
|
I love (not) when stuff that touches a bunch of files gets fast-tracked and merged with input from a small subset of those affected. First I saw of this was #24769 (review) and it was already too late. This removed compatibles at the root that are not present anywhere else: e.g. there's no longer a visible compatible to identify a It's not that it's a big deal, but I put that there for a reason, and now I need another solution. |
|
I misread what this changed (it conflicted with a local change during rebase). Sorry, looks like this wasn't the cause of my problem. |
The SoC node has compatibles for the specific SoC in place, having the
same compatible at the top level is technically a conflict and the
top-level one should really just be about the board. Remove the SoC
related compatibles at the top-level.
Signed-off-by: Kumar Gala [email protected]