* Re: [PATCH v1] Input: Drop unused assignments from pnp_device_id arrays
From: Dmitry Torokhov @ 2026-06-10 16:57 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub)
Cc: Kees Cook, Werner Sembach, Christoffer Sandberg, feng, gongqi,
linux-input, linux-kernel
In-Reply-To: <ailQutR_TfyCQ-SP@monoceros>
On Wed, Jun 10, 2026 at 01:58:17PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> Hello Dmitry,
>
> On Tue, Jun 09, 2026 at 11:12:49PM -0700, Dmitry Torokhov wrote:
> > On Tue, Jun 09, 2026 at 04:53:25PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> > > Explicitly assigning .driver_data in drivers that don't use this member
> > > is silly and a bit irritating. Drop these. Also simplify the list
> > > terminator entry to be just empty to match what most other device_id
> > > tables do.
> > >
> > > There is no changed semantic, not even a change in the compiled result.
> > >
> > > Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
> > > ---
> > > drivers/input/gameport/ns558.c | 46 +++++++++++-----------
> > > drivers/input/serio/i8042-acpipnpio.h | 56 +++++++++++++--------------
> > > 2 files changed, 51 insertions(+), 51 deletions(-)
> > >
> > > diff --git a/drivers/input/gameport/ns558.c b/drivers/input/gameport/ns558.c
> > > index fdece6ec1df3..f70a96c4f1fd 100644
> > > --- a/drivers/input/gameport/ns558.c
> > > +++ b/drivers/input/gameport/ns558.c
> > > @@ -148,29 +148,29 @@ static int ns558_isa_probe(int io)
> > > #ifdef CONFIG_PNP
> > >
> > > static const struct pnp_device_id pnp_devids[] = {
> > > - [...]
> > > - { .id = "", },
> > > + [...]
> > > + { }
> >
> > This goes BOOOM! You have to keep empty .id string as terminator.
>
> How so? Given that my patch doesn't modify the resulting ns558.o I doubt
> that. If .id was a char *, I'd agree, but it's a char[], so there should
> be no difference (and the compiler agrees).
Sorry, brain fart on my part.
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] ARM: footbridge: convert to sparse IRQs
From: Dmitry Torokhov @ 2026-06-10 17:03 UTC (permalink / raw)
To: Ethan Nelson-Moore
Cc: Arnd Bergmann, linux-arm-kernel, linux-input, linux-serial,
Russell King, Greg Kroah-Hartman, Jiri Slaby, Russell King,
Linus Walleij, Kees Cook, Nathan Chancellor,
Sebastian Andrzej Siewior, Steven Rostedt, Thomas Weißschuh,
Peter Zijlstra
In-Reply-To: <CADkSEUhh1NdOMTHVsErhqzyCpDGFA-FkNFaWp94e9LnB3njxqw@mail.gmail.com>
On Mon, Jun 08, 2026 at 10:13:50AM -0700, Ethan Nelson-Moore wrote:
> Hi, Arnd,
>
> On Mon, Jun 8, 2026 at 10:11 AM Arnd Bergmann <arnd@arndb.de> wrote:
> > I think this is correct, as footbridge is the only one that selects
> > CONFIG_ARCH_MIGHT_HAVE_PC_SERIO and defines I8042_KBD_IRQ on arm.
>
> I came to the same conclusion.
I see. In this case:
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> # for input
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] amd-sfh-hid: tablet mode switch and asus quirk
From: Basavaraj Natikar @ 2026-06-10 17:12 UTC (permalink / raw)
To: Helge Bahmann, Jiri Kosina, linux-input; +Cc: Basavaraj Natikar, bentiss
In-Reply-To: <2632507.ElGaqSPkdT@lothlorien>
On 5/14/2026 1:29 PM, Helge Bahmann wrote:
> [You don't often get email from hcb@chaoticmind.net. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> On Tue, 12 May 2026, Jiri Kosina wrote:
>> On Mon, 27 Apr 2026, Helge Bahmann wrote:
>>
>>> Add an input driver that interprets the "operation mode" sensor offered
>>> by the amd sfh on some laptop models.
>>>
>>> Add a quirk to make the driver work again with the Asus VivoBook
>>> VivoBook (turn off the "disable interrupts" flag).
>>>
>>> Expose the intr_disable flag as a module parameter in case it turns out
>>> to be needed on further laptop models.
>>>
>>> Signed-off-by: Helge Bahmann <hcb@chaoticmind.net>
>> Basavaraj, can you please review this one?
> Some additional context, maybe helpful for review:
>
> 1. The numbers and behavior were extracted from the ACPI tables
> (WMI driver of sorts) of the notebook; I don't have access to any
> official AMD / ASUS docs or similar.
>
> 2. I have an alternate version of this change that is more indirect:
> - create a HID driver providing an "abstract table mode" message
> - have an input driver attaching to this newly defined HID driver
>
> While that is keeping "more in line" with the current driver
> architecture, I am not sure this indirection really helps. Particularly,
> there is no "canonical" HID tablet mode switch message defined,
> so it all remains completely bespoke. I am happy to change it if
> you prefer, but would need your input.
>
> 3. Since this is based on Asus VivoBook and its ACPI tables,
> there is a possibility that this "op sensor / tablet mode" behavior
> is not as universal as I surmise. A point could be made to make this
> entire behavior model-dependent (with a mod param to override
> / activate for other models). Happy to take input / advice.
Thanks Helge.
I'd like to go with the dedicated input-driver approach (your option with
a standalone input driver) rather than the HID-message indirection -- it
keeps clean subsystem boundaries.
For splitting the work, either of these works for me -- whichever you
prefer:
Option 1: I create the new input driver
drivers/input/misc/amd_sfh_tabletmode.c and once done we will review your ASUS VivoBook
quirk + intr_disable module-param patches on top of it.
Option 2: If you'd rather keep ownership of it, you write
drivers/input/misc/amd_sfh_tabletmode.c consuming the mode exposed by
amd-sfh, and I'll review and help.
Let me know which option you'd like and I'll proceed.
Thanks,
--
Basavaraj
^ permalink raw reply
* Re: [PATCH] input: remove changelogs, tracked in git
From: Dmitry Torokhov @ 2026-06-10 18:05 UTC (permalink / raw)
To: Elliot Tester; +Cc: linux-input, linux-kernel
In-Reply-To: <20260514193302.117488-1-elliotctester1@gmail.com>
On Thu, May 14, 2026 at 09:33:02PM +0200, Elliot Tester wrote:
> Signed-off-by: Elliot Tester <elliotctester1@gmail.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 0/7] ASUS Zenbook Duo keyboard support
From: Jiri Kosina @ 2026-06-10 18:06 UTC (permalink / raw)
To: Paolo Pisati; +Cc: Benjamin Tissoires, linux-input
In-Reply-To: <5sn10614-2s0s-7r22-s9p9-5np4921p047n@xreary.bet>
On Wed, 10 Jun 2026, Jiri Kosina wrote:
> > Add support for the ASUS Zenbook Duo line of usb/BT wireless convertible keyboards.
> >
> > This patchset is a collective effort, gathered here:
> >
> > https://github.com/NeroReflex/asusctl/issues/25
>
> Applied to hid.git#for-7.2/asus, thanks.
And now dropped due to build breakage reported by Benjamin's CI:
https://gitlab.freedesktop.org/bentiss/hid/-/jobs/102074691
Please fix that and resubmit, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] Input: ads7846 - consolidate coordinate filtering logic
From: Aaro Koskinen @ 2026-06-10 18:26 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Kris Bahnsen, linux-input, Mark Featherston, Marek Vasut,
linux-kernel
In-Reply-To: <afzo03UP-shPfNMe@google.com>
Hi,
On Thu, May 07, 2026 at 12:35:04PM -0700, Dmitry Torokhov wrote:
> I think if you pull 'next' branch of my tree, then the following patches
> seem to apply cleanly on 7.0:
>
> c68bc840f06c ("Input: ads7846 - restore half-duplex support")
> 011bdf9f3a9d ("Input: ads7846 - consolidate coordinate filtering logic")
>
> I hope they also build ;)
>
> c68bc840f06c should appear in linux-next soon too.
Did you forgot to push this, or was it dropped? Because I fail to see
it any any tree...
A.
^ permalink raw reply
* Re: [PATCH] HID: uhid: convert to hid_safe_input_report()
From: Jiri Kosina @ 2026-06-10 18:32 UTC (permalink / raw)
To: Carlos Llamas
Cc: David Rheinsberg, Benjamin Tissoires, Lee Jones, kernel-team,
linux-kernel, stable, open list:UHID USERSPACE HID IO DRIVER
In-Reply-To: <20260606181552.3095967-1-cmllamas@google.com>
On Sat, 6 Jun 2026, Carlos Llamas wrote:
> Commit 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
> bogus memset()"), added a check in hid_report_raw_event() to reject
> reports if the received data size is smaller than expected. This was
> intended to prevent OOB errors by no longer allowing zeroing-out of
> shorter reports due to the lack of buffer size information.
>
> However, this leads to regressions in hid_report_raw_event(), where
> shorter than expected reports are rejected, even though their buffers
> are sufficiently large to be zero-padded.
>
> To solve this issue, Benjamin introduced a safer alternative in commit
> 206342541fc8 ("HID: core: introduce hid_safe_input_report()"), which
> forwards the buffer size and allows hid_report_raw_event() to safely
> zero-pad the data.
>
> Convert uhid to use hid_safe_input_report() and pass UHID_DATA_MAX as
> the buffer size. This prevents the reported regressions [1], allowing
> hid core to zero-pad the shorter reports safely as expected.
>
> Cc: stable@vger.kernel.org
> Fixes: 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing bogus memset()")
> Closes: https://lore.kernel.org/all/ahsh0UtTX6e0ZeHa@google.com/ [1]
> Signed-off-by: Carlos Llamas <cmllamas@google.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* [dtor-input:next] BUILD SUCCESS ead562291438b5657d7e4d5e8d6d54611b132370
From: kernel test robot @ 2026-06-10 19:02 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
branch HEAD: ead562291438b5657d7e4d5e8d6d54611b132370 Input: ipaq-micro-keys - simplify allocation
elapsed time: 761m
configs tested: 236
configs skipped: 3
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc-16.1.0
alpha allyesconfig gcc-16.1.0
alpha defconfig gcc-16.1.0
arc allmodconfig clang-23
arc allmodconfig gcc-16.1.0
arc allnoconfig gcc-16.1.0
arc allyesconfig clang-23
arc allyesconfig gcc-16.1.0
arc defconfig gcc-16.1.0
arc randconfig-001-20260610 gcc-8.5.0
arc randconfig-001-20260611 gcc-14.3.0
arc randconfig-002-20260610 gcc-8.5.0
arc randconfig-002-20260611 gcc-14.3.0
arm allnoconfig clang-23
arm allnoconfig gcc-16.1.0
arm allyesconfig clang-23
arm allyesconfig gcc-16.1.0
arm defconfig gcc-16.1.0
arm mvebu_v7_defconfig clang-23
arm randconfig-001-20260610 gcc-8.5.0
arm randconfig-001-20260611 gcc-14.3.0
arm randconfig-002-20260610 gcc-8.5.0
arm randconfig-002-20260611 gcc-14.3.0
arm randconfig-003-20260610 gcc-8.5.0
arm randconfig-003-20260611 gcc-14.3.0
arm randconfig-004-20260610 gcc-8.5.0
arm randconfig-004-20260611 gcc-14.3.0
arm64 allmodconfig clang-23
arm64 allnoconfig gcc-16.1.0
arm64 defconfig gcc-16.1.0
arm64 randconfig-001-20260610 gcc-11.5.0
arm64 randconfig-002-20260610 gcc-11.5.0
arm64 randconfig-003-20260610 gcc-11.5.0
arm64 randconfig-004-20260610 gcc-11.5.0
csky allmodconfig gcc-16.1.0
csky allnoconfig gcc-16.1.0
csky defconfig gcc-16.1.0
csky randconfig-001-20260610 gcc-11.5.0
csky randconfig-002-20260610 gcc-11.5.0
hexagon allmodconfig clang-23
hexagon allmodconfig gcc-16.1.0
hexagon allnoconfig clang-23
hexagon allnoconfig gcc-16.1.0
hexagon defconfig gcc-16.1.0
hexagon randconfig-001-20260610 clang-22
hexagon randconfig-002-20260610 clang-22
i386 allmodconfig clang-22
i386 allmodconfig gcc-14
i386 allnoconfig gcc-14
i386 allnoconfig gcc-16.1.0
i386 allyesconfig clang-22
i386 allyesconfig gcc-14
i386 buildonly-randconfig-001-20260610 gcc-14
i386 buildonly-randconfig-002-20260610 gcc-14
i386 buildonly-randconfig-003-20260610 gcc-14
i386 buildonly-randconfig-004-20260610 gcc-14
i386 buildonly-randconfig-005-20260610 gcc-14
i386 buildonly-randconfig-006-20260610 gcc-14
i386 defconfig gcc-16.1.0
i386 randconfig-001-20260610 gcc-14
i386 randconfig-002-20260610 gcc-14
i386 randconfig-003-20260610 gcc-14
i386 randconfig-004-20260610 gcc-14
i386 randconfig-005-20260610 gcc-14
i386 randconfig-006-20260610 gcc-14
i386 randconfig-007-20260610 gcc-14
i386 randconfig-011-20260610 gcc-14
i386 randconfig-012-20260610 gcc-14
i386 randconfig-013-20260610 gcc-14
i386 randconfig-014-20260610 gcc-14
i386 randconfig-015-20260610 gcc-14
i386 randconfig-016-20260610 gcc-14
i386 randconfig-017-20260610 gcc-14
loongarch allmodconfig clang-19
loongarch allmodconfig clang-23
loongarch allnoconfig clang-20
loongarch allnoconfig gcc-16.1.0
loongarch defconfig clang-23
loongarch randconfig-001-20260610 clang-22
loongarch randconfig-002-20260610 clang-22
m68k allmodconfig gcc-16.1.0
m68k allnoconfig gcc-16.1.0
m68k allyesconfig clang-23
m68k allyesconfig gcc-16.1.0
m68k apollo_defconfig gcc-16.1.0
m68k defconfig clang-23
m68k m5272c3_defconfig gcc-16.1.0
microblaze allnoconfig gcc-16.1.0
microblaze allyesconfig gcc-16.1.0
microblaze defconfig clang-23
mips allmodconfig gcc-16.1.0
mips allnoconfig gcc-16.1.0
mips allyesconfig gcc-16.1.0
nios2 allmodconfig clang-20
nios2 allmodconfig gcc-11.5.0
nios2 allnoconfig clang-23
nios2 allnoconfig gcc-11.5.0
nios2 defconfig clang-23
nios2 randconfig-001-20260610 clang-22
nios2 randconfig-002-20260610 clang-22
openrisc allmodconfig clang-20
openrisc allmodconfig gcc-16.1.0
openrisc allnoconfig clang-23
openrisc allnoconfig gcc-16.1.0
openrisc defconfig gcc-16.1.0
parisc allmodconfig gcc-16.1.0
parisc allnoconfig clang-23
parisc allnoconfig gcc-16.1.0
parisc allyesconfig clang-17
parisc allyesconfig clang-23
parisc allyesconfig gcc-16.1.0
parisc defconfig gcc-16.1.0
parisc randconfig-001-20260610 gcc-8.5.0
parisc randconfig-001-20260611 gcc-13.4.0
parisc randconfig-002-20260610 gcc-8.5.0
parisc randconfig-002-20260611 gcc-13.4.0
parisc64 defconfig clang-23
powerpc allmodconfig gcc-16.1.0
powerpc allnoconfig clang-23
powerpc allnoconfig gcc-16.1.0
powerpc randconfig-001-20260610 gcc-8.5.0
powerpc randconfig-001-20260611 gcc-13.4.0
powerpc randconfig-002-20260610 gcc-8.5.0
powerpc randconfig-002-20260611 gcc-13.4.0
powerpc64 randconfig-001-20260610 gcc-8.5.0
powerpc64 randconfig-001-20260611 gcc-13.4.0
powerpc64 randconfig-002-20260610 gcc-8.5.0
powerpc64 randconfig-002-20260611 gcc-13.4.0
riscv allmodconfig clang-23
riscv allnoconfig clang-23
riscv allnoconfig gcc-16.1.0
riscv allyesconfig clang-23
riscv defconfig gcc-16.1.0
riscv randconfig-001 gcc-16.1.0
riscv randconfig-001-20260610 gcc-16.1.0
riscv randconfig-002 gcc-16.1.0
riscv randconfig-002-20260610 gcc-16.1.0
s390 allmodconfig clang-17
s390 allmodconfig clang-23
s390 allnoconfig clang-23
s390 allyesconfig gcc-16.1.0
s390 defconfig gcc-16.1.0
s390 randconfig-001 gcc-16.1.0
s390 randconfig-001-20260610 gcc-16.1.0
s390 randconfig-002 gcc-16.1.0
s390 randconfig-002-20260610 gcc-16.1.0
sh allmodconfig gcc-16.1.0
sh allnoconfig clang-23
sh allnoconfig gcc-16.1.0
sh allyesconfig clang-17
sh allyesconfig clang-23
sh allyesconfig gcc-16.1.0
sh defconfig gcc-14
sh randconfig-001 gcc-16.1.0
sh randconfig-001-20260610 gcc-16.1.0
sh randconfig-002 gcc-16.1.0
sh randconfig-002-20260610 gcc-16.1.0
sparc allnoconfig clang-23
sparc allnoconfig gcc-16.1.0
sparc defconfig gcc-16.1.0
sparc randconfig-001 gcc-14.3.0
sparc randconfig-001-20260610 gcc-14.3.0
sparc randconfig-001-20260611 gcc-15.2.0
sparc randconfig-002 gcc-14.3.0
sparc randconfig-002-20260610 gcc-14.3.0
sparc randconfig-002-20260611 gcc-15.2.0
sparc64 allmodconfig clang-20
sparc64 defconfig gcc-14
sparc64 randconfig-001 gcc-14.3.0
sparc64 randconfig-001-20260610 gcc-14.3.0
sparc64 randconfig-001-20260611 gcc-15.2.0
sparc64 randconfig-002 gcc-14.3.0
sparc64 randconfig-002-20260610 gcc-14.3.0
sparc64 randconfig-002-20260611 gcc-15.2.0
um allmodconfig clang-17
um allmodconfig clang-23
um allnoconfig clang-16
um allnoconfig clang-23
um allyesconfig gcc-14
um allyesconfig gcc-16.1.0
um defconfig gcc-14
um i386_defconfig gcc-14
um randconfig-001 gcc-14.3.0
um randconfig-001-20260610 gcc-14.3.0
um randconfig-001-20260611 gcc-15.2.0
um randconfig-002 gcc-14.3.0
um randconfig-002-20260610 gcc-14.3.0
um randconfig-002-20260611 gcc-15.2.0
um x86_64_defconfig gcc-14
x86_64 allmodconfig clang-22
x86_64 allnoconfig clang-22
x86_64 allnoconfig clang-23
x86_64 allyesconfig clang-22
x86_64 buildonly-randconfig-001-20260610 gcc-14
x86_64 buildonly-randconfig-002-20260610 gcc-14
x86_64 buildonly-randconfig-003-20260610 gcc-14
x86_64 buildonly-randconfig-004-20260610 gcc-14
x86_64 buildonly-randconfig-005-20260610 gcc-14
x86_64 buildonly-randconfig-006-20260610 gcc-14
x86_64 defconfig gcc-14
x86_64 kexec clang-22
x86_64 randconfig-001-20260610 gcc-13
x86_64 randconfig-002-20260610 gcc-13
x86_64 randconfig-003-20260610 gcc-13
x86_64 randconfig-004-20260610 gcc-13
x86_64 randconfig-005-20260610 gcc-13
x86_64 randconfig-006-20260610 gcc-13
x86_64 randconfig-011-20260610 gcc-14
x86_64 randconfig-012-20260610 gcc-14
x86_64 randconfig-013-20260610 gcc-14
x86_64 randconfig-014-20260610 gcc-14
x86_64 randconfig-015-20260610 gcc-14
x86_64 randconfig-016-20260610 gcc-14
x86_64 randconfig-071-20260610 gcc-14
x86_64 randconfig-072-20260610 gcc-14
x86_64 randconfig-073-20260610 gcc-14
x86_64 randconfig-074-20260610 gcc-14
x86_64 randconfig-075-20260610 gcc-14
x86_64 randconfig-076-20260610 gcc-14
x86_64 rhel-9.4 clang-22
x86_64 rhel-9.4-bpf gcc-14
x86_64 rhel-9.4-func clang-22
x86_64 rhel-9.4-kselftests clang-22
x86_64 rhel-9.4-kunit gcc-14
x86_64 rhel-9.4-ltp gcc-14
x86_64 rhel-9.4-rust clang-22
xtensa allnoconfig clang-23
xtensa allnoconfig gcc-16.1.0
xtensa allyesconfig clang-20
xtensa allyesconfig gcc-16.1.0
xtensa randconfig-001 gcc-14.3.0
xtensa randconfig-001-20260610 gcc-14.3.0
xtensa randconfig-001-20260611 gcc-15.2.0
xtensa randconfig-002 gcc-14.3.0
xtensa randconfig-002-20260610 gcc-14.3.0
xtensa randconfig-002-20260611 gcc-15.2.0
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v2] HID: intel-thc-hid: intel-quickspi: reset touch IC on system resume
From: Jiri Kosina @ 2026-06-10 20:08 UTC (permalink / raw)
To: d3z
Cc: even.xu, bentiss, xinpeng.sun, linux-input, linux-kernel,
sakari.ailus, abhishektamboli9
In-Reply-To: <20260601211828.112626-1-d3z.the.dev@gmail.com>
On Tue, 2 Jun 2026, d3z wrote:
> From: Danny D. <d3z.the.dev@gmail.com>
>
> On the Surface Pro 10 (Meteor Lake) the touchscreen stops working after a
> suspend/resume cycle and only recovers after a reboot. The driver logs
> "GET_DEVICE_INFO: recv failed: -11" on resume.
>
> This platform suspends through s2idle: /sys/power/mem_sleep exposes
> "[s2idle]" as the only state, there is no "deep"/S3 entry at all. The
> touch IC nonetheless loses power across that s2idle suspend, the same
> way it does across hibernation. quickspi_resume() only re-selects the
> THC port, restores interrupts and DMA and sends a HIDSPI_ON command,
> assuming the touch IC kept its power and state. When it has actually
> lost power the HIDSPI_ON command is never acknowledged and the
> descriptor read fails, leaving the touchscreen dead until the module is
> reloaded.
>
> quickspi_restore() already handles this for hibernation by
> reconfiguring the THC SPI/LTR settings and running reset_tic() to
> re-enumerate the device. Make quickspi_resume() do the same when the
> device is not a wake source. A wake-enabled device keeps its power and
> state across suspend, so it stays on the light restore path: resetting
> it would discard a pending wake touch event and break wake-on-touch.
>
> The non-wake path mirrors the existing quickspi_restore() sequence,
> including enabling interrupts before reset_tic(), so it introduces no
> new ordering relative to code already in the driver.
>
> This change has been validated on a Surface Pro 10 running the
> linux-surface kernel across multiple s2idle suspend/resume cycles; it
> has not been tested on a mainline build.
>
> Closes: https://github.com/linux-surface/linux-surface/issues/1799
> Signed-off-by: Danny D. <d3z.the.dev@gmail.com>
> ---
> v1 -> v2:
> - Only run the full reset when the device is not a wake source
> (device_may_wakeup()), so wake-on-touch is no longer disturbed.
> - Reword the changelog around s2idle: the SP10 has no "deep"/S3 state, the
> touch IC loses power across s2idle.
>
> .../hid/intel-thc-hid/intel-quickspi/pci-quickspi.c | 60 +++++++++++++++++--
> 1 file changed, 56 insertions(+), 4 deletions(-)
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] Input: ads7846 - consolidate coordinate filtering logic
From: Dmitry Torokhov @ 2026-06-10 22:49 UTC (permalink / raw)
To: Aaro Koskinen
Cc: Kris Bahnsen, linux-input, Mark Featherston, Marek Vasut,
linux-kernel
In-Reply-To: <aimsXWeg3wouoiB9@darkstar.musicnaut.iki.fi>
On Wed, Jun 10, 2026 at 09:26:37PM +0300, Aaro Koskinen wrote:
> Hi,
>
> On Thu, May 07, 2026 at 12:35:04PM -0700, Dmitry Torokhov wrote:
> > I think if you pull 'next' branch of my tree, then the following patches
> > seem to apply cleanly on 7.0:
> >
> > c68bc840f06c ("Input: ads7846 - restore half-duplex support")
> > 011bdf9f3a9d ("Input: ads7846 - consolidate coordinate filtering logic")
> >
> > I hope they also build ;)
> >
> > c68bc840f06c should appear in linux-next soon too.
>
> Did you forgot to push this, or was it dropped? Because I fail to see
> it any any tree...
Well, that explains why Kris could not find them... Pushed out.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4] Input: ads7846 - don't use scratch for tx_buf when clearing register
From: Dmitry Torokhov @ 2026-06-10 22:49 UTC (permalink / raw)
To: Kris Bahnsen
Cc: Marek Vasut, stable, Mark Featherston, linux-input, linux-kernel
In-Reply-To: <20260507164943.760009-1-kris@embeddedTS.com>
On Thu, May 07, 2026 at 04:49:43PM +0000, Kris Bahnsen wrote:
> The workaround for XPT2046 clears the command register, giving the
> touchscreen controller a NOP. The change incorrectly re-uses the
> req->scratch variable which is used as rx_buf for xfer[5], so by
> the time xfer[6] occurs, the contents of req->scratch may not be
> 0. It was found that the touchscreen controller can end up in
> a completely unresponsive state due to it being given a command
> the driver does not expect.
>
> Instead, rely on the spi_transfer behavior of tx_buf being NULL to
> transmit all 0 bits and use the scratch variable for the rx_buf for
> both the 1 byte command to and 2 byte response from the controller.
>
> Also relocates the scratch member of struct ser_req to force it
> into a different cache line to prevent any potential issues of
> DMA stepping on unrelated data in other struct members due to
> sharing the same cache line.
>
> This change was tested on real TSC2046 and ADS7843 controllers,
> but not the XPT2046 the workaround was originally created for.
> Confirming that the original modification to clear the command
> register does not impact either real controller.
>
> Fixes: 781a07da9bb94 ("Input: ads7846 - add dummy command register clearing cycle")
> Cc: stable@vger.kernel.org
> Co-developed-by: Mark Featherston <mark@embeddedTS.com>
> Signed-off-by: Mark Featherston <mark@embeddedTS.com>
> Signed-off-by: Kris Bahnsen <kris@embeddedTS.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/2] HID: lenovo: Add support for ThinkPad X13 Folio keyboard
From: Vishnu Sankar @ 2026-06-10 22:59 UTC (permalink / raw)
To: Jiri Kosina
Cc: bentiss, derekjohn.clark, mpearson-lenovo, vsankar, linux-input,
linux-kernel
In-Reply-To: <09prqp54-684n-r4q1-02or-1nqr1s4516on@xreary.bet>
Hi Jiri,
Thank you so much.
On Thu, Jun 11, 2026 at 12:39 AM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Wed, 10 Jun 2026, Vishnu Sankar wrote:
>
> > Hi Jiri / Benjamin,
> >
> > Gently pinging on this series.
> > Is there anything blocking acceptance or any changes needed from my side?
> > The series has been tested on physical hardware.
>
> Sorry for the delay. Now applied to hid.git#for-7.1/upstream-fixes.
>
> --
> Jiri Kosina
> SUSE Labs
>
--
Regards,
Vishnu Sankar
^ permalink raw reply
* [PATCH] Input: ipaq-micro-keys - add length check in micro_key_receive
From: Dmitry Torokhov @ 2026-06-10 23:02 UTC (permalink / raw)
To: linux-input; +Cc: Rosen Penev, linux-kernel
The driver accesses the message payload (msg[0]) without checking if
the length is greater than zero. The parent MFD driver can produce a
payload with a length of 0, in which case msg[0] would be uninitialized
or stale.
Add a check to return early if len is less than 1.
Reported-by: sashiko-bot@kernel.org
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/keyboard/ipaq-micro-keys.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/input/keyboard/ipaq-micro-keys.c b/drivers/input/keyboard/ipaq-micro-keys.c
index 3c7d6aa0fe29..ebd991de70f8 100644
--- a/drivers/input/keyboard/ipaq-micro-keys.c
+++ b/drivers/input/keyboard/ipaq-micro-keys.c
@@ -43,6 +43,9 @@ static void micro_key_receive(void *data, int len, unsigned char *msg)
struct ipaq_micro_keys *keys = data;
int key, down;
+ if (len < 1)
+ return;
+
down = 0x80 & msg[0];
key = 0x7f & msg[0];
--
2.54.0.1099.g489fc7bff1-goog
--
Dmitry
^ permalink raw reply related
* Re: [PATCH] Input: ipaq-micro-keys - add length check in micro_key_receive
From: sashiko-bot @ 2026-06-10 23:14 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <aintAvTyw4CVb5hG@google.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] `micro_key_start()` and `micro_key_stop()` acquire `keys->micro->lock` without disabling interrupts, leading to a potential deadlock with the MFD serial interrupt handler.
--
commit 33b9102060e60926d04c255f54b3c13bbafe5ca3
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Input: ipaq-micro-keys - add length check in micro_key_receive
The driver accesses the message payload (msg[0]) without checking if
the length is greater than zero. The parent MFD driver can produce a
payload with a length of 0, in which case msg[0] would be uninitialized
or stale.
Add a check to return early if len is less than 1.
Reported-by: sashiko-bot@kernel.org
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> diff --git a/drivers/input/keyboard/ipaq-micro-keys.c b/drivers/input/keyboard/ipaq-micro-keys.c
> index 695ef3c2081a4..b2068fbc82651 100644
> --- a/drivers/input/keyboard/ipaq-micro-keys.c
> +++ b/drivers/input/keyboard/ipaq-micro-keys.c
> @@ -43,6 +43,9 @@ static void micro_key_receive(void *data, int len, unsigned char *msg)
> struct ipaq_micro_keys *keys = data;
> int key, down;
>
> + if (len < 1)
> + return;
> +
> down = 0x80 & msg[0];
> key = 0x7f & msg[0];
>
[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential deadlock
involving keys->micro->lock when looking at the surrounding code.
Does the use of guard(spinlock) in micro_key_start() and micro_key_stop()
risk a deadlock with the MFD serial interrupt handler?
drivers/input/keyboard/ipaq-micro-keys.c:micro_key_start() {
guard(spinlock)(&keys->micro->lock);
...
}
drivers/input/keyboard/ipaq-micro-keys.c:micro_key_stop() {
guard(spinlock)(&keys->micro->lock);
...
}
The keys->micro->lock spinlock is also acquired within the MFD driver's hard
IRQ handler in micro_serial_isr(). Since micro_key_start() and
micro_key_stop() are called in process context without disabling local
interrupts, could an interrupt firing on the same CPU while the lock is held
cause the ISR to try acquiring the lock again and hang?
Should these functions be using guard(spinlock_irqsave) or equivalent to
prevent this?
--
Sashiko AI review · https://sashiko.dev/#/patchset/aintAvTyw4CVb5hG@google.com?part=1
^ permalink raw reply
* Re: [PATCH v3 2/4] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad
From: Dmitry Torokhov @ 2026-06-10 23:17 UTC (permalink / raw)
To: Lee Jones
Cc: Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Peter Hutterer, linux-input, linux-kernel
In-Reply-To: <20260609121353.3743782-2-lee@kernel.org>
On Tue, Jun 09, 2026 at 01:13:38PM +0100, Lee Jones wrote:
> @@ -3308,13 +3309,12 @@ static void wacom_bamboo_pad_pen_event(struct wacom_wac *wacom,
> prefix = data[0];
> data[0] = WACOM_REPORT_BPAD_PEN;
>
> - /*
> - * actually reroute the event.
> - * No need to check if wacom->shared->pen is valid, hid_input_report()
> - * will check for us.
> - */
> - hid_input_report(wacom->shared->pen, HID_INPUT_REPORT, data,
> - WACOM_PKGLEN_PENABLED, 1);
> + rcu_read_lock();
Since the driver already uses guard notation we can use
guard(rcu)();
> + pen = rcu_dereference(wacom->shared->pen);
> + if (pen)
> + hid_input_report(pen, HID_INPUT_REPORT, data,
> + WACOM_PKGLEN_PENABLED, 1);
> + rcu_read_unlock();
>
> data[0] = prefix;
> }
Otherwise:
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 1/4] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
From: Dmitry Torokhov @ 2026-06-10 23:22 UTC (permalink / raw)
To: Lee Jones
Cc: Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Peter Hutterer, linux-input, linux-kernel
In-Reply-To: <20260609121353.3743782-1-lee@kernel.org>
On Tue, Jun 09, 2026 at 01:13:37PM +0100, Lee Jones wrote:
> wacom_intuos_pad() accesses wacom->shared->touch_input locklessly
> inside the interrupt handler context. If the Touch sibling device
> is disconnected, wacom_remove_shared_data() clears 'touch_input'
> outside any lock, creating a Time-of-Check to Time-of-Use (TOCTOU)
> race condition where a preempted reader in interrupt context
> dereferences the freed pointer, leading to a Use-After-Free.
>
> Resolve this by introducing RCU protection for the touch_input
> pointer:
>
> - Annotate 'touch_input' in wacom_shared struct with __rcu
> - Wrap all lockless readers in wacom_wac.c with rcu_read_lock() and
> rcu_dereference() using a unified wacom_report_touch_mute()
> helper
> - Update writers in wacom_sys.c using rcu_assign_pointer()
> - Call synchronize_rcu() in wacom_remove_shared_data() to ensure
> all active RCU readers have finished before the input device is
> freed
>
> Also wrap wacom_set_shared_values() and touch/pen assignments in
> wacom_add_shared_data() inside the wacom_udev_list_lock to serialize
> concurrent probe assignments, and verify that 'shared->touch == hdev'
> before setting touch_input to prevent concurrent sibling probe state
> desynchronization.
>
> Finally, advertise the SW_MUTE_DEVICE capability on Touch input
> devices prior to registration in wacom_setup_touch_input_capabilities()
> to prevent invalid post-registration capability modifications.
>
> Fixes: 961794a00eab ("Input: wacom - add reporting of SW_MUTE_DEVICE events")
> Signed-off-by: Lee Jones <lee@kernel.org>
> ---
>
> v1 -> v2: Split and use RCU as per Dmitry's review
> v2 -> v3: Sashiko fixes
>
> drivers/hid/wacom_sys.c | 41 ++++++++++++++----------
> drivers/hid/wacom_wac.c | 70 ++++++++++++++++++++++-------------------
> drivers/hid/wacom_wac.h | 2 +-
> 3 files changed, 63 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 2220168bf116..7ba589826548 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -877,10 +877,16 @@ static void wacom_remove_shared_data(void *res)
> data = container_of(wacom_wac->shared, struct wacom_hdev_data,
> shared);
>
> - if (wacom_wac->shared->touch == wacom->hdev)
> - wacom_wac->shared->touch = NULL;
> - else if (wacom_wac->shared->pen == wacom->hdev)
> - wacom_wac->shared->pen = NULL;
> + scoped_guard(mutex, &wacom_udev_list_lock) {
> + if (wacom_wac->shared->touch == wacom->hdev) {
> + wacom_wac->shared->touch = NULL;
> + rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
> + } else if (wacom_wac->shared->pen == wacom->hdev) {
> + wacom_wac->shared->pen = NULL;
> + }
> + }
> +
> + synchronize_rcu();
>
> kref_put(&data->kref, wacom_release_shared_data);
> wacom_wac->shared = NULL;
> @@ -909,6 +915,11 @@ static int wacom_add_shared_data(struct hid_device *hdev)
> list_add_tail(&data->list, &wacom_udev_list);
> }
>
> + if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
> + data->shared.touch = hdev;
> + else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
> + data->shared.pen = hdev;
> +
> mutex_unlock(&wacom_udev_list_lock);
>
> wacom_wac->shared = &data->shared;
> @@ -917,11 +928,6 @@ static int wacom_add_shared_data(struct hid_device *hdev)
> if (retval)
> return retval;
>
> - if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
> - wacom_wac->shared->touch = hdev;
> - else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
> - wacom_wac->shared->pen = hdev;
> -
> return retval;
> }
>
> @@ -2345,9 +2351,15 @@ static void wacom_release_resources(struct wacom *wacom)
>
> static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
> {
> + struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
> +
> + mutex_lock(&wacom_udev_list_lock);
Why not
guard(mutex)(&wacom_udev_list_lock);
> +
> if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH) {
> - wacom_wac->shared->type = wacom_wac->features.type;
> - wacom_wac->shared->touch_input = wacom_wac->touch_input;
> + if (wacom_wac->shared->touch == wacom->hdev) {
> + wacom_wac->shared->type = wacom_wac->features.type;
> + rcu_assign_pointer(wacom_wac->shared->touch_input, wacom_wac->touch_input);
> + }
> }
>
> if (wacom_wac->has_mute_touch_switch) {
> @@ -2361,12 +2373,7 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
> wacom_wac->shared->is_touch_on = true;
> }
>
> - if (wacom_wac->shared->has_mute_touch_switch &&
> - wacom_wac->shared->touch_input) {
> - set_bit(EV_SW, wacom_wac->shared->touch_input->evbit);
> - input_set_capability(wacom_wac->shared->touch_input, EV_SW,
> - SW_MUTE_DEVICE);
> - }
> + mutex_unlock(&wacom_udev_list_lock);
> }
>
> static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index da1f0ea85625..495960227b8d 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -510,6 +510,22 @@ static void wacom_intuos_schedule_prox_event(struct wacom_wac *wacom_wac)
> }
> }
>
> +static void wacom_report_touch_mute(struct wacom_wac *wacom_wac, bool mute)
> +{
> + struct input_dev *touch_input;
> +
> + if (!wacom_wac->shared)
> + return;
Can this happen? I think callers already check this or simply
dereference.
> +
> + rcu_read_lock();
guard(rcu)();
> + touch_input = rcu_dereference(wacom_wac->shared->touch_input);
> + if (touch_input) {
> + input_report_switch(touch_input, SW_MUTE_DEVICE, mute);
> + input_sync(touch_input);
> + }
> + rcu_read_unlock();
> +}
> +
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: synaptics-rmi4 - unregister function handlers on physical driver registration failure
From: Dmitry Torokhov @ 2026-06-10 23:41 UTC (permalink / raw)
To: Haoxiang Li
Cc: git, Marge.Yang, kees, jiapeng.chong, linux-input, linux-kernel,
stable
In-Reply-To: <20260610064633.2837084-1-haoxiang_li2024@163.com>
Hi Haoxiang,
On Wed, Jun 10, 2026 at 02:46:33PM +0800, Haoxiang Li wrote:
> If rmi_register_physical_driver() fails, the current error path
> unregisters only the RMI bus. The function handlers registered
> earlier remain registered with the driver core.
>
> Add a separate error path to unregister the function handlers
> before unregistering the bus in this failure case.
>
> Fixes: d6e680837ec5 ("Input: synaptics-rmi4 - fix function name in kerneldoc")
This is not correct commit for fixes. I changed this to
2b6a321da9a2 ("Input: synaptics-rmi4 - add support for Synaptics RMI4 devices")
and applied, thank you.
Thanks.
--
Dmitry
^ permalink raw reply
* [hid:for-7.2/asus 1/7] drivers/hid/hid-asus.c:1399:14: warning: variable 'rsize_orig' is used uninitialized whenever 'if' condition is false
From: kernel test robot @ 2026-06-11 4:05 UTC (permalink / raw)
To: Joshua Leivenzon; +Cc: llvm, oe-kbuild-all, linux-input, Jiri Kosina
tree: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-7.2/asus
head: 9bde6277292c8233fb24fc6e51323027b49d1cde
commit: 92f9f783f013a27a175089950b7b22c3d5a48249 [1/7] HID: asus: Fix up Zenbook Duo report descriptors
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20260611/202606110526.QfgiXQTQ-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project f43d6834093b19baf79beda8c0337ab020ac5f17)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260611/202606110526.QfgiXQTQ-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202606110526.QfgiXQTQ-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/hid/hid-asus.c:1399:14: warning: variable 'rsize_orig' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
1399 | } else if (drvdata->quirks & QUIRK_ZENBOOK_DUO_KEYBOARD) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/hid/hid-asus.c:1410:17: note: uninitialized use occurs here
1410 | if (*rsize == rsize_orig &&
| ^~~~~~~~~~
drivers/hid/hid-asus.c:1399:10: note: remove the 'if' if its condition is always true
1399 | } else if (drvdata->quirks & QUIRK_ZENBOOK_DUO_KEYBOARD) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/hid/hid-asus.c:1390:17: note: initialize the variable 'rsize_orig' to silence this warning
1390 | int rsize_orig;
| ^
| = 0
1 warning generated.
vim +1399 drivers/hid/hid-asus.c
1370
1371 static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
1372 unsigned int *rsize)
1373 {
1374 struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
1375
1376 if (drvdata->quirks & QUIRK_FIX_NOTEBOOK_REPORT &&
1377 *rsize >= 56 && rdesc[54] == 0x25 && rdesc[55] == 0x65) {
1378 hid_info(hdev, "Fixing up Asus notebook report descriptor\n");
1379 rdesc[55] = 0xdd;
1380 }
1381 /* For the T100TA/T200TA keyboard dock */
1382 if (drvdata->quirks & QUIRK_T100_KEYBOARD &&
1383 (*rsize == 76 || *rsize == 101) &&
1384 rdesc[73] == 0x81 && rdesc[74] == 0x01) {
1385 hid_info(hdev, "Fixing up Asus T100 keyb report descriptor\n");
1386 rdesc[74] &= ~HID_MAIN_ITEM_CONSTANT;
1387 }
1388 /* For the T100CHI/T90CHI keyboard dock and Zenbook Duo 2024+ keyboards */
1389 if (drvdata->quirks & (QUIRK_T100CHI | QUIRK_T90CHI | QUIRK_ZENBOOK_DUO_KEYBOARD)) {
1390 int rsize_orig;
1391 int offs;
1392
1393 if (drvdata->quirks & QUIRK_T100CHI) {
1394 rsize_orig = 403;
1395 offs = 388;
1396 } else if (drvdata->quirks & QUIRK_T90CHI) {
1397 rsize_orig = 306;
1398 offs = 291;
> 1399 } else if (drvdata->quirks & QUIRK_ZENBOOK_DUO_KEYBOARD) {
1400 rsize_orig = 257;
1401 offs = 176;
1402 }
1403
1404 /*
1405 * Change Usage (76h) to Usage Minimum (00h), Usage Maximum
1406 * (FFh) and clear the flags in the Input() byte.
1407 * Note the descriptor has a bogus 0 byte at the end so we
1408 * only need 1 extra byte.
1409 */
1410 if (*rsize == rsize_orig &&
1411 rdesc[offs] == 0x09 && rdesc[offs + 1] == 0x76) {
1412 __u8 *new_rdesc;
1413
1414 new_rdesc = devm_kzalloc(&hdev->dev, rsize_orig + 1,
1415 GFP_KERNEL);
1416 if (!new_rdesc)
1417 return rdesc;
1418
1419 hid_info(hdev, "Fixing up %s keyb report descriptor\n",
1420 drvdata->quirks & QUIRK_T100CHI ?
1421 "T100CHI" : drvdata->quirks & QUIRK_T90CHI ?
1422 "T90CHI" : "ZENBOOK DUO");
1423
1424 memcpy(new_rdesc, rdesc, rsize_orig);
1425 *rsize = rsize_orig + 1;
1426 rdesc = new_rdesc;
1427
1428 memmove(rdesc + offs + 4, rdesc + offs + 2, 12);
1429 rdesc[offs] = 0x19;
1430 rdesc[offs + 1] = 0x00;
1431 rdesc[offs + 2] = 0x29;
1432 rdesc[offs + 3] = 0xff;
1433 rdesc[offs + 14] = 0x00;
1434 }
1435 }
1436
1437 if (drvdata->quirks & QUIRK_G752_KEYBOARD &&
1438 *rsize == 75 && rdesc[61] == 0x15 && rdesc[62] == 0x00) {
1439 /* report is missing usage minimum and maximum */
1440 __u8 *new_rdesc;
1441 size_t new_size = *rsize + sizeof(asus_g752_fixed_rdesc);
1442
1443 new_rdesc = devm_kzalloc(&hdev->dev, new_size, GFP_KERNEL);
1444 if (new_rdesc == NULL)
1445 return rdesc;
1446
1447 hid_info(hdev, "Fixing up Asus G752 keyb report descriptor\n");
1448 /* copy the valid part */
1449 memcpy(new_rdesc, rdesc, 61);
1450 /* insert missing part */
1451 memcpy(new_rdesc + 61, asus_g752_fixed_rdesc, sizeof(asus_g752_fixed_rdesc));
1452 /* copy remaining data */
1453 memcpy(new_rdesc + 61 + sizeof(asus_g752_fixed_rdesc), rdesc + 61, *rsize - 61);
1454
1455 *rsize = new_size;
1456 rdesc = new_rdesc;
1457 }
1458
1459 if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD &&
1460 *rsize == 331 && rdesc[190] == 0x85 && rdesc[191] == 0x5a &&
1461 rdesc[204] == 0x95 && rdesc[205] == 0x05) {
1462 hid_info(hdev, "Fixing up Asus N-KEY keyb report descriptor\n");
1463 rdesc[205] = 0x01;
1464 }
1465
1466 /* match many more n-key devices */
1467 if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD && *rsize > 15) {
1468 for (int i = 0; i < *rsize - 15; i++) {
1469 /* offset to the count from 0x5a report part always 14 */
1470 if (rdesc[i] == 0x85 && rdesc[i + 1] == 0x5a &&
1471 rdesc[i + 14] == 0x95 && rdesc[i + 15] == 0x05) {
1472 hid_info(hdev, "Fixing up Asus N-Key report descriptor\n");
1473 rdesc[i + 15] = 0x01;
1474 break;
1475 }
1476 }
1477 }
1478
1479 return rdesc;
1480 }
1481
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v8 2/7] mfd: Add driver for ASUS Transformer embedded controller
From: Lee Jones @ 2026-06-11 11:17 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Pavel Machek, Sebastian Reichel, Ion Agorria,
Michał Mirosław, devicetree, linux-kernel, linux-input,
linux-leds, linux-pm
In-Reply-To: <20260528053203.9339-3-clamor95@gmail.com>
On Thu, 28 May 2026, Svyatoslav Ryhel wrote:
> From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
>
> Support Nuvoton NPCE795-based ECs as used in Asus Transformer TF201,
> TF300T, TF300TG, TF300TL and TF700T pad and dock, as well as TF101 dock
> and TF600T, P1801-T and TF701T pad. This is a glue driver handling
> detection and common operations for EC's functions.
>
> Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
> drivers/mfd/Kconfig | 16 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/asus-transformer-ec.c | 542 ++++++++++++++++++++++++
> include/linux/mfd/asus-transformer-ec.h | 92 ++++
> 4 files changed, 651 insertions(+)
> create mode 100644 drivers/mfd/asus-transformer-ec.c
> create mode 100644 include/linux/mfd/asus-transformer-ec.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 7192c9d1d268..e1c32505b97a 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -137,6 +137,22 @@ config MFD_AAT2870_CORE
> additional drivers must be enabled in order to use the
> functionality of the device.
>
> +config MFD_ASUS_TRANSFORMER_EC
> + tristate "ASUS Transformer's embedded controller"
> + select MFD_CORE
> + depends on I2C && OF
> + help
> + Select this to enable support for the Embedded Controller (EC)
> + found in Tegra based ASUS Transformer series tablets and mobile
> + docks.
> +
> + This driver handles the core I2C communication with the EC and
> + provides support for its sub-devices, including battery management,
> + charger detection, LEDs and keyboard dock functions support.
> +
> + This driver can also be built as a module. If so, the module
> + will be called asus-transformer-ec.
> +
> config MFD_AT91_USART
> tristate "AT91 USART Driver"
> select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index e75e8045c28a..fd80088d8a9a 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_MFD_88PM805) += 88pm805.o 88pm80x.o
> obj-$(CONFIG_MFD_88PM886_PMIC) += 88pm886.o
> obj-$(CONFIG_MFD_ACT8945A) += act8945a.o
> obj-$(CONFIG_MFD_SM501) += sm501.o
> +obj-$(CONFIG_MFD_ASUS_TRANSFORMER_EC) += asus-transformer-ec.o
> obj-$(CONFIG_ARCH_BCM2835) += bcm2835-pm.o
> obj-$(CONFIG_MFD_BCM590XX) += bcm590xx.o
> obj-$(CONFIG_MFD_BD9571MWV) += bd9571mwv.o
> diff --git a/drivers/mfd/asus-transformer-ec.c b/drivers/mfd/asus-transformer-ec.c
> new file mode 100644
> index 000000000000..1f5900d0fdc9
> --- /dev/null
> +++ b/drivers/mfd/asus-transformer-ec.c
> @@ -0,0 +1,542 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <linux/array_size.h>
> +#include <linux/debugfs.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/asus-transformer-ec.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/property.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
> +
> +#define ASUSEC_RSP_BUFFER_SIZE (ASUSEC_ENTRIES / ASUSEC_ENTRY_SIZE)
> +
> +#define ASUSEC_RESET 0
> +#define ASUSEC_RETRY_MAX 3
> +#define ASUSEC_ACCESS_TIMEOUT 300
> +
> +enum asusec_variant {
> + ASUSEC_SL101_DOCK = 1,
> + ASUSEC_TF101_DOCK,
> + ASUSEC_TF201_PAD,
> + ASUSEC_TF600T_PAD,
> + ASUSEC_MAX
> +};
> +
> +enum asusec_mode {
> + ASUSEC_MODE_NONE,
> + ASUSEC_MODE_NORMAL,
> + ASUSEC_MODE_FACTORY,
> + ASUSEC_MODE_MAX
> +};
> +
> +/**
> + * struct asus_ec_chip_info
> + *
> + * @name: prefix associated with the EC
> + * @variant: id of programming model of EC
> + * @mode: state of Factory Mode bit in EC control register
> + */
> +struct asus_ec_chip_info {
> + const char *name;
> + enum asusec_variant variant;
> + enum asusec_mode fmode;
> +};
> +
> +/**
> + * struct asus_ec_data
> + *
> + * @ec: public part shared with all cells (must be first)
> + * @ecreq_lock: prevents simultaneous access to EC
> + * @ecreq_gpio: EC request GPIO
> + * @client: pointer to EC's i2c_client
> + * @info: pointer to EC's version description
> + * @ec_buf: buffer for EC read
> + * @logging_disabled: flag disabling logging on reset events
> + */
> +struct asus_ec_data {
> + struct asusec_core ec;
> + struct mutex ecreq_lock;
> + struct gpio_desc *ecreq_gpio;
> + struct i2c_client *client;
> + const struct asus_ec_chip_info *info;
> + u8 ec_buf[ASUSEC_ENTRY_BUFSIZE];
> + bool logging_disabled;
> +};
> +
> +/**
> + * struct dockram_ec_data
> + *
> + * @ctl_lock: prevent simultaneous access to Dockram
> + * @ctl_buf: buffer for Dockram read
> + */
> +struct dockram_ec_data {
> + struct mutex ctl_lock;
> + u8 ctl_buf[ASUSEC_ENTRY_BUFSIZE];
> +};
> +
> +/**
> + * asus_dockram_access_ctl - Read from or write to the DockRAM control register.
> + * @client: Handle to the DockRAM device.
> + * @out: Pointer to a variable where the register value will be stored.
> + * @mask: Bitmask of bits to be cleared.
> + * @xor: Bitmask of bits to be set (via XOR).
> + *
> + * This performs a control register read if @out is provided and both @mask
> + * and @xor are zero. Otherwise, it performs a control register update if
> + * @mask and @xor are provided.
> + *
> + * Returns a negative errno code else zero on success.
> + */
> +int asus_dockram_access_ctl(struct i2c_client *client, u64 *out, u64 mask,
> + u64 xor)
> +{
> + struct dockram_ec_data *ddata = i2c_get_clientdata(client);
> + u8 *buf = ddata->ctl_buf;
> + u64 val;
> + int ret = 0;
> +
> + guard(mutex)(&ddata->ctl_lock);
> +
> + memset(buf, 0, ASUSEC_ENTRY_BUFSIZE);
> + ret = i2c_smbus_read_i2c_block_data(client, ASUSEC_DOCKRAM_CONTROL,
> + ASUSEC_ENTRY_SIZE, buf);
> + if (ret < ASUSEC_ENTRY_SIZE) {
> + dev_err(&client->dev, "failed to access control buffer: %d\n",
> + ret);
> + return ret;
Should we return a negative error code here if the read is shorter than
expected, rather than propagating the positive byte count?
> + }
> +
> + if (buf[0] != ASUSEC_CTL_SIZE) {
> + dev_err(&client->dev, "buffer size exceeds %d: %d\n",
> + ASUSEC_CTL_SIZE, buf[0]);
> + return -EPROTO;
> + }
> +
> + val = get_unaligned_le64(buf + 1);
> +
> + if (out)
> + *out = val;
> +
> + if (mask || xor) {
> + put_unaligned_le64((val & ~mask) ^ xor, buf + 1);
> + ret = i2c_smbus_write_i2c_block_data(client,
> + ASUSEC_DOCKRAM_CONTROL,
> + ASUSEC_ENTRY_SIZE, buf);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(asus_dockram_access_ctl);
> +
> +static int asus_ec_signal_request(struct asus_ec_data *ddata)
> +{
> + guard(mutex)(&ddata->ecreq_lock);
> +
> + gpiod_set_value_cansleep(ddata->ecreq_gpio, 1);
> + msleep(50);
> +
> + gpiod_set_value_cansleep(ddata->ecreq_gpio, 0);
> + msleep(200);
Do these numbers come from the datasheet or were they arbitrarily chosen?
> +
> + return 0;
> +}
> +
> +static void asus_ec_clear_buffer(struct asus_ec_data *ddata)
> +{
> + int ret, retry = ASUSEC_RSP_BUFFER_SIZE;
> +
> + /*
> + * Read the buffer till we get valid data by checking ASUSEC_OBF_MASK
> + * of the status byte or till we reach end of the 256 byte buffer.
> + */
> + while (retry--) {
> + ret = i2c_smbus_read_i2c_block_data(ddata->client, ASUSEC_READ_BUF,
> + ASUSEC_ENTRY_SIZE,
> + ddata->ec_buf);
> + if (ret < ASUSEC_ENTRY_SIZE)
> + continue;
> +
> + if (ddata->ec_buf[ASUSEC_IRQ_STATUS] & ASUSEC_OBF_MASK)
> + continue;
> +
> + break;
> + }
> +}
> +
> +static int asus_ec_log_info(struct asus_ec_data *ddata, unsigned int reg,
> + const char *name, const char **out)
> +{
> + struct device *dev = &ddata->client->dev;
> + u8 buf[ASUSEC_ENTRY_BUFSIZE];
> + int ret;
> +
> + memset(buf, 0, ASUSEC_ENTRY_BUFSIZE);
> + ret = i2c_smbus_read_i2c_block_data(ddata->ec.dockram, reg,
> + ASUSEC_ENTRY_SIZE, buf);
> + if (ret < ASUSEC_ENTRY_SIZE)
> + return ret;
Same here. These should be negative.
> +
> + if (buf[0] > ASUSEC_ENTRY_SIZE) {
> + dev_err(dev, "bad data len; buffer: %*ph; ret: %d\n",
> + ASUSEC_ENTRY_BUFSIZE, buf, ret);
> + return -EPROTO;
> + }
> +
> + if (!ddata->logging_disabled) {
> + dev_info(dev, "%-14s: %.*s\n", name, buf[0], buf + 1);
> +
> + if (out) {
> + *out = devm_kasprintf(dev, GFP_KERNEL, "%.*s",
> + buf[0], buf + 1);
> + if (!*out)
> + return -ENOMEM;
> + }
> + }
FWIW, I hate this! What does it give you now that development is done?
> + return 0;
> +}
> +
> +static int asus_ec_reset(struct asus_ec_data *ddata)
> +{
> + int retry, ret;
> +
> + guard(mutex)(&ddata->ecreq_lock);
> +
> + for (retry = 0; retry < ASUSEC_RETRY_MAX; retry++) {
for (int return = ... is generally preferred for throwaway variables.
> + ret = i2c_smbus_write_word_data(ddata->client, ASUSEC_WRITE_BUF,
> + ASUSEC_RESET);
> + if (!ret)
> + return 0;
> +
> + msleep(ASUSEC_ACCESS_TIMEOUT);
I like that this is defined, can we do that with the others please?
> + }
> +
> + return ret;
> +}
> +
> +static int asus_ec_susb_on_status(struct asus_ec_data *ddata)
> +{
> + u64 flag;
> + int ret;
> +
> + ret = asus_dockram_access_ctl(ddata->ec.dockram, &flag, 0, 0);
> + if (ret)
> + return ret;
> +
> + flag &= ASUSEC_CTL_SUSB_MODE;
> + dev_info(&ddata->client->dev, "EC FW behaviour: %s\n",
> + flag ? "susb on when receive ec_req" :
> + "susb on when system wakeup");
> +
> + return 0;
> +}
> +
> +static int asus_ec_set_factory_mode(struct asus_ec_data *ddata,
> + enum asusec_mode fmode)
> +{
> + dev_info(&ddata->client->dev, "Entering %s mode.\n",
> + fmode == ASUSEC_MODE_FACTORY ? "factory" : "normal");
> +
> + return asus_dockram_access_ctl(ddata->ec.dockram, NULL,
> + ASUSEC_CTL_FACTORY_MODE,
> + fmode == ASUSEC_MODE_FACTORY ?
> + ASUSEC_CTL_FACTORY_MODE : 0);
Why not create make:
ASUSEC_MODE_FACTORY == ASUSEC_CTL_FACTORY_MODE
What happens to NORMAL?
> +}
> +
> +static int asus_ec_detect(struct asus_ec_data *ddata)
> +{
> + int ret;
> +
> + ret = asus_ec_reset(ddata);
> + if (ret)
> + goto err_exit;
> +
> + asus_ec_clear_buffer(ddata);
> +
> + ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_MODEL, "Model",
> + &ddata->ec.model);
You can use 100-chars and make the code look beautiful! :)
> + if (ret)
> + goto err_exit;
> +
> + ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_FW, "FW version",
> + NULL);
> + if (ret)
> + goto err_exit;
> +
> + ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_CFGFMT, "Config format",
> + NULL);
> + if (ret)
> + goto err_exit;
> +
> + ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_HW, "HW version",
> + NULL);
> + if (ret)
> + goto err_exit;
> +
> + /* Disable logging on next EC request */
Why, but why?
> + ddata->logging_disabled = true;
> +
> + /* Check and inform about EC firmware behavior */
> + ret = asus_ec_susb_on_status(ddata);
> + if (ret)
> + goto err_exit;
> +
> + ddata->ec.name = ddata->info->name;
> +
> + /* Some EC require factory mode to be set normal on each request */
> + if (ddata->info->fmode)
> + ret = asus_ec_set_factory_mode(ddata, ddata->info->fmode);
> +
> +err_exit:
> + if (ret)
> + dev_err(&ddata->client->dev, "failed to access EC: %d\n", ret);
> +
> + return ret;
> +}
> +
> +static void asus_ec_handle_smi(struct asus_ec_data *ddata, unsigned int code)
> +{
> + switch (code) {
> + case ASUSEC_SMI_HANDSHAKE:
> + case ASUSEC_SMI_RESET:
> + asus_ec_detect(ddata);
> + break;
> + }
> +}
> +
> +static irqreturn_t asus_ec_interrupt(int irq, void *dev_id)
> +{
> + struct asus_ec_data *ddata = dev_id;
> + unsigned long notify_action;
> + int ret;
> +
> + ret = i2c_smbus_read_i2c_block_data(ddata->client, ASUSEC_READ_BUF,
> + ASUSEC_ENTRY_SIZE, ddata->ec_buf);
> + if (ret < ASUSEC_ENTRY_SIZE ||
> + !(ddata->ec_buf[ASUSEC_IRQ_STATUS] & ASUSEC_OBF_MASK))
Unwrap for readability.
Also, I think a comment would be helpful.
> + return IRQ_NONE;
> +
> + notify_action = ddata->ec_buf[ASUSEC_IRQ_STATUS];
> + if (notify_action & ASUSEC_SMI_MASK) {
> + unsigned int code = ddata->ec_buf[ASUSEC_SMI_CODE];
> +
> + asus_ec_handle_smi(ddata, code);
> +
> + notify_action |= code << 8;
> + }
> +
> + blocking_notifier_call_chain(&ddata->ec.notify_list,
> + notify_action, ddata->ec_buf);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void asus_ec_release_dockram_dev(void *client)
> +{
> + i2c_unregister_device(client);
> +}
> +
> +static struct i2c_client *devm_asus_dockram_get(struct device *dev)
> +{
> + struct i2c_client *parent = to_i2c_client(dev);
> + struct i2c_client *dockram;
> + struct dockram_ec_data *ddata;
> + int ret;
> +
> + dockram = i2c_new_ancillary_device(parent, "dockram",
> + parent->addr + 2);
Could we define a macro for the address offset '2' here to avoid using a magic
number?
> + if (IS_ERR(dockram))
> + return dockram;
> +
> + ret = devm_add_action_or_reset(dev, asus_ec_release_dockram_dev,
> + dockram);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + ddata = devm_kzalloc(&dockram->dev, sizeof(*ddata), GFP_KERNEL);
> + if (!ddata)
> + return ERR_PTR(-ENOMEM);
> +
> + i2c_set_clientdata(dockram, ddata);
> + mutex_init(&ddata->ctl_lock);
> +
> + return dockram;
> +}
> +
> +static const struct mfd_cell asus_ec_sl101_dock_mfd_devices[] = {
> + MFD_CELL_NAME("asus-transformer-ec-kbc"),
> +};
> +
> +static const struct mfd_cell asus_ec_tf101_dock_mfd_devices[] = {
> + MFD_CELL_BASIC("asus-transformer-ec-battery", NULL, NULL, 0, 1),
> + MFD_CELL_BASIC("asus-transformer-ec-charger", NULL, NULL, 0, 1),
> + MFD_CELL_BASIC("asus-transformer-ec-led", NULL, NULL, 0, 1),
> + MFD_CELL_NAME("asus-transformer-ec-keys"),
> + MFD_CELL_NAME("asus-transformer-ec-kbc"),
> +};
> +
> +static const struct mfd_cell asus_ec_tf201_pad_mfd_devices[] = {
> + MFD_CELL_NAME("asus-transformer-ec-battery"),
> + MFD_CELL_NAME("asus-transformer-ec-led"),
> +};
> +
> +static const struct mfd_cell asus_ec_tf600t_pad_mfd_devices[] = {
> + MFD_CELL_NAME("asus-transformer-ec-battery"),
> + MFD_CELL_NAME("asus-transformer-ec-charger"),
> + MFD_CELL_NAME("asus-transformer-ec-led"),
> +};
> +
> +static int asus_ec_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct asus_ec_data *ddata;
> + const struct mfd_cell *cells;
> + unsigned int num_cells;
> + unsigned long irqflags;
> + int ret;
> +
> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_I2C_BLOCK))
> + return dev_err_probe(dev, -ENXIO,
> + "I2C bus is missing required SMBus block mode support\n");
> +
> + ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> + if (!ddata)
> + return -ENOMEM;
> +
> + ddata->info = device_get_match_data(dev);
> + if (!ddata->info)
> + return -ENODEV;
> +
> + switch (ddata->info->variant) {
> + case ASUSEC_SL101_DOCK:
> + cells = asus_ec_sl101_dock_mfd_devices;
> + num_cells = ARRAY_SIZE(asus_ec_sl101_dock_mfd_devices);
> + break;
> + case ASUSEC_TF101_DOCK:
> + cells = asus_ec_tf101_dock_mfd_devices;
> + num_cells = ARRAY_SIZE(asus_ec_tf101_dock_mfd_devices);
> + break;
> + case ASUSEC_TF201_PAD:
> + cells = asus_ec_tf201_pad_mfd_devices;
> + num_cells = ARRAY_SIZE(asus_ec_tf201_pad_mfd_devices);
> + break;
> + case ASUSEC_TF600T_PAD:
> + cells = asus_ec_tf600t_pad_mfd_devices;
> + num_cells = ARRAY_SIZE(asus_ec_tf600t_pad_mfd_devices);
> + break;
> + default:
> + return dev_err_probe(dev, -EINVAL,
> + "unknown device variant %d\n",
> + ddata->info->variant);
> + }
> +
> + i2c_set_clientdata(client, ddata);
> + ddata->client = client;
> +
> + ddata->ec.dockram = devm_asus_dockram_get(dev);
> + if (IS_ERR(ddata->ec.dockram))
> + return dev_err_probe(dev, PTR_ERR(ddata->ec.dockram),
> + "failed to get dockram\n");
> +
> + ddata->ecreq_gpio = devm_gpiod_get(dev, "request", GPIOD_OUT_LOW);
> + if (IS_ERR(ddata->ecreq_gpio))
> + return dev_err_probe(dev, PTR_ERR(ddata->ecreq_gpio),
> + "failed to get EC request GPIO\n");
> +
> + BLOCKING_INIT_NOTIFIER_HEAD(&ddata->ec.notify_list);
> + mutex_init(&ddata->ecreq_lock);
> +
> + asus_ec_signal_request(ddata);
> +
> + ret = asus_ec_detect(ddata);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to detect EC version\n");
> +
> + /*
> + * Systems using device tree should set up interrupt via DTS,
> + * the rest will use the default low interrupt.
> + */
> + irqflags = dev->of_node ? 0 : IRQF_TRIGGER_LOW;
> +
> + ret = devm_request_threaded_irq(dev, client->irq, NULL,
> + &asus_ec_interrupt,
> + IRQF_ONESHOT | irqflags,
> + client->name, ddata);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to register IRQ\n");
> +
> + /* Parent I2C controller uses DMA, ASUS EC and child devices do not */
> + client->dev.coherent_dma_mask = 0;
> + client->dev.dma_mask = &client->dev.coherent_dma_mask;
> +
> + return devm_mfd_add_devices(dev, 0, cells, num_cells, NULL, 0, NULL);
> +}
> +
> +static const struct asus_ec_chip_info asus_ec_sl101_dock_data = {
> + .name = "dock",
> + .variant = ASUSEC_SL101_DOCK,
> + .fmode = ASUSEC_MODE_NONE,
> +};
> +
> +static const struct asus_ec_chip_info asus_ec_tf101_dock_data = {
> + .name = "dock",
> + .variant = ASUSEC_TF101_DOCK,
> + .fmode = ASUSEC_MODE_NONE,
> +};
> +
> +static const struct asus_ec_chip_info asus_ec_tf201_pad_data = {
> + .name = "pad",
> + .variant = ASUSEC_TF201_PAD,
> + .fmode = ASUSEC_MODE_NORMAL,
> +};
> +
> +static const struct asus_ec_chip_info asus_ec_tf600t_pad_data = {
> + .name = "pad",
> + .variant = ASUSEC_TF600T_PAD,
> + .fmode = ASUSEC_MODE_NORMAL,
> +};
Any reason not to just pass the identifier (variant) and add the name
and fmode attribues to the switch() above?
> +
> +static const struct of_device_id asus_ec_match[] = {
> + {
> + .compatible = "asus,sl101-ec-dock",
> + .data = &asus_ec_sl101_dock_data
> + }, {
> + .compatible = "asus,tf101-ec-dock",
> + .data = &asus_ec_tf101_dock_data
> + }, {
> + .compatible = "asus,tf201-ec-pad",
> + .data = &asus_ec_tf201_pad_data
> + }, {
> + .compatible = "asus,tf600t-ec-pad",
> + .data = &asus_ec_tf600t_pad_data
> + },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, asus_ec_match);
> +
> +static struct i2c_driver asus_ec_driver = {
> + .driver = {
> + .name = "asus-transformer-ec",
> + .of_match_table = asus_ec_match,
> + },
> + .probe = asus_ec_probe,
> +};
> +module_i2c_driver(asus_ec_driver);
> +
> +MODULE_AUTHOR("Michał Mirosław <mirq-linux@rere.qmqm.pl>");
> +MODULE_AUTHOR("Svyatoslav Ryhel <clamor95@gmail.com>");
> +MODULE_DESCRIPTION("ASUS Transformer's EC driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/asus-transformer-ec.h b/include/linux/mfd/asus-transformer-ec.h
> new file mode 100644
> index 000000000000..f085eea2193e
> --- /dev/null
> +++ b/include/linux/mfd/asus-transformer-ec.h
> @@ -0,0 +1,92 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef __MFD_ASUS_TRANSFORMER_EC_H
> +#define __MFD_ASUS_TRANSFORMER_EC_H
> +
> +#include <linux/notifier.h>
> +#include <linux/platform_device.h>
> +
> +struct i2c_client;
> +
> +/**
> + * struct asusec_core - public part shared with all cells
> + *
> + * @model: firmware version running on the EC
> + * @name: prefix associated with the EC
> + * @dockram: pointer to Dockram's i2c_client
> + * @notify_list: notify list used by cells
> + */
> +struct asusec_core {
> + const char *model;
> + const char *name;
> + struct i2c_client *dockram;
> + struct blocking_notifier_head notify_list;
> +};
> +
> +#define ASUSEC_ENTRIES 0x100
> +#define ASUSEC_ENTRY_SIZE 32
> +#define ASUSEC_ENTRY_BUFSIZE (ASUSEC_ENTRY_SIZE + 1)
> +
> +/* interrupt sources */
> +#define ASUSEC_IRQ_STATUS 1
> +#define ASUSEC_OBF_MASK BIT(0)
> +#define ASUSEC_KEY_MASK BIT(2)
> +#define ASUSEC_KBC_MASK BIT(3)
> +#define ASUSEC_AUX_MASK BIT(5)
> +#define ASUSEC_SCI_MASK BIT(6)
> +#define ASUSEC_SMI_MASK BIT(7)
> +
> +/* SMI notification codes */
> +#define ASUSEC_SMI_CODE 2
> +#define ASUSEC_SMI_POWER_NOTIFY 0x31 /* USB cable plug event */
> +#define ASUSEC_SMI_HANDSHAKE 0x50 /* response to ec_req edge */
> +#define ASUSEC_SMI_WAKE 0x53
> +#define ASUSEC_SMI_RESET 0x5f
> +#define ASUSEC_SMI_ADAPTER_EVENT 0x60 /* charger to dock plug event */
> +#define ASUSEC_SMI_BACKLIGHT_ON 0x63
> +#define ASUSEC_SMI_AUDIO_DOCK_IN 0x70
> +
> +#define ASUSEC_SMI_ACTION(code) (ASUSEC_SMI_MASK | ASUSEC_OBF_MASK | \
> + (ASUSEC_SMI_##code << 8))
> +
> +/* control register [0x0a] layout */
> +#define ASUSEC_CTL_SIZE 8
> +
> +/*
> + * EC reports power from 40-pin connector in the LSB of the control
> + * register. The following values have been observed (xor 0x02):
> + *
> + * PAD-ec no-plug 0x40 / PAD-ec DOCK 0x20 / DOCK-ec no-plug 0x40
> + * PAD-ec AC 0x25 / PAD-ec DOCK+AC 0x24 / DOCK-ec AC 0x25
> + * PAD-ec USB 0x45 / PAD-ec DOCK+USB 0x24 / DOCK-ec USB 0x41
> + */
> +
> +#define ASUSEC_CTL_DIRECT_POWER_SOURCE BIT_ULL(0)
> +#define ASUSEC_STAT_CHARGING BIT_ULL(2)
> +#define ASUSEC_CTL_FULL_POWER_SOURCE BIT_ULL(5)
> +#define ASUSEC_CTL_SUSB_MODE BIT_ULL(9)
> +#define ASUSEC_CMD_SUSPEND_S3 BIT_ULL(33)
> +#define ASUSEC_CTL_TEST_DISCHARGE BIT_ULL(35)
> +#define ASUSEC_CMD_SUSPEND_INHIBIT BIT_ULL(37)
> +#define ASUSEC_CTL_FACTORY_MODE BIT_ULL(38)
> +#define ASUSEC_CTL_KEEP_AWAKE BIT_ULL(39)
> +#define ASUSEC_CTL_USB_CHARGE BIT_ULL(40)
> +#define ASUSEC_CTL_LED_BLINK BIT_ULL(40)
> +#define ASUSEC_CTL_LED_AMBER BIT_ULL(41)
> +#define ASUSEC_CTL_LED_GREEN BIT_ULL(42)
> +#define ASUSEC_CMD_SWITCH_HDMI BIT_ULL(56)
> +#define ASUSEC_CMD_WIN_SHUTDOWN BIT_ULL(62)
> +
> +#define ASUSEC_DOCKRAM_INFO_MODEL 0x01
> +#define ASUSEC_DOCKRAM_INFO_FW 0x02
> +#define ASUSEC_DOCKRAM_INFO_CFGFMT 0x03
> +#define ASUSEC_DOCKRAM_INFO_HW 0x04
> +#define ASUSEC_DOCKRAM_CONTROL 0x0a
> +#define ASUSEC_DOCKRAM_BATT_CTL 0x14
> +
> +#define ASUSEC_WRITE_BUF 0x64
> +#define ASUSEC_READ_BUF 0x6a
> +
> +int asus_dockram_access_ctl(struct i2c_client *client,
> + u64 *out, u64 mask, u64 xor);
> +
> +#endif /* __MFD_ASUS_TRANSFORMER_EC_H */
> --
> 2.51.0
>
--
Lee Jones
^ permalink raw reply
* Re: [PATCH v8 5/7] leds: Add driver for ASUS Transformer LEDs
From: Lee Jones @ 2026-06-11 11:30 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Pavel Machek, Sebastian Reichel, Ion Agorria,
Michał Mirosław, devicetree, linux-kernel, linux-input,
linux-leds, linux-pm
In-Reply-To: <20260528053203.9339-6-clamor95@gmail.com>
On Thu, 28 May 2026, Svyatoslav Ryhel wrote:
> From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
>
> ASUS Transformer tablets have a green and an amber LED on both the Pad
> and the Dock. If both LEDs are enabled simultaneously, the emitted light
> will be yellow.
>
> Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
> drivers/leds/Kconfig | 11 +++
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-asus-transformer-ec.c | 125 ++++++++++++++++++++++++
> 3 files changed, 137 insertions(+)
> create mode 100644 drivers/leds/leds-asus-transformer-ec.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index f4a0a3c8c870..f637d23400a8 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -120,6 +120,17 @@ config LEDS_OSRAM_AMS_AS3668
> To compile this driver as a module, choose M here: the module
> will be called leds-as3668.
>
> +config LEDS_ASUS_TRANSFORMER_EC
> + tristate "LED Support for Asus Transformer charging LED"
> + depends on LEDS_CLASS
> + depends on MFD_ASUS_TRANSFORMER_EC
> + help
> + This option enables support for charging indicator on
> + Asus Transformer's Pad and it's Dock.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called leds-asus-transformer-ec.
> +
> config LEDS_AW200XX
> tristate "LED support for Awinic AW20036/AW20054/AW20072/AW20108"
> depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 8fdb45d5b439..d5395c3f1124 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_LEDS_AN30259A) += leds-an30259a.o
> obj-$(CONFIG_LEDS_APU) += leds-apu.o
> obj-$(CONFIG_LEDS_ARIEL) += leds-ariel.o
> obj-$(CONFIG_LEDS_AS3668) += leds-as3668.o
> +obj-$(CONFIG_LEDS_ASUS_TRANSFORMER_EC) += leds-asus-transformer-ec.o
> obj-$(CONFIG_LEDS_AW200XX) += leds-aw200xx.o
> obj-$(CONFIG_LEDS_AW2013) += leds-aw2013.o
> obj-$(CONFIG_LEDS_BCM6328) += leds-bcm6328.o
> diff --git a/drivers/leds/leds-asus-transformer-ec.c b/drivers/leds/leds-asus-transformer-ec.c
> new file mode 100644
> index 000000000000..09503e76331c
> --- /dev/null
> +++ b/drivers/leds/leds-asus-transformer-ec.c
> @@ -0,0 +1,125 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/err.h>
> +#include <linux/leds.h>
> +#include <linux/mfd/asus-transformer-ec.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +enum {
> + ASUSEC_LED_AMBER,
> + ASUSEC_LED_GREEN,
> + ASUSEC_LED_MAX
> +};
> +
> +struct asus_ec_led_config {
> + const char *name;
> + unsigned int color;
> + unsigned long long ctrl_bit;
Should we use 'u64' here instead of 'unsigned long long' to align with standard
kernel integer types?
> +};
> +
> +struct asus_ec_led {
> + struct asus_ec_leds_data *ddata;
> + struct led_classdev cdev;
> + unsigned long long ctrl_bit;
Should we use 'u64' here as well to keep it consistent?
> +};
> +
> +struct asus_ec_leds_data {
> + const struct asusec_core *ec;
> + struct asus_ec_led leds[ASUSEC_LED_MAX];
> +};
> +
> +static const struct asus_ec_led_config asus_ec_leds[] = {
> + [ASUSEC_LED_AMBER] = {
> + .name = "amber",
> + .color = LED_COLOR_ID_AMBER,
> + .ctrl_bit = ASUSEC_CTL_LED_AMBER,
> + },
> + [ASUSEC_LED_GREEN] = {
> + .name = "green",
> + .color = LED_COLOR_ID_GREEN,
> + .ctrl_bit = ASUSEC_CTL_LED_GREEN,
> + },
> +};
> +
> +static enum led_brightness asus_ec_led_get_brightness(struct led_classdev *cdev)
> +{
> + struct asus_ec_led *led = container_of(cdev, struct asus_ec_led, cdev);
> + const struct asusec_core *ec = led->ddata->ec;
I'm getting confused here.
ddata is what I'd be calling the device data struct passed by the parent?
In fact, ddata is a little known concept in Leds. Any reason to go for
this over the standard nomenclature?
> + u64 ctl;
> + int ret;
> +
> + ret = asus_dockram_access_ctl(ec->dockram, &ctl, 0, 0);
Did we discuss preferring regmap already?
> + if (ret)
> + return LED_OFF;
> +
> + return ctl & led->ctrl_bit ? LED_ON : LED_OFF;
> +}
> +
> +static int asus_ec_led_set_brightness(struct led_classdev *cdev,
> + enum led_brightness brightness)
> +{
> + struct asus_ec_led *led = container_of(cdev, struct asus_ec_led, cdev);
> + const struct asusec_core *ec = led->ddata->ec;
> +
> + if (brightness)
> + return asus_dockram_access_ctl(ec->dockram, NULL,
> + led->ctrl_bit, led->ctrl_bit);
> +
> + return asus_dockram_access_ctl(ec->dockram, NULL, led->ctrl_bit, 0);
> +}
> +
> +static int asus_ec_led_probe(struct platform_device *pdev)
> +{
> + const struct asusec_core *ec = dev_get_drvdata(pdev->dev.parent);
> + struct asus_ec_leds_data *ddata;
> + struct device *dev = &pdev->dev;
> + int i, ret;
Could we declare the loop counter 'i' directly within the 'for' statement's
scope to keep its scope limited? For example, 'for (int i = 0; ...)'.
> +
> + ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> + if (!ddata)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, ddata);
> + ddata->ec = ec;
> +
> + for (i = 0; i < ASUSEC_LED_MAX; i++) {
Nit: for (int i = ...
> + const struct asus_ec_led_config *cfg = &asus_ec_leds[i];
> + struct asus_ec_led *led = &ddata->leds[i];
> +
> + led->cdev.name = devm_kasprintf(dev, GFP_KERNEL, "%s::%s",
> + ddata->ec->name, cfg->name);
> + if (!led->cdev.name)
> + return -ENOMEM;
> +
> + led->cdev.max_brightness = 1;
> + led->cdev.color = cfg->color;
> + led->cdev.flags = LED_CORE_SUSPENDRESUME | LED_RETAIN_AT_SHUTDOWN;
> + led->cdev.brightness_get = asus_ec_led_get_brightness;
> + led->cdev.brightness_set_blocking = asus_ec_led_set_brightness;
> +
> + led->ddata = ddata;
> + led->ctrl_bit = cfg->ctrl_bit;
> +
> + ret = devm_led_classdev_register(dev, &led->cdev);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to register %s LED\n",
> + cfg->name);
Should we capitalise the error message here to match our style guidelines
(e.g. 'Failed to register...')?
> + }
> +
> + return 0;
> +}
> +
> +static struct platform_driver asus_ec_led_driver = {
> + .driver.name = "asus-transformer-ec-led",
> + .probe = asus_ec_led_probe,
> +};
> +module_platform_driver(asus_ec_led_driver);
> +
> +MODULE_ALIAS("platform:asus-transformer-ec-led");
> +MODULE_AUTHOR("Michał Mirosław <mirq-linux@rere.qmqm.pl>");
> +MODULE_AUTHOR("Svyatoslav Ryhel <clamor95@gmail.com>");
> +MODULE_DESCRIPTION("ASUS Transformer's charging LED driver");
> +MODULE_LICENSE("GPL");
> --
> 2.51.0
>
>
--
Lee Jones
^ permalink raw reply
* Re: [PATCH v2 0/7] HID: iio: basic clean up for usage_id
From: Jonathan Cameron @ 2026-06-11 11:59 UTC (permalink / raw)
To: Sanjay Chitroda
Cc: Jiri Kosina, Srinivas Pandruvada, David Lechner, Nuno Sá,
Andy Shevchenko, linux-input, linux-iio, linux-kernel,
Maxwell Doose
In-Reply-To: <20260610-6-june-hid-iio-correct-usage-id-v2-0-c3c5f0720493@gmail.com>
On Wed, 10 Jun 2026 21:07:01 +0530
Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
> Hi all,
>
> This series updates all HID IIO drivers to use 'u32' instead of
> bare 'unsigned' for the usage_id parameter.
>
> This matches expected callback API type and improves code clarity,
> as HID usage IDs are defined as 32-bit values.
>
> No functional changes are introduced.
>
> Testing:
> - Compiled with W=1 for each patch in the series
LGTM so I've applied it to the testing branch of iio.git. However, I won't
be doing another pull request this cycle so this tree will be rebased once rc1
is available. In meantime I'm happy to add tags or indeed back this out
if people have feedback.
Thanks,
Jonathan
>
> ---
> Changes in v2:
> - rectify commit message with input from Jonathan
> - added reviewed by tag in all change of series
> - Link to v1: https://patch.msgid.link/20260606-6-june-hid-iio-correct-usage-id-v1-0-dd4a6820b674@gmail.com
>
> ---
> Sanjay Chitroda (7):
> iio: gyro: hid-sensor-gyro-3d: use u32 instead of unsigned
> iio: accel: hid-sensor-accel-3d: use u32 instead of unsigned
> iio: light: hid-sensor-als: use u32 instead of unsigned
> iio: light: hid-sensor-prox: use u32 instead of unsigned
> iio: orientation: hid-sensor-incl-3d: use u32 instead of unsigned
> iio: orientation: hid-sensor-rotation: use u32 instead of unsigned
> iio: pressure: hid-sensor-press: use u32 instead of unsigned
>
> drivers/iio/accel/hid-sensor-accel-3d.c | 6 +++---
> drivers/iio/gyro/hid-sensor-gyro-3d.c | 6 +++---
> drivers/iio/light/hid-sensor-als.c | 6 +++---
> drivers/iio/light/hid-sensor-prox.c | 4 ++--
> drivers/iio/orientation/hid-sensor-incl-3d.c | 6 +++---
> drivers/iio/orientation/hid-sensor-rotation.c | 6 +++---
> drivers/iio/pressure/hid-sensor-press.c | 6 +++---
> 7 files changed, 20 insertions(+), 20 deletions(-)
> ---
> base-commit: ae696dfa47c30016cd429b9db5e70b259b8f509e
> change-id: 20260606-6-june-hid-iio-correct-usage-id-57ce92cb102b
>
> Best regards,
> --
> Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
^ permalink raw reply
* Re: [PATCH v8 2/7] mfd: Add driver for ASUS Transformer embedded controller
From: Svyatoslav Ryhel @ 2026-06-11 12:16 UTC (permalink / raw)
To: Lee Jones
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Pavel Machek, Sebastian Reichel, Ion Agorria,
Michał Mirosław, devicetree, linux-kernel, linux-input,
linux-leds, linux-pm
In-Reply-To: <20260611111732.GN4151951@google.com>
чт, 11 черв. 2026 р. о 14:17 Lee Jones <lee@kernel.org> пише:
>
> On Thu, 28 May 2026, Svyatoslav Ryhel wrote:
> > From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> >
> > Support Nuvoton NPCE795-based ECs as used in Asus Transformer TF201,
> > TF300T, TF300TG, TF300TL and TF700T pad and dock, as well as TF101 dock
> > and TF600T, P1801-T and TF701T pad. This is a glue driver handling
> > detection and common operations for EC's functions.
> >
> > Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > ---
> > drivers/mfd/Kconfig | 16 +
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/asus-transformer-ec.c | 542 ++++++++++++++++++++++++
> > include/linux/mfd/asus-transformer-ec.h | 92 ++++
> > 4 files changed, 651 insertions(+)
> > create mode 100644 drivers/mfd/asus-transformer-ec.c
> > create mode 100644 include/linux/mfd/asus-transformer-ec.h
> >
...
> > +
> > + memset(buf, 0, ASUSEC_ENTRY_BUFSIZE);
> > + ret = i2c_smbus_read_i2c_block_data(client, ASUSEC_DOCKRAM_CONTROL,
> > + ASUSEC_ENTRY_SIZE, buf);
> > + if (ret < ASUSEC_ENTRY_SIZE) {
> > + dev_err(&client->dev, "failed to access control buffer: %d\n",
> > + ret);
> > + return ret;
>
> Should we return a negative error code here if the read is shorter than
> expected, rather than propagating the positive byte count?
>
Yes, I have adjusted it already locally for the next iteration. It
will return ret if negative and -EIO if ret is pos but less then
ASUSEC_ENTRY_SIZE (return ret < 0 ? ret : -EIO)
> > + }
> > +
> > + if (buf[0] != ASUSEC_CTL_SIZE) {
> > + dev_err(&client->dev, "buffer size exceeds %d: %d\n",
> > + ASUSEC_CTL_SIZE, buf[0]);
> > + return -EPROTO;
> > + }
> > +
> > + val = get_unaligned_le64(buf + 1);
> > +
> > + if (out)
> > + *out = val;
> > +
> > + if (mask || xor) {
> > + put_unaligned_le64((val & ~mask) ^ xor, buf + 1);
> > + ret = i2c_smbus_write_i2c_block_data(client,
> > + ASUSEC_DOCKRAM_CONTROL,
> > + ASUSEC_ENTRY_SIZE, buf);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(asus_dockram_access_ctl);
> > +
> > +static int asus_ec_signal_request(struct asus_ec_data *ddata)
> > +{
> > + guard(mutex)(&ddata->ecreq_lock);
> > +
> > + gpiod_set_value_cansleep(ddata->ecreq_gpio, 1);
> > + msleep(50);
> > +
> > + gpiod_set_value_cansleep(ddata->ecreq_gpio, 0);
> > + msleep(200);
>
> Do these numbers come from the datasheet or were they arbitrarily chosen?
>
There is no datasheet. These delays come from downstream driver and
with lower values or removed delays request fails.
> > +
> > + return 0;
> > +}
> > +
> > +static void asus_ec_clear_buffer(struct asus_ec_data *ddata)
> > +{
> > + int ret, retry = ASUSEC_RSP_BUFFER_SIZE;
> > +
> > + /*
> > + * Read the buffer till we get valid data by checking ASUSEC_OBF_MASK
> > + * of the status byte or till we reach end of the 256 byte buffer.
> > + */
> > + while (retry--) {
> > + ret = i2c_smbus_read_i2c_block_data(ddata->client, ASUSEC_READ_BUF,
> > + ASUSEC_ENTRY_SIZE,
> > + ddata->ec_buf);
> > + if (ret < ASUSEC_ENTRY_SIZE)
> > + continue;
> > +
> > + if (ddata->ec_buf[ASUSEC_IRQ_STATUS] & ASUSEC_OBF_MASK)
> > + continue;
> > +
> > + break;
> > + }
> > +}
> > +
> > +static int asus_ec_log_info(struct asus_ec_data *ddata, unsigned int reg,
> > + const char *name, const char **out)
> > +{
> > + struct device *dev = &ddata->client->dev;
> > + u8 buf[ASUSEC_ENTRY_BUFSIZE];
> > + int ret;
> > +
> > + memset(buf, 0, ASUSEC_ENTRY_BUFSIZE);
> > + ret = i2c_smbus_read_i2c_block_data(ddata->ec.dockram, reg,
> > + ASUSEC_ENTRY_SIZE, buf);
> > + if (ret < ASUSEC_ENTRY_SIZE)
> > + return ret;
>
> Same here. These should be negative.
>
return ret < 0 ? ret : -EIO same as above
> > +
> > + if (buf[0] > ASUSEC_ENTRY_SIZE) {
> > + dev_err(dev, "bad data len; buffer: %*ph; ret: %d\n",
> > + ASUSEC_ENTRY_BUFSIZE, buf, ret);
> > + return -EPROTO;
> > + }
> > +
> > + if (!ddata->logging_disabled) {
> > + dev_info(dev, "%-14s: %.*s\n", name, buf[0], buf + 1);
> > +
> > + if (out) {
> > + *out = devm_kasprintf(dev, GFP_KERNEL, "%.*s",
> > + buf[0], buf + 1);
> > + if (!*out)
> > + return -ENOMEM;
> > + }
> > + }
>
> FWIW, I hate this! What does it give you now that development is done?
>
We have already discussed this, and you agreed that EC and firmware
prints may stay! This prints EC model and firmware info as well as EC
firmware behavior. It allows identify possible new revisions of EC -
Firmware combo and address possible regressions (check if it is chip
malfunction or firmware needs a new programming model) without
rebuilding kernel and digging downstream kernel for needed bits of
code.
> > + return 0;
> > +}
> > +
> > +static int asus_ec_reset(struct asus_ec_data *ddata)
> > +{
> > + int retry, ret;
> > +
> > + guard(mutex)(&ddata->ecreq_lock);
> > +
> > + for (retry = 0; retry < ASUSEC_RETRY_MAX; retry++) {
>
> for (int return = ... is generally preferred for throwaway variables.
>
Not that I care too much, but I am defining ret anyway, why not add
retry too there?
>
> > + ret = i2c_smbus_write_word_data(ddata->client, ASUSEC_WRITE_BUF,
> > + ASUSEC_RESET);
> > + if (!ret)
> > + return 0;
> > +
> > + msleep(ASUSEC_ACCESS_TIMEOUT);
>
> I like that this is defined, can we do that with the others please?
>
I don't see any benefits of defining delays, they are all arbitrary
and derive from downstream kernel, removing or altering them caused
malfunction.
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int asus_ec_susb_on_status(struct asus_ec_data *ddata)
> > +{
> > + u64 flag;
> > + int ret;
> > +
> > + ret = asus_dockram_access_ctl(ddata->ec.dockram, &flag, 0, 0);
> > + if (ret)
> > + return ret;
> > +
> > + flag &= ASUSEC_CTL_SUSB_MODE;
> > + dev_info(&ddata->client->dev, "EC FW behaviour: %s\n",
> > + flag ? "susb on when receive ec_req" :
> > + "susb on when system wakeup");
> > +
> > + return 0;
> > +}
> > +
> > +static int asus_ec_set_factory_mode(struct asus_ec_data *ddata,
> > + enum asusec_mode fmode)
> > +{
> > + dev_info(&ddata->client->dev, "Entering %s mode.\n",
> > + fmode == ASUSEC_MODE_FACTORY ? "factory" : "normal");
> > +
> > + return asus_dockram_access_ctl(ddata->ec.dockram, NULL,
> > + ASUSEC_CTL_FACTORY_MODE,
> > + fmode == ASUSEC_MODE_FACTORY ?
> > + ASUSEC_CTL_FACTORY_MODE : 0);
>
> Why not create make:
>
> ASUSEC_MODE_FACTORY == ASUSEC_CTL_FACTORY_MODE
>
> What happens to NORMAL?
>
ASUSEC_CTL_FACTORY_MODE is a bit in the ctl register. For NORMAL mode
bit is cleared,
for FACTORY bit it set, for NONE bit is ignored.
> > +}
> > +
> > +static int asus_ec_detect(struct asus_ec_data *ddata)
> > +{
> > + int ret;
> > +
> > + ret = asus_ec_reset(ddata);
> > + if (ret)
> > + goto err_exit;
> > +
> > + asus_ec_clear_buffer(ddata);
> > +
> > + ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_MODEL, "Model",
> > + &ddata->ec.model);
>
> You can use 100-chars and make the code look beautiful! :)
>
Not every subsystem permits 100 chars, some stick to 80 as a strict
rule, so it is better be safe.
> > + if (ret)
> > + goto err_exit;
> > +
> > + ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_FW, "FW version",
> > + NULL);
> > + if (ret)
> > + goto err_exit;
> > +
> > + ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_CFGFMT, "Config format",
> > + NULL);
> > + if (ret)
> > + goto err_exit;
> > +
> > + ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_HW, "HW version",
> > + NULL);
> > + if (ret)
> > + goto err_exit;
> > +
> > + /* Disable logging on next EC request */
>
> Why, but why?
>
Cause EC requests are frequent (handshake/reset) and constant logging
same data is not acceptable.
> > + ddata->logging_disabled = true;
> > +
> > + /* Check and inform about EC firmware behavior */
> > + ret = asus_ec_susb_on_status(ddata);
> > + if (ret)
> > + goto err_exit;
> > +
> > + ddata->ec.name = ddata->info->name;
> > +
> > + /* Some EC require factory mode to be set normal on each request */
> > + if (ddata->info->fmode)
> > + ret = asus_ec_set_factory_mode(ddata, ddata->info->fmode);
> > +
> > +err_exit:
> > + if (ret)
> > + dev_err(&ddata->client->dev, "failed to access EC: %d\n", ret);
> > +
> > + return ret;
> > +}
> > +
> > +static void asus_ec_handle_smi(struct asus_ec_data *ddata, unsigned int code)
> > +{
> > + switch (code) {
> > + case ASUSEC_SMI_HANDSHAKE:
> > + case ASUSEC_SMI_RESET:
> > + asus_ec_detect(ddata);
> > + break;
> > + }
> > +}
> > +
> > +static irqreturn_t asus_ec_interrupt(int irq, void *dev_id)
> > +{
> > + struct asus_ec_data *ddata = dev_id;
> > + unsigned long notify_action;
> > + int ret;
> > +
> > + ret = i2c_smbus_read_i2c_block_data(ddata->client, ASUSEC_READ_BUF,
> > + ASUSEC_ENTRY_SIZE, ddata->ec_buf);
> > + if (ret < ASUSEC_ENTRY_SIZE ||
> > + !(ddata->ec_buf[ASUSEC_IRQ_STATUS] & ASUSEC_OBF_MASK))
>
> Unwrap for readability.
>
> Also, I think a comment would be helpful.
>
if (ret < ASUSEC_ENTRY_SIZE)
return IRQ_NONE;
ret = ddata->ec_buf[ASUSEC_IRQ_STATUS] & ASUSEC_OBF_MASK;
if (!ret)
return IRQ_NONE;
This would be acceptable? (I will add comments later on)
> > + return IRQ_NONE;
> > +
> > + notify_action = ddata->ec_buf[ASUSEC_IRQ_STATUS];
> > + if (notify_action & ASUSEC_SMI_MASK) {
> > + unsigned int code = ddata->ec_buf[ASUSEC_SMI_CODE];
> > +
> > + asus_ec_handle_smi(ddata, code);
> > +
> > + notify_action |= code << 8;
> > + }
> > +
> > + blocking_notifier_call_chain(&ddata->ec.notify_list,
> > + notify_action, ddata->ec_buf);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static void asus_ec_release_dockram_dev(void *client)
> > +{
> > + i2c_unregister_device(client);
> > +}
> > +
> > +static struct i2c_client *devm_asus_dockram_get(struct device *dev)
> > +{
> > + struct i2c_client *parent = to_i2c_client(dev);
> > + struct i2c_client *dockram;
> > + struct dockram_ec_data *ddata;
> > + int ret;
> > +
> > + dockram = i2c_new_ancillary_device(parent, "dockram",
> > + parent->addr + 2);
>
> Could we define a macro for the address offset '2' here to avoid using a magic
> number?
>
It seems that you are excessively concerned with "magic numbers".
> > + if (IS_ERR(dockram))
> > + return dockram;
> > +
> > + ret = devm_add_action_or_reset(dev, asus_ec_release_dockram_dev,
> > + dockram);
> > + if (ret)
> > + return ERR_PTR(ret);
> > +
> > + ddata = devm_kzalloc(&dockram->dev, sizeof(*ddata), GFP_KERNEL);
> > + if (!ddata)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + i2c_set_clientdata(dockram, ddata);
> > + mutex_init(&ddata->ctl_lock);
> > +
> > + return dockram;
> > +}
> > +
> > +static const struct mfd_cell asus_ec_sl101_dock_mfd_devices[] = {
> > + MFD_CELL_NAME("asus-transformer-ec-kbc"),
> > +};
> > +
> > +static const struct mfd_cell asus_ec_tf101_dock_mfd_devices[] = {
> > + MFD_CELL_BASIC("asus-transformer-ec-battery", NULL, NULL, 0, 1),
> > + MFD_CELL_BASIC("asus-transformer-ec-charger", NULL, NULL, 0, 1),
> > + MFD_CELL_BASIC("asus-transformer-ec-led", NULL, NULL, 0, 1),
> > + MFD_CELL_NAME("asus-transformer-ec-keys"),
> > + MFD_CELL_NAME("asus-transformer-ec-kbc"),
> > +};
> > +
> > +static const struct mfd_cell asus_ec_tf201_pad_mfd_devices[] = {
> > + MFD_CELL_NAME("asus-transformer-ec-battery"),
> > + MFD_CELL_NAME("asus-transformer-ec-led"),
> > +};
> > +
> > +static const struct mfd_cell asus_ec_tf600t_pad_mfd_devices[] = {
> > + MFD_CELL_NAME("asus-transformer-ec-battery"),
> > + MFD_CELL_NAME("asus-transformer-ec-charger"),
> > + MFD_CELL_NAME("asus-transformer-ec-led"),
> > +};
> > +
> > +static int asus_ec_probe(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > + struct asus_ec_data *ddata;
> > + const struct mfd_cell *cells;
> > + unsigned int num_cells;
> > + unsigned long irqflags;
> > + int ret;
> > +
> > + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_I2C_BLOCK))
> > + return dev_err_probe(dev, -ENXIO,
> > + "I2C bus is missing required SMBus block mode support\n");
> > +
> > + ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> > + if (!ddata)
> > + return -ENOMEM;
> > +
> > + ddata->info = device_get_match_data(dev);
> > + if (!ddata->info)
> > + return -ENODEV;
> > +
> > + switch (ddata->info->variant) {
> > + case ASUSEC_SL101_DOCK:
> > + cells = asus_ec_sl101_dock_mfd_devices;
> > + num_cells = ARRAY_SIZE(asus_ec_sl101_dock_mfd_devices);
> > + break;
> > + case ASUSEC_TF101_DOCK:
> > + cells = asus_ec_tf101_dock_mfd_devices;
> > + num_cells = ARRAY_SIZE(asus_ec_tf101_dock_mfd_devices);
> > + break;
> > + case ASUSEC_TF201_PAD:
> > + cells = asus_ec_tf201_pad_mfd_devices;
> > + num_cells = ARRAY_SIZE(asus_ec_tf201_pad_mfd_devices);
> > + break;
> > + case ASUSEC_TF600T_PAD:
> > + cells = asus_ec_tf600t_pad_mfd_devices;
> > + num_cells = ARRAY_SIZE(asus_ec_tf600t_pad_mfd_devices);
> > + break;
> > + default:
> > + return dev_err_probe(dev, -EINVAL,
> > + "unknown device variant %d\n",
> > + ddata->info->variant);
> > + }
> > +
> > + i2c_set_clientdata(client, ddata);
> > + ddata->client = client;
> > +
> > + ddata->ec.dockram = devm_asus_dockram_get(dev);
> > + if (IS_ERR(ddata->ec.dockram))
> > + return dev_err_probe(dev, PTR_ERR(ddata->ec.dockram),
> > + "failed to get dockram\n");
> > +
> > + ddata->ecreq_gpio = devm_gpiod_get(dev, "request", GPIOD_OUT_LOW);
> > + if (IS_ERR(ddata->ecreq_gpio))
> > + return dev_err_probe(dev, PTR_ERR(ddata->ecreq_gpio),
> > + "failed to get EC request GPIO\n");
> > +
> > + BLOCKING_INIT_NOTIFIER_HEAD(&ddata->ec.notify_list);
> > + mutex_init(&ddata->ecreq_lock);
> > +
> > + asus_ec_signal_request(ddata);
> > +
> > + ret = asus_ec_detect(ddata);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to detect EC version\n");
> > +
> > + /*
> > + * Systems using device tree should set up interrupt via DTS,
> > + * the rest will use the default low interrupt.
> > + */
> > + irqflags = dev->of_node ? 0 : IRQF_TRIGGER_LOW;
> > +
> > + ret = devm_request_threaded_irq(dev, client->irq, NULL,
> > + &asus_ec_interrupt,
> > + IRQF_ONESHOT | irqflags,
> > + client->name, ddata);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to register IRQ\n");
> > +
> > + /* Parent I2C controller uses DMA, ASUS EC and child devices do not */
> > + client->dev.coherent_dma_mask = 0;
> > + client->dev.dma_mask = &client->dev.coherent_dma_mask;
> > +
> > + return devm_mfd_add_devices(dev, 0, cells, num_cells, NULL, 0, NULL);
> > +}
> > +
> > +static const struct asus_ec_chip_info asus_ec_sl101_dock_data = {
> > + .name = "dock",
> > + .variant = ASUSEC_SL101_DOCK,
> > + .fmode = ASUSEC_MODE_NONE,
> > +};
> > +
> > +static const struct asus_ec_chip_info asus_ec_tf101_dock_data = {
> > + .name = "dock",
> > + .variant = ASUSEC_TF101_DOCK,
> > + .fmode = ASUSEC_MODE_NONE,
> > +};
> > +
> > +static const struct asus_ec_chip_info asus_ec_tf201_pad_data = {
> > + .name = "pad",
> > + .variant = ASUSEC_TF201_PAD,
> > + .fmode = ASUSEC_MODE_NORMAL,
> > +};
> > +
> > +static const struct asus_ec_chip_info asus_ec_tf600t_pad_data = {
> > + .name = "pad",
> > + .variant = ASUSEC_TF600T_PAD,
> > + .fmode = ASUSEC_MODE_NORMAL,
> > +};
>
> Any reason not to just pass the identifier (variant) and add the name
> and fmode attribues to the switch() above?
Why not set it here, I am not passing any mfd or any other API via of data.
> > +
> > +static const struct of_device_id asus_ec_match[] = {
> > + {
> > + .compatible = "asus,sl101-ec-dock",
> > + .data = &asus_ec_sl101_dock_data
> > + }, {
> > + .compatible = "asus,tf101-ec-dock",
> > + .data = &asus_ec_tf101_dock_data
> > + }, {
> > + .compatible = "asus,tf201-ec-pad",
> > + .data = &asus_ec_tf201_pad_data
> > + }, {
> > + .compatible = "asus,tf600t-ec-pad",
> > + .data = &asus_ec_tf600t_pad_data
> > + },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, asus_ec_match);
> > +
> > +static struct i2c_driver asus_ec_driver = {
> > + .driver = {
> > + .name = "asus-transformer-ec",
> > + .of_match_table = asus_ec_match,
> > + },
> > + .probe = asus_ec_probe,
> > +};
> > +module_i2c_driver(asus_ec_driver);
> > +
> > +MODULE_AUTHOR("Michał Mirosław <mirq-linux@rere.qmqm.pl>");
> > +MODULE_AUTHOR("Svyatoslav Ryhel <clamor95@gmail.com>");
> > +MODULE_DESCRIPTION("ASUS Transformer's EC driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/linux/mfd/asus-transformer-ec.h b/include/linux/mfd/asus-transformer-ec.h
> > new file mode 100644
> > index 000000000000..f085eea2193e
> > --- /dev/null
> > +++ b/include/linux/mfd/asus-transformer-ec.h
> > @@ -0,0 +1,92 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +#ifndef __MFD_ASUS_TRANSFORMER_EC_H
> > +#define __MFD_ASUS_TRANSFORMER_EC_H
> > +
> > +#include <linux/notifier.h>
> > +#include <linux/platform_device.h>
> > +
> > +struct i2c_client;
> > +
> > +/**
> > + * struct asusec_core - public part shared with all cells
> > + *
> > + * @model: firmware version running on the EC
> > + * @name: prefix associated with the EC
> > + * @dockram: pointer to Dockram's i2c_client
> > + * @notify_list: notify list used by cells
> > + */
> > +struct asusec_core {
> > + const char *model;
> > + const char *name;
> > + struct i2c_client *dockram;
> > + struct blocking_notifier_head notify_list;
> > +};
> > +
> > +#define ASUSEC_ENTRIES 0x100
> > +#define ASUSEC_ENTRY_SIZE 32
> > +#define ASUSEC_ENTRY_BUFSIZE (ASUSEC_ENTRY_SIZE + 1)
> > +
> > +/* interrupt sources */
> > +#define ASUSEC_IRQ_STATUS 1
> > +#define ASUSEC_OBF_MASK BIT(0)
> > +#define ASUSEC_KEY_MASK BIT(2)
> > +#define ASUSEC_KBC_MASK BIT(3)
> > +#define ASUSEC_AUX_MASK BIT(5)
> > +#define ASUSEC_SCI_MASK BIT(6)
> > +#define ASUSEC_SMI_MASK BIT(7)
> > +
> > +/* SMI notification codes */
> > +#define ASUSEC_SMI_CODE 2
> > +#define ASUSEC_SMI_POWER_NOTIFY 0x31 /* USB cable plug event */
> > +#define ASUSEC_SMI_HANDSHAKE 0x50 /* response to ec_req edge */
> > +#define ASUSEC_SMI_WAKE 0x53
> > +#define ASUSEC_SMI_RESET 0x5f
> > +#define ASUSEC_SMI_ADAPTER_EVENT 0x60 /* charger to dock plug event */
> > +#define ASUSEC_SMI_BACKLIGHT_ON 0x63
> > +#define ASUSEC_SMI_AUDIO_DOCK_IN 0x70
> > +
> > +#define ASUSEC_SMI_ACTION(code) (ASUSEC_SMI_MASK | ASUSEC_OBF_MASK | \
> > + (ASUSEC_SMI_##code << 8))
> > +
> > +/* control register [0x0a] layout */
> > +#define ASUSEC_CTL_SIZE 8
> > +
> > +/*
> > + * EC reports power from 40-pin connector in the LSB of the control
> > + * register. The following values have been observed (xor 0x02):
> > + *
> > + * PAD-ec no-plug 0x40 / PAD-ec DOCK 0x20 / DOCK-ec no-plug 0x40
> > + * PAD-ec AC 0x25 / PAD-ec DOCK+AC 0x24 / DOCK-ec AC 0x25
> > + * PAD-ec USB 0x45 / PAD-ec DOCK+USB 0x24 / DOCK-ec USB 0x41
> > + */
> > +
> > +#define ASUSEC_CTL_DIRECT_POWER_SOURCE BIT_ULL(0)
> > +#define ASUSEC_STAT_CHARGING BIT_ULL(2)
> > +#define ASUSEC_CTL_FULL_POWER_SOURCE BIT_ULL(5)
> > +#define ASUSEC_CTL_SUSB_MODE BIT_ULL(9)
> > +#define ASUSEC_CMD_SUSPEND_S3 BIT_ULL(33)
> > +#define ASUSEC_CTL_TEST_DISCHARGE BIT_ULL(35)
> > +#define ASUSEC_CMD_SUSPEND_INHIBIT BIT_ULL(37)
> > +#define ASUSEC_CTL_FACTORY_MODE BIT_ULL(38)
> > +#define ASUSEC_CTL_KEEP_AWAKE BIT_ULL(39)
> > +#define ASUSEC_CTL_USB_CHARGE BIT_ULL(40)
> > +#define ASUSEC_CTL_LED_BLINK BIT_ULL(40)
> > +#define ASUSEC_CTL_LED_AMBER BIT_ULL(41)
> > +#define ASUSEC_CTL_LED_GREEN BIT_ULL(42)
> > +#define ASUSEC_CMD_SWITCH_HDMI BIT_ULL(56)
> > +#define ASUSEC_CMD_WIN_SHUTDOWN BIT_ULL(62)
> > +
> > +#define ASUSEC_DOCKRAM_INFO_MODEL 0x01
> > +#define ASUSEC_DOCKRAM_INFO_FW 0x02
> > +#define ASUSEC_DOCKRAM_INFO_CFGFMT 0x03
> > +#define ASUSEC_DOCKRAM_INFO_HW 0x04
> > +#define ASUSEC_DOCKRAM_CONTROL 0x0a
> > +#define ASUSEC_DOCKRAM_BATT_CTL 0x14
> > +
> > +#define ASUSEC_WRITE_BUF 0x64
> > +#define ASUSEC_READ_BUF 0x6a
> > +
> > +int asus_dockram_access_ctl(struct i2c_client *client,
> > + u64 *out, u64 mask, u64 xor);
> > +
> > +#endif /* __MFD_ASUS_TRANSFORMER_EC_H */
> > --
> > 2.51.0
> >
>
> --
> Lee Jones
^ permalink raw reply
* Re: [PATCH v8 5/7] leds: Add driver for ASUS Transformer LEDs
From: Svyatoslav Ryhel @ 2026-06-11 12:27 UTC (permalink / raw)
To: Lee Jones
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Pavel Machek, Sebastian Reichel, Ion Agorria,
Michał Mirosław, devicetree, linux-kernel, linux-input,
linux-leds, linux-pm
In-Reply-To: <20260611113037.GO4151951@google.com>
чт, 11 черв. 2026 р. о 14:30 Lee Jones <lee@kernel.org> пише:
>
> On Thu, 28 May 2026, Svyatoslav Ryhel wrote:
>
> > From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> >
> > ASUS Transformer tablets have a green and an amber LED on both the Pad
> > and the Dock. If both LEDs are enabled simultaneously, the emitted light
> > will be yellow.
> >
> > Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > ---
> > drivers/leds/Kconfig | 11 +++
> > drivers/leds/Makefile | 1 +
> > drivers/leds/leds-asus-transformer-ec.c | 125 ++++++++++++++++++++++++
> > 3 files changed, 137 insertions(+)
> > create mode 100644 drivers/leds/leds-asus-transformer-ec.c
> >
> > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> > index f4a0a3c8c870..f637d23400a8 100644
> > --- a/drivers/leds/Kconfig
> > +++ b/drivers/leds/Kconfig
> > @@ -120,6 +120,17 @@ config LEDS_OSRAM_AMS_AS3668
> > To compile this driver as a module, choose M here: the module
> > will be called leds-as3668.
> >
> > +config LEDS_ASUS_TRANSFORMER_EC
> > + tristate "LED Support for Asus Transformer charging LED"
> > + depends on LEDS_CLASS
> > + depends on MFD_ASUS_TRANSFORMER_EC
> > + help
> > + This option enables support for charging indicator on
> > + Asus Transformer's Pad and it's Dock.
> > +
> > + To compile this driver as a module, choose M here: the module
> > + will be called leds-asus-transformer-ec.
> > +
> > config LEDS_AW200XX
> > tristate "LED support for Awinic AW20036/AW20054/AW20072/AW20108"
> > depends on LEDS_CLASS
> > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> > index 8fdb45d5b439..d5395c3f1124 100644
> > --- a/drivers/leds/Makefile
> > +++ b/drivers/leds/Makefile
> > @@ -16,6 +16,7 @@ obj-$(CONFIG_LEDS_AN30259A) += leds-an30259a.o
> > obj-$(CONFIG_LEDS_APU) += leds-apu.o
> > obj-$(CONFIG_LEDS_ARIEL) += leds-ariel.o
> > obj-$(CONFIG_LEDS_AS3668) += leds-as3668.o
> > +obj-$(CONFIG_LEDS_ASUS_TRANSFORMER_EC) += leds-asus-transformer-ec.o
> > obj-$(CONFIG_LEDS_AW200XX) += leds-aw200xx.o
> > obj-$(CONFIG_LEDS_AW2013) += leds-aw2013.o
> > obj-$(CONFIG_LEDS_BCM6328) += leds-bcm6328.o
> > diff --git a/drivers/leds/leds-asus-transformer-ec.c b/drivers/leds/leds-asus-transformer-ec.c
> > new file mode 100644
> > index 000000000000..09503e76331c
> > --- /dev/null
> > +++ b/drivers/leds/leds-asus-transformer-ec.c
> > @@ -0,0 +1,125 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +#include <linux/err.h>
> > +#include <linux/leds.h>
> > +#include <linux/mfd/asus-transformer-ec.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +
> > +enum {
> > + ASUSEC_LED_AMBER,
> > + ASUSEC_LED_GREEN,
> > + ASUSEC_LED_MAX
> > +};
> > +
> > +struct asus_ec_led_config {
> > + const char *name;
> > + unsigned int color;
> > + unsigned long long ctrl_bit;
>
> Should we use 'u64' here instead of 'unsigned long long' to align with standard
> kernel integer types?
>
sure
> > +};
> > +
> > +struct asus_ec_led {
> > + struct asus_ec_leds_data *ddata;
> > + struct led_classdev cdev;
> > + unsigned long long ctrl_bit;
>
> Should we use 'u64' here as well to keep it consistent?
>
sure
> > +};
> > +
> > +struct asus_ec_leds_data {
> > + const struct asusec_core *ec;
> > + struct asus_ec_led leds[ASUSEC_LED_MAX];
> > +};
> > +
> > +static const struct asus_ec_led_config asus_ec_leds[] = {
> > + [ASUSEC_LED_AMBER] = {
> > + .name = "amber",
> > + .color = LED_COLOR_ID_AMBER,
> > + .ctrl_bit = ASUSEC_CTL_LED_AMBER,
> > + },
> > + [ASUSEC_LED_GREEN] = {
> > + .name = "green",
> > + .color = LED_COLOR_ID_GREEN,
> > + .ctrl_bit = ASUSEC_CTL_LED_GREEN,
> > + },
> > +};
> > +
> > +static enum led_brightness asus_ec_led_get_brightness(struct led_classdev *cdev)
> > +{
> > + struct asus_ec_led *led = container_of(cdev, struct asus_ec_led, cdev);
> > + const struct asusec_core *ec = led->ddata->ec;
>
> I'm getting confused here.
>
> ddata is what I'd be calling the device data struct passed by the parent?
>
> In fact, ddata is a little known concept in Leds. Any reason to go for
> this over the standard nomenclature?
>
How then it should be named? It holds each LED's control bit.
> > + u64 ctl;
> > + int ret;
> > +
> > + ret = asus_dockram_access_ctl(ec->dockram, &ctl, 0, 0);
>
> Did we discuss preferring regmap already?
>
Yes, you were fine with all and even said that you will merge
everything with Dmitry Ack for input
HERE https://lore.kernel.org/lkml/20260527144619.GA671544@google.com/
Yet now I get a new pile of complaints and nits.
> > + if (ret)
> > + return LED_OFF;
> > +
> > + return ctl & led->ctrl_bit ? LED_ON : LED_OFF;
> > +}
> > +
> > +static int asus_ec_led_set_brightness(struct led_classdev *cdev,
> > + enum led_brightness brightness)
> > +{
> > + struct asus_ec_led *led = container_of(cdev, struct asus_ec_led, cdev);
> > + const struct asusec_core *ec = led->ddata->ec;
> > +
> > + if (brightness)
> > + return asus_dockram_access_ctl(ec->dockram, NULL,
> > + led->ctrl_bit, led->ctrl_bit);
> > +
> > + return asus_dockram_access_ctl(ec->dockram, NULL, led->ctrl_bit, 0);
> > +}
> > +
> > +static int asus_ec_led_probe(struct platform_device *pdev)
> > +{
> > + const struct asusec_core *ec = dev_get_drvdata(pdev->dev.parent);
> > + struct asus_ec_leds_data *ddata;
> > + struct device *dev = &pdev->dev;
> > + int i, ret;
>
> Could we declare the loop counter 'i' directly within the 'for' statement's
> scope to keep its scope limited? For example, 'for (int i = 0; ...)'.
>
> > +
> > + ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> > + if (!ddata)
> > + return -ENOMEM;
> > +
> > + platform_set_drvdata(pdev, ddata);
> > + ddata->ec = ec;
> > +
> > + for (i = 0; i < ASUSEC_LED_MAX; i++) {
>
> Nit: for (int i = ...
>
> > + const struct asus_ec_led_config *cfg = &asus_ec_leds[i];
> > + struct asus_ec_led *led = &ddata->leds[i];
> > +
> > + led->cdev.name = devm_kasprintf(dev, GFP_KERNEL, "%s::%s",
> > + ddata->ec->name, cfg->name);
> > + if (!led->cdev.name)
> > + return -ENOMEM;
> > +
> > + led->cdev.max_brightness = 1;
> > + led->cdev.color = cfg->color;
> > + led->cdev.flags = LED_CORE_SUSPENDRESUME | LED_RETAIN_AT_SHUTDOWN;
> > + led->cdev.brightness_get = asus_ec_led_get_brightness;
> > + led->cdev.brightness_set_blocking = asus_ec_led_set_brightness;
> > +
> > + led->ddata = ddata;
> > + led->ctrl_bit = cfg->ctrl_bit;
> > +
> > + ret = devm_led_classdev_register(dev, &led->cdev);
> > + if (ret)
> > + return dev_err_probe(dev, ret,
> > + "failed to register %s LED\n",
> > + cfg->name);
>
> Should we capitalise the error message here to match our style guidelines
> (e.g. 'Failed to register...')?
>
dev messages end with ":" so it should continue with lower case. You
want cap, I don't mind
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static struct platform_driver asus_ec_led_driver = {
> > + .driver.name = "asus-transformer-ec-led",
> > + .probe = asus_ec_led_probe,
> > +};
> > +module_platform_driver(asus_ec_led_driver);
> > +
> > +MODULE_ALIAS("platform:asus-transformer-ec-led");
> > +MODULE_AUTHOR("Michał Mirosław <mirq-linux@rere.qmqm.pl>");
> > +MODULE_AUTHOR("Svyatoslav Ryhel <clamor95@gmail.com>");
> > +MODULE_DESCRIPTION("ASUS Transformer's charging LED driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.51.0
> >
> >
>
> --
> Lee Jones
^ permalink raw reply
* Re: [PATCH] HID: hiddev: Use kref to track struct hiddev lifetime
From: Heitor Alves de Siqueira @ 2026-06-11 12:36 UTC (permalink / raw)
To: Dmitry Torokhov, Heitor Alves de Siqueira
Cc: Jiri Kosina, Benjamin Tissoires, kernel-dev, linux-usb,
linux-input, linux-kernel, syzbot+563191a4939ddbfe73d4
In-Reply-To: <aib06cgHVLIb9vtl@google.com>
Hi Dmitry,
On Mon Jun 8, 2026 at 3:44 PM -03, Dmitry Torokhov wrote:
> Hi Heitor,
>
> On Mon, Jun 08, 2026 at 01:33:03PM -0300, Heitor Alves de Siqueira wrote:
>> If a USB HID device is disconnected while userspace still holds the
>> hiddev node open, hiddev_disconnect() and hiddev_release() can race on
>> the embedded existancelock mutex. Syzbot has triggered this with kfree()
>> happening during the mutex slow path.
>>
>> Fix by introducing a kref in struct hiddev, and moving kfree() into a
>> dedicated release callback. This way, struct hiddev will only be freed
>> after both hiddev_release() and hiddev_disconnect() are done.
>
> This looks like a common issue with usb_register_dev() that does not
> allow tying the lifetime of the created device, lifetime of user of the
> created device, and userspace accessing it. Ideally the class device
> would be embedded into struct hiddev, and tie its lifetime with lifetime
> of the chardev associated with it and userspace accessors using it. tie
> its lifetime with lifetime of the chardev associated with it and
> userspace accessors using it. See cdev_device_add() and how it is being
> used by multiple subsystems and how they handle class devices.
Thank you for your review! I see now, usb_register_dev() does not seem
to be the best if we want to tie the lifetimes together... At least, I
don't think its intended for this usecase, especially if we want to
embed the cdev into struct hiddev. I've been looking at some of the
other drivers that use cdev_device_add(), and noticed that we'd have to
manually handle some things that usb_register_dev() conveniently does
for us (although most seem quite straightforward like minor allocation
and device creation).
I'll give it a try for hiddev, and make sure we don't change any of the
class device paths or leave any open ends with the rest of the usbhid
subsystem. One of the USB gadget drivers already uses cdev_device_add(),
so hopefully it won't be too much trouble.
Thanks,
Heitor
^ permalink raw reply
* Re: [PATCH] HID: logitech-hidpp: sync wheel multiplier on wheel mode changes
From: Jiri Kosina @ 2026-06-11 13:56 UTC (permalink / raw)
To: Lauri Saurus
Cc: linux-input, Filipe Laíns, Bastien Nocera,
Benjamin Tissoires, linux-kernel
In-Reply-To: <20260518192649.245691-1-saurla@saurla.com>
On Mon, 18 May 2026, Lauri Saurus wrote:
> The hid-logitech-hidpp driver enables high resolution scrolling on
> device connect for capable HID++ 2.0 devices. Driver also reads the
> wheel capability and caches the returned high resolution wheel scroll
> multiplier, that is used for scroll scaling when handling wheel scroll
> events.
>
> Wheel mode can also be set externally through HID++ requests, which
> can leave the cached multiplier stale and cause incorrect scroll
> scaling. If external SetWheelMode HID++ request sets the mode to
> low resolution, the cached multiplier is not updated accordingly. This
> causes extremely slow scrolling since driver expects multiple wheel
> scroll events per detent but is only getting one.
>
> The fix listens for HID++ SetWheelMode request responses and updates
> the wheel scroll multiplier based on the set high resolution scroll
> mode. The fix has been tested with Logitech G502X lightspeed mouse.
>
> Signed-off-by: Lauri Saurus <saurla@saurla.com>
Applied, thank you.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox