On 17:33 Fri 17 Sep , Michal Marek wrote:
> On 16.9.2010 11:10, matt mooney wrote:
> > A couple of notes:
> > I chose to use the environment variable $PWD in the examples for both the commandline
> > and Makefile; however, I was wondering if I should have set PWD in the Makefile and
> > used $(PWD) instead.
>
> I'd rather keep the example Makefile minimalistic.
Ok, I was planning on still using the environment variable if that is okay with
you.
>
> > Also, the $@ variable was removed from the 'make' command within
> > the Makefile. I did not understand the purpose of this for building kernel modules and
> > it would break my test build.
>
> $@ is the name of the target, so in
> all::
> $(MAKE) -C $(KERNELDIR) M=`pwd` $@
> this translates to '$(MAKE) -C $(KERNELDIR) M=`pwd` all' which is the
> default. So with your change it does the same, but I'm surprised that
> the $@ didn't work for you.
I think we should leave this out anyway to simplify the command syntax.
> [...]
> > - $KDIR refers to the path to the kernel source top-level directory
> > + $KDIR is the path to the kernel source directory.
>
> I'm not a native English speaker, but I read the new sentence as "the
> $KDIR variable holds the path to the kernel source directory, you can
> use the following commands verbatim", instead of "$KDIR is used in place
> of the path to the kernel source directory, replace it with the actual
> path".
You are right, I will reword this.
> >
> > - make -C $KDIR M=`pwd`
> > - Will build the module(s) located in current directory.
> > - All output files will be located in the same directory
> > - as the module source.
> > - No attempts are made to update the kernel source, and it is
> > - a precondition that a successful make has been executed
> > - for the kernel.
> > + make -C $KDIR
> > + -C is used to specify where to find the kernel source.
> > + 'make' will actually change to the specified directory
> > + when executing and will change back when finished.
> >
> > - make -C $KDIR M=`pwd` modules
> > - The modules target is implied when no target is given.
> > - Same functionality as if no target was specified.
> > - See description above.
> > + make -C $KDIR M=$PWD
> > + M= is used to tell kbuild that an external module is being
> > + built. The option given to M= is the directory where the
> > + external module (kbuild file) is located.
> >
> > - make -C $KDIR M=`pwd` modules_install
> > - Install the external module(s).
> > - Installation default is in /lib/modules/<kernel-version>/extra,
> > - but may be prefixed with INSTALL_MOD_PATH - see separate
> > - chapter.
> > + make -C $KDIR SUBDIRS=$PWD
> > + Same as M=. The SUBDIRS= syntax is kept for backwards
> > + compatibility, but its usage is deprecated.
>
> While you are rewriting the file, you can also drop the reference to
> SUBDIRS. It has been deprecated for over six years, so it doesn't need
> to be documented anymore, IMO. BTW, I like how you swapped the two
> sections, it is more logical that way.
>
> [...]
> > - Examples (module foo.ko, consist of bar.o, baz.o):
> > - make -C $KDIR M=`pwd` bar.lst
> [...]
> > + make -C $KDIR M=$PWD bar.o
>
> make -C $KDIR M=`pwd` bar.lst was a valid command, see make help:
>
> dir/file.lst - Build specified mixed source/assembly target only
> (requires a recent binutils and recent build
> (System.map))
>
> Perhaps there should be a short sentence explaining what the four
> commands do.
Ah, an explanation may help.
> The rest looks OK to me (although I can't really comment on the language
> part), thanks a lot for looking into this.
I have actually reedited some of my changes.
After further review of the document, I believe some modifications might help.
First, I am thinking that the "Options" and "Targets" sections could introduce
the full command syntax and then list the individual items with an explanation.
For example:
make -C $KDIR M=$PWD
-C
info on the option
M=
...
Next, section 3 "Example commands" seems redundant because the commands are
first explained in section 2.1. Also, section 4 seems to not be fully divided
into sections; looking at the table of contents, it does not list
sub-sections. Each example within that section, IMHO, should be an individual
section and not fall under the heading "Shared Makefile for module and kernel."
The new headings could be something like:
4.1 Shared Makefile
4.2 Kbuild file and Makefile
Backwards compatibility can be included in 4.2 if it is still needed? An
explanation that obj-m is the module being built would be nice along with a
sub-section showing how multiple modules can be built at the same time. And,
the intro in section 5 could use some serious rewording,
Besides that, the rest of the document looks good and only needs some minor
corrections, such as EXTRA_CFLAGS -> ccflags-y.
Let me know what you think.
Thanks,
matt
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
