[RFC 2/5] rust: hw: core: Add rust bindings/funcs for i2c bus

[RFC 2/5] rust: hw: core: Add rust bindings/funcs for i2c bus

[chenmiao]

We have implemented the I2CBus and I2CSlave infrastructure in Rust by referring
to the SysBus device model.

Initially, we assumed that the I2CBus was at the same hierarchical level as the
PL011 device. Therefore, we followed the implementation paradigm of the PL011
device as a reference. However, in the end, we discovered that the I2CBus is
actually at the same level as the SysBus. As a result, we adopted the binding
implementation paradigm used for SysBus devices. With this adjustment, we
successfully compiled the code locally.

During the implementation process, we found that the current two paradigms in
Rust — bindings and impl — are extremely complex and lack comprehensive
documentation. There is no clear explanation as to why Bus and Device models
need to be implemented using different approaches. Furthermore, the
implementation of Bus and Device following these paradigms still has many
limitations. At present, at least vmstate is not easily supported.

Signed-off-by: Chao Liu <chao.liu@openatom.club>
Signed-off-by: chenmiao <chenmiao@openatom.club>
---
 rust/hw/core/meson.build   |   1 +
 rust/hw/core/src/i2cbus.rs | 291 +++++++++++++++++++++++++++++++++++++
 rust/hw/core/src/lib.rs    |   3 +
 rust/hw/core/wrapper.h     |   1 +
 4 files changed, 296 insertions(+)
 create mode 100644 rust/hw/core/src/i2cbus.rs

diff --git a/rust/hw/core/meson.build b/rust/hw/core/meson.build
index efcda50..7c44786 100644
--- a/rust/hw/core/meson.build
+++ b/rust/hw/core/meson.build
@@ -52,6 +52,7 @@ _hwcore_rs = static_library(
       'src/bindings.rs',
       'src/bus.rs',
       'src/irq.rs',
+      'src/i2cbus.rs',
       'src/qdev.rs',
       'src/sysbus.rs',
     ],
diff --git a/rust/hw/core/src/i2cbus.rs b/rust/hw/core/src/i2cbus.rs
new file mode 100644
index 0000000..c026955
--- /dev/null
+++ b/rust/hw/core/src/i2cbus.rs
@@ -0,0 +1,291 @@
+// Copyright 2025 HUST OpenAtom Open Source Club.
+// Author(s): Chao Liu <chao.liu@openatom.club>
+// Author(s): Chen Miao <chenmiao@openatom.club>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+//! Bindings to access `i2c` functionality from Rust.
+
+use std::ffi::CStr;
+
+pub use bindings::I2CSlaveClass;
+use common::Opaque;
+use qom::{prelude::*, Owned};
+use util::Result;
+
+use crate::{
+    bindings,
+    bus::{BusClass, BusState},
+    qdev::{DeviceImpl, DeviceState},
+};
+
+/// A safe wrapper around [`bindings::I2CBus`].
+#[repr(transparent)]
+#[derive(Debug, common::Wrapper)]
+pub struct I2CBus(Opaque<bindings::I2CBus>);
+
+unsafe impl Send for I2CBus {}
+unsafe impl Sync for I2CBus {}
+
+unsafe impl ObjectType for I2CBus {
+    type Class = BusClass;
+    const TYPE_NAME: &'static CStr =
+        unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_I2C_BUS) };
+}
+
+qom_isa!(I2CBus: BusState, Object);
+
+// TODO: add virtual methods
+pub trait I2CBusImpl: DeviceImpl + IsA<I2CBus> {}
+
+/// Trait for methods of [`I2CBus`] and its subclasses.
+pub trait I2CBusMethods: ObjectDeref
+where
+    Self::Target: IsA<I2CBus>,
+{
+    /// Initialize an I2C bus
+    fn init_bus(&self, parent: &DeviceState, name: &str) -> *mut bindings::I2CBus {
+        assert!(bql::is_locked());
+        unsafe { bindings::i2c_init_bus(parent.as_mut_ptr(), name.as_ptr().cast()) }
+    }
+
+    /// Start a transfer on an I2C bus
+    fn start_transfer(&self, address: u8, is_recv: bool) -> i32 {
+        assert!(bql::is_locked());
+        unsafe { bindings::i2c_start_transfer(self.upcast().as_mut_ptr(), address, is_recv) }
+    }
+
+    /// Start a receive transfer on an I2C bus
+    fn start_recv(&self, address: u8) -> i32 {
+        assert!(bql::is_locked());
+        unsafe { bindings::i2c_start_recv(self.upcast().as_mut_ptr(), address) }
+    }
+
+    /// Start a send transfer on an I2C bus
+    fn start_send(&self, address: u8) -> i32 {
+        assert!(bql::is_locked());
+        unsafe { bindings::i2c_start_send(self.upcast().as_mut_ptr(), address) }
+    }
+
+    /// Start an asynchronous send transfer on an I2C bus
+    fn start_send_async(&self, address: u8) -> i32 {
+        assert!(bql::is_locked());
+        unsafe { bindings::i2c_start_send_async(self.upcast().as_mut_ptr(), address) }
+    }
+
+    /// End a transfer on an I2C bus
+    fn end_transfer(&self) {
+        assert!(bql::is_locked());
+        unsafe { bindings::i2c_end_transfer(self.upcast().as_mut_ptr()) }
+    }
+
+    /// Send NACK on an I2C bus
+    fn nack(&self) {
+        assert!(bql::is_locked());
+        unsafe { bindings::i2c_nack(self.upcast().as_mut_ptr()) }
+    }
+
+    /// Send ACK on an I2C bus
+    fn ack(&self) {
+        assert!(bql::is_locked());
+        unsafe { bindings::i2c_ack(self.upcast().as_mut_ptr()) }
+    }
+
+    /// Send data on an I2C bus
+    fn send(&self, data: u8) -> i32 {
+        assert!(bql::is_locked());
+        unsafe { bindings::i2c_send(self.upcast().as_mut_ptr(), data) }
+    }
+
+    /// Send data asynchronously on an I2C bus
+    fn send_async(&self, data: u8) -> i32 {
+        assert!(bql::is_locked());
+        unsafe { bindings::i2c_send_async(self.upcast().as_mut_ptr(), data) }
+    }
+
+    /// Receive data from an I2C bus
+    fn recv(&self) -> u8 {
+        assert!(bql::is_locked());
+        unsafe { bindings::i2c_recv(self.upcast().as_mut_ptr()) }
+    }
+
+    /// Check if the I2C bus is busy.
+    ///
+    /// Returns `true` if the bus is busy, `false` otherwise.
+    fn is_busy(&self) -> bool {
+        assert!(bql::is_locked());
+        unsafe { bindings::i2c_bus_busy(self.upcast().as_mut_ptr()) != 0 }
+    }
+
+    /// Schedule pending master on an I2C bus
+    fn schedule_pending_master(&self) {
+        assert!(bql::is_locked());
+        unsafe { bindings::i2c_schedule_pending_master(self.upcast().as_mut_ptr()) }
+    }
+
+    /// Sets the I2C bus master.
+    ///
+    /// # Safety
+    ///
+    /// This function is unsafe because:
+    /// - `bh` must be a valid pointer to a `QEMUBH`.
+    /// - The caller must ensure that `self` is in a valid state.
+    /// - The caller must guarantee no data races occur during execution.
+    unsafe fn set_master(&self, bh: *mut bindings::QEMUBH) {
+        assert!(bql::is_locked());
+        unsafe { bindings::i2c_bus_master(self.upcast().as_mut_ptr(), bh) }
+    }
+
+    /// Release an I2C bus
+    fn release(&self) {
+        assert!(bql::is_locked());
+        unsafe { bindings::i2c_bus_release(self.upcast().as_mut_ptr()) }
+    }
+}
+
+impl<R: ObjectDeref> I2CBusMethods for R where R::Target: IsA<I2CBus> {}
+
+/// A safe wrapper around [`bindings::I2CSlave`].
+#[repr(transparent)]
+#[derive(Debug, common::Wrapper)]
+pub struct I2CSlave(Opaque<bindings::I2CSlave>);
+
+unsafe impl Send for I2CSlave {}
+unsafe impl Sync for I2CSlave {}
+
+unsafe impl ObjectType for I2CSlave {
+    type Class = I2CSlaveClass;
+    const TYPE_NAME: &'static CStr =
+        unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_I2C_SLAVE) };
+}
+
+qom_isa!(I2CSlave: DeviceState, Object);
+
+// TODO: add virtual methods
+pub trait I2CSlaveImpl: DeviceImpl + IsA<I2CSlave> {
+    /// Master to slave. Returns non-zero for a NAK, 0 for success.
+    const SEND: Option<fn(&Self, data: u8) -> Result<bool>> = None;
+
+    /// Master to slave (asynchronous). Receiving slave must call `i2c_ack()`.
+    const SEND_ASYNC: Option<fn(&Self, data: u8) -> Result<()>> = None;
+
+    /// Slave to master. This cannot fail, the device should always return something here.
+    const RECV: Option<fn(&Self) -> Result<bool>> = None;
+
+    /// Notify the slave of a bus state change. For start event,
+    /// returns non-zero to NAK an operation. For other events the
+    /// return code is not used and should be zero.
+    const EVENT: Option<fn(&Self, event: I2CEvent) -> Result<I2CEvent>> = None;
+
+    /// Check if this device matches the address provided. Returns bool of
+    /// true if it matches (or broadcast), and updates the device list, false
+    /// otherwise.
+    ///
+    /// If broadcast is true, match should add the device and return true.
+    type AddressMatcher;
+    const MATCH_AND_ADD: Option<Self::AddressMatcher> = None;
+}
+
+impl I2CSlaveClass {
+    /// Fill in the virtual methods of `I2CSlaveClass` based on the
+    /// definitions in the `I2CSlaveImpl` trait.
+    pub fn class_init<T: I2CSlaveImpl>(self: &mut I2CSlaveClass) {
+        self.parent_class.class_init::<T>();
+    }
+}
+
+/// Trait for methods of [`I2CSlave`] and its subclasses.
+pub trait I2CSlaveMethods: ObjectDeref
+where
+    Self::Target: IsA<I2CSlave>,
+{
+    /// Create an I2C slave device on the heap.
+    ///
+    /// # Arguments
+    /// * `name` - a device type name
+    /// * `addr` - I2C address of the slave when put on a bus
+    ///
+    /// This only initializes the device state structure and allows
+    /// properties to be set. Type `name` must exist. The device still
+    /// needs to be realized.
+    fn init_new(name: &str, addr: u8) -> Owned<I2CSlave> {
+        assert!(bql::is_locked());
+        unsafe {
+            let slave = bindings::i2c_slave_new(name.as_ptr().cast(), addr);
+            Owned::from(I2CSlave::from_raw(slave))
+        }
+    }
+
+    /// Create and realize an I2C slave device on the heap.
+    ///
+    /// # Arguments
+    /// * `bus` - I2C bus to put it on
+    /// * `name` - I2C slave device type name
+    /// * `addr` - I2C address of the slave when put on a bus
+    ///
+    /// Create the device state structure, initialize it, put it on the
+    /// specified `bus`, and drop the reference to it (the device is realized).
+    fn create_simple(&self, bus: &I2CBus, name: &str, addr: u8) -> Owned<I2CSlave> {
+        assert!(bql::is_locked());
+        unsafe {
+            let slave =
+                bindings::i2c_slave_create_simple(bus.as_mut_ptr(), name.as_ptr().cast(), addr);
+            Owned::from(I2CSlave::from_raw(slave))
+        }
+    }
+
+    /// Set the I2C bus address of a slave device
+    ///
+    /// # Arguments
+    /// * `address` - I2C address of the slave when put on a bus
+    fn set_address(&self, address: u8) {
+        assert!(bql::is_locked());
+        unsafe { bindings::i2c_slave_set_address(self.upcast().as_mut_ptr(), address) }
+    }
+
+    /// Get the I2C bus address of a slave device
+    fn get_address(&self) -> u8 {
+        assert!(bql::is_locked());
+        // SAFETY: the BQL ensures that no one else writes to the I2CSlave structure,
+        // and the I2CSlave must be initialized to get an IsA<I2CSlave>.
+        let slave = unsafe { *self.upcast().as_ptr() };
+        slave.address
+    }
+}
+
+impl<R: ObjectDeref> I2CSlaveMethods for R where R::Target: IsA<I2CSlave> {}
+
+/// Enum representing I2C events
+#[repr(u32)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum I2CEvent {
+    StartRecv = 0,
+    StartSend = 1,
+    StartSendAsync = 2,
+    Finish = 3,
+    Nack = 4,
+}
+
+impl From<bindings::i2c_event> for I2CEvent {
+    fn from(event: bindings::i2c_event) -> Self {
+        match event {
+            bindings::I2C_START_RECV => I2CEvent::StartRecv,
+            bindings::I2C_START_SEND => I2CEvent::StartSend,
+            bindings::I2C_START_SEND_ASYNC => I2CEvent::StartSendAsync,
+            bindings::I2C_FINISH => I2CEvent::Finish,
+            bindings::I2C_NACK => I2CEvent::Nack,
+            _ => panic!("Unknown I2C event: {event}"),
+        }
+    }
+}
+
+impl From<I2CEvent> for bindings::i2c_event {
+    fn from(event: I2CEvent) -> Self {
+        match event {
+            I2CEvent::StartRecv => bindings::I2C_START_RECV,
+            I2CEvent::StartSend => bindings::I2C_START_SEND,
+            I2CEvent::StartSendAsync => bindings::I2C_START_SEND_ASYNC,
+            I2CEvent::Finish => bindings::I2C_FINISH,
+            I2CEvent::Nack => bindings::I2C_NACK,
+        }
+    }
+}
diff --git a/rust/hw/core/src/lib.rs b/rust/hw/core/src/lib.rs
index 10cc516..fb0ee82 100644
--- a/rust/hw/core/src/lib.rs
+++ b/rust/hw/core/src/lib.rs
@@ -16,3 +16,6 @@
 
 mod bus;
 pub use bus::*;
+
+mod i2cbus;
+pub use i2cbus::*;
diff --git a/rust/hw/core/wrapper.h b/rust/hw/core/wrapper.h
index 3bdbd12..399be59 100644
--- a/rust/hw/core/wrapper.h
+++ b/rust/hw/core/wrapper.h
@@ -30,3 +30,4 @@ typedef enum memory_order {
 #include "hw/qdev-properties.h"
 #include "hw/qdev-properties-system.h"
 #include "hw/irq.h"
+#include "hw/i2c/i2c.h"
-- 
2.43.0

Re: [RFC 2/5] rust: hw: core: Add rust bindings/funcs for i2c bus

[Zhao Liu]

On Sat, Oct 25, 2025 at 12:38:50PM +0000, chenmiao wrote:
> Date: Sat, 25 Oct 2025 12:38:50 +0000
> From: chenmiao <chenmiao@openatom.club>
> Subject: [RFC 2/5] rust: hw: core: Add rust bindings/funcs for i2c bus
> X-Mailer: git-send-email 2.43.0
> 
> We have implemented the I2CBus and I2CSlave infrastructure in Rust by referring
> to the SysBus device model.
> 
> Initially, we assumed that the I2CBus was at the same hierarchical level as the
> PL011 device. Therefore, we followed the implementation paradigm of the PL011
> device as a reference. However, in the end, we discovered that the I2CBus is
> actually at the same level as the SysBus. As a result, we adopted the binding
> implementation paradigm used for SysBus devices. With this adjustment, we
> successfully compiled the code locally.
> 
> During the implementation process, we found that the current two paradigms in
> Rust — bindings and impl — are extremely complex and lack comprehensive
> documentation. There is no clear explanation as to why Bus and Device models
> need to be implemented using different approaches. Furthermore, the
> implementation of Bus and Device following these paradigms still has many
> limitations. At present, at least vmstate is not easily supported.
> 
> Signed-off-by: Chao Liu <chao.liu@openatom.club>
> Signed-off-by: chenmiao <chenmiao@openatom.club>
> ---
>  rust/hw/core/meson.build   |   1 +
>  rust/hw/core/src/i2cbus.rs | 291 +++++++++++++++++++++++++++++++++++++
>  rust/hw/core/src/lib.rs    |   3 +
>  rust/hw/core/wrapper.h     |   1 +
>  4 files changed, 296 insertions(+)
>  create mode 100644 rust/hw/core/src/i2cbus.rs
> 
> diff --git a/rust/hw/core/meson.build b/rust/hw/core/meson.build
> index efcda50..7c44786 100644
> --- a/rust/hw/core/meson.build
> +++ b/rust/hw/core/meson.build
> @@ -52,6 +52,7 @@ _hwcore_rs = static_library(
>        'src/bindings.rs',
>        'src/bus.rs',
>        'src/irq.rs',
> +      'src/i2cbus.rs',
>        'src/qdev.rs',
>        'src/sysbus.rs',
>      ],
> diff --git a/rust/hw/core/src/i2cbus.rs b/rust/hw/core/src/i2cbus.rs
> new file mode 100644
> index 0000000..c026955
> --- /dev/null
> +++ b/rust/hw/core/src/i2cbus.rs

This file implements both I2CBus and I2C device (Slave).

To be general, I think it's better to name it as "i2c.rs".

> @@ -0,0 +1,291 @@
> +// Copyright 2025 HUST OpenAtom Open Source Club.
> +// Author(s): Chao Liu <chao.liu@openatom.club>
> +// Author(s): Chen Miao <chenmiao@openatom.club>
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +//! Bindings to access `i2c` functionality from Rust.
> +
> +use std::ffi::CStr;
> +
> +pub use bindings::I2CSlaveClass;
> +use common::Opaque;
> +use qom::{prelude::*, Owned};
> +use util::Result;
> +
> +use crate::{
> +    bindings,
> +    bus::{BusClass, BusState},
> +    qdev::{DeviceImpl, DeviceState},
> +};
> +
> +/// A safe wrapper around [`bindings::I2CBus`].
> +#[repr(transparent)]
> +#[derive(Debug, common::Wrapper)]
> +pub struct I2CBus(Opaque<bindings::I2CBus>);
> +
> +unsafe impl Send for I2CBus {}
> +unsafe impl Sync for I2CBus {}
> +
> +unsafe impl ObjectType for I2CBus {
> +    type Class = BusClass;
> +    const TYPE_NAME: &'static CStr =
> +        unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_I2C_BUS) };
> +}
> +
> +qom_isa!(I2CBus: BusState, Object);
> +
> +// TODO: add virtual methods
> +pub trait I2CBusImpl: DeviceImpl + IsA<I2CBus> {}
> +
> +/// Trait for methods of [`I2CBus`] and its subclasses.
> +pub trait I2CBusMethods: ObjectDeref
> +where
> +    Self::Target: IsA<I2CBus>,
> +{
> +    /// Initialize an I2C bus
> +    fn init_bus(&self, parent: &DeviceState, name: &str) -> *mut bindings::I2CBus {
> +        assert!(bql::is_locked());

It's better to add # SFAETY comment (others methods below also need it).

> +        unsafe { bindings::i2c_init_bus(parent.as_mut_ptr(), name.as_ptr().cast()) }
> +    }

...

> +    /// Sets the I2C bus master.
> +    ///
> +    /// # Safety
> +    ///
> +    /// This function is unsafe because:
> +    /// - `bh` must be a valid pointer to a `QEMUBH`.
> +    /// - The caller must ensure that `self` is in a valid state.
> +    /// - The caller must guarantee no data races occur during execution.
> +    unsafe fn set_master(&self, bh: *mut bindings::QEMUBH) {

I think it's better to have a Opaque<> wrapper over bindings::QEMUBH.
However, async has not yet been fully considered.

IMO, It seems i2c_bus_master() is only used by i2c-echo, which is not
implememnted in this series. Perhaps this series could initially omit
QEMUBH (with a TODO comment) until QEMUBH binding is introduced.

> +        assert!(bql::is_locked());
> +        unsafe { bindings::i2c_bus_master(self.upcast().as_mut_ptr(), bh) }
> +    }
> +
> +    /// Release an I2C bus
> +    fn release(&self) {
> +        assert!(bql::is_locked());
> +        unsafe { bindings::i2c_bus_release(self.upcast().as_mut_ptr()) }
> +    }
> +}
> +
> +impl<R: ObjectDeref> I2CBusMethods for R where R::Target: IsA<I2CBus> {}
> +
> +/// A safe wrapper around [`bindings::I2CSlave`].
> +#[repr(transparent)]
> +#[derive(Debug, common::Wrapper)]
> +pub struct I2CSlave(Opaque<bindings::I2CSlave>);
> +
> +unsafe impl Send for I2CSlave {}
> +unsafe impl Sync for I2CSlave {}
> +
> +unsafe impl ObjectType for I2CSlave {
> +    type Class = I2CSlaveClass;
> +    const TYPE_NAME: &'static CStr =
> +        unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_I2C_SLAVE) };
> +}
> +
> +qom_isa!(I2CSlave: DeviceState, Object);
> +
> +// TODO: add virtual methods
> +pub trait I2CSlaveImpl: DeviceImpl + IsA<I2CSlave> {
> +    /// Master to slave. Returns non-zero for a NAK, 0 for success.
> +    const SEND: Option<fn(&Self, data: u8) -> Result<bool>> = None;
> +
> +    /// Master to slave (asynchronous). Receiving slave must call `i2c_ack()`.
> +    const SEND_ASYNC: Option<fn(&Self, data: u8) -> Result<()>> = None;
> +
> +    /// Slave to master. This cannot fail, the device should always return something here.
> +    const RECV: Option<fn(&Self) -> Result<bool>> = None;
> +
> +    /// Notify the slave of a bus state change. For start event,
> +    /// returns non-zero to NAK an operation. For other events the
> +    /// return code is not used and should be zero.
> +    const EVENT: Option<fn(&Self, event: I2CEvent) -> Result<I2CEvent>> = None;
> +
> +    /// Check if this device matches the address provided. Returns bool of
> +    /// true if it matches (or broadcast), and updates the device list, false
> +    /// otherwise.
> +    ///
> +    /// If broadcast is true, match should add the device and return true.
> +    type AddressMatcher;
> +    const MATCH_AND_ADD: Option<Self::AddressMatcher> = None;
> +}
> +
> +impl I2CSlaveClass {
> +    /// Fill in the virtual methods of `I2CSlaveClass` based on the
> +    /// definitions in the `I2CSlaveImpl` trait.
> +    pub fn class_init<T: I2CSlaveImpl>(self: &mut I2CSlaveClass) {

It seems the callbacks (SEND/SEND_ASYNC/...) are missing to set here.

For example, add a callback wrapper first:

unsafe extern "C" fn rust_i2c_slave_send_fn<T: I2CSlaveImpl>(
    slave: *mut bindings::I2CSlave,
    date: u8,
) {
    let state = NonNull::new(slave).unwrap().cast::<T>();
    T::SEND.unwrap()(unsafe { state.as_ref() }, data);
}

Then create C callback like:

pub fn class_init<T: I2CSlaveImpl>(self: &mut I2CSlaveClass) {
    ...
    if <T as I2CSlaveImpl>::SEND.is_some() {
        self.send = Some(rust_i2c_slave_send_fn::<T>);
    }
    ...
}

Similarly, you can refer to ResettableClass::class_init and
DeviceClass::class_init.

> +        self.parent_class.class_init::<T>();
> +    }
> +}
> +

Regards,
Zhao

Re: [RFC 2/5] rust: hw: core: Add rust bindings/funcs for i2c bus

[Chen Miao]

On 10/27/2025 3:17 PM, Zhao Liu wrote:
> On Sat, Oct 25, 2025 at 12:38:50PM +0000, chenmiao wrote:
>> Date: Sat, 25 Oct 2025 12:38:50 +0000
>> From: chenmiao <chenmiao@openatom.club>
>> Subject: [RFC 2/5] rust: hw: core: Add rust bindings/funcs for i2c bus
>> X-Mailer: git-send-email 2.43.0
>>
>> We have implemented the I2CBus and I2CSlave infrastructure in Rust by referring
>> to the SysBus device model.
>>
>> Initially, we assumed that the I2CBus was at the same hierarchical level as the
>> PL011 device. Therefore, we followed the implementation paradigm of the PL011
>> device as a reference. However, in the end, we discovered that the I2CBus is
>> actually at the same level as the SysBus. As a result, we adopted the binding
>> implementation paradigm used for SysBus devices. With this adjustment, we
>> successfully compiled the code locally.
>>
>> During the implementation process, we found that the current two paradigms in
>> Rust — bindings and impl — are extremely complex and lack comprehensive
>> documentation. There is no clear explanation as to why Bus and Device models
>> need to be implemented using different approaches. Furthermore, the
>> implementation of Bus and Device following these paradigms still has many
>> limitations. At present, at least vmstate is not easily supported.
>>
>> Signed-off-by: Chao Liu <chao.liu@openatom.club>
>> Signed-off-by: chenmiao <chenmiao@openatom.club>
>> ---
>>   rust/hw/core/meson.build   |   1 +
>>   rust/hw/core/src/i2cbus.rs | 291 +++++++++++++++++++++++++++++++++++++
>>   rust/hw/core/src/lib.rs    |   3 +
>>   rust/hw/core/wrapper.h     |   1 +
>>   4 files changed, 296 insertions(+)
>>   create mode 100644 rust/hw/core/src/i2cbus.rs
>>
>> diff --git a/rust/hw/core/meson.build b/rust/hw/core/meson.build
>> index efcda50..7c44786 100644
>> --- a/rust/hw/core/meson.build
>> +++ b/rust/hw/core/meson.build
>> @@ -52,6 +52,7 @@ _hwcore_rs = static_library(
>>         'src/bindings.rs',
>>         'src/bus.rs',
>>         'src/irq.rs',
>> +      'src/i2cbus.rs',
>>         'src/qdev.rs',
>>         'src/sysbus.rs',
>>       ],
>> diff --git a/rust/hw/core/src/i2cbus.rs b/rust/hw/core/src/i2cbus.rs
>> new file mode 100644
>> index 0000000..c026955
>> --- /dev/null
>> +++ b/rust/hw/core/src/i2cbus.rs
> This file implements both I2CBus and I2C device (Slave).
>
> To be general, I think it's better to name it as "i2c.rs".
>
>> @@ -0,0 +1,291 @@
>> +// Copyright 2025 HUST OpenAtom Open Source Club.
>> +// Author(s): Chao Liu <chao.liu@openatom.club>
>> +// Author(s): Chen Miao <chenmiao@openatom.club>
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +
>> +//! Bindings to access `i2c` functionality from Rust.
>> +
>> +use std::ffi::CStr;
>> +
>> +pub use bindings::I2CSlaveClass;
>> +use common::Opaque;
>> +use qom::{prelude::*, Owned};
>> +use util::Result;
>> +
>> +use crate::{
>> +    bindings,
>> +    bus::{BusClass, BusState},
>> +    qdev::{DeviceImpl, DeviceState},
>> +};
>> +
>> +/// A safe wrapper around [`bindings::I2CBus`].
>> +#[repr(transparent)]
>> +#[derive(Debug, common::Wrapper)]
>> +pub struct I2CBus(Opaque<bindings::I2CBus>);
>> +
>> +unsafe impl Send for I2CBus {}
>> +unsafe impl Sync for I2CBus {}
>> +
>> +unsafe impl ObjectType for I2CBus {
>> +    type Class = BusClass;
>> +    const TYPE_NAME: &'static CStr =
>> +        unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_I2C_BUS) };
>> +}
>> +
>> +qom_isa!(I2CBus: BusState, Object);
>> +
>> +// TODO: add virtual methods
>> +pub trait I2CBusImpl: DeviceImpl + IsA<I2CBus> {}
>> +
>> +/// Trait for methods of [`I2CBus`] and its subclasses.
>> +pub trait I2CBusMethods: ObjectDeref
>> +where
>> +    Self::Target: IsA<I2CBus>,
>> +{
>> +    /// Initialize an I2C bus
>> +    fn init_bus(&self, parent: &DeviceState, name: &str) -> *mut bindings::I2CBus {
>> +        assert!(bql::is_locked());
> It's better to add # SFAETY comment (others methods below also need it).
>
>> +        unsafe { bindings::i2c_init_bus(parent.as_mut_ptr(), name.as_ptr().cast()) }
>> +    }
> ...
>
>> +    /// Sets the I2C bus master.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// This function is unsafe because:
>> +    /// - `bh` must be a valid pointer to a `QEMUBH`.
>> +    /// - The caller must ensure that `self` is in a valid state.
>> +    /// - The caller must guarantee no data races occur during execution.
>> +    unsafe fn set_master(&self, bh: *mut bindings::QEMUBH) {
> I think it's better to have a Opaque<> wrapper over bindings::QEMUBH.
> However, async has not yet been fully considered.
>
> IMO, It seems i2c_bus_master() is only used by i2c-echo, which is not
> implememnted in this series. Perhaps this series could initially omit
> QEMUBH (with a TODO comment) until QEMUBH binding is introduced.
I think our team is also interested in the implementation of QEMUBH, and we 
will try to implement this part as well. Regarding the omission of QEMUBH, I 
believe we can add a comment here for now!
>> +        assert!(bql::is_locked());
>> +        unsafe { bindings::i2c_bus_master(self.upcast().as_mut_ptr(), bh) }
>> +    }
>> +
>> +    /// Release an I2C bus
>> +    fn release(&self) {
>> +        assert!(bql::is_locked());
>> +        unsafe { bindings::i2c_bus_release(self.upcast().as_mut_ptr()) }
>> +    }
>> +}
>> +
>> +impl<R: ObjectDeref> I2CBusMethods for R where R::Target: IsA<I2CBus> {}
>> +
>> +/// A safe wrapper around [`bindings::I2CSlave`].
>> +#[repr(transparent)]
>> +#[derive(Debug, common::Wrapper)]
>> +pub struct I2CSlave(Opaque<bindings::I2CSlave>);
>> +
>> +unsafe impl Send for I2CSlave {}
>> +unsafe impl Sync for I2CSlave {}
>> +
>> +unsafe impl ObjectType for I2CSlave {
>> +    type Class = I2CSlaveClass;
>> +    const TYPE_NAME: &'static CStr =
>> +        unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_I2C_SLAVE) };
>> +}
>> +
>> +qom_isa!(I2CSlave: DeviceState, Object);
>> +
>> +// TODO: add virtual methods
>> +pub trait I2CSlaveImpl: DeviceImpl + IsA<I2CSlave> {
>> +    /// Master to slave. Returns non-zero for a NAK, 0 for success.
>> +    const SEND: Option<fn(&Self, data: u8) -> Result<bool>> = None;
>> +
>> +    /// Master to slave (asynchronous). Receiving slave must call `i2c_ack()`.
>> +    const SEND_ASYNC: Option<fn(&Self, data: u8) -> Result<()>> = None;
>> +
>> +    /// Slave to master. This cannot fail, the device should always return something here.
>> +    const RECV: Option<fn(&Self) -> Result<bool>> = None;
>> +
>> +    /// Notify the slave of a bus state change. For start event,
>> +    /// returns non-zero to NAK an operation. For other events the
>> +    /// return code is not used and should be zero.
>> +    const EVENT: Option<fn(&Self, event: I2CEvent) -> Result<I2CEvent>> = None;
>> +
>> +    /// Check if this device matches the address provided. Returns bool of
>> +    /// true if it matches (or broadcast), and updates the device list, false
>> +    /// otherwise.
>> +    ///
>> +    /// If broadcast is true, match should add the device and return true.
>> +    type AddressMatcher;
>> +    const MATCH_AND_ADD: Option<Self::AddressMatcher> = None;
>> +}
>> +
>> +impl I2CSlaveClass {
>> +    /// Fill in the virtual methods of `I2CSlaveClass` based on the
>> +    /// definitions in the `I2CSlaveImpl` trait.
>> +    pub fn class_init<T: I2CSlaveImpl>(self: &mut I2CSlaveClass) {
> It seems the callbacks (SEND/SEND_ASYNC/...) are missing to set here.
>
> For example, add a callback wrapper first:
>
> unsafe extern "C" fn rust_i2c_slave_send_fn<T: I2CSlaveImpl>(
>      slave: *mut bindings::I2CSlave,
>      date: u8,
> ) {
>      let state = NonNull::new(slave).unwrap().cast::<T>();
>      T::SEND.unwrap()(unsafe { state.as_ref() }, data);
> }
>
> Then create C callback like:
>
> pub fn class_init<T: I2CSlaveImpl>(self: &mut I2CSlaveClass) {
>      ...
>      if <T as I2CSlaveImpl>::SEND.is_some() {
>          self.send = Some(rust_i2c_slave_send_fn::<T>);
>      }
>      ...
> }
>
> Similarly, you can refer to ResettableClass::class_init and
> DeviceClass::class_init.

Ok, I'll complete it strictly according to your instructions.

>
>> +        self.parent_class.class_init::<T>();
>> +    }
>> +}
>> +
> Regards,
> Zhao

Waiting for your next review for other patch.

Thanks,

Chen Miao

>