You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/04/30 03:01:52 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
xiaoxiang781216 opened a new issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-621984404
> We just want to change up_xxx function common across armv7-a, armv7-m... to arm_xxx, but up_ is also used for the common function across arm, riscv..., so it is impossible to replace all up_ inside arch/arm to arm_ auotmaitcally.
Common functions across RISC-V (only) should begin with riscv_, not up and not up_ or arch_
There is not interaction between the private functions in ARM and the private functions in other archtectures such as RISC-V. That is proper modular design. The naming must all be private and internale to each architecture and should NOT be shared.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-621992310
> > We just want to change up_xxx function common across armv7-a, armv7-m... to arm_xxx, but up_ is also used for the common function across arm, riscv..., so it is impossible to replace all up_ inside arch/arm to arm_ auotmaitcally.
>
> Common functions across RISC-V (only) should begin with riscv_, not up and not up_ or arch_
>
> There is not interaction between the private functions in ARM and the private functions in other archtectures such as RISC-V. That is proper modular design. The naming must all be private and internale to each architecture and should NOT be shared.
>
> Private functions used only within arch/ and board/ for the same MCU should never have the prefix up_ (or arch_) and should never be exposed via arch.h. That is very bad modular thinking and we must not even consider that.
>
> up_ is intended for common MCU interfaces that are used by the OS, usually in sched/ and must always have the prefix up_ and must always be prototypes in include/nuttx/arch.h. I disagree with you completely and think that this is a terrible idea. It is not good modular thinking and we must not consider it.
You misunderstand my meaning, let me give an example:
1.Find and replace all up_idle to arch_idle
2.Repeat step 1 for all functions inside include/nuttx/arch.h
3.Find and replace all up_doirq to arm_doirq under arch/arm
4.Repeat step 3 for all up_ function under arch/arm
5.Change up_xxx function under arch/riscv to riscv_xxx
The final result is what you want:
1.The code outside arch call arch_xxx
2.The common function for arm prefix with arm_
3.The common fucntion for riscv prefix with rsicv_
But this approach we can do the find/replace automatically.
The key point is that include/nuttx/arch.h need take a new prefix arch_, then we know that it's safe to change the remaining up_ functions in each arch folder to the corrospending prefix(e.g. arm_ or riscv_).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-621967674
> In #916, the error is in the name up_fullcontextrestore(tcb->xcp.regs); which is incorrect. That should be arm_fullcontextrestore(). The naming in other ARM architectures is incorrect. The other architectures are not following the naming standard. ARMv8-M is correctly following the naming standard.
>
> So yes, there will be issues UNTIL other ARM architectures are mad to follow the naming standard. Let's do that!! Let's fix the other ARM architectures and not screw up the naming standard.
We just want to change up_xxx function common across armv7-a, armv7-m... to arm_xxx, but up_ is also used for the common function across arm, riscv..., so it is impossible to replace all up_ inside arch/arm to arm_ auotmaitcally.
if https://cwiki.apache.org/confluence/display/NUTTX/Naming+FAQ state correctly, arch_ is better prefix than up_. We can apply the sequence I suggested before:
1.Rename functions in arch.h from up_xxx to arch_xxx
2.Rename files in arch/yyy/ from up_xxx.* to yyy_xxx.*
3.Rename function in arch/yyy/ from up_xxx to yyy_xxx
Since functions for all arch change to arch_ in the first step, we can find the remaining up_xxx and replace with arm_xxx safely and automatically.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-621584274
@patacongo does this make sense?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 removed a comment on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 removed a comment on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-621584274
@patacongo does this make sense?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-621948079
The naming convenions required that global function platform fucntions .. that is, functions used outside of arch/ and board/ MUST begion with up. Functions used only within arch/ and board/ must NOT begin with up_. See https://cwiki.apache.org/confluence/display/NUTTX/Naming+FAQ
arch_ is never permitted as a function prefix.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-621983011
I have started the renaming with PR #924 After that is merged I will do more, a little at a time.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-621627164
The best step is:
1.Rename functions in arch.h from up_xxx to arch_xxx
2.Rename files in arch/yyy/ from up_xxx.* to yyy_xxx.*
3.Rename function in arch/yyy/ from up_xxx to yyy_xxx
we can avoid the error like this:
https://github.com/apache/incubator-nuttx/pull/916
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-621987730
Private functions used only within arch/ and board/ for the same MCU should never have the prefix up_ (or arch_) and should never be exposed via arch.h. That is very bad modular thinking and we must not even consider that.
up_ is intended for common MCU interfaces that are used by the OS, usually in sched/ and must always have the prefix up_ and must always be prototypes in include/nuttx/arch.h. I disagree with you completely and think that this is a terrible idea. It is not good modular thinking and we must not consider it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-621627164
The best step is:
1.Rename functions in arch.h from up_xxx to arch_xxx
2.Rename files in arch/yyy/ from up_xxx.* to yyy_xxx.*
3.Rename function in arch/yyy/ from up_xxx to yyy_xxx
We can avoid the error like this:
https://github.com/apache/incubator-nuttx/pull/916
And the global find and replace should work correctly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-621960709
> The naming convenions required that global function platform fucntions .. that is, functions used outside of arch/ and board/ MUST begion with up. Functions used only within arch/ and board/ must NOT begin with up_. See https://cwiki.apache.org/confluence/display/NUTTX/Naming+FAQ
>
> arch_ is never permitted as a function prefix.
As you state in https://cwiki.apache.org/confluence/display/NUTTX/Naming+FAQ:
Q: What's the meaning of the prefix up_ in NuttX? Which functions should be prefixed with up_?
A: up_ is supposed to stand for microprocessor; the u is like the Greek letter micron: µ. So it would be µP which is a common shortening of the word microprocessor. I don't like that name very much. **I wish I would have used arch_ instead**. But now I think I am stuck with up_.
Aslo there are some functions start with arch_ in arch.h now:
int arch_phy_irq(FAR const char *intf, xcpt_t handler, void *arg,
phy_enable_t *enable);
void arch_sporadic_start(FAR struct tcb_s *tcb);
void arch_sporadic_lowpriority(FAR struct tcb_s *tcb);
void arch_sporadic_suspend(FAR struct tcb_s *tcb);
void arch_sporadic_resume(FAR struct tcb_s *tcb);
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-622447264
The internal architecture interfaces are not documented and, hence, have no impact outside of the architecture-specific code and the architecture-specific documentation.
The up_ functions has global scope are part of the well-controlled, modular interface. The naming and location where the function is prototyped is a critical part of the modular architecutre. arch.h defines the common, generic interface between ALL archtectures and the rest of the OS.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-621950081
In #916, the error is in the name up_fullcontextrestore(tcb->xcp.regs); which is incorrect. That should be arm_fullcontextrestore(). The naming in other ARM architectures is incorrect.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-621950081
In #916, the error is in the name up_fullcontextrestore(tcb->xcp.regs); which is incorrect. That should be arm_fullcontextrestore(). The naming in other ARM architectures is incorrect. The other architectures are not following the naming standard. ARMv8-M is correctly following the naming standard.
So yes, there will be issues UNTIL other ARM architectures are mad to follow the naming standard. Let's do that!! Let's fix the other ARM architectures and not screw up the naming standard.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-621992310
> > We just want to change up_xxx function common across armv7-a, armv7-m... to arm_xxx, but up_ is also used for the common function across arm, riscv..., so it is impossible to replace all up_ inside arch/arm to arm_ auotmaitcally.
>
> Common functions across RISC-V (only) should begin with riscv_, not up and not up_ or arch_
>
> There is not interaction between the private functions in ARM and the private functions in other archtectures such as RISC-V. That is proper modular design. The naming must all be private and internale to each architecture and should NOT be shared.
>
> Private functions used only within arch/ and board/ for the same MCU should never have the prefix up_ (or arch_) and should never be exposed via arch.h. That is very bad modular thinking and we must not even consider that.
>
> up_ is intended for common MCU interfaces that are used by the OS, usually in sched/ and must always have the prefix up_ and must always be prototypes in include/nuttx/arch.h. I disagree with you completely and think that this is a terrible idea. It is not good modular thinking and we must not consider it.
You misunderstand my meaning, let me give an example:
1.Find and replace all up_idle to arch_idle
2.Repeat step 1 for all functions inside include/nuttx/arch.h
3.Find and replace all up_doirq to arm_doirq under arch/arm
4.Repeat step 3 for all up_ function under arch/arm
5.Change up_xxx function under arch/riscv to riscv_xxx
The final result is what you want:
1.The code outside arch call arch_xxx
2.The common function for arm prefix with arm_
3.The common fucntion for riscv prefix with rsicv_
But this approach we can do the find/replace automatically.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-621967674
> In #916, the error is in the name up_fullcontextrestore(tcb->xcp.regs); which is incorrect. That should be arm_fullcontextrestore(). The naming in other ARM architectures is incorrect. The other architectures are not following the naming standard. ARMv8-M is correctly following the naming standard.
>
> So yes, there will be issues UNTIL other ARM architectures are mad to follow the naming standard. Let's do that!! Let's fix the other ARM architectures and not screw up the naming standard.
We just want to change up_xxx function common across armv7-a, armv7-m... to arm_xxx, but up_ is also used for the common function across arm, riscv..., so it is impossible to replace all up_ inside arch/arm to arm_ auotmaitcally.
if https://cwiki.apache.org/confluence/display/NUTTX/Naming+FAQ state correctly, arch_ is better prefix than up_. We can apply the sequence I suggested before:
1.Rename functions in arch.h from up_xxx to arch_xxx
2.Rename files in arch/arm/ from up_xxx.* to arm_xxx.*
3.Rename function in arch/arm/ from up_xxx to arm_xxx
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-621627164
The best step is:
1.Rename functions in arch.h from up_xxx to arch_xxx
2.Rename files in arch/yyy/ from up_xxx.* to yyy_xxx.*
3.Rename function in arch/yyy/ from up_xxx to yyy_xxx
We can avoid the error like this:
https://github.com/apache/incubator-nuttx/pull/916
And find and replace should work correctly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-622441942
@xiaoxiang781216 I have put a lot of effort in the past few days and submitted several PRs to bring the arch/arm directories into compliance with the naming convention of https://cwiki.apache.org/confluence/display/NUTTX/Naming+FAQ
I think if we follow the naming convention, we should not run into any future issues like #916
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-622444414
Sure, let's clean up arm arch first, and see whether change up_ to arch_ for all function inside arch.h can help rename other arch more efficiently.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-622446399
I just want to make sure that we protect the modular architecture. I don't want to expose ARM internal functions though arch.h. That would not be a good decision. Just changing the naming from up_ to arch_ is not so important (but effects a lot of code, lots of documentation, and the Wiki).
It is probably better to stay wtih the prefix up_ for historic reasons.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on issue #915: Change all function prefix from up_ to arch_ in include/nuttx/arch.h
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on issue #915:
URL: https://github.com/apache/incubator-nuttx/issues/915#issuecomment-621967674
> In #916, the error is in the name up_fullcontextrestore(tcb->xcp.regs); which is incorrect. That should be arm_fullcontextrestore(). The naming in other ARM architectures is incorrect. The other architectures are not following the naming standard. ARMv8-M is correctly following the naming standard.
>
> So yes, there will be issues UNTIL other ARM architectures are mad to follow the naming standard. Let's do that!! Let's fix the other ARM architectures and not screw up the naming standard.
We just want to change up_xxx function common across armv7-a, armv7-m... to arm_xxx, but up_ is also used for the common function across arm, riscv..., so it is impossible to replace all up_ inside arch/arm to arm_ auotmaitcally.
if https://cwiki.apache.org/confluence/display/NUTTX/Naming+FAQ state correctly, arch_ is better prefix than up_. We can apply the sequence I suggested before:
1.Rename functions in arch.h from up_xxx to arch_xxx
2.Rename files in arch/yyy/ from up_xxx.* to yyy_xxx.*
3.Rename function in arch/yyy/ from up_xxx to yyy_xxx
Since functions for all arch change to arm_ in the first step, we can find the remaining up_xxx and replace with arm_xxx safely and automatically.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org