[PATCH v5 0/2] Input: hideep - 2 HiDeep touchscreen patches

[PATCH v5 0/2] Input: hideep - 2 HiDeep touchscreen patches

[Hans de Goede]

Hi Dmitry,

Here are 2 patches for the HiDeep touchscreen driver.

Changes in v5:
- Rename the property to "hideep,force-native-protocol"
- Add a comment to the property checking code that this property
  should not be used in devicetree without first adding it to
  the devicetree-bindings.

Changes in v4:
- Rebase on v6.3-rc1

Changes in v3:
- Do not add the x86/acpi only "hideep,reset-work-mode" property to
  the dt-bindings doc

Changes in v2:
- Add "Optionally reset controller work mode to native HiDeep protocol"
  patch

Regards,

Hans


Hans de Goede (2):
  Input: hideep - Silence error in SW_RESET()
  Input: hideep - Optionally reset controller work mode to native HiDeep
    protocol

 drivers/input/touchscreen/hideep.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

-- 
2.39.1

[PATCH v5 1/2] Input: hideep - Silence error in SW_RESET()

[Hans de Goede]

On some models the first HIDEEP_SYSCON_WDT_CON write alone is enough to
cause the controller to reset, causing the second write to fail:

i2c-hideep_ts: write to register 0x52000014 (0x000001) failed: -121

Switch this write to a raw hideep_pgm_w_mem() to avoid an error getting
logged in this case.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/touchscreen/hideep.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/hideep.c b/drivers/input/touchscreen/hideep.c
index bd454d93f1f7..5e6e43d56448 100644
--- a/drivers/input/touchscreen/hideep.c
+++ b/drivers/input/touchscreen/hideep.c
@@ -271,9 +271,14 @@ static int hideep_pgm_w_reg(struct hideep_ts *ts, u32 addr, u32 val)
 
 #define SW_RESET_IN_PGM(clk)					\
 {								\
+	__be32 data = cpu_to_be32(0x01);			\
 	hideep_pgm_w_reg(ts, HIDEEP_SYSCON_WDT_CNT, (clk));	\
 	hideep_pgm_w_reg(ts, HIDEEP_SYSCON_WDT_CON, 0x03);	\
-	hideep_pgm_w_reg(ts, HIDEEP_SYSCON_WDT_CON, 0x01);	\
+	/*							\
+	 * The first write may already cause a reset, use a raw	\
+	 * write for the second write to avoid error logging.	\
+	 */							\
+	hideep_pgm_w_mem(ts, HIDEEP_SYSCON_WDT_CON, &data, 1);	\
 }
 
 #define SET_FLASH_PIO(ce)					\
-- 
2.39.1

[PATCH v5 2/2] Input: hideep - Optionally reset controller work mode to native HiDeep protocol

[Hans de Goede]

The HiDeep IST940E touchscreen controller used on the Lenovo Yoga Book X90F
convertible comes up in HID mode by default.

This works well on the X91F Windows model where the touchscreen is
correctly described in ACPI and ACPI takes care of controlling
the reset GPIO and regulators.

But the X90F ships with Android and the ACPI tables on this model don't
describe the touchscreen. Instead this is hardcoded in the vendor kernel.

The vendor kernel uses the touchscreen in native HiDeep 20 (2.0?) protocol
mode and switches the controller to this mode by writing 0 to reg 0x081e.

Adjusting the i2c-hid code to deal with the reset-gpio and regulators on
this non devicetree (but rather broken ACPI) convertible is somewhat tricky
and the native protocol reports ABS_MT_PRESSURE and ABS_MT_TOUCH_MAJOR
which are not reported in HID mode, so it is preferable to use the native
mode.

Add support to the hideep driver to reset the work-mode to the native
HiDeep protocol to allow using it on the Lenovo Yoga Book X90F.
This is guarded behind a new "hideep,force-native-protocol" boolean
property, to avoid changing behavior on other devices.

For the record: I did test using the i2c-hid driver with some quick hacks
and it does work. The I2C-HID descriptor is available from address 0x0020,
just like on the X91F Windows model.

So far the new "hideep,force-native-protocol" property is only used on
x86/ACPI (non devicetree) devs. IOW it is not used in actual devicetree
files. The devicetree-bindings maintainers have requested properties like
these to not be added to the devicetree-bindings, so the new property is
deliberately not added to the existing devicetree-bindings.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v5:
- Rename the property to "hideep,force-native-protocol"
- Add a comment to the property checking code that this property
  should not be used in devicetree without first adding it to
  the devicetree-bindings.

Changes in v3:
- Do not add the x86/acpi only property to the dt-bindings doc

Changes in v2:
- New patch in v2 of this patch-set
---
 drivers/input/touchscreen/hideep.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/input/touchscreen/hideep.c b/drivers/input/touchscreen/hideep.c
index 5e6e43d56448..74673db06ed0 100644
--- a/drivers/input/touchscreen/hideep.c
+++ b/drivers/input/touchscreen/hideep.c
@@ -35,6 +35,7 @@
 #define HIDEEP_EVENT_ADDR		0x240
 
 /* command list */
+#define HIDEEP_WORK_MODE		0x081e
 #define HIDEEP_RESET_CMD		0x9800
 
 /* event bit */
@@ -964,6 +965,21 @@ static const struct attribute_group hideep_ts_attr_group = {
 	.attrs = hideep_ts_sysfs_entries,
 };
 
+static void hideep_set_work_mode(struct hideep_ts *ts)
+{
+	/*
+	 * Reset touch report format to the native HiDeep 20 protocol if requested.
+	 * This is necessary to make touchscreens which come up in I2C-HID mode
+	 * work with this driver.
+	 *
+	 * Note this is a kernel internal device-property set by x86 platform code,
+	 * this MUST not be used in devicetree files without first adding it to
+	 * the DT bindings.
+	 */
+	if (device_property_read_bool(&ts->client->dev, "hideep,force-native-protocol"))
+		regmap_write(ts->reg, HIDEEP_WORK_MODE, 0x00);
+}
+
 static int hideep_suspend(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
@@ -987,6 +1003,8 @@ static int hideep_resume(struct device *dev)
 		return error;
 	}
 
+	hideep_set_work_mode(ts);
+
 	enable_irq(client->irq);
 
 	return 0;
@@ -1063,6 +1081,8 @@ static int hideep_probe(struct i2c_client *client)
 		return error;
 	}
 
+	hideep_set_work_mode(ts);
+
 	error = hideep_init_input(ts);
 	if (error)
 		return error;
-- 
2.39.1

Re: [PATCH v5 1/2] Input: hideep - Silence error in SW_RESET()

[Dmitry Torokhov]

On Sat, Mar 11, 2023 at 12:47:25PM +0100, Hans de Goede wrote:
> On some models the first HIDEEP_SYSCON_WDT_CON write alone is enough to
> cause the controller to reset, causing the second write to fail:
> 
> i2c-hideep_ts: write to register 0x52000014 (0x000001) failed: -121
> 
> Switch this write to a raw hideep_pgm_w_mem() to avoid an error getting
> logged in this case.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Applied, thank you.

-- 
Dmitry

Re: [PATCH v5 2/2] Input: hideep - Optionally reset controller work mode to native HiDeep protocol

[Dmitry Torokhov]

On Sat, Mar 11, 2023 at 12:47:26PM +0100, Hans de Goede wrote:
> The HiDeep IST940E touchscreen controller used on the Lenovo Yoga Book X90F
> convertible comes up in HID mode by default.
> 
> This works well on the X91F Windows model where the touchscreen is
> correctly described in ACPI and ACPI takes care of controlling
> the reset GPIO and regulators.
> 
> But the X90F ships with Android and the ACPI tables on this model don't
> describe the touchscreen. Instead this is hardcoded in the vendor kernel.
> 
> The vendor kernel uses the touchscreen in native HiDeep 20 (2.0?) protocol
> mode and switches the controller to this mode by writing 0 to reg 0x081e.
> 
> Adjusting the i2c-hid code to deal with the reset-gpio and regulators on
> this non devicetree (but rather broken ACPI) convertible is somewhat tricky
> and the native protocol reports ABS_MT_PRESSURE and ABS_MT_TOUCH_MAJOR
> which are not reported in HID mode, so it is preferable to use the native
> mode.
> 
> Add support to the hideep driver to reset the work-mode to the native
> HiDeep protocol to allow using it on the Lenovo Yoga Book X90F.
> This is guarded behind a new "hideep,force-native-protocol" boolean
> property, to avoid changing behavior on other devices.
> 
> For the record: I did test using the i2c-hid driver with some quick hacks
> and it does work. The I2C-HID descriptor is available from address 0x0020,
> just like on the X91F Windows model.
> 
> So far the new "hideep,force-native-protocol" property is only used on
> x86/ACPI (non devicetree) devs. IOW it is not used in actual devicetree
> files. The devicetree-bindings maintainers have requested properties like
> these to not be added to the devicetree-bindings, so the new property is
> deliberately not added to the existing devicetree-bindings.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Applied, thank you.

-- 
Dmitry