* [PATCH v4 1/2] rust: core abstractions for HID drivers
From: Rahul Rameshbabu @ 2025-09-17 22:53 UTC (permalink / raw)
To: linux-input, linux-kernel, rust-for-linux
Cc: a.hindborg, alex.gaynor, aliceryhl, benjamin.tissoires,
benno.lossin, bjorn3_gh, boqun.feng, dakr, db48x, gary, jikos,
ojeda, peter.hutterer, tmgross, Rahul Rameshbabu
In-Reply-To: <20250917225341.4572-1-sergeantsagara@protonmail.com>
These abstractions enable the development of HID drivers in Rust by binding
with the HID core C API. They provide Rust types that map to the
equivalents in C. In this initial draft, only hid_device and hid_device_id
are provided direct Rust type equivalents. hid_driver is specially wrapped
with a custom Driver type. The module_hid_driver! macro provides analogous
functionality to its C equivalent. Only the .report_fixup callback is
binded to Rust so far.
Future work for these abstractions would include more bindings for common
HID-related types, such as hid_field, hid_report_enum, and hid_report as
well as more bus callbacks. Providing Rust equivalents to useful core HID
functions will also be necessary for HID driver development in Rust.
Signed-off-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>
---
Notes:
Changelog:
v3->v4:
* Removed specifying tree in MAINTAINERS file since that is up for
debate
* Minor rebase cleanup
* Moved driver logic under drivers/hid/rust
v2->v3:
* Implemented AlwaysRefCounted trait using embedded struct device's
reference counts instead of the separate reference counter in struct
hid_device
* Used &raw mut as appropriate
* Binded include/linux/device.h for get_device and put_device
* Cleaned up various comment related formatting
* Minified dev_err! format string
* Updated Group enum to be repr(u16)
* Implemented From<u16> trait for Group
* Added TODO comment when const_trait_impl stabilizes
* Made group getter functions return a Group variant instead of a raw
number
* Made sure example code builds
v1->v2:
* Binded drivers/hid/hid-ids.h for use in Rust drivers
* Remove pre-emptive referencing of a C HID driver instance before
it is fully initialized in the driver registration path
* Moved static getters to generic Device trait implementation, so
they can be used by all device::DeviceContext
* Use core macros for supporting DeviceContext transitions
* Implemented the AlwaysRefCounted and AsRef traits
* Make use for dev_err! as appropriate
RFC->v1:
* Use Danilo's core infrastructure
* Account for HID device groups
* Remove probe and remove callbacks
* Implement report_fixup support
* Properly comment code including SAFETY comments
MAINTAINERS | 8 +
drivers/hid/Kconfig | 2 +
drivers/hid/rust/Kconfig | 12 +
rust/bindings/bindings_helper.h | 3 +
rust/kernel/hid.rs | 497 ++++++++++++++++++++++++++++++++
rust/kernel/lib.rs | 2 +
6 files changed, 524 insertions(+)
create mode 100644 drivers/hid/rust/Kconfig
create mode 100644 rust/kernel/hid.rs
diff --git a/MAINTAINERS b/MAINTAINERS
index cd7ff55b5d32..dc597bfe1a54 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10850,6 +10850,14 @@ F: include/uapi/linux/hid*
F: samples/hid/
F: tools/testing/selftests/hid/
+HID CORE LAYER [RUST]
+M: Rahul Rameshbabu <sergeantsagara@protonmail.com>
+R: Benjamin Tissoires <bentiss@kernel.org>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/hid/rust/*.rs
+F: rust/kernel/hid.rs
+
HID LOGITECH DRIVERS
R: Filipe Laíns <lains@riseup.net>
L: linux-input@vger.kernel.org
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 79997553d8f9..073771d95966 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1420,6 +1420,8 @@ endmenu
source "drivers/hid/bpf/Kconfig"
+source "drivers/hid/rust/Kconfig"
+
source "drivers/hid/i2c-hid/Kconfig"
source "drivers/hid/intel-ish-hid/Kconfig"
diff --git a/drivers/hid/rust/Kconfig b/drivers/hid/rust/Kconfig
new file mode 100644
index 000000000000..d3247651829e
--- /dev/null
+++ b/drivers/hid/rust/Kconfig
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0-only
+menu "Rust HID support"
+
+config RUST_HID_ABSTRACTIONS
+ bool "Rust HID abstractions support"
+ depends on RUST
+ depends on HID=y
+ help
+ Adds support needed for HID drivers written in Rust. It provides a
+ wrapper around the C hid core.
+
+endmenu # Rust HID support
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 4ad9add117ea..773468788d98 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -46,6 +46,7 @@
#include <linux/cpufreq.h>
#include <linux/cpumask.h>
#include <linux/cred.h>
+#include <linux/device.h>
#include <linux/device/faux.h>
#include <linux/dma-mapping.h>
#include <linux/errname.h>
@@ -53,6 +54,8 @@
#include <linux/file.h>
#include <linux/firmware.h>
#include <linux/fs.h>
+#include <linux/hid.h>
+#include "../../drivers/hid/hid-ids.h"
#include <linux/ioport.h>
#include <linux/jiffies.h>
#include <linux/jump_label.h>
diff --git a/rust/kernel/hid.rs b/rust/kernel/hid.rs
new file mode 100644
index 000000000000..588483bf7204
--- /dev/null
+++ b/rust/kernel/hid.rs
@@ -0,0 +1,497 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Rahul Rameshbabu <sergeantsagara@protonmail.com>
+
+//! Abstractions for the HID interface.
+//!
+//! C header: [`include/linux/hid.h`](srctree/include/linux/hid.h)
+
+use crate::{device, device_id::RawDeviceId, driver, error::*, prelude::*, types::Opaque};
+use core::{
+ marker::PhantomData,
+ ptr::{addr_of_mut, NonNull},
+};
+
+/// Indicates the item is static read-only.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_CONSTANT: u8 = bindings::HID_MAIN_ITEM_CONSTANT as u8;
+
+/// Indicates the item represents data from a physical control.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_VARIABLE: u8 = bindings::HID_MAIN_ITEM_VARIABLE as u8;
+
+/// Indicates the item should be treated as a relative change from the previous
+/// report.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_RELATIVE: u8 = bindings::HID_MAIN_ITEM_RELATIVE as u8;
+
+/// Indicates the item should wrap around when reaching the extreme high or
+/// extreme low values.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_WRAP: u8 = bindings::HID_MAIN_ITEM_WRAP as u8;
+
+/// Indicates the item should wrap around when reaching the extreme high or
+/// extreme low values.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_NONLINEAR: u8 = bindings::HID_MAIN_ITEM_NONLINEAR as u8;
+
+/// Indicates whether the control has a preferred state it will physically
+/// return to without user intervention.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_NO_PREFERRED: u8 = bindings::HID_MAIN_ITEM_NO_PREFERRED as u8;
+
+/// Indicates whether the control has a physical state where it will not send
+/// any reports.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_NULL_STATE: u8 = bindings::HID_MAIN_ITEM_NULL_STATE as u8;
+
+/// Indicates whether the control requires host system logic to change state.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_VOLATILE: u8 = bindings::HID_MAIN_ITEM_VOLATILE as u8;
+
+/// Indicates whether the item is fixed size or a variable buffer of bytes.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_BUFFERED_BYTE: u8 = bindings::HID_MAIN_ITEM_BUFFERED_BYTE as u8;
+
+/// HID device groups are intended to help categories HID devices based on a set
+/// of common quirks and logic that they will require to function correctly.
+#[repr(u16)]
+pub enum Group {
+ /// Used to match a device against any group when probing.
+ Any = bindings::HID_GROUP_ANY as u16,
+
+ /// Indicates a generic device that should need no custom logic from the
+ /// core HID stack.
+ Generic = bindings::HID_GROUP_GENERIC as u16,
+
+ /// Maps multitouch devices to hid-multitouch instead of hid-generic.
+ Multitouch = bindings::HID_GROUP_MULTITOUCH as u16,
+
+ /// Used for autodetecing and mapping of HID sensor hubs to
+ /// hid-sensor-hub.
+ SensorHub = bindings::HID_GROUP_SENSOR_HUB as u16,
+
+ /// Used for autodetecing and mapping Win 8 multitouch devices to set the
+ /// needed quirks.
+ MultitouchWin8 = bindings::HID_GROUP_MULTITOUCH_WIN_8 as u16,
+
+ // Vendor-specific device groups.
+ /// Used to distinguish Synpatics touchscreens from other products. The
+ /// touchscreens will be handled by hid-multitouch instead, while everything
+ /// else will be managed by hid-rmi.
+ RMI = bindings::HID_GROUP_RMI as u16,
+
+ /// Used for hid-core handling to automatically identify Wacom devices and
+ /// have them probed by hid-wacom.
+ Wacom = bindings::HID_GROUP_WACOM as u16,
+
+ /// Used by logitech-djreceiver and logitech-djdevice to autodetect if
+ /// devices paied to the DJ receivers are DJ devices and handle them with
+ /// the device driver.
+ LogitechDJDevice = bindings::HID_GROUP_LOGITECH_DJ_DEVICE as u16,
+
+ /// Since the Valve Steam Controller only has vendor-specific usages,
+ /// prevent hid-generic from parsing its reports since there would be
+ /// nothing hid-generic could do for the device.
+ Steam = bindings::HID_GROUP_STEAM as u16,
+
+ /// Used to differentiate 27 Mhz frequency Logitech DJ devices from other
+ /// Logitech DJ devices.
+ Logitech27MHzDevice = bindings::HID_GROUP_LOGITECH_27MHZ_DEVICE as u16,
+
+ /// Used for autodetecting and mapping Vivaldi devices to hid-vivaldi.
+ Vivaldi = bindings::HID_GROUP_VIVALDI as u16,
+}
+
+// TODO: use `const_trait_impl` once stabilized:
+//
+// ```
+// impl const From<Group> for u16 {
+// /// [`Group`] variants are represented by [`u16`] values.
+// fn from(value: Group) -> Self {
+// value as Self
+// }
+// }
+// ```
+impl Group {
+ /// Internal function used to convert [`Group`] variants into [`u16`].
+ const fn into(self) -> u16 {
+ self as u16
+ }
+}
+
+impl From<u16> for Group {
+ /// [`u16`] values can be safely converted to [`Group`] variants.
+ fn from(value: u16) -> Self {
+ match value.into() {
+ bindings::HID_GROUP_GENERIC => Group::Generic,
+ bindings::HID_GROUP_MULTITOUCH => Group::Multitouch,
+ bindings::HID_GROUP_SENSOR_HUB => Group::SensorHub,
+ bindings::HID_GROUP_MULTITOUCH_WIN_8 => Group::MultitouchWin8,
+ bindings::HID_GROUP_RMI => Group::RMI,
+ bindings::HID_GROUP_WACOM => Group::Wacom,
+ bindings::HID_GROUP_LOGITECH_DJ_DEVICE => Group::LogitechDJDevice,
+ bindings::HID_GROUP_STEAM => Group::Steam,
+ bindings::HID_GROUP_LOGITECH_27MHZ_DEVICE => Group::Logitech27MHzDevice,
+ bindings::HID_GROUP_VIVALDI => Group::Vivaldi,
+ _ => Group::Any,
+ }
+ }
+}
+
+/// The HID device representation.
+///
+/// This structure represents the Rust abstraction for a C `struct hid_device`.
+/// The implementation abstracts the usage of an already existing C `struct
+/// hid_device` within Rust code that we get passed from the C side.
+///
+/// # Invariants
+///
+/// A [`Device`] instance represents a valid `struct hid_device` created by the
+/// C portion of the kernel.
+#[repr(transparent)]
+pub struct Device<Ctx: device::DeviceContext = device::Normal>(
+ Opaque<bindings::hid_device>,
+ PhantomData<Ctx>,
+);
+
+impl<Ctx: device::DeviceContext> Device<Ctx> {
+ fn as_raw(&self) -> *mut bindings::hid_device {
+ self.0.get()
+ }
+
+ /// Returns the HID transport bus ID.
+ pub fn bus(&self) -> u16 {
+ // SAFETY: `self.as_raw` is a valid pointer to a `struct hid_device`
+ unsafe { *self.as_raw() }.bus
+ }
+
+ /// Returns the HID report group.
+ pub fn group(&self) -> Group {
+ // SAFETY: `self.as_raw` is a valid pointer to a `struct hid_device`
+ unsafe { *self.as_raw() }.group.into()
+ }
+
+ /// Returns the HID vendor ID.
+ pub fn vendor(&self) -> u32 {
+ // SAFETY: `self.as_raw` is a valid pointer to a `struct hid_device`
+ unsafe { *self.as_raw() }.vendor
+ }
+
+ /// Returns the HID product ID.
+ pub fn product(&self) -> u32 {
+ // SAFETY: `self.as_raw` is a valid pointer to a `struct hid_device`
+ unsafe { *self.as_raw() }.product
+ }
+}
+
+// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
+// argument.
+kernel::impl_device_context_deref!(unsafe { Device });
+kernel::impl_device_context_into_aref!(Device);
+
+// SAFETY: Instances of `Device` are always reference-counted.
+unsafe impl crate::types::AlwaysRefCounted for Device {
+ fn inc_ref(&self) {
+ // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
+ unsafe { bindings::get_device(&raw mut (*self.as_raw()).dev) };
+ }
+
+ unsafe fn dec_ref(obj: NonNull<Self>) {
+ // SAFETY: The safety requirements guarantee that the refcount is non-zero.
+ unsafe { bindings::put_device(&raw mut (*obj.cast::<bindings::hid_device>().as_ptr()).dev) }
+ }
+}
+
+impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
+ fn as_ref(&self) -> &device::Device<Ctx> {
+ // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
+ // `struct hid_device`.
+ let dev = unsafe { addr_of_mut!((*self.as_raw()).dev) };
+
+ // SAFETY: `dev` points to a valid `struct device`.
+ unsafe { device::Device::from_raw(dev) }
+ }
+}
+
+/// Abstraction for the HID device ID structure `struct hid_device_id`.
+#[repr(transparent)]
+#[derive(Clone, Copy)]
+pub struct DeviceId(bindings::hid_device_id);
+
+impl DeviceId {
+ /// Equivalent to C's `HID_USB_DEVICE` macro.
+ ///
+ /// Create a new `hid::DeviceId` from a group, vendor ID, and device ID
+ /// number.
+ pub const fn new_usb(group: Group, vendor: u32, product: u32) -> Self {
+ Self(bindings::hid_device_id {
+ bus: 0x3, // BUS_USB
+ group: group.into(),
+ vendor,
+ product,
+ driver_data: 0,
+ })
+ }
+
+ /// Returns the HID transport bus ID.
+ pub fn bus(&self) -> u16 {
+ self.0.bus
+ }
+
+ /// Returns the HID report group.
+ pub fn group(&self) -> Group {
+ self.0.group.into()
+ }
+
+ /// Returns the HID vendor ID.
+ pub fn vendor(&self) -> u32 {
+ self.0.vendor
+ }
+
+ /// Returns the HID product ID.
+ pub fn product(&self) -> u32 {
+ self.0.product
+ }
+}
+
+// SAFETY:
+// * `DeviceId` is a `#[repr(transparent)` wrapper of `hid_device_id` and does not add
+// additional invariants, so it's safe to transmute to `RawType`.
+// * `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
+unsafe impl RawDeviceId for DeviceId {
+ type RawType = bindings::hid_device_id;
+}
+
+/// [`IdTable`] type for HID.
+pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<DeviceId, T>;
+
+/// Create a HID [`IdTable`] with its alias for modpost.
+#[macro_export]
+macro_rules! hid_device_table {
+ ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
+ const $table_name: $crate::device_id::IdArray<
+ $crate::hid::DeviceId,
+ $id_info_type,
+ { $table_data.len() },
+ > = $crate::device_id::IdArray::new($table_data);
+
+ $crate::module_device_table!("hid", $module_table_name, $table_name);
+ };
+}
+
+/// The HID driver trait.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::{bindings, device, hid};
+///
+/// struct MyDriver;
+///
+/// kernel::hid_device_table!(
+/// HID_TABLE,
+/// MODULE_HID_TABLE,
+/// <MyDriver as hid::Driver>::IdInfo,
+/// [(
+/// hid::DeviceId::new_usb(
+/// hid::Group::Steam,
+/// bindings::USB_VENDOR_ID_VALVE,
+/// bindings::USB_DEVICE_ID_STEAM_DECK,
+/// ),
+/// (),
+/// )]
+/// );
+///
+/// #[vtable]
+/// impl hid::Driver for MyDriver {
+/// type IdInfo = ();
+/// const ID_TABLE: hid::IdTable<Self::IdInfo> = &HID_TABLE;
+///
+/// /// This function is optional to implement.
+/// fn report_fixup<'a, 'b: 'a>(_hdev: &hid::Device<device::Core>, rdesc: &'b mut [u8]) -> &'a [u8] {
+/// // Perform some report descriptor fixup.
+/// rdesc
+/// }
+/// }
+/// ```
+/// Drivers must implement this trait in order to get a HID driver registered.
+/// Please refer to the `Adapter` documentation for an example.
+#[vtable]
+pub trait Driver: Send {
+ /// The type holding information about each device id supported by the driver.
+ // TODO: Use `associated_type_defaults` once stabilized:
+ //
+ // ```
+ // type IdInfo: 'static = ();
+ // ```
+ type IdInfo: 'static;
+
+ /// The table of device ids supported by the driver.
+ const ID_TABLE: IdTable<Self::IdInfo>;
+
+ /// Called before report descriptor parsing. Can be used to mutate the
+ /// report descriptor before the core HID logic processes the descriptor.
+ /// Useful for problematic report descriptors that prevent HID devices from
+ /// functioning correctly.
+ ///
+ /// Optional to implement.
+ fn report_fixup<'a, 'b: 'a>(_hdev: &Device<device::Core>, _rdesc: &'b mut [u8]) -> &'a [u8] {
+ build_error!(VTABLE_DEFAULT_ERROR)
+ }
+}
+
+/// An adapter for the registration of HID drivers.
+pub struct Adapter<T: Driver>(T);
+
+// SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if
+// a preceding call to `register` has been successful.
+unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
+ type RegType = bindings::hid_driver;
+
+ unsafe fn register(
+ hdrv: &Opaque<Self::RegType>,
+ name: &'static CStr,
+ module: &'static ThisModule,
+ ) -> Result {
+ // SAFETY: It's safe to set the fields of `struct hid_driver` on initialization.
+ unsafe {
+ (*hdrv.get()).name = name.as_char_ptr();
+ (*hdrv.get()).id_table = T::ID_TABLE.as_ptr();
+ (*hdrv.get()).report_fixup = if T::HAS_REPORT_FIXUP {
+ Some(Self::report_fixup_callback)
+ } else {
+ None
+ };
+ }
+
+ // SAFETY: `hdrv` is guaranteed to be a valid `RegType`
+ to_result(unsafe {
+ bindings::__hid_register_driver(hdrv.get(), module.0, name.as_char_ptr())
+ })
+ }
+
+ unsafe fn unregister(hdrv: &Opaque<Self::RegType>) {
+ // SAFETY: `hdrv` is guaranteed to be a valid `RegType`
+ unsafe { bindings::hid_unregister_driver(hdrv.get()) }
+ }
+}
+
+impl<T: Driver + 'static> Adapter<T> {
+ extern "C" fn report_fixup_callback(
+ hdev: *mut bindings::hid_device,
+ buf: *mut u8,
+ size: *mut kernel::ffi::c_uint,
+ ) -> *const u8 {
+ // SAFETY: The HID subsystem only ever calls the report_fixup callback
+ // with a valid pointer to a `struct hid_device`.
+ //
+ // INVARIANT: `hdev` is valid for the duration of
+ // `report_fixup_callback()`.
+ let hdev = unsafe { &*hdev.cast::<Device<device::Core>>() };
+
+ // SAFETY: The HID subsystem only ever calls the report_fixup callback
+ // with a valid pointer to a `kernel::ffi::c_uint`.
+ //
+ // INVARIANT: `size` is valid for the duration of
+ // `report_fixup_callback()`.
+ let buf_len: usize = match unsafe { *size }.try_into() {
+ Ok(len) => len,
+ Err(e) => {
+ dev_err!(
+ hdev.as_ref(),
+ "Cannot fix report description due to {}!\n",
+ e
+ );
+
+ return buf;
+ }
+ };
+
+ // Build a mutable Rust slice from `buf` and `size`.
+ //
+ // SAFETY: The HID subsystem only ever calls the `report_fixup callback`
+ // with a valid pointer to a `u8` buffer.
+ //
+ // INVARIANT: `buf` is valid for the duration of
+ // `report_fixup_callback()`.
+ let rdesc_slice = unsafe { core::slice::from_raw_parts_mut(buf, buf_len) };
+ let rdesc_slice = T::report_fixup(hdev, rdesc_slice);
+
+ match rdesc_slice.len().try_into() {
+ // SAFETY: The HID subsystem only ever calls the report_fixup
+ // callback with a valid pointer to a `kernel::ffi::c_uint`.
+ //
+ // INVARIANT: `size` is valid for the duration of
+ // `report_fixup_callback()`.
+ Ok(len) => unsafe { *size = len },
+ Err(e) => {
+ dev_err!(
+ hdev.as_ref(),
+ "Fixed report description will not be used due to {}!\n",
+ e
+ );
+
+ return buf;
+ }
+ }
+
+ rdesc_slice.as_ptr()
+ }
+}
+
+/// Declares a kernel module that exposes a single HID driver.
+///
+/// # Examples
+///
+/// ```ignore
+/// kernel::module_hid_driver! {
+/// type: MyDriver,
+/// name: "Module name",
+/// authors: ["Author name"],
+/// description: "Description",
+/// license: "GPL",
+/// }
+/// ```
+#[macro_export]
+macro_rules! module_hid_driver {
+ ($($f:tt)*) => {
+ $crate::module_driver!(<T>, $crate::hid::Adapter<T>, { $($f)* });
+ };
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 733f4d4e9bae..1e50912e81f6 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -89,6 +89,8 @@
pub mod firmware;
pub mod fmt;
pub mod fs;
+#[cfg(CONFIG_RUST_HID_ABSTRACTIONS)]
+pub mod hid;
pub mod init;
pub mod io;
pub mod ioctl;
--
2.51.0
^ permalink raw reply related
* [PATCH v4 2/2] rust: hid: Glorious PC Gaming Race Model O and O- mice reference driver
From: Rahul Rameshbabu @ 2025-09-17 22:54 UTC (permalink / raw)
To: linux-input, linux-kernel, rust-for-linux
Cc: a.hindborg, alex.gaynor, aliceryhl, benjamin.tissoires,
benno.lossin, bjorn3_gh, boqun.feng, dakr, db48x, gary, jikos,
ojeda, peter.hutterer, tmgross, Rahul Rameshbabu
In-Reply-To: <20250917225341.4572-1-sergeantsagara@protonmail.com>
Demonstrate how to perform a report fixup from a Rust HID driver. The mice
specify the const flag incorrectly in the consumer input report descriptor,
which leads to inputs being ignored. Correctly patch the report descriptor
for the Model O and O- mice.
Portions of the HID report post-fixup:
device 0:0
...
0x81, 0x06, // Input (Data,Var,Rel) 84
...
0x81, 0x06, // Input (Data,Var,Rel) 112
...
0x81, 0x06, // Input (Data,Var,Rel) 140
Signed-off-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>
---
Notes:
Changelog:
v3->v4:
* Removed specifying tree in MAINTAINERS file since that is up for
debate
* Minor rebase cleanup
* Moved driver logic under drivers/hid/rust
* Use hex instead of decimal for the report descriptor comparisons
v2->v3:
* Fixed docstring formatting
* Updated MAINTAINERS file based on v1 and v2 discussion
v1->v2:
* Use vendor id and device id from drivers/hid/hid-ids.h bindings
* Make use for dev_err! as appropriate
MAINTAINERS | 6 +++
drivers/hid/hid-glorious.c | 2 +
drivers/hid/hid_glorious_rust.rs | 60 +++++++++++++++++++++++++++
drivers/hid/rust/Kconfig | 16 +++++++
drivers/hid/rust/Makefile | 6 +++
drivers/hid/rust/hid_glorious_rust.rs | 60 +++++++++++++++++++++++++++
6 files changed, 150 insertions(+)
create mode 100644 drivers/hid/hid_glorious_rust.rs
create mode 100644 drivers/hid/rust/Makefile
create mode 100644 drivers/hid/rust/hid_glorious_rust.rs
diff --git a/MAINTAINERS b/MAINTAINERS
index dc597bfe1a54..ad2f071efbaa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10362,6 +10362,12 @@ L: platform-driver-x86@vger.kernel.org
S: Maintained
F: drivers/platform/x86/gigabyte-wmi.c
+GLORIOUS RUST DRIVER [RUST]
+M: Rahul Rameshbabu <sergeantsagara@protonmail.com>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/hid/rust/hid_glorious_rust.rs
+
GNSS SUBSYSTEM
M: Johan Hovold <johan@kernel.org>
S: Maintained
diff --git a/drivers/hid/hid-glorious.c b/drivers/hid/hid-glorious.c
index 5bbd81248053..d7362852c20f 100644
--- a/drivers/hid/hid-glorious.c
+++ b/drivers/hid/hid-glorious.c
@@ -76,8 +76,10 @@ static int glorious_probe(struct hid_device *hdev,
}
static const struct hid_device_id glorious_devices[] = {
+#if !IS_ENABLED(CONFIG_HID_GLORIOUS_RUST)
{ HID_USB_DEVICE(USB_VENDOR_ID_SINOWEALTH,
USB_DEVICE_ID_GLORIOUS_MODEL_O) },
+#endif
{ HID_USB_DEVICE(USB_VENDOR_ID_SINOWEALTH,
USB_DEVICE_ID_GLORIOUS_MODEL_D) },
{ HID_USB_DEVICE(USB_VENDOR_ID_LAVIEW,
diff --git a/drivers/hid/hid_glorious_rust.rs b/drivers/hid/hid_glorious_rust.rs
new file mode 100644
index 000000000000..8cffc1c605dd
--- /dev/null
+++ b/drivers/hid/hid_glorious_rust.rs
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Rahul Rameshbabu <sergeantsagara@protonmail.com>
+
+//! Rust reference HID driver for Glorious Model O and O- mice.
+
+use kernel::{self, bindings, device, hid, prelude::*};
+
+struct GloriousRust;
+
+kernel::hid_device_table!(
+ HID_TABLE,
+ MODULE_HID_TABLE,
+ <GloriousRust as hid::Driver>::IdInfo,
+ [(
+ hid::DeviceId::new_usb(
+ hid::Group::Generic,
+ bindings::USB_VENDOR_ID_SINOWEALTH,
+ bindings::USB_DEVICE_ID_GLORIOUS_MODEL_O,
+ ),
+ (),
+ )]
+);
+
+#[vtable]
+impl hid::Driver for GloriousRust {
+ type IdInfo = ();
+ const ID_TABLE: hid::IdTable<Self::IdInfo> = &HID_TABLE;
+
+ /// Fix the Glorious Model O and O- consumer input report descriptor to use
+ /// the variable and relative flag, while clearing the const flag.
+ ///
+ /// Without this fixup, inputs from the mice will be ignored.
+ fn report_fixup<'a, 'b: 'a>(hdev: &hid::Device<device::Core>, rdesc: &'b mut [u8]) -> &'a [u8] {
+ if rdesc.len() == 213
+ && (rdesc[84] == 129 && rdesc[85] == 3)
+ && (rdesc[112] == 129 && rdesc[113] == 3)
+ && (rdesc[140] == 129 && rdesc[141] == 3)
+ {
+ dev_info!(
+ hdev.as_ref(),
+ "patching Glorious Model O consumer control report descriptor\n"
+ );
+
+ rdesc[85] = hid::MAIN_ITEM_VARIABLE | hid::MAIN_ITEM_RELATIVE;
+ rdesc[113] = hid::MAIN_ITEM_VARIABLE | hid::MAIN_ITEM_RELATIVE;
+ rdesc[141] = hid::MAIN_ITEM_VARIABLE | hid::MAIN_ITEM_RELATIVE;
+ }
+
+ rdesc
+ }
+}
+
+kernel::module_hid_driver! {
+ type: GloriousRust,
+ name: "GloriousRust",
+ authors: ["Rahul Rameshbabu <sergeantsagara@protonmail.com>"],
+ description: "Rust reference HID driver for Glorious Model O and O- mice",
+ license: "GPL",
+}
diff --git a/drivers/hid/rust/Kconfig b/drivers/hid/rust/Kconfig
index d3247651829e..d7a1bf26bed0 100644
--- a/drivers/hid/rust/Kconfig
+++ b/drivers/hid/rust/Kconfig
@@ -9,4 +9,20 @@ config RUST_HID_ABSTRACTIONS
Adds support needed for HID drivers written in Rust. It provides a
wrapper around the C hid core.
+if RUST_HID_ABSTRACTIONS
+
+menu "Special HID drivers"
+
+config HID_GLORIOUS_RUST
+ tristate "Glorious O and O- mice Rust reference driver"
+ depends on USB_HID
+ depends on RUST_HID_ABSTRACTIONS
+ help
+ Support for Glorious PC Gaming Race O and O- mice
+ in Rust.
+
+endmenu # Special HID drivers
+
+endif # RUST_HID_ABSTRACTIONS
+
endmenu # Rust HID support
diff --git a/drivers/hid/rust/Makefile b/drivers/hid/rust/Makefile
new file mode 100644
index 000000000000..6676030a2f87
--- /dev/null
+++ b/drivers/hid/rust/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for Rust HID support
+#
+
+obj-$(CONFIG_HID_GLORIOUS_RUST) += hid_glorious_rust.o
diff --git a/drivers/hid/rust/hid_glorious_rust.rs b/drivers/hid/rust/hid_glorious_rust.rs
new file mode 100644
index 000000000000..dfc3f2323b60
--- /dev/null
+++ b/drivers/hid/rust/hid_glorious_rust.rs
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Rahul Rameshbabu <sergeantsagara@protonmail.com>
+
+//! Rust reference HID driver for Glorious Model O and O- mice.
+
+use kernel::{self, bindings, device, hid, prelude::*};
+
+struct GloriousRust;
+
+kernel::hid_device_table!(
+ HID_TABLE,
+ MODULE_HID_TABLE,
+ <GloriousRust as hid::Driver>::IdInfo,
+ [(
+ hid::DeviceId::new_usb(
+ hid::Group::Generic,
+ bindings::USB_VENDOR_ID_SINOWEALTH,
+ bindings::USB_DEVICE_ID_GLORIOUS_MODEL_O,
+ ),
+ (),
+ )]
+);
+
+#[vtable]
+impl hid::Driver for GloriousRust {
+ type IdInfo = ();
+ const ID_TABLE: hid::IdTable<Self::IdInfo> = &HID_TABLE;
+
+ /// Fix the Glorious Model O and O- consumer input report descriptor to use
+ /// the variable and relative flag, while clearing the const flag.
+ ///
+ /// Without this fixup, inputs from the mice will be ignored.
+ fn report_fixup<'a, 'b: 'a>(hdev: &hid::Device<device::Core>, rdesc: &'b mut [u8]) -> &'a [u8] {
+ if rdesc.len() == 213
+ && (rdesc[84] == 0x81 && rdesc[85] == 0x3)
+ && (rdesc[112] == 0x81 && rdesc[113] == 0x3)
+ && (rdesc[140] == 0x81 && rdesc[141] == 0x3)
+ {
+ dev_info!(
+ hdev.as_ref(),
+ "patching Glorious Model O consumer control report descriptor\n"
+ );
+
+ rdesc[85] = hid::MAIN_ITEM_VARIABLE | hid::MAIN_ITEM_RELATIVE;
+ rdesc[113] = hid::MAIN_ITEM_VARIABLE | hid::MAIN_ITEM_RELATIVE;
+ rdesc[141] = hid::MAIN_ITEM_VARIABLE | hid::MAIN_ITEM_RELATIVE;
+ }
+
+ rdesc
+ }
+}
+
+kernel::module_hid_driver! {
+ type: GloriousRust,
+ name: "GloriousRust",
+ authors: ["Rahul Rameshbabu <sergeantsagara@protonmail.com>"],
+ description: "Rust reference HID driver for Glorious Model O and O- mice",
+ license: "GPL",
+}
--
2.51.0
^ permalink raw reply related
* [PATCH v6] HID: lg-g15 - Add support for Logitech G13.
From: Leo L. Schwab @ 2025-09-17 23:05 UTC (permalink / raw)
To: Hans de Goede
Cc: Kate Hsuan, Leo L. Schwab, Jiri Kosina, Benjamin Tissoires,
linux-input, linux-kernel
The Logitech G13 is a gaming keypad with general-purpose macro keys,
four LED-backlit macro preset keys, five "menu" keys, backlight toggle
key, an analog thumbstick, RGB LED backlight, and a monochrome LCD
display.
Support input event generation for all keys and the thumbstick, and
expose all LEDs.
Signed-off-by: Leo L. Schwab <ewhac@ewhac.org>
Reviewed-by: Hans de Goede <hansg@kernel.org>
Tested-by: Kate Hsuan <hpa@redhat.com>
---
Changes in v6:
- Alter interaction between `brightness` and `brightness_hw_changed`
for the backlight as advised by Hans de Goede <hansg@kernel.org>.
- On probe, query device for current state of HW backlight toggle;
track in `backlight_disabled` and update sysfs.
- Ensure non-backlight LED brightnesses report either 0 or 1.
Changes in v5:
- None; resend v4 due to bounced email submission.
Changes in v4:
- Minor changes recommended by Hans de Goede <hansg@kernel.org>.
Changes in v3:
- Re-revise commit message.
- Conditionally compile the section depending on
CONFIG_LEDS_BRIGHTNESS_HW_CHANGED correctly this time.
- Use led-class-multicolor facilities for the RGB backlight.
- Changes recommended by Kate Hsuan <hpa@redhat.com>:
- Use guard(mutex) construct.
- Fix numerous style nits.
Changes in v2:
- Add `#ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED` bracket around new
code segment dependent on that feature (fixes test robot build
error).
- Use `guard(mutex)` construct in new code (existing code left
unmodified).
- Commit message revised.
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-lg-g15.c | 448 +++++++++++++++++++++++++++++++++++++--
2 files changed, 433 insertions(+), 16 deletions(-)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 33cc5820f2be..7ed1e402b80a 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -870,6 +870,7 @@
#define USB_DEVICE_ID_LOGITECH_DUAL_ACTION 0xc216
#define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2 0xc218
#define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2_2 0xc219
+#define USB_DEVICE_ID_LOGITECH_G13 0xc21c
#define USB_DEVICE_ID_LOGITECH_G15_LCD 0xc222
#define USB_DEVICE_ID_LOGITECH_G11 0xc225
#define USB_DEVICE_ID_LOGITECH_G15_V2_LCD 0xc227
diff --git a/drivers/hid/hid-lg-g15.c b/drivers/hid/hid-lg-g15.c
index f8605656257b..7b8df2d5b57f 100644
--- a/drivers/hid/hid-lg-g15.c
+++ b/drivers/hid/hid-lg-g15.c
@@ -26,7 +26,21 @@
#define LG_G510_FEATURE_BACKLIGHT_RGB 0x05
#define LG_G510_FEATURE_POWER_ON_RGB 0x06
+#define LG_G13_INPUT_REPORT 0x01
+#define LG_G13_FEATURE_M_KEYS_LEDS 0x05
+#define LG_G13_FEATURE_BACKLIGHT_RGB 0x07
+#define LG_G13_BACKLIGHT_HW_ON_BIT 23
+
+/**
+ * g13_input_report.keybits[] is not 32-bit aligned, so we can't use the bitops macros.
+ *
+ * @ary: Pointer to array of u8s
+ * @b: Bit index into ary, LSB first. Not range checked.
+ */
+#define TEST_BIT(ary, b) ((1 << ((b) & 7)) & (ary)[(b) >> 3])
+
enum lg_g15_model {
+ LG_G13,
LG_G15,
LG_G15_V2,
LG_G510,
@@ -45,6 +59,12 @@ enum lg_g15_led_type {
LG_G15_LED_MAX
};
+struct g13_input_report {
+ u8 report_id; /* Report ID is always set to 1. */
+ u8 joy_x, joy_y;
+ u8 keybits[5];
+};
+
struct lg_g15_led {
union {
struct led_classdev cdev;
@@ -63,12 +83,188 @@ struct lg_g15_data {
struct mutex mutex;
struct work_struct work;
struct input_dev *input;
+ struct input_dev *input_js; /* Separate joystick device for G13. */
struct hid_device *hdev;
enum lg_g15_model model;
struct lg_g15_led leds[LG_G15_LED_MAX];
bool game_mode_enabled;
+ bool backlight_disabled; /* true == HW backlight toggled *OFF* */
};
+/********* G13 LED functions ***********/
+/*
+ * G13 retains no state across power cycles, and always powers up with the backlight on,
+ * color #5AFF6E, all macro key LEDs off.
+ */
+static int lg_g13_get_leds_state(struct lg_g15_data *g15)
+{
+ u8 * const tbuf = g15->transfer_buf;
+ int ret, high;
+
+ /* RGB backlight. */
+ ret = hid_hw_raw_request(g15->hdev, LG_G13_FEATURE_BACKLIGHT_RGB,
+ tbuf, 5,
+ HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+ if (ret != 5) {
+ hid_err(g15->hdev, "Error getting backlight brightness: %d\n", ret);
+ return (ret < 0) ? ret : -EIO;
+ }
+
+ /* Normalize RGB intensities against the highest component. */
+ high = max3(tbuf[1], tbuf[2], tbuf[3]);
+ if (high) {
+ g15->leds[LG_G15_KBD_BRIGHTNESS].red =
+ DIV_ROUND_CLOSEST(tbuf[1] * 255, high);
+ g15->leds[LG_G15_KBD_BRIGHTNESS].green =
+ DIV_ROUND_CLOSEST(tbuf[2] * 255, high);
+ g15->leds[LG_G15_KBD_BRIGHTNESS].blue =
+ DIV_ROUND_CLOSEST(tbuf[3] * 255, high);
+ g15->leds[LG_G15_KBD_BRIGHTNESS].brightness = high;
+ } else {
+ g15->leds[LG_G15_KBD_BRIGHTNESS].red = 255;
+ g15->leds[LG_G15_KBD_BRIGHTNESS].green = 255;
+ g15->leds[LG_G15_KBD_BRIGHTNESS].blue = 255;
+ g15->leds[LG_G15_KBD_BRIGHTNESS].brightness = 0;
+ }
+
+ /* Macro LEDs. */
+ ret = hid_hw_raw_request(g15->hdev, LG_G13_FEATURE_M_KEYS_LEDS,
+ tbuf, 5,
+ HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+ if (ret != 5) {
+ hid_err(g15->hdev, "Error getting macro LED brightness: %d\n", ret);
+ return (ret < 0) ? ret : -EIO;
+ }
+
+ for (int i = LG_G15_MACRO_PRESET1; i < LG_G15_LED_MAX; ++i)
+ g15->leds[i].brightness = !!(tbuf[1] & (1 << (i - LG_G15_MACRO_PRESET1)));
+
+ /*
+ * Bit 23 of g13_input_report.keybits[] contains the backlight's
+ * current HW toggle state. Retrieve it from the device.
+ */
+ ret = hid_hw_raw_request(g15->hdev, LG_G13_INPUT_REPORT,
+ tbuf, sizeof(struct g13_input_report),
+ HID_INPUT_REPORT, HID_REQ_GET_REPORT);
+ if (ret != sizeof(struct g13_input_report)) {
+ hid_err(g15->hdev, "Error getting backlight on/off state: %d\n", ret);
+ return (ret < 0) ? ret : -EIO;
+ }
+ g15->backlight_disabled =
+ !TEST_BIT(((struct g13_input_report *) tbuf)->keybits,
+ LG_G13_BACKLIGHT_HW_ON_BIT);
+
+ return 0;
+}
+
+static int lg_g13_kbd_led_write(struct lg_g15_data *g15,
+ struct lg_g15_led *g15_led,
+ enum led_brightness brightness)
+{
+ struct mc_subled const * const subleds = g15_led->mcdev.subled_info;
+ u8 * const tbuf = g15->transfer_buf;
+ int ret;
+
+ guard(mutex)(&g15->mutex);
+
+ led_mc_calc_color_components(&g15_led->mcdev, brightness);
+
+ tbuf[0] = 5;
+ tbuf[1] = subleds[0].brightness;
+ tbuf[2] = subleds[1].brightness;
+ tbuf[3] = subleds[2].brightness;
+ tbuf[4] = 0;
+
+ ret = hid_hw_raw_request(g15->hdev, LG_G13_FEATURE_BACKLIGHT_RGB,
+ tbuf, 5,
+ HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+ if (ret != 5) {
+ hid_err(g15->hdev, "Error setting backlight brightness: %d\n", ret);
+ return (ret < 0) ? ret : -EIO;
+ }
+
+ g15_led->brightness = brightness;
+ return 0;
+}
+
+static int lg_g13_kbd_led_set(struct led_classdev *led_cdev, enum led_brightness brightness)
+{
+ struct led_classdev_mc *mc = lcdev_to_mccdev(led_cdev);
+ struct lg_g15_led *g15_led =
+ container_of(mc, struct lg_g15_led, mcdev);
+ struct lg_g15_data *g15 = dev_get_drvdata(led_cdev->dev->parent);
+
+ /* Ignore LED off on unregister / keyboard unplug */
+ if (led_cdev->flags & LED_UNREGISTERING)
+ return 0;
+
+ return lg_g13_kbd_led_write(g15, g15_led, brightness);
+}
+
+static enum led_brightness lg_g13_kbd_led_get(struct led_classdev *led_cdev)
+{
+ struct led_classdev_mc const * const mc = lcdev_to_mccdev(led_cdev);
+ struct lg_g15_led const *g15_led =
+ container_of(mc, struct lg_g15_led, mcdev);
+
+ return g15_led->brightness;
+}
+
+static int lg_g13_mkey_led_set(struct led_classdev *led_cdev, enum led_brightness brightness)
+{
+ struct lg_g15_led *g15_led =
+ container_of(led_cdev, struct lg_g15_led, cdev);
+ struct lg_g15_data *g15 = dev_get_drvdata(led_cdev->dev->parent);
+ int i, ret;
+ u8 * const tbuf = g15->transfer_buf;
+ u8 val, mask = 0;
+
+ /* Ignore LED off on unregister / keyboard unplug */
+ if (led_cdev->flags & LED_UNREGISTERING)
+ return 0;
+
+ guard(mutex)(&g15->mutex);
+
+ for (i = LG_G15_MACRO_PRESET1; i < LG_G15_LED_MAX; ++i) {
+ if (i == g15_led->led)
+ val = brightness;
+ else
+ val = g15->leds[i].brightness;
+
+ if (val)
+ mask |= 1 << (i - LG_G15_MACRO_PRESET1);
+ }
+
+ tbuf[0] = 5;
+ tbuf[1] = mask;
+ tbuf[2] = 0;
+ tbuf[3] = 0;
+ tbuf[4] = 0;
+
+ ret = hid_hw_raw_request(g15->hdev, LG_G13_FEATURE_M_KEYS_LEDS,
+ tbuf, 5,
+ HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+ if (ret != 5) {
+ hid_err(g15->hdev, "Error setting LED brightness: %d\n", ret);
+ return (ret < 0) ? ret : -EIO;
+ }
+
+ g15_led->brightness = brightness;
+ return 0;
+}
+
+static enum led_brightness lg_g13_mkey_led_get(struct led_classdev *led_cdev)
+{
+ /*
+ * G13 doesn't change macro key LEDs behind our back, so they're
+ * whatever we last set them to.
+ */
+ struct lg_g15_led *g15_led =
+ container_of(led_cdev, struct lg_g15_led, cdev);
+
+ return g15_led->brightness;
+}
+
/******** G15 and G15 v2 LED functions ********/
static int lg_g15_update_led_brightness(struct lg_g15_data *g15)
@@ -390,6 +586,8 @@ static int lg_g15_get_initial_led_brightness(struct lg_g15_data *g15)
int ret;
switch (g15->model) {
+ case LG_G13:
+ return lg_g13_get_leds_state(g15);
case LG_G15:
case LG_G15_V2:
return lg_g15_update_led_brightness(g15);
@@ -417,6 +615,108 @@ static int lg_g15_get_initial_led_brightness(struct lg_g15_data *g15)
/******** Input functions ********/
+/* Table mapping keybits[] bit positions to event codes. */
+/* Note: Indices are discontinuous to aid readability. */
+static const u16 g13_keys_for_bits[] = {
+ /* Main keypad - keys G1 - G22 */
+ [0] = KEY_MACRO1,
+ [1] = KEY_MACRO2,
+ [2] = KEY_MACRO3,
+ [3] = KEY_MACRO4,
+ [4] = KEY_MACRO5,
+ [5] = KEY_MACRO6,
+ [6] = KEY_MACRO7,
+ [7] = KEY_MACRO8,
+ [8] = KEY_MACRO9,
+ [9] = KEY_MACRO10,
+ [10] = KEY_MACRO11,
+ [11] = KEY_MACRO12,
+ [12] = KEY_MACRO13,
+ [13] = KEY_MACRO14,
+ [14] = KEY_MACRO15,
+ [15] = KEY_MACRO16,
+ [16] = KEY_MACRO17,
+ [17] = KEY_MACRO18,
+ [18] = KEY_MACRO19,
+ [19] = KEY_MACRO20,
+ [20] = KEY_MACRO21,
+ [21] = KEY_MACRO22,
+
+ /* LCD menu buttons. */
+ [24] = KEY_KBD_LCD_MENU5, /* "Next page" button */
+ [25] = KEY_KBD_LCD_MENU1, /* Left-most */
+ [26] = KEY_KBD_LCD_MENU2,
+ [27] = KEY_KBD_LCD_MENU3,
+ [28] = KEY_KBD_LCD_MENU4, /* Right-most */
+
+ /* Macro preset and record buttons; have red LEDs under them. */
+ [29] = KEY_MACRO_PRESET1,
+ [30] = KEY_MACRO_PRESET2,
+ [31] = KEY_MACRO_PRESET3,
+ [32] = KEY_MACRO_RECORD_START,
+
+ /* 33-35 handled by joystick device. */
+
+ /* Backlight toggle. */
+ [37] = KEY_LIGHTS_TOGGLE,
+};
+
+#define G13_JS_KEYBITS_OFFSET 33
+
+static const u16 g13_keys_for_bits_js[] = {
+ /* Joystick buttons */
+ /* These keybits are at bit indices 33, 34, and 35. */
+ BTN_BASE, /* Left side */
+ BTN_BASE2, /* Bottom side */
+ BTN_THUMB, /* Stick depress */
+};
+
+static int lg_g13_event(struct lg_g15_data *g15, u8 const *data)
+{
+ struct g13_input_report const * const rep = (struct g13_input_report *) data;
+ int i, val;
+ bool backlight_disabled;
+
+ /*
+ * Main macropad and menu keys.
+ * Emit key events defined for each bit position.
+ */
+ for (i = 0; i < ARRAY_SIZE(g13_keys_for_bits); ++i) {
+ if (g13_keys_for_bits[i]) {
+ val = TEST_BIT(rep->keybits, i);
+ input_report_key(g15->input, g13_keys_for_bits[i], val);
+ }
+ }
+ input_sync(g15->input);
+
+ /*
+ * Joystick.
+ * Emit button and deflection events.
+ */
+ for (i = 0; i < ARRAY_SIZE(g13_keys_for_bits_js); ++i) {
+ val = TEST_BIT(rep->keybits, i + G13_JS_KEYBITS_OFFSET);
+ input_report_key(g15->input_js, g13_keys_for_bits_js[i], val);
+ }
+ input_report_abs(g15->input_js, ABS_X, rep->joy_x);
+ input_report_abs(g15->input_js, ABS_Y, rep->joy_y);
+ input_sync(g15->input_js);
+
+ /*
+ * Bit 23 of keybits[] reports the current backlight on/off state. If
+ * it has changed from the last cached value, apply an update.
+ */
+ backlight_disabled = !TEST_BIT(rep->keybits, LG_G13_BACKLIGHT_HW_ON_BIT);
+ if (backlight_disabled ^ g15->backlight_disabled) {
+ led_classdev_notify_brightness_hw_changed(
+ &g15->leds[LG_G15_KBD_BRIGHTNESS].mcdev.led_cdev,
+ backlight_disabled
+ ? 0 : g15->leds[LG_G15_KBD_BRIGHTNESS].brightness);
+ g15->backlight_disabled = backlight_disabled;
+ }
+
+ return 0;
+}
+
/* On the G15 Mark I Logitech has been quite creative with which bit is what */
static void lg_g15_handle_lcd_menu_keys(struct lg_g15_data *g15, u8 *data)
{
@@ -572,6 +872,10 @@ static int lg_g15_raw_event(struct hid_device *hdev, struct hid_report *report,
return 0;
switch (g15->model) {
+ case LG_G13:
+ if (data[0] == 0x01 && size == sizeof(struct g13_input_report))
+ return lg_g13_event(g15, data);
+ break;
case LG_G15:
if (data[0] == 0x02 && size == 9)
return lg_g15_event(g15, data);
@@ -616,13 +920,22 @@ static void lg_g15_setup_led_rgb(struct lg_g15_data *g15, int index)
{
int i;
struct mc_subled *subled_info;
-
- g15->leds[index].mcdev.led_cdev.brightness_set_blocking =
- lg_g510_kbd_led_set;
- g15->leds[index].mcdev.led_cdev.brightness_get =
- lg_g510_kbd_led_get;
- g15->leds[index].mcdev.led_cdev.max_brightness = 255;
- g15->leds[index].mcdev.num_colors = 3;
+ struct lg_g15_led * const gled = &g15->leds[index];
+
+ if (g15->model == LG_G13) {
+ gled->mcdev.led_cdev.brightness_set_blocking =
+ lg_g13_kbd_led_set;
+ gled->mcdev.led_cdev.brightness_get =
+ lg_g13_kbd_led_get;
+ gled->mcdev.led_cdev.flags = LED_BRIGHT_HW_CHANGED;
+ } else {
+ gled->mcdev.led_cdev.brightness_set_blocking =
+ lg_g510_kbd_led_set;
+ gled->mcdev.led_cdev.brightness_get =
+ lg_g510_kbd_led_get;
+ }
+ gled->mcdev.led_cdev.max_brightness = 255;
+ gled->mcdev.num_colors = 3;
subled_info = devm_kcalloc(&g15->hdev->dev, 3, sizeof(*subled_info), GFP_KERNEL);
if (!subled_info)
@@ -632,20 +945,20 @@ static void lg_g15_setup_led_rgb(struct lg_g15_data *g15, int index)
switch (i + 1) {
case LED_COLOR_ID_RED:
subled_info[i].color_index = LED_COLOR_ID_RED;
- subled_info[i].intensity = g15->leds[index].red;
+ subled_info[i].intensity = gled->red;
break;
case LED_COLOR_ID_GREEN:
subled_info[i].color_index = LED_COLOR_ID_GREEN;
- subled_info[i].intensity = g15->leds[index].green;
+ subled_info[i].intensity = gled->green;
break;
case LED_COLOR_ID_BLUE:
subled_info[i].color_index = LED_COLOR_ID_BLUE;
- subled_info[i].intensity = g15->leds[index].blue;
+ subled_info[i].intensity = gled->blue;
break;
}
subled_info[i].channel = i;
}
- g15->leds[index].mcdev.subled_info = subled_info;
+ gled->mcdev.subled_info = subled_info;
}
static int lg_g15_register_led(struct lg_g15_data *g15, int i, const char *name)
@@ -656,6 +969,23 @@ static int lg_g15_register_led(struct lg_g15_data *g15, int i, const char *name)
g15->leds[i].cdev.name = name;
switch (g15->model) {
+ case LG_G13:
+ if (i < LG_G15_BRIGHTNESS_MAX) {
+ /* RGB backlight. */
+ lg_g15_setup_led_rgb(g15, i);
+ ret = devm_led_classdev_multicolor_register_ext(&g15->hdev->dev,
+ &g15->leds[i].mcdev,
+ NULL);
+ } else {
+ /* Macro keys */
+ g15->leds[i].cdev.brightness_set_blocking = lg_g13_mkey_led_set;
+ g15->leds[i].cdev.brightness_get = lg_g13_mkey_led_get;
+ g15->leds[i].cdev.max_brightness = 1;
+
+ ret = devm_led_classdev_register(&g15->hdev->dev,
+ &g15->leds[i].cdev);
+ }
+ break;
case LG_G15:
case LG_G15_V2:
g15->leds[i].cdev.brightness_get = lg_g15_led_get;
@@ -702,11 +1032,9 @@ static int lg_g15_register_led(struct lg_g15_data *g15, int i, const char *name)
}
/* Common input device init code shared between keyboards and Z-10 speaker handling */
-static void lg_g15_init_input_dev(struct hid_device *hdev, struct input_dev *input,
- const char *name)
+static void lg_g15_init_input_dev_core(struct hid_device *hdev, struct input_dev *input,
+ char const *name)
{
- int i;
-
input->name = name;
input->phys = hdev->phys;
input->uniq = hdev->uniq;
@@ -717,12 +1045,42 @@ static void lg_g15_init_input_dev(struct hid_device *hdev, struct input_dev *inp
input->dev.parent = &hdev->dev;
input->open = lg_g15_input_open;
input->close = lg_g15_input_close;
+}
+
+static void lg_g15_init_input_dev(struct hid_device *hdev, struct input_dev *input,
+ const char *name)
+{
+ int i;
+
+ lg_g15_init_input_dev_core(hdev, input, name);
/* Keys below the LCD, intended for controlling a menu on the LCD */
for (i = 0; i < 5; i++)
input_set_capability(input, EV_KEY, KEY_KBD_LCD_MENU1 + i);
}
+static void lg_g13_init_input_dev(struct hid_device *hdev,
+ struct input_dev *input, const char *name,
+ struct input_dev *input_js, const char *name_js)
+{
+ /* Macropad. */
+ lg_g15_init_input_dev_core(hdev, input, name);
+ for (int i = 0; i < ARRAY_SIZE(g13_keys_for_bits); ++i) {
+ if (g13_keys_for_bits[i])
+ input_set_capability(input, EV_KEY, g13_keys_for_bits[i]);
+ }
+
+ /* OBTW, we're a joystick, too... */
+ lg_g15_init_input_dev_core(hdev, input_js, name_js);
+ for (int i = 0; i < ARRAY_SIZE(g13_keys_for_bits_js); ++i)
+ input_set_capability(input_js, EV_KEY, g13_keys_for_bits_js[i]);
+
+ input_set_capability(input_js, EV_ABS, ABS_X);
+ input_set_abs_params(input_js, ABS_X, 0, 255, 0, 0);
+ input_set_capability(input_js, EV_ABS, ABS_Y);
+ input_set_abs_params(input_js, ABS_Y, 0, 255, 0, 0);
+}
+
static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
static const char * const led_names[] = {
@@ -739,7 +1097,7 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
unsigned int connect_mask = 0;
bool has_ff000000 = false;
struct lg_g15_data *g15;
- struct input_dev *input;
+ struct input_dev *input, *input_js;
struct hid_report *rep;
int ret, i, gkeys = 0;
@@ -778,6 +1136,25 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
hid_set_drvdata(hdev, (void *)g15);
switch (g15->model) {
+ case LG_G13:
+ /*
+ * The G13 has an analog thumbstick with nearby buttons. Some
+ * libraries and applications are known to ignore devices that
+ * don't "look like" a joystick, and a device with two ABS axes
+ * and 25+ macro keys would confuse them.
+ *
+ * Create an additional input device dedicated to appear as a
+ * simplified joystick (two ABS axes, three BTN buttons).
+ */
+ input_js = devm_input_allocate_device(&hdev->dev);
+ if (!input_js)
+ return -ENOMEM;
+ g15->input_js = input_js;
+ input_set_drvdata(input_js, hdev);
+
+ connect_mask = HID_CONNECT_HIDRAW;
+ gkeys = 25;
+ break;
case LG_G15:
INIT_WORK(&g15->work, lg_g15_leds_changed_work);
/*
@@ -859,6 +1236,38 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
goto error_hw_stop;
return 0; /* All done */
+ } else if (g15->model == LG_G13) {
+ static char const * const g13_led_names[] = {
+ /* Backlight is shared between LCD and keys. */
+ "g13:rgb:kbd_backlight",
+ NULL, /* Keep in sync with led_type enum */
+ "g13:red:macro_preset_1",
+ "g13:red:macro_preset_2",
+ "g13:red:macro_preset_3",
+ "g13:red:macro_record",
+ };
+ lg_g13_init_input_dev(hdev,
+ input, "Logitech G13 Gaming Keypad",
+ input_js, "Logitech G13 Thumbstick");
+ ret = input_register_device(input);
+ if (ret)
+ goto error_hw_stop;
+ ret = input_register_device(input_js);
+ if (ret)
+ goto error_hw_stop;
+
+ for (i = 0; i < ARRAY_SIZE(g13_led_names); ++i) {
+ if (g13_led_names[i]) {
+ ret = lg_g15_register_led(g15, i, g13_led_names[i]);
+ if (ret)
+ goto error_hw_stop;
+ }
+ }
+ led_classdev_notify_brightness_hw_changed(
+ &g15->leds[LG_G15_KBD_BRIGHTNESS].mcdev.led_cdev,
+ g15->backlight_disabled
+ ? 0 : g15->leds[LG_G15_KBD_BRIGHTNESS].brightness);
+ return 0;
}
/* Setup and register input device */
@@ -903,6 +1312,13 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
}
static const struct hid_device_id lg_g15_devices[] = {
+ /*
+ * The G13 is a macropad-only device with an LCD, LED backlighing,
+ * and joystick.
+ */
+ { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
+ USB_DEVICE_ID_LOGITECH_G13),
+ .driver_data = LG_G13 },
/* The G11 is a G15 without the LCD, treat it as a G15 */
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
USB_DEVICE_ID_LOGITECH_G11),
--
2.51.0
^ permalink raw reply related
* [BUG] FTSC1000:00 2808:5662 touchscreen not working until HID modules reloaded (Huawei MateBook 14 2024)
From: Vladimir Staykov @ 2025-09-18 0:15 UTC (permalink / raw)
To: linux-input@vger.kernel.org
Hello,
On a Huawei MateBook 14 (2024) with Arch Linux and kernel 6.16.7, the built-in OLED touchscreen
(FTSC1000:00 2808:5662) is detected but does not generate any touch or pen events until HID modules
are reloaded.
Steps to reproduce:
1. Boot kernel 6.16.7 (vanilla Arch package).
2. Touchscreen appears as /dev/input/event5, stylus as /dev/input/event6.
3. dmesg shows:
i2c_hid_acpi i2c-FTSC1000:00: failed to get a report from device: -11
hid-multitouch 0018:2808:5662.0002: failed to fetch feature 5
hid-multitouch 0018:2808:5662.0002: failed to fetch feature 12
4. evtest shows ABS ranges but no events when touching.
5. After running:
modprobe -r i2c_hid_acpi i2c_hid hid_multitouch
modprobe i2c_hid_acpi hid_multitouch
touchscreen and stylus work normally and produce multitouch + pen events.
Hardware:
- Huawei MateBook 14 (2024), 14.2" 2880x1920 OLED touchscreen
- Touch controller: FTSC1000:00 2808:5662
Kernel: 6.16.7-arch1-1
Firmware: linux-firmware up to date (Sep 2025)
It seems this FTSC1000 variant may need a quirk in hid-multitouch or i2c_hid_acpi.
Please advise if more logs are needed. I can provide full dmesg, libinput list-devices,
and evtest output before and after module reload.
Best regards,
Vladimir Staykov
Sent with Proton Mail secure email.
^ permalink raw reply
* Re: [PATCH] HID: hid-ntrig: Fix potential memory leak in ntrig_report_version()
From: Masami Ichikawa @ 2025-09-18 1:46 UTC (permalink / raw)
To: Markus Elfring
Cc: linux-input, LKML, Benjamin Tissoires, Jiri Kosina, Minjong Kim
In-Reply-To: <34b16512-b098-470a-afff-bc8321e2499a@web.de>
Thank you for the review.
On Wed, Sep 17, 2025 at 4:46 PM Markus Elfring <Markus.Elfring@web.de> wrote:
>
> …
> > It is safe to move the kmalloc() call after the hid_is_usb() check to avoid
> > unnecessary allocation and potential memory leak.
>
> * See also:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.17-rc6#n94
>
> * How do you think about to increase the application of scope-based resource management?
> https://elixir.bootlin.com/linux/v6.17-rc6/source/include/linux/slab.h#L476
>
I think using the DEFINE_FREE macro simplifies cleanup, especially
when an error occurs.
> * Can a summary phrase like “Prevent memory leak in ntrig_report_version()”
> be nicer?
>
I see. I will rewrite commit log.
>
> Regards,
> Markus
Regards,
--
Masami Ichikawa
^ permalink raw reply
* Re: [PATCH v3 1/5] dt-bindings: touchscreen: convert bu21013 bindings to json schema
From: Krzysztof Kozlowski @ 2025-09-18 2:12 UTC (permalink / raw)
To: Dario Binacchi
Cc: linux-kernel, linux-amarula, Conor Dooley, Dmitry Torokhov,
Krzysztof Kozlowski, Rob Herring, devicetree, linux-input
In-Reply-To: <20250914203812.1055696-1-dario.binacchi@amarulasolutions.com>
On Sun, Sep 14, 2025 at 10:37:52PM +0200, Dario Binacchi wrote:
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/bu21013.yaml b/Documentation/devicetree/bindings/input/touchscreen/bu21013.yaml
> new file mode 100644
> index 000000000000..aeb581fcaf29
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/bu21013.yaml
Filename based on compatible, so rohm,bu21013.yaml
> @@ -0,0 +1,96 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/touchscreen/bu21013.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rohm BU21013 touchscreen
> +
> +description:
> + Rohm BU21013 I2C driven touchscreen controller.
> +
> +maintainers:
> + - Dario Binacchi <dario.binacchi@amarulasolutions.com>
> +
> +allOf:
> + - $ref: touchscreen.yaml#
> +
> +properties:
> + compatible:
> + enum:
> + - rohm,bu21013_tp
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + reset-gpios:
> + maxItems: 1
> + description: GPIO resetting the chip
Drop description, obvious.
> +
> + touch-gpios:
> + maxItems: 1
> + description: GPIO registering a touch event.
> +
> + avdd-supply:
> + description: Phandle to the regulator supplying the analog circuit.
Analog circuit supply.
(rest is redundant, it cannot be something else than phandle)
With these fixed:
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 1/3] input: mouse: trackpoint: Add doubletap enable/disable support
From: Vishnu Sankar @ 2025-09-18 2:37 UTC (permalink / raw)
To: dmitry.torokhov, hmh, hansg, ilpo.jarvinen, derekjohn.clark
Cc: mpearson-lenovo, linux-input, linux-kernel, ibm-acpi-devel,
platform-driver-x86, vsankar
In-Reply-To: <20250901135308.52340-1-vishnuocv@gmail.com>
Hello all,
Do we have any questions or concerns?
Thanks in advance!
On Mon, Sep 1, 2025 at 10:53 PM Vishnu Sankar <vishnuocv@gmail.com> wrote:
>
> Add support for enabling and disabling doubletap on TrackPoint devices
> that support this functionality. The feature is detected using firmware
> ID and exposed via sysfs as `doubletap_enabled`.
>
> The feature is only available on newer ThinkPads (2023 and later).The driver
> exposes this capability via a new sysfs attribute:
> "/sys/bus/serio/devices/seriox/doubletap_enabled".
>
> The attribute is only created if the device is detected to be capable of
> doubletap via firmware and variant ID checks. This functionality will be
> used by platform drivers such as thinkpad_acpi to expose and control doubletap
> via user interfaces.
>
> Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
> Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> Changes in v2:
> - Improve commit messages
> - Sysfs attributes moved to trackpoint.c
> - Removed unnecessary comments
> - Removed unnecessary debug messages
> - Using strstarts() instead of strcmp()
> - is_trackpoint_dt_capable() modified
> - Removed _BIT suffix and used BIT() define.
> - Reverse the trackpoint_doubletap_status() logic to return error first.
> - Removed export functions as a result of the design change
> - Changed trackpoint_dev->psmouse to parent_psmouse
> - The path of trackpoint.h is not changed.
> Changes in v3:
> - No changes.
> ---
> drivers/input/mouse/trackpoint.c | 149 +++++++++++++++++++++++++++++++
> drivers/input/mouse/trackpoint.h | 15 ++++
> 2 files changed, 164 insertions(+)
>
> diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
> index 5f6643b69a2c..c6f17b0dec3a 100644
> --- a/drivers/input/mouse/trackpoint.c
> +++ b/drivers/input/mouse/trackpoint.c
> @@ -16,6 +16,8 @@
> #include "psmouse.h"
> #include "trackpoint.h"
>
> +static struct trackpoint_data *trackpoint_dev;
> +
> static const char * const trackpoint_variants[] = {
> [TP_VARIANT_IBM] = "IBM",
> [TP_VARIANT_ALPS] = "ALPS",
> @@ -63,6 +65,21 @@ static int trackpoint_write(struct ps2dev *ps2dev, u8 loc, u8 val)
> return ps2_command(ps2dev, param, MAKE_PS2_CMD(3, 0, TP_COMMAND));
> }
>
> +/* Read function for TrackPoint extended registers */
> +static int trackpoint_extended_read(struct ps2dev *ps2dev, u8 loc, u8 *val)
> +{
> + u8 ext_param[2] = {TP_READ_MEM, loc};
> + int error;
> +
> + error = ps2_command(ps2dev,
> + ext_param, MAKE_PS2_CMD(2, 1, TP_COMMAND));
> +
> + if (!error)
> + *val = ext_param[0];
> +
> + return error;
> +}
> +
> static int trackpoint_toggle_bit(struct ps2dev *ps2dev, u8 loc, u8 mask)
> {
> u8 param[3] = { TP_TOGGLE, loc, mask };
> @@ -393,6 +410,131 @@ static int trackpoint_reconnect(struct psmouse *psmouse)
> return 0;
> }
>
> +/* List of known incapable device PNP IDs */
> +static const char * const dt_incompatible_devices[] = {
> + "LEN0304",
> + "LEN0306",
> + "LEN0317",
> + "LEN031A",
> + "LEN031B",
> + "LEN031C",
> + "LEN031D",
> +};
> +
> +/*
> + * checks if it’s a doubletap capable device
> + * The PNP ID format eg: is "PNP: LEN030d PNP0f13".
> + */
> +static bool is_trackpoint_dt_capable(const char *pnp_id)
> +{
> + const char *id_start;
> + char id[8];
> +
> + if (!strstarts(pnp_id, "PNP: LEN03"))
> + return false;
> +
> + /* Points to "LEN03xxxx" */
> + id_start = pnp_id + 5;
> + if (sscanf(id_start, "%7s", id) != 1)
> + return false;
> +
> + /* Check if it's blacklisted */
> + for (size_t i = 0; i < ARRAY_SIZE(dt_incompatible_devices); ++i) {
> + if (strcmp(id, dt_incompatible_devices[i]) == 0)
> + return false;
> + }
> + return true;
> +}
> +
> +/* Trackpoint doubletap status function */
> +static int trackpoint_doubletap_status(bool *status)
> +{
> + struct trackpoint_data *tp = trackpoint_dev;
> + struct ps2dev *ps2dev = &tp->parent_psmouse->ps2dev;
> + u8 reg_val;
> + int rc;
> +
> + /* Reading the Doubletap register using extended read */
> + rc = trackpoint_extended_read(ps2dev, TP_DOUBLETAP, ®_val);
> + if (rc)
> + return rc;
> +
> + *status = reg_val & TP_DOUBLETAP_STATUS ? true : false;
> +
> + return 0;
> +}
> +
> +/* Trackpoint doubletap enable/disable function */
> +static int trackpoint_set_doubletap(bool enable)
> +{
> + struct trackpoint_data *tp = trackpoint_dev;
> + struct ps2dev *ps2dev = &tp->parent_psmouse->ps2dev;
> + static u8 doubletap_state;
> + u8 new_val;
> +
> + if (!tp)
> + return -ENODEV;
> +
> + new_val = enable ? TP_DOUBLETAP_ENABLE : TP_DOUBLETAP_DISABLE;
> +
> + if (doubletap_state == new_val)
> + return 0;
> +
> + doubletap_state = new_val;
> +
> + return trackpoint_write(ps2dev, TP_DOUBLETAP, new_val);
> +}
> +
> +/*
> + * Trackpoint Doubletap Interface
> + * Control/Monitoring of Trackpoint Doubletap from:
> + * /sys/bus/serio/devices/seriox/doubletap_enabled
> + */
> +static ssize_t doubletap_enabled_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct serio *serio = to_serio_port(dev);
> + struct psmouse *psmouse = psmouse_from_serio(serio);
> + struct trackpoint_data *tp = psmouse->private;
> + bool status;
> + int rc;
> +
> + if (!tp || !tp->doubletap_capable)
> + return -ENODEV;
> +
> + rc = trackpoint_doubletap_status(&status);
> + if (rc)
> + return rc;
> +
> + return sysfs_emit(buf, "%d\n", status ? 1 : 0);
> +}
> +
> +static ssize_t doubletap_enabled_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct serio *serio = to_serio_port(dev);
> + struct psmouse *psmouse = psmouse_from_serio(serio);
> + struct trackpoint_data *tp = psmouse->private;
> + bool enable;
> + int err;
> +
> + if (!tp || !tp->doubletap_capable)
> + return -ENODEV;
> +
> + err = kstrtobool(buf, &enable);
> + if (err)
> + return err;
> +
> + err = trackpoint_set_doubletap(enable);
> + if (err)
> + return err;
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR_RW(doubletap_enabled);
> +
> int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
> {
> struct ps2dev *ps2dev = &psmouse->ps2dev;
> @@ -425,6 +567,9 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
> psmouse->reconnect = trackpoint_reconnect;
> psmouse->disconnect = trackpoint_disconnect;
>
> + trackpoint_dev = psmouse->private;
> + trackpoint_dev->parent_psmouse = psmouse;
> +
> if (variant_id != TP_VARIANT_IBM) {
> /* Newer variants do not support extended button query. */
> button_info = 0x33;
> @@ -470,6 +615,10 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
> psmouse->vendor, firmware_id,
> (button_info & 0xf0) >> 4, button_info & 0x0f);
>
> + tp->doubletap_capable = is_trackpoint_dt_capable(ps2dev->serio->firmware_id);
> + if (tp->doubletap_capable)
> + device_create_file(&psmouse->ps2dev.serio->dev, &dev_attr_doubletap_enabled);
> +
> return 0;
> }
>
> diff --git a/drivers/input/mouse/trackpoint.h b/drivers/input/mouse/trackpoint.h
> index eb5412904fe0..256e8cb35581 100644
> --- a/drivers/input/mouse/trackpoint.h
> +++ b/drivers/input/mouse/trackpoint.h
> @@ -8,6 +8,8 @@
> #ifndef _TRACKPOINT_H
> #define _TRACKPOINT_H
>
> +#include <linux/bitops.h>
> +
> /*
> * These constants are from the TrackPoint System
> * Engineering documentation Version 4 from IBM Watson
> @@ -69,6 +71,8 @@
> /* (how hard it is to drag */
> /* with Z-axis pressed) */
>
> +#define TP_DOUBLETAP 0x58 /* TrackPoint doubletap register */
> +
> #define TP_MINDRAG 0x59 /* Minimum amount of force needed */
> /* to trigger dragging */
>
> @@ -139,6 +143,14 @@
> #define TP_DEF_TWOHAND 0x00
> #define TP_DEF_SOURCE_TAG 0x00
>
> +/* Doubletap register values */
> +#define TP_DOUBLETAP_ENABLE 0xFF /* Enable value */
> +#define TP_DOUBLETAP_DISABLE 0xFE /* Disable value */
> +
> +#define TP_DOUBLETAP_STATUS_BIT 0 /* 0th bit defines enable/disable */
> +
> +#define TP_DOUBLETAP_STATUS BIT(TP_DOUBLETAP_STATUS_BIT)
> +
> #define MAKE_PS2_CMD(params, results, cmd) ((params<<12) | (results<<8) | (cmd))
>
> struct trackpoint_data {
> @@ -150,11 +162,14 @@ struct trackpoint_data {
> u8 thresh, upthresh;
> u8 ztime, jenks;
> u8 drift_time;
> + bool doubletap_capable;
>
> /* toggles */
> bool press_to_select;
> bool skipback;
> bool ext_dev;
> +
> + struct psmouse *parent_psmouse;
> };
>
> int trackpoint_detect(struct psmouse *psmouse, bool set_properties);
> --
> 2.48.1
>
--
Regards,
Vishnu Sankar
+817015150407 (Japan)
^ permalink raw reply
* Re: [PATCH v3 1/3] input: mouse: trackpoint: Add doubletap enable/disable support
From: Dmitry Torokhov @ 2025-09-18 4:57 UTC (permalink / raw)
To: Vishnu Sankar
Cc: hmh, hansg, ilpo.jarvinen, derekjohn.clark, mpearson-lenovo,
linux-input, linux-kernel, ibm-acpi-devel, platform-driver-x86,
vsankar
In-Reply-To: <20250901135308.52340-1-vishnuocv@gmail.com>
Hi Vishnu,
On Mon, Sep 01, 2025 at 10:53:05PM +0900, Vishnu Sankar wrote:
> Add support for enabling and disabling doubletap on TrackPoint devices
> that support this functionality. The feature is detected using firmware
> ID and exposed via sysfs as `doubletap_enabled`.
>
> The feature is only available on newer ThinkPads (2023 and later).The driver
> exposes this capability via a new sysfs attribute:
> "/sys/bus/serio/devices/seriox/doubletap_enabled".
>
> The attribute is only created if the device is detected to be capable of
> doubletap via firmware and variant ID checks. This functionality will be
> used by platform drivers such as thinkpad_acpi to expose and control doubletap
> via user interfaces.
>
> Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
> Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> Changes in v2:
> - Improve commit messages
> - Sysfs attributes moved to trackpoint.c
> - Removed unnecessary comments
> - Removed unnecessary debug messages
> - Using strstarts() instead of strcmp()
> - is_trackpoint_dt_capable() modified
> - Removed _BIT suffix and used BIT() define.
> - Reverse the trackpoint_doubletap_status() logic to return error first.
> - Removed export functions as a result of the design change
> - Changed trackpoint_dev->psmouse to parent_psmouse
> - The path of trackpoint.h is not changed.
> Changes in v3:
> - No changes.
> ---
> drivers/input/mouse/trackpoint.c | 149 +++++++++++++++++++++++++++++++
> drivers/input/mouse/trackpoint.h | 15 ++++
> 2 files changed, 164 insertions(+)
>
> diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
> index 5f6643b69a2c..c6f17b0dec3a 100644
> --- a/drivers/input/mouse/trackpoint.c
> +++ b/drivers/input/mouse/trackpoint.c
> @@ -16,6 +16,8 @@
> #include "psmouse.h"
> #include "trackpoint.h"
>
> +static struct trackpoint_data *trackpoint_dev;
Please do not use globals.
> +
> static const char * const trackpoint_variants[] = {
> [TP_VARIANT_IBM] = "IBM",
> [TP_VARIANT_ALPS] = "ALPS",
> @@ -63,6 +65,21 @@ static int trackpoint_write(struct ps2dev *ps2dev, u8 loc, u8 val)
> return ps2_command(ps2dev, param, MAKE_PS2_CMD(3, 0, TP_COMMAND));
> }
>
> +/* Read function for TrackPoint extended registers */
> +static int trackpoint_extended_read(struct ps2dev *ps2dev, u8 loc, u8 *val)
> +{
> + u8 ext_param[2] = {TP_READ_MEM, loc};
> + int error;
> +
> + error = ps2_command(ps2dev,
> + ext_param, MAKE_PS2_CMD(2, 1, TP_COMMAND));
> +
> + if (!error)
> + *val = ext_param[0];
> +
> + return error;
> +}
> +
> static int trackpoint_toggle_bit(struct ps2dev *ps2dev, u8 loc, u8 mask)
> {
> u8 param[3] = { TP_TOGGLE, loc, mask };
> @@ -393,6 +410,131 @@ static int trackpoint_reconnect(struct psmouse *psmouse)
> return 0;
> }
>
> +/* List of known incapable device PNP IDs */
> +static const char * const dt_incompatible_devices[] = {
> + "LEN0304",
> + "LEN0306",
> + "LEN0317",
> + "LEN031A",
> + "LEN031B",
> + "LEN031C",
> + "LEN031D",
> +};
> +
> +/*
> + * checks if it’s a doubletap capable device
> + * The PNP ID format eg: is "PNP: LEN030d PNP0f13".
> + */
> +static bool is_trackpoint_dt_capable(const char *pnp_id)
> +{
> + const char *id_start;
> + char id[8];
> +
> + if (!strstarts(pnp_id, "PNP: LEN03"))
> + return false;
> +
> + /* Points to "LEN03xxxx" */
> + id_start = pnp_id + 5;
> + if (sscanf(id_start, "%7s", id) != 1)
> + return false;
> +
> + /* Check if it's blacklisted */
> + for (size_t i = 0; i < ARRAY_SIZE(dt_incompatible_devices); ++i) {
> + if (strcmp(id, dt_incompatible_devices[i]) == 0)
> + return false;
> + }
> + return true;
> +}
> +
> +/* Trackpoint doubletap status function */
> +static int trackpoint_doubletap_status(bool *status)
> +{
> + struct trackpoint_data *tp = trackpoint_dev;
> + struct ps2dev *ps2dev = &tp->parent_psmouse->ps2dev;
> + u8 reg_val;
> + int rc;
> +
> + /* Reading the Doubletap register using extended read */
> + rc = trackpoint_extended_read(ps2dev, TP_DOUBLETAP, ®_val);
> + if (rc)
> + return rc;
> +
> + *status = reg_val & TP_DOUBLETAP_STATUS ? true : false;
> +
> + return 0;
> +}
> +
> +/* Trackpoint doubletap enable/disable function */
> +static int trackpoint_set_doubletap(bool enable)
> +{
> + struct trackpoint_data *tp = trackpoint_dev;
> + struct ps2dev *ps2dev = &tp->parent_psmouse->ps2dev;
> + static u8 doubletap_state;
> + u8 new_val;
> +
> + if (!tp)
> + return -ENODEV;
> +
> + new_val = enable ? TP_DOUBLETAP_ENABLE : TP_DOUBLETAP_DISABLE;
> +
> + if (doubletap_state == new_val)
> + return 0;
> +
> + doubletap_state = new_val;
> +
> + return trackpoint_write(ps2dev, TP_DOUBLETAP, new_val);
> +}
> +
> +/*
> + * Trackpoint Doubletap Interface
> + * Control/Monitoring of Trackpoint Doubletap from:
> + * /sys/bus/serio/devices/seriox/doubletap_enabled
> + */
> +static ssize_t doubletap_enabled_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct serio *serio = to_serio_port(dev);
> + struct psmouse *psmouse = psmouse_from_serio(serio);
> + struct trackpoint_data *tp = psmouse->private;
> + bool status;
> + int rc;
> +
> + if (!tp || !tp->doubletap_capable)
> + return -ENODEV;
> +
> + rc = trackpoint_doubletap_status(&status);
> + if (rc)
> + return rc;
> +
> + return sysfs_emit(buf, "%d\n", status ? 1 : 0);
> +}
> +
> +static ssize_t doubletap_enabled_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct serio *serio = to_serio_port(dev);
> + struct psmouse *psmouse = psmouse_from_serio(serio);
> + struct trackpoint_data *tp = psmouse->private;
> + bool enable;
> + int err;
> +
> + if (!tp || !tp->doubletap_capable)
> + return -ENODEV;
> +
> + err = kstrtobool(buf, &enable);
> + if (err)
> + return err;
> +
> + err = trackpoint_set_doubletap(enable);
> + if (err)
> + return err;
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR_RW(doubletap_enabled);
> +
> int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
> {
> struct ps2dev *ps2dev = &psmouse->ps2dev;
> @@ -425,6 +567,9 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
> psmouse->reconnect = trackpoint_reconnect;
> psmouse->disconnect = trackpoint_disconnect;
>
> + trackpoint_dev = psmouse->private;
> + trackpoint_dev->parent_psmouse = psmouse;
> +
> if (variant_id != TP_VARIANT_IBM) {
> /* Newer variants do not support extended button query. */
> button_info = 0x33;
> @@ -470,6 +615,10 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
> psmouse->vendor, firmware_id,
> (button_info & 0xf0) >> 4, button_info & 0x0f);
>
> + tp->doubletap_capable = is_trackpoint_dt_capable(ps2dev->serio->firmware_id);
> + if (tp->doubletap_capable)
> + device_create_file(&psmouse->ps2dev.serio->dev, &dev_attr_doubletap_enabled);
Please use existing facilities in psmouse driver to define and register
protocol-specific attributes. Use is_visible() to control whether the
attribute is accessible or not.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 1/1] dt-bindings: input: convert tca8418_keypad.txt to yaml format
From: Dmitry Torokhov @ 2025-09-18 5:09 UTC (permalink / raw)
To: Frank Li
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list, imx
In-Reply-To: <20250916171327.3773620-1-Frank.Li@nxp.com>
On Tue, Sep 16, 2025 at 01:13:26PM -0400, Frank Li wrote:
> Convert tca8418_keypad.txt to yaml format.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v5 2/2] Input: add Himax HX852x(ES) touchscreen driver
From: Dmitry Torokhov @ 2025-09-18 5:20 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg,
Jeff LaBundy, Jonathan Albrieux, linux-input, devicetree,
linux-kernel, Stephan Gerhold
In-Reply-To: <20250915-hx852x-v5-2-b938182f1056@linaro.org>
Hi Stephan,
On Mon, Sep 15, 2025 at 04:19:57PM +0200, Stephan Gerhold wrote:
> +static int hx852x_i2c_read(struct hx852x *hx, u8 cmd, void *data, u16 len)
> +{
> + struct i2c_client *client = hx->client;
> + int ret;
> +
> + struct i2c_msg msg[] = {
> + {
> + .addr = client->addr,
> + .flags = 0,
> + .len = 1,
> + .buf = &cmd,
> + },
> + {
> + .addr = client->addr,
> + .flags = I2C_M_RD,
> + .len = len,
> + .buf = data,
> + },
> + };
> +
> + ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
> + if (ret != ARRAY_SIZE(msg)) {
Added
error = ret < 0 ? ret : -EIO;
because theoretically i2c_transfer() can return 0 as number of messages
transferred.
> +
> +err_test_mode:
> + error = i2c_smbus_write_byte_data(hx->client, HX852X_REG_SRAM_SWITCH, 0) ? : error;
You want to return the first error that happened, not the last one.
Changed to:
error2 = i2c_smbus_write_byte_data(...);
error = error ?: error2;
> +
> +static int hx852x_suspend(struct device *dev)
> +{
> + struct hx852x *hx = dev_get_drvdata(dev);
> + int error = 0;
> +
> + mutex_lock(&hx->input_dev->mutex);
Changed to use
guard(mutex)(&hx->input_dev->mutex);
style and applied.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v5 1/2] dt-bindings: input: touchscreen: document Himax HX852x(ES)
From: Dmitry Torokhov @ 2025-09-18 5:21 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg,
Jeff LaBundy, Jonathan Albrieux, linux-input, devicetree,
linux-kernel, Stephan Gerhold, Krzysztof Kozlowski
In-Reply-To: <20250915-hx852x-v5-1-b938182f1056@linaro.org>
On Mon, Sep 15, 2025 at 04:19:56PM +0200, Stephan Gerhold wrote:
> From: Stephan Gerhold <stephan@gerhold.net>
>
> Himax HX852x(ES) is a touch panel controller with optional support
> for capacitive touch keys.
>
> Unfortunately, the model naming is quite unclear and confusing. There
> seems to be a distinction between models (e.g. HX8526) and the "series"
> suffix (e.g. -A, -B, -C, -D, -E, -ES). But this doesn't seem to be
> applied very consistently because e.g. HX8527-E(44) actually seems to
> belong to the -ES series.
>
> The compatible consists of the actual part number followed by the
> "series" as fallback compatible. Typically only the latter will be
> interesting for drivers as there is no relevant difference on the
> driver side.
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4 1/6] Input: imx6ul_tsc - fix typo in register name
From: Dmitry Torokhov @ 2025-09-18 5:39 UTC (permalink / raw)
To: Dario Binacchi
Cc: linux-kernel, linux-amarula, Frank Li, Michael Trimarchi,
Fabio Estevam, Pengutronix Kernel Team, Sascha Hauer, Shawn Guo,
imx, linux-arm-kernel, linux-input
In-Reply-To: <20250917080534.1772202-2-dario.binacchi@amarulasolutions.com>
On Wed, Sep 17, 2025 at 10:05:06AM +0200, Dario Binacchi wrote:
> From: Michael Trimarchi <michael@amarulasolutions.com>
>
> Replace 'SETING' with 'SETTING'.
>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4 2/6] Input: imx6ul_tsc - use BIT, FIELD_{GET,PREP} and GENMASK macros
From: Dmitry Torokhov @ 2025-09-18 5:39 UTC (permalink / raw)
To: Dario Binacchi
Cc: linux-kernel, linux-amarula, Frank Li, Fabio Estevam,
Michael Trimarchi, Pengutronix Kernel Team, Sascha Hauer,
Shawn Guo, imx, linux-arm-kernel, linux-input
In-Reply-To: <20250917080534.1772202-3-dario.binacchi@amarulasolutions.com>
On Wed, Sep 17, 2025 at 10:05:07AM +0200, Dario Binacchi wrote:
> Replace opencoded masking and shifting, with BIT(), GENMASK(),
> FIELD_GET() and FIELD_PREP() macros.
>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* [PATCH] hid/usbhid: add reset device for EPROTO
From: zhangjinpeng @ 2025-09-18 5:55 UTC (permalink / raw)
To: jikos, benjamin.tissoires
Cc: linux-usb, linux-input, linux-kernel, zhangjinpeng
[ 792.354988] input: PixArt USB Optical Mouse as /devices/platform/PHYT0039:03/usb7/7-1/7-1.2/7-1.2:1.0/0003:093A:2510.0028/input/input53
[ 792.355081] hid-generic 0003:093A:2510.0028: input,hidraw1: USB HID v1.11 Mouse [PixArt USB Optical Mouse] on usb-PHYT0039:03-1.2/input0
[ 792.355137] hub 7-1:1.0: state 7 ports 4 chg 0000 evt 0004
： xhci-hcd PHYT0039:03: Transfer error for slot 4 ep 2 on endpoint
[ 794.579339] xhci-hcd PHYT0039:03: Giveback URB 00000000ab6c1cac, len = 0, expected = 4, status = -71
[ 794.596152] xhci-hcd PHYT0039:03: WARN halted endpoint, queueing URB anyway.
[ 917.451251] hub 7-1:1.0: state 7 ports 4 chg 0000 evt 0004
[ 917.451323] usb 7-1-port2: status 0100, change 0001, 12 Mb/s
[ 917.451362] usb 7-1-port2: indicator auto status 0
[ 917.451365] usb 7-1.2: USB disconnect, device number 45
[ 917.451367] usb 7-1.2: unregistering device
[ 917.451369] usb 7-1.2: unregistering interface 7-1.2:1.0
[ 917.451429] xhci-hcd PHYT0039:03: Cancel URB 00000000ab6c1cac, dev 1.2, ep 0x81, starting at offset 0x2361ea6280
[ 917.451432] xhci-hcd PHYT0039:03: // Ding dong!
[ 917.451436] xhci-hcd PHYT0039:03: shutdown urb ffffffa2ebc8e400 ep1in-intr
[ 917.451440] xhci-hcd PHYT0039:03: Removing canceled TD starting at 0x2361ea6280 (dma).
[ 917.500303] usb 7-1.2: usb_disable_device nuking all URBs
[ 917.500310] xhci-hcd PHYT0039:03: xhci_drop_endpoint called for udev 00000000e00ae900
[ 917.500324] xhci-hcd PHYT0039:03: drop ep 0x81, slot id 4, new drop flags = 0x8, new add flags = 0x0
[ 917.500326] xhci-hcd PHYT0039:03: xhci_check_bandwidth called for udev 00000000e00ae900
[ 917.500330] xhci-hcd PHYT0039:03: // Ding dong!
[ 917.500351] xhci-hcd PHYT0039:03: Successful Endpoint Configure command
[ 917.500579] xhci-hcd PHYT0039:03: // Ding dong!
[ 917.656189] usb 7-1-port2: debounce total 100ms stable 100ms status 0x100
Signed-off-by: zhangjinpeng <zhangjinpeng@kylinos.cn>
---
drivers/hid/usbhid/hid-core.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 257dd73e37bf..253f82f33b08 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -306,8 +306,13 @@ static void hid_irq_in(struct urb *urb)
case -ESHUTDOWN: /* unplug */
clear_bit(HID_IN_RUNNING, &usbhid->iofl);
return;
- case -EILSEQ: /* protocol error or unplug */
case -EPROTO: /* protocol error or unplug */
+ usbhid_mark_busy(usbhid);
+ clear_bit(HID_IN_RUNNING, &usbhid->iofl);
+ set_bit(HID_CLEAR_HALT, &usbhid->iofl);
+ usb_queue_reset_device(usbhid->intf);
+ return;
+ case -EILSEQ: /* protocol error or unplug */
case -ETIME: /* protocol error or unplug */
case -ETIMEDOUT: /* Should never happen, but... */
usbhid_mark_busy(usbhid);
2.25.1
^ permalink raw reply related
* Re: [PATCH 1/1] dt-bindings: touchscreen: resistive-adc-touch: change to unevaluatedProperties
From: Dmitry Torokhov @ 2025-09-18 5:58 UTC (permalink / raw)
To: Frank Li
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Oleksij Rempel,
open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list, imx
In-Reply-To: <20250910224402.994046-1-Frank.Li@nxp.com>
On Wed, Sep 10, 2025 at 06:44:01PM -0400, Frank Li wrote:
> Change additionalProperties to unevaluatedProperties because it refs to
> touchscreen.yaml.
>
> Fix below CHECK_DTBS warnings:
> arch/arm/boot/dts/nxp/imx/imx6dl-skov-revc-lt6.dtb: touchscreen (resistive-adc-touch): 'touchscreen-y-plate-ohms' does not match any of the regexes: '^pinctrl-[0-9]+$'
> from schema $id: http://devicetree.org/schemas/input/touchscreen/resistive-adc-touch.yaml#
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* [PATCH v5 0/2] Initial work for Rust abstraction for HID device driver development
From: Rahul Rameshbabu @ 2025-09-18 7:08 UTC (permalink / raw)
To: linux-input, linux-kernel, rust-for-linux
Cc: a.hindborg, alex.gaynor, aliceryhl, benjamin.tissoires,
benno.lossin, bjorn3_gh, boqun.feng, dakr, db48x, gary, jikos,
ojeda, peter.hutterer, tmgross, Rahul Rameshbabu
Haa.... My v4 had a build system bug, so I never properly test
GloriousRust. I did not implement the RawDeviceIdIndex trait in v4. My v5
should properly exercise the reference driver.
I wanted to thank Benjamin for his response on the previous v3 RESEND.
Greatly appreciated. I have gone ahead with some minor logistical changes
in the series, based on his response. I have dropped the C patch since he
also took that patch into the hid tree in v3.
Link: https://lore.kernel.org/rust-for-linux/wjfjzjc626n55zvhksiyldobwubr2imbvfavqej333lvnka2wn@r4zfcjqtanvu/
Link: https://lore.kernel.org/rust-for-linux/175810473311.3076338.14309101339951114135.b4-ty@kernel.org/
Rahul Rameshbabu (2):
rust: core abstractions for HID drivers
rust: hid: Glorious PC Gaming Race Model O and O- mice reference
driver
MAINTAINERS | 14 +
drivers/hid/Kconfig | 2 +
drivers/hid/Makefile | 2 +
drivers/hid/hid-glorious.c | 2 +
drivers/hid/hid_glorious_rust.rs | 60 +++
drivers/hid/rust/Kconfig | 28 ++
drivers/hid/rust/Makefile | 6 +
drivers/hid/rust/hid_glorious_rust.rs | 60 +++
rust/bindings/bindings_helper.h | 3 +
rust/kernel/hid.rs | 513 ++++++++++++++++++++++++++
rust/kernel/lib.rs | 2 +
11 files changed, 692 insertions(+)
create mode 100644 drivers/hid/hid_glorious_rust.rs
create mode 100644 drivers/hid/rust/Kconfig
create mode 100644 drivers/hid/rust/Makefile
create mode 100644 drivers/hid/rust/hid_glorious_rust.rs
create mode 100644 rust/kernel/hid.rs
base-commit: 657403637f7d343352efb29b53d9f92dcf86aebb
--
2.51.0
^ permalink raw reply
* [PATCH v5 1/2] rust: core abstractions for HID drivers
From: Rahul Rameshbabu @ 2025-09-18 7:08 UTC (permalink / raw)
To: linux-input, linux-kernel, rust-for-linux
Cc: a.hindborg, alex.gaynor, aliceryhl, benjamin.tissoires,
benno.lossin, bjorn3_gh, boqun.feng, dakr, db48x, gary, jikos,
ojeda, peter.hutterer, tmgross, Rahul Rameshbabu
In-Reply-To: <20250918070824.70822-1-sergeantsagara@protonmail.com>
These abstractions enable the development of HID drivers in Rust by binding
with the HID core C API. They provide Rust types that map to the
equivalents in C. In this initial draft, only hid_device and hid_device_id
are provided direct Rust type equivalents. hid_driver is specially wrapped
with a custom Driver type. The module_hid_driver! macro provides analogous
functionality to its C equivalent. Only the .report_fixup callback is
binded to Rust so far.
Future work for these abstractions would include more bindings for common
HID-related types, such as hid_field, hid_report_enum, and hid_report as
well as more bus callbacks. Providing Rust equivalents to useful core HID
functions will also be necessary for HID driver development in Rust.
Signed-off-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>
---
Notes:
Changelog:
v4->v5:
* Add rust/ to drivers/hid/Makefile
* Implement RawDeviceIdIndex trait
v3->v4:
* Removed specifying tree in MAINTAINERS file since that is up for
debate
* Minor rebase cleanup
* Moved driver logic under drivers/hid/rust
v2->v3:
* Implemented AlwaysRefCounted trait using embedded struct device's
reference counts instead of the separate reference counter in struct
hid_device
* Used &raw mut as appropriate
* Binded include/linux/device.h for get_device and put_device
* Cleaned up various comment related formatting
* Minified dev_err! format string
* Updated Group enum to be repr(u16)
* Implemented From<u16> trait for Group
* Added TODO comment when const_trait_impl stabilizes
* Made group getter functions return a Group variant instead of a raw
number
* Made sure example code builds
v1->v2:
* Binded drivers/hid/hid-ids.h for use in Rust drivers
* Remove pre-emptive referencing of a C HID driver instance before
it is fully initialized in the driver registration path
* Moved static getters to generic Device trait implementation, so
they can be used by all device::DeviceContext
* Use core macros for supporting DeviceContext transitions
* Implemented the AlwaysRefCounted and AsRef traits
* Make use for dev_err! as appropriate
RFC->v1:
* Use Danilo's core infrastructure
* Account for HID device groups
* Remove probe and remove callbacks
* Implement report_fixup support
* Properly comment code including SAFETY comments
MAINTAINERS | 8 +
drivers/hid/Kconfig | 2 +
drivers/hid/Makefile | 2 +
drivers/hid/rust/Kconfig | 12 +
rust/bindings/bindings_helper.h | 3 +
rust/kernel/hid.rs | 513 ++++++++++++++++++++++++++++++++
rust/kernel/lib.rs | 2 +
7 files changed, 542 insertions(+)
create mode 100644 drivers/hid/rust/Kconfig
create mode 100644 rust/kernel/hid.rs
diff --git a/MAINTAINERS b/MAINTAINERS
index cd7ff55b5d32..dc597bfe1a54 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10850,6 +10850,14 @@ F: include/uapi/linux/hid*
F: samples/hid/
F: tools/testing/selftests/hid/
+HID CORE LAYER [RUST]
+M: Rahul Rameshbabu <sergeantsagara@protonmail.com>
+R: Benjamin Tissoires <bentiss@kernel.org>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/hid/rust/*.rs
+F: rust/kernel/hid.rs
+
HID LOGITECH DRIVERS
R: Filipe Laíns <lains@riseup.net>
L: linux-input@vger.kernel.org
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 79997553d8f9..073771d95966 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1420,6 +1420,8 @@ endmenu
source "drivers/hid/bpf/Kconfig"
+source "drivers/hid/rust/Kconfig"
+
source "drivers/hid/i2c-hid/Kconfig"
source "drivers/hid/intel-ish-hid/Kconfig"
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 10ae5dedbd84..f4a0fea769c3 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -7,6 +7,8 @@ hid-$(CONFIG_DEBUG_FS) += hid-debug.o
obj-$(CONFIG_HID_BPF) += bpf/
+obj-$(CONFIG_RUST_HID_ABSTRACTIONS) += rust/
+
obj-$(CONFIG_HID) += hid.o
obj-$(CONFIG_UHID) += uhid.o
diff --git a/drivers/hid/rust/Kconfig b/drivers/hid/rust/Kconfig
new file mode 100644
index 000000000000..d3247651829e
--- /dev/null
+++ b/drivers/hid/rust/Kconfig
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0-only
+menu "Rust HID support"
+
+config RUST_HID_ABSTRACTIONS
+ bool "Rust HID abstractions support"
+ depends on RUST
+ depends on HID=y
+ help
+ Adds support needed for HID drivers written in Rust. It provides a
+ wrapper around the C hid core.
+
+endmenu # Rust HID support
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 4ad9add117ea..773468788d98 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -46,6 +46,7 @@
#include <linux/cpufreq.h>
#include <linux/cpumask.h>
#include <linux/cred.h>
+#include <linux/device.h>
#include <linux/device/faux.h>
#include <linux/dma-mapping.h>
#include <linux/errname.h>
@@ -53,6 +54,8 @@
#include <linux/file.h>
#include <linux/firmware.h>
#include <linux/fs.h>
+#include <linux/hid.h>
+#include "../../drivers/hid/hid-ids.h"
#include <linux/ioport.h>
#include <linux/jiffies.h>
#include <linux/jump_label.h>
diff --git a/rust/kernel/hid.rs b/rust/kernel/hid.rs
new file mode 100644
index 000000000000..7bb136a1e753
--- /dev/null
+++ b/rust/kernel/hid.rs
@@ -0,0 +1,513 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Rahul Rameshbabu <sergeantsagara@protonmail.com>
+
+//! Abstractions for the HID interface.
+//!
+//! C header: [`include/linux/hid.h`](srctree/include/linux/hid.h)
+
+use crate::{
+ device,
+ device_id::{RawDeviceId, RawDeviceIdIndex},
+ driver,
+ error::*,
+ prelude::*,
+ types::Opaque
+};
+use core::{
+ marker::PhantomData,
+ ptr::{addr_of_mut, NonNull},
+};
+
+/// Indicates the item is static read-only.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_CONSTANT: u8 = bindings::HID_MAIN_ITEM_CONSTANT as u8;
+
+/// Indicates the item represents data from a physical control.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_VARIABLE: u8 = bindings::HID_MAIN_ITEM_VARIABLE as u8;
+
+/// Indicates the item should be treated as a relative change from the previous
+/// report.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_RELATIVE: u8 = bindings::HID_MAIN_ITEM_RELATIVE as u8;
+
+/// Indicates the item should wrap around when reaching the extreme high or
+/// extreme low values.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_WRAP: u8 = bindings::HID_MAIN_ITEM_WRAP as u8;
+
+/// Indicates the item should wrap around when reaching the extreme high or
+/// extreme low values.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_NONLINEAR: u8 = bindings::HID_MAIN_ITEM_NONLINEAR as u8;
+
+/// Indicates whether the control has a preferred state it will physically
+/// return to without user intervention.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_NO_PREFERRED: u8 = bindings::HID_MAIN_ITEM_NO_PREFERRED as u8;
+
+/// Indicates whether the control has a physical state where it will not send
+/// any reports.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_NULL_STATE: u8 = bindings::HID_MAIN_ITEM_NULL_STATE as u8;
+
+/// Indicates whether the control requires host system logic to change state.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_VOLATILE: u8 = bindings::HID_MAIN_ITEM_VOLATILE as u8;
+
+/// Indicates whether the item is fixed size or a variable buffer of bytes.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_BUFFERED_BYTE: u8 = bindings::HID_MAIN_ITEM_BUFFERED_BYTE as u8;
+
+/// HID device groups are intended to help categories HID devices based on a set
+/// of common quirks and logic that they will require to function correctly.
+#[repr(u16)]
+pub enum Group {
+ /// Used to match a device against any group when probing.
+ Any = bindings::HID_GROUP_ANY as u16,
+
+ /// Indicates a generic device that should need no custom logic from the
+ /// core HID stack.
+ Generic = bindings::HID_GROUP_GENERIC as u16,
+
+ /// Maps multitouch devices to hid-multitouch instead of hid-generic.
+ Multitouch = bindings::HID_GROUP_MULTITOUCH as u16,
+
+ /// Used for autodetecing and mapping of HID sensor hubs to
+ /// hid-sensor-hub.
+ SensorHub = bindings::HID_GROUP_SENSOR_HUB as u16,
+
+ /// Used for autodetecing and mapping Win 8 multitouch devices to set the
+ /// needed quirks.
+ MultitouchWin8 = bindings::HID_GROUP_MULTITOUCH_WIN_8 as u16,
+
+ // Vendor-specific device groups.
+ /// Used to distinguish Synpatics touchscreens from other products. The
+ /// touchscreens will be handled by hid-multitouch instead, while everything
+ /// else will be managed by hid-rmi.
+ RMI = bindings::HID_GROUP_RMI as u16,
+
+ /// Used for hid-core handling to automatically identify Wacom devices and
+ /// have them probed by hid-wacom.
+ Wacom = bindings::HID_GROUP_WACOM as u16,
+
+ /// Used by logitech-djreceiver and logitech-djdevice to autodetect if
+ /// devices paied to the DJ receivers are DJ devices and handle them with
+ /// the device driver.
+ LogitechDJDevice = bindings::HID_GROUP_LOGITECH_DJ_DEVICE as u16,
+
+ /// Since the Valve Steam Controller only has vendor-specific usages,
+ /// prevent hid-generic from parsing its reports since there would be
+ /// nothing hid-generic could do for the device.
+ Steam = bindings::HID_GROUP_STEAM as u16,
+
+ /// Used to differentiate 27 Mhz frequency Logitech DJ devices from other
+ /// Logitech DJ devices.
+ Logitech27MHzDevice = bindings::HID_GROUP_LOGITECH_27MHZ_DEVICE as u16,
+
+ /// Used for autodetecting and mapping Vivaldi devices to hid-vivaldi.
+ Vivaldi = bindings::HID_GROUP_VIVALDI as u16,
+}
+
+// TODO: use `const_trait_impl` once stabilized:
+//
+// ```
+// impl const From<Group> for u16 {
+// /// [`Group`] variants are represented by [`u16`] values.
+// fn from(value: Group) -> Self {
+// value as Self
+// }
+// }
+// ```
+impl Group {
+ /// Internal function used to convert [`Group`] variants into [`u16`].
+ const fn into(self) -> u16 {
+ self as u16
+ }
+}
+
+impl From<u16> for Group {
+ /// [`u16`] values can be safely converted to [`Group`] variants.
+ fn from(value: u16) -> Self {
+ match value.into() {
+ bindings::HID_GROUP_GENERIC => Group::Generic,
+ bindings::HID_GROUP_MULTITOUCH => Group::Multitouch,
+ bindings::HID_GROUP_SENSOR_HUB => Group::SensorHub,
+ bindings::HID_GROUP_MULTITOUCH_WIN_8 => Group::MultitouchWin8,
+ bindings::HID_GROUP_RMI => Group::RMI,
+ bindings::HID_GROUP_WACOM => Group::Wacom,
+ bindings::HID_GROUP_LOGITECH_DJ_DEVICE => Group::LogitechDJDevice,
+ bindings::HID_GROUP_STEAM => Group::Steam,
+ bindings::HID_GROUP_LOGITECH_27MHZ_DEVICE => Group::Logitech27MHzDevice,
+ bindings::HID_GROUP_VIVALDI => Group::Vivaldi,
+ _ => Group::Any,
+ }
+ }
+}
+
+/// The HID device representation.
+///
+/// This structure represents the Rust abstraction for a C `struct hid_device`.
+/// The implementation abstracts the usage of an already existing C `struct
+/// hid_device` within Rust code that we get passed from the C side.
+///
+/// # Invariants
+///
+/// A [`Device`] instance represents a valid `struct hid_device` created by the
+/// C portion of the kernel.
+#[repr(transparent)]
+pub struct Device<Ctx: device::DeviceContext = device::Normal>(
+ Opaque<bindings::hid_device>,
+ PhantomData<Ctx>,
+);
+
+impl<Ctx: device::DeviceContext> Device<Ctx> {
+ fn as_raw(&self) -> *mut bindings::hid_device {
+ self.0.get()
+ }
+
+ /// Returns the HID transport bus ID.
+ pub fn bus(&self) -> u16 {
+ // SAFETY: `self.as_raw` is a valid pointer to a `struct hid_device`
+ unsafe { *self.as_raw() }.bus
+ }
+
+ /// Returns the HID report group.
+ pub fn group(&self) -> Group {
+ // SAFETY: `self.as_raw` is a valid pointer to a `struct hid_device`
+ unsafe { *self.as_raw() }.group.into()
+ }
+
+ /// Returns the HID vendor ID.
+ pub fn vendor(&self) -> u32 {
+ // SAFETY: `self.as_raw` is a valid pointer to a `struct hid_device`
+ unsafe { *self.as_raw() }.vendor
+ }
+
+ /// Returns the HID product ID.
+ pub fn product(&self) -> u32 {
+ // SAFETY: `self.as_raw` is a valid pointer to a `struct hid_device`
+ unsafe { *self.as_raw() }.product
+ }
+}
+
+// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
+// argument.
+kernel::impl_device_context_deref!(unsafe { Device });
+kernel::impl_device_context_into_aref!(Device);
+
+// SAFETY: Instances of `Device` are always reference-counted.
+unsafe impl crate::types::AlwaysRefCounted for Device {
+ fn inc_ref(&self) {
+ // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
+ unsafe { bindings::get_device(&raw mut (*self.as_raw()).dev) };
+ }
+
+ unsafe fn dec_ref(obj: NonNull<Self>) {
+ // SAFETY: The safety requirements guarantee that the refcount is non-zero.
+ unsafe { bindings::put_device(&raw mut (*obj.cast::<bindings::hid_device>().as_ptr()).dev) }
+ }
+}
+
+impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
+ fn as_ref(&self) -> &device::Device<Ctx> {
+ // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
+ // `struct hid_device`.
+ let dev = unsafe { addr_of_mut!((*self.as_raw()).dev) };
+
+ // SAFETY: `dev` points to a valid `struct device`.
+ unsafe { device::Device::from_raw(dev) }
+ }
+}
+
+/// Abstraction for the HID device ID structure `struct hid_device_id`.
+#[repr(transparent)]
+#[derive(Clone, Copy)]
+pub struct DeviceId(bindings::hid_device_id);
+
+impl DeviceId {
+ /// Equivalent to C's `HID_USB_DEVICE` macro.
+ ///
+ /// Create a new `hid::DeviceId` from a group, vendor ID, and device ID
+ /// number.
+ pub const fn new_usb(group: Group, vendor: u32, product: u32) -> Self {
+ Self(bindings::hid_device_id {
+ bus: 0x3, // BUS_USB
+ group: group.into(),
+ vendor,
+ product,
+ driver_data: 0,
+ })
+ }
+
+ /// Returns the HID transport bus ID.
+ pub fn bus(&self) -> u16 {
+ self.0.bus
+ }
+
+ /// Returns the HID report group.
+ pub fn group(&self) -> Group {
+ self.0.group.into()
+ }
+
+ /// Returns the HID vendor ID.
+ pub fn vendor(&self) -> u32 {
+ self.0.vendor
+ }
+
+ /// Returns the HID product ID.
+ pub fn product(&self) -> u32 {
+ self.0.product
+ }
+}
+
+// SAFETY:
+// * `DeviceId` is a `#[repr(transparent)` wrapper of `hid_device_id` and does not add
+// additional invariants, so it's safe to transmute to `RawType`.
+// * `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
+unsafe impl RawDeviceId for DeviceId {
+ type RawType = bindings::hid_device_id;
+}
+
+// SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
+unsafe impl RawDeviceIdIndex for DeviceId {
+ const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::hid_device_id, driver_data);
+
+ fn index(&self) -> usize {
+ self.0.driver_data
+ }
+}
+
+/// [`IdTable`] type for HID.
+pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<DeviceId, T>;
+
+/// Create a HID [`IdTable`] with its alias for modpost.
+#[macro_export]
+macro_rules! hid_device_table {
+ ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
+ const $table_name: $crate::device_id::IdArray<
+ $crate::hid::DeviceId,
+ $id_info_type,
+ { $table_data.len() },
+ > = $crate::device_id::IdArray::new($table_data);
+
+ $crate::module_device_table!("hid", $module_table_name, $table_name);
+ };
+}
+
+/// The HID driver trait.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::{bindings, device, hid};
+///
+/// struct MyDriver;
+///
+/// kernel::hid_device_table!(
+/// HID_TABLE,
+/// MODULE_HID_TABLE,
+/// <MyDriver as hid::Driver>::IdInfo,
+/// [(
+/// hid::DeviceId::new_usb(
+/// hid::Group::Steam,
+/// bindings::USB_VENDOR_ID_VALVE,
+/// bindings::USB_DEVICE_ID_STEAM_DECK,
+/// ),
+/// (),
+/// )]
+/// );
+///
+/// #[vtable]
+/// impl hid::Driver for MyDriver {
+/// type IdInfo = ();
+/// const ID_TABLE: hid::IdTable<Self::IdInfo> = &HID_TABLE;
+///
+/// /// This function is optional to implement.
+/// fn report_fixup<'a, 'b: 'a>(_hdev: &hid::Device<device::Core>, rdesc: &'b mut [u8]) -> &'a [u8] {
+/// // Perform some report descriptor fixup.
+/// rdesc
+/// }
+/// }
+/// ```
+/// Drivers must implement this trait in order to get a HID driver registered.
+/// Please refer to the `Adapter` documentation for an example.
+#[vtable]
+pub trait Driver: Send {
+ /// The type holding information about each device id supported by the driver.
+ // TODO: Use `associated_type_defaults` once stabilized:
+ //
+ // ```
+ // type IdInfo: 'static = ();
+ // ```
+ type IdInfo: 'static;
+
+ /// The table of device ids supported by the driver.
+ const ID_TABLE: IdTable<Self::IdInfo>;
+
+ /// Called before report descriptor parsing. Can be used to mutate the
+ /// report descriptor before the core HID logic processes the descriptor.
+ /// Useful for problematic report descriptors that prevent HID devices from
+ /// functioning correctly.
+ ///
+ /// Optional to implement.
+ fn report_fixup<'a, 'b: 'a>(_hdev: &Device<device::Core>, _rdesc: &'b mut [u8]) -> &'a [u8] {
+ build_error!(VTABLE_DEFAULT_ERROR)
+ }
+}
+
+/// An adapter for the registration of HID drivers.
+pub struct Adapter<T: Driver>(T);
+
+// SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if
+// a preceding call to `register` has been successful.
+unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
+ type RegType = bindings::hid_driver;
+
+ unsafe fn register(
+ hdrv: &Opaque<Self::RegType>,
+ name: &'static CStr,
+ module: &'static ThisModule,
+ ) -> Result {
+ // SAFETY: It's safe to set the fields of `struct hid_driver` on initialization.
+ unsafe {
+ (*hdrv.get()).name = name.as_char_ptr();
+ (*hdrv.get()).id_table = T::ID_TABLE.as_ptr();
+ (*hdrv.get()).report_fixup = if T::HAS_REPORT_FIXUP {
+ Some(Self::report_fixup_callback)
+ } else {
+ None
+ };
+ }
+
+ // SAFETY: `hdrv` is guaranteed to be a valid `RegType`
+ to_result(unsafe {
+ bindings::__hid_register_driver(hdrv.get(), module.0, name.as_char_ptr())
+ })
+ }
+
+ unsafe fn unregister(hdrv: &Opaque<Self::RegType>) {
+ // SAFETY: `hdrv` is guaranteed to be a valid `RegType`
+ unsafe { bindings::hid_unregister_driver(hdrv.get()) }
+ }
+}
+
+impl<T: Driver + 'static> Adapter<T> {
+ extern "C" fn report_fixup_callback(
+ hdev: *mut bindings::hid_device,
+ buf: *mut u8,
+ size: *mut kernel::ffi::c_uint,
+ ) -> *const u8 {
+ // SAFETY: The HID subsystem only ever calls the report_fixup callback
+ // with a valid pointer to a `struct hid_device`.
+ //
+ // INVARIANT: `hdev` is valid for the duration of
+ // `report_fixup_callback()`.
+ let hdev = unsafe { &*hdev.cast::<Device<device::Core>>() };
+
+ // SAFETY: The HID subsystem only ever calls the report_fixup callback
+ // with a valid pointer to a `kernel::ffi::c_uint`.
+ //
+ // INVARIANT: `size` is valid for the duration of
+ // `report_fixup_callback()`.
+ let buf_len: usize = match unsafe { *size }.try_into() {
+ Ok(len) => len,
+ Err(e) => {
+ dev_err!(
+ hdev.as_ref(),
+ "Cannot fix report description due to {}!\n",
+ e
+ );
+
+ return buf;
+ }
+ };
+
+ // Build a mutable Rust slice from `buf` and `size`.
+ //
+ // SAFETY: The HID subsystem only ever calls the `report_fixup callback`
+ // with a valid pointer to a `u8` buffer.
+ //
+ // INVARIANT: `buf` is valid for the duration of
+ // `report_fixup_callback()`.
+ let rdesc_slice = unsafe { core::slice::from_raw_parts_mut(buf, buf_len) };
+ let rdesc_slice = T::report_fixup(hdev, rdesc_slice);
+
+ match rdesc_slice.len().try_into() {
+ // SAFETY: The HID subsystem only ever calls the report_fixup
+ // callback with a valid pointer to a `kernel::ffi::c_uint`.
+ //
+ // INVARIANT: `size` is valid for the duration of
+ // `report_fixup_callback()`.
+ Ok(len) => unsafe { *size = len },
+ Err(e) => {
+ dev_err!(
+ hdev.as_ref(),
+ "Fixed report description will not be used due to {}!\n",
+ e
+ );
+
+ return buf;
+ }
+ }
+
+ rdesc_slice.as_ptr()
+ }
+}
+
+/// Declares a kernel module that exposes a single HID driver.
+///
+/// # Examples
+///
+/// ```ignore
+/// kernel::module_hid_driver! {
+/// type: MyDriver,
+/// name: "Module name",
+/// authors: ["Author name"],
+/// description: "Description",
+/// license: "GPL",
+/// }
+/// ```
+#[macro_export]
+macro_rules! module_hid_driver {
+ ($($f:tt)*) => {
+ $crate::module_driver!(<T>, $crate::hid::Adapter<T>, { $($f)* });
+ };
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 733f4d4e9bae..1e50912e81f6 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -89,6 +89,8 @@
pub mod firmware;
pub mod fmt;
pub mod fs;
+#[cfg(CONFIG_RUST_HID_ABSTRACTIONS)]
+pub mod hid;
pub mod init;
pub mod io;
pub mod ioctl;
--
2.51.0
^ permalink raw reply related
* [PATCH v5 2/2] rust: hid: Glorious PC Gaming Race Model O and O- mice reference driver
From: Rahul Rameshbabu @ 2025-09-18 7:08 UTC (permalink / raw)
To: linux-input, linux-kernel, rust-for-linux
Cc: a.hindborg, alex.gaynor, aliceryhl, benjamin.tissoires,
benno.lossin, bjorn3_gh, boqun.feng, dakr, db48x, gary, jikos,
ojeda, peter.hutterer, tmgross, Rahul Rameshbabu
In-Reply-To: <20250918070824.70822-1-sergeantsagara@protonmail.com>
Demonstrate how to perform a report fixup from a Rust HID driver. The mice
specify the const flag incorrectly in the consumer input report descriptor,
which leads to inputs being ignored. Correctly patch the report descriptor
for the Model O and O- mice.
Portions of the HID report post-fixup:
device 0:0
...
0x81, 0x06, // Input (Data,Var,Rel) 84
...
0x81, 0x06, // Input (Data,Var,Rel) 112
...
0x81, 0x06, // Input (Data,Var,Rel) 140
Signed-off-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>
---
Notes:
Changelog:
v4->v5:
* NONE
v3->v4:
* Removed specifying tree in MAINTAINERS file since that is up for
debate
* Minor rebase cleanup
* Moved driver logic under drivers/hid/rust
* Use hex instead of decimal for the report descriptor comparisons
v2->v3:
* Fixed docstring formatting
* Updated MAINTAINERS file based on v1 and v2 discussion
v1->v2:
* Use vendor id and device id from drivers/hid/hid-ids.h bindings
* Make use for dev_err! as appropriate
MAINTAINERS | 6 +++
drivers/hid/hid-glorious.c | 2 +
drivers/hid/hid_glorious_rust.rs | 60 +++++++++++++++++++++++++++
drivers/hid/rust/Kconfig | 16 +++++++
drivers/hid/rust/Makefile | 6 +++
drivers/hid/rust/hid_glorious_rust.rs | 60 +++++++++++++++++++++++++++
6 files changed, 150 insertions(+)
create mode 100644 drivers/hid/hid_glorious_rust.rs
create mode 100644 drivers/hid/rust/Makefile
create mode 100644 drivers/hid/rust/hid_glorious_rust.rs
diff --git a/MAINTAINERS b/MAINTAINERS
index dc597bfe1a54..ad2f071efbaa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10362,6 +10362,12 @@ L: platform-driver-x86@vger.kernel.org
S: Maintained
F: drivers/platform/x86/gigabyte-wmi.c
+GLORIOUS RUST DRIVER [RUST]
+M: Rahul Rameshbabu <sergeantsagara@protonmail.com>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/hid/rust/hid_glorious_rust.rs
+
GNSS SUBSYSTEM
M: Johan Hovold <johan@kernel.org>
S: Maintained
diff --git a/drivers/hid/hid-glorious.c b/drivers/hid/hid-glorious.c
index 5bbd81248053..d7362852c20f 100644
--- a/drivers/hid/hid-glorious.c
+++ b/drivers/hid/hid-glorious.c
@@ -76,8 +76,10 @@ static int glorious_probe(struct hid_device *hdev,
}
static const struct hid_device_id glorious_devices[] = {
+#if !IS_ENABLED(CONFIG_HID_GLORIOUS_RUST)
{ HID_USB_DEVICE(USB_VENDOR_ID_SINOWEALTH,
USB_DEVICE_ID_GLORIOUS_MODEL_O) },
+#endif
{ HID_USB_DEVICE(USB_VENDOR_ID_SINOWEALTH,
USB_DEVICE_ID_GLORIOUS_MODEL_D) },
{ HID_USB_DEVICE(USB_VENDOR_ID_LAVIEW,
diff --git a/drivers/hid/hid_glorious_rust.rs b/drivers/hid/hid_glorious_rust.rs
new file mode 100644
index 000000000000..8cffc1c605dd
--- /dev/null
+++ b/drivers/hid/hid_glorious_rust.rs
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Rahul Rameshbabu <sergeantsagara@protonmail.com>
+
+//! Rust reference HID driver for Glorious Model O and O- mice.
+
+use kernel::{self, bindings, device, hid, prelude::*};
+
+struct GloriousRust;
+
+kernel::hid_device_table!(
+ HID_TABLE,
+ MODULE_HID_TABLE,
+ <GloriousRust as hid::Driver>::IdInfo,
+ [(
+ hid::DeviceId::new_usb(
+ hid::Group::Generic,
+ bindings::USB_VENDOR_ID_SINOWEALTH,
+ bindings::USB_DEVICE_ID_GLORIOUS_MODEL_O,
+ ),
+ (),
+ )]
+);
+
+#[vtable]
+impl hid::Driver for GloriousRust {
+ type IdInfo = ();
+ const ID_TABLE: hid::IdTable<Self::IdInfo> = &HID_TABLE;
+
+ /// Fix the Glorious Model O and O- consumer input report descriptor to use
+ /// the variable and relative flag, while clearing the const flag.
+ ///
+ /// Without this fixup, inputs from the mice will be ignored.
+ fn report_fixup<'a, 'b: 'a>(hdev: &hid::Device<device::Core>, rdesc: &'b mut [u8]) -> &'a [u8] {
+ if rdesc.len() == 213
+ && (rdesc[84] == 129 && rdesc[85] == 3)
+ && (rdesc[112] == 129 && rdesc[113] == 3)
+ && (rdesc[140] == 129 && rdesc[141] == 3)
+ {
+ dev_info!(
+ hdev.as_ref(),
+ "patching Glorious Model O consumer control report descriptor\n"
+ );
+
+ rdesc[85] = hid::MAIN_ITEM_VARIABLE | hid::MAIN_ITEM_RELATIVE;
+ rdesc[113] = hid::MAIN_ITEM_VARIABLE | hid::MAIN_ITEM_RELATIVE;
+ rdesc[141] = hid::MAIN_ITEM_VARIABLE | hid::MAIN_ITEM_RELATIVE;
+ }
+
+ rdesc
+ }
+}
+
+kernel::module_hid_driver! {
+ type: GloriousRust,
+ name: "GloriousRust",
+ authors: ["Rahul Rameshbabu <sergeantsagara@protonmail.com>"],
+ description: "Rust reference HID driver for Glorious Model O and O- mice",
+ license: "GPL",
+}
diff --git a/drivers/hid/rust/Kconfig b/drivers/hid/rust/Kconfig
index d3247651829e..d7a1bf26bed0 100644
--- a/drivers/hid/rust/Kconfig
+++ b/drivers/hid/rust/Kconfig
@@ -9,4 +9,20 @@ config RUST_HID_ABSTRACTIONS
Adds support needed for HID drivers written in Rust. It provides a
wrapper around the C hid core.
+if RUST_HID_ABSTRACTIONS
+
+menu "Special HID drivers"
+
+config HID_GLORIOUS_RUST
+ tristate "Glorious O and O- mice Rust reference driver"
+ depends on USB_HID
+ depends on RUST_HID_ABSTRACTIONS
+ help
+ Support for Glorious PC Gaming Race O and O- mice
+ in Rust.
+
+endmenu # Special HID drivers
+
+endif # RUST_HID_ABSTRACTIONS
+
endmenu # Rust HID support
diff --git a/drivers/hid/rust/Makefile b/drivers/hid/rust/Makefile
new file mode 100644
index 000000000000..6676030a2f87
--- /dev/null
+++ b/drivers/hid/rust/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for Rust HID support
+#
+
+obj-$(CONFIG_HID_GLORIOUS_RUST) += hid_glorious_rust.o
diff --git a/drivers/hid/rust/hid_glorious_rust.rs b/drivers/hid/rust/hid_glorious_rust.rs
new file mode 100644
index 000000000000..dfc3f2323b60
--- /dev/null
+++ b/drivers/hid/rust/hid_glorious_rust.rs
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Rahul Rameshbabu <sergeantsagara@protonmail.com>
+
+//! Rust reference HID driver for Glorious Model O and O- mice.
+
+use kernel::{self, bindings, device, hid, prelude::*};
+
+struct GloriousRust;
+
+kernel::hid_device_table!(
+ HID_TABLE,
+ MODULE_HID_TABLE,
+ <GloriousRust as hid::Driver>::IdInfo,
+ [(
+ hid::DeviceId::new_usb(
+ hid::Group::Generic,
+ bindings::USB_VENDOR_ID_SINOWEALTH,
+ bindings::USB_DEVICE_ID_GLORIOUS_MODEL_O,
+ ),
+ (),
+ )]
+);
+
+#[vtable]
+impl hid::Driver for GloriousRust {
+ type IdInfo = ();
+ const ID_TABLE: hid::IdTable<Self::IdInfo> = &HID_TABLE;
+
+ /// Fix the Glorious Model O and O- consumer input report descriptor to use
+ /// the variable and relative flag, while clearing the const flag.
+ ///
+ /// Without this fixup, inputs from the mice will be ignored.
+ fn report_fixup<'a, 'b: 'a>(hdev: &hid::Device<device::Core>, rdesc: &'b mut [u8]) -> &'a [u8] {
+ if rdesc.len() == 213
+ && (rdesc[84] == 0x81 && rdesc[85] == 0x3)
+ && (rdesc[112] == 0x81 && rdesc[113] == 0x3)
+ && (rdesc[140] == 0x81 && rdesc[141] == 0x3)
+ {
+ dev_info!(
+ hdev.as_ref(),
+ "patching Glorious Model O consumer control report descriptor\n"
+ );
+
+ rdesc[85] = hid::MAIN_ITEM_VARIABLE | hid::MAIN_ITEM_RELATIVE;
+ rdesc[113] = hid::MAIN_ITEM_VARIABLE | hid::MAIN_ITEM_RELATIVE;
+ rdesc[141] = hid::MAIN_ITEM_VARIABLE | hid::MAIN_ITEM_RELATIVE;
+ }
+
+ rdesc
+ }
+}
+
+kernel::module_hid_driver! {
+ type: GloriousRust,
+ name: "GloriousRust",
+ authors: ["Rahul Rameshbabu <sergeantsagara@protonmail.com>"],
+ description: "Rust reference HID driver for Glorious Model O and O- mice",
+ license: "GPL",
+}
--
2.51.0
^ permalink raw reply related
* [bug report] HID: haptic: initialize haptic device
From: Dan Carpenter @ 2025-09-18 7:18 UTC (permalink / raw)
To: Angela Czubak; +Cc: linux-input
Hello Angela Czubak,
Commit 344ff3584957 ("HID: haptic: initialize haptic device") from
Aug 18, 2025 (linux-next), leads to the following Smatch static
checker warning:
drivers/hid/hid-haptic.c:528 hid_haptic_init()
warn: missing error code here? '_dev_err()' failed. 'ret' = '0'
drivers/hid/hid-haptic.c
518 }
519
520 ff = dev->ff;
521 ff->private = haptic;
522 ff->upload = hid_haptic_upload_effect;
523 ff->playback = hid_haptic_playback;
524 ff->erase = hid_haptic_erase;
525 ff->destroy = hid_haptic_destroy;
526 if (!try_module_get(THIS_MODULE)) {
527 dev_err(&hdev->dev, "Failed to increase module count.\n");
--> 528 goto input_free;
Missing error code. I think we're trying to pump the module count so
this module is unloadable. That's a discouraged thing so the
__module_get() function has a double underscore. But here we're kind
of dressing it up so it looks like we're doing a legit module count
bump of a different module that we rely on (instead of THIS_MODULE). We
should just use __module_get() because it's more honest and remove the
check.
529 }
530 if (!get_device(&hdev->dev)) {
531 dev_err(&hdev->dev, "Failed to get hdev device.\n");
532 module_put(THIS_MODULE);
533 goto input_free;
Missing error code, but get_device() can't really fail here. Just remove
the check.
534 }
535 return 0;
536
537 input_free:
538 input_ff_destroy(dev);
539 /* Do not let double free happen, input_ff_destroy will call
540 * hid_haptic_destroy.
541 */
542 *haptic_ptr = NULL;
543 /* Restore dev flush and event */
544 dev->flush = flush;
545 dev->event = event;
546 return ret;
547 stop_buffer_free:
548 kfree(haptic->stop_effect.report_buf);
549 haptic->stop_effect.report_buf = NULL;
550 buffer_free:
551 while (--r >= 0)
552 kfree(haptic->effect[r].report_buf);
553 kfree(haptic->effect);
554 haptic->effect = NULL;
555 output_queue:
556 destroy_workqueue(haptic->wq);
557 haptic->wq = NULL;
558 duration_map:
559 kfree(haptic->duration_map);
560 haptic->duration_map = NULL;
561 usage_map:
562 kfree(haptic->hid_usage_map);
563 haptic->hid_usage_map = NULL;
564 exit:
565 return ret;
566 }
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH v4 4/8] mfd: mc13xxx: Use devm_mfd_add_devices and devm_regmap_add_irq_chip
From: Alexander Kurz @ 2025-09-18 7:25 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dzmitry Sankouski, Dr. David Alan Gilbert, Heiko Stuebner,
Uwe Kleine-König, devicetree, linux-input, linux-kernel
In-Reply-To: <fqhldiqylcsp6kp4tvhopxekgszabbemnvbseygkqaipgp5mhl@wtz6c7kjloko>
Hi Dimitry
On Tue, 16 Sep 2025, Dmitry Torokhov wrote:
> Hi Alexander,
>
> On Sun, Sep 14, 2025 at 07:37:19PM +0000, Alexander Kurz wrote:
> > Use devm_mfd_add_devices() for adding MFD child devices and
> > devm_regmap_add_irq_chip() for IRQ chip registration.
> >
> > This reduces the amount of required cleanup.
> >
> > Signed-off-by: Alexander Kurz <akurz@blala.de>
> > ---
> > drivers/mfd/mc13xxx-core.c | 9 ++++-----
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c
> > index 920797b806ce..091c9171b2b7 100644
> > --- a/drivers/mfd/mc13xxx-core.c
> > +++ b/drivers/mfd/mc13xxx-core.c
> > @@ -381,7 +381,7 @@ static int mc13xxx_add_subdevice_pdata(struct mc13xxx *mc13xxx,
> > if (!cell.name)
> > return -ENOMEM;
> >
> > - return mfd_add_devices(mc13xxx->dev, -1, &cell, 1, NULL, 0,
> > + return devm_mfd_add_devices(mc13xxx->dev, -1, &cell, 1, NULL, 0,
> > regmap_irq_get_domain(mc13xxx->irq_data));
> > }
> >
> > @@ -455,8 +455,9 @@ int mc13xxx_common_init(struct device *dev)
> > mc13xxx->irq_chip.irqs = mc13xxx->irqs;
> > mc13xxx->irq_chip.num_irqs = ARRAY_SIZE(mc13xxx->irqs);
> >
> > - ret = regmap_add_irq_chip(mc13xxx->regmap, mc13xxx->irq, IRQF_ONESHOT,
> > - 0, &mc13xxx->irq_chip, &mc13xxx->irq_data);
> > + ret = devm_regmap_add_irq_chip(dev, mc13xxx->regmap, mc13xxx->irq,
> > + IRQF_ONESHOT, 0, &mc13xxx->irq_chip,
> > + &mc13xxx->irq_data);
> > if (ret)
> > return ret;
> >
> > @@ -502,8 +503,6 @@ void mc13xxx_common_exit(struct device *dev)
> > {
> > struct mc13xxx *mc13xxx = dev_get_drvdata(dev);
> >
> > - mfd_remove_devices(dev);
> > - regmap_del_irq_chip(mc13xxx->irq, mc13xxx->irq_data);
> > mutex_destroy(&mc13xxx->lock);
>
> This causes the mutex be destroyed while the sub-devices are still
> present. The power button will try to call mc13xxx_lock() and
> mc13xxx_unlock() and of mutex debugging is enabled you'll get errors.
Thanks for noting this, actually I have introduced this change
in v4 of this series.
> I'd remove mutex_destroy() as well (and transitively get rid of
> mc13xxx_common_exit()) and then look into getting rid of mc13xxx_lock()
> and mc13xxx_unlock() because, as I mentioned in another email, they are
> IMO not needed.
I would prefer not to extend the scope of this series even further
and just drop this patch for v5.
There are still more issues todo with mc13xxx, e.g. mc13xxx-led does
not work since commit 78efa53e715e ("leds: Init leds class earlier").
Cleaning up potentially legacy mutex is just one more topic on this list.
>
> But this version of the patch is broken as far as I can tell.
>
> Thanks.
>
> --
> Dmitry
>
Thanks, Alexander
^ permalink raw reply
* Re: [PATCH v3 1/3] input: mouse: trackpoint: Add doubletap enable/disable support
From: Hans de Goede @ 2025-09-18 7:31 UTC (permalink / raw)
To: Vishnu Sankar, dmitry.torokhov, hmh, ilpo.jarvinen,
derekjohn.clark
Cc: mpearson-lenovo, linux-input, linux-kernel, ibm-acpi-devel,
platform-driver-x86, vsankar
In-Reply-To: <CABxCQKtEcFozTtuV3sutU3OyobTbpA82Uy=MyU0FQePPT7S2Wg@mail.gmail.com>
Hi Vishnu,
On 18-Sep-25 4:37 AM, Vishnu Sankar wrote:
> Hello all,
>
> Do we have any questions or concerns?
> Thanks in advance!
>
> On Mon, Sep 1, 2025 at 10:53 PM Vishnu Sankar <vishnuocv@gmail.com> wrote:
>>
>> Add support for enabling and disabling doubletap on TrackPoint devices
>> that support this functionality. The feature is detected using firmware
>> ID and exposed via sysfs as `doubletap_enabled`.
Hmm, you seem to be using a firmware ID prefix match, combined with
a deny list of some firmware IDs with that prefix which do not support
this. How do we know this deny list is complete?
Also as Dmitry says you really should use the is_visible() callback
to not show the attribute at all on unsupported systems.
>> The feature is only available on newer ThinkPads (2023 and later).The driver
>> exposes this capability via a new sysfs attribute:
>> "/sys/bus/serio/devices/seriox/doubletap_enabled".
>>
>> The attribute is only created if the device is detected to be capable of
>> doubletap via firmware and variant ID checks. This functionality will be
>> used by platform drivers such as thinkpad_acpi to expose and control doubletap
>> via user interfaces.
Hmm, you refer to thinkpad_acpi as a possible consumer of this
functionality. But you only add a sysfs interface.
thinkpad_acpi will need some in kernel interface to use this.
Which brings me to my main question: thinkpad_acpi is the driver
receiving the doubletap events since these are send out-of-bound
and not through the ps/2 trackpoint protocol.
thinkpad_acpi already has the capability to filter out these doubletap
events and report nothing. Why is it necessary / better to disable
the doubletap at the trackpoint fw-level, rather then just filtering
it at the thinkpad_acpi level ?
I don't really see a big advantage in filtering these events at
the fw-level rather then in the kernel and we already have the
in kernel filtering.
Since this is highly ThinkPad specific it seems that the current
handling in thinkpad_acpi also logically is the best place to
handle this.
What new use-cases if any does this enable?
If you e.g. want some Lenovo specific control-panel GUI to
enable/disable this, why not expose the existing filtering
in thinkpad_acpi (which is hotkey controller only atm)
in sysfs through thinkpad_acpi ?
If we go the route of using the trackpoint fw-level filtering
as is done in this patch, then IMHO we really also should
make the existing code in thinkpad_acpi:
static bool hotkey_notify_8xxx(const u32 hkey, bool *send_acpi_ev)
{
switch (hkey) {
case TP_HKEY_EV_TRACK_DOUBLETAP:
if (tp_features.trackpoint_doubletap)
tpacpi_input_send_key(hkey, send_acpi_ev);
return true;
default:
return false;
}
}
static bool tpacpi_driver_event(const unsigned int hkey_event)
{
...
case TP_HKEY_EV_DOUBLETAP_TOGGLE:
tp_features.trackpoint_doubletap = !tp_features.trackpoint_doubletap;
return true;
...
}
Also use the fw-level filtering rather then having 2 different
filters/enable-flags active with events only coming through if
both let them through.
But making the thinkpad_acpi code use the fw-level filtering
will require some sort of in kernel API for this which is
going to be tricky since these are 2 completely different
subsystems ...
So to me it seems KISS to just stick with the existing thinkpad_acpi
level filtering.
TL;DR:
- What use-cases does this new code enable ?
- Why can't those use-cases be implemented with
the thinkpad_acpi level filtering?
Regards,
Hans
>>
>> Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
>> Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
>> Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> ---
>> Changes in v2:
>> - Improve commit messages
>> - Sysfs attributes moved to trackpoint.c
>> - Removed unnecessary comments
>> - Removed unnecessary debug messages
>> - Using strstarts() instead of strcmp()
>> - is_trackpoint_dt_capable() modified
>> - Removed _BIT suffix and used BIT() define.
>> - Reverse the trackpoint_doubletap_status() logic to return error first.
>> - Removed export functions as a result of the design change
>> - Changed trackpoint_dev->psmouse to parent_psmouse
>> - The path of trackpoint.h is not changed.
>> Changes in v3:
>> - No changes.
>> ---
>> drivers/input/mouse/trackpoint.c | 149 +++++++++++++++++++++++++++++++
>> drivers/input/mouse/trackpoint.h | 15 ++++
>> 2 files changed, 164 insertions(+)
>>
>> diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
>> index 5f6643b69a2c..c6f17b0dec3a 100644
>> --- a/drivers/input/mouse/trackpoint.c
>> +++ b/drivers/input/mouse/trackpoint.c
>> @@ -16,6 +16,8 @@
>> #include "psmouse.h"
>> #include "trackpoint.h"
>>
>> +static struct trackpoint_data *trackpoint_dev;
>> +
>> static const char * const trackpoint_variants[] = {
>> [TP_VARIANT_IBM] = "IBM",
>> [TP_VARIANT_ALPS] = "ALPS",
>> @@ -63,6 +65,21 @@ static int trackpoint_write(struct ps2dev *ps2dev, u8 loc, u8 val)
>> return ps2_command(ps2dev, param, MAKE_PS2_CMD(3, 0, TP_COMMAND));
>> }
>>
>> +/* Read function for TrackPoint extended registers */
>> +static int trackpoint_extended_read(struct ps2dev *ps2dev, u8 loc, u8 *val)
>> +{
>> + u8 ext_param[2] = {TP_READ_MEM, loc};
>> + int error;
>> +
>> + error = ps2_command(ps2dev,
>> + ext_param, MAKE_PS2_CMD(2, 1, TP_COMMAND));
>> +
>> + if (!error)
>> + *val = ext_param[0];
>> +
>> + return error;
>> +}
>> +
>> static int trackpoint_toggle_bit(struct ps2dev *ps2dev, u8 loc, u8 mask)
>> {
>> u8 param[3] = { TP_TOGGLE, loc, mask };
>> @@ -393,6 +410,131 @@ static int trackpoint_reconnect(struct psmouse *psmouse)
>> return 0;
>> }
>>
>> +/* List of known incapable device PNP IDs */
>> +static const char * const dt_incompatible_devices[] = {
>> + "LEN0304",
>> + "LEN0306",
>> + "LEN0317",
>> + "LEN031A",
>> + "LEN031B",
>> + "LEN031C",
>> + "LEN031D",
>> +};
>> +
>> +/*
>> + * checks if it’s a doubletap capable device
>> + * The PNP ID format eg: is "PNP: LEN030d PNP0f13".
>> + */
>> +static bool is_trackpoint_dt_capable(const char *pnp_id)
>> +{
>> + const char *id_start;
>> + char id[8];
>> +
>> + if (!strstarts(pnp_id, "PNP: LEN03"))
>> + return false;
>> +
>> + /* Points to "LEN03xxxx" */
>> + id_start = pnp_id + 5;
>> + if (sscanf(id_start, "%7s", id) != 1)
>> + return false;
>> +
>> + /* Check if it's blacklisted */
>> + for (size_t i = 0; i < ARRAY_SIZE(dt_incompatible_devices); ++i) {
>> + if (strcmp(id, dt_incompatible_devices[i]) == 0)
>> + return false;
>> + }
>> + return true;
>> +}
>> +
>> +/* Trackpoint doubletap status function */
>> +static int trackpoint_doubletap_status(bool *status)
>> +{
>> + struct trackpoint_data *tp = trackpoint_dev;
>> + struct ps2dev *ps2dev = &tp->parent_psmouse->ps2dev;
>> + u8 reg_val;
>> + int rc;
>> +
>> + /* Reading the Doubletap register using extended read */
>> + rc = trackpoint_extended_read(ps2dev, TP_DOUBLETAP, ®_val);
>> + if (rc)
>> + return rc;
>> +
>> + *status = reg_val & TP_DOUBLETAP_STATUS ? true : false;
>> +
>> + return 0;
>> +}
>> +
>> +/* Trackpoint doubletap enable/disable function */
>> +static int trackpoint_set_doubletap(bool enable)
>> +{
>> + struct trackpoint_data *tp = trackpoint_dev;
>> + struct ps2dev *ps2dev = &tp->parent_psmouse->ps2dev;
>> + static u8 doubletap_state;
>> + u8 new_val;
>> +
>> + if (!tp)
>> + return -ENODEV;
>> +
>> + new_val = enable ? TP_DOUBLETAP_ENABLE : TP_DOUBLETAP_DISABLE;
>> +
>> + if (doubletap_state == new_val)
>> + return 0;
>> +
>> + doubletap_state = new_val;
>> +
>> + return trackpoint_write(ps2dev, TP_DOUBLETAP, new_val);
>> +}
>> +
>> +/*
>> + * Trackpoint Doubletap Interface
>> + * Control/Monitoring of Trackpoint Doubletap from:
>> + * /sys/bus/serio/devices/seriox/doubletap_enabled
>> + */
>> +static ssize_t doubletap_enabled_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct serio *serio = to_serio_port(dev);
>> + struct psmouse *psmouse = psmouse_from_serio(serio);
>> + struct trackpoint_data *tp = psmouse->private;
>> + bool status;
>> + int rc;
>> +
>> + if (!tp || !tp->doubletap_capable)
>> + return -ENODEV;
>> +
>> + rc = trackpoint_doubletap_status(&status);
>> + if (rc)
>> + return rc;
>> +
>> + return sysfs_emit(buf, "%d\n", status ? 1 : 0);
>> +}
>> +
>> +static ssize_t doubletap_enabled_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + struct serio *serio = to_serio_port(dev);
>> + struct psmouse *psmouse = psmouse_from_serio(serio);
>> + struct trackpoint_data *tp = psmouse->private;
>> + bool enable;
>> + int err;
>> +
>> + if (!tp || !tp->doubletap_capable)
>> + return -ENODEV;
>> +
>> + err = kstrtobool(buf, &enable);
>> + if (err)
>> + return err;
>> +
>> + err = trackpoint_set_doubletap(enable);
>> + if (err)
>> + return err;
>> +
>> + return count;
>> +}
>> +
>> +static DEVICE_ATTR_RW(doubletap_enabled);
>> +
>> int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
>> {
>> struct ps2dev *ps2dev = &psmouse->ps2dev;
>> @@ -425,6 +567,9 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
>> psmouse->reconnect = trackpoint_reconnect;
>> psmouse->disconnect = trackpoint_disconnect;
>>
>> + trackpoint_dev = psmouse->private;
>> + trackpoint_dev->parent_psmouse = psmouse;
>> +
>> if (variant_id != TP_VARIANT_IBM) {
>> /* Newer variants do not support extended button query. */
>> button_info = 0x33;
>> @@ -470,6 +615,10 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
>> psmouse->vendor, firmware_id,
>> (button_info & 0xf0) >> 4, button_info & 0x0f);
>>
>> + tp->doubletap_capable = is_trackpoint_dt_capable(ps2dev->serio->firmware_id);
>> + if (tp->doubletap_capable)
>> + device_create_file(&psmouse->ps2dev.serio->dev, &dev_attr_doubletap_enabled);
>> +
>> return 0;
>> }
>>
>> diff --git a/drivers/input/mouse/trackpoint.h b/drivers/input/mouse/trackpoint.h
>> index eb5412904fe0..256e8cb35581 100644
>> --- a/drivers/input/mouse/trackpoint.h
>> +++ b/drivers/input/mouse/trackpoint.h
>> @@ -8,6 +8,8 @@
>> #ifndef _TRACKPOINT_H
>> #define _TRACKPOINT_H
>>
>> +#include <linux/bitops.h>
>> +
>> /*
>> * These constants are from the TrackPoint System
>> * Engineering documentation Version 4 from IBM Watson
>> @@ -69,6 +71,8 @@
>> /* (how hard it is to drag */
>> /* with Z-axis pressed) */
>>
>> +#define TP_DOUBLETAP 0x58 /* TrackPoint doubletap register */
>> +
>> #define TP_MINDRAG 0x59 /* Minimum amount of force needed */
>> /* to trigger dragging */
>>
>> @@ -139,6 +143,14 @@
>> #define TP_DEF_TWOHAND 0x00
>> #define TP_DEF_SOURCE_TAG 0x00
>>
>> +/* Doubletap register values */
>> +#define TP_DOUBLETAP_ENABLE 0xFF /* Enable value */
>> +#define TP_DOUBLETAP_DISABLE 0xFE /* Disable value */
>> +
>> +#define TP_DOUBLETAP_STATUS_BIT 0 /* 0th bit defines enable/disable */
>> +
>> +#define TP_DOUBLETAP_STATUS BIT(TP_DOUBLETAP_STATUS_BIT)
>> +
>> #define MAKE_PS2_CMD(params, results, cmd) ((params<<12) | (results<<8) | (cmd))
>>
>> struct trackpoint_data {
>> @@ -150,11 +162,14 @@ struct trackpoint_data {
>> u8 thresh, upthresh;
>> u8 ztime, jenks;
>> u8 drift_time;
>> + bool doubletap_capable;
>>
>> /* toggles */
>> bool press_to_select;
>> bool skipback;
>> bool ext_dev;
>> +
>> + struct psmouse *parent_psmouse;
>> };
>>
>> int trackpoint_detect(struct psmouse *psmouse, bool set_properties);
>> --
>> 2.48.1
>>
>
>
^ permalink raw reply
* Re: [PATCH] hid/usbhid: add reset device for EPROTO
From: Michal Pecio @ 2025-09-18 9:21 UTC (permalink / raw)
To: zhangjinpeng
Cc: jikos, benjamin.tissoires, linux-usb, linux-input, linux-kernel
In-Reply-To: <20250918055527.4157212-1-zhangjinpeng@kylinos.cn>
Hi,
I think this patch may deserve more explanation than just a log snippet.
Is this for case when the mouse is actually being unplugged? Why bother?
Or is the mouse disconnecting itself in response to missing clear halt
request or other misbehavior of host system and the patch fixes it?
On Thu, 18 Sep 2025 13:55:27 +0800, zhangjinpeng wrote:
> [ 792.354988] input: PixArt USB Optical Mouse as /devices/platform/PHYT0039:03/usb7/7-1/7-1.2/7-1.2:1.0/0003:093A:2510.0028/input/input53
> [ 792.355081] hid-generic 0003:093A:2510.0028: input,hidraw1: USB HID v1.11 Mouse [PixArt USB Optical Mouse] on usb-PHYT0039:03-1.2/input0
> [ 792.355137] hub 7-1:1.0: state 7 ports 4 chg 0000 evt 0004
> ： xhci-hcd PHYT0039:03: Transfer error for slot 4 ep 2 on endpoint
No timestamp. Log corruption? Missing fragment? Copy-paste mistake?
> [ 794.579339] xhci-hcd PHYT0039:03: Giveback URB 00000000ab6c1cac, len = 0, expected = 4, status = -71
This looks wrong, shouldn't there be some noise about cancellation of
this URB and Set TR Deq before it is given back with -EPROTO status?
If not a missing log fragment, this may be another case of HW failing
to update EP Context state. What sort of xHCI controller is this?
Is this condition reproducible?
> [ 794.596152] xhci-hcd PHYT0039:03: WARN halted endpoint, queueing URB anyway.
Either the dreadful resubmission - async giveback race, or indeed
a halt has not been recognized by xhci_hcd.
Is this log noise the actual thing which you wanted to fix?
> [ 917.451251] hub 7-1:1.0: state 7 ports 4 chg 0000 evt 0004
> [ 917.451323] usb 7-1-port2: status 0100, change 0001, 12 Mb/s
> [ 917.451362] usb 7-1-port2: indicator auto status 0
> [ 917.451365] usb 7-1.2: USB disconnect, device number 45
> [ 917.451367] usb 7-1.2: unregistering device
> [ 917.451369] usb 7-1.2: unregistering interface 7-1.2:1.0
> [ 917.451429] xhci-hcd PHYT0039:03: Cancel URB 00000000ab6c1cac, dev 1.2, ep 0x81, starting at offset 0x2361ea6280
> [ 917.451432] xhci-hcd PHYT0039:03: // Ding dong!
> [ 917.451436] xhci-hcd PHYT0039:03: shutdown urb ffffffa2ebc8e400 ep1in-intr
> [ 917.451440] xhci-hcd PHYT0039:03: Removing canceled TD starting at 0x2361ea6280 (dma).
> [ 917.500303] usb 7-1.2: usb_disable_device nuking all URBs
> [ 917.500310] xhci-hcd PHYT0039:03: xhci_drop_endpoint called for udev 00000000e00ae900
> [ 917.500324] xhci-hcd PHYT0039:03: drop ep 0x81, slot id 4, new drop flags = 0x8, new add flags = 0x0
> [ 917.500326] xhci-hcd PHYT0039:03: xhci_check_bandwidth called for udev 00000000e00ae900
> [ 917.500330] xhci-hcd PHYT0039:03: // Ding dong!
> [ 917.500351] xhci-hcd PHYT0039:03: Successful Endpoint Configure command
> [ 917.500579] xhci-hcd PHYT0039:03: // Ding dong!
> [ 917.656189] usb 7-1-port2: debounce total 100ms stable 100ms status 0x100
>
> Signed-off-by: zhangjinpeng <zhangjinpeng@kylinos.cn>
I guess if I won't do it then Greg will: you should spell your
name properly, with spaces (if applicable) and capitalization.
> ---
> drivers/hid/usbhid/hid-core.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index 257dd73e37bf..253f82f33b08 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -306,8 +306,13 @@ static void hid_irq_in(struct urb *urb)
> case -ESHUTDOWN: /* unplug */
> clear_bit(HID_IN_RUNNING, &usbhid->iofl);
> return;
> - case -EILSEQ: /* protocol error or unplug */
> case -EPROTO: /* protocol error or unplug */
> + usbhid_mark_busy(usbhid);
> + clear_bit(HID_IN_RUNNING, &usbhid->iofl);
> + set_bit(HID_CLEAR_HALT, &usbhid->iofl);
> + usb_queue_reset_device(usbhid->intf);
Isn't usb_clear_halt() on the affected endpoint enough?
> + return;
> + case -EILSEQ: /* protocol error or unplug */
> case -ETIME: /* protocol error or unplug */
Documentation/driver-api/usb/error-codes.rst is clear as mud, but
these cases seem roughly equivalent to -EPROTO and all seem to imply
a halted endpoint. Why handle them differently?
On xhci_hcd it's true that -EPROTO is the only one you normally get,
and if you ever see -EILSEQ you have a serious driver problem which
no reset is likely to fix, but other HCDs could be different.
> case -ETIMEDOUT: /* Should never happen, but... */
> usbhid_mark_busy(usbhid);
> 2.25.1
>
Regards,
Michal
^ permalink raw reply
* [bug report] HID: haptic: initialize haptic device
From: Dan Carpenter @ 2025-09-18 9:46 UTC (permalink / raw)
To: Angela Czubak; +Cc: linux-input
Hello Angela Czubak,
Commit 344ff3584957 ("HID: haptic: initialize haptic device") from
Aug 18, 2025 (linux-next), leads to the following Smatch static
checker warning:
drivers/hid/hid-haptic.c:193 fill_effect_buf()
error: uninitialized symbol 'value'.
drivers/hid/hid-haptic.c
147 static void fill_effect_buf(struct hid_haptic_device *haptic,
148 struct ff_haptic_effect *effect,
149 struct hid_haptic_effect *haptic_effect,
150 int waveform_ordinal)
151 {
152 struct hid_report *rep = haptic->manual_trigger_report;
153 struct hid_usage *usage;
154 struct hid_field *field;
155 s32 value;
156 int i, j;
157 u8 *buf = haptic_effect->report_buf;
158
159 mutex_lock(&haptic->manual_trigger_mutex);
160 for (i = 0; i < rep->maxfield; i++) {
161 field = rep->field[i];
162 /* Ignore if report count is out of bounds. */
163 if (field->report_count < 1)
164 continue;
165
166 for (j = 0; j < field->maxusage; j++) {
167 usage = &field->usage[j];
168
169 switch (usage->hid) {
170 case HID_HP_INTENSITY:
171 if (effect->intensity > 100) {
172 value = field->logical_maximum;
173 } else {
174 value = field->logical_minimum +
175 effect->intensity *
176 (field->logical_maximum -
177 field->logical_minimum) / 100;
178 }
179 break;
180 case HID_HP_REPEATCOUNT:
181 value = effect->repeat_count;
182 break;
183 case HID_HP_RETRIGGERPERIOD:
184 value = effect->retrigger_period;
185 break;
186 case HID_HP_MANUALTRIGGER:
187 value = waveform_ordinal;
188 break;
189 default:
190 break;
value is not set on the default path.
191 }
192
--> 193 field->value[j] = value;
194 }
195 }
196
197 hid_output_report(rep, buf);
198 mutex_unlock(&haptic->manual_trigger_mutex);
199 }
regards,
dan carpenter
^ permalink raw reply
* [PATCH] HID: amd_sfh: Add sync across amd sfh work functions
From: Basavaraj Natikar @ 2025-09-18 12:32 UTC (permalink / raw)
To: jikos, bentiss, linux-input
Cc: Basavaraj Natikar, Matthew Schwartz, Prakruthi SP,
Akshata MukundShetty
The process of the report is delegated across different work functions.
Hence, add a sync mechanism to protect SFH work data across functions.
Fixes: 4b2c53d93a4b ("SFH:Transport Driver to add support of AMD Sensor Fusion Hub (SFH)")
Reported-by: Matthew Schwartz <matthew.schwartz@linux.dev>
Closes: https://lore.kernel.org/all/a21abca5-4268-449d-95f1-bdd7a25894a5@linux.dev/
Tested-by: Prakruthi SP <Prakruthi.SP@amd.com>
Co-developed-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
Signed-off-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
drivers/hid/amd-sfh-hid/amd_sfh_client.c | 12 ++++++++++--
drivers/hid/amd-sfh-hid/amd_sfh_common.h | 3 +++
drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 4 ++++
3 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
index 0f2cbae39b2b..7017bfa59093 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
@@ -39,8 +39,12 @@ int amd_sfh_get_report(struct hid_device *hid, int report_id, int report_type)
struct amdtp_hid_data *hid_data = hid->driver_data;
struct amdtp_cl_data *cli_data = hid_data->cli_data;
struct request_list *req_list = &cli_data->req_list;
+ struct amd_input_data *in_data = cli_data->in_data;
+ struct amd_mp2_dev *mp2;
int i;
+ mp2 = container_of(in_data, struct amd_mp2_dev, in_data);
+ guard(mutex)(&mp2->lock);
for (i = 0; i < cli_data->num_hid_devices; i++) {
if (cli_data->hid_sensor_hubs[i] == hid) {
struct request_list *new = kzalloc(sizeof(*new), GFP_KERNEL);
@@ -75,6 +79,8 @@ void amd_sfh_work(struct work_struct *work)
u8 report_id, node_type;
u8 report_size = 0;
+ mp2 = container_of(in_data, struct amd_mp2_dev, in_data);
+ guard(mutex)(&mp2->lock);
req_node = list_last_entry(&req_list->list, struct request_list, list);
list_del(&req_node->list);
current_index = req_node->current_index;
@@ -83,7 +89,6 @@ void amd_sfh_work(struct work_struct *work)
node_type = req_node->report_type;
kfree(req_node);
- mp2 = container_of(in_data, struct amd_mp2_dev, in_data);
mp2_ops = mp2->mp2_ops;
if (node_type == HID_FEATURE_REPORT) {
report_size = mp2_ops->get_feat_rep(sensor_index, report_id,
@@ -107,6 +112,8 @@ void amd_sfh_work(struct work_struct *work)
cli_data->cur_hid_dev = current_index;
cli_data->sensor_requested_cnt[current_index] = 0;
amdtp_hid_wakeup(cli_data->hid_sensor_hubs[current_index]);
+ if (!list_empty(&req_list->list))
+ schedule_delayed_work(&cli_data->work, 0);
}
void amd_sfh_work_buffer(struct work_struct *work)
@@ -117,9 +124,10 @@ void amd_sfh_work_buffer(struct work_struct *work)
u8 report_size;
int i;
+ mp2 = container_of(in_data, struct amd_mp2_dev, in_data);
+ guard(mutex)(&mp2->lock);
for (i = 0; i < cli_data->num_hid_devices; i++) {
if (cli_data->sensor_sts[i] == SENSOR_ENABLED) {
- mp2 = container_of(in_data, struct amd_mp2_dev, in_data);
report_size = mp2->mp2_ops->get_in_rep(i, cli_data->sensor_idx[i],
cli_data->report_id[i], in_data);
hid_input_report(cli_data->hid_sensor_hubs[i], HID_INPUT_REPORT,
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
index f44a3bb2fbd4..78f830c133e5 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
@@ -10,6 +10,7 @@
#ifndef AMD_SFH_COMMON_H
#define AMD_SFH_COMMON_H
+#include <linux/mutex.h>
#include <linux/pci.h>
#include "amd_sfh_hid.h"
@@ -59,6 +60,8 @@ struct amd_mp2_dev {
u32 mp2_acs;
struct sfh_dev_status dev_en;
struct work_struct work;
+ /* mp2 to protect data */
+ struct mutex lock;
u8 init_done;
u8 rver;
};
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
index 2983af969579..1d9f955573aa 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
@@ -466,6 +466,10 @@ static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
if (!privdata->cl_data)
return -ENOMEM;
+ rc = devm_mutex_init(&pdev->dev, &privdata->lock);
+ if (rc)
+ return rc;
+
privdata->sfh1_1_ops = (const struct amd_sfh1_1_ops *)id->driver_data;
if (privdata->sfh1_1_ops) {
if (boot_cpu_data.x86 >= 0x1A)
--
2.25.1
^ permalink raw reply related
* [PATCH RFC 0/3] Input: add initial support for Goodix GTX8 touchscreen ICs
From: Jens Reidel @ 2025-09-18 14:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Hans de Goede, Neil Armstrong, Henrik Rydberg
Cc: linux-input, devicetree, linux-kernel, linux, phone-devel,
~postmarketos/upstreaming, Jens Reidel
These ICs support SPI and I2C interfaces, up to 10 finger touch, stylus
and gesture events.
This driver is derived from the Goodix gtx8_driver_linux available at
[1] and only supports the GT9886 and GT9896 ICs present in the Xiaomi
Mi 9T and Xiaomi Redmi Note 10 Pro smartphones.
The current implementation only supports Normandy and Yellowstone type
ICs, aka only GT9886 and GT9896. It is also limited to I2C only, since I
don't have a device with GTX8 over SPI at hand. Adding support for SPI
should be fairly easy in the future, since the code uses a regmap.
Support for advanced features like:
- Firmware updates
- Stylus events
- Gesture events
- Nanjing IC support
is not included in current version.
The current support requires a previously flashed firmware to be
present.
As I did not have access to datasheets for these ICs, I extracted the
addresses from a couple of config files using a small tool [2]. The
addresses are identical for the same IC families in all configs I
observed, however not all of them make sense and I stubbed out firmware
request support due to this.
[1] https://github.com/goodix/gtx8_driver_linux
[2] https://github.com/sm7150-mainline/goodix-cfg-bin
Signed-off-by: Jens Reidel <adrian@mainlining.org>
---
Jens Reidel (3):
dt-bindings: input: document Goodix GTX8 Touchscreen ICs
Input: add support for Goodix GTX8 Touchscreen ICs
MAINTAINERS: add an entry for Goodix GTX8 Touchscreen driver
.../bindings/input/touchscreen/goodix,gt9886.yaml | 71 +++
MAINTAINERS | 7 +
drivers/input/touchscreen/Kconfig | 15 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/goodix_gtx8.c | 562 +++++++++++++++++++++
drivers/input/touchscreen/goodix_gtx8.h | 137 +++++
6 files changed, 793 insertions(+)
---
base-commit: ae2d20002576d2893ecaff25db3d7ef9190ac0b6
change-id: 20250918-gtx8-59a50ccd78a5
Best regards,
--
Jens Reidel <adrian@mainlining.org>
^ 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