This series of patches adds / fixes problems with pazquel360's
touchscreen. A few notes here:
1. Originally the touchscreen was supposed to be added as part of the
first landing of the pazquel360 device tree. ...but the fact that
Yunlong changed email addresses seems to have messed up Bjorn's
scripts. What landed was v3 [1] instead of v5 [2]. The pazquel360 part
of this series is that diff.
2. We delayed sending the fixup till now because soon after the series
landed upstream we found that some laptops were having trouble
initting the touchscreen in cases where the eDP/touchscreen regulator
was left on by the bootloader. We've been struggling to make sense of
all of this. As part of this investigation we landed a85fbd649844
("Input: elants_i2c - properly handle the reset GPIO when power is
off") but that wasn't enough. That fix, together with the fixes in
this series, is enough though.
3. This series is mostly device tree changes with one more change to
the Elan driver. They are fine to land in separate trees. It turns out
that with _just_ the device tree changes things are actually working
OK but the timing is tight, so getting a little extra breathing room
from the Linux driver is nice.
4. Despite the fact that we did debugging here on pazquel360, many of
the changes here are made in general for trogdor devices. I believe
that this will make the timing more correct on those devices even if
we weren't actually seeing problems.
[1] https://lore.kernel.org/r/20220901024827.v3.2.Iea2d2918adfff2825b87d428b5732717425c196f@changeid
[2] https://lore.kernel.org/r/20220923083657.v5.3.Iea2d2918adfff2825b87d428b5732717425c196f@changeid
Douglas Anderson (5):
arm64: dts: qcom: sc7180: Bump up trogdor ts_reset_l drive strength
arm64: dts: qcom: sc7180: Add trogdor eDP/touchscreen regulator
off-on-time
arm64: dts: qcom: sc7180: Start the trogdor eDP/touchscreen regulator
on
arm64: dts: qcom: sc7180: Add pazquel360 touschreen
Input: elants_i2c: Delay longer with reset asserted
.../dts/qcom/sc7180-trogdor-homestar.dtsi | 18 ++++++++++++++++
.../qcom/sc7180-trogdor-parade-ps8640.dtsi | 20 ++++++++++++++++++
.../dts/qcom/sc7180-trogdor-pazquel360.dtsi | 21 +++++++++++++++++++
.../dts/qcom/sc7180-trogdor-ti-sn65dsi86.dtsi | 20 ++++++++++++++++++
arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 10 ++++++++-
drivers/input/touchscreen/elants_i2c.c | 2 +-
6 files changed, 89 insertions(+), 2 deletions(-)
--
2.39.0.rc1.256.g54fd8350bd-goog
Now that we've added the `off-on-delay-us` for the touchpanel
regulator, we can see that we're actually hitting that delay at
bootup. I saw about 200 ms of delay.
Let's avoid that delay by starting the regulator on. We'll only do
this for eDP devices for the time being.
NOTE: we _won't_ do this for homestar. Homestar's panel really likes
to be power cycled. It's why the Linux driver for this panel has a
pm_runtime_put_sync_suspend() when the panel is being unprepared but
the normal panel-edp driver doesn't. It's also why this hardware has a
separate power rail for eDP vs. touchscreen, unlike all the other
trogdor boards. We won't start homestar's regulator on. While this
could mean a slight delay on homestar, it is probably a _correct_
delay. The bootloader might have left the regulator on (it does so in
dev and recovery modes), so if we turned the regulator off at probe
time and we actually hit the delay then we were probably violating T12
in the panel spec.
Signed-off-by: Douglas Anderson <[email protected]>
---
.../boot/dts/qcom/sc7180-trogdor-homestar.dtsi | 18 ++++++++++++++++++
.../dts/qcom/sc7180-trogdor-parade-ps8640.dtsi | 8 ++++++++
.../dts/qcom/sc7180-trogdor-ti-sn65dsi86.dtsi | 8 ++++++++
3 files changed, 34 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
index d3cf64c16dcd..b3ba23a88a0b 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
@@ -85,6 +85,24 @@ map1 {
};
};
+/*
+ * ADDITIONS TO FIXED REGULATORS DEFINED IN PARENT DEVICE TREE FILES
+ *
+ * Sort order matches the order in the parent files (parents before children).
+ */
+
+&pp3300_dx_edp {
+ /*
+ * The atna33xc20 really likes to be power cycled to keep it from
+ * getting in a bad state. This is the reason that the touchscreen
+ * rail and eDP rails are separate from each other on homestar (but
+ * not other trogdor devices) Make sure it starts "off" at bootup.
+ */
+ /delete-property/ regulator-boot-on;
+};
+
+/* ADDITIONS TO NODES DEFINED IN PARENT DEVICE TREE FILES */
+
ap_ts_pen_1v8: &i2c4 {
status = "okay";
clock-frequency = <400000>;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-parade-ps8640.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-parade-ps8640.dtsi
index e27a769f8cd4..5aa7949b5328 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-parade-ps8640.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-parade-ps8640.dtsi
@@ -34,6 +34,14 @@ pp3300_brij_ps8640: pp3300-brij-ps8640-regulator {
&pp3300_dx_edp {
off-on-delay-us = <500000>;
+
+ /*
+ * It's nicer to start with this regulator enabled. The
+ * bootloader may have left it on and it's nice not to cause an
+ * extra power cycle of the touchscreen and eDP panel at bootup.
+ * This should help speed bootup because we have off-on-delay-us.
+ */
+ regulator-boot-on;
};
/* ADDITIONS TO NODES DEFINED IN PARENT DEVICE TREE FILES */
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-ti-sn65dsi86.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-ti-sn65dsi86.dtsi
index 3188788306d0..e52b8776755d 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-ti-sn65dsi86.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-ti-sn65dsi86.dtsi
@@ -15,6 +15,14 @@
&pp3300_dx_edp {
off-on-delay-us = <500000>;
+
+ /*
+ * It's nicer to start with this regulator enabled. The
+ * bootloader may have left it on and it's nice not to cause an
+ * extra power cycle of the touchscreen and eDP panel at bootup.
+ * This should help speed bootup because we have off-on-delay-us.
+ */
+ regulator-boot-on;
};
/* ADDITIONS TO NODES DEFINED IN PARENT DEVICE TREE FILES */
--
2.39.0.rc1.256.g54fd8350bd-goog
The touchscreen was supposed to have been added when pazquel360 first
was added upstream but was missed. Add it now.
Signed-off-by: Douglas Anderson <[email protected]>
---
.../dts/qcom/sc7180-trogdor-pazquel360.dtsi | 21 +++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel360.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel360.dtsi
index 5702325d0c7b..54b89def8402 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel360.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel360.dtsi
@@ -14,6 +14,27 @@ &alc5682 {
realtek,dmic-clk-rate-hz = <2048000>;
};
+ap_ts_pen_1v8: &i2c4 {
+ status = "okay";
+ clock-frequency = <400000>;
+
+ ap_ts: touchscreen@10 {
+ compatible = "elan,ekth3915", "elan,ekth3500";
+ reg = <0x10>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&ts_int_l>, <&ts_reset_l>;
+
+ interrupt-parent = <&tlmm>;
+ interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
+
+ hid-descr-addr = <0x0001>;
+
+ vcc33-supply = <&pp3300_ts>;
+ vccio-supply = <&pp1800_l10a>;
+ reset-gpios = <&tlmm 8 GPIO_ACTIVE_LOW>;
+ };
+};
+
&keyboard_controller {
function-row-physmap = <
MATRIX_KEY(0x00, 0x02, 0) /* T1 */
--
2.39.0.rc1.256.g54fd8350bd-goog
The elan touchscreen datasheet says that the reset GPIO only needs to
be asserted for 500us in order to reset the regulator. The problem is
that some boards need a level shifter between the signals on the GPIO
controller and the signals on the touchscreen. All of these extra
components on the line can slow the transition of the signals. On one
board, we measured the reset line and saw that it took almost 1.8ms to
go low. Even after we bumped up the "drive strength" of the signal
from the default 2mA to 8mA we still saw it take 421us for the signal
to go low.
In order to account for this we let's lengthen the amount of time that
we keep the reset asserted. Let's bump it up from 500us to 5000us.
That's still a relatively short amount of time and is much safer.
It should be noted that this fixes real problems. Case in point:
1. The touchscreen power rail may be shared with another device (like
an eDP panel). That means that at probe time power might already be
on.
2. In probe we grab the reset GPIO and assert it (make it low).
3. We turn on power (a noop since it was already on).
4. We wait 500us.
5. We deassert the reset GPIO.
With the above case and only a 500us delay we saw only a partial reset
asserted, which is bad. Giving it 5ms is overkill but feels safer in
case someone else has a different level shifter setup.
Signed-off-by: Douglas Anderson <[email protected]>
---
drivers/input/touchscreen/elants_i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
index 879a4d984c90..377adf89b25c 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -114,7 +114,7 @@
/* calibration timeout definition */
#define ELAN_CALI_TIMEOUT_MSEC 12000
-#define ELAN_POWERON_DELAY_USEC 500
+#define ELAN_POWERON_DELAY_USEC 5000
#define ELAN_RESET_DELAY_MSEC 20
/* FW boot code version */
--
2.39.0.rc1.256.g54fd8350bd-goog
On Thu, Dec 08, 2022 at 11:20:05AM -0800, Douglas Anderson wrote:
> The touchscreen was supposed to have been added when pazquel360 first
> was added upstream but was missed. Add it now.
>
> Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
On Thu, Dec 08, 2022 at 11:20:04AM -0800, Douglas Anderson wrote:
> Now that we've added the `off-on-delay-us` for the touchpanel
> regulator, we can see that we're actually hitting that delay at
> bootup. I saw about 200 ms of delay.
>
> Let's avoid that delay by starting the regulator on. We'll only do
> this for eDP devices for the time being.
>
> NOTE: we _won't_ do this for homestar. Homestar's panel really likes
> to be power cycled. It's why the Linux driver for this panel has a
> pm_runtime_put_sync_suspend() when the panel is being unprepared but
> the normal panel-edp driver doesn't. It's also why this hardware has a
> separate power rail for eDP vs. touchscreen, unlike all the other
> trogdor boards. We won't start homestar's regulator on. While this
> could mean a slight delay on homestar, it is probably a _correct_
> delay. The bootloader might have left the regulator on (it does so in
> dev and recovery modes), so if we turned the regulator off at probe
> time and we actually hit the delay then we were probably violating T12
> in the panel spec.
>
> Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
On Thu, Dec 08, 2022 at 11:20:06AM -0800, Douglas Anderson wrote:
> The elan touchscreen datasheet says that the reset GPIO only needs to
> be asserted for 500us in order to reset the regulator. The problem is
> that some boards need a level shifter between the signals on the GPIO
> controller and the signals on the touchscreen. All of these extra
> components on the line can slow the transition of the signals. On one
> board, we measured the reset line and saw that it took almost 1.8ms to
> go low. Even after we bumped up the "drive strength" of the signal
> from the default 2mA to 8mA we still saw it take 421us for the signal
> to go low.
>
> In order to account for this we let's lengthen the amount of time that
nit: s/we let's/we/ || s/we let's/let's/
no need to re-spin just for this
> we keep the reset asserted. Let's bump it up from 500us to 5000us.
> That's still a relatively short amount of time and is much safer.
>
> It should be noted that this fixes real problems. Case in point:
> 1. The touchscreen power rail may be shared with another device (like
> an eDP panel). That means that at probe time power might already be
> on.
> 2. In probe we grab the reset GPIO and assert it (make it low).
> 3. We turn on power (a noop since it was already on).
> 4. We wait 500us.
> 5. We deassert the reset GPIO.
>
> With the above case and only a 500us delay we saw only a partial reset
> asserted, which is bad. Giving it 5ms is overkill but feels safer in
> case someone else has a different level shifter setup.
>
> Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
Hi,
On Thu, Dec 8, 2022 at 5:48 PM Dmitry Torokhov
<[email protected]> wrote:
>
> On Thu, Dec 08, 2022 at 05:38:28PM -0800, Dmitry Torokhov wrote:
> > On Thu, Dec 08, 2022 at 11:20:06AM -0800, Douglas Anderson wrote:
> > > The elan touchscreen datasheet says that the reset GPIO only needs to
> > > be asserted for 500us in order to reset the regulator. The problem is
> > > that some boards need a level shifter between the signals on the GPIO
> > > controller and the signals on the touchscreen. All of these extra
> > > components on the line can slow the transition of the signals. On one
> > > board, we measured the reset line and saw that it took almost 1.8ms to
> > > go low. Even after we bumped up the "drive strength" of the signal
> > > from the default 2mA to 8mA we still saw it take 421us for the signal
> > > to go low.
> > >
> > > In order to account for this we let's lengthen the amount of time that
> > > we keep the reset asserted. Let's bump it up from 500us to 5000us.
> > > That's still a relatively short amount of time and is much safer.
> > >
> > > It should be noted that this fixes real problems. Case in point:
> > > 1. The touchscreen power rail may be shared with another device (like
> > > an eDP panel). That means that at probe time power might already be
> > > on.
> > > 2. In probe we grab the reset GPIO and assert it (make it low).
> > > 3. We turn on power (a noop since it was already on).
> > > 4. We wait 500us.
> > > 5. We deassert the reset GPIO.
> > >
> > > With the above case and only a 500us delay we saw only a partial reset
> > > asserted, which is bad. Giving it 5ms is overkill but feels safer in
> > > case someone else has a different level shifter setup.
> > >
> > > Signed-off-by: Douglas Anderson <[email protected]>
> >
> > Applied, thank you.
>
> Unapplied ;) I guess we should follow kernel test robot's advise and
> switch to using usleep_range().
Crud. I'll send a v2 right away.
-Doug
On Thu, Dec 08, 2022 at 05:38:28PM -0800, Dmitry Torokhov wrote:
> On Thu, Dec 08, 2022 at 11:20:06AM -0800, Douglas Anderson wrote:
> > The elan touchscreen datasheet says that the reset GPIO only needs to
> > be asserted for 500us in order to reset the regulator. The problem is
> > that some boards need a level shifter between the signals on the GPIO
> > controller and the signals on the touchscreen. All of these extra
> > components on the line can slow the transition of the signals. On one
> > board, we measured the reset line and saw that it took almost 1.8ms to
> > go low. Even after we bumped up the "drive strength" of the signal
> > from the default 2mA to 8mA we still saw it take 421us for the signal
> > to go low.
> >
> > In order to account for this we let's lengthen the amount of time that
> > we keep the reset asserted. Let's bump it up from 500us to 5000us.
> > That's still a relatively short amount of time and is much safer.
> >
> > It should be noted that this fixes real problems. Case in point:
> > 1. The touchscreen power rail may be shared with another device (like
> > an eDP panel). That means that at probe time power might already be
> > on.
> > 2. In probe we grab the reset GPIO and assert it (make it low).
> > 3. We turn on power (a noop since it was already on).
> > 4. We wait 500us.
> > 5. We deassert the reset GPIO.
> >
> > With the above case and only a 500us delay we saw only a partial reset
> > asserted, which is bad. Giving it 5ms is overkill but feels safer in
> > case someone else has a different level shifter setup.
> >
> > Signed-off-by: Douglas Anderson <[email protected]>
>
> Applied, thank you.
Unapplied ;) I guess we should follow kernel test robot's advise and
switch to using usleep_range().
--
Dmitry
Hi Douglas,
I love your patch! Yet something to improve:
[auto build test ERROR on next-20221208]
[cannot apply to robh/for-next dtor-input/next dtor-input/for-linus hid/for-next v6.1-rc8 v6.1-rc7 v6.1-rc6 linus/master v6.1-rc8]
[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/Douglas-Anderson/arm64-dts-qcom-sc7180-Make-pazquel360-s-touchscreen-work/20221209-032214
patch link: https://lore.kernel.org/r/20221208111910.5.I6edfb3f459662c041563a54e5b7df727c27caaba%40changeid
patch subject: [PATCH 5/5] Input: elants_i2c: Delay longer with reset asserted
config: arm-randconfig-r046-20221207
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 6e4cea55f0d1104408b26ac574566a0e4de48036)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
# https://github.com/intel-lab-lkp/linux/commit/8e4ac288d13f505629004cb39e1621bcf877b8a3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Douglas-Anderson/arm64-dts-qcom-sc7180-Make-pazquel360-s-touchscreen-work/20221209-032214
git checkout 8e4ac288d13f505629004cb39e1621bcf877b8a3
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> ld.lld: error: undefined symbol: __bad_udelay
>>> referenced by elants_i2c.c
>>> drivers/input/touchscreen/elants_i2c.o:(elants_i2c_power_on) in archive vmlinux.a
--
0-DAY CI Kernel Test Service
https://01.org/lkp
On Thu, Dec 08, 2022 at 11:20:06AM -0800, Douglas Anderson wrote:
> The elan touchscreen datasheet says that the reset GPIO only needs to
> be asserted for 500us in order to reset the regulator. The problem is
> that some boards need a level shifter between the signals on the GPIO
> controller and the signals on the touchscreen. All of these extra
> components on the line can slow the transition of the signals. On one
> board, we measured the reset line and saw that it took almost 1.8ms to
> go low. Even after we bumped up the "drive strength" of the signal
> from the default 2mA to 8mA we still saw it take 421us for the signal
> to go low.
>
> In order to account for this we let's lengthen the amount of time that
> we keep the reset asserted. Let's bump it up from 500us to 5000us.
> That's still a relatively short amount of time and is much safer.
>
> It should be noted that this fixes real problems. Case in point:
> 1. The touchscreen power rail may be shared with another device (like
> an eDP panel). That means that at probe time power might already be
> on.
> 2. In probe we grab the reset GPIO and assert it (make it low).
> 3. We turn on power (a noop since it was already on).
> 4. We wait 500us.
> 5. We deassert the reset GPIO.
>
> With the above case and only a 500us delay we saw only a partial reset
> asserted, which is bad. Giving it 5ms is overkill but feels safer in
> case someone else has a different level shifter setup.
>
> Signed-off-by: Douglas Anderson <[email protected]>
Applied, thank you.
--
Dmitry