On Thu, Sep 23, 2010 at 01:06:22AM -0400, samu.p.onkalo@xxxxxxxxx wrote: > Added lm-sensors list... > > >-----Original Message----- > >From: ext Takashi Iwai [mailto:tiwai@xxxxxxx] > >Sent: 22 September, 2010 16:47 > >To: Onkalo Samu.P (Nokia-MS/Tampere) > >Cc: akpm@xxxxxxxxxxxxxxxxxxxx; Eric.Piel@xxxxxxxxxxxxxxxx; linux- > >kernel@xxxxxxxxxxxxxxx > >Subject: Re: [PATCH resent] lis3: Fix Oops with NULL platform data > > > >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 > > > >=== > >From 0e5bdc7516915e92fe471a77da36ecb75d995c0f Mon Sep 17 00:00:00 2001 > >From: Takashi Iwai <tiwai@xxxxxxx> > >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 <tiwai@xxxxxxx> > >--- > >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; > >-- > > > With hp_accel.c irq-thread is not needed so this is ok from that point of view. > On the other hand, wakeup / click flags are not configured without platformdata > so there is no need to run irq-thread. If the pdata is not prodived at all > there is no need to IRQ thread. > > Acked-by: Samu Onkalo <samu.p.onkalo@xxxxxxxxx> > Looks ok to me. Acked-by: Guenter Roeck <guenter.roeck@xxxxxxxxxxxx> _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
