Messages in this thread | | | Date | Fri, 26 Aug 2005 14:27:37 -0500 | From | Dmitry Torokhov <> | Subject | Re: [patch] IBM HDAPS accelerometer driver. |
| |
On 8/26/05, Robert Love <rml@novell.com> wrote: > +/* device class stuff */ > + > +static DECLARE_COMPLETION(hdaps_obj_is_free); > +static void hdaps_release_dev(struct device *dev) > +{ > + complete(&hdaps_obj_is_free); > +} > +
What this completion is used for? I don't see any other references to it.
> + > +static void hdaps_mousedev_poll(unsigned long unused) > +{ > + int movex, movey, x, y, ret; > + > + ret = accelerometer_read_pair(HDAPS_PORT_XPOS, HDAPS_PORT_YPOS, &x, &y); > + if (unlikely(ret)) > + return; > + > + movex = rest_x - x; > + movey = rest_y - y; > + if (abs(movex) > hdaps_mousedev_threshold) > + input_report_rel(&hdaps_idev, REL_Y, movex); > + if (abs(movey) > hdaps_mousedev_threshold) > + input_report_rel(&hdaps_idev, REL_X, movey); > + input_sync(&hdaps_idev); > + > + mod_timer(&hdaps_poll_timer, jiffies + msecs_to_jiffies(hdaps_poll_ms)); > +} > +
I'd rather you used absolute coordinates and set up hdaps_idev->absfuzz to do the filtering.
> +static ssize_t hdaps_mousedev_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + int enable; > + > + if (sscanf(buf, "%d\n", &enable) != 1) > + return -EINVAL; > + > + if (enable == 1) > + hdaps_mousedev_enable(); > + else if (enable == 0) > + hdaps_mousedev_disable(); > + > + return count; > +} > +
This is racy - 2 threads can try to do this simultaneously.
> + > + device_create_file(&hdaps_plat_dev.dev, &dev_attr_position); > + device_create_file(&hdaps_plat_dev.dev, &dev_attr_variance); > + device_create_file(&hdaps_plat_dev.dev, &dev_attr_temp); > + device_create_file(&hdaps_plat_dev.dev, &dev_attr_calibrate); > + device_create_file(&hdaps_plat_dev.dev, &dev_attr_mousedev); > + device_create_file(&hdaps_plat_dev.dev, &dev_attr_mousedev_threshold); > + device_create_file(&hdaps_plat_dev.dev, &dev_attr_mousedev_poll_ms); > +
What about using sysfs_attribute_group?
-- Dmitry - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |