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

[Timur Tabi <ttabi@nvidia.com>]

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.

> 
> >  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`?

> > +fn request_tlv(
> > +    dev: &device::Device,
> > +    chipset: gpu::Chipset,
> > +    name: &str,
> > +) -> Result<firmware::Firmware> {
> > +    let chip_name = chipset.name();
> > +
> > +    dev_info!(dev, "loading firmware image {}.tlv\n", name);
> 
> Please use dev_dbg!() instead, this should not be printed under normal
> operation.

Ok.