Re: [PATCH 2/8] gpu: nova-core: add request_tlv to load TLV images

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

On Thu Jun 11, 2026 at 6:45 PM CEST, Timur Tabi wrote:
> On Thu, 2026-06-11 at 00:00 +0200, Danilo Krummrich wrote:
>
>> 
>> > diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
>> > index 279fbacd0b8e..2749c196416d 100644
>> > --- a/drivers/gpu/nova-core/firmware.rs
>> > +++ b/drivers/gpu/nova-core/firmware.rs
>> > @@ -33,9 +33,11 @@
>> >  pub(crate) mod gsp;
>> >  pub(crate) mod riscv;
>> >  
>> > +#[allow(unused)]
>> 
>> This shouldn't be needed, it can just be removed when once it becomes unused,
>> no?
>
> Technically, yes.  As a convenience, I removed it here so that I wouldn't have to worry about it
> in a later patch.  I find that the patches themselves are cleaner that way.  I don't want to
> have a patch where the only change to firmware.rs is adding or removing the `unused` statement.

You remove FIRMWARE_VERSION in patch 8 together with its (last) user, so adding
#[allow(unused)] here in unnecessary. Am I missing anything?

>> 
>> >  pub(crate) const FIRMWARE_VERSION: &str = "570.144";
>> >  
>> >  /// Requests the GPU firmware `name` suitable for `chipset`, with version `ver`.
>> > +#[allow(unused)]
>> >  fn request_firmware(
>> >      dev: &device::Device,
>> >      chipset: gpu::Chipset,
>> > @@ -48,6 +50,21 @@ fn request_firmware(
>> >          .and_then(|path| firmware::Firmware::request(&path, dev))
>> >  }
>> >  
>> > +/// Requests the GPU firmware TLV `name` suitable for `chipset`.
>> > +#[allow(unused)]
>> 
>> Please use expect instead.
>
> Ok, but why should I use `expect` here but `allowed` is for FIRMWARE_VERSION?  As soon as I use
> request_tlv() in patch #4, won't get I get a compiler warning and be forced to change it to
> `allow`?

Gary already explained it very well.

Thanks,
Danilo