-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Makefile*: Allow external CPU dirs #20447
base: master
Are you sure you want to change the base?
Makefile*: Allow external CPU dirs #20447
Conversation
From a quick peek, this looks good to me. To be honest, I'd much rather see people upstreaming CPU code - I don't think that drivers are were the secret sauce of a firmware is and upstreaming them helps everyone. But then again, an OS should enable the users to do what they want, no matter what @maribu's personal feelings are on that. And I guess this could be handy while working with experimental RIOT ports that are not yet ready for upstreaming. I'm sadly afk for the most part of the next two weeks, so I can't do any testing now. Also, I tent to overlook things on a tiny screen that I would notice on a big monitor, so I don't feel comfortable to ACK this right now. But feel invited to ping me again in two weeks, if this has not been approved by then anyway. |
I agree with @maribu - why not upstream the CPU code? That said, we wouldn't make any API grantees for 'external' CPU ports in the first place. |
I am working with a "custom" native CPU where the IO from peripherals (gpio, i2c, spi, adc) is routed in a custom manner. I don't think it is particularly interesting or useful to other users of upstream RIOT, so I think my first preference would be to manage it "on the side", which this change allows. In this case, there is not a strong need to "keep up with API changes".
I agree - while the use-cases may be few, or obtuse, there seems to be little harm in supporting this capability, and it makes the CPUs more congruous with BOARDS and PACKAGEs, which have their own notion of "external." |
I thinks this fits well within out EXTERNAL_... scheme and might also help while developing new cpu that will go upstream later or as the author test some native thingy with something changed up (use parallelport as gpio?, USB-gpios? , firmata-gpio? (there are other things then gpio)) but maybe we should add warning / info /message that asks the user of any of our EXTERNAL_... features to think about upstreaming his code (just a little). |
I do like the idea, but I hate the proposed implementation.
I really don't want to pull a SEGGER here and start harassing our users with warnings as well. And I don't think that bulling people into upstreaming their code won't work anyway. How about instead revisit the documentation of the external directories, group that properly (now that there are options to use external boards, modules, packages, and soon CPUs), and add a friendly pointer there with a few reasons why upstreaming can be beneficial and when we would encourage upstreaming code? |
@maribu would you prefer that documentation update to happen on this PR, or separately? |
If you want to do that in this PR, I would very grateful. However, I don't think this needed for this PR to get in. That documentation cleanup was needed regardless of this PR. So if you don't want to do that, I can create a follow up. |
16b7feb
to
2369baa
Compare
I've added a blurb to the |
2369baa
to
4c12321
Compare
4c12321
to
f00e385
Compare
Contribution description
Following the patterns set by
EXTERNAL_BOARD_DIRS
, establish an optionalEXTERNAL_CPU_DIRS
parameter. This allows CPU definitions to reside outside the RIOT code tree.This change also commonizes the repeating pattern of
$(RIOTCPU)/$(CPU)
into a more succinct$(CPUDIR)
.Testing procedure
In a separate code base, set the
EXTERNAL_CPU_DIRS
variable, define a new CPU in that directory, and point a BOARD at that CPU, and build an application.Issues/PRs references
N/A