Re: [PATCH resent] lis3: Fix Oops with NULL platform data

From: Takashi Iwai
Date: Wed Sep 22 2010 - 09:47:14 EST


At Wed, 22 Sep 2010 13:56:15 +0200,
<samu.p.onkalo@xxxxxxxxx> wrote:
>
>
> Hi
>
> >-----Original Message-----
> >From: ext Takashi Iwai [mailto:tiwai@xxxxxxx]
> >Sent: 22 September, 2010 14:34
> >To: Andrew Morton
> >Cc: Onkalo Samu.P (Nokia-MS/Tampere); Ãric Piel; linux-
> >kernel@xxxxxxxxxxxxxxx
> >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: <stable@xxxxxxxxxx>
> >Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> >---
> >
> >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.p.onkalo@xxxxxxxxx>
>
> -Samu


thanks,

Takashi

===