Hi, On Tue, Apr 23, 2024 at 6:46 AM Johan Hovold <johan+linaro@xxxxxxxxxx> wrote: > > The Elan eKTH5015M touch controller on the X13s requires a 300 ms delay > before sending commands after having deasserted reset during power on. > > This series switches the X13s devicetree to use the Elan specific > binding so that the OS can determine the required power-on sequence and > make sure that the controller is always detected during boot. [1] > > The Elan hid-i2c driver currently asserts reset unconditionally during > suspend, which does not work on the X13s where the touch controller > supply is shared with other peripherals that may remain powered. Holding > the controller in reset can increase power consumption and also leaks > current through the reset circuitry pull ups. Can you provide more details about which devices exactly it shares power with? I'm worried that you may be shooting yourself in the foot to avoid shooting yourself in the arm. Specifically, if those other peripherals that may remain powered ever power themselves off then you'll end up back-driving the touchscreen through the reset line, won't you? Since reset is active low then not asserting reset drives the reset line high and, if you power it off, it can leach power backwards through the reset line. The "goodix,no-reset-during-suspend" property that I added earlier specifically worked on systems where the rail was always-on so I could guarantee that didn't happen. >From looking at your dts patch it looks like your power _is_ on an always-on rail so you should be OK, but it should be documented that this only works for always-on rails. ...also, from your patch description it sounds as if (maybe?) you intend to eventually let the rail power off if the trackpad isn't a wakeup source. If you eventually plan to do that then you definitely need something more complex here... > Note that the latter also affects X13s variants where the touchscreen is > not populated as the driver also exits probe() with reset asserted. I assume driving against an external pull is _probably_ not a huge deal (should be a pretty small amount of power), but I agree it would be nice to fix. I'm a bit leery of actively driving the reset pin high (deasserting the reset) just to match the pull. It feels like in your case it would be better to make it an input w/ no pulls. It almost feels like something in the pinctrl system should handle this. Something where the pin is default "input no pull" at the board level and when the driver exits it should go back to the pinctrl default... I guess one last thought is: what do we do if/when someone needs the same solution but they want multiple sources of touchscreens, assuming we ever get the second-sourcing problem solved well. In that case the different touchscreen drivers might have a different idea of how the GPIO should be left when the driver exits... -Doug
