Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH] Input: add driver for Neonode zForce based touchscreens
From: Henrik Rydberg @ 2013-09-01 12:57 UTC (permalink / raw)
  To: Heiko Stübner; +Cc: Dmitry Torokhov, linux-kernel, linux-input
In-Reply-To: <201308161359.40134.heiko@sntech.de>

Hi Heiko,

> This adds a driver for touchscreens using the zforce infrared
> technology from Neonode connected via i2c to the host system.
> 
> It supports multitouch with up to two fingers and tracking of the
> contacts in hardware.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

Thanks for the driver. Please find some comments below.

> +static int zforce_touch_event(struct zforce_ts *ts, u8 *payload)
> +{
> +	struct i2c_client *client = ts->client;
> +	struct zforce_point point[ZFORCE_REPORT_POINTS];

You do not really need the array here, do you? A single member should
suffice, stack is precious.

> +	const struct zforce_ts_platdata *pdata = client->dev.platform_data;
> +	int count, i;
> +
> +	count = payload[0];
> +	if (count > ZFORCE_REPORT_POINTS) {
> +		dev_warn(&client->dev, "to many coordinates %d, expected max %d\n",
> +			 count, ZFORCE_REPORT_POINTS);
> +		count = ZFORCE_REPORT_POINTS;
> +	}
> +
> +	for (i = 0; i < count; i++) {
> +		point[i].coord_x =
> +			payload[9 * i + 2] << 8 | payload[9 * i + 1];
> +		point[i].coord_y =
> +			payload[9 * i + 4] << 8 | payload[9 * i + 3];
> +
> +		if (point[i].coord_x > pdata->x_max ||
> +		    point[i].coord_y > pdata->y_max) {
> +			dev_warn(&client->dev, "coordinates (%d,%d) invalid\n",
> +				point[i].coord_x, point[i].coord_y);
> +			point[i].coord_x = point[i].coord_y = 0;
> +		}
> +
> +		point[i].state = payload[9 * i + 5] & 0x03;
> +		point[i].id = (payload[9 * i + 5] & 0xfc) >> 2;
> +
> +		/* determine touch major, minor and orientation */
> +		point[i].area_major = max(payload[9 * i + 6],
> +					  payload[9 * i + 7]);
> +		point[i].area_minor = min(payload[9 * i + 6],
> +					  payload[9 * i + 7]);
> +		point[i].orientation = payload[9 * i + 6] > payload[9 * i + 7];
> +
> +		point[i].pressure = payload[9 * i + 8];
> +		point[i].prblty = payload[9 * i + 9];
> +	}
> +
> +	for (i = 0; i < count; i++) {

just continue the loop here (or move the conversion out as a function).

> +		dev_dbg(&client->dev, "point %d/%d: state %d, id %d, pressure %d, prblty %d, x %d, y %d, amajor %d, aminor %d, ori %d\n",
> +			i, count, point[i].state, point[i].id,
> +			point[i].pressure, point[i].prblty,
> +			point[i].coord_x, point[i].coord_y,
> +			point[i].area_major, point[i].area_minor,
> +			point[i].orientation);
> +
> +		/* the zforce id starts with "1", so needs to be decreased */
> +		input_mt_slot(ts->input, point[i].id - 1);
> +
> +		input_mt_report_slot_state(ts->input, MT_TOOL_FINGER,
> +						point[i].state != STATE_UP);
> +
> +		if (point[i].state != STATE_UP) {
> +			input_report_abs(ts->input, ABS_MT_POSITION_X,
> +					 point[i].coord_x);
> +			input_report_abs(ts->input, ABS_MT_POSITION_Y,
> +					 point[i].coord_y);
> +			input_report_abs(ts->input, ABS_MT_TOUCH_MAJOR,
> +					 point[i].area_major);
> +			input_report_abs(ts->input, ABS_MT_TOUCH_MINOR,
> +					 point[i].area_minor);
> +			input_report_abs(ts->input, ABS_MT_ORIENTATION,
> +					 point[i].orientation);
> +		}
> +	}
> +
> +	if (point[0].state != STATE_UP) {
> +		input_report_abs(ts->input, ABS_X, point[0].coord_x);
> +		input_report_abs(ts->input, ABS_Y, point[0].coord_y);
> +	}
> +
> +	input_report_key(ts->input, BTN_TOUCH, point[0].state != STATE_UP);

You can use input_mt_sync() here instead of the single-touch stuff.

> +
> +	input_sync(ts->input);
> +
> +	return 0;
> +}
> +
> +static int zforce_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	const struct zforce_ts_platdata *pdata = client->dev.platform_data;
> +	struct zforce_ts *ts;
> +	struct input_dev *input_dev;
> +	int ret;
> +
> +	if (!pdata)
> +		return -EINVAL;
> +
> +	ts = devm_kzalloc(&client->dev, sizeof(struct zforce_ts), GFP_KERNEL);
> +	if (!ts)
> +		return -ENOMEM;
> +
> +	ret = devm_gpio_request_one(&client->dev, pdata->gpio_int, GPIOF_IN,
> +				    "zforce_ts_int");
> +	if (ret) {
> +		dev_err(&client->dev, "request of gpio %d failed, %d\n",
> +			pdata->gpio_int, ret);
> +		return ret;
> +	}
> +
> +	ret = devm_gpio_request_one(&client->dev, pdata->gpio_rst,
> +				    GPIOF_OUT_INIT_LOW, "zforce_ts_rst");
> +	if (ret) {
> +		dev_err(&client->dev, "request of gpio %d failed, %d\n",
> +			pdata->gpio_rst, ret);
> +		return ret;
> +	}
> +
> +	ret = devm_add_action(&client->dev, zforce_reset, ts);
> +	if (ret) {
> +		dev_err(&client->dev, "failed to register reset action, %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	msleep(20);
> +
> +	snprintf(ts->phys, sizeof(ts->phys),
> +		 "%s/input0", dev_name(&client->dev));
> +
> +	input_dev = devm_input_allocate_device(&client->dev);
> +	if (!input_dev) {
> +		dev_err(&client->dev, "could not allocate input device\n");
> +		return -ENOMEM;
> +	}
> +
> +	mutex_init(&ts->access_mutex);
> +	mutex_init(&ts->command_mutex);
> +
> +	ts->pdata = pdata;
> +	ts->client = client;
> +	ts->input = input_dev;
> +
> +	input_dev->name = "Neonode zForce touchscreen";
> +	input_dev->phys = ts->phys;
> +	input_dev->id.bustype = BUS_I2C;
> +	input_dev->dev.parent = &client->dev;
> +
> +	input_dev->open = zforce_input_open;
> +	input_dev->close = zforce_input_close;
> +
> +	set_bit(EV_KEY, input_dev->evbit);
> +	set_bit(EV_SYN, input_dev->evbit);
> +	set_bit(EV_ABS, input_dev->evbit);
> +	set_bit(BTN_TOUCH, input_dev->keybit);

BTN_TOUCH can be skipped,

> +
> +	/* For single touch */
> +	input_set_abs_params(input_dev, ABS_X, 0, pdata->x_max, 0, 0);
> +	input_set_abs_params(input_dev, ABS_Y, 0, pdata->y_max, 0, 0);

and these too,

> +
> +	/* For multi touch */
> +	input_mt_init_slots(input_dev, ZFORCE_REPORT_POINTS, 0);

if you add INPUT_MT_DIRECT as argument here.

> +	input_set_abs_params(input_dev, ABS_MT_POSITION_X, 0,
> +			     pdata->x_max, 0, 0);
> +	input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0,
> +			     pdata->y_max, 0, 0);
> +
> +	input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0,
> +			     ZFORCE_MAX_AREA, 0, 0);
> +	input_set_abs_params(input_dev, ABS_MT_TOUCH_MINOR, 0,
> +			     ZFORCE_MAX_AREA, 0, 0);
> +	input_set_abs_params(input_dev, ABS_MT_ORIENTATION, 0, 1, 0, 0);
> +
> +	input_set_drvdata(ts->input, ts);
> +
> +	init_completion(&ts->command_done);
> +
> +	/* The zforce pulls the interrupt low when it has data ready.
> +	 * After it is triggered the isr thread runs until all the available
> +	 * packets have been read and the interrupt is high again.
> +	 * Therefore we can trigger the interrupt anytime it is low and do
> +	 * not need to limit it to the interrupt edge.
> +	 */
> +	ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> +					zforce_interrupt,
> +					IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> +					input_dev->name, ts);
> +	if (ret) {
> +		dev_err(&client->dev, "irq %d request failed\n", client->irq);
> +		return ret;
> +	}
> +
> +	i2c_set_clientdata(client, ts);
> +
> +	/* let the controller boot */
> +	gpio_set_value(pdata->gpio_rst, 1);
> +
> +	ts->command_waiting = NOTIFICATION_BOOTCOMPLETE;
> +	if (wait_for_completion_timeout(&ts->command_done, WAIT_TIMEOUT) == 0)
> +		dev_warn(&client->dev, "bootcomplete timed out\n");
> +
> +	/* need to start device to get version information */
> +	ret = zforce_command_wait(ts, COMMAND_INITIALIZE);
> +	if (ret) {
> +		dev_err(&client->dev, "unable to initialize, %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* this gets the firmware version among other informations */
> +	ret = zforce_command_wait(ts, COMMAND_STATUS);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "couldn't get status, %d\n", ret);
> +		zforce_stop(ts);
> +		return ret;
> +	}
> +
> +	/* stop device and put it into sleep until it is opened */
> +	ret = zforce_stop(ts);
> +	if (ret < 0)
> +		return ret;
> +
> +	device_set_wakeup_capable(&client->dev, true);
> +
> +	ret = input_register_device(input_dev);
> +	if (ret) {
> +		dev_err(&client->dev, "could not register input device, %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}

Thanks,
Henrik

^ permalink raw reply

* [PATCH V8 0/2] iio: input: ti_am335x_tscadc: Add continuous sampling support to adc
From: Zubair Lutfullah @ 2013-09-01 11:17 UTC (permalink / raw)
  To: jic23, dmitry.torokhov
  Cc: linux-iio, linux-input, linux-kernel, bigeasy, gregkh,
	zubair.lutfullah

Round 8 updates
Rebased to togreg branch of IIO..

Round 7 updates
- Fixes to error handling path for trigger register

- Optimization of step enable calculations based
on input from Sebastian

- trigger unregister added to remove section. was missing

- missing entry in Kconfig for triggered buffer support. added

- TSC/ADC IRQ handling was based on wierd ack system. not required. removed.
each irq handler deals with its own stuff *only*

Thanks for all the input

Regards
Zubair Lutfullah Kakakhel

Zubair Lutfullah (2):
  input: ti_am335x_tsc: Enable shared IRQ for TSC
  iio: ti_am335x_adc: Add continuous sampling support

 drivers/iio/adc/Kconfig                   |    1 +
 drivers/iio/adc/ti_am335x_adc.c           |  243 ++++++++++++++++++++++++++---
 drivers/input/touchscreen/ti_am335x_tsc.c |   12 +-
 include/linux/mfd/ti_am335x_tscadc.h      |   13 ++
 4 files changed, 246 insertions(+), 23 deletions(-)

-- 
1.7.9.5


^ permalink raw reply

* [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support
From: Zubair Lutfullah @ 2013-09-01 11:17 UTC (permalink / raw)
  To: jic23, dmitry.torokhov
  Cc: linux-iio, linux-input, linux-kernel, bigeasy, gregkh,
	zubair.lutfullah
In-Reply-To: <1378034277-26728-1-git-send-email-zubair.lutfullah@gmail.com>

Previously the driver had only one-shot reading functionality.
This patch adds triggered buffer support to the driver.

Continuous sampling starts when buffer is enabled.
And samples are pushed to userpace by the trigger which
triggers automatically at every hardware interrupt
of FIFO1 filling with samples upto threshold value.

Userspace responsibility to stop sampling by writing zero
in the buffer enable file.

Patil Rachna (TI) laid the ground work for ADC HW register access.
Russ Dill (TI) fixed bugs in the driver relevant to FIFOs and IRQs.

I fixed channel scanning so multiple ADC channels can be read
simultaneously and pushed to userspace.
Restructured the driver to fit IIO ABI.
And added trigger support.

Signed-off-by: Zubair Lutfullah <zubair.lutfullah@gmail.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Russ Dill <Russ.Dill@ti.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
Acked-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/iio/adc/Kconfig              |    1 +
 drivers/iio/adc/ti_am335x_adc.c      |  243 +++++++++++++++++++++++++++++++---
 include/linux/mfd/ti_am335x_tscadc.h |   13 ++
 3 files changed, 237 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 09371cb..cfa2039 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -167,6 +167,7 @@ config TI_ADC081C
 config TI_AM335X_ADC
 	tristate "TI's AM335X ADC driver"
 	depends on MFD_TI_AM335X_TSCADC
+	select IIO_TRIGGERED_BUFFER
 	help
 	  Say yes here to build support for Texas Instruments ADC
 	  driver which is also a MFD client.
diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index a952538..1626e16 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -24,16 +24,23 @@
 #include <linux/iio/iio.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
-#include <linux/iio/machine.h>
 #include <linux/iio/driver.h>
 
 #include <linux/mfd/ti_am335x_tscadc.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
 
 struct tiadc_device {
 	struct ti_tscadc_dev *mfd_tscadc;
 	int channels;
 	u8 channel_line[8];
 	u8 channel_step[8];
+	int irq;
+	int buffer_en_ch_steps;
+	struct iio_trigger *trig;
+	u32 *data;
 };
 
 static unsigned int tiadc_readl(struct tiadc_device *adc, unsigned int reg)
@@ -56,10 +63,16 @@ static u32 get_adc_step_mask(struct tiadc_device *adc_dev)
 	return step_en;
 }
 
-static void tiadc_step_config(struct tiadc_device *adc_dev)
+static u32 get_adc_step_bit(struct tiadc_device *adc_dev, int chan)
 {
+	return 1 << adc_dev->channel_step[chan];
+}
+
+static void tiadc_step_config(struct iio_dev *indio_dev)
+{
+	struct tiadc_device *adc_dev = iio_priv(indio_dev);
 	unsigned int stepconfig;
-	int i, steps;
+	int i, steps, chan;
 
 	/*
 	 * There are 16 configurable steps and 8 analog input
@@ -72,11 +85,13 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
 	 */
 
 	steps = TOTAL_STEPS - adc_dev->channels;
-	stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1;
+	if (iio_buffer_enabled(indio_dev))
+		stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1
+					| STEPCONFIG_MODE_SWCNT;
+	else
+		stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1;
 
 	for (i = 0; i < adc_dev->channels; i++) {
-		int chan;
-
 		chan = adc_dev->channel_line[i];
 		tiadc_writel(adc_dev, REG_STEPCONFIG(steps),
 				stepconfig | STEPCONFIG_INP(chan));
@@ -85,7 +100,167 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
 		adc_dev->channel_step[i] = steps;
 		steps++;
 	}
+}
+
+static irqreturn_t tiadc_irq(int irq, void *private)
+{
+	struct iio_dev *indio_dev = private;
+	struct tiadc_device *adc_dev = iio_priv(indio_dev);
+	unsigned int status, config;
+	status = tiadc_readl(adc_dev, REG_IRQSTATUS);
+
+	/*
+	 * ADC and touchscreen share the IRQ line.
+	 * FIFO0 interrupts are used by TSC. Handle FIFO1 IRQs here only
+	 */
+	if (status & IRQENB_FIFO1OVRRUN) {
+		/* FIFO Overrun. Clear flag. Disable/Enable ADC to recover */
+		config = tiadc_readl(adc_dev, REG_CTRL);
+		config &= ~(CNTRLREG_TSCSSENB);
+		tiadc_writel(adc_dev, REG_CTRL, config);
+		tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1OVRRUN
+				| IRQENB_FIFO1UNDRFLW | IRQENB_FIFO1THRES);
+		tiadc_writel(adc_dev, REG_CTRL, (config | CNTRLREG_TSCSSENB));
+		return IRQ_HANDLED;
+	} else if (status & IRQENB_FIFO1THRES) {
+		/* Trigger to push FIFO data to iio buffer */
+		tiadc_writel(adc_dev, REG_IRQCLR, IRQENB_FIFO1THRES);
+		iio_trigger_poll(indio_dev->trig, iio_get_time_ns());
+		return IRQ_HANDLED;
+	} else
+		return IRQ_NONE;
+
+}
+
+static irqreturn_t tiadc_trigger_h(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct tiadc_device *adc_dev = iio_priv(indio_dev);
+	int i, k, fifo1count, read;
+	u32 *data = adc_dev->data;
+
+	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
+	for (k = 0; k < fifo1count; k = k + i) {
+		for (i = 0; i < (indio_dev->scan_bytes)/4; i++) {
+			read = tiadc_readl(adc_dev, REG_FIFO1);
+			data[i] = read & FIFOREAD_DATA_MASK;
+		}
+		iio_push_to_buffers(indio_dev, (u8 *) data);
+	}
 
+	tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES);
+	tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES);
+
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
+{
+	struct tiadc_device *adc_dev = iio_priv(indio_dev);
+	int i, fifo1count, read;
+
+	tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
+				IRQENB_FIFO1OVRRUN |
+				IRQENB_FIFO1UNDRFLW));
+
+	/* Flush FIFO before starting sampling */
+	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
+	for (i = 0; i < fifo1count; i++)
+		read = tiadc_readl(adc_dev, REG_FIFO1);
+
+	return iio_sw_buffer_preenable(indio_dev);
+}
+
+static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
+{
+	struct tiadc_device *adc_dev = iio_priv(indio_dev);
+	struct iio_buffer *buffer = indio_dev->buffer;
+	unsigned int enb = 0;
+	u8 bit;
+
+	adc_dev->data = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
+	if (adc_dev->data == NULL)
+		return -ENOMEM;
+
+	tiadc_step_config(indio_dev);
+	for_each_set_bit(bit, buffer->scan_mask, adc_dev->channels)
+		enb |= (get_adc_step_bit(adc_dev, bit) << 1);
+	adc_dev->buffer_en_ch_steps = enb;
+
+	am335x_tsc_se_set(adc_dev->mfd_tscadc, enb);
+
+	tiadc_writel(adc_dev,  REG_IRQSTATUS, IRQENB_FIFO1THRES
+				| IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
+	tiadc_writel(adc_dev,  REG_IRQENABLE, IRQENB_FIFO1THRES
+				| IRQENB_FIFO1OVRRUN);
+
+	return iio_triggered_buffer_postenable(indio_dev);
+}
+
+static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
+{
+	struct tiadc_device *adc_dev = iio_priv(indio_dev);
+	int fifo1count, i, read;
+
+	tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
+				IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
+	am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
+
+	/* Flush FIFO of any leftover data */
+	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
+	for (i = 0; i < fifo1count; i++)
+		read = tiadc_readl(adc_dev, REG_FIFO1);
+
+	return iio_triggered_buffer_predisable(indio_dev);
+}
+
+static int tiadc_buffer_postdisable(struct iio_dev *indio_dev)
+{
+	struct tiadc_device *adc_dev = iio_priv(indio_dev);
+
+	tiadc_step_config(indio_dev);
+	kfree(adc_dev->data);
+	return 0;
+}
+
+static const struct iio_buffer_setup_ops tiadc_buffer_setup_ops = {
+	.preenable = &tiadc_buffer_preenable,
+	.postenable = &tiadc_buffer_postenable,
+	.predisable = &tiadc_buffer_predisable,
+	.postdisable = &tiadc_buffer_postdisable,
+};
+
+static const struct iio_trigger_ops tiadc_trigger_ops = {
+	.owner = THIS_MODULE,
+};
+
+static int tiadc_iio_allocate_trigger(struct iio_dev *indio_dev)
+{
+	struct tiadc_device *adc_dev = iio_priv(indio_dev);
+	struct iio_trigger *trig = adc_dev->trig;
+	int ret;
+
+	trig = iio_trigger_alloc("%s-dev%d", indio_dev->name, indio_dev->id);
+	if (trig == NULL)
+		return -ENOMEM;
+
+	trig->dev.parent = indio_dev->dev.parent;
+	trig->ops = &tiadc_trigger_ops;
+	iio_trigger_set_drvdata(trig, indio_dev);
+
+	ret = iio_trigger_register(trig);
+	if (ret)
+		goto err_free_trigger;
+
+	return 0;
+
+err_free_trigger:
+	iio_trigger_free(adc_dev->trig);
+
+	return ret;
 }
 
 static const char * const chan_name_ain[] = {
@@ -120,6 +295,7 @@ static int tiadc_channel_init(struct iio_dev *indio_dev, int channels)
 		chan->channel = adc_dev->channel_line[i];
 		chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
 		chan->datasheet_name = chan_name_ain[chan->channel];
+		chan->scan_index = i;
 		chan->scan_type.sign = 'u';
 		chan->scan_type.realbits = 12;
 		chan->scan_type.storagebits = 32;
@@ -142,11 +318,14 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
 	struct tiadc_device *adc_dev = iio_priv(indio_dev);
 	int i, map_val;
 	unsigned int fifo1count, read, stepid;
-	u32 step = UINT_MAX;
 	bool found = false;
 	u32 step_en;
 	unsigned long timeout = jiffies + usecs_to_jiffies
 				(IDLE_TIMEOUT * adc_dev->channels);
+
+	if (iio_buffer_enabled(indio_dev))
+		return -EBUSY;
+
 	step_en = get_adc_step_mask(adc_dev);
 	am335x_tsc_se_set(adc_dev->mfd_tscadc, step_en);
 
@@ -168,15 +347,6 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
 	 * Hence we need to flush out this data.
 	 */
 
-	for (i = 0; i < ARRAY_SIZE(adc_dev->channel_step); i++) {
-		if (chan->channel == adc_dev->channel_line[i]) {
-			step = adc_dev->channel_step[i];
-			break;
-		}
-	}
-	if (WARN_ON_ONCE(step == UINT_MAX))
-		return -EINVAL;
-
 	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
 	for (i = 0; i < fifo1count; i++) {
 		read = tiadc_readl(adc_dev, REG_FIFO1);
@@ -231,26 +401,49 @@ static int tiadc_probe(struct platform_device *pdev)
 		channels++;
 	}
 	adc_dev->channels = channels;
+	adc_dev->irq = adc_dev->mfd_tscadc->irq;
 
 	indio_dev->dev.parent = &pdev->dev;
 	indio_dev->name = dev_name(&pdev->dev);
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &tiadc_info;
 
-	tiadc_step_config(adc_dev);
+	tiadc_step_config(indio_dev);
+	tiadc_writel(adc_dev, REG_FIFO1THR, FIFO1_THRESHOLD);
 
 	err = tiadc_channel_init(indio_dev, adc_dev->channels);
 	if (err < 0)
 		return err;
 
-	err = iio_device_register(indio_dev);
+	err = tiadc_iio_allocate_trigger(indio_dev);
 	if (err)
 		goto err_free_channels;
 
+	err = request_irq(adc_dev->irq, tiadc_irq, IRQF_SHARED,
+		indio_dev->name, indio_dev);
+	if (err)
+		goto err_unregister_trigger;
+
+	err = iio_triggered_buffer_setup(indio_dev, NULL,
+			&tiadc_trigger_h, &tiadc_buffer_setup_ops);
+	if (err)
+		goto err_free_irq;
+
+	err = iio_device_register(indio_dev);
+	if (err)
+		goto err_buffer_unregister;
+
 	platform_set_drvdata(pdev, indio_dev);
 
 	return 0;
 
+err_buffer_unregister:
+	iio_buffer_unregister(indio_dev);
+err_free_irq:
+	free_irq(adc_dev->irq, indio_dev);
+err_unregister_trigger:
+	iio_trigger_unregister(adc_dev->trig);
+	iio_trigger_free(adc_dev->trig);
 err_free_channels:
 	tiadc_channels_remove(indio_dev);
 	return err;
@@ -262,7 +455,11 @@ static int tiadc_remove(struct platform_device *pdev)
 	struct tiadc_device *adc_dev = iio_priv(indio_dev);
 	u32 step_en;
 
+	iio_trigger_unregister(adc_dev->trig);
+	iio_trigger_free(adc_dev->trig);
+	free_irq(adc_dev->irq, indio_dev);
 	iio_device_unregister(indio_dev);
+	iio_buffer_unregister(indio_dev);
 	tiadc_channels_remove(indio_dev);
 
 	step_en = get_adc_step_mask(adc_dev);
@@ -298,10 +495,16 @@ static int tiadc_resume(struct device *dev)
 
 	/* Make sure ADC is powered up */
 	restore = tiadc_readl(adc_dev, REG_CTRL);
-	restore &= ~(CNTRLREG_POWERDOWN);
+	restore &= ~(CNTRLREG_TSCSSENB);
 	tiadc_writel(adc_dev, REG_CTRL, restore);
 
-	tiadc_step_config(adc_dev);
+	tiadc_writel(adc_dev, REG_FIFO1THR, FIFO1_THRESHOLD);
+	tiadc_step_config(indio_dev);
+
+	/* Make sure ADC is powered up */
+	restore &= ~(CNTRLREG_POWERDOWN);
+	restore |= CNTRLREG_TSCSSENB;
+	tiadc_writel(adc_dev, REG_CTRL, restore);
 
 	return 0;
 }
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index db1791b..a372ebf 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -46,17 +46,25 @@
 /* Step Enable */
 #define STEPENB_MASK		(0x1FFFF << 0)
 #define STEPENB(val)		((val) << 0)
+#define ENB(val)			(1 << (val))
+#define STPENB_STEPENB		STEPENB(0x1FFFF)
+#define STPENB_STEPENB_TC	STEPENB(0x1FFF)
 
 /* IRQ enable */
 #define IRQENB_HW_PEN		BIT(0)
 #define IRQENB_FIFO0THRES	BIT(2)
+#define IRQENB_FIFO0OVRRUN	BIT(3)
+#define IRQENB_FIFO0UNDRFLW	BIT(4)
 #define IRQENB_FIFO1THRES	BIT(5)
+#define IRQENB_FIFO1OVRRUN	BIT(6)
+#define IRQENB_FIFO1UNDRFLW	BIT(7)
 #define IRQENB_PENUP		BIT(9)
 
 /* Step Configuration */
 #define STEPCONFIG_MODE_MASK	(3 << 0)
 #define STEPCONFIG_MODE(val)	((val) << 0)
 #define STEPCONFIG_MODE_HWSYNC	STEPCONFIG_MODE(2)
+#define STEPCONFIG_MODE_SWCNT	STEPCONFIG_MODE(1)
 #define STEPCONFIG_AVG_MASK	(7 << 2)
 #define STEPCONFIG_AVG(val)	((val) << 2)
 #define STEPCONFIG_AVG_16	STEPCONFIG_AVG(4)
@@ -124,6 +132,7 @@
 #define	MAX_CLK_DIV		7
 #define TOTAL_STEPS		16
 #define TOTAL_CHANNELS		8
+#define FIFO1_THRESHOLD		19
 
 /*
 * ADC runs at 3MHz, and it takes
@@ -153,6 +162,10 @@ struct ti_tscadc_dev {
 
 	/* adc device */
 	struct adc_device *adc;
+
+	/* Context save */
+	unsigned int irqstat;
+	unsigned int ctrl;
 };
 
 static inline struct ti_tscadc_dev *ti_tscadc_dev_get(struct platform_device *p)
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC
From: Zubair Lutfullah @ 2013-09-01 11:17 UTC (permalink / raw)
  To: jic23-KWPb1pKIrIJaa/9Udqfwiw,
	dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	bigeasy-hfZtesqFncYOwBW4kG4KsQ,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1378034277-26728-1-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Enable shared IRQ to allow ADC to share IRQ line from
parent MFD core. Only FIFO0 IRQs are for TSC and handled
on the TSC side.

Step mask would be updated from cached variable only previously.
In rare cases when both TSC and ADC are used, the cached
variable gets mixed up.
The step mask is written with the required mask every time.

Rachna Patil (TI) laid ground work for shared IRQ.

Signed-off-by: Zubair Lutfullah <zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/input/touchscreen/ti_am335x_tsc.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index e1c5300..24e625c 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -52,6 +52,7 @@ struct titsc {
 	u32			config_inp[4];
 	u32			bit_xp, bit_xn, bit_yp, bit_yn;
 	u32			inp_xp, inp_xn, inp_yp, inp_yn;
+	u32			step_mask;
 };
 
 static unsigned int titsc_readl(struct titsc *ts, unsigned int reg)
@@ -196,7 +197,8 @@ static void titsc_step_config(struct titsc *ts_dev)
 
 	/* The steps1 … end and bit 0 for TS_Charge */
 	stepenable = (1 << (end_step + 2)) - 1;
-	am335x_tsc_se_set(ts_dev->mfd_tscadc, stepenable);
+	ts_dev->step_mask = stepenable;
+	am335x_tsc_se_set(ts_dev->mfd_tscadc, ts_dev->step_mask);
 }
 
 static void titsc_read_coordinates(struct titsc *ts_dev,
@@ -260,6 +262,10 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 	unsigned int fsm;
 
 	status = titsc_readl(ts_dev, REG_IRQSTATUS);
+	/*
+	 * ADC and touchscreen share the IRQ line.
+	 * FIFO1 interrupts are used by ADC. Handle FIFO0 IRQs here only
+	 */
 	if (status & IRQENB_FIFO0THRES) {
 
 		titsc_read_coordinates(ts_dev, &x, &y, &z1, &z2);
@@ -316,7 +322,7 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 
 	if (irqclr) {
 		titsc_writel(ts_dev, REG_IRQSTATUS, irqclr);
-		am335x_tsc_se_update(ts_dev->mfd_tscadc);
+		am335x_tsc_se_set(ts_dev->mfd_tscadc, ts_dev->step_mask);
 		return IRQ_HANDLED;
 	}
 	return IRQ_NONE;
@@ -389,7 +395,7 @@ static int titsc_probe(struct platform_device *pdev)
 	}
 
 	err = request_irq(ts_dev->irq, titsc_irq,
-			  0, pdev->dev.driver->name, ts_dev);
+			  IRQF_SHARED, pdev->dev.driver->name, ts_dev);
 	if (err) {
 		dev_err(&pdev->dev, "failed to allocate irq.\n");
 		goto err_free_mem;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH V7 0/2] iio: input: ti_am335x_tscadc: Add continuous sampling support to adc
From: Zubair Lutfullah @ 2013-09-01 11:07 UTC (permalink / raw)
  To: jic23, dmitry.torokhov
  Cc: linux-iio, linux-input, linux-kernel, bigeasy, gregkh,
	zubair.lutfullah

Round 7 updates
- Fixes to error handling path for trigger register

- Optimization of step enable calculations based 
on input from Sebastian

- trigger unregister added to remove section. was missing

- missing entry in Kconfig for triggered buffer support. added

- TSC/ADC IRQ handling was based on wierd ack system. not required. removed.
each irq handler deals with its own stuff *only*

Thanks for all the input 

Regards
Zubair Lutfullah Kakakhel

Zubair Lutfullah (2):
  input: ti_am335x_tsc: Enable shared IRQ for TSC
  iio: ti_am335x_adc: Add continuous sampling support

 drivers/iio/adc/Kconfig                   |    1 +
 drivers/iio/adc/ti_am335x_adc.c           |  243 ++++++++++++++++++++++++++---
 drivers/input/touchscreen/ti_am335x_tsc.c |   12 +-
 include/linux/mfd/ti_am335x_tscadc.h      |   13 ++
 4 files changed, 246 insertions(+), 23 deletions(-)

-- 
1.7.9.5


^ permalink raw reply

* [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support
From: Zubair Lutfullah @ 2013-09-01 11:07 UTC (permalink / raw)
  To: jic23, dmitry.torokhov
  Cc: linux-iio, linux-input, linux-kernel, bigeasy, gregkh,
	zubair.lutfullah
In-Reply-To: <1378033633-26301-1-git-send-email-zubair.lutfullah@gmail.com>

Previously the driver had only one-shot reading functionality.
This patch adds triggered buffer support to the driver.

Continuous sampling starts when buffer is enabled.
And samples are pushed to userpace by the trigger which
triggers automatically at every hardware interrupt
of FIFO1 filling with samples upto threshold value.

Userspace responsibility to stop sampling by writing zero
in the buffer enable file.

Patil Rachna (TI) laid the ground work for ADC HW register access.
Russ Dill (TI) fixed bugs in the driver relevant to FIFOs and IRQs.

I fixed channel scanning so multiple ADC channels can be read
simultaneously and pushed to userspace.
Restructured the driver to fit IIO ABI.
And added trigger support.

Signed-off-by: Zubair Lutfullah <zubair.lutfullah@gmail.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Russ Dill <Russ.Dill@ti.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
Acked-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/iio/adc/Kconfig              |    1 +
 drivers/iio/adc/ti_am335x_adc.c      |  243 +++++++++++++++++++++++++++++++---
 include/linux/mfd/ti_am335x_tscadc.h |   13 ++
 3 files changed, 237 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 09371cb..cfa2039 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -167,6 +167,7 @@ config TI_ADC081C
 config TI_AM335X_ADC
 	tristate "TI's AM335X ADC driver"
 	depends on MFD_TI_AM335X_TSCADC
+	select IIO_TRIGGERED_BUFFER
 	help
 	  Say yes here to build support for Texas Instruments ADC
 	  driver which is also a MFD client.
diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index a952538..1626e16 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -24,16 +24,23 @@
 #include <linux/iio/iio.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
-#include <linux/iio/machine.h>
 #include <linux/iio/driver.h>
 
 #include <linux/mfd/ti_am335x_tscadc.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
 
 struct tiadc_device {
 	struct ti_tscadc_dev *mfd_tscadc;
 	int channels;
 	u8 channel_line[8];
 	u8 channel_step[8];
+	int irq;
+	int buffer_en_ch_steps;
+	struct iio_trigger *trig;
+	u32 *data;
 };
 
 static unsigned int tiadc_readl(struct tiadc_device *adc, unsigned int reg)
@@ -56,10 +63,16 @@ static u32 get_adc_step_mask(struct tiadc_device *adc_dev)
 	return step_en;
 }
 
-static void tiadc_step_config(struct tiadc_device *adc_dev)
+static u32 get_adc_step_bit(struct tiadc_device *adc_dev, int chan)
 {
+	return 1 << adc_dev->channel_step[chan];
+}
+
+static void tiadc_step_config(struct iio_dev *indio_dev)
+{
+	struct tiadc_device *adc_dev = iio_priv(indio_dev);
 	unsigned int stepconfig;
-	int i, steps;
+	int i, steps, chan;
 
 	/*
 	 * There are 16 configurable steps and 8 analog input
@@ -72,11 +85,13 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
 	 */
 
 	steps = TOTAL_STEPS - adc_dev->channels;
-	stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1;
+	if (iio_buffer_enabled(indio_dev))
+		stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1
+					| STEPCONFIG_MODE_SWCNT;
+	else
+		stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1;
 
 	for (i = 0; i < adc_dev->channels; i++) {
-		int chan;
-
 		chan = adc_dev->channel_line[i];
 		tiadc_writel(adc_dev, REG_STEPCONFIG(steps),
 				stepconfig | STEPCONFIG_INP(chan));
@@ -85,7 +100,167 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
 		adc_dev->channel_step[i] = steps;
 		steps++;
 	}
+}
+
+static irqreturn_t tiadc_irq(int irq, void *private)
+{
+	struct iio_dev *indio_dev = private;
+	struct tiadc_device *adc_dev = iio_priv(indio_dev);
+	unsigned int status, config;
+	status = tiadc_readl(adc_dev, REG_IRQSTATUS);
+
+	/*
+	 * ADC and touchscreen share the IRQ line.
+	 * FIFO0 interrupts are used by TSC. Handle FIFO1 IRQs here only
+	 */
+	if (status & IRQENB_FIFO1OVRRUN) {
+		/* FIFO Overrun. Clear flag. Disable/Enable ADC to recover */
+		config = tiadc_readl(adc_dev, REG_CTRL);
+		config &= ~(CNTRLREG_TSCSSENB);
+		tiadc_writel(adc_dev, REG_CTRL, config);
+		tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1OVRRUN
+				| IRQENB_FIFO1UNDRFLW | IRQENB_FIFO1THRES);
+		tiadc_writel(adc_dev, REG_CTRL, (config | CNTRLREG_TSCSSENB));
+		return IRQ_HANDLED;
+	} else if (status & IRQENB_FIFO1THRES) {
+		/* Trigger to push FIFO data to iio buffer */
+		tiadc_writel(adc_dev, REG_IRQCLR, IRQENB_FIFO1THRES);
+		iio_trigger_poll(indio_dev->trig, iio_get_time_ns());
+		return IRQ_HANDLED;
+	} else
+		return IRQ_NONE;
+
+}
+
+static irqreturn_t tiadc_trigger_h(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct tiadc_device *adc_dev = iio_priv(indio_dev);
+	int i, k, fifo1count, read;
+	u32 *data = adc_dev->data;
+
+	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
+	for (k = 0; k < fifo1count; k = k + i) {
+		for (i = 0; i < (indio_dev->scan_bytes)/4; i++) {
+			read = tiadc_readl(adc_dev, REG_FIFO1);
+			data[i] = read & FIFOREAD_DATA_MASK;
+		}
+		iio_push_to_buffers(indio_dev, (u8 *) data);
+	}
 
+	tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES);
+	tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES);
+
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
+{
+	struct tiadc_device *adc_dev = iio_priv(indio_dev);
+	int i, fifo1count, read;
+
+	tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
+				IRQENB_FIFO1OVRRUN |
+				IRQENB_FIFO1UNDRFLW));
+
+	/* Flush FIFO before starting sampling */
+	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
+	for (i = 0; i < fifo1count; i++)
+		read = tiadc_readl(adc_dev, REG_FIFO1);
+
+	return iio_sw_buffer_preenable(indio_dev);
+}
+
+static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
+{
+	struct tiadc_device *adc_dev = iio_priv(indio_dev);
+	struct iio_buffer *buffer = indio_dev->buffer;
+	unsigned int enb = 0;
+	u8 bit;
+
+	adc_dev->data = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
+	if (adc_dev->data == NULL)
+		return -ENOMEM;
+
+	tiadc_step_config(indio_dev);
+	for_each_set_bit(bit, buffer->scan_mask, adc_dev->channels)
+		enb |= (get_adc_step_bit(adc_dev, bit) << 1);
+	adc_dev->buffer_en_ch_steps = enb;
+
+	am335x_tsc_se_set(adc_dev->mfd_tscadc, enb);
+
+	tiadc_writel(adc_dev,  REG_IRQSTATUS, IRQENB_FIFO1THRES
+				| IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
+	tiadc_writel(adc_dev,  REG_IRQENABLE, IRQENB_FIFO1THRES
+				| IRQENB_FIFO1OVRRUN);
+
+	return iio_triggered_buffer_postenable(indio_dev);
+}
+
+static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
+{
+	struct tiadc_device *adc_dev = iio_priv(indio_dev);
+	int fifo1count, i, read;
+
+	tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
+				IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
+	am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
+
+	/* Flush FIFO of any leftover data */
+	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
+	for (i = 0; i < fifo1count; i++)
+		read = tiadc_readl(adc_dev, REG_FIFO1);
+
+	return iio_triggered_buffer_predisable(indio_dev);
+}
+
+static int tiadc_buffer_postdisable(struct iio_dev *indio_dev)
+{
+	struct tiadc_device *adc_dev = iio_priv(indio_dev);
+
+	tiadc_step_config(indio_dev);
+	kfree(adc_dev->data);
+	return 0;
+}
+
+static const struct iio_buffer_setup_ops tiadc_buffer_setup_ops = {
+	.preenable = &tiadc_buffer_preenable,
+	.postenable = &tiadc_buffer_postenable,
+	.predisable = &tiadc_buffer_predisable,
+	.postdisable = &tiadc_buffer_postdisable,
+};
+
+static const struct iio_trigger_ops tiadc_trigger_ops = {
+	.owner = THIS_MODULE,
+};
+
+static int tiadc_iio_allocate_trigger(struct iio_dev *indio_dev)
+{
+	struct tiadc_device *adc_dev = iio_priv(indio_dev);
+	struct iio_trigger *trig = adc_dev->trig;
+	int ret;
+
+	trig = iio_trigger_alloc("%s-dev%d", indio_dev->name, indio_dev->id);
+	if (trig == NULL)
+		return -ENOMEM;
+
+	trig->dev.parent = indio_dev->dev.parent;
+	trig->ops = &tiadc_trigger_ops;
+	iio_trigger_set_drvdata(trig, indio_dev);
+
+	ret = iio_trigger_register(trig);
+	if (ret)
+		goto err_free_trigger;
+
+	return 0;
+
+err_free_trigger:
+	iio_trigger_free(adc_dev->trig);
+
+	return ret;
 }
 
 static const char * const chan_name_ain[] = {
@@ -120,6 +295,7 @@ static int tiadc_channel_init(struct iio_dev *indio_dev, int channels)
 		chan->channel = adc_dev->channel_line[i];
 		chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
 		chan->datasheet_name = chan_name_ain[chan->channel];
+		chan->scan_index = i;
 		chan->scan_type.sign = 'u';
 		chan->scan_type.realbits = 12;
 		chan->scan_type.storagebits = 32;
@@ -142,11 +318,14 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
 	struct tiadc_device *adc_dev = iio_priv(indio_dev);
 	int i, map_val;
 	unsigned int fifo1count, read, stepid;
-	u32 step = UINT_MAX;
 	bool found = false;
 	u32 step_en;
 	unsigned long timeout = jiffies + usecs_to_jiffies
 				(IDLE_TIMEOUT * adc_dev->channels);
+
+	if (iio_buffer_enabled(indio_dev))
+		return -EBUSY;
+
 	step_en = get_adc_step_mask(adc_dev);
 	am335x_tsc_se_set(adc_dev->mfd_tscadc, step_en);
 
@@ -168,15 +347,6 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
 	 * Hence we need to flush out this data.
 	 */
 
-	for (i = 0; i < ARRAY_SIZE(adc_dev->channel_step); i++) {
-		if (chan->channel == adc_dev->channel_line[i]) {
-			step = adc_dev->channel_step[i];
-			break;
-		}
-	}
-	if (WARN_ON_ONCE(step == UINT_MAX))
-		return -EINVAL;
-
 	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
 	for (i = 0; i < fifo1count; i++) {
 		read = tiadc_readl(adc_dev, REG_FIFO1);
@@ -231,26 +401,49 @@ static int tiadc_probe(struct platform_device *pdev)
 		channels++;
 	}
 	adc_dev->channels = channels;
+	adc_dev->irq = adc_dev->mfd_tscadc->irq;
 
 	indio_dev->dev.parent = &pdev->dev;
 	indio_dev->name = dev_name(&pdev->dev);
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &tiadc_info;
 
-	tiadc_step_config(adc_dev);
+	tiadc_step_config(indio_dev);
+	tiadc_writel(adc_dev, REG_FIFO1THR, FIFO1_THRESHOLD);
 
 	err = tiadc_channel_init(indio_dev, adc_dev->channels);
 	if (err < 0)
 		return err;
 
-	err = iio_device_register(indio_dev);
+	err = tiadc_iio_allocate_trigger(indio_dev);
 	if (err)
 		goto err_free_channels;
 
+	err = request_irq(adc_dev->irq, tiadc_irq, IRQF_SHARED,
+		indio_dev->name, indio_dev);
+	if (err)
+		goto err_unregister_trigger;
+
+	err = iio_triggered_buffer_setup(indio_dev, NULL,
+			&tiadc_trigger_h, &tiadc_buffer_setup_ops);
+	if (err)
+		goto err_free_irq;
+
+	err = iio_device_register(indio_dev);
+	if (err)
+		goto err_buffer_unregister;
+
 	platform_set_drvdata(pdev, indio_dev);
 
 	return 0;
 
+err_buffer_unregister:
+	iio_buffer_unregister(indio_dev);
+err_free_irq:
+	free_irq(adc_dev->irq, indio_dev);
+err_unregister_trigger:
+	iio_trigger_unregister(adc_dev->trig);
+	iio_trigger_free(adc_dev->trig);
 err_free_channels:
 	tiadc_channels_remove(indio_dev);
 	return err;
@@ -262,7 +455,11 @@ static int tiadc_remove(struct platform_device *pdev)
 	struct tiadc_device *adc_dev = iio_priv(indio_dev);
 	u32 step_en;
 
+	iio_trigger_unregister(adc_dev->trig);
+	iio_trigger_free(adc_dev->trig);
+	free_irq(adc_dev->irq, indio_dev);
 	iio_device_unregister(indio_dev);
+	iio_buffer_unregister(indio_dev);
 	tiadc_channels_remove(indio_dev);
 
 	step_en = get_adc_step_mask(adc_dev);
@@ -298,10 +495,16 @@ static int tiadc_resume(struct device *dev)
 
 	/* Make sure ADC is powered up */
 	restore = tiadc_readl(adc_dev, REG_CTRL);
-	restore &= ~(CNTRLREG_POWERDOWN);
+	restore &= ~(CNTRLREG_TSCSSENB);
 	tiadc_writel(adc_dev, REG_CTRL, restore);
 
-	tiadc_step_config(adc_dev);
+	tiadc_writel(adc_dev, REG_FIFO1THR, FIFO1_THRESHOLD);
+	tiadc_step_config(indio_dev);
+
+	/* Make sure ADC is powered up */
+	restore &= ~(CNTRLREG_POWERDOWN);
+	restore |= CNTRLREG_TSCSSENB;
+	tiadc_writel(adc_dev, REG_CTRL, restore);
 
 	return 0;
 }
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index db1791b..a372ebf 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -46,17 +46,25 @@
 /* Step Enable */
 #define STEPENB_MASK		(0x1FFFF << 0)
 #define STEPENB(val)		((val) << 0)
+#define ENB(val)			(1 << (val))
+#define STPENB_STEPENB		STEPENB(0x1FFFF)
+#define STPENB_STEPENB_TC	STEPENB(0x1FFF)
 
 /* IRQ enable */
 #define IRQENB_HW_PEN		BIT(0)
 #define IRQENB_FIFO0THRES	BIT(2)
+#define IRQENB_FIFO0OVRRUN	BIT(3)
+#define IRQENB_FIFO0UNDRFLW	BIT(4)
 #define IRQENB_FIFO1THRES	BIT(5)
+#define IRQENB_FIFO1OVRRUN	BIT(6)
+#define IRQENB_FIFO1UNDRFLW	BIT(7)
 #define IRQENB_PENUP		BIT(9)
 
 /* Step Configuration */
 #define STEPCONFIG_MODE_MASK	(3 << 0)
 #define STEPCONFIG_MODE(val)	((val) << 0)
 #define STEPCONFIG_MODE_HWSYNC	STEPCONFIG_MODE(2)
+#define STEPCONFIG_MODE_SWCNT	STEPCONFIG_MODE(1)
 #define STEPCONFIG_AVG_MASK	(7 << 2)
 #define STEPCONFIG_AVG(val)	((val) << 2)
 #define STEPCONFIG_AVG_16	STEPCONFIG_AVG(4)
@@ -124,6 +132,7 @@
 #define	MAX_CLK_DIV		7
 #define TOTAL_STEPS		16
 #define TOTAL_CHANNELS		8
+#define FIFO1_THRESHOLD		19
 
 /*
 * ADC runs at 3MHz, and it takes
@@ -153,6 +162,10 @@ struct ti_tscadc_dev {
 
 	/* adc device */
 	struct adc_device *adc;
+
+	/* Context save */
+	unsigned int irqstat;
+	unsigned int ctrl;
 };
 
 static inline struct ti_tscadc_dev *ti_tscadc_dev_get(struct platform_device *p)
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC
From: Zubair Lutfullah @ 2013-09-01 11:07 UTC (permalink / raw)
  To: jic23, dmitry.torokhov
  Cc: linux-iio, linux-input, linux-kernel, bigeasy, gregkh,
	zubair.lutfullah
In-Reply-To: <1378033633-26301-1-git-send-email-zubair.lutfullah@gmail.com>

Enable shared IRQ to allow ADC to share IRQ line from
parent MFD core. Only FIFO0 IRQs are for TSC and handled
on the TSC side.

Step mask would be updated from cached variable only previously.
In rare cases when both TSC and ADC are used, the cached
variable gets mixed up.
The step mask is written with the required mask every time.

Rachna Patil (TI) laid ground work for shared IRQ.

Signed-off-by: Zubair Lutfullah <zubair.lutfullah@gmail.com>
---
 drivers/input/touchscreen/ti_am335x_tsc.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index e1c5300..24e625c 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -52,6 +52,7 @@ struct titsc {
 	u32			config_inp[4];
 	u32			bit_xp, bit_xn, bit_yp, bit_yn;
 	u32			inp_xp, inp_xn, inp_yp, inp_yn;
+	u32			step_mask;
 };
 
 static unsigned int titsc_readl(struct titsc *ts, unsigned int reg)
@@ -196,7 +197,8 @@ static void titsc_step_config(struct titsc *ts_dev)
 
 	/* The steps1 … end and bit 0 for TS_Charge */
 	stepenable = (1 << (end_step + 2)) - 1;
-	am335x_tsc_se_set(ts_dev->mfd_tscadc, stepenable);
+	ts_dev->step_mask = stepenable;
+	am335x_tsc_se_set(ts_dev->mfd_tscadc, ts_dev->step_mask);
 }
 
 static void titsc_read_coordinates(struct titsc *ts_dev,
@@ -260,6 +262,10 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 	unsigned int fsm;
 
 	status = titsc_readl(ts_dev, REG_IRQSTATUS);
+	/*
+	 * ADC and touchscreen share the IRQ line.
+	 * FIFO1 interrupts are used by ADC. Handle FIFO0 IRQs here only
+	 */
 	if (status & IRQENB_FIFO0THRES) {
 
 		titsc_read_coordinates(ts_dev, &x, &y, &z1, &z2);
@@ -316,7 +322,7 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 
 	if (irqclr) {
 		titsc_writel(ts_dev, REG_IRQSTATUS, irqclr);
-		am335x_tsc_se_update(ts_dev->mfd_tscadc);
+		am335x_tsc_se_set(ts_dev->mfd_tscadc, ts_dev->step_mask);
 		return IRQ_HANDLED;
 	}
 	return IRQ_NONE;
@@ -389,7 +395,7 @@ static int titsc_probe(struct platform_device *pdev)
 	}
 
 	err = request_irq(ts_dev->irq, titsc_irq,
-			  0, pdev->dev.driver->name, ts_dev);
+			  IRQF_SHARED, pdev->dev.driver->name, ts_dev);
 	if (err) {
 		dev_err(&pdev->dev, "failed to allocate irq.\n");
 		goto err_free_mem;
-- 
1.7.9.5

^ permalink raw reply related

* HID: picolcd: Prevent NULL pointer dereference on _remove()
From: Bruno Prémont @ 2013-08-31 12:07 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-input

When picolcd is switched into bootloader mode (for FW flashing) make
sure not to try to dereference NULL-pointers of feature-devices during
unplug/unbind.

This fixes following BUG:
  BUG: unable to handle kernel NULL pointer dereference at 00000298
  IP: [<f811f56b>] picolcd_exit_framebuffer+0x1b/0x80 [hid_picolcd]
  *pde = 00000000
  Oops: 0000 [#1]
  Modules linked in: hid_picolcd syscopyarea sysfillrect sysimgblt fb_sys_fops
  CPU: 0 PID: 15 Comm: khubd Not tainted 3.11.0-rc7-00002-g50d62d4 #2
  EIP: 0060:[<f811f56b>] EFLAGS: 00010292 CPU: 0
  EIP is at picolcd_exit_framebuffer+0x1b/0x80 [hid_picolcd]
  Call Trace:
   [<f811d1ab>] picolcd_remove+0xcb/0x120 [hid_picolcd]
   [<c1469b09>] hid_device_remove+0x59/0xc0
   [<c13464ca>] __device_release_driver+0x5a/0xb0
   [<c134653f>] device_release_driver+0x1f/0x30
   [<c134603d>] bus_remove_device+0x9d/0xd0
   [<c13439a5>] device_del+0xd5/0x150
   [<c14696a4>] hid_destroy_device+0x24/0x60
   [<c1474cbb>] usbhid_disconnect+0x1b/0x40
   ...

Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
Cc: stable@kernel.org
---
 drivers/hid/hid-picolcd_cir.c | 3 ++-
 drivers/hid/hid-picolcd_fb.c  | 6 +++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-picolcd_cir.c b/drivers/hid/hid-picolcd_cir.c
index e346038..59d5eb1 100644
--- a/drivers/hid/hid-picolcd_cir.c
+++ b/drivers/hid/hid-picolcd_cir.c
@@ -145,6 +145,7 @@ void picolcd_exit_cir(struct picolcd_data *data)
 	struct rc_dev *rdev = data->rc_dev;
 
 	data->rc_dev = NULL;
-	rc_unregister_device(rdev);
+	if (rdev)
+		rc_unregister_device(rdev);
 }
 
diff --git a/drivers/hid/hid-picolcd_fb.c b/drivers/hid/hid-picolcd_fb.c
index 591f6b2..c930ab8 100644
--- a/drivers/hid/hid-picolcd_fb.c
+++ b/drivers/hid/hid-picolcd_fb.c
@@ -593,10 +593,14 @@ err_nomem:
 void picolcd_exit_framebuffer(struct picolcd_data *data)
 {
 	struct fb_info *info = data->fb_info;
-	struct picolcd_fb_data *fbdata = info->par;
+	struct picolcd_fb_data *fbdata;
 	unsigned long flags;
 
+	if (!info)
+		return;
+
 	device_remove_file(&data->hdev->dev, &dev_attr_fb_update_rate);
+	fbdata = info->par;
 
 	/* disconnect framebuffer from HID dev */
 	spin_lock_irqsave(&fbdata->lock, flags);
-- 
1.8.1.5

--
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

* Re: [PATCH 13/14] HID: picolcd_core: validate output report details
From: Bruno Prémont @ 2013-08-31 12:04 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-input, Kees Cook
In-Reply-To: <alpine.LNX.2.00.1308282222460.22181@pobox.suse.cz>

Hi Kees, Jiri,

On Wed, 28 August 2013 Jiri Kosina <jkosina@suse.cz> wrote:
> From: Kees Cook <keescook@chromium.org>
> 
> A HID device could send a malicious output report that would cause the
> picolcd HID driver to trigger a NULL dereference during attr file writing.
> 
> CVE-2013-2899
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Cc: stable@kernel.org
> ---
>  drivers/hid/hid-picolcd_core.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-picolcd_core.c
> index b48092d..72bba1e 100644
> --- a/drivers/hid/hid-picolcd_core.c
> +++ b/drivers/hid/hid-picolcd_core.c
> @@ -290,7 +290,7 @@ static ssize_t picolcd_operation_mode_store(struct device *dev,
>  		buf += 10;
>  		cnt -= 10;
>  	}
> -	if (!report)
> +	if (!report || report->maxfield < 1)

Please do
  +     if (!report || report->maxfield != 1)

That way we are consistent with whole picolcd driver and a device
deciding to change its HID-ABI will be properly detected.

With that adjustment, Acked-by/Reviewed-by me

Thanks,
Bruno

>  		return -EINVAL;
>  
>  	while (cnt > 0 && (buf[cnt-1] == '\n' || buf[cnt-1] == '\r'))

^ permalink raw reply

* [PATCH 0/3] driver: input: fix missing of_node_put
From: Libo Chen @ 2013-08-31  6:44 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: peter.ujfalusi, Bill Pemberton, broonie, linux-input, LKML,
	Li Zefan, zhangwei(Jovi)


decrease np device_node refcount after task completion

Libo Chen (3):
  driver: input: 88pm860x-ts: fix missing of_node_put
  driver: input: twl4030-vibra: fix missing of_node_put
  driver: input: twl6040-vibra: fix missing of_node_put

 drivers/input/misc/twl4030-vibra.c      |    4 +++-
 drivers/input/misc/twl6040-vibra.c      |    3 +++
 drivers/input/touchscreen/88pm860x-ts.c |   22 +++++++++++++++-------
 3 files changed, 21 insertions(+), 8 deletions(-)


^ permalink raw reply

* [PATCH 3/3] driver: input: twl6040-vibra: fix missing of_node_put
From: Libo Chen @ 2013-08-31  6:45 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: Bill Pemberton, peter.ujfalusi, broonie, javier, linux-input,
	LKML, Li Zefan, zhangwei(Jovi)


decrease twl6040_core_node device_node refcount after task completion

There are two ways to implement the function of_node_put through
the marco CONFIG_OF_DYNAMIC, so it is save to call directly.

Signed-off-by: Libo Chen <libo.chen@huawei.com>
---
 drivers/input/misc/twl6040-vibra.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/input/misc/twl6040-vibra.c b/drivers/input/misc/twl6040-vibra.c
index 0c2dfc8..f16193a 100644
--- a/drivers/input/misc/twl6040-vibra.c
+++ b/drivers/input/misc/twl6040-vibra.c
@@ -271,12 +271,14 @@ static int twl6040_vibra_probe(struct platform_device *pdev)
 #endif

 	if (!pdata && !twl6040_core_node) {
+		of_node_put(twl6040_core_node);
 		dev_err(&pdev->dev, "platform_data not available\n");
 		return -EINVAL;
 	}

 	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
 	if (!info) {
+		of_node_put(twl6040_core_node);
 		dev_err(&pdev->dev, "couldn't allocate memory\n");
 		return -ENOMEM;
 	}
@@ -304,6 +306,7 @@ static int twl6040_vibra_probe(struct platform_device *pdev)
 				     &vddvibl_uV);
 		of_property_read_u32(twl6040_core_node, "ti,vddvibr-uV",
 				     &vddvibr_uV);
+		of_node_put(twl6040_core_node);
 	}

 	if ((!info->vibldrv_res && !info->viblmotor_res) ||
-- 
1.7.1

^ permalink raw reply related

* [PATCH 2/3] driver: input: twl4030-vibra: fix missing of_node_put
From: Libo Chen @ 2013-08-31  6:45 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: peter.ujfalusi, Bill Pemberton, broonie, linux-input, LKML


decrease node device_node refcount after task completion

Signed-off-by: Libo Chen <libo.chen@huawei.com>
---
 drivers/input/misc/twl4030-vibra.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/input/misc/twl4030-vibra.c b/drivers/input/misc/twl4030-vibra.c
index 68a5f33..b8d1526 100644
--- a/drivers/input/misc/twl4030-vibra.c
+++ b/drivers/input/misc/twl4030-vibra.c
@@ -185,8 +185,10 @@ static bool twl4030_vibra_check_coexist(struct twl4030_vibra_data *pdata,
 	if (pdata && pdata->coexist)
 		return true;

-	if (of_find_node_by_name(node, "codec"))
+	if (of_find_node_by_name(node, "codec")) {
+		of_node_put(node);
 		return true;
+	}

 	return false;
 }
-- 
1.7.1

^ permalink raw reply related

* [PATCH 1/3] driver: input: 88pm860x-ts: fix missing of_node_put
From: Libo Chen @ 2013-08-31  6:44 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: peter.ujfalusi, Bill Pemberton, broonie, linux-input, LKML,
	Li Zefan, zhangwei(Jovi)


decrease np device_node refcount after task completion

Signed-off-by: Libo Chen <libo.chen@huawei.com>
---
 drivers/input/touchscreen/88pm860x-ts.c |   22 +++++++++++++++-------
 1 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/input/touchscreen/88pm860x-ts.c b/drivers/input/touchscreen/88pm860x-ts.c
index f7de14a..4a2fd46 100644
--- a/drivers/input/touchscreen/88pm860x-ts.c
+++ b/drivers/input/touchscreen/88pm860x-ts.c
@@ -142,14 +142,18 @@ static int pm860x_touch_dt_init(struct platform_device *pdev,
 		data |= (n << 7) & PM8607_GPADC_SW_CAL_MASK;
 	if (data) {
 		ret = pm860x_reg_write(i2c, PM8607_GPADC_MISC1, data);
-		if (ret < 0)
-			return -EINVAL;
+		if (ret < 0) {
+			ret = -EINVAL;
+			goto err;
+		}
 	}
 	/* set tsi prebias time */
 	if (!of_property_read_u32(np, "marvell,88pm860x-tsi-prebias", &data)) {
 		ret = pm860x_reg_write(i2c, PM8607_TSI_PREBIAS, data);
-		if (ret < 0)
-			return -EINVAL;
+		if (ret < 0) {
+			ret = -EINVAL;
+			goto err;
+		}
 	}
 	/* set prebias & prechg time of pen detect */
 	data = 0;
@@ -159,11 +163,15 @@ static int pm860x_touch_dt_init(struct platform_device *pdev,
 		data |= n & PM8607_PD_PRECHG_MASK;
 	if (data) {
 		ret = pm860x_reg_write(i2c, PM8607_PD_PREBIAS, data);
-		if (ret < 0)
-			return -EINVAL;
+		if (ret < 0) {
+			ret = -EINVAL;
+			goto err;
+		}
 	}
 	of_property_read_u32(np, "marvell,88pm860x-resistor-X", res_x);
-	return 0;
+err:
+	of_node_put(np);
+	return ret;
 }
 #else
 #define pm860x_touch_dt_init(x, y, z)	(-1)
-- 
1.7.1

^ permalink raw reply related

* Re: hid: egalax 0x0eef:0x0001 no longer detected by hid-multitouch
From: Sebastian Dalfuß @ 2013-08-30 23:23 UTC (permalink / raw)
  To: Forest Bond; +Cc: linux-input
In-Reply-To: <20130830184502.GH13685@alittletooquiet.net>

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

Hi Forest,

On Fri, Aug 30, 2013 at 02:45:02PM -0400, Forest Bond wrote:
> For now look at hid_ignore in hid-core.c.  You can probably set
> HID_QUIRK_NO_IGNORE to work around the problem.  You'll probably also want to
> blacklist usbtouchscreen.

That helped indeed.

Changing

        case USB_VENDOR_ID_DWAV:
                /* These are handled by usbtouchscreen. hdev->type is probably
                 * HID_TYPE_USBNONE, but we say !HID_TYPE_USBMOUSE to match
                 * usbtouchscreen. */
                if ((hdev->product == USB_DEVICE_ID_EGALAX_TOUCHCONTROLLER ||
                     hdev->product == USB_DEVICE_ID_DWAV_TOUCHCONTROLLER) &&
                    hdev->type != HID_TYPE_USBMOUSE)
                        return true;
                break;

to

        case USB_VENDOR_ID_DWAV:
                /* These are handled by usbtouchscreen. hdev->type is probably
                 * HID_TYPE_USBNONE, but we say !HID_TYPE_USBMOUSE to match
                 * usbtouchscreen. */
                if (hdev->product == USB_DEVICE_ID_DWAV_TOUCHCONTROLLER &&
                    hdev->type != HID_TYPE_USBMOUSE)
                        return true;
                break;

did the trick.
I also commented the dev id in usbtouchscreen.c out.

//      {USB_DEVICE(0x0eef, 0x0001), .driver_info = DEVTYPE_EGALAX},

Thanks,
Sebastian Dalfuß

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

^ permalink raw reply

* [PATCH v2] HID: usbhid: quirk for N-Trig DuoSense Touch Screen
From: Vasily Titskiy @ 2013-08-30 22:25 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-input, Jiri Kosina

The DuoSense touchscreen device causes a 10 second timeout. This fix
removes the delay.

Signed-off-by: Vasily Titskiy <qehgt0@gmail.com>
---
 drivers/hid/hid-ids.h           |    1 +
 drivers/hid/usbhid/hid-quirks.c |    1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 2168885..376170e 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -646,6 +646,7 @@
 #define USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_16   0x0012
 #define USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_17   0x0013
 #define USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_18   0x0014
+#define USB_DEVICE_ID_NTRIG_DUOSENSE 0x1500

 #define USB_VENDOR_ID_ONTRAK 0x0a07
 #define USB_DEVICE_ID_ONTRAK_ADU100 0x0064
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 19b8360..1671b47 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -109,6 +109,7 @@ static const struct hid_blacklist {
  { USB_VENDOR_ID_SIGMA_MICRO, USB_DEVICE_ID_SIGMA_MICRO_KEYBOARD,
HID_QUIRK_NO_INIT_REPORTS },
  { USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_MOUSEPEN_I608X,
HID_QUIRK_MULTI_INPUT },
  { USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M610X, HID_QUIRK_MULTI_INPUT },
+ { USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_DUOSENSE,
HID_QUIRK_NO_INIT_REPORTS },
  { 0, 0 }
 };

-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH] HID: usbhid: quirk for N-Trig DuoSense Touch Screen
From: Benjamin Tissoires @ 2013-08-30 21:31 UTC (permalink / raw)
  To: Vasily Titskiy; +Cc: linux-kernel@vger.kernel.org, linux-input, Jiri Kosina
In-Reply-To: <CABSveYhSMyCxPZS3nqSdiG-UZwAMMTX7UjgFG=emY+L-K60sdQ@mail.gmail.com>

On Fri, Aug 30, 2013 at 9:34 PM, Vasily Titskiy <qehgt0@gmail.com> wrote:
> Hi,
>
> I've re-tested it with HID_QUIRK_NO_INIT_REPORTS parameter. Looks
> fine, still no delays.
>

Yes please. Send a v2 so that Jiri can take it through his tree.

Cheers,
Benjamin

>
>
> On Fri, Aug 30, 2013 at 3:18 PM, Benjamin Tissoires
> <benjamin.tissoires@gmail.com> wrote:
>> Hi,
>>
>> first, for the next submission, do not forget to add Jiri (the HID
>> maintainer) in CC, otherwise there are huge chances that he will miss
>> your patch.
>>
>> On Fri, Aug 30, 2013 at 8:47 PM, Vasily Titskiy <qehgt0@gmail.com> wrote:
>>> The DuoSense touchscreen device causes a 10 second timeout. This fix
>>> removes the delay.
>>>
>>> Signed-off-by: Vasily Titskiy <qehgt0@gmail.com>
>>> ---
>>>  drivers/hid/hid-ids.h           |    2 ++
>>>  drivers/hid/usbhid/hid-quirks.c |    3 +++
>>>  2 files changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
>>> index 2168885..c3929ea 100644
>>> --- a/drivers/hid/hid-ids.h
>>> +++ b/drivers/hid/hid-ids.h
>>> @@ -898,4 +898,6 @@
>>>  #define USB_VENDOR_ID_PRIMAX 0x0461
>>>  #define USB_DEVICE_ID_PRIMAX_KEYBOARD 0x4e05
>>>
>>> +#define USB_DEVICE_ID_NTRIG_DUOSENSE 0x1500
>>> +
>>
>> Please keep the list sorted. This define should go into the *_NTRIG_* block.
>>
>>>  #endif
>>> diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
>>> index 19b8360..3fd4dfd 100644
>>> --- a/drivers/hid/usbhid/hid-quirks.c
>>> +++ b/drivers/hid/usbhid/hid-quirks.c
>>> @@ -89,6 +89,9 @@ static const struct hid_blacklist {
>>>   { USB_VENDOR_ID_SYMBOL, USB_DEVICE_ID_SYMBOL_SCANNER_2, HID_QUIRK_NOGET },
>>>   { USB_VENDOR_ID_TPV, USB_DEVICE_ID_TPV_OPTICAL_TOUCHSCREEN, HID_QUIRK_NOGET },
>>>   { USB_VENDOR_ID_TURBOX, USB_DEVICE_ID_TURBOX_KEYBOARD, HID_QUIRK_NOGET },
>>> +
>>
>> extra whitespace
>>
>>> + { USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_DUOSENSE, HID_QUIRK_NOGET },
>>
>> I'm sure we could try here the new quirk
>> HID_QUIRK_NO_INIT_INPUT_REPORTS, this would be more accurate I think.
>> At least can you test if HID_QUIRK_NO_INIT_REPORTS is enough?
>> HID_QUIRK_NOGET prevent all request from the host to the device, which
>> is a little bit too much in many cases.
>>
>>> +
>>
>> extra whitespace too.
>>
>> Cheers,
>> Benjamin
>>
>>
>>>   { USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_PF1209,
>>> HID_QUIRK_MULTI_INPUT },
>>>   { USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_WP4030U,
>>> HID_QUIRK_MULTI_INPUT },
>>>   { USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_KNA5,
>>> HID_QUIRK_MULTI_INPUT },
>>> --
>>> 1.7.10.4
>>> --
>>> 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 v2 2/2] input: misc: New USB eBeam input driver
From: Yann Cantin @ 2013-08-30 20:53 UTC (permalink / raw)
  To: linux-input, linux-usb
  Cc: linux-kernel, linux-doc, dmitry.torokhov, jkosina, rob, gregkh,
	Yann Cantin
In-Reply-To: <1377895987-27715-1-git-send-email-yann.cantin@laposte.net>


Signed-off-by: Yann Cantin <yann.cantin@laposte.net>
---
 Documentation/ABI/testing/sysfs-driver-ebeam |  53 ++
 drivers/input/misc/Kconfig                   |  22 +
 drivers/input/misc/Makefile                  |   1 +
 drivers/input/misc/ebeam.c                   | 759 +++++++++++++++++++++++++++
 4 files changed, 835 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-ebeam
 create mode 100644 drivers/input/misc/ebeam.c

diff --git a/Documentation/ABI/testing/sysfs-driver-ebeam b/Documentation/ABI/testing/sysfs-driver-ebeam
new file mode 100644
index 0000000..552a428
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-ebeam
@@ -0,0 +1,53 @@
+What:		/sys/class/input/inputXX/device/min_x
+		/sys/class/input/inputXX/device/min_y
+		/sys/class/input/inputXX/device/max_x
+		/sys/class/input/inputXX/device/max_y
+Date:		Jul 2013
+Kernel Version:	3.11
+Contact:	yann.cantin@laposte.net
+		linux-usb@vger.kernel.org
+Description:
+		Reading from these files return the actually used range values of
+		the reported coordinates.
+		Writing to these files preset these range values.
+		See below for the calibration procedure.
+
+What:		/sys/class/input/inputXX/device/h[1..9]
+Date:		Jul 2013
+Kernel Version:	3.11
+Contact:	yann.cantin@laposte.net
+		linux-usb@vger.kernel.org
+Description:
+		Reading from these files return the 3x3 transformation matrix elements
+		actually used, in row-major.
+		Writing to these files preset these elements values.
+		See below for the calibration procedure.
+
+What:		/sys/class/input/inputXX/device/calibrated
+Date:		Jul 2013
+Kernel Version:	3.11
+Contact:	yann.cantin@laposte.net
+		linux-usb@vger.kernel.org
+Description:
+		Reading from this file :
+		- Return 0 if the driver is in "un-calibrated" mode : it actually send
+		  raw coordinates in the device's internal coordinates system.
+		- Return 1 if the driver is in "calibrated" mode : it send computed coordinates
+		  that (hopefully) matches the screen's coordinates system.
+		Writing 1 to this file enable the "calibrated" mode, 0 reset the driver in
+		"un-calibrated" mode.
+
+Calibration procedure :
+
+When loaded, the driver is in "un-calibrated" mode : it send device's raw coordinates
+in the [0..65535]x[0..65535] range, the transformation matrix is the identity.
+
+A calibration program have to compute a homography transformation matrix that convert
+the device's raw coordinates to the matching screen's ones.
+It then write to the appropriate sysfs files the computed values, pre-setting the
+driver's parameters : xy range, and the matrix's elements.
+When all values are passed, it write 1 to the calibrated sysfs file to enable the "calibrated" mode.
+
+Warning : The parameters aren't used until 1 is writen to the calibrated sysfs file.
+
+Writing 0 to the calibrated sysfs file reset the driver in "un-calibrated" mode.
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 0b541cd..454df2d 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -93,6 +93,28 @@ config INPUT_BMA150
 	  To compile this driver as a module, choose M here: the
 	  module will be called bma150.
 
+config INPUT_EBEAM_USB
+	tristate "USB eBeam driver"
+	depends on USB_ARCH_HAS_HCD
+	select USB
+	help
+	  Say Y here if you have a USB eBeam pointing device and want to
+	  use it without any proprietary user space tools.
+
+	  Have a look at <http://sourceforge.net/projects/ebeam/> for
+	  a usage description and the required user-space tools.
+
+	  Supported devices :
+	    - Luidia eBeam Classic Projection and eBeam Edge Projection
+	    - Nec NP01Wi1 & NP01Wi2 interactive solution
+
+	  Supposed working devices, need test, may lack functionnality :
+	    - Luidia eBeam Edge Whiteboard and eBeam Engage
+	    - Hitachi Starboard FX-63, FX-77, FX-82, FX-77GII
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ebeam.
+
 config INPUT_PCSPKR
 	tristate "PC Speaker support"
 	depends on PCSPKR_PLATFORM
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 829de43..dd2fe33 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_INPUT_COBALT_BTNS)		+= cobalt_btns.o
 obj-$(CONFIG_INPUT_DA9052_ONKEY)	+= da9052_onkey.o
 obj-$(CONFIG_INPUT_DA9055_ONKEY)	+= da9055_onkey.o
 obj-$(CONFIG_INPUT_DM355EVM)		+= dm355evm_keys.o
+obj-$(CONFIG_INPUT_EBEAM_USB)		+= ebeam.o
 obj-$(CONFIG_INPUT_GP2A)		+= gp2ap002a00f.o
 obj-$(CONFIG_INPUT_GPIO_TILT_POLLED)	+= gpio_tilt_polled.o
 obj-$(CONFIG_HP_SDC_RTC)		+= hp_sdc_rtc.o
diff --git a/drivers/input/misc/ebeam.c b/drivers/input/misc/ebeam.c
new file mode 100644
index 0000000..5052723
--- /dev/null
+++ b/drivers/input/misc/ebeam.c
@@ -0,0 +1,759 @@
+/******************************************************************************
+ *
+ * eBeam driver
+ *
+ * Copyright (C) 2012 Yann Cantin (yann.cantin@laposte.net)
+ *
+ *	This program is free software; you can redistribute it and/or
+ *	modify it under the terms of the GNU General Public License as
+ *	published by the Free Software Foundation; either version 2 of the
+ *	License, or (at your option) any later version.
+ *
+ *  based on
+ *
+ *	usbtouchscreen.c by Daniel Ritz <daniel.ritz@gmx.ch>
+ *	aiptek.c (sysfs/settings) by Chris Atenasio <chris@crud.net>
+ *				     Bryan W. Headley <bwheadley@earthlink.net>
+ *
+ *****************************************************************************/
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/math64.h>
+#include <linux/input.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/usb.h>
+#include <linux/usb/input.h>
+#include <asm/unaligned.h>
+
+#define DRIVER_AUTHOR	"Yann Cantin <yann.cantin@laposte.net>"
+#define DRIVER_DESC	"USB eBeam Driver"
+
+/* Common values for eBeam devices */
+#define REPT_SIZE	8		/* packet size		  */
+#define MIN_RAW_X	0		/* raw coordinates ranges */
+#define MAX_RAW_X	0xFFFF
+#define MIN_RAW_Y	0
+#define MAX_RAW_Y	0xFFFF
+
+
+#define USB_VENDOR_ID_EFI	0x2650	/* Electronics For Imaging, Inc	*/
+#define USB_DEVICE_ID_EFI_EBEAM	0x1311	/* eBeam hardware :		*/
+					/*	Luidia Classic and Edge	*/
+					/*	Nec NP01Wi1 & NP01Wi2	*/
+
+#define EBEAM_BTN_TIP	0x1      /* tip    */
+#define EBEAM_BTN_LIT	0x2      /* little */
+#define EBEAM_BTN_BIG	0x4      /* big    */
+
+/* ebeam settings */
+struct ebeam_settings {
+	int min_x;	/* computed coordinates ranges for the input layer */
+	int max_x;
+	int min_y;
+	int max_y;
+
+	/* H matrix */
+	s64 h1;
+	s64 h2;
+	s64 h3;
+	s64 h4;
+	s64 h5;
+	s64 h6;
+	s64 h7;
+	s64 h8;
+	s64 h9;
+};
+
+/* ebeam device */
+struct ebeam_device {
+	unsigned char		 *data;
+	dma_addr_t		 data_dma;
+	unsigned char		 *buffer;
+	int			 buf_len;
+	struct urb		 *irq;
+	struct usb_interface	 *interface;
+	struct input_dev	 *input;
+	char			 name[128];
+	char			 phys[64];
+	void			 *priv;
+
+	struct ebeam_settings	 cursetting;	/* device's current settings */
+	struct ebeam_settings	 newsetting;	/* ... and new ones	     */
+
+	bool			 calibrated;	/* false : send raw	     */
+						/* true  : send computed     */
+
+	u16			 raw_x, raw_y;	/* raw coordinates	     */
+	int			 x, y;		/* computed coordinates      */
+	int			 btn_map;	/* internal buttons map      */
+};
+
+/* device types */
+enum {
+	DEVTYPE_EBEAM,
+};
+
+static const struct usb_device_id ebeam_devices[] = {
+	{USB_DEVICE(USB_VENDOR_ID_EFI, USB_DEVICE_ID_EFI_EBEAM),
+			.driver_info = DEVTYPE_EBEAM},
+	{}
+};
+
+static void ebeam_init_settings(struct ebeam_device *ebeam)
+{
+	ebeam->calibrated = false;
+
+	/* Init (x,y) min/max to raw ones */
+	ebeam->cursetting.min_x = ebeam->newsetting.min_x = MIN_RAW_X;
+	ebeam->cursetting.max_x = ebeam->newsetting.max_x = MAX_RAW_X;
+	ebeam->cursetting.min_y = ebeam->newsetting.min_y = MIN_RAW_Y;
+	ebeam->cursetting.max_y = ebeam->newsetting.max_y = MAX_RAW_Y;
+
+	/* Safe values for the H matrix (Identity) */
+	ebeam->cursetting.h1 = ebeam->newsetting.h1 = 1;
+	ebeam->cursetting.h2 = ebeam->newsetting.h2 = 0;
+	ebeam->cursetting.h3 = ebeam->newsetting.h3 = 0;
+
+	ebeam->cursetting.h4 = ebeam->newsetting.h4 = 0;
+	ebeam->cursetting.h5 = ebeam->newsetting.h5 = 1;
+	ebeam->cursetting.h6 = ebeam->newsetting.h6 = 0;
+
+	ebeam->cursetting.h7 = ebeam->newsetting.h7 = 0;
+	ebeam->cursetting.h8 = ebeam->newsetting.h8 = 0;
+	ebeam->cursetting.h9 = ebeam->newsetting.h9 = 1;
+}
+
+static void ebeam_setup_input(struct ebeam_device *ebeam,
+			      struct input_dev *input_dev)
+{
+	unsigned long flags;
+
+	/* Take event lock while modifying parameters */
+	spin_lock_irqsave(&input_dev->event_lock, flags);
+
+	/* Properties */
+	set_bit(INPUT_PROP_DIRECT,  input_dev->propbit);
+
+	/* Events generated */
+	set_bit(EV_KEY, input_dev->evbit);
+	set_bit(EV_ABS, input_dev->evbit);
+
+	/* Keys */
+	set_bit(BTN_LEFT,   input_dev->keybit);
+	set_bit(BTN_MIDDLE, input_dev->keybit);
+	set_bit(BTN_RIGHT,  input_dev->keybit);
+
+	/* Axis */
+	if (!ebeam->calibrated) {
+		ebeam->cursetting.min_x = MIN_RAW_X;
+		ebeam->cursetting.max_x = MAX_RAW_X;
+		ebeam->cursetting.min_y = MIN_RAW_Y;
+		ebeam->cursetting.max_y = MAX_RAW_Y;
+	}
+
+	input_set_abs_params(input_dev, ABS_X,
+			     ebeam->cursetting.min_x, ebeam->cursetting.max_x,
+			     0, 0);
+	input_set_abs_params(input_dev, ABS_Y,
+			     ebeam->cursetting.min_y, ebeam->cursetting.max_y,
+			     0, 0);
+
+	spin_unlock_irqrestore(&input_dev->event_lock, flags);
+}
+
+/*******************************************************************************
+ * sysfs part
+ */
+
+/*
+ * screen min/max and H matrix coefs
+ * _get return *current* value
+ * _set set new value
+ */
+#define DEVICE_MINMAX_ATTR(MM)						       \
+static ssize_t ebeam_##MM##_get(struct device *dev,			       \
+				struct device_attribute *attr,		       \
+				char *buf)				       \
+{									       \
+	struct ebeam_device *ebeam = dev_get_drvdata(dev);		       \
+									       \
+	return snprintf(buf, PAGE_SIZE, "%d\n", ebeam->cursetting.MM);	       \
+}									       \
+static ssize_t ebeam_##MM##_set(struct device *dev,			       \
+				struct device_attribute *attr,		       \
+				const char *buf,			       \
+				size_t count)				       \
+{									       \
+	struct ebeam_device *ebeam = dev_get_drvdata(dev);		       \
+	int err, MM;							       \
+									       \
+	err = kstrtoint(buf, 10, &MM);					       \
+	if (err)							       \
+		return err;						       \
+									       \
+	ebeam->newsetting.MM = MM;					       \
+	return count;							       \
+}									       \
+static DEVICE_ATTR(MM, S_IRUGO | S_IWUGO,				       \
+		   ebeam_##MM##_get,					       \
+		   ebeam_##MM##_set)
+
+DEVICE_MINMAX_ATTR(min_x);
+DEVICE_MINMAX_ATTR(max_x);
+DEVICE_MINMAX_ATTR(min_y);
+DEVICE_MINMAX_ATTR(max_y);
+
+#define DEVICE_H_ATTR(SET_ID)						       \
+static ssize_t ebeam_h##SET_ID##_get(struct device *dev,		       \
+				     struct device_attribute *attr,	       \
+				     char *buf)				       \
+{									       \
+	struct ebeam_device *ebeam = dev_get_drvdata(dev);		       \
+									       \
+	return snprintf(buf, PAGE_SIZE, "%lld\n", ebeam->cursetting.h##SET_ID);\
+}									       \
+static ssize_t ebeam_h##SET_ID##_set(struct device *dev,		       \
+				     struct device_attribute *attr,	       \
+				     const char *buf,			       \
+				     size_t count)			       \
+{									       \
+	struct ebeam_device *ebeam = dev_get_drvdata(dev);		       \
+	int err;							       \
+	u64 h;								       \
+									       \
+	err = kstrtoll(buf, 10, &h);					       \
+	if (err)							       \
+		return err;						       \
+									       \
+	ebeam->newsetting.h##SET_ID = h;				       \
+	return count;							       \
+}									       \
+static DEVICE_ATTR(h##SET_ID, S_IRUGO | S_IWUGO,			       \
+		   ebeam_h##SET_ID##_get,				       \
+		   ebeam_h##SET_ID##_set)
+
+DEVICE_H_ATTR(1);
+DEVICE_H_ATTR(2);
+DEVICE_H_ATTR(3);
+DEVICE_H_ATTR(4);
+DEVICE_H_ATTR(5);
+DEVICE_H_ATTR(6);
+DEVICE_H_ATTR(7);
+DEVICE_H_ATTR(8);
+DEVICE_H_ATTR(9);
+
+/*
+ * sysfs calibrated
+ * Once H matrix coefs are set, writing 1 to this file triggers
+ * coordinates mapping.
+ * Anything else reset the device to un-calibrated mode,
+ * storing cursetting in newsetting.
+*/
+static ssize_t ebeam_calibrated_get(struct device *dev,
+				    struct device_attribute *attr,
+				    char *buf)
+{
+	struct ebeam_device *ebeam = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", ebeam->calibrated);
+}
+
+static ssize_t ebeam_calibrated_set(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf,
+				    size_t count)
+{
+	struct ebeam_device *ebeam = dev_get_drvdata(dev);
+	int err, c;
+
+	err = kstrtoint(buf, 10, &c);
+	if (err)
+		return err;
+
+	if (c == 1) {
+		memcpy(&ebeam->cursetting, &ebeam->newsetting,
+		       sizeof(struct ebeam_settings));
+		ebeam->calibrated = true;
+		ebeam_setup_input(ebeam, ebeam->input);
+	} else {
+		memcpy(&ebeam->newsetting, &ebeam->cursetting,
+		       sizeof(struct ebeam_settings));
+		ebeam->calibrated = false;
+		ebeam_setup_input(ebeam, ebeam->input);
+	}
+
+	return count;
+}
+
+static DEVICE_ATTR(calibrated, S_IRUGO | S_IWUGO,
+		   ebeam_calibrated_get, ebeam_calibrated_set);
+
+static struct attribute *ebeam_attrs[] = {
+	&dev_attr_min_x.attr,
+	&dev_attr_min_y.attr,
+	&dev_attr_max_x.attr,
+	&dev_attr_max_y.attr,
+	&dev_attr_h1.attr,
+	&dev_attr_h2.attr,
+	&dev_attr_h3.attr,
+	&dev_attr_h4.attr,
+	&dev_attr_h5.attr,
+	&dev_attr_h6.attr,
+	&dev_attr_h7.attr,
+	&dev_attr_h8.attr,
+	&dev_attr_h9.attr,
+	&dev_attr_calibrated.attr,
+	NULL
+};
+
+static const struct attribute_group ebeam_attr_group = {
+	.attrs = ebeam_attrs,
+};
+
+/* IRQ */
+static int ebeam_read_data(struct ebeam_device *ebeam, unsigned char *pkt)
+{
+
+/*
+ * Packet description : 8 bytes
+ *
+ *  nop packet : FF FF FF FF FF FF FF FF
+ *
+ *  pkt[0] : Sensors
+ *	bit 1 : ultrasound signal (guessed)
+ *	bit 2 : IR signal (tested with a remote...) ;
+ *	readings OK : 0x03 (anything else is a show-stopper)
+ *
+ *  pkt[1] : raw_x low
+ *  pkt[2] : raw_x high
+ *
+ *  pkt[3] : raw_y low
+ *  pkt[4] : raw_y high
+ *
+ *  pkt[5] : fiability ?
+ *	often 0xC0
+ *	> 0x80 : OK
+ *
+ *  pkt[6] :
+ *	buttons state (low 4 bits)
+ *		0x1 = no buttons
+ *		bit 0 : tip (WARNING inversed : 0=pressed)
+ *		bit 1 : ? (always 0 during tests)
+ *		bit 2 : little (1=pressed)
+ *		bit 3 : big (1=pressed)
+ *
+ *	pointer ID : (high 4 bits)
+ *		Tested  : 0x6=wand ;
+ *		Guessed : 0x1=red ; 0x2=blue ; 0x3=green ; 0x4=black ;
+ *			  0x5=eraser
+ *		bit 4 : pointer ID
+ *		bit 5 : pointer ID
+ *		bit 6 : pointer ID
+ *		bit 7 : pointer ID
+ *
+ *
+ *  pkt[7] : fiability ?
+ *	often 0xFF
+ *
+ */
+
+	/* Filtering bad/nop packet */
+	if (pkt[0] != 0x03)
+		return 0;
+
+	ebeam->raw_x = get_unaligned_le16(&pkt[1]);
+	ebeam->raw_y = get_unaligned_le16(&pkt[3]);
+
+	ebeam->btn_map = (!(pkt[6] & 0x1))	|
+			 ((pkt[6] & 0x4) >> 1)	|
+			 ((pkt[6] & 0x8) >> 1);
+
+	return 1;
+}
+
+/*
+ * IRQ
+ * compute screen coordinates from raw
+ * Overflow and negative values are ignored
+ */
+static bool ebeam_calculate_xy(struct ebeam_device *ebeam)
+{
+	/* 64-bits divisions handled by div64_s64 */
+
+	s64 scale;
+
+	if (!ebeam->calibrated) {
+		ebeam->x = ebeam->raw_x;
+		ebeam->y = ebeam->raw_y;
+	} else {
+		scale = ebeam->cursetting.h7 * ebeam->raw_x +
+			ebeam->cursetting.h8 * ebeam->raw_y +
+			ebeam->cursetting.h9;
+
+		/* Who want a division by zero in kernel ? */
+		if (scale == 0) {
+			dev_err(&(ebeam->interface)->dev,
+				"%s - Division by zero, wrong calibration.\n",
+				__func__);
+			dev_err(&(ebeam->interface)->dev,
+				"%s - Resetting to un-calibrated mode.\n",
+				__func__);
+			ebeam->calibrated = false;
+			return 0;
+		}
+
+		/*
+		 * We *must* round the result, but can't do (int) (v1/v2 + 0.5)
+		 *
+		 * (int) (v1/v2 + 0.5) <=> (int) ( (2*v1 + v2)/(2*v2) )
+		 */
+		ebeam->x =
+			(int) div64_s64((((ebeam->cursetting.h1 * ebeam->raw_x +
+					   ebeam->cursetting.h2 * ebeam->raw_y +
+					   ebeam->cursetting.h3) << 1) + scale),
+					   (scale << 1));
+		ebeam->y =
+			(int) div64_s64((((ebeam->cursetting.h4 * ebeam->raw_x +
+					   ebeam->cursetting.h5 * ebeam->raw_y +
+					   ebeam->cursetting.h6) << 1) + scale),
+					   (scale << 1));
+
+		/* check range */
+		if ((ebeam->x < ebeam->cursetting.min_x) ||
+		    (ebeam->x > ebeam->cursetting.max_x) ||
+		    (ebeam->y < ebeam->cursetting.min_y) ||
+		    (ebeam->y > ebeam->cursetting.max_y))
+			return 0;
+	}
+
+	return 1;
+}
+
+/* IRQ */
+static void ebeam_report_input(struct ebeam_device *ebeam)
+{
+	input_report_key(ebeam->input, BTN_LEFT,
+			 (ebeam->btn_map & EBEAM_BTN_TIP));
+	input_report_key(ebeam->input, BTN_MIDDLE,
+			 (ebeam->btn_map & EBEAM_BTN_LIT));
+	input_report_key(ebeam->input, BTN_RIGHT,
+			 (ebeam->btn_map & EBEAM_BTN_BIG));
+
+	input_report_abs(ebeam->input, ABS_X, ebeam->x);
+	input_report_abs(ebeam->input, ABS_Y, ebeam->y);
+
+	input_sync(ebeam->input);
+}
+
+/* IRQ */
+static void ebeam_process_pkt(struct ebeam_device *ebeam,
+			      unsigned char *pkt, int len)
+{
+	if (!ebeam_read_data(ebeam, pkt))
+		return;
+
+	if (!ebeam_calculate_xy(ebeam))
+		return;
+
+	ebeam_report_input(ebeam);
+}
+
+/* IRQ
+ * handler */
+static void ebeam_irq(struct urb *urb)
+{
+	struct ebeam_device *ebeam = urb->context;
+	struct device *dev = &ebeam->interface->dev;
+	int retval;
+
+	switch (urb->status) {
+	case 0:
+		/* success */
+		break;
+	case -ETIME:
+		/* this urb is timing out */
+		dev_dbg(dev,
+			"%s - urb timed out - was the device unplugged?\n",
+			__func__);
+		return;
+	case -ECONNRESET:
+	case -ENOENT:
+	case -ESHUTDOWN:
+	case -EPIPE:
+		/* this urb is terminated, clean up */
+		dev_dbg(dev, "%s - urb shutting down with status: %d\n",
+			__func__, urb->status);
+		return;
+	default:
+		dev_dbg(dev, "%s - nonzero urb status received: %d\n",
+			__func__, urb->status);
+		goto exit;
+	}
+
+	ebeam_process_pkt(ebeam, ebeam->data, urb->actual_length);
+
+exit:
+	usb_mark_last_busy(interface_to_usbdev(ebeam->interface));
+	retval = usb_submit_urb(urb, GFP_ATOMIC);
+	if (retval)
+		dev_err(dev, "%s - usb_submit_urb failed with result: %d\n",
+			__func__, retval);
+}
+
+static int ebeam_open(struct input_dev *input)
+{
+	struct ebeam_device *ebeam = input_get_drvdata(input);
+	int r;
+
+	ebeam->irq->dev = interface_to_usbdev(ebeam->interface);
+
+	r = usb_autopm_get_interface(ebeam->interface) ? -EIO : 0;
+	if (r < 0)
+		goto out;
+
+	if (usb_submit_urb(ebeam->irq, GFP_KERNEL)) {
+		r = -EIO;
+		goto out_put;
+	}
+
+	ebeam->interface->needs_remote_wakeup = 1;
+out_put:
+	usb_autopm_put_interface(ebeam->interface);
+out:
+	return r;
+}
+
+static void ebeam_close(struct input_dev *input)
+{
+	struct ebeam_device *ebeam = input_get_drvdata(input);
+	int r;
+
+	r = usb_autopm_get_interface(ebeam->interface);
+
+	usb_kill_urb(ebeam->irq);
+	ebeam->interface->needs_remote_wakeup = 0;
+
+	if (!r)
+		usb_autopm_put_interface(ebeam->interface);
+}
+
+static int ebeam_suspend(struct usb_interface *intf, pm_message_t message)
+{
+	struct ebeam_device *ebeam = usb_get_intfdata(intf);
+
+	usb_kill_urb(ebeam->irq);
+
+	return 0;
+}
+
+static int ebeam_resume(struct usb_interface *intf)
+{
+	struct ebeam_device *ebeam = usb_get_intfdata(intf);
+	struct input_dev *input = ebeam->input;
+	int result = 0;
+
+	mutex_lock(&input->mutex);
+	if (input->users)
+		result = usb_submit_urb(ebeam->irq, GFP_NOIO);
+	mutex_unlock(&input->mutex);
+
+	return result;
+}
+
+static void ebeam_free_buffers(struct usb_device *udev,
+			       struct ebeam_device *ebeam)
+{
+	usb_free_coherent(udev, REPT_SIZE,
+			  ebeam->data, ebeam->data_dma);
+	kfree(ebeam->buffer);
+}
+
+static struct usb_endpoint_descriptor *
+ebeam_get_input_endpoint(struct usb_host_interface *interface)
+{
+	int i;
+
+	for (i = 0; i < interface->desc.bNumEndpoints; i++)
+		if (usb_endpoint_dir_in(&interface->endpoint[i].desc))
+			return &interface->endpoint[i].desc;
+
+	return NULL;
+}
+
+static int ebeam_probe(struct usb_interface *intf,
+		       const struct usb_device_id *id)
+{
+	struct ebeam_device *ebeam;
+	struct input_dev *input_dev;
+	struct usb_endpoint_descriptor *endpoint;
+	struct usb_device *udev = interface_to_usbdev(intf);
+	int err = -ENOMEM;
+
+	endpoint = ebeam_get_input_endpoint(intf->cur_altsetting);
+	if (!endpoint)
+		return -ENXIO;
+
+	ebeam = kzalloc(sizeof(struct ebeam_device), GFP_KERNEL);
+	input_dev = input_allocate_device();
+	if (!ebeam || !input_dev)
+		goto out_free;
+
+	ebeam_init_settings(ebeam);
+
+	ebeam->data = usb_alloc_coherent(udev, REPT_SIZE,
+					 GFP_KERNEL, &ebeam->data_dma);
+	if (!ebeam->data)
+		goto out_free;
+
+	ebeam->irq = usb_alloc_urb(0, GFP_KERNEL);
+	if (!ebeam->irq) {
+		dev_dbg(&intf->dev,
+			"%s - usb_alloc_urb failed: ebeam->irq\n", __func__);
+		goto out_free_buffers;
+	}
+
+	ebeam->interface = intf;
+	ebeam->input = input_dev;
+
+	/* setup name */
+	snprintf(ebeam->name, sizeof(ebeam->name),
+		 "USB eBeam %04x:%04x",
+		 le16_to_cpu(udev->descriptor.idVendor),
+		 le16_to_cpu(udev->descriptor.idProduct));
+
+	if (udev->manufacturer || udev->product) {
+		strlcat(ebeam->name,
+			" (",
+			sizeof(ebeam->name));
+
+		if (udev->manufacturer)
+			strlcat(ebeam->name,
+				udev->manufacturer,
+				sizeof(ebeam->name));
+
+		if (udev->product) {
+			if (udev->manufacturer)
+				strlcat(ebeam->name,
+					" ",
+					sizeof(ebeam->name));
+			strlcat(ebeam->name,
+				udev->product,
+				sizeof(ebeam->name));
+		}
+
+		if (strlcat(ebeam->name, ")", sizeof(ebeam->name))
+			>= sizeof(ebeam->name)) {
+			/* overflowed, closing ) anyway */
+			ebeam->name[sizeof(ebeam->name)-2] = ')';
+		}
+	}
+
+	/* usb tree */
+	usb_make_path(udev, ebeam->phys, sizeof(ebeam->phys));
+	strlcat(ebeam->phys, "/input0", sizeof(ebeam->phys));
+
+	/* input setup */
+	input_dev->name = ebeam->name;
+	input_dev->phys = ebeam->phys;
+	usb_to_input_id(udev, &input_dev->id);
+	input_dev->dev.parent = &intf->dev;
+
+	input_set_drvdata(input_dev, ebeam);
+
+	input_dev->open = ebeam_open;
+	input_dev->close = ebeam_close;
+
+	/* usb urb setup */
+	if (usb_endpoint_type(endpoint) == USB_ENDPOINT_XFER_INT)
+		usb_fill_int_urb(ebeam->irq, udev,
+			usb_rcvintpipe(udev, endpoint->bEndpointAddress),
+			ebeam->data, REPT_SIZE,
+			ebeam_irq, ebeam, endpoint->bInterval);
+	else
+		usb_fill_bulk_urb(ebeam->irq, udev,
+			usb_rcvbulkpipe(udev, endpoint->bEndpointAddress),
+			ebeam->data, REPT_SIZE,
+			ebeam_irq, ebeam);
+
+	ebeam->irq->dev = udev;
+	ebeam->irq->transfer_dma = ebeam->data_dma;
+	ebeam->irq->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+
+	/* usb final setup */
+	usb_set_intfdata(intf, ebeam);
+
+	/* sysfs setup */
+	err = sysfs_create_group(&intf->dev.kobj, &ebeam_attr_group);
+	if (err) {
+		dev_dbg(&intf->dev,
+			"%s - cannot create sysfs group, err: %d\n",
+			__func__, err);
+		goto out_free_usb;
+	}
+
+	/* input final setup */
+	ebeam_setup_input(ebeam, input_dev);
+
+	err = input_register_device(ebeam->input);
+	if (err) {
+		dev_dbg(&intf->dev,
+			"%s - input_register_device failed, err: %d\n",
+			__func__, err);
+		goto out_remove_sysfs;
+	}
+
+	return 0;
+
+out_remove_sysfs:
+	sysfs_remove_group(&intf->dev.kobj, &ebeam_attr_group);
+out_free_usb:
+	usb_set_intfdata(intf, NULL);
+	usb_free_urb(ebeam->irq);
+out_free_buffers:
+	ebeam_free_buffers(udev, ebeam);
+out_free:
+	input_free_device(input_dev);
+	kfree(ebeam);
+	return err;
+}
+
+static void ebeam_disconnect(struct usb_interface *intf)
+{
+	struct ebeam_device *ebeam = usb_get_intfdata(intf);
+
+	if (!ebeam)
+		return;
+
+	dev_dbg(&intf->dev,
+		"%s - ebeam is initialized, cleaning up\n", __func__);
+
+	usb_set_intfdata(intf, NULL);
+	/* this will stop IO via close */
+	input_unregister_device(ebeam->input);
+	sysfs_remove_group(&intf->dev.kobj, &ebeam_attr_group);
+	usb_free_urb(ebeam->irq);
+	ebeam_free_buffers(interface_to_usbdev(intf), ebeam);
+	kfree(ebeam);
+}
+
+MODULE_DEVICE_TABLE(usb, ebeam_devices);
+
+static struct usb_driver ebeam_driver = {
+	.name			= "ebeam",
+	.probe			= ebeam_probe,
+	.disconnect		= ebeam_disconnect,
+	.suspend		= ebeam_suspend,
+	.resume			= ebeam_resume,
+	.reset_resume		= ebeam_resume,
+	.id_table		= ebeam_devices,
+	.supports_autosuspend	= 1,
+};
+
+module_usb_driver(ebeam_driver);
+
+MODULE_AUTHOR(DRIVER_AUTHOR);
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL");
+
-- 
1.8.1.5

^ permalink raw reply related

* [PATCH v2 1/2] hid: Blacklist eBeam devices
From: Yann Cantin @ 2013-08-30 20:53 UTC (permalink / raw)
  To: linux-input, linux-usb
  Cc: linux-kernel, linux-doc, dmitry.torokhov, jkosina, rob, gregkh,
	Yann Cantin
In-Reply-To: <1377895987-27715-1-git-send-email-yann.cantin@laposte.net>


Signed-off-by: Yann Cantin <yann.cantin@laposte.net>
---
 drivers/hid/hid-core.c | 3 +++
 drivers/hid/hid-ids.h  | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 36668d1..da5dfa0 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1985,6 +1985,9 @@ static const struct hid_device_id hid_ignore_list[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_DELORME, USB_DEVICE_ID_DELORME_EM_LT20) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY, 0x0004) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY, 0x000a) },
+#if defined(CONFIG_INPUT_EBEAM_USB)
+	{ HID_USB_DEVICE(USB_VENDOR_ID_EFI, USB_DEVICE_ID_EFI_EBEAM) },
+#endif
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ESSENTIAL_REALITY, USB_DEVICE_ID_ESSENTIAL_REALITY_P5) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ETT, USB_DEVICE_ID_TC5UH) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ETT, USB_DEVICE_ID_TC4UM) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index ffe4c7a..abd1c53 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -288,6 +288,9 @@
 #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_73F7	0x73f7
 #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_A001	0xa001
 
+#define USB_VENDOR_ID_EFI		0x2650
+#define USB_DEVICE_ID_EFI_EBEAM		0x1311
+
 #define USB_VENDOR_ID_ELECOM		0x056e
 #define USB_DEVICE_ID_ELECOM_BM084	0x0061
 
-- 
1.8.1.5


^ permalink raw reply related

* [PATCH v2 0/2] new USB eBeam input driver
From: Yann Cantin @ 2013-08-30 20:53 UTC (permalink / raw)
  To: linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w, jkosina-AlSwsSmVLrQ,
	rob-VoJi6FS/r0vR7s880joybQ,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r


Hi,

New USB input driver for eBeam devices.

Currently supported (tested) :
- Luidia eBeam classic projection and edge projection models
- Nec "interactive solution" NP01Wi1 & NP01Wi2 accessories.

>From basic usb point of view, all these devices are
indistinguishable : they have the same usb ids and (blank) hid
report descriptors. There's other re-branded hardware that need to be
test, but i bet on same ids.

Patch 1 to blacklist the devices for hid generic-usb.

Patch 2 is the actual driver.

Notable stuff :
- use div64_s64 for portable 64/64-bits divisions.
- 13 sysfs custom files : 9 values for the transformation matrix,
  4 for xy ranges and a calibration trigger.

The module run fine since 3.3.6 kernel, both x86_32 and 64, actually
run in production on a 3.8.13 and compile without errors on
today's linus tree.

Changes from v1 -> v2:
- fix sysfs/input creation race in ebeam_probe
- use get_unaligned_le16 macro in ebeam_read_data
- move usb_kill_urb after usb_autopm_get_interface in ebeam_close
- remove reset_resume(), use resume() in reset_resume entry

Thanks for your help.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] HID: usbhid: quirk for N-Trig DuoSense Touch Screen
From: Vasily Titskiy @ 2013-08-30 19:34 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: linux-kernel@vger.kernel.org, linux-input, Jiri Kosina
In-Reply-To: <CAN+gG=H=r3HH=J+1eMUWSiruEm3F64E6UfH5djjR_Xfk+kmmtQ@mail.gmail.com>

Hi,

I've re-tested it with HID_QUIRK_NO_INIT_REPORTS parameter. Looks
fine, still no delays.

Shoud I re-format & re-submit the patch?
  Vasily Titskiy


On Fri, Aug 30, 2013 at 3:18 PM, Benjamin Tissoires
<benjamin.tissoires@gmail.com> wrote:
> Hi,
>
> first, for the next submission, do not forget to add Jiri (the HID
> maintainer) in CC, otherwise there are huge chances that he will miss
> your patch.
>
> On Fri, Aug 30, 2013 at 8:47 PM, Vasily Titskiy <qehgt0@gmail.com> wrote:
>> The DuoSense touchscreen device causes a 10 second timeout. This fix
>> removes the delay.
>>
>> Signed-off-by: Vasily Titskiy <qehgt0@gmail.com>
>> ---
>>  drivers/hid/hid-ids.h           |    2 ++
>>  drivers/hid/usbhid/hid-quirks.c |    3 +++
>>  2 files changed, 5 insertions(+)
>>
>> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
>> index 2168885..c3929ea 100644
>> --- a/drivers/hid/hid-ids.h
>> +++ b/drivers/hid/hid-ids.h
>> @@ -898,4 +898,6 @@
>>  #define USB_VENDOR_ID_PRIMAX 0x0461
>>  #define USB_DEVICE_ID_PRIMAX_KEYBOARD 0x4e05
>>
>> +#define USB_DEVICE_ID_NTRIG_DUOSENSE 0x1500
>> +
>
> Please keep the list sorted. This define should go into the *_NTRIG_* block.
>
>>  #endif
>> diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
>> index 19b8360..3fd4dfd 100644
>> --- a/drivers/hid/usbhid/hid-quirks.c
>> +++ b/drivers/hid/usbhid/hid-quirks.c
>> @@ -89,6 +89,9 @@ static const struct hid_blacklist {
>>   { USB_VENDOR_ID_SYMBOL, USB_DEVICE_ID_SYMBOL_SCANNER_2, HID_QUIRK_NOGET },
>>   { USB_VENDOR_ID_TPV, USB_DEVICE_ID_TPV_OPTICAL_TOUCHSCREEN, HID_QUIRK_NOGET },
>>   { USB_VENDOR_ID_TURBOX, USB_DEVICE_ID_TURBOX_KEYBOARD, HID_QUIRK_NOGET },
>> +
>
> extra whitespace
>
>> + { USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_DUOSENSE, HID_QUIRK_NOGET },
>
> I'm sure we could try here the new quirk
> HID_QUIRK_NO_INIT_INPUT_REPORTS, this would be more accurate I think.
> At least can you test if HID_QUIRK_NO_INIT_REPORTS is enough?
> HID_QUIRK_NOGET prevent all request from the host to the device, which
> is a little bit too much in many cases.
>
>> +
>
> extra whitespace too.
>
> Cheers,
> Benjamin
>
>
>>   { USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_PF1209,
>> HID_QUIRK_MULTI_INPUT },
>>   { USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_WP4030U,
>> HID_QUIRK_MULTI_INPUT },
>>   { USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_KNA5,
>> HID_QUIRK_MULTI_INPUT },
>> --
>> 1.7.10.4
>> --
>> 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] HID: usbhid: quirk for N-Trig DuoSense Touch Screen
From: Benjamin Tissoires @ 2013-08-30 19:18 UTC (permalink / raw)
  To: Vasily Titskiy; +Cc: linux-kernel@vger.kernel.org, linux-input, Jiri Kosina
In-Reply-To: <CABSveYg3gAnx=cKTqN-iqZdypzivXDuvVAJCdKdbCS6fz3gYkw@mail.gmail.com>

Hi,

first, for the next submission, do not forget to add Jiri (the HID
maintainer) in CC, otherwise there are huge chances that he will miss
your patch.

On Fri, Aug 30, 2013 at 8:47 PM, Vasily Titskiy <qehgt0@gmail.com> wrote:
> The DuoSense touchscreen device causes a 10 second timeout. This fix
> removes the delay.
>
> Signed-off-by: Vasily Titskiy <qehgt0@gmail.com>
> ---
>  drivers/hid/hid-ids.h           |    2 ++
>  drivers/hid/usbhid/hid-quirks.c |    3 +++
>  2 files changed, 5 insertions(+)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 2168885..c3929ea 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -898,4 +898,6 @@
>  #define USB_VENDOR_ID_PRIMAX 0x0461
>  #define USB_DEVICE_ID_PRIMAX_KEYBOARD 0x4e05
>
> +#define USB_DEVICE_ID_NTRIG_DUOSENSE 0x1500
> +

Please keep the list sorted. This define should go into the *_NTRIG_* block.

>  #endif
> diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
> index 19b8360..3fd4dfd 100644
> --- a/drivers/hid/usbhid/hid-quirks.c
> +++ b/drivers/hid/usbhid/hid-quirks.c
> @@ -89,6 +89,9 @@ static const struct hid_blacklist {
>   { USB_VENDOR_ID_SYMBOL, USB_DEVICE_ID_SYMBOL_SCANNER_2, HID_QUIRK_NOGET },
>   { USB_VENDOR_ID_TPV, USB_DEVICE_ID_TPV_OPTICAL_TOUCHSCREEN, HID_QUIRK_NOGET },
>   { USB_VENDOR_ID_TURBOX, USB_DEVICE_ID_TURBOX_KEYBOARD, HID_QUIRK_NOGET },
> +

extra whitespace

> + { USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_DUOSENSE, HID_QUIRK_NOGET },

I'm sure we could try here the new quirk
HID_QUIRK_NO_INIT_INPUT_REPORTS, this would be more accurate I think.
At least can you test if HID_QUIRK_NO_INIT_REPORTS is enough?
HID_QUIRK_NOGET prevent all request from the host to the device, which
is a little bit too much in many cases.

> +

extra whitespace too.

Cheers,
Benjamin


>   { USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_PF1209,
> HID_QUIRK_MULTI_INPUT },
>   { USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_WP4030U,
> HID_QUIRK_MULTI_INPUT },
>   { USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_KNA5,
> HID_QUIRK_MULTI_INPUT },
> --
> 1.7.10.4
> --
> 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: hid: egalax 0x0eef:0x0001 no longer detected by hid-multitouch
From: Forest Bond @ 2013-08-30 18:45 UTC (permalink / raw)
  To: Sebastian Dalfuß; +Cc: linux-input
In-Reply-To: <20130830175946.GA17584@sedf.de>

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

Hi Sebastian,

On Fri, Aug 30, 2013 at 07:59:46PM +0200, Sebastian Dalfuß wrote:
> In 3.6.x the egalax touch panel (Vendor: 0eef, Dev: 0001) worked 
> perfectly well with hid-multitouch, right out of the box. In 3.9 and 
> 3.10 it doesn't; the panel doesn't show up in /proc/bus/input/devices .
> If the module usbtouchscreen is present, 3.9/3.10 wrongly try to use 
> that one (flipped y-axis, wrong scale, get's stuck) instead of 
> hid-multitouch(worked previously perfectly well, no need for 
> calibration, quirks, configuration). If usbtouchscreen isn't present, 
> the panel isn't detected, even if hid-multitouch gets poked with appropriate
> parameters in /sys/module/hid_multitouch/drivers/hid\:hid-multitouch/new_id .
> In an desperate attempt, I manually added the device/vendor id to
> hid-multitouch.c, which didn't yield any improvement. It seems that the hid
> subsystem doesn't even recognize that this device is indeed a hid 
> touchpanel.

That's probably this:

commit 729b814acec20db66fc891b5392cb653ad6598ef
Author: Forest Bond <forest.bond@rapidrollout.com>
Date:   Tue Nov 6 13:41:22 2012 -0500

    HID: Ignore D-WAV/eGalax devices handled by usbtouchscreen
    
    Previously, both usbhid and usbtouchscreen would bind to D-WAV devices
    with class HID and protocol None, so they would be claimed by whichever
    driver was loaded first.  Some of these devices do in fact work with
    usbhid, but not all of them do.  OTOH they all work with usbtouchscreen
    as of commit 037a833ed05a86d01ea27a2c32043b86c549be1b ("Input:
    usbtouchscreen - initialize eGalax devices").  So we ignore them in
    usbhid to prevent getting in the way of usbtouchscreen and claiming an
    interface that we may not be able to do anything useful with.
    
    Signed-off-by: Forest Bond <forest.bond@rapidrollout.com>
    Signed-off-by: Jiri Kosina <jkosina@suse.cz>

The primary issue is that 0eef:0001 has been repeatedly re-used for a wide
variety of devices and as such cannot be meaningfully used on its own to select
a driver.  Some of these devices have class HID and worked fine with
usbtouchscreen.  When usbhid was introduced they stopped working because they
don't actually work with the HID driver.

Historically this is dealt with by submitting a patch that regresses everyone
else's screens but makes yours work better. ;)

So we need to figure out how to handle these devices.  I need to dig out the
screen I have that did not work with the HID driver and test it again.  Might
take some time for me to get to this.

> Is there a way to force hid to take care of that vendor/device-id, as a
> temporary workaround?

For now look at hid_ignore in hid-core.c.  You can probably set
HID_QUIRK_NO_IGNORE to work around the problem.  You'll probably also want to
blacklist usbtouchscreen.

Hope this helps.

Thanks,
Forest
-- 
Forest Bond
http://www.alittletooquiet.net
http://www.rapidrollout.com

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

^ permalink raw reply

* [PATCH] HID: usbhid: quirk for N-Trig DuoSense Touch Screen
From: Vasily Titskiy @ 2013-08-30 18:47 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-input

The DuoSense touchscreen device causes a 10 second timeout. This fix
removes the delay.

Signed-off-by: Vasily Titskiy <qehgt0@gmail.com>
---
 drivers/hid/hid-ids.h           |    2 ++
 drivers/hid/usbhid/hid-quirks.c |    3 +++
 2 files changed, 5 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 2168885..c3929ea 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -898,4 +898,6 @@
 #define USB_VENDOR_ID_PRIMAX 0x0461
 #define USB_DEVICE_ID_PRIMAX_KEYBOARD 0x4e05

+#define USB_DEVICE_ID_NTRIG_DUOSENSE 0x1500
+
 #endif
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 19b8360..3fd4dfd 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -89,6 +89,9 @@ static const struct hid_blacklist {
  { USB_VENDOR_ID_SYMBOL, USB_DEVICE_ID_SYMBOL_SCANNER_2, HID_QUIRK_NOGET },
  { USB_VENDOR_ID_TPV, USB_DEVICE_ID_TPV_OPTICAL_TOUCHSCREEN, HID_QUIRK_NOGET },
  { USB_VENDOR_ID_TURBOX, USB_DEVICE_ID_TURBOX_KEYBOARD, HID_QUIRK_NOGET },
+
+ { USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_DUOSENSE, HID_QUIRK_NOGET },
+
  { USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_PF1209,
HID_QUIRK_MULTI_INPUT },
  { USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_WP4030U,
HID_QUIRK_MULTI_INPUT },
  { USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_KNA5,
HID_QUIRK_MULTI_INPUT },
-- 
1.7.10.4

^ permalink raw reply related

* hid: egalax 0x0eef:0x0001 no longer detected by hid-multitouch
From: Sebastian Dalfuß @ 2013-08-30 17:59 UTC (permalink / raw)
  To: linux-input

Dear Maintainers,

In 3.6.x the egalax touch panel (Vendor: 0eef, Dev: 0001) worked 
perfectly well with hid-multitouch, right out of the box. In 3.9 and 
3.10 it doesn't; the panel doesn't show up in /proc/bus/input/devices .
If the module usbtouchscreen is present, 3.9/3.10 wrongly try to use 
that one (flipped y-axis, wrong scale, get's stuck) instead of 
hid-multitouch(worked previously perfectly well, no need for 
calibration, quirks, configuration). If usbtouchscreen isn't present, 
the panel isn't detected, even if hid-multitouch gets poked with appropriate
parameters in /sys/module/hid_multitouch/drivers/hid\:hid-multitouch/new_id .
In an desperate attempt, I manually added the device/vendor id to
hid-multitouch.c, which didn't yield any improvement. It seems that the hid
subsystem doesn't even recognize that this device is indeed a hid 
touchpanel.

Is there a way to force hid to take care of that vendor/device-id, as a
temporary workaround?

This issue is also filed in Bugzilla: 
https://bugzilla.kernel.org/show_bug.cgi?id=60816

lsusb-snippet:
https://bugzilla.kernel.org/attachment.cgi?id=107362

Greetings,
Sebastian Dalfuß
--
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 11/14] HID: multitouch: validate feature report details
From: Kees Cook @ 2013-08-30 18:27 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Benjamin Tissoires, Jiri Kosina, linux-input, Henrik Rydberg
In-Reply-To: <CAN+gG=EvuO4R2d=bj=_9aWo1e96ee_i0sqSDJNd=3h+dVf=oWQ@mail.gmail.com>

On Fri, Aug 30, 2013 at 8:27 AM, Benjamin Tissoires
<benjamin.tissoires@gmail.com> wrote:
> On Thu, Aug 29, 2013 at 9:41 PM, Kees Cook <keescook@chromium.org> wrote:
>> On Thu, Aug 29, 2013 at 1:59 AM, Benjamin Tissoires
>> <benjamin.tissoires@redhat.com> wrote:
>>> Hi Kees,
>>>
>>> I would be curious to have the HID report descriptors (maybe off list)
>>> to understand how things can be that bad.
>>
>> Certainly! I'll send them your way. I did have to get pretty creative
>> to tickle these conditions.
>>
>>> On overall, I'd prefer all those checks to be in hid-core so that we
>>> have the guarantee that we don't have to open a new CVE each time a
>>> specific hid driver do not check for these ranges.
>>
>> I pondered doing this, but it seemed like something that needed wider
>> discussion, so I thought I'd start with just the dump of fixes. It
>> seems like the entire HID report interface should use access functions
>> to enforce range checking -- perhaps further enforced by making the
>> structure opaque to the drivers.
>
> The problem with access functions with range checking is when they are
> used at each input report. This will overload the kernel processing
> for static checks.
>
>>
>>> More specific comments inlined:
>>>
>>> On 28/08/13 22:31, Jiri Kosina wrote:
>>>> From: Kees Cook <keescook@chromium.org>
>>>>
>>>> When working on report indexes, always validate that they are in bounds.
>>>> Without this, a HID device could report a malicious feature report that
>>>> could trick the driver into a heap overflow:
>>>>
>>>> [  634.885003] usb 1-1: New USB device found, idVendor=0596, idProduct=0500
>>>> ...
>>>> [  676.469629] BUG kmalloc-192 (Tainted: G        W   ): Redzone overwritten
>>>>
>>>> CVE-2013-2897
>>>>
>>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>>> Cc: stable@kernel.org
>>>> ---
>>>>  drivers/hid/hid-multitouch.c |   25 ++++++++++++++++++++-----
>>>>  1 file changed, 20 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>>>> index cb0e361..2aa275e 100644
>>>> --- a/drivers/hid/hid-multitouch.c
>>>> +++ b/drivers/hid/hid-multitouch.c
>>>> @@ -330,9 +330,18 @@ static void mt_feature_mapping(struct hid_device *hdev,
>>>>                               break;
>>>>                       }
>>>>               }
>>>> +             /* Ignore if value index is out of bounds. */
>>>> +             if (td->inputmode_index < 0 ||
>>>
>>> td->inputmode_index can not be less than 0
>>
>> Well, it certainly _shouldn't_ be less than zero. :) However, it is
>> defined as s8, and gets set from an int:
>>
>>                 for (i=0; i < field->maxusage; i++) {
>>                         if (field->usage[i].hid == usage->hid) {
>>                                 td->inputmode_index = i;
>>                                 break;
>>                         }
>>                 }
>>
>> Both "i" and "maxusage" are int, and I can generate a large maxusage
>> and usage array where the first matching hid equality happens when i
>> is >127, causing inputmode_index to wrap.
>
> ouch. Sorry. This piece of code (the current code) is junk. We should
> switch to usage->usage_index and change from __s8 to __s16 the
> declaration ASAP.
>
>>
>>>> +                 td->inputmode_index >= field->report_count) {
>>>
>>> if this is really required, we could just change the for loop above to
>>> go from 0 to field->report_count instead.
>>
>> That's certainly true, but since usage count and report count are not
>> directly associated, I don't know if there are devices that will freak
>> out with this restriction.
>
> Actually, I just again another long time understanding all the mess of
> having usage count and report count different in hid-core.
>
> I think it is a bug at some point (but it looks like I tend to be
> wrong on a lot of things recently :-P ), because when you look at the
> actual code of hid_input_field() in hid-core.c, we never go further
> than field->report_count when dealing with input report.
> So my guess is that we are declaring extra usages that are never used
> to parse incoming data.
>
>>
>>> However, I think we could just rely on usage->usage_index to get the
>>> actual index.
>>> I'll do more tests today and tell you later.
>>>
>>>> +                     dev_err(&hdev->dev, "HID_DG_INPUTMODE out of range\n");
>>>> +                     td->inputmode = -1;
>>>> +             }
>>>>
>>>>               break;
>>>>       case HID_DG_CONTACTMAX:
>>>> +             /* Ignore if value count is out of bounds. */
>>>> +             if (field->report_count < 1)
>>>> +                     break;
>>>
>>> If you can trigger this, I would say that the fix should be in hid-core,
>>> not in every driver. A null report_count should not be allowed by the
>>> HID protocol.
>>
>> Again, I have no problem with that idea, but there seem to be lots of
>> general cases where this may not be possible (i.e. a HID with 0 OUTPUT
>> or FEATURE reports). I didn't see a sensible way to approach this in
>> core without declaring a "I require $foo many OUTPUT reports, $bar
>> many INPUT, and $baz many FEATURE" for each driver. I instead opted
>> for the report validation function instead.
>>
>
> I think we can just add this check in both hidinput_configure_usage()
> and report_features() (both in hid-input.c) so that we don't call the
> *_mapping() callbacks with non sense fields.
> This way, the specific hid drivers will know only the correct fields,
> and don't need to check this. So patches 9, 11 and 13 can be amended.
>
> It looks to me that you mismatched field->report_count and the actual
> report_count in your answer: field->report_count is the number of
> consecutive fields of field->report_size that are present for the
> parsing of the report. It has nothing to do with the total report
> count.
>
>>>>               td->maxcontact_report_id = field->report->id;
>>>>               td->maxcontacts = field->value[0];
>>>>               if (!td->maxcontacts &&
>>>> @@ -743,15 +752,21 @@ static void mt_touch_report(struct hid_device *hid, struct hid_report *report)
>>>>       unsigned count;
>>>>       int r, n;
>>>>
>>>> +     if (report->maxfield == 0)
>>>> +             return;
>>>> +
>>>>       /*
>>>>        * Includes multi-packet support where subsequent
>>>>        * packets are sent with zero contactcount.
>>>>        */
>>>> -     if (td->cc_index >= 0) {
>>>> -             struct hid_field *field = report->field[td->cc_index];
>>>> -             int value = field->value[td->cc_value_index];
>>>> -             if (value)
>>>> -                     td->num_expected = value;
>>>> +     if (td->cc_index >= 0 && td->cc_index < report->maxfield) {
>>>> +             field = report->field[td->cc_index];
>>>
>>> looks like we previously overwrote the definition of field :(
>>>
>>>> +             if (td->cc_value_index >= 0 &&
>>>> +                 td->cc_value_index < field->report_count) {
>>>> +                     int value = field->value[td->cc_value_index];
>>>> +                     if (value)
>>>> +                             td->num_expected = value;
>>>> +             }
>>>
>>> I can not see why td->cc_index and td->cc_value_index could have bad
>>> values. They are initially created by hid-core, and are not provided by
>>> the device.
>>> Anyway, if you are able to produce bad values with them, I'd rather have
>>> those checks during the assignment of td->cc_index (in
>>> mt_touch_input_mapping()), instead of checking them for each report.
>>
>> Well, the problem comes again from there not being a hard link between
>> the usage array and the value array:
>>
>>                         td->cc_value_index = usage->usage_index;
>> ...
>>                         int value = field->value[td->cc_value_index];
>>
>> And all of this would be irrelevant if there was an access function
>> for field->value[].
>
> True, with the current implementation, td->cc_value_index can be
> greater than field->report_count. Still, moving this check in
> mt_touch_input_mapping() will allow us to make it only once.
>
>>
>> Thanks for the review!
>
> you are welcome :)

Okay, so, where does the whole patch series stand? It seems like the
checks are all worth adding; and my fixes are, I think, "minimal" so
having them go into the tree (so that they land in stable) seems like
a good idea. The work beyond that could be done on top of the existing
patches? There seems to be a lot of redesign ideas, but those probably
aren't so great for stable, and I'm probably not the best person to
write them, since everything I know about HID code I learned two weeks
ago. :)

Given the 12 flaws, what do you see as the best way forward?

-Kees

-- 
Kees Cook
Chrome OS Security

^ permalink raw reply


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