From patchwork Sat Feb 25 13:54:09 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mauro Carvalho Chehab X-Patchwork-Id: 9591725 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 89CBE6042B for ; Sat, 25 Feb 2017 13:54:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 78E8A1FF29 for ; Sat, 25 Feb 2017 13:54:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6B8B8285B1; Sat, 25 Feb 2017 13:54:25 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 009BF1FF29 for ; Sat, 25 Feb 2017 13:54:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=L+E454E02+NAMdL5ZyajjsluFlP0XXGCAml20UPT6ZY=; b=ITowdmwqFtfcDY L59/VfH+tACOzo5JdFhgMLrnat2wicKgapcr3iYcA58HVWT+kvUk2HFu9XlQzv0kB81UfkBW1+W1k Kyl4VdLdhnWU681X8u1nywu2uyn9rv6O/F5VNimwo2flYGrNfOZZ5QAY3Jdhu/BzLGE4Sm/VgEwPE GJ/w+K/0nhTBosvMAl/ybRqUafe/Drk3E+QN5MczHiszM7VpNBPDJV02FDjcuJ7ezwTDevOiSfAVG NvjviMvcBOu/NzvxMghYD8GMK0UWodAPevFwRmDjRpEPDz2PJZRgbA7CWFlaMBsfztGbwYRx5jdbQ 0y50923JIr+XdZXQTOyg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1chcni-0003Hd-H1; Sat, 25 Feb 2017 13:54:18 +0000 Received: from 177.133.90.92.dynamic.adsl.gvt.net.br ([177.133.90.92] helo=vento.lan) by bombadil.infradead.org with esmtpsa (Exim 4.87 #1 (Red Hat Linux)) id 1chcnf-0003Gp-G4; Sat, 25 Feb 2017 13:54:16 +0000 Date: Sat, 25 Feb 2017 10:54:09 -0300 From: Mauro Carvalho Chehab To: Sean Young Subject: Re: [PATCH] [media] serial_ir: ensure we're ready to receive interrupts Message-ID: <20170225105409.6a059861@vento.lan> In-Reply-To: <20170225103437.58c5a199@vento.lan> References: <58b07b30.9XFLj9Hhl7F6HMc2%fengguang.wu@intel.com> <20170225111424.GA7659@gofer.mess.org> <20170225112816.GA7981@gofer.mess.org> <20170225103437.58c5a199@vento.lan> X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-redhat-linux-gnu) MIME-Version: 1.0 X-BeenThere: linux-amlogic@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "devicetree@vger.kernel.org" , Ruslan Ruslichenko , "linux-omap@vger.kernel.org" , kernel@stlinux.com, wfg@linux.intel.com, Linus Torvalds , LKML , linux-mediatek@lists.infradead.org, Linux LED Subsystem , "linux-input@vger.kernel.org" , linux-amlogic@lists.infradead.org, Thomas Gleixner , kernel test robot , LKP , Ingo Molnar , "linux-arm-kernel@lists.infradead.org" , Linux Media Mailing List Sender: "linux-amlogic" Errors-To: linux-amlogic-bounces+patchwork-linux-amlogic=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Em Sat, 25 Feb 2017 10:34:37 -0300 Mauro Carvalho Chehab escreveu: > Em Sat, 25 Feb 2017 11:28:16 +0000 > Sean Young 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+TsnQ@mail.gmail.com > > > > Cc: # 4.10 > > Signed-off-by: Sean Young > > --- > > 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. Ok, just reordered it. The diff now looks sane. Patch looks ok on my eyes. [PATCH v2] serial_ir: ensure we're ready to receive interrupts From: Sean Young 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+TsnQ@mail.gmail.com [mchehab@s-opensource.com: moved serial_ir_probe() back to its original place] Cc: # 4.10 Signed-off-by: Sean Young Signed-off-by: Mauro Carvalho Chehab diff --git a/drivers/media/rc/serial_ir.c b/drivers/media/rc/serial_ir.c index 923fb2299553..4687c34c655a 100644 --- a/drivers/media/rc/serial_ir.c +++ b/drivers/media/rc/serial_ir.c @@ -489,8 +489,59 @@ static void serial_ir_timeout(unsigned long arg) 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); @@ -516,9 +567,6 @@ static int serial_ir_probe(struct platform_device *dev) return -EBUSY; } - setup_timer(&serial_ir.timeout_timer, serial_ir_timeout, - (unsigned long)&serial_ir); - result = hardware_init_port(); if (result < 0) return result; @@ -552,7 +600,8 @@ static int serial_ir_probe(struct platform_device *dev) sense ? "low" : "high"); dev_dbg(&dev->dev, "Interrupt %d, port %04x obtained\n", irq, io); - return 0; + + return devm_rc_register_device(&dev->dev, rcdev); } static int serial_ir_open(struct rc_dev *rcdev) @@ -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(); }