[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aSlrVLT92kmazgyh@google.com>
Date: Fri, 28 Nov 2025 09:28:52 +0000
From: Alice Ryhl <aliceryhl@...gle.com>
To: Markus Probst <markus.probst@...teo.de>
Cc: Drew Fustini <fustini@...nel.org>, Guo Ren <guoren@...nel.org>, Fu Wei <wefu@...hat.com>,
"Uwe Kleine-König" <ukleinek@...nel.org>, Michal Wilczynski <m.wilczynski@...sung.com>,
Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
"Björn Roy Baron" <bjorn3_gh@...tonmail.com>, Benno Lossin <lossin@...nel.org>,
Andreas Hindborg <a.hindborg@...nel.org>, Trevor Gross <tmgross@...ch.edu>,
Danilo Krummrich <dakr@...nel.org>, linux-riscv@...ts.infradead.org,
linux-pwm@...r.kernel.org, linux-kernel@...r.kernel.org,
rust-for-linux@...r.kernel.org
Subject: Re: [PATCH] Move pwm registration into pwm::Chip::new
On Thu, Nov 27, 2025 at 05:15:06PM +0000, Markus Probst wrote:
> The `pwm::Registration::register` function provides no guarantee that the
> function isn't called twice with the same pwm chip, which is considered
> unsafe.
>
> Add the code responsible for the registration into `pwm::Chip::new`. The
> registration will happen before the driver gets access to the refcounted
> pwm chip and can therefore guarantee that the registration isn't called
> twice on the same pwm chip.
>
> Signed-off-by: Markus Probst <markus.probst@...teo.de>
> ---
> This patch provides the additional guarantee that the pwm chip doesn't
> get registered twice.
>
> The following changes were made:
> - change the visibility of `pwm::Registration` to private
> - remove the `pwm::Registration::register` function
> - add code for registering the pwm chip in `pwm::Chip::new`
> - add Send + Sync bounds to `PwmOps`
>
> Note that I wasn't able to test this patch, due to the lack of hardware.
Overall looks reasonable, but I have one question:
> @@ -654,50 +668,23 @@ unsafe fn dec_ref(obj: NonNull<Chip<T>>) {
> // structure's state is managed and synchronized by the kernel's device model
> // and PWM core locking mechanisms. Therefore, it is safe to move the `Chip`
> // wrapper (and the pointer it contains) across threads.
> -unsafe impl<T: PwmOps + Send> Send for Chip<T> {}
> +unsafe impl<T: PwmOps> Send for Chip<T> {}
>
> // SAFETY: It is safe for multiple threads to have shared access (`&Chip`) because
> // the `Chip` data is immutable from the Rust side without holding the appropriate
> // kernel locks, which the C core is responsible for. Any interior mutability is
> // handled and synchronized by the C kernel code.
> -unsafe impl<T: PwmOps + Sync> Sync for Chip<T> {}
> +unsafe impl<T: PwmOps> Sync for Chip<T> {}
Why was this changed?
Alice
Powered by blists - more mailing lists