Nouveau - Dec 2025 - [PATCH v2 12/13] gpu: nova-core: add PIO support for loading firmware images

On 12/2/25 4:42 PM, Timur Tabi wrote:
> On Tue, 2025-12-02 at 16:35 -0800, John Hubbard wrote:
>> ??????? for chunk in data.chunks_exact(4) {
>> ??????????? // PANIC: `chunks_exact(4)` guarantees each chunk is
exactly 4 bytes.
>> ??????????? let word = u32::from_le_bytes(chunk.try_into().unwrap());
>> ??????????? regs::NV_PFALCON_FALCON_EMEM_DATA::default()
>> ??????????????? .set_data(word)
>> ??????????????? .write(bar, &Fsp::ID);
>> ??????? }
>>
>> ...but actually, I think your way is better, because you don't have
>> just justify an .unwrap().
>>
>> What do you think?
> 
> I agree.  Avoiding unwrap(), even with a comment, is always a good idea.
> 
>> I figured you'd enjoy this, coming as it does just one email after
I
>> wrote "never .unwrap()". haha :)
> 
> I think your code is effectively identical to mine, except that I don't
need the PANIC comment.  I
Yes. I'm changing my code over to the non-unwrap approach now. That
really is clearly better.

> suspect that in both cases, the compiler cannot tell that each chunk is
always 4 bytes and
> try_into() will never panic.  In my case, word[3] always exists and will
never panic either.  So I'm
> guess that the compiler will still emit code to check for panic.  I
don't know.
> 
Good question. Maybe someone with Rust experience can enlighten us on
that one.

thanks,
-- 
John Hubbard

> On Dec 2, 2025, at 7:46?PM, John Hubbard <jhubbard at nvidia.com>
wrote:
> 
> ?On 12/2/25 4:42 PM, Timur Tabi wrote:
>>> On Tue, 2025-12-02 at 16:35 -0800, John Hubbard wrote:
>>>         for chunk in data.chunks_exact(4) {
>>>             // PANIC: `chunks_exact(4)` guarantees each chunk is
exactly 4 bytes.
>>>             let word =
u32::from_le_bytes(chunk.try_into().unwrap());
>>>             regs::NV_PFALCON_FALCON_EMEM_DATA::default()
>>>                 .set_data(word)
>>>                 .write(bar, &Fsp::ID);
>>>         }
>>> 
>>> ...but actually, I think your way is better, because you don't
have
>>> just justify an .unwrap().
>>> 
>>> What do you think?
>> 
>> I agree.  Avoiding unwrap(), even with a comment, is always a good
idea.
>> 
>>> I figured you'd enjoy this, coming as it does just one email
after I
>>> wrote "never .unwrap()". haha :)
>> 
>> I think your code is effectively identical to mine, except that I
don't need the PANIC comment.  I
> 
> Yes. I'm changing my code over to the non-unwrap approach now. That
> really is clearly better.
> 
> 
>> suspect that in both cases, the compiler cannot tell that each chunk is
always 4 bytes and
>> try_into() will never panic.  In my case, word[3] always exists and
will never panic either.  So I'm
>> guess that the compiler will still emit code to check for panic.  I
don't know.
>> 
> 
> Good question. Maybe someone with Rust experience can enlighten us on
> that one.
If the optimizer could not eliminate the dead code, IMO that is a bug, pretty
much like how we rely on dead code elimination optimization for build_assert.

It is worth compiling and checking but I am almost certain the dead code for an
unreachable case (in this case, panicking, will be eliminated).

Thanks.