Patchwork powerpc: build modules outside the kernel tree fails, if it was built using O=

login
register
mail settings
Submitter Yuri Frolov
Date Sept. 24, 2009, 11:28 a.m.
Message ID <4ABB57CB.3000409@ru.mvista.com>
Download mbox | patch
Permalink /patch/34218/
State Superseded
Headers show

Comments

Yuri Frolov - Sept. 24, 2009, 11:28 a.m.
Hello,

here is a corresponding bug: http://bugzilla.kernel.org/show_bug.cgi?id=11143
This patch should correctly export crtsavres.o in order to make O= option working.
Please, consider to apply.


Fix linking modules against crtsavres.o

Previously we got
  CC      drivers/char/hw_random/rng-core.mod.o
  LD [M]  drivers/char/hw_random/rng-core.ko
/there/src/buildroot.git.ppc/build_powerpc_nofpu/staging_dir/usr/bin/powerpc-linux-uclibc-ld: arch/powerpc/lib/crtsavres.o: No such file: No such file or directory

	* Makefile (LDFLAGS_MODULE_PREREQ): New variable to hold prerequisite
          files for modules.
	* arch/powerpc/Makefile: add crtsavres.o to LDFLAGS_MODULE_PREREQ.
	* scripts/Makefile.modpost (cmd_as_o_S): Copy from Makefile.build.
	  (cmd_ld_ko_o): Also link LDFLAGS_MODULE_PREREQ.
	  Provide rule to build objects from assembler.

Signed-off-by:  Bernhard Reutner-Fischer  <rep.dot.nop@gmail.com>
Signed-off by:  Yuri Frolov <yfrolov@ru.mvista.com>

Makefile                 |    2 ++
arch/powerpc/Makefile    |    2 +-
scripts/Makefile.modpost |   12 ++++++++++--
3 files changed, 13 insertions(+), 3 deletions(-)
Benjamin Herrenschmidt - Sept. 25, 2009, 1:12 a.m.
On Thu, 2009-09-24 at 15:28 +0400, Yuri Frolov wrote:
> Hello,
> 
> here is a corresponding bug: http://bugzilla.kernel.org/show_bug.cgi?id=11143
> This patch should correctly export crtsavres.o in order to make O= option working.
> Please, consider to apply.
> 
> 
> Fix linking modules against crtsavres.o

Hi !

This is the same patch you already posted as "


[PATCH] Fix linking modules against
crtsavres.o
"

Or it's an update ?

I've asked Sam to review it already since it affects the main kernel
makefiles, waiting for his answer.

Cheers,
Ben.

> Previously we got
>   CC      drivers/char/hw_random/rng-core.mod.o
>   LD [M]  drivers/char/hw_random/rng-core.ko
> /there/src/buildroot.git.ppc/build_powerpc_nofpu/staging_dir/usr/bin/powerpc-linux-uclibc-ld: arch/powerpc/lib/crtsavres.o: No such file: No such file or directory
> 
> 	* Makefile (LDFLAGS_MODULE_PREREQ): New variable to hold prerequisite
>           files for modules.
> 	* arch/powerpc/Makefile: add crtsavres.o to LDFLAGS_MODULE_PREREQ.
> 	* scripts/Makefile.modpost (cmd_as_o_S): Copy from Makefile.build.
> 	  (cmd_ld_ko_o): Also link LDFLAGS_MODULE_PREREQ.
> 	  Provide rule to build objects from assembler.
> 
> Signed-off-by:  Bernhard Reutner-Fischer  <rep.dot.nop@gmail.com>
> Signed-off by:  Yuri Frolov <yfrolov@ru.mvista.com>
> 
> Makefile                 |    2 ++
> arch/powerpc/Makefile    |    2 +-
> scripts/Makefile.modpost |   12 ++++++++++--
> 3 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff -urpN -X linux-2.6/Documentation/dontdiff linux-2.6/arch/powerpc/Makefile linux-2.6-powerpc-crtsavres/arch/powerpc/Makefile
> --- linux-2.6/arch/powerpc/Makefile	2009-09-17 20:04:31.000000000 +0400
> +++ linux-2.6-powerpc-crtsavres/arch/powerpc/Makefile	2009-09-23 22:08:03.000000000 +0400
> @@ -93,7 +93,7 @@ else
>  	KBUILD_CFLAGS += $(call cc-option,-mtune=power4)
>  endif
>  else
> -LDFLAGS_MODULE	+= arch/powerpc/lib/crtsavres.o
> +LDFLAGS_MODULE_PREREQ += arch/powerpc/lib/crtsavres.o
>  endif
>  
>  ifeq ($(CONFIG_TUNE_CELL),y)
> diff -urpN -X linux-2.6/Documentation/dontdiff linux-2.6/Makefile linux-2.6-powerpc-crtsavres/Makefile
> --- linux-2.6/Makefile	2009-09-17 20:04:30.000000000 +0400
> +++ linux-2.6-powerpc-crtsavres/Makefile	2009-09-23 21:41:16.000000000 +0400
> @@ -326,6 +326,7 @@ MODFLAGS	= -DMODULE
>  CFLAGS_MODULE   = $(MODFLAGS)
>  AFLAGS_MODULE   = $(MODFLAGS)
>  LDFLAGS_MODULE  = -T $(srctree)/scripts/module-common.lds
> +LDFLAGS_MODULE_PREREQ  =
>  CFLAGS_KERNEL	=
>  AFLAGS_KERNEL	=
>  CFLAGS_GCOV	= -fprofile-arcs -ftest-coverage
> @@ -355,6 +356,7 @@ export VERSION PATCHLEVEL SUBLEVEL KERNE
>  export ARCH SRCARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC
>  export CPP AR NM STRIP OBJCOPY OBJDUMP MAKE AWK GENKSYMS PERL UTS_MACHINE
>  export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS
> +export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE LDFLAGS_MODULE_PREREQ CHECK CHECKFLAGS
>  
>  export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS LDFLAGS
>  export KBUILD_CFLAGS CFLAGS_KERNEL CFLAGS_MODULE CFLAGS_GCOV
> diff -urpN -X linux-2.6/Documentation/dontdiff linux-2.6/scripts/Makefile.modpost linux-2.6-powerpc-crtsavres/scripts/Makefile.modpost
> --- linux-2.6/scripts/Makefile.modpost	2009-09-17 20:04:42.000000000 +0400
> +++ linux-2.6-powerpc-crtsavres/scripts/Makefile.modpost	2009-09-23 22:15:00.000000000 +0400
> @@ -122,14 +122,22 @@ quiet_cmd_cc_o_c = CC      $@
>        cmd_cc_o_c = $(CC) $(c_flags) $(CFLAGS_MODULE)	\
>  		   -c -o $@ $<
>  
> -$(modules:.ko=.mod.o): %.mod.o: %.mod.c FORCE
> +quiet_cmd_as_o_S = AS $(quiet_modtag)  $@
> +cmd_as_o_S       = $(CC) $(a_flags) $(AFLAGS_MODULE) -c -o $@ $<
> +
> +$(LDFLAGS_MODULE_PREREQ): %.o: %.S FORCE
> +	$(Q)mkdir -p $(dir $@)
> +	$(call if_changed_dep,as_o_S)
> +
> +$(modules:.ko=.mod.o): %.mod.o: %.mod.c $(LDFLAGS_MODULE_PREREQ) FORCE
>  	$(call if_changed_dep,cc_o_c)
>  
>  targets += $(modules:.ko=.mod.o)
>  
>  # Step 6), final link of the modules
>  quiet_cmd_ld_ko_o = LD [M]  $@
> -      cmd_ld_ko_o = $(LD) -r $(LDFLAGS) $(LDFLAGS_MODULE) -o $@		\
> +      cmd_ld_ko_o = $(LD) -r $(LDFLAGS) $(LDFLAGS_MODULE_PREREQ)	\
> +			  $(LDFLAGS_MODULE) -o $@			\
>  			  $(filter-out FORCE,$^)
>  
>  $(modules): %.ko :%.o %.mod.o FORCE
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
Sam Ravnborg - Sept. 25, 2009, 4:39 a.m.
On Fri, Sep 25, 2009 at 11:12:21AM +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2009-09-24 at 15:28 +0400, Yuri Frolov wrote:
> > Hello,
> > 
> > here is a corresponding bug: http://bugzilla.kernel.org/show_bug.cgi?id=11143
> > This patch should correctly export crtsavres.o in order to make O= option working.
> > Please, consider to apply.
> > 
> > 
> > Fix linking modules against crtsavres.o
> 
> Hi !
> 
> This is the same patch you already posted as "
> 
> 
> [PATCH] Fix linking modules against
> crtsavres.o
> "
> 
> Or it's an update ?
> 
> I've asked Sam to review it already since it affects the main kernel
> makefiles, waiting for his answer.
Saw the duplicates. Will get back to it tonight (morning here now).

	Sam
Yuri Frolov - Sept. 25, 2009, 9:39 a.m.
On 09/25/2009 08:39 AM, Sam Ravnborg wrote:
> On Fri, Sep 25, 2009 at 11:12:21AM +1000, Benjamin Herrenschmidt wrote:
>> On Thu, 2009-09-24 at 15:28 +0400, Yuri Frolov wrote:
>>> Hello,
>>>
>>> here is a corresponding bug: http://bugzilla.kernel.org/show_bug.cgi?id=11143
>>> This patch should correctly export crtsavres.o in order to make O= option working.
>>> Please, consider to apply.
>>>
>>>
>>> Fix linking modules against crtsavres.o
>> Hi !
>>
>> This is the same patch you already posted as "
>>
>>
>> [PATCH] Fix linking modules against
>> crtsavres.o
>> "
>>
>> Or it's an update ?

It's the same, sorry for not mentioning it. The previous letter contains attachment crap, so I sent the letter with patch in-lined.

>>
>> I've asked Sam to review it already since it affects the main kernel
>> makefiles, waiting for his answer.
> Saw the duplicates. Will get back to it tonight (morning here now).
> 
> 	Sam
Ok, thank you.
Sam Ravnborg - Sept. 25, 2009, 7:45 p.m.
On Thu, Sep 24, 2009 at 03:28:11PM +0400, Yuri Frolov wrote:
> Hello,
> 
> here is a corresponding bug: http://bugzilla.kernel.org/show_bug.cgi?id=11143
> This patch should correctly export crtsavres.o in order to make O= option working.
> Please, consider to apply.

Hi Yuri.

I like the way you do the extra link in Makefile.modpost.
But you need to redo some parts as per comments below.

> 
> 
> Fix linking modules against crtsavres.o

Please elaborate more on what this commit does.

> 
> Previously we got
>   CC      drivers/char/hw_random/rng-core.mod.o
>   LD [M]  drivers/char/hw_random/rng-core.ko
> /there/src/buildroot.git.ppc/build_powerpc_nofpu/staging_dir/usr/bin/powerpc-linux-uclibc-ld: arch/powerpc/lib/crtsavres.o: No such file: No such file or directory

Always good to include error messages.

> 	* Makefile (LDFLAGS_MODULE_PREREQ): New variable to hold prerequisite
>           files for modules.
> 	* arch/powerpc/Makefile: add crtsavres.o to LDFLAGS_MODULE_PREREQ.
> 	* scripts/Makefile.modpost (cmd_as_o_S): Copy from Makefile.build.
> 	  (cmd_ld_ko_o): Also link LDFLAGS_MODULE_PREREQ.
> 	  Provide rule to build objects from assembler.
But this GNUism can go - we do not use it in the kernel.
 
> 
> Signed-off-by:  Bernhard Reutner-Fischer  <rep.dot.nop@gmail.com>
> Signed-off by:  Yuri Frolov <yfrolov@ru.mvista.com>
> 
> Makefile                 |    2 ++
> arch/powerpc/Makefile    |    2 +-
> scripts/Makefile.modpost |   12 ++++++++++--
> 3 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff -urpN -X linux-2.6/Documentation/dontdiff linux-2.6/arch/powerpc/Makefile linux-2.6-powerpc-crtsavres/arch/powerpc/Makefile
> --- linux-2.6/arch/powerpc/Makefile	2009-09-17 20:04:31.000000000 +0400
> +++ linux-2.6-powerpc-crtsavres/arch/powerpc/Makefile	2009-09-23 22:08:03.000000000 +0400
> @@ -93,7 +93,7 @@ else
>  	KBUILD_CFLAGS += $(call cc-option,-mtune=power4)
>  endif
>  else
> -LDFLAGS_MODULE	+= arch/powerpc/lib/crtsavres.o
> +LDFLAGS_MODULE_PREREQ += arch/powerpc/lib/crtsavres.o
>  endif

The naming sucks.
How about:

KBUILD_MODULE_LINK_SOURCE

This would tell the reader that this is source to be linked on a module.

And this is an arch specific thing so no need to preset it in top-level
Makefile.
But it is mandatory to include a description in Documentation/kbuild/kbuild.txt


> --- linux-2.6/scripts/Makefile.modpost	2009-09-17 20:04:42.000000000 +0400
> +++ linux-2.6-powerpc-crtsavres/scripts/Makefile.modpost	2009-09-23 22:15:00.000000000 +0400
> @@ -122,14 +122,22 @@ quiet_cmd_cc_o_c = CC      $@
>        cmd_cc_o_c = $(CC) $(c_flags) $(CFLAGS_MODULE)	\
>  		   -c -o $@ $<
>  
> -$(modules:.ko=.mod.o): %.mod.o: %.mod.c FORCE
> +quiet_cmd_as_o_S = AS $(quiet_modtag)  $@
> +cmd_as_o_S       = $(CC) $(a_flags) $(AFLAGS_MODULE) -c -o $@ $<

Align this so cmd_as_o_S is under each other - as we do for cmd_cc_o_c


> +
> +$(LDFLAGS_MODULE_PREREQ): %.o: %.S FORCE
> +	$(Q)mkdir -p $(dir $@)
> +	$(call if_changed_dep,as_o_S)
Good catch with the mkdir - needed for O= builds.
I think we shall wrap this in
ifdef KBUILD_MODULE_LINK_SOURCE
...
endif

So we do not have an empty rule when it is not defined.

Please fix up these things and resubmit.

Thanks,
	Sam
Benjamin Herrenschmidt - Sept. 25, 2009, 9:57 p.m.
On Fri, 2009-09-25 at 21:45 +0200, Sam Ravnborg wrote:
> On Thu, Sep 24, 2009 at 03:28:11PM +0400, Yuri Frolov wrote:
> > Hello,
> > 
> > here is a corresponding bug: http://bugzilla.kernel.org/show_bug.cgi?id=11143
> > This patch should correctly export crtsavres.o in order to make O= option working.
> > Please, consider to apply.
> 
> Hi Yuri.
> 
> I like the way you do the extra link in Makefile.modpost.
> But you need to redo some parts as per comments below.

> > 
> > Fix linking modules against crtsavres.o
> 
> Please elaborate more on what this commit does.

It's a support file that needs to be linked against every module
(aka libgcc like) and thus needs to be built before any module.

Cheers,
Ben.
Sam Ravnborg - Sept. 25, 2009, 10:01 p.m.
On Sat, Sep 26, 2009 at 07:57:32AM +1000, Benjamin Herrenschmidt wrote:
> On Fri, 2009-09-25 at 21:45 +0200, Sam Ravnborg wrote:
> > On Thu, Sep 24, 2009 at 03:28:11PM +0400, Yuri Frolov wrote:
> > > Hello,
> > > 
> > > here is a corresponding bug: http://bugzilla.kernel.org/show_bug.cgi?id=11143
> > > This patch should correctly export crtsavres.o in order to make O= option working.
> > > Please, consider to apply.
> > 
> > Hi Yuri.
> > 
> > I like the way you do the extra link in Makefile.modpost.
> > But you need to redo some parts as per comments below.
> 
> > > 
> > > Fix linking modules against crtsavres.o
> > 
> > Please elaborate more on what this commit does.
> 
> It's a support file that needs to be linked against every module
> (aka libgcc like) and thus needs to be built before any module.

Yes.

My point was that the next version of the changelog should
include this information - as well as kbuild.txt.

	Sam

Patch

diff -urpN -X linux-2.6/Documentation/dontdiff linux-2.6/arch/powerpc/Makefile linux-2.6-powerpc-crtsavres/arch/powerpc/Makefile
--- linux-2.6/arch/powerpc/Makefile	2009-09-17 20:04:31.000000000 +0400
+++ linux-2.6-powerpc-crtsavres/arch/powerpc/Makefile	2009-09-23 22:08:03.000000000 +0400
@@ -93,7 +93,7 @@  else
 	KBUILD_CFLAGS += $(call cc-option,-mtune=power4)
 endif
 else
-LDFLAGS_MODULE	+= arch/powerpc/lib/crtsavres.o
+LDFLAGS_MODULE_PREREQ += arch/powerpc/lib/crtsavres.o
 endif
 
 ifeq ($(CONFIG_TUNE_CELL),y)
diff -urpN -X linux-2.6/Documentation/dontdiff linux-2.6/Makefile linux-2.6-powerpc-crtsavres/Makefile
--- linux-2.6/Makefile	2009-09-17 20:04:30.000000000 +0400
+++ linux-2.6-powerpc-crtsavres/Makefile	2009-09-23 21:41:16.000000000 +0400
@@ -326,6 +326,7 @@  MODFLAGS	= -DMODULE
 CFLAGS_MODULE   = $(MODFLAGS)
 AFLAGS_MODULE   = $(MODFLAGS)
 LDFLAGS_MODULE  = -T $(srctree)/scripts/module-common.lds
+LDFLAGS_MODULE_PREREQ  =
 CFLAGS_KERNEL	=
 AFLAGS_KERNEL	=
 CFLAGS_GCOV	= -fprofile-arcs -ftest-coverage
@@ -355,6 +356,7 @@  export VERSION PATCHLEVEL SUBLEVEL KERNE
 export ARCH SRCARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC
 export CPP AR NM STRIP OBJCOPY OBJDUMP MAKE AWK GENKSYMS PERL UTS_MACHINE
 export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS
+export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE LDFLAGS_MODULE_PREREQ CHECK CHECKFLAGS
 
 export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS LDFLAGS
 export KBUILD_CFLAGS CFLAGS_KERNEL CFLAGS_MODULE CFLAGS_GCOV
diff -urpN -X linux-2.6/Documentation/dontdiff linux-2.6/scripts/Makefile.modpost linux-2.6-powerpc-crtsavres/scripts/Makefile.modpost
--- linux-2.6/scripts/Makefile.modpost	2009-09-17 20:04:42.000000000 +0400
+++ linux-2.6-powerpc-crtsavres/scripts/Makefile.modpost	2009-09-23 22:15:00.000000000 +0400
@@ -122,14 +122,22 @@  quiet_cmd_cc_o_c = CC      $@
       cmd_cc_o_c = $(CC) $(c_flags) $(CFLAGS_MODULE)	\
 		   -c -o $@ $<
 
-$(modules:.ko=.mod.o): %.mod.o: %.mod.c FORCE
+quiet_cmd_as_o_S = AS $(quiet_modtag)  $@
+cmd_as_o_S       = $(CC) $(a_flags) $(AFLAGS_MODULE) -c -o $@ $<
+
+$(LDFLAGS_MODULE_PREREQ): %.o: %.S FORCE
+	$(Q)mkdir -p $(dir $@)
+	$(call if_changed_dep,as_o_S)
+
+$(modules:.ko=.mod.o): %.mod.o: %.mod.c $(LDFLAGS_MODULE_PREREQ) FORCE
 	$(call if_changed_dep,cc_o_c)
 
 targets += $(modules:.ko=.mod.o)
 
 # Step 6), final link of the modules
 quiet_cmd_ld_ko_o = LD [M]  $@
-      cmd_ld_ko_o = $(LD) -r $(LDFLAGS) $(LDFLAGS_MODULE) -o $@		\
+      cmd_ld_ko_o = $(LD) -r $(LDFLAGS) $(LDFLAGS_MODULE_PREREQ)	\
+			  $(LDFLAGS_MODULE) -o $@			\
 			  $(filter-out FORCE,$^)
 
 $(modules): %.ko :%.o %.mod.o FORCE