Subject: [PATCH] Input: add bu21029 touch driver

From: Zhu Yi <[email protected]>

Add the ROHM BU21029 resistive touch panel controller
support with i2c interface.

Signed-off-by: Zhu Yi <[email protected]>
Signed-off-by: Mark Jonas <[email protected]>
Reviewed-by: Heiko Schocher <[email protected]>
---
.../bindings/input/touchscreen/bu21029.txt | 30 ++
drivers/input/touchscreen/Kconfig | 12 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/bu21029_ts.c | 456 +++++++++++++++++++++
4 files changed, 499 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
create mode 100644 drivers/input/touchscreen/bu21029_ts.c

diff --git a/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
new file mode 100644
index 0000000..7b61602
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
@@ -0,0 +1,30 @@
+* Rohm BU21029 Touch Screen Controller
+
+Required properties:
+ - compatible : must be "rohm,bu21029"
+ - reg : i2c device address of the chip
+ - interrupt-parent : the phandle for the gpio controller
+ - interrupts : (gpio) interrupt to which the chip is connected
+ - reset-gpios : gpio pin to reset the chip
+ - rohm,x-plate-ohms : x-plate resistance in ohms
+
+Optional properties:
+ - touchscreen-max-pressure: maximum pressure value
+
+Example:
+
+ &i2c1 {
+ /* ... */
+
+ bu21029: bu21029@40 {
+ compatible = "rohm,bu21029";
+ reg = <0x40>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
+ reset-gpios = <&gpio6 16 GPIO_ACTIVE_LOW>;
+ rohm,x-plate-ohms = <600>;
+ touchscreen-max-pressure = <4095>;
+ };
+
+ /* ... */
+ };
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 4f15496..e09fe8f 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -151,6 +151,18 @@ config TOUCHSCREEN_BU21013
To compile this driver as a module, choose M here: the
module will be called bu21013_ts.

+config TOUCHSCREEN_BU21029
+ tristate "Rohm BU21029 based touch panel controllers"
+ depends on I2C
+ help
+ Say Y here if you have a Rohm BU21029 touchscreen controller
+ connected to your system.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called bu21029_ts.
+
config TOUCHSCREEN_CHIPONE_ICN8318
tristate "chipone icn8318 touchscreen controller"
depends on GPIOLIB || COMPILE_TEST
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index dddae79..f50624c 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_TOUCHSCREEN_AR1021_I2C) += ar1021_i2c.o
obj-$(CONFIG_TOUCHSCREEN_ATMEL_MXT) += atmel_mxt_ts.o
obj-$(CONFIG_TOUCHSCREEN_AUO_PIXCIR) += auo-pixcir-ts.o
obj-$(CONFIG_TOUCHSCREEN_BU21013) += bu21013_ts.o
+obj-$(CONFIG_TOUCHSCREEN_BU21029) += bu21029_ts.o
obj-$(CONFIG_TOUCHSCREEN_CHIPONE_ICN8318) += chipone_icn8318.o
obj-$(CONFIG_TOUCHSCREEN_CY8CTMG110) += cy8ctmg110_ts.o
obj-$(CONFIG_TOUCHSCREEN_CYTTSP_CORE) += cyttsp_core.o
diff --git a/drivers/input/touchscreen/bu21029_ts.c b/drivers/input/touchscreen/bu21029_ts.c
new file mode 100644
index 0000000..d5cbf11
--- /dev/null
+++ b/drivers/input/touchscreen/bu21029_ts.c
@@ -0,0 +1,456 @@
+/*
+ * Rohm BU21029 touchscreen controller driver
+ *
+ * Copyright (C) 2015 Bosch Sicherheitssysteme GmbH
+ *
+ * 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/delay.h>
+#include <linux/gpio.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/timer.h>
+
+/* HW_ID1 Register (PAGE=0, ADDR=0x0E, Reset value=0x02, Read only)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | HW_IDH |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * HW_ID2 Register (PAGE=0, ADDR=0x0F, Reset value=0x29, Read only)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | HW_IDL |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * HW_IDH: high 8bits of IC's ID
+ * HW_IDL: low 8bits of IC's ID
+ */
+#define BU21029_HWID_REG (0x0E << 3)
+#define SUPPORTED_HWID 0x0229
+
+/* CFR0 Register (PAGE=0, ADDR=0x00, Reset value=0x20)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | 0 | 0 | CALIB | INTRM | 0 | 0 | 0 | 0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * CALIB: 0 = not to use calibration result (*)
+ * 1 = use calibration result
+ * INTRM: 0 = INT output depend on "pen down" (*)
+ * 1 = INT output always "0"
+ */
+#define BU21029_CFR0_REG (0x00 << 3)
+#define CFR0_VALUE 0x00
+
+/* CFR1 Register (PAGE=0, ADDR=0x01, Reset value=0xA6)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | MAV | AVE[2:0] | 0 | SMPL[2:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * MAV: 0 = median average filter off
+ * 1 = median average filter on (*)
+ * AVE: AVE+1 = number of average samples for MAV,
+ * if AVE>SMPL, then AVE=SMPL (=3)
+ * SMPL: SMPL+1 = number of conversion samples for MAV (=7)
+ */
+#define BU21029_CFR1_REG (0x01 << 3)
+#define CFR1_VALUE 0xA6
+
+/* CFR2 Register (PAGE=0, ADDR=0x02, Reset value=0x04)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | INTVL_TIME[3:0] | TIME_ST_ADC[3:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * INTVL_TIME: waiting time between completion of conversion
+ * and start of next conversion, only usable in
+ * autoscan mode (=20.480ms)
+ * TIME_ST_ADC: waiting time between application of voltage
+ * to panel and start of A/D conversion (=100us)
+ */
+#define BU21029_CFR2_REG (0x02 << 3)
+#define CFR2_VALUE 0xC9
+
+/* CFR3 Register (PAGE=0, ADDR=0x0B, Reset value=0x72)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | RM8 | STRETCH| PU90K | DUAL | PIDAC_OFS[3:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * RM8: 0 = coordinate resolution is 12bit (*)
+ * 1 = coordinate resolution is 8bit
+ * STRETCH: 0 = SCL_STRETCH function off
+ * 1 = SCL_STRETCH function on (*)
+ * PU90K: 0 = internal pull-up resistance for touch detection is ~50kohms (*)
+ * 1 = internal pull-up resistance for touch detection is ~90kohms
+ * DUAL: 0 = dual touch detection off (*)
+ * 1 = dual touch detection on
+ * PIDAC_OFS: dual touch detection circuit adjustment, it is not necessary
+ * to change this from initial value
+ */
+#define BU21029_CFR3_REG (0x0B << 3)
+#define CFR3_VALUE 0x42
+
+/* LDO Register (PAGE=0, ADDR=0x0C, Reset value=0x00)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | 0 | PVDD[2:0] | 0 | AVDD[2:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * PVDD: output voltage of panel output regulator (=2.000V)
+ * AVDD: output voltage of analog circuit regulator (=2.000V)
+ */
+#define BU21029_LDO_REG (0x0C << 3)
+#define LDO_VALUE 0x77
+
+/* Serial Interface Command Byte 1 (CID=1)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | 1 | CF | CMSK | PDM | STP |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * CF: conversion function, see table 3 in datasheet p6 (=0000, automatic scan)
+ * CMSK: 0 = executes convert function (*)
+ * 1 = reads the convert result
+ * PDM: 0 = power down after convert function stops (*)
+ * 1 = keep power on after convert function stops
+ * STP: 1 = abort current conversion and power down, set to "0" automatically
+ */
+#define BU21029_AUTOSCAN 0x80
+
+/* The timeout value needs to be larger than INTVL_TIME + tConv4 (sample and
+ * conversion time), where tConv4 is calculated by formula:
+ * tPON + tDLY1 + (tTIME_ST_ADC + (tADC * tSMPL) * 2 + tDLY2) * 3
+ * see figure 8 in datasheet p15 for details of each field.
+ */
+#define PEN_UP_TIMEOUT msecs_to_jiffies(50)
+
+#define STOP_DELAY_US 50L
+#define START_DELAY_MS 2L
+#define BUF_LEN 8L
+#define SCALE_12BIT (1 << 12)
+#define MAX_12BIT ((1 << 12) - 1)
+#define DRIVER_NAME "bu21029"
+
+struct bu21029_ts_data {
+ struct i2c_client *client;
+ struct input_dev *in_dev;
+ struct timer_list timer;
+ u32 reset_gpios;
+ u32 reset_gpios_assert;
+ u32 x_plate_ohms;
+ u32 max_pressure;
+};
+
+static int bu21029_touch_report(struct bu21029_ts_data *bu21029)
+{
+ struct i2c_client *i2c = bu21029->client;
+ u8 buf[BUF_LEN];
+ u16 x, y, z1, z2;
+ u32 rz;
+
+ /* read touch data and deassert INT (by restarting the autoscan mode) */
+ int error = i2c_smbus_read_i2c_block_data(i2c,
+ BU21029_AUTOSCAN,
+ BUF_LEN,
+ buf);
+ if (error < 0)
+ return error;
+
+ /* compose upper 8 and lower 4 bits into a 12bit value:
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ * | ByteH | ByteL |
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ * |b07|b06|b05|b04|b03|b02|b01|b00|b07|b06|b05|b04|b03|b02|b01|b00|
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ * |v11|v10|v09|v08|v07|v06|v05|v04|v03|v02|v01|v00| 0 | 0 | 0 | 0 |
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ */
+ x = (buf[0] << 4) | (buf[1] >> 4);
+ y = (buf[2] << 4) | (buf[3] >> 4);
+ z1 = (buf[4] << 4) | (buf[5] >> 4);
+ z2 = (buf[6] << 4) | (buf[7] >> 4);
+
+ if (z1 == 0 || z2 == 0)
+ return 0;
+
+ /* calculate Rz (pressure resistance value) by equation:
+ * Rz = Rx * (x/Q) * ((z2/z1) - 1), where
+ * Rx is x-plate resistance,
+ * Q is the touch screen resolution (8bit = 256, 12bit = 4096)
+ * x, z1, z2 are the measured positions.
+ */
+ rz = z2 - z1;
+ rz *= x;
+ rz *= bu21029->x_plate_ohms;
+ rz /= z1;
+ rz = DIV_ROUND_CLOSEST(rz, SCALE_12BIT);
+ if (rz <= bu21029->max_pressure) {
+ input_report_abs(bu21029->in_dev, ABS_X, x);
+ input_report_abs(bu21029->in_dev, ABS_Y, y);
+ input_report_abs(bu21029->in_dev, ABS_PRESSURE, rz);
+ input_report_key(bu21029->in_dev, BTN_TOUCH, 1);
+ input_sync(bu21029->in_dev);
+ }
+
+ return 0;
+}
+
+static void bu21029_touch_release(struct timer_list *t)
+{
+ struct bu21029_ts_data *bu21029 = from_timer(bu21029, t, timer);
+
+ input_report_abs(bu21029->in_dev, ABS_PRESSURE, 0);
+ input_report_key(bu21029->in_dev, BTN_TOUCH, 0);
+ input_sync(bu21029->in_dev);
+}
+
+static irqreturn_t bu21029_touch_soft_irq(int irq, void *data)
+{
+ struct bu21029_ts_data *bu21029 = data;
+ struct i2c_client *i2c = bu21029->client;
+
+ /* report touch and deassert interrupt (will assert again after
+ * INTVL_TIME + tConv4 for continuous touch)
+ */
+ int error = bu21029_touch_report(bu21029);
+
+ if (error) {
+ dev_err(&i2c->dev, "failed to report (error: %d)\n", error);
+ return IRQ_NONE;
+ }
+
+ /* reset timer for pen up detection */
+ mod_timer(&bu21029->timer, jiffies + PEN_UP_TIMEOUT);
+
+ return IRQ_HANDLED;
+}
+
+static void bu21029_reset_chip(struct bu21029_ts_data *bu21029)
+{
+ gpio_set_value(bu21029->reset_gpios,
+ bu21029->reset_gpios_assert);
+ udelay(STOP_DELAY_US);
+ gpio_set_value(bu21029->reset_gpios,
+ !bu21029->reset_gpios_assert);
+ mdelay(START_DELAY_MS);
+}
+
+static int bu21029_init_chip(struct bu21029_ts_data *bu21029)
+{
+ struct i2c_client *i2c = bu21029->client;
+ struct {
+ u8 reg;
+ u8 value;
+ } init_table[] = {
+ {BU21029_CFR0_REG, CFR0_VALUE},
+ {BU21029_CFR1_REG, CFR1_VALUE},
+ {BU21029_CFR2_REG, CFR2_VALUE},
+ {BU21029_CFR3_REG, CFR3_VALUE},
+ {BU21029_LDO_REG, LDO_VALUE}
+ };
+ int error, i;
+ u16 hwid;
+
+ bu21029_reset_chip(bu21029);
+
+ error = i2c_smbus_read_i2c_block_data(i2c,
+ BU21029_HWID_REG,
+ 2,
+ (u8 *)&hwid);
+ if (error < 0) {
+ dev_err(&i2c->dev, "failed to read HW ID\n");
+ return error;
+ }
+
+ if (cpu_to_be16(hwid) != SUPPORTED_HWID) {
+ dev_err(&i2c->dev, "unsupported HW ID 0x%x\n", hwid);
+ return -ENODEV;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(init_table); ++i) {
+ error = i2c_smbus_write_byte_data(i2c,
+ init_table[i].reg,
+ init_table[i].value);
+ if (error < 0) {
+ dev_err(&i2c->dev,
+ "failed to write 0x%x to register 0x%x\n",
+ init_table[i].value,
+ init_table[i].reg);
+ return error;
+ }
+ }
+
+ error = i2c_smbus_write_byte(i2c, BU21029_AUTOSCAN);
+ if (error < 0) {
+ dev_err(&i2c->dev, "failed to start autoscan\n");
+ return error;
+ }
+
+ return 0;
+}
+
+static int bu21029_parse_dt(struct bu21029_ts_data *bu21029)
+{
+ struct device *dev = &bu21029->client->dev;
+ struct device_node *np = dev->of_node;
+ enum of_gpio_flags flags;
+ u32 val32;
+ int gpio;
+
+ if (!np) {
+ dev_err(dev, "no device tree data\n");
+ return -EINVAL;
+ }
+
+ gpio = of_get_named_gpio_flags(np, "reset-gpios", 0, &flags);
+ if (!gpio_is_valid(gpio)) {
+ dev_err(dev, "invalid 'reset-gpios' supplied\n");
+ return -EINVAL;
+ }
+ bu21029->reset_gpios = gpio;
+ bu21029->reset_gpios_assert = (flags & OF_GPIO_ACTIVE_LOW) ? 0 : 1;
+
+ if (of_property_read_u32(np, "rohm,x-plate-ohms", &val32)) {
+ dev_err(dev, "invalid 'x-plate-ohms' supplied\n");
+ return -EINVAL;
+ }
+ bu21029->x_plate_ohms = val32;
+
+ if (of_property_read_u32(np, "touchscreen-max-pressure", &val32))
+ bu21029->max_pressure = MAX_12BIT;
+ else
+ bu21029->max_pressure = val32;
+
+ return 0;
+}
+
+static int bu21029_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct bu21029_ts_data *bu21029;
+ struct input_dev *in_dev;
+ int error;
+
+ if (!i2c_check_functionality(client->adapter,
+ I2C_FUNC_SMBUS_WRITE_BYTE |
+ I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
+ I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
+ dev_err(&client->dev,
+ "i2c functionality support is not sufficient\n");
+ return -EIO;
+ }
+
+ bu21029 = devm_kzalloc(&client->dev, sizeof(*bu21029), GFP_KERNEL);
+ if (!bu21029)
+ return -ENOMEM;
+
+ in_dev = devm_input_allocate_device(&client->dev);
+ if (!in_dev) {
+ dev_err(&client->dev, "unable to allocate input device\n");
+ return -ENOMEM;
+ }
+
+ bu21029->client = client;
+ bu21029->in_dev = in_dev;
+ timer_setup(&bu21029->timer, bu21029_touch_release, 0);
+
+ error = bu21029_parse_dt(bu21029);
+ if (error)
+ return error;
+
+ error = devm_gpio_request_one(&client->dev,
+ bu21029->reset_gpios,
+ GPIOF_OUT_INIT_HIGH,
+ DRIVER_NAME);
+ if (error) {
+ dev_err(&client->dev, "unable to request reset-gpios\n");
+ return error;
+ }
+
+ error = bu21029_init_chip(bu21029);
+ if (error) {
+ dev_err(&client->dev, "unable to config bu21029\n");
+ return error;
+ }
+
+ in_dev->name = DRIVER_NAME;
+ in_dev->id.bustype = BUS_I2C;
+ in_dev->dev.parent = &client->dev;
+
+ __set_bit(EV_SYN, in_dev->evbit);
+ __set_bit(EV_KEY, in_dev->evbit);
+ __set_bit(EV_ABS, in_dev->evbit);
+ __set_bit(ABS_X, in_dev->absbit);
+ __set_bit(ABS_Y, in_dev->absbit);
+ __set_bit(ABS_PRESSURE, in_dev->absbit);
+ __set_bit(BTN_TOUCH, in_dev->keybit);
+
+ input_set_abs_params(in_dev, ABS_X, 0, MAX_12BIT, 0, 0);
+ input_set_abs_params(in_dev, ABS_Y, 0, MAX_12BIT, 0, 0);
+ input_set_abs_params(in_dev, ABS_PRESSURE,
+ 0, bu21029->max_pressure, 0, 0);
+ input_set_drvdata(in_dev, bu21029);
+
+ error = input_register_device(in_dev);
+ if (error) {
+ dev_err(&client->dev, "unable to register input device\n");
+ return error;
+ }
+
+ i2c_set_clientdata(client, bu21029);
+
+ error = devm_request_threaded_irq(&client->dev,
+ client->irq,
+ NULL,
+ bu21029_touch_soft_irq,
+ IRQF_ONESHOT,
+ DRIVER_NAME,
+ bu21029);
+ if (error) {
+ dev_err(&client->dev, "unable to request touch irq\n");
+ return error;
+ }
+
+ return 0;
+}
+
+static int bu21029_remove(struct i2c_client *client)
+{
+ struct bu21029_ts_data *bu21029 = i2c_get_clientdata(client);
+
+ del_timer_sync(&bu21029->timer);
+
+ return 0;
+}
+
+static const struct i2c_device_id bu21029_ids[] = {
+ {DRIVER_NAME, 0},
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, bu21029_ids);
+
+static struct i2c_driver bu21029_driver = {
+ .driver = {
+ .name = DRIVER_NAME,
+ .owner = THIS_MODULE,
+ },
+ .id_table = bu21029_ids,
+ .probe = bu21029_probe,
+ .remove = bu21029_remove,
+};
+module_i2c_driver(bu21029_driver);
+
+MODULE_AUTHOR("Zhu Yi <[email protected]>");
+MODULE_DESCRIPTION("Rohm BU21029 touchscreen controller driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4



2018-03-26 22:32:05

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH] Input: add bu21029 touch driver

On Wed, Mar 21, 2018 at 06:04:34PM +0100, Mark Jonas wrote:
> From: Zhu Yi <[email protected]>
>
> Add the ROHM BU21029 resistive touch panel controller
> support with i2c interface.
>
> Signed-off-by: Zhu Yi <[email protected]>
> Signed-off-by: Mark Jonas <[email protected]>
> Reviewed-by: Heiko Schocher <[email protected]>
> ---
> .../bindings/input/touchscreen/bu21029.txt | 30 ++
> drivers/input/touchscreen/Kconfig | 12 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/bu21029_ts.c | 456 +++++++++++++++++++++
> 4 files changed, 499 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
> create mode 100644 drivers/input/touchscreen/bu21029_ts.c
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
> new file mode 100644
> index 0000000..7b61602
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
> @@ -0,0 +1,30 @@
> +* Rohm BU21029 Touch Screen Controller
> +
> +Required properties:
> + - compatible : must be "rohm,bu21029"
> + - reg : i2c device address of the chip

What the valid value(s)?

> + - interrupt-parent : the phandle for the gpio controller
> + - interrupts : (gpio) interrupt to which the chip is connected
> + - reset-gpios : gpio pin to reset the chip

Active high or low?

> + - rohm,x-plate-ohms : x-plate resistance in ohms

IIRC, we have a standard touchscreen property for this?

> +
> +Optional properties:
> + - touchscreen-max-pressure: maximum pressure value
> +
> +Example:
> +
> + &i2c1 {
> + /* ... */
> +
> + bu21029: bu21029@40 {
> + compatible = "rohm,bu21029";
> + reg = <0x40>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
> + reset-gpios = <&gpio6 16 GPIO_ACTIVE_LOW>;
> + rohm,x-plate-ohms = <600>;
> + touchscreen-max-pressure = <4095>;
> + };
> +
> + /* ... */
> + };

2018-03-26 22:41:34

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] Input: add bu21029 touch driver

On Mon, Mar 26, 2018 at 05:24:26PM -0500, Rob Herring wrote:
> On Wed, Mar 21, 2018 at 06:04:34PM +0100, Mark Jonas wrote:
> > From: Zhu Yi <[email protected]>
> >
> > Add the ROHM BU21029 resistive touch panel controller
> > support with i2c interface.
> >
> > Signed-off-by: Zhu Yi <[email protected]>
> > Signed-off-by: Mark Jonas <[email protected]>
> > Reviewed-by: Heiko Schocher <[email protected]>
> > ---
> > .../bindings/input/touchscreen/bu21029.txt | 30 ++
> > drivers/input/touchscreen/Kconfig | 12 +
> > drivers/input/touchscreen/Makefile | 1 +
> > drivers/input/touchscreen/bu21029_ts.c | 456 +++++++++++++++++++++
> > 4 files changed, 499 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
> > create mode 100644 drivers/input/touchscreen/bu21029_ts.c
> >
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
> > new file mode 100644
> > index 0000000..7b61602
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
> > @@ -0,0 +1,30 @@
> > +* Rohm BU21029 Touch Screen Controller
> > +
> > +Required properties:
> > + - compatible : must be "rohm,bu21029"
> > + - reg : i2c device address of the chip
>
> What the valid value(s)?
>
> > + - interrupt-parent : the phandle for the gpio controller
> > + - interrupts : (gpio) interrupt to which the chip is connected
> > + - reset-gpios : gpio pin to reset the chip
>
> Active high or low?
>
> > + - rohm,x-plate-ohms : x-plate resistance in ohms
>
> IIRC, we have a standard touchscreen property for this?

I do not think so: it is specific for resistive touchscreens, whereas
"standard" touchscreen properties are technology-independent ones (size,
rotation, etc).

>
> > +
> > +Optional properties:
> > + - touchscreen-max-pressure: maximum pressure value
> > +
> > +Example:
> > +
> > + &i2c1 {
> > + /* ... */
> > +
> > + bu21029: bu21029@40 {
> > + compatible = "rohm,bu21029";
> > + reg = <0x40>;
> > + interrupt-parent = <&gpio1>;
> > + interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
> > + reset-gpios = <&gpio6 16 GPIO_ACTIVE_LOW>;
> > + rohm,x-plate-ohms = <600>;
> > + touchscreen-max-pressure = <4095>;
> > + };
> > +
> > + /* ... */
> > + };

--
Dmitry

2018-03-26 22:54:53

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] Input: add bu21029 touch driver

Hi Mark,

On Wed, Mar 21, 2018 at 06:04:34PM +0100, Mark Jonas wrote:
> From: Zhu Yi <[email protected]>
>
> Add the ROHM BU21029 resistive touch panel controller
> support with i2c interface.
>
> Signed-off-by: Zhu Yi <[email protected]>
> Signed-off-by: Mark Jonas <[email protected]>
> Reviewed-by: Heiko Schocher <[email protected]>
> ---
> .../bindings/input/touchscreen/bu21029.txt | 30 ++
> drivers/input/touchscreen/Kconfig | 12 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/bu21029_ts.c | 456 +++++++++++++++++++++
> 4 files changed, 499 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
> create mode 100644 drivers/input/touchscreen/bu21029_ts.c
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
> new file mode 100644
> index 0000000..7b61602
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
> @@ -0,0 +1,30 @@
> +* Rohm BU21029 Touch Screen Controller
> +
> +Required properties:
> + - compatible : must be "rohm,bu21029"
> + - reg : i2c device address of the chip
> + - interrupt-parent : the phandle for the gpio controller
> + - interrupts : (gpio) interrupt to which the chip is connected
> + - reset-gpios : gpio pin to reset the chip
> + - rohm,x-plate-ohms : x-plate resistance in ohms
> +
> +Optional properties:
> + - touchscreen-max-pressure: maximum pressure value
> +
> +Example:
> +
> + &i2c1 {
> + /* ... */
> +
> + bu21029: bu21029@40 {
> + compatible = "rohm,bu21029";
> + reg = <0x40>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
> + reset-gpios = <&gpio6 16 GPIO_ACTIVE_LOW>;
> + rohm,x-plate-ohms = <600>;
> + touchscreen-max-pressure = <4095>;
> + };
> +
> + /* ... */
> + };
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 4f15496..e09fe8f 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -151,6 +151,18 @@ config TOUCHSCREEN_BU21013
> To compile this driver as a module, choose M here: the
> module will be called bu21013_ts.
>
> +config TOUCHSCREEN_BU21029
> + tristate "Rohm BU21029 based touch panel controllers"
> + depends on I2C
> + help
> + Say Y here if you have a Rohm BU21029 touchscreen controller
> + connected to your system.
> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called bu21029_ts.
> +
> config TOUCHSCREEN_CHIPONE_ICN8318
> tristate "chipone icn8318 touchscreen controller"
> depends on GPIOLIB || COMPILE_TEST
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index dddae79..f50624c 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_TOUCHSCREEN_AR1021_I2C) += ar1021_i2c.o
> obj-$(CONFIG_TOUCHSCREEN_ATMEL_MXT) += atmel_mxt_ts.o
> obj-$(CONFIG_TOUCHSCREEN_AUO_PIXCIR) += auo-pixcir-ts.o
> obj-$(CONFIG_TOUCHSCREEN_BU21013) += bu21013_ts.o
> +obj-$(CONFIG_TOUCHSCREEN_BU21029) += bu21029_ts.o
> obj-$(CONFIG_TOUCHSCREEN_CHIPONE_ICN8318) += chipone_icn8318.o
> obj-$(CONFIG_TOUCHSCREEN_CY8CTMG110) += cy8ctmg110_ts.o
> obj-$(CONFIG_TOUCHSCREEN_CYTTSP_CORE) += cyttsp_core.o
> diff --git a/drivers/input/touchscreen/bu21029_ts.c b/drivers/input/touchscreen/bu21029_ts.c
> new file mode 100644
> index 0000000..d5cbf11
> --- /dev/null
> +++ b/drivers/input/touchscreen/bu21029_ts.c
> @@ -0,0 +1,456 @@

Please add SPDX tag for the driver.

> +/*
> + * Rohm BU21029 touchscreen controller driver
> + *
> + * Copyright (C) 2015 Bosch Sicherheitssysteme GmbH
> + *
> + * 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/delay.h>
> +#include <linux/gpio.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>


Please use GPIOD API (and include linux/gpio/consumer.h instead of
of_gpio.h).

> +#include <linux/timer.h>
> +
> +/* HW_ID1 Register (PAGE=0, ADDR=0x0E, Reset value=0x02, Read only)

Multi-line comments start with empty comment line:

/*
* Multi
* line.
*/

> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * | HW_IDH |
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * HW_ID2 Register (PAGE=0, ADDR=0x0F, Reset value=0x29, Read only)
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * | HW_IDL |
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * HW_IDH: high 8bits of IC's ID
> + * HW_IDL: low 8bits of IC's ID
> + */
> +#define BU21029_HWID_REG (0x0E << 3)
> +#define SUPPORTED_HWID 0x0229
> +
> +/* CFR0 Register (PAGE=0, ADDR=0x00, Reset value=0x20)
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * | 0 | 0 | CALIB | INTRM | 0 | 0 | 0 | 0 |
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * CALIB: 0 = not to use calibration result (*)
> + * 1 = use calibration result
> + * INTRM: 0 = INT output depend on "pen down" (*)
> + * 1 = INT output always "0"
> + */
> +#define BU21029_CFR0_REG (0x00 << 3)
> +#define CFR0_VALUE 0x00
> +
> +/* CFR1 Register (PAGE=0, ADDR=0x01, Reset value=0xA6)
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * | MAV | AVE[2:0] | 0 | SMPL[2:0] |
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * MAV: 0 = median average filter off
> + * 1 = median average filter on (*)
> + * AVE: AVE+1 = number of average samples for MAV,
> + * if AVE>SMPL, then AVE=SMPL (=3)
> + * SMPL: SMPL+1 = number of conversion samples for MAV (=7)
> + */
> +#define BU21029_CFR1_REG (0x01 << 3)
> +#define CFR1_VALUE 0xA6
> +
> +/* CFR2 Register (PAGE=0, ADDR=0x02, Reset value=0x04)
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * | INTVL_TIME[3:0] | TIME_ST_ADC[3:0] |
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * INTVL_TIME: waiting time between completion of conversion
> + * and start of next conversion, only usable in
> + * autoscan mode (=20.480ms)
> + * TIME_ST_ADC: waiting time between application of voltage
> + * to panel and start of A/D conversion (=100us)
> + */
> +#define BU21029_CFR2_REG (0x02 << 3)
> +#define CFR2_VALUE 0xC9
> +
> +/* CFR3 Register (PAGE=0, ADDR=0x0B, Reset value=0x72)
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * | RM8 | STRETCH| PU90K | DUAL | PIDAC_OFS[3:0] |
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * RM8: 0 = coordinate resolution is 12bit (*)
> + * 1 = coordinate resolution is 8bit
> + * STRETCH: 0 = SCL_STRETCH function off
> + * 1 = SCL_STRETCH function on (*)
> + * PU90K: 0 = internal pull-up resistance for touch detection is ~50kohms (*)
> + * 1 = internal pull-up resistance for touch detection is ~90kohms
> + * DUAL: 0 = dual touch detection off (*)
> + * 1 = dual touch detection on
> + * PIDAC_OFS: dual touch detection circuit adjustment, it is not necessary
> + * to change this from initial value
> + */
> +#define BU21029_CFR3_REG (0x0B << 3)
> +#define CFR3_VALUE 0x42
> +
> +/* LDO Register (PAGE=0, ADDR=0x0C, Reset value=0x00)
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * | 0 | PVDD[2:0] | 0 | AVDD[2:0] |
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * PVDD: output voltage of panel output regulator (=2.000V)
> + * AVDD: output voltage of analog circuit regulator (=2.000V)
> + */
> +#define BU21029_LDO_REG (0x0C << 3)
> +#define LDO_VALUE 0x77
> +
> +/* Serial Interface Command Byte 1 (CID=1)
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * | 1 | CF | CMSK | PDM | STP |
> + * +--------+--------+--------+--------+--------+--------+--------+--------+
> + * CF: conversion function, see table 3 in datasheet p6 (=0000, automatic scan)
> + * CMSK: 0 = executes convert function (*)
> + * 1 = reads the convert result
> + * PDM: 0 = power down after convert function stops (*)
> + * 1 = keep power on after convert function stops
> + * STP: 1 = abort current conversion and power down, set to "0" automatically
> + */
> +#define BU21029_AUTOSCAN 0x80
> +
> +/* The timeout value needs to be larger than INTVL_TIME + tConv4 (sample and
> + * conversion time), where tConv4 is calculated by formula:
> + * tPON + tDLY1 + (tTIME_ST_ADC + (tADC * tSMPL) * 2 + tDLY2) * 3
> + * see figure 8 in datasheet p15 for details of each field.
> + */
> +#define PEN_UP_TIMEOUT msecs_to_jiffies(50)
> +
> +#define STOP_DELAY_US 50L
> +#define START_DELAY_MS 2L
> +#define BUF_LEN 8L
> +#define SCALE_12BIT (1 << 12)
> +#define MAX_12BIT ((1 << 12) - 1)
> +#define DRIVER_NAME "bu21029"
> +
> +struct bu21029_ts_data {
> + struct i2c_client *client;
> + struct input_dev *in_dev;
> + struct timer_list timer;
> + u32 reset_gpios;
> + u32 reset_gpios_assert;
> + u32 x_plate_ohms;
> + u32 max_pressure;
> +};
> +
> +static int bu21029_touch_report(struct bu21029_ts_data *bu21029)
> +{
> + struct i2c_client *i2c = bu21029->client;
> + u8 buf[BUF_LEN];
> + u16 x, y, z1, z2;
> + u32 rz;
> +
> + /* read touch data and deassert INT (by restarting the autoscan mode) */
> + int error = i2c_smbus_read_i2c_block_data(i2c,
> + BU21029_AUTOSCAN,
> + BUF_LEN,
> + buf);
> + if (error < 0)
> + return error;
> +
> + /* compose upper 8 and lower 4 bits into a 12bit value:
> + * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
> + * | ByteH | ByteL |
> + * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
> + * |b07|b06|b05|b04|b03|b02|b01|b00|b07|b06|b05|b04|b03|b02|b01|b00|
> + * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
> + * |v11|v10|v09|v08|v07|v06|v05|v04|v03|v02|v01|v00| 0 | 0 | 0 | 0 |
> + * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
> + */
> + x = (buf[0] << 4) | (buf[1] >> 4);
> + y = (buf[2] << 4) | (buf[3] >> 4);
> + z1 = (buf[4] << 4) | (buf[5] >> 4);
> + z2 = (buf[6] << 4) | (buf[7] >> 4);
> +
> + if (z1 == 0 || z2 == 0)
> + return 0;
> +
> + /* calculate Rz (pressure resistance value) by equation:
> + * Rz = Rx * (x/Q) * ((z2/z1) - 1), where
> + * Rx is x-plate resistance,
> + * Q is the touch screen resolution (8bit = 256, 12bit = 4096)
> + * x, z1, z2 are the measured positions.
> + */
> + rz = z2 - z1;
> + rz *= x;
> + rz *= bu21029->x_plate_ohms;
> + rz /= z1;
> + rz = DIV_ROUND_CLOSEST(rz, SCALE_12BIT);
> + if (rz <= bu21029->max_pressure) {
> + input_report_abs(bu21029->in_dev, ABS_X, x);
> + input_report_abs(bu21029->in_dev, ABS_Y, y);
> + input_report_abs(bu21029->in_dev, ABS_PRESSURE, rz);

What is the values of pressure reported when finger is touching the
surface? IOW is 'rz' pressure or resistance?

> + input_report_key(bu21029->in_dev, BTN_TOUCH, 1);
> + input_sync(bu21029->in_dev);
> + }
> +
> + return 0;
> +}
> +
> +static void bu21029_touch_release(struct timer_list *t)
> +{
> + struct bu21029_ts_data *bu21029 = from_timer(bu21029, t, timer);
> +
> + input_report_abs(bu21029->in_dev, ABS_PRESSURE, 0);
> + input_report_key(bu21029->in_dev, BTN_TOUCH, 0);
> + input_sync(bu21029->in_dev);
> +}
> +
> +static irqreturn_t bu21029_touch_soft_irq(int irq, void *data)
> +{
> + struct bu21029_ts_data *bu21029 = data;
> + struct i2c_client *i2c = bu21029->client;
> +
> + /* report touch and deassert interrupt (will assert again after
> + * INTVL_TIME + tConv4 for continuous touch)
> + */
> + int error = bu21029_touch_report(bu21029);
> +
> + if (error) {
> + dev_err(&i2c->dev, "failed to report (error: %d)\n", error);
> + return IRQ_NONE;
> + }
> +
> + /* reset timer for pen up detection */
> + mod_timer(&bu21029->timer, jiffies + PEN_UP_TIMEOUT);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void bu21029_reset_chip(struct bu21029_ts_data *bu21029)
> +{
> + gpio_set_value(bu21029->reset_gpios,
> + bu21029->reset_gpios_assert);
> + udelay(STOP_DELAY_US);
> + gpio_set_value(bu21029->reset_gpios,
> + !bu21029->reset_gpios_assert);
> + mdelay(START_DELAY_MS);
> +}
> +
> +static int bu21029_init_chip(struct bu21029_ts_data *bu21029)
> +{
> + struct i2c_client *i2c = bu21029->client;
> + struct {
> + u8 reg;
> + u8 value;
> + } init_table[] = {
> + {BU21029_CFR0_REG, CFR0_VALUE},
> + {BU21029_CFR1_REG, CFR1_VALUE},
> + {BU21029_CFR2_REG, CFR2_VALUE},
> + {BU21029_CFR3_REG, CFR3_VALUE},
> + {BU21029_LDO_REG, LDO_VALUE}
> + };
> + int error, i;
> + u16 hwid;
> +
> + bu21029_reset_chip(bu21029);
> +
> + error = i2c_smbus_read_i2c_block_data(i2c,
> + BU21029_HWID_REG,
> + 2,
> + (u8 *)&hwid);
> + if (error < 0) {
> + dev_err(&i2c->dev, "failed to read HW ID\n");
> + return error;
> + }
> +
> + if (cpu_to_be16(hwid) != SUPPORTED_HWID) {
> + dev_err(&i2c->dev, "unsupported HW ID 0x%x\n", hwid);
> + return -ENODEV;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(init_table); ++i) {
> + error = i2c_smbus_write_byte_data(i2c,
> + init_table[i].reg,
> + init_table[i].value);
> + if (error < 0) {
> + dev_err(&i2c->dev,
> + "failed to write 0x%x to register 0x%x\n",
> + init_table[i].value,
> + init_table[i].reg);
> + return error;
> + }
> + }
> +
> + error = i2c_smbus_write_byte(i2c, BU21029_AUTOSCAN);
> + if (error < 0) {
> + dev_err(&i2c->dev, "failed to start autoscan\n");
> + return error;
> + }
> +
> + return 0;
> +}
> +
> +static int bu21029_parse_dt(struct bu21029_ts_data *bu21029)
> +{
> + struct device *dev = &bu21029->client->dev;
> + struct device_node *np = dev->of_node;
> + enum of_gpio_flags flags;
> + u32 val32;
> + int gpio;
> +
> + if (!np) {
> + dev_err(dev, "no device tree data\n");
> + return -EINVAL;
> + }
> +
> + gpio = of_get_named_gpio_flags(np, "reset-gpios", 0, &flags);
> + if (!gpio_is_valid(gpio)) {
> + dev_err(dev, "invalid 'reset-gpios' supplied\n");
> + return -EINVAL;
> + }
> + bu21029->reset_gpios = gpio;
> + bu21029->reset_gpios_assert = (flags & OF_GPIO_ACTIVE_LOW) ? 0 : 1;
> +
> + if (of_property_read_u32(np, "rohm,x-plate-ohms", &val32)) {
> + dev_err(dev, "invalid 'x-plate-ohms' supplied\n");
> + return -EINVAL;
> + }
> + bu21029->x_plate_ohms = val32;
> +
> + if (of_property_read_u32(np, "touchscreen-max-pressure", &val32))
> + bu21029->max_pressure = MAX_12BIT;
> + else
> + bu21029->max_pressure = val32;

Please use infrastructure form include/linux/input/touchscreen.h
so that you handle different sizes and orientations.

> +
> + return 0;
> +}
> +
> +static int bu21029_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct bu21029_ts_data *bu21029;
> + struct input_dev *in_dev;
> + int error;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_WRITE_BYTE |
> + I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
> + I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
> + dev_err(&client->dev,
> + "i2c functionality support is not sufficient\n");
> + return -EIO;
> + }
> +
> + bu21029 = devm_kzalloc(&client->dev, sizeof(*bu21029), GFP_KERNEL);
> + if (!bu21029)
> + return -ENOMEM;
> +
> + in_dev = devm_input_allocate_device(&client->dev);
> + if (!in_dev) {
> + dev_err(&client->dev, "unable to allocate input device\n");
> + return -ENOMEM;
> + }
> +
> + bu21029->client = client;
> + bu21029->in_dev = in_dev;
> + timer_setup(&bu21029->timer, bu21029_touch_release, 0);
> +
> + error = bu21029_parse_dt(bu21029);
> + if (error)
> + return error;
> +
> + error = devm_gpio_request_one(&client->dev,
> + bu21029->reset_gpios,
> + GPIOF_OUT_INIT_HIGH,
> + DRIVER_NAME);
> + if (error) {
> + dev_err(&client->dev, "unable to request reset-gpios\n");
> + return error;
> + }
> +
> + error = bu21029_init_chip(bu21029);
> + if (error) {
> + dev_err(&client->dev, "unable to config bu21029\n");
> + return error;
> + }
> +
> + in_dev->name = DRIVER_NAME;
> + in_dev->id.bustype = BUS_I2C;
> + in_dev->dev.parent = &client->dev;

Not needed with devm_input_allocate_device().

> +
> + __set_bit(EV_SYN, in_dev->evbit);
> + __set_bit(EV_KEY, in_dev->evbit);
> + __set_bit(EV_ABS, in_dev->evbit);
> + __set_bit(ABS_X, in_dev->absbit);
> + __set_bit(ABS_Y, in_dev->absbit);
> + __set_bit(ABS_PRESSURE, in_dev->absbit);
> + __set_bit(BTN_TOUCH, in_dev->keybit);
> +
> + input_set_abs_params(in_dev, ABS_X, 0, MAX_12BIT, 0, 0);
> + input_set_abs_params(in_dev, ABS_Y, 0, MAX_12BIT, 0, 0);
> + input_set_abs_params(in_dev, ABS_PRESSURE,
> + 0, bu21029->max_pressure, 0, 0);
> + input_set_drvdata(in_dev, bu21029);
> +
> + error = input_register_device(in_dev);
> + if (error) {
> + dev_err(&client->dev, "unable to register input device\n");
> + return error;
> + }
> +
> + i2c_set_clientdata(client, bu21029);
> +
> + error = devm_request_threaded_irq(&client->dev,
> + client->irq,
> + NULL,
> + bu21029_touch_soft_irq,
> + IRQF_ONESHOT,
> + DRIVER_NAME,
> + bu21029);
> + if (error) {
> + dev_err(&client->dev, "unable to request touch irq\n");
> + return error;
> + }
> +
> + return 0;
> +}
> +
> +static int bu21029_remove(struct i2c_client *client)
> +{
> + struct bu21029_ts_data *bu21029 = i2c_get_clientdata(client);
> +
> + del_timer_sync(&bu21029->timer);

If interrupt comes here kernel will be unhappy. You need to either work
canceling timer into devm unwid stream (devm_add_action_or_reset()) or
somehow make sure that you shut off interrupts before canceling the
timer.

> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id bu21029_ids[] = {
> + {DRIVER_NAME, 0},
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, bu21029_ids);
> +
> +static struct i2c_driver bu21029_driver = {
> + .driver = {
> + .name = DRIVER_NAME,
> + .owner = THIS_MODULE,

Not needed.

> + },
> + .id_table = bu21029_ids,
> + .probe = bu21029_probe,
> + .remove = bu21029_remove,
> +};
> +module_i2c_driver(bu21029_driver);
> +
> +MODULE_AUTHOR("Zhu Yi <[email protected]>");
> +MODULE_DESCRIPTION("Rohm BU21029 touchscreen controller driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>

Thanks.

--
Dmitry

Subject: Re: [PATCH] Input: add bu21029 touch driver

Hello Rob,

> > +* Rohm BU21029 Touch Screen Controller
> > +
> > +Required properties:
> > + - compatible : must be "rohm,bu21029"
> > + - reg : i2c device address of the chip
>
> What the valid value(s)?

The chip itself can be wired to 0x40 or 0x41. The driver can work with
any valid I2C address.

Is this what you are looking for?

- reg : i2c device address of the chip (0x40 or 0x41)

> > + - interrupt-parent : the phandle for the gpio controller
> > + - interrupts : (gpio) interrupt to which the chip is connected
> > + - reset-gpios : gpio pin to reset the chip
>
> Active high or low?

The chip itself needs an active low reset. But depending on whether
there is an inverter between the touch screen controller and the CPU or
not, the CPU's GPIO might need to be active high or active low. This
shall be configured by the polarity given in the device tree. The
driver uses this information to drive the reset appropriately.

Are you looking for documenting the required polarity of the touch
controller itself?

- reset-gpios : gpio pin to reset the chip (active low)

> > +Example:
> > +
> > + &i2c1 {
> > + /* ... */
> > +
> > + bu21029: bu21029@40 {
> > + compatible = "rohm,bu21029";
> > + reg = <0x40>;
> > + interrupt-parent = <&gpio1>;
> > + interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
> > + reset-gpios = <&gpio6 16 GPIO_ACTIVE_LOW>;
> > + rohm,x-plate-ohms = <600>;
> > + touchscreen-max-pressure = <4095>;
> > + };
> > +
> > + /* ... */
> > + };

Greetings,
Mark Jonas

Building Technologies, Panel Software Fire (BT-FIR/ENG1)
Bosch Sicherheitssysteme GmbH | Postfach 11 11 | 85626 Grasbrunn | GERMANY | http://www.boschsecurity.com

Sitz: Stuttgart, Registergericht: Amtsgericht Stuttgart HRB 23118
Aufsichtsratsvorsitzender: Stefan Hartung; Gesch?ftsf?hrung: Gert van Iperen, Andreas Bartz, Thomas Quante, Bernhard Schuster

Subject: Re: [PATCH] Input: add bu21029 touch driver

Hello Dmitry,

> > +++ b/drivers/input/touchscreen/bu21029_ts.c
> > @@ -0,0 +1,456 @@
>
> Please add SPDX tag for the driver.

We will do.

> Please use GPIOD API (and include linux/gpio/consumer.h instead of
> of_gpio.h).

We will do the change.

> > +/* HW_ID1 Register (PAGE=0, ADDR=0x0E, Reset value=0x02, Read only)
>
> Multi-line comments start with empty comment line:
>
> /*
> * Multi
> * line.
> */

Will be fixed.

> > + /* calculate Rz (pressure resistance value) by equation:
> > + * Rz = Rx * (x/Q) * ((z2/z1) - 1), where
> > + * Rx is x-plate resistance,
> > + * Q is the touch screen resolution (8bit = 256, 12bit = 4096)
> > + * x, z1, z2 are the measured positions.
> > + */
> > + rz = z2 - z1;
> > + rz *= x;
> > + rz *= bu21029->x_plate_ohms;
> > + rz /= z1;
> > + rz = DIV_ROUND_CLOSEST(rz, SCALE_12BIT);
> > + if (rz <= bu21029->max_pressure) {
> > + input_report_abs(bu21029->in_dev, ABS_X, x);
> > + input_report_abs(bu21029->in_dev, ABS_Y, y);
> > + input_report_abs(bu21029->in_dev, ABS_PRESSURE, rz);
>
> What is the values of pressure reported when finger is touching the
> surface? IOW is 'rz' pressure or resistance?

Rz is pressure measured in Ohms. That is, it is a resistance which
correlates with finger pressure.

I fear that I do not understand your question. Does ABS_PRESSURE have
to be reported in a specific unit, e.g. milli Newton? We thought that
it is a device specific scale and that it will be converted into a
calibrated value (just like the coordinates) in user space.

> > + if (of_property_read_u32(np, "touchscreen-max-pressure", &val32))
> > + bu21029->max_pressure = MAX_12BIT;
> > + else
> > + bu21029->max_pressure = val32;
>
> Please use infrastructure form include/linux/input/touchscreen.h
> so that you handle different sizes and orientations.

Thank you for this recommendation. We will check it out and send a new
proposal.

> > + in_dev->name = DRIVER_NAME;
> > + in_dev->id.bustype = BUS_I2C;
> > + in_dev->dev.parent = &client->dev;
>
> Not needed with devm_input_allocate_device().

We will have a look at the other drivers.

> > +static int bu21029_remove(struct i2c_client *client)
> > +{
> > + struct bu21029_ts_data *bu21029 = i2c_get_clientdata(client);
> > +
> > + del_timer_sync(&bu21029->timer);
>
> If interrupt comes here kernel will be unhappy. You need to either work
> canceling timer into devm unwid stream (devm_add_action_or_reset()) or
> somehow make sure that you shut off interrupts before canceling the
> timer.

We will work on a solution. Thank you for spotting this.

>
> > +
> > + return 0;
> > +}

> > +static struct i2c_driver bu21029_driver = {
> > + .driver = {
> > + .name = DRIVER_NAME,
> > + .owner = THIS_MODULE,
>
> Not needed.

OK, we will remove the .owner assignment.

Greetings,
Mark Jonas

Building Technologies, Panel Software Fire (BT-FIR/ENG1)
Bosch Sicherheitssysteme GmbH | Postfach 11 11 | 85626 Grasbrunn | GERMANY | http://www.boschsecurity.com

Sitz: Stuttgart, Registergericht: Amtsgericht Stuttgart HRB 23118
Aufsichtsratsvorsitzender: Stefan Hartung; Gesch?ftsf?hrung: Gert van Iperen, Andreas Bartz, Thomas Quante, Bernhard Schuster

2018-03-30 18:21:48

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] Input: add bu21029 touch driver

On Tue, Mar 27, 2018 at 06:57:42AM +0000, Jonas Mark (BT-FIR/ENG1) wrote:
> > > + /* calculate Rz (pressure resistance value) by equation:
> > > + * Rz = Rx * (x/Q) * ((z2/z1) - 1), where
> > > + * Rx is x-plate resistance,
> > > + * Q is the touch screen resolution (8bit = 256, 12bit = 4096)
> > > + * x, z1, z2 are the measured positions.
> > > + */
> > > + rz = z2 - z1;
> > > + rz *= x;
> > > + rz *= bu21029->x_plate_ohms;
> > > + rz /= z1;
> > > + rz = DIV_ROUND_CLOSEST(rz, SCALE_12BIT);
> > > + if (rz <= bu21029->max_pressure) {
> > > + input_report_abs(bu21029->in_dev, ABS_X, x);
> > > + input_report_abs(bu21029->in_dev, ABS_Y, y);
> > > + input_report_abs(bu21029->in_dev, ABS_PRESSURE, rz);
> >
> > What is the values of pressure reported when finger is touching the
> > surface? IOW is 'rz' pressure or resistance?
>
> Rz is pressure measured in Ohms. That is, it is a resistance which
> correlates with finger pressure.
>
> I fear that I do not understand your question. Does ABS_PRESSURE have
> to be reported in a specific unit, e.g. milli Newton? We thought that
> it is a device specific scale and that it will be converted into a
> calibrated value (just like the coordinates) in user space.

What I was trying to say is that it is expected that ABS_PRESSURE values
grow the harder you press on the screen, and reduce back to 0 when
finger is about to be removed from the surface. Here, it seems, we have
the opposite case, where resistance decreases the harder you press and
increases up to some maximum value when you remove the finger.

IOW, I think you want to report:

input_report_abs(bu21029->in_dev, ABS_PRESSURE,
bu21029->max_pressure - rz);

Thanks.

--
Dmitry

Subject: [PATCH v2] Input: add bu21029 touch driver

From: Zhu Yi <[email protected]>

Add Rohm BU21029 resistive touch panel controller support with I2C
interface.

Signed-off-by: Zhu Yi <[email protected]>
Signed-off-by: Mark Jonas <[email protected]>
Reviewed-by: Heiko Schocher <[email protected]>
---
Changes in v2:
- make ABS_PRESSURE proportionally rising with finger pressure
- fix race between interrupt and timer during shutdown
- use infrastructure from include/linux/input/touchscreen.h
- add SPDX tag for the driver
- improve binding documentation
- fix multi-line comments
---
.../bindings/input/touchscreen/bu21029.txt | 34 ++
drivers/input/touchscreen/Kconfig | 12 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/bu21029_ts.c | 485 +++++++++++++++++++++
4 files changed, 532 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
create mode 100644 drivers/input/touchscreen/bu21029_ts.c

diff --git a/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
new file mode 100644
index 0000000..030a888
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
@@ -0,0 +1,34 @@
+* Rohm BU21029 Touch Screen Controller
+
+Required properties:
+ - compatible : must be "rohm,bu21029"
+ - reg : i2c device address of the chip (0x40 or 0x41)
+ - interrupt-parent : the phandle for the gpio controller
+ - interrupts : (gpio) interrupt to which the chip is connected
+ - reset-gpios : gpio pin to reset the chip (active low)
+ - rohm,x-plate-ohms : x-plate resistance in Ohm
+
+Optional properties:
+ - touchscreen-size-x : horizontal resolution of touchscreen (in pixels)
+ - touchscreen-size-y : vertical resolution of touchscreen (in pixels)
+ - touchscreen-max-pressure: maximum pressure value
+
+Example:
+
+ &i2c1 {
+ /* ... */
+
+ bu21029: bu21029@40 {
+ compatible = "rohm,bu21029";
+ reg = <0x40>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
+ reset-gpios = <&gpio6 16 GPIO_ACTIVE_LOW>;
+ rohm,x-plate-ohms = <600>;
+ touchscreen-size-x = <800>;
+ touchscreen-size-y = <480>;
+ touchscreen-max-pressure = <4095>;
+ };
+
+ /* ... */
+ };
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 4f15496..e09fe8f 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -151,6 +151,18 @@ config TOUCHSCREEN_BU21013
To compile this driver as a module, choose M here: the
module will be called bu21013_ts.

+config TOUCHSCREEN_BU21029
+ tristate "Rohm BU21029 based touch panel controllers"
+ depends on I2C
+ help
+ Say Y here if you have a Rohm BU21029 touchscreen controller
+ connected to your system.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called bu21029_ts.
+
config TOUCHSCREEN_CHIPONE_ICN8318
tristate "chipone icn8318 touchscreen controller"
depends on GPIOLIB || COMPILE_TEST
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index dddae79..f50624c 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_TOUCHSCREEN_AR1021_I2C) += ar1021_i2c.o
obj-$(CONFIG_TOUCHSCREEN_ATMEL_MXT) += atmel_mxt_ts.o
obj-$(CONFIG_TOUCHSCREEN_AUO_PIXCIR) += auo-pixcir-ts.o
obj-$(CONFIG_TOUCHSCREEN_BU21013) += bu21013_ts.o
+obj-$(CONFIG_TOUCHSCREEN_BU21029) += bu21029_ts.o
obj-$(CONFIG_TOUCHSCREEN_CHIPONE_ICN8318) += chipone_icn8318.o
obj-$(CONFIG_TOUCHSCREEN_CY8CTMG110) += cy8ctmg110_ts.o
obj-$(CONFIG_TOUCHSCREEN_CYTTSP_CORE) += cyttsp_core.o
diff --git a/drivers/input/touchscreen/bu21029_ts.c b/drivers/input/touchscreen/bu21029_ts.c
new file mode 100644
index 0000000..8d0cbe51
--- /dev/null
+++ b/drivers/input/touchscreen/bu21029_ts.c
@@ -0,0 +1,485 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Rohm BU21029 touchscreen controller driver
+ *
+ * Copyright (C) 2015-2018 Bosch Sicherheitssysteme GmbH
+ *
+ * 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/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/touchscreen.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/timer.h>
+
+/*
+ * HW_ID1 Register (PAGE=0, ADDR=0x0E, Reset value=0x02, Read only)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | HW_IDH |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * HW_ID2 Register (PAGE=0, ADDR=0x0F, Reset value=0x29, Read only)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | HW_IDL |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * HW_IDH: high 8bits of IC's ID
+ * HW_IDL: low 8bits of IC's ID
+ */
+#define BU21029_HWID_REG (0x0E << 3)
+#define SUPPORTED_HWID 0x0229
+
+/*
+ * CFR0 Register (PAGE=0, ADDR=0x00, Reset value=0x20)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | 0 | 0 | CALIB | INTRM | 0 | 0 | 0 | 0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * CALIB: 0 = not to use calibration result (*)
+ * 1 = use calibration result
+ * INTRM: 0 = INT output depend on "pen down" (*)
+ * 1 = INT output always "0"
+ */
+#define BU21029_CFR0_REG (0x00 << 3)
+#define CFR0_VALUE 0x00
+
+/*
+ * CFR1 Register (PAGE=0, ADDR=0x01, Reset value=0xA6)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | MAV | AVE[2:0] | 0 | SMPL[2:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * MAV: 0 = median average filter off
+ * 1 = median average filter on (*)
+ * AVE: AVE+1 = number of average samples for MAV,
+ * if AVE>SMPL, then AVE=SMPL (=3)
+ * SMPL: SMPL+1 = number of conversion samples for MAV (=7)
+ */
+#define BU21029_CFR1_REG (0x01 << 3)
+#define CFR1_VALUE 0xA6
+
+/*
+ * CFR2 Register (PAGE=0, ADDR=0x02, Reset value=0x04)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | INTVL_TIME[3:0] | TIME_ST_ADC[3:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * INTVL_TIME: waiting time between completion of conversion
+ * and start of next conversion, only usable in
+ * autoscan mode (=20.480ms)
+ * TIME_ST_ADC: waiting time between application of voltage
+ * to panel and start of A/D conversion (=100us)
+ */
+#define BU21029_CFR2_REG (0x02 << 3)
+#define CFR2_VALUE 0xC9
+
+/*
+ * CFR3 Register (PAGE=0, ADDR=0x0B, Reset value=0x72)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | RM8 | STRETCH| PU90K | DUAL | PIDAC_OFS[3:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * RM8: 0 = coordinate resolution is 12bit (*)
+ * 1 = coordinate resolution is 8bit
+ * STRETCH: 0 = SCL_STRETCH function off
+ * 1 = SCL_STRETCH function on (*)
+ * PU90K: 0 = internal pull-up resistance for touch detection is ~50kohms (*)
+ * 1 = internal pull-up resistance for touch detection is ~90kohms
+ * DUAL: 0 = dual touch detection off (*)
+ * 1 = dual touch detection on
+ * PIDAC_OFS: dual touch detection circuit adjustment, it is not necessary
+ * to change this from initial value
+ */
+#define BU21029_CFR3_REG (0x0B << 3)
+#define CFR3_VALUE 0x42
+
+/*
+ * LDO Register (PAGE=0, ADDR=0x0C, Reset value=0x00)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | 0 | PVDD[2:0] | 0 | AVDD[2:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * PVDD: output voltage of panel output regulator (=2.000V)
+ * AVDD: output voltage of analog circuit regulator (=2.000V)
+ */
+#define BU21029_LDO_REG (0x0C << 3)
+#define LDO_VALUE 0x77
+
+/*
+ * Serial Interface Command Byte 1 (CID=1)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | 1 | CF | CMSK | PDM | STP |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * CF: conversion function, see table 3 in datasheet p6 (=0000, automatic scan)
+ * CMSK: 0 = executes convert function (*)
+ * 1 = reads the convert result
+ * PDM: 0 = power down after convert function stops (*)
+ * 1 = keep power on after convert function stops
+ * STP: 1 = abort current conversion and power down, set to "0" automatically
+ */
+#define BU21029_AUTOSCAN 0x80
+
+/*
+ * The timeout value needs to be larger than INTVL_TIME + tConv4 (sample and
+ * conversion time), where tConv4 is calculated by formula:
+ * tPON + tDLY1 + (tTIME_ST_ADC + (tADC * tSMPL) * 2 + tDLY2) * 3
+ * see figure 8 in datasheet p15 for details of each field.
+ */
+#define PEN_UP_TIMEOUT msecs_to_jiffies(50)
+
+#define STOP_DELAY_US 50L
+#define START_DELAY_MS 2L
+#define BUF_LEN 8L
+#define SCALE_12BIT (1 << 12)
+#define MAX_12BIT ((1 << 12) - 1)
+#define DRIVER_NAME "bu21029"
+
+struct bu21029_ts_data {
+ struct i2c_client *client;
+ struct input_dev *in_dev;
+ struct timer_list timer;
+ struct gpio_desc *reset_gpios;
+ u32 x_plate_ohms;
+ struct touchscreen_properties prop;
+};
+
+static int bu21029_touch_report(struct bu21029_ts_data *bu21029)
+{
+ struct i2c_client *i2c = bu21029->client;
+ u8 buf[BUF_LEN];
+ u16 x, y, z1, z2;
+ u32 rz;
+ s32 max_pressure = bu21029->in_dev->absinfo[ABS_PRESSURE].maximum;
+
+ /* read touch data and deassert INT (by restarting the autoscan mode) */
+ int error = i2c_smbus_read_i2c_block_data(i2c,
+ BU21029_AUTOSCAN,
+ BUF_LEN,
+ buf);
+ if (error < 0)
+ return error;
+
+ /*
+ * compose upper 8 and lower 4 bits into a 12bit value:
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ * | ByteH | ByteL |
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ * |b07|b06|b05|b04|b03|b02|b01|b00|b07|b06|b05|b04|b03|b02|b01|b00|
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ * |v11|v10|v09|v08|v07|v06|v05|v04|v03|v02|v01|v00| 0 | 0 | 0 | 0 |
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ */
+ x = (buf[0] << 4) | (buf[1] >> 4);
+ y = (buf[2] << 4) | (buf[3] >> 4);
+ z1 = (buf[4] << 4) | (buf[5] >> 4);
+ z2 = (buf[6] << 4) | (buf[7] >> 4);
+
+ if (z1 == 0 || z2 == 0)
+ return 0;
+
+ /*
+ * calculate Rz (pressure resistance value) by equation:
+ * Rz = Rx * (x/Q) * ((z2/z1) - 1), where
+ * Rx is x-plate resistance,
+ * Q is the touch screen resolution (8bit = 256, 12bit = 4096)
+ * x, z1, z2 are the measured positions.
+ */
+ rz = z2 - z1;
+ rz *= x;
+ rz *= bu21029->x_plate_ohms;
+ rz /= z1;
+ rz = DIV_ROUND_CLOSEST(rz, SCALE_12BIT);
+ if (rz <= max_pressure) {
+ touchscreen_report_pos(bu21029->in_dev, &bu21029->prop,
+ x, y, false);
+ input_report_abs(bu21029->in_dev, ABS_PRESSURE,
+ max_pressure - rz);
+ input_report_key(bu21029->in_dev, BTN_TOUCH, 1);
+ input_sync(bu21029->in_dev);
+ }
+
+ return 0;
+}
+
+static void bu21029_touch_release(struct timer_list *t)
+{
+ struct bu21029_ts_data *bu21029 = from_timer(bu21029, t, timer);
+
+ input_report_abs(bu21029->in_dev, ABS_PRESSURE, 0);
+ input_report_key(bu21029->in_dev, BTN_TOUCH, 0);
+ input_sync(bu21029->in_dev);
+}
+
+static irqreturn_t bu21029_touch_soft_irq(int irq, void *data)
+{
+ struct bu21029_ts_data *bu21029 = data;
+ struct i2c_client *i2c = bu21029->client;
+
+ /*
+ * report touch and deassert interrupt (will assert again after
+ * INTVL_TIME + tConv4 for continuous touch)
+ */
+ int error = bu21029_touch_report(bu21029);
+
+ if (error) {
+ dev_err(&i2c->dev, "failed to report (error: %d)\n", error);
+ return IRQ_NONE;
+ }
+
+ /* reset timer for pen up detection */
+ mod_timer(&bu21029->timer, jiffies + PEN_UP_TIMEOUT);
+
+ return IRQ_HANDLED;
+}
+
+static void bu21029_stop_chip(struct input_dev *dev)
+{
+ struct bu21029_ts_data *bu21029 = input_get_drvdata(dev);
+
+ disable_irq(bu21029->client->irq);
+ del_timer_sync(&bu21029->timer);
+
+ /* put chip into reset */
+ gpiod_set_value_cansleep(bu21029->reset_gpios, 1);
+ udelay(STOP_DELAY_US);
+}
+
+static int bu21029_start_chip(struct input_dev *dev)
+{
+ struct bu21029_ts_data *bu21029 = input_get_drvdata(dev);
+ struct i2c_client *i2c = bu21029->client;
+ struct {
+ u8 reg;
+ u8 value;
+ } init_table[] = {
+ {BU21029_CFR0_REG, CFR0_VALUE},
+ {BU21029_CFR1_REG, CFR1_VALUE},
+ {BU21029_CFR2_REG, CFR2_VALUE},
+ {BU21029_CFR3_REG, CFR3_VALUE},
+ {BU21029_LDO_REG, LDO_VALUE}
+ };
+ int error, i;
+ u16 hwid;
+
+ /* take chip out of reset */
+ gpiod_set_value_cansleep(bu21029->reset_gpios, 0);
+ mdelay(START_DELAY_MS);
+
+ error = i2c_smbus_read_i2c_block_data(i2c,
+ BU21029_HWID_REG,
+ 2,
+ (u8 *)&hwid);
+ if (error < 0) {
+ dev_err(&i2c->dev, "failed to read HW ID\n");
+ goto out;
+ }
+
+ if (cpu_to_be16(hwid) != SUPPORTED_HWID) {
+ dev_err(&i2c->dev, "unsupported HW ID 0x%x\n", hwid);
+ error = -ENODEV;
+ goto out;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(init_table); ++i) {
+ error = i2c_smbus_write_byte_data(i2c,
+ init_table[i].reg,
+ init_table[i].value);
+ if (error < 0) {
+ dev_err(&i2c->dev,
+ "failed to write 0x%x to register 0x%x\n",
+ init_table[i].value,
+ init_table[i].reg);
+ goto out;
+ }
+ }
+
+ error = i2c_smbus_write_byte(i2c, BU21029_AUTOSCAN);
+ if (error < 0) {
+ dev_err(&i2c->dev, "failed to start autoscan\n");
+ goto out;
+ }
+
+ enable_irq(bu21029->client->irq);
+ return 0;
+
+out:
+ bu21029_stop_chip(dev);
+ return error;
+}
+
+static int bu21029_parse_dt(struct bu21029_ts_data *bu21029)
+{
+ struct device *dev = &bu21029->client->dev;
+ struct device_node *np = dev->of_node;
+ u32 val32;
+ int error;
+
+ if (!np) {
+ dev_err(dev, "no device tree data\n");
+ return -EINVAL;
+ }
+
+ bu21029->reset_gpios = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(bu21029->reset_gpios)) {
+ error = PTR_ERR(bu21029->reset_gpios);
+ if (error != -EPROBE_DEFER)
+ dev_err(dev, "invalid 'reset-gpios':%d\n", error);
+ return error;
+ }
+
+ if (of_property_read_u32(np, "rohm,x-plate-ohms", &val32)) {
+ dev_err(dev, "invalid 'x-plate-ohms' supplied\n");
+ return -EINVAL;
+ }
+ bu21029->x_plate_ohms = val32;
+
+ touchscreen_parse_properties(bu21029->in_dev, false, &bu21029->prop);
+
+ return 0;
+}
+
+static int bu21029_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct bu21029_ts_data *bu21029;
+ struct input_dev *in_dev;
+ int error;
+
+ if (!i2c_check_functionality(client->adapter,
+ I2C_FUNC_SMBUS_WRITE_BYTE |
+ I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
+ I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
+ dev_err(&client->dev,
+ "i2c functionality support is not sufficient\n");
+ return -EIO;
+ }
+
+ bu21029 = devm_kzalloc(&client->dev, sizeof(*bu21029), GFP_KERNEL);
+ if (!bu21029)
+ return -ENOMEM;
+
+ in_dev = devm_input_allocate_device(&client->dev);
+ if (!in_dev) {
+ dev_err(&client->dev, "unable to allocate input device\n");
+ return -ENOMEM;
+ }
+
+ bu21029->client = client;
+ bu21029->in_dev = in_dev;
+ timer_setup(&bu21029->timer, bu21029_touch_release, 0);
+
+ in_dev->name = DRIVER_NAME;
+ in_dev->id.bustype = BUS_I2C;
+ in_dev->open = bu21029_start_chip;
+ in_dev->close = bu21029_stop_chip;
+
+ input_set_capability(in_dev, EV_KEY, BTN_TOUCH);
+ input_set_abs_params(in_dev, ABS_X, 0, MAX_12BIT, 0, 0);
+ input_set_abs_params(in_dev, ABS_Y, 0, MAX_12BIT, 0, 0);
+ input_set_abs_params(in_dev, ABS_PRESSURE, 0, MAX_12BIT, 0, 0);
+
+ error = bu21029_parse_dt(bu21029);
+ if (error)
+ return error;
+
+ input_set_drvdata(in_dev, bu21029);
+
+ error = devm_request_threaded_irq(&client->dev,
+ client->irq,
+ NULL,
+ bu21029_touch_soft_irq,
+ IRQF_ONESHOT,
+ DRIVER_NAME,
+ bu21029);
+ if (error) {
+ dev_err(&client->dev, "unable to request touch irq\n");
+ return error;
+ }
+
+ bu21029_stop_chip(in_dev);
+
+ error = input_register_device(in_dev);
+ if (error) {
+ dev_err(&client->dev, "unable to register input device\n");
+ return error;
+ }
+
+ i2c_set_clientdata(client, bu21029);
+
+ return 0;
+}
+
+static int bu21029_remove(struct i2c_client *client)
+{
+ struct bu21029_ts_data *bu21029 = i2c_get_clientdata(client);
+
+ bu21029_stop_chip(bu21029->in_dev);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int bu21029_suspend(struct device *dev)
+{
+ struct i2c_client *i2c = to_i2c_client(dev);
+ struct bu21029_ts_data *bu21029 = i2c_get_clientdata(i2c);
+
+ mutex_lock(&bu21029->in_dev->mutex);
+ if (bu21029->in_dev->users)
+ bu21029_stop_chip(bu21029->in_dev);
+ mutex_unlock(&bu21029->in_dev->mutex);
+
+ return 0;
+}
+
+static int bu21029_resume(struct device *dev)
+{
+ struct i2c_client *i2c = to_i2c_client(dev);
+ struct bu21029_ts_data *bu21029 = i2c_get_clientdata(i2c);
+
+ mutex_lock(&bu21029->in_dev->mutex);
+ if (bu21029->in_dev->users)
+ bu21029_start_chip(bu21029->in_dev);
+ mutex_unlock(&bu21029->in_dev->mutex);
+
+ return 0;
+}
+#endif
+static SIMPLE_DEV_PM_OPS(bu21029_pm_ops, bu21029_suspend, bu21029_resume);
+
+static const struct i2c_device_id bu21029_ids[] = {
+ {DRIVER_NAME, 0},
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, bu21029_ids);
+
+static struct i2c_driver bu21029_driver = {
+ .driver = {
+ .name = DRIVER_NAME,
+ .pm = &bu21029_pm_ops,
+ },
+ .id_table = bu21029_ids,
+ .probe = bu21029_probe,
+ .remove = bu21029_remove,
+};
+module_i2c_driver(bu21029_driver);
+
+MODULE_AUTHOR("Zhu Yi <[email protected]>");
+MODULE_DESCRIPTION("Rohm BU21029 touchscreen controller driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4


2018-04-16 21:46:16

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2] Input: add bu21029 touch driver

On Mon, Apr 16, 2018 at 06:49:38PM +0200, Mark Jonas wrote:
> From: Zhu Yi <[email protected]>
>
> Add Rohm BU21029 resistive touch panel controller support with I2C
> interface.
>
> Signed-off-by: Zhu Yi <[email protected]>
> Signed-off-by: Mark Jonas <[email protected]>
> Reviewed-by: Heiko Schocher <[email protected]>
> ---
> Changes in v2:
> - make ABS_PRESSURE proportionally rising with finger pressure
> - fix race between interrupt and timer during shutdown
> - use infrastructure from include/linux/input/touchscreen.h
> - add SPDX tag for the driver
> - improve binding documentation
> - fix multi-line comments
> ---
> .../bindings/input/touchscreen/bu21029.txt | 34 ++

Reviewed-by: Rob Herring <[email protected]>

> drivers/input/touchscreen/Kconfig | 12 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/bu21029_ts.c | 485 +++++++++++++++++++++
> 4 files changed, 532 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
> create mode 100644 drivers/input/touchscreen/bu21029_ts.c

Subject: AW: [PATCH v2] Input: add bu21029 touch driver

Hello,

> Betreff: [PATCH v2] Input: add bu21029 touch driver
>
> From: Zhu Yi <[email protected]>
>
> Add Rohm BU21029 resistive touch panel controller support with I2C
> interface.
>
> Signed-off-by: Zhu Yi <[email protected]>
> Signed-off-by: Mark Jonas <[email protected]>
> Reviewed-by: Heiko Schocher <[email protected]>
> ---
> Changes in v2:
> - make ABS_PRESSURE proportionally rising with finger pressure
> - fix race between interrupt and timer during shutdown
> - use infrastructure from include/linux/input/touchscreen.h
> - add SPDX tag for the driver
> - improve binding documentation
> - fix multi-line comments

In the mean time I received a Reviewed-by from Rob Herring.

How shall I proceed? I am convinced that we did all the requested
changes and published them 2018-04-16 in the cited patch above.

Greetings,
Mark Jonas

Building Technologies, Panel Software Fire (BT-FIR/ENG1)
Bosch Sicherheitssysteme GmbH | Postfach 11 11 | 85626 Grasbrunn | GERMANY | http://www.boschsecurity.com

Sitz: Stuttgart, Registergericht: Amtsgericht Stuttgart HRB 23118
Aufsichtsratsvorsitzender: Stefan Hartung; Gesch?ftsf?hrung: Gert van Iperen, Andreas Bartz, Thomas Quante, Bernhard Schuster

Subject: [PATCH v3] Input: add bu21029 touch driver

From: Zhu Yi <[email protected]>

Add Rohm BU21029 resistive touch panel controller support with I2C
interface.

Signed-off-by: Zhu Yi <[email protected]>
Signed-off-by: Mark Jonas <[email protected]>
Reviewed-by: Heiko Schocher <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
Changes in v3:
- reviewed by Rob Herring
---
Changes in v2:
- make ABS_PRESSURE proportionally rising with finger pressure
- fix race between interrupt and timer during shutdown
- use infrastructure from include/linux/input/touchscreen.h
- add SPDX tag for the driver
- improve binding documentation
- fix multi-line comments
---
.../bindings/input/touchscreen/bu21029.txt | 34 ++
drivers/input/touchscreen/Kconfig | 12 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/bu21029_ts.c | 485 +++++++++++++++++++++
4 files changed, 532 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
create mode 100644 drivers/input/touchscreen/bu21029_ts.c

diff --git a/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
new file mode 100644
index 0000000..030a888
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
@@ -0,0 +1,34 @@
+* Rohm BU21029 Touch Screen Controller
+
+Required properties:
+ - compatible : must be "rohm,bu21029"
+ - reg : i2c device address of the chip (0x40 or 0x41)
+ - interrupt-parent : the phandle for the gpio controller
+ - interrupts : (gpio) interrupt to which the chip is connected
+ - reset-gpios : gpio pin to reset the chip (active low)
+ - rohm,x-plate-ohms : x-plate resistance in Ohm
+
+Optional properties:
+ - touchscreen-size-x : horizontal resolution of touchscreen (in pixels)
+ - touchscreen-size-y : vertical resolution of touchscreen (in pixels)
+ - touchscreen-max-pressure: maximum pressure value
+
+Example:
+
+ &i2c1 {
+ /* ... */
+
+ bu21029: bu21029@40 {
+ compatible = "rohm,bu21029";
+ reg = <0x40>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
+ reset-gpios = <&gpio6 16 GPIO_ACTIVE_LOW>;
+ rohm,x-plate-ohms = <600>;
+ touchscreen-size-x = <800>;
+ touchscreen-size-y = <480>;
+ touchscreen-max-pressure = <4095>;
+ };
+
+ /* ... */
+ };
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 4f15496..e09fe8f 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -151,6 +151,18 @@ config TOUCHSCREEN_BU21013
To compile this driver as a module, choose M here: the
module will be called bu21013_ts.

+config TOUCHSCREEN_BU21029
+ tristate "Rohm BU21029 based touch panel controllers"
+ depends on I2C
+ help
+ Say Y here if you have a Rohm BU21029 touchscreen controller
+ connected to your system.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called bu21029_ts.
+
config TOUCHSCREEN_CHIPONE_ICN8318
tristate "chipone icn8318 touchscreen controller"
depends on GPIOLIB || COMPILE_TEST
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index dddae79..f50624c 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_TOUCHSCREEN_AR1021_I2C) += ar1021_i2c.o
obj-$(CONFIG_TOUCHSCREEN_ATMEL_MXT) += atmel_mxt_ts.o
obj-$(CONFIG_TOUCHSCREEN_AUO_PIXCIR) += auo-pixcir-ts.o
obj-$(CONFIG_TOUCHSCREEN_BU21013) += bu21013_ts.o
+obj-$(CONFIG_TOUCHSCREEN_BU21029) += bu21029_ts.o
obj-$(CONFIG_TOUCHSCREEN_CHIPONE_ICN8318) += chipone_icn8318.o
obj-$(CONFIG_TOUCHSCREEN_CY8CTMG110) += cy8ctmg110_ts.o
obj-$(CONFIG_TOUCHSCREEN_CYTTSP_CORE) += cyttsp_core.o
diff --git a/drivers/input/touchscreen/bu21029_ts.c b/drivers/input/touchscreen/bu21029_ts.c
new file mode 100644
index 0000000..8d0cbe51
--- /dev/null
+++ b/drivers/input/touchscreen/bu21029_ts.c
@@ -0,0 +1,485 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Rohm BU21029 touchscreen controller driver
+ *
+ * Copyright (C) 2015-2018 Bosch Sicherheitssysteme GmbH
+ *
+ * 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/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/touchscreen.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/timer.h>
+
+/*
+ * HW_ID1 Register (PAGE=0, ADDR=0x0E, Reset value=0x02, Read only)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | HW_IDH |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * HW_ID2 Register (PAGE=0, ADDR=0x0F, Reset value=0x29, Read only)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | HW_IDL |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * HW_IDH: high 8bits of IC's ID
+ * HW_IDL: low 8bits of IC's ID
+ */
+#define BU21029_HWID_REG (0x0E << 3)
+#define SUPPORTED_HWID 0x0229
+
+/*
+ * CFR0 Register (PAGE=0, ADDR=0x00, Reset value=0x20)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | 0 | 0 | CALIB | INTRM | 0 | 0 | 0 | 0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * CALIB: 0 = not to use calibration result (*)
+ * 1 = use calibration result
+ * INTRM: 0 = INT output depend on "pen down" (*)
+ * 1 = INT output always "0"
+ */
+#define BU21029_CFR0_REG (0x00 << 3)
+#define CFR0_VALUE 0x00
+
+/*
+ * CFR1 Register (PAGE=0, ADDR=0x01, Reset value=0xA6)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | MAV | AVE[2:0] | 0 | SMPL[2:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * MAV: 0 = median average filter off
+ * 1 = median average filter on (*)
+ * AVE: AVE+1 = number of average samples for MAV,
+ * if AVE>SMPL, then AVE=SMPL (=3)
+ * SMPL: SMPL+1 = number of conversion samples for MAV (=7)
+ */
+#define BU21029_CFR1_REG (0x01 << 3)
+#define CFR1_VALUE 0xA6
+
+/*
+ * CFR2 Register (PAGE=0, ADDR=0x02, Reset value=0x04)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | INTVL_TIME[3:0] | TIME_ST_ADC[3:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * INTVL_TIME: waiting time between completion of conversion
+ * and start of next conversion, only usable in
+ * autoscan mode (=20.480ms)
+ * TIME_ST_ADC: waiting time between application of voltage
+ * to panel and start of A/D conversion (=100us)
+ */
+#define BU21029_CFR2_REG (0x02 << 3)
+#define CFR2_VALUE 0xC9
+
+/*
+ * CFR3 Register (PAGE=0, ADDR=0x0B, Reset value=0x72)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | RM8 | STRETCH| PU90K | DUAL | PIDAC_OFS[3:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * RM8: 0 = coordinate resolution is 12bit (*)
+ * 1 = coordinate resolution is 8bit
+ * STRETCH: 0 = SCL_STRETCH function off
+ * 1 = SCL_STRETCH function on (*)
+ * PU90K: 0 = internal pull-up resistance for touch detection is ~50kohms (*)
+ * 1 = internal pull-up resistance for touch detection is ~90kohms
+ * DUAL: 0 = dual touch detection off (*)
+ * 1 = dual touch detection on
+ * PIDAC_OFS: dual touch detection circuit adjustment, it is not necessary
+ * to change this from initial value
+ */
+#define BU21029_CFR3_REG (0x0B << 3)
+#define CFR3_VALUE 0x42
+
+/*
+ * LDO Register (PAGE=0, ADDR=0x0C, Reset value=0x00)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | 0 | PVDD[2:0] | 0 | AVDD[2:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * PVDD: output voltage of panel output regulator (=2.000V)
+ * AVDD: output voltage of analog circuit regulator (=2.000V)
+ */
+#define BU21029_LDO_REG (0x0C << 3)
+#define LDO_VALUE 0x77
+
+/*
+ * Serial Interface Command Byte 1 (CID=1)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | 1 | CF | CMSK | PDM | STP |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * CF: conversion function, see table 3 in datasheet p6 (=0000, automatic scan)
+ * CMSK: 0 = executes convert function (*)
+ * 1 = reads the convert result
+ * PDM: 0 = power down after convert function stops (*)
+ * 1 = keep power on after convert function stops
+ * STP: 1 = abort current conversion and power down, set to "0" automatically
+ */
+#define BU21029_AUTOSCAN 0x80
+
+/*
+ * The timeout value needs to be larger than INTVL_TIME + tConv4 (sample and
+ * conversion time), where tConv4 is calculated by formula:
+ * tPON + tDLY1 + (tTIME_ST_ADC + (tADC * tSMPL) * 2 + tDLY2) * 3
+ * see figure 8 in datasheet p15 for details of each field.
+ */
+#define PEN_UP_TIMEOUT msecs_to_jiffies(50)
+
+#define STOP_DELAY_US 50L
+#define START_DELAY_MS 2L
+#define BUF_LEN 8L
+#define SCALE_12BIT (1 << 12)
+#define MAX_12BIT ((1 << 12) - 1)
+#define DRIVER_NAME "bu21029"
+
+struct bu21029_ts_data {
+ struct i2c_client *client;
+ struct input_dev *in_dev;
+ struct timer_list timer;
+ struct gpio_desc *reset_gpios;
+ u32 x_plate_ohms;
+ struct touchscreen_properties prop;
+};
+
+static int bu21029_touch_report(struct bu21029_ts_data *bu21029)
+{
+ struct i2c_client *i2c = bu21029->client;
+ u8 buf[BUF_LEN];
+ u16 x, y, z1, z2;
+ u32 rz;
+ s32 max_pressure = bu21029->in_dev->absinfo[ABS_PRESSURE].maximum;
+
+ /* read touch data and deassert INT (by restarting the autoscan mode) */
+ int error = i2c_smbus_read_i2c_block_data(i2c,
+ BU21029_AUTOSCAN,
+ BUF_LEN,
+ buf);
+ if (error < 0)
+ return error;
+
+ /*
+ * compose upper 8 and lower 4 bits into a 12bit value:
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ * | ByteH | ByteL |
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ * |b07|b06|b05|b04|b03|b02|b01|b00|b07|b06|b05|b04|b03|b02|b01|b00|
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ * |v11|v10|v09|v08|v07|v06|v05|v04|v03|v02|v01|v00| 0 | 0 | 0 | 0 |
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ */
+ x = (buf[0] << 4) | (buf[1] >> 4);
+ y = (buf[2] << 4) | (buf[3] >> 4);
+ z1 = (buf[4] << 4) | (buf[5] >> 4);
+ z2 = (buf[6] << 4) | (buf[7] >> 4);
+
+ if (z1 == 0 || z2 == 0)
+ return 0;
+
+ /*
+ * calculate Rz (pressure resistance value) by equation:
+ * Rz = Rx * (x/Q) * ((z2/z1) - 1), where
+ * Rx is x-plate resistance,
+ * Q is the touch screen resolution (8bit = 256, 12bit = 4096)
+ * x, z1, z2 are the measured positions.
+ */
+ rz = z2 - z1;
+ rz *= x;
+ rz *= bu21029->x_plate_ohms;
+ rz /= z1;
+ rz = DIV_ROUND_CLOSEST(rz, SCALE_12BIT);
+ if (rz <= max_pressure) {
+ touchscreen_report_pos(bu21029->in_dev, &bu21029->prop,
+ x, y, false);
+ input_report_abs(bu21029->in_dev, ABS_PRESSURE,
+ max_pressure - rz);
+ input_report_key(bu21029->in_dev, BTN_TOUCH, 1);
+ input_sync(bu21029->in_dev);
+ }
+
+ return 0;
+}
+
+static void bu21029_touch_release(struct timer_list *t)
+{
+ struct bu21029_ts_data *bu21029 = from_timer(bu21029, t, timer);
+
+ input_report_abs(bu21029->in_dev, ABS_PRESSURE, 0);
+ input_report_key(bu21029->in_dev, BTN_TOUCH, 0);
+ input_sync(bu21029->in_dev);
+}
+
+static irqreturn_t bu21029_touch_soft_irq(int irq, void *data)
+{
+ struct bu21029_ts_data *bu21029 = data;
+ struct i2c_client *i2c = bu21029->client;
+
+ /*
+ * report touch and deassert interrupt (will assert again after
+ * INTVL_TIME + tConv4 for continuous touch)
+ */
+ int error = bu21029_touch_report(bu21029);
+
+ if (error) {
+ dev_err(&i2c->dev, "failed to report (error: %d)\n", error);
+ return IRQ_NONE;
+ }
+
+ /* reset timer for pen up detection */
+ mod_timer(&bu21029->timer, jiffies + PEN_UP_TIMEOUT);
+
+ return IRQ_HANDLED;
+}
+
+static void bu21029_stop_chip(struct input_dev *dev)
+{
+ struct bu21029_ts_data *bu21029 = input_get_drvdata(dev);
+
+ disable_irq(bu21029->client->irq);
+ del_timer_sync(&bu21029->timer);
+
+ /* put chip into reset */
+ gpiod_set_value_cansleep(bu21029->reset_gpios, 1);
+ udelay(STOP_DELAY_US);
+}
+
+static int bu21029_start_chip(struct input_dev *dev)
+{
+ struct bu21029_ts_data *bu21029 = input_get_drvdata(dev);
+ struct i2c_client *i2c = bu21029->client;
+ struct {
+ u8 reg;
+ u8 value;
+ } init_table[] = {
+ {BU21029_CFR0_REG, CFR0_VALUE},
+ {BU21029_CFR1_REG, CFR1_VALUE},
+ {BU21029_CFR2_REG, CFR2_VALUE},
+ {BU21029_CFR3_REG, CFR3_VALUE},
+ {BU21029_LDO_REG, LDO_VALUE}
+ };
+ int error, i;
+ u16 hwid;
+
+ /* take chip out of reset */
+ gpiod_set_value_cansleep(bu21029->reset_gpios, 0);
+ mdelay(START_DELAY_MS);
+
+ error = i2c_smbus_read_i2c_block_data(i2c,
+ BU21029_HWID_REG,
+ 2,
+ (u8 *)&hwid);
+ if (error < 0) {
+ dev_err(&i2c->dev, "failed to read HW ID\n");
+ goto out;
+ }
+
+ if (cpu_to_be16(hwid) != SUPPORTED_HWID) {
+ dev_err(&i2c->dev, "unsupported HW ID 0x%x\n", hwid);
+ error = -ENODEV;
+ goto out;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(init_table); ++i) {
+ error = i2c_smbus_write_byte_data(i2c,
+ init_table[i].reg,
+ init_table[i].value);
+ if (error < 0) {
+ dev_err(&i2c->dev,
+ "failed to write 0x%x to register 0x%x\n",
+ init_table[i].value,
+ init_table[i].reg);
+ goto out;
+ }
+ }
+
+ error = i2c_smbus_write_byte(i2c, BU21029_AUTOSCAN);
+ if (error < 0) {
+ dev_err(&i2c->dev, "failed to start autoscan\n");
+ goto out;
+ }
+
+ enable_irq(bu21029->client->irq);
+ return 0;
+
+out:
+ bu21029_stop_chip(dev);
+ return error;
+}
+
+static int bu21029_parse_dt(struct bu21029_ts_data *bu21029)
+{
+ struct device *dev = &bu21029->client->dev;
+ struct device_node *np = dev->of_node;
+ u32 val32;
+ int error;
+
+ if (!np) {
+ dev_err(dev, "no device tree data\n");
+ return -EINVAL;
+ }
+
+ bu21029->reset_gpios = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(bu21029->reset_gpios)) {
+ error = PTR_ERR(bu21029->reset_gpios);
+ if (error != -EPROBE_DEFER)
+ dev_err(dev, "invalid 'reset-gpios':%d\n", error);
+ return error;
+ }
+
+ if (of_property_read_u32(np, "rohm,x-plate-ohms", &val32)) {
+ dev_err(dev, "invalid 'x-plate-ohms' supplied\n");
+ return -EINVAL;
+ }
+ bu21029->x_plate_ohms = val32;
+
+ touchscreen_parse_properties(bu21029->in_dev, false, &bu21029->prop);
+
+ return 0;
+}
+
+static int bu21029_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct bu21029_ts_data *bu21029;
+ struct input_dev *in_dev;
+ int error;
+
+ if (!i2c_check_functionality(client->adapter,
+ I2C_FUNC_SMBUS_WRITE_BYTE |
+ I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
+ I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
+ dev_err(&client->dev,
+ "i2c functionality support is not sufficient\n");
+ return -EIO;
+ }
+
+ bu21029 = devm_kzalloc(&client->dev, sizeof(*bu21029), GFP_KERNEL);
+ if (!bu21029)
+ return -ENOMEM;
+
+ in_dev = devm_input_allocate_device(&client->dev);
+ if (!in_dev) {
+ dev_err(&client->dev, "unable to allocate input device\n");
+ return -ENOMEM;
+ }
+
+ bu21029->client = client;
+ bu21029->in_dev = in_dev;
+ timer_setup(&bu21029->timer, bu21029_touch_release, 0);
+
+ in_dev->name = DRIVER_NAME;
+ in_dev->id.bustype = BUS_I2C;
+ in_dev->open = bu21029_start_chip;
+ in_dev->close = bu21029_stop_chip;
+
+ input_set_capability(in_dev, EV_KEY, BTN_TOUCH);
+ input_set_abs_params(in_dev, ABS_X, 0, MAX_12BIT, 0, 0);
+ input_set_abs_params(in_dev, ABS_Y, 0, MAX_12BIT, 0, 0);
+ input_set_abs_params(in_dev, ABS_PRESSURE, 0, MAX_12BIT, 0, 0);
+
+ error = bu21029_parse_dt(bu21029);
+ if (error)
+ return error;
+
+ input_set_drvdata(in_dev, bu21029);
+
+ error = devm_request_threaded_irq(&client->dev,
+ client->irq,
+ NULL,
+ bu21029_touch_soft_irq,
+ IRQF_ONESHOT,
+ DRIVER_NAME,
+ bu21029);
+ if (error) {
+ dev_err(&client->dev, "unable to request touch irq\n");
+ return error;
+ }
+
+ bu21029_stop_chip(in_dev);
+
+ error = input_register_device(in_dev);
+ if (error) {
+ dev_err(&client->dev, "unable to register input device\n");
+ return error;
+ }
+
+ i2c_set_clientdata(client, bu21029);
+
+ return 0;
+}
+
+static int bu21029_remove(struct i2c_client *client)
+{
+ struct bu21029_ts_data *bu21029 = i2c_get_clientdata(client);
+
+ bu21029_stop_chip(bu21029->in_dev);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int bu21029_suspend(struct device *dev)
+{
+ struct i2c_client *i2c = to_i2c_client(dev);
+ struct bu21029_ts_data *bu21029 = i2c_get_clientdata(i2c);
+
+ mutex_lock(&bu21029->in_dev->mutex);
+ if (bu21029->in_dev->users)
+ bu21029_stop_chip(bu21029->in_dev);
+ mutex_unlock(&bu21029->in_dev->mutex);
+
+ return 0;
+}
+
+static int bu21029_resume(struct device *dev)
+{
+ struct i2c_client *i2c = to_i2c_client(dev);
+ struct bu21029_ts_data *bu21029 = i2c_get_clientdata(i2c);
+
+ mutex_lock(&bu21029->in_dev->mutex);
+ if (bu21029->in_dev->users)
+ bu21029_start_chip(bu21029->in_dev);
+ mutex_unlock(&bu21029->in_dev->mutex);
+
+ return 0;
+}
+#endif
+static SIMPLE_DEV_PM_OPS(bu21029_pm_ops, bu21029_suspend, bu21029_resume);
+
+static const struct i2c_device_id bu21029_ids[] = {
+ {DRIVER_NAME, 0},
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, bu21029_ids);
+
+static struct i2c_driver bu21029_driver = {
+ .driver = {
+ .name = DRIVER_NAME,
+ .pm = &bu21029_pm_ops,
+ },
+ .id_table = bu21029_ids,
+ .probe = bu21029_probe,
+ .remove = bu21029_remove,
+};
+module_i2c_driver(bu21029_driver);
+
+MODULE_AUTHOR("Zhu Yi <[email protected]>");
+MODULE_DESCRIPTION("Rohm BU21029 touchscreen controller driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4


2018-05-13 14:57:37

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3] Input: add bu21029 touch driver

On Fri, May 11, 2018 at 5:22 PM, Mark Jonas <[email protected]> wrote:

> Add Rohm BU21029 resistive touch panel controller support with I2C
> interface.

> +#include <linux/of.h>

This becomes redundant (see below).

> +#define STOP_DELAY_US 50L
> +#define START_DELAY_MS 2L
> +#define BUF_LEN 8L

No need to use L for such small numbers. Integer promotion is a part
of C standard.

> +#define SCALE_12BIT (1 << 12)
> +#define MAX_12BIT ((1 << 12) - 1)

BIT(12)
GENMASK(11, 0)

> +static int bu21029_touch_report(struct bu21029_ts_data *bu21029)
> +{
> + struct i2c_client *i2c = bu21029->client;
> + u8 buf[BUF_LEN];
> + int error = bu21029_touch_report(bu21029);

> +

Redundant empty line.

> + if (error) {

> + dev_err(&i2c->dev, "failed to report (error: %d)\n", error);

Potential spamming case.

> + return IRQ_NONE;
> + }

> +static void bu21029_stop_chip(struct input_dev *dev)
> +{
> + struct bu21029_ts_data *bu21029 = input_get_drvdata(dev);
> +
> + disable_irq(bu21029->client->irq);
> + del_timer_sync(&bu21029->timer);
> +
> + /* put chip into reset */
> + gpiod_set_value_cansleep(bu21029->reset_gpios, 1);

> + udelay(STOP_DELAY_US);

udelay() ?!

> +}
> +

> +static int bu21029_start_chip(struct input_dev *dev)
> +{

> + u16 hwid;
> +
> + /* take chip out of reset */
> + gpiod_set_value_cansleep(bu21029->reset_gpios, 0);

> + mdelay(START_DELAY_MS);

mdelay()?!

> +
> + error = i2c_smbus_read_i2c_block_data(i2c,
> + BU21029_HWID_REG,
> + 2,
> + (u8 *)&hwid);
> + if (error < 0) {
> + dev_err(&i2c->dev, "failed to read HW ID\n");
> + goto out;
> + }
> +

> + if (cpu_to_be16(hwid) != SUPPORTED_HWID) {

Hmm... Why cpu_to_be16() is required?

> + dev_err(&i2c->dev, "unsupported HW ID 0x%x\n", hwid);
> + error = -ENODEV;
> + goto out;
> + }
> +}

> +static int bu21029_parse_dt(struct bu21029_ts_data *bu21029)

You can get rid of DT requirement by...

> +{
> + struct device *dev = &bu21029->client->dev;
> + struct device_node *np = dev->of_node;
> + u32 val32;
> + int error;

> + if (!np) {
> + dev_err(dev, "no device tree data\n");
> + return -EINVAL;
> + }

(this becomes redundant)

> +
> + bu21029->reset_gpios = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(bu21029->reset_gpios)) {
> + error = PTR_ERR(bu21029->reset_gpios);
> + if (error != -EPROBE_DEFER)
> + dev_err(dev, "invalid 'reset-gpios':%d\n", error);
> + return error;
> + }
> +

> + if (of_property_read_u32(np, "rohm,x-plate-ohms", &val32)) {

...simple calling device_property_read_u32() instead.

> + dev_err(dev, "invalid 'x-plate-ohms' supplied\n");
> + return -EINVAL;
> + }
> + bu21029->x_plate_ohms = val32;
> +
> + touchscreen_parse_properties(bu21029->in_dev, false, &bu21029->prop);
> +
> + return 0;
> +}

> +#ifdef CONFIG_PM_SLEEP

Instead...

> +static int bu21029_suspend(struct device *dev)

...use __maby_unused annotation.

> +static int bu21029_resume(struct device *dev)

Ditto.

--
With Best Regards,
Andy Shevchenko

Subject: AW: [PATCH v3] Input: add bu21029 touch driver

Hello Andy,

> > Add Rohm BU21029 resistive touch panel controller support with I2C
> > interface.
>
> > +#include <linux/of.h>
>
> This becomes redundant (see below).

Removed.

> > +#define STOP_DELAY_US 50L
> > +#define START_DELAY_MS 2L
> > +#define BUF_LEN 8L
>
> No need to use L for such small numbers. Integer promotion is a part
> of C standard.

OK.

> > +#define SCALE_12BIT (1 << 12)
> > +#define MAX_12BIT ((1 << 12) - 1)
>
> BIT(12)
> GENMASK(11, 0)

We are not convinced that we should use BIT() and GENMASK() here.

The reason is that SCALE_12BIT is actually not used as a bit but as an
input value for DIV_ROUND_CLOSEST. We think that the BIT() macro will
hide the meaning of the value.

MAX_12BIT is also a value and not a bit mask. Thus, we also think that
using the GENMASK() macro will hide its purpose. Also, the
documentation of GENMASK() says that it is a mask and not a value.

> > +static int bu21029_touch_report(struct bu21029_ts_data *bu21029)
> > +{
> > + struct i2c_client *i2c = bu21029->client;
> > + u8 buf[BUF_LEN];
> > + int error = bu21029_touch_report(bu21029);
>
> > +
>
> Redundant empty line.

Removed.

> > + if (error) {
>
> > + dev_err(&i2c->dev, "failed to report (error: %d)\n", error);
>
> Potential spamming case.
>
> > + return IRQ_NONE;
> > + }

You are right, we will remove the error message.

> > +static void bu21029_stop_chip(struct input_dev *dev)
> > +{
> > + struct bu21029_ts_data *bu21029 = input_get_drvdata(dev);
> > +
> > + disable_irq(bu21029->client->irq);
> > + del_timer_sync(&bu21029->timer);
> > +
> > + /* put chip into reset */
> > + gpiod_set_value_cansleep(bu21029->reset_gpios, 1);
>
> > + udelay(STOP_DELAY_US);
>
> udelay() ?!
>
> > +}

According to the datasheet disabling the chip will take 30 microseconds.
In the defines we added a buffer of 20 microseconds and thus
STOP_DELAY_US is 50. The function guarantees that the chip is stopped
before it returns.

We think that it is ok to use udelay() here because in normal operation
the chip is not stopped. It is only stopped when loading or unloading
the driver, or when the system suspends.

We would like to keep it like it is.

> > +static int bu21029_start_chip(struct input_dev *dev)
> > +{
>
> > + u16 hwid;
> > +
> > + /* take chip out of reset */
> > + gpiod_set_value_cansleep(bu21029->reset_gpios, 0);
>
> > + mdelay(START_DELAY_MS);
>
> mdelay()?!
>
> > +
> > + error = i2c_smbus_read_i2c_block_data(i2c,
> > + BU21029_HWID_REG,
> > + 2,
> > + (u8 *)&hwid);
> > + if (error < 0) {
> > + dev_err(&i2c->dev, "failed to read HW ID\n");
> > + goto out;
> > + }

After de-asserting the reset chip takes 1 millisecond until it is
operational. We added a 1 millisecond buffer to it. Thus,
START_DELAY_MS is 2.

The following I2C read will not succeed without waiting for the chip
being ready.

> > + if (cpu_to_be16(hwid) != SUPPORTED_HWID) {
>
> Hmm... Why cpu_to_be16() is required?
>
> > + dev_err(&i2c->dev, "unsupported HW ID 0x%x\n", hwid);
> > + error = -ENODEV;
> > + goto out;
> > + }
> > +}

You are right, it works but what we meant to do here is to convert the
chip's value (big endian) into the CPU endianness. We will change it to
be16_to_cpu().

> > +static int bu21029_parse_dt(struct bu21029_ts_data *bu21029)
>
> You can get rid of DT requirement by...
>
> > +{
> > + struct device *dev = &bu21029->client->dev;
> > + struct device_node *np = dev->of_node;
> > + u32 val32;
> > + int error;
>
> > + if (!np) {
> > + dev_err(dev, "no device tree data\n");
> > + return -EINVAL;
> > + }
>
> (this becomes redundant)
>
> > +
> > + bu21029->reset_gpios = devm_gpiod_get(dev, "reset",
> GPIOD_OUT_HIGH);
> > + if (IS_ERR(bu21029->reset_gpios)) {
> > + error = PTR_ERR(bu21029->reset_gpios);
> > + if (error != -EPROBE_DEFER)
> > + dev_err(dev, "invalid 'reset-gpios':%d\n", error);
> > + return error;
> > + }
> > +
>
> > + if (of_property_read_u32(np, "rohm,x-plate-ohms", &val32)) {
>
> ...simple calling device_property_read_u32() instead.
>
> > + dev_err(dev, "invalid 'x-plate-ohms' supplied\n");
> > + return -EINVAL;
> > + }
> > + bu21029->x_plate_ohms = val32;
> > +
> > + touchscreen_parse_properties(bu21029->in_dev, false, &bu21029->prop);
> > +
> > + return 0;
> > +}

Thank you, changed.

> > +#ifdef CONFIG_PM_SLEEP
>
> Instead...
>
> > +static int bu21029_suspend(struct device *dev)
>
> ...use __maby_unused annotation.
>
> > +static int bu21029_resume(struct device *dev)
>
> Ditto.

OK, added.

Regards,
Mark

Mark Jonas

Building Technologies, Panel Software Fire (BT-FIR/ENG1)
Bosch Sicherheitssysteme GmbH | Postfach 11 11 | 85626 Grasbrunn | GERMANY | http://www.boschsecurity.com

Sitz: Stuttgart, Registergericht: Amtsgericht Stuttgart HRB 23118
Aufsichtsratsvorsitzender: Stefan Hartung; Geschäftsführung: Gert van Iperen, Andreas Bartz, Thomas Quante, Bernhard Schuster

Subject: [PATCH v4] Input: add bu21029 touch driver

From: Zhu Yi <[email protected]>

Add Rohm BU21029 resistive touch panel controller support with I2C
interface.

Signed-off-by: Zhu Yi <[email protected]>
Signed-off-by: Mark Jonas <[email protected]>
Reviewed-by: Heiko Schocher <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
Changes in v4:
- remove potential kernel log spamming from irq handler
- fix bug in endianess conversion of the chip's HW ID
- simplify device tree reading
- flag resume and suspend functions as potentially unused
---
Changes in v3:
- reviewed by Rob Herring
---
Changes in v2:
- make ABS_PRESSURE proportionally rising with finger pressure
- fix race between interrupt and timer during shutdown
- use infrastructure from include/linux/input/touchscreen.h
- add SPDX tag for the driver
- improve binding documentation
- fix multi-line comments
---
.../bindings/input/touchscreen/bu21029.txt | 34 ++
drivers/input/touchscreen/Kconfig | 12 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/bu21029_ts.c | 475 +++++++++++++++++++++
4 files changed, 522 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
create mode 100644 drivers/input/touchscreen/bu21029_ts.c

diff --git a/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
new file mode 100644
index 0000000..030a888
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
@@ -0,0 +1,34 @@
+* Rohm BU21029 Touch Screen Controller
+
+Required properties:
+ - compatible : must be "rohm,bu21029"
+ - reg : i2c device address of the chip (0x40 or 0x41)
+ - interrupt-parent : the phandle for the gpio controller
+ - interrupts : (gpio) interrupt to which the chip is connected
+ - reset-gpios : gpio pin to reset the chip (active low)
+ - rohm,x-plate-ohms : x-plate resistance in Ohm
+
+Optional properties:
+ - touchscreen-size-x : horizontal resolution of touchscreen (in pixels)
+ - touchscreen-size-y : vertical resolution of touchscreen (in pixels)
+ - touchscreen-max-pressure: maximum pressure value
+
+Example:
+
+ &i2c1 {
+ /* ... */
+
+ bu21029: bu21029@40 {
+ compatible = "rohm,bu21029";
+ reg = <0x40>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
+ reset-gpios = <&gpio6 16 GPIO_ACTIVE_LOW>;
+ rohm,x-plate-ohms = <600>;
+ touchscreen-size-x = <800>;
+ touchscreen-size-y = <480>;
+ touchscreen-max-pressure = <4095>;
+ };
+
+ /* ... */
+ };
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 4f15496..e09fe8f 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -151,6 +151,18 @@ config TOUCHSCREEN_BU21013
To compile this driver as a module, choose M here: the
module will be called bu21013_ts.

+config TOUCHSCREEN_BU21029
+ tristate "Rohm BU21029 based touch panel controllers"
+ depends on I2C
+ help
+ Say Y here if you have a Rohm BU21029 touchscreen controller
+ connected to your system.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called bu21029_ts.
+
config TOUCHSCREEN_CHIPONE_ICN8318
tristate "chipone icn8318 touchscreen controller"
depends on GPIOLIB || COMPILE_TEST
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index dddae79..f50624c 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_TOUCHSCREEN_AR1021_I2C) += ar1021_i2c.o
obj-$(CONFIG_TOUCHSCREEN_ATMEL_MXT) += atmel_mxt_ts.o
obj-$(CONFIG_TOUCHSCREEN_AUO_PIXCIR) += auo-pixcir-ts.o
obj-$(CONFIG_TOUCHSCREEN_BU21013) += bu21013_ts.o
+obj-$(CONFIG_TOUCHSCREEN_BU21029) += bu21029_ts.o
obj-$(CONFIG_TOUCHSCREEN_CHIPONE_ICN8318) += chipone_icn8318.o
obj-$(CONFIG_TOUCHSCREEN_CY8CTMG110) += cy8ctmg110_ts.o
obj-$(CONFIG_TOUCHSCREEN_CYTTSP_CORE) += cyttsp_core.o
diff --git a/drivers/input/touchscreen/bu21029_ts.c b/drivers/input/touchscreen/bu21029_ts.c
new file mode 100644
index 0000000..2bd63d1
--- /dev/null
+++ b/drivers/input/touchscreen/bu21029_ts.c
@@ -0,0 +1,475 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Rohm BU21029 touchscreen controller driver
+ *
+ * Copyright (C) 2015-2018 Bosch Sicherheitssysteme GmbH
+ *
+ * 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/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/touchscreen.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/timer.h>
+
+/*
+ * HW_ID1 Register (PAGE=0, ADDR=0x0E, Reset value=0x02, Read only)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | HW_IDH |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * HW_ID2 Register (PAGE=0, ADDR=0x0F, Reset value=0x29, Read only)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | HW_IDL |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * HW_IDH: high 8bits of IC's ID
+ * HW_IDL: low 8bits of IC's ID
+ */
+#define BU21029_HWID_REG (0x0E << 3)
+#define SUPPORTED_HWID 0x0229
+
+/*
+ * CFR0 Register (PAGE=0, ADDR=0x00, Reset value=0x20)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | 0 | 0 | CALIB | INTRM | 0 | 0 | 0 | 0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * CALIB: 0 = not to use calibration result (*)
+ * 1 = use calibration result
+ * INTRM: 0 = INT output depend on "pen down" (*)
+ * 1 = INT output always "0"
+ */
+#define BU21029_CFR0_REG (0x00 << 3)
+#define CFR0_VALUE 0x00
+
+/*
+ * CFR1 Register (PAGE=0, ADDR=0x01, Reset value=0xA6)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | MAV | AVE[2:0] | 0 | SMPL[2:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * MAV: 0 = median average filter off
+ * 1 = median average filter on (*)
+ * AVE: AVE+1 = number of average samples for MAV,
+ * if AVE>SMPL, then AVE=SMPL (=3)
+ * SMPL: SMPL+1 = number of conversion samples for MAV (=7)
+ */
+#define BU21029_CFR1_REG (0x01 << 3)
+#define CFR1_VALUE 0xA6
+
+/*
+ * CFR2 Register (PAGE=0, ADDR=0x02, Reset value=0x04)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | INTVL_TIME[3:0] | TIME_ST_ADC[3:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * INTVL_TIME: waiting time between completion of conversion
+ * and start of next conversion, only usable in
+ * autoscan mode (=20.480ms)
+ * TIME_ST_ADC: waiting time between application of voltage
+ * to panel and start of A/D conversion (=100us)
+ */
+#define BU21029_CFR2_REG (0x02 << 3)
+#define CFR2_VALUE 0xC9
+
+/*
+ * CFR3 Register (PAGE=0, ADDR=0x0B, Reset value=0x72)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | RM8 | STRETCH| PU90K | DUAL | PIDAC_OFS[3:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * RM8: 0 = coordinate resolution is 12bit (*)
+ * 1 = coordinate resolution is 8bit
+ * STRETCH: 0 = SCL_STRETCH function off
+ * 1 = SCL_STRETCH function on (*)
+ * PU90K: 0 = internal pull-up resistance for touch detection is ~50kohms (*)
+ * 1 = internal pull-up resistance for touch detection is ~90kohms
+ * DUAL: 0 = dual touch detection off (*)
+ * 1 = dual touch detection on
+ * PIDAC_OFS: dual touch detection circuit adjustment, it is not necessary
+ * to change this from initial value
+ */
+#define BU21029_CFR3_REG (0x0B << 3)
+#define CFR3_VALUE 0x42
+
+/*
+ * LDO Register (PAGE=0, ADDR=0x0C, Reset value=0x00)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | 0 | PVDD[2:0] | 0 | AVDD[2:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * PVDD: output voltage of panel output regulator (=2.000V)
+ * AVDD: output voltage of analog circuit regulator (=2.000V)
+ */
+#define BU21029_LDO_REG (0x0C << 3)
+#define LDO_VALUE 0x77
+
+/*
+ * Serial Interface Command Byte 1 (CID=1)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | 1 | CF | CMSK | PDM | STP |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * CF: conversion function, see table 3 in datasheet p6 (=0000, automatic scan)
+ * CMSK: 0 = executes convert function (*)
+ * 1 = reads the convert result
+ * PDM: 0 = power down after convert function stops (*)
+ * 1 = keep power on after convert function stops
+ * STP: 1 = abort current conversion and power down, set to "0" automatically
+ */
+#define BU21029_AUTOSCAN 0x80
+
+/*
+ * The timeout value needs to be larger than INTVL_TIME + tConv4 (sample and
+ * conversion time), where tConv4 is calculated by formula:
+ * tPON + tDLY1 + (tTIME_ST_ADC + (tADC * tSMPL) * 2 + tDLY2) * 3
+ * see figure 8 in datasheet p15 for details of each field.
+ */
+#define PEN_UP_TIMEOUT msecs_to_jiffies(50)
+
+#define STOP_DELAY_US 50
+#define START_DELAY_MS 2
+#define BUF_LEN 8
+#define SCALE_12BIT (1 << 12)
+#define MAX_12BIT ((1 << 12) - 1)
+#define DRIVER_NAME "bu21029"
+
+struct bu21029_ts_data {
+ struct i2c_client *client;
+ struct input_dev *in_dev;
+ struct timer_list timer;
+ struct gpio_desc *reset_gpios;
+ u32 x_plate_ohms;
+ struct touchscreen_properties prop;
+};
+
+static int bu21029_touch_report(struct bu21029_ts_data *bu21029)
+{
+ struct i2c_client *i2c = bu21029->client;
+ u8 buf[BUF_LEN];
+ u16 x, y, z1, z2;
+ u32 rz;
+ s32 max_pressure = bu21029->in_dev->absinfo[ABS_PRESSURE].maximum;
+
+ /* read touch data and deassert INT (by restarting the autoscan mode) */
+ int error = i2c_smbus_read_i2c_block_data(i2c,
+ BU21029_AUTOSCAN,
+ BUF_LEN,
+ buf);
+ if (error < 0)
+ return error;
+
+ /*
+ * compose upper 8 and lower 4 bits into a 12bit value:
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ * | ByteH | ByteL |
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ * |b07|b06|b05|b04|b03|b02|b01|b00|b07|b06|b05|b04|b03|b02|b01|b00|
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ * |v11|v10|v09|v08|v07|v06|v05|v04|v03|v02|v01|v00| 0 | 0 | 0 | 0 |
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ */
+ x = (buf[0] << 4) | (buf[1] >> 4);
+ y = (buf[2] << 4) | (buf[3] >> 4);
+ z1 = (buf[4] << 4) | (buf[5] >> 4);
+ z2 = (buf[6] << 4) | (buf[7] >> 4);
+
+ if (z1 == 0 || z2 == 0)
+ return 0;
+
+ /*
+ * calculate Rz (pressure resistance value) by equation:
+ * Rz = Rx * (x/Q) * ((z2/z1) - 1), where
+ * Rx is x-plate resistance,
+ * Q is the touch screen resolution (8bit = 256, 12bit = 4096)
+ * x, z1, z2 are the measured positions.
+ */
+ rz = z2 - z1;
+ rz *= x;
+ rz *= bu21029->x_plate_ohms;
+ rz /= z1;
+ rz = DIV_ROUND_CLOSEST(rz, SCALE_12BIT);
+ if (rz <= max_pressure) {
+ touchscreen_report_pos(bu21029->in_dev, &bu21029->prop,
+ x, y, false);
+ input_report_abs(bu21029->in_dev, ABS_PRESSURE,
+ max_pressure - rz);
+ input_report_key(bu21029->in_dev, BTN_TOUCH, 1);
+ input_sync(bu21029->in_dev);
+ }
+
+ return 0;
+}
+
+static void bu21029_touch_release(struct timer_list *t)
+{
+ struct bu21029_ts_data *bu21029 = from_timer(bu21029, t, timer);
+
+ input_report_abs(bu21029->in_dev, ABS_PRESSURE, 0);
+ input_report_key(bu21029->in_dev, BTN_TOUCH, 0);
+ input_sync(bu21029->in_dev);
+}
+
+static irqreturn_t bu21029_touch_soft_irq(int irq, void *data)
+{
+ struct bu21029_ts_data *bu21029 = data;
+
+ /*
+ * report touch and deassert interrupt (will assert again after
+ * INTVL_TIME + tConv4 for continuous touch)
+ */
+ int error = bu21029_touch_report(bu21029);
+
+ if (error)
+ return IRQ_NONE;
+
+ /* reset timer for pen up detection */
+ mod_timer(&bu21029->timer, jiffies + PEN_UP_TIMEOUT);
+
+ return IRQ_HANDLED;
+}
+
+static void bu21029_stop_chip(struct input_dev *dev)
+{
+ struct bu21029_ts_data *bu21029 = input_get_drvdata(dev);
+
+ disable_irq(bu21029->client->irq);
+ del_timer_sync(&bu21029->timer);
+
+ /* put chip into reset */
+ gpiod_set_value_cansleep(bu21029->reset_gpios, 1);
+ udelay(STOP_DELAY_US);
+}
+
+static int bu21029_start_chip(struct input_dev *dev)
+{
+ struct bu21029_ts_data *bu21029 = input_get_drvdata(dev);
+ struct i2c_client *i2c = bu21029->client;
+ struct {
+ u8 reg;
+ u8 value;
+ } init_table[] = {
+ {BU21029_CFR0_REG, CFR0_VALUE},
+ {BU21029_CFR1_REG, CFR1_VALUE},
+ {BU21029_CFR2_REG, CFR2_VALUE},
+ {BU21029_CFR3_REG, CFR3_VALUE},
+ {BU21029_LDO_REG, LDO_VALUE}
+ };
+ int error, i;
+ u16 hwid;
+
+ /* take chip out of reset */
+ gpiod_set_value_cansleep(bu21029->reset_gpios, 0);
+ mdelay(START_DELAY_MS);
+
+ error = i2c_smbus_read_i2c_block_data(i2c,
+ BU21029_HWID_REG,
+ 2,
+ (u8 *)&hwid);
+ if (error < 0) {
+ dev_err(&i2c->dev, "failed to read HW ID\n");
+ goto out;
+ }
+
+ if (be16_to_cpu(hwid) != SUPPORTED_HWID) {
+ dev_err(&i2c->dev, "unsupported HW ID 0x%x\n", hwid);
+ error = -ENODEV;
+ goto out;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(init_table); ++i) {
+ error = i2c_smbus_write_byte_data(i2c,
+ init_table[i].reg,
+ init_table[i].value);
+ if (error < 0) {
+ dev_err(&i2c->dev,
+ "failed to write 0x%x to register 0x%x\n",
+ init_table[i].value,
+ init_table[i].reg);
+ goto out;
+ }
+ }
+
+ error = i2c_smbus_write_byte(i2c, BU21029_AUTOSCAN);
+ if (error < 0) {
+ dev_err(&i2c->dev, "failed to start autoscan\n");
+ goto out;
+ }
+
+ enable_irq(bu21029->client->irq);
+ return 0;
+
+out:
+ bu21029_stop_chip(dev);
+ return error;
+}
+
+static int bu21029_parse_dt(struct bu21029_ts_data *bu21029)
+{
+ struct device *dev = &bu21029->client->dev;
+ int error;
+
+ bu21029->reset_gpios = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(bu21029->reset_gpios)) {
+ error = PTR_ERR(bu21029->reset_gpios);
+ if (error != -EPROBE_DEFER)
+ dev_err(dev, "invalid 'reset-gpios':%d\n", error);
+ return error;
+ }
+
+ error = device_property_read_u32(dev, "rohm,x-plate-ohms",
+ &bu21029->x_plate_ohms);
+ if (error) {
+ dev_err(dev, "invalid 'x-plate-ohms' supplied:%d\n", error);
+ return error;
+ }
+
+ touchscreen_parse_properties(bu21029->in_dev, false, &bu21029->prop);
+
+ return 0;
+}
+
+static int bu21029_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct bu21029_ts_data *bu21029;
+ struct input_dev *in_dev;
+ int error;
+
+ if (!i2c_check_functionality(client->adapter,
+ I2C_FUNC_SMBUS_WRITE_BYTE |
+ I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
+ I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
+ dev_err(&client->dev,
+ "i2c functionality support is not sufficient\n");
+ return -EIO;
+ }
+
+ bu21029 = devm_kzalloc(&client->dev, sizeof(*bu21029), GFP_KERNEL);
+ if (!bu21029)
+ return -ENOMEM;
+
+ in_dev = devm_input_allocate_device(&client->dev);
+ if (!in_dev) {
+ dev_err(&client->dev, "unable to allocate input device\n");
+ return -ENOMEM;
+ }
+
+ bu21029->client = client;
+ bu21029->in_dev = in_dev;
+ timer_setup(&bu21029->timer, bu21029_touch_release, 0);
+
+ in_dev->name = DRIVER_NAME;
+ in_dev->id.bustype = BUS_I2C;
+ in_dev->open = bu21029_start_chip;
+ in_dev->close = bu21029_stop_chip;
+
+ input_set_capability(in_dev, EV_KEY, BTN_TOUCH);
+ input_set_abs_params(in_dev, ABS_X, 0, MAX_12BIT, 0, 0);
+ input_set_abs_params(in_dev, ABS_Y, 0, MAX_12BIT, 0, 0);
+ input_set_abs_params(in_dev, ABS_PRESSURE, 0, MAX_12BIT, 0, 0);
+
+ error = bu21029_parse_dt(bu21029);
+ if (error)
+ return error;
+
+ input_set_drvdata(in_dev, bu21029);
+
+ error = devm_request_threaded_irq(&client->dev,
+ client->irq,
+ NULL,
+ bu21029_touch_soft_irq,
+ IRQF_ONESHOT,
+ DRIVER_NAME,
+ bu21029);
+ if (error) {
+ dev_err(&client->dev, "unable to request touch irq\n");
+ return error;
+ }
+
+ bu21029_stop_chip(in_dev);
+
+ error = input_register_device(in_dev);
+ if (error) {
+ dev_err(&client->dev, "unable to register input device\n");
+ return error;
+ }
+
+ i2c_set_clientdata(client, bu21029);
+
+ return 0;
+}
+
+static int bu21029_remove(struct i2c_client *client)
+{
+ struct bu21029_ts_data *bu21029 = i2c_get_clientdata(client);
+
+ bu21029_stop_chip(bu21029->in_dev);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int __maybe_unused bu21029_suspend(struct device *dev)
+{
+ struct i2c_client *i2c = to_i2c_client(dev);
+ struct bu21029_ts_data *bu21029 = i2c_get_clientdata(i2c);
+
+ mutex_lock(&bu21029->in_dev->mutex);
+ if (bu21029->in_dev->users)
+ bu21029_stop_chip(bu21029->in_dev);
+ mutex_unlock(&bu21029->in_dev->mutex);
+
+ return 0;
+}
+
+static int __maybe_unused bu21029_resume(struct device *dev)
+{
+ struct i2c_client *i2c = to_i2c_client(dev);
+ struct bu21029_ts_data *bu21029 = i2c_get_clientdata(i2c);
+
+ mutex_lock(&bu21029->in_dev->mutex);
+ if (bu21029->in_dev->users)
+ bu21029_start_chip(bu21029->in_dev);
+ mutex_unlock(&bu21029->in_dev->mutex);
+
+ return 0;
+}
+#endif
+static SIMPLE_DEV_PM_OPS(bu21029_pm_ops, bu21029_suspend, bu21029_resume);
+
+static const struct i2c_device_id bu21029_ids[] = {
+ {DRIVER_NAME, 0},
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, bu21029_ids);
+
+static struct i2c_driver bu21029_driver = {
+ .driver = {
+ .name = DRIVER_NAME,
+ .pm = &bu21029_pm_ops,
+ },
+ .id_table = bu21029_ids,
+ .probe = bu21029_probe,
+ .remove = bu21029_remove,
+};
+module_i2c_driver(bu21029_driver);
+
+MODULE_AUTHOR("Zhu Yi <[email protected]>");
+MODULE_DESCRIPTION("Rohm BU21029 touchscreen controller driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4


2018-05-16 17:44:33

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH v3] Input: add bu21029 touch driver

On Wed, May 16, 2018 at 01:24:24PM +0000, Jonas Mark (BT-FIR/ENG1) wrote:
> Hello Andy,
>
> > > Add Rohm BU21029 resistive touch panel controller support with I2C
> > > interface.
> >
> > > +#include <linux/of.h>
> >
> > This becomes redundant (see below).
>
> Removed.
>
> > > +#define STOP_DELAY_US 50L
> > > +#define START_DELAY_MS 2L
> > > +#define BUF_LEN 8L
> >
> > No need to use L for such small numbers. Integer promotion is a part
> > of C standard.
>
> OK.
>
> > > +#define SCALE_12BIT (1 << 12)
> > > +#define MAX_12BIT ((1 << 12) - 1)
> >
> > BIT(12)
> > GENMASK(11, 0)
>
> We are not convinced that we should use BIT() and GENMASK() here.
>
> The reason is that SCALE_12BIT is actually not used as a bit but as an
> input value for DIV_ROUND_CLOSEST. We think that the BIT() macro will
> hide the meaning of the value.
>
> MAX_12BIT is also a value and not a bit mask. Thus, we also think that
> using the GENMASK() macro will hide its purpose. Also, the
> documentation of GENMASK() says that it is a mask and not a value.

I agree.

>
> > > +static int bu21029_touch_report(struct bu21029_ts_data *bu21029)
> > > +{
> > > + struct i2c_client *i2c = bu21029->client;
> > > + u8 buf[BUF_LEN];
> > > + int error = bu21029_touch_report(bu21029);
> >
> > > +
> >
> > Redundant empty line.
>
> Removed.
>
> > > + if (error) {
> >
> > > + dev_err(&i2c->dev, "failed to report (error: %d)\n", error);
> >
> > Potential spamming case.
> >
> > > + return IRQ_NONE;
> > > + }
>
> You are right, we will remove the error message.
>
> > > +static void bu21029_stop_chip(struct input_dev *dev)
> > > +{
> > > + struct bu21029_ts_data *bu21029 = input_get_drvdata(dev);
> > > +
> > > + disable_irq(bu21029->client->irq);
> > > + del_timer_sync(&bu21029->timer);
> > > +
> > > + /* put chip into reset */
> > > + gpiod_set_value_cansleep(bu21029->reset_gpios, 1);
> >
> > > + udelay(STOP_DELAY_US);
> >
> > udelay() ?!
> >
> > > +}
>
> According to the datasheet disabling the chip will take 30 microseconds.
> In the defines we added a buffer of 20 microseconds and thus
> STOP_DELAY_US is 50. The function guarantees that the chip is stopped
> before it returns.
>
> We think that it is ok to use udelay() here because in normal operation
> the chip is not stopped. It is only stopped when loading or unloading
> the driver, or when the system suspends.
>
> We would like to keep it like it is.

The issue is not with having delay here, but the kind of delay you are
using: udelay makes CPU spin for given amount of time; you really want
msleep() or usleep_range() here.

>
> > > +static int bu21029_start_chip(struct input_dev *dev)
> > > +{
> >
> > > + u16 hwid;
> > > +
> > > + /* take chip out of reset */
> > > + gpiod_set_value_cansleep(bu21029->reset_gpios, 0);
> >
> > > + mdelay(START_DELAY_MS);
> >
> > mdelay()?!

Same here - replace with msleep().

> >
> > > +
> > > + error = i2c_smbus_read_i2c_block_data(i2c,
> > > + BU21029_HWID_REG,
> > > + 2,
> > > + (u8 *)&hwid);
> > > + if (error < 0) {
> > > + dev_err(&i2c->dev, "failed to read HW ID\n");
> > > + goto out;
> > > + }
>
> After de-asserting the reset chip takes 1 millisecond until it is
> operational. We added a 1 millisecond buffer to it. Thus,
> START_DELAY_MS is 2.
>
> The following I2C read will not succeed without waiting for the chip
> being ready.
>
> > > + if (cpu_to_be16(hwid) != SUPPORTED_HWID) {
> >
> > Hmm... Why cpu_to_be16() is required?
> >
> > > + dev_err(&i2c->dev, "unsupported HW ID 0x%x\n", hwid);
> > > + error = -ENODEV;
> > > + goto out;
> > > + }
> > > +}
>
> You are right, it works but what we meant to do here is to convert the
> chip's value (big endian) into the CPU endianness. We will change it to
> be16_to_cpu().
>
> > > +static int bu21029_parse_dt(struct bu21029_ts_data *bu21029)
> >
> > You can get rid of DT requirement by...
> >
> > > +{
> > > + struct device *dev = &bu21029->client->dev;
> > > + struct device_node *np = dev->of_node;
> > > + u32 val32;
> > > + int error;
> >
> > > + if (!np) {
> > > + dev_err(dev, "no device tree data\n");
> > > + return -EINVAL;
> > > + }
> >
> > (this becomes redundant)
> >
> > > +
> > > + bu21029->reset_gpios = devm_gpiod_get(dev, "reset",
> > GPIOD_OUT_HIGH);
> > > + if (IS_ERR(bu21029->reset_gpios)) {
> > > + error = PTR_ERR(bu21029->reset_gpios);
> > > + if (error != -EPROBE_DEFER)
> > > + dev_err(dev, "invalid 'reset-gpios':%d\n", error);
> > > + return error;
> > > + }
> > > +
> >
> > > + if (of_property_read_u32(np, "rohm,x-plate-ohms", &val32)) {
> >
> > ...simple calling device_property_read_u32() instead.
> >
> > > + dev_err(dev, "invalid 'x-plate-ohms' supplied\n");
> > > + return -EINVAL;
> > > + }
> > > + bu21029->x_plate_ohms = val32;
> > > +
> > > + touchscreen_parse_properties(bu21029->in_dev, false, &bu21029->prop);
> > > +
> > > + return 0;
> > > +}
>
> Thank you, changed.
>
> > > +#ifdef CONFIG_PM_SLEEP
> >
> > Instead...
> >
> > > +static int bu21029_suspend(struct device *dev)
> >
> > ...use __maby_unused annotation.
> >
> > > +static int bu21029_resume(struct device *dev)
> >
> > Ditto.
>
> OK, added.

You also need to drop #ifdef CONFIG_SLEEP. That's the point: we want to
reduce amount of conditionally compiled code and use __maybe_unused to
shut off compiler warning about some functions not used in certain
configurations. We rely on linker to eliminate dead functions from
executable.

Thanks.

--
Dmitry

2018-05-17 10:06:36

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v4] Input: add bu21029 touch driver

Hi Zhu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v4.17-rc5]
[also build test WARNING on next-20180516]
[cannot apply to input/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Mark-Jonas/Input-add-bu21029-touch-driver/20180517-133332
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/input/touchscreen/bu21029_ts.c:289:13: sparse: cast to restricted __be16
>> drivers/input/touchscreen/bu21029_ts.c:289:13: sparse: cast to restricted __be16
>> drivers/input/touchscreen/bu21029_ts.c:289:13: sparse: cast to restricted __be16
>> drivers/input/touchscreen/bu21029_ts.c:289:13: sparse: cast to restricted __be16

vim +289 drivers/input/touchscreen/bu21029_ts.c

258
259 static int bu21029_start_chip(struct input_dev *dev)
260 {
261 struct bu21029_ts_data *bu21029 = input_get_drvdata(dev);
262 struct i2c_client *i2c = bu21029->client;
263 struct {
264 u8 reg;
265 u8 value;
266 } init_table[] = {
267 {BU21029_CFR0_REG, CFR0_VALUE},
268 {BU21029_CFR1_REG, CFR1_VALUE},
269 {BU21029_CFR2_REG, CFR2_VALUE},
270 {BU21029_CFR3_REG, CFR3_VALUE},
271 {BU21029_LDO_REG, LDO_VALUE}
272 };
273 int error, i;
274 u16 hwid;
275
276 /* take chip out of reset */
277 gpiod_set_value_cansleep(bu21029->reset_gpios, 0);
278 mdelay(START_DELAY_MS);
279
280 error = i2c_smbus_read_i2c_block_data(i2c,
281 BU21029_HWID_REG,
282 2,
283 (u8 *)&hwid);
284 if (error < 0) {
285 dev_err(&i2c->dev, "failed to read HW ID\n");
286 goto out;
287 }
288
> 289 if (be16_to_cpu(hwid) != SUPPORTED_HWID) {
290 dev_err(&i2c->dev, "unsupported HW ID 0x%x\n", hwid);
291 error = -ENODEV;
292 goto out;
293 }
294
295 for (i = 0; i < ARRAY_SIZE(init_table); ++i) {
296 error = i2c_smbus_write_byte_data(i2c,
297 init_table[i].reg,
298 init_table[i].value);
299 if (error < 0) {
300 dev_err(&i2c->dev,
301 "failed to write 0x%x to register 0x%x\n",
302 init_table[i].value,
303 init_table[i].reg);
304 goto out;
305 }
306 }
307
308 error = i2c_smbus_write_byte(i2c, BU21029_AUTOSCAN);
309 if (error < 0) {
310 dev_err(&i2c->dev, "failed to start autoscan\n");
311 goto out;
312 }
313
314 enable_irq(bu21029->client->irq);
315 return 0;
316
317 out:
318 bu21029_stop_chip(dev);
319 return error;
320 }
321

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

Subject: [PATCH v5] Input: add bu21029 touch driver

From: Zhu Yi <[email protected]>

Add Rohm BU21029 resistive touch panel controller support with I2C
interface.

Signed-off-by: Zhu Yi <[email protected]>
Signed-off-by: Mark Jonas <[email protected]>
Reviewed-by: Heiko Schocher <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
Changes in v5:
- use usleep_range() and msleep() instead of atomic delays
- drop #ifdef CONFIG_PM_SLEEP
- annotate HW ID as big endian (sparse check finding)
---
Changes in v4:
- remove potential kernel log spamming from irq handler
- fix bug in endianess conversion of the chip's HW ID
- simplify device tree reading
- flag resume and suspend functions as potentially unused
---
Changes in v3:
- reviewed by Rob Herring
---
Changes in v2:
- make ABS_PRESSURE proportionally rising with finger pressure
- fix race between interrupt and timer during shutdown
- use infrastructure from include/linux/input/touchscreen.h
- add SPDX tag for the driver
- improve binding documentation
- fix multi-line comments
---
.../bindings/input/touchscreen/bu21029.txt | 34 ++
drivers/input/touchscreen/Kconfig | 12 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/bu21029_ts.c | 474 +++++++++++++++++++++
4 files changed, 521 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
create mode 100644 drivers/input/touchscreen/bu21029_ts.c

diff --git a/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
new file mode 100644
index 0000000..030a888
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
@@ -0,0 +1,34 @@
+* Rohm BU21029 Touch Screen Controller
+
+Required properties:
+ - compatible : must be "rohm,bu21029"
+ - reg : i2c device address of the chip (0x40 or 0x41)
+ - interrupt-parent : the phandle for the gpio controller
+ - interrupts : (gpio) interrupt to which the chip is connected
+ - reset-gpios : gpio pin to reset the chip (active low)
+ - rohm,x-plate-ohms : x-plate resistance in Ohm
+
+Optional properties:
+ - touchscreen-size-x : horizontal resolution of touchscreen (in pixels)
+ - touchscreen-size-y : vertical resolution of touchscreen (in pixels)
+ - touchscreen-max-pressure: maximum pressure value
+
+Example:
+
+ &i2c1 {
+ /* ... */
+
+ bu21029: bu21029@40 {
+ compatible = "rohm,bu21029";
+ reg = <0x40>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
+ reset-gpios = <&gpio6 16 GPIO_ACTIVE_LOW>;
+ rohm,x-plate-ohms = <600>;
+ touchscreen-size-x = <800>;
+ touchscreen-size-y = <480>;
+ touchscreen-max-pressure = <4095>;
+ };
+
+ /* ... */
+ };
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 4f15496..e09fe8f 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -151,6 +151,18 @@ config TOUCHSCREEN_BU21013
To compile this driver as a module, choose M here: the
module will be called bu21013_ts.

+config TOUCHSCREEN_BU21029
+ tristate "Rohm BU21029 based touch panel controllers"
+ depends on I2C
+ help
+ Say Y here if you have a Rohm BU21029 touchscreen controller
+ connected to your system.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called bu21029_ts.
+
config TOUCHSCREEN_CHIPONE_ICN8318
tristate "chipone icn8318 touchscreen controller"
depends on GPIOLIB || COMPILE_TEST
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index dddae79..f50624c 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_TOUCHSCREEN_AR1021_I2C) += ar1021_i2c.o
obj-$(CONFIG_TOUCHSCREEN_ATMEL_MXT) += atmel_mxt_ts.o
obj-$(CONFIG_TOUCHSCREEN_AUO_PIXCIR) += auo-pixcir-ts.o
obj-$(CONFIG_TOUCHSCREEN_BU21013) += bu21013_ts.o
+obj-$(CONFIG_TOUCHSCREEN_BU21029) += bu21029_ts.o
obj-$(CONFIG_TOUCHSCREEN_CHIPONE_ICN8318) += chipone_icn8318.o
obj-$(CONFIG_TOUCHSCREEN_CY8CTMG110) += cy8ctmg110_ts.o
obj-$(CONFIG_TOUCHSCREEN_CYTTSP_CORE) += cyttsp_core.o
diff --git a/drivers/input/touchscreen/bu21029_ts.c b/drivers/input/touchscreen/bu21029_ts.c
new file mode 100644
index 0000000..5a72671
--- /dev/null
+++ b/drivers/input/touchscreen/bu21029_ts.c
@@ -0,0 +1,474 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Rohm BU21029 touchscreen controller driver
+ *
+ * Copyright (C) 2015-2018 Bosch Sicherheitssysteme GmbH
+ *
+ * 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/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/touchscreen.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/timer.h>
+
+/*
+ * HW_ID1 Register (PAGE=0, ADDR=0x0E, Reset value=0x02, Read only)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | HW_IDH |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * HW_ID2 Register (PAGE=0, ADDR=0x0F, Reset value=0x29, Read only)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | HW_IDL |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * HW_IDH: high 8bits of IC's ID
+ * HW_IDL: low 8bits of IC's ID
+ */
+#define BU21029_HWID_REG (0x0E << 3)
+#define SUPPORTED_HWID 0x0229
+
+/*
+ * CFR0 Register (PAGE=0, ADDR=0x00, Reset value=0x20)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | 0 | 0 | CALIB | INTRM | 0 | 0 | 0 | 0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * CALIB: 0 = not to use calibration result (*)
+ * 1 = use calibration result
+ * INTRM: 0 = INT output depend on "pen down" (*)
+ * 1 = INT output always "0"
+ */
+#define BU21029_CFR0_REG (0x00 << 3)
+#define CFR0_VALUE 0x00
+
+/*
+ * CFR1 Register (PAGE=0, ADDR=0x01, Reset value=0xA6)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | MAV | AVE[2:0] | 0 | SMPL[2:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * MAV: 0 = median average filter off
+ * 1 = median average filter on (*)
+ * AVE: AVE+1 = number of average samples for MAV,
+ * if AVE>SMPL, then AVE=SMPL (=3)
+ * SMPL: SMPL+1 = number of conversion samples for MAV (=7)
+ */
+#define BU21029_CFR1_REG (0x01 << 3)
+#define CFR1_VALUE 0xA6
+
+/*
+ * CFR2 Register (PAGE=0, ADDR=0x02, Reset value=0x04)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | INTVL_TIME[3:0] | TIME_ST_ADC[3:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * INTVL_TIME: waiting time between completion of conversion
+ * and start of next conversion, only usable in
+ * autoscan mode (=20.480ms)
+ * TIME_ST_ADC: waiting time between application of voltage
+ * to panel and start of A/D conversion (=100us)
+ */
+#define BU21029_CFR2_REG (0x02 << 3)
+#define CFR2_VALUE 0xC9
+
+/*
+ * CFR3 Register (PAGE=0, ADDR=0x0B, Reset value=0x72)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | RM8 | STRETCH| PU90K | DUAL | PIDAC_OFS[3:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * RM8: 0 = coordinate resolution is 12bit (*)
+ * 1 = coordinate resolution is 8bit
+ * STRETCH: 0 = SCL_STRETCH function off
+ * 1 = SCL_STRETCH function on (*)
+ * PU90K: 0 = internal pull-up resistance for touch detection is ~50kohms (*)
+ * 1 = internal pull-up resistance for touch detection is ~90kohms
+ * DUAL: 0 = dual touch detection off (*)
+ * 1 = dual touch detection on
+ * PIDAC_OFS: dual touch detection circuit adjustment, it is not necessary
+ * to change this from initial value
+ */
+#define BU21029_CFR3_REG (0x0B << 3)
+#define CFR3_VALUE 0x42
+
+/*
+ * LDO Register (PAGE=0, ADDR=0x0C, Reset value=0x00)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | 0 | PVDD[2:0] | 0 | AVDD[2:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * PVDD: output voltage of panel output regulator (=2.000V)
+ * AVDD: output voltage of analog circuit regulator (=2.000V)
+ */
+#define BU21029_LDO_REG (0x0C << 3)
+#define LDO_VALUE 0x77
+
+/*
+ * Serial Interface Command Byte 1 (CID=1)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | 1 | CF | CMSK | PDM | STP |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * CF: conversion function, see table 3 in datasheet p6 (=0000, automatic scan)
+ * CMSK: 0 = executes convert function (*)
+ * 1 = reads the convert result
+ * PDM: 0 = power down after convert function stops (*)
+ * 1 = keep power on after convert function stops
+ * STP: 1 = abort current conversion and power down, set to "0" automatically
+ */
+#define BU21029_AUTOSCAN 0x80
+
+/*
+ * The timeout value needs to be larger than INTVL_TIME + tConv4 (sample and
+ * conversion time), where tConv4 is calculated by formula:
+ * tPON + tDLY1 + (tTIME_ST_ADC + (tADC * tSMPL) * 2 + tDLY2) * 3
+ * see figure 8 in datasheet p15 for details of each field.
+ */
+#define PEN_UP_TIMEOUT msecs_to_jiffies(50)
+
+#define STOP_DELAY_MIN_US 50
+#define STOP_DELAY_MAX_US 1000
+#define START_DELAY_MS 2
+#define BUF_LEN 8
+#define SCALE_12BIT (1 << 12)
+#define MAX_12BIT ((1 << 12) - 1)
+#define DRIVER_NAME "bu21029"
+
+struct bu21029_ts_data {
+ struct i2c_client *client;
+ struct input_dev *in_dev;
+ struct timer_list timer;
+ struct gpio_desc *reset_gpios;
+ u32 x_plate_ohms;
+ struct touchscreen_properties prop;
+};
+
+static int bu21029_touch_report(struct bu21029_ts_data *bu21029)
+{
+ struct i2c_client *i2c = bu21029->client;
+ u8 buf[BUF_LEN];
+ u16 x, y, z1, z2;
+ u32 rz;
+ s32 max_pressure = bu21029->in_dev->absinfo[ABS_PRESSURE].maximum;
+
+ /* read touch data and deassert INT (by restarting the autoscan mode) */
+ int error = i2c_smbus_read_i2c_block_data(i2c,
+ BU21029_AUTOSCAN,
+ BUF_LEN,
+ buf);
+ if (error < 0)
+ return error;
+
+ /*
+ * compose upper 8 and lower 4 bits into a 12bit value:
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ * | ByteH | ByteL |
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ * |b07|b06|b05|b04|b03|b02|b01|b00|b07|b06|b05|b04|b03|b02|b01|b00|
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ * |v11|v10|v09|v08|v07|v06|v05|v04|v03|v02|v01|v00| 0 | 0 | 0 | 0 |
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ */
+ x = (buf[0] << 4) | (buf[1] >> 4);
+ y = (buf[2] << 4) | (buf[3] >> 4);
+ z1 = (buf[4] << 4) | (buf[5] >> 4);
+ z2 = (buf[6] << 4) | (buf[7] >> 4);
+
+ if (z1 == 0 || z2 == 0)
+ return 0;
+
+ /*
+ * calculate Rz (pressure resistance value) by equation:
+ * Rz = Rx * (x/Q) * ((z2/z1) - 1), where
+ * Rx is x-plate resistance,
+ * Q is the touch screen resolution (8bit = 256, 12bit = 4096)
+ * x, z1, z2 are the measured positions.
+ */
+ rz = z2 - z1;
+ rz *= x;
+ rz *= bu21029->x_plate_ohms;
+ rz /= z1;
+ rz = DIV_ROUND_CLOSEST(rz, SCALE_12BIT);
+ if (rz <= max_pressure) {
+ touchscreen_report_pos(bu21029->in_dev, &bu21029->prop,
+ x, y, false);
+ input_report_abs(bu21029->in_dev, ABS_PRESSURE,
+ max_pressure - rz);
+ input_report_key(bu21029->in_dev, BTN_TOUCH, 1);
+ input_sync(bu21029->in_dev);
+ }
+
+ return 0;
+}
+
+static void bu21029_touch_release(struct timer_list *t)
+{
+ struct bu21029_ts_data *bu21029 = from_timer(bu21029, t, timer);
+
+ input_report_abs(bu21029->in_dev, ABS_PRESSURE, 0);
+ input_report_key(bu21029->in_dev, BTN_TOUCH, 0);
+ input_sync(bu21029->in_dev);
+}
+
+static irqreturn_t bu21029_touch_soft_irq(int irq, void *data)
+{
+ struct bu21029_ts_data *bu21029 = data;
+
+ /*
+ * report touch and deassert interrupt (will assert again after
+ * INTVL_TIME + tConv4 for continuous touch)
+ */
+ int error = bu21029_touch_report(bu21029);
+
+ if (error)
+ return IRQ_NONE;
+
+ /* reset timer for pen up detection */
+ mod_timer(&bu21029->timer, jiffies + PEN_UP_TIMEOUT);
+
+ return IRQ_HANDLED;
+}
+
+static void bu21029_stop_chip(struct input_dev *dev)
+{
+ struct bu21029_ts_data *bu21029 = input_get_drvdata(dev);
+
+ disable_irq(bu21029->client->irq);
+ del_timer_sync(&bu21029->timer);
+
+ /* put chip into reset */
+ gpiod_set_value_cansleep(bu21029->reset_gpios, 1);
+ usleep_range(STOP_DELAY_MIN_US, STOP_DELAY_MAX_US);
+}
+
+static int bu21029_start_chip(struct input_dev *dev)
+{
+ struct bu21029_ts_data *bu21029 = input_get_drvdata(dev);
+ struct i2c_client *i2c = bu21029->client;
+ struct {
+ u8 reg;
+ u8 value;
+ } init_table[] = {
+ {BU21029_CFR0_REG, CFR0_VALUE},
+ {BU21029_CFR1_REG, CFR1_VALUE},
+ {BU21029_CFR2_REG, CFR2_VALUE},
+ {BU21029_CFR3_REG, CFR3_VALUE},
+ {BU21029_LDO_REG, LDO_VALUE}
+ };
+ int error, i;
+ __be16 hwid;
+
+ /* take chip out of reset */
+ gpiod_set_value_cansleep(bu21029->reset_gpios, 0);
+ msleep(START_DELAY_MS);
+
+ error = i2c_smbus_read_i2c_block_data(i2c,
+ BU21029_HWID_REG,
+ 2,
+ (u8 *)&hwid);
+ if (error < 0) {
+ dev_err(&i2c->dev, "failed to read HW ID\n");
+ goto out;
+ }
+
+ if (be16_to_cpu(hwid) != SUPPORTED_HWID) {
+ dev_err(&i2c->dev, "unsupported HW ID 0x%x\n", hwid);
+ error = -ENODEV;
+ goto out;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(init_table); ++i) {
+ error = i2c_smbus_write_byte_data(i2c,
+ init_table[i].reg,
+ init_table[i].value);
+ if (error < 0) {
+ dev_err(&i2c->dev,
+ "failed to write 0x%x to register 0x%x\n",
+ init_table[i].value,
+ init_table[i].reg);
+ goto out;
+ }
+ }
+
+ error = i2c_smbus_write_byte(i2c, BU21029_AUTOSCAN);
+ if (error < 0) {
+ dev_err(&i2c->dev, "failed to start autoscan\n");
+ goto out;
+ }
+
+ enable_irq(bu21029->client->irq);
+ return 0;
+
+out:
+ bu21029_stop_chip(dev);
+ return error;
+}
+
+static int bu21029_parse_dt(struct bu21029_ts_data *bu21029)
+{
+ struct device *dev = &bu21029->client->dev;
+ int error;
+
+ bu21029->reset_gpios = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(bu21029->reset_gpios)) {
+ error = PTR_ERR(bu21029->reset_gpios);
+ if (error != -EPROBE_DEFER)
+ dev_err(dev, "invalid 'reset-gpios':%d\n", error);
+ return error;
+ }
+
+ error = device_property_read_u32(dev, "rohm,x-plate-ohms",
+ &bu21029->x_plate_ohms);
+ if (error) {
+ dev_err(dev, "invalid 'x-plate-ohms' supplied:%d\n", error);
+ return error;
+ }
+
+ touchscreen_parse_properties(bu21029->in_dev, false, &bu21029->prop);
+
+ return 0;
+}
+
+static int bu21029_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct bu21029_ts_data *bu21029;
+ struct input_dev *in_dev;
+ int error;
+
+ if (!i2c_check_functionality(client->adapter,
+ I2C_FUNC_SMBUS_WRITE_BYTE |
+ I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
+ I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
+ dev_err(&client->dev,
+ "i2c functionality support is not sufficient\n");
+ return -EIO;
+ }
+
+ bu21029 = devm_kzalloc(&client->dev, sizeof(*bu21029), GFP_KERNEL);
+ if (!bu21029)
+ return -ENOMEM;
+
+ in_dev = devm_input_allocate_device(&client->dev);
+ if (!in_dev) {
+ dev_err(&client->dev, "unable to allocate input device\n");
+ return -ENOMEM;
+ }
+
+ bu21029->client = client;
+ bu21029->in_dev = in_dev;
+ timer_setup(&bu21029->timer, bu21029_touch_release, 0);
+
+ in_dev->name = DRIVER_NAME;
+ in_dev->id.bustype = BUS_I2C;
+ in_dev->open = bu21029_start_chip;
+ in_dev->close = bu21029_stop_chip;
+
+ input_set_capability(in_dev, EV_KEY, BTN_TOUCH);
+ input_set_abs_params(in_dev, ABS_X, 0, MAX_12BIT, 0, 0);
+ input_set_abs_params(in_dev, ABS_Y, 0, MAX_12BIT, 0, 0);
+ input_set_abs_params(in_dev, ABS_PRESSURE, 0, MAX_12BIT, 0, 0);
+
+ error = bu21029_parse_dt(bu21029);
+ if (error)
+ return error;
+
+ input_set_drvdata(in_dev, bu21029);
+
+ error = devm_request_threaded_irq(&client->dev,
+ client->irq,
+ NULL,
+ bu21029_touch_soft_irq,
+ IRQF_ONESHOT,
+ DRIVER_NAME,
+ bu21029);
+ if (error) {
+ dev_err(&client->dev, "unable to request touch irq\n");
+ return error;
+ }
+
+ bu21029_stop_chip(in_dev);
+
+ error = input_register_device(in_dev);
+ if (error) {
+ dev_err(&client->dev, "unable to register input device\n");
+ return error;
+ }
+
+ i2c_set_clientdata(client, bu21029);
+
+ return 0;
+}
+
+static int bu21029_remove(struct i2c_client *client)
+{
+ struct bu21029_ts_data *bu21029 = i2c_get_clientdata(client);
+
+ bu21029_stop_chip(bu21029->in_dev);
+
+ return 0;
+}
+
+static int __maybe_unused bu21029_suspend(struct device *dev)
+{
+ struct i2c_client *i2c = to_i2c_client(dev);
+ struct bu21029_ts_data *bu21029 = i2c_get_clientdata(i2c);
+
+ mutex_lock(&bu21029->in_dev->mutex);
+ if (bu21029->in_dev->users)
+ bu21029_stop_chip(bu21029->in_dev);
+ mutex_unlock(&bu21029->in_dev->mutex);
+
+ return 0;
+}
+
+static int __maybe_unused bu21029_resume(struct device *dev)
+{
+ struct i2c_client *i2c = to_i2c_client(dev);
+ struct bu21029_ts_data *bu21029 = i2c_get_clientdata(i2c);
+
+ mutex_lock(&bu21029->in_dev->mutex);
+ if (bu21029->in_dev->users)
+ bu21029_start_chip(bu21029->in_dev);
+ mutex_unlock(&bu21029->in_dev->mutex);
+
+ return 0;
+}
+static SIMPLE_DEV_PM_OPS(bu21029_pm_ops, bu21029_suspend, bu21029_resume);
+
+static const struct i2c_device_id bu21029_ids[] = {
+ {DRIVER_NAME, 0},
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, bu21029_ids);
+
+static struct i2c_driver bu21029_driver = {
+ .driver = {
+ .name = DRIVER_NAME,
+ .pm = &bu21029_pm_ops,
+ },
+ .id_table = bu21029_ids,
+ .probe = bu21029_probe,
+ .remove = bu21029_remove,
+};
+module_i2c_driver(bu21029_driver);
+
+MODULE_AUTHOR("Zhu Yi <[email protected]>");
+MODULE_DESCRIPTION("Rohm BU21029 touchscreen controller driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4


Subject: AW: [PATCH v3] Input: add bu21029 touch driver

Hello Dmitry,

> > > > +static void bu21029_stop_chip(struct input_dev *dev)
> > > > +{
> > > > + struct bu21029_ts_data *bu21029 = input_get_drvdata(dev);
> > > > +
> > > > + disable_irq(bu21029->client->irq);
> > > > + del_timer_sync(&bu21029->timer);
> > > > +
> > > > + /* put chip into reset */
> > > > + gpiod_set_value_cansleep(bu21029->reset_gpios, 1);
> > >
> > > > + udelay(STOP_DELAY_US);
> > >
> > > udelay() ?!
> > >
> > > > +}
> >
> > According to the datasheet disabling the chip will take 30 microseconds.
> > In the defines we added a buffer of 20 microseconds and thus
> > STOP_DELAY_US is 50. The function guarantees that the chip is stopped
> > before it returns.
> >
> > We think that it is ok to use udelay() here because in normal operation
> > the chip is not stopped. It is only stopped when loading or unloading
> > the driver, or when the system suspends.
> >
> > We would like to keep it like it is.
>
> The issue is not with having delay here, but the kind of delay you are
> using: udelay makes CPU spin for given amount of time; you really want
> msleep() or usleep_range() here.

Understood and changed.

> > > > +static int bu21029_start_chip(struct input_dev *dev)
> > > > +{
> > >
> > > > + u16 hwid;
> > > > +
> > > > + /* take chip out of reset */
> > > > + gpiod_set_value_cansleep(bu21029->reset_gpios, 0);
> > >
> > > > + mdelay(START_DELAY_MS);
> > >
> > > mdelay()?!
>
> Same here - replace with msleep().

Replaced.

> > > Instead...
> > >
> > > > +static int bu21029_suspend(struct device *dev)
> > >
> > > ...use __maby_unused annotation.
> > >
> > > > +static int bu21029_resume(struct device *dev)
> > >
> > > Ditto.
> >
> > OK, added.
>
> You also need to drop #ifdef CONFIG_SLEEP. That's the point: we want to
> reduce amount of conditionally compiled code and use __maybe_unused to
> shut off compiler warning about some functions not used in certain
> configurations. We rely on linker to eliminate dead functions from
> executable.

Done.

Greetings,
Mark

Mark Jonas

Building Technologies, Panel Software Fire (BT-FIR/ENG1)
Bosch Sicherheitssysteme GmbH | Postfach 11 11 | 85626 Grasbrunn | GERMANY | http://www.boschsecurity.com

Sitz: Stuttgart, Registergericht: Amtsgericht Stuttgart HRB 23118
Aufsichtsratsvorsitzender: Stefan Hartung; Gesch?ftsf?hrung: Gert van Iperen, Andreas Bartz, Thomas Quante, Bernhard Schuster

Subject: Re: [PATCH v5] Input: add bu21029 touch driver

Hi,

> [PATCH v5] Input: add bu21029 touch driver
>
> Add Rohm BU21029 resistive touch panel controller support with I2C
> interface.

Is the patch ready to be pushed upstream? Is there anything I still need to do?

Regards,
Mark

Mark Jonas

Building Technologies, Panel Software Fire (BT-FIR/ENG1)
Bosch Sicherheitssysteme GmbH | Postfach 11 11 | 85626 Grasbrunn | GERMANY | http://www.boschsecurity.com

Sitz: Stuttgart, Registergericht: Amtsgericht Stuttgart HRB 23118
Aufsichtsratsvorsitzender: Stefan Hartung; Gesch?ftsf?hrung: Gert van Iperen, Andreas Bartz, Thomas Quante, Bernhard Schuster

Subject: Re: [PATCH v5] Input: add bu21029 touch driver

Hello Dmitry,

> > [PATCH v5] Input: add bu21029 touch driver
> >
> > Add Rohm BU21029 resistive touch panel controller support with I2C
> > interface.
>
> Is the patch ready to be pushed upstream? Is there anything I still need to do?

I would like to kindly remind you of the BU21029 touch screen driver.
Could you please forward it to the mainline kernel?

Greetings,
Mark

Mark Jonas

Building Technologies, Panel Software Fire (BT-FIR/ENG1)
Bosch Sicherheitssysteme GmbH | Postfach 11 11 | 85626 Grasbrunn | GERMANY | http://www.boschsecurity.com

Sitz: Stuttgart, Registergericht: Amtsgericht Stuttgart HRB 23118
Aufsichtsratsvorsitzender: Stefan Hartung; Gesch?ftsf?hrung: Gert van Iperen, Andreas Bartz, Thomas Quante, Bernhard Schuster

2018-06-13 22:21:33

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH v5] Input: add bu21029 touch driver

Hi Mark,

On Wed, Jun 13, 2018 at 03:27:16PM +0000, Jonas Mark (BT-FIR/ENG1) wrote:
> Hello Dmitry,
>
> > > [PATCH v5] Input: add bu21029 touch driver
> > >
> > > Add Rohm BU21029 resistive touch panel controller support with I2C
> > > interface.
> >
> > Is the patch ready to be pushed upstream? Is there anything I still need to do?
>
> I would like to kindly remind you of the BU21029 touch screen driver.
> Could you please forward it to the mainline kernel?

Sorry for the delay. Could you please tell me if the patch below (shoudl
apply on top of your v5 version) works?

Thanks!

--
Dmitry


Input: bu - misc fixes

From: Dmitry Torokhov <[email protected]>

- add support for VDD supply
- reset GPIO is optional - it is not necessarily wired to AP
- add OF match table
- rudimentary handling of touchscreen as wakeup source
- removed bu21029_remove() as we have ->close method so touchscreen will be
disabled/powered down upon unregistration
- misc code rearrangements

Signed-off-by: Dmitry Torokhov <[email protected]>
---
.../bindings/input/touchscreen/bu21029.txt | 3
drivers/input/touchscreen/bu21029_ts.c | 325 ++++++++++----------
2 files changed, 169 insertions(+), 159 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
index 030a888365dff..1aaa647cc3eaa 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
@@ -5,13 +5,14 @@ Required properties:
- reg : i2c device address of the chip (0x40 or 0x41)
- interrupt-parent : the phandle for the gpio controller
- interrupts : (gpio) interrupt to which the chip is connected
- - reset-gpios : gpio pin to reset the chip (active low)
- rohm,x-plate-ohms : x-plate resistance in Ohm

Optional properties:
+ - reset-gpios : gpio pin to reset the chip (active low)
- touchscreen-size-x : horizontal resolution of touchscreen (in pixels)
- touchscreen-size-y : vertical resolution of touchscreen (in pixels)
- touchscreen-max-pressure: maximum pressure value
+ - vdd-supply : power supply for the controoler

Example:

diff --git a/drivers/input/touchscreen/bu21029_ts.c b/drivers/input/touchscreen/bu21029_ts.c
index 5a7267187e7b1..65469c8617630 100644
--- a/drivers/input/touchscreen/bu21029_ts.c
+++ b/drivers/input/touchscreen/bu21029_ts.c
@@ -16,6 +16,7 @@
#include <linux/input/touchscreen.h>
#include <linux/interrupt.h>
#include <linux/module.h>
+#include <linux/regulator/consumer.h>
#include <linux/timer.h>

/*
@@ -34,8 +35,8 @@
* HW_IDH: high 8bits of IC's ID
* HW_IDL: low 8bits of IC's ID
*/
-#define BU21029_HWID_REG (0x0E << 3)
-#define SUPPORTED_HWID 0x0229
+#define BU21029_HWID_REG (0x0E << 3)
+#define SUPPORTED_HWID 0x0229

/*
* CFR0 Register (PAGE=0, ADDR=0x00, Reset value=0x20)
@@ -49,8 +50,8 @@
* INTRM: 0 = INT output depend on "pen down" (*)
* 1 = INT output always "0"
*/
-#define BU21029_CFR0_REG (0x00 << 3)
-#define CFR0_VALUE 0x00
+#define BU21029_CFR0_REG (0x00 << 3)
+#define CFR0_VALUE 0x00

/*
* CFR1 Register (PAGE=0, ADDR=0x01, Reset value=0xA6)
@@ -65,8 +66,8 @@
* if AVE>SMPL, then AVE=SMPL (=3)
* SMPL: SMPL+1 = number of conversion samples for MAV (=7)
*/
-#define BU21029_CFR1_REG (0x01 << 3)
-#define CFR1_VALUE 0xA6
+#define BU21029_CFR1_REG (0x01 << 3)
+#define CFR1_VALUE 0xA6

/*
* CFR2 Register (PAGE=0, ADDR=0x02, Reset value=0x04)
@@ -81,8 +82,8 @@
* TIME_ST_ADC: waiting time between application of voltage
* to panel and start of A/D conversion (=100us)
*/
-#define BU21029_CFR2_REG (0x02 << 3)
-#define CFR2_VALUE 0xC9
+#define BU21029_CFR2_REG (0x02 << 3)
+#define CFR2_VALUE 0xC9

/*
* CFR3 Register (PAGE=0, ADDR=0x0B, Reset value=0x72)
@@ -102,8 +103,8 @@
* PIDAC_OFS: dual touch detection circuit adjustment, it is not necessary
* to change this from initial value
*/
-#define BU21029_CFR3_REG (0x0B << 3)
-#define CFR3_VALUE 0x42
+#define BU21029_CFR3_REG (0x0B << 3)
+#define CFR3_VALUE 0x42

/*
* LDO Register (PAGE=0, ADDR=0x0C, Reset value=0x00)
@@ -115,8 +116,8 @@
* PVDD: output voltage of panel output regulator (=2.000V)
* AVDD: output voltage of analog circuit regulator (=2.000V)
*/
-#define BU21029_LDO_REG (0x0C << 3)
-#define LDO_VALUE 0x77
+#define BU21029_LDO_REG (0x0C << 3)
+#define LDO_VALUE 0x77

/*
* Serial Interface Command Byte 1 (CID=1)
@@ -132,7 +133,7 @@
* 1 = keep power on after convert function stops
* STP: 1 = abort current conversion and power down, set to "0" automatically
*/
-#define BU21029_AUTOSCAN 0x80
+#define BU21029_AUTOSCAN 0x80

/*
* The timeout value needs to be larger than INTVL_TIME + tConv4 (sample and
@@ -140,40 +141,31 @@
* tPON + tDLY1 + (tTIME_ST_ADC + (tADC * tSMPL) * 2 + tDLY2) * 3
* see figure 8 in datasheet p15 for details of each field.
*/
-#define PEN_UP_TIMEOUT msecs_to_jiffies(50)
+#define PEN_UP_TIMEOUT_MS 50

-#define STOP_DELAY_MIN_US 50
-#define STOP_DELAY_MAX_US 1000
-#define START_DELAY_MS 2
-#define BUF_LEN 8
-#define SCALE_12BIT (1 << 12)
-#define MAX_12BIT ((1 << 12) - 1)
-#define DRIVER_NAME "bu21029"
+#define STOP_DELAY_MIN_US 50
+#define STOP_DELAY_MAX_US 1000
+#define START_DELAY_MS 2
+#define BUF_LEN 8
+#define SCALE_12BIT (1 << 12)
+#define MAX_12BIT ((1 << 12) - 1)
+#define DRIVER_NAME "bu21029"

struct bu21029_ts_data {
- struct i2c_client *client;
- struct input_dev *in_dev;
- struct timer_list timer;
- struct gpio_desc *reset_gpios;
- u32 x_plate_ohms;
- struct touchscreen_properties prop;
+ struct i2c_client *client;
+ struct input_dev *in_dev;
+ struct timer_list timer;
+ struct regulator *vdd;
+ struct gpio_desc *reset_gpios;
+ u32 x_plate_ohms;
+ struct touchscreen_properties prop;
};

-static int bu21029_touch_report(struct bu21029_ts_data *bu21029)
+static void bu21029_touch_report(struct bu21029_ts_data *bu21029, const u8 *buf)
{
- struct i2c_client *i2c = bu21029->client;
- u8 buf[BUF_LEN];
u16 x, y, z1, z2;
u32 rz;
- s32 max_pressure = bu21029->in_dev->absinfo[ABS_PRESSURE].maximum;
-
- /* read touch data and deassert INT (by restarting the autoscan mode) */
- int error = i2c_smbus_read_i2c_block_data(i2c,
- BU21029_AUTOSCAN,
- BUF_LEN,
- buf);
- if (error < 0)
- return error;
+ s32 max_pressure = input_abs_get_max(bu21029->in_dev, ABS_PRESSURE);

/*
* compose upper 8 and lower 4 bits into a 12bit value:
@@ -190,31 +182,28 @@ static int bu21029_touch_report(struct bu21029_ts_data *bu21029)
z1 = (buf[4] << 4) | (buf[5] >> 4);
z2 = (buf[6] << 4) | (buf[7] >> 4);

- if (z1 == 0 || z2 == 0)
- return 0;
-
- /*
- * calculate Rz (pressure resistance value) by equation:
- * Rz = Rx * (x/Q) * ((z2/z1) - 1), where
- * Rx is x-plate resistance,
- * Q is the touch screen resolution (8bit = 256, 12bit = 4096)
- * x, z1, z2 are the measured positions.
- */
- rz = z2 - z1;
- rz *= x;
- rz *= bu21029->x_plate_ohms;
- rz /= z1;
- rz = DIV_ROUND_CLOSEST(rz, SCALE_12BIT);
- if (rz <= max_pressure) {
- touchscreen_report_pos(bu21029->in_dev, &bu21029->prop,
- x, y, false);
- input_report_abs(bu21029->in_dev, ABS_PRESSURE,
- max_pressure - rz);
- input_report_key(bu21029->in_dev, BTN_TOUCH, 1);
- input_sync(bu21029->in_dev);
+ if (z1 && z2) {
+ /*
+ * calculate Rz (pressure resistance value) by equation:
+ * Rz = Rx * (x/Q) * ((z2/z1) - 1), where
+ * Rx is x-plate resistance,
+ * Q is the touch screen resolution (8bit = 256, 12bit = 4096)
+ * x, z1, z2 are the measured positions.
+ */
+ rz = z2 - z1;
+ rz *= x;
+ rz *= bu21029->x_plate_ohms;
+ rz /= z1;
+ rz = DIV_ROUND_CLOSEST(rz, SCALE_12BIT);
+ if (rz <= max_pressure) {
+ touchscreen_report_pos(bu21029->in_dev, &bu21029->prop,
+ x, y, false);
+ input_report_abs(bu21029->in_dev, ABS_PRESSURE,
+ max_pressure - rz);
+ input_report_key(bu21029->in_dev, BTN_TOUCH, 1);
+ input_sync(bu21029->in_dev);
+ }
}
-
- return 0;
}

static void bu21029_touch_release(struct timer_list *t)
@@ -229,32 +218,34 @@ static void bu21029_touch_release(struct timer_list *t)
static irqreturn_t bu21029_touch_soft_irq(int irq, void *data)
{
struct bu21029_ts_data *bu21029 = data;
+ u8 buf[BUF_LEN];
+ int error;

/*
- * report touch and deassert interrupt (will assert again after
+ * Read touch data and deassert interrupt (will assert again after
* INTVL_TIME + tConv4 for continuous touch)
*/
- int error = bu21029_touch_report(bu21029);
+ error = i2c_smbus_read_i2c_block_data(bu21029->client, BU21029_AUTOSCAN,
+ sizeof(buf), buf);
+ if (error < 0)
+ goto out;

- if (error)
- return IRQ_NONE;
+ bu21029_touch_report(bu21029, buf);

/* reset timer for pen up detection */
- mod_timer(&bu21029->timer, jiffies + PEN_UP_TIMEOUT);
+ mod_timer(&bu21029->timer,
+ jiffies + msecs_to_jiffies(PEN_UP_TIMEOUT_MS));

+out:
return IRQ_HANDLED;
}

-static void bu21029_stop_chip(struct input_dev *dev)
+static void bu21029_put_chip_in_reset(struct bu21029_ts_data *bu21029)
{
- struct bu21029_ts_data *bu21029 = input_get_drvdata(dev);
-
- disable_irq(bu21029->client->irq);
- del_timer_sync(&bu21029->timer);
-
- /* put chip into reset */
- gpiod_set_value_cansleep(bu21029->reset_gpios, 1);
- usleep_range(STOP_DELAY_MIN_US, STOP_DELAY_MAX_US);
+ if (bu21029->reset_gpios) {
+ gpiod_set_value_cansleep(bu21029->reset_gpios, 1);
+ usleep_range(STOP_DELAY_MIN_US, STOP_DELAY_MAX_US);
+ }
}

static int bu21029_start_chip(struct input_dev *dev)
@@ -274,23 +265,30 @@ static int bu21029_start_chip(struct input_dev *dev)
int error, i;
__be16 hwid;

+ error = regulator_enable(bu21029->vdd);
+ if (error) {
+ dev_err(&i2c->dev, "failed to power up chip: %d", error);
+ return error;
+ }
+
/* take chip out of reset */
- gpiod_set_value_cansleep(bu21029->reset_gpios, 0);
- msleep(START_DELAY_MS);
+ if (bu21029->reset_gpios) {
+ gpiod_set_value_cansleep(bu21029->reset_gpios, 0);
+ msleep(START_DELAY_MS);
+ }

- error = i2c_smbus_read_i2c_block_data(i2c,
- BU21029_HWID_REG,
- 2,
- (u8 *)&hwid);
+ error = i2c_smbus_read_i2c_block_data(i2c, BU21029_HWID_REG,
+ sizeof(hwid), (u8 *)&hwid);
if (error < 0) {
dev_err(&i2c->dev, "failed to read HW ID\n");
- goto out;
+ goto err_out;
}

if (be16_to_cpu(hwid) != SUPPORTED_HWID) {
- dev_err(&i2c->dev, "unsupported HW ID 0x%x\n", hwid);
+ dev_err(&i2c->dev,
+ "unsupported HW ID 0x%x\n", be16_to_cpu(hwid));
error = -ENODEV;
- goto out;
+ goto err_out;
}

for (i = 0; i < ARRAY_SIZE(init_table); ++i) {
@@ -299,50 +297,37 @@ static int bu21029_start_chip(struct input_dev *dev)
init_table[i].value);
if (error < 0) {
dev_err(&i2c->dev,
- "failed to write 0x%x to register 0x%x\n",
- init_table[i].value,
- init_table[i].reg);
- goto out;
+ "failed to write %#02x to register %#02x: %d\n",
+ init_table[i].value, init_table[i].reg,
+ error);
+ goto err_out;
}
}

error = i2c_smbus_write_byte(i2c, BU21029_AUTOSCAN);
if (error < 0) {
dev_err(&i2c->dev, "failed to start autoscan\n");
- goto out;
+ goto err_out;
}

enable_irq(bu21029->client->irq);
return 0;

-out:
- bu21029_stop_chip(dev);
+err_out:
+ bu21029_put_chip_in_reset(bu21029);
+ regulator_disable(bu21029->vdd);
return error;
}

-static int bu21029_parse_dt(struct bu21029_ts_data *bu21029)
+static void bu21029_stop_chip(struct input_dev *dev)
{
- struct device *dev = &bu21029->client->dev;
- int error;
-
- bu21029->reset_gpios = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
- if (IS_ERR(bu21029->reset_gpios)) {
- error = PTR_ERR(bu21029->reset_gpios);
- if (error != -EPROBE_DEFER)
- dev_err(dev, "invalid 'reset-gpios':%d\n", error);
- return error;
- }
-
- error = device_property_read_u32(dev, "rohm,x-plate-ohms",
- &bu21029->x_plate_ohms);
- if (error) {
- dev_err(dev, "invalid 'x-plate-ohms' supplied:%d\n", error);
- return error;
- }
+ struct bu21029_ts_data *bu21029 = input_get_drvdata(dev);

- touchscreen_parse_properties(bu21029->in_dev, false, &bu21029->prop);
+ disable_irq(bu21029->client->irq);
+ del_timer_sync(&bu21029->timer);

- return 0;
+ bu21029_put_chip_in_reset(bu21029);
+ regulator_disable(bu21029->vdd);
}

static int bu21029_probe(struct i2c_client *client,
@@ -365,6 +350,33 @@ static int bu21029_probe(struct i2c_client *client,
if (!bu21029)
return -ENOMEM;

+ error = device_property_read_u32(&client->dev, "rohm,x-plate-ohms",
+ &bu21029->x_plate_ohms);
+ if (error) {
+ dev_err(&client->dev,
+ "invalid 'x-plate-ohms' supplied: %d\n", error);
+ return error;
+ }
+
+ bu21029->vdd = devm_regulator_get(&client->dev, "vdd");
+ if (IS_ERR(bu21029->vdd)) {
+ error = PTR_ERR(bu21029->vdd);
+ if (error != -EPROBE_DEFER)
+ dev_err(&client->dev,
+ "failed to acquire 'vdd' supply: %d\n", error);
+ return error;
+ }
+
+ bu21029->reset_gpios = devm_gpiod_get_optional(&client->dev,
+ "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(bu21029->reset_gpios)) {
+ error = PTR_ERR(bu21029->reset_gpios);
+ if (error != -EPROBE_DEFER)
+ dev_err(&client->dev,
+ "failed to acquire 'reset' gpio: %d\n", error);
+ return error;
+ }
+
in_dev = devm_input_allocate_device(&client->dev);
if (!in_dev) {
dev_err(&client->dev, "unable to allocate input device\n");
@@ -372,42 +384,36 @@ static int bu21029_probe(struct i2c_client *client,
}

bu21029->client = client;
- bu21029->in_dev = in_dev;
+ bu21029->in_dev = in_dev;
timer_setup(&bu21029->timer, bu21029_touch_release, 0);

- in_dev->name = DRIVER_NAME;
- in_dev->id.bustype = BUS_I2C;
- in_dev->open = bu21029_start_chip;
- in_dev->close = bu21029_stop_chip;
+ in_dev->name = DRIVER_NAME;
+ in_dev->id.bustype = BUS_I2C;
+ in_dev->open = bu21029_start_chip;
+ in_dev->close = bu21029_stop_chip;

input_set_capability(in_dev, EV_KEY, BTN_TOUCH);
input_set_abs_params(in_dev, ABS_X, 0, MAX_12BIT, 0, 0);
input_set_abs_params(in_dev, ABS_Y, 0, MAX_12BIT, 0, 0);
input_set_abs_params(in_dev, ABS_PRESSURE, 0, MAX_12BIT, 0, 0);
-
- error = bu21029_parse_dt(bu21029);
- if (error)
- return error;
+ touchscreen_parse_properties(in_dev, false, &bu21029->prop);

input_set_drvdata(in_dev, bu21029);

- error = devm_request_threaded_irq(&client->dev,
- client->irq,
- NULL,
- bu21029_touch_soft_irq,
- IRQF_ONESHOT,
- DRIVER_NAME,
- bu21029);
+ irq_set_status_flags(client->irq, IRQ_NOAUTOEN);
+ error = devm_request_threaded_irq(&client->dev, client->irq,
+ NULL, bu21029_touch_soft_irq,
+ IRQF_ONESHOT, DRIVER_NAME, bu21029);
if (error) {
- dev_err(&client->dev, "unable to request touch irq\n");
+ dev_err(&client->dev,
+ "unable to request touch irq: %d\n", error);
return error;
}

- bu21029_stop_chip(in_dev);
-
error = input_register_device(in_dev);
if (error) {
- dev_err(&client->dev, "unable to register input device\n");
+ dev_err(&client->dev,
+ "unable to register input device: %d\n", error);
return error;
}

@@ -416,24 +422,17 @@ static int bu21029_probe(struct i2c_client *client,
return 0;
}

-static int bu21029_remove(struct i2c_client *client)
-{
- struct bu21029_ts_data *bu21029 = i2c_get_clientdata(client);
-
- bu21029_stop_chip(bu21029->in_dev);
-
- return 0;
-}
-
static int __maybe_unused bu21029_suspend(struct device *dev)
{
struct i2c_client *i2c = to_i2c_client(dev);
struct bu21029_ts_data *bu21029 = i2c_get_clientdata(i2c);

- mutex_lock(&bu21029->in_dev->mutex);
- if (bu21029->in_dev->users)
- bu21029_stop_chip(bu21029->in_dev);
- mutex_unlock(&bu21029->in_dev->mutex);
+ if (!device_may_wakeup(dev)) {
+ mutex_lock(&bu21029->in_dev->mutex);
+ if (bu21029->in_dev->users)
+ bu21029_stop_chip(bu21029->in_dev);
+ mutex_unlock(&bu21029->in_dev->mutex);
+ }

return 0;
}
@@ -443,29 +442,39 @@ static int __maybe_unused bu21029_resume(struct device *dev)
struct i2c_client *i2c = to_i2c_client(dev);
struct bu21029_ts_data *bu21029 = i2c_get_clientdata(i2c);

- mutex_lock(&bu21029->in_dev->mutex);
- if (bu21029->in_dev->users)
- bu21029_start_chip(bu21029->in_dev);
- mutex_unlock(&bu21029->in_dev->mutex);
+ if (!device_may_wakeup(dev)) {
+ mutex_lock(&bu21029->in_dev->mutex);
+ if (bu21029->in_dev->users)
+ bu21029_start_chip(bu21029->in_dev);
+ mutex_unlock(&bu21029->in_dev->mutex);
+ }

return 0;
}
static SIMPLE_DEV_PM_OPS(bu21029_pm_ops, bu21029_suspend, bu21029_resume);

static const struct i2c_device_id bu21029_ids[] = {
- {DRIVER_NAME, 0},
- {}
+ { DRIVER_NAME, 0 },
+ { /* sentinel */ }
};
MODULE_DEVICE_TABLE(i2c, bu21029_ids);

+#ifdef CONFIG_OF
+static const struct of_device_id bu21029_of_ids[] = {
+ { .compatible = "rohm,bu21029" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, bu21029_of_ids);
+#endif
+
static struct i2c_driver bu21029_driver = {
- .driver = {
- .name = DRIVER_NAME,
- .pm = &bu21029_pm_ops,
+ .driver = {
+ .name = DRIVER_NAME,
+ .of_match_table = of_match_ptr(bu21029_of_ids),
+ .pm = &bu21029_pm_ops,
},
- .id_table = bu21029_ids,
- .probe = bu21029_probe,
- .remove = bu21029_remove,
+ .id_table = bu21029_ids,
+ .probe = bu21029_probe,
};
module_i2c_driver(bu21029_driver);


Subject: AW: [PATCH v5] Input: add bu21029 touch driver

Hello Dmitry,

> Sorry for the delay. Could you please tell me if the patch below (shoudl
> apply on top of your v5 version) works?

The patch applies cleanly and we will give the code a thorough test.

> diff --git a/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
> b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
> index 030a888365dff..1aaa647cc3eaa 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
> @@ -5,13 +5,14 @@ Required properties:
> - reg : i2c device address of the chip (0x40 or 0x41)
> - interrupt-parent : the phandle for the gpio controller
> - interrupts : (gpio) interrupt to which the chip is connected
> - - reset-gpios : gpio pin to reset the chip (active low)
> - rohm,x-plate-ohms : x-plate resistance in Ohm
>
> Optional properties:
> + - reset-gpios : gpio pin to reset the chip (active low)
> - touchscreen-size-x : horizontal resolution of touchscreen (in pixels)
> - touchscreen-size-y : vertical resolution of touchscreen (in pixels)
> - touchscreen-max-pressure: maximum pressure value
> + - vdd-supply : power supply for the controoler

There is a typo in controller.

Greetings,
Mark

Building Technologies, Panel Software Fire (BT-FIR/ENG1)
Bosch Sicherheitssysteme GmbH | Postfach 11 11 | 85626 Grasbrunn | GERMANY | http://www.boschsecurity.com

Sitz: Stuttgart, Registergericht: Amtsgericht Stuttgart HRB 23118
Aufsichtsratsvorsitzender: Stefan Hartung; Gesch?ftsf?hrung: Gert van Iperen, Andreas Bartz, Thomas Quante, Bernhard Schuster

Subject: Re: [PATCH v5] Input: add bu21029 touch driver

Hi Dmitry,

> > > > [PATCH v5] Input: add bu21029 touch driver
> > > >
> > > > Add Rohm BU21029 resistive touch panel controller support with I2C
> > > > interface.
> > >
> > > Is the patch ready to be pushed upstream? Is there anything I still need to do?
> >
> > I would like to kindly remind you of the BU21029 touch screen driver.
> > Could you please forward it to the mainline kernel?
>
> Sorry for the delay. Could you please tell me if the patch below (shoudl
> apply on top of your v5 version) works?

Yes, the patch works. We applied and compiled it with 4.14 and
4.17-rc3. We tested on our hardware with 4.14.

Only for 4.14 an #include <linux/regulator/consumer.h> was missing. I
Guess the API changed here a little in the meantime.

Because we do not have a controllable power supply on our board we see
the following complaint. But I think it is intentional and ok, right?

[ 1.715963] bu21029 0-0040: 0-0040 supply vdd not found, using dummy regulator
[ 1.723806] input: bu21029 as /devices/soc0/soc/2100000.aips-bus/21a0000.i2c/i2c-0/0-0040/input/input0

> + - vdd-supply : power supply for the controoler

There is a typo in controller.

What's the next step?

Greetings,
Mark

Building Technologies, Panel Software Fire (BT-FIR/ENG1)
Bosch Sicherheitssysteme GmbH | Postfach 11 11 | 85626 Grasbrunn | GERMANY | http://www.boschsecurity.com

Sitz: Stuttgart, Registergericht: Amtsgericht Stuttgart HRB 23118
Aufsichtsratsvorsitzender: Stefan Hartung; Gesch?ftsf?hrung: Gert van Iperen, Andreas Bartz, Thomas Quante, Bernhard Schuster

2018-06-17 19:20:44

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3] Input: add bu21029 touch driver

Hi Zhu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v4.17-rc4]
[also build test WARNING on next-20180615]
[cannot apply to input/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Mark-Jonas/Input-add-bu21029-touch-driver/20180512-091305
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/input/touchscreen/bu21029_ts.c:293:13: sparse: restricted __be16 degrades to integer

vim +293 drivers/input/touchscreen/bu21029_ts.c

262
263 static int bu21029_start_chip(struct input_dev *dev)
264 {
265 struct bu21029_ts_data *bu21029 = input_get_drvdata(dev);
266 struct i2c_client *i2c = bu21029->client;
267 struct {
268 u8 reg;
269 u8 value;
270 } init_table[] = {
271 {BU21029_CFR0_REG, CFR0_VALUE},
272 {BU21029_CFR1_REG, CFR1_VALUE},
273 {BU21029_CFR2_REG, CFR2_VALUE},
274 {BU21029_CFR3_REG, CFR3_VALUE},
275 {BU21029_LDO_REG, LDO_VALUE}
276 };
277 int error, i;
278 u16 hwid;
279
280 /* take chip out of reset */
281 gpiod_set_value_cansleep(bu21029->reset_gpios, 0);
282 mdelay(START_DELAY_MS);
283
284 error = i2c_smbus_read_i2c_block_data(i2c,
285 BU21029_HWID_REG,
286 2,
287 (u8 *)&hwid);
288 if (error < 0) {
289 dev_err(&i2c->dev, "failed to read HW ID\n");
290 goto out;
291 }
292
> 293 if (cpu_to_be16(hwid) != SUPPORTED_HWID) {
294 dev_err(&i2c->dev, "unsupported HW ID 0x%x\n", hwid);
295 error = -ENODEV;
296 goto out;
297 }
298
299 for (i = 0; i < ARRAY_SIZE(init_table); ++i) {
300 error = i2c_smbus_write_byte_data(i2c,
301 init_table[i].reg,
302 init_table[i].value);
303 if (error < 0) {
304 dev_err(&i2c->dev,
305 "failed to write 0x%x to register 0x%x\n",
306 init_table[i].value,
307 init_table[i].reg);
308 goto out;
309 }
310 }
311
312 error = i2c_smbus_write_byte(i2c, BU21029_AUTOSCAN);
313 if (error < 0) {
314 dev_err(&i2c->dev, "failed to start autoscan\n");
315 goto out;
316 }
317
318 enable_irq(bu21029->client->irq);
319 return 0;
320
321 out:
322 bu21029_stop_chip(dev);
323 return error;
324 }
325

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

Subject: Re: [PATCH v3] Input: add bu21029 touch driver

Hi kbuild test robot,

> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on v4.17-rc4]
> [also build test WARNING on next-20180615]
> [cannot apply to input/next]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Mark-Jonas/Input-add-bu21029-
> touch-driver/20180512-091305
> reproduce:
> # apt-get install sparse
> make ARCH=x86_64 allmodconfig
> make C=1 CF=-D__CHECK_ENDIAN__
>
>
> sparse warnings: (new ones prefixed by >>)
>
> >> drivers/input/touchscreen/bu21029_ts.c:293:13: sparse: restricted __be16
> degrades to integer
[..]
> 288 if (error < 0) {
> 289 dev_err(&i2c->dev, "failed to read HW ID\n");
> 290 goto out;
> 291 }
> 292
> > 293 if (cpu_to_be16(hwid) != SUPPORTED_HWID) {
> 294 dev_err(&i2c->dev, "unsupported HW ID 0x%x\n", hwid);
> 295 error = -ENODEV;
> 296 goto out;
> 297 }

Thank you for your effort. The reported problem was already fixed.
Please have a look at the most recent v5 patch posted on this list.

Greetings,
Mark

Mark Jonas

Building Technologies, Panel Software Fire (BT-FIR/ENG1)
Bosch Sicherheitssysteme GmbH | Postfach 11 11 | 85626 Grasbrunn | GERMANY | http://www.boschsecurity.com

Sitz: Stuttgart, Registergericht: Amtsgericht Stuttgart HRB 23118
Aufsichtsratsvorsitzender: Stefan Hartung; Gesch?ftsf?hrung: Gert van Iperen, Andreas Bartz, Thomas Quante, Bernhard Schuster