pkg-kernel-module: new infra to ease building kernel modules (branch yem/kernel-modules)

The Linux kernel offers a nice and easy-to-use infra to build out-of-tree kernel modules.

Currently, we have quite a few packages that build kernel modules, and most duplicate (or rewrite) the same code over-and-over again.

Introduce a new infrastructure that provides helpers to build kernel modules, so packages do not have to duplicate/rewrite that.

The infrastructure, unlike any other package infra, is not standalone. It needs another package infra to be used. This is so that packages that provide both userland and kernel modules can be built easily. So, this infra only defines post-build and post-install hooks, that will build the kernel modules after the rest of the package.

Also, no host version is provided, since it does not make sense to build kernel modules for the host.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Baruch Siach <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
Changes v1 -> v2:
- drop third and fourth args to inner macro: we're never going to build kernel modules for the host (Baruch, Arnout)
- add missing LINUX_MAKE_ENV (Arnout)
- use $$(@D), not $$($(2)_DIR) (Arnout)
- print a single MESSAGE (Arnout, Thomas)
- typoes (Arnout)

---
 package/Makefile.in          |  1 +
 package/pkg-kernel-module.mk | 89 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+)
 create mode 100644 package/pkg-kernel-module.mk

package/Makefile.in +1 -0

index c02d31f..180fd46 100644
@@ -398,3 +398,4 @@ include package/pkg-virtual.mk
 include package/pkg-generic.mk
 include package/pkg-kconfig.mk
 include package/pkg-rebar.mk
+include package/pkg-kernel-module.mk

package/pkg-kernel-module.mk new file +89 -0

new file mode 100644
index 0000000..608c9e6
@@ -0,0 +1,89 @@
+################################################################################
+# kernel module infrastructure for building Linux kernel modules
+#
+# This file implements an infrastructure that eases development of package
+# .mk files for out-of-tree Linux kernel modules. It should be used for all
+# packages that build a Linux kernel module using the kernel's out-of-tree
+# buildsystem, unless they use a complex custom build-system.
+#
+# In terms of implementation, this infrastructure relies on another package
+# infrastructure, and only defines post-build and post-install hooks, so that
+# packages can both build user-space (with any of the other *-package infra)
+# and/or build kernel modules.
+#
+# As such, it is to be used in conjunction with another *-package infra,
+# like so:
+#
+#   $(eval $(kernel-module))
+#   $(eval $(generic-package))
+#
+# Note: if the caller needs access to the kernel modules (either after they
+# are built or after they are installed), it will have to define its own
+# post-build/install hooks after calling kernel-module, but before calling
+# the other *-package infra, like so:
+#
+#   $(eval $(kernel-module))
+#   define FOO_MOD_TWEAK
+#   	# do something
+#   endef
+#   FOO_POST_BUILD_HOOKS += FOO_MOD_TWEAK
+#   $(eval $(generic-package))
+#
+# Note: this infra does not check that the kernel is enabled; it is expected
+# to be enforced at the Kconfig level with proper 'depends on'.
+################################################################################
+
+################################################################################
+# inner-kernel-module -- generates the make targets needed to support building
+# a kernel module
+#
+#  argument 1 is the lowercase package name
+#  argument 2 is the uppercase package name
+################################################################################
+
+define inner-kernel-module
+
+# The kernel must be built first.
+$(2)_DEPENDENCIES += linux
+
+# This is only defined in some infrastructures (e.g. autotools, cmake), but
+# not in others (generic). So define it if not already defined.
+$(2)_MAKE ?= $$(MAKE)
+
+# If not specified, consider the source of the kernel module to be at
+# the root of the package.
+$(2)_MODULE_SUBDIRS ?= .
+
+# Build the kernel module(s)
+define $(2)_KERNEL_MODULES_BUILD
+	@$$(call MESSAGE,"Building kernel module(s)")
+	$$(foreach d,$$($(2)_MODULE_SUBDIRS), \
+		$$(LINUX_MAKE_ENV) $$($$(PKG)_MAKE) \
+			-C $$(LINUX_DIR) \
+			$$(LINUX_MAKE_FLAGS) \
+			$$($(2)_MODULE_MAKE_OPTS) \
+			M=$$(@D)/$$(d) \
+			modules$$(sep))
+endef
+$(2)_POST_BUILD_HOOKS += $(2)_KERNEL_MODULES_BUILD
+
+# Install the kernel module(s)
+define $(2)_KERNEL_MODULES_INSTALL
+	@$$(call MESSAGE,"Installing kernel module(s)")
+	$$(foreach d,$$($(2)_MODULE_SUBDIRS), \
+		$$(LINUX_MAKE_ENV) $$($$(PKG)_MAKE) \
+			-C $$(LINUX_DIR) \
+			$$(LINUX_MAKE_FLAGS) \
+			$$($(2)_MODULE_MAKE_OPTS) \
+			M=$$(@D)/$$(d) \
+			modules_install$$(sep))
+endef
+$(2)_POST_INSTALL_TARGET_HOOKS += $(2)_KERNEL_MODULES_INSTALL
+
+endef
+
+################################################################################
+# kernel-module -- the target generator macro for kernel module packages
+################################################################################
+
+kernel-module = $(call inner-kernel-module,$(pkgname),$(call UPPERCASE,$(pkgname)))

Signature · 6 lines

1.9.1

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Quoted 15 lines

The Linux kernel offers a nice and easy-to-use infra to build out-of-tree kernel modules.

Currently, we have quite a few packages that build kernel modules, and most duplicate (or rewrite) the same code over-and-over again.

Introduce a new infrastructure that provides helpers to build kernel modules, so packages do not have to duplicate/rewrite that.

The infrastructure, unlike any other package infra, is not standalone. It needs another package infra to be used. This is so that packages that provide both userland and kernel modules can be built easily. So, this infra only defines post-build and post-install hooks, that will build the kernel modules after the rest of the package.

Quoted 7 lines

Also, no host version is provided, since it does not make sense to build kernel modules for the host.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Baruch Siach <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

Tested-by Doug Kehn <[email protected]>

The Linux kernel offers a nice and easy-to-use infra to build out-of-tree kernel modules.

Currently, we have quite a few packages that build kernel modules, and most duplicate (or rewrite) the same code over-and-over again.

Introduce a new infrastructure that provides helpers to build kernel modules, so packages do not have to duplicate/rewrite that.

The infrastructure, unlike any other package infra, is not standalone. It needs another package infra to be used. This is so that packages that provide both userland and kernel modules can be built easily. So, this infra only defines post-build and post-install hooks, that will build the kernel modules after the rest of the package.

Also, no host version is provided, since it does not make sense to build kernel modules for the host.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Baruch Siach <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

package/pkg-kernel-module.mk new file +8 -0

+# the other *-package infra, like so:
+#
+#   $(eval $(kernel-module))
+#   define FOO_MOD_TWEAK
+#   	# do something
+#   endef
+#   FOO_POST_BUILD_HOOKS += FOO_MOD_TWEAK
+#   $(eval $(generic-package))

package/pkg-kernel-module.mk new file +39 -0

+$(2)_MAKE ?= $$(MAKE)
+
+# If not specified, consider the source of the kernel module to be at
+# the root of the package.
+$(2)_MODULE_SUBDIRS ?= .
+
+# Build the kernel module(s)
+define $(2)_KERNEL_MODULES_BUILD
+	@$$(call MESSAGE,"Building kernel module(s)")
+	$$(foreach d,$$($(2)_MODULE_SUBDIRS), \
+		$$(LINUX_MAKE_ENV) $$($$(PKG)_MAKE) \
+			-C $$(LINUX_DIR) \
+			$$(LINUX_MAKE_FLAGS) \
+			$$($(2)_MODULE_MAKE_OPTS) \
+			M=$$(@D)/$$(d) \
+			modules$$(sep))
+endef
+$(2)_POST_BUILD_HOOKS += $(2)_KERNEL_MODULES_BUILD
+
+# Install the kernel module(s)
+define $(2)_KERNEL_MODULES_INSTALL
+	@$$(call MESSAGE,"Installing kernel module(s)")
+	$$(foreach d,$$($(2)_MODULE_SUBDIRS), \
+		$$(LINUX_MAKE_ENV) $$($$(PKG)_MAKE) \
+			-C $$(LINUX_DIR) \
+			$$(LINUX_MAKE_FLAGS) \
+			$$($(2)_MODULE_MAKE_OPTS) \
+			M=$$(@D)/$$(d) \
+			modules_install$$(sep))
+endef
+$(2)_POST_INSTALL_TARGET_HOOKS += $(2)_KERNEL_MODULES_INSTALL
+
+endef
+
+################################################################################
+# kernel-module -- the target generator macro for kernel module packages
+################################################################################
+
+kernel-module = $(call inner-kernel-module,$(pkgname),$(call UPPERCASE,$(pkgname)))

Quoted 2 lines

The Linux kernel offers a nice and easy-to-use infra to build out-of-tree kernel modules.

Quoted 7 lines

package/pkg-kernel-module.mk new file +1 -0

new file mode 100644
index 0000000..608c9e6
@@ -0,0 +1,89 @@
+################################################################################

package/pkg-kernel-module.mk line 7

Quoted 6 lines

package/pkg-kernel-module.mk new file +6 -0

+# kernel module infrastructure for building Linux kernel modules
+#
+# This file implements an infrastructure that eases development of package
+# .mk files for out-of-tree Linux kernel modules. It should be used for all
+# packages that build a Linux kernel module using the kernel's out-of-tree
+# buildsystem, unless they use a complex custom build-system.

buildsystem, build-system. What about consistently choosing "build system"? :-)

package/pkg-kernel-module.mk line 12

Quoted 4 lines

package/pkg-kernel-module.mk new file +4 -0

+# In terms of implementation, this infrastructure relies on another package
+# infrastructure, and only defines post-build and post-install hooks, so that
+# packages can both build user-space (with any of the other *-package infra)
+# and/or build kernel modules.

# The kernel-module infrastructure requires the packages that use it to also

# include another package infrastructure. kernel-module only defines post-build

# and post-install hooks. This allows the package to build both kernel modules
# and/or user-space components (with any of the other *-package infra).

Quoted 4 lines

package/pkg-kernel-module.mk new file +4 -0

+#
+# As such, it is to be used in conjunction with another *-package infra,
+# like so:
+#

package/pkg-kernel-module.mk line 22

Quoted 6 lines

package/pkg-kernel-module.mk new file +6 -0

+#   $(eval $(kernel-module))
+#   $(eval $(generic-package))
+#
+# Note: if the caller needs access to the kernel modules (either after they
+# are built or after they are installed), it will have to define its own
+# post-build/install hooks after calling kernel-module, but before calling

after -> *after*

Quoted 12 lines

package/pkg-kernel-module.mk new file +12 -0

+# the other *-package infra, like so:
+#
+#   $(eval $(kernel-module))
+#   define FOO_MOD_TWEAK
+#   	# do something
+#   endef
+#   FOO_POST_BUILD_HOOKS += FOO_MOD_TWEAK
+#   $(eval $(generic-package))
+#
+# Note: this infra does not check that the kernel is enabled; it is expected
+# to be enforced at the Kconfig level with proper 'depends on'.
+################################################################################

package/pkg-kernel-module.mk line 40

Quoted 6 lines

package/pkg-kernel-module.mk new file +6 -0

+
+################################################################################
+# inner-kernel-module -- generates the make targets needed to support building
+# a kernel module
+#
+#  argument 1 is the lowercase package name

Is there any reason to keep this, since it's not used?

Quoted 4 lines

package/pkg-kernel-module.mk new file +4 -0

+#  argument 2 is the uppercase package name
+################################################################################
+
+define inner-kernel-module

package/pkg-kernel-module.mk line 50

Quoted 6 lines

package/pkg-kernel-module.mk new file +6 -0

+
+# The kernel must be built first.
+$(2)_DEPENDENCIES += linux
+
+# This is only defined in some infrastructures (e.g. autotools, cmake), but
+# not in others (generic). So define it if not already defined.

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ So define it here as well.

Arnout, All,

On 2015-06-12 00:44 +0200, Arnout Vandecappelle spake thusly:

Quoted 3 lines · Yann E. MORIN

Quoted 2 lines

The Linux kernel offers a nice and easy-to-use infra to build out-of-tree kernel modules.

[--SNIP--]

Quoted 7 lines

Quoted 7 lines

package/pkg-kernel-module.mk new file +1 -0

new file mode 100644
index 0000000..608c9e6
@@ -0,0 +1,89 @@
+################################################################################

Quoted 8 lines

package/pkg-kernel-module.mk line 7

Quoted 6 lines

package/pkg-kernel-module.mk new file +6 -0

+# kernel module infrastructure for building Linux kernel modules
+#
+# This file implements an infrastructure that eases development of package
+# .mk files for out-of-tree Linux kernel modules. It should be used for all
+# packages that build a Linux kernel module using the kernel's out-of-tree
+# buildsystem, unless they use a complex custom build-system.

buildsystem, build-system. What about consistently choosing "build system"? :-)

I think we've settled for "buildsystem" in other places, so I'll stick to that (and fix here).

Quoted 8 lines

package/pkg-kernel-module.mk line 12

Quoted 5 lines

package/pkg-kernel-module.mk new file +4 -0

+# In terms of implementation, this infrastructure relies on another package
+# infrastructure, and only defines post-build and post-install hooks, so that
+# packages can both build user-space (with any of the other *-package infra)
+# and/or build kernel modules.

# The kernel-module infrastructure requires the packages that use it to also

# include another package infrastructure. kernel-module only defines post-build

s/include/use/ because we're not 'including' pkg infras, we're not even calling them, we're evaluating them. So I'd still word that as 'use' if you don't mind. ;-)

Quoted 3 lines

# and post-install hooks. This allows the package to build both kernel modules
# and/or user-space components (with any of the other *-package infra).

Otherwise, OK, it's simpler and more explixit thn what I wrote.

Quoted 4 lines

Quoted 4 lines

package/pkg-kernel-module.mk new file +4 -0

+#
+# As such, it is to be used in conjunction with another *-package infra,
+# like so:
+#

Quoted 8 lines

package/pkg-kernel-module.mk line 22

Quoted 6 lines

package/pkg-kernel-module.mk new file +6 -0

+#   $(eval $(kernel-module))
+#   $(eval $(generic-package))
+#
+# Note: if the caller needs access to the kernel modules (either after they
+# are built or after they are installed), it will have to define its own
+# post-build/install hooks after calling kernel-module, but before calling

after -> *after*

OK. I guess ditto for: before -> *before* , no?

Quoted 13 lines

Quoted 13 lines

package/pkg-kernel-module.mk new file +12 -0

+# the other *-package infra, like so:
+#
+#   $(eval $(kernel-module))
+#   define FOO_MOD_TWEAK
+#   	# do something
+#   endef
+#   FOO_POST_BUILD_HOOKS += FOO_MOD_TWEAK
+#   $(eval $(generic-package))
+#
+# Note: this infra does not check that the kernel is enabled; it is expected
+# to be enforced at the Kconfig level with proper 'depends on'.
+################################################################################

Quoted 8 lines

package/pkg-kernel-module.mk line 40

Quoted 6 lines

package/pkg-kernel-module.mk new file +6 -0

+
+################################################################################
+# inner-kernel-module -- generates the make targets needed to support building
+# a kernel module
+#
+#  argument 1 is the lowercase package name

Is there any reason to keep this, since it's not used?

Well, I wanted to keep it as much aligned to existing infras as possible, even though we're not using it.

Quoted 13 lines

package/pkg-kernel-module.mk line 50

Quoted 11 lines

package/pkg-kernel-module.mk new file +10 -0

+#  argument 2 is the uppercase package name
+################################################################################
+
+define inner-kernel-module
+
+# The kernel must be built first.
+$(2)_DEPENDENCIES += linux
+
+# This is only defined in some infrastructures (e.g. autotools, cmake), but
+# not in others (generic). So define it if not already defined.

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ So define it here as well.

OK.

Thanks! :-)

Regards,
Yann E. MORIN.

docs/manual/adding-packages-kernel-module.txt new file +5 -0

+
+Let's look at a more complex example:
+
+----
+01:

################################################################################
+02: #
+03: # foo
+04: #
+05:
################################################################################
+06:
+07: FOO_VERSION = 1.2.3

+08: FOO_SOURCE = foo-$(FOO_VERSION).tar.xz
+09: FOO_SITE = http://www.foosoftware.org/download
+10: FOO_LICENSE = GPLv2
+11: FOO_LICENSE_FILES = COPYING
+12:
+13: FOO_MODULE_SUBDIRS = driver/base
+14: FOO_MODULE_MAKE_OPTS = KVERSION=$(LINUX_VERSION_PROBED) +15:

Quoted 13 lines

docs/manual/adding-packages-kernel-module.txt new file +4 -0

+Let's look at a more complex example:
+
+----
+01:

################################################################################
+02: #
+03: # foo
+04: #
+05:
################################################################################
+06:
+07: FOO_VERSION = 1.2.3
+08: FOO_SOURCE = foo-$(FOO_VERSION).tar.xz

Quoted 6 lines

+09: FOO_SITE = http://www.foosoftware.org/download
+10: FOO_LICENSE = GPLv2
+11: FOO_LICENSE_FILES = COPYING
+12:
+13: FOO_MODULE_SUBDIRS = driver/base
+14: FOO_MODULE_MAKE_OPTS = KVERSION=$(LINUX_VERSION_PROBED)

^ This variable is not documented anywhere, you might want to do that...

Quoted 7 lines

Quoted 4 lines

+13: FOO_MODULE_SUBDIRS = driver/base
+14: FOO_MODULE_MAKE_OPTS = KVERSION=$(LINUX_VERSION_PROBED)

^ This variable is not documented anywhere, you might want to do that...

Yup, thanks.

What about:

    http://git.buildroot.org/~ymorin/git/buildroot/commit/?h=yem/kernel-modules&id=663ab1ec1c3e5b80eae0bdb355bc0e2e0d58497c

(toward the end).

Regards,
Yann E. MORIN.

--
.-----------------.--------------------.------------------.--------------------.

|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics'
|  conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___
|               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/
|  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v
|   conspiracy.  |

'------------------------------^-------^------------------^--------------------'

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 package/lttng-modules/lttng-modules.mk | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

package/lttng-modules/lttng-modules.mk +1 -10

index 6833ad2..30c581e 100644
@@ -10,14 +10,5 @@ LTTNG_MODULES_SOURCE = lttng-modules-$(LTTNG_MODULES_VERSION).tar.bz2
 LTTNG_MODULES_LICENSE = LGPLv2.1/GPLv2 for kernel modules; MIT for lib/bitfield.h and lib/prio_heap/*
 LTTNG_MODULES_LICENSE_FILES = lgpl-2.1.txt gpl-2.0.txt mit-license.txt LICENSE
 
-LTTNG_MODULES_DEPENDENCIES = linux
-
-define LTTNG_MODULES_BUILD_CMDS
-	$(MAKE) -C $(@D) $(LINUX_MAKE_FLAGS) KERNELDIR=$(LINUX_DIR)
-endef
-
-define LTTNG_MODULES_INSTALL_TARGET_CMDS
-	$(MAKE) -C $(@D) $(LINUX_MAKE_FLAGS) KERNELDIR=$(LINUX_DIR) modules_install
-endef
-
+$(eval $(kernel-module))
 $(eval $(generic-package))

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 package/igh-ethercat/igh-ethercat.mk | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

package/igh-ethercat/igh-ethercat.mk +1 -13

index b5221f8..32c49a3 100644
@@ -10,7 +10,6 @@ IGH_ETHERCAT_SOURCE = ethercat-$(IGH_ETHERCAT_VERSION).tar.bz2
 IGH_ETHERCAT_LICENSE = GPLv2 (IgH EtherCAT master), LGPLv2.1 (libraries)
 IGH_ETHERCAT_LICENSE_FILES = COPYING COPYING.LESSER
 
-IGH_ETHERCAT_DEPENDENCIES = linux
 IGH_ETHERCAT_INSTALL_STAGING = YES
 
 IGH_ETHERCAT_CONF_OPTS = \

@@ -22,16 +21,5 @@ IGH_ETHERCAT_CONF_OPTS += $(if $(BR2_PACKAGE_IGH_ETHERCAT_E1000),--enable-e1000,
 IGH_ETHERCAT_CONF_OPTS += $(if $(BR2_PACKAGE_IGH_ETHERCAT_E1000E),--enable-e1000e,--disable-e1000e)
 IGH_ETHERCAT_CONF_OPTS += $(if $(BR2_PACKAGE_IGH_ETHERCAT_R8169),--enable-r8169,--disable-r8169)
 
-define IGH_ETHERCAT_BUILD_MODULES
-	$(MAKE) -C $(@D) $(LINUX_MAKE_FLAGS) modules
-endef
-
-IGH_ETHERCAT_POST_BUILD_HOOKS += IGH_ETHERCAT_BUILD_MODULES
-
-define IGH_ETHERCAT_INSTALL_MODULES
-	$(MAKE) -C $(@D) $(LINUX_MAKE_FLAGS) modules_install
-endef
-
-IGH_ETHERCAT_POST_INSTALL_TARGET_HOOKS += IGH_ETHERCAT_INSTALL_MODULES
-
+$(eval $(kernel-module))
 $(eval $(autotools-package))

Passing the KERNEL_SRC variable is no longer needed, since it was only used to run commands like:

    $(MAKE) -C $(KERNEL_SRC) M=$(PWD) modules

which is basically what the new kernel-module infra is now doing.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

---
Changes v1 -> v2;
- explain why we get rid of a variable (Thomas)

---
 package/ktap/ktap.mk | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

package/ktap/ktap.mk +3 -4

index 4a0e51c..966b51c 100644
@@ -9,8 +9,6 @@ KTAP_SITE = $(call github,ktap,ktap,$(KTAP_VERSION))
 KTAP_LICENSE = GPLv2
 KTAP_LICENSE_FILES = LICENSE-GPL
 
-KTAP_DEPENDENCIES = linux
-
 ifeq ($(BR2_PACKAGE_ELFUTILS),y)
 KTAP_DEPENDENCIES += elfutils
 else

@@ -19,12 +17,13 @@ endif
 
 define KTAP_BUILD_CMDS
 	$(MAKE) -C $(@D) $(TARGET_CONFIGURE_OPTS) $(KTAP_FLAGS) ktap
-	$(MAKE) -C $(@D) $(LINUX_MAKE_FLAGS) KERNEL_SRC=$(LINUX_DIR) KVERSION=$(LINUX_VERSION_PROBED) mod
 endef
 
 define KTAP_INSTALL_TARGET_CMDS
 	$(INSTALL) -D -m755 $(@D)/ktap  $(TARGET_DIR)/usr/bin/ktap
-	$(MAKE) -C $(@D) $(LINUX_MAKE_FLAGS) KERNEL_SRC=$(LINUX_DIR) KVERSION=$(LINUX_VERSION_PROBED) modules_install
 endef
 
+KTAP_MODULE_MAKE_OPTS = KVERSION=$(LINUX_VERSION_PROBED)
+
+$(eval $(kernel-module))
 $(eval $(generic-package))

Signed-off-by "Yann E. MORIN" <[email protected]>

[Doug: add pre-build hook to build version.h]

Signed-off-by Doug Kehn <[email protected]>

define CRYPTODEV_LINUX_INSTALL_STAGING_CMDS

package/cryptodev-linux/cryptodev-linux.mk +1 -0

 	$(INSTALL) -D -m 644 $(@D)/crypto/cryptodev.h \
 		$(STAGING_DIR)/usr/include/crypto/cryptodev.h
 endef
 
+$(eval $(kernel-module))
 $(eval $(generic-package))

Quoted 2 lines

Signed-off-by "Yann E. MORIN" <[email protected]>

[Doug: add pre-build hook to build version.h]

The version.h thing deserves a slightly more explicit commit message, like:

The cryptodev-linux Makefile defines the following:

build: version.h

        make ${KERNEL_MAKE_OPTS} modules

modules_install:

        make -C $(KERNEL_DIR) SUBDIRS=`pwd` modules_install
        @echo "Installing cryptodev.h in $(PREFIX)/usr/include/crypto ..."
        @install -D crypto/cryptodev.h $(PREFIX)/usr/include/crypto/cryptodev.h

So when we use the kernel-module infrastructure, we have to explicitly handle version.h and cryptodev.h. cryptodev.h was already handled specifically because it goes to STAGING_DIR. For version.h, we add a pre-build hook.

That said, SUBDIRS= does not behave exactly the same as M=, but I don't know
what the difference is. So that part should probably be mentioned explicitly in the commit message as well.

Quoted 1 line

Signed-off-by Doug Kehn <[email protected]>

[snip]

package/cryptodev-linux/cryptodev-linux.mk line 20

Quoted 7 lines

package/cryptodev-linux/cryptodev-linux.mk +3 -4

-define CRYPTODEV_LINUX_INSTALL_TARGET_CMDS
-	$(MAKE) -C $(@D) $(LINUX_MAKE_FLAGS) KERNEL_DIR=$(LINUX_DIR) \
-		PREFIX=$(TARGET_DIR) modules_install
-endef
+CRYPTODEV_LINUX_MODULE_MAKE_OPTS = \
+	KERNEL_DIR=$(LINUX_DIR) \
+	PREFIX=$(TARGET_DIR)

PREFIX is only used for installing cryptodev.h, which is now no longer done. So
we don't need to define it, right? Same for KERNEL_DIR, it's only used by the rules we no longer use.

If you remove these, it should of course also be mentioned explicitly in the
commit message.

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 package/on2-8170-modules/on2-8170-modules.mk | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

package/on2-8170-modules/on2-8170-modules.mk +1 -10

index 209bcb5..b40ed86 100644
@@ -7,17 +7,8 @@
 ON2_8170_MODULES_VERSION = 73b08061d30789178e692bc332b73d1d9922bf39
 ON2_8170_MODULES_SITE = $(call github,alexandrebelloni,on2-8170-modules,$(ON2_8170_MODULES_VERSION))
 
-ON2_8170_MODULES_DEPENDENCIES = linux
-
 ON2_8170_MODULES_LICENSE = GPLv2+
 #There is no license file
 
-define ON2_8170_MODULES_BUILD_CMDS
-	$(MAKE) -C $(LINUX_DIR) $(LINUX_MAKE_FLAGS) M=$(@D)
-endef
-
-define ON2_8170_MODULES_INSTALL_TARGET_CMDS
-	$(MAKE) -C $(LINUX_DIR) $(LINUX_MAKE_FLAGS) M=$(@D) modules_install
-endef
-
+$(eval $(kernel-module))
 $(eval $(generic-package))

Git rid of first patch, no longer needed; rename remaining patch.

Signed-off-by "Yann E. MORIN" <[email protected]>

package/owl-linux/owl-linux.mk +0 -1

index 371cc02..844b11e 100644
@@ -10,14 +10,11 @@ OWL_LINUX_LICENSE = PROPRIETARY
 OWL_LINUX_LICENSE_FILES = LICENSE
 OWL_LINUX_REDISTRIBUTE = NO
 
-OWL_LINUX_DEPENDENCIES = linux

package/owl-linux/owl-linux.mk +4 -0

+# $(TOPDIR)  (i.e. the place where make is run from).
+# So, override PWD to the proper value.
+OWL_LINUX_MODULE_MAKE_OPTS = PWD=$(@D)
 
+$(eval $(kernel-module))
 $(eval $(generic-package))

Quoted 1 line

Git rid of first patch, no longer needed; rename remaining patch.

Quoted 2 lines

Signed-off-by "Yann E. MORIN" <[email protected]>

[snip]

Quoted 8 lines

package/owl-linux/owl-linux.mk +0 -0

index 371cc02..844b11e 100644
@@ -10,14 +10,11 @@ OWL_LINUX_LICENSE = PROPRIETARY
 OWL_LINUX_LICENSE_FILES = LICENSE
 OWL_LINUX_REDISTRIBUTE = NO
 

package/owl-linux/owl-linux.mk OWL_LINUX_LICENSE = PROPRIETARY · line 14

Quoted 11 lines

package/owl-linux/owl-linux.mk +2 -9

-OWL_LINUX_DEPENDENCIES = linux
-
-define OWL_LINUX_BUILD_CMDS
-	$(MAKE) -C $(@D) $(LINUX_MAKE_FLAGS) KERNELDIR=$(LINUX_DIR)
-endef
-
-define OWL_LINUX_INSTALL_TARGET_CMDS
-	$(MAKE) -C $(LINUX_DIR) $(LINUX_MAKE_FLAGS) M="$(@D)" modules_install
-endef
+# owl-linux wants to use $(PWD) to find its includes, however $(PWD) is not
+# the top of the package's source tree, but the top of Buildroot source tree

Unless I'm very much mistaken, PWD will be $(LINUX_DIR) when the Makefile is
included.

With that small thing fixed:

Reviewed-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

Arnout, All,

On 2015-06-12 01:13 +0200, Arnout Vandecappelle spake thusly:

Quoted 2 lines · Yann E. MORIN

Quoted 1 line

Git rid of first patch, no longer needed; rename remaining patch.

s/Git/Get/ Damn... :-]

Quoted 11 lines

Quoted 2 lines

Signed-off-by "Yann E. MORIN" <[email protected]>

[snip]

Quoted 8 lines

package/owl-linux/owl-linux.mk +0 -0

index 371cc02..844b11e 100644
@@ -10,14 +10,11 @@ OWL_LINUX_LICENSE = PROPRIETARY
 OWL_LINUX_LICENSE_FILES = LICENSE
 OWL_LINUX_REDISTRIBUTE = NO
 

Quoted 14 lines

package/owl-linux/owl-linux.mk OWL_LINUX_LICENSE = PROPRIETARY · line 14

Quoted 11 lines

package/owl-linux/owl-linux.mk +2 -9

-OWL_LINUX_DEPENDENCIES = linux
-
-define OWL_LINUX_BUILD_CMDS
-	$(MAKE) -C $(@D) $(LINUX_MAKE_FLAGS) KERNELDIR=$(LINUX_DIR)
-endef
-
-define OWL_LINUX_INSTALL_TARGET_CMDS
-	$(MAKE) -C $(LINUX_DIR) $(LINUX_MAKE_FLAGS) M="$(@D)" modules_install
-endef
+# owl-linux wants to use $(PWD) to find its includes, however $(PWD) is not
+# the top of the package's source tree, but the top of Buildroot source tree

Unless I'm very much mistaken, PWD will be $(LINUX_DIR) when the Makefile is
included.

I think you are very much mistaken, indeed! ;-)

What happens is that all commands we run are run from Buildroot's $(TOPDIR). So $(PWD) is set to that location.

So, this command, which we run from our Makefile:

    make -C $(LINUX_DIR) M=$(@D) foo-bar

would get its $(PWD) make-variable set from whatever value it had in the shell that called make, which happens to itself be called by Buildroot's Makefile, which runs from $(TOPDIR), always (even for out-of-tree builds).

Then, the Linux buildsystem does *not* chdir() when it builds modules, it just includes $(M)/Kbuild (or if it does not exist, $(M)/Makefile ), so $(PWD) as used in those will still be Buildroot's $(TOPDIR).

Quoted 3 lines

With that small thing fixed:

Reviewed-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

Still valid with that small thing *not* fixed? ;-)

Regards,
Yann E. MORIN.

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 package/simicsfs/simicsfs.mk | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

package/simicsfs/simicsfs.mk +1 -9

index 51987c5..91ed413 100644
@@ -8,14 +8,6 @@ SIMICSFS_VERSION = 1.18
 SIMICSFS_SITE = http://download.simics.net/pub
 SIMICSFS_LICENSE = GPLv2+
 SIMICSFS_LICENSE_FILES = hostfs.h
-SIMICSFS_DEPENDENCIES = linux
-
-define SIMICSFS_BUILD_CMDS
-	$(MAKE) $(LINUX_MAKE_FLAGS) -C $(LINUX_DIR) M=$(@D) modules
-endef
-
-define SIMICSFS_INSTALL_TARGET_CMDS
-	$(MAKE) $(LINUX_MAKE_FLAGS) -C $(LINUX_DIR) M=$(@D) INSTALL_MOD_PATH=$(TARGET_DIR) modules_install
-endef
 
+$(eval $(kernel-module))
 $(eval $(generic-package))

This series introduces a new infrastructure to help writing packages that build kernel modules.

Hello All,

On 2015-06-10 21:22 +0200, Yann E. MORIN spake thusly:

Quoted 2 lines

This series introduces a new infrastructure to help writing packages that build kernel modules.

I'll mark this series as "Changes Requested" in patchwork, because I took Jeremy's comment into account, and because I was able to improve it yet a bit more and convert one more kernel-module-providing package (namely, nvidia-driver; who would have thought it would be possible? ;-) ).

So, I'll respin later, but I'm still interested in reviews on this iteration.

Regards,
Yann E. MORIN.

Quoted 6 lines

Hello All,

On 2015-06-10 21:22 +0200, Yann E. MORIN spake thusly:

Quoted 2 lines

This series introduces a new infrastructure to help writing packages that build kernel modules.

Quoted 4 lines

I'll mark this series as "Changes Requested" in patchwork, because I took Jeremy's comment into account, and because I was able to improve it yet a bit more and convert one more kernel-module-providing package (namely, nvidia-driver; who would have thought it would be possible? ;-) ).

Early feedback on that one:

NVIDIA_DRIVER_HEADERS_H -> NVIDIA_DRIVER_MODULE_BUILD_HEADERS

Commit message should explain why the custom install isn't needed anymore either.