From: Ben Hutchings <ben@decadent.org.uk>
To: Mauro Carvalho Chehab <mchehab@redhat.com>,
Greg Kroah-Hartman <gregkh@suse.de>
Cc: linux-media@vger.kernel.org, devel@driverdev.osuosl.org
Subject: [PATCH 1/5] staging: lirc_serial: Fix init/exit order
Date: Wed, 16 Nov 2011 05:49:41 +0000 [thread overview]
Message-ID: <1321422581.2885.50.camel@deadeye> (raw)
Currently the module init function registers a platform_device and
only then allocates its IRQ and I/O region. This allows allocation to
race with the device's suspend() function. Instead, allocate
resources in the platform driver's probe() function and free them in
the remove() function.
The module exit function removes the platform device before the
character device that provides access to it. Change it to reverse the
order of initialisation.
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
The down-side of this is that module insertion now succeeds even if the
device can't be probed. But that's how most driver modules work, and
there will be obvious error messages logged on failure.
Ben.
drivers/staging/media/lirc/lirc_serial.c | 56 +++++++++++------------------
1 files changed, 21 insertions(+), 35 deletions(-)
diff --git a/drivers/staging/media/lirc/lirc_serial.c b/drivers/staging/media/lirc/lirc_serial.c
index 8a060a8..8637631 100644
--- a/drivers/staging/media/lirc/lirc_serial.c
+++ b/drivers/staging/media/lirc/lirc_serial.c
@@ -836,7 +836,7 @@ static int hardware_init_port(void)
return 0;
}
-static int init_port(void)
+static int __devinit lirc_serial_probe(struct platform_device *dev)
{
int i, nlow, nhigh, result;
@@ -913,6 +913,18 @@ static int init_port(void)
return 0;
}
+static int __devexit lirc_serial_remove(struct platform_device *dev)
+{
+ free_irq(irq, (void *)&hardware);
+
+ if (iommap != 0)
+ release_mem_region(iommap, 8 << ioshift);
+ else
+ release_region(io, 8);
+
+ return 0;
+}
+
static int set_use_inc(void *data)
{
unsigned long flags;
@@ -1076,16 +1088,6 @@ static struct lirc_driver driver = {
static struct platform_device *lirc_serial_dev;
-static int __devinit lirc_serial_probe(struct platform_device *dev)
-{
- return 0;
-}
-
-static int __devexit lirc_serial_remove(struct platform_device *dev)
-{
- return 0;
-}
-
static int lirc_serial_suspend(struct platform_device *dev,
pm_message_t state)
{
@@ -1188,10 +1190,6 @@ static int __init lirc_serial_init_module(void)
{
int result;
- result = lirc_serial_init();
- if (result)
- return result;
-
switch (type) {
case LIRC_HOMEBREW:
case LIRC_IRDEO:
@@ -1211,8 +1209,7 @@ static int __init lirc_serial_init_module(void)
break;
#endif
default:
- result = -EINVAL;
- goto exit_serial_exit;
+ return -EINVAL;
}
if (!softcarrier) {
switch (type) {
@@ -1228,37 +1225,26 @@ static int __init lirc_serial_init_module(void)
}
}
- result = init_port();
- if (result < 0)
- goto exit_serial_exit;
+ result = lirc_serial_init();
+ if (result)
+ return result;
+
driver.features = hardware[type].features;
driver.dev = &lirc_serial_dev->dev;
driver.minor = lirc_register_driver(&driver);
if (driver.minor < 0) {
printk(KERN_ERR LIRC_DRIVER_NAME
": register_chrdev failed!\n");
- result = -EIO;
- goto exit_release;
+ lirc_serial_exit();
+ return -EIO;
}
return 0;
-exit_release:
- release_region(io, 8);
-exit_serial_exit:
- lirc_serial_exit();
- return result;
}
static void __exit lirc_serial_exit_module(void)
{
- lirc_serial_exit();
-
- free_irq(irq, (void *)&hardware);
-
- if (iommap != 0)
- release_mem_region(iommap, 8 << ioshift);
- else
- release_region(io, 8);
lirc_unregister_driver(driver.minor);
+ lirc_serial_exit();
dprintk("cleaned up module\n");
}
--
1.7.7.2
next reply other threads:[~2011-11-16 5:49 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-11-16 5:49 Ben Hutchings [this message]
2011-11-16 5:52 ` [PATCH 2/5] staging: lirc_serial: Free resources on failure paths of lirc_serial_probe() Ben Hutchings
2011-11-16 5:53 ` [PATCH 3/5] staging: lirc_serial: Fix deadlock on resume failure Ben Hutchings
2011-11-16 5:53 ` [PATCH 4/5] staging: lirc_serial: Fix bogus error codes Ben Hutchings
2011-11-16 5:54 ` [PATCH 5/5] staging: lirc_serial: Do not assume error codes returned by request_irq() Ben Hutchings
2012-03-02 3:45 ` [PATCH 1/5] staging: lirc_serial: Fix init/exit order Jonathan Nieder
2012-03-02 4:35 ` Ben Hutchings
2012-03-02 5:29 ` VDR User
2012-03-02 20:39 ` [PATCH 3.0.y 0/4] Re: lirc_serial spuriously claims assigned port and irq to be in use Jonathan Nieder
2012-03-02 20:40 ` [PATCH 1/4] [media] staging: lirc_serial: Fix init/exit order Jonathan Nieder
2012-03-02 20:40 ` [PATCH 2/4] [media] staging: lirc_serial: Free resources on failure paths of lirc_serial_probe() Jonathan Nieder
2012-03-02 20:41 ` [PATCH 3/4] [media] staging: lirc_serial: Fix deadlock on resume failure Jonathan Nieder
2012-03-02 20:41 ` [PATCH 4/4] [media] staging: lirc_serial: Do not assume error codes returned by request_irq() Jonathan Nieder
2012-03-02 21:13 ` [PATCH 3.0.y 0/4] Re: lirc_serial spuriously claims assigned port and irq to be in use Greg Kroah-Hartman
2012-03-03 1:13 ` Jonathan Nieder
2012-03-07 20:04 ` Greg Kroah-Hartman
2012-03-07 20:17 ` Jonathan Nieder
2012-03-08 18:34 ` Greg Kroah-Hartman
2012-03-07 21:34 ` Ben Hutchings
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1321422581.2885.50.camel@deadeye \
--to=ben@decadent.org.uk \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@suse.de \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox