Linux Input/HID development
 help / color / mirror / Atom feed
* RE: [PATCH v16 00/12] input: cyapa: instruction of cyapa patches
From: Dudley Du @ 2014-12-18 11:06 UTC (permalink / raw)
  To: Dudley Du, dmitry.torokhov@gmail.com, jmmahler@gmail.com,
	rydberg@euromail.se
  Cc: bleung@google.com, David Solda, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <1418896856-15766-1-git-send-email-dudl@cypress.com>

Jeremiah,

I re-sent the v16 patches through the new internal email server with my emal dudl@cypress.com.
Could you help check if all patches were fine and not broken.

Thanks,
Dudley

> -----Original Message-----
> From: Dudley Du [mailto:dudl@cypress.com]
> Sent: 2014?12?18? 18:01
> To: dmitry.torokhov@gmail.com; jmmahler@gmail.com; rydberg@euromail.se
> Cc: Dudley Du; bleung@google.com; David Solda; linux-input@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: [PATCH v16 00/12] input: cyapa: instruction of cyapa patches
>
> V16 patches have below updates, details of other updates see history list:
> 1) Fix all miss-spelling and space issue.
> 2) Rename variables and functions with much more clearer names.
> 3) Initialize and document tries near where it will be used.
> 4) Modify cmd buffer to struct for more descriptive way.
>
>
> This patch series is aimed to re-design the cyapa driver to support
> old gen3 trackpad devices and new gen5 trackpad devices in one
> cyapa driver, it's for easily productions support based on
> customers' requirements. And add sysfs functions and interfaces
> supported that required by users and customers.
>
> Since the earlier gen3 and the latest gen5 trackpad devices using
> two different chipsets, and have different protocols and interfaces,
> so if supported these two type trackpad devices in two different drivers,
> then it will be difficult to manage productions and later firmware updates.
> e.g.: It will cause customer don't know which one trackpad device firmware
> image to use and update when it has been used and integrated
> in same one productions, so here we support these two trackpad
> devices in same on driver.
>
>
> Dudley Du (12):
>   input: cyapa: re-design driver to support multi-trackpad in one driver
>   input: cyapa: add gen5 trackpad device basic functions support
>   input: cyapa: add power management interfaces support for the device
>   input: cyapa: add runtime power management interfaces support for the
>     device
>   input: cyapa: add sysfs interfaces support in the cyapa driver
>   input: cyapa: add gen3 trackpad device firmware update function
>     support
>   input: cyapa: add gen3 trackpad device read baseline function support
>   input: cyapa: add gen3 trackpad device force re-calibrate function
>     support
>   input: cyapa: add gen5 trackpad device firmware update function
>     support
>   input: cyapa: add gen5 trackpad device read baseline function support
>   input: cyapa: add gen5 trackpad device force re-calibrate function
>     support
>   input: cyapa: add acpi device id support
>
>  drivers/input/mouse/Kconfig      |    1 +
>  drivers/input/mouse/Makefile     |    3 +-
>  drivers/input/mouse/cyapa.c      | 1692 ++++++++++++++---------
>  drivers/input/mouse/cyapa.h      |  314 +++++
>  drivers/input/mouse/cyapa_gen3.c | 1220 +++++++++++++++++
>  drivers/input/mouse/cyapa_gen5.c | 2767
> ++++++++++++++++++++++++++++++++++++++
>  6 files changed, 5325 insertions(+), 672 deletions(-)
>  create mode 100644 drivers/input/mouse/cyapa.h
>  create mode 100644 drivers/input/mouse/cyapa_gen3.c
>  create mode 100644 drivers/input/mouse/cyapa_gen5.c
>
>
> History patch series modifications list:
> V15 patches have below main updates compared with v14 patches:
> 1) Fix all warning errors of sparse tool when running with "make C=1".
> 2) Change variable name "unique_str" to "product_id" for clearer meanings.
> 3) Update cyapa_i2c_write function to return error directly when length > 31.
>
> V14 patches have below main updates compared with v13 patches:
> 1) Correct 9 miss spelling issues of "bufferred" to "buffered".
> 2) Fix the upgrade issue of removing MOUSE_CYAPA config when make oldconfig
>    by replase "depends on I2C && CRC_ITU_T" with
> "depends on I2C"
> "select CRC_ITU_T"
>    in patch 9.
>
> V13 patches have below main updates compared with v12 patches:
> 1) Remove all debugfs interface, including read_fw and raw_data interfaces.
> 2) This patches are made based linux next-20141208.
>
> V12 patches have below main updates compared with v11 patches:
> 1) Add check that when TP is detected but not operational, do not exit driver
>    immediately, but wait and export the update_fw interface for recovering.
> 2) Re-arrange the function codes, remove unnesseary protype definitions in
>    the header file.
>
> V11 patches have below main updates compared with v10 patches:
> 1) Add add acpi device id supported for old gen3 and new gen5 trackpad devices.
> 2) Fix the unable to update firmware issue when cyapa_open is not called
>    which means the irq for firwmare update process is not enabled. This fix
>    by checking if the irq is enabled, if not then enable irq before start to
>    do firmware update.
>
> V10 patches have below main updates compared with v9 patches:
> 1) Modify code to following kernel code style.
>    e.g.: correct to use error as return name when there is only error path,
>    and fix the checkpatch.sh wanting in the driver.
> 2) Remove cyapa_remove method and use input open and close interface to
>    following device resouse management infrastructure.
> 3) Modify cyapa_detect method to return tristate issue to make the return value
>    much more consistent and clear.
> 4) Use platform supplied functions as possible instead of driver
>    specific rewritten version.
>
> V9 patches have below updates compared with v8 patches:
> 1) Removed all async thread stuff from the driver.
> 2) Split driver into 18 patches for each function change one patch.
>
> V8 patches have below updates compared with v7 patches:
> 1) [PATCH v8 01/13] - Remove the async thread for device detect in
>    probe routine, now the device detect process is completely done within
>    the device probe routine.
> 2) [PATCH v8 01/13] - Split the irq cmd hander function to separated
>    function cyapa_default_irq_cmd_handler() and set it to interface
>    cyapa_default_ops.irq_cmd_handler.
> 3) [PATCH v8 06/13] - Add cyapa->gen check in cyapa_gen3_irq_cmd_handler()
>    to avoid miss-enter when device protocol is still in detecting.
>
> V7 patches have below updates compared with v6 patches:
> 1) [PATCH v7 01/13] - Split the irq cmd hander function to separated
>    function cyapa_default_irq_cmd_handler() and set it to interface
>    cyapa_default_ops.irq_cmd_handler.
> 2) [PATCH v7 06/13] - Add cyapa->gen check in cyapa_gen3_irq_cmd_handler()
>    to avoid miss-enter when device protocol is still in detecting.
>
>
> V6 patches have below updates compared with v5 patches:
> 1) Remove patch 14 of the lid filtering from the cyapa driver.
>
> V5 patches have below updates compared with v4 patches:
> 1) Uses get_device()/put_device() instead of kobject_get()/kobject_put();
> 2) Fix memories freed before debugfs entries issue;
> 3) Make cyapa_debugs_root valid in driver module level
>    in module_init()/moudle_exit() ;
> 4) Fix i2c_transfer() may return partial transfer issues.
> 5) Add cyapa->removed flag to avoid detecting thread may still running
>    when driver module is removed.
> 6) Fix the meanings of some comments and return error code not clear issue.

This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.

^ permalink raw reply

* Re: [PATCH] input: adxl34x: Add OF match support
From: Laurent Pinchart @ 2014-12-18 12:49 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Laurent Pinchart, linux-input, linux-sh, linux-i2c,
	Geert Uytterhoeven
In-Reply-To: <20141218082151.GA1038@katana>

Hi Wolfram,

On Thursday 18 December 2014 09:21:51 Wolfram Sang wrote:
> On Thu, Dec 18, 2014 at 04:15:23AM +0200, Laurent Pinchart wrote:
> > The I2C subsystem can match devices without explicit OF support based on
> > the part of their compatible property after the comma. However, this
> > mechanism uses the first compatible value only. For adxl34x OF device
> > nodes the compatible property should list the more specific
> > "adi,adxl345" or "adi,adxl346" value first and the "adi,adxl34x"
> > fallback value second. This prevents the device node from being matched
> > with the adxl34x driver.
> > 
> > Fix this by adding an OF match table with an "adi,adxl34x" compatible
> > entry.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  drivers/input/misc/adxl34x-i2c.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > Another option would have been to add "adxl325" and "adxl326" entries to
> > the adxl34x_id I2C match table, but it would have had the drawback of
> > requiring a driver update for every new device.
> 
> AFAIK this is even required for compatible entries, to be as specific as
> possible. I think this makes sense. With platform_ids, we already had
> the problem that pca954x was too generic and was used for both GPIO
> extenders and I2C muxers (IIRC).

There are three compatible strings defined for the ADXL345 and ADXL346 in 
Documentation/devicetree/bindings/i2c/trivial-devices.txt: "adi,adxl345", 
"adi,adxl346", "adi,adxl34x". Given that the last one is a fallback for the 
first two I don't see a need to add the specific compatible strings to the 
driver for now. If a new totally incompatible chip named ADXL347 comes out we 
will need a new driver which won't be allowed to use the "adi,adxl34x" 
compatible string.

An option would be to remove "adi,adxl34x" from 
Documentation/devicetree/bindings/i2c/trivial-devices.txt, in which case the 
driver should match explicitly on "adi,adxl345" and "adi,adxl346". That might 
clash with the DT ABI stability requirements though.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH] input: adxl34x: Add OF match support
From: Geert Uytterhoeven @ 2014-12-18 13:03 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Wolfram Sang, Laurent Pinchart, linux-input@vger.kernel.org,
	Linux-sh list, Linux I2C
In-Reply-To: <1492007.8cQN1myWLD@avalon>

On Thu, Dec 18, 2014 at 1:49 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> There are three compatible strings defined for the ADXL345 and ADXL346 in
> Documentation/devicetree/bindings/i2c/trivial-devices.txt: "adi,adxl345",
> "adi,adxl346", "adi,adxl34x". Given that the last one is a fallback for the
> first two I don't see a need to add the specific compatible strings to the
> driver for now. If a new totally incompatible chip named ADXL347 comes out we
> will need a new driver which won't be allowed to use the "adi,adxl34x"
> compatible string.

FWIW, I'm the evil person who added the adxl entries to that file...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [RFC] [Patch] implement video driver for sur40
From: Florian Echtler @ 2014-12-18 13:34 UTC (permalink / raw)
  To: linux-input, linux-media


[-- Attachment #1.1: Type: text/plain, Size: 691 bytes --]

Hello everyone,

as promised, I've finally implemented the missing raw video feature for
the SUR40 touchscreen. Since this is a bit of a weird hybrid device
(multitouch input as well as video), I'm hoping for comments from both
communities (linux-input and linux-media). I'm also attaching the full
source code for the moment (not yet a proper patch submission) so it's
perhaps easier to get a view of the whole thing.

There's definitely some obvious cleanup still to be done (e.g. endpoint
selection), but I'd like to ask for feedback early so I can get most
issues fixed before really submitting a patch.

Thanks & best regards, Florian
-- 
SENT FROM MY DEC VT50 TERMINAL

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: sur40.c --]
[-- Type: text/x-csrc; name="sur40.c", Size: 25340 bytes --]

/*
 * Surface2.0/SUR40/PixelSense input driver
 *
 * Copyright (c) 2013 by Florian 'floe' Echtler <floe@butterbrot.org>
 *
 * Derived from the USB Skeleton driver 1.1,
 * Copyright (c) 2003 Greg Kroah-Hartman (greg@kroah.com)
 *
 * and from the Apple USB BCM5974 multitouch driver,
 * Copyright (c) 2008 Henrik Rydberg (rydberg@euromail.se)
 *
 * and from the generic hid-multitouch driver,
 * Copyright (c) 2010-2012 Stephane Chatty <chatty@enac.fr>
 *
 * 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.
 */

#include <linux/kernel.h>
#include <linux/errno.h>
#include <linux/delay.h>
#include <linux/init.h>
#include <linux/slab.h>
#include <linux/module.h>
#include <linux/completion.h>
#include <linux/uaccess.h>
#include <linux/usb.h>
#include <linux/printk.h>
#include <linux/input-polldev.h>
#include <linux/input/mt.h>
#include <linux/usb/input.h>
#include <linux/videodev2.h>
#include <media/v4l2-device.h>
#include <media/v4l2-dev.h>
#include <media/v4l2-ioctl.h>
#include <media/videobuf2-dma-contig.h>

/* read 512 bytes from endpoint 0x86 -> get header + blobs */
struct sur40_header {

	__le16 type;       /* always 0x0001 */
	__le16 count;      /* count of blobs (if 0: continue prev. packet) */

	__le32 packet_id;  /* unique ID for all packets in one frame */

	__le32 timestamp;  /* milliseconds (inc. by 16 or 17 each frame) */
	__le32 unknown;    /* "epoch?" always 02/03 00 00 00 */

} __packed;

struct sur40_blob {

	__le16 blob_id;

	u8 action;         /* 0x02 = enter/exit, 0x03 = update (?) */
	u8 unknown;        /* always 0x01 or 0x02 (no idea what this is?) */

	__le16 bb_pos_x;   /* upper left corner of bounding box */
	__le16 bb_pos_y;

	__le16 bb_size_x;  /* size of bounding box */
	__le16 bb_size_y;

	__le16 pos_x;      /* finger tip position */
	__le16 pos_y;

	__le16 ctr_x;      /* centroid position */
	__le16 ctr_y;

	__le16 axis_x;     /* somehow related to major/minor axis, mostly: */
	__le16 axis_y;     /* axis_x == bb_size_y && axis_y == bb_size_x */

	__le32 angle;      /* orientation in radians relative to x axis -
	                      actually an IEEE754 float, don't use in kernel */

	__le32 area;       /* size in pixels/pressure (?) */

	u8 padding[32];

} __packed;

/* combined header/blob data */
struct sur40_data {
	struct sur40_header header;
	struct sur40_blob   blobs[];
} __packed;

/* read 512 bytes from endpoint 0x82 -> get header below
 * continue reading 16k blocks until header.size bytes read */
struct sur40_image_header {
	__le32 magic;     /* "SUBF" */
	__le32 packet_id;
	__le32 size;      /* always 0x0007e900 = 960x540 */
	__le32 timestamp; /* milliseconds (increases by 16 or 17 each frame) */
	__le32 unknown;   /* "epoch?" always 02/03 00 00 00 */
} __packed;

/* version information */
#define DRIVER_SHORT   "sur40"
#define DRIVER_LONG    "Samsung SUR40"
#define DRIVER_AUTHOR  "Florian 'floe' Echtler <floe@butterbrot.org>"
#define DRIVER_DESC    "Surface2.0/SUR40/PixelSense input driver"

/* vendor and device IDs */
#define ID_MICROSOFT 0x045e
#define ID_SUR40     0x0775

/* sensor resolution */
#define SENSOR_RES_X 1920
#define SENSOR_RES_Y 1080

/* touch data endpoint */
#define TOUCH_ENDPOINT 0x86

/* video data endpoint */
#define VIDEO_ENDPOINT 0x82

/* video header fields */
#define VIDEO_HEADER_MAGIC 0x46425553
#define VIDEO_PACKET_SIZE  16384

/* polling interval (ms) */
#define POLL_INTERVAL 10

/* maximum number of contacts FIXME: this is a guess? */
#define MAX_CONTACTS 64

/* control commands */
#define SUR40_GET_VERSION 0xb0 /* 12 bytes string    */
#define SUR40_UNKNOWN1    0xb3 /*  5 bytes           */
#define SUR40_UNKNOWN2    0xc1 /* 24 bytes           */

#define SUR40_GET_STATE   0xc5 /*  4 bytes state (?) */
#define SUR40_GET_SENSORS 0xb1 /*  8 bytes sensors   */

/*
 * Note: an earlier, non-public version of this driver used USB_RECIP_ENDPOINT
 * here by mistake which is very likely to have corrupted the firmware EEPROM
 * on two separate SUR40 devices. Thanks to Alan Stern who spotted this bug.
 * Should you ever run into a similar problem, the background story to this
 * incident and instructions on how to fix the corrupted EEPROM are available
 * at https://floe.butterbrot.org/matrix/hacking/surface/brick.html
*/

struct sur40_state {

	struct usb_device *usbdev;
	struct device *dev;
	struct input_polled_dev *input;

	struct v4l2_device v4l2;
	struct video_device vdev;
	struct mutex lock;

	struct vb2_queue queue;
	struct vb2_alloc_ctx *alloc_ctx;
	struct list_head buf_list;
	spinlock_t qlock;
	int sequence;

	struct sur40_data *bulk_in_buffer;
	size_t bulk_in_size;
	u8 bulk_in_epaddr;

	char phys[64];
};

struct sur40_buffer {
	struct vb2_buffer vb;
	struct list_head list;
};

/* forward declarations */
static struct video_device sur40_video_device;
static struct v4l2_pix_format sur40_video_format;
static struct vb2_queue sur40_queue;

/* command wrapper */
static int sur40_command(struct sur40_state *dev,
			 u8 command, u16 index, void *buffer, u16 size)
{
	return usb_control_msg(dev->usbdev, usb_rcvctrlpipe(dev->usbdev, 0),
			       command,
			       USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
			       0x00, index, buffer, size, 1000);
}

/* Initialization routine, called from sur40_open */
static int sur40_init(struct sur40_state *dev)
{
	int result;
	u8 buffer[24];

	/* stupidly replay the original MS driver init sequence */
	result = sur40_command(dev, SUR40_GET_VERSION, 0x00, buffer, 12);
	if (result < 0)
		return result;

	result = sur40_command(dev, SUR40_GET_VERSION, 0x01, buffer, 12);
	if (result < 0)
		return result;

	result = sur40_command(dev, SUR40_GET_VERSION, 0x02, buffer, 12);
	if (result < 0)
		return result;

	result = sur40_command(dev, SUR40_UNKNOWN2,    0x00, buffer, 24);
	if (result < 0)
		return result;

	result = sur40_command(dev, SUR40_UNKNOWN1,    0x00, buffer,  5);
	if (result < 0)
		return result;

	result = sur40_command(dev, SUR40_GET_VERSION, 0x03, buffer, 12);

	/*
	 * Discard the result buffer - no known data inside except
	 * some version strings, maybe extract these sometime...
	 */

	return result;
}

/*
 * Callback routines from input_polled_dev
 */

/* Enable the device, polling will now start. */
static void sur40_open(struct input_polled_dev *polldev)
{
	struct sur40_state *sur40 = polldev->private;

	dev_dbg(sur40->dev, "open\n");
	sur40_init(sur40);
}

/* Disable device, polling has stopped. */
static void sur40_close(struct input_polled_dev *polldev)
{
	struct sur40_state *sur40 = polldev->private;

	dev_dbg(sur40->dev, "close\n");
	/*
	 * There is no known way to stop the device, so we simply
	 * stop polling.
	 */
}

/*
 * This function is called when a whole contact has been processed,
 * so that it can assign it to a slot and store the data there.
 */
static void sur40_report_blob(struct sur40_blob *blob, struct input_dev *input)
{
	int wide, major, minor;

	int bb_size_x = le16_to_cpu(blob->bb_size_x);
	int bb_size_y = le16_to_cpu(blob->bb_size_y);

	int pos_x = le16_to_cpu(blob->pos_x);
	int pos_y = le16_to_cpu(blob->pos_y);

	int ctr_x = le16_to_cpu(blob->ctr_x);
	int ctr_y = le16_to_cpu(blob->ctr_y);

	int slotnum = input_mt_get_slot_by_key(input, blob->blob_id);
	if (slotnum < 0 || slotnum >= MAX_CONTACTS)
		return;

	input_mt_slot(input, slotnum);
	input_mt_report_slot_state(input, MT_TOOL_FINGER, 1);
	wide = (bb_size_x > bb_size_y);
	major = max(bb_size_x, bb_size_y);
	minor = min(bb_size_x, bb_size_y);

	input_report_abs(input, ABS_MT_POSITION_X, pos_x);
	input_report_abs(input, ABS_MT_POSITION_Y, pos_y);
	input_report_abs(input, ABS_MT_TOOL_X, ctr_x);
	input_report_abs(input, ABS_MT_TOOL_Y, ctr_y);

	/* TODO: use a better orientation measure */
	input_report_abs(input, ABS_MT_ORIENTATION, wide);
	input_report_abs(input, ABS_MT_TOUCH_MAJOR, major);
	input_report_abs(input, ABS_MT_TOUCH_MINOR, minor);
}

/* core function: poll for new input data */
static void sur40_poll(struct input_polled_dev *polldev)
{
	struct sur40_buffer *new_buf;
	uint8_t *buffer;

	struct sur40_state *sur40 = polldev->private;
	struct input_dev *input = polldev->input;
	int result, bulk_read, need_blobs, packet_blobs, i, bufpos;
	u32 uninitialized_var(packet_id);

	struct sur40_header *header = &sur40->bulk_in_buffer->header;
	struct sur40_blob *inblob = &sur40->bulk_in_buffer->blobs[0];
	struct sur40_image_header *img = (void*)(sur40->bulk_in_buffer);

	dev_dbg(sur40->dev, "poll\n");

	need_blobs = -1;

	do {

		/* perform a blocking bulk read to get data from the device */
		result = usb_bulk_msg(sur40->usbdev,
			usb_rcvbulkpipe(sur40->usbdev, sur40->bulk_in_epaddr),
			sur40->bulk_in_buffer, sur40->bulk_in_size,
			&bulk_read, 1000);

		dev_dbg(sur40->dev, "received %d bytes\n", bulk_read);

		if (result < 0) {
			dev_err(sur40->dev, "error in usb_bulk_read\n");
			return;
		}

		result = bulk_read - sizeof(struct sur40_header);

		if (result % sizeof(struct sur40_blob) != 0) {
			dev_err(sur40->dev, "transfer size mismatch\n");
			return;
		}

		/* first packet? */
		if (need_blobs == -1) {
			need_blobs = le16_to_cpu(header->count);
			dev_dbg(sur40->dev, "need %d blobs\n", need_blobs);
			packet_id = le32_to_cpu(header->packet_id);
		}

		/*
		 * Sanity check. when video data is also being retrieved, the
		 * packet ID will usually increase in the middle of a series
		 * instead of at the end.
		 */
		if (packet_id != header->packet_id)
			dev_warn(sur40->dev, "packet ID mismatch\n");

		packet_blobs = result / sizeof(struct sur40_blob);
		dev_dbg(sur40->dev, "received %d blobs\n", packet_blobs);

		/* packets always contain at least 4 blobs, even if empty */
		if (packet_blobs > need_blobs)
			packet_blobs = need_blobs;

		for (i = 0; i < packet_blobs; i++) {
			need_blobs--;
			dev_dbg(sur40->dev, "processing blob\n");
			sur40_report_blob(&(inblob[i]), input);
		}

	} while (need_blobs > 0);

	input_mt_sync_frame(input);
	input_sync(input);

	/* deal with video data here - should be moved to own method */
	if (list_empty(&sur40->buf_list))
		return;

	/* get a new buffer from the list */
	spin_lock(&sur40->qlock);
	new_buf = list_entry(sur40->buf_list.next, struct sur40_buffer, list);
	list_del(&new_buf->list);
	spin_unlock(&sur40->qlock);

	/* retrieve data via bulk read */
	bufpos = 0;

	result = usb_bulk_msg(sur40->usbdev,
			usb_rcvbulkpipe(sur40->usbdev, VIDEO_ENDPOINT), // sur40->bulk_in_epaddr),
			sur40->bulk_in_buffer, sur40->bulk_in_size,
			&bulk_read, 1000);
	if (result < 0) { dev_err(sur40->dev,"error in usb_bulk_read\n"); goto err_poll; }
	if (bulk_read != sizeof(struct sur40_image_header)) {
		dev_err(sur40->dev, "received %d bytes (%ld expected)\n", bulk_read, sizeof(struct sur40_image_header));
		goto err_poll;
	}

	if (img->magic != VIDEO_HEADER_MAGIC) { dev_err(sur40->dev,"image magic mismatch\n"); goto err_poll; }
	if (img->size  != sur40_video_format.sizeimage) { dev_err(sur40->dev,"image size  mismatch\n"); goto err_poll; }

	buffer = vb2_plane_vaddr(&new_buf->vb, 0);
	while (bufpos < sur40_video_format.sizeimage) {
		result = usb_bulk_msg(sur40->usbdev,
			usb_rcvbulkpipe(sur40->usbdev, VIDEO_ENDPOINT), // sur40->bulk_in_epaddr),
			buffer+bufpos, VIDEO_PACKET_SIZE,
			&bulk_read, 1000);
		if (result < 0) { dev_err(sur40->dev,"error in usb_bulk_read\n"); goto err_poll; }
		bufpos += bulk_read;
	}

	/* mark as finished */
	vb2_set_plane_payload(&new_buf->vb, 0, sur40_video_format.sizeimage);
	v4l2_get_timestamp(&new_buf->vb.v4l2_buf.timestamp);
	new_buf->vb.v4l2_buf.sequence = sur40->sequence++;
	vb2_buffer_done(&new_buf->vb, VB2_BUF_STATE_DONE);
	return;

err_poll:
	vb2_buffer_done(&new_buf->vb, VB2_BUF_STATE_ERROR);
}

/* Initialize input device parameters. */
static void sur40_input_setup(struct input_dev *input_dev)
{
	__set_bit(EV_KEY, input_dev->evbit);
	__set_bit(EV_ABS, input_dev->evbit);

	input_set_abs_params(input_dev, ABS_MT_POSITION_X,
			     0, SENSOR_RES_X, 0, 0);
	input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
			     0, SENSOR_RES_Y, 0, 0);

	input_set_abs_params(input_dev, ABS_MT_TOOL_X,
			     0, SENSOR_RES_X, 0, 0);
	input_set_abs_params(input_dev, ABS_MT_TOOL_Y,
			     0, SENSOR_RES_Y, 0, 0);

	/* max value unknown, but major/minor axis
	 * can never be larger than screen */
	input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
			     0, SENSOR_RES_X, 0, 0);
	input_set_abs_params(input_dev, ABS_MT_TOUCH_MINOR,
			     0, SENSOR_RES_Y, 0, 0);

	input_set_abs_params(input_dev, ABS_MT_ORIENTATION, 0, 1, 0, 0);

	input_mt_init_slots(input_dev, MAX_CONTACTS,
			    INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
}

/* Check candidate USB interface. */
static int sur40_probe(struct usb_interface *interface,
		       const struct usb_device_id *id)
{
	struct usb_device *usbdev = interface_to_usbdev(interface);
	struct sur40_state *sur40;
	struct usb_host_interface *iface_desc;
	struct usb_endpoint_descriptor *endpoint;
	struct input_polled_dev *poll_dev;
	int error;

	/* Check if we really have the right interface. */
	iface_desc = &interface->altsetting[0];
	if (iface_desc->desc.bInterfaceClass != 0xFF)
		return -ENODEV;

	/* Use endpoint #4 (0x86). */
	endpoint = &iface_desc->endpoint[4].desc;
	if (endpoint->bEndpointAddress != TOUCH_ENDPOINT)
		return -ENODEV;

	/* Allocate memory for our device state and initialize it. */
	sur40 = kzalloc(sizeof(struct sur40_state), GFP_KERNEL);
	if (!sur40)
		return -ENOMEM;

	poll_dev = input_allocate_polled_device();
	if (!poll_dev) {
		error = -ENOMEM;
		goto err_free_dev;
	}

	/* initialize locks/lists */
	INIT_LIST_HEAD(&sur40->buf_list);
	spin_lock_init(&sur40->qlock);
	mutex_init(&sur40->lock);

	/* Set up polled input device control structure */
	poll_dev->private = sur40;
	poll_dev->poll_interval = POLL_INTERVAL;
	poll_dev->open = sur40_open;
	poll_dev->poll = sur40_poll;
	poll_dev->close = sur40_close;

	/* Set up regular input device structure */
	sur40_input_setup(poll_dev->input);

	poll_dev->input->name = DRIVER_LONG;
	usb_to_input_id(usbdev, &poll_dev->input->id);
	usb_make_path(usbdev, sur40->phys, sizeof(sur40->phys));
	strlcat(sur40->phys, "/input0", sizeof(sur40->phys));
	poll_dev->input->phys = sur40->phys;
	poll_dev->input->dev.parent = &interface->dev;

	sur40->usbdev = usbdev;
	sur40->dev = &interface->dev;
	sur40->input = poll_dev;

	/* use the bulk-in endpoint tested above */
	sur40->bulk_in_size = usb_endpoint_maxp(endpoint);
	sur40->bulk_in_epaddr = endpoint->bEndpointAddress;
	sur40->bulk_in_buffer = kmalloc(sur40->bulk_in_size, GFP_KERNEL);
	if (!sur40->bulk_in_buffer) {
		dev_err(&interface->dev, "Unable to allocate input buffer.");
		error = -ENOMEM;
		goto err_free_polldev;
	}

	/* register the polled input device */
	error = input_register_polled_device(poll_dev);
	if (error) {
		dev_err(&interface->dev,
			"Unable to register polled input device.");
		goto err_free_buffer;
	}

	/* register the video master device */
	snprintf(sur40->v4l2.name, sizeof(sur40->v4l2.name), "%s", DRIVER_LONG);
	error = v4l2_device_register(sur40->dev, &sur40->v4l2);
	if (error) {
		dev_err(&interface->dev,
			"Unable to register video device.");
		goto err_free_buffer;
	}

	/* initialize the lock and subdevice */
	sur40->queue = sur40_queue;
	sur40->queue.drv_priv = sur40;
	sur40->queue.lock = &sur40->lock;

	// init the queue
	error = vb2_queue_init(&sur40->queue);
	if (error)
		goto err_free_buffer;

	sur40->alloc_ctx = vb2_dma_contig_init_ctx(sur40->dev);
	if (IS_ERR(sur40->alloc_ctx)) {
		dev_err(sur40->dev, "Can't allocate buffer context");
		goto err_free_buffer;
	}

	sur40->vdev = sur40_video_device;
	sur40->vdev.v4l2_dev = &sur40->v4l2;
	sur40->vdev.lock = &sur40->lock;
	sur40->vdev.queue = &sur40->queue;
	video_set_drvdata(&sur40->vdev, sur40);

	error = video_register_device(&sur40->vdev, VFL_TYPE_GRABBER, -1);
	if (error)
		goto err_free_buffer;

	/* we can register the device now, as it is ready */
	usb_set_intfdata(interface, sur40);
	dev_dbg(&interface->dev, "%s is now attached\n", DRIVER_DESC);

	return 0;

/*v4l2_device_unregister(&sur40->v4l2);
video_unregister_device(&sur40->vdev);
vb2_dma_contig_cleanup_ctx(sur40->alloc_ctx);*/

err_free_buffer:
	kfree(sur40->bulk_in_buffer);
err_free_polldev:
	input_free_polled_device(sur40->input);
err_free_dev:
	kfree(sur40);

	return error;
}

/* Unregister device & clean up. */
static void sur40_disconnect(struct usb_interface *interface)
{
	struct sur40_state *sur40 = usb_get_intfdata(interface);

	v4l2_device_unregister(&sur40->v4l2);
	video_unregister_device(&sur40->vdev);
	vb2_dma_contig_cleanup_ctx(sur40->alloc_ctx);

	input_unregister_polled_device(sur40->input);
	input_free_polled_device(sur40->input);
	kfree(sur40->bulk_in_buffer);
	kfree(sur40);

	usb_set_intfdata(interface, NULL);
	dev_dbg(&interface->dev, "%s is now disconnected\n", DRIVER_DESC);
}

/*
 * Setup the constraints of the queue: besides setting the number of planes
 * per buffer and the size and allocation context of each plane, it also
 * checks if sufficient buffers have been allocated. Usually 3 is a good
 * minimum number: many DMA engines need a minimum of 2 buffers in the
 * queue and you need to have another available for userspace processing.
 */
static int sur40_queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
		       unsigned int *nbuffers, unsigned int *nplanes,
		       unsigned int sizes[], void *alloc_ctxs[])
{
	struct sur40_state *sur40 = vb2_get_drv_priv(vq);

	if (vq->num_buffers + *nbuffers < 3)
		*nbuffers = 3 - vq->num_buffers;

	if (fmt && fmt->fmt.pix.sizeimage < sur40_video_format.sizeimage)
		return -EINVAL;

	*nplanes = 1;
	sizes[0] = fmt ? fmt->fmt.pix.sizeimage : sur40_video_format.sizeimage;
	alloc_ctxs[0] = sur40->alloc_ctx;

	return 0;
}

/*
 * Prepare the buffer for queueing to the DMA engine: check and set the
 * payload size.
 */
static int sur40_buffer_prepare(struct vb2_buffer *vb)
{
	struct sur40_state *sur40 = vb2_get_drv_priv(vb->vb2_queue);
	unsigned long size = sur40_video_format.sizeimage;

	if (vb2_plane_size(vb, 0) < size) {
		dev_err(&sur40->usbdev->dev, "buffer too small (%lu < %lu)\n",
			 vb2_plane_size(vb, 0), size);
		return -EINVAL;
	}

	vb2_set_plane_payload(vb, 0, size);
	return 0;
}

/*
 * Queue this buffer to the DMA engine.
 */
static void sur40_buffer_queue(struct vb2_buffer *vb)
{
	struct sur40_state *sur40 = vb2_get_drv_priv(vb->vb2_queue);
	struct sur40_buffer *buf = (struct sur40_buffer*)(vb);

	spin_lock(&sur40->qlock);
	list_add_tail(&buf->list, &sur40->buf_list);
	spin_unlock(&sur40->qlock);
}

static void return_all_buffers(struct sur40_state *sur40,
			       enum vb2_buffer_state state)
{
	struct sur40_buffer *buf, *node;

	spin_lock(&sur40->qlock);
	list_for_each_entry_safe(buf, node, &sur40->buf_list, list) {
		vb2_buffer_done(&buf->vb, state);
		list_del(&buf->list);
	}
	spin_unlock(&sur40->qlock);
}

/*
 * Start streaming. First check if the minimum number of buffers have been
 * queued. If not, then return -ENOBUFS and the vb2 framework will call
 * this function again the next time a buffer has been queued until enough
 * buffers are available to actually start the DMA engine.
 */
static int sur40_start_streaming(struct vb2_queue *vq, unsigned int count)
{
	struct sur40_state *sur40 = vb2_get_drv_priv(vq);
	int ret = 0;

	sur40->sequence = 0;

	/* TODO: start DMA */

	if (ret) {
		/*
		 * In case of an error, return all active buffers to the
		 * QUEUED state
		 */
		return_all_buffers(sur40, VB2_BUF_STATE_QUEUED);
	}
	return ret;
}

/*
 * Stop the DMA engine. Any remaining buffers in the DMA queue are dequeued
 * and passed on to the vb2 framework marked as STATE_ERROR.
 */
static void sur40_stop_streaming(struct vb2_queue *vq)
{
	struct sur40_state *sur40 = vb2_get_drv_priv(vq);

	/* TODO: stop DMA */

	/* Release all active buffers */
	return_all_buffers(sur40, VB2_BUF_STATE_ERROR);
}

/* V4L ioctl */
static int sur40_vidioc_querycap(struct file *file, void *priv,
				 struct v4l2_capability *cap)
{
	struct sur40_state *sur40 = video_drvdata(file);

	strlcpy(cap->driver, DRIVER_SHORT, sizeof(cap->driver));
	strlcpy(cap->card, DRIVER_LONG, sizeof(cap->card));
	snprintf(cap->bus_info, sizeof(cap->bus_info), "USB:%s",
		 sur40->usbdev->devpath);
	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE |
	                   V4L2_CAP_READWRITE |
	                   V4L2_CAP_STREAMING;
	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
	return 0;
}

static int sur40_vidioc_enum_input(struct file *file, void *priv,
				   struct v4l2_input *i)
{
	if (i->index != 0)
		return -EINVAL;
	i->type = V4L2_INPUT_TYPE_CAMERA;
	i->std = V4L2_STD_UNKNOWN;
	strlcpy(i->name, "In-Cell Sensor", sizeof(i->name));
	i->capabilities = 0;
	return 0;
}

static int sur40_vidioc_s_input(struct file *file, void *priv, unsigned int i)
{
	return (i != 0);
}

static int sur40_vidioc_g_input(struct file *file, void *priv, unsigned int *i)
{
	*i = 0;
	return 0;
}

static int sur40_vidioc_fmt(struct file *file, void *priv,
			    struct v4l2_format *f)
{
	f->fmt.pix = sur40_video_format;
	return 0;
}

static int sur40_vidioc_enum_fmt(struct file *file, void *priv,
				 struct v4l2_fmtdesc *f)
{
	if (f->index != 0)
		return -EINVAL;
	strlcpy(f->description, "8-bit greyscale", sizeof(f->description));
	f->pixelformat = V4L2_PIX_FMT_GREY;
	f->flags = 0;
	return 0;
}

static const struct usb_device_id sur40_table[] = {
	{ USB_DEVICE(ID_MICROSOFT, ID_SUR40) },  /* Samsung SUR40 */
	{ }                                      /* terminating null entry */
};
MODULE_DEVICE_TABLE(usb, sur40_table);

/* V4L2 structures */
static struct vb2_ops sur40_queue_ops = {
	.queue_setup		= sur40_queue_setup,
	.buf_prepare		= sur40_buffer_prepare,
	.buf_queue		= sur40_buffer_queue,
	.start_streaming	= sur40_start_streaming,
	.stop_streaming		= sur40_stop_streaming,
	.wait_prepare		= vb2_ops_wait_prepare,
	.wait_finish		= vb2_ops_wait_finish,
};

static struct vb2_queue sur40_queue = {
	.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
	.io_modes = VB2_MMAP | VB2_DMABUF | VB2_READ,
	.buf_struct_size = sizeof(struct sur40_buffer),
	.ops = &sur40_queue_ops,
	.mem_ops = &vb2_dma_contig_memops,
	.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC,
	.min_buffers_needed = 3,
	.gfp_flags = GFP_DMA,
};

static const struct v4l2_file_operations sur40_video_fops = {
	.owner = THIS_MODULE,
	.open = v4l2_fh_open,
	.release = vb2_fop_release,
	.unlocked_ioctl = video_ioctl2,
	.read = vb2_fop_read,
	.mmap = vb2_fop_mmap,
	.poll = vb2_fop_poll,
};

static const struct v4l2_ioctl_ops sur40_video_ioctl_ops = {

	.vidioc_querycap	= sur40_vidioc_querycap,

	.vidioc_enum_fmt_vid_cap = sur40_vidioc_enum_fmt,
	.vidioc_try_fmt_vid_cap	= sur40_vidioc_fmt,
	.vidioc_s_fmt_vid_cap	= sur40_vidioc_fmt,
	.vidioc_g_fmt_vid_cap	= sur40_vidioc_fmt,

	.vidioc_enum_input	= sur40_vidioc_enum_input,
	.vidioc_g_input		= sur40_vidioc_g_input,
	.vidioc_s_input		= sur40_vidioc_s_input,

	.vidioc_reqbufs 	= vb2_ioctl_reqbufs,
	.vidioc_create_bufs 	= vb2_ioctl_create_bufs,
	.vidioc_querybuf 	= vb2_ioctl_querybuf,
	.vidioc_qbuf 		= vb2_ioctl_qbuf,
	.vidioc_dqbuf 		= vb2_ioctl_dqbuf,
	.vidioc_expbuf 		= vb2_ioctl_expbuf,

	.vidioc_streamon 	= vb2_ioctl_streamon,
	.vidioc_streamoff 	= vb2_ioctl_streamoff,

	/*.vidioc_log_status      = v4l2_ctrl_log_status,
	.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,*/
};

static struct video_device sur40_video_device = {
	.name = DRIVER_LONG,
	.fops = &sur40_video_fops,
	.ioctl_ops = &sur40_video_ioctl_ops,
	.release = video_device_release_empty,
};

static struct v4l2_pix_format sur40_video_format = {
	.pixelformat = V4L2_PIX_FMT_GREY,
	.width  = SENSOR_RES_X / 2,
	.height = SENSOR_RES_Y / 2,
	.field = V4L2_FIELD_NONE,
	//.colorspace = V4L2_COLORSPACE_..., FIXME is this required?
	.bytesperline = SENSOR_RES_X / 2,
	.sizeimage = (SENSOR_RES_X/2) * (SENSOR_RES_Y/2),
	.priv = 0,
};

/* USB-specific object needed to register this driver with the USB subsystem. */
static struct usb_driver sur40_driver = {
	.name = DRIVER_SHORT,
	.probe = sur40_probe,
	.disconnect = sur40_disconnect,
	.id_table = sur40_table,
};

module_usb_driver(sur40_driver);

MODULE_AUTHOR(DRIVER_AUTHOR);
MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_LICENSE("GPL");

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: [RFC] [Patch] implement video driver for sur40
From: Hans Verkuil @ 2014-12-18 14:11 UTC (permalink / raw)
  To: Florian Echtler, linux-input, linux-media
In-Reply-To: <5492D7E8.504@butterbrot.org>

On 12/18/14 14:34, Florian Echtler wrote:
> Hello everyone,
> 
> as promised, I've finally implemented the missing raw video feature for
> the SUR40 touchscreen. Since this is a bit of a weird hybrid device
> (multitouch input as well as video), I'm hoping for comments from both
> communities (linux-input and linux-media). I'm also attaching the full
> source code for the moment (not yet a proper patch submission) so it's
> perhaps easier to get a view of the whole thing.
> 
> There's definitely some obvious cleanup still to be done (e.g. endpoint
> selection), but I'd like to ask for feedback early so I can get most
> issues fixed before really submitting a patch.
> 
> Thanks & best regards, Florian
> 

One thing you should do is to run the v4l2-compliance utility. Available
here: http://git.linuxtv.org/cgit.cgi/v4l-utils.git/

Compile from the git repo to ensure you have the latest version.

Run as 'v4l2-compliance -s' (-s starts streaming tests as well and it
assumes you have a valid input signal).

The driver shouldn't give any failures. When you post the actual patch,
then make sure you also paste in the output of 'v4l2-compliance -s' as
I'd like to see what it says.

Mail if you have any questions about the v4l2-compliance output. The failure
messages expect you to look at the v4l2-compliance source code as well,
but even than it is not always clear what is going on.

Regards,

	Hans

^ permalink raw reply

* Re: [PATCH 1/3] HID: logitech-hidpp: detect HID++ 2.0 errors too
From: Peter Wu @ 2014-12-18 17:26 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Jiri Kosina, Nestor Lopez Casado, linux-input,
	linux-kernel@vger.kernel.org
In-Reply-To: <CAN+gG=FiL1SY84W5acowe8hkoHnp6ObkbCtoeFu0s2juone=+Q@mail.gmail.com>

On Tuesday 16 December 2014 09:33:44 Benjamin Tissoires wrote:
> Hi Peter,
> 
> On Mon, Dec 15, 2014 at 7:50 PM, Peter Wu <peter@lekensteyn.nl> wrote:
> > Devices speaking HID++ 2.0 report a different error code (0xff). Detect
> > these errors too to avoid 5 second delays when the device reports an
> > error. Caught by... well, a bug in the QEMU emulation of this receiver.
> >
> > Renamed fap to rap for HID++ 1.0 errors because it is more logical,
> > it has no functional difference.
> >
> > Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> > ---
> 
> I'd like to have Nestor's opinion on this. I did not manage to find on
> the documentation that HID++ 2.0 Long report error code is 0xff, so
> introducing this change without Logitech's blessing would be
> unfortunate.
> I understand this will fix your qemu problem, but I am not entirely
> sure if we do not have to check on 0xff and 0x8f in both short and
> long responses.
> 
> Cheers,
> Benjamin

Hi Benjamin,

The Logitech Unifying extension for Chrome[1] is documented quite well
and contains details which were not public before (including names and
descriptions for all registers and subIDs!).

In lib/devices/HidppFap.js you can find this logic for handling HID++
2.0 messages:

    if ((reqView.getUint8(1) == rspView.getUint8(1)) //  device index
        && (reqView.getUint8(2) == rspView.getUint8(2)) // feature index
        && (reqView.getUint8(3) == rspView.getUint8(3))) // function/event ID + software ID
    {
        result.matchResult = devices.MATCH_RESULT.SUCCESS;
    } else if ((reqView.getUint8(1) == rspView.getUint8(1)) //  device index
        && (0xFF == rspView.getUint8(2)) // Hid++ 2.0 error
        && (reqView.getUint8(2) == rspView.getUint8(3)) // feature index
        && (reqView.getUint8(3) == rspView.getUint8(4))) // function/event ID + software ID
    {
        result.errCode = rspView.getUint8(5); // FAP_ERROR
        result.matchResult = devices.MATCH_RESULT.ERROR;
    }

Looks like a sufficient proof that 0xFF is the correct number to detect
HID++ 2.0 errors right?

In HID++ 1.0 devices ("rap"), 0xFF is named as "SYNC" (with no further
comments), so this will probably not trigger false positives either.

Kind regards,
Peter

 [1]: https://chrome.google.com/webstore/detail/logitech-unifying-for-chr/agpmgihmmmfkbhckmciedmhincdggomo

> >  drivers/hid/hid-logitech-hidpp.c | 17 ++++++++++++++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> > index 2f420c0..ae23dec 100644
> > --- a/drivers/hid/hid-logitech-hidpp.c
> > +++ b/drivers/hid/hid-logitech-hidpp.c
> > @@ -105,6 +105,7 @@ struct hidpp_device {
> >  };
> >
> >
> > +/* HID++ 1.0 error codes */
> >  #define HIDPP_ERROR                            0x8f
> >  #define HIDPP_ERROR_SUCCESS                    0x00
> >  #define HIDPP_ERROR_INVALID_SUBID              0x01
> > @@ -119,6 +120,8 @@ struct hidpp_device {
> >  #define HIDPP_ERROR_REQUEST_UNAVAILABLE                0x0a
> >  #define HIDPP_ERROR_INVALID_PARAM_VALUE                0x0b
> >  #define HIDPP_ERROR_WRONG_PIN_CODE             0x0c
> > +/* HID++ 2.0 error codes */
> > +#define HIDPP20_ERROR                          0xff
> >
> >  static void hidpp_connect_event(struct hidpp_device *hidpp_dev);
> >
> > @@ -192,9 +195,16 @@ static int hidpp_send_message_sync(struct hidpp_device *hidpp,
> >         }
> >
> >         if (response->report_id == REPORT_ID_HIDPP_SHORT &&
> > -           response->fap.feature_index == HIDPP_ERROR) {
> > +           response->rap.sub_id == HIDPP_ERROR) {
> > +               ret = response->rap.params[1];
> > +               dbg_hid("%s:got hidpp error %02X\n", __func__, ret);
> > +               goto exit;
> > +       }
> > +
> > +       if (response->report_id == REPORT_ID_HIDPP_LONG &&
> > +           response->fap.feature_index == HIDPP20_ERROR) {
> >                 ret = response->fap.params[1];
> > -               dbg_hid("__hidpp_send_report got hidpp error %02X\n", ret);
> > +               dbg_hid("%s:got hidpp 2.0 error %02X\n", __func__, ret);
> >                 goto exit;
> >         }
> >
> > @@ -271,7 +281,8 @@ static inline bool hidpp_match_answer(struct hidpp_report *question,
> >  static inline bool hidpp_match_error(struct hidpp_report *question,
> >                 struct hidpp_report *answer)
> >  {
> > -       return (answer->fap.feature_index == HIDPP_ERROR) &&
> > +       return ((answer->rap.sub_id == HIDPP_ERROR) ||
> > +           (answer->fap.feature_index == HIDPP20_ERROR)) &&
> >             (answer->fap.funcindex_clientid == question->fap.feature_index) &&
> >             (answer->fap.params[0] == question->fap.funcindex_clientid);
> >  }
> > --
> > 2.1.3

^ permalink raw reply

* Re: [PATCH v3] input: Add new sun4i-lradc-keys driver
From: Dmitry Torokhov @ 2014-12-18 17:51 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Maxime Ripard, linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <1418898193-9160-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Hi Hans,

On Thu, Dec 18, 2014 at 11:23:13AM +0100, Hans de Goede wrote:
> Allwinnner sunxi SoCs have a low resolution adc (called lradc) which is
> specifically designed to have various (tablet) keys (ie home, back, search,
> etc). attached to it using a resistor network. This adds a driver for this.
> 
> There are 2 channels, currently this driver only supports chan0 since there
> are no boards known to use chan1.
> 
> This has been tested on an olimex a10s-olinuxino-micro, a13-olinuxino, and
> a20-olinuxino-micro.
> 
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> --
> Changes in v2:
> -Change devicetree bindings to use a per key subnode, like gpio-keys does
> Changes in v3:
> -Handle keyup irq flag before irqdown, in case we get both at once

Thank you for making changes. Can you please tell me if the driver still
works if you drop the patch below on top of it? The changes are:

- split DT parsing into a separate function;
- make sure keymap is not empty;
- change 'ret' variable to 'error'; 

Thanks!

-- 
Dmitry

Input: sun4i-lradc-keys - misc changes

From: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Signed-off-by: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/input/keyboard/sun4i-lradc-keys.c |   87 +++++++++++++++++++----------
 1 file changed, 57 insertions(+), 30 deletions(-)

diff --git a/drivers/input/keyboard/sun4i-lradc-keys.c b/drivers/input/keyboard/sun4i-lradc-keys.c
index 06d5c69..cc8f7dd 100644
--- a/drivers/input/keyboard/sun4i-lradc-keys.c
+++ b/drivers/input/keyboard/sun4i-lradc-keys.c
@@ -122,11 +122,11 @@ static irqreturn_t sun4i_lradc_irq(int irq, void *dev_id)
 static int sun4i_lradc_open(struct input_dev *dev)
 {
 	struct sun4i_lradc_data *lradc = input_get_drvdata(dev);
-	int ret;
+	int error;
 
-	ret = regulator_enable(lradc->vref_supply);
-	if (ret)
-		return ret;
+	error = regulator_enable(lradc->vref_supply);
+	if (error)
+		return error;
 
 	/* lradc Vref internally is divided by 2/3 */
 	lradc->vref = regulator_get_voltage(lradc->vref_supply) * 2 / 3;
@@ -155,42 +155,48 @@ static void sun4i_lradc_close(struct input_dev *dev)
 	regulator_disable(lradc->vref_supply);
 }
 
-static int sun4i_lradc_probe(struct platform_device *pdev)
+static int sun4i_lradc_load_dt_keymap(struct device *dev,
+				      struct sun4i_lradc_data *lradc)
 {
-	struct sun4i_lradc_data *lradc;
-	struct device *dev = &pdev->dev;
-	struct device_node *pp, *np = dev->of_node;
-	u32 channel;
-	int i, ret;
+	struct device_node *np, *pp;
+	int i;
+	int error;
 
-	lradc = devm_kzalloc(dev, sizeof(struct sun4i_lradc_data), GFP_KERNEL);
-	if (!lradc)
-		return -ENOMEM;
+	np = dev->of_node;
+	if (!np)
+		return -EINVAL;
 
 	lradc->chan0_map_count = of_get_child_count(np);
-	lradc->chan0_map = devm_kmalloc(dev, lradc->chan0_map_count *
-				sizeof(struct sun4i_lradc_keymap), GFP_KERNEL);
+	if (lradc->chan0_map_count == 0) {
+		dev_err(dev, "keymap is missing in device tree\n");
+		return -EINVAL;
+	}
+
+	lradc->chan0_map = devm_kmalloc_array(dev, lradc->chan0_map_count,
+					      sizeof(struct sun4i_lradc_keymap),
+					      GFP_KERNEL);
 	if (!lradc->chan0_map)
 		return -ENOMEM;
 
 	i = 0;
 	for_each_child_of_node(np, pp) {
 		struct sun4i_lradc_keymap *map = &lradc->chan0_map[i];
+		u32 channel;
 
-		ret = of_property_read_u32(pp, "channel", &channel);
-		if (ret || channel != 0) {
+		error = of_property_read_u32(pp, "channel", &channel);
+		if (error || channel != 0) {
 			dev_err(dev, "%s: Inval channel prop\n", pp->name);
 			return -EINVAL;
 		}
 
-		ret = of_property_read_u32(pp, "voltage", &map->voltage);
-		if (ret) {
+		error = of_property_read_u32(pp, "voltage", &map->voltage);
+		if (error) {
 			dev_err(dev, "%s: Inval voltage prop\n", pp->name);
 			return -EINVAL;
 		}
 
-		ret = of_property_read_u32(pp, "linux,code", &map->keycode);
-		if (ret) {
+		error = of_property_read_u32(pp, "linux,code", &map->keycode);
+		if (error) {
 			dev_err(dev, "%s: Inval linux,code prop\n", pp->name);
 			return -EINVAL;
 		}
@@ -198,6 +204,24 @@ static int sun4i_lradc_probe(struct platform_device *pdev)
 		i++;
 	}
 
+	return 0;
+}
+
+static int sun4i_lradc_probe(struct platform_device *pdev)
+{
+	struct sun4i_lradc_data *lradc;
+	struct device *dev = &pdev->dev;
+	int i;
+	int error;
+
+	lradc = devm_kzalloc(dev, sizeof(struct sun4i_lradc_data), GFP_KERNEL);
+	if (!lradc)
+		return -ENOMEM;
+
+	error = sun4i_lradc_load_dt_keymap(dev, lradc);
+	if (error)
+		return error;
+
 	lradc->vref_supply = devm_regulator_get(dev, "vref");
 	if (IS_ERR(lradc->vref_supply))
 		return PTR_ERR(lradc->vref_supply);
@@ -215,9 +239,11 @@ static int sun4i_lradc_probe(struct platform_device *pdev)
 	lradc->input->id.vendor = 0x0001;
 	lradc->input->id.product = 0x0001;
 	lradc->input->id.version = 0x0100;
-	lradc->input->evbit[0] =  BIT(EV_SYN) | BIT(EV_KEY);
+
+	__set_bit(EV_KEY, lradc->input->evbit);
 	for (i = 0; i < lradc->chan0_map_count; i++)
-		set_bit(lradc->chan0_map[i].keycode, lradc->input->keybit);
+		__set_bit(lradc->chan0_map[i].keycode, lradc->input->keybit);
+
 	input_set_drvdata(lradc->input, lradc);
 
 	lradc->base = devm_ioremap_resource(dev,
@@ -225,14 +251,15 @@ static int sun4i_lradc_probe(struct platform_device *pdev)
 	if (IS_ERR(lradc->base))
 		return PTR_ERR(lradc->base);
 
-	ret = devm_request_irq(dev, platform_get_irq(pdev, 0), sun4i_lradc_irq,
-			       0, "sun4i-a10-lradc-keys", lradc);
-	if (ret)
-		return ret;
+	error = devm_request_irq(dev, platform_get_irq(pdev, 0),
+				 sun4i_lradc_irq, 0,
+				 "sun4i-a10-lradc-keys", lradc);
+	if (error)
+		return error;
 
-	ret = input_register_device(lradc->input);
-	if (ret)
-		return ret;
+	error = input_register_device(lradc->input);
+	if (error)
+		return error;
 
 	platform_set_drvdata(pdev, lradc);
 	return 0;

^ permalink raw reply related

* Re: [PATCH 1/3] HID: logitech-hidpp: detect HID++ 2.0 errors too
From: Benjamin Tissoires @ 2014-12-18 17:57 UTC (permalink / raw)
  To: Peter Wu
  Cc: Jiri Kosina, Nestor Lopez Casado, linux-input,
	linux-kernel@vger.kernel.org
In-Reply-To: <4013567.XXXMc2jNn9@al>

On Thu, Dec 18, 2014 at 12:26 PM, Peter Wu <peter@lekensteyn.nl> wrote:
> On Tuesday 16 December 2014 09:33:44 Benjamin Tissoires wrote:
>> Hi Peter,
>>
>> On Mon, Dec 15, 2014 at 7:50 PM, Peter Wu <peter@lekensteyn.nl> wrote:
>> > Devices speaking HID++ 2.0 report a different error code (0xff). Detect
>> > these errors too to avoid 5 second delays when the device reports an
>> > error. Caught by... well, a bug in the QEMU emulation of this receiver.
>> >
>> > Renamed fap to rap for HID++ 1.0 errors because it is more logical,
>> > it has no functional difference.
>> >
>> > Signed-off-by: Peter Wu <peter@lekensteyn.nl>
>> > ---
>>
>> I'd like to have Nestor's opinion on this. I did not manage to find on
>> the documentation that HID++ 2.0 Long report error code is 0xff, so
>> introducing this change without Logitech's blessing would be
>> unfortunate.
>> I understand this will fix your qemu problem, but I am not entirely
>> sure if we do not have to check on 0xff and 0x8f in both short and
>> long responses.
>>
>> Cheers,
>> Benjamin
>
> Hi Benjamin,
>
> The Logitech Unifying extension for Chrome[1] is documented quite well
> and contains details which were not public before (including names and
> descriptions for all registers and subIDs!).
>
> In lib/devices/HidppFap.js you can find this logic for handling HID++
> 2.0 messages:
>
>     if ((reqView.getUint8(1) == rspView.getUint8(1)) //  device index
>         && (reqView.getUint8(2) == rspView.getUint8(2)) // feature index
>         && (reqView.getUint8(3) == rspView.getUint8(3))) // function/event ID + software ID
>     {
>         result.matchResult = devices.MATCH_RESULT.SUCCESS;
>     } else if ((reqView.getUint8(1) == rspView.getUint8(1)) //  device index
>         && (0xFF == rspView.getUint8(2)) // Hid++ 2.0 error
>         && (reqView.getUint8(2) == rspView.getUint8(3)) // feature index
>         && (reqView.getUint8(3) == rspView.getUint8(4))) // function/event ID + software ID
>     {
>         result.errCode = rspView.getUint8(5); // FAP_ERROR
>         result.matchResult = devices.MATCH_RESULT.ERROR;
>     }
>
> Looks like a sufficient proof that 0xFF is the correct number to detect
> HID++ 2.0 errors right?

Cool :)

Then the patch is:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Cheers,
Benjamin


>
> In HID++ 1.0 devices ("rap"), 0xFF is named as "SYNC" (with no further
> comments), so this will probably not trigger false positives either.
>
> Kind regards,
> Peter
>
>  [1]: https://chrome.google.com/webstore/detail/logitech-unifying-for-chr/agpmgihmmmfkbhckmciedmhincdggomo
>
>> >  drivers/hid/hid-logitech-hidpp.c | 17 ++++++++++++++---
>> >  1 file changed, 14 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
>> > index 2f420c0..ae23dec 100644
>> > --- a/drivers/hid/hid-logitech-hidpp.c
>> > +++ b/drivers/hid/hid-logitech-hidpp.c
>> > @@ -105,6 +105,7 @@ struct hidpp_device {
>> >  };
>> >
>> >
>> > +/* HID++ 1.0 error codes */
>> >  #define HIDPP_ERROR                            0x8f
>> >  #define HIDPP_ERROR_SUCCESS                    0x00
>> >  #define HIDPP_ERROR_INVALID_SUBID              0x01
>> > @@ -119,6 +120,8 @@ struct hidpp_device {
>> >  #define HIDPP_ERROR_REQUEST_UNAVAILABLE                0x0a
>> >  #define HIDPP_ERROR_INVALID_PARAM_VALUE                0x0b
>> >  #define HIDPP_ERROR_WRONG_PIN_CODE             0x0c
>> > +/* HID++ 2.0 error codes */
>> > +#define HIDPP20_ERROR                          0xff
>> >
>> >  static void hidpp_connect_event(struct hidpp_device *hidpp_dev);
>> >
>> > @@ -192,9 +195,16 @@ static int hidpp_send_message_sync(struct hidpp_device *hidpp,
>> >         }
>> >
>> >         if (response->report_id == REPORT_ID_HIDPP_SHORT &&
>> > -           response->fap.feature_index == HIDPP_ERROR) {
>> > +           response->rap.sub_id == HIDPP_ERROR) {
>> > +               ret = response->rap.params[1];
>> > +               dbg_hid("%s:got hidpp error %02X\n", __func__, ret);
>> > +               goto exit;
>> > +       }
>> > +
>> > +       if (response->report_id == REPORT_ID_HIDPP_LONG &&
>> > +           response->fap.feature_index == HIDPP20_ERROR) {
>> >                 ret = response->fap.params[1];
>> > -               dbg_hid("__hidpp_send_report got hidpp error %02X\n", ret);
>> > +               dbg_hid("%s:got hidpp 2.0 error %02X\n", __func__, ret);
>> >                 goto exit;
>> >         }
>> >
>> > @@ -271,7 +281,8 @@ static inline bool hidpp_match_answer(struct hidpp_report *question,
>> >  static inline bool hidpp_match_error(struct hidpp_report *question,
>> >                 struct hidpp_report *answer)
>> >  {
>> > -       return (answer->fap.feature_index == HIDPP_ERROR) &&
>> > +       return ((answer->rap.sub_id == HIDPP_ERROR) ||
>> > +           (answer->fap.feature_index == HIDPP20_ERROR)) &&
>> >             (answer->fap.funcindex_clientid == question->fap.feature_index) &&
>> >             (answer->fap.params[0] == question->fap.funcindex_clientid);
>> >  }
>> > --
>> > 2.1.3
>

^ permalink raw reply

* Re: [PATCH 1/4] alps: v7: Document the v7 touchpad packet protocol
From: Dmitry Torokhov @ 2014-12-18 18:04 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Hans de Goede, linux-input, Benjamin Tissoires
In-Reply-To: <CAN+gG=FMY8Mna0vAx16_6qiK9jXOhGGPaCLrAvM4z1-CUOFzUQ@mail.gmail.com>

On Fri, Dec 05, 2014 at 04:58:00PM -0500, Benjamin Tissoires wrote:
> On Wed, Dec 3, 2014 at 8:27 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> > Add a table documenting where all the bits are in the v7 touchpad packets.
> >
> > Cc: stable@vger.kernel.org # 3.17
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > ---
> 
> Hi Hans,
> 
> I tested yesterday the series on a Toshiba W50-A and a Z30-A if my
> memory is accurate. Internal QA tested these on a Dell M4700 (don't
> know if it has a v7 touchpad though).
> Both tests were conclusive, i.e. they fixes the spurious release when
> right-clicking.
> 
> Tested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> I am not confident enough with the code to review 3/4 and 4/4, but
> they look sane to me.

Applied all, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] Input: atmel_mxt_ts - implement T100 touch object support
From: Benson Leung @ 2014-12-18 18:49 UTC (permalink / raw)
  To: Nick Dyer
  Cc: Dmitry Torokhov, Yufeng Shen, Daniel Kurtz, Henrik Rydberg,
	Joonyoung Shim, Alan Bowens, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org, Peter Meerwald, Olof Johansson,
	Sekhar Nori, Stephen Warren, Chung-yih Wang
In-Reply-To: <1418156167-11724-1-git-send-email-nick.dyer@itdev.co.uk>

Hi Nick

On Tue, Dec 9, 2014 at 12:16 PM, Nick Dyer <nick.dyer@itdev.co.uk> wrote:
> Add support for the new T100 object which replaces the previous T9 multitouch
> touchscreen object in recent maXTouch devices. T100 provides improved
> reporting with selectable auxiliary information, and a type field for
> hover/stylus/glove reporting.
>
> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
> Acked-by: Yufeng Shen <miletus@chromium.org>
> ---
>  drivers/input/touchscreen/atmel_mxt_ts.c | 326 ++++++++++++++++++++++++++++++-
>  1 file changed, 323 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index bb07020..569346e 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -79,6 +79,7 @@
>  #define MXT_SPT_DIGITIZER_T43          43
>  #define MXT_SPT_MESSAGECOUNT_T44       44
>  #define MXT_SPT_CTECONFIG_T46          46
> +#define MXT_TOUCH_MULTITOUCHSCREEN_T100 100
>
>  /* MXT_GEN_MESSAGE_T5 object */
>  #define MXT_RPTID_NOMSG                0xff
> @@ -188,6 +189,33 @@ struct t9_range {
>  #define MXT_RESET_VALUE                0x01
>  #define MXT_BACKUP_VALUE       0x55
>
> +/* T100 Multiple Touch Touchscreen */
> +#define MXT_T100_CTRL          0
> +#define MXT_T100_CFG1          1
> +#define MXT_T100_TCHAUX                3
> +#define MXT_T100_XRANGE                13
> +#define MXT_T100_YRANGE                24
> +
> +#define MXT_T100_CFG_SWITCHXY  (1 << 5)
> +
> +#define MXT_T100_TCHAUX_VECT   (1 << 0)
> +#define MXT_T100_TCHAUX_AMPL   (1 << 1)
> +#define MXT_T100_TCHAUX_AREA   (1 << 2)
> +
> +#define MXT_T100_DETECT                (1 << 7)
> +#define MXT_T100_TYPE_MASK     0x70
> +
> +enum t100_type {
> +       MXT_T100_TYPE_FINGER            = 1,
> +       MXT_T100_TYPE_PASSIVE_STYLUS    = 2,
> +       MXT_T100_TYPE_HOVERING_FINGER   = 4,
> +       MXT_T100_TYPE_GLOVE             = 5,
> +       MXT_T100_TYPE_LARGE_TOUCH       = 6,
> +};
> +
> +#define MXT_TOUCH_MAJOR_DEFAULT        1
> +#define MXT_PRESSURE_DEFAULT   1
> +
>  /* Delay times */
>  #define MXT_BACKUP_TIME                50      /* msec */
>  #define MXT_RESET_TIME         200     /* msec */
> @@ -247,6 +275,9 @@ struct mxt_data {
>         unsigned int max_y;
>         bool in_bootloader;
>         u16 mem_size;
> +       u8 t100_aux_ampl;
> +       u8 t100_aux_area;
> +       u8 t100_aux_vect;
>         u8 max_reportid;
>         u32 config_crc;
>         u32 info_crc;
> @@ -268,6 +299,8 @@ struct mxt_data {
>         u8 T9_reportid_max;
>         u8 T19_reportid;
>         u16 T44_address;
> +       u8 T100_reportid_min;
> +       u8 T100_reportid_max;
>
>         /* for fw update in bootloader */
>         struct completion bl_completion;
> @@ -761,6 +794,113 @@ static void mxt_proc_t9_message(struct mxt_data *data, u8 *message)
>         data->update_input = true;
>  }
>
> +static void mxt_proc_t100_message(struct mxt_data *data, u8 *message)
> +{
> +       struct device *dev = &data->client->dev;
> +       struct input_dev *input_dev = data->input_dev;
> +       int id;
> +       u8 status;
> +       u8 type;
> +       int x;
> +       int y;
> +       int tool;
> +       u8 major = 0;
> +       u8 pressure = 0;
> +       u8 orientation = 0;
> +       bool active = false;
> +       bool hover = false;
> +
> +       id = message[0] - data->T100_reportid_min - 2;
> +
> +       /* ignore SCRSTATUS events */
> +       if (id < 0)
> +               return;
> +
> +       status = message[1];
> +       x = (message[3] << 8) | message[2];
> +       y = (message[5] << 8) | message[4];
> +
> +       if (status & MXT_T100_DETECT) {
> +               type = (status & MXT_T100_TYPE_MASK) >> 4;
> +
> +               switch (type) {
> +               case MXT_T100_TYPE_HOVERING_FINGER:
> +                       hover = true;
> +                       /* fall through */
> +               case MXT_T100_TYPE_FINGER:
> +               case MXT_T100_TYPE_GLOVE:
> +                       active = true;
> +                       tool = MT_TOOL_FINGER;
> +
> +                       if (data->t100_aux_area)
> +                               major = message[data->t100_aux_area];
> +                       if (data->t100_aux_ampl)
> +                               pressure = message[data->t100_aux_ampl];
> +                       if (data->t100_aux_vect)
> +                               orientation = message[data->t100_aux_vect];
> +
> +                       break;
> +
> +               case MXT_T100_TYPE_PASSIVE_STYLUS:
> +                       active = true;
> +                       tool = MT_TOOL_PEN;
> +
> +                       /* Passive stylus is reported with size zero so
> +                        * hardcode */
> +                       major = MXT_TOUCH_MAJOR_DEFAULT;
> +
> +                       if (data->t100_aux_ampl)
> +                               pressure = message[data->t100_aux_ampl];
> +
> +                       break;
> +
> +               case MXT_T100_TYPE_LARGE_TOUCH:
> +                       /* Ignore suppressed touch */
> +                       break;
> +
> +               default:
> +                       dev_dbg(dev, "Unexpected T100 type\n");
> +                       return;
> +               }
> +       }
> +
> +       if (hover) {
> +               pressure = 0;
> +               major = 0;
> +       } else if (active) {
> +               /*
> +                * Values reported should be non-zero if tool is touching the
> +                * device
> +                */
> +               if (pressure == 0)
> +                       pressure = MXT_PRESSURE_DEFAULT;
> +
> +               if (major == 0)
> +                       major = MXT_TOUCH_MAJOR_DEFAULT;
> +       }
> +
> +       input_mt_slot(input_dev, id);
> +
> +       if (active) {
> +               dev_dbg(dev, "[%u] type:%u x:%u y:%u a:%02X p:%02X v:%02X\n",
> +                       id, type, x, y, major, pressure, orientation);
> +
> +               input_mt_report_slot_state(input_dev, tool, 1);
> +               input_report_abs(input_dev, ABS_MT_POSITION_X, x);
> +               input_report_abs(input_dev, ABS_MT_POSITION_Y, y);
> +               input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, major);
> +               input_report_abs(input_dev, ABS_MT_PRESSURE, pressure);
> +               input_report_abs(input_dev, ABS_MT_ORIENTATION, orientation);
> +       } else {
> +               dev_dbg(dev, "[%u] release\n", id);
> +
> +               /* close out slot */
> +               input_mt_report_slot_state(input_dev, 0, 0);
> +       }
> +
> +       data->update_input = true;
> +}
> +

Dmitry and Chung-Yih have had some discussion about how to handle
hovering fingers, and I think the direction they wanted to go was to
use ABS_MT_DISTANCE instead.

http://www.spinics.net/lists/linux-input/msg33950.html

Chung-Yih's also done some work on the forked chromeos-kernel driver
for this as well, if you want to see :
https://chromium-review.googlesource.com/#/c/219280/


-- 
Benson Leung
Software Engineer, Chrom* OS
bleung@chromium.org

^ permalink raw reply

* Re: [PATCH] input: adxl34x: Add OF match support
From: Laurent Pinchart @ 2014-12-18 19:23 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Wolfram Sang, Laurent Pinchart,
	linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux-sh list, Linux I2C
In-Reply-To: <CAMuHMdXcfe0JQhZzGSAoqjKR346-QJrvyi=afHF7cAaHKa56pw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Geert,

On Thursday 18 December 2014 14:03:18 Geert Uytterhoeven wrote:
> On Thu, Dec 18, 2014 at 1:49 PM, Laurent Pinchart wrote:
> > There are three compatible strings defined for the ADXL345 and ADXL346 in
> > Documentation/devicetree/bindings/i2c/trivial-devices.txt: "adi,adxl345",
> > "adi,adxl346", "adi,adxl34x". Given that the last one is a fallback for
> > the first two I don't see a need to add the specific compatible strings to
> > the driver for now. If a new totally incompatible chip named ADXL347 comes
> > out we will need a new driver which won't be allowed to use the
> > "adi,adxl34x" compatible string.
> 
> FWIW, I'm the evil person who added the adxl entries to that file...

git-blame had already reported you.

Do you think we should remove that compatible string ?

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* Re: [PATCH v16 00/12] input: cyapa: instruction of cyapa patches
From: Benson Leung @ 2014-12-18 20:17 UTC (permalink / raw)
  To: Dudley Du
  Cc: dmitry.torokhov@gmail.com, jmmahler@gmail.com,
	rydberg@euromail.se, David Solda, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <CO1PR06MB078DB45B606C923F712A57BD16A0@CO1PR06MB078.namprd06.prod.outlook.com>

Hi Dudley,


On Thu, Dec 18, 2014 at 3:06 AM, Dudley Du <dudl@cypress.com> wrote:
> Jeremiah,
>
> I re-sent the v16 patches through the new internal email server with my emal dudl@cypress.com.
> Could you help check if all patches were fine and not broken.

I tried applying your patches onto dtor's input branch master, and it
applied clean for me. Thanks!


-- 
Benson Leung
Software Engineer, Chrome OS
Google Inc.
bleung@google.com

^ permalink raw reply

* Re: [PATCH v16 00/12] input: cyapa: instruction of cyapa patches
From: Jeremiah Mahler @ 2014-12-18 22:14 UTC (permalink / raw)
  To: Dudley Du
  Cc: dmitry.torokhov@gmail.com, rydberg@euromail.se, bleung@google.com,
	David Solda, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <CO1PR06MB078DB45B606C923F712A57BD16A0@CO1PR06MB078.namprd06.prod.outlook.com>

Dudley,

On Thu, Dec 18, 2014 at 11:06:40AM +0000, Dudley Du wrote:
> Jeremiah,
> 
> I re-sent the v16 patches through the new internal email server with my emal dudl@cypress.com.
> Could you help check if all patches were fine and not broken.
> 
> Thanks,
> Dudley
> 
[...]

These patches apply without any problems.

I will have a look and review them again.

-- 
- Jeremiah Mahler

^ permalink raw reply

* RE: [PATCH v16 00/12] input: cyapa: instruction of cyapa patches
From: Dudley Du @ 2014-12-19  1:45 UTC (permalink / raw)
  To: Jeremiah Mahler
  Cc: dmitry.torokhov@gmail.com, rydberg@euromail.se, bleung@google.com,
	David Solda, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20141218221414.GA6742@hudson.localdomain>

Benson, Jeremiah,

Thank you for the help and confirm.

Dudley

> -----Original Message-----
> From: Jeremiah Mahler [mailto:jmmahler@gmail.com]
> Sent: 2014?12?19? 6:14
> To: Dudley Du
> Cc: dmitry.torokhov@gmail.com; rydberg@euromail.se; bleung@google.com;
> David Solda; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v16 00/12] input: cyapa: instruction of cyapa patches
>
> Dudley,
>
> On Thu, Dec 18, 2014 at 11:06:40AM +0000, Dudley Du wrote:
> > Jeremiah,
> >
> > I re-sent the v16 patches through the new internal email server with my emal
> dudl@cypress.com.
> > Could you help check if all patches were fine and not broken.
> >
> > Thanks,
> > Dudley
> >
> [...]
>
> These patches apply without any problems.
>
> I will have a look and review them again.
>
> --
> - Jeremiah Mahler

This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.

^ permalink raw reply

* [hisi:integration-hilt-linux-linaro 17/28] drivers/input/touchscreen/atmel_mXT224E.c:460:5: sparse: symbol 'i2c_atmel_read' was not declared. Should it be static?
From: kbuild test robot @ 2014-12-19  1:56 UTC (permalink / raw)
  To: Zhangfei Gao; +Cc: kbuild-all, Dmitry Torokhov, linux-input, linux-kernel

tree:   git://github.com/hisilicon/linux-hisi integration-hilt-linux-linaro
head:   791a2af480ea27735b4669c623b665c5bfea9b5c
commit: d7e9caafdf706341b4165d0a8c2089e60dcebc68 [17/28] Input: enable touch atmel_mXT224E
reproduce:
  # apt-get install sparse
  git checkout d7e9caafdf706341b4165d0a8c2089e60dcebc68
  make ARCH=x86_64 allmodconfig
  make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/input/touchscreen/atmel_mXT224E.c:460:5: sparse: symbol 'i2c_atmel_read' was not declared. Should it be static?
>> drivers/input/touchscreen/atmel_mXT224E.c:499:28: sparse: Variable length array is used.
>> drivers/input/touchscreen/atmel_mXT224E.c:496:5: sparse: symbol 'i2c_atmel_write' was not declared. Should it be static?
>> drivers/input/touchscreen/atmel_mXT224E.c:532:5: sparse: symbol 'i2c_atmel_write_byte_data' was not declared. Should it be static?
>> drivers/input/touchscreen/atmel_mXT224E.c:538:10: sparse: symbol 'get_object_address' was not declared. Should it be static?
>> drivers/input/touchscreen/atmel_mXT224E.c:547:9: sparse: symbol 'get_object_size' was not declared. Should it be static?
>> drivers/input/touchscreen/atmel_mXT224E.c:557:9: sparse: symbol 'get_object_size_from_address' was not declared. Should it be static?
>> drivers/input/touchscreen/atmel_mXT224E.c:847:32: sparse: symbol 'atmel_ts_get_pdata' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
http://lists.01.org/mailman/listinfo/kbuild                 Intel Corporation

^ permalink raw reply

* [PATCH hisi] Input: i2c_atmel_read() can be static
From: kbuild test robot @ 2014-12-19  1:56 UTC (permalink / raw)
  To: Zhangfei Gao; +Cc: kbuild-all, Dmitry Torokhov, linux-input, linux-kernel
In-Reply-To: <201412190926.IzkyYvvl%fengguang.wu@intel.com>

drivers/input/touchscreen/atmel_mXT224E.c:460:5: sparse: symbol 'i2c_atmel_read' was not declared. Should it be static?
drivers/input/touchscreen/atmel_mXT224E.c:496:5: sparse: symbol 'i2c_atmel_write' was not declared. Should it be static?
drivers/input/touchscreen/atmel_mXT224E.c:532:5: sparse: symbol 'i2c_atmel_write_byte_data' was not declared. Should it be static?
drivers/input/touchscreen/atmel_mXT224E.c:538:10: sparse: symbol 'get_object_address' was not declared. Should it be static?
drivers/input/touchscreen/atmel_mXT224E.c:547:9: sparse: symbol 'get_object_size' was not declared. Should it be static?
drivers/input/touchscreen/atmel_mXT224E.c:557:9: sparse: symbol 'get_object_size_from_address' was not declared. Should it be static?
drivers/input/touchscreen/atmel_mXT224E.c:847:32: sparse: symbol 'atmel_ts_get_pdata' was not declared. Should it be static?

Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
 atmel_mXT224E.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mXT224E.c b/drivers/input/touchscreen/atmel_mXT224E.c
index 0a7fab2..99bcfde 100644
--- a/drivers/input/touchscreen/atmel_mXT224E.c
+++ b/drivers/input/touchscreen/atmel_mXT224E.c
@@ -457,7 +457,7 @@ static struct atmel_ts_data *private_ts;
 #define LDO_POWR_VOLTAGE 2700000 /*2.7v*/
 static	struct regulator		*LDO;
 
-int i2c_atmel_read(struct i2c_client *client, uint16_t address, uint8_t *data, uint8_t length)
+static int i2c_atmel_read(struct i2c_client *client, uint16_t address, uint8_t *data, uint8_t length)
 {
 	int retry, ret;
 	uint8_t addr[2];
@@ -493,7 +493,7 @@ int i2c_atmel_read(struct i2c_client *client, uint16_t address, uint8_t *data, u
 	return 0;
 }
 
-int i2c_atmel_write(struct i2c_client *client, uint16_t address, uint8_t *data, uint8_t length)
+static int i2c_atmel_write(struct i2c_client *client, uint16_t address, uint8_t *data, uint8_t length)
 {
 	int retry, loop_i, ret;
 	uint8_t buf[length + 2];
@@ -529,13 +529,13 @@ int i2c_atmel_write(struct i2c_client *client, uint16_t address, uint8_t *data,
 
 }
 
-int i2c_atmel_write_byte_data(struct i2c_client *client, uint16_t address, uint8_t value)
+static int i2c_atmel_write_byte_data(struct i2c_client *client, uint16_t address, uint8_t value)
 {
 	i2c_atmel_write(client, address, &value, 1);
 	return 0;
 }
 
-uint16_t get_object_address(struct atmel_ts_data *ts, uint8_t object_type)
+static uint16_t get_object_address(struct atmel_ts_data *ts, uint8_t object_type)
 {
 	uint8_t loop_i;
 	for (loop_i = 0; loop_i < ts->id->num_declared_objects; loop_i++) {
@@ -544,7 +544,7 @@ uint16_t get_object_address(struct atmel_ts_data *ts, uint8_t object_type)
 	}
 	return 0;
 }
-uint8_t get_object_size(struct atmel_ts_data *ts, uint8_t object_type)
+static uint8_t get_object_size(struct atmel_ts_data *ts, uint8_t object_type)
 {
 	uint8_t loop_i;
 	for (loop_i = 0; loop_i < ts->id->num_declared_objects; loop_i++) {
@@ -554,7 +554,7 @@ uint8_t get_object_size(struct atmel_ts_data *ts, uint8_t object_type)
 	return 0;
 }
 
-uint8_t get_object_size_from_address(struct atmel_ts_data *ts, int address)
+static uint8_t get_object_size_from_address(struct atmel_ts_data *ts, int address)
 {
 	uint8_t loop_i;
 	for (loop_i = 0; loop_i < ts->id->num_declared_objects; loop_i++) {
@@ -844,7 +844,7 @@ static int read_object_table(struct atmel_ts_data *ts)
 	return 0;
 }
 
-struct atmel_i2c_platform_data *atmel_ts_get_pdata(struct i2c_client *client)
+static struct atmel_i2c_platform_data *atmel_ts_get_pdata(struct i2c_client *client)
 {
 	struct device_node *node = client->dev.of_node;
 	struct atmel_i2c_platform_data *pdata = client->dev.platform_data;

^ permalink raw reply related

* [PATCH] MAINTAINERS: Update rydberg's addresses
From: Henrik Rydberg @ 2014-12-19  9:06 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Dmitry Torokhov, David S. Miller, Greg Kroah-Hartman, Joe Perches,
	Mauro Carvalho Chehab, linux-input, linux-kernel, Henrik Rydberg

My ISP finally gave up on the old mail address, so I am moving things
over to bitmath.org instead. Also change the status fields to better
reflect reality.

Signed-off-by: Henrik Rydberg <rydberg@bitmath.org>
---
 MAINTAINERS | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index c721042..62b53e8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -714,15 +714,15 @@ F:	include/uapi/linux/apm_bios.h
 F:	drivers/char/apm-emulation.c
 
 APPLE BCM5974 MULTITOUCH DRIVER
-M:	Henrik Rydberg <rydberg@euromail.se>
+M:	Henrik Rydberg <rydberg@bitmath.org>
 L:	linux-input@vger.kernel.org
-S:	Maintained
+S:	Odd fixes
 F:	drivers/input/mouse/bcm5974.c
 
 APPLE SMC DRIVER
-M:	Henrik Rydberg <rydberg@euromail.se>
+M:	Henrik Rydberg <rydberg@bitmath.org>
 L:	lm-sensors@lm-sensors.org
-S:	Maintained
+S:	Odd fixes
 F:	drivers/hwmon/applesmc.c
 
 APPLETALK NETWORK LAYER
@@ -4813,10 +4813,10 @@ F:	include/uapi/linux/input.h
 F:	include/linux/input/
 
 INPUT MULTITOUCH (MT) PROTOCOL
-M:	Henrik Rydberg <rydberg@euromail.se>
+M:	Henrik Rydberg <rydberg@bitmath.org>
 L:	linux-input@vger.kernel.org
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/rydberg/input-mt.git
-S:	Maintained
+S:	Odd fixes
 F:	Documentation/input/multi-touch-protocol.txt
 F:	drivers/input/input-mt.c
 K:	\b(ABS|SYN)_MT_
-- 
2.1.3


^ permalink raw reply related

* Re: [PATCH 1/3] HID: logitech-hidpp: detect HID++ 2.0 errors too
From: Jiri Kosina @ 2014-12-19 10:03 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Peter Wu, Nestor Lopez Casado, linux-input, linux-kernel
In-Reply-To: <20141217153039.GB16047@mail.corp.redhat.com>

On Wed, 17 Dec 2014, Benjamin Tissoires wrote:

> > Devices speaking HID++ 2.0 report a different error code (0xff). Detect
> > these errors too to avoid 5 second delays when the device reports an
> > error. Caught by... well, a bug in the QEMU emulation of this receiver.
> > 
> > Renamed fap to rap for HID++ 1.0 errors because it is more logical,
> > it has no functional difference.
> > 
> > Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> > ---
> 
> Jiri, it looks like this one fall off from your radar.
> 
> It's not a problem per-se, I'd like to have some feedbacks from Logitech
> first, but still, there is a bug and Peter fixed it :)

It's actually still on my radar, but that was exactly the reason I have it 
on hold, because my understanding was that you are waiting for Logitech to 
review it.

Nestor ... ?

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH 1/3] HID: logitech-hidpp: detect HID++ 2.0 errors too
From: Jiri Kosina @ 2014-12-19 10:44 UTC (permalink / raw)
  To: Peter Wu; +Cc: Benjamin Tissoires, Nestor Lopez Casado, linux-input,
	linux-kernel
In-Reply-To: <1418691016-30681-2-git-send-email-peter@lekensteyn.nl>

On Tue, 16 Dec 2014, Peter Wu wrote:

> Devices speaking HID++ 2.0 report a different error code (0xff). Detect
> these errors too to avoid 5 second delays when the device reports an
> error. Caught by... well, a bug in the QEMU emulation of this receiver.
> 
> Renamed fap to rap for HID++ 1.0 errors because it is more logical,
> it has no functional difference.
> 
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>

Applied to for-3.20/logitech.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH 3/3] HID: logitech-hidpp: avoid unintended fall-through
From: Jiri Kosina @ 2014-12-19 10:45 UTC (permalink / raw)
  To: Peter Wu; +Cc: Benjamin Tissoires, Nestor Lopez Casado, linux-input,
	linux-kernel
In-Reply-To: <1418691016-30681-4-git-send-email-peter@lekensteyn.nl>

On Tue, 16 Dec 2014, Peter Wu wrote:

> Add a return to avoid a fall-through. Introduced in commit
> 57ac86cf52e903d9e3e0f12b34c814cce6b65550 ("HID: logitech-hidpp: add
> support of the first Logitech Wireless Touchpad").
> 
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>

Applied to for-3.19/upstream-fixes.

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH 2/3] HID: logitech-{dj,hidpp}: check report length
From: Jiri Kosina @ 2014-12-19 10:48 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Peter Wu, Nestor Lopez Casado, linux-input,
	linux-kernel@vger.kernel.org
In-Reply-To: <CAN+gG=EL0kYim=_uaS4yj7c_XC8rKa1eELxpVXaOuvnWgT_eQw@mail.gmail.com>

On Tue, 16 Dec 2014, Benjamin Tissoires wrote:

> This is my personal opinion and Jiri can say something different. I
> tend not to send big patches while there is a window opened. Sometimes
> Jiri has the time to get through them, sometime he does not.
> In this case, I think the patches you sent should be in the bugs fixes
> categories, and, IMO should make into 3.19-rc1 or 3.19-rc2 (especially
> the length check which could lead to CVEs if not tackled soon enough).
> For these kind of things there is no timing, and the sooner the
> better.
> That being said, make sure that you keep track of those patches in
> case they get lost for obvious reasons and be prepared to remind about
> them if they do not make their way in Jiri's tree.
> 
> Jiri, comments?

I don't mind patches being sent during a merge window, it doesn't disturb 
my workflow.

But it's always good to explicitly mark patches which are bugfixes and 
should go to -rc, so that I bump up the priority for reviewing them.

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH v2] HID: logitech-hidpp: prefix the name with Logitech
From: Jiri Kosina @ 2014-12-19 10:53 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Peter Wu, Nestor Lopez Casado, Peter Hutterer, linux-input,
	linux-kernel
In-Reply-To: <20141217152837.GA16047@mail.corp.redhat.com>

On Wed, 17 Dec 2014, Benjamin Tissoires wrote:

> Thanks for applying most of the patches (2 are missing, I'll raise them
> in your inbox :-P )
> 
> Regarding this one, I was wondering if we could not force it into 3.19,
> or at least add a stable@ tag. I had requested this in the first
> submission, and the rationale was to not change a third time the name of
> the device (from "Logitech Unifying Device. Wireless PID:XXXX" to
> "[TMK]XXX" to "Logitech [TMK]XXX").
> 
> Userspace would be grateful to have a reliable name.

Fair enough. I've cherry-picked it from for-3.20/logitech into 
for-3.19/upstream-fixes as well.

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [RFC] [Patch] implement video driver for sur40
From: Florian Echtler @ 2014-12-19 14:30 UTC (permalink / raw)
  To: Hans Verkuil, linux-input, linux-media
In-Reply-To: <5492E091.1060404@xs4all.nl>

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

On 18.12.2014 15:11, Hans Verkuil wrote:
> Run as 'v4l2-compliance -s' (-s starts streaming tests as well and it
> assumes you have a valid input signal).
> Mail if you have any questions about the v4l2-compliance output. The failure
> messages expect you to look at the v4l2-compliance source code as well,
> but even than it is not always clear what is going on.
Ran the most recent version from git master, got a total of 6 fails, 4
of which are probably easy fixes:

> fail: v4l2-compliance.cpp(306): missing bus_info prefix ('USB:1')
> test VIDIOC_QUERYCAP: FAIL
Changed the relevant code to:
  usb_make_path(sur40->usbdev, cap->bus_info, sizeof(cap->bus_info));
	
> fail: v4l2-test-input-output.cpp(455): could set input to invalid input 1
> test VIDIOC_G/S/ENUMINPUT: FAIL
Now returning -EINVAL when S_INPUT called with input != 0.

> fail: v4l2-test-formats.cpp(322): !colorspace
> fail: v4l2-test-formats.cpp(429): testColorspace(pix.pixelformat,
pix.colorspace, pix.ycbcr_enc, pix.quantization)
> test VIDIOC_G_FMT: FAIL
Setting colorspace in v4l2_pix_format to V4L2_COLORSPACE_SRGB.	

> fail: v4l2-compliance.cpp(365): doioctl(node, VIDIOC_G_PRIORITY, &prio)
> test VIDIOC_G/S_PRIORITY: FAIL
Don't know how to fix this - does this mean VIDIOC_G/S_PRIORITY _must_
be implemented?

> fail: v4l2-test-buffers.cpp(500): q.has_expbuf(node)
> test VIDIOC_EXPBUF: FAIL
Also not clear how to fix this one.

Could you give some hints on the last two?

Thanks & best regards, Florian
-- 
SENT FROM MY DEC VT50 TERMINAL


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: [RFC] [Patch] implement video driver for sur40
From: Hans Verkuil @ 2014-12-19 14:36 UTC (permalink / raw)
  To: Florian Echtler, linux-input, linux-media
In-Reply-To: <54943680.3020007@butterbrot.org>



On 12/19/2014 03:30 PM, Florian Echtler wrote:
> On 18.12.2014 15:11, Hans Verkuil wrote:
>> Run as 'v4l2-compliance -s' (-s starts streaming tests as well and it
>> assumes you have a valid input signal).
>> Mail if you have any questions about the v4l2-compliance output. The failure
>> messages expect you to look at the v4l2-compliance source code as well,
>> but even than it is not always clear what is going on.
> Ran the most recent version from git master, got a total of 6 fails, 4
> of which are probably easy fixes:
> 
>> fail: v4l2-compliance.cpp(306): missing bus_info prefix ('USB:1')
>> test VIDIOC_QUERYCAP: FAIL
> Changed the relevant code to:
>   usb_make_path(sur40->usbdev, cap->bus_info, sizeof(cap->bus_info));
> 	
>> fail: v4l2-test-input-output.cpp(455): could set input to invalid input 1
>> test VIDIOC_G/S/ENUMINPUT: FAIL
> Now returning -EINVAL when S_INPUT called with input != 0.
> 
>> fail: v4l2-test-formats.cpp(322): !colorspace
>> fail: v4l2-test-formats.cpp(429): testColorspace(pix.pixelformat,
> pix.colorspace, pix.ycbcr_enc, pix.quantization)
>> test VIDIOC_G_FMT: FAIL
> Setting colorspace in v4l2_pix_format to V4L2_COLORSPACE_SRGB.	
> 
>> fail: v4l2-compliance.cpp(365): doioctl(node, VIDIOC_G_PRIORITY, &prio)
>> test VIDIOC_G/S_PRIORITY: FAIL
> Don't know how to fix this - does this mean VIDIOC_G/S_PRIORITY _must_
> be implemented?
> 
>> fail: v4l2-test-buffers.cpp(500): q.has_expbuf(node)
>> test VIDIOC_EXPBUF: FAIL
> Also not clear how to fix this one.
> 
> Could you give some hints on the last two?

Can you post the driver code you used to run these tests? And which kernel version
and git tree did you base your patch on?

Regards,

	Hans

> 
> Thanks & best regards, Florian
> 

^ permalink raw reply

* Re: [RFC] [Patch] implement video driver for sur40
From: Florian Echtler @ 2014-12-19 14:57 UTC (permalink / raw)
  To: Hans Verkuil, linux-input, linux-media
In-Reply-To: <549437DA.6090601@xs4all.nl>


[-- Attachment #1.1: Type: text/plain, Size: 2013 bytes --]

On 19.12.2014 15:36, Hans Verkuil wrote:
> On 12/19/2014 03:30 PM, Florian Echtler wrote:
>> Ran the most recent version from git master, got a total of 6 fails, 4
>> of which are probably easy fixes:
>>
>>> fail: v4l2-compliance.cpp(306): missing bus_info prefix ('USB:1')
>>> test VIDIOC_QUERYCAP: FAIL
>> Changed the relevant code to:
>>   usb_make_path(sur40->usbdev, cap->bus_info, sizeof(cap->bus_info));
>> 	
>>> fail: v4l2-test-input-output.cpp(455): could set input to invalid input 1
>>> test VIDIOC_G/S/ENUMINPUT: FAIL
>> Now returning -EINVAL when S_INPUT called with input != 0.
>>
>>> fail: v4l2-test-formats.cpp(322): !colorspace
>>> fail: v4l2-test-formats.cpp(429): testColorspace(pix.pixelformat,
>> pix.colorspace, pix.ycbcr_enc, pix.quantization)
>>> test VIDIOC_G_FMT: FAIL
>> Setting colorspace in v4l2_pix_format to V4L2_COLORSPACE_SRGB.	
>>
>>> fail: v4l2-compliance.cpp(365): doioctl(node, VIDIOC_G_PRIORITY, &prio)
>>> test VIDIOC_G/S_PRIORITY: FAIL
>> Don't know how to fix this - does this mean VIDIOC_G/S_PRIORITY _must_
>> be implemented?
>>
>>> fail: v4l2-test-buffers.cpp(500): q.has_expbuf(node)
>>> test VIDIOC_EXPBUF: FAIL
>> Also not clear how to fix this one.
>>
>> Could you give some hints on the last two?
> 
> Can you post the driver code you used to run these tests? And which kernel version
> and git tree did you base your patch on?
Driver code is attached (should be identical to the one from initial
mail). Kernel version used for the tests is 3.16.0-25 from Ubuntu
Utopic, git tree for patches is currently
https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git

I'm building the module standalone on the target machine, since it's not
powerful enough that you would want to do a full kernel build. However,
since the driver was merged into mainline, no other changes have been
made, so I think it shouldn't be a problem to patch against the original
git tree?

Best, Florian
-- 
SENT FROM MY DEC VT50 TERMINAL

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: sur40.c --]
[-- Type: text/x-csrc; name="sur40.c", Size: 25340 bytes --]

/*
 * Surface2.0/SUR40/PixelSense input driver
 *
 * Copyright (c) 2013 by Florian 'floe' Echtler <floe@butterbrot.org>
 *
 * Derived from the USB Skeleton driver 1.1,
 * Copyright (c) 2003 Greg Kroah-Hartman (greg@kroah.com)
 *
 * and from the Apple USB BCM5974 multitouch driver,
 * Copyright (c) 2008 Henrik Rydberg (rydberg@euromail.se)
 *
 * and from the generic hid-multitouch driver,
 * Copyright (c) 2010-2012 Stephane Chatty <chatty@enac.fr>
 *
 * 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.
 */

#include <linux/kernel.h>
#include <linux/errno.h>
#include <linux/delay.h>
#include <linux/init.h>
#include <linux/slab.h>
#include <linux/module.h>
#include <linux/completion.h>
#include <linux/uaccess.h>
#include <linux/usb.h>
#include <linux/printk.h>
#include <linux/input-polldev.h>
#include <linux/input/mt.h>
#include <linux/usb/input.h>
#include <linux/videodev2.h>
#include <media/v4l2-device.h>
#include <media/v4l2-dev.h>
#include <media/v4l2-ioctl.h>
#include <media/videobuf2-dma-contig.h>

/* read 512 bytes from endpoint 0x86 -> get header + blobs */
struct sur40_header {

	__le16 type;       /* always 0x0001 */
	__le16 count;      /* count of blobs (if 0: continue prev. packet) */

	__le32 packet_id;  /* unique ID for all packets in one frame */

	__le32 timestamp;  /* milliseconds (inc. by 16 or 17 each frame) */
	__le32 unknown;    /* "epoch?" always 02/03 00 00 00 */

} __packed;

struct sur40_blob {

	__le16 blob_id;

	u8 action;         /* 0x02 = enter/exit, 0x03 = update (?) */
	u8 unknown;        /* always 0x01 or 0x02 (no idea what this is?) */

	__le16 bb_pos_x;   /* upper left corner of bounding box */
	__le16 bb_pos_y;

	__le16 bb_size_x;  /* size of bounding box */
	__le16 bb_size_y;

	__le16 pos_x;      /* finger tip position */
	__le16 pos_y;

	__le16 ctr_x;      /* centroid position */
	__le16 ctr_y;

	__le16 axis_x;     /* somehow related to major/minor axis, mostly: */
	__le16 axis_y;     /* axis_x == bb_size_y && axis_y == bb_size_x */

	__le32 angle;      /* orientation in radians relative to x axis -
	                      actually an IEEE754 float, don't use in kernel */

	__le32 area;       /* size in pixels/pressure (?) */

	u8 padding[32];

} __packed;

/* combined header/blob data */
struct sur40_data {
	struct sur40_header header;
	struct sur40_blob   blobs[];
} __packed;

/* read 512 bytes from endpoint 0x82 -> get header below
 * continue reading 16k blocks until header.size bytes read */
struct sur40_image_header {
	__le32 magic;     /* "SUBF" */
	__le32 packet_id;
	__le32 size;      /* always 0x0007e900 = 960x540 */
	__le32 timestamp; /* milliseconds (increases by 16 or 17 each frame) */
	__le32 unknown;   /* "epoch?" always 02/03 00 00 00 */
} __packed;

/* version information */
#define DRIVER_SHORT   "sur40"
#define DRIVER_LONG    "Samsung SUR40"
#define DRIVER_AUTHOR  "Florian 'floe' Echtler <floe@butterbrot.org>"
#define DRIVER_DESC    "Surface2.0/SUR40/PixelSense input driver"

/* vendor and device IDs */
#define ID_MICROSOFT 0x045e
#define ID_SUR40     0x0775

/* sensor resolution */
#define SENSOR_RES_X 1920
#define SENSOR_RES_Y 1080

/* touch data endpoint */
#define TOUCH_ENDPOINT 0x86

/* video data endpoint */
#define VIDEO_ENDPOINT 0x82

/* video header fields */
#define VIDEO_HEADER_MAGIC 0x46425553
#define VIDEO_PACKET_SIZE  16384

/* polling interval (ms) */
#define POLL_INTERVAL 10

/* maximum number of contacts FIXME: this is a guess? */
#define MAX_CONTACTS 64

/* control commands */
#define SUR40_GET_VERSION 0xb0 /* 12 bytes string    */
#define SUR40_UNKNOWN1    0xb3 /*  5 bytes           */
#define SUR40_UNKNOWN2    0xc1 /* 24 bytes           */

#define SUR40_GET_STATE   0xc5 /*  4 bytes state (?) */
#define SUR40_GET_SENSORS 0xb1 /*  8 bytes sensors   */

/*
 * Note: an earlier, non-public version of this driver used USB_RECIP_ENDPOINT
 * here by mistake which is very likely to have corrupted the firmware EEPROM
 * on two separate SUR40 devices. Thanks to Alan Stern who spotted this bug.
 * Should you ever run into a similar problem, the background story to this
 * incident and instructions on how to fix the corrupted EEPROM are available
 * at https://floe.butterbrot.org/matrix/hacking/surface/brick.html
*/

struct sur40_state {

	struct usb_device *usbdev;
	struct device *dev;
	struct input_polled_dev *input;

	struct v4l2_device v4l2;
	struct video_device vdev;
	struct mutex lock;

	struct vb2_queue queue;
	struct vb2_alloc_ctx *alloc_ctx;
	struct list_head buf_list;
	spinlock_t qlock;
	int sequence;

	struct sur40_data *bulk_in_buffer;
	size_t bulk_in_size;
	u8 bulk_in_epaddr;

	char phys[64];
};

struct sur40_buffer {
	struct vb2_buffer vb;
	struct list_head list;
};

/* forward declarations */
static struct video_device sur40_video_device;
static struct v4l2_pix_format sur40_video_format;
static struct vb2_queue sur40_queue;

/* command wrapper */
static int sur40_command(struct sur40_state *dev,
			 u8 command, u16 index, void *buffer, u16 size)
{
	return usb_control_msg(dev->usbdev, usb_rcvctrlpipe(dev->usbdev, 0),
			       command,
			       USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
			       0x00, index, buffer, size, 1000);
}

/* Initialization routine, called from sur40_open */
static int sur40_init(struct sur40_state *dev)
{
	int result;
	u8 buffer[24];

	/* stupidly replay the original MS driver init sequence */
	result = sur40_command(dev, SUR40_GET_VERSION, 0x00, buffer, 12);
	if (result < 0)
		return result;

	result = sur40_command(dev, SUR40_GET_VERSION, 0x01, buffer, 12);
	if (result < 0)
		return result;

	result = sur40_command(dev, SUR40_GET_VERSION, 0x02, buffer, 12);
	if (result < 0)
		return result;

	result = sur40_command(dev, SUR40_UNKNOWN2,    0x00, buffer, 24);
	if (result < 0)
		return result;

	result = sur40_command(dev, SUR40_UNKNOWN1,    0x00, buffer,  5);
	if (result < 0)
		return result;

	result = sur40_command(dev, SUR40_GET_VERSION, 0x03, buffer, 12);

	/*
	 * Discard the result buffer - no known data inside except
	 * some version strings, maybe extract these sometime...
	 */

	return result;
}

/*
 * Callback routines from input_polled_dev
 */

/* Enable the device, polling will now start. */
static void sur40_open(struct input_polled_dev *polldev)
{
	struct sur40_state *sur40 = polldev->private;

	dev_dbg(sur40->dev, "open\n");
	sur40_init(sur40);
}

/* Disable device, polling has stopped. */
static void sur40_close(struct input_polled_dev *polldev)
{
	struct sur40_state *sur40 = polldev->private;

	dev_dbg(sur40->dev, "close\n");
	/*
	 * There is no known way to stop the device, so we simply
	 * stop polling.
	 */
}

/*
 * This function is called when a whole contact has been processed,
 * so that it can assign it to a slot and store the data there.
 */
static void sur40_report_blob(struct sur40_blob *blob, struct input_dev *input)
{
	int wide, major, minor;

	int bb_size_x = le16_to_cpu(blob->bb_size_x);
	int bb_size_y = le16_to_cpu(blob->bb_size_y);

	int pos_x = le16_to_cpu(blob->pos_x);
	int pos_y = le16_to_cpu(blob->pos_y);

	int ctr_x = le16_to_cpu(blob->ctr_x);
	int ctr_y = le16_to_cpu(blob->ctr_y);

	int slotnum = input_mt_get_slot_by_key(input, blob->blob_id);
	if (slotnum < 0 || slotnum >= MAX_CONTACTS)
		return;

	input_mt_slot(input, slotnum);
	input_mt_report_slot_state(input, MT_TOOL_FINGER, 1);
	wide = (bb_size_x > bb_size_y);
	major = max(bb_size_x, bb_size_y);
	minor = min(bb_size_x, bb_size_y);

	input_report_abs(input, ABS_MT_POSITION_X, pos_x);
	input_report_abs(input, ABS_MT_POSITION_Y, pos_y);
	input_report_abs(input, ABS_MT_TOOL_X, ctr_x);
	input_report_abs(input, ABS_MT_TOOL_Y, ctr_y);

	/* TODO: use a better orientation measure */
	input_report_abs(input, ABS_MT_ORIENTATION, wide);
	input_report_abs(input, ABS_MT_TOUCH_MAJOR, major);
	input_report_abs(input, ABS_MT_TOUCH_MINOR, minor);
}

/* core function: poll for new input data */
static void sur40_poll(struct input_polled_dev *polldev)
{
	struct sur40_buffer *new_buf;
	uint8_t *buffer;

	struct sur40_state *sur40 = polldev->private;
	struct input_dev *input = polldev->input;
	int result, bulk_read, need_blobs, packet_blobs, i, bufpos;
	u32 uninitialized_var(packet_id);

	struct sur40_header *header = &sur40->bulk_in_buffer->header;
	struct sur40_blob *inblob = &sur40->bulk_in_buffer->blobs[0];
	struct sur40_image_header *img = (void*)(sur40->bulk_in_buffer);

	dev_dbg(sur40->dev, "poll\n");

	need_blobs = -1;

	do {

		/* perform a blocking bulk read to get data from the device */
		result = usb_bulk_msg(sur40->usbdev,
			usb_rcvbulkpipe(sur40->usbdev, sur40->bulk_in_epaddr),
			sur40->bulk_in_buffer, sur40->bulk_in_size,
			&bulk_read, 1000);

		dev_dbg(sur40->dev, "received %d bytes\n", bulk_read);

		if (result < 0) {
			dev_err(sur40->dev, "error in usb_bulk_read\n");
			return;
		}

		result = bulk_read - sizeof(struct sur40_header);

		if (result % sizeof(struct sur40_blob) != 0) {
			dev_err(sur40->dev, "transfer size mismatch\n");
			return;
		}

		/* first packet? */
		if (need_blobs == -1) {
			need_blobs = le16_to_cpu(header->count);
			dev_dbg(sur40->dev, "need %d blobs\n", need_blobs);
			packet_id = le32_to_cpu(header->packet_id);
		}

		/*
		 * Sanity check. when video data is also being retrieved, the
		 * packet ID will usually increase in the middle of a series
		 * instead of at the end.
		 */
		if (packet_id != header->packet_id)
			dev_warn(sur40->dev, "packet ID mismatch\n");

		packet_blobs = result / sizeof(struct sur40_blob);
		dev_dbg(sur40->dev, "received %d blobs\n", packet_blobs);

		/* packets always contain at least 4 blobs, even if empty */
		if (packet_blobs > need_blobs)
			packet_blobs = need_blobs;

		for (i = 0; i < packet_blobs; i++) {
			need_blobs--;
			dev_dbg(sur40->dev, "processing blob\n");
			sur40_report_blob(&(inblob[i]), input);
		}

	} while (need_blobs > 0);

	input_mt_sync_frame(input);
	input_sync(input);

	/* deal with video data here - should be moved to own method */
	if (list_empty(&sur40->buf_list))
		return;

	/* get a new buffer from the list */
	spin_lock(&sur40->qlock);
	new_buf = list_entry(sur40->buf_list.next, struct sur40_buffer, list);
	list_del(&new_buf->list);
	spin_unlock(&sur40->qlock);

	/* retrieve data via bulk read */
	bufpos = 0;

	result = usb_bulk_msg(sur40->usbdev,
			usb_rcvbulkpipe(sur40->usbdev, VIDEO_ENDPOINT), // sur40->bulk_in_epaddr),
			sur40->bulk_in_buffer, sur40->bulk_in_size,
			&bulk_read, 1000);
	if (result < 0) { dev_err(sur40->dev,"error in usb_bulk_read\n"); goto err_poll; }
	if (bulk_read != sizeof(struct sur40_image_header)) {
		dev_err(sur40->dev, "received %d bytes (%ld expected)\n", bulk_read, sizeof(struct sur40_image_header));
		goto err_poll;
	}

	if (img->magic != VIDEO_HEADER_MAGIC) { dev_err(sur40->dev,"image magic mismatch\n"); goto err_poll; }
	if (img->size  != sur40_video_format.sizeimage) { dev_err(sur40->dev,"image size  mismatch\n"); goto err_poll; }

	buffer = vb2_plane_vaddr(&new_buf->vb, 0);
	while (bufpos < sur40_video_format.sizeimage) {
		result = usb_bulk_msg(sur40->usbdev,
			usb_rcvbulkpipe(sur40->usbdev, VIDEO_ENDPOINT), // sur40->bulk_in_epaddr),
			buffer+bufpos, VIDEO_PACKET_SIZE,
			&bulk_read, 1000);
		if (result < 0) { dev_err(sur40->dev,"error in usb_bulk_read\n"); goto err_poll; }
		bufpos += bulk_read;
	}

	/* mark as finished */
	vb2_set_plane_payload(&new_buf->vb, 0, sur40_video_format.sizeimage);
	v4l2_get_timestamp(&new_buf->vb.v4l2_buf.timestamp);
	new_buf->vb.v4l2_buf.sequence = sur40->sequence++;
	vb2_buffer_done(&new_buf->vb, VB2_BUF_STATE_DONE);
	return;

err_poll:
	vb2_buffer_done(&new_buf->vb, VB2_BUF_STATE_ERROR);
}

/* Initialize input device parameters. */
static void sur40_input_setup(struct input_dev *input_dev)
{
	__set_bit(EV_KEY, input_dev->evbit);
	__set_bit(EV_ABS, input_dev->evbit);

	input_set_abs_params(input_dev, ABS_MT_POSITION_X,
			     0, SENSOR_RES_X, 0, 0);
	input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
			     0, SENSOR_RES_Y, 0, 0);

	input_set_abs_params(input_dev, ABS_MT_TOOL_X,
			     0, SENSOR_RES_X, 0, 0);
	input_set_abs_params(input_dev, ABS_MT_TOOL_Y,
			     0, SENSOR_RES_Y, 0, 0);

	/* max value unknown, but major/minor axis
	 * can never be larger than screen */
	input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
			     0, SENSOR_RES_X, 0, 0);
	input_set_abs_params(input_dev, ABS_MT_TOUCH_MINOR,
			     0, SENSOR_RES_Y, 0, 0);

	input_set_abs_params(input_dev, ABS_MT_ORIENTATION, 0, 1, 0, 0);

	input_mt_init_slots(input_dev, MAX_CONTACTS,
			    INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
}

/* Check candidate USB interface. */
static int sur40_probe(struct usb_interface *interface,
		       const struct usb_device_id *id)
{
	struct usb_device *usbdev = interface_to_usbdev(interface);
	struct sur40_state *sur40;
	struct usb_host_interface *iface_desc;
	struct usb_endpoint_descriptor *endpoint;
	struct input_polled_dev *poll_dev;
	int error;

	/* Check if we really have the right interface. */
	iface_desc = &interface->altsetting[0];
	if (iface_desc->desc.bInterfaceClass != 0xFF)
		return -ENODEV;

	/* Use endpoint #4 (0x86). */
	endpoint = &iface_desc->endpoint[4].desc;
	if (endpoint->bEndpointAddress != TOUCH_ENDPOINT)
		return -ENODEV;

	/* Allocate memory for our device state and initialize it. */
	sur40 = kzalloc(sizeof(struct sur40_state), GFP_KERNEL);
	if (!sur40)
		return -ENOMEM;

	poll_dev = input_allocate_polled_device();
	if (!poll_dev) {
		error = -ENOMEM;
		goto err_free_dev;
	}

	/* initialize locks/lists */
	INIT_LIST_HEAD(&sur40->buf_list);
	spin_lock_init(&sur40->qlock);
	mutex_init(&sur40->lock);

	/* Set up polled input device control structure */
	poll_dev->private = sur40;
	poll_dev->poll_interval = POLL_INTERVAL;
	poll_dev->open = sur40_open;
	poll_dev->poll = sur40_poll;
	poll_dev->close = sur40_close;

	/* Set up regular input device structure */
	sur40_input_setup(poll_dev->input);

	poll_dev->input->name = DRIVER_LONG;
	usb_to_input_id(usbdev, &poll_dev->input->id);
	usb_make_path(usbdev, sur40->phys, sizeof(sur40->phys));
	strlcat(sur40->phys, "/input0", sizeof(sur40->phys));
	poll_dev->input->phys = sur40->phys;
	poll_dev->input->dev.parent = &interface->dev;

	sur40->usbdev = usbdev;
	sur40->dev = &interface->dev;
	sur40->input = poll_dev;

	/* use the bulk-in endpoint tested above */
	sur40->bulk_in_size = usb_endpoint_maxp(endpoint);
	sur40->bulk_in_epaddr = endpoint->bEndpointAddress;
	sur40->bulk_in_buffer = kmalloc(sur40->bulk_in_size, GFP_KERNEL);
	if (!sur40->bulk_in_buffer) {
		dev_err(&interface->dev, "Unable to allocate input buffer.");
		error = -ENOMEM;
		goto err_free_polldev;
	}

	/* register the polled input device */
	error = input_register_polled_device(poll_dev);
	if (error) {
		dev_err(&interface->dev,
			"Unable to register polled input device.");
		goto err_free_buffer;
	}

	/* register the video master device */
	snprintf(sur40->v4l2.name, sizeof(sur40->v4l2.name), "%s", DRIVER_LONG);
	error = v4l2_device_register(sur40->dev, &sur40->v4l2);
	if (error) {
		dev_err(&interface->dev,
			"Unable to register video device.");
		goto err_free_buffer;
	}

	/* initialize the lock and subdevice */
	sur40->queue = sur40_queue;
	sur40->queue.drv_priv = sur40;
	sur40->queue.lock = &sur40->lock;

	// init the queue
	error = vb2_queue_init(&sur40->queue);
	if (error)
		goto err_free_buffer;

	sur40->alloc_ctx = vb2_dma_contig_init_ctx(sur40->dev);
	if (IS_ERR(sur40->alloc_ctx)) {
		dev_err(sur40->dev, "Can't allocate buffer context");
		goto err_free_buffer;
	}

	sur40->vdev = sur40_video_device;
	sur40->vdev.v4l2_dev = &sur40->v4l2;
	sur40->vdev.lock = &sur40->lock;
	sur40->vdev.queue = &sur40->queue;
	video_set_drvdata(&sur40->vdev, sur40);

	error = video_register_device(&sur40->vdev, VFL_TYPE_GRABBER, -1);
	if (error)
		goto err_free_buffer;

	/* we can register the device now, as it is ready */
	usb_set_intfdata(interface, sur40);
	dev_dbg(&interface->dev, "%s is now attached\n", DRIVER_DESC);

	return 0;

/*v4l2_device_unregister(&sur40->v4l2);
video_unregister_device(&sur40->vdev);
vb2_dma_contig_cleanup_ctx(sur40->alloc_ctx);*/

err_free_buffer:
	kfree(sur40->bulk_in_buffer);
err_free_polldev:
	input_free_polled_device(sur40->input);
err_free_dev:
	kfree(sur40);

	return error;
}

/* Unregister device & clean up. */
static void sur40_disconnect(struct usb_interface *interface)
{
	struct sur40_state *sur40 = usb_get_intfdata(interface);

	v4l2_device_unregister(&sur40->v4l2);
	video_unregister_device(&sur40->vdev);
	vb2_dma_contig_cleanup_ctx(sur40->alloc_ctx);

	input_unregister_polled_device(sur40->input);
	input_free_polled_device(sur40->input);
	kfree(sur40->bulk_in_buffer);
	kfree(sur40);

	usb_set_intfdata(interface, NULL);
	dev_dbg(&interface->dev, "%s is now disconnected\n", DRIVER_DESC);
}

/*
 * Setup the constraints of the queue: besides setting the number of planes
 * per buffer and the size and allocation context of each plane, it also
 * checks if sufficient buffers have been allocated. Usually 3 is a good
 * minimum number: many DMA engines need a minimum of 2 buffers in the
 * queue and you need to have another available for userspace processing.
 */
static int sur40_queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
		       unsigned int *nbuffers, unsigned int *nplanes,
		       unsigned int sizes[], void *alloc_ctxs[])
{
	struct sur40_state *sur40 = vb2_get_drv_priv(vq);

	if (vq->num_buffers + *nbuffers < 3)
		*nbuffers = 3 - vq->num_buffers;

	if (fmt && fmt->fmt.pix.sizeimage < sur40_video_format.sizeimage)
		return -EINVAL;

	*nplanes = 1;
	sizes[0] = fmt ? fmt->fmt.pix.sizeimage : sur40_video_format.sizeimage;
	alloc_ctxs[0] = sur40->alloc_ctx;

	return 0;
}

/*
 * Prepare the buffer for queueing to the DMA engine: check and set the
 * payload size.
 */
static int sur40_buffer_prepare(struct vb2_buffer *vb)
{
	struct sur40_state *sur40 = vb2_get_drv_priv(vb->vb2_queue);
	unsigned long size = sur40_video_format.sizeimage;

	if (vb2_plane_size(vb, 0) < size) {
		dev_err(&sur40->usbdev->dev, "buffer too small (%lu < %lu)\n",
			 vb2_plane_size(vb, 0), size);
		return -EINVAL;
	}

	vb2_set_plane_payload(vb, 0, size);
	return 0;
}

/*
 * Queue this buffer to the DMA engine.
 */
static void sur40_buffer_queue(struct vb2_buffer *vb)
{
	struct sur40_state *sur40 = vb2_get_drv_priv(vb->vb2_queue);
	struct sur40_buffer *buf = (struct sur40_buffer*)(vb);

	spin_lock(&sur40->qlock);
	list_add_tail(&buf->list, &sur40->buf_list);
	spin_unlock(&sur40->qlock);
}

static void return_all_buffers(struct sur40_state *sur40,
			       enum vb2_buffer_state state)
{
	struct sur40_buffer *buf, *node;

	spin_lock(&sur40->qlock);
	list_for_each_entry_safe(buf, node, &sur40->buf_list, list) {
		vb2_buffer_done(&buf->vb, state);
		list_del(&buf->list);
	}
	spin_unlock(&sur40->qlock);
}

/*
 * Start streaming. First check if the minimum number of buffers have been
 * queued. If not, then return -ENOBUFS and the vb2 framework will call
 * this function again the next time a buffer has been queued until enough
 * buffers are available to actually start the DMA engine.
 */
static int sur40_start_streaming(struct vb2_queue *vq, unsigned int count)
{
	struct sur40_state *sur40 = vb2_get_drv_priv(vq);
	int ret = 0;

	sur40->sequence = 0;

	/* TODO: start DMA */

	if (ret) {
		/*
		 * In case of an error, return all active buffers to the
		 * QUEUED state
		 */
		return_all_buffers(sur40, VB2_BUF_STATE_QUEUED);
	}
	return ret;
}

/*
 * Stop the DMA engine. Any remaining buffers in the DMA queue are dequeued
 * and passed on to the vb2 framework marked as STATE_ERROR.
 */
static void sur40_stop_streaming(struct vb2_queue *vq)
{
	struct sur40_state *sur40 = vb2_get_drv_priv(vq);

	/* TODO: stop DMA */

	/* Release all active buffers */
	return_all_buffers(sur40, VB2_BUF_STATE_ERROR);
}

/* V4L ioctl */
static int sur40_vidioc_querycap(struct file *file, void *priv,
				 struct v4l2_capability *cap)
{
	struct sur40_state *sur40 = video_drvdata(file);

	strlcpy(cap->driver, DRIVER_SHORT, sizeof(cap->driver));
	strlcpy(cap->card, DRIVER_LONG, sizeof(cap->card));
	snprintf(cap->bus_info, sizeof(cap->bus_info), "USB:%s",
		 sur40->usbdev->devpath);
	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE |
	                   V4L2_CAP_READWRITE |
	                   V4L2_CAP_STREAMING;
	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
	return 0;
}

static int sur40_vidioc_enum_input(struct file *file, void *priv,
				   struct v4l2_input *i)
{
	if (i->index != 0)
		return -EINVAL;
	i->type = V4L2_INPUT_TYPE_CAMERA;
	i->std = V4L2_STD_UNKNOWN;
	strlcpy(i->name, "In-Cell Sensor", sizeof(i->name));
	i->capabilities = 0;
	return 0;
}

static int sur40_vidioc_s_input(struct file *file, void *priv, unsigned int i)
{
	return (i != 0);
}

static int sur40_vidioc_g_input(struct file *file, void *priv, unsigned int *i)
{
	*i = 0;
	return 0;
}

static int sur40_vidioc_fmt(struct file *file, void *priv,
			    struct v4l2_format *f)
{
	f->fmt.pix = sur40_video_format;
	return 0;
}

static int sur40_vidioc_enum_fmt(struct file *file, void *priv,
				 struct v4l2_fmtdesc *f)
{
	if (f->index != 0)
		return -EINVAL;
	strlcpy(f->description, "8-bit greyscale", sizeof(f->description));
	f->pixelformat = V4L2_PIX_FMT_GREY;
	f->flags = 0;
	return 0;
}

static const struct usb_device_id sur40_table[] = {
	{ USB_DEVICE(ID_MICROSOFT, ID_SUR40) },  /* Samsung SUR40 */
	{ }                                      /* terminating null entry */
};
MODULE_DEVICE_TABLE(usb, sur40_table);

/* V4L2 structures */
static struct vb2_ops sur40_queue_ops = {
	.queue_setup		= sur40_queue_setup,
	.buf_prepare		= sur40_buffer_prepare,
	.buf_queue		= sur40_buffer_queue,
	.start_streaming	= sur40_start_streaming,
	.stop_streaming		= sur40_stop_streaming,
	.wait_prepare		= vb2_ops_wait_prepare,
	.wait_finish		= vb2_ops_wait_finish,
};

static struct vb2_queue sur40_queue = {
	.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
	.io_modes = VB2_MMAP | VB2_DMABUF | VB2_READ,
	.buf_struct_size = sizeof(struct sur40_buffer),
	.ops = &sur40_queue_ops,
	.mem_ops = &vb2_dma_contig_memops,
	.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC,
	.min_buffers_needed = 3,
	.gfp_flags = GFP_DMA,
};

static const struct v4l2_file_operations sur40_video_fops = {
	.owner = THIS_MODULE,
	.open = v4l2_fh_open,
	.release = vb2_fop_release,
	.unlocked_ioctl = video_ioctl2,
	.read = vb2_fop_read,
	.mmap = vb2_fop_mmap,
	.poll = vb2_fop_poll,
};

static const struct v4l2_ioctl_ops sur40_video_ioctl_ops = {

	.vidioc_querycap	= sur40_vidioc_querycap,

	.vidioc_enum_fmt_vid_cap = sur40_vidioc_enum_fmt,
	.vidioc_try_fmt_vid_cap	= sur40_vidioc_fmt,
	.vidioc_s_fmt_vid_cap	= sur40_vidioc_fmt,
	.vidioc_g_fmt_vid_cap	= sur40_vidioc_fmt,

	.vidioc_enum_input	= sur40_vidioc_enum_input,
	.vidioc_g_input		= sur40_vidioc_g_input,
	.vidioc_s_input		= sur40_vidioc_s_input,

	.vidioc_reqbufs 	= vb2_ioctl_reqbufs,
	.vidioc_create_bufs 	= vb2_ioctl_create_bufs,
	.vidioc_querybuf 	= vb2_ioctl_querybuf,
	.vidioc_qbuf 		= vb2_ioctl_qbuf,
	.vidioc_dqbuf 		= vb2_ioctl_dqbuf,
	.vidioc_expbuf 		= vb2_ioctl_expbuf,

	.vidioc_streamon 	= vb2_ioctl_streamon,
	.vidioc_streamoff 	= vb2_ioctl_streamoff,

	/*.vidioc_log_status      = v4l2_ctrl_log_status,
	.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,*/
};

static struct video_device sur40_video_device = {
	.name = DRIVER_LONG,
	.fops = &sur40_video_fops,
	.ioctl_ops = &sur40_video_ioctl_ops,
	.release = video_device_release_empty,
};

static struct v4l2_pix_format sur40_video_format = {
	.pixelformat = V4L2_PIX_FMT_GREY,
	.width  = SENSOR_RES_X / 2,
	.height = SENSOR_RES_Y / 2,
	.field = V4L2_FIELD_NONE,
	//.colorspace = V4L2_COLORSPACE_..., FIXME is this required?
	.bytesperline = SENSOR_RES_X / 2,
	.sizeimage = (SENSOR_RES_X/2) * (SENSOR_RES_Y/2),
	.priv = 0,
};

/* USB-specific object needed to register this driver with the USB subsystem. */
static struct usb_driver sur40_driver = {
	.name = DRIVER_SHORT,
	.probe = sur40_probe,
	.disconnect = sur40_disconnect,
	.id_table = sur40_table,
};

module_usb_driver(sur40_driver);

MODULE_AUTHOR(DRIVER_AUTHOR);
MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_LICENSE("GPL");

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ 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