Re: [PATCH v9 02/31] gpu: nova-core: factor .fwsignature* selection into a new find_gsp_sigs_section()

[Alexandre Courbot]

On Thu Mar 26, 2026 at 10:38 AM JST, John Hubbard wrote:
> Keep Gsp::new() from getting too cluttered, by factoring out the
> selection of .fwsignature* items. This will continue to grow as we add
> GPUs.
>
> Reviewed-by: Gary Guo <gary@xxxxxxxxxxx>
> Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
> ---
>  drivers/gpu/nova-core/firmware/gsp.rs | 31 +++++++++++++++------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/firmware/gsp.rs b/drivers/gpu/nova-core/firmware/gsp.rs
> index 2bbea1db0238..60ea730d1bd5 100644
> --- a/drivers/gpu/nova-core/firmware/gsp.rs
> +++ b/drivers/gpu/nova-core/firmware/gsp.rs
> @@ -146,6 +146,22 @@ pub(crate) struct GspFirmware {
>  }
>  
>  impl GspFirmware {
> +    fn find_gsp_sigs_section(chipset: Chipset) -> Option<&'static str> {
> +        match chipset.arch() {
> +            Architecture::Turing if matches!(chipset, Chipset::TU116 | Chipset::TU117) => {
> +                Some(".fwsignature_tu11x")
> +            }
> +            Architecture::Turing => Some(".fwsignature_tu10x"),
> +            // GA100 uses the same firmware as Turing
> +            Architecture::Ampere if chipset == Chipset::GA100 => Some(".fwsignature_tu10x"),
> +            Architecture::Ampere => Some(".fwsignature_ga10x"),
> +            Architecture::Ada => Some(".fwsignature_ad10x"),
> +            Architecture::Hopper => Some(".fwsignature_gh10x"),
> +            Architecture::BlackwellGB10x => Some(".fwsignature_gb10x"),
> +            Architecture::BlackwellGB20x => Some(".fwsignature_gb20x"),
> +        }
> +    }
> +

So in v8 I pointed out, on this very patch, that this method doesn't
need to return an `Option` [1]. Which you agreed to [2]. And yet this is
unchanged?

Oh, actually this is fixed in the next patch, for whatever reason. Why?
The original match statement was even exhaustive to begin with, so I
don't see why that `Option` was even introduced in the first place.

Please apply the feedback on the patch it was given on.

[1] https://lore.kernel.org/all/177443554510.105362.9416236198599659187.b4-review@b4/
[2] https://lore.kernel.org/all/5db75d43-830d-460e-b377-9a1f72c21fc7@xxxxxxxxxx/