lkml.org 
[lkml]   [2005]   [Aug]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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/

    \
     
     \ /
      Last update: 2005-08-26 21:30    [W:4.904 / U:0.340 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site