[PATCH v2 0/2] arm64: dts: qcom: sc8280xp-x13s: Enable touchscreen

[PATCH v2 0/2] arm64: dts: qcom: sc8280xp-x13s: Enable touchscreen

[Bjorn Andersson]

This documents and defines the necessary properties for the I2C
HID-based touchscreen found in some SKUs of the Lenovo Thinkpad X13s to
work.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
Changes in v2:
- Dropped output-high from &ts0_default, to avoid bouncing the reset
  line unnecessarily
- Link to v1: https://lore.kernel.org/r/20240125-x13s-touchscreen-v1-0-ab8c882def9c@quicinc.com

---
Bjorn Andersson (2):
      dt-bindings: HID: i2c-hid: Document reset-related properties
      arm64: dts: qcom: sc8280xp-x13s: Fix/enable touchscreen

 Documentation/devicetree/bindings/input/hid-over-i2c.yaml  | 6 ++++++
 arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 7 +++++--
 2 files changed, 11 insertions(+), 2 deletions(-)
---
base-commit: 8bf1262c53f50fa91fe15d01e5ef5629db55313c
change-id: 20240125-x13s-touchscreen-48012ff3c24e

Best regards,
-- 
Bjorn Andersson <quic_bjorande@quicinc.com>

[PATCH v2 1/2] dt-bindings: HID: i2c-hid: Document reset-related properties

[Bjorn Andersson]

Some I2C HID devices has a reset pin and requires that some specified
time elapses after this reset pin is deasserted, before communication
with the device is attempted.

The Linux implementation is looking for these in the "reset-gpios" and
"post-reset-deassert-delay-ms" properties already, so use these property
names.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 Documentation/devicetree/bindings/input/hid-over-i2c.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/hid-over-i2c.yaml b/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
index 138caad96a29..f07ff4cb3d26 100644
--- a/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
+++ b/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
@@ -50,6 +50,12 @@ properties:
     description: Time required by the device after enabling its regulators
       or powering it on, before it is ready for communication.
 
+  post-reset-deassert-delay-ms:
+    description: Time required by the device after reset has been deasserted,
+      before it is ready for communication.
+
+  reset-gpios: true
+
   touchscreen-inverted-x: true
 
   touchscreen-inverted-y: true

-- 
2.25.1

[PATCH v2 2/2] arm64: dts: qcom: sc8280xp-x13s: Fix/enable touchscreen

[Bjorn Andersson]

The failing read-test in __i2c_hid_core_probe() determines that there's
nothing connected at the documented address of the touchscreen.

Introduce the 5ms after-power and 200ms after-reset delays found in the
ACPI tables. Also wire up the reset-gpio, for good measure.

Fixes: 32c231385ed4 ("arm64: dts: qcom: sc8280xp: add Lenovo Thinkpad X13s devicetree")
Tested-by: Daniel Thompson <daniel.thompson@linaro.org>
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
index def3976bd5bb..33731b95ad51 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
@@ -620,7 +620,6 @@ &i2c4 {
 
 	status = "okay";
 
-	/* FIXME: verify */
 	touchscreen@10 {
 		compatible = "hid-over-i2c";
 		reg = <0x10>;
@@ -630,6 +629,11 @@ touchscreen@10 {
 		vdd-supply = <&vreg_misc_3p3>;
 		vddl-supply = <&vreg_s10b>;
 
+		reset-gpios = <&tlmm 99 GPIO_ACTIVE_LOW>;
+
+		post-power-on-delay-ms = <5>;
+		post-reset-deassert-delay-ms = <200>;
+
 		pinctrl-names = "default";
 		pinctrl-0 = <&ts0_default>;
 	};
@@ -1450,7 +1454,6 @@ int-n-pins {
 		reset-n-pins {
 			pins = "gpio99";
 			function = "gpio";
-			output-high;
 			drive-strength = <16>;
 		};
 	};

-- 
2.25.1

Re: [PATCH v2 1/2] dt-bindings: HID: i2c-hid: Document reset-related properties

[Johan Hovold]

On Fri, Jan 26, 2024 at 08:41:38AM -0800, Bjorn Andersson wrote:
> Some I2C HID devices has a reset pin and requires that some specified
> time elapses after this reset pin is deasserted, before communication
> with the device is attempted.
> 
> The Linux implementation is looking for these in the "reset-gpios" and
> "post-reset-deassert-delay-ms" properties already, so use these property
> names.
> 
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  Documentation/devicetree/bindings/input/hid-over-i2c.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/hid-over-i2c.yaml b/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
> index 138caad96a29..f07ff4cb3d26 100644
> --- a/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
> +++ b/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
> @@ -50,6 +50,12 @@ properties:
>      description: Time required by the device after enabling its regulators
>        or powering it on, before it is ready for communication.
>  
> +  post-reset-deassert-delay-ms:
> +    description: Time required by the device after reset has been deasserted,
> +      before it is ready for communication.

You forgot to remove the now obsolete comment from the HID driver that I
mentioned.

> +
> +  reset-gpios: true
> +
>    touchscreen-inverted-x: true
>  
>    touchscreen-inverted-y: true

Reviewed-by: Johan Hovold <johan+linaro@kernel.org>

Re: [PATCH v2 2/2] arm64: dts: qcom: sc8280xp-x13s: Fix/enable touchscreen

[Johan Hovold]

On Fri, Jan 26, 2024 at 08:41:39AM -0800, Bjorn Andersson wrote:
> The failing read-test in __i2c_hid_core_probe() determines that there's
> nothing connected at the documented address of the touchscreen.
> 
> Introduce the 5ms after-power and 200ms after-reset delays found in the
> ACPI tables. Also wire up the reset-gpio, for good measure.

Please amend the commit message so that it reflects the discussion we
just had (e.g. wiring up the reset-gpio is not just for good measure).

Johan