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> _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
