[PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c compiling

[PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c compiling

[Michael Opdenacker]

[PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c compiling

Applies to 2.6.24-rc8-git1

This patch results from Linux Tiny's efforts to hunt for unnecessary
code added unconditionally with "obj-y +="

This patch make the compilation of arch/x86/kernel/pcspeaker.c
depend on PC speaker support. Before that, this file was always
compiled even if pc speaker support wasn't enabled.

I'm checking that CONFIG_INPUT_PCSPKR is not set to "m",
as arch/x86/kernel/pcspeaker.c shouldn't be compiled as a module:
- It defines no init and exit functions
- It defines an initcall which only makes sense at
  kernel boot time.

Results: if PC speaker support is disabled
- It reduces the uncompressed kernel size by 218 bytes!!! ;)
- It must also bring a tiny reduction in boot time
  but suppressing a useless initcall.

Your comments?

---
Signed-off-by: Michael Opdenacker <michael@free-electrons.com>

diff -Naur linux-2.6.24-rc8-git1/arch/x86/kernel/Makefile_32 linux-2.6.24-rc8-git1-nopcspeaker/arch/x86/kernel/Makefile_32
--- linux-2.6.24-rc8-git1/arch/x86/kernel/Makefile_32	2008-01-17 09:48:58.000000000 +0100
+++ linux-2.6.24-rc8-git1-nopcspeaker/arch/x86/kernel/Makefile_32	2008-01-17 10:06:56.000000000 +0100
@@ -45,10 +45,13 @@
 
 obj-$(CONFIG_VMI)		+= vmi_32.o vmiclock_32.o
 obj-$(CONFIG_PARAVIRT)		+= paravirt_32.o
-obj-y				+= pcspeaker.o
-
 obj-$(CONFIG_SCx200)		+= scx200_32.o
 
+ifeq ($(CONFIG_INPUT_PCSPKR),y)
+	obj-y			+= pcspeaker.o
+endif
+
+
 # vsyscall_32.o contains the vsyscall DSO images as __initdata.
 # We must build both images before we can assemble it.
 # Note: kbuild does not track this dependency due to usage of .incbin
diff -Naur linux-2.6.24-rc8-git1/arch/x86/kernel/Makefile_64 linux-2.6.24-rc8-git1-nopcspeaker/arch/x86/kernel/Makefile_64
--- linux-2.6.24-rc8-git1/arch/x86/kernel/Makefile_64	2008-01-17 09:48:58.000000000 +0100
+++ linux-2.6.24-rc8-git1-nopcspeaker/arch/x86/kernel/Makefile_64	2008-01-17 10:19:08.000000000 +0100
@@ -40,6 +40,9 @@
 obj-$(CONFIG_PCI)		+= early-quirks.o
 
 obj-y				+= topology.o
-obj-y				+= pcspeaker.o
+
+ifeq ($(CONFIG_INPUT_PCSPKR),y)
+        obj-y                   += pcspeaker.o
+endif
 
 CFLAGS_vsyscall_64.o		:= $(PROFILING) -g0

-- 
Michael Opdenacker, Free Electrons
Free Embedded Linux Training Materials
on http://free-electrons.com/training
(More than 1500 pages!)

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c compiling

[Matt Mackall]

On Thu, 2008-01-17 at 16:43 +0100, Michael Opdenacker wrote:
> [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c compiling
> 
> Applies to 2.6.24-rc8-git1
> 
> This patch results from Linux Tiny's efforts to hunt for unnecessary
> code added unconditionally with "obj-y +="
> 
> This patch make the compilation of arch/x86/kernel/pcspeaker.c
> depend on PC speaker support. Before that, this file was always
> compiled even if pc speaker support wasn't enabled.
> 
> I'm checking that CONFIG_INPUT_PCSPKR is not set to "m",
> as arch/x86/kernel/pcspeaker.c shouldn't be compiled as a module:
> - It defines no init and exit functions
> - It defines an initcall which only makes sense at
>   kernel boot time.
> 
> Results: if PC speaker support is disabled
> - It reduces the uncompressed kernel size by 218 bytes!!! ;)
> - It must also bring a tiny reduction in boot time
>   but suppressing a useless initcall.
> 
> Your comments?
> 
> ---
> Signed-off-by: Michael Opdenacker <michael@free-electrons.com>
> 
> diff -Naur linux-2.6.24-rc8-git1/arch/x86/kernel/Makefile_32 linux-2.6.24-rc8-git1-nopcspeaker/arch/x86/kernel/Makefile_32
> --- linux-2.6.24-rc8-git1/arch/x86/kernel/Makefile_32	2008-01-17 09:48:58.000000000 +0100
> +++ linux-2.6.24-rc8-git1-nopcspeaker/arch/x86/kernel/Makefile_32	2008-01-17 10:06:56.000000000 +0100
> @@ -45,10 +45,13 @@
>  
>  obj-$(CONFIG_VMI)		+= vmi_32.o vmiclock_32.o
>  obj-$(CONFIG_PARAVIRT)		+= paravirt_32.o
> -obj-y				+= pcspeaker.o
> -
>  obj-$(CONFIG_SCx200)		+= scx200_32.o
>  
> +ifeq ($(CONFIG_INPUT_PCSPKR),y)
> +	obj-y			+= pcspeaker.o
> +endif

I'm not sure this does what you want. What if CONFIG_INPUT_PCSPKR = m?

You ought to cc: this to Ingo (added). I'm pretty sure these Makefiles
have been unified in x86.git already.

-- 
Mathematics is the supreme nostalgia of our time.

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c compiling

[Michael Opdenacker]

Hi Matt,

Thanks for your feedback!

On 01/17/2008 05:36 PM, Matt Mackall wrote:
> On Thu, 2008-01-17 at 16:43 +0100, Michael Opdenacker wrote:
>   
>> diff -Naur linux-2.6.24-rc8-git1/arch/x86/kernel/Makefile_32 linux-2.6.24-rc8-git1-nopcspeaker/arch/x86/kernel/Makefile_32
>> --- linux-2.6.24-rc8-git1/arch/x86/kernel/Makefile_32	2008-01-17 09:48:58.000000000 +0100
>> +++ linux-2.6.24-rc8-git1-nopcspeaker/arch/x86/kernel/Makefile_32	2008-01-17 10:06:56.000000000 +0100
>> @@ -45,10 +45,13 @@
>>  
>>  obj-$(CONFIG_VMI)		+= vmi_32.o vmiclock_32.o
>>  obj-$(CONFIG_PARAVIRT)		+= paravirt_32.o
>> -obj-y				+= pcspeaker.o
>> -
>>  obj-$(CONFIG_SCx200)		+= scx200_32.o
>>  
>> +ifeq ($(CONFIG_INPUT_PCSPKR),y)
>> +	obj-y			+= pcspeaker.o
>> +endif
>>     
>
> I'm not sure this does what you want. What if CONFIG_INPUT_PCSPKR = m?
>   
Does it make sense to compile arch/x86/kernel/pcspeaker.c as a module?
It defines no init and exit functions, and it defines an initcall, which
only makes sense at boot time.

That's why I didn't use
obj-$(CONFIG_INPUT_PCSPKR)+=pcspeaker.o

Michael.

-- 
Michael Opdenacker, Free Electrons
Free Embedded Linux Training Materials
on http://free-electrons.com/training
(More than 1500 pages!)

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c compiling

[Matt Mackall]

On Thu, 2008-01-17 at 18:05 +0100, Michael Opdenacker wrote:
> Hi Matt,
> 
> Thanks for your feedback!
> 
> On 01/17/2008 05:36 PM, Matt Mackall wrote:
> > On Thu, 2008-01-17 at 16:43 +0100, Michael Opdenacker wrote:
> >   
> >> diff -Naur linux-2.6.24-rc8-git1/arch/x86/kernel/Makefile_32 linux-2.6.24-rc8-git1-nopcspeaker/arch/x86/kernel/Makefile_32
> >> --- linux-2.6.24-rc8-git1/arch/x86/kernel/Makefile_32	2008-01-17 09:48:58.000000000 +0100
> >> +++ linux-2.6.24-rc8-git1-nopcspeaker/arch/x86/kernel/Makefile_32	2008-01-17 10:06:56.000000000 +0100
> >> @@ -45,10 +45,13 @@
> >>  
> >>  obj-$(CONFIG_VMI)		+= vmi_32.o vmiclock_32.o
> >>  obj-$(CONFIG_PARAVIRT)		+= paravirt_32.o
> >> -obj-y				+= pcspeaker.o
> >> -
> >>  obj-$(CONFIG_SCx200)		+= scx200_32.o
> >>  
> >> +ifeq ($(CONFIG_INPUT_PCSPKR),y)
> >> +	obj-y			+= pcspeaker.o
> >> +endif
> >>     
> >
> > I'm not sure this does what you want. What if CONFIG_INPUT_PCSPKR = m?
> >   
> Does it make sense to compile arch/x86/kernel/pcspeaker.c as a module?
> It defines no init and exit functions, and it defines an initcall, which
> only makes sense at boot time.

Probably not. However, the above code won't build pcspeaker.o if
CONFIG_INPUT_PCSPKR = m. In other words, it'll break.

-- 
Mathematics is the supreme nostalgia of our time.

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c compiling

[Michael Opdenacker]

On 01/17/2008 06:13 PM, Matt Mackall wrote:
> On Thu, 2008-01-17 at 18:05 +0100, Michael Opdenacker wrote:
>   
>> Hi Matt,
>>
>> Thanks for your feedback!
>>
>> On 01/17/2008 05:36 PM, Matt Mackall wrote:
>>     
>>> On Thu, 2008-01-17 at 16:43 +0100, Michael Opdenacker wrote:
>>>   
>>>       
>>>> diff -Naur linux-2.6.24-rc8-git1/arch/x86/kernel/Makefile_32 linux-2.6.24-rc8-git1-nopcspeaker/arch/x86/kernel/Makefile_32
>>>> --- linux-2.6.24-rc8-git1/arch/x86/kernel/Makefile_32	2008-01-17 09:48:58.000000000 +0100
>>>> +++ linux-2.6.24-rc8-git1-nopcspeaker/arch/x86/kernel/Makefile_32	2008-01-17 10:06:56.000000000 +0100
>>>> @@ -45,10 +45,13 @@
>>>>  
>>>>  obj-$(CONFIG_VMI)		+= vmi_32.o vmiclock_32.o
>>>>  obj-$(CONFIG_PARAVIRT)		+= paravirt_32.o
>>>> -obj-y				+= pcspeaker.o
>>>> -
>>>>  obj-$(CONFIG_SCx200)		+= scx200_32.o
>>>>  
>>>> +ifeq ($(CONFIG_INPUT_PCSPKR),y)
>>>> +	obj-y			+= pcspeaker.o
>>>> +endif
>>>>     
>>>>         
>>> I'm not sure this does what you want. What if CONFIG_INPUT_PCSPKR = m?
>>>   
>>>       
>> Does it make sense to compile arch/x86/kernel/pcspeaker.c as a module?
>> It defines no init and exit functions, and it defines an initcall, which
>> only makes sense at boot time.
>>     
>
> Probably not. However, the above code won't build pcspeaker.o if
> CONFIG_INPUT_PCSPKR = m. In other words, it'll break.
>
>   
Oops, I got it. I will post a fix soon. Thanks!

:-)

Michael.

-- 
Michael Opdenacker, Free Electrons
Free Embedded Linux Training Materials
on http://free-electrons.com/training
(More than 1500 pages!)

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c compiling

[Michael Opdenacker]

On Thursday 17 January 2008, Matt Mackall wrote:
> > >> diff -Naur linux-2.6.24-rc8-git1/arch/x86/kernel/Makefile_32 linux-2.6.24-rc8-git1-nopcspeaker/arch/x86/kernel/Makefile_32
> > >> --- linux-2.6.24-rc8-git1/arch/x86/kernel/Makefile_32	2008-01-17 09:48:58.000000000 +0100
> > >> +++ linux-2.6.24-rc8-git1-nopcspeaker/arch/x86/kernel/Makefile_32	2008-01-17 10:06:56.000000000 +0100
> > >> @@ -45,10 +45,13 @@
> > >>  
> > >>  obj-$(CONFIG_VMI)		+= vmi_32.o vmiclock_32.o
> > >>  obj-$(CONFIG_PARAVIRT)		+= paravirt_32.o
> > >> -obj-y				+= pcspeaker.o
> > >> -
> > >>  obj-$(CONFIG_SCx200)		+= scx200_32.o
> > >>  
> > >> +ifeq ($(CONFIG_INPUT_PCSPKR),y)
> > >> +	obj-y			+= pcspeaker.o
> > >> +endif
> > >>     
> > >
> > > I'm not sure this does what you want. What if CONFIG_INPUT_PCSPKR = m?
> > >   
> > Does it make sense to compile arch/x86/kernel/pcspeaker.c as a module?
> > It defines no init and exit functions, and it defines an initcall, which
> > only makes sense at boot time.
> 
> Probably not. However, the above code won't build pcspeaker.o if
> CONFIG_INPUT_PCSPKR = m. In other words, it'll break.
> 

Here's a corrected version.

Even if this patch doesn't seem to cause any bug
(tested on a QEMU based PC), I'm wondering if this is "legal" (according
to Linux standards) not to declare a device we don't care about,
but that probably always exists in any PC-compatible machine?
Doesn't this hurt the consistency of the device model?

Another issue would be that we would no longer be able 
to load the speaker driver module from a kernel which
wasn't originally compiled with support for this module.

Perhaps we should only use this trick when CONFIG_EMBEDDED is set.
In this case, we know we're using a limited, non-standard kernel,
and having a partial device tree could be acceptable.

What do you think?

Thanks,

Michael.

Signed-off-by: Michael Opdenacker <michael@free-electrons.com>

diff -Naur linux-2.6.24-rc8-git1/arch/x86/kernel/Makefile_32 linux-2.6.24-rc8-git1-nopcspeaker/arch/x86/kernel/Makefile_32
--- linux-2.6.24-rc8-git1/arch/x86/kernel/Makefile_32	2008-01-17 09:48:58.000000000 +0100
+++ linux-2.6.24-rc8-git1-nopcspeaker/arch/x86/kernel/Makefile_32	2008-01-17 22:43:33.000000000 +0100
@@ -45,10 +45,13 @@
 
 obj-$(CONFIG_VMI)		+= vmi_32.o vmiclock_32.o
 obj-$(CONFIG_PARAVIRT)		+= paravirt_32.o
-obj-y				+= pcspeaker.o
-
 obj-$(CONFIG_SCx200)		+= scx200_32.o
 
+ifdef CONFIG_INPUT_PCSPKR
+	obj-y			+= pcspeaker.o
+endif
+
+
 # vsyscall_32.o contains the vsyscall DSO images as __initdata.
 # We must build both images before we can assemble it.
 # Note: kbuild does not track this dependency due to usage of .incbin
diff -Naur linux-2.6.24-rc8-git1/arch/x86/kernel/Makefile_64 linux-2.6.24-rc8-git1-nopcspeaker/arch/x86/kernel/Makefile_64
--- linux-2.6.24-rc8-git1/arch/x86/kernel/Makefile_64	2008-01-17 09:48:58.000000000 +0100
+++ linux-2.6.24-rc8-git1-nopcspeaker/arch/x86/kernel/Makefile_64	2008-01-17 22:46:07.000000000 +0100
@@ -40,6 +40,9 @@
 obj-$(CONFIG_PCI)		+= early-quirks.o
 
 obj-y				+= topology.o
-obj-y				+= pcspeaker.o
+
+ifdef CONFIG_INPUT_PCSPKR
+        obj-y			+= pcspeaker.o
+endif
 
 CFLAGS_vsyscall_64.o		:= $(PROFILING) -g0

-- 
Michael Opdenacker, Free Electrons
Free Embedded Linux Training Materials
on http://free-electrons.com/training
(More than 1500 pages!)

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c compiling

[Jan Engelhardt]

On Jan 17 2008 18:05, Michael Opdenacker wrote:
>>> +ifeq ($(CONFIG_INPUT_PCSPKR),y)
>>> +	obj-y			+= pcspeaker.o
>>> +endif
>>>     
>>
>> I'm not sure this does what you want. What if CONFIG_INPUT_PCSPKR = m?
>>   
>Does it make sense to compile arch/x86/kernel/pcspeaker.c as a module?

Hardly. No. Absolutely not. pcspeaker.c is a mere 20 lines - when
compiled (on x86_64) a mere 1824 bytes (and most of that is probably
ELF overhead). If you build that as a module, be sure to add an extra
6 to 7 kilobyte as module overhead. No, this is not really worth it.

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c compiling

[Taral]

On 1/17/08, Michael Opdenacker <michael-lists@free-electrons.com> wrote:
> Another issue would be that we would no longer be able
> to load the speaker driver module from a kernel which
> wasn't originally compiled with support for this module.

Have you looked at pcspeaker.o? As far as I can tell, it does *nothing*.

-- 
Taral <taralx@gmail.com>
"Please let me know if there's any further trouble I can give you."
    -- Unknown

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c compiling

[Michael Opdenacker]

On 01/18/2008 04:16 AM, Taral wrote:
> On 1/17/08, Michael Opdenacker <michael-lists@free-electrons.com> wrote:
>   
>> Another issue would be that we would no longer be able
>> to load the speaker driver module from a kernel which
>> wasn't originally compiled with support for this module.
>>     
>
> Have you looked at pcspeaker.o? As far as I can tell, it does *nothing*.
>   
Do you mean "almost nothing"? It still allocates and adds a platform
device, and the corresponding function always gets called at boot time.

I know that not compiling this piece of code just reduces the
uncompressed kernel size by just a few bytes (218). However, many small
contributions of this kind can have a significant impact on embedded
systems (or on boot media or on Linux based bootloaders).

As I said earlier, I'm starting to think that this trick should only be
used when CONFIG_EMBEDDED is set. In the non-embedded case, it's
probably not acceptable not to declare a platform device that is always
present in the system (while it's perfectly fine not to load the
corresponding driver).

Your comments and suggestions are more than welcome!

Michael.

-- 
Michael Opdenacker, Free Electrons
Free Embedded Linux Training Materials
on http://free-electrons.com/training
(More than 1500 pages!)

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Ingo Molnar]

* Michael Opdenacker <michael-lists@free-electrons.com> wrote:

>  obj-$(CONFIG_PARAVIRT)		+= paravirt_32.o
> -obj-y				+= pcspeaker.o
> -
>  obj-$(CONFIG_SCx200)		+= scx200_32.o
>  
> +ifdef CONFIG_INPUT_PCSPKR
> +	obj-y			+= pcspeaker.o
> +endif

why didnt you make this:

  obj-$(CONFIG_INPUT_PCSPKR)		+= pcspeaker.o

?

Your patch looks fine to me otherwise, obviously if someone disables 
PCSPKR intentionally in the .config, the kernel should just do that. 
Could you resend it with the above thing fixed, and against x86.git#mm? 
The x86.git coordinates are at:

 http://redhat.com/~mingo/x86.git/README

	Ingo

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Michael Opdenacker]

On 01/18/2008 12:02 PM, Ingo Molnar wrote:
>
> why didnt you make this:
>
>   obj-$(CONFIG_INPUT_PCSPKR)		+= pcspeaker.o
>
> ?
>   
Many thanks for your feedback.

That's what I did first, but if  CONFIG_INPUT_PCSPKR=m,
arch/x86/kernel/pcspeaker.c gets compiled as a module. While compiling
doesn't fail, is this still a valid module? It defines no init and exit
functions, and it defines an initcall, which only makes sense at boot time.

We could make pcspeaker.c depend on another switch, like
CONFIG_PCSPEAKER on mips. We could offer the possibility to disable it
when CONFIG_EMBEDDED is set.

What do you think?

Michael.

-- 
Michael Opdenacker, Free Electrons
Free Embedded Linux Training Materials
on http://free-electrons.com/training
(More than 1500 pages!)

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Ingo Molnar]

* Michael Opdenacker <michael-lists@free-electrons.com> wrote:

> On 01/18/2008 12:02 PM, Ingo Molnar wrote:
> >
> > why didnt you make this:
> >
> >   obj-$(CONFIG_INPUT_PCSPKR)		+= pcspeaker.o
> >
> > ?
> >   
> Many thanks for your feedback.
> 
> That's what I did first, but if CONFIG_INPUT_PCSPKR=m, 
> arch/x86/kernel/pcspeaker.c gets compiled as a module. While compiling 
> doesn't fail, is this still a valid module? It defines no init and 
> exit functions, and it defines an initcall, which only makes sense at 
> boot time.
> 
> We could make pcspeaker.c depend on another switch, like 
> CONFIG_PCSPEAKER on mips. We could offer the possibility to disable it 
> when CONFIG_EMBEDDED is set.

i'm confused, the .ko definitely exists:

 europe:~> uname -r
 2.6.24-0.123.rc6.fc9

 europe:~> lsmod | grep pcspkr
 pcspkr                  6400  0

ah, this is a different .ko.

perhaps the right solution would be to only build it in if 
CONFIG_PCSPEAKER is "y" or "m". I.e. your original patch?

	Ingo

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

> perhaps the right solution would be to only build it in if 
> CONFIG_PCSPEAKER is "y" or "m". I.e. your original patch?

i've added your patch to x86.git - see below.

	Ingo

----------->
Subject: x86: fix unconditional arch/x86/kernel/pcspeaker.o compiling
From: Michael Opdenacker <michael-lists@free-electrons.com>

do not add the pcspkr platform device if pcspkr support is disabled.

Signed-off-by: Michael Opdenacker <michael@free-electrons.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/kernel/Makefile |    3 +++
 1 file changed, 3 insertions(+)

Index: linux-x86.q/arch/x86/kernel/Makefile
===================================================================
--- linux-x86.q.orig/arch/x86/kernel/Makefile
+++ linux-x86.q/arch/x86/kernel/Makefile
@@ -68,7 +68,10 @@ obj-$(CONFIG_MGEODE_LX)		+= geode_32.o m
 
 obj-$(CONFIG_VMI)		+= vmi_32.o vmiclock_32.o
 obj-$(CONFIG_PARAVIRT)		+= paravirt.o paravirt_patch_$(BITS).o
+
+ifdef CONFIG_INPUT_PCSPKR
 obj-y				+= pcspeaker.o
+endif
 
 obj-$(CONFIG_SCx200)		+= scx200_32.o
 

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Michael Opdenacker]

On 01/18/2008 01:29 PM, Ingo Molnar wrote:
>> perhaps the right solution would be to only build it in if 
>> CONFIG_PCSPEAKER is "y" or "m". I.e. your original patch?
>>     
>
> i've added your patch to x86.git - see below.
>   
Many thanks Ingo!
> Index: linux-x86.q/arch/x86/kernel/Makefile
> ===================================================================
> --- linux-x86.q.orig/arch/x86/kernel/Makefile
> +++ linux-x86.q/arch/x86/kernel/Makefile
> @@ -68,7 +68,10 @@ obj-$(CONFIG_MGEODE_LX)		+= geode_32.o m
>  
>  obj-$(CONFIG_VMI)		+= vmi_32.o vmiclock_32.o
>  obj-$(CONFIG_PARAVIRT)		+= paravirt.o paravirt_patch_$(BITS).o
> +
> +ifdef CONFIG_INPUT_PCSPKR
>  obj-y				+= pcspeaker.o
> +endif
>  
>  obj-$(CONFIG_SCx200)		+= scx200_32.o
>   
However, wouldn't the Makefile look nicer if we introduced a
CONFIG_PCSPEAKER setting as in the mips platform? We would just have:

obj-$(CONFIG_PCSPEAKER)		+= pcspeaker.o


I can propose a corresponding patch, and I'd suggest to make
CONFIG_PCSPEAKER depend on CONFIG_EMBEDDED.

Thanks again!

:-)

Michael.

-- 
Michael Opdenacker, Free Electrons
Free Embedded Linux Training Materials
on http://free-electrons.com/training
(More than 1500 pages!)

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Matt Mackall]

On Fri, 2008-01-18 at 14:03 +0100, Michael Opdenacker wrote:
> However, wouldn't the Makefile look nicer if we introduced a
> CONFIG_PCSPEAKER setting as in the mips platform? We would just have:
> 
> obj-$(CONFIG_PCSPEAKER)		+= pcspeaker.o

Yes, that would be cleaner. Not worth it for just x86, but there are
about 6 other arches that use CONFIG_INPUT_PCSPKR. Do that in one patch
on top of this one and send it to Andrew (who will pull the above into
his tree from Ingo shortly).

> I can propose a corresponding patch, and I'd suggest to make
> CONFIG_PCSPEAKER depend on CONFIG_EMBEDDED.

No, don't. It's fine the way it is. If INPUT_PCSPKR isn't set, we don't
support the speaker, end of story.

Also, bear in mind that there is a fair amount of lower-hanging fruit
than this, so bouncing a bunch of patches back and forth to make this
perfect is not the best use of people's time.

-- 
Mathematics is the supreme nostalgia of our time.

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Ingo Molnar]

* Matt Mackall <mpm@selenic.com> wrote:

> > I can propose a corresponding patch, and I'd suggest to make 
> > CONFIG_PCSPEAKER depend on CONFIG_EMBEDDED.
> 
> No, don't. It's fine the way it is. If INPUT_PCSPKR isn't set, we 
> don't support the speaker, end of story.

yeah.

> Also, bear in mind that there is a fair amount of lower-hanging fruit 
> than this, so bouncing a bunch of patches back and forth to make this 
> perfect is not the best use of people's time.

as long as it's arch/x86 stuff i can pick up patches no problem.

[ Sidenote: "too small" and "too insignificant" is not a patch attribute
  that is in my vocabulary - by definition the best patches are very
  small and very insignificant (they just happen to end up doing
  something cool a 1000 steps later ;-) 99% of our problems come from
  'too large' and 'too intrusive' patches. ]

	Ingo

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Matt Mackall]

On Fri, 2008-01-18 at 14:57 +0100, Ingo Molnar wrote:
> [ Sidenote: "too small" and "too insignificant" is not a patch attribute
>   that is in my vocabulary - by definition the best patches are very
>   small and very insignificant (they just happen to end up doing
>   something cool a 1000 steps later ;-) 99% of our problems come from
>   'too large' and 'too intrusive' patches. ]

I tend to agree, but it's well-established that not everyone here is so
patient.

-- 
Mathematics is the supreme nostalgia of our time.

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Michael Opdenacker]

On 01/18/2008 02:50 PM, Matt Mackall wrote:
> On Fri, 2008-01-18 at 14:03 +0100, Michael Opdenacker wrote:
>   
>> However, wouldn't the Makefile look nicer if we introduced a
>> CONFIG_PCSPEAKER setting as in the mips platform? We would just have:
>>
>> obj-$(CONFIG_PCSPEAKER)		+= pcspeaker.o
>>     
>
> Yes, that would be cleaner. Not worth it for just x86, but there are
> about 6 other arches that use CONFIG_INPUT_PCSPKR. Do that in one patch
> on top of this one and send it to Andrew (who will pull the above into
> his tree from Ingo shortly).
>   
Well, sorry for consuming your time with this very small change, but I'm
not sure I understand what you're suggesting... Here's what I'd do:

    * I'd define  CONFIG_PCSPEAKER for x86 (typically in
      arch/x86/Kconfig), to be set to "y"  when CONFIG_INPUT_SPKR is
      set. This is what minimizes the changes to make.
    * This way, we can just have:
      obj-$(CONFIG_PCSPEAKER) += pcspeaker.o

Did I understand things right? Anyway, the current situation is not so
bad either.
> Also, bear in mind that there is a fair amount of lower-hanging fruit
> than this, so bouncing a bunch of patches back and forth to make this
> perfect is not the best use of people's time.
>   
Sounds fine! Don't hesitate to let us know about the lower-hanging fruit
you're thinking about. Here are the main things I have so far:

    * Ideas in the existing Linux-Tiny patchset.
    * Disable support for non-Intel processors in x86 (cyrix.c,
      centaur.c, transmeta.c, nexgen.c, umc.c in arch/x86/kernel/cpu).
      As far as I remember, I saved 15 KB when I first experimented with
      this).
    * Disable support for readahead, page writeback, pdflush and swap
      when we have no storage at all (typically booting from an
      initramfs). This corresponds to 69 KB of source code!

So, more ideas are welcome!

Thanks for your time,

Michael.

-- 
Michael Opdenacker, Free Electrons
Free Embedded Linux Training Materials
on http://free-electrons.com/training
(More than 1500 pages!)

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Matt Mackall]

On Fri, 2008-01-18 at 17:29 +0100, Michael Opdenacker wrote:
> On 01/18/2008 02:50 PM, Matt Mackall wrote:
> > On Fri, 2008-01-18 at 14:03 +0100, Michael Opdenacker wrote:
> >   
> >> However, wouldn't the Makefile look nicer if we introduced a
> >> CONFIG_PCSPEAKER setting as in the mips platform? We would just have:
> >>
> >> obj-$(CONFIG_PCSPEAKER)		+= pcspeaker.o
> >>     
> >
> > Yes, that would be cleaner. Not worth it for just x86, but there are
> > about 6 other arches that use CONFIG_INPUT_PCSPKR. Do that in one patch
> > on top of this one and send it to Andrew (who will pull the above into
> > his tree from Ingo shortly).
> >   
> Well, sorry for consuming your time with this very small change, but I'm
> not sure I understand what you're suggesting... Here's what I'd do:
> 
>     * I'd define  CONFIG_PCSPEAKER for x86 (typically in
>       arch/x86/Kconfig), to be set to "y"  when CONFIG_INPUT_SPKR is
>       set. This is what minimizes the changes to make.
>     * This way, we can just have:
>       obj-$(CONFIG_PCSPEAKER) += pcspeaker.o

Probably makes sense to define it right next to INPUT_PCSPKR in
drivers/input/Kconfig.

Then do the appropriate fix for all arches mentioned in INPUT_PCSPKR.

For extra points, you can move the duplicate pcspeaker.c code out of all
those arches and stash it somewhere in drivers/input. Presumably it's
possible to get it to link into the kernel even when INPUT is modular.

> Sounds fine! Don't hesitate to let us know about the lower-hanging fruit
> you're thinking about. Here are the main things I have so far:
> 
>     * Ideas in the existing Linux-Tiny patchset.
>     * Disable support for non-Intel processors in x86 (cyrix.c,
>       centaur.c, transmeta.c, nexgen.c, umc.c in arch/x86/kernel/cpu).
>       As far as I remember, I saved 15 KB when I first experimented with
>       this).

Isn't that already in -tiny?

>     * Disable support for readahead, page writeback, pdflush and swap
>       when we have no storage at all (typically booting from an
>       initramfs). This corresponds to 69 KB of source code!

That'd be nice, yes. It would probably make sense to be able to disable
just readahead support when we're working with only solid-state devices.

-- 
Mathematics is the supreme nostalgia of our time.

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Ingo Molnar]

* Matt Mackall <mpm@selenic.com> wrote:

> > Sounds fine! Don't hesitate to let us know about the lower-hanging 
> > fruit you're thinking about. Here are the main things I have so far:
> > 
> >     * Ideas in the existing Linux-Tiny patchset.
> >     * Disable support for non-Intel processors in x86 (cyrix.c,
> >       centaur.c, transmeta.c, nexgen.c, umc.c in arch/x86/kernel/cpu).
> >       As far as I remember, I saved 15 KB when I first experimented with
> >       this).
> 
> Isn't that already in -tiny?

btw., are there any pending arch/x86 bits in -tiny? (stupid question: 
were can i get the most uptodate version of -tiny from?)

	Ingo

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Matt Mackall]

On Fri, 2008-01-18 at 22:09 +0100, Ingo Molnar wrote:
> * Matt Mackall <mpm@selenic.com> wrote:
> 
> > > Sounds fine! Don't hesitate to let us know about the lower-hanging 
> > > fruit you're thinking about. Here are the main things I have so far:
> > > 
> > >     * Ideas in the existing Linux-Tiny patchset.
> > >     * Disable support for non-Intel processors in x86 (cyrix.c,
> > >       centaur.c, transmeta.c, nexgen.c, umc.c in arch/x86/kernel/cpu).
> > >       As far as I remember, I saved 15 KB when I first experimented with
> > >       this).
> > 
> > Isn't that already in -tiny?
> 
> btw., are there any pending arch/x86 bits in -tiny? (stupid question: 
> were can i get the most uptodate version of -tiny from?)

It's not a stupid question. I dropped updating the tree regulary some
time ago to focus on merging bits and then got a bit side-tracked by
this little thing called "version control".

Michael is attempting to get the tree started again and has put a quilt
up here:

http://elinux.org/images/3/3c/Tiny-quilt-2.6.23-0.tar.bz2

-- 
Mathematics is the supreme nostalgia of our time.

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c compiling

[Taral]

On 1/18/08, Michael Opdenacker <michael-lists@free-electrons.com> wrote:
> Do you mean "almost nothing"? It still allocates and adds a platform
> device, and the corresponding function always gets called at boot time.

Nothing significant then. I don't see any added functionality from this file.

-- 
Taral <taralx@gmail.com>
"Please let me know if there's any further trouble I can give you."
    -- Unknown

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Rob Landley]

On Friday 18 January 2008 11:10:19 Matt Mackall wrote:
> >     * Disable support for readahead, page writeback, pdflush and swap
> >       when we have no storage at all (typically booting from an
> >       initramfs). This corresponds to 69 KB of source code!
>
> That'd be nice, yes. It would probably make sense to be able to disable
> just readahead support when we're working with only solid-state devices.

Very nice.  From a UI standpoint, shouldn't disabling the block layer take at 
least some of that out?  (Or disabling the block layer and all network 
filesystems.  /me tries to remember whether jffs2 depends on the block layer.  
Now that qemu can fake an mtd, I really need to start playing with that...)

Rob
-- 
"One of my most productive days was throwing away 1000 lines of code."
  - Ken Thompson.

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Rob Landley]

On Friday 18 January 2008 10:29:23 Michael Opdenacker wrote:
> Sounds fine! Don't hesitate to let us know about the lower-hanging fruit
> you're thinking about. Here are the main things I have so far:
>
>     * Ideas in the existing Linux-Tiny patchset.
>     * Disable support for non-Intel processors in x86 (cyrix.c,
>       centaur.c, transmeta.c, nexgen.c, umc.c in arch/x86/kernel/cpu).
>       As far as I remember, I saved 15 KB when I first experimented with
>       this).
>     * Disable support for readahead, page writeback, pdflush and swap
>       when we have no storage at all (typically booting from an
>       initramfs). This corresponds to 69 KB of source code!
>
> So, more ideas are welcome!

Personally, I'm trying to figure out how to build a "hello world" kernel.

I.E. the minimal set of files I need to compile to get a kernel I can boot 
under qemu that does something visible.  (Printing "hello world" to the 
serial port via the early_serial_write(), perhaps?)

No VFS, no process management, preferably no memory management...  Just get to 
a stub start_kernel() that calls early_serial_write() and then does a HLT.  I 
note that start_kernel() calls 11 things before the first printk(), and that 
without the early_printk stuff that's just going into a buffer.

Preferably, I even want to take this out of the kernel's build process and 
build it from the command line or a small shell script.  (zImage packaging 
and all.)  I.E. some kind of:

  gcc blah blah blah -o vmlinux
  wrap.sh vmlinux zImage

The reason for this is, of course, that I'm trying to build the kernel with my 
tinycc fork, and the whole kernel build is kind of intimidating and 
complicated.  (And I currently need to pregenerate asm-offsets.h until I 
teach tinycc to produce assembly output or come up with an alternative.)

Currently, a "make allnoconfig" build runs for 479 lines, and builds radix 
tree code and /dev/random and sysfs and all sorts of stuff.  
Counter-intuitively, allnoconfig _isn't_ the smallest kernel you can build 
(by a long shot), to do that you have to switch on the embedded menu and then 
switch off everything in it (and all the stuff that shows up right _after_ 
it, which smells like a bug to me), and switch to SLOB and switch off the 
block layer at the top level.  But doing that only gets the build down to 449 
lines, and the resulting x86-64 kernel is over 400k.  That is _not_ a "hello 
world" kernel.  Not even close. :)

Alas, working from the other end and building your way _up_ isn't trivial 
either.  (But has the advantage that I'm starting much closer to my goal than 
the stripping down approach.)

> Thanks for your time,
>
> Michael.

Rob
-- 
"One of my most productive days was throwing away 1000 lines of code."
  - Ken Thompson.

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Matt Mackall]

On Sat, 2008-01-19 at 22:59 -0600, Rob Landley wrote:
> On Friday 18 January 2008 11:10:19 Matt Mackall wrote:
> > >     * Disable support for readahead, page writeback, pdflush and swap
> > >       when we have no storage at all (typically booting from an
> > >       initramfs). This corresponds to 69 KB of source code!
> >
> > That'd be nice, yes. It would probably make sense to be able to disable
> > just readahead support when we're working with only solid-state devices.
> 
> Very nice.  From a UI standpoint, shouldn't disabling the block layer take at 
> least some of that out?

There are a number of laptops now that ship with solid-state disks.
These things look like normal IDE block devices to the kernel, but have
zero seek time and zero rotational latency. So here, prefetch is a waste
of memory and probably increases latency on average.

This will also be true for using prefetch on a typical embedded board
using compact flash through an IDE interface controller (extremely
common).

-- 
Mathematics is the supreme nostalgia of our time.

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Michael Opdenacker]

On 01/18/2008 06:10 PM, Matt Mackall wrote:
>> Sounds fine! Don't hesitate to let us know about the lower-hanging fruit
>> you're thinking about. Here are the main things I have so far:
>>
>>     * Ideas in the existing Linux-Tiny patchset.
>>     * Disable support for non-Intel processors in x86 (cyrix.c,
>>       centaur.c, transmeta.c, nexgen.c, umc.c in arch/x86/kernel/cpu).
>>       As far as I remember, I saved 15 KB when I first experimented with
>>       this).
>>     
>
> Isn't that already in -tiny?
>   
Oops, yes! I'll update these patches and post them again as soon as I
get a chance (hopefully soon).

Thanks for your support,

:-)

Michael.

-- 
Michael Opdenacker, Free Electrons
Free Embedded Linux Training Materials
on http://free-electrons.com/training
(More than 1500 pages!)

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Ingo Molnar]

* Matt Mackall <mpm@selenic.com> wrote:

> > btw., are there any pending arch/x86 bits in -tiny? (stupid 
> > question: were can i get the most uptodate version of -tiny from?)
> 
> It's not a stupid question. I dropped updating the tree regulary some 
> time ago to focus on merging bits and then got a bit side-tracked by 
> this little thing called "version control".
> 
> Michael is attempting to get the tree started again and has put a 
> quilt up here:
> 
> http://elinux.org/images/3/3c/Tiny-quilt-2.6.23-0.tar.bz2

cool! A few comments about the patches that affect arch/x86:

dmi_blacklist.patch:

  +#ifdef CONFIG_DMI_SCAN
          dmi_scan_machine();
  +#endif

put that #ifdef into the include file and make it an inlined NOP in the 
!DMI_SCAN case. That de-#ifdef-ifies the .c files.

cpu-support.patch: ditto.

mtrr.patch: ditto.

inflate-error.patch: please use:

   quilt refresh --diffstat --sort --no-timestamps -p 1

when generating patches, to make them reviewable, uniform and 
noise-free.

no-doublefault.patch: looks ok. Please add proper metadata and submit to 
lkml.

sbf.patch: looks ok.

sysenter.patch:

+#ifdef CONFIG_SYSENTER
        sysenter_setup();
        enable_sep_cpu();
+#endif

hide these #ifdefs in the include file. (as it already does) Looks ok 
otherwise.

threadinfo-ool.patch: doesnt this break the scheduler? Had something 
like this in -rt, i turned 'current' into a const actually, but had 
trouble with keeping from gcc to do the right thing in sched.c. I guess 
we could live with current_non_const() that gets it from assembly. (and 
hence gcc wont be able to optimize it away in the middle of the 
context-switch) But when done properly, this is both a nice, measurable 
speedup on UP, and a nice space-saver.

tiny-cflags.patch: obsolete? Isnt CFLAGS already extendable? Question to 
Sam i guess.

tsc.patch: look ok. needs x86.git porting.

	Ingo

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Matt Mackall]

On Tue, 2008-01-22 at 15:39 +0100, Ingo Molnar wrote:
> threadinfo-ool.patch: doesnt this break the scheduler? 

It didn't when I wrote it, 3+ years ago. But I'm sure it needs to be
revisited.

> tiny-cflags.patch: obsolete? Isnt CFLAGS already extendable? Question to 
> Sam i guess.

Yup.

-- 
Mathematics is the supreme nostalgia of our time.

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Sam Ravnborg]

On Tue, Jan 22, 2008 at 10:37:19AM -0600, Matt Mackall wrote:
> 
> On Tue, 2008-01-22 at 15:39 +0100, Ingo Molnar wrote:
> > threadinfo-ool.patch: doesnt this break the scheduler? 
> 
> It didn't when I wrote it, 3+ years ago. But I'm sure it needs to be
> revisited.
> 
> > tiny-cflags.patch: obsolete? Isnt CFLAGS already extendable? Question to 
> > Sam i guess.
And what was the question then?

We have today the possibility to say:
make KCFLAGS=-whatever

and we have plenty of kconfig adjustmenst affecting the gcc options.

I do not know if this covers it.

	Sam

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Matt Mackall]

On Tue, 2008-01-22 at 19:58 +0100, Sam Ravnborg wrote:
> On Tue, Jan 22, 2008 at 10:37:19AM -0600, Matt Mackall wrote:
> > 
> > On Tue, 2008-01-22 at 15:39 +0100, Ingo Molnar wrote:
> > > threadinfo-ool.patch: doesnt this break the scheduler? 
> > 
> > It didn't when I wrote it, 3+ years ago. But I'm sure it needs to be
> > revisited.
> > 
> > > tiny-cflags.patch: obsolete? Isnt CFLAGS already extendable? Question to 
> > > Sam i guess.
> And what was the question then?
> 
> We have today the possibility to say:
> make KCFLAGS=-whatever
> 
> and we have plenty of kconfig adjustmenst affecting the gcc options.
> 
> I do not know if this covers it.

Basically the idea was you could specify various flags that affected
kernel size, in particular overriding the various bloated alignment
defaults.

If I were to do this today (if they haven't already become the default),
I'd probably add a config var to request minimal alignment instead.

-- 
Mathematics is the supreme nostalgia of our time.

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Michael Opdenacker]

On Friday 18 January 2008, Matt Mackall wrote:
> 
> Probably makes sense to define it right next to INPUT_PCSPKR in
> drivers/input/Kconfig.
> 
> Then do the appropriate fix for all arches mentioned in INPUT_PCSPKR.
> 
> For extra points, you can move the duplicate pcspeaker.c code out of all
> those arches and stash it somewhere in drivers/input. Presumably it's
> possible to get it to link into the kernel even when INPUT is modular.

Here's the patch, after spending some time to get familiar with git.

The patch is against git x86/mm, and it seems to work fine on x86. However,
on x86, you no longer have /sys/devices/platform/pcspkr after
running "make allnoconfig". To get it, you have to explicitely add support
to CONFIG_INPUT_PCSPKR (m or y). I hope this is fine.

I'm copying the MIPS maintainer as this patch touches his tree too.

On MIPS, there should be no impact though, as CONFIG_PCSPEAKER is set in
defconfig files.

In other architectures where CONFIG_INPUT_PCSPKR can exist,
there is a change: when CONFIG_INPUT_PCSPKR is set, the platform device
will be added, while it didn't exist before. I hope this is fine.

In a nutshell, this patch looks good because it removes a small amount
of duplication between mips and x86. On the other hand, it introduces
changes that may not be wanted. Is this worth it?

Michael.

--
Signed-off-by: Michael Opdenacker <michael@free-electrons.com>

---
 arch/mips/kernel/Makefile      |    1 -
 arch/mips/kernel/pcspeaker.c   |   28 ----------------------------
 arch/x86/kernel/Makefile       |    4 ----
 arch/x86/kernel/pcspeaker.c    |   20 --------------------
 drivers/input/misc/Kconfig     |    4 ++++
 drivers/input/misc/Makefile    |    1 +
 drivers/input/misc/pcspeaker.c |   28 ++++++++++++++++++++++++++++
 7 files changed, 33 insertions(+), 53 deletions(-)
 delete mode 100644 arch/mips/kernel/pcspeaker.c
 delete mode 100644 arch/x86/kernel/pcspeaker.c
 create mode 100644 drivers/input/misc/pcspeaker.c

diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile
index ffa0836..9e78e1a 100644
--- a/arch/mips/kernel/Makefile
+++ b/arch/mips/kernel/Makefile
@@ -76,7 +76,6 @@ obj-$(CONFIG_PROC_FS)		+= proc.o
 obj-$(CONFIG_64BIT)		+= cpu-bugs64.o
 
 obj-$(CONFIG_I8253)		+= i8253.o
-obj-$(CONFIG_PCSPEAKER)		+= pcspeaker.o
 
 obj-$(CONFIG_KEXEC)		+= machine_kexec.o relocate_kernel.o
 obj-$(CONFIG_EARLY_PRINTK)	+= early_printk.o
diff --git a/arch/mips/kernel/pcspeaker.c b/arch/mips/kernel/pcspeaker.c
deleted file mode 100644
index 475df69..0000000
--- a/arch/mips/kernel/pcspeaker.c
+++ /dev/null
@@ -1,28 +0,0 @@
-/*
- * Copyright (C) 2006 IBM Corporation
- *
- * Implements device information for i8253 timer chip
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License version
- * 2 as published by the Free Software Foundation
- */
-
-#include <linux/platform_device.h>
-
-static __init int add_pcspkr(void)
-{
-	struct platform_device *pd;
-	int ret;
-
-	pd = platform_device_alloc("pcspkr", -1);
-	if (!pd)
-		return -ENOMEM;
-
-	ret = platform_device_add(pd);
-	if (ret)
-		platform_device_put(pd);
-
-	return ret;
-}
-device_initcall(add_pcspkr);
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index e8386eb..959ab30 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -69,10 +69,6 @@ obj-$(CONFIG_MGEODE_LX)		+= geode_32.o mfgpt_32.o
 obj-$(CONFIG_VMI)		+= vmi_32.o vmiclock_32.o
 obj-$(CONFIG_PARAVIRT)		+= paravirt.o paravirt_patch_$(BITS).o
 
-ifdef CONFIG_INPUT_PCSPKR
-obj-y				+= pcspeaker.o
-endif
-
 obj-$(CONFIG_SCx200)		+= scx200_32.o
 
 ###
diff --git a/arch/x86/kernel/pcspeaker.c b/arch/x86/kernel/pcspeaker.c
deleted file mode 100644
index bc1f2d3..0000000
--- a/arch/x86/kernel/pcspeaker.c
+++ /dev/null
@@ -1,20 +0,0 @@
-#include <linux/platform_device.h>
-#include <linux/errno.h>
-#include <linux/init.h>
-
-static __init int add_pcspkr(void)
-{
-	struct platform_device *pd;
-	int ret;
-
-	pd = platform_device_alloc("pcspkr", -1);
-	if (!pd)
-		return -ENOMEM;
-
-	ret = platform_device_add(pd);
-	if (ret)
-		platform_device_put(pd);
-
-	return ret;
-}
-device_initcall(add_pcspkr);
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 8f5c7b9..45b14ce 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -14,6 +14,7 @@ if INPUT_MISC
 
 config INPUT_PCSPKR
 	tristate "PC Speaker support"
+	select PCSPEAKER
 	depends on ALPHA || X86 || MIPS || PPC_PREP || PPC_CHRP || PPC_PSERIES
 	help
 	  Say Y here if you want the standard PC Speaker to be used for
@@ -24,6 +25,9 @@ config INPUT_PCSPKR
 	  To compile this driver as a module, choose M here: the
 	  module will be called pcspkr.
 
+config PCSPEAKER
+        bool
+
 config INPUT_SPARCSPKR
 	tristate "SPARC Speaker support"
 	depends on PCI && SPARC64
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 3585b50..4e51822 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -6,6 +6,7 @@
 
 obj-$(CONFIG_INPUT_SPARCSPKR)		+= sparcspkr.o
 obj-$(CONFIG_INPUT_PCSPKR)		+= pcspkr.o
+obj-$(CONFIG_PCSPEAKER)			+= pcspeaker.o
 obj-$(CONFIG_INPUT_M68K_BEEP)		+= m68kspkr.o
 obj-$(CONFIG_INPUT_IXP4XX_BEEPER)	+= ixp4xx-beeper.o
 obj-$(CONFIG_INPUT_COBALT_BTNS)		+= cobalt_btns.o
diff --git a/drivers/input/misc/pcspeaker.c b/drivers/input/misc/pcspeaker.c
new file mode 100644
index 0000000..475df69
--- /dev/null
+++ b/drivers/input/misc/pcspeaker.c
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 2006 IBM Corporation
+ *
+ * Implements device information for i8253 timer chip
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation
+ */
+
+#include <linux/platform_device.h>
+
+static __init int add_pcspkr(void)
+{
+	struct platform_device *pd;
+	int ret;
+
+	pd = platform_device_alloc("pcspkr", -1);
+	if (!pd)
+		return -ENOMEM;
+
+	ret = platform_device_add(pd);
+	if (ret)
+		platform_device_put(pd);
+
+	return ret;
+}
+device_initcall(add_pcspkr);
-- 
1.5.2.5

-- 
Michael Opdenacker, Free Electrons
Free Embedded Linux Training Materials
on http://free-electrons.com/training
(More than 1500 pages!)

Re: [PATCH] x86: fix?unconditional?arch/x86/kernel/pcspeaker.c?compiling

[Adrian Bunk]

On Wed, Jan 23, 2008 at 11:30:11PM +0100, Michael Opdenacker wrote:
> On Friday 18 January 2008, Matt Mackall wrote:
> > 
> > Probably makes sense to define it right next to INPUT_PCSPKR in
> > drivers/input/Kconfig.
> > 
> > Then do the appropriate fix for all arches mentioned in INPUT_PCSPKR.
> > 
> > For extra points, you can move the duplicate pcspeaker.c code out of all
> > those arches and stash it somewhere in drivers/input. Presumably it's
> > possible to get it to link into the kernel even when INPUT is modular.
> 
> Here's the patch, after spending some time to get familiar with git.
> 
> The patch is against git x86/mm, and it seems to work fine on x86. However,
> on x86, you no longer have /sys/devices/platform/pcspkr after
> running "make allnoconfig". To get it, you have to explicitely add support
> to CONFIG_INPUT_PCSPKR (m or y). I hope this is fine.
> 
> I'm copying the MIPS maintainer as this patch touches his tree too.
> 
> On MIPS, there should be no impact though, as CONFIG_PCSPEAKER is set in
> defconfig files.
>...

The defconfig files do not set anything, they are just .config's you can 
use as a start for configuring your kernel.

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

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Dmitri Vorobiev]

Michael Opdenacker пишет:
> On Friday 18 January 2008, Matt Mackall wrote:
>> Probably makes sense to define it right next to INPUT_PCSPKR in
>> drivers/input/Kconfig.
>>
>> Then do the appropriate fix for all arches mentioned in INPUT_PCSPKR.
>>
>> For extra points, you can move the duplicate pcspeaker.c code out of all
>> those arches and stash it somewhere in drivers/input. Presumably it's
>> possible to get it to link into the kernel even when INPUT is modular.
> 
> Here's the patch, after spending some time to get familiar with git.
> 
> The patch is against git x86/mm, and it seems to work fine on x86. However,
> on x86, you no longer have /sys/devices/platform/pcspkr after
> running "make allnoconfig". To get it, you have to explicitely add support
> to CONFIG_INPUT_PCSPKR (m or y). I hope this is fine.
> 
> I'm copying the MIPS maintainer as this patch touches his tree too.

This patch does not apply cleanly to linux-mips Git tree:

----> snip
patching file arch/x86/kernel/Makefile
Hunk #1 FAILED at 69.
1 out of 1 hunk FAILED -- saving rejects to file arch/x86/kernel/Makefile.rej
----> snip

I believe this is because the linux-mips tree and the -mm tree are not in sync.

> 
> On MIPS, there should be no impact though, as CONFIG_PCSPEAKER is set in
> defconfig files.
> 
> In other architectures where CONFIG_INPUT_PCSPKR can exist,
> there is a change: when CONFIG_INPUT_PCSPKR is set, the platform device
> will be added, while it didn't exist before. I hope this is fine.
> 

It seems tempting to include at least Alpha in this patch, because they
have a device initcall identical to the one, which you're removing from
the MIPS and x86 arch code.

Thanks,

Dmitri

Re: [PATCH] x86: fix unconditional arch/x86/kernel/pcspeaker.c?compiling

[Ralf Baechle]

On Thu, Jan 24, 2008 at 11:12:12PM +0300, Dmitri Vorobiev wrote:

> > running "make allnoconfig". To get it, you have to explicitely add support
> > to CONFIG_INPUT_PCSPKR (m or y). I hope this is fine.
> > 
> > I'm copying the MIPS maintainer as this patch touches his tree too.
> 
> This patch does not apply cleanly to linux-mips Git tree:
> 
> ----> snip
> patching file arch/x86/kernel/Makefile
> Hunk #1 FAILED at 69.
> 1 out of 1 hunk FAILED -- saving rejects to file arch/x86/kernel/Makefile.rej
> ----> snip
> 
> I believe this is because the linux-mips tree and the -mm tree are not in sync.

The main linux-mips git tree is derived of Linus' kernel.org tree and
I usually update it daily.  Patch scheduled for the next kernel release or
later go into a special tree from which akpm pulls.  For significant
patch submissions that means patches should be generated either against
the linux-queue tree from linux-mips.org (which only contains the MIPS
specific stuff) or possibly akpm's -mm tree.

  Ralf