* Re: [PATCH] nios2: remove the architecture
From: Simon Schuster @ 2026-05-18 17:24 UTC (permalink / raw)
To: Peter Zijlstra, Arnd Bergmann, Ethan Nelson-Moore, Dinh Nguyen
Cc: linux-doc, devicetree, workflows, Linux-Arch, dmaengine,
linux-i2c, linux-iio, Netdev, linux-pci, linux-pwm,
linux-hardening, linux-kbuild, linux-csky@vger.kernel.org,
Jonathan Corbet, Shuah Khan, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Daniel Lezcano, Thomas Gleixner, Alex Shi,
Yanteng Si, Dongliang Mu, Hu Haowen, Kees Cook, Oleg Nesterov,
Will Deacon, Aneesh Kumar K.V (Arm), Andrew Morton,
Nicholas Piggin, Vinod Koul, Frank Li, Dave Penkler, Andi Shyti,
Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Lorenzo Pieralisi, Krzysztof WilczyDski,
Andreas Oetken
In-Reply-To: <20260518105735.GW3126523@noisy.programming.kicks-ass.net>
Hi Ethan, Arnd, Peter and Dinh,
On Mon, May 18, 2026 at 11:29:48AM +0200, Arnd Bergmann wrote:
> We last discussed this a year ago when Simon Schuster mentioned[1]
> that Siemens Energy is still using NIOS-2 in production and would
> prefer to have this still included in Linux for at least another
> few years until the obligation for kernel updates ends.
First off, thank you, Arnd, for remembering us as this patch series came
up and also to Dinh for his maintenance of the architecture!
Regarding our status in relation to nios2, Arnd's response already gives
you the gist:
We are well aware that the architecture was deprecated by Intel and are
therefore phasing it out in favour of more contemporary hardware.
I'm also fully aware of the uncertain future of 32-bit architectures as
a whole [0] and that this fate will come to nios2 sooner or later.
But as of now, the mainline support is still in very good shape.
On Mon, May 18, 2026 at 12:57:35PM +0200, Peter Zijlstra wrote:
> Isn't that what we have LTS branches for?
Unfortunately, as we are an infrastructure provider for civil energy
infrastructure, the refurbishment cycle is a bit slower than for
traditional consumer systems. This implies that the traditional LTS
support duration (max. Dec 2028 as of writing [1]) is rather short, and
we would be glad if we could keep the architecture in mainline for at
least 5 years and only then "decay" to LTS.
On Mon, May 18, 2026 at 11:29:48AM +0200, Arnd Bergmann wrote:
> My feeling is that the maintenance burden of keeping nios2 is
> relatively low. On the other hand, maintaining it out of tree
> as a patch set is also something that should not be all that
> hard if it does get removed.
Judging from the architecture's git history, it seems that it's
currently mainly touched by treewide refactors, which are extremely
helpful as we therefore do not have to piece these changes together
downstream. In other respects, we try to be good citizens and contribute
bugfixes as well as required cleanups (such as implementing clone3 [2]
and fixing its flag behaviour on 32-bit architectures) as they come up.
If desired, we also would be happy to intensify our support regarding
reviews or testing to share the maintnance burden if it helps to keep
nios2 in mainline a bit longer.
Best regards,
Simon
0: https://lwn.net/Articles/1035727/
1: https://www.kernel.org/category/releases.html
2: https://lore.kernel.org/lkml/20250821-nios2-implement-clone3-v1-0-1bb24017376a@siemens-energy.com/
^ permalink raw reply
* Re: [Linaro-mm-sig] Re: [PATCH RFC 2/5] dma-heap: charge dma-buf memory via explicit memcg
From: Barry Song @ 2026-05-18 23:00 UTC (permalink / raw)
To: Christian König
Cc: T.J. Mercier, Albert Esteve, Tejun Heo, Johannes Weiner,
Michal Koutný, Jonathan Corbet, Shuah Khan, Sumit Semwal,
Michal Hocko, Roman Gushchin, Shakeel Butt, Muchun Song,
Andrew Morton, Benjamin Gaignard, Brian Starkey, John Stultz,
Christian Brauner, Paul Moore, James Morris, Serge E. Hallyn,
Stephen Smalley, Ondrej Mosnacek, Shuah Khan, cgroups, linux-doc,
linux-kernel, linux-media, dri-, linaro-mm-sig, linux-mm,
linux-security-module, selinux, linux-kselftest, mripard,
echanude
In-Reply-To: <cb84c2ee-9de1-4565-b2e0-60984721228f@amd.com>
On Mon, May 18, 2026 at 3:34 PM Christian König
<christian.koenig@amd.com> wrote:
>
> On 5/16/26 11:19, Barry Song wrote:
> > On Thu, May 14, 2026 at 12:35 AM T.J. Mercier <tjmercier@google.com> wrote:
> > [...]
> >>>> I have a question about this part. Albert I guess you are interested
> >>>> only in accounting dmabuf-heap allocations, or do you expect to add
> >>>> __GFP_ACCOUNT or mem_cgroup_charge_dmabuf calls to other
> >>>> non-dmabuf-heap exporters?
> >>>
> >>> We're scoping this to dma-buf heaps for now. CMA heaps and the dmem
> >>> controller are on the radar for follow-up/parallel work (there will be
> >>> dragons and will surely need discussion). For DRM and V4L2 the
> >>> long-term intent is migration to heaps, which would make direct
> >>> accounting on those paths unnecessary.
> >>
> >> Ah I see. GEM buffers exported to dmabufs are what I had in mind. I
> >> guess this would only leave the odd non-DRM driver with the need to
> >> add their own accounting calls, which I don't expect would be a big
> >> problem.
> >>
> >
> > sounds like we still have a long way to go to correctly account for
> > various v4l2, drm, GEM, CMA, etc. In patch 1, the charging is done in
> > dma_buf_export(), so I guess it covers all dma-buf types except
> > dma_heap, but the problem is that it has no remote charging support at
> > all?
>
> No, just the other way around
>
> DMA-buf heaps can be handled here because we know that it is pure system memory and nothing special so memcg always applies.
>
> dma_buf_export() on the other hand handles tons of different use cases, ranging from buffer accounted to dmem, over special resources which aren't even memory all the way to buffers which can migrate from dmem to memcg and back during their lifetime.
>
Hi Christian,
Thanks very much for your explanation. So basically it seems that
dma_buf_export() is not the proper place to charge, since it may end up
mixing in non-system-memory accounting?
My question is also about the global view for both heap and non-heap cases.
After reading the discussion, I’ve tried to summarize it—please let me know
if my understanding is correct.
for dma_heap, we have the ioctl DMA_HEAP_IOCTL_ALLOC, where users can pass a
remote pidfd or similar information to indicate where the dma-buf should be
charged, as in Albert's patchset.
For non-dma_heap dma-bufs, we don’t have an obvious userspace entry point that
triggers the allocation. So we likely need other approaches. We could either
move more drivers over to dma-heap, or introduce something like
DMA_BUF_IOCTL_XFER_CHARGE, as you are discussing, to let userspace explicitly
declare a charge.
Best Regards
Barry
^ permalink raw reply
* Re: [PATCH RFC 2/5] dma-heap: charge dma-buf memory via explicit memcg
From: Barry Song @ 2026-05-18 22:43 UTC (permalink / raw)
To: Albert Esteve
Cc: Tejun Heo, Johannes Weiner, Michal Koutný, Jonathan Corbet,
Shuah Khan, Sumit Semwal, Christian König, Michal Hocko,
Roman Gushchin, Shakeel Butt, Muchun Song, Andrew Morton,
Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier,
Christian Brauner, Paul Moore, James Morris, Serge E. Hallyn,
Stephen Smalley, Ondrej Mosnacek, Shuah Khan, cgroups, linux-doc,
linux-kernel, linux-media, dri-devel, linaro-mm-sig, linux-mm,
linux-security-module, selinux, linux-kselftest, mripard,
echanude
In-Reply-To: <CADSE00LjJcL8P5M-UPEpzZijU70uEmUirnin29N8YR5W5D-oFg@mail.gmail.com>
On Mon, May 18, 2026 at 8:16 PM Albert Esteve <aesteve@redhat.com> wrote:
>
> On Sat, May 16, 2026 at 9:37 AM Barry Song <baohua@kernel.org> wrote:
> >
> > On Tue, May 12, 2026 at 5:18 PM Albert Esteve <aesteve@redhat.com> wrote:
> > >
> > > On embedded platforms a central process often allocates dma-buf
> > > memory on behalf of client applications. Without a way to
> > > attribute the charge to the requesting client's cgroup, the
> > > cost lands on the allocator, making per-cgroup memory limits
> > > ineffective for the actual consumers.
> > >
> > > Add charge_pid_fd to struct dma_heap_allocation_data. When set to
> > > a valid pidfd, DMA_HEAP_IOCTL_ALLOC resolves the target task's
> > > memcg and charges the buffer there via mem_cgroup_charge_dmabuf()
> > > inside dma_heap_buffer_alloc(). Without charge_pid_fd, and with
> > > the mem_accounting module parameter enabled, the buffer is charged
> > > to the allocator's own cgroup.
> > >
> > > Additionally, commit 3c227be90659 ("dma-buf: system_heap: account for
> > > system heap allocation in memcg") adds __GFP_ACCOUNT to system-heap
> > > page allocations. Keeping __GFP_ACCOUNT would charge the same pages
> > > twice (once to kmem, once to MEMCG_DMABUF), thus remove it and route
> > > all accounting through a single MEMCG_DMABUF path.
> > >
> > [...]
> >
> > > - if (mem_accounting)
> > > - flags |= __GFP_ACCOUNT;
> >
> > Hi Albert,
> >
> > would it be better to move this and its description to patch 1? It
> > looks like patch 1 already introduces the double accounting changes,
> > and patch 2 is mainly just supporting remote charging.
>
> Hi Barry,
>
> Thanks for looking into this series! Yes, in my head I was trying to
> keep patch 1, which was taken from a previous, different series, and
> then diverge from it starting with patch 2. This would clarify the
> difference between the two. But I can see it just added some confusion
> (for example, patch 1 charges on dma_buf_export() and then it is moved
> to dma_heap_buffer_alloc() in patch 2). I will reorganize it better
> for the next version, including your suggestion.
Yep, I understand the situation now. I also understand
that you were referring to T.J.'s patch, which caused
some back-and-forth confusion for readers when reading
patches 1 and 2.
>
> >
> > Also, mem_accounting is only used by system_heap.c; has this patchset
> > also eliminated its need?
>
> No, mem_accounting is still handled in this patch for the general case
> where no `charge_pid_fd` is used. See dma_heap_buffer_alloc() code:
>
> + if (memcg)
> + css_get(&memcg->css);
> + else if (mem_accounting)
> + memcg = get_mem_cgroup_from_mm(current->mm);
I see. What feels a bit odd to me is that mem_accounting
could either be dropped (with unconditional charging), or
it should cover both remote and local charge cases.
I don’t have a strong opinion here—it just feels a bit
strange, since its description is quite generic for memcg:
"Enable cgroup-based memory accounting for dma-buf heap
allocations (default=false)."
Best Regards
Barry
^ permalink raw reply
* [PATCH v6 4/4] HID: hid-msi: Add Rumble Intensity Attributes
From: Derek J. Clark @ 2026-05-18 22:29 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: <20260518222935.1802071-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>
---
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 | 167 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 167 insertions(+)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 4d267b896da72..339f256b60dbe 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -78,6 +78,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 {
@@ -265,6 +267,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];
@@ -317,9 +324,13 @@ struct claw_drvdata {
enum claw_gamepad_mode_index gamepad_mode;
u8 m1_codes[CLAW_KEYS_MAX];
u8 m2_codes[CLAW_KEYS_MAX];
+ u8 rumble_intensity_right;
+ u8 rumble_intensity_left;
bool gamepad_registered;
+ spinlock_t rumble_lock; /* lock for rumble_intensity read/write */
spinlock_t mode_lock; /* Lock for mode data read/write */
const u16 *bmap_addr;
+ bool rumble_support;
bool bmap_support;
/* RGB Variables */
@@ -409,6 +420,14 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
}
break;
+ case CLAW_RUMBLE_LEFT_PENDING:
+ scoped_guard(spinlock_irqsave, &drvdata->rumble_lock)
+ drvdata->rumble_intensity_left = cmd_rep->data[4];
+ break;
+ case CLAW_RUMBLE_RIGHT_PENDING:
+ scoped_guard(spinlock_irqsave, &drvdata->rumble_lock)
+ drvdata->rumble_intensity_right = cmd_rep->data[4];
+ break;
default:
dev_dbg(&drvdata->hdev->dev,
"Got profile event without changes pending from command: %x\n",
@@ -882,6 +901,142 @@ 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)
+{
+ u8 data[] = { 0x01, (rumble_addr[0] >> 8) & 0xff, rumble_addr[0] & 0xff, 0x01, 0x00 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 val;
+ int ret;
+
+ if (!drvdata->gamepad_registered)
+ return -ENODEV;
+
+ ret = kstrtou8(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 100)
+ return -EINVAL;
+
+ data[4] = val;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ data, ARRAY_SIZE(data), 25);
+ if (ret)
+ return ret;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 25);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t rumble_intensity_left_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 data[4] = { 0x01, (rumble_addr[0] >> 8) & 0xff, rumble_addr[0] & 0xff, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret;
+ u8 val;
+
+ if (!drvdata->gamepad_registered)
+ return -ENODEV;
+
+ guard(mutex)(&drvdata->profile_mutex);
+ drvdata->profile_pending = CLAW_RUMBLE_LEFT_PENDING;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE, data,
+ ARRAY_SIZE(data), 25);
+ if (ret) {
+ drvdata->profile_pending = CLAW_NO_PENDING;
+ 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)
+{
+ u8 data[] = { 0x01, (rumble_addr[1] >> 8) & 0xff, rumble_addr[1] & 0xff, 0x01, 0x00 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 val;
+ int ret;
+
+ if (!drvdata->gamepad_registered)
+ return -ENODEV;
+
+ ret = kstrtou8(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 100)
+ return -EINVAL;
+
+ data[4] = val;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ data, ARRAY_SIZE(data), 25);
+ if (ret)
+ return ret;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 25);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t rumble_intensity_right_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 data[4] = { 0x01, (rumble_addr[1] >> 8) & 0xff, rumble_addr[1] & 0xff, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret;
+ u8 val;
+
+ if (!drvdata->gamepad_registered)
+ return -ENODEV;
+
+ guard(mutex)(&drvdata->profile_mutex);
+ drvdata->profile_pending = CLAW_RUMBLE_RIGHT_PENDING;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE, data,
+ ARRAY_SIZE(data), 25);
+ if (ret) {
+ drvdata->profile_pending = CLAW_NO_PENDING;
+ 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)
{
@@ -902,6 +1057,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;
}
@@ -915,6 +1076,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,
};
@@ -1430,6 +1594,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;
@@ -1441,6 +1606,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;
}
@@ -1488,6 +1654,7 @@ 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->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 v6 3/4] HID: hid-msi: Add RGB control interface
From: Derek J. Clark @ 2026-05-18 22:29 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: <20260518222935.1802071-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>
---
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 | 596 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 587 insertions(+), 9 deletions(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index e58d35dba5b40..4d267b896da72 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_READ_PROFILE = 0x04,
CLAW_COMMAND_TYPE_READ_PROFILE_ACK = 0x05,
@@ -72,6 +77,7 @@ enum claw_profile_ack_pending {
CLAW_NO_PENDING,
CLAW_M1_PENDING,
CLAW_M2_PENDING,
+ CLAW_RGB_PENDING,
};
enum claw_key_index {
@@ -230,6 +236,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 */
@@ -240,6 +262,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];
@@ -248,6 +273,28 @@ struct claw_command_report {
u8 data[59];
} __packed;
+struct rgb_zone {
+ u8 red;
+ u8 green;
+ u8 blue;
+};
+
+struct rgb_frame {
+ struct rgb_zone zone[CLAW_RGB_ZONES];
+};
+
+struct rgb_report {
+ u8 profile;
+ __be16 read_addr;
+ 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;
@@ -274,6 +321,18 @@ struct claw_drvdata {
spinlock_t mode_lock; /* Lock for mode data read/write */
const u16 *bmap_addr;
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)
@@ -307,8 +366,11 @@ 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)
{
- u8 *codes;
- int i;
+ struct rgb_report *frame;
+ u16 rgb_addr, read_addr;
+ u8 *codes, f_idx;
+ u16 frame_calc;
+ int i, ret = 0;
switch (drvdata->profile_pending) {
case CLAW_M1_PENDING:
@@ -318,6 +380,35 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
for (i = 0; i < CLAW_KEYS_MAX; i++)
codes[i] = (cmd_rep->data[6 + i]);
break;
+ case CLAW_RGB_PENDING:
+ frame = (struct 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);
+ drvdata->profile_pending = CLAW_NO_PENDING;
+ return -EINVAL;
+ }
+ f_idx = frame_calc;
+
+ scoped_guard(spinlock_irqsave, &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",
@@ -326,7 +417,7 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
}
drvdata->profile_pending = CLAW_NO_PENDING;
- return 0;
+ return ret;
}
static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
@@ -832,6 +923,441 @@ 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 < 8; i++) {
+ 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) {
+ drvdata->profile_pending = CLAW_NO_PENDING;
+ 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 rgb_report report = { 0x01, 0x0000, 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;
+ size_t len = sizeof(report);
+ 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, len, 25);
+ if (ret)
+ return ret;
+
+ /* Increment the write addr by the offset for the next frame */
+ write_addr += CLAW_RGB_FRAME_OFFSET;
+ }
+
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 25);
+
+ 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->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;
+
+ if (!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);
+
+ if (!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;
+
+ if (!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);
+
+ if (!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;
+
+ if (!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;
+
+ if (!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);
+
+ if (!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);
@@ -846,16 +1372,44 @@ static void cfg_setup_fn(struct work_struct *work)
return;
}
- /* Add sysfs attributes after we get the device state */
- ret = devm_device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
+ 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);
+ "Failed to setup device, can't read RGB config: %d\n", ret);
return;
}
- drvdata->gamepad_registered = true;
+
+ /* Add sysfs attributes after we get the device state */
+ if (!drvdata->gamepad_registered) {
+ ret = devm_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;
+ }
+ drvdata->gamepad_registered = true;
+ }
+
+ /* Add and enable RGB interface once we have the device state */
+ if (!drvdata->rgb_registered) {
+ ret = devm_led_classdev_multicolor_register(&drvdata->hdev->dev, &drvdata->led_mc);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to setup device, can't create led device: %d\n", ret);
+ return;
+ }
+
+ ret = devm_device_add_group(drvdata->led_mc.led_cdev.dev, &claw_rgb_attr_group);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to setup device, can't create led attributes: %d\n", ret);
+ return;
+ }
+ drvdata->rgb_registered = true;
+ }
kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
+ kobject_uevent(&drvdata->led_mc.led_cdev.dev->kobj, KOBJ_CHANGE);
}
static void cfg_resume_fn(struct work_struct *work)
@@ -874,18 +1428,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)
@@ -900,6 +1460,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;
@@ -910,14 +1471,27 @@ 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);
spin_lock_init(&drvdata->cmd_lock);
spin_lock_init(&drvdata->mode_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);
@@ -979,10 +1553,14 @@ static void claw_remove(struct hid_device *hdev)
return;
}
+ /* Block writes to brightness/multi_intensity during teardown */
+ drvdata->led_mc.led_cdev.brightness_set = NULL;
cancel_delayed_work_sync(&drvdata->cfg_setup);
cancel_delayed_work_sync(&drvdata->cfg_resume);
+ cancel_delayed_work_sync(&drvdata->rgb_queue);
drvdata->gamepad_registered = false;
+ drvdata->rgb_enabled = false;
hid_hw_close(hdev);
}
--
2.53.0
^ permalink raw reply related
* [PATCH v6 2/4] HID: hid-msi: Add M-key mapping attributes
From: Derek J. Clark @ 2026-05-18 22:29 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: <20260518222935.1802071-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>
---
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 | 415 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 414 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index c79eb0bfeb776..e58d35dba5b40 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_READ_PROFILE = 0x04,
CLAW_COMMAND_TYPE_READ_PROFILE_ACK = 0x05,
@@ -66,6 +68,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,
@@ -78,6 +91,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];
@@ -88,22 +250,30 @@ struct claw_command_report {
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;
+ struct mutex profile_mutex; /* mutex for profile_pending calls */
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];
bool gamepad_registered;
spinlock_t mode_lock; /* Lock for mode data read/write */
+ const u16 *bmap_addr;
+ bool bmap_support;
};
static int get_endpoint_address(struct hid_device *hdev)
@@ -135,6 +305,30 @@ 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)
+{
+ u8 *codes;
+ int i;
+
+ switch (drvdata->profile_pending) {
+ case CLAW_M1_PENDING:
+ case CLAW_M2_PENDING:
+ codes = (drvdata->profile_pending == CLAW_M1_PENDING) ?
+ drvdata->m1_codes : drvdata->m2_codes;
+ for (i = 0; i < CLAW_KEYS_MAX; i++)
+ codes[i] = (cmd_rep->data[6 + i]);
+ break;
+ default:
+ dev_dbg(&drvdata->hdev->dev,
+ "Got profile event without changes pending from command: %x\n",
+ cmd_rep->cmd);
+ return -EINVAL;
+ }
+ 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,6 +359,19 @@ static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *repor
}
}
+ break;
+ case CLAW_COMMAND_TYPE_READ_PROFILE_ACK:
+ ret = claw_profile_event(drvdata, cmd_rep);
+
+ scoped_guard(spinlock_irqsave, &drvdata->cmd_lock) {
+ 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:
scoped_guard(spinlock_irqsave, &drvdata->cmd_lock) {
@@ -422,6 +629,168 @@ static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_WO(reset);
+static int button_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 *button_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_buttons_store(struct device *dev, const char *buf, u8 mkey_idx)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 data[] = { 0x01, (drvdata->bmap_addr[mkey_idx] >> 8) & 0xff,
+ drvdata->bmap_addr[mkey_idx] & 0xff, 0x07,
+ 0x04, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff };
+ char **raw_keys __free(argv_free) = NULL;
+ size_t len = ARRAY_SIZE(data);
+ int ret, key_count, i;
+
+ if (!drvdata->gamepad_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 = button_mapping_name_to_code(raw_keys[i]);
+ if (ret < 0)
+ return ret;
+
+ data[6 + i] = ret;
+ }
+
+set_buttons:
+ scoped_guard(mutex, &drvdata->rom_mutex) {
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ data, len, 25);
+ if (ret)
+ return ret;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 25);
+ }
+
+ return ret;
+}
+
+static int claw_buttons_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);
+ u8 data[] = { 0x01, (drvdata->bmap_addr[m_key] >> 8) & 0xff,
+ drvdata->bmap_addr[m_key] & 0xff, 0x07 };
+ size_t len = ARRAY_SIZE(data);
+ int i, ret, count = 0;
+ const char *name;
+ u8 *codes;
+
+ if (!drvdata->gamepad_registered)
+ return -ENODEV;
+
+ codes = (m_key == CLAW_KEY_M1) ? drvdata->m1_codes : drvdata->m2_codes;
+
+ guard(mutex)(&drvdata->profile_mutex);
+ 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, data, len, 25);
+ if (ret) {
+ drvdata->profile_pending = CLAW_NO_PENDING;
+ return ret;
+ }
+ for (i = 0; i < CLAW_KEYS_MAX; i++) {
+ name = button_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_buttons_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_buttons_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_buttons_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_buttons_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);
+
+ 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)
{
@@ -434,10 +803,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,
@@ -486,8 +867,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;
@@ -499,7 +903,16 @@ 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->cmd_lock);
spin_lock_init(&drvdata->mode_lock);
init_completion(&drvdata->send_cmd_complete);
--
2.53.0
^ permalink raw reply related
* [PATCH v6 1/4] HID: hid-msi: Add MSI Claw configuration driver
From: Derek J. Clark @ 2026-05-18 22:29 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: <20260518222935.1802071-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>
---
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 | 675 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 700 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..c79eb0bfeb776
--- /dev/null
+++ b/drivers/hid/hid-msi.c
@@ -0,0 +1,675 @@
+// 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_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;
+ 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;
+ bool gamepad_registered;
+ spinlock_t mode_lock; /* Lock for mode data read/write */
+};
+
+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_irqsave, &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);
+
+ switch (cmd_rep->cmd) {
+ case CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK:
+ ret = claw_gamepad_mode_event(drvdata, cmd_rep);
+
+ scoped_guard(spinlock_irqsave, &drvdata->cmd_lock) {
+ 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:
+ scoped_guard(spinlock_irqsave, &drvdata->cmd_lock) {
+ 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) {
+ scoped_guard(spinlock_irqsave, &drvdata->cmd_lock) {
+ drvdata->waiting_cmd = index;
+ drvdata->waiting_for_ack = true;
+ drvdata->cmd_status = -ETIMEDOUT;
+ }
+ reinit_completion(&drvdata->send_cmd_complete);
+ }
+
+ 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)
+ return ret;
+
+ ret = ret == CLAW_PACKET_SIZE ? 0 : -EIO;
+ if (ret)
+ return ret;
+
+ 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;
+ scoped_guard(spinlock_irqsave, &drvdata->cmd_lock)
+ 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];
+
+ if (!drvdata->gamepad_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;
+
+ if (drvdata->gamepad_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];
+
+ if (!drvdata->gamepad_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;
+
+ if (!drvdata->gamepad_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;
+
+ if (!drvdata->gamepad_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 = devm_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;
+ }
+ drvdata->gamepad_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);
+
+ if (!drvdata->gamepad_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->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);
+
+ if (!drvdata) {
+ hid_hw_close(hdev);
+ return;
+ }
+
+ cancel_delayed_work_sync(&drvdata->cfg_setup);
+ cancel_delayed_work_sync(&drvdata->cfg_resume);
+
+ drvdata->gamepad_registered = false;
+
+ hid_hw_close(hdev);
+}
+
+static void msi_remove(struct hid_device *hdev)
+{
+ int ret;
+ u8 ep;
+
+ 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;
+
+ 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_setup);
+ cancel_delayed_work_sync(&drvdata->cfg_resume);
+
+ return 0;
+}
+
+static int msi_suspend(struct hid_device *hdev, pm_message_t msg)
+{
+ int ret;
+ u8 ep;
+
+ 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 v6 0/4] Add MSI Claw HID Configuration Driver
From: Derek J. Clark @ 2026-05-18 22:29 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>
---
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 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 | 1766 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 1791 insertions(+)
create mode 100644 drivers/hid/hid-msi.c
--
2.53.0
^ permalink raw reply
* Re: [PATCH RFC 2/5] dma-heap: charge dma-buf memory via explicit memcg
From: Barry Song @ 2026-05-18 22:19 UTC (permalink / raw)
To: T.J. Mercier
Cc: Christian König, Albert Esteve, Tejun Heo, Johannes Weiner,
Michal Koutný, Jonathan Corbet, Shuah Khan, Sumit Semwal,
Michal Hocko, Roman Gushchin, Shakeel Butt, Muchun Song,
Andrew Morton, Benjamin Gaignard, Brian Starkey, John Stultz,
Christian Brauner, Paul Moore, James Morris, Serge E. Hallyn,
Stephen Smalley, Ondrej Mosnacek, Shuah Khan, cgroups, linux-doc,
linux-kernel, linux-media, dri-devel, linaro-mm-sig, linux-mm,
linux-security-module, selinux, linux-kselftest, mripard,
echanude
In-Reply-To: <CABdmKX3wwgovwS-V8rVC3=+EZcTvPs_cttpQb1w6WemwLAVhsw@mail.gmail.com>
On Tue, May 19, 2026 at 5:17 AM T.J. Mercier <tjmercier@google.com> wrote:
[...]
> > > > Yeah I think this might work. I know of 3 cases, and it trivially
> > > > solves the first two. The third requires some work on our end to
> > > > extend our userspace interfaces to include the pidfd but it seems
> > > > doable. I'm checking with our graphics folks.
> > > >
> > > > 1) Direct allocation from user (e.g. app -> allocation ioctl on
> > > > /dev/dma_heap/foo)
> > > > No changes required to userspace. mem_accounting=1 charges the app.
> > > >
> > > > 2) Single hop remote allocation (e.g. app -> AHardwareBuffer_allocate
> > > > -> gralloc)
> > > > gralloc has the caller's pid as described in the commit message. Open
> > > > a pidfd and pass it in the dma_heap_allocation_data.
> > > >
> > > > 3) Double hop remote allocation (e.g. app -> dequeueBuffer ->
> > > > SurfaceFlinger -> gralloc)
> > > > In this case gralloc knows SurfaceFlinger's pid, but not the app's. So
> > > > we need to add the app's pidfd to the SurfaceFlinger -> gralloc
> > > > interface, or transfer the memcg charge from SurfaceFlinger to the app
> > > > after the allocation.
> > > > It'd be nice to avoid the charge transfer option entirely, but if we
> > > > need it that doesn't seem so bad in this case because it's a bulk
> > > > charge for the entire dmabuf rather than per-page. So the exporter
> > > > doesn't need to get involved (we wouldn't need a new dma_buf_op) and
> > > > we wouldn't have to worry about looping and locking for each page.
> > > >
> > >
> > > Hi T.J.,
> > >
> > > Your description of the three different cases sounds very interesting.
> > > It helps me understand how difficult it can be to correctly charge
> > > dma-buf in the current user scenarios.
> > >
> > > I’m wondering where I can find Android userspace code that transfers
> > > the PID of RPC callers. Do we have any existing sample code in Android
> > > for this?
> >
> > Hi Barry,
> >
> > In Java android.os.Binder.getCallingPid() will provide it. Here
>
> ... let me try again
>
> Here are some examples from the framework code:
>
> https://cs.android.com/search?q=getCallingPid%20f:ActivityManager&sq=&ss=android%2Fplatform%2Fsuperproject
>
> In native code we have AIBinder_getCallingPid and
> android::IPCThreadState::self()->getCallingPid() (or
> android::hardware::IPCThreadState::self()->getCallingPid() for HIDL)
>
> https://cs.android.com/search?q=getCallingPid%20l:cpp%20-f:prebuilt&ss=android%2Fplatform%2Fsuperproject
Thanks very much, T.J. That is very helpful. I guess
that would require user space to understand the RPC
procedure, including single-hop and two-hop cases, and
make the corresponding changes.
You pointed out the SurfaceFlinger cases, which are
two hops. It seems that AI models are also using
dma_heap, at least from what I have observed on MTK
and Qualcomm phones. Likely, we need to understand
those RPC relationships in userspace and make the
corresponding changes.
I assume AI models are a single-hop case?
Best Regards
Barry
^ permalink raw reply
* Re: [PATCH v3 1/3] soc: bcm2835: raspberrypi-firmware: Add voltage domain IDs
From: Guenter Roeck @ 2026-05-18 22:17 UTC (permalink / raw)
To: Shubham Chakraborty
Cc: Florian Fainelli, Jonathan Corbet, Shuah Khan,
Broadcom internal kernel review list, Ray Jui, Scott Branden,
linux-hwmon, linux-doc, linux-rpi-kernel, linux-arm-kernel,
linux-kernel
In-Reply-To: <20260517080445.103962-2-chakrabortyshubham66@gmail.com>
On Sun, May 17, 2026 at 01:34:43PM +0530, Shubham Chakraborty wrote:
> Add Raspberry Pi firmware voltage domain identifiers for the mailbox
> property interface.
>
> Also add the voltage request structure used with
> RPI_FIRMWARE_GET_VOLTAGE so firmware clients can share the common API
> definition from the firmware header.
>
> Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>
I'll need an Acked-by: from a maintainer to apply this patch,
or some other maintainer will have to pick it up.
Thanks,
Guenter
> ---
> include/soc/bcm2835/raspberrypi-firmware.h | 25 ++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
> index e1f87fbfe554..975bef529854 100644
> --- a/include/soc/bcm2835/raspberrypi-firmware.h
> +++ b/include/soc/bcm2835/raspberrypi-firmware.h
> @@ -156,6 +156,31 @@ enum rpi_firmware_clk_id {
> RPI_FIRMWARE_NUM_CLK_ID,
> };
>
> +enum rpi_firmware_volt_id {
> + RPI_FIRMWARE_VOLT_ID_CORE = 1,
> + RPI_FIRMWARE_VOLT_ID_SDRAM_C = 2,
> + RPI_FIRMWARE_VOLT_ID_SDRAM_P = 3,
> + RPI_FIRMWARE_VOLT_ID_SDRAM_I = 4,
> + RPI_FIRMWARE_NUM_VOLT_ID,
> +};
> +
> +/**
> + * struct rpi_firmware_get_voltage_request - Firmware request for a voltage
> + * @id: ID of the voltage being queried
> + * @value: Voltage in microvolts. Set by the firmware.
> + *
> + * Used by @RPI_FIRMWARE_GET_VOLTAGE.
> + */
> +struct rpi_firmware_get_voltage_request {
> + __le32 id;
> + __le32 value;
> +} __packed;
> +
> +#define RPI_FIRMWARE_GET_VOLTAGE_REQUEST(_id) \
> + { \
> + .id = cpu_to_le32(_id), \
> + }
> +
> /**
> * struct rpi_firmware_clk_rate_request - Firmware Request for a rate
> * @id: ID of the clock being queried
^ permalink raw reply
* Re: [PATCH] Documentation: hwmon: ad7314: document sysfs interface
From: Guenter Roeck @ 2026-05-18 22:14 UTC (permalink / raw)
To: Chen-Shi-Hong
Cc: Jonathan Corbet, Shuah Khan, linux-hwmon, linux-doc, linux-kernel
In-Reply-To: <20260518182744.1302-1-eric039eric@gmail.com>
On 5/18/26 11:27, Chen-Shi-Hong wrote:
> Document the temp1_input sysfs attribute supported by the ad7314
> driver.
>
> Signed-off-by: Chen-Shi-Hong <eric039eric@gmail.com>
> ---
> Documentation/hwmon/ad7314.rst | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/hwmon/ad7314.rst b/Documentation/hwmon/ad7314.rst
> index bf389736bcd1..b454e617d48c 100644
> --- a/Documentation/hwmon/ad7314.rst
> +++ b/Documentation/hwmon/ad7314.rst
> @@ -28,6 +28,12 @@ Driver supports the above parts. The ad7314 has a 10 bit
> sensor with 1lsb = 0.25 degrees centigrade. The adt7301 and
> adt7302 have 14 bit sensors with 1lsb = 0.03125 degrees centigrade.
>
> +sysfs-Interface
> +---------------
> +
> +temp1_input
> + temperature input
> +
> Notes
> -----
>
Please stop doing that. It adds little if any value and takes time away
from reviewing and handing real patches.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH v5 00/14] module: Introduce hash-based integrity checking
From: Sami Tolvanen @ 2026-05-18 21:55 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Nathan Chancellor,
Nicolas Schier, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Paul Moore, James Morris, Serge E. Hallyn,
Jonathan Corbet, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Naveen N Rao, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin, Eric Snowberg, Nicolas Schier, Daniel Gomez,
Aaron Tomlin, Christophe Leroy (CS GROUP), Nicolas Bouchinet,
Xiu Jianfeng, Martin KaFai Lau, Song Liu, Yonghong Song,
Jiri Olsa, bpf, Fabian Grünbichler, Arnout Engelen,
Mattia Rizzolo, kpcyrd, Christian Heusel, Câju Mihai-Drosi,
Eric Biggers, Sebastian Andrzej Siewior, linux-kbuild,
linux-kernel, linux-arch, linux-modules, linux-security-module,
linux-doc, linuxppc-dev, linux-integrity, debian-kernel
In-Reply-To: <20260505-module-hashes-v5-0-e174a5a49fce@weissschuh.net>
Hi Thomas,
On Tue, May 05, 2026 at 11:05:04AM +0200, Thomas Weißschuh wrote:
> The current signature-based module integrity checking has some drawbacks
> in combination with reproducible builds. Either the module signing key
> is generated at build time, which makes the build unreproducible, or a
> static signing key is used, which precludes rebuilds by third parties
> and makes the whole build and packaging process much more complicated.
>
> The goal is to reach bit-for-bit reproducibility. Excluding certain
> parts of the build output from the reproducibility analysis would be
> error-prone and force each downstream consumer to introduce new tooling.
>
> Introduce a new mechanism to ensure only well-known modules are loaded
> by embedding a merkle tree root of all modules built as part of the full
> kernel build into vmlinux.
I noticed Sashiko had a few concerns about the build changes. Would you
mind taking a look to see if they're valid?
https://sashiko.dev/#/patchset/20260505-module-hashes-v5-0-e174a5a49fce%40weissschuh.net
Sami
^ permalink raw reply
* Re: [PATCH net-next v3 02/14] libie: add PCI device initialization helpers to libie
From: Bjorn Helgaas @ 2026-05-18 21:54 UTC (permalink / raw)
To: Tony Nguyen
Cc: davem, kuba, pabeni, edumazet, andrew+netdev, netdev,
Phani R Burra, larysa.zaremba, przemyslaw.kitszel,
aleksander.lobakin, sridhar.samudrala, anjali.singhai,
michal.swiatkowski, maciej.fijalkowski, emil.s.tantilov,
madhu.chittim, joshua.a.hay, jacob.e.keller,
jayaprakash.shanmugam, jiri, horms, corbet, richardcochran,
linux-doc, bhelgaas, linux-pci, Bharath R, Samuel Salin,
Aleksandr Loktionov, Philipp Stanner
In-Reply-To: <20260515224443.2772147-3-anthony.l.nguyen@intel.com>
[+cc Philipp]
On Fri, May 15, 2026 at 03:44:26PM -0700, Tony Nguyen wrote:
> From: Phani R Burra <phani.r.burra@intel.com>
>
> Add support functions for drivers to configure PCI functionality and access
> MMIO space.
This looks kind of like what pcim_iomap_range() does, i.e., a way to
ioremap (BAR-idx, offset, size) pieces of PCI BARs. That sounds like
useful functionality.
Is there something Intel-specific or even ethernet-specific about
this? If devm_* and pcim_* don't do what you need, maybe they should
be extended or this could be made generic so any drivers could use it?
This looks like a mix of managed (pcim_enable_device(),
pcim_request_region()), and unmanaged (ioremap(), iounmap()) things.
I haven't looked at how all this is used, but it's pretty easy to get
things wrong when mixing models.
> +++ b/drivers/net/ethernet/intel/libie/pci.c
> @@ -0,0 +1,208 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (C) 2025 Intel Corporation */
> +
> +#include <linux/intel/libie/pci.h>
> +
> +/**
> + * libie_find_mmio_region - find MMIO region containing a range
> + * @mmio_list: list that contains MMIO region info
> + * @offset: range start offset
> + * @size: range size
> + * @bar_idx: BAR index containing the range to search
> + *
> + * Return: pointer to a MMIO region overlapping with the range in any way or
> + * NULL if no such region is mapped.
> + */
> +static struct libie_pci_mmio_region *
> +libie_find_mmio_region(const struct list_head *mmio_list,
> + resource_size_t offset, resource_size_t size,
> + int bar_idx)
> +{
> + resource_size_t end_offset = offset + size;
> + struct libie_pci_mmio_region *mr;
> +
> + list_for_each_entry(mr, mmio_list, list) {
> + resource_size_t mr_end = mr->offset + mr->size;
> + resource_size_t mr_start = mr->offset;
> +
> + if (mr->bar_idx != bar_idx)
> + continue;
> + if (offset < mr_end && end_offset > mr_start)
> + return mr;
> + }
> +
> + return NULL;
> +}
> +
> +/**
> + * __libie_pci_get_mmio_addr - get the MMIO virtual address
> + * @mmio_info: contains list of MMIO regions
> + * @offset: register offset to find
> + * @num_args: number of additional arguments present
> + *
> + * This function finds the virtual address of a register offset by iterating
> + * through the non-linear MMIO regions that are mapped by the driver.
> + *
> + * Return: valid MMIO virtual address or NULL.
> + */
> +void __iomem *__libie_pci_get_mmio_addr(struct libie_mmio_info *mmio_info,
> + resource_size_t offset,
> + int num_args, ...)
> +{
> + struct libie_pci_mmio_region *mr;
> + int bar_idx = 0;
> + va_list args;
> +
> + if (num_args) {
> + va_start(args, num_args);
> + bar_idx = va_arg(args, int);
> + va_end(args);
> + }
> +
> + list_for_each_entry(mr, &mmio_info->mmio_list, list)
> + if (bar_idx == mr->bar_idx && offset >= mr->offset &&
> + offset < mr->offset + mr->size) {
> + offset -= mr->offset;
> +
> + return mr->addr + offset;
> + }
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL_NS_GPL(__libie_pci_get_mmio_addr, "LIBIE_PCI");
> +
> +/**
> + * __libie_pci_map_mmio_region - map PCI device MMIO region
> + * @mmio_info: struct to store the mapped MMIO region
> + * @offset: MMIO region start offset
> + * @size: MMIO region size
> + * @num_args: number of additional arguments present
> + *
> + * Return: true on success, false on memory map failure.
> + */
> +bool __libie_pci_map_mmio_region(struct libie_mmio_info *mmio_info,
> + resource_size_t offset,
> + resource_size_t size, int num_args, ...)
> +{
> + struct pci_dev *pdev = mmio_info->pdev;
> + struct libie_pci_mmio_region *mr;
> + resource_size_t pa;
> + void __iomem *va;
> + int bar_idx = 0;
> + va_list args;
> +
> + if (num_args) {
> + va_start(args, num_args);
> + bar_idx = va_arg(args, int);
> + va_end(args);
> + }
> +
> + if (offset + size > pci_resource_len(pdev, bar_idx))
> + return false;
> +
> + mr = libie_find_mmio_region(&mmio_info->mmio_list, offset, size,
> + bar_idx);
> + if (mr) {
> + pci_warn(pdev,
> + "Mapping of BAR%u (offset=%llu, size=%llu) intersecting region (offset=%llu, size=%llu) already exists\n",
> + bar_idx, (unsigned long long)mr->offset,
> + (unsigned long long)mr->size,
> + (unsigned long long)offset, (unsigned long long)size);
> + return mr->offset <= offset &&
> + mr->offset + mr->size >= offset + size;
> + }
> +
> + pa = pci_resource_start(pdev, bar_idx) + offset;
> + va = ioremap(pa, size);
> + if (!va) {
> + pci_err(pdev, "Failed to map BAR%u region\n", bar_idx);
> + return false;
> + }
> +
> + mr = kvzalloc_obj(*mr);
> + if (!mr) {
> + iounmap(va);
> + return false;
> + }
> +
> + mr->addr = va;
> + mr->offset = offset;
> + mr->size = size;
> + mr->bar_idx = bar_idx;
> +
> + list_add_tail(&mr->list, &mmio_info->mmio_list);
> +
> + return true;
> +}
> +EXPORT_SYMBOL_NS_GPL(__libie_pci_map_mmio_region, "LIBIE_PCI");
> +
> +/**
> + * libie_pci_unmap_fltr_regs - unmap selected PCI device MMIO regions
> + * @mmio_info: contains list of MMIO regions to unmap
> + * @fltr: returns true, if region is to be unmapped
> + */
> +void libie_pci_unmap_fltr_regs(struct libie_mmio_info *mmio_info,
> + bool (*fltr)(struct libie_mmio_info *mmio_info,
> + struct libie_pci_mmio_region *reg))
> +{
> + struct libie_pci_mmio_region *mr, *tmp;
> +
> + list_for_each_entry_safe(mr, tmp, &mmio_info->mmio_list, list) {
> + if (!fltr(mmio_info, mr))
> + continue;
> + iounmap(mr->addr);
> + list_del(&mr->list);
> + kvfree(mr);
> + }
> +}
> +EXPORT_SYMBOL_NS_GPL(libie_pci_unmap_fltr_regs, "LIBIE_PCI");
> +
> +/**
> + * libie_pci_unmap_all_mmio_regions - unmap all PCI device MMIO regions
> + * @mmio_info: contains list of MMIO regions to unmap
> + */
> +void libie_pci_unmap_all_mmio_regions(struct libie_mmio_info *mmio_info)
> +{
> + struct libie_pci_mmio_region *mr, *tmp;
> +
> + list_for_each_entry_safe(mr, tmp, &mmio_info->mmio_list, list) {
> + iounmap(mr->addr);
> + list_del(&mr->list);
> + kvfree(mr);
> + }
> +}
> +EXPORT_SYMBOL_NS_GPL(libie_pci_unmap_all_mmio_regions, "LIBIE_PCI");
> +
> +/**
> + * libie_pci_init_dev - enable and reserve PCI regions of the device
> + * @pdev: PCI device information
> + *
> + * Return: %0 on success, -%errno on failure.
> + */
> +int libie_pci_init_dev(struct pci_dev *pdev)
> +{
> + int err;
> +
> + err = pcim_enable_device(pdev);
> + if (err)
> + return err;
> +
> + for (int bar = 0; bar < PCI_STD_NUM_BARS; bar++)
> + if (pci_resource_flags(pdev, bar) & IORESOURCE_MEM) {
> + err = pcim_request_region(pdev, bar, pci_name(pdev));
> + if (err)
> + return err;
> + }
> +
> + err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> + if (err)
> + return err;
> +
> + pci_set_master(pdev);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(libie_pci_init_dev, "LIBIE_PCI");
> +
> +MODULE_DESCRIPTION("Common Ethernet PCI library");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/intel/libie/pci.h b/include/linux/intel/libie/pci.h
> new file mode 100644
> index 000000000000..effd072c55c8
> --- /dev/null
> +++ b/include/linux/intel/libie/pci.h
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* Copyright (C) 2025 Intel Corporation */
> +
> +#ifndef __LIBIE_PCI_H
> +#define __LIBIE_PCI_H
> +
> +#include <linux/pci.h>
> +
> +/**
> + * struct libie_pci_mmio_region - structure for MMIO region info
> + * @list: used to add a MMIO region to the list of MMIO regions in
> + * libie_mmio_info
> + * @addr: virtual address of MMIO region start
> + * @offset: start offset of the MMIO region
> + * @size: size of the MMIO region
> + * @bar_idx: BAR index to which the MMIO region belongs to
> + */
> +struct libie_pci_mmio_region {
> + struct list_head list;
> + void __iomem *addr;
> + resource_size_t offset;
> + resource_size_t size;
> + u16 bar_idx;
> +};
> +
> +/**
> + * struct libie_mmio_info - contains list of MMIO regions
> + * @pdev: PCI device pointer
> + * @mmio_list: list of MMIO regions
> + */
> +struct libie_mmio_info {
> + struct pci_dev *pdev;
> + struct list_head mmio_list;
> +};
> +
> +#define libie_pci_map_mmio_region(mmio_info, offset, size, ...) \
> + __libie_pci_map_mmio_region(mmio_info, offset, size, \
> + COUNT_ARGS(__VA_ARGS__), ##__VA_ARGS__)
> +
> +#define libie_pci_get_mmio_addr(mmio_info, offset, ...) \
> + __libie_pci_get_mmio_addr(mmio_info, offset, \
> + COUNT_ARGS(__VA_ARGS__), ##__VA_ARGS__)
> +
> +bool __libie_pci_map_mmio_region(struct libie_mmio_info *mmio_info,
> + resource_size_t offset, resource_size_t size,
> + int num_args, ...);
> +void __iomem *__libie_pci_get_mmio_addr(struct libie_mmio_info *mmio_info,
> + resource_size_t offset,
> + int num_args, ...);
> +void libie_pci_unmap_all_mmio_regions(struct libie_mmio_info *mmio_info);
> +void libie_pci_unmap_fltr_regs(struct libie_mmio_info *mmio_info,
> + bool (*fltr)(struct libie_mmio_info *mmio_info,
> + struct libie_pci_mmio_region *reg));
> +int libie_pci_init_dev(struct pci_dev *pdev);
> +
> +#endif /* __LIBIE_PCI_H */
> --
> 2.47.1
>
^ permalink raw reply
* Re: [PATCH v6 1/2] usb: xhci-pci: add AMD Promontory 21 PCI glue
From: Michal Pecio @ 2026-05-18 21:37 UTC (permalink / raw)
To: Jihong Min
Cc: Greg Kroah-Hartman, Mathias Nyman, Guenter Roeck, Jonathan Corbet,
Shuah Khan, Mario Limonciello, Basavaraj Natikar, linux-usb,
linux-hwmon, linux-doc, linux-pci, linux-kernel,
Mario Limonciello (AMD), Yaroslav Isakov
In-Reply-To: <144ec61c-4cc1-4986-a16c-7c1b99f3a72e@gmail.com>
On Tue, 19 May 2026 05:30:47 +0900, Jihong Min wrote:
> On 5/18/26 06:21, Michal Pecio wrote:
> > Instead of the X86 heuristic, would it be possible to build glue
> > code if and only if SENSORS_PROM21_XHCI is enabled?
> >
> > This seems to work:
> >
> > config SENSORS_PROM21_XHCI
> > tristate "AMD Promontory 21 xHCI temperature sensor"
> > - depends on USB_XHCI_PCI_PROM21
> > + depends on USB_XHCI_PCI
> >
> > config USB_XHCI_PCI_PROM21
> > tristate
> > - depends on X86
> > depends on USB_XHCI_PCI
> > - default USB_XHCI_PCI
> > + default USB_XHCI_PCI if SENSORS_PROM21_XHCI != 'n'
> > select AUXILIARY_BUS
> >
> > I don't know if it's the best way, perhaps it would be preferable
> > for the hwmon driver to select the glue, but then I'm not sure how
> > to force glue to become 'y' when xhci-pci is 'y'.
>
> I think I should keep the current hidden glue option for now.
>
> The PROM21 PCI glue is part of the PCI binding path for the xHCI
> controller when enabled, while SENSORS_PROM21_XHCI is only the
> optional user-visible hwmon driver. Tying the glue to the hwmon
> option would make the sensor option affect which driver binds the USB
> controller.
That's true.
Making this possible is the whole purpose of "if IS_ENABLED" here:
> static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
> {
> if (IS_ENABLED(CONFIG_USB_XHCI_PCI_RENESAS) &&
> pci_match_id(pci_ids_renesas, dev))
> return -ENODEV;
>
> + if (IS_ENABLED(CONFIG_USB_XHCI_PCI_PROM21) &&
> + pci_match_id(pci_ids_prom21, dev))
> + return -ENODEV;
> +
> return xhci_pci_common_probe(dev, id);
> }
Currently, you have a weird situation where xhci-pci-prom21 always
binds on x86 and xhci-pci on other platforms (with the unofficial PCIe
card you mentioned), plus the sensor cannot work on other platforms.
> As Guenter pointed out, that would be too strong; the USB controller
> should not depend on whether the optional hwmon driver is enabled.
One could further argue that neither should it care whether some hwmon
driver exists at all, or which kernel releases it exists in :)
Regards,
Michal
^ permalink raw reply
* Re: [PATCH] killswitch: add per-function short-circuit mitigation primitive
From: Paul Moore @ 2026-05-18 21:29 UTC (permalink / raw)
To: Song Liu
Cc: Sasha Levin, corbet, akpm, skhan, linux-doc, linux-kernel,
linux-kselftest, gregkh, linux-security-module
In-Reply-To: <CAPhsuW4TJRqQKXgcBYog8YgFLU2h2Zq9ReahxTYp_zpDyvO8AA@mail.gmail.com>
On Mon, May 18, 2026 at 2:31 AM Song Liu <song@kernel.org> wrote:
> On Thu, May 14, 2026 at 8:48 PM Paul Moore <paul@paul-moore.com> wrote:
> > On Thu, May 7, 2026 at 3:05 AM Sasha Levin <sashal@kernel.org> wrote:
> > >
> > > When a (security) issue goes public, fleets stay exposed until a patched kernel
> > > is built, distributed, and rebooted into.
> > >
> > > For many such issues the simplest mitigation is to stop calling the buggy
> > > function. Killswitch provides that. An admin writes:
> > >
> > > echo "engage af_alg_sendmsg -1" \
> > > > /sys/kernel/security/killswitch/control
> > >
> > > After this, af_alg_sendmsg() returns -EPERM on every call without
> > > running its body. The mitigation takes effect immediately, and is dropped on
> > > the next reboot.
> > >
> > > A lot of recent kernel issues sit in code paths most installs only have enabled
> > > to support a relative minority of users: AF_ALG, ksmbd, nf_tables, vsock, ax25,
> > > and friends.
> > >
> > > For most users, the cost of "this socket family stops working for the day" is
> > > much smaller than the cost of running a known vulnerable kernel until the fix
> > > land.
> > >
> > > Assisted-by: Claude:claude-opus-4-7
> > > Signed-off-by: Sasha Levin <sashal@kernel.org>
> > > ---
> > > Documentation/admin-guide/index.rst | 1 +
> > > Documentation/admin-guide/killswitch.rst | 159 ++++
> > > Documentation/admin-guide/tainted-kernels.rst | 8 +
> > > MAINTAINERS | 11 +
> > > include/linux/killswitch.h | 19 +
> > > include/linux/panic.h | 3 +-
> > > init/Kconfig | 2 +
> > > kernel/Kconfig.killswitch | 31 +
> > > kernel/Makefile | 1 +
> > > kernel/killswitch.c | 798 ++++++++++++++++++
> > > kernel/panic.c | 1 +
> > > lib/Kconfig.debug | 13 +
> > > lib/Makefile | 1 +
> > > lib/test_killswitch.c | 85 ++
> > > tools/testing/selftests/Makefile | 1 +
> > > tools/testing/selftests/killswitch/.gitignore | 1 +
> > > tools/testing/selftests/killswitch/Makefile | 8 +
> > > .../selftests/killswitch/cve_31431_test.c | 162 ++++
> > > .../selftests/killswitch/killswitch_test.sh | 147 ++++
> > > 19 files changed, 1451 insertions(+), 1 deletion(-)
> > > create mode 100644 Documentation/admin-guide/killswitch.rst
> > > create mode 100644 include/linux/killswitch.h
> > > create mode 100644 kernel/Kconfig.killswitch
> > > create mode 100644 kernel/killswitch.c
> > > create mode 100644 lib/test_killswitch.c
> > > create mode 100644 tools/testing/selftests/killswitch/.gitignore
> > > create mode 100644 tools/testing/selftests/killswitch/Makefile
> > > create mode 100644 tools/testing/selftests/killswitch/cve_31431_test.c
> > > create mode 100755 tools/testing/selftests/killswitch/killswitch_test.sh
> >
> > If we made Lockdown an LSM, we should probably also make killswitch an LSM.
>
> I don't think killswitch can stack with other LSMs. In fact, killswitch
> can be used to bypass other LSMs, for example:
>
> echo engage security_file_open 0 > /sys/kernel/security/killswitch/control
>
> will bypass all hooks on security_file_open.
From my perspective there are two different issues here: should
killswitch be a LSM, and should killswitch leverage kprobes to be able
to "kill" security related symbols. After all, are we okay with
killswitch killing capable() and friends?
In my opinion, making killswitch an LSM is more of a procedural item
that deals with how we view a capability like killswitch. I
personally view killswitch as somewhat similar to Lockdown, which is
why I made the suggestion.
The use of kprobes, while an interesting idea, presents problems as
allowing any kernel symbol to be killed introduces the potential for
security regressions. As a reminder, some LSMs, as well as other
kernel subsystems, have mechanisms in place to restrict root and/or
enforce one-way configuration locks; while many people equate "root"
with full control, in many cases today that is not strictly correct.
Yes, kprobes have been around for some time, this is not a new
problem, but killswitch makes it far more convenient and accessible to
do dangerous things with kprobes. If killswitch makes it past the RFC
stage without any significant changes to its kill mechanism, we may
need to start considering more liberal usage of NOKPROBE_SYMBOL()
which I think would be an unfortunate casualty.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 2/5] dma-heap: charge dma-buf memory via explicit memcg
From: T.J. Mercier @ 2026-05-18 21:17 UTC (permalink / raw)
To: Barry Song
Cc: Christian König, Albert Esteve, Tejun Heo, Johannes Weiner,
Michal Koutný, Jonathan Corbet, Shuah Khan, Sumit Semwal,
Michal Hocko, Roman Gushchin, Shakeel Butt, Muchun Song,
Andrew Morton, Benjamin Gaignard, Brian Starkey, John Stultz,
Christian Brauner, Paul Moore, James Morris, Serge E. Hallyn,
Stephen Smalley, Ondrej Mosnacek, Shuah Khan, cgroups, linux-doc,
linux-kernel, linux-media, dri-devel, linaro-mm-sig, linux-mm,
linux-security-module, selinux, linux-kselftest, mripard,
echanude
In-Reply-To: <CABdmKX0gqg309hcXcOHSj_yTg0h1zwDL34GDk8mX3wp4YoyfDg@mail.gmail.com>
On Mon, May 18, 2026 at 2:12 PM T.J. Mercier <tjmercier@google.com> wrote:
>
> On Sat, May 16, 2026 at 1:40 AM Barry Song <baohua@kernel.org> wrote:
> >
> > On Wed, May 13, 2026 at 2:54 AM T.J. Mercier <tjmercier@google.com> wrote:
> > >
> > > On Tue, May 12, 2026 at 3:14 AM Christian König
> > > <christian.koenig@amd.com> wrote:
> > > >
> > > > On 5/12/26 11:10, Albert Esteve wrote:
> > > > > On embedded platforms a central process often allocates dma-buf
> > > > > memory on behalf of client applications. Without a way to
> > > > > attribute the charge to the requesting client's cgroup, the
> > > > > cost lands on the allocator, making per-cgroup memory limits
> > > > > ineffective for the actual consumers.
> > > > >
> > > > > Add charge_pid_fd to struct dma_heap_allocation_data. When set to
> > > > > a valid pidfd, DMA_HEAP_IOCTL_ALLOC resolves the target task's
> > > > > memcg and charges the buffer there via mem_cgroup_charge_dmabuf()
> > > > > inside dma_heap_buffer_alloc(). Without charge_pid_fd, and with
> > > > > the mem_accounting module parameter enabled, the buffer is charged
> > > > > to the allocator's own cgroup.
> > > > >
> > > > > Additionally, commit 3c227be90659 ("dma-buf: system_heap: account for
> > > > > system heap allocation in memcg") adds __GFP_ACCOUNT to system-heap
> > > > > page allocations. Keeping __GFP_ACCOUNT would charge the same pages
> > > > > twice (once to kmem, once to MEMCG_DMABUF), thus remove it and route
> > > > > all accounting through a single MEMCG_DMABUF path.
> > > > >
> > > > > Usage examples:
> > > > >
> > > > > 1. Central allocator charging to a client at allocation time.
> > > > > The allocator knows the client's PID (e.g., from binder's
> > > > > sender_pid) and uses pidfd to attribute the charge:
> > > > >
> > > > > pid_t client_pid = txn->sender_pid;
> > > > > int pidfd = pidfd_open(client_pid, 0);
> > > > >
> > > > > struct dma_heap_allocation_data alloc = {
> > > > > .len = buffer_size,
> > > > > .fd_flags = O_RDWR | O_CLOEXEC,
> > > > > .charge_pid_fd = pidfd,
> > > > > };
> > > > > ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, &alloc);
> > > > > close(pidfd);
> > > > > /* alloc.fd is now charged to client's cgroup */
> > > > >
> > > > > 2. Default allocation (no pidfd, mem_accounting=1).
> > > > > When charge_pid_fd is not set and the mem_accounting module
> > > > > parameter is enabled, the buffer is charged to the allocator's
> > > > > own cgroup:
> > > > >
> > > > > struct dma_heap_allocation_data alloc = {
> > > > > .len = buffer_size,
> > > > > .fd_flags = O_RDWR | O_CLOEXEC,
> > > > > };
> > > > > ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, &alloc);
> > > > > /* charged to current process's cgroup */
> > > > >
> > > > > Current limitations:
> > > > >
> > > > > - Single-owner model: a dma-buf carries one memcg charge regardless of
> > > > > how many processes share it. Means only the first owner (and exporter)
> > > > > of the shared buffer bears the charge.
> > > > > - Only memcg accounting supported. While this makes sense for system
> > > > > heap buffers, other heaps (e.g., CMA heaps) will require selectively
> > > > > charging also for the dmem controller.
> > > >
> > > > Well that doesn't looks soo bad, it at least seems to tackle the problem at hand for Android and some of other embedded use cases.
> > >
> > > Yeah I think this might work. I know of 3 cases, and it trivially
> > > solves the first two. The third requires some work on our end to
> > > extend our userspace interfaces to include the pidfd but it seems
> > > doable. I'm checking with our graphics folks.
> > >
> > > 1) Direct allocation from user (e.g. app -> allocation ioctl on
> > > /dev/dma_heap/foo)
> > > No changes required to userspace. mem_accounting=1 charges the app.
> > >
> > > 2) Single hop remote allocation (e.g. app -> AHardwareBuffer_allocate
> > > -> gralloc)
> > > gralloc has the caller's pid as described in the commit message. Open
> > > a pidfd and pass it in the dma_heap_allocation_data.
> > >
> > > 3) Double hop remote allocation (e.g. app -> dequeueBuffer ->
> > > SurfaceFlinger -> gralloc)
> > > In this case gralloc knows SurfaceFlinger's pid, but not the app's. So
> > > we need to add the app's pidfd to the SurfaceFlinger -> gralloc
> > > interface, or transfer the memcg charge from SurfaceFlinger to the app
> > > after the allocation.
> > > It'd be nice to avoid the charge transfer option entirely, but if we
> > > need it that doesn't seem so bad in this case because it's a bulk
> > > charge for the entire dmabuf rather than per-page. So the exporter
> > > doesn't need to get involved (we wouldn't need a new dma_buf_op) and
> > > we wouldn't have to worry about looping and locking for each page.
> > >
> >
> > Hi T.J.,
> >
> > Your description of the three different cases sounds very interesting.
> > It helps me understand how difficult it can be to correctly charge
> > dma-buf in the current user scenarios.
> >
> > I’m wondering where I can find Android userspace code that transfers
> > the PID of RPC callers. Do we have any existing sample code in Android
> > for this?
>
> Hi Barry,
>
> In Java android.os.Binder.getCallingPid() will provide it. Here
... let me try again
Here are some examples from the framework code:
https://cs.android.com/search?q=getCallingPid%20f:ActivityManager&sq=&ss=android%2Fplatform%2Fsuperproject
In native code we have AIBinder_getCallingPid and
android::IPCThreadState::self()->getCallingPid() (or
android::hardware::IPCThreadState::self()->getCallingPid() for HIDL)
https://cs.android.com/search?q=getCallingPid%20l:cpp%20-f:prebuilt&ss=android%2Fplatform%2Fsuperproject
^ permalink raw reply
* Re: [PATCH RFC 2/5] dma-heap: charge dma-buf memory via explicit memcg
From: T.J. Mercier @ 2026-05-18 21:12 UTC (permalink / raw)
To: Barry Song
Cc: Christian König, Albert Esteve, Tejun Heo, Johannes Weiner,
Michal Koutný, Jonathan Corbet, Shuah Khan, Sumit Semwal,
Michal Hocko, Roman Gushchin, Shakeel Butt, Muchun Song,
Andrew Morton, Benjamin Gaignard, Brian Starkey, John Stultz,
Christian Brauner, Paul Moore, James Morris, Serge E. Hallyn,
Stephen Smalley, Ondrej Mosnacek, Shuah Khan, cgroups, linux-doc,
linux-kernel, linux-media, dri-devel, linaro-mm-sig, linux-mm,
linux-security-module, selinux, linux-kselftest, mripard,
echanude
In-Reply-To: <CAGsJ_4zjrFJYQQsLThTGXR6g+2PXzeAhjyDpLHfDFqVViWvyBQ@mail.gmail.com>
On Sat, May 16, 2026 at 1:40 AM Barry Song <baohua@kernel.org> wrote:
>
> On Wed, May 13, 2026 at 2:54 AM T.J. Mercier <tjmercier@google.com> wrote:
> >
> > On Tue, May 12, 2026 at 3:14 AM Christian König
> > <christian.koenig@amd.com> wrote:
> > >
> > > On 5/12/26 11:10, Albert Esteve wrote:
> > > > On embedded platforms a central process often allocates dma-buf
> > > > memory on behalf of client applications. Without a way to
> > > > attribute the charge to the requesting client's cgroup, the
> > > > cost lands on the allocator, making per-cgroup memory limits
> > > > ineffective for the actual consumers.
> > > >
> > > > Add charge_pid_fd to struct dma_heap_allocation_data. When set to
> > > > a valid pidfd, DMA_HEAP_IOCTL_ALLOC resolves the target task's
> > > > memcg and charges the buffer there via mem_cgroup_charge_dmabuf()
> > > > inside dma_heap_buffer_alloc(). Without charge_pid_fd, and with
> > > > the mem_accounting module parameter enabled, the buffer is charged
> > > > to the allocator's own cgroup.
> > > >
> > > > Additionally, commit 3c227be90659 ("dma-buf: system_heap: account for
> > > > system heap allocation in memcg") adds __GFP_ACCOUNT to system-heap
> > > > page allocations. Keeping __GFP_ACCOUNT would charge the same pages
> > > > twice (once to kmem, once to MEMCG_DMABUF), thus remove it and route
> > > > all accounting through a single MEMCG_DMABUF path.
> > > >
> > > > Usage examples:
> > > >
> > > > 1. Central allocator charging to a client at allocation time.
> > > > The allocator knows the client's PID (e.g., from binder's
> > > > sender_pid) and uses pidfd to attribute the charge:
> > > >
> > > > pid_t client_pid = txn->sender_pid;
> > > > int pidfd = pidfd_open(client_pid, 0);
> > > >
> > > > struct dma_heap_allocation_data alloc = {
> > > > .len = buffer_size,
> > > > .fd_flags = O_RDWR | O_CLOEXEC,
> > > > .charge_pid_fd = pidfd,
> > > > };
> > > > ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, &alloc);
> > > > close(pidfd);
> > > > /* alloc.fd is now charged to client's cgroup */
> > > >
> > > > 2. Default allocation (no pidfd, mem_accounting=1).
> > > > When charge_pid_fd is not set and the mem_accounting module
> > > > parameter is enabled, the buffer is charged to the allocator's
> > > > own cgroup:
> > > >
> > > > struct dma_heap_allocation_data alloc = {
> > > > .len = buffer_size,
> > > > .fd_flags = O_RDWR | O_CLOEXEC,
> > > > };
> > > > ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, &alloc);
> > > > /* charged to current process's cgroup */
> > > >
> > > > Current limitations:
> > > >
> > > > - Single-owner model: a dma-buf carries one memcg charge regardless of
> > > > how many processes share it. Means only the first owner (and exporter)
> > > > of the shared buffer bears the charge.
> > > > - Only memcg accounting supported. While this makes sense for system
> > > > heap buffers, other heaps (e.g., CMA heaps) will require selectively
> > > > charging also for the dmem controller.
> > >
> > > Well that doesn't looks soo bad, it at least seems to tackle the problem at hand for Android and some of other embedded use cases.
> >
> > Yeah I think this might work. I know of 3 cases, and it trivially
> > solves the first two. The third requires some work on our end to
> > extend our userspace interfaces to include the pidfd but it seems
> > doable. I'm checking with our graphics folks.
> >
> > 1) Direct allocation from user (e.g. app -> allocation ioctl on
> > /dev/dma_heap/foo)
> > No changes required to userspace. mem_accounting=1 charges the app.
> >
> > 2) Single hop remote allocation (e.g. app -> AHardwareBuffer_allocate
> > -> gralloc)
> > gralloc has the caller's pid as described in the commit message. Open
> > a pidfd and pass it in the dma_heap_allocation_data.
> >
> > 3) Double hop remote allocation (e.g. app -> dequeueBuffer ->
> > SurfaceFlinger -> gralloc)
> > In this case gralloc knows SurfaceFlinger's pid, but not the app's. So
> > we need to add the app's pidfd to the SurfaceFlinger -> gralloc
> > interface, or transfer the memcg charge from SurfaceFlinger to the app
> > after the allocation.
> > It'd be nice to avoid the charge transfer option entirely, but if we
> > need it that doesn't seem so bad in this case because it's a bulk
> > charge for the entire dmabuf rather than per-page. So the exporter
> > doesn't need to get involved (we wouldn't need a new dma_buf_op) and
> > we wouldn't have to worry about looping and locking for each page.
> >
>
> Hi T.J.,
>
> Your description of the three different cases sounds very interesting.
> It helps me understand how difficult it can be to correctly charge
> dma-buf in the current user scenarios.
>
> I’m wondering where I can find Android userspace code that transfers
> the PID of RPC callers. Do we have any existing sample code in Android
> for this?
Hi Barry,
In Java android.os.Binder.getCallingPid() will provide it. Here
> > > I'm just not sure if this is future prove and will work for all use cases, e.g. cloud gaming, native context for automotive etc...
>
> Thanks
> Barry
^ permalink raw reply
* Re: [PATCH v4 04/30] KVM: x86: Add KVM_[GS]ET_CLOCK_GUEST for accurate KVM clock migration
From: Sean Christopherson @ 2026-05-18 20:52 UTC (permalink / raw)
To: David Woodhouse
Cc: Dongli Zhang, kvm, Paolo Bonzini, Jonathan Corbet, Shuah Khan,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
Dave Hansen, Vitaly Kuznetsov, x86, Marc Zyngier, Juergen Gross,
Boris Ostrovsky, Paul Durrant, Jonathan Cameron, Sascha Bischoff,
Jack Allister, Joey Gouly, joe.jin, linux-doc, linux-kernel,
xen-devel, linux-kselftest
In-Reply-To: <d3c461415e05345a9b82e6f995828c1ae64a4e61.camel@infradead.org>
On Mon, May 18, 2026, David Woodhouse wrote:
> On Mon, 2026-05-18 at 00:52 -0700, Dongli Zhang wrote:
> > On 5/9/26 3:46 PM, David Woodhouse wrote:
>
> Huh, I didn't write that then; it isn't September yet. Did you mean
> 2026-05-09? We aren't all in the US...
>
> Strictly speaking, you just misattributed a quote of mine, which is
> very poor form :)
But also pretty darn hilarious given that the thread is all about accurate
timekeeping :-)
^ permalink raw reply
* Re: [PATCH] PCI/AER: Clear non-fatal errors on AER recovery failure
From: Yury M. @ 2026-05-18 20:49 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: bhelgaas, mahesh, oohall, corbet, skhan, linux-pci, linux-doc,
linux-kernel, linuxppc-dev, Lukas Wunner
In-Reply-To: <20260518202903.GA641158@bhelgaas>
Current behavior has existed for a long time and I could easily imagine
that there is software which relies on the fact that the system is in a
non-modified state if AER recovery failed. The software can analyze the
system and do cleanup afterwards. Sometimes, if something fails in the
system, it is better to have it in a non-modified state.
In short, I just wanted to preserve the current logic by default because
there is a chance that we have software which relies on the current
behavior.
On 5/18/26 21:29, Bjorn Helgaas wrote:
> [+cc Lukas]
>
> On Mon, May 18, 2026 at 02:23:36PM +0100, Yury Murashka wrote:
>> pci_aer_clear_nonfatal_status() is not called when AER recovery fails.
>> If a new AER error is subsequently reported, the AER driver calls
>> find_source_device() to find the source of the error. It rescans the
>> whole bus and picks the first device reporting an AER error. Because the
>> previous error was never cleared, the error is attributed to the wrong
>> device and AER recovery is started for the wrong device.
>>
>> Add a kernel boot parameter pci=aer_clear_on_recovery_failure to clear
>> AER error status even when recovery fails, preventing stale errors from
>> causing incorrect device identification on subsequent AER events.
> Why should we add a kernel parameter for this? How would a user
> decide whether to use the parameter? Are there cases where we
> find the source of the first error, but we *wouldn't* want to clear
> it if recovery fails?
^ permalink raw reply
* Re: [PATCH] nios2: remove the architecture
From: Wolfram Sang @ 2026-05-18 20:46 UTC (permalink / raw)
To: Simon Schuster
Cc: Peter Zijlstra, Arnd Bergmann, Ethan Nelson-Moore, Dinh Nguyen,
linux-doc, devicetree, workflows, Linux-Arch, dmaengine,
linux-i2c, linux-iio, Netdev, linux-pci, linux-pwm,
linux-hardening, linux-kbuild, linux-csky@vger.kernel.org,
Jonathan Corbet, Shuah Khan, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Daniel Lezcano, Thomas Gleixner, Alex Shi,
Yanteng Si, Dongliang Mu, Hu Haowen, Kees Cook, Oleg Nesterov,
Will Deacon, Aneesh Kumar K.V (Arm), Andrew Morton,
Nicholas Piggin, Vinod Koul, Frank Li, Dave Penkler, Andi Shyti,
Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Lorenzo Pieralisi, Krzysztof WilczyDski,
Andreas Oetken
In-Reply-To: <20260518172444.zyd47mcagrcwu7wt@dev-vm-schuster>
Hi Simon,
> downstream. In other respects, we try to be good citizens and contribute
> bugfixes as well as required cleanups (such as implementing clone3 [2]
> and fixing its flag behaviour on 32-bit architectures) as they come up.
Well, this is definitely 'good citizen'...
> If desired, we also would be happy to intensify our support regarding
> reviews or testing to share the maintnance burden if it helps to keep
> nios2 in mainline a bit longer.
... but given this, you might want to get added in MAINTAINERS as
reviewer (or even maintainer) for nios2? Besides that your efforts are
already worth it in my book, it would also ensure you get CCed on
patches like this. Then, you are not depending on people like Arnd
putting you in the loop manually.
Happy hacking,
Wolfram
^ permalink raw reply
* Re: [PATCH v6 1/2] usb: xhci-pci: add AMD Promontory 21 PCI glue
From: Michal Pecio @ 2026-05-18 20:34 UTC (permalink / raw)
To: Guenter Roeck
Cc: Jihong Min, Greg Kroah-Hartman, Mathias Nyman, Jonathan Corbet,
Shuah Khan, Mario Limonciello, Basavaraj Natikar, linux-usb,
linux-hwmon, linux-doc, linux-pci, linux-kernel,
Mario Limonciello (AMD), Yaroslav Isakov
In-Reply-To: <f05e075d-a87e-49b5-95f8-5858d21acf64@roeck-us.net>
On Mon, 18 May 2026 03:55:52 -0700, Guenter Roeck wrote:
> On 5/17/26 14:21, Michal Pecio wrote:
> > Instead of the X86 heuristic, would it be possible to build glue
> > code if and only if SENSORS_PROM21_XHCI is enabled?
> >
> > This seems to work:
> >
> > config SENSORS_PROM21_XHCI
> > tristate "AMD Promontory 21 xHCI temperature sensor"
> > - depends on USB_XHCI_PCI_PROM21
> > + depends on USB_XHCI_PCI
> >
> > config USB_XHCI_PCI_PROM21
> > tristate
> > - depends on X86
> > depends on USB_XHCI_PCI
> > - default USB_XHCI_PCI
> > + default USB_XHCI_PCI if SENSORS_PROM21_XHCI != 'n'
> > select AUXILIARY_BUS
> >
> > I don't know if it's the best way, perhaps it would be preferable for
> > the hwmon driver to select the glue, but then I'm not sure how to force
> > glue to become 'y' when xhci-pci is 'y'.
> >
>
> Unless I am missing something, that would disable the entire controller
> if the hwmon device is not enabled. That seems a bit draconian to me.
I haven't tested (I don't have this chipset), but it should work like
the similar xhci-pci-renesas module, which I'm familiar with.
When the special unicorn module is disabled by Kconfig, xhci-pci no
longer rejects its devices and works with them normally, like it always
did before the unicorn module even existed.
It should be the same with xhci-pci-prom21. You don't need to enable
this module to use USB, only for the special functions. So if hwmon
is disabled then you can disable it too.
I always found this dual-driver solution (for Renesas) rather ugly and
confusing, but so far it's the least bad option tried. Hmm, maybe the
next iteration should be an aux bus interface for FW loaders...
Regards,
Michal
^ permalink raw reply
* Re: [PATCH v6 1/2] usb: xhci-pci: add AMD Promontory 21 PCI glue
From: Jihong Min @ 2026-05-18 20:30 UTC (permalink / raw)
To: Michal Pecio
Cc: Greg Kroah-Hartman, Mathias Nyman, Guenter Roeck, Jonathan Corbet,
Shuah Khan, Mario Limonciello, Basavaraj Natikar, linux-usb,
linux-hwmon, linux-doc, linux-pci, linux-kernel,
Mario Limonciello (AMD), Yaroslav Isakov
In-Reply-To: <20260517232147.34931718.michal.pecio@gmail.com>
On 5/18/26 06:21, Michal Pecio wrote:
> Instead of the X86 heuristic, would it be possible to build glue
> code if and only if SENSORS_PROM21_XHCI is enabled?
>
> This seems to work:
>
> config SENSORS_PROM21_XHCI
> tristate "AMD Promontory 21 xHCI temperature sensor"
> - depends on USB_XHCI_PCI_PROM21
> + depends on USB_XHCI_PCI
>
> config USB_XHCI_PCI_PROM21
> tristate
> - depends on X86
> depends on USB_XHCI_PCI
> - default USB_XHCI_PCI
> + default USB_XHCI_PCI if SENSORS_PROM21_XHCI != 'n'
> select AUXILIARY_BUS
>
> I don't know if it's the best way, perhaps it would be preferable for
> the hwmon driver to select the glue, but then I'm not sure how to force
> glue to become 'y' when xhci-pci is 'y'.
I think I should keep the current hidden glue option for now.
The PROM21 PCI glue is part of the PCI binding path for the xHCI controller
when enabled, while SENSORS_PROM21_XHCI is only the optional user-visible
hwmon driver. Tying the glue to the hwmon option would make the sensor
option
affect which driver binds the USB controller. As Guenter pointed out, that
would be too strong; the USB controller should not depend on whether the
optional hwmon driver is enabled.
So I would prefer to keep USB_XHCI_PCI_PROM21 as hidden plumbing that
follows
USB_XHCI_PCI, and keep SENSORS_PROM21_XHCI as the user-visible sensor
option.
> +static int prom21_xhci_create_auxdev(struct pci_dev *pdev)
> +{
> + struct prom21_xhci_auxdev *prom21_auxdev;
> + struct usb_hcd *hcd = pci_get_drvdata(pdev);
> +
> + if (!hcd)
> + return -ENODEV;
>
> Shouldn't be necessary after successful xhci_pci_common_probe().
Agreed. I removed the unnecessary NULL check from
prom21_xhci_create_auxdev() locally for v7.
> + prom21_auxdev->id = ida_alloc(&prom21_xhci_auxdev_ida, GFP_KERNEL);
> + if (prom21_auxdev->id < 0) {
> + int ret = prom21_auxdev->id;
> +
> + devres_free(prom21_auxdev);
> + return ret;
> + }
> +
> + prom21_auxdev->auxdev = auxiliary_device_create(&pdev->dev,
> + KBUILD_MODNAME, "hwmon",
> + &prom21_auxdev->pdata,
> + prom21_auxdev->id);
> + if (!prom21_auxdev->auxdev) {
> + ida_free(&prom21_xhci_auxdev_ida, prom21_auxdev->id);
> + devres_free(prom21_auxdev);
> + return -ENOMEM;
>
> The usual "goto error" pattern could be used instead of increasingly
> long sequences of xxx_free() calls.
Agreed. I changed prom21_xhci_create_auxdev() to use a goto-based
cleanup path
locally for v7.
> It seems that these three functions above are everything that you truly
> want to add; the rest is boilerplate required by this two-module scheme
> to work, plus ID tables which must be duplicated and kept in sync.
>
> I wonder if a separate module is really justified, as opposed to simply
> linking this file into xhci_pci.ko when directed by Kconfig.
>
> The downside would be slightly higher memory usage on systems where the
> hwmon driver is enabled but not needed. OTOH, same systems would likely
> see reduced disk waste.
I understand the concern. Linking the PROM21 auxiliary-device publisher
into xhci_pci.ko would reduce some boilerplate and avoid the extra PCI
driver, while still keeping the hwmon driver itself separate.
The reason I kept the current split is that the earlier review direction
was to keep the hwmon functionality out of xhci-pci and bind a
drivers/hwmon driver through an auxiliary device. The current PROM21 PCI
glue keeps the PROM21-specific auxiliary-device lifetime handling outside
the common xhci-pci driver and leaves xhci-pci.c with only the PCI ID
handoff, similar in spirit to the Renesas handoff path.
That said, I agree this is a tradeoff. If Mathias or the USB maintainers
prefer the PROM21 auxiliary-device publisher to be linked into xhci_pci.ko
instead of being a separate PCI glue driver, I can rework it in that
direction while still keeping the hwmon driver under drivers/hwmon and
bound through the auxiliary bus.
Sincerely,
Jihong Min
^ permalink raw reply
* Re: [PATCH] PCI/AER: Clear non-fatal errors on AER recovery failure
From: Bjorn Helgaas @ 2026-05-18 20:29 UTC (permalink / raw)
To: Yury Murashka
Cc: bhelgaas, mahesh, oohall, corbet, skhan, linux-pci, linux-doc,
linux-kernel, linuxppc-dev, Lukas Wunner
In-Reply-To: <CAPzpGcRCTCZtaX1EVaJNZ103THZKsoszZduY7=gwfYdcrMo-SQ@mail.gmail.com>
[+cc Lukas]
On Mon, May 18, 2026 at 02:23:36PM +0100, Yury Murashka wrote:
> pci_aer_clear_nonfatal_status() is not called when AER recovery fails.
> If a new AER error is subsequently reported, the AER driver calls
> find_source_device() to find the source of the error. It rescans the
> whole bus and picks the first device reporting an AER error. Because the
> previous error was never cleared, the error is attributed to the wrong
> device and AER recovery is started for the wrong device.
>
> Add a kernel boot parameter pci=aer_clear_on_recovery_failure to clear
> AER error status even when recovery fails, preventing stale errors from
> causing incorrect device identification on subsequent AER events.
Why should we add a kernel parameter for this? How would a user
decide whether to use the parameter? Are there cases where we
find the source of the first error, but we *wouldn't* want to clear
it if recovery fails?
^ permalink raw reply
* Re: [PATCH mm-unstable v17 04/14] mm/khugepaged: generalize __collapse_huge_page_* for mTHP support
From: Lorenzo Stoakes @ 2026-05-18 19:32 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Wei Yang, Lance Yang, npache, linux-doc, linux-kernel, linux-mm,
linux-trace-kernel, aarcange, akpm, anshuman.khandual, apopple,
baohua, baolin.wang, byungchul, catalin.marinas, cl, corbet,
dave.hansen, dev.jain, gourry, hannes, hughd, jack, jackmanb,
jannh, jglisse, joshua.hahnjy, kas, liam, mathieu.desnoyers,
matthew.brost, mhiramat, mhocko, peterx, pfalcato, rakie.kim,
raquini, rdunlap, rientjes, rostedt, rppt, ryan.roberts, shivankg,
sunnanyong, surenb, thomas.hellstrom, tiwai, usamaarif642, vbabka,
vishal.moola, wangkefeng.wang, will, willy, yang, ying.huang, ziy,
zokeefe
In-Reply-To: <9b33339e-157a-45b7-942e-3be3418a5142@kernel.org>
On Mon, May 18, 2026 at 03:16:11PM +0200, David Hildenbrand (Arm) wrote:
> On 5/14/26 05:10, Wei Yang wrote:
> > On Tue, May 12, 2026 at 03:42:02PM +0800, Lance Yang wrote:
> >>
> >> On Mon, May 11, 2026 at 12:58:04PM -0600, Nico Pache wrote:
> >>> generalize the order of the __collapse_huge_page_* and collapse_max_*
> >>> functions to support future mTHP collapse.
> >>>
> >>> The current mechanism for determining collapse with the
> >>> khugepaged_max_ptes_none value is not designed with mTHP in mind. This
> >>> raises a key design issue: if we support user defined max_pte_none values
> >>> (even those scaled by order), a collapse of a lower order can introduces
> >>> an feedback loop, or "creep", when max_ptes_none is set to a value greater
> >>> than HPAGE_PMD_NR / 2. [1]
> >>>
> >>> With this configuration, a successful collapse to order N will populate
> >>> enough pages to satisfy the collapse condition on order N+1 on the next
> >>> scan. This leads to unnecessary work and memory churn.
> >>>
> >>> To fix this issue introduce a helper function that will limit mTHP
> >>> collapse support to two max_ptes_none values, 0 and HPAGE_PMD_NR - 1.
> >>> This effectively supports two modes: [2]
> >>>
> >>> - max_ptes_none=0: never collapses if it encounters an empty PTE or a PTE
> >>> that maps the shared zeropage. Consequently, no memory bloat.
> >>> - max_ptes_none=511 (on 4k pagesz): Always collapse to the highest
> >>> available mTHP order.
> >>>
> >>> This removes the possiblilty of "creep", while not modifying any uAPI
> >>> expectations. A warning will be emitted if any non-supported
> >>> max_ptes_none value is configured with mTHP enabled.
> >>>
> >>> mTHP collapse will not honor the khugepaged_max_ptes_shared or
> >>> khugepaged_max_ptes_swap parameters, and will fail if it encounters a
> >>> shared or swapped entry.
> >>>
> >>> No functional changes in this patch; however it defines future behavior
> >>> for mTHP collapse.
> >>>
> >>> [1] - https://lore.kernel.org/all/e46ab3ab-a3d7-4fb7-9970-d0704bd5d05a@arm.com
> >>> [2] - https://lore.kernel.org/all/37375ace-5601-4d6c-9dac-d1c8268698e9@redhat.com
> >>>
> >>> Co-developed-by: Dev Jain <dev.jain@arm.com>
> >>> Signed-off-by: Dev Jain <dev.jain@arm.com>
> >>> Signed-off-by: Nico Pache <npache@redhat.com>
> >>> ---
> >>> include/trace/events/huge_memory.h | 3 +-
> >>> mm/khugepaged.c | 117 ++++++++++++++++++++---------
> >>> 2 files changed, 85 insertions(+), 35 deletions(-)
> >>>
> >>> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> >>> index bcdc57eea270..443e0bd13fdb 100644
> >>> --- a/include/trace/events/huge_memory.h
> >>> +++ b/include/trace/events/huge_memory.h
> >>> @@ -39,7 +39,8 @@
> >>> EM( SCAN_STORE_FAILED, "store_failed") \
> >>> EM( SCAN_COPY_MC, "copy_poisoned_page") \
> >>> EM( SCAN_PAGE_FILLED, "page_filled") \
> >>> - EMe(SCAN_PAGE_DIRTY_OR_WRITEBACK, "page_dirty_or_writeback")
> >>> + EM(SCAN_PAGE_DIRTY_OR_WRITEBACK, "page_dirty_or_writeback") \
> >>> + EMe(SCAN_INVALID_PTES_NONE, "invalid_ptes_none")
> >>>
> >>> #undef EM
> >>> #undef EMe
> >>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> >>> index f68853b3caa7..27465161fa6d 100644
> >>> --- a/mm/khugepaged.c
> >>> +++ b/mm/khugepaged.c
> >>> @@ -61,6 +61,7 @@ enum scan_result {
> >>> SCAN_COPY_MC,
> >>> SCAN_PAGE_FILLED,
> >>> SCAN_PAGE_DIRTY_OR_WRITEBACK,
> >>> + SCAN_INVALID_PTES_NONE,
> >>> };
> >>>
> >>> #define CREATE_TRACE_POINTS
> >>> @@ -353,37 +354,60 @@ static bool pte_none_or_zero(pte_t pte)
> >>> * PTEs for the given collapse operation.
> >>> * @cc: The collapse control struct
> >>> * @vma: The vma to check for userfaultfd
> >>> + * @order: The folio order being collapsed to
> >>> *
> >>> * Return: Maximum number of none-page or zero-page PTEs allowed for the
> >>> * collapse operation.
> >>> */
> >>> -static unsigned int collapse_max_ptes_none(struct collapse_control *cc,
> >>> - struct vm_area_struct *vma)
> >>> +static int collapse_max_ptes_none(struct collapse_control *cc,
> >>> + struct vm_area_struct *vma, unsigned int order)
> >>> {
> >>> + unsigned int max_ptes_none = khugepaged_max_ptes_none;
> >>> // If the vma is userfaultfd-armed, allow no none-page or zero-page PTEs.
> >>
> >> One thing I still want to call out: kernel code usually uses C-style
> >> comments :)
> >>
> >>> if (vma && userfaultfd_armed(vma))
> >>> return 0;
> >>> // for MADV_COLLAPSE, allow any none-page or zero-page PTEs.
> >>> if (!cc->is_khugepaged)
> >>> return HPAGE_PMD_NR;
> >>> - // For all other cases repect the user defined maximum.
> >>> - return khugepaged_max_ptes_none;
> >>> + // for PMD collapse, respect the user defined maximum.
> >>> + if (is_pmd_order(order))
> >>> + return max_ptes_none;
> >>> + /* Zero/non-present collapse disabled. */
> >>> + if (!max_ptes_none)
> >>> + return 0;
> >>> + // for mTHP collapse with the sysctl value set to KHUGEPAGED_MAX_PTES_LIMIT,
> >>> + // scale the maximum number of PTEs to the order of the collapse.
> >>> + if (max_ptes_none == KHUGEPAGED_MAX_PTES_LIMIT)
> >>> + return (1 << order) - 1;
> >>> +
> >>> + // We currently only support max_ptes_none values of 0 or KHUGEPAGED_MAX_PTES_LIMIT.
> >>> + // Emit a warning and return -EINVAL.
> >>> + pr_warn_once("mTHP collapse only supports max_ptes_none values of 0 or %u\n",
> >>> + KHUGEPAGED_MAX_PTES_LIMIT);
> >>
> >> Maybe fallback to 0 instead, as David suggested earlier?
> >>
> >
> > It looks reasonable to fallback to 0.
> >
> > But as the updated Document says in patch 14:
> >
> > For mTHP collapse, only 0 or (HPAGE_PMD_NR - 1) are supported. Any other
> > value will emit a warning and no mTHP collapse will be attempted.
> >
> > This is why it does like this now.
> >
> > mthp_collapse()
> > max_ptes_none = collapse_max_ptes_none();
> > if (max_ptes_none < 0)
> > return collapsed;
> >
> >> max_ptes_none is mostly legacy PMD THP behavior. mTHP is new, and any
> >> intermediate value in (0, KHUGEPAGED_MAX_PTES_LIMIT) would implicitly
> >> disable it :(
> >>
> >
> > So it depends on what we want to do here :-)
> >
> > For me, I would vote for fallback to 0.
>
> At this point I'll prefer to not return errors from collapse_max_ptes_none().
> It's just rather awkward to return an error deep down in collapse code for a
> configuration problem.
>
> For mthp collapse, we only support max_ptes_none==0 and
> max_ptes_none=="HPAGE_PMD_NR - 1" (default).
>
> If another value is specified while collapsing mTHP, print a warning and treat
> it as 0 (save value, no creep, no memory waste).
>
> In a sense, this is similar to how we handle max_ptes_shared + max_ptes_swap:
> for mTHP: we always treat them as being 0 for mTHP collapse (and don't issue a
> warning, because we would issue a warning with the default settings).
>
> @Lorenzo, fine with you?
Yes 100%, this sounds sensible both in terms of the error and the default. Let's
keep our lives simple(-ish) please :)
>
> --
> Cheers,
>
> David
Cheers, Lorenzo
^ permalink raw reply
* [PATCH 6/6] selftests/damon: add a test for update_schemes_quota_goals
From: Maksym Shcherba @ 2026-05-18 19:09 UTC (permalink / raw)
To: sj, akpm
Cc: david, ljs, liam, vbabka, rppt, surenb, mhocko, corbet, skhan,
damon, linux-mm, linux-kernel, linux-doc, linux-kselftest,
Maksym Shcherba
In-Reply-To: <20260518190932.42270-1-maksym.shcherba@lnu.edu.ua>
The new update_schemes_quota_goals sysfs command allows users to manually
update the current_value of quota goals.
Add a selftest for the command. The test writes a dummy value to
current_value, executes the update command, and verifies that the dummy
value is successfully overwritten by the kernel.
Assisted-by: Antigravity:Gemini-3.1-Pro
Signed-off-by: Maksym Shcherba <maksym.shcherba@lnu.edu.ua>
---
tools/testing/selftests/damon/Makefile | 1 +
.../damon/sysfs_update_schemes_quota_goals.py | 86 +++++++++++++++++++
2 files changed, 87 insertions(+)
create mode 100755 tools/testing/selftests/damon/sysfs_update_schemes_quota_goals.py
diff --git a/tools/testing/selftests/damon/Makefile b/tools/testing/selftests/damon/Makefile
index 2180c328a825..a692ebaa6c8a 100644
--- a/tools/testing/selftests/damon/Makefile
+++ b/tools/testing/selftests/damon/Makefile
@@ -13,6 +13,7 @@ TEST_PROGS += sysfs.py
TEST_PROGS += sysfs_update_schemes_tried_regions_wss_estimation.py
TEST_PROGS += damos_quota.py damos_quota_goal.py damos_apply_interval.py
TEST_PROGS += damos_tried_regions.py damon_nr_regions.py
+TEST_PROGS += sysfs_update_schemes_quota_goals.py
TEST_PROGS += reclaim.sh lru_sort.sh
# regression tests (reproducers of previously found bugs)
diff --git a/tools/testing/selftests/damon/sysfs_update_schemes_quota_goals.py b/tools/testing/selftests/damon/sysfs_update_schemes_quota_goals.py
new file mode 100755
index 000000000000..745b97f75bc2
--- /dev/null
+++ b/tools/testing/selftests/damon/sysfs_update_schemes_quota_goals.py
@@ -0,0 +1,86 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+"""
+Test the update_schemes_quota_goals sysfs command.
+
+Start DAMON with a scheme that has a some_mem_psi_us quota goal. Write a
+physically impossible dummy value to the goal's current_value sysfs file.
+Wait for a while, ensure the dummy value is not overwritten asynchronously,
+then write 'update_schemes_quota_goals' to the state file and verify that
+the dummy value is overwritten by the kernel.
+"""
+
+import os
+import time
+
+import _damon_sysfs
+
+
+def main():
+ goal = _damon_sysfs.DamosQuotaGoal(
+ metric=_damon_sysfs.qgoal_metric_some_mem_psi_us,
+ target_value=1000)
+ kdamonds = _damon_sysfs.Kdamonds([_damon_sysfs.Kdamond(
+ contexts=[_damon_sysfs.DamonCtx(
+ ops='paddr',
+ schemes=[_damon_sysfs.Damos(
+ action='stat',
+ quota=_damon_sysfs.DamosQuota(
+ goals=[goal], reset_interval_ms=100),
+ )] # schemes
+ )] # contexts
+ )]) # kdamonds
+
+ err = kdamonds.start()
+ if err is not None:
+ print('kdamond start failed: %s' % err)
+ exit(1)
+
+ # Write a dummy value to current_value to ensure the command actually
+ # overwrites it. We use 2x the quota reset interval in microseconds,
+ # which is a physically impossible value for the kernel to measure.
+ impossible_value = goal.quota.reset_interval_ms * 2000
+ err = _damon_sysfs.write_file(
+ os.path.join(goal.sysfs_dir(), 'current_value'),
+ '%d' % impossible_value)
+ if err is not None:
+ kdamonds.stop()
+ print('Writing dummy current_value failed: %s' % err)
+ exit(1)
+
+ # wait a couple of aggregation intervals so that the kernel has a chance
+ # to compute the first current_value measurement
+ time.sleep(0.5)
+
+ content, err = _damon_sysfs.read_file(
+ os.path.join(goal.sysfs_dir(), 'current_value'))
+ if err is not None:
+ kdamonds.stop()
+ print('Reading current_value before update failed: %s' % err)
+ exit(1)
+ if int(content) != impossible_value:
+ kdamonds.stop()
+ print('current_value changed before update (%s)' % content)
+ exit(1)
+
+ err = kdamonds.kdamonds[0].update_schemes_quota_goals()
+ if err is not None:
+ kdamonds.stop()
+ print('update_schemes_quota_goals failed: %s' % err)
+ exit(1)
+
+ # current_value must be updated and different from our dummy value
+ if goal.current_value is None or goal.current_value == impossible_value:
+ kdamonds.stop()
+ print('update_schemes_quota_goals failed to update current_value')
+ exit(1)
+
+ print('current_value after update_schemes_quota_goals: %d' %
+ goal.current_value)
+
+ kdamonds.stop()
+
+
+if __name__ == '__main__':
+ main()
--
2.43.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