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

[John Hubbard]

On 3/30/26 7:29 AM, Alexandre Courbot wrote:
> 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?

It's an off-by-one error in chosing which commit to edit during git
rebase.

> 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.

Because at various points during the many months that this patchset
has existed, there were unsupported Arches. Turing, for example.


thanks,
-- 
John Hubbard