On 5/18/26 7:55 PM, Eliot Courtney wrote: > `FwSecBiosBuilder` now only contains `falcon_ucode_offset` which just > gets passed directly into `FwSecBiosImage`. Remove `FwSecBiosBuilder` > and construct `FwSecBiosImage` directly, as a simplification. > > Reviewed-by: Joel Fernandes <[email protected]> > Signed-off-by: Eliot Courtney <[email protected]> > --- > drivers/gpu/nova-core/vbios.rs | 98 > +++++++++++++++++------------------------- > 1 file changed, 39 insertions(+), 59 deletions(-) ... > -impl FwSecBiosBuilder { > - fn setup_falcon_data( > - &mut self, > - pci_at_image: &PciAtBiosImage, > - first_fwsec: &FwSecBiosBuilder, > - ) -> Result { > +impl FwSecBiosImage { > + /// Build the final `FwSecBiosImage` from the PCI-AT and FWSEC BIOS > images
This needs a period at the end of the sentence. The patch looks good, I had to look pretty hard in order find that tiny nit. :) Reviewed-by: John Hubbard <[email protected]> thanks, -- John Hubbard > + fn new( > + pci_at_image: PciAtBiosImage, > + first_fwsec: BiosImage, > + second_fwsec: BiosImage, > + ) -> Result<FwSecBiosImage> { > let offset = pci_at_image.falcon_data_offset()?; > > // The offset is from the start of the first FwSec image, but it > // may point into the second FwSec image. Treat the two FwSec images > // as contiguous here and subtract the first image length when the > // target lies in the second one. > - let pmu_lookup_data = if offset < first_fwsec.base.data.len() { > - first_fwsec.base.data.get(offset..) > + let pmu_lookup_data = if offset < first_fwsec.data.len() { > + first_fwsec.data.get(offset..) > } else { > - self.base.data.get(offset - first_fwsec.base.data.len()..) > + second_fwsec.data.get(offset - first_fwsec.data.len()..) > } > .ok_or(EINVAL)?; > > - let pmu_lookup_table = PmuLookupTable::new(&self.base.dev, > pmu_lookup_data)?; > + let pmu_lookup_table = PmuLookupTable::new(&second_fwsec.dev, > pmu_lookup_data)?; > > let entry = pmu_lookup_table > .find_entry_by_type(FALCON_UCODE_ENTRY_APPID_FWSEC_PROD) > .inspect_err(|e| { > dev_err!( > - self.base.dev, > + second_fwsec.dev, > "PmuLookupTableEntry not found, error: {:?}\n", > e > ); > @@ -987,34 +980,21 @@ fn setup_falcon_data( > > let falcon_ucode_offset = usize::from_safe_cast(entry.data) > .checked_sub(pci_at_image.base.data.len()) > - .and_then(|o| o.checked_sub(first_fwsec.base.data.len())) > + .and_then(|o| o.checked_sub(first_fwsec.data.len())) > .ok_or(EINVAL) > .inspect_err(|_| { > - dev_err!(self.base.dev, "Falcon Ucode offset not in second > Fwsec.\n"); > + dev_err!( > + second_fwsec.dev, > + "Falcon Ucode offset not in second Fwsec.\n" > + ); > })?; > > - self.falcon_ucode_offset = Some(falcon_ucode_offset); > - Ok(()) > + Ok(FwSecBiosImage { > + base: second_fwsec, > + falcon_ucode_offset, > + }) > } > > - /// Build the final FwSecBiosImage from this builder > - fn build(self) -> Result<FwSecBiosImage> { > - let ret = FwSecBiosImage { > - base: self.base, > - falcon_ucode_offset: self.falcon_ucode_offset.ok_or(EINVAL)?, > - }; > - > - if cfg!(debug_assertions) { > - // Print the desc header for debugging > - let desc = ret.header()?; > - dev_dbg!(ret.base.dev, "PmuLookupTableEntry desc: {:#?}\n", > desc); > - } > - > - Ok(ret) > - } > -} > - > -impl FwSecBiosImage { > /// Get the FwSec header ([`FalconUCodeDesc`]). > pub(crate) fn header(&self) -> Result<FalconUCodeDesc> { > let data = self >
