On Thu, Jun 12, 2025 at 11:01:43PM +0900, Alexandre Courbot wrote:
> + /// Perform a DMA write according to `load_offsets` from `dma_handle`
> into the falcon's
> + /// `target_mem`.
> + ///
> + /// `sec` is set if the loaded firmware is expected to run in secure
> mode.
> + fn dma_wr(
> + &self,
> + bar: &Bar0,
> + dma_handle: bindings::dma_addr_t,
I think we should pass &F from dma_load() rather than the raw handle.
<snip>
> +fn select_core_ga102<E: FalconEngine>(bar: &Bar0) -> Result {
> + let bcr_ctrl = regs::NV_PRISCV_RISCV_BCR_CTRL::read(bar, E::BASE);
> + if bcr_ctrl.core_select() != PeregrineCoreSelect::Falcon {
> + regs::NV_PRISCV_RISCV_BCR_CTRL::default()
> + .set_core_select(PeregrineCoreSelect::Falcon)
> + .write(bar, E::BASE);
> +
> + util::wait_on(Duration::from_millis(10), || {
As agreed, can you please add a brief comment to justify the timeout?
> + let r = regs::NV_PRISCV_RISCV_BCR_CTRL::read(bar, E::BASE);
> + if r.valid() {
> + Some(())
> + } else {
> + None
> + }
> + })?;
> + }
> +
> + Ok(())
> +}
> +
> +fn signature_reg_fuse_version_ga102(
> + dev: &device::Device,
> + bar: &Bar0,
> + engine_id_mask: u16,
> + ucode_id: u8,
> +) -> Result<u32> {
> + // The ucode fuse versions are contained in the
> FUSE_OPT_FPF_<ENGINE>_UCODE<X>_VERSION
> + // registers, which are an array. Our register definition macros do not
> allow us to manage them
> + // properly, so we need to hardcode their addresses for now.
Sounds like a TODO?
> +
> + // Each engine has 16 ucode version registers numbered from 1 to 16.
> + if ucode_id == 0 || ucode_id > 16 {
> + dev_err!(dev, "invalid ucode id {:#x}", ucode_id);
> + return Err(EINVAL);
> + }
> +
> + // Base address of the FUSE registers array corresponding to the engine.
> + let reg_fuse_base = if engine_id_mask & 0x0001 != 0 {
> + regs::NV_FUSE_OPT_FPF_SEC2_UCODE1_VERSION::OFFSET
> + } else if engine_id_mask & 0x0004 != 0 {
> + regs::NV_FUSE_OPT_FPF_NVDEC_UCODE1_VERSION::OFFSET
> + } else if engine_id_mask & 0x0400 != 0 {
> + regs::NV_FUSE_OPT_FPF_GSP_UCODE1_VERSION::OFFSET
> + } else {
> + dev_err!(dev, "unexpected engine_id_mask {:#x}", engine_id_mask);
> + return Err(EINVAL);
> + };
> +
> + // Read `reg_fuse_base[ucode_id - 1]`.
> + let reg_fuse_version =
> + bar.read32(reg_fuse_base + ((ucode_id - 1) as usize *
> core::mem::size_of::<u32>()));
> +
> + Ok(fls_u32(reg_fuse_version))
> +}
> +
> +fn program_brom_ga102<E: FalconEngine>(bar: &Bar0, params:
> &FalconBromParams) -> Result {
> + regs::NV_PFALCON2_FALCON_BROM_PARAADDR::default()
> + .set_value(params.pkc_data_offset)
> + .write(bar, E::BASE);
> + regs::NV_PFALCON2_FALCON_BROM_ENGIDMASK::default()
> + .set_value(params.engine_id_mask as u32)
> + .write(bar, E::BASE);
> + regs::NV_PFALCON2_FALCON_BROM_CURR_UCODE_ID::default()
> + .set_ucode_id(params.ucode_id)
> + .write(bar, E::BASE);
> + regs::NV_PFALCON2_FALCON_MOD_SEL::default()
> + .set_algo(FalconModSelAlgo::Rsa3k)
> + .write(bar, E::BASE);
> +
> + Ok(())
> +}
> +
> +pub(super) struct Ga102<E: FalconEngine>(PhantomData<E>);
> +
> +impl<E: FalconEngine> Ga102<E> {
> + pub(super) fn new() -> Self {
> + Self(PhantomData)
> + }
> +}
> +
> +impl<E: FalconEngine> FalconHal<E> for Ga102<E> {
> + fn select_core(&self, _falcon: &Falcon<E>, bar: &Bar0) -> Result {
> + select_core_ga102::<E>(bar)
> + }
> +
> + fn signature_reg_fuse_version(
> + &self,
> + falcon: &Falcon<E>,
> + bar: &Bar0,
> + engine_id_mask: u16,
> + ucode_id: u8,
> + ) -> Result<u32> {
> + signature_reg_fuse_version_ga102(&falcon.dev, bar, engine_id_mask,
> ucode_id)
> + }
> +
> + fn program_brom(&self, _falcon: &Falcon<E>, bar: &Bar0, params:
> &FalconBromParams) -> Result {
> + program_brom_ga102::<E>(bar, params)
> + }
Why are those two separate functions?
