[PATCH 1/5] staging: lirc_serial: Fix init/exit order

[PATCH 1/5] staging: lirc_serial: Fix init/exit order

[Ben Hutchings]

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

[PATCH 2/5] staging: lirc_serial: Free resources on failure paths of lirc_serial_probe()

[Ben Hutchings]

Failure to allocate the I/O region leaves the IRQ allocated.
A later failure leaves them both allocated.

Reported-by: Torsten Crass <torsten.crass@eBiology.de>
References: http://bugs.debian.org/645811
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/staging/media/lirc/lirc_serial.c |   19 ++++++++++++++++---
 1 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_serial.c b/drivers/staging/media/lirc/lirc_serial.c
index 8637631..d833772 100644
--- a/drivers/staging/media/lirc/lirc_serial.c
+++ b/drivers/staging/media/lirc/lirc_serial.c
@@ -875,11 +875,14 @@ static int __devinit lirc_serial_probe(struct platform_device *dev)
 		       ": or compile the serial port driver as module and\n");
 		printk(KERN_WARNING LIRC_DRIVER_NAME
 		       ": make sure this module is loaded first\n");
-		return -EBUSY;
+		result = -EBUSY;
+		goto exit_free_irq;
 	}
 
-	if (hardware_init_port() < 0)
-		return -EINVAL;
+	if (hardware_init_port() < 0) {
+		result = -EINVAL;
+		goto exit_release_region;
+	}
 
 	/* Initialize pulse/space widths */
 	init_timing_params(duty_cycle, freq);
@@ -911,6 +914,16 @@ static int __devinit lirc_serial_probe(struct platform_device *dev)
 
 	dprintk("Interrupt %d, port %04x obtained\n", irq, io);
 	return 0;
+
+exit_release_region:
+	if (iommap != 0)
+		release_mem_region(iommap, 8 << ioshift);
+	else
+		release_region(io, 8);
+exit_free_irq:
+	free_irq(irq, (void *)&hardware);
+
+	return result;
 }
 
 static int __devexit lirc_serial_remove(struct platform_device *dev)
-- 
1.7.7.2

[PATCH 3/5] staging: lirc_serial: Fix deadlock on resume failure

[Ben Hutchings]

A resume function cannot remove the device it is resuming!

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
I haven't seen any report of this deadlock, but it seems pretty obvious.

Ben.

 drivers/staging/media/lirc/lirc_serial.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_serial.c b/drivers/staging/media/lirc/lirc_serial.c
index d833772..befe626 100644
--- a/drivers/staging/media/lirc/lirc_serial.c
+++ b/drivers/staging/media/lirc/lirc_serial.c
@@ -1127,10 +1127,8 @@ static int lirc_serial_resume(struct platform_device *dev)
 {
 	unsigned long flags;
 
-	if (hardware_init_port() < 0) {
-		lirc_serial_exit();
+	if (hardware_init_port() < 0)
 		return -EINVAL;
-	}
 
 	spin_lock_irqsave(&hardware[type].lock, flags);
 	/* Enable Interrupt */
-- 
1.7.7.2

[PATCH 4/5] staging: lirc_serial: Fix bogus error codes

[Ben Hutchings]

Device not found?  ENODEV, not EINVAL.
Write to read-only device?  EPERM, not EBADF.
Invalid argument?  EINVAL, not ENOSYS.
Unsupported ioctl?  ENOIOCTLCMD, not ENOSYS.
Another function returned an error code?  Use that, don't replace it.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/staging/media/lirc/lirc_serial.c |   23 ++++++++++++-----------
 1 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_serial.c b/drivers/staging/media/lirc/lirc_serial.c
index befe626..6f5257e 100644
--- a/drivers/staging/media/lirc/lirc_serial.c
+++ b/drivers/staging/media/lirc/lirc_serial.c
@@ -773,7 +773,7 @@ static int hardware_init_port(void)
 		/* we fail, there's nothing here */
 		printk(KERN_ERR LIRC_DRIVER_NAME ": port existence test "
 		       "failed, cannot continue\n");
-		return -EINVAL;
+		return -ENODEV;
 	}
 

@@ -879,10 +879,9 @@ static int __devinit lirc_serial_probe(struct platform_device *dev)
 		goto exit_free_irq;
 	}
 
-	if (hardware_init_port() < 0) {
-		result = -EINVAL;
+	result = hardware_init_port();
+	if (result < 0)
 		goto exit_release_region;
-	}
 
 	/* Initialize pulse/space widths */
 	init_timing_params(duty_cycle, freq);
@@ -980,7 +979,7 @@ static ssize_t lirc_write(struct file *file, const char *buf,
 	int *wbuf;
 
 	if (!(hardware[type].features & LIRC_CAN_SEND_PULSE))
-		return -EBADF;
+		return -EPERM;
 
 	count = n / sizeof(int);
 	if (n % sizeof(int) || count % 2 == 0)
@@ -1031,11 +1030,11 @@ static long lirc_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
 			return result;
 		/* only LIRC_MODE_PULSE supported */
 		if (value != LIRC_MODE_PULSE)
-			return -ENOSYS;
+			return -EINVAL;
 		break;
 
 	case LIRC_GET_LENGTH:
-		return -ENOSYS;
+		return -ENOIOCTLCMD;
 		break;
 
 	case LIRC_SET_SEND_DUTY_CYCLE:
@@ -1126,9 +1125,11 @@ static void lirc_serial_exit(void);
 static int lirc_serial_resume(struct platform_device *dev)
 {
 	unsigned long flags;
+	int result;
 
-	if (hardware_init_port() < 0)
-		return -EINVAL;
+	result = hardware_init_port();
+	if (result < 0)
+		return result;
 
 	spin_lock_irqsave(&hardware[type].lock, flags);
 	/* Enable Interrupt */
@@ -1161,7 +1162,7 @@ static int __init lirc_serial_init(void)
 	/* Init read buffer. */
 	result = lirc_buffer_init(&rbuf, sizeof(int), RBUF_LEN);
 	if (result < 0)
-		return -ENOMEM;
+		return result;
 
 	result = platform_driver_register(&lirc_serial_driver);
 	if (result) {
@@ -1247,7 +1248,7 @@ static int __init lirc_serial_init_module(void)
 		printk(KERN_ERR  LIRC_DRIVER_NAME
 		       ": register_chrdev failed!\n");
 		lirc_serial_exit();
-		return -EIO;
+		return driver.minor;
 	}
 	return 0;
 }
-- 
1.7.7.2

[PATCH 5/5] staging: lirc_serial: Do not assume error codes returned by request_irq()

[Ben Hutchings]

lirc_serial_probe() must fail if request_irq() returns an error, even if
it isn't EBUSY or EINVAL,

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/staging/media/lirc/lirc_serial.c |   21 +++++++++------------
 1 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_serial.c b/drivers/staging/media/lirc/lirc_serial.c
index 6f5257e..0ca308a 100644
--- a/drivers/staging/media/lirc/lirc_serial.c
+++ b/drivers/staging/media/lirc/lirc_serial.c
@@ -843,18 +843,15 @@ static int __devinit lirc_serial_probe(struct platform_device *dev)
 	result = request_irq(irq, irq_handler,
 			     (share_irq ? IRQF_SHARED : 0),
 			     LIRC_DRIVER_NAME, (void *)&hardware);
-
-	switch (result) {
-	case -EBUSY:
-		printk(KERN_ERR LIRC_DRIVER_NAME ": IRQ %d busy\n", irq);
-		return -EBUSY;
-	case -EINVAL:
-		printk(KERN_ERR LIRC_DRIVER_NAME
-		       ": Bad irq number or handler\n");
-		return -EINVAL;
-	default:
-		break;
-	};
+	if (result < 0) {
+		if (result == -EBUSY)
+			printk(KERN_ERR LIRC_DRIVER_NAME ": IRQ %d busy\n",
+			       irq);
+		else if (result == -EINVAL)
+			printk(KERN_ERR LIRC_DRIVER_NAME
+			       ": Bad irq number or handler\n");
+		return result;
+	}
 
 	/* Reserve io region. */
 	/*
-- 
1.7.7.2

Re: [PATCH 1/5] staging: lirc_serial: Fix init/exit order

[Jonathan Nieder]

Hi Ben,

Ben Hutchings wrote[1]:

> 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.

>From <http://bugs.debian.org/645811> I see that you tested these patches:

 affc9a0d59ac [media] staging: lirc_serial: Do not assume error codes
              returned by request_irq()
 9b98d6067971 [media] staging: lirc_serial: Fix bogus error codes
 1ff1d88e8629 [media] staging: lirc_serial: Fix deadlock on resume failure
 c8e57e1b766c [media] staging: lirc_serial: Free resources on failure
              paths of lirc_serial_probe()
 9105b8b20041 [media] staging: lirc_serial: Fix init/exit order

in a VM.  They were applied in 3.3-rc1 and have been in the Debian
kernel since 3.1.4-1 at the end of November.

Would some of these patches (e.g., at least patches 1, 2, and 5) be
appropriate for inclusion in the 3.0.y and 3.2.y stable kernels from
kernel.org?

Thanks,
Jonathan

[1] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/40486

Re: [PATCH 1/5] staging: lirc_serial: Fix init/exit order

[Ben Hutchings]

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

On Thu, 2012-03-01 at 21:45 -0600, Jonathan Nieder wrote:
[...]
> From <http://bugs.debian.org/645811> I see that you tested these patches:
> 
>  affc9a0d59ac [media] staging: lirc_serial: Do not assume error codes
>               returned by request_irq()
>  9b98d6067971 [media] staging: lirc_serial: Fix bogus error codes
>  1ff1d88e8629 [media] staging: lirc_serial: Fix deadlock on resume failure
>  c8e57e1b766c [media] staging: lirc_serial: Free resources on failure
>               paths of lirc_serial_probe()
>  9105b8b20041 [media] staging: lirc_serial: Fix init/exit order
> 
> in a VM.  They were applied in 3.3-rc1 and have been in the Debian
> kernel since 3.1.4-1 at the end of November.
> 
> Would some of these patches (e.g., at least patches 1, 2, and 5) be
> appropriate for inclusion in the 3.0.y and 3.2.y stable kernels from
> kernel.org?

Assuming they haven't caused any regressions, I think everything except
9b98d6067971 (4/5) would be appropriate.

Ben.

-- 
Ben Hutchings
One of the nice things about standards is that there are so many of them.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 1/5] staging: lirc_serial: Fix init/exit order

[VDR User]

I have a few questions about lirc_serial...  It seems that it's now a
part of v4l and currently residing in the driver staging area. I was
told it will not move from staging until it has been converted to
rc_core. I was also told there are no plans for anyone to do this so
it would seem lirc_serial will be stuck indefinitely in staging..

My questions are:

Rather than keep working on a driver that apparently won't be moved
out of staging, doesn't it make more sense to just convert the thing
to rc_core so it can be properly accepted, and then continue work from
there?

Is anyone, or will anyone consider converting the driver to rc_core?

Thanks

[PATCH 3.0.y 0/4] Re: lirc_serial spuriously claims assigned port and irq to be in use

[Jonathan Nieder]

Ben Hutchings wrote:
> On Thu, 2012-03-01 at 21:45 -0600, Jonathan Nieder wrote:

>> Would some of these patches (e.g., at least patches 1, 2, and 5) be
>> appropriate for inclusion in the 3.0.y and 3.2.y stable kernels from
>> kernel.org?
>
> Assuming they haven't caused any regressions, I think everything except
> 9b98d6067971 (4/5) would be appropriate.

Great.  Here are the aforementioned patches rebased against 3.0.y, in
the hope that some interested person can confirm they still work.  The
only backporting needed was to adjust to the lack of
drivers/staging/lirc -> drivers/staging/media/lirc renaming.

Ben Hutchings (4):
  [media] staging: lirc_serial: Fix init/exit order
  [media] staging: lirc_serial: Free resources on failure paths of
	lirc_serial_probe()
  [media] staging: lirc_serial: Fix deadlock on resume failure
  [media] staging: lirc_serial: Do not assume error codes returned by
	request_irq()

 drivers/staging/lirc/lirc_serial.c |  100 +++++++++++++++++-------------------
 1 file changed, 47 insertions(+), 53 deletions(-)

[PATCH 1/4] [media] staging: lirc_serial: Fix init/exit order

[Jonathan Nieder]

From: Ben Hutchings <ben@decadent.org.uk>
Date: Wed, 16 Nov 2011 01:49:41 -0300

commit 9105b8b200410383d0854bbe237ee385d7d33ba6 upstream.

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>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/staging/lirc/lirc_serial.c |   56 ++++++++++++++----------------------
 1 file changed, 21 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/lirc/lirc_serial.c b/drivers/staging/lirc/lirc_serial.c
index 805df913bb6e..c8a4b7bc5879 100644
--- a/drivers/staging/lirc/lirc_serial.c
+++ b/drivers/staging/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.9.2

[PATCH 2/4] [media] staging: lirc_serial: Free resources on failure paths of lirc_serial_probe()

[Jonathan Nieder]

From: Ben Hutchings <ben@decadent.org.uk>
Date: Wed, 16 Nov 2011 01:52:11 -0300

commit c8e57e1b766c2321aa76ee5e6878c69bd2313d62 upstream.

Failure to allocate the I/O region leaves the IRQ allocated.
A later failure leaves them both allocated.

Reported-by: Torsten Crass <torsten.crass@eBiology.de>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/staging/lirc/lirc_serial.c |   19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/lirc/lirc_serial.c b/drivers/staging/lirc/lirc_serial.c
index c8a4b7bc5879..fa023da6bdaa 100644
--- a/drivers/staging/lirc/lirc_serial.c
+++ b/drivers/staging/lirc/lirc_serial.c
@@ -875,11 +875,14 @@ static int __devinit lirc_serial_probe(struct platform_device *dev)
 		       ": or compile the serial port driver as module and\n");
 		printk(KERN_WARNING LIRC_DRIVER_NAME
 		       ": make sure this module is loaded first\n");
-		return -EBUSY;
+		result = -EBUSY;
+		goto exit_free_irq;
 	}
 
-	if (hardware_init_port() < 0)
-		return -EINVAL;
+	if (hardware_init_port() < 0) {
+		result = -EINVAL;
+		goto exit_release_region;
+	}
 
 	/* Initialize pulse/space widths */
 	init_timing_params(duty_cycle, freq);
@@ -911,6 +914,16 @@ static int __devinit lirc_serial_probe(struct platform_device *dev)
 
 	dprintk("Interrupt %d, port %04x obtained\n", irq, io);
 	return 0;
+
+exit_release_region:
+	if (iommap != 0)
+		release_mem_region(iommap, 8 << ioshift);
+	else
+		release_region(io, 8);
+exit_free_irq:
+	free_irq(irq, (void *)&hardware);
+
+	return result;
 }
 
 static int __devexit lirc_serial_remove(struct platform_device *dev)
-- 
1.7.9.2

[PATCH 3/4] [media] staging: lirc_serial: Fix deadlock on resume failure

[Jonathan Nieder]

From: Ben Hutchings <ben@decadent.org.uk>
Date: Wed, 16 Nov 2011 01:53:25 -0300

commit 1ff1d88e862948ae5bfe490248c023ff8ac2855d upstream.

A resume function cannot remove the device it is resuming!

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/staging/lirc/lirc_serial.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/lirc/lirc_serial.c b/drivers/staging/lirc/lirc_serial.c
index fa023da6bdaa..4b8fefb954d3 100644
--- a/drivers/staging/lirc/lirc_serial.c
+++ b/drivers/staging/lirc/lirc_serial.c
@@ -1127,10 +1127,8 @@ static int lirc_serial_resume(struct platform_device *dev)
 {
 	unsigned long flags;
 
-	if (hardware_init_port() < 0) {
-		lirc_serial_exit();
+	if (hardware_init_port() < 0)
 		return -EINVAL;
-	}
 
 	spin_lock_irqsave(&hardware[type].lock, flags);
 	/* Enable Interrupt */
-- 
1.7.9.2

[PATCH 4/4] [media] staging: lirc_serial: Do not assume error codes returned by request_irq()

[Jonathan Nieder]

From: Ben Hutchings <ben@decadent.org.uk>
Date: Wed, 16 Nov 2011 01:54:04 -0300

commit affc9a0d59ac49bd304e2137bd5e4ffdd6fdfa52 upstream.

lirc_serial_probe() must fail if request_irq() returns an error, even if
it isn't EBUSY or EINVAL,

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/staging/lirc/lirc_serial.c |   21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/lirc/lirc_serial.c b/drivers/staging/lirc/lirc_serial.c
index 4b8fefb954d3..21cbc9ae79c9 100644
--- a/drivers/staging/lirc/lirc_serial.c
+++ b/drivers/staging/lirc/lirc_serial.c
@@ -843,18 +843,15 @@ static int __devinit lirc_serial_probe(struct platform_device *dev)
 	result = request_irq(irq, irq_handler,
 			     IRQF_DISABLED | (share_irq ? IRQF_SHARED : 0),
 			     LIRC_DRIVER_NAME, (void *)&hardware);
-
-	switch (result) {
-	case -EBUSY:
-		printk(KERN_ERR LIRC_DRIVER_NAME ": IRQ %d busy\n", irq);
-		return -EBUSY;
-	case -EINVAL:
-		printk(KERN_ERR LIRC_DRIVER_NAME
-		       ": Bad irq number or handler\n");
-		return -EINVAL;
-	default:
-		break;
-	};
+	if (result < 0) {
+		if (result == -EBUSY)
+			printk(KERN_ERR LIRC_DRIVER_NAME ": IRQ %d busy\n",
+			       irq);
+		else if (result == -EINVAL)
+			printk(KERN_ERR LIRC_DRIVER_NAME
+			       ": Bad irq number or handler\n");
+		return result;
+	}
 
 	/* Reserve io region. */
 	/*
-- 
1.7.9.2

Re: [PATCH 3.0.y 0/4] Re: lirc_serial spuriously claims assigned port and irq to be in use

[Greg Kroah-Hartman]

On Fri, Mar 02, 2012 at 02:39:13PM -0600, Jonathan Nieder wrote:
> Ben Hutchings wrote:
> > On Thu, 2012-03-01 at 21:45 -0600, Jonathan Nieder wrote:
> 
> >> Would some of these patches (e.g., at least patches 1, 2, and 5) be
> >> appropriate for inclusion in the 3.0.y and 3.2.y stable kernels from
> >> kernel.org?
> >
> > Assuming they haven't caused any regressions, I think everything except
> > 9b98d6067971 (4/5) would be appropriate.
> 
> Great.  Here are the aforementioned patches rebased against 3.0.y, in
> the hope that some interested person can confirm they still work.  The
> only backporting needed was to adjust to the lack of
> drivers/staging/lirc -> drivers/staging/media/lirc renaming.

I'll look at these, but please, in the future, send all stable patches
to stable@vger.kernel.org as there are other stable trees that people
maintain and they watch that list...

thanks,

greg k-h

Re: [PATCH 3.0.y 0/4] Re: lirc_serial spuriously claims assigned port and irq to be in use

[Jonathan Nieder]

Greg Kroah-Hartman wrote:

> I'll look at these, but please, in the future, send all stable patches
> to stable@vger.kernel.org as there are other stable trees that people
> maintain and they watch that list...

Thanks, Greg.  Will do.

Re: [PATCH 3.0.y 0/4] Re: lirc_serial spuriously claims assigned port and irq to be in use

[Greg Kroah-Hartman]

On Fri, Mar 02, 2012 at 02:39:13PM -0600, Jonathan Nieder wrote:
> Ben Hutchings wrote:
> > On Thu, 2012-03-01 at 21:45 -0600, Jonathan Nieder wrote:
> 
> >> Would some of these patches (e.g., at least patches 1, 2, and 5) be
> >> appropriate for inclusion in the 3.0.y and 3.2.y stable kernels from
> >> kernel.org?
> >
> > Assuming they haven't caused any regressions, I think everything except
> > 9b98d6067971 (4/5) would be appropriate.
> 
> Great.  Here are the aforementioned patches rebased against 3.0.y, in
> the hope that some interested person can confirm they still work.  The
> only backporting needed was to adjust to the lack of
> drivers/staging/lirc -> drivers/staging/media/lirc renaming.

So they should also go to 3.2-stable, right?

greg k-h

Re: [PATCH 3.0.y 0/4] Re: lirc_serial spuriously claims assigned port and irq to be in use

[Jonathan Nieder]

Greg Kroah-Hartman wrote:
> On Fri, Mar 02, 2012 at 02:39:13PM -0600, Jonathan Nieder wrote:
>> Ben Hutchings wrote:
>>> On Thu, 2012-03-01 at 21:45 -0600, Jonathan Nieder wrote:

>>>> Would some of these patches (e.g., at least patches 1, 2, and 5) be
>>>> appropriate for inclusion in the 3.0.y and 3.2.y stable kernels from
>>>> kernel.org?
>>>
>>> Assuming they haven't caused any regressions, I think everything except
>>> 9b98d6067971 (4/5) would be appropriate.
>>
>> Great.  Here are the aforementioned patches rebased against 3.0.y,
[...]
> So they should also go to 3.2-stable, right?

Yes.

Thanks,
Jonathan

Re: [PATCH 3.0.y 0/4] Re: lirc_serial spuriously claims assigned port and irq to be in use

[Greg Kroah-Hartman]

On Wed, Mar 07, 2012 at 02:17:09PM -0600, Jonathan Nieder wrote:
> Greg Kroah-Hartman wrote:
> > On Fri, Mar 02, 2012 at 02:39:13PM -0600, Jonathan Nieder wrote:
> >> Ben Hutchings wrote:
> >>> On Thu, 2012-03-01 at 21:45 -0600, Jonathan Nieder wrote:
> 
> >>>> Would some of these patches (e.g., at least patches 1, 2, and 5) be
> >>>> appropriate for inclusion in the 3.0.y and 3.2.y stable kernels from
> >>>> kernel.org?
> >>>
> >>> Assuming they haven't caused any regressions, I think everything except
> >>> 9b98d6067971 (4/5) would be appropriate.
> >>
> >> Great.  Here are the aforementioned patches rebased against 3.0.y,
> [...]
> > So they should also go to 3.2-stable, right?
> 
> Yes.

Thanks, all now queued up.

greg k-h

Re: [PATCH 3.0.y 0/4] Re: lirc_serial spuriously claims assigned port and irq to be in use

[Ben Hutchings]

On Wed, Mar 07, 2012 at 12:04:07PM -0800, Greg Kroah-Hartman wrote:
> On Fri, Mar 02, 2012 at 02:39:13PM -0600, Jonathan Nieder wrote:
> > Ben Hutchings wrote:
> > > On Thu, 2012-03-01 at 21:45 -0600, Jonathan Nieder wrote:
> > 
> > >> Would some of these patches (e.g., at least patches 1, 2, and 5) be
> > >> appropriate for inclusion in the 3.0.y and 3.2.y stable kernels from
> > >> kernel.org?
> > >
> > > Assuming they haven't caused any regressions, I think everything except
> > > 9b98d6067971 (4/5) would be appropriate.
> > 
> > Great.  Here are the aforementioned patches rebased against 3.0.y, in
> > the hope that some interested person can confirm they still work.  The
> > only backporting needed was to adjust to the lack of
> > drivers/staging/lirc -> drivers/staging/media/lirc renaming.
> 
> So they should also go to 3.2-stable, right?
 
Yes, only for 3.2 a simple cherry-pick should work.

Ben.

-- 
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
                                                              - Albert Camus