staging: lirc_serial: Fix init/exit order

PATCH In review Ben Hutchings 4 participants 19 messages Nov 16, 2011 – Mar 8, 2012

Subsystems (2)

Media (V4L/DVB) Staging

lore

Share & tools

lore

Ben Hutchings

author

2011-11-16 05:49 UTC #0 lore raw

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

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 <[email protected]>

---
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(-)

drivers/staging/media/lirc/lirc_serial.c +21 ~~-35~~

index 8a060a8..8637631 100644
@@ -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");
 }

Signature · 1 line

1.7.7.2

Ben Hutchings

author

2011-11-16 05:52 UTC #1 lore raw

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

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

Reported-by Torsten Crass <[email protected]>

References: http://bugs.debian.org/645811

Signed-off-by Ben Hutchings <[email protected]>

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

drivers/staging/media/lirc/lirc_serial.c +16 -3

index 8637631..d833772 100644
@@ -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)

Signature · 1 line

1.7.7.2

Ben Hutchings

author

2011-11-16 05:53 UTC #2 lore raw

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

In reply to Ben Hutchings

A resume function cannot remove the device it is resuming!

Signed-off-by Ben Hutchings <[email protected]>

---
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(-)

drivers/staging/media/lirc/lirc_serial.c +1 -3

index d833772..befe626 100644
@@ -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 */

Signature · 1 line

1.7.7.2

Ben Hutchings

author

2011-11-16 05:53 UTC #3 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/staging/media/lirc/lirc_serial.c |   23 ++++++++++++-----------
 1 files changed, 12 insertions(+), 11 deletions(-)

drivers/staging/media/lirc/lirc_serial.c +12 ~~-11~~

index befe626..6f5257e 100644
@@ -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;
 }

Signature · 1 line

1.7.7.2

Ben Hutchings

author

2011-11-16 05:54 UTC #4 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/staging/media/lirc/lirc_serial.c |   21 +++++++++------------
 1 files changed, 9 insertions(+), 12 deletions(-)

drivers/staging/media/lirc/lirc_serial.c +9 ~~-12~~

index 6f5257e..0ca308a 100644
@@ -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. */
 	/*

Signature · 1 line

1.7.7.2

Jonathan Nieder

2012-03-02 03:45 UTC #5 lore raw

In reply to Ben Hutchings

Hi Ben,

Ben Hutchings wrote[1]:

Quoted 10 lines

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.

Quoted 7 lines

Signed-off-by Ben Hutchings <[email protected]>
---
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

Ben Hutchings

author

2012-03-02 04:35 UTC #6 lore raw

On Thu, 2012-03-01 at 21:45 -0600, Jonathan Nieder wrote:
[...]

Quoted 10 lines

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

Quoted 6 lines

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.

Signature · 2 lines

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

VDR User

2012-03-02 05:29 UTC #7 lore raw

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

Jonathan Nieder

2012-03-02 20:39 UTC #8 lore raw

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

In reply to Ben Hutchings

Quoted 8 lines · Ben Hutchings

Quoted 3 lines · Jonathan Nieder
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(-)

Jonathan Nieder

2012-03-02 20:40 UTC #9 lore raw

PATCH 1/4 staging: lirc_serial: Fix init/exit order

From: Ben Hutchings <[email protected]>
Date: Wed, 16 Nov 2011 01:49:41 -0300

commit 9105b8b200410383d0854bbe237ee385d7d33ba6 upstream.

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 <[email protected]>

Signed-off-by Mauro Carvalho Chehab <[email protected]>

Signed-off-by Jonathan Nieder <[email protected]>

---
 drivers/staging/lirc/lirc_serial.c |   56 ++++++++++++++----------------------
 1 file changed, 21 insertions(+), 35 deletions(-)

drivers/staging/lirc/lirc_serial.c +21 ~~-35~~

index 805df913bb6e..c8a4b7bc5879 100644
@@ -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");
 }

Signature · 1 line

1.7.9.2

Jonathan Nieder

2012-03-02 20:40 UTC #10 lore raw

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

In reply to Jonathan Nieder

From: Ben Hutchings <[email protected]>
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 <[email protected]>

Signed-off-by Ben Hutchings <[email protected]>

Signed-off-by Mauro Carvalho Chehab <[email protected]>

Signed-off-by Jonathan Nieder <[email protected]>

---
 drivers/staging/lirc/lirc_serial.c |   19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

drivers/staging/lirc/lirc_serial.c +16 -3

index c8a4b7bc5879..fa023da6bdaa 100644
@@ -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)

Signature · 1 line

1.7.9.2

Jonathan Nieder

2012-03-02 20:41 UTC #11 lore raw

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

In reply to Jonathan Nieder

From: Ben Hutchings <[email protected]>
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 <[email protected]>

Signed-off-by Mauro Carvalho Chehab <[email protected]>

Signed-off-by Jonathan Nieder <[email protected]>

---
 drivers/staging/lirc/lirc_serial.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

drivers/staging/lirc/lirc_serial.c +1 -3

index fa023da6bdaa..4b8fefb954d3 100644
@@ -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 */

Signature · 1 line

1.7.9.2

Jonathan Nieder

2012-03-02 20:41 UTC #12 lore raw

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

In reply to Jonathan Nieder

From: Ben Hutchings <[email protected]>
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 <[email protected]>

Signed-off-by Mauro Carvalho Chehab <[email protected]>

Signed-off-by Jonathan Nieder <[email protected]>

---
 drivers/staging/lirc/lirc_serial.c |   21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

drivers/staging/lirc/lirc_serial.c +9 ~~-12~~

index 4b8fefb954d3..21cbc9ae79c9 100644
@@ -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. */
 	/*

Signature · 1 line

1.7.9.2

Greg Kroah-Hartman

maintainer

2012-03-02 21:16 UTC #13 lore raw

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

In reply to Jonathan Nieder

Quoted 7 lines · Jonathan Nieder

Quoted 6 lines · Ben Hutchings
Quoted 3 lines · Jonathan Nieder
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?

Quoted 7 lines

Quoted 2 lines
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 [email protected] as there are other stable trees that people maintain and they watch that list...

thanks,

greg k-h

Jonathan Nieder

2012-03-03 01:14 UTC #14 lore raw

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

Quoted 3 lines · Greg Kroah-Hartman

I'll look at these, but please, in the future, send all stable patches to [email protected] as there are other stable trees that people maintain and they watch that list...

Thanks, Greg. Will do.

Greg Kroah-Hartman

maintainer

2012-03-07 20:04 UTC #15 lore raw

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

In reply to Jonathan Nieder

Quoted 7 lines · Jonathan Nieder

Quoted 6 lines · Ben Hutchings
Quoted 3 lines · Jonathan Nieder
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?

Quoted 7 lines

Quoted 2 lines
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

Jonathan Nieder

2012-03-07 20:17 UTC #16 lore raw

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

Quoted 12 lines · Greg Kroah-Hartman

Quoted 11 lines · Jonathan Nieder
Quoted 8 lines · Ben Hutchings
Quoted 3 lines · Jonathan Nieder
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,

[...]

Quoted 1 line

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

Yes.

Thanks,
Jonathan

Greg Kroah-Hartman

maintainer

2012-03-08 18:34 UTC #17 lore raw

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

Quoted 9 lines · Jonathan Nieder

Quoted 8 lines · Greg Kroah-Hartman
Quoted 7 lines · Jonathan Nieder
Quoted 6 lines · Ben Hutchings
Quoted 3 lines · Jonathan Nieder
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?

Quoted 8 lines

Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
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,
[...]
Quoted 1 line
So they should also go to 3.2-stable, right?
Yes.

Thanks, all now queued up.

greg k-h

Ben Hutchings

author

2012-03-07 21:34 UTC #18 lore raw

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

In reply to Greg Kroah-Hartman

Quoted 11 lines · Greg Kroah-Hartman

Quoted 10 lines · Jonathan Nieder
Quoted 8 lines · Ben Hutchings
Quoted 3 lines · Jonathan Nieder
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.

Quoted 6 lines

Quoted 4 lines
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.

Signature · 3 lines

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