Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH v2 1/4] Add ff-memless-next driver
From: Michal Malý @ 2014-02-23 23:29 UTC (permalink / raw)
  To: linux-input
  Cc: linux-kernel, dmitry.torokhov, anssi.hannula, elias.vds, jkosina,
	simon
In-Reply-To: <1516865.M993BQAYe4@geidi-prime>

Introduce ff-memless-next module as a possible future replacement of
ff-memless.

Tested-by: Elias Vanderstuyft <elias.vds@gmail.com>
Signed-off-by: Michal Malý <madcatxster@prifuk.cz>
---
 v2:
  Handle upload and removal of uncombinable effects correctly
  Remove redundant information from the documentation file
  Invert direction of force along Y axis to conform to common conventions
  Set FF_GAIN bit

 Documentation/input/ff-memless-next.txt | 141 ++++++
 drivers/input/Kconfig                   |  11 +
 drivers/input/Makefile                  |   1 +
 drivers/input/ff-memless-next.c         | 789 ++++++++++++++++++++++++++++++++
 include/linux/input/ff-memless-next.h   |  32 ++
 5 files changed, 974 insertions(+)
 create mode 100644 Documentation/input/ff-memless-next.txt
 create mode 100644 drivers/input/ff-memless-next.c
 create mode 100644 include/linux/input/ff-memless-next.h

diff --git a/Documentation/input/ff-memless-next.txt b/Documentation/input/ff-memless-next.txt
new file mode 100644
index 0000000..1b550dc
--- /dev/null
+++ b/Documentation/input/ff-memless-next.txt
@@ -0,0 +1,141 @@
+"ff-memless-next" force feedback module for memoryless devices.
+By Michal Malý <madcatxster@prifuk.cz> on 2013/12/21
+----------------------------------------------------------------------------
+
+1. Introduction
+~~~~~~~~~~~~~~~
+This document describes basic working principles of the "ff-memless-next"
+module and its purpose. This document also contains a summary
+of the "ff-memless-next" API and brief instructions on how to use it to write
+a hardware-specific backend with "ff-memless-next". Some specifics
+of ff-memless-next that might be of interest for userspace developers
+are also discussed.
+
+2. Basic principles of ff-memless-next
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+A lot of commonly used force feedback devices do not have a memory of their
+own. This means that it is not possible to upload a force feedback effect
+to such a device and let the device's CPU handle the playback. Instead,
+the device relies solely on its driver to tell it what force to generate.
+"ff-memless-next" was written to serve in this capacity. Its purpose is to
+calculate the overall force the device should apply and pass the result to
+a hardware-specific backend which then submits the appropriate command to
+the device.
+
+"ff-memless-next" differentiates between two types of force feedback effects,
+"combinable" and "uncombinable".
+"Combinable" effects are effects that generate a force of a given
+magnitude and direction. The magnitude can change in time according to the
+envelope of the effect and its waveform; the latter applies only to periodic
+effects. Force generated by "combinable" effect does not depend on the position
+of the device. Forces generated by each "combinable" effect that is active
+are periodically recalculated as needed and superposed into one overall force.
+"Combinable" effects are FF_CONSTANT, FF_PERIODIC and FF_RAMP.
+
+"Uncombinable" effects generate a force that depends on the position of
+the device. "ff-memless-next" assumes that the device takes care of processing
+such an effect. However, a device might have a limit on how many "uncombinable"
+effects can be active at once and this limit might be lower than the maximum
+effect count set in "ff-memless-next". "ff-memless-next" allows a
+hardware-specific driver to check whether the device is able to play
+an "uncombinable" effect. As of now an error during effect upload is not
+reported back to userspace. Please be prepared that this might change
+in the future.
+"Uncombinable" effects are FF_DAMPER, FF_FRICTION, FF_INERTIA and FF_SPRING.
+
+FF_CUSTOM is currently not handled by "ff-memless-next".
+
+3. API provided by "ff-memless-next"
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+"ff-memless-next" provides an API for developers of hardware-specific
+drivers. In order to use the API, the hardware-specific driver should
+include <linux/input/ff-memless-next.h>
+Functions and structs defined by this API are:
+
+int input_ff_create_mlnx(struct input_dev *dev, void *data,
+			 int(*control_effect)(struct input_dev *, void *, const struct mlnx_effect_command *),
+			 const u16 update_rate)
+- Any hardware-specific driver that wants to use "ff-memless-next" must call
+this function. The function takes care of creating and registering a force
+feedback device within the kernel. It also initializes resources needed by
+"ff-memless-next" to handle a new device. No other initialization steps are necessary.
+	Parameters:
+	* dev - pointer to valid struct input_dev
+	* data - pointer to custom data the hardware-specific backend
+		 might need to pass to the control_effect() callback function
+		 (discussed later). * data will be freed automatically by
+		 "ff-memless-next" upon device's destruction.
+	* control_effect - A callback function. "ff-memless-next" will call
+			   this function when it is done processing effects.
+			   Implementation of control_effect() in the
+			   hardware-specific driver should create an appropriate
+			   command and submit it to the device.
+			   This function is called with dev->event_lock
+			   spinlock held.
+	update_rate - Rate in milliseconds at which envelopes and periodic
+		      effects are recalculated. Minimum value is limited by the
+		      kernel timer resolution and changes with value of
+		      CONFIG_HZ.
+
+struct mlnx_effect_command
+^^^^^^^^^^^^^^^^^^^^^^^^^^
+- This struct is passed to the hardware-specific backend in
+the control_effect() function. See "ff-memless-next.h" for details.
+
+enum mlnx_commands
+^^^^^^^^^^^^^^^^^^
+- Types of commands generated by ff-memless-next
+	MLNX_START_COMBINED - Start or update "combinable" effect
+	MLNX_STOP_COMBINED - Stop "combinable" effect. In most cases this means
+			     to set the force to zero.
+	MLNX_UPLOAD_UNCOMB - Check if the device can accept and play
+			     "uncombinable" effect and upload the effect into
+			     the device's internal memory.
+			     Hardware-specific driver should return 0
+			     on success.
+	MLNX_ERASE_UNCOMB - Remove "uncombinable" effect from device's
+			    internal memory.
+			    Hardware-specific driver should return 0
+			    on success.
+	MLNX_START_UNCOMB - Start playback of "uncombinable" effect.
+	MLNX_STOP_UNCOMB - Stop playback of "uncombinable" effect.
+
+struct mlnx_simple_force
+^^^^^^^^^^^^^^^^^^^^^^^^
+	x - overall force along X axis
+	y - overall force along Y axis
+
+struct mlnx_uncomb_effect
+^^^^^^^^^^^^^^^^^^^^^^^^^
+- Information about "uncombinable" effect.
+	id - internal ID of the effect
+	* effect - pointer to the internal copy of the effect kept by
+		   "ff-memless-next". This pointer will remain valid until
+		   the effect is removed.
+
+Sample implementation of a dummy driver that uses "ff-memless-next" is
+available at "git://prifuk.cz/ff-dummy-device". Link to the source will
+be kept up to date.
+
+4. Specifics of "ff-memless-next" for userspace developers
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+None of the persons involved in development or testing of "ff-memless-next"
+is aware of any reference force feedback specifications. "ff-memless-next"
+tries to adhere to Microsoft's DirectInput specifications because we
+believe that is what most developers have experience with.
+
+- Waveforms of periodic effects do not start at the origin, but as follows:
+	SAW_UP: At minimum
+	SAW_DOWN: At maximum
+	SQUARE: At maximum
+	TRIANGLE: At maximum
+	SINE: At origin
+
+- Updating periodic effects:
+	- All periodic effects are restarted when their parameters are updated.
+
+- Updating effects of finite duration:
+	- Once an effect of finite length finishes playing, it is considered
+	  stopped. Only effects that are playing can be updated on the fly.
+	  Therefore effects of finite duration can be updated only until
+	  they finish playing.
diff --git a/drivers/input/Kconfig b/drivers/input/Kconfig
index a11ff74..ba05100 100644
--- a/drivers/input/Kconfig
+++ b/drivers/input/Kconfig
@@ -77,6 +77,17 @@ config INPUT_MATRIXKMAP
 	  To compile this driver as a module, choose M here: the
 	  module will be called matrix-keymap.
 
+config INPUT_FF_MEMLESS_NEXT
+	tristate "New version of support for memoryless force feedback devices"
+	help
+	  Say Y here if you want to enable support for various memoryless
+	  force feedback devices.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ff-memless-next.
+
 comment "Userland interfaces"
 
 config INPUT_MOUSEDEV
diff --git a/drivers/input/Makefile b/drivers/input/Makefile
index 5ca3f63..169e99c 100644
--- a/drivers/input/Makefile
+++ b/drivers/input/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_INPUT_FF_MEMLESS)	+= ff-memless.o
 obj-$(CONFIG_INPUT_POLLDEV)	+= input-polldev.o
 obj-$(CONFIG_INPUT_SPARSEKMAP)	+= sparse-keymap.o
 obj-$(CONFIG_INPUT_MATRIXKMAP)	+= matrix-keymap.o
+obj-$(CONFIG_INPUT_FF_MEMLESS_NEXT)	+= ff-memless-next.o
 
 obj-$(CONFIG_INPUT_MOUSEDEV)	+= mousedev.o
 obj-$(CONFIG_INPUT_JOYDEV)	+= joydev.o
diff --git a/drivers/input/ff-memless-next.c b/drivers/input/ff-memless-next.c
new file mode 100644
index 0000000..843a223
--- /dev/null
+++ b/drivers/input/ff-memless-next.c
@@ -0,0 +1,789 @@
+/*
+ * Force feedback support for memoryless devices
+ *
+ * This module is based on "ff-memless" orignally written by Anssi Hannula.
+ * It is extended to support all force feedback effects currently supported
+ * by the Linux input stack.
+ *
+ * Copyright(c) 2013 Michal Maly <madcatxster@prifuk.cz>
+ *
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/spinlock.h>
+#include <linux/jiffies.h>
+#include <linux/fixp-arith.h>
+#include <linux/input/ff-memless-next.h>
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Michal \"MadCatX\" Maly");
+MODULE_DESCRIPTION("Force feedback support for memoryless force feedback devices");
+
+#define FF_MAX_EFFECTS 16
+#define FF_MIN_EFFECT_LENGTH ((1000 / CONFIG_HZ) + 1)
+#define FF_EFFECT_STARTED 1
+#define FF_EFFECT_PLAYING 2
+
+
+struct mlnx_effect {
+	struct ff_effect effect;
+	unsigned long flags;
+	unsigned long begin_at;
+	unsigned long stop_at;
+	unsigned long updated_at;
+	unsigned long attack_stop;
+	unsigned long fade_begin;
+	int repeat;
+};
+
+struct mlnx_device {
+	u8 combinable_playing;
+	unsigned long update_rate_jiffies;
+	void *private;
+	struct mlnx_effect effects[FF_MAX_EFFECTS];
+	u16 gain;
+	struct timer_list timer;
+	struct input_dev *dev;
+
+	int (*control_effect)(struct input_dev *, void *,
+			      const struct mlnx_effect_command *);
+};
+
+static s32 mlnx_calculate_x_force(const s32 level,
+						  const u16 direction)
+{
+	s32 new = (level * -fixp_sin(direction)) >> FRAC_N;
+	pr_debug("x force: %d\n", new);
+	return new;
+}
+
+static s32 mlnx_calculate_y_force(const s32 level,
+						  const u16 direction)
+{
+	s32 new = (level * fixp_cos(direction)) >> FRAC_N;
+	pr_debug("y force: %d\n", new);
+	return new;
+}
+
+static bool mlnx_is_combinable(const struct ff_effect *effect)
+{
+	switch (effect->type) {
+	case FF_CONSTANT:
+	case FF_PERIODIC:
+	case FF_RAMP:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool mlnx_is_conditional(const struct ff_effect *effect)
+{
+	switch (effect->type) {
+	case FF_DAMPER:
+	case FF_FRICTION:
+	case FF_INERTIA:
+	case FF_SPRING:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static void mlnx_clr_playing(struct mlnx_effect *mlnxeff)
+{
+	__clear_bit(FF_EFFECT_PLAYING, &mlnxeff->flags);
+}
+
+static void mlnx_clr_started(struct mlnx_effect *mlnxeff)
+{
+	__clear_bit(FF_EFFECT_STARTED, &mlnxeff->flags);
+}
+
+static bool mlnx_is_playing(const struct mlnx_effect *mlnxeff)
+{
+	return test_bit(FF_EFFECT_PLAYING, &mlnxeff->flags);
+}
+
+static bool mlnx_is_started(const struct mlnx_effect *mlnxeff)
+{
+	return test_bit(FF_EFFECT_STARTED, &mlnxeff->flags);
+}
+
+static bool mlnx_test_set_playing(struct mlnx_effect *mlnxeff)
+{
+	return test_and_set_bit(FF_EFFECT_PLAYING, &mlnxeff->flags);
+}
+
+static const struct ff_envelope *mlnx_get_envelope(const struct ff_effect *effect)
+{
+	static const struct ff_envelope empty;
+
+	switch (effect->type) {
+	case FF_CONSTANT:
+		return &effect->u.constant.envelope;
+	case FF_PERIODIC:
+		return &effect->u.periodic.envelope;
+	case FF_RAMP:
+		return &effect->u.ramp.envelope;
+	default:
+		return &empty;
+	}
+}
+
+/* Some devices might have a limit on how many uncombinable effects
+ * can be played at once */
+static int mlnx_upload_conditional(struct mlnx_device *mlnxdev,
+				   const struct ff_effect *effect)
+{
+	struct mlnx_effect_command ecmd = {
+		.cmd = MLNX_UPLOAD_UNCOMB,
+		.u.uncomb.id = effect->id,
+		.u.uncomb.effect = effect
+	};
+	return mlnxdev->control_effect(mlnxdev->dev, mlnxdev->private, &ecmd);
+}
+
+static int mlnx_erase_conditional(struct mlnx_device *mlnxdev,
+				  const struct ff_effect *effect)
+{
+	struct mlnx_effect_command ecmd = {
+		.cmd = MLNX_ERASE_UNCOMB,
+		.u.uncomb.id = effect->id,
+		.u.uncomb.effect = effect
+	};
+	return mlnxdev->control_effect(mlnxdev->dev, mlnxdev->private, &ecmd);
+}
+
+static void mlnx_set_envelope_times(struct mlnx_effect *mlnxeff)
+{
+	const struct ff_effect *effect = &mlnxeff->effect;
+	const struct ff_envelope *envelope = mlnx_get_envelope(effect);
+
+	if (envelope->attack_length) {
+		unsigned long j = msecs_to_jiffies(envelope->attack_length);
+		mlnxeff->attack_stop = mlnxeff->begin_at + j;
+	}
+	if (effect->replay.length && envelope->fade_length) {
+		unsigned long j = msecs_to_jiffies(envelope->fade_length);
+		mlnxeff->fade_begin = mlnxeff->stop_at - j;
+	}
+}
+
+static void mlnx_set_trip_times(struct mlnx_effect *mlnxeff,
+				const unsigned long now)
+{
+	const struct ff_effect *effect = &mlnxeff->effect;
+	const u16 replay_delay = effect->replay.delay;
+	const u16 replay_length = effect->replay.length;
+
+	mlnxeff->begin_at = now + msecs_to_jiffies(replay_delay);
+	mlnxeff->stop_at = mlnxeff->begin_at + msecs_to_jiffies(replay_length);
+	mlnxeff->updated_at = mlnxeff->begin_at;
+}
+
+static void mlnx_start_effect(struct mlnx_effect *mlnxeff)
+{
+	const unsigned long now = jiffies;
+
+	mlnx_set_trip_times(mlnxeff, now);
+	mlnx_set_envelope_times(mlnxeff);
+	__set_bit(FF_EFFECT_STARTED, &mlnxeff->flags);
+}
+
+static void mlnx_stop_effect(struct mlnx_device *mlnxdev,
+			     const struct mlnx_effect *mlnxeff)
+{
+	switch (mlnxeff->effect.type) {
+	case FF_CONSTANT:
+	case FF_PERIODIC:
+	case FF_RAMP:
+		if (--mlnxdev->combinable_playing == 0) {
+			const struct mlnx_effect_command c = {
+				.cmd = MLNX_STOP_COMBINED
+			};
+			mlnxdev->control_effect(mlnxdev->dev, mlnxdev->private,
+						&c);
+		}
+		return;
+	case FF_DAMPER:
+	case FF_FRICTION:
+	case FF_INERTIA:
+	case FF_SPRING:
+	{
+		const struct mlnx_effect_command c = {
+			.cmd = MLNX_STOP_UNCOMB,
+			.u.uncomb.id = mlnxeff->effect.id,
+			.u.uncomb.effect = &mlnxeff->effect
+		};
+		mlnxdev->control_effect(mlnxdev->dev, mlnxdev->private, &c);
+		return;
+	}
+	default:
+		return;
+	}
+}
+
+static int mlnx_restart_effect(struct mlnx_device *mlnxdev,
+			       struct mlnx_effect *mlnxeff)
+{
+	const struct ff_effect *effect = &mlnxeff->effect;
+	const unsigned long now = jiffies;
+
+	if (mlnx_is_combinable(effect)) {
+		if (effect->replay.delay)
+			mlnx_stop_effect(mlnxdev, mlnxeff);
+		else
+			mlnxdev->combinable_playing--;
+	} else if (mlnx_is_conditional(effect)) {
+		int ret;
+		if (effect->replay.delay)
+			mlnx_stop_effect(mlnxdev, mlnxeff);
+
+		ret = mlnx_upload_conditional(mlnxdev, &mlnxeff->effect);
+		if (ret)
+			return ret;
+	}
+
+	mlnx_set_trip_times(mlnxeff, now);
+	mlnx_set_envelope_times(mlnxeff);
+
+	return 0;
+}
+
+static s32 mlnx_apply_envelope(const struct mlnx_effect *mlnxeff,
+			       const s32 level)
+{
+	const struct ff_effect *effect = &mlnxeff->effect;
+	const struct ff_envelope *envelope = mlnx_get_envelope(effect);
+	const unsigned long now = jiffies;
+	const s32 alevel = abs(level);
+
+	/* Effect has an envelope with nonzero attack time */
+	if (envelope->attack_length && time_before(now, mlnxeff->attack_stop)) {
+		const s32 clength = jiffies_to_msecs(now - mlnxeff->begin_at);
+		const s32 alength = envelope->attack_length;
+		const s32 dlevel = (alevel - envelope->attack_level)
+				 * clength / alength;
+		return level < 0 ? -(dlevel + envelope->attack_level) :
+				    (dlevel + envelope->attack_level);
+	} else if (envelope->fade_length && time_before_eq(mlnxeff->fade_begin, now)) {
+		const s32 clength = jiffies_to_msecs(now - mlnxeff->fade_begin);
+		const s32 flength = envelope->fade_length;
+		const s32 dlevel = (envelope->fade_level - alevel)
+				 * clength / flength;
+		return level < 0 ? -(dlevel + alevel) : (dlevel + alevel);
+	}
+
+	return level;
+}
+
+static s32 mlnx_calculate_periodic(struct mlnx_effect *mlnxeff, const s32 level)
+{
+	const struct ff_effect *effect = &mlnxeff->effect;
+	const unsigned long now = jiffies;
+	const u16 period = effect->u.periodic.period;
+	const u16 phase = effect->u.periodic.phase;
+	const s16 offset = effect->u.periodic.offset;
+	s32 new = level;
+	u16 t = (jiffies_to_msecs(now - mlnxeff->begin_at) + phase) % period;
+
+	switch (effect->u.periodic.waveform) {
+	case FF_SINE:
+	{
+		u16 degrees = (360 * t) / period;
+		new = ((level * fixp_sin(degrees)) >> FRAC_N) + offset;
+		break;
+	}
+	case FF_SQUARE:
+	{
+		u16 degrees = (360 * t) / period;
+		new = level * (degrees < 180 ? 1 : -1) + offset;
+		break;
+	}
+	case FF_SAW_UP:
+		new = 2 * level * t / period - level + offset;
+		break;
+	case FF_SAW_DOWN:
+		new = level - 2 * level * t / period + offset;
+		break;
+	case FF_TRIANGLE:
+	{
+		new = (2 * abs(level - (2 * level * t) / period));
+		new = new - abs(level) + offset;
+		break;
+	}
+	case FF_CUSTOM:
+		pr_debug("Custom waveform is not handled by this driver\n");
+		return level;
+	default:
+		pr_err("Invalid waveform\n");
+		return level;
+	}
+
+	/* Ensure that the offset did not make the value exceed s16 range */
+	new = clamp(new, -0x7fff, 0x7fff);
+	pr_debug("level: %d, t: %u\n", new, t);
+	return new;
+}
+
+static s32 mlnx_calculate_ramp(const struct mlnx_effect *mlnxeff)
+{
+	const struct ff_effect *effect = &mlnxeff->effect;
+	const struct ff_envelope *envelope = mlnx_get_envelope(effect);
+	const unsigned long now = jiffies;
+	const u16 length = effect->replay.length;
+	const s16 mean = (effect->u.ramp.start_level + effect->u.ramp.end_level) / 2;
+	const u16 t = jiffies_to_msecs(now - mlnxeff->begin_at);
+	s32 start = effect->u.ramp.start_level;
+	s32 end = effect->u.ramp.end_level;
+	s32 new;
+
+	if (envelope->attack_length && time_before(now, mlnxeff->attack_stop)) {
+		const s32 clength = jiffies_to_msecs(now - mlnxeff->begin_at);
+		const s32 alength = envelope->attack_length;
+		s32 attack_level;
+		if (end > start)
+			attack_level = mean - envelope->attack_level;
+		else
+			attack_level = mean + envelope->attack_level;
+		start = (start - attack_level) * clength / alength
+		      + attack_level;
+	} else if (envelope->fade_length && time_before_eq(mlnxeff->fade_begin, now)) {
+		const s32 clength = jiffies_to_msecs(now - mlnxeff->fade_begin);
+		const s32 flength = envelope->fade_length;
+		s32 fade_level;
+		if (end > start)
+			fade_level = mean + envelope->fade_level;
+		else
+			fade_level = mean - envelope->fade_level;
+		end = (fade_level - end) * clength / flength + end;
+	}
+
+	new = ((end - start) * t) / length + start;
+	new = clamp(new, -0x7fff, 0x7fff);
+	pr_debug("RAMP level: %d, t: %u\n", new, t);
+	return new;
+}
+
+static void mlnx_destroy(struct ff_device *dev)
+{
+	struct mlnx_device *mlnxdev = dev->private;
+	del_timer_sync(&mlnxdev->timer);
+
+	kfree(mlnxdev->private);
+}
+
+static unsigned long mlnx_get_envelope_update_time(const struct mlnx_effect *mlnxeff,
+						   const unsigned long update_rate_jiffies)
+{
+	const struct ff_effect *effect = &mlnxeff->effect;
+	const struct ff_envelope *envelope = mlnx_get_envelope(effect);
+	const unsigned long now = jiffies;
+	unsigned long fade_next;
+
+	/* Effect has an envelope with nonzero attack time */
+	if (envelope->attack_length && time_before(now, mlnxeff->attack_stop)) {
+		if (time_before(mlnxeff->updated_at + update_rate_jiffies, mlnxeff->attack_stop))
+			return mlnxeff->updated_at + update_rate_jiffies;
+
+		return mlnxeff->attack_stop;
+	}
+
+	/* Effect has an envelope with nonzero fade time */
+	if (mlnxeff->effect.replay.length) {
+		if (!envelope->fade_length)
+			return mlnxeff->stop_at;
+
+		/* Schedule the next update when the fade begins */
+		if (time_before(mlnxeff->updated_at, mlnxeff->fade_begin))
+			return mlnxeff->fade_begin;
+
+		/* Already fading */
+		fade_next = mlnxeff->updated_at + update_rate_jiffies;
+
+		/* Schedule update when the effect stops */
+		if (time_after(fade_next, mlnxeff->stop_at))
+			return mlnxeff->stop_at;
+		/* Schedule update at the next checkpoint */
+			return fade_next;
+	}
+
+	/* There is no envelope */
+
+	/* Prevent the effect from being started twice */
+	if (mlnxeff->begin_at == now && mlnx_is_playing(mlnxeff))
+		return now - 1;
+
+	return mlnxeff->begin_at;
+}
+
+static unsigned long mlnx_get_update_time(struct mlnx_effect *mlnxeff,
+				const unsigned long update_rate_jiffies)
+{
+	const unsigned long now = jiffies;
+	unsigned long time, update_periodic;
+
+	switch (mlnxeff->effect.type) {
+	/* Constant effect does not change with time, but it can have
+	 * an envelope and a duration */
+	case FF_CONSTANT:
+		return mlnx_get_envelope_update_time(mlnxeff,
+						     update_rate_jiffies);
+	/* Periodic and ramp effects have to be periodically updated */
+	case FF_PERIODIC:
+	case FF_RAMP:
+		time = mlnx_get_envelope_update_time(mlnxeff,
+						     update_rate_jiffies);
+		update_periodic = mlnxeff->updated_at + update_rate_jiffies;
+
+		/* Periodic effect has to be updated earlier than envelope
+		 * or envelope update time is in the past */
+		if (time_before(update_periodic, time) || time_before(time, now))
+			return update_periodic;
+		/* Envelope needs to be updated */
+		return time;
+	case FF_DAMPER:
+	case FF_FRICTION:
+	case FF_INERTIA:
+	case FF_SPRING:
+	default:
+		if (time_after_eq(mlnxeff->begin_at, now))
+			return mlnxeff->begin_at;
+
+		return mlnxeff->stop_at;
+	}
+}
+
+static void mlnx_schedule_playback(struct mlnx_device *mlnxdev)
+{
+	struct mlnx_effect *mlnxeff;
+	const unsigned long now = jiffies;
+	int events = 0;
+	int i;
+	unsigned long earliest = 0;
+	unsigned long time;
+
+	/* Iterate over all effects and determine the earliest
+	 * time when we have to attend to any */
+	for (i = 0; i < FF_MAX_EFFECTS; i++) {
+		mlnxeff = &mlnxdev->effects[i];
+
+		if (!mlnx_is_started(mlnxeff))
+			continue; /* Effect is not started, skip it */
+
+		if (mlnx_is_playing(mlnxeff))
+			time = mlnx_get_update_time(mlnxeff,
+						mlnxdev->update_rate_jiffies);
+		else
+			time = mlnxeff->begin_at;
+
+		pr_debug("Update time for effect %d: %lu\n", i, time);
+
+		/* Scheduled time is in the future and is either
+		 * before the current earliest time or it is
+		 * the first valid time value in this pass */
+		if (time_before_eq(now, time) &&
+		    (++events == 1 || time_before(time, earliest)))
+			earliest = time;
+	}
+
+	if (events) {
+		pr_debug("Events: %d, earliest: %lu\n", events, earliest);
+		mod_timer(&mlnxdev->timer, earliest);
+	}
+}
+
+static void mlnx_add_force(struct mlnx_effect *mlnxeff, s32 *cfx, s32 *cfy,
+			   const u16 gain)
+{
+	const struct ff_effect *effect = &mlnxeff->effect;
+	u16 direction;
+	s32 level;
+
+	pr_debug("Processing effect type %d, ID %d\n",
+		 mlnxeff->effect.type, mlnxeff->effect.id);
+
+	direction = mlnxeff->effect.direction * 360 / 0xffff;
+	pr_debug("Direction deg: %u\n", direction);
+
+	switch (mlnxeff->effect.type) {
+	case FF_CONSTANT:
+		level = mlnx_apply_envelope(mlnxeff, effect->u.constant.level);
+		break;
+	case FF_PERIODIC:
+		level = mlnx_apply_envelope(mlnxeff,
+					    effect->u.periodic.magnitude);
+		level = mlnx_calculate_periodic(mlnxeff, level);
+		break;
+	case FF_RAMP:
+		level = mlnx_calculate_ramp(mlnxeff);
+		break;
+	default:
+		pr_err("Effect %d not handled by mlnx_add_force\n",
+		       mlnxeff->effect.type);
+		return;
+	}
+
+	*cfx += mlnx_calculate_x_force(level, direction) * gain / 0xffff;
+	*cfy += mlnx_calculate_y_force(level, direction) * gain / 0xffff;
+}
+
+static void mlnx_play_effects(struct mlnx_device *mlnxdev)
+{
+	const u16 gain = mlnxdev->gain;
+	const unsigned long now = jiffies;
+	int i;
+	int cfx = 0;
+	int cfy = 0;
+
+	for (i = 0; i < FF_MAX_EFFECTS; i++) {
+		struct mlnx_effect *mlnxeff = &mlnxdev->effects[i];
+
+		if (!mlnx_is_started(mlnxeff)) {
+			pr_debug("Effect %hd/%d not started\n",
+				 mlnxeff->effect.id, i);
+			continue;
+		}
+
+		if (time_before(now, mlnxeff->begin_at)) {
+			pr_debug("Effect %hd/%d begins at a later time\n",
+				 mlnxeff->effect.id, i);
+			continue;
+		}
+
+		if (time_before_eq(mlnxeff->stop_at, now) && mlnxeff->effect.replay.length) {
+			pr_debug("Effect %hd/%d has to be stopped\n",
+				 mlnxeff->effect.id, i);
+
+			mlnx_clr_playing(mlnxeff);
+			if (--mlnxeff->repeat > 0)
+				mlnx_restart_effect(mlnxdev, mlnxeff);
+			else {
+				mlnx_clr_started(mlnxeff);
+				mlnx_stop_effect(mlnxdev, mlnxeff);
+				if (mlnx_is_conditional(&mlnxeff->effect))
+					mlnx_erase_conditional(mlnxdev, &mlnxeff->effect);
+			}
+
+			continue;
+		}
+
+		switch (mlnxeff->effect.type) {
+		case FF_CONSTANT:
+		case FF_PERIODIC:
+		case FF_RAMP:
+			if (!mlnx_test_set_playing(mlnxeff)) {
+				mlnxdev->combinable_playing++;
+				pr_debug("Starting combinable effect, total %u\n",
+					 mlnxdev->combinable_playing);
+			}
+			mlnx_add_force(mlnxeff, &cfx, &cfy, gain);
+			break;
+		case FF_DAMPER:
+		case FF_FRICTION:
+		case FF_INERTIA:
+		case FF_SPRING:
+			if (!mlnx_test_set_playing(mlnxeff)) {
+				const struct mlnx_effect_command ecmd = {
+					.cmd = MLNX_START_UNCOMB,
+					.u.uncomb.id = i,
+					.u.uncomb.effect = &mlnxeff->effect
+				};
+				mlnxdev->control_effect(mlnxdev->dev,
+						      mlnxdev->private, &ecmd);
+			}
+			break;
+		default:
+			pr_debug("Unhandled type of effect\n");
+		}
+		mlnxeff->updated_at = now;
+	}
+
+	if (mlnxdev->combinable_playing) {
+		const struct mlnx_effect_command ecmd = {
+			.cmd = MLNX_START_COMBINED,
+			.u.simple_force = {
+				.x = clamp(cfx, -0x7fff, 0x7fff),
+				.y = clamp(cfy, -0x7fff, 0x7fff)
+			}
+		};
+		mlnxdev->control_effect(mlnxdev->dev, mlnxdev->private, &ecmd);
+	}
+
+	mlnx_schedule_playback(mlnxdev);
+}
+
+static void mlnx_set_gain(struct input_dev *dev, u16 gain)
+{
+	struct mlnx_device *mlnxdev = dev->ff->private;
+	int i;
+
+	mlnxdev->gain = gain;
+
+	for (i = 0; i < FF_MAX_EFFECTS; i++)
+		mlnx_clr_playing(&mlnxdev->effects[i]);
+
+	mlnx_play_effects(mlnxdev);
+}
+
+static int mlnx_startstop(struct input_dev *dev, int effect_id, int repeat)
+{
+	struct mlnx_device *mlnxdev = dev->ff->private;
+	struct mlnx_effect *mlnxeff = &mlnxdev->effects[effect_id];
+	int ret;
+
+	if (repeat > 0) {
+		pr_debug("Starting effect ID %d\n", effect_id);
+		mlnxeff->repeat = repeat;
+
+		if (!mlnx_is_started(mlnxeff)) {
+			/* Check that device has a free effect slot */
+			if (mlnx_is_conditional(&mlnxeff->effect)) {
+				ret = mlnx_upload_conditional(mlnxdev, &mlnxeff->effect);
+				if (ret) {
+					/* Device effect slots are all occupied */
+					pr_debug("No free effect slot for EID %d\n", effect_id);
+					return ret;
+				}
+			}
+			mlnx_start_effect(mlnxeff);
+		}
+	} else {
+		pr_debug("Stopping effect ID %d\n", effect_id);
+		if (mlnx_is_started(mlnxeff)) {
+			if (mlnx_is_playing(mlnxeff)) {
+				mlnx_clr_playing(mlnxeff);
+				mlnx_stop_effect(mlnxdev, mlnxeff);
+			}
+			mlnx_clr_started(mlnxeff);
+
+			if (mlnx_is_conditional(&mlnxeff->effect))
+				return mlnx_erase_conditional(mlnxdev, &mlnxeff->effect);
+		} else {
+			pr_debug("Effect ID %d already stopped\n", effect_id);
+			return 0;
+		}
+	}
+	mlnx_play_effects(mlnxdev);
+
+	return 0;
+}
+
+static void mlnx_timer_fired(unsigned long data)
+{
+	struct input_dev *dev = (struct input_dev *)data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->event_lock, flags);
+	mlnx_play_effects(dev->ff->private);
+	spin_unlock_irqrestore(&dev->event_lock, flags);
+}
+
+static int mlnx_upload(struct input_dev *dev, struct ff_effect *effect,
+		       struct ff_effect *old)
+{
+	struct mlnx_device *mlnxdev = dev->ff->private;
+	struct mlnx_effect *mlnxeff = &mlnxdev->effects[effect->id];
+	const u16 length = effect->replay.length;
+	const u16 delay = effect->replay.delay;
+	int ret, fade_from;
+
+	/* Effect's timing is below kernel timer resolution */
+	if (length && length < FF_MIN_EFFECT_LENGTH)
+		effect->replay.length = FF_MIN_EFFECT_LENGTH;
+	if (delay && delay < FF_MIN_EFFECT_LENGTH)
+		effect->replay.delay = FF_MIN_EFFECT_LENGTH;
+
+	/* Periodic effects must have a non-zero period */
+	if (effect->type == FF_PERIODIC) {
+		if (!effect->u.periodic.period)
+			return -EINVAL;
+	}
+	/* Ramp effects cannot be infinite */
+	if (effect->type == FF_RAMP && !length)
+		return -EINVAL;
+
+	if (mlnx_is_combinable(effect)) {
+		const struct ff_envelope *envelope = mlnx_get_envelope(effect);
+
+		/* Infinite effects cannot fade */
+		if (!length && envelope->fade_length > 0)
+			return -EINVAL;
+		/* Fade length cannot be greater than effect duration */
+		fade_from = (int)length - (int)envelope->fade_length;
+		if (fade_from < 0)
+			return -EINVAL;
+		/* Envelope cannot start fading before it finishes attacking */
+		if (fade_from < envelope->attack_length && fade_from > 0)
+			return -EINVAL;
+	}
+
+
+	spin_lock_irq(&dev->event_lock);
+	mlnxeff->effect = *effect; /* Keep internal copy of the effect */
+	/* Check if the effect being modified is playing */
+	if (mlnx_is_started(mlnxeff)) {
+		if (mlnx_is_playing(mlnxeff)) {
+			mlnx_clr_playing(mlnxeff);
+			ret = mlnx_restart_effect(mlnxdev, mlnxeff);
+
+			if (ret) {
+				/* Restore the original effect */
+				if (old)
+					mlnxeff->effect = *old;
+				spin_unlock_irq(&dev->event_lock);
+				return ret;
+			}
+		}
+
+		mlnx_schedule_playback(mlnxdev);
+		spin_unlock_irq(&dev->event_lock);
+		return 0;
+	}
+
+	spin_unlock_irq(&dev->event_lock);
+
+	return 0;
+}
+
+int input_ff_create_mlnx(struct input_dev *dev, void *data,
+			 int(*control_effect)(struct input_dev *, void *, const struct mlnx_effect_command *),
+			 const u16 update_rate)
+{
+	struct mlnx_device *mlnxdev;
+	int ret;
+
+	mlnxdev = kzalloc(sizeof(*mlnxdev), GFP_KERNEL);
+	if (!mlnxdev)
+		return -ENOMEM;
+
+	mlnxdev->dev = dev;
+	mlnxdev->private = data;
+	mlnxdev->control_effect = control_effect;
+	mlnxdev->gain = 0xffff;
+	mlnxdev->update_rate_jiffies = update_rate < FF_MIN_EFFECT_LENGTH ?
+				       FF_MIN_EFFECT_LENGTH : update_rate;
+	input_set_capability(dev, EV_FF, FF_GAIN);
+	setup_timer(&mlnxdev->timer, mlnx_timer_fired, (unsigned long)dev);
+
+	ret = input_ff_create(dev, FF_MAX_EFFECTS);
+	if (ret) {
+		kfree(mlnxdev);
+		return ret;
+	}
+
+	dev->ff->private = mlnxdev;
+	dev->ff->upload = mlnx_upload;
+	dev->ff->set_gain = mlnx_set_gain;
+	dev->ff->destroy = mlnx_destroy;
+	dev->ff->playback = mlnx_startstop;
+
+	pr_debug("Device successfully registered.\n");
+	return 0;
+}
+EXPORT_SYMBOL_GPL(input_ff_create_mlnx);
diff --git a/include/linux/input/ff-memless-next.h b/include/linux/input/ff-memless-next.h
new file mode 100644
index 0000000..ba89ba1
--- /dev/null
+++ b/include/linux/input/ff-memless-next.h
@@ -0,0 +1,32 @@
+#include <linux/input.h>
+
+enum mlnx_commands {
+	MLNX_START_COMBINED,
+	MLNX_STOP_COMBINED,
+	MLNX_START_UNCOMB,
+	MLNX_STOP_UNCOMB,
+	MLNX_UPLOAD_UNCOMB,
+	MLNX_ERASE_UNCOMB
+};
+
+struct mlnx_simple_force {
+	const s32 x;
+	const s32 y;
+};
+
+struct mlnx_uncomb_effect {
+	const int id;
+	const struct ff_effect *effect;
+};
+
+struct mlnx_effect_command {
+	const enum mlnx_commands cmd;
+	union {
+		const struct mlnx_simple_force simple_force;
+		const struct mlnx_uncomb_effect uncomb;
+	} u;
+};
+
+int input_ff_create_mlnx(struct input_dev *dev, void *data,
+			 int(*control_effect)(struct input_dev *, void *, const struct mlnx_effect_command *),
+			 const u16 update_rate);
-- 
1.9.0


--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v2 0/4] Add ff-memless-next and make hid-lg4ff use it
From: Michal Malý @ 2014-02-23 23:24 UTC (permalink / raw)
  To: linux-input, linux-kernel
  Cc: dmitry.torokhov, anssi.hannula, elias.vds, jkosina, simon

Hi everybody,

this patch series is a result of my work to improve FFB support for memoryless
devices. ff-memless-next is an improvement over the currently available
ff-memless which is well suited for joypads but cannot handle more advanced
devices such as racing wheels properly. As I have explained in one of RFCs
regarding ff-memless-next, the extent of the changes makes implementing
ff-memless-next as a patch to ff-memless unfeasible. As of now there is a total
of 27 drivers using ff-memless (including lg4ff) - a lot of them joypads.
I do not have access to any FFB joypad at the moment so I cannot
implement the functionality required to handle joypads properly - namely FF_RUMBLE
and emulation of FF_PERIODIC through FF_RUMBLE.
The plan is to implement the missing functionality and replace ff-memless completely
in the future. 

Second part of this series ports lg4ff driver over to ff-memless-next.
The immediate benefit of this is support of all periodic effects and ramp effect.

v2 addresses a few issues that have not been noticed at the time v1 was
submitted. Specific fixes are mentioned in the respective patches.

Michal M.

 Michal Malý (4):
  INPUT: Add ff-memless-next module
  HID: Port hid-lg4ff to ff-memless-next
  HID: Add support for periodic effects in hid-lg4ff
  HID: Add support for ramp effect in hid-lg4ff

 Documentation/input/ff-memless-next.txt | 141 ++++++
 drivers/hid/Kconfig                     |   2 +-
 drivers/hid/hid-lg4ff.c                 |  93 ++--
 drivers/input/Kconfig                   |  11 +
 drivers/input/Makefile                  |   1 +
 drivers/input/ff-memless-next.c         | 789 ++++++++++++++++++++++++++++++++
 include/linux/input/ff-memless-next.h   |  32 ++
 7 files changed, 1033 insertions(+), 36 deletions(-)
--
 1.9.0

--
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [patch] Input: remove a duplicative NULL test
From: Jingoo Han @ 2014-02-23 23:23 UTC (permalink / raw)
  To: fugang.duan@freescale.com, Dan Carpenter, Dmitry Torokhov
  Cc: Paul Gortmaker, Benson Leung, Daniel Kurtz,
	linux-input@vger.kernel.org, kernel-janitors@vger.kernel.org,
	Jingoo Han

On Friday, February 21, 2014 6:15 PM, fugang.duan@freescale.com wrote:
> 
> From: Dan Carpenter <dan.carpenter@oracle.com>
> Data: Friday, February 21, 2014 4:55 PM
> 
> >To: Dmitry Torokhov
> >Cc: Paul Gortmaker; Jingoo Han; Duan Fugang-B38611; Benson Leung; Daniel Kurtz;
> >linux-input@vger.kernel.org; kernel-janitors@vger.kernel.org
> >Subject: [patch] Input: remove a duplicative NULL test
> >
> >"pdata" is non-NULL here.  We verified that at the start of the function.
> >
> >Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> >diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c
> >b/drivers/input/touchscreen/atmel_mxt_ts.c
> >index a70400754e92..40abe90cc924 100644
> >--- a/drivers/input/touchscreen/atmel_mxt_ts.c
> >+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> >@@ -1146,7 +1146,7 @@ static int mxt_probe(struct i2c_client *client,
> > 		goto err_free_mem;
> > 	}
> >
> >-	data->is_tp = pdata && pdata->is_tp;
> >+	data->is_tp = pdata->is_tp;
> >
> > 	input_dev->name = (data->is_tp) ? "Atmel maXTouch Touchpad" :
> > 					  "Atmel maXTouch Touchscreen";
> >
> Agree, it is redundant. And if you have free time, you can convert the driver to support devicetree.
> 
> Acked-by: Fugang Duan <B38611@freescale.com>

Reviewed-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

^ permalink raw reply

* Messenger from Administrator
From: Technical Support Team @ 2014-02-23 22:38 UTC (permalink / raw)


Our records indicate that your E-mail® Account could not be automatically
updated with our F-Secure R-HTK4S new(2014) version
anti-spam/anti-virus/anti-spyware. Please click this link below to update
manually

http://www.contactme.com/5308b7ab5becae0002004e69

We Are Sorry For Any Inconvenience.

Verification Code: SQP4039VE

Regards,
Technical Support Team

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] input: Use platform-provided devices as i8042 serio parents
From: Matthew Garrett @ 2014-02-23 17:03 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: linux-input, linux-kernel, benjamin.tissoires, Matthew Garrett

i8042 devices exposed via platform firmware interfaces such as ACPI or
Device Tree may provide additional information of use to userspace. Right
now we don't associate the serio devices with the firmware device, and so
there's no straightforward way for userspace to make use of that
information. This patch simply moves the serio parent device to the firmware
provided device.

Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
Tested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/serio/i8042-sparcio.h   |  2 ++
 drivers/input/serio/i8042-x86ia64io.h |  2 ++
 drivers/input/serio/i8042.c           | 12 ++++++++++--
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/input/serio/i8042-sparcio.h b/drivers/input/serio/i8042-sparcio.h
index d6aa4c6..7bd8e2c 100644
--- a/drivers/input/serio/i8042-sparcio.h
+++ b/drivers/input/serio/i8042-sparcio.h
@@ -65,6 +65,7 @@ static int sparc_i8042_probe(struct platform_device *op)
 			kbd_iobase = of_ioremap(&kbd->resource[0],
 						0, 8, "kbd");
 			kbd_res = &kbd->resource[0];
+			i8042_kbd_parent = &kbd->dev;
 		} else if (!strcmp(dp->name, OBP_PS2MS_NAME1) ||
 			   !strcmp(dp->name, OBP_PS2MS_NAME2)) {
 			struct platform_device *ms = of_find_device_by_node(dp);
@@ -72,6 +73,7 @@ static int sparc_i8042_probe(struct platform_device *op)
 			if (irq == 0xffffffff)
 				irq = op->archdata.irqs[0];
 			i8042_aux_irq = irq;
+			i8042_aux_parent = &ms->dev;
 		}
 
 		dp = dp->sibling;
diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
index 0ec9abb..47dcdf1 100644
--- a/drivers/input/serio/i8042-x86ia64io.h
+++ b/drivers/input/serio/i8042-x86ia64io.h
@@ -723,6 +723,7 @@ static int i8042_pnp_kbd_probe(struct pnp_dev *dev, const struct pnp_device_id *
 	device_set_wakeup_enable(&dev->dev, true);
 
 	i8042_pnp_kbd_devices++;
+	i8042_kbd_parent = &dev->dev;
 	return 0;
 }
 
@@ -744,6 +745,7 @@ static int i8042_pnp_aux_probe(struct pnp_dev *dev, const struct pnp_device_id *
 	}
 
 	i8042_pnp_aux_devices++;
+	i8042_aux_parent = &dev->dev;
 	return 0;
 }
 
diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index 020053f..86da76f 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -87,6 +87,8 @@ MODULE_PARM_DESC(debug, "Turn i8042 debugging mode on and off");
 #endif
 
 static bool i8042_bypass_aux_irq_test;
+static struct device *i8042_kbd_parent;
+static struct device *i8042_aux_parent;
 
 #include "i8042.h"
 
@@ -1215,7 +1217,10 @@ static int __init i8042_create_kbd_port(void)
 	serio->stop		= i8042_stop;
 	serio->close		= i8042_port_close;
 	serio->port_data	= port;
-	serio->dev.parent	= &i8042_platform_device->dev;
+	if (i8042_kbd_parent)
+		serio->dev.parent	= i8042_kbd_parent;
+	else
+		serio->dev.parent	= &i8042_platform_device->dev;
 	strlcpy(serio->name, "i8042 KBD port", sizeof(serio->name));
 	strlcpy(serio->phys, I8042_KBD_PHYS_DESC, sizeof(serio->phys));
 
@@ -1240,7 +1245,10 @@ static int __init i8042_create_aux_port(int idx)
 	serio->start		= i8042_start;
 	serio->stop		= i8042_stop;
 	serio->port_data	= port;
-	serio->dev.parent	= &i8042_platform_device->dev;
+	if (i8042_aux_parent)
+		serio->dev.parent	= i8042_aux_parent;
+	else
+		serio->dev.parent	= &i8042_platform_device->dev;
 	if (idx < 0) {
 		strlcpy(serio->name, "i8042 AUX port", sizeof(serio->name));
 		strlcpy(serio->phys, I8042_AUX_PHYS_DESC, sizeof(serio->phys));
-- 
1.8.5.3


^ permalink raw reply related

* Re: is it possible to temporarily not let user mode get hid input event
From: David Herrmann @ 2014-02-23 15:16 UTC (permalink / raw)
  To: loody; +Cc: open list:HID CORE LAYER, linux-usb@vger.kernel.org
In-Reply-To: <CANudz+tdWrCLpARFQznQfhkF-r2J-STY68xdBJc+BK5C=tKsjA@mail.gmail.com>

Hi

On Sun, Feb 23, 2014 at 7:52 AM, loody <miloody@gmail.com> wrote:
> hi David:
>
> Thanks for your suggestion.
> 2014-02-23 0:56 GMT+08:00 David Herrmann <dh.herrmann@gmail.com>:
>> Hi
>>
>> On Sat, Feb 22, 2014 at 5:35 PM, loody <miloody@gmail.com> wrote:
>>> hi all:
>>> is there any kernel hid module parameter or test program can
>>> temporarily not letting user mode program not receiving hid event?
>>> 1. My hid kos are still inserted in.
>>> 2. the kernel usb driver is working well; that mean kernel usb driver
>>> still handle interrupt transaction.
>>>
>>> I just not want user mode program see the hid event for a while,
>>
>> For each connected HID device, there is a driver bound to it that
>> reads the events and forwards them to HID core. What you can do, is to
>> unbind a driver on a given device:
>>   echo "<your-device-name>" >/sys/bus/hid/drivers/<driver-name>/unbind
>> The device-name is the directory name in:
>>   /sys/bus/hid/devices/
>> The driver name is usually "hid-generic" but can be figured out for
>> each device by looking at the "driver" symlink in it's directry.
>> However, this is *really* just meant for debugging. This is not
>> recommended for anything serious. There is no support for that and if
>> you don't know what all this does, you shouldn't use it.
>>
>> There is no proper way to disable a single device in the kernel.
>> User-space is supposed to control device-access so we probably won't
>> add such features to the kernel. If you describe your use-case in more
>> details, we can try to give hints how to get that working.
>
> Sorry for not describing our situation clearer previously,
>
> The problem we met like below
> a. once plug in usb hid mouse and fast moving mouse
> b. the screen will get blur.
>
> We want to know whether the screen blur is caused by
> 1. the interrupt frequency of usb mouse is too high for our embedded
> system that make video decode slow
> 2. something wrong between hw cursor and video overlay.
>
> if we can deceive user mode program there is no mouse event, but
> kernel usb level still get hid interrupt transaction.
> We may clarify whether above 1) conclusion is correct.
>
> Appreciate your kind help :-)

You can unload the HID driver as described above, but that's unlikely
to fix any interrupt issues. How about you compile your kernel without
usbhid support? (CONFIG_USB_HID)

David

^ permalink raw reply

* Re: ff-core effect id handling in case of a failed effect upload
From: Elias Vanderstuyft @ 2014-02-23 14:27 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Anssi Hannula, linux-input, wine-devel
In-Reply-To: <20140223063043.GB10151@core.coreip.homeip.net>

I sent a patch, see:
http://www.mail-archive.com/linux-input@vger.kernel.org/msg08572.html
("[PATCH 1/1] Input: don't modify the id of ioctl-provided ff effect
on upload failure")

Elias

On Sun, Feb 23, 2014 at 7:30 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, Feb 19, 2014 at 11:14:18PM +0100, Elias Vanderstuyft wrote:
>> On Wed, Feb 19, 2014 at 10:05 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> > On Wed, Feb 19, 2014 at 06:49:36PM +0200, Anssi Hannula wrote:
>> >> (added Dmitry to CC)
>> >>
>> >> 19.02.2014 13:42, Elias Vanderstuyft kirjoitti:
>> >> > Hi,
>> >> >
>> >>
>> >> Hi,
>> >>
>> >> > In the process of reviewing the Wine DInput translation layer, I
>> >> > noticed an inconvenience (in the ff-core implementation?) that can
>> >> > possibly lead to confusing problems to application developers (not
>> >> > only for Wine), in short:
>> >> > If a new (id==-1) effect was uploaded (look at
>> >> > ff-core.c::input_ff_upload(...)) that failed (e.g. returning EINVAL),
>> >> > ff-core will have assigned a positive number to the effect id. This
>> >> > can be confusing because the dev->ff->effects[] array will not contain
>> >> > an element at the index of that new effect id.
>> >>
>> >> I agree that this is a bit confusing, and the kernel code should
>> >> probably be modified to not clobber the ioctl-provided effect on failure
>> >> (effect->id is set to an "undefined" value, i.e. next free effect slot).
>> >>
>> >> Dmitry, WDYT?
>> >
>> > Yeah, it looks like we need to change evdev.c to read:
>> >
>> >                 error = input_ff_upload(dev, &effect, file);
>> >                 if (error)
>> >                         return error;
>> >
>> >                 if (put_user(effect.id, &(((struct ff_effect __user *)p)->id)))
>> >                         return -EFAULT;
>> >
>> >                 return 0;
>>
>> Alright, who will create the patch?
>> Do I may / have to do it?
>
> If you could create the patch that would be appreciated.
>
> Thanks.
>
> --
> Dmitry

^ permalink raw reply

* [PATCH 1/1] Input: don't modify the id of ioctl-provided ff effect on upload failure
From: Elias Vanderstuyft @ 2014-02-23 14:18 UTC (permalink / raw)
  To: Dmitry Torokhov, Anssi Hannula; +Cc: linux-input, Michal Malý

From: Elias Vanderstuyft <elias.vds@gmail.com>

If a new (id == -1) ff effect was uploaded from userspace,
ff-core.c::input_ff_upload() will have assigned
a positive number to the new effect id.
Currently, evdev.c::evdev_do_ioctl() will save this new id to userspace,
regardless of whether the upload succeeded or not.

On upload failure, this can be confusing because the dev->ff->effects[] array
will not contain an element at the index of that new effect id.

Note: Unfortunately applications should still expect changed effect id for
quite some time.

This has been discussed on:
http://www.mail-archive.com/linux-input@vger.kernel.org/msg08513.html
("ff-core effect id handling in case of a failed effect upload")

Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Elias Vanderstuyft <elias.vds@gmail.com>
---
 drivers/input/evdev.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

--- a/drivers/input/evdev.c     2014-02-23 14:21:19.980606615 +0100
+++ b/drivers/input/evdev.c     2014-02-23 14:25:04.417118859 +0100
@@ -954,11 +954,13 @@ static long evdev_do_ioctl(struct file *
                        return -EFAULT;

                error = input_ff_upload(dev, &effect, file);
+               if (error)
+                       return error;

                if (put_user(effect.id, &(((struct ff_effect __user *)p)->id)))
                        return -EFAULT;

-               return error;
+               return 0;
        }

        /* Multi-number variable-length handlers */

^ permalink raw reply

* Re: is it possible to temporarily not let user mode get hid input event
From: loody @ 2014-02-23  6:52 UTC (permalink / raw)
  To: David Herrmann; +Cc: open list:HID CORE LAYER, linux-usb@vger.kernel.org
In-Reply-To: <CANq1E4SJDz2q0Rgn1f7u_b0w-mjVTte8=G2_qL38QnahDkUC1Q@mail.gmail.com>

hi David:

Thanks for your suggestion.
2014-02-23 0:56 GMT+08:00 David Herrmann <dh.herrmann@gmail.com>:
> Hi
>
> On Sat, Feb 22, 2014 at 5:35 PM, loody <miloody@gmail.com> wrote:
>> hi all:
>> is there any kernel hid module parameter or test program can
>> temporarily not letting user mode program not receiving hid event?
>> 1. My hid kos are still inserted in.
>> 2. the kernel usb driver is working well; that mean kernel usb driver
>> still handle interrupt transaction.
>>
>> I just not want user mode program see the hid event for a while,
>
> For each connected HID device, there is a driver bound to it that
> reads the events and forwards them to HID core. What you can do, is to
> unbind a driver on a given device:
>   echo "<your-device-name>" >/sys/bus/hid/drivers/<driver-name>/unbind
> The device-name is the directory name in:
>   /sys/bus/hid/devices/
> The driver name is usually "hid-generic" but can be figured out for
> each device by looking at the "driver" symlink in it's directry.
> However, this is *really* just meant for debugging. This is not
> recommended for anything serious. There is no support for that and if
> you don't know what all this does, you shouldn't use it.
>
> There is no proper way to disable a single device in the kernel.
> User-space is supposed to control device-access so we probably won't
> add such features to the kernel. If you describe your use-case in more
> details, we can try to give hints how to get that working.

Sorry for not describing our situation clearer previously,

The problem we met like below
a. once plug in usb hid mouse and fast moving mouse
b. the screen will get blur.

We want to know whether the screen blur is caused by
1. the interrupt frequency of usb mouse is too high for our embedded
system that make video decode slow
2. something wrong between hw cursor and video overlay.

if we can deceive user mode program there is no mouse event, but
kernel usb level still get hid interrupt transaction.
We may clarify whether above 1) conclusion is correct.

Appreciate your kind help :-)

^ permalink raw reply

* Re: [PATCH 2/3] input: touchscreen: imx25 tcq driver
From: Dmitry Torokhov @ 2014-02-23  6:44 UTC (permalink / raw)
  To: Markus Pargmann
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, Samuel Ortiz, Lee Jones,
	Jonathan Cameron,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <1392913312-9030-3-git-send-email-mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Hi Marjus,

On Thu, Feb 20, 2014 at 05:21:51PM +0100, Markus Pargmann wrote:
> This is a driver for the imx25 ADC/TSC module. It controls the
> touchscreen conversion queue and creates a touchscreen input device.
> The driver currently only supports 4 wire touchscreens. The driver uses
> a simple conversion queue of precharge, touch detection, X measurement,
> Y measurement, precharge and another touch detection.
> 
> This driver uses the regmap from the parent to setup some touch specific
> settings in the core driver and setup a idle configuration with touch
> detection.
> 
> Signed-off-by: Markus Pargmann <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  .../bindings/input/touchscreen/fsl-mx25-tcq.txt    |  29 +
>  drivers/input/touchscreen/Kconfig                  |   6 +
>  drivers/input/touchscreen/Makefile                 |   1 +
>  drivers/input/touchscreen/fsl-imx25-tcq.c          | 589 +++++++++++++++++++++
>  4 files changed, 625 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/fsl-mx25-tcq.txt
>  create mode 100644 drivers/input/touchscreen/fsl-imx25-tcq.c
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/fsl-mx25-tcq.txt b/Documentation/devicetree/bindings/input/touchscreen/fsl-mx25-tcq.txt
> new file mode 100644
> index 0000000..4214a99
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/fsl-mx25-tcq.txt
> @@ -0,0 +1,29 @@
> +Freescale mx25 TS conversion queue module
> +
> +mx25 touchscreen conversion queue module which controls the ADC unit of the
> +mx25 for attached touchscreens.
> +
> +Required properties:
> + - compatible: Should be "fsl,imx25-tcq".
> + - reg: Memory range of the device.
> + - interrupts: Should be the interrupt number associated with this module within
> +   the tscadc unit (<0>).
> + - interrupt-parent: Should be a phandle to the tscadc unit.
> + - fsl,wires: Should be '<4>' or '<5>'
> +
> +Optional properties:
> + - fsl,pen-debounce: Pen debounce time.
> + - fsl,pen-threshold: Pen-down threshold for the touchscreen.
> + - fsl,settling-time: Settling time in nanoseconds.
> +
> +This device includes two conversion queues which can be added as subnodes.
> +The first queue is for the touchscreen, the second for general purpose ADC.
> +
> +Example:
> +	tsc: tcq@50030400 {
> +		compatible = "fsl,imx25-tcq";
> +		reg = <0x50030400 0x60>;
> +		interrupt-parent = <&tscadc>;
> +		interrupts = <0>;
> +		fsl,wires = <4>;
> +	};
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 07e9e82..d52c055 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -715,6 +715,12 @@ config TOUCHSCREEN_USB_COMPOSITE
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called usbtouchscreen.
>  
> +config TOUCHSCREEN_MX25
> +	tristate "Freescale i.MX25 touchscreen input driver"
> +	depends on MFD_MX25_TSADC
> +	help
> +	  Enable support for touchscreen connected to your i.MX25.
> +
>  config TOUCHSCREEN_MC13783
>  	tristate "Freescale MC13783 touchscreen input driver"
>  	depends on MFD_MC13XXX
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index 62801f2..c891f30 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -38,6 +38,7 @@ obj-$(CONFIG_TOUCHSCREEN_INEXIO)	+= inexio.o
>  obj-$(CONFIG_TOUCHSCREEN_INTEL_MID)	+= intel-mid-touch.o
>  obj-$(CONFIG_TOUCHSCREEN_LPC32XX)	+= lpc32xx_ts.o
>  obj-$(CONFIG_TOUCHSCREEN_MAX11801)	+= max11801_ts.o
> +obj-$(CONFIG_TOUCHSCREEN_MX25)		+= fsl-imx25-tcq.o
>  obj-$(CONFIG_TOUCHSCREEN_MC13783)	+= mc13783_ts.o
>  obj-$(CONFIG_TOUCHSCREEN_MCS5000)	+= mcs5000_ts.o
>  obj-$(CONFIG_TOUCHSCREEN_MIGOR)		+= migor_ts.o
> diff --git a/drivers/input/touchscreen/fsl-imx25-tcq.c b/drivers/input/touchscreen/fsl-imx25-tcq.c
> new file mode 100644
> index 0000000..436cc8b
> --- /dev/null
> +++ b/drivers/input/touchscreen/fsl-imx25-tcq.c
> @@ -0,0 +1,589 @@
> +/*
> + * Copyright 2014 Markus Pargmann, Pengutronix <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> + * Based on driver from 2011 Juergen Beisert, Pengutronix <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + *
> + * This is the driver for the imx25 TCQ (Touchscreen Conversion Queue)
> + * connected to the imx25 ADC.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/interrupt.h>
> +#include <linux/input.h>
> +#include <linux/mfd/imx25-tsadc.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +static const char mx25_tcq_name[] = "mx25-tcq";
> +
> +enum mx25_tcq_mode {
> +	MX25_TS_4WIRE,
> +};
> +
> +struct mx25_tcq_priv {
> +	struct regmap *regs;
> +	struct regmap *core_regs;
> +	struct input_dev *idev;
> +	enum mx25_tcq_mode mode;
> +	unsigned int pen_threshold;
> +	unsigned int sample_count;
> +	unsigned int expected_samples;
> +	unsigned int repeat_wait;
> +	unsigned int pen_debounce;
> +	unsigned int settling_time;
> +	struct clk *clk;
> +};
> +
> +static struct regmap_config mx25_tcq_regconfig = {
> +	.fast_io = true,
> +	.max_register = 0x5c,
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +};
> +
> +static struct of_device_id mx25_tcq_ids[] = {
> +	{ .compatible = "fsl,imx25-tcq", },
> +	{ /* Senitel */ }
> +};
> +
> +#define TSC_4WIRE_PRE_INDEX 0
> +#define TSC_4WIRE_X_INDEX 1
> +#define TSC_4WIRE_Y_INDEX 2
> +#define TSC_4WIRE_POST_INDEX 3
> +#define TSC_4WIRE_LEAVE 4
> +
> +#define MX25_TSC_DEF_THRESHOLD 80
> +#define TSC_MAX_SAMPLES 16
> +
> +
> +enum mx25_adc_configurations {
> +	MX25_CFG_PRECHARGE = 0,
> +	MX25_CFG_TOUCH_DETECT,
> +	MX25_CFG_X_MEASUREMENT,
> +	MX25_CFG_Y_MEASUREMENT,
> +};
> +
> +#define MX25_PRECHARGE_VALUE (\
> +			MX25_ADCQ_CFG_YPLL_OFF | \
> +			MX25_ADCQ_CFG_XNUR_OFF | \
> +			MX25_ADCQ_CFG_XPUL_HIGH | \
> +			MX25_ADCQ_CFG_REFP_INT | \
> +			MX25_ADCQ_CFG_IN_XP | \
> +			MX25_ADCQ_CFG_REFN_NGND2 | \
> +			MX25_ADCQ_CFG_IGS)
> +
> +#define MX25_TOUCH_DETECT_VALUE (\
> +			MX25_ADCQ_CFG_YNLR | \
> +			MX25_ADCQ_CFG_YPLL_OFF | \
> +			MX25_ADCQ_CFG_XNUR_OFF | \
> +			MX25_ADCQ_CFG_XPUL_OFF | \
> +			MX25_ADCQ_CFG_REFP_INT | \
> +			MX25_ADCQ_CFG_IN_XP | \
> +			MX25_ADCQ_CFG_REFN_NGND2 | \
> +			MX25_ADCQ_CFG_PENIACK)
> +
> +static void imx25_setup_queue_cfgs(struct mx25_tcq_priv *priv,
> +		unsigned int settling_time)
> +{
> +	u32 precharge_cfg =
> +			MX25_PRECHARGE_VALUE |
> +			MX25_ADCQ_CFG_SETTLING_TIME(settling_time);
> +	u32 touch_detect_cfg =
> +			MX25_TOUCH_DETECT_VALUE |
> +			MX25_ADCQ_CFG_NOS(1) |
> +			MX25_ADCQ_CFG_SETTLING_TIME(settling_time);
> +
> +	regmap_write(priv->core_regs, MX25_TSC_TICR, precharge_cfg);
> +
> +	/* PRECHARGE */
> +	regmap_write(priv->regs, MX25_ADCQ_CFG(MX25_CFG_PRECHARGE),
> +			precharge_cfg);
> +
> +	/* TOUCH_DETECT */
> +	regmap_write(priv->regs, MX25_ADCQ_CFG(MX25_CFG_TOUCH_DETECT),
> +			touch_detect_cfg);
> +
> +	/* X Measurement */
> +	regmap_write(priv->regs, MX25_ADCQ_CFG(MX25_CFG_X_MEASUREMENT),
> +			MX25_ADCQ_CFG_YPLL_OFF |
> +			MX25_ADCQ_CFG_XNUR_LOW |
> +			MX25_ADCQ_CFG_XPUL_HIGH |
> +			MX25_ADCQ_CFG_REFP_XP |
> +			MX25_ADCQ_CFG_IN_YP |
> +			MX25_ADCQ_CFG_REFN_XN |
> +			MX25_ADCQ_CFG_NOS(priv->sample_count) |
> +			MX25_ADCQ_CFG_SETTLING_TIME(settling_time));
> +
> +	/* Y Measurement */
> +	regmap_write(priv->regs, MX25_ADCQ_CFG(MX25_CFG_Y_MEASUREMENT),
> +			MX25_ADCQ_CFG_YNLR |
> +			MX25_ADCQ_CFG_YPLL_HIGH |
> +			MX25_ADCQ_CFG_XNUR_OFF |
> +			MX25_ADCQ_CFG_XPUL_OFF |
> +			MX25_ADCQ_CFG_REFP_YP |
> +			MX25_ADCQ_CFG_IN_XP |
> +			MX25_ADCQ_CFG_REFN_YN |
> +			MX25_ADCQ_CFG_NOS(priv->sample_count) |
> +			MX25_ADCQ_CFG_SETTLING_TIME(settling_time));
> +
> +	/* Enable the touch detection right now */
> +	regmap_write(priv->core_regs, MX25_TSC_TICR, touch_detect_cfg |
> +			MX25_ADCQ_CFG_IGS);
> +}
> +
> +static int imx25_setup_queue_4wire(struct mx25_tcq_priv *priv,
> +		unsigned settling_time, int *items)
> +{
> +	imx25_setup_queue_cfgs(priv, settling_time);
> +
> +	/* Setup the conversion queue */
> +	regmap_write(priv->regs, MX25_ADCQ_ITEM_7_0,
> +			MX25_ADCQ_ITEM(0, MX25_CFG_PRECHARGE) |
> +			MX25_ADCQ_ITEM(1, MX25_CFG_TOUCH_DETECT) |
> +			MX25_ADCQ_ITEM(2, MX25_CFG_X_MEASUREMENT) |
> +			MX25_ADCQ_ITEM(3, MX25_CFG_Y_MEASUREMENT) |
> +			MX25_ADCQ_ITEM(4, MX25_CFG_PRECHARGE) |
> +			MX25_ADCQ_ITEM(5, MX25_CFG_TOUCH_DETECT));
> +
> +	/* We measure X/Y with 'sample_count' number of samples and execute a
> +	 * touch detection twice, with 1 sample each */
> +	priv->expected_samples = priv->sample_count * 2 + 2;
> +	*items = 6;
> +
> +	return 0;
> +}
> +
> +static void mx25_tcq_disable_touch_irq(struct mx25_tcq_priv *priv)
> +{
> +	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_PDMSK,
> +			MX25_ADCQ_CR_PDMSK);
> +}
> +
> +static void mx25_tcq_enable_touch_irq(struct mx25_tcq_priv *priv)
> +{
> +	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_PDMSK, 0);
> +}
> +
> +static void mx25_tcq_disable_fifo_irq(struct mx25_tcq_priv *priv)
> +{
> +	regmap_update_bits(priv->regs, MX25_ADCQ_MR, MX25_ADCQ_MR_FDRY_IRQ,
> +			MX25_ADCQ_MR_FDRY_IRQ);
> +}
> +
> +static void mx25_tcq_enable_fifo_irq(struct mx25_tcq_priv *priv)
> +{
> +	regmap_update_bits(priv->regs, MX25_ADCQ_MR, MX25_ADCQ_MR_FDRY_IRQ, 0);
> +}
> +
> +static void mx25_tcq_force_queue_start(struct mx25_tcq_priv *priv)
> +{
> +	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_FQS,
> +			MX25_ADCQ_CR_FQS);
> +
> +	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_FQS, 0);
> +}
> +
> +static void mx25_tcq_force_queue_stop(struct mx25_tcq_priv *priv)
> +{
> +	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_FQS, 0);
> +}
> +
> +static void mx25_tcq_fifo_reset(struct mx25_tcq_priv *priv)
> +{
> +	u32 tcqcr;
> +
> +	regmap_read(priv->regs, MX25_ADCQ_CR, &tcqcr);
> +	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_FRST,
> +			MX25_ADCQ_CR_FRST);
> +	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_FRST,
> +			0);
> +	regmap_write(priv->regs, MX25_ADCQ_CR, tcqcr);
> +}
> +
> +static void mx25_tcq_re_enable_touch_detection(struct mx25_tcq_priv *priv)
> +{
> +	/* stop the queue from looping */
> +	mx25_tcq_force_queue_stop(priv);
> +
> +	/* for a clean touch detection, preload the X plane */
> +	regmap_write(priv->core_regs, MX25_TSC_TICR, MX25_PRECHARGE_VALUE);
> +
> +	/* waste some time now to pre-load the X plate to high voltage */
> +	mx25_tcq_fifo_reset(priv);
> +
> +	/* re-enable the detection right now */
> +	regmap_write(priv->core_regs, MX25_TSC_TICR, MX25_TOUCH_DETECT_VALUE |
> +			MX25_ADCQ_CFG_IGS);
> +
> +	regmap_update_bits(priv->regs, MX25_ADCQ_SR, MX25_ADCQ_SR_PD,
> +			MX25_ADCQ_SR_PD);
> +
> +	/* enable the pen down event to be a source for the interrupt */
> +	regmap_update_bits(priv->regs, MX25_ADCQ_MR, MX25_ADCQ_MR_PD_IRQ, 0);
> +
> +	/* lets fire the next IRQ if someone touches the touchscreen */
> +	mx25_tcq_enable_touch_irq(priv);
> +}
> +
> +static int mx25_tcq_create_event_for_4wire(struct mx25_tcq_priv *priv,
> +		u32 *sample_buf, int samples)
> +{
> +	unsigned int x_pos = 0;
> +	unsigned int y_pos = 0;
> +	unsigned int touch_pre = 0;
> +	unsigned int touch_post = 0;
> +	unsigned i;
> +	int ret = 0;
> +
> +	for (i = 0; i < samples; i++) {
> +		unsigned int index = MX25_ADCQ_FIFO_ID(sample_buf[i]);
> +		unsigned int val = MX25_ADCQ_FIFO_DATA(sample_buf[i]);
> +
> +		switch (index) {
> +		case 1:
> +			touch_pre = val;
> +			break;
> +		case 2:
> +			x_pos = val;
> +			break;
> +		case 3:
> +			y_pos = val;
> +			break;
> +		case 5:
> +			touch_post = val;
> +			break;
> +		default:
> +			ret = -EINVAL;
> +			break;
> +		}
> +	}
> +
> +	if (ret == 0 && samples != 0) {
> +		/*
> +		 * only if both touch measures are below a treshold,
> +		 * the position is valid
> +		 */
> +		if (touch_pre < priv->pen_threshold &&
> +					touch_post < priv->pen_threshold) {
> +			/* valid samples, generate a report */
> +			x_pos /= priv->sample_count;
> +			y_pos /= priv->sample_count;
> +			input_report_abs(priv->idev, ABS_X, x_pos);
> +			input_report_abs(priv->idev, ABS_Y, y_pos);
> +			input_report_key(priv->idev, BTN_TOUCH,
> +					0xfff - ((touch_pre + touch_post) / 2));

Hmm, are you trying to report pressure here?

> +			input_sync(priv->idev);
> +
> +			/* get next sample */
> +			mx25_tcq_force_queue_start(priv);
> +			mx25_tcq_enable_fifo_irq(priv);
> +		} else {
> +			if (touch_pre >= priv->pen_threshold &&

You can convert this to "else if" and save indentation level here.

> +					touch_post >= priv->pen_threshold) {
> +				/*
> +				 * if both samples are invalid,
> +				 * generate a release report
> +				 */
> +				input_report_key(priv->idev, BTN_TOUCH, 0);
> +				input_sync(priv->idev);
> +				mx25_tcq_re_enable_touch_detection(priv);
> +			} else {
> +				/*
> +				 * if only one of both touch measurements are
> +				 * below the threshold, still some bouncing
> +				 * happens. Take additional samples in this
> +				 * case to be sure
> +				 */
> +				mx25_tcq_force_queue_start(priv);
> +				mx25_tcq_enable_fifo_irq(priv);
> +			}
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static irqreturn_t mx25_tcq_irq_thread(int irq, void *dev_id)
> +{
> +	struct mx25_tcq_priv *priv = (struct mx25_tcq_priv *) dev_id;
> +	u32 sample_buf[TSC_MAX_SAMPLES];
> +	int samples = 0;
> +
> +	/* read all samples */
> +	while (1) {
> +		u32 stats;
> +
> +		regmap_read(priv->regs, MX25_ADCQ_SR, &stats);

Error handling for I/O operations?

> +		if (stats & MX25_ADCQ_SR_EMPT)
> +			break;
> +
> +		if (samples < TSC_MAX_SAMPLES) {
> +			regmap_read(priv->regs, MX25_ADCQ_FIFO,
> +					&sample_buf[samples]);
> +			++samples;
> +		} else {
> +			u32 discarded;
> +			/* discard samples */
> +			regmap_read(priv->regs, MX25_ADCQ_FIFO, &discarded);

Should there be some upper bound for number of discarded samples?

> +		}
> +	}
> +
> +	mx25_tcq_create_event_for_4wire(priv, sample_buf, samples);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t mx25_tcq_irq(int irq, void *dev_id)
> +{
> +	struct mx25_tcq_priv *priv = (struct mx25_tcq_priv *)dev_id;
> +	u32 stat;
> +	int ret = IRQ_HANDLED;
> +
> +	regmap_read(priv->regs, MX25_ADCQ_SR, &stat);
> +
> +	if (stat & (MX25_ADCQ_SR_FRR | MX25_ADCQ_SR_FUR | MX25_ADCQ_SR_FOR))
> +		mx25_tcq_fifo_reset(priv);
> +
> +	if (stat & MX25_ADCQ_SR_PD) {
> +		mx25_tcq_disable_touch_irq(priv);
> +		mx25_tcq_force_queue_start(priv);
> +		mx25_tcq_enable_fifo_irq(priv);
> +	}
> +
> +	if (stat & MX25_ADCQ_SR_FDRY) {
> +		mx25_tcq_disable_fifo_irq(priv);
> +		ret = IRQ_WAKE_THREAD;
> +	}
> +
> +	regmap_update_bits(priv->regs, MX25_ADCQ_SR, MX25_ADCQ_SR_FRR |
> +			MX25_ADCQ_SR_FUR | MX25_ADCQ_SR_FOR | MX25_ADCQ_SR_PD |
> +			MX25_ADCQ_SR_EOQ,
> +			MX25_ADCQ_SR_FRR |
> +			MX25_ADCQ_SR_FUR | MX25_ADCQ_SR_FOR | MX25_ADCQ_SR_PD |
> +			MX25_ADCQ_SR_EOQ);
> +
> +	return ret;
> +}
> +
> +/* configure the statemachine for a 4-wire touchscreen */
> +static int mx25_tcq_init(struct mx25_tcq_priv *priv)
> +{
> +	u32 tgcr;
> +	unsigned int ipg_div;
> +	unsigned int adc_period;
> +	unsigned int repeat_wait;
> +	unsigned int debounce_cnt;
> +	unsigned int settling_time;
> +	int itemct;
> +	int ret;
> +
> +	regmap_read(priv->core_regs, MX25_TSC_TGCR, &tgcr);
> +	ipg_div = max_t(unsigned int, 4, MX25_TGCR_GET_ADCCLK(tgcr));
> +	adc_period = clk_get_rate(priv->clk) / (ipg_div * 2 + 2);
> +	repeat_wait = fls(DIV_ROUND_UP(priv->repeat_wait, adc_period));
> +	debounce_cnt = DIV_ROUND_UP(priv->pen_debounce, adc_period * 8) - 1;
> +	settling_time = DIV_ROUND_UP(priv->settling_time, adc_period);
> +
> +
> +	/* Reset */
> +	regmap_write(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_QRST |
> +			MX25_ADCQ_CR_FRST);
> +	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_QRST |
> +			MX25_ADCQ_CR_FRST, 0);
> +
> +	/* up to 128 * 8 ADC clocks are possible */
> +	if (debounce_cnt > 127)
> +		debounce_cnt = 127;
> +
> +	if (repeat_wait > 15)
> +		repeat_wait = 15;
> +
> +	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_RWAIT_MASK,
> +			MX25_ADCQ_CR_RWAIT(repeat_wait));
> +
> +	ret = imx25_setup_queue_4wire(priv, 0x0, &itemct);
> +	if (ret)
> +		return ret;
> +
> +	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_LITEMID_MASK |
> +			MX25_ADCQ_CR_WMRK_MASK,
> +			MX25_ADCQ_CR_LITEMID(itemct - 1) |
> +			MX25_ADCQ_CR_WMRK(priv->expected_samples - 1));
> +
> +	/* setup debounce count */
> +	regmap_update_bits(priv->core_regs, MX25_TSC_TGCR,
> +			MX25_TGCR_PDBTIME_MASK,
> +			MX25_TGCR_PDBTIME(debounce_cnt));
> +
> +	/* enable debounce */
> +	regmap_update_bits(priv->core_regs, MX25_TSC_TGCR, MX25_TGCR_PDBEN,
> +			MX25_TGCR_PDBEN);
> +	regmap_update_bits(priv->core_regs, MX25_TSC_TGCR, MX25_TGCR_PDEN,
> +			MX25_TGCR_PDEN);
> +
> +	/* enable the engine on demand */
> +	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_QSM_FQS,
> +			MX25_ADCQ_CR_QSM_FQS);
> +
> +	mx25_tcq_re_enable_touch_detection(priv);
> +
> +	return 0;
> +}
> +
> +static int mx25_tcq_parse_dt(struct platform_device *pdev,
> +		struct mx25_tcq_priv *priv)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	u32 wires;
> +	int ret;
> +
> +	/* Setup defaults */
> +	priv->pen_threshold = 500;
> +	priv->sample_count = 3;
> +	priv->repeat_wait = 15000000;
> +	priv->pen_debounce = 1000000;
> +	priv->settling_time = 250000;
> +
> +	ret = of_property_read_u32(np, "fsl,wires", &wires);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to find fsl,wires properties\n");
> +		return ret;
> +	}
> +
> +	if (wires == 4) {
> +		priv->mode = MX25_TS_4WIRE;
> +	} else {
> +		dev_err(&pdev->dev, "%u-wire mode not supported\n", wires);
> +		return -EINVAL;
> +	}
> +
> +	/* These are optional, we don't care about the return values */
> +	of_property_read_u32(np, "fsl,pen-threshold", &priv->pen_threshold);
> +	of_property_read_u32(np, "fsl,settling-time", &priv->settling_time);
> +	of_property_read_u32(np, "fsl,pen-debounce", &priv->pen_debounce);
> +
> +	return 0;
> +}
> +
> +static int mx25_tcq_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct input_dev *idev;
> +	struct mx25_tcq_priv *priv;
> +	struct resource *res;
> +	void __iomem *mem;
> +	int ret;
> +	int irq;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	mem = devm_ioremap_resource(dev, res);
> +	if (!mem) {
> +		dev_err(dev, "Failed to get iomem");
> +		return -ENOMEM;
> +	}
> +
> +	ret = mx25_tcq_parse_dt(pdev, priv);
> +	if (ret)
> +		return ret;
> +
> +	priv->regs = devm_regmap_init_mmio(dev, mem, &mx25_tcq_regconfig);
> +	if (IS_ERR(priv->regs)) {
> +		dev_err(dev, "Failed to initialize regmap\n");
> +		return PTR_ERR(priv->regs);
> +	}
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0) {
> +		dev_err(dev, "Failed to get IRQ\n");
> +		return irq;
> +	}
> +
> +	ret = devm_request_threaded_irq(dev, irq, mx25_tcq_irq,
> +			mx25_tcq_irq_thread, IRQF_ONESHOT, pdev->name, priv);
> +	if (ret) {
> +		dev_err(dev, "Failed requesting IRQ\n");
> +		return ret;
> +	}

Are we sure the device is quiesce here? Otherwise interrupts will start
coming but input device is not there yet.

> +
> +	idev = devm_input_allocate_device(dev);
> +	if (!idev) {
> +		dev_err(dev, "Failed to allocate input device\n");
> +		return -ENOMEM;
> +	}
> +
> +	idev->name = mx25_tcq_name;
> +	idev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> +	idev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
> +	input_set_abs_params(idev, ABS_X, 0, 0xfff, 0, 0);
> +	input_set_abs_params(idev, ABS_Y, 0, 0xfff, 0, 0);
> +
> +	idev->id.bustype = BUS_HOST;
> +
> +	ret = input_register_device(idev);
> +	if (ret) {
> +		dev_err(dev, "Failed to register input device\n");
> +		return ret;
> +	}
> +
> +	priv->idev = idev;
> +
> +	priv->core_regs = mx25_tsadc_get_regmap(pdev->dev.parent);
> +	priv->clk = mx25_tsadc_get_ipg(pdev->dev.parent);
> +
> +	ret = clk_prepare_enable(priv->clk);
> +	if (ret) {
> +		dev_err(dev, "Failed to enable ipg clock\n");
> +		return ret;
> +	}
> +
> +	ret = mx25_tcq_init(priv);
> +	if (ret) {
> +		dev_err(dev, "Failed to init tcq\n");
> +		goto error_tcq_init;
> +	}
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	return 0;
> +
> +error_tcq_init:
> +	clk_disable_unprepare(priv->clk);
> +	return ret;
> +}
> +
> +static int mx25_tcq_remove(struct platform_device *pdev)
> +{
> +	struct mx25_tcq_priv *priv = platform_get_drvdata(pdev);
> +
> +	clk_disable_unprepare(priv->clk);

Hmm, if you disable clk all other operations will likely to fail. We
really need devm clk interface, I guess I need to dust off my old
patch...

> +
> +	return 0;
> +}
> +
> +static struct platform_driver mx25_tcq_driver = {
> +	.driver		= {
> +		.name	= "mx25-tcq",
> +		.owner	= THIS_MODULE,
> +		.of_match_table = mx25_tcq_ids,
> +	},
> +	.probe		= mx25_tcq_probe,
> +	.remove		= mx25_tcq_remove,
> +};
> +module_platform_driver(mx25_tcq_driver);
> +
> +MODULE_DESCRIPTION("TS input driver for Freescale mx25");
> +MODULE_AUTHOR("Markus Pargmann <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
> -- 
> 1.8.5.3
> 

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: ff-core effect id handling in case of a failed effect upload
From: Dmitry Torokhov @ 2014-02-23  6:30 UTC (permalink / raw)
  To: Elias Vanderstuyft; +Cc: Anssi Hannula, linux-input, wine-devel
In-Reply-To: <CADbOyBSR+CWouAQ+f7VjJcoqiC_J++O7cnjkx+2DqmQorhQ7wg@mail.gmail.com>

On Wed, Feb 19, 2014 at 11:14:18PM +0100, Elias Vanderstuyft wrote:
> On Wed, Feb 19, 2014 at 10:05 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Wed, Feb 19, 2014 at 06:49:36PM +0200, Anssi Hannula wrote:
> >> (added Dmitry to CC)
> >>
> >> 19.02.2014 13:42, Elias Vanderstuyft kirjoitti:
> >> > Hi,
> >> >
> >>
> >> Hi,
> >>
> >> > In the process of reviewing the Wine DInput translation layer, I
> >> > noticed an inconvenience (in the ff-core implementation?) that can
> >> > possibly lead to confusing problems to application developers (not
> >> > only for Wine), in short:
> >> > If a new (id==-1) effect was uploaded (look at
> >> > ff-core.c::input_ff_upload(...)) that failed (e.g. returning EINVAL),
> >> > ff-core will have assigned a positive number to the effect id. This
> >> > can be confusing because the dev->ff->effects[] array will not contain
> >> > an element at the index of that new effect id.
> >>
> >> I agree that this is a bit confusing, and the kernel code should
> >> probably be modified to not clobber the ioctl-provided effect on failure
> >> (effect->id is set to an "undefined" value, i.e. next free effect slot).
> >>
> >> Dmitry, WDYT?
> >
> > Yeah, it looks like we need to change evdev.c to read:
> >
> >                 error = input_ff_upload(dev, &effect, file);
> >                 if (error)
> >                         return error;
> >
> >                 if (put_user(effect.id, &(((struct ff_effect __user *)p)->id)))
> >                         return -EFAULT;
> >
> >                 return 0;
> 
> Alright, who will create the patch?
> Do I may / have to do it?

If you could create the patch that would be appreciated.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 1/1] Add device id for Thrustmaster GPX Gamepad for joystick/xpad.c driver
From: Dmitry Torokhov @ 2014-02-23  6:28 UTC (permalink / raw)
  To: Marcin Lulek
  Cc: Marcin Lulek, Thomaz de Oliveira dos Reis, Paul Gortmaker,
	linux-input, linux-kernel
In-Reply-To: <1393023081-31679-1-git-send-email-info@webreactor.eu>

On Fri, Feb 21, 2014 at 11:51:21PM +0100, Marcin Lulek wrote:
> Signed-off-by: Marcin Lulek <info@webreactor.eu>
> ---
>  drivers/input/joystick/xpad.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index 603fe0d..b94668f 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -124,6 +124,7 @@ static const struct xpad_device {
>  	{ 0x045e, 0x0291, "Xbox 360 Wireless Receiver (XBOX)", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360W },
>  	{ 0x045e, 0x0719, "Xbox 360 Wireless Receiver", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360W },
>  	{ 0x044f, 0x0f07, "Thrustmaster, Inc. Controller", 0, XTYPE_XBOX },
> +	{ 0x24c6, 0x5b02, "Thrustmaster, Inc. GPX Controller", 0, XTYPE_XBOX360 },
>  	{ 0x046d, 0xc21d, "Logitech Gamepad F310", 0, XTYPE_XBOX360 },
>  	{ 0x046d, 0xc21f, "Logitech Gamepad F710", 0, XTYPE_XBOX360 },
>  	{ 0x046d, 0xc242, "Logitech Chillstream Controller", 0, XTYPE_XBOX360 },
> -- 
> 1.8.3.2
> 

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: Mixed HID descriptors
From: David Herrmann @ 2014-02-22 17:04 UTC (permalink / raw)
  To: Nuno Santos
  Cc: open list:HID CORE LAYER, Benjamin Tissoires, Jiri Kosina,
	Henrik Rydberg
In-Reply-To: <5305E327.2020504@displax.com>

Hi

CC Jiri, Benjamin and Henrik

On Thu, Feb 20, 2014 at 12:12 PM, Nuno Santos <nsantos@displax.com> wrote:
> Hi,
>
> We are developing an HID multitouch device and we want to make it fully
> compatible with Linux and Windows.
>
> The device descriptor describes a mouse, keyboard and multitouch digitizer
> as well as a set of features. When we have mouse or keyboard descriptor
> along the touch device I can no longer get or set features. It gives me a
> timeout error:
>
> ioctl (GFEATURE): Connection timed out
>
> This only happens in Linux.
>
> I would like to know if this is normal, or if there is anyway of getting
> more information about the parsing itself in order to understand the
> problem.
>
> Below is the device report descriptor

I don't have much time to test it myself, but you can get a lot of
debug information via debugfs. Most distros enable it by default. Try
looking into /sys/kernel/debug/hid/<dev>/
These debugfs files contain runtime information about HID internals.

Thanks
David

> __ALIGN_BEGIN
> static uint8_t HID_ReportDesc[]
> __ALIGN_END =
> {
>     #if 0 // IS MESSING HID ON LINUX
>     0x05, 0x01,                            // Usage Page (Generic Desktop)
>     0x09, 0x06,                            // Usage (Keyboard)
>     0xA1, 0x01,                            // Collection (Application)
>     0x85, REPORTID_KEYBOARD,               //   REPORT_ID (Mouse)
>     0x05, 0x07,                            //   Usage page (Key Codes)
>     0x19, 0xE0,                            //   Usage minimum (224)
>     0x29, 0xE7,                            //   Usage maximum (231)
>     0x15, 0x00,                            //   Logical minimum (0)
>     0x25, 0x01,                            //   Logical maximum (1)
>     0x75, 0x01,                            //   Report size (1)
>     0x95, 0x08,                            //   Report count (8)
>     0x81, 0x02,                            //   Input (data, variable,
> absolute)
>     0x95, 0x01,                            //   Report count (1)
>     0x75, 0x08,                            //   Report size (8)
>     0x81, 0x01,                            //   Input (constant)
>     0x95, 0x06,                            //   Report count (6)
>     0x75, 0x08,                            //   Report size (8)
>     0x15, 0x00,                            //   Logical minimum (0)
>     0x25, 0x65,                            //   Logical maximum (101)
>     0x05, 0x07,                            //   Usage page (key codes)
>     0x19, 0x00,                            //   Usage minimum (0)
>     0x29, 0x65,                            //   Usage maximum (101)
>     0x81, 0x00,                            //   Input (data, array)
>     0xC0,
>     #endif
>
>     #if 0 // IS MESSING HID ON LINUX
>     0x05, 0x01,                         // USAGE_PAGE (Generic Desktop)
>     0x09, 0x01,                         // USAGE (Pointer)
>     0xa1, 0x01,                         // COLLECTION (Application)
>     0x85, REPORTID_MOUSE,               //   REPORT_ID (Mouse)
>     0x09, 0x01,                         //   USAGE (Pointer)
>     0xa1, 0x00,                         //   COLLECTION (Physical)
>     0x05, 0x09,                         //     USAGE_PAGE (Buttons)
>     0x09, 0x01,                         //     USAGE (Button 1)
>     0x95, 0x01,                         //     REPORT_COUNT (1)
>     0x75, 0x01,                         //     REPORT_SIZE (1)
>     0x15, 0x00,                         //     LOGICAL_MINIMUM (0)
>     0x25, 0x01,                         //     LOGICAL_MAXIMUM (1)
>     0x81, 0x02,                         //     INPUT (Data,Var,Abs)
>     0x95, 0x07,                         //     REPORT_COUNT (7)
>     0x75, 0x01,                         //     REPORT_SIZE (1)
>     0x81, 0x03,                         //     INPUT (Const,Var,Abs)
>     0x95, 0x08,                         //     REPORT_COUNT (8)
>     0x75, 0x01,                         //     REPORT_SIZE (1)
>     0x81, 0x03,                         //     INPUT (Const,Var,Abs)
>     0x05, 0x01,                         //     USAGE_PAGE (Generic Desktop)
>     0x09, 0x30,                         //     USAGE (X)
>     0x09, 0x31,                         //     USAGE (Y)
>     0x15, 0x00,                         //     LOGICAL_MINIMUM (0)
>     0x26, 0xff, 0x7f,                   //     LOGICAL_MAXIMUM (32767)
>     0x35, 0x00,                         //     PHYSICAL_MINIMUM (0)
>     0x46, 0x00, 0x00,                   //     PHYSICAL_MAXIMUM (0)
>     0x95, 0x02,                         //     REPORT_COUNT (2)
>     0x75, 0x10,                         //     REPORT_SIZE (16)
>     0x81, 0x02,                         //     INPUT (Data,Var,Abs)
>     0xc0,                               //   END_COLLECTION
>     0xa1, 0x02,                         //   COLLECTION (Logical)
>     0x15, 0x00,                         //     LOGICAL_MINIMUM (0)
>     0x26, 0xff, 0x00,                   //     LOGICAL_MAXIMUM (255)
>     0x09, 0x01,                         //     USAGE (Pointer)
>     0x95, 0x39,                         //     REPORT_COUNT (57)
>     0x75, 0x08,                         //     REPORT_SIZE (8)
>     0x81, 0x01,                         //     INPUT (Data,Var,Abs)
>     0xc0,                               //   END_COLLECTION
>     0xc0,                               // END_COLLECTION
>     #endif
>
>     #if 1
>     0x05, 0x0d,                            // USAGE_PAGE (Digitizers)
>     0x09, 0x0E,                         // USAGE (Configuration)
>     0xa1, 0x01,                         // COLLECTION (Application)
>     0x85, REPORTID_MODE,                 //   REPORT_ID (Feature)
>     0x09, 0x23,                         //   USAGE (Device Settings)
>     0xa1, 0x02,                         //   COLLECTION (logical)
>     0x09, 0x52,                         //     USAGE (Device Mode)
>     0x09, 0x53,                         //     USAGE (Device Index)
>     0x15, 0x00,                         //     LOGICAL_MINIMUM (0)
>     0x25, 0x28,                         //     LOGICAL_MAXIMUM (40)
>     0x75, 0x08,                         //     REPORT_SIZE (8)
>     0x95, 0x02,                         //     REPORT_COUNT (2)
>     0xb1, 0x02,                         //     FEATURE (Data,Var,Abs)
>     0xc0,                               //   END_COLLECTION
>     0xc0,                               // END_COLLECTION
>
>     0x09, 0x04,                            // USAGE (Touch Screen)
>     0xa1, 0x01,                         // COLLECTION (Application)
>     0x85, REPORTID_TOUCH,               //      REPORT_ID (Touch)
>     0x09, 0x22,                         //   USAGE (Finger)
>
>     0x05, 0x0d,                         /*     USAGE_PAGE (Digitizers)
> */
>     0xa1, 0x02,                         /*     COLLECTION (Logical)
> */
>     0x09, 0x42,                         /*       USAGE (Tip Switch)
> */
>     0x15, 0x00,                         /*       LOGICAL_MINIMUM (0)
> */
>     0x25, 0x01,                         /*       LOGICAL_MAXIMUM (1)
> */
>     0x75, 0x01,                         /*       REPORT_SIZE (1)
> */
>     0x95, 0x01,                         /*       REPORT_COUNT (1)
> */
>     0x81, 0x02,                         /*       INPUT (Data,Var,Abs)
> */
>     0x09, 0x32,                         /*       USAGE (In Range)
> */
>     0x81, 0x02,                         /*       INPUT (Data,Var,Abs)
> */
>     0x09, 0x47,                         /*       USAGE (Confidence)
> */
>     0x81, 0x02,                         /*       INPUT (Data,Var,Abs)
> */
>     0x95, 0x05,                         /*       REPORT_COUNT (5)
> */
>     0x81, 0x03,                         /*       INPUT (Cnst,Ary,Abs)
> */
>     0x09, 0x51,                         /*       USAGE (Contact Identifier)
> */
>     0x75, 0x08,                         /*       REPORT_SIZE (8)
> */
>     0x95, 0x01,                         /*       REPORT_COUNT (1)
> */
>     0x81, 0x02,                         /*       INPUT (Data,Var,Abs)
> */
>     0xa1, 0x00,                         /*       COLLECTION (Physical)
> */
>     0x05, 0x01,                         /*         USAGE_PAGE (Generic
> Desktop) */
>     0x15, 0x00,                         /*         LOGICAL_MINIMUM (0)
> */
>     0x26, 0x3A, 0x20,                   /*         LOGICAL_MAXIMUM (8200)
> */
>     0x75, 0x10,                         /*         REPORT_SIZE (16)
> */
>     0x09, 0x30,                         /*         USAGE (X)
> */
>     0x81, 0x02,                         /*         INPUT (Data,Var,Abs)
> */
>     0x15, 0x00,                         /*         LOGICAL_MINIMUM (0)
> */
>     0x26, 0x5C, 0x12,                   /*         LOGICAL_MAXIMUM (4700)
> */
>     0x09, 0x31,                         /*         USAGE (Y)
> */
>     0x81, 0x02,                         /*         INPUT (Data,Var,Abs)
> */
>     0x05, 0x0d,                         /*         USAGE_PAGE (Digitizers)
> */
>     0x09, 0x30,                         /*         USAGE (X)
> */
>     0x75, 0x10,                         /*         REPORT_SIZE (16)
> */
>     0x95, 0x01,                         /*         REPORT_COUNT (1)
> */
>     0x81, 0x02,                         /*         INPUT (Data,Var,Abs)
> */
>     0xc0,                               /* END_COLLECTION
> */
>     0xc0,                               /* END_COLLECTION
> */
>
>     0x05, 0x0d,                         //   USAGE_PAGE (Digitizers)
>     0x09, 0x54,                         //   USAGE (Actual count)
>     0x15, 0x00,                         //   LOGICAL_MAXIMUM (0)
>     0x25, 0x14,                         //   LOGICAL_MAXIMUM (20)
>     0x95, 0x01,                         //   REPORT_COUNT (1)
>     0x75, 0x08,                         //   REPORT_SIZE (8)
>     0x81, 0x02,                         //   INPUT (Data,Var,Abs)
>
>     0x85, REPORTID_MAX_COUNT,           //   REPORT_ID (Feature)
>     0x09, 0x55,                         //   USAGE(Maximum Count)
>     0x15, 0x00,                         //   LOGICAL_MINIMUM (0)
>     0x25, 0x14,                         //   LOGICAL_MAXIMUM (20)
>     0x95, 0x01,                         //   REPORT_COUNT (1)
>     0x75, 0x08,                         //   REPORT_SIZE (8)
>     0xb1, 0x02,                         //   FEATURE (Data,Var,Abs)
>     0xc0,                               // END_COLLECTION
>     #endif
>
>     #if 1
>     0x09, 0x00,                            // USAGE (Undefined)
>     0xa1, 0x01,                         // COLLECTION (Application)
>     0x85, REPORTID_ENABLE,              //   REPORT_ID (Feature)
>     0x09, 0x00,                         //   USAGE(Undefined)
>     0x95, 0x01,                         //   REPORT_COUNT (1)
>     0x75, 0x08,                         //   REPORT_SIZE (8)
>     0xb1, 0x02,                         //   FEATURE (Data,Var,Abs)
>     0x85, REPORTID_GAIN,                //   REPORT_ID (Feature)
>     0x09, 0x00,                         //   USAGE(Undefined)
>     0x15, 0x00,                         //   LOGICAL_MINIMUM (0)
>     0x25, 0x07,                         //   LOGICAL_MAXIMUM (7)
>     0x95, 0x01,                         //   REPORT_COUNT (1)
>     0x75, 0x08,                         //   REPORT_SIZE (8)
>     0xb1, 0x02,                         //   FEATURE (Data,Var,Abs)
>     0xc0,                               // END_COLLECTION
>     #endif
> };
>
> With my best regards,
>
> Nuno Santos
> www.displax.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: is it possible to temporarily not let user mode get hid input event
From: David Herrmann @ 2014-02-22 16:56 UTC (permalink / raw)
  To: loody; +Cc: open list:HID CORE LAYER, linux-usb@vger.kernel.org
In-Reply-To: <CANudz+smSLiEtTRC4t7VxzshDAdR6D4E=zYVJHT8xw0w4JiwGw@mail.gmail.com>

Hi

On Sat, Feb 22, 2014 at 5:35 PM, loody <miloody@gmail.com> wrote:
> hi all:
> is there any kernel hid module parameter or test program can
> temporarily not letting user mode program not receiving hid event?
> 1. My hid kos are still inserted in.
> 2. the kernel usb driver is working well; that mean kernel usb driver
> still handle interrupt transaction.
>
> I just not want user mode program see the hid event for a while,

For each connected HID device, there is a driver bound to it that
reads the events and forwards them to HID core. What you can do, is to
unbind a driver on a given device:
  echo "<your-device-name>" >/sys/bus/hid/drivers/<driver-name>/unbind
The device-name is the directory name in:
  /sys/bus/hid/devices/
The driver name is usually "hid-generic" but can be figured out for
each device by looking at the "driver" symlink in it's directry.
However, this is *really* just meant for debugging. This is not
recommended for anything serious. There is no support for that and if
you don't know what all this does, you shouldn't use it.

There is no proper way to disable a single device in the kernel.
User-space is supposed to control device-access so we probably won't
add such features to the kernel. If you describe your use-case in more
details, we can try to give hints how to get that working.

Thanks
David

^ permalink raw reply

* is it possible to temporarily not let user mode get hid input event
From: loody @ 2014-02-22 16:35 UTC (permalink / raw)
  To: linux-input, linux-usb@vger.kernel.org

hi all:
is there any kernel hid module parameter or test program can
temporarily not letting user mode program not receiving hid event?
1. My hid kos are still inserted in.
2. the kernel usb driver is working well; that mean kernel usb driver
still handle interrupt transaction.

I just not want user mode program see the hid event for a while,

Appreciate your kind help in advance,

^ permalink raw reply

* (unknown), 
From: christy walton @ 2014-02-22 15:00 UTC (permalink / raw)



Good day i am Mrs christy walton

I brought to you a proposal worth $ 9,000,000,000.00(Nine Billion United
State
Dollars) which i intend to use for CHARITY. Please reply me back if you
are interested.





----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.



^ permalink raw reply

* I-Tech tablet PC manufacturer
From: angelafan @ 2014-02-22  2:53 UTC (permalink / raw)
  To: linux-input; +Cc: username

Dear Sir ,
How are you?
This is Angela from i-tech company,our company was founded in 2003,existing staff of more than 100 people. 
We are commited to research, develope and manufacture fashion digital products, like Tablet PC, game player and so on.
 
Now please allowed me to introduce some hot sale models and popular product to you and your colleagues for tablet PC. 
We do promotion  for tablets for July:
7 inch Dual core IPS 3G and without 3G
7 inch Quadcore IPS screen
7.85 inch dual core IPS with 3G Bluetooth and GPS
8.1 inch dual core with 3G Bluetooth and GPS
9 inch dual core
10.1 inch quadcore IPS
10.1 inch Quadcore
10.1 inch MTK 3G bluetooth GPS
 
Variety of competitive products and solutions looking forward to you to visit our factory and consulting.
We will provide the best product quality and service to win the trust of customers
more info,please visit our website:www.i-techglobal.com.

If you are interested in please contact me by Skype angelafan714,MSN angelafanteg@hotmail.com or email at sales@i-techglobal.com,
we will be pleased to send it to you offer.



--------------------------------------------------------------------------------

thanks and warm regards, 
Angela Fan 
Sales director 
i-Tech Shenzhen Electronics Co.,Limited
TeL:0086-755-61679830 
Tel:0086-755-27791864
FAX: 86-755-61679829
Skype:angelafan714
Website:www.i-techglobal.com

^ permalink raw reply

* RE: [PATCH] input synaptics-rmi4: Use put_device() and device_type.release() to free storage.
From: Christopher Heiny @ 2014-02-21 23:29 UTC (permalink / raw)
  To: Dmitry Torokhov, Courtney Cavin
  Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
	Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
	Benjamin Tissoires, David Herrmann, Jiri Kosina
In-Reply-To: <20140213221048.GA6650@core.coreip.homeip.net>

Sorry for top posting - using web mail right now.

I think something like allocate_device and free_device will be needed sooner rather than later. I'll get a patch out for that in the next couple of days.

Chris

________________________________________
From: linux-input-owner@vger.kernel.org [linux-input-owner@vger.kernel.org] on behalf of Dmitry Torokhov [dmitry.torokhov@gmail.com]
Sent: Thursday, February 13, 2014 2:10 PM
To: Courtney Cavin
Cc: Christopher Heiny; Linux Input; Andrew Duggan; Vincent Huang; Vivian Ly; Daniel Rosenberg; Jean Delvare; Joerie de Gram; Linus Walleij; Benjamin Tissoires; David Herrmann; Jiri Kosina
Subject: Re: [PATCH] input synaptics-rmi4: Use put_device() and device_type.release() to free storage.

On Thu, Feb 13, 2014 at 01:59:31PM -0800, Courtney Cavin wrote:
> On Thu, Feb 13, 2014 at 07:15:24AM +0100, Dmitry Torokhov wrote:
> > On Wed, Feb 12, 2014 at 06:31:04PM -0800, Christopher Heiny wrote:
> > > Input: synaptics-rmi4 - Use put_device() and device_type.release()
> > > to free storage.
> > >
> > > From: Christopher Heiny <cheiny@synaptics.com>
> > >
> > > For rmi_sensor and rmi_function device_types, use put_device() and
> > > the assocated device_type.release() function to clean up related
> > > structures and storage in the correct and safe order.
> > >
> > > Allocate irq_mask as part of struct rmi_function.
> > >
> > > Delete unused rmi_driver_irq_get_mask() function.
> [...]
> > Input: synaptics-rmi4 - use put_device() to free devices
> >
> > From: Christopher Heiny <cheiny@synaptics.com>
> >
> > For rmi_sensor and rmi_function device_types, use put_device() and
> > the associated device_type->release() function to clean up related
> > structures and storage in the correct and safe order.
> >
> > Allocate irq_mask as part of struct rmi_function.
> >
> > Suggested-by: Courtney Cavin <courtney.cavin@sonymobile.com>
> > Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> >  drivers/input/rmi4/rmi_bus.c    |   30 +++++++++++++++++++++---------
> >  drivers/input/rmi4/rmi_bus.h    |    7 ++++---
> >  drivers/input/rmi4/rmi_driver.c |   25 +++++++------------------
> >  3 files changed, 32 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
> [...]
> > @@ -142,6 +150,7 @@ EXPORT_SYMBOL(rmi_unregister_transport_device);
> >  static void rmi_release_function(struct device *dev)
> >  {
> >     struct rmi_function *fn = to_rmi_function(dev);
> > +
> >     kfree(fn);
> >  }
>
> Ownership of this memory is a bit weird...
>
> [...]
> >  void rmi_unregister_function(struct rmi_function *fn)
> >  {
> > +   device_del(&fn->dev);
> >     rmi_function_teardown_debugfs(fn);
> > -   device_unregister(&fn->dev);
> > +   put_device(&fn->dev);
> >  }
>
> Here clearly the bus code owns it...
>
> > diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> [...]
> >  static int rmi_create_function(struct rmi_device *rmi_dev,
> > -                         void *ctx, const struct pdt_entry *pdt)
> > +                          void *ctx, const struct pdt_entry *pdt)
> >  {
> >     struct device *dev = &rmi_dev->dev;
> >     struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
> > @@ -630,7 +629,9 @@ static int rmi_create_function(struct rmi_device *rmi_dev,
> >     dev_dbg(dev, "Initializing F%02X for %s.\n",
> >             pdt->function_number, pdata->sensor_name);
> >
> > -   fn = kzalloc(sizeof(struct rmi_function), GFP_KERNEL);
> > +   fn = kzalloc(sizeof(struct rmi_function) +
> > +                   BITS_TO_LONGS(data->irq_count) * sizeof(unsigned long),
> > +                GFP_KERNEL);
>
> But it's allocated in the chip driver...
>
> >     if (!fn) {
> >             dev_err(dev, "Failed to allocate memory for F%02X\n",
> >                     pdt->function_number);
> > @@ -646,22 +647,12 @@ static int rmi_create_function(struct rmi_device *rmi_dev,
> >     fn->irq_pos = *current_irq_count;
> >     *current_irq_count += fn->num_of_irqs;
> >
> > -   fn->irq_mask = kzalloc(
> > -           BITS_TO_LONGS(data->irq_count) * sizeof(unsigned long),
> > -           GFP_KERNEL);
> > -   if (!fn->irq_mask) {
> > -           dev_err(dev, "%s: Failed to create irq_mask for F%02X.\n",
> > -                   __func__, pdt->function_number);
> > -           error = -ENOMEM;
> > -           goto err_free_mem;
> > -   }
> > -
> >     for (i = 0; i < fn->num_of_irqs; i++)
> >             set_bit(fn->irq_pos + i, fn->irq_mask);
> >
> >     error = rmi_register_function(fn);
> >     if (error)
> > -           goto err_free_irq_mask;
> > +           goto err_put_fn;
> >
> >     if (pdt->function_number == 0x01)
> >             data->f01_container = fn;
> > @@ -670,10 +661,8 @@ static int rmi_create_function(struct rmi_device *rmi_dev,
> >
> >     return RMI_SCAN_CONTINUE;
> >
> > -err_free_irq_mask:
> > -   kfree(fn->irq_mask);
> > -err_free_mem:
> > -   kfree(fn);
> > +err_put_fn:
> > +   put_device(&fn->dev);
>
> And the chip driver now is expected to know it's a device, and trust
> that the bus code knows how to free the memory.

Yeah. That is why for input devices I have a separate
input_allocate_device and input_free_device... But given that RMI is
pretty-much self-contained I think we can live with this.

>
> As this clearly fixes a bug or two, I say we should take this patch
> as-is and worry about proper ownership at some other time.
>
> -Courtney

--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH 1/1] Add device id for Thrustmaster GPX Gamepad for joystick/xpad.c driver
From: Marcin Lulek @ 2014-02-21 22:51 UTC (permalink / raw)
  To: Marcin Lulek, Dmitry Torokhov, Thomaz de Oliveira dos Reis,
	Paul Gortmaker, linux-input
  Cc: linux-kernel

Signed-off-by: Marcin Lulek <info@webreactor.eu>
---
 drivers/input/joystick/xpad.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 603fe0d..b94668f 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -124,6 +124,7 @@ static const struct xpad_device {
 	{ 0x045e, 0x0291, "Xbox 360 Wireless Receiver (XBOX)", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360W },
 	{ 0x045e, 0x0719, "Xbox 360 Wireless Receiver", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360W },
 	{ 0x044f, 0x0f07, "Thrustmaster, Inc. Controller", 0, XTYPE_XBOX },
+	{ 0x24c6, 0x5b02, "Thrustmaster, Inc. GPX Controller", 0, XTYPE_XBOX360 },
 	{ 0x046d, 0xc21d, "Logitech Gamepad F310", 0, XTYPE_XBOX360 },
 	{ 0x046d, 0xc21f, "Logitech Gamepad F710", 0, XTYPE_XBOX360 },
 	{ 0x046d, 0xc242, "Logitech Chillstream Controller", 0, XTYPE_XBOX360 },
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH 3.14-rc3] input: Add device id for Thrustmaster GPX Gamepad for joystick/xpad.c driver
From: Marcin Lulek @ 2014-02-21 21:59 UTC (permalink / raw)
  To: Dmitry Torokhov, Thomaz de Oliveira dos Reis, Paul Gortmaker
  Cc: linux-input, linux-kernel

Hello,

It seems xpad driver misses quite a few newer device id's for gamepads 
which leads to wrong recognition.


 From 18d5a1b290e4b39596e200449aaac6b93c035d63 Mon Sep 17 00:00:00 2001
From: Marcin Lulek <info@webreactor.eu>
Date: Fri, 21 Feb 2014 22:11:46 +0100
Subject: [PATCH 1/1] Add device id for Thrustmaster GPX Gamepad for
  joystick/xpad.c driver

Signed-off-by: Marcin Lulek <info@webreactor.eu>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Thomaz de Oliveira dos Reis <thor27@gmail.com>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
  drivers/input/joystick/xpad.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 603fe0d..b94668f 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -124,6 +124,7 @@ static const struct xpad_device {
      { 0x045e, 0x0291, "Xbox 360 Wireless Receiver (XBOX)", 
MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360W },
      { 0x045e, 0x0719, "Xbox 360 Wireless Receiver", 
MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360W },
      { 0x044f, 0x0f07, "Thrustmaster, Inc. Controller", 0, XTYPE_XBOX },
+    { 0x24c6, 0x5b02, "Thrustmaster, Inc. GPX Controller", 0, 
XTYPE_XBOX360 },
      { 0x046d, 0xc21d, "Logitech Gamepad F310", 0, XTYPE_XBOX360 },
      { 0x046d, 0xc21f, "Logitech Gamepad F710", 0, XTYPE_XBOX360 },
      { 0x046d, 0xc242, "Logitech Chillstream Controller", 0, 
XTYPE_XBOX360 },
-- 
1.8.3.2


^ permalink raw reply related

* Re: [PATCH 1/3] mfd: fsl imx25 Touchscreen ADC driver
From: Markus Pargmann @ 2014-02-21 10:18 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, Dmitry Torokhov, Samuel Ortiz,
	Lee Jones, Jonathan Cameron,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Sascha Hauer
In-Reply-To: <CAOMZO5BDyyvXs9CxJQOkuEpxZ8fqamCPB67sNhJxES+XU2XkXQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 4217 bytes --]

Hi Fabio,

On Thu, Feb 20, 2014 at 02:17:33PM -0300, Fabio Estevam wrote:
> Hi Markus,
> 
> On Thu, Feb 20, 2014 at 1:21 PM, Markus Pargmann <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
> > This is the core driver for imx25 touchscreen/adc driver. The module
> > has one shared ADC and two different conversion queues which use the
> > ADC. The two queues are identical. Both can be used for general purpose
> > ADC but one is meant to be used for touchscreens.
> >
> > This driver is the core which manages the central components and
> > registers of the TSC/ADC unit. It manages the IRQs and forwards them to
> > the correct components.
> >
> > Signed-off-by: Markus Pargmann <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> 
> That's great :-) Nice work!
> 
> > ---
> >  .../devicetree/bindings/mfd/fsl-imx25-tsadc.txt    |  46 ++++
> >  drivers/mfd/Kconfig                                |   9 +
> >  drivers/mfd/Makefile                               |   2 +
> >  drivers/mfd/fsl-imx25-tsadc.c                      | 234 +++++++++++++++++++++
> >  include/linux/mfd/imx25-tsadc.h                    | 138 ++++++++++++
> >  5 files changed, 429 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/fsl-imx25-tsadc.txt
> >  create mode 100644 drivers/mfd/fsl-imx25-tsadc.c
> >  create mode 100644 include/linux/mfd/imx25-tsadc.h
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/fsl-imx25-tsadc.txt b/Documentation/devicetree/bindings/mfd/fsl-imx25-tsadc.txt
> > new file mode 100644
> > index 0000000..a857af0e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/fsl-imx25-tsadc.txt
> > @@ -0,0 +1,46 @@
> > +Freescale mx25 ADC/TSC multifunction device
> > +
> > +This device combines two general purpose conversion queues one used for general
> > +ADC and the other used for touchscreens.
> > +
> > +Required properties:
> > + - compatible: Should be "fsl,imx25-tsadc".
> > + - reg: Memory range of the device.
> > + - interrupts: Interrupt for this device as described in
> > +   interrupts/interrupts.txt
> > + - clocks: An 'ipg' clock defined as described in clocks/clock.txt
> > + - interrupt-controller: This device is an interrupt controller. It controls
> > +   the interrupts of both conversion queues.
> > + - #interrupt-cells: Should be '<1>'.
> > + - #address-cells: Should be '<1>'.
> > + - #size-cells: Should be '<1>'.
> > + - ranges
> > +
> > +This device includes two conversion queues which can be added as subnodes.
> > +The first queue is for the touchscreen, the second for general purpose ADC.
> > +
> > +Example:
> > +       tscadc: tscadc@50030000 {
> > +               compatible = "fsl,imx25-tsadc";
> > +               reg = <0x50030000 0xc>;
> > +               interrupts = <46>;
> > +               clocks = <&clks 119>;
> > +               clock-names = "ipg";
> > +               interrupt-controller;
> > +               #interrupt-cells = <1>;
> > +               #address-cells = <1>;
> > +               #size-cells = <1>;
> > +               ranges;
> > +
> > +               tsc: tcq@50030400 {
> > +                       compatible = "fsl,imx25-tcq";
> > +                       reg = <0x50030400 0x60>;
> > +                       ...
> > +               };
> > +
> > +               adc: gcq@50030800 {
> > +                       compatible = "fsl,imx25-gcq";
> > +                       reg = <0x50030800 0x60>;
> > +                       ...
> > +               };
> > +       };
> 
> The meaning of 'tcq' and 'gcq' acronyms are not obvious. Could they be
> written more explicitily?
> 
> Also, what does the '...' mean?

I forgot to answer this one. The bindings for tcq and gcq are described
in the other files, so I didn't want to show any details that are not
relevant for this driver.

Regards,

Markus

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH 3/3] iio: adc: fsl,imx25-gcq driver
From: Markus Pargmann @ 2014-02-21 10:12 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: devicetree, linux-input, linux-iio, Dmitry Torokhov, Samuel Ortiz,
	Lee Jones, Jonathan Cameron, linux-arm-kernel, kernel
In-Reply-To: <53063F72.9070006@metafoo.de>

[-- Attachment #1: Type: text/plain, Size: 8952 bytes --]

Hi,

On Thu, Feb 20, 2014 at 06:46:26PM +0100, Lars-Peter Clausen wrote:
> On 02/20/2014 05:21 PM, Markus Pargmann wrote:
> >This is a conversion queue driver for the mx25 SoC. It uses the central
> >ADC which is used by two seperate independent queues. This driver
> >prepares different conversion configurations for each possible input.
> >For a conversion it creates a conversionqueue of one item with the
> >correct configuration for the chosen channel. It then executes the queue
> >once and disables the conversion queue afterwards.
> >
> >The reference voltages are configurable through devicetree subnodes,
> >depending on the connections of the ADC inputs.
> >
> >Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> 
> Looks good mostly. Just a couple of minor code-style issues. Try to
> run checkpatch, sparse and friends on your driver before submission.
> They catch most of the stuff that needs to be fixed in this driver.

I used checkpatch and will have a look at sparse.

> 
> [..]
> >diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> >index 2209f28..a421445 100644
> >--- a/drivers/iio/adc/Kconfig
> >+++ b/drivers/iio/adc/Kconfig
> >@@ -113,6 +113,13 @@ config EXYNOS_ADC
> >  	  of SoCs for drivers such as the touchscreen and hwmon to use to share
> >  	  this resource.
> >
> >+config MX25_ADC
> >+	tristate "Freescale MX25 ADC driver"
> >+	depends on MFD_MX25_TSADC
> >+	help
> >+	  Generic Conversion Queue driver used for general purpose ADC in the
> >+	  MX25. This driver supports single measurements using the MX25 ADC.
> >+
> 
> alphabetical order

I changed the config option name to "FSL_MX25_ADC" to have it in
alphabetical order for the user visible menuconfig and the config
symbols.

> 
> >  config LP8788_ADC
> >  	bool "LP8788 ADC driver"
> >  	depends on MFD_LP8788
> >diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> >index ba9a10a..63daa2c 100644
> >--- a/drivers/iio/adc/Makefile
> >+++ b/drivers/iio/adc/Makefile
> >@@ -22,3 +22,4 @@ obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
> >  obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
> >  obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
> >  obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
> >+obj-$(CONFIG_MX25_ADC) += fsl-imx25-gcq.o
> 
> Here too

Fixed.

> 
> [...]
> 
> >+const struct iio_chan_spec mx25_gcq_channels[MX25_NUM_CFGS] = {
> 
> static

Fixed.

> 
> >+	MX25_IIO_CHAN(0, "xp"),
> >+	MX25_IIO_CHAN(1, "yp"),
> >+	MX25_IIO_CHAN(2, "xn"),
> >+	MX25_IIO_CHAN(3, "yn"),
> >+	MX25_IIO_CHAN(4, "wiper"),
> >+	MX25_IIO_CHAN(5, "inaux0"),
> >+	MX25_IIO_CHAN(6, "inaux1"),
> >+	MX25_IIO_CHAN(7, "inaux2"),
> >+};
> >+
> 
> >+struct iio_info mx25_gcq_iio_info = {
> 
> static const

Fixed.

> 
> >+	.read_raw = mx25_gcq_read_raw,
> >+};
> >+
> >+static struct regmap_config mx25_gcq_regconfig = {
> 
> const

Fixed.

> 
> >+	.fast_io = true,
> 
> You don't need to set fast_io since this is already done at the
> regmap_bus level.

Okay, I removed it.

> 
> >+	.max_register = 0x5c,
> >+	.reg_bits = 32,
> >+	.val_bits = 32,
> >+	.reg_stride = 4,
> >+};
> >+
> >+static void mx25_gcq_iio_dev_setup(struct platform_device *pdev,
> >+		struct iio_dev *idev)
> >+{
> >+	idev->dev.parent = &pdev->dev;
> >+	idev->channels = mx25_gcq_channels;
> >+	idev->num_channels = ARRAY_SIZE(mx25_gcq_channels);
> >+	idev->info = &mx25_gcq_iio_info;
> 
> Any reason why this needs to be in a separate function and can't be
> inlined in probe()?

No reason for that. I moved it back to probe().

> 
> >+}
> >+
> >+static int mx25_gcq_setup_cfgs(struct platform_device *pdev,
> >+		struct mx25_gcq_priv *priv)
> >+{
> >+	struct device_node *np = pdev->dev.of_node;
> >+	struct device_node *child;
> >+	struct device *dev = &pdev->dev;
> >+	int ret;
> >+	int i;
> >+
> >+	/* Setup all configurations registers with a default conversion
> >+	 * configuration for each input */
> >+	for (i = 0; i != MX25_NUM_CFGS; ++i)
> >+		regmap_write(priv->regs, MX25_ADCQ_CFG(i),
> >+				MX25_ADCQ_CFG_YPLL_OFF |
> >+				MX25_ADCQ_CFG_XNUR_OFF |
> >+				MX25_ADCQ_CFG_XPUL_OFF |
> >+				MX25_ADCQ_CFG_REFP_INT |
> >+				(i << 4) |
> >+				MX25_ADCQ_CFG_REFN_NGND2);
> >+
> >+	for_each_child_of_node(np, child) {
> >+		u32 reg;
> >+		u32 refn;
> >+		u32 refp;
> >+
> >+		ret = of_property_read_u32(child, "reg", &reg);
> >+		if (ret) {
> >+			dev_err(dev, "Failed to get reg property\n");
> >+			return ret;
> >+		}
> >+		if (reg > MX25_NUM_CFGS) {
> >+			dev_err(dev, "reg value is greater than the number of available configuration registers\n");
> >+			return -EINVAL;
> >+		}
> >+
> >+		ret = of_property_read_u32(child, "fsl,adc-refn", &refn);
> >+		if (ret) {
> >+			dev_err(dev, "Failed to get fsl,adc-refn property\n");
> >+			return ret;
> >+		}
> >+
> >+		ret = of_property_read_u32(child, "fsl,adc-refp", &refp);
> >+		if (ret) {
> >+			dev_err(dev, "Failed to get fsl,adc-refp property\n");
> >+			return ret;
> >+		}
> 
> Range check for refp and refn?

Range check added for both.

> 
> >+
> >+		regmap_update_bits(priv->regs, MX25_ADCQ_CFG(reg),
> >+				MX25_ADCQ_CFG_REFP_MASK |
> >+				MX25_ADCQ_CFG_REFN_MASK,
> >+				(refp << 7) | (refn << 2));
> >+	}
> >+	regmap_update_bits(priv->regs, MX25_ADCQ_CR,
> >+			MX25_ADCQ_CR_FRST | MX25_ADCQ_CR_QRST,
> >+			MX25_ADCQ_CR_FRST | MX25_ADCQ_CR_QRST);
> >+
> >+	regmap_write(priv->regs, MX25_ADCQ_CR,
> >+			MX25_ADCQ_CR_PDMSK |
> >+			MX25_ADCQ_CR_QSM_FQS);
> >+
> >+	return 0;
> >+}
> >+
> >+static int mx25_gcq_probe(struct platform_device *pdev)
> >+{
> >+	struct iio_dev *idev;
> >+	struct mx25_gcq_priv *priv;
> >+	struct resource *res;
> >+	struct device *dev = &pdev->dev;
> >+	int ret;
> >+	int irq;
> >+	void __iomem *mem;
> >+
> >+	idev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
> >+	if (!idev)
> >+		return -ENOMEM;
> >+
> >+	priv = iio_priv(idev);
> >+
> >+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >+	mem = devm_ioremap_resource(dev, res);
> >+	if (!mem) {
> >+		dev_err(dev, "Failed to get iomem");
> 
> devm_ioremap_resource already prints a error message for you

Okay, I removed dev_err.
> 
> >+		return -ENOMEM;
> >+	}
> >+
> >+	priv->regs = devm_regmap_init_mmio(dev, mem, &mx25_gcq_regconfig);
> >+	if (IS_ERR(priv->regs)) {
> >+		dev_err(dev, "Failed to initialize regmap\n");
> >+		return PTR_ERR(priv->regs);
> >+	}
> >+
> >+	irq = platform_get_irq(pdev, 0);
> >+	if (irq < 0) {
> 
> 0 is not a valid IRQ number either

Fixed.
> 
> >+		dev_err(dev, "Failed to get IRQ\n");
> >+		return irq;
> >+	}
> >+
> >+	ret = devm_request_irq(dev, irq, mx25_gcq_irq, IRQF_ONESHOT, pdev->name,
> >+			priv);
> 
> IRQF_ONESHOT does not make much sense for non-threaded IRQs. Also it
> is probably safer to use the non managed variant of request_irq
> here.

Right, I removed the IRQF_ONESHOT flag. Why is it safer to use the non
managed request_irq?

> 
> >+	if (ret) {
> >+		dev_err(dev, "Failed requesting IRQ\n");
> 
> It usually makes sense to include the error number in the message.
> Same for the other error messages in probe.

The error values are returned from the probe function, so the error
number should be displayed by really_probe() in drivers/base/dd.c .

> 
> >+		return ret;
> >+	}
> >+
> >+	ret = mx25_gcq_setup_cfgs(pdev, priv);
> >+	if (ret)
> >+		return ret;
> >+
> >+	mx25_gcq_iio_dev_setup(pdev, idev);
> >+
> >+	ret = iio_device_register(idev);
> >+	if (ret) {
> >+		dev_err(dev, "Failed to register iio device\n");
> >+		return ret;
> >+	}
> >+
> >+	init_completion(&priv->completed);
> 
> Should be done before the interrupt handler is registered. You
> reference it in there.
> 
> >+
> >+	priv->clk = mx25_tsadc_get_ipg(pdev->dev.parent);
> >+	ret = clk_prepare_enable(priv->clk);
> 
> The clock should probably also be enabled before the interrupt
> handler and the IIO device are registered.

Fixed. The last two items are request_irq() and iio_device_register().

> 
> >+	if (ret) {
> >+		dev_err(dev, "Failed to enable clock\n");
> >+		return ret;
> >+	}
> >+
> >+	platform_set_drvdata(pdev, priv);
> >+
> >+	return 0;
> >+}
> >+
> >+static int mx25_gcq_remove(struct platform_device *pdev)
> >+{
> >+	struct mx25_gcq_priv *priv = platform_get_drvdata(pdev);
> >+
> 
> iio_device_unregister().

Fixed.
> 
> >+	clk_disable_unprepare(priv->clk);
> >+
> >+	return 0;
> >+}
> 
> 

Thanks,

Markus

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH 1/3] mfd: fsl imx25 Touchscreen ADC driver
From: Markus Pargmann @ 2014-02-21  9:18 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, Dmitry Torokhov, Samuel Ortiz,
	Lee Jones, Jonathan Cameron,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Sascha Hauer
In-Reply-To: <CAOMZO5BDyyvXs9CxJQOkuEpxZ8fqamCPB67sNhJxES+XU2XkXQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 8010 bytes --]

Hi Fabio,

On Thu, Feb 20, 2014 at 02:17:33PM -0300, Fabio Estevam wrote:
> Hi Markus,
> 
> On Thu, Feb 20, 2014 at 1:21 PM, Markus Pargmann <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
> > This is the core driver for imx25 touchscreen/adc driver. The module
> > has one shared ADC and two different conversion queues which use the
> > ADC. The two queues are identical. Both can be used for general purpose
> > ADC but one is meant to be used for touchscreens.
> >
> > This driver is the core which manages the central components and
> > registers of the TSC/ADC unit. It manages the IRQs and forwards them to
> > the correct components.
> >
> > Signed-off-by: Markus Pargmann <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> 
> That's great :-) Nice work!

Thanks :)

> > ---
> >  .../devicetree/bindings/mfd/fsl-imx25-tsadc.txt    |  46 ++++
> >  drivers/mfd/Kconfig                                |   9 +
> >  drivers/mfd/Makefile                               |   2 +
> >  drivers/mfd/fsl-imx25-tsadc.c                      | 234 +++++++++++++++++++++
> >  include/linux/mfd/imx25-tsadc.h                    | 138 ++++++++++++
> >  5 files changed, 429 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/fsl-imx25-tsadc.txt
> >  create mode 100644 drivers/mfd/fsl-imx25-tsadc.c
> >  create mode 100644 include/linux/mfd/imx25-tsadc.h
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/fsl-imx25-tsadc.txt b/Documentation/devicetree/bindings/mfd/fsl-imx25-tsadc.txt
> > new file mode 100644
> > index 0000000..a857af0e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/fsl-imx25-tsadc.txt
> > @@ -0,0 +1,46 @@
> > +Freescale mx25 ADC/TSC multifunction device
> > +
> > +This device combines two general purpose conversion queues one used for general
> > +ADC and the other used for touchscreens.
> > +
> > +Required properties:
> > + - compatible: Should be "fsl,imx25-tsadc".
> > + - reg: Memory range of the device.
> > + - interrupts: Interrupt for this device as described in
> > +   interrupts/interrupts.txt
> > + - clocks: An 'ipg' clock defined as described in clocks/clock.txt
> > + - interrupt-controller: This device is an interrupt controller. It controls
> > +   the interrupts of both conversion queues.
> > + - #interrupt-cells: Should be '<1>'.
> > + - #address-cells: Should be '<1>'.
> > + - #size-cells: Should be '<1>'.
> > + - ranges
> > +
> > +This device includes two conversion queues which can be added as subnodes.
> > +The first queue is for the touchscreen, the second for general purpose ADC.
> > +
> > +Example:
> > +       tscadc: tscadc@50030000 {
> > +               compatible = "fsl,imx25-tsadc";
> > +               reg = <0x50030000 0xc>;
> > +               interrupts = <46>;
> > +               clocks = <&clks 119>;
> > +               clock-names = "ipg";
> > +               interrupt-controller;
> > +               #interrupt-cells = <1>;
> > +               #address-cells = <1>;
> > +               #size-cells = <1>;
> > +               ranges;
> > +
> > +               tsc: tcq@50030400 {
> > +                       compatible = "fsl,imx25-tcq";
> > +                       reg = <0x50030400 0x60>;
> > +                       ...
> > +               };
> > +
> > +               adc: gcq@50030800 {
> > +                       compatible = "fsl,imx25-gcq";
> > +                       reg = <0x50030800 0x60>;
> > +                       ...
> > +               };
> > +       };
> 
> The meaning of 'tcq' and 'gcq' acronyms are not obvious. Could they be
> written more explicitily?

I assume you mean the node names? Perhaps something like 'adcqueue'?

> 
> Also, what does the '...' mean?
> 
> > +static int mx25_tsadc_domain_map(struct irq_domain *d, unsigned int irq,
> > +                            irq_hw_number_t hwirq)
> > +{
> > +       struct mx25_tsadc_priv *priv = d->host_data;
> > +
> > +       irq_set_chip_data(irq, priv);
> > +       irq_set_chip_and_handler(irq, &mx25_tsadc_irq_chip,
> > +                                handle_level_irq);
> > +
> > +#ifdef CONFIG_ARM
> > +       set_irq_flags(irq, IRQF_VALID);
> > +#else
> > +       irq_set_noprobe(irq);
> > +#endif
> 
> Do we really need these ifdef's? Can't we just assume that CONFIG_ARM
> is always selected for this driver?

Yes right, this is not necessary here.

> 
> > +static int mx25_tsadc_probe(struct platform_device *pdev)
> > +{
> > +       struct device *dev = &pdev->dev;
> > +       struct device_node *np = dev->of_node;
> > +       struct mx25_tsadc_priv *priv;
> > +       struct resource *iores;
> > +       int ret;
> > +       void __iomem *iomem;
> > +
> > +       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > +       if (!priv)
> > +               return -ENOMEM;
> > +
> > +       iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +       iomem = devm_ioremap_resource(dev, iores);
> > +       if (IS_ERR(iomem)) {
> > +               dev_err(dev, "Failed to remap iomem\n");
> > +               return PTR_ERR(iomem);
> > +       }
> > +
> > +       priv->regs = regmap_init_mmio(dev, iomem, &mx25_tsadc);
> > +       if (IS_ERR(priv->regs)) {
> > +               dev_err(dev, "Failed to initialize regmap\n");
> > +               return PTR_ERR(priv->regs);
> > +       }
> > +
> > +       priv->clk = devm_clk_get(dev, "ipg");
> > +       if (IS_ERR(priv->clk)) {
> > +               dev_err(dev, "Failed to get ipg clock\n");
> > +               return PTR_ERR(priv->clk);
> > +       }
> > +
> > +       /* Enable clock and reset the component */
> > +       regmap_update_bits(priv->regs, MX25_TSC_TGCR, MX25_TGCR_CLK_EN,
> > +                       MX25_TGCR_CLK_EN);
> > +       regmap_update_bits(priv->regs, MX25_TSC_TGCR, MX25_TGCR_TSC_RST,
> > +                       MX25_TGCR_TSC_RST);
> > +
> > +       /* Setup powersaving mode, but enable internal reference voltage */
> > +       regmap_update_bits(priv->regs, MX25_TSC_TGCR, MX25_TGCR_POWERMODE_MASK,
> > +                       MX25_TGCR_POWERMODE_SAVE);
> > +       regmap_update_bits(priv->regs, MX25_TSC_TGCR, MX25_TGCR_INTREFEN,
> > +                       MX25_TGCR_INTREFEN);
> > +
> > +       ret = mx25_tsadc_setup_irq(pdev, priv);
> > +       if (ret) {
> > +               dev_err(dev, "Failed to setup irqs\n");
> > +               return ret;
> > +       }
> > +
> > +       platform_set_drvdata(pdev, priv);
> > +
> > +       of_platform_populate(np, NULL, NULL, dev);
> > +
> > +       dev_info(dev, "i.MX25/25 Touchscreen and ADC core driver loaded\n");
> 
> You could remove the double '25/25'.

Yes, fixed.

> 
> > +
> > +       return 0;
> > +}
> > +
> > +static const struct of_device_id mx25_tsadc_ids[] = {
> > +       { .compatible = "fsl,imx25-tsadc", },
> > +       { /* Sentinel */ }
> > +};
> > +
> > +static struct platform_driver mx25_tsadc_driver = {
> > +       .driver         = {
> > +               .name   = "mx25-tsadc",
> > +               .owner  = THIS_MODULE,
> > +               .of_match_table = mx25_tsadc_ids,
> > +       },
> > +       .probe          = mx25_tsadc_probe,
> > +};
> > +module_platform_driver(mx25_tsadc_driver);
> > +
> > +MODULE_DESCRIPTION("MFD for ADC/TSC for Freescale mx25");
> > +MODULE_AUTHOR("Markus Pargmann <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>");
> > +MODULE_LICENSE("GPL v2");
> 
> MODULE_ALIAS() as well?

I am not sure if this is necessary, but I added
MODULE_ALIAS("platform:mx25-tsadc")

Thanks,

Markus

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* RE: [patch] Input: remove a duplicative NULL test
From: fugang.duan @ 2014-02-21  9:14 UTC (permalink / raw)
  To: Dan Carpenter, Dmitry Torokhov
  Cc: Paul Gortmaker, Jingoo Han, Benson Leung, Daniel Kurtz,
	linux-input@vger.kernel.org, kernel-janitors@vger.kernel.org
In-Reply-To: <20140221085506.GB13185@elgon.mountain>

From: Dan Carpenter <dan.carpenter@oracle.com>
Data: Friday, February 21, 2014 4:55 PM

>To: Dmitry Torokhov
>Cc: Paul Gortmaker; Jingoo Han; Duan Fugang-B38611; Benson Leung; Daniel Kurtz;
>linux-input@vger.kernel.org; kernel-janitors@vger.kernel.org
>Subject: [patch] Input: remove a duplicative NULL test
>
>"pdata" is non-NULL here.  We verified that at the start of the function.
>
>Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
>diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c
>b/drivers/input/touchscreen/atmel_mxt_ts.c
>index a70400754e92..40abe90cc924 100644
>--- a/drivers/input/touchscreen/atmel_mxt_ts.c
>+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
>@@ -1146,7 +1146,7 @@ static int mxt_probe(struct i2c_client *client,
> 		goto err_free_mem;
> 	}
>
>-	data->is_tp = pdata && pdata->is_tp;
>+	data->is_tp = pdata->is_tp;
>
> 	input_dev->name = (data->is_tp) ? "Atmel maXTouch Touchpad" :
> 					  "Atmel maXTouch Touchscreen";
>
Agree, it is redundant. And if you have free time, you can convert the driver to support devicetree.

Acked-by: Fugang Duan <B38611@freescale.com>

^ permalink raw reply

* [patch] Input: remove a duplicative NULL test
From: Dan Carpenter @ 2014-02-21  8:55 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Paul Gortmaker, Jingoo Han, Fugang Duan, Benson Leung,
	Daniel Kurtz, linux-input, kernel-janitors

"pdata" is non-NULL here.  We verified that at the start of the
function.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index a70400754e92..40abe90cc924 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1146,7 +1146,7 @@ static int mxt_probe(struct i2c_client *client,
 		goto err_free_mem;
 	}
 
-	data->is_tp = pdata && pdata->is_tp;
+	data->is_tp = pdata->is_tp;
 
 	input_dev->name = (data->is_tp) ? "Atmel maXTouch Touchpad" :
 					  "Atmel maXTouch Touchscreen";

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox