Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Moving the unload bundle warning to `Gsp::boot` using `hal.boot(...)?.or_else(...)` drops the warning on error paths and is dead code for `Gh100`. --
commit 6422f17d0c05bd4c3b60d0655dea96518aadd1a5 Author: Alexandre Courbot <[email protected]> gpu: nova-core: gsp: centralize missing unload bundle warnings This commit moves the warning emitted when the unload bundle cannot be constructed from the Tu102-specific boot code to the generic Gsp::boot method so it applies to all boot methods. > diff --git a/drivers/gpu/nova-core/gsp/boot.rs > b/drivers/gpu/nova-core/gsp/boot.rs > index 75054424a9516..430ffc57b1366 100644 > --- a/drivers/gpu/nova-core/gsp/boot.rs > +++ b/drivers/gpu/nova-core/gsp/boot.rs > @@ -56,7 +56,15 @@ pub(crate) fn boot( > let wpr_meta = Coherent::init(dev, GFP_KERNEL, > GspFwWprMeta::new(&gsp_fw, &fb_layout))?; > > // Perform the chipset-specific boot sequence, and retrieve the > unload bundle. > - let unload_bundle = hal.boot(&self, &ctx, &fb_layout, &wpr_meta)?; > + let unload_bundle = hal.boot(&self, &ctx, &fb_layout, > &wpr_meta)?.or_else(|| { [Severity: Medium] Will this `.or_else()` block be skipped if `hal.boot()` returns an error due to the `?` operator? For example, if `Gh100::boot()` returns an `Err` directly when it fails to build its bundle (e.g., KBox::new failure) rather than returning `Ok(None)`, it seems this block would be completely unreachable, contrary to the commit message's goal of applying it to all boot methods. Likewise for `Tu102`, if `Sec2UnloadBundle::build()` fails and results in `None`, but a subsequent step in `Tu102::boot()` fails and returns an `Err`, the immediate error propagation would swallow these warnings. > + dev_warn!(dev, "The GSP won't be able to unload properly on > unbind.\n"); > + dev_warn!( > + dev, > + "The GPU will need to be reset before the driver can bind > again.\n" > + ); > + > + None > + }); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
