diff mbox

[01/11] package-infra: add helper to build kernel modules

Message ID 7f37dfb955f969acf85ea5044c1c4020096d2270.1433628825.git.yann.morin.1998@free.fr
State Changes Requested
Headers show

Commit Message

Yann E. MORIN June 6, 2015, 10:20 p.m. UTC
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 dupliacte (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 infrastrucutre, 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.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/Makefile.in          |  1 +
 package/pkg-kernel-module.mk | 94 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+)
 create mode 100644 package/pkg-kernel-module.mk

Comments

Baruch Siach June 7, 2015, 3:22 a.m. UTC | #1
Hi Yann,

On Sun, Jun 07, 2015 at 12:20:37AM +0200, Yann E. MORIN wrote:
> +################################################################################
> +# 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, including a HOST_ prefix
> +#             for host packages
> +#  argument 3 is the uppercase package name, without the HOST_ prefix
> +#             for host packages

What is the meaning of building "host" kernel module? These are the target 
kernel headers we are using here. Besides, you do not provide a 
host-kernel-module macro, at least in this patch.

> +#  argument 4 is the type (always 'target')

And here only target is allowed.

baruch
Yann E. MORIN June 7, 2015, 9:59 a.m. UTC | #2
Baruch, All,

On 2015-06-07 06:22 +0300, Baruch Siach spake thusly:
> On Sun, Jun 07, 2015 at 12:20:37AM +0200, Yann E. MORIN wrote:
> > +################################################################################
> > +# 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, including a HOST_ prefix
> > +#             for host packages
> > +#  argument 3 is the uppercase package name, without the HOST_ prefix
> > +#             for host packages
> 
> What is the meaning of building "host" kernel module? These are the target 
> kernel headers we are using here.

Indeed, I just forgot to strip it out when I copied from another infra.

> Besides, you do not provide a 
> host-kernel-module macro, at least in this patch.

And I explicitly dociument that it is not availble (see below, as you
already noticed, and the manual, in the following patch).

> > +#  argument 4 is the type (always 'target')
> 
> And here only target is allowed.

Yup.

I'll fix that before resubmitting. Thanks! :-)

Regards,
Yann E. MORIN.
Doug Kehn June 8, 2015, 12:49 p.m. UTC | #3
On Sun, Jun 07, 2015 at 12:20:37AM +0200, Yann E. MORIN wrote:
> 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 dupliacte (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 infrastrucutre, 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.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Tested-by: Doug Kehn <rdkehn@yahoo.com>
> ---
>  package/Makefile.in          |  1 +
>  package/pkg-kernel-module.mk | 94 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 95 insertions(+)
>  create mode 100644 package/pkg-kernel-module.mk
> 
> diff --git a/package/Makefile.in b/package/Makefile.in
> index c02d31f..180fd46 100644
> --- a/package/Makefile.in
> +++ b/package/Makefile.in
> @@ -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
> diff --git a/package/pkg-kernel-module.mk b/package/pkg-kernel-module.mk
> new file mode 100644
> index 0000000..2a2a2cb
> --- /dev/null
> +++ b/package/pkg-kernel-module.mk
> @@ -0,0 +1,94 @@
> +################################################################################
> +# kernel module infrastructure for building Linux kernel modules
> +#
> +# This file implements an frastructure 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.
> +#
> +# In terms of implementation, this infrastructure requires the .mk file to
> +# only specify metadata information about the package: name, version,
> +# download URL, etc.
> +#
> +# It 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, including a HOST_ prefix
> +#             for host packages
> +#  argument 3 is the uppercase package name, without the HOST_ prefix
> +#             for host packages
> +#  argument 4 is the type (always 'target')
> +################################################################################
> +
> +define inner-kernel-module
> +
> +# The kernel must be built first.
> +$(2)_DEPENDENCIES += linux
> +
> +# Duplicate that from pkg-generic because we need it now
> +ifndef $(2)_MAKE
> +  $(2)_MAKE = $(MAKE)
> +endif
> +
> +ifndef $(2)_MODULE_SUBDIRS
> +  $(2)_MODULE_SUBDIRS = .
> +endif
> +
> +# Build the kernel module(s)
> +define $(2)_KERNEL_MODULES_BUILD
> +	$$(foreach d,$$($(2)_MODULE_SUBDIRS), \
> +		@$$(call MESSAGE,"Building kernel module '$$(d)'")$$(sep) \
> +		$$($$(PKG)_MAKE) -C $$(LINUX_DIR) \
> +			$$(LINUX_MAKE_FLAGS) \
> +			$$($(2)_MODULE_MAKE_OPTS) \
> +			M=$$($(2)_DIR)/$$(d) \
> +			modules$$(sep))
> +endef
> +$(2)_POST_BUILD_HOOKS += $(2)_KERNEL_MODULES_BUILD
> +
> +# Install the kernel module(s)
> +define $(2)_KERNEL_MODULES_INSTALL
> +	$$(foreach d,$$($(2)_MODULE_SUBDIRS), \
> +		@$$(call MESSAGE,"Installing kernel module '$$(d)'")$$(sep) \
> +		$$($$(PKG)_MAKE) -C $$(LINUX_DIR) \
> +			$$(LINUX_MAKE_FLAGS) \
> +			$$($(2)_MODULE_MAKE_OPTS) \
> +			M=$$($(2)_DIR)/$$(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)),$(call UPPERCASE,$(pkgname)),target)
> -- 
> 1.9.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Arnout Vandecappelle June 8, 2015, 9:09 p.m. UTC | #4
On 06/07/15 00:20, Yann E. MORIN wrote:
> 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 dupliacte (or rewrite) the same code over-and-over again.

 duplicate

> 
> Introduce a new infrastructure that provides helpers to build kernel
> modules, so packages do not have to duplicate/rewrite that.
> 
> The infrastrucutre, unlike any other package infra, is not standalone.

 infrastructure

> 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.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  package/Makefile.in          |  1 +
>  package/pkg-kernel-module.mk | 94 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 95 insertions(+)
>  create mode 100644 package/pkg-kernel-module.mk
> 
> diff --git a/package/Makefile.in b/package/Makefile.in
> index c02d31f..180fd46 100644
> --- a/package/Makefile.in
> +++ b/package/Makefile.in
> @@ -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

 I think we should at some point put this either in alphabetical or some other
logical order. But for now, it's chaos anyway so OK to just add at the end.

> diff --git a/package/pkg-kernel-module.mk b/package/pkg-kernel-module.mk
> new file mode 100644
> index 0000000..2a2a2cb
> --- /dev/null
> +++ b/package/pkg-kernel-module.mk
> @@ -0,0 +1,94 @@
> +################################################################################
> +# kernel module infrastructure for building Linux kernel modules
> +#
> +# This file implements an frastructure that eases development of package .mk

 infrastructure

> +# files for out-of-tree Linux kernel modules. It should be used for all
> +# packages that build a Linux kernel module.

 Well, as you say in the cover text, there are a few packages with awkward build
systems that can't use this infra.

> +#
> +# In terms of implementation, this infrastructure requires the .mk file to
> +# only specify metadata information about the package: name, version,
> +# download URL, etc.

 I think this is another cut&paste from pkg-generic that is not appropriate.

> +#
> +# It 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))

 That is not so nice, but I don't see a better alternative.

> +#
> +# 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, including a HOST_ prefix
> +#             for host packages
> +#  argument 3 is the uppercase package name, without the HOST_ prefix
> +#             for host packages
> +#  argument 4 is the type (always 'target')

 $(2) is the only one that is used...

> +################################################################################
> +
> +define inner-kernel-module
> +
> +# The kernel must be built first.
> +$(2)_DEPENDENCIES += linux
> +
> +# Duplicate that from pkg-generic because we need it now
> +ifndef $(2)_MAKE
> +  $(2)_MAKE = $(MAKE)
> +endif

 I don't see why this is needed... The defines below will only be expanded when
the rule is executed (otherwise $(PKG) would not even be defined), so the
definition from pkg-generic is enough, no?

> +
> +ifndef $(2)_MODULE_SUBDIRS
> +  $(2)_MODULE_SUBDIRS = .
> +endif
> +
> +# Build the kernel module(s)
> +define $(2)_KERNEL_MODULES_BUILD
> +	$$(foreach d,$$($(2)_MODULE_SUBDIRS), \
> +		@$$(call MESSAGE,"Building kernel module '$$(d)'")$$(sep) \

 I think that "Building kernel module '.'" looks a bit weird... But again, I
can't think of an alternative.

> +		$$($$(PKG)_MAKE) -C $$(LINUX_DIR) \

 Missing $(LINUX_MAKE_ENV) (or at least TARGET_MAKE_ENV, but I think
LINUX_MAKE_ENV is more appropriate even if it just adds BR_BINARIES_DIR).


> +			$$(LINUX_MAKE_FLAGS) \
> +			$$($(2)_MODULE_MAKE_OPTS) \
> +			M=$$($(2)_DIR)/$$(d) \

 We usually use $(@D) instead of $(2)_DIR.

> +			modules$$(sep))
> +endef
> +$(2)_POST_BUILD_HOOKS += $(2)_KERNEL_MODULES_BUILD
> +
> +# Install the kernel module(s)
> +define $(2)_KERNEL_MODULES_INSTALL
> +	$$(foreach d,$$($(2)_MODULE_SUBDIRS), \
> +		@$$(call MESSAGE,"Installing kernel module '$$(d)'")$$(sep) \
> +		$$($$(PKG)_MAKE) -C $$(LINUX_DIR) \
> +			$$(LINUX_MAKE_FLAGS) \
> +			$$($(2)_MODULE_MAKE_OPTS) \
> +			M=$$($(2)_DIR)/$$(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)),$(call UPPERCASE,$(pkgname)),target)

 $(2) is the only one used, so just

kernel-module = $(call inner-kernel-module,$(call UPPERCASE,$(pkgname)))


 Regards,
 Arnout

>
Yann E. MORIN June 8, 2015, 9:25 p.m. UTC | #5
Arnout, All,

On 2015-06-08 23:09 +0200, Arnout Vandecappelle spake thusly:
> On 06/07/15 00:20, Yann E. MORIN wrote:
> > 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 dupliacte (or rewrite) the same code over-and-over again.
> 
>  duplicate
> 
> > 
> > Introduce a new infrastructure that provides helpers to build kernel
> > modules, so packages do not have to duplicate/rewrite that.
> > 
> > The infrastrucutre, unlike any other package infra, is not standalone.
> 
>  infrastructure

Will fix both.

> > 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.
> > 
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > ---
> >  package/Makefile.in          |  1 +
> >  package/pkg-kernel-module.mk | 94 ++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 95 insertions(+)
> >  create mode 100644 package/pkg-kernel-module.mk
> > 
> > diff --git a/package/Makefile.in b/package/Makefile.in
> > index c02d31f..180fd46 100644
> > --- a/package/Makefile.in
> > +++ b/package/Makefile.in
> > @@ -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
> 
>  I think we should at some point put this either in alphabetical or some other
> logical order. But for now, it's chaos anyway so OK to just add at the end.

Yup, the rule so far has been "append new infra". So I did the same...

> > diff --git a/package/pkg-kernel-module.mk b/package/pkg-kernel-module.mk
> > new file mode 100644
> > index 0000000..2a2a2cb
> > --- /dev/null
> > +++ b/package/pkg-kernel-module.mk
> > @@ -0,0 +1,94 @@
> > +################################################################################
> > +# kernel module infrastructure for building Linux kernel modules
> > +#
> > +# This file implements an frastructure that eases development of package .mk
> 
>  infrastructure

Yup.

> > +# files for out-of-tree Linux kernel modules. It should be used for all
> > +# packages that build a Linux kernel module.
> 
>  Well, as you say in the cover text, there are a few packages with awkward build
> systems that can't use this infra.

Will rephrase.

> > +#
> > +# In terms of implementation, this infrastructure requires the .mk file to
> > +# only specify metadata information about the package: name, version,
> > +# download URL, etc.
> 
>  I think this is another cut&paste from pkg-generic that is not appropriate.

Yup, will remove.

> > +#
> > +# It 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))
> 
>  That is not so nice, but I don't see a better alternative.
> 
> > +#
> > +# 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, including a HOST_ prefix
> > +#             for host packages
> > +#  argument 3 is the uppercase package name, without the HOST_ prefix
> > +#             for host packages
> > +#  argument 4 is the type (always 'target')
> 
>  $(2) is the only one that is used...

Already fixed locally, see:
    http://git.buildroot.org/~ymorin/git/buildroot/commit/?h=yem/kernel-modules&id=37668dc37654cdcb9e874e62ba994990f8835486

> > +################################################################################
> > +
> > +define inner-kernel-module
> > +
> > +# The kernel must be built first.
> > +$(2)_DEPENDENCIES += linux
> > +
> > +# Duplicate that from pkg-generic because we need it now
> > +ifndef $(2)_MAKE
> > +  $(2)_MAKE = $(MAKE)
> > +endif
> 
>  I don't see why this is needed... The defines below will only be expanded when
> the rule is executed (otherwise $(PKG) would not even be defined), so the
> definition from pkg-generic is enough, no?

That's what I thought, too, but encountered an error in the beginings of
this infra. I'll retest without that.

> > +ifndef $(2)_MODULE_SUBDIRS
> > +  $(2)_MODULE_SUBDIRS = .
> > +endif
> > +
> > +# Build the kernel module(s)
> > +define $(2)_KERNEL_MODULES_BUILD
> > +	$$(foreach d,$$($(2)_MODULE_SUBDIRS), \
> > +		@$$(call MESSAGE,"Building kernel module '$$(d)'")$$(sep) \
> 
>  I think that "Building kernel module '.'" looks a bit weird... But again, I
> can't think of an alternative.

Neither do I. I'll see if I can do something about that...

> > +		$$($$(PKG)_MAKE) -C $$(LINUX_DIR) \
> 
>  Missing $(LINUX_MAKE_ENV) (or at least TARGET_MAKE_ENV, but I think
> LINUX_MAKE_ENV is more appropriate even if it just adds BR_BINARIES_DIR).
> 
> 
> > +			$$(LINUX_MAKE_FLAGS) \
> > +			$$($(2)_MODULE_MAKE_OPTS) \
> > +			M=$$($(2)_DIR)/$$(d) \
> 
>  We usually use $(@D) instead of $(2)_DIR.

Ack.

> > +			modules$$(sep))
> > +endef
> > +$(2)_POST_BUILD_HOOKS += $(2)_KERNEL_MODULES_BUILD
> > +
> > +# Install the kernel module(s)
> > +define $(2)_KERNEL_MODULES_INSTALL
> > +	$$(foreach d,$$($(2)_MODULE_SUBDIRS), \
> > +		@$$(call MESSAGE,"Installing kernel module '$$(d)'")$$(sep) \
> > +		$$($$(PKG)_MAKE) -C $$(LINUX_DIR) \
> > +			$$(LINUX_MAKE_FLAGS) \
> > +			$$($(2)_MODULE_MAKE_OPTS) \
> > +			M=$$($(2)_DIR)/$$(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)),$(call UPPERCASE,$(pkgname)),target)
> 
>  $(2) is the only one used, so just
> 
> kernel-module = $(call inner-kernel-module,$(call UPPERCASE,$(pkgname)))

Yup, already done. ;-)

Thanks! :-)
Thomas Petazzoni June 8, 2015, 9:33 p.m. UTC | #6
Dear Yann E. MORIN,

On Mon, 8 Jun 2015 23:25:19 +0200, Yann E. MORIN wrote:

> > > +# Build the kernel module(s)
> > > +define $(2)_KERNEL_MODULES_BUILD
> > > +	$$(foreach d,$$($(2)_MODULE_SUBDIRS), \
> > > +		@$$(call MESSAGE,"Building kernel module '$$(d)'")$$(sep) \
> > 
> >  I think that "Building kernel module '.'" looks a bit weird... But again, I
> > can't think of an alternative.
> 
> Neither do I. I'll see if I can do something about that...

Just don't show a message for each subdir, only one for the overall
build command:

	$$(call MESAGE,"Building kernel module(s)")

Thomas
Yann E. MORIN June 8, 2015, 9:35 p.m. UTC | #7
Thomas, All,

On 2015-06-08 23:33 +0200, Thomas Petazzoni spake thusly:
> On Mon, 8 Jun 2015 23:25:19 +0200, Yann E. MORIN wrote:
> > > > +# Build the kernel module(s)
> > > > +define $(2)_KERNEL_MODULES_BUILD
> > > > +	$$(foreach d,$$($(2)_MODULE_SUBDIRS), \
> > > > +		@$$(call MESSAGE,"Building kernel module '$$(d)'")$$(sep) \
> > > 
> > >  I think that "Building kernel module '.'" looks a bit weird... But again, I
> > > can't think of an alternative.
> > 
> > Neither do I. I'll see if I can do something about that...
> 
> Just don't show a message for each subdir, only one for the overall
> build command:
> 
> 	$$(call MESAGE,"Building kernel module(s)")

Yep, I was going to do so. Thanks for confirming! :-)

Regards,
Yann E. MORIN.
Yann E. MORIN June 8, 2015, 9:44 p.m. UTC | #8
Arnout, All,

On 2015-06-08 23:09 +0200, Arnout Vandecappelle spake thusly:
> On 06/07/15 00:20, Yann E. MORIN wrote:
[--SNIP--]
> > +# Duplicate that from pkg-generic because we need it now
> > +ifndef $(2)_MAKE
> > +  $(2)_MAKE = $(MAKE)
> > +endif
> 
>  I don't see why this is needed... The defines below will only be expanded when
> the rule is executed (otherwise $(PKG) would not even be defined), so the
> definition from pkg-generic is enough, no?

Well, I tried to remove it, and it does not work (indetation added by me):

>>> cryptodev-linux 1.7 Building kernel module(s)
C /home/ymorin/dev/buildroot/O/build/linux-4.0.4 HOSTCC="/usr/bin/gcc" HOSTCFLAGS="" ARCH=i386
    INSTALL_MOD_PATH=/home/ymorin/dev/buildroot/O/target CROSS_COMPILE="/home/ymorin/dev/buildroot/O/host/usr/bin/i686-pc-linux-gnu-"
    DEPMOD=/home/ymorin/dev/buildroot/O/host/sbin/depmod KERNEL_DIR=/home/ymorin/dev/buildroot/O/build/linux-4.0.4
    PREFIX=/home/ymorin/dev/buildroot/O/target M=/home/ymorin/dev/buildroot/O/build/cryptodev-linux-1.7/. modules
/bin/sh: C: command not found
make[1]:
[/home/ymorin/dev/buildroot/O/build/cryptodev-linux-1.7/.stamp_built]
Error 127 (ignored)

Regards,
Yann E. MORIN.
Arnout Vandecappelle June 8, 2015, 9:52 p.m. UTC | #9
On 06/08/15 23:44, Yann E. MORIN wrote:
> Arnout, All,
> 
> On 2015-06-08 23:09 +0200, Arnout Vandecappelle spake thusly:
>> On 06/07/15 00:20, Yann E. MORIN wrote:
> [--SNIP--]
>>> +# Duplicate that from pkg-generic because we need it now
>>> +ifndef $(2)_MAKE
>>> +  $(2)_MAKE = $(MAKE)
>>> +endif
>>
>>  I don't see why this is needed... The defines below will only be expanded when
>> the rule is executed (otherwise $(PKG) would not even be defined), so the
>> definition from pkg-generic is enough, no?
> 
> Well, I tried to remove it, and it does not work (indetation added by me):

 D'oh, it's actually defined in pkg-autotools, not pkg-generic, but the comment
above confused me. Which in fact makes the comment redundant, because it's not a
redifinition.

 That said, I think it's better like this:

$(2)_MAKE ?= $$(MAKE)


 Regards,
 Arnout

> 
>>>> cryptodev-linux 1.7 Building kernel module(s)
> C /home/ymorin/dev/buildroot/O/build/linux-4.0.4 HOSTCC="/usr/bin/gcc" HOSTCFLAGS="" ARCH=i386
>     INSTALL_MOD_PATH=/home/ymorin/dev/buildroot/O/target CROSS_COMPILE="/home/ymorin/dev/buildroot/O/host/usr/bin/i686-pc-linux-gnu-"
>     DEPMOD=/home/ymorin/dev/buildroot/O/host/sbin/depmod KERNEL_DIR=/home/ymorin/dev/buildroot/O/build/linux-4.0.4
>     PREFIX=/home/ymorin/dev/buildroot/O/target M=/home/ymorin/dev/buildroot/O/build/cryptodev-linux-1.7/. modules
> /bin/sh: C: command not found
> make[1]:
> [/home/ymorin/dev/buildroot/O/build/cryptodev-linux-1.7/.stamp_built]
> Error 127 (ignored)
> 
> Regards,
> Yann E. MORIN.
>
Yann E. MORIN June 8, 2015, 9:58 p.m. UTC | #10
Arnout, All,

On 2015-06-08 23:52 +0200, Arnout Vandecappelle spake thusly:
> On 06/08/15 23:44, Yann E. MORIN wrote:
> > On 2015-06-08 23:09 +0200, Arnout Vandecappelle spake thusly:
> >> On 06/07/15 00:20, Yann E. MORIN wrote:
> >>> +# Duplicate that from pkg-generic because we need it now
> >>> +ifndef $(2)_MAKE
> >>> +  $(2)_MAKE = $(MAKE)
> >>> +endif
> >>
> >>  I don't see why this is needed... The defines below will only be expanded when
> >> the rule is executed (otherwise $(PKG) would not even be defined), so the
> >> definition from pkg-generic is enough, no?
> > 
> > Well, I tried to remove it, and it does not work (indetation added by me):
> 
>  D'oh, it's actually defined in pkg-autotools, not pkg-generic, but the comment
> above confused me. Which in fact makes the comment redundant, because it's not a
> redifinition.

Ah, yes, right, my bad.

I tested with a generic-package. Maybe an autotools-package (or any
other infra) would have no issue, since it is defined for them.

>  That said, I think it's better like this:
> 
> $(2)_MAKE ?= $$(MAKE)

Yup, I'll do (already usinf $$(MaKE) locally.

Thanks! :-)

Regards,
Yann E. MORIN.
Arnout Vandecappelle June 8, 2015, 9:59 p.m. UTC | #11
On 06/08/15 23:58, Yann E. MORIN wrote:
> Arnout, All,
> 
> On 2015-06-08 23:52 +0200, Arnout Vandecappelle spake thusly:
>> On 06/08/15 23:44, Yann E. MORIN wrote:
>>> On 2015-06-08 23:09 +0200, Arnout Vandecappelle spake thusly:
>>>> On 06/07/15 00:20, Yann E. MORIN wrote:
>>>>> +# Duplicate that from pkg-generic because we need it now
>>>>> +ifndef $(2)_MAKE
>>>>> +  $(2)_MAKE = $(MAKE)
>>>>> +endif
>>>>
>>>>  I don't see why this is needed... The defines below will only be expanded when
>>>> the rule is executed (otherwise $(PKG) would not even be defined), so the
>>>> definition from pkg-generic is enough, no?
>>>
>>> Well, I tried to remove it, and it does not work (indetation added by me):
>>
>>  D'oh, it's actually defined in pkg-autotools, not pkg-generic, but the comment
>> above confused me. Which in fact makes the comment redundant, because it's not a
>> redifinition.
> 
> Ah, yes, right, my bad.
> 
> I tested with a generic-package. Maybe an autotools-package (or any
> other infra) would have no issue, since it is defined for them.
> 
>>  That said, I think it's better like this:
>>
>> $(2)_MAKE ?= $$(MAKE)
> 
> Yup, I'll do (already usinf $$(MaKE) locally.

 I also mean ?= is better than ifndef.

 Regards,
 Arnout

> 
> Thanks! :-)
> 
> Regards,
> Yann E. MORIN.
>
diff mbox

Patch

diff --git a/package/Makefile.in b/package/Makefile.in
index c02d31f..180fd46 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -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
diff --git a/package/pkg-kernel-module.mk b/package/pkg-kernel-module.mk
new file mode 100644
index 0000000..2a2a2cb
--- /dev/null
+++ b/package/pkg-kernel-module.mk
@@ -0,0 +1,94 @@ 
+################################################################################
+# kernel module infrastructure for building Linux kernel modules
+#
+# This file implements an frastructure 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.
+#
+# In terms of implementation, this infrastructure requires the .mk file to
+# only specify metadata information about the package: name, version,
+# download URL, etc.
+#
+# It 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, including a HOST_ prefix
+#             for host packages
+#  argument 3 is the uppercase package name, without the HOST_ prefix
+#             for host packages
+#  argument 4 is the type (always 'target')
+################################################################################
+
+define inner-kernel-module
+
+# The kernel must be built first.
+$(2)_DEPENDENCIES += linux
+
+# Duplicate that from pkg-generic because we need it now
+ifndef $(2)_MAKE
+  $(2)_MAKE = $(MAKE)
+endif
+
+ifndef $(2)_MODULE_SUBDIRS
+  $(2)_MODULE_SUBDIRS = .
+endif
+
+# Build the kernel module(s)
+define $(2)_KERNEL_MODULES_BUILD
+	$$(foreach d,$$($(2)_MODULE_SUBDIRS), \
+		@$$(call MESSAGE,"Building kernel module '$$(d)'")$$(sep) \
+		$$($$(PKG)_MAKE) -C $$(LINUX_DIR) \
+			$$(LINUX_MAKE_FLAGS) \
+			$$($(2)_MODULE_MAKE_OPTS) \
+			M=$$($(2)_DIR)/$$(d) \
+			modules$$(sep))
+endef
+$(2)_POST_BUILD_HOOKS += $(2)_KERNEL_MODULES_BUILD
+
+# Install the kernel module(s)
+define $(2)_KERNEL_MODULES_INSTALL
+	$$(foreach d,$$($(2)_MODULE_SUBDIRS), \
+		@$$(call MESSAGE,"Installing kernel module '$$(d)'")$$(sep) \
+		$$($$(PKG)_MAKE) -C $$(LINUX_DIR) \
+			$$(LINUX_MAKE_FLAGS) \
+			$$($(2)_MODULE_MAKE_OPTS) \
+			M=$$($(2)_DIR)/$$(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)),$(call UPPERCASE,$(pkgname)),target)