Le lundi 03 juillet 2006 ? 00:26 +0200, Michael Hanselmann a ?crit :
> [AMS patch]
Okay, I've tried the driver and it oopsed on rmmod:
Unable to handle kernel paging request for data at address 0x00000000
Faulting instruction address: 0xc01cfd1c
Oops: Kernel access of bad area, sig: 11 [#1]
Modules linked in: ams nfsd exportfs lockd sunrpc appletouch bcm43xx ohci1394 ie ee80211softmac ieee80211 pcmcia ieee80211_crypt yenta_socket rsrc_nonstatic pcmc ia_core ohci_hcd ehci_hcd
NIP: C01CFD1C LR: C01CFCFC CTR: C01CFC6C
REGS: c48b9d90 TRAP: 0300 Not tainted (2.6.17.2)
MSR: 00009032 <EE,ME,IR,DR> CR: 22004422 XER: 20000000
DAR: 00000000, DSISR: 42000000
TASK = dd79b340[26173] 'rmmod' THREAD: c48b8000
GPR00: 00100100 C48B9E40 DD79B340 C0CDA828 C00F4CD0 00000001 C48B8000 F22451F8
GPR08: 00000000 00200200 00000000 F2245238 00000000 1001A188 100D0000 00000000
GPR16: 00000000 10104308 100D0000 100B0000 100D0000 100B0000 10013008 00000000
GPR24: C0380000 C0380000 EFFCCC88 C0350000 C0CDA828 F22450D0 00000000 F22450B8
NIP [C01CFD1C] i2c_detach_client+0xb0/0x16c
LR [C01CFCFC] i2c_detach_client+0x90/0x16c
Call Trace:
[C48B9E40] [DDF8077C] 0xddf8077c (unreliable)
[C48B9E60] [F2242984] ams_i2c_detach+0x20/0x60 [ams]
[C48B9E80] [C01D1024] i2c_del_driver+0x60/0x1d0
[C48B9EB0] [F22429DC] ams_i2c_exit+0x18/0x28 [ams]
[C48B9EC0] [F2242428] cleanup_module+0x58/0xd4 [ams]
[C48B9ED0] [C0045C70] sys_delete_module+0x180/0x1e8
[C48B9F40] [C000EE7C] ret_from_syscall+0x0/0x38
--- Exception: c01 at 0xff7320c
LR = 0x1000103c
Instruction dump:
3b9c0028 3bbf0018 7f83e378 480c4839 38ff0140 815f0140 3c000010 81070004
60000100 3d200020 397f0180 61290200 <91480000> 3c80c038 7fa3eb78 901f0140
I haven't had the time too look into the issue yet.
Below are a few comments on the patch:
> + /* calibrated null values */
> + int xcalib, ycalib;
Add zcalib too for movement on z-axis.
> + input_report_abs(ams.idev, ABS_X, x - ams.xcalib);
> + input_report_abs(ams.idev, ABS_Y, y - ams.ycalib);
Ditto, ABS_Z...
> + input_set_abs_params(ams.idev, ABS_X, -50, 50, 3, 0);
> + input_set_abs_params(ams.idev, ABS_Y, -50, 50, 3, 0);
And here...
> +static ssize_t ams_mouse_store_mouse(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + int oldmouse = mouse;
> +
> + if (sscanf(buf, "%d\n", &mouse) != 1)
> + return -EINVAL;
> +
> + mouse = !!mouse;
> +
> + if (oldmouse != mouse) {
> + mutex_lock(&ams.lock);
> +
> + if (mouse)
> + ams_mouse_enable();
> + else
> + ams_mouse_disable();
> +
> + mutex_unlock(&ams.lock);
> + }
No need for oldmouse here. Just call ams_mouse_enable() or disable() in
every case.
> +
> +static DEVICE_ATTR(mouse, S_IRUGO | S_IWUSR,
> + ams_mouse_show_mouse, ams_mouse_store_mouse);
I would prefer three different files for x, y and z instead of a single
one...
> + result = ams_sensor_attach();
> + if (result < 0) {
> + goto exit;
> + }
No need for braces.
> +config SENSORS_AMS_MOUSE
> + bool "Support for mouse mode with motion sensor"
> + depends on SENSORS_AMS && INPUT
> + help
> + Support for mouse emulation with motion sensor.
Is it really necessary to have a config option here ?
Stelian.
--
Stelian Pop <stelian at popies.net>
