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 <[email protected]>
>>+ */
>>+
>>+
>>+/**
>
>
> "/**" 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
