Ok, I can't resist jumping in again. Thanks to everyone for their prompt and
detailed responses; I can't tell you how happy it makes me to see that the
ARM Linux crowd jumps on the opportunity to help out. Since yesterday, I've
come to understand much more of the initialization than I did previously.

There are quite a few puzzles to unwind during initialization. For instance,
I was a bit confused by how proc_info structures (arch/arm/mm/proc-*.S) were
searched during initialization. Further snooping revealed the
arch/arm/vmlinux-armv.lds.in file, which goes a long way in explaining this.
A simple comment in the head-armv.S that describes this bit of magic would
be invaluable. (See below for my example)

((Now, all of the above I had written in a different message to send to the
group before I saw Russel's response to S A McConnell (which follows). I
have to agree to some extent with SAM in that the ARM portions of the kernel
are not entirely as well documented as some might think, the above is only
one example. In fact, I only today managed to figure out the entire flow of
control between __entry and start_kernel, which is _not_ documented clearly
at all.))

----- Original Message -----
From: Russell King - ARM Linux Admin <[EMAIL PROTECTED]>
Sent: Wednesday, April 26, 2000 11:29 AM
Subject: Re: *TODO* Document the pseudo-ops (adr, ops)
[snip]
> > I would love to see some documentation about the MMU setup for the ARM
> > and how the tables are constructed. Since I started looking at the
> > kernel in January I have been wondering why there is no documentation.
[snip]
> The various bits where the tables are initialised are commented.  The
> initial setup code that allows the kernel to get going is very well self-
> documented.  Looking at the other code, it also seems to have adequate
> comments for what the code is doing.

I disagree. Adequate for "THE developer of ARM Linux" perhaps, but not for
the rest of us mere mortals. ;^) The initial page table setup should, at the
very least, tell the programmer where to look for the definition of the
structure that is being created. Also, where is then used? That would answer
why the code is here in the first place. How do I understand it well enough
to change it? A simple reference as to what files to look in would be
immensely helpful. In addition...

> Please note that the kernel sources are not supposed to contain reference
> information, but are supposed to contain enough for a programmer to
> understand what the code is doing.  The general guidance given here is
that
> the code should be written such that it is understandable, and with
comments
> that do not describe what every line of code does, but describes the
purpose
> behind what the code is doing.

I abosultely agree on this point, with one small exception. Reference
manuals are the ultimate source of information, but the code should contain
enough information to figure out _where_ in _which_ of the several books
piled next to my computer I should start looking in. Nothing more.

As for the density of comments, I believe your statement stands on its own
for high level programming languages, and generally agrees with the Linux
Kernel Coding Style Guide. However, I partially disagree when it comes to
assembly; I'm sorry but it doesn't just document itself.

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.
In other words, you should be able to remove the code and be able to
reimplement those portions of the code. (I'm not entirely happy with the way
I've phrased this last paragraph, and I'm sure I'll catch flak for it).

To show that I'm not all talk, here's an example of what I mean from
head-armv.S before and after my meddling. I intend to pepper things as I run
across them in a similar fashion, so I would appreciate feedback on my
efforts, particularly since I hope to make these changes stick in the main
distribution.

<------------ snip ---------------->

__lookup_processor_type:
  adr r5, 2f
  ldmia r5, {r7, r9, r10}
  sub r5, r5, r9
  add r7, r7, r5
  add r10, r10, r5
  mrc p15, 0, r9, c0, c0  @ get processor id
1:  ldmia r10, {r5, r6, r8}  @ value, mask, mmuflags
  eor r5, r5, r9
  tst r5, r6
  moveq pc, lr
  add r10, r10, #36   @ sizeof(proc_info_list)
  cmp r10, r7
  blt 1b
  mov r10, #0
  mov pc, lr

2:  .long __proc_info_end
  .long 2b
  .long __proc_info_begin

<------------ snip ---------------->

__lookup_processor_type:
  /* load __proc_info_lookup_table */
  adr r5, __proc_info_lookup_table
  ldmia r5, {r7, r9, r10}

  /* calclulate difference between where we thought .proc.info was
    going to be placed and where the linker actually put it  ??? */
  sub r5, r5, r9

  /* readjust __proc_info_end and __proc_info_begin */
  add r7, r7, r5
  add r10, r10, r5

  mrc p15, 0, r9, c0, c0  @ get processor id

  /* now check against the current proc_info table entry */
1:  ldmia r10, {r5, r6, r8}  @ value, mask, mmuflags
  eor r5, r5, r9
  tst r5, r6
  moveq pc, lr

  /* table entry does not match actual proc id, move to next entry */
  add r10, r10, #36   @ sizeof(proc_info_list)
  cmp r10, r7
  blt 1b

  /* no more entries, return error */
  mov r10, #0
  mov pc, lr

  /* proc_info_lookup_table - one entry for each processor type */
 /* __proc_info_end and __proc_info_begin are defined by the linker -
  see the file arch/arm/vmlinux-armv.lds.in */
/*  proc_info structure is declared in include/asm-arm/procinfo.h */
/*  proc_info structures are defined in arch/arm/mm/proc-*.S */
__proc_info_lookup_table:
  .long __proc_info_end
  .long __proc_info_lookup_table
  .long __proc_info_begin

<------------ snip ---------------->

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. It's too
dense to read, never mind modify, without thorough documentation. I BELIEVE
that, when you sit down in front of the kernel source For The First Time,
you should be able to _mentally_ walk through the code. Furthermore, those
of you who have been doing this for a while may be able to read the assembly
without comments and know what's going on, but the comments aren't for you,
are they?

Now that I've shared these quasi-religious beliefs, feel free to bash away
at the cornerstones of my reasoning.... (I know, I too find it hard to
believe that everyone won't agree with me outright... :) :)

Cheers,

Zach Welch
Firmware Architect
At Work Computers
www.atworkcom.com



unsubscribe: body of `unsubscribe linux-arm' to [EMAIL PROTECTED]
++        Please use [EMAIL PROTECTED] for           ++
++                        kernel-related discussions.                      ++

Reply via email to