[PATCH 1/2] rust: hwmon: add hwmon abstractions

[PATCH 1/2] rust: hwmon: add hwmon abstractions

[skxby]

>From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: skxby <2374604267@qq.com>
Date: Fri, 16 Oct 2026 15:12:27 +0800
Subject: [PATCH 1/2] rust: hwmon: add hwmon abstractions

Add Rust abstractions for the hwmon subsystem.

The main type is Registration<T>, which owns and pins its inner state,
registers the hwmon device, and unregisters it on drop. A Driver trait
provides callbacks for the temperature and chip attribute families; the
chip callbacks are optional, following the existing #[vtable] idiom used
by other subsystems. Channel configuration masks are derived from the
modeled attribute enums.

This is a rework of the abstraction proposed in the RFC "rust: hwmon:
add basic hwmon abstractions" [1], brought up to the current rust tree:
registration does not use devm, the sensor type enumeration covers the
chip family in addition to the temperature family, and the resource is
released explicitly on the registration error path.

[1] https://lore.kernel.org/all/20260512155747.10136-1-donplat@barrensea.org/

Signed-off-by: skxby <2374604267@qq.com>
---
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/hwmon.rs            | 491 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 3 files changed, 494 insertions(+)
 create mode 100644 rust/kernel/hwmon.rs

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index c706dda..1fc7a86 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -62,6 +62,7 @@
 #include <linux/firmware.h>
 #include <linux/fwctl.h>
 #include <linux/fs.h>
+#include <linux/hwmon.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/io-pgtable.h>
diff --git a/rust/kernel/hwmon.rs b/rust/kernel/hwmon.rs
new file mode 100644
index 0000000..d7e3fd6
--- /dev/null
+++ b/rust/kernel/hwmon.rs
@@ -0,0 +1,491 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Hardware Monitoring (Hwmon) abstractions.
+//!
+//! C header: [`include/linux/hwmon.h`](srctree/include/linux/hwmon.h)
+//!
+//! This abstraction supports registering temperature sensors with the full standard
+//! `temp[1-*]` attribute set, plus the chip-level attribute channel. Which attributes
+//! a driver actually exposes is decided by [`Driver::is_visible`] /
+//! [`Driver::chip_is_visible`]. Additional sensor types (voltage, current, power, fan)
+//! and `read_string` are deferred to a future version.
+
+use crate::{
+    bindings,
+    device::{Bound, Device},
+    error::{from_err_ptr, from_result, Error, Result},
+    error::code::{EINVAL, EOPNOTSUPP},
+    ffi::{c_int, c_long},
+    prelude::*,
+    str::CStr,
+};
+
+use core::marker::{PhantomData, PhantomPinned};
+
+/// Sensor type.
+///
+/// `Chip` corresponds to `hwmon_chip`, the first member of `enum hwmon_sensor_types`
+/// and the channel carrying chip-level attributes such as `update_interval`.
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+pub enum SensorType {
+    /// Chip-level attributes (`hwmon_chip_*`).
+    Chip,
+    /// Temperature sensor.
+    Temp,
+}
+
+impl TryFrom<u32> for SensorType {
+    type Error = Error;
+
+    fn try_from(value: u32) -> Result<Self> {
+        match value {
+            bindings::hwmon_sensor_types_hwmon_chip => Ok(Self::Chip),
+            bindings::hwmon_sensor_types_hwmon_temp => Ok(Self::Temp),
+            _ => Err(EINVAL),
+        }
+    }
+}
+
+/// Temperature attributes covered by the abstraction layer.
+///
+/// This is the subset that can be expressed in terms of a temperature channel for any
+/// sensor: input, limits, hysteresis and alarm flags. Attributes that exist only for
+/// specific hardware (e.g. `crit`, `emergency`) are not part of the common shape and are
+/// left out rather than modelled and then filtered per driver.
+///
+/// A driver only exposes the subset it implements; [`Driver::is_visible`]
+/// returns `0` for the rest, and the hwmon core will not create the sysfs file.
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+pub enum TempAttr {
+    /// `temp1_input`, millidegrees Celsius, read-only.
+    Input,
+    /// `temp1_min` (low limit), millidegrees Celsius.
+    Min,
+    /// `temp1_max` (high limit), millidegrees Celsius.
+    Max,
+    /// `temp1_min_hyst`, millidegrees Celsius.
+    MinHyst,
+    /// `temp1_max_hyst`, millidegrees Celsius.
+    MaxHyst,
+    /// `temp1_min_alarm`, 0 or 1, read-only.
+    MinAlarm,
+    /// `temp1_max_alarm`, 0 or 1, read-only.
+    MaxAlarm,
+}
+
+impl TryFrom<u32> for TempAttr {
+    type Error = Error;
+
+    fn try_from(value: u32) -> Result<Self> {
+        match value {
+            bindings::hwmon_temp_attributes_hwmon_temp_input => Ok(Self::Input),
+            bindings::hwmon_temp_attributes_hwmon_temp_min => Ok(Self::Min),
+            bindings::hwmon_temp_attributes_hwmon_temp_max => Ok(Self::Max),
+            bindings::hwmon_temp_attributes_hwmon_temp_min_hyst => Ok(Self::MinHyst),
+            bindings::hwmon_temp_attributes_hwmon_temp_max_hyst => Ok(Self::MaxHyst),
+            bindings::hwmon_temp_attributes_hwmon_temp_min_alarm => Ok(Self::MinAlarm),
+            bindings::hwmon_temp_attributes_hwmon_temp_max_alarm => Ok(Self::MaxAlarm),
+            _ => Err(EOPNOTSUPP),
+        }
+    }
+}
+
+/// Chip-level attributes covered by the abstraction layer.
+///
+/// Mirror of the `hwmon_chip_*` attribute namespace. Only the attributes that are
+/// meaningful for a generic chip are listed; the remaining ones are deferred until a
+/// driver needs them.
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+pub enum ChipAttr {
+    /// `update_interval`, milliseconds.
+    UpdateInterval,
+}
+
+impl TryFrom<u32> for ChipAttr {
+    type Error = Error;
+
+    fn try_from(value: u32) -> Result<Self> {
+        match value {
+            bindings::hwmon_chip_attributes_hwmon_chip_update_interval => Ok(Self::UpdateInterval),
+            _ => Err(EOPNOTSUPP),
+        }
+    }
+}
+
+/// Bitmask declaring the temperature attribute set in the temperature channel info.
+///
+/// Derived from [`TempAttr`] so that the declared channel shape and the modelled
+/// attribute set cannot drift apart. Visibility is filtered afterwards by
+/// [`Driver::is_visible`].
+const TEMP_ATTR_MASK: u32 = 1u32 << bindings::hwmon_temp_attributes_hwmon_temp_input
+    | 1u32 << bindings::hwmon_temp_attributes_hwmon_temp_min
+    | 1u32 << bindings::hwmon_temp_attributes_hwmon_temp_max
+    | 1u32 << bindings::hwmon_temp_attributes_hwmon_temp_min_hyst
+    | 1u32 << bindings::hwmon_temp_attributes_hwmon_temp_max_hyst
+    | 1u32 << bindings::hwmon_temp_attributes_hwmon_temp_min_alarm
+    | 1u32 << bindings::hwmon_temp_attributes_hwmon_temp_max_alarm;
+
+/// Bitmask declaring the chip attribute set in the chip channel info.
+///
+/// Derived from [`ChipAttr`]. A driver that does not implement any chip attribute
+/// leaves its `chip_config` zeroed, in which case no chip attribute file is created.
+const CHIP_ATTR_MASK: u32 =
+    1u32 << bindings::hwmon_chip_attributes_hwmon_chip_update_interval;
+
+/// The hwmon driver trait.
+///
+/// # Concurrency and Driver Responsibilities
+///
+/// The hwmon C core provides **no internal locking** for attribute access. When user-space
+/// reads or writes `/sys/class/hwmon/hwmon*/temp*_*`, the core directly invokes the
+/// registered callbacks with a **shared `&self` reference** to `HwmonInner<T>`.
+///
+/// - **Read-only operations** ([`Driver::read`]) receiving `&self` are safe if the hardware
+///   state is read-only or the driver uses interior mutability (e.g., `Mutex<HwState>`).
+/// - **Write operations** ([`Driver::write`]) **must** synchronize mutable access inside the
+///   driver implementation. Concurrent writes to the same attribute, or a write racing with
+///   a read, are **not serialized by hwmon** -- the driver must use `Mutex`, `RwLock`, or
+///   hardware-level atomics as appropriate.
+///
+/// **Lifetime guarantee**: The `drvdata` pointer (`&HwmonInner<T>`) remains valid for the
+/// entire duration of every callback. `Drop::drop` calls `hwmon_device_unregister`, which
+/// **blocks until all in-flight callbacks return** before releasing the allocation. Drivers
+/// need not implement additional lifetime tracking beyond Rust's ownership model.
+///
+/// # Chip attributes
+///
+/// The three `chip_*` methods are optional. A driver that does not implement
+/// [`Driver::chip_is_visible`] exposes no chip attribute at all: no entry is created in
+/// the corresponding channel config, and the hwmon core never calls back for one.
+#[vtable]
+pub trait Driver: Send + Sync {
+    /// Reads a temperature attribute value, in millidegrees Celsius.
+    fn read(&self, attr: TempAttr, channel: u32) -> Result<c_long>;
+
+    /// Writes a temperature attribute value, in millidegrees Celsius.
+    fn write(&self, attr: TempAttr, channel: u32, value: c_long) -> Result<()>;
+
+    /// Returns the sysfs file permission bits for a temperature attribute.
+    fn is_visible(&self, attr: TempAttr, channel: u32) -> u16;
+
+    /// Reads a chip-level attribute value.
+    #[optional]
+    fn chip_read(&self, _attr: ChipAttr) -> Result<c_long>;
+
+    /// Writes a chip-level attribute value.
+    #[optional]
+    fn chip_write(&self, _attr: ChipAttr, _value: c_long) -> Result<()>;
+
+    /// Returns the sysfs file permission bits for a chip-level attribute.
+    #[optional]
+    fn chip_is_visible(&self, _attr: ChipAttr) -> u16;
+}
+
+/// Adapter translating C hwmon callbacks to [`Driver`] trait method calls.
+struct Adapter<T: Driver> {
+    _p: PhantomData<T>,
+}
+
+impl<T: Driver> Adapter<T> {
+    /// # Safety
+    ///
+    /// Called by the hwmon core during and after registration with the `drvdata`
+    /// pointer set in `hwmon_device_register_with_info`. The pointer remains valid
+    /// until `hwmon_device_unregister` returns in [`Registration`]'s `Drop`.
+    unsafe extern "C" fn is_visible_callback(
+        drvdata: *const core::ffi::c_void,
+        type_: u32,
+        attr: u32,
+        channel: c_int,
+    ) -> u16 {
+        if drvdata.is_null() {
+            return 0;
+        }
+        // SAFETY: `drvdata` is `inner_ptr` set in `Registration::new`. The
+        // hwmon core's barrier in `hwmon_device_unregister` ensures this
+        // pointer is valid for the lifetime of any callback invocation.
+        let inner = unsafe { &*drvdata.cast::<HwmonInner<T>>() };
+
+        match SensorType::try_from(type_) {
+            Ok(SensorType::Chip) => match ChipAttr::try_from(attr) {
+                // A driver that does not implement chip attributes exposes none.
+                Ok(a) if T::HAS_CHIP_IS_VISIBLE => T::chip_is_visible(&inner.driver, a),
+                _ => 0,
+            },
+            Ok(SensorType::Temp) => match TempAttr::try_from(attr) {
+                // C core guarantees `channel >= 0`.
+                Ok(a) => T::is_visible(&inner.driver, a, channel as u32),
+                Err(_) => 0,
+            },
+            // Sensor types the abstraction does not model are invisible.
+            Err(_) => 0,
+        }
+    }
+
+    /// # Safety
+    ///
+    /// Called by the hwmon core. `dev` is the device created during registration, and `val`
+    /// points to writable memory for the result. This callback is only installed when the
+    /// driver implements at least one readable attribute kind.
+    unsafe extern "C" fn read_callback(
+        dev: *mut bindings::device,
+        type_: u32,
+        attr: u32,
+        channel: c_int,
+        val: *mut c_long,
+    ) -> c_int {
+        from_result(|| {
+            // SAFETY: `dev_get_drvdata` returns the pointer set during registration, valid
+            // until `hwmon_device_unregister` completes.
+            let drvdata = unsafe { bindings::dev_get_drvdata(dev) };
+            if drvdata.is_null() {
+                return Err(EINVAL);
+            }
+            // SAFETY: `drvdata` is the `HwmonInner<T>` pointer set in `Registration::new`.
+            // The hwmon core's barrier in `hwmon_device_unregister` keeps it valid for the
+            // lifetime of every callback invocation.
+            let inner = unsafe { &*drvdata.cast::<HwmonInner<T>>() };
+
+            let result = match SensorType::try_from(type_)? {
+                SensorType::Chip => T::chip_read(&inner.driver, ChipAttr::try_from(attr)?)?,
+                // C core guarantees `channel >= 0`.
+                SensorType::Temp => {
+                    T::read(&inner.driver, TempAttr::try_from(attr)?, channel as u32)?
+                }
+            };
+
+            // SAFETY: `val` is provided by the hwmon core and points to a valid `long`.
+            unsafe { *val = result };
+            Ok(0)
+        })
+    }
+
+    /// # Safety
+    ///
+    /// Same `drvdata` argument as [`Self::read_callback`]. Additional constraints:
+    ///
+    /// - `Driver::write` receives `&self`; the driver implementation must synchronize
+    ///   mutable access itself (`Mutex`/`RwLock`) -- hwmon does not serialize read/write.
+    /// - Callback-side `&HwmonInner<T>` validity is identical: `Registration` not dropped
+    ///   means the `KBox` allocation is alive, so the raw pointer may be dereferenced.
+    ///
+    /// `write` returns `Result<()>`; `from_result` converts errors to C errno (0 on success).
+    unsafe extern "C" fn write_callback(
+        dev: *mut bindings::device,
+        type_: u32,
+        attr: u32,
+        channel: c_int,
+        val: c_long,
+    ) -> c_int {
+        from_result(|| {
+            // SAFETY: `dev_get_drvdata` returns the pointer set during registration, valid
+            // until `hwmon_device_unregister` completes.
+            let drvdata = unsafe { bindings::dev_get_drvdata(dev) };
+            if drvdata.is_null() {
+                return Err(EINVAL);
+            }
+            // SAFETY: `drvdata` is the `HwmonInner<T>` pointer set in `Registration::new`.
+            // The hwmon core's barrier in `hwmon_device_unregister` keeps it valid for the
+            // lifetime of every callback invocation.
+            let inner = unsafe { &*drvdata.cast::<HwmonInner<T>>() };
+
+            match SensorType::try_from(type_)? {
+                SensorType::Chip => T::chip_write(&inner.driver, ChipAttr::try_from(attr)?, val)?,
+                // C core guarantees `channel >= 0`.
+                SensorType::Temp => {
+                    T::write(&inner.driver, TempAttr::try_from(attr)?, channel as u32, val)?
+                }
+            }
+            Ok(0)
+        })
+    }
+}
+
+/// Container holding the driver and all C structures for hwmon registration.
+///
+/// # Initialization Invariants (Self-Referential Structure)
+///
+/// `HwmonInner<T>` contains self-referential raw pointers that must be wired in a specific
+/// order:
+///
+/// 1. **Field initialization**: All owned data (driver, ops, chip_config, chip_channel,
+///    temp_config, temp_channel, etc.) are written into the `KBox` allocation first. Any
+///    dynamic collections (e.g. `KVec` channels) must be fully built before step 2--
+///    reallocation invalidates pointers.
+///
+/// 2. **Pointer wiring**: Once the final address is known, write raw pointers via `&raw mut`:
+///    `chip_info.ops` points to `inner.ops`, `channel_info_array[0]` points to
+///    `inner.chip_channel`, `channel_info_array[1]` to `inner.temp_channel`, etc. No external
+///    references exist yet, so `&raw mut` is safe.
+///
+/// 3. **Pinning**: Call `Pin::new_unchecked` to freeze the address. From this point onward,
+///    the allocation cannot move, ensuring all interior pointers remain valid.
+///
+/// 4. **Registration**: Pass `&inner.chip_info` and `&inner` (as drvdata) to
+///    `hwmon_device_register_with_info`. C now holds these pointers until unregistration.
+///
+/// 5. **Deallocation fence**: `Drop::drop` calls `hwmon_device_unregister`, which blocks
+///    until all callbacks return. Only after it returns is the `KBox` deallocated.
+///
+/// Violating this order (e.g., moving `inner` after wiring, or dropping before unregister)
+/// results in **use-after-free or dangling pointers in C callbacks**.
+struct HwmonInner<T: Driver> {
+    driver: T,
+    ops: bindings::hwmon_ops,
+    chip_config: [u32; 1],
+    temp_config: [u32; 2],
+    chip_channel: bindings::hwmon_channel_info,
+    temp_channel: bindings::hwmon_channel_info,
+    channel_info_array: [*const bindings::hwmon_channel_info; 3],
+    chip_info: bindings::hwmon_chip_info,
+    _pin: PhantomPinned,
+}
+
+impl<T: Driver> HwmonInner<T> {
+    /// Returns a placeholder with all pointer fields set to null. The returned structure is
+    /// safe to drop -- no dynamic resources are held before registration completes.
+    fn new_placeholder(driver: T) -> Self {
+        // A driver that exposes no chip attribute must not advertise the chip channel
+        // either: leaving `chip_config` zeroed makes the channel carry no attributes, and
+        // `is_visible` would not be consulted for it in the first place.
+        let chip_config = if T::HAS_CHIP_IS_VISIBLE {
+            [CHIP_ATTR_MASK]
+        } else {
+            [0]
+        };
+
+        Self {
+            driver,
+            ops: bindings::hwmon_ops {
+                is_visible: Some(Adapter::<T>::is_visible_callback),
+                visible: 0,
+                read: Some(Adapter::<T>::read_callback),
+                read_string: None,
+                write: Some(Adapter::<T>::write_callback),
+            },
+            chip_config,
+            temp_config: [TEMP_ATTR_MASK, 0],
+            chip_channel: bindings::hwmon_channel_info {
+                type_: 0,
+                config: core::ptr::null(),
+            },
+            temp_channel: bindings::hwmon_channel_info {
+                type_: 0,
+                config: core::ptr::null(),
+            },
+            channel_info_array: [
+                core::ptr::null(),
+                core::ptr::null(),
+                core::ptr::null(),
+            ],
+            chip_info: bindings::hwmon_chip_info {
+                ops: core::ptr::null(),
+                info: core::ptr::null(),
+            },
+            _pin: PhantomPinned,
+        }
+    }
+}
+
+/// A registered hwmon device.
+pub struct Registration<T: Driver> {
+    hwmon_dev: *mut bindings::device,
+    // Held exclusively for drop ordering: keeps `HwmonInner<T>` alive until
+    // `hwmon_device_unregister` returns. The value is never read.
+    #[expect(dead_code)]
+    inner: Pin<KBox<HwmonInner<T>>>,
+}
+
+impl<T: Driver> Registration<T> {
+    /// Registers a new hwmon device.
+    ///
+    /// The device is registered as a child of `parent`. `name` must not contain characters
+    /// rejected by `hwmon_is_bad_char` (hyphens, spaces, asterisks).
+    ///
+    /// `parent` is `&Device<Bound>`: rust-next's `Device<Ctx>` is generic over
+    /// `DeviceContext`, and a registered parent device always has the `Bound` context.
+    /// The RFC's unparameterized `&Device` does not compile against that split (E0308).
+    pub fn new(parent: &Device<Bound>, name: &CStr, data: T) -> Result<Self> {
+        let mut boxed = KBox::new(HwmonInner::new_placeholder(data), GFP_KERNEL)?;
+
+        // Wire self-referential pointers before pinning. We obtain a mutable raw pointer
+        // from the uniquely-owned `KBox` -- no intermediate `&mut` reference is created,
+        // so no aliasing rules are violated.
+        let inner_ptr: *mut HwmonInner<T> = &raw mut *boxed;
+        // SAFETY: `boxed` is exclusively owned at this point. We write only to C-struct
+        // fields meant for one-time initialization, never to `driver`.
+        unsafe {
+            (*inner_ptr).chip_channel = bindings::hwmon_channel_info {
+                type_: bindings::hwmon_sensor_types_hwmon_chip,
+                config: (*inner_ptr).chip_config.as_ptr(),
+            };
+            (*inner_ptr).temp_channel = bindings::hwmon_channel_info {
+                type_: bindings::hwmon_sensor_types_hwmon_temp,
+                config: (*inner_ptr).temp_config.as_ptr(),
+            };
+            // The channel info array is NULL-terminated, as required by the C core.
+            (*inner_ptr).channel_info_array = [
+                &raw const (*inner_ptr).chip_channel,
+                &raw const (*inner_ptr).temp_channel,
+                core::ptr::null(),
+            ];
+            (*inner_ptr).chip_info = bindings::hwmon_chip_info {
+                ops: &raw const (*inner_ptr).ops,
+                info: (*inner_ptr).channel_info_array.as_ptr(),
+            };
+        }
+
+        // SAFETY: `HwmonInner<T>` is `!Unpin` (via `PhantomPinned`). All self-referential
+        // pointers are now set to their final values. The struct will never be moved.
+        let inner = unsafe { Pin::new_unchecked(boxed) };
+
+        let drvdata: *mut core::ffi::c_void = inner_ptr.cast();
+
+        // SAFETY: `chip_info` and all nested pointers target memory within the same
+        // allocation, which remains valid until `hwmon_device_unregister` in `Drop`.
+        // `parent.as_raw()` and `name.as_char_ptr()` are valid. The name is copied by
+        // the kernel (via `dev_set_name` -> `kvasprintf_const`), so no lifetime issue.
+        let hwmon_dev = from_err_ptr(unsafe {
+            bindings::hwmon_device_register_with_info(
+                parent.as_raw(),
+                name.as_char_ptr(),
+                drvdata,
+                &raw const (*inner_ptr).chip_info,
+                core::ptr::null_mut(),
+            )
+        })?;
+
+        Ok(Self { hwmon_dev, inner })
+    }
+}
+
+impl<T: Driver> Drop for Registration<T> {
+    fn drop(&mut self) {
+        // SAFETY: `hwmon_dev` was returned by a successful registration. This call waits for
+        // all in-flight callbacks before returning.
+        unsafe { bindings::hwmon_device_unregister(self.hwmon_dev) };
+    }
+}
+
+// SAFETY: `Registration` contains two fields:
+// - `hwmon_dev: *mut bindings::device`: this raw pointer is only ever read once, in
+//   `Drop::drop`, to call `hwmon_device_unregister`. The pointed-to `struct device` is
+//   owned by the hwmon core and outlives the `Registration` (unregister blocks until all
+//   in-flight callbacks finish). Moving the `Registration` across threads merely moves the
+//   pointer value; the device is never accessed through it except in `drop`, which has
+//   exclusive access (`&mut self`). No data race is possible.
+// - `inner: Pin<KBox<HwmonInner<T>>>`: `KBox<T>` is `Send` when `T: Send`, and `Pin<P>`
+//   preserves `Send`. `T: Driver` requires `T: Send`, so the field is `Send`.
+// Therefore `Registration<T>` is `Send`.
+unsafe impl<T: Driver> Send for Registration<T> {}
+
+// SAFETY: sharing a `&Registration<T>` cannot produce a data race:
+// - `hwmon_dev` is never read through a shared reference (only `drop` reads it, via
+//   `&mut self`).
+// - `inner: Pin<KBox<HwmonInner<T>>>` is `Sync` when `HwmonInner<T>` is `Sync`.
+//   `HwmonInner<T>` is `Sync` because `T: Driver` requires `T: Sync`, and the remaining
+//   fields are C structures written once during registration and only read by the hwmon
+//   core (which synchronizes access with the unregister barrier).
+// Therefore `Registration<T>` is `Sync`.
+unsafe impl<T: Driver> Sync for Registration<T> {}
+
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index a4747f7..541d171 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -80,6 +80,8 @@ pub mod fs;
 pub mod fwctl;
 #[cfg(CONFIG_GPU_BUDDY = "y")]
 pub mod gpu;
+#[cfg(CONFIG_HWMON = "y")]
+pub mod hwmon;
 #[cfg(CONFIG_I2C = "y")]
 pub mod i2c;
 pub mod id_pool;

[PATCH 2/2] hwmon: (tmp108) add Rust TMP108 driver

[skxby]

>From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: skxby <2374604267@qq.com>
Date: Fri, 16 Oct 2026 15:12:27 +0800
Subject: [PATCH 2/2] hwmon: (tmp108) add Rust TMP108 driver

Add a Rust driver for the TI TMP108 temperature sensor, mirroring the
existing C driver (drivers/hwmon/tmp108.c).

The driver exposes seven temperature attributes (input, min, max,
min_hyst, max_hyst, min_alarm, max_alarm) plus update_interval on the
chip channel. Numeric and error semantics follow the C driver:
millidegree conversion with truncation, write clamping, absolute-value
hysteresis, four conversion-rate buckets selected with the same
closest-descending search, and EAGAIN while a one-shot conversion is not
ready.

Signed-off-by: skxby <2374604267@qq.com>
---
 samples/rust/Kconfig   |  21 ++
 samples/rust/Makefile  |   2 +
 samples/rust/tmp108.rs | 573 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 596 insertions(+)
 create mode 100755 samples/rust/tmp108.rs

diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index 31d6253..18df428 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -172,6 +172,27 @@ config SAMPLE_RUST_DRIVER_SERDEV
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_HWMON
+	tristate "Hwmon Driver"
+	depends on HWMON=y
+	help
+	  This option builds the Rust hwmon driver sample.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_driver_hwmon.
+
+	  If unsure, say N.
+
+config SAMPLE_RUST_TMP108
+	tristate "TMP108 Driver"
+	depends on HWMON=y
+	depends on I2C=y
+	help
+	  This option builds the Rust TMP108 temperature sensor driver.
+
+	  To compile this as a module, choose M here:
+	  the module will be called tmp108.
+
 config SAMPLE_RUST_SOC
 	tristate "SoC Driver"
 	select SOC_BUS
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index b986b68..c5602ce 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -16,6 +16,8 @@ obj-$(CONFIG_SAMPLE_RUST_DRIVER_FAUX)		+= rust_driver_faux.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_AUXILIARY)	+= rust_driver_auxiliary.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_SERDEV)		+= rust_driver_serdev.o
 obj-$(CONFIG_SAMPLE_RUST_CONFIGFS)		+= rust_configfs.o
+obj-$(CONFIG_SAMPLE_RUST_HWMON)			+= rust_driver_hwmon.o
+obj-$(CONFIG_SAMPLE_RUST_TMP108) += tmp108.o
 obj-$(CONFIG_SAMPLE_RUST_SOC)			+= rust_soc.o
 
 rust_print-y := rust_print_main.o rust_print_events.o
diff --git a/samples/rust/tmp108.rs b/samples/rust/tmp108.rs
new file mode 100755
index 0000000..a070547
--- /dev/null
+++ b/samples/rust/tmp108.rs
@@ -0,0 +1,573 @@
+// SPDX-License-Identifier: GPL-2.0
+//!
+//! TMP108 temperature sensor driver (Rust, i2c + hwmon).
+//!
+//! I2C channel: `kernel::i2c::Registration::new` requires a bound parent device,
+//! which is not available during module init, and `I2cClient` exposes neither
+//! SMBus methods nor a public `as_raw`. The driver therefore uses the raw
+//! bindings directly:
+//!   i2c_get_adapter -> i2c_new_client_device -> i2c_put_adapter ->
+//!   Device::from_raw(&raw mut (*client).dev) -> hwmon::Registration::new
+//!
+//! word_swapped transfers: `include/linux/i2c.h` provides
+//! `i2c_smbus_{read,write}_word_swapped` as static inline helpers (word_data plus
+//! swab16) with no exported symbol, so the driver declares the exported
+//! `i2c_smbus_{read,write}_word_data` instead and replicates the i2c.h inline
+//! expansion (swab16) on the Rust side, bit-exact.
+//!
+//! Byte order: the C driver uses word_swapped transfers (regmap with val_bits=16,
+//! REGMAP_ENDIAN_BIG and single accesses); the register value seen by the driver
+//! is always swab16 of the on-wire word.
+
+use kernel::{
+    bindings,
+    device::{Bound, Device},
+    error::{
+        code::{EAGAIN, EINVAL, EOPNOTSUPP, ENODEV},
+        from_err_ptr, Error, Result,
+    },
+    ffi::c_long,
+    hwmon::{ChipAttr, Driver, Registration, TempAttr},
+    prelude::*,
+};
+
+// === TMP108 register addresses ===
+
+const TEMP: u16 = 0x00;
+const CONF: u16 = 0x01;
+const TLOW: u16 = 0x02;
+const THIGH: u16 = 0x03;
+
+// CONF bit fields (on-wire big-endian; decoded with the masks below after word_swapped transfer)
+const CONF_FL: u16 = 0x0800; // TLOW limit flag, read-only
+const CONF_FH: u16 = 0x1000; // THIGH limit flag, read-only
+const CONF_HYS_MASK: u16 = 0x0030; // HYS1:HYS0
+
+// === CONF.CR conversion rate bit field ===
+//
+// TMP108_CONF_CR0 = 0x2000 (bit13) / CR1 = 0x4000 (bit14)
+// CONVRATE_MASK = CR0|CR1 = 0x6000 / CR0_POS = 13
+// Encoding: 0P25HZ=0b00 / 1HZ=0b01 (default) / 4HZ=0b10 / 16HZ=0b11
+const CONF_CR0: u16 = 0x2000;
+const CONF_CR1: u16 = 0x4000;
+const CONF_CONVRATE_MASK: u16 = CONF_CR0 | CONF_CR1; // 0x6000
+const CONF_CR0_POS: u32 = 13;
+
+/// The four TMP108 conversion periods (ms); `find_closest_descending` requires
+/// **descending** order.
+///
+/// Matches the C driver's `tmp108_sample_times[] = {4000, 1000, 250, 63}`.
+/// Note: the same C file also has `p3t1035_sample_times[] = {4000, 1000, 250, 125}`
+/// (last entry 125 != 63). This driver supports only the tmp108 (16-bit CONF), so the
+/// 63 entry must be used.
+const SAMPLE_TIMES: [u16; 4] = [4000, 1000, 250, 63];
+
+// Threshold clamp bounds
+const CLAMP_MIN_MC: i32 = -50000;
+const CLAMP_MAX_MC: i32 = 127937;
+
+// Hysteresis bucket bounds: delta < 500 -> 0 C; delta < 1500 -> 1 C; delta < 3000 -> 2 C; else 4 C
+const HYST_STEP_1_MC: i32 = 500;
+const HYST_STEP_2_MC: i32 = 1500;
+const HYST_STEP_4_MC: i32 = 3000;
+
+// One-shot conversion time (unused in continuous mode)
+#[allow(dead_code)] // unused until the one-shot path is enabled with set_ready_time
+const CONVERSION_TIME_MS: u64 = 30;
+
+// TMP108 default I2C address (the C driver obtains it from DT/ACPI/i2c_device_id)
+const TMP108_ADDR: u16 = 0x48;
+
+// === i2c_board_info construction at runtime ===
+//
+// Built at runtime rather than via a const initializer: `kernel::i2c::I2cBoardInfo::new` is const
+// but returns the abstraction type (needing the Registration path); a directly zeroed
+// `bindings::i2c_board_info` avoids the const unsafe restriction and resists field drift.
+
+fn board_info() -> bindings::i2c_board_info {
+    // SAFETY: `i2c_board_info` is a plain C POD (char arrays, scalars and nullable
+    // pointers); all-zero is a valid value, and type_/addr are filled in afterwards.
+    let mut info: bindings::i2c_board_info = unsafe { core::mem::zeroed() };
+    let name = b"tmp108\0";
+    // Note: bindgen generates the C char array as [u8; N] (not [c_char; N]) in this
+    // configuration, so it is assigned u8 directly.
+    for (i, b) in name.iter().enumerate() {
+        info.type_[i] = *b;
+    }
+    info.addr = TMP108_ADDR;
+    info
+}
+
+// === SMBus primitives: word_data + Rust-side swab16 (replicating the i2c.h static inline) ===
+//
+// `include/linux/i2c.h` defines the swapped variants as static inline (no exported symbol):
+//   s32 i2c_smbus_read_word_swapped(client, command) {
+//       s32 value = i2c_smbus_read_word_data(client, command);
+//       return value < 0 ? value : swab16(value);      // error code returned as-is
+//   }
+//   s32 i2c_smbus_write_word_swapped(client, command, value) {
+//       return i2c_smbus_write_word_data(client, command, swab16(value));
+//   }
+// The exported word_data variants live in drivers/i2c/i2c-core-smbus.c
+// (EXPORT_SYMBOL). u16::swap_bytes() is equivalent to swab16.
+//
+// SAFETY: the signature matches the C prototype parameter by parameter; the caller
+// guarantees that client is valid.
+// Note on `#[allow(improper_ctypes)]`: in the bindings, `i2c_client` contains a
+// bindgen-generated **empty struct** via the `struct device -> struct mutex ->
+// lock_class_key` chain, which rustc deems not FFI-safe. In-tree precedents:
+// rust/bindings/lib.rs (crate-level allow), rust/uapi/lib.rs and
+// drivers/gpu/nova-core/gsp/fw/r570_144.rs (file-level allow). This can later go
+// through the kernel::i2c abstraction or via bindings_helper.h.
+#[allow(improper_ctypes)]
+unsafe extern "C" {
+    fn i2c_smbus_read_word_data(client: *mut bindings::i2c_client, command: u8) -> i32;
+    fn i2c_smbus_write_word_data(
+        client: *mut bindings::i2c_client,
+        command: u8,
+        value: u16,
+    ) -> i32;
+}
+
+// === find_closest_descending replication (util_macros.h) ===
+//
+// The C side is a **macro** (not a static inline) with no exported symbol, so it must be
+// replicated.
+//
+// Semantics (matched one by one):
+//  1. i counts **down** from len-1 to 1 (excluding 0, since the body reads a[i-1])
+//  2. match condition `x <= (a[i] + a[i-1]) / 2` (integer division, rounded down)
+//  3. on match, left = x - a[i], right = a[i-1] - x
+//  4. if **right < left (strictly)** then i-- (a tie at the midpoint does not step down)
+//  5. loop exhausted -> i == 0 (clamped to the largest step)
+//
+// Degenerate case: with `samples.len() == 1` i starts at 0 and the loop does not run, so it
+//   returns 0 (matching the C `as=1` -> `__fc_i=0` where the condition `0>=1` is false).
+fn find_closest_descending(x: c_long, samples: &[u16]) -> usize {
+    // An empty array has no index 0; the caller guarantees non-empty (SAMPLE_TIMES has 4).
+    if samples.is_empty() {
+        return 0;
+    }
+    let mut i: usize = samples.len() - 1;
+    while i >= 1 {
+        let mid = (samples[i] as c_long + samples[i - 1] as c_long) / 2;
+        if x <= mid {
+            let left = x - samples[i] as c_long;
+            let right = samples[i - 1] as c_long - x;
+            if right < left {
+                i -= 1;
+            }
+            break;
+        }
+        i -= 1;
+    }
+    i
+}
+
+// === Driver private data ===
+
+/// TMP108 driver private data.
+///
+/// - `client`: raw i2c client pointer (obtained from i2c_new_client_device during
+///   module init; lives as long as the module, freed by i2c_unregister_device in Drop)
+/// - `ready_time`: one-shot conversion ready time (a jiffies snapshot).
+///   It is a plain u64 rather than a Mutex: the new kernel::sync::Mutex::new requires a
+///   lock class key (pin_init API), while set_ready_time is never called here, so
+///   ready_time stays 0 (continuous mode, always ready). Enabling the one-shot path
+///   later will switch to the locked version, since the read callback uses &self.
+///
+/// Note: **deliberately not `pub`** -- the vtable macro injects
+/// `type OwnerModule = crate::LocalModule;` into every `#[vtable] impl` (an alias for
+/// the private type generated by `module!`, i.e. Tmp108Module). If `Self` were pub, that
+/// associated type would be part of the public interface and the private Tmp108Module
+/// would leak, triggering E0446 (private type in public interface).
+struct Tmp108 {
+    client: *mut bindings::i2c_client,
+    ready_time: u64,
+}
+
+// SAFETY: `client` is a raw pointer owned by this module: it is created in `init`, only
+// dereferenced by the hwmon callbacks while the registration is live, and unregistered in
+// `drop` after `hwmon_device_unregister()` has drained any in-flight callback. `ready_time`
+// is a plain `u64` that this revision never writes.
+unsafe impl Send for Tmp108 {}
+
+// SAFETY: the read/write callbacks access `client` via `&self` only to issue i2c
+// transactions (the i2c core serializes transfers on the same adapter itself); with no
+// write path, ready_time is read-only, so there is no data race.
+unsafe impl Sync for Tmp108 {}
+
+impl Tmp108 {
+    /// `client` must be a valid pointer living for the whole module (created in init,
+    /// unregistered in Drop). Note: not `pub` -- Tmp108 is private and a `pub fn` would
+    /// trip `-D unreachable-pub` (the kernel tree enables -D warnings); a plain `fn` suffices.
+    fn new(client: *mut bindings::i2c_client) -> Self {
+        Self {
+            client,
+            ready_time: 0,
+        }
+    }
+
+    // === i2c primitives (word_data + Rust-side swab16 = word_swapped) ===
+
+    /// Read-side replication of `value < 0 ? value : swab16(value)`: an error code is
+    /// returned as-is, otherwise the value is byte-swapped. Returns the driver-visible
+    /// register value (= swab16 of the on-wire word).
+    fn read_word_swapped(&self, cmd: u8) -> Result<u16> {
+        // SAFETY: `client` lives for the whole module (struct invariant); the word_data
+        // helper only requires `client` to be valid.
+        let v = unsafe { i2c_smbus_read_word_data(self.client, cmd) };
+        if v < 0 {
+            return Err(Error::from_errno(v));
+        }
+        Ok((v as u16).swap_bytes())
+    }
+
+    /// Write-side replication: `i2c_smbus_write_word_data(client, cmd, swab16(value))`.
+    fn write_word_swapped(&self, cmd: u8, val: u16) -> Result {
+        // SAFETY: same as `read_word_swapped`.
+        let rc = unsafe { i2c_smbus_write_word_data(self.client, cmd, val.swap_bytes()) };
+        if rc < 0 {
+            return Err(Error::from_errno(rc));
+        }
+        Ok(())
+    }
+
+    fn read_conf(&self) -> Result<u16> {
+        self.read_word_swapped(CONF as u8)
+    }
+
+    fn write_conf(&self, val: u16) -> Result {
+        // CONF is writable (TM/MODE/CR/HYS/POL bits).
+        self.write_word_swapped(CONF as u8, val)
+    }
+
+    /// Reads TEMP/TLOW/THIGH (all three share the same encoding: 12-bit left-justified
+    /// two's complement).
+    fn read_temp_reg(&self, reg: u16) -> Result<c_long> {
+        let v = self.read_word_swapped(reg as u8)?;
+        // reg -> mC: `(val & ~0x0f) * 1000 / 256` with val as s16.
+        // The C code computes in int; i32 is required because a direct i16 multiply by 1000
+        // overflows (32000*1000 > i16::MAX). Rust integer division truncates toward zero
+        // (as in C), so no special handling is needed.
+        let mc = i32::from((v & !0x0f) as i16) * 1000 / 256;
+        Ok(mc as c_long)
+    }
+
+    /// Writes TLOW/THIGH: clamp, then convert.
+    fn write_threshold(&self, reg: u16, value_mc: c_long) -> Result {
+        let clamped = value_mc.clamp(CLAMP_MIN_MC as c_long, CLAMP_MAX_MC as c_long);
+        // `(val * 256) / 1000`, truncating toward zero; the C code computes in int and the
+        // post-clamp range [-50000, 127937] makes the i32 computation lossless.
+        let reg_val = ((clamped as i32) * 256 / 1000) as u16;
+        // TLOW/THIGH are writable (the C code imposes no read-only restriction).
+        self.write_word_swapped(reg as u8, reg_val)
+    }
+
+    // === Hysteresis decode/encode ===
+
+    /// CONF.HYS1:HYS0 -> 0/1000/2000/4000 mC
+    fn hyst_mc(&self, conf: u16) -> i32 {
+        match (conf & CONF_HYS_MASK) >> 4 {
+            0 => 0,
+            1 => 1000,
+            2 => 2000,
+            _ => 4000,
+        }
+    }
+
+    /// Bucketing: returns the CONF.HYS field value (0x0000/0x0010/0x0020/0x0030)
+    fn hyst_field(deg_mc: i32) -> u16 {
+        let code = if deg_mc < HYST_STEP_1_MC {
+            0
+        } else if deg_mc < HYST_STEP_2_MC {
+            1
+        } else if deg_mc < HYST_STEP_4_MC {
+            2
+        } else {
+            3
+        };
+        code << 4
+    }
+
+    /// One-shot window check: returns false when not ready (mapped to EAGAIN).
+    /// `ready_time == 0` means continuous mode (always ready).
+    /// With no one-shot write path, ready_time stays 0, so this is always ready; the
+    /// jiffies comparison arrives together with set_ready_time.
+    fn one_shot_ready(&self) -> bool {
+        self.ready_time == 0
+    }
+
+    // === Chip channel update_interval ===
+
+    /// Reads `update_interval` (in **ms**, not mC).
+    ///
+    /// Reads CONF, extracts the CONVRATE field and indexes `sample_times`:
+    /// `FIELD_GET(GENMASK(14,13), v)` == `(v & 0x6000) >> 13`.
+    fn read_update_interval(&self) -> Result<c_long> {
+        let conf = self.read_conf()?;
+        let idx = ((conf & CONF_CONVRATE_MASK) >> CONF_CR0_POS) as usize;
+        // idx comes from a 2-bit field, so it is always < 4 == SAMPLE_TIMES.len(),
+        // making the index safe.
+        Ok(SAMPLE_TIMES[idx] as c_long)
+    }
+
+    /// Writes `update_interval`: pick the step with `find_closest_descending`, then
+    /// read-modify-write.
+    ///
+    /// The C driver uses `regmap_update_bits(regmap, CONF, CONVRATE_MASK, FIELD_PREP(FLD, idx))`.
+    /// This driver does not use regmap (it goes through i2c_smbus_word_data), so this is
+    /// expanded into a read-modify-write (the same shape as the hysteresis write):
+    ///   `conf = (conf & !MASK) | (idx << 13)`
+    ///
+    /// There is no clamp here (unlike the temperature thresholds); out-of-range values are
+    /// clamped by find_closest_descending.
+    fn write_update_interval(&self, value: c_long) -> Result {
+        let idx = find_closest_descending(value, &SAMPLE_TIMES);
+        let mut conf = self.read_conf()?;
+        conf = (conf & !CONF_CONVRATE_MASK) | ((idx as u16) << CONF_CR0_POS);
+        self.write_conf(conf)
+    }
+}
+
+// === hwmon Driver implementation (attribute by attribute) ===
+
+#[vtable]
+impl Driver for Tmp108 {
+    fn read(&self, attr: TempAttr, channel: u32) -> Result<c_long> {
+        // The channel is fixed at 0 (hwmon channel numbers start at 0; TMP108 has a single
+        // temperature channel). The abstraction already rejects non-temp types, so this is a
+        // defensive check on channel.
+        if channel != 0 {
+            return Err(EINVAL);
+        }
+
+        match attr {
+            TempAttr::Input => {
+                // One-shot window check: not ready -> -EAGAIN
+                if !self.one_shot_ready() {
+                    return Err(EAGAIN);
+                }
+                self.read_temp_reg(TEMP)
+            }
+            TempAttr::Min => self.read_temp_reg(TLOW),
+            TempAttr::Max => self.read_temp_reg(THIGH),
+            TempAttr::MinAlarm => {
+                // !!(regval & CONF_FL)
+                let conf = self.read_conf()?;
+                Ok(if conf & CONF_FL != 0 { 1 } else { 0 })
+            }
+            TempAttr::MaxAlarm => {
+                // !!(regval & CONF_FH)
+                let conf = self.read_conf()?;
+                Ok(if conf & CONF_FH != 0 { 1 } else { 0 })
+            }
+            TempAttr::MinHyst => {
+                // min_hyst = TLOW_mC + hyst_mC (absolute-value semantics)
+                let conf = self.read_conf()?;
+                let tlow = self.read_temp_reg(TLOW)?;
+                Ok(tlow + self.hyst_mc(conf) as c_long)
+            }
+            TempAttr::MaxHyst => {
+                // max_hyst = THIGH_mC - hyst_mC
+                let conf = self.read_conf()?;
+                let thigh = self.read_temp_reg(THIGH)?;
+                Ok(thigh - self.hyst_mc(conf) as c_long)
+            }
+        }
+    }
+
+    fn write(&self, attr: TempAttr, channel: u32, value: c_long) -> Result {
+        if channel != 0 {
+            return Err(EINVAL);
+        }
+
+        match attr {
+            TempAttr::Min => self.write_threshold(TLOW, value),
+            TempAttr::Max => self.write_threshold(THIGH, value),
+            TempAttr::MinHyst | TempAttr::MaxHyst => {
+                // The requested value is clamped first, then differenced against the current
+                // register value: min_hyst: delta = request - TLOW_mC; max_hyst:
+                // delta = THIGH_mC - request. Only the CONF.HYS field is changed, leaving
+                // TLOW/THIGH untouched (regmap_update_bits).
+                let value = value.clamp(CLAMP_MIN_MC as c_long, CLAMP_MAX_MC as c_long);
+                let reg = if attr == TempAttr::MinHyst { TLOW } else { THIGH };
+                let current = self.read_temp_reg(reg)?;
+                let delta = if attr == TempAttr::MinHyst {
+                    (value - current) as i32
+                } else {
+                    (current - value) as i32
+                };
+                let field = Self::hyst_field(delta);
+                let mut conf = self.read_conf()?;
+                conf = (conf & !CONF_HYS_MASK) | field;
+                self.write_conf(conf)
+            }
+            // Read-only attributes (no C write implementation): `is_visible` returns 0444, so
+            // the framework never calls write. They must be listed explicitly (a `_` arm would
+            // be unreachable since all 7 variants are covered; omitting them would trigger
+            // E0004 non-exhaustiveness).
+            TempAttr::Input | TempAttr::MinAlarm | TempAttr::MaxAlarm => Err(EOPNOTSUPP),
+        }
+    }
+
+    fn is_visible(&self, attr: TempAttr, channel: u32) -> u16 {
+        if channel != 0 {
+            return 0;
+        }
+        // Matched attribute by attribute:
+        // - Input/MinAlarm/MaxAlarm: unconditionally 0444
+        // - Min/Max: unconditionally 0644
+        // - MinHyst/MaxHyst: conditionally 0644, visible only when config_reg_16bits=true;
+        //   this driver supports only the tmp108 (16-bit CONF), so it is always true
+        // - Crit family: not exposed by TMP108 (the C default branch)
+        // - chip type (update_interval): see `chip_is_visible` below
+        match attr {
+            TempAttr::Input | TempAttr::MinAlarm | TempAttr::MaxAlarm => 0o444,
+            TempAttr::Min | TempAttr::Max => 0o644,
+            TempAttr::MinHyst | TempAttr::MaxHyst => 0o644,
+            // Note: no catch-all arm -- all 7 variants are enumerated (none maps to
+            // permission 0).
+        }
+    }
+
+    // === Chip channel ===
+    //
+    // The methods are declared `#[optional]` (no default body) in `rust/kernel/hwmon.rs`;
+    // when not overridden, `HAS_CHIP_* == false` and the abstraction registers no callback.
+
+    /// Reads a chip-level attribute. `ChipAttr` has a single variant, so the match is
+    /// exhaustive.
+    fn chip_read(&self, attr: ChipAttr) -> Result<c_long> {
+        match attr {
+            ChipAttr::UpdateInterval => self.read_update_interval(),
+        }
+    }
+
+    /// Writes a chip-level attribute.
+    fn chip_write(&self, attr: ChipAttr, value: c_long) -> Result {
+        match attr {
+            ChipAttr::UpdateInterval => self.write_update_interval(value),
+        }
+    }
+
+    /// Chip attribute permissions: `tmp108_is_visible` returns 0644 unconditionally for
+    /// the chip channel.
+    fn chip_is_visible(&self, attr: ChipAttr) -> u16 {
+        match attr {
+            ChipAttr::UpdateInterval => 0o644,
+        }
+    }
+}
+
+// === Module ===
+
+module! {
+    type: Tmp108Module,
+    name: "tmp108",
+    authors: ["skxby <2374604267@qq.com>"],
+    description: "TMP108 temperature sensor driver (Rust, i2c + hwmon)",
+    license: "GPL",
+    params: {
+        busnum: i64 {
+            default: 0,
+            description: "I2C bus number of the adapter hosting the TMP108 (i2c-stub in tests)",
+        },
+    },
+}
+
+struct Tmp108Module {
+    // The Option is only for Drop ordering: unregister hwmon first (its barrier drains
+    // in-flight callbacks), then unregister client.
+    hwmon: Option<Registration<Tmp108>>,
+    client: *mut bindings::i2c_client,
+}
+
+// SAFETY: the `client` pointer lives for the whole module (Drop ordering unregisters
+// hwmon first and client afterwards, so nothing dangles past the barrier). Moving the
+// pointer value across threads is not a data race: client is only touched in init/drop
+// (mutually exclusive module lifecycle callbacks) and in the hwmon read/write callbacks
+// (synchronized by Registration's unregister barrier). The Send/Sync of the `hwmon`
+// field is proven field by field by the abstraction's Registration SAFETY comment.
+unsafe impl Send for Tmp108Module {}
+
+// SAFETY: sharing a `&Tmp108Module` cannot produce a data race -- `client` is only read
+// in `drop` (&mut self), and the `hwmon` field is Sync (proven by the abstraction).
+unsafe impl Sync for Tmp108Module {}
+
+impl kernel::Module for Tmp108Module {
+    fn init(_module: &'static ThisModule) -> Result<Self> {
+        let busnum = module_parameters::busnum.value() as i32;
+        pr_info!("TMP108: attaching to i2c bus {}\n", busnum);
+
+        // SAFETY: i2c_get_adapter is an EXPORT_SYMBOL kernel API; nr is the bus number and
+        // the call returns a referenced adapter, or null if the bus does not exist.
+        let adapter = unsafe { bindings::i2c_get_adapter(busnum) };
+        if adapter.is_null() {
+            return Err(ENODEV);
+        }
+
+        let board_info = board_info();
+
+        // SAFETY: adapter is valid (null-checked above); board_info is a fully initialized
+        // i2c_board_info (type_/addr filled in, the rest zero, which i2c-core allows).
+        let client = unsafe { bindings::i2c_new_client_device(adapter, &board_info) };
+        // The client's dev kobject holds a reference to the adapter via kobject_add (the
+        // driver-model parent/child relationship), so the reference taken by i2c_get_adapter
+        // is dropped here.
+        // SAFETY: adapter was obtained above via get_adapter; its reference count is held on
+        // the client side.
+        unsafe { bindings::i2c_put_adapter(adapter) };
+
+        // from_err_ptr handles ERR_PTR (the i2c_new_client_device failure path); the null
+        // check is a second line of defense (the path should not return plain null).
+        let client = from_err_ptr(client)?;
+        if client.is_null() {
+            return Err(ENODEV);
+        }
+
+        // SAFETY: client was created successfully above (non-null, non-ERR_PTR); from_raw
+        // only reinterprets the pointer as a &Device. Lifetime argument: client lives for the
+        // whole module (Drop unregisters hwmon before client) and this reference is only used
+        // as the registration parent argument. client was just registered, so its dev context
+        // is Bound.
+        let parent: &Device<Bound> = unsafe { Device::from_raw(&raw mut (*client).dev) };
+
+        // Registration failure path: if `Registration::new` fails and `?` returns early, the
+        // already-created i2c client is never unregistered. Such a leftover client keeps the
+        // bus address 0x48 occupied forever, making every subsequent insmod fail with EBUSY.
+        // The failure branch must therefore unregister the client explicitly (cleanup on the
+        // hwmon side is handled by the hwmon core itself).
+        let hwmon = match Registration::new(parent, c"tmp108", Tmp108::new(client)) {
+            Ok(hwmon) => hwmon,
+            Err(e) => {
+                // SAFETY: client was created successfully in this function (non-null,
+                // non-ERR_PTR) and is not yet owned by hwmon (registration failed), so no
+                // in-flight callback touches the pointer.
+                unsafe { bindings::i2c_unregister_device(client) };
+                return Err(e);
+            }
+        };
+
+        pr_info!("TMP108: hwmon device registered\n");
+        Ok(Self {
+            hwmon: Some(hwmon),
+            client,
+        })
+    }
+}
+
+impl Drop for Tmp108Module {
+    fn drop(&mut self) {
+        // Unregistration order (critical): hwmon first -- Registration::drop calls
+        // hwmon_device_unregister, which blocks until all in-flight callbacks return, after
+        // which no callback touches the Tmp108.client pointer; then unregister the i2c
+        // client. The reverse order is a callback UAF.
+        drop(self.hwmon.take());
+        if !self.client.is_null() {
+            // SAFETY: client was created by this module's init; hwmon has already been
+            // unregistered (take + drop above), so no in-flight callback accesses client.
+            unsafe { bindings::i2c_unregister_device(self.client) };
+        }
+    }
+}

Re: [PATCH 1/2] rust: hwmon: add hwmon abstractions

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] hwmon: use-after-free of hwdev->name due to unconstrained &CStr lifetime
- [High] hwmon: out-of-bounds read due to missing null-terminator in chip_config
- [High] hwmon: linker error for drivers omitting #[optional] chip_read/write
- [Medium] hwmon: missing hwmon_lock() API and incorrect locking documentation
- [Medium] hwmon: manual pointer wiring bypasses required pin-init API
- [Low] hwmon: abstraction limits drivers to a single temperature channel
- [Low] hwmon: missing #[inline] attribute on Registration drop method
--

commit a2d95b04547e27fa736c176ee2cb6a3c7255a3eb
Author: skxby <2374604267@qq.com>
Date:   Fri Oct 16 15:12:27 2026 +0800

rust: hwmon: add hwmon abstractions
    
Add Rust abstractions for the hwmon subsystem, supporting temp[1-*]
attributes and chip-level attributes. Registration owns its inner state
and uses a Driver trait for callbacks.

Link: https://lore.kernel.org/all/20260512155747.10136-1-donplat@barrensea.org/

> diff --git a/rust/kernel/hwmon.rs b/rust/kernel/hwmon.rs
> new file mode 100644
> index 0000000000000..d7e3fd683b862
> --- /dev/null
> +++ b/rust/kernel/hwmon.rs
[ ... ]
> +/// The hwmon driver trait.
> +///
> +/// # Concurrency and Driver Responsibilities
> +///
> +/// The hwmon C core provides **no internal locking** for attribute access. When user-space
                                     ^^^^^^^^^^^^^^^^^^^^^^
[Severity: Medium]
This isn't a bug, but is this statement accurate?

The documentation incorrectly claims that the 'hwmon C core provides no
internal locking for attribute access'. The C core uses a per-device mutex
(guard(mutex)(&hwdev->lock)) during sysfs attribute reads and writes.

Furthermore, the hwmon subsystem guidelines explicitly state that drivers
should use hwmon_lock() and hwmon_unlock() to implement locking required
for interrupt handling. This abstraction fails to expose or wrap these
functions, making it impossible for Rust drivers to comply with this
official rule.

> +/// reads or writes `/sys/class/hwmon/hwmon*/temp*_*`, the core directly invokes the
> +/// registered callbacks with a **shared `&self` reference** to `HwmonInner<T>`.
[ ... ]
> +    unsafe extern "C" fn read_callback(
> +        dev: *mut bindings::device,
> +        type_: u32,
> +        attr: u32,
> +        channel: c_int,
> +        val: *mut c_long,
> +    ) -> c_int {
> +        from_result(|| {
[ ... ]
> +            let result = match SensorType::try_from(type_)? {
> +                SensorType::Chip => T::chip_read(&inner.driver, ChipAttr::try_from(attr)?)?,
                                       ^^^^^^^^^^^
[Severity: High]
Does this code cause a compilation failure for drivers that do not implement
the optional chip methods?

The Driver trait marks chip_read and chip_write as #[optional]. The
#[vtable] macro replaces missing optional methods with a build_error!()
call. Here, the calls to T::chip_read and T::chip_write are unconditional
within the dynamically evaluated SensorType::Chip branch.

Because the compiler cannot prove this branch is dead at compile time, the
build_error!() call is retained in the binary, causing a fatal linker error
for any driver that chooses not to implement these methods.

Should the calls be guarded by if T::HAS_CHIP_READ and
if T::HAS_CHIP_WRITE?

> +                // C core guarantees `channel >= 0`.
> +                SensorType::Temp => {
> +                    T::read(&inner.driver, TempAttr::try_from(attr)?, channel as u32)?
> +                }
> +            };
[ ... ]
> +    fn new_placeholder(driver: T) -> Self {
> +        // A driver that exposes no chip attribute must not advertise the chip channel
> +        // either: leaving `chip_config` zeroed makes the channel carry no attributes, and
> +        // `is_visible` would not be consulted for it in the first place.
> +        let chip_config = if T::HAS_CHIP_IS_VISIBLE {
> +            [CHIP_ATTR_MASK]
             ^^^^^^^^^^^^^^^^
[Severity: High]
Can this out-of-bounds read adjacent memory when the C core reads the
configuration?

The C hwmon core strictly requires the hwmon_channel_info.config array to
be null-terminated (ending with a 0). Here, chip_config is defined as
[u32; 1] and initialized to [CHIP_ATTR_MASK]. It lacks a trailing 0
terminator.

When hwmon_device_register_with_info reads the channel configuration, it
will fail to stop at the end of chip_config and read past the array bounds
into adjacent memory, which can cause the C core to misinterpret memory as
attribute masks.

> +        } else {
> +            [0]
> +        };
> +
> +        Self {
> +            driver,
> +            ops: bindings::hwmon_ops {
> +                is_visible: Some(Adapter::<T>::is_visible_callback),
> +                visible: 0,
> +                read: Some(Adapter::<T>::read_callback),
> +                read_string: None,
> +                write: Some(Adapter::<T>::write_callback),
> +            },
> +            chip_config,
> +            temp_config: [TEMP_ATTR_MASK, 0],
                            ^^^^^^^^^^^^^^^^^
[Severity: Low]
Does this hardcoded array strictly limit the abstraction to a single
temperature channel?

The commit message asserts that the abstraction supports registering
temperature sensors with the full standard temp[1-*] attribute set.
However, this single configuration mask means the C core will only create
attributes for temp1_* and will never query channel > 0, preventing drivers
from registering multiple temperature sensors.

> +            chip_channel: bindings::hwmon_channel_info {
> +                type_: 0,
> +                config: core::ptr::null(),
> +            },
[ ... ]
> +    pub fn new(parent: &Device<Bound>, name: &CStr, data: T) -> Result<Self> {
> +        let mut boxed = KBox::new(HwmonInner::new_placeholder(data), GFP_KERNEL)?;
> +
> +        // Wire self-referential pointers before pinning. We obtain a mutable raw pointer
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: Medium]
Does this manual pointer wiring bypass the required pin-init API?

The Rust subsystem guidelines require that try_pin_init! or pin_init! be
used to initialize structs that require pinning. The abstraction bypasses
this safe pin-init framework by allocating a KBox, acquiring a raw pointer,
mutating fields unsafely to wire self-referential pointers, and manually
calling Pin::new_unchecked.

> +        // from the uniquely-owned `KBox` -- no intermediate `&mut` reference is created,
> +        // so no aliasing rules are violated.
> +        let inner_ptr: *mut HwmonInner<T> = &raw mut *boxed;
[ ... ]
> +        // SAFETY: `chip_info` and all nested pointers target memory within the same
> +        // allocation, which remains valid until `hwmon_device_unregister` in `Drop`.
> +        // `parent.as_raw()` and `name.as_char_ptr()` are valid. The name is copied by
> +        // the kernel (via `dev_set_name` -> `kvasprintf_const`), so no lifetime issue.
> +        let hwmon_dev = from_err_ptr(unsafe {
> +            bindings::hwmon_device_register_with_info(
> +                parent.as_raw(),
> +                name.as_char_ptr(),
                 ^^^^^^^^^^^^^^^^^^
[Severity: High]
Can this cause a use-after-free of hwdev->name?

The Registration::new function accepts a name: &CStr parameter without a
lifetime constraint tying it to the Registration object. The C core stores
this pointer directly in hwdev->name (which is returned by the sysfs
name_show attribute) without copying the string.

If a caller passes a temporary string (e.g., a short-lived CString), the
string will be freed after registration. When userspace later reads
/sys/class/hwmon/hwmon*/name, the C core will dereference the freed memory.

> +                drvdata,
> +                &raw const (*inner_ptr).chip_info,
> +                core::ptr::null_mut(),
> +            )
> +        })?;
> +
> +        Ok(Self { hwmon_dev, inner })
> +    }
> +}
> +
> +impl<T: Driver> Drop for Registration<T> {
> +    fn drop(&mut self) {
       ^^^^^^^
[Severity: Low]
This isn't a bug, but should this method have an #[inline] attribute?

According to the Rust subsystem guidelines, small functions or functions
forwarding to a C binding call in abstractions should be annotated with
#[inline]. This method forwards directly to bindings::hwmon_device_unregister
but lacks this required annotation.

> +        // SAFETY: `hwmon_dev` was returned by a successful registration. This call waits for
> +        // all in-flight callbacks before returning.
> +        unsafe { bindings::hwmon_device_unregister(self.hwmon_dev) };
> +    }
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_95E7F9B5CE294CA4027A0CD4C550B454050A@qq.com?part=1

Re: [PATCH 2/2] hwmon: (tmp108) add Rust TMP108 driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] hwmon: tmp108: panic via arithmetic underflow in write_update_interval()
- [Medium] hwmon/tmp108: missing hardware initialization in init
- [Low] hwmon: hardware monitoring driver registered from outside drivers/hwmon/ violates layering
- [Low] kconfig: SAMPLE_RUST_TMP108 tristate config depends on HWMON=y preventing module builds
--

commit 78120c93b1be27841c1bf9909b41399a14b830f6
Author: skxby <2374604267@qq.com>

hwmon: (tmp108) add Rust TMP108 driver
    
Add a Rust driver for the TI TMP108 temperature sensor, mirroring the
existing C driver (drivers/hwmon/tmp108.c).

> diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
> index 31d62533ef25f..18df4282756c3 100644
> --- a/samples/rust/Kconfig
> +++ b/samples/rust/Kconfig
[ ... ]
> +config SAMPLE_RUST_TMP108
> +	tristate "TMP108 Driver"
> +	depends on HWMON=y
> +	depends on I2C=y

[Severity: Low]
Does this introduce a Kconfig regression? If CONFIG_HWMON=m or CONFIG_I2C=m,
this prevents the sample driver from being built at all, even as a module.

Should standard dependency strings like depends on HWMON and
depends on I2C be used to inherit module constraints properly?

> diff --git a/samples/rust/tmp108.rs b/samples/rust/tmp108.rs
> new file mode 100755
> index 0000000000000..a070547f29ee4
> --- /dev/null
> +++ b/samples/rust/tmp108.rs
[ ... ]
> +fn find_closest_descending(x: c_long, samples: &[u16]) -> usize {
[ ... ]
> +    while i >= 1 {
> +        let mid = (samples[i] as c_long + samples[i - 1] as c_long) / 2;
> +        if x <= mid {
> +            let left = x - samples[i] as c_long;

[Severity: High]
Can this code panic due to an arithmetic underflow regression?

If userspace writes a large negative value (e.g., LONG_MIN) to the
update_interval sysfs attribute, it is passed unmodified to this function.

Subtracting a positive sample value from c_long::MIN will underflow here.

In debug builds or when CONFIG_RUST_OVERFLOW_CHECKS=y is set, this
triggers a panic, crashing the kernel. In release builds, it silently
wraps around and sets an incorrect hardware conversion rate.

Could we clamp the value before subtraction, or use saturating math?

[ ... ]
> +    fn write_update_interval(&self, value: c_long) -> Result {
> +        let idx = find_closest_descending(value, &SAMPLE_TIMES);

[Severity: High]
Is it possible to pass an unclamped value here?

Since the value passed from userspace via sysfs is not clamped before
calling find_closest_descending(), an extreme negative value can reach
the subtraction above and cause an underflow panic.

[ ... ]
> +    fn init(_module: &'static ThisModule) -> Result<Self> {
[ ... ]
> +        // Registration failure path: if `Registration::new` fails and `?` returns early, the
> +        // already-created i2c client is never unregistered. Such a leftover client keeps the
> +        // bus address 0x48 occupied forever, making every subsequent insmod fail with EBUSY.
> +        // The failure branch must therefore unregister the client explicitly (cleanup on the
> +        // hwmon side is handled by the hwmon core itself).
> +        let hwmon = match Registration::new(parent, c"tmp108", Tmp108::new(client)) {

[Severity: Low]
Does registering a hardware monitoring device from outside drivers/hwmon/
violate subsystem layering rules?

The HWMON subsystem guidelines state that hardware monitoring drivers should
reside in drivers/hwmon/. While this is a sample driver, directly using
the hwmon registration API from samples/rust/ bypasses this architectural
layout rule.

[Severity: Medium]
Are we missing hardware initialization for the TMP108 device during init?

Unlike the C driver, this code registers the hwmon device without enabling
the regulator or configuring the CONF register (e.g., writing
TMP108_MODE_CONTINUOUS and clearing TMP108_CONF_TM).

If the chip is in shutdown mode from the bootloader, continuous conversions
will not occur, and accessing sysfs attributes like temp1_input could
return stale temperature readings to userspace.

> +            Ok(hwmon) => hwmon,
> +            Err(e) => {
> +                // SAFETY: client was created successfully in this function (non-null,
> +                // non-ERR_PTR) and is not yet owned by hwmon (registration failed), so no
> +                // in-flight callback touches the pointer.
> +                unsafe { bindings::i2c_unregister_device(client) };
> +                return Err(e);
> +            }
> +        };
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_95E7F9B5CE294CA4027A0CD4C550B454050A@qq.com?part=2