| Message ID | 20241205141533.111830-13-dakr@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Device / Driver PCI / Platform Rust abstractions | expand |
On Thu, Dec 05, 2024 at 03:14:43PM +0100, Danilo Krummrich wrote: > Implement the basic platform bus abstractions required to write a basic > platform driver. This includes the following data structures: > > The `platform::Driver` trait represents the interface to the driver and > provides `pci::Driver::probe` for the driver to implement. > > The `platform::Device` abstraction represents a `struct platform_device`. > > In order to provide the platform bus specific parts to a generic > `driver::Registration` the `driver::RegistrationOps` trait is implemented > by `platform::Adapter`. > > Signed-off-by: Danilo Krummrich <dakr@kernel.org> > --- > MAINTAINERS | 1 + > rust/bindings/bindings_helper.h | 2 + > rust/helpers/helpers.c | 1 + > rust/helpers/platform.c | 13 ++ > rust/kernel/lib.rs | 1 + > rust/kernel/platform.rs | 222 ++++++++++++++++++++++++++++++++ > 6 files changed, 240 insertions(+) > create mode 100644 rust/helpers/platform.c > create mode 100644 rust/kernel/platform.rs > > diff --git a/MAINTAINERS b/MAINTAINERS > index 7d6bb4b15d2c..365fc48b7041 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -7034,6 +7034,7 @@ F: rust/kernel/device.rs > F: rust/kernel/device_id.rs > F: rust/kernel/devres.rs > F: rust/kernel/driver.rs > +F: rust/kernel/platform.rs > > DRIVERS FOR OMAP ADAPTIVE VOLTAGE SCALING (AVS) > M: Nishanth Menon <nm@ti.com> > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h > index 6d7a68e2ecb7..e9fdceb568b8 100644 > --- a/rust/bindings/bindings_helper.h > +++ b/rust/bindings/bindings_helper.h > @@ -20,9 +20,11 @@ > #include <linux/jump_label.h> > #include <linux/mdio.h> > #include <linux/miscdevice.h> > +#include <linux/of_device.h> > #include <linux/pci.h> > #include <linux/phy.h> > #include <linux/pid_namespace.h> > +#include <linux/platform_device.h> > #include <linux/poll.h> > #include <linux/refcount.h> > #include <linux/sched.h> > diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c > index 3fda33cd42d4..0640b7e115be 100644 > --- a/rust/helpers/helpers.c > +++ b/rust/helpers/helpers.c > @@ -20,6 +20,7 @@ > #include "kunit.c" > #include "mutex.c" > #include "page.c" > +#include "platform.c" > #include "pci.c" > #include "pid_namespace.c" > #include "rbtree.c" > diff --git a/rust/helpers/platform.c b/rust/helpers/platform.c > new file mode 100644 > index 000000000000..ab9b9f317301 > --- /dev/null > +++ b/rust/helpers/platform.c > @@ -0,0 +1,13 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include <linux/platform_device.h> > + > +void *rust_helper_platform_get_drvdata(const struct platform_device *pdev) > +{ > + return platform_get_drvdata(pdev); > +} > + > +void rust_helper_platform_set_drvdata(struct platform_device *pdev, void *data) > +{ > + platform_set_drvdata(pdev, data); > +} > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > index 7a0e4c82ad0c..cc8f48aa162b 100644 > --- a/rust/kernel/lib.rs > +++ b/rust/kernel/lib.rs > @@ -59,6 +59,7 @@ > pub mod of; > pub mod page; > pub mod pid_namespace; > +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..868cfddb75a2 > --- /dev/null > +++ b/rust/kernel/platform.rs > @@ -0,0 +1,222 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +//! Abstractions for the platform bus. > +//! > +//! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h) > + > +use crate::{ > + bindings, container_of, device, driver, > + error::{to_result, Result}, > + of, > + prelude::*, > + str::CStr, > + types::{ARef, ForeignOwnable}, > + ThisModule, > +}; > + > +/// An adapter for the registration of platform drivers. > +pub struct Adapter<T: Driver>(T); > + > +impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> { > + type RegType = bindings::platform_driver; > + > + fn register( > + pdrv: &mut Self::RegType, > + name: &'static CStr, > + module: &'static ThisModule, > + ) -> Result { > + pdrv.driver.name = name.as_char_ptr(); > + pdrv.probe = Some(Self::probe_callback); > + > + // Both members of this union are identical in data layout and semantics. > + pdrv.__bindgen_anon_1.remove = Some(Self::remove_callback); > + pdrv.driver.of_match_table = T::OF_ID_TABLE.as_ptr(); > + > + // SAFETY: `pdrv` is guaranteed to be a valid `RegType`. > + to_result(unsafe { bindings::__platform_driver_register(pdrv, module.0) }) > + } > + > + fn unregister(pdrv: &mut Self::RegType) { > + // SAFETY: `pdrv` is guaranteed to be a valid `RegType`. > + unsafe { bindings::platform_driver_unregister(pdrv) }; > + } > +} > + > +impl<T: Driver + 'static> Adapter<T> { > + #[cfg(CONFIG_OF)] > + fn of_id_info(pdev: &Device) -> Option<&'static T::IdInfo> { > + let table = T::OF_ID_TABLE; > + > + // SAFETY: > + // - `table` has static lifetime, hence it's valid for read, > + // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`. > + let raw_id = unsafe { bindings::of_match_device(table.as_ptr(), pdev.as_ref().as_raw()) }; > + > + if raw_id.is_null() { > + None > + } else { > + // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and > + // does not add additional invariants, so it's safe to transmute. > + let id = unsafe { &*raw_id.cast::<of::DeviceId>() }; > + > + Some(table.info(<of::DeviceId as crate::device_id::RawDeviceId>::index(id))) > + } > + } > + > + #[cfg(not(CONFIG_OF))] > + fn of_id_info(_pdev: &Device) -> Option<&'static T::IdInfo> { > + None > + } > + > + // Try to retrieve an `IdInfo` from any of the ID tables; if we can't find one for a particular > + // table, it means we don't have a match in there. If we don't match any of the ID tables, it > + // means we were matched by name. > + fn id_info(pdev: &Device) -> Option<&'static T::IdInfo> { > + let id = Self::of_id_info(pdev); > + if id.is_some() { > + return id; > + } > + > + None > + } These methods are going to have to be duplicated by every bus type which can do DT matching (and later ACPI). Can't this be moved to be part of the common Driver trait. I'll say it again for Greg to comment (doubtful he will look at v3 again). Really, I think we should also align the probe method interface across bus types. That means getting rid of the 'id' in the PCI probe (or add it for everyone). Most drivers never need it. The typical case is needing nothing or the matched data. In a quick scan[1], there's only a handful of cases. So I think probe should match the common scenario and make retrieving the id match explicit if needed. Rob [1] git grep -W 'const struct pci_device_id \*' drivers/ | grep -P 'id->(?!driver_data)'
On Mon, Dec 09, 2024 at 04:37:06PM -0600, Rob Herring wrote: > On Thu, Dec 05, 2024 at 03:14:43PM +0100, Danilo Krummrich wrote: > > Implement the basic platform bus abstractions required to write a basic > > platform driver. This includes the following data structures: > > > > The `platform::Driver` trait represents the interface to the driver and > > provides `pci::Driver::probe` for the driver to implement. > > > > The `platform::Device` abstraction represents a `struct platform_device`. > > > > In order to provide the platform bus specific parts to a generic > > `driver::Registration` the `driver::RegistrationOps` trait is implemented > > by `platform::Adapter`. > > > > Signed-off-by: Danilo Krummrich <dakr@kernel.org> > > --- > > MAINTAINERS | 1 + > > rust/bindings/bindings_helper.h | 2 + > > rust/helpers/helpers.c | 1 + > > rust/helpers/platform.c | 13 ++ > > rust/kernel/lib.rs | 1 + > > rust/kernel/platform.rs | 222 ++++++++++++++++++++++++++++++++ > > 6 files changed, 240 insertions(+) > > create mode 100644 rust/helpers/platform.c > > create mode 100644 rust/kernel/platform.rs > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 7d6bb4b15d2c..365fc48b7041 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -7034,6 +7034,7 @@ F: rust/kernel/device.rs > > F: rust/kernel/device_id.rs > > F: rust/kernel/devres.rs > > F: rust/kernel/driver.rs > > +F: rust/kernel/platform.rs > > > > DRIVERS FOR OMAP ADAPTIVE VOLTAGE SCALING (AVS) > > M: Nishanth Menon <nm@ti.com> > > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h > > index 6d7a68e2ecb7..e9fdceb568b8 100644 > > --- a/rust/bindings/bindings_helper.h > > +++ b/rust/bindings/bindings_helper.h > > @@ -20,9 +20,11 @@ > > #include <linux/jump_label.h> > > #include <linux/mdio.h> > > #include <linux/miscdevice.h> > > +#include <linux/of_device.h> > > #include <linux/pci.h> > > #include <linux/phy.h> > > #include <linux/pid_namespace.h> > > +#include <linux/platform_device.h> > > #include <linux/poll.h> > > #include <linux/refcount.h> > > #include <linux/sched.h> > > diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c > > index 3fda33cd42d4..0640b7e115be 100644 > > --- a/rust/helpers/helpers.c > > +++ b/rust/helpers/helpers.c > > @@ -20,6 +20,7 @@ > > #include "kunit.c" > > #include "mutex.c" > > #include "page.c" > > +#include "platform.c" > > #include "pci.c" > > #include "pid_namespace.c" > > #include "rbtree.c" > > diff --git a/rust/helpers/platform.c b/rust/helpers/platform.c > > new file mode 100644 > > index 000000000000..ab9b9f317301 > > --- /dev/null > > +++ b/rust/helpers/platform.c > > @@ -0,0 +1,13 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > + > > +#include <linux/platform_device.h> > > + > > +void *rust_helper_platform_get_drvdata(const struct platform_device *pdev) > > +{ > > + return platform_get_drvdata(pdev); > > +} > > + > > +void rust_helper_platform_set_drvdata(struct platform_device *pdev, void *data) > > +{ > > + platform_set_drvdata(pdev, data); > > +} > > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > > index 7a0e4c82ad0c..cc8f48aa162b 100644 > > --- a/rust/kernel/lib.rs > > +++ b/rust/kernel/lib.rs > > @@ -59,6 +59,7 @@ > > pub mod of; > > pub mod page; > > pub mod pid_namespace; > > +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..868cfddb75a2 > > --- /dev/null > > +++ b/rust/kernel/platform.rs > > @@ -0,0 +1,222 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > + > > +//! Abstractions for the platform bus. > > +//! > > +//! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h) > > + > > +use crate::{ > > + bindings, container_of, device, driver, > > + error::{to_result, Result}, > > + of, > > + prelude::*, > > + str::CStr, > > + types::{ARef, ForeignOwnable}, > > + ThisModule, > > +}; > > + > > +/// An adapter for the registration of platform drivers. > > +pub struct Adapter<T: Driver>(T); > > + > > +impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> { > > + type RegType = bindings::platform_driver; > > + > > + fn register( > > + pdrv: &mut Self::RegType, > > + name: &'static CStr, > > + module: &'static ThisModule, > > + ) -> Result { > > + pdrv.driver.name = name.as_char_ptr(); > > + pdrv.probe = Some(Self::probe_callback); > > + > > + // Both members of this union are identical in data layout and semantics. > > + pdrv.__bindgen_anon_1.remove = Some(Self::remove_callback); > > + pdrv.driver.of_match_table = T::OF_ID_TABLE.as_ptr(); > > + > > + // SAFETY: `pdrv` is guaranteed to be a valid `RegType`. > > + to_result(unsafe { bindings::__platform_driver_register(pdrv, module.0) }) > > + } > > + > > + fn unregister(pdrv: &mut Self::RegType) { > > + // SAFETY: `pdrv` is guaranteed to be a valid `RegType`. > > + unsafe { bindings::platform_driver_unregister(pdrv) }; > > + } > > +} > > + > > +impl<T: Driver + 'static> Adapter<T> { > > + #[cfg(CONFIG_OF)] > > + fn of_id_info(pdev: &Device) -> Option<&'static T::IdInfo> { > > + let table = T::OF_ID_TABLE; > > + > > + // SAFETY: > > + // - `table` has static lifetime, hence it's valid for read, > > + // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`. > > + let raw_id = unsafe { bindings::of_match_device(table.as_ptr(), pdev.as_ref().as_raw()) }; > > + > > + if raw_id.is_null() { > > + None > > + } else { > > + // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and > > + // does not add additional invariants, so it's safe to transmute. > > + let id = unsafe { &*raw_id.cast::<of::DeviceId>() }; > > + > > + Some(table.info(<of::DeviceId as crate::device_id::RawDeviceId>::index(id))) > > + } > > + } > > + > > + #[cfg(not(CONFIG_OF))] > > + fn of_id_info(_pdev: &Device) -> Option<&'static T::IdInfo> { > > + None > > + } > > + > > + // Try to retrieve an `IdInfo` from any of the ID tables; if we can't find one for a particular > > + // table, it means we don't have a match in there. If we don't match any of the ID tables, it > > + // means we were matched by name. > > + fn id_info(pdev: &Device) -> Option<&'static T::IdInfo> { > > + let id = Self::of_id_info(pdev); > > + if id.is_some() { > > + return id; > > + } > > + > > + None > > + } > > These methods are going to have to be duplicated by every bus type which > can do DT matching (and later ACPI). Can't this be moved to be part of > the common Driver trait. As mentioned in v3, I agree, but I'd prefer to do this in a follow up series. > > > I'll say it again for Greg to comment (doubtful he will look at v3 > again). Really, I think we should also align the probe method interface > across bus types. That means getting rid of the 'id' in the PCI probe > (or add it for everyone). Most drivers never need it. The typical case > is needing nothing or the matched data. In a quick scan[1], there's > only a handful of cases. So I think probe should match the common > scenario and make retrieving the id match explicit if needed. I agree, but I will keep it until Greg commented on it, since I explicitly promised to add it on request. > > Rob > > [1] git grep -W 'const struct pci_device_id \*' drivers/ | grep -P 'id->(?!driver_data)'
On Mon, Dec 09, 2024 at 04:37:06PM -0600, Rob Herring wrote: > On Thu, Dec 05, 2024 at 03:14:43PM +0100, Danilo Krummrich wrote: > > Implement the basic platform bus abstractions required to write a basic > > platform driver. This includes the following data structures: > > > > The `platform::Driver` trait represents the interface to the driver and > > provides `pci::Driver::probe` for the driver to implement. > > > > The `platform::Device` abstraction represents a `struct platform_device`. > > > > In order to provide the platform bus specific parts to a generic > > `driver::Registration` the `driver::RegistrationOps` trait is implemented > > by `platform::Adapter`. > > > > Signed-off-by: Danilo Krummrich <dakr@kernel.org> > > --- > > MAINTAINERS | 1 + > > rust/bindings/bindings_helper.h | 2 + > > rust/helpers/helpers.c | 1 + > > rust/helpers/platform.c | 13 ++ > > rust/kernel/lib.rs | 1 + > > rust/kernel/platform.rs | 222 ++++++++++++++++++++++++++++++++ > > 6 files changed, 240 insertions(+) > > create mode 100644 rust/helpers/platform.c > > create mode 100644 rust/kernel/platform.rs > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 7d6bb4b15d2c..365fc48b7041 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -7034,6 +7034,7 @@ F: rust/kernel/device.rs > > F: rust/kernel/device_id.rs > > F: rust/kernel/devres.rs > > F: rust/kernel/driver.rs > > +F: rust/kernel/platform.rs > > > > DRIVERS FOR OMAP ADAPTIVE VOLTAGE SCALING (AVS) > > M: Nishanth Menon <nm@ti.com> > > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h > > index 6d7a68e2ecb7..e9fdceb568b8 100644 > > --- a/rust/bindings/bindings_helper.h > > +++ b/rust/bindings/bindings_helper.h > > @@ -20,9 +20,11 @@ > > #include <linux/jump_label.h> > > #include <linux/mdio.h> > > #include <linux/miscdevice.h> > > +#include <linux/of_device.h> > > #include <linux/pci.h> > > #include <linux/phy.h> > > #include <linux/pid_namespace.h> > > +#include <linux/platform_device.h> > > #include <linux/poll.h> > > #include <linux/refcount.h> > > #include <linux/sched.h> > > diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c > > index 3fda33cd42d4..0640b7e115be 100644 > > --- a/rust/helpers/helpers.c > > +++ b/rust/helpers/helpers.c > > @@ -20,6 +20,7 @@ > > #include "kunit.c" > > #include "mutex.c" > > #include "page.c" > > +#include "platform.c" > > #include "pci.c" > > #include "pid_namespace.c" > > #include "rbtree.c" > > diff --git a/rust/helpers/platform.c b/rust/helpers/platform.c > > new file mode 100644 > > index 000000000000..ab9b9f317301 > > --- /dev/null > > +++ b/rust/helpers/platform.c > > @@ -0,0 +1,13 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > + > > +#include <linux/platform_device.h> > > + > > +void *rust_helper_platform_get_drvdata(const struct platform_device *pdev) > > +{ > > + return platform_get_drvdata(pdev); > > +} > > + > > +void rust_helper_platform_set_drvdata(struct platform_device *pdev, void *data) > > +{ > > + platform_set_drvdata(pdev, data); > > +} > > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > > index 7a0e4c82ad0c..cc8f48aa162b 100644 > > --- a/rust/kernel/lib.rs > > +++ b/rust/kernel/lib.rs > > @@ -59,6 +59,7 @@ > > pub mod of; > > pub mod page; > > pub mod pid_namespace; > > +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..868cfddb75a2 > > --- /dev/null > > +++ b/rust/kernel/platform.rs > > @@ -0,0 +1,222 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > + > > +//! Abstractions for the platform bus. > > +//! > > +//! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h) > > + > > +use crate::{ > > + bindings, container_of, device, driver, > > + error::{to_result, Result}, > > + of, > > + prelude::*, > > + str::CStr, > > + types::{ARef, ForeignOwnable}, > > + ThisModule, > > +}; > > + > > +/// An adapter for the registration of platform drivers. > > +pub struct Adapter<T: Driver>(T); > > + > > +impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> { > > + type RegType = bindings::platform_driver; > > + > > + fn register( > > + pdrv: &mut Self::RegType, > > + name: &'static CStr, > > + module: &'static ThisModule, > > + ) -> Result { > > + pdrv.driver.name = name.as_char_ptr(); > > + pdrv.probe = Some(Self::probe_callback); > > + > > + // Both members of this union are identical in data layout and semantics. > > + pdrv.__bindgen_anon_1.remove = Some(Self::remove_callback); > > + pdrv.driver.of_match_table = T::OF_ID_TABLE.as_ptr(); > > + > > + // SAFETY: `pdrv` is guaranteed to be a valid `RegType`. > > + to_result(unsafe { bindings::__platform_driver_register(pdrv, module.0) }) > > + } > > + > > + fn unregister(pdrv: &mut Self::RegType) { > > + // SAFETY: `pdrv` is guaranteed to be a valid `RegType`. > > + unsafe { bindings::platform_driver_unregister(pdrv) }; > > + } > > +} > > + > > +impl<T: Driver + 'static> Adapter<T> { > > + #[cfg(CONFIG_OF)] > > + fn of_id_info(pdev: &Device) -> Option<&'static T::IdInfo> { > > + let table = T::OF_ID_TABLE; > > + > > + // SAFETY: > > + // - `table` has static lifetime, hence it's valid for read, > > + // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`. > > + let raw_id = unsafe { bindings::of_match_device(table.as_ptr(), pdev.as_ref().as_raw()) }; > > + > > + if raw_id.is_null() { > > + None > > + } else { > > + // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and > > + // does not add additional invariants, so it's safe to transmute. > > + let id = unsafe { &*raw_id.cast::<of::DeviceId>() }; > > + > > + Some(table.info(<of::DeviceId as crate::device_id::RawDeviceId>::index(id))) > > + } > > + } > > + > > + #[cfg(not(CONFIG_OF))] > > + fn of_id_info(_pdev: &Device) -> Option<&'static T::IdInfo> { > > + None > > + } > > + > > + // Try to retrieve an `IdInfo` from any of the ID tables; if we can't find one for a particular > > + // table, it means we don't have a match in there. If we don't match any of the ID tables, it > > + // means we were matched by name. > > + fn id_info(pdev: &Device) -> Option<&'static T::IdInfo> { > > + let id = Self::of_id_info(pdev); > > + if id.is_some() { > > + return id; > > + } > > + > > + None > > + } > > These methods are going to have to be duplicated by every bus type which > can do DT matching (and later ACPI). Can't this be moved to be part of > the common Driver trait. > > > I'll say it again for Greg to comment (doubtful he will look at v3 > again). Really, I think we should also align the probe method interface > across bus types. That means getting rid of the 'id' in the PCI probe > (or add it for everyone). Most drivers never need it. The typical case > is needing nothing or the matched data. In a quick scan[1], there's > only a handful of cases. So I think probe should match the common > scenario and make retrieving the id match explicit if needed. If there's a way for the probe callback to get access to the id that caused it to match, then it's fine with me to remove it. But is that possible? Many many drivers encode "stuff" in the id that the driver needs to know (quirks, functions, etc.) If that's not possible, then it needs to stay. thanks, greg k-h
On Tue, Dec 10, 2024 at 08:46:29AM +0100, Greg KH wrote: > On Mon, Dec 09, 2024 at 04:37:06PM -0600, Rob Herring wrote: > > On Thu, Dec 05, 2024 at 03:14:43PM +0100, Danilo Krummrich wrote: > > > Implement the basic platform bus abstractions required to write a basic > > > platform driver. This includes the following data structures: > > > > > > The `platform::Driver` trait represents the interface to the driver and > > > provides `pci::Driver::probe` for the driver to implement. > > > > > > The `platform::Device` abstraction represents a `struct platform_device`. > > > > > > In order to provide the platform bus specific parts to a generic > > > `driver::Registration` the `driver::RegistrationOps` trait is implemented > > > by `platform::Adapter`. > > > > > > Signed-off-by: Danilo Krummrich <dakr@kernel.org> > > > --- > > > MAINTAINERS | 1 + > > > rust/bindings/bindings_helper.h | 2 + > > > rust/helpers/helpers.c | 1 + > > > rust/helpers/platform.c | 13 ++ > > > rust/kernel/lib.rs | 1 + > > > rust/kernel/platform.rs | 222 ++++++++++++++++++++++++++++++++ > > > 6 files changed, 240 insertions(+) > > > create mode 100644 rust/helpers/platform.c > > > create mode 100644 rust/kernel/platform.rs > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index 7d6bb4b15d2c..365fc48b7041 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -7034,6 +7034,7 @@ F: rust/kernel/device.rs > > > F: rust/kernel/device_id.rs > > > F: rust/kernel/devres.rs > > > F: rust/kernel/driver.rs > > > +F: rust/kernel/platform.rs > > > > > > DRIVERS FOR OMAP ADAPTIVE VOLTAGE SCALING (AVS) > > > M: Nishanth Menon <nm@ti.com> > > > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h > > > index 6d7a68e2ecb7..e9fdceb568b8 100644 > > > --- a/rust/bindings/bindings_helper.h > > > +++ b/rust/bindings/bindings_helper.h > > > @@ -20,9 +20,11 @@ > > > #include <linux/jump_label.h> > > > #include <linux/mdio.h> > > > #include <linux/miscdevice.h> > > > +#include <linux/of_device.h> > > > #include <linux/pci.h> > > > #include <linux/phy.h> > > > #include <linux/pid_namespace.h> > > > +#include <linux/platform_device.h> > > > #include <linux/poll.h> > > > #include <linux/refcount.h> > > > #include <linux/sched.h> > > > diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c > > > index 3fda33cd42d4..0640b7e115be 100644 > > > --- a/rust/helpers/helpers.c > > > +++ b/rust/helpers/helpers.c > > > @@ -20,6 +20,7 @@ > > > #include "kunit.c" > > > #include "mutex.c" > > > #include "page.c" > > > +#include "platform.c" > > > #include "pci.c" > > > #include "pid_namespace.c" > > > #include "rbtree.c" > > > diff --git a/rust/helpers/platform.c b/rust/helpers/platform.c > > > new file mode 100644 > > > index 000000000000..ab9b9f317301 > > > --- /dev/null > > > +++ b/rust/helpers/platform.c > > > @@ -0,0 +1,13 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > + > > > +#include <linux/platform_device.h> > > > + > > > +void *rust_helper_platform_get_drvdata(const struct platform_device *pdev) > > > +{ > > > + return platform_get_drvdata(pdev); > > > +} > > > + > > > +void rust_helper_platform_set_drvdata(struct platform_device *pdev, void *data) > > > +{ > > > + platform_set_drvdata(pdev, data); > > > +} > > > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > > > index 7a0e4c82ad0c..cc8f48aa162b 100644 > > > --- a/rust/kernel/lib.rs > > > +++ b/rust/kernel/lib.rs > > > @@ -59,6 +59,7 @@ > > > pub mod of; > > > pub mod page; > > > pub mod pid_namespace; > > > +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..868cfddb75a2 > > > --- /dev/null > > > +++ b/rust/kernel/platform.rs > > > @@ -0,0 +1,222 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > + > > > +//! Abstractions for the platform bus. > > > +//! > > > +//! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h) > > > + > > > +use crate::{ > > > + bindings, container_of, device, driver, > > > + error::{to_result, Result}, > > > + of, > > > + prelude::*, > > > + str::CStr, > > > + types::{ARef, ForeignOwnable}, > > > + ThisModule, > > > +}; > > > + > > > +/// An adapter for the registration of platform drivers. > > > +pub struct Adapter<T: Driver>(T); > > > + > > > +impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> { > > > + type RegType = bindings::platform_driver; > > > + > > > + fn register( > > > + pdrv: &mut Self::RegType, > > > + name: &'static CStr, > > > + module: &'static ThisModule, > > > + ) -> Result { > > > + pdrv.driver.name = name.as_char_ptr(); > > > + pdrv.probe = Some(Self::probe_callback); > > > + > > > + // Both members of this union are identical in data layout and semantics. > > > + pdrv.__bindgen_anon_1.remove = Some(Self::remove_callback); > > > + pdrv.driver.of_match_table = T::OF_ID_TABLE.as_ptr(); > > > + > > > + // SAFETY: `pdrv` is guaranteed to be a valid `RegType`. > > > + to_result(unsafe { bindings::__platform_driver_register(pdrv, module.0) }) > > > + } > > > + > > > + fn unregister(pdrv: &mut Self::RegType) { > > > + // SAFETY: `pdrv` is guaranteed to be a valid `RegType`. > > > + unsafe { bindings::platform_driver_unregister(pdrv) }; > > > + } > > > +} > > > + > > > +impl<T: Driver + 'static> Adapter<T> { > > > + #[cfg(CONFIG_OF)] > > > + fn of_id_info(pdev: &Device) -> Option<&'static T::IdInfo> { > > > + let table = T::OF_ID_TABLE; > > > + > > > + // SAFETY: > > > + // - `table` has static lifetime, hence it's valid for read, > > > + // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`. > > > + let raw_id = unsafe { bindings::of_match_device(table.as_ptr(), pdev.as_ref().as_raw()) }; > > > + > > > + if raw_id.is_null() { > > > + None > > > + } else { > > > + // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and > > > + // does not add additional invariants, so it's safe to transmute. > > > + let id = unsafe { &*raw_id.cast::<of::DeviceId>() }; > > > + > > > + Some(table.info(<of::DeviceId as crate::device_id::RawDeviceId>::index(id))) > > > + } > > > + } > > > + > > > + #[cfg(not(CONFIG_OF))] > > > + fn of_id_info(_pdev: &Device) -> Option<&'static T::IdInfo> { > > > + None > > > + } > > > + > > > + // Try to retrieve an `IdInfo` from any of the ID tables; if we can't find one for a particular > > > + // table, it means we don't have a match in there. If we don't match any of the ID tables, it > > > + // means we were matched by name. > > > + fn id_info(pdev: &Device) -> Option<&'static T::IdInfo> { > > > + let id = Self::of_id_info(pdev); > > > + if id.is_some() { > > > + return id; > > > + } > > > + > > > + None > > > + } > > > > These methods are going to have to be duplicated by every bus type which > > can do DT matching (and later ACPI). Can't this be moved to be part of > > the common Driver trait. > > > > > > I'll say it again for Greg to comment (doubtful he will look at v3 > > again). Really, I think we should also align the probe method interface > > across bus types. That means getting rid of the 'id' in the PCI probe > > (or add it for everyone). Most drivers never need it. The typical case > > is needing nothing or the matched data. In a quick scan[1], there's > > only a handful of cases. So I think probe should match the common > > scenario and make retrieving the id match explicit if needed. > > If there's a way for the probe callback to get access to the id that > caused it to match, then it's fine with me to remove it. But is that > possible? Many many drivers encode "stuff" in the id that the driver > needs to know (quirks, functions, etc.) The signature would be `fn probe(dev: &mut Device, id_info: &Self::IdInfo) -> Result<Pin<KBox<Self>>>` where `id_info` is the driver private data. The other fields of struct pci_device_id should also be available from struct pci_dev`, i.e. `dev`; just need to add the corresponding accessors for it. - Danilo > > If that's not possible, then it needs to stay. > > thanks, > > greg k-h
On Tue, Dec 10, 2024 at 10:34:07AM +0100, Danilo Krummrich wrote: > On Tue, Dec 10, 2024 at 08:46:29AM +0100, Greg KH wrote: > > On Mon, Dec 09, 2024 at 04:37:06PM -0600, Rob Herring wrote: > > > On Thu, Dec 05, 2024 at 03:14:43PM +0100, Danilo Krummrich wrote: > > > > Implement the basic platform bus abstractions required to write a basic > > > > platform driver. This includes the following data structures: > > > > > > > > The `platform::Driver` trait represents the interface to the driver and > > > > provides `pci::Driver::probe` for the driver to implement. > > > > > > > > The `platform::Device` abstraction represents a `struct platform_device`. > > > > > > > > In order to provide the platform bus specific parts to a generic > > > > `driver::Registration` the `driver::RegistrationOps` trait is implemented > > > > by `platform::Adapter`. > > > > > > > > Signed-off-by: Danilo Krummrich <dakr@kernel.org> > > > > --- > > > > MAINTAINERS | 1 + > > > > rust/bindings/bindings_helper.h | 2 + > > > > rust/helpers/helpers.c | 1 + > > > > rust/helpers/platform.c | 13 ++ > > > > rust/kernel/lib.rs | 1 + > > > > rust/kernel/platform.rs | 222 ++++++++++++++++++++++++++++++++ > > > > 6 files changed, 240 insertions(+) > > > > create mode 100644 rust/helpers/platform.c > > > > create mode 100644 rust/kernel/platform.rs > > > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > > index 7d6bb4b15d2c..365fc48b7041 100644 > > > > --- a/MAINTAINERS > > > > +++ b/MAINTAINERS > > > > @@ -7034,6 +7034,7 @@ F: rust/kernel/device.rs > > > > F: rust/kernel/device_id.rs > > > > F: rust/kernel/devres.rs > > > > F: rust/kernel/driver.rs > > > > +F: rust/kernel/platform.rs > > > > > > > > DRIVERS FOR OMAP ADAPTIVE VOLTAGE SCALING (AVS) > > > > M: Nishanth Menon <nm@ti.com> > > > > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h > > > > index 6d7a68e2ecb7..e9fdceb568b8 100644 > > > > --- a/rust/bindings/bindings_helper.h > > > > +++ b/rust/bindings/bindings_helper.h > > > > @@ -20,9 +20,11 @@ > > > > #include <linux/jump_label.h> > > > > #include <linux/mdio.h> > > > > #include <linux/miscdevice.h> > > > > +#include <linux/of_device.h> > > > > #include <linux/pci.h> > > > > #include <linux/phy.h> > > > > #include <linux/pid_namespace.h> > > > > +#include <linux/platform_device.h> > > > > #include <linux/poll.h> > > > > #include <linux/refcount.h> > > > > #include <linux/sched.h> > > > > diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c > > > > index 3fda33cd42d4..0640b7e115be 100644 > > > > --- a/rust/helpers/helpers.c > > > > +++ b/rust/helpers/helpers.c > > > > @@ -20,6 +20,7 @@ > > > > #include "kunit.c" > > > > #include "mutex.c" > > > > #include "page.c" > > > > +#include "platform.c" > > > > #include "pci.c" > > > > #include "pid_namespace.c" > > > > #include "rbtree.c" > > > > diff --git a/rust/helpers/platform.c b/rust/helpers/platform.c > > > > new file mode 100644 > > > > index 000000000000..ab9b9f317301 > > > > --- /dev/null > > > > +++ b/rust/helpers/platform.c > > > > @@ -0,0 +1,13 @@ > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > + > > > > +#include <linux/platform_device.h> > > > > + > > > > +void *rust_helper_platform_get_drvdata(const struct platform_device *pdev) > > > > +{ > > > > + return platform_get_drvdata(pdev); > > > > +} > > > > + > > > > +void rust_helper_platform_set_drvdata(struct platform_device *pdev, void *data) > > > > +{ > > > > + platform_set_drvdata(pdev, data); > > > > +} > > > > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > > > > index 7a0e4c82ad0c..cc8f48aa162b 100644 > > > > --- a/rust/kernel/lib.rs > > > > +++ b/rust/kernel/lib.rs > > > > @@ -59,6 +59,7 @@ > > > > pub mod of; > > > > pub mod page; > > > > pub mod pid_namespace; > > > > +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..868cfddb75a2 > > > > --- /dev/null > > > > +++ b/rust/kernel/platform.rs > > > > @@ -0,0 +1,222 @@ > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > + > > > > +//! Abstractions for the platform bus. > > > > +//! > > > > +//! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h) > > > > + > > > > +use crate::{ > > > > + bindings, container_of, device, driver, > > > > + error::{to_result, Result}, > > > > + of, > > > > + prelude::*, > > > > + str::CStr, > > > > + types::{ARef, ForeignOwnable}, > > > > + ThisModule, > > > > +}; > > > > + > > > > +/// An adapter for the registration of platform drivers. > > > > +pub struct Adapter<T: Driver>(T); > > > > + > > > > +impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> { > > > > + type RegType = bindings::platform_driver; > > > > + > > > > + fn register( > > > > + pdrv: &mut Self::RegType, > > > > + name: &'static CStr, > > > > + module: &'static ThisModule, > > > > + ) -> Result { > > > > + pdrv.driver.name = name.as_char_ptr(); > > > > + pdrv.probe = Some(Self::probe_callback); > > > > + > > > > + // Both members of this union are identical in data layout and semantics. > > > > + pdrv.__bindgen_anon_1.remove = Some(Self::remove_callback); > > > > + pdrv.driver.of_match_table = T::OF_ID_TABLE.as_ptr(); > > > > + > > > > + // SAFETY: `pdrv` is guaranteed to be a valid `RegType`. > > > > + to_result(unsafe { bindings::__platform_driver_register(pdrv, module.0) }) > > > > + } > > > > + > > > > + fn unregister(pdrv: &mut Self::RegType) { > > > > + // SAFETY: `pdrv` is guaranteed to be a valid `RegType`. > > > > + unsafe { bindings::platform_driver_unregister(pdrv) }; > > > > + } > > > > +} > > > > + > > > > +impl<T: Driver + 'static> Adapter<T> { > > > > + #[cfg(CONFIG_OF)] > > > > + fn of_id_info(pdev: &Device) -> Option<&'static T::IdInfo> { > > > > + let table = T::OF_ID_TABLE; > > > > + > > > > + // SAFETY: > > > > + // - `table` has static lifetime, hence it's valid for read, > > > > + // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`. > > > > + let raw_id = unsafe { bindings::of_match_device(table.as_ptr(), pdev.as_ref().as_raw()) }; > > > > + > > > > + if raw_id.is_null() { > > > > + None > > > > + } else { > > > > + // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and > > > > + // does not add additional invariants, so it's safe to transmute. > > > > + let id = unsafe { &*raw_id.cast::<of::DeviceId>() }; > > > > + > > > > + Some(table.info(<of::DeviceId as crate::device_id::RawDeviceId>::index(id))) > > > > + } > > > > + } > > > > + > > > > + #[cfg(not(CONFIG_OF))] > > > > + fn of_id_info(_pdev: &Device) -> Option<&'static T::IdInfo> { > > > > + None > > > > + } > > > > + > > > > + // Try to retrieve an `IdInfo` from any of the ID tables; if we can't find one for a particular > > > > + // table, it means we don't have a match in there. If we don't match any of the ID tables, it > > > > + // means we were matched by name. > > > > + fn id_info(pdev: &Device) -> Option<&'static T::IdInfo> { > > > > + let id = Self::of_id_info(pdev); > > > > + if id.is_some() { > > > > + return id; > > > > + } > > > > + > > > > + None > > > > + } > > > > > > These methods are going to have to be duplicated by every bus type which > > > can do DT matching (and later ACPI). Can't this be moved to be part of > > > the common Driver trait. > > > > > > > > > I'll say it again for Greg to comment (doubtful he will look at v3 > > > again). Really, I think we should also align the probe method interface > > > across bus types. That means getting rid of the 'id' in the PCI probe > > > (or add it for everyone). Most drivers never need it. The typical case > > > is needing nothing or the matched data. In a quick scan[1], there's > > > only a handful of cases. So I think probe should match the common > > > scenario and make retrieving the id match explicit if needed. > > > > If there's a way for the probe callback to get access to the id that > > caused it to match, then it's fine with me to remove it. But is that > > possible? Many many drivers encode "stuff" in the id that the driver > > needs to know (quirks, functions, etc.) > > The signature would be > > `fn probe(dev: &mut Device, id_info: &Self::IdInfo) -> Result<Pin<KBox<Self>>>` > > where `id_info` is the driver private data. > > The other fields of struct pci_device_id should also be available from struct > pci_dev`, i.e. `dev`; just need to add the corresponding accessors for it. Ok, then yes, that's fine, let's make this generic and only pass in the private data, I think that's what Rob is asking for, right? thanks, greg k-h
diff --git a/MAINTAINERS b/MAINTAINERS index 7d6bb4b15d2c..365fc48b7041 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7034,6 +7034,7 @@ F: rust/kernel/device.rs F: rust/kernel/device_id.rs F: rust/kernel/devres.rs F: rust/kernel/driver.rs +F: rust/kernel/platform.rs DRIVERS FOR OMAP ADAPTIVE VOLTAGE SCALING (AVS) M: Nishanth Menon <nm@ti.com> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h index 6d7a68e2ecb7..e9fdceb568b8 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h @@ -20,9 +20,11 @@ #include <linux/jump_label.h> #include <linux/mdio.h> #include <linux/miscdevice.h> +#include <linux/of_device.h> #include <linux/pci.h> #include <linux/phy.h> #include <linux/pid_namespace.h> +#include <linux/platform_device.h> #include <linux/poll.h> #include <linux/refcount.h> #include <linux/sched.h> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c index 3fda33cd42d4..0640b7e115be 100644 --- a/rust/helpers/helpers.c +++ b/rust/helpers/helpers.c @@ -20,6 +20,7 @@ #include "kunit.c" #include "mutex.c" #include "page.c" +#include "platform.c" #include "pci.c" #include "pid_namespace.c" #include "rbtree.c" diff --git a/rust/helpers/platform.c b/rust/helpers/platform.c new file mode 100644 index 000000000000..ab9b9f317301 --- /dev/null +++ b/rust/helpers/platform.c @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <linux/platform_device.h> + +void *rust_helper_platform_get_drvdata(const struct platform_device *pdev) +{ + return platform_get_drvdata(pdev); +} + +void rust_helper_platform_set_drvdata(struct platform_device *pdev, void *data) +{ + platform_set_drvdata(pdev, data); +} diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index 7a0e4c82ad0c..cc8f48aa162b 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -59,6 +59,7 @@ pub mod of; pub mod page; pub mod pid_namespace; +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..868cfddb75a2 --- /dev/null +++ b/rust/kernel/platform.rs @@ -0,0 +1,222 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Abstractions for the platform bus. +//! +//! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h) + +use crate::{ + bindings, container_of, device, driver, + error::{to_result, Result}, + of, + prelude::*, + str::CStr, + types::{ARef, ForeignOwnable}, + ThisModule, +}; + +/// An adapter for the registration of platform drivers. +pub struct Adapter<T: Driver>(T); + +impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> { + type RegType = bindings::platform_driver; + + fn register( + pdrv: &mut Self::RegType, + name: &'static CStr, + module: &'static ThisModule, + ) -> Result { + pdrv.driver.name = name.as_char_ptr(); + pdrv.probe = Some(Self::probe_callback); + + // Both members of this union are identical in data layout and semantics. + pdrv.__bindgen_anon_1.remove = Some(Self::remove_callback); + pdrv.driver.of_match_table = T::OF_ID_TABLE.as_ptr(); + + // SAFETY: `pdrv` is guaranteed to be a valid `RegType`. + to_result(unsafe { bindings::__platform_driver_register(pdrv, module.0) }) + } + + fn unregister(pdrv: &mut Self::RegType) { + // SAFETY: `pdrv` is guaranteed to be a valid `RegType`. + unsafe { bindings::platform_driver_unregister(pdrv) }; + } +} + +impl<T: Driver + 'static> Adapter<T> { + #[cfg(CONFIG_OF)] + fn of_id_info(pdev: &Device) -> Option<&'static T::IdInfo> { + let table = T::OF_ID_TABLE; + + // SAFETY: + // - `table` has static lifetime, hence it's valid for read, + // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`. + let raw_id = unsafe { bindings::of_match_device(table.as_ptr(), pdev.as_ref().as_raw()) }; + + if raw_id.is_null() { + None + } else { + // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and + // does not add additional invariants, so it's safe to transmute. + let id = unsafe { &*raw_id.cast::<of::DeviceId>() }; + + Some(table.info(<of::DeviceId as crate::device_id::RawDeviceId>::index(id))) + } + } + + #[cfg(not(CONFIG_OF))] + fn of_id_info(_pdev: &Device) -> Option<&'static T::IdInfo> { + None + } + + // Try to retrieve an `IdInfo` from any of the ID tables; if we can't find one for a particular + // table, it means we don't have a match in there. If we don't match any of the ID tables, it + // means we were matched by name. + fn id_info(pdev: &Device) -> Option<&'static T::IdInfo> { + let id = Self::of_id_info(pdev); + if id.is_some() { + return id; + } + + None + } + + extern "C" fn probe_callback(pdev: *mut bindings::platform_device) -> core::ffi::c_int { + // SAFETY: The platform bus only ever calls the probe callback with a valid `pdev`. + let dev = unsafe { device::Device::get_device(&mut (*pdev).dev) }; + // SAFETY: `dev` is guaranteed to be embedded in a valid `struct platform_device` by the + // call above. + let mut pdev = unsafe { Device::from_dev(dev) }; + + let info = Self::id_info(&pdev); + match T::probe(&mut pdev, info) { + Ok(data) => { + // Let the `struct platform_device` own a reference of the driver's private data. + // SAFETY: By the type invariant `pdev.as_raw` returns a valid pointer to a + // `struct platform_device`. + unsafe { bindings::platform_set_drvdata(pdev.as_raw(), data.into_foreign() as _) }; + } + Err(err) => return Error::to_errno(err), + } + + 0 + } + + extern "C" fn remove_callback(pdev: *mut bindings::platform_device) { + // SAFETY: `pdev` is a valid pointer to a `struct platform_device`. + let ptr = unsafe { bindings::platform_get_drvdata(pdev) }; + + // SAFETY: `remove_callback` is only ever called after a successful call to + // `probe_callback`, hence it's guaranteed that `ptr` points to a valid and initialized + // `KBox<T>` pointer created through `KBox::into_foreign`. + let _ = unsafe { KBox::<T>::from_foreign(ptr) }; + } +} + +/// Declares a kernel module that exposes a single platform driver. +/// +/// # Examples +/// +/// ```ignore +/// kernel::module_platform_driver! { +/// type: MyDriver, +/// name: "Module name", +/// author: "Author name", +/// description: "Description", +/// license: "GPL v2", +/// } +/// ``` +#[macro_export] +macro_rules! module_platform_driver { + ($($f:tt)*) => { + $crate::module_driver!(<T>, $crate::platform::Adapter<T>, { $($f)* }); + }; +} + +/// IdTable type for platform drivers. +pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<of::DeviceId, T>; + +/// The platform driver trait. +/// +/// Drivers must implement this trait in order to get a platform driver registered. +/// +/// # Example +/// +///``` +/// # use kernel::{bindings, c_str, of, platform}; +/// +/// struct MyDriver; +/// +/// kernel::of_device_table!( +/// OF_TABLE, +/// MODULE_OF_TABLE, +/// <MyDriver as platform::Driver>::IdInfo, +/// [ +/// (of::DeviceId::new(c_str!("test,device")), ()) +/// ] +/// ); +/// +/// impl platform::Driver for MyDriver { +/// type IdInfo = (); +/// const OF_ID_TABLE: platform::IdTable<Self::IdInfo> = &OF_TABLE; +/// +/// fn probe( +/// _pdev: &mut platform::Device, +/// _id_info: Option<&Self::IdInfo>, +/// ) -> Result<Pin<KBox<Self>>> { +/// Err(ENODEV) +/// } +/// } +///``` +pub trait Driver { + /// The type holding information about each device id supported by the driver. + /// + /// TODO: Use associated_type_defaults once stabilized: + /// + /// type IdInfo: 'static = (); + type IdInfo: 'static; + + /// The table of OF device ids supported by the driver. + const OF_ID_TABLE: IdTable<Self::IdInfo>; + + /// Platform driver probe. + /// + /// Called when a new platform device is added or discovered. + /// Implementers should attempt to initialize the device here. + fn probe(dev: &mut Device, id_info: Option<&Self::IdInfo>) -> Result<Pin<KBox<Self>>>; +} + +/// The platform device representation. +/// +/// A platform device is based on an always reference counted `device:Device` instance. Cloning a +/// platform device, hence, also increments the base device' reference count. +/// +/// # Invariants +/// +/// `Device` holds a valid reference of `ARef<device::Device>` whose underlying `struct device` is a +/// member of a `struct platform_device`. +#[derive(Clone)] +pub struct Device(ARef<device::Device>); + +impl Device { + /// Convert a raw kernel device into a `Device` + /// + /// # Safety + /// + /// `dev` must be an `Aref<device::Device>` whose underlying `bindings::device` is a member of a + /// `bindings::platform_device`. + unsafe fn from_dev(dev: ARef<device::Device>) -> Self { + Self(dev) + } + + fn as_raw(&self) -> *mut bindings::platform_device { + // SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device` + // embedded in `struct platform_device`. + unsafe { container_of!(self.0.as_raw(), bindings::platform_device, dev) }.cast_mut() + } +} + +impl AsRef<device::Device> for Device { + fn as_ref(&self) -> &device::Device { + &self.0 + } +}
Implement the basic platform bus abstractions required to write a basic platform driver. This includes the following data structures: The `platform::Driver` trait represents the interface to the driver and provides `pci::Driver::probe` for the driver to implement. The `platform::Device` abstraction represents a `struct platform_device`. In order to provide the platform bus specific parts to a generic `driver::Registration` the `driver::RegistrationOps` trait is implemented by `platform::Adapter`. Signed-off-by: Danilo Krummrich <dakr@kernel.org> --- MAINTAINERS | 1 + rust/bindings/bindings_helper.h | 2 + rust/helpers/helpers.c | 1 + rust/helpers/platform.c | 13 ++ rust/kernel/lib.rs | 1 + rust/kernel/platform.rs | 222 ++++++++++++++++++++++++++++++++ 6 files changed, 240 insertions(+) create mode 100644 rust/helpers/platform.c create mode 100644 rust/kernel/platform.rs