Re: [PATCH v3 15/16] rust: platform: add basic platform device / driver abstractions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 29.10.2024 09:50, Danilo Krummrich wrote:
On Tue, Oct 29, 2024 at 08:20:55AM +0100, Dirk Behme wrote:
On 28.10.2024 11:19, Danilo Krummrich wrote:
On Thu, Oct 24, 2024 at 11:11:50AM +0200, Dirk Behme wrote:
+/// IdTable type for platform drivers.
+pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<of::DeviceId, T>;
+
+/// The platform driver trait.
+///
+/// # Example
+///
+///```
+/// # use kernel::{bindings, c_str, of, platform};
+///
+/// struct MyDriver;
+///
+/// kernel::of_device_table!(
+///     OF_TABLE,
+///     MODULE_OF_TABLE,

It looks to me that OF_TABLE and MODULE_OF_TABLE are quite generic names
used here. Shouldn't they be somehow driver specific, e.g. OF_TABLE_MYDRIVER
and MODULE_OF_TABLE_MYDRIVER or whatever? Same for the other
examples/samples in this patch series. Found that while using the *same*
somewhere else ;)

I think the names by themselves are fine. They're local to the module. However,
we stringify `OF_TABLE` in `module_device_table` to build the export name, i.e.
"__mod_of__OF_TABLE_device_table". Hence the potential duplicate symbols.

I think we somehow need to build the module name into the symbol name as well.

Something like this?

No, I think we should just encode the Rust module name / path, which should make
this a unique symbol name.

diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index 5b1329fba528..63e81ec2d6fd 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -154,7 +154,7 @@ macro_rules! module_device_table {
      ($table_type: literal, $module_table_name:ident, $table_name:ident) => {
          #[rustfmt::skip]
          #[export_name =
-            concat!("__mod_", $table_type, "__", stringify!($table_name), "_device_table")
+            concat!("__mod_", $table_type, "__", module_path!(), "_", stringify!($table_name), "_device_table")
          ]
          static $module_table_name: [core::mem::MaybeUninit<u8>; $table_name.raw_ids().size()] =
              unsafe { core::mem::transmute_copy($table_name.raw_ids()) };

For the doctests for instance this

   "__mod_of__OF_TABLE_device_table"

becomes

   "__mod_of__doctests_kernel_generated_OF_TABLE_device_table".


What implies *one* OF/PCI_TABLE per path (file)?

For example adding a second FooDriver example to platform.rs won't be possible?

+/// struct FooDriver;
+///
+/// kernel::of_device_table!(
+///     OF_TABLE,
+///     MODULE_OF_TABLE,
+///     <FooDriver as platform::Driver>::IdInfo,
+///     [
+///         (of::DeviceId::new(c_str!("test,rust-device2")), ())
+///     ]
+/// );

Best regards

Dirk








[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux