From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753932Ab0IVNrK (ORCPT ); Wed, 22 Sep 2010 09:47:10 -0400 Received: from cantor2.suse.de ([195.135.220.15]:44958 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752543Ab0IVNrI (ORCPT ); Wed, 22 Sep 2010 09:47:08 -0400 Date: Wed, 22 Sep 2010 15:47:07 +0200 Message-ID: From: Takashi Iwai To: Cc: , , Subject: Re: [PATCH resent] lis3: Fix Oops with NULL platform data In-Reply-To: <62697B07E9803846BC582181BD6FB6B831300D2218@NOK-EUMSG-02.mgdnok.nokia.com> References: <62697B07E9803846BC582181BD6FB6B831300D2218@NOK-EUMSG-02.mgdnok.nokia.com> User-Agent: Wanderlust/2.15.6 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.7 Emacs/23.1 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org At Wed, 22 Sep 2010 13:56:15 +0200, wrote: > > > Hi > > >-----Original Message----- > >From: ext Takashi Iwai [mailto:tiwai@suse.de] > >Sent: 22 September, 2010 14:34 > >To: Andrew Morton > >Cc: Onkalo Samu.P (Nokia-MS/Tampere); Éric Piel; linux- > >kernel@vger.kernel.org > >Subject: [PATCH resent] lis3: Fix Oops with NULL platform data > > > >The recent addition of threaded irq handler causes a NULL dereference > >when used with hp_accel driver, which has NULL pdata. > > > >Cc: > >Signed-off-by: Takashi Iwai > >--- > > > >This should go to 2.6.36 and 2.6.35-stable tree. > > > > drivers/hwmon/lis3lv02d.c | 6 ++++-- > > 1 files changed, 4 insertions(+), 2 deletions(-) > > > >diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c > >index 5e15967..1c266ec 100644 > >--- a/drivers/hwmon/lis3lv02d.c > >+++ b/drivers/hwmon/lis3lv02d.c > >@@ -354,7 +354,8 @@ static irqreturn_t lis302dl_interrupt_thread1_8b(int > >irq, void *data) > > > > struct lis3lv02d *lis3 = data; > > > >- if ((lis3->pdata->irq_cfg & LIS3_IRQ1_MASK) == LIS3_IRQ1_CLICK) > >+ if (lis3->pdata && > >+ (lis3->pdata->irq_cfg & LIS3_IRQ1_MASK) == LIS3_IRQ1_CLICK) > > lis302dl_interrupt_handle_click(lis3); > > else > > lis302dl_interrupt_handle_ff_wu(lis3); > >@@ -367,7 +368,8 @@ static irqreturn_t lis302dl_interrupt_thread2_8b(int > >irq, void *data) > > > > struct lis3lv02d *lis3 = data; > > > >- if ((lis3->pdata->irq_cfg & LIS3_IRQ2_MASK) == LIS3_IRQ2_CLICK) > >+ if (lis3->pdata && > >+ (lis3->pdata->irq_cfg & LIS3_IRQ2_MASK) == LIS3_IRQ2_CLICK) > > lis302dl_interrupt_handle_click(lis3); > > else > > lis302dl_interrupt_handle_ff_wu(lis3); > > Yes, that will remove the kernel oops definitely, but I'm wondering if > this thread should be executed at all if there is no pdata available. We may control the caller of request_threaded_irq(), instead. The revised patch is below. (With this patch, the patch for the new LIS3CD chip must be revised, too.) > Eric, is it so that in laptops interrupts are used for free fall detection > and polled input device is for joystick? Yes, it's so for HP laptops. > Interrupts can (and are used) also for other purposes but in that case > some additional configurations are done based on platform data. > > With this patch, there are just some additional input event refreshes in > top of polled device updates. It should really not matter. So, > > Acked-by: Samu Onkalo > > -Samu thanks, Takashi === >>From 0e5bdc7516915e92fe471a77da36ecb75d995c0f Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 22 Sep 2010 12:16:04 +0200 Subject: [PATCH] lis3: Fix Oops with NULL platform data The recent addition of threaded irq handler causes a NULL dereference when used with hp_accel driver, which has NULL pdata. Signed-off-by: Takashi Iwai --- this version avoids the unnecessary thread_irq call instead of checking pdata in the irq handler. drivers/hwmon/lis3lv02d.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c index 5e15967..0a24177 100644 --- a/drivers/hwmon/lis3lv02d.c +++ b/drivers/hwmon/lis3lv02d.c @@ -301,7 +301,7 @@ static irqreturn_t lis302dl_interrupt(int irq, void *dummy) wake_up_interruptible(&lis3_dev.misc_wait); kill_fasync(&lis3_dev.async_queue, SIGIO, POLL_IN); out: - if (lis3_dev.whoami == WAI_8B && lis3_dev.idev && + if (lis3_dev.pdata && lis3_dev.whoami == WAI_8B && lis3_dev.idev && lis3_dev.idev->input->users) return IRQ_WAKE_THREAD; return IRQ_HANDLED; @@ -742,7 +742,7 @@ int lis3lv02d_init_device(struct lis3lv02d *dev) * io-apic is not configurable (and generates a warning) but I keep it * in case of support for other hardware. */ - if (dev->whoami == WAI_8B) + if (dev->pdata && dev->whoami == WAI_8B) thread_fn = lis302dl_interrupt_thread1_8b; else thread_fn = NULL; -- 1.7.2.2