* Re: [PATCH RESEND 2] Documentation: hwmon: fix typo in heading for max31730
From: Guenter Roeck @ 2026-05-16 22:49 UTC (permalink / raw)
To: Hassan Maazu
Cc: skhan@linuxfoundation.org, corbet@lwn.net,
linux-doc@vger.kernel.org, linux-hwmon@vger.kernel.org,
linux-kernel@vger.kernel.org, Randy Dunlap
In-Reply-To: <6-vpIVJnccYzmznZMj4zfXmOKnHhtaXdeyJqyqTm3KJwLIEj3iSiWzBVxHnBhkNZHZ8E3KfHn7pYQSt3xrfQOQeN5RCJNnBVwmgyJcaw_zM=@proton.me>
On Sat, May 16, 2026 at 10:09:26PM +0000, Hassan Maazu wrote:
> Generated heading & link to driver doc for max31730 wrongly named
> max31790 under hwmon docs. This patch fixes typo so link to max31730
> is easily identifiable without confusion with max31790.
>
> Signed-off-by: Hassan Maazu <maazudev@proton.me>
> Acked-by: Randy Dunlap <rdunlap@infradead.org>
Applied.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH v2] dcache: add fs.dentry-limit sysctl with negative-first reaper
From: Matthew Wilcox @ 2026-05-16 23:09 UTC (permalink / raw)
To: Horst Birthelmer
Cc: Miklos Szeredi, Jonathan Corbet, Shuah Khan, Alexander Viro,
Christian Brauner, Jan Kara, linux-doc, linux-kernel,
linux-fsdevel, Horst Birthelmer
In-Reply-To: <20260516-limit-dentries-cache-v2-1-c733a78e603b@ddn.com>
On Sat, May 16, 2026 at 04:52:54PM +0200, Horst Birthelmer wrote:
> There was a discussion at LSFMM about servers with too many cached
> negative dentries.
> That gave me the idea to keep the dentries in general limited
> if the system administrator needs it to.
I feel you should link to the dozens of previous attempts at this kind
of thing to show that you're aware that this has been tried before and
you're doing something meaningfully different.
^ permalink raw reply
* Re: [PATCH v2 1/2] soc: bcm2835: raspberrypi-firmware: Add voltage domain IDs
From: Guenter Roeck @ 2026-05-16 23:09 UTC (permalink / raw)
To: Shubham Chakraborty, Florian Fainelli, Jonathan Corbet
Cc: 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: <20260516191555.17978-2-chakrabortyshubham66@gmail.com>
On 5/16/26 12:15, Shubham Chakraborty wrote:
> Add firmware voltage domain identifiers for the Raspberry Pi
> mailbox property interface.
>
> These IDs are used by firmware clients to query voltage rails
> through the RPI_FIRMWARE_GET_VOLTAGE property.
>
> Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>
> ---
> include/soc/bcm2835/raspberrypi-firmware.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
> index e1f87fbfe554..fd2e051ce05b 100644
> --- a/include/soc/bcm2835/raspberrypi-firmware.h
> +++ b/include/soc/bcm2835/raspberrypi-firmware.h
> @@ -156,6 +156,14 @@ enum rpi_firmware_clk_id {
> RPI_FIRMWARE_NUM_CLK_ID,
> };
>
> +enum rpi_firmware_volt_id {
> + RPI_FIRMWARE_VOLT_ID_RESERVED = 0,
Is that needed ?
> + RPI_FIRMWARE_VOLT_ID_CORE = 1,
> + RPI_FIRMWARE_VOLT_ID_SDRAM_C = 2,
> + RPI_FIRMWARE_VOLT_ID_SDRAM_I = 3,
> + RPI_FIRMWARE_VOLT_ID_SDRAM_P = 4,
Regarding Sashiko's feedback: I don't know where it got the
information from, but a web search suggests that it has a point;
RPI_FIRMWARE_VOLT_ID_SDRAM_I and RPI_FIRMWARE_VOLT_ID_SDRAM_P appear
to be swapped. If that is not the case, please provide evidence.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH] docs: fix typo in mpo-overview.rst
From: Cheesecake @ 2026-05-16 23:13 UTC (permalink / raw)
To: Jonathan Corbet, Alex Deucher, Christian König,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Shuah Khan
Cc: amd-gfx, dri-devel, linux-doc, linux-kernel
In-Reply-To: <87se7rmont.fsf@trenco.lwn.net>
On 2026/05/16 23:09, Jonathan Corbet wrote:
> Cheesecake <cheesecake2960@icloud.com> writes:
>
>> Replace "transparant" with "transparent"
>>
>> Signed-off-by: Cheesecake <cheesecake2960@icloud.com>
> Patches need a proper signoff with a real name, please.
>
> Thanks,
>
> jo
Thanks for pointing that out.
I accidentally sent this patch using an anonymous address/name.
I'll resend it properly later.
Thank you for your time.
^ permalink raw reply
* Re: [PATCH] docs: fix typos in design.rst
From: Cheesecake @ 2026-05-16 23:14 UTC (permalink / raw)
To: SeongJae Park
Cc: Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Jonathan Corbet, Shuah Khan,
damon, linux-mm, linux-doc, linux-kernel
In-Reply-To: <20260516170847.146524-1-sj@kernel.org>
On 2026/05/17 2:08, SeongJae Park wrote:
> Hello Cheesecake,
>
> Thank you for this patch!
>
> For the consistency, let's use 'Docs/mm/damon/design:' as the prefix of the
> subject. E.g., Docs/mm/damon/design: fix three typos
>
> On Sat, 16 May 2026 18:35:37 +0900 Cheesecake <cheesecake2960@icloud.com> wrote:
>
>> L140: "unsinged" -> "unsigned"
>> L371: "sampleing" -> "sampling"
>> L387: "multipled" -> "multiplied"
> Thank you for finding and fixing these!
>
>> Signed-off-by: Cheesecake <cheesecake2960@icloud.com>
> Is Cheesecake your real name or known identity? We don't allow anonymous
> contributions [1], and mm community prefer to use real names.
>
> [...]
>
> The file changes look good.
>
> Could you please send v2 of this patch with changed subject and the name (if
> Cheesecake is not your real name or known identity)?
>
> [1] https://docs.kernel.org/process/submitting-patches.html#developer-s-certificate-of-origin-1-1
>
>
> Thanks,
> SJ
Thanks for pointing that out.
I accidentally sent this patch using an anonymous address/name.
I'll resend it properly later.
Thank you for your time.
^ permalink raw reply
* Re: [PATCH v2 2/2] hwmon: raspberrypi: Add voltage input support
From: Guenter Roeck @ 2026-05-16 23:20 UTC (permalink / raw)
To: Shubham Chakraborty, Florian Fainelli, Jonathan Corbet
Cc: 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: <20260516191555.17978-3-chakrabortyshubham66@gmail.com>
On 5/16/26 12:15, Shubham Chakraborty wrote:
> Extend the raspberrypi-hwmon driver to expose firmware-provided
> voltage measurements through the hwmon subsystem.
>
> The driver now exports the following voltage inputs:
>
> - in0_input (core)
> - in1_input (sdram_c)
> - in2_input (sdram_i)
> - in3_input (sdram_p)
>
> Voltage values returned by firmware are converted from microvolts
> to millivolts as expected by the hwmon subsystem.
>
> Update the documentation related to it.
>
> The existing undervoltage sticky alarm handling is preserved and
> associated with the first voltage channel.
>
> Tested in -
> - Raspberry Pi 3b+ (Linux raspberrypi 6.12.75+rpt-rpi-v8 #1 SMP PREEMPT
> Debian 1:6.12.75-1+rpt1 (2026-03-11) aarch64 GNU/Linux)
>
> Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>
> ---
> Documentation/hwmon/raspberrypi-hwmon.rst | 15 ++-
> drivers/hwmon/raspberrypi-hwmon.c | 134 +++++++++++++++++++++-
> 2 files changed, 144 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/hwmon/raspberrypi-hwmon.rst b/Documentation/hwmon/raspberrypi-hwmon.rst
> index 8038ade36490..db315184b861 100644
> --- a/Documentation/hwmon/raspberrypi-hwmon.rst
> +++ b/Documentation/hwmon/raspberrypi-hwmon.rst
> @@ -20,6 +20,17 @@ undervoltage conditions.
> Sysfs entries
> -------------
>
> -======================= ==================
> +======================= ======================================================
> +in0_input Core voltage in millivolts
> +in1_input SDRAM controller voltage in millivolts
> +in2_input SDRAM I/O voltage in millivolts
> +in3_input SDRAM PHY voltage in millivolts
> +in0_label "core"
> +in1_label "sdram_c"
> +in2_label "sdram_i"
> +in3_label "sdram_p"
> in0_lcrit_alarm Undervoltage alarm
> -======================= ==================
> +======================= ======================================================
> +
> +The voltage inputs and labels are only exposed if the firmware reports support
> +for the corresponding voltage ID.
> diff --git a/drivers/hwmon/raspberrypi-hwmon.c b/drivers/hwmon/raspberrypi-hwmon.c
> index a2938881ccd2..4f96f37116f3 100644
> --- a/drivers/hwmon/raspberrypi-hwmon.c
> +++ b/drivers/hwmon/raspberrypi-hwmon.c
> @@ -5,6 +5,7 @@
> * Based on firmware/raspberrypi.c by Noralf Trønnes
> *
> * Copyright (C) 2018 Stefan Wahren <stefan.wahren@i2se.com>
> + * Copyright (C) 2026 Shubham Chakraborty <chakrabortyshubham66@gmail.com>
> */
> #include <linux/device.h>
> #include <linux/devm-helpers.h>
> @@ -18,13 +19,26 @@
>
> #define UNDERVOLTAGE_STICKY_BIT BIT(16)
>
> +struct rpi_firmware_get_value {
> + __le32 id;
> + __le32 val;
> +} __packed;
My earlier comment is still valid: This should be defined in
the include file, and it should be query-specific, just like
struct rpi_firmware_clk_rate_request.
> +
> struct rpi_hwmon_data {
> struct device *hwmon_dev;
> struct rpi_firmware *fw;
> + u32 valid_inputs;
> u32 last_throttled;
> struct delayed_work get_values_poll_work;
> };
>
> +static const char * const rpi_hwmon_labels[] = {
> + "core",
> + "sdram_c",
> + "sdram_i",
> + "sdram_p",
> +};
> +
> static void rpi_firmware_get_throttled(struct rpi_hwmon_data *data)
> {
> u32 new_uv, old_uv, value;
> @@ -56,6 +70,23 @@ static void rpi_firmware_get_throttled(struct rpi_hwmon_data *data)
> hwmon_notify_event(data->hwmon_dev, hwmon_in, hwmon_in_lcrit_alarm, 0);
> }
>
> +static int rpi_firmware_get_voltage(struct rpi_hwmon_data *data, u32 id,
> + long *val)
> +{
> + struct rpi_firmware_get_value packet;
> + int ret;
> +
> + packet.id = cpu_to_le32(id);
> + packet.val = 0;
> + ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_VOLTAGE,
> + &packet, sizeof(packet));
> + if (ret)
> + return ret;
> +
> + *val = le32_to_cpu(packet.val) / 1000;
I would suggest to use DIV_ROUND_CLOSEST().
> + return 0;
> +}
> +
> static void get_values_poll(struct work_struct *work)
> {
> struct rpi_hwmon_data *data;
> @@ -77,19 +108,94 @@ static int rpi_read(struct device *dev, enum hwmon_sensor_types type,
> {
> struct rpi_hwmon_data *data = dev_get_drvdata(dev);
>
> - *val = !!(data->last_throttled & UNDERVOLTAGE_STICKY_BIT);
> + if (type == hwmon_in) {
> + switch (attr) {
> + case hwmon_in_input:
> + switch (channel) {
> + case 0:
> + return rpi_firmware_get_voltage(data,
> + RPI_FIRMWARE_VOLT_ID_CORE,
> + val);
> + case 1:
> + return rpi_firmware_get_voltage(data,
> + RPI_FIRMWARE_VOLT_ID_SDRAM_C,
> + val);
> + case 2:
> + return rpi_firmware_get_voltage(data,
> + RPI_FIRMWARE_VOLT_ID_SDRAM_I,
> + val);
> + case 3:
> + return rpi_firmware_get_voltage(data,
> + RPI_FIRMWARE_VOLT_ID_SDRAM_P,
> + val);
> + default:
> + return -EOPNOTSUPP;
With
static const int voltage_regs[] = {
RPI_FIRMWARE_VOLT_ID_CORE, RPI_FIRMWARE_VOLT_ID_SDRAM_C, RPI_FIRMWARE_VOLT_ID_SDRAM_I,
RPI_FIRMWARE_VOLT_ID_SDRAM_P };
this can be simplified to
return rpi_firmware_get_voltage(data, voltage_regs[channel];
> + }
> + case hwmon_in_lcrit_alarm:
> + if (channel == 0) {
> + *val = !!(data->last_throttled & UNDERVOLTAGE_STICKY_BIT);
> + return 0;
> + }
The channel check is not really necessary.
> + return -EOPNOTSUPP;
> + default:
> + return -EOPNOTSUPP;
> + }
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static int rpi_read_string(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, const char **str)
> +{
> + if (type == hwmon_in && attr == hwmon_in_label) {
> + if (channel >= ARRAY_SIZE(rpi_hwmon_labels))
> + return -EOPNOTSUPP;
Unnecessary check.
> +
> + *str = rpi_hwmon_labels[channel];
> + return 0;
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static umode_t rpi_is_visible(const void *_data, enum hwmon_sensor_types type,
> + u32 attr, int channel)
> +{
> + const struct rpi_hwmon_data *data = _data;
> +
> + if (type == hwmon_in) {
> + switch (attr) {
> + case hwmon_in_input:
> + case hwmon_in_label:
> + if (!(data->valid_inputs & BIT(channel)))
> + return 0;
> + return 0444;
> + case hwmon_in_lcrit_alarm:
> + if (channel == 0)
> + return 0444;
> + return 0;
> + default:
> + return 0;
> + }
> + }
> +
> return 0;
> }
>
> static const struct hwmon_channel_info * const rpi_info[] = {
> HWMON_CHANNEL_INFO(in,
> - HWMON_I_LCRIT_ALARM),
> + HWMON_I_INPUT | HWMON_I_LABEL | HWMON_I_LCRIT_ALARM,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL),
> NULL
> };
>
> static const struct hwmon_ops rpi_hwmon_ops = {
> - .visible = 0444,
> + .is_visible = rpi_is_visible,
> .read = rpi_read,
> + .read_string = rpi_read_string,
> };
>
> static const struct hwmon_chip_info rpi_chip_info = {
> @@ -101,6 +207,7 @@ static int rpi_hwmon_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> struct rpi_hwmon_data *data;
> + long voltage;
> int ret;
>
> data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> @@ -110,6 +217,26 @@ static int rpi_hwmon_probe(struct platform_device *pdev)
> /* Parent driver assure that firmware is correct */
> data->fw = dev_get_drvdata(dev->parent);
>
> + ret = rpi_firmware_get_voltage(data, RPI_FIRMWARE_VOLT_ID_CORE,
> + &voltage);
> + if (!ret)
> + data->valid_inputs |= BIT(0);
> +
> + ret = rpi_firmware_get_voltage(data, RPI_FIRMWARE_VOLT_ID_SDRAM_C,
> + &voltage);
> + if (!ret)
> + data->valid_inputs |= BIT(1);
> +
> + ret = rpi_firmware_get_voltage(data, RPI_FIRMWARE_VOLT_ID_SDRAM_I,
> + &voltage);
> + if (!ret)
> + data->valid_inputs |= BIT(2);
> +
> + ret = rpi_firmware_get_voltage(data, RPI_FIRMWARE_VOLT_ID_SDRAM_P,
> + &voltage);
> + if (!ret)
> + data->valid_inputs |= BIT(3);
> +
This can be implemented in a loop, using the above voltage_regs array.
Thanks,
Guenter
> data->hwmon_dev = devm_hwmon_device_register_with_info(dev, "rpi_volt",
> data,
> &rpi_chip_info,
> @@ -159,6 +286,7 @@ static struct platform_driver rpi_hwmon_driver = {
> module_platform_driver(rpi_hwmon_driver);
>
> MODULE_AUTHOR("Stefan Wahren <wahrenst@gmx.net>");
> +MODULE_AUTHOR("Shubham Chakraborty <chakrabortyshubham66@gmail.com>");
> MODULE_DESCRIPTION("Raspberry Pi voltage sensor driver");
> MODULE_LICENSE("GPL v2");
> MODULE_ALIAS("platform:raspberrypi-hwmon");
^ permalink raw reply
* [PATCH v2] docs: fix typos in design.rst
From: Sakurai Shun @ 2026-05-17 1:02 UTC (permalink / raw)
To: SeongJae Park, Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Jonathan Corbet, Shuah Khan
Cc: Sakurai Shun, damon, linux-mm, linux-doc, linux-kernel
In-Reply-To: <20260516170847.146524-1-sj@kernel.org>
L140: "unsinged" -> "unsigned"
L371: "sampleing" -> "sampling"
L387: "multipled" -> "multiplied"
Signed-off-by: Sakurai Shun <ssh1326@icloud.com>
---
Documentation/mm/damon/design.rst | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/mm/damon/design.rst b/Documentation/mm/damon/design.rst
index afc7d52bd..9cc70a296 100644
--- a/Documentation/mm/damon/design.rst
+++ b/Documentation/mm/damon/design.rst
@@ -140,7 +140,7 @@ as Idle page tracking does.
Address Unit
------------
-DAMON core layer uses ``unsinged long`` type for monitoring target address
+DAMON core layer uses ``unsigned long`` type for monitoring target address
ranges. In some cases, the address space for a given operations set could be
too large to be handled with the type. ARM (32-bit) with large physical
address extension is an example. For such cases, a per-operations set
@@ -371,7 +371,7 @@ with theoretical maximum ``nr_accesses``, which can be calculated as
``aggregation interval / sampling interval``.
The mechanism calculates the ratio of access events for ``aggrs`` aggregations,
-and increases or decrease the ``sampleing interval`` and ``aggregation
+and increases or decrease the ``sampling interval`` and ``aggregation
interval`` in same ratio, if the observed access ratio is lower or higher than
the target, respectively. The ratio of the intervals change is decided in
proportion to the distance between current samples ratio and the target ratio.
@@ -387,7 +387,7 @@ The tuning is turned off by default, and need to be set explicitly by the user.
As a rule of thumbs and the Parreto principle, 4% access samples ratio target
is recommended. Note that Parreto principle (80/20 rule) has applied twice.
That is, assumes 4% (20% of 20%) DAMON-observed access events ratio (source)
-to capture 64% (80% multipled by 80%) real access events (outcomes).
+to capture 64% (80% multiplied by 80%) real access events (outcomes).
To know how user-space can use this feature via :ref:`DAMON sysfs interface
<sysfs_interface>`, refer to :ref:`intervals_goal
--
2.54.0
^ permalink raw reply related
* [PATCH v5 0/4] Add MSI Claw HID Configuration Driver
From: Derek J. Clark @ 2026-05-17 1:39 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>
---
v5:
- 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 | 12 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 6 +
drivers/hid/hid-msi.c | 1728 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 1753 insertions(+)
create mode 100644 drivers/hid/hid-msi.c
--
2.53.0
^ permalink raw reply
* [PATCH v5 1/4] HID: hid-msi: Add MSI Claw configuration driver
From: Derek J. Clark @ 2026-05-17 1:39 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: <20260517013925.3120314-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>
---
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 | 12 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 5 +
drivers/hid/hid-msi.c | 630 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 654 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..af146691bd481 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -492,6 +492,18 @@ 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 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..3f809dc70a4cc
--- /dev/null
+++ b/drivers/hid/hid-msi.c
@@ -0,0 +1,630 @@
+// 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/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 mode_mutex; /* mutex for mode calls */
+ struct mutex cfg_mutex; /* mutex for synchronous data */
+ u8 ep;
+
+ /* Gamepad Variables */
+ enum claw_mkeys_function_index mkeys_function;
+ enum claw_gamepad_mode_index gamepad_mode;
+ bool gamepad_registered;
+};
+
+static int get_endpoint_address(struct hid_device *hdev)
+{
+ struct usb_host_endpoint *ep;
+ struct usb_interface *intf;
+
+ intf = to_usb_interface(hdev->dev.parent);
+ ep = intf->cur_altsetting->endpoint;
+ if (ep)
+ return ep->desc.bEndpointAddress;
+
+ return -ENODEV;
+}
+
+static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
+ struct claw_command_report *cmd_rep)
+{
+ if (cmd_rep->data[0] >= ARRAY_SIZE(claw_gamepad_mode_text) ||
+ !claw_gamepad_mode_text[cmd_rep->data[0]] ||
+ cmd_rep->data[1] >= ARRAY_SIZE(claw_mkeys_function_text))
+ return -EINVAL;
+
+ 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);
+ break;
+ case CLAW_COMMAND_TYPE_ACK:
+ break;
+ default:
+ dev_dbg(&drvdata->hdev->dev, "Unknown command: %x\n", cmd_rep->cmd);
+ return 0;
+ }
+
+ complete(&drvdata->send_cmd_complete);
+
+ return ret;
+}
+
+static int msi_raw_event(struct hid_device *hdev, struct hid_report *report,
+ u8 *data, int size)
+{
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata || (drvdata->ep != CLAW_XINPUT_CFG_INTF_IN &&
+ drvdata->ep != CLAW_DINPUT_CFG_INTF_IN))
+ return 0;
+
+ return claw_raw_event(drvdata, report, data, size);
+}
+
+static int claw_hw_output_report(struct hid_device *hdev, u8 index, u8 *data,
+ size_t len, unsigned int timeout)
+{
+ unsigned char *dmabuf __free(kfree) = NULL;
+ u8 header[] = { CLAW_OUTPUT_REPORT_ID, 0, 0, 0x3c, index };
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ size_t header_size = ARRAY_SIZE(header);
+ int ret;
+
+ if (header_size + len > CLAW_PACKET_SIZE)
+ return -EINVAL;
+
+ /* We can't use a devm_alloc reusable buffer without side effects during suspend */
+ dmabuf = kzalloc(CLAW_PACKET_SIZE, GFP_KERNEL);
+ if (!dmabuf)
+ return -ENOMEM;
+
+ memcpy(dmabuf, header, header_size);
+ if (data && len)
+ memcpy(dmabuf + header_size, data, len);
+
+ guard(mutex)(&drvdata->cfg_mutex);
+ if (timeout)
+ reinit_completion(&drvdata->send_cmd_complete);
+
+ 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);
+ if (ret >= 0) /* preserve errors */
+ ret = ret == 0 ? -EBUSY : 0; /* timeout occurred : time remained */
+ }
+
+ 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];
+
+ 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;
+
+ guard(mutex)(&drvdata->mode_mutex);
+ data[0] = ret;
+ 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;
+
+ guard(mutex)(&drvdata->mode_mutex);
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE, NULL, 0, 8);
+ if (ret)
+ return ret;
+
+ 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]);
+ }
+
+ 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];
+
+ 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;
+
+ guard(mutex)(&drvdata->mode_mutex);
+ 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;
+
+ guard(mutex)(&drvdata->mode_mutex);
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE, NULL, 0, 8);
+ if (ret)
+ return ret;
+
+ 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]);
+
+ 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);
+ bool val;
+ int ret;
+
+ 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, 8);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to setup device, can't read gamepad mode: %d\n", ret);
+ return;
+ }
+
+ /* Add sysfs attributes after we get the device state */
+ ret = device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to setup device, can't create gamepad attrs: %d\n", ret);
+ return;
+ }
+ 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);
+ u8 data[2] = { drvdata->gamepad_mode, drvdata->mkeys_function };
+ int ret;
+
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data,
+ ARRAY_SIZE(data), 0);
+ if (ret)
+ dev_err(&drvdata->hdev->dev, "Failed to set gamepad mode settings: %d\n", ret);
+}
+
+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->mode_mutex);
+ mutex_init(&drvdata->cfg_mutex);
+ 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);
+
+ guard(mutex)(&drvdata->cfg_mutex);
+ if (drvdata->gamepad_registered)
+ device_remove_group(&hdev->dev, &claw_gamepad_attr_group);
+
+ hid_hw_close(hdev);
+}
+
+static void msi_remove(struct hid_device *hdev)
+{
+ int ret;
+ u8 ep;
+
+ 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 v5 2/4] HID: hid-msi: Add M-key mapping attributes
From: Derek J. Clark @ 2026-05-17 1:39 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: <20260517013925.3120314-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>
---
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 | 401 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 400 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 3f809dc70a4cc..991d5a25d3de0 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -41,6 +41,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,
@@ -65,6 +67,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,
@@ -77,6 +90,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_LEFT_RIGHT"},
+ { 0x19, "ABS_RY_UP"},
+ { 0x1a, "ABS_RY_DOWN"},
+ { 0x1b, "ABS_RX_LEFT"},
+ { 0x1c, "ABS_RX_RIGHT"},
+ /* Keyboard keys */
+ { 0x32, "KEY_ESC" },
+ { 0x33, "KEY_F1" },
+ { 0x34, "KEY_F2" },
+ { 0x35, "KEY_F3" },
+ { 0x36, "KEY_F4" },
+ { 0x37, "KEY_F5" },
+ { 0x38, "KEY_F6" },
+ { 0x39, "KEY_F7" },
+ { 0x3a, "KEY_F8" },
+ { 0x3b, "KEY_F9" },
+ { 0x3c, "KEY_F10" },
+ { 0x3d, "KEY_F11" },
+ { 0x3e, "KEY_F12" },
+ { 0x3f, "KEY_GRAVE" },
+ { 0x40, "KEY_1" },
+ { 0x41, "KEY_2" },
+ { 0x42, "KEY_3" },
+ { 0x43, "KEY_4" },
+ { 0x44, "KEY_5" },
+ { 0x45, "KEY_6" },
+ { 0x46, "KEY_7" },
+ { 0x47, "KEY_8" },
+ { 0x48, "KEY_9" },
+ { 0x49, "KEY_0" },
+ { 0x4a, "KEY_MINUS" },
+ { 0x4b, "KEY_EQUAL" },
+ { 0x4c, "KEY_BACKSPACE" },
+ { 0x4d, "KEY_TAB" },
+ { 0x4e, "KEY_Q" },
+ { 0x4f, "KEY_W" },
+ { 0x50, "KEY_E" },
+ { 0x51, "KEY_R" },
+ { 0x52, "KEY_T" },
+ { 0x53, "KEY_Y" },
+ { 0x54, "KEY_U" },
+ { 0x55, "KEY_I" },
+ { 0x56, "KEY_O" },
+ { 0x57, "KEY_P" },
+ { 0x58, "KEY_LEFTBRACE" },
+ { 0x59, "KEY_RIGHTBRACE" },
+ { 0x5a, "KEY_BACKSLASH" },
+ { 0x5b, "KEY_CAPSLOCK" },
+ { 0x5c, "KEY_A" },
+ { 0x5d, "KEY_S" },
+ { 0x5e, "KEY_D" },
+ { 0x5f, "KEY_F" },
+ { 0x60, "KEY_G" },
+ { 0x61, "KEY_H" },
+ { 0x62, "KEY_J" },
+ { 0x63, "KEY_K" },
+ { 0x64, "KEY_L" },
+ { 0x65, "KEY_SEMICOLON" },
+ { 0x66, "KEY_APOSTROPHE" },
+ { 0x67, "KEY_ENTER" },
+ { 0x68, "KEY_LEFTSHIFT" },
+ { 0x69, "KEY_Z" },
+ { 0x6a, "KEY_X" },
+ { 0x6b, "KEY_C" },
+ { 0x6c, "KEY_V" },
+ { 0x6d, "KEY_B" },
+ { 0x6e, "KEY_N" },
+ { 0x6f, "KEY_M" },
+ { 0x70, "KEY_COMMA" },
+ { 0x71, "KEY_DOT" },
+ { 0x72, "KEY_SLASH" },
+ { 0x73, "KEY_RIGHTSHIFT" },
+ { 0x74, "KEY_LEFTCTRL" },
+ { 0x75, "KEY_LEFTMETA" },
+ { 0x76, "KEY_LEFTALT" },
+ { 0x77, "KEY_SPACE" },
+ { 0x78, "KEY_RIGHTALT" },
+ { 0x79, "KEY_RIGHTCTRL" },
+ { 0x7a, "KEY_INSERT" },
+ { 0x7b, "KEY_HOME" },
+ { 0x7c, "KEY_PAGEUP" },
+ { 0x7d, "KEY_DELETE" },
+ { 0x7e, "KEY_END" },
+ { 0x7f, "KEY_PAGEDOWN" },
+ { 0x8a, "KEY_KPENTER" },
+ { 0x8b, "KEY_KP0" },
+ { 0x8c, "KEY_KP1" },
+ { 0x8d, "KEY_KP2" },
+ { 0x8e, "KEY_KP3" },
+ { 0x8f, "KEY_KP4" },
+ { 0x90, "KEY_KP5" },
+ { 0x91, "KEY_KP6" },
+ { 0x92, "KEY_KP7" },
+ { 0x93, "KEY_KP8" },
+ { 0x94, "KEY_KP9" },
+ { 0x95, "MD_PLAY" },
+ { 0x96, "MD_STOP" },
+ { 0x97, "MD_NEXT" },
+ { 0x98, "MD_PREV" },
+ { 0x99, "MD_VOL_UP" },
+ { 0x9a, "MD_VOL_DOWN" },
+ { 0x9b, "MD_VOL_MUTE" },
+ { 0x9c, "KEY_F23" },
+ /* Mouse events */
+ { 0xc8, "BTN_LEFT" },
+ { 0xc9, "BTN_MIDDLE" },
+ { 0xca, "BTN_RIGHT" },
+ { 0xcb, "BTN_SIDE" },
+ { 0xcc, "BTN_EXTRA" },
+ { 0xcd, "REL_WHEEL_UP" },
+ { 0xce, "REL_WHEEL_DOWN" },
+ { 0xff, "DISABLED" },
+};
+
+static const u16 button_mapping_addr_old[] = {
+ 0x007a, /* M1 */
+ 0x011f, /* M2 */
+};
+
+static const u16 button_mapping_addr_new[] = {
+ 0x00bb, /* M1 */
+ 0x0164, /* M2 */
+};
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -87,18 +249,26 @@ 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 mode_mutex; /* mutex for mode calls */
struct mutex cfg_mutex; /* mutex for synchronous data */
+ struct mutex rom_mutex; /* mutex for SYNC_TO_ROM calls */
+ 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;
+ const u16 *bmap_addr;
+ bool bmap_support;
};
static int get_endpoint_address(struct hid_device *hdev)
@@ -128,6 +298,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)
{
@@ -149,6 +343,9 @@ static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *repor
case CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK:
ret = claw_gamepad_mode_event(drvdata, cmd_rep);
break;
+ case CLAW_COMMAND_TYPE_READ_PROFILE_ACK:
+ ret = claw_profile_event(drvdata, cmd_rep);
+ break;
case CLAW_COMMAND_TYPE_ACK:
break;
default:
@@ -373,6 +570,164 @@ 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;
+}
+
+DEFINE_FREE(argv, char **, if (_T) argv_free(_T))
+
+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) = NULL;
+ size_t len = ARRAY_SIZE(data);
+ int ret, key_count, i;
+
+ 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, 8);
+ if (ret)
+ return ret;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 8);
+ }
+
+ 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;
+
+ 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, 8);
+ 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)
{
@@ -385,10 +740,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,
@@ -440,8 +807,31 @@ static void cfg_resume_fn(struct work_struct *work)
dev_err(&drvdata->hdev->dev, "Failed to set gamepad mode settings: %d\n", ret);
}
+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;
@@ -453,8 +843,17 @@ 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->mode_mutex);
mutex_init(&drvdata->cfg_mutex);
+ mutex_init(&drvdata->profile_mutex);
+ mutex_init(&drvdata->rom_mutex);
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 v5 3/4] HID: hid-msi: Add RGB control interface
From: Derek J. Clark @ 2026-05-17 1:39 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: <20260517013925.3120314-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>
---
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 | 573 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 568 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 991d5a25d3de0..4443f53b54cb1 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -21,10 +21,13 @@
#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>
#include <linux/pm.h>
+#include <linux/spinlock.h>
+#include <linux/spinlock_types.h>
#include <linux/sysfs.h>
#include <linux/types.h>
#include <linux/unaligned.h>
@@ -43,6 +46,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,
@@ -71,6 +78,7 @@ enum claw_profile_ack_pending {
CLAW_NO_PENDING,
CLAW_M1_PENDING,
CLAW_M2_PENDING,
+ CLAW_RGB_PENDING,
};
enum claw_key_index {
@@ -229,6 +237,22 @@ static const struct {
{ 0xff, "DISABLED" },
};
+enum claw_rgb_effect_index {
+ CLAW_RGB_EFFECT_MONOCOLOR,
+ CLAW_RGB_EFFECT_BREATHE,
+ CLAW_RGB_EFFECT_CHROMA,
+ CLAW_RGB_EFFECT_RAINBOW,
+ CLAW_RGB_EFFECT_FROSTFIRE,
+};
+
+static const char * const claw_rgb_effect_text[] = {
+ [CLAW_RGB_EFFECT_MONOCOLOR] = "monocolor",
+ [CLAW_RGB_EFFECT_BREATHE] = "breathe",
+ [CLAW_RGB_EFFECT_CHROMA] = "chroma",
+ [CLAW_RGB_EFFECT_RAINBOW] = "rainbow",
+ [CLAW_RGB_EFFECT_FROSTFIRE] = "frostfire",
+};
+
static const u16 button_mapping_addr_old[] = {
0x007a, /* M1 */
0x011f, /* M2 */
@@ -239,6 +263,9 @@ static const u16 button_mapping_addr_new[] = {
0x0164, /* M2 */
};
+static const u16 rgb_addr_old = 0x01fa;
+static const u16 rgb_addr_new = 0x024a;
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -247,6 +274,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;
@@ -258,6 +307,7 @@ struct claw_drvdata {
struct mutex mode_mutex; /* mutex for mode calls */
struct mutex cfg_mutex; /* mutex for synchronous data */
struct mutex rom_mutex; /* mutex for SYNC_TO_ROM calls */
+ spinlock_t frame_lock; /* lock for read/write rgb_frames */
u16 bcd_device;
u8 ep;
@@ -269,6 +319,17 @@ struct claw_drvdata {
bool gamepad_registered;
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;
+ 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)
@@ -300,8 +361,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:
@@ -311,15 +375,45 @@ 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->led_mc.led_cdev.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",
cmd_rep->cmd);
return -EINVAL;
}
+
drvdata->profile_pending = CLAW_NO_PENDING;
- return 0;
+ return ret;
}
static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
@@ -769,6 +863,420 @@ 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, 8);
+ 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, 8);
+ 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, 8);
+
+ 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;
+
+ 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_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;
+
+ 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);
+
+ 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;
+
+ 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;
+
+ 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);
+
+ 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);
@@ -782,6 +1290,13 @@ static void cfg_setup_fn(struct work_struct *work)
return;
}
+ ret = claw_read_rgb_config(drvdata->hdev);
+ if (ret) {
+ dev_err(drvdata->led_mc.led_cdev.dev,
+ "Failed to setup device, can't read RGB config: %d\n", ret);
+ return;
+ }
+
/* Add sysfs attributes after we get the device state */
ret = device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
if (ret) {
@@ -791,7 +1306,25 @@ static void cfg_setup_fn(struct work_struct *work)
}
drvdata->gamepad_registered = true;
+ /* Add and enable RGB interface once we have the device state */
+ INIT_DELAYED_WORK(&drvdata->rgb_queue, &claw_rgb_queue_fn);
+ 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 = 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)
@@ -801,6 +1334,10 @@ static void cfg_resume_fn(struct work_struct *work)
u8 data[2] = { drvdata->gamepad_mode, drvdata->mkeys_function };
int ret;
+ ret = claw_read_rgb_config(drvdata->hdev);
+ if (ret)
+ dev_err(&drvdata->hdev->dev, "Failed to read RGB config: %d\n", ret);
+
ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data,
ARRAY_SIZE(data), 0);
if (ret)
@@ -814,18 +1351,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)
@@ -840,6 +1383,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;
@@ -850,10 +1394,22 @@ 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->mode_mutex);
mutex_init(&drvdata->cfg_mutex);
mutex_init(&drvdata->profile_mutex);
mutex_init(&drvdata->rom_mutex);
+ 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);
@@ -864,6 +1420,7 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
return ret;
hid_set_drvdata(hdev, drvdata);
+
schedule_delayed_work(&drvdata->cfg_setup, msecs_to_jiffies(500));
return 0;
@@ -918,10 +1475,15 @@ 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);
guard(mutex)(&drvdata->cfg_mutex);
+ if (drvdata->rgb_registered)
+ device_remove_group(drvdata->led_mc.led_cdev.dev, &claw_rgb_attr_group);
if (drvdata->gamepad_registered)
device_remove_group(&hdev->dev, &claw_gamepad_attr_group);
@@ -982,6 +1544,7 @@ static int claw_suspend(struct hid_device *hdev)
cancel_delayed_work_sync(&drvdata->cfg_setup);
cancel_delayed_work_sync(&drvdata->cfg_resume);
+ cancel_delayed_work_sync(&drvdata->rgb_queue);
return 0;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v5 4/4] HID: hid-msi: Add Rumble Intensity Attributes
From: Derek J. Clark @ 2026-05-17 1:39 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: <20260517013925.3120314-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>
---
v5:
- Remove mkey related changes.
v2:
- Use pending_profile and sync to rom mutexes.
---
drivers/hid/hid-msi.c | 149 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 148 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 4443f53b54cb1..d9951bffc8019 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -79,6 +79,8 @@ enum claw_profile_ack_pending {
CLAW_M1_PENDING,
CLAW_M2_PENDING,
CLAW_RGB_PENDING,
+ CLAW_RUMBLE_LEFT_PENDING,
+ CLAW_RUMBLE_RIGHT_PENDING,
};
enum claw_key_index {
@@ -266,6 +268,11 @@ static const u16 button_mapping_addr_new[] = {
static const u16 rgb_addr_old = 0x01fa;
static const u16 rgb_addr_new = 0x024a;
+static const u16 rumble_addr[] = {
+ 0x0022, /* left */
+ 0x0023, /* right */
+};
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -316,8 +323,11 @@ struct claw_drvdata {
enum claw_gamepad_mode_index gamepad_mode;
u8 m1_codes[CLAW_KEYS_MAX];
u8 m2_codes[CLAW_KEYS_MAX];
- bool gamepad_registered;
+ u8 rumble_intensity_right;
+ u8 rumble_intensity_left;
+ bool gamepad_registered;
const u16 *bmap_addr;
+ bool rumble_support;
bool bmap_support;
/* RGB Variables */
@@ -403,6 +413,12 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
drvdata->led_mc.subled_info[2].intensity = frame->zone_data.zone[0].blue;
}
+ break;
+ case CLAW_RUMBLE_LEFT_PENDING:
+ drvdata->rumble_intensity_left = cmd_rep->data[4];
+ break;
+ case CLAW_RUMBLE_RIGHT_PENDING:
+ drvdata->rumble_intensity_right = cmd_rep->data[4];
break;
default:
dev_dbg(&drvdata->hdev->dev,
@@ -822,6 +838,126 @@ 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;
+
+ 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), 8);
+ if (ret)
+ return ret;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 8);
+ if (ret)
+ return ret;
+
+ drvdata->rumble_intensity_left = val;
+
+ 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;
+
+ 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), 8);
+ if (ret) {
+ drvdata->profile_pending = CLAW_NO_PENDING;
+ return ret;
+ }
+
+ return sysfs_emit(buf, "%u\n", drvdata->rumble_intensity_left);
+}
+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;
+
+ 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), 8);
+ if (ret)
+ return ret;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 8);
+ if (ret)
+ return ret;
+
+ drvdata->rumble_intensity_right = val;
+
+ 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;
+
+ 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), 8);
+ if (ret) {
+ drvdata->profile_pending = CLAW_NO_PENDING;
+ return ret;
+ }
+
+ return sysfs_emit(buf, "%u\n", drvdata->rumble_intensity_right);
+}
+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)
{
@@ -842,6 +978,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;
}
@@ -855,6 +997,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,
};
@@ -1353,6 +1498,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;
@@ -1364,6 +1510,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;
}
--
2.53.0
^ permalink raw reply related
* [PATCH] docs: fix typo in user_mode_linux_howto_v2.rst
From: Sakurai Shun @ 2026-05-17 2:24 UTC (permalink / raw)
To: Richard Weinberger, Anton Ivanov, Johannes Berg, Jonathan Corbet,
Shuah Khan
Cc: Sakurai Shun, linux-um, linux-doc, linux-kernel
Replace "privilges" with "privileges"
Signed-off-by: Sakurai Shun <ssh1326@icloud.com>
---
Documentation/virt/uml/user_mode_linux_howto_v2.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/virt/uml/user_mode_linux_howto_v2.rst b/Documentation/virt/uml/user_mode_linux_howto_v2.rst
index c37e8e594..9224bea5e 100644
--- a/Documentation/virt/uml/user_mode_linux_howto_v2.rst
+++ b/Documentation/virt/uml/user_mode_linux_howto_v2.rst
@@ -234,7 +234,7 @@ an ioctl to setup the tun interface and/or use raw sockets where needed.
This can be achieved by granting the user a particular capability instead
of running UML as root. In case of vector transport, a user can add the
capability ``CAP_NET_ADMIN`` or ``CAP_NET_RAW`` to the uml binary.
-Thenceforth, UML can be run with normal user privilges, along with
+Thenceforth, UML can be run with normal user privileges, along with
full networking.
For example::
--
2.54.0
^ permalink raw reply related
* [PATCH v2] docs: fix typo in uniwill-laptop.rst
From: Sakurai Shun @ 2026-05-17 2:41 UTC (permalink / raw)
To: Armin Wolf, Jonathan Corbet, Shuah Khan
Cc: Sakurai Shun, platform-driver-x86, linux-doc, linux-kernel
In-Reply-To: <20260516070650.9454-1-cheesecake2960@icloud.com>
Replace "benifit" with "benefit".
Signed-off-by: Sakurai Shun <ssh1326@icloud.com>
---
Documentation/wmi/devices/uniwill-laptop.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/wmi/devices/uniwill-laptop.rst b/Documentation/wmi/devices/uniwill-laptop.rst
index e246bf293..65583b239 100644
--- a/Documentation/wmi/devices/uniwill-laptop.rst
+++ b/Documentation/wmi/devices/uniwill-laptop.rst
@@ -189,7 +189,7 @@ Indexed IO
Indexed IO with IO ports with a granularity of a single byte can be performed using the ``RIOP``
(read) and ``WIOP`` (write) ACPI control methods. Those ACPI methods are unused because they
-provide no benifit when compared to the native IO port access functions provided by the kernel.
+provide no benefit when compared to the native IO port access functions provided by the kernel.
Special thanks go to github user `pobrn` which developed the
`qc71_laptop <https://github.com/pobrn/qc71_laptop>`_ driver on which this driver is partly based.
--
2.54.0
^ permalink raw reply related
* [PATCH v2] Replace "transparant" with "transparent"
From: Sakurai Shun @ 2026-05-17 2:58 UTC (permalink / raw)
To: Alex Deucher, Christian König, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Jonathan Corbet, Shuah Khan
Cc: Sakurai Shun, amd-gfx, dri-devel, linux-doc, linux-kernel
In-Reply-To: <87se7rmont.fsf@trenco.lwn.net>
Signed-off-by: Sakurai Shun <ssh1326@icloud.com>
---
Documentation/gpu/amdgpu/display/mpo-overview.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/gpu/amdgpu/display/mpo-overview.rst b/Documentation/gpu/amdgpu/display/mpo-overview.rst
index 59a4f54a3..ed39e53ff 100644
--- a/Documentation/gpu/amdgpu/display/mpo-overview.rst
+++ b/Documentation/gpu/amdgpu/display/mpo-overview.rst
@@ -167,7 +167,7 @@ and interactions with operations such as DPMS and S3:
- ``kms_plane_multiple@atomic-pipe-*-tiling-``
- ``kms_plane_scaling@pipe-*-plane-scaling``
- ``kms_plane_alpha_blend@pipe-*-alpha-basic``
-- ``kms_plane_alpha_blend@pipe-*-alpha-transparant-fb``
+- ``kms_plane_alpha_blend@pipe-*-alpha-transparent-fb``
- ``kms_plane_alpha_blend@pipe-*-alpha-opaque-fb``
- ``kms_plane_alpha_blend@pipe-*-constant-alpha-min``
- ``kms_plane_alpha_blend@pipe-*-constant-alpha-mid``
--
2.54.0
^ permalink raw reply related
* [PATCH] docs: fix typo in list.rst
From: Sakurai Shun @ 2026-05-17 4:07 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan; +Cc: Sakurai Shun, linux-doc, linux-kernel
Replace "reinitalizes" with "reinitializes"
Signed-off-by: Sakurai Shun <ssh1326@icloud.com>
---
Documentation/core-api/list.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/core-api/list.rst b/Documentation/core-api/list.rst
index 241464ca0..4819343a2 100644
--- a/Documentation/core-api/list.rst
+++ b/Documentation/core-api/list.rst
@@ -752,7 +752,7 @@ This is because list_splice() did not reinitialize the list_head it took
entries from, leaving its pointer pointing into what is now a different list.
If we want to avoid this situation, list_splice_init() can be used. It does the
-same thing as list_splice(), except reinitalizes the donor list_head after the
+same thing as list_splice(), except reinitializes the donor list_head after the
transplant.
Concurrency considerations
--
2.54.0
^ permalink raw reply related
* [PATCH] docs: fix typo in leds-lp55xx.rst
From: Sakurai Shun @ 2026-05-17 4:32 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Jonathan Corbet, Shuah Khan
Cc: Sakurai Shun, linux-leds, linux-doc, linux-kernel
Replace "regsister" with "register"
Signed-off-by: Sakurai Shun <ssh1326@icloud.com>
---
Documentation/leds/leds-lp55xx.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/leds/leds-lp55xx.rst b/Documentation/leds/leds-lp55xx.rst
index 632e41cec..f60c7ec39 100644
--- a/Documentation/leds/leds-lp55xx.rst
+++ b/Documentation/leds/leds-lp55xx.rst
@@ -18,7 +18,7 @@ The LP55xx common driver provides these features using exported functions.
lp55xx_init_device() / lp55xx_deinit_device()
lp55xx_register_leds() / lp55xx_unregister_leds()
- lp55xx_regsister_sysfs() / lp55xx_unregister_sysfs()
+ lp55xx_register_sysfs() / lp55xx_unregister_sysfs()
( Driver Structure Data )
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] cpufreq: Documentation: fix sampling_down_factor documentation range
From: Zhongqiu Han @ 2026-05-17 5:04 UTC (permalink / raw)
To: Pengjie Zhang, rafael, viresh.kumar, corbet
Cc: skhan, linux-pm, linux-doc, zhanjie9, zhenglifeng1, lihuisong,
yubowen8, linhongye, linuxarm, wangzhi12, zhongqiu.han
In-Reply-To: <20260515094930.273599-1-zhangpengjie2@huawei.com>
On 5/15/2026 5:49 PM, Pengjie Zhang wrote:
> The ondemand governor implementation accepts sampling_down_factor values
> from 1 to 100000 via MAX_SAMPLING_DOWN_FACTOR, but the documentation in
> admin-guide/pm/cpufreq.rst still says the valid range is 1 to 100.
>
> Update the documentation to match the actual code.
>
> Fixes: 2a0e49279850 ("cpufreq: User/admin documentation update and consolidation")
Thanks Pengjie,
Yes, commit 3f78a9f7fcee introduced MAX_SAMPLING_DOWN_FACTOR (100000),
and commit 2a0e49279850 updated the documentation later, so the Fixes
tag is correct.
Small nit: "documentation range" feels a bit redundant; just "range"
might be enough.
Looks good to me.
Reviewed-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
> Signed-off-by: Pengjie Zhang <zhangpengjie2@huawei.com>
> ---
> Documentation/admin-guide/pm/cpufreq.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/admin-guide/pm/cpufreq.rst b/Documentation/admin-guide/pm/cpufreq.rst
> index dbe6d23a5d67..fdca59c955dc 100644
> --- a/Documentation/admin-guide/pm/cpufreq.rst
> +++ b/Documentation/admin-guide/pm/cpufreq.rst
> @@ -516,7 +516,7 @@ This governor exposes the following tunables:
> of those tasks above 0 and set this attribute to 1.
>
> ``sampling_down_factor``
> - Temporary multiplier, between 1 (default) and 100 inclusive, to apply to
> + Temporary multiplier, between 1 (default) and 100000 inclusive, to apply to
> the ``sampling_rate`` value if the CPU load goes above ``up_threshold``.
>
> This causes the next execution of the governor's worker routine (after
--
Thx and BRs,
Zhongqiu Han
^ permalink raw reply
* Re: [RFC PATCH v3 2/3] Documentation: add kconfirm
From: Miguel Ojeda @ 2026-05-17 6:05 UTC (permalink / raw)
To: Julian Braha
Cc: nathan, nsc, jani.nikula, akpm, gary, ljs, arnd, gregkh,
masahiroy, ojeda, corbet, qingfang.deng, yann.prono, demiobenour,
ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild
In-Reply-To: <20260516215354.449807-3-julianbraha@gmail.com>
On Sat, May 16, 2026 at 11:54 PM Julian Braha <julianbraha@gmail.com> wrote:
>
> +kconfirm's Minimum Supported Rust Version (MSRV) is v1.85.0, because
> +it uses Rust edition 2024, and this is the earliest supported version.
Note: this means it will be the first code within the kernel tree
using the new edition.
I think it is fine, since in general no one should be copying code
from here to kernel code or vice versa.
(For context for others: code in one edition can have different
behavior than in another edition, and thus it is risky to mix them up
by mistake).
> +In ``scripts/kconfirm/`` run the following to download the dependencies::
> +
> + cargo vendor
I am not sure how important this is for `scripts/` and/or `tools/`
(Kbuild may have a policy), but this should probably handle `O=`
builds.
In some cases, the source tree may even be read-only, i.e. we wouldn't
be able to create `target/` there.
Cheers,
Miguel
^ permalink raw reply
* Re: [RFC PATCH v3 1/3] scripts: add kconfirm
From: Demi Marie Obenour @ 2026-05-17 6:10 UTC (permalink / raw)
To: Julian Braha, nathan, nsc
Cc: jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda,
corbet, qingfang.deng, yann.prono, ej, linux-kernel,
rust-for-linux, linux-doc, linux-kbuild
In-Reply-To: <20260516215354.449807-2-julianbraha@gmail.com>
[-- Attachment #1.1.1: Type: text/plain, Size: 91429 bytes --]
On 5/16/26 17:53, Julian Braha wrote:
> Add kconfirm into scripts/
>
> kconfirm is a static analysis tool with various checks for Kconfig, and
> intended to have zero false alarms by default. These default checks
> currently include dead code, constant conditions, and invalid (reverse)
> ranges.
>
> There are also optional checks for dead links in the help texts, and for
> config options that select visible config options.
>
> Checks are performed on the same architecture as the kernel build, using
> a single thread. More architectures can be enabled by passing
> `--enable-arch`. Alarms are tagged using the architectures' config options,
> like so: [X86] if specific to x86, or [X86, ARM] if the alarm appears for
> both x86 and arm.
>
> Each alarm gets a single line (deduplicated across architectures) and is
> formatted like this:
> [<SEVERITY>] [<ARCH_1>, <ARCH_2>] config <OPTION_NAME>: <alarm message>
>
> The tool source contains two Rust packages: kconfirm-lib and
> kconfirm-linux.
>
> kconfirm-lib is the underlying library that analyzes Kconfig code, and
> formats alarms for usability. It analyzes the entire Linux Kconfig spec,
> including all architectures. This package exposes the symbol table that it
> constructs so that other tools can import this library, and make use of it
> for their own Kconfig analyses.
>
> kconfirm-linux imports kconfirm-lib, and provides the CLI, which is
> intended for either manual usage, or integration with the Linux build
> system so that users can simply run `make kconfirm` from the root.
> kconfirm-linux also handles some of the specificities of how Kconfig is
> used in the Linux tree, in contrast to other open source software. E.g.
> the way that each architecture has its own Kconfig and Kconfig.debug
> files.
>
> The tool's dependencies need to be downloaded from crates.io by running
> `cargo vendor` in scripts/kconfirm/ before building and running the tool.
>
> Signed-off-by: Julian Braha <julianbraha@gmail.com>
> ---
> Makefile | 15 +-
> scripts/Makefile | 2 +-
> scripts/kconfirm/.gitignore | 3 +
> scripts/kconfirm/Cargo.lock | 60 ++
> scripts/kconfirm/Cargo.toml | 12 +
> scripts/kconfirm/Makefile | 14 +
> scripts/kconfirm/kconfirm-lib/Cargo.toml | 12 +
> scripts/kconfirm/kconfirm-lib/src/analyze.rs | 643 ++++++++++++++++
> scripts/kconfirm/kconfirm-lib/src/checks.rs | 701 ++++++++++++++++++
> scripts/kconfirm/kconfirm-lib/src/curl_ffi.rs | 182 +++++
> .../kconfirm/kconfirm-lib/src/dead_links.rs | 138 ++++
> scripts/kconfirm/kconfirm-lib/src/lib.rs | 62 ++
> scripts/kconfirm/kconfirm-lib/src/output.rs | 111 +++
> .../kconfirm/kconfirm-lib/src/symbol_table.rs | 223 ++++++
> scripts/kconfirm/kconfirm-linux/Cargo.toml | 10 +
> .../kconfirm/kconfirm-linux/src/getopt_ffi.rs | 99 +++
> scripts/kconfirm/kconfirm-linux/src/lib.rs | 78 ++
> scripts/kconfirm/kconfirm-linux/src/main.rs | 192 +++++
> 18 files changed, 2552 insertions(+), 5 deletions(-)
> create mode 100644 scripts/kconfirm/.gitignore
> create mode 100644 scripts/kconfirm/Cargo.lock
> create mode 100644 scripts/kconfirm/Cargo.toml
> create mode 100644 scripts/kconfirm/Makefile
> create mode 100644 scripts/kconfirm/kconfirm-lib/Cargo.toml
> create mode 100644 scripts/kconfirm/kconfirm-lib/src/analyze.rs
> create mode 100644 scripts/kconfirm/kconfirm-lib/src/checks.rs
> create mode 100644 scripts/kconfirm/kconfirm-lib/src/curl_ffi.rs
> create mode 100644 scripts/kconfirm/kconfirm-lib/src/dead_links.rs
> create mode 100644 scripts/kconfirm/kconfirm-lib/src/lib.rs
> create mode 100644 scripts/kconfirm/kconfirm-lib/src/output.rs
> create mode 100644 scripts/kconfirm/kconfirm-lib/src/symbol_table.rs
> create mode 100644 scripts/kconfirm/kconfirm-linux/Cargo.toml
> create mode 100644 scripts/kconfirm/kconfirm-linux/src/getopt_ffi.rs
> create mode 100644 scripts/kconfirm/kconfirm-linux/src/lib.rs
> create mode 100644 scripts/kconfirm/kconfirm-linux/src/main.rs
>
> diff --git a/Makefile b/Makefile
> index b7b80e84e1eb..99aaed5bdbc5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -296,7 +296,7 @@ no-dot-config-targets := $(clean-targets) \
> $(version_h) headers headers_% archheaders archscripts \
> %asm-generic kernelversion %src-pkg dt_binding_check \
> outputmakefile rustavailable rustfmt rustfmtcheck \
> - run-command
> + run-command kconfirm
> no-sync-config-targets := $(no-dot-config-targets) %install modules_sign kernelrelease \
> image_name
> single-targets := %.a %.i %.ko %.lds %.ll %.lst %.mod %.o %.rsi %.s %/
> @@ -536,6 +536,7 @@ OBJDUMP = $(CROSS_COMPILE)objdump
> READELF = $(CROSS_COMPILE)readelf
> STRIP = $(CROSS_COMPILE)strip
> endif
> +CARGO = cargo
> RUSTC = rustc
> RUSTDOC = rustdoc
> RUSTFMT = rustfmt
> @@ -633,7 +634,7 @@ export RUSTC_BOOTSTRAP := 1
> export CLIPPY_CONF_DIR := $(srctree)
>
> export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC HOSTPKG_CONFIG
> -export RUSTC RUSTDOC RUSTFMT RUSTC_OR_CLIPPY_QUIET RUSTC_OR_CLIPPY BINDGEN LLVM_LINK
> +export CARGO RUSTC RUSTDOC RUSTFMT RUSTC_OR_CLIPPY_QUIET RUSTC_OR_CLIPPY BINDGEN LLVM_LINK
> export HOSTRUSTC KBUILD_HOSTRUSTFLAGS
> export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS LEX YACC AWK INSTALLKERNEL
> export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX
> @@ -1705,7 +1706,7 @@ MRPROPER_FILES += include/config include/generated \
> vmlinux-gdb.py \
> rpmbuild \
> rust/libmacros.so rust/libmacros.dylib \
> - rust/libpin_init_internal.so rust/libpin_init_internal.dylib
> + rust/libpin_init_internal.so rust/libpin_init_internal.dylib \
>
> # clean - Delete most, but leave enough to build external modules
> #
> @@ -2227,7 +2228,7 @@ endif
> # Scripts to check various things for consistency
> # ---------------------------------------------------------------------------
>
> -PHONY += includecheck versioncheck coccicheck
> +PHONY += includecheck versioncheck coccicheck kconfirm
>
> includecheck:
> find $(srctree)/* $(RCS_FIND_IGNORE) \
> @@ -2242,6 +2243,12 @@ versioncheck:
> coccicheck:
> $(Q)$(BASH) $(srctree)/scripts/$@
>
> +
> +kconfirm:
> + $(Q)$(MAKE) -C $(srctree)/scripts/kconfirm srctree=$(abs_srctree) SRCARCH=$(SRCARCH) || \
> + (printf "\n kconfirm failed to compile and run. Have you set up its dependencies yet?\n See Documentation/dev-tools/kconfirm.rst\n\n" && false)
> +clean-dirs += scripts/kconfirm
> +
> PHONY += checkstack kernelrelease kernelversion image_name
>
> # UML needs a little special treatment here. It wants to use the host
> diff --git a/scripts/Makefile b/scripts/Makefile
> index 3434a82a119f..460655bd2de1 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -66,4 +66,4 @@ subdir-$(CONFIG_SECURITY_SELINUX) += selinux
> subdir-$(CONFIG_SECURITY_IPE) += ipe
>
> # Let clean descend into subdirs
> -subdir- += basic dtc gdb kconfig mod
> +subdir- += basic dtc gdb kconfig kconfirm mod
> diff --git a/scripts/kconfirm/.gitignore b/scripts/kconfirm/.gitignore
> new file mode 100644
> index 000000000000..f63ee0251591
> --- /dev/null
> +++ b/scripts/kconfirm/.gitignore
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +/target
> +/vendor
> diff --git a/scripts/kconfirm/Cargo.lock b/scripts/kconfirm/Cargo.lock
> new file mode 100644
> index 000000000000..d90bc7d2e2a3
> --- /dev/null
> +++ b/scripts/kconfirm/Cargo.lock
> @@ -0,0 +1,60 @@
> +# This file is automatically @generated by Cargo.
> +# It is not intended for manual editing.
> +version = 4
> +
> +[[package]]
> +name = "bytecount"
> +version = "0.6.9"
> +source = "registry+https://github.com/rust-lang/crates.io-index"
> +checksum = "175812e0be2bccb6abe50bb8d566126198344f707e304f45c648fd8f2cc0365e"
> +
> +[[package]]
> +name = "kconfirm-lib"
> +version = "0.10.0"
> +dependencies = [
> + "nom-kconfig",
> +]
> +
> +[[package]]
> +name = "kconfirm-linux"
> +version = "0.10.0"
> +dependencies = [
> + "kconfirm-lib",
> + "nom-kconfig",
> +]
> +
> +[[package]]
> +name = "memchr"
> +version = "2.8.0"
> +source = "registry+https://github.com/rust-lang/crates.io-index"
> +checksum = "f8ca58f447f06ed17d5fc4043ce1b10dd205e060fb3ce5b979b8ed8e59ff3f79"
> +
> +[[package]]
> +name = "nom"
> +version = "8.0.0"
> +source = "registry+https://github.com/rust-lang/crates.io-index"
> +checksum = "df9761775871bdef83bee530e60050f7e54b1105350d6884eb0fb4f46c2f9405"
> +dependencies = [
> + "memchr",
> +]
> +
> +[[package]]
> +name = "nom-kconfig"
> +version = "0.11.0"
> +source = "registry+https://github.com/rust-lang/crates.io-index"
> +checksum = "5a0220bb2c8e5ad29b06fe0f75a276affeb10c9504726bf46d81fef78d69b1e3"
> +dependencies = [
> + "nom",
> + "nom_locate",
> +]
> +
> +[[package]]
> +name = "nom_locate"
> +version = "5.0.0"
> +source = "registry+https://github.com/rust-lang/crates.io-index"
> +checksum = "0b577e2d69827c4740cba2b52efaad1c4cc7c73042860b199710b3575c68438d"
> +dependencies = [
> + "bytecount",
> + "memchr",
> + "nom",
> +]
> diff --git a/scripts/kconfirm/Cargo.toml b/scripts/kconfirm/Cargo.toml
> new file mode 100644
> index 000000000000..5880b06c4116
> --- /dev/null
> +++ b/scripts/kconfirm/Cargo.toml
> @@ -0,0 +1,12 @@
> +# SPDX-License-Identifier: GPL-2.0
> +[workspace]
> +members = ["kconfirm-lib", "kconfirm-linux"]
> +resolver = "3"
> +
> +[workspace.package]
> +rust-version = "1.85.0"
> +
> +[workspace.dependencies]
> +nom-kconfig = { version = "0.11", default-features = false, features = [
> + "display",
> +] }
> diff --git a/scripts/kconfirm/Makefile b/scripts/kconfirm/Makefile
> new file mode 100644
> index 000000000000..6a0b7389103e
> --- /dev/null
> +++ b/scripts/kconfirm/Makefile
> @@ -0,0 +1,14 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# kconfirm makefile
> +
> +TARGET := kconfirm
> +
> +# Extra arguments forwarded to kconfirm.
> +# Example: make kconfirm KCONFIRM_ARGS="--enable-check dead_links"
> +KCONFIRM_ARGS ?=
> +
> +$(TARGET):
> + $(CARGO) run --release --offline -p kconfirm-linux -- --linux-path $(srctree) --enable-arch $(SRCARCH) $(KCONFIRM_ARGS)
> +
> +
> +clean-files += target vendor
> diff --git a/scripts/kconfirm/kconfirm-lib/Cargo.toml b/scripts/kconfirm/kconfirm-lib/Cargo.toml
> new file mode 100644
> index 000000000000..dd3d7cb1aa1d
> --- /dev/null
> +++ b/scripts/kconfirm/kconfirm-lib/Cargo.toml
> @@ -0,0 +1,12 @@
> +# SPDX-License-Identifier: GPL-2.0
> +[package]
> +name = "kconfirm-lib"
> +version = "0.10.0"
> +edition = "2024"
> +rust-version.workspace = true
> +
> +[dependencies]
> +nom-kconfig = { workspace = true }
> +
> +[features]
> +default = []
> diff --git a/scripts/kconfirm/kconfirm-lib/src/analyze.rs b/scripts/kconfirm/kconfirm-lib/src/analyze.rs
> new file mode 100644
> index 000000000000..24798581dc3d
> --- /dev/null
> +++ b/scripts/kconfirm/kconfirm-lib/src/analyze.rs
> @@ -0,0 +1,643 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +use crate::AnalysisArgs;
> +use crate::Check;
> +use crate::SymbolTable;
> +use crate::dead_links;
> +use crate::dead_links::LinkStatus;
> +use crate::dead_links::check_link;
> +use crate::output::Finding;
> +use crate::output::Severity;
> +use crate::symbol_table::ChoiceData;
> +use nom_kconfig::Attribute::*;
> +use nom_kconfig::Entry;
> +use nom_kconfig::attribute::DefaultAttribute;
> +use nom_kconfig::attribute::Expression;
> +use nom_kconfig::attribute::Imply;
> +use nom_kconfig::attribute::Select;
> +use nom_kconfig::attribute::r#type::Type;
> +use nom_kconfig::entry::Choice;
> +use nom_kconfig::entry::Config;
> +use nom_kconfig::entry::If;
> +use nom_kconfig::entry::Menu;
> +use nom_kconfig::entry::Source;
> +use std::collections::HashSet;
> +use std::option::Option;
> +
> +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
> +enum FunctionalAttributes {
> + // only tracking the attributes that affect the semantics, e.g. not help texts
> + Dependencies,
> + Selects,
> + Implies,
> + Ranges,
> + Defaults,
> +}
> +
> +struct AttributeGroupingChecker {
> + current_group: Option<FunctionalAttributes>,
> + finished_groups: HashSet<FunctionalAttributes>,
> +}
> +
> +impl AttributeGroupingChecker {
> + fn new() -> Self {
> + Self {
> + current_group: None,
> + finished_groups: HashSet::new(),
> + }
> + }
> +
> + // doesn't modify `findings` if the style check is disabled
> + fn check(
> + &mut self,
> + group: FunctionalAttributes,
> + args: &AnalysisArgs,
> + findings: &mut Vec<Finding>,
> + symbol: &str,
> + arch: &String,
> + message: String,
> + ) {
> + if !args.is_enabled(Check::UngroupedAttribute) {
> + return;
> + }
> +
> + match self.current_group {
> + // still contiguous
> + Some(current) if current == group => {}
> +
> + // start of group
> + None => {
> + self.current_group = Some(group);
> + }
> +
> + Some(current) => {
> + // the previous group finished
> + self.finished_groups.insert(current);
> +
> + // we've already finished this group, it's ungrouped
> + if self.finished_groups.contains(&group) {
> + findings.push(Finding {
> + severity: Severity::Style,
> + check: Check::UngroupedAttribute,
> + symbol: Some(symbol.to_string()),
> + message,
> + arch: arch.to_owned(),
> + });
> + }
> +
> + // switch to the new group
> + self.current_group = Some(group);
> + }
> + }
> + }
> +}
> +
> +struct DeadLinkChecker {
> + visited_links: HashSet<String>,
> +}
> +
> +impl DeadLinkChecker {
> + fn new() -> Self {
> + Self {
> + visited_links: HashSet::new(),
> + }
> + }
> +
> + fn check_text(
> + &mut self,
> + text: &str,
> + args: &AnalysisArgs,
> + findings: &mut Vec<Finding>,
> + symbol: Option<&str>,
> + arch: &String,
> + context: &str,
> + ) {
> + if !args.is_enabled(Check::DeadLink) {
> + return;
> + }
> +
> + let links = dead_links::find_links(text);
> +
> + if links.is_empty() {
> + return;
> + }
> +
> + for link in links {
> + // avoid rechecking identical links
> + if !self.visited_links.insert(link.clone()) {
> + continue;
> + }
> +
> + let status = check_link(&link);
> + if status != LinkStatus::Ok && status != LinkStatus::ProbablyBlocked {
> + findings.push(Finding {
> + severity: Severity::Warning,
> + check: Check::DeadLink,
> + symbol: symbol.map(|s| s.to_string()),
> + message: format!(
> + "{} contains link {} with status {:?}",
> + context, link, status
> + ),
> + arch: arch.to_owned(),
> + });
> + }
> + }
> + }
> +}
> +
> +#[derive(Clone)]
> +pub struct Context {
> + pub arch: String,
> + pub definition_condition: Vec<Expression>,
> + pub visibility: Vec<Option<Expression>>,
> + pub dependencies: Vec<Expression>,
> + pub in_choice: bool,
> +}
> +
> +impl Context {
> + fn with_arch(arch: String) -> Context {
> + Context {
> + arch,
> + definition_condition: vec![],
> + visibility: vec![],
> + dependencies: vec![],
> + in_choice: false,
> + }
> + }
> +
> + fn child(&self) -> Self {
> + self.clone()
> + }
> +
> + fn with_dep(mut self, dep: Expression) -> Self {
> + self.dependencies.push(dep);
> + self
> + }
> +
> + fn with_visibility(mut self, cond: Option<Expression>) -> Self {
> + self.visibility.push(cond);
> + self
> + }
> +
> + fn with_definition(mut self, cond: Expression) -> Self {
> + self.definition_condition.push(cond);
> + self
> + }
> +
> + fn in_choice(mut self) -> Self {
> + self.in_choice = true;
> + self
> + }
> +}
> +
> +fn recurse_entries(
> + args: &AnalysisArgs,
> + symtab: &mut SymbolTable,
> + entries: Vec<Entry>,
> + ctx: Context,
> + findings: &mut Vec<Finding>,
> +) {
> + for entry in entries {
> + process_entry(args, symtab, entry, ctx.clone(), findings);
> + }
> +}
> +
> +pub fn analyze(
> + args: &AnalysisArgs,
> + symtab: &mut SymbolTable,
> + arch: String,
> + entries: Vec<Entry>,
> +) -> Vec<Finding> {
> + let mut findings = Vec::new();
> +
> + let ctx = Context::with_arch(arch);
> +
> + recurse_entries(args, symtab, entries, ctx, &mut findings);
> +
> + findings
> +}
> +
> +fn handle_config(
> + args: &AnalysisArgs,
> + symtab: &mut SymbolTable,
> + entry: Config,
> + ctx: &Context,
> + findings: &mut Vec<Finding>,
> +) {
> + let config_symbol = entry.symbol;
> +
> + let mut child_ctx = ctx.child();
> +
> + let mut config_type = None;
> + let mut kconfig_dependencies = Vec::new();
> + let mut kconfig_selects: Vec<Select> = Vec::new();
> + let mut kconfig_implies: Vec<Imply> = Vec::new();
> + let mut kconfig_ranges = Vec::new();
> + let mut kconfig_defaults = Vec::new();
> + let mut found_prompt = false;
> +
> + /*
> + * style check: ungrouped attributes
> + * - need to check that dependencies, selects, ranges, and defaults are each kept together.
> + */
> + let mut attribute_grouping_checker = AttributeGroupingChecker::new();
> + let mut dead_link_checker = DeadLinkChecker::new();
> + for attribute in entry.attributes {
> + match attribute {
> + Type(kconfig_type) => match kconfig_type.r#type.clone() {
> + // hybrid type definition and default
> + Type::DefBool(db) => {
> + let default_attribute: DefaultAttribute = DefaultAttribute {
> + expression: db.clone(),
> + r#if: kconfig_type.clone().r#if,
> + };
> +
> + kconfig_defaults.push(default_attribute);
> + config_type = Some(kconfig_type);
> +
> + // NOTE: as a style, we prefer to keep the hybrid default-typedef with the standalone defaults
> + attribute_grouping_checker.check(
> + FunctionalAttributes::Defaults,
> + args,
> + findings,
> + &config_symbol,
> + &ctx.arch,
> + format!("ungrouped default {}", db),
> + );
> + }
> + Type::Bool(unconditional_prompt) => {
> + if unconditional_prompt.is_some() {
> + found_prompt = true;
> + }
> + config_type = Some(kconfig_type);
> + }
> +
> + // hybrid type definition and default
> + Type::DefTristate(dt) => {
> + // NOTE: as a style, we prefer to keep the hybrid default-typedef with the standalone defaults
> + attribute_grouping_checker.check(
> + FunctionalAttributes::Defaults,
> + args,
> + findings,
> + &config_symbol,
> + &ctx.arch,
> + format!("ungrouped default {}", &dt),
> + );
> +
> + let default_attribute: DefaultAttribute = DefaultAttribute {
> + expression: dt,
> + r#if: kconfig_type.clone().r#if,
> + };
> +
> + kconfig_defaults.push(default_attribute);
> + config_type = Some(kconfig_type);
> + }
> + Type::Tristate(unconditional_prompt) => {
> + if unconditional_prompt.is_some() {
> + found_prompt = true;
> + }
> +
> + config_type = Some(kconfig_type.clone())
> + }
> + Type::Hex(unconditional_prompt) => {
> + if unconditional_prompt.is_some() {
> + found_prompt = true;
> + }
> +
> + config_type = Some(kconfig_type);
> + }
> + Type::Int(unconditional_prompt) => {
> + if unconditional_prompt.is_some() {
> + found_prompt = true;
> + }
> + config_type = Some(kconfig_type);
> + }
> + Type::String(unconditional_prompt) => {
> + if unconditional_prompt.is_some() {
> + found_prompt = true;
> + }
> + config_type = Some(kconfig_type);
> + }
> + },
> + Default(default) => {
> + attribute_grouping_checker.check(
> + FunctionalAttributes::Defaults,
> + args,
> + findings,
> + &config_symbol,
> + &ctx.arch,
> + format!("ungrouped default {}", &default),
> + );
> +
> + kconfig_defaults.push(default);
> + }
> +
> + DependsOn(depends_on) => {
> + attribute_grouping_checker.check(
> + FunctionalAttributes::Dependencies,
> + args,
> + findings,
> + &config_symbol,
> + &ctx.arch,
> + format!("ungrouped dependency {}", &depends_on),
> + );
> +
> + kconfig_dependencies.push(depends_on);
> + }
> + Select(select) => {
> + attribute_grouping_checker.check(
> + FunctionalAttributes::Selects,
> + args,
> + findings,
> + &config_symbol,
> + &ctx.arch,
> + format!("ungrouped select {}", &select),
> + );
> +
> + kconfig_selects.push(select);
> + }
> + Imply(imply) => {
> + attribute_grouping_checker.check(
> + FunctionalAttributes::Implies,
> + args,
> + findings,
> + &config_symbol,
> + &ctx.arch,
> + format!("ungrouped imply {}", imply),
> + );
> +
> + kconfig_implies.push(imply);
> +
> + // TODO: may be relevant for nonvisible config options when building an SMT model...
> + }
> + // NOTE: range bounds are inclusive
> + Range(r) => {
> + attribute_grouping_checker.check(
> + FunctionalAttributes::Ranges,
> + args,
> + findings,
> + &config_symbol,
> + &ctx.arch,
> + format!("ungrouped range {}", r),
> + );
> +
> + kconfig_ranges.push(r);
> + }
> + Help(h) => {
> + // doing nothing for menu help right now
> +
> + dead_link_checker.check_text(
> + &h,
> + args,
> + findings,
> + Some(&config_symbol),
> + &ctx.arch,
> + "help text",
> + );
> + }
> +
> + Modules => {
> + // the modules attribute designates this config option as the one that determines if the `m` state is available for tristates options.
> +
> + // just making a special note of this in the symtab for now...
> + symtab.modules_option = Some(config_symbol.clone());
> + }
> +
> + // the prompt's option `if` determines "visibility"
> + Prompt(prompt) => {
> + // TODO: once we have SMT solving, we can also check if the prompt condition is always true or never true (and therefore, effectively unconditional)
> +
> + found_prompt = true;
> + if let Some(c) = prompt.r#if {
> + child_ctx = child_ctx.with_visibility(Some(c));
> + }
> + }
> + Transitional => {
> + // doing nothing for transitional right now
> + }
> + Optional | Visible(_) | Requires(_) | Option(_) => {
> + eprintln!("Error: unexpected attribute encountered: {:?}", attribute);
> +
> + if cfg!(debug_assertions) {
> + panic!();
> + }
> + }
> + }
> + }
> +
> + if !found_prompt {
> + child_ctx = child_ctx.with_visibility(None);
> + }
> +
> + // there can be multiple entries that get merged. so we need to do the same for our symtab.
> + let kconfig_type = config_type.clone().map(|c| c.r#type);
> +
> + // at the time of writing this, linux's kconfig only uses Bool inside Choice.
> + // however, the kconfig documentation doesn't specify whether or not this is guaranteed to be the case.
> + // we add this check to ensure that we don't cause undefined behavior in future linux versions if something changes...
> + if child_ctx.in_choice {
> + if let Some(kt) = &kconfig_type {
> + match kt {
> + Type::Bool(_) | Type::DefBool(_) => {
> + // expected in a choice...
> + }
> +
> + _ => {
> + // TODO: old versions of linux (like 5.4.4) have tristates in the choice
> + // - u-boot also currently has hex options in the choice!
> + eprintln!(
> + "Error: found something unexpected in a choice-statement: {:?}",
> + kt
> + );
> + }
> + }
> + }
> + }
> +
> + // at the end, add the file's cur_dependencies to this var's invididual dependencies.
> + kconfig_dependencies.extend(child_ctx.dependencies.clone());
> + symtab.merge_insert_new_solved(
> + config_symbol.clone(),
> + kconfig_type,
> + kconfig_dependencies,
> + //z3_dependency,
> + kconfig_ranges,
> + kconfig_defaults,
> + child_ctx.visibility.clone(),
> + child_ctx.arch.clone(),
> + child_ctx.definition_condition.clone(),
> + None,
> + kconfig_selects
> + .clone()
> + .into_iter()
> + .map(|sel| (sel.symbol, sel.r#if))
> + .collect(),
> + kconfig_implies
> + .into_iter()
> + .map(|imply| (imply.symbol.to_string(), imply.r#if))
> + .collect(),
> + );
> + // TODO: file a github issue, imply can never imply a constant (this is technically parsing incorrectly)
> +
> + // TODO: when SMT solving, we may need to keep track of the implies the same way we keep track of selects,
> + // in cases when the implied config option is non-visible
> +
> + // need to add the select condition to the definedness condition if it exists
> + for select in kconfig_selects {
> + match select.r#if {
> + None => symtab.merge_insert_new_solved(
> + select.symbol,
> + None,
> + Vec::new(),
> + Vec::new(),
> + Vec::new(),
> + Vec::new(),
> + child_ctx.arch.clone(),
> + child_ctx.definition_condition.clone(),
> + Some((config_symbol.clone(), None)),
> + Vec::new(),
> + Vec::new(),
> + ),
> + Some(select_condition) => {
> + symtab.merge_insert_new_solved(
> + select.symbol,
> + None,
> + Vec::new(),
> + Vec::new(),
> + Vec::new(),
> + Vec::new(),
> + child_ctx.arch.clone(),
> + child_ctx.definition_condition.clone(),
> + Some((config_symbol.clone(), Some(select_condition))),
> + Vec::new(),
> + Vec::new(),
> + );
> + }
> + }
> + }
> +}
> +
> +fn handle_menu(
> + args: &AnalysisArgs,
> + symtab: &mut SymbolTable,
> + entry: Menu,
> + ctx: &Context,
> + findings: &mut Vec<Finding>,
> +) {
> + // menus can set the visibility of their menu items
> +
> + let mut child_ctx = ctx.child();
> +
> + for dep in entry.depends_on {
> + child_ctx = child_ctx.with_dep(dep.clone());
> + child_ctx = child_ctx.with_visibility(Some(dep)); // not a typo, the config options inside of a menu are only visible if the menu's dependencies are satisfied
> + }
> +
> + let nested_entries = entry.entries;
> +
> + recurse_entries(args, symtab, nested_entries, child_ctx.clone(), findings);
> +}
> +
> +fn handle_choice(
> + args: &AnalysisArgs,
> + symtab: &mut SymbolTable,
> + entry: Choice,
> + ctx: &Context,
> + findings: &mut Vec<Finding>,
> +) {
> + let mut child_ctx = ctx.child();
> + child_ctx = child_ctx.in_choice();
> +
> + // we are going to add the dependencies of the choice to the dependencies of the entries.
> + // we start with the dependencies inherited from the file
> + let mut choice_visibility_condition = None;
> + let mut defaults = Vec::new();
> + for attribute in entry.options {
> + match attribute {
> + DependsOn(depends_on) => {
> + child_ctx = child_ctx.with_dep(depends_on);
> + }
> +
> + Default(default) => {
> + defaults.push(default);
> + }
> +
> + // the prompt's `if` determines visibility
> + Prompt(prompt) => {
> + choice_visibility_condition = prompt.r#if;
> + if let Some(i) = choice_visibility_condition.clone() {
> + child_ctx = child_ctx.with_visibility(Some(i));
> + }
> + }
> + _ => {
> + // skip
> + }
> + }
> + }
> +
> + // all of the variables in the choice menu
> + //let mut contained_vars = Vec::with_capacity(c.entries.len());
> + let nested_entries = entry.entries;
> +
> + recurse_entries(args, symtab, nested_entries, child_ctx.clone(), findings);
> +
> + let choice_data = ChoiceData {
> + //inner_vars: contained_vars,
> + arch: child_ctx.arch.clone(),
> + visibility: choice_visibility_condition,
> + dependencies: child_ctx.dependencies,
> + defaults,
> + };
> + symtab.choices.push(choice_data);
> +}
> +
> +fn handle_if(
> + args: &AnalysisArgs,
> + symtab: &mut SymbolTable,
> + entry: If,
> + ctx: &Context,
> + findings: &mut Vec<Finding>,
> +) {
> + let mut child_ctx = ctx.child();
> + child_ctx = child_ctx.with_definition(entry.condition.clone());
> + child_ctx = child_ctx.with_dep(entry.condition);
> + let nested_entries = entry.entries;
> +
> + recurse_entries(args, symtab, nested_entries, child_ctx, findings);
> +}
> +
> +fn handle_source(
> + args: &AnalysisArgs,
> + symtab: &mut SymbolTable,
> + entry: Source,
> + ctx: &Context,
> + findings: &mut Vec<Finding>,
> +) {
> + let sourced_kconfig = entry.kconfigs;
> +
> + for sourced_kconfig in sourced_kconfig {
> + recurse_entries(args, symtab, sourced_kconfig.entries, ctx.clone(), findings);
> + }
> +}
> +
> +pub fn process_entry(
> + args: &AnalysisArgs,
> + symtab: &mut SymbolTable,
> + entry: Entry,
> + ctx: Context,
> + findings: &mut Vec<Finding>,
> +) {
> + // NOTE: in general, each handler should update the context as it encounters that construct.
> + // e.g. Context.in_choice() should be called at the start of handle_choice(), not right before call to process_entry() when a choice is found and process_entry is called
> + match entry {
> + Entry::Config(c) | Entry::MenuConfig(c) => {
> + handle_config(args, symtab, c, &ctx, findings);
> + }
> + Entry::Menu(m) => handle_menu(args, symtab, m, &ctx, findings),
> + Entry::Choice(c) => handle_choice(args, symtab, c, &ctx, findings),
> + Entry::If(i) => handle_if(args, symtab, i, &ctx, findings),
> + Entry::Source(s) => handle_source(args, symtab, s, &ctx, findings),
> + Entry::Comment(_) => {}
> + Entry::MainMenu(_) => {}
> + _ => {}
> + }
> +}
> diff --git a/scripts/kconfirm/kconfirm-lib/src/checks.rs b/scripts/kconfirm/kconfirm-lib/src/checks.rs
> new file mode 100644
> index 000000000000..2ad67f4390ea
> --- /dev/null
> +++ b/scripts/kconfirm/kconfirm-lib/src/checks.rs
> @@ -0,0 +1,701 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +use crate::output::Finding;
> +use crate::output::Severity;
> +use crate::symbol_table::AttributeDef;
> +use crate::symbol_table::TypeInfo;
> +use nom_kconfig::attribute::Expression;
> +use nom_kconfig::attribute::range::RangeBound;
> +use std::collections::HashSet;
> +use std::num::ParseIntError;
> +use std::str::FromStr;
> +
> +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
> +pub enum Check {
> + FailedParse,
> + UngroupedAttribute, // check for duplicate default values, and ungrouped attributes
> + DeadLink, // check for dead links in the help texts
> + SelectVisible,
> + // need SMT solving before we can detect select-undefineds
> + //SelectUndefined,
> + DuplicateDependency,
> + DuplicateRange,
> + DeadRange,
> + DuplicateSelect,
> + DeadSelect,
> + DeadDefault,
> + ConstantCondition,
> + DuplicateDefault,
> + DuplicateDefaultValue,
> + DuplicateImply,
> + DeadImply,
> + ReverseRange,
> +}
> +
> +impl Check {
> + pub fn as_str(self) -> &'static str {
> + match self {
> + Check::FailedParse => "failed_parse",
> + Check::UngroupedAttribute => "ungrouped_attribute",
> + Check::DeadLink => "dead_link",
> + Check::SelectVisible => "select_visible",
> + Check::DuplicateDependency => "duplicate_dependency",
> + Check::DuplicateRange => "duplicate_range",
> + Check::DeadRange => "dead_range",
> + Check::DuplicateSelect => "duplicate_select",
> + Check::DeadSelect => "dead_select",
> + Check::DeadDefault => "dead_default",
> + Check::ConstantCondition => "constant_condition",
> + Check::DuplicateDefault => "duplicate_default",
> + Check::DuplicateDefaultValue => "duplicate_default_value",
> + Check::DuplicateImply => "duplicate_imply",
> + Check::DeadImply => "dead_imply",
> + Check::ReverseRange => "reverse_range",
> + }
> + }
> +}
> +
> +#[derive(Debug)]
> +pub struct ParseCheckError {
> + pub input: String,
> +}
> +
> +impl std::fmt::Display for ParseCheckError {
> + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
> + write!(f, "unknown check '{}'", self.input)
> + }
> +}
> +
> +impl std::error::Error for ParseCheckError {}
> +
> +impl FromStr for Check {
> + type Err = ParseCheckError;
> +
> + fn from_str(name: &str) -> Result<Self, Self::Err> {
> + match name {
> + "failed_parse" => Ok(Check::FailedParse),
> + "ungrouped_attribute" => Ok(Check::UngroupedAttribute),
> + "dead_link" => Ok(Check::DeadLink),
> + "select_visible" => Ok(Check::SelectVisible),
> + "duplicate_dependency" => Ok(Check::DuplicateDependency),
> + "duplicate_range" => Ok(Check::DuplicateRange),
> + "dead_range" => Ok(Check::DeadRange),
> + "duplicate_select" => Ok(Check::DuplicateSelect),
> + "dead_select" => Ok(Check::DeadSelect),
> + "dead_default" => Ok(Check::DeadDefault),
> + "constant_condition" => Ok(Check::ConstantCondition),
> + "duplicate_default" => Ok(Check::DuplicateDefault),
> + "duplicate_default_value" => Ok(Check::DuplicateDefaultValue),
> + "duplicate_imply" => Ok(Check::DuplicateImply),
> + "dead_imply" => Ok(Check::DeadImply),
> + "reverse_range" => Ok(Check::ReverseRange),
> + _ => Err(ParseCheckError {
> + input: name.to_string(),
> + }),
> + }
> + }
> +}
> +
> +#[derive(Clone, Debug)]
> +pub struct AnalysisArgs {
> + // check for duplicate default values
> + pub enabled_checks: HashSet<Check>,
> +}
> +
> +impl AnalysisArgs {
> + pub fn is_enabled(&self, check: Check) -> bool {
> + self.enabled_checks.contains(&check)
> + }
> +}
> +
> +// returns an Error if a hex range bound cannot be parsed as an u64
> +pub fn check_reverse_ranges(arch: &String, var_symbol: &str, info: &AttributeDef) -> Vec<Finding> {
> + let mut findings = Vec::new();
> +
> + for range in &info.kconfig_ranges {
> + // returns an Error if a hex range bound cannot be parsed as an u64
> + fn range_bound_to_int(range_bound: &RangeBound) -> Result<i128, ParseIntError> {
> + match range_bound {
> + RangeBound::Number(b) => {
> + return Ok(b.to_owned() as i128);
> + }
> + RangeBound::Hex(b_str) => {
> + let trimmed = b_str.trim_start_matches("0x").trim_start_matches("0X");
> +
> + return i128::from_str_radix(trimmed, 16);
> + }
> + RangeBound::Variable(_) => {
> + // for now, the caller is expected not to pass these cases.
> + unreachable!("not handling variable ranges until SMT solving");
> + }
> + RangeBound::Symbol(_) => {
> + // TODO: need SMT solving for this case
> + // for now, the caller is expected not to pass these cases.
> + unreachable!("not handling CONFIG ranges until SMT solving");
> + }
> + }
> + }
> +
> + if matches!(range.lower_bound, RangeBound::Symbol(_))
> + || matches!(range.upper_bound, RangeBound::Symbol(_))
> + {
> + // not handling these cases until SMT solving.
> + // don't return though, because we stil want to check the other ranges.
> + continue;
> + }
> +
> + let maybe_lower_bound = range_bound_to_int(&range.lower_bound);
> + let maybe_upper_bound = range_bound_to_int(&range.upper_bound);
> +
> + match (maybe_lower_bound, maybe_upper_bound) {
> + (Ok(lower_bound), Ok(upper_bound)) => {
> + if lower_bound > upper_bound {
> + let message = format!(
> + "reverse range {} for config option: {}, no value is valid",
> + range.to_string(),
> + var_symbol,
> + );
> + findings.push(Finding {
> + severity: Severity::Warning,
> + check: Check::ReverseRange,
> + symbol: Some(var_symbol.to_owned()),
> + arch: arch.to_owned(),
> + message,
> + });
> + }
> + }
> + (Result::Err(_), _) | (_, Result::Err(_)) => {
> + eprintln!(
> + "Error: couldn't parse hex range bound as i128 for config option: {}",
> + var_symbol
> + );
> + // still want to check the other range bounds
> + continue;
> + }
> + }
> + }
> +
> + findings
> +}
> +
> +pub fn check_constant_conditions(
> + arch: &String,
> + var_symbol: &str,
> + info: &AttributeDef,
> +) -> Vec<Finding> {
> + let mut findings = Vec::new();
> + let default_conditions: Vec<&Expression> = info
> + .kconfig_defaults
> + .iter()
> + .filter_map(|conditional_default| conditional_default.r#if.as_ref())
> + .collect();
> +
> + check_conditions(
> + arch,
> + &mut findings,
> + &var_symbol,
> + &info.kconfig_dependencies,
> + default_conditions,
> + "default",
> + );
> +
> + let select_conditions: Vec<&Expression> = info
> + .selects
> + .iter()
> + .filter_map(|conditional_select| conditional_select.1.as_ref())
> + .collect();
> +
> + check_conditions(
> + arch,
> + &mut findings,
> + var_symbol,
> + &info.kconfig_dependencies,
> + select_conditions,
> + "select",
> + );
> +
> + let imply_conditions: Vec<&Expression> = info
> + .implies
> + .iter()
> + .filter_map(|imp| imp.1.as_ref())
> + .collect();
> +
> + check_conditions(
> + arch,
> + &mut findings,
> + var_symbol,
> + &info.kconfig_dependencies,
> + imply_conditions,
> + "imply",
> + );
> +
> + let range_conditions: Vec<&Expression> = info
> + .kconfig_ranges
> + .iter()
> + .filter_map(|conditional_range| conditional_range.r#if.as_ref())
> + .collect();
> +
> + check_conditions(
> + arch,
> + &mut findings,
> + var_symbol,
> + &info.kconfig_dependencies,
> + range_conditions,
> + "range",
> + );
> +
> + fn check_conditions(
> + arch: &String,
> + findings: &mut Vec<Finding>,
> + symbol: &str,
> + kconfig_dependencies: &[Expression],
> + attribute_conditions: Vec<&Expression>,
> + context: &str,
> + ) {
> + for attribute_condition in attribute_conditions.into_iter() {
> + if kconfig_dependencies.contains(attribute_condition) {
> + let message = format!(
> + "constant {} condition 'if {}' for config option: {}, this condition is a dependency and will always be true",
> + context,
> + attribute_condition.to_string(),
> + symbol,
> + );
> + findings.push(Finding {
> + severity: Severity::Warning,
> + check: Check::ConstantCondition,
> + symbol: Some(symbol.to_owned()),
> + arch: arch.to_owned(),
> + message,
> + });
> + }
> + }
> + }
> + findings
> +}
> +
> +pub fn check_variable_info(
> + args: &AnalysisArgs,
> + var_symbol: &str,
> + arch_specific: &String,
> + info: &AttributeDef,
> +) -> Vec<Finding> {
> + let mut findings = Vec::new();
> +
> + if args.is_enabled(Check::DuplicateDependency) {
> + findings.extend(check_duplicate_dependencies(
> + arch_specific,
> + var_symbol,
> + info,
> + ));
> + }
> +
> + if args.is_enabled(Check::DuplicateImply) {
> + findings.extend(check_duplicate_implies(arch_specific, var_symbol, info));
> + }
> +
> + if args.is_enabled(Check::DuplicateRange) {
> + findings.extend(check_duplicate_ranges(arch_specific, var_symbol, info));
> + }
> +
> + if args.is_enabled(Check::DuplicateSelect) {
> + findings.extend(check_duplicate_selects(arch_specific, var_symbol, info));
> + }
> +
> + if args.is_enabled(Check::ConstantCondition) {
> + findings.extend(check_constant_conditions(arch_specific, var_symbol, info));
> + }
> +
> + if args.is_enabled(Check::DeadDefault)
> + || args.is_enabled(Check::DuplicateDefault)
> + || args.is_enabled(Check::DuplicateDefaultValue)
> + {
> + findings.extend(check_defaults(arch_specific, var_symbol, info, args));
> + }
> +
> + if args.is_enabled(Check::ReverseRange) {
> + findings.extend(check_reverse_ranges(arch_specific, var_symbol, info));
> + }
> +
> + findings
> +}
> +
> +// TODO: also check if a config option in one arch unconditionally references a config option that only exists in another arch (need SMT for this first)
> +pub fn check_select_visible(var_symbol: &str, info: &TypeInfo) -> Vec<Finding> {
> + let mut findings = Vec::new();
> +
> + // only interested in the options that are selected
> + if info.selected_by.is_empty() {
> + return Vec::new();
> + }
> +
> + for (selector, select_info) in &info.selected_by {
> + for (arch, _cond) in select_info {
> + // NOTE: we don't care if the select is conditional or unconditional, just the selectee's visibility
> +
> + // at this point, we know that `selector` unconditionally selects `var_symbol`
> + // now, we need to check if `var_symbol` is unconditionally visible
> +
> + let message = format!(
> + "selects the visible {}; consider using 'depends on' or 'imply' instead",
> + var_symbol
> + );
> +
> + // match the architecture that the select happens under with the architecture of the unconditional visibility
> + match info.attribute_defs.get(arch) {
> + None => {
> + // not selected in this architecture
> + }
> + Some(cur_arch_attribute_def) => {
> + for (if_conditions, attributes) in cur_arch_attribute_def {
> + if if_conditions.is_empty() && attributes.visibility.is_empty() {
> + // empty visiblity means that it is unconditionally visible, within the current arch (assuming arch is not `None`)
> +
> + findings.push(Finding {
> + severity: Severity::Warning,
> + check: Check::SelectVisible,
> + symbol: Some(selector.to_owned()),
> + message: message.clone(),
> + arch: arch.to_owned(),
> + });
> + }
> + }
> + }
> + }
> + }
> + }
> +
> + findings
> +}
> +
> +fn is_duplicate<T: Eq + std::hash::Hash>(set: &mut HashSet<T>, key: T) -> bool {
> + !set.insert(key)
> +}
> +
> +fn check_duplicate_dependencies(
> + arch_specific: &String,
> + var_symbol: &str,
> + info: &AttributeDef,
> +) -> Vec<Finding> {
> + let mut findings = Vec::new();
> + let mut seen = HashSet::new();
> +
> + for dep in &info.kconfig_dependencies {
> + if is_duplicate(&mut seen, dep.to_string()) {
> + let message = format!("duplicate dependency on {}", dep.to_string());
> + findings.push(Finding {
> + severity: Severity::Warning,
> + check: Check::DuplicateDependency,
> + symbol: Some(var_symbol.to_owned()),
> + message,
> + arch: arch_specific.to_owned(),
> + });
> + }
> + }
> +
> + findings
> +}
> +
> +fn check_duplicate_implies(arch: &String, var_symbol: &str, info: &AttributeDef) -> Vec<Finding> {
> + let mut findings = Vec::new();
> +
> + // symbols implied unconditionally
> + let mut unconditional: HashSet<String> = HashSet::new();
> +
> + // (symbol, condition)
> + let mut conditional: HashSet<(String, String)> = HashSet::new();
> +
> + for imp in &info.implies {
> + let imply_var = imp.0.clone();
> +
> + match &imp.1 {
> + Some(cond) => {
> + let cond_str = cond.to_string();
> +
> + // duplicate conditional imply
> + if !conditional.insert((imply_var.clone(), cond_str.clone())) {
> + findings.push(Finding {
> + severity: Severity::Warning,
> + check: Check::DuplicateImply,
> + symbol: Some(var_symbol.to_owned()),
> + message: format!(
> + "duplicate imply of {:?} with condition {}",
> + imp.0, cond_str
> + ),
> + arch: arch.to_owned(),
> + });
> + }
> +
> + // conditional imply is dead if unconditional exists
> + if unconditional.contains(&imply_var) {
> + findings.push(Finding {
> + severity: Severity::Warning,
> + check: Check::DeadImply,
> + symbol: Some(var_symbol.to_owned()),
> + message: format!("dead imply of {:?}", imp),
> + arch: arch.to_owned(),
> + });
> + }
> + }
> +
> + None => {
> + // duplicate unconditional imply
> + if !unconditional.insert(imply_var.clone()) {
> + findings.push(Finding {
> + severity: Severity::Warning,
> + check: Check::DuplicateImply,
> + symbol: Some(var_symbol.to_owned()),
> + message: format!("duplicate imply of {:?}", imp),
> + arch: arch.to_owned(),
> + });
> + }
> +
> + // previous conditionals with same symbol are dead
> + for (sym, _) in &conditional {
> + if sym == &imply_var {
> + findings.push(Finding {
> + severity: Severity::Warning,
> + check: Check::DeadImply,
> + symbol: Some(var_symbol.to_owned()),
> + message: format!("dead imply of {:?}", imp),
> + arch: arch.to_owned(),
> + });
> + }
> + }
> + }
> + }
> + }
> +
> + findings
> +}
> +
> +fn check_duplicate_ranges(arch: &String, var_symbol: &str, info: &AttributeDef) -> Vec<Finding> {
> + let mut findings = Vec::new();
> +
> + // unconditional ranges by bounds
> + let mut unconditional: HashSet<String> = HashSet::new();
> +
> + // (bounds, condition)
> + let mut conditional: HashSet<(String, String)> = HashSet::new();
> +
> + for range in &info.kconfig_ranges {
> + // uniquely identify the range bounds
> + let range_key = format!("{} {}", range.lower_bound, range.upper_bound);
> +
> + match &range.r#if {
> + Some(cond) => {
> + let cond_str = cond.to_string();
> +
> + // duplicate conditional range
> + if !conditional.insert((range_key.clone(), cond_str.clone())) {
> + findings.push(Finding {
> + severity: Severity::Warning,
> + check: Check::DuplicateRange,
> + symbol: Some(var_symbol.to_owned()),
> + message: format!("duplicate range {:?} with condition {}", range, cond_str),
> + arch: arch.to_owned(),
> + });
> + }
> +
> + // conditional range is dead if unconditional exists
> + if unconditional.contains(&range_key) {
> + findings.push(Finding {
> + severity: Severity::Warning,
> + check: Check::DeadRange,
> + symbol: Some(var_symbol.to_owned()),
> + message: format!("dead range of {:?}", range),
> + arch: arch.to_owned(),
> + });
> + }
> + }
> +
> + None => {
> + // duplicate unconditional range
> + if !unconditional.insert(range_key.clone()) {
> + findings.push(Finding {
> + severity: Severity::Warning,
> + check: Check::DeadRange,
> + symbol: Some(var_symbol.to_owned()),
> + message: format!("duplicate range {:?}", range),
> + arch: arch.to_owned(),
> + });
> + }
> +
> + // previous conditionals with same bounds are dead
> + for (bounds, _) in &conditional {
> + if bounds == &range_key {
> + findings.push(Finding {
> + severity: Severity::Warning,
> + check: Check::DeadRange,
> + symbol: Some(var_symbol.to_owned()),
> + message: format!("dead range of {:?}", range),
> + arch: arch.to_owned(),
> + });
> + }
> + }
> + }
> + }
> + }
> +
> + findings
> +}
> +
> +fn check_duplicate_selects(arch: &String, var_symbol: &str, info: &AttributeDef) -> Vec<Finding> {
> + let mut findings = Vec::new();
> +
> + // symbols selected unconditionally
> + let mut unconditional: HashSet<String> = HashSet::new();
> +
> + // (symbol, condition)
> + let mut conditional: HashSet<(String, String)> = HashSet::new();
> +
> + for select in &info.selects {
> + let select_var = select.0.clone();
> +
> + match &select.1 {
> + Some(cond) => {
> + let cond_str = cond.to_string();
> +
> + // duplicate conditional select
> + if !conditional.insert((select_var.clone(), cond_str.clone())) {
> + findings.push(Finding {
> + severity: Severity::Warning,
> + check: Check::DuplicateSelect,
> + symbol: Some(var_symbol.to_owned()),
> + message: format!(
> + "duplicate select of {:?} with condition {}",
> + select.0, cond_str
> + ),
> + arch: arch.to_owned(),
> + });
> + }
> +
> + // conditional is dead if unconditional exists
> + if unconditional.contains(&select_var) {
> + findings.push(Finding {
> + severity: Severity::Warning,
> + check: Check::DeadSelect,
> + symbol: Some(var_symbol.to_owned()),
> + message: format!("dead select of {:?}", select.0),
> + arch: arch.to_owned(),
> + });
> + }
> + }
> +
> + None => {
> + // duplicate unconditional select
> + if !unconditional.insert(select_var.clone()) {
> + findings.push(Finding {
> + severity: Severity::Warning,
> + check: Check::DuplicateSelect,
> + symbol: Some(var_symbol.to_owned()),
> + message: format!("duplicate select of {:?}", select.0),
> + arch: arch.to_owned(),
> + });
> + }
> +
> + // any previous conditional selects are now dead too
> + for (sym, _) in &conditional {
> + if sym == &select_var {
> + findings.push(Finding {
> + severity: Severity::Warning,
> + check: Check::DeadSelect,
> + symbol: Some(var_symbol.to_owned()),
> + message: format!("dead select of {:?}", select.0),
> + arch: arch.to_owned(),
> + });
> + }
> + }
> + }
> + }
> + }
> +
> + findings
> +}
> +
> +#[allow(clippy::collapsible_if)]
> +fn check_defaults(
> + arch: &String,
> + var_symbol: &str,
> + info: &AttributeDef,
> + args: &AnalysisArgs,
> +) -> Vec<Finding> {
> + let mut findings = Vec::new();
> + let mut seen_conditions = HashSet::new();
> + let mut seen_values = HashSet::new();
> + let mut already_unconditional = false;
> +
> + for default in &info.kconfig_defaults {
> + let val_str = default.expression.to_string();
> +
> + let has_real_condition = match &default.r#if {
> + Some(cond) => {
> + let cond_str = cond.to_string();
> + !cond_str.is_empty()
> + }
> + None => false,
> + };
> +
> + let is_value_dup = if has_real_condition {
> + is_duplicate(&mut seen_values, val_str.clone())
> + } else {
> + false
> + };
> +
> + if already_unconditional && args.is_enabled(Check::DeadDefault) {
> + findings.push(Finding {
> + severity: Severity::Warning,
> + check: Check::DeadDefault,
> + symbol: Some(var_symbol.to_owned()),
> + message: format!("dead default of {}", val_str),
> + arch: arch.to_owned(),
> + });
> + }
> +
> + if args.is_enabled(Check::DuplicateDefaultValue) {
> + if default.r#if.is_some() && is_value_dup {
> + findings.push(Finding {
> + severity: Severity::Style,
> + check: Check::DuplicateDefaultValue,
> + symbol: Some(var_symbol.to_owned()),
> + message: format!(
> + "duplicate default value of {}; consider combining the conditions with a logical-or: ||",
> + val_str
> + ),
> + arch: arch.to_owned(),
> + });
> + }
> + }
> +
> + match &default.r#if {
> + Some(cond) => {
> + if is_duplicate(&mut seen_conditions, cond.to_string()) {
> + if is_value_dup {
> + if args.is_enabled(Check::DuplicateDefault) {
> + findings.push(Finding {
> + severity: Severity::Warning,
> + check: Check::DuplicateDefault,
> + symbol: Some(var_symbol.to_owned()),
> + message: format!("duplicate default condition of {:?}", cond),
> + arch: arch.to_owned(),
> + });
> + }
> + } else {
> + if args.is_enabled(Check::DeadDefault) {
> + findings.push(Finding {
> + severity: Severity::Warning,
> + check: Check::DeadDefault,
> + symbol: Some(var_symbol.to_owned()),
> + message: format!("dead default of {}", val_str),
> + arch: arch.to_owned(),
> + });
> + }
> + }
> + }
> + }
> + None => {
> + already_unconditional = true;
> + }
> + }
> + }
> +
> + findings
> +}
> diff --git a/scripts/kconfirm/kconfirm-lib/src/curl_ffi.rs b/scripts/kconfirm/kconfirm-lib/src/curl_ffi.rs
> new file mode 100644
> index 000000000000..d458010cc3f1
> --- /dev/null
> +++ b/scripts/kconfirm/kconfirm-lib/src/curl_ffi.rs
> @@ -0,0 +1,182 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +use core::ffi::c_void;
> +use std::ffi::CStr;
> +use std::ffi::CString;
> +use std::os::raw::c_char;
> +use std::os::raw::c_int;
> +use std::os::raw::c_long;
> +use std::sync::OnceLock;
> +
> +static CURL_INIT: OnceLock<()> = OnceLock::new();
> +
> +#[repr(C)]
> +pub struct CURL {
> + _private: [u8; 0],
> +}
> +
> +type CURLcode = c_int;
> +type CURLoption = u32;
> +type CURLINFO = u32;
> +
> +const CURLE_OK: CURLcode = 0;
> +
> +const CURL_GLOBAL_DEFAULT: c_long = 3;
> +
> +const CURLOPT_URL: CURLoption = 10002;
> +const CURLOPT_NOBODY: CURLoption = 44;
> +const CURLOPT_TIMEOUT: CURLoption = 13;
> +const CURLOPT_FOLLOWLOCATION: CURLoption = 52;
> +const CURLOPT_USERAGENT: CURLoption = 10018;
> +const CURLOPT_HEADERFUNCTION: CURLoption = 20079;
> +const CURLOPT_HEADERDATA: CURLoption = 10029;
> +
> +const CURLINFO_RESPONSE_CODE: CURLINFO = 0x200002;
> +
> +#[link(name = "curl")]
> +unsafe extern "C" {}
> +
> +unsafe extern "C" {
> + fn curl_global_init(flags: c_long) -> CURLcode;
> +
> + fn curl_easy_init() -> *mut CURL;
> +
> + fn curl_easy_cleanup(handle: *mut CURL);
> +
> + fn curl_easy_perform(handle: *mut CURL) -> CURLcode;
> +
> + fn curl_easy_strerror(code: CURLcode) -> *const c_char;
> +
> + fn curl_easy_setopt(handle: *mut CURL, option: CURLoption, ...) -> CURLcode;
> +
> + fn curl_easy_getinfo(handle: *mut CURL, info: CURLINFO, ...) -> CURLcode;
> +}
> +
> +fn init_curl() {
> + CURL_INIT.get_or_init(|| unsafe {
> + curl_global_init(CURL_GLOBAL_DEFAULT);
> + });
> +}
> +
> +fn curl_error(code: CURLcode) -> String {
> + unsafe {
> + let ptr = curl_easy_strerror(code);
> +
> + if ptr.is_null() {
> + return format!("curl error {}", code);
> + }
> +
> + CStr::from_ptr(ptr).to_string_lossy().into_owned()
> + }
> +}
> +
> +struct HeaderCapture {
> + location: Option<String>,
> +}
> +
> +extern "C" fn header_callback(
> + buffer: *mut c_char,
> + size: usize,
> + nitems: usize,
> + userdata: *mut c_void,
> +) -> usize {
> + let total = size * nitems;
> +
> + unsafe {
> + let bytes = std::slice::from_raw_parts(buffer as *const u8, total);
> +
> + if let Ok(header) = std::str::from_utf8(bytes) {
> + let lower = header.to_ascii_lowercase();
> +
> + if lower.starts_with("location:") {
> + if let Some((_, value)) = header.split_once(':') {
> + let capture = &mut *(userdata as *mut HeaderCapture);
> +
> + capture.location = Some(value.trim().to_string());
> + }
> + }
> + }
> + }
> +
> + total
> +}
> +
> +#[derive(Debug)]
> +pub struct HttpResponse {
> + pub response_code: u16,
> + pub location: Option<String>,
> +}
> +
> +pub fn head_request(url: &str) -> Result<HttpResponse, String> {
> + init_curl();
> +
> + unsafe {
> + let curl = curl_easy_init();
> +
> + if curl.is_null() {
> + return Err("curl_easy_init failed".into());
> + }
> +
> + let url_c = match CString::new(url) {
> + Ok(v) => v,
> + Err(_) => {
> + curl_easy_cleanup(curl);
> +
> + return Err("invalid URL".into());
> + }
> + };
> +
> + let ua_c = CString::new("link-checker/1.0").unwrap();
> +
> + let mut headers = HeaderCapture { location: None };
> +
> + macro_rules! setopt {
> + ($opt:expr, $val:expr) => {{
> + let rc = curl_easy_setopt(curl, $opt, $val);
> +
> + if rc != CURLE_OK {
> + curl_easy_cleanup(curl);
> +
> + return Err(curl_error(rc));
> + }
> + }};
> + }
> +
> + setopt!(CURLOPT_URL, url_c.as_ptr());
> + setopt!(CURLOPT_NOBODY, 1 as c_long);
> + setopt!(CURLOPT_TIMEOUT, 10 as c_long);
> + setopt!(CURLOPT_FOLLOWLOCATION, 0 as c_long);
> + setopt!(CURLOPT_USERAGENT, ua_c.as_ptr());
> +
> + setopt!(
> + CURLOPT_HEADERFUNCTION,
> + header_callback as extern "C" fn(_, _, _, _) -> _
> + );
> +
> + setopt!(CURLOPT_HEADERDATA, &mut headers as *mut _ as *mut c_void);
> +
> + let rc = curl_easy_perform(curl);
> +
> + if rc != CURLE_OK {
> + curl_easy_cleanup(curl);
> +
> + return Err(curl_error(rc));
> + }
> +
> + let mut response_code: c_long = 0;
> +
> + let rc = curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &mut response_code);
> +
> + if rc != CURLE_OK {
> + curl_easy_cleanup(curl);
> +
> + return Err(curl_error(rc));
> + }
> +
> + curl_easy_cleanup(curl);
> +
> + Ok(HttpResponse {
> + response_code: response_code as u16,
> + location: headers.location,
> + })
> + }
> +}
> diff --git a/scripts/kconfirm/kconfirm-lib/src/dead_links.rs b/scripts/kconfirm/kconfirm-lib/src/dead_links.rs
> new file mode 100644
> index 000000000000..47bbd5c09114
> --- /dev/null
> +++ b/scripts/kconfirm/kconfirm-lib/src/dead_links.rs
> @@ -0,0 +1,138 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +use crate::curl_ffi::head_request;
> +use std::collections::HashSet;
> +
> +#[derive(PartialEq, Debug)]
> +pub enum LinkStatus {
> + Ok,
> + ProbablyBlocked,
> + Redirected(String),
> + NotFound,
> + ServerError,
> + Unreachable(String),
> + UnsupportedScheme(String),
> +}
> +
> +pub fn check_link(url: &str) -> LinkStatus {
> + if let Some(scheme) = url.split("://").next() {
> + match scheme {
> + "http" | "https" => return check_http(url),
> +
> + "git" | "ftp" => {
> + return LinkStatus::UnsupportedScheme(scheme.into());
> + }
> +
> + _ => {
> + return LinkStatus::UnsupportedScheme(scheme.into());
> + }
> + }
> + }
> +
> + LinkStatus::Unreachable("invalid URL".into())
> +}
> +
> +fn check_http(url: &str) -> LinkStatus {
> + let response = match head_request(url) {
> + Ok(r) => r,
> + Err(e) => return LinkStatus::Unreachable(e),
> + };
> +
> + match response.response_code {
> + 200..=299 => LinkStatus::Ok,
> +
> + 301 | 302 => LinkStatus::Redirected(response.location.unwrap_or_else(|| "unknown".into())),
> +
> + 403 | 429 => LinkStatus::ProbablyBlocked,
> +
> + 404 => LinkStatus::NotFound,
> +
> + 500..=599 => LinkStatus::ServerError,
> +
> + _ => LinkStatus::ProbablyBlocked,
> + }
> +}
> +
> +pub fn find_links(text: &str) -> Vec<String> {
> + fn is_scheme_char(c: u8) -> bool {
> + c.is_ascii_alphanumeric() || matches!(c, b'+' | b'-' | b'.')
> + }
> +
> + fn is_url_terminator(c: u8) -> bool {
> + c.is_ascii_whitespace()
> + || matches!(
> + c,
> + b'"' | b'\'' | b'<' | b'>' | b'(' | b')' | b'[' | b']' | b'{' | b'}'
> + )
> + }
> +
> + let bytes = text.as_bytes();
> +
> + let mut links = Vec::new();
> + let mut seen = HashSet::new();
> +
> + let mut i = 0;
> +
> + while i + 3 < bytes.len() {
> + if bytes[i] == b':' && bytes[i + 1] == b'/' && bytes[i + 2] == b'/' {
> + // walk backward to find scheme start
> + let mut start = i;
> +
> + while start > 0 && is_scheme_char(bytes[start - 1]) {
> + start -= 1;
> + }
> +
> + // require non-empty scheme
> + if start == i {
> + i += 3;
> + continue;
> + }
> +
> + // first char must be alphabetic
> + if !bytes[start].is_ascii_alphabetic() {
> + i += 3;
> + continue;
> + }
> +
> + // walk forward to url end
> + let mut end = i + 3;
> +
> + while end < bytes.len() && !is_url_terminator(bytes[end]) {
> + end += 1;
> + }
> +
> + let mut url = &text[start..end];
> +
> + // trim trailing punctuation
> + url = url.trim_end_matches(&['.', ',', ';', ':', '!', '?'][..]);
> +
> + // trim unmatched markdown
> + while let Some(last) = url.chars().last() {
> + let trim = match last {
> + ')' => url.matches('(').count() < url.matches(')').count(),
> +
> + ']' => url.matches('[').count() < url.matches(']').count(),
> +
> + '}' => url.matches('{').count() < url.matches('}').count(),
> +
> + _ => false,
> + };
> +
> + if trim {
> + url = &url[..url.len() - last.len_utf8()];
> + } else {
> + break;
> + }
> + }
> +
> + if seen.insert(url) {
> + links.push(url.to_string());
> + }
> +
> + i = end;
> + } else {
> + i += 1;
> + }
> + }
> +
> + links
> +}
> diff --git a/scripts/kconfirm/kconfirm-lib/src/lib.rs b/scripts/kconfirm/kconfirm-lib/src/lib.rs
> new file mode 100644
> index 000000000000..6be0199f0785
> --- /dev/null
> +++ b/scripts/kconfirm/kconfirm-lib/src/lib.rs
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +use analyze::analyze;
> +pub use checks::AnalysisArgs;
> +pub use checks::Check;
> +pub use checks::check_select_visible;
> +pub use checks::check_variable_info;
> +use nom_kconfig::Entry;
> +use nom_kconfig::KconfigInput;
> +use nom_kconfig::parse_kconfig;
> +use output::*;
> +use symbol_table::*;
> +mod analyze;
> +mod checks;
> +mod curl_ffi;
> +mod dead_links;
> +pub mod output;
> +pub mod symbol_table;
> +
> +pub fn check_kconfig(
> + args: AnalysisArgs,
> + kconfig_files: Vec<(String, KconfigInput)>,
> +) -> Vec<Finding> {
> + let mut findings = Vec::new();
> + let mut symbol_table = SymbolTable::new();
> +
> + for (arch_config_option, kconfig_file) in kconfig_files {
> + match parse_kconfig(kconfig_file) {
> + Ok(parsed) => {
> + let entries: Vec<Entry> = parsed.1.entries;
> + findings.extend(analyze(
> + &args,
> + &mut symbol_table,
> + arch_config_option,
> + entries,
> + ));
> + }
> + Err(e) => {
> + findings.push(Finding {
> + severity: Severity::Fatal,
> + check: Check::FailedParse,
> + symbol: None,
> + message: format!("Failed to parse kconfig, error is: {}", e),
> + arch: arch_config_option,
> + });
> + }
> + }
> + }
> +
> + for (var_symbol, type_info) in &symbol_table.raw {
> + for (arch_specific, redefinitions) in &type_info.attribute_defs {
> + for (_definition_condition, info) in redefinitions {
> + findings.extend(check_variable_info(&args, var_symbol, arch_specific, info));
> + }
> + }
> +
> + if args.is_enabled(Check::SelectVisible) {
> + findings.extend(check_select_visible(var_symbol, type_info));
> + }
> + }
> +
> + findings
> +}
> diff --git a/scripts/kconfirm/kconfirm-lib/src/output.rs b/scripts/kconfirm/kconfirm-lib/src/output.rs
> new file mode 100644
> index 000000000000..e0d8bf8342d5
> --- /dev/null
> +++ b/scripts/kconfirm/kconfirm-lib/src/output.rs
> @@ -0,0 +1,111 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +use crate::Check;
> +use std::fmt;
> +
> +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)]
> +pub enum Severity {
> + Fatal,
> + Error, // will be used for known bugs, e.g. unmet dependencies
> + Warning,
> + Style,
> +}
> +
> +#[derive(Debug)]
> +pub struct Finding {
> + pub severity: Severity,
> + pub check: Check,
> + pub symbol: Option<String>,
> + pub message: String,
> + pub arch: String,
> +}
> +
> +impl Finding {
> + fn fmt_with_arches(&self, f: &mut fmt::Formatter, arches: &[&str]) -> fmt::Result {
> + let arch_part = if arches.is_empty() {
> + String::new()
> + } else {
> + format!(" [{}]", arches.join(", "))
> + };
> +
> + match &self.symbol {
> + Some(s) => write!(
> + f,
> + "{} [{}]{} config {}: {}",
> + self.severity,
> + self.check.as_str(),
> + arch_part,
> + s,
> + self.message
> + ),
> + None => write!(
> + f,
> + "{} [{}]{} {}",
> + self.severity,
> + self.check.as_str(),
> + arch_part,
> + self.message
> + ),
> + }
> + }
> +}
> +
> +impl fmt::Display for Finding {
> + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
> + self.fmt_with_arches(f, &[])
> + }
> +}
> +
> +pub fn print_findings(mut findings: Vec<Finding>) {
> + findings.sort_by(|a, b| {
> + (
> + &a.severity,
> + a.check.as_str(),
> + &a.symbol,
> + &a.message,
> + &a.arch,
> + )
> + .cmp(&(
> + &b.severity,
> + b.check.as_str(),
> + &b.symbol,
> + &b.message,
> + &b.arch,
> + ))
> + });
> +
> + for group in findings.chunk_by(|a, b| {
> + a.severity == b.severity
> + && a.check.as_str() == b.check.as_str()
> + && a.symbol == b.symbol
> + && a.message == b.message
> + }) {
> + let head = &group[0];
> +
> + let mut arches: Vec<&str> = Vec::new();
> + for f in group {
> + if arches.last() != Some(&f.arch.as_str()) {
> + arches.push(&f.arch);
> + }
> + }
> +
> + // Use a small wrapper so we can call our custom formatter via println!
> + struct Wrap<'a>(&'a Finding, &'a [&'a str]);
> + impl fmt::Display for Wrap<'_> {
> + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
> + self.0.fmt_with_arches(f, self.1)
> + }
> + }
> + println!("{}", Wrap(head, &arches));
> + }
> +}
> +
> +impl fmt::Display for Severity {
> + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
> + match self {
> + Severity::Fatal => write!(f, "FATAL "),
> + Severity::Error => write!(f, "ERROR "),
> + Severity::Warning => write!(f, "WARNING"),
> + Severity::Style => write!(f, "STYLE "),
> + }
> + }
> +}
> diff --git a/scripts/kconfirm/kconfirm-lib/src/symbol_table.rs b/scripts/kconfirm/kconfirm-lib/src/symbol_table.rs
> new file mode 100644
> index 000000000000..48abb46c1945
> --- /dev/null
> +++ b/scripts/kconfirm/kconfirm-lib/src/symbol_table.rs
> @@ -0,0 +1,223 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +use nom_kconfig::attribute::DefaultAttribute;
> +use nom_kconfig::attribute::Expression;
> +use nom_kconfig::attribute::OrExpression;
> +use nom_kconfig::attribute::Range;
> +use nom_kconfig::attribute::r#type::Type;
> +use std::collections::HashMap;
> +use std::collections::hash_map;
> +
> +type KconfigSymbol = String;
> +type Arch = String;
> +type Cond = Option<Expression>;
> +
> +// NOTE: we cannot add these elements to the solver until we've processed all variables,
> +// because we need to know all of the selectors.
> +#[derive(Debug, Clone)]
> +pub struct TypeInfo {
> + pub kconfig_type: Option<Type>, // 'None' when we don't know the type (e.g. if it's a dangling reference)
> +
> + // maps the selector to an (ARCH, select_cond)
> + // - if the ARCH is None, then it's not arch-specific
> + // if the select_cond is None, then it's unconditional
> + pub selected_by: HashMap<KconfigSymbol, Vec<(Arch, Cond)>>, // .0 only selects it when .1 is true.
> +
> + // there is one of these per entry (each entry expected to have a different definedness condition)
> + // maps architecture option name (or none if not arch-specific) to:
> + // [([condition], config definition)]
> + // - NOTE: there can be multiple partial definitions under the same condition, or mutually-exclusive conditions, or a subset condition.
> + pub attribute_defs: HashMap<Arch, Vec<(Vec<Expression>, AttributeDef)>>, // the innermost `Vec<Expression>` represents each nested condition that was reached (we will eventually need to AND them all)
> +}
> +
> +// everything is a vector because we may encounter multiple over time,
> +// so we won't know until the end what the condition is.
> +#[derive(Debug, Clone)]
> +pub struct AttributeDef {
> + pub kconfig_dependencies: Vec<OrExpression>,
> + pub kconfig_ranges: Vec<Range>,
> + pub kconfig_defaults: Vec<DefaultAttribute>,
> + pub visibility: Vec<Option<OrExpression>>,
> + pub selects: Vec<(KconfigSymbol, Cond)>,
> + pub implies: Vec<(KconfigSymbol, Cond)>,
> +}
> +
> +impl TypeInfo {
> + fn new_empty() -> Self {
> + Self {
> + kconfig_type: None,
> + selected_by: HashMap::new(),
> + attribute_defs: HashMap::new(),
> + }
> + }
> +
> + // TODO: we should consider having separate functions for:
> + // 1. merge-inserting a redef of attributes (NOTE: the type definition is actually part of the redef, but we aren't handling type-redefinitions for now)
> + // 2. selectors
> + fn insert(
> + &mut self,
> + kconfig_type: Option<Type>,
> + raw_constraints: Vec<OrExpression>,
> + kconfig_ranges: Vec<Range>,
> + kconfig_defaults: Vec<DefaultAttribute>,
> + visibility: Vec<Option<OrExpression>>,
> + arch: String,
> + definition_condition: Vec<OrExpression>,
> + selected_by: Option<(KconfigSymbol, Cond)>,
> + selects: Vec<(KconfigSymbol, Cond)>,
> + implies: Vec<(KconfigSymbol, Cond)>,
> + ) {
> + // type merge
> + match (&self.kconfig_type, &kconfig_type) {
> + (None, Some(_)) => self.kconfig_type = kconfig_type.clone(),
> + (Some(_), Some(new)) if Some(new) != self.kconfig_type.as_ref() => {
> + // TODO: not doing anything with redefined types yet.
> + // later, we will want to consider e.g. bool/def_bool the same type (and possibly int/hex?) but not bool/tristate, so we need to build out typechecking.
> + }
> + _ => {}
> + }
> +
> + // selected_by merge
> + if let Some(sb) = selected_by {
> + merge_selected_by(&mut self.selected_by, arch.clone(), sb);
> + }
> +
> + // variable_info merge:
> + // we only want to add an attribute redefinition if the things in the attribute def aren't empty
> + // (the visibility is just additional info to capture)
> + if (&kconfig_type).is_some() // we need to ensure that we have an empty definition here if the config option had a type definition
> + || !raw_constraints.is_empty()
> + || !kconfig_ranges.is_empty()
> + || !kconfig_defaults.is_empty()
> + || !selects.is_empty()
> + || !implies.is_empty()
> + {
> + insert_variable_info(
> + &mut self.attribute_defs,
> + arch,
> + definition_condition,
> + AttributeDef {
> + kconfig_dependencies: raw_constraints,
> + kconfig_ranges,
> + kconfig_defaults,
> + visibility,
> + selects,
> + implies,
> + },
> + );
> + }
> + }
> +}
> +
> +// the visibility and the dependencies will each need to be AND'd (separately)
> +// the defaults should each be handled separately.
> +pub struct ChoiceData {
> + //pub inner_vars: Vec<String>,
> + pub arch: Arch,
> + pub visibility: Cond,
> + pub dependencies: Vec<OrExpression>, // this is the menu's dependencies (and inherited dependencies from the file)
> + pub defaults: Vec<DefaultAttribute>, // these are each of the conditional defaults for the choice
> +}
> +
> +// NOTE: it might be better if TypeInfo is an enum with a single value,
> +// e.g. Unsolved(kconfig_raw) and Solved(z3_ast)
> +pub struct SymbolTable {
> + pub raw: HashMap<KconfigSymbol, TypeInfo>,
> + pub choices: Vec<ChoiceData>,
> + pub modules_option: Option<KconfigSymbol>, // None until we find the modules attribute in exactly 1 config option
> +}
> +
> +impl SymbolTable {
> + pub fn new() -> Self {
> + SymbolTable {
> + raw: HashMap::new(),
> + choices: Vec::new(),
> + modules_option: None,
> + }
> + }
> +
> + pub fn from_parts(
> + raw: HashMap<KconfigSymbol, TypeInfo>,
> + choices: Vec<ChoiceData>,
> + modules_option: Option<KconfigSymbol>,
> + ) -> Self {
> + SymbolTable {
> + raw,
> + choices,
> + modules_option,
> + }
> + }
> +
> + pub fn merge_insert_new_solved(
> + &mut self,
> + var: KconfigSymbol,
> + kconfig_type: Option<Type>,
> + raw_constraints: Vec<OrExpression>,
> + kconfig_ranges: Vec<Range>,
> + kconfig_defaults: Vec<DefaultAttribute>,
> + visibility: Vec<Option<OrExpression>>,
> + arch: Arch,
> + definition_condition: Vec<OrExpression>,
> + selected_by: Option<(KconfigSymbol, Cond)>,
> + selects: Vec<(KconfigSymbol, Cond)>,
> + implies: Vec<(KconfigSymbol, Cond)>,
> + ) {
> + let entry = self.raw.entry(var.clone());
> +
> + match entry {
> + hash_map::Entry::Vacant(v) => {
> + let mut t = TypeInfo::new_empty();
> + t.insert(
> + kconfig_type,
> + raw_constraints,
> + kconfig_ranges,
> + kconfig_defaults,
> + visibility,
> + arch,
> + definition_condition,
> + selected_by,
> + selects,
> + implies,
> + );
> + v.insert(t);
> + }
> +
> + hash_map::Entry::Occupied(mut o) => {
> + let t = o.get_mut();
> +
> + t.insert(
> + kconfig_type,
> + raw_constraints,
> + kconfig_ranges,
> + kconfig_defaults,
> + visibility,
> + arch,
> + definition_condition,
> + selected_by,
> + selects,
> + implies,
> + );
> + }
> + }
> + }
> +}
> +
> +fn merge_selected_by(
> + map: &mut HashMap<String, Vec<(Arch, Cond)>>,
> + arch: Arch,
> + selected_by: (KconfigSymbol, Cond),
> +) {
> + map.entry(selected_by.0)
> + .or_default() // empty vec
> + .push((arch, selected_by.1));
> +}
> +
> +fn insert_variable_info(
> + map: &mut HashMap<Arch, Vec<(Vec<Expression>, AttributeDef)>>,
> + arch: Arch,
> + definition_condition: Vec<Expression>,
> + info: AttributeDef,
> +) {
> + map.entry(arch)
> + .or_default() // empty vec
> + .push((definition_condition, info));
> +}
> diff --git a/scripts/kconfirm/kconfirm-linux/Cargo.toml b/scripts/kconfirm/kconfirm-linux/Cargo.toml
> new file mode 100644
> index 000000000000..9516399e1dae
> --- /dev/null
> +++ b/scripts/kconfirm/kconfirm-linux/Cargo.toml
> @@ -0,0 +1,10 @@
> +# SPDX-License-Identifier: GPL-2.0
> +[package]
> +name = "kconfirm-linux"
> +version = "0.10.0"
> +edition = "2024"
> +rust-version.workspace = true
> +
> +[dependencies]
> +kconfirm-lib = { path = "../kconfirm-lib" }
> +nom-kconfig = { workspace = true }
> diff --git a/scripts/kconfirm/kconfirm-linux/src/getopt_ffi.rs b/scripts/kconfirm/kconfirm-linux/src/getopt_ffi.rs
> new file mode 100644
> index 000000000000..227faa17b962
> --- /dev/null
> +++ b/scripts/kconfirm/kconfirm-linux/src/getopt_ffi.rs
> @@ -0,0 +1,99 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +use std::env;
> +use std::ffi::CStr;
> +use std::ffi::CString;
> +use std::os::raw::c_char;
> +use std::os::raw::c_int;
> +use std::ptr;
> +
> +pub const REQUIRED_ARGUMENT: c_int = 1;
> +
> +#[repr(C)]
> +pub struct option {
> + pub name: *const c_char,
> + pub has_arg: c_int,
> + pub flag: *mut c_int,
> + pub val: c_int,
> +}
> +
> +#[link(name = "c")]
> +unsafe extern "C" {
> + fn getopt_long(
> + argc: c_int,
> + argv: *mut *mut c_char,
> + optstring: *const c_char,
> + longopts: *const option,
> + longindex: *mut c_int,
> + ) -> c_int;
> +
> + static mut optarg: *mut c_char;
> + static mut optind: c_int;
> +}
> +
> +pub struct Getopt {
> + _cstrings: Vec<CString>,
> + argv: Vec<*mut c_char>,
> + argc: c_int,
> +}
> +
> +impl Getopt {
> + pub fn new() -> Self {
> + let raw_args: Vec<String> = env::args().collect();
> +
> + let cstrings: Vec<CString> = raw_args
> + .iter()
> + .map(|s| CString::new(s.as_str()).unwrap())
> + .collect();
This panics if the arguments aren't valid UTF-8. Using env::args_os()
might be better. getopt_long() doesn't care if the arguments are
valid UTF-8 or not.
> + let mut argv: Vec<*mut c_char> =
> + cstrings.iter().map(|s| s.as_ptr() as *mut c_char).collect();
> +
> + argv.push(ptr::null_mut());
> +
> + let argc = (argv.len() - 1) as c_int;
> +
> + Self {
> + _cstrings: cstrings,
> + argv,
> + argc,
> + }
> + }
> +
> + pub fn reset(&mut self) {
> + unsafe {
> + optind = 1;
> + }
> + }
> +
> + pub fn next(
> + &mut self,
> + optstring: &CStr,
> + longopts: &[option],
> + ) -> Option<Result<(char, Option<String>), String>> {
> + unsafe {
> + let c = getopt_long(
> + self.argc,
> + self.argv.as_mut_ptr(),
> + optstring.as_ptr(),
> + longopts.as_ptr(),
> + ptr::null_mut(),
> + );
> +
> + if c == -1 {
> + return None;
> + }
> +
> + if c == '?' as c_int {
if c == c_int::from(b'?') {
Might be a bit clearer.
> + return Some(Err("invalid argument".into()));
You can just std::process::exit() here. getopt_long() will have
printed an error message. This assumes that the
> + }
> +
> + let arg = if optarg.is_null() {
> + None
> + } else {
> + Some(CStr::from_ptr(optarg).to_string_lossy().into_owned())
> + };
I'd prefer to return an OsString here.
> + Some(Ok((c as u8 as char, arg)))
> + }
> + }
> +}
I think it is simpler to just inline all of this code into its
single call-site. The safety of the code is obvious in context,
and you can avoid checking for impossible errors. For instance,
since all of the options have required arguments, it really is safe
to dereference optarg without any null check.
Yes, this is C with Rust syntax, but it's *simple* C with Rust syntax.
As an aside, I don't know if you want to accept abbreviated long
options. Most tools using getopt_long() allow them. If you don't,
I can show you how to reject them.
--
Sincerely,
Demi Marie Obenour (she/her/hers)
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] docs: submitting-patches: Clarify that in English "reviewer" is a person
From: Greg Kroah-Hartman @ 2026-05-17 6:13 UTC (permalink / raw)
To: Vlastimil Babka (SUSE)
Cc: Krzysztof Kozlowski, Jonathan Corbet, Shuah Khan, workflows,
linux-doc, linux-kernel, Andrew Morton, David Hildenbrand,
Linus Torvalds, Guenter Roeck
In-Reply-To: <ce1e5e9b-83d0-4971-aee3-dc5a8f85ce22@kernel.org>
On Sat, May 16, 2026 at 04:39:45PM +0200, Vlastimil Babka (SUSE) wrote:
> On 5/16/26 14:38, Krzysztof Kozlowski wrote:
> > Common understanding of word "Reviewer" is: a person performing a review
> > work [1]. Tools are not persons, thus cannot be reviewers in this term.
> > Also tools cannot make statements ("A Reviewed-by tag is a statement of
> > opinion"), since making a statement needs some sort of conscious mind.
> >
> > Our docs already clearly mark that "Reviewed-by" must come from a
> > person:
> >
> > - "By offering my Reviewed-by: tag, I state that:"
> >
> > Usage of first person "I" and word "state"
> >
> > - "A Reviewed-by tag is *a statement of opinion* that the patch is an
> > appropriate modification of the kernel without any remaining serious"
> >
> > Only a person can make a statement of opinion.
> >
> > - "Any interested reviewer (who has done the work) can offer a
> > Reviewed-by"
> >
> > A person can offer a tag thus above does not grant the tool
> > permission to offer a tag.
> >
> > However this is not enough and apparently English is not that precise,
> > so let's clarify that only a person can state the "Reviewer's statement
> > of oversight".
> >
> > Link: https://en.wiktionary.org/wiki/reviewer [1]
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Vlastimil Babka <vbabka@kernel.org>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: David Hildenbrand <david@kernel.org>
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
>
> I agree with the intent that the tag is for people (whether they use a tool
> or not to help them). We also don't put "Tested-by: kernel test robot" or
> syzkaller on every commit that they test and find no bugs. Review is also
> not just about absence of bugs, but agreeing with the larger design and
> whether the change makes sense to do in the first place.
>
> So whether that's achieved with this particular wording or differently,
>
> Acked-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply
* Re: [RFC v3 0/3] add kconfirm
From: Demi Marie Obenour @ 2026-05-17 6:14 UTC (permalink / raw)
To: Julian Braha, nathan, nsc
Cc: jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda,
corbet, qingfang.deng, yann.prono, ej, linux-kernel,
rust-for-linux, linux-doc, linux-kbuild
In-Reply-To: <20260516215354.449807-1-julianbraha@gmail.com>
[-- Attachment #1.1.1: Type: text/plain, Size: 457 bytes --]
On 5/16/26 17:53, Julian Braha wrote:
> Hi all,
>
> kconfirm has shrunk a lot since v2!
Thanks for dropping so many of the dependencies!
It might be able to shrink it further by using the existing C Kconfig
parser. This has the advantage that it ensures kconfirm and Kconfig
will interpret the Kconfig files the same way. I'm not sure if
that would be too much of a change. That's up to you.
--
Sincerely,
Demi Marie Obenour (she/her/hers)
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [RFC PATCH v3 1/3] scripts: add kconfirm
From: Miguel Ojeda @ 2026-05-17 6:28 UTC (permalink / raw)
To: Julian Braha
Cc: nathan, nsc, jani.nikula, akpm, gary, ljs, arnd, gregkh,
masahiroy, ojeda, corbet, qingfang.deng, yann.prono, demiobenour,
ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild
In-Reply-To: <20260516215354.449807-2-julianbraha@gmail.com>
On Sat, May 16, 2026 at 11:54 PM Julian Braha <julianbraha@gmail.com> wrote:
>
> +CARGO = cargo
Question to Kbuild: would it hurt to hardcore `--offline` here?
If someone within Make actually ever needs Cargo to fetch something,
then they should be very explicit about it (in which case we could
have another variable etc.).
> - rust/libpin_init_internal.so rust/libpin_init_internal.dylib
> + rust/libpin_init_internal.so rust/libpin_init_internal.dylib \
Spurious change?
> +$(TARGET):
> + $(CARGO) run --release --offline -p kconfirm-linux -- --linux-path $(srctree) --enable-arch $(SRCARCH) $(KCONFIRM_ARGS)
This probably does not work in `O=` builds or in cases where the
`srctree` is read-only (please see my other reply on the docs patch).
> +// SPDX-License-Identifier: GPL-2.0-only
> +use crate::AnalysisArgs;
This appears to use a quite different, custom Rust style. If this is
going to be developed in-tree, then we should do our best to follow
the guidelines:
https://docs.kernel.org/rust/coding-guidelines.html
For instance, a few key points are not followed here:
- Public items are not documented. When they are, they seem to use
comments instead of actual docs. Markdown is not used either.
- No examples, doctests or tests.
- No `// SAFETY` comments for unsafe code.
> +unsafe extern "C" {
> + fn curl_global_init(flags: c_long) -> CURLcode;
> +
> + fn curl_easy_init() -> *mut CURL;
> +
> + fn curl_easy_cleanup(handle: *mut CURL);
> +
> + fn curl_easy_perform(handle: *mut CURL) -> CURLcode;
> +
> + fn curl_easy_strerror(code: CURLcode) -> *const c_char;
> +
> + fn curl_easy_setopt(handle: *mut CURL, option: CURLoption, ...) -> CURLcode;
> +
> + fn curl_easy_getinfo(handle: *mut CURL, info: CURLINFO, ...) -> CURLcode;
> +}
I like minimizing dependencies, but since we require vendored
dependencies anyway, then it may be simpler to use a common
("standard") Rust dependency for these things. Then we can all agree
on a particular one and use that when the same need arises.
In fact, it seems like FFI here and in the other file is the only
source of `unsafe` code, no? We could perhaps even `forbid` it
otherwise.
If we truly want to minimize dependencies even if we have vendored
ones, then we could call into the `curl` CLI instead, just like we
e.g. call into `bindgen` instead of using its library.
Thanks!
Cheers,
Miguel
^ permalink raw reply
* Re: [PATCH] kbuild: deb-pkg: Allow setting package name at build
From: Nicolas Schier @ 2026-05-17 6:57 UTC (permalink / raw)
To: Mario Limonciello (AMD)
Cc: mario.limonciello, nathan, corbet, skhan, linux-kbuild, linux-doc
In-Reply-To: <20260505152957.695641-1-superm1@kernel.org>
On Tue, May 05, 2026 at 10:29:54AM -0500, Mario Limonciello (AMD) wrote:
> Users can change the source package by setting variable `KDEB_SOURCENAME`,
> but the binary package name is hardcoded.
>
> Add support for setting binary package name by using `KDEB_PACKAGENAME`
> and let it affect both kernel image and debug image packages.
>
> Update kbuild documentation to include defaults and mention both
> `KDEB_PACKAGENAME` and `KDEB_SOURCENAME`.
>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> Documentation/kbuild/kbuild.rst | 10 ++++++++++
> scripts/package/mkdebian | 6 ++++--
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst
> index 5a9013bacfb75..cbdd2224c3a55 100644
> --- a/Documentation/kbuild/kbuild.rst
> +++ b/Documentation/kbuild/kbuild.rst
> @@ -177,6 +177,16 @@ the UTS_MACHINE variable, and on some architectures also the kernel config.
> The value of KBUILD_DEBARCH is assumed (not checked) to be a valid Debian
> architecture.
>
> +KDEB_SOURCENAME
> +----------------
> +For the deb-pkg target, allows overriding the default source package name.
> +The default package name is "linux-upstream".
> +
> +KDEB_PACKAGENAME
> +----------------
> +For the deb-pkg target, allows overriding the default binary package name.
> +The default package name is "linux-image".
Do you see a real need for that? (We should prevent adding
complexity when it's not neccessary.)
> +
> KDOCFLAGS
> ---------
> Specify extra (warning/error) flags for kernel-doc checks during the build,
> diff --git a/scripts/package/mkdebian b/scripts/package/mkdebian
> index d4b007b38a475..cbe4266fac732 100755
> --- a/scripts/package/mkdebian
> +++ b/scripts/package/mkdebian
> @@ -166,7 +166,9 @@ else
> fi
> sourcename=${KDEB_SOURCENAME:-linux-upstream}
>
> -if [ "$ARCH" = "um" ] ; then
> +if [ "${KDEB_PACKAGENAME:+set}" ]; then
> + packagename=$KDEB_PACKAGENAME
> +elif [ "$ARCH" = "um" ]; then
> packagename=user-mode-linux
> else
> packagename=linux-image
> @@ -252,7 +254,7 @@ fi
> if is_enabled CONFIG_DEBUG_INFO; then
> cat <<EOF >> debian/control
>
> -Package: linux-image-${KERNELRELEASE}-dbg
> +Package: $packagename-${KERNELRELEASE}-dbg
> Section: debug
> Architecture: $debarch
> Build-Profiles: <!pkg.${sourcename}.nokerneldbg>
> --
> 2.53.0
>
Did you test that patch?
These changes are not enough, as scripts/package/builddeb does not pick
up the non-standard binary package name in its final 'case', and thus,
_no_ binary package will be generated if KDEB_PACKAGENAME !=
"linux-image".
Kind regards,
Nicolas
^ permalink raw reply
* Re: [RFC PATCH v3 1/3] scripts: add kconfirm
From: Demi Marie Obenour @ 2026-05-17 7:32 UTC (permalink / raw)
To: Miguel Ojeda, Julian Braha
Cc: nathan, nsc, jani.nikula, akpm, gary, ljs, arnd, gregkh,
masahiroy, ojeda, corbet, qingfang.deng, yann.prono, ej,
linux-kernel, rust-for-linux, linux-doc, linux-kbuild
In-Reply-To: <CANiq72kr=tzvEitYj6xyT=jGnKQZK1dmekSU3us7MWGTrv0FNA@mail.gmail.com>
[-- Attachment #1.1.1: Type: text/plain, Size: 3067 bytes --]
On 5/17/26 02:28, Miguel Ojeda wrote:
> On Sat, May 16, 2026 at 11:54 PM Julian Braha <julianbraha@gmail.com> wrote:
>>
>> +CARGO = cargo
>
> Question to Kbuild: would it hurt to hardcore `--offline` here?
>
> If someone within Make actually ever needs Cargo to fetch something,
> then they should be very explicit about it (in which case we could
> have another variable etc.).
>
>> - rust/libpin_init_internal.so rust/libpin_init_internal.dylib
>> + rust/libpin_init_internal.so rust/libpin_init_internal.dylib \
>
> Spurious change?
>
>> +$(TARGET):
>> + $(CARGO) run --release --offline -p kconfirm-linux -- --linux-path $(srctree) --enable-arch $(SRCARCH) $(KCONFIRM_ARGS)
>
> This probably does not work in `O=` builds or in cases where the
> `srctree` is read-only (please see my other reply on the docs patch).
>
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +use crate::AnalysisArgs;
>
> This appears to use a quite different, custom Rust style. If this is
> going to be developed in-tree, then we should do our best to follow
> the guidelines:
>
> https://docs.kernel.org/rust/coding-guidelines.html
>
> For instance, a few key points are not followed here:
>
> - Public items are not documented. When they are, they seem to use
> comments instead of actual docs. Markdown is not used either.
>
> - No examples, doctests or tests.
>
> - No `// SAFETY` comments for unsafe code.
>
>> +unsafe extern "C" {
>> + fn curl_global_init(flags: c_long) -> CURLcode;
>> +
>> + fn curl_easy_init() -> *mut CURL;
>> +
>> + fn curl_easy_cleanup(handle: *mut CURL);
>> +
>> + fn curl_easy_perform(handle: *mut CURL) -> CURLcode;
>> +
>> + fn curl_easy_strerror(code: CURLcode) -> *const c_char;
>> +
>> + fn curl_easy_setopt(handle: *mut CURL, option: CURLoption, ...) -> CURLcode;
>> +
>> + fn curl_easy_getinfo(handle: *mut CURL, info: CURLINFO, ...) -> CURLcode;
>> +}
>
> I like minimizing dependencies, but since we require vendored
> dependencies anyway, then it may be simpler to use a common
> ("standard") Rust dependency for these things. Then we can all agree
> on a particular one and use that when the same need arises.
Using a ton of vendored dependencies would make for unreviewable
patches. It's also the kind of thing that gives distro packagers
headaches.
> In fact, it seems like FFI here and in the other file is the only
> source of `unsafe` code, no? We could perhaps even `forbid` it
> otherwise.
I don't want to pull in a huge library like clap. A simple getopt_long
implementation could also be an option.
> If we truly want to minimize dependencies even if we have vendored
> ones, then we could call into the `curl` CLI instead, just like we
> e.g. call into `bindgen` instead of using its library.
I'm the one who suggested using FFI here and for command-line parsing.
The command-line interface would also work.
--
Sincerely,
Demi Marie Obenour (she/her/hers)
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox