From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.4 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id AE12AC433E2 for ; Fri, 12 Jun 2020 08:30:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8D76020792 for ; Fri, 12 Jun 2020 08:30:37 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="L7UzKNXQ" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726264AbgFLIag (ORCPT ); Fri, 12 Jun 2020 04:30:36 -0400 Received: from us-smtp-2.mimecast.com ([207.211.31.81]:57419 "EHLO us-smtp-delivery-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726335AbgFLIaf (ORCPT ); Fri, 12 Jun 2020 04:30:35 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1591950632; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=OPWeA1qjGBNWCHfcjBnCq0fW8SyKcT5HP9U/4tli77g=; b=L7UzKNXQTHcY6HfVEPHn5AoUGhn/eafDvWcXcv/3aB11AOkv0/cWO3uCEItPFW/JZ3ipct hqkGZzeDi4OxdwUkJRbcywZhZQ4mt3aFEe7k9T1HBzlb2b5ZKSPow8YbjMQ4ktXpuXtSwe znoruux6VNXH3YhKEbWAD5dfhh/yI6Y= Received: from mail-ej1-f69.google.com (mail-ej1-f69.google.com [209.85.218.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-399-Oq2-hslcOYGcfoErqpX1xg-1; Fri, 12 Jun 2020 04:30:31 -0400 X-MC-Unique: Oq2-hslcOYGcfoErqpX1xg-1 Received: by mail-ej1-f69.google.com with SMTP id c30so3858012ejj.10 for ; Fri, 12 Jun 2020 01:30:28 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=OPWeA1qjGBNWCHfcjBnCq0fW8SyKcT5HP9U/4tli77g=; b=BMbcZ9qLvS2qYHGICCWTb8Zi8GgaFfYa6IIw2pwGWPKuWiJhf61m9IvSDCC43NJnKt Tmixv5e9n5XeLtigctpz/V/5QPP/wPG51S/TF2FZJLKrBBgAdOCxRO7YBplp0Ya9PFF4 Edi0hWI97ZqWbcUQZitAEoxpepJDytp/wt2OXwLA9ZNj2xKn1/359qxooeus0wm9B/s2 8Tytt7xFozYBgIV/DmFrQwJtxvQhsBmPJD72tH2zL48Wz/zaqHJtcvHPPWRHbwBKo9I9 6VOwVX8Y2DroJ5HqmNL2Lp8epOaDtXhJHpltMJ6zDfuRLNS7gUmmmnPxhTkkUUxofnEd gXZw== X-Gm-Message-State: AOAM533oQH1aG1C7yj63Z8Jg9kMPxke4LLCCid0Rh7o23Tb38SEIe5kV iH169RAbSr03yPWbFES7nUcGvIGyGtV4rG9aDpi0YeCmwEHBGzeVp65y4JMa4QAk/GuRUEMKHJY aIIFzIWNz0Voj0XZlpNSnhg== X-Received: by 2002:a17:906:150c:: with SMTP id b12mr11118842ejd.545.1591950627662; Fri, 12 Jun 2020 01:30:27 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzK+lnhXJPHcxkk+0REwmXPmNz4y9unDpw9LWEppS+/VYsL4Zuy/fGvFVeGkarTC5boVCicqQ== X-Received: by 2002:a17:906:150c:: with SMTP id b12mr11118797ejd.545.1591950627331; Fri, 12 Jun 2020 01:30:27 -0700 (PDT) Received: from x1.localdomain (2001-1c00-0c0c-fe00-d2ea-f29d-118b-24dc.cable.dynamic.v6.ziggo.nl. [2001:1c00:c0c:fe00:d2ea:f29d:118b:24dc]) by smtp.gmail.com with ESMTPSA id k24sm2761134edk.95.2020.06.12.01.30.25 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 12 Jun 2020 01:30:26 -0700 (PDT) Subject: Re: [PATCH v4 0/7] Support inhibiting input devices To: Andrzej Pietrasiewicz , "Rafael J. Wysocki" Cc: Linux PM , ACPI Devel Maling List , Linux Kernel Mailing List , linux-iio@vger.kernel.org, Linux ARM , Linux Samsung SoC , linux-input@vger.kernel.org, linux-tegra , patches@opensource.cirrus.com, ibm-acpi-devel@lists.sourceforge.net, Platform Driver , "Rafael J . Wysocki" , Len Brown , Jonathan Cameron , Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald-Stadler , Kukjin Kim , Krzysztof Kozlowski , Dmitry Torokhov , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , NXP Linux Team , Vladimir Zapolskiy , Sylvain Lemieux , Laxman Dewangan , Thierry Reding , Jonathan Hunter , Barry Song , Michael Hennerich , Nick Dyer , Ferruh Yigit , Sangwon Jee , Peter Hutterer , Henrique de Moraes Holschuh , Collabora Kernel ML References: <2336e15d-ff4b-bbb6-c701-dbf3aa110fcd@redhat.com> <20200608112211.12125-1-andrzej.p@collabora.com> <964ca07a-3da5-101f-7edf-64bdeec98a4b@redhat.com> <6136f26c-e090-e025-af55-4c5f3a6aec92@collabora.com> <3e61c9c1-b211-da9f-c55b-b44eb6522f2a@redhat.com> <2d5fd063-66bc-c707-4041-84a17c0a7d04@collabora.com> From: Hans de Goede Message-ID: <40988408-8f36-3a52-6439-34084de6b129@redhat.com> Date: Fri, 12 Jun 2020 10:30:25 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 MIME-Version: 1.0 In-Reply-To: <2d5fd063-66bc-c707-4041-84a17c0a7d04@collabora.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org Hi, On 6/10/20 3:41 PM, Andrzej Pietrasiewicz wrote: > Hi Hans, > > W dniu 10.06.2020 o 15:21, Hans de Goede pisze: >> Hi, >> >> On 6/10/20 3:12 PM, Andrzej Pietrasiewicz wrote: >>> Hi All, >>> >>> W dniu 10.06.2020 o 12:38, Rafael J. Wysocki pisze: >>>> On Wed, Jun 10, 2020 at 11:50 AM Hans de Goede wrote: >>>>> >>>>> Hi All, >>>>> >>>>> On 6/8/20 1:22 PM, Andrzej Pietrasiewicz wrote: >>>>>> This is a quick respin of v3, with just two small changes, please see >>>>>> the changelog below. >>>>>> >>>>>> Userspace might want to implement a policy to temporarily disregard input >>>>>> from certain devices. >>>>>> >>>>>> An example use case is a convertible laptop, whose keyboard can be folded >>>>>> under the screen to create tablet-like experience. The user then must hold >>>>>> the laptop in such a way that it is difficult to avoid pressing the keyboard >>>>>> keys. It is therefore desirable to temporarily disregard input from the >>>>>> keyboard, until it is folded back. This obviously is a policy which should >>>>>> be kept out of the kernel, but the kernel must provide suitable means to >>>>>> implement such a policy. >>>>> >>>>> First of all sorry to start a somewhat new discussion about this >>>>> while this patch set is also somewhat far along in the review process, >>>>> but I believe what I discuss below needs to be taken into account. >>>>> >>>>> Yesterday I have been looking into why an Asus T101HA would not stay >>>>> suspended when the LID is closed. The cause is that the USB HID multi-touch >>>>> touchpad in the base of the device starts sending events when the screen >>>>> gets close to the touchpad (so when the LID is fully closed) and these >>>>> events are causing a wakeup from suspend. HID multi-touch devices >>>>> do have a way to tell them to fully stop sending events, also disabling >>>>> the USB remote wakeup the device is doing. The question is when to tell >>>>> it to not send events though ... >>>>> >>>>> So now I've been thinking about how to fix this and I believe that there >>>>> is some interaction between this problem and this patch-set. >>>>> >>>>> The problem I'm seeing on the T101HA is about wakeups, so the question >>>>> which I want to discuss is: >>>>> >>>>> 1. How does inhibiting interact with enabling / >>>>> disabling the device as a wakeup source ? >>>>> >>>>> 2. Since we have now made inhibiting equal open/close how does open/close >>>>> interact with a device being a wakeup source ? >>>>> >>>>> And my own initial (to be discussed) answers to these questions: >>>>> >>>>> 1. It seems to me that when a device is inhibited it should not be a >>>>> wakeup source, so where possible a input-device-driver should disable >>>>> a device's wakeup capabilities on suspend if inhibited >>>> >>>> If "inhibit" means "do not generate any events going forward", then >>>> this must also cover wakeup events, so I agree. >>> >>> I agree, too. >>> >>>> >>>>> 2. This one is trickier I don't think we have really clearly specified >>>>> any behavior here. The default behavior of most drivers seems to be >>>>> using something like this in their suspend callback: >>>>> >>>>>           if (device_may_wakeup(dev)) >>>>>                   enable_irq_wake(data->irq); >>>>>           else if (input->users) >>>>>                   foo_stop_receiving_events(data); >>>>> >>>>> Since this is what most drivers seem to do I believe we should keep >>>>> this as is and that we should just clearly document that if the >>>>> input_device has users (has been opened) or not does not matter >>>>> for its wakeup behavior. >>>>> >>>>> Combining these 2 answers leads to this new pseudo code template >>>>> for an input-device's suspend method: >>>>> >>>>>          /* >>>>>           * If inhibited we have already disabled events and >>>>>           * we do NOT want to setup the device as wake source. >>>>>           */ >>>>>          if (input->inhibited) >>>>>                  return 0; >>> >>> Right, if a device is inhibited it shouldn't become a wakeup source, >>> because that would contradict the purpose of being inhibited. >> >> Ack. Note I do think that we need to document this (and more >> in general the answer to both questions from above) clearly so >> that going forward if there are any questions about how this is >> supposed to work we can just point to the docs. >> >> Can you do a follow-up patch, or include a patch in your next >> version which documents this (once we agree on what "this" >> exactly is) ? > > Sure I can. Just need to know when "this" becomes stable enough ;) > If this series otherwise looks mature enough I would opt for a > follow-up patch. FWIW after my flip-flop to agreeing with Dmitry that the 2 (inhibit vs wakeup) should be completely orthogonal this new policy is stable/mature from my pov (and consistent with how we handle wakeup vs input_dev->users). I still think it would be good to do a follow-up documentation patch documenting that these (and esp. inhibit) are orthogonal. This will mean for example that if a device is inhibit but still wakeup enabled and the device's close method silences the devices, that it needs to be unsilenced in suspend. This might be worth mentioning in the docs even though drivers which silence the device on close should already unsilence the device on suspend when it is wakeup-enabled. Note maybe we should give it a couple of days for others to give their opinion before you submit the follow-up documentation patch. Regards, Hans