linux-kernel - Re: [PATCH 0/6] MN10300: Add the MN10300 architecture to Linux kernel [try #4]

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20071108182937.GA26852@uranus.ravnborg.org>
Date:	Thu, 8 Nov 2007 19:29:38 +0100
From:	Sam Ravnborg <sam@...nborg.org>
To:	David Howells <dhowells@...hat.com>
Cc:	torvalds@...l.org, akpm@...ux-foundation.org,
	linux-kernel@...r.kernel.org, linux-am33-list@...hat.com
Subject: Re: [PATCH 0/6] MN10300: Add the MN10300 architecture to Linux kernel [try #4]

Hi David.

A few comments to patch #5.
I only looked at the kbuild related stuff - and I have seen
stuff much worse than this.
Yet I have a few comments:

arch/mn10300/Makefile:

1) Use KBUILD_CFLAGS & KBUILD_AFLAGS & KBUILD_CPPFLAGS - they
have replaced the former xFLAGS.

2) Consider using unit-y as replacement for UNIT.
This enable you to do:
unit-$(CONFIG_MN10300_UNIT_ASB2303) += asb2303

3) Same for PROCESSOR => processor-y

4) Drop the symlinks - they are evil...
Use a structure like:
include/asm-mn10300/asb2303/proc/*.h

Then you adjust -I include/asm-mn10300/asb2303
and no symlinks needed.
And if you change processor kbuild will notice
and recompile everything.

No-one else does it this way but mn10300 could show how to do it.

5)
+zImage: vmlinux
+	$(Q)$(MAKE) $(build)=$(boot) $(KBUILD_IMAGE)
+
+all: zImage
+
+Image: vmlinux
+	$(Q)$(MAKE) $(build)=arch/mn10300/boot $@

This could be done as:
+Image zImage: vmlinux
+	$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@

And then modify boot/Makefile accordingly

6) archdep is long gone - delete the following

+archdep: scripts/mkdep symlinks
+	$(Q)$(MAKE) -C arch/mn10300/boot dep

7) Please set KBUILD_DEFCONFIG := asb2303_defconfig
This enable build monkeys(1) to do "make ARCH=mn10300 defconfig"

(1) such as me

arch/mn10300/boot/Makefile:

1) You can safely remove the copyright of Linus..
   Same goes for other places where his copyright are kept but filecontent
   is new.

2) The Image target is missing (as defined in arch/mn10300/Makefile
   Can you just kill it?

arch/mn10300/boot/compressed/Makefile:
1) Is -tarditional really needed. We try to kill it all over (slowly)

2) Please name the linker script *.lds

arch/mn10300/kernel/Makefile:
+# This next line shouldn't be necessary, but it is.
+CPPFLAGS += $(CPPFLAGS_$(@F))
+CPPFLAGS_asm-offsets.s = -D__ASM_OFFSETS_H__

Please explain why...

arch/mn10300/oprofile/Makefile:
+profdrvr-y				:= op_model_null.o
+
+oprofile-y				:= $(DRIVER_OBJS) $(profdrvr-y)

The variable profdrvr seems useless.

arch/mn10300/unit-asb2303/Makefile:

Please drop this wrong comment:
+# Note! Dependencies are done automagically by 'make dep', which also
+# removes any old dependencies. DON'T put your own dependencies here
+# unless it's something special (ie not a .c file).
+#
+# Note 2! The CFLAGS definitions are now in the main makefile...
+

Seen in other places where they are equally wrong/outdated.

	Sam

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/