On Sat, 15 Aug 2026 13:33:01 +0200
Danilo Krummrich <[email protected]> wrote:
> On Sat Aug 15, 2026 at 12:23 PM CEST, Onur Özkan wrote:
> > +#[pin_data]
> > +struct Controller<'bound> {
>
> Please only use the lifetime name 'bound for bus device private data (which
> defines the 'bound lifetime). Everything within is shorter lived and should
> carry a different name (see also [1]).
>
> I'd suggest 'ctrl or just 'a, once you have self-referencial fields 'ctrl (or
> similar) is preferred.
>
> [1] https://lore.kernel.org/all/[email protected]/
>
Noted, thanks.
> > + fn reset_work(self: &Arc<Self>) {
> > + if !self.try_transition(ResetState::Pending,
> > ResetState::InProgress) {
> > + return;
> > + }
> > +
> > + dev_info!(self.pdev, "Starting GPU reset.\n");
>
> Please use dev_dbg!().
>
> > + // Wait for current hardware accesses to finish before resetting.
> > + let reset_guard = self.hw.close();
> > + let reset_result = run_reset(self.pdev.as_ref(), &self.iomem);
> > + drop(reset_guard);
> > +
> > + if let Err(e) = reset_result {
> > + dev_err!(self.pdev, "GPU reset failed: {:?}\n", e);
> > +
> > + // TODO: Unplug the GPU.
> > + // There is no API for unplugging the GPU and this is
> > unreachable
> > + // for now since there are no hardware users for reset API.
> > + } else {
> > + dev_info!(self.pdev, "GPU reset completed.\n");
>
> Same here.
>
> > + }
> > +
> > + let _ = self.try_transition(ResetState::InProgress,
> > ResetState::Idle);
> > + }
> > +}
> > +
> > +/// User-facing handle for scheduling resets.
> > +///
> > +/// Dropping the handle drains any queued or in-flight reset work before
> > the
> > +/// [`ScopedQueue`] and the clock and regulator resources are released.
> > +pub(crate) struct ResetHandle<'bound> {
>
> Same as above 'reset or just 'a.
>
> > + controller: Arc<Controller<'bound>>,
>
> This can just be ScopedWork<Controller<'a>>; no extra reference count and
> allocation needed, as the constructor gives you an impl PinInit.
>
> It also gets you rid of the open-coded cancel_sync() call in the destructor of
> ResetHandle.
Yeah I am aware that (you already said that couple times in weekly calls),
I am just being a bit lazy on this, sorry :/. I will cover this in v6.
>
> Also note that [2] already has Send + Sync for OwnedQueue when OwnedQueue is
> introduced, so you can drop patch 1 of this series.
Okay, I will pull your series and drop this patch along with ScopedQueue and
OwnedQueue as they are already included in your series.
Thanks,
Onur
>
> Thanks,
> Danilo
>
> [2] https://lore.kernel.org/all/[email protected]/
