linux-kernel - Re: [RFC PATCHv2 4/5] hwmon: lis3: Remove the referencies to the global variable in core driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110407215817.GB5065@nautilus.holoscopio.com>
Date:	Thu, 7 Apr 2011 18:58:17 -0300
From:	Thadeu Lima de Souza Cascardo <cascardo@...oscopio.com>
To:	Ilkka Koskinen <ikoskine@...ia.com>
Cc:	Ilkka Koskinen <ilkka.koskinen@...ia.com>,
	eric.piel@...mplin-utc.net, mjg@...hat.com,
	linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org,
	samu.p.onkalo@...ia.com, linux-input@...r.kernel.org
Subject: Re: [RFC PATCHv2 4/5] hwmon: lis3: Remove the referencies to the
 global variable in core driver

On Thu, Apr 07, 2011 at 02:59:48PM +0300, Ilkka Koskinen wrote:
> 
> Hi,
> 
> Thanks for the comments!
> 
> On Tue, 5 Apr 2011, ext Thadeu Lima de Souza Cascardo wrote:
> >On Tue, Apr 05, 2011 at 05:45:13PM +0300, Ilkka Koskinen wrote:
> >>Signed-off-by: Ilkka Koskinen <ilkka.koskinen@...ia.com>
> >>---
> >> drivers/misc/lis3lv02d/lis3lv02d.c |  237 ++++++++++++++++++++----------------
> >> drivers/misc/lis3lv02d/lis3lv02d.h |    3 +
> >> 2 files changed, 135 insertions(+), 105 deletions(-)
> 
> >>@@ -980,14 +1003,18 @@ int lis3lv02d_init_device(struct lis3lv02d *lis3)
> >> 				thread_fn,
> >> 				IRQF_TRIGGER_RISING | IRQF_ONESHOT |
> >> 				irq_flags,
> >>-				DRIVER_NAME, &lis3_dev);
> >>+				DRIVER_NAME, lis3);
> >>
> >> 	if (err < 0) {
> >> 		pr_err("Cannot get IRQ\n");
> >> 		goto out;
> >> 	}
> >>
> >>-	if (misc_register(&lis3lv02d_misc_device))
> >>+	lis3->miscdev.minor	= MISC_DYNAMIC_MINOR;
> >>+	lis3->miscdev.name	= "freefall";
> >>+	lis3->miscdev.fops	= &lis3lv02d_misc_fops;
> >>+
> >>+	if (misc_register(&lis3->miscdev))
> >> 		pr_err("misc_register failed\n");
> >
> >You should not use miscdevice in case there will be multiple devices.
> >First, it will fail. There cannot be more than one device with the same
> >name. Second, current dynamic minor devices is restricted to 64 devices.
> >Since this is reserved to one-shot devices, freefall was OK as a misc
> >device until you fixed it to allow multiple freefall devices.  :-)
> 
> Ah, true :)
> 
> >So, I'd recommend switching to a new device class, and have freefall0,
> >freefall1, etc.
> 
> I wonder if introducing a new class makes sense. I mean, I can
> figure out use cases for several accelerometers but for several free
> fall sensors? :/
> 
> Would it be better to add a mechanism to tell to the core module if
> the particular device is used for free fall detection or not?
> 
> Cheers, Ilkka
> 

I would suggest an input handler or just letting userspace handle that
using input events. Then, I checked out the code and realized that this
is done by the hardware, using an interrupt. Should we handle said
interrupt as a particular kind of event (perhaps a new misc event)?

I've written classmate-laptop driver, which has an accelerometer too
(using ACPI). Should we start documenting how accelerometer drivers
should be written? Input event devices are the best class, but I've seen
cases where drivers use sysfs files. In fact, I've just checked and seen
that lis3lv02d does that too, in the file position. But I've seen
drivers out there that use one file per axis. I guess that's the way
hwmon drivers usually do things, creating sysfs files.

Should we start standardizing these drivers? I think input devices are
good enough to report the data, including for 2-axis devices (I have one
SPI "inclinometer" here - how those should be distinguished?). sysfs
files would be nice for setting parameters. classmate-laptop, for
example, has a sensitivity parameter. Besides freefall, lis3 seems to
have a selftest function and a rate setting.

One last email in linux-input and pdx lists has requested a new MISC
event to report a change in direction for some devices. Although I think
freefall and change in direction could be computed using the
accelerometer values in userspace, these devices do it directly. How
should we handle them in a standard way?

I would like to gather some opinions before writing a draft of
recommendations for writing such drivers. Is linux-input the best forum
for this?

Regards,
Cascardo.

> >Anyway, good job on this.
> >
> >Regards,
> >Cascardo.
> 

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)