* [PATCH resent] lis3: Fix Oops with NULL platform data @ 2010-09-22 11:34 Takashi Iwai 2010-09-22 11:37 ` Takashi Iwai 2010-09-22 11:56 ` samu.p.onkalo 0 siblings, 2 replies; 8+ messages in thread From: Takashi Iwai @ 2010-09-22 11:34 UTC (permalink / raw) To: Andrew Morton; +Cc: Samu Onkalo, Éric Piel, linux-kernel The recent addition of threaded irq handler causes a NULL dereference when used with hp_accel driver, which has NULL pdata. Cc: <stable@kernel.org> Signed-off-by: Takashi Iwai <tiwai@suse.de> --- 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); -- 1.7.2.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH resent] lis3: Fix Oops with NULL platform data 2010-09-22 11:34 [PATCH resent] lis3: Fix Oops with NULL platform data Takashi Iwai @ 2010-09-22 11:37 ` Takashi Iwai 2010-09-22 11:56 ` samu.p.onkalo 1 sibling, 0 replies; 8+ messages in thread From: Takashi Iwai @ 2010-09-22 11:37 UTC (permalink / raw) To: Andrew Morton; +Cc: Samu Onkalo, Éric Piel, linux-kernel Sorry, wrongly tagged as "resent". This is a first post. A resent patch was the previous one, <s5hwrqe816g.wl%tiwai@suse.de>. Takashi ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH resent] lis3: Fix Oops with NULL platform data 2010-09-22 11:34 [PATCH resent] lis3: Fix Oops with NULL platform data Takashi Iwai 2010-09-22 11:37 ` Takashi Iwai @ 2010-09-22 11:56 ` samu.p.onkalo 2010-09-22 13:47 ` Takashi Iwai 1 sibling, 1 reply; 8+ messages in thread From: samu.p.onkalo @ 2010-09-22 11:56 UTC (permalink / raw) To: tiwai, akpm; +Cc: Eric.Piel, linux-kernel 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: <stable@kernel.org> >Signed-off-by: Takashi Iwai <tiwai@suse.de> >--- > >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. Eric, is it so that in laptops interrupts are used for free fall detection and polled input device is for joystick? 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@nokia.com> -Samu ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH resent] lis3: Fix Oops with NULL platform data 2010-09-22 11:56 ` samu.p.onkalo @ 2010-09-22 13:47 ` Takashi Iwai 2010-09-23 5:06 ` [PATCH resent] lis3: Fix Oops with NULL platform data samu.p.onkalo 0 siblings, 1 reply; 8+ messages in thread From: Takashi Iwai @ 2010-09-22 13:47 UTC (permalink / raw) To: samu.p.onkalo; +Cc: akpm, Eric.Piel, linux-kernel At Wed, 22 Sep 2010 13:56:15 +0200, <samu.p.onkalo@nokia.com> 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: <stable@kernel.org> > >Signed-off-by: Takashi Iwai <tiwai@suse.de> > >--- > > > >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@nokia.com> > > -Samu thanks, Takashi === >From 0e5bdc7516915e92fe471a77da36ecb75d995c0f Mon Sep 17 00:00:00 2001 From: Takashi Iwai <tiwai@suse.de> 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@suse.de> --- 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [lm-sensors] [PATCH resent] lis3: Fix Oops with NULL platform 2010-09-22 13:47 ` Takashi Iwai @ 2010-09-23 5:06 ` samu.p.onkalo 0 siblings, 0 replies; 8+ messages in thread From: samu.p.onkalo @ 2010-09-23 5:06 UTC (permalink / raw) To: tiwai; +Cc: akpm, Eric.Piel, linux-kernel, lm-sensors Added lm-sensors list... >-----Original Message----- >From: ext Takashi Iwai [mailto:tiwai@suse.de] >Sent: 22 September, 2010 16:47 >To: Onkalo Samu.P (Nokia-MS/Tampere) >Cc: akpm@linux-foundation.org; Eric.Piel@tremplin-utc.net; linux- >kernel@vger.kernel.org >Subject: Re: [PATCH resent] lis3: Fix Oops with NULL platform data > >At Wed, 22 Sep 2010 13:56:15 +0200, ><samu.p.onkalo@nokia.com> 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: <stable@kernel.org> >> >Signed-off-by: Takashi Iwai <tiwai@suse.de> >> >--- >> > >> >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@nokia.com> >> >> -Samu > > >thanks, > >Takashi > >=>From 0e5bdc7516915e92fe471a77da36ecb75d995c0f Mon Sep 17 00:00:00 2001 >From: Takashi Iwai <tiwai@suse.de> >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@suse.de> >--- >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@nokia.com> _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH resent] lis3: Fix Oops with NULL platform data @ 2010-09-23 5:06 ` samu.p.onkalo 0 siblings, 0 replies; 8+ messages in thread From: samu.p.onkalo @ 2010-09-23 5:06 UTC (permalink / raw) To: tiwai; +Cc: akpm, Eric.Piel, linux-kernel, lm-sensors [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4952 bytes --] Added lm-sensors list... >-----Original Message----- >From: ext Takashi Iwai [mailto:tiwai@suse.de] >Sent: 22 September, 2010 16:47 >To: Onkalo Samu.P (Nokia-MS/Tampere) >Cc: akpm@linux-foundation.org; Eric.Piel@tremplin-utc.net; linux- >kernel@vger.kernel.org >Subject: Re: [PATCH resent] lis3: Fix Oops with NULL platform data > >At Wed, 22 Sep 2010 13:56:15 +0200, ><samu.p.onkalo@nokia.com> 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: <stable@kernel.org> >> >Signed-off-by: Takashi Iwai <tiwai@suse.de> >> >--- >> > >> >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@nokia.com> >> >> -Samu > > >thanks, > >Takashi > >=== >From 0e5bdc7516915e92fe471a77da36ecb75d995c0f Mon Sep 17 00:00:00 2001 >From: Takashi Iwai <tiwai@suse.de> >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@suse.de> >--- >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@nokia.com> ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [lm-sensors] [PATCH resent] lis3: Fix Oops with NULL platform 2010-09-23 5:06 ` [PATCH resent] lis3: Fix Oops with NULL platform data samu.p.onkalo @ 2010-09-23 5:33 ` Guenter Roeck -1 siblings, 0 replies; 8+ messages in thread From: Guenter Roeck @ 2010-09-23 5:33 UTC (permalink / raw) To: samu.p.onkalo@nokia.com Cc: tiwai@suse.de, akpm@linux-foundation.org, Eric.Piel@tremplin-utc.net, linux-kernel@vger.kernel.org, lm-sensors@lm-sensors.org On Thu, Sep 23, 2010 at 01:06:22AM -0400, samu.p.onkalo@nokia.com wrote: > Added lm-sensors list... > > >-----Original Message----- > >From: ext Takashi Iwai [mailto:tiwai@suse.de] > >Sent: 22 September, 2010 16:47 > >To: Onkalo Samu.P (Nokia-MS/Tampere) > >Cc: akpm@linux-foundation.org; Eric.Piel@tremplin-utc.net; linux- > >kernel@vger.kernel.org > >Subject: Re: [PATCH resent] lis3: Fix Oops with NULL platform data > > > >At Wed, 22 Sep 2010 13:56:15 +0200, > ><samu.p.onkalo@nokia.com> 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: <stable@kernel.org> > >> >Signed-off-by: Takashi Iwai <tiwai@suse.de> > >> >--- > >> > > >> >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@nokia.com> > >> > >> -Samu > > > > > >thanks, > > > >Takashi > > > >=> >From 0e5bdc7516915e92fe471a77da36ecb75d995c0f Mon Sep 17 00:00:00 2001 > >From: Takashi Iwai <tiwai@suse.de> > >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@suse.de> > >--- > >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@nokia.com> > Looks ok to me. Acked-by: Guenter Roeck <guenter.roeck@ericsson.com> _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [lm-sensors] [PATCH resent] lis3: Fix Oops with NULL platform data @ 2010-09-23 5:33 ` Guenter Roeck 0 siblings, 0 replies; 8+ messages in thread From: Guenter Roeck @ 2010-09-23 5:33 UTC (permalink / raw) To: samu.p.onkalo@nokia.com Cc: tiwai@suse.de, akpm@linux-foundation.org, Eric.Piel@tremplin-utc.net, linux-kernel@vger.kernel.org, lm-sensors@lm-sensors.org On Thu, Sep 23, 2010 at 01:06:22AM -0400, samu.p.onkalo@nokia.com wrote: > Added lm-sensors list... > > >-----Original Message----- > >From: ext Takashi Iwai [mailto:tiwai@suse.de] > >Sent: 22 September, 2010 16:47 > >To: Onkalo Samu.P (Nokia-MS/Tampere) > >Cc: akpm@linux-foundation.org; Eric.Piel@tremplin-utc.net; linux- > >kernel@vger.kernel.org > >Subject: Re: [PATCH resent] lis3: Fix Oops with NULL platform data > > > >At Wed, 22 Sep 2010 13:56:15 +0200, > ><samu.p.onkalo@nokia.com> 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: <stable@kernel.org> > >> >Signed-off-by: Takashi Iwai <tiwai@suse.de> > >> >--- > >> > > >> >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@nokia.com> > >> > >> -Samu > > > > > >thanks, > > > >Takashi > > > >=== > >From 0e5bdc7516915e92fe471a77da36ecb75d995c0f Mon Sep 17 00:00:00 2001 > >From: Takashi Iwai <tiwai@suse.de> > >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@suse.de> > >--- > >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@nokia.com> > Looks ok to me. Acked-by: Guenter Roeck <guenter.roeck@ericsson.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-09-23 5:35 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-09-22 11:34 [PATCH resent] lis3: Fix Oops with NULL platform data Takashi Iwai 2010-09-22 11:37 ` Takashi Iwai 2010-09-22 11:56 ` samu.p.onkalo 2010-09-22 13:47 ` Takashi Iwai 2010-09-23 5:06 ` [lm-sensors] [PATCH resent] lis3: Fix Oops with NULL platform samu.p.onkalo 2010-09-23 5:06 ` [PATCH resent] lis3: Fix Oops with NULL platform data samu.p.onkalo 2010-09-23 5:33 ` [lm-sensors] [PATCH resent] lis3: Fix Oops with NULL platform Guenter Roeck 2010-09-23 5:33 ` [lm-sensors] [PATCH resent] lis3: Fix Oops with NULL platform data Guenter Roeck
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.