sir_ir fixes and update todo
Subsystems (3)
Sean Young
maintainercover sir_ir fixes and update todo
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
Signature · 1 line
2.9.4
Sean Young
maintainerPATCH 1/5 sir_ir: attempt to free already free_irq
If the probe fails (e.g. port already in use), rmmod causes null deref.
Signed-off-by Sean Young <[email protected]>
--- drivers/media/rc/sir_ir.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/media/rc/sir_ir.c +2 -2
@@ -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); }
Signature · 1 line
2.9.4
Sean Young
maintainerPATCH 2/5 sir_ir: use dev managed resources
In reply to Sean YoungSeveral error paths do not free up resources. This simplifies the code and fixes this.
Signed-off-by Sean Young <[email protected]>
--- drivers/media/rc/sir_ir.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
drivers/media/rc/sir_ir.c +3 -6
@@ -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 )
Signature · 1 line
2.9.4
Sean Young
maintainerPATCH 3/5 sir_ir: remove init_port and drop_port functions
In reply to Sean YoungThese functions are too short and removing them makes the code more readable.
Signed-off-by Sean Young <[email protected]>
--- drivers/media/rc/sir_ir.c | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-)
drivers/media/rc/sir_ir.c +5 -22
@@ -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 ; }
Signature · 1 line
2.9.4
Sean Young
maintainerPATCH 4/5 sir_ir: remove init_chrdev and init_sir_ir functions
In reply to Sean YoungInlining these functions into the probe function makes it much more readable.
Signed-off-by Sean Young <[email protected]>
--- drivers/media/rc/sir_ir.c | 58 ++++++++++++++++++----------------------------- 1 file changed, 22 insertions(+), 36 deletions(-)
drivers/media/rc/sir_ir.c +22 -36
@@ -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)
Signature · 1 line
2.9.4
Sean Young
maintainerPATCH 5/5 staging: remove todo and replace with lirc_zilog todo
In reply to Sean YoungThe lirc_zilog driver is the last remaining lirc driver, so the existing todo is no longer relevant.
Signed-off-by Sean Young <[email protected]>
--- 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
drivers/staging/media/lirc/TODO +35 -12
@@ -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 <[email protected]> -Greg Kroah-Hartman <[email protected]> +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
drivers/staging/media/lirc/TODO.lirc_zilog deleted +0 -36
@@ -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 -
Signature · 1 line
2.9.4