[PATCH 1/5] Input: zinitix - Helper dev variable in probe()

[PATCH 1/5] Input: zinitix - Helper dev variable in probe()

[Linus Walleij]

Create a helper variable struct device *dev in probe() to
make the code more compact and easier to read.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/input/touchscreen/zinitix.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
index 8bd03278ad9a..cd13075ae3ab 100644
--- a/drivers/input/touchscreen/zinitix.c
+++ b/drivers/input/touchscreen/zinitix.c
@@ -503,15 +503,16 @@ static int zinitix_init_input_dev(struct bt541_ts_data *bt541)
 static int zinitix_ts_probe(struct i2c_client *client)
 {
 	struct bt541_ts_data *bt541;
+	struct device *dev = &client->dev;
 	int error;
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
-		dev_err(&client->dev,
+		dev_err(dev,
 			"Failed to assert adapter's support for plain I2C.\n");
 		return -ENXIO;
 	}
 
-	bt541 = devm_kzalloc(&client->dev, sizeof(*bt541), GFP_KERNEL);
+	bt541 = devm_kzalloc(dev, sizeof(*bt541), GFP_KERNEL);
 	if (!bt541)
 		return -ENOMEM;
 
@@ -520,28 +521,28 @@ static int zinitix_ts_probe(struct i2c_client *client)
 
 	error = zinitix_init_regulators(bt541);
 	if (error) {
-		dev_err(&client->dev,
+		dev_err(dev,
 			"Failed to initialize regulators: %d\n", error);
 		return error;
 	}
 
-	error = devm_request_threaded_irq(&client->dev, client->irq,
+	error = devm_request_threaded_irq(dev, client->irq,
 					  NULL, zinitix_ts_irq_handler,
 					  IRQF_ONESHOT | IRQF_NO_AUTOEN,
 					  client->name, bt541);
 	if (error) {
-		dev_err(&client->dev, "Failed to request IRQ: %d\n", error);
+		dev_err(dev, "Failed to request IRQ: %d\n", error);
 		return error;
 	}
 
 	error = zinitix_init_input_dev(bt541);
 	if (error) {
-		dev_err(&client->dev,
+		dev_err(dev,
 			"Failed to initialize input device: %d\n", error);
 		return error;
 	}
 
-	error = device_property_read_u32(&client->dev, "zinitix,mode",
+	error = device_property_read_u32(dev, "zinitix,mode",
 					 &bt541->zinitix_mode);
 	if (error < 0) {
 		/* fall back to mode 2 */
@@ -553,7 +554,7 @@ static int zinitix_ts_probe(struct i2c_client *client)
 		 * If there are devices that don't support mode 2, support
 		 * for other modes (0, 1) will be needed.
 		 */
-		dev_err(&client->dev,
+		dev_err(dev,
 			"Malformed zinitix,mode property, must be 2 (supplied: %d)\n",
 			bt541->zinitix_mode);
 		return -EINVAL;
-- 
2.35.1

[PATCH 2/5] Input: zinitix - Add dev variable in state

[Linus Walleij]

To avoid several steps of dereferencing the struct device from
the client, add a struct device *dev pointer to the state
container so we can easily get to the struct device. This makes
the code more compact and easier to read.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/input/touchscreen/zinitix.c | 37 +++++++++++++++--------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
index cd13075ae3ab..11af1264cafa 100644
--- a/drivers/input/touchscreen/zinitix.c
+++ b/drivers/input/touchscreen/zinitix.c
@@ -142,6 +142,7 @@ struct touch_event {
 
 struct bt541_ts_data {
 	struct i2c_client *client;
+	struct device *dev;
 	struct input_dev *input_dev;
 	struct touchscreen_properties prop;
 	struct regulator_bulk_data supplies[2];
@@ -198,13 +199,13 @@ static int zinitix_init_touch(struct bt541_ts_data *bt541)
 
 	error = zinitix_write_cmd(client, BT541_SWRESET_CMD);
 	if (error) {
-		dev_err(&client->dev, "Failed to write reset command\n");
+		dev_err(bt541->dev, "Failed to write reset command\n");
 		return error;
 	}
 
 	error = zinitix_write_u16(client, BT541_INT_ENABLE_FLAG, 0x0);
 	if (error) {
-		dev_err(&client->dev,
+		dev_err(bt541->dev,
 			"Failed to reset interrupt enable flag\n");
 		return error;
 	}
@@ -252,7 +253,7 @@ static int zinitix_init_touch(struct bt541_ts_data *bt541)
 
 static int zinitix_init_regulators(struct bt541_ts_data *bt541)
 {
-	struct device *dev = &bt541->client->dev;
+	struct device *dev = bt541->dev;
 	int error;
 
 	/*
@@ -286,7 +287,7 @@ static int zinitix_send_power_on_sequence(struct bt541_ts_data *bt541)
 
 	error = zinitix_write_u16(client, 0xc000, 0x0001);
 	if (error) {
-		dev_err(&client->dev,
+		dev_err(bt541->dev,
 			"Failed to send power sequence(vendor cmd enable)\n");
 		return error;
 	}
@@ -294,7 +295,7 @@ static int zinitix_send_power_on_sequence(struct bt541_ts_data *bt541)
 
 	error = zinitix_write_cmd(client, 0xc004);
 	if (error) {
-		dev_err(&client->dev,
+		dev_err(bt541->dev,
 			"Failed to send power sequence (intn clear)\n");
 		return error;
 	}
@@ -302,7 +303,7 @@ static int zinitix_send_power_on_sequence(struct bt541_ts_data *bt541)
 
 	error = zinitix_write_u16(client, 0xc002, 0x0001);
 	if (error) {
-		dev_err(&client->dev,
+		dev_err(bt541->dev,
 			"Failed to send power sequence (nvm init)\n");
 		return error;
 	}
@@ -310,7 +311,7 @@ static int zinitix_send_power_on_sequence(struct bt541_ts_data *bt541)
 
 	error = zinitix_write_u16(client, 0xc001, 0x0001);
 	if (error) {
-		dev_err(&client->dev,
+		dev_err(bt541->dev,
 			"Failed to send power sequence (program start)\n");
 		return error;
 	}
@@ -353,7 +354,6 @@ static void zinitix_report_finger(struct bt541_ts_data *bt541, int slot,
 static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler)
 {
 	struct bt541_ts_data *bt541 = bt541_handler;
-	struct i2c_client *client = bt541->client;
 	struct touch_event touch_event;
 	unsigned long finger_mask;
 	int error;
@@ -364,7 +364,7 @@ static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler)
 	error = zinitix_read_data(bt541->client, BT541_POINT_STATUS_REG,
 				  &touch_event, sizeof(struct touch_event));
 	if (error) {
-		dev_err(&client->dev, "Failed to read in touchpoint struct\n");
+		dev_err(bt541->dev, "Failed to read in touchpoint struct\n");
 		goto out;
 	}
 
@@ -392,7 +392,7 @@ static int zinitix_start(struct bt541_ts_data *bt541)
 	error = regulator_bulk_enable(ARRAY_SIZE(bt541->supplies),
 				      bt541->supplies);
 	if (error) {
-		dev_err(&bt541->client->dev,
+		dev_err(bt541->dev,
 			"Failed to enable regulators: %d\n", error);
 		return error;
 	}
@@ -401,14 +401,14 @@ static int zinitix_start(struct bt541_ts_data *bt541)
 
 	error = zinitix_send_power_on_sequence(bt541);
 	if (error) {
-		dev_err(&bt541->client->dev,
+		dev_err(bt541->dev,
 			"Error while sending power-on sequence: %d\n", error);
 		return error;
 	}
 
 	error = zinitix_init_touch(bt541);
 	if (error) {
-		dev_err(&bt541->client->dev,
+		dev_err(bt541->dev,
 			"Error while configuring touch IC\n");
 		return error;
 	}
@@ -427,7 +427,7 @@ static int zinitix_stop(struct bt541_ts_data *bt541)
 	error = regulator_bulk_disable(ARRAY_SIZE(bt541->supplies),
 				       bt541->supplies);
 	if (error) {
-		dev_err(&bt541->client->dev,
+		dev_err(bt541->dev,
 			"Failed to disable regulators: %d\n", error);
 		return error;
 	}
@@ -454,9 +454,9 @@ static int zinitix_init_input_dev(struct bt541_ts_data *bt541)
 	struct input_dev *input_dev;
 	int error;
 
-	input_dev = devm_input_allocate_device(&bt541->client->dev);
+	input_dev = devm_input_allocate_device(bt541->dev);
 	if (!input_dev) {
-		dev_err(&bt541->client->dev,
+		dev_err(bt541->dev,
 			"Failed to allocate input device.");
 		return -ENOMEM;
 	}
@@ -477,7 +477,7 @@ static int zinitix_init_input_dev(struct bt541_ts_data *bt541)
 
 	touchscreen_parse_properties(input_dev, true, &bt541->prop);
 	if (!bt541->prop.max_x || !bt541->prop.max_y) {
-		dev_err(&bt541->client->dev,
+		dev_err(bt541->dev,
 			"Touchscreen-size-x and/or touchscreen-size-y not set in dts\n");
 		return -EINVAL;
 	}
@@ -485,14 +485,14 @@ static int zinitix_init_input_dev(struct bt541_ts_data *bt541)
 	error = input_mt_init_slots(input_dev, MAX_SUPPORTED_FINGER_NUM,
 				    INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
 	if (error) {
-		dev_err(&bt541->client->dev,
+		dev_err(bt541->dev,
 			"Failed to initialize MT slots: %d", error);
 		return error;
 	}
 
 	error = input_register_device(input_dev);
 	if (error) {
-		dev_err(&bt541->client->dev,
+		dev_err(bt541->dev,
 			"Failed to register input device: %d", error);
 		return error;
 	}
@@ -517,6 +517,7 @@ static int zinitix_ts_probe(struct i2c_client *client)
 		return -ENOMEM;
 
 	bt541->client = client;
+	bt541->dev = dev;
 	i2c_set_clientdata(client, bt541);
 
 	error = zinitix_init_regulators(bt541);
-- 
2.35.1

Re: [PATCH 2/5] Input: zinitix - Add dev variable in state

[Dmitry Torokhov]

On Sun, Apr 10, 2022 at 02:00:56PM +0200, Linus Walleij wrote:
> To avoid several steps of dereferencing the struct device from
> the client, add a struct device *dev pointer to the state
> container so we can easily get to the struct device. This makes
> the code more compact and easier to read.

Same concern as the previous patch...

Thanks.

-- 
Dmitry

Re: [PATCH 2/5] Input: zinitix - Add dev variable in state

[Linus Walleij]

On Fri, May 27, 2022 at 7:37 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Sun, Apr 10, 2022 at 02:00:56PM +0200, Linus Walleij wrote:
> > To avoid several steps of dereferencing the struct device from
> > the client, add a struct device *dev pointer to the state
> > container so we can easily get to the struct device. This makes
> > the code more compact and easier to read.
>
> Same concern as the previous patch...

No problem, I drop this too.

Yours,
Linus Walleij

[PATCH 3/5] Input: zinitix - Rename defines ZINITIX_*

[Linus Walleij]

The defines are sometimes named BT541_* and sometimes
ZINITIX_*, name them all ZINITIX_* because they certainly
apply to a lot more touchscreens than the BT541.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/input/touchscreen/zinitix.c | 112 ++++++++++++++--------------
 1 file changed, 56 insertions(+), 56 deletions(-)

diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
index 11af1264cafa..f89dea49cf1f 100644
--- a/drivers/input/touchscreen/zinitix.c
+++ b/drivers/input/touchscreen/zinitix.c
@@ -15,75 +15,75 @@
 
 /* Register Map */
 
-#define BT541_SWRESET_CMD			0x0000
-#define BT541_WAKEUP_CMD			0x0001
+#define ZINITIX_SWRESET_CMD			0x0000
+#define ZINITIX_WAKEUP_CMD			0x0001
 
-#define BT541_IDLE_CMD				0x0004
-#define BT541_SLEEP_CMD				0x0005
+#define ZINITIX_IDLE_CMD			0x0004
+#define ZINITIX_SLEEP_CMD			0x0005
 
-#define BT541_CLEAR_INT_STATUS_CMD		0x0003
-#define BT541_CALIBRATE_CMD			0x0006
-#define BT541_SAVE_STATUS_CMD			0x0007
-#define BT541_SAVE_CALIBRATION_CMD		0x0008
-#define BT541_RECALL_FACTORY_CMD		0x000f
+#define ZINITIX_CLEAR_INT_STATUS_CMD		0x0003
+#define ZINITIX_CALIBRATE_CMD			0x0006
+#define ZINITIX_SAVE_STATUS_CMD			0x0007
+#define ZINITIX_SAVE_CALIBRATION_CMD		0x0008
+#define ZINITIX_RECALL_FACTORY_CMD		0x000f
 
-#define BT541_THRESHOLD				0x0020
+#define ZINITIX_THRESHOLD			0x0020
 
-#define BT541_LARGE_PALM_REJECT_AREA_TH		0x003F
+#define ZINITIX_LARGE_PALM_REJECT_AREA_TH	0x003F
 
-#define BT541_DEBUG_REG				0x0115 /* 0~7 */
+#define ZINITIX_DEBUG_REG			0x0115 /* 0~7 */
 
-#define BT541_TOUCH_MODE			0x0010
-#define BT541_CHIP_REVISION			0x0011
-#define BT541_FIRMWARE_VERSION			0x0012
+#define ZINITIX_TOUCH_MODE			0x0010
+#define ZINITIX_CHIP_REVISION			0x0011
+#define ZINITIX_FIRMWARE_VERSION		0x0012
 
 #define ZINITIX_USB_DETECT			0x116
 
-#define BT541_MINOR_FW_VERSION			0x0121
+#define ZINITIX_MINOR_FW_VERSION		0x0121
 
-#define BT541_VENDOR_ID				0x001C
-#define BT541_HW_ID				0x0014
+#define ZINITIX_VENDOR_ID			0x001C
+#define ZINITIX_HW_ID				0x0014
 
-#define BT541_DATA_VERSION_REG			0x0013
-#define BT541_SUPPORTED_FINGER_NUM		0x0015
-#define BT541_EEPROM_INFO			0x0018
-#define BT541_INITIAL_TOUCH_MODE		0x0019
+#define ZINITIX_DATA_VERSION_REG		0x0013
+#define ZINITIX_SUPPORTED_FINGER_NUM		0x0015
+#define ZINITIX_EEPROM_INFO			0x0018
+#define ZINITIX_INITIAL_TOUCH_MODE		0x0019
 
-#define BT541_TOTAL_NUMBER_OF_X			0x0060
-#define BT541_TOTAL_NUMBER_OF_Y			0x0061
+#define ZINITIX_TOTAL_NUMBER_OF_X		0x0060
+#define ZINITIX_TOTAL_NUMBER_OF_Y		0x0061
 
-#define BT541_DELAY_RAW_FOR_HOST		0x007f
+#define ZINITIX_DELAY_RAW_FOR_HOST		0x007f
 
-#define BT541_BUTTON_SUPPORTED_NUM		0x00B0
-#define BT541_BUTTON_SENSITIVITY		0x00B2
-#define BT541_DUMMY_BUTTON_SENSITIVITY		0X00C8
+#define ZINITIX_BUTTON_SUPPORTED_NUM		0x00B0
+#define ZINITIX_BUTTON_SENSITIVITY		0x00B2
+#define ZINITIX_DUMMY_BUTTON_SENSITIVITY	0X00C8
 
-#define BT541_X_RESOLUTION			0x00C0
-#define BT541_Y_RESOLUTION			0x00C1
+#define ZINITIX_X_RESOLUTION			0x00C0
+#define ZINITIX_Y_RESOLUTION			0x00C1
 
-#define BT541_POINT_STATUS_REG			0x0080
-#define BT541_ICON_STATUS_REG			0x00AA
+#define ZINITIX_POINT_STATUS_REG		0x0080
+#define ZINITIX_ICON_STATUS_REG			0x00AA
 
-#define BT541_POINT_COORD_REG			(BT541_POINT_STATUS_REG + 2)
+#define ZINITIX_POINT_COORD_REG			(ZINITIX_POINT_STATUS_REG + 2)
 
-#define BT541_AFE_FREQUENCY			0x0100
-#define BT541_DND_N_COUNT			0x0122
-#define BT541_DND_U_COUNT			0x0135
+#define ZINITIX_AFE_FREQUENCY			0x0100
+#define ZINITIX_DND_N_COUNT			0x0122
+#define ZINITIX_DND_U_COUNT			0x0135
 
-#define BT541_RAWDATA_REG			0x0200
+#define ZINITIX_RAWDATA_REG			0x0200
 
-#define BT541_EEPROM_INFO_REG			0x0018
+#define ZINITIX_EEPROM_INFO_REG			0x0018
 
-#define BT541_INT_ENABLE_FLAG			0x00f0
-#define BT541_PERIODICAL_INTERRUPT_INTERVAL	0x00f1
+#define ZINITIX_INT_ENABLE_FLAG			0x00f0
+#define ZINITIX_PERIODICAL_INTERRUPT_INTERVAL	0x00f1
 
-#define BT541_BTN_WIDTH				0x016d
+#define ZINITIX_BTN_WIDTH			0x016d
 
-#define BT541_CHECKSUM_RESULT			0x012c
+#define ZINITIX_CHECKSUM_RESULT			0x012c
 
-#define BT541_INIT_FLASH			0x01d0
-#define BT541_WRITE_FLASH			0x01d1
-#define BT541_READ_FLASH			0x01d2
+#define ZINITIX_INIT_FLASH			0x01d0
+#define ZINITIX_WRITE_FLASH			0x01d1
+#define ZINITIX_READ_FLASH			0x01d2
 
 #define ZINITIX_INTERNAL_FLAG_02		0x011e
 #define ZINITIX_INTERNAL_FLAG_03		0x011f
@@ -197,13 +197,13 @@ static int zinitix_init_touch(struct bt541_ts_data *bt541)
 	int i;
 	int error;
 
-	error = zinitix_write_cmd(client, BT541_SWRESET_CMD);
+	error = zinitix_write_cmd(client, ZINITIX_SWRESET_CMD);
 	if (error) {
 		dev_err(bt541->dev, "Failed to write reset command\n");
 		return error;
 	}
 
-	error = zinitix_write_u16(client, BT541_INT_ENABLE_FLAG, 0x0);
+	error = zinitix_write_u16(client, ZINITIX_INT_ENABLE_FLAG, 0x0);
 	if (error) {
 		dev_err(bt541->dev,
 			"Failed to reset interrupt enable flag\n");
@@ -211,32 +211,32 @@ static int zinitix_init_touch(struct bt541_ts_data *bt541)
 	}
 
 	/* initialize */
-	error = zinitix_write_u16(client, BT541_X_RESOLUTION,
+	error = zinitix_write_u16(client, ZINITIX_X_RESOLUTION,
 				  bt541->prop.max_x);
 	if (error)
 		return error;
 
-	error = zinitix_write_u16(client, BT541_Y_RESOLUTION,
+	error = zinitix_write_u16(client, ZINITIX_Y_RESOLUTION,
 				  bt541->prop.max_y);
 	if (error)
 		return error;
 
-	error = zinitix_write_u16(client, BT541_SUPPORTED_FINGER_NUM,
+	error = zinitix_write_u16(client, ZINITIX_SUPPORTED_FINGER_NUM,
 				  MAX_SUPPORTED_FINGER_NUM);
 	if (error)
 		return error;
 
-	error = zinitix_write_u16(client, BT541_INITIAL_TOUCH_MODE,
+	error = zinitix_write_u16(client, ZINITIX_INITIAL_TOUCH_MODE,
 				  bt541->zinitix_mode);
 	if (error)
 		return error;
 
-	error = zinitix_write_u16(client, BT541_TOUCH_MODE,
+	error = zinitix_write_u16(client, ZINITIX_TOUCH_MODE,
 				  bt541->zinitix_mode);
 	if (error)
 		return error;
 
-	error = zinitix_write_u16(client, BT541_INT_ENABLE_FLAG,
+	error = zinitix_write_u16(client, ZINITIX_INT_ENABLE_FLAG,
 				  BIT_PT_CNT_CHANGE | BIT_DOWN | BIT_MOVE |
 					BIT_UP);
 	if (error)
@@ -244,7 +244,7 @@ static int zinitix_init_touch(struct bt541_ts_data *bt541)
 
 	/* clear queue */
 	for (i = 0; i < 10; i++) {
-		zinitix_write_cmd(client, BT541_CLEAR_INT_STATUS_CMD);
+		zinitix_write_cmd(client, ZINITIX_CLEAR_INT_STATUS_CMD);
 		udelay(10);
 	}
 
@@ -361,7 +361,7 @@ static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler)
 
 	memset(&touch_event, 0, sizeof(struct touch_event));
 
-	error = zinitix_read_data(bt541->client, BT541_POINT_STATUS_REG,
+	error = zinitix_read_data(bt541->client, ZINITIX_POINT_STATUS_REG,
 				  &touch_event, sizeof(struct touch_event));
 	if (error) {
 		dev_err(bt541->dev, "Failed to read in touchpoint struct\n");
@@ -381,7 +381,7 @@ static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler)
 	input_sync(bt541->input_dev);
 
 out:
-	zinitix_write_cmd(bt541->client, BT541_CLEAR_INT_STATUS_CMD);
+	zinitix_write_cmd(bt541->client, ZINITIX_CLEAR_INT_STATUS_CMD);
 	return IRQ_HANDLED;
 }
 
-- 
2.35.1

Re: [PATCH 3/5] Input: zinitix - Rename defines ZINITIX_*

[Dmitry Torokhov]

On Sun, Apr 10, 2022 at 02:00:57PM +0200, Linus Walleij wrote:
> The defines are sometimes named BT541_* and sometimes
> ZINITIX_*, name them all ZINITIX_* because they certainly
> apply to a lot more touchscreens than the BT541.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

I'll apply this, thank you.

-- 
Dmitry

[PATCH 4/5] Input: zinitix - Read and cache device version numbers

[Linus Walleij]

The chip hardware revision, firmware version and regdata
revision is useful to know, so read this out and print it
the first time we open the device. Example output from
BT404:

Zinitix-TS 3-0020: chip revision 4040 firmware version 0088
  regdata version 0004

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/input/touchscreen/zinitix.c | 37 +++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
index f89dea49cf1f..ea807b972f26 100644
--- a/drivers/input/touchscreen/zinitix.c
+++ b/drivers/input/touchscreen/zinitix.c
@@ -35,6 +35,9 @@
 
 #define ZINITIX_TOUCH_MODE			0x0010
 #define ZINITIX_CHIP_REVISION			0x0011
+#define ZINITIX_CHIP_BT404			0x4040
+#define ZINITIX_CHIP_BT532			0x5320
+#define ZINITIX_CHIP_BT541			0x5410
 #define ZINITIX_FIRMWARE_VERSION		0x0012
 
 #define ZINITIX_USB_DETECT			0x116
@@ -147,6 +150,9 @@ struct bt541_ts_data {
 	struct touchscreen_properties prop;
 	struct regulator_bulk_data supplies[2];
 	u32 zinitix_mode;
+	u16 chip_revision;
+	u16 firmware_version;
+	u16 regdata_version;
 };
 
 static int zinitix_read_data(struct i2c_client *client,
@@ -191,11 +197,24 @@ static int zinitix_write_cmd(struct i2c_client *client, u16 reg)
 	return 0;
 }
 
+static u16 zinitix_get_u16_reg(struct bt541_ts_data *bt541, u16 vreg)
+{
+	struct i2c_client *client = bt541->client;
+	int error;
+	__le16 val;
+
+	error = zinitix_read_data(client, vreg, (void *)&val, 2);
+	if (error)
+		return U8_MAX;
+        return le16_to_cpu(val);
+}
+
 static int zinitix_init_touch(struct bt541_ts_data *bt541)
 {
 	struct i2c_client *client = bt541->client;
 	int i;
 	int error;
+	static bool read_static = false;
 
 	error = zinitix_write_cmd(client, ZINITIX_SWRESET_CMD);
 	if (error) {
@@ -203,6 +222,24 @@ static int zinitix_init_touch(struct bt541_ts_data *bt541)
 		return error;
 	}
 
+	/*
+	 * Read and cache the chip revision and firmware version the first time
+	 * we get here.
+	 */
+	if (!read_static) {
+		bt541->chip_revision = zinitix_get_u16_reg(bt541,
+					ZINITIX_CHIP_REVISION);
+		bt541->firmware_version = zinitix_get_u16_reg(bt541,
+					ZINITIX_FIRMWARE_VERSION);
+		bt541->regdata_version = zinitix_get_u16_reg(bt541,
+					ZINITIX_DATA_VERSION_REG);
+		dev_info(bt541->dev,
+			 "chip revision %04x firmware version %04x regdata version %04x\n",
+			 bt541->chip_revision, bt541->firmware_version,
+			 bt541->regdata_version);
+		read_static = true;
+	}
+
 	error = zinitix_write_u16(client, ZINITIX_INT_ENABLE_FLAG, 0x0);
 	if (error) {
 		dev_err(bt541->dev,
-- 
2.35.1

Re: [PATCH 4/5] Input: zinitix - Read and cache device version numbers

[Dmitry Torokhov]

On Sun, Apr 10, 2022 at 02:00:58PM +0200, Linus Walleij wrote:
>  static int zinitix_init_touch(struct bt541_ts_data *bt541)
>  {
>  	struct i2c_client *client = bt541->client;
>  	int i;
>  	int error;
> +	static bool read_static = false;

This needs to be per-device state.

>  
>  	error = zinitix_write_cmd(client, ZINITIX_SWRESET_CMD);
>  	if (error) {
> @@ -203,6 +222,24 @@ static int zinitix_init_touch(struct bt541_ts_data *bt541)
>  		return error;
>  	}
>  
> +	/*
> +	 * Read and cache the chip revision and firmware version the first time
> +	 * we get here.
> +	 */
> +	if (!read_static) {
> +		bt541->chip_revision = zinitix_get_u16_reg(bt541,
> +					ZINITIX_CHIP_REVISION);
> +		bt541->firmware_version = zinitix_get_u16_reg(bt541,
> +					ZINITIX_FIRMWARE_VERSION);
> +		bt541->regdata_version = zinitix_get_u16_reg(bt541,
> +					ZINITIX_DATA_VERSION_REG);
> +		dev_info(bt541->dev,
> +			 "chip revision %04x firmware version %04x regdata version %04x\n",
> +			 bt541->chip_revision, bt541->firmware_version,
> +			 bt541->regdata_version);

Is this only for bringup efforts?

> +		read_static = true;
> +	}
> +
>  	error = zinitix_write_u16(client, ZINITIX_INT_ENABLE_FLAG, 0x0);
>  	if (error) {
>  		dev_err(bt541->dev,
> -- 
> 2.35.1
> 

Thanks.

-- 
Dmitry

[PATCH 5/5] Input: zinitix - Support calibration

[Linus Walleij]

Support both hardware and software calibration on the Zinitix
touchscreens. This is lifted from the vendor driver but rewritten
to use the chip revision number in the hardware instead of
compile-time defines.

Use defines for the new "calibration mode" and add a new define
for the default touchpoint mode.

The registers for reading the EEPROM status actually differs
between BT5x2 and BT4x3+ so create very unique defines for
those registers and use a register variable in the state to
keep track of the right one to use.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/input/touchscreen/zinitix.c | 138 ++++++++++++++++++++++++++--
 1 file changed, 132 insertions(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
index ea807b972f26..440ffc7c9b32 100644
--- a/drivers/input/touchscreen/zinitix.c
+++ b/drivers/input/touchscreen/zinitix.c
@@ -34,6 +34,8 @@
 #define ZINITIX_DEBUG_REG			0x0115 /* 0~7 */
 
 #define ZINITIX_TOUCH_MODE			0x0010
+#define ZINITIX_MODE_TOUCH_POINT		0x02
+#define ZINITIX_MODE_CALIBRATION		0x07
 #define ZINITIX_CHIP_REVISION			0x0011
 #define ZINITIX_CHIP_BT404			0x4040
 #define ZINITIX_CHIP_BT532			0x5320
@@ -49,7 +51,11 @@
 
 #define ZINITIX_DATA_VERSION_REG		0x0013
 #define ZINITIX_SUPPORTED_FINGER_NUM		0x0015
-#define ZINITIX_EEPROM_INFO			0x0018
+
+#define BT4X2_EEPROM_INFO			0x0014
+#define BT4X3_EEPROM_INFO			0x0018
+#define ZINITIX_EEPROM_HW_CALIB			BIT(0)
+
 #define ZINITIX_INITIAL_TOUCH_MODE		0x0019
 
 #define ZINITIX_TOTAL_NUMBER_OF_X		0x0060
@@ -57,6 +63,8 @@
 
 #define ZINITIX_DELAY_RAW_FOR_HOST		0x007f
 
+#define BT4X2_EEPROM_INFO_REG			0x00aa
+
 #define ZINITIX_BUTTON_SUPPORTED_NUM		0x00B0
 #define ZINITIX_BUTTON_SENSITIVITY		0x00B2
 #define ZINITIX_DUMMY_BUTTON_SENSITIVITY	0X00C8
@@ -75,8 +83,6 @@
 
 #define ZINITIX_RAWDATA_REG			0x0200
 
-#define ZINITIX_EEPROM_INFO_REG			0x0018
-
 #define ZINITIX_INT_ENABLE_FLAG			0x00f0
 #define ZINITIX_PERIODICAL_INTERRUPT_INTERVAL	0x00f1
 
@@ -120,7 +126,6 @@
 #define SUB_BIT_UPDATE				BIT(4)
 #define SUB_BIT_WAIT				BIT(5)
 
-#define DEFAULT_TOUCH_POINT_MODE		2
 #define MAX_SUPPORTED_FINGER_NUM		5
 
 #define CHIP_ON_DELAY				15 // ms
@@ -153,8 +158,33 @@ struct bt541_ts_data {
 	u16 chip_revision;
 	u16 firmware_version;
 	u16 regdata_version;
+	u16 eeprom_info_reg;
 };
 
+static bool zinitix_is_4x2(struct bt541_ts_data *bt541)
+{
+	u16 ver;
+
+	/* The revision is the high nybbles so shift down */
+	ver = bt541->chip_revision >> 4;
+	/* Skip middle digit */
+	ver &= 0xf0f;
+
+	return (ver == 0x402);
+}
+
+static bool zinitix_is_4x3_and_above(struct bt541_ts_data *bt541)
+{
+	u16 ver;
+
+	/* The revision is the high nybbles so shift down */
+	ver = bt541->chip_revision >> 4;
+	/* Skip middle digit */
+	ver &= 0xf0f;
+
+	return (ver >= 0x403);
+}
+
 static int zinitix_read_data(struct i2c_client *client,
 			     u16 reg, void *values, size_t length)
 {
@@ -209,12 +239,92 @@ static u16 zinitix_get_u16_reg(struct bt541_ts_data *bt541, u16 vreg)
         return le16_to_cpu(val);
 }
 
+static int zinitix_calibrate(struct bt541_ts_data *bt541)
+{
+	struct i2c_client *client = bt541->client;
+	int maxsec = 10;
+	int error;
+	u16 val;
+
+	dev_info(bt541->dev, "HW calibration - calibrating\n");
+
+	error = zinitix_write_u16(client, ZINITIX_TOUCH_MODE,
+				  ZINITIX_MODE_CALIBRATION);
+	if (error)
+		return error;
+	error = zinitix_write_cmd(client, ZINITIX_CALIBRATE_CMD);
+	if (error)
+		return error;
+	error = zinitix_write_cmd(client, ZINITIX_SWRESET_CMD);
+	if (error)
+		return error;
+	msleep(1);
+	error = zinitix_write_cmd(client, ZINITIX_CLEAR_INT_STATUS_CMD);
+	if (error)
+		return error;
+
+	/* Wait for hardware calibration to commence */
+	do {
+		msleep(1000);
+		val = zinitix_get_u16_reg(bt541, bt541->eeprom_info_reg);
+		maxsec --;
+	} while ((val & ZINITIX_EEPROM_HW_CALIB) && maxsec);
+
+	if (!maxsec) {
+		dev_err(bt541->dev, "timeout waiting for HW calibration\n");
+		return -ETIMEDOUT;
+	}
+
+	/* Get out of calibration mode */
+	error = zinitix_write_u16(client, ZINITIX_TOUCH_MODE,
+				  bt541->zinitix_mode);
+	if (error)
+		return error;
+
+	error = zinitix_write_u16(client, ZINITIX_INT_ENABLE_FLAG, 0x0);
+	if (error)
+		return error;
+
+	msleep(10);
+
+	error = zinitix_write_cmd(client, ZINITIX_SWRESET_CMD);
+	if (error)
+		return error;
+
+	if (zinitix_is_4x3_and_above(bt541)) {
+		error = zinitix_write_cmd(client, ZINITIX_SAVE_CALIBRATION_CMD);
+		if (error)
+			return error;
+		msleep(500);
+	}
+
+	if (zinitix_is_4x2(bt541)) {
+		error = zinitix_write_cmd(client, ZINITIX_SAVE_STATUS_CMD);
+		if (error)
+			return error;
+		/* Wait for fusing EEPROM */
+		msleep(1000);
+		error = zinitix_write_cmd(client, ZINITIX_SWRESET_CMD);
+		if (error)
+			return error;
+	}
+
+	error = zinitix_write_u16(client, ZINITIX_INT_ENABLE_FLAG, 0x0);
+	if (error)
+		return error;
+
+	dev_info(bt541->dev, "HW calibration complete\n");
+
+	return 0;
+}
+
 static int zinitix_init_touch(struct bt541_ts_data *bt541)
 {
 	struct i2c_client *client = bt541->client;
 	int i;
 	int error;
 	static bool read_static = false;
+	u16 val;
 
 	error = zinitix_write_cmd(client, ZINITIX_SWRESET_CMD);
 	if (error) {
@@ -237,9 +347,20 @@ static int zinitix_init_touch(struct bt541_ts_data *bt541)
 			 "chip revision %04x firmware version %04x regdata version %04x\n",
 			 bt541->chip_revision, bt541->firmware_version,
 			 bt541->regdata_version);
+		if (zinitix_is_4x2(bt541))
+			bt541->eeprom_info_reg = BT4X2_EEPROM_INFO_REG;
+		else
+			bt541->eeprom_info_reg = BT4X3_EEPROM_INFO;
 		read_static = true;
 	}
 
+	val = zinitix_get_u16_reg(bt541, bt541->eeprom_info_reg);
+	if (val & ZINITIX_EEPROM_HW_CALIB) {
+		error = zinitix_calibrate(bt541);
+		if (error)
+			return error;
+	}
+
 	error = zinitix_write_u16(client, ZINITIX_INT_ENABLE_FLAG, 0x0);
 	if (error) {
 		dev_err(bt541->dev,
@@ -273,6 +394,11 @@ static int zinitix_init_touch(struct bt541_ts_data *bt541)
 	if (error)
 		return error;
 
+	/* Software calibration */
+	error = zinitix_write_cmd(client, ZINITIX_CALIBRATE_CMD);
+	if (error)
+		return error;
+
 	error = zinitix_write_u16(client, ZINITIX_INT_ENABLE_FLAG,
 				  BIT_PT_CNT_CHANGE | BIT_DOWN | BIT_MOVE |
 					BIT_UP);
@@ -584,10 +710,10 @@ static int zinitix_ts_probe(struct i2c_client *client)
 					 &bt541->zinitix_mode);
 	if (error < 0) {
 		/* fall back to mode 2 */
-		bt541->zinitix_mode = DEFAULT_TOUCH_POINT_MODE;
+		bt541->zinitix_mode = ZINITIX_MODE_TOUCH_POINT;
 	}
 
-	if (bt541->zinitix_mode != 2) {
+	if (bt541->zinitix_mode != ZINITIX_MODE_TOUCH_POINT) {
 		/*
 		 * If there are devices that don't support mode 2, support
 		 * for other modes (0, 1) will be needed.
-- 
2.35.1

Re: [PATCH 5/5] Input: zinitix - Support calibration

[Dmitry Torokhov]

On Sun, Apr 10, 2022 at 02:00:59PM +0200, Linus Walleij wrote:
>  
> +	val = zinitix_get_u16_reg(bt541, bt541->eeprom_info_reg);
> +	if (val & ZINITIX_EEPROM_HW_CALIB) {
> +		error = zinitix_calibrate(bt541);
> +		if (error)
> +			return error;
> +	}
> +

My understanding that calibration might take pretty long time. Are you
sure you want it to happen automatically when opening the device? Or
maybe userspace needs to control this?

Thanks.

-- 
Dmitry

Re: [PATCH 1/5] Input: zinitix - Helper dev variable in probe()

[Linus Walleij]

On Sun, Apr 10, 2022 at 2:03 PM Linus Walleij <linus.walleij@linaro.org> wrote:

> Create a helper variable struct device *dev in probe() to
> make the code more compact and easier to read.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Anything needed with this patch set?

I'd be fine if we just merge the cleanups (patches 1-3) as a base
to begin with.

Sorry for pushing, I know there may be much to do.

Yours,
Linus Walleij

Re: [PATCH 1/5] Input: zinitix - Helper dev variable in probe()

[Dmitry Torokhov]

Hi Linus,

On Sun, Apr 10, 2022 at 02:00:55PM +0200, Linus Walleij wrote:
> Create a helper variable struct device *dev in probe() to
> make the code more compact and easier to read.

Actually I wonder if this is a good idea: when just seeing "dev" I often
have hard time to remember what device we are dealing with, whereas
"client->dev" gives a very string hint that we are dealing with I2C
peripheral physical device.

Did you observe object code savings from the conversion by chance?

Thanks.

-- 
Dmitry

Re: [PATCH 1/5] Input: zinitix - Helper dev variable in probe()

[Linus Walleij]

On Fri, May 27, 2022 at 7:36 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Sun, Apr 10, 2022 at 02:00:55PM +0200, Linus Walleij wrote:

> > Create a helper variable struct device *dev in probe() to
> > make the code more compact and easier to read.
>
> Actually I wonder if this is a good idea: when just seeing "dev" I often
> have hard time to remember what device we are dealing with, whereas
> "client->dev" gives a very string hint that we are dealing with I2C
> peripheral physical device.

Hm yeah that has a point I suppose.

> Did you observe object code savings from the conversion by chance?

No it is just cognitively easier for me (less characters on the screen)
but that is admittedly a bit of personal preference.

I'll drop this patch, because the subsystem maintainer's taste is
more important than mine.

Yours,
Linus Walleij