On Sat, Jul 08, 2023 at 11:26:42PM +0800, Zhangjin Wu wrote:
Currently, the architecture specific arch.h has two parts, one is the syscall declarations for sys.h, another is the _start code definition for startup support.
The coming crt.h will provide the startup support with a new common _start_c(), it will replace most of the assembly _start code and shrink the original _start code to be minimal, as a result, _start_c() and the left minimal _start code will work together to provide the startup support, therefore, the left _start code will be only required by crt.h.
So, the syscall declarations part of arch.h can be split to sys_arch.h and the _start code part of arch.h can be split to crt_arch.h and then, they should only be included in sys.h and crt.h respectively.
At the same time, the architecture specific arch-<ARCH>.h should be split to <ARCH>/crt.h and <ARCH>/sys.h.
As a preparation, this creates the architecture specific directory and moves tools/include/nolibc/arch-<ARCH>.h to tools/include/nolibc/<ARCH>/arch.h.
I'm sorry but I still don't understand what it *provides*. I'm reading it as "we *can* do this so let's do it". But what is the specific purpose of adding this extra directory structure ? It's really unclear to me and worries me that it'll only result in complicating maintenance by adding even more files, thus even more "include" lines and cross dependencies.
Zhangjin, very often in your series, the justification for a change is missing, instead it's only explaining what is being changed, and I must confess that it makes it particularly difficult to figure the benefits. I'm only seeing this as an opportunity for a change ("can be split"). I could have missed something of course, but I can't figure what problem it is trying to solve.
As a general advice, I tend to remind people that when sending a patch series, they should consider they're trying to sell it, so they must emphasize the benefits of accepting the series for the maintainer(s).
You very likely have a good reason for doing this but I can't see it here so I'm just seeing a change that will possibly add some extra cost (if at least because file locations change again) and nothing more. When you try to reorganize things, it's often much more efficient to try to discuss it before proposing patches, because reorg patches are generally unreadable and take time for you to create and for others to review. Instead, just explaining what you think you can improve is faster for everyone, and others can chime in and propose alternate approaches (something which is very hard to do with a patch series).
Thanks! Willy