From: Fabien Parent <fabien.parent@linaro.org>

Ports Platform device and driver abstractions from Fabien's tree [0].

These abstractions do not depend on any generic driver registration and
id table. Instead, the minimal abstractions have been implemented
specifically for platform subsystem taking heavy inspiration from the
existing phy device and driver abstractions.

[0]: https://github.com/Fabo/linux/commits/fparent/rust-platform

Signed-off-by: Fabien Parent <fabien.parent@linaro.org>
Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/lib.rs              |   1 +
 rust/kernel/platform.rs         | 380 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 382 insertions(+)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index ae82e9c941af..10cbcdd74089 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -16,6 +16,7 @@
 #include <linux/jiffies.h>
 #include <linux/mdio.h>
 #include <linux/phy.h>
+#include <linux/platform_device.h>
 #include <linux/refcount.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index b5f4b3ce6b48..b3a318fde46c 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -42,6 +42,7 @@
 #[cfg(CONFIG_NET)]
 pub mod net;
 pub mod page;
+pub mod platform;
 pub mod prelude;
 pub mod print;
 pub mod rbtree;
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
new file mode 100644
index 000000000000..de28429f5551
--- /dev/null
+++ b/rust/kernel/platform.rs
@@ -0,0 +1,380 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Platform devices and drivers.
+//!
+//! Also called `platformdev`, `pdev`.
+//!
+//! C header: [`include/linux/platform_device.h`](../../../../include/linux/platform_device.h)
+
+use core::{marker::PhantomData, pin::Pin, ptr::addr_of_mut};
+
+use macros::vtable;
+
+use crate::{
+    bindings, device,
+    error::{from_result, Result},
+    str::CStr,
+    types::Opaque,
+};
+
+/// A platform device.
+///
+/// # Invariants
+///
+/// The field `ptr` is non-null and valid for the lifetime of the object.
+#[repr(transparent)]
+pub struct Device(Opaque<bindings::platform_device>);
+
+impl Device {
+    /// Creates a new [`Device`] instance from a raw pointer.
+    ///
+    /// # Safety
+    ///
+    /// For the duration of `'a`,
+    /// - the pointer must point at a valid `platform_device`, and the caller
+    ///   must be in a context where all methods defined on this struct
+    ///   are safe to call.
+    unsafe fn from_raw<'a>(ptr: *mut bindings::platform_device) -> &'a mut Self {
+        // CAST: `Self` is a `repr(transparent)` wrapper around `bindings::platform_device`.
+        let ptr = ptr.cast::<Self>();
+        // SAFETY: by the function requirements the pointer is valid and we have unique access for
+        // the duration of `'a`.
+        unsafe { &mut *ptr }
+    }
+
+    /// Returns id of the platform device.
+    pub fn id(&self) -> i32 {
+        let platformdev = self.0.get();
+        // SAFETY: By the type invariants, we know that `self.ptr` is non-null and valid.
+        unsafe { (*platformdev).id }
+    }
+}
+
+impl AsRef<device::Device> for Device {
+    fn as_ref(&self) -> &device::Device {
+        let platformdev = self.0.get();
+        // SAFETY: By the type invariants, we know that `self.ptr` is non-null and valid.
+        unsafe { device::Device::as_ref(addr_of_mut!((*platformdev).dev)) }
+    }
+}
+
+/// An adapter for the registration of a Platform driver.
+struct Adapter<T: Driver> {
+    _p: PhantomData<T>,
+}
+
+impl<T: Driver> Adapter<T> {
+    /// # Safety
+    ///
+    /// `pdev` must be passed by the corresponding callback in `platform_driver`.
+    unsafe extern "C" fn probe_callback(pdev: *mut bindings::platform_device) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: This callback is called only in contexts
+            // where we can exclusively access `platform_device` because
+            // it's not published yet, so the accessors on `Device` are okay
+            // to call.
+            let dev = unsafe { Device::from_raw(pdev) };
+            T::probe(dev)?;
+            Ok(0)
+        })
+    }
+
+    /// # Safety
+    ///
+    /// `pdev` must be passed by the corresponding callback in `platform_driver`.
+    unsafe extern "C" fn remove_callback(pdev: *mut bindings::platform_device) {
+        // SAFETY: This callback is called only in contexts
+        // where we can exclusively access `platform_device` because
+        // it's not published yet, so the accessors on `Device` are okay
+        // to call.
+        let dev = unsafe { Device::from_raw(pdev) };
+        T::remove(dev);
+    }
+}
+
+/// Driver structure for a particular Platform driver.
+///
+/// Wraps the kernel's [`struct platform_driver`].
+/// This is used to register a driver for a particular PHY type with the kernel.
+///
+/// # Invariants
+///
+/// `self.0` is always in a valid state.
+///
+/// [`struct platform_driver`]: srctree/include/linux/platform.h
+#[repr(transparent)]
+pub struct DriverVTable(Opaque<bindings::platform_driver>);
+
+// SAFETY: `DriverVTable` doesn't expose any &self method to access internal data, so it's safe to
+// share `&DriverVTable` across execution context boundaries.
+unsafe impl Sync for DriverVTable {}
+
+impl DriverVTable {
+    /// Creates a [`DriverVTable`] instance from [`Driver`].
+    ///
+    /// This is used by [`module_platform_driver`] macro to create a static array of `phy_driver`.
+    ///
+    /// [`module_platform_driver`]: crate::module_platform_driver
+    pub const fn new<T: Driver, const C: usize>(match_tbl: &'static DeviceIdTable<C>) -> Self {
+        let drv = Opaque::new(bindings::platform_driver {
+            probe: if T::HAS_PROBE {
+                Some(Adapter::<T>::probe_callback)
+            } else {
+                None
+            },
+            __bindgen_anon_1: bindings::platform_driver__bindgen_ty_1 {
+                remove: if T::HAS_REMOVE {
+                    Some(Adapter::<T>::remove_callback)
+                } else {
+                    None
+                },
+            },
+            driver: create_device_driver::<T, C>(match_tbl),
+            // SAFETY: The rest is zeroed out to initialize `struct platform_driver`.
+            ..unsafe { core::mem::MaybeUninit::<bindings::platform_driver>::zeroed().assume_init() }
+        });
+
+        DriverVTable(drv)
+    }
+}
+
+const fn create_device_driver<T: Driver, const C: usize>(
+    match_tbl: &'static DeviceIdTable<C>,
+) -> bindings::device_driver {
+    bindings::device_driver {
+        name: T::NAME.as_char_ptr(),
+        of_match_table: match_tbl.get(),
+        // SAFETY: The rest is zeroed out to initialize `struct device_driver`.
+        ..unsafe { core::mem::MaybeUninit::<bindings::device_driver>::zeroed().assume_init() }
+    }
+}
+
+/// A platform driver.
+#[vtable]
+pub trait Driver {
+    /// The friendly name
+    const NAME: &'static CStr;
+
+    /// Sets up device-specific structures during discovery.
+    fn probe(_dev: &mut Device) -> Result;
+
+    /// Clean up device-specific structures during removal.
+    fn remove(_dev: &mut Device);
+}
+
+/// Registration structure for Platform driver.
+///
+/// Registers [`DriverVTable`] instance with the kernel. It will be unregistered when dropped.
+///
+/// # Invariants
+///
+/// The `driver` is currently registered to the kernel via `__platform_driver_register`.
+pub struct Registration(Pin<&'static DriverVTable>);
+
+// SAFETY: The only action allowed in a `Registration` instance is dropping it, which is safe to do
+// from any thread because `platform_drivers_unregister` can be called from any thread context.
+unsafe impl Send for Registration {}
+
+impl Registration {
+    /// Registers a Platform driver.
+    pub fn new(drv: Pin<&'static DriverVTable>, m: &'static crate::ThisModule) -> Registration {
+        unsafe {
+            bindings::__platform_driver_register(drv.0.get(), m.0);
+        }
+
+        Self(drv)
+    }
+}
+
+impl Drop for Registration {
+    fn drop(&mut self) {
+        unsafe { bindings::platform_driver_unregister(self.0 .0.get()) }
+    }
+}
+
+/// An identifier for Platform devices.
+///
+/// Represents the kernel's [`struct of_device_id`]. This is used to find an appropriate
+/// Platform driver.
+///
+/// [`struct of_device_id`]: srctree/include/linux/mod_devicetable.h
+pub struct DeviceId(&'static CStr);
+
+impl DeviceId {
+    /// A zeroed [`struct of_device_id`] used to signify end of of_device_id array.
+    ///
+    /// [`struct of_device_id`]: srctree/include/linux/mod_devicetable.h
+    pub const ZERO: bindings::of_device_id = bindings::of_device_id {
+        // SAFETY: The rest is zeroed out to initialize `struct of_device_id`.
+        ..unsafe { core::mem::MaybeUninit::<bindings::of_device_id>::zeroed().assume_init() }
+    };
+
+    /// Create new instance
+    pub const fn new(s: &'static CStr) -> Self {
+        Self(s)
+    }
+
+    const fn compatible(&self) -> [i8; 128] {
+        let compatible = self.0.as_bytes_with_nul();
+        let mut comp = [0i8; 128];
+        let mut i = 0;
+
+        while i < compatible.len() {
+            comp[i] = compatible[i] as _;
+            i += 1;
+        }
+
+        comp
+    }
+
+    // macro use only
+    #[doc(hidden)]
+    pub const fn to_rawid(&self) -> bindings::of_device_id {
+        let comp = self.compatible();
+
+        bindings::of_device_id {
+            compatible: comp,
+            // SAFETY: The rest is zeroed out to initialize `struct of_device_id`.
+            ..unsafe { core::mem::MaybeUninit::<bindings::of_device_id>::zeroed().assume_init() }
+        }
+    }
+}
+
+/// An array of identifiers for platform driver
+#[repr(transparent)]
+pub struct DeviceIdTable<const C: usize>([bindings::of_device_id; C]);
+
+impl<const C: usize> DeviceIdTable<C> {
+    /// Create a new instance
+    pub const fn new(ids: [bindings::of_device_id; C]) -> Self {
+        Self(ids)
+    }
+
+    /// Returns a raw pointer to static table.
+    pub const fn get(&'static self) -> *const bindings::of_device_id {
+        self.0.as_ptr()
+    }
+}
+
+// SAFETY: `DeviceIdTable` is only used in C side behind a *const pointer, and thus remains
+// immutable and thus can be shared across execution context boundaries.
+unsafe impl<const C: usize> Sync for DeviceIdTable<C> {}
+
+/// Declares a kernel module for Platform drivers.
+///
+/// This creates a static [`struct platform_driver`] and registers it. It also creates an array of
+/// [`struct of_device_id`] for matching the driver to devicetree device.
+///
+/// [`struct platform_driver`]: srctree/include/linux/platform.h
+/// [`struct of_device_id`]: srctree/include/linux/mod_devicetable.h
+///
+/// # Examples
+///
+/// ```
+/// # mod module_platform_driver_sample {
+/// use kernel::c_str;
+/// use kernel::platform::{self, DeviceId};
+/// use kernel::prelude::*;
+///
+/// kernel::module_platform_driver! {
+///     driver: PlatformSimple,
+///     of_table: [DeviceId::new(c_str!("platform-simple"))],
+///     name: "rust_sample_platform",
+///     author: "Rust for Linux Contributors",
+///     description: "Rust sample Platform driver",
+///     license: "GPL",
+/// }
+///
+/// struct PlatformSimple;
+///
+/// #[vtable]
+/// impl platform::Driver for PlatformSimple {
+///     const NAME: &'static CStr = c_str!("PlatformSimple");
+/// }
+/// # }
+/// ```
+///
+/// This expands to the following code:
+///
+/// ```ignore
+/// use kernel::c_str;
+/// use kernel::platform::{self, DeviceId};
+/// use kernel::prelude::*;
+///
+///
+/// struct Module {
+///     _reg: $crate::platform::Registration,
+/// }
+///
+/// module! {
+///     type: Module,
+///     name: "rust_sample_platform",
+///     author: "Rust for Linux Contributors",
+///     description: "Rust sample Platform driver",
+///     license: "GPL",
+/// }
+///
+/// const _: () = {
+///     static OF_TABLE: $crate::platform::DeviceIdTable = $crate::platform::DeviceIdTable<2>([
+///         (DeviceId::new(c_str!("platform-simple"))).to_rawid(),
+///         $crate::platform::DeviceId::ZERO,
+///     ]);
+///     static DRIVER: $crate::platform::DriverVTable =
+///         $crate::platform::DriverVTable::new::<MikrobusDriver, 2>(&OF_TABLE);
+///     impl $crate::Module for Module {
+///         fn init(module: &'static ThisModule) -> Result<Self> {
+///             let reg =
+///                 $crate::platform::Registration::new(
+///                     ::core::pin::Pin::static_ref(&DRIVER), module);
+///             Ok(Module { _reg: reg })
+///         }
+///     }
+/// }
+///
+/// struct PlatformSimple;
+///
+/// #[vtable]
+/// impl platform::Driver for PlatformSimple {
+///     const NAME: &'static CStr = c_str!("PlatformSimple");
+/// }
+/// ```
+#[macro_export]
+macro_rules! module_platform_driver {
+    (@replace_expr $_t:tt $sub:expr) => {$sub};
+
+    (@count_devices $($x:expr),*) => {
+        0usize $(+ $crate::module_platform_driver!(@replace_expr $x 1usize))*
+    };
+
+    (driver: $driver:ident, of_table: [$($of_id:expr),+ $(,)?], $($f:tt)*) => {
+        struct Module {
+            _reg: $crate::platform::Registration,
+        }
+
+        $crate::prelude::module! {
+            type: Module,
+            $($f)*
+        }
+
+        const _: () = {
+            // SAFETY: C will not read off the end of this constant since the last element is zero.
+            static OF_TABLE: $crate::platform::DeviceIdTable<
+                {$crate::module_platform_driver!(@count_devices $($of_id),+) + 1} > =
+                $crate::platform::DeviceIdTable::new(
+                    [$($of_id.to_rawid()),*, $crate::platform::DeviceId::ZERO]);
+
+            static DRIVER: $crate::platform::DriverVTable =
+                $crate::platform::DriverVTable::new::<
+                    $driver, {$crate::module_platform_driver!(@count_devices $($of_id),+) + 1}
+                >(&OF_TABLE);
+
+            impl $crate::Module for Module {
+                fn init(module: &'static ThisModule) -> Result<Self> {
+                    let reg = $crate::platform::Registration::new(
+                        ::core::pin::Pin::static_ref(&DRIVER), module);
+                    Ok(Module { _reg: reg })
+                }
+            }
+        };
+    };
+}

-- 
2.46.0

On Wed, Sep 11, 2024 at 07:57:18PM +0530, Ayush Singh wrote:
> +/// An identifier for Platform devices.
> +///
> +/// Represents the kernel's [`struct of_device_id`]. This is used to find an appropriate
> +/// Platform driver.
> +///
> +/// [`struct of_device_id`]: srctree/include/linux/mod_devicetable.h
> +pub struct DeviceId(&'static CStr);
> +
> +impl DeviceId {

<snip>

I appreciate posting this, but this really should go on top of the
device driver work Danilo Krummrich has been doing.  He and I spent a
lot of time working through this this past weekend (well, him talking
and explaining, and me asking too many stupid questions...)

I think what he has will make the platform driver/device work simpler
here, and I'll be glad to take it based on that, this "independent" code
that doesn't interact with that isn't the best idea overall.

It also will properly handle the "Driver" interaction as well, which we
need to get right, not a one-off like this for a platform driver.
Hopefully that will not cause much, if any, changes for your use of this
in your driver, but let's see.

thanks,

greg k-h

On Wed, Sep 11, 2024 at 04:56:14PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Sep 11, 2024 at 07:57:18PM +0530, Ayush Singh wrote:
> > +/// An identifier for Platform devices.
> > +///
> > +/// Represents the kernel's [`struct of_device_id`]. This is used to find an appropriate
> > +/// Platform driver.
> > +///
> > +/// [`struct of_device_id`]: srctree/include/linux/mod_devicetable.h
> > +pub struct DeviceId(&'static CStr);
> > +
> > +impl DeviceId {
> 
> <snip>
> 
> I appreciate posting this, but this really should go on top of the
> device driver work Danilo Krummrich has been doing.

If everyone agrees, I'd offer to just provide platform device / driver
abstractions with my next patch series. This way you don't need to worry
about aligning things with the rest of the abstractions yourself and throughout
potential further versions of the series.

Just be aware that I probably won't get to work on it until after LPC.

> He and I spent a
> lot of time working through this this past weekend (well, him talking
> and explaining, and me asking too many stupid questions...)
> 
> I think what he has will make the platform driver/device work simpler
> here, and I'll be glad to take it based on that, this "independent" code
> that doesn't interact with that isn't the best idea overall.
> 
> It also will properly handle the "Driver" interaction as well, which we
> need to get right, not a one-off like this for a platform driver.
> Hopefully that will not cause much, if any, changes for your use of this
> in your driver, but let's see.
> 
> thanks,
> 
> greg k-h
>

On Wed, Sep 11, 2024 at 07:35:35PM +0200, Danilo Krummrich wrote:
> On Wed, Sep 11, 2024 at 04:56:14PM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Sep 11, 2024 at 07:57:18PM +0530, Ayush Singh wrote:
> > > +/// An identifier for Platform devices.
> > > +///
> > > +/// Represents the kernel's [`struct of_device_id`]. This is used to find an appropriate
> > > +/// Platform driver.
> > > +///
> > > +/// [`struct of_device_id`]: srctree/include/linux/mod_devicetable.h
> > > +pub struct DeviceId(&'static CStr);
> > > +
> > > +impl DeviceId {
> > 
> > <snip>
> > 
> > I appreciate posting this, but this really should go on top of the
> > device driver work Danilo Krummrich has been doing.
> 
> If everyone agrees, I'd offer to just provide platform device / driver
> abstractions with my next patch series. This way you don't need to worry
> about aligning things with the rest of the abstractions yourself and throughout
> potential further versions of the series.

Covering platform device/driver abstractions in the same series would
be appreciated from asahi side. It hopefully results in earlier merge
since it avoids a dependency on the device driver abstractions.
Feel free to reach out to me for an earlier preview / rabsing of the
asahi driver.
https://github.com/AsahiLinux/linux/tree/bits/210-gpu has all relevant
rust changes from 6.11-rc but I plan to rebase onto 6.11 in the next
days and possibly import changes from 6.12-rc* / rust-next.

Thanks
Janne

On Wed, Sep 11, 2024 at 07:35:35PM +0200, Danilo Krummrich wrote:
> On Wed, Sep 11, 2024 at 04:56:14PM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Sep 11, 2024 at 07:57:18PM +0530, Ayush Singh wrote:
> > > +/// An identifier for Platform devices.
> > > +///
> > > +/// Represents the kernel's [`struct of_device_id`]. This is used to find an appropriate
> > > +/// Platform driver.
> > > +///
> > > +/// [`struct of_device_id`]: srctree/include/linux/mod_devicetable.h
> > > +pub struct DeviceId(&'static CStr);
> > > +
> > > +impl DeviceId {
> > 
> > <snip>
> > 
> > I appreciate posting this, but this really should go on top of the
> > device driver work Danilo Krummrich has been doing.
> 
> If everyone agrees, I'd offer to just provide platform device / driver
> abstractions with my next patch series. This way you don't need to worry
> about aligning things with the rest of the abstractions yourself and throughout
> potential further versions of the series.

That sounds good to me, thanks!

> Just be aware that I probably won't get to work on it until after LPC.

We will not be able to review anything until after LPC either :)

thanks,

greg k-h

On 9/11/24 20:26, Greg Kroah-Hartman wrote:
> On Wed, Sep 11, 2024 at 07:57:18PM +0530, Ayush Singh wrote:
>> +/// An identifier for Platform devices.
>> +///
>> +/// Represents the kernel's [`struct of_device_id`]. This is used to find an appropriate
>> +/// Platform driver.
>> +///
>> +/// [`struct of_device_id`]: srctree/include/linux/mod_devicetable.h
>> +pub struct DeviceId(&'static CStr);
>> +
>> +impl DeviceId {
> <snip>
>
> I appreciate posting this, but this really should go on top of the
> device driver work Danilo Krummrich has been doing.  He and I spent a
> lot of time working through this this past weekend (well, him talking
> and explaining, and me asking too many stupid questions...)
>
> I think what he has will make the platform driver/device work simpler
> here, and I'll be glad to take it based on that, this "independent" code
> that doesn't interact with that isn't the best idea overall.
>
> It also will properly handle the "Driver" interaction as well, which we
> need to get right, not a one-off like this for a platform driver.
> Hopefully that will not cause much, if any, changes for your use of this
> in your driver, but let's see.
>
> thanks,
>
> greg k-h
>
Sure, can you provide me a link to patches or maybe the branch 
containing that work? I also think it would be good to have the link in 
Zulip discussion for Platform Device and Driver.


Ayush Singh

On 9/11/24 5:52 PM, Ayush Singh wrote:
> Sure, can you provide me a link to patches or maybe the branch containing that 
> work? I also think it would be good to have the link in Zulip discussion for 
> Platform Device and Driver.

Sure, please see [1]. But please be aware that I plan to rework some parts
before sending out the next version.

[1] https://github.com/Rust-for-Linux/linux/tree/staging/rust-device

On 9/11/24 23:09, Danilo Krummrich wrote:

> On 9/11/24 5:52 PM, Ayush Singh wrote:
>> Sure, can you provide me a link to patches or maybe the branch 
>> containing that work? I also think it would be good to have the link 
>> in Zulip discussion for Platform Device and Driver.
>
> Sure, please see [1]. But please be aware that I plan to rework some 
> parts
> before sending out the next version.
>
> [1] https://github.com/Rust-for-Linux/linux/tree/staging/rust-device


Maybe the branch is just out of date? It still contains the generic 
structures for IdArray, IdTable and RawDeviceId.

Has something changed since the discussion here [0]?


[0]: 
https://lore.kernel.org/rust-for-linux/2024062031-untracked-opt-3ff1@gregkh/


Ayush Singh

On Wed, Sep 11, 2024 at 11:35:43PM +0530, Ayush Singh wrote:
> On 9/11/24 23:09, Danilo Krummrich wrote:
> 
> > On 9/11/24 5:52 PM, Ayush Singh wrote:
> > > Sure, can you provide me a link to patches or maybe the branch
> > > containing that work? I also think it would be good to have the link
> > > in Zulip discussion for Platform Device and Driver.
> > 
> > Sure, please see [1]. But please be aware that I plan to rework some
> > parts
> > before sending out the next version.
> > 
> > [1] https://github.com/Rust-for-Linux/linux/tree/staging/rust-device
> 
> 
> Maybe the branch is just out of date? It still contains the generic
> structures for IdArray, IdTable and RawDeviceId.
> 
> Has something changed since the discussion here [0]?

Yes, it has; this refers to what Greg mentioned when he said that we worked
through this last weekend in his original reply.

Things will probably not remain exactly this way, but the general concept of the
abstractions will, i.e. the existance of `IdArray`, `IdTable`, and some
`DeviceId` abstraction. Please also see [1].

[1] https://lore.kernel.org/rust-for-linux/ZuHU5yrJUOKnJGrB@pollux/

> 
> 
> [0]:
> https://lore.kernel.org/rust-for-linux/2024062031-untracked-opt-3ff1@gregkh/
> 
> 
> Ayush Singh
>