[PATCH v2] lis3: Fix Oops with NULL platform data

[PATCH v2] lis3: Fix Oops with NULL platform data

[Takashi Iwai]

The recent addition of threaded irq handler causes a NULL dereference
when used with hp_accel driver, which has NULL pdata.

Acked-by: Samu Onkalo <samu.p.onkalo@nokia.com>
Acked-by: Acked-by: Guenter Roeck <guenter.roeck@ericsson.com>
Cc: <stable@kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
For 2.6.35/36 kernel

v1->v2
  NULL-check in caller of request_threaded_irq() instead of checking in
  threaded 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

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

[Guenter Roeck]

On Thu, Sep 23, 2010 at 01:58:33AM -0400, Takashi Iwai wrote:
> The recent addition of threaded irq handler causes a NULL dereference
> when used with hp_accel driver, which has NULL pdata.
> 
> Acked-by: Samu Onkalo <samu.p.onkalo@nokia.com>
> Acked-by: Acked-by: Guenter Roeck <guenter.roeck@ericsson.com>
> Cc: <stable@kernel.org>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

Applied.

Thanks,
Guenter

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

[Takashi Iwai]

At Fri, 24 Sep 2010 06:58:02 -0700,
Guenter Roeck wrote:
> 
> On Thu, Sep 23, 2010 at 01:58:33AM -0400, Takashi Iwai wrote:
> > The recent addition of threaded irq handler causes a NULL dereference
> > when used with hp_accel driver, which has NULL pdata.
> > 
> > Acked-by: Samu Onkalo <samu.p.onkalo@nokia.com>
> > Acked-by: Acked-by: Guenter Roeck <guenter.roeck@ericsson.com>
> > Cc: <stable@kernel.org>
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> 
> Applied.

Oh, does this driver belong to hwmon tree?
Jean told me that this isn't managed in hwmon, so I had an impression
that it's an orphan driver.


thanks,

Takashi

Re: [lm-sensors] [PATCH v2] lis3: Fix Oops with NULL platform data

[Jean Delvare]

On Fri, 24 Sep 2010 16:14:26 +0200, Takashi Iwai wrote:
> At Fri, 24 Sep 2010 06:58:02 -0700,
> Guenter Roeck wrote:
> > 
> > On Thu, Sep 23, 2010 at 01:58:33AM -0400, Takashi Iwai wrote:
> > > The recent addition of threaded irq handler causes a NULL dereference
> > > when used with hp_accel driver, which has NULL pdata.
> > > 
> > > Acked-by: Samu Onkalo <samu.p.onkalo@nokia.com>
> > > Acked-by: Acked-by: Guenter Roeck <guenter.roeck@ericsson.com>
> > > Cc: <stable@kernel.org>
> > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > 
> > Applied.
> 
> Oh, does this driver belong to hwmon tree?
> Jean told me that this isn't managed in hwmon, so I had an impression
> that it's an orphan driver.

Well, as long as the driver lives in drivers/hwmon and the hwmon
subsystem is maintained, it seems fair to consider the driver
maintained.

That being said, I indeed would like all non-hwmon drivers to go away
from drivers/hwmon. Originally we were waiting for iio to settle first,
but apparently this is taking forever. The 4 drivers I would like to
kick are: ams, hdaps, lis3lv02d and applesmc. They are primarily
accelerometer device drivers. Not sure where to put them,
drivers/accel(erometer), drivers/misc, drivers/misc/accel(erometer),
drivers/input/accel(erometer)... Opinions welcome.

-- 
Jean Delvare

Re: [lm-sensors] [PATCH v2] lis3: Fix Oops with NULL platform data

[Jonathan Cameron]

On 09/24/10 15:30, Jean Delvare wrote:
> On Fri, 24 Sep 2010 16:14:26 +0200, Takashi Iwai wrote:
>> At Fri, 24 Sep 2010 06:58:02 -0700,
>> Guenter Roeck wrote:
>>>
>>> On Thu, Sep 23, 2010 at 01:58:33AM -0400, Takashi Iwai wrote:
>>>> The recent addition of threaded irq handler causes a NULL dereference
>>>> when used with hp_accel driver, which has NULL pdata.
>>>>
>>>> Acked-by: Samu Onkalo <samu.p.onkalo@nokia.com>
>>>> Acked-by: Acked-by: Guenter Roeck <guenter.roeck@ericsson.com>
>>>> Cc: <stable@kernel.org>
>>>> Signed-off-by: Takashi Iwai <tiwai@suse.de>
>>>
>>> Applied.
>>
>> Oh, does this driver belong to hwmon tree?
>> Jean told me that this isn't managed in hwmon, so I had an impression
>> that it's an orphan driver.
> 
> Well, as long as the driver lives in drivers/hwmon and the hwmon
> subsystem is maintained, it seems fair to consider the driver
> maintained.
Technically hwmon is orphan (according to MAINTAINERS.) But that's not to
say that various people aren't looking after various parts of it and
I believe Andrew Morton is acting as maintainer of last resort.
> 
> That being said, I indeed would like all non-hwmon drivers to go away
> from drivers/hwmon. Originally we were waiting for iio to settle first,
> but apparently this is taking forever.
More developers and code reviewers always welcome!
> The 4 drivers I would like to
> kick are: ams, hdaps, lis3lv02d and applesmc. They are primarily
> accelerometer device drivers. Not sure where to put them,
> drivers/accel(erometer), drivers/misc, drivers/misc/accel(erometer),
> drivers/input/accel(erometer)... Opinions welcome.
If the primary use is as an input device then talk to Dmitry. Some
accelerometers are being taken into input. There are some open questions
on naming etc currently being debated so anyone interested should get
involved in those asap.

Jonathan

Re: [lm-sensors] [PATCH v2] lis3: Fix Oops with NULL platform data

[Guenter Roeck]

On Fri, Sep 24, 2010 at 11:10:06AM -0400, Jonathan Cameron wrote:
[...]
> Technically hwmon is orphan (according to MAINTAINERS.) But that's not to

If you pull the latest tree, you'll notice that this is no longer correct.

Thanks,
Guenter

Re: [lm-sensors] [PATCH v2] lis3: Fix Oops with NULL platform data

[Jonathan Cameron]

On 09/24/10 16:17, Guenter Roeck wrote:
> On Fri, Sep 24, 2010 at 11:10:06AM -0400, Jonathan Cameron wrote:
> [...]
>> Technically hwmon is orphan (according to MAINTAINERS.) But that's not to
> 
> If you pull the latest tree, you'll notice that this is no longer correct.
Excellent!  Glad you two have stepped up, sorry for the misinformation.

I may well have missed it, but if not perhaps a general announce email to the
lm-sensors list to announce it may be worth while to alert anyone like me who
is working on slightly aging branches? (staging-next :)

Re: [lm-sensors] [PATCH v2] lis3: Fix Oops with NULL platform data

[Jean Delvare]

On Fri, 24 Sep 2010 18:07:34 +0100, Jonathan Cameron wrote:
> On 09/24/10 16:17, Guenter Roeck wrote:
> > On Fri, Sep 24, 2010 at 11:10:06AM -0400, Jonathan Cameron wrote:
> > [...]
> >> Technically hwmon is orphan (according to MAINTAINERS.) But that's not to
> > 
> > If you pull the latest tree, you'll notice that this is no longer correct.
> Excellent!  Glad you two have stepped up, sorry for the misinformation.
> 
> I may well have missed it, but if not perhaps a general announce email to the
> lm-sensors list to announce it may be worth while to alert anyone like me who
> is working on slightly aging branches? (staging-next :)

It was posted on the list, but not under a subject line you would notice:
http://lists.lm-sensors.org/pipermail/lm-sensors/2010-September/029405.html

I should have updated the subject line before replying, sorry about that.

-- 
Jean Delvare

Re: [lm-sensors] [PATCH v2] lis3: Fix Oops with NULL platform data

[Jean Delvare]

On Fri, 24 Sep 2010 16:10:06 +0100, Jonathan Cameron wrote:
> On 09/24/10 15:30, Jean Delvare wrote:
> > That being said, I indeed would like all non-hwmon drivers to go away
> > from drivers/hwmon. Originally we were waiting for iio to settle first,
> > but apparently this is taking forever.
> More developers and code reviewers always welcome!

Just to clear any confusion: this wasn't a criticism from my side. I
understand why it takes time. All I mean is that it is not reasonable
to delay other changes waiting for IIO to hit mainline. Because we have
no idea when that will happen.

> > The 4 drivers I would like to
> > kick are: ams, hdaps, lis3lv02d and applesmc. They are primarily
> > accelerometer device drivers. Not sure where to put them,
> > drivers/accel(erometer), drivers/misc, drivers/misc/accel(erometer),
> > drivers/input/accel(erometer)... Opinions welcome.
> If the primary use is as an input device then talk to Dmitry. Some
> accelerometers are being taken into input. There are some open questions
> on naming etc currently being debated so anyone interested should get
> involved in those asap.

Let's see if he wants them. I seem to recall he objected to them going
to input, on the basis that the input function was a not the key
feature. But apparently we have one accel driver in drivers/input/misc
(adxl34x) so maybe we can have more.

-- 
Jean Delvare

Re: [lm-sensors] [PATCH v2] lis3: Fix Oops with NULL platform data

[Jonathan Cameron]

On 09/25/10 12:01, Jean Delvare wrote:
> On Fri, 24 Sep 2010 16:10:06 +0100, Jonathan Cameron wrote:
>> On 09/24/10 15:30, Jean Delvare wrote:
>>> That being said, I indeed would like all non-hwmon drivers to go away
>>> from drivers/hwmon. Originally we were waiting for iio to settle first,
>>> but apparently this is taking forever.
>> More developers and code reviewers always welcome!
> 
> Just to clear any confusion: this wasn't a criticism from my side. I
> understand why it takes time. All I mean is that it is not reasonable
> to delay other changes waiting for IIO to hit mainline. Because we have
> no idea when that will happen.
Not to worry ;)  I was just taking advantage to drop in a plug.
It's certainly going slower than I would like and I'm fully in favour
of drivers going into misc or similar even if we would like to
eventually take them into IIO. This is part of the reason we
have been trying to pin down interface naming etc in a more
general forum than our internal list.   Basically we are trying to
avoid a repeat of the ALS debacle. 
> 
>>> The 4 drivers I would like to
>>> kick are: ams, hdaps, lis3lv02d and applesmc. They are primarily
>>> accelerometer device drivers. Not sure where to put them,
>>> drivers/accel(erometer), drivers/misc, drivers/misc/accel(erometer),
>>> drivers/input/accel(erometer)... Opinions welcome.
>> If the primary use is as an input device then talk to Dmitry. Some
>> accelerometers are being taken into input. There are some open questions
>> on naming etc currently being debated so anyone interested should get
>> involved in those asap.
> 
> Let's see if he wants them. I seem to recall he objected to them going
> to input, on the basis that the input function was a not the key
> feature. But apparently we have one accel driver in drivers/input/misc
> (adxl34x) so maybe we can have more.
Yup, Dmitry has softened somewhat on this point for device where it really
can be argued that input is the key feature (typically the ones being used
for exactly this purpose in smart phones) in recent months so things may
have changed.

Lets wait and see.

Jonathan

Re: [lm-sensors] [PATCH v2] lis3: Fix Oops with NULL platform data

[Guenter Roeck]

On Fri, Sep 24, 2010 at 10:30:05AM -0400, Jean Delvare wrote:
> On Fri, 24 Sep 2010 16:14:26 +0200, Takashi Iwai wrote:
> > At Fri, 24 Sep 2010 06:58:02 -0700,
> > Guenter Roeck wrote:
> > > 
> > > On Thu, Sep 23, 2010 at 01:58:33AM -0400, Takashi Iwai wrote:
> > > > The recent addition of threaded irq handler causes a NULL dereference
> > > > when used with hp_accel driver, which has NULL pdata.
> > > > 
> > > > Acked-by: Samu Onkalo <samu.p.onkalo@nokia.com>
> > > > Acked-by: Acked-by: Guenter Roeck <guenter.roeck@ericsson.com>
> > > > Cc: <stable@kernel.org>
> > > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > > 
> > > Applied.
> > 
> > Oh, does this driver belong to hwmon tree?
> > Jean told me that this isn't managed in hwmon, so I had an impression
> > that it's an orphan driver.
> 
> Well, as long as the driver lives in drivers/hwmon and the hwmon
> subsystem is maintained, it seems fair to consider the driver
> maintained.
> 
I absolutely second that. Otherwise things would get too complicated.

Guenter

Re: [lm-sensors] [PATCH v2] lis3: Fix Oops with NULL platform data

[Éric Piel]

Op 24-09-10 16:30, Jean Delvare schreef:
> Well, as long as the driver lives in drivers/hwmon and the hwmon
> subsystem is maintained, it seems fair to consider the driver
> maintained.
> 
> That being said, I indeed would like all non-hwmon drivers to go away
> from drivers/hwmon. Originally we were waiting for iio to settle first,
> but apparently this is taking forever. The 4 drivers I would like to
> kick are: ams, hdaps, lis3lv02d and applesmc. They are primarily
> accelerometer device drivers. Not sure where to put them,
> drivers/accel(erometer), drivers/misc, drivers/misc/accel(erometer),
> drivers/input/accel(erometer)... Opinions welcome.
As maintainer of the lis3lv02d driver, I'd completely support the move too.

IMHO, an accelerometer is just another input device (usually with 2 or 3
axes), so moving them to drivers/input/accel would make sense.

Eric

Re: [lm-sensors] [PATCH v2] lis3: Fix Oops with NULL platform data

[Jonathan Cameron]

On 09/24/10 16:22, Éric Piel wrote:
> Op 24-09-10 16:30, Jean Delvare schreef:
>> Well, as long as the driver lives in drivers/hwmon and the hwmon
>> subsystem is maintained, it seems fair to consider the driver
>> maintained.
>>
>> That being said, I indeed would like all non-hwmon drivers to go away
>> from drivers/hwmon. Originally we were waiting for iio to settle first,
>> but apparently this is taking forever. The 4 drivers I would like to
>> kick are: ams, hdaps, lis3lv02d and applesmc. They are primarily
>> accelerometer device drivers. Not sure where to put them,
>> drivers/accel(erometer), drivers/misc, drivers/misc/accel(erometer),
>> drivers/input/accel(erometer)... Opinions welcome.
> As maintainer of the lis3lv02d driver, I'd completely support the move too.
> 
> IMHO, an accelerometer is just another input device (usually with 2 or 3
> axes),
That is not actually all that true in general. There are an awful lot of parts
that aren't even vaguely designed for 'human' input.
In some restricted cases I'm happy to agree though and that definitely includes
3 axis sensors with input specific features like those your driver covers.

> so moving them to drivers/input/accel would make sense.
Propose a move on linux-input and see what the response is.  Currently
they are going in drivers/input/misc, but that is probably just because there
aren't enough of them yet to justify their own directory.

Dmitry is taking them slowly (mainly I think because he wants the interfaces to
be more pinned down).   The adxl34x driver is already there, Hemanth's
cma3000 is close (either dropping some knobs in sysfs or working out
naming that everyone interested can agree on).  I would imagine the awkward
bits will be things like freefall detectors that no one can reasonably argue
are a conventional form of 'human' input.

As an aside, we are trying to work out a unified interface between IIO and
input for some elements and a userspace bridge where that doesn't make sense.
There are ongoing (if rather quiet) discussions on this on linux-input / lkml.
http://lkml.org/lkml/2010/9/21/167 I would certainly welcome any comments / 
suggestions anyone would like to make.  The intent here is to pin down the
naming conventions as independently as possible of the actual subsystem handling
the device, but to keep them general enough as to be able to cover a wide range
of different sensors.
 
Cheers,

Jonathan