lkml.org 
[lkml]   [2010]   [Sep]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] Documentation/kbuild: major edit of modules.txt sections 1-4
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


\
 
 \ /
  Last update: 2010-09-17 20:39    [W:0.047 / U:23.524 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site