| Message ID | 20221208192006.1070898-1-dianders@chromium.org |
|---|---|
| Headers | show |
| Series | arm64: dts: qcom: sc7180: Make pazquel360's touchscreen work | expand |
On Thu, Dec 08, 2022 at 11:20:02AM -0800, Douglas Anderson wrote: > On at least one board (pazquel360) the reset line for the touchscreen > was scoped and found to take almost 2 ms to fall when we drove it > low. This wasn't great because the Linux driver for the touchscreen > (the elants_i2c driver) thinks it can do a 500 us reset pulse. If we > bump the drive strength to 8 mA then the reset line went down in ~421 > us. > > NOTE: we could apply this fix just for pazquel360, but: > * Probably other trogdor devices have similar timings and it's just > that nobody has noticed it before. > * There are other trogdor boards using the same elan driver that tries > to do 500 us reset pulses. > * Bumping the drive strength to 8mA across the board won't hurt. This > isn't a high speed signal or anything. > > Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
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 <dianders@chromium.org> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
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 <dianders@chromium.org> > > Applied, thank you. Unapplied ;) I guess we should follow kernel test robot's advise and switch to using usleep_range().
Hi, On Thu, Dec 8, 2022 at 5:48 PM Dmitry Torokhov <dmitry.torokhov@gmail.com> 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 <dianders@chromium.org> > > > > 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
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(-)