* Re: [PATCH] Documentation: Fix syntax of kmalloc_objs example in coding style doc
From: Geert Uytterhoeven @ 2026-05-29 8:18 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jonathan Corbet, Manuel Ebner, Vlastimil Babka (SUSE),
SeongJae Park, Shuah Khan, workflows, linux-doc, linux-kernel
In-Reply-To: <20260529081006.2019687-2-ukleinek@kernel.org>
On Fri, 29 May 2026 at 10:13, Uwe Kleine-König <ukleinek@kernel.org> wrote:
> The first parameter should match the variable that the allocated memory
> is assigned to. Fix the example accordingly, the one for kmalloc_obj got
> it right already.
>
> Fixes: 7c6d969d5349 ("Documentation: adopt new coding style of type-aware kmalloc-family")
> Signed-off-by: Uwe Kleine-König <ukleinek@kernel.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH] Documentation: Fix syntax of kmalloc_objs example in coding style doc
From: Uwe Kleine-König @ 2026-05-29 8:10 UTC (permalink / raw)
To: Jonathan Corbet, Manuel Ebner, Vlastimil Babka (SUSE),
SeongJae Park
Cc: Shuah Khan, workflows, linux-doc, linux-kernel
The first parameter should match the variable that the allocated memory
is assigned to. Fix the example accordingly, the one for kmalloc_obj got
it right already.
Fixes: 7c6d969d5349 ("Documentation: adopt new coding style of type-aware kmalloc-family")
Signed-off-by: Uwe Kleine-König <ukleinek@kernel.org>
---
Documentation/process/coding-style.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
index a3bf75dc7c88..a8336582f60b 100644
--- a/Documentation/process/coding-style.rst
+++ b/Documentation/process/coding-style.rst
@@ -959,13 +959,13 @@ The preferred form for allocating an array is the following:
.. code-block:: c
- p = kmalloc_objs(*ptr, n, ...);
+ p = kmalloc_objs(*p, n, ...);
The preferred form for allocating a zeroed array is the following:
.. code-block:: c
- p = kzalloc_objs(*ptr, n, ...);
+ p = kzalloc_objs(*p, n, ...);
Both forms check for overflow on the allocation size n * sizeof(...),
and return NULL if that occurred.
base-commit: f7af91adc230aa99e23330ecf85bc9badd9780ad
--
2.47.3
^ permalink raw reply related
* Re: [PATCH v3 0/4] cpufreq: Set policy->min and max as real QoS constraints
From: Zhongqiu Han @ 2026-05-29 8:08 UTC (permalink / raw)
To: Pierre Gondois, linux-kernel
Cc: Jie Zhan, Lifeng Zheng, Ionela Voinescu, Sumit Gupta,
Rafael J. Wysocki, Viresh Kumar, Jonathan Corbet, Shuah Khan,
Huang Rui, Mario Limonciello, Perry Yuan, K Prateek Nayak,
Srinivas Pandruvada, Len Brown, Saravana Kannan, linux-pm,
linux-doc, zhongqiu.han
In-Reply-To: <20260528090913.2759118-1-pierre.gondois@arm.com>
On 5/28/2026 5:09 PM, Pierre Gondois wrote:
> This patch is a follow-up from the serie:
> - [PATCH v6 0/4] cpufreq: Introduce boost frequency QoS
> https://lore.kernel.org/lkml/20260317101753.2284763-1-pierre.gondois@arm.com/
>
> v3:
> - Added Reviewed-by tags
> - Return 0 on cpufreq_policy_init_qos() success instead of
> a random positive value
> - Moved policy->min/max assignment out of cpufreq_policy_init_qos()
> - Updated documentation
Sorry for the delay.
Looks good to me for patches 2/4, 3/4 and 4/4.
Reviewed-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
> v2:
> - Split the patch in multiple steps
> - Replace min/max -> min_freq/max_freq
> - Add references to commit 521223d8b3ec ("cpufreq: Fix initialization
> of min and max frequency QoS requests") to explain the intent
> of the patch
> - Update documentation (cpu-drivers.rst)
> - Set default policy->min/max values before the call to
> blocking_notifier_call_chain()
> - Create a new cpufreq_policy_init_qos() function to put all
> the QoS and policy->min/max logic inside.
> - Didn't add Sumit's reviewed-by as the patches changed a bit.
> v1:
> https://lore.kernel.org/lkml/20260423084731.1090384-1-pierre.gondois@arm.com/#t
>
> Pierre Gondois (4):
> cpufreq: Extract cpufreq_policy_init_qos() function
> cpufreq: Set default policy->min/max values for all drivers
> cpufreq: Remove driver default policy->min/max init
> cpufreq: Use policy->min/max init as QoS request
>
> Documentation/cpu-freq/cpu-drivers.rst | 9 +++-
> drivers/cpufreq/amd-pstate.c | 14 +++---
> drivers/cpufreq/cppc_cpufreq.c | 5 +-
> drivers/cpufreq/cpufreq-nforce2.c | 4 +-
> drivers/cpufreq/cpufreq.c | 68 ++++++++++++++++++--------
> drivers/cpufreq/freq_table.c | 7 ++-
> drivers/cpufreq/gx-suspmod.c | 2 +-
> drivers/cpufreq/intel_pstate.c | 3 --
> drivers/cpufreq/pcc-cpufreq.c | 10 ++--
> drivers/cpufreq/pxa3xx-cpufreq.c | 5 +-
> drivers/cpufreq/sh-cpufreq.c | 6 +--
> drivers/cpufreq/virtual-cpufreq.c | 5 +-
> 12 files changed, 77 insertions(+), 61 deletions(-)
>
> --
> 2.43.0
--
Thx and BRs,
Zhongqiu Han
^ permalink raw reply
* Re: [PATCH net-next] Documentation: networking: Add a test plan for ethtool pause validation
From: Maxime Chevallier @ 2026-05-29 8:07 UTC (permalink / raw)
To: Andrew Lunn
Cc: Jakub Kicinski, davem, Eric Dumazet, Paolo Abeni, Simon Horman,
Russell King, Heiner Kallweit, Jonathan Corbet, Shuah Khan,
Oleksij Rempel, Vladimir Oltean, Florian Fainelli,
thomas.petazzoni, netdev, linux-kernel, linux-doc
In-Reply-To: <2293244a-c6a9-4642-a721-dada8a081dbc@lunn.ch>
On 5/28/26 03:15, Andrew Lunn wrote:
>> +A.1 : Sanity Checks
>> +~~~~~~~~~~~~~~~~~~~
>> +
>> +Pause autoneg is set to off::
>> +
>> + ethtool -A <iface> autoneg off
>> +
>> +The 'supported' fields retrieved using the ETHTOOL_MSG_LINKMODES_GET includes
>> +a "Pause" bit and an "Asym" bit.
>> +
>> +The ETHTOOL_MSG_PAUSE_GET command returns the currently configured pause modes
>> +in the "tx" and "rx" attributes.
>
> What you have left unspecified here is the state a link autoneg,
>
> ethtool -s <iface> autoneg on|off
>
> At minimum, it should be specified. I've not read the other tests yet,
> but we may also want to run this basic test with both possible
> setting.
True ! With a more formalized test description, it should be easier to
get non-ambiguous starting conditions here.
>
> One possible bug which we are trying to detect is that pause autoneg
> is off, the values are forced, but -s autoneg is on, it completes, and
> overwrites the forces values with negotiated values.
I'm trying to cover that on the latter tests indeed
>
>
>> +A.2 : Half-duplex operation
>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> +
>> +Pause settings as exposed with the ethtool API only concern full-duplex modes.
>> +
>> +Test scenario:
>> +--------------
>> +
>> +Set the interface under test in half-duplex mode with::
>> +
>> + ethtool -s <iface> duplex half
>> +
>> +Expected behaviour::
>> +
>> + - ethtool <iface>
>> +
>> +shows no Pause settings advertised.
>
> Since we are talking about advertisement of pause, i think you actually want
>
> ethtool -A <iface> autoneg on
> ethtool -s <iface> duplex half autoneg on
>
> If -s autoneg is off, nothing should be advertised. So we need that
> turned on. And if -A autoneg is off, no pause values should be
> advertised, so you need that turned on as well.
>
> Also, since we expect this to trigger an autoneg, we probably want a
> sleep(2) in there before looking at the results, to ensure autoneg has
> completed.
>
> There is also the possibility that the link does not come up, because
> the link peer does not support half duplex. This is quite common for
> 1G interfaces. It could also be the local interface does not support
> 1G half. So maybe
>
> ethtool -s <iface> duplex half speed 100 autoneg on
I think that
ethtool -s <iface> duplex half autoneg on
should be enough, the link should still establish at 100M, I've tested
that on a 1G/FULL 100MHalf+Full interface and this is the result we
get :)
That said I've tested the following on mcbin, and it seems that acually
nothing in the code currently deals with Half duplex / Pause
interaction, and we don't get any EOPNOTSUPP.
So the broader question is, should we reflect the current behaviour or
an ideal one ?
And if we reflect the current one, the canonical behaviour is probably
the phylink one ?
>
> Or let it first autoneg unrestricted, look at both the local and LP
> values, and pick a half duplex link mode both support and set it to do
> that?
>
> The tests defined so far also don't cover all the possible settings of
> -A autoneg and -s autoneg. There are combinations:
>
> Link autoneg with pause autoneg
> Link autoneg with forced pause
> Force link, with forced pause
>
> It would be good to consider what can be tested for these three. Maybe
> nothing can be tested with forced link, because that it likely to
> result in no link, when only the local side can be configured.
>
>> +B : Combined devices testing
>> +============================
>> +
>> +Requirements : The interface under test must be connected to a link-partner that
>> +can be actively configured during the tests. It must at least support full-duplex
>> +modes, and optionally (but ideally) symmetric and asymmetric flow-control, as
>> +well as autonegotiation of the Pause parameters.
>
> and forced link parameters.
>
>> +
>> +B.1 : Autoneg advertising
>> +~~~~~~~~~~~~~~~~~~~~~~~~~
>> +
>> +Goal: Validate that the *advertised* Pause and AsymDir bits match the configured
>> +pausemarams.
>
> typo.
>
>> +
>> +The link-level autonegotiation must be enabled::
>> +
>> + ethtool <iface> autoneg on
>> +
>> +Pause parameters are set with::
>> +
>> + ethtool -A <iface> rx <val> tx <val> autoneg on
>> +
>> +Pause advertising is retrieved with::
>> +
>> + ethtool <iface>
>> +
>> +Case 1
>> +------
>> +
>> +Pause parameters : rx **off** tx **off**
>> +
>> +Expected advertisement : **None** (Pause = 0, AsymDir = 0)
>> +
>> +Case 2
>> +------
>> +
>> +Pause parameters : rx **off** tx **on**
>> +
>> +Expected advertisement : **Transmit-only** (Pause = 0, AsymDir = 1)
>> +
>> +Case 3
>> +------
>> +
>> +Pause parameters : rx **on** tx **off**
>> +
>> +Expected advertisement : **Symmetric receive-only** (Pause = 1, Asymdir = 1)
>> +
>> +Case 4
>> +------
>> +
>> +Pause parameters : rx **on** tx **on**
>> +
>> +Expected advertisement : **Symmetric** (Pause = 1, Asymdir = 0)
>
> We should consider here what happens when the local side only supports
> symmetric pause. We would expect EOPNOTSUPP, or maybe EINVAL. If
> ethtool report:
>
> Supported pause frame use: Symmetric Receive-only
>
> not getting an error for the asymmetric settings would be a bug.
Ack, I think all test cases will need to account for the local device
support
>
>> +
>> +B.2 : Autoneg resolution
>> +~~~~~~~~~~~~~~~~~~~~~~~~
>> +
>> +Goal: Validate that the Pause and AsymDir negotiation translates to the right
>> +TX and RX pause parameters.
>> +
>> +The following table, from the 802.3 standard, exposes the autoneg resolution
>> +result for the advertised pause parameters by each link partner.
>> +
>> ++-------------+--------------+--------------------------+
>> +|Local device | Link partner | Pause settings resolution|
>> ++------+------+-------+------+-----------+--------------+
>> +|Pause | Asym | Pause | Asym | RX | TX |
>
> There is a tab vs space issue here.
>
>
>> ++======+======+=======+======+===========+==============+
>> +| 0 | 0 | Any | Any | No | No |
>> ++------+------+-------+------+-----------+--------------+
>> +| 0 | 1 | 0 | Any | No | No |
>> ++------+------+-------+------+-----------+--------------+
>> +| 0 | 1 | 1 | 0 | No | No |
>> ++------+------+-------+------+-----------+--------------+
>> +| 0 | 1 | 1 | 1 | No | Yes |
>> ++------+------+-------+------+-----------+--------------+
>> +| 1 | 0 | 0 | Any | No | No |
>> ++------+------+-------+------+-----------+--------------+
>> +| 1 | Any | 1 | Any | Yes | Yes |
>> ++------+------+-------+------+-----------+--------------+
>> +| 1 | 1 | 0 | 0 | No | No |
>> ++------+------+-------+------+-----------+--------------+
>> +| 1 | 1 | 0 | 1 | Yes | No |
>> ++------+------+-------+------+-----------+--------------+
>> +
>> +The mapping between the configured pause parameters and advertised modes follow
>> +the following truth table :
>> +
>> ++----+----+-------+---------+
>> +| tx | rx | Pause | AsymDir |
>> ++====+====+=======+=========+
>> +| 0 | 0 | 0 | 0 |
>> ++----+----+-------+---------+
>> +| 0 | 1 | 1 | 1 |
>> ++----+----+-------+---------+
>> +| 1 | 0 | 0 | 1 |
>> ++----+----+-------+---------+
>> +| 1 | 1 | 1 | 0 |
>> ++----+----+-------+---------+
>> +
>> +We can boil that down to the following cases to test, keeping the number small
>> +to avoid dealing with the whole combinatory::
>
> Why not do the whole set of combination? There are 16 combinations,
> autoneg takes a little over 1 second, so we are probably talking 32
> seconds in total. That is a reasonable runtime for a test.
True, I'll describe the whole set then :)
>
>> +Case 1
>> +------
>> +
>> +Local device : rx **off**, tx **off**
>> +Remote device : rx **on** tx **on**
>> +
>> +Expected result on local device after autonegotiation completes :
>> + rx negotiated **off**
>> + tx negotiated **off**
>> +
>
> ...
>
>> +Case 7
>> +------
>> +
>> +Local device : rx **on** tx **on**
>> +Remote device : rx **off** tx **off**
>> +
>> +Expected result on local device after autonegotiation completes :
>> + rx negotiated **off**
>> + tx negotiated **off**
>> +
>> +Case 8
>> +------
>> +
>> +Local device : rx **on** tx **on**
>> +Remote device : rx **off** tx **on**
>> +
>> +Expected result on local device after autonegotiation completes :
>> + rx negotiated **on**
>> + tx negotiated **off**
>
> what also needs to be considered here is:
>
> What if the local side only supports symmetric pause?
> What if the LP only supports symmetric pause?
>
> The expect results should take this into account, that the
> configuration fails, but that is not a test failure, just a hardware
> limitation.
>
>> +
>> +B.3 : Pause Autoneg
>> +~~~~~~~~~~~~~~~~~~~
>> +
>> +Goal: Validate that the Pause autonegotiation flag correctly toggles the
>> +advertised Pause and AsymDir link parameters.
>> +
>> +Test scenario:
>> +--------------
>> +
>> + - Enable pause autoneg and at least rx or tx pause::
>> +
>> + ethtool -A <iface> rx on tx on autoneg on
>> +
>> + - Check the Advertised pause frame use::
>> +
>> + ethtool <iface>
>> +
>> + ...
>> + Advertised pause frame use: Symmetric Receive-only
>> +
>> + - Disable pause autoneg::
>> +
>> + ethtool -A <iface> autoneg off
>> +
>> + - Check the Advertised pause frame use, which must be 'No'::
>> +
>> + ethtool <iface>
>> +
>> + ...
>> + Advertised pause frame use: No
>
> Please describe configuration for both sides. This is needed for all
> the tests when there are two devices involved.
>
> Also, when local pause advertisement is turned off, check what the
> link partner is reporting it received from its link partner.
Very well, I'll add this.
Thanks for the extensive feedback Andrew,
Maxime
^ permalink raw reply
* Re: [PATCH net-next] Documentation: networking: Add a test plan for ethtool pause validation
From: Oleksij Rempel @ 2026-05-29 7:50 UTC (permalink / raw)
To: Maxime Chevallier
Cc: Jakub Kicinski, Andrew Lunn, davem, Eric Dumazet, Paolo Abeni,
Simon Horman, Russell King, Heiner Kallweit, Jonathan Corbet,
Shuah Khan, Vladimir Oltean, Florian Fainelli, thomas.petazzoni,
netdev, linux-kernel, linux-doc
In-Reply-To: <b84a5fe0-a080-45fc-87d1-790402d423e4@bootlin.com>
Hi,
On Fri, May 29, 2026 at 09:42:14AM +0200, Maxime Chevallier wrote:
> Hi
>
> On 5/28/26 01:25, Jakub Kicinski wrote:
> > On Wed, 27 May 2026 04:47:47 +0200 Andrew Lunn wrote:
> > > > It'd be great to hear from others but IMHO in the current form this is
> > > > not suitable for Documentation/networking/ We can commit the "knowledge"
> > > > part but enumerating the test cases seems odd for Documentation/.
> > >
> > > Sorry, not looked too deeply at the actual content yet.
> > >
> > > What i was thinking was a python file, which sphinx can ingest to
> > > produce documentation, and place holders were code would be added to
> > > implement the actual test during the next phase.
> > >
> > > This is how i've done testing in the past. I would be the evil one who
> > > thought up the tests and described them in detail using sphinx markup
> > > in a python test template file. After some review they got passed off
> > > to a python developer for implementation. And when they got run and
> > > failed, sometimes the feature developer, the test developer and myself
> > > got together to figure who made the error.
> > >
> > > I'm not sure we even need sphinx. What i find important is that the
> > > test is documented. What kAPI calls should be made with what
> > > parameters. What results we are expected and why? So that when a test
> > > fails, a developer has the information they need to fix their
> > > code. The Why? is important, and often missing from the kernel tests.
> >
> > All makes sense. The question is primarily how we fit that into
> > the existing project layout we have in the kernel :(
> >
> > The python tests can be hacked up to print the test case docstring
> > before the failure.
> >
> > But I think for human and AI reviewer consumption it may be nice
> > to keep the condensed knowledge / common mistakes in Documentation/
> > If we had the ability to exercise the submissions it'd be a different
> > story test output would be a sufficient signal and/or could be fed into
> > the review. But for AI making a guess at whether the submitted driver is
> > correct purely from the driver source - knowledge is useful.
>
> Also for the knowledge part, we had Oleksij's series that documented
> more aspects of flow control (not just the pause part) :
>
> https://lore.kernel.org/netdev/20260304094811.2779953-1-o.rempel@pengutronix.de/
>
> Oleksij, maybe we can merge some of the information here with your
> doc for the Docuentation/networking part ?
I have nothing against it. We need a documentation for proper testing
and implementations too :)
Best Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH v3 0/4] cpufreq: Set policy->min and max as real QoS constraints
From: Viresh Kumar @ 2026-05-29 7:45 UTC (permalink / raw)
To: Pierre Gondois
Cc: linux-kernel, Jie Zhan, Lifeng Zheng, Ionela Voinescu,
Sumit Gupta, Zhongqiu Han, Rafael J. Wysocki, Jonathan Corbet,
Shuah Khan, Huang Rui, Mario Limonciello, Perry Yuan,
K Prateek Nayak, Srinivas Pandruvada, Len Brown, Saravana Kannan,
linux-pm, linux-doc
In-Reply-To: <20260528090913.2759118-1-pierre.gondois@arm.com>
On 28-05-26, 11:09, Pierre Gondois wrote:
> This patch is a follow-up from the serie:
> - [PATCH v6 0/4] cpufreq: Introduce boost frequency QoS
> https://lore.kernel.org/lkml/20260317101753.2284763-1-pierre.gondois@arm.com/
>
> v3:
> - Added Reviewed-by tags
> - Return 0 on cpufreq_policy_init_qos() success instead of
> a random positive value
> - Moved policy->min/max assignment out of cpufreq_policy_init_qos()
> - Updated documentation
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
* Re: [PATCH net-next] Documentation: networking: Add a test plan for ethtool pause validation
From: Maxime Chevallier @ 2026-05-29 7:42 UTC (permalink / raw)
To: Jakub Kicinski, Andrew Lunn
Cc: davem, Eric Dumazet, Paolo Abeni, Simon Horman, Russell King,
Heiner Kallweit, Jonathan Corbet, Shuah Khan, Oleksij Rempel,
Vladimir Oltean, Florian Fainelli, thomas.petazzoni, netdev,
linux-kernel, linux-doc
In-Reply-To: <20260527162516.0d012503@kernel.org>
Hi
On 5/28/26 01:25, Jakub Kicinski wrote:
> On Wed, 27 May 2026 04:47:47 +0200 Andrew Lunn wrote:
>>> It'd be great to hear from others but IMHO in the current form this is
>>> not suitable for Documentation/networking/ We can commit the "knowledge"
>>> part but enumerating the test cases seems odd for Documentation/.
>>
>> Sorry, not looked too deeply at the actual content yet.
>>
>> What i was thinking was a python file, which sphinx can ingest to
>> produce documentation, and place holders were code would be added to
>> implement the actual test during the next phase.
>>
>> This is how i've done testing in the past. I would be the evil one who
>> thought up the tests and described them in detail using sphinx markup
>> in a python test template file. After some review they got passed off
>> to a python developer for implementation. And when they got run and
>> failed, sometimes the feature developer, the test developer and myself
>> got together to figure who made the error.
>>
>> I'm not sure we even need sphinx. What i find important is that the
>> test is documented. What kAPI calls should be made with what
>> parameters. What results we are expected and why? So that when a test
>> fails, a developer has the information they need to fix their
>> code. The Why? is important, and often missing from the kernel tests.
>
> All makes sense. The question is primarily how we fit that into
> the existing project layout we have in the kernel :(
>
> The python tests can be hacked up to print the test case docstring
> before the failure.
>
> But I think for human and AI reviewer consumption it may be nice
> to keep the condensed knowledge / common mistakes in Documentation/
> If we had the ability to exercise the submissions it'd be a different
> story test output would be a sufficient signal and/or could be fed into
> the review. But for AI making a guess at whether the submitted driver is
> correct purely from the driver source - knowledge is useful.
Also for the knowledge part, we had Oleksij's series that documented
more aspects of flow control (not just the pause part) :
https://lore.kernel.org/netdev/20260304094811.2779953-1-o.rempel@pengutronix.de/
Oleksij, maybe we can merge some of the information here with your
doc for the Docuentation/networking part ?
Maxime
^ permalink raw reply
* Re: [PATCH v3] cgroup/dmem: introduce a peak file
From: Maarten Lankhorst @ 2026-05-29 7:34 UTC (permalink / raw)
To: Michal Koutný, Thadeu Lima de Souza Cascardo
Cc: Tejun Heo, Johannes Weiner, Michal Hocko, Roman Gushchin,
Shakeel Butt, Muchun Song, Andrew Morton, Jonathan Corbet,
Shuah Khan, Maxime Ripard, Natalie Vock, Tvrtko Ursulin, cgroups,
linux-kernel, linux-mm, linux-doc, dri-devel, kernel-dev
In-Reply-To: <ahCISfTlN10gD8e6@localhost.localdomain>
Den 2026-05-22 kl. 18:48, skrev Michal Koutný:
> On Thu, May 14, 2026 at 02:36:08PM -0300, Thadeu Lima de Souza Cascardo <cascardo@igalia.com> wrote:
>> Just like we have memory.peak, introduce a dmem.peak, which uses the
>> page_counter support for that.
>>
>> For now, make it read-only.
>>
>> This allows for memory usage monitoring without polling dmem.current when
>> the information needed is the maximum device memory used. That can be used
>> for capacity planning, such that dmem.max can be properly setup for a given
>> workload. It can also be used for debugging to determine whether a given
>> workload would have caused eviction or system memory use.
>>
>> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
>> ---
>> Changes in v3:
>> - EDITME: describe what is new in this series revision.
>> - EDITME: use bulletpoints and terse descriptions.
>> - Link to v2: https://patch.msgid.link/20260513-dmem_peak-v2-1-dac06999db9e@igalia.com
>>
>> Changes in v2:
>> - Make it read-only for now and adjust documentation accordingly.
>> - Link to v1: https://patch.msgid.link/20260506-dmem_peak-v1-0-8d803eb3449c@igalia.com
>> ---
>> Documentation/admin-guide/cgroup-v2.rst | 6 ++++++
>> kernel/cgroup/dmem.c | 15 +++++++++++++++
>> 2 files changed, 21 insertions(+)
>>
>> diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
>> index 6efd0095ed99..d103623b2be4 100644
>> --- a/Documentation/admin-guide/cgroup-v2.rst
>> +++ b/Documentation/admin-guide/cgroup-v2.rst
>> @@ -2808,6 +2808,12 @@ DMEM Interface Files
>> The semantics are the same as for the memory cgroup controller, and are
>> calculated in the same way.
>>
>> + dmem.peak
>> + A read-only nested-keyed file that exists on non-root cgroups.
>
> s/nested-keyed/flat-keyed/
>
>
> With that
>
> Reviewed-by: Michal Koutný <mkoutny@suse.com>
Reviewed-by: Maarten Lankhorst <dev@lankhorst.se>
With your r-b it's ok to push it to the dmemcg tree?
Kind regards,
~Maarten Lankhorst
^ permalink raw reply
* [wireless-next:main 10/20] htmldocs: Documentation/networking/checksum-offloads:157: ./include/linux/skbuff.h:181: WARNING: Failed to create a cross reference. A title or caption not found: 'crc' [ref.ref]
From: kernel test robot @ 2026-05-29 7:30 UTC (permalink / raw)
To: Jakub Kicinski
Cc: oe-kbuild-all, Johannes Berg, linux-wireless, Willem de Bruijn,
linux-doc
tree: https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
head: e7d6bd24e883bf7c328d73c99bf6bcde19bf5e61
commit: 25bfb3a8edcbadf4d9d27450c547a631e7513f14 [10/20] docs: net: render the checksum comment in checksum-offloads.rst
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
docutils: docutils (Docutils 0.21.2, Python 3.13.5, on linux)
reproduce: (https://download.01.org/0day-ci/archive/20260529/202605290940.E4u11iiQ-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/202605290940.E4u11iiQ-lkp@intel.com/
All warnings (new ones prefixed by >>):
Documentation/userspace-api/landlock:550: ./include/uapi/linux/landlock.h:45: ERROR: Unknown target name: "network flags". [docutils]
Documentation/userspace-api/landlock:550: ./include/uapi/linux/landlock.h:50: ERROR: Unknown target name: "scope flags". [docutils]
Documentation/userspace-api/landlock:550: ./include/uapi/linux/landlock.h:24: ERROR: Unknown target name: "filesystem flags". [docutils]
Documentation/userspace-api/landlock:559: ./include/uapi/linux/landlock.h:168: ERROR: Unknown target name: "filesystem flags". [docutils]
Documentation/userspace-api/landlock:559: ./include/uapi/linux/landlock.h:191: ERROR: Unknown target name: "network flags". [docutils]
>> Documentation/networking/checksum-offloads:157: ./include/linux/skbuff.h:181: WARNING: Failed to create a cross reference. A title or caption not found: 'crc' [ref.ref]
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v5 08/18] mm: add VM_UFFD_RWP VMA flag
From: Lorenzo Stoakes @ 2026-05-29 7:24 UTC (permalink / raw)
To: Kiryl Shutsemau
Cc: akpm, rppt, peterx, david, surenb, vbabka, Liam.Howlett, ziy,
corbet, skhan, seanjc, pbonzini, jthoughton, aarcange, sj,
usama.arif, linux-mm, linux-kernel, linux-doc, linux-kselftest,
kvm, kernel-team, Kiryl Shutsemau (Meta)
In-Reply-To: <20260526130509.2748441-9-kirill@shutemov.name>
On Tue, May 26, 2026 at 02:04:56PM +0100, Kiryl Shutsemau wrote:
> From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>
>
> Preparatory patch for userfaultfd read-write protection (RWP). RWP
> extends userfaultfd protection from plain write-protection (WP) to
> full read-write protection: accesses to an RWP-protected range --
> reads as well as writes -- trap through userfaultfd.
>
> Reserve VM_UFFD_RWP, add the userfaultfd_rwp() and
> userfaultfd_protected() helpers, and wire up the smaps "ur" entry and
> the trace-flag table the rest of the series will use. The flag is
> gated on CONFIG_USERFAULTFD_RWP, which is introduced together with the
> UAPI in a later patch; until then VM_UFFD_RWP aliases VM_NONE and
> every downstream check folds to dead code.
>
> Nothing sets or queries the flag yet.
>
> Signed-off-by: Kiryl Shutsemau <kas@kernel.org>
> Assisted-by: Claude:claude-opus-4-6
Hm, if you've just used claude to bounce ideas off, I'm really not sure if
it's necessary to disclose, though I respect your thoroughness for doing so
:)
I guess determining the threshold at which it makes sense to do so is still
a WIP for us in the kernel.
> Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> Reviewed-by: SeongJae Park <sj@kernel.org>
> ---
> Documentation/filesystems/proc.rst | 1 +
> fs/proc/task_mmu.c | 3 +++
> include/linux/mm.h | 28 +++++++++++++++++----------
> include/linux/userfaultfd_k.h | 31 +++++++++++++++++++++++++-----
> include/trace/events/mmflags.h | 7 +++++++
> 5 files changed, 55 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> index db6167befb7b..db28207c5290 100644
> --- a/Documentation/filesystems/proc.rst
> +++ b/Documentation/filesystems/proc.rst
> @@ -607,6 +607,7 @@ encoded manner. The codes are the following:
> um userfaultfd missing tracking
> uw userfaultfd wr-protect tracking
> ui userfaultfd minor fault
> + ur userfaultfd read-write-protect tracking
> ss shadow/guarded control stack page
> sl sealed
> lf lock on fault pages
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 1e5f6ee8a3b6..974c5f4aa533 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -1237,6 +1237,9 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
> #ifdef CONFIG_HAVE_ARCH_USERFAULTFD_MINOR
> [ilog2(VM_UFFD_MINOR)] = "ui",
> #endif /* CONFIG_HAVE_ARCH_USERFAULTFD_MINOR */
> +#ifdef CONFIG_USERFAULTFD_RWP
> + [ilog2(VM_UFFD_RWP)] = "ur",
> +#endif
> #ifdef CONFIG_ARCH_HAS_USER_SHADOW_STACK
> [ilog2(VM_SHADOW_STACK)] = "ss",
> #endif
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 71b11945e4fc..6499cfb61dc4 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -362,6 +362,7 @@ enum {
> #endif
> DECLARE_VMA_BIT(UFFD_MINOR, 41),
> DECLARE_VMA_BIT(SEALED, 42),
> + DECLARE_VMA_BIT(UFFD_RWP, 43),
I'm guessing CONFIG_USERFAULTFD_RWP is predicated on CONFIG_64BIT?
It's a silly situation and once my VMA flags stuff is done it'll be
eliminated but for now... :)
> /* Flags that reuse flags above. */
> DECLARE_VMA_BIT_ALIAS(PKEY_BIT0, HIGH_ARCH_0),
> DECLARE_VMA_BIT_ALIAS(PKEY_BIT1, HIGH_ARCH_1),
> @@ -505,6 +506,11 @@ enum {
> #else
> #define VM_UFFD_MINOR VM_NONE
> #endif
> +#ifdef CONFIG_USERFAULTFD_RWP
> +#define VM_UFFD_RWP INIT_VM_FLAG(UFFD_RWP)
> +#else
> +#define VM_UFFD_RWP VM_NONE
> +#endif
> #ifdef CONFIG_64BIT
> #define VM_ALLOW_ANY_UNCACHED INIT_VM_FLAG(ALLOW_ANY_UNCACHED)
> #define VM_SEALED INIT_VM_FLAG(SEALED)
> @@ -642,22 +648,24 @@ enum {
> * reconsistuted upon page fault, so necessitate page table copying upon fork.
> *
> * Note that these flags should be compared with the DESTINATION VMA not the
> - * source, as VM_UFFD_WP may not be propagated to destination, while all other
> - * flags will be.
> + * source: VM_UFFD_WP and VM_UFFD_RWP may be cleared on the destination
> + * (dup_userfaultfd() -> userfaultfd_reset_ctx() when the parent context did
> + * not negotiate UFFD_FEATURE_EVENT_FORK), while all other flags propagate.
> *
> * VM_PFNMAP / VM_MIXEDMAP - These contain kernel-mapped data which cannot be
> * reasonably reconstructed on page fault.
> *
> * VM_UFFD_WP - Encodes metadata about an installed uffd
> - * write protect handler, which cannot be
> - * reconstructed on page fault.
> + * VM_UFFD_RWP write- or read-write-protect handler, which
> + * cannot be reconstructed on page fault.
> *
> - * We always copy pgtables when dst_vma has uffd-wp
> - * enabled even if it's file-backed
> - * (e.g. shmem). Because when uffd-wp is enabled,
> - * pgtable contains uffd-wp protection information,
> - * that's something we can't retrieve from page cache,
> - * and skip copying will lose those info.
> + * We always copy pgtables when dst_vma has the
> + * uffd PTE bit in use even if it's file-backed
> + * (e.g. shmem). Because when the uffd bit is
> + * in use, the pgtable contains the protection
> + * information, that's something we can't
> + * retrieve from page cache, and skip copying
> + * will lose those info.
> *
> * VM_MAYBE_GUARD - Could contain page guard region markers which
> * by design are a property of the page tables
> diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
> index f4cf5763f92c..0aef628514df 100644
> --- a/include/linux/userfaultfd_k.h
> +++ b/include/linux/userfaultfd_k.h
> @@ -21,10 +21,11 @@
> #include <linux/hugetlb_inline.h>
>
> /* The set of all possible UFFD-related VM flags. */
> -#define __VM_UFFD_FLAGS (VM_UFFD_MISSING | VM_UFFD_WP | VM_UFFD_MINOR)
> +#define __VM_UFFD_FLAGS (VM_UFFD_MISSING | VM_UFFD_MINOR | \
> + VM_UFFD_WP | VM_UFFD_RWP)
>
> #define __VMA_UFFD_FLAGS mk_vma_flags(VMA_UFFD_MISSING_BIT, VMA_UFFD_WP_BIT, \
> - VMA_UFFD_MINOR_BIT)
> + VMA_UFFD_MINOR_BIT, VMA_UFFD_RWP_BIT)
>
> /*
> * CAREFUL: Check include/uapi/asm-generic/fcntl.h when defining
> @@ -178,7 +179,7 @@ static inline bool is_mergeable_vm_userfaultfd_ctx(struct vm_area_struct *vma,
> */
> static inline bool uffd_disable_huge_pmd_share(struct vm_area_struct *vma)
> {
> - return vma->vm_flags & (VM_UFFD_WP | VM_UFFD_MINOR);
> + return vma->vm_flags & (VM_UFFD_MINOR | VM_UFFD_WP | VM_UFFD_RWP);
While we're here we might as well switch to using the new API?
Can do:
return vma_test_any_mask(vma, __VMA_UFFD_FLAGS);
One unfortunate thing is using bit values means we can't do the VM_NONE
trick, but if !CONFIG_USERFAULTFD_RWP then VMA_UFFD_RWP_BIT wouldn't be set
anyway, same for minor so this should be fine?
> }
>
> /*
> @@ -208,6 +209,16 @@ static inline bool userfaultfd_minor(struct vm_area_struct *vma)
> return vma->vm_flags & VM_UFFD_MINOR;
> }
>
> +static inline bool userfaultfd_rwp(struct vm_area_struct *vma)
> +{
> + return vma->vm_flags & VM_UFFD_RWP;
> +}
Can be:
return vma_test(vma, VMA_UFFD_RWP_BIT);
> +
> +static inline bool userfaultfd_protected(struct vm_area_struct *vma)
> +{
> + return userfaultfd_wp(vma) || userfaultfd_rwp(vma);
> +}
> +
> static inline bool userfaultfd_pte_wp(struct vm_area_struct *vma,
> pte_t pte)
> {
> @@ -328,6 +339,16 @@ static inline bool userfaultfd_minor(struct vm_area_struct *vma)
> return false;
> }
>
> +static inline bool userfaultfd_rwp(struct vm_area_struct *vma)
> +{
> + return false;
> +}
> +
> +static inline bool userfaultfd_protected(struct vm_area_struct *vma)
> +{
> + return false;
> +}
> +
> static inline bool userfaultfd_pte_wp(struct vm_area_struct *vma,
> pte_t pte)
> {
> @@ -421,8 +442,8 @@ static inline bool userfaultfd_wp_use_markers(struct vm_area_struct *vma)
> }
>
> /*
> - * Returns true if this is a swap pte and was uffd-wp wr-protected in either
> - * forms (pte marker or a normal swap pte), false otherwise.
> + * Returns true if this swap pte carries uffd-tracked state in either
> + * form (pte marker or a normal swap pte), false otherwise.
> */
> static inline bool pte_swp_uffd_any(pte_t pte)
> {
> diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h
> index a6e5a44c9b42..bfface3d0203 100644
> --- a/include/trace/events/mmflags.h
> +++ b/include/trace/events/mmflags.h
> @@ -194,6 +194,12 @@ IF_HAVE_PG_ARCH_3(arch_3)
> # define IF_HAVE_UFFD_MINOR(flag, name)
> #endif
>
> +#ifdef CONFIG_USERFAULTFD_RWP
> +# define IF_HAVE_UFFD_RWP(flag, name) {flag, name},
> +#else
> +# define IF_HAVE_UFFD_RWP(flag, name)
> +#endif
> +
> #if defined(CONFIG_64BIT) || defined(CONFIG_PPC32)
> # define IF_HAVE_VM_DROPPABLE(flag, name) {flag, name},
> #else
> @@ -215,6 +221,7 @@ IF_HAVE_UFFD_MINOR(VM_UFFD_MINOR, "uffd_minor" ) \
> {VM_PFNMAP, "pfnmap" }, \
> {VM_MAYBE_GUARD, "maybe_guard" }, \
> {VM_UFFD_WP, "uffd_wp" }, \
> +IF_HAVE_UFFD_RWP(VM_UFFD_RWP, "uffd_rwp" ) \
> {VM_LOCKED, "locked" }, \
> {VM_IO, "io" }, \
> {VM_SEQ_READ, "seqread" }, \
> --
> 2.54.0
>
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH net-next] Documentation: networking: Add a test plan for ethtool pause validation
From: Maxime Chevallier @ 2026-05-29 7:24 UTC (permalink / raw)
To: Jakub Kicinski, Andrew Lunn
Cc: davem, Eric Dumazet, Paolo Abeni, Simon Horman, Russell King,
Heiner Kallweit, Jonathan Corbet, Shuah Khan, Oleksij Rempel,
Vladimir Oltean, Florian Fainelli, thomas.petazzoni, netdev,
linux-kernel, linux-doc
In-Reply-To: <20260527162516.0d012503@kernel.org>
Hi,
On 5/28/26 01:25, Jakub Kicinski wrote:
> On Wed, 27 May 2026 04:47:47 +0200 Andrew Lunn wrote:
>>> It'd be great to hear from others but IMHO in the current form this is
>>> not suitable for Documentation/networking/ We can commit the "knowledge"
>>> part but enumerating the test cases seems odd for Documentation/.
>>
>> Sorry, not looked too deeply at the actual content yet.
>>
>> What i was thinking was a python file, which sphinx can ingest to
>> produce documentation, and place holders were code would be added to
>> implement the actual test during the next phase.
>>
>> This is how i've done testing in the past. I would be the evil one who
>> thought up the tests and described them in detail using sphinx markup
>> in a python test template file. After some review they got passed off
>> to a python developer for implementation. And when they got run and
>> failed, sometimes the feature developer, the test developer and myself
>> got together to figure who made the error.
>>
>> I'm not sure we even need sphinx. What i find important is that the
>> test is documented. What kAPI calls should be made with what
>> parameters. What results we are expected and why? So that when a test
>> fails, a developer has the information they need to fix their
>> code. The Why? is important, and often missing from the kernel tests.
>
> All makes sense. The question is primarily how we fit that into
> the existing project layout we have in the kernel :(
>
> The python tests can be hacked up to print the test case docstring
> before the failure.
>
> But I think for human and AI reviewer consumption it may be nice
> to keep the condensed knowledge / common mistakes in Documentation/
> If we had the ability to exercise the submissions it'd be a different
> story test output would be a sufficient signal and/or could be fed into
> the review. But for AI making a guess at whether the submitted driver is
> correct purely from the driver source - knowledge is useful.
Ok so let's split this then, on one hand the knowledge part, and on the
other hand the test definitions.
From what I get from Andrew and you, the test definitions is lacking in
details w.r.t the start conditions, the return code handling as well.
Having that in the doc is going to be too verbose, but in some python
test that could really be great.
I'll use that as a basis for V2
Maxime
^ permalink raw reply
* [PATCH v11 4/4] HID: hid-msi: Add Rumble Intensity Attributes
From: Derek J. Clark @ 2026-05-29 7:21 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260529072111.7565-1-derekjohn.clark@gmail.com>
Adds intensity adjustment for the left and right rumble motors.
Claude was used during the reverse-engineering data gathering for this
feature done by Zhouwang Huang. As the code had already been affected,
I used Claude to create the initial framing for the feature, then did
manual cleanup of the _show and _store functions afterwards to fix bugs
and keep the coding style consistent. Claude was also used as an initial
reviewer of this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v11:
- Restore dropped changes from v10.
v7:
- Match on write address for rumble reports to prevent late ACK
from causing synchronization errors.
- Use spinlock for read/write profile_pending.
- Use smp_[store_release|load_acquire] pattern for checking
gamepad_registered to avoid possible races during teardown.
- Use struct for rumble reports.
v6:
- Make all timeouts 25ms to ensure at least 2 jiffies in a 100Hz
config.
- Add spinlock_irqsave for read/write access on rumble_intensity
variables.
- Gate all attribute show/store functions with gamepad_registered.
v5:
- Remove mkey related changes.
v2:
- Use pending_profile and sync to rom mutexes.
---
drivers/hid/hid-msi.c | 202 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 201 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 96710d80543d4..f5c54cd62f3bb 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -79,6 +79,8 @@ enum claw_profile_ack_pending {
CLAW_M1_PENDING,
CLAW_M2_PENDING,
CLAW_RGB_PENDING,
+ CLAW_RUMBLE_LEFT_PENDING,
+ CLAW_RUMBLE_RIGHT_PENDING,
};
enum claw_key_index {
@@ -266,6 +268,11 @@ static const u16 button_mapping_addr_new[] = {
static const u16 rgb_addr_old = 0x01fa;
static const u16 rgb_addr_new = 0x024a;
+static const u16 rumble_addr[] = {
+ 0x0022, /* left */
+ 0x0023, /* right */
+};
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -308,6 +315,12 @@ struct claw_rgb_report {
struct rgb_frame zone_data;
} __packed;
+struct claw_rumble_report {
+ struct claw_profile_report;
+ u8 padding;
+ u8 intensity;
+} __packed;
+
struct claw_drvdata {
/* MCU General Variables */
enum claw_profile_ack_pending profile_pending;
@@ -332,8 +345,12 @@ struct claw_drvdata {
enum claw_gamepad_mode_index gamepad_mode;
u8 m1_codes[CLAW_KEYS_MAX];
u8 m2_codes[CLAW_KEYS_MAX];
- spinlock_t mode_lock; /* Lock for mode data read/write */
+ u8 rumble_intensity_right;
+ u8 rumble_intensity_left;
const u16 *bmap_addr;
+ spinlock_t rumble_lock; /* lock for rumble_intensity read/write */
+ spinlock_t mode_lock; /* Lock for mode data read/write */
+ bool rumble_support;
bool gp_registered;
bool bmap_support;
@@ -382,6 +399,7 @@ static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_report *cmd_rep)
{
enum claw_profile_ack_pending profile;
+ struct claw_rumble_report *rumble;
struct claw_mkey_report *mkeys;
struct claw_rgb_report *frame;
u16 rgb_addr, read_addr;
@@ -431,6 +449,20 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
}
break;
+ case CLAW_RUMBLE_LEFT_PENDING:
+ rumble = (struct claw_rumble_report *)cmd_rep->data;
+ if (be16_to_cpu(rumble->read_addr) != rumble_addr[0])
+ return -EINVAL;
+ scoped_guard(spinlock, &drvdata->rumble_lock)
+ drvdata->rumble_intensity_left = rumble->intensity;
+ break;
+ case CLAW_RUMBLE_RIGHT_PENDING:
+ rumble = (struct claw_rumble_report *)cmd_rep->data;
+ if (be16_to_cpu(rumble->read_addr) != rumble_addr[1])
+ return -EINVAL;
+ scoped_guard(spinlock, &drvdata->rumble_lock)
+ drvdata->rumble_intensity_right = rumble->intensity;
+ break;
default:
dev_dbg(&drvdata->hdev->dev,
"Got profile event without changes pending from command: %x\n",
@@ -930,6 +962,162 @@ static ssize_t button_mapping_options_show(struct device *dev,
}
static DEVICE_ATTR_RO(button_mapping_options);
+static ssize_t rumble_intensity_left_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct claw_rumble_report report = { {0x01, cpu_to_be16(rumble_addr[0])}, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 val;
+ int ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gp_registered))
+ return -ENODEV;
+ }
+
+ ret = kstrtou8(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 100)
+ return -EINVAL;
+
+ report.intensity = val;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ /* MCU will not send ACK until the USB transaction completes. ACK is sent
+ * immediately after and will hit the stale state machine, before the next
+ * command re-arms the state machine. Timeout 0 ensures no deadlock waiting
+ * for ACK that ill never come.
+ */
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t rumble_intensity_left_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct claw_rumble_report report = { {0x01, cpu_to_be16(rumble_addr[0])}, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret;
+ u8 val;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gp_registered))
+ return -ENODEV;
+ }
+
+ guard(mutex)(&drvdata->profile_mutex);
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ drvdata->profile_pending = CLAW_RUMBLE_LEFT_PENDING;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->rumble_lock)
+ val = drvdata->rumble_intensity_left;
+
+ return sysfs_emit(buf, "%u\n", val);
+}
+static DEVICE_ATTR_RW(rumble_intensity_left);
+
+static ssize_t rumble_intensity_right_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct claw_rumble_report report = { {0x01, cpu_to_be16(rumble_addr[1])}, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 val;
+ int ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gp_registered))
+ return -ENODEV;
+ }
+
+ ret = kstrtou8(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 100)
+ return -EINVAL;
+
+ report.intensity = val;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ /* MCU will not send ACK until the USB transaction completes. ACK is sent
+ * immediately after and will hit the stale state machine, before the next
+ * command re-arms the state machine. Timeout 0 ensures no deadlock waiting
+ * for ACK that ill never come.
+ */
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t rumble_intensity_right_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct claw_rumble_report report = { {0x01, cpu_to_be16(rumble_addr[1])}, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret;
+ u8 val;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gp_registered))
+ return -ENODEV;
+ }
+
+ guard(mutex)(&drvdata->profile_mutex);
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ drvdata->profile_pending = CLAW_RUMBLE_RIGHT_PENDING;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->rumble_lock)
+ val = drvdata->rumble_intensity_right;
+
+ return sysfs_emit(buf, "%u\n", val);
+}
+static DEVICE_ATTR_RW(rumble_intensity_right);
+
+static ssize_t rumble_intensity_range_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "0-100\n");
+}
+static DEVICE_ATTR_RO(rumble_intensity_range);
+
static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribute *attr,
int n)
{
@@ -950,6 +1138,12 @@ static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribu
attr == &dev_attr_reset.attr)
return attr->mode;
+ /* Hide rumble attrs if not supported */
+ if (attr == &dev_attr_rumble_intensity_left.attr ||
+ attr == &dev_attr_rumble_intensity_right.attr ||
+ attr == &dev_attr_rumble_intensity_range.attr)
+ return drvdata->rumble_support ? attr->mode : 0;
+
/* Hide button mapping attrs if it isn't supported */
return drvdata->bmap_support ? attr->mode : 0;
}
@@ -963,6 +1157,9 @@ static struct attribute *claw_gamepad_attrs[] = {
&dev_attr_mkeys_function.attr,
&dev_attr_mkeys_function_index.attr,
&dev_attr_reset.attr,
+ &dev_attr_rumble_intensity_left.attr,
+ &dev_attr_rumble_intensity_right.attr,
+ &dev_attr_rumble_intensity_range.attr,
NULL,
};
@@ -1542,6 +1739,7 @@ static void claw_features_supported(struct claw_drvdata *drvdata)
drvdata->bmap_support = true;
if (minor >= 0x66) {
drvdata->bmap_addr = button_mapping_addr_new;
+ drvdata->rumble_support = true;
drvdata->rgb_addr = rgb_addr_new;
} else {
drvdata->bmap_addr = button_mapping_addr_old;
@@ -1553,6 +1751,7 @@ static void claw_features_supported(struct claw_drvdata *drvdata)
if ((major == 0x02 && minor >= 0x17) || major >= 0x03) {
drvdata->bmap_support = true;
drvdata->bmap_addr = button_mapping_addr_new;
+ drvdata->rumble_support = true;
drvdata->rgb_addr = rgb_addr_new;
return;
}
@@ -1602,6 +1801,7 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
spin_lock_init(&drvdata->mode_lock);
spin_lock_init(&drvdata->profile_lock);
spin_lock_init(&drvdata->frame_lock);
+ spin_lock_init(&drvdata->rumble_lock);
init_completion(&drvdata->send_cmd_complete);
INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
--
2.53.0
^ permalink raw reply related
* [PATCH v11 3/4] HID: hid-msi: Add RGB control interface
From: Derek J. Clark @ 2026-05-29 7:21 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260529072111.7565-1-derekjohn.clark@gmail.com>
Adds RGB control interface for MSI Claw devices. The MSI Claw uses a
fairly unique RGB interface. It has 9 total zones (4 per joystick ring
and 1 for the ABXY buttons), and supports up to 8 sequential frames of
RGB zone data. Each frame is written to a specific area of MCU memory by
the profile command, the value of which changes based on the firmware of
the device. Unlike other devices (such as the Legion Go or the OneXPlayer
devices), there are no hard coded effects built into the MCU. Instead,
the basic effects are provided as a series of frame data. I have
mirrored the effects available in Windows in this driver, while keeping
the effect names consistent with the Lenovo drivers for the effects that
are similar.
Initial reverse-engineering and implementation of this feature was done
by Zhouwang Huang. I refactored the overall format to conform to kernel
driver best practices and style guides. Claude was used as an initial
reviewer of this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v9:
- Limit guard usage in cfg_setup_fn to avoid holding a lock during
registration and add group events.
- Don't use devm_ functions in cfg_setup_fn. Manually setup/teardown
devices and groups to prevent use after free.
- Ensure rgb_queue work is canceled during suspend.
- Check for drvdata in rgb_queue_fn to avoid use after free during
teardown.
v8:
- Ensure led_classdev is unregistered if adding attribute group fails.
- Reorder remove actions to ensure no use-after free or rearming cleared
flags.
v7:
- Use smp_[store_release|load_acquire] pattern for checking
rgb_registered to avoid possible races during teardown.
- Add gating to cfg_setup_fn, allowing either gamepad settings or rgb
settings to populate if the other fails for any reason.
- Use spinlock when writing profile_pending.
v6:
- Make all timeouts 25ms to ensure at least 2 jiffies in a 100Hz
config.
- Gate all attribute show/store functions with rgb_registered,
enabling use of devm_device_add_group.
v5:
- Move adding the RGB device into cfg_setup to prevent led core
attributes from being written to prior to setup completing.
- Ensure frame_lock is properly init.
- Change variable names in RGB functions from frame and zone to f and
z respectively to fit all scoped_guard actions in 100 columns.
v4:
- Fix frame_calc validity check to use >=.
- USe spinlock instead of mutex in raw_event and related attribute
_store function.
- Ensure delayed work is canceled in suspend & canceled before sysfs
attribute removal.
v3:
- Add mutex for read/write of rgb frame data.
- Remove setting rgb_frame_count when reading rgb profiles as it always
returns garbage data.
- Ensure rgb_speed is getting drvdata from a valid lookup (not hdev).
v2:
- Use pending_profile mutex
---
drivers/hid/hid-msi.c | 682 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 663 insertions(+), 19 deletions(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index f099d6cb7ea8d..96710d80543d4 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -21,6 +21,7 @@
#include <linux/device.h>
#include <linux/hid.h>
#include <linux/kobject.h>
+#include <linux/led-class-multicolor.h>
#include <linux/leds.h>
#include <linux/module.h>
#include <linux/mutex.h>
@@ -44,6 +45,10 @@
#define CLAW_KEYS_MAX 5
+#define CLAW_RGB_ZONES 9
+#define CLAW_RGB_MAX_FRAMES 8
+#define CLAW_RGB_FRAME_OFFSET 0x24
+
enum claw_command_index {
CLAW_COMMAND_TYPE_NONE = 0x00,
CLAW_COMMAND_TYPE_READ_PROFILE = 0x04,
@@ -73,6 +78,7 @@ enum claw_profile_ack_pending {
CLAW_NO_PENDING,
CLAW_M1_PENDING,
CLAW_M2_PENDING,
+ CLAW_RGB_PENDING,
};
enum claw_key_index {
@@ -231,6 +237,22 @@ static const struct {
{ 0xff, "DISABLED" },
};
+enum claw_rgb_effect_index {
+ CLAW_RGB_EFFECT_MONOCOLOR,
+ CLAW_RGB_EFFECT_BREATHE,
+ CLAW_RGB_EFFECT_CHROMA,
+ CLAW_RGB_EFFECT_RAINBOW,
+ CLAW_RGB_EFFECT_FROSTFIRE,
+};
+
+static const char * const claw_rgb_effect_text[] = {
+ [CLAW_RGB_EFFECT_MONOCOLOR] = "monocolor",
+ [CLAW_RGB_EFFECT_BREATHE] = "breathe",
+ [CLAW_RGB_EFFECT_CHROMA] = "chroma",
+ [CLAW_RGB_EFFECT_RAINBOW] = "rainbow",
+ [CLAW_RGB_EFFECT_FROSTFIRE] = "frostfire",
+};
+
static const u16 button_mapping_addr_old[] = {
0x007a, /* M1 */
0x011f, /* M2 */
@@ -241,6 +263,9 @@ static const u16 button_mapping_addr_new[] = {
0x0164, /* M2 */
};
+static const u16 rgb_addr_old = 0x01fa;
+static const u16 rgb_addr_new = 0x024a;
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -262,6 +287,27 @@ struct claw_mkey_report {
u8 codes[5];
} __packed;
+struct rgb_zone {
+ u8 red;
+ u8 green;
+ u8 blue;
+};
+
+struct rgb_frame {
+ struct rgb_zone zone[CLAW_RGB_ZONES];
+};
+
+struct claw_rgb_report {
+ struct claw_profile_report;
+ u8 frame_bytes;
+ u8 padding;
+ u8 frame_count;
+ u8 state; /* Always 0x09 */
+ u8 speed;
+ u8 brightness;
+ struct rgb_frame zone_data;
+} __packed;
+
struct claw_drvdata {
/* MCU General Variables */
enum claw_profile_ack_pending profile_pending;
@@ -286,10 +332,22 @@ struct claw_drvdata {
enum claw_gamepad_mode_index gamepad_mode;
u8 m1_codes[CLAW_KEYS_MAX];
u8 m2_codes[CLAW_KEYS_MAX];
- const u16 *bmap_addr;
spinlock_t mode_lock; /* Lock for mode data read/write */
+ const u16 *bmap_addr;
bool gp_registered;
bool bmap_support;
+
+ /* RGB Variables */
+ struct rgb_frame rgb_frames[CLAW_RGB_MAX_FRAMES];
+ enum claw_rgb_effect_index rgb_effect;
+ struct led_classdev_mc led_mc;
+ struct delayed_work rgb_queue;
+ spinlock_t frame_lock; /* lock for rgb_frames read/write */
+ bool rgb_registered;
+ u8 rgb_frame_count;
+ bool rgb_enabled;
+ u8 rgb_speed;
+ u16 rgb_addr;
};
static int get_endpoint_address(struct hid_device *hdev)
@@ -325,8 +383,11 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
{
enum claw_profile_ack_pending profile;
struct claw_mkey_report *mkeys;
- u8 *codes, key;
- int i;
+ struct claw_rgb_report *frame;
+ u16 rgb_addr, read_addr;
+ u8 *codes, key, f_idx;
+ u16 frame_calc;
+ int i, ret = 0;
scoped_guard(spinlock, &drvdata->profile_lock)
profile = drvdata->profile_pending;
@@ -342,6 +403,34 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
for (i = 0; i < CLAW_KEYS_MAX; i++)
codes[i] = (mkeys->codes[i]);
break;
+ case CLAW_RGB_PENDING:
+ frame = (struct claw_rgb_report *)cmd_rep->data;
+ rgb_addr = drvdata->rgb_addr;
+ read_addr = be16_to_cpu(frame->read_addr);
+ frame_calc = (read_addr - rgb_addr) / CLAW_RGB_FRAME_OFFSET;
+ if (frame_calc >= CLAW_RGB_MAX_FRAMES) {
+ dev_err(&drvdata->hdev->dev, "Got unsupported frame index: %x\n",
+ frame_calc);
+ return -EINVAL;
+ }
+ f_idx = frame_calc;
+
+ scoped_guard(spinlock, &drvdata->frame_lock) {
+ memcpy(&drvdata->rgb_frames[f_idx], &frame->zone_data,
+ sizeof(struct rgb_frame));
+
+ /* Only use frame 0 for remaining variable assignment */
+ if (f_idx != 0)
+ break;
+
+ drvdata->rgb_speed = frame->speed;
+ drvdata->led_mc.led_cdev.brightness = frame->brightness;
+ drvdata->led_mc.subled_info[0].intensity = frame->zone_data.zone[0].red;
+ drvdata->led_mc.subled_info[1].intensity = frame->zone_data.zone[0].green;
+ drvdata->led_mc.subled_info[2].intensity = frame->zone_data.zone[0].blue;
+ }
+
+ break;
default:
dev_dbg(&drvdata->hdev->dev,
"Got profile event without changes pending from command: %x\n",
@@ -351,7 +440,7 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
scoped_guard(spinlock, &drvdata->profile_lock)
drvdata->profile_pending = CLAW_NO_PENDING;
- return 0;
+ return ret;
}
static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
@@ -707,7 +796,7 @@ static int claw_mkey_store(struct device *dev, const char *buf, u8 mkey)
int ret, key_count, i;
scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
- /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
if (!smp_load_acquire(&drvdata->gp_registered))
return -ENODEV;
}
@@ -757,7 +846,7 @@ static int claw_mkey_show(struct device *dev, char *buf, enum claw_key_index m_k
u8 *codes;
scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
- /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
if (!smp_load_acquire(&drvdata->gp_registered))
return -ENODEV;
}
@@ -882,32 +971,553 @@ static const struct attribute_group claw_gamepad_attr_group = {
.is_visible = claw_gamepad_attr_is_visible,
};
+/* Read RGB config from device */
+static int claw_read_rgb_config(struct hid_device *hdev)
+{
+ u8 data[4] = { 0x01, 0x00, 0x00, CLAW_RGB_FRAME_OFFSET };
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u16 read_addr = drvdata->rgb_addr;
+ size_t len = ARRAY_SIZE(data);
+ int ret, i;
+
+ if (!drvdata->rgb_addr)
+ return -ENODEV;
+
+ /* Loop through all 8 pages of RGB data */
+ guard(mutex)(&drvdata->profile_mutex);
+ for (i = 0; i < CLAW_RGB_MAX_FRAMES; i++) {
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ drvdata->profile_pending = CLAW_RGB_PENDING;
+ data[1] = (read_addr >> 8) & 0xff;
+ data[2] = read_addr & 0x00ff;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE, data, len, 25);
+ if (ret)
+ return ret;
+
+ read_addr += CLAW_RGB_FRAME_OFFSET;
+ }
+
+ return 0;
+}
+
+/* Send RGB configuration to device */
+static int claw_write_rgb_state(struct claw_drvdata *drvdata)
+{
+ struct claw_rgb_report report = { {0x01, 0}, CLAW_RGB_FRAME_OFFSET, 0x00,
+ drvdata->rgb_frame_count, 0x09, drvdata->rgb_speed,
+ drvdata->led_mc.led_cdev.brightness };
+ u16 write_addr = drvdata->rgb_addr;
+ int f, ret;
+
+ if (!drvdata->rgb_addr)
+ return -ENODEV;
+
+ if (!drvdata->rgb_frame_count)
+ return -EINVAL;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ /* Loop through (up to) 8 pages of RGB data */
+ for (f = 0; f < drvdata->rgb_frame_count; f++) {
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock)
+ report.zone_data = drvdata->rgb_frames[f];
+
+ /* Set the MCU address to write the frame data to */
+ report.read_addr = cpu_to_be16(write_addr);
+
+ /* Serialize the rgb_report and write it to MCU */
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ /* Increment the write addr by the offset for the next frame */
+ write_addr += CLAW_RGB_FRAME_OFFSET;
+ }
+
+ /* MCU will not send ACK until the USB transaction completes. ACK is sent
+ * immediately after and will hit the stale state machine, before the next
+ * command re-arms the state machine. Timeout 0 ensures no deadlock waiting
+ * for ACK that ill never come.
+ */
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);
+
+ return ret;
+}
+
+/* Fill all zones with the same color */
+static void claw_frame_fill_solid(struct rgb_frame *frame, struct rgb_zone zone)
+{
+ int z;
+
+ for (z = 0; z < CLAW_RGB_ZONES; z++)
+ frame->zone[z] = zone;
+}
+
+/* Apply solid effect (1 frame, no color) */
+static int claw_apply_disabled(struct claw_drvdata *drvdata)
+{
+ struct rgb_zone off = { 0x00, 0x00, 0x00};
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = 1;
+ claw_frame_fill_solid(&drvdata->rgb_frames[0], off);
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply solid effect (1 frame, all zones same color) */
+static int claw_apply_monocolor(struct claw_drvdata *drvdata)
+{
+ struct mc_subled *subleds = drvdata->led_mc.subled_info;
+ struct rgb_zone zone = { subleds[0].intensity, subleds[1].intensity,
+ subleds[2].intensity };
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = 1;
+ claw_frame_fill_solid(&drvdata->rgb_frames[0], zone);
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply breathe effect (2 frames: color -> off) */
+static int claw_apply_breathe(struct claw_drvdata *drvdata)
+{
+ struct mc_subled *subleds = drvdata->led_mc.subled_info;
+ struct rgb_zone zone = { subleds[0].intensity, subleds[1].intensity,
+ subleds[2].intensity };
+ static const struct rgb_zone off = { 0, 0, 0 };
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = 2;
+ claw_frame_fill_solid(&drvdata->rgb_frames[0], zone);
+ claw_frame_fill_solid(&drvdata->rgb_frames[1], off);
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply chroma effect (6 frames: rainbow cycle, all zones sync) */
+static int claw_apply_chroma(struct claw_drvdata *drvdata)
+{
+ static const struct rgb_zone colors[] = {
+ {255, 0, 0}, /* red */
+ {255, 255, 0}, /* yellow */
+ { 0, 255, 0}, /* green */
+ { 0, 255, 255}, /* cyan */
+ { 0, 0, 255}, /* blue */
+ {255, 0, 255}, /* magenta */
+ };
+ u8 frame_count = ARRAY_SIZE(colors);
+ int f;
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = frame_count;
+
+ for (f = 0; f < frame_count; f++)
+ claw_frame_fill_solid(&drvdata->rgb_frames[f], colors[f]);
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply rainbow effect (4 frames: rotating colors around joysticks) */
+static int claw_apply_rainbow(struct claw_drvdata *drvdata)
+{
+ static const struct rgb_zone colors[] = {
+ {255, 0, 0}, /* red */
+ { 0, 255, 0}, /* green */
+ { 0, 255, 255}, /* cyan */
+ { 0, 0, 255}, /* blue */
+ };
+ u8 frame_count = ARRAY_SIZE(colors);
+ int f, z;
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = frame_count;
+
+ for (f = 0; f < frame_count; f++) {
+ for (z = 0; z < 4; z++) {
+ drvdata->rgb_frames[f].zone[z] = colors[(z + f) % 4];
+ drvdata->rgb_frames[f].zone[z + 4] = colors[(z + f) % 4];
+ }
+ drvdata->rgb_frames[f].zone[8] = colors[f];
+ }
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/*
+ * Apply frostfire effect (4 frames: fire vs ice rotating)
+ * Right joystick: fire red -> dark -> ice blue -> dark (clockwise)
+ * Left joystick: ice blue -> dark -> fire red -> dark (counter-clockwise)
+ * ABXY: fire red -> dark -> ice blue -> dark
+ */
+static int claw_apply_frostfire(struct claw_drvdata *drvdata)
+{
+ static const struct rgb_zone colors[] = {
+ {255, 0, 0}, /* fire red */
+ { 0, 0, 0}, /* dark */
+ { 0, 0, 255}, /* ice blue */
+ { 0, 0, 0}, /* dark */
+ };
+ u8 frame_count = ARRAY_SIZE(colors);
+ int f, z;
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = frame_count;
+
+ for (f = 0; f < frame_count; f++) {
+ for (z = 0; z < 4; z++) {
+ drvdata->rgb_frames[f].zone[z] = colors[(z + f) % 4];
+ drvdata->rgb_frames[f].zone[z + 4] = colors[(z - f + 6) % 4];
+ }
+ drvdata->rgb_frames[f].zone[8] = colors[f];
+ }
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply current state to device */
+static int claw_apply_rgb_state(struct claw_drvdata *drvdata)
+{
+ if (!drvdata->rgb_enabled)
+ return claw_apply_disabled(drvdata);
+
+ switch (drvdata->rgb_effect) {
+ case CLAW_RGB_EFFECT_MONOCOLOR:
+ return claw_apply_monocolor(drvdata);
+ case CLAW_RGB_EFFECT_BREATHE:
+ return claw_apply_breathe(drvdata);
+ case CLAW_RGB_EFFECT_CHROMA:
+ return claw_apply_chroma(drvdata);
+ case CLAW_RGB_EFFECT_RAINBOW:
+ return claw_apply_rainbow(drvdata);
+ case CLAW_RGB_EFFECT_FROSTFIRE:
+ return claw_apply_frostfire(drvdata);
+ default:
+ dev_err(drvdata->led_mc.led_cdev.dev,
+ "No supported rgb_effect selected\n");
+ return -EINVAL;
+ }
+}
+
+static void claw_rgb_queue_fn(struct work_struct *work)
+{
+ struct delayed_work *dwork = container_of(work, struct delayed_work, work);
+ struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, rgb_queue);
+ int ret;
+
+ if (!drvdata)
+ return;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return;
+ }
+
+ ret = claw_apply_rgb_state(drvdata);
+ if (ret)
+ dev_err(drvdata->led_mc.led_cdev.dev,
+ "Failed to apply RGB state: %d\n", ret);
+}
+
+static ssize_t effect_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ int ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+ }
+
+ ret = sysfs_match_string(claw_rgb_effect_text, buf);
+ if (ret < 0)
+ return ret;
+
+ drvdata->rgb_effect = ret;
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+
+ return count;
+}
+
+static ssize_t effect_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+ }
+
+ if (drvdata->rgb_effect >= ARRAY_SIZE(claw_rgb_effect_text))
+ return -EINVAL;
+
+ return sysfs_emit(buf, "%s\n", claw_rgb_effect_text[drvdata->rgb_effect]);
+}
+
+static DEVICE_ATTR_RW(effect);
+
+static ssize_t effect_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, count = 0;
+
+ for (i = 0; i < ARRAY_SIZE(claw_rgb_effect_text); i++)
+ count += sysfs_emit_at(buf, count, "%s ", claw_rgb_effect_text[i]);
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(effect_index);
+
+static ssize_t enabled_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ bool val;
+ int ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+ }
+
+ ret = kstrtobool(buf, &val);
+ if (ret)
+ return ret;
+
+ drvdata->rgb_enabled = val;
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+
+ return count;
+}
+
+static ssize_t enabled_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+ }
+
+ return sysfs_emit(buf, "%s\n", drvdata->rgb_enabled ? "true" : "false");
+}
+static DEVICE_ATTR_RW(enabled);
+
+static ssize_t enabled_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "true false\n");
+}
+static DEVICE_ATTR_RO(enabled_index);
+
+static ssize_t speed_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ unsigned int val, speed;
+ int ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+ }
+
+ ret = kstrtouint(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 20)
+ return -EINVAL;
+
+ /* 0 is fastest, invert value for intuitive userspace speed */
+ speed = 20 - val;
+
+ drvdata->rgb_speed = speed;
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+
+ return count;
+}
+
+static ssize_t speed_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ u8 speed = 20 - drvdata->rgb_speed;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+ }
+
+ return sysfs_emit(buf, "%u\n", speed);
+}
+static DEVICE_ATTR_RW(speed);
+
+static ssize_t speed_range_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "0-20\n");
+}
+static DEVICE_ATTR_RO(speed_range);
+
+static void claw_led_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness _brightness)
+{
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return;
+ }
+
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+}
+
+static struct attribute *claw_rgb_attrs[] = {
+ &dev_attr_effect.attr,
+ &dev_attr_effect_index.attr,
+ &dev_attr_enabled.attr,
+ &dev_attr_enabled_index.attr,
+ &dev_attr_speed.attr,
+ &dev_attr_speed_range.attr,
+ NULL,
+};
+
+static const struct attribute_group claw_rgb_attr_group = {
+ .attrs = claw_rgb_attrs,
+};
+
+static struct mc_subled claw_rgb_subled_info[] = {
+ {
+ .color_index = LED_COLOR_ID_RED,
+ .channel = 0x1,
+ },
+ {
+ .color_index = LED_COLOR_ID_GREEN,
+ .channel = 0x2,
+ },
+ {
+ .color_index = LED_COLOR_ID_BLUE,
+ .channel = 0x3,
+ },
+};
+
static void cfg_setup_fn(struct work_struct *work)
{
struct delayed_work *dwork = container_of(work, struct delayed_work, work);
struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, cfg_setup);
+ bool gamepad_ready = false, rgb_ready = false, gp_registered, rgb_registered;
int ret;
ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE,
NULL, 0, 25);
if (ret) {
dev_err(&drvdata->hdev->dev,
- "Failed to setup device, can't read gamepad mode: %d\n", ret);
- return;
+ "Failed to read gamepad mode: %d\n", ret);
+ goto prep_rgb;
}
+ gamepad_ready = true;
- /* Add sysfs attributes after we get the device state */
- ret = device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
+prep_rgb:
+ ret = claw_read_rgb_config(drvdata->hdev);
if (ret) {
dev_err(&drvdata->hdev->dev,
- "Failed to setup device, can't create gamepad attrs: %d\n", ret);
- return;
+ "Failed to read RGB config: %d\n", ret);
+ goto try_gamepad;
}
+ rgb_ready = true;
+
+ /* Add sysfs attributes after we get the device state */
+try_gamepad:
scoped_guard(spinlock_irqsave, &drvdata->registration_lock)
- /* Pairs with smp_load_acquire in attribute show/store functions */
- smp_store_release(&drvdata->gp_registered, true);
+ /* Pairs with smp_store_release from below */
+ gp_registered = smp_load_acquire(&drvdata->gp_registered);
+
+ if (!gp_registered && gamepad_ready) {
+ ret = device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to create gamepad attrs: %d\n", ret);
+ goto try_rgb;
+ }
- kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_load_acquire in attribute show/store functions */
+ smp_store_release(&drvdata->gp_registered, true);
+ gp_registered = true;
+ }
+ }
+
+try_rgb:
+ /* Add and enable RGB interface once we have the device state */
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock)
+ /* Pairs with smp_store_release from below */
+ rgb_registered = smp_load_acquire(&drvdata->rgb_registered);
+
+ if (!rgb_registered && rgb_ready) {
+ ret = led_classdev_multicolor_register(&drvdata->hdev->dev,
+ &drvdata->led_mc);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to create led device: %d\n", ret);
+ goto update_kobjects;
+ }
+
+ ret = device_add_group(drvdata->led_mc.led_cdev.dev, &claw_rgb_attr_group);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to create RGB attrs: %d\n", ret);
+ led_classdev_multicolor_unregister(&drvdata->led_mc);
+ goto update_kobjects;
+ }
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_load_acquire in attribute show/store functions */
+ smp_store_release(&drvdata->rgb_registered, true);
+ rgb_registered = true;
+ }
+ }
+
+update_kobjects:
+ if (gp_registered)
+ kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
+ if (rgb_registered)
+ kobject_uevent(&drvdata->led_mc.led_cdev.dev->kobj, KOBJ_CHANGE);
}
static void cfg_resume_fn(struct work_struct *work)
@@ -916,8 +1526,10 @@ static void cfg_resume_fn(struct work_struct *work)
struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, cfg_resume);
guard(spinlock_irqsave)(&drvdata->registration_lock);
- /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
- if (!smp_load_acquire(&drvdata->gp_registered))
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gp_registered) ||
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ !smp_load_acquire(&drvdata->rgb_registered))
schedule_delayed_work(&drvdata->cfg_setup, msecs_to_jiffies(500));
}
@@ -928,18 +1540,24 @@ static void claw_features_supported(struct claw_drvdata *drvdata)
if (major == 0x01) {
drvdata->bmap_support = true;
- if (minor >= 0x66)
+ if (minor >= 0x66) {
drvdata->bmap_addr = button_mapping_addr_new;
- else
+ drvdata->rgb_addr = rgb_addr_new;
+ } else {
drvdata->bmap_addr = button_mapping_addr_old;
+ drvdata->rgb_addr = rgb_addr_old;
+ }
return;
}
if ((major == 0x02 && minor >= 0x17) || major >= 0x03) {
drvdata->bmap_support = true;
drvdata->bmap_addr = button_mapping_addr_new;
+ drvdata->rgb_addr = rgb_addr_new;
return;
}
+
+ drvdata->rgb_addr = rgb_addr_old;
}
static int claw_probe(struct hid_device *hdev, u8 ep)
@@ -954,6 +1572,7 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
return -ENOMEM;
drvdata->gamepad_mode = CLAW_GAMEPAD_MODE_XINPUT;
+ drvdata->rgb_enabled = true;
drvdata->hdev = hdev;
drvdata->ep = ep;
@@ -964,6 +1583,17 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
if (!drvdata->bmap_support)
dev_dbg(&hdev->dev, "M-Key mapping is not supported. Update firmware to enable.\n");
+ drvdata->led_mc.led_cdev.name = "msi_claw:rgb:joystick_rings";
+ drvdata->led_mc.led_cdev.brightness = 0x50;
+ drvdata->led_mc.led_cdev.max_brightness = 0x64;
+ drvdata->led_mc.led_cdev.color = LED_COLOR_ID_RGB;
+ drvdata->led_mc.led_cdev.brightness_set = claw_led_brightness_set;
+ drvdata->led_mc.num_colors = 3;
+ drvdata->led_mc.subled_info = devm_kmemdup(&hdev->dev, claw_rgb_subled_info,
+ sizeof(claw_rgb_subled_info), GFP_KERNEL);
+ if (!drvdata->led_mc.subled_info)
+ return -ENOMEM;
+
mutex_init(&drvdata->cfg_mutex);
mutex_init(&drvdata->profile_mutex);
mutex_init(&drvdata->rom_mutex);
@@ -971,9 +1601,11 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
spin_lock_init(&drvdata->cmd_lock);
spin_lock_init(&drvdata->mode_lock);
spin_lock_init(&drvdata->profile_lock);
+ spin_lock_init(&drvdata->frame_lock);
init_completion(&drvdata->send_cmd_complete);
INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
+ INIT_DELAYED_WORK(&drvdata->rgb_queue, &claw_rgb_queue_fn);
/* For control interface: open the HID transport for sending commands. */
ret = hid_hw_open(hdev);
@@ -1030,6 +1662,7 @@ static void claw_remove(struct hid_device *hdev)
{
struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
bool gp_registered;
+ bool rgb_registered;
if (!drvdata)
return;
@@ -1042,11 +1675,21 @@ static void claw_remove(struct hid_device *hdev)
gp_registered = smp_load_acquire(&drvdata->gp_registered);
/* Pairs with smp_load_acquire in attribute show/store functions */
smp_store_release(&drvdata->gp_registered, false);
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ rgb_registered = smp_load_acquire(&drvdata->rgb_registered);
+ /* Pairs with smp_load_acquire in attribute show/store functions */
+ smp_store_release(&drvdata->rgb_registered, false);
}
if (gp_registered)
device_remove_group(&hdev->dev, &claw_gamepad_attr_group);
+ cancel_delayed_work_sync(&drvdata->rgb_queue);
+ if (rgb_registered) {
+ device_remove_group(drvdata->led_mc.led_cdev.dev, &claw_rgb_attr_group);
+ led_classdev_multicolor_unregister(&drvdata->led_mc);
+ }
+
hid_hw_close(hdev);
}
@@ -1106,6 +1749,7 @@ static int claw_suspend(struct hid_device *hdev)
cancel_delayed_work_sync(&drvdata->cfg_resume);
cancel_delayed_work_sync(&drvdata->cfg_setup);
+ cancel_delayed_work_sync(&drvdata->rgb_queue);
return 0;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v11 2/4] HID: hid-msi: Add M-key mapping attributes
From: Derek J. Clark @ 2026-05-29 7:21 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260529072111.7565-1-derekjohn.clark@gmail.com>
Adds attributes that allow for remapping the M-keys with up to 5 values
when in macro mode. There are 2 mappable buttons on the rear of the
device, M1 on the right and M2 on the left. When mapped, the events will
fire from one of three event devices: gamepad buttons will fire from the
device handled by xpad, while keyboard and mouse events will fire from
respectively typed evdevs provided by the input core. Names of each
mapping have been kept as close to the event that will fire from the evdev
as possible, with context added to the ABS_ events on the direction of the
movement.
Initial reverse-engineering and implementation of this feature was done
by Zhouwang Huang. I refactored the overall format to conform to kernel
driver best practices and style guides. Claude was used as an initial
reviewer of this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v10:
- Remove additional gamepad_registered variable left over after rename
to gp_registered.
v8:
- Wrap all branches under single cmd_lock guard in claw_raw_event.
- Reject generic ACK in claw_raw_event if waiting_cmd is for another
branch.
v7:
- Use smp_[store_release|load_acquire] pattern for checking
gamepad_registered to avoid possible races during teardown.
- Add profile_lock for read/write profile_pending.
- Match on write address for mkey reports to prevent late ACK
from causing synchronization errors.
- Use struct for mkey reports.
v6:
- Make all timeouts 25ms to ensure at least 2 jiffies in a 100Hz
config.
- Gate all attribute show/store functions with gamepad_registered.
- Remove duplicated argv_free macro.
v5:
- Ensure adding "DISABLED" key to valid entries is done in the correct
patch.
- Re-enable sending an empty string to clear button mappings in
addition to setting DISABLED.
v4:
- Change dev_warn to dev_dbg in claw_profile_event.
- use __free with DEFINE_FREE macro for argv instead of manually
running argv_free, cleaining up scoped_guard goto.
v3:
- Use scoped_guard where necessary.
v2:
- Add mutex for SYNC_TO_ROM commands to ensure every SYNC is completed
before more data is written to the MCU volatile memory.
- Add mutex for profile_pending to ensure every profile action
response is serialized to the generating command.
---
drivers/hid/hid-msi.c | 448 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 446 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 3a176f44f2c43..f099d6cb7ea8d 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -42,6 +42,8 @@
#define CLAW_DINPUT_CFG_INTF_IN 0x82
#define CLAW_XINPUT_CFG_INTF_IN 0x83
+#define CLAW_KEYS_MAX 5
+
enum claw_command_index {
CLAW_COMMAND_TYPE_NONE = 0x00,
CLAW_COMMAND_TYPE_READ_PROFILE = 0x04,
@@ -67,6 +69,17 @@ static const char * const claw_gamepad_mode_text[] = {
[CLAW_GAMEPAD_MODE_DESKTOP] = "desktop",
};
+enum claw_profile_ack_pending {
+ CLAW_NO_PENDING,
+ CLAW_M1_PENDING,
+ CLAW_M2_PENDING,
+};
+
+enum claw_key_index {
+ CLAW_KEY_M1,
+ CLAW_KEY_M2,
+};
+
enum claw_mkeys_function_index {
CLAW_MKEY_FUNCTION_MACRO,
CLAW_MKEY_FUNCTION_DISABLED,
@@ -79,6 +92,155 @@ static const char * const claw_mkeys_function_text[] = {
[CLAW_MKEY_FUNCTION_COMBO] = "combination",
};
+static const struct {
+ u8 code;
+ const char *name;
+} claw_button_mapping_key_map[] = {
+ /* Gamepad buttons */
+ { 0x01, "ABS_HAT0Y_UP" },
+ { 0x02, "ABS_HAT0Y_DOWN" },
+ { 0x03, "ABS_HAT0X_LEFT" },
+ { 0x04, "ABS_HAT0X_RIGHT" },
+ { 0x05, "BTN_TL" },
+ { 0x06, "BTN_TR" },
+ { 0x07, "BTN_THUMBL" },
+ { 0x08, "BTN_THUMBR" },
+ { 0x09, "BTN_SOUTH" },
+ { 0x0a, "BTN_EAST" },
+ { 0x0b, "BTN_NORTH" },
+ { 0x0c, "BTN_WEST" },
+ { 0x0d, "BTN_MODE" },
+ { 0x0e, "BTN_SELECT" },
+ { 0x0f, "BTN_START" },
+ { 0x13, "BTN_TL2"},
+ { 0x14, "BTN_TR2"},
+ { 0x15, "ABS_Y_UP"},
+ { 0x16, "ABS_Y_DOWN"},
+ { 0x17, "ABS_X_LEFT"},
+ { 0x18, "ABS_X_RIGHT"},
+ { 0x19, "ABS_RY_UP"},
+ { 0x1a, "ABS_RY_DOWN"},
+ { 0x1b, "ABS_RX_LEFT"},
+ { 0x1c, "ABS_RX_RIGHT"},
+ /* Keyboard keys */
+ { 0x32, "KEY_ESC" },
+ { 0x33, "KEY_F1" },
+ { 0x34, "KEY_F2" },
+ { 0x35, "KEY_F3" },
+ { 0x36, "KEY_F4" },
+ { 0x37, "KEY_F5" },
+ { 0x38, "KEY_F6" },
+ { 0x39, "KEY_F7" },
+ { 0x3a, "KEY_F8" },
+ { 0x3b, "KEY_F9" },
+ { 0x3c, "KEY_F10" },
+ { 0x3d, "KEY_F11" },
+ { 0x3e, "KEY_F12" },
+ { 0x3f, "KEY_GRAVE" },
+ { 0x40, "KEY_1" },
+ { 0x41, "KEY_2" },
+ { 0x42, "KEY_3" },
+ { 0x43, "KEY_4" },
+ { 0x44, "KEY_5" },
+ { 0x45, "KEY_6" },
+ { 0x46, "KEY_7" },
+ { 0x47, "KEY_8" },
+ { 0x48, "KEY_9" },
+ { 0x49, "KEY_0" },
+ { 0x4a, "KEY_MINUS" },
+ { 0x4b, "KEY_EQUAL" },
+ { 0x4c, "KEY_BACKSPACE" },
+ { 0x4d, "KEY_TAB" },
+ { 0x4e, "KEY_Q" },
+ { 0x4f, "KEY_W" },
+ { 0x50, "KEY_E" },
+ { 0x51, "KEY_R" },
+ { 0x52, "KEY_T" },
+ { 0x53, "KEY_Y" },
+ { 0x54, "KEY_U" },
+ { 0x55, "KEY_I" },
+ { 0x56, "KEY_O" },
+ { 0x57, "KEY_P" },
+ { 0x58, "KEY_LEFTBRACE" },
+ { 0x59, "KEY_RIGHTBRACE" },
+ { 0x5a, "KEY_BACKSLASH" },
+ { 0x5b, "KEY_CAPSLOCK" },
+ { 0x5c, "KEY_A" },
+ { 0x5d, "KEY_S" },
+ { 0x5e, "KEY_D" },
+ { 0x5f, "KEY_F" },
+ { 0x60, "KEY_G" },
+ { 0x61, "KEY_H" },
+ { 0x62, "KEY_J" },
+ { 0x63, "KEY_K" },
+ { 0x64, "KEY_L" },
+ { 0x65, "KEY_SEMICOLON" },
+ { 0x66, "KEY_APOSTROPHE" },
+ { 0x67, "KEY_ENTER" },
+ { 0x68, "KEY_LEFTSHIFT" },
+ { 0x69, "KEY_Z" },
+ { 0x6a, "KEY_X" },
+ { 0x6b, "KEY_C" },
+ { 0x6c, "KEY_V" },
+ { 0x6d, "KEY_B" },
+ { 0x6e, "KEY_N" },
+ { 0x6f, "KEY_M" },
+ { 0x70, "KEY_COMMA" },
+ { 0x71, "KEY_DOT" },
+ { 0x72, "KEY_SLASH" },
+ { 0x73, "KEY_RIGHTSHIFT" },
+ { 0x74, "KEY_LEFTCTRL" },
+ { 0x75, "KEY_LEFTMETA" },
+ { 0x76, "KEY_LEFTALT" },
+ { 0x77, "KEY_SPACE" },
+ { 0x78, "KEY_RIGHTALT" },
+ { 0x79, "KEY_RIGHTCTRL" },
+ { 0x7a, "KEY_INSERT" },
+ { 0x7b, "KEY_HOME" },
+ { 0x7c, "KEY_PAGEUP" },
+ { 0x7d, "KEY_DELETE" },
+ { 0x7e, "KEY_END" },
+ { 0x7f, "KEY_PAGEDOWN" },
+ { 0x8a, "KEY_KPENTER" },
+ { 0x8b, "KEY_KP0" },
+ { 0x8c, "KEY_KP1" },
+ { 0x8d, "KEY_KP2" },
+ { 0x8e, "KEY_KP3" },
+ { 0x8f, "KEY_KP4" },
+ { 0x90, "KEY_KP5" },
+ { 0x91, "KEY_KP6" },
+ { 0x92, "KEY_KP7" },
+ { 0x93, "KEY_KP8" },
+ { 0x94, "KEY_KP9" },
+ { 0x95, "MD_PLAY" },
+ { 0x96, "MD_STOP" },
+ { 0x97, "MD_NEXT" },
+ { 0x98, "MD_PREV" },
+ { 0x99, "MD_VOL_UP" },
+ { 0x9a, "MD_VOL_DOWN" },
+ { 0x9b, "MD_VOL_MUTE" },
+ { 0x9c, "KEY_F23" },
+ /* Mouse events */
+ { 0xc8, "BTN_LEFT" },
+ { 0xc9, "BTN_MIDDLE" },
+ { 0xca, "BTN_RIGHT" },
+ { 0xcb, "BTN_SIDE" },
+ { 0xcc, "BTN_EXTRA" },
+ { 0xcd, "REL_WHEEL_UP" },
+ { 0xce, "REL_WHEEL_DOWN" },
+ { 0xff, "DISABLED" },
+};
+
+static const u16 button_mapping_addr_old[] = {
+ 0x007a, /* M1 */
+ 0x011f, /* M2 */
+};
+
+static const u16 button_mapping_addr_new[] = {
+ 0x00bb, /* M1 */
+ 0x0164, /* M2 */
+};
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -87,25 +249,47 @@ struct claw_command_report {
u8 data[59];
} __packed;
+struct claw_profile_report {
+ u8 profile;
+ __be16 read_addr;
+} __packed;
+
+struct claw_mkey_report {
+ struct claw_profile_report;
+ u8 padding_0;
+ u8 padding_1;
+ u8 padding_2;
+ u8 codes[5];
+} __packed;
+
struct claw_drvdata {
/* MCU General Variables */
+ enum claw_profile_ack_pending profile_pending;
struct completion send_cmd_complete;
struct delayed_work cfg_resume;
struct delayed_work cfg_setup;
spinlock_t registration_lock; /* Lock for registration read/write */
+ struct mutex profile_mutex; /* mutex for profile_pending calls */
+ spinlock_t profile_lock; /* Lock for profile_pending read/write */
struct hid_device *hdev;
struct mutex cfg_mutex; /* mutex for synchronous data */
+ struct mutex rom_mutex; /* mutex for SYNC_TO_ROM calls */
bool waiting_for_ack;
spinlock_t cmd_lock; /* Lock for cmd data read/write */
u8 waiting_cmd;
int cmd_status;
+ u16 bcd_device;
u8 ep;
/* Gamepad Variables */
enum claw_mkeys_function_index mkeys_function;
enum claw_gamepad_mode_index gamepad_mode;
+ u8 m1_codes[CLAW_KEYS_MAX];
+ u8 m2_codes[CLAW_KEYS_MAX];
+ const u16 *bmap_addr;
spinlock_t mode_lock; /* Lock for mode data read/write */
bool gp_registered;
+ bool bmap_support;
};
static int get_endpoint_address(struct hid_device *hdev)
@@ -137,6 +321,39 @@ static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
return 0;
}
+static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_report *cmd_rep)
+{
+ enum claw_profile_ack_pending profile;
+ struct claw_mkey_report *mkeys;
+ u8 *codes, key;
+ int i;
+
+ scoped_guard(spinlock, &drvdata->profile_lock)
+ profile = drvdata->profile_pending;
+
+ switch (profile) {
+ case CLAW_M1_PENDING:
+ case CLAW_M2_PENDING:
+ key = (profile == CLAW_M1_PENDING) ? CLAW_KEY_M1 : CLAW_KEY_M2;
+ mkeys = (struct claw_mkey_report *)cmd_rep->data;
+ if (be16_to_cpu(mkeys->read_addr) != drvdata->bmap_addr[key])
+ return -EINVAL;
+ codes = (profile == CLAW_M1_PENDING) ? drvdata->m1_codes : drvdata->m2_codes;
+ for (i = 0; i < CLAW_KEYS_MAX; i++)
+ codes[i] = (mkeys->codes[i]);
+ break;
+ default:
+ dev_dbg(&drvdata->hdev->dev,
+ "Got profile event without changes pending from command: %x\n",
+ cmd_rep->cmd);
+ return -EINVAL;
+ }
+ scoped_guard(spinlock, &drvdata->profile_lock)
+ drvdata->profile_pending = CLAW_NO_PENDING;
+
+ return 0;
+}
+
static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
u8 *data, int size)
{
@@ -165,9 +382,20 @@ static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *repor
complete(&drvdata->send_cmd_complete);
}
+ break;
+ case CLAW_COMMAND_TYPE_READ_PROFILE_ACK:
+ ret = claw_profile_event(drvdata, cmd_rep);
+ if (drvdata->waiting_for_ack &&
+ drvdata->waiting_cmd == CLAW_COMMAND_TYPE_READ_PROFILE) {
+ drvdata->cmd_status = ret;
+ drvdata->waiting_for_ack = false;
+ complete(&drvdata->send_cmd_complete);
+ }
+
break;
case CLAW_COMMAND_TYPE_ACK:
- if (drvdata->waiting_cmd == CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE)
+ if (drvdata->waiting_cmd == CLAW_COMMAND_TYPE_READ_PROFILE ||
+ drvdata->waiting_cmd == CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE)
break;
if (drvdata->waiting_for_ack) {
drvdata->cmd_status = 0;
@@ -442,6 +670,177 @@ static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_WO(reset);
+static int mkey_mapping_name_to_code(const char *name)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(claw_button_mapping_key_map); i++) {
+ if (!strcmp(name, claw_button_mapping_key_map[i].name))
+ return claw_button_mapping_key_map[i].code;
+ }
+
+ return -EINVAL;
+}
+
+static const char *mkey_mapping_code_to_name(u8 code)
+{
+ int i;
+
+ if (code == 0xff)
+ return NULL;
+
+ for (i = 0; i < ARRAY_SIZE(claw_button_mapping_key_map); i++) {
+ if (claw_button_mapping_key_map[i].code == code)
+ return claw_button_mapping_key_map[i].name;
+ }
+
+ return NULL;
+}
+
+static int claw_mkey_store(struct device *dev, const char *buf, u8 mkey)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct claw_mkey_report report = { {0x01, cpu_to_be16(drvdata->bmap_addr[mkey])},
+ 0x07, 0x04, 0x00, {0xff, 0xff, 0xff, 0xff, 0xff} };
+ char **raw_keys __free(argv_free) = NULL;
+ int ret, key_count, i;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gp_registered))
+ return -ENODEV;
+ }
+
+ raw_keys = argv_split(GFP_KERNEL, buf, &key_count);
+ if (!raw_keys)
+ return -ENOMEM;
+
+ if (key_count > CLAW_KEYS_MAX)
+ return -EINVAL;
+
+ if (key_count == 0)
+ goto set_buttons;
+
+ for (i = 0; i < key_count; i++) {
+ ret = mkey_mapping_name_to_code(raw_keys[i]);
+ if (ret < 0)
+ return ret;
+
+ report.codes[i] = ret;
+ }
+
+set_buttons:
+ scoped_guard(mutex, &drvdata->rom_mutex) {
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+ /* MCU will not send ACK until the USB transaction completes. ACK is sent
+ * immediately after and will hit the stale state machine, before the next
+ * command re-arms the state machine. Timeout 0 ensures no deadlock waiting
+ * for ACK that ill never come.
+ */
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);
+ }
+
+ return ret;
+}
+
+static int claw_mkey_show(struct device *dev, char *buf, enum claw_key_index m_key)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct claw_mkey_report report = { {0x01, cpu_to_be16(drvdata->bmap_addr[m_key])}, 0x07 };
+ int i, ret, count = 0;
+ const char *name;
+ u8 *codes;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gp_registered))
+ return -ENODEV;
+ }
+
+ codes = (m_key == CLAW_KEY_M1) ? drvdata->m1_codes : drvdata->m2_codes;
+
+ guard(mutex)(&drvdata->profile_mutex);
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ drvdata->profile_pending = (m_key == CLAW_KEY_M1) ? CLAW_M1_PENDING
+ : CLAW_M2_PENDING;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < CLAW_KEYS_MAX; i++) {
+ name = mkey_mapping_code_to_name(codes[i]);
+ if (name)
+ count += sysfs_emit_at(buf, count, "%s ", name);
+ }
+
+ if (!count)
+ return sysfs_emit(buf, "(not set)\n");
+
+ buf[count - 1] = '\n';
+
+ return count;
+}
+
+static ssize_t button_m1_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+
+ ret = claw_mkey_store(dev, buf, CLAW_KEY_M1);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t button_m1_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return claw_mkey_show(dev, buf, CLAW_KEY_M1);
+}
+static DEVICE_ATTR_RW(button_m1);
+
+static ssize_t button_m2_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+
+ ret = claw_mkey_store(dev, buf, CLAW_KEY_M2);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t button_m2_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return claw_mkey_show(dev, buf, CLAW_KEY_M2);
+}
+static DEVICE_ATTR_RW(button_m2);
+
+static ssize_t button_mapping_options_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, count = 0;
+
+ for (i = 0; i < ARRAY_SIZE(claw_button_mapping_key_map); i++)
+ count += sysfs_emit_at(buf, count, "%s ", claw_button_mapping_key_map[i].name);
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(button_mapping_options);
+
static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribute *attr,
int n)
{
@@ -454,10 +853,22 @@ static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribu
return 0;
}
- return attr->mode;
+ /* Always show attrs available on all firmware */
+ if (attr == &dev_attr_gamepad_mode.attr ||
+ attr == &dev_attr_gamepad_mode_index.attr ||
+ attr == &dev_attr_mkeys_function.attr ||
+ attr == &dev_attr_mkeys_function_index.attr ||
+ attr == &dev_attr_reset.attr)
+ return attr->mode;
+
+ /* Hide button mapping attrs if it isn't supported */
+ return drvdata->bmap_support ? attr->mode : 0;
}
static struct attribute *claw_gamepad_attrs[] = {
+ &dev_attr_button_m1.attr,
+ &dev_attr_button_m2.attr,
+ &dev_attr_button_mapping_options.attr,
&dev_attr_gamepad_mode.attr,
&dev_attr_gamepad_mode_index.attr,
&dev_attr_mkeys_function.attr,
@@ -510,8 +921,31 @@ static void cfg_resume_fn(struct work_struct *work)
schedule_delayed_work(&drvdata->cfg_setup, msecs_to_jiffies(500));
}
+static void claw_features_supported(struct claw_drvdata *drvdata)
+{
+ u8 major = (drvdata->bcd_device >> 8) & 0xff;
+ u8 minor = drvdata->bcd_device & 0xff;
+
+ if (major == 0x01) {
+ drvdata->bmap_support = true;
+ if (minor >= 0x66)
+ drvdata->bmap_addr = button_mapping_addr_new;
+ else
+ drvdata->bmap_addr = button_mapping_addr_old;
+ return;
+ }
+
+ if ((major == 0x02 && minor >= 0x17) || major >= 0x03) {
+ drvdata->bmap_support = true;
+ drvdata->bmap_addr = button_mapping_addr_new;
+ return;
+ }
+}
+
static int claw_probe(struct hid_device *hdev, u8 ep)
{
+ struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+ struct usb_device *udev = interface_to_usbdev(intf);
struct claw_drvdata *drvdata;
int ret;
@@ -523,10 +957,20 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
drvdata->hdev = hdev;
drvdata->ep = ep;
+ /* Determine feature level from firmware version */
+ drvdata->bcd_device = le16_to_cpu(udev->descriptor.bcdDevice);
+ claw_features_supported(drvdata);
+
+ if (!drvdata->bmap_support)
+ dev_dbg(&hdev->dev, "M-Key mapping is not supported. Update firmware to enable.\n");
+
mutex_init(&drvdata->cfg_mutex);
+ mutex_init(&drvdata->profile_mutex);
+ mutex_init(&drvdata->rom_mutex);
spin_lock_init(&drvdata->registration_lock);
spin_lock_init(&drvdata->cmd_lock);
spin_lock_init(&drvdata->mode_lock);
+ spin_lock_init(&drvdata->profile_lock);
init_completion(&drvdata->send_cmd_complete);
INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
--
2.53.0
^ permalink raw reply related
* [PATCH v11 1/4] HID: hid-msi: Add MSI Claw configuration driver
From: Derek J. Clark @ 2026-05-29 7:21 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260529072111.7565-1-derekjohn.clark@gmail.com>
Adds configuration HID driver for the MSI Claw series of handheld PC's.
In this initial patch add the initial driver outline and attributes for
changing the gamepad mode, M-key behavior, and add a WO reset function.
Sending the SWITCH_MODE and RESET commands causes a USB disconnect in
the device. The completion will therefore never get hit and would trigger
an -EIO. To avoid showing the user an error for every write to these
attrs a bypass for the completion handling is introduced when timeout ==
0.
The initial version of this patch was written by Denis Benato, which
contained the initial reverse-engineering and implementation for the
gamepad mode switching. This work was later expanded by Zhouwang Huang
to include more gamepad modes. Finally, I refactored the drivers data
in/out flow and overall format to conform to kernel driver best
practices and style guides. Claude was used as an initial reviewer of
this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Denis Benato <denis.benato@linux.dev>
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v9:
- Don't use devm_device_add_group in cfg_setup_fn, do manual adding
and cleanup to prevent possible use after free.
- Use scoped_guard instead of guard in claw_remove.
- Rename gamepad_registered to gp_registered for brevity.
v8:
- Use spinlock when accessing gamepad_registered.
- Clear state machine on all errors in claw_hw_output_report.
- Wrap all branches under single cmd_lock guard in claw_raw_event.
- Reject generic ACK in claw_raw_event if waiting_cmd is for another
branch.
- Don't close hid devices that couldn't have been opened.
v7:
- Use smp_[store_release|load_acquire] pattern for checking
gamepad_registered to avoid possible races during teardown.
- Reorder reinit_completion in claw_hw_output_report to avoid race
with possible incoming ACKs.
- Reorder cancel_delayed_work_sync to ensure setup can't be re-armed
after cancel.
- Reset command state machine if hw_output_report has an error.
- Add comments to (hopefully) silence sashinko-bot warnings about the
use of endpoint matching and the impossible scenario of switching to
the alternate endpoint from userspace while the driver is bound.
- Don't use spinlock_irqsave when already in irq context.
v6:
- Add send/ack pattern to ensure synchronous acks.
- Use spinlock_irqsave instead of mutex for read/write MODE event
data.
- add select NEW_LEDS to kconfig.
- Make all timeouts 25ms to ensure at least 2 jiffies in a 100Hz
config.
- Gate all attribute show/store functions with gamepad_registered,
enabling use of devm_device_add_group.
- Re-arm cfg_setup in resume if it was canceled in an early suspend.
- Don't set gamepad_mode on resume, MCU preserves state.
- Ensure all count variables are checked for > 0 characters before
setting buf - 1 to \n.
v5:
- Swap disabled & combination mkeys_function enum values.
- Ensure mode_mutex is properly init.
- Ensure claw_remove is calling hid_hw_close and not hid_hw_stop for
all paths.
v4:
- Add msi_suspend/claw_suspend.
- Reorder claw_remove to cancel all work before removing sysfs.
- Add mutex lock for removing sysfs attributes.
- Add mutex lock for MODE command data read/write.
v3:
- Ensure claw_hw_output_report is properly guarded.
- Reoder claw_probe to ensure all mutex, completion, and variable
assignments are in place prior to setting drvdata.
- Ensure gamepad_mode is set to a valid enum value in claw_probe.
v2:
- Rename driver to hid-msi from hid-msi-claw.
- Rename reusable/generic functions to msi_* from claw_*, retaining
claw specific functions.
- Add generic entrypoints for probe, remove, and raw event that route
to claw specific functions.
---
MAINTAINERS | 6 +
drivers/hid/Kconfig | 13 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 5 +
drivers/hid/hid-msi.c | 710 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 735 insertions(+)
create mode 100644 drivers/hid/hid-msi.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 6f6517bf4f970..8e2de98b768f7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17965,6 +17965,12 @@ S: Odd Fixes
F: Documentation/devicetree/bindings/net/ieee802154/mrf24j40.txt
F: drivers/net/ieee802154/mrf24j40.c
+MSI HID DRIVER
+M: Derek J. Clark <derekjohn.clark@gmail.com>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/hid/hid-msi.c
+
MSI EC DRIVER
M: Nikita Kravets <teackot@gmail.com>
L: platform-driver-x86@vger.kernel.org
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 10c12d8e65579..7766676051a52 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -492,6 +492,19 @@ config HID_GT683R
Currently the following devices are know to be supported:
- MSI GT683R
+config HID_MSI
+ tristate "MSI Claw Gamepad Support"
+ depends on USB_HID
+ select NEW_LEDS
+ select LEDS_CLASS
+ select LEDS_CLASS_MULTICOLOR
+ help
+ Support for the MSI Claw RGB and controller configuration
+
+ Say Y here to include configuration interface support for the MSI Claw Line
+ of Handheld Console Controllers. Say M here to compile this driver as a
+ module. The module will be called hid-msi.
+
config HID_KEYTOUCH
tristate "Keytouch HID devices"
help
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 07dfdb6a49c59..80925a17b059c 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -92,6 +92,7 @@ obj-$(CONFIG_HID_MAYFLASH) += hid-mf.o
obj-$(CONFIG_HID_MEGAWORLD_FF) += hid-megaworld.o
obj-$(CONFIG_HID_MICROSOFT) += hid-microsoft.o
obj-$(CONFIG_HID_MONTEREY) += hid-monterey.o
+obj-$(CONFIG_HID_MSI) += hid-msi.o
obj-$(CONFIG_HID_MULTITOUCH) += hid-multitouch.o
obj-$(CONFIG_HID_NINTENDO) += hid-nintendo.o
obj-$(CONFIG_HID_NTI) += hid-nti.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 933b7943bdb50..94a9b89dc240a 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1047,7 +1047,12 @@
#define USB_DEVICE_ID_MOZA_R16_R21_2 0x0010
#define USB_VENDOR_ID_MSI 0x1770
+#define USB_VENDOR_ID_MSI_2 0x0db0
#define USB_DEVICE_ID_MSI_GT683R_LED_PANEL 0xff00
+#define USB_DEVICE_ID_MSI_CLAW_XINPUT 0x1901
+#define USB_DEVICE_ID_MSI_CLAW_DINPUT 0x1902
+#define USB_DEVICE_ID_MSI_CLAW_DESKTOP 0x1903
+#define USB_DEVICE_ID_MSI_CLAW_BIOS 0x1904
#define USB_VENDOR_ID_NATIONAL_SEMICONDUCTOR 0x0400
#define USB_DEVICE_ID_N_S_HARMONY 0xc359
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
new file mode 100644
index 0000000000000..3a176f44f2c43
--- /dev/null
+++ b/drivers/hid/hid-msi.c
@@ -0,0 +1,710 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * HID driver for MSI Claw Handheld PC gamepads.
+ *
+ * Provides configuration support for the MSI Claw series of handheld PC
+ * gamepads. Multiple iterations of the device firmware has led to some
+ * quirks for how certain attributes are handled. The original firmware
+ * did not support remapping of the M1 (right) and M2 (left) rear paddles.
+ * Additionally, the MCU RAM address for writing configuration data has
+ * changed twice. Checks are done during probe to enumerate these variances.
+ *
+ * Copyright (c) 2026 Zhouwang Huang <honjow311@gmail.com>
+ * Copyright (c) 2026 Denis Benato <denis.benato@linux.dev>
+ * Copyright (c) 2026 Valve Corporation
+ */
+
+#include <linux/array_size.h>
+#include <linux/cleanup.h>
+#include <linux/completion.h>
+#include <linux/container_of.h>
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/kobject.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pm.h>
+#include <linux/spinlock.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+#include <linux/usb.h>
+#include <linux/workqueue.h>
+
+#include "hid-ids.h"
+
+#define CLAW_OUTPUT_REPORT_ID 0x0f
+#define CLAW_INPUT_REPORT_ID 0x10
+
+#define CLAW_PACKET_SIZE 64
+
+#define CLAW_DINPUT_CFG_INTF_IN 0x82
+#define CLAW_XINPUT_CFG_INTF_IN 0x83
+
+enum claw_command_index {
+ CLAW_COMMAND_TYPE_NONE = 0x00,
+ CLAW_COMMAND_TYPE_READ_PROFILE = 0x04,
+ CLAW_COMMAND_TYPE_READ_PROFILE_ACK = 0x05,
+ CLAW_COMMAND_TYPE_ACK = 0x06,
+ CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA = 0x21,
+ CLAW_COMMAND_TYPE_SYNC_TO_ROM = 0x22,
+ CLAW_COMMAND_TYPE_SWITCH_MODE = 0x24,
+ CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE = 0x26,
+ CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK = 0x27,
+ CLAW_COMMAND_TYPE_RESET_DEVICE = 0x28,
+};
+
+enum claw_gamepad_mode_index {
+ CLAW_GAMEPAD_MODE_XINPUT = 0x01,
+ CLAW_GAMEPAD_MODE_DINPUT = 0x02,
+ CLAW_GAMEPAD_MODE_DESKTOP = 0x04,
+};
+
+static const char * const claw_gamepad_mode_text[] = {
+ [CLAW_GAMEPAD_MODE_XINPUT] = "xinput",
+ [CLAW_GAMEPAD_MODE_DINPUT] = "dinput",
+ [CLAW_GAMEPAD_MODE_DESKTOP] = "desktop",
+};
+
+enum claw_mkeys_function_index {
+ CLAW_MKEY_FUNCTION_MACRO,
+ CLAW_MKEY_FUNCTION_DISABLED,
+ CLAW_MKEY_FUNCTION_COMBO,
+};
+
+static const char * const claw_mkeys_function_text[] = {
+ [CLAW_MKEY_FUNCTION_MACRO] = "macro",
+ [CLAW_MKEY_FUNCTION_DISABLED] = "disabled",
+ [CLAW_MKEY_FUNCTION_COMBO] = "combination",
+};
+
+struct claw_command_report {
+ u8 report_id;
+ u8 padding[2];
+ u8 header_tail;
+ u8 cmd;
+ u8 data[59];
+} __packed;
+
+struct claw_drvdata {
+ /* MCU General Variables */
+ struct completion send_cmd_complete;
+ struct delayed_work cfg_resume;
+ struct delayed_work cfg_setup;
+ spinlock_t registration_lock; /* Lock for registration read/write */
+ struct hid_device *hdev;
+ struct mutex cfg_mutex; /* mutex for synchronous data */
+ bool waiting_for_ack;
+ spinlock_t cmd_lock; /* Lock for cmd data read/write */
+ u8 waiting_cmd;
+ int cmd_status;
+ u8 ep;
+
+ /* Gamepad Variables */
+ enum claw_mkeys_function_index mkeys_function;
+ enum claw_gamepad_mode_index gamepad_mode;
+ spinlock_t mode_lock; /* Lock for mode data read/write */
+ bool gp_registered;
+};
+
+static int get_endpoint_address(struct hid_device *hdev)
+{
+ struct usb_host_endpoint *ep;
+ struct usb_interface *intf;
+
+ intf = to_usb_interface(hdev->dev.parent);
+ ep = intf->cur_altsetting->endpoint;
+ if (ep)
+ return ep->desc.bEndpointAddress;
+
+ return -ENODEV;
+}
+
+static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
+ struct claw_command_report *cmd_rep)
+{
+ if (cmd_rep->data[0] >= ARRAY_SIZE(claw_gamepad_mode_text) ||
+ !claw_gamepad_mode_text[cmd_rep->data[0]] ||
+ cmd_rep->data[1] >= ARRAY_SIZE(claw_mkeys_function_text))
+ return -EINVAL;
+
+ scoped_guard(spinlock, &drvdata->mode_lock) {
+ drvdata->gamepad_mode = cmd_rep->data[0];
+ drvdata->mkeys_function = cmd_rep->data[1];
+ }
+
+ return 0;
+}
+
+static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
+ u8 *data, int size)
+{
+ struct claw_command_report *cmd_rep;
+ int ret = 0;
+
+ if (size != CLAW_PACKET_SIZE)
+ return 0;
+
+ cmd_rep = (struct claw_command_report *)data;
+
+ if (cmd_rep->report_id != CLAW_INPUT_REPORT_ID || cmd_rep->header_tail != 0x3c)
+ return 0;
+
+ dev_dbg(&drvdata->hdev->dev, "Rx data as raw input report: [%*ph]\n",
+ CLAW_PACKET_SIZE, data);
+
+ guard(spinlock)(&drvdata->cmd_lock);
+ switch (cmd_rep->cmd) {
+ case CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK:
+ ret = claw_gamepad_mode_event(drvdata, cmd_rep);
+ if (drvdata->waiting_for_ack &&
+ drvdata->waiting_cmd == CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE) {
+ drvdata->cmd_status = ret;
+ drvdata->waiting_for_ack = false;
+ complete(&drvdata->send_cmd_complete);
+ }
+
+ break;
+ case CLAW_COMMAND_TYPE_ACK:
+ if (drvdata->waiting_cmd == CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE)
+ break;
+ if (drvdata->waiting_for_ack) {
+ drvdata->cmd_status = 0;
+ drvdata->waiting_for_ack = false;
+ complete(&drvdata->send_cmd_complete);
+ }
+ dev_dbg(&drvdata->hdev->dev, "Waiting CMD: %x\n", drvdata->waiting_cmd);
+
+ break;
+ default:
+ dev_dbg(&drvdata->hdev->dev, "Unknown command: %x\n", cmd_rep->cmd);
+ return 0;
+ }
+
+ return ret;
+}
+
+static int msi_raw_event(struct hid_device *hdev, struct hid_report *report,
+ u8 *data, int size)
+{
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata || (drvdata->ep != CLAW_XINPUT_CFG_INTF_IN &&
+ drvdata->ep != CLAW_DINPUT_CFG_INTF_IN))
+ return 0;
+
+ return claw_raw_event(drvdata, report, data, size);
+}
+
+static int claw_hw_output_report(struct hid_device *hdev, u8 index, u8 *data,
+ size_t len, unsigned int timeout)
+{
+ unsigned char *dmabuf __free(kfree) = NULL;
+ u8 header[] = { CLAW_OUTPUT_REPORT_ID, 0, 0, 0x3c, index };
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ size_t header_size = ARRAY_SIZE(header);
+ int ret;
+
+ if (header_size + len > CLAW_PACKET_SIZE)
+ return -EINVAL;
+
+ /* We can't use a devm_alloc reusable buffer without side effects during suspend */
+ dmabuf = kzalloc(CLAW_PACKET_SIZE, GFP_KERNEL);
+ if (!dmabuf)
+ return -ENOMEM;
+
+ memcpy(dmabuf, header, header_size);
+ if (data && len)
+ memcpy(dmabuf + header_size, data, len);
+
+ guard(mutex)(&drvdata->cfg_mutex);
+ if (timeout) {
+ reinit_completion(&drvdata->send_cmd_complete);
+ scoped_guard(spinlock_irqsave, &drvdata->cmd_lock) {
+ drvdata->waiting_cmd = index;
+ drvdata->waiting_for_ack = true;
+ drvdata->cmd_status = -ETIMEDOUT;
+ }
+ }
+
+ dev_dbg(&hdev->dev, "Send data as raw output report: [%*ph]\n",
+ CLAW_PACKET_SIZE, dmabuf);
+
+ ret = hid_hw_output_report(hdev, dmabuf, CLAW_PACKET_SIZE);
+ if (ret < 0)
+ goto err_clear_state;
+
+ ret = ret == CLAW_PACKET_SIZE ? 0 : -EIO;
+ if (ret)
+ goto err_clear_state;
+
+ if (timeout) {
+ ret = wait_for_completion_interruptible_timeout(&drvdata->send_cmd_complete,
+ msecs_to_jiffies(timeout));
+
+ dev_dbg(&hdev->dev, "Remaining timeout: %u\n", ret);
+ ret = ret > 0 ? drvdata->cmd_status : ret ?: -EBUSY;
+ }
+
+err_clear_state:
+ if (timeout) {
+ guard(spinlock_irqsave)(&drvdata->cmd_lock);
+ drvdata->waiting_cmd = CLAW_COMMAND_TYPE_NONE;
+ drvdata->waiting_for_ack = false;
+ }
+
+ return ret;
+}
+
+static ssize_t gamepad_mode_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int i, ret = -EINVAL;
+ u8 data[2];
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gp_registered))
+ return -ENODEV;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(claw_gamepad_mode_text); i++) {
+ if (claw_gamepad_mode_text[i] && sysfs_streq(buf, claw_gamepad_mode_text[i])) {
+ ret = i;
+ break;
+ }
+ }
+ if (ret < 0)
+ return ret;
+
+ data[0] = ret;
+ scoped_guard(spinlock_irqsave, &drvdata->mode_lock)
+ data[1] = drvdata->mkeys_function;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data, ARRAY_SIZE(data), 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t gamepad_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret, i;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gp_registered))
+ return -ENODEV;
+ }
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE, NULL, 0, 25);
+ if (ret)
+ return ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->mode_lock)
+ i = drvdata->gamepad_mode;
+
+ if (!claw_gamepad_mode_text[i] || claw_gamepad_mode_text[i][0] == '\0')
+ return sysfs_emit(buf, "unsupported\n");
+
+ return sysfs_emit(buf, "%s\n", claw_gamepad_mode_text[i]);
+}
+static DEVICE_ATTR_RW(gamepad_mode);
+
+static ssize_t gamepad_mode_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ ssize_t count = 0;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(claw_gamepad_mode_text); i++) {
+ if (!claw_gamepad_mode_text[i] || claw_gamepad_mode_text[i][0] == '\0')
+ continue;
+ count += sysfs_emit_at(buf, count, "%s ", claw_gamepad_mode_text[i]);
+ }
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(gamepad_mode_index);
+
+static ssize_t mkeys_function_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int i, ret = -EINVAL;
+ u8 data[2];
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gp_registered))
+ return -ENODEV;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(claw_mkeys_function_text); i++) {
+ if (claw_mkeys_function_text[i] && sysfs_streq(buf, claw_mkeys_function_text[i])) {
+ ret = i;
+ break;
+ }
+ }
+ if (ret < 0)
+ return ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->mode_lock)
+ data[0] = drvdata->gamepad_mode;
+ data[1] = ret;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data, ARRAY_SIZE(data), 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t mkeys_function_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret, i;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gp_registered))
+ return -ENODEV;
+ }
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE, NULL, 0, 25);
+ if (ret)
+ return ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->mode_lock)
+ i = drvdata->mkeys_function;
+
+ if (i >= ARRAY_SIZE(claw_mkeys_function_text))
+ return sysfs_emit(buf, "unsupported\n");
+
+ return sysfs_emit(buf, "%s\n", claw_mkeys_function_text[i]);
+}
+static DEVICE_ATTR_RW(mkeys_function);
+
+static ssize_t mkeys_function_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, count = 0;
+
+ for (i = 0; i < ARRAY_SIZE(claw_mkeys_function_text); i++)
+ count += sysfs_emit_at(buf, count, "%s ", claw_mkeys_function_text[i]);
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(mkeys_function_index);
+
+static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ bool val;
+ int ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gp_registered))
+ return -ENODEV;
+ }
+
+ ret = kstrtobool(buf, &val);
+ if (ret)
+ return ret;
+
+ if (!val)
+ return -EINVAL;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_RESET_DEVICE, NULL, 0, 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+static DEVICE_ATTR_WO(reset);
+
+static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribute *attr,
+ int n)
+{
+ struct hid_device *hdev = to_hid_device(kobj_to_dev(kobj));
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata) {
+ dev_warn(&hdev->dev,
+ "Failed to get drvdata from kobj. Gamepad attributes are not available.\n");
+ return 0;
+ }
+
+ return attr->mode;
+}
+
+static struct attribute *claw_gamepad_attrs[] = {
+ &dev_attr_gamepad_mode.attr,
+ &dev_attr_gamepad_mode_index.attr,
+ &dev_attr_mkeys_function.attr,
+ &dev_attr_mkeys_function_index.attr,
+ &dev_attr_reset.attr,
+ NULL,
+};
+
+static const struct attribute_group claw_gamepad_attr_group = {
+ .attrs = claw_gamepad_attrs,
+ .is_visible = claw_gamepad_attr_is_visible,
+};
+
+static void cfg_setup_fn(struct work_struct *work)
+{
+ struct delayed_work *dwork = container_of(work, struct delayed_work, work);
+ struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, cfg_setup);
+ int ret;
+
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE,
+ NULL, 0, 25);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to setup device, can't read gamepad mode: %d\n", ret);
+ return;
+ }
+
+ /* Add sysfs attributes after we get the device state */
+ ret = device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to setup device, can't create gamepad attrs: %d\n", ret);
+ return;
+ }
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock)
+ /* Pairs with smp_load_acquire in attribute show/store functions */
+ smp_store_release(&drvdata->gp_registered, true);
+
+ kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
+}
+
+static void cfg_resume_fn(struct work_struct *work)
+{
+ struct delayed_work *dwork = container_of(work, struct delayed_work, work);
+ struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, cfg_resume);
+
+ guard(spinlock_irqsave)(&drvdata->registration_lock);
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gp_registered))
+ schedule_delayed_work(&drvdata->cfg_setup, msecs_to_jiffies(500));
+}
+
+static int claw_probe(struct hid_device *hdev, u8 ep)
+{
+ struct claw_drvdata *drvdata;
+ int ret;
+
+ drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
+ if (!drvdata)
+ return -ENOMEM;
+
+ drvdata->gamepad_mode = CLAW_GAMEPAD_MODE_XINPUT;
+ drvdata->hdev = hdev;
+ drvdata->ep = ep;
+
+ mutex_init(&drvdata->cfg_mutex);
+ spin_lock_init(&drvdata->registration_lock);
+ spin_lock_init(&drvdata->cmd_lock);
+ spin_lock_init(&drvdata->mode_lock);
+ init_completion(&drvdata->send_cmd_complete);
+ INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
+ INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
+
+ /* For control interface: open the HID transport for sending commands. */
+ ret = hid_hw_open(hdev);
+ if (ret)
+ return ret;
+
+ hid_set_drvdata(hdev, drvdata);
+ schedule_delayed_work(&drvdata->cfg_setup, msecs_to_jiffies(500));
+
+ return 0;
+}
+
+static int msi_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ int ret;
+ u8 ep;
+
+ if (!hid_is_usb(hdev)) {
+ ret = -ENODEV;
+ goto err_probe;
+ }
+
+ ret = hid_parse(hdev);
+ if (ret)
+ goto err_probe;
+
+ /* Set quirk to create separate input devices per HID application */
+ hdev->quirks |= HID_QUIRK_INPUT_PER_APP | HID_QUIRK_MULTI_INPUT;
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (ret)
+ goto err_probe;
+
+ /* For non-control interfaces (keyboard/mouse), allow userspace to grab the devices. */
+ ret = get_endpoint_address(hdev);
+ if (ret < 0)
+ goto err_stop_hw;
+
+ ep = ret;
+ if (ep == CLAW_XINPUT_CFG_INTF_IN || ep == CLAW_DINPUT_CFG_INTF_IN) {
+ ret = claw_probe(hdev, ep);
+ if (ret)
+ goto err_stop_hw;
+ }
+
+ return 0;
+
+err_stop_hw:
+ hid_hw_stop(hdev);
+err_probe:
+ return dev_err_probe(&hdev->dev, ret, "Failed to init device\n");
+}
+
+static void claw_remove(struct hid_device *hdev)
+{
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ bool gp_registered;
+
+ if (!drvdata)
+ return;
+
+ cancel_delayed_work_sync(&drvdata->cfg_resume);
+ cancel_delayed_work_sync(&drvdata->cfg_setup);
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ gp_registered = smp_load_acquire(&drvdata->gp_registered);
+ /* Pairs with smp_load_acquire in attribute show/store functions */
+ smp_store_release(&drvdata->gp_registered, false);
+ }
+
+ if (gp_registered)
+ device_remove_group(&hdev->dev, &claw_gamepad_attr_group);
+
+ hid_hw_close(hdev);
+}
+
+static void msi_remove(struct hid_device *hdev)
+{
+ int ret;
+ u8 ep;
+
+ /* Safe assumption. SET_INTERFACE ioctl can't be used while driver is bound */
+ ret = get_endpoint_address(hdev);
+ if (ret <= 0)
+ goto hw_stop;
+
+ ep = ret;
+ if (ep == CLAW_XINPUT_CFG_INTF_IN || ep == CLAW_DINPUT_CFG_INTF_IN)
+ claw_remove(hdev);
+
+hw_stop:
+ hid_hw_stop(hdev);
+}
+
+static int claw_resume(struct hid_device *hdev)
+{
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata)
+ return -ENODEV;
+
+ /* MCU can take up to 500ms to be ready after resume */
+ schedule_delayed_work(&drvdata->cfg_resume, msecs_to_jiffies(500));
+ return 0;
+}
+
+static int msi_resume(struct hid_device *hdev)
+{
+ int ret;
+ u8 ep;
+
+ /* Safe assumption. SET_INTERFACE ioctl can't be used while driver is bound */
+ ret = get_endpoint_address(hdev);
+ if (ret <= 0)
+ return 0;
+
+ ep = ret;
+ if (ep == CLAW_XINPUT_CFG_INTF_IN || ep == CLAW_DINPUT_CFG_INTF_IN)
+ return claw_resume(hdev);
+
+ return 0;
+}
+
+static int claw_suspend(struct hid_device *hdev)
+{
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata)
+ return -ENODEV;
+
+ cancel_delayed_work_sync(&drvdata->cfg_resume);
+ cancel_delayed_work_sync(&drvdata->cfg_setup);
+
+ return 0;
+}
+
+static int msi_suspend(struct hid_device *hdev, pm_message_t msg)
+{
+ int ret;
+ u8 ep;
+
+ /* Safe assumption. SET_INTERFACE ioctl can't be used while driver is bound */
+ ret = get_endpoint_address(hdev);
+ if (ret <= 0)
+ return 0;
+
+ ep = ret;
+ if (ep == CLAW_XINPUT_CFG_INTF_IN || ep == CLAW_DINPUT_CFG_INTF_IN)
+ return claw_suspend(hdev);
+
+ return 0;
+}
+
+static const struct hid_device_id msi_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI_2, USB_DEVICE_ID_MSI_CLAW_XINPUT) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI_2, USB_DEVICE_ID_MSI_CLAW_DINPUT) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI_2, USB_DEVICE_ID_MSI_CLAW_DESKTOP) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI_2, USB_DEVICE_ID_MSI_CLAW_BIOS) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, msi_devices);
+
+static struct hid_driver msi_driver = {
+ .name = "hid-msi",
+ .id_table = msi_devices,
+ .raw_event = msi_raw_event,
+ .probe = msi_probe,
+ .remove = msi_remove,
+ .resume = msi_resume,
+ .suspend = pm_ptr(msi_suspend),
+};
+module_hid_driver(msi_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Denis Benato <denis.benato@linux.dev>");
+MODULE_AUTHOR("Zhouwang Huang <honjow311@gmail.com>");
+MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>");
+MODULE_DESCRIPTION("HID driver for MSI Claw Handheld PC gamepads");
--
2.53.0
^ permalink raw reply related
* [PATCH v11 0/4] Add MSI Claw HID Configuration Driver
From: Derek J. Clark @ 2026-05-29 7:21 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
This series adds an HID Configuration driver for the MSI Claw line of
Handheld Gaming PC's. The MSI Claw HID interface provides multiple
features, such as the ability to switch between xinput, dinput, and a
desktop mode, RGB control, rumble intensity, and mapping of the rear "M"
keys. There are additional gamepad modes that are not included in this
driver as they appear to be used in assembly line testing or are
incomplete in the firmware. During my testing I found them to be unstable.
The initial version of this driver was written by Denis Benato, which
contained the initial reverse-engineering and implementation for the
gamepad mode switching. This work was later expanded by Zhouwang Huang
to include more gamepad modes and additional features. Finally, I
refactored the entire driver, fixed multiple bugs, and refined the overall
format to conform to kernel driver best practices and style guide.
Claude was used initially by Zhouwang Huang to quickly parse HID captures
during the reverse-engineering of some of the features. Since Claude had
already been used, as a test of its capabilities I had it implement the
rumble intensity attribute after I had already rewritten most of the
driver, which I then manually edited to fix some mistakes. I also used
Claude to review the driver and these patches for any mistakes and bugs.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Denis Benato <denis.benato@linux.dev>
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v11:
- Restore dropped changes from v10.
v10: https://lore.kernel.org/linux-input/20260527222122.10620-1-derekjohn.clark@gmail.com/
- Remove additional gamepad_registered variable left over after rename
to gp_registered.
v9: https://lore.kernel.org/linux-input/20260525020543.519082-1-derekjohn.clark@gmail.com/
- Don't use devm_* functions in cfg_setup_fn, do manual adding
and cleanup to prevent possible use after free.
- Use scoped_guard instead of guard in claw_remove.
- Rename gamepad_registered to gp_registered for brevity.
- Check for drvdata in rgb_queue_fn to avoid use after free during
teardown.
- Ensure rgb_queue work is canceled during suspend.
- Limit guard usage in cfg_setup_fn to avoid holding a lock during
registration and add group events.
v8: https://lore.kernel.org/linux-input/20260522015518.1111290-1-derekjohn.clark@gmail.com/
- Use spinlock when accessing gamepad_registered.
- Clear state machine on all errors in claw_hw_output_report.
- Wrap all branches under single cmd_lock guard in claw_raw_event.
- Reject generic ACK in claw_raw_event if waiting_cmd is for another
branch.
- Wrap all branches under single cmd_lock guard in claw_raw_event.
- Reject generic ACK in claw_raw_event if waiting_cmd is for another
branch.
- Don't close hid devices that couldn't have been opened.
- Ensure led_classdev is unregistered if adding attribute group fails.
- Reorder remove actions to ensure no use-after free or rearming cleared
flags.
v7: https://lore.kernel.org/linux-input/20260520013158.3633277-1-derekjohn.clark@gmail.com/
- Use smp_[store_release|load_acquire] pattern for checking
gamepad_registered and rgb_registered to avoid possible races during
teardown.
- Reorder reinit_completion in claw_hw_output_report to avoid race
with possible incoming ACKs.
- Reorder cancel_delayed_work_sync to ensure setup can't be re-armed
after cancel.
- Reset command state machine if hw_output_report has an error.
- Add comments to (hopefully) silence sashinko-bot warnings about the
use of endpoint matching and the impossible scenario of switching to
the alternate endpoint from userspace while the driver is bound.
- Don't use spinlock_irqsave when already in irq context.
- Add profile_lock for read/write profile_pending.
- Use struct for mkey reports and rumble reports, following the
pattern established by rgb reports previously.
- Add gating to cfg_setup_fn, allowing either gamepad settings or rgb
settings to populate if the other fails for any reason.
- Match on write address for rumble and mkey reports to prevent late
ACK from causing synchronization errors.
v6: https://lore.kernel.org/linux-input/20260518222935.1802071-1-derekjohn.clark@gmail.com/
- Add send/ack pattern to ensure synchronous acks.
- Use spinlock_irqsave instead of mutex for read/write MODE event
data.
- add select NEW_LEDS to kconfig.
- Make all timeouts 25ms to ensure at least 2 jiffies in a 100Hz
config.
- Gate all attribute show/store functions with gamepad_registered or
rgb_registered, enabling use of devm_device_add_group and ending
the need to hold a mutex during remove.
- Don't set gamepad_mode on resume, MCU preserves state.
- Ensure all count variables are checked for > 0 characters before
setting buf - 1 to \n.
- Re-arm cfg_setup in resume if it was canceled in an early suspend.
- Remove duplicated argv_free macro.
- Add spinlock_irqsave vice mutex for read/write access on attribute
variables.
v5: https://lore.kernel.org/linux-input/20260517013925.3120314-1-derekjohn.clark@gmail.com/
- Swap disabled & combination mkeys_function enum values.
- Fix bug introduced in v5 where claw_buttons_store would return
-EINVAL on all valid key entries.
- Ensure mode_mutex is properly init.
- Ensure claw_remove is calling hid_hw_close and not hid_hw_stop for
all paths.
- Ensure adding "DISABLED" key to valid entries is done in the correct
patch.
- Re-enable sending an empty string to clear button mappings in
addition to setting DISABLED.
- Move adding the RGB device into cfg_setup to prevent led core
attributes from being written to prior to setup completing.
- Ensure frame_lock is properly init.
- Change variable names in RGB functions from frame and zone to f and
z respectively to fit all scoped_guard actions in 100 columns.
v4: https://lore.kernel.org/linux-input/20260516042841.500299-1-derekjohn.clark@gmail.com/
- Add msi_suspend/claw_suspend.
- Reorder claw_remove to cancel all work before removing sysfs.
- Add mutex lock for removing sysfs attributes.
- Add mutex lock for MODE command data read/write.
- Change dev_warn to dev_dbg in claw_profile_event.
- use __free with DEFINE_FREE macro for argv instead of manually
running argv_free, cleaining up scoped_guard goto.
- Fix frame_calc validity check to use >=.
- Use spinlock instead of mutex in raw_event and related attribute
_store function.
- Ensure delayed work is canceled in suspend & canceled before sysfs
attribute removal.
v3: https://lore.kernel.org/linux-input/20260515033622.2095277-1-derekjohn.clark@gmail.com/
- Add mutex for read/write if rgb frame data.
- Ensure claw_hw_output_report is properly guarded.
- Remove setting rgb_frame_count when reading rgb profiles as it always
returns garbage data.
- Ensure rgb_speed is getting drvdata from a valid lookup (not hdev).
- Use scoped_guard where necessary.
- Reoder claw_probe to ensure all mutex, completion, and variable
assignments are in place prior to setting drvdata.
- Ensure gamepad_mode is set to a valid enum value in claw_probe.
v2: https://lore.kernel.org/linux-input/20260513231445.3213501-1-derekjohn.clark@gmail.com/
- Use mutexes to guard SYNC_TO_ROM calls and pending_profile calls.
- Rename driver to hid-msi and add generic entrypoints for
probe/resume/remove that call claw specific functions in order to
future proof the driver for other MSI HID interfaces.
- Fix various bugs and formatting issues.
v1: https://lore.kernel.org/linux-input/20260510043510.442807-1-derekjohn.clark@gmail.com/
Derek J. Clark (4):
HID: hid-msi: Add MSI Claw configuration driver
HID: hid-msi: Add M-key mapping attributes
HID: hid-msi: Add RGB control interface
HID: hid-msi: Add Rumble Intensity Attributes
MAINTAINERS | 6 +
drivers/hid/Kconfig | 13 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 5 +
drivers/hid/hid-msi.c | 1998 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 2023 insertions(+)
create mode 100644 drivers/hid/hid-msi.c
--
2.53.0
^ permalink raw reply
* Re: [PATCH v10 1/4] HID: hid-msi: Add MSI Claw configuration driver
From: Derek John Clark @ 2026-05-29 6:34 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jiri Kosina, Benjamin Tissoires, Pierre-Loup A . Griffais,
Denis Benato, Zhouwang Huang, linux-input, linux-doc,
linux-kernel
In-Reply-To: <ahfQW54YoHj2Pal_@google.com>
On Wed, May 27, 2026 at 10:32 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hi Derek,
>
> On Wed, May 27, 2026 at 10:21:19PM +0000, Derek J. Clark wrote:
> > Adds configuration HID driver for the MSI Claw series of handheld PC's.
> > In this initial patch add the initial driver outline and attributes for
> > changing the gamepad mode, M-key behavior, and add a WO reset function.
> >
> > Sending the SWITCH_MODE and RESET commands causes a USB disconnect in
> > the device. The completion will therefore never get hit and would trigger
> > an -EIO. To avoid showing the user an error for every write to these
> > attrs a bypass for the completion handling is introduced when timeout ==
> > 0.
> >
> > The initial version of this patch was written by Denis Benato, which
> > contained the initial reverse-engineering and implementation for the
> > gamepad mode switching. This work was later expanded by Zhouwang Huang
> > to include more gamepad modes. Finally, I refactored the drivers data
> > in/out flow and overall format to conform to kernel driver best
> > practices and style guides. Claude was used as an initial reviewer of
> > this patch.
>
> I wonder why do you need to roll asynchronous probing and asynchronous
> resume by hand? This I think complicates the driver greatly and forces
> you to use a ton of works, spinlocks, and checks.
>
> Thanks.
>
Hi Dmitry,
I suppose being asked this means my cover letter and commit
descriptions need some additional context. The MCU in these Claw
devices is quite temperamental. There are a few specific issues that
cause the need for multiple work queues, a serialization mutex, and
subsequently spinlocks to prevent stale data reads.
1.) The MCU will halt function if it receives any output reports
before ~500MS after probe or resume. This can either manifest as the
device never responding to a command, or it can cause the entire
system to become unstable and reboot. This creates the need for
cfg_setup to query the MCU and then add the gamepad attrs, led_mc
device, and rgb attrs. As a side effect, because a system could
technically be suspended during that 500ms delay, there exists the
need to re-queue the work if it was never triggered, hence the resume
queue.
2.) The MCU will not always respond in order if two or more output
reports are sent within a few ms of each other. Since many of the
commands use a generic "ACK", or share an "ACK" type but don't provide
specific context about what sub-function called them, we could
potentially have cross talk where data is saved in the wrong attribute
or errors propagate because of a missed message. To get around this
serialization issue we hold a mutex through a completion triggered in
raw_event and, for most events, save a state machine on what command
is expected and what sub-command was the initiator. (I.E. profile
events handle the M1, M2, RGB, Left rumble, and right rumble). Since
the state machine is accessed on both sides, we need spinlocks
guarding the reads. This essentially serializes the data and makes it
predictable. Using this pattern I haven't had any issues reading from
or writing to the MCU.
3.) Some commands will never return their "ACK" while a completion is
held, so we have a workaround to basically ignore them and hope the
command worked. This is only needed for SYNC_TO_ROM, from which we
don't need to set anything on its "ACK", and switching the gamepad
mode, which causes USB disconnect/reconnect and the driver fully
reloads, so we'll never be able to read it anyway.
4.) The RGB work queue is used to free the userspace write while the
completion is held. I found that use without it could stall userspace
quite significantly if it has multiple writes back to back. I
experienced this using Steam's customization menu, which sends a
single write for every increment of its color and brightness sliders.
when traversing the full length of the slider it is possible to have
effects changing for nearly a minute after stopping. With the queue,
only the most recent write is eventually sent to the MCU. This issue
also affects the Go 2 driver as well, though not to the same extent,
but for which I'll be adding a similar de-bounce queue soon. Go S is
also technically affected by this bug, but that returns quickly enough
that it isn't really feasible to trigger the bug with much frequency.
I'll still fix that one as well though.
TBH I'm not "happy" with the complexity of the driver, but I don't see
a reasonable alternative. If you have any specific suggestions that I
could try that might simplify it, I'd be more than happy to give it a
shot. That being said, I'm not very optimistic about it. Development
on this device has been like wrestling a bear.
- Derek
> --
> Dmitry
^ permalink raw reply
* Re: [PATCH v4 0/5] selftests/mm: separate GUP microbenchmarking from functional testing
From: Sarthak Sharma @ 2026-05-29 6:31 UTC (permalink / raw)
To: Dev Jain, Andrew Morton
Cc: David Hildenbrand, Lorenzo Stoakes, Liam R . Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Shuah Khan, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Barry Song, Lance Yang, Jason Gunthorpe, John Hubbard, Peter Xu,
Leon Romanovsky, Jonathan Corbet, Shuah Khan, Mark Brown,
linux-mm, linux-kselftest, linux-doc, linux-kernel
In-Reply-To: <403181bc-eab0-4f4a-b986-dab1d8c49bf3@arm.com>
Hi Dev!
On 5/29/26 11:36 AM, Dev Jain wrote:
>
>
> On 29/05/26 11:27 am, Sarthak Sharma wrote:
>> Hi Andrew!
>>
>> On 5/28/26 12:17 AM, Andrew Morton wrote:
>>> On Wed, 27 May 2026 19:54:27 +0530 Sarthak Sharma <sarthak.sharma@arm.com> wrote:
>>>
>>>> gup_test.c currently serves two distinct purposes: microbenchmarking
>>>> (GUP_FAST_BENCHMARK, PIN_FAST_BENCHMARK, PIN_LONGTERM_BENCHMARK) and
>>>> functional correctness testing (GUP_BASIC_TEST, PIN_BASIC_TEST,
>>>> DUMP_USER_PAGES_TEST). Mixing these in a single binary means functional
>>>> tests cannot be run or reported individually and run_vmtests.sh must
>>>> invoke the binary multiple times with different flag combinations to
>>>> cover all configurations.
>>>>
>>>> This patch series separates the two concerns: tools/mm/gup_bench for
>>>> benchmarking and tools/testing/selftests/mm/gup_test for functional
>>>> testing. To avoid duplicating HugeTLB and related file helpers, the
>>>> series first prepares the existing file helpers for sharing, then moves
>>>> the common helper code to tools/lib/mm/ so it can be shared by both
>>>> selftests and tools/mm.
>>>
>>> Thanks. I'll duck this for now, see what reviewers have to say.
>>>
>>> Sashiko still has a couple of nags. Minor stuff, arguably ignorable.
>>> https://sashiko.dev/#/patchset/20260527142432.230127-1-sarthak.sharma@arm.com
>>
>> Thanks! I went through Sashiko's review.
>>
>> Both the points are valid but look very minor to me: one is a temporary
>> x86 build issue fixed by the next patch and the other is stricter
>
> For the sake of bisection, we try to make patches such that each patch
> when applied one by one, does not break the build. Sashiko says that
> applying this patch breaks mm-selftests build for x86, so this needs
> to be fixed : )
>
> Although if there is no other major review comment, instead of respinning
> you could simply reply to this patch with a fix patch and Andrew can
> incorporate that : )
Thanks, that makes sense.
I'll wait for reviewer feedback for now. If no major comment comes in,
I'll send a fix patch to move the x86 Makefile change from patch 3 to
patch 2.
>
>> argument validation for -F in gup_bench.
>
>>
>> I don't feel they require a v5 on their own. Let's wait for reviewer
>> feedback. If a respin is needed then I'll fold these cleanups in as well.
>>
>
^ permalink raw reply
* [PATCH v2] arm64: kernel: Unify CNP disable workaround into ARM64_WORKAROUND_DISABLE_CNP
From: Zeng Heng @ 2026-05-29 6:31 UTC (permalink / raw)
To: vladimir.murzin, xuwei5, wangyushan12, maz, skhan,
miko.lenczewski, lucaswei, broonie, thuth, ryan.roberts,
tongtiangen, oupton, kuninori.morimoto.gx, mark.rutland, will,
corbet, catalin.marinas, kevin.brodsky, lpieralisi, yangyicong,
james.clark, yeoreum.yun
Cc: linux-doc, wangkefeng.wang, linux-arm-kernel, linux-kernel
From: Zeng Heng <zengheng4@huawei.com>
HiSilicon HIP09 implements TLB entry matching behavior that deviates
from the ARM architecture specification when the CnP (Common not Private)
bit is set in TTBRx_ELx.
When TTBRx.CNP=1, TLB entries may be incorrectly shared between CPU
cores, leading to TLB conflicts and stale mappings. This breaks
coherency and can result in incorrect translations.
Add the hardware erratum workaround (Hisilicon erratum 162100125) to
disable CNP on affected HIP09 cores.
Merge the existing NVIDIA Carmel and the HiSilicon HIP09 CNP errata
workarounds into a single generic capability ARM64_WORKAROUND_DISABLE_CNP.
Both NVIDIA Carmel and HiSilicon HIP09 have hardware errata where
CNP (Common Not Private) behavior differs from the ARM specification,
causing incorrect TLB entry sharing between cores. The existing
NVIDIA_CARMEL_CNP_ERRATUM and the newly added HISILICON_ERRATUM_162100125
are now both handled by the unified ARM64_WORKAROUND_DISABLE_CNP.
Co-developed-by: Tong Tiangen <tongtiangen@huawei.com>
Signed-off-by: Tong Tiangen <tongtiangen@huawei.com>
Signed-off-by: Zeng Heng <zengheng4@huawei.com>
---
v1: https://lore.kernel.org/all/20260526015720.206854-1-zengheng@huaweicloud.com/
Changes in v2:
- Unify CNP disable workaround into ARM64_WORKAROUND_DISABLE_CNP
---
Documentation/arch/arm64/silicon-errata.rst | 4 +++-
arch/arm64/Kconfig | 17 ++++++++++++-----
arch/arm64/include/asm/cpucaps.h | 4 ++--
arch/arm64/kernel/cpu_errata.c | 17 ++++++++++++-----
arch/arm64/kernel/cpufeature.c | 2 +-
arch/arm64/tools/cpucaps | 2 +-
6 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
index 211119ce7adc..b4565e1a726d 100644
--- a/Documentation/arch/arm64/silicon-errata.rst
+++ b/Documentation/arch/arm64/silicon-errata.rst
@@ -254,7 +254,7 @@ stable kernels.
| Marvell | ARM-MMU-500 | #582743 | N/A |
+----------------+-----------------+-----------------+-----------------------------+
+----------------+-----------------+-----------------+-----------------------------+
-| NVIDIA | Carmel Core | N/A | NVIDIA_CARMEL_CNP_ERRATUM |
+| NVIDIA | Carmel Core | N/A | ARM64_WORKAROUND_DISABLE_CNP|
+----------------+-----------------+-----------------+-----------------------------+
| NVIDIA | T241 GICv3/4.x | T241-FABRIC-4 | N/A |
+----------------+-----------------+-----------------+-----------------------------+
@@ -284,6 +284,8 @@ stable kernels.
+----------------+-----------------+-----------------+-----------------------------+
| Hisilicon | Hip09 | #162100801 | HISILICON_ERRATUM_162100801 |
+----------------+-----------------+-----------------+-----------------------------+
+| Hisilicon | Hip09 | #162100125 | ARM64_WORKAROUND_DISABLE_CNP|
++----------------+-----------------+-----------------+-----------------------------+
+----------------+-----------------+-----------------+-----------------------------+
| Qualcomm Tech. | Kryo/Falkor v1 | E1003 | QCOM_FALKOR_ERRATUM_1003 |
+----------------+-----------------+-----------------+-----------------------------+
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index fe60738e5943..dc0bd32ea2d1 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1315,13 +1315,20 @@ config QCOM_FALKOR_ERRATUM_E1041
If unsure, say Y.
-config NVIDIA_CARMEL_CNP_ERRATUM
- bool "NVIDIA Carmel CNP: CNP on Carmel semantically different than ARM cores"
+config ARM64_WORKAROUND_DISABLE_CNP
+ bool "Disable CNP on affected CPUs"
default y
help
- If CNP is enabled on Carmel cores, non-sharable TLBIs on a core will not
- invalidate shared TLB entries installed by a different core, as it would
- on standard ARM cores.
+ This option disables the CNP (Common Not Private) feature on CPUs
+ that have hardware errata affecting CNP behavior.
+
+ On NVIDIA Carmel cores, CNP behaves differently than on standard ARM
+ cores: non-shareable TLBIs on a core may not invalidate shared TLB
+ entries installed by a different core.
+
+ On Hisilicon HIP09 cores, TLB entries may be incorrectly shared
+ between cores when TTBRx.CNP=1, leading to TLB conflicts and
+ stale mappings.
If unsure, say Y.
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index d0d3cdd5763c..25c61cda901c 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -58,8 +58,8 @@ cpucap_is_possible(const unsigned int cap)
return IS_ENABLED(CONFIG_ARM64_ERRATUM_2658417);
case ARM64_WORKAROUND_CAVIUM_23154:
return IS_ENABLED(CONFIG_CAVIUM_ERRATUM_23154);
- case ARM64_WORKAROUND_NVIDIA_CARMEL_CNP:
- return IS_ENABLED(CONFIG_NVIDIA_CARMEL_CNP_ERRATUM);
+ case ARM64_WORKAROUND_DISABLE_CNP:
+ return IS_ENABLED(CONFIG_ARM64_WORKAROUND_DISABLE_CNP);
case ARM64_WORKAROUND_REPEAT_TLBI:
return IS_ENABLED(CONFIG_ARM64_WORKAROUND_REPEAT_TLBI);
case ARM64_WORKAROUND_SPECULATIVE_SSBS:
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 5377e4c2eba2..675cd059165c 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -394,6 +394,14 @@ static const struct arm64_cpu_capabilities qcom_erratum_1003_list[] = {
};
#endif
+#ifdef CONFIG_ARM64_WORKAROUND_DISABLE_CNP
+static const struct midr_range cnp_erratum_cpus[] = {
+ MIDR_ALL_VERSIONS(MIDR_NVIDIA_CARMEL),
+ MIDR_ALL_VERSIONS(MIDR_HISI_HIP09),
+ {},
+};
+#endif
+
#ifdef CONFIG_ARM64_WORKAROUND_CLEAN_CACHE
static const struct midr_range workaround_clean_cache[] = {
#if defined(CONFIG_ARM64_ERRATUM_826319) || \
@@ -801,12 +809,11 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
1, 0),
},
#endif
-#ifdef CONFIG_NVIDIA_CARMEL_CNP_ERRATUM
+#ifdef CONFIG_ARM64_WORKAROUND_DISABLE_CNP
{
- /* NVIDIA Carmel */
- .desc = "NVIDIA Carmel CNP erratum",
- .capability = ARM64_WORKAROUND_NVIDIA_CARMEL_CNP,
- ERRATA_MIDR_ALL_VERSIONS(MIDR_NVIDIA_CARMEL),
+ .desc = "NVIDIA Carmel CNP erratum, or Hisilicon erratum 162100125",
+ .capability = ARM64_WORKAROUND_DISABLE_CNP,
+ ERRATA_MIDR_RANGE_LIST(cnp_erratum_cpus),
},
#endif
#ifdef CONFIG_ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 6d53bb15cf7b..20c5f24f74a9 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1785,7 +1785,7 @@ has_useable_cnp(const struct arm64_cpu_capabilities *entry, int scope)
if (is_kdump_kernel())
return false;
- if (cpus_have_cap(ARM64_WORKAROUND_NVIDIA_CARMEL_CNP))
+ if (cpus_have_cap(ARM64_WORKAROUND_DISABLE_CNP))
return false;
return has_cpuid_feature(entry, scope);
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index 811c2479e82d..9b85a84f6fd4 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -120,7 +120,7 @@ WORKAROUND_CAVIUM_TX2_219_PRFM
WORKAROUND_CAVIUM_TX2_219_TVM
WORKAROUND_CLEAN_CACHE
WORKAROUND_DEVICE_LOAD_ACQUIRE
-WORKAROUND_NVIDIA_CARMEL_CNP
+WORKAROUND_DISABLE_CNP
WORKAROUND_PMUV3_IMPDEF_TRAPS
WORKAROUND_QCOM_FALKOR_E1003
WORKAROUND_QCOM_ORYON_CNTVOFF
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 0/3] AF_ALG: Remove support for AIO and old-style drivers
From: Herbert Xu @ 2026-05-29 6:09 UTC (permalink / raw)
To: demiobenour
Cc: David S. Miller, Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni,
Willem de Bruijn, Jens Axboe, Jakub Kicinski, Simon Horman,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, James Clark, Jonathan Corbet,
Shuah Khan, Eric Biggers, Ard Biesheuvel, linux-crypto,
linux-kernel, io-uring, netdev, linux-perf-users, linux-doc
In-Reply-To: <20260523-af-alg-harden-v1-0-c76755c3a5c5@gmail.com>
On Sat, May 23, 2026 at 03:43:01PM -0400, Demi Marie Obenour via B4 Relay wrote:
> AF_ALG is a deprecated API only useful for compatibility with existing
> userspace. It has had a lot of vulnerabilities, including the infamous
> CopyFail.
>
> Rip out support for offload drivers, which tend to be buggy. Also rip
> out support for AIO, which actually bloats the entire socket subsystem.
>
> Only compile-tested.
>
> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
> ---
> Demi Marie Obenour (3):
> net: Remove support for AIO on sockets
> AF_ALG: Drop support for off-CPU cryptography
> AF_ALG: Document that it is *always* slower
>
> Documentation/crypto/userspace-if.rst | 26 ++++++++--
> crypto/af_alg.c | 35 ++------------
> crypto/algif_aead.c | 43 ++++-------------
> crypto/algif_hash.c | 4 +-
> crypto/algif_rng.c | 4 +-
> crypto/algif_skcipher.c | 66 ++++++--------------------
> include/crypto/if_alg.h | 19 ++++++--
> include/linux/socket.h | 1 -
> io_uring/net.c | 1 -
> net/compat.c | 1 -
> net/socket.c | 7 +--
> tools/perf/trace/beauty/include/linux/socket.h | 1 -
> 12 files changed, 70 insertions(+), 138 deletions(-)
> ---
> base-commit: 49e05bb00f2e8168695f7af4d694c39e1423e8a2
> change-id: 20260502-af-alg-harden-900849451653
All applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v4 0/5] selftests/mm: separate GUP microbenchmarking from functional testing
From: Dev Jain @ 2026-05-29 6:06 UTC (permalink / raw)
To: Sarthak Sharma, Andrew Morton
Cc: David Hildenbrand, Lorenzo Stoakes, Liam R . Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Shuah Khan, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Barry Song, Lance Yang, Jason Gunthorpe, John Hubbard, Peter Xu,
Leon Romanovsky, Jonathan Corbet, Shuah Khan, Mark Brown,
linux-mm, linux-kselftest, linux-doc, linux-kernel
In-Reply-To: <3b4176f4-e099-490c-a5e2-6a36d00b1207@arm.com>
On 29/05/26 11:27 am, Sarthak Sharma wrote:
> Hi Andrew!
>
> On 5/28/26 12:17 AM, Andrew Morton wrote:
>> On Wed, 27 May 2026 19:54:27 +0530 Sarthak Sharma <sarthak.sharma@arm.com> wrote:
>>
>>> gup_test.c currently serves two distinct purposes: microbenchmarking
>>> (GUP_FAST_BENCHMARK, PIN_FAST_BENCHMARK, PIN_LONGTERM_BENCHMARK) and
>>> functional correctness testing (GUP_BASIC_TEST, PIN_BASIC_TEST,
>>> DUMP_USER_PAGES_TEST). Mixing these in a single binary means functional
>>> tests cannot be run or reported individually and run_vmtests.sh must
>>> invoke the binary multiple times with different flag combinations to
>>> cover all configurations.
>>>
>>> This patch series separates the two concerns: tools/mm/gup_bench for
>>> benchmarking and tools/testing/selftests/mm/gup_test for functional
>>> testing. To avoid duplicating HugeTLB and related file helpers, the
>>> series first prepares the existing file helpers for sharing, then moves
>>> the common helper code to tools/lib/mm/ so it can be shared by both
>>> selftests and tools/mm.
>>
>> Thanks. I'll duck this for now, see what reviewers have to say.
>>
>> Sashiko still has a couple of nags. Minor stuff, arguably ignorable.
>> https://sashiko.dev/#/patchset/20260527142432.230127-1-sarthak.sharma@arm.com
>
> Thanks! I went through Sashiko's review.
>
> Both the points are valid but look very minor to me: one is a temporary
> x86 build issue fixed by the next patch and the other is stricter
For the sake of bisection, we try to make patches such that each patch
when applied one by one, does not break the build. Sashiko says that
applying this patch breaks mm-selftests build for x86, so this needs
to be fixed : )
Although if there is no other major review comment, instead of respinning
you could simply reply to this patch with a fix patch and Andrew can
incorporate that : )
> argument validation for -F in gup_bench.
>
> I don't feel they require a v5 on their own. Let's wait for reviewer
> feedback. If a respin is needed then I'll fold these cleanups in as well.
>
^ permalink raw reply
* Re: [PATCH linux next] tools headers UAPI: sync linux/taskstats.h for procacct.c
From: Thomas Weißschuh @ 2026-05-29 5:58 UTC (permalink / raw)
To: wang.yaxin
Cc: akpm, fan.yu9, yang.yang29, corbet, linux-kernel, linux-doc,
xu.xin16
In-Reply-To: <20260529124410822pexFmlFsVNGRXq_DUHUmg@zte.com.cn>
Hi!
On 2026-05-29 12:44:10+0800, wang.yaxin@zte.com.cn wrote:
> >> From: Wang Yaxin <wang.yaxin@zte.com.cn>
> >>
> >> Background
> >> ==========
> >> After commit 9b93f7e32774 ("tools/getdelays: use the static UAPI
> >> headers from tools/include/uapi"), the Makefile was changed to use
> >> -I../include/uapi/ instead of -I../../usr/include to ensure tools
> >> always use the up-to-date UAPI headers.
> >>
> >> However, only linux/taskstats.h was added to tools/include/uapi/ in
> >> commit e5bbb35a07b3 ("tools headers UAPI: sync linux/taskstats.h"),
> >> but linux/acct.h was missing.
> >
> >Please let's Cc the author of both of these commits!
>
> Ok
>
> >> Problem
> >> =======
> >> This causes procacct.c to fail to compile with:
> >>
> >> procacct.c:234:37: error: 'AGROUP' undeclared (first use in this function)
> >>
> >> gcc -I../include/uapi/ getdelays.c -o getdelays
> >> gcc -I../include/uapi/ procacct.c -o procacct
> >> procacct.c: In function ‘print_procacct’:
> >> procacct.c:234:37: error: ‘AGROUP’ undeclared (first use in this function)
> >> did you mean ‘NOGROUP’?
> >> 234 | , t->version >= 12 ? (t->ac_flag & AGROUP ? 'P' : 'T') : '?'
> >> | ^~~~~~
> >> | NOGROUP
> >> procacct.c:234:37: note: each undeclared ident
> >>
> >> because procacct.c uses the AGROUP macro defined in linux/acct.h.
AGROUP is somewhat old. When linux/acct.h is not present in
tools/include/uapi the system header will be used. In my case this
symbol existed there, so I didn't see the error.
> >> Solution
> >> ========
> >> Add the missing linux/acct.h to complete the static UAPI header set.
Reviewed-by: Thomas Weißschuh <linux@weissschuh.net>
> >
> >It has always annoyed me that we do this by copying the file. I'm
> >surprised that there isn't a way of using the original file directly.
>
> Using -I../../usr/include avoids duplicate headers and relies on a
> single original copy, but requires make headers_install. This aligns
> with commit 9b93f7e32774 ("tools/getdelays: use the static UAPI headers
> from tools/include/uapi").
This requires the presence of a kernel build. Which is not guaranteed
for tools/. You could put it in samples/ which is part of the regular
kernel build and can depend on up-to-date UAPI headers.
> Using -I../include/uapi/ keeps two separate header copies, which may
> cause inconsistencies when one copy is updated. Can we optimize it
> with symbolic links, or revert to the first solution?
See tools/include/uapi/README for the background of the current
solution. I would stick with it.
diff --git a/tools/include/uapi/linux/acct.h b/tools/include/uapi/linux/acct.h
new file mode 100644
index 000000000000..0e591152aa8a
--- /dev/null
+++ b/tools/include/uapi/linux/acct.h
@@ -0,0 +1,128 @@
(...)
+#ifndef _UAPI_LINUX_ACCT_H
+#define _UAPI_LINUX_ACCT_H
+
+#include <linux/types.h>
+
+#include <asm/param.h>
+#include <asm/byteorder.h>
These headers do not exist in exist in tools/include/uapi/ either.
So we need to rely on them to exist in the system UAPI headers.
This should be fine.
(...)
+struct acct
+{
+ char ac_flag; /* Flags */
+ char ac_version; /* Always set to ACCT_VERSION */
+ /* for binary compatibility back until 2.0 */
+ __u16 ac_uid16; /* LSB of Real User ID */
+ __u16 ac_gid16; /* LSB of Real Group ID */
+ __u16 ac_tty; /* Control Terminal */
+ /* __u32 range means times from 1970 to 2106 */
+ __u32 ac_btime; /* Process Creation Time */
+ comp_t ac_utime; /* User Time */
+ comp_t ac_stime; /* System Time */
+ comp_t ac_etime; /* Elapsed Time */
+ comp_t ac_mem; /* Average Memory Usage */
+ comp_t ac_io; /* Chars Transferred */
+ comp_t ac_rw; /* Blocks Read or Written */
+ comp_t ac_minflt; /* Minor Pagefaults */
+ comp_t ac_majflt; /* Major Pagefaults */
+ comp_t ac_swaps; /* Number of Swaps */
+/* m68k had no padding here. */
+#if !defined(CONFIG_M68K) || !defined(__KERNEL__)
+ __u16 ac_ahz; /* AHZ */
+#endif
Pre-existing issue, unvalidated:
This looks problematic, as on m68k userspace and kernel disagree on
the existence of this field. This should probably use
!defined(__m68k__).
+ __u32 ac_exitcode; /* Exitcode */
+ char ac_comm[ACCT_COMM + 1]; /* Command Name */
+ __u8 ac_etime_hi; /* Elapsed Time MSB */
+ __u16 ac_etime_lo; /* Elapsed Time LSB */
+ __u32 ac_uid; /* Real User ID */
+ __u32 ac_gid; /* Real Group ID */
+};
(...)
Thomas
^ permalink raw reply related
* Re: [PATCH v4 0/5] selftests/mm: separate GUP microbenchmarking from functional testing
From: Sarthak Sharma @ 2026-05-29 5:57 UTC (permalink / raw)
To: Andrew Morton
Cc: David Hildenbrand, Lorenzo Stoakes, Liam R . Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Shuah Khan, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Jason Gunthorpe, John Hubbard,
Peter Xu, Leon Romanovsky, Jonathan Corbet, Shuah Khan,
Mark Brown, linux-mm, linux-kselftest, linux-doc, linux-kernel
In-Reply-To: <20260527114752.cf4dd3e1bda93fb8ebab5a76@linux-foundation.org>
Hi Andrew!
On 5/28/26 12:17 AM, Andrew Morton wrote:
> On Wed, 27 May 2026 19:54:27 +0530 Sarthak Sharma <sarthak.sharma@arm.com> wrote:
>
>> gup_test.c currently serves two distinct purposes: microbenchmarking
>> (GUP_FAST_BENCHMARK, PIN_FAST_BENCHMARK, PIN_LONGTERM_BENCHMARK) and
>> functional correctness testing (GUP_BASIC_TEST, PIN_BASIC_TEST,
>> DUMP_USER_PAGES_TEST). Mixing these in a single binary means functional
>> tests cannot be run or reported individually and run_vmtests.sh must
>> invoke the binary multiple times with different flag combinations to
>> cover all configurations.
>>
>> This patch series separates the two concerns: tools/mm/gup_bench for
>> benchmarking and tools/testing/selftests/mm/gup_test for functional
>> testing. To avoid duplicating HugeTLB and related file helpers, the
>> series first prepares the existing file helpers for sharing, then moves
>> the common helper code to tools/lib/mm/ so it can be shared by both
>> selftests and tools/mm.
>
> Thanks. I'll duck this for now, see what reviewers have to say.
>
> Sashiko still has a couple of nags. Minor stuff, arguably ignorable.
> https://sashiko.dev/#/patchset/20260527142432.230127-1-sarthak.sharma@arm.com
Thanks! I went through Sashiko's review.
Both the points are valid but look very minor to me: one is a temporary
x86 build issue fixed by the next patch and the other is stricter
argument validation for -F in gup_bench.
I don't feel they require a v5 on their own. Let's wait for reviewer
feedback. If a respin is needed then I'll fold these cleanups in as well.
^ permalink raw reply
* Re: [PATCH net-next v07 1/6] hinic3: Add ethtool queue ops
From: Mohsin Bashir @ 2026-05-29 5:30 UTC (permalink / raw)
To: Fan Gong, Zhu Yikai, netdev, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Andrew Lunn,
Ioana Ciornei
Cc: linux-kernel, linux-doc, luosifu, Xin Guo, Zhou Shuai, Wu Like,
Shi Jing, Zheng Jiezhen, Maxime Chevallier
In-Reply-To: <1323cad7ba784efa29a3fd1475b744974e0a0cf5.1779940072.git.zhuyikai1@h-partners.com>
> +int
> +hinic3_change_channel_settings(struct net_device *netdev,
> + struct hinic3_dyna_txrxq_params *trxq_params)
> +{
> + struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
> + struct hinic3_dyna_txrxq_params cur_trxq_params = {};
> + struct hinic3_dyna_qp_params new_qp_params = {};
> + struct hinic3_dyna_qp_params cur_qp_params = {};
> + int err;
> +
> + cur_trxq_params = nic_dev->q_params;
> +
> + hinic3_config_num_qps(netdev, trxq_params);
> +
> + err = hinic3_alloc_channel_resources(netdev, &new_qp_params,
> + trxq_params);
> + if (err) {
> + netdev_err(netdev, "Failed to alloc channel resources\n");
> + return err;
> + }
> +
> + if (!test_and_set_bit(HINIC3_CHANGE_RES_INVALID, &nic_dev->flags)) {
> + hinic3_vport_down(netdev);
> + hinic3_close_channel(netdev);
> + hinic3_get_cur_qps(nic_dev, &cur_qp_params);
> + }
> +
> + hinic3_init_qps(nic_dev, &new_qp_params);
> +
> + err = hinic3_prepare_channel(netdev, trxq_params);
> + if (err)
> + goto err_uninit_qps;
It may just be me, but looks like a failure here is not restoring the
old state. You are correctly getting the new resources and freeing them,
but don't you think a call to hinic3_init_qps() is needed here with
older config?
> +
> + if (nic_dev->num_qp_irq > trxq_params->num_qps)
> + hinic3_qp_irq_change(netdev, trxq_params->num_qps);
> +
> + nic_dev->q_params = *trxq_params;
> +
> + err = hinic3_open_channel(netdev);
> + if (err)
> + goto err_qp_irq_reset;
> +
> + err = hinic3_vport_up(netdev);
> + if (err)
> + goto err_close_channel;
> +
> + hinic3_free_channel_resources(netdev, &cur_qp_params, &cur_trxq_params);
> +
> + clear_bit(HINIC3_CHANGE_RES_INVALID, &nic_dev->flags);
> +
> + return 0;
> +
> +err_close_channel:
> + hinic3_close_channel(netdev);
> +err_qp_irq_reset:
> + nic_dev->q_params = cur_trxq_params;
> +
> + if (trxq_params->num_qps > cur_trxq_params.num_qps)
> + hinic3_qp_irq_change(netdev, cur_trxq_params.num_qps);
> +err_uninit_qps:
> + hinic3_get_cur_qps(nic_dev, &new_qp_params);
> + hinic3_free_channel_resources(netdev, &new_qp_params, trxq_params);
> +
> + return err;
> +}
> +
^ permalink raw reply
* [PATCH 3/4] dt-bindings: net: dsa: remove obsolete dsa.txt
From: Akash Sukhavasi @ 2026-05-29 5:22 UTC (permalink / raw)
To: devicetree
Cc: Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Simon Horman, Jonathan Corbet, Shuah Khan, netdev,
linux-kernel, linux-doc
In-Reply-To: <20260529052246.4934-1-akash.sukhavasi@gmail.com>
dsa.txt has been a redirect to dsa.yaml since commit bce58590d1bd
("dt-bindings: net: dsa: Add DSA yaml binding") introduced the .yaml
schema. The .yaml has the same filename in the same directory, making
this redirect unnecessary for discoverability.
Two files still reference dsa.txt, forcing readers through an extra
hop to reach the .yaml. The stub has not been touched since August
2020. Update references in lan9303.txt and
Documentation/networking/dsa/dsa.rst to point directly to dsa.yaml
and remove the stub.
Signed-off-by: Akash Sukhavasi <akash.sukhavasi@gmail.com>
---
Documentation/devicetree/bindings/net/dsa/dsa.txt | 4 ----
Documentation/devicetree/bindings/net/dsa/lan9303.txt | 2 +-
Documentation/networking/dsa/dsa.rst | 2 +-
3 files changed, 2 insertions(+), 6 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/net/dsa/dsa.txt
diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.txt b/Documentation/devicetree/bindings/net/dsa/dsa.txt
deleted file mode 100644
index dab208b5c7c7..000000000000
--- a/Documentation/devicetree/bindings/net/dsa/dsa.txt
+++ /dev/null
@@ -1,4 +0,0 @@
-Distributed Switch Architecture Device Tree Bindings
-----------------------------------------------------
-
-See Documentation/devicetree/bindings/net/dsa/dsa.yaml for the documentation.
diff --git a/Documentation/devicetree/bindings/net/dsa/lan9303.txt b/Documentation/devicetree/bindings/net/dsa/lan9303.txt
index 46a732087f5c..0337c2ccfa9a 100644
--- a/Documentation/devicetree/bindings/net/dsa/lan9303.txt
+++ b/Documentation/devicetree/bindings/net/dsa/lan9303.txt
@@ -16,7 +16,7 @@ Optional properties:
Subnodes:
The integrated switch subnode should be specified according to the binding
-described in dsa/dsa.txt. The CPU port of this switch is always port 0.
+described in dsa/dsa.yaml. The CPU port of this switch is always port 0.
Note: always use 'reg = <0/1/2>;' for the three DSA ports, even if the device is
configured to use 1/2/3 instead. This hardware configuration will be
diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
index fd3c254ced1d..42a99f5dfa2e 100644
--- a/Documentation/networking/dsa/dsa.rst
+++ b/Documentation/networking/dsa/dsa.rst
@@ -509,7 +509,7 @@ Device Tree
-----------
DSA features a standardized binding which is documented in
-``Documentation/devicetree/bindings/net/dsa/dsa.txt``. PHY/MDIO library helper
+``Documentation/devicetree/bindings/net/dsa/dsa.yaml``. PHY/MDIO library helper
functions such as ``of_get_phy_mode()``, ``of_phy_connect()`` are also used to query
per-port PHY specific details: interface connection, MDIO bus location, etc.
--
2.54.0
^ permalink raw reply related
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