my comments are inline with Russel's reply
----- Original Message -----
From: Russell King - ARM Linux Admin <[EMAIL PROTECTED]>
> Each section of code has a description of what it does in head-armv.S.
In my opinion these DO NOT sufficiently describe each section.
> Err, no, not really. The documentation incorporated into the kernel is
> merely there to let people who know what they're doing. It's not designed
> so that someone who knows nothing about ARM can work out what's going on.
> To do so would significantly increase the size of the source code, which
> people already think is too big. Don't forget that the Linux kernel is
> organised to make the "hackers" life easier, not to make it easy for
> others to understand or even learn.
I believe that I understand ARM fairly well. I certainly know more than
nothing about it. In addition, I believe trying to maintain a small size of
the source base by reducing the limiting the overall number of comments is
self-defeating. I would be happy to see the code size increase drastically
if it was purely from more commenting. Very infrequently do comments
actually decrease the value of the code (the most notable exception being
when they're wrong).
> For something as simple as the ARM page tables, its something that any
> programmer who is thinking about looking at ARM code should know before.
> The split between the ARM ARM and the individual TRMs is quite well
> defined - one describes the "architecture" and the other is the "specific
> implementation details". Therefore, most, if not all of the stuff outside
> the proc-*.S files relates to the ARM ARM, and the stuff inside proc-*.S
> relates to the TRMs.
I'm not disagreeing with the seperation of code, which I think is _very_
well accomplished. My concern is with bringing it together in the mind of
the programmer. I used the page table example because it had just come up,
which turns out not to have been a clear case. The proc_info trick comes
closer to making my point.
> > I BELIEVE that an extended description should be inserted before every
> > logical step that can be described in one line of comment. This is not
to
> > say that every op gets a line of comment, but I BELIEVE that you should
be
> > able to recreate the function of the code by looking solely at the
comments.
>
> I disagree. I'm sure you'd say that about C code if you didn't know
> anything about C.
NO, because in C you can use "meaningful" variable and function names, flow
control is much more obvious, and you don't have to contort operands to make
them fit into the assemblers limited vocabulary. It's not to difficult to
"speak" your way through C code, in many instances, even if you don't
understand the details. In assembly, all you've got is the details, and this
is the point I am trying to make: inline comments are the only thing that
give that higher picture of things back to programmer.
In the Linux coding style guide, it talks about using <10 locals. Aren't
registers the same thing? And yet, the code in head-armv.s doesn't
(sufficiently, IMO) describe how these are being flung around. The same
arguement applies here - I can't keep the contents of all the registers
straight in my mind as they are carried from one block to another.
> > I know that in some ways this example directly violates the Linux Coding
> > Style Guide in several places, but I BELIEVE that style does not fully
apply
> > here - its goal is to direct C programmers in their efforts. Assembly
> > language is a whole other beast, and should be treated as such.
>
> Several points in reply here:
>
> 1. You missed out the most important comments of all there - the ones at
> the head of the section of code that tells you what the following code
> is doing. This should really be all you need.
Yes, it should. But in many cases, it isn't. When trying to get a high level
view of the code, those are the most important. But to see how the pieces
actually fit together, you really need that next level of detail (that I'm
preaching for).
> 2. The comments should definitely not be there to describe how the
> instruction set works, which is almost what your comments were doing.
> This is something that the person working on the code should know
> already.
I went a little overboard with my example, but I certainly am NOT describing
what the instruction set is doing. For instance, the sub r5, r5, r9
instruction is completely opaque until the programmer figures out exactly
what the comment says. By way of another example, right before __switch_data
label, the add pc, r10, #12 instruction (which jumps to initialize the
processor dependent mm code) is similarly mindboggling until you figure out
that the proc_info structure actually has an opcode built into it (which is
a pretty big deviation from the traditional seperation of data and code).
Presently, neither of these tricks are mentioned at all, and it is these
kinds of things that _need_ to be documented!!!
> You've got to know what you're doing, really. The general idea behind
> most of the assembler code that is now in the kernel is that it should
> be generic across many processors, and as such the newly acquanted
> programmer should not have to delve into this code deeply.
>
> If you do need to delve into it deeply, and you don't know much about
> ARM code, then that is the time to contact the mailing lists stating what
> you'd like to do - since this is code which runs on many different
> processors, you've got to be aware of all the issues surounding all
> the ARM processors that Linux is expected to run on.
I do know what I am doing (Really!), I am familiar with ARM, and I _want_ to
delve deeply into how the kernel is working. I should NOT be required to go
any further than the distribution to do it. All the magic should be
documented. Relying on someone else to understand the magic can have tragic
consequences - if they're hit by a bolt of lightning, all that information
goes up in smoke.
I am not disputing the value of mailing lists and peer assistence, but it
should not be the fastest way to get that information. Describing some
problems takes longer than it does to go chasing down references that could
be made in the comments. And figuring out stuff for yourself (in the true
hacker spirit) goes much further than getting a simple answer to fix one
problem. You often get exposed to other aspects of the system that you were
previously unaware, expanding your entire knowledge base.
> You wouldn't get the plumber to comment every T and bend in your heating
> system so that the gas engineer can modify your plumbing or vice versa?
> Wouldn't you prefer to get someone in who was qualified to change the
> plumbing in, rather than someone who may flood your house with water?
I don't think this example is valid - we're all plumbers here. Some more
experienced than others, but plumbers nonetheless. And anyone who's had to
work on their own plumbing will tell you that having a _detailed_ blueprint
is essential, particularly if you can't immediately see the parts you'll be
working on or don't even know that those parts even existed. Also, I must
re-emphasize that I DO NOT want every T and bend commented, and it should
not be necessary to do so.
Argh, I really didn't want to get involved in this type of debate (wholly
opinionated and unresolvable to anyone's complete satisfaction), but I
honestly believe that the quality of the comments could be improved markedly
from where they currently are. This stems directly from the fact that I (an
experienced programmer who was already fairly well familiar with ARM) would
_really_ have appreciated better comments having been there, and that I'd
like to see future programmers get off easier.
Cheers,
Zach
unsubscribe: body of `unsubscribe linux-arm' to [EMAIL PROTECTED]
++ Please use [EMAIL PROTECTED] for ++
++ kernel-related discussions. ++
