[PATCH] Move pwm registration into pwm::Chip::new

[Alice Ryhl]

On Fri, Nov 28, 2025 at 12:25:04PM +0000, Markus Probst wrote:
> On Fri, 2025-11-28 at 09:28 +0000, Alice Ryhl wrote:
> > 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 at posteo.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?
> 
> Registration::register required PwmOps to be Send + Sync.
> Prior to this change, Chip::new didn't require it for PwmOps. Meaning
> it was possible to allocate a new Chip with a PwmOps that is not Send +
> Sync. As there was no use for it and it isn't possible anymore to
> allocate a new Chip without registering it, I added Send + Sync as
> trait dependency (see 1. hunk of rust/kernel/pwm.rs in the patch).
> 
> Because PwmOps now implied Send + Sync, it wasn't necessary anymore to
> have the additional bounds there.

Ah ok, I missed that. That's fine then.

Alice