2.4.19-pre6 standardize {aic7xxx,aicasm}/Makefile

Standardize the makefiles for aic7xxx and aic7xxx/aicasm.
Use standard kbuild 2.4 rules.
Add missing dependencies for generated files.
Remove .NOTPARALLEL, only required because the dependencies were missing. Add yet more generated aic7xxx files to clean list.

Correct drivers/scsi/Makefile for invocation of aic7xxx.

Remove $(MOD_TARGET) from Rules.make. It was only used by the old aic7xxx makefile and it breaks the placement rules for modules.

Index: 19-pre6.1/drivers/scsi/Makefile

@@ -69,7 +69,7 @@ ifeq ($(CONFIG_SCSI_AACRAID),y)
   obj-$(CONFIG_SCSI_AACRAID)	+= aacraid/aacraid.o
 endif
 ifeq ($(CONFIG_SCSI_AIC7XXX),y)
-obj-$(CONFIG_SCSI_AIC7XXX)	+= aic7xxx/aic7xxx_drv.o
+  obj-$(CONFIG_SCSI_AIC7XXX)	+= aic7xxx/aic7xxx.o
 endif
 obj-$(CONFIG_SCSI_AIC7XXX_OLD)	+= aic7xxx_old.o
 obj-$(CONFIG_SCSI_IPS)		+= ips.o

Index: 19-pre6.1/Rules.make

@@ -176,7 +176,7 @@ modules: $(ALL_MOBJS) dummy \
 _modinst__: dummy
 ifneq "$(strip $(ALL_MOBJS))" ""
 	mkdir -p $(MODLIB)/kernel/$(MOD_DESTDIR)
-	cp $(ALL_MOBJS) $(MODLIB)/kernel/$(MOD_DESTDIR)$(MOD_TARGET)
+	cp $(ALL_MOBJS) $(MODLIB)/kernel/$(MOD_DESTDIR)
 endif
 
 .PHONY: modules_install

Index: 19-pre6.1/Makefile

@@ -204,10 +204,15 @@ CLEAN_FILES = \
 	drivers/zorro/devlist.h drivers/zorro/gen-devlist \
 	drivers/sound/bin2hex drivers/sound/hex2hex \
 	drivers/atm/fore200e_mkfirm drivers/atm/{pca,sba}*{.bin,.bin1,.bin2} \
+	drivers/scsi/aic7xxx/aicasm/aicasm \
 	drivers/scsi/aic7xxx/aicasm/aicasm_gram.c \
+	drivers/scsi/aic7xxx/aicasm/aicasm_gram.h \
+	drivers/scsi/aic7xxx/aicasm/aicasm_macro_gram.c \
+	drivers/scsi/aic7xxx/aicasm/aicasm_macro_gram.h \
+	drivers/scsi/aic7xxx/aicasm/aicasm_macro_scan.c \
 	drivers/scsi/aic7xxx/aicasm/aicasm_scan.c \
+	drivers/scsi/aic7xxx/aicasm/aicdb.h \
 	drivers/scsi/aic7xxx/aicasm/y.tab.h \
-	drivers/scsi/aic7xxx/aicasm/aicasm \
 	drivers/scsi/53c700_d.h \
 	net/khttpd/make_times_h \
 	net/khttpd/times.h \

Index: 19-pre6.1/drivers/scsi/aic7xxx/aicasm/Makefile

@@ -29,8 +29,6 @@ YFLAGS+= -t -v
 LFLAGS= -d
 endif
 
-.NOTPARALLEL:
-
 $(PROG):  ${GENHDRS} $(SRCS)
 	$(AICASM_CC) $(AICASM_CFLAGS) $(SRCS) -o $(PROG)
 

Index: 19-pre6.1/drivers/scsi/aic7xxx/Makefile

Are these documented somewhere? I emulated "scsi/Makefile" some time back, but it must be out of date with current conventions.

Three other quick questions:

1) Why doesn't "make dep" pickup the dependencies you have added explicitly the aic7xxx/Makefile? I would expect at least this dependency to be picked up:

+
+# Only aic7xxx_core.c includes aic7xxx_seq.h.
+aic7xxx_core.o: aic7xxx_seq.h

The other, indirect, dependencies (through aic7xxx.h) are picked up in every other OS this driver supports. I suppose "make dep"'s shortcuts mean you have to hard code stuff to make it work.

2) Are there plans to allow "down tree" Makefiles to list there own clean files? Having to modify the top level Makefile is a bit messy.

3) Why remove the ability to override the module target name? I eventually renamed my driver's main file to avoid this issue because I will be adding a second driver to the aic7xxx/Makefile (U320), but this same thing will probably come up again.

Lastly, I'm not sure why there was a discrepancy between aic7xxx_drv.o and aic7xxx.o. I just checked my local tree, and scsi/Makefile was already corrected. I will have to verify the files in the future, post merge, to make sure all of the changes have been accepted.

Thanks for you help on this.

--
Justin

No, you have to understand the interaction between the magic make variables and the undocumented processing in Rules.make. This is one of the reasons that the new kernel build mechanism (kbuild 2.5) has complete documentation on its language and how it works.

Already done in kbuild 2.5. Most clean targets are automatically generate as a side effect of the kbuild 2.5 statements, no user action required. You can add your own clean and mrproper entries for the very rare cases when they are not automatically added.

Your code was the only one that tried to override the module name and that was because your code was the only one that was breaking the kbuild rules. When a conglomerate object exists, all the objects that make up that conglomerate must have unique names. As long as you follow the rules and do not have foo.c at the the same time as you link multiple objects into foo.o then there are no problems.

For u320, ensure that the source is u320_core.c (not u320.c) and the conglomerate is u320.o and there will be no problems with the module name.

My complaint is that kbuild mixes module naming convention with source file names when they should be separate. For instance, if RedHat want to ship my driver as "aic7xxx_experimental.o", they have to rename files instead of make a single line change to the Makefile.

I'm complaining about the install target, not the link or compile targets. The user doesn't really care what the latter are so long as they can find the module if they build it.

That convention is already being followed.

The only problem I have now is how to map your changes to aic7xxx/Makefile when it has two targets. What I'd done prior to your mail was:

1) Build each driver independently using AIC7XXX_OBJS and AIC79XX_OBJS and a final LD step.
2) Add each driver .o to obj-$(CONFIG_SCSI_AIC7XXX) and obj-$(CONFIG_SCSI_AIC79XX) respectively.
3) Set O_TARGET to aic7xxx_drv.o to handle the case of one or more being configured for static linkage.
4) Have scsi/Makefile pull in aic7xxx_drv.o as appropriate.

The version of the Makefile you changed is just an edited down copy of the above. Perhaps that explains partially why it was the way it was. I'd be more than happy to fix this up some other way, but I don't think I can use the obj-y stuff directly as you have proposed in your patch once I add the second driver (as soon as we hit Beta which should be ~two weeks away).

--
Justin

That is a deliberate design decision, the installed module under /lib/modules has the same name and relative path as the object that is built in the kernel tree. Keeping them the same makes it much easier to debug kernel problems, a module called aic7xxx_experimental.o could come from anywhere. If somebody wants to create a new version of an existing driver while keeping the original name, they have to set up the new files and the rules accordingly.

See above.

Agreed, this is one of the limitations of kbuild 2.4. It is relatively easy to build everything in a directory into a single conglomerate, that requirement is so common that it is the default case for Rules.make.

Having multiple conglomerates gets messy, especially if you allow a mixture of built in and modular selection and if it is possible for everything to be a module with no built in stubs. The generic case looks like this

drivers/scsi/yourdir/Makefile

O_TARGET := dummy.o # needed to trigger some Rules.make processing
list-multi := module1.o module2.o
module1-objs := obj1a.o obj1b.o ...
module2-objs := obj2a.o obj2b.o ...
obj-$(CONFIG_module1) += module1.o
obj-$(CONFIG_module2) += module2.o
include Rules.make
module1.o: $(module1-objs)

        $(LD) -r -o $@ $<
module2.o: $(module2-objs)
        $(LD) -r -o $@ $<

and in drivers/scsi/Makefile

subdir-y += yourdir
subdir-m += yourdir
ifeq ($(CONFIG_module1),y)
obj-y += yourdir/module1.o
endif
ifeq ($(CONFIG_module2),y)
obj-y += yourdir/module2.o
endif

That should handle all combinations of CONFIG_module1=[ymn] and CONFIG_module2=[ymn]. As long as module1.o and module2.o have no common sub-objects, i.e. the module1-objs and module2-objs lists have no duplicates. If there are common sub-objects then it gets even more complicated.

Because the above processing for multiple conglomerates is so messy, a lot of developers use multiple directories, each containing one conglomerate. Then you can fall back on the default Rules.make processing in each directory.

<blatant plug>

BTW, this is _so_ much simpler in kbuild 2.5. Unlike kbuild 2.4 which makes one case easy and everything else messy, kbuild 2.5 makes everything easy. Single and multiple conglomerates are all handled the same, even the single case is easier to code in 2.5.

objlink(module1.o obj1a.o obj1b.o ...)
objlink(module2.o obj2a.o obj2b.o ...)
select(CONFIG_module1 module1.o)
select(CONFIG_module2 module2.o)

and in drivers/scsi/Makefile.in

link_subdirs(yourdir)

That is it!

</blatant plug>

That's a pretty weak argument coming from an OS that doesn't ship
with a kernel debugger <grumble> by default. After all, modules
can still come from anywhere (say a floppy). I still think this is a silly policy, but as you can see by the recent renaming of the driver files, I've come to accept it. 8-)

Since this is the case, and the Makefile will return to this format as soon as the U320 driver is released, can we come up with an interrim Makefile that assumes the new driver will show up shortly?

Both drivers use the same assembler. Otherwise I would have split the second out into its own directory.

--
Justin

I renew my standing offer that goes back to June 2001. If you agree to

* use the standard Linux kernel build methods
* stop shipping files and overwriting them at run time
* make the decision about firmware generation automatic instead of manual
* remove the BSD'isms from the Linux aic7xxx Makefiles

then I will happily write clean aic7xxx makefiles and support them. But if you insist on doing it your own way that does not match the Linux kernel build, then I am afraid that you are on your own.

Why shipping files and overwritten them at build time is a bad idea, and why relying on a manual switch to regenerate files is the lazy approach. From kbuild-2.5/Documentation/kbuild/kbuild-2.5.txt.

SHIPPING GENERATED FILES

In general it is bad practice to include generated files in the kernel source tree. The only exceptions are when the generating code is not in the kernel or when the generating code is in the kernel but it requires additional tools which not all users have installed. An example of a generated file without the code to regenerate it is firmware for a network driver. An example of a generation process that requires extra tools is drivers/char/defkeymap.c which requires the loadkeys program.

There is no justification for shipping any other generated file as part of the kernel. This is especially true if the user is always expected to regenerate the file, even more so if the generated data depends upon the user's .config. Under these circumstances it is dangerous to ship a generated file because it can be used when it does not reflect the user's kernel.

When a generated file has no kernel support to regenerate it, the file can be shipped as is. The question of whether including generated files in the kernel without the underlying code is a violation of the kernel license is outside the scope of this document.

For generated files where the code is included but not all users can run it, you must be careful about when the generated file is rebuilt. make looks at the time stamp of the generating and generated files to decide if the output needs to be regenerated. This works for the person making the changes because they control when the files are updated.

Time stamps do not work when the change is issued to the rest of the world as a diff. After patch has run you cannot guarantee which of the updated files has the more recent time. The patch program does not preserve time stamps (it must not) so the time stamps on the changed files depend on the order of the entries in the patch set. There is no defined order for entries in a patch set, it depends on the program that generated the difference listing. GNU diff generates patches in alphabetical order but only when it is given an entire directory. When diff is invoked from a source repository tool it is typically called once for each file and the file order is controlled by the repository tool. Even if your repository generates the patch in the correct order, that order can change when the patch is merged into the kernel.

Since you cannot rely on time stamps to decide if a generated and shipped file needs to be regenerated, you must use another mechanism for this case. There are two basic possibilities, manual or automatic (otherwise known as lazy or correct).

In the manual case you ignore changes to the generated or generating files until the user performs some manual operation, such as selecting CONFIG_REGENERATE_foo. This is easy to implement but it is also the lazy approach, making the end user do extra work to save the maintainer doing anything. This goes against the whole idea of *automatically* deciding what needs to be rebuilt, instead it relies on human intervention.

In the automatic case the maintainer has to do more work but the make rules can automatically determine if the end user has changed any of the related files and regenerate automatically. This is the correct approach. In the absence of reliable time stamps you need another method to check if the generating and generated files are in sync or not. The obvious method is a checksum of files.

Given a generated file 'foo' and a script to generate it 'foo-gen' which not all users can run because they may not have the required software installed.

    foo_files := $(srcfile foo-gen) $(srcfile foo.out_shipped)
    $(objfile foo_sum.d): $(srcfile foo_sum) $(foo_files)
            $(KBUILD_QUIET)(set -e;
	      mkdir -p $(objdir); \
              sed -e 's:$$[ABD-Z][^$$]*\$$::g' $(foo_files) > \
                $(objfile .tmp_foo_files); \
              ($(MD5SUM) -c $(srcfile foo_sum) >/dev/null 2>&1 && \
                echo S OK || echo F) > $@; \
              rm $(objfile .tmp_foo_files); \
            )

    user_command(foo.out
            ($(objfile foo_sum.d))
            ([ "`cat $<`" = "S OK" ] &&
              (cp $(srcfile $(@F)_shipped) $@) ||
              (set -ex ; $(srcfile foo-gen) > $(objfile foo.out)))
            ()
            )

    # Exception: this writes to the source tree.  The target must be
    # explicitly requested.
    .PHONY: foo_sum_shipped
    foo_sum_shipped: $(objfile foo.out)
            @cmp -s $(objfile foo.out) $(srcfile foo.out_shipped) || \
                 mv $(objfile foo.out) $(srcfile foo.out_shipped)
            @( \
             set -e; \
             sed -e 's:$$[ABD-Z][^$$]*\$$::g' $(foo_files) > \
                $(objfile .tmp_foo_files); \
              $(MD5SUM) $(objfile .tmp_foo_files) > $(objfile .tmp_$(@F)) && \
              cmp -s $(objfile .tmp_$(@F)) $(srcfile foo_sum) || \
                (echo Updating checksum for foo; \
                 mv $(objfile .tmp_$(@F)) $(srcfile foo_sum)); \
            )
            @rm -f $(objfile foo_sum.d) $(objfile .tmp_$(@F)) \
              $(objfile .tmp_foo_files)

    ifsel(CONFIG_foo)
      all_sum_shipped: foo_sum_shipped
    endif

That is boiler plate code which can be used for any shipped files, by globally replacing 'foo-gen' and 'foo'. To initialize the sequence, touch $(srcfile foo_sum) and $(srcfile foo.out_shipped) then make foo_sum_shipped. Thereafter any real change to the foo files will regenerate foo, ignoring any spurious time stamp differences caused by patch. It removes all need for special configuration options, if anybody changes the input files then kbuild automatically rebuilds foo when it is used. Users who do not change the input files get the shipped version when foo is required.

foo_files - List all the source files, including the generating
code and the shipped version of foo. The list does not include foo_sum.
foo_sum - Checksum over $(foo_files), i.e. the master copies.
The checksum ignores RCS id and date strings.
foo_sum.d - Built on the first make and when any of the input files
are updated after the first make. foo_sum.d either contains 'S OK' (sum is valid) or 'F' (failed).
user_command - Creates the working copy of foo, either from the
shipped copy (sum is valid) or by running foo-gen (sum does not match, a real change has occurred). Watch the parenthesis in the user_command, in particular the use of foo-gen requires () around the set of commands.
foo_sum_shipped - Takes the latest version of foo, generating it if
necessary and writes it back to the source tree as foo.out_shipped. Generates a new checksum automatically. Note: This command must be manually specified by the foo maintainer before shipping a new version of foo.

For a concrete example, see defkeymap.c in drivers/char/Makefile.in.

Because this is boiler plate code, it has been automated.

shipped(prefix
(generated files)
(generating files)
(commands to create all the generated files) (CONFIG dependencies to select all_sum_shipped)
)

The first field is the prefix for variables and for _sum files. It must be globally unique in the kbuild files.

The generated files are bare names, without '/', without '_shipped', no $(objfile) or $(srcfile) is allowed, in fact no $(variables) are accepted. These names are used to create the user_command() and the 'cp' commands, one for each generated file. Because user_command() only takes one target, all the other generated files are defined as side_effect()s of the last target, you must list the generated files in the order that they are created by your commands.

The generating files are bare names and list the source files used to generate the targets. For example, scripts, tables, lex or yacc code.

The commands define how you create the target files if the shipped versions of the targets are no longer valid, i.e. the checksum has failed. I recommend that the first command is 'set -ex;'.

The config dependencies are zero or more configs to check. If any are selected then all_sum_shipped is extended with foo_sum_shipped.

The code above shrinks to these few lines.

    shipped(foo
             (foo.out)
             (foo-gen)
             (set -ex ; $(srcfile foo-gen) > $(objfile foo.out))
             (CONFIG_foo)
           )

This is already true of the aic7xxx/Makefile for 2.4 since it will have a second target shortly. I even spent yesturday changing the file from AIC7XXX-OBJ to obj-aic7xxx to better follow your example. In the next release, scsi/Makefile will also include aic7xxx.o instead of aic7xxx_drv.o as you requested.

I have not looked at 2.5.

Not this again.

SHIPPING GENERATED FILES

In general, it is bad practice to even attempt to ship generated files in the Linux kernel build with the source of the utilities to build those files as in Linux you can't know what miss-match of tools the user is going to have. If you're lucky, they have a compiler with bugs that won't affect you, but since there is no "standard toolset" that userland distributions include, you cannot rely on even the tools that are necessary to rebuild the
compiler: lex and yacc. Even if the tools to build the tools to generate your files exist in all cases (toungue twister), triggering the tool build from within the kernel build can be "interesting" as you try and dissassociate the tool make environment from the kernel build environment which may make assumptions that don't apply to your tool.

The whole reliance on using patches as a general purpose upgrade tool in Linux is left for another discussion. My only hope is that with the move to using bitkeeper, this reliance will fade.

So, we are left with three options:

1) Don't bother shipping the code to regenerate the file.
2) Make it manually selectable for the .00000001% of users that might want or need to modify the generated files.
3) Make the makefiles rely on sed, md5, and cmp in addition to gmake and sh, an additional generated file and a script to automate a process that really doesn't benefit from being automated.

Option #3 is simply complicated and overengineered.

The mechanism we have in place today works. Other than one mixup in May or June of 2001, during the early adoption of this driver, it has always worked.

The old driver included the firmware source but no tools to rebuild it. I will happily do the same if this is more aesthetically pleasing. Considering that this is an open source project, it just seems silly to me to not ship the tools too as there is already a simple mechanism to allow their use in place already.

Are you talking about aic7xxx/aicasm/Makefile? That makefile does not use kbuild mecanisms becaues it is building a userland application. The aic7xxx/Makefile (at least for 2.4 - again haven't even looked at 2.5) seems to follow your own suggestions assuming there will be two targets.

Then why bother sending a patch to Marcello?

--
Justin

Hi,

Why don't we use a script like this:

set -e
src=$1
dst=$2
shift 2

test -f $dst && tail -1 $dst | sed 's,/\* \(.*\) \*/,\1,' | md5sum -c && touch $dst && exit 0
echo "$@"
"$@"
echo "/* $(md5sum $src) */" >> $dst

Then just call it with:
<script> <src> <dst> <build command>

This is much simpler and also it also gets rid of these small checksum files.

bye, Roman

There can be multiple destination files, e.g. running yacc produces a .c and a .h file.

The generated file is not necessarily .[ch], wrapping the md5sum in /* */ may break some generated files. AFAIK all currently generated files are .[ch] but I do not want to restrict future builds.

The output can change without the inputs changing. For example, the distributor might find a bug in the tool that generates the file, install a new version of the tool and regenerate. The inputs have not changed but the output has. To detect this, the md5sum is across all files, including the outputs, which makes it impossible to store the sum in one of the output files.

Unlikely I know, but I want 100% coverage on these special cases. 90% reliability on a kernel build was acceptable when everybody was an expert, but not now that the population of kernel builders is in the tens of thousands. There are far too many build problems where the response is "make mrproper", because of the special cases that fail.

Hi,

The checksum only needs to be stored in one of them.

The wrapping could be changed with an argument and if everything fails, the checksum can still be put into a separate file.

What exactly are you detecting? Simply regenerate the files and distribute the changes, the checksum won't change, but I don't understand how that should matter.
What other problem are you trying to solve? My simple script solves the problem of unreliable time stamps, when applying patches. Unless the user changes the inputs, the output files won't be regenerated.

bye, Roman