From: Danilo Krummrich <dakr@kernel.org>
To: Benno Lossin <benno.lossin@proton.me>
Cc: gregkh@linuxfoundation.org, rafael@kernel.org,
bhelgaas@google.com, ojeda@kernel.org, alex.gaynor@gmail.com,
boqun.feng@gmail.com, gary@garyguo.net, bjorn3_gh@protonmail.com,
a.hindborg@kernel.org, aliceryhl@google.com, tmgross@umich.edu,
linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
rust-for-linux@vger.kernel.org
Subject: Re: [PATCH 4/4] rust: platform: fix unrestricted &mut platform::Device
Date: Thu, 13 Mar 2025 15:28:55 +0100 [thread overview]
Message-ID: <Z9Lrp2fC4b22QkPj@pollux> (raw)
In-Reply-To: <D8F2WCIXO6RQ.3OQHU95WZFB61@proton.me>
On Thu, Mar 13, 2025 at 10:49:59AM +0000, Benno Lossin wrote:
> On Thu Mar 13, 2025 at 3:13 AM CET, Danilo Krummrich wrote:
> > As by now, platform::Device is implemented as:
> >
> > #[derive(Clone)]
> > pub struct Device(ARef<device::Device>);
> >
> > This may be convenient, but has the implication that drivers can call
> > device methods that require a mutable reference concurrently at any
> > point of time.
>
> Similar to the other patch, I didn't find any methods taking `&mut self`
> but I might have missed them.
`platform::Device` does not have any yet. But we still need the pattern. Once we
land the `dma::Device` trait, we'll need:
impl dma::Device for platform::Device<Core> {}
to derive the DMA methods.
Besides that, I want bus device implementations to be consistent.
>
> > Instead define platform::Device as
> >
> > pub struct Device<Ctx: DeviceContext = Normal>(
> > Opaque<bindings::platform_dev>,
> > PhantomData<Ctx>,
> > );
> >
> > and manually implement the AlwaysRefCounted trait.
> >
> > With this we can implement methods that should only be called from
> > bus callbacks (such as probe()) for platform::Device<Core>. Consequently,
> > we make this type accessible in bus callbacks only.
> >
> > Arbitrary references taken by the driver are still of type
> > ARef<platform::Device> and hence don't provide access to methods that are
> > reserved for bus callbacks.
> >
> > Fixes: 683a63befc73 ("rust: platform: add basic platform device / driver abstractions")
> > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>
> The same two nits from patch #3 also apply.
>
> Reviewed-by: Benno Lossin <benno.lossin@proton.me>
>
> ---
> Cheers,
> Benno
>
> > ---
> > rust/kernel/platform.rs | 93 ++++++++++++++++++----------
> > samples/rust/rust_driver_platform.rs | 16 +++--
> > 2 files changed, 74 insertions(+), 35 deletions(-)
>
next prev parent reply other threads:[~2025-03-13 14:29 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-13 2:13 [PATCH 0/4] Improve soundness of bus device abstractions Danilo Krummrich
2025-03-13 2:13 ` [PATCH 1/4] rust: pci: use to_result() in enable_device_mem() Danilo Krummrich
2025-03-13 10:21 ` Benno Lossin
2025-03-13 2:13 ` [PATCH 2/4] rust: device: implement device context marker Danilo Krummrich
2025-03-13 10:29 ` Benno Lossin
2025-03-13 10:41 ` Danilo Krummrich
2025-03-13 10:52 ` Benno Lossin
2025-03-13 14:20 ` Danilo Krummrich
2025-03-13 14:31 ` Benno Lossin
2025-03-13 10:47 ` Benno Lossin
2025-03-13 2:13 ` [PATCH 3/4] rust: pci: fix unrestricted &mut pci::Device Danilo Krummrich
2025-03-13 10:44 ` Benno Lossin
2025-03-13 14:25 ` Danilo Krummrich
2025-03-13 14:30 ` Benno Lossin
2025-03-13 2:13 ` [PATCH 4/4] rust: platform: fix unrestricted &mut platform::Device Danilo Krummrich
2025-03-13 10:49 ` Benno Lossin
2025-03-13 14:28 ` Danilo Krummrich [this message]
2025-03-13 14:41 ` Benno Lossin
2025-03-13 6:08 ` [PATCH 0/4] Improve soundness of bus device abstractions Boqun Feng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z9Lrp2fC4b22QkPj@pollux \
--to=dakr@kernel.org \
--cc=a.hindborg@kernel.org \
--cc=alex.gaynor@gmail.com \
--cc=aliceryhl@google.com \
--cc=benno.lossin@proton.me \
--cc=bhelgaas@google.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=gary@garyguo.net \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=ojeda@kernel.org \
--cc=rafael@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=tmgross@umich.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.