All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/3] input: touchscreen: ad7879: move header to platform_data directory
@ 2016-02-02 23:20 Stefan Agner
  2016-02-02 23:20 ` [PATCH v3 2/3] input: touchscreen: ad7879: fix default x/y axis assignment Stefan Agner
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Stefan Agner @ 2016-02-02 23:20 UTC (permalink / raw)
  To: dmitry.torokhov, michael.hennerich, robh+dt
  Cc: mark.rutland, ijc+devicetree, galak, realmz6, broonie, jic23,
	linux-input, devicetree, linux-kernel, Stefan Agner

The header file is used by the SPI and I2C variant of the driver.
Therefore, move it to a more generic place under platform_data.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
Changes since v2:
- (none)
Changes since v1:
- Move to include/linux/platform_data/

 arch/blackfin/mach-bf527/boards/ezbrd.c    |  2 +-
 arch/blackfin/mach-bf527/boards/ezkit.c    |  2 +-
 arch/blackfin/mach-bf527/boards/tll6527m.c |  2 +-
 arch/blackfin/mach-bf537/boards/stamp.c    |  2 +-
 arch/blackfin/mach-bf538/boards/ezkit.c    |  2 +-
 drivers/input/touchscreen/ad7879.c         | 10 ++++----
 include/linux/platform_data/ad7879.h       | 41 ++++++++++++++++++++++++++++++
 include/linux/spi/ad7879.h                 | 41 ------------------------------
 8 files changed, 51 insertions(+), 51 deletions(-)
 create mode 100644 include/linux/platform_data/ad7879.h
 delete mode 100644 include/linux/spi/ad7879.h

diff --git a/arch/blackfin/mach-bf527/boards/ezbrd.c b/arch/blackfin/mach-bf527/boards/ezbrd.c
index a3a5723..80bcfd1 100644
--- a/arch/blackfin/mach-bf527/boards/ezbrd.c
+++ b/arch/blackfin/mach-bf527/boards/ezbrd.c
@@ -279,7 +279,7 @@ static const struct ad7877_platform_data bfin_ad7877_ts_info = {
 #endif
 
 #if IS_ENABLED(CONFIG_TOUCHSCREEN_AD7879)
-#include <linux/spi/ad7879.h>
+#include <linux/platform_data/ad7879.h>
 static const struct ad7879_platform_data bfin_ad7879_ts_info = {
 	.model			= 7879,	/* Model = AD7879 */
 	.x_plate_ohms		= 620,	/* 620 Ohm from the touch datasheet */
diff --git a/arch/blackfin/mach-bf527/boards/ezkit.c b/arch/blackfin/mach-bf527/boards/ezkit.c
index d4219e8..571edfd 100644
--- a/arch/blackfin/mach-bf527/boards/ezkit.c
+++ b/arch/blackfin/mach-bf527/boards/ezkit.c
@@ -477,7 +477,7 @@ static const struct ad7877_platform_data bfin_ad7877_ts_info = {
 #endif
 
 #if IS_ENABLED(CONFIG_TOUCHSCREEN_AD7879)
-#include <linux/spi/ad7879.h>
+#include <linux/platform_data/ad7879.h>
 static const struct ad7879_platform_data bfin_ad7879_ts_info = {
 	.model			= 7879,	/* Model = AD7879 */
 	.x_plate_ohms		= 620,	/* 620 Ohm from the touch datasheet */
diff --git a/arch/blackfin/mach-bf527/boards/tll6527m.c b/arch/blackfin/mach-bf527/boards/tll6527m.c
index a0f5856..c1acce4 100644
--- a/arch/blackfin/mach-bf527/boards/tll6527m.c
+++ b/arch/blackfin/mach-bf527/boards/tll6527m.c
@@ -29,7 +29,7 @@
 #include <asm/dpmc.h>
 
 #if IS_ENABLED(CONFIG_TOUCHSCREEN_AD7879)
-#include <linux/spi/ad7879.h>
+#include <linux/platform_data/ad7879.h>
 #define LCD_BACKLIGHT_GPIO 0x40
 /* TLL6527M uses TLL7UIQ35 / ADI LCD EZ Extender. AD7879 AUX GPIO is used for
  * LCD Backlight Enable
diff --git a/arch/blackfin/mach-bf537/boards/stamp.c b/arch/blackfin/mach-bf537/boards/stamp.c
index c181543..eaec7b4 100644
--- a/arch/blackfin/mach-bf537/boards/stamp.c
+++ b/arch/blackfin/mach-bf537/boards/stamp.c
@@ -776,7 +776,7 @@ static const struct ad7877_platform_data bfin_ad7877_ts_info = {
 #endif
 
 #if IS_ENABLED(CONFIG_TOUCHSCREEN_AD7879)
-#include <linux/spi/ad7879.h>
+#include <linux/platform_data/ad7879.h>
 static const struct ad7879_platform_data bfin_ad7879_ts_info = {
 	.model			= 7879,	/* Model = AD7879 */
 	.x_plate_ohms		= 620,	/* 620 Ohm from the touch datasheet */
diff --git a/arch/blackfin/mach-bf538/boards/ezkit.c b/arch/blackfin/mach-bf538/boards/ezkit.c
index ae2fcbb..4a03c44 100644
--- a/arch/blackfin/mach-bf538/boards/ezkit.c
+++ b/arch/blackfin/mach-bf538/boards/ezkit.c
@@ -521,7 +521,7 @@ static struct bfin5xx_spi_chip spi_flash_chip_info = {
 #endif	/* CONFIG_SPI_BFIN5XX */
 
 #if IS_ENABLED(CONFIG_TOUCHSCREEN_AD7879)
-#include <linux/spi/ad7879.h>
+#include <linux/platform_data/ad7879.h>
 static const struct ad7879_platform_data bfin_ad7879_ts_info = {
 	.model			= 7879,	/* Model = AD7879 */
 	.x_plate_ohms		= 620,	/* 620 Ohm from the touch datasheet */
diff --git a/drivers/input/touchscreen/ad7879.c b/drivers/input/touchscreen/ad7879.c
index 16b5cc2..93b8ea2 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -31,7 +31,7 @@
 #include <linux/i2c.h>
 #include <linux/gpio.h>
 
-#include <linux/spi/ad7879.h>
+#include <linux/platform_data/ad7879.h>
 #include <linux/module.h>
 #include "ad7879.h"
 
@@ -170,10 +170,10 @@ static int ad7879_report(struct ad7879 *ts)
 	 * filter.  The combination of these two techniques provides a robust
 	 * solution, discarding the spurious noise in the signal and keeping
 	 * only the data of interest.  The size of both filters is
-	 * programmable. (dev.platform_data, see linux/spi/ad7879.h) Other
-	 * user-programmable conversion controls include variable acquisition
-	 * time, and first conversion delay. Up to 16 averages can be taken
-	 * per conversion.
+	 * programmable. (dev.platform_data, see linux/platform_data/ad7879.h)
+	 * Other user-programmable conversion controls include variable
+	 * acquisition time, and first conversion delay. Up to 16 averages can
+	 * be taken per conversion.
 	 */
 
 	if (likely(x && z1)) {
diff --git a/include/linux/platform_data/ad7879.h b/include/linux/platform_data/ad7879.h
new file mode 100644
index 0000000..69e2e1fd
--- /dev/null
+++ b/include/linux/platform_data/ad7879.h
@@ -0,0 +1,41 @@
+/* linux/platform_data/ad7879.h */
+
+/* Touchscreen characteristics vary between boards and models.  The
+ * platform_data for the device's "struct device" holds this information.
+ *
+ * It's OK if the min/max values are zero.
+ */
+struct ad7879_platform_data {
+	u16	model;			/* 7879 */
+	u16	x_plate_ohms;
+	u16	x_min, x_max;
+	u16	y_min, y_max;
+	u16	pressure_min, pressure_max;
+
+	bool	swap_xy;		/* swap x and y axes */
+
+	/* [0..255] 0=OFF Starts at 1=550us and goes
+	 * all the way to 9.440ms in steps of 35us.
+	 */
+	u8	pen_down_acc_interval;
+	/* [0..15] Starts at 0=128us and goes all the
+	 * way to 4.096ms in steps of 128us.
+	 */
+	u8	first_conversion_delay;
+	/* [0..3] 0 = 2us, 1 = 4us, 2 = 8us, 3 = 16us */
+	u8	acquisition_time;
+	/* [0..3] Average X middle samples 0 = 2, 1 = 4, 2 = 8, 3 = 16 */
+	u8	averaging;
+	/* [0..3] Perform X measurements 0 = OFF,
+	 * 1 = 4, 2 = 8, 3 = 16 (median > averaging)
+	 */
+	u8	median;
+	/* 1 = AUX/VBAT/GPIO export GPIO to gpiolib
+	 * requires CONFIG_GPIOLIB
+	 */
+	bool	gpio_export;
+	/* identifies the first GPIO number handled by this chip;
+	 * or, if negative, requests dynamic ID allocation.
+	 */
+	s32	gpio_base;
+};
diff --git a/include/linux/spi/ad7879.h b/include/linux/spi/ad7879.h
deleted file mode 100644
index 58368be..0000000
--- a/include/linux/spi/ad7879.h
+++ /dev/null
@@ -1,41 +0,0 @@
-/* linux/spi/ad7879.h */
-
-/* Touchscreen characteristics vary between boards and models.  The
- * platform_data for the device's "struct device" holds this information.
- *
- * It's OK if the min/max values are zero.
- */
-struct ad7879_platform_data {
-	u16	model;			/* 7879 */
-	u16	x_plate_ohms;
-	u16	x_min, x_max;
-	u16	y_min, y_max;
-	u16	pressure_min, pressure_max;
-
-	bool	swap_xy;		/* swap x and y axes */
-
-	/* [0..255] 0=OFF Starts at 1=550us and goes
-	 * all the way to 9.440ms in steps of 35us.
-	 */
-	u8	pen_down_acc_interval;
-	/* [0..15] Starts at 0=128us and goes all the
-	 * way to 4.096ms in steps of 128us.
-	 */
-	u8	first_conversion_delay;
-	/* [0..3] 0 = 2us, 1 = 4us, 2 = 8us, 3 = 16us */
-	u8	acquisition_time;
-	/* [0..3] Average X middle samples 0 = 2, 1 = 4, 2 = 8, 3 = 16 */
-	u8	averaging;
-	/* [0..3] Perform X measurements 0 = OFF,
-	 * 1 = 4, 2 = 8, 3 = 16 (median > averaging)
-	 */
-	u8	median;
-	/* 1 = AUX/VBAT/GPIO export GPIO to gpiolib
-	 * requires CONFIG_GPIOLIB
-	 */
-	bool	gpio_export;
-	/* identifies the first GPIO number handled by this chip;
-	 * or, if negative, requests dynamic ID allocation.
-	 */
-	s32	gpio_base;
-};
-- 
2.7.0

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

* [PATCH v3 2/3] input: touchscreen: ad7879: fix default x/y axis assignment
  2016-02-02 23:20 [PATCH v3 1/3] input: touchscreen: ad7879: move header to platform_data directory Stefan Agner
@ 2016-02-02 23:20 ` Stefan Agner
  2016-02-03  7:41     ` Michael Hennerich
  2016-02-02 23:20 ` [PATCH v3 3/3] input: touchscreen: ad7879: add device tree support Stefan Agner
  2016-03-08 17:48 ` [PATCH v3 1/3] input: touchscreen: ad7879: move header to platform_data directory Stefan Agner
  2 siblings, 1 reply; 10+ messages in thread
From: Stefan Agner @ 2016-02-02 23:20 UTC (permalink / raw)
  To: dmitry.torokhov, michael.hennerich, robh+dt
  Cc: mark.rutland, ijc+devicetree, galak, realmz6, broonie, jic23,
	linux-input, devicetree, linux-kernel, Stefan Agner

The X/Y position measurements read from the controller are interpreted
wrong. The first measurement X+ contains the Y position, and the second
measurement Y+ the X position (see also Table 11 Register Table in the
data sheet).

The problem is already known and a swap option has been introduced:
commit 6680884a4420 ("Input: ad7879 - add option to correct xy axis")

However, the meaning of the new boolean is inverted since the underlying
values are already swapped. Let ts->swap_xy set to true actually be the
swapped configuration of the two axis.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
Changes since v2:
- (none)
Changes since v1:
- Keep axis swapped by default when using platform data

 drivers/input/touchscreen/ad7879.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/ad7879.c b/drivers/input/touchscreen/ad7879.c
index 93b8ea2..abd0220 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -94,8 +94,8 @@
 #define AD7879_TEMP_BIT			(1<<1)
 
 enum {
-	AD7879_SEQ_XPOS  = 0,
-	AD7879_SEQ_YPOS  = 1,
+	AD7879_SEQ_YPOS  = 0,
+	AD7879_SEQ_XPOS  = 1,
 	AD7879_SEQ_Z1    = 2,
 	AD7879_SEQ_Z2    = 3,
 	AD7879_NR_SENSE  = 4,
@@ -517,7 +517,9 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
 	ts->dev = dev;
 	ts->input = input_dev;
 	ts->irq = irq;
-	ts->swap_xy = pdata->swap_xy;
+
+	/* Use swapped axis by default (backward compatibility) */
+	ts->swap_xy = !pdata->swap_xy;
 
 	setup_timer(&ts->timer, ad7879_timer, (unsigned long) ts);
 
-- 
2.7.0

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

* [PATCH v3 3/3] input: touchscreen: ad7879: add device tree support
  2016-02-02 23:20 [PATCH v3 1/3] input: touchscreen: ad7879: move header to platform_data directory Stefan Agner
  2016-02-02 23:20 ` [PATCH v3 2/3] input: touchscreen: ad7879: fix default x/y axis assignment Stefan Agner
@ 2016-02-02 23:20 ` Stefan Agner
  2016-02-03  7:41     ` Michael Hennerich
  2016-03-08 18:50   ` Dmitry Torokhov
  2016-03-08 17:48 ` [PATCH v3 1/3] input: touchscreen: ad7879: move header to platform_data directory Stefan Agner
  2 siblings, 2 replies; 10+ messages in thread
From: Stefan Agner @ 2016-02-02 23:20 UTC (permalink / raw)
  To: dmitry.torokhov, michael.hennerich, robh+dt
  Cc: mark.rutland, ijc+devicetree, galak, realmz6, broonie, jic23,
	linux-input, devicetree, linux-kernel, Stefan Agner

Add device tree support for the I2C and SPI variant of AD7879(-1).
This allows to specify the touchscreen controller as a I2C client
node or SPI slave device. Most of the options available in platform
data are also available as device tree properties, the only exception
being GPIO capabilities, which can not be activated through device
tree currently.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
Changes since v2:
- do not free driver data and irq on remove (since we are using devm now)
Changes since v1:
- Move device tree parsing to main driver file ad7879.c
- Use common touchscreen_parse_properties for common properties
- Use device_property_* API
- Use struct ad7879 directly to store parsed values
- Support SPI variant through device tree too (untested)
- Add vendor prefix to vendor specific dt properties

 .../bindings/input/touchscreen/ad7879.txt          |  53 ++++++++
 drivers/input/touchscreen/ad7879-i2c.c             |  10 ++
 drivers/input/touchscreen/ad7879-spi.c             |  10 ++
 drivers/input/touchscreen/ad7879.c                 | 145 +++++++++++++--------
 4 files changed, 161 insertions(+), 57 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ad7879.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/ad7879.txt b/Documentation/devicetree/bindings/input/touchscreen/ad7879.txt
new file mode 100644
index 0000000..e3f22d2
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/ad7879.txt
@@ -0,0 +1,53 @@
+* Analog Devices AD7879(-1)/AD7889(-1) touchscreen interface (SPI/I2C)
+
+Required properties:
+- compatible			: for SPI slave, use "adi,ad7879"
+				  for I2C slave, use "adi,ad7879-1"
+- reg				: SPI chipselect/I2C slave address
+				  See spi-bus.txt for more SPI slave properties
+- interrupt-parent		: the phandle for the interrupt controller
+- interrupts			: touch controller interrupt
+- touchscreen-max-pressure	: maximum reported pressure
+- adi,resistance-plate-x	: total resistance of X-plate (for pressure
+				  calculation)
+Optional properties:
+- touchscreen-swapped-x-y	: X and Y axis are swapped (boolean)
+- adi,first-conversion-delay	: 0-12: In 128us steps (starting with 128us)
+				  13  : 2.560ms
+				  14  : 3.584ms
+				  15  : 4.096ms
+				  This property has to be a '/bits/ 8' value
+- adi,acquisition-time		: 0: 2us
+				  1: 4us
+				  2: 8us
+				  3: 16us
+				  This property has to be a '/bits/ 8' value
+- adi,median-filter-size	: 0: disabled
+				  1: 4 measurements
+				  2: 8 measurements
+				  3: 16 measurements
+				  This property has to be a '/bits/ 8' value
+- adi,averaging			: 0: 2 middle values (1 if median disabled)
+				  1: 4 middle values
+				  2: 8 middle values
+				  3: 16 values
+				  This property has to be a '/bits/ 8' value
+- adi,conversion-interval:	: 0    : convert one time only
+				  1-255: 515us + val * 35us (up to 9.440ms)
+				  This property has to be a '/bits/ 8' value
+
+Example:
+
+	ad7879@2c {
+		compatible = "adi,ad7879-1";
+		reg = <0x2c>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <13 IRQ_TYPE_EDGE_FALLING>;
+		touchscreen-max-pressure = <4096>;
+		adi,resistance-plate-x = <120>;
+		adi,first-conversion-delay = /bits/ 8 <3>;
+		adi,acquisition-time = /bits/ 8 <1>;
+		adi,median-filter-size = /bits/ 8 <2>;
+		adi,averaging = /bits/ 8 <1>;
+		adi,conversion-interval = /bits/ 8 <255>;
+	};
diff --git a/drivers/input/touchscreen/ad7879-i2c.c b/drivers/input/touchscreen/ad7879-i2c.c
index d66962c..58f72e0 100644
--- a/drivers/input/touchscreen/ad7879-i2c.c
+++ b/drivers/input/touchscreen/ad7879-i2c.c
@@ -10,6 +10,7 @@
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/types.h>
+#include <linux/of.h>
 #include <linux/pm.h>
 
 #include "ad7879.h"
@@ -91,10 +92,19 @@ static const struct i2c_device_id ad7879_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, ad7879_id);
 
+#ifdef CONFIG_OF
+static const struct of_device_id ad7879_i2c_dt_ids[] = {
+	{ .compatible = "adi,ad7879-1", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad7879_i2c_dt_ids);
+#endif
+
 static struct i2c_driver ad7879_i2c_driver = {
 	.driver = {
 		.name	= "ad7879",
 		.pm	= &ad7879_pm_ops,
+		.of_match_table = of_match_ptr(ad7879_i2c_dt_ids),
 	},
 	.probe		= ad7879_i2c_probe,
 	.remove		= ad7879_i2c_remove,
diff --git a/drivers/input/touchscreen/ad7879-spi.c b/drivers/input/touchscreen/ad7879-spi.c
index 48033c2..d42b6b9 100644
--- a/drivers/input/touchscreen/ad7879-spi.c
+++ b/drivers/input/touchscreen/ad7879-spi.c
@@ -10,6 +10,7 @@
 #include <linux/pm.h>
 #include <linux/spi/spi.h>
 #include <linux/module.h>
+#include <linux/of.h>
 
 #include "ad7879.h"
 
@@ -146,10 +147,19 @@ static int ad7879_spi_remove(struct spi_device *spi)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id ad7879_spi_dt_ids[] = {
+	{ .compatible = "adi,ad7879", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad7879_spi_dt_ids);
+#endif
+
 static struct spi_driver ad7879_spi_driver = {
 	.driver = {
 		.name	= "ad7879",
 		.pm	= &ad7879_pm_ops,
+		.of_match_table = of_match_ptr(ad7879_spi_dt_ids),
 	},
 	.probe		= ad7879_spi_probe,
 	.remove		= ad7879_spi_remove,
diff --git a/drivers/input/touchscreen/ad7879.c b/drivers/input/touchscreen/ad7879.c
index abd0220..093cb04 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -31,6 +31,7 @@
 #include <linux/i2c.h>
 #include <linux/gpio.h>
 
+#include <linux/input/touchscreen.h>
 #include <linux/platform_data/ad7879.h>
 #include <linux/module.h>
 #include "ad7879.h"
@@ -126,7 +127,6 @@ struct ad7879 {
 	u8			pen_down_acc_interval;
 	u8			median;
 	u16			x_plate_ohms;
-	u16			pressure_max;
 	u16			cmd_crtl1;
 	u16			cmd_crtl2;
 	u16			cmd_crtl3;
@@ -186,7 +186,7 @@ static int ad7879_report(struct ad7879 *ts)
 		 * Sample found inconsistent, pressure is beyond
 		 * the maximum. Don't report it to user space.
 		 */
-		if (Rt > ts->pressure_max)
+		if (Rt > input_abs_get_max(input_dev, ABS_PRESSURE))
 			return -EINVAL;
 
 		/*
@@ -469,7 +469,7 @@ static void ad7879_gpio_remove(struct ad7879 *ts)
 {
 	const struct ad7879_platform_data *pdata = dev_get_platdata(ts->dev);
 
-	if (pdata->gpio_export)
+	if (pdata && pdata->gpio_export)
 		gpiochip_remove(&ts->gc);
 
 }
@@ -485,6 +485,29 @@ static inline void ad7879_gpio_remove(struct ad7879 *ts)
 }
 #endif
 
+static int ad7879_parse_dt(struct device *dev, struct ad7879 *ts)
+{
+	int err;
+	u32 tmp;
+
+	err = device_property_read_u32(dev, "adi,resistance-plate-x", &tmp);
+	if (err) {
+		dev_err(dev, "failed to get resistance-plate-x property\n");
+		return err;
+	}
+	ts->x_plate_ohms = (u16)tmp;
+
+	device_property_read_u8(dev, "adi,first-conversion-delay", &ts->first_conversion_delay);
+	device_property_read_u8(dev, "adi,acquisition-time", &ts->acquisition_time);
+	device_property_read_u8(dev, "adi,median-filter-size", &ts->median);
+	device_property_read_u8(dev, "adi,averaging", &ts->averaging);
+	device_property_read_u8(dev, "adi,conversion-interval", &ts->pen_down_acc_interval);
+
+	ts->swap_xy = device_property_read_bool(dev, "touchscreen-swapped-x-y");
+
+	return 0;
+}
+
 struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
 			    const struct ad7879_bus_ops *bops)
 {
@@ -495,22 +518,37 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
 	u16 revid;
 
 	if (!irq) {
-		dev_err(dev, "no IRQ?\n");
-		err = -EINVAL;
-		goto err_out;
+		dev_err(dev, "No IRQ specified\n");
+		return ERR_PTR(-EINVAL);
 	}
 
-	if (!pdata) {
-		dev_err(dev, "no platform data?\n");
-		err = -EINVAL;
-		goto err_out;
+	ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
+	if (!ts)
+		return ERR_PTR(-ENOMEM);
+
+	if (pdata) {
+		/* Platform data use swapped axis (backward compatibility) */
+		ts->swap_xy = !pdata->swap_xy;
+
+		ts->x_plate_ohms = pdata->x_plate_ohms ? : 400;
+
+		ts->first_conversion_delay = pdata->first_conversion_delay;
+		ts->acquisition_time = pdata->acquisition_time;
+		ts->averaging = pdata->averaging;
+		ts->pen_down_acc_interval = pdata->pen_down_acc_interval;
+		ts->median = pdata->median;
+
+	} else if (dev->of_node) {
+		ad7879_parse_dt(dev, ts);
+	} else {
+		dev_err(dev, "No platform data\n");
+		return ERR_PTR(-EINVAL);
 	}
 
-	ts = kzalloc(sizeof(*ts), GFP_KERNEL);
-	input_dev = input_allocate_device();
-	if (!ts || !input_dev) {
-		err = -ENOMEM;
-		goto err_free_mem;
+	input_dev = devm_input_allocate_device(dev);
+	if (!input_dev) {
+		dev_err(dev, "Failed to allocate input device\n");
+		return ERR_PTR(-ENOMEM);
 	}
 
 	ts->bops = bops;
@@ -518,20 +556,7 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
 	ts->input = input_dev;
 	ts->irq = irq;
 
-	/* Use swapped axis by default (backward compatibility) */
-	ts->swap_xy = !pdata->swap_xy;
-
 	setup_timer(&ts->timer, ad7879_timer, (unsigned long) ts);
-
-	ts->x_plate_ohms = pdata->x_plate_ohms ? : 400;
-	ts->pressure_max = pdata->pressure_max ? : ~0;
-
-	ts->first_conversion_delay = pdata->first_conversion_delay;
-	ts->acquisition_time = pdata->acquisition_time;
-	ts->averaging = pdata->averaging;
-	ts->pen_down_acc_interval = pdata->pen_down_acc_interval;
-	ts->median = pdata->median;
-
 	snprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(dev));
 
 	input_dev->name = "AD7879 Touchscreen";
@@ -552,21 +577,33 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
 	__set_bit(EV_KEY, input_dev->evbit);
 	__set_bit(BTN_TOUCH, input_dev->keybit);
 
-	input_set_abs_params(input_dev, ABS_X,
-			pdata->x_min ? : 0,
-			pdata->x_max ? : MAX_12BIT,
-			0, 0);
-	input_set_abs_params(input_dev, ABS_Y,
-			pdata->y_min ? : 0,
-			pdata->y_max ? : MAX_12BIT,
-			0, 0);
-	input_set_abs_params(input_dev, ABS_PRESSURE,
-			pdata->pressure_min, pdata->pressure_max, 0, 0);
+	if (pdata) {
+		input_set_abs_params(input_dev, ABS_X,
+				pdata->x_min ? : 0,
+				pdata->x_max ? : MAX_12BIT,
+				0, 0);
+		input_set_abs_params(input_dev, ABS_Y,
+				pdata->y_min ? : 0,
+				pdata->y_max ? : MAX_12BIT,
+				0, 0);
+		input_set_abs_params(input_dev, ABS_PRESSURE,
+				pdata->pressure_min,
+				pdata->pressure_max ? : ~0,
+				0, 0);
+	} else {
+		input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, 0, 0);
+		input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, 0, 0);
+		touchscreen_parse_properties(input_dev, false);
+		if (!input_abs_get_max(input_dev, ABS_PRESSURE)) {
+			dev_err(dev, "Touchscreen pressure is not specified\n");
+			return ERR_PTR(-EINVAL);
+		}
+	}
 
 	err = ad7879_write(ts, AD7879_REG_CTRL2, AD7879_RESET);
 	if (err < 0) {
 		dev_err(dev, "Failed to write %s\n", input_dev->name);
-		goto err_free_mem;
+		return ERR_PTR(err);
 	}
 
 	revid = ad7879_read(ts, AD7879_REG_REVID);
@@ -575,8 +612,7 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
 	if (input_dev->id.product != devid) {
 		dev_err(dev, "Failed to probe %s (%x vs %x)\n",
 			input_dev->name, devid, revid);
-		err = -ENODEV;
-		goto err_free_mem;
+		return ERR_PTR(-ENODEV);
 	}
 
 	ts->cmd_crtl3 = AD7879_YPLUS_BIT |
@@ -596,23 +632,25 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
 			AD7879_ACQ(ts->acquisition_time) |
 			AD7879_TMR(ts->pen_down_acc_interval);
 
-	err = request_threaded_irq(ts->irq, NULL, ad7879_irq,
-				   IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-				   dev_name(dev), ts);
+	err = devm_request_threaded_irq(dev, ts->irq, NULL, ad7879_irq,
+					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					dev_name(dev), ts);
 	if (err) {
-		dev_err(dev, "irq %d busy?\n", ts->irq);
-		goto err_free_mem;
+		dev_err(dev, "Failed to request IRQ: %d\n", err);
+		return ERR_PTR(err);
 	}
 
 	__ad7879_disable(ts);
 
 	err = sysfs_create_group(&dev->kobj, &ad7879_attr_group);
 	if (err)
-		goto err_free_irq;
+		goto err_out;
 
-	err = ad7879_gpio_add(ts, pdata);
-	if (err)
-		goto err_remove_attr;
+	if (pdata) {
+		err = ad7879_gpio_add(ts, pdata);
+		if (err)
+			goto err_remove_attr;
+	}
 
 	err = input_register_device(input_dev);
 	if (err)
@@ -624,11 +662,6 @@ err_remove_gpio:
 	ad7879_gpio_remove(ts);
 err_remove_attr:
 	sysfs_remove_group(&dev->kobj, &ad7879_attr_group);
-err_free_irq:
-	free_irq(ts->irq, ts);
-err_free_mem:
-	input_free_device(input_dev);
-	kfree(ts);
 err_out:
 	return ERR_PTR(err);
 }
@@ -638,9 +671,7 @@ void ad7879_remove(struct ad7879 *ts)
 {
 	ad7879_gpio_remove(ts);
 	sysfs_remove_group(&ts->dev->kobj, &ad7879_attr_group);
-	free_irq(ts->irq, ts);
 	input_unregister_device(ts->input);
-	kfree(ts);
 }
 EXPORT_SYMBOL(ad7879_remove);
 
-- 
2.7.0

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

* Re: [PATCH v3 3/3] input: touchscreen: ad7879: add device tree support
  2016-02-02 23:20 ` [PATCH v3 3/3] input: touchscreen: ad7879: add device tree support Stefan Agner
@ 2016-02-03  7:41     ` Michael Hennerich
  2016-03-08 18:50   ` Dmitry Torokhov
  1 sibling, 0 replies; 10+ messages in thread
From: Michael Hennerich @ 2016-02-03  7:41 UTC (permalink / raw)
  To: Stefan Agner, dmitry.torokhov, robh+dt
  Cc: mark.rutland, ijc+devicetree, galak, realmz6, broonie, jic23,
	linux-input, devicetree, linux-kernel

On 02/03/2016 12:20 AM, Stefan Agner wrote:
> Add device tree support for the I2C and SPI variant of AD7879(-1).
> This allows to specify the touchscreen controller as a I2C client
> node or SPI slave device. Most of the options available in platform
> data are also available as device tree properties, the only exception
> being GPIO capabilities, which can not be activated through device
> tree currently.
>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Acked-by: Michael Hennerich <michael.hennerich@analog.com>

> ---
> Changes since v2:
> - do not free driver data and irq on remove (since we are using devm now)
> Changes since v1:
> - Move device tree parsing to main driver file ad7879.c
> - Use common touchscreen_parse_properties for common properties
> - Use device_property_* API
> - Use struct ad7879 directly to store parsed values
> - Support SPI variant through device tree too (untested)
> - Add vendor prefix to vendor specific dt properties
>
>   .../bindings/input/touchscreen/ad7879.txt          |  53 ++++++++
>   drivers/input/touchscreen/ad7879-i2c.c             |  10 ++
>   drivers/input/touchscreen/ad7879-spi.c             |  10 ++
>   drivers/input/touchscreen/ad7879.c                 | 145 +++++++++++++--------
>   4 files changed, 161 insertions(+), 57 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ad7879.txt
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/ad7879.txt b/Documentation/devicetree/bindings/input/touchscreen/ad7879.txt
> new file mode 100644
> index 0000000..e3f22d2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/ad7879.txt
> @@ -0,0 +1,53 @@
> +* Analog Devices AD7879(-1)/AD7889(-1) touchscreen interface (SPI/I2C)
> +
> +Required properties:
> +- compatible			: for SPI slave, use "adi,ad7879"
> +				  for I2C slave, use "adi,ad7879-1"
> +- reg				: SPI chipselect/I2C slave address
> +				  See spi-bus.txt for more SPI slave properties
> +- interrupt-parent		: the phandle for the interrupt controller
> +- interrupts			: touch controller interrupt
> +- touchscreen-max-pressure	: maximum reported pressure
> +- adi,resistance-plate-x	: total resistance of X-plate (for pressure
> +				  calculation)
> +Optional properties:
> +- touchscreen-swapped-x-y	: X and Y axis are swapped (boolean)
> +- adi,first-conversion-delay	: 0-12: In 128us steps (starting with 128us)
> +				  13  : 2.560ms
> +				  14  : 3.584ms
> +				  15  : 4.096ms
> +				  This property has to be a '/bits/ 8' value
> +- adi,acquisition-time		: 0: 2us
> +				  1: 4us
> +				  2: 8us
> +				  3: 16us
> +				  This property has to be a '/bits/ 8' value
> +- adi,median-filter-size	: 0: disabled
> +				  1: 4 measurements
> +				  2: 8 measurements
> +				  3: 16 measurements
> +				  This property has to be a '/bits/ 8' value
> +- adi,averaging			: 0: 2 middle values (1 if median disabled)
> +				  1: 4 middle values
> +				  2: 8 middle values
> +				  3: 16 values
> +				  This property has to be a '/bits/ 8' value
> +- adi,conversion-interval:	: 0    : convert one time only
> +				  1-255: 515us + val * 35us (up to 9.440ms)
> +				  This property has to be a '/bits/ 8' value
> +
> +Example:
> +
> +	ad7879@2c {
> +		compatible = "adi,ad7879-1";
> +		reg = <0x2c>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <13 IRQ_TYPE_EDGE_FALLING>;
> +		touchscreen-max-pressure = <4096>;
> +		adi,resistance-plate-x = <120>;
> +		adi,first-conversion-delay = /bits/ 8 <3>;
> +		adi,acquisition-time = /bits/ 8 <1>;
> +		adi,median-filter-size = /bits/ 8 <2>;
> +		adi,averaging = /bits/ 8 <1>;
> +		adi,conversion-interval = /bits/ 8 <255>;
> +	};
> diff --git a/drivers/input/touchscreen/ad7879-i2c.c b/drivers/input/touchscreen/ad7879-i2c.c
> index d66962c..58f72e0 100644
> --- a/drivers/input/touchscreen/ad7879-i2c.c
> +++ b/drivers/input/touchscreen/ad7879-i2c.c
> @@ -10,6 +10,7 @@
>   #include <linux/i2c.h>
>   #include <linux/module.h>
>   #include <linux/types.h>
> +#include <linux/of.h>
>   #include <linux/pm.h>
>
>   #include "ad7879.h"
> @@ -91,10 +92,19 @@ static const struct i2c_device_id ad7879_id[] = {
>   };
>   MODULE_DEVICE_TABLE(i2c, ad7879_id);
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id ad7879_i2c_dt_ids[] = {
> +	{ .compatible = "adi,ad7879-1", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ad7879_i2c_dt_ids);
> +#endif
> +
>   static struct i2c_driver ad7879_i2c_driver = {
>   	.driver = {
>   		.name	= "ad7879",
>   		.pm	= &ad7879_pm_ops,
> +		.of_match_table = of_match_ptr(ad7879_i2c_dt_ids),
>   	},
>   	.probe		= ad7879_i2c_probe,
>   	.remove		= ad7879_i2c_remove,
> diff --git a/drivers/input/touchscreen/ad7879-spi.c b/drivers/input/touchscreen/ad7879-spi.c
> index 48033c2..d42b6b9 100644
> --- a/drivers/input/touchscreen/ad7879-spi.c
> +++ b/drivers/input/touchscreen/ad7879-spi.c
> @@ -10,6 +10,7 @@
>   #include <linux/pm.h>
>   #include <linux/spi/spi.h>
>   #include <linux/module.h>
> +#include <linux/of.h>
>
>   #include "ad7879.h"
>
> @@ -146,10 +147,19 @@ static int ad7879_spi_remove(struct spi_device *spi)
>   	return 0;
>   }
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id ad7879_spi_dt_ids[] = {
> +	{ .compatible = "adi,ad7879", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ad7879_spi_dt_ids);
> +#endif
> +
>   static struct spi_driver ad7879_spi_driver = {
>   	.driver = {
>   		.name	= "ad7879",
>   		.pm	= &ad7879_pm_ops,
> +		.of_match_table = of_match_ptr(ad7879_spi_dt_ids),
>   	},
>   	.probe		= ad7879_spi_probe,
>   	.remove		= ad7879_spi_remove,
> diff --git a/drivers/input/touchscreen/ad7879.c b/drivers/input/touchscreen/ad7879.c
> index abd0220..093cb04 100644
> --- a/drivers/input/touchscreen/ad7879.c
> +++ b/drivers/input/touchscreen/ad7879.c
> @@ -31,6 +31,7 @@
>   #include <linux/i2c.h>
>   #include <linux/gpio.h>
>
> +#include <linux/input/touchscreen.h>
>   #include <linux/platform_data/ad7879.h>
>   #include <linux/module.h>
>   #include "ad7879.h"
> @@ -126,7 +127,6 @@ struct ad7879 {
>   	u8			pen_down_acc_interval;
>   	u8			median;
>   	u16			x_plate_ohms;
> -	u16			pressure_max;
>   	u16			cmd_crtl1;
>   	u16			cmd_crtl2;
>   	u16			cmd_crtl3;
> @@ -186,7 +186,7 @@ static int ad7879_report(struct ad7879 *ts)
>   		 * Sample found inconsistent, pressure is beyond
>   		 * the maximum. Don't report it to user space.
>   		 */
> -		if (Rt > ts->pressure_max)
> +		if (Rt > input_abs_get_max(input_dev, ABS_PRESSURE))
>   			return -EINVAL;
>
>   		/*
> @@ -469,7 +469,7 @@ static void ad7879_gpio_remove(struct ad7879 *ts)
>   {
>   	const struct ad7879_platform_data *pdata = dev_get_platdata(ts->dev);
>
> -	if (pdata->gpio_export)
> +	if (pdata && pdata->gpio_export)
>   		gpiochip_remove(&ts->gc);
>
>   }
> @@ -485,6 +485,29 @@ static inline void ad7879_gpio_remove(struct ad7879 *ts)
>   }
>   #endif
>
> +static int ad7879_parse_dt(struct device *dev, struct ad7879 *ts)
> +{
> +	int err;
> +	u32 tmp;
> +
> +	err = device_property_read_u32(dev, "adi,resistance-plate-x", &tmp);
> +	if (err) {
> +		dev_err(dev, "failed to get resistance-plate-x property\n");
> +		return err;
> +	}
> +	ts->x_plate_ohms = (u16)tmp;
> +
> +	device_property_read_u8(dev, "adi,first-conversion-delay", &ts->first_conversion_delay);
> +	device_property_read_u8(dev, "adi,acquisition-time", &ts->acquisition_time);
> +	device_property_read_u8(dev, "adi,median-filter-size", &ts->median);
> +	device_property_read_u8(dev, "adi,averaging", &ts->averaging);
> +	device_property_read_u8(dev, "adi,conversion-interval", &ts->pen_down_acc_interval);
> +
> +	ts->swap_xy = device_property_read_bool(dev, "touchscreen-swapped-x-y");
> +
> +	return 0;
> +}
> +
>   struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
>   			    const struct ad7879_bus_ops *bops)
>   {
> @@ -495,22 +518,37 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
>   	u16 revid;
>
>   	if (!irq) {
> -		dev_err(dev, "no IRQ?\n");
> -		err = -EINVAL;
> -		goto err_out;
> +		dev_err(dev, "No IRQ specified\n");
> +		return ERR_PTR(-EINVAL);
>   	}
>
> -	if (!pdata) {
> -		dev_err(dev, "no platform data?\n");
> -		err = -EINVAL;
> -		goto err_out;
> +	ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
> +	if (!ts)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (pdata) {
> +		/* Platform data use swapped axis (backward compatibility) */
> +		ts->swap_xy = !pdata->swap_xy;
> +
> +		ts->x_plate_ohms = pdata->x_plate_ohms ? : 400;
> +
> +		ts->first_conversion_delay = pdata->first_conversion_delay;
> +		ts->acquisition_time = pdata->acquisition_time;
> +		ts->averaging = pdata->averaging;
> +		ts->pen_down_acc_interval = pdata->pen_down_acc_interval;
> +		ts->median = pdata->median;
> +
> +	} else if (dev->of_node) {
> +		ad7879_parse_dt(dev, ts);
> +	} else {
> +		dev_err(dev, "No platform data\n");
> +		return ERR_PTR(-EINVAL);
>   	}
>
> -	ts = kzalloc(sizeof(*ts), GFP_KERNEL);
> -	input_dev = input_allocate_device();
> -	if (!ts || !input_dev) {
> -		err = -ENOMEM;
> -		goto err_free_mem;
> +	input_dev = devm_input_allocate_device(dev);
> +	if (!input_dev) {
> +		dev_err(dev, "Failed to allocate input device\n");
> +		return ERR_PTR(-ENOMEM);
>   	}
>
>   	ts->bops = bops;
> @@ -518,20 +556,7 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
>   	ts->input = input_dev;
>   	ts->irq = irq;
>
> -	/* Use swapped axis by default (backward compatibility) */
> -	ts->swap_xy = !pdata->swap_xy;
> -
>   	setup_timer(&ts->timer, ad7879_timer, (unsigned long) ts);
> -
> -	ts->x_plate_ohms = pdata->x_plate_ohms ? : 400;
> -	ts->pressure_max = pdata->pressure_max ? : ~0;
> -
> -	ts->first_conversion_delay = pdata->first_conversion_delay;
> -	ts->acquisition_time = pdata->acquisition_time;
> -	ts->averaging = pdata->averaging;
> -	ts->pen_down_acc_interval = pdata->pen_down_acc_interval;
> -	ts->median = pdata->median;
> -
>   	snprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(dev));
>
>   	input_dev->name = "AD7879 Touchscreen";
> @@ -552,21 +577,33 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
>   	__set_bit(EV_KEY, input_dev->evbit);
>   	__set_bit(BTN_TOUCH, input_dev->keybit);
>
> -	input_set_abs_params(input_dev, ABS_X,
> -			pdata->x_min ? : 0,
> -			pdata->x_max ? : MAX_12BIT,
> -			0, 0);
> -	input_set_abs_params(input_dev, ABS_Y,
> -			pdata->y_min ? : 0,
> -			pdata->y_max ? : MAX_12BIT,
> -			0, 0);
> -	input_set_abs_params(input_dev, ABS_PRESSURE,
> -			pdata->pressure_min, pdata->pressure_max, 0, 0);
> +	if (pdata) {
> +		input_set_abs_params(input_dev, ABS_X,
> +				pdata->x_min ? : 0,
> +				pdata->x_max ? : MAX_12BIT,
> +				0, 0);
> +		input_set_abs_params(input_dev, ABS_Y,
> +				pdata->y_min ? : 0,
> +				pdata->y_max ? : MAX_12BIT,
> +				0, 0);
> +		input_set_abs_params(input_dev, ABS_PRESSURE,
> +				pdata->pressure_min,
> +				pdata->pressure_max ? : ~0,
> +				0, 0);
> +	} else {
> +		input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, 0, 0);
> +		input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, 0, 0);
> +		touchscreen_parse_properties(input_dev, false);
> +		if (!input_abs_get_max(input_dev, ABS_PRESSURE)) {
> +			dev_err(dev, "Touchscreen pressure is not specified\n");
> +			return ERR_PTR(-EINVAL);
> +		}
> +	}
>
>   	err = ad7879_write(ts, AD7879_REG_CTRL2, AD7879_RESET);
>   	if (err < 0) {
>   		dev_err(dev, "Failed to write %s\n", input_dev->name);
> -		goto err_free_mem;
> +		return ERR_PTR(err);
>   	}
>
>   	revid = ad7879_read(ts, AD7879_REG_REVID);
> @@ -575,8 +612,7 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
>   	if (input_dev->id.product != devid) {
>   		dev_err(dev, "Failed to probe %s (%x vs %x)\n",
>   			input_dev->name, devid, revid);
> -		err = -ENODEV;
> -		goto err_free_mem;
> +		return ERR_PTR(-ENODEV);
>   	}
>
>   	ts->cmd_crtl3 = AD7879_YPLUS_BIT |
> @@ -596,23 +632,25 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
>   			AD7879_ACQ(ts->acquisition_time) |
>   			AD7879_TMR(ts->pen_down_acc_interval);
>
> -	err = request_threaded_irq(ts->irq, NULL, ad7879_irq,
> -				   IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> -				   dev_name(dev), ts);
> +	err = devm_request_threaded_irq(dev, ts->irq, NULL, ad7879_irq,
> +					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +					dev_name(dev), ts);
>   	if (err) {
> -		dev_err(dev, "irq %d busy?\n", ts->irq);
> -		goto err_free_mem;
> +		dev_err(dev, "Failed to request IRQ: %d\n", err);
> +		return ERR_PTR(err);
>   	}
>
>   	__ad7879_disable(ts);
>
>   	err = sysfs_create_group(&dev->kobj, &ad7879_attr_group);
>   	if (err)
> -		goto err_free_irq;
> +		goto err_out;
>
> -	err = ad7879_gpio_add(ts, pdata);
> -	if (err)
> -		goto err_remove_attr;
> +	if (pdata) {
> +		err = ad7879_gpio_add(ts, pdata);
> +		if (err)
> +			goto err_remove_attr;
> +	}
>
>   	err = input_register_device(input_dev);
>   	if (err)
> @@ -624,11 +662,6 @@ err_remove_gpio:
>   	ad7879_gpio_remove(ts);
>   err_remove_attr:
>   	sysfs_remove_group(&dev->kobj, &ad7879_attr_group);
> -err_free_irq:
> -	free_irq(ts->irq, ts);
> -err_free_mem:
> -	input_free_device(input_dev);
> -	kfree(ts);
>   err_out:
>   	return ERR_PTR(err);
>   }
> @@ -638,9 +671,7 @@ void ad7879_remove(struct ad7879 *ts)
>   {
>   	ad7879_gpio_remove(ts);
>   	sysfs_remove_group(&ts->dev->kobj, &ad7879_attr_group);
> -	free_irq(ts->irq, ts);
>   	input_unregister_device(ts->input);
> -	kfree(ts);
>   }
>   EXPORT_SYMBOL(ad7879_remove);
>
>


-- 
Greetings,
Michael

--
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

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

* Re: [PATCH v3 3/3] input: touchscreen: ad7879: add device tree support
@ 2016-02-03  7:41     ` Michael Hennerich
  0 siblings, 0 replies; 10+ messages in thread
From: Michael Hennerich @ 2016-02-03  7:41 UTC (permalink / raw)
  To: Stefan Agner, dmitry.torokhov, robh+dt
  Cc: mark.rutland, ijc+devicetree, galak, realmz6, broonie, jic23,
	linux-input, devicetree, linux-kernel

On 02/03/2016 12:20 AM, Stefan Agner wrote:
> Add device tree support for the I2C and SPI variant of AD7879(-1).
> This allows to specify the touchscreen controller as a I2C client
> node or SPI slave device. Most of the options available in platform
> data are also available as device tree properties, the only exception
> being GPIO capabilities, which can not be activated through device
> tree currently.
>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Acked-by: Michael Hennerich <michael.hennerich@analog.com>

> ---
> Changes since v2:
> - do not free driver data and irq on remove (since we are using devm now)
> Changes since v1:
> - Move device tree parsing to main driver file ad7879.c
> - Use common touchscreen_parse_properties for common properties
> - Use device_property_* API
> - Use struct ad7879 directly to store parsed values
> - Support SPI variant through device tree too (untested)
> - Add vendor prefix to vendor specific dt properties
>
>   .../bindings/input/touchscreen/ad7879.txt          |  53 ++++++++
>   drivers/input/touchscreen/ad7879-i2c.c             |  10 ++
>   drivers/input/touchscreen/ad7879-spi.c             |  10 ++
>   drivers/input/touchscreen/ad7879.c                 | 145 +++++++++++++--------
>   4 files changed, 161 insertions(+), 57 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ad7879.txt
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/ad7879.txt b/Documentation/devicetree/bindings/input/touchscreen/ad7879.txt
> new file mode 100644
> index 0000000..e3f22d2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/ad7879.txt
> @@ -0,0 +1,53 @@
> +* Analog Devices AD7879(-1)/AD7889(-1) touchscreen interface (SPI/I2C)
> +
> +Required properties:
> +- compatible			: for SPI slave, use "adi,ad7879"
> +				  for I2C slave, use "adi,ad7879-1"
> +- reg				: SPI chipselect/I2C slave address
> +				  See spi-bus.txt for more SPI slave properties
> +- interrupt-parent		: the phandle for the interrupt controller
> +- interrupts			: touch controller interrupt
> +- touchscreen-max-pressure	: maximum reported pressure
> +- adi,resistance-plate-x	: total resistance of X-plate (for pressure
> +				  calculation)
> +Optional properties:
> +- touchscreen-swapped-x-y	: X and Y axis are swapped (boolean)
> +- adi,first-conversion-delay	: 0-12: In 128us steps (starting with 128us)
> +				  13  : 2.560ms
> +				  14  : 3.584ms
> +				  15  : 4.096ms
> +				  This property has to be a '/bits/ 8' value
> +- adi,acquisition-time		: 0: 2us
> +				  1: 4us
> +				  2: 8us
> +				  3: 16us
> +				  This property has to be a '/bits/ 8' value
> +- adi,median-filter-size	: 0: disabled
> +				  1: 4 measurements
> +				  2: 8 measurements
> +				  3: 16 measurements
> +				  This property has to be a '/bits/ 8' value
> +- adi,averaging			: 0: 2 middle values (1 if median disabled)
> +				  1: 4 middle values
> +				  2: 8 middle values
> +				  3: 16 values
> +				  This property has to be a '/bits/ 8' value
> +- adi,conversion-interval:	: 0    : convert one time only
> +				  1-255: 515us + val * 35us (up to 9.440ms)
> +				  This property has to be a '/bits/ 8' value
> +
> +Example:
> +
> +	ad7879@2c {
> +		compatible = "adi,ad7879-1";
> +		reg = <0x2c>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <13 IRQ_TYPE_EDGE_FALLING>;
> +		touchscreen-max-pressure = <4096>;
> +		adi,resistance-plate-x = <120>;
> +		adi,first-conversion-delay = /bits/ 8 <3>;
> +		adi,acquisition-time = /bits/ 8 <1>;
> +		adi,median-filter-size = /bits/ 8 <2>;
> +		adi,averaging = /bits/ 8 <1>;
> +		adi,conversion-interval = /bits/ 8 <255>;
> +	};
> diff --git a/drivers/input/touchscreen/ad7879-i2c.c b/drivers/input/touchscreen/ad7879-i2c.c
> index d66962c..58f72e0 100644
> --- a/drivers/input/touchscreen/ad7879-i2c.c
> +++ b/drivers/input/touchscreen/ad7879-i2c.c
> @@ -10,6 +10,7 @@
>   #include <linux/i2c.h>
>   #include <linux/module.h>
>   #include <linux/types.h>
> +#include <linux/of.h>
>   #include <linux/pm.h>
>
>   #include "ad7879.h"
> @@ -91,10 +92,19 @@ static const struct i2c_device_id ad7879_id[] = {
>   };
>   MODULE_DEVICE_TABLE(i2c, ad7879_id);
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id ad7879_i2c_dt_ids[] = {
> +	{ .compatible = "adi,ad7879-1", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ad7879_i2c_dt_ids);
> +#endif
> +
>   static struct i2c_driver ad7879_i2c_driver = {
>   	.driver = {
>   		.name	= "ad7879",
>   		.pm	= &ad7879_pm_ops,
> +		.of_match_table = of_match_ptr(ad7879_i2c_dt_ids),
>   	},
>   	.probe		= ad7879_i2c_probe,
>   	.remove		= ad7879_i2c_remove,
> diff --git a/drivers/input/touchscreen/ad7879-spi.c b/drivers/input/touchscreen/ad7879-spi.c
> index 48033c2..d42b6b9 100644
> --- a/drivers/input/touchscreen/ad7879-spi.c
> +++ b/drivers/input/touchscreen/ad7879-spi.c
> @@ -10,6 +10,7 @@
>   #include <linux/pm.h>
>   #include <linux/spi/spi.h>
>   #include <linux/module.h>
> +#include <linux/of.h>
>
>   #include "ad7879.h"
>
> @@ -146,10 +147,19 @@ static int ad7879_spi_remove(struct spi_device *spi)
>   	return 0;
>   }
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id ad7879_spi_dt_ids[] = {
> +	{ .compatible = "adi,ad7879", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ad7879_spi_dt_ids);
> +#endif
> +
>   static struct spi_driver ad7879_spi_driver = {
>   	.driver = {
>   		.name	= "ad7879",
>   		.pm	= &ad7879_pm_ops,
> +		.of_match_table = of_match_ptr(ad7879_spi_dt_ids),
>   	},
>   	.probe		= ad7879_spi_probe,
>   	.remove		= ad7879_spi_remove,
> diff --git a/drivers/input/touchscreen/ad7879.c b/drivers/input/touchscreen/ad7879.c
> index abd0220..093cb04 100644
> --- a/drivers/input/touchscreen/ad7879.c
> +++ b/drivers/input/touchscreen/ad7879.c
> @@ -31,6 +31,7 @@
>   #include <linux/i2c.h>
>   #include <linux/gpio.h>
>
> +#include <linux/input/touchscreen.h>
>   #include <linux/platform_data/ad7879.h>
>   #include <linux/module.h>
>   #include "ad7879.h"
> @@ -126,7 +127,6 @@ struct ad7879 {
>   	u8			pen_down_acc_interval;
>   	u8			median;
>   	u16			x_plate_ohms;
> -	u16			pressure_max;
>   	u16			cmd_crtl1;
>   	u16			cmd_crtl2;
>   	u16			cmd_crtl3;
> @@ -186,7 +186,7 @@ static int ad7879_report(struct ad7879 *ts)
>   		 * Sample found inconsistent, pressure is beyond
>   		 * the maximum. Don't report it to user space.
>   		 */
> -		if (Rt > ts->pressure_max)
> +		if (Rt > input_abs_get_max(input_dev, ABS_PRESSURE))
>   			return -EINVAL;
>
>   		/*
> @@ -469,7 +469,7 @@ static void ad7879_gpio_remove(struct ad7879 *ts)
>   {
>   	const struct ad7879_platform_data *pdata = dev_get_platdata(ts->dev);
>
> -	if (pdata->gpio_export)
> +	if (pdata && pdata->gpio_export)
>   		gpiochip_remove(&ts->gc);
>
>   }
> @@ -485,6 +485,29 @@ static inline void ad7879_gpio_remove(struct ad7879 *ts)
>   }
>   #endif
>
> +static int ad7879_parse_dt(struct device *dev, struct ad7879 *ts)
> +{
> +	int err;
> +	u32 tmp;
> +
> +	err = device_property_read_u32(dev, "adi,resistance-plate-x", &tmp);
> +	if (err) {
> +		dev_err(dev, "failed to get resistance-plate-x property\n");
> +		return err;
> +	}
> +	ts->x_plate_ohms = (u16)tmp;
> +
> +	device_property_read_u8(dev, "adi,first-conversion-delay", &ts->first_conversion_delay);
> +	device_property_read_u8(dev, "adi,acquisition-time", &ts->acquisition_time);
> +	device_property_read_u8(dev, "adi,median-filter-size", &ts->median);
> +	device_property_read_u8(dev, "adi,averaging", &ts->averaging);
> +	device_property_read_u8(dev, "adi,conversion-interval", &ts->pen_down_acc_interval);
> +
> +	ts->swap_xy = device_property_read_bool(dev, "touchscreen-swapped-x-y");
> +
> +	return 0;
> +}
> +
>   struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
>   			    const struct ad7879_bus_ops *bops)
>   {
> @@ -495,22 +518,37 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
>   	u16 revid;
>
>   	if (!irq) {
> -		dev_err(dev, "no IRQ?\n");
> -		err = -EINVAL;
> -		goto err_out;
> +		dev_err(dev, "No IRQ specified\n");
> +		return ERR_PTR(-EINVAL);
>   	}
>
> -	if (!pdata) {
> -		dev_err(dev, "no platform data?\n");
> -		err = -EINVAL;
> -		goto err_out;
> +	ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
> +	if (!ts)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (pdata) {
> +		/* Platform data use swapped axis (backward compatibility) */
> +		ts->swap_xy = !pdata->swap_xy;
> +
> +		ts->x_plate_ohms = pdata->x_plate_ohms ? : 400;
> +
> +		ts->first_conversion_delay = pdata->first_conversion_delay;
> +		ts->acquisition_time = pdata->acquisition_time;
> +		ts->averaging = pdata->averaging;
> +		ts->pen_down_acc_interval = pdata->pen_down_acc_interval;
> +		ts->median = pdata->median;
> +
> +	} else if (dev->of_node) {
> +		ad7879_parse_dt(dev, ts);
> +	} else {
> +		dev_err(dev, "No platform data\n");
> +		return ERR_PTR(-EINVAL);
>   	}
>
> -	ts = kzalloc(sizeof(*ts), GFP_KERNEL);
> -	input_dev = input_allocate_device();
> -	if (!ts || !input_dev) {
> -		err = -ENOMEM;
> -		goto err_free_mem;
> +	input_dev = devm_input_allocate_device(dev);
> +	if (!input_dev) {
> +		dev_err(dev, "Failed to allocate input device\n");
> +		return ERR_PTR(-ENOMEM);
>   	}
>
>   	ts->bops = bops;
> @@ -518,20 +556,7 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
>   	ts->input = input_dev;
>   	ts->irq = irq;
>
> -	/* Use swapped axis by default (backward compatibility) */
> -	ts->swap_xy = !pdata->swap_xy;
> -
>   	setup_timer(&ts->timer, ad7879_timer, (unsigned long) ts);
> -
> -	ts->x_plate_ohms = pdata->x_plate_ohms ? : 400;
> -	ts->pressure_max = pdata->pressure_max ? : ~0;
> -
> -	ts->first_conversion_delay = pdata->first_conversion_delay;
> -	ts->acquisition_time = pdata->acquisition_time;
> -	ts->averaging = pdata->averaging;
> -	ts->pen_down_acc_interval = pdata->pen_down_acc_interval;
> -	ts->median = pdata->median;
> -
>   	snprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(dev));
>
>   	input_dev->name = "AD7879 Touchscreen";
> @@ -552,21 +577,33 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
>   	__set_bit(EV_KEY, input_dev->evbit);
>   	__set_bit(BTN_TOUCH, input_dev->keybit);
>
> -	input_set_abs_params(input_dev, ABS_X,
> -			pdata->x_min ? : 0,
> -			pdata->x_max ? : MAX_12BIT,
> -			0, 0);
> -	input_set_abs_params(input_dev, ABS_Y,
> -			pdata->y_min ? : 0,
> -			pdata->y_max ? : MAX_12BIT,
> -			0, 0);
> -	input_set_abs_params(input_dev, ABS_PRESSURE,
> -			pdata->pressure_min, pdata->pressure_max, 0, 0);
> +	if (pdata) {
> +		input_set_abs_params(input_dev, ABS_X,
> +				pdata->x_min ? : 0,
> +				pdata->x_max ? : MAX_12BIT,
> +				0, 0);
> +		input_set_abs_params(input_dev, ABS_Y,
> +				pdata->y_min ? : 0,
> +				pdata->y_max ? : MAX_12BIT,
> +				0, 0);
> +		input_set_abs_params(input_dev, ABS_PRESSURE,
> +				pdata->pressure_min,
> +				pdata->pressure_max ? : ~0,
> +				0, 0);
> +	} else {
> +		input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, 0, 0);
> +		input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, 0, 0);
> +		touchscreen_parse_properties(input_dev, false);
> +		if (!input_abs_get_max(input_dev, ABS_PRESSURE)) {
> +			dev_err(dev, "Touchscreen pressure is not specified\n");
> +			return ERR_PTR(-EINVAL);
> +		}
> +	}
>
>   	err = ad7879_write(ts, AD7879_REG_CTRL2, AD7879_RESET);
>   	if (err < 0) {
>   		dev_err(dev, "Failed to write %s\n", input_dev->name);
> -		goto err_free_mem;
> +		return ERR_PTR(err);
>   	}
>
>   	revid = ad7879_read(ts, AD7879_REG_REVID);
> @@ -575,8 +612,7 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
>   	if (input_dev->id.product != devid) {
>   		dev_err(dev, "Failed to probe %s (%x vs %x)\n",
>   			input_dev->name, devid, revid);
> -		err = -ENODEV;
> -		goto err_free_mem;
> +		return ERR_PTR(-ENODEV);
>   	}
>
>   	ts->cmd_crtl3 = AD7879_YPLUS_BIT |
> @@ -596,23 +632,25 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
>   			AD7879_ACQ(ts->acquisition_time) |
>   			AD7879_TMR(ts->pen_down_acc_interval);
>
> -	err = request_threaded_irq(ts->irq, NULL, ad7879_irq,
> -				   IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> -				   dev_name(dev), ts);
> +	err = devm_request_threaded_irq(dev, ts->irq, NULL, ad7879_irq,
> +					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +					dev_name(dev), ts);
>   	if (err) {
> -		dev_err(dev, "irq %d busy?\n", ts->irq);
> -		goto err_free_mem;
> +		dev_err(dev, "Failed to request IRQ: %d\n", err);
> +		return ERR_PTR(err);
>   	}
>
>   	__ad7879_disable(ts);
>
>   	err = sysfs_create_group(&dev->kobj, &ad7879_attr_group);
>   	if (err)
> -		goto err_free_irq;
> +		goto err_out;
>
> -	err = ad7879_gpio_add(ts, pdata);
> -	if (err)
> -		goto err_remove_attr;
> +	if (pdata) {
> +		err = ad7879_gpio_add(ts, pdata);
> +		if (err)
> +			goto err_remove_attr;
> +	}
>
>   	err = input_register_device(input_dev);
>   	if (err)
> @@ -624,11 +662,6 @@ err_remove_gpio:
>   	ad7879_gpio_remove(ts);
>   err_remove_attr:
>   	sysfs_remove_group(&dev->kobj, &ad7879_attr_group);
> -err_free_irq:
> -	free_irq(ts->irq, ts);
> -err_free_mem:
> -	input_free_device(input_dev);
> -	kfree(ts);
>   err_out:
>   	return ERR_PTR(err);
>   }
> @@ -638,9 +671,7 @@ void ad7879_remove(struct ad7879 *ts)
>   {
>   	ad7879_gpio_remove(ts);
>   	sysfs_remove_group(&ts->dev->kobj, &ad7879_attr_group);
> -	free_irq(ts->irq, ts);
>   	input_unregister_device(ts->input);
> -	kfree(ts);
>   }
>   EXPORT_SYMBOL(ad7879_remove);
>
>


-- 
Greetings,
Michael

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

* Re: [PATCH v3 2/3] input: touchscreen: ad7879: fix default x/y axis assignment
  2016-02-02 23:20 ` [PATCH v3 2/3] input: touchscreen: ad7879: fix default x/y axis assignment Stefan Agner
@ 2016-02-03  7:41     ` Michael Hennerich
  0 siblings, 0 replies; 10+ messages in thread
From: Michael Hennerich @ 2016-02-03  7:41 UTC (permalink / raw)
  To: Stefan Agner, dmitry.torokhov, robh+dt
  Cc: mark.rutland, ijc+devicetree, galak, realmz6, broonie, jic23,
	linux-input, devicetree, linux-kernel

On 02/03/2016 12:20 AM, Stefan Agner wrote:
> The X/Y position measurements read from the controller are interpreted
> wrong. The first measurement X+ contains the Y position, and the second
> measurement Y+ the X position (see also Table 11 Register Table in the
> data sheet).
>
> The problem is already known and a swap option has been introduced:
> commit 6680884a4420 ("Input: ad7879 - add option to correct xy axis")
>
> However, the meaning of the new boolean is inverted since the underlying
> values are already swapped. Let ts->swap_xy set to true actually be the
> swapped configuration of the two axis.
>
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Acked-by: Michael Hennerich <michael.hennerich@analog.com>

> ---
> Changes since v2:
> - (none)
> Changes since v1:
> - Keep axis swapped by default when using platform data
>
>   drivers/input/touchscreen/ad7879.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/touchscreen/ad7879.c b/drivers/input/touchscreen/ad7879.c
> index 93b8ea2..abd0220 100644
> --- a/drivers/input/touchscreen/ad7879.c
> +++ b/drivers/input/touchscreen/ad7879.c
> @@ -94,8 +94,8 @@
>   #define AD7879_TEMP_BIT			(1<<1)
>
>   enum {
> -	AD7879_SEQ_XPOS  = 0,
> -	AD7879_SEQ_YPOS  = 1,
> +	AD7879_SEQ_YPOS  = 0,
> +	AD7879_SEQ_XPOS  = 1,
>   	AD7879_SEQ_Z1    = 2,
>   	AD7879_SEQ_Z2    = 3,
>   	AD7879_NR_SENSE  = 4,
> @@ -517,7 +517,9 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
>   	ts->dev = dev;
>   	ts->input = input_dev;
>   	ts->irq = irq;
> -	ts->swap_xy = pdata->swap_xy;
> +
> +	/* Use swapped axis by default (backward compatibility) */
> +	ts->swap_xy = !pdata->swap_xy;
>
>   	setup_timer(&ts->timer, ad7879_timer, (unsigned long) ts);
>
>


-- 
Greetings,
Michael

--
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

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

* Re: [PATCH v3 2/3] input: touchscreen: ad7879: fix default x/y axis assignment
@ 2016-02-03  7:41     ` Michael Hennerich
  0 siblings, 0 replies; 10+ messages in thread
From: Michael Hennerich @ 2016-02-03  7:41 UTC (permalink / raw)
  To: Stefan Agner, dmitry.torokhov, robh+dt
  Cc: mark.rutland, ijc+devicetree, galak, realmz6, broonie, jic23,
	linux-input, devicetree, linux-kernel

On 02/03/2016 12:20 AM, Stefan Agner wrote:
> The X/Y position measurements read from the controller are interpreted
> wrong. The first measurement X+ contains the Y position, and the second
> measurement Y+ the X position (see also Table 11 Register Table in the
> data sheet).
>
> The problem is already known and a swap option has been introduced:
> commit 6680884a4420 ("Input: ad7879 - add option to correct xy axis")
>
> However, the meaning of the new boolean is inverted since the underlying
> values are already swapped. Let ts->swap_xy set to true actually be the
> swapped configuration of the two axis.
>
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Acked-by: Michael Hennerich <michael.hennerich@analog.com>

> ---
> Changes since v2:
> - (none)
> Changes since v1:
> - Keep axis swapped by default when using platform data
>
>   drivers/input/touchscreen/ad7879.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/touchscreen/ad7879.c b/drivers/input/touchscreen/ad7879.c
> index 93b8ea2..abd0220 100644
> --- a/drivers/input/touchscreen/ad7879.c
> +++ b/drivers/input/touchscreen/ad7879.c
> @@ -94,8 +94,8 @@
>   #define AD7879_TEMP_BIT			(1<<1)
>
>   enum {
> -	AD7879_SEQ_XPOS  = 0,
> -	AD7879_SEQ_YPOS  = 1,
> +	AD7879_SEQ_YPOS  = 0,
> +	AD7879_SEQ_XPOS  = 1,
>   	AD7879_SEQ_Z1    = 2,
>   	AD7879_SEQ_Z2    = 3,
>   	AD7879_NR_SENSE  = 4,
> @@ -517,7 +517,9 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
>   	ts->dev = dev;
>   	ts->input = input_dev;
>   	ts->irq = irq;
> -	ts->swap_xy = pdata->swap_xy;
> +
> +	/* Use swapped axis by default (backward compatibility) */
> +	ts->swap_xy = !pdata->swap_xy;
>
>   	setup_timer(&ts->timer, ad7879_timer, (unsigned long) ts);
>
>


-- 
Greetings,
Michael

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

* Re: [PATCH v3 1/3] input: touchscreen: ad7879: move header to platform_data directory
  2016-02-02 23:20 [PATCH v3 1/3] input: touchscreen: ad7879: move header to platform_data directory Stefan Agner
  2016-02-02 23:20 ` [PATCH v3 2/3] input: touchscreen: ad7879: fix default x/y axis assignment Stefan Agner
  2016-02-02 23:20 ` [PATCH v3 3/3] input: touchscreen: ad7879: add device tree support Stefan Agner
@ 2016-03-08 17:48 ` Stefan Agner
  2016-03-08 18:48   ` Dmitry Torokhov
  2 siblings, 1 reply; 10+ messages in thread
From: Stefan Agner @ 2016-03-08 17:48 UTC (permalink / raw)
  To: dmitry.torokhov, michael.hennerich, robh+dt
  Cc: mark.rutland, ijc+devicetree, galak, realmz6, broonie, jic23,
	linux-input, devicetree, linux-kernel

Hi Dimitry,

Any chance this patchset makes it into 4.6?

Best regards,
Stefan

On 2016-02-02 15:20, Stefan Agner wrote:
> The header file is used by the SPI and I2C variant of the driver.
> Therefore, move it to a more generic place under platform_data.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
> Changes since v2:
> - (none)
> Changes since v1:
> - Move to include/linux/platform_data/
> 
>  arch/blackfin/mach-bf527/boards/ezbrd.c    |  2 +-
>  arch/blackfin/mach-bf527/boards/ezkit.c    |  2 +-
>  arch/blackfin/mach-bf527/boards/tll6527m.c |  2 +-
>  arch/blackfin/mach-bf537/boards/stamp.c    |  2 +-
>  arch/blackfin/mach-bf538/boards/ezkit.c    |  2 +-
>  drivers/input/touchscreen/ad7879.c         | 10 ++++----
>  include/linux/platform_data/ad7879.h       | 41 ++++++++++++++++++++++++++++++
>  include/linux/spi/ad7879.h                 | 41 ------------------------------
>  8 files changed, 51 insertions(+), 51 deletions(-)
>  create mode 100644 include/linux/platform_data/ad7879.h
>  delete mode 100644 include/linux/spi/ad7879.h
> 
> diff --git a/arch/blackfin/mach-bf527/boards/ezbrd.c
> b/arch/blackfin/mach-bf527/boards/ezbrd.c
> index a3a5723..80bcfd1 100644
> --- a/arch/blackfin/mach-bf527/boards/ezbrd.c
> +++ b/arch/blackfin/mach-bf527/boards/ezbrd.c
> @@ -279,7 +279,7 @@ static const struct ad7877_platform_data
> bfin_ad7877_ts_info = {
>  #endif
>  
>  #if IS_ENABLED(CONFIG_TOUCHSCREEN_AD7879)
> -#include <linux/spi/ad7879.h>
> +#include <linux/platform_data/ad7879.h>
>  static const struct ad7879_platform_data bfin_ad7879_ts_info = {
>  	.model			= 7879,	/* Model = AD7879 */
>  	.x_plate_ohms		= 620,	/* 620 Ohm from the touch datasheet */
> diff --git a/arch/blackfin/mach-bf527/boards/ezkit.c
> b/arch/blackfin/mach-bf527/boards/ezkit.c
> index d4219e8..571edfd 100644
> --- a/arch/blackfin/mach-bf527/boards/ezkit.c
> +++ b/arch/blackfin/mach-bf527/boards/ezkit.c
> @@ -477,7 +477,7 @@ static const struct ad7877_platform_data
> bfin_ad7877_ts_info = {
>  #endif
>  
>  #if IS_ENABLED(CONFIG_TOUCHSCREEN_AD7879)
> -#include <linux/spi/ad7879.h>
> +#include <linux/platform_data/ad7879.h>
>  static const struct ad7879_platform_data bfin_ad7879_ts_info = {
>  	.model			= 7879,	/* Model = AD7879 */
>  	.x_plate_ohms		= 620,	/* 620 Ohm from the touch datasheet */
> diff --git a/arch/blackfin/mach-bf527/boards/tll6527m.c
> b/arch/blackfin/mach-bf527/boards/tll6527m.c
> index a0f5856..c1acce4 100644
> --- a/arch/blackfin/mach-bf527/boards/tll6527m.c
> +++ b/arch/blackfin/mach-bf527/boards/tll6527m.c
> @@ -29,7 +29,7 @@
>  #include <asm/dpmc.h>
>  
>  #if IS_ENABLED(CONFIG_TOUCHSCREEN_AD7879)
> -#include <linux/spi/ad7879.h>
> +#include <linux/platform_data/ad7879.h>
>  #define LCD_BACKLIGHT_GPIO 0x40
>  /* TLL6527M uses TLL7UIQ35 / ADI LCD EZ Extender. AD7879 AUX GPIO is used for
>   * LCD Backlight Enable
> diff --git a/arch/blackfin/mach-bf537/boards/stamp.c
> b/arch/blackfin/mach-bf537/boards/stamp.c
> index c181543..eaec7b4 100644
> --- a/arch/blackfin/mach-bf537/boards/stamp.c
> +++ b/arch/blackfin/mach-bf537/boards/stamp.c
> @@ -776,7 +776,7 @@ static const struct ad7877_platform_data
> bfin_ad7877_ts_info = {
>  #endif
>  
>  #if IS_ENABLED(CONFIG_TOUCHSCREEN_AD7879)
> -#include <linux/spi/ad7879.h>
> +#include <linux/platform_data/ad7879.h>
>  static const struct ad7879_platform_data bfin_ad7879_ts_info = {
>  	.model			= 7879,	/* Model = AD7879 */
>  	.x_plate_ohms		= 620,	/* 620 Ohm from the touch datasheet */
> diff --git a/arch/blackfin/mach-bf538/boards/ezkit.c
> b/arch/blackfin/mach-bf538/boards/ezkit.c
> index ae2fcbb..4a03c44 100644
> --- a/arch/blackfin/mach-bf538/boards/ezkit.c
> +++ b/arch/blackfin/mach-bf538/boards/ezkit.c
> @@ -521,7 +521,7 @@ static struct bfin5xx_spi_chip spi_flash_chip_info = {
>  #endif	/* CONFIG_SPI_BFIN5XX */
>  
>  #if IS_ENABLED(CONFIG_TOUCHSCREEN_AD7879)
> -#include <linux/spi/ad7879.h>
> +#include <linux/platform_data/ad7879.h>
>  static const struct ad7879_platform_data bfin_ad7879_ts_info = {
>  	.model			= 7879,	/* Model = AD7879 */
>  	.x_plate_ohms		= 620,	/* 620 Ohm from the touch datasheet */
> diff --git a/drivers/input/touchscreen/ad7879.c
> b/drivers/input/touchscreen/ad7879.c
> index 16b5cc2..93b8ea2 100644
> --- a/drivers/input/touchscreen/ad7879.c
> +++ b/drivers/input/touchscreen/ad7879.c
> @@ -31,7 +31,7 @@
>  #include <linux/i2c.h>
>  #include <linux/gpio.h>
>  
> -#include <linux/spi/ad7879.h>
> +#include <linux/platform_data/ad7879.h>
>  #include <linux/module.h>
>  #include "ad7879.h"
>  
> @@ -170,10 +170,10 @@ static int ad7879_report(struct ad7879 *ts)
>  	 * filter.  The combination of these two techniques provides a robust
>  	 * solution, discarding the spurious noise in the signal and keeping
>  	 * only the data of interest.  The size of both filters is
> -	 * programmable. (dev.platform_data, see linux/spi/ad7879.h) Other
> -	 * user-programmable conversion controls include variable acquisition
> -	 * time, and first conversion delay. Up to 16 averages can be taken
> -	 * per conversion.
> +	 * programmable. (dev.platform_data, see linux/platform_data/ad7879.h)
> +	 * Other user-programmable conversion controls include variable
> +	 * acquisition time, and first conversion delay. Up to 16 averages can
> +	 * be taken per conversion.
>  	 */
>  
>  	if (likely(x && z1)) {
> diff --git a/include/linux/platform_data/ad7879.h
> b/include/linux/platform_data/ad7879.h
> new file mode 100644
> index 0000000..69e2e1fd
> --- /dev/null
> +++ b/include/linux/platform_data/ad7879.h
> @@ -0,0 +1,41 @@
> +/* linux/platform_data/ad7879.h */
> +
> +/* Touchscreen characteristics vary between boards and models.  The
> + * platform_data for the device's "struct device" holds this information.
> + *
> + * It's OK if the min/max values are zero.
> + */
> +struct ad7879_platform_data {
> +	u16	model;			/* 7879 */
> +	u16	x_plate_ohms;
> +	u16	x_min, x_max;
> +	u16	y_min, y_max;
> +	u16	pressure_min, pressure_max;
> +
> +	bool	swap_xy;		/* swap x and y axes */
> +
> +	/* [0..255] 0=OFF Starts at 1=550us and goes
> +	 * all the way to 9.440ms in steps of 35us.
> +	 */
> +	u8	pen_down_acc_interval;
> +	/* [0..15] Starts at 0=128us and goes all the
> +	 * way to 4.096ms in steps of 128us.
> +	 */
> +	u8	first_conversion_delay;
> +	/* [0..3] 0 = 2us, 1 = 4us, 2 = 8us, 3 = 16us */
> +	u8	acquisition_time;
> +	/* [0..3] Average X middle samples 0 = 2, 1 = 4, 2 = 8, 3 = 16 */
> +	u8	averaging;
> +	/* [0..3] Perform X measurements 0 = OFF,
> +	 * 1 = 4, 2 = 8, 3 = 16 (median > averaging)
> +	 */
> +	u8	median;
> +	/* 1 = AUX/VBAT/GPIO export GPIO to gpiolib
> +	 * requires CONFIG_GPIOLIB
> +	 */
> +	bool	gpio_export;
> +	/* identifies the first GPIO number handled by this chip;
> +	 * or, if negative, requests dynamic ID allocation.
> +	 */
> +	s32	gpio_base;
> +};
> diff --git a/include/linux/spi/ad7879.h b/include/linux/spi/ad7879.h
> deleted file mode 100644
> index 58368be..0000000
> --- a/include/linux/spi/ad7879.h
> +++ /dev/null
> @@ -1,41 +0,0 @@
> -/* linux/spi/ad7879.h */
> -
> -/* Touchscreen characteristics vary between boards and models.  The
> - * platform_data for the device's "struct device" holds this information.
> - *
> - * It's OK if the min/max values are zero.
> - */
> -struct ad7879_platform_data {
> -	u16	model;			/* 7879 */
> -	u16	x_plate_ohms;
> -	u16	x_min, x_max;
> -	u16	y_min, y_max;
> -	u16	pressure_min, pressure_max;
> -
> -	bool	swap_xy;		/* swap x and y axes */
> -
> -	/* [0..255] 0=OFF Starts at 1=550us and goes
> -	 * all the way to 9.440ms in steps of 35us.
> -	 */
> -	u8	pen_down_acc_interval;
> -	/* [0..15] Starts at 0=128us and goes all the
> -	 * way to 4.096ms in steps of 128us.
> -	 */
> -	u8	first_conversion_delay;
> -	/* [0..3] 0 = 2us, 1 = 4us, 2 = 8us, 3 = 16us */
> -	u8	acquisition_time;
> -	/* [0..3] Average X middle samples 0 = 2, 1 = 4, 2 = 8, 3 = 16 */
> -	u8	averaging;
> -	/* [0..3] Perform X measurements 0 = OFF,
> -	 * 1 = 4, 2 = 8, 3 = 16 (median > averaging)
> -	 */
> -	u8	median;
> -	/* 1 = AUX/VBAT/GPIO export GPIO to gpiolib
> -	 * requires CONFIG_GPIOLIB
> -	 */
> -	bool	gpio_export;
> -	/* identifies the first GPIO number handled by this chip;
> -	 * or, if negative, requests dynamic ID allocation.
> -	 */
> -	s32	gpio_base;
> -};

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

* Re: [PATCH v3 1/3] input: touchscreen: ad7879: move header to platform_data directory
  2016-03-08 17:48 ` [PATCH v3 1/3] input: touchscreen: ad7879: move header to platform_data directory Stefan Agner
@ 2016-03-08 18:48   ` Dmitry Torokhov
  0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2016-03-08 18:48 UTC (permalink / raw)
  To: Stefan Agner
  Cc: michael.hennerich, robh+dt, mark.rutland, ijc+devicetree, galak,
	realmz6, broonie, jic23, linux-input, devicetree, linux-kernel

On Tue, Mar 08, 2016 at 09:48:04AM -0800, Stefan Agner wrote:
> Hi Dimitry,
> 
> Any chance this patchset makes it into 4.6?
> 

Applied all 3, thank you.

> Best regards,
> Stefan
> 
> On 2016-02-02 15:20, Stefan Agner wrote:
> > The header file is used by the SPI and I2C variant of the driver.
> > Therefore, move it to a more generic place under platform_data.
> > 
> > Signed-off-by: Stefan Agner <stefan@agner.ch>
> > ---
> > Changes since v2:
> > - (none)
> > Changes since v1:
> > - Move to include/linux/platform_data/
> > 
> >  arch/blackfin/mach-bf527/boards/ezbrd.c    |  2 +-
> >  arch/blackfin/mach-bf527/boards/ezkit.c    |  2 +-
> >  arch/blackfin/mach-bf527/boards/tll6527m.c |  2 +-
> >  arch/blackfin/mach-bf537/boards/stamp.c    |  2 +-
> >  arch/blackfin/mach-bf538/boards/ezkit.c    |  2 +-
> >  drivers/input/touchscreen/ad7879.c         | 10 ++++----
> >  include/linux/platform_data/ad7879.h       | 41 ++++++++++++++++++++++++++++++
> >  include/linux/spi/ad7879.h                 | 41 ------------------------------
> >  8 files changed, 51 insertions(+), 51 deletions(-)
> >  create mode 100644 include/linux/platform_data/ad7879.h
> >  delete mode 100644 include/linux/spi/ad7879.h
> > 
> > diff --git a/arch/blackfin/mach-bf527/boards/ezbrd.c
> > b/arch/blackfin/mach-bf527/boards/ezbrd.c
> > index a3a5723..80bcfd1 100644
> > --- a/arch/blackfin/mach-bf527/boards/ezbrd.c
> > +++ b/arch/blackfin/mach-bf527/boards/ezbrd.c
> > @@ -279,7 +279,7 @@ static const struct ad7877_platform_data
> > bfin_ad7877_ts_info = {
> >  #endif
> >  
> >  #if IS_ENABLED(CONFIG_TOUCHSCREEN_AD7879)
> > -#include <linux/spi/ad7879.h>
> > +#include <linux/platform_data/ad7879.h>
> >  static const struct ad7879_platform_data bfin_ad7879_ts_info = {
> >  	.model			= 7879,	/* Model = AD7879 */
> >  	.x_plate_ohms		= 620,	/* 620 Ohm from the touch datasheet */
> > diff --git a/arch/blackfin/mach-bf527/boards/ezkit.c
> > b/arch/blackfin/mach-bf527/boards/ezkit.c
> > index d4219e8..571edfd 100644
> > --- a/arch/blackfin/mach-bf527/boards/ezkit.c
> > +++ b/arch/blackfin/mach-bf527/boards/ezkit.c
> > @@ -477,7 +477,7 @@ static const struct ad7877_platform_data
> > bfin_ad7877_ts_info = {
> >  #endif
> >  
> >  #if IS_ENABLED(CONFIG_TOUCHSCREEN_AD7879)
> > -#include <linux/spi/ad7879.h>
> > +#include <linux/platform_data/ad7879.h>
> >  static const struct ad7879_platform_data bfin_ad7879_ts_info = {
> >  	.model			= 7879,	/* Model = AD7879 */
> >  	.x_plate_ohms		= 620,	/* 620 Ohm from the touch datasheet */
> > diff --git a/arch/blackfin/mach-bf527/boards/tll6527m.c
> > b/arch/blackfin/mach-bf527/boards/tll6527m.c
> > index a0f5856..c1acce4 100644
> > --- a/arch/blackfin/mach-bf527/boards/tll6527m.c
> > +++ b/arch/blackfin/mach-bf527/boards/tll6527m.c
> > @@ -29,7 +29,7 @@
> >  #include <asm/dpmc.h>
> >  
> >  #if IS_ENABLED(CONFIG_TOUCHSCREEN_AD7879)
> > -#include <linux/spi/ad7879.h>
> > +#include <linux/platform_data/ad7879.h>
> >  #define LCD_BACKLIGHT_GPIO 0x40
> >  /* TLL6527M uses TLL7UIQ35 / ADI LCD EZ Extender. AD7879 AUX GPIO is used for
> >   * LCD Backlight Enable
> > diff --git a/arch/blackfin/mach-bf537/boards/stamp.c
> > b/arch/blackfin/mach-bf537/boards/stamp.c
> > index c181543..eaec7b4 100644
> > --- a/arch/blackfin/mach-bf537/boards/stamp.c
> > +++ b/arch/blackfin/mach-bf537/boards/stamp.c
> > @@ -776,7 +776,7 @@ static const struct ad7877_platform_data
> > bfin_ad7877_ts_info = {
> >  #endif
> >  
> >  #if IS_ENABLED(CONFIG_TOUCHSCREEN_AD7879)
> > -#include <linux/spi/ad7879.h>
> > +#include <linux/platform_data/ad7879.h>
> >  static const struct ad7879_platform_data bfin_ad7879_ts_info = {
> >  	.model			= 7879,	/* Model = AD7879 */
> >  	.x_plate_ohms		= 620,	/* 620 Ohm from the touch datasheet */
> > diff --git a/arch/blackfin/mach-bf538/boards/ezkit.c
> > b/arch/blackfin/mach-bf538/boards/ezkit.c
> > index ae2fcbb..4a03c44 100644
> > --- a/arch/blackfin/mach-bf538/boards/ezkit.c
> > +++ b/arch/blackfin/mach-bf538/boards/ezkit.c
> > @@ -521,7 +521,7 @@ static struct bfin5xx_spi_chip spi_flash_chip_info = {
> >  #endif	/* CONFIG_SPI_BFIN5XX */
> >  
> >  #if IS_ENABLED(CONFIG_TOUCHSCREEN_AD7879)
> > -#include <linux/spi/ad7879.h>
> > +#include <linux/platform_data/ad7879.h>
> >  static const struct ad7879_platform_data bfin_ad7879_ts_info = {
> >  	.model			= 7879,	/* Model = AD7879 */
> >  	.x_plate_ohms		= 620,	/* 620 Ohm from the touch datasheet */
> > diff --git a/drivers/input/touchscreen/ad7879.c
> > b/drivers/input/touchscreen/ad7879.c
> > index 16b5cc2..93b8ea2 100644
> > --- a/drivers/input/touchscreen/ad7879.c
> > +++ b/drivers/input/touchscreen/ad7879.c
> > @@ -31,7 +31,7 @@
> >  #include <linux/i2c.h>
> >  #include <linux/gpio.h>
> >  
> > -#include <linux/spi/ad7879.h>
> > +#include <linux/platform_data/ad7879.h>
> >  #include <linux/module.h>
> >  #include "ad7879.h"
> >  
> > @@ -170,10 +170,10 @@ static int ad7879_report(struct ad7879 *ts)
> >  	 * filter.  The combination of these two techniques provides a robust
> >  	 * solution, discarding the spurious noise in the signal and keeping
> >  	 * only the data of interest.  The size of both filters is
> > -	 * programmable. (dev.platform_data, see linux/spi/ad7879.h) Other
> > -	 * user-programmable conversion controls include variable acquisition
> > -	 * time, and first conversion delay. Up to 16 averages can be taken
> > -	 * per conversion.
> > +	 * programmable. (dev.platform_data, see linux/platform_data/ad7879.h)
> > +	 * Other user-programmable conversion controls include variable
> > +	 * acquisition time, and first conversion delay. Up to 16 averages can
> > +	 * be taken per conversion.
> >  	 */
> >  
> >  	if (likely(x && z1)) {
> > diff --git a/include/linux/platform_data/ad7879.h
> > b/include/linux/platform_data/ad7879.h
> > new file mode 100644
> > index 0000000..69e2e1fd
> > --- /dev/null
> > +++ b/include/linux/platform_data/ad7879.h
> > @@ -0,0 +1,41 @@
> > +/* linux/platform_data/ad7879.h */
> > +
> > +/* Touchscreen characteristics vary between boards and models.  The
> > + * platform_data for the device's "struct device" holds this information.
> > + *
> > + * It's OK if the min/max values are zero.
> > + */
> > +struct ad7879_platform_data {
> > +	u16	model;			/* 7879 */
> > +	u16	x_plate_ohms;
> > +	u16	x_min, x_max;
> > +	u16	y_min, y_max;
> > +	u16	pressure_min, pressure_max;
> > +
> > +	bool	swap_xy;		/* swap x and y axes */
> > +
> > +	/* [0..255] 0=OFF Starts at 1=550us and goes
> > +	 * all the way to 9.440ms in steps of 35us.
> > +	 */
> > +	u8	pen_down_acc_interval;
> > +	/* [0..15] Starts at 0=128us and goes all the
> > +	 * way to 4.096ms in steps of 128us.
> > +	 */
> > +	u8	first_conversion_delay;
> > +	/* [0..3] 0 = 2us, 1 = 4us, 2 = 8us, 3 = 16us */
> > +	u8	acquisition_time;
> > +	/* [0..3] Average X middle samples 0 = 2, 1 = 4, 2 = 8, 3 = 16 */
> > +	u8	averaging;
> > +	/* [0..3] Perform X measurements 0 = OFF,
> > +	 * 1 = 4, 2 = 8, 3 = 16 (median > averaging)
> > +	 */
> > +	u8	median;
> > +	/* 1 = AUX/VBAT/GPIO export GPIO to gpiolib
> > +	 * requires CONFIG_GPIOLIB
> > +	 */
> > +	bool	gpio_export;
> > +	/* identifies the first GPIO number handled by this chip;
> > +	 * or, if negative, requests dynamic ID allocation.
> > +	 */
> > +	s32	gpio_base;
> > +};
> > diff --git a/include/linux/spi/ad7879.h b/include/linux/spi/ad7879.h
> > deleted file mode 100644
> > index 58368be..0000000
> > --- a/include/linux/spi/ad7879.h
> > +++ /dev/null
> > @@ -1,41 +0,0 @@
> > -/* linux/spi/ad7879.h */
> > -
> > -/* Touchscreen characteristics vary between boards and models.  The
> > - * platform_data for the device's "struct device" holds this information.
> > - *
> > - * It's OK if the min/max values are zero.
> > - */
> > -struct ad7879_platform_data {
> > -	u16	model;			/* 7879 */
> > -	u16	x_plate_ohms;
> > -	u16	x_min, x_max;
> > -	u16	y_min, y_max;
> > -	u16	pressure_min, pressure_max;
> > -
> > -	bool	swap_xy;		/* swap x and y axes */
> > -
> > -	/* [0..255] 0=OFF Starts at 1=550us and goes
> > -	 * all the way to 9.440ms in steps of 35us.
> > -	 */
> > -	u8	pen_down_acc_interval;
> > -	/* [0..15] Starts at 0=128us and goes all the
> > -	 * way to 4.096ms in steps of 128us.
> > -	 */
> > -	u8	first_conversion_delay;
> > -	/* [0..3] 0 = 2us, 1 = 4us, 2 = 8us, 3 = 16us */
> > -	u8	acquisition_time;
> > -	/* [0..3] Average X middle samples 0 = 2, 1 = 4, 2 = 8, 3 = 16 */
> > -	u8	averaging;
> > -	/* [0..3] Perform X measurements 0 = OFF,
> > -	 * 1 = 4, 2 = 8, 3 = 16 (median > averaging)
> > -	 */
> > -	u8	median;
> > -	/* 1 = AUX/VBAT/GPIO export GPIO to gpiolib
> > -	 * requires CONFIG_GPIOLIB
> > -	 */
> > -	bool	gpio_export;
> > -	/* identifies the first GPIO number handled by this chip;
> > -	 * or, if negative, requests dynamic ID allocation.
> > -	 */
> > -	s32	gpio_base;
> > -};

-- 
Dmitry

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

* Re: [PATCH v3 3/3] input: touchscreen: ad7879: add device tree support
  2016-02-02 23:20 ` [PATCH v3 3/3] input: touchscreen: ad7879: add device tree support Stefan Agner
  2016-02-03  7:41     ` Michael Hennerich
@ 2016-03-08 18:50   ` Dmitry Torokhov
  1 sibling, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2016-03-08 18:50 UTC (permalink / raw)
  To: Stefan Agner
  Cc: michael.hennerich, robh+dt, mark.rutland, ijc+devicetree, galak,
	realmz6, broonie, jic23, linux-input, devicetree, linux-kernel

On Tue, Feb 02, 2016 at 03:20:18PM -0800, Stefan Agner wrote:
> Add device tree support for the I2C and SPI variant of AD7879(-1).
> This allows to specify the touchscreen controller as a I2C client
> node or SPI slave device. Most of the options available in platform
> data are also available as device tree properties, the only exception
> being GPIO capabilities, which can not be activated through device
> tree currently.
> 
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
> Changes since v2:
> - do not free driver data and irq on remove (since we are using devm now)
> Changes since v1:
> - Move device tree parsing to main driver file ad7879.c
> - Use common touchscreen_parse_properties for common properties
> - Use device_property_* API
> - Use struct ad7879 directly to store parsed values
> - Support SPI variant through device tree too (untested)
> - Add vendor prefix to vendor specific dt properties
> 
>  .../bindings/input/touchscreen/ad7879.txt          |  53 ++++++++
>  drivers/input/touchscreen/ad7879-i2c.c             |  10 ++
>  drivers/input/touchscreen/ad7879-spi.c             |  10 ++
>  drivers/input/touchscreen/ad7879.c                 | 145 +++++++++++++--------
>  4 files changed, 161 insertions(+), 57 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ad7879.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/ad7879.txt b/Documentation/devicetree/bindings/input/touchscreen/ad7879.txt
> new file mode 100644
> index 0000000..e3f22d2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/ad7879.txt
> @@ -0,0 +1,53 @@
> +* Analog Devices AD7879(-1)/AD7889(-1) touchscreen interface (SPI/I2C)
> +
> +Required properties:
> +- compatible			: for SPI slave, use "adi,ad7879"
> +				  for I2C slave, use "adi,ad7879-1"
> +- reg				: SPI chipselect/I2C slave address
> +				  See spi-bus.txt for more SPI slave properties
> +- interrupt-parent		: the phandle for the interrupt controller
> +- interrupts			: touch controller interrupt
> +- touchscreen-max-pressure	: maximum reported pressure
> +- adi,resistance-plate-x	: total resistance of X-plate (for pressure
> +				  calculation)
> +Optional properties:
> +- touchscreen-swapped-x-y	: X and Y axis are swapped (boolean)
> +- adi,first-conversion-delay	: 0-12: In 128us steps (starting with 128us)
> +				  13  : 2.560ms
> +				  14  : 3.584ms
> +				  15  : 4.096ms
> +				  This property has to be a '/bits/ 8' value
> +- adi,acquisition-time		: 0: 2us
> +				  1: 4us
> +				  2: 8us
> +				  3: 16us
> +				  This property has to be a '/bits/ 8' value
> +- adi,median-filter-size	: 0: disabled
> +				  1: 4 measurements
> +				  2: 8 measurements
> +				  3: 16 measurements
> +				  This property has to be a '/bits/ 8' value
> +- adi,averaging			: 0: 2 middle values (1 if median disabled)
> +				  1: 4 middle values
> +				  2: 8 middle values
> +				  3: 16 values
> +				  This property has to be a '/bits/ 8' value
> +- adi,conversion-interval:	: 0    : convert one time only
> +				  1-255: 515us + val * 35us (up to 9.440ms)
> +				  This property has to be a '/bits/ 8' value
> +
> +Example:
> +
> +	ad7879@2c {
> +		compatible = "adi,ad7879-1";
> +		reg = <0x2c>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <13 IRQ_TYPE_EDGE_FALLING>;
> +		touchscreen-max-pressure = <4096>;
> +		adi,resistance-plate-x = <120>;
> +		adi,first-conversion-delay = /bits/ 8 <3>;
> +		adi,acquisition-time = /bits/ 8 <1>;
> +		adi,median-filter-size = /bits/ 8 <2>;
> +		adi,averaging = /bits/ 8 <1>;
> +		adi,conversion-interval = /bits/ 8 <255>;
> +	};
> diff --git a/drivers/input/touchscreen/ad7879-i2c.c b/drivers/input/touchscreen/ad7879-i2c.c
> index d66962c..58f72e0 100644
> --- a/drivers/input/touchscreen/ad7879-i2c.c
> +++ b/drivers/input/touchscreen/ad7879-i2c.c
> @@ -10,6 +10,7 @@
>  #include <linux/i2c.h>
>  #include <linux/module.h>
>  #include <linux/types.h>
> +#include <linux/of.h>
>  #include <linux/pm.h>
>  
>  #include "ad7879.h"
> @@ -91,10 +92,19 @@ static const struct i2c_device_id ad7879_id[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, ad7879_id);
>  
> +#ifdef CONFIG_OF
> +static const struct of_device_id ad7879_i2c_dt_ids[] = {
> +	{ .compatible = "adi,ad7879-1", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ad7879_i2c_dt_ids);
> +#endif
> +
>  static struct i2c_driver ad7879_i2c_driver = {
>  	.driver = {
>  		.name	= "ad7879",
>  		.pm	= &ad7879_pm_ops,
> +		.of_match_table = of_match_ptr(ad7879_i2c_dt_ids),
>  	},
>  	.probe		= ad7879_i2c_probe,
>  	.remove		= ad7879_i2c_remove,
> diff --git a/drivers/input/touchscreen/ad7879-spi.c b/drivers/input/touchscreen/ad7879-spi.c
> index 48033c2..d42b6b9 100644
> --- a/drivers/input/touchscreen/ad7879-spi.c
> +++ b/drivers/input/touchscreen/ad7879-spi.c
> @@ -10,6 +10,7 @@
>  #include <linux/pm.h>
>  #include <linux/spi/spi.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
>  
>  #include "ad7879.h"
>  
> @@ -146,10 +147,19 @@ static int ad7879_spi_remove(struct spi_device *spi)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_OF
> +static const struct of_device_id ad7879_spi_dt_ids[] = {
> +	{ .compatible = "adi,ad7879", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ad7879_spi_dt_ids);
> +#endif
> +
>  static struct spi_driver ad7879_spi_driver = {
>  	.driver = {
>  		.name	= "ad7879",
>  		.pm	= &ad7879_pm_ops,
> +		.of_match_table = of_match_ptr(ad7879_spi_dt_ids),
>  	},
>  	.probe		= ad7879_spi_probe,
>  	.remove		= ad7879_spi_remove,
> diff --git a/drivers/input/touchscreen/ad7879.c b/drivers/input/touchscreen/ad7879.c
> index abd0220..093cb04 100644
> --- a/drivers/input/touchscreen/ad7879.c
> +++ b/drivers/input/touchscreen/ad7879.c
> @@ -31,6 +31,7 @@
>  #include <linux/i2c.h>
>  #include <linux/gpio.h>
>  
> +#include <linux/input/touchscreen.h>
>  #include <linux/platform_data/ad7879.h>
>  #include <linux/module.h>
>  #include "ad7879.h"
> @@ -126,7 +127,6 @@ struct ad7879 {
>  	u8			pen_down_acc_interval;
>  	u8			median;
>  	u16			x_plate_ohms;
> -	u16			pressure_max;
>  	u16			cmd_crtl1;
>  	u16			cmd_crtl2;
>  	u16			cmd_crtl3;
> @@ -186,7 +186,7 @@ static int ad7879_report(struct ad7879 *ts)
>  		 * Sample found inconsistent, pressure is beyond
>  		 * the maximum. Don't report it to user space.
>  		 */
> -		if (Rt > ts->pressure_max)
> +		if (Rt > input_abs_get_max(input_dev, ABS_PRESSURE))
>  			return -EINVAL;
>  
>  		/*
> @@ -469,7 +469,7 @@ static void ad7879_gpio_remove(struct ad7879 *ts)
>  {
>  	const struct ad7879_platform_data *pdata = dev_get_platdata(ts->dev);
>  
> -	if (pdata->gpio_export)
> +	if (pdata && pdata->gpio_export)
>  		gpiochip_remove(&ts->gc);
>  
>  }
> @@ -485,6 +485,29 @@ static inline void ad7879_gpio_remove(struct ad7879 *ts)
>  }
>  #endif
>  
> +static int ad7879_parse_dt(struct device *dev, struct ad7879 *ts)
> +{
> +	int err;
> +	u32 tmp;
> +
> +	err = device_property_read_u32(dev, "adi,resistance-plate-x", &tmp);
> +	if (err) {
> +		dev_err(dev, "failed to get resistance-plate-x property\n");
> +		return err;
> +	}
> +	ts->x_plate_ohms = (u16)tmp;
> +
> +	device_property_read_u8(dev, "adi,first-conversion-delay", &ts->first_conversion_delay);
> +	device_property_read_u8(dev, "adi,acquisition-time", &ts->acquisition_time);
> +	device_property_read_u8(dev, "adi,median-filter-size", &ts->median);
> +	device_property_read_u8(dev, "adi,averaging", &ts->averaging);
> +	device_property_read_u8(dev, "adi,conversion-interval", &ts->pen_down_acc_interval);

I wrapped this block so it does not exceed 80 columns.

> +
> +	ts->swap_xy = device_property_read_bool(dev, "touchscreen-swapped-x-y");
> +
> +	return 0;
> +}
> +
>  struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
>  			    const struct ad7879_bus_ops *bops)
>  {
> @@ -495,22 +518,37 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
>  	u16 revid;
>  
>  	if (!irq) {
> -		dev_err(dev, "no IRQ?\n");
> -		err = -EINVAL;
> -		goto err_out;
> +		dev_err(dev, "No IRQ specified\n");
> +		return ERR_PTR(-EINVAL);
>  	}
>  
> -	if (!pdata) {
> -		dev_err(dev, "no platform data?\n");
> -		err = -EINVAL;
> -		goto err_out;
> +	ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
> +	if (!ts)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (pdata) {
> +		/* Platform data use swapped axis (backward compatibility) */
> +		ts->swap_xy = !pdata->swap_xy;
> +
> +		ts->x_plate_ohms = pdata->x_plate_ohms ? : 400;
> +
> +		ts->first_conversion_delay = pdata->first_conversion_delay;
> +		ts->acquisition_time = pdata->acquisition_time;
> +		ts->averaging = pdata->averaging;
> +		ts->pen_down_acc_interval = pdata->pen_down_acc_interval;
> +		ts->median = pdata->median;
> +
> +	} else if (dev->of_node) {
> +		ad7879_parse_dt(dev, ts);
> +	} else {
> +		dev_err(dev, "No platform data\n");
> +		return ERR_PTR(-EINVAL);
>  	}
>  
> -	ts = kzalloc(sizeof(*ts), GFP_KERNEL);
> -	input_dev = input_allocate_device();
> -	if (!ts || !input_dev) {
> -		err = -ENOMEM;
> -		goto err_free_mem;
> +	input_dev = devm_input_allocate_device(dev);
> +	if (!input_dev) {
> +		dev_err(dev, "Failed to allocate input device\n");
> +		return ERR_PTR(-ENOMEM);
>  	}
>  
>  	ts->bops = bops;
> @@ -518,20 +556,7 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
>  	ts->input = input_dev;
>  	ts->irq = irq;
>  
> -	/* Use swapped axis by default (backward compatibility) */
> -	ts->swap_xy = !pdata->swap_xy;
> -
>  	setup_timer(&ts->timer, ad7879_timer, (unsigned long) ts);
> -
> -	ts->x_plate_ohms = pdata->x_plate_ohms ? : 400;
> -	ts->pressure_max = pdata->pressure_max ? : ~0;
> -
> -	ts->first_conversion_delay = pdata->first_conversion_delay;
> -	ts->acquisition_time = pdata->acquisition_time;
> -	ts->averaging = pdata->averaging;
> -	ts->pen_down_acc_interval = pdata->pen_down_acc_interval;
> -	ts->median = pdata->median;
> -
>  	snprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(dev));
>  
>  	input_dev->name = "AD7879 Touchscreen";
> @@ -552,21 +577,33 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
>  	__set_bit(EV_KEY, input_dev->evbit);
>  	__set_bit(BTN_TOUCH, input_dev->keybit);
>  
> -	input_set_abs_params(input_dev, ABS_X,
> -			pdata->x_min ? : 0,
> -			pdata->x_max ? : MAX_12BIT,
> -			0, 0);
> -	input_set_abs_params(input_dev, ABS_Y,
> -			pdata->y_min ? : 0,
> -			pdata->y_max ? : MAX_12BIT,
> -			0, 0);
> -	input_set_abs_params(input_dev, ABS_PRESSURE,
> -			pdata->pressure_min, pdata->pressure_max, 0, 0);
> +	if (pdata) {
> +		input_set_abs_params(input_dev, ABS_X,
> +				pdata->x_min ? : 0,
> +				pdata->x_max ? : MAX_12BIT,
> +				0, 0);
> +		input_set_abs_params(input_dev, ABS_Y,
> +				pdata->y_min ? : 0,
> +				pdata->y_max ? : MAX_12BIT,
> +				0, 0);
> +		input_set_abs_params(input_dev, ABS_PRESSURE,
> +				pdata->pressure_min,
> +				pdata->pressure_max ? : ~0,
> +				0, 0);
> +	} else {
> +		input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, 0, 0);
> +		input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, 0, 0);
> +		touchscreen_parse_properties(input_dev, false);
> +		if (!input_abs_get_max(input_dev, ABS_PRESSURE)) {
> +			dev_err(dev, "Touchscreen pressure is not specified\n");
> +			return ERR_PTR(-EINVAL);
> +		}
> +	}
>  
>  	err = ad7879_write(ts, AD7879_REG_CTRL2, AD7879_RESET);
>  	if (err < 0) {
>  		dev_err(dev, "Failed to write %s\n", input_dev->name);
> -		goto err_free_mem;
> +		return ERR_PTR(err);
>  	}
>  
>  	revid = ad7879_read(ts, AD7879_REG_REVID);
> @@ -575,8 +612,7 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
>  	if (input_dev->id.product != devid) {
>  		dev_err(dev, "Failed to probe %s (%x vs %x)\n",
>  			input_dev->name, devid, revid);
> -		err = -ENODEV;
> -		goto err_free_mem;
> +		return ERR_PTR(-ENODEV);
>  	}
>  
>  	ts->cmd_crtl3 = AD7879_YPLUS_BIT |
> @@ -596,23 +632,25 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, unsigned int irq,
>  			AD7879_ACQ(ts->acquisition_time) |
>  			AD7879_TMR(ts->pen_down_acc_interval);
>  
> -	err = request_threaded_irq(ts->irq, NULL, ad7879_irq,
> -				   IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> -				   dev_name(dev), ts);
> +	err = devm_request_threaded_irq(dev, ts->irq, NULL, ad7879_irq,
> +					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +					dev_name(dev), ts);
>  	if (err) {
> -		dev_err(dev, "irq %d busy?\n", ts->irq);
> -		goto err_free_mem;
> +		dev_err(dev, "Failed to request IRQ: %d\n", err);
> +		return ERR_PTR(err);
>  	}
>  
>  	__ad7879_disable(ts);
>  
>  	err = sysfs_create_group(&dev->kobj, &ad7879_attr_group);
>  	if (err)
> -		goto err_free_irq;
> +		goto err_out;
>  
> -	err = ad7879_gpio_add(ts, pdata);
> -	if (err)
> -		goto err_remove_attr;
> +	if (pdata) {
> +		err = ad7879_gpio_add(ts, pdata);
> +		if (err)
> +			goto err_remove_attr;
> +	}
>  
>  	err = input_register_device(input_dev);
>  	if (err)
> @@ -624,11 +662,6 @@ err_remove_gpio:
>  	ad7879_gpio_remove(ts);
>  err_remove_attr:
>  	sysfs_remove_group(&dev->kobj, &ad7879_attr_group);
> -err_free_irq:
> -	free_irq(ts->irq, ts);
> -err_free_mem:
> -	input_free_device(input_dev);
> -	kfree(ts);
>  err_out:
>  	return ERR_PTR(err);
>  }
> @@ -638,9 +671,7 @@ void ad7879_remove(struct ad7879 *ts)
>  {
>  	ad7879_gpio_remove(ts);
>  	sysfs_remove_group(&ts->dev->kobj, &ad7879_attr_group);
> -	free_irq(ts->irq, ts);
>  	input_unregister_device(ts->input);

You do not need to call input_unregister_device() for devm-managed input
devices, I dropped it.

> -	kfree(ts);
>  }
>  EXPORT_SYMBOL(ad7879_remove);
>  
> -- 
> 2.7.0
> 

Thanks.

-- 
Dmitry

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

end of thread, other threads:[~2016-03-08 18:50 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-02 23:20 [PATCH v3 1/3] input: touchscreen: ad7879: move header to platform_data directory Stefan Agner
2016-02-02 23:20 ` [PATCH v3 2/3] input: touchscreen: ad7879: fix default x/y axis assignment Stefan Agner
2016-02-03  7:41   ` Michael Hennerich
2016-02-03  7:41     ` Michael Hennerich
2016-02-02 23:20 ` [PATCH v3 3/3] input: touchscreen: ad7879: add device tree support Stefan Agner
2016-02-03  7:41   ` Michael Hennerich
2016-02-03  7:41     ` Michael Hennerich
2016-03-08 18:50   ` Dmitry Torokhov
2016-03-08 17:48 ` [PATCH v3 1/3] input: touchscreen: ad7879: move header to platform_data directory Stefan Agner
2016-03-08 18:48   ` Dmitry Torokhov

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.