Re: [patch] pcspkr: fix dependancies

Previous thread: [PATCH] xfs: reduce stack usage in xfs_bmap_btalloc() by Denys Vlasenko on Saturday, April 26, 2008 - 7:51 am. (8 messages)

Next thread: [PATCH] [ASYNC_TX] don't include <asm/> headers by Sebastian Siewior on Saturday, April 26, 2008 - 8:48 am. (5 messages)
From: Stas Sergeev
Date: Saturday, April 26, 2008 - 8:16 am

Hello.

linux-2.6.25 has the strange dependancies
for pc-speaker, which prevents snd-pcsp
from working.
Namely, the platform device (created in
arch/x86/kernel/pcspeaker.c) depends on
the platform driver (CONFIG_INPUT_PCSPKR).
I wonder if it is a good practice to make
the platform device to depend on the driver,
I guess it is not.

The attached patch reverses that.
It adds the config option for the pcspkr
platform device and makes the platform
drivers to depend on that.
This allows snd-pcsp to work at least in
some configurations.

Does that patch look reasonable?

---
fix pcspkr dependancies: make the pcspkr platform
drivers to depend on a platform device, and
not the other way around.

Signed-off-by: Stas Sergeev &lt;stsp@aknet.ru&gt;
CC: Takashi Iwai &lt;tiwai@suse.de&gt;
CC: Dmitry Torokhov &lt;dmitry.torokhov@gmail.com&gt;
CC: Vojtech Pavlik &lt;vojtech@suse.cz&gt;
From: Dmitry Torokhov
Date: Saturday, April 26, 2008 - 4:18 pm

Hi Stas,


No, not another config option please. Either make pcspkr platform
device be created unconditionally or if either pcspkr or snd-pcsp
is selected. 

-- 
Dmitry
--

From: Stas Sergeev
Date: Saturday, April 26, 2008 - 4:39 pm

Hello.

I also think this would be the best
fix. But this will return us to the
state of 2.6.24. Presumably the dependancy
was added for the reason, even if I
fail to see one.
I don't think it is correct, but if
people think it is, then this can be
done.


---
Compile pcspkr platform device unconditionally.
That allows snd-pcsp to work.

Signed-off-by: Stas Sergeev &lt;stsp@aknet.ru&gt;
From: Dmitry Torokhov
Date: Saturday, April 26, 2008 - 9:23 pm

It looks like the change was introduced by commit

67926892ef7a7fbc76de607120d44416019fdf07

I think that we should register devices even if there is no driver in
the kernel for it because driver may be compiled at later time or be out
of tree. Adding patch author and other people that signed off the patch


-- 
Dmitry
--

From: Stas Sergeev
Date: Monday, April 28, 2008 - 12:34 pm

Hello.

Indeed. But the log message
http://www.mail-archive.com/git-commits-head@vger.kernel.org/msg36026.html
doesn't seem to specify the reason
for the change, so this doesn't
Since there seem to be no objections,
would you mind adding the patch to your
git tree? Or who should that be?
--

From: Dmitry Torokhov
Date: Monday, April 28, 2008 - 12:58 pm

I try to stay within drivers/input boundaries ;) Ingo I think is the
person you need.

-- 
Dmitry
--

From: Sam Ravnborg
Date: Monday, April 28, 2008 - 1:12 pm

If you google you will find a long thread about this patch.
It is all about saving memory for embedded platforms.
No need to have the driver if there is no speaker on the board.

Ask Michael (author) if you need more info.

	Sam
--

From: Dmitry Torokhov
Date: Monday, April 28, 2008 - 1:19 pm

The patch does it backwards though - it disables platform device if
there is no driver. We don't stop enumerating PCI devices if some of
them don't have a driver for them, do we? The arch code should not
create the device if it knows that the boards does not have it.

-- 
Dmitry
--

From: Stas Sergeev
Date: Tuesday, April 29, 2008 - 12:56 pm

Hello.

Yes, indeed, thankyou.
OK. This wasn't clear from the log
message for some reason, but it is
Very probably, and for that case I
made the patch like this:
Reading the thread, it actually seems
like Michael wanted to submit the patch
much like the aforementioned one of
mine:
http://kerneltrap.org/mailarchive/linux-kernel/2008/1/18/580583
but haven't done so in time, and as the
result, the wrong one stuck in.
But this is never too late to correct, I
hope.

So I see 2 options: either revert the
patch completely, or revert the dependancies
like Michael suggested initially (if I
understand his suggestion right).
Both patches are here, in this thread.
Thoughts?
--

From: Michael Opdenacker
Date: Wednesday, April 30, 2008 - 12:31 am

Stas, Dmitry,

I agree with you... on a regular system, all the platform devices should
be enumerated, even if we don't use their drivers. It's only for use in
embedded devices (CONFIG_EMBEDDED) that we could omit this enumeration
to reduce kernel size.

Would you post a patch doing this?


Michael.

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

--

From: Stas Sergeev
Date: Wednesday, April 30, 2008 - 4:23 am

Hello.

Certainly, this was done.
http://uwsg.ucs.indiana.edu/hypermail/linux/kernel/0804.3/1189.html
Does this help?
--

From: Stas Sergeev
Date: Thursday, May 1, 2008 - 12:08 am

Guys, there really needs to be a
few more replies to make a decision.

So far only Dmitry have clearly stated
his opinion regarding both patches.
And that opinion was: no need for the
new config option, just get the 2.6.25
patch reverted.
Dmitry, after reading the older Michael's
thread, would you perhaps reconsider?

Sam and Michael said that just revering
may not be a good idea, but have not
commented on the patch that adds a new
option under &quot;if EMBEDDED&quot;.

That puts me in a position where both
patches are effectively blocked, and
in a mean time snd-pcsp remains broken.

Guys, please, give your ACK or NACK on
the two posted patches! :)
Namely:
http://lkml.org/lkml/2008/4/26/105
http://lkml.org/lkml/2008/4/26/283
--

Previous thread: [PATCH] xfs: reduce stack usage in xfs_bmap_btalloc() by Denys Vlasenko on Saturday, April 26, 2008 - 7:51 am. (8 messages)

Next thread: [PATCH] [ASYNC_TX] don't include <asm/> headers by Sebastian Siewior on Saturday, April 26, 2008 - 8:48 am. (5 messages)