[PATCH] Convert dmasound_awacs to dynamic input_dev allocation

[PATCH] Convert dmasound_awacs to dynamic input_dev allocation

[Ian Wienand]

[-- Attachment #1: Type: text/plain, Size: 1626 bytes --]

Hi,

This patch converts sound/oss/dmasound/dmasound_awacs.c to use dynamic
input_dev allocation, stopping an oops on boot with the latest
kernels.

Thanks,

-i

Signed-off-by: Ian Wienand <ianw@gelato.unsw.edu.au>

---
diff --git a/sound/oss/dmasound/dmasound_awacs.c b/sound/oss/dmasound/dmasound_awacs.c
--- a/sound/oss/dmasound/dmasound_awacs.c
+++ b/sound/oss/dmasound/dmasound_awacs.c
@@ -2805,16 +2805,7 @@ __init setup_beep(void)
 	return 0 ;
 }
 
-static struct input_dev awacs_beep_dev = {
-	.evbit		= { BIT(EV_SND) },
-	.sndbit		= { BIT(SND_BELL) | BIT(SND_TONE) },
-	.event		= awacs_beep_event,
-	.name		= "dmasound beeper",
-	.phys		= "macio/input0", /* what the heck is this?? */
-	.id		= {
-		.bustype	= BUS_HOST,
-	},
-};
+static struct input_dev *awacs_beep_dev;
 
 int __init dmasound_awacs_init(void)
 {
@@ -3140,14 +3131,22 @@ printk("dmasound_pmac: Awacs/Screamer Co
 	 * XXX: we should handle errors here, but that would mean
 	 * rewriting the whole init code.  later..
 	 */
-	input_register_device(&awacs_beep_dev);
+	awacs_beep_dev = input_allocate_device();
+	awacs_beep_dev->name = "dmasound beeper";
+	awacs_beep_dev->phys = "macio/input0";
+	awacs_beep_dev->id.bustype = BUS_HOST;
+	awacs_beep_dev->event = awacs_beep_event;
+	awacs_beep_dev->sndbit[0] = BIT(SND_BELL) | BIT(SND_TONE);
+	awacs_beep_dev->evbit[0] = BIT(EV_SND);
+
+	input_register_device(awacs_beep_dev);
 
 	return dmasound_init();
 }
 
 static void __exit dmasound_awacs_cleanup(void)
 {
-	input_unregister_device(&awacs_beep_dev);
+	input_unregister_device(awacs_beep_dev);
 
 	switch (awacs_revision) {
 		case AWACS_TUMBLER:

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Convert dmasound_awacs to dynamic input_dev allocation

[Dmitry Torokhov]

On Monday 31 October 2005 21:03, Ian Wienand wrote:
> Hi,
> 
> This patch converts sound/oss/dmasound/dmasound_awacs.c to use dynamic
> input_dev allocation, stopping an oops on boot with the latest
> kernels.
>

Oh, I see that the piece of code that bitches about device not being
dynamically allocated and bails out was lost on its way to Linus...
Will resend shortly...
 
> +	awacs_beep_dev = input_allocate_device();

We really need to check whether device was allocated here...

-- 
Dmitry

Re: [PATCH] Convert dmasound_awacs to dynamic input_dev allocation

[Ian Wienand]

On Mon, Oct 31, 2005 at 10:17:08PM -0500, Dmitry Torokhov wrote:
> > +	awacs_beep_dev = input_allocate_device();
> 
> We really need to check whether device was allocated here...

AFAICS there is no easy way to bail without leaking from that point
(the comment already there suggests as much).  Would it be appropriate
to just BUG() out in that case and save somebody auditing and
re-architecturing for an unlikely error in a deprecated interface?

-i

Signed-off-by: Ian Wienand <ianw@gelato.unsw.edu.au>

---
diff --git a/sound/oss/dmasound/dmasound_awacs.c b/sound/oss/dmasound/dmasound_awacs.c
--- a/sound/oss/dmasound/dmasound_awacs.c
+++ b/sound/oss/dmasound/dmasound_awacs.c
@@ -2805,16 +2805,7 @@ __init setup_beep(void)
 	return 0 ;
 }
 
-static struct input_dev awacs_beep_dev = {
-	.evbit		= { BIT(EV_SND) },
-	.sndbit		= { BIT(SND_BELL) | BIT(SND_TONE) },
-	.event		= awacs_beep_event,
-	.name		= "dmasound beeper",
-	.phys		= "macio/input0", /* what the heck is this?? */
-	.id		= {
-		.bustype	= BUS_HOST,
-	},
-};
+static struct input_dev *awacs_beep_dev;
 
 int __init dmasound_awacs_init(void)
 {
@@ -3136,18 +3127,32 @@ printk("dmasound_pmac: Awacs/Screamer Co
 			break ;
 	}
 
+	awacs_beep_dev = input_allocate_device();
 	/*
 	 * XXX: we should handle errors here, but that would mean
 	 * rewriting the whole init code.  later..
 	 */
-	input_register_device(&awacs_beep_dev);
+	if (awacs_beep_dev == NULL)
+	{
+		printk(KERN_ERR "%s() input_allocate_device failed\n", __FUNCTION__);
+		BUG();
+	}
+
+	awacs_beep_dev->name = "dmasound beeper";
+	awacs_beep_dev->phys = "macio/input0";
+	awacs_beep_dev->id.bustype = BUS_HOST;
+	awacs_beep_dev->event = awacs_beep_event;
+	awacs_beep_dev->sndbit[0] = BIT(SND_BELL) | BIT(SND_TONE);
+	awacs_beep_dev->evbit[0] = BIT(EV_SND);
+
+	input_register_device(awacs_beep_dev);
 
 	return dmasound_init();
 }
 
 static void __exit dmasound_awacs_cleanup(void)
 {
-	input_unregister_device(&awacs_beep_dev);
+	input_unregister_device(awacs_beep_dev);
 
 	switch (awacs_revision) {
 		case AWACS_TUMBLER:

Re: [PATCH] Convert dmasound_awacs to dynamic input_dev allocation

[Dmitry Torokhov]

On Monday 31 October 2005 22:59, Ian Wienand wrote:
> On Mon, Oct 31, 2005 at 10:17:08PM -0500, Dmitry Torokhov wrote:
> > > +	awacs_beep_dev = input_allocate_device();
> > 
> > We really need to check whether device was allocated here...
> 
> AFAICS there is no easy way to bail without leaking from that point
> (the comment already there suggests as much).  Would it be appropriate
> to just BUG() out in that case and save somebody auditing and
> re-architecturing for an unlikely error in a deprecated interface?

It seems that the change is pretty straightforward... Could you please try
this one?
 
-- 
Dmitry

From: Ian Wienand <ianw@gelato.unsw.edu.au>

Input: convert dmasound_awacs (OSS) to dynamic input allocation

Signed-off-by: Ian Wienand <ianw@gelato.unsw.edu.au>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 sound/oss/dmasound/dmasound_awacs.c |   31 +++++++++++++++++++------------
 1 files changed, 19 insertions(+), 12 deletions(-)

Index: work/sound/oss/dmasound/dmasound_awacs.c
===================================================================
--- work.orig/sound/oss/dmasound/dmasound_awacs.c
+++ work/sound/oss/dmasound/dmasound_awacs.c
@@ -2805,16 +2805,7 @@ __init setup_beep(void)
 	return 0 ;
 }
 
-static struct input_dev awacs_beep_dev = {
-	.evbit		= { BIT(EV_SND) },
-	.sndbit		= { BIT(SND_BELL) | BIT(SND_TONE) },
-	.event		= awacs_beep_event,
-	.name		= "dmasound beeper",
-	.phys		= "macio/input0", /* what the heck is this?? */
-	.id		= {
-		.bustype	= BUS_HOST,
-	},
-};
+static struct input_dev *awacs_beep_dev;
 
 int __init dmasound_awacs_init(void)
 {
@@ -2907,6 +2898,22 @@ printk("dmasound_pmac: couldn't find a C
 		return -ENODEV;
 	}
 
+	awacs_beep_dev = input_allocate_device();
+	if (!awacs_beep_dev) {
+		release_OF_resource(io, 0);
+		release_OF_resource(io, 1);
+		release_OF_resource(io, 2);
+		printk(KERN_ERR "dmasound: can't allocate input device !\n");
+		return -ENOMEM;
+	}
+
+	awacs_beep_dev->name = "dmasound beeper";
+	awacs_beep_dev->phys = "macio/input0";
+	awacs_beep_dev->id.bustype = BUS_HOST;
+	awacs_beep_dev->event = awacs_beep_event;
+	awacs_beep_dev->sndbit[0] = BIT(SND_BELL) | BIT(SND_TONE);
+	awacs_beep_dev->evbit[0] = BIT(EV_SND);
+
 	/* all OF versions I've seen use this value */
 	if (i2s_node)
 		i2s = ioremap(io->addrs[0].address, 0x1000);
@@ -3140,14 +3147,14 @@ printk("dmasound_pmac: Awacs/Screamer Co
 	 * XXX: we should handle errors here, but that would mean
 	 * rewriting the whole init code.  later..
 	 */
-	input_register_device(&awacs_beep_dev);
+	input_register_device(awacs_beep_dev);
 
 	return dmasound_init();
 }
 
 static void __exit dmasound_awacs_cleanup(void)
 {
-	input_unregister_device(&awacs_beep_dev);
+	input_unregister_device(awacs_beep_dev);
 
 	switch (awacs_revision) {
 		case AWACS_TUMBLER:

Re: [PATCH] Convert dmasound_awacs to dynamic input_dev allocation

[Ian Wienand]

On Tue, Nov 01, 2005 at 12:14:56AM -0500, Dmitry Torokhov wrote:
> It seems that the change is pretty straightforward... Could you please try
> this one?

Ahh, ok!  I thought that that comment meant it was deliberately
ignoring the return of input_register_device (by which time it's got
memory, irq's, io, etc), but now I see it's void anyway.  So why not
just move the setup right to the top?

-i

Signed-off-by: Ian Wienand <ianw@gelato.unsw.edu.au>
---
diff --git a/sound/oss/dmasound/dmasound_awacs.c b/sound/oss/dmasound/dmasound_awacs.c
--- a/sound/oss/dmasound/dmasound_awacs.c
+++ b/sound/oss/dmasound/dmasound_awacs.c
@@ -2805,16 +2805,7 @@ __init setup_beep(void)
 	return 0 ;
 }
 
-static struct input_dev awacs_beep_dev = {
-	.evbit		= { BIT(EV_SND) },
-	.sndbit		= { BIT(SND_BELL) | BIT(SND_TONE) },
-	.event		= awacs_beep_event,
-	.name		= "dmasound beeper",
-	.phys		= "macio/input0", /* what the heck is this?? */
-	.id		= {
-		.bustype	= BUS_HOST,
-	},
-};
+static struct input_dev *awacs_beep_dev;
 
 int __init dmasound_awacs_init(void)
 {
@@ -2828,6 +2819,20 @@ int __init dmasound_awacs_init(void)
 	awacs_revision = 0;
 	hw_can_byteswap = 1 ; /* most can */
 
+	/* setup the beep input event */
+	awacs_beep_dev = input_allocate_device();
+	if (!awacs_beep_dev)
+	{
+		printk(KERN_ERR "dmasound: can't allocate input device!\n");
+		return -ENOMEM;
+	}
+	awacs_beep_dev->name = "dmasound beeper";
+	awacs_beep_dev->phys = "macio/input0";
+	awacs_beep_dev->id.bustype = BUS_HOST;
+	awacs_beep_dev->event = awacs_beep_event;
+	awacs_beep_dev->sndbit[0] = BIT(SND_BELL) | BIT(SND_TONE);
+	awacs_beep_dev->evbit[0] = BIT(EV_SND);
+
 	/* look for models we need to handle specially */
 	set_model() ;
 
@@ -3136,18 +3141,14 @@ printk("dmasound_pmac: Awacs/Screamer Co
 			break ;
 	}
 
-	/*
-	 * XXX: we should handle errors here, but that would mean
-	 * rewriting the whole init code.  later..
-	 */
-	input_register_device(&awacs_beep_dev);
+	input_register_device(awacs_beep_dev);
 
 	return dmasound_init();
 }
 
 static void __exit dmasound_awacs_cleanup(void)
 {
-	input_unregister_device(&awacs_beep_dev);
+	input_unregister_device(awacs_beep_dev);
 
 	switch (awacs_revision) {
 		case AWACS_TUMBLER:

Re: [PATCH] Convert dmasound_awacs to dynamic input_dev allocation

[Dmitry Torokhov]

On Tuesday 01 November 2005 00:50, Ian Wienand wrote:
> On Tue, Nov 01, 2005 at 12:14:56AM -0500, Dmitry Torokhov wrote:
> > It seems that the change is pretty straightforward... Could you please try
> > this one?
> 
> Ahh, ok!  I thought that that comment meant it was deliberately
> ignoring the return of input_register_device (by which time it's got
> memory, irq's, io, etc), but now I see it's void anyway.  So why not
> just move the setup right to the top?
>

Now you are leaking memory if something else fails... FOr example when
chip is not present.


-- 
Dmitry

Re: [PATCH] Convert dmasound_awacs to dynamic input_dev allocation

[Ian Wienand]

[-- Attachment #1: Type: text/plain, Size: 397 bytes --]

On Tue, Nov 01, 2005 at 12:55:31AM -0500, Dmitry Torokhov wrote:
> Now you are leaking memory if something else fails... FOr example when
> chip is not present.

Good point.  I guess the original comment is because the final
dmasound_init() can fail but we'll still have all sorts of memory,
irq's and io that aren't cleaned up.  So your previous patch probably
introduces the least problems.

-i

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Convert dmasound_awacs to dynamic input_dev allocation

[Dmitry Torokhov]

On Tuesday 01 November 2005 01:04, Ian Wienand wrote:
> On Tue, Nov 01, 2005 at 12:55:31AM -0500, Dmitry Torokhov wrote:
> > Now you are leaking memory if something else fails... FOr example when
> > chip is not present.
> 
> Good point.  I guess the original comment is because the final
> dmasound_init() can fail but we'll still have all sorts of memory,
> irq's and io that aren't cleaned up.  So your previous patch probably
> introduces the least problems.
>

Have you tried it by any chance? I'd feel much better pushing it upstream
knowing that it was tested at least once...

-- 
Dmitry

Re: [PATCH] Convert dmasound_awacs to dynamic input_dev allocation

[Ian Wienand]

[-- Attachment #1: Type: text/plain, Size: 442 bytes --]

On Tue, Nov 01, 2005 at 01:14:38AM -0500, Dmitry Torokhov wrote:
> Have you tried it by any chance? I'd feel much better pushing it upstream
> knowing that it was tested at least once...

Yes, works fine on my iBook, thanks.

Does anyone know what the eqivalent of 'cvs update -C file' is with
git?  It is a common use-case for me to make a few changes, send off a
diff and then get another one back which I want to apply to the
orignal.

-i

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Convert dmasound_awacs to dynamic input_dev allocation

[Pekka Enberg]

On 11/1/05, Ian Wienand <ianw@gelato.unsw.edu.au> wrote:
> Does anyone know what the eqivalent of 'cvs update -C file' is with
> git?  It is a common use-case for me to make a few changes, send off a
> diff and then get another one back which I want to apply to the
> orignal.

Sounds you're looking for git checkout -f ?

                        Pekka