Re: [PATCH] gpu: nova-core: take advantage of pci::Device::unbind()

["Danilo Krummrich" <dakr@kernel.org>]

On Mon Sep 1, 2025 at 3:13 PM CEST, Alexandre Courbot wrote:
> On Mon Sep 1, 2025 at 7:41 PM JST, Danilo Krummrich wrote:
>> On Sun Aug 31, 2025 at 3:50 PM CEST, Alexandre Courbot wrote:
>>>> +    pub(crate) fn unbind(&self, pdev: &pci::Device<device::Bound>) {
>>>> +        // Unregister the sysmem flush page before we release it.
>>>> +        kernel::warn_on!(self.bar.access(pdev.as_ref()).map_or(true, |bar| {
>>>> +            self.sysmem_flush.unregister(bar);
>>>> +
>>>> +            false
>>>> +        }));
>>>> +    }
>>>
>>> Maybe I'm overtly cautious, but this method can be called from a large
>>> part of the driver, leaving the Gpu device in a half-unbound state. The
>>> `PinnedDrop` approach had the benefit of not allowing this.
>>>
>>> One way to solve the problem would be to make this method `pub(in
>>> crate::driver)`, so other modules cannot call it.
>>
>> pub(in crate::driver) doesn't work, since hierarchically it's a sibling. :(
>
> Argh. TIL.
>
>>
>> However, I can add a doc-comment to make it a bit more obvious.
>
> Would it also help if we made `Gpu::unbind` take a
> `pci::Device<device::Core>`? That way, driver functions that only have a
> bound device could not invoke it.

This was my intention, but somehow I typed Bound.

> (also, should we make the argument a `device::Device` instead of a
> `pci::Device`?)

I think it makes sense to abstract the specific bus device, since long term we
also want to support tegra.