2007-02-15 21:56:33

by Adrian Bunk

[permalink] [raw]
Subject: [2.6 patch] drivers/isdn/gigaset/: build asyncdata.o into the gigaset module

This patch fixes the following compile error:

<-- snip -->

...
LD drivers/isdn/gigaset/built-in.o
drivers/isdn/gigaset/ser_gigaset.o: In function `gigaset_m10x_send_skb':
(.text+0xe50): multiple definition of `gigaset_m10x_send_skb'
drivers/isdn/gigaset/usb_gigaset.o:(.text+0x0): first defined here
drivers/isdn/gigaset/ser_gigaset.o: In function `gigaset_m10x_input':
(.text+0x1121): multiple definition of `gigaset_m10x_input'
drivers/isdn/gigaset/usb_gigaset.o:(.text+0x2d1): first defined here
make[4]: *** [drivers/isdn/gigaset/built-in.o] Error 1

<-- snip -->

Signed-off-by: Adrian Bunk <[email protected]>

---

drivers/isdn/gigaset/Makefile | 6 +++---
drivers/isdn/gigaset/asyncdata.c | 2 ++
2 files changed, 5 insertions(+), 3 deletions(-)

--- linux-2.6.20-mm1/drivers/isdn/gigaset/Makefile.old 2007-02-15 19:35:30.000000000 +0100
+++ linux-2.6.20-mm1/drivers/isdn/gigaset/Makefile 2007-02-15 19:35:46.000000000 +0100
@@ -1,7 +1,7 @@
-gigaset-y := common.o interface.o proc.o ev-layer.o i4l.o
-usb_gigaset-y := usb-gigaset.o asyncdata.o
+gigaset-y := common.o interface.o proc.o ev-layer.o i4l.o asyncdata.o
+usb_gigaset-y := usb-gigaset.o
bas_gigaset-y := bas-gigaset.o isocdata.o
-ser_gigaset-y := ser-gigaset.o asyncdata.o
+ser_gigaset-y := ser-gigaset.o

obj-$(CONFIG_GIGASET_M105) += usb_gigaset.o gigaset.o
obj-$(CONFIG_GIGASET_BASE) += bas_gigaset.o gigaset.o
--- linux-2.6.20-mm1/drivers/isdn/gigaset/asyncdata.c.old 2007-02-15 19:36:14.000000000 +0100
+++ linux-2.6.20-mm1/drivers/isdn/gigaset/asyncdata.c 2007-02-15 19:37:08.000000000 +0100
@@ -444,6 +444,7 @@
atomic_set(&inbuf->head, head);
}
}
+EXPORT_SYMBOL_GPL(gigaset_m10x_input);


/* == data output ========================================================== */
@@ -591,3 +592,4 @@

return len; /* ok so far */
}
+EXPORT_SYMBOL_GPL(gigaset_m10x_send_skb);


2007-02-16 00:16:16

by Tilman Schmidt

[permalink] [raw]
Subject: Re: [2.6 patch] drivers/isdn/gigaset/: build asyncdata.o into the gigaset module

Am 15.02.2007 22:56 schrieb Adrian Bunk:
> This patch fixes the following compile error:
>
> <-- snip -->
>
> ...
> LD drivers/isdn/gigaset/built-in.o
> drivers/isdn/gigaset/ser_gigaset.o: In function `gigaset_m10x_send_skb':
> (.text+0xe50): multiple definition of `gigaset_m10x_send_skb'
> drivers/isdn/gigaset/usb_gigaset.o:(.text+0x0): first defined here
> drivers/isdn/gigaset/ser_gigaset.o: In function `gigaset_m10x_input':
> (.text+0x1121): multiple definition of `gigaset_m10x_input'
> drivers/isdn/gigaset/usb_gigaset.o:(.text+0x2d1): first defined here
> make[4]: *** [drivers/isdn/gigaset/built-in.o] Error 1

How did you manage to produce that error? I have never encountered it.

Thanks,
Tilman

--
Tilman Schmidt E-Mail: [email protected]
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Unge?ffnet mindestens haltbar bis: (siehe R?ckseite)


Attachments:
signature.asc (253.00 B)
OpenPGP digital signature

2007-02-16 08:33:35

by Adrian Bunk

[permalink] [raw]
Subject: Re: [2.6 patch] drivers/isdn/gigaset/: build asyncdata.o into the gigaset module

On Fri, Feb 16, 2007 at 01:17:19AM +0100, Tilman Schmidt wrote:
> Am 15.02.2007 22:56 schrieb Adrian Bunk:
> > This patch fixes the following compile error:
> >
> > <-- snip -->
> >
> > ...
> > LD drivers/isdn/gigaset/built-in.o
> > drivers/isdn/gigaset/ser_gigaset.o: In function `gigaset_m10x_send_skb':
> > (.text+0xe50): multiple definition of `gigaset_m10x_send_skb'
> > drivers/isdn/gigaset/usb_gigaset.o:(.text+0x0): first defined here
> > drivers/isdn/gigaset/ser_gigaset.o: In function `gigaset_m10x_input':
> > (.text+0x1121): multiple definition of `gigaset_m10x_input'
> > drivers/isdn/gigaset/usb_gigaset.o:(.text+0x2d1): first defined here
> > make[4]: *** [drivers/isdn/gigaset/built-in.o] Error 1
>
> How did you manage to produce that error? I have never encountered it.

CONFIG_ISDN_DRV_GIGASET=y
CONFIG_GIGASET_BASE=y
CONFIG_GIGASET_M105=y
CONFIG_GIGASET_M101=y
CONFIG_GIGASET_DEBUG=y
CONFIG_GIGASET_UNDOCREQ=y

It might depend on the binutils version.

> Thanks,
> Tilman

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2007-02-17 00:03:40

by Tilman Schmidt

[permalink] [raw]
Subject: Kbuild problem (was: [2.6 patch] drivers/isdn/gigaset/: build asyncdata.o into the gigaset module)

Am 16.02.2007 09:33 schrieb Adrian Bunk:
>>> ...
>>> LD drivers/isdn/gigaset/built-in.o
>>> drivers/isdn/gigaset/ser_gigaset.o: In function `gigaset_m10x_send_skb':
>>> (.text+0xe50): multiple definition of `gigaset_m10x_send_skb'
>>> drivers/isdn/gigaset/usb_gigaset.o:(.text+0x0): first defined here
>>> drivers/isdn/gigaset/ser_gigaset.o: In function `gigaset_m10x_input':
>>> (.text+0x1121): multiple definition of `gigaset_m10x_input'
>>> drivers/isdn/gigaset/usb_gigaset.o:(.text+0x2d1): first defined here
>>> make[4]: *** [drivers/isdn/gigaset/built-in.o] Error 1

> CONFIG_ISDN_DRV_GIGASET=y
> CONFIG_GIGASET_BASE=y
> CONFIG_GIGASET_M105=y
> CONFIG_GIGASET_M101=y

Ah, thanks, I can see what's happening now. When both the M101 and M105
drivers are built into the kernel, asyncdata.o gets linked in twice, via

usb_gigaset-y := usb-gigaset.o asyncdata.o
obj-$(CONFIG_GIGASET_M105) += usb_gigaset.o gigaset.o

and

ser_gigaset-y := ser-gigaset.o asyncdata.o
obj-$(CONFIG_GIGASET_M101) += ser_gigaset.o gigaset.o

The assertion in Documentation/kbuild/makefiles.txt:

The order of files in $(obj-y) is significant. Duplicates in
the lists are allowed: the first instance will be linked into
built-in.o and succeeding instances will be ignored.

doesn't work out in this case because asyncdata.o isn't added directly
to $(obj-y), but as part of usb_gigaset.o and ser_gigaset.o.

I'm not quite happy with your solution, though. I'd prefer a
Makefile which builds modular usb_gigaset.ko and/or ser_gigaset.ko
like the present one (including asyncdata.o), but when linking
usb-gigaset.o and ser-gigaset.o into the kernel includes asyncdata.o
only once. Trouble is, I don't know how to express that in Kbuild.

Any ideas?

Thanks,
Tilman

--
Tilman Schmidt E-Mail: [email protected]
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Unge?ffnet mindestens haltbar bis: (siehe R?ckseite)


Attachments:
signature.asc (253.00 B)
OpenPGP digital signature

2007-02-17 10:51:59

by Adrian Bunk

[permalink] [raw]
Subject: Re: Kbuild problem (was: [2.6 patch] drivers/isdn/gigaset/: build asyncdata.o into the gigaset module)

On Sat, Feb 17, 2007 at 01:04:33AM +0100, Tilman Schmidt wrote:
> Am 16.02.2007 09:33 schrieb Adrian Bunk:
> >>> ...
> >>> LD drivers/isdn/gigaset/built-in.o
> >>> drivers/isdn/gigaset/ser_gigaset.o: In function `gigaset_m10x_send_skb':
> >>> (.text+0xe50): multiple definition of `gigaset_m10x_send_skb'
> >>> drivers/isdn/gigaset/usb_gigaset.o:(.text+0x0): first defined here
> >>> drivers/isdn/gigaset/ser_gigaset.o: In function `gigaset_m10x_input':
> >>> (.text+0x1121): multiple definition of `gigaset_m10x_input'
> >>> drivers/isdn/gigaset/usb_gigaset.o:(.text+0x2d1): first defined here
> >>> make[4]: *** [drivers/isdn/gigaset/built-in.o] Error 1
>
> > CONFIG_ISDN_DRV_GIGASET=y
> > CONFIG_GIGASET_BASE=y
> > CONFIG_GIGASET_M105=y
> > CONFIG_GIGASET_M101=y
>
> Ah, thanks, I can see what's happening now. When both the M101 and M105
> drivers are built into the kernel, asyncdata.o gets linked in twice, via
>
> usb_gigaset-y := usb-gigaset.o asyncdata.o
> obj-$(CONFIG_GIGASET_M105) += usb_gigaset.o gigaset.o
>
> and
>
> ser_gigaset-y := ser-gigaset.o asyncdata.o
> obj-$(CONFIG_GIGASET_M101) += ser_gigaset.o gigaset.o
>
> The assertion in Documentation/kbuild/makefiles.txt:
>
> The order of files in $(obj-y) is significant. Duplicates in
> the lists are allowed: the first instance will be linked into
> built-in.o and succeeding instances will be ignored.
>
> doesn't work out in this case because asyncdata.o isn't added directly
> to $(obj-y), but as part of usb_gigaset.o and ser_gigaset.o.
>
> I'm not quite happy with your solution, though. I'd prefer a
> Makefile which builds modular usb_gigaset.ko and/or ser_gigaset.ko
> like the present one (including asyncdata.o), but when linking
> usb-gigaset.o and ser-gigaset.o into the kernel includes asyncdata.o
> only once. Trouble is, I don't know how to express that in Kbuild.
>
> Any ideas?

CONFIG_GIGASET_M105=y, CONFIG_GIGASET_M101=m and similar problems might
make this quite tricky.

Shared functionality simply doesn't belong into any of the affected
modules, but to a different place where all users can access it.

> Thanks,
> Tilman

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2007-02-17 18:36:04

by Tilman Schmidt

[permalink] [raw]
Subject: Re: Kbuild problem

Am 17.02.2007 11:52 schrieb Adrian Bunk:
> On Sat, Feb 17, 2007 at 01:04:33AM +0100, Tilman Schmidt wrote:
>> [...] I'd prefer a
>> Makefile which builds modular usb_gigaset.ko and/or ser_gigaset.ko
>> like the present one (including asyncdata.o), but when linking
>> usb-gigaset.o and ser-gigaset.o into the kernel includes asyncdata.o
>> only once. Trouble is, I don't know how to express that in Kbuild.
>>
>> Any ideas?
>
> CONFIG_GIGASET_M105=y, CONFIG_GIGASET_M101=m and similar problems might
> make this quite tricky.

I am no judge of that. All I know about the possibilities of Kbuild
is what the files in Documentation/kbuild tell me. From browsing
through other kernel Makefiles it would appear that there are more
possibilities though, such as if/then/else constructs.

> Shared functionality simply doesn't belong into any of the affected
> modules, but to a different place where all users can access it.

Alright, then so be it. But that raises another question:
asyncdata.o is only needed for M105 and M101, not for the base
driver. How do I express in Kbuild that asyncdata.o is to be added
to gigaset-y only if CONFIG_GIGASET_M105 and CONFIG_GIGASET_M101
are not both 'n'?

Thanks,
Tilman

--
Tilman Schmidt E-Mail: [email protected]
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Unge?ffnet mindestens haltbar bis: (siehe R?ckseite)


Attachments:
signature.asc (253.00 B)
OpenPGP digital signature

2007-02-18 05:22:15

by Kai Germaschewski

[permalink] [raw]
Subject: Re: Kbuild problem

On Sat, 17 Feb 2007, Tilman Schmidt wrote:

> Alright, then so be it. But that raises another question:
> asyncdata.o is only needed for M105 and M101, not for the base
> driver. How do I express in Kbuild that asyncdata.o is to be added
> to gigaset-y only if CONFIG_GIGASET_M105 and CONFIG_GIGASET_M101
> are not both 'n'?

The way this is typically done is via Kconfig. Add a config option
ASYNCDATA (actually something more descriptive/specific would be better),
add a "select ASYNCDATA" to the config options for m101 and m105, and then
you can use CONFIG_ASYNCDATA to decide whether to add asyncdata.o in the
Makefile.

--Kai


2007-02-18 10:40:23

by Adrian Bunk

[permalink] [raw]
Subject: Re: Kbuild problem

On Sun, Feb 18, 2007 at 12:22:10AM -0500, Kai Germaschewski wrote:
> On Sat, 17 Feb 2007, Tilman Schmidt wrote:
>
> > Alright, then so be it. But that raises another question:
> > asyncdata.o is only needed for M105 and M101, not for the base
> > driver. How do I express in Kbuild that asyncdata.o is to be added
> > to gigaset-y only if CONFIG_GIGASET_M105 and CONFIG_GIGASET_M101
> > are not both 'n'?
>
> The way this is typically done is via Kconfig. Add a config option
> ASYNCDATA (actually something more descriptive/specific would be better),
> add a "select ASYNCDATA" to the config options for m101 and m105, and then
> you can use CONFIG_ASYNCDATA to decide whether to add asyncdata.o in the
> Makefile.

One disadvantage of this approach is that in a kernel with
CONFIG_GIGASET_BASE=y, you can't later compile and load the usb_gigaset
or ser_gigaset modules without rebooting since they require a change to
the kernel image.

> --Kai

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2007-02-20 13:58:20

by Tilman Schmidt

[permalink] [raw]
Subject: Re: Kbuild problem

Adrian Bunk schrieb:
> On Sun, Feb 18, 2007 at 12:22:10AM -0500, Kai Germaschewski wrote:
>> On Sat, 17 Feb 2007, Tilman Schmidt wrote:
>>
>> > asyncdata.o is only needed for M105 and M101, not for the base
>> > driver. How do I express in Kbuild that asyncdata.o is to be added
>> > to gigaset-y only if CONFIG_GIGASET_M105 and CONFIG_GIGASET_M101
>> > are not both 'n'?
>>
>> The way this is typically done is via Kconfig. Add a config option
>> ASYNCDATA (actually something more descriptive/specific would be better),
>> add a "select ASYNCDATA" to the config options for m101 and m105, and then
>> you can use CONFIG_ASYNCDATA to decide whether to add asyncdata.o in the
>> Makefile.
>
> One disadvantage of this approach is that in a kernel with
> CONFIG_GIGASET_BASE=y, you can't later compile and load the usb_gigaset
> or ser_gigaset modules without rebooting since they require a change to
> the kernel image.

You've got a point there. So linking asyncdata.o into the modules that
need it, as it is currently done, would perhaps be better after all?
The original problem (asyncdata.o linked in twice, causing duplicate
symbol definitions) could be fixed with this (admittedly somewhat
awkward) change to the Makefile (build tested):

--- linux-2.6.20-mm1-work/drivers/isdn/gigaset/Makefile 2007-02-20 13:39:44.000000000 +0100
+++ u/drivers/isdn/gigaset/Makefile 2007-02-20 13:38:58.000000000 +0100
@@ -1,7 +1,10 @@
gigaset-y := common.o interface.o proc.o ev-layer.o i4l.o
usb_gigaset-y := usb-gigaset.o asyncdata.o
bas_gigaset-y := bas-gigaset.o isocdata.o
-ser_gigaset-y := ser-gigaset.o asyncdata.o
+ser_gigaset-y := ser-gigaset.o
+ifneq ($(CONFIG_GIGASET_M101)$(CONFIG_GIGASET_M105),yy)
+ser_gigaset-y += asyncdata.o
+endif

obj-$(CONFIG_GIGASET_M105) += usb_gigaset.o gigaset.o
obj-$(CONFIG_GIGASET_BASE) += bas_gigaset.o gigaset.o

The alternative would be to always link asyncdata.o into the gigaset
module whether it's needed or not. "size asyncdata.o" says:
text data bss dec hex filename
4200 0 0 4200 1068 asyncdata.o
which appears tolerable.

Opinions?

--
Tilman Schmidt E-Mail: [email protected]
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Unge?ffnet mindestens haltbar bis: (siehe R?ckseite)


Attachments:
signature.asc (250.00 B)
OpenPGP digital signature

2007-02-20 14:38:11

by Joerg Dorchain

[permalink] [raw]
Subject: Re: Kbuild problem

On Tue, Feb 20, 2007 at 02:56:27PM +0100, Tilman Schmidt wrote:
> > One disadvantage of this approach is that in a kernel with
> > CONFIG_GIGASET_BASE=y, you can't later compile and load the usb_gigaset
> > or ser_gigaset modules without rebooting since they require a change to
> > the kernel image.
>
> You've got a point there. So linking asyncdata.o into the modules that
> need it, as it is currently done, would perhaps be better after all?

From the architectural point of view, what odds against making it a
module of its own? Dependancies are resolved by modprobe, so users should
be fine.
There are other library-like parts in the kernel where an object is
built statically when at least one needing driver is static, a module
when all nedding drivers are modules, or not at all but appears are a
config option when no in-kernel driver needs it.

>
> The alternative would be to always link asyncdata.o into the gigaset
> module whether it's needed or not. "size asyncdata.o" says:
> text data bss dec hex filename
> 4200 0 0 4200 1068 asyncdata.o
> which appears tolerable.

Ugly. But I've seen worse.

Bye,

Joerg


Attachments:
(No filename) (1.14 kB)
signature.asc (189.00 B)
Digital signature
Download all attachments

2007-02-20 14:59:45

by Adrian Bunk

[permalink] [raw]
Subject: Re: Kbuild problem

On Tue, Feb 20, 2007 at 02:56:27PM +0100, Tilman Schmidt wrote:
> Adrian Bunk schrieb:
> > On Sun, Feb 18, 2007 at 12:22:10AM -0500, Kai Germaschewski wrote:
> >> On Sat, 17 Feb 2007, Tilman Schmidt wrote:
> >>
> >> > asyncdata.o is only needed for M105 and M101, not for the base
> >> > driver. How do I express in Kbuild that asyncdata.o is to be added
> >> > to gigaset-y only if CONFIG_GIGASET_M105 and CONFIG_GIGASET_M101
> >> > are not both 'n'?
> >>
> >> The way this is typically done is via Kconfig. Add a config option
> >> ASYNCDATA (actually something more descriptive/specific would be better),
> >> add a "select ASYNCDATA" to the config options for m101 and m105, and then
> >> you can use CONFIG_ASYNCDATA to decide whether to add asyncdata.o in the
> >> Makefile.
> >
> > One disadvantage of this approach is that in a kernel with
> > CONFIG_GIGASET_BASE=y, you can't later compile and load the usb_gigaset
> > or ser_gigaset modules without rebooting since they require a change to
> > the kernel image.
>
> You've got a point there. So linking asyncdata.o into the modules that
> need it, as it is currently done, would perhaps be better after all?
> The original problem (asyncdata.o linked in twice, causing duplicate
> symbol definitions) could be fixed with this (admittedly somewhat
> awkward) change to the Makefile (build tested):
>
> --- linux-2.6.20-mm1-work/drivers/isdn/gigaset/Makefile 2007-02-20 13:39:44.000000000 +0100
> +++ u/drivers/isdn/gigaset/Makefile 2007-02-20 13:38:58.000000000 +0100
> @@ -1,7 +1,10 @@
> gigaset-y := common.o interface.o proc.o ev-layer.o i4l.o
> usb_gigaset-y := usb-gigaset.o asyncdata.o
> bas_gigaset-y := bas-gigaset.o isocdata.o
> -ser_gigaset-y := ser-gigaset.o asyncdata.o
> +ser_gigaset-y := ser-gigaset.o
> +ifneq ($(CONFIG_GIGASET_M101)$(CONFIG_GIGASET_M105),yy)
> +ser_gigaset-y += asyncdata.o
> +endif

What happens with CONFIG_GIGASET_M101=y, CONFIG_GIGASET_M105=m ?

> obj-$(CONFIG_GIGASET_M105) += usb_gigaset.o gigaset.o
> obj-$(CONFIG_GIGASET_BASE) += bas_gigaset.o gigaset.o
>
> The alternative would be to always link asyncdata.o into the gigaset
> module whether it's needed or not. "size asyncdata.o" says:
> text data bss dec hex filename
> 4200 0 0 4200 1068 asyncdata.o
> which appears tolerable.
>
> Opinions?

I'm usually someone who likes to avoid including unneeded code in the
kernel, but in this case I'd say KISS - and build it always into the
gigaset module.

> Tilman Schmidt

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2007-02-20 21:54:21

by Tilman Schmidt

[permalink] [raw]
Subject: Re: Kbuild problem

Am 20.02.2007 15:59 schrieb Adrian Bunk:
> I'm usually someone who likes to avoid including unneeded code in the
> kernel, but in this case I'd say KISS - and build it always into the
> gigaset module.

There seems to be a clear majority for this solution. So KISS it
will be. Thanks for all the advice. I'll do a new patch.

--
Tilman Schmidt E-Mail: [email protected]
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Unge?ffnet mindestens haltbar bis: (siehe R?ckseite)


Attachments:
signature.asc (253.00 B)
OpenPGP digital signature