Re: [PATCH v3 02/11] gpu: nova-core: move GSP boot code out of `Gpu` constructor

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

On Tue Sep 9, 2025 at 4:11 PM CEST, Alexandre Courbot wrote:
> On Wed Sep 3, 2025 at 5:27 PM JST, Danilo Krummrich wrote:
>> On Wed Sep 3, 2025 at 9:10 AM CEST, Alexandre Courbot wrote:
>>> On Wed Sep 3, 2025 at 8:12 AM JST, Danilo Krummrich wrote:
>>>> On 9/2/25 4:31 PM, Alexandre Courbot wrote:
>>>>>       pub(crate) fn new(
>>>>>           pdev: &pci::Device<device::Bound>,
>>>>>           devres_bar: Arc<Devres<Bar0>>,
>>>>
>>>> The diff is hiding it, but with this patch we should also make sure that this 
>>>> returns impl PinInit<Self, Error> rather than Result<impl PinInit<Self>.
>>>>
>>>> I think this should be possible now.
>>>
>>> There is still code that can return errors (falcon creation, etc) - do
>>> you mean that we should move it into the pin initializer and turn it
>>> into a `try_pin_init`?
>>
>> Yeah, that would be better practice, if it doesn't work out for a good reason
>> we can also fall back to Result<impl PinInit<Self, Error>, but we should at
>> least try to avoid it.
>
> I tried but could not do it in a way that is satisfying. The problem is
> that `Gpu::new` receives a `Arc<Devres<Bar0>>`, which we need to
> `access` in order to do anything useful with it. If we first store it
> into the `Gpu` structure, then every subsequent member needs to `access`
> it in its own code block in order to perform their own initialization.
> This is quite cumbersome.
>
> If there is a way to obtain the `Bar0` once after the `bar` member of
> `Gpu` is initialized, and then use that instance with each remaining
> member, then that problem would go away but I am not aware of such a
> thing.

What about this?

	impl Gpu {
	    pub(crate) fn new<'a>(
	        dev: &'a Device<Bound>,
	        bar: &'a Bar0
	        devres_bar: Arc<Devres<Bar0>>,
	    ) -> impl PinInit<Self, Error> + 'a {
	        try_pin_init(Self {
	            bar: devres_bar,
	            spec: Spec::new(bar)?,
	            gsp_falcon: Falcon::<Gsp>::new(dev, spec.chipset)?,
	            sec2_falcon: Falcon::<Sec2>::new(dev, spec.chipset)?,
	            sysmem_flush: SysmemFlush::register(dev, bar, spec.chipset)?
	            gsp <- Gsp::new(gsp_falcon, sec2_falcon, sysmem_flush)?,
	        })
	    }
	}