rust: update amba to use the new constant device id table

Also remove the non-const id table processing as `amba` was the only
user.

Signed-off-by: Wedson Almeida Filho <wedsonaf@google.com>

Author: wedsonaf

drivers/gpio/gpio_pl061_rust.rs

@@ -8,7 +8,7 @@
 #![feature(global_asm, allocator_api)]
 
 use kernel::{
-    amba, bit, bits_iter, declare_amba_id_table, device, gpio,
+    amba, bit, bits_iter, define_amba_id_table, device, gpio,
     io_mem::IoMem,
     irq::{self, ExtraResult, IrqData, LockedIrqData},
     power,
@@ -261,11 +261,11 @@ impl amba::Driver for PL061Device {
     type Data = Ref<DeviceData>;
     type PowerOps = Self;
 
-    declare_amba_id_table! [
-        { id: 0x00041061, mask: 0x000fffff, data: () },
-    ];
+    define_amba_id_table! {(), [
+        ({id: 0x00041061, mask: 0x000fffff}, None),
+    ]}
 
-    fn probe(dev: &mut amba::Device, _id: &amba::DeviceId) -> Result<Ref<DeviceData>> {
+    fn probe(dev: &mut amba::Device, _data: Option<&Self::IdInfo>) -> Result<Ref<DeviceData>> {
         let res = dev.take_resource().ok_or(Error::ENXIO)?;
         let irq = dev.irq(0).ok_or(Error::ENXIO)?;
 

rust/kernel/amba.rs

@@ -1,28 +1,43 @@
 // SPDX-License-Identifier: GPL-2.0
 
-//! Amba devices drivers.
+//! Amba devices and drivers.
 //!
 //! C header: [`include/linux/amba/bus.h`](../../../../include/linux/amba/bus.h)
 
 use crate::{
     bindings, c_types, device, driver, error::from_kernel_result, io_mem::Resource, power,
-    str::CStr, to_result, types::PointerWrapper, Error, Result, ThisModule,
+    str::CStr, to_result, types::PointerWrapper, Result, ThisModule,
 };
 
 /// A registration of an amba driver.
 pub type Registration<T> = driver::Registration<Adapter<T>>;
 
 /// Id of an Amba device.
-pub struct DeviceId<T = ()> {
+#[derive(Clone, Copy)]
+pub struct DeviceId {
     /// Device id.
     pub id: u32,
 
     /// Mask that identifies which bits are valid in the device id.
     pub mask: u32,
+}
+
+// SAFETY: `ZERO` is all zeroed-out and `to_rawid` stores `offset` in `amba_id::data`.
+unsafe impl const driver::RawDeviceId for DeviceId {
+    type RawType = bindings::amba_id;
+    const ZERO: Self::RawType = bindings::amba_id {
+        id: 0,
+        mask: 0,
+        data: core::ptr::null_mut(),
+    };
 
-    /// Context data to be associated with the device id. This is carried over to [`Driver::probe`]
-    /// so that drivers can encode any information they may need then.
-    pub data: T,
+    fn to_rawid(&self, offset: isize) -> Self::RawType {
+        bindings::amba_id {
+            id: self.id,
+            mask: self.mask,
+            data: offset as _,
+        }
+    }
 }
 
 /// An amba driver.
@@ -39,11 +54,11 @@ pub trait Driver {
     /// The type holding information about each device id supported by the driver.
     type IdInfo: 'static = ();
 
-    /// The table of device ids supported by the drivers.
-    const ID_TABLE: &'static [DeviceId<Self::IdInfo>];
+    /// The table of device ids supported by the driver.
+    const ID_TABLE: Option<driver::IdTable<'static, DeviceId, Self::IdInfo>> = None;
 
     /// Probes for the device with the given id.
-    fn probe(dev: &mut Device, id: &DeviceId<Self::IdInfo>) -> Result<Self::Data>;
+    fn probe(dev: &mut Device, id_info: Option<&Self::IdInfo>) -> Result<Self::Data>;
 
     /// Cleans any resources up that are associated with the device.
     ///
@@ -56,24 +71,22 @@ pub struct Adapter<T: Driver>(T);
 
 impl<T: Driver> driver::DriverOps for Adapter<T> {
     type RegType = bindings::amba_driver;
-    type RawIdType = bindings::amba_id;
-    type IdType = DeviceId<T::IdInfo>;
-    const ID_TABLE: &'static [Self::IdType] = T::ID_TABLE;
 
     unsafe fn register(
         reg: *mut bindings::amba_driver,
         name: &'static CStr,
         module: &'static ThisModule,
-        id_table: *const bindings::amba_id,
     ) -> Result {
         // SAFETY: By the safety requirements of this function (defined in the trait defintion),
         // `reg` is non-null and valid.
         let amba = unsafe { &mut *reg };
         amba.drv.name = name.as_char_ptr();
         amba.drv.owner = module.0;
-        amba.id_table = id_table;
         amba.probe = Some(probe_callback::<T>);
         amba.remove = Some(remove_callback::<T>);
+        if let Some(t) = T::ID_TABLE {
+            amba.id_table = t.as_ref();
+        }
         if cfg!(CONFIG_PM) {
             // SAFETY: `probe_callback` sets the driver data after calling `T::Data::into_pointer`,
             // and we guarantee that `T::Data` is the same as `T::PowerOps::Data` by a constraint
@@ -90,14 +103,6 @@ impl<T: Driver> driver::DriverOps for Adapter<T> {
         // `reg` was passed (and updated) by a previous successful call to `amba_driver_register`.
         unsafe { bindings::amba_driver_unregister(reg) };
     }
-
-    fn to_raw_id(index: usize, id: &Self::IdType) -> Self::RawIdType {
-        bindings::amba_id {
-            id: id.id,
-            mask: id.mask,
-            data: index as _,
-        }
-    }
 }
 
 unsafe extern "C" fn probe_callback<T: Driver>(
@@ -109,11 +114,18 @@ unsafe extern "C" fn probe_callback<T: Driver>(
         // duration of this call, so it is guaranteed to remain alive for the lifetime of `dev`.
         let mut dev = unsafe { Device::from_ptr(adev) };
         // SAFETY: `aid` is valid by the requirements the contract with the C code.
-        let index = unsafe { (*aid).data } as usize;
-        if index >= T::ID_TABLE.len() {
-            return Err(Error::ENXIO);
-        }
-        let data = T::probe(&mut dev, &T::ID_TABLE[index])?;
+        let offset = unsafe { (*aid).data };
+        let info = if offset.is_null() {
+            None
+        } else {
+            // SAFETY: The offset comes from a previous call to `offset_from` in `IdArray::new`,
+            // which guarantees that the resulting pointer is within the table.
+            let ptr = unsafe { aid.cast::<u8>().offset(offset as _).cast::<Option<T::IdInfo>>() };
+            // SAFETY: The id table has a static lifetime, so `ptr` is guaranteed to be valid for
+            // read.
+            unsafe { (&*ptr).as_ref() }
+        };
+        let data = T::probe(&mut dev, info)?;
         let ptr = T::Data::into_pointer(data);
         // SAFETY: `adev` is valid for write by the contract with the C code.
         unsafe { bindings::amba_set_drvdata(adev, ptr as _) };
@@ -193,17 +205,17 @@ unsafe impl device::RawDevice for Device {
 ///
 /// ```ignore
 /// # use kernel::prelude::*;
-/// # use kernel::{amba, declare_amba_id_table, module_amba_driver};
+/// # use kernel::{amba, define_amba_id_table, module_amba_driver};
 /// #
 /// struct MyDriver;
 /// impl amba::Driver for MyDriver {
 ///     // [...]
-/// #   fn probe(_dev: &mut amba::Device, _id: &amba::DeviceId<Self::IdInfo>) -> Result {
+/// #   fn probe(_dev: &mut amba::Device, _id: Option<&Self::IdInfo>) -> Result {
 /// #       Ok(())
 /// #   }
-/// #   declare_amba_id_table! [
-/// #       { id: 0x00041061, mask: 0x000fffff, data: () },
-/// #   ];
+/// #   define_amba_id_table! {(), [
+/// #       ({ id: 0x00041061, mask: 0x000fffff }, None),
+/// #   ]}
 /// }
 ///
 /// module_amba_driver! {
@@ -220,34 +232,28 @@ macro_rules! module_amba_driver {
     };
 }
 
-/// Declares the id table for amba devices.
+/// Defines the id table for amba devices.
 ///
 /// # Examples
 ///
 /// ```
 /// # use kernel::prelude::*;
-/// # use kernel::{amba, declare_amba_id_table};
+/// # use kernel::{amba, define_amba_id_table};
 /// #
 /// # struct Sample;
 /// # impl kernel::amba::Driver for Sample {
-/// #   fn probe(_dev: &mut amba::Device, _id: &amba::DeviceId<Self::IdInfo>) -> Result {
+/// #   fn probe(_dev: &mut amba::Device, _id: Option<&Self::IdInfo>) -> Result {
 /// #       Ok(())
 /// #   }
-///     declare_amba_id_table! [
-///         { id: 0x00041061, mask: 0x000fffff, data: () },
-///     ];
+///     define_amba_id_table! {(), [
+///         ({ id: 0x00041061, mask: 0x000fffff }, None),
+///     ]}
 /// # }
 /// ```
 #[macro_export]
-macro_rules! declare_amba_id_table {
-    ($({$($entry:tt)*},)*) => {
-        const ID_TABLE: &'static [$crate::amba::DeviceId<Self::IdInfo>] = &[
-            $( $crate::amba::DeviceId { $($entry)* },)*
-        ];
+macro_rules! define_amba_id_table {
+    ($data_type:ty, $($t:tt)*) => {
+        type IdInfo = $data_type;
+        $crate::define_id_table!(ID_TABLE, $crate::amba::DeviceId, $data_type, $($t)*);
     };
-
-    // Cover case without a trailing comma.
-    ($(($($entry:tt)*)),*) => {
-        $crate::declare_amba_id_table!{ $({$($entry)*},)*}
-    }
 }

rust/kernel/driver.rs

@@ -5,47 +5,29 @@
 //! Each bus/subsystem is expected to implement [`DriverOps`], which allows drivers to register
 //! using the [`Registration`] class.
 
-use crate::{str::CStr, sync::Ref, Error, KernelModule, Result, ScopeGuard, ThisModule};
-use alloc::{boxed::Box, vec::Vec};
-use core::{cell::UnsafeCell, marker::PhantomData, mem::MaybeUninit, ops::Deref, pin::Pin};
+use crate::{str::CStr, sync::Ref, Error, KernelModule, Result, ThisModule};
+use alloc::boxed::Box;
+use core::{cell::UnsafeCell, marker::PhantomData, ops::Deref, pin::Pin};
 
 /// A subsystem (e.g., PCI, Platform, Amba, etc.) that allows drivers to be written for it.
 pub trait DriverOps {
     /// The type that holds information about the registration. This is typically a struct defined
     /// by the C portion of the kernel.
     type RegType: Default;
 
-    /// The type that holds identification data for the devices supported by the driver. In
-    /// addition to the information required by the bus, it may also store device-specific data
-    /// using Rust types.
-    type IdType: 'static;
-
-    /// The table of ids containing all supported devices.
-    const ID_TABLE: &'static [Self::IdType];
-
-    /// The raw type that holds identification data for the devices supported by the driver. This
-    /// is typically a struct defined by the C portion of the kernel.
-    ///
-    /// A zero-terminated array of this type is produced and passed to the C portion during
-    /// registration.
-    type RawIdType;
-
     /// Registers a driver.
     ///
     /// # Safety
     ///
     /// `reg` must point to valid, initialised, and writable memory. It may be modified by this
     /// function to hold registration state.
     ///
-    /// `id_table` must point to a valid for read zero-terminated array of ids.
-    ///
-    /// On success, `reg` and `id_table` must remain pinned and valid until the matching call to
+    /// On success, `reg` must remain pinned and valid until the matching call to
     /// [`DriverOps::unregister`].
     unsafe fn register(
         reg: *mut Self::RegType,
         name: &'static CStr,
         module: &'static ThisModule,
-        id_table: *const Self::RawIdType,
     ) -> Result;
 
     /// Unregisters a driver previously registered with [`DriverOps::register`].
@@ -55,18 +37,12 @@ pub trait DriverOps {
     /// `reg` must point to valid writable memory, initialised by a previous successful call to
     /// [`DriverOps::register`].
     unsafe fn unregister(reg: *mut Self::RegType);
-
-    /// Converts an id into a raw id.
-    ///
-    /// This is used when building a zero-terminated array from the Rust array.
-    fn to_raw_id(index: usize, id: &Self::IdType) -> Self::RawIdType;
 }
 
 /// The registration of a driver.
 pub struct Registration<T: DriverOps> {
     is_registered: bool,
     concrete_reg: UnsafeCell<T::RegType>,
-    id_table: Vec<MaybeUninit<T::RawIdType>>,
 }
 
 // SAFETY: `Registration` has no fields or methods accessible via `&Registration`, so it is safe to
@@ -79,7 +55,6 @@ impl<T: DriverOps> Registration<T> {
         Self {
             is_registered: false,
             concrete_reg: UnsafeCell::new(T::RegType::default()),
-            id_table: Vec::new(),
         }
     }
 
@@ -108,42 +83,13 @@ impl<T: DriverOps> Registration<T> {
             return Err(Error::EINVAL);
         }
 
-        if this.id_table.is_empty() {
-            this.build_table()?;
-        }
-
-        // SAFETY: `concrete_reg` was initialised via its default constructor. `id_table` was just
-        // initialised above with a zero terminating entry. Both are only freed after `Self::drop`
-        // is called, which first calls `T::unregister`.
-        unsafe {
-            T::register(
-                this.concrete_reg.get(),
-                name,
-                module,
-                &this.id_table[0] as *const _ as *const _,
-            )
-        }?;
+        // SAFETY: `concrete_reg` was initialised via its default constructor. It is only freed
+        // after `Self::drop` is called, which first calls `T::unregister`.
+        unsafe { T::register(this.concrete_reg.get(), name, module) }?;
 
         this.is_registered = true;
         Ok(())
     }
-
-    /// Builds the zero-terminated raw-type array of supported devices.
-    ///
-    /// This is not ideal because the table is built at runtime. Once Rust fully supports const
-    /// generics, we can build the table at compile time.
-    fn build_table(&mut self) -> Result {
-        // Clear the table on failure, to indicate that the table isn't initialised.
-        let mut table = ScopeGuard::new_with_data(&mut self.id_table, |t| t.clear());
-
-        table.try_reserve_exact(T::ID_TABLE.len() + 1)?;
-        for (i, id) in T::ID_TABLE.iter().enumerate() {
-            table.try_push(MaybeUninit::new(T::to_raw_id(i, id)))?;
-        }
-        table.try_push(MaybeUninit::zeroed())?;
-        table.dismiss();
-        Ok(())
-    }
 }
 
 impl<T: DriverOps> Default for Registration<T> {
@@ -394,15 +340,14 @@ macro_rules! define_id_array {
     };
 }
 
-// TODO: Remove `ignore` tag from example once we go to 1.58.x.
 /// Defines a new constant [`IdTable`] with a concise syntax.
 ///
 /// It is meant to be used by buses and subsystems to create a similar macro with their device id
 /// type already specified, i.e., with fewer parameters to the end user.
 ///
 /// # Examples
 ///
-/// ```ignore
+/// ```
 /// #![feature(const_trait_impl)]
 /// # use kernel::{define_id_table, driver::RawDeviceId};
 ///

rust/kernel/lib.rs

@@ -18,7 +18,6 @@
     concat_idents,
     const_fn_trait_bound,
     const_mut_refs,
-    const_precise_live_drops, // TODO: Remove once we go to 1.58.x.
     const_ptr_offset_from,
     const_refs_to_cell,
     const_trait_impl,