Message-ID: <c0ca6ca1-ffee-4b12-bf96-ee9efb93c4d2@kernel.org>
Date: Tue, 2 Sep 2025 12:27:24 +0200
From: Konrad Dybcio <konradybcio@...nel.org>
To: Sebastian Reichel <sre@...nel.org>,
 Mark Pearson <mpearson-lenovo@...ebb.ca>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Hans de Goede <hansg@...nel.org>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
 Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
 Bjorn Andersson <andersson@...nel.org>,
 Konrad Dybcio <konradybcio@...nel.org>,
 "Derek J . Clark" <derekjohn.clark@...il.com>,
 Henrique de Moraes Holschuh <hmh@....eng.br>, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org,
 "platform-driver-x86@...r.kernel.org" <platform-driver-x86@...r.kernel.org>,
 linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH 2/3] platform: arm64: thinkpad-t14s-ec: new driver

On 9/1/25 6:10 PM, Sebastian Reichel wrote:
> Hello Mark,
> 
> On Mon, Sep 01, 2025 at 09:48:39AM -0400, Mark Pearson wrote:
>> On Sun, Aug 31, 2025, at 5:28 PM, Sebastian Reichel wrote:
>>> Introduce EC driver for the ThinkPad T14s Gen6 Snapdragon, which
>>> is in theory compatible with ThinkPad ACPI. On Linux the system
>>> is booted with device tree, which is not supported by the ThinkPad
>>> ACPI driver. Also most of the hardware compatibility is handled
>>> via ACPI tables, which are obviously not used when booting via
>>> device tree. Thus adding DT compatibility to the existing driver
>>> is not worth it (almost no code sharing).
>>>
>>> The driver currently exposes features, which are not available
>>> via other means:
>>>
>>>  * Extra Keys
>>>  * System LEDs
>>>  * Keyboard Backlight Control
>>>
>>> The driver has been developed by reading the ACPI DSDT. There
>>> are some more features around thermal control, which are not
>>> yet supported by the driver.
>>>
>>
>> Thanks for working on this - it's great.
> 
> It's a personal scratch your own itch project, as I daily drive the
> machine.
> 
>> I'll see if I can get the EC spec so I can do some checking on the
>> values (I thought I had it already, but I can't find it). If this
>> file can be used for other platforms then it might be good to
>> rename the file to not be specific to the t14s? I'm curious if it
>> can be used on the X13s or the Yoga platform.
> 
> Maybe. I only have the T14s (apart of my older Intel/AMD ThinkPads,
> which use the ACPI driver). The ACPI DSDT functions completley
> abstract the lowlevel I2C interface, so in theory every ThinkPad
> could have a completley different EC and still use the same ACPI
> driver. So this needs to be checked per-device. Hopefully the low
> level interface is similar in those, so that we don't need to spam
> the kernel tree with multiple different EC drivers :)
> 
>> Couple of notes
>>  - I do agree it doesn't make sense to add this to thinkpad_acpi.
>>    That file is too big anyway.
>>  - If there are other pieces like this where some detail of the
>>    platform is needed, please do let me know. I never got enough
>>    time to work on this platform directly, and it wasn't in our
>>    Linux program, but I do have access and support from the
>>    platform team for getting details on it. If I can help, so not
>>    too much reverse engineering is needed, I'm happy to.
> 
> Thanks for the offer.
> 
> I would be interested in bits around system suspend. Right now
> support on X1E is limited to sending the CPU into suspend. Much of
> the machine seems to be still powered. Right now the keyboard
> backlight and all the status LEDs stay on and the LID + power led
> does not go into the typical breathing pattern. Additionally I had
> to disable wakeup capabilities for the EC interrupt, as closing the
> LID generates an event and thus an interrupt, which wakes the
> system. Obviousy that is undesired from user's perspective. My guess
> is, that there might be some register to mask events, but I haven't
> found it so far. Alternatively the EC might mask them automatically
> when the system is send into suspend, which I also have not yet
> figured out :) The only bit I know is, that EC register 0xE0 is
> involved in modern standby.
> 
> Apart from that and (probably) unrelated to the EC: I noticed that
> accessing the built-in webcam (with the X1E camera patches from
> Bryan O'Donoghue) does not enable the status LED. It would be
> nice if you can check how that is wired, so that it can be enabled
> when a camera stream is started.

FWIW a couple years ago I tried to do something similar for the X13s
EC, and the software interface looks somewhat familiar..

This never ended up becoming anything big, but just in case this is
useful for anyone:

https://github.com/SoMainline/linux/commit/c0cc2c60177a33597c33586bfe27b5f440e36745

Konrad

Powered by blists - more mailing lists