All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.