Em Sat, 25 Feb 2017 11:28:16 +0000 Sean Young <s...@mess.org> escreveu:
> When the interrupt requested with devm_request_irq(), serial_ir.rcdev > is still null so will cause null deference if the irq handler is called > early on. > > Also ensure that timeout_timer is setup. > > Link: > http://lkml.kernel.org/r/ca+55afxsh2uf8gi5sn_guy3z+tilv7lpjykbw+y8vqlzp+t...@mail.gmail.com > > Cc: <sta...@vger.kernel.org> # 4.10 > Signed-off-by: Sean Young <s...@mess.org> > --- > drivers/media/rc/serial_ir.c | 243 > +++++++++++++++++++++---------------------- > 1 file changed, 118 insertions(+), 125 deletions(-) > > diff --git a/drivers/media/rc/serial_ir.c b/drivers/media/rc/serial_ir.c > index 923fb22..22144b4 100644 > --- a/drivers/media/rc/serial_ir.c > +++ b/drivers/media/rc/serial_ir.c > @@ -487,74 +487,6 @@ static void serial_ir_timeout(unsigned long arg) > ir_raw_event_handle(serial_ir.rcdev); > } > > -static int serial_ir_probe(struct platform_device *dev) > -{ > - int i, nlow, nhigh, result; Hmm... why did you move this function to be after serial_ir_open()? That messes with the diff without no good reason, making harder to identify what you changed here. > - > - result = devm_request_irq(&dev->dev, irq, serial_ir_irq_handler, > - share_irq ? IRQF_SHARED : 0, > - KBUILD_MODNAME, &hardware); > - if (result < 0) { > - if (result == -EBUSY) > - dev_err(&dev->dev, "IRQ %d busy\n", irq); > - else if (result == -EINVAL) > - dev_err(&dev->dev, "Bad irq number or handler\n"); > - return result; > - } > - > - /* Reserve io region. */ > - if ((iommap && > - (devm_request_mem_region(&dev->dev, iommap, 8 << ioshift, > - KBUILD_MODNAME) == NULL)) || > - (!iommap && (devm_request_region(&dev->dev, io, 8, > - KBUILD_MODNAME) == NULL))) { > - dev_err(&dev->dev, "port %04x already in use\n", io); > - dev_warn(&dev->dev, "use 'setserial /dev/ttySX uart none'\n"); > - dev_warn(&dev->dev, > - "or compile the serial port driver as module and\n"); > - dev_warn(&dev->dev, "make sure this module is loaded first\n"); > - return -EBUSY; > - } > - > - setup_timer(&serial_ir.timeout_timer, serial_ir_timeout, > - (unsigned long)&serial_ir); > - > - result = hardware_init_port(); > - if (result < 0) > - return result; > - > - /* Initialize pulse/space widths */ > - init_timing_params(50, 38000); > - > - /* If pin is high, then this must be an active low receiver. */ > - if (sense == -1) { > - /* wait 1/2 sec for the power supply */ > - msleep(500); > - > - /* > - * probe 9 times every 0.04s, collect "votes" for > - * active high/low > - */ > - nlow = 0; > - nhigh = 0; > - for (i = 0; i < 9; i++) { > - if (sinp(UART_MSR) & hardware[type].signal_pin) > - nlow++; > - else > - nhigh++; > - msleep(40); > - } > - sense = nlow >= nhigh ? 1 : 0; > - dev_info(&dev->dev, "auto-detected active %s receiver\n", > - sense ? "low" : "high"); > - } else > - dev_info(&dev->dev, "Manually using active %s receiver\n", > - sense ? "low" : "high"); > - > - dev_dbg(&dev->dev, "Interrupt %d, port %04x obtained\n", irq, io); > - return 0; > -} > - > static int serial_ir_open(struct rc_dev *rcdev) > { > unsigned long flags; > @@ -679,6 +611,123 @@ static int serial_ir_resume(struct platform_device *dev) > return 0; > } > > +static int serial_ir_probe(struct platform_device *dev) > +{ > + struct rc_dev *rcdev; > + int i, nlow, nhigh, result; > + > + rcdev = devm_rc_allocate_device(&dev->dev, RC_DRIVER_IR_RAW); > + if (!rcdev) > + return -ENOMEM; > + > + if (hardware[type].send_pulse && hardware[type].send_space) > + rcdev->tx_ir = serial_ir_tx; > + if (hardware[type].set_send_carrier) > + rcdev->s_tx_carrier = serial_ir_tx_carrier; > + if (hardware[type].set_duty_cycle) > + rcdev->s_tx_duty_cycle = serial_ir_tx_duty_cycle; > + > + switch (type) { > + case IR_HOMEBREW: > + rcdev->input_name = "Serial IR type home-brew"; > + break; > + case IR_IRDEO: > + rcdev->input_name = "Serial IR type IRdeo"; > + break; > + case IR_IRDEO_REMOTE: > + rcdev->input_name = "Serial IR type IRdeo remote"; > + break; > + case IR_ANIMAX: > + rcdev->input_name = "Serial IR type AnimaX"; > + break; > + case IR_IGOR: > + rcdev->input_name = "Serial IR type IgorPlug"; > + break; > + } > + > + 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->open = serial_ir_open; > + rcdev->close = serial_ir_close; > + rcdev->dev.parent = &serial_ir.pdev->dev; > + rcdev->allowed_protocols = RC_BIT_ALL_IR_DECODER; > + rcdev->driver_name = KBUILD_MODNAME; > + rcdev->map_name = RC_MAP_RC6_MCE; > + rcdev->min_timeout = 1; > + rcdev->timeout = IR_DEFAULT_TIMEOUT; > + rcdev->max_timeout = 10 * IR_DEFAULT_TIMEOUT; > + rcdev->rx_resolution = 250000; > + > + serial_ir.rcdev = rcdev; > + > + setup_timer(&serial_ir.timeout_timer, serial_ir_timeout, > + (unsigned long)&serial_ir); > + > + result = devm_request_irq(&dev->dev, irq, serial_ir_irq_handler, > + share_irq ? IRQF_SHARED : 0, > + KBUILD_MODNAME, &hardware); > + if (result < 0) { > + if (result == -EBUSY) > + dev_err(&dev->dev, "IRQ %d busy\n", irq); > + else if (result == -EINVAL) > + dev_err(&dev->dev, "Bad irq number or handler\n"); > + return result; > + } > + > + /* Reserve io region. */ > + if ((iommap && > + (devm_request_mem_region(&dev->dev, iommap, 8 << ioshift, > + KBUILD_MODNAME) == NULL)) || > + (!iommap && (devm_request_region(&dev->dev, io, 8, > + KBUILD_MODNAME) == NULL))) { > + dev_err(&dev->dev, "port %04x already in use\n", io); > + dev_warn(&dev->dev, "use 'setserial /dev/ttySX uart none'\n"); > + dev_warn(&dev->dev, > + "or compile the serial port driver as module and\n"); > + dev_warn(&dev->dev, "make sure this module is loaded first\n"); > + return -EBUSY; > + } > + > + result = hardware_init_port(); > + if (result < 0) > + return result; > + > + /* Initialize pulse/space widths */ > + init_timing_params(50, 38000); > + > + /* If pin is high, then this must be an active low receiver. */ > + if (sense == -1) { > + /* wait 1/2 sec for the power supply */ > + msleep(500); > + > + /* > + * probe 9 times every 0.04s, collect "votes" for > + * active high/low > + */ > + nlow = 0; > + nhigh = 0; > + for (i = 0; i < 9; i++) { > + if (sinp(UART_MSR) & hardware[type].signal_pin) > + nlow++; > + else > + nhigh++; > + msleep(40); > + } > + sense = nlow >= nhigh ? 1 : 0; > + dev_info(&dev->dev, "auto-detected active %s receiver\n", > + sense ? "low" : "high"); > + } else > + dev_info(&dev->dev, "Manually using active %s receiver\n", > + sense ? "low" : "high"); > + > + dev_dbg(&dev->dev, "Interrupt %d, port %04x obtained\n", irq, io); > + > + return devm_rc_register_device(&dev->dev, rcdev); > +} > + > static struct platform_driver serial_ir_driver = { > .probe = serial_ir_probe, > .suspend = serial_ir_suspend, > @@ -723,7 +772,6 @@ static void serial_ir_exit(void) > > static int __init serial_ir_init_module(void) > { > - struct rc_dev *rcdev; > int result; > > switch (type) { > @@ -754,63 +802,9 @@ static int __init serial_ir_init_module(void) > sense = !!sense; > > result = serial_ir_init(); > - if (result) > - return result; > - > - rcdev = devm_rc_allocate_device(&serial_ir.pdev->dev, RC_DRIVER_IR_RAW); > - if (!rcdev) { > - result = -ENOMEM; > - goto serial_cleanup; > - } > - > - if (hardware[type].send_pulse && hardware[type].send_space) > - rcdev->tx_ir = serial_ir_tx; > - if (hardware[type].set_send_carrier) > - rcdev->s_tx_carrier = serial_ir_tx_carrier; > - if (hardware[type].set_duty_cycle) > - rcdev->s_tx_duty_cycle = serial_ir_tx_duty_cycle; > - > - switch (type) { > - case IR_HOMEBREW: > - rcdev->input_name = "Serial IR type home-brew"; > - break; > - case IR_IRDEO: > - rcdev->input_name = "Serial IR type IRdeo"; > - break; > - case IR_IRDEO_REMOTE: > - rcdev->input_name = "Serial IR type IRdeo remote"; > - break; > - case IR_ANIMAX: > - rcdev->input_name = "Serial IR type AnimaX"; > - break; > - case IR_IGOR: > - rcdev->input_name = "Serial IR type IgorPlug"; > - break; > - } > - > - 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->open = serial_ir_open; > - rcdev->close = serial_ir_close; > - rcdev->dev.parent = &serial_ir.pdev->dev; > - rcdev->allowed_protocols = RC_BIT_ALL_IR_DECODER; > - rcdev->driver_name = KBUILD_MODNAME; > - rcdev->map_name = RC_MAP_RC6_MCE; > - rcdev->min_timeout = 1; > - rcdev->timeout = IR_DEFAULT_TIMEOUT; > - rcdev->max_timeout = 10 * IR_DEFAULT_TIMEOUT; > - rcdev->rx_resolution = 250000; > - > - serial_ir.rcdev = rcdev; > - > - result = rc_register_device(rcdev); > - > if (!result) > return 0; > -serial_cleanup: > + > serial_ir_exit(); > return result; > } > @@ -818,7 +812,6 @@ static int __init serial_ir_init_module(void) > static void __exit serial_ir_exit_module(void) > { > del_timer_sync(&serial_ir.timeout_timer); > - rc_unregister_device(serial_ir.rcdev); > serial_ir_exit(); > } > Thanks, Mauro
