linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] sir_ir fixes and update todo
@ 2017-05-17 17:32 Sean Young
  2017-05-17 17:32 ` [PATCH 1/5] [media] sir_ir: attempt to free already free_irq Sean Young
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Sean Young @ 2017-05-17 17:32 UTC (permalink / raw)
  To: linux-media

Just some minor cleanups.

Sean Young (5):
  [media] sir_ir: attempt to free already free_irq
  [media] sir_ir: use dev managed resources
  [media] sir_ir: remove init_port and drop_port functions
  [media] sir_ir: remove init_chrdev and init_sir_ir functions
  [media] staging: remove todo and replace with lirc_zilog todo

 drivers/media/rc/sir_ir.c                  | 90 ++++++++++--------------------
 drivers/staging/media/lirc/TODO            | 47 ++++++++++++----
 drivers/staging/media/lirc/TODO.lirc_zilog | 36 ------------
 3 files changed, 63 insertions(+), 110 deletions(-)
 delete mode 100644 drivers/staging/media/lirc/TODO.lirc_zilog

-- 
2.9.4

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/5] [media] sir_ir: attempt to free already free_irq
  2017-05-17 17:32 [PATCH 0/5] sir_ir fixes and update todo Sean Young
@ 2017-05-17 17:32 ` Sean Young
  2017-05-17 17:32 ` [PATCH 2/5] [media] sir_ir: use dev managed resources Sean Young
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Sean Young @ 2017-05-17 17:32 UTC (permalink / raw)
  To: linux-media

If the probe fails (e.g. port already in use), rmmod causes null deref.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/sir_ir.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/rc/sir_ir.c b/drivers/media/rc/sir_ir.c
index 90a5f8f..c27d6b4 100644
--- a/drivers/media/rc/sir_ir.c
+++ b/drivers/media/rc/sir_ir.c
@@ -381,6 +381,8 @@ static int sir_ir_probe(struct platform_device *dev)
 
 static int sir_ir_remove(struct platform_device *dev)
 {
+	drop_hardware();
+	drop_port();
 	return 0;
 }
 
@@ -421,8 +423,6 @@ static int __init sir_ir_init(void)
 
 static void __exit sir_ir_exit(void)
 {
-	drop_hardware();
-	drop_port();
 	platform_device_unregister(sir_ir_dev);
 	platform_driver_unregister(&sir_ir_driver);
 }
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/5] [media] sir_ir: use dev managed resources
  2017-05-17 17:32 [PATCH 0/5] sir_ir fixes and update todo Sean Young
  2017-05-17 17:32 ` [PATCH 1/5] [media] sir_ir: attempt to free already free_irq Sean Young
@ 2017-05-17 17:32 ` Sean Young
  2017-05-17 17:32 ` [PATCH 3/5] [media] sir_ir: remove init_port and drop_port functions Sean Young
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Sean Young @ 2017-05-17 17:32 UTC (permalink / raw)
  To: linux-media

Several error paths do not free up resources. This simplifies the code
and fixes this.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/sir_ir.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/media/rc/sir_ir.c b/drivers/media/rc/sir_ir.c
index c27d6b4..1ee41adb 100644
--- a/drivers/media/rc/sir_ir.c
+++ b/drivers/media/rc/sir_ir.c
@@ -334,14 +334,13 @@ static int init_port(void)
 	setup_timer(&timerlist, sir_timeout, 0);
 
 	/* get I/O port access and IRQ line */
-	if (!request_region(io, 8, KBUILD_MODNAME)) {
+	if (!devm_request_region(&sir_ir_dev->dev, io, 8, KBUILD_MODNAME)) {
 		pr_err("i/o port 0x%.4x already in use.\n", io);
 		return -EBUSY;
 	}
-	retval = request_irq(irq, sir_interrupt, 0,
-			     KBUILD_MODNAME, NULL);
+	retval = devm_request_irq(&sir_ir_dev->dev, irq, sir_interrupt, 0,
+				  KBUILD_MODNAME, NULL);
 	if (retval < 0) {
-		release_region(io, 8);
 		pr_err("IRQ %d already in use.\n", irq);
 		return retval;
 	}
@@ -352,9 +351,7 @@ static int init_port(void)
 
 static void drop_port(void)
 {
-	free_irq(irq, NULL);
 	del_timer_sync(&timerlist);
-	release_region(io, 8);
 }
 
 static int init_sir_ir(void)
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/5] [media] sir_ir: remove init_port and drop_port functions
  2017-05-17 17:32 [PATCH 0/5] sir_ir fixes and update todo Sean Young
  2017-05-17 17:32 ` [PATCH 1/5] [media] sir_ir: attempt to free already free_irq Sean Young
  2017-05-17 17:32 ` [PATCH 2/5] [media] sir_ir: use dev managed resources Sean Young
@ 2017-05-17 17:32 ` Sean Young
  2017-05-17 17:32 ` [PATCH 4/5] [media] sir_ir: remove init_chrdev and init_sir_ir functions Sean Young
  2017-05-17 17:32 ` [PATCH 5/5] [media] staging: remove todo and replace with lirc_zilog todo Sean Young
  4 siblings, 0 replies; 6+ messages in thread
From: Sean Young @ 2017-05-17 17:32 UTC (permalink / raw)
  To: linux-media

These functions are too short and removing them makes the code more
readable.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/sir_ir.c | 27 +++++----------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

diff --git a/drivers/media/rc/sir_ir.c b/drivers/media/rc/sir_ir.c
index 1ee41adb..fdac570 100644
--- a/drivers/media/rc/sir_ir.c
+++ b/drivers/media/rc/sir_ir.c
@@ -58,11 +58,9 @@ static int init_chrdev(void);
 static irqreturn_t sir_interrupt(int irq, void *dev_id);
 static void send_space(unsigned long len);
 static void send_pulse(unsigned long len);
-static int init_hardware(void);
+static void init_hardware(void);
 static void drop_hardware(void);
 /* Initialisation */
-static int init_port(void);
-static void drop_port(void);
 
 static inline unsigned int sinp(int offset)
 {
@@ -288,7 +286,7 @@ static void send_pulse(unsigned long len)
 	}
 }
 
-static int init_hardware(void)
+static void init_hardware(void)
 {
 	unsigned long flags;
 
@@ -310,7 +308,6 @@ static int init_hardware(void)
 	/* turn on UART */
 	outb(UART_MCR_DTR | UART_MCR_RTS | UART_MCR_OUT2, io + UART_MCR);
 	spin_unlock_irqrestore(&hardware_lock, flags);
-	return 0;
 }
 
 static void drop_hardware(void)
@@ -327,7 +324,7 @@ static void drop_hardware(void)
 
 /* SECTION: Initialisation */
 
-static int init_port(void)
+static int init_sir_ir(void)
 {
 	int retval;
 
@@ -346,22 +343,8 @@ static int init_port(void)
 	}
 	pr_info("I/O port 0x%.4x, IRQ %d.\n", io, irq);
 
-	return 0;
-}
-
-static void drop_port(void)
-{
-	del_timer_sync(&timerlist);
-}
-
-static int init_sir_ir(void)
-{
-	int retval;
-
-	retval = init_port();
-	if (retval < 0)
-		return retval;
 	init_hardware();
+
 	return 0;
 }
 
@@ -379,7 +362,7 @@ static int sir_ir_probe(struct platform_device *dev)
 static int sir_ir_remove(struct platform_device *dev)
 {
 	drop_hardware();
-	drop_port();
+	del_timer_sync(&timerlist);
 	return 0;
 }
 
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/5] [media] sir_ir: remove init_chrdev and init_sir_ir functions
  2017-05-17 17:32 [PATCH 0/5] sir_ir fixes and update todo Sean Young
                   ` (2 preceding siblings ...)
  2017-05-17 17:32 ` [PATCH 3/5] [media] sir_ir: remove init_port and drop_port functions Sean Young
@ 2017-05-17 17:32 ` Sean Young
  2017-05-17 17:32 ` [PATCH 5/5] [media] staging: remove todo and replace with lirc_zilog todo Sean Young
  4 siblings, 0 replies; 6+ messages in thread
From: Sean Young @ 2017-05-17 17:32 UTC (permalink / raw)
  To: linux-media

Inlining these functions into the probe function makes it much
more readable.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/sir_ir.c | 58 ++++++++++++++++++-----------------------------
 1 file changed, 22 insertions(+), 36 deletions(-)

diff --git a/drivers/media/rc/sir_ir.c b/drivers/media/rc/sir_ir.c
index fdac570..5ee3a23 100644
--- a/drivers/media/rc/sir_ir.c
+++ b/drivers/media/rc/sir_ir.c
@@ -53,7 +53,6 @@ static DEFINE_SPINLOCK(hardware_lock);
 
 /* Communication with user-space */
 static void add_read_queue(int flag, unsigned long val);
-static int init_chrdev(void);
 /* Hardware */
 static irqreturn_t sir_interrupt(int irq, void *dev_id);
 static void send_space(unsigned long len);
@@ -120,28 +119,6 @@ static void add_read_queue(int flag, unsigned long val)
 	ir_raw_event_store_with_filter(rcdev, &ev);
 }
 
-static int init_chrdev(void)
-{
-	rcdev = devm_rc_allocate_device(&sir_ir_dev->dev, RC_DRIVER_IR_RAW);
-	if (!rcdev)
-		return -ENOMEM;
-
-	rcdev->input_name = "SIR IrDA port";
-	rcdev->input_phys = KBUILD_MODNAME "/input0";
-	rcdev->input_id.bustype = BUS_HOST;
-	rcdev->input_id.vendor = 0x0001;
-	rcdev->input_id.product = 0x0001;
-	rcdev->input_id.version = 0x0100;
-	rcdev->tx_ir = sir_tx_ir;
-	rcdev->allowed_protocols = RC_BIT_ALL_IR_DECODER;
-	rcdev->driver_name = KBUILD_MODNAME;
-	rcdev->map_name = RC_MAP_RC6_MCE;
-	rcdev->timeout = IR_DEFAULT_TIMEOUT;
-	rcdev->dev.parent = &sir_ir_dev->dev;
-
-	return devm_rc_register_device(&sir_ir_dev->dev, rcdev);
-}
-
 /* SECTION: Hardware */
 static void sir_timeout(unsigned long data)
 {
@@ -323,11 +300,27 @@ static void drop_hardware(void)
 }
 
 /* SECTION: Initialisation */
-
-static int init_sir_ir(void)
+static int sir_ir_probe(struct platform_device *dev)
 {
 	int retval;
 
+	rcdev = devm_rc_allocate_device(&sir_ir_dev->dev, RC_DRIVER_IR_RAW);
+	if (!rcdev)
+		return -ENOMEM;
+
+	rcdev->input_name = "SIR IrDA port";
+	rcdev->input_phys = KBUILD_MODNAME "/input0";
+	rcdev->input_id.bustype = BUS_HOST;
+	rcdev->input_id.vendor = 0x0001;
+	rcdev->input_id.product = 0x0001;
+	rcdev->input_id.version = 0x0100;
+	rcdev->tx_ir = sir_tx_ir;
+	rcdev->allowed_protocols = RC_BIT_ALL_IR_DECODER;
+	rcdev->driver_name = KBUILD_MODNAME;
+	rcdev->map_name = RC_MAP_RC6_MCE;
+	rcdev->timeout = IR_DEFAULT_TIMEOUT;
+	rcdev->dev.parent = &sir_ir_dev->dev;
+
 	setup_timer(&timerlist, sir_timeout, 0);
 
 	/* get I/O port access and IRQ line */
@@ -343,20 +336,13 @@ static int init_sir_ir(void)
 	}
 	pr_info("I/O port 0x%.4x, IRQ %d.\n", io, irq);
 
-	init_hardware();
-
-	return 0;
-}
-
-static int sir_ir_probe(struct platform_device *dev)
-{
-	int retval;
-
-	retval = init_chrdev();
+	retval = devm_rc_register_device(&sir_ir_dev->dev, rcdev);
 	if (retval < 0)
 		return retval;
 
-	return init_sir_ir();
+	init_hardware();
+
+	return 0;
 }
 
 static int sir_ir_remove(struct platform_device *dev)
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 5/5] [media] staging: remove todo and replace with lirc_zilog todo
  2017-05-17 17:32 [PATCH 0/5] sir_ir fixes and update todo Sean Young
                   ` (3 preceding siblings ...)
  2017-05-17 17:32 ` [PATCH 4/5] [media] sir_ir: remove init_chrdev and init_sir_ir functions Sean Young
@ 2017-05-17 17:32 ` Sean Young
  4 siblings, 0 replies; 6+ messages in thread
From: Sean Young @ 2017-05-17 17:32 UTC (permalink / raw)
  To: linux-media

The lirc_zilog driver is the last remaining lirc driver, so the existing
todo is no longer relevant.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/staging/media/lirc/TODO            | 47 ++++++++++++++++++++++--------
 drivers/staging/media/lirc/TODO.lirc_zilog | 36 -----------------------
 2 files changed, 35 insertions(+), 48 deletions(-)
 delete mode 100644 drivers/staging/media/lirc/TODO.lirc_zilog

diff --git a/drivers/staging/media/lirc/TODO b/drivers/staging/media/lirc/TODO
index cbea5d8..a97800a 100644
--- a/drivers/staging/media/lirc/TODO
+++ b/drivers/staging/media/lirc/TODO
@@ -1,13 +1,36 @@
-- All drivers should either be ported to ir-core, or dropped entirely
-  (see drivers/media/IR/mceusb.c vs. lirc_mceusb.c in lirc cvs for an
-  example of a previously completed port).
-
-- lirc_bt829 uses registers on a Mach64 VT, which has a separate kernel
-  framebuffer driver (atyfb) and userland X driver (mach64).  It can't
-  simply be converted to a normal PCI driver, but ideally it should be
-  coordinated with the other drivers.
-
-Please send patches to:
-Jarod Wilson <jarod@wilsonet.com>
-Greg Kroah-Hartman <greg@kroah.com>
+1. Both ir-kbd-i2c and lirc_zilog provide support for RX events for
+the chips supported by lirc_zilog.  Before moving lirc_zilog out of staging:
+
+a. ir-kbd-i2c needs a module parameter added to allow the user to tell
+   ir-kbd-i2c to ignore Z8 IR units.
+
+b. lirc_zilog should provide Rx key presses to the rc core like ir-kbd-i2c
+   does.
+
+
+2. lirc_zilog module ref-counting need examination.  It has not been
+verified that cdev and lirc_dev will take the proper module references on
+lirc_zilog to prevent removal of lirc_zilog when the /dev/lircN device node
+is open.
+
+(The good news is ref-counting of lirc_zilog internal structures appears to be
+complete.  Testing has shown the cx18 module can be unloaded out from under
+irw + lircd + lirc_dev, with the /dev/lirc0 device node open, with no adverse
+effects.  The cx18 module could then be reloaded and irw properly began
+receiving button presses again and ir_send worked without error.)
+
+
+3. Bridge drivers, if able, should provide a chip reset() callback
+to lirc_zilog via struct IR_i2c_init_data.  cx18 and ivtv already have routines
+to perform Z8 chip resets via GPIO manipulations.  This would allow lirc_zilog
+to bring the chip back to normal when it hangs, in the same places the
+original lirc_pvr150 driver code does.  This is not strictly needed, so it
+is not required to move lirc_zilog out of staging.
+
+Note: Both lirc_zilog and ir-kbd-i2c support the Zilog Z8 for IR, as programmed
+and installed on Hauppauge products.  When working on either module, developers
+must consider at least the following bridge drivers which mention an IR Rx unit
+at address 0x71 (indicative of a Z8):
+
+	ivtv cx18 hdpvr pvrusb2 bt8xx cx88 saa7134
 
diff --git a/drivers/staging/media/lirc/TODO.lirc_zilog b/drivers/staging/media/lirc/TODO.lirc_zilog
deleted file mode 100644
index a97800a..0000000
--- a/drivers/staging/media/lirc/TODO.lirc_zilog
+++ /dev/null
@@ -1,36 +0,0 @@
-1. Both ir-kbd-i2c and lirc_zilog provide support for RX events for
-the chips supported by lirc_zilog.  Before moving lirc_zilog out of staging:
-
-a. ir-kbd-i2c needs a module parameter added to allow the user to tell
-   ir-kbd-i2c to ignore Z8 IR units.
-
-b. lirc_zilog should provide Rx key presses to the rc core like ir-kbd-i2c
-   does.
-
-
-2. lirc_zilog module ref-counting need examination.  It has not been
-verified that cdev and lirc_dev will take the proper module references on
-lirc_zilog to prevent removal of lirc_zilog when the /dev/lircN device node
-is open.
-
-(The good news is ref-counting of lirc_zilog internal structures appears to be
-complete.  Testing has shown the cx18 module can be unloaded out from under
-irw + lircd + lirc_dev, with the /dev/lirc0 device node open, with no adverse
-effects.  The cx18 module could then be reloaded and irw properly began
-receiving button presses again and ir_send worked without error.)
-
-
-3. Bridge drivers, if able, should provide a chip reset() callback
-to lirc_zilog via struct IR_i2c_init_data.  cx18 and ivtv already have routines
-to perform Z8 chip resets via GPIO manipulations.  This would allow lirc_zilog
-to bring the chip back to normal when it hangs, in the same places the
-original lirc_pvr150 driver code does.  This is not strictly needed, so it
-is not required to move lirc_zilog out of staging.
-
-Note: Both lirc_zilog and ir-kbd-i2c support the Zilog Z8 for IR, as programmed
-and installed on Hauppauge products.  When working on either module, developers
-must consider at least the following bridge drivers which mention an IR Rx unit
-at address 0x71 (indicative of a Z8):
-
-	ivtv cx18 hdpvr pvrusb2 bt8xx cx88 saa7134
-
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-05-17 17:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-17 17:32 [PATCH 0/5] sir_ir fixes and update todo Sean Young
2017-05-17 17:32 ` [PATCH 1/5] [media] sir_ir: attempt to free already free_irq Sean Young
2017-05-17 17:32 ` [PATCH 2/5] [media] sir_ir: use dev managed resources Sean Young
2017-05-17 17:32 ` [PATCH 3/5] [media] sir_ir: remove init_port and drop_port functions Sean Young
2017-05-17 17:32 ` [PATCH 4/5] [media] sir_ir: remove init_chrdev and init_sir_ir functions Sean Young
2017-05-17 17:32 ` [PATCH 5/5] [media] staging: remove todo and replace with lirc_zilog todo Sean Young

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).