[PATCH v2 2/4] rust: device: implement device context marker

Danilo Krummrich posted 4 patches 9 months, 1 week ago
[PATCH v2 2/4] rust: device: implement device context marker
Posted by Danilo Krummrich 9 months, 1 week ago
Some bus device functions should only be called from bus callbacks,
such as probe(), remove(), resume(), suspend(), etc.

To ensure this add device context marker structs, that can be used as
generics for bus device implementations.

Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Suggested-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/device.rs | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index db2d9658ba47..21b343a1dc4d 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -209,6 +209,32 @@ unsafe impl Send for Device {}
 // synchronization in `struct device`.
 unsafe impl Sync for Device {}
 
+/// Marker trait for the context of a bus specific device.
+///
+/// Some functions of a bus specific device should only be called from a certain context, i.e. bus
+/// callbacks, such as `probe()`.
+///
+/// This is the marker trait for structures representing the context of a bus specific device.
+pub trait DeviceContext: private::Sealed {}
+
+/// The [`Normal`] context is the context of a bus specific device when it is not an argument of
+/// any bus callback.
+pub struct Normal;
+
+/// The [`Core`] context is the context of a bus specific device when it is supplied as argument of
+/// any of the bus callbacks, such as `probe()`.
+pub struct Core;
+
+mod private {
+    pub trait Sealed {}
+
+    impl Sealed for super::Core {}
+    impl Sealed for super::Normal {}
+}
+
+impl DeviceContext for Core {}
+impl DeviceContext for Normal {}
+
 #[doc(hidden)]
 #[macro_export]
 macro_rules! dev_printk {
-- 
2.48.1
Re: [PATCH v2 2/4] rust: device: implement device context marker
Posted by Boqun Feng 9 months, 1 week ago
On Fri, Mar 14, 2025 at 05:09:05PM +0100, Danilo Krummrich wrote:
> Some bus device functions should only be called from bus callbacks,
> such as probe(), remove(), resume(), suspend(), etc.
> 
> To ensure this add device context marker structs, that can be used as
> generics for bus device implementations.
> 
> Reviewed-by: Benno Lossin <benno.lossin@proton.me>
> Suggested-by: Benno Lossin <benno.lossin@proton.me>

Try chronological order for the tags? It was suggested first and then
reviewed.

Regards,
Boqun

> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/device.rs | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index db2d9658ba47..21b343a1dc4d 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -209,6 +209,32 @@ unsafe impl Send for Device {}
>  // synchronization in `struct device`.
>  unsafe impl Sync for Device {}
>  
> +/// Marker trait for the context of a bus specific device.
> +///
> +/// Some functions of a bus specific device should only be called from a certain context, i.e. bus
> +/// callbacks, such as `probe()`.
> +///
> +/// This is the marker trait for structures representing the context of a bus specific device.
> +pub trait DeviceContext: private::Sealed {}
> +
> +/// The [`Normal`] context is the context of a bus specific device when it is not an argument of
> +/// any bus callback.
> +pub struct Normal;
> +
> +/// The [`Core`] context is the context of a bus specific device when it is supplied as argument of
> +/// any of the bus callbacks, such as `probe()`.
> +pub struct Core;
> +
> +mod private {
> +    pub trait Sealed {}
> +
> +    impl Sealed for super::Core {}
> +    impl Sealed for super::Normal {}
> +}
> +
> +impl DeviceContext for Core {}
> +impl DeviceContext for Normal {}
> +
>  #[doc(hidden)]
>  #[macro_export]
>  macro_rules! dev_printk {
> -- 
> 2.48.1
>
Re: [PATCH v2 2/4] rust: device: implement device context marker
Posted by Danilo Krummrich 9 months, 1 week ago
On Fri, Mar 14, 2025 at 10:21:58AM -0700, Boqun Feng wrote:
> On Fri, Mar 14, 2025 at 05:09:05PM +0100, Danilo Krummrich wrote:
> > Some bus device functions should only be called from bus callbacks,
> > such as probe(), remove(), resume(), suspend(), etc.
> > 
> > To ensure this add device context marker structs, that can be used as
> > generics for bus device implementations.
> > 
> > Reviewed-by: Benno Lossin <benno.lossin@proton.me>
> > Suggested-by: Benno Lossin <benno.lossin@proton.me>
> 
> Try chronological order for the tags? It was suggested first and then
> reviewed.

Is that a thing? When I apply patches I usully keep ACKs, RBs and SOBs together
at the bottom.
Re: [PATCH v2 2/4] rust: device: implement device context marker
Posted by Boqun Feng 9 months, 1 week ago
On Fri, Mar 14, 2025 at 06:31:48PM +0100, Danilo Krummrich wrote:
> On Fri, Mar 14, 2025 at 10:21:58AM -0700, Boqun Feng wrote:
> > On Fri, Mar 14, 2025 at 05:09:05PM +0100, Danilo Krummrich wrote:
> > > Some bus device functions should only be called from bus callbacks,
> > > such as probe(), remove(), resume(), suspend(), etc.
> > > 
> > > To ensure this add device context marker structs, that can be used as
> > > generics for bus device implementations.
> > > 
> > > Reviewed-by: Benno Lossin <benno.lossin@proton.me>
> > > Suggested-by: Benno Lossin <benno.lossin@proton.me>
> > 
> > Try chronological order for the tags? It was suggested first and then
> > reviewed.
> 
> Is that a thing? When I apply patches I usully keep ACKs, RBs and SOBs together
> at the bottom.

I don't think it's a hard requirement, but it makes logical sense to
order the tags except your own SoB based on chronological order when
re-submitting a new version IMO. It's in the same spirit of putting SoBs
in chronological when multiple people handle the patches.

But it's your choice, I just feel it's a bit odd in the current order
;-)

Regards,
Boqun
Re: [PATCH v2 2/4] rust: device: implement device context marker
Posted by Miguel Ojeda 9 months, 1 week ago
On Fri, Mar 14, 2025 at 6:31 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> Is that a thing? When I apply patches I usully keep ACKs, RBs and SOBs together
> at the bottom.

It depends on the maintainers/subsystem. Some do chronological, some
do groups (even to the point of having a defined order). Chronological
loses less information but "looks worse". Some consider RBs should go
on top, others below.

I think most people respect the SoB boundary though, when applying a
patch from someone else, and that is likely the most important part.

Cheers,
Miguel