lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <C5AFA553-6EB1-453E-B396-DD19139E7228@collabora.com>
Date: Wed, 27 Aug 2025 16:38:46 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Igor Korotin <igor.korotin.linux@...il.com>
Cc: Miguel Ojeda <ojeda@...nel.org>,
 Alex Gaynor <alex.gaynor@...il.com>,
 Wolfram Sang <wsa+renesas@...g-engineering.com>,
 Boqun Feng <boqun.feng@...il.com>,
 Gary Guo <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>,
 Benno Lossin <lossin@...nel.org>,
 Andreas Hindborg <a.hindborg@...nel.org>,
 Alice Ryhl <aliceryhl@...gle.com>,
 Trevor Gross <tmgross@...ch.edu>,
 Danilo Krummrich <dakr@...nel.org>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Viresh Kumar <viresh.kumar@...aro.org>,
 Asahi Lina <lina+kernel@...hilina.net>,
 Wedson Almeida Filho <wedsonaf@...il.com>,
 Alex Hung <alex.hung@....com>,
 Tamir Duberstein <tamird@...il.com>,
 Xiangfei Ding <dingxiangfei2009@...il.com>,
 linux-kernel@...r.kernel.org,
 rust-for-linux@...r.kernel.org,
 linux-i2c@...r.kernel.org
Subject: Re: [PATCH v4 3/3] samples: rust: add Rust I2C sample driver

Hi Igor,

> On 20 Aug 2025, at 12:23, Igor Korotin <igor.korotin.linux@...il.com> wrote:
> 
> Add a new `rust_driver_i2c` sample, showing how to create a new
> i2c client using `i2c::Registration` and bind a driver to it
> via legacy I2C-ID table.
> 
> Signed-off-by: Igor Korotin <igor.korotin.linux@...il.com>
> ---
> MAINTAINERS                     |   1 +
> samples/rust/Kconfig            |  11 +++
> samples/rust/Makefile           |   1 +
> samples/rust/rust_driver_i2c.rs | 128 ++++++++++++++++++++++++++++++++
> 4 files changed, 141 insertions(+)
> create mode 100644 samples/rust/rust_driver_i2c.rs
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c44c7ac317b1..2654a7ea0c80 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11523,6 +11523,7 @@ R: Daniel Almeida <daniel.almeida@...labora.com>
> L: rust-for-linux@...r.kernel.org
> S: Maintained
> F: rust/kernel/i2c.rs
> +F: samples/rust/rust_driver_i2c.rs
> 
> I2C SUBSYSTEM HOST DRIVERS
> M: Andi Shyti <andi.shyti@...nel.org>
> diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
> index 7f7371a004ee..28dae070b365 100644
> --- a/samples/rust/Kconfig
> +++ b/samples/rust/Kconfig
> @@ -62,6 +62,17 @@ config SAMPLE_RUST_DMA
> 
>  If unsure, say N.
> 
> +config SAMPLE_RUST_DRIVER_I2C
> + tristate "I2C Driver"
> + depends on I2C=y
> + help
> +  This option builds the Rust I2C driver sample.
> +
> +  To compile this as a module, choose M here:
> +  the module will be called rust_driver_i2c.
> +
> +  If unsure, say N.
> +
> config SAMPLE_RUST_DRIVER_PCI
> tristate "PCI Driver"
> depends on PCI
> diff --git a/samples/rust/Makefile b/samples/rust/Makefile
> index bd2faad63b4f..141d8f078248 100644
> --- a/samples/rust/Makefile
> +++ b/samples/rust/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_SAMPLE_RUST_MINIMAL) += rust_minimal.o
> obj-$(CONFIG_SAMPLE_RUST_MISC_DEVICE) += rust_misc_device.o
> obj-$(CONFIG_SAMPLE_RUST_PRINT) += rust_print.o
> obj-$(CONFIG_SAMPLE_RUST_DMA) += rust_dma.o
> +obj-$(CONFIG_SAMPLE_RUST_DRIVER_I2C) += rust_driver_i2c.o
> obj-$(CONFIG_SAMPLE_RUST_DRIVER_PCI) += rust_driver_pci.o
> obj-$(CONFIG_SAMPLE_RUST_DRIVER_PLATFORM) += rust_driver_platform.o
> obj-$(CONFIG_SAMPLE_RUST_DRIVER_FAUX) += rust_driver_faux.o
> diff --git a/samples/rust/rust_driver_i2c.rs b/samples/rust/rust_driver_i2c.rs
> new file mode 100644
> index 000000000000..6dfc299d5aea
> --- /dev/null
> +++ b/samples/rust/rust_driver_i2c.rs
> @@ -0,0 +1,128 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Rust I2C driver sample.
> +//!
> +//! This module shows how to:
> +//!
> +//! 1. Manually create an `i2c_client` at address `SAMPLE_I2C_CLIENT_ADDR`
> +//!    on the adapter with index `SAMPLE_I2C_ADAPTER_INDEX`.

Blank here.

> +//! 2. Register a matching Rust-based I2C driver for that client.
> +//!
> +//! # Requirements
> +//!
> +//! - The target system must expose an I2C adapter at index
> +//!   `SAMPLE_I2C_ADAPTER_INDEX`.

Blank here

> +//! - To emulate an adapter for testing, you can load the
> +//!   `i2c-stub` kernel module with an option `chip_addr`
> +//!   For example for this sample driver to emulate an I2C device with
> +//!   an address 0x30 you can use:
> +//!      `modprobe i2c-stub chip_addr=0x30`
> +//!
> +
> +use kernel::{
> +    acpi, c_str,
> +    device::{Core, Normal},
> +    i2c, of,
> +    prelude::*,
> +    types::ARef,
> +};
> +
> +const SAMPLE_I2C_CLIENT_ADDR: u16 = 0x30;
> +const SAMPLE_I2C_ADAPTER_INDEX: i32 = 0;
> +const BOARD_INFO: i2c::I2cBoardInfo =
> +    i2c::I2cBoardInfo::new(c_str!("rust_driver_i2c"), SAMPLE_I2C_CLIENT_ADDR);
> +
> +struct SampleDriver {
> +    pdev: ARef<i2c::I2cClient>,

FYI: the pdev nomenclature is still here.

> +}
> +
> +kernel::acpi_device_table! {
> +    ACPI_TABLE,
> +    MODULE_ACPI_TABLE,
> +    <SampleDriver as i2c::Driver>::IdInfo,
> +    [(acpi::DeviceId::new(c_str!("LNUXBEEF")), 0)]
> +}
> +
> +kernel::i2c_device_table! {
> +    I2C_TABLE,
> +    MODULE_I2C_TABLE,
> +    <SampleDriver as i2c::Driver>::IdInfo,
> +    [(i2c::DeviceId::new(c_str!("rust_driver_i2c")), 0)]
> +}
> +
> +kernel::of_device_table! {
> +    OF_TABLE,
> +    MODULE_OF_TABLE,
> +    <SampleDriver as i2c::Driver>::IdInfo,
> +    [(of::DeviceId::new(c_str!("test,rust_driver_i2c")), 0)]
> +}
> +
> +impl i2c::Driver for SampleDriver {
> +    type IdInfo = u32;
> +
> +    const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
> +    const I2C_ID_TABLE: Option<i2c::IdTable<Self::IdInfo>> = Some(&I2C_TABLE);
> +    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
> +
> +    fn probe(pdev: &i2c::I2cClient<Core>, info: Option<&Self::IdInfo>) -> Result<Pin<KBox<Self>>> {
> +        let dev = pdev.as_ref();
> +
> +        dev_dbg!(dev, "Probe Rust I2C driver sample.\n");
> +
> +        if let Some(info) = info {
> +            dev_info!(dev, "Probed with info: '{}'.\n", info);
> +        }
> +
> +        let drvdata = KBox::new(Self { pdev: pdev.into() }, GFP_KERNEL)?;
> +
> +        Ok(drvdata.into())
> +    }
> +
> +    fn shutdown(pdev: &i2c::I2cClient<Core>) {
> +        dev_dbg!(pdev.as_ref(), "Shutdown Rust I2C driver sample.\n");

This should probably be dev_info!()

I know that in general people want drivers to be quiet but:

a) this a sample driver that is probably the only way to test the abstractions
for the time being, so we must know whether probe() and shudown() are working
by inspecting these traces.

b) Unless something changed recently, there is no way to get dev_dbg() to print
in Rust for now, as there is no support for dynamic debug. Andrew was working
on it recently [0], but I don't think the patch was accepted yet.

> +    }
> +}
> +
> +impl Drop for SampleDriver {
> +    fn drop(&mut self) {
> +        dev_dbg!(self.pdev.as_ref(), "Remove Rust I2C driver sample.\n");
> +    }
> +}
> +
> +// NOTE: The code below is expanded macro module_i2c_driver. It is not used here
> +//       because we need to manually create an I2C client in `init()`. The macro
> +//       hides `init()`, so to demo client creation on adapter SAMPLE_I2C_ADAPTER_INDEX
> +//       we expand it by hand.
> +type Ops<T> = kernel::i2c::Adapter<T>;

I am not sure I understand. How is a type alias related to module_i2c_driver at
all?

Do all drivers need to introduce the alias above?

> +
> +#[pin_data]
> +struct DriverModule {
> +    #[pin]
> +    _driver: kernel::driver::Registration<Ops<SampleDriver>>,
> +    _reg: i2c::Registration,
> +}

I was expecting this to be ARef of something, most likely I2cClient?

This is where my knowledge of i2c drivers start to fall short, but others will
probably chime in :)

> +
> +impl kernel::InPlaceModule for DriverModule {
> +    fn init(
> +        module: &'static kernel::ThisModule,
> +    ) -> impl ::pin_init::PinInit<Self, kernel::error::Error> {
> +        kernel::try_pin_init!(Self {
> +            _reg <- {
> +                let adapter = i2c::I2cAdapter::<Normal>::get(SAMPLE_I2C_ADAPTER_INDEX)?;
> +
> +                i2c::Registration::new(adapter, &BOARD_INFO)
> +            },
> +            _driver <- kernel::driver::Registration::new(
> +                 <Self as kernel::ModuleMetadata>::NAME, module
> +            ),
> +        })
> +    }
> +}
> +
> +kernel::prelude::module! {
> +    type: DriverModule,
> +    name: "rust_driver_i2c",
> +    authors: ["Igor Korotin"],
> +    description: "Rust I2C driver",
> +    license: "GPL v2",
> +}
> -- 
> 2.43.0
> 
> 

[0]: https://lore.kernel.org/rust-for-linux/20250611202952.1670168-1-andrewjballance@gmail.com/


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ