Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [Regression]: Commit 74d905d2 breaks the touchpad and touchscreen
 of Google Chromebook "samus"
To:     "Wang, Jiada" <jiada_wang@mentor.com>,
        Nick Dyer <nick.dyer@itdev.co.uk>
Cc:     Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        "George G. Davis" <george_davis@mentor.com>,
        linux-kernel@vger.kernel.org
References: <735253bd-5500-2b8d-c98d-99bddb0617ce@web.de>
 <beae40be-4e84-ceea-ee00-351b18b26e06@mentor.com>
 <69305cdf-1ba7-a51a-302c-9b1b83e788aa@web.de>
 <e1d1c27e-3c02-d29c-7bf4-2b302a292e70@mentor.com>
 <fd717b9c-f363-d8e8-87e9-d967bd514cd4@web.de>
 <67f8e922-941d-8538-15d7-5125356082d3@mentor.com>
From:   Andre Muller <andre.muller@web.de>
Message-ID: <2ba73d1a-30d1-48a1-e2c3-89c4da28f88a@web.de>
Date:   Tue, 24 Nov 2020 09:07:24 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.4.2
MIME-Version: 1.0
In-Reply-To: <67f8e922-941d-8538-15d7-5125356082d3@mentor.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi,

just a shot in the dark:

I consulted my notes on the convoluted initial setup of a mainline kernel
for this hardware, and found the Atmel chip needed reprogramming
to start working. The settings are preserved, once set.

I ran "enable-atmel.sh", last in 2017,
from here: https://github.com/raphael/linux-samus/tree/master/scripts/setu=
p/touchpad

I don't see or think this is related to the issue at hand,
but my expertise on this hardware is limited, to put it kindly :)

Thanks,
Andre


On 24/11/2020 07.32, Wang, Jiada wrote:
> Hi Andre
>
> Thanks for the log,
> Hmmm, from the log (also as you have observed)
> Seems "data->use_retrigen_workaround" is true on your device
> so workaround mxt_process_messages_until_invalid() is used.
> which is as same as with the commit reverted,
> I am not sure what caused IRQ get generated.
>
> @dmitry
> I would suggest to revert the commit until we find out the root cause
>
> Thanks,
> Jiada
>
>
> On 2020/11/24 15:15, Andre Muller wrote:
>> On 24/11/2020 04.02, Wang, Jiada wrote:
>>> Hello Andre
>>>
>>> Thanks for the log,
>>> can you add more debug information like following diff,
>>> and get full log?
>>
>> Hi Jiada,
>>
>> I added the warnings, but none of them triggers.
>> I double-checked the generated object file, it includes the debug strin=
gs.
>> (Also tested that touchscreen/touchpad don't work, as expected.)
>>
>> Please find the full log attached.
>>
>> Thank you,
>> Andre
>>
>>
>>>
>>> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/=
touchscreen/atmel_mxt_ts.c
>>> index 98f17fa3a892..60bccd5c42f6 100644
>>> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
>>> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
>>> @@ -1298,21 +1298,29 @@ static int mxt_check_retrigen(struct mxt_data =
*data)
>>> =A0=A0=A0=A0=A0=A0=A0=A0 data->use_retrigen_workaround =3D false;
>>>
>>> =A0=A0=A0=A0=A0=A0=A0=A0 irqd =3D irq_get_irq_data(data->irq);
>>> -=A0=A0=A0=A0=A0=A0 if (!irqd)
>>> +=A0=A0=A0=A0=A0=A0 if (!irqd) {
>>> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dev_warn(&client->dev, "un=
able to get IRQ data\n");
>>> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return -EINVAL;
>>> +=A0=A0=A0=A0=A0=A0 }
>>>
>>> -=A0=A0=A0=A0=A0=A0 if (irqd_is_level_type(irqd))
>>> +=A0=A0=A0=A0=A0=A0 if (irqd_is_level_type(irqd)) {
>>> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dev_warn(&client->dev, "IR=
Q is level type\n");
>>> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return 0;
>>> +=A0=A0=A0=A0=A0=A0 }
>>>
>>> =A0=A0=A0=A0=A0=A0=A0=A0 if (data->T18_address) {
>>> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 error =3D __mxt_read_=
reg(client,
>>> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 data->T18_address + MXT_C=
OMMS_CTRL,
>>> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 1, &val);
>>> -=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 if (error)
>>> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 if (error) {
>>> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 de=
v_warn(&client->dev, "failed to read reg: MXT_COMMS_CTRL\n");
>>> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0 return error;
>>> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 }
>>>
>>> -=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 if (val & MXT_COMMS_RETRIG=
EN)
>>> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 if (val & MXT_COMMS_RETRIG=
EN) {
>>> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 de=
v_warn(&client->dev, "RETRIGEN feature available\n");
>>> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0 return 0;
>>> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 }
>>> =A0=A0=A0=A0=A0=A0=A0=A0 }
>>>
>>> =A0=A0=A0=A0=A0=A0=A0=A0 dev_warn(&client->dev, "Enabling RETRIGEN wor=
karound\n");
>>>
>>>
>>> Thanks,
>>> Jiada
>>>
>>> On 2020/11/05 23:23, Andre Muller wrote:
>>>> On 05/11/2020 14.25, Wang, Jiada wrote:
>>>>> Hi Andre
>>>>>
>>>>> Thanks for your report,
>>>>> could you also please post the log when with this commit reverted?
>>>>>
>>>>> Thanks,
>>>>> Jiada
>>>>
>>>> Shure!
>>>> The full dmesg with the revert is attached.
>>>>
>>>> The atmel_mxt bits are:
>>>>
>>>> [=A0=A0=A0 0.195879] atmel_mxt_ts i2c-ATML0000:01: Family: 164 Varian=
t: 17 Firmware V1.0.AA Objects: 32
>>>> [=A0=A0=A0 0.211712] atmel_mxt_ts i2c-ATML0000:01: Direct firmware lo=
ad for maxtouch.cfg failed with error -2
>>>> [=A0=A0=A0 0.212986] atmel_mxt_ts i2c-ATML0000:01: Touchscreen size X=
960Y540
>>>> [=A0=A0=A0 0.213025] input: Atmel maXTouch Touchpad as /devices/pci00=
00:00/INT3432:00/i2c-0/i2c-ATML0000:01/input/input4
>>>> [=A0=A0=A0 0.219208] atmel_mxt_ts i2c-ATML0001:01: Family: 164 Varian=
t: 13 Firmware V1.0.AA Objects: 41
>>>> [=A0=A0=A0 0.238825] atmel_mxt_ts i2c-ATML0001:01: Direct firmware lo=
ad for maxtouch.cfg failed with error -2
>>>> [=A0=A0=A0 0.238949] intel_rapl_common: Found RAPL domain package
>>>> [=A0=A0=A0 0.238955] intel_rapl_common: Found RAPL domain core
>>>> [=A0=A0=A0 0.238961] intel_rapl_common: Found RAPL domain uncore
>>>> [=A0=A0=A0 0.238966] intel_rapl_common: Found RAPL domain dram
>>>> [=A0=A0=A0 0.240121] atmel_mxt_ts i2c-ATML0001:01: Touchscreen size X=
2559Y1699
>>>> [=A0=A0=A0 0.240157] input: Atmel maXTouch Touchscreen as /devices/pc=
i0000:00/INT3433:00/i2c-1/i2c-ATML0001:01/input/input5
>>>>
>>>> Regards,
>>>> Andre
>>>>
>>>>>
>>>>> On 2020/11/04 17:13, Andre wrote:
>>>>>> Hi,
>>>>>>
>>>>>> commit 74d905d2: Input: atmel_mxt_ts - only read messages in
>>>>>> mxt_acquire_irq() when necessary
>>>>>>
>>>>>> breaks the touchpad and touchscreen of the 2015 Chromebook Pixel "S=
amus".
>>>>>>
>>>>>> Reverting the commit from the current git tree gets them to work ag=
ain.
>>>>>>
>>>>>> I am not at all shure what info to include, but I will happily prov=
ide
>>>>>> it on request.
>>>>>>
>>>>>> The dmesgs of a boot with commit 74d905d2 show "Enabling RETRIGEN
>>>>>> workaround", but otherwise looks the same as a boot without.
>>>>>>
>>>>>> Here is the relevant bit (with 74d905d2):
>>>>>>
>>>>>> atmel_mxt_ts i2c-ATML0000:01: Family: 164 Variant: 17 Firmware V1.0=
.AA
>>>>>> Objects: 32
>>>>>> atmel_mxt_ts i2c-ATML0000:01: Enabling RETRIGEN workaround
>>>>>> atmel_mxt_ts i2c-ATML0000:01: Direct firmware load for maxtouch.cfg
>>>>>> failed with error -2
>>>>>> atmel_mxt_ts i2c-ATML0000:01: Touchscreen size X960Y540
>>>>>> input: Atmel maXTouch Touchpad as
>>>>>> /devices/pci0000:00/INT3432:00/i2c-0/i2c-ATML0000:01/input/input4
>>>>>> atmel_mxt_ts i2c-ATML0001:01: Family: 164 Variant: 13 Firmware V1.0=
.AA
>>>>>> Objects: 41
>>>>>> atmel_mxt_ts i2c-ATML0001:01: Enabling RETRIGEN workaround
>>>>>> atmel_mxt_ts i2c-ATML0001:01: Direct firmware load for maxtouch.cfg
>>>>>> failed with error -2
>>>>>>
>>>>>> Thank you,
>>>>>> Andre M=FCller
>>>>
>>