[PATCH v2 1/3] example-module: improve Makefile to be more realistic

[PATCH v2 1/3] example-module: improve Makefile to be more realistic

[Henning Schild]

A real out-of-tree module would be able to "make" on its own, finding
its own KDIR and not rely on some external entity to provide that. But
of cause allow KDIR to be provided from outside.

Fixes: 8d9e4e3c0857 ("Add exemplary kernel module")
Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 .../example-module/files/src/Makefile              | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/meta-isar/recipes-kernel/example-module/files/src/Makefile b/meta-isar/recipes-kernel/example-module/files/src/Makefile
index 2561cfd410e5..df3241652cf1 100644
--- a/meta-isar/recipes-kernel/example-module/files/src/Makefile
+++ b/meta-isar/recipes-kernel/example-module/files/src/Makefile
@@ -1,8 +1,20 @@
 # Example module
 #
 # This software is a part of ISAR.
-# Copyright (c) Siemens AG, 2018
+# Copyright (c) Siemens AG, 2018-2022
 #
 # SPDX-License-Identifier: GPL-2.0
 
 obj-m := example-module.o
+
+INSTALL_MOD_PATH ?= $(DESTDIR)
+export INSTALL_MOD_PATH
+
+KDIR ?= /lib/modules/$(shell uname -r)/build
+
+modules modules_install clean:
+	$(MAKE) -C $(KDIR) M=$(PWD) $@
+
+install: modules_install
+
+.PHONY: modules modules_install install clean
-- 
2.35.1

[PATCH v2 2/3] linux-module: honor modules Makefile

[Henning Schild]

External modules might have their own appends and target definitions in
their Makefile. All we need to give them is the target name and KDIR,
not dive into KDIR right away.

Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 meta/recipes-kernel/linux-module/files/debian/rules.tmpl | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/meta/recipes-kernel/linux-module/files/debian/rules.tmpl b/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
index d3bd7dc30f21..0d16186b5ff3 100755
--- a/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
+++ b/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
@@ -48,13 +48,13 @@ KDIR := $(shell dpkg -L $(KERNEL_DEP) | grep "/lib/modules/.*/build")
 endif
 
 override_dh_auto_clean:
-	$(MAKE) -C $(KDIR) M=$(PWD) clean
+	$(MAKE) KDIR=$(KDIR) clean
 
 override_dh_auto_build:
-	$(MAKE) -C $(KDIR) M=$(PWD) modules
+	$(MAKE) KDIR=$(KDIR) modules
 
 override_dh_auto_install:
-	$(MAKE) -C $(KDIR) M=$(PWD) INSTALL_MOD_PATH=$(PWD)/debian/${PN} modules_install
+	$(MAKE) KDIR=$(KDIR) INSTALL_MOD_PATH=$(PWD)/debian/${PN} modules_install
 
 %:
 	CFLAGS= LDFLAGS= dh $@ --parallel
-- 
2.35.1

Re: [PATCH v2 2/3] linux-module: honor modules Makefile

[Jan Kiszka]

On 27.08.22 10:59, Henning Schild wrote:
> External modules might have their own appends and target definitions in
> their Makefile. All we need to give them is the target name and KDIR,
> not dive into KDIR right away.

My observation with external modules makefile having their own rules is
that they generally also have their own KDIR variables. There is no
standard way of telling those pick up the kernel from a standard place
and install it at the standard location.

> 
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
>  meta/recipes-kernel/linux-module/files/debian/rules.tmpl | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/meta/recipes-kernel/linux-module/files/debian/rules.tmpl b/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
> index d3bd7dc30f21..0d16186b5ff3 100755
> --- a/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
> +++ b/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
> @@ -48,13 +48,13 @@ KDIR := $(shell dpkg -L $(KERNEL_DEP) | grep "/lib/modules/.*/build")
>  endif
>  
>  override_dh_auto_clean:
> -	$(MAKE) -C $(KDIR) M=$(PWD) clean
> +	$(MAKE) KDIR=$(KDIR) clean
>  
>  override_dh_auto_build:
> -	$(MAKE) -C $(KDIR) M=$(PWD) modules
> +	$(MAKE) KDIR=$(KDIR) modules
>  
>  override_dh_auto_install:
> -	$(MAKE) -C $(KDIR) M=$(PWD) INSTALL_MOD_PATH=$(PWD)/debian/${PN} modules_install
> +	$(MAKE) KDIR=$(KDIR) INSTALL_MOD_PATH=$(PWD)/debian/${PN} modules_install
>  
>  %:
>  	CFLAGS= LDFLAGS= dh $@ --parallel

This breaks existing users with makefile that are unusable or do not
have own rules. I would refrain from that breakage.

If you have a more complex case than what we support, you can simply
provide your own rules and can then even account for the other
specialties that usually come with such module makefiles.

Same arguing applies to patch 3.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

Re: [PATCH v2 2/3] linux-module: honor modules Makefile

[Henning Schild]

Am Thu, 1 Sep 2022 18:08:08 +0200
schrieb Jan Kiszka <jan.kiszka@siemens.com>:

> On 27.08.22 10:59, Henning Schild wrote:
> > External modules might have their own appends and target
> > definitions in their Makefile. All we need to give them is the
> > target name and KDIR, not dive into KDIR right away.  
> 
> My observation with external modules makefile having their own rules
> is that they generally also have their own KDIR variables. There is no
> standard way of telling those pick up the kernel from a standard place
> and install it at the standard location.
> 
> > 
> > Signed-off-by: Henning Schild <henning.schild@siemens.com>
> > ---
> >  meta/recipes-kernel/linux-module/files/debian/rules.tmpl | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git
> > a/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
> > b/meta/recipes-kernel/linux-module/files/debian/rules.tmpl index
> > d3bd7dc30f21..0d16186b5ff3 100755 ---
> > a/meta/recipes-kernel/linux-module/files/debian/rules.tmpl +++
> > b/meta/recipes-kernel/linux-module/files/debian/rules.tmpl @@
> > -48,13 +48,13 @@ KDIR := $(shell dpkg -L $(KERNEL_DEP) | grep
> > "/lib/modules/.*/build") endif override_dh_auto_clean:
> > -	$(MAKE) -C $(KDIR) M=$(PWD) clean
> > +	$(MAKE) KDIR=$(KDIR) clean
> >  
> >  override_dh_auto_build:
> > -	$(MAKE) -C $(KDIR) M=$(PWD) modules
> > +	$(MAKE) KDIR=$(KDIR) modules
> >  
> >  override_dh_auto_install:
> > -	$(MAKE) -C $(KDIR) M=$(PWD)
> > INSTALL_MOD_PATH=$(PWD)/debian/${PN} modules_install
> > +	$(MAKE) KDIR=$(KDIR) INSTALL_MOD_PATH=$(PWD)/debian/${PN}
> > modules_install 
> >  %:
> >  	CFLAGS= LDFLAGS= dh $@ --parallel  
> 
> This breaks existing users with makefile that are unusable or do not
> have own rules. I would refrain from that breakage.
> 
> If you have a more complex case than what we support, you can simply
> provide your own rules and can then even account for the other
> specialties that usually come with such module makefiles.
> 
> Same arguing applies to patch 3.

I am not sure i would agree that the example Makefile is indeed valid,
since it does not allow self sustained building with a simple "make
modules_install". But yes for modules with such Makefiles this would be
a breaking change.

Will go and carry the special rules entries downstream, while i still
think Isar should take that and rather make those weirdos carry their
own rules ... accepting a breakage.

But i have no clue how "weird" vs "common" it would be to have such a
super-minimal Makefile. And my case is probably also not "common"
enough to ague for too long.

Henning

> Jan
> 

Re: [PATCH v2 2/3] linux-module: honor modules Makefile

[Jan Kiszka]

On 01.09.22 19:17, Henning Schild wrote:
> Am Thu, 1 Sep 2022 18:08:08 +0200
> schrieb Jan Kiszka <jan.kiszka@siemens.com>:
> 
>> On 27.08.22 10:59, Henning Schild wrote:
>>> External modules might have their own appends and target
>>> definitions in their Makefile. All we need to give them is the
>>> target name and KDIR, not dive into KDIR right away.  
>>
>> My observation with external modules makefile having their own rules
>> is that they generally also have their own KDIR variables. There is no
>> standard way of telling those pick up the kernel from a standard place
>> and install it at the standard location.
>>
>>>
>>> Signed-off-by: Henning Schild <henning.schild@siemens.com>
>>> ---
>>>  meta/recipes-kernel/linux-module/files/debian/rules.tmpl | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git
>>> a/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
>>> b/meta/recipes-kernel/linux-module/files/debian/rules.tmpl index
>>> d3bd7dc30f21..0d16186b5ff3 100755 ---
>>> a/meta/recipes-kernel/linux-module/files/debian/rules.tmpl +++
>>> b/meta/recipes-kernel/linux-module/files/debian/rules.tmpl @@
>>> -48,13 +48,13 @@ KDIR := $(shell dpkg -L $(KERNEL_DEP) | grep
>>> "/lib/modules/.*/build") endif override_dh_auto_clean:
>>> -	$(MAKE) -C $(KDIR) M=$(PWD) clean
>>> +	$(MAKE) KDIR=$(KDIR) clean
>>>  
>>>  override_dh_auto_build:
>>> -	$(MAKE) -C $(KDIR) M=$(PWD) modules
>>> +	$(MAKE) KDIR=$(KDIR) modules
>>>  
>>>  override_dh_auto_install:
>>> -	$(MAKE) -C $(KDIR) M=$(PWD)
>>> INSTALL_MOD_PATH=$(PWD)/debian/${PN} modules_install
>>> +	$(MAKE) KDIR=$(KDIR) INSTALL_MOD_PATH=$(PWD)/debian/${PN}
>>> modules_install 
>>>  %:
>>>  	CFLAGS= LDFLAGS= dh $@ --parallel  
>>
>> This breaks existing users with makefile that are unusable or do not
>> have own rules. I would refrain from that breakage.
>>
>> If you have a more complex case than what we support, you can simply
>> provide your own rules and can then even account for the other
>> specialties that usually come with such module makefiles.
>>
>> Same arguing applies to patch 3.
> 
> I am not sure i would agree that the example Makefile is indeed valid,
> since it does not allow self sustained building with a simple "make
> modules_install". But yes for modules with such Makefiles this would be
> a breaking change.
> 
> Will go and carry the special rules entries downstream, while i still
> think Isar should take that and rather make those weirdos carry their
> own rules ... accepting a breakage.
> 
> But i have no clue how "weird" vs "common" it would be to have such a
> super-minimal Makefile. And my case is probably also not "common"
> enough to ague for too long.

Isar implements to official way of writing external module makefiles and
invoking them, see
https://www.kernel.org/doc/html/latest/kbuild/modules.html#command-syntax.
Anything beyond in its generic code would be guesswork about how
makefiles were written, what they expect as input etc.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

[PATCH v2 3/3] linux-module: call the install target of external modules

[Henning Schild]

An "install" target would allow out-of-tree modules to deploy additional
files. Like depmod.d ordering files, udev rules or modprobe.d files
which they might require.

The default "install" target just does nothing or maybe
"modules_install" again, so there should be no harm. While the kernel
documentation for external modules does not mention the "install" target
some might have it to install things on top of "modules_install".

Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 meta/recipes-kernel/linux-module/files/debian/rules.tmpl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meta/recipes-kernel/linux-module/files/debian/rules.tmpl b/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
index 0d16186b5ff3..cb62dc99f1cf 100755
--- a/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
+++ b/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
@@ -55,6 +55,7 @@ override_dh_auto_build:
 
 override_dh_auto_install:
 	$(MAKE) KDIR=$(KDIR) INSTALL_MOD_PATH=$(PWD)/debian/${PN} modules_install
+	$(MAKE) KDIR=$(KDIR) INSTALL_MOD_PATH=$(PWD)/debian/${PN} install
 
 %:
 	CFLAGS= LDFLAGS= dh $@ --parallel
-- 
2.35.1

Re: [PATCH v2 1/3] example-module: improve Makefile to be more realistic

[Jan Kiszka]

On 27.08.22 10:59, Henning Schild wrote:
> A real out-of-tree module would be able to "make" on its own, finding
> its own KDIR and not rely on some external entity to provide that. But
> of cause allow KDIR to be provided from outside.
> 
> Fixes: 8d9e4e3c0857 ("Add exemplary kernel module")
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
>  .../example-module/files/src/Makefile              | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/meta-isar/recipes-kernel/example-module/files/src/Makefile b/meta-isar/recipes-kernel/example-module/files/src/Makefile
> index 2561cfd410e5..df3241652cf1 100644
> --- a/meta-isar/recipes-kernel/example-module/files/src/Makefile
> +++ b/meta-isar/recipes-kernel/example-module/files/src/Makefile
> @@ -1,8 +1,20 @@
>  # Example module
>  #
>  # This software is a part of ISAR.
> -# Copyright (c) Siemens AG, 2018
> +# Copyright (c) Siemens AG, 2018-2022
>  #
>  # SPDX-License-Identifier: GPL-2.0
>  
>  obj-m := example-module.o
> +
> +INSTALL_MOD_PATH ?= $(DESTDIR)
> +export INSTALL_MOD_PATH
> +
> +KDIR ?= /lib/modules/$(shell uname -r)/build
> +
> +modules modules_install clean:
> +	$(MAKE) -C $(KDIR) M=$(PWD) $@
> +
> +install: modules_install
> +
> +.PHONY: modules modules_install install clean

This is convenient of you call things on the command line but unneeded
for the minimal case that we also support in Isar. Therefore, it was not
added to this example.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux