• Douglas Anderson's avatar
    HID: i2c-hid: goodix: Stop tying the reset line to the regulator · 557e05fa
    Douglas Anderson authored
    In commit 18eeef46 ("HID: i2c-hid: goodix: Tie the reset line to
    true state of the regulator"), we started tying the reset line of
    Goodix touchscreens to the regulator.
    
    The primary motivation for that patch was some pre-production hardware
    (specifically sc7180-trogdor-homestar) where it was proposed to hook
    the touchscreen's main 3.3V power rail to an always-on supply. In such
    a case, when we turned "off" the touchscreen in Linux it was bad to
    assert the "reset" GPIO because that was causing a power drain. The
    patch accomplished that goal and did it in a general sort of way that
    didn't require special properties to be added in the device tree for
    homestar.
    
    It turns out that the design of using an always-on power rail for the
    touchscreen was rejected soon after the patch was written and long
    before sc7180-trogdor-homestar went into production. The final design
    of homestar actually fully separates the rail for the touchscreen and
    the display panel and both can be powered off and on. That means that
    the original motivation for the feature is gone.
    
    There are 3 other users of the goodix i2c-hid driver in mainline.
    
    I'll first talk about 2 of the other users in mainline: coachz and
    mrbland. On both coachz and mrbland the touchscreen power and panel
    power _are_ shared. That means that the patch to tie the reset line to
    the true state of the regulator _is_ doing something on those
    boards. Specifically, the patch reduced power consumption by tens of
    mA in the case where we turned the touchscreen off but left the panel
    on. Other than saving a small bit of power, the patch wasn't truly
    necessary. That being said, even though a small bit of power was saved
    in the state of "panel on + touchscreen off", that's not actually a
    state we ever expect to be in, except perhaps for very short periods
    of time at boot or during suspend/resume. Thus, the patch is truly not
    necessary. It should be further noted that, as documented in the
    original patch, the current code still didn't optimize power for every
    corner case of the "shared rail" situation.
    
    The last user in mainline was very recently added: evoker. Evoker is
    actually the motivation for me removing this bit of code. It turns out
    that for evoker we need to manage a second power rail for IO to the
    touchscreen. Trying to fit the management of this IO rail into the
    regulator notifiers turns out to be extremely hard. To avoid lockdep
    splats you shouldn't enable/disable other regulators in regulator
    notifiers and trying to find a way around this was going to be fairly
    difficult.
    
    Given the lack of any true motivation to tie the reset line to the
    regulator, lets go back to the simpler days and remove the code. This
    is, effectively, a revert of commit bdbc65eb77ee ("HID: i2c-hid:
    goodix: Fix a lockdep splat"), commit 25ddd7cf ("HID: i2c-hid:
    goodix: Use the devm variant of regulator_register_notifier()"), and
    commit 18eeef46 ("HID: i2c-hid: goodix: Tie the reset line to true
    state of the regulator").
    Signed-off-by: default avatarDouglas Anderson <dianders@chromium.org>
    Reviewed-by: default avatarMatthias Kaehlcke <mka@chromium.org>
    Reviewed-by: default avatarDmitry Torokhov <dmitry.torokhov@gmail.com>
    Link: https://lore.kernel.org/r/20230206184744.4.I085b32b6140c7d1ac4e7e97b712bff9dd5962b62@changeidSigned-off-by: default avatarBenjamin Tissoires <benjamin.tissoires@redhat.com>
    557e05fa
i2c-hid-of-goodix.c 3.06 KB