linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/7] Input: synaptics-rmi4: add quirks for third party touchscreen controllers
@ 2023-10-15 21:11 Caleb Connolly
  2023-10-15 21:11 ` [PATCH v2 1/7] dt-bindings: input: syna,rmi4: document syna,pdt-fallback-desc Caleb Connolly
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Caleb Connolly @ 2023-10-15 21:11 UTC (permalink / raw)
  To: Dmitry Torokhov, Vincent Huang
  Cc: methanal, linux-input, devicetree, phone-devel,
	~postmarketos/upstreaming, Caleb Connolly, Jason A. Donenfeld,
	Matthias Schiffer, Rob Herring, Krzysztof Kozlowski, Conor Dooley

With the growing popularity of running upstream Linux on mobile devices,
we're beginning to run into more and more edgecases. The OnePlus 6 is a
fairly well supported 2018 era smartphone, selling over a million units
in it's first 22 days. With this level of popularity, it's almost
inevitable that we get third party replacement displays, and as a
result, replacement touchscreen controllers.

The OnePlus 6 shipped with an extremely usecase specific touchscreen
driver, it implemented only the bare minimum parts of the highly generic
rmi4 protocol, instead hardcoding most of the register addresses.

As a result, the third party touchscreen controllers that are often
found in replacement screens, implement only the registers that the
downstream driver reads from. They additionally have other restrictions
such as heavy penalties on unaligned reads.

This series attempts to implement the necessary workaround to support
some of these chips with the rmi4 driver. Although it's worth noting
that at the time of writing there are other unofficial controllers in
the wild that don't work even with these patches.

We have been shipping these patches in postmarketOS for the last several
months, and they are known to not cause any regressions on the OnePlus
6/6T (with the official Synaptics controller), however I don't own any
other rmi4 hardware to further validate this.

These patches were contributed by a community developer who has given
permission for me to submit them on their behalf.

---
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Vincent Huang <vincent.huang@tw.synaptics.com>
Cc: methanal <baclofen@tuta.io>
Cc: linux-input@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: phone-devel@vger.kernel.org
Cc: ~postmarketos/upstreaming@lists.sr.ht

---
Changes since v1:
- Improve dt-bindings patch (thanks Rob)
- Add missing cast in patch 5 to fix the pointer arithmetic
- Link to v1: https://lore.kernel.org/r/20230929-caleb-rmi4-quirks-v1-0-cc3c703f022d@linaro.org

---
Caleb Connolly (2):
      dt-bindings: input: syna,rmi4: document syna,pdt-fallback-desc
      Input: synaptics-rmi4 - handle duplicate/unknown PDT entries

methanal (5):
      Input: synaptics-rmi4 - f12: use hardcoded values for aftermarket touch ICs
      Input: synaptics-rmi4 - f55: handle zero electrode count
      Input: synaptics-rmi4 - don't do unaligned reads in IRQ context
      Input: synaptics-rmi4 - read product ID on aftermarket touch ICs
      Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes

 .../devicetree/bindings/input/syna,rmi4.yaml       |  15 +++
 drivers/input/rmi4/rmi_driver.c                    | 134 ++++++++++++++++++---
 drivers/input/rmi4/rmi_driver.h                    |   8 ++
 drivers/input/rmi4/rmi_f01.c                       |  14 +++
 drivers/input/rmi4/rmi_f12.c                       | 117 ++++++++++++++----
 drivers/input/rmi4/rmi_f55.c                       |   5 +
 include/linux/rmi.h                                |   3 +
 7 files changed, 252 insertions(+), 44 deletions(-)
---
base-commit: b0d95ff7653ef6ace66a24d6c09147d0731825ce

// Caleb (they/them)


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

* [PATCH v2 1/7] dt-bindings: input: syna,rmi4: document syna,pdt-fallback-desc
  2023-10-15 21:11 [PATCH v2 0/7] Input: synaptics-rmi4: add quirks for third party touchscreen controllers Caleb Connolly
@ 2023-10-15 21:11 ` Caleb Connolly
  2023-10-16  5:31   ` Krzysztof Kozlowski
  2023-10-15 21:11 ` [PATCH v2 2/7] Input: synaptics-rmi4 - handle duplicate/unknown PDT entries Caleb Connolly
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Caleb Connolly @ 2023-10-15 21:11 UTC (permalink / raw)
  To: Dmitry Torokhov, Vincent Huang
  Cc: methanal, linux-input, devicetree, phone-devel,
	~postmarketos/upstreaming, Caleb Connolly, Jason A. Donenfeld,
	Matthias Schiffer, Rob Herring, Krzysztof Kozlowski, Conor Dooley

This new property allows devices to specify some register values which
are missing on units with third party replacement displays. These
displays use unofficial touch ICs which only implement a subset of the
RMI4 specification.

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
To: Rob Herring <robh+dt@kernel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
To: Conor Dooley <conor+dt@kernel.org>
Cc: devicetree@vger.kernel.org
---
 Documentation/devicetree/bindings/input/syna,rmi4.yaml | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/syna,rmi4.yaml b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
index 4d4e1a8e36be..1f4a2179e4d3 100644
--- a/Documentation/devicetree/bindings/input/syna,rmi4.yaml
+++ b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
@@ -49,6 +49,21 @@ properties:
     description:
       Delay to wait after powering on the device.
 
+  syna,pdt-fallback-desc:
+    $ref: /schemas/types.yaml#/definitions/uint8-matrix
+    description:
+      An array of pairs of function number and version. These are used
+      on some devices with replacement displays that use unofficial touch
+      controllers. These controllers do report the properties of their Page
+      Descriptor Table (PDT) entries, but leave the function_number and
+      function_version registers blank. These values should match exactly
+      the 5th and 4th bytes of each PDT entry from the original display's
+      touch controller.
+    items:
+      items:
+        - description: The 5th byte of the PDT entry
+        - description: The 4th byte of the PDT entry
+
   vdd-supply: true
   vio-supply: true
 

-- 
2.42.0


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

* [PATCH v2 2/7] Input: synaptics-rmi4 - handle duplicate/unknown PDT entries
  2023-10-15 21:11 [PATCH v2 0/7] Input: synaptics-rmi4: add quirks for third party touchscreen controllers Caleb Connolly
  2023-10-15 21:11 ` [PATCH v2 1/7] dt-bindings: input: syna,rmi4: document syna,pdt-fallback-desc Caleb Connolly
@ 2023-10-15 21:11 ` Caleb Connolly
  2023-10-15 21:11 ` [PATCH v2 3/7] Input: synaptics-rmi4 - f12: use hardcoded values for aftermarket touch ICs Caleb Connolly
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Caleb Connolly @ 2023-10-15 21:11 UTC (permalink / raw)
  To: Dmitry Torokhov, Vincent Huang
  Cc: methanal, linux-input, devicetree, phone-devel,
	~postmarketos/upstreaming, Caleb Connolly

Some third party rmi4-compatible ICs don't expose their PDT entries
very well. Add a few checks to skip duplicate entries as well as entries
for unsupported functions.

This is required to support some phones with third party displays.

Validated on a stock OnePlus 6T (original parts):
manufacturer: Synaptics, product: S3706B, fw id: 2852315

Co-developed-by: methanal <baclofen@tuta.io>
Signed-off-by: methanal <baclofen@tuta.io>
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 drivers/input/rmi4/rmi_driver.c | 47 +++++++++++++++++++++++++++++++++++------
 drivers/input/rmi4/rmi_driver.h |  6 ++++++
 2 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 258d5fe3d395..cd3e4e77ab9b 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -493,12 +493,44 @@ static void rmi_driver_copy_pdt_to_fd(const struct pdt_entry *pdt,
 	fd->function_version = pdt->function_version;
 }
 
+static bool rmi_pdt_entry_is_valid(struct rmi_device *rmi_dev,
+				  struct pdt_scan_state *state, u8 fn)
+{
+	int i;
+
+	switch (fn) {
+	case 0x01:
+	case 0x03:
+	case 0x11:
+	case 0x12:
+	case 0x30:
+	case 0x34:
+	case 0x3a:
+	case 0x54:
+	case 0x55:
+		break;
+
+	default:
+		rmi_dbg(RMI_DEBUG_CORE, &rmi_dev->dev,
+			"PDT has unknown function number %#02x\n", fn);
+		return false;
+	}
+
+	for (i = 0; i < state->pdt_count; i++) {
+		if (state->pdts[i] == fn)
+			return false;
+	}
+
+	state->pdts[state->pdt_count++] = fn;
+	return true;
+}
+
 #define RMI_SCAN_CONTINUE	0
 #define RMI_SCAN_DONE		1
 
 static int rmi_scan_pdt_page(struct rmi_device *rmi_dev,
 			     int page,
-			     int *empty_pages,
+			     struct pdt_scan_state *state,
 			     void *ctx,
 			     int (*callback)(struct rmi_device *rmi_dev,
 					     void *ctx,
@@ -521,6 +553,9 @@ static int rmi_scan_pdt_page(struct rmi_device *rmi_dev,
 		if (RMI4_END_OF_PDT(pdt_entry.function_number))
 			break;
 
+		if (!rmi_pdt_entry_is_valid(rmi_dev, state, pdt_entry.function_number))
+			continue;
+
 		retval = callback(rmi_dev, ctx, &pdt_entry);
 		if (retval != RMI_SCAN_CONTINUE)
 			return retval;
@@ -531,11 +566,11 @@ static int rmi_scan_pdt_page(struct rmi_device *rmi_dev,
 	 * or more is found, stop scanning.
 	 */
 	if (addr == pdt_start)
-		++*empty_pages;
+		++state->empty_pages;
 	else
-		*empty_pages = 0;
+		state->empty_pages = 0;
 
-	return (data->bootloader_mode || *empty_pages >= 2) ?
+	return (data->bootloader_mode || state->empty_pages >= 2) ?
 					RMI_SCAN_DONE : RMI_SCAN_CONTINUE;
 }
 
@@ -544,11 +579,11 @@ int rmi_scan_pdt(struct rmi_device *rmi_dev, void *ctx,
 		 void *ctx, const struct pdt_entry *entry))
 {
 	int page;
-	int empty_pages = 0;
+	struct pdt_scan_state state = {0, 0, {0}};
 	int retval = RMI_SCAN_DONE;
 
 	for (page = 0; page <= RMI4_MAX_PAGE; page++) {
-		retval = rmi_scan_pdt_page(rmi_dev, page, &empty_pages,
+		retval = rmi_scan_pdt_page(rmi_dev, page, &state,
 					   ctx, callback);
 		if (retval != RMI_SCAN_CONTINUE)
 			break;
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index 1c6c6086c0e5..e1a5412f2f8f 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -46,6 +46,12 @@ struct pdt_entry {
 	u8 function_number;
 };
 
+struct pdt_scan_state {
+	u8 empty_pages;
+	u8 pdt_count;
+	u8 pdts[254];
+};
+
 #define RMI_REG_DESC_PRESENSE_BITS	(32 * BITS_PER_BYTE)
 #define RMI_REG_DESC_SUBPACKET_BITS	(37 * BITS_PER_BYTE)
 

-- 
2.42.0


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

* [PATCH v2 3/7] Input: synaptics-rmi4 - f12: use hardcoded values for aftermarket touch ICs
  2023-10-15 21:11 [PATCH v2 0/7] Input: synaptics-rmi4: add quirks for third party touchscreen controllers Caleb Connolly
  2023-10-15 21:11 ` [PATCH v2 1/7] dt-bindings: input: syna,rmi4: document syna,pdt-fallback-desc Caleb Connolly
  2023-10-15 21:11 ` [PATCH v2 2/7] Input: synaptics-rmi4 - handle duplicate/unknown PDT entries Caleb Connolly
@ 2023-10-15 21:11 ` Caleb Connolly
  2023-10-15 21:11 ` [PATCH v2 4/7] Input: synaptics-rmi4 - f55: handle zero electrode count Caleb Connolly
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Caleb Connolly @ 2023-10-15 21:11 UTC (permalink / raw)
  To: Dmitry Torokhov, Vincent Huang
  Cc: methanal, linux-input, devicetree, phone-devel,
	~postmarketos/upstreaming, Caleb Connolly

From: methanal <baclofen@tuta.io>

Some replacement displays include third-party touch ICs which are
devoid of register descriptors. Create a fake data register descriptor
for such ICs and provide hardcoded default values.

It isn't possible to reliably determine if the touch IC is original or
not, so these fallback values are offered as an alternative to the error
path when register descriptors aren't available.

Signed-off-by: methanal <baclofen@tuta.io>
[changes for readability / codeflow, checkpatch fixes]
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 drivers/input/rmi4/rmi_f12.c | 117 +++++++++++++++++++++++++++++++++----------
 1 file changed, 91 insertions(+), 26 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c
index 7e97944f7616..719ee3b42550 100644
--- a/drivers/input/rmi4/rmi_f12.c
+++ b/drivers/input/rmi4/rmi_f12.c
@@ -195,6 +195,41 @@ static void rmi_f12_process_objects(struct f12_data *f12, u8 *data1, int size)
 		rmi_2d_sensor_abs_report(sensor, &sensor->objs[i], i);
 }
 
+static void rmi_f12_set_hardcoded_desc(struct rmi_function *fn, struct f12_data *f12)
+{
+	struct rmi_2d_sensor *sensor = &f12->sensor;
+	struct rmi_register_desc_item *reg_desc;
+
+	/* We have no f12->data_reg_desc, so the pkt_size is 0, override it with
+	 * a somewhat sensible default (this corresponds to 10 fingers).
+	 */
+	sensor->pkt_size = 88;
+
+	/*
+	 * There are no register descriptors to get these values from.
+	 * We set them to high values to either be overwritten by the clip
+	 * properties from devicetree, or to just not get in the way.
+	 */
+	sensor->max_x = 65535;
+	sensor->max_y = 65535;
+
+	/*
+	 * Create the Data1 register descriptor so that touch events
+	 * can work properly.
+	 */
+	reg_desc = devm_kcalloc(&fn->dev, 1,
+			sizeof(struct rmi_register_desc_item), GFP_KERNEL);
+	reg_desc->reg = 1;
+	reg_desc->reg_size = 80;
+	reg_desc->num_subpackets = 10;
+
+	f12->data1 = reg_desc;
+	f12->data1_offset = 0;
+	sensor->nbr_fingers = reg_desc->num_subpackets;
+	sensor->report_abs = 1;
+	sensor->attn_size += reg_desc->reg_size;
+}
+
 static irqreturn_t rmi_f12_attention(int irq, void *ctx)
 {
 	int retval;
@@ -315,6 +350,40 @@ static int rmi_f12_config(struct rmi_function *fn)
 	return 0;
 }
 
+static int rmi_f12_sensor_init(struct rmi_function *fn, struct f12_data *f12)
+{
+	struct rmi_2d_sensor *sensor = &f12->sensor;
+
+	sensor->fn = fn;
+	f12->data_addr = fn->fd.data_base_addr;
+
+	/* On quirky devices that don't have a data_reg_desc we hardcode the packet
+	 * in rmi_f12_set_hardcoded_desc(). Make sure not to set it to 0 here.
+	 */
+	if (!sensor->pkt_size)
+		sensor->pkt_size = rmi_register_desc_calc_size(&f12->data_reg_desc);
+
+	sensor->axis_align =
+		f12->sensor_pdata.axis_align;
+
+	sensor->x_mm = f12->sensor_pdata.x_mm;
+	sensor->y_mm = f12->sensor_pdata.y_mm;
+	sensor->dribble = f12->sensor_pdata.dribble;
+
+	if (sensor->sensor_type == rmi_sensor_default)
+		sensor->sensor_type =
+			f12->sensor_pdata.sensor_type;
+
+	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s: data packet size: %d\n", __func__,
+		sensor->pkt_size);
+
+	sensor->data_pkt = devm_kzalloc(&fn->dev, sensor->pkt_size, GFP_KERNEL);
+	if (!sensor->data_pkt)
+		return -ENOMEM;
+
+	return 0;
+}
+
 static int rmi_f12_probe(struct rmi_function *fn)
 {
 	struct f12_data *f12;
@@ -328,6 +397,7 @@ static int rmi_f12_probe(struct rmi_function *fn)
 	struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
 	u16 data_offset = 0;
 	int mask_size;
+	bool hardcoded_desc_quirk = false;
 
 	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s\n", __func__);
 
@@ -342,9 +412,9 @@ static int rmi_f12_probe(struct rmi_function *fn)
 	++query_addr;
 
 	if (!(buf & BIT(0))) {
-		dev_err(&fn->dev,
-			"Behavior of F12 without register descriptors is undefined.\n");
-		return -ENODEV;
+		rmi_dbg(RMI_DEBUG_FN, &fn->dev,
+			"No register descriptors defined for F12, using fallback\n");
+		hardcoded_desc_quirk = true;
 	}
 
 	f12 = devm_kzalloc(&fn->dev, sizeof(struct f12_data) + mask_size * 2,
@@ -352,6 +422,8 @@ static int rmi_f12_probe(struct rmi_function *fn)
 	if (!f12)
 		return -ENOMEM;
 
+	dev_set_drvdata(&fn->dev, f12);
+
 	f12->abs_mask = (unsigned long *)((char *)f12
 			+ sizeof(struct f12_data));
 	f12->rel_mask = (unsigned long *)((char *)f12
@@ -370,6 +442,18 @@ static int rmi_f12_probe(struct rmi_function *fn)
 		f12->sensor_pdata = pdata->sensor_pdata;
 	}
 
+	sensor = &f12->sensor;
+
+	if (hardcoded_desc_quirk) {
+		rmi_f12_set_hardcoded_desc(fn, f12);
+
+		ret = rmi_f12_sensor_init(fn, f12);
+		if (ret)
+			return ret;
+
+		goto skip_register_desc;
+	}
+
 	ret = rmi_read_register_desc(rmi_dev, query_addr,
 					&f12->query_reg_desc);
 	if (ret) {
@@ -400,29 +484,9 @@ static int rmi_f12_probe(struct rmi_function *fn)
 	}
 	query_addr += 3;
 
-	sensor = &f12->sensor;
-	sensor->fn = fn;
-	f12->data_addr = fn->fd.data_base_addr;
-	sensor->pkt_size = rmi_register_desc_calc_size(&f12->data_reg_desc);
-
-	sensor->axis_align =
-		f12->sensor_pdata.axis_align;
-
-	sensor->x_mm = f12->sensor_pdata.x_mm;
-	sensor->y_mm = f12->sensor_pdata.y_mm;
-	sensor->dribble = f12->sensor_pdata.dribble;
-
-	if (sensor->sensor_type == rmi_sensor_default)
-		sensor->sensor_type =
-			f12->sensor_pdata.sensor_type;
-
-	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s: data packet size: %d\n", __func__,
-		sensor->pkt_size);
-	sensor->data_pkt = devm_kzalloc(&fn->dev, sensor->pkt_size, GFP_KERNEL);
-	if (!sensor->data_pkt)
-		return -ENOMEM;
-
-	dev_set_drvdata(&fn->dev, f12);
+	ret = rmi_f12_sensor_init(fn, f12);
+	if (ret)
+		return ret;
 
 	ret = rmi_f12_read_sensor_tuning(f12);
 	if (ret)
@@ -520,6 +584,7 @@ static int rmi_f12_probe(struct rmi_function *fn)
 		data_offset += item->reg_size;
 	}
 
+skip_register_desc:
 	/* allocate the in-kernel tracking buffers */
 	sensor->tracking_pos = devm_kcalloc(&fn->dev,
 			sensor->nbr_fingers, sizeof(struct input_mt_pos),

-- 
2.42.0


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

* [PATCH v2 4/7] Input: synaptics-rmi4 - f55: handle zero electrode count
  2023-10-15 21:11 [PATCH v2 0/7] Input: synaptics-rmi4: add quirks for third party touchscreen controllers Caleb Connolly
                   ` (2 preceding siblings ...)
  2023-10-15 21:11 ` [PATCH v2 3/7] Input: synaptics-rmi4 - f12: use hardcoded values for aftermarket touch ICs Caleb Connolly
@ 2023-10-15 21:11 ` Caleb Connolly
  2023-10-15 21:11 ` [PATCH v2 5/7] Input: synaptics-rmi4 - don't do unaligned reads in IRQ context Caleb Connolly
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Caleb Connolly @ 2023-10-15 21:11 UTC (permalink / raw)
  To: Dmitry Torokhov, Vincent Huang
  Cc: methanal, linux-input, devicetree, phone-devel,
	~postmarketos/upstreaming, Caleb Connolly

From: methanal <baclofen@tuta.io>

Some third party ICs claim to support f55 but report an electrode count
of 0. Catch this and bail out early so that we don't confuse the i2c bus
with 0 sized reads.

Signed-off-by: methanal <baclofen@tuta.io>
[simplify code, adjust wording]
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 drivers/input/rmi4/rmi_f55.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/input/rmi4/rmi_f55.c b/drivers/input/rmi4/rmi_f55.c
index 488adaca4dd0..ad2ef14ae9f4 100644
--- a/drivers/input/rmi4/rmi_f55.c
+++ b/drivers/input/rmi4/rmi_f55.c
@@ -52,6 +52,11 @@ static int rmi_f55_detect(struct rmi_function *fn)
 
 	f55->num_rx_electrodes = f55->qry[F55_NUM_RX_OFFSET];
 	f55->num_tx_electrodes = f55->qry[F55_NUM_TX_OFFSET];
+	if (!f55->num_rx_electrodes || !f55->num_tx_electrodes) {
+		rmi_dbg(RMI_DEBUG_FN, &fn->dev,
+			"F55 query returned no electrodes, giving up\n");
+		return 0;
+	}
 
 	f55->cfg_num_rx_electrodes = f55->num_rx_electrodes;
 	f55->cfg_num_tx_electrodes = f55->num_rx_electrodes;

-- 
2.42.0


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

* [PATCH v2 5/7] Input: synaptics-rmi4 - don't do unaligned reads in IRQ context
  2023-10-15 21:11 [PATCH v2 0/7] Input: synaptics-rmi4: add quirks for third party touchscreen controllers Caleb Connolly
                   ` (3 preceding siblings ...)
  2023-10-15 21:11 ` [PATCH v2 4/7] Input: synaptics-rmi4 - f55: handle zero electrode count Caleb Connolly
@ 2023-10-15 21:11 ` Caleb Connolly
  2023-10-15 21:11 ` [PATCH v2 6/7] Input: synaptics-rmi4 - read product ID on aftermarket touch ICs Caleb Connolly
  2023-10-15 21:11 ` [PATCH v2 7/7] Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes Caleb Connolly
  6 siblings, 0 replies; 12+ messages in thread
From: Caleb Connolly @ 2023-10-15 21:11 UTC (permalink / raw)
  To: Dmitry Torokhov, Vincent Huang
  Cc: methanal, linux-input, devicetree, phone-devel,
	~postmarketos/upstreaming, Caleb Connolly

From: methanal <baclofen@tuta.io>

Some replacement displays include third-party touch ICs which incur a
significant penalty (1-2 seconds) when doing certain unaligned reads.
This is enough to break functionality when it happens in the hot path,
so adjust the interrupt handler to not read from an unaligned address.

Signed-off-by: methanal <baclofen@tuta.io>
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
This solution does wind up doing the unaligned reads on the CPU instead,
although I'm not sure how significant of a penalty this is in practise.
---
 drivers/input/rmi4/rmi_driver.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index cd3e4e77ab9b..22f0b35bb70b 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -136,9 +136,14 @@ static int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
 		return 0;
 
 	if (!data->attn_data.data) {
+		/*
+		 * Read the device status register as well and ignore it.
+		 * Some aftermarket ICs have issues with interrupt requests
+		 * otherwise.
+		 */
 		error = rmi_read_block(rmi_dev,
-				data->f01_container->fd.data_base_addr + 1,
-				data->irq_status, data->num_of_irq_regs);
+				data->f01_container->fd.data_base_addr,
+				(u8*)data->irq_status - 1, data->num_of_irq_regs + 1);
 		if (error < 0) {
 			dev_err(dev, "Failed to read irqs, code=%d\n", error);
 			return error;
@@ -1083,16 +1088,17 @@ int rmi_probe_interrupts(struct rmi_driver_data *data)
 	data->num_of_irq_regs = (data->irq_count + 7) / 8;
 
 	size = BITS_TO_LONGS(data->irq_count) * sizeof(unsigned long);
-	data->irq_memory = devm_kcalloc(dev, size, 4, GFP_KERNEL);
+	data->irq_memory = devm_kzalloc(dev, size * 4 + 1, GFP_KERNEL);
 	if (!data->irq_memory) {
 		dev_err(dev, "Failed to allocate memory for irq masks.\n");
 		return -ENOMEM;
 	}
 
-	data->irq_status	= data->irq_memory + size * 0;
-	data->fn_irq_bits	= data->irq_memory + size * 1;
-	data->current_irq_mask	= data->irq_memory + size * 2;
-	data->new_irq_mask	= data->irq_memory + size * 3;
+	/* The first byte is reserved for the device status register */
+	data->irq_status	= data->irq_memory + size * 0 + 1;
+	data->fn_irq_bits	= data->irq_memory + size * 1 + 1;
+	data->current_irq_mask	= data->irq_memory + size * 2 + 1;
+	data->new_irq_mask	= data->irq_memory + size * 3 + 1;
 
 	return retval;
 }

-- 
2.42.0


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

* [PATCH v2 6/7] Input: synaptics-rmi4 - read product ID on aftermarket touch ICs
  2023-10-15 21:11 [PATCH v2 0/7] Input: synaptics-rmi4: add quirks for third party touchscreen controllers Caleb Connolly
                   ` (4 preceding siblings ...)
  2023-10-15 21:11 ` [PATCH v2 5/7] Input: synaptics-rmi4 - don't do unaligned reads in IRQ context Caleb Connolly
@ 2023-10-15 21:11 ` Caleb Connolly
  2023-10-15 21:11 ` [PATCH v2 7/7] Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes Caleb Connolly
  6 siblings, 0 replies; 12+ messages in thread
From: Caleb Connolly @ 2023-10-15 21:11 UTC (permalink / raw)
  To: Dmitry Torokhov, Vincent Huang
  Cc: methanal, linux-input, devicetree, phone-devel,
	~postmarketos/upstreaming, Caleb Connolly

From: methanal <baclofen@tuta.io>

Some replacement displays include third-party touch ICs which do not
report the product ID correctly unless we read directly from the
product ID register. Add a check and a fallback read to handle this.

Signed-off-by: methanal <baclofen@tuta.io>
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 drivers/input/rmi4/rmi_f01.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index d7603c50f864..4aee30d2dcde 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -250,6 +250,20 @@ static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
 		}
 	}
 
+	/*
+	 * Some aftermarket ICs put garbage into the product id field unless
+	 * we read directly from the product id register.
+	 */
+	if (props->product_id[0] < 0x20) {
+		ret = rmi_read_block(rmi_dev, query_base_addr + 11,
+				       props->product_id, RMI_PRODUCT_ID_LENGTH);
+		if (ret) {
+			dev_err(&rmi_dev->dev,
+				"Failed to read product id: %d\n", ret);
+			return ret;
+		}
+	}
+
 	return 0;
 }
 

-- 
2.42.0


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

* [PATCH v2 7/7] Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes
  2023-10-15 21:11 [PATCH v2 0/7] Input: synaptics-rmi4: add quirks for third party touchscreen controllers Caleb Connolly
                   ` (5 preceding siblings ...)
  2023-10-15 21:11 ` [PATCH v2 6/7] Input: synaptics-rmi4 - read product ID on aftermarket touch ICs Caleb Connolly
@ 2023-10-15 21:11 ` Caleb Connolly
  2023-10-18 18:14   ` kernel test robot
  2023-10-18 22:17   ` kernel test robot
  6 siblings, 2 replies; 12+ messages in thread
From: Caleb Connolly @ 2023-10-15 21:11 UTC (permalink / raw)
  To: Dmitry Torokhov, Vincent Huang
  Cc: methanal, linux-input, devicetree, phone-devel,
	~postmarketos/upstreaming, Caleb Connolly

From: methanal <baclofen@tuta.io>

Some replacement displays include third-party touch ICs which do not
expose the function number and the interrupt status in its PDT entries.

OnePlus 6 (original touch IC)
  rmi4_i2c 12-0020: read 6 bytes at 0x00e3: 0 (2b 22 0d 06 01 01)

OnePlus 6 (aftermarket touch IC)
  rmi4_i2c 12-0020: read 6 bytes at 0x00e3: 0 (2c 23 0d 06 00 00)

Introduce a new devicetree property `syna,pdt-desc` which can be used to
provide platform-specific fallback values for users with a replacement
display with an aftermarket touch IC.

Signed-off-by: methanal <baclofen@tuta.io>
[codeflow adjustments, checkpatch fixes, wording]
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 drivers/input/rmi4/rmi_driver.c | 67 ++++++++++++++++++++++++++++++++++++++---
 drivers/input/rmi4/rmi_driver.h |  2 ++
 include/linux/rmi.h             |  3 ++
 3 files changed, 67 insertions(+), 5 deletions(-)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 22f0b35bb70b..f1c09fdb8d73 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -461,9 +461,10 @@ static int rmi_driver_reset_handler(struct rmi_device *rmi_dev)
 	return 0;
 }
 
-static int rmi_read_pdt_entry(struct rmi_device *rmi_dev,
-			      struct pdt_entry *entry, u16 pdt_address)
+static int rmi_read_pdt_entry(struct rmi_device *rmi_dev, struct pdt_entry *entry,
+			      struct pdt_scan_state *state, u16 pdt_address)
 {
+	const struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
 	u8 buf[RMI_PDT_ENTRY_SIZE];
 	int error;
 
@@ -474,6 +475,21 @@ static int rmi_read_pdt_entry(struct rmi_device *rmi_dev,
 		return error;
 	}
 
+	if (pdata->pdt_fallback_size > state->pdt_count * RMI_OF_PDT_DESC_CELLS + 1) {
+		/* Use the description bytes from DT */
+		buf[5] = pdata->pdt_fallback_desc[state->pdt_count * RMI_OF_PDT_DESC_CELLS];
+		buf[4] = pdata->pdt_fallback_desc[state->pdt_count * RMI_OF_PDT_DESC_CELLS + 1];
+
+		error = rmi_read_block(rmi_dev, pdt_address, buf,
+				RMI_PDT_ENTRY_SIZE - 2);
+		if (error) {
+			dev_err(&rmi_dev->dev,
+					"Read PDT entry at %#06x failed, code: %d.\n",
+					pdt_address, error);
+			return error;
+		}
+	}
+
 	entry->page_start = pdt_address & RMI4_PAGE_MASK;
 	entry->query_base_addr = buf[0];
 	entry->command_base_addr = buf[1];
@@ -551,7 +567,7 @@ static int rmi_scan_pdt_page(struct rmi_device *rmi_dev,
 	int retval;
 
 	for (addr = pdt_start; addr >= pdt_end; addr -= RMI_PDT_ENTRY_SIZE) {
-		error = rmi_read_pdt_entry(rmi_dev, &pdt_entry, addr);
+		error = rmi_read_pdt_entry(rmi_dev, &pdt_entry, state, addr);
 		if (error)
 			return error;
 
@@ -1028,9 +1044,11 @@ static int rmi_driver_remove(struct device *dev)
 }
 
 #ifdef CONFIG_OF
-static int rmi_driver_of_probe(struct device *dev,
+static int rmi_driver_of_probe(struct rmi_device *rmi_dev,
 				struct rmi_device_platform_data *pdata)
 {
+	struct device *dev = rmi_dev->xport->dev;
+	u8 buf[RMI_PDT_ENTRY_SIZE];
 	int retval;
 
 	retval = rmi_of_property_read_u32(dev, &pdata->reset_delay_ms,
@@ -1038,6 +1056,45 @@ static int rmi_driver_of_probe(struct device *dev,
 	if (retval)
 		return retval;
 
+	/*
+	 * In some aftermerket touch ICs, the first PDT entry is empty and
+	 * the function number register is 0. If so, the platform
+	 * may have provided backup PDT entries in the device tree.
+	 */
+
+	retval = rmi_read_block(rmi_dev, PDT_START_SCAN_LOCATION,
+			buf, RMI_PDT_ENTRY_SIZE);
+	if (retval) {
+		dev_err(dev, "Read PDT entry at %#06x failed, code: %d.\n",
+			PDT_START_SCAN_LOCATION, retval);
+		return retval;
+	}
+
+	if (!RMI4_END_OF_PDT(buf[5]))
+		return 0;
+
+	pdata->pdt_fallback_size = of_property_count_u8_elems(dev->of_node,
+						  "syna,pdt-fallback-desc");
+
+	/* The rmi4 driver would fail later anyway, so just error out now. */
+	if (pdata->pdt_fallback_size == -EINVAL) {
+		dev_err(dev, "First PDT entry is empty and no backup values provided\n");
+		return -EINVAL;
+	}
+
+	if (pdata->pdt_fallback_size < 0) {
+		dev_err(dev, "syna,pdt-fallback-desc property was not specified properly: %d\n",
+			pdata->pdt_fallback_size);
+		return pdata->pdt_fallback_size;
+	}
+
+	pdata->pdt_fallback_desc = devm_kzalloc(dev, pdata->pdt_fallback_size, GFP_KERNEL);
+
+	retval = of_property_read_u8_array(dev->of_node, "syna,pdt-fallback-desc",
+		pdata->pdt_fallback_desc, pdata->pdt_fallback_size);
+	if (retval < 0)
+		return retval;
+
 	return 0;
 }
 #else
@@ -1163,7 +1220,7 @@ static int rmi_driver_probe(struct device *dev)
 	pdata = rmi_get_platform_data(rmi_dev);
 
 	if (rmi_dev->xport->dev->of_node) {
-		retval = rmi_driver_of_probe(rmi_dev->xport->dev, pdata);
+		retval = rmi_driver_of_probe(rmi_dev, pdata);
 		if (retval)
 			return retval;
 	}
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index e1a5412f2f8f..2531c32d6163 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -31,6 +31,8 @@
 #define RMI_PDT_FUNCTION_VERSION_MASK   0x60
 #define RMI_PDT_INT_SOURCE_COUNT_MASK   0x07
 
+#define RMI_OF_PDT_DESC_CELLS 2
+
 #define PDT_START_SCAN_LOCATION 0x00e9
 #define PDT_END_SCAN_LOCATION	0x0005
 #define RMI4_END_OF_PDT(id) ((id) == 0x00 || (id) == 0xff)
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index ab7eea01ab42..974597960b5e 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -214,6 +214,9 @@ struct rmi_device_platform_data {
 	int reset_delay_ms;
 	int irq;
 
+	u8 *pdt_fallback_desc;
+	int pdt_fallback_size;
+
 	struct rmi_device_platform_data_spi spi_data;
 
 	/* function handler pdata */

-- 
2.42.0


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

* Re: [PATCH v2 1/7] dt-bindings: input: syna,rmi4: document syna,pdt-fallback-desc
  2023-10-15 21:11 ` [PATCH v2 1/7] dt-bindings: input: syna,rmi4: document syna,pdt-fallback-desc Caleb Connolly
@ 2023-10-16  5:31   ` Krzysztof Kozlowski
  2023-10-16 11:06     ` Caleb Connolly
  0 siblings, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-16  5:31 UTC (permalink / raw)
  To: Caleb Connolly, Dmitry Torokhov, Vincent Huang
  Cc: methanal, linux-input, devicetree, phone-devel,
	~postmarketos/upstreaming, Jason A. Donenfeld, Matthias Schiffer,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley

On 15/10/2023 23:11, Caleb Connolly wrote:
> This new property allows devices to specify some register values which
> are missing on units with third party replacement displays. These
> displays use unofficial touch ICs which only implement a subset of the
> RMI4 specification.
> 
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
> To: "Jason A. Donenfeld" <Jason@zx2c4.com>
> To: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> To: Rob Herring <robh+dt@kernel.org>
> To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> To: Conor Dooley <conor+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> ---
>  Documentation/devicetree/bindings/input/syna,rmi4.yaml | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/syna,rmi4.yaml b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
> index 4d4e1a8e36be..1f4a2179e4d3 100644
> --- a/Documentation/devicetree/bindings/input/syna,rmi4.yaml
> +++ b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
> @@ -49,6 +49,21 @@ properties:
>      description:
>        Delay to wait after powering on the device.
>  
> +  syna,pdt-fallback-desc:
> +    $ref: /schemas/types.yaml#/definitions/uint8-matrix
> +    description:
> +      An array of pairs of function number and version. These are used

You nicely explained what is expected to be here, but what is the
purpose of adding this property? Please add it to the description.

> +      on some devices with replacement displays that use unofficial touch
> +      controllers. These controllers do report the properties of their Page
> +      Descriptor Table (PDT) entries, but leave the function_number and
> +      function_version registers blank. These values should match exactly
> +      the 5th and 4th bytes of each PDT entry from the original display's
> +      touch controller.
> +    items:
> +      items:
> +        - description: The 5th byte of the PDT entry
> +        - description: The 4th byte of the PDT entry

Missing constraints on outer level:
    maxItems: 1



Best regards,
Krzysztof


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

* Re: [PATCH v2 1/7] dt-bindings: input: syna,rmi4: document syna,pdt-fallback-desc
  2023-10-16  5:31   ` Krzysztof Kozlowski
@ 2023-10-16 11:06     ` Caleb Connolly
  0 siblings, 0 replies; 12+ messages in thread
From: Caleb Connolly @ 2023-10-16 11:06 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Dmitry Torokhov, Vincent Huang
  Cc: methanal, linux-input, devicetree, phone-devel,
	~postmarketos/upstreaming, Jason A. Donenfeld, Matthias Schiffer,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley



On 16/10/2023 06:31, Krzysztof Kozlowski wrote:
> On 15/10/2023 23:11, Caleb Connolly wrote:
>> This new property allows devices to specify some register values which
>> are missing on units with third party replacement displays. These
>> displays use unofficial touch ICs which only implement a subset of the
>> RMI4 specification.
>>
>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
>> ---
>> To: "Jason A. Donenfeld" <Jason@zx2c4.com>
>> To: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
>> To: Rob Herring <robh+dt@kernel.org>
>> To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
>> To: Conor Dooley <conor+dt@kernel.org>
>> Cc: devicetree@vger.kernel.org
>> ---
>>  Documentation/devicetree/bindings/input/syna,rmi4.yaml | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/input/syna,rmi4.yaml b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
>> index 4d4e1a8e36be..1f4a2179e4d3 100644
>> --- a/Documentation/devicetree/bindings/input/syna,rmi4.yaml
>> +++ b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
>> @@ -49,6 +49,21 @@ properties:
>>      description:
>>        Delay to wait after powering on the device.
>>  
>> +  syna,pdt-fallback-desc:
>> +    $ref: /schemas/types.yaml#/definitions/uint8-matrix
>> +    description:
>> +      An array of pairs of function number and version. These are used
> 
> You nicely explained what is expected to be here, but what is the
> purpose of adding this property? Please add it to the description.

This property is to provide some "expected" register values to handle
hardware where registers that should have values are empty (MCUs running
dodgy firmware pretending to be an rmi4 capable controller).
> 
>> +      on some devices with replacement displays that use unofficial touch
>> +      controllers. These controllers do report the properties of their Page
>> +      Descriptor Table (PDT) entries, but leave the function_number and
>> +      function_version registers blank. These values should match exactly
>> +      the 5th and 4th bytes of each PDT entry from the original display's
>> +      touch controller.
>> +    items:
>> +      items:
>> +        - description: The 5th byte of the PDT entry
>> +        - description: The 4th byte of the PDT entry
> 
> Missing constraints on outer level:
>     maxItems: 1

There can be (technically) an arbitrary number of pairs here, it should
align with the number of Page Descriptor Tables supported by the device.
This is basically meant to done on a best-effort basis. The OnePlus 6
has 3 pairs here.

I'll give another pass at the description and see if I can explain this
better heh.
> 
> 
> 
> Best regards,
> Krzysztof
> 

-- 
// Caleb (they/them)

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

* Re: [PATCH v2 7/7] Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes
  2023-10-15 21:11 ` [PATCH v2 7/7] Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes Caleb Connolly
@ 2023-10-18 18:14   ` kernel test robot
  2023-10-18 22:17   ` kernel test robot
  1 sibling, 0 replies; 12+ messages in thread
From: kernel test robot @ 2023-10-18 18:14 UTC (permalink / raw)
  To: Caleb Connolly, Dmitry Torokhov, Vincent Huang
  Cc: oe-kbuild-all, methanal, linux-input, devicetree, phone-devel,
	~postmarketos/upstreaming, Caleb Connolly

Hi Caleb,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.6-rc6 next-20231018]
[cannot apply to dtor-input/next dtor-input/for-linus]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Caleb-Connolly/Input-synaptics-rmi4-handle-duplicate-unknown-PDT-entries/20231017-132844
base:   linus/master
patch link:    https://lore.kernel.org/r/20230929-caleb-rmi4-quirks-v2-7-b227ac498d88%40linaro.org
patch subject: [PATCH v2 7/7] Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20231019/202310190215.hvnjZ9RG-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231019/202310190215.hvnjZ9RG-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310190215.hvnjZ9RG-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/input/rmi4/rmi_driver.c: In function 'rmi_driver_probe':
>> drivers/input/rmi4/rmi_driver.c:1223:46: error: passing argument 1 of 'rmi_driver_of_probe' from incompatible pointer type [-Werror=incompatible-pointer-types]
    1223 |                 retval = rmi_driver_of_probe(rmi_dev, pdata);
         |                                              ^~~~~~~
         |                                              |
         |                                              struct rmi_device *
   drivers/input/rmi4/rmi_driver.c:1101:54: note: expected 'struct device *' but argument is of type 'struct rmi_device *'
    1101 | static inline int rmi_driver_of_probe(struct device *dev,
         |                                       ~~~~~~~~~~~~~~~^~~
   cc1: some warnings being treated as errors


vim +/rmi_driver_of_probe +1223 drivers/input/rmi4/rmi_driver.c

  1199	
  1200	static int rmi_driver_probe(struct device *dev)
  1201	{
  1202		struct rmi_driver *rmi_driver;
  1203		struct rmi_driver_data *data;
  1204		struct rmi_device_platform_data *pdata;
  1205		struct rmi_device *rmi_dev;
  1206		int retval;
  1207	
  1208		rmi_dbg(RMI_DEBUG_CORE, dev, "%s: Starting probe.\n",
  1209				__func__);
  1210	
  1211		if (!rmi_is_physical_device(dev)) {
  1212			rmi_dbg(RMI_DEBUG_CORE, dev, "Not a physical device.\n");
  1213			return -ENODEV;
  1214		}
  1215	
  1216		rmi_dev = to_rmi_device(dev);
  1217		rmi_driver = to_rmi_driver(dev->driver);
  1218		rmi_dev->driver = rmi_driver;
  1219	
  1220		pdata = rmi_get_platform_data(rmi_dev);
  1221	
  1222		if (rmi_dev->xport->dev->of_node) {
> 1223			retval = rmi_driver_of_probe(rmi_dev, pdata);
  1224			if (retval)
  1225				return retval;
  1226		}
  1227	
  1228		data = devm_kzalloc(dev, sizeof(struct rmi_driver_data), GFP_KERNEL);
  1229		if (!data)
  1230			return -ENOMEM;
  1231	
  1232		INIT_LIST_HEAD(&data->function_list);
  1233		data->rmi_dev = rmi_dev;
  1234		dev_set_drvdata(&rmi_dev->dev, data);
  1235	
  1236		/*
  1237		 * Right before a warm boot, the sensor might be in some unusual state,
  1238		 * such as F54 diagnostics, or F34 bootloader mode after a firmware
  1239		 * or configuration update.  In order to clear the sensor to a known
  1240		 * state and/or apply any updates, we issue a initial reset to clear any
  1241		 * previous settings and force it into normal operation.
  1242		 *
  1243		 * We have to do this before actually building the PDT because
  1244		 * the reflash updates (if any) might cause various registers to move
  1245		 * around.
  1246		 *
  1247		 * For a number of reasons, this initial reset may fail to return
  1248		 * within the specified time, but we'll still be able to bring up the
  1249		 * driver normally after that failure.  This occurs most commonly in
  1250		 * a cold boot situation (where then firmware takes longer to come up
  1251		 * than from a warm boot) and the reset_delay_ms in the platform data
  1252		 * has been set too short to accommodate that.  Since the sensor will
  1253		 * eventually come up and be usable, we don't want to just fail here
  1254		 * and leave the customer's device unusable.  So we warn them, and
  1255		 * continue processing.
  1256		 */
  1257		retval = rmi_scan_pdt(rmi_dev, NULL, rmi_initial_reset);
  1258		if (retval < 0)
  1259			dev_warn(dev, "RMI initial reset failed! Continuing in spite of this.\n");
  1260	
  1261		retval = rmi_read(rmi_dev, PDT_PROPERTIES_LOCATION, &data->pdt_props);
  1262		if (retval < 0) {
  1263			/*
  1264			 * we'll print out a warning and continue since
  1265			 * failure to get the PDT properties is not a cause to fail
  1266			 */
  1267			dev_warn(dev, "Could not read PDT properties from %#06x (code %d). Assuming 0x00.\n",
  1268				 PDT_PROPERTIES_LOCATION, retval);
  1269		}
  1270	
  1271		mutex_init(&data->irq_mutex);
  1272		mutex_init(&data->enabled_mutex);
  1273	
  1274		retval = rmi_probe_interrupts(data);
  1275		if (retval)
  1276			goto err;
  1277	
  1278		if (rmi_dev->xport->input) {
  1279			/*
  1280			 * The transport driver already has an input device.
  1281			 * In some cases it is preferable to reuse the transport
  1282			 * devices input device instead of creating a new one here.
  1283			 * One example is some HID touchpads report "pass-through"
  1284			 * button events are not reported by rmi registers.
  1285			 */
  1286			data->input = rmi_dev->xport->input;
  1287		} else {
  1288			data->input = devm_input_allocate_device(dev);
  1289			if (!data->input) {
  1290				dev_err(dev, "%s: Failed to allocate input device.\n",
  1291					__func__);
  1292				retval = -ENOMEM;
  1293				goto err;
  1294			}
  1295			rmi_driver_set_input_params(rmi_dev, data->input);
  1296			data->input->phys = devm_kasprintf(dev, GFP_KERNEL,
  1297							"%s/input0", dev_name(dev));
  1298		}
  1299	
  1300		retval = rmi_init_functions(data);
  1301		if (retval)
  1302			goto err;
  1303	
  1304		retval = rmi_f34_create_sysfs(rmi_dev);
  1305		if (retval)
  1306			goto err;
  1307	
  1308		if (data->input) {
  1309			rmi_driver_set_input_name(rmi_dev, data->input);
  1310			if (!rmi_dev->xport->input) {
  1311				retval = input_register_device(data->input);
  1312				if (retval) {
  1313					dev_err(dev, "%s: Failed to register input device.\n",
  1314						__func__);
  1315					goto err_destroy_functions;
  1316				}
  1317			}
  1318		}
  1319	
  1320		retval = rmi_irq_init(rmi_dev);
  1321		if (retval < 0)
  1322			goto err_destroy_functions;
  1323	
  1324		if (data->f01_container->dev.driver) {
  1325			/* Driver already bound, so enable ATTN now. */
  1326			retval = rmi_enable_sensor(rmi_dev);
  1327			if (retval)
  1328				goto err_disable_irq;
  1329		}
  1330	
  1331		return 0;
  1332	
  1333	err_disable_irq:
  1334		rmi_disable_irq(rmi_dev, false);
  1335	err_destroy_functions:
  1336		rmi_free_function_list(rmi_dev);
  1337	err:
  1338		return retval;
  1339	}
  1340	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH v2 7/7] Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes
  2023-10-15 21:11 ` [PATCH v2 7/7] Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes Caleb Connolly
  2023-10-18 18:14   ` kernel test robot
@ 2023-10-18 22:17   ` kernel test robot
  1 sibling, 0 replies; 12+ messages in thread
From: kernel test robot @ 2023-10-18 22:17 UTC (permalink / raw)
  To: Caleb Connolly, Dmitry Torokhov, Vincent Huang
  Cc: llvm, oe-kbuild-all, methanal, linux-input, devicetree,
	phone-devel, ~postmarketos/upstreaming, Caleb Connolly

Hi Caleb,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.6-rc6 next-20231018]
[cannot apply to dtor-input/next dtor-input/for-linus]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Caleb-Connolly/Input-synaptics-rmi4-handle-duplicate-unknown-PDT-entries/20231017-132844
base:   linus/master
patch link:    https://lore.kernel.org/r/20230929-caleb-rmi4-quirks-v2-7-b227ac498d88%40linaro.org
patch subject: [PATCH v2 7/7] Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20231019/202310190640.pxJQCbWc-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231019/202310190640.pxJQCbWc-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310190640.pxJQCbWc-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/input/rmi4/rmi_driver.c:1223:32: error: incompatible pointer types passing 'struct rmi_device *' to parameter of type 'struct device *' [-Werror,-Wincompatible-pointer-types]
                   retval = rmi_driver_of_probe(rmi_dev, pdata);
                                                ^~~~~~~
   drivers/input/rmi4/rmi_driver.c:1101:54: note: passing argument to parameter 'dev' here
   static inline int rmi_driver_of_probe(struct device *dev,
                                                        ^
   1 error generated.


vim +1223 drivers/input/rmi4/rmi_driver.c

  1199	
  1200	static int rmi_driver_probe(struct device *dev)
  1201	{
  1202		struct rmi_driver *rmi_driver;
  1203		struct rmi_driver_data *data;
  1204		struct rmi_device_platform_data *pdata;
  1205		struct rmi_device *rmi_dev;
  1206		int retval;
  1207	
  1208		rmi_dbg(RMI_DEBUG_CORE, dev, "%s: Starting probe.\n",
  1209				__func__);
  1210	
  1211		if (!rmi_is_physical_device(dev)) {
  1212			rmi_dbg(RMI_DEBUG_CORE, dev, "Not a physical device.\n");
  1213			return -ENODEV;
  1214		}
  1215	
  1216		rmi_dev = to_rmi_device(dev);
  1217		rmi_driver = to_rmi_driver(dev->driver);
  1218		rmi_dev->driver = rmi_driver;
  1219	
  1220		pdata = rmi_get_platform_data(rmi_dev);
  1221	
  1222		if (rmi_dev->xport->dev->of_node) {
> 1223			retval = rmi_driver_of_probe(rmi_dev, pdata);
  1224			if (retval)
  1225				return retval;
  1226		}
  1227	
  1228		data = devm_kzalloc(dev, sizeof(struct rmi_driver_data), GFP_KERNEL);
  1229		if (!data)
  1230			return -ENOMEM;
  1231	
  1232		INIT_LIST_HEAD(&data->function_list);
  1233		data->rmi_dev = rmi_dev;
  1234		dev_set_drvdata(&rmi_dev->dev, data);
  1235	
  1236		/*
  1237		 * Right before a warm boot, the sensor might be in some unusual state,
  1238		 * such as F54 diagnostics, or F34 bootloader mode after a firmware
  1239		 * or configuration update.  In order to clear the sensor to a known
  1240		 * state and/or apply any updates, we issue a initial reset to clear any
  1241		 * previous settings and force it into normal operation.
  1242		 *
  1243		 * We have to do this before actually building the PDT because
  1244		 * the reflash updates (if any) might cause various registers to move
  1245		 * around.
  1246		 *
  1247		 * For a number of reasons, this initial reset may fail to return
  1248		 * within the specified time, but we'll still be able to bring up the
  1249		 * driver normally after that failure.  This occurs most commonly in
  1250		 * a cold boot situation (where then firmware takes longer to come up
  1251		 * than from a warm boot) and the reset_delay_ms in the platform data
  1252		 * has been set too short to accommodate that.  Since the sensor will
  1253		 * eventually come up and be usable, we don't want to just fail here
  1254		 * and leave the customer's device unusable.  So we warn them, and
  1255		 * continue processing.
  1256		 */
  1257		retval = rmi_scan_pdt(rmi_dev, NULL, rmi_initial_reset);
  1258		if (retval < 0)
  1259			dev_warn(dev, "RMI initial reset failed! Continuing in spite of this.\n");
  1260	
  1261		retval = rmi_read(rmi_dev, PDT_PROPERTIES_LOCATION, &data->pdt_props);
  1262		if (retval < 0) {
  1263			/*
  1264			 * we'll print out a warning and continue since
  1265			 * failure to get the PDT properties is not a cause to fail
  1266			 */
  1267			dev_warn(dev, "Could not read PDT properties from %#06x (code %d). Assuming 0x00.\n",
  1268				 PDT_PROPERTIES_LOCATION, retval);
  1269		}
  1270	
  1271		mutex_init(&data->irq_mutex);
  1272		mutex_init(&data->enabled_mutex);
  1273	
  1274		retval = rmi_probe_interrupts(data);
  1275		if (retval)
  1276			goto err;
  1277	
  1278		if (rmi_dev->xport->input) {
  1279			/*
  1280			 * The transport driver already has an input device.
  1281			 * In some cases it is preferable to reuse the transport
  1282			 * devices input device instead of creating a new one here.
  1283			 * One example is some HID touchpads report "pass-through"
  1284			 * button events are not reported by rmi registers.
  1285			 */
  1286			data->input = rmi_dev->xport->input;
  1287		} else {
  1288			data->input = devm_input_allocate_device(dev);
  1289			if (!data->input) {
  1290				dev_err(dev, "%s: Failed to allocate input device.\n",
  1291					__func__);
  1292				retval = -ENOMEM;
  1293				goto err;
  1294			}
  1295			rmi_driver_set_input_params(rmi_dev, data->input);
  1296			data->input->phys = devm_kasprintf(dev, GFP_KERNEL,
  1297							"%s/input0", dev_name(dev));
  1298		}
  1299	
  1300		retval = rmi_init_functions(data);
  1301		if (retval)
  1302			goto err;
  1303	
  1304		retval = rmi_f34_create_sysfs(rmi_dev);
  1305		if (retval)
  1306			goto err;
  1307	
  1308		if (data->input) {
  1309			rmi_driver_set_input_name(rmi_dev, data->input);
  1310			if (!rmi_dev->xport->input) {
  1311				retval = input_register_device(data->input);
  1312				if (retval) {
  1313					dev_err(dev, "%s: Failed to register input device.\n",
  1314						__func__);
  1315					goto err_destroy_functions;
  1316				}
  1317			}
  1318		}
  1319	
  1320		retval = rmi_irq_init(rmi_dev);
  1321		if (retval < 0)
  1322			goto err_destroy_functions;
  1323	
  1324		if (data->f01_container->dev.driver) {
  1325			/* Driver already bound, so enable ATTN now. */
  1326			retval = rmi_enable_sensor(rmi_dev);
  1327			if (retval)
  1328				goto err_disable_irq;
  1329		}
  1330	
  1331		return 0;
  1332	
  1333	err_disable_irq:
  1334		rmi_disable_irq(rmi_dev, false);
  1335	err_destroy_functions:
  1336		rmi_free_function_list(rmi_dev);
  1337	err:
  1338		return retval;
  1339	}
  1340	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

end of thread, other threads:[~2023-10-18 22:17 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-15 21:11 [PATCH v2 0/7] Input: synaptics-rmi4: add quirks for third party touchscreen controllers Caleb Connolly
2023-10-15 21:11 ` [PATCH v2 1/7] dt-bindings: input: syna,rmi4: document syna,pdt-fallback-desc Caleb Connolly
2023-10-16  5:31   ` Krzysztof Kozlowski
2023-10-16 11:06     ` Caleb Connolly
2023-10-15 21:11 ` [PATCH v2 2/7] Input: synaptics-rmi4 - handle duplicate/unknown PDT entries Caleb Connolly
2023-10-15 21:11 ` [PATCH v2 3/7] Input: synaptics-rmi4 - f12: use hardcoded values for aftermarket touch ICs Caleb Connolly
2023-10-15 21:11 ` [PATCH v2 4/7] Input: synaptics-rmi4 - f55: handle zero electrode count Caleb Connolly
2023-10-15 21:11 ` [PATCH v2 5/7] Input: synaptics-rmi4 - don't do unaligned reads in IRQ context Caleb Connolly
2023-10-15 21:11 ` [PATCH v2 6/7] Input: synaptics-rmi4 - read product ID on aftermarket touch ICs Caleb Connolly
2023-10-15 21:11 ` [PATCH v2 7/7] Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes Caleb Connolly
2023-10-18 18:14   ` kernel test robot
2023-10-18 22:17   ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).