All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/9] Output raw touch data via V4L2
@ 2016-06-17 14:16 Nick Dyer
  2016-06-17 14:16 ` [PATCH v4 1/9] [media] Add signed 16-bit pixel format Nick Dyer
                   ` (8 more replies)
  0 siblings, 9 replies; 22+ messages in thread
From: Nick Dyer @ 2016-06-17 14:16 UTC (permalink / raw)
  To: Dmitry Torokhov, Hans Verkuil
  Cc: linux-input, linux-kernel, linux-media, Benjamin Tissoires,
	Benson Leung, Alan Bowens, Javier Martinez Canillas, Chris Healy,
	Henrik Rydberg, Andrew Duggan, James Chen, Dudley Du,
	Andrew de los Reyes, sheckylin, Peter Hutterer, Florian Echtler,
	mchehab

This is a series of patches to add output of raw touch diagnostic data via V4L2
to the Atmel maXTouch and Synaptics RMI4 drivers.

It's a rewrite of the previous implementation which output via debugfs: it now
uses a V4L2 device in a similar way to the sur40 driver.

We have a utility which can read the data and display it in a useful format:
    https://github.com/ndyer/heatmap/commits/heatmap-v4l

These patches are also available from
    https://github.com/ndyer/linux/commits/v4l-touch-2016-06-17

Changes in v4:
- Address nits from the input side in atmel_mxt_ts patches (Dmitry Torokhov)
- Add Synaptics RMI4 F54 support patch

Changes in v3:
- Address V4L2 review comments from Hans Verkuil
- Run v4l-compliance and fix all issues - needs minor patch here:
  https://github.com/ndyer/v4l-utils/commit/cf50469773f

Changes in v2:
- Split pixfmt changes into separate commit and add DocBook
- Introduce VFL_TYPE_TOUCH_SENSOR and /dev/v4l-touch
- Remove "single node" support for now, it may be better to treat it as metadata later
- Explicitly set VFL_DIR_RX
- Fix Kconfig

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v4 1/9] [media] Add signed 16-bit pixel format
  2016-06-17 14:16 [PATCH v4 0/9] Output raw touch data via V4L2 Nick Dyer
@ 2016-06-17 14:16 ` Nick Dyer
  2016-06-20 11:00   ` Hans Verkuil
  2016-06-17 14:16 ` [PATCH v4 2/9] [media] v4l2-core: Add VFL_TYPE_TOUCH_SENSOR Nick Dyer
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Nick Dyer @ 2016-06-17 14:16 UTC (permalink / raw)
  To: Dmitry Torokhov, Hans Verkuil
  Cc: linux-input, linux-kernel, linux-media, Benjamin Tissoires,
	Benson Leung, Alan Bowens, Javier Martinez Canillas, Chris Healy,
	Henrik Rydberg, Andrew Duggan, James Chen, Dudley Du,
	Andrew de los Reyes, sheckylin, Peter Hutterer, Florian Echtler,
	mchehab, Nick Dyer

This will be used for output of raw touch delta data. This format is
used by Atmel maXTouch (atmel_mxt_ts) and also Synaptics RMI4.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 Documentation/DocBook/media/v4l/pixfmt-ys16.xml | 79 +++++++++++++++++++++++++
 Documentation/DocBook/media/v4l/pixfmt.xml      |  1 +
 drivers/media/v4l2-core/v4l2-ioctl.c            |  1 +
 include/uapi/linux/videodev2.h                  |  1 +
 4 files changed, 82 insertions(+)
 create mode 100644 Documentation/DocBook/media/v4l/pixfmt-ys16.xml

diff --git a/Documentation/DocBook/media/v4l/pixfmt-ys16.xml b/Documentation/DocBook/media/v4l/pixfmt-ys16.xml
new file mode 100644
index 0000000..f92d65e
--- /dev/null
+++ b/Documentation/DocBook/media/v4l/pixfmt-ys16.xml
@@ -0,0 +1,79 @@
+<refentry id="V4L2-PIX-FMT-YS16">
+  <refmeta>
+    <refentrytitle>V4L2_PIX_FMT_YS16 ('YS16')</refentrytitle>
+    &manvol;
+  </refmeta>
+  <refnamediv>
+    <refname><constant>V4L2_PIX_FMT_YS16</constant></refname>
+    <refpurpose>Grey-scale image</refpurpose>
+  </refnamediv>
+  <refsect1>
+    <title>Description</title>
+
+    <para>This is a signed grey-scale image with a depth of 16 bits per
+pixel. The most significant byte is stored at higher memory addresses
+(little-endian).</para>
+
+    <example>
+      <title><constant>V4L2_PIX_FMT_YS16</constant> 4 &times; 4
+pixel image</title>
+
+      <formalpara>
+	<title>Byte Order.</title>
+	<para>Each cell is one byte.
+	  <informaltable frame="none">
+	    <tgroup cols="9" align="center">
+	      <colspec align="left" colwidth="2*" />
+	      <tbody valign="top">
+		<row>
+		  <entry>start&nbsp;+&nbsp;0:</entry>
+		  <entry>Y'<subscript>00low</subscript></entry>
+		  <entry>Y'<subscript>00high</subscript></entry>
+		  <entry>Y'<subscript>01low</subscript></entry>
+		  <entry>Y'<subscript>01high</subscript></entry>
+		  <entry>Y'<subscript>02low</subscript></entry>
+		  <entry>Y'<subscript>02high</subscript></entry>
+		  <entry>Y'<subscript>03low</subscript></entry>
+		  <entry>Y'<subscript>03high</subscript></entry>
+		</row>
+		<row>
+		  <entry>start&nbsp;+&nbsp;8:</entry>
+		  <entry>Y'<subscript>10low</subscript></entry>
+		  <entry>Y'<subscript>10high</subscript></entry>
+		  <entry>Y'<subscript>11low</subscript></entry>
+		  <entry>Y'<subscript>11high</subscript></entry>
+		  <entry>Y'<subscript>12low</subscript></entry>
+		  <entry>Y'<subscript>12high</subscript></entry>
+		  <entry>Y'<subscript>13low</subscript></entry>
+		  <entry>Y'<subscript>13high</subscript></entry>
+		</row>
+		<row>
+		  <entry>start&nbsp;+&nbsp;16:</entry>
+		  <entry>Y'<subscript>20low</subscript></entry>
+		  <entry>Y'<subscript>20high</subscript></entry>
+		  <entry>Y'<subscript>21low</subscript></entry>
+		  <entry>Y'<subscript>21high</subscript></entry>
+		  <entry>Y'<subscript>22low</subscript></entry>
+		  <entry>Y'<subscript>22high</subscript></entry>
+		  <entry>Y'<subscript>23low</subscript></entry>
+		  <entry>Y'<subscript>23high</subscript></entry>
+		</row>
+		<row>
+		  <entry>start&nbsp;+&nbsp;24:</entry>
+		  <entry>Y'<subscript>30low</subscript></entry>
+		  <entry>Y'<subscript>30high</subscript></entry>
+		  <entry>Y'<subscript>31low</subscript></entry>
+		  <entry>Y'<subscript>31high</subscript></entry>
+		  <entry>Y'<subscript>32low</subscript></entry>
+		  <entry>Y'<subscript>32high</subscript></entry>
+		  <entry>Y'<subscript>33low</subscript></entry>
+		  <entry>Y'<subscript>33high</subscript></entry>
+		</row>
+	      </tbody>
+	    </tgroup>
+	  </informaltable>
+	</para>
+      </formalpara>
+    </example>
+  </refsect1>
+</refentry>
diff --git a/Documentation/DocBook/media/v4l/pixfmt.xml b/Documentation/DocBook/media/v4l/pixfmt.xml
index 5a08aee..f3e3e6d 100644
--- a/Documentation/DocBook/media/v4l/pixfmt.xml
+++ b/Documentation/DocBook/media/v4l/pixfmt.xml
@@ -1619,6 +1619,7 @@ information.</para>
     &sub-y12;
     &sub-y10b;
     &sub-y16;
+    &sub-ys16;
     &sub-y16-be;
     &sub-y8i;
     &sub-y12i;
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 28e5be2..ecf7e0b 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1164,6 +1164,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 	case V4L2_PIX_FMT_Y10:		descr = "10-bit Greyscale"; break;
 	case V4L2_PIX_FMT_Y12:		descr = "12-bit Greyscale"; break;
 	case V4L2_PIX_FMT_Y16:		descr = "16-bit Greyscale"; break;
+	case V4L2_PIX_FMT_YS16:		descr = "16-bit Greyscale (Signed)"; break;
 	case V4L2_PIX_FMT_Y16_BE:	descr = "16-bit Greyscale BE"; break;
 	case V4L2_PIX_FMT_Y10BPACK:	descr = "10-bit Greyscale (Packed)"; break;
 	case V4L2_PIX_FMT_PAL8:		descr = "8-bit Palette"; break;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 8f95191..e0125cf 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -493,6 +493,7 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_Y12     v4l2_fourcc('Y', '1', '2', ' ') /* 12  Greyscale     */
 #define V4L2_PIX_FMT_Y16     v4l2_fourcc('Y', '1', '6', ' ') /* 16  Greyscale     */
 #define V4L2_PIX_FMT_Y16_BE  v4l2_fourcc_be('Y', '1', '6', ' ') /* 16  Greyscale BE  */
+#define V4L2_PIX_FMT_YS16    v4l2_fourcc('Y', 'S', '1', '6') /* signed 16-bit Greyscale */
 
 /* Grey bit-packed formats */
 #define V4L2_PIX_FMT_Y10BPACK    v4l2_fourcc('Y', '1', '0', 'B') /* 10  Greyscale bit-packed */
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v4 2/9] [media] v4l2-core: Add VFL_TYPE_TOUCH_SENSOR
  2016-06-17 14:16 [PATCH v4 0/9] Output raw touch data via V4L2 Nick Dyer
  2016-06-17 14:16 ` [PATCH v4 1/9] [media] Add signed 16-bit pixel format Nick Dyer
@ 2016-06-17 14:16 ` Nick Dyer
  2016-06-20 12:00   ` Hans Verkuil
  2016-06-17 14:16 ` [PATCH v4 3/9] Input: atmel_mxt_ts - add support for T37 diagnostic data Nick Dyer
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Nick Dyer @ 2016-06-17 14:16 UTC (permalink / raw)
  To: Dmitry Torokhov, Hans Verkuil
  Cc: linux-input, linux-kernel, linux-media, Benjamin Tissoires,
	Benson Leung, Alan Bowens, Javier Martinez Canillas, Chris Healy,
	Henrik Rydberg, Andrew Duggan, James Chen, Dudley Du,
	Andrew de los Reyes, sheckylin, Peter Hutterer, Florian Echtler,
	mchehab, Nick Dyer

Some touch controllers send out raw touch data in a similar way to a
greyscale frame grabber. Add a new device type for these devices.

Use a new device prefix v4l-touch for these devices, to stop generic
capture software from treating them as webcams.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 drivers/input/touchscreen/sur40.c    |  4 ++--
 drivers/media/v4l2-core/v4l2-dev.c   | 13 ++++++++++---
 drivers/media/v4l2-core/v4l2-ioctl.c | 15 ++++++++++-----
 include/media/v4l2-dev.h             |  3 ++-
 include/uapi/linux/videodev2.h       |  1 +
 5 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 880c40b..2daa786 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -599,7 +599,7 @@ static int sur40_probe(struct usb_interface *interface,
 	sur40->vdev.queue = &sur40->queue;
 	video_set_drvdata(&sur40->vdev, sur40);
 
-	error = video_register_device(&sur40->vdev, VFL_TYPE_GRABBER, -1);
+	error = video_register_device(&sur40->vdev, VFL_TYPE_TOUCH_SENSOR, -1);
 	if (error) {
 		dev_err(&interface->dev,
 			"Unable to register video subdevice.");
@@ -763,7 +763,7 @@ static int sur40_vidioc_enum_input(struct file *file, void *priv,
 {
 	if (i->index != 0)
 		return -EINVAL;
-	i->type = V4L2_INPUT_TYPE_CAMERA;
+	i->type = V4L2_INPUT_TYPE_TOUCH_SENSOR;
 	i->std = V4L2_STD_UNKNOWN;
 	strlcpy(i->name, "In-Cell Sensor", sizeof(i->name));
 	i->capabilities = 0;
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 70b559d..7068e12 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -529,6 +529,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
 	bool is_sdr = vdev->vfl_type == VFL_TYPE_SDR;
 	bool is_rx = vdev->vfl_dir != VFL_DIR_TX;
 	bool is_tx = vdev->vfl_dir != VFL_DIR_RX;
+	bool is_touch = vdev->vfl_type == VFL_TYPE_TOUCH_SENSOR;
 
 	bitmap_zero(valid_ioctls, BASE_VIDIOC_PRIVATE);
 
@@ -573,7 +574,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
 	if (ops->vidioc_enum_freq_bands || ops->vidioc_g_tuner || ops->vidioc_g_modulator)
 		set_bit(_IOC_NR(VIDIOC_ENUM_FREQ_BANDS), valid_ioctls);
 
-	if (is_vid) {
+	if (is_vid || is_touch) {
 		/* video specific ioctls */
 		if ((is_rx && (ops->vidioc_enum_fmt_vid_cap ||
 			       ops->vidioc_enum_fmt_vid_cap_mplane ||
@@ -662,7 +663,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
 			set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
 	}
 
-	if (is_vid || is_vbi || is_sdr) {
+	if (is_vid || is_vbi || is_sdr || is_touch) {
 		/* ioctls valid for video, vbi or sdr */
 		SET_VALID_IOCTL(ops, VIDIOC_REQBUFS, vidioc_reqbufs);
 		SET_VALID_IOCTL(ops, VIDIOC_QUERYBUF, vidioc_querybuf);
@@ -675,7 +676,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
 		SET_VALID_IOCTL(ops, VIDIOC_STREAMOFF, vidioc_streamoff);
 	}
 
-	if (is_vid || is_vbi) {
+	if (is_vid || is_vbi || is_touch) {
 		/* ioctls valid for video or vbi */
 		if (ops->vidioc_s_std)
 			set_bit(_IOC_NR(VIDIOC_ENUMSTD), valid_ioctls);
@@ -739,6 +740,7 @@ static int video_register_media_controller(struct video_device *vdev, int type)
 	vdev->entity.function = MEDIA_ENT_F_UNKNOWN;
 
 	switch (type) {
+	case VFL_TYPE_TOUCH_SENSOR:
 	case VFL_TYPE_GRABBER:
 		intf_type = MEDIA_INTF_T_V4L_VIDEO;
 		vdev->entity.function = MEDIA_ENT_F_IO_V4L;
@@ -845,6 +847,8 @@ static int video_register_media_controller(struct video_device *vdev, int type)
  *	%VFL_TYPE_SUBDEV - A subdevice
  *
  *	%VFL_TYPE_SDR - Software Defined Radio
+ *
+ *	%VFL_TYPE_TOUCH_SENSOR - A touch sensor
  */
 int __video_register_device(struct video_device *vdev, int type, int nr,
 		int warn_if_nr_in_use, struct module *owner)
@@ -888,6 +892,9 @@ int __video_register_device(struct video_device *vdev, int type, int nr,
 		/* Use device name 'swradio' because 'sdr' was already taken. */
 		name_base = "swradio";
 		break;
+	case VFL_TYPE_TOUCH_SENSOR:
+		name_base = "v4l-touch";
+		break;
 	default:
 		printk(KERN_ERR "%s called with unknown type: %d\n",
 		       __func__, type);
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index ecf7e0b..352c4d0 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -924,6 +924,7 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
 	bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER;
 	bool is_vbi = vfd->vfl_type == VFL_TYPE_VBI;
 	bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
+	bool is_touch = vfd->vfl_type == VFL_TYPE_TOUCH_SENSOR;
 	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
 	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
 
@@ -932,7 +933,7 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
 
 	switch (type) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
-		if (is_vid && is_rx &&
+		if ((is_vid || is_touch) && is_rx &&
 		    (ops->vidioc_g_fmt_vid_cap || ops->vidioc_g_fmt_vid_cap_mplane))
 			return 0;
 		break;
@@ -1310,13 +1311,14 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
 	struct video_device *vfd = video_devdata(file);
 	bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER;
 	bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
+	bool is_touch = vfd->vfl_type == VFL_TYPE_TOUCH_SENSOR;
 	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
 	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
 	int ret = -EINVAL;
 
 	switch (p->type) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
-		if (unlikely(!is_rx || !is_vid || !ops->vidioc_enum_fmt_vid_cap))
+		if (unlikely(!is_rx || (!is_vid && !is_touch) || !ops->vidioc_enum_fmt_vid_cap))
 			break;
 		ret = ops->vidioc_enum_fmt_vid_cap(file, fh, arg);
 		break;
@@ -1363,6 +1365,7 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
 	struct video_device *vfd = video_devdata(file);
 	bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER;
 	bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
+	bool is_touch = vfd->vfl_type == VFL_TYPE_TOUCH_SENSOR;
 	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
 	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
 	int ret;
@@ -1393,7 +1396,7 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
 
 	switch (p->type) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
-		if (unlikely(!is_rx || !is_vid || !ops->vidioc_g_fmt_vid_cap))
+		if (unlikely(!is_rx || (!is_vid && !is_touch) || !ops->vidioc_g_fmt_vid_cap))
 			break;
 		p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
 		ret = ops->vidioc_g_fmt_vid_cap(file, fh, arg);
@@ -1459,6 +1462,7 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
 	struct video_device *vfd = video_devdata(file);
 	bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER;
 	bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
+	bool is_touch = vfd->vfl_type == VFL_TYPE_TOUCH_SENSOR;
 	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
 	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
 	int ret;
@@ -1470,7 +1474,7 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
 
 	switch (p->type) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
-		if (unlikely(!is_rx || !is_vid || !ops->vidioc_s_fmt_vid_cap))
+		if (unlikely(!is_rx || (!is_vid && !is_touch) || !ops->vidioc_s_fmt_vid_cap))
 			break;
 		CLEAR_AFTER_FIELD(p, fmt.pix);
 		ret = ops->vidioc_s_fmt_vid_cap(file, fh, arg);
@@ -1546,6 +1550,7 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops,
 	struct video_device *vfd = video_devdata(file);
 	bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER;
 	bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
+	bool is_touch = vfd->vfl_type == VFL_TYPE_TOUCH_SENSOR;
 	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
 	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
 	int ret;
@@ -1554,7 +1559,7 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops,
 
 	switch (p->type) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
-		if (unlikely(!is_rx || !is_vid || !ops->vidioc_try_fmt_vid_cap))
+		if (unlikely(!is_rx || (!is_vid && !is_touch) || !ops->vidioc_try_fmt_vid_cap))
 			break;
 		CLEAR_AFTER_FIELD(p, fmt.pix);
 		ret = ops->vidioc_try_fmt_vid_cap(file, fh, arg);
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index 25a3190..99eb87a 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -25,7 +25,8 @@
 #define VFL_TYPE_RADIO		2
 #define VFL_TYPE_SUBDEV		3
 #define VFL_TYPE_SDR		4
-#define VFL_TYPE_MAX		5
+#define VFL_TYPE_TOUCH_SENSOR	5
+#define VFL_TYPE_MAX		6
 
 /* Is this a receiver, transmitter or mem-to-mem? */
 /* Ignored for VFL_TYPE_SUBDEV. */
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index e0125cf..765a6cb 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1400,6 +1400,7 @@ struct v4l2_input {
 /*  Values for the 'type' field */
 #define V4L2_INPUT_TYPE_TUNER		1
 #define V4L2_INPUT_TYPE_CAMERA		2
+#define V4L2_INPUT_TYPE_TOUCH_SENSOR	3
 
 /* field 'status' - general */
 #define V4L2_IN_ST_NO_POWER    0x00000001  /* Attached device is off */
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v4 3/9] Input: atmel_mxt_ts - add support for T37 diagnostic data
  2016-06-17 14:16 [PATCH v4 0/9] Output raw touch data via V4L2 Nick Dyer
  2016-06-17 14:16 ` [PATCH v4 1/9] [media] Add signed 16-bit pixel format Nick Dyer
  2016-06-17 14:16 ` [PATCH v4 2/9] [media] v4l2-core: Add VFL_TYPE_TOUCH_SENSOR Nick Dyer
@ 2016-06-17 14:16 ` Nick Dyer
  2016-06-17 14:16 ` [PATCH v4 4/9] Input: atmel_mxt_ts - output diagnostic debug via v4l2 device Nick Dyer
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Nick Dyer @ 2016-06-17 14:16 UTC (permalink / raw)
  To: Dmitry Torokhov, Hans Verkuil
  Cc: linux-input, linux-kernel, linux-media, Benjamin Tissoires,
	Benson Leung, Alan Bowens, Javier Martinez Canillas, Chris Healy,
	Henrik Rydberg, Andrew Duggan, James Chen, Dudley Du,
	Andrew de los Reyes, sheckylin, Peter Hutterer, Florian Echtler,
	mchehab, Nick Dyer

Atmel maXTouch devices have a T37 object which can be used to read raw
touch deltas from the device. This consists of an array of 16-bit
integers, one for each node on the touchscreen matrix.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 drivers/input/touchscreen/Kconfig        |   6 ++
 drivers/input/touchscreen/atmel_mxt_ts.c | 159 +++++++++++++++++++++++++++++++
 2 files changed, 165 insertions(+)

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 8ecdc38..da96ecf 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -115,6 +115,12 @@ config TOUCHSCREEN_ATMEL_MXT
 	  To compile this driver as a module, choose M here: the
 	  module will be called atmel_mxt_ts.
 
+config TOUCHSCREEN_ATMEL_MXT_T37
+	bool "Support T37 Diagnostic Data"
+	depends on TOUCHSCREEN_ATMEL_MXT
+	help
+	  Say Y here if you want support for the T37 Diagnostic Data object.
+
 config TOUCHSCREEN_AUO_PIXCIR
 	tristate "AUO in-cell touchscreen using Pixcir ICs"
 	depends on I2C
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 5af7907..0048233 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -124,6 +124,19 @@ struct t9_range {
 #define MXT_COMMS_CTRL		0
 #define MXT_COMMS_CMD		1
 
+/* MXT_DEBUG_DIAGNOSTIC_T37 */
+#define MXT_DIAGNOSTIC_PAGEUP	0x01
+#define MXT_DIAGNOSTIC_DELTAS	0x10
+#define MXT_DIAGNOSTIC_SIZE	128
+
+struct t37_debug {
+#ifdef CONFIG_TOUCHSCREEN_ATMEL_MXT_T37
+	u8 mode;
+	u8 page;
+	u8 data[MXT_DIAGNOSTIC_SIZE];
+#endif
+};
+
 /* Define for MXT_GEN_COMMAND_T6 */
 #define MXT_BOOT_VALUE		0xa5
 #define MXT_RESET_VALUE		0x01
@@ -205,6 +218,14 @@ struct mxt_object {
 	u8 num_report_ids;
 } __packed;
 
+struct mxt_dbg {
+	u16 t37_address;
+	u16 diag_cmd_address;
+	struct t37_debug *t37_buf;
+	unsigned int t37_pages;
+	unsigned int t37_nodes;
+};
+
 /* Each client has this additional data */
 struct mxt_data {
 	struct i2c_client *client;
@@ -233,6 +254,7 @@ struct mxt_data {
 	u8 num_touchids;
 	u8 multitouch;
 	struct t7_config t7_cfg;
+	struct mxt_dbg dbg;
 
 	/* Cached parameters from object table */
 	u16 T5_address;
@@ -2043,6 +2065,141 @@ recheck:
 	return 0;
 }
 
+#ifdef CONFIG_TOUCHSCREEN_ATMEL_MXT_T37
+static u16 mxt_get_debug_value(struct mxt_data *data, unsigned int x,
+			       unsigned int y)
+{
+	struct mxt_dbg *dbg = &data->dbg;
+	unsigned int ofs, page;
+
+	ofs = (y + (x * data->info.matrix_ysize)) * sizeof(u16);
+	page = ofs / MXT_DIAGNOSTIC_SIZE;
+	ofs %= MXT_DIAGNOSTIC_SIZE;
+
+	return get_unaligned_le16(&dbg->t37_buf[page].data[ofs]);
+}
+
+static int mxt_convert_debug_pages(struct mxt_data *data, u16 *outbuf)
+{
+	struct mxt_dbg *dbg = &data->dbg;
+	unsigned int x = 0;
+	unsigned int y = 0;
+	unsigned int i;
+
+	for (i = 0; i < dbg->t37_nodes; i++) {
+		outbuf[i] = mxt_get_debug_value(data, x, y);
+
+		/* Next value */
+		if (++x >= data->info.matrix_xsize) {
+			x = 0;
+			y++;
+		}
+	}
+
+	return 0;
+}
+
+static int mxt_read_diagnostic_debug(struct mxt_data *data, u8 mode,
+				     u16 *outbuf)
+{
+	struct mxt_dbg *dbg = &data->dbg;
+	int retries = 0;
+	int page;
+	int ret;
+	u8 cmd = mode;
+	struct t37_debug *p;
+	u8 cmd_poll;
+
+	for (page = 0; page < dbg->t37_pages; page++) {
+		p = dbg->t37_buf + page;
+
+		ret = mxt_write_reg(data->client, dbg->diag_cmd_address,
+				    cmd);
+		if (ret)
+			return ret;
+
+		retries = 0;
+		msleep(20);
+wait_cmd:
+		/* Read back command byte */
+		ret = __mxt_read_reg(data->client, dbg->diag_cmd_address,
+				     sizeof(cmd_poll), &cmd_poll);
+		if (ret)
+			return ret;
+
+		/* Field is cleared once the command has been processed */
+		if (cmd_poll) {
+			if (retries++ > 100)
+				return -EINVAL;
+
+			msleep(20);
+			goto wait_cmd;
+		}
+
+		/* Read T37 page */
+		ret = __mxt_read_reg(data->client, dbg->t37_address,
+				     sizeof(struct t37_debug), p);
+		if (ret)
+			return ret;
+
+		if (p->mode != mode || p->page != page) {
+			dev_err(&data->client->dev, "T37 page mismatch\n");
+			return -EINVAL;
+		}
+
+		dev_dbg(&data->client->dev, "%s page:%d retries:%d\n",
+			__func__, page, retries);
+
+		/* For remaining pages, write PAGEUP rather than mode */
+		cmd = MXT_DIAGNOSTIC_PAGEUP;
+	}
+
+	return mxt_convert_debug_pages(data, outbuf);
+}
+
+static void mxt_debug_init(struct mxt_data *data)
+{
+	struct mxt_dbg *dbg = &data->dbg;
+	struct mxt_object *object;
+
+	object = mxt_get_object(data, MXT_GEN_COMMAND_T6);
+	if (!object)
+		goto error;
+
+	dbg->diag_cmd_address = object->start_address + MXT_COMMAND_DIAGNOSTIC;
+
+	object = mxt_get_object(data, MXT_DEBUG_DIAGNOSTIC_T37);
+	if (!object)
+		goto error;
+
+	if (mxt_obj_size(object) != sizeof(struct t37_debug)) {
+		dev_warn(&data->client->dev, "Bad T37 size");
+		goto error;
+	}
+
+	dbg->t37_address = object->start_address;
+
+	/* Calculate size of data and allocate buffer */
+	dbg->t37_nodes = data->info.matrix_xsize * data->info.matrix_ysize;
+	dbg->t37_pages = DIV_ROUND_UP(dbg->t37_nodes * sizeof(u16),
+				      sizeof(dbg->t37_buf->data));
+
+	dbg->t37_buf = devm_kmalloc_array(&data->client->dev, dbg->t37_pages,
+					  sizeof(struct t37_debug), GFP_KERNEL);
+	if (!dbg->t37_buf)
+		goto error;
+
+	return;
+
+error:
+	dev_warn(&data->client->dev, "Error initializing T37\n");
+}
+#else
+static void mxt_debug_init(struct mxt_data *data)
+{
+}
+#endif
+
 static int mxt_configure_objects(struct mxt_data *data,
 				 const struct firmware *cfg)
 {
@@ -2070,6 +2227,8 @@ static int mxt_configure_objects(struct mxt_data *data,
 		dev_warn(dev, "No touch object detected\n");
 	}
 
+	mxt_debug_init(data);
+
 	dev_info(dev,
 		 "Family: %u Variant: %u Firmware V%u.%u.%02X Objects: %u\n",
 		 info->family_id, info->variant_id, info->version >> 4,
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v4 4/9] Input: atmel_mxt_ts - output diagnostic debug via v4l2 device
  2016-06-17 14:16 [PATCH v4 0/9] Output raw touch data via V4L2 Nick Dyer
                   ` (2 preceding siblings ...)
  2016-06-17 14:16 ` [PATCH v4 3/9] Input: atmel_mxt_ts - add support for T37 diagnostic data Nick Dyer
@ 2016-06-17 14:16 ` Nick Dyer
  2016-06-17 14:16 ` [PATCH v4 5/9] Input: atmel_mxt_ts - read touchscreen size Nick Dyer
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Nick Dyer @ 2016-06-17 14:16 UTC (permalink / raw)
  To: Dmitry Torokhov, Hans Verkuil
  Cc: linux-input, linux-kernel, linux-media, Benjamin Tissoires,
	Benson Leung, Alan Bowens, Javier Martinez Canillas, Chris Healy,
	Henrik Rydberg, Andrew Duggan, James Chen, Dudley Du,
	Andrew de los Reyes, sheckylin, Peter Hutterer, Florian Echtler,
	mchehab, Nick Dyer

Register a video device to output T37 diagnostic data.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 drivers/input/touchscreen/Kconfig        |   6 +-
 drivers/input/touchscreen/atmel_mxt_ts.c | 244 +++++++++++++++++++++++++++++++
 2 files changed, 248 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index da96ecf..7c1c5ec 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -117,9 +117,11 @@ config TOUCHSCREEN_ATMEL_MXT
 
 config TOUCHSCREEN_ATMEL_MXT_T37
 	bool "Support T37 Diagnostic Data"
-	depends on TOUCHSCREEN_ATMEL_MXT
+	depends on TOUCHSCREEN_ATMEL_MXT && VIDEO_V4L2
+	select VIDEOBUF2_VMALLOC
 	help
-	  Say Y here if you want support for the T37 Diagnostic Data object.
+	  Say Y here if you want support to output data from the T37
+	  Diagnostic Data object using a V4L device.
 
 config TOUCHSCREEN_AUO_PIXCIR
 	tristate "AUO in-cell touchscreen using Pixcir ICs"
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 0048233..3a7f705 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -28,6 +28,10 @@
 #include <linux/of.h>
 #include <linux/slab.h>
 #include <asm/unaligned.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-ioctl.h>
+#include <media/videobuf2-v4l2.h>
+#include <media/videobuf2-vmalloc.h>
 
 /* Firmware files */
 #define MXT_FW_NAME		"maxtouch.fw"
@@ -224,6 +228,23 @@ struct mxt_dbg {
 	struct t37_debug *t37_buf;
 	unsigned int t37_pages;
 	unsigned int t37_nodes;
+
+	struct v4l2_device v4l2;
+	struct v4l2_pix_format format;
+	struct video_device vdev;
+	struct vb2_queue queue;
+	struct mutex lock;
+	int input;
+};
+
+static const struct v4l2_file_operations mxt_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,
 };
 
 /* Each client has this additional data */
@@ -279,6 +300,11 @@ struct mxt_data {
 	struct completion crc_completion;
 };
 
+struct mxt_vb2_buffer {
+	struct vb2_buffer	vb;
+	struct list_head	list;
+};
+
 static size_t mxt_obj_size(const struct mxt_object *obj)
 {
 	return obj->size_minus_one + 1;
@@ -1525,6 +1551,11 @@ static void mxt_free_input_device(struct mxt_data *data)
 
 static void mxt_free_object_table(struct mxt_data *data)
 {
+#ifdef CONFIG_TOUCHSCREEN_ATMEL_MXT_T37
+	video_unregister_device(&data->dbg.vdev);
+	v4l2_device_unregister(&data->dbg.v4l2);
+#endif
+
 	kfree(data->object_table);
 	data->object_table = NULL;
 	kfree(data->msg_buf);
@@ -2157,10 +2188,191 @@ wait_cmd:
 	return mxt_convert_debug_pages(data, outbuf);
 }
 
+static int mxt_queue_setup(struct vb2_queue *q,
+		       unsigned int *nbuffers, unsigned int *nplanes,
+		       unsigned int sizes[], void *alloc_ctxs[])
+{
+	struct mxt_data *data = q->drv_priv;
+	size_t size = data->dbg.t37_nodes * sizeof(u16);
+
+	if (*nplanes)
+		return sizes[0] < size ? -EINVAL : 0;
+
+	*nplanes = 1;
+	sizes[0] = size;
+
+	return 0;
+}
+
+static void mxt_buffer_queue(struct vb2_buffer *vb)
+{
+	struct mxt_data *data = vb2_get_drv_priv(vb->vb2_queue);
+	u16 *ptr;
+	int ret;
+
+	ptr = vb2_plane_vaddr(vb, 0);
+	if (!ptr) {
+		dev_err(&data->client->dev, "Error acquiring frame ptr\n");
+		goto fault;
+	}
+
+	ret = mxt_read_diagnostic_debug(data, MXT_DIAGNOSTIC_DELTAS, ptr);
+	if (ret)
+		goto fault;
+
+	vb2_set_plane_payload(vb, 0, data->dbg.t37_nodes * sizeof(u16));
+	vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
+	return;
+
+fault:
+	vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
+}
+
+/* V4L2 structures */
+static const struct vb2_ops mxt_queue_ops = {
+	.queue_setup		= mxt_queue_setup,
+	.buf_queue		= mxt_buffer_queue,
+	.wait_prepare		= vb2_ops_wait_prepare,
+	.wait_finish		= vb2_ops_wait_finish,
+};
+
+static const struct vb2_queue mxt_queue = {
+	.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
+	.io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF | VB2_READ,
+	.buf_struct_size = sizeof(struct mxt_vb2_buffer),
+	.ops = &mxt_queue_ops,
+	.mem_ops = &vb2_vmalloc_memops,
+	.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC,
+	.min_buffers_needed = 1,
+};
+
+static int mxt_vidioc_querycap(struct file *file, void *priv,
+				 struct v4l2_capability *cap)
+{
+	struct mxt_data *data = video_drvdata(file);
+
+	strlcpy(cap->driver, "atmel_mxt_ts", sizeof(cap->driver));
+	strlcpy(cap->card, "atmel_mxt_ts touch", sizeof(cap->card));
+	snprintf(cap->bus_info, sizeof(cap->bus_info),
+		 "I2C:%s", dev_name(&data->client->dev));
+	return 0;
+}
+
+static int mxt_vidioc_enum_input(struct file *file, void *priv,
+				   struct v4l2_input *i)
+{
+	if (i->index > 0)
+		return -EINVAL;
+
+	i->type = V4L2_INPUT_TYPE_TOUCH_SENSOR;
+	strlcpy(i->name, "Mutual References", sizeof(i->name));
+	return 0;
+}
+
+static int mxt_set_input(struct mxt_data *data, unsigned int i)
+{
+	struct v4l2_pix_format *f = &data->dbg.format;
+
+	if (i > 0)
+		return -EINVAL;
+
+	f->width = data->info.matrix_xsize;
+	f->height = data->info.matrix_ysize;
+	f->pixelformat = V4L2_PIX_FMT_YS16;
+	f->field = V4L2_FIELD_NONE;
+	f->colorspace = V4L2_COLORSPACE_RAW;
+	f->bytesperline = f->width * sizeof(u16);
+	f->sizeimage = f->width * f->height * sizeof(u16);
+
+	data->dbg.input = i;
+
+	return 0;
+}
+
+static int mxt_vidioc_s_input(struct file *file, void *priv, unsigned int i)
+{
+	return mxt_set_input(video_drvdata(file), i);
+}
+
+static int mxt_vidioc_g_input(struct file *file, void *priv, unsigned int *i)
+{
+	struct mxt_data *data = video_drvdata(file);
+
+	*i = data->dbg.input;
+
+	return 0;
+}
+
+static int mxt_vidioc_fmt(struct file *file, void *priv, struct v4l2_format *f)
+{
+	struct mxt_data *data = video_drvdata(file);
+
+	f->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	f->fmt.pix = data->dbg.format;
+
+	return 0;
+}
+
+static int mxt_vidioc_enum_fmt(struct file *file, void *priv,
+				 struct v4l2_fmtdesc *fmt)
+{
+	if (fmt->index > 0 || fmt->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
+	fmt->pixelformat = V4L2_PIX_FMT_YS16;
+	return 0;
+}
+
+static int mxt_vidioc_g_parm(struct file *file, void *fh,
+			     struct v4l2_streamparm *a)
+{
+	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
+	a->parm.capture.readbuffers = 1;
+	a->parm.capture.timeperframe.numerator = 1;
+	a->parm.capture.timeperframe.denominator = 10;
+	return 0;
+}
+
+static const struct v4l2_ioctl_ops mxt_video_ioctl_ops = {
+	.vidioc_querycap        = mxt_vidioc_querycap,
+
+	.vidioc_enum_fmt_vid_cap = mxt_vidioc_enum_fmt,
+	.vidioc_s_fmt_vid_cap   = mxt_vidioc_fmt,
+	.vidioc_g_fmt_vid_cap   = mxt_vidioc_fmt,
+	.vidioc_try_fmt_vid_cap	= mxt_vidioc_fmt,
+	.vidioc_g_parm		= mxt_vidioc_g_parm,
+
+	.vidioc_enum_input      = mxt_vidioc_enum_input,
+	.vidioc_g_input         = mxt_vidioc_g_input,
+	.vidioc_s_input         = mxt_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,
+};
+
+static const struct video_device mxt_video_device = {
+	.name = "Atmel maxTouch",
+	.fops = &mxt_video_fops,
+	.ioctl_ops = &mxt_video_ioctl_ops,
+	.release = video_device_release_empty,
+	.device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
+		       V4L2_CAP_STREAMING,
+};
+
 static void mxt_debug_init(struct mxt_data *data)
 {
 	struct mxt_dbg *dbg = &data->dbg;
 	struct mxt_object *object;
+	int error;
 
 	object = mxt_get_object(data, MXT_GEN_COMMAND_T6);
 	if (!object)
@@ -2189,8 +2401,40 @@ static void mxt_debug_init(struct mxt_data *data)
 	if (!dbg->t37_buf)
 		goto error;
 
+	/* init channel to zero */
+	mxt_set_input(data, 0);
+
+	/* register video device */
+	snprintf(dbg->v4l2.name, sizeof(dbg->v4l2.name), "%s", "atmel_mxt_ts");
+	error = v4l2_device_register(&data->client->dev, &dbg->v4l2);
+	if (error)
+		goto error;
+
+	/* initialize the queue */
+	mutex_init(&dbg->lock);
+	dbg->queue = mxt_queue;
+	dbg->queue.drv_priv = data;
+	dbg->queue.lock = &dbg->lock;
+
+	error = vb2_queue_init(&dbg->queue);
+	if (error)
+		goto error_unreg_v4l2;
+
+	dbg->vdev = mxt_video_device;
+	dbg->vdev.v4l2_dev = &dbg->v4l2;
+	dbg->vdev.lock = &dbg->lock;
+	dbg->vdev.vfl_dir = VFL_DIR_RX;
+	dbg->vdev.queue = &dbg->queue;
+	video_set_drvdata(&dbg->vdev, data);
+
+	error = video_register_device(&dbg->vdev, VFL_TYPE_TOUCH_SENSOR, -1);
+	if (error)
+		goto error_unreg_v4l2;
+
 	return;
 
+error_unreg_v4l2:
+	v4l2_device_unregister(&dbg->v4l2);
 error:
 	dev_warn(&data->client->dev, "Error initializing T37\n");
 }
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v4 5/9] Input: atmel_mxt_ts - read touchscreen size
  2016-06-17 14:16 [PATCH v4 0/9] Output raw touch data via V4L2 Nick Dyer
                   ` (3 preceding siblings ...)
  2016-06-17 14:16 ` [PATCH v4 4/9] Input: atmel_mxt_ts - output diagnostic debug via v4l2 device Nick Dyer
@ 2016-06-17 14:16 ` Nick Dyer
  2016-06-17 14:16 ` [PATCH v4 6/9] Input: atmel_mxt_ts - handle diagnostic data orientation Nick Dyer
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Nick Dyer @ 2016-06-17 14:16 UTC (permalink / raw)
  To: Dmitry Torokhov, Hans Verkuil
  Cc: linux-input, linux-kernel, linux-media, Benjamin Tissoires,
	Benson Leung, Alan Bowens, Javier Martinez Canillas, Chris Healy,
	Henrik Rydberg, Andrew Duggan, James Chen, Dudley Du,
	Andrew de los Reyes, sheckylin, Peter Hutterer, Florian Echtler,
	mchehab, Nick Dyer

The touchscreen may have a margin where not all the matrix is used. Read
the parameters from T9 and T100 and take account of the difference.

Note: this does not read the XORIGIN/YORIGIN fields so it assumes that
the touchscreen starts at (0,0)

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 42 +++++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 3a7f705..3c5b7d3 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -103,6 +103,8 @@ struct t7_config {
 
 /* MXT_TOUCH_MULTI_T9 field */
 #define MXT_T9_CTRL		0
+#define MXT_T9_XSIZE		3
+#define MXT_T9_YSIZE		4
 #define MXT_T9_ORIENT		9
 #define MXT_T9_RANGE		18
 
@@ -150,7 +152,9 @@ struct t37_debug {
 #define MXT_T100_CTRL		0
 #define MXT_T100_CFG1		1
 #define MXT_T100_TCHAUX		3
+#define MXT_T100_XSIZE		9
 #define MXT_T100_XRANGE		13
+#define MXT_T100_YSIZE		20
 #define MXT_T100_YRANGE		24
 
 #define MXT_T100_CFG_SWITCHXY	BIT(5)
@@ -259,6 +263,8 @@ struct mxt_data {
 	unsigned int max_x;
 	unsigned int max_y;
 	bool xy_switch;
+	u8 xsize;
+	u8 ysize;
 	bool in_bootloader;
 	u16 mem_size;
 	u8 t100_aux_ampl;
@@ -1714,6 +1720,18 @@ static int mxt_read_t9_resolution(struct mxt_data *data)
 		return -EINVAL;
 
 	error = __mxt_read_reg(client,
+			       object->start_address + MXT_T9_XSIZE,
+			       sizeof(data->xsize), &data->xsize);
+	if (error)
+		return error;
+
+	error = __mxt_read_reg(client,
+			       object->start_address + MXT_T9_YSIZE,
+			       sizeof(data->ysize), &data->ysize);
+	if (error)
+		return error;
+
+	error = __mxt_read_reg(client,
 			       object->start_address + MXT_T9_RANGE,
 			       sizeof(range), &range);
 	if (error)
@@ -1763,6 +1781,18 @@ static int mxt_read_t100_config(struct mxt_data *data)
 
 	data->max_y = get_unaligned_le16(&range_y);
 
+	error = __mxt_read_reg(client,
+			       object->start_address + MXT_T100_XSIZE,
+			       sizeof(data->xsize), &data->xsize);
+	if (error)
+		return error;
+
+	error = __mxt_read_reg(client,
+			       object->start_address + MXT_T100_YSIZE,
+			       sizeof(data->ysize), &data->ysize);
+	if (error)
+		return error;
+
 	/* read orientation config */
 	error =  __mxt_read_reg(client,
 				object->start_address + MXT_T100_CFG1,
@@ -2121,7 +2151,7 @@ static int mxt_convert_debug_pages(struct mxt_data *data, u16 *outbuf)
 		outbuf[i] = mxt_get_debug_value(data, x, y);
 
 		/* Next value */
-		if (++x >= data->info.matrix_xsize) {
+		if (++x >= data->xsize) {
 			x = 0;
 			y++;
 		}
@@ -2276,8 +2306,8 @@ static int mxt_set_input(struct mxt_data *data, unsigned int i)
 	if (i > 0)
 		return -EINVAL;
 
-	f->width = data->info.matrix_xsize;
-	f->height = data->info.matrix_ysize;
+	f->width = data->xsize;
+	f->height = data->ysize;
 	f->pixelformat = V4L2_PIX_FMT_YS16;
 	f->field = V4L2_FIELD_NONE;
 	f->colorspace = V4L2_COLORSPACE_RAW;
@@ -2392,9 +2422,9 @@ static void mxt_debug_init(struct mxt_data *data)
 	dbg->t37_address = object->start_address;
 
 	/* Calculate size of data and allocate buffer */
-	dbg->t37_nodes = data->info.matrix_xsize * data->info.matrix_ysize;
-	dbg->t37_pages = DIV_ROUND_UP(dbg->t37_nodes * sizeof(u16),
-				      sizeof(dbg->t37_buf->data));
+	dbg->t37_nodes = data->xsize * data->ysize;
+	dbg->t37_pages = DIV_ROUND_UP(data->xsize * data->info.matrix_ysize *
+				      sizeof(u16), sizeof(dbg->t37_buf->data));
 
 	dbg->t37_buf = devm_kmalloc_array(&data->client->dev, dbg->t37_pages,
 					  sizeof(struct t37_debug), GFP_KERNEL);
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v4 6/9] Input: atmel_mxt_ts - handle diagnostic data orientation
  2016-06-17 14:16 [PATCH v4 0/9] Output raw touch data via V4L2 Nick Dyer
                   ` (4 preceding siblings ...)
  2016-06-17 14:16 ` [PATCH v4 5/9] Input: atmel_mxt_ts - read touchscreen size Nick Dyer
@ 2016-06-17 14:16 ` Nick Dyer
  2016-06-17 14:16 ` [PATCH v4 7/9] Input: atmel_mxt_ts - add diagnostic data support for mXT1386 Nick Dyer
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Nick Dyer @ 2016-06-17 14:16 UTC (permalink / raw)
  To: Dmitry Torokhov, Hans Verkuil
  Cc: linux-input, linux-kernel, linux-media, Benjamin Tissoires,
	Benson Leung, Alan Bowens, Javier Martinez Canillas, Chris Healy,
	Henrik Rydberg, Andrew Duggan, James Chen, Dudley Du,
	Andrew de los Reyes, sheckylin, Peter Hutterer, Florian Echtler,
	mchehab, Nick Dyer

Invert the diagnostic data to match the orientation of the input device.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 3c5b7d3..2a03636 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -125,6 +125,8 @@ struct t9_range {
 
 /* MXT_TOUCH_MULTI_T9 orient */
 #define MXT_T9_ORIENT_SWITCH	(1 << 0)
+#define MXT_T9_ORIENT_INVERTX	(1 << 1)
+#define MXT_T9_ORIENT_INVERTY	(1 << 2)
 
 /* MXT_SPT_COMMSCONFIG_T18 */
 #define MXT_COMMS_CTRL		0
@@ -158,6 +160,8 @@ struct t37_debug {
 #define MXT_T100_YRANGE		24
 
 #define MXT_T100_CFG_SWITCHXY	BIT(5)
+#define MXT_T100_CFG_INVERTY	BIT(6)
+#define MXT_T100_CFG_INVERTX	BIT(7)
 
 #define MXT_T100_TCHAUX_VECT	BIT(0)
 #define MXT_T100_TCHAUX_AMPL	BIT(1)
@@ -262,6 +266,8 @@ struct mxt_data {
 	unsigned int irq;
 	unsigned int max_x;
 	unsigned int max_y;
+	bool invertx;
+	bool inverty;
 	bool xy_switch;
 	u8 xsize;
 	u8 ysize;
@@ -1747,6 +1753,8 @@ static int mxt_read_t9_resolution(struct mxt_data *data)
 		return error;
 
 	data->xy_switch = orient & MXT_T9_ORIENT_SWITCH;
+	data->invertx = orient & MXT_T9_ORIENT_INVERTX;
+	data->inverty = orient & MXT_T9_ORIENT_INVERTY;
 
 	return 0;
 }
@@ -1801,6 +1809,8 @@ static int mxt_read_t100_config(struct mxt_data *data)
 		return error;
 
 	data->xy_switch = cfg & MXT_T100_CFG_SWITCHXY;
+	data->invertx = cfg & MXT_T100_CFG_INVERTX;
+	data->inverty = cfg & MXT_T100_CFG_INVERTY;
 
 	/* allocate aux bytes */
 	error =  __mxt_read_reg(client,
@@ -2145,13 +2155,19 @@ static int mxt_convert_debug_pages(struct mxt_data *data, u16 *outbuf)
 	struct mxt_dbg *dbg = &data->dbg;
 	unsigned int x = 0;
 	unsigned int y = 0;
-	unsigned int i;
+	unsigned int i, rx, ry;
 
 	for (i = 0; i < dbg->t37_nodes; i++) {
-		outbuf[i] = mxt_get_debug_value(data, x, y);
+		/* Handle orientation */
+		rx = data->xy_switch ? y : x;
+		ry = data->xy_switch ? x : y;
+		rx = data->invertx ? (data->xsize - 1 - rx) : rx;
+		ry = data->inverty ? (data->ysize - 1 - ry) : ry;
+
+		outbuf[i] = mxt_get_debug_value(data, rx, ry);
 
 		/* Next value */
-		if (++x >= data->xsize) {
+		if (++x >= (data->xy_switch ? data->ysize : data->xsize)) {
 			x = 0;
 			y++;
 		}
@@ -2306,8 +2322,8 @@ static int mxt_set_input(struct mxt_data *data, unsigned int i)
 	if (i > 0)
 		return -EINVAL;
 
-	f->width = data->xsize;
-	f->height = data->ysize;
+	f->width = data->xy_switch ? data->ysize : data->xsize;
+	f->height = data->xy_switch ? data->xsize : data->ysize;
 	f->pixelformat = V4L2_PIX_FMT_YS16;
 	f->field = V4L2_FIELD_NONE;
 	f->colorspace = V4L2_COLORSPACE_RAW;
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v4 7/9] Input: atmel_mxt_ts - add diagnostic data support for mXT1386
  2016-06-17 14:16 [PATCH v4 0/9] Output raw touch data via V4L2 Nick Dyer
                   ` (5 preceding siblings ...)
  2016-06-17 14:16 ` [PATCH v4 6/9] Input: atmel_mxt_ts - handle diagnostic data orientation Nick Dyer
@ 2016-06-17 14:16 ` Nick Dyer
  2016-06-17 14:16 ` [PATCH v4 8/9] Input: atmel_mxt_ts - add support for reference data Nick Dyer
  2016-06-17 14:16 ` [PATCH v4 9/9] Input: synaptics-rmi4 - add support for F54 diagnostics Nick Dyer
  8 siblings, 0 replies; 22+ messages in thread
From: Nick Dyer @ 2016-06-17 14:16 UTC (permalink / raw)
  To: Dmitry Torokhov, Hans Verkuil
  Cc: linux-input, linux-kernel, linux-media, Benjamin Tissoires,
	Benson Leung, Alan Bowens, Javier Martinez Canillas, Chris Healy,
	Henrik Rydberg, Andrew Duggan, James Chen, Dudley Du,
	Andrew de los Reyes, sheckylin, Peter Hutterer, Florian Echtler,
	mchehab, Nick Dyer

The mXT1386 family of chips have a different architecture which splits
the diagnostic data into 3 columns.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 2a03636..5281325d 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -137,6 +137,10 @@ struct t9_range {
 #define MXT_DIAGNOSTIC_DELTAS	0x10
 #define MXT_DIAGNOSTIC_SIZE	128
 
+#define MXT_FAMILY_1386			160
+#define MXT1386_COLUMNS			3
+#define MXT1386_PAGES_PER_COLUMN	8
+
 struct t37_debug {
 #ifdef CONFIG_TOUCHSCREEN_ATMEL_MXT_T37
 	u8 mode;
@@ -2140,13 +2144,27 @@ recheck:
 static u16 mxt_get_debug_value(struct mxt_data *data, unsigned int x,
 			       unsigned int y)
 {
+	struct mxt_info *info = &data->info;
 	struct mxt_dbg *dbg = &data->dbg;
 	unsigned int ofs, page;
+	unsigned int col = 0;
+	unsigned int col_width;
+
+	if (info->family_id == MXT_FAMILY_1386) {
+		col_width = info->matrix_ysize / MXT1386_COLUMNS;
+		col = y / col_width;
+		y = y % col_width;
+	} else {
+		col_width = info->matrix_ysize;
+	}
 
-	ofs = (y + (x * data->info.matrix_ysize)) * sizeof(u16);
+	ofs = (y + (x * col_width)) * sizeof(u16);
 	page = ofs / MXT_DIAGNOSTIC_SIZE;
 	ofs %= MXT_DIAGNOSTIC_SIZE;
 
+	if (info->family_id == MXT_FAMILY_1386)
+		page += col * MXT1386_PAGES_PER_COLUMN;
+
 	return get_unaligned_le16(&dbg->t37_buf[page].data[ofs]);
 }
 
@@ -2416,6 +2434,7 @@ static const struct video_device mxt_video_device = {
 
 static void mxt_debug_init(struct mxt_data *data)
 {
+	struct mxt_info *info = &data->info;
 	struct mxt_dbg *dbg = &data->dbg;
 	struct mxt_object *object;
 	int error;
@@ -2439,8 +2458,14 @@ static void mxt_debug_init(struct mxt_data *data)
 
 	/* Calculate size of data and allocate buffer */
 	dbg->t37_nodes = data->xsize * data->ysize;
-	dbg->t37_pages = DIV_ROUND_UP(data->xsize * data->info.matrix_ysize *
-				      sizeof(u16), sizeof(dbg->t37_buf->data));
+
+	if (info->family_id == MXT_FAMILY_1386)
+		dbg->t37_pages = MXT1386_COLUMNS * MXT1386_PAGES_PER_COLUMN;
+	else
+		dbg->t37_pages = DIV_ROUND_UP(data->xsize *
+					      data->info.matrix_ysize *
+					      sizeof(u16),
+					      sizeof(dbg->t37_buf->data));
 
 	dbg->t37_buf = devm_kmalloc_array(&data->client->dev, dbg->t37_pages,
 					  sizeof(struct t37_debug), GFP_KERNEL);
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v4 8/9] Input: atmel_mxt_ts - add support for reference data
  2016-06-17 14:16 [PATCH v4 0/9] Output raw touch data via V4L2 Nick Dyer
                   ` (6 preceding siblings ...)
  2016-06-17 14:16 ` [PATCH v4 7/9] Input: atmel_mxt_ts - add diagnostic data support for mXT1386 Nick Dyer
@ 2016-06-17 14:16 ` Nick Dyer
  2016-06-20 16:09   ` Hans Verkuil
  2016-06-17 14:16 ` [PATCH v4 9/9] Input: synaptics-rmi4 - add support for F54 diagnostics Nick Dyer
  8 siblings, 1 reply; 22+ messages in thread
From: Nick Dyer @ 2016-06-17 14:16 UTC (permalink / raw)
  To: Dmitry Torokhov, Hans Verkuil
  Cc: linux-input, linux-kernel, linux-media, Benjamin Tissoires,
	Benson Leung, Alan Bowens, Javier Martinez Canillas, Chris Healy,
	Henrik Rydberg, Andrew Duggan, James Chen, Dudley Du,
	Andrew de los Reyes, sheckylin, Peter Hutterer, Florian Echtler,
	mchehab, Nick Dyer

There are different datatypes available from a maXTouch chip. Add
support to retrieve reference data as well.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 58 ++++++++++++++++++++++++++++----
 1 file changed, 51 insertions(+), 7 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 5281325d..bb01fb0 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -135,6 +135,7 @@ struct t9_range {
 /* MXT_DEBUG_DIAGNOSTIC_T37 */
 #define MXT_DIAGNOSTIC_PAGEUP	0x01
 #define MXT_DIAGNOSTIC_DELTAS	0x10
+#define MXT_DIAGNOSTIC_REFS	0x11
 #define MXT_DIAGNOSTIC_SIZE	128
 
 #define MXT_FAMILY_1386			160
@@ -249,6 +250,12 @@ struct mxt_dbg {
 	int input;
 };
 
+enum v4l_dbg_inputs {
+	MXT_V4L_INPUT_DELTAS,
+	MXT_V4L_INPUT_REFS,
+	MXT_V4L_INPUT_MAX,
+};
+
 static const struct v4l2_file_operations mxt_video_fops = {
 	.owner = THIS_MODULE,
 	.open = v4l2_fh_open,
@@ -2273,6 +2280,7 @@ static void mxt_buffer_queue(struct vb2_buffer *vb)
 	struct mxt_data *data = vb2_get_drv_priv(vb->vb2_queue);
 	u16 *ptr;
 	int ret;
+	u8 mode;
 
 	ptr = vb2_plane_vaddr(vb, 0);
 	if (!ptr) {
@@ -2280,7 +2288,18 @@ static void mxt_buffer_queue(struct vb2_buffer *vb)
 		goto fault;
 	}
 
-	ret = mxt_read_diagnostic_debug(data, MXT_DIAGNOSTIC_DELTAS, ptr);
+	switch (data->dbg.input) {
+	case MXT_V4L_INPUT_DELTAS:
+	default:
+		mode = MXT_DIAGNOSTIC_DELTAS;
+		break;
+
+	case MXT_V4L_INPUT_REFS:
+		mode = MXT_DIAGNOSTIC_REFS;
+		break;
+	}
+
+	ret = mxt_read_diagnostic_debug(data, mode, ptr);
 	if (ret)
 		goto fault;
 
@@ -2325,11 +2344,20 @@ static int mxt_vidioc_querycap(struct file *file, void *priv,
 static int mxt_vidioc_enum_input(struct file *file, void *priv,
 				   struct v4l2_input *i)
 {
-	if (i->index > 0)
+	if (i->index >= MXT_V4L_INPUT_MAX)
 		return -EINVAL;
 
 	i->type = V4L2_INPUT_TYPE_TOUCH_SENSOR;
-	strlcpy(i->name, "Mutual References", sizeof(i->name));
+
+	switch (i->index) {
+	case MXT_V4L_INPUT_REFS:
+		strlcpy(i->name, "Mutual References", sizeof(i->name));
+		break;
+	case MXT_V4L_INPUT_DELTAS:
+		strlcpy(i->name, "Mutual Deltas", sizeof(i->name));
+		break;
+	}
+
 	return 0;
 }
 
@@ -2337,12 +2365,16 @@ static int mxt_set_input(struct mxt_data *data, unsigned int i)
 {
 	struct v4l2_pix_format *f = &data->dbg.format;
 
-	if (i > 0)
+	if (i >= MXT_V4L_INPUT_MAX)
 		return -EINVAL;
 
+	if (i == MXT_V4L_INPUT_DELTAS)
+		f->pixelformat = V4L2_PIX_FMT_YS16;
+	else
+		f->pixelformat = V4L2_PIX_FMT_Y16;
+
 	f->width = data->xy_switch ? data->ysize : data->xsize;
 	f->height = data->xy_switch ? data->xsize : data->ysize;
-	f->pixelformat = V4L2_PIX_FMT_YS16;
 	f->field = V4L2_FIELD_NONE;
 	f->colorspace = V4L2_COLORSPACE_RAW;
 	f->bytesperline = f->width * sizeof(u16);
@@ -2380,10 +2412,22 @@ static int mxt_vidioc_fmt(struct file *file, void *priv, struct v4l2_format *f)
 static int mxt_vidioc_enum_fmt(struct file *file, void *priv,
 				 struct v4l2_fmtdesc *fmt)
 {
-	if (fmt->index > 0 || fmt->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+	if (fmt->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
+	switch (fmt->index) {
+	case 0:
+		fmt->pixelformat = V4L2_PIX_FMT_Y16;
+		break;
+
+	case 1:
+		fmt->pixelformat = V4L2_PIX_FMT_YS16;
+		break;
+
+	default:
 		return -EINVAL;
+	}
 
-	fmt->pixelformat = V4L2_PIX_FMT_YS16;
 	return 0;
 }
 
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v4 9/9] Input: synaptics-rmi4 - add support for F54 diagnostics
  2016-06-17 14:16 [PATCH v4 0/9] Output raw touch data via V4L2 Nick Dyer
                   ` (7 preceding siblings ...)
  2016-06-17 14:16 ` [PATCH v4 8/9] Input: atmel_mxt_ts - add support for reference data Nick Dyer
@ 2016-06-17 14:16 ` Nick Dyer
  2016-06-20 16:20   ` Hans Verkuil
  8 siblings, 1 reply; 22+ messages in thread
From: Nick Dyer @ 2016-06-17 14:16 UTC (permalink / raw)
  To: Dmitry Torokhov, Hans Verkuil
  Cc: linux-input, linux-kernel, linux-media, Benjamin Tissoires,
	Benson Leung, Alan Bowens, Javier Martinez Canillas, Chris Healy,
	Henrik Rydberg, Andrew Duggan, James Chen, Dudley Du,
	Andrew de los Reyes, sheckylin, Peter Hutterer, Florian Echtler,
	mchehab, Nick Dyer

Function 54 implements access to various RMI4 diagnostic features.

This patch adds support for retrieving this data. It registers a V4L2
device to output the data to user space.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 drivers/input/rmi4/Kconfig      |  11 +
 drivers/input/rmi4/Makefile     |   1 +
 drivers/input/rmi4/rmi_bus.c    |   3 +
 drivers/input/rmi4/rmi_driver.h |   1 +
 drivers/input/rmi4/rmi_f54.c    | 743 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 759 insertions(+)
 create mode 100644 drivers/input/rmi4/rmi_f54.c

diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
index f73df24..f3418b6 100644
--- a/drivers/input/rmi4/Kconfig
+++ b/drivers/input/rmi4/Kconfig
@@ -61,3 +61,14 @@ config RMI4_F30
 
 	  Function 30 provides GPIO and LED support for RMI4 devices. This
 	  includes support for buttons on TouchPads and ClickPads.
+
+config RMI4_F54
+	bool "RMI4 Function 54 (Analog diagnostics)"
+	depends on RMI4_CORE
+	depends on VIDEO_V4L2
+	select VIDEOBUF2_VMALLOC
+	help
+	  Say Y here if you want to add support for RMI4 function 54
+
+	  Function 54 provides access to various diagnostic features in certain
+	  RMI4 touch sensors.
diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile
index 95c00a7..0bafc85 100644
--- a/drivers/input/rmi4/Makefile
+++ b/drivers/input/rmi4/Makefile
@@ -7,6 +7,7 @@ rmi_core-$(CONFIG_RMI4_2D_SENSOR) += rmi_2d_sensor.o
 rmi_core-$(CONFIG_RMI4_F11) += rmi_f11.o
 rmi_core-$(CONFIG_RMI4_F12) += rmi_f12.o
 rmi_core-$(CONFIG_RMI4_F30) += rmi_f30.o
+rmi_core-$(CONFIG_RMI4_F54) += rmi_f54.o
 
 # Transports
 obj-$(CONFIG_RMI4_I2C) += rmi_i2c.o
diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
index b368b05..3aedc65 100644
--- a/drivers/input/rmi4/rmi_bus.c
+++ b/drivers/input/rmi4/rmi_bus.c
@@ -315,6 +315,9 @@ static struct rmi_function_handler *fn_handlers[] = {
 #ifdef CONFIG_RMI4_F30
 	&rmi_f30_handler,
 #endif
+#ifdef CONFIG_RMI4_F54
+	&rmi_f54_handler,
+#endif
 };
 
 static void __rmi_unregister_function_handlers(int start_idx)
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index 6e140fa..8dfbebe 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -102,4 +102,5 @@ extern struct rmi_function_handler rmi_f01_handler;
 extern struct rmi_function_handler rmi_f11_handler;
 extern struct rmi_function_handler rmi_f12_handler;
 extern struct rmi_function_handler rmi_f30_handler;
+extern struct rmi_function_handler rmi_f54_handler;
 #endif
diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
new file mode 100644
index 0000000..bd06788
--- /dev/null
+++ b/drivers/input/rmi4/rmi_f54.c
@@ -0,0 +1,743 @@
+/*
+ * Copyright (c) 2012-2015 Synaptics Incorporated
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/rmi.h>
+#include <linux/input.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-ioctl.h>
+#include <media/videobuf2-v4l2.h>
+#include <media/videobuf2-vmalloc.h>
+#include "rmi_driver.h"
+
+#define F54_NAME		"rmi4_f54"
+
+/* F54 data offsets */
+#define F54_REPORT_DATA_OFFSET  3
+#define F54_FIFO_OFFSET         1
+#define F54_NUM_TX_OFFSET       1
+#define F54_NUM_RX_OFFSET       0
+
+/* F54 commands */
+#define F54_GET_REPORT          1
+#define F54_FORCE_CAL           2
+
+/* Fixed sizes of reports */
+#define F54_QUERY_LEN			27
+#define F54_FULL_RAW_CAP_MIN_MAX_SIZE   4
+#define F54_HIGH_RESISTANCE_SIZE(rx, tx) \
+					(2 * ((rx) * (tx) + (rx) + (tx) + 3))
+#define F54_MAX_REPORT_SIZE(rx, tx)	F54_HIGH_RESISTANCE_SIZE((rx), (tx))
+
+/* F54 capabilities */
+#define F54_CAP_BASELINE	(1 << 2)
+#define F54_CAP_IMAGE8		(1 << 3)
+#define F54_CAP_IMAGE16		(1 << 6)
+
+enum f54_report_type {
+	F54_REPORT_NONE = 0,
+	F54_8BIT_IMAGE = 1,
+	F54_16BIT_IMAGE = 2,
+	F54_RAW_16BIT_IMAGE = 3,
+	F54_HIGH_RESISTANCE = 4,
+	F54_TX_TO_TX_SHORT = 5,
+	F54_RX_TO_RX1 = 7,
+	F54_TRUE_BASELINE = 9,
+	F54_FULL_RAW_CAP_MIN_MAX = 13,
+	F54_RX_OPENS1 = 14,
+	F54_TX_OPEN = 15,
+	F54_TX_TO_GROUND = 16,
+	F54_RX_TO_RX2 = 17,
+	F54_RX_OPENS2 = 18,
+	F54_FULL_RAW_CAP = 19,
+	F54_FULL_RAW_CAP_RX_COUPLING_COMP = 20,
+	F54_MAX_REPORT_TYPE,
+};
+
+struct f54_data {
+	struct rmi_function *fn;
+
+	u8 qry[F54_QUERY_LEN];
+	u8 num_rx_electrodes;
+	u8 num_tx_electrodes;
+	u8 capabilities;
+	u16 clock_rate;
+	u8 family;
+
+	enum f54_report_type report_type;
+	u8 *report_data;
+	int report_size;
+
+	bool is_busy;
+	struct mutex status_mutex;
+	struct mutex data_mutex;
+
+	struct workqueue_struct *workqueue;
+	struct delayed_work work;
+	unsigned long timeout;
+
+	struct completion cmd_done;
+
+	/* V4L2 support */
+	struct v4l2_device v4l2;
+	struct v4l2_pix_format format;
+	struct video_device vdev;
+	struct vb2_queue queue;
+	struct mutex lock;
+	int input;
+	enum f54_report_type inputs[F54_MAX_REPORT_TYPE];
+};
+
+/*
+ * Basic checks on report_type to ensure we write a valid type
+ * to the sensor.
+ */
+static bool is_f54_report_type_valid(struct f54_data *f54,
+				     enum f54_report_type reptype)
+{
+	switch (reptype) {
+	case F54_8BIT_IMAGE:
+		return f54->capabilities & F54_CAP_IMAGE8;
+	case F54_16BIT_IMAGE:
+	case F54_RAW_16BIT_IMAGE:
+		return f54->capabilities & F54_CAP_IMAGE16;
+	case F54_TRUE_BASELINE:
+		return f54->capabilities & F54_CAP_IMAGE16;
+	case F54_HIGH_RESISTANCE:
+	case F54_TX_TO_TX_SHORT:
+	case F54_RX_TO_RX1:
+	case F54_FULL_RAW_CAP_MIN_MAX:
+	case F54_RX_OPENS1:
+	case F54_TX_OPEN:
+	case F54_TX_TO_GROUND:
+	case F54_RX_TO_RX2:
+	case F54_RX_OPENS2:
+	case F54_FULL_RAW_CAP:
+	case F54_FULL_RAW_CAP_RX_COUPLING_COMP:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static enum f54_report_type rmi_f54_get_reptype(struct f54_data *f54,
+						unsigned int i)
+{
+	if (i > F54_MAX_REPORT_TYPE)
+		return F54_REPORT_NONE;
+
+	return f54->inputs[i];
+}
+
+static void rmi_f54_create_input_map(struct f54_data *f54)
+{
+	int i = 0;
+	enum f54_report_type reptype;
+
+	for (reptype = 1; reptype < F54_MAX_REPORT_TYPE; reptype++) {
+		if (!is_f54_report_type_valid(f54, reptype))
+			continue;
+
+		f54->inputs[i++] = reptype;
+	}
+
+	/* Remaining values are zero via kzalloc */
+}
+
+static int rmi_f54_request_report(struct rmi_function *fn, u8 report_type)
+{
+	struct f54_data *f54 = dev_get_drvdata(&fn->dev);
+	struct rmi_device *rmi_dev = fn->rmi_dev;
+	int error;
+
+	/* Write Report Type into F54_AD_Data0 */
+	if (f54->report_type != report_type) {
+		error = rmi_write(rmi_dev, f54->fn->fd.data_base_addr,
+				  report_type);
+		if (error)
+			return error;
+		f54->report_type = report_type;
+	}
+
+	/*
+	 * Small delay after disabling interrupts to avoid race condition
+	 * in firmare. This value is a bit higher than absolutely necessary.
+	 * Should be removed once issue is resolved in firmware.
+	 */
+	usleep_range(2000, 3000);
+
+	mutex_lock(&f54->data_mutex);
+
+	error = rmi_write(rmi_dev, fn->fd.command_base_addr, F54_GET_REPORT);
+	if (error < 0)
+		return error;
+
+	init_completion(&f54->cmd_done);
+
+	f54->is_busy = 1;
+	f54->timeout = jiffies + msecs_to_jiffies(100);
+
+	queue_delayed_work(f54->workqueue, &f54->work, 0);
+
+	mutex_unlock(&f54->data_mutex);
+
+	return 0;
+}
+
+static int rmi_f54_get_report_size(struct f54_data *f54)
+{
+	u8 rx = f54->num_rx_electrodes ? : f54->num_rx_electrodes;
+	u8 tx = f54->num_tx_electrodes ? : f54->num_tx_electrodes;
+	int size;
+
+	switch (rmi_f54_get_reptype(f54, f54->input)) {
+	case F54_8BIT_IMAGE:
+		size = rx * tx;
+		break;
+	case F54_16BIT_IMAGE:
+	case F54_RAW_16BIT_IMAGE:
+	case F54_TRUE_BASELINE:
+	case F54_FULL_RAW_CAP:
+	case F54_FULL_RAW_CAP_RX_COUPLING_COMP:
+		size = 2 * rx * tx;
+		break;
+	case F54_HIGH_RESISTANCE:
+		size = F54_HIGH_RESISTANCE_SIZE(rx, tx);
+		break;
+	case F54_FULL_RAW_CAP_MIN_MAX:
+		size = F54_FULL_RAW_CAP_MIN_MAX_SIZE;
+		break;
+	case F54_TX_TO_TX_SHORT:
+	case F54_TX_OPEN:
+	case F54_TX_TO_GROUND:
+		size =  (tx + 7) / 8;
+		break;
+	case F54_RX_TO_RX1:
+	case F54_RX_OPENS1:
+		if (rx < tx)
+			size = 2 * rx * rx;
+		else
+			size = 2 * rx * tx;
+		break;
+	case F54_RX_TO_RX2:
+	case F54_RX_OPENS2:
+		if (rx <= tx)
+			size = 0;
+		else
+			size = 2 * rx * (rx - tx);
+		break;
+	default:
+		size = 0;
+	}
+
+	return size;
+}
+
+static const struct v4l2_file_operations rmi_f54_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 int rmi_f54_queue_setup(struct vb2_queue *q,
+			       unsigned int *nbuffers, unsigned int *nplanes,
+			       unsigned int sizes[], void *alloc_ctxs[])
+{
+	struct f54_data *f54 = q->drv_priv;
+
+	if (*nplanes)
+		return sizes[0] < rmi_f54_get_report_size(f54) ? -EINVAL : 0;
+
+	*nplanes = 1;
+	sizes[0] = rmi_f54_get_report_size(f54);
+
+	return 0;
+}
+
+static void rmi_f54_buffer_queue(struct vb2_buffer *vb)
+{
+	struct f54_data *f54 = vb2_get_drv_priv(vb->vb2_queue);
+	u16 *ptr;
+	enum vb2_buffer_state state;
+	enum f54_report_type reptype;
+	int ret;
+
+	mutex_lock(&f54->status_mutex);
+
+	reptype = rmi_f54_get_reptype(f54, f54->input);
+	if (reptype == F54_REPORT_NONE) {
+		state = VB2_BUF_STATE_ERROR;
+		goto done;
+	}
+
+	if (f54->is_busy) {
+		state = VB2_BUF_STATE_ERROR;
+		goto done;
+	}
+
+	ret = rmi_f54_request_report(f54->fn, reptype);
+	if (ret) {
+		dev_err(&f54->fn->dev, "Error requesting F54 report\n");
+		state = VB2_BUF_STATE_ERROR;
+		goto done;
+	}
+
+	/* get frame data */
+	mutex_lock(&f54->data_mutex);
+
+	while (f54->is_busy) {
+		mutex_unlock(&f54->data_mutex);
+		if (!wait_for_completion_timeout(&f54->cmd_done,
+						 msecs_to_jiffies(1000))) {
+			dev_err(&f54->fn->dev, "Timed out\n");
+			state = VB2_BUF_STATE_ERROR;
+			goto done;
+		}
+		mutex_lock(&f54->data_mutex);
+	}
+
+	ptr = vb2_plane_vaddr(vb, 0);
+	if (!ptr) {
+		dev_err(&f54->fn->dev, "Error acquiring frame ptr\n");
+		state = VB2_BUF_STATE_ERROR;
+		goto data_done;
+	}
+
+	memcpy(ptr, f54->report_data, f54->report_size);
+	vb2_set_plane_payload(vb, 0, rmi_f54_get_report_size(f54));
+	state = VB2_BUF_STATE_DONE;
+
+data_done:
+	mutex_unlock(&f54->data_mutex);
+done:
+	vb2_buffer_done(vb, state);
+	mutex_unlock(&f54->status_mutex);
+}
+
+/* V4L2 structures */
+static const struct vb2_ops rmi_f54_queue_ops = {
+	.queue_setup            = rmi_f54_queue_setup,
+	.buf_queue              = rmi_f54_buffer_queue,
+	.wait_prepare           = vb2_ops_wait_prepare,
+	.wait_finish            = vb2_ops_wait_finish,
+};
+
+static const struct vb2_queue rmi_f54_queue = {
+	.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
+	.io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF | VB2_READ,
+	.buf_struct_size = sizeof(struct vb2_buffer),
+	.ops = &rmi_f54_queue_ops,
+	.mem_ops = &vb2_vmalloc_memops,
+	.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC,
+	.min_buffers_needed = 1,
+};
+
+static int rmi_f54_vidioc_querycap(struct file *file, void *priv,
+				   struct v4l2_capability *cap)
+{
+	struct f54_data *f54 = video_drvdata(file);
+
+	strlcpy(cap->driver, F54_NAME, sizeof(cap->driver));
+	strlcpy(cap->card, SYNAPTICS_INPUT_DEVICE_NAME, sizeof(cap->card));
+	strlcpy(cap->bus_info, dev_name(&f54->fn->dev), sizeof(cap->bus_info));
+
+	return 0;
+}
+
+static const char *rmi_f54_reptype_str(enum f54_report_type reptype)
+{
+	switch (reptype) {
+	default:
+	case F54_REPORT_NONE: return "No report";
+	case F54_8BIT_IMAGE: return "8 bit image";
+	case F54_16BIT_IMAGE: return "16 bit image";
+	case F54_RAW_16BIT_IMAGE: return "Raw 16 bit image";
+	case F54_HIGH_RESISTANCE: return "High resistance";
+	case F54_TX_TO_TX_SHORT: return "TX to TX short";
+	case F54_RX_TO_RX1: return "RX to RX1";
+	case F54_TRUE_BASELINE: return "True baseline";
+	case F54_FULL_RAW_CAP_MIN_MAX: return "Full raw cap min max";
+	case F54_RX_OPENS1: return "RX open S1";
+	case F54_TX_OPEN: return "TX open";
+	case F54_TX_TO_GROUND: return "TX to ground";
+	case F54_RX_TO_RX2: return "RX to RX2";
+	case F54_RX_OPENS2: return "RX Open S2";
+	case F54_FULL_RAW_CAP: return "Full raw cap";
+	case F54_FULL_RAW_CAP_RX_COUPLING_COMP:
+		return "Full raw cap RX coupling comp";
+	case F54_MAX_REPORT_TYPE: return "Max report type";
+	}
+}
+
+static int rmi_f54_vidioc_enum_input(struct file *file, void *priv,
+				     struct v4l2_input *i)
+{
+	struct f54_data *f54 = video_drvdata(file);
+	enum f54_report_type reptype;
+
+	reptype = rmi_f54_get_reptype(f54, i->index);
+	if (reptype == F54_REPORT_NONE)
+		return -EINVAL;
+
+	i->type = V4L2_INPUT_TYPE_TOUCH_SENSOR;
+	strlcpy(i->name, rmi_f54_reptype_str(reptype), sizeof(i->name));
+	return 0;
+}
+
+static int rmi_f54_set_input(struct f54_data *f54, unsigned int i)
+{
+	struct v4l2_pix_format *f = &f54->format;
+	enum f54_report_type reptype;
+
+	reptype = rmi_f54_get_reptype(f54, i);
+	if (reptype == F54_REPORT_NONE)
+		return -EINVAL;
+
+	f54->input = i;
+
+	f->width = f54->num_rx_electrodes;
+	f->height = f54->num_tx_electrodes;
+	f->pixelformat = V4L2_PIX_FMT_YS16;
+	f->field = V4L2_FIELD_NONE;
+	f->colorspace = V4L2_COLORSPACE_RAW;
+	f->bytesperline = f->width * sizeof(u16);
+	f->sizeimage = f->width * f->height * sizeof(u16);
+
+	return 0;
+}
+
+static int rmi_f54_vidioc_s_input(struct file *file, void *priv, unsigned int i)
+{
+	return rmi_f54_set_input(video_drvdata(file), i);
+}
+
+static int rmi_f54_vidioc_g_input(struct file *file, void *priv,
+				  unsigned int *i)
+{
+	struct f54_data *f54 = video_drvdata(file);
+
+	*i = f54->input;
+
+	return 0;
+}
+
+static int rmi_f54_vidioc_fmt(struct file *file, void *priv,
+			      struct v4l2_format *f)
+{
+	struct f54_data *f54 = video_drvdata(file);
+
+	f->fmt.pix = f54->format;
+
+	return 0;
+}
+
+static int rmi_f54_vidioc_enum_fmt(struct file *file, void *priv,
+				   struct v4l2_fmtdesc *fmt)
+{
+	if (fmt->index > 0 || fmt->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
+	fmt->pixelformat = V4L2_PIX_FMT_YS16;
+	return 0;
+}
+
+static int rmi_f54_vidioc_g_parm(struct file *file, void *fh,
+				 struct v4l2_streamparm *a)
+{
+	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
+	a->parm.capture.readbuffers = 1;
+	a->parm.capture.timeperframe.numerator = 1;
+	a->parm.capture.timeperframe.denominator = 10;
+	return 0;
+}
+
+static const struct v4l2_ioctl_ops rmi_f54_video_ioctl_ops = {
+	.vidioc_querycap	= rmi_f54_vidioc_querycap,
+
+	.vidioc_enum_fmt_vid_cap = rmi_f54_vidioc_enum_fmt,
+	.vidioc_s_fmt_vid_cap	= rmi_f54_vidioc_fmt,
+	.vidioc_g_fmt_vid_cap	= rmi_f54_vidioc_fmt,
+	.vidioc_try_fmt_vid_cap	= rmi_f54_vidioc_fmt,
+	.vidioc_g_parm		= rmi_f54_vidioc_g_parm,
+
+	.vidioc_enum_input	= rmi_f54_vidioc_enum_input,
+	.vidioc_g_input		= rmi_f54_vidioc_g_input,
+	.vidioc_s_input		= rmi_f54_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,
+};
+
+static const struct video_device rmi_f54_video_device = {
+	.name = "Synaptics RMI4",
+	.fops = &rmi_f54_video_fops,
+	.ioctl_ops = &rmi_f54_video_ioctl_ops,
+	.release = video_device_release_empty,
+	.device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
+		       V4L2_CAP_STREAMING,
+};
+
+struct rmi_f54_reports {
+	int start;
+	int size;
+};
+
+static struct rmi_f54_reports rmi_f54_standard_report[] = {
+	{ 0, 0 },           /* report size to be added in code */
+	{ 0, 0 },
+};
+
+static void rmi_f54_work(struct work_struct *work)
+{
+	struct f54_data *f54 = container_of(work, struct f54_data, work.work);
+	struct rmi_function *fn = f54->fn;
+	u8 fifo[2];
+	struct rmi_f54_reports *report;
+	int report_size;
+	u8 command;
+	u8 *data;
+	int error;
+
+	data = f54->report_data;
+	report_size = rmi_f54_get_report_size(f54);
+	if (report_size == 0) {
+		dev_err(&fn->dev, "Bad report size, report type=%d\n",
+				f54->report_type);
+		error = -EINVAL;
+		goto error;     /* retry won't help */
+	}
+	rmi_f54_standard_report[0].size = report_size;
+	report = rmi_f54_standard_report;
+
+	mutex_lock(&f54->data_mutex);
+
+	/*
+	 * Need to check if command has completed.
+	 * If not try again later.
+	 */
+	error = rmi_read(fn->rmi_dev, f54->fn->fd.command_base_addr,
+			 &command);
+	if (error) {
+		dev_err(&fn->dev, "Failed to read back command\n");
+		goto error;
+	}
+	if (command & F54_GET_REPORT) {
+		if (time_after(jiffies, f54->timeout)) {
+			dev_err(&fn->dev, "Get report command timed out\n");
+			error = -ETIMEDOUT;
+		}
+		report_size = 0;
+		goto error;
+	}
+
+	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "Get report command completed, reading data\n");
+
+	report_size = 0;
+	for (; report->size; report++) {
+		fifo[0] = report->start & 0xff;
+		fifo[1] = (report->start >> 8) & 0xff;
+		error = rmi_write_block(fn->rmi_dev,
+					fn->fd.data_base_addr + F54_FIFO_OFFSET,
+					fifo, sizeof(fifo));
+		if (error) {
+			dev_err(&fn->dev, "Failed to set fifo start offset\n");
+			goto abort;
+		}
+
+		error = rmi_read_block(fn->rmi_dev, fn->fd.data_base_addr +
+				       F54_REPORT_DATA_OFFSET, data,
+				       report->size);
+		if (error) {
+			dev_err(&fn->dev, "%s: read [%d bytes] returned %d\n",
+				__func__, report->size, error);
+			goto abort;
+		}
+		data += report->size;
+		report_size += report->size;
+	}
+
+abort:
+	f54->report_size = error ? 0 : report_size;
+error:
+	if (error)
+		report_size = 0;
+
+	if (report_size == 0 && !error) {
+		queue_delayed_work(f54->workqueue, &f54->work,
+				   msecs_to_jiffies(1));
+	} else {
+		f54->is_busy = false;
+		complete(&f54->cmd_done);
+	}
+
+	mutex_unlock(&f54->data_mutex);
+}
+
+static int rmi_f54_attention(struct rmi_function *fn, unsigned long *irqbits)
+{
+	return 0;
+}
+
+static int rmi_f54_config(struct rmi_function *fn)
+{
+	struct rmi_driver *drv = fn->rmi_dev->driver;
+
+	drv->set_irq_bits(fn->rmi_dev, fn->irq_mask);
+
+	return 0;
+}
+
+static int rmi_f54_detect(struct rmi_function *fn)
+{
+	int error;
+	struct f54_data *f54;
+
+	f54 = dev_get_drvdata(&fn->dev);
+
+	error = rmi_read_block(fn->rmi_dev, fn->fd.query_base_addr,
+			       &f54->qry, sizeof(f54->qry));
+	if (error) {
+		dev_err(&fn->dev, "%s: Failed to query F54 properties\n",
+			__func__);
+		return error;
+	}
+
+	f54->num_rx_electrodes = f54->qry[0];
+	f54->num_tx_electrodes = f54->qry[1];
+	f54->capabilities = f54->qry[2];
+	f54->clock_rate = f54->qry[3] | (f54->qry[4] << 8);
+	f54->family = f54->qry[5];
+
+	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "F54 num_rx_electrodes: %d\n",
+		f54->num_rx_electrodes);
+	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "F54 num_tx_electrodes: %d\n",
+		f54->num_tx_electrodes);
+	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "F54 capabilities: 0x%x\n",
+		f54->capabilities);
+	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "F54 clock rate: 0x%x\n",
+		f54->clock_rate);
+	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "F54 family: 0x%x\n",
+		f54->family);
+
+	f54->is_busy = false;
+
+	return 0;
+}
+
+static int rmi_f54_probe(struct rmi_function *fn)
+{
+	struct f54_data *f54;
+	int ret;
+	u8 rx, tx;
+
+	f54 = devm_kzalloc(&fn->dev, sizeof(struct f54_data), GFP_KERNEL);
+	if (!f54)
+		return -ENOMEM;
+
+	f54->fn = fn;
+	dev_set_drvdata(&fn->dev, f54);
+
+	ret = rmi_f54_detect(fn);
+	if (ret)
+		return ret;
+
+	mutex_init(&f54->data_mutex);
+	mutex_init(&f54->status_mutex);
+
+	rx = f54->num_rx_electrodes;
+	tx = f54->num_tx_electrodes;
+	f54->report_data = devm_kzalloc(&fn->dev,
+					F54_MAX_REPORT_SIZE(rx, tx),
+					GFP_KERNEL);
+	if (f54->report_data == NULL)
+		return -ENOMEM;
+
+	INIT_DELAYED_WORK(&f54->work, rmi_f54_work);
+
+	f54->workqueue = create_singlethread_workqueue("rmi4-poller");
+	if (!f54->workqueue)
+		return -ENOMEM;
+
+	rmi_f54_create_input_map(f54);
+
+	/* register video device */
+	strlcpy(f54->v4l2.name, F54_NAME, sizeof(f54->v4l2.name));
+	ret = v4l2_device_register(&fn->dev, &f54->v4l2);
+	if (ret) {
+		dev_err(&fn->dev, "Unable to register video dev.\n");
+		goto remove_wq;
+	}
+
+	/* initialize the queue */
+	mutex_init(&f54->lock);
+	f54->queue = rmi_f54_queue;
+	f54->queue.drv_priv = f54;
+	f54->queue.lock = &f54->lock;
+
+	ret = vb2_queue_init(&f54->queue);
+	if (ret)
+		goto remove_v4l2;
+
+	f54->vdev = rmi_f54_video_device;
+	f54->vdev.v4l2_dev = &f54->v4l2;
+	f54->vdev.lock = &f54->lock;
+	f54->vdev.vfl_dir = VFL_DIR_RX;
+	f54->vdev.queue = &f54->queue;
+	video_set_drvdata(&f54->vdev, f54);
+
+	ret = video_register_device(&f54->vdev, VFL_TYPE_TOUCH_SENSOR, -1);
+	if (ret) {
+		dev_err(&fn->dev, "Unable to register video subdevice.");
+		goto remove_v4l2;
+	}
+
+	return 0;
+
+remove_v4l2:
+	v4l2_device_unregister(&f54->v4l2);
+remove_wq:
+	cancel_delayed_work_sync(&f54->work);
+	flush_workqueue(f54->workqueue);
+	destroy_workqueue(f54->workqueue);
+	return ret;
+}
+
+static void rmi_f54_remove(struct rmi_function *fn)
+{
+	struct f54_data *f54 = dev_get_drvdata(&fn->dev);
+
+	video_unregister_device(&f54->vdev);
+	v4l2_device_unregister(&f54->v4l2);
+}
+
+struct rmi_function_handler rmi_f54_handler = {
+	.driver = {
+		.name = F54_NAME,
+	},
+	.func = 0x54,
+	.probe = rmi_f54_probe,
+	.config = rmi_f54_config,
+	.attention = rmi_f54_attention,
+	.remove = rmi_f54_remove,
+};
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/9] [media] Add signed 16-bit pixel format
  2016-06-17 14:16 ` [PATCH v4 1/9] [media] Add signed 16-bit pixel format Nick Dyer
@ 2016-06-20 11:00   ` Hans Verkuil
  2016-06-20 11:51     ` Nick Dyer
  0 siblings, 1 reply; 22+ messages in thread
From: Hans Verkuil @ 2016-06-20 11:00 UTC (permalink / raw)
  To: Nick Dyer, Dmitry Torokhov
  Cc: linux-input, linux-kernel, linux-media, Benjamin Tissoires,
	Benson Leung, Alan Bowens, Javier Martinez Canillas, Chris Healy,
	Henrik Rydberg, Andrew Duggan, James Chen, Dudley Du,
	Andrew de los Reyes, sheckylin, Peter Hutterer, Florian Echtler,
	mchehab

On 06/17/2016 04:16 PM, Nick Dyer wrote:
> This will be used for output of raw touch delta data. This format is
> used by Atmel maXTouch (atmel_mxt_ts) and also Synaptics RMI4.
> 
> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
> ---
>  Documentation/DocBook/media/v4l/pixfmt-ys16.xml | 79 +++++++++++++++++++++++++
>  Documentation/DocBook/media/v4l/pixfmt.xml      |  1 +
>  drivers/media/v4l2-core/v4l2-ioctl.c            |  1 +
>  include/uapi/linux/videodev2.h                  |  1 +
>  4 files changed, 82 insertions(+)
>  create mode 100644 Documentation/DocBook/media/v4l/pixfmt-ys16.xml
> 
> diff --git a/Documentation/DocBook/media/v4l/pixfmt-ys16.xml b/Documentation/DocBook/media/v4l/pixfmt-ys16.xml
> new file mode 100644
> index 0000000..f92d65e
> --- /dev/null
> +++ b/Documentation/DocBook/media/v4l/pixfmt-ys16.xml
> @@ -0,0 +1,79 @@
> +<refentry id="V4L2-PIX-FMT-YS16">
> +  <refmeta>
> +    <refentrytitle>V4L2_PIX_FMT_YS16 ('YS16')</refentrytitle>
> +    &manvol;
> +  </refmeta>
> +  <refnamediv>
> +    <refname><constant>V4L2_PIX_FMT_YS16</constant></refname>
> +    <refpurpose>Grey-scale image</refpurpose>
> +  </refnamediv>
> +  <refsect1>
> +    <title>Description</title>
> +
> +    <para>This is a signed grey-scale image with a depth of 16 bits per
> +pixel. The most significant byte is stored at higher memory addresses
> +(little-endian).</para>

This is too generic. I think something like V4L2_TOUCH_FMT_DELTA_S16 is much
more appropriate since this is neither luma (Y) data nor a picture in the
classic sense. Since we already use V4L2_SDR_FMT_* defines for software defined
radio formats, it makes sense to use V4L2_TOUCH_FMT_* for these touch panel
formats.

The description can be based around what you told here:

https://lkml.org/lkml/2016/5/27/278

It's also important that you clearly state what the delta is against. A delta
implies a difference from something, but what that something is isn't explained.

I'm sorry for being pedantic about this, but it should be possible to make an
application that can correctly interpret this data based on this format
description. Otherwise there would be no point in documenting this...

Regards,

	Hans

> +
> +    <example>
> +      <title><constant>V4L2_PIX_FMT_YS16</constant> 4 &times; 4
> +pixel image</title>
> +
> +      <formalpara>
> +	<title>Byte Order.</title>
> +	<para>Each cell is one byte.
> +	  <informaltable frame="none">
> +	    <tgroup cols="9" align="center">
> +	      <colspec align="left" colwidth="2*" />
> +	      <tbody valign="top">
> +		<row>
> +		  <entry>start&nbsp;+&nbsp;0:</entry>
> +		  <entry>Y'<subscript>00low</subscript></entry>
> +		  <entry>Y'<subscript>00high</subscript></entry>
> +		  <entry>Y'<subscript>01low</subscript></entry>
> +		  <entry>Y'<subscript>01high</subscript></entry>
> +		  <entry>Y'<subscript>02low</subscript></entry>
> +		  <entry>Y'<subscript>02high</subscript></entry>
> +		  <entry>Y'<subscript>03low</subscript></entry>
> +		  <entry>Y'<subscript>03high</subscript></entry>
> +		</row>
> +		<row>
> +		  <entry>start&nbsp;+&nbsp;8:</entry>
> +		  <entry>Y'<subscript>10low</subscript></entry>
> +		  <entry>Y'<subscript>10high</subscript></entry>
> +		  <entry>Y'<subscript>11low</subscript></entry>
> +		  <entry>Y'<subscript>11high</subscript></entry>
> +		  <entry>Y'<subscript>12low</subscript></entry>
> +		  <entry>Y'<subscript>12high</subscript></entry>
> +		  <entry>Y'<subscript>13low</subscript></entry>
> +		  <entry>Y'<subscript>13high</subscript></entry>
> +		</row>
> +		<row>
> +		  <entry>start&nbsp;+&nbsp;16:</entry>
> +		  <entry>Y'<subscript>20low</subscript></entry>
> +		  <entry>Y'<subscript>20high</subscript></entry>
> +		  <entry>Y'<subscript>21low</subscript></entry>
> +		  <entry>Y'<subscript>21high</subscript></entry>
> +		  <entry>Y'<subscript>22low</subscript></entry>
> +		  <entry>Y'<subscript>22high</subscript></entry>
> +		  <entry>Y'<subscript>23low</subscript></entry>
> +		  <entry>Y'<subscript>23high</subscript></entry>
> +		</row>
> +		<row>
> +		  <entry>start&nbsp;+&nbsp;24:</entry>
> +		  <entry>Y'<subscript>30low</subscript></entry>
> +		  <entry>Y'<subscript>30high</subscript></entry>
> +		  <entry>Y'<subscript>31low</subscript></entry>
> +		  <entry>Y'<subscript>31high</subscript></entry>
> +		  <entry>Y'<subscript>32low</subscript></entry>
> +		  <entry>Y'<subscript>32high</subscript></entry>
> +		  <entry>Y'<subscript>33low</subscript></entry>
> +		  <entry>Y'<subscript>33high</subscript></entry>
> +		</row>
> +	      </tbody>
> +	    </tgroup>
> +	  </informaltable>
> +	</para>
> +      </formalpara>
> +    </example>
> +  </refsect1>
> +</refentry>
> diff --git a/Documentation/DocBook/media/v4l/pixfmt.xml b/Documentation/DocBook/media/v4l/pixfmt.xml
> index 5a08aee..f3e3e6d 100644
> --- a/Documentation/DocBook/media/v4l/pixfmt.xml
> +++ b/Documentation/DocBook/media/v4l/pixfmt.xml
> @@ -1619,6 +1619,7 @@ information.</para>
>      &sub-y12;
>      &sub-y10b;
>      &sub-y16;
> +    &sub-ys16;
>      &sub-y16-be;
>      &sub-y8i;
>      &sub-y12i;
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 28e5be2..ecf7e0b 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1164,6 +1164,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
>  	case V4L2_PIX_FMT_Y10:		descr = "10-bit Greyscale"; break;
>  	case V4L2_PIX_FMT_Y12:		descr = "12-bit Greyscale"; break;
>  	case V4L2_PIX_FMT_Y16:		descr = "16-bit Greyscale"; break;
> +	case V4L2_PIX_FMT_YS16:		descr = "16-bit Greyscale (Signed)"; break;
>  	case V4L2_PIX_FMT_Y16_BE:	descr = "16-bit Greyscale BE"; break;
>  	case V4L2_PIX_FMT_Y10BPACK:	descr = "10-bit Greyscale (Packed)"; break;
>  	case V4L2_PIX_FMT_PAL8:		descr = "8-bit Palette"; break;
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 8f95191..e0125cf 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -493,6 +493,7 @@ struct v4l2_pix_format {
>  #define V4L2_PIX_FMT_Y12     v4l2_fourcc('Y', '1', '2', ' ') /* 12  Greyscale     */
>  #define V4L2_PIX_FMT_Y16     v4l2_fourcc('Y', '1', '6', ' ') /* 16  Greyscale     */
>  #define V4L2_PIX_FMT_Y16_BE  v4l2_fourcc_be('Y', '1', '6', ' ') /* 16  Greyscale BE  */
> +#define V4L2_PIX_FMT_YS16    v4l2_fourcc('Y', 'S', '1', '6') /* signed 16-bit Greyscale */
>  
>  /* Grey bit-packed formats */
>  #define V4L2_PIX_FMT_Y10BPACK    v4l2_fourcc('Y', '1', '0', 'B') /* 10  Greyscale bit-packed */
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/9] [media] Add signed 16-bit pixel format
  2016-06-20 11:00   ` Hans Verkuil
@ 2016-06-20 11:51     ` Nick Dyer
  0 siblings, 0 replies; 22+ messages in thread
From: Nick Dyer @ 2016-06-20 11:51 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Dmitry Torokhov, linux-input, linux-kernel, linux-media,
	Benjamin Tissoires, Benson Leung, Alan Bowens,
	Javier Martinez Canillas, Chris Healy, Henrik Rydberg,
	Andrew Duggan, James Chen, Dudley Du, Andrew de los Reyes,
	sheckylin, Peter Hutterer, Florian Echtler, mchehab

Hi Hans-

On 20/06/2016 12:00, Hans Verkuil wrote:
> On 06/17/2016 04:16 PM, Nick Dyer wrote:
>> This will be used for output of raw touch delta data. This format is
>> used by Atmel maXTouch (atmel_mxt_ts) and also Synaptics RMI4.
>>
>> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
>> ---
>>  Documentation/DocBook/media/v4l/pixfmt-ys16.xml | 79 +++++++++++++++++++++++++
>>  Documentation/DocBook/media/v4l/pixfmt.xml      |  1 +
>>  drivers/media/v4l2-core/v4l2-ioctl.c            |  1 +
>>  include/uapi/linux/videodev2.h                  |  1 +
>>  4 files changed, 82 insertions(+)
>>  create mode 100644 Documentation/DocBook/media/v4l/pixfmt-ys16.xml
>>
>> diff --git a/Documentation/DocBook/media/v4l/pixfmt-ys16.xml b/Documentation/DocBook/media/v4l/pixfmt-ys16.xml
>> new file mode 100644
>> index 0000000..f92d65e
>> --- /dev/null
>> +++ b/Documentation/DocBook/media/v4l/pixfmt-ys16.xml
>> @@ -0,0 +1,79 @@
>> +<refentry id="V4L2-PIX-FMT-YS16">
>> +  <refmeta>
>> +    <refentrytitle>V4L2_PIX_FMT_YS16 ('YS16')</refentrytitle>
>> +    &manvol;
>> +  </refmeta>
>> +  <refnamediv>
>> +    <refname><constant>V4L2_PIX_FMT_YS16</constant></refname>
>> +    <refpurpose>Grey-scale image</refpurpose>
>> +  </refnamediv>
>> +  <refsect1>
>> +    <title>Description</title>
>> +
>> +    <para>This is a signed grey-scale image with a depth of 16 bits per
>> +pixel. The most significant byte is stored at higher memory addresses
>> +(little-endian).</para>
> 
> This is too generic. I think something like V4L2_TOUCH_FMT_DELTA_S16 is much
> more appropriate since this is neither luma (Y) data nor a picture in the
> classic sense. Since we already use V4L2_SDR_FMT_* defines for software defined
> radio formats, it makes sense to use V4L2_TOUCH_FMT_* for these touch panel
> formats.

OK, that sounds sensible to me.

> The description can be based around what you told here:
> 
> https://lkml.org/lkml/2016/5/27/278
> 
> It's also important that you clearly state what the delta is against. A delta
> implies a difference from something, but what that something is isn't explained.

To explain, in PCAP capacitive touch, a touch input is determined by
comparing the raw capacitance measurement to a no-touch reference (or
"baseline") measurement. Hence:

Delta = Raw - Reference

The reference measurement takes account of variations in the capacitance
characteristics across the nodes of the touch sensor, for example
manufacturing irregularities or edge effects.

I'll put something about this in the docs.

> I'm sorry for being pedantic about this, but it should be possible to make an
> application that can correctly interpret this data based on this format
> description. Otherwise there would be no point in documenting this...

No problem: thanks for your clear feedback.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 2/9] [media] v4l2-core: Add VFL_TYPE_TOUCH_SENSOR
  2016-06-17 14:16 ` [PATCH v4 2/9] [media] v4l2-core: Add VFL_TYPE_TOUCH_SENSOR Nick Dyer
@ 2016-06-20 12:00   ` Hans Verkuil
  2016-06-22 11:48     ` Florian Echtler
  0 siblings, 1 reply; 22+ messages in thread
From: Hans Verkuil @ 2016-06-20 12:00 UTC (permalink / raw)
  To: Nick Dyer, Dmitry Torokhov
  Cc: linux-input, linux-kernel, linux-media, Benjamin Tissoires,
	Benson Leung, Alan Bowens, Javier Martinez Canillas, Chris Healy,
	Henrik Rydberg, Andrew Duggan, James Chen, Dudley Du,
	Andrew de los Reyes, sheckylin, Peter Hutterer, Florian Echtler,
	mchehab

On 06/17/2016 04:16 PM, Nick Dyer wrote:
> Some touch controllers send out raw touch data in a similar way to a
> greyscale frame grabber. Add a new device type for these devices.
> 
> Use a new device prefix v4l-touch for these devices, to stop generic
> capture software from treating them as webcams.
> 
> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
> ---
>  drivers/input/touchscreen/sur40.c    |  4 ++--
>  drivers/media/v4l2-core/v4l2-dev.c   | 13 ++++++++++---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 15 ++++++++++-----
>  include/media/v4l2-dev.h             |  3 ++-
>  include/uapi/linux/videodev2.h       |  1 +

The new interface also needs to be documented in the media DocBook: section 4
describes all the interfaces, so a new one should be added here.

Sorry, just realized this.

I recommend that you grep for swradio:

git grep swradio Documentation/DocBook/media

and add a line there for v4l-touch as well. It also looks like a new interface
define is needed as a media type.

Regards,

	Hans

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 8/9] Input: atmel_mxt_ts - add support for reference data
  2016-06-17 14:16 ` [PATCH v4 8/9] Input: atmel_mxt_ts - add support for reference data Nick Dyer
@ 2016-06-20 16:09   ` Hans Verkuil
  2016-06-20 16:18     ` Nick Dyer
  0 siblings, 1 reply; 22+ messages in thread
From: Hans Verkuil @ 2016-06-20 16:09 UTC (permalink / raw)
  To: Nick Dyer, Dmitry Torokhov
  Cc: linux-input, linux-kernel, linux-media, Benjamin Tissoires,
	Benson Leung, Alan Bowens, Javier Martinez Canillas, Chris Healy,
	Henrik Rydberg, Andrew Duggan, James Chen, Dudley Du,
	Andrew de los Reyes, sheckylin, Peter Hutterer, Florian Echtler,
	mchehab

On 06/17/2016 04:16 PM, Nick Dyer wrote:
> There are different datatypes available from a maXTouch chip. Add
> support to retrieve reference data as well.
> 
> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
> ---
>  drivers/input/touchscreen/atmel_mxt_ts.c | 58 ++++++++++++++++++++++++++++----
>  1 file changed, 51 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 5281325d..bb01fb0 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -135,6 +135,7 @@ struct t9_range {
>  /* MXT_DEBUG_DIAGNOSTIC_T37 */
>  #define MXT_DIAGNOSTIC_PAGEUP	0x01
>  #define MXT_DIAGNOSTIC_DELTAS	0x10
> +#define MXT_DIAGNOSTIC_REFS	0x11
>  #define MXT_DIAGNOSTIC_SIZE	128
>  
>  #define MXT_FAMILY_1386			160
> @@ -249,6 +250,12 @@ struct mxt_dbg {
>  	int input;
>  };
>  
> +enum v4l_dbg_inputs {
> +	MXT_V4L_INPUT_DELTAS,
> +	MXT_V4L_INPUT_REFS,
> +	MXT_V4L_INPUT_MAX,
> +};
> +
>  static const struct v4l2_file_operations mxt_video_fops = {
>  	.owner = THIS_MODULE,
>  	.open = v4l2_fh_open,
> @@ -2273,6 +2280,7 @@ static void mxt_buffer_queue(struct vb2_buffer *vb)
>  	struct mxt_data *data = vb2_get_drv_priv(vb->vb2_queue);
>  	u16 *ptr;
>  	int ret;
> +	u8 mode;
>  
>  	ptr = vb2_plane_vaddr(vb, 0);
>  	if (!ptr) {
> @@ -2280,7 +2288,18 @@ static void mxt_buffer_queue(struct vb2_buffer *vb)
>  		goto fault;
>  	}
>  
> -	ret = mxt_read_diagnostic_debug(data, MXT_DIAGNOSTIC_DELTAS, ptr);
> +	switch (data->dbg.input) {
> +	case MXT_V4L_INPUT_DELTAS:
> +	default:
> +		mode = MXT_DIAGNOSTIC_DELTAS;
> +		break;
> +
> +	case MXT_V4L_INPUT_REFS:
> +		mode = MXT_DIAGNOSTIC_REFS;
> +		break;
> +	}
> +
> +	ret = mxt_read_diagnostic_debug(data, mode, ptr);
>  	if (ret)
>  		goto fault;
>  
> @@ -2325,11 +2344,20 @@ static int mxt_vidioc_querycap(struct file *file, void *priv,
>  static int mxt_vidioc_enum_input(struct file *file, void *priv,
>  				   struct v4l2_input *i)
>  {
> -	if (i->index > 0)
> +	if (i->index >= MXT_V4L_INPUT_MAX)
>  		return -EINVAL;
>  
>  	i->type = V4L2_INPUT_TYPE_TOUCH_SENSOR;
> -	strlcpy(i->name, "Mutual References", sizeof(i->name));
> +
> +	switch (i->index) {
> +	case MXT_V4L_INPUT_REFS:
> +		strlcpy(i->name, "Mutual References", sizeof(i->name));
> +		break;
> +	case MXT_V4L_INPUT_DELTAS:
> +		strlcpy(i->name, "Mutual Deltas", sizeof(i->name));

I don't think this name is very clear. I have no idea how to interpret "Mutual"
in this context.

> +		break;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -2337,12 +2365,16 @@ static int mxt_set_input(struct mxt_data *data, unsigned int i)
>  {
>  	struct v4l2_pix_format *f = &data->dbg.format;
>  
> -	if (i > 0)
> +	if (i >= MXT_V4L_INPUT_MAX)
>  		return -EINVAL;
>  
> +	if (i == MXT_V4L_INPUT_DELTAS)
> +		f->pixelformat = V4L2_PIX_FMT_YS16;
> +	else
> +		f->pixelformat = V4L2_PIX_FMT_Y16;

You probably need a V4L2_TOUCH_FMT_U16 or something for this as well. It certainly
needs to be documented.

Regards,

	Hans

> +
>  	f->width = data->xy_switch ? data->ysize : data->xsize;
>  	f->height = data->xy_switch ? data->xsize : data->ysize;
> -	f->pixelformat = V4L2_PIX_FMT_YS16;
>  	f->field = V4L2_FIELD_NONE;
>  	f->colorspace = V4L2_COLORSPACE_RAW;
>  	f->bytesperline = f->width * sizeof(u16);
> @@ -2380,10 +2412,22 @@ static int mxt_vidioc_fmt(struct file *file, void *priv, struct v4l2_format *f)
>  static int mxt_vidioc_enum_fmt(struct file *file, void *priv,
>  				 struct v4l2_fmtdesc *fmt)
>  {
> -	if (fmt->index > 0 || fmt->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +	if (fmt->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +		return -EINVAL;
> +
> +	switch (fmt->index) {
> +	case 0:
> +		fmt->pixelformat = V4L2_PIX_FMT_Y16;
> +		break;
> +
> +	case 1:
> +		fmt->pixelformat = V4L2_PIX_FMT_YS16;
> +		break;
> +
> +	default:
>  		return -EINVAL;
> +	}
>  
> -	fmt->pixelformat = V4L2_PIX_FMT_YS16;
>  	return 0;
>  }
>  
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 8/9] Input: atmel_mxt_ts - add support for reference data
  2016-06-20 16:09   ` Hans Verkuil
@ 2016-06-20 16:18     ` Nick Dyer
  2016-06-20 16:22       ` Hans Verkuil
  0 siblings, 1 reply; 22+ messages in thread
From: Nick Dyer @ 2016-06-20 16:18 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Dmitry Torokhov, linux-input, linux-kernel, linux-media,
	Benjamin Tissoires, Benson Leung, Alan Bowens,
	Javier Martinez Canillas, Chris Healy, Henrik Rydberg,
	Andrew Duggan, James Chen, Dudley Du, Andrew de los Reyes,
	sheckylin, Peter Hutterer, Florian Echtler, mchehab

On 20/06/2016 17:09, Hans Verkuil wrote:
> On 06/17/2016 04:16 PM, Nick Dyer wrote:
>> @@ -2325,11 +2344,20 @@ static int mxt_vidioc_querycap(struct file *file, void *priv,
>>  static int mxt_vidioc_enum_input(struct file *file, void *priv,
>>  				   struct v4l2_input *i)
>>  {
>> -	if (i->index > 0)
>> +	if (i->index >= MXT_V4L_INPUT_MAX)
>>  		return -EINVAL;
>>  
>>  	i->type = V4L2_INPUT_TYPE_TOUCH_SENSOR;
>> -	strlcpy(i->name, "Mutual References", sizeof(i->name));
>> +
>> +	switch (i->index) {
>> +	case MXT_V4L_INPUT_REFS:
>> +		strlcpy(i->name, "Mutual References", sizeof(i->name));
>> +		break;
>> +	case MXT_V4L_INPUT_DELTAS:
>> +		strlcpy(i->name, "Mutual Deltas", sizeof(i->name));
> 
> I don't think this name is very clear. I have no idea how to interpret "Mutual"
> in this context.

"Mutual" is a touch domain specific term, it means the delta value is for
the capacitance between the horizontal and vertical lines at a particular
"node" on the touchscreen matrix, see
https://en.wikipedia.org/wiki/Touchscreen#Mutual_capacitance

I'll put in a comment.

> 
>> +		break;
>> +	}
>> +
>>  	return 0;
>>  }
>>  
>> @@ -2337,12 +2365,16 @@ static int mxt_set_input(struct mxt_data *data, unsigned int i)
>>  {
>>  	struct v4l2_pix_format *f = &data->dbg.format;
>>  
>> -	if (i > 0)
>> +	if (i >= MXT_V4L_INPUT_MAX)
>>  		return -EINVAL;
>>  
>> +	if (i == MXT_V4L_INPUT_DELTAS)
>> +		f->pixelformat = V4L2_PIX_FMT_YS16;
>> +	else
>> +		f->pixelformat = V4L2_PIX_FMT_Y16;
> 
> You probably need a V4L2_TOUCH_FMT_U16 or something for this as well. It certainly
> needs to be documented.

OK, will change this.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 9/9] Input: synaptics-rmi4 - add support for F54 diagnostics
  2016-06-17 14:16 ` [PATCH v4 9/9] Input: synaptics-rmi4 - add support for F54 diagnostics Nick Dyer
@ 2016-06-20 16:20   ` Hans Verkuil
  2016-06-21 22:16     ` Nick Dyer
  0 siblings, 1 reply; 22+ messages in thread
From: Hans Verkuil @ 2016-06-20 16:20 UTC (permalink / raw)
  To: Nick Dyer, Dmitry Torokhov
  Cc: linux-input, linux-kernel, linux-media, Benjamin Tissoires,
	Benson Leung, Alan Bowens, Javier Martinez Canillas, Chris Healy,
	Henrik Rydberg, Andrew Duggan, James Chen, Dudley Du,
	Andrew de los Reyes, sheckylin, Peter Hutterer, Florian Echtler,
	mchehab

On 06/17/2016 04:16 PM, Nick Dyer wrote:
> Function 54 implements access to various RMI4 diagnostic features.
> 
> This patch adds support for retrieving this data. It registers a V4L2
> device to output the data to user space.
> 
> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
> ---
>  drivers/input/rmi4/Kconfig      |  11 +
>  drivers/input/rmi4/Makefile     |   1 +
>  drivers/input/rmi4/rmi_bus.c    |   3 +
>  drivers/input/rmi4/rmi_driver.h |   1 +
>  drivers/input/rmi4/rmi_f54.c    | 743 ++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 759 insertions(+)
>  create mode 100644 drivers/input/rmi4/rmi_f54.c
> 
> diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
> index f73df24..f3418b6 100644
> --- a/drivers/input/rmi4/Kconfig
> +++ b/drivers/input/rmi4/Kconfig
> @@ -61,3 +61,14 @@ config RMI4_F30
>  
>  	  Function 30 provides GPIO and LED support for RMI4 devices. This
>  	  includes support for buttons on TouchPads and ClickPads.
> +
> +config RMI4_F54
> +	bool "RMI4 Function 54 (Analog diagnostics)"
> +	depends on RMI4_CORE
> +	depends on VIDEO_V4L2
> +	select VIDEOBUF2_VMALLOC
> +	help
> +	  Say Y here if you want to add support for RMI4 function 54
> +
> +	  Function 54 provides access to various diagnostic features in certain
> +	  RMI4 touch sensors.
> diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile
> index 95c00a7..0bafc85 100644
> --- a/drivers/input/rmi4/Makefile
> +++ b/drivers/input/rmi4/Makefile
> @@ -7,6 +7,7 @@ rmi_core-$(CONFIG_RMI4_2D_SENSOR) += rmi_2d_sensor.o
>  rmi_core-$(CONFIG_RMI4_F11) += rmi_f11.o
>  rmi_core-$(CONFIG_RMI4_F12) += rmi_f12.o
>  rmi_core-$(CONFIG_RMI4_F30) += rmi_f30.o
> +rmi_core-$(CONFIG_RMI4_F54) += rmi_f54.o
>  
>  # Transports
>  obj-$(CONFIG_RMI4_I2C) += rmi_i2c.o
> diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
> index b368b05..3aedc65 100644
> --- a/drivers/input/rmi4/rmi_bus.c
> +++ b/drivers/input/rmi4/rmi_bus.c
> @@ -315,6 +315,9 @@ static struct rmi_function_handler *fn_handlers[] = {
>  #ifdef CONFIG_RMI4_F30
>  	&rmi_f30_handler,
>  #endif
> +#ifdef CONFIG_RMI4_F54
> +	&rmi_f54_handler,
> +#endif
>  };
>  
>  static void __rmi_unregister_function_handlers(int start_idx)
> diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
> index 6e140fa..8dfbebe 100644
> --- a/drivers/input/rmi4/rmi_driver.h
> +++ b/drivers/input/rmi4/rmi_driver.h
> @@ -102,4 +102,5 @@ extern struct rmi_function_handler rmi_f01_handler;
>  extern struct rmi_function_handler rmi_f11_handler;
>  extern struct rmi_function_handler rmi_f12_handler;
>  extern struct rmi_function_handler rmi_f30_handler;
> +extern struct rmi_function_handler rmi_f54_handler;
>  #endif
> diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
> new file mode 100644
> index 0000000..bd06788
> --- /dev/null
> +++ b/drivers/input/rmi4/rmi_f54.c
> @@ -0,0 +1,743 @@
> +/*
> + * Copyright (c) 2012-2015 Synaptics Incorporated
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/rmi.h>
> +#include <linux/input.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/videobuf2-v4l2.h>
> +#include <media/videobuf2-vmalloc.h>
> +#include "rmi_driver.h"
> +
> +#define F54_NAME		"rmi4_f54"
> +
> +/* F54 data offsets */
> +#define F54_REPORT_DATA_OFFSET  3
> +#define F54_FIFO_OFFSET         1
> +#define F54_NUM_TX_OFFSET       1
> +#define F54_NUM_RX_OFFSET       0
> +
> +/* F54 commands */
> +#define F54_GET_REPORT          1
> +#define F54_FORCE_CAL           2
> +
> +/* Fixed sizes of reports */
> +#define F54_QUERY_LEN			27
> +#define F54_FULL_RAW_CAP_MIN_MAX_SIZE   4
> +#define F54_HIGH_RESISTANCE_SIZE(rx, tx) \
> +					(2 * ((rx) * (tx) + (rx) + (tx) + 3))
> +#define F54_MAX_REPORT_SIZE(rx, tx)	F54_HIGH_RESISTANCE_SIZE((rx), (tx))
> +
> +/* F54 capabilities */
> +#define F54_CAP_BASELINE	(1 << 2)
> +#define F54_CAP_IMAGE8		(1 << 3)
> +#define F54_CAP_IMAGE16		(1 << 6)
> +
> +enum f54_report_type {
> +	F54_REPORT_NONE = 0,
> +	F54_8BIT_IMAGE = 1,
> +	F54_16BIT_IMAGE = 2,
> +	F54_RAW_16BIT_IMAGE = 3,
> +	F54_HIGH_RESISTANCE = 4,
> +	F54_TX_TO_TX_SHORT = 5,
> +	F54_RX_TO_RX1 = 7,
> +	F54_TRUE_BASELINE = 9,
> +	F54_FULL_RAW_CAP_MIN_MAX = 13,
> +	F54_RX_OPENS1 = 14,
> +	F54_TX_OPEN = 15,
> +	F54_TX_TO_GROUND = 16,
> +	F54_RX_TO_RX2 = 17,
> +	F54_RX_OPENS2 = 18,
> +	F54_FULL_RAW_CAP = 19,
> +	F54_FULL_RAW_CAP_RX_COUPLING_COMP = 20,
> +	F54_MAX_REPORT_TYPE,
> +};
> +
> +struct f54_data {
> +	struct rmi_function *fn;
> +
> +	u8 qry[F54_QUERY_LEN];
> +	u8 num_rx_electrodes;
> +	u8 num_tx_electrodes;
> +	u8 capabilities;
> +	u16 clock_rate;
> +	u8 family;
> +
> +	enum f54_report_type report_type;
> +	u8 *report_data;
> +	int report_size;
> +
> +	bool is_busy;
> +	struct mutex status_mutex;
> +	struct mutex data_mutex;
> +
> +	struct workqueue_struct *workqueue;
> +	struct delayed_work work;
> +	unsigned long timeout;
> +
> +	struct completion cmd_done;
> +
> +	/* V4L2 support */
> +	struct v4l2_device v4l2;
> +	struct v4l2_pix_format format;
> +	struct video_device vdev;
> +	struct vb2_queue queue;
> +	struct mutex lock;
> +	int input;
> +	enum f54_report_type inputs[F54_MAX_REPORT_TYPE];
> +};
> +
> +/*
> + * Basic checks on report_type to ensure we write a valid type
> + * to the sensor.
> + */
> +static bool is_f54_report_type_valid(struct f54_data *f54,
> +				     enum f54_report_type reptype)
> +{
> +	switch (reptype) {
> +	case F54_8BIT_IMAGE:
> +		return f54->capabilities & F54_CAP_IMAGE8;
> +	case F54_16BIT_IMAGE:
> +	case F54_RAW_16BIT_IMAGE:
> +		return f54->capabilities & F54_CAP_IMAGE16;
> +	case F54_TRUE_BASELINE:
> +		return f54->capabilities & F54_CAP_IMAGE16;
> +	case F54_HIGH_RESISTANCE:
> +	case F54_TX_TO_TX_SHORT:
> +	case F54_RX_TO_RX1:
> +	case F54_FULL_RAW_CAP_MIN_MAX:
> +	case F54_RX_OPENS1:
> +	case F54_TX_OPEN:
> +	case F54_TX_TO_GROUND:
> +	case F54_RX_TO_RX2:
> +	case F54_RX_OPENS2:
> +	case F54_FULL_RAW_CAP:
> +	case F54_FULL_RAW_CAP_RX_COUPLING_COMP:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static enum f54_report_type rmi_f54_get_reptype(struct f54_data *f54,
> +						unsigned int i)
> +{
> +	if (i > F54_MAX_REPORT_TYPE)

This should be >=

> +		return F54_REPORT_NONE;
> +
> +	return f54->inputs[i];
> +}
> +
> +static void rmi_f54_create_input_map(struct f54_data *f54)
> +{
> +	int i = 0;
> +	enum f54_report_type reptype;
> +
> +	for (reptype = 1; reptype < F54_MAX_REPORT_TYPE; reptype++) {
> +		if (!is_f54_report_type_valid(f54, reptype))
> +			continue;
> +
> +		f54->inputs[i++] = reptype;
> +	}
> +
> +	/* Remaining values are zero via kzalloc */
> +}
> +
> +static int rmi_f54_request_report(struct rmi_function *fn, u8 report_type)
> +{
> +	struct f54_data *f54 = dev_get_drvdata(&fn->dev);
> +	struct rmi_device *rmi_dev = fn->rmi_dev;
> +	int error;
> +
> +	/* Write Report Type into F54_AD_Data0 */
> +	if (f54->report_type != report_type) {
> +		error = rmi_write(rmi_dev, f54->fn->fd.data_base_addr,
> +				  report_type);
> +		if (error)
> +			return error;
> +		f54->report_type = report_type;
> +	}
> +
> +	/*
> +	 * Small delay after disabling interrupts to avoid race condition
> +	 * in firmare. This value is a bit higher than absolutely necessary.
> +	 * Should be removed once issue is resolved in firmware.
> +	 */
> +	usleep_range(2000, 3000);
> +
> +	mutex_lock(&f54->data_mutex);
> +
> +	error = rmi_write(rmi_dev, fn->fd.command_base_addr, F54_GET_REPORT);
> +	if (error < 0)
> +		return error;
> +
> +	init_completion(&f54->cmd_done);
> +
> +	f54->is_busy = 1;
> +	f54->timeout = jiffies + msecs_to_jiffies(100);
> +
> +	queue_delayed_work(f54->workqueue, &f54->work, 0);
> +
> +	mutex_unlock(&f54->data_mutex);
> +
> +	return 0;
> +}
> +
> +static int rmi_f54_get_report_size(struct f54_data *f54)
> +{
> +	u8 rx = f54->num_rx_electrodes ? : f54->num_rx_electrodes;
> +	u8 tx = f54->num_tx_electrodes ? : f54->num_tx_electrodes;
> +	int size;
> +
> +	switch (rmi_f54_get_reptype(f54, f54->input)) {
> +	case F54_8BIT_IMAGE:
> +		size = rx * tx;
> +		break;
> +	case F54_16BIT_IMAGE:
> +	case F54_RAW_16BIT_IMAGE:
> +	case F54_TRUE_BASELINE:
> +	case F54_FULL_RAW_CAP:
> +	case F54_FULL_RAW_CAP_RX_COUPLING_COMP:
> +		size = 2 * rx * tx;
> +		break;
> +	case F54_HIGH_RESISTANCE:
> +		size = F54_HIGH_RESISTANCE_SIZE(rx, tx);
> +		break;
> +	case F54_FULL_RAW_CAP_MIN_MAX:
> +		size = F54_FULL_RAW_CAP_MIN_MAX_SIZE;
> +		break;
> +	case F54_TX_TO_TX_SHORT:
> +	case F54_TX_OPEN:
> +	case F54_TX_TO_GROUND:
> +		size =  (tx + 7) / 8;
> +		break;
> +	case F54_RX_TO_RX1:
> +	case F54_RX_OPENS1:
> +		if (rx < tx)
> +			size = 2 * rx * rx;
> +		else
> +			size = 2 * rx * tx;
> +		break;
> +	case F54_RX_TO_RX2:
> +	case F54_RX_OPENS2:
> +		if (rx <= tx)
> +			size = 0;
> +		else
> +			size = 2 * rx * (rx - tx);
> +		break;
> +	default:
> +		size = 0;
> +	}
> +
> +	return size;
> +}
> +
> +static const struct v4l2_file_operations rmi_f54_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 int rmi_f54_queue_setup(struct vb2_queue *q,
> +			       unsigned int *nbuffers, unsigned int *nplanes,
> +			       unsigned int sizes[], void *alloc_ctxs[])
> +{
> +	struct f54_data *f54 = q->drv_priv;
> +
> +	if (*nplanes)
> +		return sizes[0] < rmi_f54_get_report_size(f54) ? -EINVAL : 0;
> +
> +	*nplanes = 1;
> +	sizes[0] = rmi_f54_get_report_size(f54);
> +
> +	return 0;
> +}
> +
> +static void rmi_f54_buffer_queue(struct vb2_buffer *vb)
> +{
> +	struct f54_data *f54 = vb2_get_drv_priv(vb->vb2_queue);
> +	u16 *ptr;
> +	enum vb2_buffer_state state;
> +	enum f54_report_type reptype;
> +	int ret;
> +
> +	mutex_lock(&f54->status_mutex);
> +
> +	reptype = rmi_f54_get_reptype(f54, f54->input);
> +	if (reptype == F54_REPORT_NONE) {
> +		state = VB2_BUF_STATE_ERROR;
> +		goto done;
> +	}
> +
> +	if (f54->is_busy) {
> +		state = VB2_BUF_STATE_ERROR;
> +		goto done;
> +	}
> +
> +	ret = rmi_f54_request_report(f54->fn, reptype);
> +	if (ret) {
> +		dev_err(&f54->fn->dev, "Error requesting F54 report\n");
> +		state = VB2_BUF_STATE_ERROR;
> +		goto done;
> +	}
> +
> +	/* get frame data */
> +	mutex_lock(&f54->data_mutex);
> +
> +	while (f54->is_busy) {
> +		mutex_unlock(&f54->data_mutex);
> +		if (!wait_for_completion_timeout(&f54->cmd_done,
> +						 msecs_to_jiffies(1000))) {
> +			dev_err(&f54->fn->dev, "Timed out\n");
> +			state = VB2_BUF_STATE_ERROR;
> +			goto done;
> +		}
> +		mutex_lock(&f54->data_mutex);
> +	}
> +
> +	ptr = vb2_plane_vaddr(vb, 0);
> +	if (!ptr) {
> +		dev_err(&f54->fn->dev, "Error acquiring frame ptr\n");
> +		state = VB2_BUF_STATE_ERROR;
> +		goto data_done;
> +	}
> +
> +	memcpy(ptr, f54->report_data, f54->report_size);
> +	vb2_set_plane_payload(vb, 0, rmi_f54_get_report_size(f54));
> +	state = VB2_BUF_STATE_DONE;
> +
> +data_done:
> +	mutex_unlock(&f54->data_mutex);
> +done:
> +	vb2_buffer_done(vb, state);
> +	mutex_unlock(&f54->status_mutex);
> +}
> +
> +/* V4L2 structures */
> +static const struct vb2_ops rmi_f54_queue_ops = {
> +	.queue_setup            = rmi_f54_queue_setup,
> +	.buf_queue              = rmi_f54_buffer_queue,
> +	.wait_prepare           = vb2_ops_wait_prepare,
> +	.wait_finish            = vb2_ops_wait_finish,
> +};
> +
> +static const struct vb2_queue rmi_f54_queue = {
> +	.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
> +	.io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF | VB2_READ,
> +	.buf_struct_size = sizeof(struct vb2_buffer),
> +	.ops = &rmi_f54_queue_ops,
> +	.mem_ops = &vb2_vmalloc_memops,
> +	.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC,
> +	.min_buffers_needed = 1,
> +};
> +
> +static int rmi_f54_vidioc_querycap(struct file *file, void *priv,
> +				   struct v4l2_capability *cap)
> +{
> +	struct f54_data *f54 = video_drvdata(file);
> +
> +	strlcpy(cap->driver, F54_NAME, sizeof(cap->driver));
> +	strlcpy(cap->card, SYNAPTICS_INPUT_DEVICE_NAME, sizeof(cap->card));
> +	strlcpy(cap->bus_info, dev_name(&f54->fn->dev), sizeof(cap->bus_info));
> +
> +	return 0;
> +}
> +
> +static const char *rmi_f54_reptype_str(enum f54_report_type reptype)
> +{
> +	switch (reptype) {
> +	default:
> +	case F54_REPORT_NONE: return "No report";
> +	case F54_8BIT_IMAGE: return "8 bit image";
> +	case F54_16BIT_IMAGE: return "16 bit image";
> +	case F54_RAW_16BIT_IMAGE: return "Raw 16 bit image";
> +	case F54_HIGH_RESISTANCE: return "High resistance";
> +	case F54_TX_TO_TX_SHORT: return "TX to TX short";
> +	case F54_RX_TO_RX1: return "RX to RX1";
> +	case F54_TRUE_BASELINE: return "True baseline";
> +	case F54_FULL_RAW_CAP_MIN_MAX: return "Full raw cap min max";
> +	case F54_RX_OPENS1: return "RX open S1";
> +	case F54_TX_OPEN: return "TX open";
> +	case F54_TX_TO_GROUND: return "TX to ground";
> +	case F54_RX_TO_RX2: return "RX to RX2";
> +	case F54_RX_OPENS2: return "RX Open S2";
> +	case F54_FULL_RAW_CAP: return "Full raw cap";
> +	case F54_FULL_RAW_CAP_RX_COUPLING_COMP:
> +		return "Full raw cap RX coupling comp";
> +	case F54_MAX_REPORT_TYPE: return "Max report type";
> +	}
> +}
> +
> +static int rmi_f54_vidioc_enum_input(struct file *file, void *priv,
> +				     struct v4l2_input *i)
> +{
> +	struct f54_data *f54 = video_drvdata(file);
> +	enum f54_report_type reptype;
> +
> +	reptype = rmi_f54_get_reptype(f54, i->index);
> +	if (reptype == F54_REPORT_NONE)
> +		return -EINVAL;
> +
> +	i->type = V4L2_INPUT_TYPE_TOUCH_SENSOR;
> +	strlcpy(i->name, rmi_f54_reptype_str(reptype), sizeof(i->name));

Hmm, this doesn't feel right, but I don't have enough knowledge to decide if
using inputs for this is the right approach.

One thing that strikes me as odd is that both F54_8BIT_IMAGE and F54_16BIT_IMAGE
both seem to return signed 16 bit samples. I would expect this to result in
different pixel formats.

I have no idea what all these inputs mean. Are they all actually needed?
Would this perhaps be better implemented through a menu control?

I generally go by the philosophy that if I can't understand it, then it is
likely that others won't either :-)

Regards,

	Hans

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 8/9] Input: atmel_mxt_ts - add support for reference data
  2016-06-20 16:18     ` Nick Dyer
@ 2016-06-20 16:22       ` Hans Verkuil
  0 siblings, 0 replies; 22+ messages in thread
From: Hans Verkuil @ 2016-06-20 16:22 UTC (permalink / raw)
  To: Nick Dyer
  Cc: Dmitry Torokhov, linux-input, linux-kernel, linux-media,
	Benjamin Tissoires, Benson Leung, Alan Bowens,
	Javier Martinez Canillas, Chris Healy, Henrik Rydberg,
	Andrew Duggan, James Chen, Dudley Du, Andrew de los Reyes,
	sheckylin, Peter Hutterer, Florian Echtler, mchehab

On 06/20/2016 06:18 PM, Nick Dyer wrote:
> On 20/06/2016 17:09, Hans Verkuil wrote:
>> On 06/17/2016 04:16 PM, Nick Dyer wrote:
>>> @@ -2325,11 +2344,20 @@ static int mxt_vidioc_querycap(struct file *file, void *priv,
>>>  static int mxt_vidioc_enum_input(struct file *file, void *priv,
>>>  				   struct v4l2_input *i)
>>>  {
>>> -	if (i->index > 0)
>>> +	if (i->index >= MXT_V4L_INPUT_MAX)
>>>  		return -EINVAL;
>>>  
>>>  	i->type = V4L2_INPUT_TYPE_TOUCH_SENSOR;
>>> -	strlcpy(i->name, "Mutual References", sizeof(i->name));
>>> +
>>> +	switch (i->index) {
>>> +	case MXT_V4L_INPUT_REFS:
>>> +		strlcpy(i->name, "Mutual References", sizeof(i->name));
>>> +		break;
>>> +	case MXT_V4L_INPUT_DELTAS:
>>> +		strlcpy(i->name, "Mutual Deltas", sizeof(i->name));
>>
>> I don't think this name is very clear. I have no idea how to interpret "Mutual"
>> in this context.
> 
> "Mutual" is a touch domain specific term, it means the delta value is for
> the capacitance between the horizontal and vertical lines at a particular
> "node" on the touchscreen matrix, see
> https://en.wikipedia.org/wiki/Touchscreen#Mutual_capacitance
> 
> I'll put in a comment.

As I mentioned in an earlier review, we need a v4l-touch interface description anyway.
I think it might be useful to describe some of these touch-specific terms there.
That way that could be a useful reference for end-users.

Nobody reads comments, but people do read the spec (well, I do).

Regards,

	Hans

> 
>>
>>> +		break;
>>> +	}
>>> +
>>>  	return 0;
>>>  }
>>>  
>>> @@ -2337,12 +2365,16 @@ static int mxt_set_input(struct mxt_data *data, unsigned int i)
>>>  {
>>>  	struct v4l2_pix_format *f = &data->dbg.format;
>>>  
>>> -	if (i > 0)
>>> +	if (i >= MXT_V4L_INPUT_MAX)
>>>  		return -EINVAL;
>>>  
>>> +	if (i == MXT_V4L_INPUT_DELTAS)
>>> +		f->pixelformat = V4L2_PIX_FMT_YS16;
>>> +	else
>>> +		f->pixelformat = V4L2_PIX_FMT_Y16;
>>
>> You probably need a V4L2_TOUCH_FMT_U16 or something for this as well. It certainly
>> needs to be documented.
> 
> OK, will change this.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 9/9] Input: synaptics-rmi4 - add support for F54 diagnostics
  2016-06-20 16:20   ` Hans Verkuil
@ 2016-06-21 22:16     ` Nick Dyer
  0 siblings, 0 replies; 22+ messages in thread
From: Nick Dyer @ 2016-06-21 22:16 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Dmitry Torokhov, linux-input, linux-kernel, linux-media,
	Benjamin Tissoires, Benson Leung, Alan Bowens,
	Javier Martinez Canillas, Chris Healy, Henrik Rydberg,
	Andrew Duggan, James Chen, Dudley Du, Andrew de los Reyes,
	sheckylin, Peter Hutterer, Florian Echtler, mchehab

On 20/06/2016 17:20, Hans Verkuil wrote:
> On 06/17/2016 04:16 PM, Nick Dyer wrote:
>> +static int rmi_f54_vidioc_enum_input(struct file *file, void *priv,
>> +				     struct v4l2_input *i)
>> +{
>> +	struct f54_data *f54 = video_drvdata(file);
>> +	enum f54_report_type reptype;
>> +
>> +	reptype = rmi_f54_get_reptype(f54, i->index);
>> +	if (reptype == F54_REPORT_NONE)
>> +		return -EINVAL;
>> +
>> +	i->type = V4L2_INPUT_TYPE_TOUCH_SENSOR;
>> +	strlcpy(i->name, rmi_f54_reptype_str(reptype), sizeof(i->name));
> 
> Hmm, this doesn't feel right, but I don't have enough knowledge to decide if
> using inputs for this is the right approach.
> 
> One thing that strikes me as odd is that both F54_8BIT_IMAGE and F54_16BIT_IMAGE
> both seem to return signed 16 bit samples. I would expect this to result in
> different pixel formats.

Yes, you're right. I've had a look at this area now and fixed it up. I've
added pixel formats for the various types of touch data so we will never
emit anything that's not under V4L2_TCH_FMT.

I've also worked on adding the DocBook documentation that was missing,
cribbing from SDR as you suggested. I think I've found all the places it
was needed now.

> I have no idea what all these inputs mean. Are they all actually needed?
> Would this perhaps be better implemented through a menu control?

These are the various types of tuning or fault finding data that users
expect to be able to access via the RMI4 F54 diagnostics function. Although
I would have to do some digging to give an detailed use case for each one.
Due to the way that F54 is implemented, only one data source can be
selected at once. So it seemed fairly reasonable to me to map it to inputs.

> I generally go by the philosophy that if I can't understand it, then it is
> likely that others won't either :-)

No problem.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 2/9] [media] v4l2-core: Add VFL_TYPE_TOUCH_SENSOR
  2016-06-20 12:00   ` Hans Verkuil
@ 2016-06-22 11:48     ` Florian Echtler
  2016-06-22 11:59       ` Nick Dyer
  0 siblings, 1 reply; 22+ messages in thread
From: Florian Echtler @ 2016-06-22 11:48 UTC (permalink / raw)
  To: Hans Verkuil, Nick Dyer, Dmitry Torokhov
  Cc: linux-input, linux-kernel, linux-media, Benjamin Tissoires,
	Benson Leung, Alan Bowens, Javier Martinez Canillas, Chris Healy,
	Henrik Rydberg, Andrew Duggan, James Chen, Dudley Du,
	Andrew de los Reyes, sheckylin, Peter Hutterer, mchehab


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

On 20.06.2016 14:00, Hans Verkuil wrote:
> On 06/17/2016 04:16 PM, Nick Dyer wrote:
>> Some touch controllers send out raw touch data in a similar way to a
>> greyscale frame grabber. Add a new device type for these devices.
>>
>> Use a new device prefix v4l-touch for these devices, to stop generic
>> capture software from treating them as webcams.
>>
>> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
>> ---
>>  drivers/input/touchscreen/sur40.c    |  4 ++--
>>  drivers/media/v4l2-core/v4l2-dev.c   | 13 ++++++++++---
>>  drivers/media/v4l2-core/v4l2-ioctl.c | 15 ++++++++++-----
>>  include/media/v4l2-dev.h             |  3 ++-
>>  include/uapi/linux/videodev2.h       |  1 +

Generally a good idea in my opinion, but I think the SUR40 is a special
case: the whole point of putting in a V4L2 driver was that software like
reacTIVision, which already has a V4L2 interface, can then use that
device like any other camera.

Come to think of it, wouldn't it make sense to expose the other touch
devices as generic frame grabbers, too, so you can easily view the debug
output with any generic tool like cheese?

Best, Florian
-- 
SENT FROM MY DEC VT50 TERMINAL


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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 2/9] [media] v4l2-core: Add VFL_TYPE_TOUCH_SENSOR
  2016-06-22 11:48     ` Florian Echtler
@ 2016-06-22 11:59       ` Nick Dyer
  2016-06-22 20:38         ` Florian Echtler
  0 siblings, 1 reply; 22+ messages in thread
From: Nick Dyer @ 2016-06-22 11:59 UTC (permalink / raw)
  To: Florian Echtler, Hans Verkuil, Dmitry Torokhov
  Cc: linux-input, linux-kernel, linux-media, Benjamin Tissoires,
	Benson Leung, Alan Bowens, Javier Martinez Canillas, Chris Healy,
	Henrik Rydberg, Andrew Duggan, James Chen, Dudley Du,
	Andrew de los Reyes, sheckylin, Peter Hutterer, mchehab

On 22/06/2016 12:48, Florian Echtler wrote:
> On 20.06.2016 14:00, Hans Verkuil wrote:
>> On 06/17/2016 04:16 PM, Nick Dyer wrote:
>>> Some touch controllers send out raw touch data in a similar way to a
>>> greyscale frame grabber. Add a new device type for these devices.
>>>
>>> Use a new device prefix v4l-touch for these devices, to stop generic
>>> capture software from treating them as webcams.
>>>
>>> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
>>> ---
>>>  drivers/input/touchscreen/sur40.c    |  4 ++--
>>>  drivers/media/v4l2-core/v4l2-dev.c   | 13 ++++++++++---
>>>  drivers/media/v4l2-core/v4l2-ioctl.c | 15 ++++++++++-----
>>>  include/media/v4l2-dev.h             |  3 ++-
>>>  include/uapi/linux/videodev2.h       |  1 +
> 
> Generally a good idea in my opinion, but I think the SUR40 is a special
> case: the whole point of putting in a V4L2 driver was that software like
> reacTIVision, which already has a V4L2 interface, can then use that
> device like any other camera.

Thanks. I see that reactivision definitely uses this already
(https://github.com/mkalten/reacTIVision/issues/3 ) and we don't want to
break it - I've split the sur40.c change out of this patch now so it can be
considered separately.

> Come to think of it, wouldn't it make sense to expose the other touch
> devices as generic frame grabbers, too, so you can easily view the debug
> output with any generic tool like cheese?

While I like the idea of being able to use the generic tools, I think we
needed to do something to stop these devices turning up in e.g. video
conferencing software - it would cause a lot of confusion. There's nothing
stopping particular tools adding the necessary code to handle touch devices
if they feel their users want it.

Also, the RMI4 and Atmel mXT touchscreens output signed data, which
unfortunately would confuse the generic tools.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 2/9] [media] v4l2-core: Add VFL_TYPE_TOUCH_SENSOR
  2016-06-22 11:59       ` Nick Dyer
@ 2016-06-22 20:38         ` Florian Echtler
  2016-06-22 21:22           ` Nick Dyer
  0 siblings, 1 reply; 22+ messages in thread
From: Florian Echtler @ 2016-06-22 20:38 UTC (permalink / raw)
  To: Nick Dyer
  Cc: Hans Verkuil, Dmitry Torokhov, linux-input, linux-kernel,
	linux-media, Benjamin Tissoires, Benson Leung, Alan Bowens,
	Javier Martinez Canillas, Chris Healy, Henrik Rydberg,
	Andrew Duggan, James Chen, Dudley Du, Andrew de los Reyes,
	sheckylin, Peter Hutterer, mchehab

On Wed, 22 Jun 2016, Nick Dyer wrote:

> On 22/06/2016 12:48, Florian Echtler wrote:
>> On 20.06.2016 14:00, Hans Verkuil wrote:
>>> On 06/17/2016 04:16 PM, Nick Dyer wrote:
>>>>
>>>> Use a new device prefix v4l-touch for these devices, to stop generic
>>>> capture software from treating them as webcams.
>
>> Come to think of it, wouldn't it make sense to expose the other touch
>> devices as generic frame grabbers, too, so you can easily view the debug
>> output with any generic tool like cheese?
>
> While I like the idea of being able to use the generic tools, I think we
> needed to do something to stop these devices turning up in e.g. video
> conferencing software - it would cause a lot of confusion. There's nothing
> stopping particular tools adding the necessary code to handle touch devices
> if they feel their users want it.

Just to clarify: from the userspace point-of-view, would this change 
simply modify the prefix of the device node (i.e. /dev/video1 -> 
/dev/v4l-touch1), or would it somehow affect the API? If it's just the 
device node name, then that shouldn't be a problem after all, because e.g. 
reacTIVision requires you to specify the exact camera to be used anyway.

Florian
-- 
"_Nothing_ brightens up my morning. Coffee simply provides a shade of
grey just above the pitch-black of the infinite depths of the _abyss_."

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 2/9] [media] v4l2-core: Add VFL_TYPE_TOUCH_SENSOR
  2016-06-22 20:38         ` Florian Echtler
@ 2016-06-22 21:22           ` Nick Dyer
  0 siblings, 0 replies; 22+ messages in thread
From: Nick Dyer @ 2016-06-22 21:22 UTC (permalink / raw)
  To: Florian Echtler
  Cc: Hans Verkuil, Dmitry Torokhov, linux-input, linux-kernel,
	linux-media, Benjamin Tissoires, Benson Leung, Alan Bowens,
	Javier Martinez Canillas, Chris Healy, Henrik Rydberg,
	Andrew Duggan, James Chen, Dudley Du, Andrew de los Reyes,
	sheckylin, Peter Hutterer, mchehab

On 22/06/2016 21:38, Florian Echtler wrote:
> On Wed, 22 Jun 2016, Nick Dyer wrote:
> 
>> On 22/06/2016 12:48, Florian Echtler wrote:
>>> On 20.06.2016 14:00, Hans Verkuil wrote:
>>>> On 06/17/2016 04:16 PM, Nick Dyer wrote:
>>>>>
>>>>> Use a new device prefix v4l-touch for these devices, to stop generic
>>>>> capture software from treating them as webcams.
>>
>>> Come to think of it, wouldn't it make sense to expose the other touch
>>> devices as generic frame grabbers, too, so you can easily view the debug
>>> output with any generic tool like cheese?
>>
>> While I like the idea of being able to use the generic tools, I think we
>> needed to do something to stop these devices turning up in e.g. video
>> conferencing software - it would cause a lot of confusion. There's nothing
>> stopping particular tools adding the necessary code to handle touch devices
>> if they feel their users want it.
> 
> Just to clarify: from the userspace point-of-view, would this change simply
> modify the prefix of the device node (i.e. /dev/video1 -> /dev/v4l-touch1),
> or would it somehow affect the API? If it's just the device node name, then
> that shouldn't be a problem after all, because e.g. reacTIVision requires
> you to specify the exact camera to be used anyway.

With the changes that Hans Verkuil has asked me to do:
* The device node is /dev/v4l-touch0-255
* There are several new formats eg. V4L2_TCH_FMT_DELTA_TD16 (16 bit deltas)
* I've defined new types VFL_TYPE_TOUCH, V4L2_BUF_TYPE_TOUCH_CAPTURE,
V4L2_INPUT_TYPE_TOUCH

We're just testing the changes, I hope to post an updated version soon.

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2016-06-22 21:22 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-17 14:16 [PATCH v4 0/9] Output raw touch data via V4L2 Nick Dyer
2016-06-17 14:16 ` [PATCH v4 1/9] [media] Add signed 16-bit pixel format Nick Dyer
2016-06-20 11:00   ` Hans Verkuil
2016-06-20 11:51     ` Nick Dyer
2016-06-17 14:16 ` [PATCH v4 2/9] [media] v4l2-core: Add VFL_TYPE_TOUCH_SENSOR Nick Dyer
2016-06-20 12:00   ` Hans Verkuil
2016-06-22 11:48     ` Florian Echtler
2016-06-22 11:59       ` Nick Dyer
2016-06-22 20:38         ` Florian Echtler
2016-06-22 21:22           ` Nick Dyer
2016-06-17 14:16 ` [PATCH v4 3/9] Input: atmel_mxt_ts - add support for T37 diagnostic data Nick Dyer
2016-06-17 14:16 ` [PATCH v4 4/9] Input: atmel_mxt_ts - output diagnostic debug via v4l2 device Nick Dyer
2016-06-17 14:16 ` [PATCH v4 5/9] Input: atmel_mxt_ts - read touchscreen size Nick Dyer
2016-06-17 14:16 ` [PATCH v4 6/9] Input: atmel_mxt_ts - handle diagnostic data orientation Nick Dyer
2016-06-17 14:16 ` [PATCH v4 7/9] Input: atmel_mxt_ts - add diagnostic data support for mXT1386 Nick Dyer
2016-06-17 14:16 ` [PATCH v4 8/9] Input: atmel_mxt_ts - add support for reference data Nick Dyer
2016-06-20 16:09   ` Hans Verkuil
2016-06-20 16:18     ` Nick Dyer
2016-06-20 16:22       ` Hans Verkuil
2016-06-17 14:16 ` [PATCH v4 9/9] Input: synaptics-rmi4 - add support for F54 diagnostics Nick Dyer
2016-06-20 16:20   ` Hans Verkuil
2016-06-21 22:16     ` Nick Dyer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.