lkml.org 
[lkml]   [2006]   [May]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch 03/12] input: new force feedback interface
    Randy.Dunlap wrote:
    > On Tue, 30 May 2006 13:57:08 +0300 Anssi Hannula wrote:
    >

    Thanks for reviewing.


    >> drivers/input/Kconfig | 5
    >> drivers/input/Makefile | 1
    >> drivers/input/evdev.c | 30 -
    >> drivers/input/ff-effects.c | 894 +++++++++++++++++++++++++++++++++++++++++++++
    >> drivers/input/input.c | 12
    >> include/linux/input.h | 81 ++++
    >> 6 files changed, 1006 insertions(+), 17 deletions(-)
    >>
    >>Index: linux-2.6.17-rc4-git12/include/linux/input.h
    >>===================================================================
    >>--- linux-2.6.17-rc4-git12.orig/include/linux/input.h 2006-05-27 02:28:33.000000000 +0300
    >>+++ linux-2.6.17-rc4-git12/include/linux/input.h 2006-05-27 03:06:34.000000000 +0300
    >>@@ -1000,6 +1003,84 @@ struct input_handle {
    >>+
    >>+#ifdef CONFIG_INPUT_FF_EFFECTS
    >>+int input_ff_allocate(struct input_dev *dev);
    >>+int input_ff_register(struct input_dev *dev, struct ff_driver *driver);
    >>+int input_ff_upload(struct input_dev *dev, struct ff_effect *effect,
    >>+ struct file *file);
    >>+int input_ff_erase(struct input_dev *dev, int effect_id, struct file *file);
    >>+int input_ff_event(struct input_dev *dev, unsigned int type,
    >>+ unsigned int code, int value);
    >>+#else
    >>+static inline int input_ff_allocate(struct input_dev *dev)
    >>+{
    >>+ return -EPERM;
    >>+}
    >
    >
    > -ENOSYS seems more reasonable to me (above + below).
    >

    Okay, I'll send a patch to change -EPERM back to -ENOSYS.

    I think the -EINVAL should be left as -EINVAL because they get called
    only when the user performs upload or erase ioctls on device that is not
    marked with EV_FF.

    >>+static inline int input_ff_register(struct input_dev *dev,
    >>+ struct ff_driver *driver)
    >>+{
    >>+ return -EPERM;
    >>+}
    >>+static inline int input_ff_upload(struct input_dev *dev,
    >>+ struct ff_effect *effect, struct file *file)
    >>+{
    >>+ return -EINVAL;
    >>+}
    >>+static inline int input_ff_erase(struct input_dev *dev, int effect_id,
    >>+ struct file *file)
    >>+{
    >>+ return -EINVAL;
    >>+}
    >>+static inline int input_ff_event(struct input_dev *dev, unsigned int type,
    >>+ unsigned int code, int value) { }
    >>+#endif
    >>+
    >>+
    >> static inline void init_input_dev(struct input_dev *dev)
    >> {
    >> INIT_LIST_HEAD(&dev->h_list);
    >
    >
    >>Index: linux-2.6.17-rc4-git12/drivers/input/ff-effects.c
    >>===================================================================
    >>--- /dev/null 1970-01-01 00:00:00.000000000 +0000
    >>+++ linux-2.6.17-rc4-git12/drivers/input/ff-effects.c 2006-05-27 03:06:56.000000000 +0300
    >>@@ -0,0 +1,894 @@
    >>+/*
    >>+ * Force feedback support for Linux input subsystem
    >>+ *
    >>+ * Copyright (c) 2006 Anssi Hannula <anssi.hannula@gmail.com>
    >>+ */
    >>+
    >>+
    >>+/**
    >
    >
    > "/**" indicates kernel-doc format, but the rest of the function
    > comment is not in kernel-doc format. Please use kernel-doc
    > for all non-static/non-inline functions (basically for
    > public/exported symbols) and at your discretion for static
    > functions.
    > (applies to many functions here)

    Okay, I didn't know about kernel-doc. I'll adapt the comments.

    >
    >>+ * Lock the mutex and check the device has not been deleted
    >>+ */
    >>+static inline int input_ff_safe_lock(struct input_dev *dev)
    >>+{
    >>+ mutex_lock(&dev->ff_lock);
    >>+ if (dev->ff)
    >>+ return 0;
    >>+
    >>+ mutex_unlock(&dev->ff_lock);
    >>+ return 1;
    >>+}
    >>+
    >>+/**
    >>+ * Check that the effect_id is a valid effect and the user has access to it
    >>+ */
    >>+static inline int input_ff_effect_access(struct input_dev *dev, int effect_id,
    >>+ struct file *file)
    >>+{
    >>+ struct ff_device *ff = dev->ff;
    >>+
    >>+ if (effect_id >= dev->ff_effects_max || effect_id < 0
    >>+ || !test_bit(FF_EFFECT_USED, ff->effects[effect_id].flags))
    >>+ return -EINVAL;
    >>+
    >>+ if (ff->effects[effect_id].owner != file)
    >>+ return -EACCES;
    >>+
    >>+ return 0;
    >>+}
    >>+
    >
    >
    > See Documentation/kernel-doc-nano-HOWTO.txt for more info:

    Ok.

    >
    >>+/**
    >>+ * Check for the next time envelope requires an update on memoryless devices
    >>+ * @event_time: Time of the next update
    >>+ *
    >>+ * If an event is found before @event_time, @event_time is changed and the
    >>+ * functions returns 1. Otherwise it returns 0.
    >>+ */
    >>+static int input_ff_envelope_time(struct ff_effect_status *effect,
    >>+ struct ff_envelope *envelope,
    >>+ unsigned long *event_time)
    >>+{
    >>+}
    >>+
    >>+/**
    >>+ * Calculate the next time effect requires an update on memoryless devices
    >>+ * @ff: The device
    >>+ *
    >>+ * Runs through the effects and updates the timer if necessary. This function
    >>+ * should be called only when the spinlock is locked.
    >>+ */
    >>+static void input_ff_calc_timer(struct ff_device *ff)
    >>+{
    >>+ int i;
    >>+ int events = 0;
    >>+ unsigned long next_time = 0;
    >
    >
    > We generally like a blank line between data and code lines....

    Ok.

    >
    >>+ debug("calculating next timer");
    >>+ for (i = 0; i < FF_MEMLESS_EFFECTS; ++i) {
    >>+ unsigned long event_time;
    >>+ struct ff_envelope *envelope = NULL;
    >>+ int event_set = 0;
    >>+
    >>+ if (!test_bit(FF_EFFECT_STARTED, ff->effects[i].flags)) {
    >>+ if (test_bit(FF_EFFECT_PLAYING, ff->effects[i].flags)) {
    >>+ event_time = ff->effects[i].stop_at = jiffies;
    >>+ event_set = 1;
    >>+ } else
    >>+ continue;
    >>+ } else {
    >>+ if (!test_bit(FF_EFFECT_PLAYING, ff->effects[i].flags)) {
    >>+ event_time = ff->effects[i].play_at;
    >>+ event_set = 1;
    >>+ } else {
    >>+ if (ff->effects[i].effect.type == FF_PERIODIC)
    >>+ envelope =
    >>+ &ff->effects[i].effect.u.periodic.
    >>+ envelope;
    >>+ else if (ff->effects[i].effect.type ==
    >>+ FF_CONSTANT)
    >>+ envelope =
    >>+ &ff->effects[i].effect.u.constant.
    >>+ envelope;
    >>+
    >>+ event_set =
    >>+ input_ff_envelope_time(&ff->effects[i],
    >>+ envelope,
    >>+ &event_time);
    >>+ if (!event_set
    >>+ && ff->effects[i].effect.replay.length) {
    >>+ event_time = ff->effects[i].stop_at;
    >>+ event_set = 1;
    >>+ }
    >>+ }
    >>+ }
    >>+
    >>+ if (!event_set)
    >>+ continue;
    >>+ events++;
    >>+
    >>+ if (time_after(jiffies, event_time)) {
    >>+ event_time = jiffies;
    >>+ break;
    >>+ }
    >>+ if (events == 1)
    >>+ next_time = event_time;
    >>+ else {
    >>+ if (time_after(next_time, event_time))
    >>+ next_time = event_time;
    >>+ }
    >>+ }
    >>+ if (!events) {
    >>+ debug("no actions");
    >>+ del_timer(&ff->timer);
    >>+ } else {
    >>+ debug("timer set");
    >>+ mod_timer(&ff->timer, next_time);
    >>+ }
    >>+}
    >>+
    >>+
    >>+
    >>+/**
    >>+ * Safe sum
    >>+ * @a: Integer to sum
    >>+ * @b: Integer to sum
    >>+ * @limit: The sum limit
    >>+ *
    >>+ * If @a+@b is above @limit, return @limit
    >
    >
    > unless a == 0, then return b, even if b > limit
    >

    Right.

    >>+ */
    >>+static int input_ff_safe_sum(int a, int b, int limit)
    >>+{
    >>+ int c;
    >>+ if (!a)
    >>+ return b;
    >>+ c = a + b;
    >>+ if (c > limit)
    >>+ return limit;
    >>+ return c;
    >>+}
    >>+
    >>+/**
    >>+ * Convert an effect (for devices with memory)
    >>+ */
    >>+static void input_ff_convert_effect(struct input_dev *dev,
    >>+ struct ff_effect *effect)
    >>+{
    >>+ int strong_magnitude, weak_magnitude;
    >>+
    >>+ if (effect->type == FF_RUMBLE && test_bit(FF_PERIODIC, dev->ff->flags)) {
    >>+ /* Strong magnitude as 2/3 full and weak 1/3 full */
    >>+ /* Also divide by 2 */
    >
    >
    > why? Comments should explain why, not just what.

    Okay, will add.

    >
    >>+ strong_magnitude = effect->u.rumble.strong_magnitude * 2 / 6;
    >>+ weak_magnitude = effect->u.rumble.weak_magnitude / 6;
    >>+
    >>+ effect->type = FF_PERIODIC;
    >>+
    >>+ effect->u.periodic.waveform = FF_SINE;
    >>+ effect->u.periodic.period = 50;
    >>+ effect->u.periodic.magnitude =
    >>+ input_ff_safe_sum(strong_magnitude, weak_magnitude, 0x7fff);
    >>+ effect->u.periodic.offset = 0;
    >>+ effect->u.periodic.phase = 0;
    >>+ effect->u.periodic.envelope.attack_length = 0;
    >>+ effect->u.periodic.envelope.attack_level = 0;
    >>+ effect->u.periodic.envelope.fade_length = 0;
    >>+ effect->u.periodic.envelope.fade_level = 0;
    >>+ return;
    >>+ }
    >>+
    >>+ printk(KERN_ERR
    >>+ "ff-effects: invalid effect type in convert_effect()\n");
    >>+}
    >>+
    >>+
    >>+
    >>+/**
    >>+ * Erase an effect
    >>+ * @file: The requester
    >
    >
    > needs all parameters listed/explained

    Ok.

    >
    >>+ *
    >>+ * Erases the effect if the requester is also the effect owner. The mutex
    >>+ * should already be locked before calling this function. If the device is a
    >>+ * memoryless device, the spinlock will be locked.
    >>+ */
    >>+static int _input_ff_erase(struct input_dev *dev, int effect_id,
    >>+ struct file *file)
    >>+{
    >>+ unsigned long flags;
    >>+ int ret = input_ff_effect_access(dev, effect_id, file);
    >>+ if (ret)
    >>+ return ret;
    >>+ if (dev->ff->driver->playback) {
    >>+ dev->ff->driver->playback(dev, effect_id, 0);
    >>+ ret = dev->ff->driver->erase(dev, effect_id);
    >>+ if (!ret)
    >>+ __clear_bit(FF_EFFECT_USED,
    >>+ dev->ff->effects[effect_id].flags);
    >>+ } else {
    >>+ spin_lock_irqsave(&dev->ff->atomiclock, flags);
    >>+ if (__test_and_clear_bit
    >>+ (FF_EFFECT_STARTED, dev->ff->effects[effect_id].flags))
    >>+ input_ff_calc_timer(dev->ff);
    >>+ spin_unlock_irqrestore(&dev->ff->atomiclock, flags);
    >>+ __clear_bit(FF_EFFECT_USED, dev->ff->effects[effect_id].flags);
    >>+ }
    >>+ return ret;
    >>+}
    >>+
    >>+
    >>+/**
    >>+ * Set the bits and handlers
    >>+ * @driver: The ff_driver struct
    >>+ *
    >>+ * If possible, this function should be called before the input device is
    >>+ * registered. It can however be ran again if the capability bits have
    >
    >
    > s/ran/run/
    >

    Will change.

    >>+ * changed.
    >>+ */
    >>+int input_ff_register(struct input_dev *dev, struct ff_driver *driver)
    >>+{
    >


    --
    Anssi Hannula

    -
    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: 2006-05-31 12:16    [W:0.059 / U:29.776 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site