Re: [PATCH 3/9] gpu: nova-core: gsp: expose GSP-RM internal client and subdevice handles
From: Alexandre Courbot
Date: Mon Mar 09 2026 - 22:37:11 EST
On Tue Mar 10, 2026 at 11:17 AM JST, Joel Fernandes wrote:
> Hi John,
>
>> On Mar 9, 2026, at 8:06 PM, John Hubbard <jhubbard@xxxxxxxxxx> wrote:
>>
>> On 3/9/26 4:41 PM, Joel Fernandes wrote:
>>>>> On Mar 9, 2026, at 5:22 PM, Joel Fernandes <joelagnelf@xxxxxxxxxx> wrote:
>>>> On Fri, Feb 27, 2026 at 09:32:08PM +0900, Eliot Courtney wrote:
>>>>> Expose the `hInternalClient` and `hInternalSubdevice` handles. These are
>>>>> needed for RM control calls.
>>>>>
>>>>> Signed-off-by: Eliot Courtney <ecourtney@xxxxxxxxxx>
>>>>> ---
>>>>> drivers/gpu/nova-core/gsp/commands.rs | 16 ++++++++++++++++
>>>>> drivers/gpu/nova-core/gsp/fw/commands.rs | 10 ++++++++++
>>>>> 2 files changed, 26 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
>>>>> index 4740cda0b51c..2cadfcaf9a8a 100644
>>>>> --- a/drivers/gpu/nova-core/gsp/commands.rs
>>>>> +++ b/drivers/gpu/nova-core/gsp/commands.rs
>>>>> @@ -197,6 +197,8 @@ fn init(&self) -> impl Init<Self::Command, Self::InitError> {
>>>>> /// The reply from the GSP to the [`GetGspInfo`] command.
>>>>> pub(crate) struct GetGspStaticInfoReply {
>>>>> gpu_name: [u8; 64],
>>>>> + h_client: u32,
>>>>> + h_subdevice: u32,
>>>>
>>>> I would rather have more descriptive names please. 'client_handle',
>>
>> Maybe it's better to mirror the Open RM names, which are ancient and
>> well known in those circles. Changing them at this point is probably
>> going to result in a slightly worse situation, because there are
>> probably millions of lines of code out there that use the existing
>> nomenclature.
>
> I have to disagree a bit here. Saying h_ in code is a bit meaningless:
> there is no mention of the word "handle" anywhere near these fields.
> h_ could mean "higher", "hardware", or any number of things. The only
> reason I know it means "handle" is because of expertise with Nvidia
> drivers. The `_handle` suffix is self-documenting; `h_` is not.
I tend to agree with Joel that we should try to avoid NVisms when they
get in the way of clarity - that's what we did so far actually. We can
always mention the RM name of fields in the doccomments.
The only exception being generated bindings, but they reside in their
own module and are opaque to the rest of the driver.