Message-ID: <9b004b3d-deed-1b63-2344-a445a9e53b61@redhat.com>
Date:   Thu, 23 Dec 2021 15:42:24 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.0
Subject: Re: [PATCH 2/3] Input: elan_i2c - Use PM subsystem to manage wake irq
Content-Language: en-US
To:     Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        Raul E Rangel <rrangel@chromium.org>
Cc:     linux-kernel@vger.kernel.org, mario.limonciello@amd.com,
        linux-input@vger.kernel.org, dianders@chromium.org,
        "jingle.wu" <jingle.wu@emc.com.tw>
References: <20211220234346.2798027-1-rrangel@chromium.org>
 <20211220163823.2.Id022caf53d01112188308520915798f08a33cd3e@changeid>
 <YcE+xrSnS7qw0G1/@google.com>
From:   Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <YcE+xrSnS7qw0G1/@google.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi,

On 12/21/21 03:41, Dmitry Torokhov wrote:
> Hi Raul,
> 
> On Mon, Dec 20, 2021 at 04:43:45PM -0700, Raul E Rangel wrote:
>> @@ -1368,11 +1367,13 @@ static int elan_probe(struct i2c_client *client,
>>  	}
>>  
>>  	/*
>> -	 * Systems using device tree should set up wakeup via DTS,
>> +	 * Systems using device tree or ACPI should set up wakeup via DTS/ACPI,
>>  	 * the rest will configure device as wakeup source by default.
>>  	 */
>> -	if (!dev->of_node)
>> +	if (!dev->of_node && !ACPI_COMPANION(dev)) {
> 
> I think this will break our Rambis that use ACPI for enumeration but
> actually lack _PRW. As far as I remember their trackpads were capable
> of waking up the system.
> 
> I think we should remove this chunk completely and instead add necessary
> code to drivers/platform/chrome/chrome-laptop.c (I suppose we need to
> have additional member in struct acpi_peripheral to indicate whether
> device needs to be configured for wakeup and then act upon it in
> chromeos_laptop_adjust_client().
> 
>>  		device_init_wakeup(dev, true);
>> +		dev_pm_set_wake_irq(dev, client->irq);
>> +	}

As I already mentioned in my other reply in this thread:

https://lore.kernel.org/linux-input/f594afab-8c1a-8821-a775-e5512e17ce8f@redhat.com/

AFAICT most x86 ACPI laptops do not use GPEs for wakeup by touchpad and
as such they do not have a _PRW method.

So for wakeup by elan_i2c touchpads to keep working this code is not
just necessary for some ChromeOS devices, but it is necessary on
most ACPI devices.

The problem of not making these calls on devices where a GPE is actually
used for touchpad wakeup (which at least for now is the exception not
the rule) should probably be fixed by no running this "chunk"
when the device has an ACPI_COMPANION (as this patch already checks)
*and* that ACPI_COMPANION has a valid _PRW method.

Simply removing this chunk, or taking this patch as is will very
likely lead to regressions on various x86 laptop models.

Regards,

Hans